[dpdk-dev] [PATCH] app/testpmd: fix testpmd doesn't show RSS hash offload

[dpdk-dev] [PATCH] app/testpmd: fix testpmd doesn't show RSS hash offload

[Jie Wang]

This patch reapply Rx/Tx offloads configuration for all ports
after the program configuring the device port. When the program
configures the ports, the default Rx/Tx offloads are modified.

So it is need to reapply Rx/Tx offloads configuration before
testpmd showing offloads.

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

Signed-off-by: Jie Wang <jie1x.wang@intel.com>
---
 app/test-pmd/testpmd.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 1cdd3cdd12..7089ae216d 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2475,6 +2475,9 @@ start_port(portid_t pid)
 		}
 
 		if (port->need_reconfig > 0) {
+			const struct rte_eth_dev *dev = &rte_eth_devices[pi];
+			int k;
+
 			port->need_reconfig = 0;
 
 			if (flow_isolate_all) {
@@ -2508,6 +2511,18 @@ start_port(portid_t pid)
 				port->need_reconfig = 1;
 				return -1;
 			}
+
+			/* Apply TxRx configuration for all ports */
+			port->dev_conf.txmode = dev->data->dev_conf.txmode;
+			port->dev_conf.rxmode = dev->data->dev_conf.rxmode;
+			/* Apply Rx offloads configuration */
+			for (k = 0; k < port->dev_info.max_rx_queues; k++)
+				port->rx_conf[k].offloads =
+					port->dev_conf.rxmode.offloads;
+			/* Apply Tx offloads configuration */
+			for (k = 0; k < port->dev_info.max_tx_queues; k++)
+				port->tx_conf[k].offloads =
+					port->dev_conf.txmode.offloads;
 		}
 		if (port->need_reconfig_queues > 0) {
 			port->need_reconfig_queues = 0;
-- 
2.25.1

Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd doesn't show RSS hash offload

[Andrew Rybchenko]

On 7/9/21 6:57 PM, Jie Wang wrote:
> This patch reapply Rx/Tx offloads configuration for all ports
> after the program configuring the device port. When the program
> configures the ports, the default Rx/Tx offloads are modified.
> 
> So it is need to reapply Rx/Tx offloads configuration before
> testpmd showing offloads.
> 
> Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> ---
>  app/test-pmd/testpmd.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 1cdd3cdd12..7089ae216d 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -2475,6 +2475,9 @@ start_port(portid_t pid)
>  		}
>  
>  		if (port->need_reconfig > 0) {
> +			const struct rte_eth_dev *dev = &rte_eth_devices[pi];
> +			int k;
> +
>  			port->need_reconfig = 0;
>  
>  			if (flow_isolate_all) {
> @@ -2508,6 +2511,18 @@ start_port(portid_t pid)
>  				port->need_reconfig = 1;
>  				return -1;
>  			}
> +
> +			/* Apply TxRx configuration for all ports */
> +			port->dev_conf.txmode = dev->data->dev_conf.txmode;
> +			port->dev_conf.rxmode = dev->data->dev_conf.rxmode;
> +			/* Apply Rx offloads configuration */
> +			for (k = 0; k < port->dev_info.max_rx_queues; k++)
> +				port->rx_conf[k].offloads =
> +					port->dev_conf.rxmode.offloads;
> +			/* Apply Tx offloads configuration */
> +			for (k = 0; k < port->dev_info.max_tx_queues; k++)
> +				port->tx_conf[k].offloads =
> +					port->dev_conf.txmode.offloads;

Does testpmd really require these copies? May be the right fix is to get
rid of these copies at all and show actual
information from data->dev_conf ?

Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd doesn't show RSS hash offload

[Li, Xiaoyun]

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Friday, July 9, 2021 17:27
> To: Wang, Jie1X <jie1x.wang@intel.com>; dev@dpdk.org
> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd doesn't show RSS
> hash offload
> 
> On 7/9/21 6:57 PM, Jie Wang wrote:
> > This patch reapply Rx/Tx offloads configuration for all ports after
> > the program configuring the device port. When the program configures
> > the ports, the default Rx/Tx offloads are modified.
> >
> > So it is need to reapply Rx/Tx offloads configuration before testpmd
> > showing offloads.
> >
> > Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> > ---
> >  app/test-pmd/testpmd.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > 1cdd3cdd12..7089ae216d 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -2475,6 +2475,9 @@ start_port(portid_t pid)
> >  		}
> >
> >  		if (port->need_reconfig > 0) {
> > +			const struct rte_eth_dev *dev = &rte_eth_devices[pi];
> > +			int k;
> > +
> >  			port->need_reconfig = 0;
> >
> >  			if (flow_isolate_all) {
> > @@ -2508,6 +2511,18 @@ start_port(portid_t pid)
> >  				port->need_reconfig = 1;
> >  				return -1;
> >  			}
> > +
> > +			/* Apply TxRx configuration for all ports */
> > +			port->dev_conf.txmode = dev->data-
> >dev_conf.txmode;
> > +			port->dev_conf.rxmode = dev->data-
> >dev_conf.rxmode;
> > +			/* Apply Rx offloads configuration */
> > +			for (k = 0; k < port->dev_info.max_rx_queues; k++)
> > +				port->rx_conf[k].offloads =
> > +					port->dev_conf.rxmode.offloads;
> > +			/* Apply Tx offloads configuration */
> > +			for (k = 0; k < port->dev_info.max_tx_queues; k++)
> > +				port->tx_conf[k].offloads =
> > +					port->dev_conf.txmode.offloads;
> 
> Does testpmd really require these copies? May be the right fix is to get rid of
> these copies at all and show actual information from data->dev_conf ?
> 
The "show" command cmd_rx_offload_get_configuration_parsed() actually should only touch port->dev_conf.
And It doesn't only show dev_conf, but also per queue rx_conf. It's impossible to show per queue rx conf info from data->dev_conf.

The copy is already done in init_config actually. But by then, dev->data->dev_conf doesn't include RSS_HASH.
RSS_HASH is added by driver in dev_configure. That's why this patch wants to copy again after dev_configure I guess.

But this patch is INCORRECT anyway.
Because any cmd that changes port offload will change port->dev_conf and port->rx/tx_conf NOT dev->data->dev_conf.
So after this patch, if a user " port config 0 rx_offload xxx on", all of the new config will disappear.

Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd doesn't show RSS hash offload

[Li, Xiaoyun]

> -----Original Message-----
> From: Wang, Jie1X <jie1x.wang@intel.com>
> Sent: Tuesday, July 13, 2021 10:34
> To: Li, Xiaoyun <xiaoyun.li@intel.com>; andrew.rybchenko@oktetlabs.ru;
> dev@dpdk.org
> Cc: stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd doesn't show RSS
> hash offload
> 
> Hi all,
> I have tested this patch, it still worked on.
> 
> When we changed the port offloads, the port->dev_conf would change in the
> same time. And in the function " rte_eth_dev_configure ", dev->data->dev_conf
> copied the port->dev_conf parameter into the dev structure.

OK. A reminder: Please answer these under every part you want to explain not the very start. It looks messy in mail-list.
But still, this patch has issues.
Please see inlines.
> 
> -----Original Message-----
> From: Li, Xiaoyun <xiaoyun.li@intel.com>
> Sent: Monday, July 12, 2021 11:12 AM
> To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; Wang, Jie1X
> <jie1x.wang@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix testpmd doesn't show RSS
> hash offload
> 
> 
> 
> > -----Original Message-----
> > From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> > Sent: Friday, July 9, 2021 17:27
> > To: Wang, Jie1X <jie1x.wang@intel.com>; dev@dpdk.org
> > Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; stable@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd doesn't show
> > RSS hash offload
> >
> > On 7/9/21 6:57 PM, Jie Wang wrote:
> > > This patch reapply Rx/Tx offloads configuration for all ports after
> > > the program configuring the device port. When the program configures
> > > the ports, the default Rx/Tx offloads are modified.
> > >
> > > So it is need to reapply Rx/Tx offloads configuration before testpmd
> > > showing offloads.
> > >
> > > Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> > > ---
> > >  app/test-pmd/testpmd.c | 15 +++++++++++++++
> > >  1 file changed, 15 insertions(+)
> > >
> > > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > > 1cdd3cdd12..7089ae216d 100644
> > > --- a/app/test-pmd/testpmd.c
> > > +++ b/app/test-pmd/testpmd.c
> > > @@ -2475,6 +2475,9 @@ start_port(portid_t pid)
> > >  		}
> > >
> > >  		if (port->need_reconfig > 0) {
> > > +			const struct rte_eth_dev *dev = &rte_eth_devices[pi];
> > > +			int k;
> > > +
> > >  			port->need_reconfig = 0;
> > >
> > >  			if (flow_isolate_all) {
> > > @@ -2508,6 +2511,18 @@ start_port(portid_t pid)
> > >  				port->need_reconfig = 1;
> > >  				return -1;
> > >  			}
> > > +
> > > +			/* Apply TxRx configuration for all ports */
> > > +			port->dev_conf.txmode = dev->data-
> > >dev_conf.txmode;
> > > +			port->dev_conf.rxmode = dev->data-
> > >dev_conf.rxmode;

Direct assignment for a big struct is dangerous. Next time, if you're going to assign a struct, please use memcpy.
And actually, your issue will only happen for one time for rss_hash because it seems only rss_hash will be added in driver.
Doing these copies every time seems very redundant.
You'd better check if port->rxmode.offloads==dev->data->dev_conf.rx_mode.offloads, if it's not equal, do the new assignment, including queue offload. Otherwise, don't.
Same with txmode.offloads.

> > > +			/* Apply Rx offloads configuration */
> > > +			for (k = 0; k < port->dev_info.max_rx_queues; k++)
> > > +				port->rx_conf[k].offloads =
> > > +					port->dev_conf.rxmode.offloads;
> > > +			/* Apply Tx offloads configuration */
> > > +			for (k = 0; k < port->dev_info.max_tx_queues; k++)
> > > +				port->tx_conf[k].offloads =
> > > +					port->dev_conf.txmode.offloads;
> >
> > Does testpmd really require these copies? May be the right fix is to
> > get rid of these copies at all and show actual information from data-
> >dev_conf ?
> >
> The "show" command cmd_rx_offload_get_configuration_parsed() actually
> should only touch port->dev_conf.
> And It doesn't only show dev_conf, but also per queue rx_conf. It's impossible to
> show per queue rx conf info from data->dev_conf.
> 
> The copy is already done in init_config actually. But by then, dev->data-
> >dev_conf doesn't include RSS_HASH.
> RSS_HASH is added by driver in dev_configure. That's why this patch wants to
> copy again after dev_configure I guess.
> 
> But this patch is INCORRECT anyway.
> Because any cmd that changes port offload will change port->dev_conf and
> port->rx/tx_conf NOT dev->data->dev_conf.
> So after this patch, if a user " port config 0 rx_offload xxx on", all of the new
> config will disappear.

Re: [dpdk-dev] [dpdk-stable] [PATCH] app/testpmd: fix testpmd doesn't show RSS hash offload

[Ferruh Yigit]

On 7/12/2021 4:12 AM, Li, Xiaoyun wrote:
> 
> 
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Sent: Friday, July 9, 2021 17:27
>> To: Wang, Jie1X <jie1x.wang@intel.com>; dev@dpdk.org
>> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; stable@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix testpmd doesn't show RSS
>> hash offload
>>
>> On 7/9/21 6:57 PM, Jie Wang wrote:
>>> This patch reapply Rx/Tx offloads configuration for all ports after
>>> the program configuring the device port. When the program configures
>>> the ports, the default Rx/Tx offloads are modified.
>>>
>>> So it is need to reapply Rx/Tx offloads configuration before testpmd
>>> showing offloads.
>>>
>>> Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Jie Wang <jie1x.wang@intel.com>
>>> ---
>>>  app/test-pmd/testpmd.c | 15 +++++++++++++++
>>>  1 file changed, 15 insertions(+)
>>>
>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
>>> 1cdd3cdd12..7089ae216d 100644
>>> --- a/app/test-pmd/testpmd.c
>>> +++ b/app/test-pmd/testpmd.c
>>> @@ -2475,6 +2475,9 @@ start_port(portid_t pid)
>>>  		}
>>>
>>>  		if (port->need_reconfig > 0) {
>>> +			const struct rte_eth_dev *dev = &rte_eth_devices[pi];
>>> +			int k;
>>> +
>>>  			port->need_reconfig = 0;
>>>
>>>  			if (flow_isolate_all) {
>>> @@ -2508,6 +2511,18 @@ start_port(portid_t pid)
>>>  				port->need_reconfig = 1;
>>>  				return -1;
>>>  			}
>>> +
>>> +			/* Apply TxRx configuration for all ports */
>>> +			port->dev_conf.txmode = dev->data-
>>> dev_conf.txmode;
>>> +			port->dev_conf.rxmode = dev->data-
>>> dev_conf.rxmode;
>>> +			/* Apply Rx offloads configuration */
>>> +			for (k = 0; k < port->dev_info.max_rx_queues; k++)
>>> +				port->rx_conf[k].offloads =
>>> +					port->dev_conf.rxmode.offloads;
>>> +			/* Apply Tx offloads configuration */
>>> +			for (k = 0; k < port->dev_info.max_tx_queues; k++)
>>> +				port->tx_conf[k].offloads =
>>> +					port->dev_conf.txmode.offloads;
>>
>> Does testpmd really require these copies? May be the right fix is to get rid of
>> these copies at all and show actual information from data->dev_conf ?
>>
> The "show" command cmd_rx_offload_get_configuration_parsed() actually should only touch port->dev_conf.
> And It doesn't only show dev_conf, but also per queue rx_conf. It's impossible to show per queue rx conf info from data->dev_conf.
> 

Testpmd having these copies also bothers me. This seems because ethdev doesn't
provide a way to get current config, so application has to keep the copy, but
this is causing complexities and diversions as this case.

So we can try to address this in a larger scale and try to eliminate the need of
application config copies.

Also RSS_HASH offload flag being set by PMDs unconventionally (instead of
application) making this issue visible, but I think there isn't much we can do
about RSS_HASH.


> The copy is already done in init_config actually. But by then, dev->data->dev_conf doesn't include RSS_HASH.
> RSS_HASH is added by driver in dev_configure. That's why this patch wants to copy again after dev_configure I guess.
> 
> But this patch is INCORRECT anyway.
> Because any cmd that changes port offload will change port->dev_conf and port->rx/tx_conf NOT dev->data->dev_conf.
> So after this patch, if a user " port config 0 rx_offload xxx on", all of the new config will disappear.
> 

[dpdk-dev] [PATCH v2] app/testpmd: fix testpmd doesn't show RSS hash offload

[Jie Wang]

This patch reapply Rx/Tx offloads configuration for all ports
after the program configuring the device port. When the program
configures the ports, the default Rx/Tx offloads are modified in
dev->data->dev_conf.

So it is need to reapply Rx/Tx offloads configuration after
dev_configure.

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

Signed-off-by: Jie Wang <jie1x.wang@intel.com>
---
v2: copy "rx/txmode.offloads", instead of copying the entire struct 
"dev->data->dev_conf.rx/txmode".
---
 app/test-pmd/testpmd.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 1cdd3cdd12..dc9ce5330b 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2475,6 +2475,11 @@ start_port(portid_t pid)
 		}
 
 		if (port->need_reconfig > 0) {
+			const struct rte_eth_dev *dev = &rte_eth_devices[pi];
+			struct rte_eth_conf *dev_conf;
+			int k;
+
+			dev_conf = &dev->data->dev_conf;
 			port->need_reconfig = 0;
 
 			if (flow_isolate_all) {
@@ -2508,6 +2513,20 @@ start_port(portid_t pid)
 				port->need_reconfig = 1;
 				return -1;
 			}
+			/* Apply Rx offloads configuration */
+			for (k = 0; k < port->dev_info.max_rx_queues; k++) {
+				if (port->rx_conf[k].offloads !=
+					dev_conf->rxmode.offloads)
+					port->rx_conf[k].offloads =
+						dev_conf->rxmode.offloads;
+			}
+			/* Apply Tx offloads configuration */
+			for (k = 0; k < port->dev_info.max_tx_queues; k++) {
+				if (port->tx_conf[k].offloads !=
+					dev_conf->txmode.offloads)
+					port->tx_conf[k].offloads =
+						dev_conf->txmode.offloads;
+			}
 		}
 		if (port->need_reconfig_queues > 0) {
 			port->need_reconfig_queues = 0;
-- 
2.25.1

Re: [dpdk-dev] [PATCH v2] app/testpmd: fix testpmd doesn't show RSS hash offload

[Li, Xiaoyun]

Hi

> -----Original Message-----
> From: Wang, Jie1X <jie1x.wang@intel.com>
> Sent: Wednesday, July 14, 2021 01:04
> To: dev@dpdk.org
> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; andrew.rybchenko@oktetlabs.ru; Wang,
> Jie1X <jie1x.wang@intel.com>; stable@dpdk.org
> Subject: [PATCH v2] app/testpmd: fix testpmd doesn't show RSS hash offload
> 
> This patch reapply Rx/Tx offloads configuration for all ports after the program
> configuring the device port. When the program configures the ports, the default
> Rx/Tx offloads are modified in
> dev->data->dev_conf.
> 
> So it is need to reapply Rx/Tx offloads configuration after dev_configure.
> 
> Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> ---
> v2: copy "rx/txmode.offloads", instead of copying the entire struct "dev->data-
> >dev_conf.rx/txmode".
> ---
>  app/test-pmd/testpmd.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> 1cdd3cdd12..dc9ce5330b 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -2475,6 +2475,11 @@ start_port(portid_t pid)
>  		}
> 
>  		if (port->need_reconfig > 0) {
> +			const struct rte_eth_dev *dev = &rte_eth_devices[pi];
> +			struct rte_eth_conf *dev_conf;
> +			int k;
> +
> +			dev_conf = &dev->data->dev_conf;
>  			port->need_reconfig = 0;
> 
>  			if (flow_isolate_all) {
> @@ -2508,6 +2513,20 @@ start_port(portid_t pid)
>  				port->need_reconfig = 1;
>  				return -1;
>  			}
> +			/* Apply Rx offloads configuration */
> +			for (k = 0; k < port->dev_info.max_rx_queues; k++) {
> +				if (port->rx_conf[k].offloads !=
> +					dev_conf->rxmode.offloads)
> +					port->rx_conf[k].offloads =
> +						dev_conf->rxmode.offloads;
> +			}

I was suggesting something like:
	if (dev_conf->rxmode.offloads != port->dev_conf->rxmode.offloads) {
		port->dev_conf->rxmode.offloads = dev_conf->rxmode.offloads;
		for (queues)
			update offloads for each queue of this port
	}

Not only check and change queue offload.

> +			/* Apply Tx offloads configuration */
> +			for (k = 0; k < port->dev_info.max_tx_queues; k++) {
> +				if (port->tx_conf[k].offloads !=
> +					dev_conf->txmode.offloads)
> +					port->tx_conf[k].offloads =
> +						dev_conf->txmode.offloads;
> +			}
>  		}
>  		if (port->need_reconfig_queues > 0) {
>  			port->need_reconfig_queues = 0;
> --
> 2.25.1

Re: [dpdk-dev] [PATCH v2] app/testpmd: fix testpmd doesn't show RSS hash offload

[Li, Xiaoyun]

Hi

> -----Original Message-----
> From: stable <stable-bounces@dpdk.org> On Behalf Of Li, Xiaoyun
> Sent: Thursday, July 15, 2021 10:30
> To: Wang, Jie1X <jie1x.wang@intel.com>; dev@dpdk.org
> Cc: andrew.rybchenko@oktetlabs.ru; stable@dpdk.org
> Subject: Re: [dpdk-stable] [PATCH v2] app/testpmd: fix testpmd doesn't show
> RSS hash offload
> 
> Hi
> 
> > -----Original Message-----
> > From: Wang, Jie1X <jie1x.wang@intel.com>
> > Sent: Wednesday, July 14, 2021 01:04
> > To: dev@dpdk.org
> > Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; andrew.rybchenko@oktetlabs.ru;
> > Wang, Jie1X <jie1x.wang@intel.com>; stable@dpdk.org
> > Subject: [PATCH v2] app/testpmd: fix testpmd doesn't show RSS hash
> > offload
> >
> > This patch reapply Rx/Tx offloads configuration for all ports after
> > the program configuring the device port. When the program configures
> > the ports, the default Rx/Tx offloads are modified in
> > dev->data->dev_conf.
> >
> > So it is need to reapply Rx/Tx offloads configuration after dev_configure.

Forgot to mention. The commit log has many grammar mistakes especially time tense.
You can use the following:
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.
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 <jie1x.wang@intel.com>
> > ---
> > v2: copy "rx/txmode.offloads", instead of copying the entire struct
> > "dev->data-
> > >dev_conf.rx/txmode".
> > ---
> >  app/test-pmd/testpmd.c | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > 1cdd3cdd12..dc9ce5330b 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -2475,6 +2475,11 @@ start_port(portid_t pid)
> >  		}
> >
> >  		if (port->need_reconfig > 0) {
> > +			const struct rte_eth_dev *dev = &rte_eth_devices[pi];
> > +			struct rte_eth_conf *dev_conf;
> > +			int k;
> > +
> > +			dev_conf = &dev->data->dev_conf;
> >  			port->need_reconfig = 0;
> >
> >  			if (flow_isolate_all) {
> > @@ -2508,6 +2513,20 @@ start_port(portid_t pid)
> >  				port->need_reconfig = 1;
> >  				return -1;
> >  			}
> > +			/* Apply Rx offloads configuration */
> > +			for (k = 0; k < port->dev_info.max_rx_queues; k++) {
> > +				if (port->rx_conf[k].offloads !=
> > +					dev_conf->rxmode.offloads)
> > +					port->rx_conf[k].offloads =
> > +						dev_conf->rxmode.offloads;
> > +			}
> 
> I was suggesting something like:
> 	if (dev_conf->rxmode.offloads != port->dev_conf->rxmode.offloads) {
> 		port->dev_conf->rxmode.offloads = dev_conf-
> >rxmode.offloads;
> 		for (queues)
> 			update offloads for each queue of this port
> 	}
> 
> Not only check and change queue offload.
> 
> > +			/* Apply Tx offloads configuration */
> > +			for (k = 0; k < port->dev_info.max_tx_queues; k++) {
> > +				if (port->tx_conf[k].offloads !=
> > +					dev_conf->txmode.offloads)
> > +					port->tx_conf[k].offloads =
> > +						dev_conf->txmode.offloads;
> > +			}
> >  		}
> >  		if (port->need_reconfig_queues > 0) {
> >  			port->need_reconfig_queues = 0;
> > --
> > 2.25.1

[dpdk-dev] [PATCH v3] app/testpmd: fix testpmd doesn't show RSS hash offload

[Jie Wang]

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.

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 <jie1x.wang@intel.com>
---
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->dev_conf.rx/txmode". 
---
 app/test-pmd/testpmd.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 1cdd3cdd12..26550340e4 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2475,6 +2475,11 @@ start_port(portid_t pid)
 		}
 
 		if (port->need_reconfig > 0) {
+			const struct rte_eth_dev *dev = &rte_eth_devices[pi];
+			struct rte_eth_conf *dev_conf;
+			int k;
+
+			dev_conf = &dev->data->dev_conf;
 			port->need_reconfig = 0;
 
 			if (flow_isolate_all) {
@@ -2508,6 +2513,28 @@ start_port(portid_t pid)
 				port->need_reconfig = 1;
 				return -1;
 			}
+			/* Apply Rx offloads configuration */
+			if (dev_conf->rxmode.offloads !=
+				port->dev_conf.rxmode.offloads) {
+				port->dev_conf.rxmode.offloads =
+					dev_conf->rxmode.offloads;
+				for (k = 0;
+				     k < port->dev_info.max_rx_queues;
+				     k++)
+					port->rx_conf[k].offloads =
+						dev_conf->rxmode.offloads;
+			}
+			/* Apply Tx offloads configuration */
+			if (dev_conf->txmode.offloads !=
+				port->dev_conf.txmode.offloads) {
+				port->dev_conf.txmode.offloads =
+					dev_conf->txmode.offloads;
+				for (k = 0;
+				     k < port->dev_info.max_tx_queues; 
+				     k++)
+					port->tx_conf[k].offloads =
+						dev_conf->txmode.offloads;
+			}
 		}
 		if (port->need_reconfig_queues > 0) {
 			port->need_reconfig_queues = 0;
-- 
2.25.1

[dpdk-dev] [PATCH v4] app/testpmd: fix testpmd doesn't show RSS hash offload

[Jie Wang]

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.

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 <jie1x.wang@intel.com>
---
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->dev_conf.rx/txmode". 
---
 app/test-pmd/testpmd.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 1cdd3cdd12..b6e8d7cdd9 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2475,6 +2475,11 @@ start_port(portid_t pid)
 		}
 
 		if (port->need_reconfig > 0) {
+			const struct rte_eth_dev *dev = &rte_eth_devices[pi];
+			struct rte_eth_conf *dev_conf;
+			int k;
+
+			dev_conf = &dev->data->dev_conf;
 			port->need_reconfig = 0;
 
 			if (flow_isolate_all) {
@@ -2508,6 +2513,28 @@ start_port(portid_t pid)
 				port->need_reconfig = 1;
 				return -1;
 			}
+			/* Apply Rx offloads configuration */
+			if (dev_conf->rxmode.offloads !=
+				port->dev_conf.rxmode.offloads) {
+				port->dev_conf.rxmode.offloads =
+					dev_conf->rxmode.offloads;
+				for (k = 0;
+				     k < port->dev_info.max_rx_queues;
+				     k++)
+					port->rx_conf[k].offloads =
+						dev_conf->rxmode.offloads;
+			}
+			/* Apply Tx offloads configuration */
+			if (dev_conf->txmode.offloads !=
+				port->dev_conf.txmode.offloads) {
+				port->dev_conf.txmode.offloads =
+					dev_conf->txmode.offloads;
+				for (k = 0;
+				     k < port->dev_info.max_tx_queues;
+				     k++)
+					port->tx_conf[k].offloads =
+						dev_conf->txmode.offloads;
+			}
 		}
 		if (port->need_reconfig_queues > 0) {
 			port->need_reconfig_queues = 0;
-- 
2.25.1

Re: [dpdk-dev] [PATCH v4] app/testpmd: fix testpmd doesn't show RSS hash offload

[Li, Xiaoyun]

> -----Original Message-----
> From: Wang, Jie1X <jie1x.wang@intel.com>
> Sent: Thursday, July 15, 2021 19:57
> To: dev@dpdk.org
> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; andrew.rybchenko@oktetlabs.ru; Wang,
> Jie1X <jie1x.wang@intel.com>; 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.
> 
> 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 <jie1x.wang@intel.com>
> ---
> 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-
> >dev_conf.rx/txmode".
> ---
>  app/test-pmd/testpmd.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)

Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>

