Received-SPF: pass (google.com: domain of linux-kernel+bounces-146700-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: Eelco Chaudron <echaudro@redhat.com>
To: "jun.gu" <jun.gu@easystack.cn>
Cc: dev@openvswitch.org, linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
 pshelar@ovn.org
Subject: Re: [PATCH v2] net: openvswitch: Check vport net device name
Date: Tue, 16 Apr 2024 13:19:00 +0200
Message-ID: <27644212-924C-4AB2-88F6-D209E2586E82@redhat.com>
In-Reply-To: <20240416092022.35887-1-jun.gu@easystack.cn>
References: <084E7217-6290-46D2-A47A-14ACB60EBBCA@redhat.com>
 <20240416092022.35887-1-jun.gu@easystack.cn>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable


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 ne=
t
> 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=E2=
=80=99ll 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-net=
dev.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, co=
nst char *name)
>  	int err;
>
>  	vport->dev =3D 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 =3D -ENODEV;
>  		goto error_free_vport;
>  	}
> -- =

> 2.25.1