Re: [dpdk-dev] [dpdk-stable] [PATCH] app/testpmd: fix display capabilities routine

[Slava Ovsiienko <viacheslavo@mellanox.com>]

Hi, Ferruh

Please, see below

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Wednesday, July 17, 2019 17:56
> To: Slava Ovsiienko <viacheslavo@mellanox.com>; dev@dpdk.org
> Cc: bernard.iremonger@intel.com; stable@dpdk.org
> Subject: Re: [dpdk-stable] [PATCH] app/testpmd: fix display capabilities
> routine
> 
> On 7/16/2019 12:39 PM, Viacheslav Ovsiienko wrote:
> > The DEV_TX_OFFLOAD_MATCH_METADATA was introduced by [1], but it is
> not
> > displayed by "show port [id] tx_offloads capabilities"
> > command in testpmd.
> >
> > [1]
> >
> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatch
> >
> es.dpdk.org%2Fpatch%2F47265%2F&amp;data=02%7C01%7Cviacheslavo%4
> 0mellan
> >
> ox.com%7C3fea893cbf43414d6f9a08d70ac6e78f%7Ca652971c7d2e4d9ba6a
> 4d14925
> >
> 6f461b%7C0%7C0%7C636989721710321485&amp;sdata=bnKjCSFr%2FVX9k
> Ds1TwijB0
> > %2Bpe0xM4j2J9cZg872hBYA%3D&amp;reserved=0
> >
> > Fixes: 839b20be0e9b ("ethdev: support metadata as flow rule criteria")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> > ---
> >  app/test-pmd/config.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> > ba43be5..8fb18be 100644
> > --- a/app/test-pmd/config.c
> > +++ b/app/test-pmd/config.c
> > @@ -830,6 +830,14 @@
> >  			printf("off\n");
> >  	}
> >
> > +	if (dev_info.tx_offload_capa &
> DEV_TX_OFFLOAD_MATCH_METADATA) {
> > +		printf("TX match Flow metafata:        ");
> > +		if (ports[port_id].dev_conf.txmode.offloads &
> > +		    DEV_TX_OFFLOAD_MATCH_METADATA)
> > +			printf("on\n");
> > +		else
> > +			printf("off\n");
> > +	}
> >  }
> >
> >  int
> >
> 
> Hi Viacheslav,
> 
> There is already another testpmd command that displays offloads, which is
> more dynamic and I think better solution:
> "show port <port_id> tx_offload capabilities"
> "show port <port_id> tx_offload configuration"

Yes, it's implemented in dynamic approach and shows tx metadata offload cap/con correctly.
It is OK, no need to update this one.

> "show port <port_id> rx_offload capabilities"
> "show port <port_id> rx_offload configuration"
> 
> As far as I can see 'metadata' already supported by these commands, can
> you please confirm?

These commands are RX related, there is no any metadata offloads for RX.

> 
> And instead of improving it, what do you think dropping the duplicated
> command "show port cap <port_id>|all" ?

I use this command, it is shorter than the "show port 0 tx_offload capabilities", even with autocompletion.
Despite this, personally me is OK with drop. But someone else may use these commands also.

With best regards, Slava