diff mbox

[dpdk-dev,08/11] examples/ipsec-secgw: iv should be be64

Message ID 1507987683-12315-8-git-send-email-aviadye@dev.mellanox.co.il (mailing list archive)
State Changes Requested, archived
Delegated to: Pablo de Lara Guarch
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Aviad Yehezkel Oct. 14, 2017, 1:28 p.m. UTC 
  From: Aviad Yehezkel <aviadye@mellanox.com>

To be compatibile with Linux kernel

Signed-off-by: Aviad Yehezkel <aviadye@mellanox.com>
---
 examples/ipsec-secgw/esp.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Aviad Yehezkel Oct. 15, 2017, 12:55 p.m. UTC | #1 
On 10/14/2017 4:28 PM, aviadye@dev.mellanox.co.il wrote:
> From: Aviad Yehezkel <aviadye@mellanox.com>
>
> To be compatibile with Linux kernel
>
> Signed-off-by: Aviad Yehezkel <aviadye@mellanox.com>
> ---
>   examples/ipsec-secgw/esp.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/examples/ipsec-secgw/esp.c b/examples/ipsec-secgw/esp.c
> index aa2233d..81ebf55 100644
> --- a/examples/ipsec-secgw/esp.c
> +++ b/examples/ipsec-secgw/esp.c
> @@ -336,7 +336,7 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa,
>   	if (sa->aead_algo == RTE_CRYPTO_AEAD_AES_GCM) {
>   		uint8_t *aad;
>   
> -		*iv = sa->seq;
> +		*iv = rte_cpu_to_be_64(sa->seq);
>   		sym_cop->aead.data.offset = ip_hdr_len +
>   			sizeof(struct esp_hdr) + sa->iv_len;
>   		sym_cop->aead.data.length = pad_payload_len;
> @@ -349,7 +349,7 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa,
>   
>   		struct cnt_blk *icb = get_cnt_blk(m);
>   		icb->salt = sa->salt;
> -		icb->iv = sa->seq;
> +		icb->iv = rte_cpu_to_be_64(sa->seq);
>   		icb->cnt = rte_cpu_to_be_32(1);
>   
>   		aad = get_aad(m);
> @@ -371,7 +371,7 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa,
>   			sym_cop->cipher.data.length = pad_payload_len + sa->iv_len;
>   			break;
>   		case RTE_CRYPTO_CIPHER_AES_CTR:
> -			*iv = sa->seq;
> +			*iv = rte_cpu_to_be_64(sa->seq);
>   			sym_cop->cipher.data.offset = ip_hdr_len +
>   				sizeof(struct esp_hdr) + sa->iv_len;
>   			sym_cop->cipher.data.length = pad_payload_len;
> @@ -390,7 +390,7 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa,
>   
>   		struct cnt_blk *icb = get_cnt_blk(m);
>   		icb->salt = sa->salt;
> -		icb->iv = sa->seq;
> +		icb->iv = rte_cpu_to_be_64(sa->seq);
>   		icb->cnt = rte_cpu_to_be_32(1);
>   
>   		switch (sa->auth_algo) {

Tested-by: Aviad Yehezkel <aviadye@mellanox.com>
Sergio Gonzalez Monroy Oct. 16, 2017, 9:42 a.m. UTC | #2 
On 14/10/2017 14:28, aviadye@dev.mellanox.co.il wrote:
> From: Aviad Yehezkel <aviadye@mellanox.com>
>
> To be compatibile with Linux kernel

I am not sure what you are trying to achieve with this change.
The requirement is that the IV is unique, IMO changing the endianess is 
irrelevant here.
Can you provide case/example where current code does not work?

Thanks,
Sergio

> Signed-off-by: Aviad Yehezkel <aviadye@mellanox.com>
> ---
>   examples/ipsec-secgw/esp.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/examples/ipsec-secgw/esp.c b/examples/ipsec-secgw/esp.c
> index aa2233d..81ebf55 100644
> --- a/examples/ipsec-secgw/esp.c
> +++ b/examples/ipsec-secgw/esp.c
> @@ -336,7 +336,7 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa,
>   	if (sa->aead_algo == RTE_CRYPTO_AEAD_AES_GCM) {
>   		uint8_t *aad;
>   
> -		*iv = sa->seq;
> +		*iv = rte_cpu_to_be_64(sa->seq);
>   		sym_cop->aead.data.offset = ip_hdr_len +
>   			sizeof(struct esp_hdr) + sa->iv_len;
>   		sym_cop->aead.data.length = pad_payload_len;
> @@ -349,7 +349,7 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa,
>   
>   		struct cnt_blk *icb = get_cnt_blk(m);
>   		icb->salt = sa->salt;
> -		icb->iv = sa->seq;
> +		icb->iv = rte_cpu_to_be_64(sa->seq);
>   		icb->cnt = rte_cpu_to_be_32(1);
>   
>   		aad = get_aad(m);
> @@ -371,7 +371,7 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa,
>   			sym_cop->cipher.data.length = pad_payload_len + sa->iv_len;
>   			break;
>   		case RTE_CRYPTO_CIPHER_AES_CTR:
> -			*iv = sa->seq;
> +			*iv = rte_cpu_to_be_64(sa->seq);
>   			sym_cop->cipher.data.offset = ip_hdr_len +
>   				sizeof(struct esp_hdr) + sa->iv_len;
>   			sym_cop->cipher.data.length = pad_payload_len;
> @@ -390,7 +390,7 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa,
>   
>   		struct cnt_blk *icb = get_cnt_blk(m);
>   		icb->salt = sa->salt;
> -		icb->iv = sa->seq;
> +		icb->iv = rte_cpu_to_be_64(sa->seq);
>   		icb->cnt = rte_cpu_to_be_32(1);
>   
>   		switch (sa->auth_algo) {
Aviad Yehezkel Oct. 16, 2017, 10:35 a.m. UTC | #3 
On 10/16/2017 12:42 PM, Sergio Gonzalez Monroy wrote:
> On 14/10/2017 14:28, aviadye@dev.mellanox.co.il wrote:
>> From: Aviad Yehezkel <aviadye@mellanox.com>
>>
>> To be compatibile with Linux kernel
>
> I am not sure what you are trying to achieve with this change.
> The requirement is that the IV is unique, IMO changing the endianess 
> is irrelevant here.
> Can you provide case/example where current code does not work?
>
> Thanks,
> Sergio
You are right, according to rfc4106 the IV should be unique and can be
implemented as counter.
The changed was created because I put analyzer on wire and compare
packets generated by this application and Linux kernel.
Linux kernel sets IV as BE, so I thought it is worth to do the same for
future debug / comparison.

Thanks,
Aviad.

>
>> Signed-off-by: Aviad Yehezkel <aviadye@mellanox.com>
>> ---
>>   examples/ipsec-secgw/esp.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/examples/ipsec-secgw/esp.c b/examples/ipsec-secgw/esp.c
>> index aa2233d..81ebf55 100644
>> --- a/examples/ipsec-secgw/esp.c
>> +++ b/examples/ipsec-secgw/esp.c
>> @@ -336,7 +336,7 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa 
>> *sa,
>>       if (sa->aead_algo == RTE_CRYPTO_AEAD_AES_GCM) {
>>           uint8_t *aad;
>>   -        *iv = sa->seq;
>> +        *iv = rte_cpu_to_be_64(sa->seq);
>>           sym_cop->aead.data.offset = ip_hdr_len +
>>               sizeof(struct esp_hdr) + sa->iv_len;
>>           sym_cop->aead.data.length = pad_payload_len;
>> @@ -349,7 +349,7 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa 
>> *sa,
>>             struct cnt_blk *icb = get_cnt_blk(m);
>>           icb->salt = sa->salt;
>> -        icb->iv = sa->seq;
>> +        icb->iv = rte_cpu_to_be_64(sa->seq);
>>           icb->cnt = rte_cpu_to_be_32(1);
>>             aad = get_aad(m);
>> @@ -371,7 +371,7 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa 
>> *sa,
>>               sym_cop->cipher.data.length = pad_payload_len + 
>> sa->iv_len;
>>               break;
>>           case RTE_CRYPTO_CIPHER_AES_CTR:
>> -            *iv = sa->seq;
>> +            *iv = rte_cpu_to_be_64(sa->seq);
>>               sym_cop->cipher.data.offset = ip_hdr_len +
>>                   sizeof(struct esp_hdr) + sa->iv_len;
>>               sym_cop->cipher.data.length = pad_payload_len;
>> @@ -390,7 +390,7 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa 
>> *sa,
>>             struct cnt_blk *icb = get_cnt_blk(m);
>>           icb->salt = sa->salt;
>> -        icb->iv = sa->seq;
>> +        icb->iv = rte_cpu_to_be_64(sa->seq);
>>           icb->cnt = rte_cpu_to_be_32(1);
>>             switch (sa->auth_algo) {
>
>
Sergio Gonzalez Monroy Oct. 16, 2017, 11:59 a.m. UTC | #4 
On 16/10/2017 11:35, Aviad Yehezkel wrote:
>
> On 10/16/2017 12:42 PM, Sergio Gonzalez Monroy wrote:
>> On 14/10/2017 14:28, aviadye@dev.mellanox.co.il wrote:
>>> From: Aviad Yehezkel <aviadye@mellanox.com>
>>>
>>> To be compatibile with Linux kernel
>>
>> I am not sure what you are trying to achieve with this change.
>> The requirement is that the IV is unique, IMO changing the endianess 
>> is irrelevant here.
>> Can you provide case/example where current code does not work?
>>
>> Thanks,
>> Sergio
> You are right, according to rfc4106 the IV should be unique and can be
> implemented as counter.
> The changed was created because I put analyzer on wire and compare
> packets generated by this application and Linux kernel.
> Linux kernel sets IV as BE, so I thought it is worth to do the same for
> future debug / comparison.
>

I guess the performance impact is small (for LE platforms), so it would 
be good to add this (or similar) information to the commit message.

Thanks,
Sergio

> Thanks,
> Aviad.
>
>>
>>> Signed-off-by: Aviad Yehezkel <aviadye@mellanox.com>
>>> ---
>>>   examples/ipsec-secgw/esp.c | 8 ++++----
>>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/examples/ipsec-secgw/esp.c b/examples/ipsec-secgw/esp.c
>>> index aa2233d..81ebf55 100644
>>> --- a/examples/ipsec-secgw/esp.c
>>> +++ b/examples/ipsec-secgw/esp.c
>>> @@ -336,7 +336,7 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa 
>>> *sa,
>>>       if (sa->aead_algo == RTE_CRYPTO_AEAD_AES_GCM) {
>>>           uint8_t *aad;
>>>   -        *iv = sa->seq;
>>> +        *iv = rte_cpu_to_be_64(sa->seq);
>>>           sym_cop->aead.data.offset = ip_hdr_len +
>>>               sizeof(struct esp_hdr) + sa->iv_len;
>>>           sym_cop->aead.data.length = pad_payload_len;
>>> @@ -349,7 +349,7 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa 
>>> *sa,
>>>             struct cnt_blk *icb = get_cnt_blk(m);
>>>           icb->salt = sa->salt;
>>> -        icb->iv = sa->seq;
>>> +        icb->iv = rte_cpu_to_be_64(sa->seq);
>>>           icb->cnt = rte_cpu_to_be_32(1);
>>>             aad = get_aad(m);
>>> @@ -371,7 +371,7 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa 
>>> *sa,
>>>               sym_cop->cipher.data.length = pad_payload_len + 
>>> sa->iv_len;
>>>               break;
>>>           case RTE_CRYPTO_CIPHER_AES_CTR:
>>> -            *iv = sa->seq;
>>> +            *iv = rte_cpu_to_be_64(sa->seq);
>>>               sym_cop->cipher.data.offset = ip_hdr_len +
>>>                   sizeof(struct esp_hdr) + sa->iv_len;
>>>               sym_cop->cipher.data.length = pad_payload_len;
>>> @@ -390,7 +390,7 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa 
>>> *sa,
>>>             struct cnt_blk *icb = get_cnt_blk(m);
>>>           icb->salt = sa->salt;
>>> -        icb->iv = sa->seq;
>>> +        icb->iv = rte_cpu_to_be_64(sa->seq);
>>>           icb->cnt = rte_cpu_to_be_32(1);
>>>             switch (sa->auth_algo) {
>>
>>
>
diff mbox

Patch

diff --git a/examples/ipsec-secgw/esp.c b/examples/ipsec-secgw/esp.c
index aa2233d..81ebf55 100644
--- a/examples/ipsec-secgw/esp.c
+++ b/examples/ipsec-secgw/esp.c
@@ -336,7 +336,7 @@  esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa,
 	if (sa->aead_algo == RTE_CRYPTO_AEAD_AES_GCM) {
 		uint8_t *aad;
 
-		*iv = sa->seq;
+		*iv = rte_cpu_to_be_64(sa->seq);
 		sym_cop->aead.data.offset = ip_hdr_len +
 			sizeof(struct esp_hdr) + sa->iv_len;
 		sym_cop->aead.data.length = pad_payload_len;
@@ -349,7 +349,7 @@  esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa,
 
 		struct cnt_blk *icb = get_cnt_blk(m);
 		icb->salt = sa->salt;
-		icb->iv = sa->seq;
+		icb->iv = rte_cpu_to_be_64(sa->seq);
 		icb->cnt = rte_cpu_to_be_32(1);
 
 		aad = get_aad(m);
@@ -371,7 +371,7 @@  esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa,
 			sym_cop->cipher.data.length = pad_payload_len + sa->iv_len;
 			break;
 		case RTE_CRYPTO_CIPHER_AES_CTR:
-			*iv = sa->seq;
+			*iv = rte_cpu_to_be_64(sa->seq);
 			sym_cop->cipher.data.offset = ip_hdr_len +
 				sizeof(struct esp_hdr) + sa->iv_len;
 			sym_cop->cipher.data.length = pad_payload_len;
@@ -390,7 +390,7 @@  esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa,
 
 		struct cnt_blk *icb = get_cnt_blk(m);
 		icb->salt = sa->salt;
-		icb->iv = sa->seq;
+		icb->iv = rte_cpu_to_be_64(sa->seq);
 		icb->cnt = rte_cpu_to_be_32(1);
 
 		switch (sa->auth_algo) {