Hi Ben,
Please see inline.
And another question:
In datapath/README.md:
- If the userspace flow key includes more fields than the
kernel's, for example if userspace decoded an IPv6 header but
the kernel stopped at the Ethernet type, then userspace can
forward the packet manually, without setting up a flow in the
kernel.
Is there an example?
Thanks,
Xiao
On Wed, Oct 5, 2016 at 4:08 AM, Ben Pfaff <b...@ovn.org> wrote:
> 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.)
Yes.
>
> /* 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]);
> + }
Agree. But match_format will then print an extra vlan_tci1, so many
many tests need change.
>
> I don't think that a max_vlan_headers global variable is a good idea.
> Different datapaths might have different limits.
There's currently no datapath table in ovsdb. Is it possible to
configure limits for different datapath?
>
> 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?
It represents the internal VLAN stack of a packet and makes the "input
- internal - output" pipeline of VLAN processing clear.
For example, a packet with VLAN 100 comes from a tag 200 QinQ port,
the internal VLAN stack is 200,100. Then on egress side, we don't need
to check the packet and ingress port configuration combination. This
struct can be replaced by vlan_hdr, but needs extra vid extracting
each time it's accessed.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
