Thanks Justin for the review, I will look at it and create v2.

Yifeng

On Thu, Sep 13, 2018 at 3:19 PM Justin Pettit <[email protected]> wrote:

>
>
> > On Sep 11, 2018, at 6:52 AM, Yifeng Sun <[email protected]> wrote:
> >
> > Please backport this patch to upstream OVS down to 2.9.
> >
> > On certain kernel versions, when openvswitch kernel module creates
> > a gre0 interface, the kernel’s gre module will jump out and compete
> > to control the gre0 interface. This will cause the failure of
> > openvswitch kernel module loading.
> >
> > This fix renames fallback devices by adding a prefix "ovs-".
> >
> > Signed-off-by: Yifeng Sun <[email protected]>
> > Issue: #2162866
> > ---
> > datapath/linux/compat/ip6_gre.c    | 5 +++--
> > datapath/linux/compat/ip6_tunnel.c | 6 +++---
> > datapath/linux/compat/ip_gre.c     | 2 +-
> > datapath/linux/compat/ip_tunnel.c  | 3 ++-
> > 4 files changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/datapath/linux/compat/ip6_gre.c
> b/datapath/linux/compat/ip6_gre.c
> > index 00dbefc..08b31da 100644
> > --- a/datapath/linux/compat/ip6_gre.c
> > +++ b/datapath/linux/compat/ip6_gre.c
> > @@ -377,7 +377,7 @@ static struct ip6_tnl *ip6gre_tunnel_locate(struct
> net *net,
> >       if (parms->name[0])
> >               strlcpy(name, parms->name, IFNAMSIZ);
> >       else
> > -             strcpy(name, "ip6gre%d");
> > +             strcpy(name, "ovs-ip6gre%d");
>
> We're starting to make this device name pretty long.  Should we be doing
> any sort of sanity-check on the length, since we're generating the name?  I
> did a quick search through the kernel sources, and couldn't determine where
> that "%d" is actually replaced and whether there's a check.
>
> >
> > diff --git a/datapath/linux/compat/ip6_tunnel.c
> b/datapath/linux/compat/ip6_tunnel.c
> > index 56fd8b4..06cebda 100644
> > --- a/datapath/linux/compat/ip6_tunnel.c
> > +++ b/datapath/linux/compat/ip6_tunnel.c
> > @@ -355,7 +355,7 @@ static struct ip6_tnl *ip6_tnl_create(struct net
> *net, struct __ip6_tnl_parm *p)
> >       if (p->name[0])
> >               strlcpy(name, p->name, IFNAMSIZ);
> >       else
> > -             sprintf(name, "ip6tnl%%d");
> > +             sprintf(name, "ovs-ip6tnl%%d");
>
> Same question here.
>
> > static void __net_exit ipgre_tap_exit_net(struct net *net)
> > diff --git a/datapath/linux/compat/ip_tunnel.c
> b/datapath/linux/compat/ip_tunnel.c
> > index d16e60f..b85ada3 100644
> > --- a/datapath/linux/compat/ip_tunnel.c
> > +++ b/datapath/linux/compat/ip_tunnel.c
> > @@ -134,7 +134,8 @@ static struct net_device *__ip_tunnel_create(struct
> net *net,
> >                       err = -E2BIG;
> >                       goto failed;
> >               }
> > -             strlcpy(name, ops->kind, IFNAMSIZ);
> > +             strlcpy(name, "ovs-", IFNAMSIZ);
> > +             strlcat(name, ops->kind, IFNAMSIZ);
> >               strncat(name, "%d", 2);
> >       }
>
> A little earlier in this function, there's a length check:
>
>         if (strlen(ops->kind) > (IFNAMSIZ - 3)) {
>             err = -E2BIG;
>             goto failed;
>         }
>
> Should we be bumping that by four to account for the "ovs-" that was added?
>
> --Justin
>
>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to