[PATCH linux-next v4 0/8] vdpa: enable user to set mac, mtu

[PATCH linux-next v4 0/8] vdpa: enable user to set mac, mtu

[Parav Pandit via Virtualization]

Currently user cannot view the vdpa device config space. Also user
cannot set the mac address and mtu of the vdpa device.
This patchset enables users to set the mac address and mtu of the vdpa
device once the device is created.
If a vendor driver supports such configuration user can set it otherwise
user gets unsupported error.

vdpa mac address and mtu are device configuration layout fields.
To keep interface generic enough for multiple types of vdpa devices, mac
address and mtu setting is implemented as configuration layout fields.

An example of query & set of config layout fields for vdpa_sim_net
driver:

Configuration layout fields are set when a vdpa device is created.

$ vdpa mgmtdev show
vdpasim_net:
  supported_classes net
pci/0000:08:00.2:
  supported_classes net

Add the device with MAC and MTU:
$ vdpa dev add name bar mgmtdev vdpasim_net mac 00:11:22:33:44:66 mtu 1500

In above command only mac address or only mtu can also be set.

View the config after setting:
$ vdpa dev config show
bar: mac 00:11:22:33:44:66 link up link_announce false mtu 1500

Patch summary:
Patch-1 introduced and use helpers for get/set config area
Patch-2 implement query device config layout
Patch-3 use kernel coding style for structure comment
Patch-4 enanble user to set mac and mtu in config space
Patch-5 vdpa_sim_net implements get and set of config layout
Patch-6 mlx vdpa driver fix to avoid clearing VIRTIO_NET_F_MAC during
        reset callback
Patch-7 mlx5 vdpa driver supports user provided mac config
Patch-8 mlx5 vdpa driver uses user provided mac during rx flow steering

changelog:
v3->v4:
 - enable setting mac and mtu of the vdpa device using creation time
 - introduced a patch to fix mlx5 driver to avoid clearing
   VIRTIO_NET_F_MAC
 - introduced a patch to use kernel coding style for structure comment
 - removed config attributes not used by sim and mlx5 net drivers
v2->v3:
 - dropped patches which are merged
 - simplified code to handle non transitional devices

v1->v2:
 - new patches to fix kdoc comment to add new kdoc section
 - new patch to have synchronized access to features and config space
 - read whole net config layout instead of individual fields
 - added error extack for unmanaged vdpa device
 - fixed several endianness issues
 - introduced vdpa device ops for get config which is synchronized
   with other get/set features ops and config ops
 - fixed mtu range checking for max
 - using NLA_POLICY_ETH_ADDR
 - set config moved to device ops instead of mgmtdev ops
 - merged build and set to single routine
 - ensuring that user has NET_ADMIN capability for configuring network
   attributes
 - using updated interface and callbacks for get/set config
 - following new api for config get/set for mgmt tool in mlx5 vdpa
   driver
 - fixes for accessing right SF dma device and bar address
 - fix for mtu calculation
 - fix for bit access in features
 - fix for index restore with suspend/resume operation


Eli Cohen (2):
  vdpa/mlx5: Support configuration of MAC
  vdpa/mlx5: Forward only packets with allowed MAC address

Parav Pandit (6):
  vdpa: Introduce and use vdpa device get, set config helpers
  vdpa: Introduce query of device config layout
  vdpa: Use kernel coding style for structure comments
  vdpa: Enable user to set mac and mtu of vdpa device
  vdpa_sim_net: Enable user to set mac address and mtu
  vdpa/mlx5: Fix clearing of VIRTIO_NET_F_MAC feature bit

 drivers/vdpa/ifcvf/ifcvf_main.c      |   3 +-
 drivers/vdpa/mlx5/net/mlx5_vnet.c    |  92 +++++++---
 drivers/vdpa/vdpa.c                  | 243 ++++++++++++++++++++++++++-
 drivers/vdpa/vdpa_sim/vdpa_sim_blk.c |   3 +-
 drivers/vdpa/vdpa_sim/vdpa_sim_net.c |  38 +++--
 drivers/vdpa/vdpa_user/vduse_dev.c   |   3 +-
 drivers/vhost/vdpa.c                 |   3 +-
 include/linux/vdpa.h                 |  47 ++++--
 include/uapi/linux/vdpa.h            |   6 +
 9 files changed, 375 insertions(+), 63 deletions(-)

-- 
2.25.4

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH linux-next v4 1/8] vdpa: Introduce and use vdpa device get, set config helpers

[Parav Pandit via Virtualization]

Subsequent patches enable get and set configuration either
via management device or via vdpa device' config ops.

This requires synchronization between multiple callers to get and set
config callbacks. Features setting also influence the layout of the
configuration fields endianness.

To avoid exposing synchronization primitives to callers, introduce
helper for setting the configuration and use it.

Signed-off-by: Parav Pandit <parav@nvidia.com>
Reviewed-by: Eli Cohen <elic@nvidia.com>
Acked-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vdpa/vdpa.c  | 36 ++++++++++++++++++++++++++++++++++++
 drivers/vhost/vdpa.c |  3 +--
 include/linux/vdpa.h | 19 ++++---------------
 3 files changed, 41 insertions(+), 17 deletions(-)

diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index 1dc121a07a93..6fdfc11ecb13 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -289,6 +289,42 @@ void vdpa_mgmtdev_unregister(struct vdpa_mgmt_dev *mdev)
 }
 EXPORT_SYMBOL_GPL(vdpa_mgmtdev_unregister);
 
