| 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 |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
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;
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 --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;