Great. By the way, the indication of how far back it should be backported is very helpful. However, can you please add it after the “---“ so that it’s not part of the commit message.
Thanks! --Justin > On Sep 13, 2018, at 3:52 PM, Yifeng Sun <[email protected]> wrote: > > 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
