diff mbox series

[v4,03/10] security: add ESN field to ipsec_xform

Message ID 20210903112626.304692-4-radu.nicolau@intel.com (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series new features for ipsec and security libraries |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Radu Nicolau Sept. 3, 2021, 11:26 a.m. UTC 
  Update ipsec_xform definition to include ESN field.

Signed-off-by: Declan Doherty <declan.doherty@intel.com>
Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
Signed-off-by: Abhijit Sinha <abhijit.sinha@intel.com>
Signed-off-by: Daniel Martin Buckley <daniel.m.buckley@intel.com>
---
 lib/security/rte_security.h | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Fan Zhang Sept. 3, 2021, 12:50 p.m. UTC | #1 
> -----Original Message-----
> From: Nicolau, Radu <radu.nicolau@intel.com>
> Sent: Friday, September 3, 2021 12:26 PM
> To: Akhil Goyal <gakhil@marvell.com>; Doherty, Declan
> <declan.doherty@intel.com>
> Cc: dev@dpdk.org; mdr@ashroe.eu; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Medvedkin, Vladimir
> <vladimir.medvedkin@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>;
> hemant.agrawal@nxp.com; anoobj@marvell.com; Sinha, Abhijit
> <abhijit.sinha@intel.com>; Buckley, Daniel M <daniel.m.buckley@intel.com>;
> marchana@marvell.com; ktejasree@marvell.com; matan@nvidia.com;
> Nicolau, Radu <radu.nicolau@intel.com>
> Subject: [PATCH v4 03/10] security: add ESN field to ipsec_xform
> 
> Update ipsec_xform definition to include ESN field.
> 
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> Signed-off-by: Abhijit Sinha <abhijit.sinha@intel.com>
> Signed-off-by: Daniel Martin Buckley <daniel.m.buckley@intel.com>
> ---
>  lib/security/rte_security.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/lib/security/rte_security.h b/lib/security/rte_security.h
> index 03572b10ab..702de58b48 100644
> --- a/lib/security/rte_security.h
> +++ b/lib/security/rte_security.h
> @@ -240,6 +240,14 @@ struct rte_security_ipsec_xform {
>  	 */
>  	uint32_t mss;
>  	/**< IPsec payload Maximum Segment Size */
> +	union {
> +		uint64_t value;
> +		struct {
> +			uint32_t low;
> +			uint32_t hi;
> +		};
> +	} esn;
> +	/**< Extended Sequence Number */
>  };
> 
>  /**
> --
> 2.25.1

Acked-by: Fan Zhang <roy.fan.zhang@intel.com>
Akhil Goyal Sept. 5, 2021, 2:47 p.m. UTC | #2 
Hi Radu,

> ----------------------------------------------------------------------
> Update ipsec_xform definition to include ESN field.
> 
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> Signed-off-by: Abhijit Sinha <abhijit.sinha@intel.com>
> Signed-off-by: Daniel Martin Buckley <daniel.m.buckley@intel.com>
> ---
>  lib/security/rte_security.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/lib/security/rte_security.h b/lib/security/rte_security.h
> index 03572b10ab..702de58b48 100644
> --- a/lib/security/rte_security.h
> +++ b/lib/security/rte_security.h
> @@ -240,6 +240,14 @@ struct rte_security_ipsec_xform {
>  	 */
>  	uint32_t mss;
>  	/**< IPsec payload Maximum Segment Size */
> +	union {
> +		uint64_t value;
> +		struct {
> +			uint32_t low;
> +			uint32_t hi;
> +		};
> +	} esn;
> +	/**< Extended Sequence Number */
>  };

Can we use the following change for monitoring ESN?
http://patches.dpdk.org/project/dpdk/patch/1629207767-262-2-git-send-email-anoobj@marvell.com/

I believe ESN is not required to be set as SA parameter, it is normally
maintained by the PMD and application should be notified if a limit is reached.