Re: [dpdk-dev] [PATCH v4] app/testpmd: fix testpmd doesn't show RSS hash offload

[Li, Xiaoyun]

> -----Original Message-----
> From: stable <stable-bounces@dpdk.org> On Behalf Of Li, Xiaoyun
> Sent: Thursday, July 15, 2021 12:54
> To: Wang, Jie1X <jie1x.wang@intel.com>; 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
> 
> > -----Original Message-----
> > From: Wang, Jie1X <jie1x.wang@intel.com>
> > Sent: Thursday, July 15, 2021 19:57
> > To: dev@dpdk.org
> > Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; andrew.rybchenko@oktetlabs.ru;
> > Wang, Jie1X <jie1x.wang@intel.com>; 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.
> >
> > 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 <jie1x.wang@intel.com>
> > ---
> > 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-
> > >dev_conf.rx/txmode".
> > ---
> >  app/test-pmd/testpmd.c | 27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> 
> Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>

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.

Re: [dpdk-dev] [dpdk-stable] [PATCH v4] app/testpmd: fix testpmd doesn't show RSS hash offload

[Ferruh Yigit]

On 7/16/2021 9:30 AM, Li, Xiaoyun wrote:
>> -----Original Message-----
>> From: stable <stable-bounces@dpdk.org> On Behalf Of Li, Xiaoyun
>> Sent: Thursday, July 15, 2021 12:54
>> To: Wang, Jie1X <jie1x.wang@intel.com>; 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
>>
>>> -----Original Message-----
>>> From: Wang, Jie1X <jie1x.wang@intel.com>
>>> Sent: Thursday, July 15, 2021 19:57
>>> To: dev@dpdk.org
>>> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; andrew.rybchenko@oktetlabs.ru;
>>> Wang, Jie1X <jie1x.wang@intel.com>; 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.
>>>
>>> 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 <jie1x.wang@intel.com>
>>> ---
>>> 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-
>>>> dev_conf.rx/txmode".
>>> ---
>>>  app/test-pmd/testpmd.c | 27 +++++++++++++++++++++++++++
>>>  1 file changed, 27 insertions(+)
>>
>> Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>
> 
> 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.

Re: [dpdk-dev] [dpdk-stable] [PATCH v4] app/testpmd: fix testpmd doesn't show RSS hash offload

[Ferruh Yigit]

On 7/19/2021 10:55 AM, Wang, Jie1X wrote:
> 
> 
>> -----Original Message-----
>> From: Yigit, Ferruh <ferruh.yigit@intel.com>
>> Sent: Friday, July 16, 2021 4:52 PM
>> To: Li, Xiaoyun <xiaoyun.li@intel.com>; Wang, Jie1X <jie1x.wang@intel.com>;
>> 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
>>
>> On 7/16/2021 9:30 AM, Li, Xiaoyun wrote:
>>>> -----Original Message-----
>>>> From: stable <stable-bounces@dpdk.org> On Behalf Of Li, Xiaoyun
>>>> Sent: Thursday, July 15, 2021 12:54
>>>> To: Wang, Jie1X <jie1x.wang@intel.com>; 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
>>>>
>>>>> -----Original Message-----
>>>>> From: Wang, Jie1X <jie1x.wang@intel.com>
>>>>> Sent: Thursday, July 15, 2021 19:57
>>>>> To: dev@dpdk.org
>>>>> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>;
>>>>> andrew.rybchenko@oktetlabs.ru; Wang, Jie1X <jie1x.wang@intel.com>;
>>>>> 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.
>>>>>
>>>>> 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 <jie1x.wang@intel.com>
>>>>> ---
>>>>> 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-
>>>>>> dev_conf.rx/txmode".
>>>>> ---
>>>>>  app/test-pmd/testpmd.c | 27 +++++++++++++++++++++++++++
>>>>>  1 file changed, 27 insertions(+)
>>>>
>>>> Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>
>>>
>>> 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.
>>
> I prefer to choose the 1st suggestion. 
> 
> Normally PMD gets data via 'rte_eth_dev_info_get()'. When we add offloads configuration 
> to it, we can get offloads as same as getting other info.
> 

Most probably it is easier to implement 1), I see your point but as said before
I think 'rte_eth_dev_info_get()' is already messy and I am worried to make it
even bigger.

I prefer option 2).

@Thomas, @Andrew, what do you think?


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

Re: [dpdk-dev] [dpdk-stable] [PATCH v4] app/testpmd: fix testpmd doesn't show RSS hash offload

[Andrew Rybchenko]

On 7/19/21 7:18 PM, Ferruh Yigit wrote:
> On 7/19/2021 10:55 AM, Wang, Jie1X wrote:
>>
>>
>>> -----Original Message-----
>>> From: Yigit, Ferruh <ferruh.yigit@intel.com>
>>> Sent: Friday, July 16, 2021 4:52 PM
>>> To: Li, Xiaoyun <xiaoyun.li@intel.com>; Wang, Jie1X <jie1x.wang@intel.com>;
>>> 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
>>>
>>> On 7/16/2021 9:30 AM, Li, Xiaoyun wrote:
>>>>> -----Original Message-----
>>>>> From: stable <stable-bounces@dpdk.org> On Behalf Of Li, Xiaoyun
>>>>> Sent: Thursday, July 15, 2021 12:54
>>>>> To: Wang, Jie1X <jie1x.wang@intel.com>; 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
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Wang, Jie1X <jie1x.wang@intel.com>
>>>>>> Sent: Thursday, July 15, 2021 19:57
>>>>>> To: dev@dpdk.org
>>>>>> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>;
>>>>>> andrew.rybchenko@oktetlabs.ru; Wang, Jie1X <jie1x.wang@intel.com>;
>>>>>> 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.
>>>>>>
>>>>>> 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 <jie1x.wang@intel.com>
>>>>>> ---
>>>>>> 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-
>>>>>>> dev_conf.rx/txmode".
>>>>>> ---
>>>>>>   app/test-pmd/testpmd.c | 27 +++++++++++++++++++++++++++
>>>>>>   1 file changed, 27 insertions(+)
>>>>>
>>>>> Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>
>>>>
>>>> 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.
>>>
>> I prefer to choose the 1st suggestion.
>>
>> Normally PMD gets data via 'rte_eth_dev_info_get()'. When we add offloads configuration
>> to it, we can get offloads as same as getting other info.
>>
> 
> Most probably it is easier to implement 1), I see your point but as said before
> I think 'rte_eth_dev_info_get()' is already messy and I am worried to make it
> even bigger.

IMHO, (1) is not an option.

> I prefer option 2).

I'm not sure that API function for each config parameter is an option as
well. We should find a balance. May be I'd add something like
rte_eth_dev_get_conf(uint16_t port_id, const struct rte_eth_conf **conf)
which returns a pointer to up-to-date configuration. I.e. option (3).

The tricky part here is to ensure that all specific API which modifies
various bits of the configuration updates dev_conf.

> 
> @Thomas, @Andrew, what do you think?
> 
> 
>>> 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.

I don't understand it. dev_conf is provided by user on
rte_eth_dev_configure().

Re: [dpdk-dev] [dpdk-stable] [PATCH v4] app/testpmd: fix testpmd doesn't show RSS hash offload

[Ferruh Yigit]

On 7/22/2021 12:03 PM, Andrew Rybchenko wrote:
> On 7/19/21 7:18 PM, Ferruh Yigit wrote:
>> On 7/19/2021 10:55 AM, Wang, Jie1X wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Yigit, Ferruh <ferruh.yigit@intel.com>
>>>> Sent: Friday, July 16, 2021 4:52 PM
>>>> To: Li, Xiaoyun <xiaoyun.li@intel.com>; Wang, Jie1X <jie1x.wang@intel.com>;
>>>> 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
>>>>
>>>> On 7/16/2021 9:30 AM, Li, Xiaoyun wrote:
>>>>>> -----Original Message-----
>>>>>> From: stable <stable-bounces@dpdk.org> On Behalf Of Li, Xiaoyun
>>>>>> Sent: Thursday, July 15, 2021 12:54
>>>>>> To: Wang, Jie1X <jie1x.wang@intel.com>; 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
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Wang, Jie1X <jie1x.wang@intel.com>
>>>>>>> Sent: Thursday, July 15, 2021 19:57
>>>>>>> To: dev@dpdk.org
>>>>>>> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>;
>>>>>>> andrew.rybchenko@oktetlabs.ru; Wang, Jie1X <jie1x.wang@intel.com>;
>>>>>>> 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.
>>>>>>>
>>>>>>> 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 <jie1x.wang@intel.com>
>>>>>>> ---
>>>>>>> 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-
>>>>>>>> dev_conf.rx/txmode".
>>>>>>> ---
>>>>>>>   app/test-pmd/testpmd.c | 27 +++++++++++++++++++++++++++
>>>>>>>   1 file changed, 27 insertions(+)
>>>>>>
>>>>>> Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>
>>>>>
>>>>> 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.
>>>>
>>> I prefer to choose the 1st suggestion.
>>>
>>> Normally PMD gets data via 'rte_eth_dev_info_get()'. When we add offloads
>>> configuration
>>> to it, we can get offloads as same as getting other info.
>>>
>>
>> Most probably it is easier to implement 1), I see your point but as said before
>> I think 'rte_eth_dev_info_get()' is already messy and I am worried to make it
>> even bigger.
> 
> IMHO, (1) is not an option.
> 
>> I prefer option 2).
> 
> I'm not sure that API function for each config parameter is an option as
> well. We should find a balance. May be I'd add something like
> rte_eth_dev_get_conf(uint16_t port_id, const struct rte_eth_conf **conf)
> which returns a pointer to up-to-date configuration. I.e. option (3).
> 

That is option 3, that can work too.

> The tricky part here is to ensure that all specific API which modifies
> various bits of the configuration updates dev_conf.
> 

They have to, aren't they? Otherwise there is no where to record the current
config for PMD too.

>>
>> @Thomas, @Andrew, what do you think?
>>
>>
>>>> 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.
> 
> I don't understand it. dev_conf is provided by user on
> rte_eth_dev_configure().

Yes but application doesn't provide all config, my concern was if some internal
config should be hidden from applications (possibly via some APIs).

Overall I am OK to go with option 3, I think it can simplify the applications
life. And later we can have some more updates on testpmd to benefit from new API.

[dpdk-dev] [PATCH v5] app/testpmd: fix testpmd doesn't show RSS hash offload

[Jie Wang]

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.

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

It adds a new API "rte_eth_dev_conf_info_get()" to help testpmd get
device configuration info.

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

Signed-off-by: Jie Wang <jie1x.wang@intel.com>
---
 app/test-pmd/testpmd.c  | 33 +++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h  |  2 ++
 app/test-pmd/util.c     | 15 +++++++++++++++
 lib/ethdev/rte_ethdev.c | 27 +++++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.h | 26 ++++++++++++++++++++++++++
 5 files changed, 103 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 6cbe9ba3c8..9a78805673 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2461,6 +2461,9 @@ start_port(portid_t pid)
 		}
 
 		if (port->need_reconfig > 0) {
+			struct rte_eth_dev_conf_info dev_conf_info;
+			int k;
+
 			port->need_reconfig = 0;
 
 			if (flow_isolate_all) {
@@ -2498,6 +2501,36 @@ start_port(portid_t pid)
 				port->need_reconfig = 1;
 				return -1;
 			}
+			/* get offloads */
+			if (0 !=
+				eth_dev_conf_info_get_print_err(pi,
+							&dev_conf_info)) {
+				rte_exit(EXIT_FAILURE,
+				    "rte_eth_dev_conf_info_get() failed\n");
+				return -1;
+			}
+			/* Apply Rx offloads configuration */
+			if (dev_conf_info.rx_offloads !=
+				port->dev_conf.rxmode.offloads) {
+				port->dev_conf.rxmode.offloads =
+					dev_conf_info.rx_offloads;
+				for (k = 0;
+				     k < port->dev_info.max_rx_queues;
+				     k++)
+					port->rx_conf[k].offloads =
+						dev_conf_info.rx_offloads;
+			}
+			/* Apply Tx offloads configuration */
+			if (dev_conf_info.tx_offloads !=
+				port->dev_conf.txmode.offloads) {
+				port->dev_conf.txmode.offloads =
+					dev_conf_info.tx_offloads;
+				for (k = 0;
+				     k < port->dev_info.max_tx_queues;
+				     k++)
+					port->tx_conf[k].offloads =
+						dev_conf_info.tx_offloads;
+			}
 		}
 		if (port->need_reconfig_queues > 0) {
 			port->need_reconfig_queues = 0;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 16a3598e48..4cf69a17a3 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -950,6 +950,8 @@ void show_gro(portid_t port_id);
 void setup_gso(const char *mode, portid_t port_id);
 int eth_dev_info_get_print_err(uint16_t port_id,
 			struct rte_eth_dev_info *dev_info);
+int eth_dev_conf_info_get_print_err(uint16_t port_id,
+			struct rte_eth_dev_conf_info *dev_info);
 void eth_set_promisc_mode(uint16_t port_id, int enable);
 void eth_set_allmulticast_mode(uint16_t port, int enable);
 int eth_link_get_nowait_print_err(uint16_t port_id, struct rte_eth_link *link);
diff --git a/app/test-pmd/util.c b/app/test-pmd/util.c
index 5dd7157947..c6992fba7a 100644
--- a/app/test-pmd/util.c
+++ b/app/test-pmd/util.c
@@ -440,6 +440,21 @@ eth_dev_info_get_print_err(uint16_t port_id,
 	return ret;
 }
 
+int
+eth_dev_conf_info_get_print_err(uint16_t port_id,
+				struct rte_eth_dev_conf_info *dev_conf_info)
+{
+	int ret;
+
+	ret = rte_eth_dev_conf_info_get(port_id, dev_conf_info);
+	if (ret != 0)
+		fprintf(stderr,
+			"Error during getting device config (port %u) info: %s\n",
+			port_id, strerror(-ret));
+
+	return ret;
+}
+
 void
 eth_set_promisc_mode(uint16_t port, int enable)
 {
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 9d95cd11e1..74184099a1 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -3458,6 +3458,33 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
 	return 0;
 }
 
+int
+rte_eth_dev_conf_info_get(uint16_t port_id,
+				struct rte_eth_dev_conf_info *dev_conf_info)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (dev_conf_info == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Cannot get ethdev port %u config info to NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
+	/*
+	 * Init dev_conf_info before port_id check since caller does not have
+	 * return status and does not know if get is successful or not.
+	 */
+	memset(dev_conf_info, 0, sizeof(struct rte_eth_dev_conf_info));
+
+	dev_conf_info->rx_offloads = dev->data->dev_conf.rxmode.offloads;
+	dev_conf_info->tx_offloads = dev->data->dev_conf.txmode.offloads;
+
+	return 0;
+}
+
 int
 rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
 				 uint32_t *ptypes, int num)
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index d2b27c351f..70a2db550f 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -1587,6 +1587,15 @@ struct rte_eth_dev_info {
 	void *reserved_ptrs[2];   /**< Reserved for future fields */
 };
 
+/**
+ * Ethernet device configuration information structure.
+ * Used to retrieve information about configured device.
+ */
+struct rte_eth_dev_conf_info {
+	uint64_t rx_offloads; /**rxmode offloads */
+	uint64_t tx_offloads; /**txmode offloads */
+};
+
 /**
  * RX/TX queue states
  */
@@ -3058,6 +3067,23 @@ int rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr);
  */
 int rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info);
 
+/**
+ * Retrieve the contextual information of an Ethernet device.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param dev_conf_info
+ *   A pointer to a structure of type *rte_eth_dev_conf_info* to be filled with
+ *   the contextual information of the Ethernet device.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if support for dev_infos_get() does not exist for the device.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+int rte_eth_dev_conf_info_get(uint16_t port_id,
+				struct rte_eth_dev_conf_info *dev_conf_info);
+
 /**
  * Retrieve the firmware version of a device.
  *
-- 
2.25.1

[dpdk-dev] [PATCH v6 0/2] testpmd shows incorrect rx_offload configuration

[Jie Wang]

Launch testpmd with multiple queues, and check rx_offload info.

When testpmd shows the port configuration, it doesn't show RSS_HASH.

---
v6: split this patch into two patches.
v5: add an API to get device configuration info.
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->dev_conf.rx/txmode".

Jie Wang (2):
  ethdev: add an API to get device configuration info
  app/testpmd: fix testpmd doesn't show RSS hash offload

 app/test-pmd/testpmd.c  | 33 +++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h  |  2 ++
 app/test-pmd/util.c     | 15 +++++++++++++++
 lib/ethdev/rte_ethdev.c | 27 +++++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.h | 26 ++++++++++++++++++++++++++
 5 files changed, 103 insertions(+)

-- 
2.25.1

[dpdk-dev] [PATCH v6 1/2] ethdev: add an API to get device configuration info

[Jie Wang]

This patch adds a new API "rte_eth_dev_conf_info_get()" to help testpmd get
device configuration info.

Signed-off-by: Jie Wang <jie1x.wang@intel.com>
---
 lib/ethdev/rte_ethdev.c | 27 +++++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.h | 26 ++++++++++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 9d95cd11e1..74184099a1 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -3458,6 +3458,33 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
 	return 0;
 }
 
+int
+rte_eth_dev_conf_info_get(uint16_t port_id,
+				struct rte_eth_dev_conf_info *dev_conf_info)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (dev_conf_info == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Cannot get ethdev port %u config info to NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
+	/*
+	 * Init dev_conf_info before port_id check since caller does not have
+	 * return status and does not know if get is successful or not.
+	 */
+	memset(dev_conf_info, 0, sizeof(struct rte_eth_dev_conf_info));
+
+	dev_conf_info->rx_offloads = dev->data->dev_conf.rxmode.offloads;
+	dev_conf_info->tx_offloads = dev->data->dev_conf.txmode.offloads;
+
+	return 0;
+}
+
 int
 rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
 				 uint32_t *ptypes, int num)
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index d2b27c351f..70a2db550f 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -1587,6 +1587,15 @@ struct rte_eth_dev_info {
 	void *reserved_ptrs[2];   /**< Reserved for future fields */
 };
 
+/**
+ * Ethernet device configuration information structure.
+ * Used to retrieve information about configured device.
+ */
+struct rte_eth_dev_conf_info {
+	uint64_t rx_offloads; /**rxmode offloads */
+	uint64_t tx_offloads; /**txmode offloads */
+};
+
 /**
  * RX/TX queue states
  */
@@ -3058,6 +3067,23 @@ int rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr);
  */
 int rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info);
 
+/**
+ * Retrieve the contextual information of an Ethernet device.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param dev_conf_info
+ *   A pointer to a structure of type *rte_eth_dev_conf_info* to be filled with
+ *   the contextual information of the Ethernet device.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if support for dev_infos_get() does not exist for the device.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+int rte_eth_dev_conf_info_get(uint16_t port_id,
+				struct rte_eth_dev_conf_info *dev_conf_info);
+
 /**
  * Retrieve the firmware version of a device.
  *
-- 
2.25.1

[dpdk-dev] [PATCH v6 2/2] app/testpmd: fix testpmd doesn't show RSS hash offload

[Jie Wang]

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.

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 <jie1x.wang@intel.com>
---
 app/test-pmd/testpmd.c | 33 +++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h |  2 ++
 app/test-pmd/util.c    | 15 +++++++++++++++
 3 files changed, 50 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 6cbe9ba3c8..9a78805673 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2461,6 +2461,9 @@ start_port(portid_t pid)
 		}
 
 		if (port->need_reconfig > 0) {
+			struct rte_eth_dev_conf_info dev_conf_info;
+			int k;
+
 			port->need_reconfig = 0;
 
 			if (flow_isolate_all) {
@@ -2498,6 +2501,36 @@ start_port(portid_t pid)
 				port->need_reconfig = 1;
 				return -1;
 			}
+			/* get offloads */
+			if (0 !=
+				eth_dev_conf_info_get_print_err(pi,
+							&dev_conf_info)) {
+				rte_exit(EXIT_FAILURE,
+				    "rte_eth_dev_conf_info_get() failed\n");
+				return -1;
+			}
+			/* Apply Rx offloads configuration */
+			if (dev_conf_info.rx_offloads !=
+				port->dev_conf.rxmode.offloads) {
+				port->dev_conf.rxmode.offloads =
+					dev_conf_info.rx_offloads;
+				for (k = 0;
+				     k < port->dev_info.max_rx_queues;
+				     k++)
+					port->rx_conf[k].offloads =
+						dev_conf_info.rx_offloads;
+			}
+			/* Apply Tx offloads configuration */
+			if (dev_conf_info.tx_offloads !=
+				port->dev_conf.txmode.offloads) {
+				port->dev_conf.txmode.offloads =
+					dev_conf_info.tx_offloads;
+				for (k = 0;
+				     k < port->dev_info.max_tx_queues;
+				     k++)
+					port->tx_conf[k].offloads =
+						dev_conf_info.tx_offloads;
+			}
 		}
 		if (port->need_reconfig_queues > 0) {
 			port->need_reconfig_queues = 0;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 16a3598e48..4cf69a17a3 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -950,6 +950,8 @@ void show_gro(portid_t port_id);
 void setup_gso(const char *mode, portid_t port_id);
 int eth_dev_info_get_print_err(uint16_t port_id,
 			struct rte_eth_dev_info *dev_info);
+int eth_dev_conf_info_get_print_err(uint16_t port_id,
+			struct rte_eth_dev_conf_info *dev_info);
 void eth_set_promisc_mode(uint16_t port_id, int enable);
 void eth_set_allmulticast_mode(uint16_t port, int enable);
 int eth_link_get_nowait_print_err(uint16_t port_id, struct rte_eth_link *link);
diff --git a/app/test-pmd/util.c b/app/test-pmd/util.c
index 5dd7157947..c6992fba7a 100644
--- a/app/test-pmd/util.c
+++ b/app/test-pmd/util.c
@@ -440,6 +440,21 @@ eth_dev_info_get_print_err(uint16_t port_id,
 	return ret;
 }
 
+int
+eth_dev_conf_info_get_print_err(uint16_t port_id,
+				struct rte_eth_dev_conf_info *dev_conf_info)
+{
+	int ret;
+
+	ret = rte_eth_dev_conf_info_get(port_id, dev_conf_info);
+	if (ret != 0)
+		fprintf(stderr,
+			"Error during getting device config (port %u) info: %s\n",
+			port_id, strerror(-ret));
+
+	return ret;
+}
+
 void
 eth_set_promisc_mode(uint16_t port, int enable)
 {
-- 
2.25.1

[dpdk-dev] [PATCH v7 0/2] testpmd shows incorrect rx_offload configuration

[Jie Wang]

Launch testpmd with multiple queues, and check rx_offload info.

When testpmd shows the port configuration, it doesn't show RSS_HASH.

---
v7:
 - delete struct "rte_eth_dev_conf_info", and reuse struct "rte_eth_conf".
 - add "__rte_experimental" to the new API "rte_eth_dev_conf_info_get" declaration.
v6: split this patch into two patches.
v5: add an API to get device configuration info.
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->dev_conf.rx/txmode".

Jie Wang (2):
  ethdev: add an API to get device configuration info
  app/testpmd: fix testpmd doesn't show RSS hash offload

 app/test-pmd/testpmd.c  | 33 +++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h  |  2 ++
 app/test-pmd/util.c     | 15 +++++++++++++++
 lib/ethdev/rte_ethdev.c | 28 ++++++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.h | 21 +++++++++++++++++++++
 lib/ethdev/version.map  |  3 +++
 6 files changed, 102 insertions(+)

-- 
2.25.1

[dpdk-dev] [PATCH v7 1/2] ethdev: add an API to get device configuration info

[Jie Wang]

This patch adds a new API "rte_eth_dev_conf_info_get()" to help users get
device configuration info.

Signed-off-by: Jie Wang <jie1x.wang@intel.com>
---
 lib/ethdev/rte_ethdev.c | 28 ++++++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.h | 21 +++++++++++++++++++++
 lib/ethdev/version.map  |  3 +++
 3 files changed, 52 insertions(+)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 9d95cd11e1..bbe24af662 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -3458,6 +3458,34 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
 	return 0;
 }
 
+int
+rte_eth_dev_conf_info_get(uint16_t port_id,
+				struct rte_eth_conf *dev_conf_info)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (dev_conf_info == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Cannot get ethdev port %u configuration info to NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
+	/*
+	 * Init dev_conf_info before port_id check since caller does not have
+	 * return status and does not know if get is successful or not.
+	 */
+	memset(dev_conf_info, 0, sizeof(struct rte_eth_conf));
+
+	/* copy dev->data->dev_conf to dev_conf_info */
+	memcpy(dev_conf_info, &dev->data->dev_conf,
+		sizeof(struct rte_eth_conf));
+
+	return 0;
+}
+
 int
 rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
 				 uint32_t *ptypes, int num)
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index d2b27c351f..3c5b02a133 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -3058,6 +3058,27 @@ int rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr);
  */
 int rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Retrieve the contextual information of an Ethernet device.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param dev_conf_info
