From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 6B25CA0C47;
	Tue,  6 Jul 2021 14:27:52 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id 4D5D541262;
	Tue,  6 Jul 2021 14:27:52 +0200 (CEST)
Received: from mail-pg1-f174.google.com (mail-pg1-f174.google.com
 [209.85.215.174])
 by mails.dpdk.org (Postfix) with ESMTP id 06FC14120E
 for <dev@dpdk.org>; Tue,  6 Jul 2021 14:27:51 +0200 (CEST)
Received: by mail-pg1-f174.google.com with SMTP id h4so21219358pgp.5
 for <dev@dpdk.org>; Tue, 06 Jul 2021 05:27:50 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;
 h=date:from:to:cc:subject:message-id:references:mime-version
 :content-disposition:in-reply-to;
 bh=w1Ta/NXtcV+dPwDRUzn+yZc/j3K0Rlm/1ARABTKaHxY=;
 b=RQHq000L+00I4WUZ7Du7EMlf1XXiiW84sCBcAIFSW833RSGy56jod/2UQXbbO6muSC
 +B+BijCeGDjvINDpKKn5vfD/R5ObmO8WH8uJMz+YE+IV4pHn1tlp/ABaA/SiqD5KKHgH
 ymRBzYnFykPIHOUvoF3s6R46FPTAyxiMfoZkjrH+ufAcJbpgtl6/N8PXaXt+QN5Z6jOw
 dMcGQk/ybnO7LbjixqKxKjK/6JzP1qbO/4T2XzNw58hfo/zjHD1llOUVJw/SqmWiIpa/
 ytWhpcfIU1kXcmD/TBc2rga+8T3g9q0Z4xeW9dQUrasyMNyyCl2zn/AZmET661gV6dlj
 ZzDg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:date:from:to:cc:subject:message-id:references
 :mime-version:content-disposition:in-reply-to;
 bh=w1Ta/NXtcV+dPwDRUzn+yZc/j3K0Rlm/1ARABTKaHxY=;
 b=Tmzlu3+AgN7ofmKXPMVz3+yAEG31a1qTAuEVkRfJmAy8mGlCHB5i6eiQbbP2wqXZ4K
 YYtWqeuOeKIGRBxuE2mTMOxr+J1dAET/PGbh0seR/BzdrMCbhHW2jYPbfji3On882NfL
 UROqwbJfVZH2YFiDmvVikm6eBS4TKuUq/QCuwvmJnOk/IdnLr2CuuSWzAlR7Ks0qy53w
 WStRaRGeL3q/Uq1+7TS5fgjbuj26fBw9Q2Q1FxiWNp6dfRjhQMXX9HDZSZBeZwaxBaEH
 93nBlEyJ/rHpt6ovAonFxV9EbLj/GKoc8x4eYBhipzCyS5bXnAzZjRkMMjsxL0MLcTQj
 U2Mg==
X-Gm-Message-State: AOAM530RURABP20wx8okaeOpS46NijPT+I1WTshCZkeyJ1YNp+j018co
 P++hpe4y1AqIkLZy1N3kguY=
X-Google-Smtp-Source: ABdhPJzaR7WRCDeVT1Il/G0IwjSuVLvllJ+CPcKL0M8+pHXwdXUIPC6RvXDMOQbqM2XkigPIgursYA==
X-Received: by 2002:aa7:9216:0:b029:2e5:6989:4f1a with SMTP id
 22-20020aa792160000b02902e569894f1amr19825269pfo.50.1625574470094; 
 Tue, 06 Jul 2021 05:27:50 -0700 (PDT)
Received: from gmail.com ([1.6.215.26])
 by smtp.gmail.com with ESMTPSA id c9sm14450603pja.7.2021.07.06.05.27.46
 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128);
 Tue, 06 Jul 2021 05:27:49 -0700 (PDT)
