api: ipsec: factor out IP protocol version parameter

Message ID 1493827207-19809-2-git-send-email-odpbot@yandex.ru
State New
Headers show

Commit Message

Github ODP bot May 3, 2017, 4 p.m.
From: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org>


Both tunnel and lookup parameters refer IP protocol version. Factor that
out as an IPsec enum used in both places.

Signed-off-by: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org>

---
/** Email created from pull request 20 (lumag:ipsec-ipv)
 ** https://github.com/Linaro/odp/pull/20
 ** Patch: https://github.com/Linaro/odp/pull/20.patch
 ** Base sha: 467285f59991e0f28d22a50c99e56f07a4380b8d
 ** Merge commit sha: cd17da2593db2313648bb4b462e751dae1b19659
 **/
 include/odp/api/spec/ipsec.h | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

Comments

Dmitry Eremin-Solenikov May 3, 2017, 4:18 p.m. | #1
On 03.05.2017 19:00, Github ODP bot wrote:
> From: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org>

> 

> Both tunnel and lookup parameters refer IP protocol version. Factor that

> out as an IPsec enum used in both places.


Subject prefixes are still not correct.

-- 
With best wishes
Dmitry
Maxim Uvarov May 3, 2017, 7:55 p.m. | #2
On 05/03/17 19:18, Dmitry Eremin-Solenikov wrote:
> On 03.05.2017 19:00, Github ODP bot wrote:

>> From: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org>

>>

>> Both tunnel and lookup parameters refer IP protocol version. Factor that

>> out as an IPsec enum used in both places.

> 

> Subject prefixes are still not correct.

> 

yes, did fixes. Hope it's fixed now.

Maxim.
Savolainen, Petri (Nokia - FI/Espoo) May 5, 2017, 10:12 a.m. | #3
> -----Original Message-----

> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

> Github ODP bot

> Sent: Wednesday, May 03, 2017 7:00 PM

> To: lng-odp@lists.linaro.org

> Subject: [lng-odp] [PATCH] api: ipsec: factor out IP protocol version

> parameter

> 

> From: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org>

> 

> Both tunnel and lookup parameters refer IP protocol version. Factor that

> out as an IPsec enum used in both places.

> 

> Signed-off-by: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org>

> ---

> /** Email created from pull request 20 (lumag:ipsec-ipv)

>  ** https://github.com/Linaro/odp/pull/20

>  ** Patch: https://github.com/Linaro/odp/pull/20.patch

>  ** Base sha: 467285f59991e0f28d22a50c99e56f07a4380b8d

>  ** Merge commit sha: cd17da2593db2313648bb4b462e751dae1b19659

>  **/

>  include/odp/api/spec/ipsec.h | 21 +++++++++------------

>  1 file changed, 9 insertions(+), 12 deletions(-)

> 

> diff --git a/include/odp/api/spec/ipsec.h b/include/odp/api/spec/ipsec.h

> index e83494d..7b3b1fe 100644

> --- a/include/odp/api/spec/ipsec.h

> +++ b/include/odp/api/spec/ipsec.h

> @@ -339,16 +339,16 @@ typedef enum odp_ipsec_protocol_t {

>  } odp_ipsec_protocol_t;

> 

>  /**

> - * IPSEC tunnel type

> + * IPSEC header type

>   */

> -typedef enum odp_ipsec_tunnel_type_t {

> -	/** Outer header is IPv4 */

> -	ODP_IPSEC_TUNNEL_IPV4 = 0,

> +typedef enum odp_ipsec_header_type_t {

> +	/** Header is IPv4 */

> +	ODP_IPSEC_IPV4 = 0,

> 

> -	/** Outer header is IPv6 */

> -	ODP_IPSEC_TUNNEL_IPV6

> +	/** Header is IPv6 */

> +	ODP_IPSEC_IPV6

> 

> -} odp_ipsec_tunnel_type_t;

> +} odp_ipsec_header_type_t;


Keep the current tunnel type. That way it's possible to extend it later with new tunnel types if needed.

> 

>  /**

>   * IPSEC crypto parameters

> @@ -378,7 +378,7 @@ typedef struct odp_ipsec_crypto_param_t {

>   */

