On Wed, Feb 24, 2021 at 08:18:37AM +0200, Parav Pandit wrote:
> 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 speed 0 duplex 0
>
> $ vdpa dev config show -jp
> {
> "config": {
> "bar": {
> "mac": "00:35:09:19:48:05",
> "link ": "up",
> "link_announce ": false,
> "mtu": 1500,
> "speed": 0,
> "duplex": 0
> }
> }
> }
>
> Signed-off-by: Parav Pandit <parav@xxxxxxxxxx>
> Reviewed-by: Eli Cohen <elic@xxxxxxxxxx>
> ---
> changelog:
> v1->v2:
> - read whole net config layout instead of individual fields
> - added error extack for unmanaged vdpa device
> ---
> drivers/vdpa/vdpa.c | 181 ++++++++++++++++++++++++++++++++++++++
> include/uapi/linux/vdpa.h | 11 +++
> 2 files changed, 192 insertions(+)
>
> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> index 3d997b389345..cebbba500638 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. */
> @@ -603,6 +605,179 @@ 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,
> + const struct virtio_net_config *config)
> +{
> + u32 hash_types;
> + u16 rss_field;
> + u64 features;
> +
> + features = vdev->config->get_features(vdev);
> + if ((features & (1ULL << VIRTIO_NET_F_MQ)) == 0)
> + return 0;
> +
> + if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP,
> + config->max_virtqueue_pairs))
> + return -EMSGSIZE;
> + if (nla_put_u8(msg, VDPA_ATTR_DEV_NET_CFG_RSS_MAX_KEY_LEN,
> + config->rss_max_key_size))
Why is it ok to poke at RSS fields without checking the relevant feature bits?
> + return -EMSGSIZE;
Did you check this with sparse?
max_virtqueue_pairs is __virtio16.
> +
> + rss_field = le16_to_cpu(config->rss_max_key_size);
> + if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_RSS_MAX_IT_LEN, rss_field))
> + return -EMSGSIZE;
> +
> + hash_types = le32_to_cpu(config->supported_hash_types);
unused variable
> + if (nla_put_u32(msg, VDPA_ATTR_DEV_NET_CFG_RSS_HASH_TYPES,
> + config->supported_hash_types))
> + return -EMSGSIZE;
> + return 0;
> +}
> +
> +static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *msg)
> +{
> + struct virtio_net_config config = {};
> +
> + vdev->config->get_config(vdev, 0, &config, sizeof(config));
> + if (nla_put(msg, VDPA_ATTR_DEV_NET_CFG_MACADDR, sizeof(config.mac), config.mac))
> + return -EMSGSIZE;
> + if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, config.status))
> + return -EMSGSIZE;
> + if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, config.mtu))
> + return -EMSGSIZE;
> + if (nla_put_u32(msg, VDPA_ATTR_DEV_NET_CFG_SPEED, config.speed))
> + return -EMSGSIZE;
looks like a bunch of endian-ness/sparse errors to me, and
a bunch of fields checked without checking the feature bits.
> + if (nla_put_u8(msg, VDPA_ATTR_DEV_NET_CFG_DUPLEX, config.duplex))
> + return -EMSGSIZE;
> +
> + return vdpa_dev_net_mq_config_fill(vdev, msg, &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 },
> @@ -634,6 +809,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/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
> index 66a41e4ec163..5c31ecc3b956 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,16 @@ 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 */
> + VDPA_ATTR_DEV_NET_CFG_SPEED, /* u16 */
> + VDPA_ATTR_DEV_NET_CFG_DUPLEX, /* u16 */
> + VDPA_ATTR_DEV_NET_CFG_RSS_MAX_KEY_LEN, /* u8 */
> + VDPA_ATTR_DEV_NET_CFG_RSS_MAX_IT_LEN, /* u16 */
> + VDPA_ATTR_DEV_NET_CFG_RSS_HASH_TYPES, /* u32 */
> +
> /* new attributes must be added above here */
> VDPA_ATTR_MAX,
> };
> --
> 2.26.2
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
