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
