[ovs-dev] [PATCH v3 0/4] Add support for 802.1ad (QinQ tunneling)
Thomas F Herbert
thomasfherbert at gmail.com
Fri Jul 15 12:08:33 UTC 2016
Eric and Xiao, I am cc'ing this to the ovs-dev list because other
reviewers who reviewed the kernel module may want to weigh in.
--TFH
On 7/12/16 2:17 PM, Eric Garver wrote:
> This looks okay to me. This function was definitely missing a call to
> parse_flow_mask_nlattrs() for the masked case. My only question is why
> the original code has "if (unlikely(inner))".
>
> Tom,
> Can you speak to why this was checking for the inner tag?
Eric, please see my comments below.
>
> On Tue, Jul 12, 2016 at 11:37:32PM +0800, Xiao Liang wrote:
>> Hi Eric,
>>
>> Nice to hear that we could work together on this feature, and many
>> thanks for you effort. Hope we could get it work in the upstream soon.
>>
>> By the way, for the kernel patch, I changed the last lines in
>> __parse_vlan_from_nlattrs:
>> if (unlikely(inner)) {
>> err = parse_flow_nlattrs(encap, a, en_attrs, log);
>> if (err)
>> return err;
>> }
>> return 0;
>> to:
>> if (is_mask)
>> err = parse_flow_mask_nlattrs(encap, a, en_attrs, log);
>> else
>> err = parse_flow_nlattrs(encap, a, en_attrs, log);
>>
>> return err;
>>
>> Is it reasonable?
Yes, I think you are correct. The mask case seems to be missing from my
V20 kernel module patch. Maybe when I refactored the code to use a
common function for inner and outer vlans, I dropped the mask case.
However, unlikely(inner) may be OK though because the single tagged vlan
is the more common case.
A complication is that the OF spec says that matching should be on the
outer tag only. (See section 7.2.3.8.) I don't recall but the intent of
this code may have been to check for a mask on the outer tag only.
However, it makes sense to code it in such a way that both tags are
handled symmetrically.
>>
>> Thanks,
>> Xiao
>>
>>
>> On Mon, Jul 11, 2016 at 9:45 PM, Eric Garver <egarver at redhat.com> wrote:
>>> Hi Xiao,
>>>
>>> I hope it's okay to unicast you. I wanted to let you know that I'm
>>> working with Thomas to get 802.1ad support accepted upstream (OVS and
>>> kernel).
>>>
>>> Your patchset here looks well done. I will continue testing and
>>> providing feedback.
>>>
>>> Regarding the kernel, I'm currently in progress of rebasing, testing,
>>> and fixing a couple issues. When I think it's ready I will post it to
>>> the netdev mailing list for review.
>>>
>>> Hopefully together we can get both user space and kernel support
>>> upstream in the very near future.
>>>
>>> Take care.
>>> Eric.
>>>
>>> On Sun, Jul 03, 2016 at 08:47:05AM +0800, Xiao Liang wrote:
>>>> These patches enable tpid 0x88a8, multiple VLAN headers, and add new vlan_mode "dot1q-tunnel". See commit message of patch1 for details.
>>>>
>>>> v2: Add VLAN handling test cases for dot1q-tunnel vlan_mode.
>>>> Adjust VLAN+MPLS test cases for multi VLAN.
>>>> v3: Set default max_vlan_headers of netdev and test-odp (fix test failure found by Tom).
>>>> Fix loop condition when parsing mask in odp-util.c.
>>>> Clean up comments.
>>>>
>>>>
>>>> Xiao Liang (4):
>>>> Add support for 802.1ad (QinQ tunneling)
>>>> tests: Add tests for QinQ VLAN handling
>>>> tests: Adjust VLAN+MPLS handling cases for QinQ
>>>> tests: Set default max_vlan_headers
>>>>
>>>> include/openvswitch/flow.h | 13 +-
>>>> include/openvswitch/ofp-actions.h | 10 +-
>>>> include/openvswitch/packets.h | 5 +
>>>> lib/dpctl.c | 29 ++-
>>>> lib/dpif-netdev.c | 8 +-
>>>> lib/flow.c | 109 ++++++----
>>>> lib/flow.h | 6 +-
>>>> lib/match.c | 47 ++--
>>>> lib/meta-flow.c | 22 +-
>>>> lib/nx-match.c | 14 +-
>>>> lib/odp-util.c | 227 ++++++++++++--------
>>>> lib/odp-util.h | 4 +-
>>>> lib/ofp-actions.c | 61 +++---
>>>> lib/ofp-util.c | 56 ++---
>>>> lib/tnl-ports.c | 2 +-
>>>> ofproto/bond.c | 2 +-
>>>> ofproto/ofproto-dpif-ipfix.c | 6 +-
>>>> ofproto/ofproto-dpif-rid.h | 2 +-
>>>> ofproto/ofproto-dpif-sflow.c | 4 +-
>>>> ofproto/ofproto-dpif-xlate.c | 436 ++++++++++++++++++++++++++------------
>>>> ofproto/ofproto-dpif-xlate.h | 6 +-
>>>> ofproto/ofproto-dpif.c | 74 ++++++-
>>>> ofproto/ofproto.h | 8 +-
>>>> ovn/controller/pinctrl.c | 5 +-
>>>> tests/ofproto-dpif.at | 238 ++++++++++++---------
>>>> tests/test-classifier.c | 15 +-
>>>> tests/test-odp.c | 1 +
>>>> utilities/ovs-ofctl.c | 29 +--
>>>> vswitchd/bridge.c | 27 ++-
>>>> vswitchd/vswitch.ovsschema | 16 +-
>>>> vswitchd/vswitch.xml | 31 +++
>>>> 31 files changed, 1002 insertions(+), 511 deletions(-)
>>>>
>>>> --
>>>> 2.9.0
>>>>
>>>> _______________________________________________
>>>> dev mailing list
>>>> dev at openvswitch.org
>>>> http://openvswitch.org/mailman/listinfo/dev
More information about the dev
mailing list