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 <[email protected]>
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
[email protected]
http://openvswitch.org/mailman/listinfo/dev
