Merge pull request #20 from ti-mo/netlink-attributedecoder

Use AttributeDecoder for unmarshal operations

enum.go

@@ -295,3 +295,10 @@ const (
 )
 
 // enum ctattr_natseq is unused in the kernel source
+
+// Unused unspec constants.
+var _ = []uint8{
+	uint8(ctaHelpUnspec), uint8(ctaCountersUnspec), uint8(ctaTimestampUnspec),
+	uint8(ctaSecCtxUnspec), uint8(ctaProtoInfoTCPUnspec), uint8(ctaProtoInfoDCCPUnspec),
+	uint8(ctaProtoInfoSCTPUnspec), uint8(ctaSeqAdjUnspec), uint8(ctaSynProxyUnspec),
+}

errors.go

@@ -7,7 +7,6 @@ var (
 	errConnHasListeners = errors.New("Conn has existing listeners, open another to listen on more groups")
 	errMultipartEvent   = errors.New("received multicast event with more than one Netlink message")
 
-	errNested          = errors.New("unexpected Nested attribute")
 	errNotNested       = errors.New("need a Nested attribute to decode this structure")
 	errNeedSingleChild = errors.New("need (at least) 1 child attribute")
 	errNeedChildren    = errors.New("need (at least) 2 child attributes")
@@ -27,9 +26,8 @@ var (
 )
 
 const (
-	errUnknownEventType   = "unknown event type %d"
-	errWorkerCount        = "invalid worker count %d"
-	errWorkerReceive      = "netlink.Receive error in listenWorker %d, exiting"
-	errAttributeWrongType = "attribute type '%d' is not a %s"
-	errAttributeChild     = "child Type '%d' unknown for attribute type %s"
+	errUnknownEventType = "unknown event type %d"
+	errWorkerCount      = "invalid worker count %d"
+	errWorkerReceive    = "netlink.Receive error in listenWorker %d, exiting"
+	errAttributeChild   = "unknown attribute child Type '%d'"
 )

event.go

@@ -74,25 +74,25 @@ func (e *Event) unmarshal(nlmsg netlink.Message) error {
 
 	var err error
 
-	// Unmarshal a netlink.Message into netfilter.Attributes and Header
-	h, attrs, err := netfilter.UnmarshalNetlink(nlmsg)
+	// Obtain the nlmsg's Netfilter header and AttributeDecoder.
+	h, ad, err := netfilter.DecodeNetlink(nlmsg)
 	if err != nil {
 		return err
 	}
 
-	// Decode the header to make sure we're dealing with a Conntrack event
+	// Decode the header to make sure we're dealing with a Conntrack event.
 	err = e.Type.unmarshal(h)
 	if err != nil {
 		return err
 	}
 
-	// Unmarshal Netfilter attributes into the event's Flow or Expect entry
+	// Unmarshal Netfilter attributes into the event's Flow or Expect entry.
 	if h.SubsystemID == netfilter.NFSubsysCTNetlink {
 		e.Flow = new(Flow)
-		err = e.Flow.unmarshal(attrs)
+		err = e.Flow.unmarshal(ad)
 	} else if h.SubsystemID == netfilter.NFSubsysCTNetlinkExp {
 		e.Expect = new(Expect)
-		err = e.Expect.unmarshal(attrs)
+		err = e.Expect.unmarshal(ad)
 	}
 
 	if err != nil {

event_test.go

@@ -163,7 +163,7 @@ func TestEventUnmarshalError(t *testing.T) {
 
 	// Netlink unmarshal error
 	emptyEvent := Event{}
-	assert.EqualError(t, emptyEvent.unmarshal(netlink.Message{}), "expected at least 4 bytes in netlink message payload")
+	assert.EqualError(t, emptyEvent.unmarshal(netlink.Message{}), "unmarshaling netfilter header: expected at least 4 bytes in netlink message payload")
 
 	// EventType unmarshal error, blank SubsystemID
 	assert.EqualError(t, emptyEvent.unmarshal(netlink.Message{

expect.go

@@ -35,34 +35,24 @@ type ExpectNAT struct {
 }
 
 // unmarshal unmarshals a netfilter.Attribute into an ExpectNAT.
-func (en *ExpectNAT) unmarshal(attr netfilter.Attribute) error {
+func (en *ExpectNAT) unmarshal(ad *netlink.AttributeDecoder) error {
 
-	if expectType(attr.Type) != ctaExpectNAT {
-		return fmt.Errorf(errAttributeWrongType, attr.Type, ctaExpectNAT)
-	}
-
-	if !attr.Nested {
-		return errors.Wrap(errNotNested, opUnExpectNAT)
-	}
-
-	if len(attr.Children) == 0 {
+	if ad.Len() == 0 {
 		return errors.Wrap(errNeedSingleChild, opUnExpectNAT)
 	}
 
-	for _, iattr := range attr.Children {
-		switch expectNATType(iattr.Type) {
+	for ad.Next() {
+		switch expectNATType(ad.Type()) {
 		case ctaExpectNATDir:
-			en.Direction = iattr.Uint32() == 1
+			en.Direction = ad.Uint32() == 1
 		case ctaExpectNATTuple:
-			if err := en.Tuple.unmarshal(iattr); err != nil {
-				return err
-			}
+			ad.Nested(en.Tuple.unmarshal)
 		default:
-			return errors.Wrap(fmt.Errorf(errAttributeChild, iattr.Type, ctaExpectNAT), opUnExpectNAT)
+			return errors.Wrap(fmt.Errorf(errAttributeChild, ad.Type()), opUnExpectNAT)
 		}
 	}
 
-	return nil
+	return ad.Err()
 }
 
 func (en ExpectNAT) marshal() (netfilter.Attribute, error) {
@@ -86,46 +76,48 @@ func (en ExpectNAT) marshal() (netfilter.Attribute, error) {
 }
 
 // unmarshal unmarshals a list of netfilter.Attributes into an Expect structure.
-func (ex *Expect) unmarshal(attrs []netfilter.Attribute) error {
-
-	for _, attr := range attrs {
-
-		switch at := expectType(attr.Type); at {
+func (ex *Expect) unmarshal(ad *netlink.AttributeDecoder) error {
 
+	for ad.Next() {
+		switch at := expectType(ad.Type()); at {
 		case ctaExpectMaster:
-			if err := ex.TupleMaster.unmarshal(attr); err != nil {
-				return err
+			if !nestedFlag(ad.TypeFlags()) {
+				return errors.Wrap(errNotNested, opUnTup)
 			}
+			ad.Nested(ex.TupleMaster.unmarshal)
 		case ctaExpectTuple:
-			if err := ex.Tuple.unmarshal(attr); err != nil {
-				return err
+			if !nestedFlag(ad.TypeFlags()) {
+				return errors.Wrap(errNotNested, opUnTup)
 			}
+			ad.Nested(ex.Tuple.unmarshal)
 		case ctaExpectMask:
-			if err := ex.Mask.unmarshal(attr); err != nil {
-				return err
+			if !nestedFlag(ad.TypeFlags()) {
+				return errors.Wrap(errNotNested, opUnTup)
 			}
+			ad.Nested(ex.Mask.unmarshal)
 		case ctaExpectTimeout:
-			ex.Timeout = attr.Uint32()
+			ex.Timeout = ad.Uint32()
 		case ctaExpectID:
-			ex.ID = attr.Uint32()
+			ex.ID = ad.Uint32()
 		case ctaExpectHelpName:
-			ex.HelpName = string(attr.Data)
+			ex.HelpName = ad.String()
 		case ctaExpectZone:
-			ex.Zone = attr.Uint16()
+			ex.Zone = ad.Uint16()
 		case ctaExpectFlags:
-			ex.Flags = attr.Uint32()
+			ex.Flags = ad.Uint32()
 		case ctaExpectClass:
-			ex.Class = attr.Uint32()
+			ex.Class = ad.Uint32()
 		case ctaExpectNAT:
-			if err := ex.NAT.unmarshal(attr); err != nil {
-				return err
+			if !nestedFlag(ad.TypeFlags()) {
+				return errors.Wrap(errNotNested, opUnExpectNAT)
 			}
+			ad.Nested(ex.NAT.unmarshal)
 		case ctaExpectFN:
-			ex.Function = string(attr.Data)
+			ex.Function = ad.String()
 		}
 	}
 
-	return nil
+	return ad.Err()
 }
 
 func (ex Expect) marshal() ([]netfilter.Attribute, error) {
@@ -194,12 +186,12 @@ func unmarshalExpect(nlm netlink.Message) (Expect, error) {
 
 	var ex Expect
 
-	_, nfa, err := netfilter.UnmarshalNetlink(nlm)
+	_, ad, err := netfilter.DecodeNetlink(nlm)
 	if err != nil {
 		return ex, err
 	}
 
-	err = ex.unmarshal(nfa)
+	err = ex.unmarshal(ad)
 	if err != nil {
 		return ex, err
 	}

expect_test.go

@@ -273,15 +273,16 @@ func TestExpectUnmarshal(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 
 			var ex Expect
-			err := ex.unmarshal(tt.attrs)
+			err := ex.unmarshal(mustDecodeAttributes(tt.attrs))
 
-			if err != nil || tt.err != nil {
+			if tt.err != nil {
 				require.Error(t, err)
-				require.Error(t, tt.err)
 				require.EqualError(t, err, tt.err.Error())
 				return
 			}
 
+			require.NoError(t, err)
+
 			if diff := cmp.Diff(tt.exp, ex); diff != "" {
 				t.Fatalf("unexpected unmarshal (-want +got):\n%s", diff)
 			}
@@ -291,7 +292,7 @@ func TestExpectUnmarshal(t *testing.T) {
 	for _, tt := range corpusExpectUnmarshalError {
 		t.Run(tt.name, func(t *testing.T) {
 			var ex Expect
-			assert.EqualError(t, ex.unmarshal([]netfilter.Attribute{tt.nfa}), tt.errStr)
+			assert.EqualError(t, ex.unmarshal(mustDecodeAttributes([]netfilter.Attribute{tt.nfa})), tt.errStr)
 		})
 	}
 }
@@ -395,76 +396,32 @@ func TestExpectMarshal(t *testing.T) {
 
 var corpusExpectNAT = []struct {
 	name string
-	attr netfilter.Attribute
+	attr []netfilter.Attribute
 	enat ExpectNAT
 	err  error
 }{
 	{
 		name: "simple direction, tuple unmarshal",
-		attr: netfilter.Attribute{
-			Type:   uint16(ctaExpectNAT),
-			Nested: true,
-			Children: []netfilter.Attribute{
-				{
-					Type: uint16(ctaExpectNATDir),
-					Data: []byte{0x00, 0x00, 0x00, 0x01},
-				},
-				{
-					Type:     uint16(ctaExpectNATTuple),
-					Nested:   true,
-					Children: nfaIPPT,
-				},
+		attr: []netfilter.Attribute{
+			{
+				Type: uint16(ctaExpectNATDir),
+				Data: []byte{0x00, 0x00, 0x00, 0x01},
+			},
+			{
+				Type:     uint16(ctaExpectNATTuple),
+				Nested:   true,
+				Children: nfaIPPT,
 			},
 		},
 		enat: ExpectNAT{
 			Direction: true,
 			Tuple:     flowIPPT,
 		},
 	},
-	{
-		name: "error bad tuple",
-		attr: netfilter.Attribute{
-			Type:   uint16(ctaExpectNAT),
-			Nested: true,
-			Children: []netfilter.Attribute{
-				{
-					Type: uint16(ctaExpectNATDir),
-					Data: []byte{0x00, 0x00, 0x00, 0x00},
-				},
-				{
-					Type: uint16(ctaExpectNATTuple),
-				},
-			},
-		},
-		err: errors.New("Tuple unmarshal: need a Nested attribute to decode this structure"),
-	},
 	{
 		name: "error unknown type",
-		attr: netfilter.Attribute{Type: 255},
-		err:  fmt.Errorf(errAttributeWrongType, 255, ctaExpectNAT),
-	},
-	{
-		name: "error not nested",
-		attr: netfilter.Attribute{Type: uint16(ctaExpectNAT)},
-		err:  errors.Wrap(errNotNested, opUnExpectNAT),
-	},
-	{
-		name: "error no children",
-		attr: netfilter.Attribute{Type: uint16(ctaExpectNAT), Nested: true},
-		err:  errors.Wrap(errNeedSingleChild, opUnExpectNAT),
-	},
-	{
-		name: "error unknown child type",
-		attr: netfilter.Attribute{
-			Type:   uint16(ctaExpectNAT),
-			Nested: true,
-			Children: []netfilter.Attribute{
-				{
-					Type: 255,
-				},
-			},
-		},
-		err: errors.Wrap(fmt.Errorf(errAttributeChild, 255, ctaExpectNAT), opUnExpectNAT),
+		attr: []netfilter.Attribute{{Type: 255}},
+		err:  errors.Wrap(fmt.Errorf(errAttributeChild, 255), opUnExpectNAT),
 	},
 }
 
@@ -474,15 +431,16 @@ func TestExpectNATUnmarshal(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 
 			var enat ExpectNAT
-			err := enat.unmarshal(tt.attr)
+			err := enat.unmarshal(mustDecodeAttributes(tt.attr))
 
-			if err != nil || tt.err != nil {
+			if tt.err != nil {
 				require.Error(t, err)
-				require.Error(t, tt.err)
 				require.EqualError(t, err, tt.err.Error())
 				return
 			}
 
+			require.NoError(t, err)
+
 			if diff := cmp.Diff(tt.enat, enat); diff != "" {
 				t.Fatalf("unexpected unmarshal (-want +got):\n%s", diff)
 			}
@@ -534,7 +492,6 @@ func BenchmarkExpectUnmarshal(b *testing.B) {
 	b.ReportAllocs()
 
 	var tests []netfilter.Attribute
-	var ex Expect
 
 	// Collect all tests from corpus that aren't expected to fail
 	for _, test := range corpusExpect {
@@ -543,7 +500,14 @@ func BenchmarkExpectUnmarshal(b *testing.B) {
 		}
 	}
 
+	// Marshal these netfilter attributes and return netlink.AttributeDecoder.
+	ad := mustDecodeAttributes(tests)
+
 	for n := 0; n < b.N; n++ {
-		_ = ex.unmarshal(tests)
+		// Make a new copy of the AD to avoid reinstantiation.
+		iad := ad
+
+		var ex Expect
+		_ = ex.unmarshal(iad)
 	}
 }