>  typedef struct odp_ipsec_tunnel_param_t {

>  	/** Tunnel type: IPv4 or IPv6 */

> -	odp_ipsec_tunnel_type_t type;

> +	odp_ipsec_header_type_t type;

> 

>  	/** Variant mappings for tunnel parameters */

>  	union {

> @@ -613,11 +613,8 @@ typedef struct odp_ipsec_sa_param_t {

>  	 *  only in ODP_IPSEC_LOOKUP_DSTADDR_SPI lookup mode. */

>  	struct {

>  		/** Select IP version

> -		 *

> -		 *  4:   IPv4

> -		 *  6:   IPv6

>  		 */

> -		uint8_t ip_version;

> +		odp_ipsec_header_type_t ip_version;


Name the type as IP header version enum. E.g.

odp_ipsec_ip_version_t 

The down side of this is that enum is int vs. uint8_t is one byte.

-Petri


> 

>  		/** IP destination address (NETWORK ENDIAN) */

>  		void    *dst_addr;
Dmitry Eremin-Solenikov May 5, 2017, 9:04 p.m. | #4
On 05.05.2017 13:12, Savolainen, Petri (Nokia - FI/Espoo) wrote:
> 

> 

>> -----Original Message-----

>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of

>> Github ODP bot

>> Sent: Wednesday, May 03, 2017 7:00 PM

>> To: lng-odp@lists.linaro.org

>> Subject: [lng-odp] [PATCH] api: ipsec: factor out IP protocol version

>> parameter

>>

>> From: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org>

>>

>> Both tunnel and lookup parameters refer IP protocol version. Factor that

>> out as an IPsec enum used in both places.

>>

>> Signed-off-by: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org>

>> ---

>> /** Email created from pull request 20 (lumag:ipsec-ipv)

>>  ** https://github.com/Linaro/odp/pull/20

>>  ** Patch: https://github.com/Linaro/odp/pull/20.patch

>>  ** Base sha: 467285f59991e0f28d22a50c99e56f07a4380b8d

>>  ** Merge commit sha: cd17da2593db2313648bb4b462e751dae1b19659

>>  **/

>>  include/odp/api/spec/ipsec.h | 21 +++++++++------------

>>  1 file changed, 9 insertions(+), 12 deletions(-)

>>

>> diff --git a/include/odp/api/spec/ipsec.h b/include/odp/api/spec/ipsec.h

>> index e83494d..7b3b1fe 100644

>> --- a/include/odp/api/spec/ipsec.h

>> +++ b/include/odp/api/spec/ipsec.h

>> @@ -339,16 +339,16 @@ typedef enum odp_ipsec_protocol_t {

>>  } odp_ipsec_protocol_t;

>>

>>  /**

>> - * IPSEC tunnel type

>> + * IPSEC header type

>>   */

>> -typedef enum odp_ipsec_tunnel_type_t {

>> -	/** Outer header is IPv4 */

>> -	ODP_IPSEC_TUNNEL_IPV4 = 0,

>> +typedef enum odp_ipsec_header_type_t {

>> +	/** Header is IPv4 */

>> +	ODP_IPSEC_IPV4 = 0,

>>

>> -	/** Outer header is IPv6 */

>> -	ODP_IPSEC_TUNNEL_IPV6

>> +	/** Header is IPv6 */

>> +	ODP_IPSEC_IPV6

>>

>> -} odp_ipsec_tunnel_type_t;

>> +} odp_ipsec_header_type_t;

> 

> Keep the current tunnel type. That way it's possible to extend it later with new tunnel types if needed.


Ack.

>>  /**

>>   * IPSEC crypto parameters

>> @@ -378,7 +378,7 @@ typedef struct odp_ipsec_crypto_param_t {

>>   */

>>  typedef struct odp_ipsec_tunnel_param_t {

>>  	/** Tunnel type: IPv4 or IPv6 */

>> -	odp_ipsec_tunnel_type_t type;

>> +	odp_ipsec_header_type_t type;

>>

>>  	/** Variant mappings for tunnel parameters */

>>  	union {

>> @@ -613,11 +613,8 @@ typedef struct odp_ipsec_sa_param_t {

>>  	 *  only in ODP_IPSEC_LOOKUP_DSTADDR_SPI lookup mode. */

>>  	struct {

>>  		/** Select IP version

>> -		 *

>> -		 *  4:   IPv4

>> -		 *  6:   IPv6

>>  		 */

>> -		uint8_t ip_version;

>> +		odp_ipsec_header_type_t ip_version;

> 

> Name the type as IP header version enum. E.g.

> 

> odp_ipsec_ip_version_t 

> 

> The down side of this is that enum is int vs. uint8_t is one byte.


Well, memory is cheap. One can use any kind of storage internally (even
1-bit flag). However inside parameters I would vote for clarity rather
than storage. Moreover for embedded implementations one can set
-fshort-enums.


-- 
With best wishes
Dmitry

Patch hide | download patch | download mbox

diff --git a/include/odp/api/spec/ipsec.h b/include/odp/api/spec/ipsec.h
index e83494d..7b3b1fe 100644
--- a/include/odp/api/spec/ipsec.h
+++ b/include/odp/api/spec/ipsec.h
@@ -339,16 +339,16 @@  typedef enum odp_ipsec_protocol_t {
 } odp_ipsec_protocol_t;
 
 /**
- * IPSEC tunnel type
+ * IPSEC header type
  */
-typedef enum odp_ipsec_tunnel_type_t {
-	/** Outer header is IPv4 */
-	ODP_IPSEC_TUNNEL_IPV4 = 0,
+typedef enum odp_ipsec_header_type_t {
+	/** Header is IPv4 */
+	ODP_IPSEC_IPV4 = 0,
 
-	/** Outer header is IPv6 */
-	ODP_IPSEC_TUNNEL_IPV6
+	/** Header is IPv6 */
+	ODP_IPSEC_IPV6
 
-} odp_ipsec_tunnel_type_t;
+} odp_ipsec_header_type_t;
 
 /**
  * IPSEC crypto parameters
@@ -378,7 +378,7 @@  typedef struct odp_ipsec_crypto_param_t {
  */
 typedef struct odp_ipsec_tunnel_param_t {
 	/** Tunnel type: IPv4 or IPv6 */
-	odp_ipsec_tunnel_type_t type;
+	odp_ipsec_header_type_t type;
 
 	/** Variant mappings for tunnel parameters */
 	union {
@@ -613,11 +613,8 @@  typedef struct odp_ipsec_sa_param_t {
 	 *  only in ODP_IPSEC_LOOKUP_DSTADDR_SPI lookup mode. */
 	struct {
 		/** Select IP version
-		 *
-		 *  4:   IPv4
-		 *  6:   IPv6
 		 */
-		uint8_t ip_version;
+		odp_ipsec_header_type_t ip_version;
 
 		/** IP destination address (NETWORK ENDIAN) */
 		void    *dst_addr;