+ *   A pointer to a structure of type *rte_eth_dev* to be filled with
+ *   the contextual information of the Ethernet device.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if support for dev_infos_get() does not exist for the device.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+__rte_experimental
+int rte_eth_dev_conf_info_get(uint16_t port_id,
+				struct rte_eth_conf *dev_conf_info);
+
 /**
  * Retrieve the firmware version of a device.
  *
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 44d30b05ae..40539f99f9 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -249,6 +249,9 @@ EXPERIMENTAL {
 	rte_mtr_meter_policy_delete;
 	rte_mtr_meter_policy_update;
 	rte_mtr_meter_policy_validate;
+
+	# added in 21.11
+	rte_eth_dev_conf_info_get;
 };
 
 INTERNAL {
-- 
2.25.1

[dpdk-dev] [PATCH v7 2/2] app/testpmd: fix testpmd doesn't show RSS hash offload

[Jie Wang]

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.

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

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")

Signed-off-by: Jie Wang <jie1x.wang@intel.com>
---
 app/test-pmd/testpmd.c | 33 +++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h |  2 ++
 app/test-pmd/util.c    | 15 +++++++++++++++
 3 files changed, 50 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 6cbe9ba3c8..c0bdb740fe 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2461,6 +2461,9 @@ start_port(portid_t pid)
 		}
 
 		if (port->need_reconfig > 0) {
+			struct rte_eth_conf dev_conf_info;
+			int k;
+
 			port->need_reconfig = 0;
 
 			if (flow_isolate_all) {
@@ -2498,6 +2501,36 @@ start_port(portid_t pid)
 				port->need_reconfig = 1;
 				return -1;
 			}
+			/* get offloads */
+			if (0 !=
+				eth_dev_conf_info_get_print_err(pi,
+							&dev_conf_info)) {
+				rte_exit(EXIT_FAILURE,
+				    "rte_eth_dev_conf_info_get() failed\n");
+				return -1;
+			}
+			/* Apply Rx offloads configuration */
+			if (dev_conf_info.rxmode.offloads !=
+				port->dev_conf.rxmode.offloads) {
+				port->dev_conf.rxmode.offloads =
+					dev_conf_info.rxmode.offloads;
+				for (k = 0;
+				     k < port->dev_info.max_rx_queues;
+				     k++)
+					port->rx_conf[k].offloads =
+						dev_conf_info.rxmode.offloads;
+			}
+			/* Apply Tx offloads configuration */
+			if (dev_conf_info.txmode.offloads !=
+				port->dev_conf.txmode.offloads) {
+				port->dev_conf.txmode.offloads =
+					dev_conf_info.txmode.offloads;
+				for (k = 0;
+				     k < port->dev_info.max_tx_queues;
+				     k++)
+					port->tx_conf[k].offloads =
+						dev_conf_info.txmode.offloads;
+			}
 		}
 		if (port->need_reconfig_queues > 0) {
 			port->need_reconfig_queues = 0;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 16a3598e48..6970a20ee4 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -950,6 +950,8 @@ void show_gro(portid_t port_id);
 void setup_gso(const char *mode, portid_t port_id);
 int eth_dev_info_get_print_err(uint16_t port_id,
 			struct rte_eth_dev_info *dev_info);
+int eth_dev_conf_info_get_print_err(uint16_t port_id,
+			struct rte_eth_conf *dev_conf_info);
 void eth_set_promisc_mode(uint16_t port_id, int enable);
 void eth_set_allmulticast_mode(uint16_t port, int enable);
 int eth_link_get_nowait_print_err(uint16_t port_id, struct rte_eth_link *link);
diff --git a/app/test-pmd/util.c b/app/test-pmd/util.c
index 5dd7157947..170d8f5c61 100644
--- a/app/test-pmd/util.c
+++ b/app/test-pmd/util.c
@@ -440,6 +440,21 @@ eth_dev_info_get_print_err(uint16_t port_id,
 	return ret;
 }
 
+int
+eth_dev_conf_info_get_print_err(uint16_t port_id,
+				struct rte_eth_conf *dev_conf_info)
+{
+	int ret;
+
+	ret = rte_eth_dev_conf_info_get(port_id, dev_conf_info);
+	if (ret != 0)
+		fprintf(stderr,
+			"Error during getting device configuration (port %u) info: %s\n",
+			port_id, strerror(-ret));
+
+	return ret;
+}
+
 void
 eth_set_promisc_mode(uint16_t port, int enable)
 {
-- 
2.25.1

Re: [dpdk-dev] [PATCH v7 2/2] app/testpmd: fix testpmd doesn't show RSS hash offload

[Li, Xiaoyun]

Hi

> -----Original Message-----
> From: Wang, Jie1X <jie1x.wang@intel.com>
> Sent: Thursday, August 26, 2021 15:09
> To: dev@dpdk.org
> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; andrew.rybchenko@oktetlabs.ru;
> thomas@monjalon.net; Yigit, Ferruh <ferruh.yigit@intel.com>; Wang, Jie1X
> <jie1x.wang@intel.com>
> Subject: [PATCH v7 2/2] 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.
> 
> This patch updates the offloads info if it changes to fix this issue.
> 
> Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
> 
> Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> ---
>  app/test-pmd/testpmd.c | 33 +++++++++++++++++++++++++++++++++
> app/test-pmd/testpmd.h |  2 ++
>  app/test-pmd/util.c    | 15 +++++++++++++++
>  3 files changed, 50 insertions(+)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> 6cbe9ba3c8..c0bdb740fe 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -2461,6 +2461,9 @@ start_port(portid_t pid)
>  		}
> 
>  		if (port->need_reconfig > 0) {
> +			struct rte_eth_conf dev_conf_info;
> +			int k;
> +
>  			port->need_reconfig = 0;
> 
>  			if (flow_isolate_all) {
> @@ -2498,6 +2501,36 @@ start_port(portid_t pid)
>  				port->need_reconfig = 1;
>  				return -1;
>  			}
> +			/* get offloads */
> +			if (0 !=
> +				eth_dev_conf_info_get_print_err(pi,
> +							&dev_conf_info)) {
> +				rte_exit(EXIT_FAILURE,
> +				    "rte_eth_dev_conf_info_get() failed\n");

rte_exit will directly exit the program. "return -1" after this won't work. Please just print errlog and return -1 like other failures.

> +				return -1;
> +			}
> +			/* Apply Rx offloads configuration */
> +			if (dev_conf_info.rxmode.offloads !=
> +				port->dev_conf.rxmode.offloads) {
> +				port->dev_conf.rxmode.offloads =
> +					dev_conf_info.rxmode.offloads;
> +				for (k = 0;
> +				     k < port->dev_info.max_rx_queues;
> +				     k++)
> +					port->rx_conf[k].offloads =
> +
> 	dev_conf_info.rxmode.offloads;
> +			}
> +			/* Apply Tx offloads configuration */
> +			if (dev_conf_info.txmode.offloads !=
> +				port->dev_conf.txmode.offloads) {
> +				port->dev_conf.txmode.offloads =
> +					dev_conf_info.txmode.offloads;
> +				for (k = 0;
> +				     k < port->dev_info.max_tx_queues;
> +				     k++)
> +					port->tx_conf[k].offloads =
> +
> 	dev_conf_info.txmode.offloads;
> +			}
>  		}
>  		if (port->need_reconfig_queues > 0) {
>  			port->need_reconfig_queues = 0;
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index
> 16a3598e48..6970a20ee4 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -950,6 +950,8 @@ void show_gro(portid_t port_id);  void setup_gso(const
> char *mode, portid_t port_id);  int eth_dev_info_get_print_err(uint16_t port_id,
>  			struct rte_eth_dev_info *dev_info);
> +int eth_dev_conf_info_get_print_err(uint16_t port_id,
> +			struct rte_eth_conf *dev_conf_info);
>  void eth_set_promisc_mode(uint16_t port_id, int enable);  void
> eth_set_allmulticast_mode(uint16_t port, int enable);  int
> eth_link_get_nowait_print_err(uint16_t port_id, struct rte_eth_link *link); diff --
> git a/app/test-pmd/util.c b/app/test-pmd/util.c index 5dd7157947..170d8f5c61
> 100644
> --- a/app/test-pmd/util.c
> +++ b/app/test-pmd/util.c
> @@ -440,6 +440,21 @@ eth_dev_info_get_print_err(uint16_t port_id,
>  	return ret;
>  }
> 
> +int
> +eth_dev_conf_info_get_print_err(uint16_t port_id,
> +				struct rte_eth_conf *dev_conf_info) {
> +	int ret;
> +
> +	ret = rte_eth_dev_conf_info_get(port_id, dev_conf_info);
> +	if (ret != 0)
> +		fprintf(stderr,
> +			"Error during getting device configuration (port %u)
> info: %s\n",
> +			port_id, strerror(-ret));
> +
> +	return ret;
> +}
> +
>  void
>  eth_set_promisc_mode(uint16_t port, int enable)  {
> --
> 2.25.1

[dpdk-dev] [PATCH v8 0/2] testpmd shows incorrect rx_offload configuration

[Jie Wang]

Launch testpmd with multiple queues, and check rx_offload info.

When testpmd shows the port configuration, it doesn't show RSS_HASH.

---
v8: delete "rte_exit" and just print error log.
v7:
 - delete struct "rte_eth_dev_conf_info", and reuse struct "rte_eth_conf".
 - add "__rte_experimental" to the new API "rte_eth_dev_conf_info_get" declaration.
v6: split this patch into two patches.
v5: add an API to get device configuration info.
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->dev_conf.rx/txmode".

Jie Wang (2):
  ethdev: add an API to get device configuration info
  app/testpmd: fix testpmd doesn't show RSS hash offload

 app/test-pmd/testpmd.c  | 34 ++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h  |  2 ++
 app/test-pmd/util.c     | 15 +++++++++++++++
 lib/ethdev/rte_ethdev.c | 28 ++++++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.h | 21 +++++++++++++++++++++
 lib/ethdev/version.map  |  3 +++
 6 files changed, 103 insertions(+)

-- 
2.25.1

[dpdk-dev] [PATCH v8 1/2] ethdev: add an API to get device configuration info

[Jie Wang]

This patch adds a new API "rte_eth_dev_conf_info_get()" to help users get
device configuration info.

Signed-off-by: Jie Wang <jie1x.wang@intel.com>
---
 lib/ethdev/rte_ethdev.c | 28 ++++++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.h | 21 +++++++++++++++++++++
 lib/ethdev/version.map  |  3 +++
 3 files changed, 52 insertions(+)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 9d95cd11e1..bbe24af662 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -3458,6 +3458,34 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
 	return 0;
 }
 
+int
+rte_eth_dev_conf_info_get(uint16_t port_id,
+				struct rte_eth_conf *dev_conf_info)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (dev_conf_info == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Cannot get ethdev port %u configuration info to NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
+	/*
+	 * Init dev_conf_info before port_id check since caller does not have
+	 * return status and does not know if get is successful or not.
+	 */
+	memset(dev_conf_info, 0, sizeof(struct rte_eth_conf));
+
+	/* copy dev->data->dev_conf to dev_conf_info */
+	memcpy(dev_conf_info, &dev->data->dev_conf,
+		sizeof(struct rte_eth_conf));
+
+	return 0;
+}
+
 int
 rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
 				 uint32_t *ptypes, int num)
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index d2b27c351f..3c5b02a133 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -3058,6 +3058,27 @@ int rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr);
  */
 int rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Retrieve the contextual information of an Ethernet device.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param dev_conf_info
+ *   A pointer to a structure of type *rte_eth_dev* to be filled with
+ *   the contextual information of the Ethernet device.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if support for dev_infos_get() does not exist for the device.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+__rte_experimental
+int rte_eth_dev_conf_info_get(uint16_t port_id,
+				struct rte_eth_conf *dev_conf_info);
+
 /**
  * Retrieve the firmware version of a device.
  *
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 44d30b05ae..40539f99f9 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -249,6 +249,9 @@ EXPERIMENTAL {
 	rte_mtr_meter_policy_delete;
 	rte_mtr_meter_policy_update;
 	rte_mtr_meter_policy_validate;
+
+	# added in 21.11
+	rte_eth_dev_conf_info_get;
 };
 
 INTERNAL {
-- 
2.25.1

Re: [dpdk-dev] [PATCH v8 1/2] ethdev: add an API to get device configuration info

[Ferruh Yigit]

On 8/27/2021 9:17 AM, Jie Wang wrote:
> This patch adds a new API "rte_eth_dev_conf_info_get()" to help users get
> device configuration info.
> 
> Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> ---
>  lib/ethdev/rte_ethdev.c | 28 ++++++++++++++++++++++++++++
>  lib/ethdev/rte_ethdev.h | 21 +++++++++++++++++++++
>  lib/ethdev/version.map  |  3 +++
>  3 files changed, 52 insertions(+)
> 

Can you add a release notes update for this?

> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index 9d95cd11e1..bbe24af662 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -3458,6 +3458,34 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
>  	return 0;
>  }
>  
> +int
> +rte_eth_dev_conf_info_get(uint16_t port_id,
> +				struct rte_eth_conf *dev_conf_info)
> +{
> +	struct rte_eth_dev *dev;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +	dev = &rte_eth_devices[port_id];
> +
> +	if (dev_conf_info == NULL) {
> +		RTE_ETHDEV_LOG(ERR, "Cannot get ethdev port %u configuration info to NULL\n",

Can you break the line after 'ERR', to reduce the column length?

> +			port_id);
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * Init dev_conf_info before port_id check since caller does not have
> +	 * return status and does not know if get is successful or not.
> +	 */
> +	memset(dev_conf_info, 0, sizeof(struct rte_eth_conf));
> +

I didn't get the reasoning for the memset, won't it be overwriten anyway by
below 'memcpy'?

> +	/* copy dev->data->dev_conf to dev_conf_info */
> +	memcpy(dev_conf_info, &dev->data->dev_conf,
> +		sizeof(struct rte_eth_conf));
> +
> +	return 0;
> +}
> +
>  int
>  rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
>  				 uint32_t *ptypes, int num)
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index d2b27c351f..3c5b02a133 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -3058,6 +3058,27 @@ int rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr);
>   */
>  int rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info);
>  
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Retrieve the contextual information of an Ethernet device.
> + *

'rte_eth_dev_info_get()' is using "contextual information" but for this API can
we say 'configuration'?

> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param dev_conf_info
> + *   A pointer to a structure of type *rte_eth_dev* to be filled with
> + *   the contextual information of the Ethernet device.

Just to make it more clear, what do you think to highlight that memory should be
allocated by the caller?

> + * @return
> + *   - (0) if successful.
> + *   - (-ENOTSUP) if support for dev_infos_get() does not exist for the device.

I think '-ENOTSUP' is not returned by the API.

> + *   - (-ENODEV) if *port_id* invalid.
> + *   - (-EINVAL) if bad parameter.
> + */
> +__rte_experimental
> +int rte_eth_dev_conf_info_get(uint16_t port_id,
> +				struct rte_eth_conf *dev_conf_info);
> +
>  /**
>   * Retrieve the firmware version of a device.
>   *
> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
> index 44d30b05ae..40539f99f9 100644
> --- a/lib/ethdev/version.map
> +++ b/lib/ethdev/version.map
> @@ -249,6 +249,9 @@ EXPERIMENTAL {
>  	rte_mtr_meter_policy_delete;
>  	rte_mtr_meter_policy_update;
>  	rte_mtr_meter_policy_validate;
> +
> +	# added in 21.11
> +	rte_eth_dev_conf_info_get;
>  };
>  
>  INTERNAL {
> 

[dpdk-dev] [PATCH v8 2/2] app/testpmd: fix testpmd doesn't show RSS hash offload

[Jie Wang]

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.

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

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")

Signed-off-by: Jie Wang <jie1x.wang@intel.com>
---
 app/test-pmd/testpmd.c | 34 ++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h |  2 ++
 app/test-pmd/util.c    | 15 +++++++++++++++
 3 files changed, 51 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 6cbe9ba3c8..bd67291160 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2461,6 +2461,9 @@ start_port(portid_t pid)
 		}
 
 		if (port->need_reconfig > 0) {
+			struct rte_eth_conf dev_conf_info;
+			int k;
+
 			port->need_reconfig = 0;
 
 			if (flow_isolate_all) {
@@ -2498,6 +2501,37 @@ start_port(portid_t pid)
 				port->need_reconfig = 1;
 				return -1;
 			}
+			/* get rte_eth_conf info */
+			if (0 !=
+				eth_dev_conf_info_get_print_err(pi,
+							&dev_conf_info)) {
+				fprintf(stderr,
+					"port %d can not get device configuration info\n",
+					pi);
+				return -1;
+			}
+			/* Apply Rx offloads configuration */
+			if (dev_conf_info.rxmode.offloads !=
+				port->dev_conf.rxmode.offloads) {
+				port->dev_conf.rxmode.offloads =
+					dev_conf_info.rxmode.offloads;
+				for (k = 0;
+				     k < port->dev_info.max_rx_queues;
+				     k++)
+					port->rx_conf[k].offloads =
+						dev_conf_info.rxmode.offloads;
+			}
+			/* Apply Tx offloads configuration */
+			if (dev_conf_info.txmode.offloads !=
+				port->dev_conf.txmode.offloads) {
+				port->dev_conf.txmode.offloads =
+					dev_conf_info.txmode.offloads;
+				for (k = 0;
+				     k < port->dev_info.max_tx_queues;
+				     k++)
+					port->tx_conf[k].offloads =
+						dev_conf_info.txmode.offloads;
+			}
 		}
 		if (port->need_reconfig_queues > 0) {
 			port->need_reconfig_queues = 0;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 16a3598e48..6970a20ee4 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -950,6 +950,8 @@ void show_gro(portid_t port_id);
 void setup_gso(const char *mode, portid_t port_id);
 int eth_dev_info_get_print_err(uint16_t port_id,
 			struct rte_eth_dev_info *dev_info);
+int eth_dev_conf_info_get_print_err(uint16_t port_id,
+			struct rte_eth_conf *dev_conf_info);
 void eth_set_promisc_mode(uint16_t port_id, int enable);
 void eth_set_allmulticast_mode(uint16_t port, int enable);
 int eth_link_get_nowait_print_err(uint16_t port_id, struct rte_eth_link *link);
diff --git a/app/test-pmd/util.c b/app/test-pmd/util.c
index 5dd7157947..170d8f5c61 100644
--- a/app/test-pmd/util.c
+++ b/app/test-pmd/util.c
@@ -440,6 +440,21 @@ eth_dev_info_get_print_err(uint16_t port_id,
 	return ret;
 }
 
+int
+eth_dev_conf_info_get_print_err(uint16_t port_id,
+				struct rte_eth_conf *dev_conf_info)
+{
+	int ret;
+
+	ret = rte_eth_dev_conf_info_get(port_id, dev_conf_info);
+	if (ret != 0)
+		fprintf(stderr,
+			"Error during getting device configuration (port %u) info: %s\n",
+			port_id, strerror(-ret));
+
+	return ret;
+}
+
 void
 eth_set_promisc_mode(uint16_t port, int enable)
 {
-- 
2.25.1

Re: [dpdk-dev] [PATCH v8 2/2] app/testpmd: fix testpmd doesn't show RSS hash offload

[Li, Xiaoyun]

> -----Original Message-----
> From: Wang, Jie1X <jie1x.wang@intel.com>
> Sent: Friday, August 27, 2021 16:18
> To: dev@dpdk.org
> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; andrew.rybchenko@oktetlabs.ru;
> thomas@monjalon.net; Yigit, Ferruh <ferruh.yigit@intel.com>; Wang, Jie1X
> <jie1x.wang@intel.com>
> Subject: [PATCH v8 2/2] 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.
> 
> This patch updates the offloads info if it changes to fix this issue.
> 
> Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
> 
> Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> ---
>  app/test-pmd/testpmd.c | 34 ++++++++++++++++++++++++++++++++++
>  app/test-pmd/testpmd.h |  2 ++
>  app/test-pmd/util.c    | 15 +++++++++++++++
>  3 files changed, 51 insertions(+)
> 
Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>

Re: [dpdk-dev] [PATCH v8 2/2] app/testpmd: fix testpmd doesn't show RSS hash offload

[Ferruh Yigit]

On 8/27/2021 9:17 AM, Jie Wang wrote:
> 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.
> 
> This patch updates the offloads info if it changes to fix this issue.
> 
> Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
> 
> Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> ---
>  app/test-pmd/testpmd.c | 34 ++++++++++++++++++++++++++++++++++
>  app/test-pmd/testpmd.h |  2 ++
>  app/test-pmd/util.c    | 15 +++++++++++++++
>  3 files changed, 51 insertions(+)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 6cbe9ba3c8..bd67291160 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -2461,6 +2461,9 @@ start_port(portid_t pid)
>  		}
>  
>  		if (port->need_reconfig > 0) {
> +			struct rte_eth_conf dev_conf_info;
> +			int k;
> +
>  			port->need_reconfig = 0;
>  
>  			if (flow_isolate_all) {
> @@ -2498,6 +2501,37 @@ start_port(portid_t pid)
>  				port->need_reconfig = 1;
>  				return -1;
>  			}
> +			/* get rte_eth_conf info */
> +			if (0 !=
> +				eth_dev_conf_info_get_print_err(pi,
> +							&dev_conf_info)) {
> +				fprintf(stderr,
> +					"port %d can not get device configuration info\n",
> +					pi);
> +				return -1;
> +			}
> +			/* Apply Rx offloads configuration */
> +			if (dev_conf_info.rxmode.offloads !=
> +				port->dev_conf.rxmode.offloads) {
> +				port->dev_conf.rxmode.offloads =
> +					dev_conf_info.rxmode.offloads;
> +				for (k = 0;
> +				     k < port->dev_info.max_rx_queues;
> +				     k++)
> +					port->rx_conf[k].offloads =
> +						dev_conf_info.rxmode.offloads;
> +			}
> +			/* Apply Tx offloads configuration */
> +			if (dev_conf_info.txmode.offloads !=
> +				port->dev_conf.txmode.offloads) {
> +				port->dev_conf.txmode.offloads =
> +					dev_conf_info.txmode.offloads;
> +				for (k = 0;
> +				     k < port->dev_info.max_tx_queues;
> +				     k++)
> +					port->tx_conf[k].offloads =
> +						dev_conf_info.txmode.offloads;
> +			}
>  		}

Above implementation gets the configuration from device and applies it to the
testpmd configuration.

Instead, what about a long level target to get rid of testpmd specific copy of
the configuration and rely and the config provided by devices. @Xiaoyun, what do
you think, does this make sense?

So instead of above code, update where RSS hash offload information printed to
use device retrieved config instead of testpmd config, will it work?

Re: [dpdk-dev] [PATCH v8 2/2] app/testpmd: fix testpmd doesn't show RSS hash offload

[Li, Xiaoyun]

Hi

> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Thursday, September 9, 2021 00:51
> To: Wang, Jie1X <jie1x.wang@intel.com>; dev@dpdk.org; Li, Xiaoyun
> <xiaoyun.li@intel.com>
> Cc: andrew.rybchenko@oktetlabs.ru; thomas@monjalon.net
> Subject: Re: [PATCH v8 2/2] app/testpmd: fix testpmd doesn't show RSS hash
> offload
> 
> On 8/27/2021 9:17 AM, Jie Wang wrote:
> > 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.
> >
> > This patch updates the offloads info if it changes to fix this issue.
> >
> > Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
> >
> > Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> > ---
> >  app/test-pmd/testpmd.c | 34 ++++++++++++++++++++++++++++++++++
> >  app/test-pmd/testpmd.h |  2 ++
> >  app/test-pmd/util.c    | 15 +++++++++++++++
> >  3 files changed, 51 insertions(+)
> >
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > 6cbe9ba3c8..bd67291160 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -2461,6 +2461,9 @@ start_port(portid_t pid)
> >  		}
> >
> >  		if (port->need_reconfig > 0) {
> > +			struct rte_eth_conf dev_conf_info;
> > +			int k;
> > +
> >  			port->need_reconfig = 0;
> >
> >  			if (flow_isolate_all) {
> > @@ -2498,6 +2501,37 @@ start_port(portid_t pid)
> >  				port->need_reconfig = 1;
> >  				return -1;
> >  			}
> > +			/* get rte_eth_conf info */
> > +			if (0 !=
> > +				eth_dev_conf_info_get_print_err(pi,
> > +							&dev_conf_info)) {
> > +				fprintf(stderr,
> > +					"port %d can not get device
> configuration info\n",
> > +					pi);
> > +				return -1;
> > +			}
> > +			/* Apply Rx offloads configuration */
> > +			if (dev_conf_info.rxmode.offloads !=
> > +				port->dev_conf.rxmode.offloads) {
> > +				port->dev_conf.rxmode.offloads =
> > +					dev_conf_info.rxmode.offloads;
> > +				for (k = 0;
> > +				     k < port->dev_info.max_rx_queues;
> > +				     k++)
> > +					port->rx_conf[k].offloads =
> > +
> 	dev_conf_info.rxmode.offloads;
> > +			}
> > +			/* Apply Tx offloads configuration */
> > +			if (dev_conf_info.txmode.offloads !=
> > +				port->dev_conf.txmode.offloads) {
> > +				port->dev_conf.txmode.offloads =
> > +					dev_conf_info.txmode.offloads;
> > +				for (k = 0;
> > +				     k < port->dev_info.max_tx_queues;
> > +				     k++)
> > +					port->tx_conf[k].offloads =
> > +
> 	dev_conf_info.txmode.offloads;
> > +			}
> >  		}
> 
> Above implementation gets the configuration from device and applies it to the
> testpmd configuration.
> 
> Instead, what about a long level target to get rid of testpmd specific copy of the
> configuration and rely and the config provided by devices. @Xiaoyun, what do
> you think, does this make sense?

You mean remove port->dev_conf and rx/tx_conf completely in the future? Or keep it in initial stage?

Now, port->dev_conf will take global tx/rx_mode, fdir_conf and change some based on dev_info capabilities. And then use dev_configure to apply them for device.
After this, actually, dev->data->dev_conf contains all device configuration.

So It seems it's OK to remove port->dev_conf completely. Just testpmd needs to be refactored a lot and regression test in case of issues.
But from long term view, it's good to keep one source and avoid copy.

As for rx/tx_conf, it takes device default tx/rx_conf in dev_info and some settings in testpmd parameters also offloads from dev_conf.
So keep port->rx/tx_conf? But then it still needs copy from dev_conf since this may change.

> 
> So instead of above code, update where RSS hash offload information printed to
> use device retrieved config instead of testpmd config, will it work?

It's OK for device offload configurations.
But queue offloads are a bit tricky since dev->data->dev_conf doesn't include queue conf.
And it's not fair to use device offload configurations for queue offloads since user can use cmdline to config queue offload and that info can only be saved in port->rx/tx_conf and configure the device in setup_queue.

Re: [dpdk-dev] [PATCH v8 2/2] app/testpmd: fix testpmd doesn't show RSS hash offload

[Ferruh Yigit]

