Do not output priority tagged frames.

Linux hosts (and probably others) tend to ignore priority-tagged frames, so
it's safest not to output them at all.

On "master" we've added a per-port setting for whether priority-tagged
frames may be output, but "master" and "branch-1.3" have diverged
significantly in how they handle port output, so it's much simpler to
port this simpler policy to "branch-1.3".

Reported-by: Michael Mao <mmao@nicira.com>
Bug #8320.

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 7af9ee2..c41f4db 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4640,12 +4640,27 @@ compose_mirror_dsts(struct action_xlate_ctx *ctx,
     }
 }
 
+static void
+compose_dst_output_action(struct action_xlate_ctx *ctx, const struct dst *dst)
+{
+    ovs_be16 tci;
+
+    tci = htons(dst->vid);
+    if (tci) {
+        tci |= ctx->flow.vlan_tci & htons(VLAN_PCP_MASK);
+        tci |= htons(VLAN_CFI);
+    }
+    commit_vlan_action(ctx, tci);
+
+    compose_output_action(ctx, dst->port->odp_port);
+}
+
 static void
 compose_actions(struct action_xlate_ctx *ctx, uint16_t vlan,
                 const struct ofbundle *in_bundle,
                 const struct ofbundle *out_bundle)
 {
-    uint16_t initial_vid, cur_vid;
+    uint16_t initial_vid;
     const struct dst *dst;
     struct dst_set set;
 
@@ -4661,31 +4676,16 @@ compose_actions(struct action_xlate_ctx *ctx, uint16_t vlan,
     commit_odp_actions(ctx);
     initial_vid = vlan_tci_to_vid(ctx->flow.vlan_tci);
     for (dst = set.dsts; dst < &set.dsts[set.n]; dst++) {
-        if (dst->vid != initial_vid) {
-            continue;
+        if (dst->vid == initial_vid) {
+            compose_dst_output_action(ctx, dst);
         }
-        compose_output_action(ctx, dst->port->odp_port);
     }
 
     /* Then output the rest. */
-    cur_vid = initial_vid;
     for (dst = set.dsts; dst < &set.dsts[set.n]; dst++) {
-        if (dst->vid == initial_vid) {
-            continue;
-        }
-        if (dst->vid != cur_vid) {
-            ovs_be16 tci;
-
-            tci = htons(dst->vid);
-            tci |= ctx->flow.vlan_tci & htons(VLAN_PCP_MASK);
-            if (tci) {
-                tci |= htons(VLAN_CFI);
-            }
-            commit_vlan_action(ctx, tci);
-
-            cur_vid = dst->vid;
+        if (dst->vid != initial_vid) {
+            compose_dst_output_action(ctx, dst);
         }
-        compose_output_action(ctx, dst->port->odp_port);
     }
 
     dst_set_free(&set);

diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index ec5c238..bdb22bd 100644 (file)
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -117,29 +117,25 @@ br0=0 p1=$1 p2=$2 p3=$3 p4=$4 p5=$5 p6=$6 p7=$7 p8=$8
 dnl Each of these specifies an in_port, a VLAN VID (or "none"), a VLAN
 dnl PCP (used if the VID isn't "none") and the expected set of datapath
 dnl actions.
-dnl
-dnl XXX Some of these actually output an 802.1Q header to an access port
-dnl (see for example the actions for in_port=p3, vlan=0) to qualify the
-dnl packet with a priority.  That should be configurable.
 for tuple in \
         "br0 none 0 drop" \
         "br0 0    0 drop" \
         "br0 0    1 drop" \
         "br0 10   0 p1,p5,p6,p7,p8,pop(vlan),p2" \
-        "br0 10   1 p1,p5,p6,p7,p8,pop(vlan),push(vlan(vid=0,pcp=1)),p2" \
+        "br0 10   1 p1,p5,p6,p7,p8,pop(vlan),p2" \
         "br0 11   0 p5,p7" \
         "br0 11   1 p5,p7" \
         "br0 12   0 p1,p5,p6,pop(vlan),p3,p4,p7,p8" \
-        "br0 12   1 p1,p5,p6,pop(vlan),push(vlan(vid=0,pcp=1)),p3,p4,p7,p8" \
+        "br0 12   1 p1,p5,p6,pop(vlan),p3,p4,p7,p8" \
         "p1  none 0 drop" \
         "p1  0    0 drop" \
         "p1  0    1 drop" \
         "p1  10   0 br0,p5,p6,p7,p8,pop(vlan),p2" \
-        "p1  10   1 br0,p5,p6,p7,p8,pop(vlan),push(vlan(vid=0,pcp=1)),p2" \
+        "p1  10   1 br0,p5,p6,p7,p8,pop(vlan),p2" \
         "p1  11   0 drop" \
         "p1  11   1 drop" \
         "p1  12   0 br0,p5,p6,pop(vlan),p3,p4,p7,p8" \
-        "p1  12   1 br0,p5,p6,pop(vlan),push(vlan(vid=0,pcp=1)),p3,p4,p7,p8" \
+        "p1  12   1 br0,p5,p6,pop(vlan),p3,p4,p7,p8" \
         "p2  none 0 push(vlan(vid=10,pcp=0)),br0,p1,p5,p6,p7,p8" \
         "p2  0    0 pop(vlan),push(vlan(vid=10,pcp=0)),br0,p1,p5,p6,p7,p8" \
         "p2  0    1 pop(vlan),push(vlan(vid=10,pcp=1)),br0,p1,p5,p6,p7,p8" \
@@ -150,8 +146,8 @@ for tuple in \
         "p2  12   0 drop" \
         "p2  12   1 drop" \
         "p3  none 0 p4,p7,p8,push(vlan(vid=12,pcp=0)),br0,p1,p5,p6" \
-        "p3  0    0 p4,p7,p8,pop(vlan),push(vlan(vid=12,pcp=0)),br0,p1,p5,p6" \
-        "p3  0    1 p4,p7,p8,pop(vlan),push(vlan(vid=12,pcp=1)),br0,p1,p5,p6" \
+        "p3  0    0 pop(vlan),p4,p7,p8,push(vlan(vid=12,pcp=0)),br0,p1,p5,p6" \
+        "p3  0    1 pop(vlan),p4,p7,p8,push(vlan(vid=12,pcp=1)),br0,p1,p5,p6" \
         "p3  10   0 drop" \
         "p3  10   1 drop" \
         "p3  11   0 drop" \
@@ -159,8 +155,8 @@ for tuple in \
         "p3  12   0 drop" \
         "p3  12   1 drop" \
         "p4  none 0 p3,p7,p8,push(vlan(vid=12,pcp=0)),br0,p1,p5,p6" \
-        "p4  0    0 p3,p7,p8,pop(vlan),push(vlan(vid=12,pcp=0)),br0,p1,p5,p6" \
-        "p4  0    1 p3,p7,p8,pop(vlan),push(vlan(vid=12,pcp=1)),br0,p1,p5,p6" \
+        "p4  0    0 pop(vlan),p3,p7,p8,push(vlan(vid=12,pcp=0)),br0,p1,p5,p6" \
+        "p4  0    1 pop(vlan),p3,p7,p8,push(vlan(vid=12,pcp=1)),br0,p1,p5,p6" \
         "p4  10   0 drop" \
         "p4  10   1 drop" \
         "p4  11   0 drop" \
@@ -168,41 +164,41 @@ for tuple in \
         "p4  12   0 drop" \
         "p4  12   1 drop" \
         "p5  none 0 p2,push(vlan(vid=10,pcp=0)),br0,p1,p6,p7,p8" \
-        "p5  0    0 p2,pop(vlan),push(vlan(vid=10,pcp=0)),br0,p1,p6,p7,p8" \
-        "p5  0    1 p2,pop(vlan),push(vlan(vid=10,pcp=1)),br0,p1,p6,p7,p8" \
+        "p5  0    0 pop(vlan),p2,push(vlan(vid=10,pcp=0)),br0,p1,p6,p7,p8" \
+        "p5  0    1 pop(vlan),p2,push(vlan(vid=10,pcp=1)),br0,p1,p6,p7,p8" \
         "p5  10   0 br0,p1,p6,p7,p8,pop(vlan),p2" \
-        "p5  10   1 br0,p1,p6,p7,p8,pop(vlan),push(vlan(vid=0,pcp=1)),p2" \
+        "p5  10   1 br0,p1,p6,p7,p8,pop(vlan),p2" \
         "p5  11   0 br0,p7" \
         "p5  11   1 br0,p7" \
         "p5  12   0 br0,p1,p6,pop(vlan),p3,p4,p7,p8" \
-        "p5  12   1 br0,p1,p6,pop(vlan),push(vlan(vid=0,pcp=1)),p3,p4,p7,p8" \
+        "p5  12   1 br0,p1,p6,pop(vlan),p3,p4,p7,p8" \
         "p6  none 0 p2,push(vlan(vid=10,pcp=0)),br0,p1,p5,p7,p8" \
-        "p6  0    0 p2,pop(vlan),push(vlan(vid=10,pcp=0)),br0,p1,p5,p7,p8" \
-        "p6  0    1 p2,pop(vlan),push(vlan(vid=10,pcp=1)),br0,p1,p5,p7,p8" \
+        "p6  0    0 pop(vlan),p2,push(vlan(vid=10,pcp=0)),br0,p1,p5,p7,p8" \
+        "p6  0    1 pop(vlan),p2,push(vlan(vid=10,pcp=1)),br0,p1,p5,p7,p8" \
         "p6  10   0 br0,p1,p5,p7,p8,pop(vlan),p2" \
-        "p6  10   1 br0,p1,p5,p7,p8,pop(vlan),push(vlan(vid=0,pcp=1)),p2" \
+        "p6  10   1 br0,p1,p5,p7,p8,pop(vlan),p2" \
         "p6  11   0 drop" \
         "p6  11   1 drop" \
         "p6  12   0 br0,p1,p5,pop(vlan),p3,p4,p7,p8" \
-        "p6  12   1 br0,p1,p5,pop(vlan),push(vlan(vid=0,pcp=1)),p3,p4,p7,p8" \
+        "p6  12   1 br0,p1,p5,pop(vlan),p3,p4,p7,p8" \
         "p7  none 0 p3,p4,p8,push(vlan(vid=12,pcp=0)),br0,p1,p5,p6" \
-        "p7  0    0 p3,p4,p8,pop(vlan),push(vlan(vid=12,pcp=0)),br0,p1,p5,p6" \
-        "p7  0    1 p3,p4,p8,pop(vlan),push(vlan(vid=12,pcp=1)),br0,p1,p5,p6" \
+        "p7  0    0 pop(vlan),p3,p4,p8,push(vlan(vid=12,pcp=0)),br0,p1,p5,p6" \
+        "p7  0    1 pop(vlan),p3,p4,p8,push(vlan(vid=12,pcp=1)),br0,p1,p5,p6" \
         "p7  10   0 br0,p1,p5,p6,p8,pop(vlan),p2" \
-        "p7  10   1 br0,p1,p5,p6,p8,pop(vlan),push(vlan(vid=0,pcp=1)),p2" \
+        "p7  10   1 br0,p1,p5,p6,p8,pop(vlan),p2" \
         "p7  11   0 br0,p5" \
         "p7  11   1 br0,p5" \
         "p7  12   0 br0,p1,p5,p6,pop(vlan),p3,p4,p8" \
-        "p7  12   1 br0,p1,p5,p6,pop(vlan),push(vlan(vid=0,pcp=1)),p3,p4,p8" \
+        "p7  12   1 br0,p1,p5,p6,pop(vlan),p3,p4,p8" \
         "p8  none 0 p3,p4,p7,push(vlan(vid=12,pcp=0)),br0,p1,p5,p6" \
-        "p8  0    0 p3,p4,p7,pop(vlan),push(vlan(vid=12,pcp=0)),br0,p1,p5,p6" \
-        "p8  0    1 p3,p4,p7,pop(vlan),push(vlan(vid=12,pcp=1)),br0,p1,p5,p6" \
+        "p8  0    0 pop(vlan),p3,p4,p7,push(vlan(vid=12,pcp=0)),br0,p1,p5,p6" \
+        "p8  0    1 pop(vlan),p3,p4,p7,push(vlan(vid=12,pcp=1)),br0,p1,p5,p6" \
         "p8  10   0 br0,p1,p5,p6,p7,pop(vlan),p2" \
-        "p8  10   1 br0,p1,p5,p6,p7,pop(vlan),push(vlan(vid=0,pcp=1)),p2" \
+        "p8  10   1 br0,p1,p5,p6,p7,pop(vlan),p2" \
         "p8  11   0 drop" \
         "p8  11   1 drop" \
         "p8  12   0 br0,p1,p5,p6,pop(vlan),p3,p4,p7" \
-        "p8  12   1 br0,p1,p5,p6,pop(vlan),push(vlan(vid=0,pcp=1)),p3,p4,p7"
+        "p8  12   1 br0,p1,p5,p6,pop(vlan),p3,p4,p7"
 do
   set $tuple
   in_port=$1