Check vport net device name to avoid the name that be used to query is
inconsistent the retured name.

Consider net device supports alias, the alias can be set to interface
table in ovs userspace. Consider the following process:
- set a net device alias to interface table.
- ovs userspace run OVS_VPORT_CMD_NEW cmd to kernel, kernel will use net
device alias to query net device by dev_get_by_name, but the net device
name that return is inconsistent the name used to query.
- the returned net device name is saved a hash table.
- ovs userspace found that the name saved to interface table is
inconsistent the name saved kernel hash table, it will run
OVS_VPORT_CMD_DEL cmd to kernel and remove vport.

ovs userspace will run OVS_VPORT_CMD_NEW and OVS_VPORT_CMD_DEL cmd
repeatedly. So the patch will check vport net device name from
dev_get_by_name to avoid the above issue.

Signed-off-by: Jun Gu <jun.gu@easystack.cn>
---
 net/openvswitch/vport-netdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/openvswitch/vport-netdev.c b/net/openvswitch/vport-netdev.c
index 903537a5da22..de8977d7f329 100644
--- a/net/openvswitch/vport-netdev.c
+++ b/net/openvswitch/vport-netdev.c
@@ -78,7 +78,7 @@ struct vport *ovs_netdev_link(struct vport *vport, const char *name)
 	int err;
 
 	vport->dev = dev_get_by_name(ovs_dp_get_net(vport->dp), name);