+/**
+ * vdpa_get_config - Get one or more device configuration fields.
+ * @vdev: vdpa device to operate on
+ * @offset: starting byte offset of the field
+ * @buf: buffer pointer to read to
+ * @len: length of the configuration fields in bytes
+ */
+void vdpa_get_config(struct vdpa_device *vdev, unsigned int offset,
+		     void *buf, unsigned int len)
+{
+	const struct vdpa_config_ops *ops = vdev->config;
+
+	/*
+	 * Config accesses aren't supposed to trigger before features are set.
+	 * If it does happen we assume a legacy guest.
+	 */
+	if (!vdev->features_valid)
+		vdpa_set_features(vdev, 0);
+	ops->get_config(vdev, offset, buf, len);
+}
+EXPORT_SYMBOL_GPL(vdpa_get_config);
+
+/**
+ * vdpa_set_config - Set one or more device configuration fields.
+ * @vdev: vdpa device to operate on
+ * @offset: starting byte offset of the field
+ * @buf: buffer pointer to read from
+ * @length: length of the configuration fields in bytes
+ */
+void vdpa_set_config(struct vdpa_device *vdev, unsigned int offset,
+		     void *buf, unsigned int length)
+{
+	vdev->config->set_config(vdev, offset, buf, length);
+}
+EXPORT_SYMBOL_GPL(vdpa_set_config);
+
 static bool mgmtdev_handle_match(const struct vdpa_mgmt_dev *mdev,
 				 const char *busname, const char *devname)
 {
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 39039e046117..01c59ce7e250 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -237,7 +237,6 @@ static long vhost_vdpa_set_config(struct vhost_vdpa *v,
 				  struct vhost_vdpa_config __user *c)
 {
 	struct vdpa_device *vdpa = v->vdpa;
-	const struct vdpa_config_ops *ops = vdpa->config;
 	struct vhost_vdpa_config config;
 	unsigned long size = offsetof(struct vhost_vdpa_config, buf);
 	u8 *buf;
@@ -251,7 +250,7 @@ static long vhost_vdpa_set_config(struct vhost_vdpa *v,
 	if (IS_ERR(buf))
 		return PTR_ERR(buf);
 
-	ops->set_config(vdpa, config.off, buf, config.len);
+	vdpa_set_config(vdpa, config.off, buf, config.len);
 
 	kvfree(buf);
 	return 0;
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index 3972ab765de1..78395331a166 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -382,21 +382,10 @@ static inline int vdpa_set_features(struct vdpa_device *vdev, u64 features)
 	return ops->set_features(vdev, features);
 }
 
-static inline void vdpa_get_config(struct vdpa_device *vdev,
-				   unsigned int offset, void *buf,
-				   unsigned int len)
-{
-	const struct vdpa_config_ops *ops = vdev->config;
-
-	/*
-	 * Config accesses aren't supposed to trigger before features are set.
-	 * If it does happen we assume a legacy guest.
-	 */
-	if (!vdev->features_valid)
-		vdpa_set_features(vdev, 0);
-	ops->get_config(vdev, offset, buf, len);
-}
-
+void vdpa_get_config(struct vdpa_device *vdev, unsigned int offset,
+		     void *buf, unsigned int len);
+void vdpa_set_config(struct vdpa_device *dev, unsigned int offset,
+		     void *buf, unsigned int length);
 /**
  * struct vdpa_mgmtdev_ops - vdpa device ops
  * @dev_add: Add a vdpa device using alloc and register
-- 
2.25.4

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH linux-next v4 2/8] vdpa: Introduce query of device config layout

[Parav Pandit via Virtualization]

Introduce a command to query a device config layout.

An example query of network vdpa device:

$ vdpa dev add name bar mgmtdev vdpasim_net

$ vdpa dev config show
bar: mac 00:35:09:19:48:05 link up link_announce false mtu 1500

$ vdpa dev config show -jp
{
    "config": {
        "bar": {
            "mac": "00:35:09:19:48:05",
            "link ": "up",
            "link_announce ": false,
            "mtu": 1500,
        }
    }
}

Signed-off-by: Parav Pandit <parav@nvidia.com>
Signed-off-by: Eli Cohen <elic@nvidia.com>

---
changelog:
v3->v4:
 - removed config space fields reporting which are not used by mlx5
   and sim drivers
---
 drivers/vdpa/vdpa.c       | 176 ++++++++++++++++++++++++++++++++++++++
 include/linux/vdpa.h      |   2 +
 include/uapi/linux/vdpa.h |   6 ++
 3 files changed, 184 insertions(+)

diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index 6fdfc11ecb13..a50a6aa1cfc4 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -14,6 +14,8 @@
 #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);
 /* A global mutex that protects vdpa management device and device level operations. */
@@ -58,6 +60,7 @@ static void vdpa_release_dev(struct device *d)
 		ops->free(vdev);
 
 	ida_simple_remove(&vdpa_index_ida, vdev->index);
+	mutex_destroy(&vdev->cf_mutex);
 	kfree(vdev);
 }
 
@@ -119,6 +122,7 @@ struct vdpa_device *__vdpa_alloc_device(struct device *parent,
 	if (err)
 		goto err_name;
 
+	mutex_init(&vdev->cf_mutex);
 	device_initialize(&vdev->dev);
 
 	return vdev;
@@ -301,6 +305,7 @@ void vdpa_get_config(struct vdpa_device *vdev, unsigned int offset,
 {
 	const struct vdpa_config_ops *ops = vdev->config;
 
+	mutex_lock(&vdev->cf_mutex);
 	/*
 	 * Config accesses aren't supposed to trigger before features are set.
 	 * If it does happen we assume a legacy guest.
@@ -308,6 +313,7 @@ void vdpa_get_config(struct vdpa_device *vdev, unsigned int offset,
 	if (!vdev->features_valid)
 		vdpa_set_features(vdev, 0);
 	ops->get_config(vdev, offset, buf, len);
+	mutex_unlock(&vdev->cf_mutex);
 }
 EXPORT_SYMBOL_GPL(vdpa_get_config);
 
@@ -321,7 +327,9 @@ EXPORT_SYMBOL_GPL(vdpa_get_config);
 void vdpa_set_config(struct vdpa_device *vdev, unsigned int offset,
 		     void *buf, unsigned int length)
 {
+	mutex_lock(&vdev->cf_mutex);
 	vdev->config->set_config(vdev, offset, buf, length);
+	mutex_unlock(&vdev->cf_mutex);
 }
 EXPORT_SYMBOL_GPL(vdpa_set_config);
 
@@ -648,6 +656,168 @@ static int vdpa_nl_cmd_dev_get_dumpit(struct sk_buff *msg, struct netlink_callba
 	return msg->len;
 }
 
+static int vdpa_dev_net_mq_config_fill(struct vdpa_device *vdev,
+				       struct sk_buff *msg, u64 features,
+				       const struct virtio_net_config *config)
+{
+	u16 val_u16;
+
+	if ((features & (1ULL << VIRTIO_NET_F_MQ)) == 0)
+		return 0;
+
+	val_u16 = le16_to_cpu(config->max_virtqueue_pairs);
+	return nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, val_u16);
+}
+
+static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *msg)
+{
+	struct virtio_net_config config = {};
+	u64 features;
+	u16 val_u16;
+
+	vdpa_get_config(vdev, 0, &config, sizeof(config));
+
+	if (nla_put(msg, VDPA_ATTR_DEV_NET_CFG_MACADDR, sizeof(config.mac),
+		    config.mac))
+		return -EMSGSIZE;
+
+	val_u16 = le16_to_cpu(config.status);
+	if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, val_u16))
+		return -EMSGSIZE;
+
+	val_u16 = le16_to_cpu(config.mtu);
+	if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, val_u16))
+		return -EMSGSIZE;
+
+	features = vdev->config->get_features(vdev);
+
+	return vdpa_dev_net_mq_config_fill(vdev, msg, features, &config);
+}
+
+static int
+vdpa_dev_config_fill(struct vdpa_device *vdev, struct sk_buff *msg, u32 portid, u32 seq,
+		     int flags, struct netlink_ext_ack *extack)
+{
+	u32 device_id;
+	void *hdr;
+	int err;
+
+	hdr = genlmsg_put(msg, portid, seq, &vdpa_nl_family, flags,
+			  VDPA_CMD_DEV_CONFIG_GET);
+	if (!hdr)
+		return -EMSGSIZE;
+
+	if (nla_put_string(msg, VDPA_ATTR_DEV_NAME, dev_name(&vdev->dev))) {
+		err = -EMSGSIZE;
+		goto msg_err;
+	}
+
+	device_id = vdev->config->get_device_id(vdev);
+	if (nla_put_u32(msg, VDPA_ATTR_DEV_ID, device_id)) {
+		err = -EMSGSIZE;
+		goto msg_err;
+	}
+
+	switch (device_id) {
+	case VIRTIO_ID_NET:
+		err = vdpa_dev_net_config_fill(vdev, msg);
+		break;
+	default:
+		err = -EOPNOTSUPP;
+		break;
+	}
+	if (err)
+		goto msg_err;
+
+	genlmsg_end(msg, hdr);
+	return 0;
+
+msg_err:
+	genlmsg_cancel(msg, hdr);
+	return err;
+}
+
+static int vdpa_nl_cmd_dev_config_get_doit(struct sk_buff *skb, struct genl_info *info)
+{
+	struct vdpa_device *vdev;
+	struct sk_buff *msg;
+	const char *devname;
+	struct device *dev;
+	int err;
+
+	if (!info->attrs[VDPA_ATTR_DEV_NAME])
+		return -EINVAL;
+	devname = nla_data(info->attrs[VDPA_ATTR_DEV_NAME]);
+	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+
+	mutex_lock(&vdpa_dev_mutex);
+	dev = bus_find_device(&vdpa_bus, NULL, devname, vdpa_name_match);
+	if (!dev) {
+		NL_SET_ERR_MSG_MOD(info->extack, "device not found");
+		err = -ENODEV;
+		goto dev_err;
+	}
+	vdev = container_of(dev, struct vdpa_device, dev);
+	if (!vdev->mdev) {
+		NL_SET_ERR_MSG_MOD(info->extack, "unmanaged vdpa device");
+		err = -EINVAL;
+		goto mdev_err;
+	}
+	err = vdpa_dev_config_fill(vdev, msg, info->snd_portid, info->snd_seq,
+				   0, info->extack);
+	if (!err)
+		err = genlmsg_reply(msg, info);
+
+mdev_err:
+	put_device(dev);
+dev_err:
+	mutex_unlock(&vdpa_dev_mutex);
+	if (err)
+		nlmsg_free(msg);
+	return err;
+}
+
+static int vdpa_dev_config_dump(struct device *dev, void *data)
+{
+	struct vdpa_device *vdev = container_of(dev, struct vdpa_device, dev);
+	struct vdpa_dev_dump_info *info = data;
+	int err;
+
+	if (!vdev->mdev)
+		return 0;
+	if (info->idx < info->start_idx) {
+		info->idx++;
+		return 0;
+	}
+	err = vdpa_dev_config_fill(vdev, info->msg, NETLINK_CB(info->cb->skb).portid,
+				   info->cb->nlh->nlmsg_seq, NLM_F_MULTI,
+				   info->cb->extack);
+	if (err)
+		return err;
+
+	info->idx++;
+	return 0;
+}
+
+static int
+vdpa_nl_cmd_dev_config_get_dumpit(struct sk_buff *msg, struct netlink_callback *cb)
+{
+	struct vdpa_dev_dump_info info;
+
+	info.msg = msg;
+	info.cb = cb;
+	info.start_idx = cb->args[0];
+	info.idx = 0;
+
+	mutex_lock(&vdpa_dev_mutex);
+	bus_for_each_dev(&vdpa_bus, NULL, &info, vdpa_dev_config_dump);
+	mutex_unlock(&vdpa_dev_mutex);
+	cb->args[0] = info.idx;
+	return msg->len;
+}
+
 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 },
@@ -679,6 +849,12 @@ static const struct genl_ops vdpa_nl_ops[] = {
 		.doit = vdpa_nl_cmd_dev_get_doit,
 		.dumpit = vdpa_nl_cmd_dev_get_dumpit,
 	},
+	{
+		.cmd = VDPA_CMD_DEV_CONFIG_GET,
+		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+		.doit = vdpa_nl_cmd_dev_config_get_doit,
+		.dumpit = vdpa_nl_cmd_dev_config_get_dumpit,
+	},
 };
 
 static struct genl_family vdpa_nl_family __ro_after_init = {
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index 78395331a166..9326020a2c55 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -63,6 +63,7 @@ struct vdpa_mgmt_dev;
  * @dev: underlying device
  * @dma_dev: the actual device that is performing DMA
  * @config: the configuration ops for this device.
+ * @cf_mutex: Protects get and set access to configuration layout.
  * @index: device index
  * @features_valid: were features initialized? for legacy guests
  * @use_va: indicate whether virtual address must be used by this device
@@ -74,6 +75,7 @@ struct vdpa_device {
 	struct device dev;
 	struct device *dma_dev;
 	const struct vdpa_config_ops *config;
+	struct mutex cf_mutex; /* Protects get/set config */
 	unsigned int index;
 	bool features_valid;
 	bool use_va;
diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
index 66a41e4ec163..37ef30130a28 100644
--- a/include/uapi/linux/vdpa.h
+++ b/include/uapi/linux/vdpa.h
@@ -17,6 +17,7 @@ enum vdpa_command {
 	VDPA_CMD_DEV_NEW,
 	VDPA_CMD_DEV_DEL,
 	VDPA_CMD_DEV_GET,		/* can dump */
+	VDPA_CMD_DEV_CONFIG_GET,	/* can dump */
 };
 
 enum vdpa_attr {
@@ -33,6 +34,11 @@ enum vdpa_attr {
 	VDPA_ATTR_DEV_MAX_VQS,			/* u32 */
 	VDPA_ATTR_DEV_MAX_VQ_SIZE,		/* u16 */
 
+	VDPA_ATTR_DEV_NET_CFG_MACADDR,		/* binary */
+	VDPA_ATTR_DEV_NET_STATUS,		/* u8 */
+	VDPA_ATTR_DEV_NET_CFG_MAX_VQP,		/* u16 */
+	VDPA_ATTR_DEV_NET_CFG_MTU,		/* u16 */
+
 	/* new attributes must be added above here */
 	VDPA_ATTR_MAX,
 };
-- 
2.25.4

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH linux-next v4 3/8] vdpa: Use kernel coding style for structure comments

[Parav Pandit via Virtualization]

As subsequent patch adds new structure field with comment, move the
structure comment to follow kernel coding style.

Signed-off-by: Parav Pandit <parav@nvidia.com>
Reviewed-by: Eli Cohen <elic@nvidia.com>
---
 include/linux/vdpa.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index 9326020a2c55..111153c9ee71 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -407,10 +407,17 @@ struct vdpa_mgmtdev_ops {
 	void (*dev_del)(struct vdpa_mgmt_dev *mdev, struct vdpa_device *dev);
 };
 
+/**
+ * struct vdpa_mgmt_dev - vdpa management device
+ * @device: Management parent device
+ * @ops: operations supported by management device
+ * @id_table: Pointer to device id table of supported ids
+ * @list: list entry
+ */
 struct vdpa_mgmt_dev {
 	struct device *device;
 	const struct vdpa_mgmtdev_ops *ops;
-	const struct virtio_device_id *id_table; /* supported ids */
+	const struct virtio_device_id *id_table;
 	struct list_head list;
 };
 
-- 
2.25.4

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

[Parav Pandit via Virtualization]

$ 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@nvidia.com>
Reviewed-by: Eli Cohen <elic@nvidia.com>
---
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;
+
 	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;
+	u64 mask;
+};
+
 /**
  * Corresponding file area for device memory mapping
  * @file: vma->vm_file for the mapping
@@ -393,6 +403,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.
@@ -403,7 +414,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);
 };
 
@@ -412,12 +424,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
  * @list: list entry
  */
 struct vdpa_mgmt_dev {
 	struct device *device;
 	const struct vdpa_mgmtdev_ops *ops;
 	const struct virtio_device_id *id_table;
+	u64 config_attr_mask;
 	struct list_head list;
 };
 
-- 
2.25.4

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH linux-next v4 5/8] vdpa_sim_net: Enable user to set mac address and mtu

[Parav Pandit via Virtualization]

Enable user to set the mac address and mtu so that each vdpa device
can have its own user specified mac address and mtu.
This is done by implementing the management device's configuration
layout fields setting callback routine.

Now that user is enabled to set the mac address, remove the module
parameter for same.

And example of setting mac addr and mtu:
$ vdpa mgmtdev show

$ vdpa dev add name bar mgmtdev vdpasim_net
$ vdpa dev config set bar mac 00:11:22:33:44:55 mtu 9000

View the config after setting:
$ vdpa dev config show
bar: mac 00:11:22:33:44:55 link up link_announce false mtu 9000

Signed-off-by: Parav Pandit <parav@nvidia.com>
Reviewed-by: Eli Cohen <elic@nvidia.com>
---
 drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 35 +++++++++++++++-------------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
index d681e423e64f..76dd24abc791 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
@@ -16,6 +16,7 @@
 #include <linux/vringh.h>
 #include <linux/vdpa.h>
 #include <uapi/linux/virtio_net.h>
+#include <uapi/linux/vdpa.h>
 
 #include "vdpa_sim.h"
 
@@ -29,12 +30,6 @@
 
 #define VDPASIM_NET_VQ_NUM	2
 
-static char *macaddr;
-module_param(macaddr, charp, 0);
-MODULE_PARM_DESC(macaddr, "Ethernet MAC address");
-
-static u8 macaddr_buf[ETH_ALEN];
-
 static void vdpasim_net_work(struct work_struct *work)
 {
 	struct vdpasim *vdpasim = container_of(work, struct vdpasim, work);
@@ -112,9 +107,21 @@ static void vdpasim_net_get_config(struct vdpasim *vdpasim, void *config)
 {
 	struct virtio_net_config *net_config = config;
 
-	net_config->mtu = cpu_to_vdpasim16(vdpasim, 1500);
 	net_config->status = cpu_to_vdpasim16(vdpasim, VIRTIO_NET_S_LINK_UP);
-	memcpy(net_config->mac, macaddr_buf, ETH_ALEN);
+}
+
+static void vdpasim_net_setup_config(struct vdpasim *vdpasim,
+				     const struct vdpa_dev_set_config *config)
+{
+	struct virtio_net_config *vio_config = vdpasim->config;
+
+	if (config->mask & (1 << VDPA_ATTR_DEV_NET_CFG_MACADDR))
+		memcpy(vio_config->mac, config->net.mac, ETH_ALEN);
+	if (config->mask & (1 << VDPA_ATTR_DEV_NET_CFG_MTU))
+		vio_config->mtu = cpu_to_vdpasim16(vdpasim, config->net.mtu);
+	else
+		/* Setup default MTU to be 1500 */
+		vio_config->mtu = cpu_to_vdpasim16(vdpasim, 1500);
 }
 
 static void vdpasim_net_mgmtdev_release(struct device *dev)
@@ -147,6 +154,8 @@ static int vdpasim_net_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
 	if (IS_ERR(simdev))
 		return PTR_ERR(simdev);
 
+	vdpasim_net_setup_config(simdev, config);
+
 	ret = _vdpa_register_device(&simdev->vdpa, VDPASIM_NET_VQ_NUM);
 	if (ret)
 		goto reg_err;
@@ -180,20 +189,14 @@ static struct vdpa_mgmt_dev mgmt_dev = {
 	.device = &vdpasim_net_mgmtdev,
 	.id_table = id_table,
 	.ops = &vdpasim_net_mgmtdev_ops,
+	.config_attr_mask = (1 << VDPA_ATTR_DEV_NET_CFG_MACADDR |
+			     1 << VDPA_ATTR_DEV_NET_CFG_MTU),
 };
 
 static int __init vdpasim_net_init(void)
 {
 	int ret;
 
-	if (macaddr) {
-		mac_pton(macaddr, macaddr_buf);
-		if (!is_valid_ether_addr(macaddr_buf))
-			return -EADDRNOTAVAIL;
-	} else {
-		eth_random_addr(macaddr_buf);
-	}
-
 	ret = device_register(&vdpasim_net_mgmtdev);
 	if (ret)
 		return ret;
-- 
2.25.4

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH linux-next v4 6/8] vdpa/mlx5: Fix clearing of VIRTIO_NET_F_MAC feature bit

[Parav Pandit via Virtualization]

Cited patch in the fixes tag clears the features bit during reset.
mlx5 vdpa device feature bits are static decided by device capabilities.

Clearing features bit cleared the VIRTIO_NET_F_MAC. Due to this MAC address
provided by the device is not honored.

Fix it by not clearing the static feature bits during reset.

Fixes: 0686082dbf7a ("vdpa: Add reset callback in vdpa_config_ops")
Signed-off-by: Parav Pandit <parav@nvidia.com>
Reviewed-by: Eli Cohen <elic@nvidia.com>
---
 drivers/vdpa/mlx5/net/mlx5_vnet.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index ca05f69054b6..0a2b79887085 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -2192,7 +2192,6 @@ static int mlx5_vdpa_reset(struct vdpa_device *vdev)
 	clear_vqs_ready(ndev);
 	mlx5_vdpa_destroy_mr(&ndev->mvdev);
 	ndev->mvdev.status = 0;
-	ndev->mvdev.mlx_features = 0;
 	memset(ndev->event_cbs, 0, sizeof(ndev->event_cbs));
 	ndev->mvdev.actual_features = 0;
 	++mvdev->generation;
-- 
2.25.4

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH linux-next v4 7/8] vdpa/mlx5: Support configuration of MAC

[Parav Pandit via Virtualization]

From: Eli Cohen <elic@nvidia.com>

Add code to accept MAC configuration through vdpa tool. The MAC is
written into the config struct and later can be retrieved through
get_config().

Examples:
1. Configure MAC while adding a device:
$ vdpa dev add mgmtdev pci/0000:06:00.2 name vdpa0 mac 00:11:22:33:44:55

2. Show configured params:
$ vdpa dev config show
vdpa0: mac 00:11:22:33:44:55 link down link_announce false max_vq_pairs 8 mtu 1500

Signed-off-by: Eli Cohen <elic@nvidia.com>
Reviewed-by: Parav Pandit <parav@nvidia.com>
---
 drivers/vdpa/mlx5/net/mlx5_vnet.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 0a2b79887085..61f17ce0892e 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -6,6 +6,7 @@
 #include <linux/vringh.h>
 #include <uapi/linux/virtio_net.h>
 #include <uapi/linux/virtio_ids.h>
+#include <uapi/linux/vdpa.h>
 #include <linux/virtio_config.h>
 #include <linux/auxiliary_bus.h>
 #include <linux/mlx5/cq.h>
@@ -2444,9 +2445,13 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
 	if (err)
 		goto err_mtu;
 
-	err = mlx5_query_nic_vport_mac_address(mdev, 0, 0, config->mac);
-	if (err)
-		goto err_mtu;
+	if (add_config->mask & (1 << VDPA_ATTR_DEV_NET_CFG_MACADDR)) {
+		memcpy(ndev->config.mac, add_config->net.mac, ETH_ALEN);
+	} else {
+		err = mlx5_query_nic_vport_mac_address(mdev, 0, 0, config->mac);
+		if (err)
+			goto err_mtu;
+	}
 
 	if (!is_zero_ether_addr(config->mac)) {
 		pfmdev = pci_get_drvdata(pci_physfn(mdev->pdev));
@@ -2541,6 +2546,7 @@ static int mlx5v_probe(struct auxiliary_device *adev,
 	mgtdev->mgtdev.ops = &mdev_ops;
 	mgtdev->mgtdev.device = mdev->device;
 	mgtdev->mgtdev.id_table = id_table;
+	mgtdev->mgtdev.config_attr_mask = (1 << VDPA_ATTR_DEV_NET_CFG_MACADDR);
 	mgtdev->madev = madev;
 
 	err = vdpa_mgmtdev_register(&mgtdev->mgtdev);
-- 
2.25.4

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH linux-next v4 8/8] vdpa/mlx5: Forward only packets with allowed MAC address

[Parav Pandit via Virtualization]

From: Eli Cohen <elic@nvidia.com>

Add rules to forward packets to the net device's TIR only if the
destination MAC is equal to the configured MAC. This is required to
prevent the netdevice from receiving traffic not destined to its
configured MAC.

Signed-off-by: Eli Cohen <elic@nvidia.com>
Reviewed-by: Parav Pandit <parav@nvidia.com>
---
 drivers/vdpa/mlx5/net/mlx5_vnet.c | 76 +++++++++++++++++++++++--------
 1 file changed, 58 insertions(+), 18 deletions(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 61f17ce0892e..e15c7f639490 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -158,7 +158,8 @@ struct mlx5_vdpa_net {
 	struct mutex reslock;
 	struct mlx5_flow_table *rxft;
 	struct mlx5_fc *rx_counter;
-	struct mlx5_flow_handle *rx_rule;
+	struct mlx5_flow_handle *rx_rule_ucast;
+	struct mlx5_flow_handle *rx_rule_mcast;
 	bool setup;
 	u16 mtu;
 	u32 cur_num_vqs;
@@ -1382,21 +1383,33 @@ static int add_fwd_to_tir(struct mlx5_vdpa_net *ndev)
 	struct mlx5_flow_table_attr ft_attr = {};
 	struct mlx5_flow_act flow_act = {};
 	struct mlx5_flow_namespace *ns;
+	struct mlx5_flow_spec *spec;
+	void *headers_c;
+	void *headers_v;
+	u8 *dmac_c;
+	u8 *dmac_v;
 	int err;
 
-	/* for now, one entry, match all, forward to tir */
-	ft_attr.max_fte = 1;
-	ft_attr.autogroup.max_num_groups = 1;
+	spec = kvzalloc(sizeof(*spec), GFP_KERNEL);
+	if (!spec)
+		return -ENOMEM;
+
+	spec->match_criteria_enable = MLX5_MATCH_OUTER_HEADERS;
+	ft_attr.max_fte = 2;
+	ft_attr.autogroup.max_num_groups = 2;
 
 	ns = mlx5_get_flow_namespace(ndev->mvdev.mdev, MLX5_FLOW_NAMESPACE_BYPASS);
 	if (!ns) {
-		mlx5_vdpa_warn(&ndev->mvdev, "get flow namespace\n");
-		return -EOPNOTSUPP;
+		mlx5_vdpa_warn(&ndev->mvdev, "failed to get flow namespace\n");
+		err = -EOPNOTSUPP;
+		goto err_ns;
 	}
 
 	ndev->rxft = mlx5_create_auto_grouped_flow_table(ns, &ft_attr);
-	if (IS_ERR(ndev->rxft))
-		return PTR_ERR(ndev->rxft);
+	if (IS_ERR(ndev->rxft)) {
+		err = PTR_ERR(ndev->rxft);
+		goto err_ns;
+	}
 
 	ndev->rx_counter = mlx5_fc_create(ndev->mvdev.mdev, false);
 	if (IS_ERR(ndev->rx_counter)) {
@@ -1404,37 +1417,64 @@ static int add_fwd_to_tir(struct mlx5_vdpa_net *ndev)
 		goto err_fc;
 	}
 
+	headers_c = MLX5_ADDR_OF(fte_match_param, spec->match_criteria, outer_headers);
+	dmac_c = MLX5_ADDR_OF(fte_match_param, headers_c, outer_headers.dmac_47_16);
+	memset(dmac_c, 0xff, ETH_ALEN);
+	headers_v = MLX5_ADDR_OF(fte_match_param, spec->match_value, outer_headers);
+	dmac_v = MLX5_ADDR_OF(fte_match_param, headers_v, outer_headers.dmac_47_16);
+	ether_addr_copy(dmac_v, ndev->config.mac);
+
 	flow_act.action = MLX5_FLOW_CONTEXT_ACTION_FWD_DEST | MLX5_FLOW_CONTEXT_ACTION_COUNT;
 	dest[0].type = MLX5_FLOW_DESTINATION_TYPE_TIR;
 	dest[0].tir_num = ndev->res.tirn;
 	dest[1].type = MLX5_FLOW_DESTINATION_TYPE_COUNTER;
 	dest[1].counter_id = mlx5_fc_id(ndev->rx_counter);
-	ndev->rx_rule = mlx5_add_flow_rules(ndev->rxft, NULL, &flow_act, dest, 2);
-	if (IS_ERR(ndev->rx_rule)) {
-		err = PTR_ERR(ndev->rx_rule);
-		ndev->rx_rule = NULL;
-		goto err_rule;
+	ndev->rx_rule_ucast = mlx5_add_flow_rules(ndev->rxft, spec, &flow_act, dest, 2);
+
+	if (IS_ERR(ndev->rx_rule_ucast)) {
+		err = PTR_ERR(ndev->rx_rule_ucast);
+		ndev->rx_rule_ucast = NULL;
+		goto err_rule_ucast;
+	}
+
+	memset(dmac_c, 0, ETH_ALEN);
+	memset(dmac_v, 0, ETH_ALEN);
+	dmac_c[0] = 1;
+	dmac_v[0] = 1;
+	flow_act.action = MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
+	ndev->rx_rule_mcast = mlx5_add_flow_rules(ndev->rxft, spec, &flow_act, dest, 1);
+	if (IS_ERR(ndev->rx_rule_mcast)) {
+		err = PTR_ERR(ndev->rx_rule_mcast);
+		ndev->rx_rule_mcast = NULL;
+		goto err_rule_mcast;
 	}
 
+	kvfree(spec);
 	return 0;
 
-err_rule:
+err_rule_mcast:
+	mlx5_del_flow_rules(ndev->rx_rule_ucast);
+	ndev->rx_rule_ucast = NULL;
+err_rule_ucast:
 	mlx5_fc_destroy(ndev->mvdev.mdev, ndev->rx_counter);
 err_fc:
 	mlx5_destroy_flow_table(ndev->rxft);
+err_ns:
+	kvfree(spec);
 	return err;
 }
 
 static void remove_fwd_to_tir(struct mlx5_vdpa_net *ndev)
 {
-	if (!ndev->rx_rule)
+	if (!ndev->rx_rule_ucast)
 		return;
 
-	mlx5_del_flow_rules(ndev->rx_rule);
+	mlx5_del_flow_rules(ndev->rx_rule_mcast);
+	ndev->rx_rule_mcast = NULL;
+	mlx5_del_flow_rules(ndev->rx_rule_ucast);
+	ndev->rx_rule_ucast = NULL;
 	mlx5_fc_destroy(ndev->mvdev.mdev, ndev->rx_counter);
 	mlx5_destroy_flow_table(ndev->rxft);
-
-	ndev->rx_rule = NULL;
 }
 
 static virtio_net_ctrl_ack handle_ctrl_mac(struct mlx5_vdpa_dev *mvdev, u8 cmd)
-- 
2.25.4

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v4 0/8] vdpa: enable user to set mac, mtu

[Michael S. Tsirkin]

On Thu, Oct 21, 2021 at 07:35:01PM +0300, Parav Pandit wrote:
> Currently user cannot view the vdpa device config space. Also user
> cannot set the mac address and mtu of the vdpa device.
> This patchset enables users to set the mac address and mtu of the vdpa
> device once the device is created.
> If a vendor driver supports such configuration user can set it otherwise
> user gets unsupported error.
> 
> vdpa mac address and mtu are device configuration layout fields.
> To keep interface generic enough for multiple types of vdpa devices, mac
> address and mtu setting is implemented as configuration layout fields.


So I got lots of "sha1 is lacking or useless" warnings with this.
I did my best to merge but please check out the result in
linux-next.

> An example of query & set of config layout fields for vdpa_sim_net
> driver:
> 
> Configuration layout fields are set when a vdpa device is created.
> 
> $ vdpa mgmtdev show
> vdpasim_net:
>   supported_classes net
> pci/0000:08:00.2:
>   supported_classes net
> 
> Add the device with MAC and MTU:
> $ vdpa dev add name bar mgmtdev vdpasim_net mac 00:11:22:33:44:66 mtu 1500
> 
> In above command only mac address or only mtu can also be set.
> 
> View the config after setting:
> $ vdpa dev config show
> bar: mac 00:11:22:33:44:66 link up link_announce false mtu 1500
> 
> Patch summary:
> Patch-1 introduced and use helpers for get/set config area
> Patch-2 implement query device config layout
> Patch-3 use kernel coding style for structure comment
> Patch-4 enanble user to set mac and mtu in config space
> Patch-5 vdpa_sim_net implements get and set of config layout
> Patch-6 mlx vdpa driver fix to avoid clearing VIRTIO_NET_F_MAC during
>         reset callback
> Patch-7 mlx5 vdpa driver supports user provided mac config
> Patch-8 mlx5 vdpa driver uses user provided mac during rx flow steering
> 
> changelog:
> v3->v4:
>  - enable setting mac and mtu of the vdpa device using creation time
>  - introduced a patch to fix mlx5 driver to avoid clearing
>    VIRTIO_NET_F_MAC
>  - introduced a patch to use kernel coding style for structure comment
>  - removed config attributes not used by sim and mlx5 net drivers
> v2->v3:
>  - dropped patches which are merged
>  - simplified code to handle non transitional devices
> 
> v1->v2:
>  - new patches to fix kdoc comment to add new kdoc section
>  - new patch to have synchronized access to features and config space
>  - read whole net config layout instead of individual fields
>  - added error extack for unmanaged vdpa device
>  - fixed several endianness issues
>  - introduced vdpa device ops for get config which is synchronized
>    with other get/set features ops and config ops
>  - fixed mtu range checking for max
>  - using NLA_POLICY_ETH_ADDR
>  - set config moved to device ops instead of mgmtdev ops
>  - merged build and set to single routine
>  - ensuring that user has NET_ADMIN capability for configuring network
>    attributes
>  - using updated interface and callbacks for get/set config
>  - following new api for config get/set for mgmt tool in mlx5 vdpa
>    driver
>  - fixes for accessing right SF dma device and bar address
>  - fix for mtu calculation
>  - fix for bit access in features
>  - fix for index restore with suspend/resume operation
> 
> 
> Eli Cohen (2):
>   vdpa/mlx5: Support configuration of MAC
>   vdpa/mlx5: Forward only packets with allowed MAC address
> 
> Parav Pandit (6):
>   vdpa: Introduce and use vdpa device get, set config helpers
>   vdpa: Introduce query of device config layout
>   vdpa: Use kernel coding style for structure comments
>   vdpa: Enable user to set mac and mtu of vdpa device
>   vdpa_sim_net: Enable user to set mac address and mtu
>   vdpa/mlx5: Fix clearing of VIRTIO_NET_F_MAC feature bit
> 
>  drivers/vdpa/ifcvf/ifcvf_main.c      |   3 +-
>  drivers/vdpa/mlx5/net/mlx5_vnet.c    |  92 +++++++---
>  drivers/vdpa/vdpa.c                  | 243 ++++++++++++++++++++++++++-
>  drivers/vdpa/vdpa_sim/vdpa_sim_blk.c |   3 +-
>  drivers/vdpa/vdpa_sim/vdpa_sim_net.c |  38 +++--
>  drivers/vdpa/vdpa_user/vduse_dev.c   |   3 +-
>  drivers/vhost/vdpa.c                 |   3 +-
>  include/linux/vdpa.h                 |  47 ++++--
>  include/uapi/linux/vdpa.h            |   6 +
>  9 files changed, 375 insertions(+), 63 deletions(-)
> 
> -- 
> 2.25.4

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH linux-next v4 0/8] vdpa: enable user to set mac, mtu

[Parav Pandit via Virtualization]

Hi Michael,

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Friday, October 22, 2021 4:11 PM
> 
> On Thu, Oct 21, 2021 at 07:35:01PM +0300, Parav Pandit wrote:
> > Currently user cannot view the vdpa device config space. Also user
> > cannot set the mac address and mtu of the vdpa device.
> > This patchset enables users to set the mac address and mtu of the vdpa
> > device once the device is created.
> > If a vendor driver supports such configuration user can set it
> > otherwise user gets unsupported error.
> >
> > vdpa mac address and mtu are device configuration layout fields.
> > To keep interface generic enough for multiple types of vdpa devices,
> > mac address and mtu setting is implemented as configuration layout fields.
> 
> 
> So I got lots of "sha1 is lacking or useless" warnings with this.
> I did my best to merge but please check out the result in linux-next.
>
I was able to test vdpasim_net on one system with rebase code with these patches.
On 2nd system I started verifying mlx5 vdpa.
System has mlx5 vdpa and virtio blk devices.
After rebase, I hit the below crash at system boot time on virtio blk device.

I have some internal issues in accessing crashed system, unable to proceed to verify it.
However, the crash doesn't seems to occur due to this patches, as its happening on virtio blk dev (non vdpa based blk dev).

[    1.238098] virtio_blk virtio0: [vda] 73400320 512-byte logical blocks (37.6 GB/35.0 GiB)
 [    1.240177] BUG: unable to handle page fault for address: 00000000fffedbeb
 [    1.240973] #PF: supervisor write access in kernel mode
 [    1.241590] #PF: error_code(0x0002) - not-present page
 [    1.242202] PGD 0 P4D 0 
 [    1.242559] Oops: 0002 [#1] SMP NOPTI
 [    1.243023] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.15.0-rc6-vdpa-mac+ #7
 [    1.243559] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
 [    1.243559] RIP: 0010:virtqueue_add_sgs+0x9e2/0xe50
 [    1.243559] Code: 0e 41 89 4d 40 49 8b 4d 78 48 89 1c 11 48 8b 5c 24 08 49 8b 4d 78 48 89 5c 11 08 49 8b 95 b0 00 00 00 48 85 d2 74 06 8b 1c 24 <89> 1c 82 41 8b 45 50 0f b7 5c 24 10 49 8b 55 60 83 e8 01 66 41 23
 [    1.243559] RSP: 0000:ffff88810092b5a8 EFLAGS: 00010006
 [    1.243559] RAX: 0000000000000000 RBX: 0000000000001001 RCX: ffff888104f91000
 [    1.243559] RDX: 00000000fffedbeb RSI: 0000000000000000 RDI: 0000000000000004
 [    1.243559] RBP: 0000000000000003 R08: 0000000000000004 R09: ffffffff8289920c
 [    1.243559] R10: 0000000000000003 R11: 0000000000000001 R12: 0000000000000030
 [    1.243559] R13: ffff88810547ef00 R14: 0000000000000001 R15: 0000000000000000
 [    1.243559] FS:  0000000000000000(0000) GS:ffff88846fa00000(0000) knlGS:0000000000000000
 [    1.243559] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 [    1.243559] CR2: 00000000fffedbeb CR3: 0000000002612001 CR4: 0000000000370eb0
 [    1.243559] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
 [    1.243559] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
 [    1.243559] Call Trace:
 [    1.243559]  virtio_queue_rq+0x1e4/0x5f0
 [    1.243559]  __blk_mq_try_issue_directly+0x138/0x1e0
 [    1.243559]  blk_mq_try_issue_directly+0x47/0xa0
 [    1.243559]  blk_mq_submit_bio+0x5af/0x890
 [    1.243559]  __submit_bio+0x2af/0x2e0
 [    1.243559]  ? create_page_buffers+0x55/0x70
 [    1.243559]  submit_bio_noacct+0x26c/0x2d0
 [    1.243559]  submit_bh_wbc.isra.0+0x122/0x150
 [    1.243559]  block_read_full_page+0x2f7/0x3f0
 [    1.243559]  ? blkdev_direct_IO+0x4b0/0x4b0
 [    1.243559]  ? lru_cache_add+0xcf/0x150
 [    1.243559]  do_read_cache_page+0x4f2/0x6a0
 [    1.243559]  ? lockdep_hardirqs_on_prepare+0xe4/0x190
 [    1.243559]  read_part_sector+0x39/0xd0
 [    1.243559]  read_lba+0xca/0x140
 [    1.243559]  efi_partition+0xe6/0x770
 [    1.243559]  ? snprintf+0x49/0x60
 [    1.243559]  ? is_gpt_valid.part.0+0x420/0x420
 [    1.243559]  bdev_disk_changed+0x21c/0x4a0
 [    1.243559]  blkdev_get_whole+0x76/0x90
 [    1.243559]  blkdev_get_by_dev+0xd4/0x2c0
 [    1.243559]  device_add_disk+0x351/0x390
 [    1.243559]  virtblk_probe+0x804/0xa40
 [    1.243559]  ? ncpus_cmp_func+0x10/0x10
 [    1.243559]  virtio_dev_probe+0x155/0x250
 [    1.243559]  really_probe+0xdb/0x3b0
 [    1.243559]  __driver_probe_device+0x8c/0x180
 [    1.243559]  driver_probe_device+0x1e/0xa0
 [    1.243559]  __driver_attach+0xaa/0x160
 [    1.243559]  ? __device_attach_driver+0x110/0x110
 [    1.243559]  ? __device_attach_driver+0x110/0x110
 [    1.243559]  bus_for_each_dev+0x7a/0xc0
 [    1.243559]  bus_add_driver+0x198/0x200
 [    1.243559]  driver_register+0x6c/0xc0
 [    1.243559]  ? loop_init+0x149/0x149
 [    1.243559]  init+0x52/0x7d
 [    1.243559]  do_one_initcall+0x63/0x300
 [    1.243559]  ? rcu_read_lock_sched_held+0x5d/0x90
 [    1.243559]  kernel_init_freeable+0x26a/0x2b2
 [    1.243559]  ? rest_init+0x2e0/0x2e0
 [    1.243559]  kernel_init+0x17/0x130
 [    1.243559]  ? rest_init+0x2e0/0x2e0
 [    1.243559]  ret_from_fork+0x1f/0x30
 [    1.243559] Modules linked in:
 [    1.243559] CR2: 00000000fffedbeb
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v4 0/8] vdpa: enable user to set mac, mtu

[Michael S. Tsirkin]

On Fri, Oct 22, 2021 at 03:07:32PM +0000, Parav Pandit wrote:
> Hi Michael,
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Friday, October 22, 2021 4:11 PM
> > 
> > On Thu, Oct 21, 2021 at 07:35:01PM +0300, Parav Pandit wrote:
> > > Currently user cannot view the vdpa device config space. Also user
> > > cannot set the mac address and mtu of the vdpa device.
> > > This patchset enables users to set the mac address and mtu of the vdpa
> > > device once the device is created.
> > > If a vendor driver supports such configuration user can set it
> > > otherwise user gets unsupported error.
> > >
> > > vdpa mac address and mtu are device configuration layout fields.
> > > To keep interface generic enough for multiple types of vdpa devices,
> > > mac address and mtu setting is implemented as configuration layout fields.
> > 
> > 
> > So I got lots of "sha1 is lacking or useless" warnings with this.
> > I did my best to merge but please check out the result in linux-next.
> >
> I was able to test vdpasim_net on one system with rebase code with these patches.
> On 2nd system I started verifying mlx5 vdpa.
> System has mlx5 vdpa and virtio blk devices.
> After rebase, I hit the below crash at system boot time on virtio blk device.
> 
> I have some internal issues in accessing crashed system, unable to proceed to verify it.
> However, the crash doesn't seems to occur due to this patches, as its happening on virtio blk dev (non vdpa based blk dev).
> 
> [    1.238098] virtio_blk virtio0: [vda] 73400320 512-byte logical blocks (37.6 GB/35.0 GiB)
>  [    1.240177] BUG: unable to handle page fault for address: 00000000fffedbeb
>  [    1.240973] #PF: supervisor write access in kernel mode
>  [    1.241590] #PF: error_code(0x0002) - not-present page
>  [    1.242202] PGD 0 P4D 0 
>  [    1.242559] Oops: 0002 [#1] SMP NOPTI
>  [    1.243023] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.15.0-rc6-vdpa-mac+ #7
>  [    1.243559] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
>  [    1.243559] RIP: 0010:virtqueue_add_sgs+0x9e2/0xe50
>  [    1.243559] Code: 0e 41 89 4d 40 49 8b 4d 78 48 89 1c 11 48 8b 5c 24 08 49 8b 4d 78 48 89 5c 11 08 49 8b 95 b0 00 00 00 48 85 d2 74 06 8b 1c 24 <89> 1c 82 41 8b 45 50 0f b7 5c 24 10 49 8b 55 60 83 e8 01 66 41 23
>  [    1.243559] RSP: 0000:ffff88810092b5a8 EFLAGS: 00010006
>  [    1.243559] RAX: 0000000000000000 RBX: 0000000000001001 RCX: ffff888104f91000
>  [    1.243559] RDX: 00000000fffedbeb RSI: 0000000000000000 RDI: 0000000000000004
>  [    1.243559] RBP: 0000000000000003 R08: 0000000000000004 R09: ffffffff8289920c
>  [    1.243559] R10: 0000000000000003 R11: 0000000000000001 R12: 0000000000000030
>  [    1.243559] R13: ffff88810547ef00 R14: 0000000000000001 R15: 0000000000000000
>  [    1.243559] FS:  0000000000000000(0000) GS:ffff88846fa00000(0000) knlGS:0000000000000000
>  [    1.243559] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>  [    1.243559] CR2: 00000000fffedbeb CR3: 0000000002612001 CR4: 0000000000370eb0
>  [    1.243559] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>  [    1.243559] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>  [    1.243559] Call Trace:
>  [    1.243559]  virtio_queue_rq+0x1e4/0x5f0
>  [    1.243559]  __blk_mq_try_issue_directly+0x138/0x1e0
>  [    1.243559]  blk_mq_try_issue_directly+0x47/0xa0
>  [    1.243559]  blk_mq_submit_bio+0x5af/0x890
>  [    1.243559]  __submit_bio+0x2af/0x2e0
>  [    1.243559]  ? create_page_buffers+0x55/0x70
>  [    1.243559]  submit_bio_noacct+0x26c/0x2d0
>  [    1.243559]  submit_bh_wbc.isra.0+0x122/0x150
>  [    1.243559]  block_read_full_page+0x2f7/0x3f0
>  [    1.243559]  ? blkdev_direct_IO+0x4b0/0x4b0
>  [    1.243559]  ? lru_cache_add+0xcf/0x150
>  [    1.243559]  do_read_cache_page+0x4f2/0x6a0
>  [    1.243559]  ? lockdep_hardirqs_on_prepare+0xe4/0x190
>  [    1.243559]  read_part_sector+0x39/0xd0
>  [    1.243559]  read_lba+0xca/0x140
>  [    1.243559]  efi_partition+0xe6/0x770
>  [    1.243559]  ? snprintf+0x49/0x60
>  [    1.243559]  ? is_gpt_valid.part.0+0x420/0x420
>  [    1.243559]  bdev_disk_changed+0x21c/0x4a0
>  [    1.243559]  blkdev_get_whole+0x76/0x90
>  [    1.243559]  blkdev_get_by_dev+0xd4/0x2c0
>  [    1.243559]  device_add_disk+0x351/0x390
>  [    1.243559]  virtblk_probe+0x804/0xa40
>  [    1.243559]  ? ncpus_cmp_func+0x10/0x10
>  [    1.243559]  virtio_dev_probe+0x155/0x250
>  [    1.243559]  really_probe+0xdb/0x3b0
>  [    1.243559]  __driver_probe_device+0x8c/0x180
>  [    1.243559]  driver_probe_device+0x1e/0xa0
>  [    1.243559]  __driver_attach+0xaa/0x160
>  [    1.243559]  ? __device_attach_driver+0x110/0x110
>  [    1.243559]  ? __device_attach_driver+0x110/0x110
>  [    1.243559]  bus_for_each_dev+0x7a/0xc0
>  [    1.243559]  bus_add_driver+0x198/0x200
>  [    1.243559]  driver_register+0x6c/0xc0
>  [    1.243559]  ? loop_init+0x149/0x149
>  [    1.243559]  init+0x52/0x7d
>  [    1.243559]  do_one_initcall+0x63/0x300
>  [    1.243559]  ? rcu_read_lock_sched_held+0x5d/0x90
>  [    1.243559]  kernel_init_freeable+0x26a/0x2b2
>  [    1.243559]  ? rest_init+0x2e0/0x2e0
>  [    1.243559]  kernel_init+0x17/0x130
>  [    1.243559]  ? rest_init+0x2e0/0x2e0
>  [    1.243559]  ret_from_fork+0x1f/0x30
>  [    1.243559] Modules linked in:
>  [    1.243559] CR2: 00000000fffedbeb

Thanks for the report. Parav, could you help bisect this please?
I also pushed out a new head with some patches dropped.
Could you test that maybe?

Thanks!

-- 
MST

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v4 0/8] vdpa: enable user to set mac, mtu

[Michael S. Tsirkin]

On Sat, Oct 23, 2021 at 04:03:12PM -0400, Michael S. Tsirkin wrote:
> On Fri, Oct 22, 2021 at 03:07:32PM +0000, Parav Pandit wrote:
> > Hi Michael,
> > 
> > > From: Michael S. Tsirkin <mst@redhat.com>
> > > Sent: Friday, October 22, 2021 4:11 PM
> > > 
> > > On Thu, Oct 21, 2021 at 07:35:01PM +0300, Parav Pandit wrote:
> > > > Currently user cannot view the vdpa device config space. Also user
> > > > cannot set the mac address and mtu of the vdpa device.
> > > > This patchset enables users to set the mac address and mtu of the vdpa
> > > > device once the device is created.
> > > > If a vendor driver supports such configuration user can set it
> > > > otherwise user gets unsupported error.
> > > >
> > > > vdpa mac address and mtu are device configuration layout fields.
> > > > To keep interface generic enough for multiple types of vdpa devices,
> > > > mac address and mtu setting is implemented as configuration layout fields.
> > > 
> > > 
> > > So I got lots of "sha1 is lacking or useless" warnings with this.
> > > I did my best to merge but please check out the result in linux-next.
> > >
> > I was able to test vdpasim_net on one system with rebase code with these patches.
> > On 2nd system I started verifying mlx5 vdpa.
> > System has mlx5 vdpa and virtio blk devices.
> > After rebase, I hit the below crash at system boot time on virtio blk device.
> > 
> > I have some internal issues in accessing crashed system, unable to proceed to verify it.
> > However, the crash doesn't seems to occur due to this patches, as its happening on virtio blk dev (non vdpa based blk dev).
> > 
> > [    1.238098] virtio_blk virtio0: [vda] 73400320 512-byte logical blocks (37.6 GB/35.0 GiB)
> >  [    1.240177] BUG: unable to handle page fault for address: 00000000fffedbeb
> >  [    1.240973] #PF: supervisor write access in kernel mode
> >  [    1.241590] #PF: error_code(0x0002) - not-present page
> >  [    1.242202] PGD 0 P4D 0 
> >  [    1.242559] Oops: 0002 [#1] SMP NOPTI
> >  [    1.243023] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.15.0-rc6-vdpa-mac+ #7
> >  [    1.243559] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
> >  [    1.243559] RIP: 0010:virtqueue_add_sgs+0x9e2/0xe50
> >  [    1.243559] Code: 0e 41 89 4d 40 49 8b 4d 78 48 89 1c 11 48 8b 5c 24 08 49 8b 4d 78 48 89 5c 11 08 49 8b 95 b0 00 00 00 48 85 d2 74 06 8b 1c 24 <89> 1c 82 41 8b 45 50 0f b7 5c 24 10 49 8b 55 60 83 e8 01 66 41 23
> >  [    1.243559] RSP: 0000:ffff88810092b5a8 EFLAGS: 00010006
> >  [    1.243559] RAX: 0000000000000000 RBX: 0000000000001001 RCX: ffff888104f91000
> >  [    1.243559] RDX: 00000000fffedbeb RSI: 0000000000000000 RDI: 0000000000000004
> >  [    1.243559] RBP: 0000000000000003 R08: 0000000000000004 R09: ffffffff8289920c
> >  [    1.243559] R10: 0000000000000003 R11: 0000000000000001 R12: 0000000000000030
> >  [    1.243559] R13: ffff88810547ef00 R14: 0000000000000001 R15: 0000000000000000
> >  [    1.243559] FS:  0000000000000000(0000) GS:ffff88846fa00000(0000) knlGS:0000000000000000
> >  [    1.243559] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >  [    1.243559] CR2: 00000000fffedbeb CR3: 0000000002612001 CR4: 0000000000370eb0
> >  [    1.243559] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> >  [    1.243559] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> >  [    1.243559] Call Trace:
> >  [    1.243559]  virtio_queue_rq+0x1e4/0x5f0
> >  [    1.243559]  __blk_mq_try_issue_directly+0x138/0x1e0
> >  [    1.243559]  blk_mq_try_issue_directly+0x47/0xa0
> >  [    1.243559]  blk_mq_submit_bio+0x5af/0x890
> >  [    1.243559]  __submit_bio+0x2af/0x2e0
> >  [    1.243559]  ? create_page_buffers+0x55/0x70
> >  [    1.243559]  submit_bio_noacct+0x26c/0x2d0
> >  [    1.243559]  submit_bh_wbc.isra.0+0x122/0x150
> >  [    1.243559]  block_read_full_page+0x2f7/0x3f0
> >  [    1.243559]  ? blkdev_direct_IO+0x4b0/0x4b0
> >  [    1.243559]  ? lru_cache_add+0xcf/0x150
> >  [    1.243559]  do_read_cache_page+0x4f2/0x6a0
> >  [    1.243559]  ? lockdep_hardirqs_on_prepare+0xe4/0x190
> >  [    1.243559]  read_part_sector+0x39/0xd0
> >  [    1.243559]  read_lba+0xca/0x140
> >  [    1.243559]  efi_partition+0xe6/0x770
> >  [    1.243559]  ? snprintf+0x49/0x60
> >  [    1.243559]  ? is_gpt_valid.part.0+0x420/0x420
> >  [    1.243559]  bdev_disk_changed+0x21c/0x4a0
> >  [    1.243559]  blkdev_get_whole+0x76/0x90
> >  [    1.243559]  blkdev_get_by_dev+0xd4/0x2c0
> >  [    1.243559]  device_add_disk+0x351/0x390
> >  [    1.243559]  virtblk_probe+0x804/0xa40
> >  [    1.243559]  ? ncpus_cmp_func+0x10/0x10
> >  [    1.243559]  virtio_dev_probe+0x155/0x250
> >  [    1.243559]  really_probe+0xdb/0x3b0
> >  [    1.243559]  __driver_probe_device+0x8c/0x180
> >  [    1.243559]  driver_probe_device+0x1e/0xa0
> >  [    1.243559]  __driver_attach+0xaa/0x160
> >  [    1.243559]  ? __device_attach_driver+0x110/0x110
> >  [    1.243559]  ? __device_attach_driver+0x110/0x110
> >  [    1.243559]  bus_for_each_dev+0x7a/0xc0
> >  [    1.243559]  bus_add_driver+0x198/0x200
> >  [    1.243559]  driver_register+0x6c/0xc0
> >  [    1.243559]  ? loop_init+0x149/0x149
> >  [    1.243559]  init+0x52/0x7d
> >  [    1.243559]  do_one_initcall+0x63/0x300
> >  [    1.243559]  ? rcu_read_lock_sched_held+0x5d/0x90
> >  [    1.243559]  kernel_init_freeable+0x26a/0x2b2
> >  [    1.243559]  ? rest_init+0x2e0/0x2e0
> >  [    1.243559]  kernel_init+0x17/0x130
> >  [    1.243559]  ? rest_init+0x2e0/0x2e0
> >  [    1.243559]  ret_from_fork+0x1f/0x30
> >  [    1.243559] Modules linked in:
> >  [    1.243559] CR2: 00000000fffedbeb
> 
> Thanks for the report. Parav, could you help bisect this please?
> I also pushed out a new head with some patches dropped.
> Could you test that maybe?
> 
> Thanks!

OK I think it's due to Jason's change to virtio blk. I dropped that
patch for now. Could you try again with the new head please?


> -- 
> MST

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH linux-next v4 0/8] vdpa: enable user to set mac, mtu

[Parav Pandit via Virtualization]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Sunday, October 24, 2021 2:46 AM
[..]

> > >  [    1.243559] Call Trace:
> > >  [    1.243559]  virtio_queue_rq+0x1e4/0x5f0
> > >  [    1.243559]  __blk_mq_try_issue_directly+0x138/0x1e0
> > >  [    1.243559]  blk_mq_try_issue_directly+0x47/0xa0
> > >  [    1.243559]  blk_mq_submit_bio+0x5af/0x890
> > >  [    1.243559]  __submit_bio+0x2af/0x2e0
> > >  [    1.243559]  ? create_page_buffers+0x55/0x70
> > >  [    1.243559]  submit_bio_noacct+0x26c/0x2d0
> > >  [    1.243559]  submit_bh_wbc.isra.0+0x122/0x150
> > >  [    1.243559]  block_read_full_page+0x2f7/0x3f0
> > >  [    1.243559]  ? blkdev_direct_IO+0x4b0/0x4b0
> > >  [    1.243559]  ? lru_cache_add+0xcf/0x150
> > >  [    1.243559]  do_read_cache_page+0x4f2/0x6a0
> > >  [    1.243559]  ? lockdep_hardirqs_on_prepare+0xe4/0x190
> > >  [    1.243559]  read_part_sector+0x39/0xd0
> > >  [    1.243559]  read_lba+0xca/0x140
> > >  [    1.243559]  efi_partition+0xe6/0x770
> > >  [    1.243559]  ? snprintf+0x49/0x60
> > >  [    1.243559]  ? is_gpt_valid.part.0+0x420/0x420
> > >  [    1.243559]  bdev_disk_changed+0x21c/0x4a0
> > >  [    1.243559]  blkdev_get_whole+0x76/0x90
> > >  [    1.243559]  blkdev_get_by_dev+0xd4/0x2c0
> > >  [    1.243559]  device_add_disk+0x351/0x390
> > >  [    1.243559]  virtblk_probe+0x804/0xa40
> > >  [    1.243559]  ? ncpus_cmp_func+0x10/0x10
> > >  [    1.243559]  virtio_dev_probe+0x155/0x250
> > >  [    1.243559]  really_probe+0xdb/0x3b0
> > >  [    1.243559]  __driver_probe_device+0x8c/0x180
> > >  [    1.243559]  driver_probe_device+0x1e/0xa0
> > >  [    1.243559]  __driver_attach+0xaa/0x160
> > >  [    1.243559]  ? __device_attach_driver+0x110/0x110
> > >  [    1.243559]  ? __device_attach_driver+0x110/0x110
> > >  [    1.243559]  bus_for_each_dev+0x7a/0xc0
> > >  [    1.243559]  bus_add_driver+0x198/0x200
> > >  [    1.243559]  driver_register+0x6c/0xc0
> > >  [    1.243559]  ? loop_init+0x149/0x149
> > >  [    1.243559]  init+0x52/0x7d
> > >  [    1.243559]  do_one_initcall+0x63/0x300
> > >  [    1.243559]  ? rcu_read_lock_sched_held+0x5d/0x90
> > >  [    1.243559]  kernel_init_freeable+0x26a/0x2b2
> > >  [    1.243559]  ? rest_init+0x2e0/0x2e0
> > >  [    1.243559]  kernel_init+0x17/0x130
> > >  [    1.243559]  ? rest_init+0x2e0/0x2e0
> > >  [    1.243559]  ret_from_fork+0x1f/0x30
> > >  [    1.243559] Modules linked in:
> > >  [    1.243559] CR2: 00000000fffedbeb
> >
> > Thanks for the report. Parav, could you help bisect this please?
> > I also pushed out a new head with some patches dropped.
> > Could you test that maybe?
> >
> > Thanks!
> 
> OK I think it's due to Jason's change to virtio blk. I dropped that patch for now.
> Could you try again with the new head please?

Yes I will try today as soon as I recover my server access and update.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v4 1/8] vdpa: Introduce and use vdpa device get, set config helpers

[Jason Wang]

在 2021/10/22 上午12:35, Parav Pandit 写道:
> Subsequent patches enable get and set configuration either
> via management device or via vdpa device' config ops.
>
> This requires synchronization between multiple callers to get and set
> config callbacks. Features setting also influence the layout of the
> configuration fields endianness.
>
> To avoid exposing synchronization primitives to callers, introduce
> helper for setting the configuration and use it.
>
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> Reviewed-by: Eli Cohen <elic@nvidia.com>
> Acked-by: Jason Wang <jasowang@redhat.com>
> ---
>   drivers/vdpa/vdpa.c  | 36 ++++++++++++++++++++++++++++++++++++
>   drivers/vhost/vdpa.c |  3 +--
>   include/linux/vdpa.h | 19 ++++---------------


Do we need to change virtio_vdpa_set() as well?

Thanks


>   3 files changed, 41 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> index 1dc121a07a93..6fdfc11ecb13 100644
> --- a/drivers/vdpa/vdpa.c
> +++ b/drivers/vdpa/vdpa.c
> @@ -289,6 +289,42 @@ void vdpa_mgmtdev_unregister(struct vdpa_mgmt_dev *mdev)
>   }
>   EXPORT_SYMBOL_GPL(vdpa_mgmtdev_unregister);
>   
> +/**
> + * vdpa_get_config - Get one or more device configuration fields.
> + * @vdev: vdpa device to operate on
> + * @offset: starting byte offset of the field
> + * @buf: buffer pointer to read to
> + * @len: length of the configuration fields in bytes
> + */
> +void vdpa_get_config(struct vdpa_device *vdev, unsigned int offset,
> +		     void *buf, unsigned int len)
> +{
> +	const struct vdpa_config_ops *ops = vdev->config;
> +
> +	/*
> +	 * Config accesses aren't supposed to trigger before features are set.
> +	 * If it does happen we assume a legacy guest.
> +	 */
> +	if (!vdev->features_valid)
> +		vdpa_set_features(vdev, 0);
> +	ops->get_config(vdev, offset, buf, len);
> +}
> +EXPORT_SYMBOL_GPL(vdpa_get_config);
> +
> +/**
> + * vdpa_set_config - Set one or more device configuration fields.
> + * @vdev: vdpa device to operate on
> + * @offset: starting byte offset of the field
> + * @buf: buffer pointer to read from
> + * @length: length of the configuration fields in bytes
> + */
> +void vdpa_set_config(struct vdpa_device *vdev, unsigned int offset,
> +		     void *buf, unsigned int length)
> +{
> +	vdev->config->set_config(vdev, offset, buf, length);
> +}
> +EXPORT_SYMBOL_GPL(vdpa_set_config);
> +
>   static bool mgmtdev_handle_match(const struct vdpa_mgmt_dev *mdev,
>   				 const char *busname, const char *devname)
>   {
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index 39039e046117..01c59ce7e250 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -237,7 +237,6 @@ static long vhost_vdpa_set_config(struct vhost_vdpa *v,
>   				  struct vhost_vdpa_config __user *c)
>   {
>   	struct vdpa_device *vdpa = v->vdpa;
> -	const struct vdpa_config_ops *ops = vdpa->config;
>   	struct vhost_vdpa_config config;
>   	unsigned long size = offsetof(struct vhost_vdpa_config, buf);
>   	u8 *buf;
> @@ -251,7 +250,7 @@ static long vhost_vdpa_set_config(struct vhost_vdpa *v,
>   	if (IS_ERR(buf))
>   		return PTR_ERR(buf);
>   
> -	ops->set_config(vdpa, config.off, buf, config.len);
> +	vdpa_set_config(vdpa, config.off, buf, config.len);
>   
>   	kvfree(buf);
>   	return 0;
> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> index 3972ab765de1..78395331a166 100644
> --- a/include/linux/vdpa.h
> +++ b/include/linux/vdpa.h
> @@ -382,21 +382,10 @@ static inline int vdpa_set_features(struct vdpa_device *vdev, u64 features)
>   	return ops->set_features(vdev, features);
>   }
>   
> -static inline void vdpa_get_config(struct vdpa_device *vdev,
> -				   unsigned int offset, void *buf,
> -				   unsigned int len)
> -{
> -	const struct vdpa_config_ops *ops = vdev->config;
> -
> -	/*
> -	 * Config accesses aren't supposed to trigger before features are set.
> -	 * If it does happen we assume a legacy guest.
> -	 */
> -	if (!vdev->features_valid)
> -		vdpa_set_features(vdev, 0);
> -	ops->get_config(vdev, offset, buf, len);
> -}
> -
> +void vdpa_get_config(struct vdpa_device *vdev, unsigned int offset,
> +		     void *buf, unsigned int len);
> +void vdpa_set_config(struct vdpa_device *dev, unsigned int offset,
> +		     void *buf, unsigned int length);
>   /**
>    * struct vdpa_mgmtdev_ops - vdpa device ops
>    * @dev_add: Add a vdpa device using alloc and register

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v4 2/8] vdpa: Introduce query of device config layout

[Jason Wang]

在 2021/10/22 上午12:35, Parav Pandit 写道:
> Introduce a command to query a device config layout.
>
> An example query of network vdpa device:
>
> $ vdpa dev add name bar mgmtdev vdpasim_net
>
> $ vdpa dev config show
> bar: mac 00:35:09:19:48:05 link up link_announce false mtu 1500


Nit: it looks to me this patch doesn't expose link_announce but 
max_virtqueue_pairs.

Other looks good.

Thanks


>
> $ vdpa dev config show -jp
> {
>      "config": {
>          "bar": {
>              "mac": "00:35:09:19:48:05",
>              "link ": "up",
>              "link_announce ": false,
>              "mtu": 1500,
>          }
>      }
> }
>
> Signed-off-by: Parav Pandit<parav@nvidia.com>
> Signed-off-by: Eli Cohen<elic@nvidia.com>
>
> ---
> changelog:
> v3->v4:
>   - removed config space fields reporting which are not used by mlx5
>     and sim drivers

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v4 3/8] vdpa: Use kernel coding style for structure comments

[Jason Wang]

在 2021/10/22 上午12:35, Parav Pandit 写道:
> As subsequent patch adds new structure field with comment, move the
> structure comment to follow kernel coding style.
>
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> Reviewed-by: Eli Cohen <elic@nvidia.com>


Acked-by: Jason Wang <jasowang@redhat.com>


> ---
>   include/linux/vdpa.h | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> index 9326020a2c55..111153c9ee71 100644
> --- a/include/linux/vdpa.h
> +++ b/include/linux/vdpa.h
> @@ -407,10 +407,17 @@ struct vdpa_mgmtdev_ops {
>   	void (*dev_del)(struct vdpa_mgmt_dev *mdev, struct vdpa_device *dev);
>   };
>   
> +/**
> + * struct vdpa_mgmt_dev - vdpa management device
> + * @device: Management parent device
> + * @ops: operations supported by management device
> + * @id_table: Pointer to device id table of supported ids
> + * @list: list entry
> + */
>   struct vdpa_mgmt_dev {
>   	struct device *device;
>   	const struct vdpa_mgmtdev_ops *ops;
> -	const struct virtio_device_id *id_table; /* supported ids */
> +	const struct virtio_device_id *id_table;
>   	struct list_head list;
>   };
>   

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH linux-next v4 2/8] vdpa: Introduce query of device config layout

[Parav Pandit via Virtualization]

> From: Jason Wang <jasowang@redhat.com>
> Sent: Monday, October 25, 2021 11:36 AM
> 
> 
> 在 2021/10/22 上午12:35, Parav Pandit 写道:
> > Introduce a command to query a device config layout.
> >
> > An example query of network vdpa device:
> >
> > $ vdpa dev add name bar mgmtdev vdpasim_net
> >
> > $ vdpa dev config show
> > bar: mac 00:35:09:19:48:05 link up link_announce false mtu 1500
> 
> 
> Nit: it looks to me this patch doesn't expose link_announce but
> max_virtqueue_pairs.
It does expose the net status field that contains the link_announce.
I missed to update the example for including max_vq_pairs.
> 
> Other looks good.
> 
> Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

[Jason Wang]

在 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@nvidia.com>
> Reviewed-by: Eli Cohen <elic@nvidia.com>
> ---
> 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?

Another question is that, do need the cap if not attrs were specified?


> +
>   	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?


> +	u64 mask;
> +};
> +
>   /**
>    * Corresponding file area for device memory mapping
>    * @file: vma->vm_file for the mapping
> @@ -393,6 +403,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.
> @@ -403,7 +414,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);
>   };
>   
> @@ -412,12 +424,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
>    * @list: list entry
>    */
>   struct vdpa_mgmt_dev {
>   	struct device *device;
>   	const struct vdpa_mgmtdev_ops *ops;
>   	const struct virtio_device_id *id_table;
> +	u64 config_attr_mask;
>   	struct list_head list;
>   };
>   

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v4 5/8] vdpa_sim_net: Enable user to set mac address and mtu

