[1/3] api: ipsec: make num_pkt/out/sa/opt unsigned

Message ID 20170410124243.17861-1-maxim.uvarov@linaro.org
State New
Headers show
Series
  • [1/3] api: ipsec: make num_pkt/out/sa/opt unsigned
Related show

Commit Message

Maxim Uvarov April 10, 2017, 12:42 p.m.
From: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org>


It does not make sense to specify negative amount inside num_*. Make
respective fields unsigned instead.

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

---
/** Email created from pull request 13 (lumag:ipsec-api)
 ** https://github.com/Linaro/odp/pull/13
 ** Patch: https://github.com/Linaro/odp/pull/13.patch
 ** Base sha: 38485719c028918b019c6a5fc67cf05053421c83
 ** Merge commit sha: 80f41ee9602b2b4282da8907c9d2fa78fdb6aeb8
 **/
 include/odp/api/spec/ipsec.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Savolainen, Petri (Nokia - FI/Espoo) April 10, 2017, 1:14 p.m. | #1
Is there a way to get Github to send individual patches to the list ?  ... Instead of this all-patches-in-one-mail format, with a misleading subject line (1/3).



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

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

> Uvarov

> Sent: Monday, April 10, 2017 3:43 PM

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

> Subject: [lng-odp] [PATCH 1/3] api: ipsec: make num_pkt/out/sa/opt

> unsigned

> 

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

> 

> It does not make sense to specify negative amount inside num_*. Make

> respective fields unsigned instead.

> 

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

> ---

> /** Email created from pull request 13 (lumag:ipsec-api)

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

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

>  ** Base sha: 38485719c028918b019c6a5fc67cf05053421c83

>  ** Merge commit sha: 80f41ee9602b2b4282da8907c9d2fa78fdb6aeb8

>  **/

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

>  1 file changed, 5 insertions(+), 5 deletions(-)

> 

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

> index a0ceb11..5655351 100644

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

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

