On Wed, Sep 14, 2016 at 12:34:36PM +0800, Xiao Liang wrote: > Flow key handleing changes: > - Add VLAN header array in struct flow, to record multiple 802.1q VLAN > headers. > - Add dpif multi-VLAN capability probing. If datapath supports multi-VLAN, > increase the maximum depth of nested OVS_KEY_ATTR_ENCAP. > > Refacter VLAN handling in dpif-xlate: > - Introduce 'xvlan' to track VLAN stack during flow processing. > - Input and output VLAN translation according to the xbundle type. > > Push VLAN action support: > - Allow ethertype 0x88a8 in VLAN headers and push_vlan action. > - Support push_vlan on dot1q packets. > > Add new port VLAN mode "dot1q-tunnel": > - Example: > ovs-vsctl set Port p1 vlan_mode=dot1q-tunnel tag=100 > Pushes another VLAN 100 header on packets (tagged and untagged) on ingress, > and pops it on egress. > - Customer VLAN check: > ovs-vsctl set Port p1 vlan_mode=dot1q-tunnel tag=100 cvlans=10,20 > Only customer VLAN of 10 and 20 are allowed. > > Use other_config:vlan-limit in table Open_vSwitch to limit maxium VLANs > that can be matched > > Add test cases for VLAN depth limit, Multi-VLAN actions and QinQ VLAN handling > > Signed-off-by: Xiao Liang <shaw.l...@gmail.com>
Thanks for working on this. "sparse" says: ../lib/odp-util.c:5052:42: warning: restricted ovs_be16 degrades to integer The fix is: diff --git a/lib/odp-util.c b/lib/odp-util.c index 527cd7a..12ebda1 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -5049,7 +5049,7 @@ parse_8021q_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1], while (encaps < max_vlan_headers && (is_mask ? - (src_flow->vlans[encaps].tci & htons(VLAN_CFI)) : + (src_flow->vlans[encaps].tci & htons(VLAN_CFI)) != 0 : eth_type_vlan(flow->dl_type))) { /* Calculate fitness of outer attributes. */ encap = (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_ENCAP) In miniflow_extract(), the following code always adds an extra 64-bit word to the miniflow, even if there are no vlans. It should avoid that if the vlan doubleword is going to be all-0s. (Previously, this optimization was not necessary because the VLAN was in the same doubleword as the Ethertype, which is always nonzero.) /* dl_type */ dl_type = parse_ethertype(&data, &size); miniflow_push_be16(mf, dl_type, dl_type); miniflow_pad_to_64(mf, dl_type); miniflow_push_words_32(mf, vlans, vlans, FLOW_MAX_VLAN_HEADERS); In flow_wildcards_init_for_packet(), I think that the safer thing to do would be to implement the FIXME: + /* No need to set mask of inner VLANs that doesn't exist. + * FIXME: set mask of the first zero VLAN? */ + WC_MASK_FIELD(wc, vlans[0]); + for (int i = 1; i < FLOW_MAX_VLAN_HEADERS; i++) { + if (flow->vlans[i].tci == htons(0)) { + break; + } + WC_MASK_FIELD(wc, vlans[i]); + } I don't think that a max_vlan_headers global variable is a good idea. Different datapaths might have different limits. I'm still really skeptical of the value of the whole "xvlan" data structure in ofproto-dpif-xlate. It seems like it adds an extra stage of translation to and from a new format, but I don't see how it adds much value. Can you show me where it removes a significant amount of code, or makes code much clearer, or has other value? Thanks, Ben. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev