Yeah, later, with a test, ~
On Fri, Jun 5, 2015 at 10:43 PM, Ben Pfaff <b...@nicira.com> wrote:
> On Fri, Jun 05, 2015 at 10:36:55PM -0700, Alex Wang wrote:
> > On Fri, Jun 5, 2015 at 10:00 PM, Ben Pfaff <b...@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?
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