> @@ -920,7 +920,7 @@ typedef struct odp_ipsec_op_status_t {

>   */

>  typedef struct odp_ipsec_op_param_t {

>  	/** Number of packets to be processed */

> -	int num_pkt;

> +	unsigned num_pkt;


I guess unsigned could be used. Usually, we use 'int num' as it matches function return value type.


> 

> From 330973e1c1b029f67d629159761c559f4e9060e8 Mon Sep 17 00:00:00 2001

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

> Date: Mon, 10 Apr 2017 14:24:55 +0300

> Subject: [PATCH 3/3] api: ipsec: add default queue for outbound events

> 

> If SA lookup fails for outbound IPsec packet in async mode, there is no

> way to report it back to application except using default queue (which

> does not exist at this moment).



There's no SA lookup but application should always give valid SA handles to the implementation. So, this queue is needed for sending an error back that an invalid SA was given, right ?

-Petri


> 

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

> ---

> /** Email created from pull request 13 (lumag:ipsec-api)

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

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

>  ** Base sha: 38485719c028918b019c6a5fc67cf05053421c83

>  ** Merge commit sha: 80f41ee9602b2b4282da8907c9d2fa78fdb6aeb8

>  **/

>  include/odp/api/spec/ipsec.h | 7 +++++++

>  1 file changed, 7 insertions(+)

> 

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

> index 33a84ee..4e3ec16 100644

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

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

> @@ -188,6 +188,13 @@ typedef struct odp_ipsec_inbound_config_t {

>   * Configuration options for IPSEC outbound processing

>   */

>  typedef struct odp_ipsec_outbound_config_t {

> +	/** Default destination queue for IPSEC events

> +	 *

> +	 *  When outbound SA lookup fails in the asynchronous mode,

> +	 *  resulting IPSEC events are enqueued into this queue.

> +	 */

> +	odp_queue_t default_queue;

> +

>  	/** Flags to control L3/L4 checksum insertion as part of

> outbound

>  	 *  packet processing. Packet must have set with valid L3/L4

> offsets.

>  	 *  Checksum configuration is ignored for packets that checksum

> cannot
Maxim Uvarov April 10, 2017, 1:30 p.m. | #2
On 10 April 2017 at 16:14, Savolainen, Petri (Nokia - FI/Espoo) <
petri.savolainen@nokia-bell-labs.com> wrote:

> Is there a way to get Github to send individual patches to the list ?  ...

> Instead of this all-patches-in-one-mail format, with a misleading subject

> line (1/3).

>

>

I will take a look. Now it sends patch which github provides
https://patch-diff.githubusercontent.com/raw/Linaro/odp/pull/13.patch
I thought git send-email  will split it up in individual patches. But it's
not.  Might be I need to do it inside the patch.

Maxim.




>

>

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

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

> Maxim

> > Uvarov

> > Sent: Monday, April 10, 2017 3:43 PM

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

> > Subject: [lng-odp] [PATCH 1/3] api: ipsec: make num_pkt/out/sa/opt

> > unsigned

> >

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

> >

> > It does not make sense to specify negative amount inside num_*. Make

> > respective fields unsigned instead.

> >

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

> linaro.org>

> > ---

> > /** Email created from pull request 13 (lumag:ipsec-api)

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

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

> >  ** Base sha: 38485719c028918b019c6a5fc67cf05053421c83

> >  ** Merge commit sha: 80f41ee9602b2b4282da8907c9d2fa78fdb6aeb8

> >  **/

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

> >  1 file changed, 5 insertions(+), 5 deletions(-)

> >

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

> > index a0ceb11..5655351 100644

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

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

> > @@ -920,7 +920,7 @@ typedef struct odp_ipsec_op_status_t {

> >   */

> >  typedef struct odp_ipsec_op_param_t {

> >       /** Number of packets to be processed */

> > -     int num_pkt;

> > +     unsigned num_pkt;

>

> I guess unsigned could be used. Usually, we use 'int num' as it matches

> function return value type.

>

>

> >

> > From 330973e1c1b029f67d629159761c559f4e9060e8 Mon Sep 17 00:00:00 2001

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

> > Date: Mon, 10 Apr 2017 14:24:55 +0300

> > Subject: [PATCH 3/3] api: ipsec: add default queue for outbound events

> >

> > If SA lookup fails for outbound IPsec packet in async mode, there is no

> > way to report it back to application except using default queue (which

> > does not exist at this moment).

>

>

> There's no SA lookup but application should always give valid SA handles

> to the implementation. So, this queue is needed for sending an error back

> that an invalid SA was given, right ?

>

> -Petri

>

>

> >

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

> linaro.org>

> > ---

> > /** Email created from pull request 13 (lumag:ipsec-api)

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

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

> >  ** Base sha: 38485719c028918b019c6a5fc67cf05053421c83

> >  ** Merge commit sha: 80f41ee9602b2b4282da8907c9d2fa78fdb6aeb8

> >  **/

> >  include/odp/api/spec/ipsec.h | 7 +++++++

> >  1 file changed, 7 insertions(+)

> >

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

> > index 33a84ee..4e3ec16 100644

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

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

> > @@ -188,6 +188,13 @@ typedef struct odp_ipsec_inbound_config_t {

> >   * Configuration options for IPSEC outbound processing

> >   */

> >  typedef struct odp_ipsec_outbound_config_t {

> > +     /** Default destination queue for IPSEC events

> > +      *

> > +      *  When outbound SA lookup fails in the asynchronous mode,

> > +      *  resulting IPSEC events are enqueued into this queue.

> > +      */

> > +     odp_queue_t default_queue;

> > +

> >       /** Flags to control L3/L4 checksum insertion as part of

> > outbound

> >        *  packet processing. Packet must have set with valid L3/L4

> > offsets.

> >        *  Checksum configuration is ignored for packets that checksum

> > cannot

>
Dmitry Eremin-Solenikov April 10, 2017, 1:54 p.m. | #3
On 10.04.2017 16:30, Maxim Uvarov wrote:
> On 10 April 2017 at 16:14, Savolainen, Petri (Nokia - FI/Espoo) <

> petri.savolainen@nokia-bell-labs.com> wrote:

> 

>> Is there a way to get Github to send individual patches to the list ?  ...

>> Instead of this all-patches-in-one-mail format, with a misleading subject

>> line (1/3).

>>

>>

> I will take a look. Now it sends patch which github provides

> https://patch-diff.githubusercontent.com/raw/Linaro/odp/pull/13.patch

> I thought git send-email  will split it up in individual patches. But it's

> not.  Might be I need to do it inside the patch.


It is an mbox. I think you can use formail to process it.

-- 
With best wishes
Dmitry
Dmitry Eremin-Solenikov April 10, 2017, 1:55 p.m. | #4
On 10.04.2017 16:14, Savolainen, Petri (Nokia - FI/Espoo) wrote:
> Is there a way to get Github to send individual patches to the list ?  ... Instead of this all-patches-in-one-mail format, with a misleading subject line (1/3).

> 

> 

> 

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

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

>> Uvarov

>> Sent: Monday, April 10, 2017 3:43 PM

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

>> Subject: [lng-odp] [PATCH 1/3] api: ipsec: make num_pkt/out/sa/opt

>> unsigned

>>

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

>>

>> It does not make sense to specify negative amount inside num_*. Make

>> respective fields unsigned instead.

>>

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

>> ---

>> /** Email created from pull request 13 (lumag:ipsec-api)

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

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

>>  ** Base sha: 38485719c028918b019c6a5fc67cf05053421c83

>>  ** Merge commit sha: 80f41ee9602b2b4282da8907c9d2fa78fdb6aeb8

>>  **/

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

>>  1 file changed, 5 insertions(+), 5 deletions(-)

>>

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

>> index a0ceb11..5655351 100644

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

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

>> @@ -920,7 +920,7 @@ typedef struct odp_ipsec_op_status_t {

>>   */

>>  typedef struct odp_ipsec_op_param_t {

>>  	/** Number of packets to be processed */

>> -	int num_pkt;

>> +	unsigned num_pkt;

> 

> I guess unsigned could be used. Usually, we use 'int num' as it matches function return value type.


Then shouldn't we change function return value type? I'd see this as int
vs size_t argument. If the value is unsigned, then the holding type also
should be unsigned, shan't it?


-- 
With best wishes
Dmitry

Patch

diff --git a/include/odp/api/spec/ipsec.h b/include/odp/api/spec/ipsec.h
index a0ceb11..5655351 100644
--- a/include/odp/api/spec/ipsec.h
+++ b/include/odp/api/spec/ipsec.h
@@ -920,7 +920,7 @@  typedef struct odp_ipsec_op_status_t {
  */
 typedef struct odp_ipsec_op_param_t {
 	/** Number of packets to be processed */
-	int num_pkt;
+	unsigned num_pkt;
 
 	/** Number of SAs
 	 *
@@ -929,7 +929,7 @@  typedef struct odp_ipsec_op_param_t {
 	 *  * 1:       Single SA for all packets
 	 *  * num_pkt: SA per packet
 	 */
-	int num_sa;
+	unsigned num_sa;
 
 	/** Number of operation options
 	 *
@@ -938,7 +938,7 @@  typedef struct odp_ipsec_op_param_t {
 	 *  * 1:       Single option for all packets
 	 *  * num_pkt: An option per packet
 	 */
-	int num_opt;
+	unsigned num_opt;
 
 	/** Pointer to an array of packets
 	 *
@@ -1010,7 +1010,7 @@  typedef struct odp_ipsec_packet_result_t {
 	 *  fragments. All the fragments (of the same source packet) are stored
 	 *  consecutively in the 'pkt' array.
 	 */
-	int num_out;
+	unsigned num_out;
 
 	/** IPSEC SA that was used to create the packet
 	 *
@@ -1051,7 +1051,7 @@  typedef struct odp_ipsec_op_result_t {
 	 *  The operation updates it with the actual number of packets
 	 *  outputted.
 	 */
-	int num_pkt;
+	unsigned num_pkt;
 
 	/** Pointer to an array of packets
 	 *

From 40213f318cc323b9d2b2a0c9b8dde0d45d8b9a0f Mon Sep 17 00:00:00 2001
From: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org>
Date: Mon, 10 Apr 2017 14:23:35 +0300
Subject: [PATCH 2/3] api: ipsec: mark odp_ipsec_sa_create argument as constant

odp_ipsec_sa_create() should not change its argument. Thus mark it as a
constant.

Signed-off-by: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org>
---
/** Email created from pull request 13 (lumag:ipsec-api)
 ** https://github.com/Linaro/odp/pull/13
 ** Patch: https://github.com/Linaro/odp/pull/13.patch
 ** Base sha: 38485719c028918b019c6a5fc67cf05053421c83
 ** Merge commit sha: 80f41ee9602b2b4282da8907c9d2fa78fdb6aeb8
 **/
 include/odp/api/spec/ipsec.h       | 2 +-
 platform/linux-generic/odp_ipsec.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/odp/api/spec/ipsec.h b/include/odp/api/spec/ipsec.h
index 5655351..33a84ee 100644
--- a/include/odp/api/spec/ipsec.h
+++ b/include/odp/api/spec/ipsec.h
@@ -785,7 +785,7 @@  void odp_ipsec_sa_param_init(odp_ipsec_sa_param_t *param);
  *
  * @see odp_ipsec_sa_param_init()
  */
-odp_ipsec_sa_t odp_ipsec_sa_create(odp_ipsec_sa_param_t *param);
+odp_ipsec_sa_t odp_ipsec_sa_create(const odp_ipsec_sa_param_t *param);
 
 /**
  * Disable IPSEC SA
diff --git a/platform/linux-generic/odp_ipsec.c b/platform/linux-generic/odp_ipsec.c
index 5eb1be3..10918df 100644
--- a/platform/linux-generic/odp_ipsec.c
+++ b/platform/linux-generic/odp_ipsec.c
@@ -52,7 +52,7 @@  void odp_ipsec_sa_param_init(odp_ipsec_sa_param_t *param)
 	memset(param, 0, sizeof(odp_ipsec_sa_param_t));
 }
 
-odp_ipsec_sa_t odp_ipsec_sa_create(odp_ipsec_sa_param_t *param)
+odp_ipsec_sa_t odp_ipsec_sa_create(const odp_ipsec_sa_param_t *param)
 {
 	(void)param;
 

From 330973e1c1b029f67d629159761c559f4e9060e8 Mon Sep 17 00:00:00 2001
From: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org>
Date: Mon, 10 Apr 2017 14:24:55 +0300
Subject: [PATCH 3/3] api: ipsec: add default queue for outbound events

If SA lookup fails for outbound IPsec packet in async mode, there is no
way to report it back to application except using default queue (which
does not exist at this moment).

Signed-off-by: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org>
---
/** Email created from pull request 13 (lumag:ipsec-api)
 ** https://github.com/Linaro/odp/pull/13
 ** Patch: https://github.com/Linaro/odp/pull/13.patch
 ** Base sha: 38485719c028918b019c6a5fc67cf05053421c83
 ** Merge commit sha: 80f41ee9602b2b4282da8907c9d2fa78fdb6aeb8
 **/
 include/odp/api/spec/ipsec.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/odp/api/spec/ipsec.h b/include/odp/api/spec/ipsec.h
index 33a84ee..4e3ec16 100644
--- a/include/odp/api/spec/ipsec.h
+++ b/include/odp/api/spec/ipsec.h
@@ -188,6 +188,13 @@  typedef struct odp_ipsec_inbound_config_t {
  * Configuration options for IPSEC outbound processing
  */
 typedef struct odp_ipsec_outbound_config_t {
+	/** Default destination queue for IPSEC events
+	 *
+	 *  When outbound SA lookup fails in the asynchronous mode,
+	 *  resulting IPSEC events are enqueued into this queue.
+	 */
+	odp_queue_t default_queue;
+
 	/** Flags to control L3/L4 checksum insertion as part of outbound
 	 *  packet processing. Packet must have set with valid L3/L4 offsets.
 	 *  Checksum configuration is ignored for packets that checksum cannot