From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id AF402A034C; Wed, 23 Feb 2022 10:59:02 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8AC6840E5A; Wed, 23 Feb 2022 10:59:02 +0100 (CET) Received: from mx0b-0016f401.pphosted.com (mx0b-0016f401.pphosted.com [67.231.156.173]) by mails.dpdk.org (Postfix) with ESMTP id 6036240DF6 for ; Wed, 23 Feb 2022 10:59:01 +0100 (CET) Received: from pps.filterd (m0045851.ppops.net [127.0.0.1]) by mx0b-0016f401.pphosted.com (8.16.1.2/8.16.1.2) with ESMTP id 21N9buUw012775; Wed, 23 Feb 2022 01:59:00 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.com; h=message-id : date : mime-version : subject : from : to : cc : references : in-reply-to : content-type : content-transfer-encoding; s=pfpt0220; bh=aK87giPrGD/WXakdUM+YyS5Z7Y3DjpCOoIvxQ/1qtQQ=; b=av0mFtmUgBKxuqStjHO5zcdTRDwf45borOo5oRbGAHmPrNejedC2zXzkl0QaeaDpXRXu tUNuf3qGKAEWos6i/4UHUpK4frVCcMTZwl87Exklj7HLWmMEQr4vzgaQcNfUnBtQMWXx 6YgfL9W+u89K18qzSnD+nOhxJB13Hc7oSZX4DaBzrBakDnnjYlgLHvq71BBQiQcoPmXX lSYNKAkRrQF9QxxesvxHGy3D2BigEd4WzyikoBUXW6KyXRgH2z8lernrA8rt5ODsrjCH ntbkGvI6pzo3apils1rAuOta2TKpDT1TTW0BtIuIQm/5VwhtdJTTULj/UByAJXQefT40 OQ== Received: from dc5-exch02.marvell.com ([199.233.59.182]) by mx0b-0016f401.pphosted.com (PPS) with ESMTPS id 3edjerg2b4-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Wed, 23 Feb 2022 01:59:00 -0800 Received: from DC5-EXCH02.marvell.com (10.69.176.39) by DC5-EXCH02.marvell.com (10.69.176.39) with Microsoft SMTP Server (TLS) id 15.0.1497.18; Wed, 23 Feb 2022 01:58:58 -0800 Received: from maili.marvell.com (10.69.176.80) by DC5-EXCH02.marvell.com (10.69.176.39) with Microsoft SMTP Server id 15.0.1497.18 via Frontend Transport; Wed, 23 Feb 2022 01:58:58 -0800 Received: from [10.28.175.194] (PE-LT1350.marvell.com [10.28.175.194]) by maili.marvell.com (Postfix) with ESMTP id C46D83F7088; Wed, 23 Feb 2022 01:58:56 -0800 (PST) Message-ID: Date: Wed, 23 Feb 2022 15:28:55 +0530 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.6.1 Subject: Re: [PATCH 2/4] examples/ipsec-secgw: disable Tx chksum offload for inline Content-Language: en-US From: Nithin Kumar Dabilpuram To: "Ananyev, Konstantin" , "dev@dpdk.org" CC: "Nicolau, Radu" , "gakhil@marvell.com" References: <20220206143022.13098-1-ndabilpuram@marvell.com> <20220206143022.13098-2-ndabilpuram@marvell.com> <01076921-41f5-97a9-9c02-808ac6a91be7@marvell.com> In-Reply-To: <01076921-41f5-97a9-9c02-808ac6a91be7@marvell.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Proofpoint-ORIG-GUID: 709g7YHZy6fLdARpGE5OTb6uqgV3yjc1 X-Proofpoint-GUID: 709g7YHZy6fLdARpGE5OTb6uqgV3yjc1 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.816,Hydra:6.0.425,FMLib:17.11.64.514 definitions=2022-02-23_03,2022-02-23_01,2022-02-23_01 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 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 >>>>> --- >>>>>    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 >>>>