[ovs-dev] [recirc datapath V4 2/5] odp-util: Always generate key/mask pair in netlink for recirc_id

Andy Zhou azhou at nicira.com
Fri Apr 18 09:50:38 UTC 2014


Currently netlink flow (and mask) recirc_id attribute is only
serialized when the recirc_id value is non-zero. For this logic
to work correctly, the interpretation of the missing recirc_id
depends on whether the datapath supports recirculation.

This patch remove the ambiguity of the meaning of missing recirc_id
attribute in netlink message.  When recirc_id is non-zero, or when
it is not a wildcard match, both key and mask attributes are
serialized.  On the other hand, when recirc_id is zero, and being
wildcarded, they are not serialized.  A missing recirc_id key and
mask attribute thus should always be interpreted as wildcard,
same as other flow fields.

Signed-off-by: Andy Zhou <azhou at nicira.com>
---
 lib/dpif-netdev.c      |  9 +++++----
 lib/odp-util.c         | 16 ++++++++--------
 lib/odp-util.h         |  4 ++--
 ofproto/ofproto-dpif.c |  4 ++--
 tests/ofproto-dpif.at  | 20 ++++++++++----------
 tests/test-odp.c       |  3 ++-
 6 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 76141e3..d17bc64 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1442,6 +1442,7 @@ dpif_netdev_flow_dump_next(const struct dpif *dpif, void *iter_, void *state_,
     struct dp_netdev_flow_state *state = state_;
     struct dp_netdev *dp = get_dp_netdev(dpif);
     struct dp_netdev_flow *netdev_flow;
+    struct flow_wildcards wc;
     int error;
 
     ovs_mutex_lock(&iter->mutex);
@@ -1464,11 +1465,13 @@ dpif_netdev_flow_dump_next(const struct dpif *dpif, void *iter_, void *state_,
         return error;
     }
 
+    minimask_expand(&netdev_flow->cr.match.mask, &wc);
+
     if (key) {
         struct ofpbuf buf;
 
         ofpbuf_use_stack(&buf, &state->keybuf, sizeof state->keybuf);
-        odp_flow_key_from_flow(&buf, &netdev_flow->flow,
+        odp_flow_key_from_flow(&buf, &netdev_flow->flow, &wc.masks,
                                netdev_flow->flow.in_port.odp_port);
 
         *key = ofpbuf_data(&buf);
@@ -1477,10 +1480,8 @@ dpif_netdev_flow_dump_next(const struct dpif *dpif, void *iter_, void *state_,
 
     if (key && mask) {
         struct ofpbuf buf;
-        struct flow_wildcards wc;
 
         ofpbuf_use_stack(&buf, &state->maskbuf, sizeof state->maskbuf);
-        minimask_expand(&netdev_flow->cr.match.mask, &wc);
         odp_flow_key_from_mask(&buf, &wc.masks, &netdev_flow->flow,
                                odp_to_u32(wc.masks.in_port.odp_port),
                                SIZE_MAX);
@@ -2070,7 +2071,7 @@ dp_netdev_output_userspace(struct dp_netdev *dp, struct ofpbuf *packet,
         ofpbuf_init(buf, buf_size);
 
         /* Put ODP flow. */
-        odp_flow_key_from_flow(buf, flow, flow->in_port.odp_port);
+        odp_flow_key_from_flow(buf, flow, NULL, flow->in_port.odp_port);
         upcall->key = ofpbuf_data(buf);
         upcall->key_len = ofpbuf_size(buf);
 
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 0969ce8..70390bf 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -2482,8 +2482,8 @@ ovs_to_odp_frag_mask(uint8_t nw_frag_mask)
 
 static void
 odp_flow_key_from_flow__(struct ofpbuf *buf, const struct flow *data,
-                         const struct flow *flow, odp_port_t odp_in_port,
-                         size_t max_mpls_depth)
+                         const struct flow *flow, const struct flow *mask,
+                         odp_port_t odp_in_port, size_t max_mpls_depth)
 {
     bool is_mask;
     struct ovs_key_ethernet *eth_key;
@@ -2501,11 +2501,11 @@ odp_flow_key_from_flow__(struct ofpbuf *buf, const struct flow *data,
 
     nl_msg_put_u32(buf, OVS_KEY_ATTR_SKB_MARK, data->pkt_mark);
 
-    if (flow->recirc_id) {
+    if (data->recirc_id || (mask && mask->recirc_id)) {
         nl_msg_put_u32(buf, OVS_KEY_ATTR_RECIRC_ID, data->recirc_id);
     }
 
-    if (flow->dp_hash) {
+    if (data->dp_hash || (mask && mask->dp_hash)) {
         nl_msg_put_u32(buf, OVS_KEY_ATTR_DP_HASH, data->dp_hash);
     }
 
@@ -2681,9 +2681,9 @@ unencap:
  * capable of being expanded to allow for that much space. */
 void
 odp_flow_key_from_flow(struct ofpbuf *buf, const struct flow *flow,
-                       odp_port_t odp_in_port)
+                       const struct flow *mask, odp_port_t odp_in_port)
 {
-    odp_flow_key_from_flow__(buf, flow, flow, odp_in_port, SIZE_MAX);
+    odp_flow_key_from_flow__(buf, flow, flow, mask, odp_in_port, SIZE_MAX);
 }
 
 /* Appends a representation of 'mask' as OVS_KEY_ATTR_* attributes to
@@ -2699,8 +2699,8 @@ odp_flow_key_from_mask(struct ofpbuf *buf, const struct flow *mask,
                        const struct flow *flow, uint32_t odp_in_port_mask,
                        size_t max_mpls_depth)
 {
-    odp_flow_key_from_flow__(buf, mask, flow, u32_to_odp(odp_in_port_mask),
-                             max_mpls_depth);
+    odp_flow_key_from_flow__(buf, mask, flow, mask,
+                             u32_to_odp(odp_in_port_mask), max_mpls_depth);
 }
 
 /* Generate ODP flow key from the given packet metadata */
diff --git a/lib/odp-util.h b/lib/odp-util.h
index 7bc64c7..0dfbcca 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -143,8 +143,8 @@ int odp_flow_from_string(const char *s,
                          const struct simap *port_names,
                          struct ofpbuf *, struct ofpbuf *);
 
-void odp_flow_key_from_flow(struct ofpbuf *, const struct flow *,
-                            odp_port_t odp_in_port);
+void odp_flow_key_from_flow(struct ofpbuf *, const struct flow * flow,
+                            const struct flow *mask, odp_port_t odp_in_port);
 void odp_flow_key_from_mask(struct ofpbuf *, const struct flow *mask,
                             const struct flow *flow, uint32_t odp_in_port,
                             size_t max_mpls_depth);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 1e51ff2..33a1b49 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -951,7 +951,7 @@ check_recirc(struct dpif_backer *backer)
     flow.dp_hash = 1;
 
     ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
-    odp_flow_key_from_flow(&key, &flow, 0);
+    odp_flow_key_from_flow(&key, &flow, NULL, 0);
 
     error = dpif_flow_put(backer->dpif, DPIF_FP_CREATE | DPIF_FP_MODIFY,
                           ofpbuf_data(&key), ofpbuf_size(&key), NULL, 0, NULL,
@@ -1084,7 +1084,7 @@ check_max_mpls_depth(struct dpif_backer *backer)
         flow_set_mpls_bos(&flow, n, 1);
 
         ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
-        odp_flow_key_from_flow(&key, &flow, 0);
+        odp_flow_key_from_flow(&key, &flow, NULL, 0);
 
         error = dpif_flow_put(backer->dpif, DPIF_FP_CREATE | DPIF_FP_MODIFY,
                               ofpbuf_data(&key), ofpbuf_size(&key), NULL, 0, NULL, 0, NULL);
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index b12b4fe..39a37a3 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -3686,21 +3686,21 @@ 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
-skb_priority(0),in_port(1),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/0xff), packets:0, bytes:0, used:never, actions:drop
-skb_priority(0),in_port(2),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/0xff), packets:0, bytes:0, used:never, actions:drop
+skb_priority(0),recirc_id(0),in_port(1),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/0xff), packets:0, bytes:0, used:never, actions:drop
+skb_priority(0),recirc_id(0),in_port(2),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/0xff), packets:0, bytes:0, used:never, actions:drop
 ])
 
 AT_CHECK([ovs-appctl dpif/dump-flows br1 | sort | STRIP_USED], [0], [dnl
-skb_priority(0),in_port(3),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/0xff), packets:0, bytes:0, used:never, actions:drop
+skb_priority(0),recirc_id(0),in_port(3),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/0xff), packets:0, bytes:0, used:never, actions:drop
 ])
 
 AT_CHECK([ovs-appctl dpif/dump-flows -m br0 | sort | STRIP_USED], [0], [dnl
-skb_priority(0),skb_mark(0/0),in_port(p1),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/0xff),icmp(type=8/0,code=0/0), packets:0, bytes:0, used:never, actions:drop
-skb_priority(0),skb_mark(0/0),in_port(p2),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/0xff),icmp(type=0/0,code=0/0), packets:0, bytes:0, used:never, actions:drop
+skb_priority(0),skb_mark(0/0),recirc_id(0),in_port(p1),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/0xff),icmp(type=8/0,code=0/0), packets:0, bytes:0, used:never, actions:drop
+skb_priority(0),skb_mark(0/0),recirc_id(0),in_port(p2),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/0xff),icmp(type=0/0,code=0/0), packets:0, bytes:0, used:never, actions:drop
 ])
 
 AT_CHECK([ovs-appctl dpif/dump-flows -m br1 | sort | STRIP_USED], [0], [dnl
-skb_priority(0),skb_mark(0/0),in_port(p3),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/0xff),icmp(type=8/0,code=0/0), packets:0, bytes:0, used:never, actions:drop
+skb_priority(0),skb_mark(0/0),recirc_id(0),in_port(p3),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/0xff),icmp(type=8/0,code=0/0), packets:0, bytes:0, used:never, actions:drop
 ])
 
 OVS_VSWITCHD_STOP
