Re: [PATCH 5/8] vmxnet3, version 6

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

Summary should say what is done. Consider:

net/vmxnet3: support virtual HW version 6

On 5/3/22 07:22, Pankaj Gupta wrote:
> vmxnet3 version 6 supports some new features, including but
> not limited to:
> - Increased max MTU up to 9190
> - Increased max number of queues, both for RX and TX

RX -> Rx, TX -> Tx in accordnace with recommended spelling

> - Removes power-of-two limitations
> - Extended interrupt structures, required implementation for
>    additional number of queues

Please, be consistent in tense used above, "increased" vs "removes".

> Tested, using testpmd, for different hardware version on
> ESXi 7.0 Update 2.
> 
> Signed-off-by: Pankaj Gupta <pagupta@vmware.com>
> ---
>   drivers/net/vmxnet3/base/vmxnet3_defs.h | 135 +++++++++-----
>   drivers/net/vmxnet3/vmxnet3_ethdev.c    | 223 +++++++++++++++++-------
>   drivers/net/vmxnet3/vmxnet3_ethdev.h    |  10 +-
>   drivers/net/vmxnet3/vmxnet3_rxtx.c      |   2 +-
>   4 files changed, 260 insertions(+), 110 deletions(-)
> 
> diff --git a/drivers/net/vmxnet3/base/vmxnet3_defs.h b/drivers/net/vmxnet3/base/vmxnet3_defs.h
> index 8d62b3e116..ceac5d64db 100644
> --- a/drivers/net/vmxnet3/base/vmxnet3_defs.h
> +++ b/drivers/net/vmxnet3/base/vmxnet3_defs.h
> @@ -72,38 +72,42 @@
>   #endif
>   
>   typedef enum {
> -   VMXNET3_CMD_FIRST_SET = 0xCAFE0000,
> -   VMXNET3_CMD_ACTIVATE_DEV = VMXNET3_CMD_FIRST_SET,
> -   VMXNET3_CMD_QUIESCE_DEV,
> -   VMXNET3_CMD_RESET_DEV,
> -   VMXNET3_CMD_UPDATE_RX_MODE,
> -   VMXNET3_CMD_UPDATE_MAC_FILTERS,
> -   VMXNET3_CMD_UPDATE_VLAN_FILTERS,
> -   VMXNET3_CMD_UPDATE_RSSIDT,
> -   VMXNET3_CMD_UPDATE_IML,
> -   VMXNET3_CMD_UPDATE_PMCFG,
> -   VMXNET3_CMD_UPDATE_FEATURE,
> -   VMXNET3_CMD_STOP_EMULATION,
> -   VMXNET3_CMD_LOAD_PLUGIN,
> -   VMXNET3_CMD_ACTIVATE_VF,
> -   VMXNET3_CMD_RESERVED3,
> -   VMXNET3_CMD_RESERVED4,
> -   VMXNET3_CMD_REGISTER_MEMREGS,
> -   VMXNET3_CMD_SET_RSS_FIELDS,
> -
> -   VMXNET3_CMD_FIRST_GET = 0xF00D0000,
> -   VMXNET3_CMD_GET_QUEUE_STATUS = VMXNET3_CMD_FIRST_GET,
> -   VMXNET3_CMD_GET_STATS,
> -   VMXNET3_CMD_GET_LINK,
> -   VMXNET3_CMD_GET_PERM_MAC_LO,
> -   VMXNET3_CMD_GET_PERM_MAC_HI,
> -   VMXNET3_CMD_GET_DID_LO,
> -   VMXNET3_CMD_GET_DID_HI,
> -   VMXNET3_CMD_GET_DEV_EXTRA_INFO,
> -   VMXNET3_CMD_GET_CONF_INTR,
> -   VMXNET3_CMD_GET_ADAPTIVE_RING_INFO,
> -   VMXNET3_CMD_GET_TXDATA_DESC_SIZE,
> -   VMXNET3_CMD_RESERVED5,
> +	VMXNET3_CMD_FIRST_SET = 0xCAFE0000,
> +	VMXNET3_CMD_ACTIVATE_DEV = VMXNET3_CMD_FIRST_SET,
> +	VMXNET3_CMD_QUIESCE_DEV,
> +	VMXNET3_CMD_RESET_DEV,
> +	VMXNET3_CMD_UPDATE_RX_MODE,
> +	VMXNET3_CMD_UPDATE_MAC_FILTERS,
> +	VMXNET3_CMD_UPDATE_VLAN_FILTERS,
> +	VMXNET3_CMD_UPDATE_RSSIDT,
> +	VMXNET3_CMD_UPDATE_IML,
> +	VMXNET3_CMD_UPDATE_PMCFG,
> +	VMXNET3_CMD_UPDATE_FEATURE,
> +	VMXNET3_CMD_STOP_EMULATION,
> +	VMXNET3_CMD_LOAD_PLUGIN,
> +	VMXNET3_CMD_ACTIVATE_VF,
> +	VMXNET3_CMD_RESERVED3,
> +	VMXNET3_CMD_RESERVED4,
> +	VMXNET3_CMD_REGISTER_MEMREGS,
> +	VMXNET3_CMD_SET_RSS_FIELDS,
> +
> +	VMXNET3_CMD_FIRST_GET = 0xF00D0000,
> +	VMXNET3_CMD_GET_QUEUE_STATUS = VMXNET3_CMD_FIRST_GET,
> +	VMXNET3_CMD_GET_STATS,
> +	VMXNET3_CMD_GET_LINK,
> +	VMXNET3_CMD_GET_PERM_MAC_LO,
> +	VMXNET3_CMD_GET_PERM_MAC_HI,
> +	VMXNET3_CMD_GET_DID_LO,
> +	VMXNET3_CMD_GET_DID_HI,
> +	VMXNET3_CMD_GET_DEV_EXTRA_INFO,
> +	VMXNET3_CMD_GET_CONF_INTR,
> +	VMXNET3_CMD_GET_ADAPTIVE_RING_INFO,
> +	VMXNET3_CMD_GET_TXDATA_DESC_SIZE,
> +	VMXNET3_CMD_RESERVED5,
> +	VMXNET3_CMD_RESERVED6,
> +	VMXNET3_CMD_RESERVED7,
> +	VMXNET3_CMD_RESERVED8,
> +	VMXNET3_CMD_GET_MAX_QUEUES_CONF,

It is hard to find what is really done above. If you wnat to fix an
alignment, please, do it in a separate prepatch without any real
changes.

>   } Vmxnet3_Cmd;
>   
>   /* Adaptive Ring Info Flags */
> @@ -571,6 +575,24 @@ enum vmxnet3_intr_type {
>   /* addition 1 for events */
>   #define VMXNET3_MAX_INTRS      25
>   
> +/* Version 6 and later will use below macros */
> +#define VMXNET3_EXT_MAX_TX_QUEUES  32
> +#define VMXNET3_EXT_MAX_RX_QUEUES  32
> +
> +/* Version-dependent MAX RX/TX queues macro */
> +#define MAX_RX_QUEUES \
> +	(VMXNET3_VERSION_GE_6(hw) ? \

I think it is beter to pass 'hw' argument explicintly.
Otherwise it looks inconsistent and confusing.

> +	VMXNET3_EXT_MAX_RX_QUEUES : \
> +	VMXNET3_MAX_RX_QUEUES)
> +#define MAX_TX_QUEUES \
> +	(VMXNET3_VERSION_GE_6(hw) ? \

same here

> +	VMXNET3_EXT_MAX_TX_QUEUES : \
> +	VMXNET3_MAX_TX_QUEUES)
> +


[snip]