github.com/gopacket/gopacket@v1.1.0/CONTRIBUTING.md (about) 1 Contributing To gopacket 2 ======================== 3 4 So you've got some code and you'd like it to be part of gopacket... wonderful! 5 We're happy to accept contributions, whether they're fixes to old protocols, new 6 protocols entirely, or anything else you think would improve the gopacket 7 library. This document is designed to help you to do just that. 8 9 The first section deals with the plumbing: how to actually get a change 10 submitted. 11 12 The second section deals with coding style... Go is great in that it 13 has a uniform style implemented by 'go fmt', but there's still some decisions 14 we've made that go above and beyond, and if you follow them, they won't come up 15 in your code review. 16 17 The third section deals with some of the implementation decisions we've made, 18 which may help you to understand the current code and which we may ask you to 19 conform to (or provide compelling reasons for ignoring). 20 21 Overall, we hope this document will help you to understand our system and write 22 great code which fits in, and help us to turn around on your code review quickly 23 so the code can make it into the master branch as quickly as possible. 24 25 26 How To Submit Code 27 ------------------ 28 29 We use github.com's Pull Request feature to receive code contributions from 30 external contributors. See 31 https://help.github.com/articles/creating-a-pull-request/ for details on 32 how to create a request. 33 34 Also, there's a local script `gc` in the base directory of GoPacket that 35 runs a local set of checks, which should give you relatively high confidence 36 that your pull won't fail github pull checks. 37 38 ```sh 39 go get github.com/gopacket/gopacket 40 cd $GOROOT/src/pkg/github.com/gopacket/gopacket 41 git checkout -b <mynewfeature> # create a new branch to work from 42 ... code code code ... 43 ./gc # Run this to do local commits, it performs a number of checks 44 ``` 45 46 To sum up: 47 48 * DO 49 + Pull down the latest version. 50 + Make a feature-specific branch. 51 + Code using the style and methods discussed in the rest of this document. 52 + Use the ./gc command to do local commits or check correctness. 53 + Push your new feature branch up to github.com, as a pull request. 54 + Handle comments and requests from reviewers, pushing new commits up to 55 your feature branch as problems are addressed. 56 + Put interesting comments and discussions into commit comments. 57 * DON'T 58 + Push to someone else's branch without their permission. 59 60 61 Coding Style 62 ------------ 63 64 * Go code must be run through `go fmt`, `go vet`, and `golint` 65 * Follow http://golang.org/doc/effective_go.html as much as possible. 66 + In particular, http://golang.org/doc/effective_go.html#mixed-caps. Enums 67 should be be CamelCase, with acronyms capitalized (TCPSourcePort, vs. 68 TcpSourcePort or TCP_SOURCE_PORT). 69 * Bonus points for giving enum types a String() field. 70 * Any exported types or functions should have commentary 71 (http://golang.org/doc/effective_go.html#commentary) 72 73 74 Coding Methods And Implementation Notes 75 --------------------------------------- 76 77 ### Error Handling 78 79 Many times, you'll be decoding a protocol and run across something bad, a packet 80 corruption or the like. How do you handle this? First off, ALWAYS report the 81 error. You can do this either by returning the error from the decode() function 82 (most common), or if you're up for it you can implement and add an ErrorLayer 83 through the packet builder (the first method is a simple shortcut that does 84 exactly this, then stops any future decoding). 85 86 Often, you'll already have decode some part of your protocol by the time you hit 87 your error. Use your own discretion to determine whether the stuff you've 88 already decoded should be returned to the caller or not: 89 90 ```go 91 func decodeMyProtocol(data []byte, p gopacket.PacketBuilder) error { 92 prot := &MyProtocol{} 93 if len(data) < 10 { 94 // This error occurred before we did ANYTHING, so there's nothing in my 95 // protocol that the caller could possibly want. Just return the error. 96 return fmt.Errorf("Length %d less than 10", len(data)) 97 } 98 prot.ImportantField1 = data[:5] 99 prot.ImportantField2 = data[5:10] 100 // At this point, we've already got enough information in 'prot' to 101 // warrant returning it to the caller, so we'll add it now. 102 p.AddLayer(prot) 103 if len(data) < 15 { 104 // We encountered an error later in the packet, but the caller already 105 // has the important info we've gleaned so far. 106 return fmt.Errorf("Length %d less than 15", len(data)) 107 } 108 prot.ImportantField3 = data[10:15] 109 return nil // We've already added the layer, we can just return success. 110 } 111 ``` 112 113 In general, our code follows the approach of returning the first error it 114 encounters. In general, we don't trust any bytes after the first error we see. 115 116 ### What Is A Layer? 117 118 The definition of a layer is up to the discretion of the coder. It should be 119 something important enough that it's actually useful to the caller (IE: every 120 TLV value should probably NOT be a layer). However, it can be more granular 121 than a single protocol... IPv6 and SCTP both implement many layers to handle the 122 various parts of the protocol. Use your best judgement, and prepare to defend 123 your decisions during code review. ;) 124 125 ### Performance 126 127 We strive to make gopacket as fast as possible while still providing lots of 128 features. In general, this means: 129 130 * Focus performance tuning on common protocols (IP4/6, TCP, etc), and optimize 131 others on an as-needed basis (tons of MPLS on your network? Time to optimize 132 MPLS!) 133 * Use fast operations. See the toplevel benchmark_test for benchmarks of some 134 of Go's underlying features and types. 135 * Test your performance changes! You should use the ./gc script's --benchmark 136 flag to submit any performance-related changes. Use pcap/gopacket_benchmark 137 to test your change against a PCAP file based on your traffic patterns. 138 * Don't be TOO hacky. Sometimes, removing an unused struct from a field causes 139 a huge performance hit, due to the way that Go currently handles its segmented 140 stack... don't be afraid to clean it up anyway. We'll trust the Go compiler 141 to get good enough over time to handle this. Also, this type of 142 compiler-specific optimization is very fragile; someone adding a field to an 143 entirely different struct elsewhere in the codebase could reverse any gains 144 you might achieve by aligning your allocations. 145 * Try to minimize memory allocations. If possible, use []byte to reference 146 pieces of the input, instead of using string, which requires copying the bytes 147 into a new memory allocation. 148 * Think hard about what should be evaluated lazily vs. not. In general, a 149 layer's struct should almost exactly mirror the layer's frame. Anything 150 that's more interesting should be a function. This may not always be 151 possible, but it's a good rule of thumb. 152 * Don't fear micro-optimizations. With the above in mind, we welcome 153 micro-optimizations that we think will have positive/neutral impacts on the 154 majority of workloads. A prime example of this is pre-allocating certain 155 structs within a larger one: 156 157 ```go 158 type MyProtocol struct { 159 // Most packets have 1-4 of VeryCommon, so we preallocate it here. 160 initialAllocation [4]uint32 161 VeryCommon []uint32 162 } 163 164 func decodeMyProtocol(data []byte, p gopacket.PacketBuilder) error { 165 prot := &MyProtocol{} 166 prot.VeryCommon = proto.initialAllocation[:0] 167 for len(data) > 4 { 168 field := binary.BigEndian.Uint32(data[:4]) 169 data = data[4:] 170 // Since we're using the underlying initialAllocation, we won't need to 171 // allocate new memory for the following append unless we more than 16 172 // bytes of data, which should be the uncommon case. 173 prot.VeryCommon = append(prot.VeryCommon, field) 174 } 175 p.AddLayer(prot) 176 if len(data) > 0 { 177 return fmt.Errorf("MyProtocol packet has %d bytes left after decoding", len(data)) 178 } 179 return nil 180 } 181 ``` 182 183 ### Slices And Data 184 185 If you're pulling a slice from the data you're decoding, don't copy it. Just 186 use the slice itself. 187 188 ```go 189 type MyProtocol struct { 190 A, B net.IP 191 } 192 func decodeMyProtocol(data []byte, p gopacket.PacketBuilder) error { 193 p.AddLayer(&MyProtocol{ 194 A: data[:4], 195 B: data[4:8], 196 }) 197 return nil 198 } 199 ``` 200 201 The caller has already agreed, by using this library, that they won't modify the 202 set of bytes they pass in to the decoder, or the library has already copied the 203 set of bytes to a read-only location. See DecodeOptions.NoCopy for more 204 information. 205 206 ### Enums/Types 207 208 If a protocol has an integer field (uint8, uint16, etc) with a couple of known 209 values that mean something special, make it a type. This allows us to do really 210 nice things like adding a String() function to them, so we can more easily 211 display those to users. Check out layers/enums.go for one example, as well as 212 layers/icmp.go for layer-specific enums. 213 214 When naming things, try for descriptiveness over suscinctness. For example, 215 choose DNSResponseRecord over DNSRR.