From: Jason Wang <jasowang@redhat.com>
Sent: Tuesday, June 22, 2021 1:13 PM

在 2021/6/17 上午3:11, Parav Pandit 写道:

quoted

$ vdpa dev add name bar mgmtdev vdpasim_net

$ vdpa dev config set bar 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 speed
0 duplex 0

$ vdpa dev config show -jp
{
     "config": {
         "bar": {
             "mac": "00:11:22:33:44:55",
             "link ": "up",
             "link_announce ": false,
             "mtu": 9000,
             "speed": 0,
             "duplex": 0
         }
     }
}

Signed-off-by: Parav Pandit <redacted>
Reviewed-by: Eli Cohen <redacted>
---
changelog:
v2->v3:
  - using new setup_config callback to setup device params via mgmt tool
    to avoid mixing with existing set_config().
---
  drivers/vdpa/vdpa.c       | 91

++++++++++++++++++++++++++++++++++++++-

quoted

  include/linux/vdpa.h      | 18 ++++++++
  include/uapi/linux/vdpa.h |  1 +
  3 files changed, 109 insertions(+), 1 deletion(-)

diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index

1295528244c3..40874bd92126 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);

@@ -849,10 +848,94 @@ vdpa_nl_cmd_dev_config_get_dumpit(struct

sk_buff *msg, struct netlink_callback *

quoted

  	return msg->len;
  }

+static int vdpa_dev_net_config_set(struct vdpa_device *vdev,
+				   struct sk_buff *skb, struct genl_info *info) {
+	struct nlattr **nl_attrs = info->attrs;
+	struct vdpa_dev_set_config config = {};
+	const u8 *macaddr;
+	int err;
+
+	if (!netlink_capable(skb, CAP_NET_ADMIN))
+		return -EPERM;

Interesting, I wonder how cap would be used for other type of devices (e.g
block).

quoted

+
+	if (!vdev->config->setup_config)
+		return -EOPNOTSUPP;
+
+	if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MACADDR]) {
+		macaddr =

nla_data(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MACADDR]);

quoted

+		memcpy(config.net.mac, macaddr, sizeof(config.net.mac));
+		config.net_mask.mac_valid = true;
+	}
+	if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]) {
+		config.net.mtu =
+

	nla_get_u16(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]);

quoted

+		config.net_mask.mtu_valid = true;
+	}

Instead of doing memcpy and pass the whole config structure like this. I
wonder if it's better to switch to use:

vdev->config->setup_config(vdev, offsetof(struct virtio_net_config,
mtu), &mtu, sizeof(mtu));

Well, we need a way to differentiate that the caller is management tool and not the vhost path.

Instead of passing some flag of the caller to setup_config(), a explicitly defined callback served better.

And secondly we need to return the error status. setup_config() cb is void. This is the minor one.

Then there's no need for the vdpa_dev_set_config structure which will
became structure virtio_net_config gradually.

The setup_config() can fail if the offset is not at the boundary of a
specific attribute.

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