DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] examples/ipsec-secgw: update incremental checksum
@ 2018-01-15 12:42 Akhil Goyal
  2018-01-15 12:48 ` Akhil Goyal
  0 siblings, 1 reply; 6+ messages in thread
From: Akhil Goyal @ 2018-01-15 12:42 UTC (permalink / raw)
  To: dev; +Cc: pablo.de.lara.guarch, hemant.agrawal, radu.nicolau, Akhil Goyal

When TTL is decremented or ecn is updated in IP header
before forwarding the packet, checksum needs to be updated.

In this patch an incremental checksum is added for ipv4 case.

Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
---
 examples/ipsec-secgw/ipip.h | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/examples/ipsec-secgw/ipip.h b/examples/ipsec-secgw/ipip.h
index fb6a6fa..13b8455 100644
--- a/examples/ipsec-secgw/ipip.h
+++ b/examples/ipsec-secgw/ipip.h
@@ -27,6 +27,10 @@ ipip_outbound(struct rte_mbuf *m, uint32_t offset, uint32_t is_ipv6,
 	if (inip4->ip_v == IPVERSION) {
 		/* XXX This should be done by the forwarding engine instead */
 		inip4->ip_ttl -= 1;
+		if (inip4->ip_sum >= rte_cpu_to_be_16(0xffff - 0x100))
+			inip4->ip_sum += rte_cpu_to_be_16(0x100) + 1;
+		else
+			inip4->ip_sum += rte_cpu_to_be_16(0x100);
 		ds_ecn = inip4->ip_tos;
 	} else {
 		inip6 = (struct ip6_hdr *)inip4;
@@ -95,8 +99,17 @@ ip6ip_outbound(struct rte_mbuf *m, uint32_t offset,
 static inline void
 ip4_ecn_setup(struct ip *ip4)
 {
-	if (ip4->ip_tos & IPTOS_ECN_MASK)
+	if (ip4->ip_tos & IPTOS_ECN_MASK) {
+		unsigned long sum;
+		uint8_t old;
+
+		old = ip4->ip_tos;
 		ip4->ip_tos |= IPTOS_ECN_CE;
+		sum = old + (~(*(uint8_t *)&ip4->ip_tos) & 0xff);
+		sum += rte_be_to_cpu_16(ip4->ip_sum);
+		sum = (sum & 0xffff) + (sum >> 16);
+		ip4->ip_sum = rte_cpu_to_be_16(sum + (sum >> 16));
+	}
 }
 
 static inline void
@@ -140,6 +153,10 @@ ipip_inbound(struct rte_mbuf *m, uint32_t offset)
 			ip4_ecn_setup(inip4);
 		/* XXX This should be done by the forwarding engine instead */
 		inip4->ip_ttl -= 1;
+		if (inip4->ip_sum >= rte_cpu_to_be_16(0xffff - 0x100))
+			inip4->ip_sum += rte_cpu_to_be_16(0x100) + 1;
+		else
+			inip4->ip_sum += rte_cpu_to_be_16(0x100);
 		m->packet_type &= ~RTE_PTYPE_L4_MASK;
 		if (inip4->ip_p == IPPROTO_UDP)
 			m->packet_type |= RTE_PTYPE_L4_UDP;
-- 
2.9.3

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: update incremental checksum
  2018-01-15 12:42 [dpdk-dev] [PATCH] examples/ipsec-secgw: update incremental checksum Akhil Goyal
@ 2018-01-15 12:48 ` Akhil Goyal
  2018-01-15 14:40   ` Nicolau, Radu
  0 siblings, 1 reply; 6+ messages in thread
From: Akhil Goyal @ 2018-01-15 12:48 UTC (permalink / raw)
  To: dev; +Cc: pablo.de.lara.guarch, hemant.agrawal, radu.nicolau

On 1/15/2018 6:12 PM, Akhil Goyal wrote:
> When TTL is decremented or ecn is updated in IP header
> before forwarding the packet, checksum needs to be updated.
> 
> In this patch an incremental checksum is added for ipv4 case.
> 
> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> ---
This patch is an update to a very old patch which was rejected earlier.
http://dpdk.org/dev/patchwork/patch/16113/

>   examples/ipsec-secgw/ipip.h | 19 ++++++++++++++++++-
>   1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/examples/ipsec-secgw/ipip.h b/examples/ipsec-secgw/ipip.h
> index fb6a6fa..13b8455 100644
> --- a/examples/ipsec-secgw/ipip.h
> +++ b/examples/ipsec-secgw/ipip.h
> @@ -27,6 +27,10 @@ ipip_outbound(struct rte_mbuf *m, uint32_t offset, uint32_t is_ipv6,
>   	if (inip4->ip_v == IPVERSION) {
>   		/* XXX This should be done by the forwarding engine instead */
>   		inip4->ip_ttl -= 1;
> +		if (inip4->ip_sum >= rte_cpu_to_be_16(0xffff - 0x100))
> +			inip4->ip_sum += rte_cpu_to_be_16(0x100) + 1;
> +		else
> +			inip4->ip_sum += rte_cpu_to_be_16(0x100);
>   		ds_ecn = inip4->ip_tos;
>   	} else {
>   		inip6 = (struct ip6_hdr *)inip4;
> @@ -95,8 +99,17 @@ ip6ip_outbound(struct rte_mbuf *m, uint32_t offset,
>   static inline void
>   ip4_ecn_setup(struct ip *ip4)
>   {
> -	if (ip4->ip_tos & IPTOS_ECN_MASK)
> +	if (ip4->ip_tos & IPTOS_ECN_MASK) {
> +		unsigned long sum;
> +		uint8_t old;
> +
> +		old = ip4->ip_tos;
>   		ip4->ip_tos |= IPTOS_ECN_CE;
> +		sum = old + (~(*(uint8_t *)&ip4->ip_tos) & 0xff);
> +		sum += rte_be_to_cpu_16(ip4->ip_sum);
> +		sum = (sum & 0xffff) + (sum >> 16);
> +		ip4->ip_sum = rte_cpu_to_be_16(sum + (sum >> 16));
> +	}
>   }
>   
>   static inline void
> @@ -140,6 +153,10 @@ ipip_inbound(struct rte_mbuf *m, uint32_t offset)
>   			ip4_ecn_setup(inip4);
>   		/* XXX This should be done by the forwarding engine instead */
>   		inip4->ip_ttl -= 1;
> +		if (inip4->ip_sum >= rte_cpu_to_be_16(0xffff - 0x100))
> +			inip4->ip_sum += rte_cpu_to_be_16(0x100) + 1;
> +		else
> +			inip4->ip_sum += rte_cpu_to_be_16(0x100);
>   		m->packet_type &= ~RTE_PTYPE_L4_MASK;
>   		if (inip4->ip_p == IPPROTO_UDP)
>   			m->packet_type |= RTE_PTYPE_L4_UDP;
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: update incremental checksum
  2018-01-15 12:48 ` Akhil Goyal
