diff options
Diffstat (limited to 'vendor/github.com/google/gopacket/CONTRIBUTING.md')
| -rw-r--r-- | vendor/github.com/google/gopacket/CONTRIBUTING.md | 215 |
1 files changed, 215 insertions, 0 deletions
diff --git a/vendor/github.com/google/gopacket/CONTRIBUTING.md b/vendor/github.com/google/gopacket/CONTRIBUTING.md new file mode 100644 index 0000000..99ab7a2 --- /dev/null +++ b/vendor/github.com/google/gopacket/CONTRIBUTING.md @@ -0,0 +1,215 @@ +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. |