[ovs-dev] [PATCH] dpif-netilnk: convert packet_type netlink attribute

Eric Garver e at erig.me
Mon Jul 3 18:51:49 UTC 2017


On Mon, Jul 03, 2017 at 02:27:18PM +0000, Zoltán Balogh wrote:
> By introducing packet type-aware pipeline, match on ethertype was
> removed when packet type is not Ethernet. As pointed out by Eric Garver,
> this could have a side effect on the kernel datapath:
> https://patchwork.ozlabs.org/patch/782991/
> 
> This patch does approach the problem from a different perspective.
> Instead of re-introducing the unconditional match on Ethernet type in all
> megaflow entries, as suggested by Eric, mapping of packet_type != PT_ETH to
> dl_type could be handled in the put_exclude_packet_type() in dpif-netlink.c.
> 
> Put_exclude_packet_type() filters the packet_type netlink attribute out,
> before all attributes are sent from userspace to kernel. This commit modifies
> the put_exclude_packet_type() function to do a proper conversation and add the
> missing OVS_KEY_ATTR_ETHERTYPE attribute if it's needed.
> 
> Signed-off-by: Zoltán Balogh <zoltan.balogh at ericsson.com>
> Reported-by: Eric Garver <e at erig.me>
> ---
>  lib/dpif-netlink.c | 34 ++++++++++++++++++++++++++++------
>  1 file changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 562f6134c..d8051a09e 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -3434,11 +3434,11 @@ 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', converts it to ETHERNET and
> + * ETHERTYPE attributes, then puts 'data' to 'buf'.
>   */
>  static void
> -put_exclude_packet_type(struct ofpbuf *buf, uint16_t type,
> +put_convert_packet_type(struct ofpbuf *buf, uint16_t type,
>                          const struct nlattr *data, uint16_t data_len)
>  {
>      const struct nlattr *packet_type;
> @@ -3446,8 +3446,9 @@ put_exclude_packet_type(struct ofpbuf *buf, uint16_t type,
>      packet_type = nl_attr_find__(data, data_len, OVS_KEY_ATTR_PACKET_TYPE);
>  
>      if (packet_type) {
> -        /* exclude PACKET_TYPE Netlink attribute. */
> +        /* Convert PACKET_TYPE Netlink attribute. */
>          ovs_assert(NLA_ALIGN(packet_type->nla_len) == NL_A_U32_SIZE);
> +        ovs_be32 value = nl_attr_get_be32(packet_type);
>          size_t packet_type_len = NL_A_U32_SIZE;
>          size_t first_chunk_size = (uint8_t *)packet_type - (uint8_t *)data;
>          size_t second_chunk_size = data_len - first_chunk_size
> @@ -3455,10 +3456,31 @@ put_exclude_packet_type(struct ofpbuf *buf, uint16_t type,
>          uint8_t *first_attr = NULL;
>          struct nlattr *next_attr = nl_attr_next(packet_type);
>  
> +        bool ethernet_present = nl_attr_find__(data, data_len,
> +                                               OVS_KEY_ATTR_ETHERNET);
> +
>          first_attr = nl_msg_put_unspec_uninit(buf, type,
>                                                data_len - packet_type_len);
>          memcpy(first_attr, data, first_chunk_size);
>          memcpy(first_attr + first_chunk_size, next_attr, second_chunk_size);
> +
> +        /* Put OVS_KEY_ATTR_ETHERTYPE if needed. */
> +        if (ntohl(value) == PT_ETH) {

I don't think this works for the mask case. I think value will be
OVS_BE32_MAX.

> +            ovs_assert(ethernet_present);
> +        } else {
> +            const struct nlattr *eth_type;
> +            ovs_be16 ns_type = pt_ns_type_be(value);
> +
> +            ovs_assert(ethernet_present == false);
> +
> +            eth_type = nl_attr_find__(data, data_len, OVS_KEY_ATTR_ETHERTYPE);
> +            if (eth_type) {
> +                ovs_assert(nl_attr_get_be16(eth_type) == ns_type);
> +            } else {
> +                nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, ns_type);

This will have to put OVS_BE16_MAX for mask case.

> +            }
> +        }
> +
>      } else {
>          nl_msg_put_unspec(buf, type, data, data_len);
>      }
> @@ -3489,11 +3511,11 @@ dpif_netlink_flow_to_ofpbuf(const struct dpif_netlink_flow *flow,
>      }
>      if (!flow->ufid_terse || !flow->ufid_present) {
>          if (flow->key_len) {
> -            put_exclude_packet_type(buf, OVS_FLOW_ATTR_KEY, flow->key,
> +            put_convert_packet_type(buf, OVS_FLOW_ATTR_KEY, flow->key,
>                                             flow->key_len);
>          }
>          if (flow->mask_len) {
> -            put_exclude_packet_type(buf, OVS_FLOW_ATTR_MASK, flow->mask,
> +            put_convert_packet_type(buf, OVS_FLOW_ATTR_MASK, flow->mask,
>                                             flow->mask_len);
>          }
>          if (flow->actions || flow->actions_len) {
> -- 
> 2.11.0
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list