@ 2018-01-15 14:40   ` Nicolau, Radu
  2018-01-16  6:29     ` Akhil Goyal
  0 siblings, 1 reply; 6+ messages in thread
From: Nicolau, Radu @ 2018-01-15 14:40 UTC (permalink / raw)
  To: Akhil Goyal, dev; +Cc: De Lara Guarch, Pablo, hemant.agrawal



> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Monday, January 15, 2018 12:48 PM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> hemant.agrawal@nxp.com; Nicolau, Radu <radu.nicolau@intel.com>
> Subject: Re: [PATCH] examples/ipsec-secgw: update incremental checksum
> 
> On 1/15/2018 6:12 PM, Akhil Goyal wrote:
> > When TTL is decremented or ecn is updated in IP header before
> > forwarding the packet, checksum needs to be updated.
> >
> > In this patch an incremental checksum is added for ipv4 case.
> >
> > Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> > ---
> This patch is an update to a very old patch which was rejected earlier.
> http://dpdk.org/dev/patchwork/patch/16113/
> 
> >   examples/ipsec-secgw/ipip.h | 19 ++++++++++++++++++-
> >   1 file changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/examples/ipsec-secgw/ipip.h b/examples/ipsec-secgw/ipip.h
> > index fb6a6fa..13b8455 100644
> > --- a/examples/ipsec-secgw/ipip.h
> > +++ b/examples/ipsec-secgw/ipip.h
> > @@ -27,6 +27,10 @@ ipip_outbound(struct rte_mbuf *m, uint32_t offset,
> uint32_t is_ipv6,
> >   	if (inip4->ip_v == IPVERSION) {
> >   		/* XXX This should be done by the forwarding engine instead
> */
> >   		inip4->ip_ttl -= 1;
> > +		if (inip4->ip_sum >= rte_cpu_to_be_16(0xffff - 0x100))
> > +			inip4->ip_sum += rte_cpu_to_be_16(0x100) + 1;
> > +		else
> > +			inip4->ip_sum += rte_cpu_to_be_16(0x100);
> >   		ds_ecn = inip4->ip_tos;
> >   	} else {
> >   		inip6 = (struct ip6_hdr *)inip4;
> > @@ -95,8 +99,17 @@ ip6ip_outbound(struct rte_mbuf *m, uint32_t offset,
> >   static inline void
> >   ip4_ecn_setup(struct ip *ip4)
> >   {
> > -	if (ip4->ip_tos & IPTOS_ECN_MASK)
> > +	if (ip4->ip_tos & IPTOS_ECN_MASK) {
> > +		unsigned long sum;
> > +		uint8_t old;
> > +
> > +		old = ip4->ip_tos;
> >   		ip4->ip_tos |= IPTOS_ECN_CE;
> > +		sum = old + (~(*(uint8_t *)&ip4->ip_tos) & 0xff);
> > +		sum += rte_be_to_cpu_16(ip4->ip_sum);
> > +		sum = (sum & 0xffff) + (sum >> 16);
> > +		ip4->ip_sum = rte_cpu_to_be_16(sum + (sum >> 16));
> > +	}
> >   }
> >
> >   static inline void
> > @@ -140,6 +153,10 @@ ipip_inbound(struct rte_mbuf *m, uint32_t offset)
> >   			ip4_ecn_setup(inip4);
> >   		/* XXX This should be done by the forwarding engine instead
> */
> >   		inip4->ip_ttl -= 1;
> > +		if (inip4->ip_sum >= rte_cpu_to_be_16(0xffff - 0x100))
> > +			inip4->ip_sum += rte_cpu_to_be_16(0x100) + 1;
> > +		else
> > +			inip4->ip_sum += rte_cpu_to_be_16(0x100);
> >   		m->packet_type &= ~RTE_PTYPE_L4_MASK;
> >   		if (inip4->ip_p == IPPROTO_UDP)
> >   			m->packet_type |= RTE_PTYPE_L4_UDP;
> >

I think instead of manipulating the checksum in this way it will be better to use rte_ipv4_cksum to re-compute it, unless the performance penalty is too significant.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: update incremental checksum
  2018-01-15 14:40   ` Nicolau, Radu
@ 2018-01-16  6:29     ` Akhil Goyal
  2018-01-16 10:56       ` Nicolau, Radu
  0 siblings, 1 reply; 6+ messages in thread
From: Akhil Goyal @ 2018-01-16  6:29 UTC (permalink / raw)
  To: Nicolau, Radu, dev; +Cc: De Lara Guarch, Pablo, hemant.agrawal

Hi Radu,
On 1/15/2018 8:10 PM, Nicolau, Radu wrote:
> 
> 
>> -----Original Message-----
>> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
>> Sent: Monday, January 15, 2018 12:48 PM
>> To: dev@dpdk.org
>> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
>> hemant.agrawal@nxp.com; Nicolau, Radu <radu.nicolau@intel.com>
>> Subject: Re: [PATCH] examples/ipsec-secgw: update incremental checksum
>>
>> On 1/15/2018 6:12 PM, Akhil Goyal wrote:
>>> When TTL is decremented or ecn is updated in IP header before
>>> forwarding the packet, checksum needs to be updated.
>>>
>>> In this patch an incremental checksum is added for ipv4 case.
>>>
>>> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
>>> ---
>> This patch is an update to a very old patch which was rejected earlier.
>> http://dpdk.org/dev/patchwork/patch/16113/
>>
>>>    examples/ipsec-secgw/ipip.h | 19 ++++++++++++++++++-
>>>    1 file changed, 18 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/examples/ipsec-secgw/ipip.h b/examples/ipsec-secgw/ipip.h
>>> index fb6a6fa..13b8455 100644
>>> --- a/examples/ipsec-secgw/ipip.h
>>> +++ b/examples/ipsec-secgw/ipip.h
>>> @@ -27,6 +27,10 @@ ipip_outbound(struct rte_mbuf *m, uint32_t offset,
>> uint32_t is_ipv6,
>>>    	if (inip4->ip_v == IPVERSION) {
>>>    		/* XXX This should be done by the forwarding engine instead
>> */
>>>    		inip4->ip_ttl -= 1;
>>> +		if (inip4->ip_sum >= rte_cpu_to_be_16(0xffff - 0x100))
>>> +			inip4->ip_sum += rte_cpu_to_be_16(0x100) + 1;
>>> +		else
>>> +			inip4->ip_sum += rte_cpu_to_be_16(0x100);
>>>    		ds_ecn = inip4->ip_tos;
>>>    	} else {
>>>    		inip6 = (struct ip6_hdr *)inip4;
>>> @@ -95,8 +99,17 @@ ip6ip_outbound(struct rte_mbuf *m, uint32_t offset,
>>>    static inline void
>>>    ip4_ecn_setup(struct ip *ip4)
>>>    {
>>> -	if (ip4->ip_tos & IPTOS_ECN_MASK)
>>> +	if (ip4->ip_tos & IPTOS_ECN_MASK) {
>>> +		unsigned long sum;
>>> +		uint8_t old;
>>> +
>>> +		old = ip4->ip_tos;
>>>    		ip4->ip_tos |= IPTOS_ECN_CE;
>>> +		sum = old + (~(*(uint8_t *)&ip4->ip_tos) & 0xff);
>>> +		sum += rte_be_to_cpu_16(ip4->ip_sum);
>>> +		sum = (sum & 0xffff) + (sum >> 16);
>>> +		ip4->ip_sum = rte_cpu_to_be_16(sum + (sum >> 16));
>>> +	}
>>>    }
>>>
>>>    static inline void
>>> @@ -140,6 +153,10 @@ ipip_inbound(struct rte_mbuf *m, uint32_t offset)
>>>    			ip4_ecn_setup(inip4);
>>>    		/* XXX This should be done by the forwarding engine instead
>> */
>>>    		inip4->ip_ttl -= 1;
>>> +		if (inip4->ip_sum >= rte_cpu_to_be_16(0xffff - 0x100))
>>> +			inip4->ip_sum += rte_cpu_to_be_16(0x100) + 1;
>>> +		else
>>> +			inip4->ip_sum += rte_cpu_to_be_16(0x100);
>>>    		m->packet_type &= ~RTE_PTYPE_L4_MASK;
>>>    		if (inip4->ip_p == IPPROTO_UDP)
>>>    			m->packet_type |= RTE_PTYPE_L4_UDP;
>>>
> 
> I think instead of manipulating the checksum in this way it will be better to use rte_ipv4_cksum to re-compute it, unless the performance penalty is too significant.
> 
There would be unnecessary wastage of cycles. This way of updating the 
checksum is implemented as per the RFC1141
https://tools.ietf.org/html/rfc1141
Do you see any issue in this way of updating the checksum?

-Akhil

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: update incremental checksum
  2018-01-16  6:29     ` Akhil Goyal
@ 2018-01-16 10:56       ` Nicolau, Radu
  2018-01-16 17:17         ` De Lara Guarch, Pablo
  0 siblings, 1 reply; 6+ messages in thread
From: Nicolau, Radu @ 2018-01-16 10:56 UTC (permalink / raw)
  To: Akhil Goyal, dev; +Cc: De Lara Guarch, Pablo, hemant.agrawal



> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Tuesday, January 16, 2018 6:30 AM
> To: Nicolau, Radu <radu.nicolau@intel.com>; dev@dpdk.org
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> hemant.agrawal@nxp.com
> Subject: Re: [PATCH] examples/ipsec-secgw: update incremental checksum
> 
> Hi Radu,
> On 1/15/2018 8:10 PM, Nicolau, Radu wrote:
> >
> >
> >> -----Original Message-----
> >> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> >> Sent: Monday, January 15, 2018 12:48 PM
> >> To: dev@dpdk.org
> >> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> >> hemant.agrawal@nxp.com; Nicolau, Radu <radu.nicolau@intel.com>
> >> Subject: Re: [PATCH] examples/ipsec-secgw: update incremental
> >> checksum
> >>
> >> On 1/15/2018 6:12 PM, Akhil Goyal wrote:
> >>> When TTL is decremented or ecn is updated in IP header before
> >>> forwarding the packet, checksum needs to be updated.
> >>>
> >>> In this patch an incremental checksum is added for ipv4 case.
> >>>
> >>> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> >>> ---
> >> This patch is an update to a very old patch which was rejected earlier.
> >> http://dpdk.org/dev/patchwork/patch/16113/
> >>
> >>>    examples/ipsec-secgw/ipip.h | 19 ++++++++++++++++++-
> >>>    1 file changed, 18 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/examples/ipsec-secgw/ipip.h
> >>> b/examples/ipsec-secgw/ipip.h index fb6a6fa..13b8455 100644
> >>> --- a/examples/ipsec-secgw/ipip.h
> >>> +++ b/examples/ipsec-secgw/ipip.h
> >>> @@ -27,6 +27,10 @@ ipip_outbound(struct rte_mbuf *m, uint32_t
> >>> offset,
> >> uint32_t is_ipv6,
> >>>    	if (inip4->ip_v == IPVERSION) {
> >>>    		/* XXX This should be done by the forwarding engine instead
> >> */
> >>>    		inip4->ip_ttl -= 1;
> >>> +		if (inip4->ip_sum >= rte_cpu_to_be_16(0xffff - 0x100))
> >>> +			inip4->ip_sum += rte_cpu_to_be_16(0x100) + 1;
> >>> +		else
> >>> +			inip4->ip_sum += rte_cpu_to_be_16(0x100);
> >>>    		ds_ecn = inip4->ip_tos;
> >>>    	} else {
> >>>    		inip6 = (struct ip6_hdr *)inip4; @@ -95,8 +99,17 @@
> >>> ip6ip_outbound(struct rte_mbuf *m, uint32_t offset,
> >>>    static inline void
> >>>    ip4_ecn_setup(struct ip *ip4)
> >>>    {
> >>> -	if (ip4->ip_tos & IPTOS_ECN_MASK)
> >>> +	if (ip4->ip_tos & IPTOS_ECN_MASK) {
> >>> +		unsigned long sum;
> >>> +		uint8_t old;
> >>> +
> >>> +		old = ip4->ip_tos;
> >>>    		ip4->ip_tos |= IPTOS_ECN_CE;
> >>> +		sum = old + (~(*(uint8_t *)&ip4->ip_tos) & 0xff);
> >>> +		sum += rte_be_to_cpu_16(ip4->ip_sum);
> >>> +		sum = (sum & 0xffff) + (sum >> 16);
> >>> +		ip4->ip_sum = rte_cpu_to_be_16(sum + (sum >> 16));
> >>> +	}
> >>>    }
> >>>
> >>>    static inline void
> >>> @@ -140,6 +153,10 @@ ipip_inbound(struct rte_mbuf *m, uint32_t
> offset)
> >>>    			ip4_ecn_setup(inip4);
> >>>    		/* XXX This should be done by the forwarding engine instead
> >> */
> >>>    		inip4->ip_ttl -= 1;
> >>> +		if (inip4->ip_sum >= rte_cpu_to_be_16(0xffff - 0x100))
> >>> +			inip4->ip_sum += rte_cpu_to_be_16(0x100) + 1;
> >>> +		else
> >>> +			inip4->ip_sum += rte_cpu_to_be_16(0x100);
> >>>    		m->packet_type &= ~RTE_PTYPE_L4_MASK;
> >>>    		if (inip4->ip_p == IPPROTO_UDP)
> >>>    			m->packet_type |= RTE_PTYPE_L4_UDP;
> >>>
> >
> > I think instead of manipulating the checksum in this way it will be better to
> use rte_ipv4_cksum to re-compute it, unless the performance penalty is too
> significant.
> >
> There would be unnecessary wastage of cycles. This way of updating the
> checksum is implemented as per the RFC1141
> https://tools.ietf.org/html/rfc1141
> Do you see any issue in this way of updating the checksum?
No, I just tought that it will make the code look nicer.

Acked-by: Radu Nicolau <radu.nicolau@intel.com>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: update incremental checksum
  2018-01-16 10:56       ` Nicolau, Radu
@ 2018-01-16 17:17         ` De Lara Guarch, Pablo
  0 siblings, 0 replies; 6+ messages in thread
From: De Lara Guarch, Pablo @ 2018-01-16 17:17 UTC (permalink / raw)
  To: Nicolau, Radu, Akhil Goyal, dev; +Cc: hemant.agrawal



> -----Original Message-----
> From: Nicolau, Radu
> Sent: Tuesday, January 16, 2018 10:56 AM
> To: Akhil Goyal <akhil.goyal@nxp.com>; dev@dpdk.org
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> hemant.agrawal@nxp.com
> Subject: RE: [PATCH] examples/ipsec-secgw: update incremental checksum

...

> 
> Acked-by: Radu Nicolau <radu.nicolau@intel.com>

Applied to dpdk-next-crypto.
Thanks,

Pablo

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-01-16 17:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-15 12:42 [dpdk-dev] [PATCH] examples/ipsec-secgw: update incremental checksum Akhil Goyal
2018-01-15 12:48 ` Akhil Goyal
2018-01-15 14:40   ` Nicolau, Radu
2018-01-16  6:29     ` Akhil Goyal
2018-01-16 10:56       ` Nicolau, Radu
2018-01-16 17:17         ` De Lara Guarch, Pablo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).