netdev - RE: [PATCH v7 2/4] vdpa: Allow for printing negotiated features of a device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <DM8PR12MB54007D9DEE61331CE6F1118FAB109@DM8PR12MB5400.namprd12.prod.outlook.com>
Date:   Tue, 15 Mar 2022 11:35:26 +0000
From:   Eli Cohen <elic@...dia.com>
To:     David Ahern <dsahern@...nel.org>,
        "stephen@...workplumber.org" <stephen@...workplumber.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "virtualization@...ts.linux-foundation.org" 
        <virtualization@...ts.linux-foundation.org>,
        "jasowang@...hat.com" <jasowang@...hat.com>,
        "si-wei.liu@...cle.com" <si-wei.liu@...cle.com>
CC:     "mst@...hat.com" <mst@...hat.com>,
        "lulu@...hat.com" <lulu@...hat.com>,
        Parav Pandit <parav@...dia.com>
Subject: RE: [PATCH v7 2/4] vdpa: Allow for printing negotiated features of a
 device

> 
> 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.