Hi Konstantin:
For your comments, please see my answers inline.
Thanks.
Trevor Tao
At 2023-10-09 16:43:01, "Konstantin Ananyev" <konstantin.v.anan...@yandex.ru>
wrote:
>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 <taozj...@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;
>+ }
>
>?
>OK, I will change it as what you gave here though I think there is no
>difference between the max_rx_queue is 1 and rss_hf is 0, both of them mean
>that RSS is not supported.
Thanks.
>
>
>>
>> if (local_port_conf.rx_adv_conf.rss_conf.rss_hf !=
>> port_conf.rx_adv_conf.rss_conf.rss_hf) {
