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.
gopacket uses the code.google.com Git version control system. If you want to make a new change, you'll first have to get our code:
go get code.google.com/p/gopacket
cd $GOROOT/src/pkg/code.google.com/p/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
... code code code ...
./gc --benchmark # Run this whenever your commit could affect performance
Nw that you're in the gopacket code directory, you can start making your initial change. PLEASE make sure you're using a new branch to develop whatever feature you're working on.
Once you've got your code to a place where you're ready to have us look at it, send an email to gopacket@googlegroups.com, detailing your change. We'll add you as a committer, and you can upload your feature branch to code.google.com. From there, the other folks working on gopacket can give you code reviews with the code.google.com code review functionality.
The code review will generally be either emails or line-by-line reviews via code.google.com. One or more folks might review your code. The review should be considered "complete" when at least one of the project Owners (see https://code.google.com/p/gopacket/people/list) gives you permission to merge to master. At that point, you can merge to master yourself, or you can have one of the other committers/owners do it for you.
When doing the final merge, please try to capture any interesting comments or discussions that came up in code review. This will help future contributors be able to find and reference those discussions later on.
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.
- Send an email asking us to make you a committer (if you're new).
- Push your new feature branch up to code.google.com.
- Handle comments and requests from reviewers, pushing new commits up to your feature branch as problems are addressed.
- Get approval from a project Owner to merge to master.
- Merge yourself, or have another Committer/Owner do it for you.
- Put interesting comments and discussions into commit comments.
- DON'T
- Push directly to master.
- Push to someone else's branch without their permission.
- Merge your own code to master without sign-off from others on the project.
- Rebase (please merge)
- OPTIONAL
- Review others' code as it comes in (politely :)
- Keep contributing!
- Go code must be run through 'go fmt'.
- 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)
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:
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.
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. ;)
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:
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 }
If you're pulling a slice from the data you're decoding, don't copy it. Just use the slice itself.
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.
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.