Hi, Ferruh
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Monday, May 20, 2019 11:23 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>
> Subject: Re: [dpdk-stable] [PATCH] app/testpmd: fix offloads overwrite by
> default configuration
>
> 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?
I AGREE with you to submit an incremental fix, and it make sense to set a
global flag when one of above occurs and use
default config only if it is not set when implement code, but I do not have
time to prepare such a patch by now, so maybe later or some else.
>
> [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 ..."
>
