| Message ID | 1537071853-20196-1-git-send-email-pkusunyifeng@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,v4] gre: Rename fallback devices to avoid udev's interference | expand |
On 9/15/2018 9:24 PM, Yifeng Sun wrote: > 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 <pkusunyifeng@gmail.com> > VMware Issue: #2162866 > --- > Please backport this patch to upstream OVS down to 2.9, thanks. > > v1->v2: Added sanity check for device names, thanks Justin. > v2->v3: Fix an indent error. > v3->v4: Fix by code review, also fix a potenial bug. > > 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 | 7 +++---- > 4 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/datapath/linux/compat/ip6_gre.c b/datapath/linux/compat/ip6_gre.c > index 00dbefc9b099..182785273c6f 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"); > + strlcpy(name, "ovs-ip6gre%d", IFNAMSIZ); > > dev = alloc_netdev(sizeof(*t), name, NET_NAME_UNKNOWN, > ip6gre_tunnel_setup); > @@ -1712,7 +1712,8 @@ static int __net_init ip6gre_init_net(struct net *net) > struct ip6gre_net *ign = net_generic(net, ip6gre_net_id); > int err; > > - ign->fb_tunnel_dev = alloc_netdev(sizeof(struct ip6_tnl), "ip6gre0", > + ign->fb_tunnel_dev = alloc_netdev(sizeof(struct ip6_tnl), > + "ovs-ip6gre0", > NET_NAME_UNKNOWN, > ip6gre_tunnel_setup); > if (!ign->fb_tunnel_dev) { > diff --git a/datapath/linux/compat/ip6_tunnel.c b/datapath/linux/compat/ip6_tunnel.c > index 56fd8b4dd342..9f4bae7dd3d1 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"); > + strlcpy(name, "ovs-ip6tnl%d", IFNAMSIZ); > > dev = alloc_netdev(sizeof(*t), name, NET_NAME_UNKNOWN, > ip6_tnl_dev_setup); > @@ -1410,7 +1410,7 @@ ip6_tnl_parm_to_user(struct ip6_tnl_parm *u, const struct __ip6_tnl_parm *p) > * %SIOCCHGTUNNEL: change tunnel parameters to those given > * %SIOCDELTUNNEL: delete tunnel > * > - * The fallback device "ip6tnl0", created during module > + * The fallback device "ovs-ip6tnl0", created during module > * initialization, can be used for creating other tunnel devices. > * > * Return: > @@ -2093,7 +2093,7 @@ static int __net_init ip6_tnl_init_net(struct net *net) > ip6n->tnls[1] = ip6n->tnls_r_l; > > err = -ENOMEM; > - ip6n->fb_tnl_dev = alloc_netdev(sizeof(struct ip6_tnl), "ip6tnl0", > + ip6n->fb_tnl_dev = alloc_netdev(sizeof(struct ip6_tnl), "ovs-ip6tnl0", > NET_NAME_UNKNOWN, ip6_tnl_dev_setup); > > if (!ip6n->fb_tnl_dev) > diff --git a/datapath/linux/compat/ip_gre.c b/datapath/linux/compat/ip_gre.c > index 05132ba9494a..b7322c58e420 100644 > --- a/datapath/linux/compat/ip_gre.c > +++ b/datapath/linux/compat/ip_gre.c > @@ -1463,7 +1463,7 @@ static struct pernet_operations erspan_net_ops = { > > static int __net_init ipgre_tap_init_net(struct net *net) > { > - return ip_tunnel_init_net(net, gre_tap_net_id, &ipgre_tap_ops, "gretap0"); > + return ip_tunnel_init_net(net, gre_tap_net_id, &ipgre_tap_ops, "ovs-gretap0"); > } > > 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 d16e60fbfef0..ae5d5a058663 100644 > --- a/datapath/linux/compat/ip_tunnel.c > +++ b/datapath/linux/compat/ip_tunnel.c > @@ -130,12 +130,11 @@ static struct net_device *__ip_tunnel_create(struct net *net, > if (parms->name[0]) > strlcpy(name, parms->name, IFNAMSIZ); > else { > - if (strlen(ops->kind) > (IFNAMSIZ - 3)) { > + err = snprintf(name, IFNAMSIZ - 3, "ovs-%s%%d", ops->kind); > + if (err >= IFNAMSIZ - 3) > err = -E2BIG; > + if (err < 0) > goto failed; > - } > - strlcpy(name, ops->kind, IFNAMSIZ); > - strncat(name, "%d", 2); > } > > ASSERT_RTNL(); Thanks Yifeng for all your work on this. This looks OK but I'm not sure the additional protections against a device name being too long are really needed. I think in all cases the parms->name will be filled out since OVS is the only user. Have you tested whether the else code path will ever be entered? Sometimes kernel code gets into our backports even though it would never be executed within the context of OVS. It would be good to check this. Thanks, - Greg
Hi Greg, Sure, thanks for the review. I will work on it. Yifeng On Mon, Sep 17, 2018 at 8:49 AM Gregory Rose <gvrose8192@gmail.com> wrote: > On 9/15/2018 9:24 PM, Yifeng Sun wrote: > > 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 <pkusunyifeng@gmail.com> > > VMware Issue: #2162866 > > --- > > Please backport this patch to upstream OVS down to 2.9, thanks. > > > > v1->v2: Added sanity check for device names, thanks Justin. > > v2->v3: Fix an indent error. > > v3->v4: Fix by code review, also fix a potenial bug. > > > > 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 | 7 +++---- > > 4 files changed, 10 insertions(+), 10 deletions(-) > > > > diff --git a/datapath/linux/compat/ip6_gre.c > b/datapath/linux/compat/ip6_gre.c > > index 00dbefc9b099..182785273c6f 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"); > > + strlcpy(name, "ovs-ip6gre%d", IFNAMSIZ); > > > > dev = alloc_netdev(sizeof(*t), name, NET_NAME_UNKNOWN, > > ip6gre_tunnel_setup); > > @@ -1712,7 +1712,8 @@ static int __net_init ip6gre_init_net(struct net > *net) > > struct ip6gre_net *ign = net_generic(net, ip6gre_net_id); > > int err; > > > > - ign->fb_tunnel_dev = alloc_netdev(sizeof(struct ip6_tnl), > "ip6gre0", > > + ign->fb_tunnel_dev = alloc_netdev(sizeof(struct ip6_tnl), > > + "ovs-ip6gre0", > > NET_NAME_UNKNOWN, > > ip6gre_tunnel_setup); > > if (!ign->fb_tunnel_dev) { > > diff --git a/datapath/linux/compat/ip6_tunnel.c > b/datapath/linux/compat/ip6_tunnel.c > > index 56fd8b4dd342..9f4bae7dd3d1 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"); > > + strlcpy(name, "ovs-ip6tnl%d", IFNAMSIZ); > > > > dev = alloc_netdev(sizeof(*t), name, NET_NAME_UNKNOWN, > > ip6_tnl_dev_setup); > > @@ -1410,7 +1410,7 @@ ip6_tnl_parm_to_user(struct ip6_tnl_parm *u, const > struct __ip6_tnl_parm *p) > > * %SIOCCHGTUNNEL: change tunnel parameters to those given > > * %SIOCDELTUNNEL: delete tunnel > > * > > - * The fallback device "ip6tnl0", created during module > > + * The fallback device "ovs-ip6tnl0", created during module > > * initialization, can be used for creating other tunnel devices. > > * > > * Return: > > @@ -2093,7 +2093,7 @@ static int __net_init ip6_tnl_init_net(struct net > *net) > > ip6n->tnls[1] = ip6n->tnls_r_l; > > > > err = -ENOMEM; > > - ip6n->fb_tnl_dev = alloc_netdev(sizeof(struct ip6_tnl), "ip6tnl0", > > + ip6n->fb_tnl_dev = alloc_netdev(sizeof(struct ip6_tnl), > "ovs-ip6tnl0", > > NET_NAME_UNKNOWN, > ip6_tnl_dev_setup); > > > > if (!ip6n->fb_tnl_dev) > > diff --git a/datapath/linux/compat/ip_gre.c > b/datapath/linux/compat/ip_gre.c > > index 05132ba9494a..b7322c58e420 100644 > > --- a/datapath/linux/compat/ip_gre.c > > +++ b/datapath/linux/compat/ip_gre.c > > @@ -1463,7 +1463,7 @@ static struct pernet_operations erspan_net_ops = { > > > > static int __net_init ipgre_tap_init_net(struct net *net) > > { > > - return ip_tunnel_init_net(net, gre_tap_net_id, &ipgre_tap_ops, > "gretap0"); > > + return ip_tunnel_init_net(net, gre_tap_net_id, &ipgre_tap_ops, > "ovs-gretap0"); > > } > > > > 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 d16e60fbfef0..ae5d5a058663 100644 > > --- a/datapath/linux/compat/ip_tunnel.c > > +++ b/datapath/linux/compat/ip_tunnel.c > > @@ -130,12 +130,11 @@ static struct net_device > *__ip_tunnel_create(struct net *net, > > if (parms->name[0]) > > strlcpy(name, parms->name, IFNAMSIZ); > > else { > > - if (strlen(ops->kind) > (IFNAMSIZ - 3)) { > > + err = snprintf(name, IFNAMSIZ - 3, "ovs-%s%%d", ops->kind); > > + if (err >= IFNAMSIZ - 3) > > err = -E2BIG; > > + if (err < 0) > > goto failed; > > - } > > - strlcpy(name, ops->kind, IFNAMSIZ); > > - strncat(name, "%d", 2); > > } > > > > ASSERT_RTNL(); > > Thanks Yifeng for all your work on this. > > This looks OK but I'm not sure the additional protections against a > device name being too long are > really needed. I think in all cases the parms->name will be filled out > since OVS is the only user. > Have you tested whether the else code path will ever be entered? > Sometimes kernel code gets > into our backports even though it would never be executed within the > context of OVS. It would > be good to check this. > > Thanks, > > - Greg >
Hi Greg, I checked that the 'else' part in the above patch is actually executed. So it should be necessary to keep them. Thanks, Yifeng On Mon, Sep 17, 2018 at 9:51 AM Yifeng Sun <pkusunyifeng@gmail.com> wrote: > Hi Greg, > > Sure, thanks for the review. I will work on it. > > Yifeng > > On Mon, Sep 17, 2018 at 8:49 AM Gregory Rose <gvrose8192@gmail.com> wrote: > >> On 9/15/2018 9:24 PM, Yifeng Sun wrote: >> > 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 <pkusunyifeng@gmail.com> >> > VMware Issue: #2162866 >> > --- >> > Please backport this patch to upstream OVS down to 2.9, thanks. >> > >> > v1->v2: Added sanity check for device names, thanks Justin. >> > v2->v3: Fix an indent error. >> > v3->v4: Fix by code review, also fix a potenial bug. >> > >> > 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 | 7 +++---- >> > 4 files changed, 10 insertions(+), 10 deletions(-) >> > >> > diff --git a/datapath/linux/compat/ip6_gre.c >> b/datapath/linux/compat/ip6_gre.c >> > index 00dbefc9b099..182785273c6f 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"); >> > + strlcpy(name, "ovs-ip6gre%d", IFNAMSIZ); >> > >> > dev = alloc_netdev(sizeof(*t), name, NET_NAME_UNKNOWN, >> > ip6gre_tunnel_setup); >> > @@ -1712,7 +1712,8 @@ static int __net_init ip6gre_init_net(struct net >> *net) >> > struct ip6gre_net *ign = net_generic(net, ip6gre_net_id); >> > int err; >> > >> > - ign->fb_tunnel_dev = alloc_netdev(sizeof(struct ip6_tnl), >> "ip6gre0", >> > + ign->fb_tunnel_dev = alloc_netdev(sizeof(struct ip6_tnl), >> > + "ovs-ip6gre0", >> > NET_NAME_UNKNOWN, >> > ip6gre_tunnel_setup); >> > if (!ign->fb_tunnel_dev) { >> > diff --git a/datapath/linux/compat/ip6_tunnel.c >> b/datapath/linux/compat/ip6_tunnel.c >> > index 56fd8b4dd342..9f4bae7dd3d1 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"); >> > + strlcpy(name, "ovs-ip6tnl%d", IFNAMSIZ); >> > >> > dev = alloc_netdev(sizeof(*t), name, NET_NAME_UNKNOWN, >> > ip6_tnl_dev_setup); >> > @@ -1410,7 +1410,7 @@ ip6_tnl_parm_to_user(struct ip6_tnl_parm *u, >> const struct __ip6_tnl_parm *p) >> > * %SIOCCHGTUNNEL: change tunnel parameters to those given >> > * %SIOCDELTUNNEL: delete tunnel >> > * >> > - * The fallback device "ip6tnl0", created during module >> > + * The fallback device "ovs-ip6tnl0", created during module >> > * initialization, can be used for creating other tunnel devices. >> > * >> > * Return: >> > @@ -2093,7 +2093,7 @@ static int __net_init ip6_tnl_init_net(struct net >> *net) >> > ip6n->tnls[1] = ip6n->tnls_r_l; >> > >> > err = -ENOMEM; >> > - ip6n->fb_tnl_dev = alloc_netdev(sizeof(struct ip6_tnl), "ip6tnl0", >> > + ip6n->fb_tnl_dev = alloc_netdev(sizeof(struct ip6_tnl), >> "ovs-ip6tnl0", >> > NET_NAME_UNKNOWN, >> ip6_tnl_dev_setup); >> > >> > if (!ip6n->fb_tnl_dev) >> > diff --git a/datapath/linux/compat/ip_gre.c >> b/datapath/linux/compat/ip_gre.c >> > index 05132ba9494a..b7322c58e420 100644 >> > --- a/datapath/linux/compat/ip_gre.c >> > +++ b/datapath/linux/compat/ip_gre.c >> > @@ -1463,7 +1463,7 @@ static struct pernet_operations erspan_net_ops = { >> > >> > static int __net_init ipgre_tap_init_net(struct net *net) >> > { >> > - return ip_tunnel_init_net(net, gre_tap_net_id, &ipgre_tap_ops, >> "gretap0"); >> > + return ip_tunnel_init_net(net, gre_tap_net_id, &ipgre_tap_ops, >> "ovs-gretap0"); >> > } >> > >> > 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 d16e60fbfef0..ae5d5a058663 100644 >> > --- a/datapath/linux/compat/ip_tunnel.c >> > +++ b/datapath/linux/compat/ip_tunnel.c >> > @@ -130,12 +130,11 @@ static struct net_device >> *__ip_tunnel_create(struct net *net, >> > if (parms->name[0]) >> > strlcpy(name, parms->name, IFNAMSIZ); >> > else { >> > - if (strlen(ops->kind) > (IFNAMSIZ - 3)) { >> > + err = snprintf(name, IFNAMSIZ - 3, "ovs-%s%%d", >> ops->kind); >> > + if (err >= IFNAMSIZ - 3) >> > err = -E2BIG; >> > + if (err < 0) >> > goto failed; >> > - } >> > - strlcpy(name, ops->kind, IFNAMSIZ); >> > - strncat(name, "%d", 2); >> > } >> > >> > ASSERT_RTNL(); >> >> Thanks Yifeng for all your work on this. >> >> This looks OK but I'm not sure the additional protections against a >> device name being too long are >> really needed. I think in all cases the parms->name will be filled out >> since OVS is the only user. >> Have you tested whether the else code path will ever be entered? >> Sometimes kernel code gets >> into our backports even though it would never be executed within the >> context of OVS. It would >> be good to check this. >> >> Thanks, >> >> - Greg >> >
On 9/17/2018 1:40 PM, Yifeng Sun wrote: > Hi Greg, > > I checked that the 'else' part in the above patch is actually > executed. So it should be necessary to keep them. > > Thanks, > Yifeng That's interesting but I expect that it is only executed once during creation of the FB device. Does it continue to get invoked if you keep adding new tunnels? Can you try adding new ip gre tunnels and see if we would ever actually get to 100 of them? Enough to create a 3 digit trailing number? The reason I ask is that the kernel code is checking IFNAMSIZ - 3 and I think that check is sufficient because only the FB devices, of which there should only be one, will cause that block to be executed. I'm not sure though! If you could try that out I'd appreciate it but I do believe that for all non-FB devices cases the name of the tunnel will be in parms->name. Confirming that would tell us whether you need to make a patch for upstream or not. Thanks Yifeng! - Greg > > On Mon, Sep 17, 2018 at 9:51 AM Yifeng Sun <pkusunyifeng@gmail.com > <mailto:pkusunyifeng@gmail.com>> wrote: > > Hi Greg, > > Sure, thanks for the review. I will work on it. > > Yifeng > > On Mon, Sep 17, 2018 at 8:49 AM Gregory Rose <gvrose8192@gmail.com > <mailto:gvrose8192@gmail.com>> wrote: > > On 9/15/2018 9:24 PM, Yifeng Sun wrote: > > 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 <pkusunyifeng@gmail.com > <mailto:pkusunyifeng@gmail.com>> > > VMware Issue: #2162866 > > --- > > Please backport this patch to upstream OVS down to 2.9, thanks. > > > > v1->v2: Added sanity check for device names, thanks Justin. > > v2->v3: Fix an indent error. > > v3->v4: Fix by code review, also fix a potenial bug. > > > > 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 | 7 +++---- > > 4 files changed, 10 insertions(+), 10 deletions(-) > > > > diff --git a/datapath/linux/compat/ip6_gre.c > b/datapath/linux/compat/ip6_gre.c > > index 00dbefc9b099..182785273c6f 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"); > > + strlcpy(name, "ovs-ip6gre%d", IFNAMSIZ); > > > > dev = alloc_netdev(sizeof(*t), name, NET_NAME_UNKNOWN, > > ip6gre_tunnel_setup); > > @@ -1712,7 +1712,8 @@ static int __net_init > ip6gre_init_net(struct net *net) > > struct ip6gre_net *ign = net_generic(net, ip6gre_net_id); > > int err; > > > > - ign->fb_tunnel_dev = alloc_netdev(sizeof(struct > ip6_tnl), "ip6gre0", > > + ign->fb_tunnel_dev = alloc_netdev(sizeof(struct ip6_tnl), > > + "ovs-ip6gre0", > > NET_NAME_UNKNOWN, > > ip6gre_tunnel_setup); > > if (!ign->fb_tunnel_dev) { > > diff --git a/datapath/linux/compat/ip6_tunnel.c > b/datapath/linux/compat/ip6_tunnel.c > > index 56fd8b4dd342..9f4bae7dd3d1 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"); > > + strlcpy(name, "ovs-ip6tnl%d", IFNAMSIZ); > > > > dev = alloc_netdev(sizeof(*t), name, NET_NAME_UNKNOWN, > > ip6_tnl_dev_setup); > > @@ -1410,7 +1410,7 @@ ip6_tnl_parm_to_user(struct > ip6_tnl_parm *u, const struct __ip6_tnl_parm *p) > > * %SIOCCHGTUNNEL: change tunnel parameters to those given > > * %SIOCDELTUNNEL: delete tunnel > > * > > - * The fallback device "ip6tnl0", created during module > > + * The fallback device "ovs-ip6tnl0", created during module > > * initialization, can be used for creating other tunnel > devices. > > * > > * Return: > > @@ -2093,7 +2093,7 @@ static int __net_init > ip6_tnl_init_net(struct net *net) > > ip6n->tnls[1] = ip6n->tnls_r_l; > > > > err = -ENOMEM; > > - ip6n->fb_tnl_dev = alloc_netdev(sizeof(struct > ip6_tnl), "ip6tnl0", > > + ip6n->fb_tnl_dev = alloc_netdev(sizeof(struct > ip6_tnl), "ovs-ip6tnl0", > > NET_NAME_UNKNOWN, ip6_tnl_dev_setup); > > > > if (!ip6n->fb_tnl_dev) > > diff --git a/datapath/linux/compat/ip_gre.c > b/datapath/linux/compat/ip_gre.c > > index 05132ba9494a..b7322c58e420 100644 > > --- a/datapath/linux/compat/ip_gre.c > > +++ b/datapath/linux/compat/ip_gre.c > > @@ -1463,7 +1463,7 @@ static struct pernet_operations > erspan_net_ops = { > > > > static int __net_init ipgre_tap_init_net(struct net *net) > > { > > - return ip_tunnel_init_net(net, gre_tap_net_id, > &ipgre_tap_ops, "gretap0"); > > + return ip_tunnel_init_net(net, gre_tap_net_id, > &ipgre_tap_ops, "ovs-gretap0"); > > } > > > > 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 d16e60fbfef0..ae5d5a058663 100644 > > --- a/datapath/linux/compat/ip_tunnel.c > > +++ b/datapath/linux/compat/ip_tunnel.c > > @@ -130,12 +130,11 @@ static struct net_device > *__ip_tunnel_create(struct net *net, > > if (parms->name[0]) > > strlcpy(name, parms->name, IFNAMSIZ); > > else { > > - if (strlen(ops->kind) > (IFNAMSIZ - 3)) { > > + err = snprintf(name, IFNAMSIZ - 3, > "ovs-%s%%d", ops->kind); > > + if (err >= IFNAMSIZ - 3) > > err = -E2BIG; > > + if (err < 0) > > goto failed; > > - } > > - strlcpy(name, ops->kind, IFNAMSIZ); > > - strncat(name, "%d", 2); > > } > > > > ASSERT_RTNL(); > > Thanks Yifeng for all your work on this. > > This looks OK but I'm not sure the additional protections > against a > device name being too long are > really needed. I think in all cases the parms->name will be > filled out > since OVS is the only user. > Have you tested whether the else code path will ever be entered? > Sometimes kernel code gets > into our backports even though it would never be executed > within the > context of OVS. It would > be good to check this. > > Thanks, > > - Greg >
Hi Greg, Your guess is correct. The FB device is only created once. Even if more tunnels are added, there are still only one FB device. Yes, three digits should be enough in our case. Thanks, Yifeng On Mon, Sep 17, 2018 at 2:28 PM Gregory Rose <gvrose8192@gmail.com> wrote: > On 9/17/2018 1:40 PM, Yifeng Sun wrote: > > Hi Greg, > > I checked that the 'else' part in the above patch is actually > executed. So it should be necessary to keep them. > > Thanks, > Yifeng > > > That's interesting but I expect that it is only executed once during > creation of the FB device. Does it continue > to get invoked if you keep adding new tunnels? Can you try adding new ip > gre tunnels and see if we would ever actually get to 100 of them? Enough to > create a 3 digit trailing number? > > The reason I ask is that the kernel code is checking IFNAMSIZ - 3 and I > think that check is sufficient because > only the FB devices, of which there should only be one, will cause that > block to be executed. I'm not sure > though! If you could try that out I'd appreciate it but I do believe that > for all non-FB devices cases the name > of the tunnel will be in parms->name. Confirming that would tell us > whether you need to make a patch > for upstream or not. > > Thanks Yifeng! > > - Greg > > > On Mon, Sep 17, 2018 at 9:51 AM Yifeng Sun <pkusunyifeng@gmail.com> wrote: > >> Hi Greg, >> >> Sure, thanks for the review. I will work on it. >> >> Yifeng >> >> On Mon, Sep 17, 2018 at 8:49 AM Gregory Rose <gvrose8192@gmail.com> >> wrote: >> >>> On 9/15/2018 9:24 PM, Yifeng Sun wrote: >>> > 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 <pkusunyifeng@gmail.com> >>> > VMware Issue: #2162866 >>> > --- >>> > Please backport this patch to upstream OVS down to 2.9, thanks. >>> > >>> > v1->v2: Added sanity check for device names, thanks Justin. >>> > v2->v3: Fix an indent error. >>> > v3->v4: Fix by code review, also fix a potenial bug. >>> > >>> > 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 | 7 +++---- >>> > 4 files changed, 10 insertions(+), 10 deletions(-) >>> > >>> > diff --git a/datapath/linux/compat/ip6_gre.c >>> b/datapath/linux/compat/ip6_gre.c >>> > index 00dbefc9b099..182785273c6f 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"); >>> > + strlcpy(name, "ovs-ip6gre%d", IFNAMSIZ); >>> > >>> > dev = alloc_netdev(sizeof(*t), name, NET_NAME_UNKNOWN, >>> > ip6gre_tunnel_setup); >>> > @@ -1712,7 +1712,8 @@ static int __net_init ip6gre_init_net(struct net >>> *net) >>> > struct ip6gre_net *ign = net_generic(net, ip6gre_net_id); >>> > int err; >>> > >>> > - ign->fb_tunnel_dev = alloc_netdev(sizeof(struct ip6_tnl), >>> "ip6gre0", >>> > + ign->fb_tunnel_dev = alloc_netdev(sizeof(struct ip6_tnl), >>> > + "ovs-ip6gre0", >>> > NET_NAME_UNKNOWN, >>> > ip6gre_tunnel_setup); >>> > if (!ign->fb_tunnel_dev) { >>> > diff --git a/datapath/linux/compat/ip6_tunnel.c >>> b/datapath/linux/compat/ip6_tunnel.c >>> > index 56fd8b4dd342..9f4bae7dd3d1 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"); >>> > + strlcpy(name, "ovs-ip6tnl%d", IFNAMSIZ); >>> > >>> > dev = alloc_netdev(sizeof(*t), name, NET_NAME_UNKNOWN, >>> > ip6_tnl_dev_setup); >>> > @@ -1410,7 +1410,7 @@ ip6_tnl_parm_to_user(struct ip6_tnl_parm *u, >>> const struct __ip6_tnl_parm *p) >>> > * %SIOCCHGTUNNEL: change tunnel parameters to those given >>> > * %SIOCDELTUNNEL: delete tunnel >>> > * >>> > - * The fallback device "ip6tnl0", created during module >>> > + * The fallback device "ovs-ip6tnl0", created during module >>> > * initialization, can be used for creating other tunnel devices. >>> > * >>> > * Return: >>> > @@ -2093,7 +2093,7 @@ static int __net_init ip6_tnl_init_net(struct >>> net *net) >>> > ip6n->tnls[1] = ip6n->tnls_r_l; >>> > >>> > err = -ENOMEM; >>> > - ip6n->fb_tnl_dev = alloc_netdev(sizeof(struct ip6_tnl), >>> "ip6tnl0", >>> > + ip6n->fb_tnl_dev = alloc_netdev(sizeof(struct ip6_tnl), >>> "ovs-ip6tnl0", >>> > NET_NAME_UNKNOWN, >>> ip6_tnl_dev_setup); >>> > >>> > if (!ip6n->fb_tnl_dev) >>> > diff --git a/datapath/linux/compat/ip_gre.c >>> b/datapath/linux/compat/ip_gre.c >>> > index 05132ba9494a..b7322c58e420 100644 >>> > --- a/datapath/linux/compat/ip_gre.c >>> > +++ b/datapath/linux/compat/ip_gre.c >>> > @@ -1463,7 +1463,7 @@ static struct pernet_operations erspan_net_ops = >>> { >>> > >>> > static int __net_init ipgre_tap_init_net(struct net *net) >>> > { >>> > - return ip_tunnel_init_net(net, gre_tap_net_id, &ipgre_tap_ops, >>> "gretap0"); >>> > + return ip_tunnel_init_net(net, gre_tap_net_id, &ipgre_tap_ops, >>> "ovs-gretap0"); >>> > } >>> > >>> > 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 d16e60fbfef0..ae5d5a058663 100644 >>> > --- a/datapath/linux/compat/ip_tunnel.c >>> > +++ b/datapath/linux/compat/ip_tunnel.c >>> > @@ -130,12 +130,11 @@ static struct net_device >>> *__ip_tunnel_create(struct net *net, >>> > if (parms->name[0]) >>> > strlcpy(name, parms->name, IFNAMSIZ); >>> > else { >>> > - if (strlen(ops->kind) > (IFNAMSIZ - 3)) { >>> > + err = snprintf(name, IFNAMSIZ - 3, "ovs-%s%%d", >>> ops->kind); >>> > + if (err >= IFNAMSIZ - 3) >>> > err = -E2BIG; >>> > + if (err < 0) >>> > goto failed; >>> > - } >>> > - strlcpy(name, ops->kind, IFNAMSIZ); >>> > - strncat(name, "%d", 2); >>> > } >>> > >>> > ASSERT_RTNL(); >>> >>> Thanks Yifeng for all your work on this. >>> >>> This looks OK but I'm not sure the additional protections against a >>> device name being too long are >>> really needed. I think in all cases the parms->name will be filled out >>> since OVS is the only user. >>> Have you tested whether the else code path will ever be entered? >>> Sometimes kernel code gets >>> into our backports even though it would never be executed within the >>> context of OVS. It would >>> be good to check this. >>> >>> Thanks, >>> >>> - Greg >>> >> >
On 9/17/2018 4:34 PM, Yifeng Sun wrote: > Hi Greg, > > Your guess is correct. The FB device is only created once. > Even if more tunnels are added, there are still only one FB device. > Yes, three digits should be enough in our case. > > Thanks, > Yifeng Excellent! Thanks for confirming. I think if you just remove that section of the patch and resubmit it should be fine. I know you've done a lot of extra work testing this but I prefer staying in synch with upstream code as much as possible. It makes life easier down the road. Thanks again for your hard work, - Greg > > > On Mon, Sep 17, 2018 at 2:28 PM Gregory Rose <gvrose8192@gmail.com > <mailto:gvrose8192@gmail.com>> wrote: > > On 9/17/2018 1:40 PM, Yifeng Sun wrote: >> Hi Greg, >> >> I checked that the 'else' part in the above patch is actually >> executed. So it should be necessary to keep them. >> >> Thanks, >> Yifeng > > That's interesting but I expect that it is only executed once > during creation of the FB device. Does it continue > to get invoked if you keep adding new tunnels? Can you try adding > new ip gre tunnels and see if we would ever actually get to 100 of > them? Enough to create a 3 digit trailing number? > > The reason I ask is that the kernel code is checking IFNAMSIZ - 3 > and I think that check is sufficient because > only the FB devices, of which there should only be one, will cause > that block to be executed. I'm not sure > though! If you could try that out I'd appreciate it but I do > believe that for all non-FB devices cases the name > of the tunnel will be in parms->name. Confirming that would tell > us whether you need to make a patch > for upstream or not. > > Thanks Yifeng! > > - Greg > >> >> On Mon, Sep 17, 2018 at 9:51 AM Yifeng Sun >> <pkusunyifeng@gmail.com <mailto:pkusunyifeng@gmail.com>> wrote: >> >> Hi Greg, >> >> Sure, thanks for the review. I will work on it. >> >> Yifeng >> >> On Mon, Sep 17, 2018 at 8:49 AM Gregory Rose >> <gvrose8192@gmail.com <mailto:gvrose8192@gmail.com>> wrote: >> >> On 9/15/2018 9:24 PM, Yifeng Sun wrote: >> > 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 <pkusunyifeng@gmail.com >> <mailto:pkusunyifeng@gmail.com>> >> > VMware Issue: #2162866 >> > --- >> > Please backport this patch to upstream OVS down to 2.9, >> thanks. >> > >> > v1->v2: Added sanity check for device names, thanks Justin. >> > v2->v3: Fix an indent error. >> > v3->v4: Fix by code review, also fix a potenial bug. >> > >> > 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 | 7 +++---- >> > 4 files changed, 10 insertions(+), 10 deletions(-) >> > >> > diff --git a/datapath/linux/compat/ip6_gre.c >> b/datapath/linux/compat/ip6_gre.c >> > index 00dbefc9b099..182785273c6f 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"); >> > + strlcpy(name, "ovs-ip6gre%d", IFNAMSIZ); >> > >> > dev = alloc_netdev(sizeof(*t), name, >> NET_NAME_UNKNOWN, >> > ip6gre_tunnel_setup); >> > @@ -1712,7 +1712,8 @@ static int __net_init >> ip6gre_init_net(struct net *net) >> > struct ip6gre_net *ign = net_generic(net, >> ip6gre_net_id); >> > int err; >> > >> > - ign->fb_tunnel_dev = alloc_netdev(sizeof(struct >> ip6_tnl), "ip6gre0", >> > + ign->fb_tunnel_dev = alloc_netdev(sizeof(struct >> ip6_tnl), >> > + "ovs-ip6gre0", >> > NET_NAME_UNKNOWN, >> > ip6gre_tunnel_setup); >> > if (!ign->fb_tunnel_dev) { >> > diff --git a/datapath/linux/compat/ip6_tunnel.c >> b/datapath/linux/compat/ip6_tunnel.c >> > index 56fd8b4dd342..9f4bae7dd3d1 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"); >> > + strlcpy(name, "ovs-ip6tnl%d", IFNAMSIZ); >> > >> > dev = alloc_netdev(sizeof(*t), name, >> NET_NAME_UNKNOWN, >> > ip6_tnl_dev_setup); >> > @@ -1410,7 +1410,7 @@ ip6_tnl_parm_to_user(struct >> ip6_tnl_parm *u, const struct __ip6_tnl_parm *p) >> > * %SIOCCHGTUNNEL: change tunnel parameters to >> those given >> > * %SIOCDELTUNNEL: delete tunnel >> > * >> > - * The fallback device "ip6tnl0", created during module >> > + * The fallback device "ovs-ip6tnl0", created during >> module >> > * initialization, can be used for creating other >> tunnel devices. >> > * >> > * Return: >> > @@ -2093,7 +2093,7 @@ static int __net_init >> ip6_tnl_init_net(struct net *net) >> > ip6n->tnls[1] = ip6n->tnls_r_l; >> > >> > err = -ENOMEM; >> > - ip6n->fb_tnl_dev = alloc_netdev(sizeof(struct >> ip6_tnl), "ip6tnl0", >> > + ip6n->fb_tnl_dev = alloc_netdev(sizeof(struct >> ip6_tnl), "ovs-ip6tnl0", >> > NET_NAME_UNKNOWN, ip6_tnl_dev_setup); >> > >> > if (!ip6n->fb_tnl_dev) >> > diff --git a/datapath/linux/compat/ip_gre.c >> b/datapath/linux/compat/ip_gre.c >> > index 05132ba9494a..b7322c58e420 100644 >> > --- a/datapath/linux/compat/ip_gre.c >> > +++ b/datapath/linux/compat/ip_gre.c >> > @@ -1463,7 +1463,7 @@ static struct pernet_operations >> erspan_net_ops = { >> > >> > static int __net_init ipgre_tap_init_net(struct net *net) >> > { >> > - return ip_tunnel_init_net(net, gre_tap_net_id, >> &ipgre_tap_ops, "gretap0"); >> > + return ip_tunnel_init_net(net, gre_tap_net_id, >> &ipgre_tap_ops, "ovs-gretap0"); >> > } >> > >> > 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 d16e60fbfef0..ae5d5a058663 100644 >> > --- a/datapath/linux/compat/ip_tunnel.c >> > +++ b/datapath/linux/compat/ip_tunnel.c >> > @@ -130,12 +130,11 @@ static struct net_device >> *__ip_tunnel_create(struct net *net, >> > if (parms->name[0]) >> > strlcpy(name, parms->name, IFNAMSIZ); >> > else { >> > - if (strlen(ops->kind) > (IFNAMSIZ - 3)) { >> > + err = snprintf(name, IFNAMSIZ - 3, >> "ovs-%s%%d", ops->kind); >> > + if (err >= IFNAMSIZ - 3) >> > err = -E2BIG; >> > + if (err < 0) >> > goto failed; >> > - } >> > - strlcpy(name, ops->kind, IFNAMSIZ); >> > - strncat(name, "%d", 2); >> > } >> > >> > ASSERT_RTNL(); >> >> Thanks Yifeng for all your work on this. >> >> This looks OK but I'm not sure the additional protections >> against a >> device name being too long are >> really needed. I think in all cases the parms->name will >> be filled out >> since OVS is the only user. >> Have you tested whether the else code path will ever be >> entered? >> Sometimes kernel code gets >> into our backports even though it would never be executed >> within the >> context of OVS. It would >> be good to check this. >> >> Thanks, >> >> - Greg >> >
diff --git a/datapath/linux/compat/ip6_gre.c b/datapath/linux/compat/ip6_gre.c index 00dbefc9b099..182785273c6f 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"); + strlcpy(name, "ovs-ip6gre%d", IFNAMSIZ); dev = alloc_netdev(sizeof(*t), name, NET_NAME_UNKNOWN, ip6gre_tunnel_setup); @@ -1712,7 +1712,8 @@ static int __net_init ip6gre_init_net(struct net *net) struct ip6gre_net *ign = net_generic(net, ip6gre_net_id); int err; - ign->fb_tunnel_dev = alloc_netdev(sizeof(struct ip6_tnl), "ip6gre0", + ign->fb_tunnel_dev = alloc_netdev(sizeof(struct ip6_tnl), + "ovs-ip6gre0", NET_NAME_UNKNOWN, ip6gre_tunnel_setup); if (!ign->fb_tunnel_dev) { diff --git a/datapath/linux/compat/ip6_tunnel.c b/datapath/linux/compat/ip6_tunnel.c index 56fd8b4dd342..9f4bae7dd3d1 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"); + strlcpy(name, "ovs-ip6tnl%d", IFNAMSIZ); dev = alloc_netdev(sizeof(*t), name, NET_NAME_UNKNOWN, ip6_tnl_dev_setup); @@ -1410,7 +1410,7 @@ ip6_tnl_parm_to_user(struct ip6_tnl_parm *u, const struct __ip6_tnl_parm *p) * %SIOCCHGTUNNEL: change tunnel parameters to those given * %SIOCDELTUNNEL: delete tunnel * - * The fallback device "ip6tnl0", created during module + * The fallback device "ovs-ip6tnl0", created during module * initialization, can be used for creating other tunnel devices. * * Return: @@ -2093,7 +2093,7 @@ static int __net_init ip6_tnl_init_net(struct net *net) ip6n->tnls[1] = ip6n->tnls_r_l; err = -ENOMEM; - ip6n->fb_tnl_dev = alloc_netdev(sizeof(struct ip6_tnl), "ip6tnl0", + ip6n->fb_tnl_dev = alloc_netdev(sizeof(struct ip6_tnl), "ovs-ip6tnl0", NET_NAME_UNKNOWN, ip6_tnl_dev_setup); if (!ip6n->fb_tnl_dev) diff --git a/datapath/linux/compat/ip_gre.c b/datapath/linux/compat/ip_gre.c index 05132ba9494a..b7322c58e420 100644 --- a/datapath/linux/compat/ip_gre.c +++ b/datapath/linux/compat/ip_gre.c @@ -1463,7 +1463,7 @@ static struct pernet_operations erspan_net_ops = { static int __net_init ipgre_tap_init_net(struct net *net) { - return ip_tunnel_init_net(net, gre_tap_net_id, &ipgre_tap_ops, "gretap0"); + return ip_tunnel_init_net(net, gre_tap_net_id, &ipgre_tap_ops, "ovs-gretap0"); } 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 d16e60fbfef0..ae5d5a058663 100644 --- a/datapath/linux/compat/ip_tunnel.c +++ b/datapath/linux/compat/ip_tunnel.c @@ -130,12 +130,11 @@ static struct net_device *__ip_tunnel_create(struct net *net, if (parms->name[0]) strlcpy(name, parms->name, IFNAMSIZ); else { - if (strlen(ops->kind) > (IFNAMSIZ - 3)) { + err = snprintf(name, IFNAMSIZ - 3, "ovs-%s%%d", ops->kind); + if (err >= IFNAMSIZ - 3) err = -E2BIG; + if (err < 0) goto failed; - } - strlcpy(name, ops->kind, IFNAMSIZ); - strncat(name, "%d", 2); } ASSERT_RTNL();
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 <pkusunyifeng@gmail.com> VMware Issue: #2162866 --- Please backport this patch to upstream OVS down to 2.9, thanks. v1->v2: Added sanity check for device names, thanks Justin. v2->v3: Fix an indent error. v3->v4: Fix by code review, also fix a potenial bug. 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 | 7 +++---- 4 files changed, 10 insertions(+), 10 deletions(-)