> From: Michael S. Tsirkin <m...@redhat.com>
> Sent: Tuesday, October 26, 2021 6:38 PM
> 
> On Tue, Oct 26, 2021 at 01:03:41PM +0000, Parav Pandit wrote:
> >
> >
> > > From: Stefano Garzarella <sgarz...@redhat.com>
> > > Sent: Tuesday, October 26, 2021 6:31 PM
> > >
> > > On Tue, Oct 26, 2021 at 07:02:39AM +0300, Parav Pandit via
> > > Virtualization
> > > wrote:
> > > >$ vdpa dev add name bar mgmtdev vdpasim_net mac 00:11:22:33:44:55
> > > >mtu
> > > >9000
> > > >
> > > >$ vdpa dev config show
> > > >bar: mac 00:11:22:33:44:55 link up link_announce false mtu 9000
> > > >
> > > >$ vdpa dev config show -jp
> > > >{
> > > >    "config": {
> > > >        "bar": {
> > > >            "mac": "00:11:22:33:44:55",
> > > >            "link ": "up",
> > > >            "link_announce ": false,
> > > >            "mtu": 9000,
> > > >        }
> > > >    }
> > > >}
> > > >
> > > >Signed-off-by: Parav Pandit <pa...@nvidia.com>
> > > >Reviewed-by: Eli Cohen <e...@nvidia.com>
> > > >Acked-by: Jason Wang <jasow...@redhat.com>
> > > >
> > > >---
> > > >changelog:
> > > >v4->v5:
> > > > - added comment for checking device capabilities
> > > >v3->v4:
> > > > - provide config attributes during device addition time
> > > >---
> > > > drivers/vdpa/ifcvf/ifcvf_main.c      |  3 ++-
> > > > drivers/vdpa/mlx5/net/mlx5_vnet.c    |  3 ++-
> > > > drivers/vdpa/vdpa.c                  | 38 ++++++++++++++++++++++++++--
> > > > 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                 | 17 ++++++++++++-
> > > > 7 files changed, 62 insertions(+), 8 deletions(-)
> > > >
> > > >diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c
> > > >b/drivers/vdpa/ifcvf/ifcvf_main.c index dcd648e1f7e7..6dc75ca70b37
> > > >100644
> > > >--- a/drivers/vdpa/ifcvf/ifcvf_main.c
> > > >+++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> > > >@@ -499,7 +499,8 @@ static u32 get_dev_type(struct pci_dev *pdev)
> > > >         return dev_type;
> > > > }
> > > >
> > > >-static int ifcvf_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const
> > > >char
> > > >*name)
> > > >+static int ifcvf_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const
> > > >+char
> > > *name,
> > > >+                              const struct vdpa_dev_set_config *config)
> > > > {
> > > >         struct ifcvf_vdpa_mgmt_dev *ifcvf_mgmt_dev;
> > > >         struct ifcvf_adapter *adapter;
> > > >diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > >b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > >index b5bd1a553256..6bbdc0ece707 100644
> > > >--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > >+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > >@@ -2482,7 +2482,8 @@ static int event_handler(struct
> > > >notifier_block *nb,
> > > unsigned long event, void *p
> > > >         return ret;
> > > > }
> > > >
> > > >-static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const
> > > >char
> > > >*name)
> > > >+static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const
> > > >+char
> > > *name,
> > > >+                             const struct vdpa_dev_set_config
> > > >*add_config)
> > > > {
> > > >         struct mlx5_vdpa_mgmtdev *mgtdev = container_of(v_mdev, struct
> > > >         mlx5_vdpa_mgmtdev, mgtdev);
> > > >         struct virtio_net_config *config; diff --git
> > > >a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index
> > > >973c56fb60b9..a1168a7fa8b8 100644
> > > >--- a/drivers/vdpa/vdpa.c
> > > >+++ b/drivers/vdpa/vdpa.c
> > > >@@ -14,7 +14,6 @@
> > > > #include <uapi/linux/vdpa.h>
> > > > #include <net/genetlink.h>
> > > > #include <linux/mod_devicetable.h> -#include <linux/virtio_net.h>
> > > >#include <linux/virtio_ids.h>
> > > >
> > > > static LIST_HEAD(mdev_head);
> > > >@@ -480,9 +479,15 @@ vdpa_nl_cmd_mgmtdev_get_dumpit(struct
> sk_buff
> > > >*msg, struct netlink_callback *cb)
> > > >         return msg->len;
> > > > }
> > > >
> > > >+#define VDPA_DEV_NET_ATTRS_MASK ((1 <<
> > > VDPA_ATTR_DEV_NET_CFG_MACADDR) | \
> > > >+                                 (1 << VDPA_ATTR_DEV_NET_CFG_MTU))
> > > >+
> > > > static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff *skb,
> > > > struct genl_info *info) {
> > > >+        struct vdpa_dev_set_config config = {};
> > > >+        struct nlattr **nl_attrs = info->attrs;
> > > >         struct vdpa_mgmt_dev *mdev;
> > > >+        const u8 *macaddr;
> > > >         const char *name;
> > > >         int err = 0;
> > > >
> > > >@@ -491,6 +496,26 @@ static int vdpa_nl_cmd_dev_add_set_doit(struct
> > > >sk_buff *skb, struct genl_info *i
> > > >
> > > >         name = nla_data(info->attrs[VDPA_ATTR_DEV_NAME]);
> > > >
> > > >+        if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MACADDR]) {
> > > >+                macaddr =
> > > nla_data(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MACADDR]);
> > > >+                memcpy(config.net.mac, macaddr, sizeof(config.net.mac));
> > > >+                config.mask |= (1 << VDPA_ATTR_DEV_NET_CFG_MACADDR);
> > > >+        }
> > > >+        if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]) {
> > > >+                config.net.mtu =
> > > >+
> > >   nla_get_u16(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]);
> > > >+                config.mask |= (1 << VDPA_ATTR_DEV_NET_CFG_MTU);
> > > >+        }
> > > >+
> > > >+        /* Skip checking capability if user didn't prefer to configure 
> > > >any
> > > >+         * device networking attributes. It is likely that user might 
> > > >have used
> > > >+         * a device specific method to configure such attributes or 
> > > >using device
> > > >+         * default attributes.
> > > >+         */
> > > >+        if ((config.mask & VDPA_DEV_NET_ATTRS_MASK) &&
> > > >+            !netlink_capable(skb, CAP_NET_ADMIN))
> > > >+                return -EPERM;
> > > >+
> > > >         mutex_lock(&vdpa_dev_mutex);
> > > >         mdev = vdpa_mgmtdev_get_from_attr(info->attrs);
> > > >         if (IS_ERR(mdev)) {
> > > >@@ -498,8 +523,14 @@ static int vdpa_nl_cmd_dev_add_set_doit(struct
> > > sk_buff *skb, struct genl_info *i
> > > >                 err = PTR_ERR(mdev);
> > > >                 goto err;
> > > >         }
> > > >+        if ((config.mask & mdev->config_attr_mask) != config.mask) {
> > > >+                NL_SET_ERR_MSG_MOD(info->extack,
> > > >+                                   "All provided attributes are not 
> > > >supported");
> > > >+                err = -EOPNOTSUPP;
> > > >+                goto err;
> > > >+        }
> > > >
> > > >-        err = mdev->ops->dev_add(mdev, name);
> > > >+        err = mdev->ops->dev_add(mdev, name, &config);
> > > > err:
> > > >         mutex_unlock(&vdpa_dev_mutex);
> > > >         return err;
> > > >@@ -835,6 +866,9 @@ static const struct nla_policy
> > > vdpa_nl_policy[VDPA_ATTR_MAX + 1] = {
> > > >         [VDPA_ATTR_MGMTDEV_BUS_NAME] = { .type = NLA_NUL_STRING },
> > > >         [VDPA_ATTR_MGMTDEV_DEV_NAME] = { .type = NLA_STRING },
> > > >         [VDPA_ATTR_DEV_NAME] = { .type = NLA_STRING },
> > > >+        [VDPA_ATTR_DEV_NET_CFG_MACADDR] = NLA_POLICY_ETH_ADDR,
> > > >+        /* virtio spec 1.1 section 5.1.4.1 for valid MTU range */
> > > >+        [VDPA_ATTR_DEV_NET_CFG_MTU] = NLA_POLICY_MIN(NLA_U16, 68),
> > > > };
> > > >
> > > > static const struct genl_ops vdpa_nl_ops[] = { diff --git
> > > >a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> > > >b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> > > >index a790903f243e..42d401d43911 100644
> > > >--- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> > > >+++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> > > >@@ -248,7 +248,8 @@ static struct device vdpasim_blk_mgmtdev = {
> > > >         .release = vdpasim_blk_mgmtdev_release,  };
> > > >
> > > >-static int vdpasim_blk_dev_add(struct vdpa_mgmt_dev *mdev, const
> > > >char
> > > >*name)
> > > >+static int vdpasim_blk_dev_add(struct vdpa_mgmt_dev *mdev, const
> > > >+char
> > > *name,
> > > >+                               const struct vdpa_dev_set_config *config)
> > > > {
> > > >         struct vdpasim_dev_attr dev_attr = {};
> > > >         struct vdpasim *simdev;
> > > >diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> > > >b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> > > >index a1ab6163f7d1..d681e423e64f 100644
> > > >--- a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> > > >+++ b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> > > >@@ -126,7 +126,8 @@ static struct device vdpasim_net_mgmtdev = {
> > > >         .release = vdpasim_net_mgmtdev_release,  };
> > > >
> > > >-static int vdpasim_net_dev_add(struct vdpa_mgmt_dev *mdev, const
> > > >char
> > > >*name)
> > > >+static int vdpasim_net_dev_add(struct vdpa_mgmt_dev *mdev, const
> > > >+char
> > > *name,
> > > >+                               const struct vdpa_dev_set_config *config)
> > > > {
> > > >         struct vdpasim_dev_attr dev_attr = {};
> > > >         struct vdpasim *simdev;
> > > >diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
> > > >b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > >index 841667a896dd..c9204c62f339 100644
> > > >--- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > > >+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > >@@ -1503,7 +1503,8 @@ static int vduse_dev_init_vdpa(struct
> > > >vduse_dev
> > > *dev, const char *name)
> > > >         return 0;
> > > > }
> > > >
> > > >-static int vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char
> > > >*name)
> > > >+static int vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char
> *name,
> > > >+                        const struct vdpa_dev_set_config *config)
> > > > {
> > > >         struct vduse_dev *dev;
> > > >         int ret;
> > > >diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h index
> > > >fafb7202482c..bf9ddf743e2f 100644
> > > >--- a/include/linux/vdpa.h
> > > >+++ b/include/linux/vdpa.h
> > > >@@ -6,6 +6,8 @@
> > > > #include <linux/device.h>
> > > > #include <linux/interrupt.h>
> > > > #include <linux/vhost_iotlb.h>
> > > >+#include <linux/virtio_net.h>
> > > >+#include <linux/if_ether.h>
> > > >
> > > > /**
> > > >  * struct vdpa_calllback - vDPA callback definition.
> > > >@@ -93,6 +95,14 @@ struct vdpa_iova_range {
> > > >         u64 last;
> > > > };
> > > >
> > > >+struct vdpa_dev_set_config {
> > > >+        struct {
> > > >+                u8 mac[ETH_ALEN];
> > > >+                u16 mtu;
> > > >+        } net;
> > > >+        u64 mask;
> > > >+};
> > > >+
> > > > /**
> > > >  * Corresponding file area for device memory mapping
> > > >  * @file: vma->vm_file for the mapping @@ -397,6 +407,7 @@ void
> > > >vdpa_set_config(struct vdpa_device *dev, unsigned int offset,
> > > >  * @dev_add: Add a vdpa device using alloc and register
> > > >  *           @mdev: parent device to use for device addition
> > > >  *           @name: name of the new vdpa device
> > > >+ *           @config: config attributes to apply to the device under
> creation
> > > >  *           Driver need to add a new device using 
> > > > _vdpa_register_device()
> > > >  *           after fully initializing the vdpa device. Driver must 
> > > > return 0
> > > >  *           on success or appropriate error code.
> > > >@@ -407,7 +418,8 @@ void vdpa_set_config(struct vdpa_device *dev,
> > > unsigned int offset,
> > > >  *           _vdpa_unregister_device().
> > > >  */
> > > > struct vdpa_mgmtdev_ops {
> > > >-        int (*dev_add)(struct vdpa_mgmt_dev *mdev, const char *name);
> > > >+        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);
> > > > };
> > > >
> > > >@@ -416,12 +428,15 @@ struct vdpa_mgmtdev_ops {
> > > >  * @device: Management parent device
> > > >  * @ops: operations supported by management device
> > > >  * @id_table: Pointer to device id table of supported ids
> > > >+ * @config_attr_mask: bit mask of attributes of type enum
> > > >+ vdpa_attr
> > > >that
> > > >+ *                    management devie support during dev_add
> callback
> > >
> > > s/devie/divice
> >
> > I ran checkpatch.pl and also codespell for extra check. None catch it.
> > good catch :-) Do you use any tool or its your sharp eyes?
> 
> OK so v7?
If you prefer v7 for this small change, I will do it.
Will wait for little more before sending v7 if Stefano has any more comments in 
rest of the patches.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Reply via email to