[dpdk-dev,v2,4/6] examples/ipsec-secgw: add correct padding to tunnel mode

  From: Aviad Yehezkel <aviadye@mellanox.com>

Issue: None
Signed-off-by: Aviad Yehezkel <aviadye@mellanox.com>
--
v2:
* Fix commit message
---
 examples/ipsec-secgw/esp.c | 51 ++++++++++++++++++++++++++--------------------
 1 file changed, 29 insertions(+), 22 deletions(-)
  

Hi Aviad,

I think you missed my question on v1 for this patch.

Could you provide an example where the pad calculation with the current 
code is wrong?

Thanks,
Sergio

On 19/10/2017 19:53, aviadye@dev.mellanox.co.il wrote:
> From: Aviad Yehezkel <aviadye@mellanox.com>
>
> Issue: None
> Signed-off-by: Aviad Yehezkel <aviadye@mellanox.com>
> --
> v2:
> * Fix commit message
> ---
>   examples/ipsec-secgw/esp.c | 51 ++++++++++++++++++++++++++--------------------
>   1 file changed, 29 insertions(+), 22 deletions(-)
>
> diff --git a/examples/ipsec-secgw/esp.c b/examples/ipsec-secgw/esp.c
> index 70bb81f..6215ad4 100644
> --- a/examples/ipsec-secgw/esp.c
> +++ b/examples/ipsec-secgw/esp.c
> @@ -229,25 +229,26 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa,
>   	uint8_t *padding, *new_ip, nlp;
>   	struct rte_crypto_sym_op *sym_cop;
>   	int32_t i;
> -	uint16_t pad_payload_len, pad_len, ip_hdr_len;
> +	uint16_t pad_payload_len, pad_len = 0;
> +	uint16_t inner_ip_hdr_len = 0, ip_hdr_len = 0;
>   
>   	RTE_ASSERT(m != NULL);
>   	RTE_ASSERT(sa != NULL);
> +	RTE_ASSERT(sa->flags == IP4_TUNNEL || sa->flags == IP6_TUNNEL ||
> +		   sa->flags == TRANSPORT);
>   	RTE_ASSERT(cop != NULL);
>   
> -	ip_hdr_len = 0;
> -
>   	ip4 = rte_pktmbuf_mtod(m, struct ip *);
>   	if (likely(ip4->ip_v == IPVERSION)) {
> -		if (unlikely(sa->flags == TRANSPORT)) {
> -			ip_hdr_len = ip4->ip_hl * 4;
> +		ip_hdr_len = ip4->ip_hl * 4;
> +		if (unlikely(sa->flags == TRANSPORT))
>   			nlp = ip4->ip_p;
> -		} else
> +		else
>   			nlp = IPPROTO_IPIP;
>   	} else if (ip4->ip_v == IP6_VERSION) {
> +		/* XXX No option headers supported */
> +		ip_hdr_len = sizeof(struct ip6_hdr);
>   		if (unlikely(sa->flags == TRANSPORT)) {
> -			/* XXX No option headers supported */
> -			ip_hdr_len = sizeof(struct ip6_hdr);
>   			ip6 = (struct ip6_hdr *)ip4;
>   			nlp = ip6->ip6_nxt;
>   		} else
> @@ -259,22 +260,28 @@ esp_outbound(struct rte_mbuf *m, struct ipsec_sa *sa,
>   	}
>   
>   	/* Padded payload length */
> -	pad_payload_len = RTE_ALIGN_CEIL(rte_pktmbuf_pkt_len(m) -
> -			ip_hdr_len + 2, sa->block_size);
> -	pad_len = pad_payload_len + ip_hdr_len - rte_pktmbuf_pkt_len(m);
> -
> -	RTE_ASSERT(sa->flags == IP4_TUNNEL || sa->flags == IP6_TUNNEL ||
> -			sa->flags == TRANSPORT);
> -
> -	if (likely(sa->flags == IP4_TUNNEL))
> +	if (unlikely(sa->flags == TRANSPORT)) {
> +		pad_payload_len = RTE_ALIGN_CEIL(rte_pktmbuf_pkt_len(m) +
> +						 sizeof(nlp) + 1 - ip_hdr_len,
> +						 sa->block_size);
> +		pad_len = pad_payload_len + ip_hdr_len - rte_pktmbuf_pkt_len(m);
> +	} else {
> +		inner_ip_hdr_len = ip_hdr_len;
>   		ip_hdr_len = sizeof(struct ip);
> -	else if (sa->flags == IP6_TUNNEL)
> -		ip_hdr_len = sizeof(struct ip6_hdr);
> -	else if (sa->flags != TRANSPORT) {
> -		RTE_LOG(ERR, IPSEC_ESP, "Unsupported SA flags: 0x%x\n",
> -				sa->flags);
> -		return -EINVAL;
> +		if (sa->flags == IP6_TUNNEL)
> +			ip_hdr_len = sizeof(struct ip6_hdr);
> +
> +		pad_payload_len = RTE_ALIGN_CEIL(rte_pktmbuf_pkt_len(m) +
> +						 sizeof(nlp) + 1,
> +						 sa->block_size);
> +		pad_len = pad_payload_len - rte_pktmbuf_pkt_len(m);
>   	}
> +	RTE_LOG(DEBUG, IPSEC_ESP, "rte_pktmbuf_pkt_len=%u "
> +		"inner_ip_hdr_len=%u ip_hdr_len=%u "
> +		"pad_payload_len=%u pad_len=%u\n",
> +		rte_pktmbuf_pkt_len(m),
> +		inner_ip_hdr_len, ip_hdr_len,
> +		pad_payload_len, pad_len);
>   
>   	/* Check maximum packet size */
>   	if (unlikely(ip_hdr_len + sizeof(struct esp_hdr) + sa->iv_len +
  

Hi Aviad,

> -----Original Message-----
> From: Gonzalez Monroy, Sergio
> Sent: Friday, October 20, 2017 6:56 AM
> To: aviadye@dev.mellanox.co.il; 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 v2 4/6] examples/ipsec-secgw: add correct
> padding to tunnel mode
> 
> Hi Aviad,
> 
> I think you missed my question on v1 for this patch.
> 
> Could you provide an example where the pad calculation with the current
> code is wrong?
> 

Could you reply to the comments from Sergio and Radu?
We should try to get this merged ASAP.

Thanks,
Pablo
  

I talked with Akhil. I will send v3 of patches by EOD and hold back this 
patch.

The reason I don't have time to deep dive regression tests and get back 
with an example in the upcoming days.

Thanks!


On 10/23/2017 1:54 PM, De Lara Guarch, Pablo wrote:
> Hi Aviad,
>
>> -----Original Message-----
>> From: Gonzalez Monroy, Sergio
>> Sent: Friday, October 20, 2017 6:56 AM
>> To: aviadye@dev.mellanox.co.il; 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 v2 4/6] examples/ipsec-secgw: add correct
>> padding to tunnel mode
>>
>> Hi Aviad,
>>
>> I think you missed my question on v1 for this patch.
>>
>> Could you provide an example where the pad calculation with the current
>> code is wrong?
>>
> Could you reply to the comments from Sergio and Radu?
> We should try to get this merged ASAP.
>
> Thanks,
> Pablo
>