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

Message ID 20220207062641.26574-2-ndabilpuram@marvell.com (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series [v2,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. 7, 2022, 6:26 a.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

Akhil Goyal Feb. 17, 2022, 6:12 p.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. 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>
> ---
Acked-by: Akhil Goyal <gakhil@marvell.com>
  
Ananyev, Konstantin Feb. 17, 2022, 7:22 p.m. UTC | #2
> 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.

I am still not sure this is a right thing to do.
Could you explain what will happen for lookaside modes?
Would they always fall-back to SW cksum calculation?

> 
> 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;
 		}