[Jason Wang]

在 2021/10/22 上午12:35, Parav Pandit 写道:
> Enable user to set the mac address and mtu so that each vdpa device
> can have its own user specified mac address and mtu.
> This is done by implementing the management device's configuration
> layout fields setting callback routine.
>
> Now that user is enabled to set the mac address, remove the module
> parameter for same.
>
> And example of setting mac addr and mtu:
> $ vdpa mgmtdev show
>
> $ vdpa dev add name bar mgmtdev vdpasim_net
> $ vdpa dev config set bar mac 00:11:22:33:44:55 mtu 9000


The attributes were set during dev add if I was not wrong.

Thanks


_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v4 6/8] vdpa/mlx5: Fix clearing of VIRTIO_NET_F_MAC feature bit

[Jason Wang]

在 2021/10/22 上午12:35, Parav Pandit 写道:
> Cited patch in the fixes tag clears the features bit during reset.
> mlx5 vdpa device feature bits are static decided by device capabilities.


This is not what I read at least from mlx5_vdpa_get_features:

static u64 mlx5_vdpa_get_features(struct vdpa_device *vdev)
{
         struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
         struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
         u16 dev_features;

         dev_features = MLX5_CAP_DEV_VDPA_EMULATION(mvdev->mdev, 
device_features_bits_mask);
         ndev->mvdev.mlx_features |= mlx_to_vritio_features(dev_features);
         if (MLX5_CAP_DEV_VDPA_EMULATION(mvdev->mdev, virtio_version_1_0))
                 ndev->mvdev.mlx_features |= BIT_ULL(VIRTIO_F_VERSION_1);
         ndev->mvdev.mlx_features |= BIT_ULL(VIRTIO_F_ACCESS_PLATFORM);
         ndev->mvdev.mlx_features |= BIT_ULL(VIRTIO_NET_F_CTRL_VQ);
         ndev->mvdev.mlx_features |= BIT_ULL(VIRTIO_NET_F_CTRL_MAC_ADDR);
         ndev->mvdev.mlx_features |= BIT_ULL(VIRTIO_NET_F_MQ);

         print_features(mvdev, ndev->mvdev.mlx_features, false);
         return ndev->mvdev.mlx_features;
}


