On 6/12/2019 2:17 AM, Zhao1, Wei wrote:
>
>
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Tuesday, June 11, 2019 10:37 PM
>> To: Zhao1, Wei <wei.zh...@intel.com>; dev@dpdk.org
>> Cc: sta...@dpdk.org; Peng, Yuan <yuan.p...@intel.com>; Lu, Wenzhuo
>> <wenzhuo...@intel.com>; Kevin Traynor <ktray...@redhat.com>
>> Subject: Re: [dpdk-stable] [PATCH] app/testpmd: fix offloads overwrite by
>> default configuration
>>
>> On 5/9/2019 8:20 AM, Wei Zhao wrote:
>>> There is an error in function rxtx_port_config(), which may overwrite
>>> offloads configuration get from function launch_args_parse() when run
>>> testpmd app. So rxtx_port_config() should do "or" for port offloads.
>>>
>>> Fixes: d44f8a485f5d ("app/testpmd: enable per queue configure")
>>> cc: sta...@dpdk.org
>>>
>>> Signed-off-by: Wei Zhao <wei.zh...@intel.com>
>>> ---
>>> app/test-pmd/testpmd.c | 5 +++++
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
>>> 6fbfd29..f0061d9 100644
>>> --- a/app/test-pmd/testpmd.c
>>> +++ b/app/test-pmd/testpmd.c
>>> @@ -2809,9 +2809,12 @@ static void
>>> rxtx_port_config(struct rte_port *port) {
>>> uint16_t qid;
>>> + uint64_t offloads;
>>>
>>> for (qid = 0; qid < nb_rxq; qid++) {
>>> + offloads = port->rx_conf[qid].offloads;
>>> port->rx_conf[qid] = port->dev_info.default_rxconf;
>>> + port->rx_conf[qid].offloads |= offloads;
>>
>> While talking with Kevin, he pointed out the error in this code.
>>
>> We are updating queue level offloads, with whatever in the 'offloads' and it
>> can
>> be non-queue level offloads in it, next time ethdev API called these values
>> are
>> caught by the API checks and causing an error.
>>
>> It looks like port level offload flags needs to be masted out before writing
>> to
>> queue level 'offloads' variable.
>
>
> By the way, this error in not introduced in this patch, it seems has exist
> long before this patch.
> This patch is just fix for overwrite problem.
I disagree, writing 'offloads' to "rx_conf[].offloads" without checking if they
queue offloads or not causing this problem. And that write introduced in this
patch.
>
>
>
>>
>>>
>>> /* Check if any Rx parameters have been passed */
>>> if (rx_pthresh != RTE_PMD_PARAM_UNSET) @@ -2833,7
>> +2836,9 @@
>>> rxtx_port_config(struct rte_port *port)
>>> }
>>>
>>> for (qid = 0; qid < nb_txq; qid++) {
>>> + offloads = port->tx_conf[qid].offloads;
>>> port->tx_conf[qid] = port->dev_info.default_txconf;
>>> + port->tx_conf[qid].offloads |= offloads;
>>>
>>> /* Check if any Tx parameters have been passed */
>>> if (tx_pthresh != RTE_PMD_PARAM_UNSET)
>>>
>
