[2/4] examples/ipsec-secgw: disable Tx chksum offload for inline

Message ID 20220206143022.13098-2-ndabilpuram@marvell.com (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series [1/4] examples/ipsec-secgw: update error prints to data path log |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Nithin Dabilpuram Feb. 6, 2022, 2:30 p.m. UTC
  Enable Tx IPv4 checksum offload only when Tx inline crypto is needed.
In other cases such as Tx Inline protocol offload, checksum computation
is implicitly taken care by HW. The advantage of having only necessary
offloads enabled is that Tx burst function can be as light as possible.

Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
---
 examples/ipsec-secgw/ipsec-secgw.c | 3 ---
 examples/ipsec-secgw/sa.c          | 9 +++++++++
 2 files changed, 9 insertions(+), 3 deletions(-)
  

Comments

Ananyev, Konstantin Feb. 7, 2022, 9:52 a.m. UTC | #1
> Enable Tx IPv4 checksum offload only when Tx inline crypto is needed.
> In other cases such as Tx Inline protocol offload, checksum computation
> is implicitly taken care by HW.

Why is that?
These is two separate HW offload and user has to enable each of them explicitly.
Also we can TX clear-text traffic.

> The advantage of having only necessary
> offloads enabled is that Tx burst function can be as light as possible.
> 
> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> ---
>  examples/ipsec-secgw/ipsec-secgw.c | 3 ---
>  examples/ipsec-secgw/sa.c          | 9 +++++++++
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
> index 21abc0d..d8a9bfa 100644
> --- a/examples/ipsec-secgw/ipsec-secgw.c
> +++ b/examples/ipsec-secgw/ipsec-secgw.c
> @@ -2314,9 +2314,6 @@ port_init(uint16_t portid, uint64_t req_rx_offloads, uint64_t req_tx_offloads)
>  		local_port_conf.txmode.offloads |=
>  			RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE;
> 
> -	if (dev_info.tx_offload_capa & RTE_ETH_TX_OFFLOAD_IPV4_CKSUM)
> -		local_port_conf.txmode.offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
> -
>  	printf("port %u configuring rx_offloads=0x%" PRIx64
>  		", tx_offloads=0x%" PRIx64 "\n",
>  		portid, local_port_conf.rxmode.offloads,
> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
> index 1839ac7..b878a48 100644
> --- a/examples/ipsec-secgw/sa.c
> +++ b/examples/ipsec-secgw/sa.c
> @@ -1790,6 +1790,15 @@ sa_check_offloads(uint16_t port_id, uint64_t *rx_offloads,
>  				RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL)
>  				&& rule->portid == port_id) {
>  			*tx_offloads |= RTE_ETH_TX_OFFLOAD_SECURITY;
> +
> +			/* Checksum offload is not needed for inline protocol as
> +			 * all processing for Outbound IPSec packets will be
> +			 * implicitly taken care and for non-IPSec packets,
> +			 * there is no need of IPv4 Checksum offload.
> +			 */
> +			if (rule_type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO)
> +				*tx_offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
> +
>  			if (rule->mss)
>  				*tx_offloads |= RTE_ETH_TX_OFFLOAD_TCP_TSO;
>  		}
> --
> 2.8.4
  
Nithin Dabilpuram Feb. 7, 2022, 2:15 p.m. UTC | #2
On 2/7/22 3:22 PM, Ananyev, Konstantin wrote:
> 
>> Enable Tx IPv4 checksum offload only when Tx inline crypto is needed.
>> In other cases such as Tx Inline protocol offload, checksum computation
>> is implicitly taken care by HW.
> 
> Why is that?
> These is two separate HW offload and user has to enable each of them explicitly.


In Inline IPSec protocol offload, the complete tunnel header for tunnel 
mode is updated by HW/PMD. So it doesn't have any dependency on 
RTE_ETH_TX_OFFLOAD_IPV4_CKSUM as there is no valid l2_len/l3_len yet in 
the mbuf. Similarly in case of Transport mode, the IPv4 header is 
updated by HW itself for next proto and hence the offsets and all can 
vary based on the HW implementation.

Hence my thought was for Inline IPsec protocol offload, there is no need 
to explicitly say that RTE_ETH_TX_OFFLOAD_IPV4_CKSUM is enabled and need 
not provide ol_flags RTE_MBUF_F_TX_IP_CKSUM and l2_len and l3_len which 
might not be correct in prepare_tx_pkt().

 >* RTE_MBUF_F_TX_IP_CKSUM.
 > *  - fill the mbuf offload information: l2_len, l3_len
(Ex: Tunnel header being inserted is IPv6 while inner header is IPv4.

For inline crypto I agree, the packet content is all in place except for 
plain text->cipher text translation so l2/l3 offsets are valid.

 > Also we can TX clear-text traffic.
Ok, I agree that we can have clear-text traffic. We are already handling 
ipv4 checksum in SW in case Tx offload doesn't have IPv4 Checksum 
offload enabled. And for clear text traffic I think that is not needed
as well as we are not updating ttl.

My overall intention was to have lighter Tx burst function for Inline 
IPsec protocol offload as mainly IPsec traffic and not plain traffic is 
primary use case for ipsec-secgw.



> 
>> The advantage of having only necessary
>> offloads enabled is that Tx burst function can be as light as possible.
>>
>> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
>> ---
>>   examples/ipsec-secgw/ipsec-secgw.c | 3 ---
>>   examples/ipsec-secgw/sa.c          | 9 +++++++++
>>   2 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
>> index 21abc0d..d8a9bfa 100644
>> --- a/examples/ipsec-secgw/ipsec-secgw.c
>> +++ b/examples/ipsec-secgw/ipsec-secgw.c
>> @@ -2314,9 +2314,6 @@ port_init(uint16_t portid, uint64_t req_rx_offloads, uint64_t req_tx_offloads)
>>   		local_port_conf.txmode.offloads |=
>>   			RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE;
>>
>> -	if (dev_info.tx_offload_capa & RTE_ETH_TX_OFFLOAD_IPV4_CKSUM)
>> -		local_port_conf.txmode.offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
>> -
>>   	printf("port %u configuring rx_offloads=0x%" PRIx64
>>   		", tx_offloads=0x%" PRIx64 "\n",
>>   		portid, local_port_conf.rxmode.offloads,
>> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
>> index 1839ac7..b878a48 100644
>> --- a/examples/ipsec-secgw/sa.c
>> +++ b/examples/ipsec-secgw/sa.c
>> @@ -1790,6 +1790,15 @@ sa_check_offloads(uint16_t port_id, uint64_t *rx_offloads,
>>   				RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL)
>>   				&& rule->portid == port_id) {
>>   			*tx_offloads |= RTE_ETH_TX_OFFLOAD_SECURITY;
>> +
>> +			/* Checksum offload is not needed for inline protocol as
>> +			 * all processing for Outbound IPSec packets will be
>> +			 * implicitly taken care and for non-IPSec packets,
>> +			 * there is no need of IPv4 Checksum offload.
>> +			 */
>> +			if (rule_type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO)
>> +				*tx_offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
>> +
>>   			if (rule->mss)
>>   				*tx_offloads |= RTE_ETH_TX_OFFLOAD_TCP_TSO;
>>   		}
>> --
>> 2.8.4
>
  
Ananyev, Konstantin Feb. 17, 2022, 7:17 p.m. UTC | #3
> >> Enable Tx IPv4 checksum offload only when Tx inline crypto is needed.
> >> In other cases such as Tx Inline protocol offload, checksum computation
> >> is implicitly taken care by HW.
> >
> > Why is that?
> > These is two separate HW offload and user has to enable each of them explicitly.
> 
> 
> In Inline IPSec protocol offload, the complete tunnel header for tunnel
> mode is updated by HW/PMD. So it doesn't have any dependency on
> RTE_ETH_TX_OFFLOAD_IPV4_CKSUM as there is no valid l2_len/l3_len yet in
> the mbuf. Similarly in case of Transport mode, the IPv4 header is
> updated by HW itself for next proto and hence the offsets and all can
> vary based on the HW implementation.
> 
> Hence my thought was for Inline IPsec protocol offload, there is no need
> to explicitly say that RTE_ETH_TX_OFFLOAD_IPV4_CKSUM is enabled and need
> not provide ol_flags RTE_MBUF_F_TX_IP_CKSUM and l2_len and l3_len which
> might not be correct in prepare_tx_pkt().
> 
>  >* RTE_MBUF_F_TX_IP_CKSUM.
>  > *  - fill the mbuf offload information: l2_len, l3_len
> (Ex: Tunnel header being inserted is IPv6 while inner header is IPv4.
> 
> For inline crypto I agree, the packet content is all in place except for
> plain text->cipher text translation so l2/l3 offsets are valid.

Ok, but apart from inline modes we also have lookaside modes.
Why to disable ip cksum for them?

> 
>  > Also we can TX clear-text traffic.
> Ok, I agree that we can have clear-text traffic. We are already handling
> ipv4 checksum in SW in case Tx offload doesn't have IPv4 Checksum
> offload enabled. And for clear text traffic I think that is not needed
> as well as we are not updating ttl.

As I remember we always recalculate ip cksum if RTE_MBUF_F_TX_IP_CKSUM
is not set:

static inline void
prepare_tx_pkt(struct rte_mbuf *pkt, uint16_t port,
                const struct lcore_conf *qconf)
{
        struct ip *ip;
        struct rte_ether_hdr *ethhdr;

        ip = rte_pktmbuf_mtod(pkt, struct ip *);

        ethhdr = (struct rte_ether_hdr *)
                rte_pktmbuf_prepend(pkt, RTE_ETHER_HDR_LEN);

        if (ip->ip_v == IPVERSION) {
                pkt->ol_flags |= qconf->outbound.ipv4_offloads;
                pkt->l3_len = sizeof(struct ip);
                pkt->l2_len = RTE_ETHER_HDR_LEN;

                ip->ip_sum = 0;

                /* calculate IPv4 cksum in SW */
                if ((pkt->ol_flags & RTE_MBUF_F_TX_IP_CKSUM) == 0)
                        ip->ip_sum = rte_ipv4_cksum((struct rte_ipv4_hdr *)ip);

...

> 
> My overall intention was to have lighter Tx burst function for Inline
> IPsec protocol offload as mainly IPsec traffic and not plain traffic is
> primary use case for ipsec-secgw.
> 
> 
> 
> >
> >> The advantage of having only necessary
> >> offloads enabled is that Tx burst function can be as light as possible.
> >>
> >> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> >> ---
> >>   examples/ipsec-secgw/ipsec-secgw.c | 3 ---
> >>   examples/ipsec-secgw/sa.c          | 9 +++++++++
> >>   2 files changed, 9 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
> >> index 21abc0d..d8a9bfa 100644
> >> --- a/examples/ipsec-secgw/ipsec-secgw.c
> >> +++ b/examples/ipsec-secgw/ipsec-secgw.c
> >> @@ -2314,9 +2314,6 @@ port_init(uint16_t portid, uint64_t req_rx_offloads, uint64_t req_tx_offloads)
> >>   		local_port_conf.txmode.offloads |=
> >>   			RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE;
> >>
> >> -	if (dev_info.tx_offload_capa & RTE_ETH_TX_OFFLOAD_IPV4_CKSUM)
> >> -		local_port_conf.txmode.offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
> >> -
> >>   	printf("port %u configuring rx_offloads=0x%" PRIx64
> >>   		", tx_offloads=0x%" PRIx64 "\n",
> >>   		portid, local_port_conf.rxmode.offloads,
> >> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
> >> index 1839ac7..b878a48 100644
> >> --- a/examples/ipsec-secgw/sa.c
> >> +++ b/examples/ipsec-secgw/sa.c
> >> @@ -1790,6 +1790,15 @@ sa_check_offloads(uint16_t port_id, uint64_t *rx_offloads,
> >>   				RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL)
> >>   				&& rule->portid == port_id) {
> >>   			*tx_offloads |= RTE_ETH_TX_OFFLOAD_SECURITY;
> >> +
> >> +			/* Checksum offload is not needed for inline protocol as
> >> +			 * all processing for Outbound IPSec packets will be
> >> +			 * implicitly taken care and for non-IPSec packets,
> >> +			 * there is no need of IPv4 Checksum offload.
> >> +			 */
> >> +			if (rule_type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO)
> >> +				*tx_offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
> >> +
> >>   			if (rule->mss)
> >>   				*tx_offloads |= RTE_ETH_TX_OFFLOAD_TCP_TSO;
> >>   		}
> >> --
> >> 2.8.4
> >
  
Nithin Dabilpuram Feb. 18, 2022, 1:58 p.m. UTC | #4
On 2/18/22 12:47 AM, Ananyev, Konstantin wrote:
> 
>>>> Enable Tx IPv4 checksum offload only when Tx inline crypto is needed.
>>>> In other cases such as Tx Inline protocol offload, checksum computation
>>>> is implicitly taken care by HW.
>>>
>>> Why is that?
>>> These is two separate HW offload and user has to enable each of them explicitly.
>>
>>
>> In Inline IPSec protocol offload, the complete tunnel header for tunnel
>> mode is updated by HW/PMD. So it doesn't have any dependency on
>> RTE_ETH_TX_OFFLOAD_IPV4_CKSUM as there is no valid l2_len/l3_len yet in
>> the mbuf. Similarly in case of Transport mode, the IPv4 header is
>> updated by HW itself for next proto and hence the offsets and all can
>> vary based on the HW implementation.
>>
>> Hence my thought was for Inline IPsec protocol offload, there is no need
>> to explicitly say that RTE_ETH_TX_OFFLOAD_IPV4_CKSUM is enabled and need
>> not provide ol_flags RTE_MBUF_F_TX_IP_CKSUM and l2_len and l3_len which
>> might not be correct in prepare_tx_pkt().
>>
>>   >* RTE_MBUF_F_TX_IP_CKSUM.
>>   > *  - fill the mbuf offload information: l2_len, l3_len
>> (Ex: Tunnel header being inserted is IPv6 while inner header is IPv4.
>>
>> For inline crypto I agree, the packet content is all in place except for
>> plain text->cipher text translation so l2/l3 offsets are valid.
> 
> Ok, but apart from inline modes we also have lookaside modes.
> Why to disable ip cksum for them?

Ack, I missed that case. I'll make the change to enable Tx checksum 
offload enabled even for Lookaside crypto.

> 
>>
>>   > Also we can TX clear-text traffic.
>> Ok, I agree that we can have clear-text traffic. We are already handling
>> ipv4 checksum in SW in case Tx offload doesn't have IPv4 Checksum
>> offload enabled. And for clear text traffic I think that is not needed
>> as well as we are not updating ttl.
> 
> As I remember we always recalculate ip cksum if RTE_MBUF_F_TX_IP_CKSUM
> is not set:
> 
> static inline void
> prepare_tx_pkt(struct rte_mbuf *pkt, uint16_t port,
>                  const struct lcore_conf *qconf)
> {
>          struct ip *ip;
>          struct rte_ether_hdr *ethhdr;
> 
>          ip = rte_pktmbuf_mtod(pkt, struct ip *);
> 
>          ethhdr = (struct rte_ether_hdr *)
>                  rte_pktmbuf_prepend(pkt, RTE_ETHER_HDR_LEN);
> 
>          if (ip->ip_v == IPVERSION) {
>                  pkt->ol_flags |= qconf->outbound.ipv4_offloads;
>                  pkt->l3_len = sizeof(struct ip);
>                  pkt->l2_len = RTE_ETHER_HDR_LEN;
> 
>                  ip->ip_sum = 0;
> 
>                  /* calculate IPv4 cksum in SW */
>                  if ((pkt->ol_flags & RTE_MBUF_F_TX_IP_CKSUM) == 0)
>                          ip->ip_sum = rte_ipv4_cksum((struct rte_ipv4_hdr *)ip);
> 
> ...
> 

I'm working on another series to restructure fast path based on 
different mode. As part of that, I think we can avoid checksum 
recalculation in cases of inline protocol offload.

>>
>> My overall intention was to have lighter Tx burst function for Inline
>> IPsec protocol offload as mainly IPsec traffic and not plain traffic is
>> primary use case for ipsec-secgw.
>>
>>
>>
>>>
>>>> The advantage of having only necessary
>>>> offloads enabled is that Tx burst function can be as light as possible.
>>>>
>>>> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
>>>> ---
>>>>    examples/ipsec-secgw/ipsec-secgw.c | 3 ---
>>>>    examples/ipsec-secgw/sa.c          | 9 +++++++++
>>>>    2 files changed, 9 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
>>>> index 21abc0d..d8a9bfa 100644
>>>> --- a/examples/ipsec-secgw/ipsec-secgw.c
>>>> +++ b/examples/ipsec-secgw/ipsec-secgw.c
>>>> @@ -2314,9 +2314,6 @@ port_init(uint16_t portid, uint64_t req_rx_offloads, uint64_t req_tx_offloads)
>>>>    		local_port_conf.txmode.offloads |=
>>>>    			RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE;
>>>>
>>>> -	if (dev_info.tx_offload_capa & RTE_ETH_TX_OFFLOAD_IPV4_CKSUM)
>>>> -		local_port_conf.txmode.offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
>>>> -
>>>>    	printf("port %u configuring rx_offloads=0x%" PRIx64
>>>>    		", tx_offloads=0x%" PRIx64 "\n",
>>>>    		portid, local_port_conf.rxmode.offloads,
>>>> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
>>>> index 1839ac7..b878a48 100644
>>>> --- a/examples/ipsec-secgw/sa.c
>>>> +++ b/examples/ipsec-secgw/sa.c
>>>> @@ -1790,6 +1790,15 @@ sa_check_offloads(uint16_t port_id, uint64_t *rx_offloads,
>>>>    				RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL)
>>>>    				&& rule->portid == port_id) {
>>>>    			*tx_offloads |= RTE_ETH_TX_OFFLOAD_SECURITY;
>>>> +
>>>> +			/* Checksum offload is not needed for inline protocol as
>>>> +			 * all processing for Outbound IPSec packets will be
>>>> +			 * implicitly taken care and for non-IPSec packets,
>>>> +			 * there is no need of IPv4 Checksum offload.
>>>> +			 */
>>>> +			if (rule_type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO)
>>>> +				*tx_offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
>>>> +
>>>>    			if (rule->mss)
>>>>    				*tx_offloads |= RTE_ETH_TX_OFFLOAD_TCP_TSO;
>>>>    		}
>>>> --
>>>> 2.8.4
>>>
  
Nithin Dabilpuram Feb. 23, 2022, 9:58 a.m. UTC | #5
On 2/18/22 7:28 PM, Nithin Kumar Dabilpuram wrote:
> 
> 
> On 2/18/22 12:47 AM, Ananyev, Konstantin wrote:
>>
>>>>> Enable Tx IPv4 checksum offload only when Tx inline crypto is needed.
>>>>> In other cases such as Tx Inline protocol offload, checksum 
>>>>> computation
>>>>> is implicitly taken care by HW.
>>>>
>>>> Why is that?
>>>> These is two separate HW offload and user has to enable each of them 
>>>> explicitly.
>>>
>>>
>>> In Inline IPSec protocol offload, the complete tunnel header for tunnel
>>> mode is updated by HW/PMD. So it doesn't have any dependency on
>>> RTE_ETH_TX_OFFLOAD_IPV4_CKSUM as there is no valid l2_len/l3_len yet in
>>> the mbuf. Similarly in case of Transport mode, the IPv4 header is
>>> updated by HW itself for next proto and hence the offsets and all can
>>> vary based on the HW implementation.
>>>
>>> Hence my thought was for Inline IPsec protocol offload, there is no need
>>> to explicitly say that RTE_ETH_TX_OFFLOAD_IPV4_CKSUM is enabled and need
>>> not provide ol_flags RTE_MBUF_F_TX_IP_CKSUM and l2_len and l3_len which
>>> might not be correct in prepare_tx_pkt().
>>>
>>>   >* RTE_MBUF_F_TX_IP_CKSUM.
>>>   > *  - fill the mbuf offload information: l2_len, l3_len
>>> (Ex: Tunnel header being inserted is IPv6 while inner header is IPv4.
>>>
>>> For inline crypto I agree, the packet content is all in place except for
>>> plain text->cipher text translation so l2/l3 offsets are valid.
>>
>> Ok, but apart from inline modes we also have lookaside modes.
>> Why to disable ip cksum for them?
> 
> Ack, I missed that case. I'll make the change to enable Tx checksum 
> offload enabled even for Lookaside crypto.
> 
>>
>>>
>>>   > Also we can TX clear-text traffic.
>>> Ok, I agree that we can have clear-text traffic. We are already handling
>>> ipv4 checksum in SW in case Tx offload doesn't have IPv4 Checksum
>>> offload enabled. And for clear text traffic I think that is not needed
>>> as well as we are not updating ttl.
>>
>> As I remember we always recalculate ip cksum if RTE_MBUF_F_TX_IP_CKSUM
>> is not set:
>>
>> static inline void
>> prepare_tx_pkt(struct rte_mbuf *pkt, uint16_t port,
>>                  const struct lcore_conf *qconf)
>> {
>>          struct ip *ip;
>>          struct rte_ether_hdr *ethhdr;
>>
>>          ip = rte_pktmbuf_mtod(pkt, struct ip *);
>>
>>          ethhdr = (struct rte_ether_hdr *)
>>                  rte_pktmbuf_prepend(pkt, RTE_ETHER_HDR_LEN);
>>
>>          if (ip->ip_v == IPVERSION) {
>>                  pkt->ol_flags |= qconf->outbound.ipv4_offloads;
>>                  pkt->l3_len = sizeof(struct ip);
>>                  pkt->l2_len = RTE_ETHER_HDR_LEN;
>>
>>                  ip->ip_sum = 0;
>>
>>                  /* calculate IPv4 cksum in SW */
>>                  if ((pkt->ol_flags & RTE_MBUF_F_TX_IP_CKSUM) == 0)
>>                          ip->ip_sum = rte_ipv4_cksum((struct 
>> rte_ipv4_hdr *)ip);
>>
>> ...
>>
> 
> I'm working on another series to restructure fast path based on 
> different mode. As part of that, I think we can avoid checksum 
> recalculation in cases of inline protocol offload.

I removed this patch from v3 of this series. I'll included it in other 
series that I'm working on to have separate worker thread when all
SA's are of particular mode. There we can avoid checksum recalculation
completely both in SW and HW.

Thanks
Nithin

> 
>>>
>>> My overall intention was to have lighter Tx burst function for Inline
>>> IPsec protocol offload as mainly IPsec traffic and not plain traffic is
>>> primary use case for ipsec-secgw.
>>>
>>>
>>>
>>>>
>>>>> The advantage of having only necessary
>>>>> offloads enabled is that Tx burst function can be as light as 
>>>>> possible.
>>>>>
>>>>> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
>>>>> ---
>>>>>    examples/ipsec-secgw/ipsec-secgw.c | 3 ---
>>>>>    examples/ipsec-secgw/sa.c          | 9 +++++++++
>>>>>    2 files changed, 9 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/examples/ipsec-secgw/ipsec-secgw.c 
>>>>> b/examples/ipsec-secgw/ipsec-secgw.c
>>>>> index 21abc0d..d8a9bfa 100644
>>>>> --- a/examples/ipsec-secgw/ipsec-secgw.c
>>>>> +++ b/examples/ipsec-secgw/ipsec-secgw.c
>>>>> @@ -2314,9 +2314,6 @@ port_init(uint16_t portid, uint64_t 
>>>>> req_rx_offloads, uint64_t req_tx_offloads)
>>>>>            local_port_conf.txmode.offloads |=
>>>>>                RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE;
>>>>>
>>>>> -    if (dev_info.tx_offload_capa & RTE_ETH_TX_OFFLOAD_IPV4_CKSUM)
>>>>> -        local_port_conf.txmode.offloads |= 
>>>>> RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
>>>>> -
>>>>>        printf("port %u configuring rx_offloads=0x%" PRIx64
>>>>>            ", tx_offloads=0x%" PRIx64 "\n",
>>>>>            portid, local_port_conf.rxmode.offloads,
>>>>> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
>>>>> index 1839ac7..b878a48 100644
>>>>> --- a/examples/ipsec-secgw/sa.c
>>>>> +++ b/examples/ipsec-secgw/sa.c
>>>>> @@ -1790,6 +1790,15 @@ sa_check_offloads(uint16_t port_id, uint64_t 
>>>>> *rx_offloads,
>>>>>                    RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL)
>>>>>                    && rule->portid == port_id) {
>>>>>                *tx_offloads |= RTE_ETH_TX_OFFLOAD_SECURITY;
>>>>> +
>>>>> +            /* Checksum offload is not needed for inline protocol as
>>>>> +             * all processing for Outbound IPSec packets will be
>>>>> +             * implicitly taken care and for non-IPSec packets,
>>>>> +             * there is no need of IPv4 Checksum offload.
>>>>> +             */
>>>>> +            if (rule_type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO)
>>>>> +                *tx_offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
>>>>> +
>>>>>                if (rule->mss)
>>>>>                    *tx_offloads |= RTE_ETH_TX_OFFLOAD_TCP_TSO;
>>>>>            }
>>>>> -- 
>>>>> 2.8.4
>>>>
  

Patch

diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
index 21abc0d..d8a9bfa 100644
--- a/examples/ipsec-secgw/ipsec-secgw.c
+++ b/examples/ipsec-secgw/ipsec-secgw.c
@@ -2314,9 +2314,6 @@  port_init(uint16_t portid, uint64_t req_rx_offloads, uint64_t req_tx_offloads)
 		local_port_conf.txmode.offloads |=
 			RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE;
 
-	if (dev_info.tx_offload_capa & RTE_ETH_TX_OFFLOAD_IPV4_CKSUM)
-		local_port_conf.txmode.offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
-
 	printf("port %u configuring rx_offloads=0x%" PRIx64
 		", tx_offloads=0x%" PRIx64 "\n",
 		portid, local_port_conf.rxmode.offloads,
diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
index 1839ac7..b878a48 100644
--- a/examples/ipsec-secgw/sa.c
+++ b/examples/ipsec-secgw/sa.c
@@ -1790,6 +1790,15 @@  sa_check_offloads(uint16_t port_id, uint64_t *rx_offloads,
 				RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL)
 				&& rule->portid == port_id) {
 			*tx_offloads |= RTE_ETH_TX_OFFLOAD_SECURITY;
+
+			/* Checksum offload is not needed for inline protocol as
+			 * all processing for Outbound IPSec packets will be
+			 * implicitly taken care and for non-IPSec packets,
+			 * there is no need of IPv4 Checksum offload.
+			 */
+			if (rule_type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO)
+				*tx_offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
+
 			if (rule->mss)
 				*tx_offloads |= RTE_ETH_TX_OFFLOAD_TCP_TSO;
 		}