From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 2EE36A04B5;
	Tue, 27 Oct 2020 17:11:08 +0100 (CET)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 74350593A;
	Tue, 27 Oct 2020 17:11:05 +0100 (CET)
Received: from new3-smtp.messagingengine.com (new3-smtp.messagingengine.com
 [66.111.4.229]) by dpdk.org (Postfix) with ESMTP id 02332378B
 for <dev@dpdk.org>; Tue, 27 Oct 2020 17:11:03 +0100 (CET)
Received: from compute2.internal (compute2.nyi.internal [10.202.2.42])
 by mailnew.nyi.internal (Postfix) with ESMTP id 8D85558013F;
 Tue, 27 Oct 2020 12:11:02 -0400 (EDT)
Received: from mailfrontend2 ([10.202.2.163])
 by compute2.internal (MEProxy); Tue, 27 Oct 2020 12:11:02 -0400
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h=
 from:to:cc:subject:date:message-id:in-reply-to:references
 :mime-version:content-transfer-encoding:content-type; s=fm2; bh=
 lmCZ2n+QV6GsQjI5iVS+DbHhrYhon/hJkbcRzYZVtks=; b=kwAtr56z+s6lReJI
 BOkG/i8YHxM69fU+SML4bxUK8bKPb4vLzWdS9sYJQAdbcTyAbzFkPTCkU/JX/JK+
 pRAC9c8B0VBf7v7lWL9Rdz7q8Q92ad6NOSMXhhNvF2+cL9lTW6i2ov4ySnHjhRe+
 eO8FFycO8Q8LfJ1WPPXmXHc6gyF4heqhCp7hSiyzFB2ssfJzhX9WCuLV5Rs8i2YL
 JcK4sSXkqiKmYfr/9NolXa1ISz+JIwya7lsi/pBaqqpny8xseq5bc7+7MxcCiUo7
 V2UXPbWZavyEe5b4hh6ZuioNUISNunFOEvhRdJRcdURr6zrUbxnxcgKfvWhcM72r
 ADJQWA==
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=
 messagingengine.com; h=cc:content-transfer-encoding:content-type
 :date:from:in-reply-to:message-id:mime-version:references
 :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender
 :x-sasl-enc; s=fm1; bh=lmCZ2n+QV6GsQjI5iVS+DbHhrYhon/hJkbcRzYZVt
 ks=; b=i4Kf6qeDTZTy5rpjB9z5aeBVkuFQDovJAA7gZ9sfi1isjfWPObDwXSB+M
 TQpwJCgaPV1wDAoU7ZSgdxrdadFoAXoJviY5FU9ptrREwJgR0+f2F7detmIFxcwu
 uZocVouFsKp9X1w43dZLG1zlJVw978rzy9rQruMg2/qlUzGDG3P0eiPCnlbrrQo5
 xWGvkdldkbZ0thUcg5kPvtLnmLdDyo1Y0oBqkA/2uJPt5XUkBjNkA3U2CLIe3G7M
 dStMiEceD2BaXXJ8wha6Ns+7rF9vO+GV4sgrNFZGjvMkkIfZlAmF//arI8lTnwgV
 vys/1GBioeSlTMH3JBXnpGmZEyeBQ==
X-ME-Sender: <xms:lEaYX2lQIZ_gI_YAOl8yn9F1qLjKY2lxWXdKpIzE_RX8b1bhiCKQsQ>
 <xme:lEaYX92uLyZEgLgZol3VyU8y9CsHBpyyp9dHo3Lc7dhw6LZSAlO2jauFmnywB06Dj
 j2wza0R6D2oxPzhGw>
X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedujedrkeelgdekgecutefuodetggdotefrodftvf
 curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu
 uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc
 fjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhmrghs
 ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucggtf
 frrghtthgvrhhnpedugefgvdefudfftdefgeelgffhueekgfffhfeujedtteeutdejueei
 iedvffegheenucfkphepjeejrddufeegrddvtdefrddukeegnecuvehluhhsthgvrhfuih
 iivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepthhhohhmrghssehmohhnjhgrlhho
 nhdrnhgvth