Regards,
Akhil
Radu Nicolau Sept. 6, 2021, 11:21 a.m. UTC | #3 
On 9/5/2021 3:47 PM, Akhil Goyal wrote:
> Hi Radu,
>
>> ----------------------------------------------------------------------
>> Update ipsec_xform definition to include ESN field.
>>
>> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
>> Signed-off-by: Abhijit Sinha <abhijit.sinha@intel.com>
>> Signed-off-by: Daniel Martin Buckley <daniel.m.buckley@intel.com>
>> ---
>>   lib/security/rte_security.h | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/lib/security/rte_security.h b/lib/security/rte_security.h
>> index 03572b10ab..702de58b48 100644
>> --- a/lib/security/rte_security.h
>> +++ b/lib/security/rte_security.h
>> @@ -240,6 +240,14 @@ struct rte_security_ipsec_xform {
>>   	 */
>>   	uint32_t mss;
>>   	/**< IPsec payload Maximum Segment Size */
>> +	union {
>> +		uint64_t value;
>> +		struct {
>> +			uint32_t low;
>> +			uint32_t hi;
>> +		};
>> +	} esn;
>> +	/**< Extended Sequence Number */
>>   };
> Can we use the following change for monitoring ESN?
> http://patches.dpdk.org/project/dpdk/patch/1629207767-262-2-git-send-email-anoobj@marvell.com/
>
> I believe ESN is not required to be set as SA parameter, it is normally
> maintained by the PMD and application should be notified if a limit is reached.
>
> Regards,
> Akhil

Hi Akhil, I suppose they can be complementary, with this one being a 
hard ESN limit that the user can enforce by setting the initial ESN 
value - but there is no requirement to do so. Also, this change doesn't 
need explicit support added in the PMDs.
Anoob Joseph Sept. 6, 2021, 11:36 a.m. UTC | #4 
Hi Radu,

> Hi Akhil, I suppose they can be complementary, with this one being a hard
> ESN limit that the user can enforce by setting the initial ESN value - but there
> is no requirement to do so. Also, this change doesn't need explicit support
> added in the PMDs.

What is the actual use case of this field (ESN)? My impression was this is to allow application to control sequence number. For normal use cases, it can be like starting sequence number. And this can be used with ``rte_security_session_update`` to allow simulating corner cases (like large anti-replay windows sizes with ESN enabled etc). Did I capture the intended use case correctly?

If it is to set max sequence number to be handled by the session, then I guess, this is getting addressed as part of SA lifetime spec proposal.

Can you confirm what is the intended use case?

Thanks,
Anoob

> -----Original Message-----
> From: Nicolau, Radu <radu.nicolau@intel.com>
> Sent: Monday, September 6, 2021 4:51 PM
> To: Akhil Goyal <gakhil@marvell.com>; Declan Doherty
> <declan.doherty@intel.com>
> Cc: dev@dpdk.org; mdr@ashroe.eu; konstantin.ananyev@intel.com;
> vladimir.medvedkin@intel.com; bruce.richardson@intel.com;
> roy.fan.zhang@intel.com; hemant.agrawal@nxp.com; Anoob Joseph
> <anoobj@marvell.com>; abhijit.sinha@intel.com;
> daniel.m.buckley@intel.com; Archana Muniganti <marchana@marvell.com>;
> Tejasree Kondoj <ktejasree@marvell.com>; matan@nvidia.com
> Subject: Re: [EXT] [PATCH v4 03/10] security: add ESN field to ipsec_xform
> 
> 
> On 9/5/2021 3:47 PM, Akhil Goyal wrote:
> > Hi Radu,
> >
> >> ---------------------------------------------------------------------
> >> - Update ipsec_xform definition to include ESN field.
> >>
> >> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
> >> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> >> Signed-off-by: Abhijit Sinha <abhijit.sinha@intel.com>
> >> Signed-off-by: Daniel Martin Buckley <daniel.m.buckley@intel.com>
> >> ---
> >>   lib/security/rte_security.h | 8 ++++++++
> >>   1 file changed, 8 insertions(+)
> >>
> >> diff --git a/lib/security/rte_security.h
> >> b/lib/security/rte_security.h index 03572b10ab..702de58b48 100644
> >> --- a/lib/security/rte_security.h
> >> +++ b/lib/security/rte_security.h
> >> @@ -240,6 +240,14 @@ struct rte_security_ipsec_xform {
> >>   	 */
> >>   	uint32_t mss;
> >>   	/**< IPsec payload Maximum Segment Size */
> >> +	union {
> >> +		uint64_t value;
> >> +		struct {
> >> +			uint32_t low;
> >> +			uint32_t hi;
> >> +		};
> >> +	} esn;
> >> +	/**< Extended Sequence Number */
> >>   };
> > Can we use the following change for monitoring ESN?
> > https://urldefense.proofpoint.com/v2/url?u=http-
> 3A__patches.dpdk.org_p
> > roject_dpdk_patch_1629207767-2D262-2D2-2Dgit-2Dsend-2Demail-
> 2Danoobj-4
> >
> 0marvell.com_&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=jPfB8rwwviRS
> xyLWs2n6
> > B-
> WYLn1v9SyTMrT5EQqh2TU&m=u4ceKpeCwgpmKFhuny3rjUzauRZVfhlNdxm
> Cy95gHMs&
> > s=OshWh8UBWrxO0abYCUCBhRZBzj423rwddyfzB9Q9rT0&e=
> >
> > I believe ESN is not required to be set as SA parameter, it is
> > normally maintained by the PMD and application should be notified if a limit
> is reached.
> >
> > Regards,
> > Akhil
> 
> Hi Akhil, I suppose they can be complementary, with this one being a hard
> ESN limit that the user can enforce by setting the initial ESN value - but there
> is no requirement to do so. Also, this change doesn't need explicit support
> added in the PMDs.
>
Radu Nicolau Sept. 6, 2021, 1:39 p.m. UTC | #5 
On 9/6/2021 12:36 PM, Anoob Joseph wrote:
> Hi Radu,
>
>> Hi Akhil, I suppose they can be complementary, with this one being a hard
>> ESN limit that the user can enforce by setting the initial ESN value - but there
>> is no requirement to do so. Also, this change doesn't need explicit support
>> added in the PMDs.
> What is the actual use case of this field (ESN)? My impression was this is to allow application to control sequence number. For normal use cases, it can be like starting sequence number. And this can be used with ``rte_security_session_update`` to allow simulating corner cases (like large anti-replay windows sizes with ESN enabled etc). Did I capture the intended use case correctly?
>
> If it is to set max sequence number to be handled by the session, then I guess, this is getting addressed as part of SA lifetime spec proposal.
>
> Can you confirm what is the intended use case?
>
> Thanks,
> Anoob

