Looks good to me,
On Tue, Dec 31, 2013 at 11:39 AM, Ben Pfaff <[email protected]> wrote: > Anytime there is a VLAN the flow needs to properly reflect that. Keeping > the TPID in dl_type never makes sense and will probably cause problems. > The existing code did the right thing in the common case but not in corner > cases where it returned ODP_FIT_TOO_MUCH or ODP_FIT_TOO_LITTLE (the cases > where it returned an error don't matter since nothing looks at the flow > in that case). > > Found by inspection. > > Signed-off-by: Ben Pfaff <[email protected]> > --- > lib/odp-util.c | 48 ++++++++++++++++++++++-------------------------- > 1 file changed, 22 insertions(+), 26 deletions(-) > > diff --git a/lib/odp-util.c b/lib/odp-util.c > index b10f72e..515b531 100644 > --- a/lib/odp-util.c > +++ b/lib/odp-util.c > @@ -3083,7 +3083,6 @@ parse_8021q_onward(const struct nlattr > *attrs[OVS_KEY_ATTR_MAX + 1], > ? attrs[OVS_KEY_ATTR_ENCAP] : NULL); > enum odp_key_fitness encap_fitness; > enum odp_key_fitness fitness; > - ovs_be16 tci; > > /* Calculate fitness of outer attributes. */ > if (!is_mask) { > @@ -3100,35 +3099,32 @@ parse_8021q_onward(const struct nlattr > *attrs[OVS_KEY_ATTR_MAX + 1], > fitness = check_expectations(present_attrs, out_of_range_attr, > expected_attrs, key, key_len); > > - /* Get the VLAN TCI value. */ > - if (!is_mask && !(present_attrs & (UINT64_C(1) << > OVS_KEY_ATTR_VLAN))) { > - return ODP_FIT_TOO_LITTLE; > + /* Set vlan_tci. > + * Remove the TPID from dl_type since it's not the real Ethertype. */ > + flow->dl_type = htons(0); > + flow->vlan_tci = (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_VLAN) > + ? nl_attr_get_be16(attrs[OVS_KEY_ATTR_VLAN]) > + : htons(0)); > + if (!is_mask) { > + if (!(present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_VLAN))) { > + return ODP_FIT_TOO_LITTLE; > + } else if (flow->vlan_tci == htons(0)) { > + /* Corner case for a truncated 802.1Q header. */ > + if (fitness == ODP_FIT_PERFECT && nl_attr_get_size(encap)) { > + return ODP_FIT_TOO_MUCH; > + } > + return fitness; > + } else if (!(flow->vlan_tci & htons(VLAN_CFI))) { > + VLOG_ERR_RL(&rl, "OVS_KEY_ATTR_VLAN 0x%04"PRIx16" is nonzero " > + "but CFI bit is not set", ntohs(flow->vlan_tci)); > + return ODP_FIT_ERROR; > + } > } else { > - tci = (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_VLAN) > - ? nl_attr_get_be16(attrs[OVS_KEY_ATTR_VLAN]) > - : htons(0)); > - if (!is_mask) { > - if (tci == htons(0)) { > - /* Corner case for a truncated 802.1Q header. */ > - if (fitness == ODP_FIT_PERFECT && > nl_attr_get_size(encap)) { > - return ODP_FIT_TOO_MUCH; > - } > - return fitness; > - } else if (!(tci & htons(VLAN_CFI))) { > - VLOG_ERR_RL(&rl, "OVS_KEY_ATTR_VLAN 0x%04"PRIx16" is > nonzero " > - "but CFI bit is not set", ntohs(tci)); > - return ODP_FIT_ERROR; > - } > + if (!(present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_ENCAP))) { > + return fitness; > } > - /* Set vlan_tci. > - * Remove the TPID from dl_type since it's not the real > Ethertype. */ > - flow->dl_type = htons(0); > - flow->vlan_tci = tci; > } > > - if (is_mask && !(present_attrs & (UINT64_C(1) << > OVS_KEY_ATTR_ENCAP))) { > - return fitness; > - } > /* Now parse the encapsulated attributes. */ > if (!parse_flow_nlattrs(nl_attr_get(encap), nl_attr_get_size(encap), > attrs, &present_attrs, &out_of_range_attr)) { > -- > 1.7.10.4 > > _______________________________________________ > dev mailing list > [email protected] > http://openvswitch.org/mailman/listinfo/dev >
_______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
