Re: [ovs-dev] 回复: [PATCH v2] netdev-vport : Fix userspace tunnel ioctl(SIOCGIFINDEX) info logs.

2021-11-22 Thread Mike Pattrick 
Hello linhuang,

This is a good idea, but I've run into a few issues with this patch.

There's still one instance of "type" that needs to be changed to
dpif_type to fix compile.

Also, the behaviour of strcmp is undefined if one of the pointers is
NULL. On my system, it throws SIGSEGV. This causes a bunch of the tests
to fail.

Somethign like:
if (dpif_type != NULL && !strcmp(dpif_type, "system")) {
return linux_get_ifindex(name);
} else {
return -ENODEV;
}

However, even with this change, I'm still getting some test failures.

Cheers,
MKP

On Sat, 2021-11-20 at 13:12 +, lin huang wrote:
> From: linhuang 
> 
> 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 -ENODEV.
> 
> Signed-off-by: linhuang 
> Reviewed-by: Aaron Conole 
> ---
>  lib/dpif.c   | 2 +-
>  lib/netdev-offload.c | 2 --
>  lib/netdev-vport.c   | 3 ++-
>  3 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/dpif.c b/lib/dpif.c
> index 8c4aed4..7adb620 100644
> --- a/lib/dpif.c
> +++ b/lib/dpif.c
> @@ -362,8 +362,8 @@ do_open(const char *name, const char *type, bool create, 
> struct dpif **dpifp)
>  }
> 
>  err = netdev_open(dpif_port.name, dpif_port.type, &netdev);
> -
>  if (!err) {
> +netdev_set_dpif_type(netdev, dpif_type_str);
>  netdev_ports_insert(netdev, dpif_type_str, &dpif_port);
>  netdev_close(netdev);
>  } else {
> diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c
> index 8075cfb..1221170 100644
> --- a/lib/netdev-offload.c
> +++ b/lib/netdev-offload.c
> @@ -596,8 +596,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));
>  ovs_rwlock_unlock(&netdev_hmap_rwlock);
> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
> index 499c029..ad24933 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 *dpif_type = netdev_get_dpif_type(netdev_);
> 
> -return linux_get_ifindex(name);
> +return !strcmp(type, "system") ? linux_get_ifindex(name) : -ENODEV;
>  }
> 
>  #define NETDEV_VPORT_GET_IFINDEX netdev_vport_get_ifindex
> --
> 2.12.2
> 
> 
> 发件人: dev  代表 lin huang 
> 发送时间: 2021年11月17日 12:39
> 收件人: dev
> 抄送: Ben Pfaff; i.maximets
> 主题: Re: [ovs-dev] [PATCH v2] netdev-vport : Fix userspace tunnel 
> ioctl(SIOCGIFINDEX) info logs.
> 
> hi all,
> 
> pls review my code.
> 
> Regards,
> Lin Huang
> 
> From: lin huang
> Date: 2021-10-25 13:16
> To: d...@openvswitch.org
> CC: Aaron Conole; 
> i.maxim...@ovn.org
> Subject: [PATCH v2] netdev-vport : Fix userspace tunnel ioctl(SIOCGIFINDEX) 
> info logs.
> From: linhuang 
> 
> 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 -ENODEV.
> 
> Signed-off-by: linhuang 
> Reviewed-by: Aaron Conole 
> ---
> 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

Re: [ovs-dev] 回复: [PATCH v2] netdev-vport : Fix userspace tunnel ioctl(SIOCGIFINDEX) info logs.

2021-11-20 Thread 0-day Robot 
Bleep bloop.  Greetings lin huang, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


git-am:
error: cannot convert from eucgb2312_cn to UTF-8
fatal: could not parse patch


Patch skipped due to previous failure.

Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] 回复: [PATCH v2] netdev-vport : Fix userspace tunnel ioctl(SIOCGIFINDEX) info logs.

2021-11-20 Thread lin huang 
From: linhuang 

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 -ENODEV.

Signed-off-by: linhuang 
Reviewed-by: Aaron Conole 
---
 lib/dpif.c   | 2 +-
 lib/netdev-offload.c | 2 --
 lib/netdev-vport.c   | 3 ++-
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/lib/dpif.c b/lib/dpif.c
index 8c4aed4..7adb620 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -362,8 +362,8 @@ do_open(const char *name, const char *type, bool create, 
struct dpif **dpifp)
 }

 err = netdev_open(dpif_port.name, dpif_port.type, &netdev);
-
 if (!err) {
+netdev_set_dpif_type(netdev, dpif_type_str);
 netdev_ports_insert(netdev, dpif_type_str, &dpif_port);
 netdev_close(netdev);
 } else {
diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c
index 8075cfb..1221170 100644
--- a/lib/netdev-offload.c
+++ b/lib/netdev-offload.c
@@ -596,8 +596,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));
 ovs_rwlock_unlock(&netdev_hmap_rwlock);
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 499c029..ad24933 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 *dpif_type = netdev_get_dpif_type(netdev_);

-return linux_get_ifindex(name);
+return !strcmp(type, "system") ? linux_get_ifindex(name) : -ENODEV;
 }

 #define NETDEV_VPORT_GET_IFINDEX netdev_vport_get_ifindex
--
2.12.2


发件人: dev  代表 lin huang 
发送时间: 2021年11月17日 12:39
收件人: dev
抄送: Ben Pfaff; i.maximets
主题: Re: [ovs-dev] [PATCH v2] netdev-vport : Fix userspace tunnel 
ioctl(SIOCGIFINDEX) info logs.

hi all,

pls review my code.

Regards,
Lin Huang

From: lin huang
Date: 2021-10-25 13:16
To: d...@openvswitch.org
CC: Aaron Conole; 
i.maxim...@ovn.org
Subject: [PATCH v2] netdev-vport : Fix userspace tunnel ioctl(SIOCGIFINDEX) 
info logs.
From: linhuang 

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 -ENODEV.

Signed-off-by: linhuang 
Reviewed-by: Aaron Conole 
---
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 struc