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 <amarpadmanab...@fb.com>
Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-discuss/2017-December/045817.html
Reported-by: Su Wang <suw...@vmware.com>
VMWare-BZ: #2070488
Signed-off-by: Ben Pfaff <b...@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
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to