Thanks


>
> Clearing features bit cleared the VIRTIO_NET_F_MAC. Due to this MAC address
> provided by the device is not honored.
>
> Fix it by not clearing the static feature bits during reset.
>
> Fixes: 0686082dbf7a ("vdpa: Add reset callback in vdpa_config_ops")
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> Reviewed-by: Eli Cohen <elic@nvidia.com>
> ---
>   drivers/vdpa/mlx5/net/mlx5_vnet.c | 1 -
>   1 file changed, 1 deletion(-)
>
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index ca05f69054b6..0a2b79887085 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -2192,7 +2192,6 @@ static int mlx5_vdpa_reset(struct vdpa_device *vdev)
>   	clear_vqs_ready(ndev);
>   	mlx5_vdpa_destroy_mr(&ndev->mvdev);
>   	ndev->mvdev.status = 0;
> -	ndev->mvdev.mlx_features = 0;
>   	memset(ndev->event_cbs, 0, sizeof(ndev->event_cbs));
>   	ndev->mvdev.actual_features = 0;
>   	++mvdev->generation;

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

[Parav Pandit via Virtualization]

> From: Jason Wang <jasowang@redhat.com>
> 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@nvidia.com>
> > Reviewed-by: Eli Cohen <elic@nvidia.com>
> > ---
> > 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.

