[dpdk-dev,05/11] examples/ipsec-secgw: Fixed transport

Message ID 1507987683-12315-5-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:27 p.m. UTC
  From: Aviad Yehezkel <aviadye@mellanox.com>

Seems like transport was broken for a long time

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

Comments

Aviad Yehezkel Oct. 15, 2017, 12:55 p.m. UTC | #1
On 10/14/2017 4:27 PM, aviadye@dev.mellanox.co.il wrote:
> From: Aviad Yehezkel <aviadye@mellanox.com>
>
> Seems like transport was broken for a long time
>
> Signed-off-by: Aviad Yehezkel <aviadye@mellanox.com>
> ---
>   examples/ipsec-secgw/esp.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/examples/ipsec-secgw/esp.c b/examples/ipsec-secgw/esp.c
> index 70bb81f..56ad7a0 100644
> --- a/examples/ipsec-secgw/esp.c
> +++ b/examples/ipsec-secgw/esp.c
> @@ -306,8 +306,8 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa,
>   				sizeof(struct esp_hdr) + sa->iv_len);
>   		memmove(new_ip, ip4, ip_hdr_len);
>   		esp = (struct esp_hdr *)(new_ip + ip_hdr_len);
> +		ip4 = (struct ip *)new_ip;
>   		if (likely(ip4->ip_v == IPVERSION)) {
> -			ip4 = (struct ip *)new_ip;
>   			ip4->ip_p = IPPROTO_ESP;
>   			ip4->ip_len = htons(rte_pktmbuf_data_len(m));
>   		} else {

Tested-by: Aviad Yehezkel <aviadye@mellanox.com>
  
Sergio Gonzalez Monroy Oct. 16, 2017, 9:30 a.m. UTC | #2
On 14/10/2017 14:27, aviadye@dev.mellanox.co.il wrote:
> From: Aviad Yehezkel <aviadye@mellanox.com>
>
> Seems like transport was broken for a long time

Commit message needs to be improved. Just mentioned what is wrong or how 
do you fix it.
Given that it is a fix, you should start the commit title with "fix ..." 
then also add the 'fixes' line with commit that added the bug.
That way you can easily see since when it was introduced.

Thanks,
Sergio

> Signed-off-by: Aviad Yehezkel <aviadye@mellanox.com>
> ---
>   examples/ipsec-secgw/esp.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/examples/ipsec-secgw/esp.c b/examples/ipsec-secgw/esp.c
> index 70bb81f..56ad7a0 100644
> --- a/examples/ipsec-secgw/esp.c
> +++ b/examples/ipsec-secgw/esp.c
> @@ -306,8 +306,8 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa,
>   				sizeof(struct esp_hdr) + sa->iv_len);
>   		memmove(new_ip, ip4, ip_hdr_len);
>   		esp = (struct esp_hdr *)(new_ip + ip_hdr_len);
> +		ip4 = (struct ip *)new_ip;
>   		if (likely(ip4->ip_v == IPVERSION)) {
> -			ip4 = (struct ip *)new_ip;
>   			ip4->ip_p = IPPROTO_ESP;
>   			ip4->ip_len = htons(rte_pktmbuf_data_len(m));
>   		} else {
  
Aviad Yehezkel Oct. 16, 2017, 10:42 a.m. UTC | #3
On 10/16/2017 12:30 PM, Sergio Gonzalez Monroy wrote:
> On 14/10/2017 14:27, aviadye@dev.mellanox.co.il wrote:
>> From: Aviad Yehezkel <aviadye@mellanox.com>
>>
>> Seems like transport was broken for a long time
>
> Commit message needs to be improved. Just mentioned what is wrong or 
> how do you fix it.
> Given that it is a fix, you should start the commit title with "fix 
> ..." then also add the 'fixes' line with commit that added the bug.
> That way you can easily see since when it was introduced.
>
> Thanks,
> Sergio
Will create such commit messages for future fixes as you instructed above.
I will remove this patch from next version of fixes since there is a 
similar fix
already provided by Tomasz Duszynski which I will review.

Thanks,
Aviad.


>> Signed-off-by: Aviad Yehezkel <aviadye@mellanox.com>
>> ---
>>   examples/ipsec-secgw/esp.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/examples/ipsec-secgw/esp.c b/examples/ipsec-secgw/esp.c
>> index 70bb81f..56ad7a0 100644
>> --- a/examples/ipsec-secgw/esp.c
>> +++ b/examples/ipsec-secgw/esp.c
>> @@ -306,8 +306,8 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa 
>> *sa,
>>                   sizeof(struct esp_hdr) + sa->iv_len);
>>           memmove(new_ip, ip4, ip_hdr_len);
>>           esp = (struct esp_hdr *)(new_ip + ip_hdr_len);
>> +        ip4 = (struct ip *)new_ip;
>>           if (likely(ip4->ip_v == IPVERSION)) {
>> -            ip4 = (struct ip *)new_ip;
>>               ip4->ip_p = IPPROTO_ESP;
>>               ip4->ip_len = htons(rte_pktmbuf_data_len(m));
>>           } else {
>
>
  
De Lara Guarch, Pablo Oct. 19, 2017, 6:16 p.m. UTC | #4
> -----Original Message-----

> From: Aviad Yehezkel [mailto:aviadye@dev.mellanox.co.il]

> Sent: Monday, October 16, 2017 11:42 AM

> To: Gonzalez Monroy, Sergio <sergio.gonzalez.monroy@intel.com>;

> dev@dpdk.org; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;

> aviadye@mellanox.com

> Cc: borisp@mellanox.com; akhil.goyal@nxp.com;

> hemant.agrawal@nxp.com; Nicolau, Radu <radu.nicolau@intel.com>;

> Doherty, Declan <declan.doherty@intel.com>; liranl@mellanox.com;

> nelio.laranjeiro@6wind.com; thomas@monjalon.net

> Subject: Re: [dpdk-dev][PATCH 05/11] examples/ipsec-secgw: Fixed

> transport

> 

> 

> On 10/16/2017 12:30 PM, Sergio Gonzalez Monroy wrote:

> > On 14/10/2017 14:27, aviadye@dev.mellanox.co.il wrote:

> >> From: Aviad Yehezkel <aviadye@mellanox.com>

> >>

> >> Seems like transport was broken for a long time

> >

> > Commit message needs to be improved. Just mentioned what is wrong or

> > how do you fix it.

> > Given that it is a fix, you should start the commit title with "fix

> > ..." then also add the 'fixes' line with commit that added the bug.

> > That way you can easily see since when it was introduced.

> >

> > Thanks,

> > Sergio

> Will create such commit messages for future fixes as you instructed above.

> I will remove this patch from next version of fixes since there is a similar fix

> already provided by Tomasz Duszynski which I will review.

> 

> Thanks,

> Aviad.


Hi Aviad,

Will you send a v2 of this patchset soon?

Thanks,
Pablo
  
Aviad Yehezkel Oct. 19, 2017, 6:29 p.m. UTC | #5
Yes, just finished my testing.

Will send v2 in a moment and Akhil will send v5 rebased above them for 
rte_security.


Thanks!

On 10/19/2017 9:16 PM, De Lara Guarch, Pablo wrote:
>
>> -----Original Message-----
>> From: Aviad Yehezkel [mailto:aviadye@dev.mellanox.co.il]
>> Sent: Monday, October 16, 2017 11:42 AM
>> To: Gonzalez Monroy, Sergio <sergio.gonzalez.monroy@intel.com>;
>> dev@dpdk.org; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
>> aviadye@mellanox.com
>> Cc: borisp@mellanox.com; akhil.goyal@nxp.com;
>> hemant.agrawal@nxp.com; Nicolau, Radu <radu.nicolau@intel.com>;
>> Doherty, Declan <declan.doherty@intel.com>; liranl@mellanox.com;
>> nelio.laranjeiro@6wind.com; thomas@monjalon.net
>> Subject: Re: [dpdk-dev][PATCH 05/11] examples/ipsec-secgw: Fixed
>> transport
>>
>>
>> On 10/16/2017 12:30 PM, Sergio Gonzalez Monroy wrote:
>>> On 14/10/2017 14:27, aviadye@dev.mellanox.co.il wrote:
>>>> From: Aviad Yehezkel <aviadye@mellanox.com>
>>>>
>>>> Seems like transport was broken for a long time
>>> Commit message needs to be improved. Just mentioned what is wrong or
>>> how do you fix it.
>>> Given that it is a fix, you should start the commit title with "fix
>>> ..." then also add the 'fixes' line with commit that added the bug.
>>> That way you can easily see since when it was introduced.
>>>
>>> Thanks,
>>> Sergio
>> Will create such commit messages for future fixes as you instructed above.
>> I will remove this patch from next version of fixes since there is a similar fix
>> already provided by Tomasz Duszynski which I will review.
>>
>> Thanks,
>> Aviad.
> Hi Aviad,
>
> Will you send a v2 of this patchset soon?
>
> Thanks,
> Pablo
  

Patch

diff --git a/examples/ipsec-secgw/esp.c b/examples/ipsec-secgw/esp.c
index 70bb81f..56ad7a0 100644
--- a/examples/ipsec-secgw/esp.c
+++ b/examples/ipsec-secgw/esp.c
@@ -306,8 +306,8 @@  esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa,
 				sizeof(struct esp_hdr) + sa->iv_len);
 		memmove(new_ip, ip4, ip_hdr_len);
 		esp = (struct esp_hdr *)(new_ip + ip_hdr_len);
+		ip4 = (struct ip *)new_ip;
 		if (likely(ip4->ip_v == IPVERSION)) {
-			ip4 = (struct ip *)new_ip;
 			ip4->ip_p = IPPROTO_ESP;
 			ip4->ip_len = htons(rte_pktmbuf_data_len(m));
 		} else {