diff mbox series

[v4] app/testpmd: fix testpmd doesn't show RSS hash offload

Message ID 20210715115720.9981-1-jie1x.wang@intel.com (mailing list archive)
State Rejected, archived
Delegated to: Andrew Rybchenko
Headers
Series [v4] app/testpmd: fix testpmd doesn't show RSS hash offload |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/github-robot success github build: passed
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-abi-testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing fail Testing issues
ci/iol-testing success Testing PASS
ci/iol-intel-Performance fail Performance Testing issues

Commit Message

Jie Wang July 15, 2021, 11:57 a.m. UTC 
  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(+)

Comments

Li, Xiaoyun July 15, 2021, 4:53 a.m. UTC | #1 
> -----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>
Li, Xiaoyun July 16, 2021, 8:30 a.m. UTC | #2 
> -----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.
Ferruh Yigit July 16, 2021, 8:52 a.m. UTC | #3 
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.
Ferruh Yigit July 19, 2021, 4:18 p.m. UTC | #4 
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.
Andrew Rybchenko July 22, 2021, 11:03 a.m. UTC | #5 
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().
Ferruh Yigit Aug. 9, 2021, 8:53 a.m. UTC | #6 
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.
diff mbox series

Patch

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;