216 lines
8.7 KiB
Markdown
216 lines
8.7 KiB
Markdown
Contributing To gopacket
|
|
========================
|
|
|
|
So you've got some code and you'd like it to be part of gopacket... wonderful!
|
|
We're happy to accept contributions, whether they're fixes to old protocols, new
|
|
protocols entirely, or anything else you think would improve the gopacket
|
|
library. This document is designed to help you to do just that.
|
|
|
|
The first section deals with the plumbing: how to actually get a change
|
|
submitted.
|
|
|
|
The second section deals with coding style... Go is great in that it
|
|
has a uniform style implemented by 'go fmt', but there's still some decisions
|
|
we've made that go above and beyond, and if you follow them, they won't come up
|
|
in your code review.
|
|
|
|
The third section deals with some of the implementation decisions we've made,
|
|
which may help you to understand the current code and which we may ask you to
|
|
conform to (or provide compelling reasons for ignoring).
|
|
|
|
Overall, we hope this document will help you to understand our system and write
|
|
great code which fits in, and help us to turn around on your code review quickly
|
|
so the code can make it into the master branch as quickly as possible.
|
|
|
|
|
|
How To Submit Code
|
|
------------------
|
|
|
|
We use github.com's Pull Request feature to receive code contributions from
|
|
external contributors. See
|
|
https://help.github.com/articles/creating-a-pull-request/ for details on
|
|
how to create a request.
|
|
|
|
Also, there's a local script `gc` in the base directory of GoPacket that
|
|
runs a local set of checks, which should give you relatively high confidence
|
|
that your pull won't fail github pull checks.
|
|
|
|
```sh
|
|
go get github.com/google/gopacket
|
|
cd $GOROOT/src/pkg/github.com/google/gopacket
|
|
git checkout -b <mynewfeature> # create a new branch to work from
|
|
... code code code ...
|
|
./gc # Run this to do local commits, it performs a number of checks
|
|
```
|
|
|
|
To sum up:
|
|
|
|
* DO
|
|
+ Pull down the latest version.
|
|
+ Make a feature-specific branch.
|
|
+ Code using the style and methods discussed in the rest of this document.
|
|
+ Use the ./gc command to do local commits or check correctness.
|
|
+ Push your new feature branch up to github.com, as a pull request.
|
|
+ Handle comments and requests from reviewers, pushing new commits up to
|
|
your feature branch as problems are addressed.
|
|
+ Put interesting comments and discussions into commit comments.
|
|
* DON'T
|
|
+ Push to someone else's branch without their permission.
|
|
|
|
|
|
Coding Style
|
|
------------
|
|
|
|
* Go code must be run through `go fmt`, `go vet`, and `golint`
|
|
* Follow http://golang.org/doc/effective_go.html as much as possible.
|
|
+ In particular, http://golang.org/doc/effective_go.html#mixed-caps. Enums
|
|
should be be CamelCase, with acronyms capitalized (TCPSourcePort, vs.
|
|
TcpSourcePort or TCP_SOURCE_PORT).
|
|
* Bonus points for giving enum types a String() field.
|
|
* Any exported types or functions should have commentary
|
|
(http://golang.org/doc/effective_go.html#commentary)
|
|
|
|
|
|
Coding Methods And Implementation Notes
|
|
---------------------------------------
|
|
|
|
### Error Handling
|
|
|
|
Many times, you'll be decoding a protocol and run across something bad, a packet
|
|
corruption or the like. How do you handle this? First off, ALWAYS report the
|
|
error. You can do this either by returning the error from the decode() function
|
|
(most common), or if you're up for it you can implement and add an ErrorLayer
|
|
through the packet builder (the first method is a simple shortcut that does
|
|
exactly this, then stops any future decoding).
|
|
|
|
Often, you'll already have decode some part of your protocol by the time you hit
|
|
your error. Use your own discretion to determine whether the stuff you've
|
|
already decoded should be returned to the caller or not:
|
|
|
|
```go
|
|
func decodeMyProtocol(data []byte, p gopacket.PacketBuilder) error {
|
|
prot := &MyProtocol{}
|
|
if len(data) < 10 {
|
|
// This error occurred before we did ANYTHING, so there's nothing in my
|
|
// protocol that the caller could possibly want. Just return the error.
|
|
return fmt.Errorf("Length %d less than 10", len(data))
|
|
}
|
|
prot.ImportantField1 = data[:5]
|
|
prot.ImportantField2 = data[5:10]
|
|
// At this point, we've already got enough information in 'prot' to
|
|
// warrant returning it to the caller, so we'll add it now.
|
|
p.AddLayer(prot)
|
|
if len(data) < 15 {
|
|
// We encountered an error later in the packet, but the caller already
|
|
// has the important info we've gleaned so far.
|
|
return fmt.Errorf("Length %d less than 15", len(data))
|
|
}
|
|
prot.ImportantField3 = data[10:15]
|
|
return nil // We've already added the layer, we can just return success.
|
|
}
|
|
```
|
|
|
|
In general, our code follows the approach of returning the first error it
|
|
encounters. In general, we don't trust any bytes after the first error we see.
|
|
|
|
### What Is A Layer?
|
|
|
|
The definition of a layer is up to the discretion of the coder. It should be
|
|
something important enough that it's actually useful to the caller (IE: every
|
|
TLV value should probably NOT be a layer). However, it can be more granular
|
|
than a single protocol... IPv6 and SCTP both implement many layers to handle the
|
|
various parts of the protocol. Use your best judgement, and prepare to defend
|
|
your decisions during code review. ;)
|
|
|
|
### Performance
|
|
|
|
We strive to make gopacket as fast as possible while still providing lots of
|
|
features. In general, this means:
|
|
|
|
* Focus performance tuning on common protocols (IP4/6, TCP, etc), and optimize
|
|
others on an as-needed basis (tons of MPLS on your network? Time to optimize
|
|
MPLS!)
|
|
* Use fast operations. See the toplevel benchmark_test for benchmarks of some
|
|
of Go's underlying features and types.
|
|
* Test your performance changes! You should use the ./gc script's --benchmark
|
|
flag to submit any performance-related changes. Use pcap/gopacket_benchmark
|
|
to test your change against a PCAP file based on your traffic patterns.
|
|
* Don't be TOO hacky. Sometimes, removing an unused struct from a field causes
|
|
a huge performance hit, due to the way that Go currently handles its segmented
|
|
stack... don't be afraid to clean it up anyway. We'll trust the Go compiler
|
|
to get good enough over time to handle this. Also, this type of
|
|
compiler-specific optimization is very fragile; someone adding a field to an
|
|
entirely different struct elsewhere in the codebase could reverse any gains
|
|
you might achieve by aligning your allocations.
|
|
* Try to minimize memory allocations. If possible, use []byte to reference
|
|
pieces of the input, instead of using string, which requires copying the bytes
|
|
into a new memory allocation.
|
|
* Think hard about what should be evaluated lazily vs. not. In general, a
|
|
layer's struct should almost exactly mirror the layer's frame. Anything
|
|
that's more interesting should be a function. This may not always be
|
|
possible, but it's a good rule of thumb.
|
|
* Don't fear micro-optimizations. With the above in mind, we welcome
|
|
micro-optimizations that we think will have positive/neutral impacts on the
|
|
majority of workloads. A prime example of this is pre-allocating certain
|
|
structs within a larger one:
|
|
|
|
```go
|
|
type MyProtocol struct {
|
|
// Most packets have 1-4 of VeryCommon, so we preallocate it here.
|
|
initialAllocation [4]uint32
|
|
VeryCommon []uint32
|
|
}
|
|
|
|
func decodeMyProtocol(data []byte, p gopacket.PacketBuilder) error {
|
|
prot := &MyProtocol{}
|
|
prot.VeryCommon = proto.initialAllocation[:0]
|
|
for len(data) > 4 {
|
|
field := binary.BigEndian.Uint32(data[:4])
|
|
data = data[4:]
|
|
// Since we're using the underlying initialAllocation, we won't need to
|
|
// allocate new memory for the following append unless we more than 16
|
|
// bytes of data, which should be the uncommon case.
|
|
prot.VeryCommon = append(prot.VeryCommon, field)
|
|
}
|
|
p.AddLayer(prot)
|
|
if len(data) > 0 {
|
|
return fmt.Errorf("MyProtocol packet has %d bytes left after decoding", len(data))
|
|
}
|
|
return nil
|
|
}
|
|
```
|
|
|
|
### Slices And Data
|
|
|
|
If you're pulling a slice from the data you're decoding, don't copy it. Just
|
|
use the slice itself.
|
|
|
|
```go
|
|
type MyProtocol struct {
|
|
A, B net.IP
|
|
}
|
|
func decodeMyProtocol(data []byte, p gopacket.PacketBuilder) error {
|
|
p.AddLayer(&MyProtocol{
|
|
A: data[:4],
|
|
B: data[4:8],
|
|
})
|
|
return nil
|
|
}
|
|
```
|
|
|
|
The caller has already agreed, by using this library, that they won't modify the
|
|
set of bytes they pass in to the decoder, or the library has already copied the
|
|
set of bytes to a read-only location. See DecodeOptions.NoCopy for more
|
|
information.
|
|
|
|
### Enums/Types
|
|
|
|
If a protocol has an integer field (uint8, uint16, etc) with a couple of known
|
|
values that mean something special, make it a type. This allows us to do really
|
|
nice things like adding a String() function to them, so we can more easily
|
|
display those to users. Check out layers/enums.go for one example, as well as
|
|
layers/icmp.go for layer-specific enums.
|
|
|
|
When naming things, try for descriptiveness over suscinctness. For example,
|
|
choose DNSResponseRecord over DNSRR.
|