-	if (!vport->dev) {
+	if (!vport->dev) || strcmp(name, ovs_vport_name(vport)) {
 		err = -ENODEV;
 		goto error_free_vport;
 	}
-- 
2.25.1

On 16 Apr 2024, at 11:20, jun.gu wrote:

> Check vport net device name to avoid the name that be used to query is
> inconsistent the retured name.
>
> Consider net device supports alias, the alias can be set to interface
> table in ovs userspace. Consider the following process:
> - set a net device alias to interface table.
> - ovs userspace run OVS_VPORT_CMD_NEW cmd to kernel, kernel will use net
> device alias to query net device by dev_get_by_name, but the net device
> name that return is inconsistent the name used to query.
> - the returned net device name is saved a hash table.
> - ovs userspace found that the name saved to interface table is
> inconsistent the name saved kernel hash table, it will run
> OVS_VPORT_CMD_DEL cmd to kernel and remove vport.
>
> ovs userspace will run OVS_VPORT_CMD_NEW and OVS_VPORT_CMD_DEL cmd
> repeatedly. So the patch will check vport net device name from
> dev_get_by_name to avoid the above issue.

Maybe the commit message needs a rewrite to be more clear (shorter)? I’ll leave this up to the maintainers to judge.

> Signed-off-by: Jun Gu <jun.gu@easystack.cn>
> ---
>  net/openvswitch/vport-netdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/openvswitch/vport-netdev.c b/net/openvswitch/vport-netdev.c
> index 903537a5da22..de8977d7f329 100644
> --- a/net/openvswitch/vport-netdev.c
> +++ b/net/openvswitch/vport-netdev.c
> @@ -78,7 +78,7 @@ struct vport *ovs_netdev_link(struct vport *vport, const char *name)
>  	int err;
>
>  	vport->dev = dev_get_by_name(ovs_dp_get_net(vport->dp), name);

I was eluding to a comment here, something like:

        /* Ensure that the device exists and that the provided
         * name is not one of its aliases.
         */

> -	if (!vport->dev) {
> +	if (!vport->dev) || strcmp(name, ovs_vport_name(vport)) {
>  		err = -ENODEV;
>  		goto error_free_vport;
>  	}
> -- 
> 2.25.1

Check vport net device name to ensure the provided name is not one of
its aliases. This can avoid that ovs userspace create and destroy vport
repeatedly.

Signed-off-by: Jun Gu <jun.gu@easystack.cn>
---
 net/openvswitch/vport-netdev.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/openvswitch/vport-netdev.c b/net/openvswitch/vport-netdev.c
index 903537a5da22..7003e76b8172 100644
--- a/net/openvswitch/vport-netdev.c
+++ b/net/openvswitch/vport-netdev.c
@@ -78,7 +78,10 @@ struct vport *ovs_netdev_link(struct vport *vport, const char *name)
 	int err;
 
 	vport->dev = dev_get_by_name(ovs_dp_get_net(vport->dp), name);
-	if (!vport->dev) {
+	/* Ensure that the device exists and that the provided
+	 * name is not one of its aliases.
+	 */
+	if ((!vport->dev) || strcmp(name, ovs_vport_name(vport))) {
 		err = -ENODEV;
 		goto error_free_vport;
 	}
-- 
2.25.1

On 17 Apr 2024, at 6:20, jun.gu wrote:

> Check vport net device name to ensure the provided name is not one of
> its aliases. This can avoid that ovs userspace create and destroy vport
> repeatedly.

How about changing the text to the following?

  Ensure that the provided netdev name is not one of its aliases to prevent unnecessary creation and destruction of the vport by ovs-vswitchd.

Maybe the maintainers are fine with the current text, in that case:

Acked-by: Eelco Chaudron <echaudro@redhat.com>

>
> Signed-off-by: Jun Gu <jun.gu@easystack.cn>
> ---
>  net/openvswitch/vport-netdev.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/net/openvswitch/vport-netdev.c b/net/openvswitch/vport-netdev.c
> index 903537a5da22..7003e76b8172 100644
> --- a/net/openvswitch/vport-netdev.c
> +++ b/net/openvswitch/vport-netdev.c
> @@ -78,7 +78,10 @@ struct vport *ovs_netdev_link(struct vport *vport, const char *name)
>  	int err;
>
>  	vport->dev = dev_get_by_name(ovs_dp_get_net(vport->dp), name);
> -	if (!vport->dev) {
> +	/* Ensure that the device exists and that the provided
> +	 * name is not one of its aliases.
> +	 */
> +	if ((!vport->dev) || strcmp(name, ovs_vport_name(vport))) {
>  		err = -ENODEV;
>  		goto error_free_vport;
>  	}
> -- 
> 2.25.1

Ensure that the provided netdev name is not one of its aliases to
prevent unnecessary creation and destruction of the vport by
ovs-vswitchd.

Signed-off-by: Jun Gu <jun.gu@easystack.cn>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
---
 net/openvswitch/vport-netdev.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/openvswitch/vport-netdev.c b/net/openvswitch/vport-netdev.c
index 903537a5da22..7003e76b8172 100644
--- a/net/openvswitch/vport-netdev.c
+++ b/net/openvswitch/vport-netdev.c
@@ -78,7 +78,10 @@ struct vport *ovs_netdev_link(struct vport *vport, const char *name)
 	int err;
 
 	vport->dev = dev_get_by_name(ovs_dp_get_net(vport->dp), name);
-	if (!vport->dev) {
+	/* Ensure that the device exists and that the provided
+	 * name is not one of its aliases.
+	 */
+	if ((!vport->dev) || strcmp(name, ovs_vport_name(vport))) {
 		err = -ENODEV;
 		goto error_free_vport;
 	}
-- 
2.25.1

On Thu, 18 Apr 2024 10:32:42 +0800 jun.gu wrote:
> +	if ((!vport->dev) || strcmp(name, ovs_vport_name(vport))) {

Please drop the unnecessary brackets.
When you repost, start a new thread, do not post new version
in-reply-to.
-- 
pw-bot: cr

From: "jun.gu" <jun.gu@easystack.cn>

Ensure that the provided netdev name is not one of its aliases to
prevent unnecessary creation and destruction of the vport by
ovs-vswitchd.

Signed-off-by: jun.gu  <jun.gu@easystack.cn>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
---
 net/openvswitch/vport-netdev.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/openvswitch/vport-netdev.c b/net/openvswitch/vport-netdev.c
index 903537a5da22..618edc346c0f 100644
--- a/net/openvswitch/vport-netdev.c
+++ b/net/openvswitch/vport-netdev.c
@@ -78,7 +78,10 @@ struct vport *ovs_netdev_link(struct vport *vport, const char *name)
 	int err;
 
 	vport->dev = dev_get_by_name(ovs_dp_get_net(vport->dp), name);
-	if (!vport->dev) {
+	/* Ensure that the device exists and that the provided
+	 * name is not one of its aliases.
+	 */
+	if (!vport->dev || strcmp(name, ovs_vport_name(vport))) {
 		err = -ENODEV;
 		goto error_free_vport;
 	}
-- 
2.25.1

On Fri, 19 Apr 2024 12:31:33 +0800 Jun Gu wrote:
> From: "jun.gu" <jun.gu@easystack.cn>
> 
> Ensure that the provided netdev name is not one of its aliases to
> prevent unnecessary creation and destruction of the vport by
> ovs-vswitchd.
> 
> Signed-off-by: jun.gu  <jun.gu@easystack.cn>
> Acked-by: Eelco Chaudron <echaudro@redhat.com>

I said: When you repost, start a new thread, do not post new version
in-reply-to.

If you don't understand what something means - ask :|
Now try again.
-- 
pw-bot: cr