[ovs-dev] [PATCH] odp-util: Print eth() for Ethernet flows if packet_type is absent.

Yi-Hung Wei yihung.wei at gmail.com
Wed Mar 14 22:48:26 UTC 2018


Thanks for the fix. I tested this patch, and it does show eth() when
the OVS_KEY_ATTR_ETHERNET is all-wildcarded.

Acked-by: Yi-Hung Wei <yihung.wei at gmail.com>

On Wed, Mar 14, 2018 at 2:57 PM, Ben Pfaff <blp at ovn.org> wrote:
> OVS datapaths have two different ways to indicate what kind of packet a
> flow matches.  One way, used by the userspace datapath, is
> OVS_KEY_ATTR_PACKET_TYPE.  Another way, used by the kernel datapath, is
> OVS_KEY_ATTR_ETHERTYPE when used in the absence of OVS_KEY_ATTR_ETHERNET;
> when the latter is present, the packet is always an Ethernet packet.  The
> code to print datapath flows wasn't paying attention to this distinction
> and always omitted eth() from the output when OVS_KEY_ATTR_ETHERNET was
> fully wildcarded, which meant that upon later re-parsing the
> OVS_KEY_ATTR_ETHERNET key was omitted, which made it look like a
> non-Ethernet match was being described.
>
> This commit makes odp_util_format() add eth() to the output when
> OVS_KEY_ATTR_ETHERNET is present and OVS_KEY_ATTR_PACKET_TYPE is absent,
> avoiding the problem.
>
> Reported-by: Amar Padmanabhan <amarpadmanabhan at fb.com>
> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2017-December/045817.html
> Reported-by: Su Wang <suwang at vmware.com>
> VMWare-BZ: #2070488
> Signed-off-by: Ben Pfaff <blp at ovn.org>
> ---
>  lib/odp-util.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index 5da83b4c64c4..874350325050 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -4008,6 +4008,7 @@ odp_flow_format(const struct nlattr *key, size_t key_len,
>          const struct nlattr *a;
>          unsigned int left;
>          bool has_ethtype_key = false;
> +        bool has_packet_type_key = false;
>          struct ofpbuf ofp;
>          bool first_field = true;
>
> @@ -4028,6 +4029,8 @@ odp_flow_format(const struct nlattr *key, size_t key_len,
>
>              if (attr_type == OVS_KEY_ATTR_ETHERTYPE) {
>                  has_ethtype_key = true;
> +            } else if (attr_type == OVS_KEY_ATTR_PACKET_TYPE) {
> +                has_packet_type_key = true;
>              }
>
>              is_nested_attr = odp_key_attr_len(ovs_flow_key_attr_lens,
> @@ -4050,6 +4053,34 @@ odp_flow_format(const struct nlattr *key, size_t key_len,
>                  }
>                  format_odp_key_attr__(a, ma, portno_names, ds, verbose);
>                  first_field = false;
> +            } else if (attr_type == OVS_KEY_ATTR_ETHERNET
> +                       && !has_packet_type_key) {
> +                /* This special case reflects differences between the kernel
> +                 * and userspace datapaths regarding the root type of the
> +                 * packet being matched (typically Ethernet but some tunnels
> +                 * can encapsulate IPv4 etc.).  The kernel datapath does not
> +                 * have an explicit way to indicate packet type; instead:
> +                 *
> +                 *   - If OVS_KEY_ATTR_ETHERNET is present, the packet is an
> +                 *     Ethernet packet and OVS_KEY_ATTR_ETHERTYPE is the
> +                 *     Ethertype encoded in the Ethernet header.
> +                 *
> +                 *   - If OVS_KEY_ATTR_ETHERNET is absent, then the packet's
> +                 *     root type is that encoded in OVS_KEY_ATTR_ETHERTYPE
> +                 *     (i.e. if OVS_KEY_ATTR_ETHERTYPE is 0x0800 then the
> +                 *     packet is an IPv4 packet).
> +                 *
> +                 * Thus, if OVS_KEY_ATTR_ETHERNET is present, even if it is
> +                 * all-wildcarded, it is important to print it.
> +                 *
> +                 * On the other hand, the userspace datapath supports
> +                 * OVS_KEY_ATTR_PACKET_TYPE and uses it to indicate the packet
> +                 * type.  Thus, if OVS_KEY_ATTR_PACKET_TYPE is present, we need
> +                 * not print an all-wildcarded OVS_KEY_ATTR_ETHERNET. */
> +                if (!first_field) {
> +                    ds_put_char(ds, ',');
> +                }
> +                ds_put_cstr(ds, "eth()");
>              }
>              ofpbuf_clear(&ofp);
>          }
> --
> 2.16.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list