Re: [PATCH linux-next v4 4/8] vdpa: Enable user to set mac and mtu of vdpa device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




在 2021/10/25 下午3:06, Parav Pandit 写道:

From: Jason Wang <jasowang@xxxxxxxxxx>
Sent: Monday, October 25, 2021 12:31 PM

在 2021/10/22 上午12:35, Parav Pandit 写道:
$ 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 <parav@xxxxxxxxxx>
Reviewed-by: Eli Cohen <elic@xxxxxxxxxx>
---
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.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 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,
+			     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
a50a6aa1cfc4..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)
   	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;

@@ -483,6 +488,21 @@ 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);
+	}
+
+	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.

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.


Yes, that is what I meant. It looks to me currently we allow unprivileged user to add vDPA device? If not, we are probably fine.

Thanks




+
   	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(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;
@@ -822,6 +848,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
111153c9ee71..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@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization



[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux