[ovs-dev] [PATCH 1/2] ofproto-dpif-xlate: don't check vlan in normal action if vid changed

Hunt Xu mhuntxu at gmail.com
Mon Oct 31 18:39:29 UTC 2016


If a packet's vlan vid has been changed(striped or modified) before
entering the "normal" processing, it should be consider a packet on the
new VLAN. Therefore vlan checking is not needed.

Signed-off-by: Hunt Xu <mhuntxu at gmail.com>
---
 ofproto/ofproto-dpif-xlate.c | 23 ++++++++++++----
 tests/ofproto-dpif.at        | 65 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 82 insertions(+), 6 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index f6391ed..c533a4e 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -374,8 +374,9 @@ struct xlate_ctx {
      * When translation is otherwise complete, ofpacts_execute_action_set()
      * converts it to a set of "struct ofpact"s that can be translated into
      * datapath actions. */
-    bool action_set_has_group;  /* Action set contains OFPACT_GROUP? */
-    struct ofpbuf action_set;   /* Action set. */
+    bool action_set_has_group;        /* Action set contains OFPACT_GROUP? */
+    bool action_set_changes_vlan_vid; /* Action set changes VLAN vid? */
+    struct ofpbuf action_set;         /* Action set. */
 
     enum xlate_error error;     /* Translation failed. */
 };
@@ -2314,11 +2315,17 @@ xlate_normal(struct xlate_ctx *ctx)
 
     /* Check VLAN. */
     vid = vlan_tci_to_vid(flow->vlan_tci);
-    if (!input_vid_is_valid(vid, in_xbundle, ctx->xin->packet != NULL)) {
-        xlate_report(ctx, "disallowed VLAN VID for this input port, dropping");
-        return;
+    if (ctx->action_set_changes_vlan_vid) {
+        xlate_report(ctx, "flow VLAN VID changed to %u, not checking", vid);
+        vlan = vid;
+    } else  {
+        if (!input_vid_is_valid(vid, in_xbundle, ctx->xin->packet != NULL)) {
+            xlate_report(ctx, "disallowed VLAN VID for this input "
+                         "port, dropping");
+            return;
+        }
+        vlan = input_vid_to_vlan(in_xbundle, vid);
     }
-    vlan = input_vid_to_vlan(in_xbundle, vid);
 
     /* Check other admissibility requirements. */
     if (in_port && !is_admissible(ctx, in_port, vlan)) {
@@ -4767,6 +4774,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
                 flow->vlan_tci &= ~htons(VLAN_VID_MASK);
                 flow->vlan_tci |= (htons(ofpact_get_SET_VLAN_VID(a)->vlan_vid)
                                    | htons(VLAN_CFI));
+                ctx->action_set_changes_vlan_vid = true;
             }
             break;
 
@@ -4783,12 +4791,14 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
         case OFPACT_STRIP_VLAN:
             memset(&wc->masks.vlan_tci, 0xff, sizeof wc->masks.vlan_tci);
             flow->vlan_tci = htons(0);
+            ctx->action_set_changes_vlan_vid = true;
             break;
 
         case OFPACT_PUSH_VLAN:
             /* XXX 802.1AD(QinQ) */
             memset(&wc->masks.vlan_tci, 0xff, sizeof wc->masks.vlan_tci);
             flow->vlan_tci = htons(VLAN_CFI);
+            ctx->action_set_changes_vlan_vid = true;
             break;
 
         case OFPACT_SET_ETH_SRC:
@@ -5362,6 +5372,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
         .ct_nat_action = NULL,
 
         .action_set_has_group = false,
+        .action_set_changes_vlan_vid = false,
         .action_set = OFPBUF_STUB_INITIALIZER(action_set_stub),
     };
 
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index ec7bd60..123394d 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -3269,6 +3269,71 @@ done
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([ofproto-dpif - VLAN handling, vid changed])
+OVS_VSWITCHD_START(
+  [set Bridge br0 fail-mode=secure -- \
+   add-port br0 p1                           tag=1              -- \
+   add-port br0 p2                           tag=2              -- \
+   add-port br0 p3                           tag=3              -- \
+   add-port br0 p4                                 trunks=4,5   -- \
+   add-port br0 p5                           tag=4              \
+                   other-config:priority-tags=true              -- \
+   add-port br0 p6 vlan_mode=native-untagged tag=6 trunks=4,5,6 \
+                   other-config:priority-tags=true              -- \
+   set Interface p1 type=dummy ofport_request=101 -- \
+   set Interface p2 type=dummy ofport_request=102 -- \
+   set Interface p3 type=dummy ofport_request=103 -- \
+   set Interface p4 type=dummy ofport_request=104 -- \
+   set Interface p5 type=dummy ofport_request=105 -- \
+   set Interface p6 type=dummy ofport_request=106 --])
+
+AT_CHECK([ovs-ofctl add-flow br0 in_port=101,actions=mod_vlan_vid:0,normal])
+AT_CHECK([ovs-ofctl add-flow br0 in_port=102,actions=strip_vlan,normal])
+AT_CHECK([ovs-ofctl add-flow br0 in_port=103,actions=mod_vlan_vid:4,normal])
+AT_CHECK([ovs-ofctl add-flow br0 in_port=104,actions=mod_vlan_vid:6,normal])
+
+dnl Each of these specifies an in_port by number, a VLAN VID (or "none"),
+dnl a VLAN PCP (used if the VID isn't "none") and the expected set of datapath
+dnl actions.
+for tuple in \
+        "1 none 0 100" \
+        "1 10   0 pop_vlan,100" \
+        "1 11   1 pop_vlan,100" \
+        "2 none 0 100" \
+        "2 12   1 pop_vlan,100" \
+        "2 13   0 pop_vlan,100" \
+        "3 none 0 5,push_vlan(vid=4,pcp=0),4,6,100" \
+        "3 14   0 pop_vlan,5,push_vlan(vid=4,pcp=0),4,6,100" \
+        "3 15   1 pop_vlan,push_vlan(vid=0,pcp=1),5,pop_vlan,push_vlan(vid=4,pcp=1),4,6,100" \
+        "4 none 0 6,push_vlan(vid=6,pcp=0),100" \
+        "4 16   1 pop_vlan,push_vlan(vid=0,pcp=1),6,pop_vlan,push_vlan(vid=6,pcp=1),100"
+do
+  set $tuple
+  in_port=$1
+  vlan=$2
+  pcp=$3
+  expected=$4
+
+  if test $vlan = none; then
+    flow="in_port($in_port),eth(src=50:54:00:00:00:01,dst=ff:ff:ff:ff:ff:ff),eth_type(0xabcd)"
+  else
+    flow="in_port($in_port),eth(src=50:54:00:00:00:01,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8100),vlan(vid=$vlan,pcp=$pcp),encap(eth_type(0xabcd))"
+  fi
+
+  echo "----------------------------------------------------------------------"
+  echo "in_port=$in_port vlan=$vlan pcp=$pcp"
+
+  AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "$flow"], [0], [stdout])
+  actual=`tail -1 stdout | sed 's/Datapath actions: //'`
+
+  AT_CHECK([ovs-dpctl normalize-actions "$flow" "$expected"], [0], [stdout])
+  mv stdout expout
+  AT_CHECK([ovs-dpctl normalize-actions "$flow" "$actual"], [0], [expout])
+done
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([ofproto-dpif - MPLS handling])
 OVS_VSWITCHD_START([dnl
    add-port br0 p1 -- set Interface p1 type=dummy
-- 
2.10.2




More information about the dev mailing list