[ovs-dev] [PATCH v2] VxLAN-gpe implementation

Yang, Yi yi.y.yang at intel.com
Wed Jun 22 00:53:21 UTC 2016


On Tue, Jun 21, 2016 at 09:26:58AM -0700, Jesse Gross wrote:
> On Mon, Jun 20, 2016 at 7:30 PM, Yang, Yi <yi.y.yang at intel.com> wrote:
> > On Mon, Jun 20, 2016 at 07:00:56PM -0700, Jesse Gross wrote:
> >> >>
> >> >> Hi, Yi Yang.
> >> >>
> >> >> Before adding the OVS_VXLAN_EXT_GPE extension to the out-of-tree module, you
> >> >> should send it to the mainline kernel. Besides, you need a very good
> >> >> justification why you can't wait for my patchset to be accepted and have
> >> >> VXLAN-GPE enabled using rtnetlink.
> >> >
> >> > Will add VS_VXLAN_EXT_GPE to include/uapi/linux/openvswitch.h and send a
> >> > kernel patch, but ovs and net-next kernel also can work together.
> >>
> >> I think that you might have misunderstood the gist of the suggestion -
> >> please do not add this extension. This is pure compatibility code for
> >> existing, old features and should not be extended with new things
> >> unless there is a very, very good reason (which would almost certainly
> >> be related to fixing bugs, not new functionality) - especially since
> >> there's already an existing mechanism to do this.
> >
> > I'm confused, kernel has this header file
> > include/uapi/linux/openvswitch.h, but ovs also has this header file
> > datapath/linux/compat/include/linux/openvswitch.h, both of them included
> > this enumeration definition:
> >
> > enum {
> >         OVS_VXLAN_EXT_UNSPEC,
> >         OVS_VXLAN_EXT_GBP,      /* Flag or __u32 */
> >         __OVS_VXLAN_EXT_MAX,
> > };
> >
> > Don't we need to make sure they are consistent with the below line?
> >
> >         OVS_VXLAN_EXT_GPE,      /* Flag, Generic Protocol Extension */
> 
> You don't need to add this, either upstream or in the OVS tree. It's
> already possible to configure VXLAN GPE through the VXLAN netlink
> interface, so that's what you should be using.
> 

Here gpe is still an extension to vxlan even if we use rtnetlink to
configure it instead of the traditional gbp way, so anyway
OVS_VXLAN_EXT_GPE definition is required, now the issue is where we
define it.  I think the best place is still in
ovs/datapath/linux/compat/include/linux/openvswitch.h, it will be better
if kernel header file include/uapi/linux/openvswitch.h also has it.

--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -287,6 +287,7 @@ enum ovs_vport_attr {
 enum {
        OVS_VXLAN_EXT_UNSPEC,
        OVS_VXLAN_EXT_GBP,      /* Flag or __u32 */
+       OVS_VXLAN_EXT_GPE,      /* Flag, Generic Protocol Extension */
        __OVS_VXLAN_EXT_MAX,
 };

> When doing backports, please do a straight backport of what is
> upstream without adding anything new. I would also like you to layer
> this on top of VXLAN previous code that needs to be backported rather
> than taking it out of order.

Make sense, will do some changes to make sure as small as possible
changes in previous VXLAN code.




More information about the dev mailing list