[ovs-dev] [PATCH] dpif: Ensure ERSPAN GRE support

Eric Garver e at erig.me
Fri Jun 1 15:30:12 UTC 2018


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 <guru at ovn.org>
> > > Signed-off-by: Greg Rose <gvrose8192 at 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.


More information about the dev mailing list