On 9/9/2021 4:31 AM, Li, Xiaoyun wrote:
> Hi
> 
>> -----Original Message-----
>> From: Yigit, Ferruh <ferruh.yigit@intel.com>
>> Sent: Thursday, September 9, 2021 00:51
>> To: Wang, Jie1X <jie1x.wang@intel.com>; dev@dpdk.org; Li, Xiaoyun
>> <xiaoyun.li@intel.com>
>> Cc: andrew.rybchenko@oktetlabs.ru; thomas@monjalon.net
>> Subject: Re: [PATCH v8 2/2] app/testpmd: fix testpmd doesn't show RSS hash
>> offload
>>
>> On 8/27/2021 9:17 AM, Jie Wang wrote:
>>> 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.
>>>
>>> This patch updates the offloads info if it changes to fix this issue.
>>>
>>> Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
>>>
>>> Signed-off-by: Jie Wang <jie1x.wang@intel.com>
>>> ---
>>>  app/test-pmd/testpmd.c | 34 ++++++++++++++++++++++++++++++++++
>>>  app/test-pmd/testpmd.h |  2 ++
>>>  app/test-pmd/util.c    | 15 +++++++++++++++
>>>  3 files changed, 51 insertions(+)
>>>
>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
>>> 6cbe9ba3c8..bd67291160 100644
>>> --- a/app/test-pmd/testpmd.c
>>> +++ b/app/test-pmd/testpmd.c
>>> @@ -2461,6 +2461,9 @@ start_port(portid_t pid)
>>>  		}
>>>
>>>  		if (port->need_reconfig > 0) {
>>> +			struct rte_eth_conf dev_conf_info;
>>> +			int k;
>>> +
>>>  			port->need_reconfig = 0;
>>>
>>>  			if (flow_isolate_all) {
>>> @@ -2498,6 +2501,37 @@ start_port(portid_t pid)
>>>  				port->need_reconfig = 1;
>>>  				return -1;
>>>  			}
>>> +			/* get rte_eth_conf info */
>>> +			if (0 !=
>>> +				eth_dev_conf_info_get_print_err(pi,
>>> +							&dev_conf_info)) {
>>> +				fprintf(stderr,
>>> +					"port %d can not get device
>> configuration info\n",
>>> +					pi);
>>> +				return -1;
>>> +			}
>>> +			/* Apply Rx offloads configuration */
>>> +			if (dev_conf_info.rxmode.offloads !=
>>> +				port->dev_conf.rxmode.offloads) {
>>> +				port->dev_conf.rxmode.offloads =
>>> +					dev_conf_info.rxmode.offloads;
>>> +				for (k = 0;
>>> +				     k < port->dev_info.max_rx_queues;
>>> +				     k++)
>>> +					port->rx_conf[k].offloads =
>>> +
>> 	dev_conf_info.rxmode.offloads;
>>> +			}
>>> +			/* Apply Tx offloads configuration */
>>> +			if (dev_conf_info.txmode.offloads !=
>>> +				port->dev_conf.txmode.offloads) {
>>> +				port->dev_conf.txmode.offloads =
>>> +					dev_conf_info.txmode.offloads;
>>> +				for (k = 0;
>>> +				     k < port->dev_info.max_tx_queues;
>>> +				     k++)
>>> +					port->tx_conf[k].offloads =
>>> +
>> 	dev_conf_info.txmode.offloads;
>>> +			}
>>>  		}
>>
>> Above implementation gets the configuration from device and applies it to the
>> testpmd configuration.
>>
>> Instead, what about a long level target to get rid of testpmd specific copy of the
>> configuration and rely and the config provided by devices. @Xiaoyun, what do
>> you think, does this make sense?
> 
> You mean remove port->dev_conf and rx/tx_conf completely in the future? Or keep it in initial stage?
> 
> Now, port->dev_conf will take global tx/rx_mode, fdir_conf and change some based on dev_info capabilities. And then use dev_configure to apply them for device.
> After this, actually, dev->data->dev_conf contains all device configuration.
> 
> So It seems it's OK to remove port->dev_conf completely. Just testpmd needs to be refactored a lot and regression test in case of issues.
> But from long term view, it's good to keep one source and avoid copy.
> 

Yes, this is the intention I have for long term. I expect that testpmd still
will keep some configuration in application level but we can prevent some
duplication.

And the main point is, by cleaning up testpmd we can recognize blockers and fix
them in libraries to help user applications.

> As for rx/tx_conf, it takes device default tx/rx_conf in dev_info and some settings in testpmd parameters also offloads from dev_conf.
> So keep port->rx/tx_conf? But then it still needs copy from dev_conf since this may change.
> 

I am not very clear what is suggested above, can you please elaborate?

And 'struct rte_port' seems has following structs that can be get from library:
struct rte_eth_dev_info dev_info;
struct rte_eth_conf     dev_conf;
struct rte_eth_rxconf   rx_conf[]
struct rte_eth_txconf   tx_conf[]

I don't think we can remove them, but perhaps reduce the usage of them, please
see below.

>>
>> So instead of above code, update where RSS hash offload information printed to
>> use device retrieved config instead of testpmd config, will it work?
> 
> It's OK for device offload configurations.
> But queue offloads are a bit tricky since dev->data->dev_conf doesn't include queue conf.
> And it's not fair to use device offload configurations for queue offloads since user can use cmdline to config queue offload and that info can only be saved in port->rx/tx_conf and configure the device in setup_queue.
> 

It is common in testpmd that, a command changes the application copy of the
configs, and mark as device configuration is required (for port or for queue).
So in later stage this changed configuration is applied to device.

This async approach has its benefits and I don't think we should change it.
(Also has some disadvantages that we hit in the past, like detecting some
configuration can't be applied in later stage when we try to apply the config,
not when command is issued at first place.).

What we can do it, reduce the testpmd config usage for the case to gather user
requests and apply them to device.
But to display device configuration, or to decide based on device configuration
we can user config values get by device by APIs.

What do you think, can above distinction makes sense, or does it work?


And there is still a chance that application copy of config diverge from device
config, and since we provide full config in our APIs (not changes), there is a
chance to overwrite a device configuration.
To prevent this it is possible to read device config and overwrite testpmd
config with that, similar to what this patch does, but I am not sure where this
sync can be done. What do you think about doing this just after device configured?

Thanks,
ferruh

Re: [dpdk-dev] [PATCH v8 2/2] app/testpmd: fix testpmd doesn't show RSS hash offload

[Li, Xiaoyun]

Hi

> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Friday, September 17, 2021 18:20
> To: Li, Xiaoyun <xiaoyun.li@intel.com>; Wang, Jie1X <jie1x.wang@intel.com>;
> dev@dpdk.org
> Cc: andrew.rybchenko@oktetlabs.ru; thomas@monjalon.net;
> jerinj@marvell.com; Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Subject: Re: [PATCH v8 2/2] app/testpmd: fix testpmd doesn't show RSS hash
> offload
> 
> On 9/9/2021 4:31 AM, Li, Xiaoyun wrote:
> > Hi
> >
> >> -----Original Message-----
> >> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> >> Sent: Thursday, September 9, 2021 00:51
> >> To: Wang, Jie1X <jie1x.wang@intel.com>; dev@dpdk.org; Li, Xiaoyun
> >> <xiaoyun.li@intel.com>
> >> Cc: andrew.rybchenko@oktetlabs.ru; thomas@monjalon.net
> >> Subject: Re: [PATCH v8 2/2] app/testpmd: fix testpmd doesn't show RSS
> >> hash offload
> >>
> >> On 8/27/2021 9:17 AM, Jie Wang wrote:
> >>> 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.
> >>>
> >>> This patch updates the offloads info if it changes to fix this issue.
> >>>
> >>> Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
> >>>
> >>> Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> >>> ---
> >>>  app/test-pmd/testpmd.c | 34 ++++++++++++++++++++++++++++++++++
> >>>  app/test-pmd/testpmd.h |  2 ++
> >>>  app/test-pmd/util.c    | 15 +++++++++++++++
> >>>  3 files changed, 51 insertions(+)
> >>>
> >>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> >>> 6cbe9ba3c8..bd67291160 100644
> >>> --- a/app/test-pmd/testpmd.c
> >>> +++ b/app/test-pmd/testpmd.c
> >>> @@ -2461,6 +2461,9 @@ start_port(portid_t pid)
> >>>  		}
> >>>
> >>>  		if (port->need_reconfig > 0) {
> >>> +			struct rte_eth_conf dev_conf_info;
> >>> +			int k;
> >>> +
> >>>  			port->need_reconfig = 0;
> >>>
> >>>  			if (flow_isolate_all) {
> >>> @@ -2498,6 +2501,37 @@ start_port(portid_t pid)
> >>>  				port->need_reconfig = 1;
> >>>  				return -1;
> >>>  			}
> >>> +			/* get rte_eth_conf info */
> >>> +			if (0 !=
> >>> +				eth_dev_conf_info_get_print_err(pi,
> >>> +							&dev_conf_info)) {
> >>> +				fprintf(stderr,
> >>> +					"port %d can not get device
> >> configuration info\n",
> >>> +					pi);
> >>> +				return -1;
> >>> +			}
> >>> +			/* Apply Rx offloads configuration */
> >>> +			if (dev_conf_info.rxmode.offloads !=
> >>> +				port->dev_conf.rxmode.offloads) {
> >>> +				port->dev_conf.rxmode.offloads =
> >>> +					dev_conf_info.rxmode.offloads;
> >>> +				for (k = 0;
> >>> +				     k < port->dev_info.max_rx_queues;
> >>> +				     k++)
> >>> +					port->rx_conf[k].offloads =
> >>> +
> >> 	dev_conf_info.rxmode.offloads;
> >>> +			}
> >>> +			/* Apply Tx offloads configuration */
> >>> +			if (dev_conf_info.txmode.offloads !=
> >>> +				port->dev_conf.txmode.offloads) {
> >>> +				port->dev_conf.txmode.offloads =
> >>> +					dev_conf_info.txmode.offloads;
> >>> +				for (k = 0;
> >>> +				     k < port->dev_info.max_tx_queues;
> >>> +				     k++)
> >>> +					port->tx_conf[k].offloads =
> >>> +
> >> 	dev_conf_info.txmode.offloads;
> >>> +			}
> >>>  		}
> >>
> >> Above implementation gets the configuration from device and applies
> >> it to the testpmd configuration.
> >>
> >> Instead, what about a long level target to get rid of testpmd
> >> specific copy of the configuration and rely and the config provided
> >> by devices. @Xiaoyun, what do you think, does this make sense?
> >
> > You mean remove port->dev_conf and rx/tx_conf completely in the future? Or
> keep it in initial stage?
> >
> > Now, port->dev_conf will take global tx/rx_mode, fdir_conf and change some
> based on dev_info capabilities. And then use dev_configure to apply them for
> device.
> > After this, actually, dev->data->dev_conf contains all device configuration.
> >
> > So It seems it's OK to remove port->dev_conf completely. Just testpmd needs
> to be refactored a lot and regression test in case of issues.
> > But from long term view, it's good to keep one source and avoid copy.
> >
> 
> Yes, this is the intention I have for long term. I expect that testpmd still will keep
> some configuration in application level but we can prevent some duplication.
> 
> And the main point is, by cleaning up testpmd we can recognize blockers and fix
> them in libraries to help user applications.
> 
> > As for rx/tx_conf, it takes device default tx/rx_conf in dev_info and some
> settings in testpmd parameters also offloads from dev_conf.
> > So keep port->rx/tx_conf? But then it still needs copy from dev_conf since this
> may change.
> >
> 
> I am not very clear what is suggested above, can you please elaborate?
> 
> And 'struct rte_port' seems has following structs that can be get from library:
> struct rte_eth_dev_info dev_info;
> struct rte_eth_conf     dev_conf;
> struct rte_eth_rxconf   rx_conf[]
> struct rte_eth_txconf   tx_conf[]
> 
> I don't think we can remove them, but perhaps reduce the usage of them, please
> see below.
> 
> >>
> >> So instead of above code, update where RSS hash offload information
> >> printed to use device retrieved config instead of testpmd config, will it work?
> >
> > It's OK for device offload configurations.
> > But queue offloads are a bit tricky since dev->data->dev_conf doesn't include
> queue conf.
> > And it's not fair to use device offload configurations for queue offloads since
> user can use cmdline to config queue offload and that info can only be saved in
> port->rx/tx_conf and configure the device in setup_queue.
> >
> 
> It is common in testpmd that, a command changes the application copy of the
> configs, and mark as device configuration is required (for port or for queue).
> So in later stage this changed configuration is applied to device.
> 
> This async approach has its benefits and I don't think we should change it.
> (Also has some disadvantages that we hit in the past, like detecting some
> configuration can't be applied in later stage when we try to apply the config, not
> when command is issued at first place.).
> 
> What we can do it, reduce the testpmd config usage for the case to gather user
> requests and apply them to device.
> But to display device configuration, or to decide based on device configuration
> we can user config values get by device by APIs.
> 
> What do you think, can above distinction makes sense, or does it work?
> 
> 
> And there is still a chance that application copy of config diverge from device
> config, and since we provide full config in our APIs (not changes), there is a
> chance to overwrite a device configuration.
> To prevent this it is possible to read device config and overwrite testpmd config
> with that, similar to what this patch does, but I am not sure where this sync can
> be done. What do you think about doing this just after device configured?

I'm not sure I fully understand.
So for showing cmd, just use API rte_eth_tx/rx_queue_info_get to get dev queue config and new added API rte_eth_dev_conf_info_get to get dev config.

And for the cases where port->dev_config is used as a right value, replace them with use getting API.
For example: "if (res->value == port->dev_conf.rxmode.max_rx_pkt_len)" will be changed like "if (res->value == rte_eth_dev_conf_info_get().rxmode.max_rx_pkt_len)"

But other things keep the same as what this patch does?

This makes sense to me if I understand it right.
> 
> Thanks,
> ferruh

Re: [dpdk-dev] [PATCH v8 2/2] app/testpmd: fix testpmd doesn't show RSS hash offload

[Ferruh Yigit]

On 9/18/2021 3:18 AM, Li, Xiaoyun wrote:
> Hi
> 
>> -----Original Message-----
>> From: Yigit, Ferruh <ferruh.yigit@intel.com>
>> Sent: Friday, September 17, 2021 18:20
>> To: Li, Xiaoyun <xiaoyun.li@intel.com>; Wang, Jie1X <jie1x.wang@intel.com>;
>> dev@dpdk.org
>> Cc: andrew.rybchenko@oktetlabs.ru; thomas@monjalon.net;
>> jerinj@marvell.com; Ananyev, Konstantin <konstantin.ananyev@intel.com>
>> Subject: Re: [PATCH v8 2/2] app/testpmd: fix testpmd doesn't show RSS hash
>> offload
>>
>> On 9/9/2021 4:31 AM, Li, Xiaoyun wrote:
>>> Hi
>>>
>>>> -----Original Message-----
>>>> From: Yigit, Ferruh <ferruh.yigit@intel.com>
>>>> Sent: Thursday, September 9, 2021 00:51
>>>> To: Wang, Jie1X <jie1x.wang@intel.com>; dev@dpdk.org; Li, Xiaoyun
>>>> <xiaoyun.li@intel.com>
>>>> Cc: andrew.rybchenko@oktetlabs.ru; thomas@monjalon.net
>>>> Subject: Re: [PATCH v8 2/2] app/testpmd: fix testpmd doesn't show RSS
>>>> hash offload
>>>>
>>>> On 8/27/2021 9:17 AM, Jie Wang wrote:
>>>>> 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.
>>>>>
>>>>> This patch updates the offloads info if it changes to fix this issue.
>>>>>
>>>>> Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
>>>>>
>>>>> Signed-off-by: Jie Wang <jie1x.wang@intel.com>
>>>>> ---
>>>>>  app/test-pmd/testpmd.c | 34 ++++++++++++++++++++++++++++++++++
>>>>>  app/test-pmd/testpmd.h |  2 ++
>>>>>  app/test-pmd/util.c    | 15 +++++++++++++++
>>>>>  3 files changed, 51 insertions(+)
>>>>>
>>>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
>>>>> 6cbe9ba3c8..bd67291160 100644
>>>>> --- a/app/test-pmd/testpmd.c
>>>>> +++ b/app/test-pmd/testpmd.c
>>>>> @@ -2461,6 +2461,9 @@ start_port(portid_t pid)
>>>>>  		}
>>>>>
>>>>>  		if (port->need_reconfig > 0) {
>>>>> +			struct rte_eth_conf dev_conf_info;
>>>>> +			int k;
>>>>> +
>>>>>  			port->need_reconfig = 0;
>>>>>
>>>>>  			if (flow_isolate_all) {
>>>>> @@ -2498,6 +2501,37 @@ start_port(portid_t pid)
>>>>>  				port->need_reconfig = 1;
>>>>>  				return -1;
>>>>>  			}
>>>>> +			/* get rte_eth_conf info */
>>>>> +			if (0 !=
>>>>> +				eth_dev_conf_info_get_print_err(pi,
>>>>> +							&dev_conf_info)) {
>>>>> +				fprintf(stderr,
>>>>> +					"port %d can not get device
>>>> configuration info\n",
>>>>> +					pi);
>>>>> +				return -1;
>>>>> +			}
>>>>> +			/* Apply Rx offloads configuration */
>>>>> +			if (dev_conf_info.rxmode.offloads !=
>>>>> +				port->dev_conf.rxmode.offloads) {
>>>>> +				port->dev_conf.rxmode.offloads =
>>>>> +					dev_conf_info.rxmode.offloads;
>>>>> +				for (k = 0;
>>>>> +				     k < port->dev_info.max_rx_queues;
>>>>> +				     k++)
>>>>> +					port->rx_conf[k].offloads =
>>>>> +
>>>> 	dev_conf_info.rxmode.offloads;
>>>>> +			}
>>>>> +			/* Apply Tx offloads configuration */
>>>>> +			if (dev_conf_info.txmode.offloads !=
>>>>> +				port->dev_conf.txmode.offloads) {
>>>>> +				port->dev_conf.txmode.offloads =
>>>>> +					dev_conf_info.txmode.offloads;
>>>>> +				for (k = 0;
>>>>> +				     k < port->dev_info.max_tx_queues;
>>>>> +				     k++)
>>>>> +					port->tx_conf[k].offloads =
>>>>> +
>>>> 	dev_conf_info.txmode.offloads;
>>>>> +			}
>>>>>  		}
>>>>
>>>> Above implementation gets the configuration from device and applies
>>>> it to the testpmd configuration.
>>>>
>>>> Instead, what about a long level target to get rid of testpmd
>>>> specific copy of the configuration and rely and the config provided
>>>> by devices. @Xiaoyun, what do you think, does this make sense?
>>>
>>> You mean remove port->dev_conf and rx/tx_conf completely in the future? Or
>> keep it in initial stage?
>>>
>>> Now, port->dev_conf will take global tx/rx_mode, fdir_conf and change some
>> based on dev_info capabilities. And then use dev_configure to apply them for
>> device.
>>> After this, actually, dev->data->dev_conf contains all device configuration.
>>>
>>> So It seems it's OK to remove port->dev_conf completely. Just testpmd needs
>> to be refactored a lot and regression test in case of issues.
>>> But from long term view, it's good to keep one source and avoid copy.
>>>
>>
>> Yes, this is the intention I have for long term. I expect that testpmd still will keep
>> some configuration in application level but we can prevent some duplication.
>>
>> And the main point is, by cleaning up testpmd we can recognize blockers and fix
>> them in libraries to help user applications.
>>
>>> As for rx/tx_conf, it takes device default tx/rx_conf in dev_info and some
>> settings in testpmd parameters also offloads from dev_conf.
>>> So keep port->rx/tx_conf? But then it still needs copy from dev_conf since this
>> may change.
>>>
>>
>> I am not very clear what is suggested above, can you please elaborate?
>>
>> And 'struct rte_port' seems has following structs that can be get from library:
>> struct rte_eth_dev_info dev_info;
>> struct rte_eth_conf     dev_conf;
>> struct rte_eth_rxconf   rx_conf[]
>> struct rte_eth_txconf   tx_conf[]
>>
>> I don't think we can remove them, but perhaps reduce the usage of them, please
>> see below.
>>
>>>>
>>>> So instead of above code, update where RSS hash offload information
>>>> printed to use device retrieved config instead of testpmd config, will it work?
>>>
>>> It's OK for device offload configurations.
>>> But queue offloads are a bit tricky since dev->data->dev_conf doesn't include
>> queue conf.
>>> And it's not fair to use device offload configurations for queue offloads since
>> user can use cmdline to config queue offload and that info can only be saved in
>> port->rx/tx_conf and configure the device in setup_queue.
>>>
>>
>> It is common in testpmd that, a command changes the application copy of the
>> configs, and mark as device configuration is required (for port or for queue).
>> So in later stage this changed configuration is applied to device.
>>
>> This async approach has its benefits and I don't think we should change it.
>> (Also has some disadvantages that we hit in the past, like detecting some
>> configuration can't be applied in later stage when we try to apply the config, not
>> when command is issued at first place.).
>>
>> What we can do it, reduce the testpmd config usage for the case to gather user
>> requests and apply them to device.
>> But to display device configuration, or to decide based on device configuration
>> we can user config values get by device by APIs.
>>
>> What do you think, can above distinction makes sense, or does it work?
>>
>>
>> And there is still a chance that application copy of config diverge from device
>> config, and since we provide full config in our APIs (not changes), there is a
>> chance to overwrite a device configuration.
>> To prevent this it is possible to read device config and overwrite testpmd config
>> with that, similar to what this patch does, but I am not sure where this sync can
>> be done. What do you think about doing this just after device configured?
> 
> I'm not sure I fully understand.
> So for showing cmd, just use API rte_eth_tx/rx_queue_info_get to get dev queue config and new added API rte_eth_dev_conf_info_get to get dev config.
> 
> And for the cases where port->dev_config is used as a right value, replace them with use getting API.
> For example: "if (res->value == port->dev_conf.rxmode.max_rx_pkt_len)" will be changed like "if (res->value == rte_eth_dev_conf_info_get().rxmode.max_rx_pkt_len)"
> 
> But other things keep the same as what this patch does?
> 

Yes. (Only I have a small comment on this patch, I will comment on other tread.)

And for this patch I don't suggest any additional change other than RSS show,
rest can be updated gradually.

> This makes sense to me if I understand it right.

Re: [dpdk-dev] [PATCH v8 2/2] app/testpmd: fix testpmd doesn't show RSS hash offload

[Ferruh Yigit]

On 8/27/2021 9:17 AM, Jie Wang wrote:
> 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.
> 
> This patch updates the offloads info if it changes to fix this issue.
> 
> Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
> 
> Signed-off-by: Jie Wang <jie1x.wang@intel.com>

<...>

> +			/* Apply Rx offloads configuration */
> +			if (dev_conf_info.rxmode.offloads !=
> +				port->dev_conf.rxmode.offloads) {
> +				port->dev_conf.rxmode.offloads =
> +					dev_conf_info.rxmode.offloads;
> +				for (k = 0;
> +				     k < port->dev_info.max_rx_queues;
> +				     k++)
> +					port->rx_conf[k].offloads =
> +						dev_conf_info.rxmode.offloads;

If queue specific offloads are used, won't this overwrite it with port offload?

Should we get queue config from device and update queue offloads with it?

Re: [dpdk-dev] [PATCH v8 2/2] app/testpmd: fix testpmd doesn't show RSS hash offload

[Wang, Jie1X]

> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Monday, September 20, 2021 5:48 PM
> To: Wang, Jie1X <jie1x.wang@intel.com>; dev@dpdk.org
> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; andrew.rybchenko@oktetlabs.ru;
> thomas@monjalon.net
> Subject: Re: [PATCH v8 2/2] app/testpmd: fix testpmd doesn't show RSS hash
> offload
> 
> On 8/27/2021 9:17 AM, Jie Wang wrote:
> > 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.
> >
> > This patch updates the offloads info if it changes to fix this issue.
> >
> > Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
> >
> > Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> 
> <...>
> 
> > +			/* Apply Rx offloads configuration */
> > +			if (dev_conf_info.rxmode.offloads !=
> > +				port->dev_conf.rxmode.offloads) {
> > +				port->dev_conf.rxmode.offloads =
> > +					dev_conf_info.rxmode.offloads;
> > +				for (k = 0;
> > +				     k < port->dev_info.max_rx_queues;
> > +				     k++)
> > +					port->rx_conf[k].offloads =
> > +
> 	dev_conf_info.rxmode.offloads;
> 
> If queue specific offloads are used, won't this overwrite it with port offload?
> 
> Should we get queue config from device and update queue offloads with it?

Only the first time the driver configures the port, "dev_conf_info.rxmode.offloads" is not equal to "port->dev_conf.rxmode.offloads". So the added code just run 1 time.

But your suggestion is correct, I should update the queue offloads instead of overwriting it.

[dpdk-dev] [PATCH v9 0/3] testpmd shows incorrect rx_offload configuration

[Jie Wang]

Launch testpmd with multiple queues, and check rx_offload info.

When testpmd shows the port configuration, it doesn't show RSS_HASH.

---
v9:
 - add a release notes update for the new API.
 - update the description of the new API.
 - optimize the new API.
 - optimize the assignment of the offloads.
v8: delete "rte_exit" and just print error log.
v7:
 - delete struct "rte_eth_dev_conf_info", and reuse struct "rte_eth_conf".
 - add "__rte_experimental" to the new API "rte_eth_dev_conf_info_get" declaration.
v6: split this patch into two patches.
v5: add an API to get device configuration info.
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->dev_conf.rx/txmode".

Jie Wang (3):
  ethdev: add an API to get device configuration info
  doc: update release notes for new API
  app/testpmd: fix testpmd doesn't show RSS hash offload

 app/test-pmd/cmdline.c                 | 14 +++++++++--
 app/test-pmd/testpmd.c                 | 34 ++++++++++++++++++++++++++
 app/test-pmd/testpmd.h                 |  2 ++
 app/test-pmd/util.c                    | 15 ++++++++++++
 doc/guides/rel_notes/release_21_11.rst |  4 +++
 lib/ethdev/rte_ethdev.c                | 23 +++++++++++++++++
 lib/ethdev/rte_ethdev.h                | 21 ++++++++++++++++
 lib/ethdev/version.map                 |  3 +++
 8 files changed, 114 insertions(+), 2 deletions(-)

-- 
2.25.1

[dpdk-dev] [PATCH v9 1/3] ethdev: add an API to get device configuration info

[Jie Wang]

This patch adds a new API "rte_eth_dev_conf_info_get()" to help users get
device configuration info.

Cc: stable@dpdk.org

Signed-off-by: Jie Wang <jie1x.wang@intel.com>
---
 lib/ethdev/rte_ethdev.c | 23 +++++++++++++++++++++++
 lib/ethdev/rte_ethdev.h | 21 +++++++++++++++++++++
 lib/ethdev/version.map  |  3 +++
 3 files changed, 47 insertions(+)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index daf5ca9242..a0f521323a 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -3457,6 +3457,29 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
 	return 0;
 }
 
+int
+rte_eth_dev_conf_info_get(uint16_t port_id,
+				struct rte_eth_conf *dev_conf_info)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (dev_conf_info == NULL) {
+		RTE_ETHDEV_LOG(ERR,
+			"Cannot get ethdev port %u configuration info to NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
+	/* copy dev->data->dev_conf to dev_conf_info */
+	memcpy(dev_conf_info, &dev->data->dev_conf,
+		sizeof(struct rte_eth_conf));
+
+	return 0;
+}
+
 int
 rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
 				 uint32_t *ptypes, int num)
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 1da37896d8..c21ee6a1fe 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -3068,6 +3068,27 @@ int rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr);
  */
 int rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Retrieve the configuration of an Ethernet device.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param dev_conf_info
