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

[Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>]

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
>