On Thu, Feb 02, 2017 at 02:37:22PM -0800, Joe Stringer wrote:
> Hi Eric,

Hi Joe,

Thanks for your feedback! Much appreciated.

> There's a few patches in this series with co-authored-by, but missing
> signed-off-by from the co-author. Also, the author does not need to be
> listed as a co-author.

Understood. I thought I followed an example from the git log, but maybe
I was mistaken. I'll fix it in the next round.

> 
> When building this series I'm seeing a bunch of redefinition errors
> (across all platforms I test on, ranging from kernel 3.10 to 4.8):
> 
> /usr/include/linux/if_tunnel.h:21:9: error: preprocessor token
> GRE_CSUM redefined
> lib/packets.h:1046:9: this was the original definition
> /usr/include/linux/if_tunnel.h:22:9: error: preprocessor token
> GRE_ROUTING redefined
> lib/packets.h:1047:9: this was the original definition
> /usr/include/linux/if_tunnel.h:23:9: error: preprocessor token GRE_KEY 
> redefined
> lib/packets.h:1048:9: this was the original definition
> /usr/include/linux/if_tunnel.h:24:9: error: preprocessor token GRE_SEQ 
> redefined
> lib/packets.h:1049:9: this was the original definition
> /usr/include/linux/if_tunnel.h:25:9: error: preprocessor token
> GRE_STRICT redefined
> lib/packets.h:1050:9: this was the original definition
> /usr/include/linux/if_tunnel.h:26:9: error: preprocessor token GRE_REC 
> redefined
> lib/packets.h:1051:9: this was the original definition
> /usr/include/linux/if_tunnel.h:27:9: error: preprocessor token
> GRE_FLAGS redefined
> lib/packets.h:1052:9: this was the original definition
> /usr/include/linux/if_tunnel.h:28:9: error: preprocessor token
> GRE_VERSION redefined
> lib/packets.h:1053:9: this was the original definition

This seems to be due to including both linux/if_tunnel.h and packets.h.
linux/if_tunnel.h was added by the original patch series. I'll
investigate if it's really needed.

> 
> I have a few comments on some of the patches.

Thanks. I addressed those separately.

> On 18 January 2017 at 11:45, Eric Garver <[email protected]> wrote:
> > This series adds support for the creation of tunnels using the rtnetlink
> > interface. This will open the possibility for new features and flags on 
> > those
> > vports without the need to change vport compatibility code.
> >
> > Support for STT and LISP have not been added because these are not upstream 
> > yet,
> > so we don't know how the interface will be like upstream. And there are no
> > features in the current drivers right now we could make use of.
> >
> > Note: This work originally started by Thadeu Lima de Souza Cascardo.
> >
> > Testing:
> >   - kernel 4.9.3, in-tree datapath
> >     - rtnetlink successfully creates devices
> >   - kernel 4.2.8, in-tree datapath
> >     - rtnetlink is tried, but fails due to no COLLECT_METADATA support
> >     - genetlink successfully creates devices
> >   - kernel 4.2.8, out-of-tree datapath
> >     - rtnetlink is not tried
> >     - genetlink successfully creates devices
> >
> > v2:
> >
> > We are able to set the MTU to UINT16_MAX since it is not restricted by the
> > driver during newlink.
> >
> > v3:
> >
> > Prefer to get type from vport before checking if device is opened. Also, 
> > disable
> > IFLA_VXLAN_LEARNING as it's not enabled on compat vports as well.
> >
> > v4:
> >   - Probe for ovs_geneve on init, this indicates out-of-tree datapath
> >     - If exists, only try genetlink/compat
> >     - else, try rtnetlink and fallback to genetlink/compat
> >   - Read back and verify devices created with rtnetlink
> >   - checkpatch fixes
> >
> > Eric Garver (4):
> >   dpif-netlink: Probe for out-of-tree datapath.
> >   dpif-netlink: add VXLAN creation support
> >   dpif-netlink: add GRE creation support
> >   dpif-netlink: add GENEVE creation support
> >
> > Thadeu Lima de Souza Cascardo (2):
> >   netdev: get device type from vport prefix if it uses one
> >   dpif-netlink: break out code to add compat and non-compat vports
> >
> >  lib/dpif-netlink.c | 679 
> > +++++++++++++++++++++++++++++++++++++++++++++++++----
> >  lib/netdev.c       |  26 +-
> >  2 files changed, 651 insertions(+), 54 deletions(-)
> >
> > --
> > 2.10.0
> >
> > _______________________________________________
> > dev mailing list
> > [email protected]
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to