[ovs-dev] [RFC PATCH v2 00/13] Add Network Service Header Support

[Jesse Gross]

On Wed, Jul 13, 2016 at 6:38 PM, Yang, Yi <yi.y.yang at intel.com> wrote:
> On Wed, Jul 13, 2016 at 09:59:34AM -0700, Jesse Gross wrote:
>> On Wed, Jul 13, 2016 at 7:55 AM, Jiri Benc <jbenc at redhat.com> wrote:
>> > On Wed, 13 Jul 2016 07:35:59 -0700, Jesse Gross wrote:
>> >> I think history tells us how this will end - similar to IPv4 options,
>> >> implementations that don't implement TLVs will become deployed and
>> >> then when there is a use for them it's no longer possible. Since I
>> >> don't want OVS to have a half implementation or contribute to this
>> >> issue, I'd like to see the whole protocol implemented before I apply
>> >> anything.
>> >
>> > I see a big difference between this and IPv4. While in IPv4, the
>> > options are extension to existing headers, here we're talking about a
>> > completely different payload. It's more comparable to http vs. ftp (of
>> > course, it's a poor comparison, but I hope it illustrates at least a
>> > bit what I mean).
>> >
>> > If NSH takes off (and it's a big "if" in my opinion), it's also well
>> > possible we'll see more metadata types. The spec is pretty much open to
>> > this. Obviously, the authors are aware of that and type 2 is optional.
>> > As I guess will be type 3 and type 4 and whatever.
>> >
>> > It's pretty much inevitable that applications and deployments built
>> > around MD type 1 won't support MD type 2. And vice versa. This is
>> > regardless whether ovs supports MD type 2 or not. They're just a
>> > different protocol.
>> >
>> > In my opinion, starting with MD type 1 is a good way to reduce the
>> > initial scope. I see no problem with adding MD type 2 later.
>>
>> I understand what you are saying but I'm not sure that I agree that
>> the two metadata types should be viewed as essentially independent
>> protocols. I guess it's probably also pretty unlikely that additional
>> metadata types will be created in the future.
>>
>> If you look at discussions in the IETF and other places, it seems like
>> a frequent response to questions about MD type 1's design is that any
>> limitations can be handled with type 2. So to me this looks like the
>> two pieces are interrelated and the situation is quite similar to
>> IPv4.
>>
>> In any case, I don't think this is a fundamental issue, just a matter
>> of timing. Since the premise of the original question was that MD type
>> 2 shouldn't be too much additional work and the series is currently
>> dependent on Simon's patches, it seems like now might be a good time
>> for the authors to look into implementing this.
>
> Jesse, MD type 2 support needs a lot of changes, you know
> tunnel_metadata GENEVE is part of tunnel, but it should be part of NSH in NSH,
> you suggested we are to leverage GENEVE tunnel_metadata, but how to
> leverage it better is a big question mark for us. they should be called
> as nsh_metadata in NSH, so we can't directly use GENEVE's match fields
> and struct flow_tnl in struct flow, we find a better way to reuse Geneve
> tunnel_metadata code, that needs to generalize Geneve tunnel_metadata
> code in order that NSH and Geneve can share them but we can have
> different match fields name but can use a struct metadata in struct
> flow, what do you think about it?
>
> If you agree this, then I can confirm we will have a lot of changes,
> especially for GENEVE.

It's possible to have aliases for field names in OVS, so that might be
one possible solution. However, you had the opportunity (and did!) to
rename these fields or otherwise make them more amenable to your needs
so I'm not really all that receptive to complaints in this area. In
any case, I don't think that it is really a good idea to have 2x 256
bytes in struct flow (which is an internal structure and can still be
changed) if that is what you are proposing.

> So my personal opinion is to merge MD type 1 support at first, MD type 2
> needs a lot of changes, most of them are in Geneve side but not in NSH
> side.

I think I've been pretty consistent about my desire to see both MD
types contained in the same patch series as well as the constraints
imposed by the current interfaces (this was also nicely summarized by
Jan Scheurich). If you need to generalize existing infrastructure
please do that (but without breaking any existing external
interfaces).