Le 22/08/2011 22:27, Ben Pfaff a écrit :

> I think you're right.  I guess nobody (except you) tests trunking a
> limited set of VLANs.  We should fix this, thanks for the bug report.

Please find enclosed the new version of the patch to add native VLAN feature. 
Compared to previous version, major changes are:
- fixed the previously mentionned bug;
- after performing more tests, I found few mistakes in code logic. they are now 
fixed;
- took into account your remarks (PVID to native, coding style, enums ...). I 
hope I fixed everything :-)

If needed, I can post my OpenOffice table where I have written all tests I have 
performed

Philippe

---

diff --git a/NEWS b/NEWS
index ae6f55e..2992bd6 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 3fd95ea..51b7887 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. */
@@ -988,6 +989,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;
@@ -1046,6 +1048,12 @@ bundle_set(struct ofproto *ofproto_, void *aux,
         return EINVAL;
     }
 
+    /* Set VLAN tagging mode */
+    if (s->vlan_mode != bundle->vlan_mode) {
+        bundle->vlan_mode = s->vlan_mode;
+        need_flush = true;
+    }
+
     /* Set VLAN tag. */
     if (s->vlan != bundle->vlan) {
         bundle->vlan = s->vlan;
@@ -1053,7 +1061,12 @@ bundle_set(struct ofproto *ofproto_, void *aux,
     }
 
     /* Get trunked VLANs. */
-    trunks = s->vlan == -1 ? NULL : s->trunks;
+    if ((s->vlan_mode == PORT_VLAN_EMPTY && s->vlan >= 0)
+        || s->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);
@@ -3373,16 +3386,57 @@ 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_EMPTY && out_bundle->vlan >= 0)
+        || out_bundle->vlan_mode == PORT_VLAN_ACCESS) {
+
+       /* Untag everything */
+       dst->vlan = OFP_VLAN_NONE;
+
+    } else {
+
+       /*
+         * Get the original flow VLAN
+         * If port mode is empty with tag>=0, 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_EMPTY
+                && in_bundle->vlan >= 0)
+            || (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;
 }
 
@@ -3444,8 +3498,22 @@ 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. Implemented as port is not an access port
+     * which is defined by
+     * - mode is EMPTY and vlan>=0
+     * - or mode is ACCESS
+     * and
+     * - trunks is empty
+     * - or trunks includes vlan
+     */
+    if (bundle->vlan_mode == PORT_VLAN_ACCESS
+        || (bundle->vlan_mode == PORT_VLAN_EMPTY && bundle->vlan >= 0)) {
+        return false;
+    } else {
+        return (!bundle->trunks || bitmap_is_set(bundle->trunks, vlan));
+    }
 }
 
 static bool
@@ -3662,8 +3730,20 @@ 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 (vlan mode= access or empty with tag>=0)
+     * - 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_EMPTY && in_bundle->vlan >= 0)
+            || 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 "
@@ -3673,10 +3753,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 "
@@ -3686,9 +3772,25 @@ 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 empty with tag>=0, 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_EMPTY && in_bundle->vlan >= 0)
+            || 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..299e000 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">

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to