diff mbox series

[v1,2/4] vdpa: Allow for printing negotiated features of a device

Message ID 20220210133115.115967-3-elic@nvidia.com (mailing list archive)
State Superseded
Headers show
Series vdpa tool enhancements | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Eli Cohen Feb. 10, 2022, 1:31 p.m. UTC
When reading the configuration of a vdpa device, check if the
VDPA_ATTR_DEV_NEGOTIATED_FEATURES is available. If it is, parse the
feature bits and print a string representation of each of the feature
bits.

We keep the strings in two different arrays. One for net device related
devices and one for generic feature bits.

In this patch we parse only net device specific features. Support for
other devices can be added later. If the device queried is not a net
device, we print its bit number only.

Examples:
1.
$ vdpa dev config show vdpa-a
vdpa-a: mac 00:00:00:00:88:88 link up link_announce false max_vq_pairs 3 \
        mtu 1500
  negotiated_features CSUM GUEST_CSUM MTU MAC HOST_TSO4 HOST_TSO6 STATUS \
                      CTRL_VQ MQ CTRL_MAC_ADDR VERSION_1 ACCESS_PLATFORM

2. json output
$ vdpa -j dev config show vdpa-a
{"config":{"vdpa-a":{"mac":"00:00:00:00:88:88","link":"up","link_announce":false, \
  "max_vq_pairs":3,"mtu":1500,"negotiated_features":["CSUM","GUEST_CSUM","MTU", \
  "MAC","HOST_TSO4","HOST_TSO6","STATUS","CTRL_VQ","MQ","CTRL_MAC_ADDR", \
  "VERSION_1","ACCESS_PLATFORM"]}}}

3. pretty json
$ vdpa -jp dev config show vdpa-a
{
    "config": {
        "vdpa-a": {
            "mac": "00:00:00:00:88:88",
            "link ": "up",
            "link_announce ": false,
            "max_vq_pairs": 3,
            "mtu": 1500,
            "negotiated_features": [
"CSUM","GUEST_CSUM","MTU","MAC","HOST_TSO4","HOST_TSO6","STATUS","CTRL_VQ", \
"MQ","CTRL_MAC_ADDR","VERSION_1","ACCESS_PLATFORM" ]
        }
    }
}

Signed-off-by: Eli Cohen <elic@nvidia.com>
---
 vdpa/include/uapi/linux/vdpa.h |   2 +
 vdpa/vdpa.c                    | 126 +++++++++++++++++++++++++++++++--
 2 files changed, 124 insertions(+), 4 deletions(-)

Comments