> 
> 
> > +
> >   	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@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v4 7/8] vdpa/mlx5: Support configuration of MAC

[Jason Wang]

在 2021/10/22 上午12:35, Parav Pandit 写道:
> From: Eli Cohen <elic@nvidia.com>
>
> Add code to accept MAC configuration through vdpa tool. The MAC is
> written into the config struct and later can be retrieved through
> get_config().
>
> Examples:
> 1. Configure MAC while adding a device:
> $ vdpa dev add mgmtdev pci/0000:06:00.2 name vdpa0 mac 00:11:22:33:44:55
>
> 2. Show configured params:
> $ vdpa dev config show
> vdpa0: mac 00:11:22:33:44:55 link down link_announce false max_vq_pairs 8 mtu 1500
>
> Signed-off-by: Eli Cohen <elic@nvidia.com>
> Reviewed-by: Parav Pandit <parav@nvidia.com>


Acked-by: Jason Wang <jasowang@redhat.com>


> ---
>   drivers/vdpa/mlx5/net/mlx5_vnet.c | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index 0a2b79887085..61f17ce0892e 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -6,6 +6,7 @@
>   #include <linux/vringh.h>
>   #include <uapi/linux/virtio_net.h>
>   #include <uapi/linux/virtio_ids.h>
> +#include <uapi/linux/vdpa.h>
>   #include <linux/virtio_config.h>
>   #include <linux/auxiliary_bus.h>
>   #include <linux/mlx5/cq.h>
> @@ -2444,9 +2445,13 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
>   	if (err)
>   		goto err_mtu;
>   
> -	err = mlx5_query_nic_vport_mac_address(mdev, 0, 0, config->mac);
> -	if (err)
> -		goto err_mtu;
> +	if (add_config->mask & (1 << VDPA_ATTR_DEV_NET_CFG_MACADDR)) {
> +		memcpy(ndev->config.mac, add_config->net.mac, ETH_ALEN);
> +	} else {
> +		err = mlx5_query_nic_vport_mac_address(mdev, 0, 0, config->mac);
> +		if (err)
> +			goto err_mtu;
> +	}
>   
>   	if (!is_zero_ether_addr(config->mac)) {
>   		pfmdev = pci_get_drvdata(pci_physfn(mdev->pdev));
> @@ -2541,6 +2546,7 @@ static int mlx5v_probe(struct auxiliary_device *adev,
>   	mgtdev->mgtdev.ops = &mdev_ops;
>   	mgtdev->mgtdev.device = mdev->device;
>   	mgtdev->mgtdev.id_table = id_table;
> +	mgtdev->mgtdev.config_attr_mask = (1 << VDPA_ATTR_DEV_NET_CFG_MACADDR);
>   	mgtdev->madev = madev;
>   
>   	err = vdpa_mgmtdev_register(&mgtdev->mgtdev);

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH linux-next v4 6/8] vdpa/mlx5: Fix clearing of VIRTIO_NET_F_MAC feature bit

[Parav Pandit via Virtualization]

> From: Jason Wang <jasowang@redhat.com>
> Sent: Monday, October 25, 2021 12:35 PM
> 
> 在 2021/10/22 上午12:35, Parav Pandit 写道:
> > Cited patch in the fixes tag clears the features bit during reset.
> > mlx5 vdpa device feature bits are static decided by device capabilities.
> 
> 
> This is not what I read at least from mlx5_vdpa_get_features:
> 
> static u64 mlx5_vdpa_get_features(struct vdpa_device *vdev) {
>          struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
>          struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
>          u16 dev_features;
> 
>          dev_features = MLX5_CAP_DEV_VDPA_EMULATION(mvdev->mdev,
> device_features_bits_mask);
Not sure I follow. Feature bits are decided by the device capabilities exposed by the MLX5_CAP_DEV_VDPA_EMULATION.
Other fields below are pretty much static.

>          ndev->mvdev.mlx_features |= mlx_to_vritio_features(dev_features);
>          if (MLX5_CAP_DEV_VDPA_EMULATION(mvdev->mdev,
> virtio_version_1_0))
>                  ndev->mvdev.mlx_features |= BIT_ULL(VIRTIO_F_VERSION_1);
>          ndev->mvdev.mlx_features |= BIT_ULL(VIRTIO_F_ACCESS_PLATFORM);
>          ndev->mvdev.mlx_features |= BIT_ULL(VIRTIO_NET_F_CTRL_VQ);
>          ndev->mvdev.mlx_features |=
> BIT_ULL(VIRTIO_NET_F_CTRL_MAC_ADDR);
>          ndev->mvdev.mlx_features |= BIT_ULL(VIRTIO_NET_F_MQ);
> 
>          print_features(mvdev, ndev->mvdev.mlx_features, false);
>          return ndev->mvdev.mlx_features; }
> 
> 
> Thanks
> 
> 
> >
> > Clearing features bit cleared the VIRTIO_NET_F_MAC. Due to this MAC
> > address provided by the device is not honored.
> >
> > Fix it by not clearing the static feature bits during reset.
> >
> > Fixes: 0686082dbf7a ("vdpa: Add reset callback in vdpa_config_ops")
> > Signed-off-by: Parav Pandit <parav@nvidia.com>
> > Reviewed-by: Eli Cohen <elic@nvidia.com>
> > ---
> >   drivers/vdpa/mlx5/net/mlx5_vnet.c | 1 -
> >   1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > index ca05f69054b6..0a2b79887085 100644
> > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > @@ -2192,7 +2192,6 @@ static int mlx5_vdpa_reset(struct vdpa_device
> *vdev)
> >   	clear_vqs_ready(ndev);
> >   	mlx5_vdpa_destroy_mr(&ndev->mvdev);
> >   	ndev->mvdev.status = 0;
> > -	ndev->mvdev.mlx_features = 0;
> >   	memset(ndev->event_cbs, 0, sizeof(ndev->event_cbs));
> >   	ndev->mvdev.actual_features = 0;
> >   	++mvdev->generation;

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH linux-next v4 5/8] vdpa_sim_net: Enable user to set mac address and mtu

