diff mbox series

[ovs-dev,v1] netdev-vport : Fix userspace tunnel ioctl(SIOCGIFINDEX) info logs.

Message ID MEYP282MB3302248EC2991B23E20C5E42CDAD9@MEYP282MB3302.AUSP282.PROD.OUTLOOK.COM
State Superseded
Headers show
Series [ovs-dev,v1] netdev-vport : Fix userspace tunnel ioctl(SIOCGIFINDEX) info logs. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

lin huang Oct. 3, 2021, 9:34 a.m. UTC
From: linhuang <linhuang@ruijie.com.cn>

Userspace tunnel doesn't have a valid device in the kernel. So
get_ifindex() function (ioctl) always get error during
adding a port, deleting a port or updating a port status.

The info log is
"2021-08-29T09:17:39.830Z|00059|netdev_linux|INFO|ioctl(SIOCGIFINDEX)
on vxlan_sys_4789 device failed: No such device"

If there are a lot of userspace tunnel ports on a bridge, the
iface_refresh_netdev_status() function will spend a lot of time.

So ignore userspace tunnel port ioctl(SIOCGIFINDEX) operation, just
return ifindex=0.

Signed-off-by: linhuang <linhuang@ruijie.com.cn>
---
 lib/netdev-offload.c | 6 ++++--
 lib/netdev-vport.c   | 3 ++-
 vswitchd/bridge.c    | 2 ++
 3 files changed, 8 insertions(+), 3 deletions(-)

Comments

Aaron Conole Oct. 20, 2021, 3:15 p.m. UTC | #1
Hi Lin,

lin huang <miterv@outlook.com> writes:

> From: linhuang <linhuang@ruijie.com.cn>
>
> Userspace tunnel doesn't have a valid device in the kernel. So
> get_ifindex() function (ioctl) always get error during
> adding a port, deleting a port or updating a port status.
>
> The info log is
> "2021-08-29T09:17:39.830Z|00059|netdev_linux|INFO|ioctl(SIOCGIFINDEX)
> on vxlan_sys_4789 device failed: No such device"
>
> If there are a lot of userspace tunnel ports on a bridge, the
> iface_refresh_netdev_status() function will spend a lot of time.
>
> So ignore userspace tunnel port ioctl(SIOCGIFINDEX) operation, just
> return ifindex=0.
>
> Signed-off-by: linhuang <linhuang@ruijie.com.cn>
> ---

Thanks for the patch - I think the commit says "return ifindex=0" but I
guess it should really say "return -ENODEV"?

Otherwise, it looks good to me.

