lkml.org 
[lkml]   [2022]   [Jun]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [PATCH] vdpa: allow vdpa dev_del management operation to return failure
    Date
    > From: Gautam Dawar <gautam.dawar@amd.com>
    > Sent: Friday, June 3, 2022 1:34 PM
    > To: netdev@vger.kernel.org
    > Cc: linux-net-drivers@amd.com; hanand@amd.com; Gautam Dawar <gautam.dawar@amd.com>; Michael S. Tsirkin
    > <mst@redhat.com>; Jason Wang <jasowang@redhat.com>; Zhu Lingshan <lingshan.zhu@intel.com>; Xie Yongji
    > <xieyongji@bytedance.com>; Eli Cohen <elic@nvidia.com>; Parav Pandit <parav@nvidia.com>; Si-Wei Liu <si-wei.liu@oracle.com>;
    > Stefano Garzarella <sgarzare@redhat.com>; Wan Jiabing <wanjiabing@vivo.com>; Dan Carpenter <dan.carpenter@oracle.com>;
    > virtualization@lists.linux-foundation.org; linux-kernel@vger.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@amd.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

    \
     
     \ /
      Last update: 2022-06-06 07:30    [W:2.214 / U:0.284 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site