Received-SPF: pass (google.com: domain of linux-kernel+bounces-198829-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
From: Bharat Bhushan <bbhushan2@marvell.com>
To: Leon Romanovsky <leon@kernel.org>
CC: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Sunil Kovvuri
 Goutham <sgoutham@marvell.com>,
        Geethasowjanya Akula <gakula@marvell.com>,
        Subbaraya Sundeep Bhatta <sbhatta@marvell.com>,
        Hariprasad Kelam
	<hkelam@marvell.com>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "edumazet@google.com" <edumazet@google.com>,
        "kuba@kernel.org"
	<kuba@kernel.org>,
        "pabeni@redhat.com" <pabeni@redhat.com>,
        Jerin Jacob
	<jerinj@marvell.com>, Linu Cherian <lcherian@marvell.com>,
        "richardcochran@gmail.com" <richardcochran@gmail.com>
Subject: RE: [EXTERNAL] Re: [net-next,v3 5/8] cn10k-ipsec: Add SA add/delete
 support for outb inline ipsec
Thread-Topic: [EXTERNAL] Re: [net-next,v3 5/8] cn10k-ipsec: Add SA add/delete
 support for outb inline ipsec
Thread-Index: AQHasQaPDAjBHvZ8ekqyrRu0MLgk67Gv3zyAgAXjqBA=
Date: Mon, 3 Jun 2024 09:04:22 +0000
Message-ID: 
 <SN7PR18MB5314D6ADA32A16AFB778B4DDE3FF2@SN7PR18MB5314.namprd18.prod.outlook.com>
References: <20240528135349.932669-1-bbhushan2@marvell.com>
 <20240528135349.932669-6-bbhushan2@marvell.com>
 <20240530144959.GC3884@unreal>
In-Reply-To: <20240530144959.GC3884@unreal>
Accept-Language: en-IN, en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: 
 =?us-ascii?Q?q+BC98qYXwonCjPOzrY5rvBnxaE2H+vEj1n2ET6uAcSCxS27abDxIcqvV3J3?=
 =?us-ascii?Q?csXvahb/edhWVsMjhYxZ/l9D30nB0vMrIJGcikKG6eBsxvyKJr7U4l7hLAQC?=
 =?us-ascii?Q?7hAgmDzejGA8ldAOaB9g/ZoJ1IgFXmW0XkbRiN6VOB9cJZmpJ7EYHlWv6T1h?=
 =?us-ascii?Q?7tZiduo6+zPFW7mw5ZRVy/SkJr/JCTiDYSxaVPaKYmS1SagzLCXPPwGGLBtN?=
 =?us-ascii?Q?CTpiYtV4KjIKS22RhAQh+0mRTE99ouetZfwF0OO91CKeM8I1CU4VgFhOIHBL?=
 =?us-ascii?Q?AoSuGdmDqAxZzUfOTc5ZP3qQebFPFsOBUs0IdtEzhm7YWNIHXumoEFfoSDxd?=
 =?us-ascii?Q?KNKTj90WfEvjaXguksCmOO2fyS1RcvlUFzjEdrT3jEJYkUnCnmSDHMx1TFGk?=
 =?us-ascii?Q?utiSsrL/sQSR31vezcEwPLMXoB3mQXLQBhXq4u2a0nzU+MYjI8zmhYfRwYVT?=
 =?us-ascii?Q?mXcBWeOPkx3O4224Q6VHFv7bh5V2lERR36jVEXSihtxiw7QzfVM+tEBfwxwz?=
 =?us-ascii?Q?F69OAFdyV4h8DtapoblJ++f70/Ewoa+AzJmWKUNI9vgwzxxWzJDj8Ujn65IA?=
 =?us-ascii?Q?tOjFjAWi1UKAYCb/6V2IbhPgeFqc6Qw4udccZrGC2Hb3FqnoDMQxMzbuHqq6?=
 =?us-ascii?Q?ANIi5JF9Vs845KgkFqNVjgcrlS8RAgt+cxTwSgNQCxOACvV6x9jGqI7/cJ3J?=
 =?us-ascii?Q?8GoM4K03ZyFImeib2MtvrzXrUgE51G1jhVNOUsS44gsOYl5m732LsY2mq43R?=
 =?us-ascii?Q?H5R74AokDaBSKkwikSn5hr6iKDyRMLGw/A4IjRZK6bRHMMyr1ieWZPcWQzs9?=
 =?us-ascii?Q?GPsH3TxHJaQGNnSjLRBRP95QDv47SH7W37W2wPt0cp064T7yYaY0Hta3Hpu/?=
 =?us-ascii?Q?gqCWnkrvaAUX9GID/gMGtVV2NVtBcPTzswe5UQiO95JUN5qc19/mL9maULQV?=
 =?us-ascii?Q?2PQ/p6/mw7rLNR4aMeyjGlsgNQsgAcxlNbJ8wE4Rzm4+XlObp40eUE4SSCap?=
 =?us-ascii?Q?2mMUVXuD8vFcSaaJ310kMbaeVRdCaEZA1xHrZbI18MRfJtUd0Hcao3zJ09T8?=
 =?us-ascii?Q?VwJM/BszjtTqCyRTRFeMp87tS/jgDFYYgqRt84sC2M7W0tKgC5fZe5xBTwum?=
 =?us-ascii?Q?V6Fg5ZX7zvpjJ6EL69IX9z0N7MHgnuGCVphFcxYPzbBjsfO34A1fEntViUGE?=
 =?us-ascii?Q?z69vOcc8dOVh/LjY8lIM+i1Weqj4GmbRI+zoou2faguyCufU0H7JSArxH4PX?=
 =?us-ascii?Q?wn76uK1OiXHzKeSxSpkJalFApcpgXKSfbR527/oZQDmN0wRyG/aolnSSstE/?=
 =?us-ascii?Q?u4yQOOH/gcmdu8R43QtzAqYaYhecrXFcuJP2iGdw/HZ15IA9JEP4sPlv2KUg?=
 =?us-ascii?Q?rew7ul9i3vyLdCv9JhveX1ETKN1mLZAQcTW9UiE5nnevVHtZrHcYbH/sHYAH?=
 =?us-ascii?Q?mgHTXMOUAronBJZEp5cguVEDQYU+lhUHAcZssMMo7Xz4QKv0Bujq0BbVbQe/?=
 =?us-ascii?Q?2EruxJW7rKL0NclNQ8HVtTREHaqHWnONSkc3mRYoy7SB7+0Eq5vuJlTwcJ/l?=
 =?us-ascii?Q?6WvJnQQvtvdczaisTHAyiSFrwMh9QgiW5Ok9C2Yt?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: marvell.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SN7PR18MB5314.namprd18.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 95ad33f5-e826-4778-ed71-08dc83ac2935
X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Jun 2024 09:04:22.8660
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: uF9Kf3InSrUYqyWNm13jQ2NfO0dU10VspfU1p03PQV4+3N5RfSJpskI+JgNKyH7M7rijnI/zjt0PYncFtQxj6w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN9PR18MB4172
X-Proofpoint-GUID: _51ClP8jwzK5x2gRB5Q6-3w2QzEFaF86
X-Proofpoint-ORIG-GUID: _51ClP8jwzK5x2gRB5Q6-3w2QzEFaF86
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1039,Hydra:6.0.650,FMLib:17.12.28.16
 definitions=2024-06-03_05,2024-05-30_01,2024-05-17_01


> -----Original Message-----
> From: Leon Romanovsky <leon@kernel.org>
> Sent: Thursday, May 30, 2024 8:20 PM
> To: Bharat Bhushan <bbhushan2@marvell.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Sunil Kovvuri
> Goutham <sgoutham@marvell.com>; Geethasowjanya Akula
> <gakula@marvell.com>; Subbaraya Sundeep Bhatta <sbhatta@marvell.com>;
> Hariprasad Kelam <hkelam@marvell.com>; davem@davemloft.net;
> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; Jerin Jacob
> <jerinj@marvell.com>; Linu Cherian <lcherian@marvell.com>;
> richardcochran@gmail.com
> Subject: [EXTERNAL] Re: [net-next,v3 5/8] cn10k-ipsec: Add SA add/delete
> support for outb inline ipsec
>=20
> ----------------------------------------------------------------------
> On Tue, May 28, 2024 at 07:23:46PM +0530, Bharat Bhushan wrote:
> > This patch adds support to add and delete Security Association
> > (SA) xfrm ops. Hardware maintains SA context in memory allocated by
> > software. Each SA context is 128 byte aligned and size of each context
> > is multiple of 128-byte. Add support for transport and tunnel ipsec
> > mode, ESP protocol, aead aes-gcm-icv16, key size 128/192/256-bits with
> > 32bit salt.
> >
> > Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com>
> > ---
> > v2->v3:
> >  - Removed memset to zero wherever possible
> >   (comment from Kalesh Anakkur Purayil)
> >  - Corrected error hanlding when setting SA for inbound
> >    (comment from Kalesh Anakkur Purayil)
> >  - Move "netdev->xfrmdev_ops =3D &cn10k_ipsec_xfrmdev_ops;" to this pat=
ch
> >    This fix build error with W=3D1
> >
> >  .../marvell/octeontx2/nic/cn10k_ipsec.c       | 452 ++++++++++++++++++
> >  .../marvell/octeontx2/nic/cn10k_ipsec.h       | 114 +++++
> >  2 files changed, 566 insertions(+)
>=20
> <...>
>=20
> > +static int cn10k_ipsec_validate_state(struct xfrm_state *x) {
> > +	struct net_device *netdev =3D x->xso.dev;
> > +
> > +	if (x->props.aalgo !=3D SADB_AALG_NONE) {
> > +		netdev_err(netdev, "Cannot offload authenticated xfrm
> states\n");
> > +		return -EINVAL;
> > +	}
> > +	if (x->props.ealgo !=3D SADB_X_EALG_AES_GCM_ICV16) {
> > +		netdev_err(netdev, "Only AES-GCM-ICV16 xfrm state may be
> offloaded\n");
> > +		return -EINVAL;
> > +	}
> > +	if (x->props.calgo !=3D SADB_X_CALG_NONE) {
> > +		netdev_err(netdev, "Cannot offload compressed xfrm
> states\n");
> > +		return -EINVAL;
> > +	}
> > +	if (x->props.flags & XFRM_STATE_ESN) {
> > +		netdev_err(netdev, "Cannot offload ESN xfrm states\n");
> > +		return -EINVAL;
> > +	}
>=20
> I afraid that this check will cause for this offload to be unusable in re=
al life
> scenarios. It is hard to imagine that someone will use offload which requ=
ires
> rekeying every 2^32 packets.

I agree, Currently ESN offload is not enabled. Enabling END is in our list.=
=20

>=20
> > +	if (x->props.family !=3D AF_INET && x->props.family !=3D AF_INET6) {
> > +		netdev_err(netdev, "Only IPv4/v6 xfrm states may be
> offloaded\n");
> > +		return -EINVAL;
> > +	}
> > +	if (x->props.mode !=3D XFRM_MODE_TRANSPORT &&
> > +	    x->props.mode !=3D XFRM_MODE_TUNNEL) {
> > +		dev_info(&netdev->dev, "Only tunnel/transport xfrm states
> may be offloaded\n");
> > +		return -EINVAL;
> > +	}
> > +	if (x->id.proto !=3D IPPROTO_ESP) {
> > +		netdev_err(netdev, "Only ESP xfrm state may be
> offloaded\n");
> > +		return -EINVAL;
> > +	}
> > +	if (x->encap) {
> > +		netdev_err(netdev, "Encapsulated xfrm state may not be
> offloaded\n");
> > +		return -EINVAL;
> > +	}
> > +	if (!x->aead) {
> > +		netdev_err(netdev, "Cannot offload xfrm states without
> aead\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (x->aead->alg_icv_len !=3D 128) {
> > +		netdev_err(netdev, "Cannot offload xfrm states with AEAD
> ICV length other than 128bit\n");
> > +		return -EINVAL;
> > +	}
> > +	if (x->aead->alg_key_len !=3D 128 + 32 &&
> > +	    x->aead->alg_key_len !=3D 192 + 32 &&
> > +	    x->aead->alg_key_len !=3D 256 + 32) {
> > +		netdev_err(netdev, "Cannot offload xfrm states with AEAD
> key length other than 128/192/256bit\n");
> > +		return -EINVAL;
> > +	}
> > +	if (x->tfcpad) {
> > +		netdev_err(netdev, "Cannot offload xfrm states with tfc
> padding\n");
> > +		return -EINVAL;
> > +	}
> > +	if (!x->geniv) {
> > +		netdev_err(netdev, "Cannot offload xfrm states without
> geniv\n");
> > +		return -EINVAL;
> > +	}
> > +	if (strcmp(x->geniv, "seqiv")) {
> > +		netdev_err(netdev, "Cannot offload xfrm states with geniv
> other than seqiv\n");
> > +		return -EINVAL;
> > +	}
> > +	return 0;
> > +}
>=20
> I don't see check for supported offload type among these checks.
> if (x->xso.type !=3D XFRM_DEV_OFFLOAD_CRYPTO) {  ....

Will add the check.

Thanks
-Bharat