RE: [PATCH net-next v7] openvswitch: enable NSH support

[Jan Scheurich <jan.scheurich@ericsson.com>]

> > 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