+ *   A pointer to a structure of type *rte_eth_conf* to be filled with
+ *   the configuration of the Ethernet device.
+ *   And the memory of the structure should be allocated by the caller.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+__rte_experimental
+int rte_eth_dev_conf_info_get(uint16_t port_id,
+				struct rte_eth_conf *dev_conf_info);
+
 /**
  * Retrieve the firmware version of a device.
  *
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 904bce6ea1..4b0a1f0fae 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -247,6 +247,9 @@ EXPERIMENTAL {
 	rte_mtr_meter_policy_delete;
 	rte_mtr_meter_policy_update;
 	rte_mtr_meter_policy_validate;
+
+	# added in 21.11
+	rte_eth_dev_conf_info_get;
 };
 
 INTERNAL {
-- 
2.25.1

Re: [dpdk-dev] [PATCH v9 1/3] ethdev: add an API to get device configuration info

[Thomas Monjalon]

26/09/2021 11:20, Jie Wang:
> This patch adds a new API "rte_eth_dev_conf_info_get()" to help users get
> device configuration info.
> 
> Cc: stable@dpdk.org

No we don't backport new features.

> Signed-off-by: Jie Wang <jie1x.wang@intel.com>
[...]
> + * Retrieve the configuration of an Ethernet device.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param dev_conf_info
> + *   A pointer to a structure of type *rte_eth_conf* to be filled with
> + *   the configuration of the Ethernet device.
> + *   And the memory of the structure should be allocated by the caller.
> + * @return
> + *   - (0) if successful.
> + *   - (-ENODEV) if *port_id* invalid.
> + *   - (-EINVAL) if bad parameter.
> + */
> +__rte_experimental
> +int rte_eth_dev_conf_info_get(uint16_t port_id,
> +				struct rte_eth_conf *dev_conf_info);

It does not make sense to me.
rte_eth_conf is passed by the app to rte_eth_dev_configure.
Why the app would need to get the same info back?

Re: [dpdk-dev] [PATCH v9 1/3] ethdev: add an API to get device configuration info

[Wang, Jie1X]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Monday, September 27, 2021 2:19 PM
> To: Wang, Jie1X <jie1x.wang@intel.com>
> Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>;
> andrew.rybchenko@oktetlabs.ru; Li, Xiaoyun <xiaoyun.li@intel.com>; Yang,
> SteveX <stevex.yang@intel.com>
> Subject: Re: [PATCH v9 1/3] ethdev: add an API to get device configuration info
> 
> 26/09/2021 11:20, Jie Wang:
> > This patch adds a new API "rte_eth_dev_conf_info_get()" to help users
> > get device configuration info.
> >
> > Cc: stable@dpdk.org
> 
> No we don't backport new features.

Ok, I'll remove 'Cc: stable@dpdk.org'.

> 
> > Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> [...]
> > + * Retrieve the configuration of an Ethernet device.
> > + *
> > + * @param port_id
> > + *   The port identifier of the Ethernet device.
> > + * @param dev_conf_info
> > + *   A pointer to a structure of type *rte_eth_conf* to be filled with
> > + *   the configuration of the Ethernet device.
> > + *   And the memory of the structure should be allocated by the caller.
> > + * @return
> > + *   - (0) if successful.
> > + *   - (-ENODEV) if *port_id* invalid.
> > + *   - (-EINVAL) if bad parameter.
> > + */
> > +__rte_experimental
> > +int rte_eth_dev_conf_info_get(uint16_t port_id,
> > +				struct rte_eth_conf *dev_conf_info);
> 
> It does not make sense to me.
> rte_eth_conf is passed by the app to rte_eth_dev_configure.
> Why the app would need to get the same info back?
> 
> 

In rte_eth_dev_configure, dev->data->dev_conf copies the info from port->dev_conf, and then the driver updates it. It doesn't same as port->dev_conf.
We need to get the updated device configuration.

Re: [dpdk-dev] [PATCH v9 1/3] ethdev: add an API to get device configuration info

[Thomas Monjalon]

27/09/2021 09:21, Wang, Jie1X:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 26/09/2021 11:20, Jie Wang:
> > > This patch adds a new API "rte_eth_dev_conf_info_get()" to help users
> > > get device configuration info.
> > [...]
> > > + * Retrieve the configuration of an Ethernet device.
> > > + *
> > > + * @param port_id
> > > + *   The port identifier of the Ethernet device.
> > > + * @param dev_conf_info
> > > + *   A pointer to a structure of type *rte_eth_conf* to be filled with
> > > + *   the configuration of the Ethernet device.
> > > + *   And the memory of the structure should be allocated by the caller.
> > > + * @return
> > > + *   - (0) if successful.
> > > + *   - (-ENODEV) if *port_id* invalid.
> > > + *   - (-EINVAL) if bad parameter.
> > > + */
> > > +__rte_experimental
> > > +int rte_eth_dev_conf_info_get(uint16_t port_id,
> > > +				struct rte_eth_conf *dev_conf_info);
> > 
> > It does not make sense to me.
> > rte_eth_conf is passed by the app to rte_eth_dev_configure.
> > Why the app would need to get the same info back?
> > 
> > 
> 
> In rte_eth_dev_configure, dev->data->dev_conf copies the info from port->dev_conf, and then the driver updates it. It doesn't same as port->dev_conf.
> We need to get the updated device configuration.

OK I see.
Please update the commit log to explain this.

Re: [dpdk-dev] [PATCH v9 1/3] ethdev: add an API to get device configuration info

[Ferruh Yigit]

On 9/27/2021 8:56 AM, Thomas Monjalon wrote:
> 27/09/2021 09:21, Wang, Jie1X:
>> From: Thomas Monjalon <thomas@monjalon.net>
>>> 26/09/2021 11:20, Jie Wang:
>>>> This patch adds a new API "rte_eth_dev_conf_info_get()" to help users
>>>> get device configuration info.
>>> [...]
>>>> + * Retrieve the configuration of an Ethernet device.
>>>> + *
>>>> + * @param port_id
>>>> + *   The port identifier of the Ethernet device.
>>>> + * @param dev_conf_info
>>>> + *   A pointer to a structure of type *rte_eth_conf* to be filled with
>>>> + *   the configuration of the Ethernet device.
>>>> + *   And the memory of the structure should be allocated by the caller.
>>>> + * @return
>>>> + *   - (0) if successful.
>>>> + *   - (-ENODEV) if *port_id* invalid.
>>>> + *   - (-EINVAL) if bad parameter.
>>>> + */
>>>> +__rte_experimental
>>>> +int rte_eth_dev_conf_info_get(uint16_t port_id,
>>>> +				struct rte_eth_conf *dev_conf_info);
>>>
>>> It does not make sense to me.
>>> rte_eth_conf is passed by the app to rte_eth_dev_configure.
>>> Why the app would need to get the same info back?
>>>
>>>
>>
>> In rte_eth_dev_configure, dev->data->dev_conf copies the info from port->dev_conf, and then the driver updates it. It doesn't same as port->dev_conf.
>> We need to get the updated device configuration.
> 
> OK I see.
> Please update the commit log to explain this.
> 

Also either an application needs to keep copy of the configuration (like testpmd
does), or won't have any way to know device configuration details.
And for the apps that keeps the configuration, it has a risk that application
copy and device copy of the configuration diverged, as Jie mentioned.

I think it makes sense to have a way to get the configuration from device, small
applications can rely on it without keeping a copy of a config at all.

And for testpmd, we have aligned with Xiaoyun to rely on the device
configuration more, in a way:
- When to display a config, use device copy as much as possible
- Use app copy of config to accumulate user config change requests to apply them
later, sync app config with device config after config applied.

Re: [dpdk-dev] [PATCH v9 1/3] ethdev: add an API to get device configuration info

[Thomas Monjalon]

04/10/2021 13:20, Ferruh Yigit:
> On 9/27/2021 8:56 AM, Thomas Monjalon wrote:
> > 27/09/2021 09:21, Wang, Jie1X:
> >> From: Thomas Monjalon <thomas@monjalon.net>
> >>> 26/09/2021 11:20, Jie Wang:
> >>>> This patch adds a new API "rte_eth_dev_conf_info_get()" to help users
> >>>> get device configuration info.
> >>> [...]
> >>>> + * Retrieve the configuration of an Ethernet device.
> >>>> + *
> >>>> + * @param port_id
> >>>> + *   The port identifier of the Ethernet device.
> >>>> + * @param dev_conf_info
> >>>> + *   A pointer to a structure of type *rte_eth_conf* to be filled with
> >>>> + *   the configuration of the Ethernet device.
> >>>> + *   And the memory of the structure should be allocated by the caller.
> >>>> + * @return
> >>>> + *   - (0) if successful.
> >>>> + *   - (-ENODEV) if *port_id* invalid.
> >>>> + *   - (-EINVAL) if bad parameter.
> >>>> + */
> >>>> +__rte_experimental
> >>>> +int rte_eth_dev_conf_info_get(uint16_t port_id,
> >>>> +				struct rte_eth_conf *dev_conf_info);
> >>>
> >>> It does not make sense to me.
> >>> rte_eth_conf is passed by the app to rte_eth_dev_configure.
> >>> Why the app would need to get the same info back?
> >>>
> >>>
> >>
> >> In rte_eth_dev_configure, dev->data->dev_conf copies the info from port->dev_conf, and then the driver updates it. It doesn't same as port->dev_conf.
> >> We need to get the updated device configuration.
> > 
> > OK I see.
> > Please update the commit log to explain this.
> > 
> 
> Also either an application needs to keep copy of the configuration (like testpmd
> does), or won't have any way to know device configuration details.
> And for the apps that keeps the configuration, it has a risk that application
> copy and device copy of the configuration diverged, as Jie mentioned.
> 
> I think it makes sense to have a way to get the configuration from device, small
> applications can rely on it without keeping a copy of a config at all.
> 
> And for testpmd, we have aligned with Xiaoyun to rely on the device
> configuration more, in a way:
> - When to display a config, use device copy as much as possible
> - Use app copy of config to accumulate user config change requests to apply them
> later, sync app config with device config after config applied.

Makes sense, thanks.

Re: [dpdk-dev] [PATCH v9 1/3] ethdev: add an API to get device configuration info

[Ferruh Yigit]

On 9/26/2021 10:20 AM, Jie Wang wrote:
> This patch adds a new API "rte_eth_dev_conf_info_get()" to help users get
> device configuration info.
> 
> Cc: stable@dpdk.org
> 

Since this is a new API, I think we can request it to be backported.

> Signed-off-by: Jie Wang <jie1x.wang@intel.com>

<...>