[Parav Pandit via Virtualization]

> From: Jason Wang <jasowang@redhat.com>
> Sent: Monday, October 25, 2021 12:32 PM
> 
> 
> 在 2021/10/22 上午12:35, Parav Pandit 写道:
> > Enable user to set the mac address and mtu so that each vdpa device
> > can have its own user specified mac address and mtu.
> > This is done by implementing the management device's configuration
> > layout fields setting callback routine.
> >
> > Now that user is enabled to set the mac address, remove the module
> > parameter for same.
> >
> > And example of setting mac addr and mtu:
> > $ vdpa mgmtdev show
> >
> > $ vdpa dev add name bar mgmtdev vdpasim_net $ vdpa dev config set bar
> > mac 00:11:22:33:44:55 mtu 9000
> 
> 
> The attributes were set during dev add if I was not wrong.
> 
You are right. I missed to update the example here. Code is just fine.

> Thanks
> 

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH linux-next v4 0/8] vdpa: enable user to set mac, mtu

[Parav Pandit via Virtualization]

Hi Michael,

> From: Parav Pandit
> Sent: Monday, October 25, 2021 9:14 AM


> > > Thanks for the report. Parav, could you help bisect this please?
> > > I also pushed out a new head with some patches dropped.
> > > Could you test that maybe?
> > >
> > > Thanks!
> >
> > OK I think it's due to Jason's change to virtio blk. I dropped that patch for now.
> > Could you try again with the new head please?
> 
> Yes I will try today as soon as I recover my server access and update.

