[ovs-dev] [PATCH 0/7] Enable NSH based Service Function Chaining support in OVS

[Jesse Gross]

On Wed, Sep 30, 2015 at 2:47 AM, mengke <mengke.liu at intel.com> wrote:
> This patch adds support for NSH function implementation which can be used for
> service function chaining. It covers control plane, kernel dataplane and
> DPDK-netdev dataplane.

I have several high level comments on this series that I think it
would be best to discuss before going through the code level details.

First and foremost, the kernel tunneling code that you are modifying
is compatibility code. Upstream Linux is now using a different model
of tunnels and any new features need to be added to that rather than
to the old model as is done here. The new model is called "lightweight
tunnels" so I would encourage you to look at what has been going on
with that upstream.

That being said, I have significant concerns with the layering
throughout this patchset and I suspect if that was resolved then at
least some of the above changes would not be necessary. In particular,
there is a great deal of conflation between VXLAN-GPE and NSH. As a
theoretical matter, I believe that these are supposed to operate
independently of each other at different protocol layers. And more
practically, this seems to result in a lot of special "modes" to
handle various specific cases.

It looks like the best way to handle this is by creating a push/pop
mechanism for inserting and removing NSH headers over various
protocols. As it is, I really don't like that there are a variety of
specific operations such as for converting between VXLAN+NSH to
Ethernet+NSH - this should be decomposed. I believe that this
decomposition would also help address some other issues. For example,
it's not OK to hard code the UDP port of VXLAN-GPE in the kernel -
this needs to be configurable. However, the port for vanilla VXLAN is
already configurable so if you can add support for GPE and then layer
NSH on top then you don't need to introduce a new configuration
mechanism.

The last thing that I wanted to mention is that I would strongly
recommend looking at supporting TLVs now. There has been extensive
work in supporting them for Geneve and not only should it now be much
easier to do than it would have been a little while back, some of
these public interfaces have not been part of a released version of
OVS yet. Therefore, if there are changes that need to be made to make
them support multiple protocols, now would be the time to do it before
they can no longer be modified.