Fri, Aug 11, 2017 at 12:09:36PM CEST, jan.scheur...@ericsson.com wrote: >> -----Original Message----- >> From: Jiri Benc [mailto:jb...@redhat.com] >> Sent: Friday, 11 August, 2017 11:45 >> >> The context field does not apply to MD type 2. It looks wrong for the >> context field to be included in netlink attribute for anything other >> than MD type 1. Perhaps it needs to be put into a separate attribute, >> too? >> >> Note that I'm talking only about the uAPI. Internally, ovs can use >> struct ovs_key_nsh that is MD type 1 only, there's no problem changing >> that later. But for the user space interface, this needs to be clean. >> This can be solved for example this way: >> >> In include/uapi/linux/openvswitch.h: >> >> struct ovs_key_nsh_base { >> __u8 flags; >> __u8 mdtype; >> __u8 np; >> __u8 pad; >> __be32 path_hdr; >> }; >> >> + one more netlink attribute carrying MD type 1 info. Will probably >> require to change OVS_KEY_ATTR_NSH to a nested attribute etc. >> >> In net/openvswitch/flow.h (or perhaps a different header would be more >> appropriate?): >> >> struct ovs_key_nsh { >> struct ovs_key_nsh_base base; >> __be32 context[4]; >> }; >> >> Plus needed conversions between OVS_KEY_ATTR_NSH and struct ovs_key_nsh >> when interfacing between the kernel and user space. >> >> That way, we can have MD type 1 support only for now while still being >> allowed to redesign things in whatever way later. >> >> Jiri > >For the OVS_KEY_ATTR_NSH I agree to move the fixed size MD1 context headers >from nsh_base to a separate struct and use nested netlink attributes. > >For OVS_ACTION_ATTR_PUSH_NSH attribute any metadata included is opaque to the >datapath and should be copied as is into the packet header. I doubt that there >is any benefit to model this with nested attributes for MD1 or MD2. This just >adds complexity on sender and receiver side and requires updates in case there >should be other MD types added later on. > >Unless someone can explain to me why the datapath should understand the >internal structure/format of metadata in push_nsh, I would strongly vote to >keep the metadata as variable length octet sequence in the non-structured >OVS_ACTION_ATTR_PUSH_NSH
Could you please wrap lines at 72 chars? This is unreadable. Thanks!
