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

[Ananyev, Konstantin]

> >> 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 at 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
> >