Thanks. An incremental patch is attached that should fix both issues raised.
On Tue, Jul 9, 2013 at 1:59 PM, Jesse Gross <je...@nicira.com> wrote:
> On Tue, Jul 9, 2013 at 12:25 AM, Andy Zhou <az...@nicira.com> wrote:
> > diff --git a/datapath/flow.c b/datapath/flow.c
> > index adb918f..a756a15 100644
> > --- a/datapath/flow.c
> > +++ b/datapath/flow.c
> > @@ -1686,16 +1688,15 @@ int ovs_flow_to_nlattrs(const struct sw_flow_key
> *swkey,
> [...]
> > - if (swkey->phy.in_port != DP_MAX_PORTS) {
> > - /* Exact match upper 16 bits. */
> > + {
> > u16 upper_u16;
> > upper_u16 = (swkey == output) ? 0 : 0xffff;
> >
>
> DP_MAX_PORTS is an internal kernel value that has no meaning to
> userspace so we shouldn't leak it. I think we need to do something
> similar to 802.2 where the lack of a attribute implies a particular
> value.
>
> I also see another problem here: on flow translation from userspace we
> don't set DP_MAX_PORTS when there is no attribute. Instead, it just
> remains as zero, which is the internal port.
>
> > @@ -1728,12 +1729,20 @@ int ovs_flow_to_nlattrs(const struct sw_flow_key
> *swkey,
> > } else
> > encap = NULL;
> >
> > - if ((swkey == output) && (swkey->eth.type == htons(ETH_P_802_2)))
> > + if (swkey->eth.type == htons(ETH_P_802_2)) {
> > + /*
> > + * Ethertype 802.2 is represented in the netlink with
> omitted
> > + * OVS_KEY_ATTR_ETHERTYPE in the flow key attribute, and
> > + * 0xffff in the mask attribute.
> > + */
> > + if (swkey != output)
> > + if (nla_put_be16(skb, OVS_KEY_ATTR_ETHERTYPE,
> htons(0xffff)))
> > + goto nla_put_failure;
> > goto unencap;
> > + }
>
> I'm not sure that this is right - it implies that we must always have
> an exact match on the EtherType for 802.2 packets but I think it
> really is that we must either have exact match or a complete wildcard
> on it. For example, it should be possible to create a flow where the
> original packet is 802.2 but you want to have a flow that just matches
> on the MAC addresses.
>
0001-review-comment.patch
Description: Binary data
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
