netdev - RE: [PATCH net-next v7] 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]

Message-ID: <CFF8EF42F1132E4CBE2BF0AB6C21C58D787F4EEB@ESESSMB107.ericsson.se>
Date:   Mon, 4 Sep 2017 14:07:45 +0000
From:   Jan Scheurich <jan.scheurich@...csson.com>
To:     Jiri Benc <jbenc@...hat.com>
CC:     "Yang, Yi" <yi.y.yang@...el.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "dev@...nvswitch.org" <dev@...nvswitch.org>,
        "e@...g.me" <e@...g.me>, "blp@....org" <blp@....org>
Subject: RE: [PATCH net-next v7] openvswitch: enable NSH support

> > So is what you are suggesting the following?
> >
> > For matching:
> > OVS_KEY_ATTR_NSH_BASE_HEADER		mandatory
> > OVS_KEY_ATTR_NSH_MD1_CONTEXT		optional, in case MDTYPE == MD1
> 
> This needs to be:
> 
> OVS_KEY_ATTR_NSH
> 	OVS_KEY_ATTR_NSH_BASE_HEADER
> 	OVS_KEY_ATTR_NSH_MD1_CONTEXT
> 
> > For setting:
> > OVS_ACTION_ATTR_SET_MASKED
> > -    OVS_KEY_ATTR_NSH_BASE_HEADER		when setting any base header fields
> > OVS_ACTION_ATTR_SET_MASKED
> > -    OVS_KEY_ATTR_NSH_MD1_CONTEXT	when setting any MD1 context data fields
> 
> This needs to be:
> 
> OVS_ACTION_ATTR_SET_MASKED
> 	OVS_KEY_ATTR_NSH
> 		OVS_KEY_ATTR_NSH_BASE_HEADER
> 		OVS_KEY_ATTR_NSH_MD1_CONTEXT
> 

Then perhaps I misunderstood your comment. I thought you didn't like that the 
SET_MASKED action wrapped OVS_KEY_ATTR_NSH which in itself was nested.
I was aiming to avoid this by lifting the two components of the NSH header 
components to the top level:
OVS_NSH_ATTR_BASE_HEADER --> OVS_KEY_ATTR_NSH_BASE_HEADER
OVS_NSH_ATTR_MD1_CONTEXT --> OVS_KEY_ATTR_NSH_MD1_CONTEXT

> In your example, there's no way to distinguish OVS_KEY_ATTR_ENCAP and
> OVS_KEY_ATTR_NSH_BASE_HEADER, they are both "1".

See above. The two would be separate values in the same enum, i.e. distinct.

> > Should we consider to stick to that simple and efficient approach? As far
> > As I can see it will be generic for the foreseeable future.
> 
> I'm not sure. From the kernel point of view, we want the same
> functionality offered by the openvswitch module and by a tc action.
> 
> In theory, it can be the tc tool that constructs the header from the
> command line but there's no precedent. Dunno.

Not sure I have the full picture here. Are you saying that the tc tool uses the same
Netlink API to the kernel as OVS? What would be the back-end for tc? Also the
openvswitch kernel module? 

BR, Jan