Re: [PATCH linux-next v4 4/8] vdpa: Enable user to set mac and mtu of vdpa device
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: 2021-10-25 08:08:40
On Mon, Oct 25, 2021 at 07:06:42AM +0000, Parav Pandit wrote:
quoted
From: Jason Wang <jasowang@redhat.com> Sent: Monday, October 25, 2021 12:31 PM 在 2021/10/22 上午12:35, Parav Pandit 写道: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> --- changelog: 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 | 33 ++++++++++++++++++++++++++-- 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, 57 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 bd56de7484dc..ca05f69054b6 100644--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c@@ -2404,7 +2404,8 @@ struct mlx5_vdpa_mgmtdev { struct mlx5_vdpa_net *ndev; }; -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, structmlx5_vdpa_mgmtdev, mgtdev);quoted
struct virtio_net_config *config;diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c indexa50a6aa1cfc4..daa34a61c898 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);@@ -472,9 +471,15 @@ vdpa_nl_cmd_mgmtdev_get_dumpit(struct sk_buff*msg, struct netlink_callback *cb)quoted
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, structgenl_info *info)quoted
{ + 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;@@ -483,6 +488,21 @@ 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); + } + + if ((config.mask & VDPA_DEV_NET_ATTRS_MASK) && + !netlink_capable(skb, CAP_NET_ADMIN)) + return -EPERM;This deserves a independent patch. And do we need backport it to stable?This patch is adding the ability to configure mac and mtu. Hence, it is in this patch. It cannot be a different patch after this.quoted
Another question is that, do need the cap if not attrs were specified?I am not sure. A user is adding the vpda device anchored on the bus. We likely need different capability check than the NET_ADMIN.
It depends on what will the user be able to do then. Inject packets? Affect RX routing? Use up networking resources? NET_ADMIN is a safe choice but we didn't check any capability in the past so it seems reasonable to keep not checking it for the time being unless we see an actual security issue.
quoted
quoted
+ mutex_lock(&vdpa_dev_mutex); mdev = vdpa_mgmtdev_get_from_attr(info->attrs); if (IS_ERR(mdev)) {@@ -490,8 +510,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;@@ -822,6 +848,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(struct vduse_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, + 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 index111153c9ee71..315da5f918dc 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;If we want to add block device, I guess we need a union as a container?Right. When that occurs in future, there will be union to contain both.
_______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization