[ovs-dev] [RFC] vlan: Make sure vlan tci mask has exact match for VLAN_CFI.

Ben Pfaff blp at nicira.com
Sat Jun 6 05:43:24 UTC 2015


On Fri, Jun 05, 2015 at 10:36:55PM -0700, Alex Wang wrote:
> On Fri, Jun 5, 2015 at 10:00 PM, Ben Pfaff <blp at nicira.com> wrote:
> 
> > This will still change the user's intent (and change the flows that the
> > user sets up), breaking some of the forms described in meta-flow.h.
> >
> > This is closer to what I had in mind:
> >
> > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> > index 71b8bef..41a03ef 100644
> > --- a/ofproto/ofproto-dpif-xlate.c
> > +++ b/ofproto/ofproto-dpif-xlate.c
> > @@ -5033,6 +5033,12 @@ xlate_actions(struct xlate_in *xin, struct
> > xlate_out *xout)
> >              wc->masks.tp_src &= htons(UINT8_MAX);
> >              wc->masks.tp_dst &= htons(UINT8_MAX);
> >          }
> > +
> > +        /* When there is a VLAN, the VLAN match must always include match
> > on
> > +         * "present" bit. */
> > +        if (flow->vlan_tci && wc->masks.vlan_tci) {
> > +            wc->masks.vlan_tci |= htons(VLAN_CFI);
> > +        }
> >      }
> >  }
> >
> 
> 
> I see what you mean,~
> 
> 
> 
> >
> > However, this still doesn't fix every case, because of the cases like
> > the vlan_tci=0x000a/0xfff you mention, where "unwildcarding" the CFI bit
> > still results in an exact-match on a 0-bit, which the kernel will
> > reject.  The most obvious place to fix that up is in the translation to
> > the kernel flow key.  I think that we could do that in
> > odp_flow_key_from_flow__().  Do you think that would work out?
> >
> >
> 
> I think kernel will always set the CFI in 'odp flow key' when the received
> pkt contains vlan header.  So my understanding is that the "unwildcarding
> the CFI bit still results in an exact-match on a 0-bit" issue should not
> happen.
> 
> datapath call sequence:
> ovs_vport_receive()->ovs_flow_key_extract()->parse_vlan()->
> key->eth.tci = qp->tci | htons(VLAN_TAG_PRESENT);
> 
> 
> I think odp_flow_key_from_flow__() is the right place to fix it, so how
> about
> this?
> 
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index 3204d16..e17712c 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -3486,10 +3486,12 @@ odp_flow_key_from_flow__(struct ofpbuf *buf, const
> struct flow *flow,
>      if (flow->vlan_tci != htons(0) || flow->dl_type ==
> htons(ETH_TYPE_VLAN)) {
>          if (export_mask) {
>              nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, OVS_BE16_MAX);
> +            nl_msg_put_be16(buf, OVS_KEY_ATTR_VLAN,
> +                            data->vlan_tci | htons(VLAN_CFI));
>          } else {
>              nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE,
> htons(ETH_TYPE_VLAN));
> +            nl_msg_put_be16(buf, OVS_KEY_ATTR_VLAN, data->vlan_tci);
>          }
> -        nl_msg_put_be16(buf, OVS_KEY_ATTR_VLAN, data->vlan_tci);
>          encap = nl_msg_start_nested(buf, OVS_KEY_ATTR_ENCAP);
>          if (flow->vlan_tci == htons(0)) {
>              goto unencap;

I think you're right, want to post a full patch?



More information about the dev mailing list