diff mbox series

[v6,02/10] security: add UDP params for IPsec NAT-T

Message ID 20210917091747.1528262-3-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. 17, 2021, 9:17 a.m. UTC 
  Add support for specifying UDP port params for UDP encapsulation option.
RFC3948 section-2.1 does not enforce using specific the UDP ports for
UDP-Encapsulated ESP Header

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>
Acked-by: Fan Zhang <roy.fan.zhang@intel.com>
---
 lib/security/rte_security.h | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Ananyev, Konstantin Sept. 23, 2021, 12:43 p.m. UTC | #1 
> Add support for specifying UDP port params for UDP encapsulation option.
> RFC3948 section-2.1 does not enforce using specific the UDP ports for
> UDP-Encapsulated ESP Header
> 
> 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>
> Acked-by: Fan Zhang <roy.fan.zhang@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 495a228915..84ba1b08f8 100644
> --- a/lib/security/rte_security.h
> +++ b/lib/security/rte_security.h
> @@ -112,6 +112,12 @@ struct rte_security_ipsec_tunnel_param {
>  	};
>  };
> 
> +struct rte_security_ipsec_udp_param {
> +
> +	uint16_t sport;
> +	uint16_t dport;
> +};

Would it be worth to have ability to access 32-bits at once.
Something like:
union rte_security_ipsec_udp_param {
	uint32_t raw;
	struct {
		uint16_t sport, dport;
	};
};
?

> +
>  /**
>   * IPsec Security Association option flags
>   */
> @@ -224,6 +230,8 @@ struct rte_security_ipsec_xform {
>  	/**< IPsec SA Mode - transport/tunnel */
>  	struct rte_security_ipsec_tunnel_param tunnel;
>  	/**< Tunnel parameters, NULL for transport mode */
> +	struct rte_security_ipsec_udp_param udp;
> +	/**< UDP parameters, ignored when udp_encap option not specified */

Any reason to insert it into the middle of the xform struct?
Why not to the end?

>  	uint64_t esn_soft_limit;
>  	/**< ESN for which the overflow event need to be raised */
>  	uint32_t replay_win_sz;
> --
> 2.25.1
Radu Nicolau Sept. 27, 2021, 12:14 p.m. UTC | #2 
On 9/23/2021 1:43 PM, Ananyev, Konstantin wrote:
>> Add support for specifying UDP port params for UDP encapsulation option.
>> RFC3948 section-2.1 does not enforce using specific the UDP ports for
>> UDP-Encapsulated ESP Header
>>
>> 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>
>> Acked-by: Fan Zhang <roy.fan.zhang@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 495a228915..84ba1b08f8 100644
>> --- a/lib/security/rte_security.h
>> +++ b/lib/security/rte_security.h
>> @@ -112,6 +112,12 @@ struct rte_security_ipsec_tunnel_param {
>>   	};
>>   };
>>
>> +struct rte_security_ipsec_udp_param {
>> +
>> +	uint16_t sport;
>> +	uint16_t dport;
>> +};
> Would it be worth to have ability to access 32-bits at once.
> Something like:
> union rte_security_ipsec_udp_param {
> 	uint32_t raw;
> 	struct {
> 		uint16_t sport, dport;
> 	};
> };
> ?

TBH I don't see any reason to access them as a 32b value...


>
>> +
>>   /**
>>    * IPsec Security Association option flags
>>    */
>> @@ -224,6 +230,8 @@ struct rte_security_ipsec_xform {
>>   	/**< IPsec SA Mode - transport/tunnel */
>>   	struct rte_security_ipsec_tunnel_param tunnel;
>>   	/**< Tunnel parameters, NULL for transport mode */
>> +	struct rte_security_ipsec_udp_param udp;
>> +	/**< UDP parameters, ignored when udp_encap option not specified */
> Any reason to insert it into the middle of the xform struct?
> Why not to the end?
I can't see any good reason I guess it just looked better, I will move 
it at the end.
diff mbox series

Patch

diff --git a/lib/security/rte_security.h b/lib/security/rte_security.h
index 495a228915..84ba1b08f8 100644
--- a/lib/security/rte_security.h
+++ b/lib/security/rte_security.h
@@ -112,6 +112,12 @@  struct rte_security_ipsec_tunnel_param {
 	};
 };
 
+struct rte_security_ipsec_udp_param {
+
+	uint16_t sport;
+	uint16_t dport;
+};
+
 /**
  * IPsec Security Association option flags
  */
@@ -224,6 +230,8 @@  struct rte_security_ipsec_xform {
 	/**< IPsec SA Mode - transport/tunnel */
 	struct rte_security_ipsec_tunnel_param tunnel;
 	/**< Tunnel parameters, NULL for transport mode */
+	struct rte_security_ipsec_udp_param udp;
+	/**< UDP parameters, ignored when udp_encap option not specified */
 	uint64_t esn_soft_limit;
 	/**< ESN for which the overflow event need to be raised */
 	uint32_t replay_win_sz;