Re: [PATCH v2 07/10] vdpa: Add support for returning device configuration information
On Tue, Dec 14, 2021 at 3:17 PM Eli Cohen wrote:
>
> On Tue, Dec 14, 2021 at 12:05:38PM +0800, Jason Wang wrote:
> > On Mon, Dec 13, 2021 at 10:43 PM Eli Cohen wrote:
> > >
> > > Add netlink attribute to store flags indicating current state of the
> > > device.
> > > In addition, introduce a flag to indicate whether control virtqueue is
> > > used.
> > >
> > > This indication can be retrieved by:
> > >
> > > vdpa dev config show vdpa-a
> > > vdpa-a: mac 00:00:00:00:88:88 link up link_announce false max_vq_pairs 1
> > > mtu 1500 ctrl_vq yes
> >
> > I think the cvq is kind of duplicated with the driver features?
>
> We don't pass to userspace the driver features. The availablity of CVQ
> is passed trough a new bit mask field that encodes (currently) just the
> availablity of CVQ.
>
> Are you suggesting the return the entire set of negotiated features and
> let user space decode that?
Yes, I think that can simplify things and save a lot of effort.
Thanks
>
> >
> > Thanks
> >
> > >
> > > Signed-off-by: Eli Cohen
> > >
> > >
> > > V1 -> V2
> > > Patch was changed to return only an indication of ctrl VQ
> > > ---
> > > drivers/vdpa/vdpa.c | 17 +
> > > include/uapi/linux/vdpa.h | 8
> > > 2 files changed, 25 insertions(+)
> > >
> > > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> > > index 7b7bef7673b4..130a8d4aeaed 100644
> > > --- a/drivers/vdpa/vdpa.c
> > > +++ b/drivers/vdpa/vdpa.c
> > > @@ -787,6 +787,19 @@ static int vdpa_nl_cmd_dev_get_dumpit(struct sk_buff
> > > *msg, struct netlink_callba
> > > return msg->len;
> > > }
> > >
> > > +static int vdpa_dev_net_ctrl_vq_fill(struct vdpa_device *vdev,
> > > +struct sk_buff *msg,
> > > +struct virtio_net_config *config,
> > > +u64 features)
> > > +{
> > > + if (!(features & BIT_ULL(VIRTIO_NET_F_CTRL_VQ)))
> > > + return 0;
> > > +
> > > + /* currently the only flag can be returned */
> > > + return nla_put_u64_64bit(msg, VDPA_ATTR_DEV_FLAGS,
> > > +BIT_ULL(VDPA_DEV_ATTR_CVQ),
> > > VDPA_ATTR_PAD);
> > > +}
> > > +
> > > static int vdpa_dev_net_mq_config_fill(struct vdpa_device *vdev,
> > >struct sk_buff *msg, u64 features,
> > >const struct virtio_net_config
> > > *config)
> > > @@ -805,6 +818,7 @@ static int vdpa_dev_net_config_fill(struct
> > > vdpa_device *vdev, struct sk_buff *ms
> > > struct virtio_net_config config = {};
> > > u64 features;
> > > u16 val_u16;
> > > + int err;
> > >
> > > vdpa_get_config(vdev, 0, , sizeof(config));
> > >
> > > @@ -821,6 +835,9 @@ static int vdpa_dev_net_config_fill(struct
> > > vdpa_device *vdev, struct sk_buff *ms
> > > return -EMSGSIZE;
> > >
> > > features = vdev->config->get_driver_features(vdev);
> > > + err = vdpa_dev_net_ctrl_vq_fill(vdev, msg, , features);
> > > + if (err)
> > > + return err;
> > >
> > > return vdpa_dev_net_mq_config_fill(vdev, msg, features, );
> > > }
> > > diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
> > > index a252f06f9dfd..23b854e3e5e2 100644
> > > --- a/include/uapi/linux/vdpa.h
> > > +++ b/include/uapi/linux/vdpa.h
> > > @@ -20,9 +20,16 @@ enum vdpa_command {
> > > VDPA_CMD_DEV_CONFIG_GET,/* can dump */
> > > };
> > >
> > > +enum {
> > > + VDPA_DEV_ATTR_CVQ,
> > > +};
> > > +
> > > enum vdpa_attr {
> > > VDPA_ATTR_UNSPEC,
> > >
> > > + /* Pad attribute for 64b alignment */
> > > + VDPA_ATTR_PAD = VDPA_ATTR_UNSPEC,
> > > +
> > > /* bus name (optional) + dev name together make the parent device
> > > handle */
> > > VDPA_ATTR_MGMTDEV_BUS_NAME, /* string */
> > > VDPA_ATTR_MGMTDEV_DEV_NAME, /* string */
> > > @@ -34,6 +41,7 @@ enum vdpa_attr {
> > > VDPA_ATTR_DEV_MAX_VQS, /* u32 */
> > > VDPA_ATTR_DEV_MAX_VQ_SIZE, /* u16 */
> > > VDPA_ATTR_DEV_MIN_VQ_SIZE, /* u16 */
> > > + VDPA_ATTR_DEV_FLAGS,/* u64 */
> > >
> > > VDPA_ATTR_DEV_NET_CFG_MACADDR, /* binary */
> > > VDPA_ATTR_DEV_NET_STATUS, /* u8 */
> > > --
> > > 2.33.1
> > >
> >
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 07/10] vdpa: Add support for returning device configuration information
On Mon, Dec 13, 2021 at 10:43 PM Eli Cohen wrote:
>
> Add netlink attribute to store flags indicating current state of the
> device.
> In addition, introduce a flag to indicate whether control virtqueue is
> used.
>
> This indication can be retrieved by:
>
> vdpa dev config show vdpa-a
> vdpa-a: mac 00:00:00:00:88:88 link up link_announce false max_vq_pairs 1
> mtu 1500 ctrl_vq yes
I think the cvq is kind of duplicated with the driver features?
Thanks
>
> Signed-off-by: Eli Cohen
>
>
> V1 -> V2
> Patch was changed to return only an indication of ctrl VQ
> ---
> drivers/vdpa/vdpa.c | 17 +
> include/uapi/linux/vdpa.h | 8
> 2 files changed, 25 insertions(+)
>
> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> index 7b7bef7673b4..130a8d4aeaed 100644
> --- a/drivers/vdpa/vdpa.c
> +++ b/drivers/vdpa/vdpa.c
> @@ -787,6 +787,19 @@ static int vdpa_nl_cmd_dev_get_dumpit(struct sk_buff
> *msg, struct netlink_callba
> return msg->len;
> }
>
> +static int vdpa_dev_net_ctrl_vq_fill(struct vdpa_device *vdev,
> +struct sk_buff *msg,
> +struct virtio_net_config *config,
> +u64 features)
> +{
> + if (!(features & BIT_ULL(VIRTIO_NET_F_CTRL_VQ)))
> + return 0;
> +
> + /* currently the only flag can be returned */
> + return nla_put_u64_64bit(msg, VDPA_ATTR_DEV_FLAGS,
> +BIT_ULL(VDPA_DEV_ATTR_CVQ), VDPA_ATTR_PAD);
> +}
> +
> static int vdpa_dev_net_mq_config_fill(struct vdpa_device *vdev,
>struct sk_buff *msg, u64 features,
>const struct virtio_net_config *config)
> @@ -805,6 +818,7 @@ static int vdpa_dev_net_config_fill(struct vdpa_device
> *vdev, struct sk_buff *ms
> struct virtio_net_config config = {};
> u64 features;
> u16 val_u16;
> + int err;
>
> vdpa_get_config(vdev, 0, , sizeof(config));
>
> @@ -821,6 +835,9 @@ static int vdpa_dev_net_config_fill(struct vdpa_device
> *vdev, struct sk_buff *ms
> return -EMSGSIZE;
>
> features = vdev->config->get_driver_features(vdev);
> + err = vdpa_dev_net_ctrl_vq_fill(vdev, msg, , features);
> + if (err)
> + return err;
>
> return vdpa_dev_net_mq_config_fill(vdev, msg, features, );
> }
> diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
> index a252f06f9dfd..23b854e3e5e2 100644
> --- a/include/uapi/linux/vdpa.h
> +++ b/include/uapi/linux/vdpa.h
> @@ -20,9 +20,16 @@ enum vdpa_command {
> VDPA_CMD_DEV_CONFIG_GET,/* can dump */
> };
>
> +enum {
> + VDPA_DEV_ATTR_CVQ,
> +};
> +
> enum vdpa_attr {
> VDPA_ATTR_UNSPEC,
>
> + /* Pad attribute for 64b alignment */
> + VDPA_ATTR_PAD = VDPA_ATTR_UNSPEC,
> +
> /* bus name (optional) + dev name together make the parent device
> handle */
> VDPA_ATTR_MGMTDEV_BUS_NAME, /* string */
> VDPA_ATTR_MGMTDEV_DEV_NAME, /* string */
> @@ -34,6 +41,7 @@ enum vdpa_attr {
> VDPA_ATTR_DEV_MAX_VQS, /* u32 */
> VDPA_ATTR_DEV_MAX_VQ_SIZE, /* u16 */
> VDPA_ATTR_DEV_MIN_VQ_SIZE, /* u16 */
> + VDPA_ATTR_DEV_FLAGS,/* u64 */
>
> VDPA_ATTR_DEV_NET_CFG_MACADDR, /* binary */
> VDPA_ATTR_DEV_NET_STATUS, /* u8 */
> --
> 2.33.1
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 07/10] vdpa: Add support for returning device configuration information
On 12/13/2021 6:42 AM, Eli Cohen wrote:
Add netlink attribute to store flags indicating current state of the
device.
In addition, introduce a flag to indicate whether control virtqueue is
used.
This indication can be retrieved by:
vdpa dev config show vdpa-a
vdpa-a: mac 00:00:00:00:88:88 link up link_announce false max_vq_pairs 1
mtu 1500 ctrl_vq yes
Signed-off-by: Eli Cohen
V1 -> V2
Patch was changed to return only an indication of ctrl VQ
---
drivers/vdpa/vdpa.c | 17 +
include/uapi/linux/vdpa.h | 8
2 files changed, 25 insertions(+)
diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index 7b7bef7673b4..130a8d4aeaed 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -787,6 +787,19 @@ static int vdpa_nl_cmd_dev_get_dumpit(struct sk_buff *msg,
struct netlink_callba
return msg->len;
}
+static int vdpa_dev_net_ctrl_vq_fill(struct vdpa_device *vdev,
+struct sk_buff *msg,
+struct virtio_net_config *config,
+u64 features)
+{
+ if (!(features & BIT_ULL(VIRTIO_NET_F_CTRL_VQ)))
+ return 0;
+
+ /* currently the only flag can be returned */
+ return nla_put_u64_64bit(msg, VDPA_ATTR_DEV_FLAGS,
+BIT_ULL(VDPA_DEV_ATTR_CVQ), VDPA_ATTR_PAD);
+}
+
static int vdpa_dev_net_mq_config_fill(struct vdpa_device *vdev,
struct sk_buff *msg, u64 features,
const struct virtio_net_config *config)
@@ -805,6 +818,7 @@ static int vdpa_dev_net_config_fill(struct vdpa_device
*vdev, struct sk_buff *ms
struct virtio_net_config config = {};
u64 features;
u16 val_u16;
+ int err;
vdpa_get_config(vdev, 0, , sizeof(config));
@@ -821,6 +835,9 @@ static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *ms
return -EMSGSIZE;
features = vdev->config->get_driver_features(vdev);
+ err = vdpa_dev_net_ctrl_vq_fill(vdev, msg, , features);
+ if (err)
+ return err;
return vdpa_dev_net_mq_config_fill(vdev, msg, features, );
}
diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
index a252f06f9dfd..23b854e3e5e2 100644
--- a/include/uapi/linux/vdpa.h
+++ b/include/uapi/linux/vdpa.h
@@ -20,9 +20,16 @@ enum vdpa_command {
VDPA_CMD_DEV_CONFIG_GET,/* can dump */
};
+enum {
+ VDPA_DEV_ATTR_CVQ,
+};
+
enum vdpa_attr {
VDPA_ATTR_UNSPEC,
+ /* Pad attribute for 64b alignment */
+ VDPA_ATTR_PAD = VDPA_ATTR_UNSPEC,
+
/* bus name (optional) + dev name together make the parent device
handle */
VDPA_ATTR_MGMTDEV_BUS_NAME, /* string */
VDPA_ATTR_MGMTDEV_DEV_NAME, /* string */
@@ -34,6 +41,7 @@ enum vdpa_attr {
VDPA_ATTR_DEV_MAX_VQS, /* u32 */
VDPA_ATTR_DEV_MAX_VQ_SIZE, /* u16 */
VDPA_ATTR_DEV_MIN_VQ_SIZE, /* u16 */
+ VDPA_ATTR_DEV_FLAGS,/* u64 */
Adding new attr here would break existing userspace that is compiled
with the older value. Need to append to the end.
-Siwei
VDPA_ATTR_DEV_NET_CFG_MACADDR, /* binary */
VDPA_ATTR_DEV_NET_STATUS, /* u8 */
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
