From fbfe01de0dedaf6d5b909b1a977d1f8164243754 Mon Sep 17 00:00:00 2001 From: Andy Zhou Date: Thu, 17 Apr 2014 23:13:46 -0700 Subject: [PATCH] odp-util: Always generate key/mask pair in netlink for recirc_id 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 Acked-by: Jarno Rajahalme --- lib/dpif-netdev.c | 9 +++++---- lib/odp-util.c | 38 +++++++++++++++++--------------------- lib/odp-util.h | 4 ++-- ofproto/ofproto-dpif.c | 4 ++-- tests/ofproto-dpif.at | 20 ++++++++++---------- tests/test-odp.c | 3 ++- 6 files changed, 38 insertions(+), 40 deletions(-) diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 790fe233d..15aa42a03 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -1447,6 +1447,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); @@ -1469,11 +1470,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); @@ -1482,10 +1485,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); @@ -2083,7 +2084,7 @@ dp_netdev_output_userspace(struct dp_netdev *dp, struct ofpbuf *packet, /* Put ODP flow. */ miniflow_expand(key, &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 0969ce848..9a4ea5e06 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -2481,37 +2481,33 @@ 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) +odp_flow_key_from_flow__(struct ofpbuf *buf, const struct flow *flow, + const struct flow *mask, odp_port_t odp_in_port, + size_t max_mpls_depth, bool export_mask) { - bool is_mask; struct ovs_key_ethernet *eth_key; size_t encap; - - /* We assume that if 'data' and 'flow' are not the same, we should - * treat 'data' as a mask. */ - is_mask = (data != flow); + const struct flow *data = export_mask ? mask : flow; nl_msg_put_u32(buf, OVS_KEY_ATTR_PRIORITY, data->skb_priority); - if (flow->tunnel.ip_dst || is_mask) { + if (flow->tunnel.ip_dst || export_mask) { tun_key_to_attr(buf, &data->tunnel); } 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); } /* Add an ingress port attribute if this is a mask or 'odp_in_port' * is not the magical value "ODPP_NONE". */ - if (is_mask || odp_in_port != ODPP_NONE) { + if (export_mask || odp_in_port != ODPP_NONE) { nl_msg_put_odp_port(buf, OVS_KEY_ATTR_IN_PORT, odp_in_port); } @@ -2521,7 +2517,7 @@ odp_flow_key_from_flow__(struct ofpbuf *buf, const struct flow *data, memcpy(eth_key->eth_dst, data->dl_dst, ETH_ADDR_LEN); if (flow->vlan_tci != htons(0) || flow->dl_type == htons(ETH_TYPE_VLAN)) { - if (is_mask) { + if (export_mask) { nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, OVS_BE16_MAX); } else { nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, htons(ETH_TYPE_VLAN)); @@ -2547,7 +2543,7 @@ odp_flow_key_from_flow__(struct ofpbuf *buf, const struct flow *data, * 0xffff Any non-Ethernet II frame (except valid * 802.3 SNAP packet with valid eth_type). */ - if (is_mask) { + if (export_mask) { nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, OVS_BE16_MAX); } goto unencap; @@ -2565,7 +2561,7 @@ odp_flow_key_from_flow__(struct ofpbuf *buf, const struct flow *data, ipv4_key->ipv4_proto = data->nw_proto; ipv4_key->ipv4_tos = data->nw_tos; ipv4_key->ipv4_ttl = data->nw_ttl; - ipv4_key->ipv4_frag = is_mask ? ovs_to_odp_frag_mask(data->nw_frag) + ipv4_key->ipv4_frag = export_mask ? ovs_to_odp_frag_mask(data->nw_frag) : ovs_to_odp_frag(data->nw_frag); } else if (flow->dl_type == htons(ETH_TYPE_IPV6)) { struct ovs_key_ipv6 *ipv6_key; @@ -2578,7 +2574,7 @@ odp_flow_key_from_flow__(struct ofpbuf *buf, const struct flow *data, ipv6_key->ipv6_proto = data->nw_proto; ipv6_key->ipv6_tclass = data->nw_tos; ipv6_key->ipv6_hlimit = data->nw_ttl; - ipv6_key->ipv6_frag = is_mask ? ovs_to_odp_frag_mask(data->nw_frag) + ipv6_key->ipv6_frag = export_mask ? ovs_to_odp_frag_mask(data->nw_frag) : ovs_to_odp_frag(data->nw_frag); } else if (flow->dl_type == htons(ETH_TYPE_ARP) || flow->dl_type == htons(ETH_TYPE_RARP)) { @@ -2650,7 +2646,7 @@ odp_flow_key_from_flow__(struct ofpbuf *buf, const struct flow *data, if (flow->tp_dst == htons(0) && (flow->tp_src == htons(ND_NEIGHBOR_SOLICIT) || flow->tp_src == htons(ND_NEIGHBOR_ADVERT)) && - (!is_mask || (data->tp_src == htons(0xffff) && + (!export_mask || (data->tp_src == htons(0xffff) && data->tp_dst == htons(0xffff)))) { struct ovs_key_nd *nd_key; @@ -2681,9 +2677,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, mask, odp_in_port, SIZE_MAX, false); } /* Appends a representation of 'mask' as OVS_KEY_ATTR_* attributes to @@ -2699,8 +2695,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, flow, mask, + u32_to_odp(odp_in_port_mask), max_mpls_depth, true); } /* Generate ODP flow key from the given packet metadata */ diff --git a/lib/odp-util.h b/lib/odp-util.h index 7bc64c7eb..0dfbccaa9 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 fe02b5101..6a725e465 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -963,7 +963,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, @@ -1096,7 +1096,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 8201d6b7a..c5a01a1f3 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 c68353ed9..30cdbbf2e 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", -- 2.43.0