Re: [PATCH net-next v4] openvswitch: enable NSH support

[Jiri Benc <jbenc-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

On Mon, 21 Aug 2017 16:39:01 +0800, Yang, Yi wrote:
> Anyway, we need to keep the code in userspace consistent with the one in
> kernel as possible as, otherwise it will be a burden for developer, I
> know userspace has different coding standard from kernel, this will make
> developer painful if we have two sets of code although they have same
> functionality.

I'm sorry, I don't get this. What's wrong with having __u8[] as the
last member of the struct? That's C99. It's 18 years old standard.
We're using that throughout our uAPI. Why that should be a problem for
any user space program?

> > MPLS supports GSO and needs this for segmentation. I don't see anything
> > GSO related in this patch.
> > 
> > How do you plan to address GSO, anyway?
> 
> No plan to do that, I'm not an expert on this, we can remove it if
> you're very sure it is necessary.

Without GSO, I don't see any use for inner_protocol.

However, don't you need to software segment the packet if it's GSO
before pushing the NSH header?

And wouldn't it be better to implement GSO for NSH, anyway?

> To make sure we make agreement, please confirm if this one is ok?
> 
> struct nsh_hdr {
>     ovs_be16 ver_flags_ttl_len;
>     uint8_t mdtype;
>     uint8_t np;
>     ovs_16aligned_be32 path_hdr;
>     union {
>         struct nsh_md1_ctx md1;
>         struct nsh_md2_tlv md2;
>     };
> };
> 
> Or it will be better if you can provide your preferred version here.

I don't really care that much about the names if it's clear what they
mean. I was merely commenting on the inconsistency which looked weird.
Whether it's md_type or mdtype, I don't have a preference (does not
mean others won't, though :-)). Just pick one and stick to it, as far
as I'm concerned.

 Jiri