X-ME-Proxy: <xmx:lEaYX0pahOpcEcOGCUg8hqNCg3_Wkdouth60HSMvARnlVyPCyF2mqQ>
 <xmx:lEaYX6lZlHBvIK30o2XoEH1U3HoOFpnHxEaX4i6I5EALGc9G3czhBA>
 <xmx:lEaYX03Hx6l4L1pCkcg4R9SpT3o_Nw2gMKwphNSRndqWv0i_831Cqw>
 <xmx:lkaYX31IZDL_3AxXCewxjrF4VVfW5Y3Xp0AbJbrPgyLrseL0BBc0hw>
Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184])
 by mail.messagingengine.com (Postfix) with ESMTPA id D43903064684;
 Tue, 27 Oct 2020 12:10:58 -0400 (EDT)
From: Thomas Monjalon <thomas@monjalon.net>
To: Olivier Matz <olivier.matz@6wind.com>
Cc: dev@dpdk.org, ferruh.yigit@intel.com, david.marchand@redhat.com,
 bruce.richardson@intel.com, andrew.rybchenko@oktetlabs.ru, akhil.goyal@nxp.com,
 Declan Doherty <declan.doherty@intel.com>,
 Ankur Dwivedi <adwivedi@marvell.com>, Anoob Joseph <anoobj@marvell.com>,
 Jeff Guo <jia.guo@intel.com>, Haiyue Wang <haiyue.wang@intel.com>,
 Jerin Jacob <jerinj@marvell.com>, Nithin Dabilpuram <ndabilpuram@marvell.com>,
 Kiran Kumar K <kirankumark@marvell.com>, Radu Nicolau <radu.nicolau@intel.com>,
 Ray Kinsella <mdr@ashroe.eu>, Neil Horman <nhorman@tuxdriver.com>
Date: Tue, 27 Oct 2020 17:10:57 +0100
Message-ID: <2626035.iGO9lq9Oia@thomas>
In-Reply-To: <20201027100541.GO1898@platinum>
References: <20201026052105.1561859-1-thomas@monjalon.net>
 <20201026222013.2147904-6-thomas@monjalon.net>
 <20201027100541.GO1898@platinum>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Subject: Re: [dpdk-dev] [PATCH v2 05/15] security: switch metadata to
	dynamic mbuf field
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
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>

27/10/2020 11:05, Olivier Matz:
> On Mon, Oct 26, 2020 at 11:20:03PM +0100, Thomas Monjalon wrote:
> > The device-specific metadata was stored in the deprecated field udata64.
> > It is moved to a dynamic mbuf field in order to allow removal of udata64.
> > 
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> <...>
> >  			/* Save security session */
> > -			pkt->userdata = sess_tbl[port_id];
> > +			if (rte_security_dynfield_is_registered())
> > +				*(struct rte_security_session **)
> > +					rte_security_dynfield(pkt) =
> > +						sess_tbl[port_id];
> >  
> 
> Maybe the last 2 lines can be on the same line (a bit more than 80,
> but less than 100 chars).

I prefer limiting to 80 if possible.

> This is not clear to me why you need to call
> rte_security_dynfield_is_registered() here, and not in other places.

I think other places are paths for rte_security only.

> Would it make sense instead to always register the dynfield at some
> place in rte_security, so that we are sure that as soon as we use
> rte_security, the dynfield is registered. A good place would be an init
> function, but I don't see one.

Indeed there is no such place in rte_security.
I keep thinking this is not a real library.

> > +bool
> > +rte_security_dynfield_is_registered(void)
> > +{
> > +	return rte_security_dynfield_offset >= 0;
> > +}
> > +
> 
> Wouldn't it be better to have it in a static inline function?
> The point is just to check that offset is >= 0, and it is used
> in data path.

Yes I'll make it inline

> > +/** Device-specific metadata field type */
> > +#define RTE_SECURITY_DYNFIELD_TYPE uint64_t
> 
> What about using a typedef instead of a #define?
> It could be in lowercase: rte_security_dynfield_t

Yes