From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <adrien.mazarguil@6wind.com>
Received: from mail-wm0-f53.google.com (mail-wm0-f53.google.com [74.125.82.53])
 by dpdk.org (Postfix) with ESMTP id A7A678E8C
 for <dev@dpdk.org>; Mon,  4 Jan 2016 12:38:33 +0100 (CET)
Received: by mail-wm0-f53.google.com with SMTP id f206so136978282wmf.0
 for <dev@dpdk.org>; Mon, 04 Jan 2016 03:38:33 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=6wind-com.20150623.gappssmtp.com; s=20150623;
 h=date:from:to:cc:subject:message-id:mail-followup-to:references
 :mime-version:content-type:content-disposition:in-reply-to;
 bh=mPLnRQF3QPUwO6MnZUKlnFXkWz66qPiRZ62Vj6LKDkQ=;
 b=grd4+1ur+5PMuvut+z2VkUf4OSkhM192bBGND10hsA9tIN4ITIiGAlOIDZkLnJVPeC
 1850A/4jRa5F0rJumlW+MRC1zKPSbHUbm/oOsxB81xARz+tyOH5p6J9Iz2MJI52Fuapd
 rAUJL7Kc5GPfyfFgqx6dRiQqFlhPgmXNZhdRl5PrPg4RUc5sfOj1NDtAPUxuTpBPDsLg
 1InY/U3Hw0QgOxaKVYq7UPSJyCpFAow6H8eL+tFpgS9qY83gSZEG9AQmQiQIHegdygxT
 EdHKs2QIYqfVtRb8pJf4uxkJkDqB4UYAvgRIlnsyyAnIeH2aB0hoQQMdV2dmGOJygzeB
 tsJw==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20130820;
 h=x-gm-message-state:date:from:to:cc:subject:message-id
 :mail-followup-to:references:mime-version:content-type
 :content-disposition:in-reply-to;
 bh=mPLnRQF3QPUwO6MnZUKlnFXkWz66qPiRZ62Vj6LKDkQ=;
 b=nA1lDVIVw0/dW4KBOfl7Ytz1LTVguyKpeSoU5Nk1YPVMzXYNyXmdd1C3IlIWB/zpLs
 oRNWW4G/1Yhs1kgVSlLfy+vZud6oXTnKY7KAFdCBA/IGIBdNSvYHWC4jzlwWXXRXuPMQ
 Ml2/MccUPxiEvGid7y7Rp5sCLiRJ0thbezX/VXxX2w9u2DaF8nUmySkbZMvh52CS2vv/
 TmwKUWOtqjeUBDL7ok0yaUwgxwP/2zizpeCKf/gIhRPTNncPff+BLURQrvl4RKF+mP8H
 HLTewyQDDS8Jvk0nsr52Z6QkGJTCmXcIT6n7/XsrNcMTVAiRAGvSTzg7fRA25bpXSAoA
 dDcw==
X-Gm-Message-State: ALoCoQmHfIfm4OpbXWKfYIFErcL3qxqncYE3Y4jzeO+vbqLcy2uSadiSkk+d/XQAQA5BQlLDpOc4C++Vq8laWQJG8giP9fuYPA==
X-Received: by 10.194.119.68 with SMTP id ks4mr94128716wjb.45.1451907513424;
 Mon, 04 Jan 2016 03:38:33 -0800 (PST)
Received: from 6wind.com (guy78-3-82-239-227-177.fbx.proxad.net.
 [82.239.227.177])
 by smtp.gmail.com with ESMTPSA id u62sm77598600wmg.4.2016.01.04.03.38.31
 (version=TLSv1/SSLv3 cipher=OTHER);
 Mon, 04 Jan 2016 03:38:32 -0800 (PST)
Date: Mon, 4 Jan 2016 12:38:14 +0100
From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
To: Jianfeng Tan <jianfeng.tan@intel.com>
Message-ID: <20160104113814.GT3806@6wind.com>
Mail-Followup-To: Jianfeng Tan <jianfeng.tan@intel.com>, dev@dpdk.org
References: <1451544799-70776-1-git-send-email-jianfeng.tan@intel.com>
 <1451544799-70776-2-git-send-email-jianfeng.tan@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1451544799-70776-2-git-send-email-jianfeng.tan@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 01/12] ethdev: add API to query what/if
 packet type is set
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Mon, 04 Jan 2016 11:38:34 -0000

I'm not sure about the usefulness of this new callback, but one issue I see
with rte_eth_dev_get_ptype_info() is that determining the proper size for
ptypes[] according to a mask is awkward. For instance suppose
RTE_PTYPE_L4_MASK is redefined to a different size at some point, the caller
must dynamically adjust its ptypes[] array size to avoid a possible
overflow, just in case.

I suggest one of these solutions:

- A callback to query for a single type at once instead (easiest method in
  my opinion).

- An additional argument with the number of entries in ptypes[], in which
  case rte_eth_dev_get_ptype_info() should return the number of entries that
  would have been filled regardless, a bit like snprintf().

On Thu, Dec 31, 2015 at 02:53:08PM +0800, Jianfeng Tan wrote:
> Add a new API rte_eth_dev_get_ptype_info to query what/if packet type will
> be set by current rx burst function.
> 
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> ---
>  lib/librte_ether/rte_ethdev.c | 12 ++++++++++++
>  lib/librte_ether/rte_ethdev.h | 22 ++++++++++++++++++++++
>  lib/librte_mbuf/rte_mbuf.h    | 13 +++++++++++++
>  3 files changed, 47 insertions(+)
> 
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index ed971b4..1885374 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -1614,6 +1614,18 @@ rte_eth_dev_info_get(uint8_t port_id, struct rte_eth_dev_info *dev_info)
>  	dev_info->driver_name = dev->data->drv_name;
>  }
>  
> +int
> +rte_eth_dev_get_ptype_info(uint8_t port_id, uint32_t ptype_mask,
> +		uint32_t ptypes[])
> +{
> +	struct rte_eth_dev *dev;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +	dev = &rte_eth_devices[port_id];
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_ptype_info_get, -ENOTSUP);
> +	return (*dev->dev_ops->dev_ptype_info_get)(dev, ptype_mask, ptypes);
> +}
> +
>  void
>  rte_eth_macaddr_get(uint8_t port_id, struct ether_addr *mac_addr)
>  {
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index bada8ad..e97b632 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -1021,6 +1021,10 @@ typedef void (*eth_dev_infos_get_t)(struct rte_eth_dev *dev,
>  				    struct rte_eth_dev_info *dev_info);
>  /**< @internal Get specific informations of an Ethernet device. */
>  
> +typedef int (*eth_dev_ptype_info_get_t)(struct rte_eth_dev *dev,
> +		uint32_t ptype_mask, uint32_t ptypes[]);
> +/**< @internal Get ptype info of eth_rx_burst_t. */
> +
>  typedef int (*eth_queue_start_t)(struct rte_eth_dev *dev,
>  				    uint16_t queue_id);
>  /**< @internal Start rx and tx of a queue of an Ethernet device. */
> @@ -1347,6 +1351,7 @@ struct eth_dev_ops {
>  	eth_queue_stats_mapping_set_t queue_stats_mapping_set;
>  	/**< Configure per queue stat counter mapping. */
>  	eth_dev_infos_get_t        dev_infos_get; /**< Get device info. */
> +	eth_dev_ptype_info_get_t   dev_ptype_info_get; /** Get ptype info */
>  	mtu_set_t                  mtu_set; /**< Set MTU. */
>  	vlan_filter_set_t          vlan_filter_set;  /**< Filter VLAN Setup. */
>  	vlan_tpid_set_t            vlan_tpid_set;      /**< Outer VLAN TPID Setup. */
> @@ -2273,6 +2278,23 @@ extern void rte_eth_dev_info_get(uint8_t port_id,
>  				 struct rte_eth_dev_info *dev_info);
>  
>  /**
> + * Retrieve the contextual information of an Ethernet device.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param ptype_mask
> + *   A hint of what kind of packet type which the caller is interested in
> + * @param ptypes
> + *   An array of packet types to be filled with
> + * @return
> + *   - (>=0) if successful. Indicate number of valid values in ptypes array.
> + *   - (-ENOTSUP) if hardware-assisted VLAN stripping not configured.
> + *   - (-ENODEV) if *port_id* invalid.
> + */
> +extern int rte_eth_dev_get_ptype_info(uint8_t port_id,
> +				 uint32_t ptype_mask, uint32_t ptypes[]);
> +
> +/**
>   * Retrieve the MTU of an Ethernet device.
>   *
>   * @param port_id
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index f234ac9..21d4aa2 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -282,6 +282,8 @@ extern "C" {
>   * It is used for outer packet for tunneling cases.
>   */
>  #define RTE_PTYPE_L2_MASK                   0x0000000f
> +
> +#define RTE_PTYPE_L2_MAX_NUM				4
>  /**
>   * IP (Internet Protocol) version 4 packet type.
>   * It is used for outer packet for tunneling cases, and does not contain any
> @@ -349,6 +351,8 @@ extern "C" {
>   * It is used for outer packet for tunneling cases.
>   */
>  #define RTE_PTYPE_L3_MASK                   0x000000f0
> +
> +#define RTE_PTYPE_L3_MAX_NUM				6
>  /**
>   * TCP (Transmission Control Protocol) packet type.
>   * It is used for outer packet for tunneling cases.
> @@ -435,6 +439,8 @@ extern "C" {
>   * It is used for outer packet for tunneling cases.
>   */
>  #define RTE_PTYPE_L4_MASK                   0x00000f00
> +
> +#define RTE_PTYPE_L4_MAX_NUM				6
>  /**
>   * IP (Internet Protocol) in IP (Internet Protocol) tunneling packet type.
>   *
> @@ -508,6 +514,8 @@ extern "C" {
>   * Mask of tunneling packet types.
>   */
>  #define RTE_PTYPE_TUNNEL_MASK               0x0000f000
> +
> +#define RTE_PTYPE_TUNNEL_MAX_NUM			6
>  /**
>   * Ethernet packet type.
>   * It is used for inner packet type only.
> @@ -527,6 +535,8 @@ extern "C" {
>   * Mask of inner layer 2 packet types.
>   */
>  #define RTE_PTYPE_INNER_L2_MASK             0x000f0000
> +
> +#define RTE_PTYPE_INNER_L2_MAX_NUM			2
>  /**
>   * IP (Internet Protocol) version 4 packet type.
>   * It is used for inner packet only, and does not contain any header option.
> @@ -588,6 +598,8 @@ extern "C" {
>   * Mask of inner layer 3 packet types.
>   */
>  #define RTE_PTYPE_INNER_L3_MASK             0x00f00000
> +
> +#define RTE_PTYPE_INNER_L3_MAX_NUM			6
>  /**
>   * TCP (Transmission Control Protocol) packet type.
>   * It is used for inner packet only.
> @@ -666,6 +678,7 @@ extern "C" {
>   */
>  #define RTE_PTYPE_INNER_L4_MASK             0x0f000000
>  
> +#define RTE_PTYPE_INNER_L4_MAX_NUM			6
>  /**
>   * Check if the (outer) L3 header is IPv4. To avoid comparing IPv4 types one by
>   * one, bit 4 is selected to be used for IPv4 only. Then checking bit 4 can
> -- 
> 2.1.4
> 

-- 
Adrien Mazarguil
6WIND