> @@ -247,6 +247,9 @@ EXPERIMENTAL {
>  	rte_mtr_meter_policy_delete;
>  	rte_mtr_meter_policy_update;
>  	rte_mtr_meter_policy_validate;
> +
> +	# added in 21.11
> +	rte_eth_dev_conf_info_get;

Not sure about the 'info' part in the API, what about 'rte_eth_dev_conf_get()'?

Re: [dpdk-dev] [PATCH v9 1/3] ethdev: add an API to get device configuration info

[Thomas Monjalon]

04/10/2021 13:22, Ferruh Yigit:
> On 9/26/2021 10:20 AM, Jie Wang wrote:
> > This patch adds a new API "rte_eth_dev_conf_info_get()" to help users get
> > device configuration info.
> > 
> > Cc: stable@dpdk.org
> > 
> 
> Since this is a new API, I think we can request it to be backported.

We cannot.

> > Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> 
> <...>
> 
> > @@ -247,6 +247,9 @@ EXPERIMENTAL {
> >  	rte_mtr_meter_policy_delete;
> >  	rte_mtr_meter_policy_update;
> >  	rte_mtr_meter_policy_validate;
> > +
> > +	# added in 21.11
> > +	rte_eth_dev_conf_info_get;
> 
> Not sure about the 'info' part in the API, what about 'rte_eth_dev_conf_get()'?

+1

Re: [dpdk-dev] [PATCH v9 1/3] ethdev: add an API to get device configuration info

[Ferruh Yigit]

On 10/4/2021 12:26 PM, Thomas Monjalon wrote:
> 04/10/2021 13:22, Ferruh Yigit:
>> On 9/26/2021 10:20 AM, Jie Wang wrote:
>>> This patch adds a new API "rte_eth_dev_conf_info_get()" to help users get
>>> device configuration info.
>>>
>>> Cc: stable@dpdk.org
>>>
>>
>> Since this is a new API, I think we can request it to be backported.
> 
> We cannot.
> 

Of course, it is a typo in my end, I mean "we can NOT request ..."

>>> Signed-off-by: Jie Wang <jie1x.wang@intel.com>
>>
>> <...>
>>
>>> @@ -247,6 +247,9 @@ EXPERIMENTAL {
>>>  	rte_mtr_meter_policy_delete;
>>>  	rte_mtr_meter_policy_update;
>>>  	rte_mtr_meter_policy_validate;
>>> +
>>> +	# added in 21.11
>>> +	rte_eth_dev_conf_info_get;
>>
>> Not sure about the 'info' part in the API, what about 'rte_eth_dev_conf_get()'?
> 
> +1
> 
> 

[dpdk-dev] [PATCH v9 2/3] doc: update release notes for new API

[Jie Wang]

Add information about new ethdev API.

Cc: stable@dpdk.org

Signed-off-by: Jie Wang <jie1x.wang@intel.com>
---
 doc/guides/rel_notes/release_21_11.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
index dcff939ae8..95e569f51c 100644
--- a/doc/guides/rel_notes/release_21_11.rst
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -111,6 +111,10 @@ New Features
   Added command-line options to specify total number of processes and
   current process ID. Each process owns subset of Rx and Tx queues.
 
+* **Added support for users get device configuration.**
+  Added an API which can help users get device configuration.
+  The declarations for the API's can be found in ``rte_ethdev.h``.
+
 
 Removed Items
 -------------
-- 
2.25.1

Re: [dpdk-dev] [PATCH v9 2/3] doc: update release notes for new API

[Ferruh Yigit]

On 9/26/2021 10:20 AM, Jie Wang wrote:
> Add information about new ethdev API.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> ---
>  doc/guides/rel_notes/release_21_11.rst | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
> index dcff939ae8..95e569f51c 100644
> --- a/doc/guides/rel_notes/release_21_11.rst
> +++ b/doc/guides/rel_notes/release_21_11.rst
> @@ -111,6 +111,10 @@ New Features
>    Added command-line options to specify total number of processes and
>    current process ID. Each process owns subset of Rx and Tx queues.
>  
> +* **Added support for users get device configuration.**
> +  Added an API which can help users get device configuration.
> +  The declarations for the API's can be found in ``rte_ethdev.h``.
> +
>  

No need to have a separate patch for release notes update, can you please merge
this one with 1/3 patch?

Re: [dpdk-dev] [PATCH v9 2/3] doc: update release notes for new API

[Thomas Monjalon]

04/10/2021 13:22, Ferruh Yigit:
> On 9/26/2021 10:20 AM, Jie Wang wrote:
> > Add information about new ethdev API.
> > 
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> > ---
> >  doc/guides/rel_notes/release_21_11.rst | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
> > index dcff939ae8..95e569f51c 100644
> > --- a/doc/guides/rel_notes/release_21_11.rst
> > +++ b/doc/guides/rel_notes/release_21_11.rst
> > @@ -111,6 +111,10 @@ New Features
> >    Added command-line options to specify total number of processes and
> >    current process ID. Each process owns subset of Rx and Tx queues.
> >  
> > +* **Added support for users get device configuration.**
> > +  Added an API which can help users get device configuration.
> > +  The declarations for the API's can be found in ``rte_ethdev.h``.
> > +
> >  
> 
> No need to have a separate patch for release notes update, can you please merge
> this one with 1/3 patch?

*not* have a separate patch

[dpdk-dev] [PATCH v9 3/3] app/testpmd: fix testpmd doesn't show RSS hash offload

[Jie Wang]

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.

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 <jie1x.wang@intel.com>
---
 app/test-pmd/cmdline.c | 14 ++++++++++++--
 app/test-pmd/testpmd.c | 34 ++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h |  2 ++
 app/test-pmd/util.c    | 15 +++++++++++++++
 4 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index a9efd027c3..3ef8f99358 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -15997,6 +15997,7 @@ cmd_rx_offload_get_configuration_parsed(
 	struct rte_eth_dev_info dev_info;
 	portid_t port_id = res->port_id;
 	struct rte_port *port = &ports[port_id];
+	struct rte_eth_conf dev_conf_info;
 	uint64_t port_offloads;
 	uint64_t queue_offloads;
 	uint16_t nb_rx_queues;
@@ -16005,7 +16006,11 @@ cmd_rx_offload_get_configuration_parsed(
 
 	printf("Rx Offloading Configuration of port %d :\n", port_id);
 
-	port_offloads = port->dev_conf.rxmode.offloads;
+	ret = eth_dev_conf_info_get_print_err(port_id, &dev_conf_info);
+	if (ret != 0)
+		return;
+
+	port_offloads = dev_conf_info.rxmode.offloads;
 	printf("  Port :");
 	print_rx_offloads(port_offloads);
 	printf("\n");
@@ -16411,6 +16416,7 @@ cmd_tx_offload_get_configuration_parsed(
 	struct rte_eth_dev_info dev_info;
 	portid_t port_id = res->port_id;
 	struct rte_port *port = &ports[port_id];
+	struct rte_eth_conf dev_conf_info;
 	uint64_t port_offloads;
 	uint64_t queue_offloads;
 	uint16_t nb_tx_queues;
@@ -16419,7 +16425,11 @@ cmd_tx_offload_get_configuration_parsed(
 
 	printf("Tx Offloading Configuration of port %d :\n", port_id);
 
-	port_offloads = port->dev_conf.txmode.offloads;
+	ret = eth_dev_conf_info_get_print_err(port_id, &dev_conf_info);
+	if (ret != 0)
+		return;
+
+	port_offloads = dev_conf_info.txmode.offloads;
 	printf("  Port :");
 	print_tx_offloads(port_offloads);
 	printf("\n");
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 97ae52e17e..56e6e80739 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2531,6 +2531,9 @@ start_port(portid_t pid)
 		}
 
 		if (port->need_reconfig > 0) {
+			struct rte_eth_conf dev_conf_info;
+			int k;
+
 			port->need_reconfig = 0;
 
 			if (flow_isolate_all) {
@@ -2568,6 +2571,37 @@ start_port(portid_t pid)
 				port->need_reconfig = 1;
 				return -1;
 			}
+			/* get rte_eth_conf info */
+			if (0 !=
+				eth_dev_conf_info_get_print_err(pi,
+							&dev_conf_info)) {
+				fprintf(stderr,
+					"port %d can not get device configuration info\n",
+					pi);
+				return -1;
+			}
+			/* Apply Rx offloads configuration */
+			if (dev_conf_info.rxmode.offloads !=
+			    port->dev_conf.rxmode.offloads) {
+				port->dev_conf.rxmode.offloads |=
+					dev_conf_info.rxmode.offloads;
+				for (k = 0;
+				     k < port->dev_info.max_rx_queues;
+				     k++)
+					port->rx_conf[k].offloads |=
+						dev_conf_info.rxmode.offloads;
+			}
+			/* Apply Tx offloads configuration */
+			if (dev_conf_info.txmode.offloads !=
+			    port->dev_conf.txmode.offloads) {
+				port->dev_conf.txmode.offloads |=
+					dev_conf_info.txmode.offloads;
+				for (k = 0;
+				     k < port->dev_info.max_tx_queues;
+				     k++)
+					port->tx_conf[k].offloads |=
+						dev_conf_info.txmode.offloads;
+			}
 		}
 		if (port->need_reconfig_queues > 0 && is_proc_primary()) {
 			port->need_reconfig_queues = 0;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 5863b2f43f..48dad40986 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -960,6 +960,8 @@ void show_gro(portid_t port_id);
 void setup_gso(const char *mode, portid_t port_id);
 int eth_dev_info_get_print_err(uint16_t port_id,
 			struct rte_eth_dev_info *dev_info);
+int eth_dev_conf_info_get_print_err(uint16_t port_id,
+			struct rte_eth_conf *dev_conf_info);
 void eth_set_promisc_mode(uint16_t port_id, int enable);
 void eth_set_allmulticast_mode(uint16_t port, int enable);
 int eth_link_get_nowait_print_err(uint16_t port_id, struct rte_eth_link *link);
diff --git a/app/test-pmd/util.c b/app/test-pmd/util.c
index 14a9a251fb..f04786c50f 100644
--- a/app/test-pmd/util.c
+++ b/app/test-pmd/util.c
@@ -444,6 +444,21 @@ eth_dev_info_get_print_err(uint16_t port_id,
 	return ret;
 }
 
+int
+eth_dev_conf_info_get_print_err(uint16_t port_id,
+				struct rte_eth_conf *dev_conf_info)
+{
+	int ret;
+
+	ret = rte_eth_dev_conf_info_get(port_id, dev_conf_info);
+	if (ret != 0)
+		fprintf(stderr,
+			"Error during getting device configuration (port %u) info: %s\n",
+			port_id, strerror(-ret));
+
+	return ret;
+}
+
 void
 eth_set_promisc_mode(uint16_t port, int enable)
 {
-- 
2.25.1

[dpdk-dev] [PATCH v10 0/2] testpmd shows incorrect rx_offload configuration

[Jie Wang]

Launch testpmd with multiple queues, and check rx_offload info.

When testpmd shows the port configuration, it doesn't show RSS_HASH.

---
v10:
 - update the commit log.
 - merge the first two patches.
 - rename the new API name.
v9:
 - add a release notes update for the new API.
 - update the description of the new API.
 - optimize the new API.
 - optimize the assignment of the offloads.
v8: delete "rte_exit" and just print error log.
v7:
 - delete struct "rte_eth_dev_conf_info", and reuse struct "rte_eth_conf".
 - add "__rte_experimental" to the new API "rte_eth_dev_conf_info_get" declaration.
v6: split this patch into two patches.
v5: add an API to get device configuration info.
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->dev_conf.rx/txmode".

Jie Wang (2):
  ethdev: add an API to get device configuration info
  app/testpmd: fix testpmd doesn't show RSS hash offload

 app/test-pmd/cmdline.c                 | 14 +++++++++--
 app/test-pmd/testpmd.c                 | 34 ++++++++++++++++++++++++++
 app/test-pmd/testpmd.h                 |  2 ++
 app/test-pmd/util.c                    | 15 ++++++++++++
 doc/guides/rel_notes/release_21_11.rst |  5 ++++
 lib/ethdev/rte_ethdev.c                | 23 +++++++++++++++++
 lib/ethdev/rte_ethdev.h                | 21 ++++++++++++++++
 lib/ethdev/version.map                 |  3 +++
 8 files changed, 115 insertions(+), 2 deletions(-)

-- 
2.25.1

[dpdk-dev] [PATCH v10 1/2] ethdev: add an API to get device configuration info

[Jie Wang]

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.

This patch adds a new API "rte_eth_dev_conf_get()" to help users
get device configuration info.

Add information about the new API in release notes.

Signed-off-by: Jie Wang <jie1x.wang@intel.com>
---
 doc/guides/rel_notes/release_21_11.rst |  5 +++++
 lib/ethdev/rte_ethdev.c                | 23 +++++++++++++++++++++++
 lib/ethdev/rte_ethdev.h                | 21 +++++++++++++++++++++
 lib/ethdev/version.map                 |  3 +++
 4 files changed, 52 insertions(+)

diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
index f099b1cca2..c16cc83fd1 100644
--- a/doc/guides/rel_notes/release_21_11.rst
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -129,6 +129,11 @@ New Features
   * Added tests to validate packets hard expiry.
   * Added tests to verify tunnel header verification in IPsec inbound.
 
+* **Added support for users get device configuration.**
+
+  * Added an API which can help users get device configuration.
+    The declarations for the API's can be found in ``rte_ethdev.h``.
+
 
 Removed Items
 -------------
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index daf5ca9242..ddbe34e276 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -3457,6 +3457,29 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
 	return 0;
 }
 
+int
+rte_eth_dev_conf_get(uint16_t port_id,
+				struct rte_eth_conf *dev_conf_info)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (dev_conf_info == NULL) {
+		RTE_ETHDEV_LOG(ERR,
+			"Cannot get ethdev port %u configuration info to NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
+	/* copy dev->data->dev_conf to dev_conf_info */
+	memcpy(dev_conf_info, &dev->data->dev_conf,
+		sizeof(struct rte_eth_conf));
+
+	return 0;
+}
+
 int
 rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
 				 uint32_t *ptypes, int num)
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index afdc53b674..2934b904ea 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -3082,6 +3082,27 @@ int rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr);
  */
 int rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Retrieve the configuration of an Ethernet device.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param dev_conf_info
+ *   A pointer to a structure of type *rte_eth_conf* to be filled with
+ *   the configuration of the Ethernet device.
+ *   And the memory of the structure should be allocated by the caller.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+__rte_experimental
+int rte_eth_dev_conf_get(uint16_t port_id,
+				struct rte_eth_conf *dev_conf_info);
+
 /**
  * Retrieve the firmware version of a device.
  *
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 904bce6ea1..06f1d8da48 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -247,6 +247,9 @@ EXPERIMENTAL {
 	rte_mtr_meter_policy_delete;
 	rte_mtr_meter_policy_update;
 	rte_mtr_meter_policy_validate;
+
+	# added in 21.11
+	rte_eth_dev_conf_get;
 };
 
 INTERNAL {
-- 
2.25.1

Re: [dpdk-dev] [PATCH v10 1/2] ethdev: add an API to get device configuration info

[Ferruh Yigit]

On 10/8/2021 4:41 AM, Jie Wang wrote:
> 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.
> 
> This patch adds a new API "rte_eth_dev_conf_get()" to help users
> get device configuration info.
> 

Not sure about "configuration info", what about just 'configuration'?
Is 'info' providing additional benefit?

> Add information about the new API in release notes.
> 
> Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> ---
>   doc/guides/rel_notes/release_21_11.rst |  5 +++++
>   lib/ethdev/rte_ethdev.c                | 23 +++++++++++++++++++++++
>   lib/ethdev/rte_ethdev.h                | 21 +++++++++++++++++++++
>   lib/ethdev/version.map                 |  3 +++
>   4 files changed, 52 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
> index f099b1cca2..c16cc83fd1 100644
> --- a/doc/guides/rel_notes/release_21_11.rst
> +++ b/doc/guides/rel_notes/release_21_11.rst
> @@ -129,6 +129,11 @@ New Features
>     * Added tests to validate packets hard expiry.
>     * Added tests to verify tunnel header verification in IPsec inbound.
>   
> +* **Added support for users get device configuration.**
> +
> +  * Added an API which can help users get device configuration.
> +    The declarations for the API's can be found in ``rte_ethdev.h``.
> +

Can you please clarify that support is for ethdev?
Also as location of the update in the file, can you please check the section comment,
on where to place this update?

>   
>   Removed Items
>   -------------
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index daf5ca9242..ddbe34e276 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -3457,6 +3457,29 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
>   	return 0;
>   }
>   
> +int
> +rte_eth_dev_conf_get(uint16_t port_id,
> +				struct rte_eth_conf *dev_conf_info)

Lines can be merged into single line.

And again what about 'dev_conf' as variable name, instead of 'dev_conf_info'?

> +{
> +	struct rte_eth_dev *dev;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +	dev = &rte_eth_devices[port_id];
> +
> +	if (dev_conf_info == NULL) {
> +		RTE_ETHDEV_LOG(ERR,
> +			"Cannot get ethdev port %u configuration info to NULL\n",

again, can we drop 'info'?

> +			port_id);
> +		return -EINVAL;
> +	}
> +
> +	/* copy dev->data->dev_conf to dev_conf_info */

This comment is not adding more info, almost same with line below.

> +	memcpy(dev_conf_info, &dev->data->dev_conf,
> +		sizeof(struct rte_eth_conf));

Can merge the lines?
  
> +
> +	return 0;
> +}
> +
>   int
>   rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
>   				 uint32_t *ptypes, int num)
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index afdc53b674..2934b904ea 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -3082,6 +3082,27 @@ int rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr);
>    */
>   int rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info);
>   
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Retrieve the configuration of an Ethernet device.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param dev_conf_info

what about dropping 'info' in the declaration too?

> + *   A pointer to a structure of type *rte_eth_conf* to be filled with
> + *   the configuration of the Ethernet device.
> + *   And the memory of the structure should be allocated by the caller.
> + * @return
> + *   - (0) if successful.
> + *   - (-ENODEV) if *port_id* invalid.
> + *   - (-EINVAL) if bad parameter.
> + */
> +__rte_experimental
> +int rte_eth_dev_conf_get(uint16_t port_id,
> +				struct rte_eth_conf *dev_conf_info);
> +

Can merge the lines?

>   /**
>    * Retrieve the firmware version of a device.
>    *
> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
> index 904bce6ea1..06f1d8da48 100644
> --- a/lib/ethdev/version.map
> +++ b/lib/ethdev/version.map
> @@ -247,6 +247,9 @@ EXPERIMENTAL {
>   	rte_mtr_meter_policy_delete;
>   	rte_mtr_meter_policy_update;
>   	rte_mtr_meter_policy_validate;
> +
> +	# added in 21.11
> +	rte_eth_dev_conf_get;
>   };
>   
>   INTERNAL {
> 

[dpdk-dev] [PATCH v10 2/2] app/testpmd: fix testpmd doesn't show RSS hash offload

[Jie Wang]

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.

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

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")

Signed-off-by: Jie Wang <jie1x.wang@intel.com>
---
 app/test-pmd/cmdline.c | 14 ++++++++++++--
 app/test-pmd/testpmd.c | 34 ++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h |  2 ++
 app/test-pmd/util.c    | 15 +++++++++++++++
 4 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index a9efd027c3..3ef8f99358 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -15997,6 +15997,7 @@ cmd_rx_offload_get_configuration_parsed(
 	struct rte_eth_dev_info dev_info;
 	portid_t port_id = res->port_id;
 	struct rte_port *port = &ports[port_id];
+	struct rte_eth_conf dev_conf_info;
 	uint64_t port_offloads;
 	uint64_t queue_offloads;
 	uint16_t nb_rx_queues;
@@ -16005,7 +16006,11 @@ cmd_rx_offload_get_configuration_parsed(
 
 	printf("Rx Offloading Configuration of port %d :\n", port_id);
 
-	port_offloads = port->dev_conf.rxmode.offloads;
+	ret = eth_dev_conf_info_get_print_err(port_id, &dev_conf_info);
+	if (ret != 0)
+		return;
+
+	port_offloads = dev_conf_info.rxmode.offloads;
 	printf("  Port :");
 	print_rx_offloads(port_offloads);
 	printf("\n");
@@ -16411,6 +16416,7 @@ cmd_tx_offload_get_configuration_parsed(
 	struct rte_eth_dev_info dev_info;
 	portid_t port_id = res->port_id;
 	struct rte_port *port = &ports[port_id];
+	struct rte_eth_conf dev_conf_info;
 	uint64_t port_offloads;
 	uint64_t queue_offloads;
 	uint16_t nb_tx_queues;
@@ -16419,7 +16425,11 @@ cmd_tx_offload_get_configuration_parsed(
 
 	printf("Tx Offloading Configuration of port %d :\n", port_id);
 
-	port_offloads = port->dev_conf.txmode.offloads;
+	ret = eth_dev_conf_info_get_print_err(port_id, &dev_conf_info);
+	if (ret != 0)
+		return;
+
+	port_offloads = dev_conf_info.txmode.offloads;
 	printf("  Port :");
 	print_tx_offloads(port_offloads);
 	printf("\n");
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 97ae52e17e..56e6e80739 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2531,6 +2531,9 @@ start_port(portid_t pid)
 		}
 
 		if (port->need_reconfig > 0) {
+			struct rte_eth_conf dev_conf_info;
+			int k;
+
 			port->need_reconfig = 0;
 
 			if (flow_isolate_all) {
@@ -2568,6 +2571,37 @@ start_port(portid_t pid)
 				port->need_reconfig = 1;
 				return -1;
 			}
+			/* get rte_eth_conf info */
+			if (0 !=
+				eth_dev_conf_info_get_print_err(pi,
+							&dev_conf_info)) {
+				fprintf(stderr,
+					"port %d can not get device configuration info\n",
+					pi);
+				return -1;
+			}
+			/* Apply Rx offloads configuration */
+			if (dev_conf_info.rxmode.offloads !=
+			    port->dev_conf.rxmode.offloads) {
+				port->dev_conf.rxmode.offloads |=
+					dev_conf_info.rxmode.offloads;
+				for (k = 0;
+				     k < port->dev_info.max_rx_queues;
+				     k++)
+					port->rx_conf[k].offloads |=
+						dev_conf_info.rxmode.offloads;
+			}
+			/* Apply Tx offloads configuration */
+			if (dev_conf_info.txmode.offloads !=
+			    port->dev_conf.txmode.offloads) {
+				port->dev_conf.txmode.offloads |=
+					dev_conf_info.txmode.offloads;
+				for (k = 0;
+				     k < port->dev_info.max_tx_queues;
+				     k++)
+					port->tx_conf[k].offloads |=
+						dev_conf_info.txmode.offloads;
+			}
 		}
 		if (port->need_reconfig_queues > 0 && is_proc_primary()) {
 			port->need_reconfig_queues = 0;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 5863b2f43f..48dad40986 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -960,6 +960,8 @@ void show_gro(portid_t port_id);
 void setup_gso(const char *mode, portid_t port_id);
 int eth_dev_info_get_print_err(uint16_t port_id,
 			struct rte_eth_dev_info *dev_info);
+int eth_dev_conf_info_get_print_err(uint16_t port_id,
+			struct rte_eth_conf *dev_conf_info);
 void eth_set_promisc_mode(uint16_t port_id, int enable);
 void eth_set_allmulticast_mode(uint16_t port, int enable);
 int eth_link_get_nowait_print_err(uint16_t port_id, struct rte_eth_link *link);
diff --git a/app/test-pmd/util.c b/app/test-pmd/util.c
index 14a9a251fb..e45ef9b2b3 100644
--- a/app/test-pmd/util.c
+++ b/app/test-pmd/util.c
@@ -444,6 +444,21 @@ eth_dev_info_get_print_err(uint16_t port_id,
 	return ret;
 }
 
+int
+eth_dev_conf_info_get_print_err(uint16_t port_id,
+				struct rte_eth_conf *dev_conf_info)
+{
+	int ret;
+
+	ret = rte_eth_dev_conf_get(port_id, dev_conf_info);
+	if (ret != 0)
+		fprintf(stderr,
+			"Error during getting device configuration (port %u) info: %s\n",
+			port_id, strerror(-ret));
+
+	return ret;
+}
+
 void
 eth_set_promisc_mode(uint16_t port, int enable)
 {
-- 
2.25.1

Re: [dpdk-dev] [PATCH v10 2/2] app/testpmd: fix testpmd doesn't show RSS hash offload

[Ferruh Yigit]

On 10/8/2021 4:41 AM, Jie Wang wrote:
> 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.
> 
> This patch updates the offloads info if it changes to fix this issue.
> 
> Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
> 
> Signed-off-by: Jie Wang <jie1x.wang@intel.com>

CI shows following tests are failing [1], can you please double check if it is a
real issue or infrastructure issue:

	| pf_smoke    | test_pf_tx_rx_queue | FAILED|
	| vf_smoke    | test_vf_tx_rx_queue | FAILED|


[1]
http://mails.dpdk.org/archives/test-report/2021-October/224962.html

[dpdk-dev] [PATCH v11 0/2] testpmd shows incorrect rx_offload configuration

[Jie Wang]

Launch testpmd with multiple queues, and check rx_offload info.

When testpmd shows the port configuration, it doesn't show RSS_HASH.

---
v11:
 - update the commit log.
 - rename the function and variable name.
v10:
 - update the commit log.
 - merge the first two patches.
 - rename the new API name.
v9:
 - add a release notes update for the new API.
 - update the description of the new API.
 - optimize the new API.
 - optimize the assignment of the offloads.
v8: delete "rte_exit" and just print error log.
v7:
 - delete struct "rte_eth_dev_conf_info", and reuse struct "rte_eth_conf".
 - add "__rte_experimental" to the new API "rte_eth_dev_conf_info_get" declaration.
v6: split this patch into two patches.
v5: add an API to get device configuration info.
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->dev_conf.rx/txmode".

Jie Wang (2):
  ethdev: add an API to get device configuration
  app/testpmd: fix testpmd doesn't show RSS hash offload

 app/test-pmd/cmdline.c                 | 14 +++++++++--
 app/test-pmd/testpmd.c                 | 33 ++++++++++++++++++++++++++
 app/test-pmd/testpmd.h                 |  2 ++
 app/test-pmd/util.c                    | 14 +++++++++++
 doc/guides/rel_notes/release_21_11.rst |  5 ++++
 lib/ethdev/rte_ethdev.c                | 20 ++++++++++++++++
 lib/ethdev/rte_ethdev.h                | 20 ++++++++++++++++
 lib/ethdev/version.map                 |  3 +++
 8 files changed, 109 insertions(+), 2 deletions(-)

-- 
2.25.1

[dpdk-dev] [PATCH v11 1/2] ethdev: add an API to get device configuration

[Jie Wang]

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.

This patch adds a new API "rte_eth_dev_conf_get()" to help users
get device configuration.

Add information about the new API in release notes.

Signed-off-by: Jie Wang <jie1x.wang@intel.com>
---
 doc/guides/rel_notes/release_21_11.rst |  5 +++++
 lib/ethdev/rte_ethdev.c                | 20 ++++++++++++++++++++
 lib/ethdev/rte_ethdev.h                | 20 ++++++++++++++++++++
 lib/ethdev/version.map                 |  3 +++
 4 files changed, 48 insertions(+)

diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
index 93082723cf..789fc2d4ba 100644
--- a/doc/guides/rel_notes/release_21_11.rst
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -67,6 +67,11 @@ New Features
   Added macros ETH_RSS_IPV4_CHKSUM and ETH_RSS_L4_CHKSUM, now IPv4 and
   TCP/UDP/SCTP header checksum field can be used as input set for RSS.
 
+* **Added support for users get device configuration in ethdev.**
+
+  Added an ethdev API which can help users get device configuration.
+  The declarations for the API's can be found in ``rte_ethdev.h``.
+
 * **Updated Broadcom bnxt PMD.**
 
   * Added flow offload support for Thor.
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 028907bc4b..be3ceb8131 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -3437,6 +3437,26 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
 	return 0;
 }
 
+int
+rte_eth_dev_conf_get(uint16_t port_id, struct rte_eth_conf *dev_conf)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (dev_conf == NULL) {
+		RTE_ETHDEV_LOG(ERR,
+			"Cannot get ethdev port %u configuration to NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
+	memcpy(dev_conf, &dev->data->dev_conf, sizeof(struct rte_eth_conf));
+
+	return 0;
+}
+
 int
 rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
 				 uint32_t *ptypes, int num)
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 6d80514ba7..f42f950fea 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -3052,6 +3052,26 @@ int rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr);
  */
 int rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Retrieve the configuration of an Ethernet device.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param dev_conf
+ *   A pointer to a structure of type *rte_eth_conf* to be filled with
+ *   the configuration of the Ethernet device.
+ *   And the memory of the structure should be allocated by the caller.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+__rte_experimental
+int rte_eth_dev_conf_get(uint16_t port_id, struct rte_eth_conf *dev_conf);
+
 /**
  * Retrieve the firmware version of a device.
  *
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index efd729c0f2..c516837723 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -245,6 +245,9 @@ EXPERIMENTAL {
 	rte_mtr_meter_policy_delete;
 	rte_mtr_meter_policy_update;
 	rte_mtr_meter_policy_validate;
+
+	# added in 21.11
+	rte_eth_dev_conf_get;
 };
 
 INTERNAL {
-- 
2.25.1

Re: [dpdk-dev] [PATCH v11 1/2] ethdev: add an API to get device configuration

[Somnath Kotur]

[-- Attachment #1: Type: text/plain, Size: 4061 bytes --]

On Mon, Oct 11, 2021 at 3:34 PM Jie Wang <jie1x.wang@intel.com> wrote:
>
> 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.
>
> This patch adds a new API "rte_eth_dev_conf_get()" to help users
> get device configuration.
>
> Add information about the new API in release notes.
>
> Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> ---
>  doc/guides/rel_notes/release_21_11.rst |  5 +++++
>  lib/ethdev/rte_ethdev.c                | 20 ++++++++++++++++++++
>  lib/ethdev/rte_ethdev.h                | 20 ++++++++++++++++++++
>  lib/ethdev/version.map                 |  3 +++
>  4 files changed, 48 insertions(+)
>
> diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
> index 93082723cf..789fc2d4ba 100644
> --- a/doc/guides/rel_notes/release_21_11.rst
> +++ b/doc/guides/rel_notes/release_21_11.rst
> @@ -67,6 +67,11 @@ New Features
>    Added macros ETH_RSS_IPV4_CHKSUM and ETH_RSS_L4_CHKSUM, now IPv4 and
>    TCP/UDP/SCTP header checksum field can be used as input set for RSS.
>
> +* **Added support for users get device configuration in ethdev.**
> +
> +  Added an ethdev API which can help users get device configuration.
> +  The declarations for the API's can be found in ``rte_ethdev.h``.
`declaration for the API ` right?
> +
>  * **Updated Broadcom bnxt PMD.**
>
>    * Added flow offload support for Thor.
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index 028907bc4b..be3ceb8131 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -3437,6 +3437,26 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
>         return 0;
>  }
>
> +int
> +rte_eth_dev_conf_get(uint16_t port_id, struct rte_eth_conf *dev_conf)
> +{
> +       struct rte_eth_dev *dev;
> +
> +       RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +       dev = &rte_eth_devices[port_id];
> +
> +       if (dev_conf == NULL) {
> +               RTE_ETHDEV_LOG(ERR,
> +                       "Cannot get ethdev port %u configuration to NULL\n",
> +                       port_id);
> +               return -EINVAL;
> +       }
> +
> +       memcpy(dev_conf, &dev->data->dev_conf, sizeof(struct rte_eth_conf));
> +
> +       return 0;
> +}
> +
>  int
>  rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
>                                  uint32_t *ptypes, int num)
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 6d80514ba7..f42f950fea 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -3052,6 +3052,26 @@ int rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr);
>   */
>  int rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info);
>
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Retrieve the configuration of an Ethernet device.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param dev_conf
> + *   A pointer to a structure of type *rte_eth_conf* to be filled with
> + *   the configuration of the Ethernet device.
> + *   And the memory of the structure should be allocated by the caller.
> + * @return
> + *   - (0) if successful.
> + *   - (-ENODEV) if *port_id* invalid.
> + *   - (-EINVAL) if bad parameter.
> + */
> +__rte_experimental
> +int rte_eth_dev_conf_get(uint16_t port_id, struct rte_eth_conf *dev_conf);
> +
>  /**
>   * Retrieve the firmware version of a device.
>   *
> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
> index efd729c0f2..c516837723 100644
> --- a/lib/ethdev/version.map
> +++ b/lib/ethdev/version.map
> @@ -245,6 +245,9 @@ EXPERIMENTAL {
>         rte_mtr_meter_policy_delete;
>         rte_mtr_meter_policy_update;
>         rte_mtr_meter_policy_validate;
> +
> +       # added in 21.11
> +       rte_eth_dev_conf_get;
>  };
>
>  INTERNAL {
> --
Acked-by: Somnath kotur <somnath.kotur@broadcom.com>
> 2.25.1
>

Re: [dpdk-dev] [PATCH v11 1/2] ethdev: add an API to get device configuration

[Andrew Rybchenko]

On 10/11/21 9:01 PM, Jie Wang wrote:
> The driver may change offloads info into dev->data->dev_conf
> in dev_configure which may cause port->dev_conf and port->rx_conf

port->dev_conf does not make sense outside of testpmd.
So, it should not be mentioned in the description here.

> contain outdated values.
> 
> This patch adds a new API "rte_eth_dev_conf_get()" to help users
> get device configuration.

May be:
Add a new API to get actual device configuration.

> 
> Add information about the new API in release notes.

It should not be in the changeset description.

> 
> Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> ---
>  doc/guides/rel_notes/release_21_11.rst |  5 +++++
>  lib/ethdev/rte_ethdev.c                | 20 ++++++++++++++++++++
>  lib/ethdev/rte_ethdev.h                | 20 ++++++++++++++++++++
>  lib/ethdev/version.map                 |  3 +++
>  4 files changed, 48 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
> index 93082723cf..789fc2d4ba 100644
> --- a/doc/guides/rel_notes/release_21_11.rst
> +++ b/doc/guides/rel_notes/release_21_11.rst
> @@ -67,6 +67,11 @@ New Features
>    Added macros ETH_RSS_IPV4_CHKSUM and ETH_RSS_L4_CHKSUM, now IPv4 and
>    TCP/UDP/SCTP header checksum field can be used as input set for RSS.
>  
> +* **Added support for users get device configuration in ethdev.**
> +
> +  Added an ethdev API which can help users get device configuration.
> +  The declarations for the API's can be found in ``rte_ethdev.h``.

The second line is not necessary since it is a well-known
location of the ethdev API declarations.

> +
>  * **Updated Broadcom bnxt PMD.**
>  
>    * Added flow offload support for Thor.
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index 028907bc4b..be3ceb8131 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -3437,6 +3437,26 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
>  	return 0;
>  }
>  
> +int
> +rte_eth_dev_conf_get(uint16_t port_id, struct rte_eth_conf *dev_conf)
> +{
> +	struct rte_eth_dev *dev;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +	dev = &rte_eth_devices[port_id];
> +
> +	if (dev_conf == NULL) {
> +		RTE_ETHDEV_LOG(ERR,
> +			"Cannot get ethdev port %u configuration to NULL\n",
> +			port_id);
> +		return -EINVAL;
> +	}
> +
> +	memcpy(dev_conf, &dev->data->dev_conf, sizeof(struct rte_eth_conf));
> +
> +	return 0;
> +}
> +
>  int
>  rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
>  				 uint32_t *ptypes, int num)
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 6d80514ba7..f42f950fea 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -3052,6 +3052,26 @@ int rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr);
>   */
>  int rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info);
>  
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Retrieve the configuration of an Ethernet device.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param dev_conf
> + *   A pointer to a structure of type *rte_eth_conf* to be filled with

I think there is no point to duplicate type in the parameter
description.

> + *   the configuration of the Ethernet device.
> + *   And the memory of the structure should be allocated by the caller.

May be just:
Location for Ethernet device configuration to be filled in.

> + * @return
> + *   - (0) if successful.
> + *   - (-ENODEV) if *port_id* invalid.
> + *   - (-EINVAL) if bad parameter.
> + */
> +__rte_experimental
> +int rte_eth_dev_conf_get(uint16_t port_id, struct rte_eth_conf *dev_conf);
> +
>  /**
>   * Retrieve the firmware version of a device.
>   *
> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
> index efd729c0f2..c516837723 100644
> --- a/lib/ethdev/version.map
> +++ b/lib/ethdev/version.map
> @@ -245,6 +245,9 @@ EXPERIMENTAL {
>  	rte_mtr_meter_policy_delete;
>  	rte_mtr_meter_policy_update;
>  	rte_mtr_meter_policy_validate;
> +
> +	# added in 21.11
> +	rte_eth_dev_conf_get;
>  };
>  
>  INTERNAL {
> 

[dpdk-dev] [PATCH v11 2/2] app/testpmd: fix testpmd doesn't show RSS hash offload

[Jie Wang]

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.

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

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")

Signed-off-by: Jie Wang <jie1x.wang@intel.com>
---
 app/test-pmd/cmdline.c | 14 ++++++++++++--
 app/test-pmd/testpmd.c | 33 +++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h |  2 ++
 app/test-pmd/util.c    | 14 ++++++++++++++
 4 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 826256b0b3..514a2d6f35 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -15717,6 +15717,7 @@ cmd_rx_offload_get_configuration_parsed(
 	struct rte_eth_dev_info dev_info;
 	portid_t port_id = res->port_id;
 	struct rte_port *port = &ports[port_id];
+	struct rte_eth_conf dev_conf;
 	uint64_t port_offloads;
 	uint64_t queue_offloads;
 	uint16_t nb_rx_queues;
@@ -15725,7 +15726,11 @@ cmd_rx_offload_get_configuration_parsed(
 
 	printf("Rx Offloading Configuration of port %d :\n", port_id);
 
-	port_offloads = port->dev_conf.rxmode.offloads;
+	ret = eth_dev_conf_get_print_err(port_id, &dev_conf);
+	if (ret != 0)
+		return;
+
+	port_offloads = dev_conf.rxmode.offloads;
 	printf("  Port :");
 	print_rx_offloads(port_offloads);
 	printf("\n");
@@ -16131,6 +16136,7 @@ cmd_tx_offload_get_configuration_parsed(
 	struct rte_eth_dev_info dev_info;
 	portid_t port_id = res->port_id;
 	struct rte_port *port = &ports[port_id];
+	struct rte_eth_conf dev_conf;
 	uint64_t port_offloads;
 	uint64_t queue_offloads;
 	uint16_t nb_tx_queues;
@@ -16139,7 +16145,11 @@ cmd_tx_offload_get_configuration_parsed(
 
 	printf("Tx Offloading Configuration of port %d :\n", port_id);
 
-	port_offloads = port->dev_conf.txmode.offloads;
+	ret = eth_dev_conf_get_print_err(port_id, &dev_conf);
+	if (ret != 0)
+		return;
+
+	port_offloads = dev_conf.txmode.offloads;
 	printf("  Port :");
 	print_tx_offloads(port_offloads);
 	printf("\n");
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 97ae52e17e..d7f463df51 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2531,6 +2531,9 @@ start_port(portid_t pid)
 		}
 
 		if (port->need_reconfig > 0) {
+			struct rte_eth_conf dev_conf;
+			int k;
+
 			port->need_reconfig = 0;
 
 			if (flow_isolate_all) {
@@ -2568,6 +2571,36 @@ start_port(portid_t pid)
 				port->need_reconfig = 1;
 				return -1;
 			}
+			/* get device configuration*/
+			if (0 !=
+				eth_dev_conf_get_print_err(pi, &dev_conf)) {
+				fprintf(stderr,
+					"port %d can not get device configuration\n",
+					pi);
+				return -1;
+			}
+			/* Apply Rx offloads configuration */
+			if (dev_conf.rxmode.offloads !=
+			    port->dev_conf.rxmode.offloads) {
+				port->dev_conf.rxmode.offloads |=
+					dev_conf.rxmode.offloads;
+				for (k = 0;
+				     k < port->dev_info.max_rx_queues;
+				     k++)
+					port->rx_conf[k].offloads |=
+						dev_conf.rxmode.offloads;
+			}
+			/* Apply Tx offloads configuration */
+			if (dev_conf.txmode.offloads !=
+			    port->dev_conf.txmode.offloads) {
+				port->dev_conf.txmode.offloads |=
+					dev_conf.txmode.offloads;
+				for (k = 0;
+				     k < port->dev_info.max_tx_queues;
+				     k++)
+					port->tx_conf[k].offloads |=
+						dev_conf.txmode.offloads;
+			}
 		}
 		if (port->need_reconfig_queues > 0 && is_proc_primary()) {
 			port->need_reconfig_queues = 0;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 5863b2f43f..45a79f9085 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -960,6 +960,8 @@ void show_gro(portid_t port_id);
 void setup_gso(const char *mode, portid_t port_id);
 int eth_dev_info_get_print_err(uint16_t port_id,
 			struct rte_eth_dev_info *dev_info);
+int eth_dev_conf_get_print_err(uint16_t port_id,
+			struct rte_eth_conf *dev_conf);
 void eth_set_promisc_mode(uint16_t port_id, int enable);
 void eth_set_allmulticast_mode(uint16_t port, int enable);
 int eth_link_get_nowait_print_err(uint16_t port_id, struct rte_eth_link *link);
diff --git a/app/test-pmd/util.c b/app/test-pmd/util.c
index 14a9a251fb..55c07e1b82 100644
--- a/app/test-pmd/util.c
+++ b/app/test-pmd/util.c
@@ -444,6 +444,20 @@ eth_dev_info_get_print_err(uint16_t port_id,
 	return ret;
 }
 
+int
+eth_dev_conf_get_print_err(uint16_t port_id, struct rte_eth_conf *dev_conf)
+{
+	int ret;
+
+	ret = rte_eth_dev_conf_get(port_id, dev_conf);
+	if (ret != 0)
+		fprintf(stderr,
+			"Error during getting device configuration (port %u): %s\n",
+			port_id, strerror(-ret));
+
+	return ret;
+}
+
 void
 eth_set_promisc_mode(uint16_t port, int enable)
 {
-- 
2.25.1

[dpdk-dev] [PATCH v12 0/2] testpmd shows incorrect rx_offload configuration

[Jie Wang]

Launch testpmd with multiple queues, and check rx_offload info.

When testpmd shows the port configuration, it doesn't show RSS_HASH.

---
v12: update the commit log and the API comment.
v11:
 - update the commit log.
 - rename the function and variable name.
v10:
 - update the commit log.
 - merge the first two patches.
 - rename the new API name.
v9:
 - add a release notes update for the new API.
 - update the description of the new API.
 - optimize the new API.
 - optimize the assignment of the offloads.
v8: delete "rte_exit" and just print error log.
v7:
 - delete struct "rte_eth_dev_conf_info", and reuse struct "rte_eth_conf".
 - add "__rte_experimental" to the new API "rte_eth_dev_conf_info_get" declaration.
v6: split this patch into two patches.
v5: add an API to get device configuration info.
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->dev_conf.rx/txmode".

Jie Wang (2):
  ethdev: add an API to get device configuration
  app/testpmd: fix testpmd doesn't show RSS hash offload

 app/test-pmd/cmdline.c                 | 14 +++++++++--
 app/test-pmd/testpmd.c                 | 33 ++++++++++++++++++++++++++
 app/test-pmd/testpmd.h                 |  2 ++
 app/test-pmd/util.c                    | 14 +++++++++++
 doc/guides/rel_notes/release_21_11.rst |  4 ++++
 lib/ethdev/rte_ethdev.c                | 20 ++++++++++++++++
 lib/ethdev/rte_ethdev.h                | 20 ++++++++++++++++
 lib/ethdev/version.map                 |  3 +++
 8 files changed, 108 insertions(+), 2 deletions(-)

-- 
2.25.1

[dpdk-dev] [PATCH v12 1/2] ethdev: add an API to get device configuration

[Jie Wang]

The driver may change offloads info into dev->data->dev_conf
in dev_configure which may cause apps use outdated values.

Add a new API to get actual device configuration.

Signed-off-by: Jie Wang <jie1x.wang@intel.com>
---
 doc/guides/rel_notes/release_21_11.rst |  4 ++++
 lib/ethdev/rte_ethdev.c                | 20 ++++++++++++++++++++
 lib/ethdev/rte_ethdev.h                | 20 ++++++++++++++++++++
 lib/ethdev/version.map                 |  3 +++
 4 files changed, 47 insertions(+)

diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
index 93082723cf..c02c5ddf6e 100644
--- a/doc/guides/rel_notes/release_21_11.rst
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -67,6 +67,10 @@ New Features
   Added macros ETH_RSS_IPV4_CHKSUM and ETH_RSS_L4_CHKSUM, now IPv4 and
   TCP/UDP/SCTP header checksum field can be used as input set for RSS.
 
+* **Added support for users get device configuration in ethdev.**
+
+  Added an ethdev API which can help users get device configuration.
+
 * **Updated Broadcom bnxt PMD.**
 
   * Added flow offload support for Thor.
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 028907bc4b..be3ceb8131 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -3437,6 +3437,26 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
 	return 0;
 }
 
+int
+rte_eth_dev_conf_get(uint16_t port_id, struct rte_eth_conf *dev_conf)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (dev_conf == NULL) {
+		RTE_ETHDEV_LOG(ERR,
+			"Cannot get ethdev port %u configuration to NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
+	memcpy(dev_conf, &dev->data->dev_conf, sizeof(struct rte_eth_conf));
+
+	return 0;
+}
+
 int
 rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
 				 uint32_t *ptypes, int num)
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 6d80514ba7..7b5d5fb379 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -3052,6 +3052,26 @@ int rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr);
  */
 int rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Retrieve the configuration of an Ethernet device.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param dev_conf
