code review 13832043: go.net/ipv4: Add Flags field to Header

ipv4/header.go

 // Copyright 2012 The Go Authors.  All rights reserved.
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
 package ipv4
 
 import (
         "errors"
         "fmt"
         "net"
@@ skipping 35 matching lines @@
         posTTL                  = 8  // time-to-live
         posProtocol             = 9  // next protocol
         posChecksum             = 10 // checksum
         posSrc                  = 12 // source address
         posDst                  = 16 // destination address
 )
 
 type HeaderFlags int
 
 const (
-        DontFragment  HeaderFlags = 1 << iota // don't Fragment flag
-        MoreFragments                         // more Fragments flag
+        MoreFragments HeaderFlags = 1 << iota // more fragments flag
+        DontFragment                          // don't fragment flag
 )
 
 // A Header represents an IPv4 header.
 type Header struct {
         Version  int         // protocol version
         Len      int         // header length
         TOS      int         // type-of-service
         TotalLen int         // packet total length
         ID       int         // identification
         Flags    HeaderFlags // flags
         FragOff  int         // fragment offset
         TTL      int         // time-to-live
         Protocol int         // next protocol
         Checksum int         // checksum
         Src      net.IP      // source address
         Dst      net.IP      // destination address
         Options  []byte      // options, extension headers
 }
 
 func (h *Header) String() string {
         if h == nil {
                 return "<nil>"
         }
-        return fmt.Sprintf("ver: %v, hdrlen: %v, tos: %#x, totallen: %v, id: %#x, fragoff: %#x, ttl: %v, proto: %v, cksum: %#x, src: %v, dst: %v", h.Version, h.Len, h.TOS, h.TotalLen, h.ID, h.Flags, h.FragOff, h.TTL, h.Protocol, h.Checksum, h.Src, h.Dst)

[mikio, 2013/10/07 16:03:53]

go tool vet says:
header.go:81: wrong number of args for format in Sprintf call: 11 needed but 12
args

[marant, 2013/10/07 17:54:36]

Done.

+        return fmt.Sprintf("ver: %v, hdrlen: %v, tos: %#x, totallen: %v, id: %#x, flags: %#x, fragoff: %#x, ttl: %v, proto: %v, cksum: %#x, src: %v, dst: %v", h.Version, h.Len, h.TOS, h.TotalLen, h.ID, h.Flags, h.FragOff, h.TTL, h.Protocol, h.Checksum, h.Src, h.Dst)
 }
 
 // Please refer to the online manual; IP(4) on Darwin, FreeBSD and
 // OpenBSD.  IP(7) on Linux.
 const supportsNewIPInput = runtime.GOOS == "linux" || runtime.GOOS == "openbsd"
 
 // Marshal returns the binary encoding of the IPv4 header h.
 func (h *Header) Marshal() ([]byte, error) {
         if h == nil {
                 return nil, syscall.EINVAL
         }
         if h.Len < HeaderLen {
                 return nil, errHeaderTooShort
         }
         hdrlen := HeaderLen + len(h.Options)
         b := make([]byte, hdrlen)
         b[0] = byte(Version<<4 | (hdrlen >> 2 & 0x0f))
         b[posTOS] = byte(h.TOS)
-        flagsAndFragOff := (h.FragOff & 0x1fff) | int(h.Flags<<14)

[mikio, 2013/10/07 16:03:53]

h.Flags<<14 doesn't look correct to me.
Does your test work correctly when we set ipv4.MoreFragments?

const (
        MoreFragments HeaderFlags = 1 << iota // more Fragments flag
        DontFragment                          // don't Fragment flag
)

and

       flagsAndFragOff := (h.FragOff & 0x1fff) | int(h.Flags<<13)
       h.Flags = HeaderFlags(h.FragOff&0xe000) >> 13

?

[marant, 2013/10/07 17:54:36]

Yes, you are correct. I had the flags defined in the wrong order which caused
the confusion. 

+        flagsAndFragOff := (h.FragOff & 0x1fff) | int(h.Flags<<13)
         if supportsNewIPInput {
                 b[posTotalLen], b[posTotalLen+1] = byte(h.TotalLen>>8), byte(h.TotalLen)
                 b[posFragOff], b[posFragOff+1] = byte(flagsAndFragOff>>8), byte(flagsAndFragOff)
         } else {
                 *(*uint16)(unsafe.Pointer(&b[posTotalLen : posTotalLen+1][0])) = uint16(h.TotalLen)
                 *(*uint16)(unsafe.Pointer(&b[posFragOff : posFragOff+1][0])) = uint16(flagsAndFragOff)
         }
         b[posID], b[posID+1] = byte(h.ID>>8), byte(h.ID)
         b[posTTL] = byte(h.TTL)
         b[posProtocol] = byte(h.Protocol)
@@ skipping 26 matching lines @@
         h.Len = hdrlen
         h.TOS = int(b[posTOS])
         if supportsNewIPInput {
                 h.TotalLen = int(b[posTotalLen])<<8 | int(b[posTotalLen+1])
                 h.FragOff = int(b[posFragOff])<<8 | int(b[posFragOff+1])
         } else {
                 h.TotalLen = int(*(*uint16)(unsafe.Pointer(&b[posTotalLen : posTotalLen+1][0])))
                 h.TotalLen += hdrlen
                 h.FragOff = int(*(*uint16)(unsafe.Pointer(&b[posFragOff : posFragOff+1][0])))
         }
-        h.Flags = HeaderFlags(h.FragOff&0xe000) >> 14
+        h.Flags = HeaderFlags(h.FragOff&0xe000) >> 13
         h.FragOff = h.FragOff & 0x1fff
         h.ID = int(b[posID])<<8 | int(b[posID+1])
         h.TTL = int(b[posTTL])
         h.Protocol = int(b[posProtocol])
         h.Checksum = int(b[posChecksum])<<8 | int(b[posChecksum+1])
         h.Src = net.IPv4(b[posSrc], b[posSrc+1], b[posSrc+2], b[posSrc+3])
         h.Dst = net.IPv4(b[posDst], b[posDst+1], b[posDst+2], b[posDst+3])
         if hdrlen-HeaderLen > 0 {
                 h.Options = make([]byte, hdrlen-HeaderLen)
                 copy(h.Options, b[HeaderLen:])
         }
         return h, nil
 }