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.y...@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.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
