On Wed, Sep 30, 2015 at 2:47 AM, mengke <mengke....@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. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
