proto: Fix ARP header field ordering

In ARP header, destination ether address sits between source IP and
destination IP addresses. Enum arp_hdr_fields had this wrong, which
in turn caused wrong ordering of entries in proto_arp->templates. When
expanding a combined payload expression, code assumes that template
entries are ordered by header offset, therefore the destination ether
address match was printed as raw if an earlier field was matched as
well:

| arp saddr ip 192.168.1.1 arp daddr ether 3e:d1:3f:d6:12:0b

was printed as:

| arp saddr ip 192.168.1.1 @nh,144,48 69068440080907

Note: Although strictly not necessary, reorder fields in
proto_arp->templates as well to match their actual ordering, just to
avoid confusion.

Fixes: 4b0f2a712b579 ("src: support for arp sender and target ethernet and IPv4 addresses")
Signed-off-by: Phil Sutter <phil@nwl.cc>

diff --git a/include/proto.h b/include/proto.h
index b78bb9bc2a7121659590d312c8f53a2ba39754a4..6ef332c3966f093255101a626108e23f6b0eb7da 100644 (file)
--- a/include/proto.h
+++ b/include/proto.h
@@ -222,8 +222,8 @@ enum arp_hdr_fields {
        ARPHDR_PLN,
        ARPHDR_OP,
        ARPHDR_SADDR_ETHER,
-       ARPHDR_DADDR_ETHER,
        ARPHDR_SADDR_IP,
+       ARPHDR_DADDR_ETHER,
        ARPHDR_DADDR_IP,
 };
 

diff --git a/src/proto.c b/src/proto.c
index 725b03851e980005fb9889e1807025c207a332b0..c42e8f517bae602ff2380ff552980051f2d1849b 100644 (file)
--- a/src/proto.c
+++ b/src/proto.c
@@ -980,8 +980,8 @@ const struct proto_desc proto_arp = {
                [ARPHDR_PLN]            = ARPHDR_FIELD("plen", plen),
                [ARPHDR_OP]             = ARPHDR_TYPE("operation", &arpop_type, oper),
                [ARPHDR_SADDR_ETHER]    = ARPHDR_TYPE("saddr ether", &etheraddr_type, sha),
-               [ARPHDR_DADDR_ETHER]    = ARPHDR_TYPE("daddr ether", &etheraddr_type, tha),
                [ARPHDR_SADDR_IP]       = ARPHDR_TYPE("saddr ip", &ipaddr_type, spa),
+               [ARPHDR_DADDR_ETHER]    = ARPHDR_TYPE("daddr ether", &etheraddr_type, tha),
                [ARPHDR_DADDR_IP]       = ARPHDR_TYPE("daddr ip", &ipaddr_type, tpa),
        },
        .format         = {

diff --git a/tests/py/arp/arp.t b/tests/py/arp/arp.t
index 2540c0a77419152e6bbe9d2863f211d84b44bd1c..109d01d7ad506eccff28a5a93cf0bd0d4a3a3c0f 100644 (file)
--- a/tests/py/arp/arp.t
+++ b/tests/py/arp/arp.t
@@ -61,4 +61,7 @@ arp daddr ip 4.3.2.1;ok
 arp saddr ether aa:bb:cc:aa:bb:cc;ok
 arp daddr ether aa:bb:cc:aa:bb:cc;ok
 
+arp saddr ip 192.168.1.1 arp daddr ether fe:ed:00:c0:ff:ee;ok
+arp daddr ether fe:ed:00:c0:ff:ee arp saddr ip 192.168.1.1;ok;arp saddr ip 192.168.1.1 arp daddr ether fe:ed:00:c0:ff:ee
+
 meta iifname "invalid" arp ptype 0x0800 arp htype 1 arp hlen 6 arp plen 4 @nh,192,32 0xc0a88f10 @nh,144,48 set 0x112233445566;ok;iifname "invalid" arp htype 1 arp ptype ip arp hlen 6 arp plen 4 arp daddr ip 192.168.143.16 arp daddr ether set 11:22:33:44:55:66

diff --git a/tests/py/arp/arp.t.json b/tests/py/arp/arp.t.json
index 5f2f6cd85b5e679d9d51aa7eafbbd8ec14eb8326..8508c1703475bb40581e48a577b762b27b4d5277 100644 (file)
--- a/tests/py/arp/arp.t.json
+++ b/tests/py/arp/arp.t.json
@@ -901,6 +901,62 @@
     }
 ]
 
