[ovs-dev] [PATCH 03/31] fixup: Make packet_type formatting more like other fields.

Ben Pfaff blp at ovn.org
Mon Jun 12 22:28:28 UTC 2017


I don't see value in making packet_type formatting different from other
fields.

Also, format_packet_type_masked() added a trailing comma to its output,
which is surprising, so this patch removes it.

Signed-off-by: Ben Pfaff <blp at ovn.org>
---
 lib/flow.c     | 42 ++++++++++++++++++++-----------
 lib/match.c    |  1 +
 lib/odp-util.c | 78 ++++++++++++++--------------------------------------------
 3 files changed, 48 insertions(+), 73 deletions(-)

diff --git a/lib/flow.c b/lib/flow.c
index b901a077d790..218ba6fa53c7 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -1153,21 +1153,35 @@ format_flags_masked(struct ds *ds, const char *name,
     }
 }
 
+static void
+put_u16_masked(struct ds *s, uint16_t value, uint16_t mask)
+{
+    if (!mask) {
+        ds_put_char(s, '*');
+    } else {
+        if (value > 9) {
+            ds_put_format(s, "0x%"PRIx16, value);
+        } else {
+            ds_put_format(s, "%"PRIu16, value);
+        }
+
+        if (mask != UINT16_MAX) {
+            ds_put_format(s, "/0x%"PRIx16, mask);
+        }
+    }
+}
+
 void
-format_packet_type_masked(struct ds *s, const ovs_be32 value, const ovs_be32 mask)
-{
-    if (pt_ns_type_be(mask) == 0) {
-        ds_put_format(s, "packet_type=(%u,*),",
-                      pt_ns(value));
-    } else if (pt_ns_type_be(mask) == OVS_BE16_MAX) {
-        ds_put_format(s, "packet_type=(%u,%#"PRIx16"),",
-                      pt_ns(value),
-                      pt_ns_type(value));
-    } else{
-        ds_put_format(s, "packet_type=(%u,%#"PRIx16"/%#"PRIx16"),",
-                      pt_ns(value),
-                      pt_ns_type(value),
-                      pt_ns_type(mask));
+format_packet_type_masked(struct ds *s, ovs_be32 value, ovs_be32 mask)
+{
+    if (value == htonl(PT_ETH) && mask == OVS_BE32_MAX) {
+        ds_put_cstr(s, "eth");
+    } else {
+        ds_put_cstr(s, "packet_type=(");
+        put_u16_masked(s, pt_ns(value), pt_ns(mask));
+        ds_put_char(s, ',');
+        put_u16_masked(s, pt_ns_type(value), pt_ns_type(mask));
+        ds_put_char(s, ')');
     }
 }
 
diff --git a/lib/match.c b/lib/match.c
index 2caf10064bd2..ec27bc117383 100644
--- a/lib/match.c
+++ b/lib/match.c
@@ -1258,6 +1258,7 @@ match_format(const struct match *match,
 
     if (wc->masks.packet_type) {
         format_packet_type_masked(s, f->packet_type, wc->masks.packet_type);
+        ds_put_char(s, ',');
         if (pt_ns(f->packet_type) == OFPHTN_ETHERTYPE) {
             dl_type = pt_ns_type_be(f->packet_type);
         }
diff --git a/lib/odp-util.c b/lib/odp-util.c
index b57934ba1cd4..798906a337f9 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -2954,23 +2954,18 @@ format_odp_key_attr(const struct nlattr *a, const struct nlattr *ma,
         break;
 
     case OVS_KEY_ATTR_PACKET_TYPE: {
-        ovs_be32 packet_type = nl_attr_get_be32(a);
-        uint16_t ns = pt_ns(packet_type);
-        uint16_t ns_type = pt_ns_type(packet_type);
+        ovs_be32 value = nl_attr_get_be32(a);
+        ovs_be32 mask = ma ? nl_attr_get_be32(ma) : OVS_BE32_MAX;
 
-        if (!is_exact) {
-            ovs_be32 mask = nl_attr_get_be32(ma);
-            uint16_t mask_ns_type = pt_ns_type(mask);
+        ovs_be16 ns = htons(pt_ns(value));
+        ovs_be16 ns_mask = htons(pt_ns(mask));
+        format_be16(ds, "ns", ns, &ns_mask, verbose);
 
-            if (mask == 0) {
-                ds_put_format(ds, "ns=%u,id=*", ns);
-            } else {
-                ds_put_format(ds, "ns=%u,id=%#"PRIx16"/%#"PRIx16,
-                              ns, ns_type, mask_ns_type);
-            }
-        } else {
-            ds_put_format(ds, "ns=%u,id=%#"PRIx16, ns, ns_type);
-        }
+        ovs_be16 ns_type = pt_ns_type_be(value);
+        ovs_be16 ns_type_mask = pt_ns_type_be(mask);
+        format_be16(ds, "id", ns_type, &ns_type_mask, verbose);
+
+        ds_chomp(ds, ',');
         break;
     }
 
@@ -4313,6 +4308,15 @@ parse_odp_key_mask_attr(const char *s, const struct simap *port_names,
         SCAN_FIELD("tll=", eth, nd_tll);
     } SCAN_END(OVS_KEY_ATTR_ND);
 
+    struct packet_type {
+        ovs_be16 ns;
+        ovs_be16 id;
+    };
+    SCAN_BEGIN("packet_type(", struct packet_type) {
+        SCAN_FIELD("ns=", be16, ns);
+        SCAN_FIELD("id=", be16, id);
+    } SCAN_END(OVS_KEY_ATTR_PACKET_TYPE);
+
     /* Encap open-coded. */
     if (!strncmp(s, "encap(", 6)) {
         const char *start = s;
@@ -4350,50 +4354,6 @@ parse_odp_key_mask_attr(const char *s, const struct simap *port_names,
         return s - start;
     }
 
-    /* Packet_type open-coded. */
-    if (strncmp(s, "packet_type(", strlen("packet_type(")) == 0) {
-        const char *start = s;
-        ovs_be32 skey = 0;
-        ovs_be32 smask = 0;
-        int len;
-        uint16_t  ns = 0;
-        uint16_t  ns_type = 0;
-
-        s += strlen("packet_type(");
-        if (ovs_scan(s, "ns=%"SCNu16",id=%n", &ns, &len)){
-            if (len == 0) {
-                return -EINVAL;
-            }
-            s += len;
-            if (strncmp(s, "*", 1) == 0) {
-                s++;
-            } else if (ovs_scan(s, "0x%"SCNx16"%n", &ns_type, &len)) {
-                s += len;
-                skey = PACKET_TYPE_BE(ns, ns_type);
-                if ( *s == '/' ) {
-                    uint16_t  ns_type_mask = 0;
-                    if (ovs_scan(s, "/0x%"SCNx16"%n", &ns_type_mask, &len)) {
-                        s += len;
-                        smask = PACKET_TYPE_BE(ns, ns_type_mask);
-                    }
-                }
-            }
-
-            if (*s++ != ')') {
-                return -EINVAL;
-            }
-
-            nl_msg_put_unspec(key, OVS_KEY_ATTR_PACKET_TYPE, &skey,
-                              sizeof(skey));
-            if (mask) {
-                nl_msg_put_unspec(mask, OVS_KEY_ATTR_PACKET_TYPE, &smask,
-                                  sizeof(smask));
-            }
-        }
-
-        return s - start;
-    }
-
     return -EINVAL;
 }
 
-- 
2.10.2



More information about the dev mailing list