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

[Slava Ovsiienko <viacheslavo@mellanox.com>]

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Friday, July 19, 2019 15:47
> To: Slava Ovsiienko <viacheslavo@mellanox.com>; dev@dpdk.org
> Cc: bernard.iremonger@intel.com; stable@dpdk.org
> Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH] app/testpmd: fix display
> capabilities routine
> 
> On 7/19/2019 6:07 AM, Slava Ovsiienko wrote:
> > 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]
> >>>
> >> http://patch
> >>>
> >>
> 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.
> 
> As long as we have another command that has this support it is not an issue I
> think. And duplication is bad :)
> 
> Would you mind helping on this, and prepare a patch to remove this?
Sure, I would, with pleasure.

With best regards, Slava