Re: [dpdk-dev] [dpdk-stable] [PATCH] app/testpmd: fix offloads overwrite by default configuration

["Zhao1, Wei" <wei.zhao1@intel.com>]

Hi,  Ferruh

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Tuesday, May 14, 2019 12:36 AM
> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org; Peng, Yuan <yuan.peng@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@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: stable@dpdk.org
> >
> > Signed-off-by: Wei Zhao <wei.zhao1@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.

> 
> >
> >  		/* 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)
> >