[ovs-dev] [PATCH 2/2] odp-util: always export priority and skb_mark in netlink

Andy Zhou azhou at nicira.com
Wed Jul 31 02:49:13 UTC 2013


Handling of missing attributes in netlink can be tricky and turns out
to be error prone. The value (savings in netlink bandwidth) does not
seem to be significant enough to justify allowing them. This patch
series make both kernel and userspace always export priority and
skb_mark attribute. There will be follow on patches in the
direction of making all attributes explicit.

Signed-off-by: Andy Zhou <azhou at nicira.com>
---
 lib/odp-util.c        |   62 +++++++++++++++++++++++++++++++++++++++----------
 tests/ofproto-dpif.at |   18 +++++++-------
 2 files changed, 59 insertions(+), 21 deletions(-)

diff --git a/lib/odp-util.c b/lib/odp-util.c
index 3c3063d..1f7db2f 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -926,6 +926,42 @@ odp_mask_attr_is_exact(const struct nlattr *ma)
     return is_exact;
 }
 
+static bool
+ommit_format(enum ovs_key_attr attr, const struct nlattr *a,
+             const struct nlattr *ma)
+{
+    switch (attr) {
+        case OVS_KEY_ATTR_PRIORITY:
+        case OVS_KEY_ATTR_SKB_MARK:
+            if (!nl_attr_get_u32(a)) {
+                if ((!ma) || !nl_attr_get_u32(ma)) {
+                    return true;  /* Omit output 0 (no mask) or 0/0  */
+                }
+            }
+            break;
+        case OVS_KEY_ATTR_UNSPEC:
+        case OVS_KEY_ATTR_ENCAP:
+        case OVS_KEY_ATTR_TUNNEL:
+        case OVS_KEY_ATTR_IN_PORT:
+        case OVS_KEY_ATTR_ETHERNET:
+        case OVS_KEY_ATTR_VLAN:
+        case OVS_KEY_ATTR_ETHERTYPE:
+        case OVS_KEY_ATTR_IPV4:
+        case OVS_KEY_ATTR_IPV6:
+        case OVS_KEY_ATTR_TCP:
+        case OVS_KEY_ATTR_UDP:
+        case OVS_KEY_ATTR_ICMP:
+        case OVS_KEY_ATTR_ICMPV6:
+        case OVS_KEY_ATTR_ARP:
+        case OVS_KEY_ATTR_ND:
+        case OVS_KEY_ATTR_MPLS:
+        case __OVS_KEY_ATTR_MAX:
+        default:
+            break;
+    }
+
+    return false;
+}
 
 static void
 format_odp_key_attr(const struct nlattr *a, const struct nlattr *ma,
@@ -1345,13 +1381,13 @@ odp_flow_format(const struct nlattr *key, size_t key_len,
         bool has_ethtype_key = false;
         const struct nlattr *ma = NULL;
         struct ofpbuf ofp;
+        bool first_key = true;
 
         ofpbuf_init(&ofp, 100);
         NL_ATTR_FOR_EACH (a, left, key, key_len) {
-            if (a != key) {
-                ds_put_char(ds, ',');
-            }
-            if (nl_attr_type(a) == OVS_KEY_ATTR_ETHERTYPE) {
+            enum ovs_key_attr attr = nl_attr_type(a);
+
+            if ( attr == OVS_KEY_ATTR_ETHERTYPE) {
                 has_ethtype_key = true;
             }
             if (mask && mask_len) {
@@ -1360,8 +1396,14 @@ odp_flow_format(const struct nlattr *key, size_t key_len,
                     ma = generate_all_wildcard_mask(&ofp, a);
                 }
             }
-            format_odp_key_attr(a, ma, ds);
-            ofpbuf_clear(&ofp);
+            if (!ommit_format(attr, a, ma)) {
+                if (!first_key) {
+                    ds_put_char(ds, ',');
+                }
+                format_odp_key_attr(a, ma, ds);
+                first_key = false;
+                ofpbuf_clear(&ofp);
+            }
         }
         ofpbuf_uninit(&ofp);
 
@@ -2323,17 +2365,13 @@ odp_flow_key_from_flow__(struct ofpbuf *buf, const struct flow *data,
      * treat 'data' as a mask. */
     is_mask = (data != flow);
 
-    if (flow->skb_priority) {
-        nl_msg_put_u32(buf, OVS_KEY_ATTR_PRIORITY, data->skb_priority);
-    }
+    nl_msg_put_u32(buf, OVS_KEY_ATTR_PRIORITY, data->skb_priority);
 
     if (flow->tunnel.ip_dst) {
         tun_key_to_attr(buf, &data->tunnel);
     }
 
-    if (flow->skb_mark) {
-        nl_msg_put_u32(buf, OVS_KEY_ATTR_SKB_MARK, data->skb_mark);
-    }
+    nl_msg_put_u32(buf, OVS_KEY_ATTR_SKB_MARK, data->skb_mark);
 
     /* Add an ingress port attribute if this is a mask or 'odp_in_port'
      * is not the magical value "ODPP_NONE". */
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 2728a28..3600fb6 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -2088,12 +2088,12 @@ AT_CHECK([ovs-appctl netdev-dummy/receive p2 'in_port(2),eth(src=50:54:00:00:00:
 AT_CHECK([ovs-appctl netdev-dummy/receive p3 'in_port(3),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)'])
 
 AT_CHECK([ovs-appctl dpif/dump-flows br0 | sort | STRIP_USED], [0], [dnl
-in_port(1),eth(src=50:54:00:00:00:05/00:00:00:00:00:00,dst=50:54:00:00:00:07/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=192.168.0.1/0.0.0.0,dst=192.168.0.2/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0x2),icmp(type=8/0,code=0/0), packets:0, bytes:0, used:0.0s, actions:userspace(pid=0,slow_path(controller))
-in_port(2),eth(src=50:54:00:00:00:07/00:00:00:00:00:00,dst=50:54:00:00:00:05/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=192.168.0.2/0.0.0.0,dst=192.168.0.1/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0x2),icmp(type=0/0,code=0/0), packets:0, bytes:0, used:0.0s, actions:userspace(pid=0,slow_path(controller))
+skb_priority(0),in_port(1),eth(src=50:54:00:00:00:05/00:00:00:00:00:00,dst=50:54:00:00:00:07/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=192.168.0.1/0.0.0.0,dst=192.168.0.2/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0x2),icmp(type=8/0,code=0/0), packets:0, bytes:0, used:0.0s, actions:userspace(pid=0,slow_path(controller))
+skb_priority(0),in_port(2),eth(src=50:54:00:00:00:07/00:00:00:00:00:00,dst=50:54:00:00:00:05/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=192.168.0.2/0.0.0.0,dst=192.168.0.1/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0x2),icmp(type=0/0,code=0/0), packets:0, bytes:0, used:0.0s, actions:userspace(pid=0,slow_path(controller))
 ])
 
 AT_CHECK([ovs-appctl dpif/dump-flows br1 | sort | STRIP_USED], [0], [dnl
-in_port(3),eth(src=50:54:00:00:00:09/00:00:00:00:00:00,dst=50:54:00:00:00:0a/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=10.0.0.2/0.0.0.0,dst=10.0.0.1/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0x2),icmp(type=8/0,code=0/0), packets:0, bytes:0, used:0.0s, actions:userspace(pid=0,slow_path(controller))
+skb_priority(0),in_port(3),eth(src=50:54:00:00:00:09/00:00:00:00:00:00,dst=50:54:00:00:00:0a/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=10.0.0.2/0.0.0.0,dst=10.0.0.1/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0x2),icmp(type=8/0,code=0/0), packets:0, bytes:0, used:0.0s, actions:userspace(pid=0,slow_path(controller))
 ])
 
 OVS_VSWITCHD_STOP
@@ -2110,12 +2110,12 @@ AT_CHECK([ovs-appctl netdev-dummy/receive p2 'in_port(2),eth(src=50:54:00:00:00:
 AT_CHECK([ovs-appctl netdev-dummy/receive p3 'in_port(3),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)'])
 
 AT_CHECK([ovs-appctl dpif/dump-flows br0 | sort | STRIP_USED], [0], [dnl
-in_port(1),eth(src=50:54:00:00:00:05/00:00:00:00:00:00,dst=50:54:00:00:00:07/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=192.168.0.1/0.0.0.0,dst=192.168.0.2/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0x2),icmp(type=8/0,code=0/0), packets:0, bytes:0, used:0.0s, actions:userspace(pid=0,slow_path(controller))
-in_port(2),eth(src=50:54:00:00:00:07/00:00:00:00:00:00,dst=50:54:00:00:00:05/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=192.168.0.2/0.0.0.0,dst=192.168.0.1/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0x2),icmp(type=0/0,code=0/0), packets:0, bytes:0, used:0.0s, actions:userspace(pid=0,slow_path(controller))
+skb_priority(0),in_port(1),eth(src=50:54:00:00:00:05/00:00:00:00:00:00,dst=50:54:00:00:00:07/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=192.168.0.1/0.0.0.0,dst=192.168.0.2/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0x2),icmp(type=8/0,code=0/0), packets:0, bytes:0, used:0.0s, actions:userspace(pid=0,slow_path(controller))
+skb_priority(0),in_port(2),eth(src=50:54:00:00:00:07/00:00:00:00:00:00,dst=50:54:00:00:00:05/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=192.168.0.2/0.0.0.0,dst=192.168.0.1/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0x2),icmp(type=0/0,code=0/0), packets:0, bytes:0, used:0.0s, actions:userspace(pid=0,slow_path(controller))
 ])
 
 AT_CHECK([ovs-appctl dpif/dump-flows br1 | sort | STRIP_USED], [0], [dnl
-in_port(3),eth(src=50:54:00:00:00:09/00:00:00:00:00:00,dst=50:54:00:00:00:0a/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=10.0.0.2/0.0.0.0,dst=10.0.0.1/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0x2),icmp(type=8/0,code=0/0), packets:0, bytes:0, used:0.0s, actions:userspace(pid=0,slow_path(controller))
+skb_priority(0),in_port(3),eth(src=50:54:00:00:00:09/00:00:00:00:00:00,dst=50:54:00:00:00:0a/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=10.0.0.2/0.0.0.0,dst=10.0.0.1/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0x2),icmp(type=8/0,code=0/0), packets:0, bytes:0, used:0.0s, actions:userspace(pid=0,slow_path(controller))
 ])
 
 AT_CHECK([ovs-appctl dpif/del-flows br0])
@@ -2123,7 +2123,7 @@ AT_CHECK([ovs-appctl dpif/dump-flows br0 | sort | STRIP_USED], [0], [dnl
 ])
 
 AT_CHECK([ovs-appctl dpif/dump-flows br1 | sort | STRIP_USED], [0], [dnl
-in_port(3),eth(src=50:54:00:00:00:09/00:00:00:00:00:00,dst=50:54:00:00:00:0a/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=10.0.0.2/0.0.0.0,dst=10.0.0.1/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0x2),icmp(type=8/0,code=0/0), packets:0, bytes:0, used:0.0s, actions:userspace(pid=0,slow_path(controller))
+skb_priority(0),in_port(3),eth(src=50:54:00:00:00:09/00:00:00:00:00:00,dst=50:54:00:00:00:0a/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=10.0.0.2/0.0.0.0,dst=10.0.0.1/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0x2),icmp(type=8/0,code=0/0), packets:0, bytes:0, used:0.0s, actions:userspace(pid=0,slow_path(controller))
 ])
 
 OVS_VSWITCHD_STOP
@@ -2170,10 +2170,10 @@ dummy at ovs-dummy: hit:13 missed:2
 ])
 
 AT_CHECK([ovs-appctl dpif/dump-flows br0 | STRIP_USED], [0], [dnl
-in_port(100),eth(src=50:54:00:00:00:05/00:00:00:00:00:00,dst=50:54:00:00:00:07/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=192.168.0.1/0.0.0.0,dst=192.168.0.2/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0x2),icmp(type=8/0,code=0/0), packets:9, bytes:540, used:0.0s, actions:101,3,2
+skb_priority(0),in_port(100),eth(src=50:54:00:00:00:05/00:00:00:00:00:00,dst=50:54:00:00:00:07/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=192.168.0.1/0.0.0.0,dst=192.168.0.2/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0x2),icmp(type=8/0,code=0/0), packets:9, bytes:540, used:0.0s, actions:101,3,2
 ]),
 AT_CHECK([ovs-appctl dpif/dump-flows br1 | STRIP_USED], [0], [dnl
-in_port(101),eth(src=50:54:00:00:00:07/00:00:00:00:00:00,dst=50:54:00:00:00:05/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=192.168.0.2/0.0.0.0,dst=192.168.0.1/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0x2),icmp(type=8/0,code=0/0), packets:4, bytes:240, used:0.0s, actions:100,2,3
+skb_priority(0),in_port(101),eth(src=50:54:00:00:00:07/00:00:00:00:00:00,dst=50:54:00:00:00:05/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=192.168.0.2/0.0.0.0,dst=192.168.0.1/0.0.0.0,proto=1/0,tos=0/0,ttl=64/0,frag=no/0x2),icmp(type=8/0,code=0/0), packets:4, bytes:240, used:0.0s, actions:100,2,3
 ])
 
 AT_CHECK([ovs-ofctl dump-ports br0 pbr0], [0], [dnl
-- 
1.7.9.5




More information about the dev mailing list