> >> 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 <ndabilpu...@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
> >
