Hello

I reviewed the userspace code and try to make it work in ovs. I found it can't 
calculate right vlan actions in a complex ovs topologies, such as a three-ovs 
topology which are connected by patch-port,like following
      Br-1                            br-2
      |                               |
Vm---->Br-1(patch-to-br2)----------(patch-to-br-1)br-2(patch-to-br-3)---------(patch-to-br2)br-3-----ethx--->


Vm: send packets tagged with 100
Br-1: pop_vlan 100
Br-2: patch-to-br-1 is a access port tagged with vlan id 2
Br-3:pop_vlan 2,push_vlan 300(0x8100), push_vlan 200(TPID:0x88a8)

Under above topology, the QinQ patch can't work at all.
I reviewd the function commit_vlan_actions, I think this function logic is too 
simple to get right vlan actions.
In commit_vlan_action, there are struct flow *base and struct flow *flow and to 
get vlan action(push or pop)by comparing there vlan depth. If QinQ is 
available, still using the following method to get vlan actions will get wrong.
For example, Double vlan changes as follows if under above topology : 
100(tci)+0(ctci)-->0+0-->1+0-->0+0-->100+0-->200+300.
Then base's double vlan is (100,0), flow's double vlan is (200,300), base_n is 
1, flow_n is 2,then according following method, it should push 200, but what it 
should do is to pop 100 1st, push 300 2nd, push 200 3rd.Apparently, it misses 
to check inner vlan tci.
Hope this example can help to enhance the function.

+static void
+commit_vlan_action(const struct flow *flow, struct flow *base,
+                   struct ofpbuf *odp_actions, struct flow_wildcards *wc)
+{
+    ovs_be16 vlan_tci = flow->vlan_tci;
+    ovs_be16 vlan_tpid = flow->vlan_tpid;
+    int base_n;
+    int flow_n;
+
+
+    if ((base->vlan_tci == vlan_tci) && (base->vlan_ctci == vlan_tci))
+        return;
+
+    base_n = flow_get_vlan_depth(base);
+    flow_n = flow_get_vlan_depth(flow);
+
+
+    if (flow_n == base_n) {
+        if (vlan_tci == base->vlan_tci) {
+            return;
+        } else {
+            pop_vlan(base, odp_actions, wc);
+            push_vlan(base, vlan_tci, vlan_tpid, odp_actions, wc);
+        }
+    }
+    else if (flow_n > base_n) {
+        push_vlan(base, vlan_tci, vlan_tpid, odp_actions, wc);
+    }
+    else if (flow_n < base_n) {
+        pop_vlan(base, odp_actions, wc);
+    }
}



_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to