netdev - Re: [PATCH 3/3] 802.1AD: Flow handling, actions, vlan parsing and netlink attributes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALnjE+oqFNGUNHbXgkr8+gWM1TDFSi-jbuL+kggrqZD+AaDK1A@mail.gmail.com>
Date:	Tue, 29 Sep 2015 15:56:20 -0700
From:	Pravin Shelar <pshelar@...ira.com>
To:	Thomas F Herbert <thomasfherbert@...il.com>
Cc:	netdev <netdev@...r.kernel.org>, therbert@...hat.com,
	"dev@...nvswitch.org" <dev@...nvswitch.org>
Subject: Re: [PATCH 3/3] 802.1AD: Flow handling, actions, vlan parsing and
 netlink attributes

On Fri, Sep 25, 2015 at 3:35 PM, Thomas F Herbert
<thomasfherbert@...il.com> wrote:
> Pravin,
>
> Another comment and question. Please seen inline below.
>
> Thanks,
>
> --Tom
>
> On 9/24/15 7:42 PM, Pravin Shelar wrote:
>>
>> On Thu, Sep 24, 2015 at 10:58 AM, Thomas F Herbert
>> <thomasfherbert@...il.com> wrote:
>>>
>>> Add support for 802.1ad including the ability to push and pop double
>>> tagged vlans. Add support for 802.1ad to netlink parsing and flow
>>> conversion. Uses double nested encap attributes to represent double
>>> tagged vlan. Inner TPID encoded along with ctci in nested attributes.
>>>
>>> Signed-off-by: Thomas F Herbert <thomasfherbert@...il.com>
>>> ---
>>>   net/openvswitch/flow.c         |  83 +++++++++++++++++----
>>>   net/openvswitch/flow.h         |   5 ++
>>>   net/openvswitch/flow_netlink.c | 166
>>> ++++++++++++++++++++++++++++++++++++++---
>>>   3 files changed, 230 insertions(+), 24 deletions(-)
>>>
...

>>> @@ -1320,6 +1437,7 @@ static int __ovs_nla_put_key(const struct
>>> sw_flow_key *swkey,
>>>   {
>>>          struct ovs_key_ethernet *eth_key;
>>>          struct nlattr *nla, *encap;
>>> +       struct nlattr *in_encap = NULL;
>>>
>>>          if (nla_put_u32(skb, OVS_KEY_ATTR_RECIRC_ID, output->recirc_id))
>>>                  goto nla_put_failure;
>>> @@ -1368,17 +1486,42 @@ static int __ovs_nla_put_key(const struct
>>> sw_flow_key *swkey,
>>>          ether_addr_copy(eth_key->eth_src, output->eth.src);
>>>          ether_addr_copy(eth_key->eth_dst, output->eth.dst);
>>>
>>> -       if (swkey->eth.tci || swkey->eth.type == htons(ETH_P_8021Q)) {
>>> +       if (swkey->eth.tci || eth_type_vlan(swkey->eth.type)) {
>>>                  __be16 eth_type;
>>> -               eth_type = !is_mask ? htons(ETH_P_8021Q) : htons(0xffff);
>>> +
>>> +               if (swkey->eth.cvlan.ctci ||
>>> +                   eth_type_vlan(swkey->eth.cvlan.c_tpid))
>>> +                       eth_type = !is_mask ? htons(ETH_P_8021AD) :
>>> +                                             htons(0xffff);
>>> +               else
>>> +                       eth_type = !is_mask ? htons(ETH_P_8021Q) :
>>> +                                             htons(0xffff);
>>> +
>>
>> Here we can directly dump output->eth.type to netlink. No need to
>> check for inner encap.
>
> The eth.type is set to the inner encapsulated protocol not to the tpid. We
> don't "know" what the outer tpid so I assume it is 802.1Q. To address this
> situation, do you think I should add the outer tpid to sw_flow_key?
> Also see comment above in flow.h.
>

With the addition of nested vlan, we need to add outer tpid. This will
simplify vlan netlink serialization too.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html