+ *   A pointer to a structure to be filled with the configuration of
+ *   the Ethernet device.
+ *   Location for Ethernet device configuration to be filled in.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+__rte_experimental
+int rte_eth_dev_conf_get(uint16_t port_id, struct rte_eth_conf *dev_conf);
+
 /**
  * Retrieve the firmware version of a device.
  *
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index efd729c0f2..c516837723 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -245,6 +245,9 @@ EXPERIMENTAL {
 	rte_mtr_meter_policy_delete;
 	rte_mtr_meter_policy_update;
 	rte_mtr_meter_policy_validate;
+
+	# added in 21.11
+	rte_eth_dev_conf_get;
 };
 
 INTERNAL {
-- 
2.25.1

Re: [dpdk-dev] [PATCH v12 1/2] ethdev: add an API to get device configuration

[Andrew Rybchenko]

On 10/12/21 5:54 AM, Jie Wang wrote:
> The driver may change offloads info into dev->data->dev_conf
> in dev_configure which may cause apps use outdated values.
> 
> Add a new API to get actual device configuration.
> 
> Signed-off-by: Jie Wang <jie1x.wang@intel.com>

Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

[snip]

> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 6d80514ba7..7b5d5fb379 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -3052,6 +3052,26 @@ int rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr);
>   */
>  int rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info);
>  
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Retrieve the configuration of an Ethernet device.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param dev_conf
> + *   A pointer to a structure to be filled with the configuration of
> + *   the Ethernet device.
> + *   Location for Ethernet device configuration to be filled in.

IMHO just the second sentence is sufficient.

> + * @return
> + *   - (0) if successful.
> + *   - (-ENODEV) if *port_id* invalid.
> + *   - (-EINVAL) if bad parameter.
> + */
> +__rte_experimental
> +int rte_eth_dev_conf_get(uint16_t port_id, struct rte_eth_conf *dev_conf);
> +
>  /**
>   * Retrieve the firmware version of a device.
>   *

[snip]

[dpdk-dev] [PATCH v12 2/2] app/testpmd: fix testpmd doesn't show RSS hash offload

[Jie Wang]

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.

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

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")

Signed-off-by: Jie Wang <jie1x.wang@intel.com>
---
 app/test-pmd/cmdline.c | 14 ++++++++++++--
 app/test-pmd/testpmd.c | 33 +++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h |  2 ++
 app/test-pmd/util.c    | 14 ++++++++++++++
 4 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 826256b0b3..514a2d6f35 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -15717,6 +15717,7 @@ cmd_rx_offload_get_configuration_parsed(
 	struct rte_eth_dev_info dev_info;
 	portid_t port_id = res->port_id;
 	struct rte_port *port = &ports[port_id];
+	struct rte_eth_conf dev_conf;
 	uint64_t port_offloads;
 	uint64_t queue_offloads;
 	uint16_t nb_rx_queues;
@@ -15725,7 +15726,11 @@ cmd_rx_offload_get_configuration_parsed(
 
 	printf("Rx Offloading Configuration of port %d :\n", port_id);
 
-	port_offloads = port->dev_conf.rxmode.offloads;
+	ret = eth_dev_conf_get_print_err(port_id, &dev_conf);
+	if (ret != 0)
+		return;
+
+	port_offloads = dev_conf.rxmode.offloads;
 	printf("  Port :");
 	print_rx_offloads(port_offloads);
 	printf("\n");
@@ -16131,6 +16136,7 @@ cmd_tx_offload_get_configuration_parsed(
 	struct rte_eth_dev_info dev_info;
 	portid_t port_id = res->port_id;
 	struct rte_port *port = &ports[port_id];
+	struct rte_eth_conf dev_conf;
 	uint64_t port_offloads;
 	uint64_t queue_offloads;
 	uint16_t nb_tx_queues;
@@ -16139,7 +16145,11 @@ cmd_tx_offload_get_configuration_parsed(
 
 	printf("Tx Offloading Configuration of port %d :\n", port_id);
 
-	port_offloads = port->dev_conf.txmode.offloads;
+	ret = eth_dev_conf_get_print_err(port_id, &dev_conf);
+	if (ret != 0)
+		return;
+
+	port_offloads = dev_conf.txmode.offloads;
 	printf("  Port :");
 	print_tx_offloads(port_offloads);
 	printf("\n");
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 97ae52e17e..d7f463df51 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2531,6 +2531,9 @@ start_port(portid_t pid)
 		}
 
 		if (port->need_reconfig > 0) {
+			struct rte_eth_conf dev_conf;
+			int k;
+
 			port->need_reconfig = 0;
 
 			if (flow_isolate_all) {
@@ -2568,6 +2571,36 @@ start_port(portid_t pid)
 				port->need_reconfig = 1;
 				return -1;
 			}
+			/* get device configuration*/
+			if (0 !=
+				eth_dev_conf_get_print_err(pi, &dev_conf)) {
+				fprintf(stderr,
+					"port %d can not get device configuration\n",
+					pi);
+				return -1;
+			}
+			/* Apply Rx offloads configuration */
+			if (dev_conf.rxmode.offloads !=
+			    port->dev_conf.rxmode.offloads) {
+				port->dev_conf.rxmode.offloads |=
+					dev_conf.rxmode.offloads;
+				for (k = 0;
+				     k < port->dev_info.max_rx_queues;
+				     k++)
+					port->rx_conf[k].offloads |=
+						dev_conf.rxmode.offloads;
+			}
+			/* Apply Tx offloads configuration */
+			if (dev_conf.txmode.offloads !=
+			    port->dev_conf.txmode.offloads) {
+				port->dev_conf.txmode.offloads |=
+					dev_conf.txmode.offloads;
+				for (k = 0;
+				     k < port->dev_info.max_tx_queues;
+				     k++)
+					port->tx_conf[k].offloads |=
+						dev_conf.txmode.offloads;
+			}
 		}
 		if (port->need_reconfig_queues > 0 && is_proc_primary()) {
 			port->need_reconfig_queues = 0;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 5863b2f43f..45a79f9085 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -960,6 +960,8 @@ void show_gro(portid_t port_id);
 void setup_gso(const char *mode, portid_t port_id);
 int eth_dev_info_get_print_err(uint16_t port_id,
 			struct rte_eth_dev_info *dev_info);
+int eth_dev_conf_get_print_err(uint16_t port_id,
+			struct rte_eth_conf *dev_conf);
 void eth_set_promisc_mode(uint16_t port_id, int enable);
 void eth_set_allmulticast_mode(uint16_t port, int enable);
 int eth_link_get_nowait_print_err(uint16_t port_id, struct rte_eth_link *link);
diff --git a/app/test-pmd/util.c b/app/test-pmd/util.c
index 14a9a251fb..55c07e1b82 100644
--- a/app/test-pmd/util.c
+++ b/app/test-pmd/util.c
@@ -444,6 +444,20 @@ eth_dev_info_get_print_err(uint16_t port_id,
 	return ret;
 }
 
+int
+eth_dev_conf_get_print_err(uint16_t port_id, struct rte_eth_conf *dev_conf)
+{
+	int ret;
+
+	ret = rte_eth_dev_conf_get(port_id, dev_conf);
+	if (ret != 0)
+		fprintf(stderr,
+			"Error during getting device configuration (port %u): %s\n",
+			port_id, strerror(-ret));
+
+	return ret;
+}
+
 void
 eth_set_promisc_mode(uint16_t port, int enable)
 {
-- 
2.25.1

Re: [dpdk-dev] [PATCH v12 2/2] app/testpmd: fix testpmd doesn't show RSS hash offload

[Ferruh Yigit]

On 10/12/2021 3:54 AM, Jie Wang wrote:
> 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.
> 
> This patch updates the offloads info if it changes to fix this issue.
> 
> Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
> 
> Signed-off-by: Jie Wang <jie1x.wang@intel.com>

Hi Jie,

ci/intel-Testing is still reporting some errors [1] with this patch,
did you able to investigate the reason of it?

This error seems not seen in other patches, so that shouldn't be an
infrastructure problem.


[1]
http://mails.dpdk.org/archives/test-report/2021-October/226766.html

Re: [dpdk-dev] [PATCH v12 2/2] app/testpmd: fix testpmd doesn't show RSS hash offload

[Wang, Jie1X]

> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Tuesday, October 12, 2021 10:38 PM
> To: Wang, Jie1X <jie1x.wang@intel.com>; dev@dpdk.org
> Cc: thomas@monjalon.net; andrew.rybchenko@oktetlabs.ru; Li, Xiaoyun
> <xiaoyun.li@intel.com>; Yang, SteveX <stevex.yang@intel.com>
> Subject: Re: [PATCH v12 2/2] app/testpmd: fix testpmd doesn't show RSS hash
> offload
> 
> On 10/12/2021 3:54 AM, Jie Wang wrote:
> > 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.
> >
> > This patch updates the offloads info if it changes to fix this issue.
> >
> > Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
> >
> > Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> 
> Hi Jie,
> 
> ci/intel-Testing is still reporting some errors [1] with this patch, did you able to
> investigate the reason of it?
> 
> This error seems not seen in other patches, so that shouldn't be an infrastructure
> problem.
> 
> 
> [1]
> http://mails.dpdk.org/archives/test-report/2021-October/226766.html

Hi Ferruh,

When the dpdk runs the test_pf_tx_rx_queue test case, the port will be configured with rxq/txq to 1. This will cause the test to fail.

In rte_eth_dev_configure, if rxq/txq is 1, dev_conf->rxmode.mq_mode is ETH_MQ_RX_NONE, and  dev_conf->rxmode.offloads contains the DEV_RX_OFFLOAD_RSS_HASH value by default, which causes the program to report an error " Ethdev port_id=0 config invalid Rx mq_mode without RSS but RSS_HASH offload is requested".

When the port needs to configure txq/txq to 1, first clear the RSS_HASH configuration of dev_conf->rxmode.offloads.

And I have talked about this error with the tester a few days ago, and they will modify the test case. They will add a command (port config port_id rx_offload rss_hash off) before configuring txq/rxq to 1.

Re: [dpdk-dev] [PATCH v12 2/2] app/testpmd: fix testpmd doesn't show RSS hash offload

[Ferruh Yigit]

On 10/13/2021 3:42 AM, Wang, Jie1X wrote:
> 
> 
>> -----Original Message-----
>> From: Yigit, Ferruh <ferruh.yigit@intel.com>
>> Sent: Tuesday, October 12, 2021 10:38 PM
>> To: Wang, Jie1X <jie1x.wang@intel.com>; dev@dpdk.org
>> Cc: thomas@monjalon.net; andrew.rybchenko@oktetlabs.ru; Li, Xiaoyun
>> <xiaoyun.li@intel.com>; Yang, SteveX <stevex.yang@intel.com>
>> Subject: Re: [PATCH v12 2/2] app/testpmd: fix testpmd doesn't show RSS hash
>> offload
>>
>> On 10/12/2021 3:54 AM, Jie Wang wrote:
>>> 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.
>>>
>>> This patch updates the offloads info if it changes to fix this issue.
>>>
>>> Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
>>>
>>> Signed-off-by: Jie Wang <jie1x.wang@intel.com>
>>
>> Hi Jie,
>>
>> ci/intel-Testing is still reporting some errors [1] with this patch, did you able to
>> investigate the reason of it?
>>
>> This error seems not seen in other patches, so that shouldn't be an infrastructure
>> problem.
>>
>>
>> [1]
>> http://mails.dpdk.org/archives/test-report/2021-October/226766.html
> 
> Hi Ferruh,
> 
> When the dpdk runs the test_pf_tx_rx_queue test case, the port will be configured with rxq/txq to 1. This will cause the test to fail.
> 
> In rte_eth_dev_configure, if rxq/txq is 1, dev_conf->rxmode.mq_mode is ETH_MQ_RX_NONE, and  dev_conf->rxmode.offloads contains the DEV_RX_OFFLOAD_RSS_HASH value by default, which causes the program to report an error " Ethdev port_id=0 config invalid Rx mq_mode without RSS but RSS_HASH offload is requested".
> 
> When the port needs to configure txq/txq to 1, first clear the RSS_HASH configuration of dev_conf->rxmode.offloads.
> 
> And I have talked about this error with the tester a few days ago, and they will modify the test case. They will add a command (port config port_id rx_offload rss_hash off) before configuring txq/rxq to 1.
> 

I see, but shouldn't this fail for all patches, why it is only failing
for this set?

Re: [dpdk-dev] [PATCH v12 2/2] app/testpmd: fix testpmd doesn't show RSS hash offload

[Ferruh Yigit]

On 10/13/2021 9:50 AM, Ferruh Yigit wrote:
> On 10/13/2021 3:42 AM, Wang, Jie1X wrote:
>>
>>
>>> -----Original Message-----
>>> From: Yigit, Ferruh <ferruh.yigit@intel.com>
>>> Sent: Tuesday, October 12, 2021 10:38 PM
>>> To: Wang, Jie1X <jie1x.wang@intel.com>; dev@dpdk.org
>>> Cc: thomas@monjalon.net; andrew.rybchenko@oktetlabs.ru; Li, Xiaoyun
>>> <xiaoyun.li@intel.com>; Yang, SteveX <stevex.yang@intel.com>
>>> Subject: Re: [PATCH v12 2/2] app/testpmd: fix testpmd doesn't show RSS hash
>>> offload
>>>
>>> On 10/12/2021 3:54 AM, Jie Wang wrote:
>>>> 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.
>>>>
>>>> This patch updates the offloads info if it changes to fix this issue.
>>>>
>>>> Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
>>>>
>>>> Signed-off-by: Jie Wang <jie1x.wang@intel.com>
>>>
>>> Hi Jie,
>>>
>>> ci/intel-Testing is still reporting some errors [1] with this patch, did you able to
>>> investigate the reason of it?
>>>
>>> This error seems not seen in other patches, so that shouldn't be an infrastructure
>>> problem.
>>>
>>>
>>> [1]
>>> http://mails.dpdk.org/archives/test-report/2021-October/226766.html
>>
>> Hi Ferruh,
>>
>> When the dpdk runs the test_pf_tx_rx_queue test case, the port will be configured with rxq/txq to 1. This will cause the test to fail.
>>
>> In rte_eth_dev_configure, if rxq/txq is 1, dev_conf->rxmode.mq_mode is ETH_MQ_RX_NONE, and  dev_conf->rxmode.offloads contains the DEV_RX_OFFLOAD_RSS_HASH value by default, which causes the program to report an error " Ethdev port_id=0 config invalid Rx mq_mode without RSS but RSS_HASH offload is requested".
>>
>> When the port needs to configure txq/txq to 1, first clear the RSS_HASH configuration of dev_conf->rxmode.offloads.
>>
>> And I have talked about this error with the tester a few days ago, and they will modify the test case. They will add a command (port config port_id rx_offload rss_hash off) before configuring txq/rxq to 1.
>>
> 
> I see, but shouldn't this fail for all patches, why it is only failing
> for this set?

Got it now, in first config some PMDs force enables RSS_HASH, when testpmd disables
the RSS later, the existing RSS_HASH request cause failure in ethdev layer.

What about disable the RSS_HASH in application level, when RSS is disables, something
like following:

  diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
  index a7841c557f99..5f6daac8a616 100644
  --- a/app/test-pmd/testpmd.c
  +++ b/app/test-pmd/testpmd.c
  @@ -3601,12 +3601,15 @@ init_port_config(void)
                  }
   
                  if (port->dcb_flag == 0) {
  -                       if( port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0)
  +                       if( port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0) {
                                 port->dev_conf.rxmode.mq_mode =
                                          (enum rte_eth_rx_mq_mode)
                                                  (rx_mq_mode & ETH_MQ_RX_RSS);
  -                       else
  +                       } else {
                                  port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_NONE;
  +                               port->dev_conf.rxmode.offloads &=
  +                                       ~DEV_RX_OFFLOAD_RSS_HASH;
  +                       }
                  }
   
                  rxtx_port_config(port);

Re: [dpdk-dev] [PATCH v12 0/2] testpmd shows incorrect rx_offload configuration

[Ferruh Yigit]

On 10/12/2021 3:54 AM, Jie Wang wrote:
> Launch testpmd with multiple queues, and check rx_offload info.
> 
> When testpmd shows the port configuration, it doesn't show RSS_HASH.
> 
> ---
> v12: update the commit log and the API comment.
> v11:
>   - update the commit log.
>   - rename the function and variable name.
> v10:
>   - update the commit log.
>   - merge the first two patches.
>   - rename the new API name.
> v9:
>   - add a release notes update for the new API.
>   - update the description of the new API.
>   - optimize the new API.
>   - optimize the assignment of the offloads.
> v8: delete "rte_exit" and just print error log.
> v7:
>   - delete struct "rte_eth_dev_conf_info", and reuse struct "rte_eth_conf".
>   - add "__rte_experimental" to the new API "rte_eth_dev_conf_info_get" declaration.
> v6: split this patch into two patches.
> v5: add an API to get device configuration info.
> 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->dev_conf.rx/txmode".
> 
> Jie Wang (2):
>    ethdev: add an API to get device configuration
>    app/testpmd: fix testpmd doesn't show RSS hash offload
> 

For series,
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

[dpdk-dev] [PATCH v13 0/2] testpmd shows incorrect rx_offload configuration

[Jie Wang]

Launch testpmd with multiple queues, and check rx_offload info.

When testpmd shows the port configuration, it doesn't show RSS_HASH.

---
v13:
 - update the API comment.
 - fix the bug that testpmd failed to run test_pf_tx_rx_queue test case.
v12: update the commit log and the API comment.
v11:
 - update the commit log.
 - rename the function and variable name.
v10:
 - update the commit log.
 - merge the first two patches.
 - rename the new API name.
v9:
 - add a release notes update for the new API.
 - update the description of the new API.
 - optimize the new API.
 - optimize the assignment of the offloads.
v8: delete "rte_exit" and just print error log.
v7:
 - delete struct "rte_eth_dev_conf_info", and reuse struct "rte_eth_conf".
 - add "__rte_experimental" to the new API "rte_eth_dev_conf_info_get" declaration.
v6: split this patch into two patches.
v5: add an API to get device configuration info.
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->dev_conf.rx/txmode".

Jie Wang (2):
  ethdev: add an API to get device configuration
  app/testpmd: fix testpmd doesn't show RSS hash offload

 app/test-pmd/cmdline.c                 | 14 ++++++--
 app/test-pmd/testpmd.c                 | 48 ++++++++++++++++++++++++--
 app/test-pmd/testpmd.h                 |  2 ++
 app/test-pmd/util.c                    | 14 ++++++++
 doc/guides/rel_notes/release_21_11.rst |  4 +++
 lib/ethdev/rte_ethdev.c                | 20 +++++++++++
 lib/ethdev/rte_ethdev.h                | 18 ++++++++++
 lib/ethdev/version.map                 |  1 +
 8 files changed, 116 insertions(+), 5 deletions(-)

-- 
2.25.1

[dpdk-dev] [PATCH v13 1/2] ethdev: add an API to get device configuration

[Jie Wang]

The driver may change offloads info into dev->data->dev_conf
in dev_configure which may cause apps use outdated values.

Add a new API to get actual device configuration.

Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Signed-off-by: Jie Wang <jie1x.wang@intel.com>
---
 doc/guides/rel_notes/release_21_11.rst |  4 ++++
 lib/ethdev/rte_ethdev.c                | 20 ++++++++++++++++++++
 lib/ethdev/rte_ethdev.h                | 18 ++++++++++++++++++
 lib/ethdev/version.map                 |  1 +
 4 files changed, 43 insertions(+)

diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
index d5c762df62..5292149981 100644
--- a/doc/guides/rel_notes/release_21_11.rst
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -81,6 +81,10 @@ New Features
   * Default VLAN strip behavior was changed. VLAN tag won't be stripped
     unless ``DEV_RX_OFFLOAD_VLAN_STRIP`` offload is enabled.
 
+* **Added support for users get device configuration in ethdev.**
+
+  Added an ethdev API which can help users get device configuration.
+
 * **Updated AF_XDP PMD.**
 
   * Disabled secondary process support.
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 5fae7357c8..063e4925f8 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -3437,6 +3437,26 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
 	return 0;
 }
 
+int
+rte_eth_dev_conf_get(uint16_t port_id, struct rte_eth_conf *dev_conf)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (dev_conf == NULL) {
+		RTE_ETHDEV_LOG(ERR,
+			"Cannot get ethdev port %u configuration to NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
+	memcpy(dev_conf, &dev->data->dev_conf, sizeof(struct rte_eth_conf));
+
+	return 0;
+}
+
 int
 rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
 				 uint32_t *ptypes, int num)
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index cb847a2c38..58d10e5699 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -3052,6 +3052,24 @@ int rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr);
  */
 int rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Retrieve the configuration of an Ethernet device.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param dev_conf