@@ -3833,10 +3833,10 @@ skb_priority(0),skb_mark(0/0),in_port(101),eth(src=50:54:00:00:00:07/00:00:00:00
 ])
 
 AT_CHECK([cat ovs-vswitchd.log | grep -e 'in_port(100).*packets:9' | FILTER_FLOW_DUMP], [0], [dnl
-skb_priority(0),skb_mark(0/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/0xff),icmp(type=8/0,code=0/0), packets:9, bytes:540, used:0.0s
+skb_priority(0),skb_mark(0/0),recirc_id(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/0xff),icmp(type=8/0,code=0/0), packets:9, bytes:540, used:0.0s
 ])
 AT_CHECK([cat ovs-vswitchd.log | grep -e 'in_port(101).*packets:4' | FILTER_FLOW_DUMP], [0], [dnl
-skb_priority(0),skb_mark(0/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/0xff),icmp(type=8/0,code=0/0), packets:4, bytes:240, used:0.0s
+skb_priority(0),skb_mark(0/0),recirc_id(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/0xff),icmp(type=8/0,code=0/0), packets:4, bytes:240, used:0.0s
 ])
 
 AT_CHECK([ovs-ofctl dump-ports br0 pbr0], [0], [dnl
@@ -4378,10 +4378,10 @@ skb_priority(0),skb_mark(0),in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00
 skb_priority(0),skb_mark(0),in_port(1),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(src=10.0.0.4,dst=10.0.0.3,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0), actions:drop
 ])
 AT_CHECK([cat ovs-vswitchd.log | grep '00:09.*packets:3' | FILTER_FLOW_DUMP], [0], [dnl
-skb_priority(0),skb_mark(0),in_port(1),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), packets:3, bytes:180, used:0.0s
+skb_priority(0),skb_mark(0),recirc_id(0),dp_hash(0),in_port(1),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), packets:3, bytes:180, used:0.0s
 ])
 AT_CHECK([cat ovs-vswitchd.log | grep '00:0b.*packets:3' | FILTER_FLOW_DUMP], [0], [dnl
-skb_priority(0),skb_mark(0),in_port(1),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(src=10.0.0.4,dst=10.0.0.3,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0), packets:3, bytes:180, used:0.0s
+skb_priority(0),skb_mark(0),recirc_id(0),dp_hash(0),in_port(1),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(src=10.0.0.4,dst=10.0.0.3,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0), packets:3, bytes:180, used:0.0s
 ])
 OVS_VSWITCHD_STOP
 AT_CLEANUP
diff --git a/tests/test-odp.c b/tests/test-odp.c
index c68353e..30cdbbf 100644
--- a/tests/test-odp.c
+++ b/tests/test-odp.c
@@ -76,7 +76,8 @@ parse_keys(bool wc_keys)
             /* Convert cls_rule back to odp_key. */
             ofpbuf_uninit(&odp_key);
             ofpbuf_init(&odp_key, 0);
-            odp_flow_key_from_flow(&odp_key, &flow, flow.in_port.odp_port);
+            odp_flow_key_from_flow(&odp_key, &flow, NULL,
+                                   flow.in_port.odp_port);
 
             if (ofpbuf_size(&odp_key) > ODPUTIL_FLOW_KEY_BYTES) {
                 printf ("too long: %"PRIu32" > %d\n",
-- 
1.9.1




More information about the dev mailing list