On Tue, Jun 21, 2016 at 5:53 PM, Yang, Yi <yi.y.y...@intel.com> wrote:
> 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.
Please take another look at the interface. Going forward, tunnel ports
are not to be configured through OVS kernel directly but through their
respective netlink interface (for VXLAN extensions this would mean
VXLAN netlink) and attached to the datapath. This does not require any
special modifications to the OVS interface and as a result these
changes are not necessary.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
