[dpdk-dev] [PATCH 2/5] app/testpmd: remove txqflags

[Shahaf Shuler]

Thursday, December 7, 2017 1:05 AM, Ferruh Yigit:
> On 12/4/2017 10:48 PM, Shahaf Shuler wrote:
> > Tuesday, December 5, 2017 12:31 AM, Ferruh Yigit:
> >> On 11/23/2017 4:08 AM, Shahaf Shuler wrote:
> >>> Since testpmd is now using the new Ethdev offloads API there is no
> >>> need for the txqflags configuration.
> >>
> >> txqflag command-line option (--txqflags=0xXXXXXXXX) and command
> (port
> >> config all txqflags value) are way to dynamically provide Tx offload
> >> configuration to testpmd.
> >>
> >> Why you are removing them without providing equivalent feature with
> >> new offload APIs?
> >
> > There is one, and this is why the new offloads API simplifies applications
> life.
> > The txq_flags were introduced so application can disable some of the Tx
> offloads from the time the Tx offloads were set by default.
> >
> > Now the Tx offloads are all disabled by default. And there is an option to
> set each one of the using DEV_TX_OFFLOAD_* flags.
> > Look on the Rx side (there the Rx offloads were disabled by default in the
> old API). there is no rxq_flags.
> 
> I mean providing a testpmd command and command-line option.
> Or am I missing it, after this update is there a way to update enabled Tx
> offloads in testpmd via command and command-line?

There is a way to update via CLI
For example when doing "tso set <mss> <port>" the DEV_TX_OFFLOAD_TSO will be set
When doing tx_vlan add <port> <vlan> the DEV_TX_OFFLOAD_VLAN_INSERT will be set
Same for every Tx offloads besides DEV_TX_OFFLOAD_MULTI_SEGS  and DEV_TX_OFFLOAD_MBUF_FAST_FREE which I added on separate patches. 

But to make it precise you didn’t had before (with the txq_flags) a way to disable TSO from the command line (the is no TXQ flag for that) nor the tunnel TSO. 

> 
> >
> >>
> >> Patch 3/4 & 4/5 adds new command-line options to enable some of
> >> recently introduced Tx offloads in testpmd, why not add a generic way
> >> to update all Tx offloads instead of those two specific offloads?
> >
> > Because those 2 new offloads were missing a way to set in testpmd.
> > The rest of the offloads can be set using the CLI commands exists today.
> >
> > Am not sure we want every Tx offloads to be set from both CLI and
> command line arguments.
> > It wasn't before (for example there was no --disable-tso flags).
> 
> I was thinking a generic command-line instead of introducing one for all,
> overall single variable represents all offload values, why not set it at once, as
> --txqflags you are removing in this patch.

We can do it. yes. 

> 
> >
> >>
> >> Also this patch removing ability to display current Tx offload
> configuration....
> >
> > Can you point where ?
> 
> I mean following:
> 
>   -	printf("  TX RS bit threshold=%d - TXQ flags=0x%"PRIx32"\n",
>   -	       tx_conf->tx_rs_thresh, tx_conf->txq_flags);
>   +	printf("  TX RS bit threshold=%d\n", tx_conf->tx_rs_thresh);
> 
> Why not an offloads replacement of this one?

Will be added. 

> 
> >
> >>
> >>>
> >>> Signed-off-by: Shahaf Shuler <shahafs at mellanox.com>
> >>> Acked-by: Nelio Laranjeiro <nelio.laranjeiro at 6wind.com>
> >>
> >> <...>
> >