>  lib/netdev-offload.c | 6 ++++--
>  lib/netdev-vport.c   | 3 ++-
>  vswitchd/bridge.c    | 2 ++
>  3 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c
> index 8075cfbd8..00b7515cf 100644
> --- a/lib/netdev-offload.c
> +++ b/lib/netdev-offload.c
> @@ -577,7 +577,9 @@ netdev_ports_insert(struct netdev *netdev, const char *dpif_type,
>                      struct dpif_port *dpif_port)
>  {
>      struct port_to_netdev_data *data;
> -    int ifindex = netdev_get_ifindex(netdev);
> +    int ifindex;
> +
> +    netdev_set_dpif_type(netdev, dpif_type);
>  
>      ovs_rwlock_wrlock(&netdev_hmap_rwlock);
>      if (netdev_ports_lookup(dpif_port->port_no, dpif_type)) {
> @@ -589,6 +591,7 @@ netdev_ports_insert(struct netdev *netdev, const char *dpif_type,
>      data->netdev = netdev_ref(netdev);
>      dpif_port_clone(&data->dpif_port, dpif_port);
>  
> +    ifindex = netdev_get_ifindex(netdev);
>      if (ifindex >= 0) {
>          data->ifindex = ifindex;
>          hmap_insert(&ifindex_to_port, &data->ifindex_node, ifindex);
> @@ -596,7 +599,6 @@ netdev_ports_insert(struct netdev *netdev, const char *dpif_type,
>          data->ifindex = -1;
>      }
>  
> -    netdev_set_dpif_type(netdev, dpif_type);
>  
>      hmap_insert(&port_to_netdev, &data->portno_node,
>                  netdev_ports_hash(dpif_port->port_no, dpif_type));
> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
> index 499c0291c..411ac343a 100644
> --- a/lib/netdev-vport.c
> +++ b/lib/netdev-vport.c
> @@ -1151,8 +1151,9 @@ netdev_vport_get_ifindex(const struct netdev *netdev_)
>  {
>      char buf[NETDEV_VPORT_NAME_BUFSIZE];
>      const char *name = netdev_vport_get_dpif_port(netdev_, buf, sizeof(buf));
> +    const char *type = netdev_get_dpif_type(netdev_);
>  
> -    return linux_get_ifindex(name);
> +    return (strcmp(type, "netdev")) ? linux_get_ifindex(name) : -ENODEV;
>  }
>  
>  #define NETDEV_VPORT_GET_IFINDEX netdev_vport_get_ifindex
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index c790a56ad..473d79acd 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -2052,6 +2052,8 @@ iface_do_create(const struct bridge *br,
>          goto error;
>      }
>  
> +    netdev_set_dpif_type(netdev, br->type);
> +
>      error = iface_set_netdev_config(iface_cfg, netdev, errp);
>      if (error) {
>          goto error;
lin huang Oct. 25, 2021, 4:56 a.m. UTC | #2
Hi Aaron,
    Thanks for review.

> Thanks for the patch - I think the commit says "return ifindex=0" but I guess it
> should really say "return -ENODEV"?

     Yes, that is "return -ENODEV" not "return ifindex=0".

> -----Original Message-----
> From: Aaron Conole [mailto:aconole@redhat.com]
> Sent: Wednesday, October 20, 2021 11:15 PM
> To: lin huang
> Cc: dev@openvswitch.org; david.marchand@redhat.com;
> i.maximets@ovn.org
> Subject: Re: [ovs-dev] [PATCH v1] netdev-vport : Fix userspace tunnel
> ioctl(SIOCGIFINDEX) info logs.
> 
> Hi Lin,
> 
> lin huang <miterv@outlook.com> writes:
> 
> > From: linhuang <linhuang@ruijie.com.cn>
> >
> > Userspace tunnel doesn't have a valid device in the kernel. So
> > get_ifindex() function (ioctl) always get error during adding a port,
> > deleting a port or updating a port status.
> >
> > The info log is
> > "2021-08-29T09:17:39.830Z|00059|netdev_linux|INFO|ioctl(SIOCGIFINDEX)
> > on vxlan_sys_4789 device failed: No such device"
> >
> > If there are a lot of userspace tunnel ports on a bridge, the
> > iface_refresh_netdev_status() function will spend a lot of time.
> >
> > So ignore userspace tunnel port ioctl(SIOCGIFINDEX) operation, just
> > return ifindex=0.
> >
> > Signed-off-by: linhuang <linhuang@ruijie.com.cn>
> > ---
> 
> Thanks for the patch - I think the commit says "return ifindex=0" but I guess it
> should really say "return -ENODEV"?
> 
> Otherwise, it looks good to me.
> 
> >  lib/netdev-offload.c | 6 ++++--
> >  lib/netdev-vport.c   | 3 ++-
> >  vswitchd/bridge.c    | 2 ++
> >  3 files changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c index
> > 8075cfbd8..00b7515cf 100644
> > --- a/lib/netdev-offload.c
> > +++ b/lib/netdev-offload.c
> > @@ -577,7 +577,9 @@ netdev_ports_insert(struct netdev *netdev, const
> char *dpif_type,
> >                      struct dpif_port *dpif_port)  {
> >      struct port_to_netdev_data *data;
> > -    int ifindex = netdev_get_ifindex(netdev);
> > +    int ifindex;
> > +
> > +    netdev_set_dpif_type(netdev, dpif_type);
> >
> >      ovs_rwlock_wrlock(&netdev_hmap_rwlock);
> >      if (netdev_ports_lookup(dpif_port->port_no, dpif_type)) { @@
> > -589,6 +591,7 @@ netdev_ports_insert(struct netdev *netdev, const char
> *dpif_type,
> >      data->netdev = netdev_ref(netdev);
> >      dpif_port_clone(&data->dpif_port, dpif_port);
> >
> > +    ifindex = netdev_get_ifindex(netdev);
> >      if (ifindex >= 0) {
> >          data->ifindex = ifindex;
> >          hmap_insert(&ifindex_to_port, &data->ifindex_node, ifindex);
> > @@ -596,7 +599,6 @@ netdev_ports_insert(struct netdev *netdev, const
> char *dpif_type,
> >          data->ifindex = -1;
> >      }
> >
> > -    netdev_set_dpif_type(netdev, dpif_type);
> >
> >      hmap_insert(&port_to_netdev, &data->portno_node,
> >                  netdev_ports_hash(dpif_port->port_no, dpif_type));
> > diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index
> > 499c0291c..411ac343a 100644
> > --- a/lib/netdev-vport.c
> > +++ b/lib/netdev-vport.c
> > @@ -1151,8 +1151,9 @@ netdev_vport_get_ifindex(const struct netdev
> > *netdev_)  {
> >      char buf[NETDEV_VPORT_NAME_BUFSIZE];
> >      const char *name = netdev_vport_get_dpif_port(netdev_, buf,
> > sizeof(buf));
> > +    const char *type = netdev_get_dpif_type(netdev_);
> >
> > -    return linux_get_ifindex(name);
> > +    return (strcmp(type, "netdev")) ? linux_get_ifindex(name) :
> > + -ENODEV;
> >  }
> >
> >  #define NETDEV_VPORT_GET_IFINDEX netdev_vport_get_ifindex diff --
> git
> > a/vswitchd/bridge.c b/vswitchd/bridge.c index c790a56ad..473d79acd
> > 100644
> > --- a/vswitchd/bridge.c
> > +++ b/vswitchd/bridge.c
> > @@ -2052,6 +2052,8 @@ iface_do_create(const struct bridge *br,
> >          goto error;
> >      }
> >
> > +    netdev_set_dpif_type(netdev, br->type);
> > +
> >      error = iface_set_netdev_config(iface_cfg, netdev, errp);
> >      if (error) {
> >          goto error;
diff mbox series

Patch

diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c
index 8075cfbd8..00b7515cf 100644
--- a/lib/netdev-offload.c
+++ b/lib/netdev-offload.c
@@ -577,7 +577,9 @@  netdev_ports_insert(struct netdev *netdev, const char *dpif_type,
                     struct dpif_port *dpif_port)
 {
     struct port_to_netdev_data *data;
-    int ifindex = netdev_get_ifindex(netdev);
+    int ifindex;
+
+    netdev_set_dpif_type(netdev, dpif_type);
 
     ovs_rwlock_wrlock(&netdev_hmap_rwlock);
     if (netdev_ports_lookup(dpif_port->port_no, dpif_type)) {
@@ -589,6 +591,7 @@  netdev_ports_insert(struct netdev *netdev, const char *dpif_type,
     data->netdev = netdev_ref(netdev);
     dpif_port_clone(&data->dpif_port, dpif_port);
 
+    ifindex = netdev_get_ifindex(netdev);
     if (ifindex >= 0) {
         data->ifindex = ifindex;
         hmap_insert(&ifindex_to_port, &data->ifindex_node, ifindex);
@@ -596,7 +599,6 @@  netdev_ports_insert(struct netdev *netdev, const char *dpif_type,
         data->ifindex = -1;
     }
 
-    netdev_set_dpif_type(netdev, dpif_type);
 
     hmap_insert(&port_to_netdev, &data->portno_node,
                 netdev_ports_hash(dpif_port->port_no, dpif_type));
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 499c0291c..411ac343a 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -1151,8 +1151,9 @@  netdev_vport_get_ifindex(const struct netdev *netdev_)
 {
     char buf[NETDEV_VPORT_NAME_BUFSIZE];
     const char *name = netdev_vport_get_dpif_port(netdev_, buf, sizeof(buf));
+    const char *type = netdev_get_dpif_type(netdev_);
 
-    return linux_get_ifindex(name);
+    return (strcmp(type, "netdev")) ? linux_get_ifindex(name) : -ENODEV;
 }
 
 #define NETDEV_VPORT_GET_IFINDEX netdev_vport_get_ifindex
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index c790a56ad..473d79acd 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -2052,6 +2052,8 @@  iface_do_create(const struct bridge *br,
         goto error;
     }
 
+    netdev_set_dpif_type(netdev, br->type);
+
     error = iface_set_netdev_config(iface_cfg, netdev, errp);
     if (error) {
         goto error;