Hi Anoob, the purpose was to have a starting value controlled by the app 
and I think you're right, it can be achieved with 
rte_security_session_update.

Thanks,
Radu
Anoob Joseph Sept. 6, 2021, 1:50 p.m. UTC | #6 
Hi Radu, Akhil,

Please see inline

Thanks,
Anoob

> 
> On 9/6/2021 12:36 PM, Anoob Joseph wrote:
> > Hi Radu,
> >
> >> Hi Akhil, I suppose they can be complementary, with this one being a
> >> hard ESN limit that the user can enforce by setting the initial ESN
> >> value - but there is no requirement to do so. Also, this change
> >> doesn't need explicit support added in the PMDs.
> > What is the actual use case of this field (ESN)? My impression was this is to
> allow application to control sequence number. For normal use cases, it can be
> like starting sequence number. And this can be used with
> ``rte_security_session_update`` to allow simulating corner cases (like large
> anti-replay windows sizes with ESN enabled etc). Did I capture the intended
> use case correctly?
> >
> > If it is to set max sequence number to be handled by the session, then I
> guess, this is getting addressed as part of SA lifetime spec proposal.
> >
> > Can you confirm what is the intended use case?
> >
> > Thanks,
> > Anoob
> 
> Hi Anoob, the purpose was to have a starting value controlled by the app and
> I think you're right, it can be achieved with rte_security_session_update.
> 

[Anoob] Thanks for the confirmation. In that case, I'm in agreement with this proposal. May be update the patch description to better explain the use case.

Acked-by: Anoob Joseph <anoobj@marvell.com>
diff mbox series

Patch

diff --git a/lib/security/rte_security.h b/lib/security/rte_security.h
index 03572b10ab..702de58b48 100644
--- a/lib/security/rte_security.h
+++ b/lib/security/rte_security.h
@@ -240,6 +240,14 @@  struct rte_security_ipsec_xform {
 	 */
 	uint32_t mss;
 	/**< IPsec payload Maximum Segment Size */
+	union {
+		uint64_t value;
+		struct {
+			uint32_t low;
+			uint32_t hi;
+		};
+	} esn;
+	/**< Extended Sequence Number */
 };
 
 /**