+ *   Location for Ethernet device configuration to be filled in.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+__rte_experimental
+int rte_eth_dev_conf_get(uint16_t port_id, struct rte_eth_conf *dev_conf);
+
 /**
  * Retrieve the firmware version of a device.
  *
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 29fb71f1af..4debda513b 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -247,6 +247,7 @@ EXPERIMENTAL {
 	rte_mtr_meter_policy_validate;
 
 	# added in 21.11
+	rte_eth_dev_conf_get;
 	rte_eth_rx_metadata_negotiate;
 };
 
-- 
2.25.1

[dpdk-dev] [PATCH v13 2/2] app/testpmd: fix testpmd doesn't show RSS hash offload

[Jie Wang]

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.

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

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")

Signed-off-by: Jie Wang <jie1x.wang@intel.com>
---
 app/test-pmd/cmdline.c | 14 ++++++++++--
 app/test-pmd/testpmd.c | 48 +++++++++++++++++++++++++++++++++++++++---
 app/test-pmd/testpmd.h |  2 ++
 app/test-pmd/util.c    | 14 ++++++++++++
 4 files changed, 73 insertions(+), 5 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 36d50fd3c7..b8f06063d2 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -16034,6 +16034,7 @@ cmd_rx_offload_get_configuration_parsed(
 	struct rte_eth_dev_info dev_info;
 	portid_t port_id = res->port_id;
 	struct rte_port *port = &ports[port_id];
+	struct rte_eth_conf dev_conf;
 	uint64_t port_offloads;
 	uint64_t queue_offloads;
 	uint16_t nb_rx_queues;
@@ -16042,7 +16043,11 @@ cmd_rx_offload_get_configuration_parsed(
 
 	printf("Rx Offloading Configuration of port %d :\n", port_id);
 
-	port_offloads = port->dev_conf.rxmode.offloads;
+	ret = eth_dev_conf_get_print_err(port_id, &dev_conf);
+	if (ret != 0)
+		return;
+
+	port_offloads = dev_conf.rxmode.offloads;
 	printf("  Port :");
 	print_rx_offloads(port_offloads);
 	printf("\n");
@@ -16448,6 +16453,7 @@ cmd_tx_offload_get_configuration_parsed(
 	struct rte_eth_dev_info dev_info;
 	portid_t port_id = res->port_id;
 	struct rte_port *port = &ports[port_id];
+	struct rte_eth_conf dev_conf;
 	uint64_t port_offloads;
 	uint64_t queue_offloads;
 	uint16_t nb_tx_queues;
@@ -16456,7 +16462,11 @@ cmd_tx_offload_get_configuration_parsed(
 
 	printf("Tx Offloading Configuration of port %d :\n", port_id);
 
-	port_offloads = port->dev_conf.txmode.offloads;
+	ret = eth_dev_conf_get_print_err(port_id, &dev_conf);
+	if (ret != 0)
+		return;
+
+	port_offloads = dev_conf.txmode.offloads;
 	printf("  Port :");
 	print_tx_offloads(port_offloads);
 	printf("\n");
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index a7841c557f..6cb00882bb 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2582,6 +2582,9 @@ start_port(portid_t pid)
 		}
 
 		if (port->need_reconfig > 0) {
+			struct rte_eth_conf dev_conf;
+			int k;
+
 			port->need_reconfig = 0;
 
 			if (flow_isolate_all) {
@@ -2619,6 +2622,36 @@ start_port(portid_t pid)
 				port->need_reconfig = 1;
 				return -1;
 			}
+			/* get device configuration*/
+			if (0 !=
+				eth_dev_conf_get_print_err(pi, &dev_conf)) {
+				fprintf(stderr,
+					"port %d can not get device configuration\n",
+					pi);
+				return -1;
+			}
+			/* Apply Rx offloads configuration */
+			if (dev_conf.rxmode.offloads !=
+			    port->dev_conf.rxmode.offloads) {
+				port->dev_conf.rxmode.offloads |=
+					dev_conf.rxmode.offloads;
+				for (k = 0;
+				     k < port->dev_info.max_rx_queues;
+				     k++)
+					port->rx_conf[k].offloads |=
+						dev_conf.rxmode.offloads;
+			}
+			/* Apply Tx offloads configuration */
+			if (dev_conf.txmode.offloads !=
+			    port->dev_conf.txmode.offloads) {
+				port->dev_conf.txmode.offloads |=
+					dev_conf.txmode.offloads;
+				for (k = 0;
+				     k < port->dev_info.max_tx_queues;
+				     k++)
+					port->tx_conf[k].offloads |=
+						dev_conf.txmode.offloads;
+			}
 		}
 		if (port->need_reconfig_queues > 0 && is_proc_primary()) {
 			port->need_reconfig_queues = 0;
@@ -3581,7 +3614,7 @@ init_port_config(void)
 {
 	portid_t pid;
 	struct rte_port *port;
-	int ret;
+	int ret, i;
 
 	RTE_ETH_FOREACH_DEV(pid) {
 		port = &ports[pid];
@@ -3601,12 +3634,21 @@ init_port_config(void)
 		}
 
 		if (port->dcb_flag == 0) {
-			if( port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0)
+			if (port->dev_conf.rx_adv_conf.rss_conf.rss_hf != 0) {
 				port->dev_conf.rxmode.mq_mode =
 					(enum rte_eth_rx_mq_mode)
 						(rx_mq_mode & ETH_MQ_RX_RSS);
-			else
+			} else {
 				port->dev_conf.rxmode.mq_mode = ETH_MQ_RX_NONE;
+				port->dev_conf.rxmode.offloads &=
+						~DEV_RX_OFFLOAD_RSS_HASH;
+
+				for (i = 0;
+				     i < port->dev_info.nb_rx_queues;
+				     i++)
+					port->rx_conf[i].offloads &=
+						~DEV_RX_OFFLOAD_RSS_HASH;
+			}
 		}
 
 		rxtx_port_config(port);
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index e9d9db06ce..33d931eb85 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -960,6 +960,8 @@ void show_gro(portid_t port_id);
 void setup_gso(const char *mode, portid_t port_id);
 int eth_dev_info_get_print_err(uint16_t port_id,
 			struct rte_eth_dev_info *dev_info);
+int eth_dev_conf_get_print_err(uint16_t port_id,
+			struct rte_eth_conf *dev_conf);
 void eth_set_promisc_mode(uint16_t port_id, int enable);
 void eth_set_allmulticast_mode(uint16_t port, int enable);
 int eth_link_get_nowait_print_err(uint16_t port_id, struct rte_eth_link *link);
diff --git a/app/test-pmd/util.c b/app/test-pmd/util.c
index 51506e4940..1256eb6e66 100644
--- a/app/test-pmd/util.c
+++ b/app/test-pmd/util.c
@@ -444,6 +444,20 @@ eth_dev_info_get_print_err(uint16_t port_id,
 	return ret;
 }
 
+int
+eth_dev_conf_get_print_err(uint16_t port_id, struct rte_eth_conf *dev_conf)
+{
+	int ret;
+
+	ret = rte_eth_dev_conf_get(port_id, dev_conf);
+	if (ret != 0)
+		fprintf(stderr,
+			"Error during getting device configuration (port %u): %s\n",
+			port_id, strerror(-ret));
+
+	return ret;
+}
+
 void
 eth_set_promisc_mode(uint16_t port, int enable)
 {
-- 
2.25.1

Re: [dpdk-dev] [PATCH v13 0/2] testpmd shows incorrect rx_offload configuration

[Ferruh Yigit]

On 10/14/2021 11:31 AM, Jie Wang wrote:
> Launch testpmd with multiple queues, and check rx_offload info.
> 
> When testpmd shows the port configuration, it doesn't show RSS_HASH.
> 
> ---
> v13:
>   - update the API comment.
>   - fix the bug that testpmd failed to run test_pf_tx_rx_queue test case.
> v12: update the commit log and the API comment.
> v11:
>   - update the commit log.
>   - rename the function and variable name.
> v10:
>   - update the commit log.
>   - merge the first two patches.
>   - rename the new API name.
> v9:
>   - add a release notes update for the new API.
>   - update the description of the new API.
>   - optimize the new API.
>   - optimize the assignment of the offloads.
> v8: delete "rte_exit" and just print error log.
> v7:
>   - delete struct "rte_eth_dev_conf_info", and reuse struct "rte_eth_conf".
>   - add "__rte_experimental" to the new API "rte_eth_dev_conf_info_get" declaration.
> v6: split this patch into two patches.
> v5: add an API to get device configuration info.
> 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->dev_conf.rx/txmode".
> 
> Jie Wang (2):
>    ethdev: add an API to get device configuration
>    app/testpmd: fix testpmd doesn't show RSS hash offload
> 

For series,
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

I am waiting CI result to proceed with patch.

Re: [dpdk-dev] [PATCH v13 0/2] testpmd shows incorrect rx_offload configuration

[Ferruh Yigit]

On 10/14/2021 1:56 PM, Ferruh Yigit wrote:
> On 10/14/2021 11:31 AM, Jie Wang wrote:
>> Launch testpmd with multiple queues, and check rx_offload info.
>>
>> When testpmd shows the port configuration, it doesn't show RSS_HASH.
>>
>> ---
>> v13:
>>   - update the API comment.
>>   - fix the bug that testpmd failed to run test_pf_tx_rx_queue test case.
>> v12: update the commit log and the API comment.
>> v11:
>>   - update the commit log.
>>   - rename the function and variable name.
>> v10:
>>   - update the commit log.
>>   - merge the first two patches.
>>   - rename the new API name.
>> v9:
>>   - add a release notes update for the new API.
>>   - update the description of the new API.
>>   - optimize the new API.
>>   - optimize the assignment of the offloads.
>> v8: delete "rte_exit" and just print error log.
>> v7:
>>   - delete struct "rte_eth_dev_conf_info", and reuse struct "rte_eth_conf".
>>   - add "__rte_experimental" to the new API "rte_eth_dev_conf_info_get" declaration.
>> v6: split this patch into two patches.
>> v5: add an API to get device configuration info.
>> 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->dev_conf.rx/txmode".
>>
>> Jie Wang (2):
>>    ethdev: add an API to get device configuration
>>    app/testpmd: fix testpmd doesn't show RSS hash offload
>>
> 
> For series,
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> I am waiting CI result to proceed with patch.
> 

CI failed to apply the patch :(, I am proceeding with the patch.

Re: [dpdk-dev] [PATCH v13 0/2] testpmd shows incorrect rx_offload configuration

[Ferruh Yigit]

On 10/14/2021 1:56 PM, Ferruh Yigit wrote:
> On 10/14/2021 11:31 AM, Jie Wang wrote:
>> Launch testpmd with multiple queues, and check rx_offload info.
>>
>> When testpmd shows the port configuration, it doesn't show RSS_HASH.
>>
>> ---
>> v13:
>>   - update the API comment.
>>   - fix the bug that testpmd failed to run test_pf_tx_rx_queue test case.
>> v12: update the commit log and the API comment.
>> v11:
>>   - update the commit log.
>>   - rename the function and variable name.
>> v10:
>>   - update the commit log.
>>   - merge the first two patches.
>>   - rename the new API name.
>> v9:
>>   - add a release notes update for the new API.
>>   - update the description of the new API.
>>   - optimize the new API.
>>   - optimize the assignment of the offloads.
>> v8: delete "rte_exit" and just print error log.
>> v7:
>>   - delete struct "rte_eth_dev_conf_info", and reuse struct "rte_eth_conf".
>>   - add "__rte_experimental" to the new API "rte_eth_dev_conf_info_get" declaration.
>> v6: split this patch into two patches.
>> v5: add an API to get device configuration info.
>> 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->dev_conf.rx/txmode".
>>
>> Jie Wang (2):
>>    ethdev: add an API to get device configuration
>>    app/testpmd: fix testpmd doesn't show RSS hash offload
>>
> 
> For series,
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 

Series applied to dpdk-next-net/main, thanks.

[dpdk-dev] [PATCH v6 0/2] testpmd shows incorrect rx_offload configuration

[Jie Wang]

Launch testpmd with multiple queues, and check rx_offload info.

When testpmd shows the port configuration, it doesn't show RSS_HASH.

---
v6: split this patch into two patches.
v5: add an API to get device configuration info.
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->dev_conf.rx/txmode".

Jie Wang (2):
  ethdev: add an API to get device configuration info
  app/testpmd: fix testpmd doesn't show RSS hash offload

 app/test-pmd/testpmd.c  | 33 +++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h  |  2 ++
 app/test-pmd/util.c     | 15 +++++++++++++++
 lib/ethdev/rte_ethdev.c | 27 +++++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.h | 26 ++++++++++++++++++++++++++
 lib/ethdev/version.map  |  3 +++
 6 files changed, 106 insertions(+)

-- 
2.25.1

[dpdk-dev] [PATCH v6 1/2] ethdev: add an API to get device configuration info

[Jie Wang]

This patch adds a new API "rte_eth_dev_conf_info_get()" to help testpmd get
device configuration info.

Signed-off-by: Jie Wang <jie1x.wang@intel.com>
---
 lib/ethdev/rte_ethdev.c | 27 +++++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.h | 26 ++++++++++++++++++++++++++
 lib/ethdev/version.map  |  3 +++
 3 files changed, 56 insertions(+)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 9d95cd11e1..74184099a1 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -3458,6 +3458,33 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
 	return 0;
 }
 
+int
+rte_eth_dev_conf_info_get(uint16_t port_id,
+				struct rte_eth_dev_conf_info *dev_conf_info)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (dev_conf_info == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Cannot get ethdev port %u config info to NULL\n",
+			port_id);
+		return -EINVAL;
+	}
+
+	/*
+	 * Init dev_conf_info before port_id check since caller does not have
+	 * return status and does not know if get is successful or not.
+	 */
+	memset(dev_conf_info, 0, sizeof(struct rte_eth_dev_conf_info));
+
+	dev_conf_info->rx_offloads = dev->data->dev_conf.rxmode.offloads;
+	dev_conf_info->tx_offloads = dev->data->dev_conf.txmode.offloads;
+
+	return 0;
+}
+
 int
 rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
 				 uint32_t *ptypes, int num)
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index d2b27c351f..70a2db550f 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -1587,6 +1587,15 @@ struct rte_eth_dev_info {
 	void *reserved_ptrs[2];   /**< Reserved for future fields */
 };
 
+/**
+ * Ethernet device configuration information structure.
+ * Used to retrieve information about configured device.
+ */
+struct rte_eth_dev_conf_info {
+	uint64_t rx_offloads; /**rxmode offloads */
+	uint64_t tx_offloads; /**txmode offloads */
+};
+
 /**
  * RX/TX queue states
  */
@@ -3058,6 +3067,23 @@ int rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr);
  */
 int rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info);
 
+/**
+ * Retrieve the contextual information of an Ethernet device.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param dev_conf_info
+ *   A pointer to a structure of type *rte_eth_dev_conf_info* to be filled with
+ *   the contextual information of the Ethernet device.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if support for dev_infos_get() does not exist for the device.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+int rte_eth_dev_conf_info_get(uint16_t port_id,
+				struct rte_eth_dev_conf_info *dev_conf_info);
+
 /**
  * Retrieve the firmware version of a device.
  *
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 44d30b05ae..40539f99f9 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -249,6 +249,9 @@ EXPERIMENTAL {
 	rte_mtr_meter_policy_delete;
 	rte_mtr_meter_policy_update;
 	rte_mtr_meter_policy_validate;
+
+	# added in 21.11
+	rte_eth_dev_conf_info_get;
 };
 
 INTERNAL {
-- 
2.25.1

Re: [dpdk-dev] [PATCH v6 1/2] ethdev: add an API to get device configuration info

[Ferruh Yigit]

On 8/24/2021 7:19 PM, Jie Wang wrote:
> This patch adds a new API "rte_eth_dev_conf_info_get()" to help testpmd get
> device configuration info.
> 
> Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> ---
>  lib/ethdev/rte_ethdev.c | 27 +++++++++++++++++++++++++++
>  lib/ethdev/rte_ethdev.h | 26 ++++++++++++++++++++++++++
>  lib/ethdev/version.map  |  3 +++
>  3 files changed, 56 insertions(+)
> 
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index 9d95cd11e1..74184099a1 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -3458,6 +3458,33 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
>  	return 0;
>  }
>  
> +int
> +rte_eth_dev_conf_info_get(uint16_t port_id,
> +				struct rte_eth_dev_conf_info *dev_conf_info)
> +{
> +	struct rte_eth_dev *dev;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +	dev = &rte_eth_devices[port_id];
> +
> +	if (dev_conf_info == NULL) {
> +		RTE_ETHDEV_LOG(ERR, "Cannot get ethdev port %u config info to NULL\n",
> +			port_id);
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * Init dev_conf_info before port_id check since caller does not have
> +	 * return status and does not know if get is successful or not.
> +	 */
> +	memset(dev_conf_info, 0, sizeof(struct rte_eth_dev_conf_info));
> +
> +	dev_conf_info->rx_offloads = dev->data->dev_conf.rxmode.offloads;
> +	dev_conf_info->tx_offloads = dev->data->dev_conf.txmode.offloads;
> +
> +	return 0;
> +}
> +
>  int
>  rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
>  				 uint32_t *ptypes, int num)
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index d2b27c351f..70a2db550f 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -1587,6 +1587,15 @@ struct rte_eth_dev_info {
>  	void *reserved_ptrs[2];   /**< Reserved for future fields */
>  };
>  
> +/**
> + * Ethernet device configuration information structure.
> + * Used to retrieve information about configured device.
> + */
> +struct rte_eth_dev_conf_info {
> +	uint64_t rx_offloads; /**rxmode offloads */
> +	uint64_t tx_offloads; /**txmode offloads */
> +};

My concern is if we need to extend this struct later, when application wants to
get more current config from the dpdk layer, it will cause ABI break and will
need to wait next LTS.

And as this struct grow, it will be kind of duplication of the 'struct
rte_eth_conf'.

What do you think to reuse 'struct rte_eth_conf' in this API, to cover future needs?

> +
>  /**
>   * RX/TX queue states
>   */
> @@ -3058,6 +3067,23 @@ int rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr);
>   */
>  int rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info);
>  
> +/**
> + * Retrieve the contextual information of an Ethernet device.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param dev_conf_info
> + *   A pointer to a structure of type *rte_eth_dev_conf_info* to be filled with
> + *   the contextual information of the Ethernet device.
> + * @return
> + *   - (0) if successful.
> + *   - (-ENOTSUP) if support for dev_infos_get() does not exist for the device.
> + *   - (-ENODEV) if *port_id* invalid.
> + *   - (-EINVAL) if bad parameter.
> + */
> +int rte_eth_dev_conf_info_get(uint16_t port_id,
> +				struct rte_eth_dev_conf_info *dev_conf_info);
> +
>  /**
>   * Retrieve the firmware version of a device.
>   *
> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
> index 44d30b05ae..40539f99f9 100644
> --- a/lib/ethdev/version.map
> +++ b/lib/ethdev/version.map
> @@ -249,6 +249,9 @@ EXPERIMENTAL {
>  	rte_mtr_meter_policy_delete;
>  	rte_mtr_meter_policy_update;
>  	rte_mtr_meter_policy_validate;
> +
> +	# added in 21.11
> +	rte_eth_dev_conf_info_get;
>  };
>  
>  INTERNAL {
> 

Re: [dpdk-dev] [PATCH v6 1/2] ethdev: add an API to get device configuration info

[Ajit Khaparde]

[-- Attachment #1: Type: text/plain, Size: 4427 bytes --]

On Wed, Aug 25, 2021 at 1:08 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> On 8/24/2021 7:19 PM, Jie Wang wrote:
> > This patch adds a new API "rte_eth_dev_conf_info_get()" to help testpmd get
> > device configuration info.
> >
> > Signed-off-by: Jie Wang <jie1x.wang@intel.com>
> > ---
> >  lib/ethdev/rte_ethdev.c | 27 +++++++++++++++++++++++++++
> >  lib/ethdev/rte_ethdev.h | 26 ++++++++++++++++++++++++++
> >  lib/ethdev/version.map  |  3 +++
> >  3 files changed, 56 insertions(+)
> >
> > diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> > index 9d95cd11e1..74184099a1 100644
> > --- a/lib/ethdev/rte_ethdev.c
> > +++ b/lib/ethdev/rte_ethdev.c
> > @@ -3458,6 +3458,33 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
> >       return 0;
> >  }
> >
> > +int
> > +rte_eth_dev_conf_info_get(uint16_t port_id,
> > +                             struct rte_eth_dev_conf_info *dev_conf_info)
> > +{
> > +     struct rte_eth_dev *dev;
> > +
> > +     RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> > +     dev = &rte_eth_devices[port_id];
> > +
> > +     if (dev_conf_info == NULL) {
> > +             RTE_ETHDEV_LOG(ERR, "Cannot get ethdev port %u config info to NULL\n",
> > +                     port_id);
> > +             return -EINVAL;
> > +     }
> > +
> > +     /*
> > +      * Init dev_conf_info before port_id check since caller does not have
> > +      * return status and does not know if get is successful or not.
> > +      */
> > +     memset(dev_conf_info, 0, sizeof(struct rte_eth_dev_conf_info));
> > +
> > +     dev_conf_info->rx_offloads = dev->data->dev_conf.rxmode.offloads;
> > +     dev_conf_info->tx_offloads = dev->data->dev_conf.txmode.offloads;
> > +
> > +     return 0;
> > +}
> > +
> >  int
> >  rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
> >                                uint32_t *ptypes, int num)
> > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> > index d2b27c351f..70a2db550f 100644
> > --- a/lib/ethdev/rte_ethdev.h
> > +++ b/lib/ethdev/rte_ethdev.h
> > @@ -1587,6 +1587,15 @@ struct rte_eth_dev_info {
> >       void *reserved_ptrs[2];   /**< Reserved for future fields */
> >  };
> >
> > +/**
> > + * Ethernet device configuration information structure.
> > + * Used to retrieve information about configured device.
> > + */
> > +struct rte_eth_dev_conf_info {
> > +     uint64_t rx_offloads; /**rxmode offloads */
> > +     uint64_t tx_offloads; /**txmode offloads */
> > +};
>
> My concern is if we need to extend this struct later, when application wants to
> get more current config from the dpdk layer, it will cause ABI break and will
> need to wait next LTS.
>
> And as this struct grow, it will be kind of duplication of the 'struct
> rte_eth_conf'.
>
> What do you think to reuse 'struct rte_eth_conf' in this API, to cover future needs?
+1
rte_eth_conf gives all the information needed and more for future enhancements!

>
> > +
> >  /**
> >   * RX/TX queue states
> >   */
> > @@ -3058,6 +3067,23 @@ int rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr);
> >   */
> >  int rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info);
> >
> > +/**
> > + * Retrieve the contextual information of an Ethernet device.
> > + *
> > + * @param port_id
> > + *   The port identifier of the Ethernet device.
> > + * @param dev_conf_info
> > + *   A pointer to a structure of type *rte_eth_dev_conf_info* to be filled with
> > + *   the contextual information of the Ethernet device.
> > + * @return
> > + *   - (0) if successful.
> > + *   - (-ENOTSUP) if support for dev_infos_get() does not exist for the device.
> > + *   - (-ENODEV) if *port_id* invalid.
> > + *   - (-EINVAL) if bad parameter.
> > + */
> > +int rte_eth_dev_conf_info_get(uint16_t port_id,
> > +                             struct rte_eth_dev_conf_info *dev_conf_info);
> > +
> >  /**
> >   * Retrieve the firmware version of a device.
> >   *
> > diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
> > index 44d30b05ae..40539f99f9 100644
> > --- a/lib/ethdev/version.map
> > +++ b/lib/ethdev/version.map
> > @@ -249,6 +249,9 @@ EXPERIMENTAL {
> >       rte_mtr_meter_policy_delete;
> >       rte_mtr_meter_policy_update;
> >       rte_mtr_meter_policy_validate;
> > +
> > +     # added in 21.11
> > +     rte_eth_dev_conf_info_get;
> >  };
> >
> >  INTERNAL {
> >
>

[dpdk-dev] [PATCH v6 2/2] app/testpmd: fix testpmd doesn't show RSS hash offload

[Jie Wang]

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.

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 <jie1x.wang@intel.com>
---
 app/test-pmd/testpmd.c | 33 +++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h |  2 ++
 app/test-pmd/util.c    | 15 +++++++++++++++
 3 files changed, 50 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 6cbe9ba3c8..9a78805673 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2461,6 +2461,9 @@ start_port(portid_t pid)
 		}
 
 		if (port->need_reconfig > 0) {
+			struct rte_eth_dev_conf_info dev_conf_info;
+			int k;
+
 			port->need_reconfig = 0;
 
 			if (flow_isolate_all) {
@@ -2498,6 +2501,36 @@ start_port(portid_t pid)
 				port->need_reconfig = 1;
 				return -1;
 			}
+			/* get offloads */
+			if (0 !=
+				eth_dev_conf_info_get_print_err(pi,
+							&dev_conf_info)) {
+				rte_exit(EXIT_FAILURE,
+				    "rte_eth_dev_conf_info_get() failed\n");
+				return -1;
+			}
+			/* Apply Rx offloads configuration */
+			if (dev_conf_info.rx_offloads !=
+				port->dev_conf.rxmode.offloads) {
+				port->dev_conf.rxmode.offloads =
+					dev_conf_info.rx_offloads;
+				for (k = 0;
+				     k < port->dev_info.max_rx_queues;
+				     k++)
+					port->rx_conf[k].offloads =
+						dev_conf_info.rx_offloads;
+			}
+			/* Apply Tx offloads configuration */
+			if (dev_conf_info.tx_offloads !=
+				port->dev_conf.txmode.offloads) {
+				port->dev_conf.txmode.offloads =
+					dev_conf_info.tx_offloads;
+				for (k = 0;
+				     k < port->dev_info.max_tx_queues;
+				     k++)
+					port->tx_conf[k].offloads =
+						dev_conf_info.tx_offloads;
+			}
 		}
 		if (port->need_reconfig_queues > 0) {
 			port->need_reconfig_queues = 0;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 16a3598e48..4cf69a17a3 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -950,6 +950,8 @@ void show_gro(portid_t port_id);
 void setup_gso(const char *mode, portid_t port_id);
 int eth_dev_info_get_print_err(uint16_t port_id,
 			struct rte_eth_dev_info *dev_info);
+int eth_dev_conf_info_get_print_err(uint16_t port_id,
+			struct rte_eth_dev_conf_info *dev_info);
 void eth_set_promisc_mode(uint16_t port_id, int enable);
 void eth_set_allmulticast_mode(uint16_t port, int enable);
 int eth_link_get_nowait_print_err(uint16_t port_id, struct rte_eth_link *link);
diff --git a/app/test-pmd/util.c b/app/test-pmd/util.c
index 5dd7157947..c6992fba7a 100644
--- a/app/test-pmd/util.c
+++ b/app/test-pmd/util.c
@@ -440,6 +440,21 @@ eth_dev_info_get_print_err(uint16_t port_id,
 	return ret;
 }
 
+int
+eth_dev_conf_info_get_print_err(uint16_t port_id,
+				struct rte_eth_dev_conf_info *dev_conf_info)
+{
+	int ret;
+
+	ret = rte_eth_dev_conf_info_get(port_id, dev_conf_info);
+	if (ret != 0)
+		fprintf(stderr,
+			"Error during getting device config (port %u) info: %s\n",
+			port_id, strerror(-ret));
+
+	return ret;
+}
+
 void
 eth_set_promisc_mode(uint16_t port, int enable)
 {
-- 
2.25.1