[ovs-dev] [PATCH 2/2] dpif-netlink: For non-Ethernet, use Ethertype from packet_type.

[Eric Garver]

On Tue, Jul 18, 2017 at 03:32:44PM -0700, Joe Stringer wrote:
> For non-Ethernet flows, when fixing up the netlink message we need make
> sure to pass down a valid Ethertype. The kernel does not understand
> packet_type so it's implicitly encoded by the absence of _ETHERNET and
> exact match of _ETHERTYPE. Without this change match_validate in the
> kernel complains when trying to match packets from L3 tunnels.
> e.g.
>   openvswitch: netlink: Unexpected mask (mask=110088, allowed=3d9804c)
> 
> The mask use to always be set in xlate_wc_init() and xlate_wc_finish(),
> but that changed for non-Ethernet frames with the commit listed below.
> 
> Fixes: 3d4b2e6eb74e ("userspace: Add OXM field MFF_PACKET_TYPE")
> Signed-off-by: Joe Stringer <joe at ovn.org>
> Co-authored-by: Eric Garver <e at erig.me>
> ---
>  lib/dpif-netlink.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 63ef3de5c7d1..55effd1f91a8 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -3434,8 +3434,9 @@ dpif_netlink_flow_from_ofpbuf(struct dpif_netlink_flow *flow,
>  
>  
>  /*
> - * If PACKET_TYPE attribute is present in 'data', it filters PACKET_TYPE out,
> - * then puts 'data' to 'buf'.
> + * If PACKET_TYPE attribute is present in 'data', it filters PACKET_TYPE out.
> + * If the flow is not Ethernet, the OVS_KEY_ATTR_PACKET_TYPE is converted to
> + * OVS_KEY_ATTR_ETHERTYPE. Puts 'data' to 'buf'.
>   */
>  static void
>  put_exclude_packet_type(struct ofpbuf *buf, uint16_t type,
> @@ -3458,6 +3459,20 @@ put_exclude_packet_type(struct ofpbuf *buf, uint16_t type,
>          ofs = nl_msg_start_nested(buf, type);
>          nl_msg_put(buf, data, first_chunk_size);
>          nl_msg_put(buf, next_attr, second_chunk_size);
> +        if (!nl_attr_find__(data, data_len, OVS_KEY_ATTR_ETHERNET)) {
> +            ovs_be16 pt = pt_ns_type_be(nl_attr_get_be32(packet_type));
> +            const struct nlattr *nla;
> +
> +            nla = nl_attr_find(buf, NLA_HDRLEN, OVS_KEY_ATTR_ETHERTYPE);
> +            if (nla) {
> +                ovs_be16 *ethertype;
> +
> +                ethertype = CONST_CAST(ovs_be16 *, nl_attr_get(nla));
> +                *ethertype = pt;
> +            } else {
> +                nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, pt);
> +            }
> +        }
>          nl_msg_end_nested(buf, ofs);
>      } else {
>          nl_msg_put_unspec(buf, type, data, data_len);
> -- 
> 2.11.1
> 

Thanks Joe! This is definitely more concise than my version.

Acked-by: Eric Garver <e at erig.me>