Si-Wei Liu Feb. 12, 2022, 1:15 a.m. UTC | #1
On 2/10/2022 5:31 AM, Eli Cohen wrote:
> When reading the configuration of a vdpa device, check if the
> VDPA_ATTR_DEV_NEGOTIATED_FEATURES is available. If it is, parse the
> feature bits and print a string representation of each of the feature
> bits.
>
> We keep the strings in two different arrays. One for net device related
> devices and one for generic feature bits.
>
> In this patch we parse only net device specific features. Support for
> other devices can be added later. If the device queried is not a net
> device, we print its bit number only.
>
> Examples:
> 1.
> $ vdpa dev config show vdpa-a
> vdpa-a: mac 00:00:00:00:88:88 link up link_announce false max_vq_pairs 3 \
>          mtu 1500
>    negotiated_features CSUM GUEST_CSUM MTU MAC HOST_TSO4 HOST_TSO6 STATUS \
>                        CTRL_VQ MQ CTRL_MAC_ADDR VERSION_1 ACCESS_PLATFORM
>
> 2. json output
> $ vdpa -j dev config show vdpa-a
> {"config":{"vdpa-a":{"mac":"00:00:00:00:88:88","link":"up","link_announce":false, \
>    "max_vq_pairs":3,"mtu":1500,"negotiated_features":["CSUM","GUEST_CSUM","MTU", \
>    "MAC","HOST_TSO4","HOST_TSO6","STATUS","CTRL_VQ","MQ","CTRL_MAC_ADDR", \
>    "VERSION_1","ACCESS_PLATFORM"]}}}
>
> 3. pretty json
> $ vdpa -jp dev config show vdpa-a
> {
>      "config": {
>          "vdpa-a": {
>              "mac": "00:00:00:00:88:88",
>              "link ": "up",
>              "link_announce ": false,
>              "max_vq_pairs": 3,
>              "mtu": 1500,
>              "negotiated_features": [
> "CSUM","GUEST_CSUM","MTU","MAC","HOST_TSO4","HOST_TSO6","STATUS","CTRL_VQ", \
> "MQ","CTRL_MAC_ADDR","VERSION_1","ACCESS_PLATFORM" ]
>          }
>      }
> }
>
> Signed-off-by: Eli Cohen <elic@nvidia.com>
> ---
>   vdpa/include/uapi/linux/vdpa.h |   2 +
>   vdpa/vdpa.c                    | 126 +++++++++++++++++++++++++++++++--
>   2 files changed, 124 insertions(+), 4 deletions(-)
>
> diff --git a/vdpa/include/uapi/linux/vdpa.h b/vdpa/include/uapi/linux/vdpa.h
> index b7eab069988a..748c350450b2 100644
> --- a/vdpa/include/uapi/linux/vdpa.h
> +++ b/vdpa/include/uapi/linux/vdpa.h
> @@ -40,6 +40,8 @@ enum vdpa_attr {
>   	VDPA_ATTR_DEV_NET_CFG_MAX_VQP,		/* u16 */
>   	VDPA_ATTR_DEV_NET_CFG_MTU,		/* u16 */
>   
> +	VDPA_ATTR_DEV_NEGOTIATED_FEATURES,	/* u64 */
> +
>   	/* new attributes must be added above here */
>   	VDPA_ATTR_MAX,
>   };
> diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c
> index 4ccb564872a0..7deab710913d 100644
> --- a/vdpa/vdpa.c
> +++ b/vdpa/vdpa.c
> @@ -10,6 +10,7 @@
>   #include <linux/virtio_net.h>
>   #include <linux/netlink.h>
>   #include <libmnl/libmnl.h>
> +#include <linux/virtio_ring.h>
>   #include "mnl_utils.h"
>   #include <rt_names.h>
>   
> @@ -78,6 +79,7 @@ static const enum mnl_attr_data_type vdpa_policy[VDPA_ATTR_MAX + 1] = {
>   	[VDPA_ATTR_DEV_VENDOR_ID] = MNL_TYPE_U32,
>   	[VDPA_ATTR_DEV_MAX_VQS] = MNL_TYPE_U32,
>   	[VDPA_ATTR_DEV_MAX_VQ_SIZE] = MNL_TYPE_U16,
> +	[VDPA_ATTR_DEV_NEGOTIATED_FEATURES] = MNL_TYPE_U64,
>   };
>   
>   static int attr_cb(const struct nlattr *attr, void *data)
> @@ -385,17 +387,120 @@ static const char *parse_class(int num)
>   	return class ? class : "< unknown class >";
>   }
>   
> +static const char * const net_feature_strs[64] = {
> +	[VIRTIO_NET_F_CSUM] = "CSUM",
> +	[VIRTIO_NET_F_GUEST_CSUM] = "GUEST_CSUM",
> +	[VIRTIO_NET_F_CTRL_GUEST_OFFLOADS] = "CTRL_GUEST_OFFLOADS",
> +	[VIRTIO_NET_F_MTU] = "MTU",
> +	[VIRTIO_NET_F_MAC] = "MAC",
> +	[VIRTIO_NET_F_GUEST_TSO4] = "GUEST_TSO4",
> +	[VIRTIO_NET_F_GUEST_TSO6] = "GUEST_TSO6",
> +	[VIRTIO_NET_F_GUEST_ECN] = "GUEST_ECN",
> +	[VIRTIO_NET_F_GUEST_UFO] = "GUEST_UFO",
> +	[VIRTIO_NET_F_HOST_TSO4] = "HOST_TSO4",
> +	[VIRTIO_NET_F_HOST_TSO6] = "HOST_TSO6",
> +	[VIRTIO_NET_F_HOST_ECN] = "HOST_ECN",
> +	[VIRTIO_NET_F_HOST_UFO] = "HOST_UFO",
> +	[VIRTIO_NET_F_MRG_RXBUF] = "MRG_RXBUF",
> +	[VIRTIO_NET_F_STATUS] = "STATUS",
> +	[VIRTIO_NET_F_CTRL_VQ] = "CTRL_VQ",
> +	[VIRTIO_NET_F_CTRL_RX] = "CTRL_RX",
> +	[VIRTIO_NET_F_CTRL_VLAN] = "CTRL_VLAN",
> +	[VIRTIO_NET_F_CTRL_RX_EXTRA] = "CTRL_RX_EXTRA",
> +	[VIRTIO_NET_F_GUEST_ANNOUNCE] = "GUEST_ANNOUNCE",
> +	[VIRTIO_NET_F_MQ] = "MQ",
> +	[VIRTIO_F_NOTIFY_ON_EMPTY] = "NOTIFY_ON_EMPTY",
> +	[VIRTIO_NET_F_CTRL_MAC_ADDR] = "CTRL_MAC_ADDR",
> +	[VIRTIO_F_ANY_LAYOUT] = "ANY_LAYOUT",
> +	[VIRTIO_NET_F_RSC_EXT] = "RSC_EXT",
> +	[VIRTIO_NET_F_HASH_REPORT] = "HASH_REPORT",
> +	[VIRTIO_NET_F_RSS] = "RSS",
> +	[VIRTIO_NET_F_STANDBY] = "STANDBY",
> +	[VIRTIO_NET_F_SPEED_DUPLEX] = "SPEED_DUPLEX",
> +};
> +
> +#define VIRTIO_F_IN_ORDER 35
> +#define VIRTIO_F_NOTIFICATION_DATA 38
> +#define VDPA_EXT_FEATURES_SZ (VIRTIO_TRANSPORT_F_END - \
> +			      VIRTIO_TRANSPORT_F_START + 1)
> +
> +static const char * const ext_feature_strs[VDPA_EXT_FEATURES_SZ] = {
> +	[VIRTIO_RING_F_INDIRECT_DESC - VIRTIO_TRANSPORT_F_START] = "RING_INDIRECT_DESC",
> +	[VIRTIO_RING_F_EVENT_IDX - VIRTIO_TRANSPORT_F_START] = "RING_EVENT_IDX",
> +	[VIRTIO_F_VERSION_1 - VIRTIO_TRANSPORT_F_START] = "VERSION_1",
> +	[VIRTIO_F_ACCESS_PLATFORM - VIRTIO_TRANSPORT_F_START] = "ACCESS_PLATFORM",
> +	[VIRTIO_F_RING_PACKED - VIRTIO_TRANSPORT_F_START] = "RING_PACKED",
> +	[VIRTIO_F_IN_ORDER - VIRTIO_TRANSPORT_F_START] = "IN_ORDER",
> +	[VIRTIO_F_ORDER_PLATFORM - VIRTIO_TRANSPORT_F_START] = "ORDER_PLATFORM",
> +	[VIRTIO_F_SR_IOV - VIRTIO_TRANSPORT_F_START] = "SR_IOV",
> +	[VIRTIO_F_NOTIFICATION_DATA - VIRTIO_TRANSPORT_F_START] = "NOTIFICATION_DATA",
> +};
> +
> +static void print_net_features(struct vdpa *vdpa, uint64_t features, bool maxf)
> +{
> +	const char *s;
> +	int i;
> +
> +	if (maxf)
This could use a better name. mgmtdevf maybe?
> +		pr_out_array_start(vdpa, "dev_features");
> +	else
> +		pr_out_array_start(vdpa, "negotiated_features");
> +
> +	for (i = 0; i < 64; i++) {
> +		if (!(features & (1ULL << i)))
> +			continue;
> +
> +		if (i >= VIRTIO_TRANSPORT_F_START && i <= VIRTIO_TRANSPORT_F_END)
> +			s = ext_feature_strs[i - VIRTIO_TRANSPORT_F_START];
> +		else
> +			s = net_feature_strs[i];
> +
> +		if (!s)
> +			print_uint(PRINT_ANY, NULL, " unrecognized_bit_%d", i);
> +		else
> +			print_string(PRINT_ANY, NULL, " %s", s);
> +	}
> +	pr_out_array_end(vdpa);
> +}
> +
> +static void print_generic_features(struct vdpa *vdpa, uint64_t features, bool maxf)
> +{
> +	const char *s;
> +	int i;
> +
> +	if (maxf)
> +		pr_out_array_start(vdpa, "dev_features");
> +	else
> +		pr_out_array_start(vdpa, "negotiated_features");
> +
> +	for (i = 0; i < 64; i++) {
> +		if (!(features & (1ULL << i)))
> +			continue;
> +
> +		if (i >= VIRTIO_TRANSPORT_F_START && i <= VIRTIO_TRANSPORT_F_END)
> +			s = ext_feature_strs[i - VIRTIO_TRANSPORT_F_START];
> +		else
> +			s = NULL;
> +
> +		if (!s)
> +			print_uint(PRINT_ANY, NULL, " bit_%d", i);
> +		else
> +			print_string(PRINT_ANY, NULL, " %s", s);
> +	}
> +	pr_out_array_end(vdpa);
> +}
> +
It looks like most of the implantation in the above two functions are 
the same and the rest are similar. I wonder if can consolidate into one 
by adding a virtio type (dev_id) argument? You may create another global 
static array that helps mapping VIRTIO_ID_NET to net_feature_strs?

>   static void pr_out_mgmtdev_show(struct vdpa *vdpa, const struct nlmsghdr *nlh,
>   				struct nlattr **tb)
>   {
> +	uint64_t classes = 0;
>   	const char *class;
>   	unsigned int i;
>   
>   	pr_out_handle_start(vdpa, tb);
>   
>   	if (tb[VDPA_ATTR_MGMTDEV_SUPPORTED_CLASSES]) {
> -		uint64_t classes = mnl_attr_get_u64(tb[VDPA_ATTR_MGMTDEV_SUPPORTED_CLASSES]);
> -
> +		classes = mnl_attr_get_u64(tb[VDPA_ATTR_MGMTDEV_SUPPORTED_CLASSES]);
>   		pr_out_array_start(vdpa, "supported_classes");
This looks like a minor adjustment to the existing code? Not sure if 
worth another patch though.

>   
>   		for (i = 1; i < 64; i++) {
> @@ -579,9 +684,10 @@ static int cmd_dev_del(struct vdpa *vdpa,  int argc, char **argv)
>   	return mnlu_gen_socket_sndrcv(&vdpa->nlg, nlh, NULL, NULL);
>   }
>   
> -static void pr_out_dev_net_config(struct nlattr **tb)
> +static void pr_out_dev_net_config(struct vdpa *vdpa, struct nlattr **tb)
>   {
>   	SPRINT_BUF(macaddr);
> +	uint64_t val_u64;
>   	uint16_t val_u16;
>   
>   	if (tb[VDPA_ATTR_DEV_NET_CFG_MACADDR]) {
> @@ -610,6 +716,18 @@ static void pr_out_dev_net_config(struct nlattr **tb)
>   		val_u16 = mnl_attr_get_u16(tb[VDPA_ATTR_DEV_NET_CFG_MTU]);
>   		print_uint(PRINT_ANY, "mtu", "mtu %d ", val_u16);
>   	}
> +	if (tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]) {
> +		uint16_t dev_id = 0;
> +
> +		if (tb[VDPA_ATTR_DEV_ID])
> +			dev_id = mnl_attr_get_u32(tb[VDPA_ATTR_DEV_ID]);
> +
> +		val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]);
> +		if (tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES] && dev_id == VIRTIO_ID_NET)
> +			print_net_features(vdpa, val_u64, false);
> +		else
> +			print_generic_features(vdpa, val_u64, true);
Why the last arg is true? That would output the dev_features line instead.

-Siwei

> +	}
>   }
>   
>   static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb)
> @@ -619,7 +737,7 @@ static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb)
>   	pr_out_vdev_handle_start(vdpa, tb);
>   	switch (device_id) {
>   	case VIRTIO_ID_NET:
> -		pr_out_dev_net_config(tb);
> +		pr_out_dev_net_config(vdpa, tb);
>   		break;
>   	default:
>   		break;
Eli Cohen Feb. 15, 2022, 3:16 p.m. UTC | #2
On Fri, Feb 11, 2022 at 05:15:51PM -0800, Si-Wei Liu wrote:
> 
> 
> On 2/10/2022 5:31 AM, Eli Cohen wrote:
> > When reading the configuration of a vdpa device, check if the
> > VDPA_ATTR_DEV_NEGOTIATED_FEATURES is available. If it is, parse the
> > feature bits and print a string representation of each of the feature
> > bits.
> > 
> > We keep the strings in two different arrays. One for net device related
> > devices and one for generic feature bits.
> > 
> > In this patch we parse only net device specific features. Support for
> > other devices can be added later. If the device queried is not a net
> > device, we print its bit number only.
> > 
> > Examples:
> > 1.
> > $ vdpa dev config show vdpa-a
> > vdpa-a: mac 00:00:00:00:88:88 link up link_announce false max_vq_pairs 3 \
> >          mtu 1500
> >    negotiated_features CSUM GUEST_CSUM MTU MAC HOST_TSO4 HOST_TSO6 STATUS \
> >                        CTRL_VQ MQ CTRL_MAC_ADDR VERSION_1 ACCESS_PLATFORM
> > 
> > 2. json output
> > $ vdpa -j dev config show vdpa-a
> > {"config":{"vdpa-a":{"mac":"00:00:00:00:88:88","link":"up","link_announce":false, \
> >    "max_vq_pairs":3,"mtu":1500,"negotiated_features":["CSUM","GUEST_CSUM","MTU", \
> >    "MAC","HOST_TSO4","HOST_TSO6","STATUS","CTRL_VQ","MQ","CTRL_MAC_ADDR", \
> >    "VERSION_1","ACCESS_PLATFORM"]}}}
> > 
> > 3. pretty json
> > $ vdpa -jp dev config show vdpa-a
> > {
> >      "config": {
> >          "vdpa-a": {
> >              "mac": "00:00:00:00:88:88",
> >              "link ": "up",
> >              "link_announce ": false,
> >              "max_vq_pairs": 3,
> >              "mtu": 1500,
> >              "negotiated_features": [
> > "CSUM","GUEST_CSUM","MTU","MAC","HOST_TSO4","HOST_TSO6","STATUS","CTRL_VQ", \
> > "MQ","CTRL_MAC_ADDR","VERSION_1","ACCESS_PLATFORM" ]
> >          }
> >      }
> > }
> > 
> > Signed-off-by: Eli Cohen <elic@nvidia.com>
> > ---
> >   vdpa/include/uapi/linux/vdpa.h |   2 +
> >   vdpa/vdpa.c                    | 126 +++++++++++++++++++++++++++++++--
> >   2 files changed, 124 insertions(+), 4 deletions(-)
> > 
> > diff --git a/vdpa/include/uapi/linux/vdpa.h b/vdpa/include/uapi/linux/vdpa.h
> > index b7eab069988a..748c350450b2 100644
> > --- a/vdpa/include/uapi/linux/vdpa.h
> > +++ b/vdpa/include/uapi/linux/vdpa.h
> > @@ -40,6 +40,8 @@ enum vdpa_attr {
> >   	VDPA_ATTR_DEV_NET_CFG_MAX_VQP,		/* u16 */
> >   	VDPA_ATTR_DEV_NET_CFG_MTU,		/* u16 */
> > +	VDPA_ATTR_DEV_NEGOTIATED_FEATURES,	/* u64 */
> > +
> >   	/* new attributes must be added above here */
> >   	VDPA_ATTR_MAX,
> >   };
> > diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c
> > index 4ccb564872a0..7deab710913d 100644
> > --- a/vdpa/vdpa.c
> > +++ b/vdpa/vdpa.c
> > @@ -10,6 +10,7 @@
> >   #include <linux/virtio_net.h>
> >   #include <linux/netlink.h>
> >   #include <libmnl/libmnl.h>
> > +#include <linux/virtio_ring.h>
> >   #include "mnl_utils.h"
> >   #include <rt_names.h>
> > @@ -78,6 +79,7 @@ static const enum mnl_attr_data_type vdpa_policy[VDPA_ATTR_MAX + 1] = {
> >   	[VDPA_ATTR_DEV_VENDOR_ID] = MNL_TYPE_U32,
> >   	[VDPA_ATTR_DEV_MAX_VQS] = MNL_TYPE_U32,
> >   	[VDPA_ATTR_DEV_MAX_VQ_SIZE] = MNL_TYPE_U16,
> > +	[VDPA_ATTR_DEV_NEGOTIATED_FEATURES] = MNL_TYPE_U64,
> >   };
> >   static int attr_cb(const struct nlattr *attr, void *data)
> > @@ -385,17 +387,120 @@ static const char *parse_class(int num)
> >   	return class ? class : "< unknown class >";
> >   }
> > +static const char * const net_feature_strs[64] = {
> > +	[VIRTIO_NET_F_CSUM] = "CSUM",
> > +	[VIRTIO_NET_F_GUEST_CSUM] = "GUEST_CSUM",
> > +	[VIRTIO_NET_F_CTRL_GUEST_OFFLOADS] = "CTRL_GUEST_OFFLOADS",
> > +	[VIRTIO_NET_F_MTU] = "MTU",
> > +	[VIRTIO_NET_F_MAC] = "MAC",
> > +	[VIRTIO_NET_F_GUEST_TSO4] = "GUEST_TSO4",
> > +	[VIRTIO_NET_F_GUEST_TSO6] = "GUEST_TSO6",
> > +	[VIRTIO_NET_F_GUEST_ECN] = "GUEST_ECN",
> > +	[VIRTIO_NET_F_GUEST_UFO] = "GUEST_UFO",
> > +	[VIRTIO_NET_F_HOST_TSO4] = "HOST_TSO4",
> > +	[VIRTIO_NET_F_HOST_TSO6] = "HOST_TSO6",
> > +	[VIRTIO_NET_F_HOST_ECN] = "HOST_ECN",
> > +	[VIRTIO_NET_F_HOST_UFO] = "HOST_UFO",
> > +	[VIRTIO_NET_F_MRG_RXBUF] = "MRG_RXBUF",
> > +	[VIRTIO_NET_F_STATUS] = "STATUS",
> > +	[VIRTIO_NET_F_CTRL_VQ] = "CTRL_VQ",
> > +	[VIRTIO_NET_F_CTRL_RX] = "CTRL_RX",
> > +	[VIRTIO_NET_F_CTRL_VLAN] = "CTRL_VLAN",
> > +	[VIRTIO_NET_F_CTRL_RX_EXTRA] = "CTRL_RX_EXTRA",
> > +	[VIRTIO_NET_F_GUEST_ANNOUNCE] = "GUEST_ANNOUNCE",
> > +	[VIRTIO_NET_F_MQ] = "MQ",
> > +	[VIRTIO_F_NOTIFY_ON_EMPTY] = "NOTIFY_ON_EMPTY",
> > +	[VIRTIO_NET_F_CTRL_MAC_ADDR] = "CTRL_MAC_ADDR",
> > +	[VIRTIO_F_ANY_LAYOUT] = "ANY_LAYOUT",
> > +	[VIRTIO_NET_F_RSC_EXT] = "RSC_EXT",
> > +	[VIRTIO_NET_F_HASH_REPORT] = "HASH_REPORT",
> > +	[VIRTIO_NET_F_RSS] = "RSS",
> > +	[VIRTIO_NET_F_STANDBY] = "STANDBY",
> > +	[VIRTIO_NET_F_SPEED_DUPLEX] = "SPEED_DUPLEX",
> > +};
> > +
> > +#define VIRTIO_F_IN_ORDER 35
> > +#define VIRTIO_F_NOTIFICATION_DATA 38
> > +#define VDPA_EXT_FEATURES_SZ (VIRTIO_TRANSPORT_F_END - \
> > +			      VIRTIO_TRANSPORT_F_START + 1)
> > +
> > +static const char * const ext_feature_strs[VDPA_EXT_FEATURES_SZ] = {
> > +	[VIRTIO_RING_F_INDIRECT_DESC - VIRTIO_TRANSPORT_F_START] = "RING_INDIRECT_DESC",
> > +	[VIRTIO_RING_F_EVENT_IDX - VIRTIO_TRANSPORT_F_START] = "RING_EVENT_IDX",
> > +	[VIRTIO_F_VERSION_1 - VIRTIO_TRANSPORT_F_START] = "VERSION_1",
> > +	[VIRTIO_F_ACCESS_PLATFORM - VIRTIO_TRANSPORT_F_START] = "ACCESS_PLATFORM",
> > +	[VIRTIO_F_RING_PACKED - VIRTIO_TRANSPORT_F_START] = "RING_PACKED",
> > +	[VIRTIO_F_IN_ORDER - VIRTIO_TRANSPORT_F_START] = "IN_ORDER",
> > +	[VIRTIO_F_ORDER_PLATFORM - VIRTIO_TRANSPORT_F_START] = "ORDER_PLATFORM",
> > +	[VIRTIO_F_SR_IOV - VIRTIO_TRANSPORT_F_START] = "SR_IOV",
> > +	[VIRTIO_F_NOTIFICATION_DATA - VIRTIO_TRANSPORT_F_START] = "NOTIFICATION_DATA",
> > +};
> > +
> > +static void print_net_features(struct vdpa *vdpa, uint64_t features, bool maxf)
> > +{
> > +	const char *s;
> > +	int i;
> > +
> > +	if (maxf)
> This could use a better name. mgmtdevf maybe?

Will change.

> > +		pr_out_array_start(vdpa, "dev_features");
> > +	else
> > +		pr_out_array_start(vdpa, "negotiated_features");
> > +
> > +	for (i = 0; i < 64; i++) {
> > +		if (!(features & (1ULL << i)))
> > +			continue;
> > +
> > +		if (i >= VIRTIO_TRANSPORT_F_START && i <= VIRTIO_TRANSPORT_F_END)
> > +			s = ext_feature_strs[i - VIRTIO_TRANSPORT_F_START];
> > +		else
> > +			s = net_feature_strs[i];
> > +
> > +		if (!s)
> > +			print_uint(PRINT_ANY, NULL, " unrecognized_bit_%d", i);
> > +		else
> > +			print_string(PRINT_ANY, NULL, " %s", s);
> > +	}
> > +	pr_out_array_end(vdpa);
> > +}
> > +
> > +static void print_generic_features(struct vdpa *vdpa, uint64_t features, bool maxf)
> > +{
> > +	const char *s;
> > +	int i;
> > +
> > +	if (maxf)
> > +		pr_out_array_start(vdpa, "dev_features");
> > +	else
> > +		pr_out_array_start(vdpa, "negotiated_features");
> > +
> > +	for (i = 0; i < 64; i++) {
> > +		if (!(features & (1ULL << i)))
> > +			continue;
> > +
> > +		if (i >= VIRTIO_TRANSPORT_F_START && i <= VIRTIO_TRANSPORT_F_END)
> > +			s = ext_feature_strs[i - VIRTIO_TRANSPORT_F_START];
> > +		else
> > +			s = NULL;
> > +
> > +		if (!s)
> > +			print_uint(PRINT_ANY, NULL, " bit_%d", i);
> > +		else
> > +			print_string(PRINT_ANY, NULL, " %s", s);
> > +	}
> > +	pr_out_array_end(vdpa);
> > +}
> > +
> It looks like most of the implantation in the above two functions are the
> same and the rest are similar. I wonder if can consolidate into one by
> adding a virtio type (dev_id) argument? You may create another global static
> array that helps mapping VIRTIO_ID_NET to net_feature_strs?
> 

So maybe just create multidimensional array of to find the string for a
device_id/bit number combination and simplify the code above. And for
now just populate it for net devices. For undefined just print "bit_n".

> >   static void pr_out_mgmtdev_show(struct vdpa *vdpa, const struct nlmsghdr *nlh,
> >   				struct nlattr **tb)
> >   {
> > +	uint64_t classes = 0;
> >   	const char *class;
> >   	unsigned int i;
> >   	pr_out_handle_start(vdpa, tb);
> >   	if (tb[VDPA_ATTR_MGMTDEV_SUPPORTED_CLASSES]) {
> > -		uint64_t classes = mnl_attr_get_u64(tb[VDPA_ATTR_MGMTDEV_SUPPORTED_CLASSES]);
> > -
> > +		classes = mnl_attr_get_u64(tb[VDPA_ATTR_MGMTDEV_SUPPORTED_CLASSES]);
> >   		pr_out_array_start(vdpa, "supported_classes");
> This looks like a minor adjustment to the existing code? Not sure if worth
> another patch though.

I will remove it from this patch.
> 
> >   		for (i = 1; i < 64; i++) {
> > @@ -579,9 +684,10 @@ static int cmd_dev_del(struct vdpa *vdpa,  int argc, char **argv)
> >   	return mnlu_gen_socket_sndrcv(&vdpa->nlg, nlh, NULL, NULL);
> >   }
> > -static void pr_out_dev_net_config(struct nlattr **tb)
> > +static void pr_out_dev_net_config(struct vdpa *vdpa, struct nlattr **tb)
> >   {
> >   	SPRINT_BUF(macaddr);
> > +	uint64_t val_u64;
> >   	uint16_t val_u16;
> >   	if (tb[VDPA_ATTR_DEV_NET_CFG_MACADDR]) {
> > @@ -610,6 +716,18 @@ static void pr_out_dev_net_config(struct nlattr **tb)
> >   		val_u16 = mnl_attr_get_u16(tb[VDPA_ATTR_DEV_NET_CFG_MTU]);
> >   		print_uint(PRINT_ANY, "mtu", "mtu %d ", val_u16);
> >   	}
> > +	if (tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]) {
> > +		uint16_t dev_id = 0;
> > +
> > +		if (tb[VDPA_ATTR_DEV_ID])
> > +			dev_id = mnl_attr_get_u32(tb[VDPA_ATTR_DEV_ID]);
> > +
> > +		val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]);
> > +		if (tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES] && dev_id == VIRTIO_ID_NET)
> > +			print_net_features(vdpa, val_u64, false);
> > +		else
> > +			print_generic_features(vdpa, val_u64, true);
> Why the last arg is true? That would output the dev_features line instead.
> 
Will fix.

> -Siwei
> 
> > +	}
> >   }
> >   static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb)
> > @@ -619,7 +737,7 @@ static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb)
> >   	pr_out_vdev_handle_start(vdpa, tb);
> >   	switch (device_id) {
> >   	case VIRTIO_ID_NET:
> > -		pr_out_dev_net_config(tb);
> > +		pr_out_dev_net_config(vdpa, tb);
> >   		break;
> >   	default:
> >   		break;
>
Si-Wei Liu Feb. 16, 2022, 9:42 p.m. UTC | #3
On 2/15/2022 7:16 AM, Eli Cohen wrote:
> On Fri, Feb 11, 2022 at 05:15:51PM -0800, Si-Wei Liu wrote:
>>
>> On 2/10/2022 5:31 AM, Eli Cohen wrote:
>>> When reading the configuration of a vdpa device, check if the
>>> VDPA_ATTR_DEV_NEGOTIATED_FEATURES is available. If it is, parse the
>>> feature bits and print a string representation of each of the feature
>>> bits.
>>>
>>> We keep the strings in two different arrays. One for net device related
>>> devices and one for generic feature bits.
>>>
>>> In this patch we parse only net device specific features. Support for
>>> other devices can be added later. If the device queried is not a net
>>> device, we print its bit number only.
>>>
>>> Examples:
>>> 1.
>>> $ vdpa dev config show vdpa-a
>>> vdpa-a: mac 00:00:00:00:88:88 link up link_announce false max_vq_pairs 3 \
>>>           mtu 1500
>>>     negotiated_features CSUM GUEST_CSUM MTU MAC HOST_TSO4 HOST_TSO6 STATUS \
>>>                         CTRL_VQ MQ CTRL_MAC_ADDR VERSION_1 ACCESS_PLATFORM
>>>
>>> 2. json output
>>> $ vdpa -j dev config show vdpa-a
>>> {"config":{"vdpa-a":{"mac":"00:00:00:00:88:88","link":"up","link_announce":false, \
>>>     "max_vq_pairs":3,"mtu":1500,"negotiated_features":["CSUM","GUEST_CSUM","MTU", \
>>>     "MAC","HOST_TSO4","HOST_TSO6","STATUS","CTRL_VQ","MQ","CTRL_MAC_ADDR", \
>>>     "VERSION_1","ACCESS_PLATFORM"]}}}
>>>
>>> 3. pretty json
>>> $ vdpa -jp dev config show vdpa-a
>>> {
>>>       "config": {
>>>           "vdpa-a": {
>>>               "mac": "00:00:00:00:88:88",
>>>               "link ": "up",
>>>               "link_announce ": false,
>>>               "max_vq_pairs": 3,
>>>               "mtu": 1500,
>>>               "negotiated_features": [
>>> "CSUM","GUEST_CSUM","MTU","MAC","HOST_TSO4","HOST_TSO6","STATUS","CTRL_VQ", \
>>> "MQ","CTRL_MAC_ADDR","VERSION_1","ACCESS_PLATFORM" ]
>>>           }
>>>       }
>>> }
>>>
>>> Signed-off-by: Eli Cohen <elic@nvidia.com>
>>> ---
>>>    vdpa/include/uapi/linux/vdpa.h |   2 +
>>>    vdpa/vdpa.c                    | 126 +++++++++++++++++++++++++++++++--
>>>    2 files changed, 124 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/vdpa/include/uapi/linux/vdpa.h b/vdpa/include/uapi/linux/vdpa.h
>>> index b7eab069988a..748c350450b2 100644
>>> --- a/vdpa/include/uapi/linux/vdpa.h
>>> +++ b/vdpa/include/uapi/linux/vdpa.h
>>> @@ -40,6 +40,8 @@ enum vdpa_attr {
>>>    	VDPA_ATTR_DEV_NET_CFG_MAX_VQP,		/* u16 */
>>>    	VDPA_ATTR_DEV_NET_CFG_MTU,		/* u16 */
>>> +	VDPA_ATTR_DEV_NEGOTIATED_FEATURES,	/* u64 */
>>> +
>>>    	/* new attributes must be added above here */
>>>    	VDPA_ATTR_MAX,
>>>    };
>>> diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c
>>> index 4ccb564872a0..7deab710913d 100644
>>> --- a/vdpa/vdpa.c
>>> +++ b/vdpa/vdpa.c
>>> @@ -10,6 +10,7 @@
>>>    #include <linux/virtio_net.h>
>>>    #include <linux/netlink.h>
>>>    #include <libmnl/libmnl.h>
>>> +#include <linux/virtio_ring.h>
>>>    #include "mnl_utils.h"
>>>    #include <rt_names.h>
>>> @@ -78,6 +79,7 @@ static const enum mnl_attr_data_type vdpa_policy[VDPA_ATTR_MAX + 1] = {
>>>    	[VDPA_ATTR_DEV_VENDOR_ID] = MNL_TYPE_U32,
>>>    	[VDPA_ATTR_DEV_MAX_VQS] = MNL_TYPE_U32,
>>>    	[VDPA_ATTR_DEV_MAX_VQ_SIZE] = MNL_TYPE_U16,
>>> +	[VDPA_ATTR_DEV_NEGOTIATED_FEATURES] = MNL_TYPE_U64,
>>>    };
>>>    static int attr_cb(const struct nlattr *attr, void *data)
>>> @@ -385,17 +387,120 @@ static const char *parse_class(int num)
>>>    	return class ? class : "< unknown class >";
>>>    }
>>> +static const char * const net_feature_strs[64] = {
>>> +	[VIRTIO_NET_F_CSUM] = "CSUM",
>>> +	[VIRTIO_NET_F_GUEST_CSUM] = "GUEST_CSUM",
>>> +	[VIRTIO_NET_F_CTRL_GUEST_OFFLOADS] = "CTRL_GUEST_OFFLOADS",
>>> +	[VIRTIO_NET_F_MTU] = "MTU",
>>> +	[VIRTIO_NET_F_MAC] = "MAC",
>>> +	[VIRTIO_NET_F_GUEST_TSO4] = "GUEST_TSO4",
>>> +	[VIRTIO_NET_F_GUEST_TSO6] = "GUEST_TSO6",
>>> +	[VIRTIO_NET_F_GUEST_ECN] = "GUEST_ECN",
>>> +	[VIRTIO_NET_F_GUEST_UFO] = "GUEST_UFO",
>>> +	[VIRTIO_NET_F_HOST_TSO4] = "HOST_TSO4",
>>> +	[VIRTIO_NET_F_HOST_TSO6] = "HOST_TSO6",
>>> +	[VIRTIO_NET_F_HOST_ECN] = "HOST_ECN",
>>> +	[VIRTIO_NET_F_HOST_UFO] = "HOST_UFO",
>>> +	[VIRTIO_NET_F_MRG_RXBUF] = "MRG_RXBUF",
>>> +	[VIRTIO_NET_F_STATUS] = "STATUS",
>>> +	[VIRTIO_NET_F_CTRL_VQ] = "CTRL_VQ",
>>> +	[VIRTIO_NET_F_CTRL_RX] = "CTRL_RX",
>>> +	[VIRTIO_NET_F_CTRL_VLAN] = "CTRL_VLAN",
>>> +	[VIRTIO_NET_F_CTRL_RX_EXTRA] = "CTRL_RX_EXTRA",
>>> +	[VIRTIO_NET_F_GUEST_ANNOUNCE] = "GUEST_ANNOUNCE",
>>> +	[VIRTIO_NET_F_MQ] = "MQ",
>>> +	[VIRTIO_F_NOTIFY_ON_EMPTY] = "NOTIFY_ON_EMPTY",
>>> +	[VIRTIO_NET_F_CTRL_MAC_ADDR] = "CTRL_MAC_ADDR",
>>> +	[VIRTIO_F_ANY_LAYOUT] = "ANY_LAYOUT",
>>> +	[VIRTIO_NET_F_RSC_EXT] = "RSC_EXT",
>>> +	[VIRTIO_NET_F_HASH_REPORT] = "HASH_REPORT",
>>> +	[VIRTIO_NET_F_RSS] = "RSS",
>>> +	[VIRTIO_NET_F_STANDBY] = "STANDBY",
>>> +	[VIRTIO_NET_F_SPEED_DUPLEX] = "SPEED_DUPLEX",
>>> +};
>>> +
>>> +#define VIRTIO_F_IN_ORDER 35
>>> +#define VIRTIO_F_NOTIFICATION_DATA 38
>>> +#define VDPA_EXT_FEATURES_SZ (VIRTIO_TRANSPORT_F_END - \
>>> +			      VIRTIO_TRANSPORT_F_START + 1)
>>> +
>>> +static const char * const ext_feature_strs[VDPA_EXT_FEATURES_SZ] = {
>>> +	[VIRTIO_RING_F_INDIRECT_DESC - VIRTIO_TRANSPORT_F_START] = "RING_INDIRECT_DESC",
>>> +	[VIRTIO_RING_F_EVENT_IDX - VIRTIO_TRANSPORT_F_START] = "RING_EVENT_IDX",
>>> +	[VIRTIO_F_VERSION_1 - VIRTIO_TRANSPORT_F_START] = "VERSION_1",
>>> +	[VIRTIO_F_ACCESS_PLATFORM - VIRTIO_TRANSPORT_F_START] = "ACCESS_PLATFORM",
>>> +	[VIRTIO_F_RING_PACKED - VIRTIO_TRANSPORT_F_START] = "RING_PACKED",
>>> +	[VIRTIO_F_IN_ORDER - VIRTIO_TRANSPORT_F_START] = "IN_ORDER",
>>> +	[VIRTIO_F_ORDER_PLATFORM - VIRTIO_TRANSPORT_F_START] = "ORDER_PLATFORM",
>>> +	[VIRTIO_F_SR_IOV - VIRTIO_TRANSPORT_F_START] = "SR_IOV",
>>> +	[VIRTIO_F_NOTIFICATION_DATA - VIRTIO_TRANSPORT_F_START] = "NOTIFICATION_DATA",
>>> +};
>>> +
>>> +static void print_net_features(struct vdpa *vdpa, uint64_t features, bool maxf)
>>> +{
>>> +	const char *s;
>>> +	int i;
>>> +
>>> +	if (maxf)
>> This could use a better name. mgmtdevf maybe?
> Will change.
>
>>> +		pr_out_array_start(vdpa, "dev_features");
>>> +	else
>>> +		pr_out_array_start(vdpa, "negotiated_features");
>>> +
>>> +	for (i = 0; i < 64; i++) {
>>> +		if (!(features & (1ULL << i)))
>>> +			continue;
>>> +
>>> +		if (i >= VIRTIO_TRANSPORT_F_START && i <= VIRTIO_TRANSPORT_F_END)
>>> +			s = ext_feature_strs[i - VIRTIO_TRANSPORT_F_START];
>>> +		else
>>> +			s = net_feature_strs[i];
>>> +
>>> +		if (!s)
>>> +			print_uint(PRINT_ANY, NULL, " unrecognized_bit_%d", i);
>>> +		else
>>> +			print_string(PRINT_ANY, NULL, " %s", s);
>>> +	}
>>> +	pr_out_array_end(vdpa);
>>> +}
>>> +
>>> +static void print_generic_features(struct vdpa *vdpa, uint64_t features, bool maxf)
>>> +{
>>> +	const char *s;
>>> +	int i;
>>> +
>>> +	if (maxf)
>>> +		pr_out_array_start(vdpa, "dev_features");
>>> +	else
>>> +		pr_out_array_start(vdpa, "negotiated_features");
>>> +
>>> +	for (i = 0; i < 64; i++) {
>>> +		if (!(features & (1ULL << i)))
>>> +			continue;
>>> +
>>> +		if (i >= VIRTIO_TRANSPORT_F_START && i <= VIRTIO_TRANSPORT_F_END)
>>> +			s = ext_feature_strs[i - VIRTIO_TRANSPORT_F_START];
>>> +		else
>>> +			s = NULL;
>>> +
>>> +		if (!s)
>>> +			print_uint(PRINT_ANY, NULL, " bit_%d", i);
>>> +		else
>>> +			print_string(PRINT_ANY, NULL, " %s", s);
>>> +	}
>>> +	pr_out_array_end(vdpa);
>>> +}
>>> +
>> It looks like most of the implantation in the above two functions are the
>> same and the rest are similar. I wonder if can consolidate into one by
>> adding a virtio type (dev_id) argument? You may create another global static
>> array that helps mapping VIRTIO_ID_NET to net_feature_strs?
>>
> So maybe just create multidimensional array of to find the string for a
> device_id/bit number combination and simplify the code above. And for
> now just populate it for net devices. For undefined just print "bit_n".
Yeah, that sounds fine, too.

Thanks,
-Siwei

>
>>>    static void pr_out_mgmtdev_show(struct vdpa *vdpa, const struct nlmsghdr *nlh,
>>>    				struct nlattr **tb)
>>>    {
>>> +	uint64_t classes = 0;
>>>    	const char *class;
>>>    	unsigned int i;
>>>    	pr_out_handle_start(vdpa, tb);
>>>    	if (tb[VDPA_ATTR_MGMTDEV_SUPPORTED_CLASSES]) {
>>> -		uint64_t classes = mnl_attr_get_u64(tb[VDPA_ATTR_MGMTDEV_SUPPORTED_CLASSES]);
>>> -
>>> +		classes = mnl_attr_get_u64(tb[VDPA_ATTR_MGMTDEV_SUPPORTED_CLASSES]);
>>>    		pr_out_array_start(vdpa, "supported_classes");
>> This looks like a minor adjustment to the existing code? Not sure if worth
>> another patch though.
> I will remove it from this patch.
>>>    		for (i = 1; i < 64; i++) {
>>> @@ -579,9 +684,10 @@ static int cmd_dev_del(struct vdpa *vdpa,  int argc, char **argv)
>>>    	return mnlu_gen_socket_sndrcv(&vdpa->nlg, nlh, NULL, NULL);
>>>    }
>>> -static void pr_out_dev_net_config(struct nlattr **tb)
>>> +static void pr_out_dev_net_config(struct vdpa *vdpa, struct nlattr **tb)
>>>    {
>>>    	SPRINT_BUF(macaddr);
>>> +	uint64_t val_u64;
>>>    	uint16_t val_u16;
>>>    	if (tb[VDPA_ATTR_DEV_NET_CFG_MACADDR]) {
>>> @@ -610,6 +716,18 @@ static void pr_out_dev_net_config(struct nlattr **tb)
>>>    		val_u16 = mnl_attr_get_u16(tb[VDPA_ATTR_DEV_NET_CFG_MTU]);
>>>    		print_uint(PRINT_ANY, "mtu", "mtu %d ", val_u16);
>>>    	}
>>> +	if (tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]) {
>>> +		uint16_t dev_id = 0;
>>> +
>>> +		if (tb[VDPA_ATTR_DEV_ID])
>>> +			dev_id = mnl_attr_get_u32(tb[VDPA_ATTR_DEV_ID]);
>>> +
>>> +		val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]);
>>> +		if (tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES] && dev_id == VIRTIO_ID_NET)
>>> +			print_net_features(vdpa, val_u64, false);
>>> +		else
>>> +			print_generic_features(vdpa, val_u64, true);
>> Why the last arg is true? That would output the dev_features line instead.
>>
> Will fix.
>
>> -Siwei
>>
>>> +	}
>>>    }
>>>    static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb)
>>> @@ -619,7 +737,7 @@ static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb)
>>>    	pr_out_vdev_handle_start(vdpa, tb);
>>>    	switch (device_id) {
>>>    	case VIRTIO_ID_NET:
>>> -		pr_out_dev_net_config(tb);
>>> +		pr_out_dev_net_config(vdpa, tb);
>>>    		break;
>>>    	default:
>>>    		break;
diff mbox series

Patch

diff --git a/vdpa/include/uapi/linux/vdpa.h b/vdpa/include/uapi/linux/vdpa.h
index b7eab069988a..748c350450b2 100644
--- a/vdpa/include/uapi/linux/vdpa.h
+++ b/vdpa/include/uapi/linux/vdpa.h
@@ -40,6 +40,8 @@  enum vdpa_attr {
 	VDPA_ATTR_DEV_NET_CFG_MAX_VQP,		/* u16 */
 	VDPA_ATTR_DEV_NET_CFG_MTU,		/* u16 */
 
+	VDPA_ATTR_DEV_NEGOTIATED_FEATURES,	/* u64 */
+
 	/* new attributes must be added above here */
 	VDPA_ATTR_MAX,
 };
diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c
index 4ccb564872a0..7deab710913d 100644
--- a/vdpa/vdpa.c
+++ b/vdpa/vdpa.c
@@ -10,6 +10,7 @@ 
 #include <linux/virtio_net.h>
 #include <linux/netlink.h>
 #include <libmnl/libmnl.h>
+#include <linux/virtio_ring.h>
 #include "mnl_utils.h"
 #include <rt_names.h>
 
@@ -78,6 +79,7 @@  static const enum mnl_attr_data_type vdpa_policy[VDPA_ATTR_MAX + 1] = {
 	[VDPA_ATTR_DEV_VENDOR_ID] = MNL_TYPE_U32,
 	[VDPA_ATTR_DEV_MAX_VQS] = MNL_TYPE_U32,
 	[VDPA_ATTR_DEV_MAX_VQ_SIZE] = MNL_TYPE_U16,
+	[VDPA_ATTR_DEV_NEGOTIATED_FEATURES] = MNL_TYPE_U64,
 };
 
 static int attr_cb(const struct nlattr *attr, void *data)
@@ -385,17 +387,120 @@  static const char *parse_class(int num)
 	return class ? class : "< unknown class >";
 }
 
+static const char * const net_feature_strs[64] = {
+	[VIRTIO_NET_F_CSUM] = "CSUM",
+	[VIRTIO_NET_F_GUEST_CSUM] = "GUEST_CSUM",
+	[VIRTIO_NET_F_CTRL_GUEST_OFFLOADS] = "CTRL_GUEST_OFFLOADS",
+	[VIRTIO_NET_F_MTU] = "MTU",
+	[VIRTIO_NET_F_MAC] = "MAC",
+	[VIRTIO_NET_F_GUEST_TSO4] = "GUEST_TSO4",
+	[VIRTIO_NET_F_GUEST_TSO6] = "GUEST_TSO6",
+	[VIRTIO_NET_F_GUEST_ECN] = "GUEST_ECN",
+	[VIRTIO_NET_F_GUEST_UFO] = "GUEST_UFO",
+	[VIRTIO_NET_F_HOST_TSO4] = "HOST_TSO4",
+	[VIRTIO_NET_F_HOST_TSO6] = "HOST_TSO6",
+	[VIRTIO_NET_F_HOST_ECN] = "HOST_ECN",
+	[VIRTIO_NET_F_HOST_UFO] = "HOST_UFO",
+	[VIRTIO_NET_F_MRG_RXBUF] = "MRG_RXBUF",
+	[VIRTIO_NET_F_STATUS] = "STATUS",
+	[VIRTIO_NET_F_CTRL_VQ] = "CTRL_VQ",
+	[VIRTIO_NET_F_CTRL_RX] = "CTRL_RX",
+	[VIRTIO_NET_F_CTRL_VLAN] = "CTRL_VLAN",
+	[VIRTIO_NET_F_CTRL_RX_EXTRA] = "CTRL_RX_EXTRA",
+	[VIRTIO_NET_F_GUEST_ANNOUNCE] = "GUEST_ANNOUNCE",
+	[VIRTIO_NET_F_MQ] = "MQ",
+	[VIRTIO_F_NOTIFY_ON_EMPTY] = "NOTIFY_ON_EMPTY",
+	[VIRTIO_NET_F_CTRL_MAC_ADDR] = "CTRL_MAC_ADDR",
+	[VIRTIO_F_ANY_LAYOUT] = "ANY_LAYOUT",
+	[VIRTIO_NET_F_RSC_EXT] = "RSC_EXT",
+	[VIRTIO_NET_F_HASH_REPORT] = "HASH_REPORT",
+	[VIRTIO_NET_F_RSS] = "RSS",
+	[VIRTIO_NET_F_STANDBY] = "STANDBY",
+	[VIRTIO_NET_F_SPEED_DUPLEX] = "SPEED_DUPLEX",
+};
+
+#define VIRTIO_F_IN_ORDER 35
+#define VIRTIO_F_NOTIFICATION_DATA 38
+#define VDPA_EXT_FEATURES_SZ (VIRTIO_TRANSPORT_F_END - \
+			      VIRTIO_TRANSPORT_F_START + 1)
+
+static const char * const ext_feature_strs[VDPA_EXT_FEATURES_SZ] = {
+	[VIRTIO_RING_F_INDIRECT_DESC - VIRTIO_TRANSPORT_F_START] = "RING_INDIRECT_DESC",
+	[VIRTIO_RING_F_EVENT_IDX - VIRTIO_TRANSPORT_F_START] = "RING_EVENT_IDX",
+	[VIRTIO_F_VERSION_1 - VIRTIO_TRANSPORT_F_START] = "VERSION_1",
+	[VIRTIO_F_ACCESS_PLATFORM - VIRTIO_TRANSPORT_F_START] = "ACCESS_PLATFORM",
+	[VIRTIO_F_RING_PACKED - VIRTIO_TRANSPORT_F_START] = "RING_PACKED",
+	[VIRTIO_F_IN_ORDER - VIRTIO_TRANSPORT_F_START] = "IN_ORDER",
+	[VIRTIO_F_ORDER_PLATFORM - VIRTIO_TRANSPORT_F_START] = "ORDER_PLATFORM",
+	[VIRTIO_F_SR_IOV - VIRTIO_TRANSPORT_F_START] = "SR_IOV",
+	[VIRTIO_F_NOTIFICATION_DATA - VIRTIO_TRANSPORT_F_START] = "NOTIFICATION_DATA",
+};
+
+static void print_net_features(struct vdpa *vdpa, uint64_t features, bool maxf)
+{
+	const char *s;
+	int i;
+
+	if (maxf)
+		pr_out_array_start(vdpa, "dev_features");
+	else
+		pr_out_array_start(vdpa, "negotiated_features");
+
+	for (i = 0; i < 64; i++) {
+		if (!(features & (1ULL << i)))
+			continue;
+
+		if (i >= VIRTIO_TRANSPORT_F_START && i <= VIRTIO_TRANSPORT_F_END)
+			s = ext_feature_strs[i - VIRTIO_TRANSPORT_F_START];
+		else
+			s = net_feature_strs[i];
+
+		if (!s)
+			print_uint(PRINT_ANY, NULL, " unrecognized_bit_%d", i);
+		else
+			print_string(PRINT_ANY, NULL, " %s", s);
+	}
+	pr_out_array_end(vdpa);
+}
+
+static void print_generic_features(struct vdpa *vdpa, uint64_t features, bool maxf)
+{
+	const char *s;
+	int i;
+
+	if (maxf)
+		pr_out_array_start(vdpa, "dev_features");
+	else
+		pr_out_array_start(vdpa, "negotiated_features");
+
+	for (i = 0; i < 64; i++) {
+		if (!(features & (1ULL << i)))
+			continue;
+
+		if (i >= VIRTIO_TRANSPORT_F_START && i <= VIRTIO_TRANSPORT_F_END)
+			s = ext_feature_strs[i - VIRTIO_TRANSPORT_F_START];
+		else
+			s = NULL;
+
+		if (!s)
+			print_uint(PRINT_ANY, NULL, " bit_%d", i);
+		else
+			print_string(PRINT_ANY, NULL, " %s", s);
+	}
+	pr_out_array_end(vdpa);
+}
+
 static void pr_out_mgmtdev_show(struct vdpa *vdpa, const struct nlmsghdr *nlh,
 				struct nlattr **tb)
 {
+	uint64_t classes = 0;
 	const char *class;
 	unsigned int i;
 
 	pr_out_handle_start(vdpa, tb);
 
 	if (tb[VDPA_ATTR_MGMTDEV_SUPPORTED_CLASSES]) {
-		uint64_t classes = mnl_attr_get_u64(tb[VDPA_ATTR_MGMTDEV_SUPPORTED_CLASSES]);
-
+		classes = mnl_attr_get_u64(tb[VDPA_ATTR_MGMTDEV_SUPPORTED_CLASSES]);
 		pr_out_array_start(vdpa, "supported_classes");
 
 		for (i = 1; i < 64; i++) {
@@ -579,9 +684,10 @@  static int cmd_dev_del(struct vdpa *vdpa,  int argc, char **argv)
 	return mnlu_gen_socket_sndrcv(&vdpa->nlg, nlh, NULL, NULL);
 }
 
-static void pr_out_dev_net_config(struct nlattr **tb)
+static void pr_out_dev_net_config(struct vdpa *vdpa, struct nlattr **tb)
 {
 	SPRINT_BUF(macaddr);
+	uint64_t val_u64;
 	uint16_t val_u16;
 
 	if (tb[VDPA_ATTR_DEV_NET_CFG_MACADDR]) {
@@ -610,6 +716,18 @@  static void pr_out_dev_net_config(struct nlattr **tb)
 		val_u16 = mnl_attr_get_u16(tb[VDPA_ATTR_DEV_NET_CFG_MTU]);
 		print_uint(PRINT_ANY, "mtu", "mtu %d ", val_u16);
 	}
+	if (tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]) {
+		uint16_t dev_id = 0;
+
+		if (tb[VDPA_ATTR_DEV_ID])
+			dev_id = mnl_attr_get_u32(tb[VDPA_ATTR_DEV_ID]);
+
+		val_u64 = mnl_attr_get_u64(tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES]);
+		if (tb[VDPA_ATTR_DEV_NEGOTIATED_FEATURES] && dev_id == VIRTIO_ID_NET)
+			print_net_features(vdpa, val_u64, false);
+		else
+			print_generic_features(vdpa, val_u64, true);
+	}
 }
 
 static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb)
@@ -619,7 +737,7 @@  static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb)
 	pr_out_vdev_handle_start(vdpa, tb);
 	switch (device_id) {
 	case VIRTIO_ID_NET:
-		pr_out_dev_net_config(tb);
+		pr_out_dev_net_config(vdpa, tb);
 		break;
 	default:
 		break;