Re: [PATCH v1 1/3] examples/l3fwd: relax RSS requirement with option

[Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>]

Hi Trevor,

> Now the port Rx mq_mode had been set to RTE_ETH_MQ_RX_RSS
> by default, but some hw and/or virtual interface does not
> support the RSS and offload mode presupposed, e.g., some
> virtio interfaces in the cloud don't support
> RSS and the error msg may like:
> 
> virtio_dev_configure(): RSS support requested but not supported by
> the device
> Port0 dev_configure = -95
> 
> So to enable the l3fwd running in that environment, the Rx mode requirement
> can be relaxed to reflect the hardware feature reality here, and the l3fwd
> can run smoothly then.
> 
> An option named "relax-rx-mode" is added to enable the relax action
> here, and it's disabled by default.
> 
> Signed-off-by: Trevor Tao <taozj888@163.com>
> ---
>   examples/l3fwd/main.c | 19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
> index 6063eb1399..2c8f528d00 100644
> --- a/examples/l3fwd/main.c
> +++ b/examples/l3fwd/main.c
> @@ -73,6 +73,7 @@ static enum L3FWD_LOOKUP_MODE lookup_mode;
>   static int numa_on = 1; /**< NUMA is enabled by default. */
>   static int parse_ptype; /**< Parse packet type using rx callback, and */
>   			/**< disabled by default */
> +static int relax_rx_mode; /**< Relax RX mode is disabled by default */
>   static int per_port_pool; /**< Use separate buffer pools per port; disabled */
>   			  /**< by default */
>   
> @@ -678,6 +679,7 @@ static const char short_options[] =
>   #define CMD_LINE_OPT_MAX_PKT_LEN "max-pkt-len"
>   #define CMD_LINE_OPT_HASH_ENTRY_NUM "hash-entry-num"
>   #define CMD_LINE_OPT_PARSE_PTYPE "parse-ptype"
> +#define CMD_LINE_OPT_RELAX_RX_MODE "relax-rx-mode"
>   #define CMD_LINE_OPT_PER_PORT_POOL "per-port-pool"
>   #define CMD_LINE_OPT_MODE "mode"
>   #define CMD_LINE_OPT_EVENTQ_SYNC "eventq-sched"
> @@ -705,6 +707,7 @@ enum {
>   	CMD_LINE_OPT_MAX_PKT_LEN_NUM,
>   	CMD_LINE_OPT_HASH_ENTRY_NUM_NUM,
>   	CMD_LINE_OPT_PARSE_PTYPE_NUM,
> +	CMD_LINE_OPT_RELAX_RX_MODE_NUM,
>   	CMD_LINE_OPT_RULE_IPV4_NUM,
>   	CMD_LINE_OPT_RULE_IPV6_NUM,
>   	CMD_LINE_OPT_ALG_NUM,
> @@ -728,6 +731,7 @@ static const struct option lgopts[] = {
>   	{CMD_LINE_OPT_MAX_PKT_LEN, 1, 0, CMD_LINE_OPT_MAX_PKT_LEN_NUM},
>   	{CMD_LINE_OPT_HASH_ENTRY_NUM, 1, 0, CMD_LINE_OPT_HASH_ENTRY_NUM_NUM},
>   	{CMD_LINE_OPT_PARSE_PTYPE, 0, 0, CMD_LINE_OPT_PARSE_PTYPE_NUM},
> +	{CMD_LINE_OPT_RELAX_RX_MODE, 0, 0, CMD_LINE_OPT_RELAX_RX_MODE_NUM},
>   	{CMD_LINE_OPT_PER_PORT_POOL, 0, 0, CMD_LINE_OPT_PARSE_PER_PORT_POOL},
>   	{CMD_LINE_OPT_MODE, 1, 0, CMD_LINE_OPT_MODE_NUM},
>   	{CMD_LINE_OPT_EVENTQ_SYNC, 1, 0, CMD_LINE_OPT_EVENTQ_SYNC_NUM},
> @@ -853,6 +857,11 @@ parse_args(int argc, char **argv)
>   			parse_ptype = 1;
>   			break;
>   
> +		case CMD_LINE_OPT_RELAX_RX_MODE_NUM:
> +			printf("Relax rx mode is enabled\n");
> +			relax_rx_mode = 1;
> +			break;
> +
>   		case CMD_LINE_OPT_PARSE_PER_PORT_POOL:
>   			printf("per port buffer pool is enabled\n");
>   			per_port_pool = 1;
> @@ -1257,8 +1266,14 @@ l3fwd_poll_resource_setup(void)
>   		local_port_conf.rx_adv_conf.rss_conf.rss_hf &=
>   			dev_info.flow_type_rss_offloads;
>   
> -		if (dev_info.max_rx_queues == 1)
> -			local_port_conf.rxmode.mq_mode = RTE_ETH_MQ_RX_NONE;
> +		/* relax the rx rss requirement */
> +		if (dev_info.max_rx_queues == 1 || !local_port_conf.rx_adv_conf.rss_conf.rss_hf) {
> +			if (relax_rx_mode) {
> +				printf("warning: modified the rx mq_mode to RTE_ETH_MQ_RX_NONE base on"
> +					" device capability\n");
> +				local_port_conf.rxmode.mq_mode = RTE_ETH_MQ_RX_NONE;
> +			}
> +		}

But that way we change current behavior - always use MQ_RX_NONE
for devices with just one RX queue.
Was it intended?
Might be it should be:
	if (dev_info.max_rx_queues == 1)
		local_port_conf.rxmode.mq_mode = RTE_ETH_MQ_RX_NONE;

+	/* relax the rx rss requirement */
+	if (relax_rx_mode &&
+ 			!local_port_conf.rx_adv_conf.rss_conf.rss_hf) {
+		printf("...");
+		local_port_conf.rxmode.mq_mode = RTE_ETH_MQ_RX_NONE;
+	}

?


>   
>   		if (local_port_conf.rx_adv_conf.rss_conf.rss_hf !=
>   				port_conf.rx_adv_conf.rss_conf.rss_hf) {