Date: Tue, 6 Jul 2021 17:57:20 +0530
From: Nithin Dabilpuram <nithind1988@gmail.com>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
Cc: Akhil Goyal <gakhil@marvell.com>, "dev@dpdk.org" <dev@dpdk.org>,
 "hemant.agrawal@nxp.com" <hemant.agrawal@nxp.com>,
 "thomas@monjalon.net" <thomas@monjalon.net>,
 "g.singh@nxp.com" <g.singh@nxp.com>,
 "Yigit, Ferruh" <ferruh.yigit@intel.com>,
 "Zhang, Roy Fan" <roy.fan.zhang@intel.com>,
 "olivier.matz@6wind.com" <olivier.matz@6wind.com>,
 "jerinj@marvell.com" <jerinj@marvell.com>
Message-ID: <YORL8sHOiuIed+Ey@gmail.com>
References: <20210624102848.3878788-1-gakhil@marvell.com>
 <DM6PR11MB449123EB3FD6B425A7F5E1C79A1B9@DM6PR11MB4491.namprd11.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <DM6PR11MB449123EB3FD6B425A7F5E1C79A1B9@DM6PR11MB4491.namprd11.prod.outlook.com>
Subject: Re: [dpdk-dev] [PATCH 1/2] security: enforce semantics for Tx
 inline processing
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
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>

On Tue, Jul 06, 2021 at 10:56:10AM +0000, Ananyev, Konstantin wrote:
> 
> > 
> > From: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > 
> > For Tx inline processing, when RTE_SECURITY_TX_OLOAD_NEED_MDATA is
> > set, rte_security_set_pkt_metadata() needs to be called for pkts
> > to associate a Security session with a mbuf before submitting
> > to Ethdev Tx. This is apart from setting PKT_TX_SEC_OFFLOAD in
> > mbuf.ol_flags. rte_security_set_pkt_metadata() is also used to
> > set some opaque metadata in mbuf for PMD's use.
> > This patch updates documentation that rte_security_set_pkt_metadata()
> > should be called only with mbuf containing Layer 3 and above data.
> > This behaviour is consistent with existing PMD's such as ixgbe.
> > 
> > On Tx, not all net PMD's/HW can parse packet and identify
> > L2 header and L3 header locations on Tx. This is inline with other
> > Tx offloads requirements such as L3 checksum, L4 checksum offload,
> > etc, where mbuf.l2_len, mbuf.l3_len etc, needs to be set for
> > HW to be able to generate checksum. Since Inline IPSec is also
> > such a Tx offload, some PMD's at least need mbuf.l2_len to be
> > valid to find L3 header and perform Outbound IPSec processing.
> > Hence, this patch updates documentation to enforce setting
> > mbuf.l2_len while setting PKT_TX_SEC_OFFLOAD in mbuf.ol_flags
> > for Inline IPSec Crypto / Protocol offload processing to
> > work on Tx.
> > 
> > Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > Reviewed-by: Akhil Goyal <gakhil@marvell.com>
> > ---
> >  doc/guides/nics/features.rst           | 2 ++
> >  doc/guides/prog_guide/rte_security.rst | 6 +++++-
> >  lib/mbuf/rte_mbuf_core.h               | 2 ++
> >  3 files changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
> > index 403c2b03a..414baf14f 100644
> > --- a/doc/guides/nics/features.rst
> > +++ b/doc/guides/nics/features.rst
> > @@ -430,6 +430,7 @@ of protocol operations. See Security library and PMD documentation for more deta
> > 
> >  * **[uses]       rte_eth_rxconf,rte_eth_rxmode**: ``offloads:DEV_RX_OFFLOAD_SECURITY``,
> >  * **[uses]       rte_eth_txconf,rte_eth_txmode**: ``offloads:DEV_TX_OFFLOAD_SECURITY``.
> > +* **[uses]       mbuf**: ``mbuf.l2_len``.
> >  * **[implements] rte_security_ops**: ``session_create``, ``session_update``,
> >    ``session_stats_get``, ``session_destroy``, ``set_pkt_metadata``, ``capabilities_get``.
> >  * **[provides] rte_eth_dev_info**: ``rx_offload_capa,rx_queue_offload_capa:DEV_RX_OFFLOAD_SECURITY``,
> > @@ -451,6 +452,7 @@ protocol operations. See security library and PMD documentation for more details
> > 
> >  * **[uses]       rte_eth_rxconf,rte_eth_rxmode**: ``offloads:DEV_RX_OFFLOAD_SECURITY``,
> >  * **[uses]       rte_eth_txconf,rte_eth_txmode**: ``offloads:DEV_TX_OFFLOAD_SECURITY``.
> > +* **[uses]       mbuf**: ``mbuf.l2_len``.
> >  * **[implements] rte_security_ops**: ``session_create``, ``session_update``,
> >    ``session_stats_get``, ``session_destroy``, ``set_pkt_metadata``, ``get_userdata``,
> >    ``capabilities_get``.
> > diff --git a/doc/guides/prog_guide/rte_security.rst b/doc/guides/prog_guide/rte_security.rst
> > index f72bc8a78..7b68c698d 100644
> > --- a/doc/guides/prog_guide/rte_security.rst
> > +++ b/doc/guides/prog_guide/rte_security.rst
> > @@ -560,7 +560,11 @@ created by the application is attached to the security session by the API
> > 
> >  For Inline Crypto and Inline protocol offload, device specific defined metadata is
> >  updated in the mbuf using ``rte_security_set_pkt_metadata()`` if
> > -``DEV_TX_OFFLOAD_SEC_NEED_MDATA`` is set.
> > +``RTE_SECURITY_TX_OLOAD_NEED_MDATA`` is set. ``rte_security_set_pkt_metadata()``
> > +should be called on mbuf only with Layer 3 and above data present and
> > +``mbuf.data_off`` should be pointing to Layer 3 Header.
> 
> Hmm... not sure why mbuf.data_off should point to L3 hdr.
> Who will add L2 hdr to the packet in that case?
> Or did you mean ``mbuf.data_off + mbuf.l2_len`` here?

