From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ferruh Yigit <ferruh.yigit@intel.com>
Subject: Re: [PATCH v2 3/3] app/testpmd: add debug command Tx
	metadata set
Date: Fri, 5 Oct 2018 14:19:36 +0100
Message-ID: <cac1e9ac-130b-3b15-b473-ba46e0430fb3@intel.com>
References: <1537104835-9034-1-git-send-email-dekelp@mellanox.com>
 <1537108670-11380-4-git-send-email-dekelp@mellanox.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Cc: shahafs@mellanox.com, orika@mellanox.com
To: Dekel Peled <dekelp@mellanox.com>, wenzhuo.lu@intel.com,
 jingjing.wu@intel.com, bernard.iremonger@intel.com, dev@dpdk.org,
 olivier.matz@6wind.com, adrien.mazarguil@6wind.com, thomas@monjalon.net,
 arybchenko@solarflare.com
Return-path: <dev-bounces@dpdk.org>
Received: from mga17.intel.com (mga17.intel.com [192.55.52.151])
 by dpdk.org (Postfix) with ESMTP id 3F5321B12D
 for <dev@dpdk.org>; Fri,  5 Oct 2018 15:23:20 +0200 (CEST)
In-Reply-To: <1537108670-11380-4-git-send-email-dekelp@mellanox.com>
Content-Language: en-US
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On 9/16/2018 3:37 PM, Dekel Peled wrote:
> As described in [1],[2] this series adds option to set metadata value as
> match pattern when creating a new flow rule.
> 
> This patch introduces code for debug porpuse only.
> The new debug command takes a 32 bit value and stores it per port.
> testpmd will add to any Tx packet sent from this port the metadata
> value, and set ol_flags accordingly.
> 
> [1] "ethdev: support metadata as flow rule criteria"
> [2] "app/testpmd: support metadata as flow rule criteria"
> 
> Signed-off-by: Dekel Peled <dekelp@mellanox.com>

<...>

> @@ -851,6 +851,13 @@ Disable hardware insertion of a VLAN header in packets sent on a port::
>  
>     testpmd> tx_vlan reset (port_id)
>  
> +tx_metadata set
> +~~~~~~~~~~~~~~~
> +
> +Set metadata value to insert in packets sent to PMD::
> +
> +   testpmd> tx_metadata set (port_id) (value)

My usual testpmd grunting:

Everybody is adding a high level command to testpmd, with own syntax and in the
scope of a specific feature, that nobody else knows about, it makes testpmd
confusing/hard to use.
If this is a command to set a port feature, why not extend existing,
"port config <port_id> ..."

Like "port config <port_id> tx_metadata <value>" ?


And second, I think it is good to have a set & show as pairs, this function sets
the "tx_metadata", is there a way to display existing "tx_metadata"? If not,
what about: "show port <port_id> tx_metadata"?