[ovs-dev] [export netlink fix v2] datapath: fix bugs in exporting netlink attributes

[Jesse Gross]

On Tue, Jul 9, 2013 at 12:25 AM, Andy Zhou <azhou at 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.