On 5/14/2019 2:56 AM, Zhao1, Wei wrote:
> Hi, Ferruh
>
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Tuesday, May 14, 2019 12:36 AM
>> 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>
>> 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;
>>
>> OK to this changes as a fix for this release.
>>
>> But I think intention is, if no offload information is provided by user to
>> use use
>> the driver provided defaults, if user explicitly provided some values to use
>> them,
>> instead of OR these two.
>>
>> With this approach it is not possible to disable a driver default value, so
>> it
>> becomes mandatory offload instead of default offload values.
>>
>> Wei, what do you think, does it make sense?
>
>
> I agree with you, but it is sure that the original code has offloads
> overwrite issue.
> What is your suggestion for code implement?
> I find that Thomas has apply it, if you has other idea, maybe you has to
> commit patch base to this patch.
Hi Wei,
Yes this needs to be incremental fix to existing code.
Queue specific offload can be altered either by providing Rx/Tx offload as
command line argument [1] (port configs set to each queues) or via testpmd
commands [2].
Does it make sense to set a global flag when one of above occurs and use default
config only if it is not set?
[1]
Tx
tx-offloads
Rx
disable-crc-strip
enable-lro
enable-scatter
enable-rx-cksum
enable-rx-timestamp
enable-hw-vlan
enable-hw-vlan-filter
enable-hw-vlan-strip
enable-hw-vlan-extend
[2]
"port config <port_id> rx_offload ..."
"port <port_id> rxq <queue_id> rx_offload ..."
"port config <port_id> tx_offload ..."
"port <port_id> txq <queue_id> tx_offload ..."
