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.