On 7/16/2021 9:30 AM, Li, Xiaoyun wrote:

quoted

-----Original Message-----
From: stable <redacted> On Behalf Of Li, Xiaoyun
Sent: Thursday, July 15, 2021 12:54
To: Wang, Jie1X <redacted>; dev@dpdk.org
Cc: andrew.rybchenko@oktetlabs.ru; stable@dpdk.org
Subject: Re: [dpdk-stable] [PATCH v4] app/testpmd: fix testpmd doesn't show
RSS hash offload

quoted

-----Original Message-----
From: Wang, Jie1X <redacted>
Sent: Thursday, July 15, 2021 19:57
To: dev@dpdk.org
Cc: Li, Xiaoyun <redacted>; andrew.rybchenko@oktetlabs.ru;
Wang, Jie1X [off-list ref]; stable@dpdk.org
Subject: [PATCH v4] app/testpmd: fix testpmd doesn't show RSS hash
offload

The driver may change offloads info into dev->data->dev_conf in
dev_configure which may cause port->dev_conf and port->rx_conf contain

outdated values.

quoted

This patch updates the offloads info if it changes to fix this issue.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Cc: stable@dpdk.org

Signed-off-by: Jie Wang <redacted>
---
v4: delete the whitespace at the end of the line.
v3:
 - check and update the "offloads" of "port->dev_conf.rx/txmode".
 - update the commit log.
v2: copy "rx/txmode.offloads", instead of copying the entire struct
"dev->data-

quoted

dev_conf.rx/txmode".

---
 app/test-pmd/testpmd.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

Acked-by: Xiaoyun Li <redacted>

Although I gave my ack, app shouldn't touch rte_eth_devices which this patch does. Usually, testpmd should only call function like eth_dev_info_get_print_err().
But dev_info doesn't contain the info dev->data->dev_conf which the driver modifies.

Probably we need a better fix.

Agree, an application accessing directly to 'rte_eth_devices' is sign of
something missing/wrong.

In this case there is no way for application to know what is the configured
offload settings per port and queue. Which is missing part I think.

As you said normally we get data from PMD mainly via 'rte_eth_dev_info_get()',
which is an overloaded function, it provides many different things, like driver
default values, limitations, current config/status, capabilities etc...

So I think we can do a few things:
1) Add current offload configuration to 'rte_eth_dev_info_get()', so application
can get it and use it.
The advantage is this API already called many places, many times, so there is a
big chance that application already have this information when it needs.
Disadvantage is, as mentioned above the API already big and messy, making it
bigger makes more error prone and makes easier to break ABI.

2) Add a new API to get configured offload information, so a specific API for it.

3) Get a more generic API to get configured config (dev_conf) which will cover
offloads too.
Disadvantage can be leaking out too many internal config to user unintentionally.