| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <DM8PR12MB540082219FBD835F40AB999AABA29@DM8PR12MB5400.namprd12.prod.outlook.com>
Date: Mon, 6 Jun 2022 05:11:43 +0000
From: Eli Cohen <elic@...dia.com>
To: Gautam Dawar <gautam.dawar@....com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC: "linux-net-drivers@....com" <linux-net-drivers@....com>,
"hanand@....com" <hanand@....com>,
"Michael S. Tsirkin" <mst@...hat.com>,
Jason Wang <jasowang@...hat.com>,
Zhu Lingshan <lingshan.zhu@...el.com>,
Xie Yongji <xieyongji@...edance.com>,
Parav Pandit <parav@...dia.com>,
Si-Wei Liu <si-wei.liu@...cle.com>,
Stefano Garzarella <sgarzare@...hat.com>,
Wan Jiabing <wanjiabing@...o.com>,
Dan Carpenter <dan.carpenter@...cle.com>,
"virtualization@...ts.linux-foundation.org"
<virtualization@...ts.linux-foundation.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] vdpa: allow vdpa dev_del management operation to return
failure
> From: Gautam Dawar <gautam.dawar@....com>
> Sent: Friday, June 3, 2022 1:34 PM
> To: netdev@...r.kernel.org
> Cc: linux-net-drivers@....com; hanand@....com; Gautam Dawar <gautam.dawar@....com>; Michael S. Tsirkin
> <mst@...hat.com>; Jason Wang <jasowang@...hat.com>; Zhu Lingshan <lingshan.zhu@...el.com>; Xie Yongji
> <xieyongji@...edance.com>; Eli Cohen <elic@...dia.com>; Parav Pandit <parav@...dia.com>; Si-Wei Liu <si-wei.liu@...cle.com>;
> Stefano Garzarella <sgarzare@...hat.com>; Wan Jiabing <wanjiabing@...o.com>; Dan Carpenter <dan.carpenter@...cle.com>;
> virtualization@...ts.linux-foundation.org; linux-kernel@...r.kernel.org
> Subject: [PATCH] vdpa: allow vdpa dev_del management operation to return failure
>
> Currently, the vdpa_nl_cmd_dev_del_set_doit() implementation allows
> returning a value to depict the operation status but the return type
> of dev_del() callback is void. So, any error while deleting the vdpa
> device in the vdpa parent driver can't be returned to the management
> layer.
> This patch changes the return type of dev_del() callback to int to
> allow returning an error code in case of failure.
>
> Signed-off-by: Gautam Dawar <gautam.dawar@....com>
> ---
This patch is not changing anything except for planning on some future device returning error upon delete.
Until that happens I don't see much reason for making this change.
> drivers/vdpa/ifcvf/ifcvf_main.c | 3 ++-
> drivers/vdpa/mlx5/net/mlx5_vnet.c | 3 ++-
> drivers/vdpa/vdpa.c | 11 ++++++++---
> drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 3 ++-
> drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 3 ++-
> drivers/vdpa/vdpa_user/vduse_dev.c | 3 ++-
> include/linux/vdpa.h | 5 +++--
> 7 files changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
> index 4366320fb68d..6a967935478b 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> @@ -800,13 +800,14 @@ static int ifcvf_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
> return ret;
> }
>
> -static void ifcvf_vdpa_dev_del(struct vdpa_mgmt_dev *mdev, struct vdpa_device *dev)
> +static int ifcvf_vdpa_dev_del(struct vdpa_mgmt_dev *mdev, struct vdpa_device *dev)
> {
> struct ifcvf_vdpa_mgmt_dev *ifcvf_mgmt_dev;
>
> ifcvf_mgmt_dev = container_of(mdev, struct ifcvf_vdpa_mgmt_dev, mdev);
> _vdpa_unregister_device(dev);
> ifcvf_mgmt_dev->adapter = NULL;
> + return 0;
> }
>
> static const struct vdpa_mgmtdev_ops ifcvf_vdpa_mgmt_dev_ops = {
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index e0de44000d92..b06204c2f3e8 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -2775,7 +2775,7 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
> return err;
> }
>
> -static void mlx5_vdpa_dev_del(struct vdpa_mgmt_dev *v_mdev, struct vdpa_device *dev)
> +static int mlx5_vdpa_dev_del(struct vdpa_mgmt_dev *v_mdev, struct vdpa_device *dev)
> {
> struct mlx5_vdpa_mgmtdev *mgtdev = container_of(v_mdev, struct mlx5_vdpa_mgmtdev, mgtdev);
> struct mlx5_vdpa_dev *mvdev = to_mvdev(dev);
> @@ -2788,6 +2788,7 @@ static void mlx5_vdpa_dev_del(struct vdpa_mgmt_dev *v_mdev, struct vdpa_device *
> destroy_workqueue(wq);
> _vdpa_unregister_device(dev);
> mgtdev->ndev = NULL;
> + return 0;
> }
>
> static const struct vdpa_mgmtdev_ops mdev_ops = {
> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> index 2b75c00b1005..65dc8bf2f37f 100644
> --- a/drivers/vdpa/vdpa.c
> +++ b/drivers/vdpa/vdpa.c
> @@ -363,10 +363,11 @@ static int vdpa_match_remove(struct device *dev, void *data)
> {
> struct vdpa_device *vdev = container_of(dev, struct vdpa_device, dev);
> struct vdpa_mgmt_dev *mdev = vdev->mdev;
> + int err = 0;
>
> if (mdev == data)
> - mdev->ops->dev_del(mdev, vdev);
> - return 0;
> + err = mdev->ops->dev_del(mdev, vdev);
> + return err;
> }
>
> void vdpa_mgmtdev_unregister(struct vdpa_mgmt_dev *mdev)
> @@ -673,7 +674,11 @@ static int vdpa_nl_cmd_dev_del_set_doit(struct sk_buff *skb, struct genl_info *i
> goto mdev_err;
> }
> mdev = vdev->mdev;
> - mdev->ops->dev_del(mdev, vdev);
> + err = mdev->ops->dev_del(mdev, vdev);
> + if (err) {
> + NL_SET_ERR_MSG_MOD(info->extack, "ops->dev_del failed");
> + goto dev_err;
> + }
> mdev_err:
> put_device(dev);
> dev_err:
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> index 42d401d43911..443d4b94268f 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> @@ -280,12 +280,13 @@ static int vdpasim_blk_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
> return ret;
> }
>
> -static void vdpasim_blk_dev_del(struct vdpa_mgmt_dev *mdev,
> +static int vdpasim_blk_dev_del(struct vdpa_mgmt_dev *mdev,
> struct vdpa_device *dev)
> {
> struct vdpasim *simdev = container_of(dev, struct vdpasim, vdpa);
>
> _vdpa_unregister_device(&simdev->vdpa);
> + return 0;
> }
>
> static const struct vdpa_mgmtdev_ops vdpasim_blk_mgmtdev_ops = {
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> index d5324f6fd8c7..9e5a5ad34e65 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> @@ -167,12 +167,13 @@ static int vdpasim_net_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
> return ret;
> }
>
> -static void vdpasim_net_dev_del(struct vdpa_mgmt_dev *mdev,
> +static int vdpasim_net_dev_del(struct vdpa_mgmt_dev *mdev,
> struct vdpa_device *dev)
> {
> struct vdpasim *simdev = container_of(dev, struct vdpasim, vdpa);
>
> _vdpa_unregister_device(&simdev->vdpa);
> + return 0;
> }
>
> static const struct vdpa_mgmtdev_ops vdpasim_net_mgmtdev_ops = {
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index f85d1a08ed87..33ff45e70ff7 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -1540,9 +1540,10 @@ static int vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
> return 0;
> }
>
> -static void vdpa_dev_del(struct vdpa_mgmt_dev *mdev, struct vdpa_device *dev)
> +static int vdpa_dev_del(struct vdpa_mgmt_dev *mdev, struct vdpa_device *dev)
> {
> _vdpa_unregister_device(dev);
> + return 0;
> }
>
> static const struct vdpa_mgmtdev_ops vdpa_dev_mgmtdev_ops = {
> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> index 8943a209202e..e547c9dfdfce 100644
> --- a/include/linux/vdpa.h
> +++ b/include/linux/vdpa.h
> @@ -443,12 +443,13 @@ void vdpa_set_status(struct vdpa_device *vdev, u8 status);
> * @mdev: parent device to use for device removal
> * @dev: vdpa device to remove
> * Driver need to remove the specified device by calling
> - * _vdpa_unregister_device().
> + * _vdpa_unregister_device(). Driver must return 0
> + * on success or appropriate error code in failure case.
> */
> struct vdpa_mgmtdev_ops {
> int (*dev_add)(struct vdpa_mgmt_dev *mdev, const char *name,
> const struct vdpa_dev_set_config *config);
> - void (*dev_del)(struct vdpa_mgmt_dev *mdev, struct vdpa_device *dev);
> + int (*dev_del)(struct vdpa_mgmt_dev *mdev, struct vdpa_device *dev);
> };
>
> /**
> --
> 2.30.1
Powered by blists - more mailing lists