diff mbox series

[ovs-dev,V2,1/2] ofproto-dpif-xlate: Change priority tagsfrom boolean to enum

Message ID 20190512055100.19524-2-elibr@mellanox.com
State Accepted
Headers show
Series Add include mode to priority tags portoption | expand

Commit Message

Eli Britstein May 12, 2019, 5:50 a.m. UTC
Priority tags is a port configuration to determine how the port treats
priority tags, e.g. zero VLAN ID. Change the type from boolean to enum
as a pre-step towards introducing additional modes. The new options are
"omit", equivalent to previously "false", and "include-non-zero",
equivalent to previously "true". "true" is still supported for backwards
compatibility.

Signed-off-by: Eli Britstein <elibr@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
---
 ofproto/ofproto-dpif-xlate.c |  9 +++++----
 ofproto/ofproto-dpif-xlate.h |  2 +-
 ofproto/ofproto-dpif.c       |  3 ++-
 ofproto/ofproto.h            | 10 +++++++++-
 tests/ofproto-dpif.at        |  6 +++---
 vswitchd/bridge.c            |  8 ++++++--
 vswitchd/vswitch.xml         |  4 ++--
 7 files changed, 28 insertions(+), 14 deletions(-)

Comments

0-day Robot May 12, 2019, 5:58 a.m. UTC | #1
Bleep bloop.  Greetings Eli Britstein, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 87 characters long (recommended limit is 79)
#175 FILE: vswitchd/vswitch.xml:1863:
              type='{"type": "string", "enum": ["set", ["omit", "include-non-zero"]]}'>

WARNING: Line is 83 characters long (recommended limit is 79)
#184 FILE: vswitchd/vswitch.xml:1876:
          <code>include-non-zero</code> to enable priority-tagged frames on a port.

Lines checked: 190, Warnings: 2, Errors: 0


Please check this out.  If you feel there has been an error, please email aconole@bytheb.org

Thanks,
0-day Robot
diff mbox series

Patch

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 5cee37f7b..4c1cdfa58 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -149,7 +149,8 @@  struct xbundle {
                                     * NULL if all VLANs are trunked. */
     unsigned long *cvlans;         /* Bitmap of allowed customer vlans,
                                     * NULL if all VLANs are allowed */
-    bool use_priority_tags;        /* Use 802.1p tag for frames in VLAN 0? */
+    enum port_priority_tags_mode use_priority_tags;
+                                   /* Use 802.1p tag for frames in VLAN 0? */
     bool floodable;                /* No port has OFPUTIL_PC_NO_FLOOD set? */
     bool protected;                /* Protected port mode */
 };
