I finally applied this (I had forgotten about it until I found it in
patchwork just now).
On Fri, Jan 17, 2014 at 06:01:54PM -0800, Alex Wang wrote:
> Looks good to me,
>
>
> On Tue, Dec 31, 2013 at 11:39 AM, Ben Pfaff <b...@nicira.com> 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 <b...@nicira.com>
> > ---
> > 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
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> >
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
