netdev - Re: [PATCH net-next v6 3/3] openvswitch: enable NSH support

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]

Date:   Tue, 05 Sep 2017 15:12:09 +0200
From:   Hannes Frederic Sowa <hannes@...essinduktion.org>
To:     "Yang\, Yi" <yi.y.yang@...el.com>
Cc:     "netdev\@vger.kernel.org" <netdev@...r.kernel.org>,
        "dev\@openvswitch.org" <dev@...nvswitch.org>,
        "jbenc\@redhat.com" <jbenc@...hat.com>, "e\@erig.me" <e@...g.me>,
        "blp\@ovn.org" <blp@....org>,
        "jan.scheurich\@ericsson.com" <jan.scheurich@...csson.com>
Subject: Re: [PATCH net-next v6 3/3] openvswitch: enable NSH support

"Yang, Yi" <yi.y.yang@...el.com> writes:

> On Tue, Sep 05, 2017 at 12:30:09PM +0200, Hannes Frederic Sowa wrote:
>> "Yang, Yi" <yi.y.yang@...el.com> writes:
>> 
>> > I'm not sure what new action you expect to bring here, I think group
>> > action is just for this, as you said it isn't only bound to NSH, you can
>> > start a new thread to discuss this. I don't think it is in scope of NSH.
>> 
>> It is in scope of this discussion as you will provide a user space API
>> that makes the NSH context fields accessible from user space in a
>> certain way. If you commit to this, there is no way going back.
>
> We can change this later if we really find a better way to handle this
> because it isn't defined in include/uapi/linux/openvswitch.h, so I still
> have backdoor to do this if needed :-)

Sorry, I can't follow you.

It doesn't matter if something is defined in uapi headers, the
observable behavior matters. If you allow users to configure flows with
specific fields, it should not stop working at a future point in time.

>> I haven't yet grasped the idea on how those fields will be used in OVS
>> besides load balancing. Even for load balancing the tunnel itself
>> (vxlan-gpe + UDP source port or ipv6 flowlabel) already provides enough
>> entropy to do per-flow load balancing. What else is needed?  Why a
>> context header for that? You just need multiple action chains and pick
>> one randomly.
>
> For our sfc use case in Opendaylight, we use context[0] for tunnel ID,
> context[1] for destination IP for reverse RSP, they are used to match
> and set in OpenFlow table, you can't limit users to use them in such
> ways.

So in your specific case you expect the masks to be completely stable
because you defined a protocol on top of NSH, understood. And that is
stable accross all possible paths. Understood as well.

> If you check GENEVE implementation, tun_metadata* can be set or matched
> as any other match field.

Yes, I wrote that in my previous mail. I wonder why NSH context metadata
is not in tun_metadata as well?

> Actually the most important information in NSH are just these context
> headers, you can't limit imagination of users by removing them from flow
> keys.
>
> My point is to bring miniflow into kernel data path to fix your concern,
> this will benefit your employer directly :-)

Okay, interesting. It will probably not help if you still have a hash of
a packet inside the flow table and use that for load balancing.

[...]

BTW I don't want to stop this patch, I am merely interested in how the
bigger picture will look like in the end.

Bye,
Hannes