From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 46A2B42B47; Fri, 19 May 2023 10:08:00 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2227141148; Fri, 19 May 2023 10:08:00 +0200 (CEST) Received: from mail-ua1-f44.google.com (mail-ua1-f44.google.com [209.85.222.44]) by mails.dpdk.org (Postfix) with ESMTP id BB41740F16; Fri, 19 May 2023 10:07:58 +0200 (CEST) Received: by mail-ua1-f44.google.com with SMTP id a1e0cc1a2514c-783f7e82f28so898366241.1; Fri, 19 May 2023 01:07:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1684483678; x=1687075678; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=DN17cBFLQlhPM1n6ZlPQzWOc1w/nwNET+pa/siINifQ=; b=WgGtXNrw+Cko8uTUsZwlMNExkwAH8LEE7Pw6NYSMEgacyfGw+rZ/1CaGnn/dmJ72LY IX4ym/P1B4879GzFaSLx7PD2hFE7kIgYt16Uov8GSAC6Um/oyahCfph35V/not1zkDXQ Tf/pJNOpmWjOTvjfxZn/iSGHYUfaC3ap8+70HmP2xHtYWTDyr0YV45n/QgWWhRR2PEe2 /rmPlpzznZnrKw7zKO7BLvVlZx4w6KlSsxyJRVqnI6OGdX1RdSnq4bVEofISVO2qIcDt U3MN7lfBPXjS3c43NhBvoAK+YLhJLjhNmUm9S68HkfkfA0KGSAvs2ERyLphknRcZK3eo tMIQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684483678; x=1687075678; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=DN17cBFLQlhPM1n6ZlPQzWOc1w/nwNET+pa/siINifQ=; b=ZELWEriF1ejO9Op5GQxcwoHpMDkfSlTi0l9hXpbSf+bc9iqejEHLN0sQj2v7kvVEaX mxcn5lPpKjONTTScARYmSyvSbk51dFIGgyRQT/8Ox3BRg6HLbzlH+siK0GOZxd67U0ha 9TgaNG5GgyjMvotpIT1Jqu9+wpKGhgxCsTP1G+j6f1qoWK7f0DerjNWFqbfw319rrSvP 9uz7Grwu9LgnYSR/f6sdt+IMDxjLXdAkkN6aengGSjIWpX1HByDH+4ogUqi9pehHZ6X1 trVonryZpZMRUPNxmZ1F3eYMJfy6xaHKDCe7CSQWMkpyQrDQ9qZQN9oQiO4965cwkyun zYkg== X-Gm-Message-State: AC+VfDyTl94u0VBYEvez57P82gT/U6cBuq9/yX9QSTb5DBSQgXl2RtPF 17pxACqKpL/6AWN6zE/mGTLOk+05AuFYhAsRdgM= X-Google-Smtp-Source: ACHHUZ7iUCdF5j2URFmpNOdA0vWvUsSiGBJs41Bj46yTsmld9ycc+EVKefzreQIAJJncxkfbfLKZJE8ioE84SrSe+hM= X-Received: by 2002:a67:fe0d:0:b0:42f:ed31:b838 with SMTP id l13-20020a67fe0d000000b0042fed31b838mr341388vsr.30.1684483677871; Fri, 19 May 2023 01:07:57 -0700 (PDT) MIME-Version: 1.0 References: <20230309085645.1630826-1-ndabilpuram@marvell.com> <20230411110553.25f7c038@hermes.local> <5925463.UjTJXf6HLC@thomas> In-Reply-To: <5925463.UjTJXf6HLC@thomas> From: Jerin Jacob Date: Fri, 19 May 2023 13:37:31 +0530 Message-ID: Subject: Re: [PATCH 1/3] security: introduce out of place support for inline ingress To: Thomas Monjalon Cc: Stephen Hemminger , Nithin Dabilpuram , Akhil Goyal , jerinj@marvell.com, dev@dpdk.org, =?UTF-8?Q?Morten_Br=C3=B8rup?= , techboard@dpdk.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Tue, Apr 25, 2023 at 4:11=E2=80=AFAM Thomas Monjalon wrote: > > 18/04/2023 10:33, Jerin Jacob: > > On Tue, Apr 11, 2023 at 11:36=E2=80=AFPM Stephen Hemminger > > wrote: > > > > > > On Tue, 11 Apr 2023 15:34:07 +0530 > > > Nithin Dabilpuram wrote: > > > > > > > diff --git a/lib/security/rte_security.h b/lib/security/rte_securit= y.h > > > > index 4bacf9fcd9..866cd4e8ee 100644 > > > > --- a/lib/security/rte_security.h > > > > +++ b/lib/security/rte_security.h > > > > @@ -275,6 +275,17 @@ struct rte_security_ipsec_sa_options { > > > > */ > > > > uint32_t ip_reassembly_en : 1; > > > > > > > > + /** Enable out of place processing on inline inbound packets. > > > > + * > > > > + * * 1: Enable driver to perform Out-of-place(OOP) processing= for this inline > > > > + * inbound SA if supported by driver. PMD need to regist= er mbuf > > > > + * dynamic field using rte_security_oop_dynfield_registe= r() > > > > + * and security session creation would fail if dynfield = is not > > > > + * registered successfully. > > > > + * * 0: Disable OOP processing for this session (default). > > > > + */ > > > > + uint32_t ingress_oop : 1; > > > > + > > > > /** Reserved bit fields for future extension > > > > * > > > > * User should ensure reserved_opts is cleared as it may chan= ge in > > > > @@ -282,7 +293,7 @@ struct rte_security_ipsec_sa_options { > > > > * > > > > * Note: Reduce number of bits in reserved_opts for every new= option. > > > > */ > > > > - uint32_t reserved_opts : 17; > > > > + uint32_t reserved_opts : 16; > > > > }; > > > > > > NAK > > > Let me repeat the reserved bit rant. YAGNI > > > > > > Reserved space is not usable without ABI breakage unless the existing > > > code enforces that reserved space has to be zero. > > > > > > Just saying "User should ensure reserved_opts is cleared" is not enou= gh. > > > > Yes. I think, we need to enforce to have _init functions for the > > structures which is using reserved filed. > > > > On the same note on YAGNI, I am wondering why NOT introduce > > RTE_NEXT_ABI marco kind of scheme to compile out ABI breaking changes. > > By keeping RTE_NEXT_ABI disable by default, enable explicitly if user > > wants it to avoid waiting for one year any ABI breaking changes. > > There are a lot of "fixed appliance" customers (not OS distribution > > driven customer) they are willing to recompile DPDK for new feature. > > What we are loosing with this scheme? > > RTE_NEXT_ABI is described in the ABI policy. > We are not doing it currently, but I think we could > when it is not too much complicate in the code. > > The only problems I see are: > - more #ifdef clutter > - 2 binary versions to test > - CI and checks must handle RTE_NEXT_ABI version I think, we have two buckets of ABI breakages via RTE_NEXT_ABI 1) Changes that introduces compilation failures like adding new argument to API or change API name etc 2) Structure size change which won't affect the compilation but breaks the ABI for shared library usage. I think, (1) is very distributive, and I don't see recently such changes. I think, we should avoid (1) for non XX.11 releases.(or two or three-year cycles if we decide that path) The (2) comes are very common due to the fact HW features are evolving. I think, to address the (2), we have two options a) Have reserved fields and have _init() function to initialize the structu= res b) Follow YAGNI style and introduce RTE_NEXT_ABI for structure size change. The above concerns[1] can greatly reduce with option b OR option a. [1] 1) more #ifdef clutter For option (a) this is not needed or option (b) the clutter will be limited, it will be around structure which add the new filed and around the FULL block where new functions are added (not inside the functions) 2) 2 binary versions to test For option (a) this is not needed, for option (b) it is limited as for new features only one needs to test another binary (rather than NOT adding a new feature). 3) CI and checks must handle RTE_NEXT_ABI version I think, it is cheap to add this, at least for compilation test. IMO, We need to change the API break release to 3 year kind of time frame to have very good end user experience and allow ABI related change to get in every release and force _rebuild_ shared objects in major LTS release. I think, in this major LTS version(23.11) if we can decide (a) vs (b) then we can align the code accordingly . e.s.p for (a) we need to add _init() functions. Thoughts?