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 4DB20A04DD; Mon, 26 Oct 2020 17:49:24 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 2C8D429C6; Mon, 26 Oct 2020 17:49:22 +0100 (CET) Received: from new3-smtp.messagingengine.com (new3-smtp.messagingengine.com [66.111.4.229]) by dpdk.org (Postfix) with ESMTP id CBEF81E2B for <dev@dpdk.org>; Mon, 26 Oct 2020 17:49:19 +0100 (CET) Received: from compute2.internal (compute2.nyi.internal [10.202.2.42]) by mailnew.nyi.internal (Postfix) with ESMTP id 7B9105805DB; Mon, 26 Oct 2020 12:49:19 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute2.internal (MEProxy); Mon, 26 Oct 2020 12:49:19 -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= 7e4vLPVTwdT5O30FrwK8SSNGjGfISOONtro/kiT09ck=; b=B3SR07/r397Vo9/F 06n1gtiCEIeRbOt1ZrWb5wg6MY2YROkj86ne9lxpf8u69f1RA5FTVAoKKjsrdXhW Zb2IUvoexk7N51XKD55RaMwh1yt+pPd0GH3aVdqrHejuFZj8JwpVviVHy2YS4Kw7 wYgHUAGB8ofeBO6FwBF2z/Qr1gA3QJ1ZeXBVO2x3tvFRDqunFcxX8ArwYUVJWLw5 OBxC0RPPilM8JKukkpWY5FlSIPTDNKfSLTSS2GueeGzW6EASBgXuqEvtq917ZqoD 1SDn4MUNlGJs+akTCttqXLoFIPc44RX86f1xIDnDtH4214PkwlI4T9vYDLgiilcM IrtqdA== 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=7e4vLPVTwdT5O30FrwK8SSNGjGfISOONtro/kiT09 ck=; b=MWTHHf6cPLz8xSpAy1JH8fP//b8LuJfjYLZ3zxpRmhCz6sCkY6fPSFXHC X5mWO/d9xIpnEt80FnPycNuWTC8EObc3pc2i1BRrN7hmXUiUybv5dR+w7YfiwqnF bu+a7MLJq6arnl0omlxSnZdRxKUYzRqw1DRQ9/KcTYppQsg0KTS5IF3oY8UInWoa MQ64l8SaRX/LmAdGqaBTb7TO64cEJ4QOc/OnUcdxFIdloF5pJ1z8L0alCua2T4Vu fc3f88sa5S19V4VaXo6j9rn8qu8CvblALvGf5VmSiKPOMEFvrQT97BEbmCAI6if6 5RcQ2jJXmsdjI5BvSxDTKQvrLijww== X-ME-Sender: <xms:Df6WXwdxh2Xsg8RMyv3tl4pHQ5M0gTFz2uYjsjjMZ-_8Fg_gB8II4A> <xme:Df6WXyOGoXbPlyNkGM5rA8W808CjHXQ82IvbB5iwpFzG0QCXtL7tqjyxHI8Xu7pOs g_56YzfKEGuo8wyqQ> X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedujedrkeejgdehjecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhmrghs ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucggtf frrghtthgvrhhnpedugefgvdefudfftdefgeelgffhueekgfffhfeujedtteeutdejueei iedvffegheenucfkphepjeejrddufeegrddvtdefrddukeegnecuvehluhhsthgvrhfuih iivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepthhhohhmrghssehmohhnjhgrlhho nhdrnhgvth X-ME-Proxy: <xmx:Df6WXxjn7Zpk6S7iKl5MazKK5CTIXXU-kUsIoNjFp8hdNekQWAhZRg> <xmx:Df6WX19h7tO_U1VQQ1o_2FGV_JsYr-zl4kvDSB3VYm2Fveq3y_aQeg> <xmx:Df6WX8uT0smUjDNXgQ0z6StSlgYJZJiSsKsyuY9mEBrO7FpC6Y0kEA> <xmx:D_6WXwMQ3gXfUlGQVxK4h9e88BWcav5aYClTH_D-4oD0vMlIlc01hg> Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id C245E3064682; Mon, 26 Oct 2020 12:49:15 -0400 (EDT) From: Thomas Monjalon <thomas@monjalon.net> To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> Cc: dev@dpdk.org, ferruh.yigit@intel.com, david.marchand@redhat.com, bruce.richardson@intel.com, olivier.matz@6wind.com, 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: Mon, 26 Oct 2020 17:49:14 +0100 Message-ID: <8861526.BORFLOU9qx@thomas> In-Reply-To: <13f99aa6-04d6-553f-0c3a-1d40ee5dbf65@oktetlabs.ru> References: <20201026052105.1561859-1-thomas@monjalon.net> <20201026052105.1561859-6-thomas@monjalon.net> <13f99aa6-04d6-553f-0c3a-1d40ee5dbf65@oktetlabs.ru> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH 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> 26/10/2020 16:06, Andrew Rybchenko: > On 10/26/20 8:20 AM, Thomas Monjalon wrote: > > --- a/drivers/net/ixgbe/ixgbe_ipsec.c > > +++ b/drivers/net/ixgbe/ixgbe_ipsec.c > > - (union ixgbe_crypto_tx_desc_md *)&m->udata64; > > + (union ixgbe_crypto_tx_desc_md *) > > + rte_security_dynfield(m); > > IMHO alignment looks a bit confusing here, may be add one more > TAB? OK, no opinion > [snip] > > - sa = pkt->userdata; > > + sa = RTE_MBUF_DYNFIELD(pkt, security_dynfield_offset, > > + struct ipsec_sa *); > > I think it should be de-reference above, i.e. > sa = (struct ipsec_sa *)*RTE_MBUF_DYNFIELD(pkt, > security_dynfield_offset, RTE_SECURITY_DYNFIELD_TYPE *); > or just > sa = *RTE_MBUF_DYNFIELD(pkt, security_dynfield_offset, struct ipsec_sa **); You're right something is wrong. I should add a dereference. > and why not rte_security_dynfield()? Because I need good reviews from you and David :) Will fix one and the other occurences. I've prepared a v2 using a macro, but I think I will follow your suggestion of having static inline functions. > It all looks very fragile. May be at least add > RTE_BUILD_BUG_ON(sizeof(RTE_SECURITY_DYNFIELD_TYPE) == > sizeof(ipsec_sa *)); > and similar checks when an application or a library does > lookup for a dynamic field. You mean adding this after the lookup in the app? > In general since lookup should not happen on data path, > may be lookup should return size of the field which must > be checked by the caller for consistency. It does. I can add a check. > Is it really different types of value in one example > application? It looks like it should be different > dynamic fields. Otherwise, I don't understand how > to work with it. Yes, udata64 was a trash bin used to store different kind of data, even inside librte_security. > In fact may be above is out of scope of the patch series... Yes, I think moving to a dedicated field is a first step. Second step (not by me) will be to split in different fields.