From mboxrd@z Thu Jan  1 00:00:00 1970
From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Subject: Re: [PATCH v2 2/2] net/mlx4: refactor RSS conversion
 functions
Date: Tue, 22 May 2018 11:58:49 +0200
Message-ID: <20180522095849.GV6497@6wind.com>
References: <20180515154853.6361-1-adrien.mazarguil@6wind.com>
 <20180521154829.6297-1-adrien.mazarguil@6wind.com>
 <20180521154829.6297-2-adrien.mazarguil@6wind.com>
 <8025075e-0962-55f1-74ef-cee891647204@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Shahaf Shuler <shahafs@mellanox.com>, dev@dpdk.org,
 Ophir Munk <ophirmu@mellanox.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-wm0-f66.google.com (mail-wm0-f66.google.com [74.125.82.66])
 by dpdk.org (Postfix) with ESMTP id 06E80293C
 for <dev@dpdk.org>; Tue, 22 May 2018 11:59:05 +0200 (CEST)
Received: by mail-wm0-f66.google.com with SMTP id a67-v6so30051720wmf.3
 for <dev@dpdk.org>; Tue, 22 May 2018 02:59:04 -0700 (PDT)
Content-Disposition: inline
In-Reply-To: <8025075e-0962-55f1-74ef-cee891647204@intel.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://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: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Tue, May 22, 2018 at 10:41:35AM +0100, Ferruh Yigit wrote:
> On 5/21/2018 4:50 PM, Adrien Mazarguil wrote:
> > Since commit 97b2217ae5bc ("net/mlx4: advertise supported RSS hash
> > functions"), this PMD includes two similar-looking functions that convert
> > RSS hash fields between Verbs and DPDK formats.
> > 
> > This patch refactors them as a single two-way function and gets rid of
> > redundant helper macros.
> > 
> > Note the loss of the "static" keyword on the out[] (now verbs[]) array
> > introduced by commit cbd737416c34 ("net/mlx4: avoid constant recreations in
> > function") is what prevents the reliance on macro definitions for static
> > initializers at the expense of a few extra instructions. An acceptable
> > trade-off given this function is not involved in data plane operations.
<snip>
> > -	static const uint64_t out[RTE_DIM(in)] = {
> > +	const uint64_t verbs[RTE_DIM(dpdk)] = {
> > +		[INNER] = IBV_RX_HASH_INNER,
> >  		[IPV4] = IBV_RX_HASH_SRC_IPV4 | IBV_RX_HASH_DST_IPV4,
> > +		[IPV4_1] = verbs[IPV4],
> > +		[IPV4_2] = verbs[IPV4],
> >  		[IPV6] = IBV_RX_HASH_SRC_IPV6 | IBV_RX_HASH_DST_IPV6,
> > +		[IPV6_1] = verbs[IPV6],
> > +		[IPV6_2] = verbs[IPV6],
> > +		[IPV6_3] = verbs[IPV6],
> >  		[TCP] = IBV_RX_HASH_SRC_PORT_TCP | IBV_RX_HASH_DST_PORT_TCP,
> >  		[UDP] = IBV_RX_HASH_SRC_PORT_UDP | IBV_RX_HASH_DST_PORT_UDP,
> > +		[IPV4_TCP] = verbs[IPV4] | verbs[TCP],
> 
> This gives following build error with ICC [1]. Is it guarantied that in above
> assignment the executing order will be the same order with code?
> 
> [1]
> .../dpdk/drivers/net/mlx4/mlx4_flow.c(135): error #592: variable "verbs" is used
> before its value is set
>                 [IPV4_TCP] = verbs[IPV4] | verbs[TCP],

Didn't see this error with GCC nor clang (compilation and validation OK with
both), however ICC is correct, initialization order is not guaranteed
here. This shortcut is unsafe, I'll have to come up with something else.

-- 
Adrien Mazarguil
6WIND