I verified the mlx5 and vdpa sim net with rebased tree. No kernel code trace seen.

The last commit in the tree that I verified is below.

commit 2b109044b081148b58974f5696ffd4383c3e9abb
Author: Michael S. Tsirkin <mst@redhat.com>
Date:   Sun Oct 24 09:41:40 2021 -0400

    virtio_blk: allow 0 as num_request_queues

    The default value is 0 meaning "no limit". However if 0
    is specified on the command line it is instead silently
    converted to 1. Further, the value is already validated
    at point of use, there's no point in duplicating code
    validating the value when it is set.

    Simplify the code while making the behaviour more consistent
    by using plain module_param.

    Fixes: 1a662cf6cb9a ("virtio-blk: add num_request_queues module parameter")
    Cc: Max Gurtovoy <mgurtovoy@nvidia.com>
    Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

[Michael S. Tsirkin]

On Mon, Oct 25, 2021 at 07:06:42AM +0000, Parav Pandit wrote:
> 
> 
> > From: Jason Wang <jasowang@redhat.com>
> > 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@nvidia.com>
> > > Reviewed-by: Eli Cohen <elic@nvidia.com>
> > > ---
> > > 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.

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.

> > 
> > 
> > > +
> > >   	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@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v4 5/8] vdpa_sim_net: Enable user to set mac address and mtu

[Michael S. Tsirkin]

