| Message ID | 164794169091.1116991.17641650221335898623.stgit@ebuild |
|---|---|
| State | Accepted |
| Commit | d632ad0aa1123af8a5bdf679d7a3fbd50ad7784f |
| Headers | show |
| Series | [ovs-dev,v4] odp_util: Fix parse_key_and_mask_to_match() vlan parsing | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/intel-ovs-compilation | success | test: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On 22 Mar 2022, at 10:35, Eelco Chaudron wrote: > The parse_key_and_mask_to_match() is a function to translate > a netlink formatted key/mask to match structure. And should > not consider any configuration setting when translating. > > In addition we also enforce the encap_eth_type[0] mask as > it's required for the VLAN match. > > Signed-off-by: Eelco Chaudron <echaudro@redhat.com> Ilya, can you take a look at this one, it’s the only remaining patch from the “netdev-offload-tc: Fix various tc-offload related problems” series. Thanks, Eelco > --- > v4: > - Fixed call to odp_flow_key_to_mask__() to ignore vlan limit. > - Force encap_eth_type[0] mask as it was used for VLAN matching. > > lib/netdev-offload-tc.c | 2 +- > lib/odp-util.c | 41 ++++++++++++++++++++++++++++------------- > 2 files changed, 29 insertions(+), 14 deletions(-) > > diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c > index 9845e8d3f..7f0d4bbff 100644 > --- a/lib/netdev-offload-tc.c > +++ b/lib/netdev-offload-tc.c > @@ -1638,7 +1638,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, > > if (mask->vlans[0].tpid && eth_type_vlan(key->vlans[0].tpid)) { > flower.key.encap_eth_type[0] = flower.key.eth_type; > - flower.mask.encap_eth_type[0] = flower.mask.eth_type; > + flower.mask.encap_eth_type[0] = CONSTANT_HTONS(0xffff); > flower.key.eth_type = key->vlans[0].tpid; > flower.mask.eth_type = mask->vlans[0].tpid; > } > diff --git a/lib/odp-util.c b/lib/odp-util.c > index 0ff302856..4389df6a6 100644 > --- a/lib/odp-util.c > +++ b/lib/odp-util.c > @@ -7188,7 +7188,8 @@ parse_8021q_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1], > uint64_t present_attrs, int out_of_range_attr, > uint64_t expected_attrs, struct flow *flow, > const struct nlattr *key, size_t key_len, > - const struct flow *src_flow, char **errorp) > + const struct flow *src_flow, char **errorp, > + bool ignore_vlan_limit) > { > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); > bool is_mask = src_flow != flow; > @@ -7196,9 +7197,11 @@ parse_8021q_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1], > const struct nlattr *encap; > enum odp_key_fitness encap_fitness; > enum odp_key_fitness fitness = ODP_FIT_ERROR; > + int vlan_limit; > int encaps = 0; > > - while (encaps < flow_vlan_limit && > + vlan_limit = ignore_vlan_limit ? FLOW_MAX_VLAN_HEADERS : flow_vlan_limit; > + while (encaps < vlan_limit && > (is_mask > ? (src_flow->vlans[encaps].tci & htons(VLAN_CFI)) != 0 > : eth_type_vlan(flow->dl_type))) { > @@ -7281,7 +7284,7 @@ parse_8021q_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1], > static enum odp_key_fitness > odp_flow_key_to_flow__(const struct nlattr *key, size_t key_len, > struct flow *flow, const struct flow *src_flow, > - char **errorp) > + char **errorp, bool ignore_vlan_limit) > { > /* New "struct flow" fields that are visible to the datapath (including all > * data fields) should be translated from equivalent datapath flow fields > @@ -7431,7 +7434,7 @@ odp_flow_key_to_flow__(const struct nlattr *key, size_t key_len, > : eth_type_vlan(src_flow->dl_type)) { > fitness = parse_8021q_onward(attrs, present_attrs, out_of_range_attr, > expected_attrs, flow, key, key_len, > - src_flow, errorp); > + src_flow, errorp, ignore_vlan_limit); > } else { > if (is_mask) { > /* A missing VLAN mask means exact match on vlan_tci 0 (== no > @@ -7497,7 +7500,7 @@ enum odp_key_fitness > odp_flow_key_to_flow(const struct nlattr *key, size_t key_len, > struct flow *flow, char **errorp) > { > - return odp_flow_key_to_flow__(key, key_len, flow, flow, errorp); > + return odp_flow_key_to_flow__(key, key_len, flow, flow, errorp, false); > } > > /* Converts the 'mask_key_len' bytes of OVS_KEY_ATTR_* attributes in 'mask_key' > @@ -7509,14 +7512,16 @@ odp_flow_key_to_flow(const struct nlattr *key, size_t key_len, > * If 'errorp' is nonnull, this function uses it for detailed error reports: if > * the return value is ODP_FIT_ERROR, it stores a malloc()'d error string in > * '*errorp', otherwise NULL. */ > -enum odp_key_fitness > -odp_flow_key_to_mask(const struct nlattr *mask_key, size_t mask_key_len, > - struct flow_wildcards *mask, const struct flow *src_flow, > - char **errorp) > +static enum odp_key_fitness > +odp_flow_key_to_mask__(const struct nlattr *mask_key, size_t mask_key_len, > + struct flow_wildcards *mask, > + const struct flow *src_flow, > + char **errorp, bool ignore_vlan_limit) > { > if (mask_key_len) { > return odp_flow_key_to_flow__(mask_key, mask_key_len, > - &mask->masks, src_flow, errorp); > + &mask->masks, src_flow, errorp, > + ignore_vlan_limit); > } else { > if (errorp) { > *errorp = NULL; > @@ -7530,6 +7535,15 @@ odp_flow_key_to_mask(const struct nlattr *mask_key, size_t mask_key_len, > } > } > > +enum odp_key_fitness > +odp_flow_key_to_mask(const struct nlattr *mask_key, size_t mask_key_len, > + struct flow_wildcards *mask, > + const struct flow *src_flow, char **errorp) > +{ > + return odp_flow_key_to_mask__(mask_key, mask_key_len, mask, src_flow, > + errorp, false); > +} > + > /* Converts the netlink formated key/mask to match. > * Fails if odp_flow_key_from_key/mask and odp_flow_key_key/mask > * disagree on the acceptable form of flow */ > @@ -7540,7 +7554,8 @@ parse_key_and_mask_to_match(const struct nlattr *key, size_t key_len, > { > enum odp_key_fitness fitness; > > - fitness = odp_flow_key_to_flow(key, key_len, &match->flow, NULL); > + fitness = odp_flow_key_to_flow__(key, key_len, &match->flow, &match->flow, > + NULL, true); > if (fitness) { > /* This should not happen: it indicates that > * odp_flow_key_from_flow() and odp_flow_key_to_flow() disagree on > @@ -7561,8 +7576,8 @@ parse_key_and_mask_to_match(const struct nlattr *key, size_t key_len, > return EINVAL; > } > > - fitness = odp_flow_key_to_mask(mask, mask_len, &match->wc, &match->flow, > - NULL); > + fitness = odp_flow_key_to_mask__(mask, mask_len, &match->wc, &match->flow, > + NULL, true); > if (fitness) { > /* This should not happen: it indicates that > * odp_flow_key_from_mask() and odp_flow_key_to_mask() > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On 2022-03-22 11:35 AM, Eelco Chaudron wrote: > The parse_key_and_mask_to_match() is a function to translate > a netlink formatted key/mask to match structure. And should > not consider any configuration setting when translating. > > In addition we also enforce the encap_eth_type[0] mask as > it's required for the VLAN match. > > Signed-off-by: Eelco Chaudron <echaudro@redhat.com> > > --- > v4: > - Fixed call to odp_flow_key_to_mask__() to ignore vlan limit. > - Force encap_eth_type[0] mask as it was used for VLAN matching. > > lib/netdev-offload-tc.c | 2 +- > lib/odp-util.c | 41 ++++++++++++++++++++++++++++------------- > 2 files changed, 29 insertions(+), 14 deletions(-) > > diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c > index 9845e8d3f..7f0d4bbff 100644 > --- a/lib/netdev-offload-tc.c > +++ b/lib/netdev-offload-tc.c > @@ -1638,7 +1638,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, > > if (mask->vlans[0].tpid && eth_type_vlan(key->vlans[0].tpid)) { > flower.key.encap_eth_type[0] = flower.key.eth_type; > - flower.mask.encap_eth_type[0] = flower.mask.eth_type; > + flower.mask.encap_eth_type[0] = CONSTANT_HTONS(0xffff); > flower.key.eth_type = key->vlans[0].tpid; > flower.mask.eth_type = mask->vlans[0].tpid; > } > diff --git a/lib/odp-util.c b/lib/odp-util.c > index 0ff302856..4389df6a6 100644 > --- a/lib/odp-util.c > +++ b/lib/odp-util.c > @@ -7188,7 +7188,8 @@ parse_8021q_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1], > uint64_t present_attrs, int out_of_range_attr, > uint64_t expected_attrs, struct flow *flow, > const struct nlattr *key, size_t key_len, > - const struct flow *src_flow, char **errorp) > + const struct flow *src_flow, char **errorp, > + bool ignore_vlan_limit) > { > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); > bool is_mask = src_flow != flow; > @@ -7196,9 +7197,11 @@ parse_8021q_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1], > const struct nlattr *encap; > enum odp_key_fitness encap_fitness; > enum odp_key_fitness fitness = ODP_FIT_ERROR; > + int vlan_limit; > int encaps = 0; > > - while (encaps < flow_vlan_limit && > + vlan_limit = ignore_vlan_limit ? FLOW_MAX_VLAN_HEADERS : flow_vlan_limit; > + while (encaps < vlan_limit && > (is_mask > ? (src_flow->vlans[encaps].tci & htons(VLAN_CFI)) != 0 > : eth_type_vlan(flow->dl_type))) { > @@ -7281,7 +7284,7 @@ parse_8021q_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1], > static enum odp_key_fitness > odp_flow_key_to_flow__(const struct nlattr *key, size_t key_len, > struct flow *flow, const struct flow *src_flow, > - char **errorp) > + char **errorp, bool ignore_vlan_limit) > { > /* New "struct flow" fields that are visible to the datapath (including all > * data fields) should be translated from equivalent datapath flow fields > @@ -7431,7 +7434,7 @@ odp_flow_key_to_flow__(const struct nlattr *key, size_t key_len, > : eth_type_vlan(src_flow->dl_type)) { > fitness = parse_8021q_onward(attrs, present_attrs, out_of_range_attr, > expected_attrs, flow, key, key_len, > - src_flow, errorp); > + src_flow, errorp, ignore_vlan_limit); > } else { > if (is_mask) { > /* A missing VLAN mask means exact match on vlan_tci 0 (== no > @@ -7497,7 +7500,7 @@ enum odp_key_fitness > odp_flow_key_to_flow(const struct nlattr *key, size_t key_len, > struct flow *flow, char **errorp) > { > - return odp_flow_key_to_flow__(key, key_len, flow, flow, errorp); > + return odp_flow_key_to_flow__(key, key_len, flow, flow, errorp, false); > } > > /* Converts the 'mask_key_len' bytes of OVS_KEY_ATTR_* attributes in 'mask_key' > @@ -7509,14 +7512,16 @@ odp_flow_key_to_flow(const struct nlattr *key, size_t key_len, > * If 'errorp' is nonnull, this function uses it for detailed error reports: if > * the return value is ODP_FIT_ERROR, it stores a malloc()'d error string in > * '*errorp', otherwise NULL. */ > -enum odp_key_fitness > -odp_flow_key_to_mask(const struct nlattr *mask_key, size_t mask_key_len, > - struct flow_wildcards *mask, const struct flow *src_flow, > - char **errorp) > +static enum odp_key_fitness > +odp_flow_key_to_mask__(const struct nlattr *mask_key, size_t mask_key_len, > + struct flow_wildcards *mask, > + const struct flow *src_flow, > + char **errorp, bool ignore_vlan_limit) > { > if (mask_key_len) { > return odp_flow_key_to_flow__(mask_key, mask_key_len, > - &mask->masks, src_flow, errorp); > + &mask->masks, src_flow, errorp, > + ignore_vlan_limit); > } else { > if (errorp) { > *errorp = NULL; > @@ -7530,6 +7535,15 @@ odp_flow_key_to_mask(const struct nlattr *mask_key, size_t mask_key_len, > } > } > > +enum odp_key_fitness > +odp_flow_key_to_mask(const struct nlattr *mask_key, size_t mask_key_len, > + struct flow_wildcards *mask, > + const struct flow *src_flow, char **errorp) > +{ > + return odp_flow_key_to_mask__(mask_key, mask_key_len, mask, src_flow, > + errorp, false); > +} > + > /* Converts the netlink formated key/mask to match. > * Fails if odp_flow_key_from_key/mask and odp_flow_key_key/mask > * disagree on the acceptable form of flow */ > @@ -7540,7 +7554,8 @@ parse_key_and_mask_to_match(const struct nlattr *key, size_t key_len, > { > enum odp_key_fitness fitness; > > - fitness = odp_flow_key_to_flow(key, key_len, &match->flow, NULL); > + fitness = odp_flow_key_to_flow__(key, key_len, &match->flow, &match->flow, > + NULL, true); > if (fitness) { > /* This should not happen: it indicates that > * odp_flow_key_from_flow() and odp_flow_key_to_flow() disagree on > @@ -7561,8 +7576,8 @@ parse_key_and_mask_to_match(const struct nlattr *key, size_t key_len, > return EINVAL; > } > > - fitness = odp_flow_key_to_mask(mask, mask_len, &match->wc, &match->flow, > - NULL); > + fitness = odp_flow_key_to_mask__(mask, mask_len, &match->wc, &match->flow, > + NULL, true); > if (fitness) { > /* This should not happen: it indicates that > * odp_flow_key_from_mask() and odp_flow_key_to_mask() > seems fine to me. thanks. Acked-by: Roi Dayan <roid@nvidia.com>
On 5/30/22 14:42, Roi Dayan wrote: > > > On 2022-03-22 11:35 AM, Eelco Chaudron wrote: >> The parse_key_and_mask_to_match() is a function to translate >> a netlink formatted key/mask to match structure. And should >> not consider any configuration setting when translating. >> >> In addition we also enforce the encap_eth_type[0] mask as >> it's required for the VLAN match. >> >> Signed-off-by: Eelco Chaudron <echaudro@redhat.com> >> >> --- >> v4: >> - Fixed call to odp_flow_key_to_mask__() to ignore vlan limit. >> - Force encap_eth_type[0] mask as it was used for VLAN matching. >> >> lib/netdev-offload-tc.c | 2 +- >> lib/odp-util.c | 41 ++++++++++++++++++++++++++++------------- >> 2 files changed, 29 insertions(+), 14 deletions(-) > > seems fine to me. thanks. > > Acked-by: Roi Dayan <roid@nvidia.com> Thanks! Applied to master and branch-2.17. Best regards, Ilya Maximets.
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c index 9845e8d3f..7f0d4bbff 100644 --- a/lib/netdev-offload-tc.c +++ b/lib/netdev-offload-tc.c @@ -1638,7 +1638,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match, if (mask->vlans[0].tpid && eth_type_vlan(key->vlans[0].tpid)) { flower.key.encap_eth_type[0] = flower.key.eth_type; - flower.mask.encap_eth_type[0] = flower.mask.eth_type; + flower.mask.encap_eth_type[0] = CONSTANT_HTONS(0xffff); flower.key.eth_type = key->vlans[0].tpid; flower.mask.eth_type = mask->vlans[0].tpid; } diff --git a/lib/odp-util.c b/lib/odp-util.c index 0ff302856..4389df6a6 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -7188,7 +7188,8 @@ parse_8021q_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1], uint64_t present_attrs, int out_of_range_attr, uint64_t expected_attrs, struct flow *flow, const struct nlattr *key, size_t key_len, - const struct flow *src_flow, char **errorp) + const struct flow *src_flow, char **errorp, + bool ignore_vlan_limit) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); bool is_mask = src_flow != flow; @@ -7196,9 +7197,11 @@ parse_8021q_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1], const struct nlattr *encap; enum odp_key_fitness encap_fitness; enum odp_key_fitness fitness = ODP_FIT_ERROR; + int vlan_limit; int encaps = 0; - while (encaps < flow_vlan_limit && + vlan_limit = ignore_vlan_limit ? FLOW_MAX_VLAN_HEADERS : flow_vlan_limit; + while (encaps < vlan_limit && (is_mask ? (src_flow->vlans[encaps].tci & htons(VLAN_CFI)) != 0 : eth_type_vlan(flow->dl_type))) { @@ -7281,7 +7284,7 @@ parse_8021q_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1], static enum odp_key_fitness odp_flow_key_to_flow__(const struct nlattr *key, size_t key_len, struct flow *flow, const struct flow *src_flow, - char **errorp) + char **errorp, bool ignore_vlan_limit) { /* New "struct flow" fields that are visible to the datapath (including all * data fields) should be translated from equivalent datapath flow fields @@ -7431,7 +7434,7 @@ odp_flow_key_to_flow__(const struct nlattr *key, size_t key_len, : eth_type_vlan(src_flow->dl_type)) { fitness = parse_8021q_onward(attrs, present_attrs, out_of_range_attr, expected_attrs, flow, key, key_len, - src_flow, errorp); + src_flow, errorp, ignore_vlan_limit); } else { if (is_mask) { /* A missing VLAN mask means exact match on vlan_tci 0 (== no @@ -7497,7 +7500,7 @@ enum odp_key_fitness odp_flow_key_to_flow(const struct nlattr *key, size_t key_len, struct flow *flow, char **errorp) { - return odp_flow_key_to_flow__(key, key_len, flow, flow, errorp); + return odp_flow_key_to_flow__(key, key_len, flow, flow, errorp, false); } /* Converts the 'mask_key_len' bytes of OVS_KEY_ATTR_* attributes in 'mask_key' @@ -7509,14 +7512,16 @@ odp_flow_key_to_flow(const struct nlattr *key, size_t key_len, * If 'errorp' is nonnull, this function uses it for detailed error reports: if * the return value is ODP_FIT_ERROR, it stores a malloc()'d error string in * '*errorp', otherwise NULL. */ -enum odp_key_fitness -odp_flow_key_to_mask(const struct nlattr *mask_key, size_t mask_key_len, - struct flow_wildcards *mask, const struct flow *src_flow, - char **errorp) +static enum odp_key_fitness +odp_flow_key_to_mask__(const struct nlattr *mask_key, size_t mask_key_len, + struct flow_wildcards *mask, + const struct flow *src_flow, + char **errorp, bool ignore_vlan_limit) { if (mask_key_len) { return odp_flow_key_to_flow__(mask_key, mask_key_len, - &mask->masks, src_flow, errorp); + &mask->masks, src_flow, errorp, + ignore_vlan_limit); } else { if (errorp) { *errorp = NULL; @@ -7530,6 +7535,15 @@ odp_flow_key_to_mask(const struct nlattr *mask_key, size_t mask_key_len, } } +enum odp_key_fitness +odp_flow_key_to_mask(const struct nlattr *mask_key, size_t mask_key_len, + struct flow_wildcards *mask, + const struct flow *src_flow, char **errorp) +{ + return odp_flow_key_to_mask__(mask_key, mask_key_len, mask, src_flow, + errorp, false); +} + /* Converts the netlink formated key/mask to match. * Fails if odp_flow_key_from_key/mask and odp_flow_key_key/mask * disagree on the acceptable form of flow */ @@ -7540,7 +7554,8 @@ parse_key_and_mask_to_match(const struct nlattr *key, size_t key_len, { enum odp_key_fitness fitness; - fitness = odp_flow_key_to_flow(key, key_len, &match->flow, NULL); + fitness = odp_flow_key_to_flow__(key, key_len, &match->flow, &match->flow, + NULL, true); if (fitness) { /* This should not happen: it indicates that * odp_flow_key_from_flow() and odp_flow_key_to_flow() disagree on @@ -7561,8 +7576,8 @@ parse_key_and_mask_to_match(const struct nlattr *key, size_t key_len, return EINVAL; } - fitness = odp_flow_key_to_mask(mask, mask_len, &match->wc, &match->flow, - NULL); + fitness = odp_flow_key_to_mask__(mask, mask_len, &match->wc, &match->flow, + NULL, true); if (fitness) { /* This should not happen: it indicates that * odp_flow_key_from_mask() and odp_flow_key_to_mask()
The parse_key_and_mask_to_match() is a function to translate a netlink formatted key/mask to match structure. And should not consider any configuration setting when translating. In addition we also enforce the encap_eth_type[0] mask as it's required for the VLAN match. Signed-off-by: Eelco Chaudron <echaudro@redhat.com> --- v4: - Fixed call to odp_flow_key_to_mask__() to ignore vlan limit. - Force encap_eth_type[0] mask as it was used for VLAN matching. lib/netdev-offload-tc.c | 2 +- lib/odp-util.c | 41 ++++++++++++++++++++++++++++------------- 2 files changed, 29 insertions(+), 14 deletions(-)