Parav Pandit
2021-Jun-16 19:11 UTC
[PATCH linux-next v3 3/6] vdpa: Enable user to set mac and mtu of vdpa device
$ 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 <parav at nvidia.com>
Reviewed-by: Eli Cohen <elic at nvidia.com>
---
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 ++++++++++++++++++++++++++++++++++++++-
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 *
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;
+
+ 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]);
+ 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]);
+ config.net_mask.mtu_valid = true;
+ }
+
+ mutex_lock(&vdev->cf_mutex);
+ err = vdev->config->setup_config(vdev, &config);
+ mutex_unlock(&vdev->cf_mutex);
+ return err;
+}
+
+static int vdpa_dev_config_set(struct vdpa_device *vdev, struct sk_buff *skb,
+ struct genl_info *info)
+{
+ int err = -EOPNOTSUPP;
+ u32 device_id;
+
+ if (!vdev->mdev)
+ return -EOPNOTSUPP;
+
+ device_id = vdev->config->get_device_id(vdev);
+ switch (device_id) {
+ case VIRTIO_ID_NET:
+ err = vdpa_dev_net_config_set(vdev, skb, info);
+ break;
+ default:
+ break;
+ }
+ return err;
+}
+
+static int vdpa_nl_cmd_dev_config_set_doit(struct sk_buff *skb, struct
genl_info *info)
+{
+ struct vdpa_device *vdev;
+ 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]);
+
+ mutex_lock(&vdpa_dev_mutex);
+ dev = bus_find_device(&vdpa_bus, NULL, devname, vdpa_name_match);
+ if (!dev) {
+ mutex_unlock(&vdpa_dev_mutex);
+ NL_SET_ERR_MSG_MOD(info->extack, "device not found");
+ return -ENODEV;
+ }
+ vdev = container_of(dev, struct vdpa_device, dev);
+ if (!vdev->mdev) {
+ mutex_unlock(&vdpa_dev_mutex);
+ put_device(dev);
+ return -EINVAL;
+ }
+ err = vdpa_dev_config_set(vdev, skb, info);
+ put_device(dev);
+ mutex_unlock(&vdpa_dev_mutex);
+ return err;
+}
+
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[] = {
@@ -886,6 +969,12 @@ static const struct genl_ops vdpa_nl_ops[] = {
.doit = vdpa_nl_cmd_dev_config_get_doit,
.dumpit = vdpa_nl_cmd_dev_config_get_dumpit,
},
+ {
+ .cmd = VDPA_CMD_DEV_CONFIG_SET,
+ .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
+ .doit = vdpa_nl_cmd_dev_config_set_doit,
+ .flags = GENL_ADMIN_PERM,
+ },
};
static struct genl_family vdpa_nl_family __ro_after_init = {
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index bf104f9f461a..9b7238d5310e 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.
@@ -70,6 +72,17 @@ struct vdpa_iova_range {
u64 last;
};
+struct vdpa_dev_set_config {
+ struct {
+ u8 mac[ETH_ALEN];
+ u16 mtu;
+ } net;
+ struct {
+ u8 mac_valid: 1;
+ u8 mtu_valid: 1;
+ } net_mask;
+};
+
/**
* struct vdpa_config_ops - operations for configuring a vDPA device.
* Note: vDPA device drivers are required to implement all of the
@@ -169,6 +182,9 @@ struct vdpa_iova_range {
* @buf: buffer used to write from
* @len: the length to write to
* configuration space
+ * @setup_config: Setup configuration space
+ * @vdev: vdpa device
+ * #config: configuration to apply to device
* @get_generation: Get device config generation (optional)
* @vdev: vdpa device
* Returns u32: device generation
@@ -241,6 +257,8 @@ struct vdpa_config_ops {
void *buf, unsigned int len);
void (*set_config)(struct vdpa_device *vdev, unsigned int offset,
const void *buf, unsigned int len);
+ int (*setup_config)(struct vdpa_device *vdev,
+ const struct vdpa_dev_set_config *config);
u32 (*get_generation)(struct vdpa_device *vdev);
struct vdpa_iova_range (*get_iova_range)(struct vdpa_device *vdev);
diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
index 5c31ecc3b956..ec349789b8d1 100644
--- a/include/uapi/linux/vdpa.h
+++ b/include/uapi/linux/vdpa.h
@@ -18,6 +18,7 @@ enum vdpa_command {
VDPA_CMD_DEV_DEL,
VDPA_CMD_DEV_GET, /* can dump */
VDPA_CMD_DEV_CONFIG_GET, /* can dump */
+ VDPA_CMD_DEV_CONFIG_SET,
};
enum vdpa_attr {
--
2.26.2
Jason Wang
2021-Jun-22 07:43 UTC
[PATCH linux-next v3 3/6] vdpa: Enable user to set mac and mtu of vdpa device
? 2021/6/17 ??3:11, Parav Pandit ??:> $ 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 <parav at nvidia.com> > Reviewed-by: Eli Cohen <elic at nvidia.com> > --- > 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 ++++++++++++++++++++++++++++++++++++++- > 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 * > 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).> + > + 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]); > + 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]); > + 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)); 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. Thanks> + > + mutex_lock(&vdev->cf_mutex); > + err = vdev->config->setup_config(vdev, &config); > + mutex_unlock(&vdev->cf_mutex); > + return err; > +} > + > +static int vdpa_dev_config_set(struct vdpa_device *vdev, struct sk_buff *skb, > + struct genl_info *info) > +{ > + int err = -EOPNOTSUPP; > + u32 device_id; > + > + if (!vdev->mdev) > + return -EOPNOTSUPP; > + > + device_id = vdev->config->get_device_id(vdev); > + switch (device_id) { > + case VIRTIO_ID_NET: > + err = vdpa_dev_net_config_set(vdev, skb, info); > + break; > + default: > + break; > + } > + return err; > +} > + > +static int vdpa_nl_cmd_dev_config_set_doit(struct sk_buff *skb, struct genl_info *info) > +{ > + struct vdpa_device *vdev; > + 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]); > + > + mutex_lock(&vdpa_dev_mutex); > + dev = bus_find_device(&vdpa_bus, NULL, devname, vdpa_name_match); > + if (!dev) { > + mutex_unlock(&vdpa_dev_mutex); > + NL_SET_ERR_MSG_MOD(info->extack, "device not found"); > + return -ENODEV; > + } > + vdev = container_of(dev, struct vdpa_device, dev); > + if (!vdev->mdev) { > + mutex_unlock(&vdpa_dev_mutex); > + put_device(dev); > + return -EINVAL; > + } > + err = vdpa_dev_config_set(vdev, skb, info); > + put_device(dev); > + mutex_unlock(&vdpa_dev_mutex); > + return err; > +} > + > 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[] = { > @@ -886,6 +969,12 @@ static const struct genl_ops vdpa_nl_ops[] = { > .doit = vdpa_nl_cmd_dev_config_get_doit, > .dumpit = vdpa_nl_cmd_dev_config_get_dumpit, > }, > + { > + .cmd = VDPA_CMD_DEV_CONFIG_SET, > + .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP, > + .doit = vdpa_nl_cmd_dev_config_set_doit, > + .flags = GENL_ADMIN_PERM, > + }, > }; > > static struct genl_family vdpa_nl_family __ro_after_init = { > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h > index bf104f9f461a..9b7238d5310e 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. > @@ -70,6 +72,17 @@ struct vdpa_iova_range { > u64 last; > }; > > +struct vdpa_dev_set_config { > + struct { > + u8 mac[ETH_ALEN]; > + u16 mtu; > + } net; > + struct { > + u8 mac_valid: 1; > + u8 mtu_valid: 1; > + } net_mask; > +}; > + > /** > * struct vdpa_config_ops - operations for configuring a vDPA device. > * Note: vDPA device drivers are required to implement all of the > @@ -169,6 +182,9 @@ struct vdpa_iova_range { > * @buf: buffer used to write from > * @len: the length to write to > * configuration space > + * @setup_config: Setup configuration space > + * @vdev: vdpa device > + * #config: configuration to apply to device > * @get_generation: Get device config generation (optional) > * @vdev: vdpa device > * Returns u32: device generation > @@ -241,6 +257,8 @@ struct vdpa_config_ops { > void *buf, unsigned int len); > void (*set_config)(struct vdpa_device *vdev, unsigned int offset, > const void *buf, unsigned int len); > + int (*setup_config)(struct vdpa_device *vdev, > + const struct vdpa_dev_set_config *config); > u32 (*get_generation)(struct vdpa_device *vdev); > struct vdpa_iova_range (*get_iova_range)(struct vdpa_device *vdev); > > diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h > index 5c31ecc3b956..ec349789b8d1 100644 > --- a/include/uapi/linux/vdpa.h > +++ b/include/uapi/linux/vdpa.h > @@ -18,6 +18,7 @@ enum vdpa_command { > VDPA_CMD_DEV_DEL, > VDPA_CMD_DEV_GET, /* can dump */ > VDPA_CMD_DEV_CONFIG_GET, /* can dump */ > + VDPA_CMD_DEV_CONFIG_SET, > }; > > enum vdpa_attr {