Fix match converstions in OF driver

This commit fixes critical issues in the match conversion process, and
adds a test for that.

openflow/driver.go

@@ -235,43 +235,43 @@ func (d *of10Driver) convAction(a nom.Action) (of10.ActionHeader, error) {
 func (d *of10Driver) nomMatch(m of10.Match) (nom.Match, error) {
 	nm := nom.Match{}
 	wc := of10.FlowWildcards(m.Wildcards())
-	if wc&of10.PFW_IN_PORT != 0 {
+	if wc&of10.PFW_IN_PORT == 0 {
 		ofp := m.InPort()
 		nomp, ok := d.ofPorts[ofp]
 		if !ok {
 			return nom.Match{}, fmt.Errorf("of10Driver: cannot find port %v", ofp)
 		}
 		nm.Fields = append(nm.Fields, nom.InPort(nomp.UID()))
 	}
-	if wc&of10.PFW_DL_SRC != 0 {
+	if wc&of10.PFW_DL_SRC == 0 {
 		nm.Fields = append(nm.Fields, nom.EthSrc{
 			Addr: m.DlSrc(),
-			Mask: nom.MaskAllMAC,
+			Mask: nom.MaskNoneMAC,
 		})
 	}
-	if wc&of10.PFW_DL_DST != 0 {
+	if wc&of10.PFW_DL_DST == 0 {
 		nm.Fields = append(nm.Fields, nom.EthDst{
 			Addr: m.DlDst(),
-			Mask: nom.MaskAllMAC,
+			Mask: nom.MaskNoneMAC,
 		})
 	}
-	if wc&of10.PFW_DL_TYPE != 0 {
+	if wc&of10.PFW_DL_TYPE == 0 {
 		nm.Fields = append(nm.Fields, nom.EthType(m.DlType()))
 	}
-	if wc&of10.PFW_NW_SRC_MASK != 0 {
-		mask := uint(wc&of10.PFW_NW_SRC_MASK) >> uint(of10.PFW_NW_DST_SHIFT)
+	if wc&of10.PFW_NW_SRC_MASK != of10.PFW_NW_SRC_ALL && m.NwSrc() != 0 {
+		mask := uint(wc&of10.PFW_NW_SRC_MASK) >> uint(of10.PFW_NW_SRC_SHIFT)
 		nm.Fields = append(nm.Fields,
 			nom.IPv4Src(nom.CIDRToMaskedIPv4(m.NwSrc(), mask)))
 	}
-	if wc&of10.PFW_NW_DST_MASK != 0 {
+	if wc&of10.PFW_NW_DST_MASK != of10.PFW_NW_DST_ALL && m.NwDst() != 0 {
 		mask := uint(wc&of10.PFW_NW_DST_MASK) >> uint(of10.PFW_NW_DST_SHIFT)
 		nm.Fields = append(nm.Fields,
 			nom.IPv4Dst(nom.CIDRToMaskedIPv4(m.NwDst(), mask)))
 	}
-	if wc&of10.PFW_TP_SRC != 0 {
+	if wc&of10.PFW_TP_SRC == 0 {
 		nm.Fields = append(nm.Fields, nom.TransportPortSrc(m.TpSrc()))
 	}
-	if wc&of10.PFW_TP_DST != 0 {
+	if wc&of10.PFW_TP_DST == 0 {
 		nm.Fields = append(nm.Fields, nom.TransportPortDst(m.TpDst()))
 	}
 	return nm, nil
@@ -291,15 +291,15 @@ func (d *of10Driver) ofMatch(m nom.Match) (of10.Match, error) {
 			w &= ^of10.PFW_IN_PORT
 
 		case nom.EthDst:
-			if f.Mask != [6]byte{} {
+			if f.Mask != nom.MaskNoneMAC {
 				return of10.Match{},
 					fmt.Errorf("of10Driver: masked ethernet address is not supported")
 			}
 			ofm.SetDlDst([6]byte(f.Addr))
 			w &= ^of10.PFW_DL_DST
 
 		case nom.EthSrc:
-			if f.Mask != [6]byte{} {
+			if f.Mask != nom.MaskNoneMAC {
 				return of10.Match{},
 					fmt.Errorf("of10Driver: masked ethernet address is not supported")
 			}
@@ -311,25 +311,23 @@ func (d *of10Driver) ofMatch(m nom.Match) (of10.Match, error) {
 			w &= ^of10.PFW_DL_TYPE
 
 		case nom.IPv4Src:
-			ofm.SetNwSrc(f.Addr.Uint())
-			mask := f.Mask.Uint()
 			w &= ^of10.PFW_NW_SRC_MASK
-			for i := uint(0); i < 32; i++ {
-				if mask&(1<<i) != 0 {
-					w |= of10.FlowWildcards(i << uint(of10.PFW_NW_SRC_SHIFT))
-					break
-				}
+			ofm.SetNwSrc(f.Addr.Uint())
+			mask := f.Mask.PopCount()
+			if mask == 32 {
+				w |= of10.PFW_NW_SRC_MASK
+			} else {
+				w |= of10.FlowWildcards(mask << uint(of10.PFW_NW_SRC_SHIFT))
 			}
 
 		case nom.IPv4Dst:
-			ofm.SetNwDst(f.Addr.Uint())
-			mask := f.Mask.Uint()
 			w &= ^of10.PFW_NW_DST_MASK
-			for i := uint(0); i < 32; i++ {
-				if mask&(1<<i) != 0 {
-					w |= of10.FlowWildcards(i << uint(of10.PFW_NW_DST_SHIFT))
-					break
-				}
+			ofm.SetNwDst(f.Addr.Uint())
+			mask := f.Mask.PopCount()
+			if mask == 32 {
+				w |= of10.PFW_NW_DST_MASK
+			} else {
+				w |= of10.FlowWildcards(mask << uint(of10.PFW_NW_DST_SHIFT))
 			}
 
 		case nom.TransportPortSrc:

openflow/driver_test.go

@@ -0,0 +1,58 @@
+package openflow
+
+import (
+	"testing"
+
+	"github.com/kandoo/beehive-netctrl/nom"
+)
+
+func TestOF10Match(t *testing.T) {
+	driver := of10Driver{}
+	matches := []nom.Match{
+		{
+			Fields: []nom.Field{
+				nom.EthSrc{
+					Addr: nom.MACAddr{1, 2, 3, 4, 5, 6},
+					Mask: nom.MaskNoneMAC,
+				},
+			},
+		},
+		{
+			Fields: []nom.Field{
+				nom.EthDst{
+					Addr: nom.MACAddr{1, 2, 3, 4, 5, 6},
+					Mask: nom.MaskNoneMAC,
+				},
+			},
+		},
+		{
+			Fields: []nom.Field{
+				nom.IPv4Src{
+					Addr: nom.IPv4Addr{1, 2, 3, 4},
+					Mask: nom.IPv4Addr{255, 255, 255, 0},
+				},
+			},
+		},
+		{
+			Fields: []nom.Field{
+				nom.IPv4Dst{
+					Addr: nom.IPv4Addr{127, 0, 0, 1},
+					Mask: nom.IPv4Addr{255, 255, 255, 128},
+				},
+			},
+		},
+	}
+	for _, m := range matches {
+		ofm, err := driver.ofMatch(m)
+		if err != nil {
+			t.Error(err)
+		}
+		nm, err := driver.nomMatch(ofm)
+		if err != nil {
+			t.Error(err)
+		}
+		if !nm.Equals(m) {
+			t.Errorf("invalid match conversion:\n\tactual=%#v\n\twant=%#v", nm, m)
+		}
+	}
+}

openflow/handshake.go

@@ -3,11 +3,11 @@ package openflow
 import (
 	"errors"
 
-	"github.com/kandoo/beehive/Godeps/_workspace/src/github.com/golang/glog"
 	"github.com/kandoo/beehive-netctrl/nom"
 	"github.com/kandoo/beehive-netctrl/openflow/of"
 	"github.com/kandoo/beehive-netctrl/openflow/of10"
 	"github.com/kandoo/beehive-netctrl/openflow/of12"
+	"github.com/kandoo/beehive/Godeps/_workspace/src/github.com/golang/glog"
 )
 
 func (c *ofConn) handshake() (ofDriver, error) {
@@ -21,7 +21,7 @@ func (c *ofConn) handshake() (ofDriver, error) {
 		return nil, err
 	}
 
-	glog.V(2).Info("Received hello from a switch")
+	glog.V(2).Info("%v received hello from a switch", c.ctx)
 
 	version := of.OPENFLOW_1_0
 	if h.Version() > uint8(of.OPENFLOW_1_2) {
@@ -34,7 +34,7 @@ func (c *ofConn) handshake() (ofDriver, error) {
 	}
 	c.Flush()
 
-	glog.V(2).Info("Sent hello to the switch")
+	glog.V(2).Info("%v sent hello to the switch", c.ctx)
 
 	var driver ofDriver
 	switch version {
@@ -49,7 +49,7 @@ func (c *ofConn) handshake() (ofDriver, error) {
 	}
 
 	if c.node.ID == nom.NodeID(0) {
-		return nil, errors.New("Invalid node after handshake")
+		return nil, errors.New("ofConn: invalid node after handshake")
 	}
 
 	return driver, nil
@@ -62,7 +62,7 @@ func (d *of10Driver) handshake(c *ofConn) error {
 	}
 	c.Flush()
 
-	glog.V(2).Info("Sent features request to the switch")
+	glog.V(2).Info("%v sent features request to the switch", c.ctx)
 
 	hdr, err := c.ReadHeader()
 	if err != nil {
@@ -79,9 +79,10 @@ func (d *of10Driver) handshake(c *ofConn) error {
 		return err
 	}
 
-	glog.Infof("Handshake completed for switch %016x", frep.DatapathId())
-
-	glog.Infof("Disabling packet buffers in the switch.")
+	glog.Infof("%v completes handshaking with switch %016x", c.ctx,
+		frep.DatapathId())
+	glog.Infof("%v disables packet buffers in switch %016x", c.ctx,
+		frep.DatapathId())
 	cfg := of10.NewSwitchSetConfig()
 	cfg.SetMissSendLen(0xFFFF)
 	c.WriteHeader(cfg.Header)
@@ -92,7 +93,7 @@ func (d *of10Driver) handshake(c *ofConn) error {
 		MACAddr:      datapathIDToMACAddr(frep.DatapathId()),
 		Capabilities: nil,
 	}
-	glog.Infof("%v connected", c.node)
+	glog.Infof("%v is connected to %v", c.ctx, c.node)
 
 	nomDriver := nom.Driver{
 		BeeID: c.ctx.ID(),

openflow/packetin.go

@@ -19,7 +19,7 @@ func (of *of10Driver) handlePacketIn(in of10.PacketIn, c *ofConn) error {
 
 	port, ok := of.ofPorts[inPort]
 	if !ok {
-		return fmt.Errorf("of10driver: port not found %v", inPort)
+		return fmt.Errorf("of10Driver: port not found %v", inPort)
 	}
 
 	glog.V(2).Infof("packet received: %v", in)