On Mon, Oct 25, 2021 at 07:11:23AM +0000, Parav Pandit wrote:
> 
> 
> > From: Jason Wang <jasowang@redhat.com>
> > Sent: Monday, October 25, 2021 12:32 PM
> > 
> > 
> > 在 2021/10/22 上午12:35, Parav Pandit 写道:
> > > Enable user to set the mac address and mtu so that each vdpa device
> > > can have its own user specified mac address and mtu.
> > > This is done by implementing the management device's configuration
> > > layout fields setting callback routine.
> > >
> > > Now that user is enabled to set the mac address, remove the module
> > > parameter for same.
> > >
> > > And example of setting mac addr and mtu:
> > > $ vdpa mgmtdev show
> > >
> > > $ vdpa dev add name bar mgmtdev vdpasim_net $ vdpa dev config set bar
> > > mac 00:11:22:33:44:55 mtu 9000
> > 
> > 
> > The attributes were set during dev add if I was not wrong.
> > 
> You are right. I missed to update the example here. Code is just fine.
> 
> > Thanks
> > 

Want to send a corrected commit log?

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH linux-next v4 5/8] vdpa_sim_net: Enable user to set mac address and mtu

[Parav Pandit via Virtualization]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Monday, October 25, 2021 1:39 PM
> 
> On Mon, Oct 25, 2021 at 07:11:23AM +0000, Parav Pandit wrote:
> >
> >
> > > From: Jason Wang <jasowang@redhat.com>
> > > Sent: Monday, October 25, 2021 12:32 PM
> > >
> > >
> > > 在 2021/10/22 上午12:35, Parav Pandit 写道:
> > > > Enable user to set the mac address and mtu so that each vdpa
> > > > device can have its own user specified mac address and mtu.
> > > > This is done by implementing the management device's configuration
> > > > layout fields setting callback routine.
> > > >
> > > > Now that user is enabled to set the mac address, remove the module
> > > > parameter for same.
> > > >
> > > > And example of setting mac addr and mtu:
> > > > $ vdpa mgmtdev show
> > > >
> > > > $ vdpa dev add name bar mgmtdev vdpasim_net $ vdpa dev config set
> > > > bar mac 00:11:22:33:44:55 mtu 9000
> > >
> > >
> > > The attributes were set during dev add if I was not wrong.
> > >
> > You are right. I missed to update the example here. Code is just fine.
> >
> > > Thanks
> > >
> 
> Want to send a corrected commit log?
Yes. would like to send v5 with the corrected commit log and ACK from Jason on other patches.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v4 2/8] vdpa: Introduce query of device config layout

[Michael S. Tsirkin]

On Mon, Oct 25, 2021 at 06:59:56AM +0000, Parav Pandit wrote:
> 
> 
> > From: Jason Wang <jasowang@redhat.com>
> > Sent: Monday, October 25, 2021 11:36 AM
> > 
> > 
> > 在 2021/10/22 上午12:35, Parav Pandit 写道:
> > > Introduce a command to query a device config layout.
> > >
> > > An example query of network vdpa device:
> > >
> > > $ vdpa dev add name bar mgmtdev vdpasim_net
> > >
> > > $ vdpa dev config show
> > > bar: mac 00:35:09:19:48:05 link up link_announce false mtu 1500
> > 
> > 
> > Nit: it looks to me this patch doesn't expose link_announce but
> > max_virtqueue_pairs.
> It does expose the net status field that contains the link_announce.
> I missed to update the example for including max_vq_pairs.
> > 
> > Other looks good.
> > 
> > Thanks

Post a corrected commit log?

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v4 5/8] vdpa_sim_net: Enable user to set mac address and mtu

[Michael S. Tsirkin]

On Mon, Oct 25, 2021 at 08:10:15AM +0000, Parav Pandit wrote:
> 
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Monday, October 25, 2021 1:39 PM
> > 
> > On Mon, Oct 25, 2021 at 07:11:23AM +0000, Parav Pandit wrote:
> > >
> > >
> > > > From: Jason Wang <jasowang@redhat.com>
> > > > Sent: Monday, October 25, 2021 12:32 PM
> > > >
> > > >
> > > > 在 2021/10/22 上午12:35, Parav Pandit 写道:
> > > > > Enable user to set the mac address and mtu so that each vdpa
> > > > > device can have its own user specified mac address and mtu.
> > > > > This is done by implementing the management device's configuration
> > > > > layout fields setting callback routine.
> > > > >
> > > > > Now that user is enabled to set the mac address, remove the module
> > > > > parameter for same.
> > > > >
> > > > > And example of setting mac addr and mtu:
> > > > > $ vdpa mgmtdev show
> > > > >
> > > > > $ vdpa dev add name bar mgmtdev vdpasim_net $ vdpa dev config set
> > > > > bar mac 00:11:22:33:44:55 mtu 9000
> > > >
> > > >
> > > > The attributes were set during dev add if I was not wrong.
> > > >
> > > You are right. I missed to update the example here. Code is just fine.
> > >
> > > > Thanks
> > > >
> > 
> > Want to send a corrected commit log?
> Yes. would like to send v5 with the corrected commit log and ACK from Jason on other patches.

Go ahead, I will pick it up if merge window does not open yet.

There was also one comment by Jason you didn't resolve patch 1 I think?.
And it can't hurt to add a comment near the capability check
explaining why don't check if no attributes.

-- 
MST

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

[Parav Pandit via Virtualization]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Monday, October 25, 2021 1:38 PM
> 
> 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.
> 
I will keep the NET_ADMIN as it is doing the interface config.

And also add the comment around this check as you suggest in other email.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH linux-next v4 1/8] vdpa: Introduce and use vdpa device get, set config helpers

[Parav Pandit via Virtualization]

> From: Jason Wang <jasowang@redhat.com>
> Sent: Monday, October 25, 2021 11:33 AM
> 
> 在 2021/10/22 上午12:35, Parav Pandit 写道:
> > Subsequent patches enable get and set configuration either via
> > management device or via vdpa device' config ops.
> >
> > This requires synchronization between multiple callers to get and set
> > config callbacks. Features setting also influence the layout of the
> > configuration fields endianness.
> >
> > To avoid exposing synchronization primitives to callers, introduce
> > helper for setting the configuration and use it.
> >
> > Signed-off-by: Parav Pandit <parav@nvidia.com>
> > Reviewed-by: Eli Cohen <elic@nvidia.com>
> > Acked-by: Jason Wang <jasowang@redhat.com>
> > ---
> >   drivers/vdpa/vdpa.c  | 36 ++++++++++++++++++++++++++++++++++++
> >   drivers/vhost/vdpa.c |  3 +--
> >   include/linux/vdpa.h | 19 ++++---------------
> 
> 
> Do we need to change virtio_vdpa_set() as well?
> 
Yes. Sending v5.
Thanks.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH linux-next v4 2/8] vdpa: Introduce query of device config layout

[Parav Pandit via Virtualization]

> From: Jason Wang <jasowang@redhat.com>
> Sent: Monday, October 25, 2021 11:36 AM
> 
> 在 2021/10/22 上午12:35, Parav Pandit 写道:
> > Introduce a command to query a device config layout.
> >
> > An example query of network vdpa device:
> >
> > $ vdpa dev add name bar mgmtdev vdpasim_net
> >
> > $ vdpa dev config show
> > bar: mac 00:35:09:19:48:05 link up link_announce false mtu 1500
> 
> 
> Nit: it looks to me this patch doesn't expose link_announce but
> max_virtqueue_pairs.
> 
> Other looks good.

I recollect now when started working on v5.
Above commit log example is correct. Max_virtqueue_pairs is not exposed for the vdpa device as this field in the config space is not valid.
It is not valid because this vdpa net device is single queue.
So this commit log is accurate.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

[Jason Wang]

在 2021/10/25 下午3:06, Parav Pandit 写道:
>
>> From: Jason Wang <jasowang@redhat.com>
>> 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@nvidia.com>
>>> Reviewed-by: Eli Cohen <elic@nvidia.com>
>>> ---
>>> 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@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v4 6/8] vdpa/mlx5: Fix clearing of VIRTIO_NET_F_MAC feature bit

[Jason Wang]

在 2021/10/25 下午3:08, Parav Pandit 写道:
>
>> From: Jason Wang <jasowang@redhat.com>
>> Sent: Monday, October 25, 2021 12:35 PM
>>
>> 在 2021/10/22 上午12:35, Parav Pandit 写道:
>>> Cited patch in the fixes tag clears the features bit during reset.
>>> mlx5 vdpa device feature bits are static decided by device capabilities.
>>
>> This is not what I read at least from mlx5_vdpa_get_features:
>>
>> static u64 mlx5_vdpa_get_features(struct vdpa_device *vdev) {
>>           struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
>>           struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
>>           u16 dev_features;
>>
>>           dev_features = MLX5_CAP_DEV_VDPA_EMULATION(mvdev->mdev,
>> device_features_bits_mask);
> Not sure I follow. Feature bits are decided by the device capabilities exposed by the MLX5_CAP_DEV_VDPA_EMULATION.
> Other fields below are pretty much static.


Ok, so I think in the commit log we need also mention that the 
VIRTIO_NET_F_MAC is determined and stored in device adding.

Thanks


>
>>           ndev->mvdev.mlx_features |= mlx_to_vritio_features(dev_features);
>>           if (MLX5_CAP_DEV_VDPA_EMULATION(mvdev->mdev,
>> virtio_version_1_0))
>>                   ndev->mvdev.mlx_features |= BIT_ULL(VIRTIO_F_VERSION_1);
>>           ndev->mvdev.mlx_features |= BIT_ULL(VIRTIO_F_ACCESS_PLATFORM);
>>           ndev->mvdev.mlx_features |= BIT_ULL(VIRTIO_NET_F_CTRL_VQ);
>>           ndev->mvdev.mlx_features |=
>> BIT_ULL(VIRTIO_NET_F_CTRL_MAC_ADDR);
>>           ndev->mvdev.mlx_features |= BIT_ULL(VIRTIO_NET_F_MQ);
>>
>>           print_features(mvdev, ndev->mvdev.mlx_features, false);
>>           return ndev->mvdev.mlx_features; }
>>
>>
>> Thanks
>>
>>
>>> Clearing features bit cleared the VIRTIO_NET_F_MAC. Due to this MAC
>>> address provided by the device is not honored.
>>>
>>> Fix it by not clearing the static feature bits during reset.
>>>
>>> Fixes: 0686082dbf7a ("vdpa: Add reset callback in vdpa_config_ops")
>>> Signed-off-by: Parav Pandit <parav@nvidia.com>
>>> Reviewed-by: Eli Cohen <elic@nvidia.com>
>>> ---
>>>    drivers/vdpa/mlx5/net/mlx5_vnet.c | 1 -
>>>    1 file changed, 1 deletion(-)
>>>
>>> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
>>> b/drivers/vdpa/mlx5/net/mlx5_vnet.c
>>> index ca05f69054b6..0a2b79887085 100644
>>> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
>>> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
>>> @@ -2192,7 +2192,6 @@ static int mlx5_vdpa_reset(struct vdpa_device
>> *vdev)
>>>    	clear_vqs_ready(ndev);
>>>    	mlx5_vdpa_destroy_mr(&ndev->mvdev);
>>>    	ndev->mvdev.status = 0;
>>> -	ndev->mvdev.mlx_features = 0;
>>>    	memset(ndev->event_cbs, 0, sizeof(ndev->event_cbs));
>>>    	ndev->mvdev.actual_features = 0;
>>>    	++mvdev->generation;

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH linux-next v4 2/8] vdpa: Introduce query of device config layout

[Jason Wang]

在 2021/10/25 下午2:59, Parav Pandit 写道:
>
>> From: Jason Wang <jasowang@redhat.com>
>> Sent: Monday, October 25, 2021 11:36 AM
>>
>>
>> 在 2021/10/22 上午12:35, Parav Pandit 写道:
>>> Introduce a command to query a device config layout.
>>>
>>> An example query of network vdpa device:
>>>
>>> $ vdpa dev add name bar mgmtdev vdpasim_net
>>>
>>> $ vdpa dev config show
>>> bar: mac 00:35:09:19:48:05 link up link_announce false mtu 1500
>>
>> Nit: it looks to me this patch doesn't expose link_announce but
>> max_virtqueue_pairs.
> It does expose the net status field that contains the link_announce.
> I missed to update the example for including max_vq_pairs.


Right.

Thanks


>> Other looks good.
>>
>> Thanks

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization