diff mbox series

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

Message ID 20220313171219.305089-3-elic@nvidia.com (mailing list archive)
State Not Applicable
Delegated to: Netdev Maintainers
Headers show
Series vdpa tool enhancements | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Eli Cohen March 13, 2022, 5:12 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. Standard presentation
$ vdpa dev config show vdpa-a
vdpa-a: mac 00:00:00:00:88:88 link up link_announce false max_vq_pairs 2 mtu 9000
  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":2,"mtu":9000,"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": 2,
            "mtu": 9000,
            "negotiated_features": [
"CSUM","GUEST_CSUM","MTU","MAC","HOST_TSO4","HOST_TSO6","STATUS","CTRL_VQ",\
"MQ","CTRL_MAC_ADDR","VERSION_1","ACCESS_PLATFORM" ]
        }
    }
}

Reviewed-by: Si-Wei Liu<si-wei.liu@oracle.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Eli Cohen <elic@nvidia.com>
---
 vdpa/vdpa.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 106 insertions(+), 2 deletions(-)

Comments

Stephen Hemminger March 13, 2022, 5:33 p.m. UTC | #1
On Sun, 13 Mar 2022 19:12:17 +0200
Eli Cohen <elic@nvidia.com> wrote:

> +			if (feature_strs)
> +				s = feature_strs[i];
> +			else
> +				s = NULL;

You really don't like trigraphs?
			s = feature_strs ? feature_strs[i] : NULL;
is more compact
Eli Cohen March 13, 2022, 6:04 p.m. UTC | #2
> 
> On Sun, 13 Mar 2022 19:12:17 +0200
> Eli Cohen <elic@nvidia.com> wrote:
> 
> > +			if (feature_strs)
> > +				s = feature_strs[i];
> > +			else
> > +				s = NULL;
> 
> You really don't like trigraphs?
> 			s = feature_strs ? feature_strs[i] : NULL;
> is more compact

If you insist I will send another version.
Let me know.
David Ahern March 14, 2022, 2:51 p.m. UTC | #3
On 3/13/22 12:04 PM, Eli Cohen wrote:
>>
>> On Sun, 13 Mar 2022 19:12:17 +0200
>> Eli Cohen <elic@nvidia.com> wrote:
>>
>>> +			if (feature_strs)
>>> +				s = feature_strs[i];
>>> +			else
>>> +				s = NULL;
>>
>> You really don't like trigraphs?
>> 			s = feature_strs ? feature_strs[i] : NULL;
>> is more compact
> 
> If you insist I will send another version.
> Let me know.

I'll change it before applying
Eli Cohen March 14, 2022, 2:54 p.m. UTC | #4
> 
> On 3/13/22 12:04 PM, Eli Cohen wrote:
> >>
> >> On Sun, 13 Mar 2022 19:12:17 +0200
> >> Eli Cohen <elic@nvidia.com> wrote:
> >>
> >>> +			if (feature_strs)
> >>> +				s = feature_strs[i];
> >>> +			else
> >>> +				s = NULL;
> >>
> >> You really don't like trigraphs?
> >> 			s = feature_strs ? feature_strs[i] : NULL;
> >> is more compact
> >
> > If you insist I will send another version.
> > Let me know.
> 
> I'll change it before applying

Cool, thanks.
David Ahern March 14, 2022, 3:07 p.m. UTC | #5
On 3/13/22 11:12 AM, Eli Cohen wrote:
> @@ -385,6 +388,97 @@ 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",

not very easy on the eyes. Please send a followup that column aligns the
strings. e.g.,

@@ -403,9 +403,9 @@ static const char *parse_class(int num)
 }

 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_CSUM]                     = "CSUM",
+       [VIRTIO_NET_F_GUEST_CSUM]               = "GUEST_CSUM",
+       [VIRTIO_NET_F_CTRL_GUEST_OFFLOADS]      = "CTRL_GUEST_OFFLOADS",
...


> +};
> +
> +#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",

and the entries here should be a macro to handle the
VIRTIO_TRANSPORT_F_START  offset with column aligned strings.
Eli Cohen March 14, 2022, 3:34 p.m. UTC | #6
> 
> On 3/13/22 11:12 AM, Eli Cohen wrote:
> > @@ -385,6 +388,97 @@ 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",
> 
> not very easy on the eyes. Please send a followup that column aligns the
> strings. e.g.,
> 
> @@ -403,9 +403,9 @@ static const char *parse_class(int num)
>  }
> 
>  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_CSUM]                     = "CSUM",
> +       [VIRTIO_NET_F_GUEST_CSUM]               = "GUEST_CSUM",
> +       [VIRTIO_NET_F_CTRL_GUEST_OFFLOADS]      = "CTRL_GUEST_OFFLOADS",
> ...
> 
> 
> > +};
> > +
> > +#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",
> 
> and the entries here should be a macro to handle the
> VIRTIO_TRANSPORT_F_START  offset with column aligned strings.
> 
Do you mean define a new macro for each line above?
I could align them nicely without new macros while being confined to column 88.
David Ahern March 14, 2022, 4 p.m. UTC | #7
On 3/14/22 9:34 AM, Eli Cohen wrote:
>> @@ -403,9 +403,9 @@ static const char *parse_class(int num)
>>  }
>>
>>  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_CSUM]                     = "CSUM",
>> +       [VIRTIO_NET_F_GUEST_CSUM]               = "GUEST_CSUM",
>> +       [VIRTIO_NET_F_CTRL_GUEST_OFFLOADS]      = "CTRL_GUEST_OFFLOADS",
>> ...
>>
>>
>>> +};
>>> +
>>> +#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",
>>
>> and the entries here should be a macro to handle the
>> VIRTIO_TRANSPORT_F_START  offset with column aligned strings.
>>
> Do you mean define a new macro for each line above?
> I could align them nicely without new macros while being confined to column 88.
> 

VIRTIO_TRANSPORT_F_START is redundant on each line; a macro can be used
to hide that and simplifies each line.
Eli Cohen March 15, 2022, 11:35 a.m. UTC | #8
> 
> On 3/13/22 11:12 AM, Eli Cohen wrote:
> > @@ -385,6 +388,97 @@ 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",
> 
> not very easy on the eyes. Please send a followup that column aligns the
> strings. e.g.,
> 
> @@ -403,9 +403,9 @@ static const char *parse_class(int num)
>  }
> 
>  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_CSUM]                     = "CSUM",
> +       [VIRTIO_NET_F_GUEST_CSUM]               = "GUEST_CSUM",
> +       [VIRTIO_NET_F_CTRL_GUEST_OFFLOADS]      = "CTRL_GUEST_OFFLOADS",
> ...
> 
> 
> > +};
> > +
> > +#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",
> 
> and the entries here should be a macro to handle the
> VIRTIO_TRANSPORT_F_START  offset with column aligned strings.
>

I guess I could define a macro here if all the flags would start with "VIRTIO_F_ =" but unfortunately,
the first two flags don't follow this pattern.
I could send kernel patches to replace VIRTIO_RING_F_INDIRECT_DESC and
VIRTIO_RING_F_EVENT_IDX to VIRTIO_F_RING_INDIRECT_DESC and VIRTIO_F_RING_EVENT_IDX
and then fix the format here.

If you meant something else, please be more specific.
David Ahern March 16, 2022, 3:46 p.m. UTC | #9
On 3/15/22 5:35 AM, Eli Cohen wrote:
>>> +};
>>> +
>>> +#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",
>>
>> and the entries here should be a macro to handle the
>> VIRTIO_TRANSPORT_F_START  offset with column aligned strings.
>>
> 
> I guess I could define a macro here if all the flags would start with "VIRTIO_F_ =" but unfortunately,
> the first two flags don't follow this pattern.
> I could send kernel patches to replace VIRTIO_RING_F_INDIRECT_DESC and
> VIRTIO_RING_F_EVENT_IDX to VIRTIO_F_RING_INDIRECT_DESC and VIRTIO_F_RING_EVENT_IDX
> and then fix the format here.
> 
> If you meant something else, please be more specific.

It can still work with the existing naming - it's the redundant use of
VIRTIO_TRANSPORT_F_START as an offset that is hidden by the macro
allowing the feature to string conversion to be easier to read.
diff mbox series

Patch

diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c
index 4ccb564872a0..f7b6e5f8a0bc 100644
--- a/vdpa/vdpa.c
+++ b/vdpa/vdpa.c
@@ -10,6 +10,8 @@ 
 #include <linux/virtio_net.h>
 #include <linux/netlink.h>
 #include <libmnl/libmnl.h>
+#include <linux/virtio_ring.h>
+#include <linux/virtio_config.h>
 #include "mnl_utils.h"
 #include <rt_names.h>
 
@@ -78,6 +80,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,6 +388,97 @@  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 const char * const *dev_to_feature_str[] = {
+	[VIRTIO_ID_NET] = net_feature_strs,
+};
+
+#define NUM_FEATURE_BITS 64
+
+static void print_features(struct vdpa *vdpa, uint64_t features, bool mgmtdevf,
+			   uint16_t dev_id)
+{
+	const char * const *feature_strs = NULL;
+	const char *s;
+	int i;
+
+	if (dev_id < ARRAY_SIZE(dev_to_feature_str))
+		feature_strs = dev_to_feature_str[dev_id];
+
+	if (mgmtdevf)
+		pr_out_array_start(vdpa, "dev_features");
+	else
+		pr_out_array_start(vdpa, "negotiated_features");
+
+	for (i = 0; i < NUM_FEATURE_BITS; i++) {
+		if (!(features & (1ULL << i)))
+			continue;
+
+		if (i < VIRTIO_TRANSPORT_F_START || i > VIRTIO_TRANSPORT_F_END) {
+			if (feature_strs)
+				s = feature_strs[i];
+			else
+				s = NULL;
+		} else {
+			s = ext_feature_strs[i - VIRTIO_TRANSPORT_F_START];
+		}
+		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)
 {
@@ -579,9 +673,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 +705,15 @@  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]);
+		print_features(vdpa, val_u64, false, dev_id);
+	}
 }
 
 static void pr_out_dev_config(struct vdpa *vdpa, struct nlattr **tb)
@@ -619,7 +723,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;