Re: [PATCH linux-next v6 4/8] vdpa: Enable user to set mac and mtu of vdpa device
From: Stefano Garzarella <sgarzare@redhat.com>
Date: 2021-10-26 13:17:13
On Tue, Oct 26, 2021 at 01:11:51PM +0000, Parav Pandit wrote:
quoted
From: Michael S. Tsirkin <mst@redhat.com> Sent: Tuesday, October 26, 2021 6:38 PM On Tue, Oct 26, 2021 at 01:03:41PM +0000, Parav Pandit wrote:quoted
quoted
From: Stefano Garzarella <sgarzare@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:quoted
$ 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 <redacted> Reviewed-by: Eli Cohen <redacted> Acked-by: Jason Wang <jasowang@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.cb/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,quoted
+ 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.cb/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(structnotifier_block *nb,unsigned long event, void *pquoted
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,quoted
+ 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(structsk_buffquoted
quoted
quoted
*msg, struct netlink_callback *cb) return msg->len; } +#define VDPA_DEV_NET_ATTRS_MASK ((1 <<VDPA_ATTR_DEV_NET_CFG_MACADDR) | \quoted
+ (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(structsk_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]);quoted
+ 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]);quoted
+ 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(structsk_buff *skb, struct genl_info *iquoted
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_policyvdpa_nl_policy[VDPA_ATTR_MAX + 1] = {quoted
[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,quoted
+ 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.cb/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,quoted
+ 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.cb/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(structvduse_dev*dev, const char *name)quoted
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,quoted
quoted
quoted
+ 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 indexfafb7202482c..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 undercreationquoted
quoted
quoted
* 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,quoted
* _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);quoted
};@@ -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_attrthat + * management devie support during dev_addcallbackquoted
quoted
s/devie/diviceI ran checkpatch.pl and also codespell for extra check. None catch it. good catch :-) Do you use any tool or its your sharp eyes?
No, I did not use tools. I was reading the comments, but these are typos that happen to everyone :-) I always mistake vdpa with vpda...
quoted
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.
I reviewed the patches that touch the code I know, but the rest seems okay. Thanks, Stefano _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization