From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas.monjalon@6wind.com>
Subject: Re: [PATCH] ethdev: fix byte order inconsistence between
	fdir flow and mask
Date: Wed, 27 Jan 2016 10:19:36 +0100
Message-ID: <2497810.HTMk7hhgUj@xps13>
References: <1453883856-31246-1-git-send-email-jingjing.wu@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7Bit
Cc: dev@dpdk.org
To: Jingjing Wu <jingjing.wu@intel.com>, john.mcnamara@intel.com
Return-path: <dev-bounces@dpdk.org>
Received: from mail-wm0-f48.google.com (mail-wm0-f48.google.com [74.125.82.48])
 by dpdk.org (Postfix) with ESMTP id B48F6957F
 for <dev@dpdk.org>; Wed, 27 Jan 2016 10:20:45 +0100 (CET)
Received: by mail-wm0-f48.google.com with SMTP id r129so136892495wmr.0
 for <dev@dpdk.org>; Wed, 27 Jan 2016 01:20:45 -0800 (PST)
In-Reply-To: <1453883856-31246-1-git-send-email-jingjing.wu@intel.com>
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>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

2016-01-27 16:37, Jingjing Wu:
> Fixed issue of byte order in ethdev library that the structure
> for setting fdir's mask and flow entry is inconsist and made
> inputs of mask be in big endian.

Please be more precise. Which one is big endian?
Wasn't it tested before?

> fixes: 76c6f89e80d4 ("ixgbe: support new flow director masks")
>        2d4c1a9ea2ac ("ethdev: add new flow director masks")

Please put Fixes: on the two lines.

> --- a/doc/guides/rel_notes/release_2_3.rst
> +++ b/doc/guides/rel_notes/release_2_3.rst
> @@ -19,6 +19,10 @@ Drivers
>  Libraries
>  ~~~~~~~~~
>  
> +* ** fix byte order inconsistence between fdir flow and mask **
> +
> +  Fixed issue in ethdev library that the structure for setting
> +  fdir's mask and flow entry is inconsist in byte order.

John, comment on release notes formatting?
It's important to have the first items well formatted.

> @@ -39,6 +43,8 @@ API Changes
>  ABI Changes
>  -----------
>  
> +* The fields in  The ethdev structures ``rte_eth_fdir_masks`` were
> +  changed to be in big endian.

Please take care of uppercase typo here.

> -	/* write all the same so that UDP, TCP and SCTP use the same mask */
> +	/* write all the same so that UDP, TCP and SCTP use the same mask
> +	 * (little-endian)
> +	*/

Spacing typo here.
Sorry for the nits ;)

> -	uint8_t mac_addr_byte_mask;  /** Per byte MAC address mask */
> +	uint8_t mac_addr_byte_mask;  /** Bit mask for associated byte */
>  	uint32_t tunnel_id_mask;  /** tunnel ID mask */
> -	uint8_t tunnel_type_mask;
> +	uint8_t tunnel_type_mask; /**< 1 - Match tunnel type,
> +				       0 - Ignore tunnel type. */

These changes seem unrelated with the patch.
It's good to improve doc of this API but it's maybe not enough.
Example:
	uint8_t mac_addr_byte_mask;  /** Bit mask for associated byte */
Are we sure everybody understand how to fill it?