@@ -604,7 +605,7 @@  static void xlate_xbundle_set(struct xbundle *xbundle,
                               enum port_vlan_mode vlan_mode,
                               uint16_t qinq_ethtype, int vlan,
                               unsigned long *trunks, unsigned long *cvlans,
-                              bool use_priority_tags,
+                              enum port_priority_tags_mode use_priority_tags,
                               const struct bond *bond, const struct lacp *lacp,
                               bool floodable, bool protected);
 static void xlate_xport_set(struct xport *xport, odp_port_t odp_port,
@@ -999,7 +1000,7 @@  static void
 xlate_xbundle_set(struct xbundle *xbundle,
                   enum port_vlan_mode vlan_mode, uint16_t qinq_ethtype,
                   int vlan, unsigned long *trunks, unsigned long *cvlans,
-                  bool use_priority_tags,
+                  enum port_priority_tags_mode use_priority_tags,
                   const struct bond *bond, const struct lacp *lacp,
                   bool floodable, bool protected)
 {
@@ -1316,7 +1317,7 @@  xlate_bundle_set(struct ofproto_dpif *ofproto, struct ofbundle *ofbundle,
                  const char *name, enum port_vlan_mode vlan_mode,
                  uint16_t qinq_ethtype, int vlan,
                  unsigned long *trunks, unsigned long *cvlans,
-                 bool use_priority_tags,
+                 enum port_priority_tags_mode use_priority_tags,
                  const struct bond *bond, const struct lacp *lacp,
                  bool floodable, bool protected)
 {
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index 5a51d7b30..c947bd801 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -181,7 +181,7 @@  void xlate_bundle_set(struct ofproto_dpif *, struct ofbundle *,
                       const char *name, enum port_vlan_mode,
                       uint16_t qinq_ethtype, int vlan,
                       unsigned long *trunks, unsigned long *cvlans,
-                      bool use_priority_tags,
+                      enum port_priority_tags_mode use_priority_tags,
                       const struct bond *, const struct lacp *,
                       bool floodable, bool protected);
 void xlate_bundle_remove(struct ofbundle *);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index db461ac07..3db97ca37 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -101,7 +101,8 @@  struct ofbundle {
     unsigned long *cvlans;
     struct lacp *lacp;          /* LACP if LACP is enabled, otherwise NULL. */
     struct bond *bond;          /* Nonnull iff more than one port. */
-    bool use_priority_tags;     /* Use 802.1p tag for frames in VLAN 0? */
+    enum port_priority_tags_mode use_priority_tags;
+                                /* Use 802.1p tag for frames in VLAN 0? */
 
     bool protected;             /* Protected port mode */
 
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 510ace103..caf5e50b5 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -416,6 +416,13 @@  enum port_vlan_mode {
     PORT_VLAN_DOT1Q_TUNNEL
 };
 
+/* The behaviour of the port regarding priority tags */
+enum port_priority_tags_mode {
+    PORT_PRIORITY_TAGS_OMIT = 0,
+    PORT_PRIORITY_TAGS_INCLUDE_NON_ZERO,
+};
+
+/* The behaviour of the port regarding priority tags */
 /* Configuration of bundles. */
 struct ofproto_bundle_settings {
     char *name;                 /* For use in log messages. */
@@ -428,7 +435,8 @@  struct ofproto_bundle_settings {
     int vlan;                   /* VLAN VID, except for PORT_VLAN_TRUNK. */
     unsigned long *trunks;      /* vlan_bitmap, except for PORT_VLAN_ACCESS. */
     unsigned long *cvlans;
-    bool use_priority_tags;     /* Use 802.1p tag for frames in VLAN 0? */
+    enum port_priority_tags_mode use_priority_tags;
+                                /* Use 802.1p tag for frames in VLAN 0? */
 
     struct bond_settings *bond; /* Must be nonnull iff if n_slaves > 1. */
 
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 9b0231b88..73242b1f6 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -3467,18 +3467,18 @@  OVS_VSWITCHD_START(
    add-port br0 p1                                  trunks=10,12 -- \
    add-port br0 p2                           tag=10              -- \
    add-port br0 p3                           tag=12              \
-                   other-config:priority-tags=true               -- \
+                   other-config:priority-tags=include-non-zero   -- \
    add-port br0 p4                           tag=12              -- \
    add-port br0 p5 vlan_mode=native-tagged   tag=10              -- \
    add-port br0 p6 vlan_mode=native-tagged   tag=10 trunks=10,12 -- \
    add-port br0 p7 vlan_mode=native-untagged tag=12              -- \
    add-port br0 p8 vlan_mode=native-untagged tag=12 trunks=10,12 \
-                   other-config:priority-tags=true               -- \
+                   other-config:priority-tags=include-non-zero   -- \
    add-port br0 p9 vlan_mode=dot1q-tunnel    tag=10              other-config:qinq-ethtype=802.1q  -- \
    add-port br0 p10 vlan_mode=dot1q-tunnel   tag=10 cvlans=10,12 other-config:qinq-ethtype=802.1q  -- \
    add-port br0 p11 vlan_mode=dot1q-tunnel   tag=12              other-config:qinq-ethtype=802.1q  -- \
    add-port br0 p12 vlan_mode=dot1q-tunnel   tag=12              other-config:qinq-ethtype=802.1q  \
-                   other-config:priority-tags=true               -- \
+                   other-config:priority-tags=include-non-zero   -- \
    set Interface p1 type=dummy -- \
    set Interface p2 type=dummy -- \
    set Interface p3 type=dummy -- \
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 48d8c4de1..06c8bf4ca 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1023,8 +1023,12 @@  port_configure(struct port *port)
                       ? ETH_TYPE_VLAN_8021Q
                       : ETH_TYPE_VLAN_8021AD);
 
-    s.use_priority_tags = smap_get_bool(&cfg->other_config, "priority-tags",
-                                        false);
+    const char *pt = smap_get_def(&cfg->other_config, "priority-tags", "");
+    if (!strcmp(pt, "include-non-zero") || !strcmp(pt, "true")) {
+        s.use_priority_tags = PORT_PRIORITY_TAGS_INCLUDE_NON_ZERO;
+    } else {
+        s.use_priority_tags = PORT_PRIORITY_TAGS_OMIT;
+    }
 
     /* Get LACP settings. */
     s.lacp = port_configure_lacp(port, &lacp_settings);
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 08001dbce..d5f2b0186 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -1860,7 +1860,7 @@ 
       </column>
 
       <column name="other_config" key="priority-tags"
-              type='{"type": "boolean"}'>
+              type='{"type": "string", "enum": ["set", ["omit", "include-non-zero"]]}'>
         <p>
           An 802.1Q header contains two important pieces of information: a VLAN
           ID and a priority.  A frame with a zero VLAN ID, called a
@@ -1873,7 +1873,7 @@ 
           header at all, even when the VLAN ID is zero.  Therefore, by default
           Open vSwitch does not output priority-tagged frames, instead omitting
           the 802.1Q header entirely if the VLAN ID is zero.  Set this key to
-          <code>true</code> to enable priority-tagged frames on a port.
+          <code>include-non-zero</code> to enable priority-tagged frames on a port.
         </p>
 
         <p>