That is the semantics I was trying to define. I think below are the sequence of
operations to be done for ipsec processing,

1. receive_pkt()
2. strip_l2_hdr()
3. Do policy lookup ()
4. Call rte_security_set_pkt_metadata() if pkt needs to be encrypted with a
particular SA. Now pkt only has L3 and above data.
5. Do route_lookup()
6. add_l2hdr() which might be different from stripped l2hdr.
7. Send packet out.

The above sequence is what I believe the current poll mode worker thread in
ipsec-secgw is following. While in event mode, step 2 and step 6 are missing.

This patch is trying to enforce semantics as above so that
rte_security_set_pkt_metadata() can predict what comes in the pkt when he is
called.

I also think above sequence is what Linux kernel stack or other stacks follow.
Does it makes sense ?

> 
> > Once called,
> > +Layer 3 and above data cannot be modified or moved around unless
> > +``rte_security_set_pkt_metadata()`` is called again.
> > 
> >  For inline protocol offloaded ingress traffic, the application can register a
> >  pointer, ``userdata`` , in the security session. When the packet is received,
> > diff --git a/lib/mbuf/rte_mbuf_core.h b/lib/mbuf/rte_mbuf_core.h
> > index bb38d7f58..9d8e3ddc8 100644
> > --- a/lib/mbuf/rte_mbuf_core.h
> > +++ b/lib/mbuf/rte_mbuf_core.h
> > @@ -228,6 +228,8 @@ extern "C" {
> > 
> >  /**
> >   * Request security offload processing on the TX packet.
> > + * To use Tx security offload, the user needs to fill l2_len in mbuf
> > + * indicating L2 header size and where L3 header starts.
> >   */
> >  #define PKT_TX_SEC_OFFLOAD	(1ULL << 43)
> > 
> > --
> > 2.25.1
>