[ovs-dev] [PATCH 1/1] vswitchd: PVID (Port VLAN ID) tagging feature

Philippe Jung phil.jung at free.fr
Thu Aug 25 22:02:51 UTC 2011


Le 25/08/2011 01:07, Ben Pfaff a écrit :
> 
> Hi Philippe.  Just dropping you a note to let you know that I haven't
> forgotten your patch.  I spent some time writing a separate commit
> that fixes the bug that you reported and posted it in this thread,
> along with some unit tests for VLANs:
> 	http://openvswitch.org/pipermail/dev/2011-August/010779.html
> 
> I'll get back to your patch soon.

So, you will have the opportunity to check this new version of the patch :-). I have included the optimisation I spoke about. set_bundle with no vlan_mode defined guesses the vlan_mode. All if(mode=access || mode=empty && tag>=0) are replaced by if(mode=access)

Philippe

---

diff --git a/NEWS b/NEWS
index ae6f55e..12b67d6 100644
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,9 @@ Post-v1.2.0
         new NXAST_RESUBMIT_TABLE action can look up in additional
         tables.  Tables 128 and above are reserved for use by the
         switch itself; please use only tables 0 through 127.
+      - Added support for native VLAN tagging.  A new "vlan_mode"
+        parameter can be set for "port". Possible values: "access",
+        "trunk", "native-tagged" and "native-untagged".
 
 v1.2.0 - 03 Aug 2011
 ------------------------
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index f09c230..d6e71b6 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -131,6 +131,7 @@ struct ofbundle {
 
     /* Configuration. */
     struct list ports;          /* Contains "struct ofport"s. */
+    enum port_vlan_mode vlan_mode; /* VLAN mode */
     int vlan;                   /* -1=trunk port, else a 12-bit VLAN ID. */
     unsigned long *trunks;      /* Bitmap of trunked VLANs, if 'vlan' == -1.
                                  * NULL if all VLANs are trunked. */
@@ -970,6 +971,7 @@ bundle_set(struct ofproto *ofproto_, void *aux,
     struct ofport_dpif *port;
     struct ofbundle *bundle;
     size_t i;
+    enum port_vlan_mode vlan_mode;
     bool ok;
 
     if (!s) {
@@ -991,6 +993,7 @@ bundle_set(struct ofproto *ofproto_, void *aux,
         bundle->name = NULL;
 
         list_init(&bundle->ports);
+	bundle->vlan_mode = PORT_VLAN_EMPTY;
         bundle->vlan = -1;
         bundle->trunks = NULL;
         bundle->lacp = NULL;
@@ -1049,6 +1052,24 @@ bundle_set(struct ofproto *ofproto_, void *aux,
         return EINVAL;
     }
 
+    /*
+     * Set VLAN tagging mode
+     * First transform PORT_VLAN_EMPTY into ACCESS or TRUNK to avoid
+     * handling this situation separately
+     */
+    vlan_mode = s->vlan_mode;
+    if (vlan_mode == PORT_VLAN_EMPTY) {
+        if (s->vlan >= 0) {
+            vlan_mode = PORT_VLAN_ACCESS;
+        } else {
+            vlan_mode = PORT_VLAN_TRUNK;
+        }
+    }
+    if (vlan_mode != bundle->vlan_mode) {
+        bundle->vlan_mode = vlan_mode;
+        need_flush = true;
+    }
+
     /* Set VLAN tag. */
     if (s->vlan != bundle->vlan) {
         bundle->vlan = s->vlan;
@@ -1056,7 +1077,11 @@ bundle_set(struct ofproto *ofproto_, void *aux,
     }
 
     /* Get trunked VLANs. */
-    trunks = s->vlan == -1 ? NULL : s->trunks;
+    if (vlan_mode == PORT_VLAN_ACCESS) {
+        trunks = NULL;
+    } else {
+        trunks = s->trunks;
+    }
     if (!vlan_bitmap_equal(trunks, bundle->trunks)) {
         free(bundle->trunks);
         bundle->trunks = vlan_bitmap_clone(trunks);
@@ -3376,16 +3401,54 @@ static bool
 set_dst(struct action_xlate_ctx *ctx, struct dst *dst,
         const struct ofbundle *in_bundle, const struct ofbundle *out_bundle)
 {
-    dst->vlan = (out_bundle->vlan >= 0 ? OFP_VLAN_NONE
-                 : in_bundle->vlan >= 0 ? in_bundle->vlan
-                 : ctx->flow.vlan_tci == 0 ? OFP_VLAN_NONE
-                 : vlan_tci_to_vid(ctx->flow.vlan_tci));
+    /*
+     * Following rules apply for traffic going out of the bundle:
+     * - if output is access (bundle mode empty with tag>=0 or access), untag
+     * - else if output bundle mode is native-untagged and current
+     *   tag = output bundle native vlan, untag
+     * - else use vlan from input flow
+     */
+
+    /* output is access ? */
+    if (out_bundle->vlan_mode == PORT_VLAN_ACCESS) {
+
+	/* Untag everything */
+	dst->vlan = OFP_VLAN_NONE;
+
+    } else {
+
+	/*
+         * Get the original flow VLAN
+         * If port mode is access, native, and the flow is
+         * unttaged then use port native tag
+         */
+	int flow_vlan = vlan_tci_to_vid(ctx->flow.vlan_tci);
+	if (in_bundle->vlan_mode == PORT_VLAN_ACCESS
+            || (in_bundle->vlan_mode == PORT_VLAN_NATIVE_UNTAGGED
+                && flow_vlan == 0)
+            || (in_bundle->vlan_mode == PORT_VLAN_NATIVE_TAGGED
+                && flow_vlan == 0)) {
+
+            flow_vlan = in_bundle->vlan;
+        }
+
+        /*
+         * post process the original flow VLAN to take into account
+         * out_bundle configuration : untag if and only if port mode
+         * is native-untagged mode and vlan=tag
+         */
+        if (out_bundle->vlan_mode == PORT_VLAN_NATIVE_UNTAGGED
+            && flow_vlan == out_bundle->vlan) {
+            dst->vlan = OFP_VLAN_NONE;
+        } else {
+            dst->vlan = flow_vlan == 0 ? OFP_VLAN_NONE : flow_vlan;
+        }
+    }
 
     dst->port = (!out_bundle->bond
                  ? ofbundle_get_a_port(out_bundle)
                  : bond_choose_output_slave(out_bundle->bond, &ctx->flow,
                                             dst->vlan, &ctx->tags));
-
     return dst->port != NULL;
 }
 
@@ -3447,8 +3510,17 @@ dst_is_duplicate(const struct dst_set *set, const struct dst *test)
 static bool
 ofbundle_trunks_vlan(const struct ofbundle *bundle, uint16_t vlan)
 {
-    return (bundle->vlan < 0
-            && (!bundle->trunks || bitmap_is_set(bundle->trunks, vlan)));
+    /*
+     * Returns true if and only if:
+     * port is working as trunk (not an access port) and
+     * - trunks is empty
+     * - or trunks includes vlan
+     */
+    if (bundle->vlan_mode == PORT_VLAN_ACCESS) {
+        return false;
+    } else {
+        return (!bundle->trunks || bitmap_is_set(bundle->trunks, vlan));
+    }
 }
 
 static bool
@@ -3665,8 +3737,19 @@ flow_get_vlan(struct ofproto_dpif *ofproto, const struct flow *flow,
               struct ofbundle *in_bundle, bool have_packet)
 {
     int vlan = vlan_tci_to_vid(flow->vlan_tci);
-    if (in_bundle->vlan >= 0) {
-        if (vlan) {
+
+    /*
+     * First case : vlan != 0 input packet is tagged. Returns:
+     * - -1 if port is access
+     * - flow vlan if vlan mode is trunk or native-(un)tagged and flow vlan
+     *   is part of the trunk
+     * - -1 if vlan mode is trunk or native-(un)tagged and flow vlan is not
+     *   part of the trunk
+     */
+    if (vlan) {
+        if (in_bundle->vlan_mode == PORT_VLAN_ACCESS) {
+
+            /* Drop tagged packet on access port */
             if (have_packet) {
                 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
                 VLOG_WARN_RL(&rl, "bridge %s: dropping VLAN %d tagged "
@@ -3676,10 +3759,16 @@ flow_get_vlan(struct ofproto_dpif *ofproto, const struct flow *flow,
                              in_bundle->name, in_bundle->vlan);
             }
             return -1;
-        }
-        vlan = in_bundle->vlan;
-    } else {
-        if (!ofbundle_includes_vlan(in_bundle, vlan)) {
+
+        } else if (ofbundle_includes_vlan(in_bundle, vlan)) {
+
+            /* Accept packet if it is comming from a VLAN
+             * that is member of the port "trunks" list */
+            return vlan;
+
+        } else {
+
+            /* Drop packets from a VLAN not member of the trunk */
             if (have_packet) {
                 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
                 VLOG_WARN_RL(&rl, "bridge %s: dropping VLAN %d tagged "
@@ -3689,9 +3778,23 @@ flow_get_vlan(struct ofproto_dpif *ofproto, const struct flow *flow,
             }
             return -1;
         }
-    }
 
-    return vlan;
+    /*
+     * Second case : vlan=0, input packet is untagged. Returns:
+     * - bundle native vlan if vlan mode is access or native-(un)tagged
+     * - 0 if mode is trunk and flow vlan (0) is part of the trunk
+     * - -1 if mode is trunk and flow vlan (0) is not part of the trunk
+     */
+    } else {
+        if (in_bundle->vlan_mode == PORT_VLAN_ACCESS
+            || in_bundle->vlan_mode == PORT_VLAN_NATIVE_TAGGED
+            || in_bundle->vlan_mode == PORT_VLAN_NATIVE_UNTAGGED) {
+
+            return in_bundle->vlan;
+        } else {
+            return ofbundle_includes_vlan(in_bundle, 0) ? 0 : -1;
+        }
+    }
 }
 
 /* A VM broadcasts a gratuitous ARP to indicate that it has resumed after
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index e0c99ea..db8b6c4 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -179,6 +179,41 @@ void ofproto_port_set_cfm(struct ofproto *, uint16_t ofp_port,
                           const struct cfm_settings *);
 int ofproto_port_is_lacp_current(struct ofproto *, uint16_t ofp_port);
 
+/* The behaviour of the port regarding VLAN handling */
+enum port_vlan_mode {
+    /*
+     * For compatibility.  Access mode if tag is defined or Trunk mode
+     * if trunk is defined.  Native VLAN feature is disabled
+     */
+    PORT_VLAN_EMPTY,
+
+    /*
+     * This port is an access port.  All packets are tagged with "vlan"
+     * that comes from the "tag" column.  The "trunk" column is ignored
+     */
+    PORT_VLAN_ACCESS,
+
+    /*
+     * This port is a trunk.  The "tag" column is ignored.  Packets are
+     * unchanged.
+     */
+    PORT_VLAN_TRUNK,
+
+    /*
+     * Untagged incoming packets are tagged with "vlan" comming from
+     * the "tag" column.  Outgoing packets tagged with "vlan" stay
+     * tagged.  The "tag" and "trunks" columns are both used.
+     */
+    PORT_VLAN_NATIVE_TAGGED,
+
+    /*
+     * Untagged incoming packets are tagged with "vlan" comming from
+     * the "tag" column.  Outgoing packets tagged with "vlan" are
+     * untagged.  The "tag" and "trunks" columns are both used.
+     */
+    PORT_VLAN_NATIVE_UNTAGGED,
+};
+
 /* Configuration of bundles. */
 struct ofproto_bundle_settings {
     char *name;                 /* For use in log messages. */
@@ -186,6 +221,7 @@ struct ofproto_bundle_settings {
     uint16_t *slaves;           /* OpenFlow port numbers for slaves. */
     size_t n_slaves;
 
+    enum port_vlan_mode vlan_mode; /* Selects mode for vlan and trunks */
     int vlan;                   /* VLAN if access port, -1 if trunk port. */
     unsigned long *trunks;      /* vlan_bitmap, NULL to trunk all VLANs. */
 
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index d99d6a1..65317b8 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -484,6 +484,23 @@ port_configure(struct port *port)
         s.slaves[s.n_slaves++] = iface->ofp_port;
     }
 
+    /* Get VLAN mode */
+    s.vlan_mode = PORT_VLAN_EMPTY;
+    if (cfg->vlan_mode) {
+        if (!strcmp(cfg->vlan_mode, "access")) {
+            s.vlan_mode = PORT_VLAN_ACCESS;
+        } else if (!strcmp(cfg->vlan_mode, "trunk")) {
+            s.vlan_mode = PORT_VLAN_TRUNK;
+        } else if (!strcmp(cfg->vlan_mode, "native-tagged")) {
+            s.vlan_mode = PORT_VLAN_NATIVE_TAGGED;
+        } else if (!strcmp(cfg->vlan_mode, "native-untagged")) {
+            s.vlan_mode = PORT_VLAN_NATIVE_UNTAGGED;
+        } else {
+            VLOG_WARN("port %s: unknown VLAN mode %s.",
+                      port->name, cfg->vlan_mode);
+        }
+    }
+
     /* Get VLAN tag. */
     s.vlan = -1;
     if (cfg->tag) {
@@ -504,9 +521,11 @@ port_configure(struct port *port)
     s.trunks = NULL;
     if (s.vlan < 0 && cfg->n_trunks) {
         s.trunks = vlan_bitmap_from_array(cfg->trunks, cfg->n_trunks);
-    } else if (s.vlan >= 0 && cfg->n_trunks) {
-        VLOG_ERR("port %s: ignoring trunks in favor of implicit vlan",
-                 port->name);
+    } else if (s.vlan >= 0 && cfg->n_trunks ) {
+        if (s.vlan_mode == PORT_VLAN_EMPTY) {
+            VLOG_ERR("port %s: ignoring trunks in favor of implicit vlan",
+                     port->name);
+        }
     }
 
     /* Get LACP settings. */
diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
index ca61a2c..138569f 100644
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -1,6 +1,6 @@
 {"name": "Open_vSwitch",
- "version": "5.2.0",
- "cksum": "434778864 14545",
+ "version": "5.3.0",
+ "cksum": "418437550 14728",
  "tables": {
    "Open_vSwitch": {
      "columns": {
@@ -115,6 +115,10 @@
                           "minInteger": 0,
                           "maxInteger": 4095},
                   "min": 0, "max": 1}},
+       "vlan_mode": {
+         "type": {"key": {"type": "string",
+           "enum": ["set", ["trunk", "access", "native-tagged", "native-untagged"]]},
+         "min": 0, "max": 1}},
        "qos": {
          "type": {"key": {"type": "uuid",
                           "refTable": "QoS"},
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 6082256..c78e745 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -499,8 +499,8 @@
     </column>
 
     <group title="VLAN Configuration">
-      <p>A bridge port must be configured for VLANs in one of two
-        mutually exclusive ways:
+      <p>A bridge port must be configured for VLANs in one of these
+        ways:
         <ul>
           <li>A ``trunk port'' has an empty value for <ref
             column="tag"/>.  Its <ref column="trunks"/> value may be
@@ -508,11 +508,32 @@
           <li>An ``implicitly tagged VLAN port'' or ``access port''
             has an nonempty value for <ref column="tag"/>.  Its
             <ref column="trunks"/> value must be empty.</li>
+          <li>An trunk port for tagged packets mixed with implicitly
+            tagged VLAN for untagged packets. The implicit tag is
+            usually called native VLAN, default VLAN or port VLAN identifier
+            (PVID). The VLAN tag persistence is configurable. When disabled
+            (native untagged), the VLAN tag is removed from leaving packets
+            whose VLAN tag matches the port native VLAN.</li>
         </ul>
-        If <ref column="trunks"/> and <ref column="tag"/> are both
-        nonempty, the configuration is ill-formed.
       </p>
 
+      <column name="vlan_mode">
+        <p>
+          This allows to select the working mode of the port. Allowed
+          values are:
+          <ul>
+            <li>empty: the OpenVswitch standard behaviour is used.</li>
+            <li>access: the port is an access port (see above).</li>
+            <li>native-untagged: tagged packets are unchanged. Arriving
+              untagged packets are tagged with native VLAN. Leaving packets
+              tagged with native VLAN are untagged.</li>
+            <li>native-tagged: tagged packets are unchanged. Arriving
+              untagged packets are tagged with native VLAN. Leaving packets
+              tagged with native VLAN are unchanged.</li>
+          </ul>
+        </p>
+      </column>
+
       <column name="tag">
         <p>
           If this is an access port (see above), the port's implicitly
@@ -531,6 +552,27 @@
           When a frame with a 802.1Q header that indicates a nonzero
           VLAN is received on an access port, it is discarded.
         </p>
+        <p>
+          If this is a pvid or untagged-pvid port, this column contains
+          the PVID value. The <ref column="trunks"/> value must also be
+          defined.
+        </p>
+        <p>
+          Frames arriving on this port with an 802.1Q header indicating
+          a nonzero VLAN will be forwarded based on destination
+          <ref column="trunks"/> value. Frames arriving on this port
+          without an 802.1Q header or with an 802.1Q header indicating
+          VLAN 0 will first be tagged with <ref column="tag"/> value
+          then will be forwarded based on destination
+          <ref column="trunks"/> value.
+	</p>
+	<p>
+          Frames leaving this port will keep their VLAN header expect
+          if mode is untagged-pvid. In this case, all frames leaving
+          this port will keep their VLAN header if it is different
+          from <ref column="tag"/>. If it is equal to <ref column="tag"/>
+          the frame is untagged.
+        </p>
       </column>
 
       <column name="trunks">



More information about the dev mailing list