+# arp saddr ip 192.168.1.1 arp daddr ether fe:ed:00:c0:ff:ee
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "saddr ip",
+                    "protocol": "arp"
+                }
+            },
+            "op": "==",
+            "right": "192.168.1.1"
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "daddr ether",
+                    "protocol": "arp"
+                }
+            },
+            "op": "==",
+            "right": "fe:ed:00:c0:ff:ee"
+        }
+    }
+]
+
+# arp daddr ether fe:ed:00:c0:ff:ee arp saddr ip 192.168.1.1
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "daddr ether",
+                    "protocol": "arp"
+                }
+            },
+            "op": "==",
+            "right": "fe:ed:00:c0:ff:ee"
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "saddr ip",
+                    "protocol": "arp"
+                }
+            },
+            "op": "==",
+            "right": "192.168.1.1"
+        }
+    }
+]
+
 # meta iifname "invalid" arp ptype 0x0800 arp htype 1 arp hlen 6 arp plen 4 @nh,192,32 0xc0a88f10 @nh,144,48 set 0x112233445566
 [
     {

diff --git a/tests/py/arp/arp.t.json.output b/tests/py/arp/arp.t.json.output
index b8507bffc8cc4e303eddd5a0f564fa56af8a97e0..afa75b2e8a0d7319fe2f1c5e2a261a1c5d0152cb 100644 (file)
--- a/tests/py/arp/arp.t.json.output
+++ b/tests/py/arp/arp.t.json.output
@@ -66,6 +66,34 @@
     }
 ]
 
+# arp daddr ether fe:ed:00:c0:ff:ee arp saddr ip 192.168.1.1
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "saddr ip",
+                    "protocol": "arp"
+                }
+            },
+            "op": "==",
+            "right": "192.168.1.1"
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "daddr ether",
+                    "protocol": "arp"
+                }
+            },
+            "op": "==",
+            "right": "fe:ed:00:c0:ff:ee"
+        }
+    }
+]
+
 # meta iifname "invalid" arp ptype 0x0800 arp htype 1 arp hlen 6 arp plen 4 @nh,192,32 0xc0a88f10 @nh,144,48 set 0x112233445566
 [
     {

diff --git a/tests/py/arp/arp.t.payload b/tests/py/arp/arp.t.payload
index 52c993294810f23f2884c31e0e2acf64caedd128..f819853f448ed1f94a7ac1fc760874c5d86e0010 100644 (file)
--- a/tests/py/arp/arp.t.payload
+++ b/tests/py/arp/arp.t.payload
@@ -307,3 +307,13 @@ arp test-arp input
   [ payload load 6b @ network header + 18 => reg 1 ]
   [ cmp eq reg 1 0xaaccbbaa 0x0000ccbb ]
 
+# arp saddr ip 192.168.1.1 arp daddr ether fe:ed:00:c0:ff:ee
+arp 
+  [ payload load 10b @ network header + 14 => reg 1 ]
+  [ cmp eq reg 1 0x0101a8c0 0xc000edfe 0x0000eeff ]
+
+# arp daddr ether fe:ed:00:c0:ff:ee arp saddr ip 192.168.1.1
+arp 
+  [ payload load 10b @ network header + 14 => reg 1 ]
+  [ cmp eq reg 1 0x0101a8c0 0xc000edfe 0x0000eeff ]
+

diff --git a/tests/py/arp/arp.t.payload.netdev b/tests/py/arp/arp.t.payload.netdev
index 667691fff2f650c6df9620e15377103b86207164..f57610cf92f7153edce024ecf29b1da1dd759d73 100644 (file)
--- a/tests/py/arp/arp.t.payload.netdev
+++ b/tests/py/arp/arp.t.payload.netdev
@@ -409,3 +409,17 @@ netdev test-netdev ingress
   [ payload load 6b @ network header + 18 => reg 1 ]
   [ cmp eq reg 1 0xaaccbbaa 0x0000ccbb ]
 
+# arp saddr ip 192.168.1.1 arp daddr ether fe:ed:00:c0:ff:ee
+netdev 
+  [ meta load protocol => reg 1 ]
+  [ cmp eq reg 1 0x00000608 ]
+  [ payload load 10b @ network header + 14 => reg 1 ]
+  [ cmp eq reg 1 0x0101a8c0 0xc000edfe 0x0000eeff ]
+
+# arp daddr ether fe:ed:00:c0:ff:ee arp saddr ip 192.168.1.1
+netdev 
+  [ meta load protocol => reg 1 ]
+  [ cmp eq reg 1 0x00000608 ]
+  [ payload load 10b @ network header + 14 => reg 1 ]
+  [ cmp eq reg 1 0x0101a8c0 0xc000edfe 0x0000eeff ]
+