| Message ID | 20190110085110.4900-3-elibr@mellanox.com |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
| Series | Do not rewrite fields with the same values as | expand |
Hi, When I try to understand this patch, I feel there may be some issue in this loop below. It looks like each loop is doing the same thing. Can you please take a look? + for (i = 0; i < size; i++) + if (memcmp(pkey0, pkey1, size) == 0) + memset(pmask, 0, size); + else + differ = true; Thanks, Yifeng On Thu, Jan 10, 2019 at 12:52 AM Eli Britstein <elibr@mellanox.com> wrote: > To improve performance and avoid wasting resources for HW offloaded > flows, do not rewrite fields that are matched with the same value. > > Signed-off-by: Eli Britstein <elibr@mellanox.com> > Reviewed-by: Roi Dayan <roid@mellanox.com> > --- > lib/odp-util.c | 110 > +++++++++++++++++++++++++++++++++++++++++++++----- > tests/mpls-xlate.at | 2 +- > tests/ofproto-dpif.at | 14 +++---- > 3 files changed, 108 insertions(+), 18 deletions(-) > > diff --git a/lib/odp-util.c b/lib/odp-util.c > index 0491bed38..7e916f9d9 100644 > --- a/lib/odp-util.c > +++ b/lib/odp-util.c > @@ -7086,12 +7086,50 @@ commit_odp_tunnel_action(const struct flow *flow, > struct flow *base, > } > } > > +struct ovs_key_field_properties { > + int offset; > + int size; > +}; > + > +/* Compare the keys similary to memcmp, but each field separately. > + * The offsets and sizes of each field is provided by field_properties > + * argument. > + * For fields with the same value, zero out their mask part in order not > to > + * rewrite them as it's unnecessary */ > +static bool > +keycmp_mask(const void *key0, const void *key1, > + struct ovs_key_field_properties *field_properties, void *mask) > +{ > + int field, i; > + bool differ = false; > + > + for (field = 0 ; ; field++) { > + int size = field_properties[field].size; > + int offset = field_properties[field].offset; > + char *pkey0 = ((char *)key0) + offset; > + char *pkey1 = ((char *)key1) + offset; > + char *pmask = ((char *)mask) + offset; > + > + if (size == 0) > + break; > + > + for (i = 0; i < size; i++) > + if (memcmp(pkey0, pkey1, size) == 0) > + memset(pmask, 0, size); > + else > + differ = true; > + } > + > + return differ; > +} > + > static bool > commit(enum ovs_key_attr attr, bool use_masked_set, > const void *key, void *base, void *mask, size_t size, > + struct ovs_key_field_properties *field_properties, > struct ofpbuf *odp_actions) > { > - if (memcmp(key, base, size)) { > + if (keycmp_mask(key, base, field_properties, mask)) { > bool fully_masked = odp_mask_is_exact(attr, mask, size); > > if (use_masked_set && !fully_masked) { > @@ -7133,6 +7171,12 @@ commit_set_ether_action(const struct flow *flow, > struct flow *base_flow, > bool use_masked) > { > struct ovs_key_ethernet key, base, mask; > + struct ovs_key_field_properties ovs_key_ethernet_field_properties[] = > { > +#define OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_ethernet, > name), sizeof(type)}, > + OVS_KEY_ETHERNET_FIELDS > + {0, 0} > +#undef OVS_KEY_FIELD > + }; > > if (flow->packet_type != htonl(PT_ETH)) { > return; > @@ -7143,7 +7187,8 @@ commit_set_ether_action(const struct flow *flow, > struct flow *base_flow, > get_ethernet_key(&wc->masks, &mask); > > if (commit(OVS_KEY_ATTR_ETHERNET, use_masked, > - &key, &base, &mask, sizeof key, odp_actions)) { > + &key, &base, &mask, sizeof key, > + ovs_key_ethernet_field_properties, odp_actions)) { > put_ethernet_key(&base, base_flow); > put_ethernet_key(&mask, &wc->masks); > } > @@ -7269,6 +7314,12 @@ commit_set_ipv4_action(const struct flow *flow, > struct flow *base_flow, > bool use_masked) > { > struct ovs_key_ipv4 key, mask, base; > + struct ovs_key_field_properties ovs_key_ipv4_field_properties[] = { > +#define OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_ipv4, name), > sizeof(type)}, > + OVS_KEY_IPV4_FIELDS > + {0, 0} > +#undef OVS_KEY_FIELD > + }; > > /* Check that nw_proto and nw_frag remain unchanged. */ > ovs_assert(flow->nw_proto == base_flow->nw_proto && > @@ -7286,7 +7337,7 @@ commit_set_ipv4_action(const struct flow *flow, > struct flow *base_flow, > } > > if (commit(OVS_KEY_ATTR_IPV4, use_masked, &key, &base, &mask, sizeof > key, > - odp_actions)) { > + ovs_key_ipv4_field_properties, odp_actions)) { > put_ipv4_key(&base, base_flow, false); > if (mask.ipv4_proto != 0) { /* Mask was changed by commit(). */ > put_ipv4_key(&mask, &wc->masks, true); > @@ -7324,6 +7375,12 @@ commit_set_ipv6_action(const struct flow *flow, > struct flow *base_flow, > bool use_masked) > { > struct ovs_key_ipv6 key, mask, base; > + struct ovs_key_field_properties ovs_key_ipv6_field_properties[] = { > +#define OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_ipv6, name), > sizeof(type)}, > + OVS_KEY_IPV6_FIELDS > + {0, 0} > +#undef OVS_KEY_FIELD > + }; > > /* Check that nw_proto and nw_frag remain unchanged. */ > ovs_assert(flow->nw_proto == base_flow->nw_proto && > @@ -7342,7 +7399,7 @@ commit_set_ipv6_action(const struct flow *flow, > struct flow *base_flow, > } > > if (commit(OVS_KEY_ATTR_IPV6, use_masked, &key, &base, &mask, sizeof > key, > - odp_actions)) { > + ovs_key_ipv6_field_properties, odp_actions)) { > put_ipv6_key(&base, base_flow, false); > if (mask.ipv6_proto != 0) { /* Mask was changed by commit(). */ > put_ipv6_key(&mask, &wc->masks, true); > @@ -7378,13 +7435,19 @@ commit_set_arp_action(const struct flow *flow, > struct flow *base_flow, > struct ofpbuf *odp_actions, struct flow_wildcards > *wc) > { > struct ovs_key_arp key, mask, base; > + struct ovs_key_field_properties ovs_key_arp_field_properties[] = { > +#define OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_arp, name), > sizeof(type)}, > + OVS_KEY_ARP_FIELDS > + {0, 0} > +#undef OVS_KEY_FIELD > + }; > > get_arp_key(flow, &key); > get_arp_key(base_flow, &base); > get_arp_key(&wc->masks, &mask); > > if (commit(OVS_KEY_ATTR_ARP, true, &key, &base, &mask, sizeof key, > - odp_actions)) { > + ovs_key_arp_field_properties, odp_actions)) { > put_arp_key(&base, base_flow); > put_arp_key(&mask, &wc->masks); > return SLOW_ACTION; > @@ -7413,6 +7476,12 @@ commit_set_icmp_action(const struct flow *flow, > struct flow *base_flow, > struct ofpbuf *odp_actions, struct flow_wildcards > *wc) > { > struct ovs_key_icmp key, mask, base; > + struct ovs_key_field_properties ovs_key_icmp_field_properties[] = { > +#define OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_icmp, name), > sizeof(type)}, > + OVS_KEY_ICMP_FIELDS > + {0, 0} > +#undef OVS_KEY_FIELD > + }; > enum ovs_key_attr attr; > > if (is_icmpv4(flow, NULL)) { > @@ -7427,7 +7496,8 @@ commit_set_icmp_action(const struct flow *flow, > struct flow *base_flow, > get_icmp_key(base_flow, &base); > get_icmp_key(&wc->masks, &mask); > > - if (commit(attr, false, &key, &base, &mask, sizeof key, odp_actions)) > { > + if (commit(attr, false, &key, &base, &mask, sizeof key, > + ovs_key_icmp_field_properties, odp_actions)) { > put_icmp_key(&base, base_flow); > put_icmp_key(&mask, &wc->masks); > return SLOW_ACTION; > @@ -7459,13 +7529,19 @@ commit_set_nd_action(const struct flow *flow, > struct flow *base_flow, > struct flow_wildcards *wc, bool use_masked) > { > struct ovs_key_nd key, mask, base; > + struct ovs_key_field_properties ovs_key_nd_field_properties[] = { > +#define OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_nd, name), > sizeof(type)}, > + OVS_KEY_ND_FIELDS > + {0, 0} > +#undef OVS_KEY_FIELD > + }; > > get_nd_key(flow, &key); > get_nd_key(base_flow, &base); > get_nd_key(&wc->masks, &mask); > > if (commit(OVS_KEY_ATTR_ND, use_masked, &key, &base, &mask, sizeof > key, > - odp_actions)) { > + ovs_key_nd_field_properties, odp_actions)) { > put_nd_key(&base, base_flow); > put_nd_key(&mask, &wc->masks); > return SLOW_ACTION; > @@ -7672,6 +7748,12 @@ commit_set_port_action(const struct flow *flow, > struct flow *base_flow, > { > enum ovs_key_attr key_type; > union ovs_key_tp key, mask, base; > + struct ovs_key_field_properties ovs_key_tp_field_properties[] = { > +#define OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_tcp, name), > sizeof(type)}, > + OVS_KEY_TCP_FIELDS > + {0, 0} > +#undef OVS_KEY_FIELD > + }; > > /* Check if 'flow' really has an L3 header. */ > if (!flow->nw_proto) { > @@ -7697,7 +7779,7 @@ commit_set_port_action(const struct flow *flow, > struct flow *base_flow, > get_tp_key(&wc->masks, &mask); > > if (commit(key_type, use_masked, &key, &base, &mask, sizeof key, > - odp_actions)) { > + ovs_key_tp_field_properties, odp_actions)) { > put_tp_key(&base, base_flow); > put_tp_key(&mask, &wc->masks); > } > @@ -7710,13 +7792,17 @@ commit_set_priority_action(const struct flow > *flow, struct flow *base_flow, > bool use_masked) > { > uint32_t key, mask, base; > + struct ovs_key_field_properties ovs_key_prio_field_properties[] = { > + {0, sizeof(uint32_t)}, > + {0, 0} > + }; > > key = flow->skb_priority; > base = base_flow->skb_priority; > mask = wc->masks.skb_priority; > > if (commit(OVS_KEY_ATTR_PRIORITY, use_masked, &key, &base, &mask, > - sizeof key, odp_actions)) { > + sizeof key, ovs_key_prio_field_properties, odp_actions)) { > base_flow->skb_priority = base; > wc->masks.skb_priority = mask; > } > @@ -7729,13 +7815,17 @@ commit_set_pkt_mark_action(const struct flow > *flow, struct flow *base_flow, > bool use_masked) > { > uint32_t key, mask, base; > + struct ovs_key_field_properties ovs_key_pkt_mark_field_properties[] = > { > + {0, sizeof(uint32_t)}, > + {0, 0} > + }; > > key = flow->pkt_mark; > base = base_flow->pkt_mark; > mask = wc->masks.pkt_mark; > > if (commit(OVS_KEY_ATTR_SKB_MARK, use_masked, &key, &base, &mask, > - sizeof key, odp_actions)) { > + sizeof key, ovs_key_pkt_mark_field_properties, > odp_actions)) { > base_flow->pkt_mark = base; > wc->masks.pkt_mark = mask; > } > diff --git a/tests/mpls-xlate.at b/tests/mpls-xlate.at > index 4392f7baa..a9ba62215 100644 > --- a/tests/mpls-xlate.at > +++ b/tests/mpls-xlate.at > @@ -182,7 +182,7 @@ AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br1 > in_port=1,ip,actions=dec_ttl,push > dnl MPLS push+pop > AT_CHECK([ovs-appctl ofproto/trace ovs-dummy > 'in_port(100),eth(src=f8:bc:12:44:34:b6,dst=f8:bc:12:46:58:e0),eth_type(0x0800),ipv4(src=10.1.1.22,dst=10.0.0.3,proto=6,tos=0,ttl=64,frag=no),tcp(src=53295,dst=8080)'], > [0], [stdout]) > AT_CHECK([tail -1 stdout], [0], > - [Datapath actions: > set(ipv4(ttl=63)),push_mpls(label=0,tc=0,ttl=63,bos=1,eth_type=0x8847),3,pop_mpls(eth_type=0x800),set(ipv4(tos=0/0xfc,ttl=64)),1,1 > + [Datapath actions: > set(ipv4(ttl=63)),push_mpls(label=0,tc=0,ttl=63,bos=1,eth_type=0x8847),3,pop_mpls(eth_type=0x800),set(ipv4(ttl=64)),1,1 > ]) > > OVS_VSWITCHD_STOP > diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at > index ded2ef013..4c69d09be 100644 > --- a/tests/ofproto-dpif.at > +++ b/tests/ofproto-dpif.at > @@ -4346,7 +4346,7 @@ do > elif test $type = later; then > echo "Datapath actions: $exp_output" >> expout > else > - echo "Datapath actions: set(tcp(src=80,dst=80)),$exp_output" >> > expout > + echo "Datapath actions: set(tcp(src=80)),$exp_output" >> expout > fi > AT_CHECK([grep 'IP fragments' stdout; tail -1 stdout], [0], [expout]) > done > @@ -7252,7 +7252,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt], [0], > [ignore]) > AT_CHECK([ovs-appctl ofproto/trace ovs-dummy > 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no),icmp(type=8,code=0)'], > [0], [stdout]) > > AT_CHECK([tail -1 stdout], [0], [dnl > -Datapath actions: > set(ipv4(src=10.10.10.2,dst=192.168.4.4)),2,set(eth(src=80:81:81:81:81:81)),set(ipv4(src=10.10.10.2,dst=192.168.5.5)),3,set(eth(src=50:54:00:00:00:09)),set(ipv4(src=10.10.10.2,dst=10.10.10.1)),4 > +Datapath actions: > set(ipv4(dst=192.168.4.4)),2,set(eth(src=80:81:81:81:81:81)),set(ipv4(dst=192.168.5.5)),3,set(eth(src=50:54:00:00:00:09)),set(ipv4(dst=10.10.10.1)),4 > ]) > > dnl Test flow xlate openflow clone action without using datapath clone > action. > @@ -7261,14 +7261,14 @@ AT_CHECK([ovs-appctl dpif/set-dp-features br0 > clone false], [0], [ignore]) > AT_CHECK([ovs-appctl ofproto/trace ovs-dummy > 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no),icmp(type=8,code=0)'], > [0], [stdout]) > > AT_CHECK([tail -1 stdout], [0], [dnl > -Datapath actions: > set(ipv4(src=10.10.10.2,dst=192.168.4.4)),2,set(eth(src=80:81:81:81:81:81)),set(ipv4(src=10.10.10.2,dst=192.168.5.5)),3,set(eth(src=50:54:00:00:00:09)),set(ipv4(src=10.10.10.2,dst=10.10.10.1)),4 > +Datapath actions: > set(ipv4(dst=192.168.4.4)),2,set(eth(src=80:81:81:81:81:81)),set(ipv4(dst=192.168.5.5)),3,set(eth(src=50:54:00:00:00:09)),set(ipv4(dst=10.10.10.1)),4 > ]) > > AT_CHECK([ovs-appctl dpif/set-dp-features br0 sample_nesting 2], [0], > [ignore]) > AT_CHECK([ovs-appctl ofproto/trace ovs-dummy > 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no),icmp(type=8,code=0)'], > [0], [stdout]) > > AT_CHECK([tail -1 stdout], [0], [dnl > -Datapath actions: > set(ipv4(src=10.10.10.2,dst=192.168.4.4)),2,set(eth(src=80:81:81:81:81:81)),set(ipv4(src=10.10.10.2,dst=192.168.5.5)),3,set(eth(src=50:54:00:00:00:09)),set(ipv4(src=10.10.10.2,dst=10.10.10.1)),4 > +Datapath actions: > set(ipv4(dst=192.168.4.4)),2,set(eth(src=80:81:81:81:81:81)),set(ipv4(dst=192.168.5.5)),3,set(eth(src=50:54:00:00:00:09)),set(ipv4(dst=10.10.10.1)),4 > ]) > > dnl Mixing reversible and irreversible open flow clone actions. Datapath > clone action > @@ -7286,7 +7286,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt], [0], > [ignore]) > AT_CHECK([ovs-appctl ofproto/trace ovs-dummy > 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no),icmp(type=8,code=0)'], > [0], [stdout]) > > AT_CHECK([tail -1 stdout], [0], [dnl > -Datapath actions: > set(ipv4(src=10.10.10.2,dst=192.168.4.4)),2,set(ipv4(src=10.10.10.2,dst=10.10.10.1)),clone(ct(commit),3),4 > +Datapath actions: > set(ipv4(dst=192.168.4.4)),2,set(ipv4(dst=10.10.10.1)),clone(ct(commit),3),4 > ]) > > AT_CHECK([ovs-appctl dpif/set-dp-features br0 clone false], [0], [ignore]) > @@ -7294,14 +7294,14 @@ AT_CHECK([ovs-appctl dpif/set-dp-features br0 > clone false], [0], [ignore]) > AT_CHECK([ovs-appctl ofproto/trace ovs-dummy > 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no),icmp(type=8,code=0)'], > [0], [stdout]) > > AT_CHECK([tail -1 stdout], [0], [dnl > -Datapath actions: > set(ipv4(src=10.10.10.2,dst=192.168.4.4)),2,set(ipv4(src=10.10.10.2,dst=10.10.10.1)),sample(sample=100.0%,actions(ct(commit),3)),4 > +Datapath actions: > set(ipv4(dst=192.168.4.4)),2,set(ipv4(dst=10.10.10.1)),sample(sample=100.0%,actions(ct(commit),3)),4 > ]) > > AT_CHECK([ovs-appctl dpif/set-dp-features br0 sample_nesting 2], [0], > [ignore]) > AT_CHECK([ovs-appctl ofproto/trace ovs-dummy > 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no),icmp(type=8,code=0)'], > [0], [stdout]) > > AT_CHECK([tail -1 stdout], [0], [dnl > -Datapath actions: > set(ipv4(src=10.10.10.2,dst=192.168.4.4)),2,set(ipv4(src=10.10.10.2,dst=10.10.10.1)),4 > +Datapath actions: set(ipv4(dst=192.168.4.4)),2,set(ipv4(dst=10.10.10.1)),4 > ]) > AT_CHECK([grep "Failed to compose clone action" stdout], [0], [ignore]) > > -- > 2.14.5 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
You are right. I'll fix it. On 1/10/2019 9:35 PM, Yifeng Sun wrote: Hi, When I try to understand this patch, I feel there may be some issue in this loop below. It looks like each loop is doing the same thing. Can you please take a look? + for (i = 0; i < size; i++) + if (memcmp(pkey0, pkey1, size) == 0) + memset(pmask, 0, size); + else + differ = true; Thanks, Yifeng On Thu, Jan 10, 2019 at 12:52 AM Eli Britstein <elibr@mellanox.com<mailto:elibr@mellanox.com>> wrote: To improve performance and avoid wasting resources for HW offloaded flows, do not rewrite fields that are matched with the same value. Signed-off-by: Eli Britstein <elibr@mellanox.com<mailto:elibr@mellanox.com>> Reviewed-by: Roi Dayan <roid@mellanox.com<mailto:roid@mellanox.com>> --- lib/odp-util.c | 110 +++++++++++++++++++++++++++++++++++++++++++++----- tests/mpls-xlate.at<https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmpls-xlate.at&data=02%7C01%7Celibr%40mellanox.com%7C8adf0dc78b1e4ac8de3608d67732cd09%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636827457423592048&sdata=z9SB16ZNpp2UA7LjYXZR02TED%2BsSY%2FAgCTSG5hh8b2U%3D&reserved=0> | 2 +- tests/ofproto-dpif.at<https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fofproto-dpif.at&data=02%7C01%7Celibr%40mellanox.com%7C8adf0dc78b1e4ac8de3608d67732cd09%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636827457423592048&sdata=w%2FdeqIvO3ay%2FO6zi9L%2BPyRglmEI0krk4CtylHXyyVwk%3D&reserved=0> | 14 +++---- 3 files changed, 108 insertions(+), 18 deletions(-) diff --git a/lib/odp-util.c b/lib/odp-util.c index 0491bed38..7e916f9d9 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -7086,12 +7086,50 @@ commit_odp_tunnel_action(const struct flow *flow, struct flow *base, } } +struct ovs_key_field_properties { + int offset; + int size; +}; + +/* Compare the keys similary to memcmp, but each field separately. + * The offsets and sizes of each field is provided by field_properties + * argument. + * For fields with the same value, zero out their mask part in order not to + * rewrite them as it's unnecessary */ +static bool +keycmp_mask(const void *key0, const void *key1, + struct ovs_key_field_properties *field_properties, void *mask) +{ + int field, i; + bool differ = false; + + for (field = 0 ; ; field++) { + int size = field_properties[field].size; + int offset = field_properties[field].offset; + char *pkey0 = ((char *)key0) + offset; + char *pkey1 = ((char *)key1) + offset; + char *pmask = ((char *)mask) + offset; + + if (size == 0) + break; + + for (i = 0; i < size; i++) + if (memcmp(pkey0, pkey1, size) == 0) + memset(pmask, 0, size); + else + differ = true; + } + + return differ; +} + static bool commit(enum ovs_key_attr attr, bool use_masked_set, const void *key, void *base, void *mask, size_t size, + struct ovs_key_field_properties *field_properties, struct ofpbuf *odp_actions) { - if (memcmp(key, base, size)) { + if (keycmp_mask(key, base, field_properties, mask)) { bool fully_masked = odp_mask_is_exact(attr, mask, size); if (use_masked_set && !fully_masked) { @@ -7133,6 +7171,12 @@ commit_set_ether_action(const struct flow *flow, struct flow *base_flow, bool use_masked) { struct ovs_key_ethernet key, base, mask; + struct ovs_key_field_properties ovs_key_ethernet_field_properties[] = { +#define OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_ethernet, name), sizeof(type)}, + OVS_KEY_ETHERNET_FIELDS + {0, 0} +#undef OVS_KEY_FIELD + }; if (flow->packet_type != htonl(PT_ETH)) { return; @@ -7143,7 +7187,8 @@ commit_set_ether_action(const struct flow *flow, struct flow *base_flow, get_ethernet_key(&wc->masks, &mask); if (commit(OVS_KEY_ATTR_ETHERNET, use_masked, - &key, &base, &mask, sizeof key, odp_actions)) { + &key, &base, &mask, sizeof key, + ovs_key_ethernet_field_properties, odp_actions)) { put_ethernet_key(&base, base_flow); put_ethernet_key(&mask, &wc->masks); } @@ -7269,6 +7314,12 @@ commit_set_ipv4_action(const struct flow *flow, struct flow *base_flow, bool use_masked) { struct ovs_key_ipv4 key, mask, base; + struct ovs_key_field_properties ovs_key_ipv4_field_properties[] = { +#define OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_ipv4, name), sizeof(type)}, + OVS_KEY_IPV4_FIELDS + {0, 0} +#undef OVS_KEY_FIELD + }; /* Check that nw_proto and nw_frag remain unchanged. */ ovs_assert(flow->nw_proto == base_flow->nw_proto && @@ -7286,7 +7337,7 @@ commit_set_ipv4_action(const struct flow *flow, struct flow *base_flow, } if (commit(OVS_KEY_ATTR_IPV4, use_masked, &key, &base, &mask, sizeof key, - odp_actions)) { + ovs_key_ipv4_field_properties, odp_actions)) { put_ipv4_key(&base, base_flow, false); if (mask.ipv4_proto != 0) { /* Mask was changed by commit(). */ put_ipv4_key(&mask, &wc->masks, true); @@ -7324,6 +7375,12 @@ commit_set_ipv6_action(const struct flow *flow, struct flow *base_flow, bool use_masked) { struct ovs_key_ipv6 key, mask, base; + struct ovs_key_field_properties ovs_key_ipv6_field_properties[] = { +#define OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_ipv6, name), sizeof(type)}, + OVS_KEY_IPV6_FIELDS + {0, 0} +#undef OVS_KEY_FIELD + }; /* Check that nw_proto and nw_frag remain unchanged. */ ovs_assert(flow->nw_proto == base_flow->nw_proto && @@ -7342,7 +7399,7 @@ commit_set_ipv6_action(const struct flow *flow, struct flow *base_flow, } if (commit(OVS_KEY_ATTR_IPV6, use_masked, &key, &base, &mask, sizeof key, - odp_actions)) { + ovs_key_ipv6_field_properties, odp_actions)) { put_ipv6_key(&base, base_flow, false); if (mask.ipv6_proto != 0) { /* Mask was changed by commit(). */ put_ipv6_key(&mask, &wc->masks, true); @@ -7378,13 +7435,19 @@ commit_set_arp_action(const struct flow *flow, struct flow *base_flow, struct ofpbuf *odp_actions, struct flow_wildcards *wc) { struct ovs_key_arp key, mask, base; + struct ovs_key_field_properties ovs_key_arp_field_properties[] = { +#define OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_arp, name), sizeof(type)}, + OVS_KEY_ARP_FIELDS + {0, 0} +#undef OVS_KEY_FIELD + }; get_arp_key(flow, &key); get_arp_key(base_flow, &base); get_arp_key(&wc->masks, &mask); if (commit(OVS_KEY_ATTR_ARP, true, &key, &base, &mask, sizeof key, - odp_actions)) { + ovs_key_arp_field_properties, odp_actions)) { put_arp_key(&base, base_flow); put_arp_key(&mask, &wc->masks); return SLOW_ACTION; @@ -7413,6 +7476,12 @@ commit_set_icmp_action(const struct flow *flow, struct flow *base_flow, struct ofpbuf *odp_actions, struct flow_wildcards *wc) { struct ovs_key_icmp key, mask, base; + struct ovs_key_field_properties ovs_key_icmp_field_properties[] = { +#define OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_icmp, name), sizeof(type)}, + OVS_KEY_ICMP_FIELDS + {0, 0} +#undef OVS_KEY_FIELD + }; enum ovs_key_attr attr; if (is_icmpv4(flow, NULL)) { @@ -7427,7 +7496,8 @@ commit_set_icmp_action(const struct flow *flow, struct flow *base_flow, get_icmp_key(base_flow, &base); get_icmp_key(&wc->masks, &mask); - if (commit(attr, false, &key, &base, &mask, sizeof key, odp_actions)) { + if (commit(attr, false, &key, &base, &mask, sizeof key, + ovs_key_icmp_field_properties, odp_actions)) { put_icmp_key(&base, base_flow); put_icmp_key(&mask, &wc->masks); return SLOW_ACTION; @@ -7459,13 +7529,19 @@ commit_set_nd_action(const struct flow *flow, struct flow *base_flow, struct flow_wildcards *wc, bool use_masked) { struct ovs_key_nd key, mask, base; + struct ovs_key_field_properties ovs_key_nd_field_properties[] = { +#define OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_nd, name), sizeof(type)}, + OVS_KEY_ND_FIELDS + {0, 0} +#undef OVS_KEY_FIELD + }; get_nd_key(flow, &key); get_nd_key(base_flow, &base); get_nd_key(&wc->masks, &mask); if (commit(OVS_KEY_ATTR_ND, use_masked, &key, &base, &mask, sizeof key, - odp_actions)) { + ovs_key_nd_field_properties, odp_actions)) { put_nd_key(&base, base_flow); put_nd_key(&mask, &wc->masks); return SLOW_ACTION; @@ -7672,6 +7748,12 @@ commit_set_port_action(const struct flow *flow, struct flow *base_flow, { enum ovs_key_attr key_type; union ovs_key_tp key, mask, base; + struct ovs_key_field_properties ovs_key_tp_field_properties[] = { +#define OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_tcp, name), sizeof(type)}, + OVS_KEY_TCP_FIELDS + {0, 0} +#undef OVS_KEY_FIELD + }; /* Check if 'flow' really has an L3 header. */ if (!flow->nw_proto) { @@ -7697,7 +7779,7 @@ commit_set_port_action(const struct flow *flow, struct flow *base_flow, get_tp_key(&wc->masks, &mask); if (commit(key_type, use_masked, &key, &base, &mask, sizeof key, - odp_actions)) { + ovs_key_tp_field_properties, odp_actions)) { put_tp_key(&base, base_flow); put_tp_key(&mask, &wc->masks); } @@ -7710,13 +7792,17 @@ commit_set_priority_action(const struct flow *flow, struct flow *base_flow, bool use_masked) { uint32_t key, mask, base; + struct ovs_key_field_properties ovs_key_prio_field_properties[] = { + {0, sizeof(uint32_t)}, + {0, 0} + }; key = flow->skb_priority; base = base_flow->skb_priority; mask = wc->masks.skb_priority; if (commit(OVS_KEY_ATTR_PRIORITY, use_masked, &key, &base, &mask, - sizeof key, odp_actions)) { + sizeof key, ovs_key_prio_field_properties, odp_actions)) { base_flow->skb_priority = base; wc->masks.skb_priority = mask; } @@ -7729,13 +7815,17 @@ commit_set_pkt_mark_action(const struct flow *flow, struct flow *base_flow, bool use_masked) { uint32_t key, mask, base; + struct ovs_key_field_properties ovs_key_pkt_mark_field_properties[] = { + {0, sizeof(uint32_t)}, + {0, 0} + }; key = flow->pkt_mark; base = base_flow->pkt_mark; mask = wc->masks.pkt_mark; if (commit(OVS_KEY_ATTR_SKB_MARK, use_masked, &key, &base, &mask, - sizeof key, odp_actions)) { + sizeof key, ovs_key_pkt_mark_field_properties, odp_actions)) { base_flow->pkt_mark = base; wc->masks.pkt_mark = mask; } diff --git a/tests/mpls-xlate.at<https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmpls-xlate.at&data=02%7C01%7Celibr%40mellanox.com%7C8adf0dc78b1e4ac8de3608d67732cd09%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636827457423602053&sdata=A1DvxGyhSteeFmTk0xJSc0z%2BZYnzNjgZqol6PQrMt2w%3D&reserved=0> b/tests/mpls-xlate.at<https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmpls-xlate.at&data=02%7C01%7Celibr%40mellanox.com%7C8adf0dc78b1e4ac8de3608d67732cd09%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636827457423602053&sdata=A1DvxGyhSteeFmTk0xJSc0z%2BZYnzNjgZqol6PQrMt2w%3D&reserved=0> index 4392f7baa..a9ba62215 100644 --- a/tests/mpls-xlate.at<https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmpls-xlate.at&data=02%7C01%7Celibr%40mellanox.com%7C8adf0dc78b1e4ac8de3608d67732cd09%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636827457423612058&sdata=W8DxhNxtNi1XX7X1TLHiJVR%2BEHQ9JEorLgOnV3Ab4ys%3D&reserved=0> +++ b/tests/mpls-xlate.at<https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmpls-xlate.at&data=02%7C01%7Celibr%40mellanox.com%7C8adf0dc78b1e4ac8de3608d67732cd09%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636827457423612058&sdata=W8DxhNxtNi1XX7X1TLHiJVR%2BEHQ9JEorLgOnV3Ab4ys%3D&reserved=0> @@ -182,7 +182,7 @@ AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br1 in_port=1,ip,actions=dec_ttl,push dnl MPLS push+pop AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(100),eth(src=f8:bc:12:44:34:b6,dst=f8:bc:12:46:58:e0),eth_type(0x0800),ipv4(src=10.1.1.22,dst=10.0.0.3,proto=6,tos=0,ttl=64,frag=no),tcp(src=53295,dst=8080)'], [0], [stdout]) AT_CHECK([tail -1 stdout], [0], - [Datapath actions: set(ipv4(ttl=63)),push_mpls(label=0,tc=0,ttl=63,bos=1,eth_type=0x8847),3,pop_mpls(eth_type=0x800),set(ipv4(tos=0/0xfc,ttl=64)),1,1 + [Datapath actions: set(ipv4(ttl=63)),push_mpls(label=0,tc=0,ttl=63,bos=1,eth_type=0x8847),3,pop_mpls(eth_type=0x800),set(ipv4(ttl=64)),1,1 ]) OVS_VSWITCHD_STOP diff --git a/tests/ofproto-dpif.at<https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fofproto-dpif.at&data=02%7C01%7Celibr%40mellanox.com%7C8adf0dc78b1e4ac8de3608d67732cd09%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636827457423622067&sdata=Td5t%2FHhAdtoXMH6jBhzOwPOM813xUwj5%2FiZ%2B%2Fcu7fnc%3D&reserved=0> b/tests/ofproto-dpif.at<https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fofproto-dpif.at&data=02%7C01%7Celibr%40mellanox.com%7C8adf0dc78b1e4ac8de3608d67732cd09%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636827457423622067&sdata=Td5t%2FHhAdtoXMH6jBhzOwPOM813xUwj5%2FiZ%2B%2Fcu7fnc%3D&reserved=0> index ded2ef013..4c69d09be 100644 --- a/tests/ofproto-dpif.at<https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fofproto-dpif.at&data=02%7C01%7Celibr%40mellanox.com%7C8adf0dc78b1e4ac8de3608d67732cd09%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636827457423632072&sdata=HetybxbX%2B0JlmZ%2B4%2B6wUKlJvmutI6kCpVWrffnh0ZhY%3D&reserved=0> +++ b/tests/ofproto-dpif.at<https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fofproto-dpif.at&data=02%7C01%7Celibr%40mellanox.com%7C8adf0dc78b1e4ac8de3608d67732cd09%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636827457423632072&sdata=HetybxbX%2B0JlmZ%2B4%2B6wUKlJvmutI6kCpVWrffnh0ZhY%3D&reserved=0> @@ -4346,7 +4346,7 @@ do elif test $type = later; then echo "Datapath actions: $exp_output" >> expout else - echo "Datapath actions: set(tcp(src=80,dst=80)),$exp_output" >> expout + echo "Datapath actions: set(tcp(src=80)),$exp_output" >> expout fi AT_CHECK([grep 'IP fragments' stdout; tail -1 stdout], [0], [expout]) done @@ -7252,7 +7252,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt], [0], [ignore]) AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout]) AT_CHECK([tail -1 stdout], [0], [dnl -Datapath actions: set(ipv4(src=10.10.10.2,dst=192.168.4.4)),2,set(eth(src=80:81:81:81:81:81)),set(ipv4(src=10.10.10.2,dst=192.168.5.5)),3,set(eth(src=50:54:00:00:00:09)),set(ipv4(src=10.10.10.2,dst=10.10.10.1)),4 +Datapath actions: set(ipv4(dst=192.168.4.4)),2,set(eth(src=80:81:81:81:81:81)),set(ipv4(dst=192.168.5.5)),3,set(eth(src=50:54:00:00:00:09)),set(ipv4(dst=10.10.10.1)),4 ]) dnl Test flow xlate openflow clone action without using datapath clone action. @@ -7261,14 +7261,14 @@ AT_CHECK([ovs-appctl dpif/set-dp-features br0 clone false], [0], [ignore]) AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout]) AT_CHECK([tail -1 stdout], [0], [dnl -Datapath actions: set(ipv4(src=10.10.10.2,dst=192.168.4.4)),2,set(eth(src=80:81:81:81:81:81)),set(ipv4(src=10.10.10.2,dst=192.168.5.5)),3,set(eth(src=50:54:00:00:00:09)),set(ipv4(src=10.10.10.2,dst=10.10.10.1)),4 +Datapath actions: set(ipv4(dst=192.168.4.4)),2,set(eth(src=80:81:81:81:81:81)),set(ipv4(dst=192.168.5.5)),3,set(eth(src=50:54:00:00:00:09)),set(ipv4(dst=10.10.10.1)),4 ]) AT_CHECK([ovs-appctl dpif/set-dp-features br0 sample_nesting 2], [0], [ignore]) AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout]) AT_CHECK([tail -1 stdout], [0], [dnl -Datapath actions: set(ipv4(src=10.10.10.2,dst=192.168.4.4)),2,set(eth(src=80:81:81:81:81:81)),set(ipv4(src=10.10.10.2,dst=192.168.5.5)),3,set(eth(src=50:54:00:00:00:09)),set(ipv4(src=10.10.10.2,dst=10.10.10.1)),4 +Datapath actions: set(ipv4(dst=192.168.4.4)),2,set(eth(src=80:81:81:81:81:81)),set(ipv4(dst=192.168.5.5)),3,set(eth(src=50:54:00:00:00:09)),set(ipv4(dst=10.10.10.1)),4 ]) dnl Mixing reversible and irreversible open flow clone actions. Datapath clone action @@ -7286,7 +7286,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt], [0], [ignore]) AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout]) AT_CHECK([tail -1 stdout], [0], [dnl -Datapath actions: set(ipv4(src=10.10.10.2,dst=192.168.4.4)),2,set(ipv4(src=10.10.10.2,dst=10.10.10.1)),clone(ct(commit),3),4 +Datapath actions: set(ipv4(dst=192.168.4.4)),2,set(ipv4(dst=10.10.10.1)),clone(ct(commit),3),4 ]) AT_CHECK([ovs-appctl dpif/set-dp-features br0 clone false], [0], [ignore]) @@ -7294,14 +7294,14 @@ AT_CHECK([ovs-appctl dpif/set-dp-features br0 clone false], [0], [ignore]) AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout]) AT_CHECK([tail -1 stdout], [0], [dnl -Datapath actions: set(ipv4(src=10.10.10.2,dst=192.168.4.4)),2,set(ipv4(src=10.10.10.2,dst=10.10.10.1)),sample(sample=100.0%,actions(ct(commit),3)),4 +Datapath actions: set(ipv4(dst=192.168.4.4)),2,set(ipv4(dst=10.10.10.1)),sample(sample=100.0%,actions(ct(commit),3)),4 ]) AT_CHECK([ovs-appctl dpif/set-dp-features br0 sample_nesting 2], [0], [ignore]) AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout]) AT_CHECK([tail -1 stdout], [0], [dnl -Datapath actions: set(ipv4(src=10.10.10.2,dst=192.168.4.4)),2,set(ipv4(src=10.10.10.2,dst=10.10.10.1)),4 +Datapath actions: set(ipv4(dst=192.168.4.4)),2,set(ipv4(dst=10.10.10.1)),4 ]) AT_CHECK([grep "Failed to compose clone action" stdout], [0], [ignore]) -- 2.14.5
diff --git a/lib/odp-util.c b/lib/odp-util.c index 0491bed38..7e916f9d9 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -7086,12 +7086,50 @@ commit_odp_tunnel_action(const struct flow *flow, struct flow *base, } } +struct ovs_key_field_properties { + int offset; + int size; +}; + +/* Compare the keys similary to memcmp, but each field separately. + * The offsets and sizes of each field is provided by field_properties + * argument. + * For fields with the same value, zero out their mask part in order not to + * rewrite them as it's unnecessary */ +static bool +keycmp_mask(const void *key0, const void *key1, + struct ovs_key_field_properties *field_properties, void *mask) +{ + int field, i; + bool differ = false; + + for (field = 0 ; ; field++) { + int size = field_properties[field].size; + int offset = field_properties[field].offset; + char *pkey0 = ((char *)key0) + offset; + char *pkey1 = ((char *)key1) + offset; + char *pmask = ((char *)mask) + offset; + + if (size == 0) + break; + + for (i = 0; i < size; i++) + if (memcmp(pkey0, pkey1, size) == 0) + memset(pmask, 0, size); + else + differ = true; + } + + return differ; +} + static bool commit(enum ovs_key_attr attr, bool use_masked_set, const void *key, void *base, void *mask, size_t size, + struct ovs_key_field_properties *field_properties, struct ofpbuf *odp_actions) { - if (memcmp(key, base, size)) { + if (keycmp_mask(key, base, field_properties, mask)) { bool fully_masked = odp_mask_is_exact(attr, mask, size); if (use_masked_set && !fully_masked) { @@ -7133,6 +7171,12 @@ commit_set_ether_action(const struct flow *flow, struct flow *base_flow, bool use_masked) { struct ovs_key_ethernet key, base, mask; + struct ovs_key_field_properties ovs_key_ethernet_field_properties[] = { +#define OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_ethernet, name), sizeof(type)}, + OVS_KEY_ETHERNET_FIELDS + {0, 0} +#undef OVS_KEY_FIELD + }; if (flow->packet_type != htonl(PT_ETH)) { return; @@ -7143,7 +7187,8 @@ commit_set_ether_action(const struct flow *flow, struct flow *base_flow, get_ethernet_key(&wc->masks, &mask); if (commit(OVS_KEY_ATTR_ETHERNET, use_masked, - &key, &base, &mask, sizeof key, odp_actions)) { + &key, &base, &mask, sizeof key, + ovs_key_ethernet_field_properties, odp_actions)) { put_ethernet_key(&base, base_flow); put_ethernet_key(&mask, &wc->masks); } @@ -7269,6 +7314,12 @@ commit_set_ipv4_action(const struct flow *flow, struct flow *base_flow, bool use_masked) { struct ovs_key_ipv4 key, mask, base; + struct ovs_key_field_properties ovs_key_ipv4_field_properties[] = { +#define OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_ipv4, name), sizeof(type)}, + OVS_KEY_IPV4_FIELDS + {0, 0} +#undef OVS_KEY_FIELD + }; /* Check that nw_proto and nw_frag remain unchanged. */ ovs_assert(flow->nw_proto == base_flow->nw_proto && @@ -7286,7 +7337,7 @@ commit_set_ipv4_action(const struct flow *flow, struct flow *base_flow, } if (commit(OVS_KEY_ATTR_IPV4, use_masked, &key, &base, &mask, sizeof key, - odp_actions)) { + ovs_key_ipv4_field_properties, odp_actions)) { put_ipv4_key(&base, base_flow, false); if (mask.ipv4_proto != 0) { /* Mask was changed by commit(). */ put_ipv4_key(&mask, &wc->masks, true); @@ -7324,6 +7375,12 @@ commit_set_ipv6_action(const struct flow *flow, struct flow *base_flow, bool use_masked) { struct ovs_key_ipv6 key, mask, base; + struct ovs_key_field_properties ovs_key_ipv6_field_properties[] = { +#define OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_ipv6, name), sizeof(type)}, + OVS_KEY_IPV6_FIELDS + {0, 0} +#undef OVS_KEY_FIELD + }; /* Check that nw_proto and nw_frag remain unchanged. */ ovs_assert(flow->nw_proto == base_flow->nw_proto && @@ -7342,7 +7399,7 @@ commit_set_ipv6_action(const struct flow *flow, struct flow *base_flow, } if (commit(OVS_KEY_ATTR_IPV6, use_masked, &key, &base, &mask, sizeof key, - odp_actions)) { + ovs_key_ipv6_field_properties, odp_actions)) { put_ipv6_key(&base, base_flow, false); if (mask.ipv6_proto != 0) { /* Mask was changed by commit(). */ put_ipv6_key(&mask, &wc->masks, true); @@ -7378,13 +7435,19 @@ commit_set_arp_action(const struct flow *flow, struct flow *base_flow, struct ofpbuf *odp_actions, struct flow_wildcards *wc) { struct ovs_key_arp key, mask, base; + struct ovs_key_field_properties ovs_key_arp_field_properties[] = { +#define OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_arp, name), sizeof(type)}, + OVS_KEY_ARP_FIELDS + {0, 0} +#undef OVS_KEY_FIELD + }; get_arp_key(flow, &key); get_arp_key(base_flow, &base); get_arp_key(&wc->masks, &mask); if (commit(OVS_KEY_ATTR_ARP, true, &key, &base, &mask, sizeof key, - odp_actions)) { + ovs_key_arp_field_properties, odp_actions)) { put_arp_key(&base, base_flow); put_arp_key(&mask, &wc->masks); return SLOW_ACTION; @@ -7413,6 +7476,12 @@ commit_set_icmp_action(const struct flow *flow, struct flow *base_flow, struct ofpbuf *odp_actions, struct flow_wildcards *wc) { struct ovs_key_icmp key, mask, base; + struct ovs_key_field_properties ovs_key_icmp_field_properties[] = { +#define OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_icmp, name), sizeof(type)}, + OVS_KEY_ICMP_FIELDS + {0, 0} +#undef OVS_KEY_FIELD + }; enum ovs_key_attr attr; if (is_icmpv4(flow, NULL)) { @@ -7427,7 +7496,8 @@ commit_set_icmp_action(const struct flow *flow, struct flow *base_flow, get_icmp_key(base_flow, &base); get_icmp_key(&wc->masks, &mask); - if (commit(attr, false, &key, &base, &mask, sizeof key, odp_actions)) { + if (commit(attr, false, &key, &base, &mask, sizeof key, + ovs_key_icmp_field_properties, odp_actions)) { put_icmp_key(&base, base_flow); put_icmp_key(&mask, &wc->masks); return SLOW_ACTION; @@ -7459,13 +7529,19 @@ commit_set_nd_action(const struct flow *flow, struct flow *base_flow, struct flow_wildcards *wc, bool use_masked) { struct ovs_key_nd key, mask, base; + struct ovs_key_field_properties ovs_key_nd_field_properties[] = { +#define OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_nd, name), sizeof(type)}, + OVS_KEY_ND_FIELDS + {0, 0} +#undef OVS_KEY_FIELD + }; get_nd_key(flow, &key); get_nd_key(base_flow, &base); get_nd_key(&wc->masks, &mask); if (commit(OVS_KEY_ATTR_ND, use_masked, &key, &base, &mask, sizeof key, - odp_actions)) { + ovs_key_nd_field_properties, odp_actions)) { put_nd_key(&base, base_flow); put_nd_key(&mask, &wc->masks); return SLOW_ACTION; @@ -7672,6 +7748,12 @@ commit_set_port_action(const struct flow *flow, struct flow *base_flow, { enum ovs_key_attr key_type; union ovs_key_tp key, mask, base; + struct ovs_key_field_properties ovs_key_tp_field_properties[] = { +#define OVS_KEY_FIELD(type, name) {offsetof(struct ovs_key_tcp, name), sizeof(type)}, + OVS_KEY_TCP_FIELDS + {0, 0} +#undef OVS_KEY_FIELD + }; /* Check if 'flow' really has an L3 header. */ if (!flow->nw_proto) { @@ -7697,7 +7779,7 @@ commit_set_port_action(const struct flow *flow, struct flow *base_flow, get_tp_key(&wc->masks, &mask); if (commit(key_type, use_masked, &key, &base, &mask, sizeof key, - odp_actions)) { + ovs_key_tp_field_properties, odp_actions)) { put_tp_key(&base, base_flow); put_tp_key(&mask, &wc->masks); } @@ -7710,13 +7792,17 @@ commit_set_priority_action(const struct flow *flow, struct flow *base_flow, bool use_masked) { uint32_t key, mask, base; + struct ovs_key_field_properties ovs_key_prio_field_properties[] = { + {0, sizeof(uint32_t)}, + {0, 0} + }; key = flow->skb_priority; base = base_flow->skb_priority; mask = wc->masks.skb_priority; if (commit(OVS_KEY_ATTR_PRIORITY, use_masked, &key, &base, &mask, - sizeof key, odp_actions)) { + sizeof key, ovs_key_prio_field_properties, odp_actions)) { base_flow->skb_priority = base; wc->masks.skb_priority = mask; } @@ -7729,13 +7815,17 @@ commit_set_pkt_mark_action(const struct flow *flow, struct flow *base_flow, bool use_masked) { uint32_t key, mask, base; + struct ovs_key_field_properties ovs_key_pkt_mark_field_properties[] = { + {0, sizeof(uint32_t)}, + {0, 0} + }; key = flow->pkt_mark; base = base_flow->pkt_mark; mask = wc->masks.pkt_mark; if (commit(OVS_KEY_ATTR_SKB_MARK, use_masked, &key, &base, &mask, - sizeof key, odp_actions)) { + sizeof key, ovs_key_pkt_mark_field_properties, odp_actions)) { base_flow->pkt_mark = base; wc->masks.pkt_mark = mask; } diff --git a/tests/mpls-xlate.at b/tests/mpls-xlate.at index 4392f7baa..a9ba62215 100644 --- a/tests/mpls-xlate.at +++ b/tests/mpls-xlate.at @@ -182,7 +182,7 @@ AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br1 in_port=1,ip,actions=dec_ttl,push dnl MPLS push+pop AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(100),eth(src=f8:bc:12:44:34:b6,dst=f8:bc:12:46:58:e0),eth_type(0x0800),ipv4(src=10.1.1.22,dst=10.0.0.3,proto=6,tos=0,ttl=64,frag=no),tcp(src=53295,dst=8080)'], [0], [stdout]) AT_CHECK([tail -1 stdout], [0], - [Datapath actions: set(ipv4(ttl=63)),push_mpls(label=0,tc=0,ttl=63,bos=1,eth_type=0x8847),3,pop_mpls(eth_type=0x800),set(ipv4(tos=0/0xfc,ttl=64)),1,1 + [Datapath actions: set(ipv4(ttl=63)),push_mpls(label=0,tc=0,ttl=63,bos=1,eth_type=0x8847),3,pop_mpls(eth_type=0x800),set(ipv4(ttl=64)),1,1 ]) OVS_VSWITCHD_STOP diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index ded2ef013..4c69d09be 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -4346,7 +4346,7 @@ do elif test $type = later; then echo "Datapath actions: $exp_output" >> expout else - echo "Datapath actions: set(tcp(src=80,dst=80)),$exp_output" >> expout + echo "Datapath actions: set(tcp(src=80)),$exp_output" >> expout fi AT_CHECK([grep 'IP fragments' stdout; tail -1 stdout], [0], [expout]) done @@ -7252,7 +7252,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt], [0], [ignore]) AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout]) AT_CHECK([tail -1 stdout], [0], [dnl -Datapath actions: set(ipv4(src=10.10.10.2,dst=192.168.4.4)),2,set(eth(src=80:81:81:81:81:81)),set(ipv4(src=10.10.10.2,dst=192.168.5.5)),3,set(eth(src=50:54:00:00:00:09)),set(ipv4(src=10.10.10.2,dst=10.10.10.1)),4 +Datapath actions: set(ipv4(dst=192.168.4.4)),2,set(eth(src=80:81:81:81:81:81)),set(ipv4(dst=192.168.5.5)),3,set(eth(src=50:54:00:00:00:09)),set(ipv4(dst=10.10.10.1)),4 ]) dnl Test flow xlate openflow clone action without using datapath clone action. @@ -7261,14 +7261,14 @@ AT_CHECK([ovs-appctl dpif/set-dp-features br0 clone false], [0], [ignore]) AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout]) AT_CHECK([tail -1 stdout], [0], [dnl -Datapath actions: set(ipv4(src=10.10.10.2,dst=192.168.4.4)),2,set(eth(src=80:81:81:81:81:81)),set(ipv4(src=10.10.10.2,dst=192.168.5.5)),3,set(eth(src=50:54:00:00:00:09)),set(ipv4(src=10.10.10.2,dst=10.10.10.1)),4 +Datapath actions: set(ipv4(dst=192.168.4.4)),2,set(eth(src=80:81:81:81:81:81)),set(ipv4(dst=192.168.5.5)),3,set(eth(src=50:54:00:00:00:09)),set(ipv4(dst=10.10.10.1)),4 ]) AT_CHECK([ovs-appctl dpif/set-dp-features br0 sample_nesting 2], [0], [ignore]) AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout]) AT_CHECK([tail -1 stdout], [0], [dnl -Datapath actions: set(ipv4(src=10.10.10.2,dst=192.168.4.4)),2,set(eth(src=80:81:81:81:81:81)),set(ipv4(src=10.10.10.2,dst=192.168.5.5)),3,set(eth(src=50:54:00:00:00:09)),set(ipv4(src=10.10.10.2,dst=10.10.10.1)),4 +Datapath actions: set(ipv4(dst=192.168.4.4)),2,set(eth(src=80:81:81:81:81:81)),set(ipv4(dst=192.168.5.5)),3,set(eth(src=50:54:00:00:00:09)),set(ipv4(dst=10.10.10.1)),4 ]) dnl Mixing reversible and irreversible open flow clone actions. Datapath clone action @@ -7286,7 +7286,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt], [0], [ignore]) AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout]) AT_CHECK([tail -1 stdout], [0], [dnl -Datapath actions: set(ipv4(src=10.10.10.2,dst=192.168.4.4)),2,set(ipv4(src=10.10.10.2,dst=10.10.10.1)),clone(ct(commit),3),4 +Datapath actions: set(ipv4(dst=192.168.4.4)),2,set(ipv4(dst=10.10.10.1)),clone(ct(commit),3),4 ]) AT_CHECK([ovs-appctl dpif/set-dp-features br0 clone false], [0], [ignore]) @@ -7294,14 +7294,14 @@ AT_CHECK([ovs-appctl dpif/set-dp-features br0 clone false], [0], [ignore]) AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout]) AT_CHECK([tail -1 stdout], [0], [dnl -Datapath actions: set(ipv4(src=10.10.10.2,dst=192.168.4.4)),2,set(ipv4(src=10.10.10.2,dst=10.10.10.1)),sample(sample=100.0%,actions(ct(commit),3)),4 +Datapath actions: set(ipv4(dst=192.168.4.4)),2,set(ipv4(dst=10.10.10.1)),sample(sample=100.0%,actions(ct(commit),3)),4 ]) AT_CHECK([ovs-appctl dpif/set-dp-features br0 sample_nesting 2], [0], [ignore]) AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout]) AT_CHECK([tail -1 stdout], [0], [dnl -Datapath actions: set(ipv4(src=10.10.10.2,dst=192.168.4.4)),2,set(ipv4(src=10.10.10.2,dst=10.10.10.1)),4 +Datapath actions: set(ipv4(dst=192.168.4.4)),2,set(ipv4(dst=10.10.10.1)),4 ]) AT_CHECK([grep "Failed to compose clone action" stdout], [0], [ignore])