Hi Ferruh, > -----Original Message----- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ferruh Yigit > Sent: Friday, July 19, 2019 5:53 PM > To: viveksha...@marvell.com; dev@dpdk.org > Cc: intoviveksha...@gmail.com > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: support QinQ strip offload > > On 7/17/2019 8:45 AM, viveksha...@marvell.com wrote: > > From: Vivek Sharma <viveksha...@marvell.com> > > > > Support QinQ strip RX offload configuration through testpmd command > > line and boot time arguments. > > For the testpmd command part, unfortunately there are two set of > commands for same purpose, the new ones are (lets both port and queue > level): > "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 ..." > > These are better implementation comparing the old one: > "port config all ..." > > Would you mind sending a patch to remove "port config all ..." variant of > setting offloads? > And you can make your changes to the new commands above.
Is it ok to remove "port config all ..." commands as they may be in use by the community? > For the application argument, ``--enable-hw-vlan-extend``, instead of adding > a parameter of each offload argument, (and event it is not clear if it is > only for > Rx or Tx), have a "--rx-offloads" argument and feed the list via this, like: > "--rx-ofloads=disable-crc-strip,enable-rx-timestamp" > > > > And lastly for the "vlan set ..." update, I think "qinq" was already defined > but > it was calling 'vlan_extend_set()', now you are changing it and making it call > 'rx_vlan_qinq_strip_set()', I think this is OK, but can you please update the > 'cmd_help_long_parsed()' accordingly? 'vlan_extend_set()' and 'rx_vlan_qinq_strip_set()' are different commands. vlan_extend_set() is for adding the second vlan and rx_vlan_qinq_strip_set() is for removing the second vlan. > And in original 'cmd_help_long_parsed()' for 'vlan set ...", it doesn't need > to > have separate lines for "strip, filter & qinq", can you please merge them, and > later the 'extend' one? > Than change needs to be documented on "testpmd_funcs.rst" > > > And as a last thing, can you please send this as multiple patches: > 1) Command line change for setting qinq offload > 2) Application argument change > 3) "vlan set " related changes > > > > > Signed-off-by: Vivek Sharma <viveksha...@marvell.com> > Regards, Bernard