On Fri, Jun 01, 2018 at 07:40:49AM -0700, Gregory Rose wrote: > On 6/1/2018 6:15 AM, Eric Garver wrote: > > I'm a bit late, but I have comments below. > > > > I'm also a bit out of touch, so I may be missing some context - if so, I > > apologize. > > > > On Thu, May 31, 2018 at 03:50:31PM -0700, Greg Rose wrote: > > > When verifying the built-in gre kernel module check for ERSPAN support. > > > > > > Reported-by: Guru Shetty <g...@ovn.org> > > > Signed-off-by: Greg Rose <gvrose8...@gmail.com> > > > --- > > > lib/dpif-netlink-rtnl.c | 10 +++++----- > > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > > > diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c > > > index bec3fce..197cfb6 100644 > > > --- a/lib/dpif-netlink-rtnl.c > > > +++ b/lib/dpif-netlink-rtnl.c > > > @@ -45,8 +45,8 @@ VLOG_DEFINE_THIS_MODULE(dpif_netlink_rtnl); > > > #ifndef IFLA_GRE_MAX > > > #define IFLA_GRE_MAX 0 > > > #endif > > > -#if IFLA_GRE_MAX < 18 > > > -#define IFLA_GRE_COLLECT_METADATA 18 > > > +#if IFLA_GRE_MAX < 24 > > > +#define IFLA_GRE_ERSPAN_HWID 24 > > > #endif > > > #ifndef IFLA_GENEVE_MAX > > > @@ -74,7 +74,7 @@ static const struct nl_policy vxlan_policy[] = { > > > [IFLA_VXLAN_GPE] = { .type = NL_A_FLAG, .optional = true }, > > > }; > > > static const struct nl_policy gre_policy[] = { > > > - [IFLA_GRE_COLLECT_METADATA] = { .type = NL_A_FLAG }, > > I think we still need to verify _COLLECT_METADATA was passed back. > > > > > + [IFLA_GRE_ERSPAN_HWID] = { .type = NL_A_U16 }, > > > }; > > > static const struct nl_policy geneve_policy[] = { > > > [IFLA_GENEVE_COLLECT_METADATA] = { .type = NL_A_FLAG }, > > > @@ -207,7 +207,7 @@ dpif_netlink_rtnl_gre_verify(const struct > > > netdev_tunnel_config OVS_UNUSED *tnl, > > > err = rtnl_policy_parse(kind, reply, gre_policy, gre, > > > ARRAY_SIZE(gre_policy)); > > > if (!err) { > > > - if (!nl_attr_get_flag(gre[IFLA_GRE_COLLECT_METADATA])) { > > > + if (!nl_attr_get_u16(gre[IFLA_GRE_ERSPAN_HWID])) { > > We need to verify COLLECT_METADATA is actually in use here. We should > > only be checking for ERSPAN if ERSPAN was requested by the user. > > I'm not super familiar with the code but the intent is to prevent the > built-in ip_gre kernel module > from being used if it doesn't support ERSPAN. If the built-in gre and
dpif_netlink_rtnl_probe_oot_tunnels() is responsible for deciding if in-tree (linux) or out-of-tree (ovs) modules should be used. There are two cases: 1) If the out-of-tree tunnel modules are loaded, the out-of-tree compat interface will be used 2) otherwise in-tree tunnel modules are used (rtnetlink, i.e. code in this file). - if device create fails, then it falls back to the compat interface > ip_gre kernel modules > are loaded and the kernel doesn't have ERSPAN (4.16 and above) then OVS > users won't > be able to use ERSPAN features. IIRC, the out-of-tree version of ip_gre will not be used on newer kernels anyway. See references to USE_UPSTREAM_TUNNEL. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev