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 CE5F3A00BE; Tue, 19 Apr 2022 15:56:40 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 718AC4068E; Tue, 19 Apr 2022 15:56:40 +0200 (CEST) Received: from mx0b-0016f401.pphosted.com (mx0b-0016f401.pphosted.com [67.231.156.173]) by mails.dpdk.org (Postfix) with ESMTP id 6540B40687 for ; Tue, 19 Apr 2022 15:56:38 +0200 (CEST) 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 23JB3Xat025381; Tue, 19 Apr 2022 06:56:37 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.com; h=message-id : date : mime-version : subject : to : cc : references : from : in-reply-to : content-type : content-transfer-encoding; s=pfpt0220; bh=yoffoeP5/hAYASGIY4R4KSS3GXLHTD0IhyctB5OvDJ0=; b=jE3ENSHq8KUiIGGhPcUmkUMqIErE/W3+7T0tmOwMCytRP+a+JDjHoIF5COeXvol2IFez 5wmYaZTDBfIpuUxNw2OXobCpiSncTblyZRQI9j9KeNBXFiz2A/UdJ7Ps7F/Xt6Zz0ece yLavN7Xvfqb/lnavLcNrsNVnVSPW5UGUzzZiPzUXC0qwrg50xtM4pRR4boPEj54g7844 /Pcj2D9Lu7+Cqgaw/8+0j1qwN1G5Pi9rVtlMebPZvpwWItBQCeQ+5bxld48zGWFXCtWq Mtjgwj+1xZBvEGEIsAbDMW29ak+O+KtOH+4z6eAhHS0SZ6Eij4WQwy4X8IxxQ/jdT3sU hQ== Received: from dc5-exch02.marvell.com ([199.233.59.182]) by mx0b-0016f401.pphosted.com (PPS) with ESMTPS id 3fhtap8ujx-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Tue, 19 Apr 2022 06:56:37 -0700 Received: from DC5-EXCH01.marvell.com (10.69.176.38) by DC5-EXCH02.marvell.com (10.69.176.39) with Microsoft SMTP Server (TLS) id 15.0.1497.18; Tue, 19 Apr 2022 06:56:35 -0700 Received: from maili.marvell.com (10.69.176.80) by DC5-EXCH01.marvell.com (10.69.176.38) with Microsoft SMTP Server id 15.0.1497.2 via Frontend Transport; Tue, 19 Apr 2022 06:56:35 -0700 Received: from [10.28.175.194] (PE-LT1350.marvell.com [10.28.175.194]) by maili.marvell.com (Postfix) with ESMTP id BDE145B6923; Tue, 19 Apr 2022 06:56:32 -0700 (PDT) Message-ID: Date: Tue, 19 Apr 2022 19:26:31 +0530 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.8.0 Subject: Re: [PATCH 1/7] examples/ipsec-secgw: disable Tx chksum offload for inline Content-Language: en-US To: "Ananyev, Konstantin" , "jerinj@marvell.com" , "Nicolau, Radu" , Akhil Goyal CC: "dev@dpdk.org" , "anoobj@marvell.com" References: <20220322175902.363520-1-ndabilpuram@marvell.com> From: Nithin Kumar Dabilpuram In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-Proofpoint-GUID: _P1KUrZUO8JhykTAUrCgOLby0HCoPTv_ X-Proofpoint-ORIG-GUID: _P1KUrZUO8JhykTAUrCgOLby0HCoPTv_ X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.858,Hydra:6.0.486,FMLib:17.11.64.514 definitions=2022-04-19_05,2022-04-15_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 Hi Konstantin, Please see inline. On 4/14/22 7:37 PM, Ananyev, Konstantin wrote: > Hi Nithin, > > >> Enable Tx IPv4 checksum offload only when Tx inline crypto, lookaside >> crypto/protocol or cpu crypto is needed. >> For Tx Inline protocol offload, checksum computation >> is implicitly taken care by HW. > > The thing is that right now it is not stated explicitly that > RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL implies TSO support. It says that it 'might', it is not guaranteed. > At least in dpdk docs. > From https://doc.dpdk.org/guides/prog_guide/rte_security.html: > "22.1.2. Inline protocol offload > ... > Egress Data path - The software will send the plain packet without any security protocol headers added to the packet. The driver will configure the security index and other requirement in tx descriptors. The hardware device will do security processing on the packet that includes adding the relevant protocol headers and encrypting the data before sending the packet out. The software should make sure that the buffer has required head room and tail room for any protocol header addition. The software may also do early fragmentation if the resultant packet is expected to cross the MTU size. > Note > The underlying device will manage state information required for egress processing. E.g. in case of IPsec, the seq number will be added to the packet, however the device shall provide indication when the sequence number is about to overflow. The underlying device may support post encryption TSO." > > So, if I am not mistaken, what you suggest will change HW/PMD requirements. > AFAIK, right now only Marvell supports RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL, > so in theory I don't mind if you'd like to harden the requirements here. > Though such change probably needs to be properly documented and > acked by other vendors. Ok, I was only thinking of IPV4 CKSUM offload without TSO and thought that is not needed in case of INLINE PROTOCOL. To maintain the behavior for TSO with INLINE_PROTO, I can set both IPV4_CKSUM offload and TCP_TSO if TSO is requested i.e rule->mss is set. We can revist the spec for TSO+INLINE_PROTOCOL offload combination later as our HW doesn't support TSO before INLINE IPSEC Processing. > >> >> Signed-off-by: Nithin Dabilpuram >> --- >> examples/ipsec-secgw/ipsec-secgw.c | 3 --- >> examples/ipsec-secgw/sa.c | 32 +++++++++++++++++++++++++------- >> 2 files changed, 25 insertions(+), 10 deletions(-) >> >> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c >> index 42b5081..76919e5 100644 >> --- a/examples/ipsec-secgw/ipsec-secgw.c >> +++ b/examples/ipsec-secgw/ipsec-secgw.c >> @@ -2330,9 +2330,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..36d890f 100644 >> --- a/examples/ipsec-secgw/sa.c >> +++ b/examples/ipsec-secgw/sa.c >> @@ -1785,13 +1785,31 @@ sa_check_offloads(uint16_t port_id, uint64_t *rx_offloads, >> for (idx_sa = 0; idx_sa < nb_sa_out; idx_sa++) { >> rule = &sa_out[idx_sa]; >> rule_type = ipsec_get_action_type(rule); >> - if ((rule_type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO || >> - rule_type == >> - RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL) >> - && rule->portid == port_id) { >> - *tx_offloads |= RTE_ETH_TX_OFFLOAD_SECURITY; >> - if (rule->mss) >> - *tx_offloads |= RTE_ETH_TX_OFFLOAD_TCP_TSO; >> + switch (rule_type) { >> + case RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL: >> + /* 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->portid == port_id) >> + *tx_offloads |= RTE_ETH_TX_OFFLOAD_SECURITY; >> + break; >> + case RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO: >> + if (rule->portid == port_id) { >> + *tx_offloads |= RTE_ETH_TX_OFFLOAD_SECURITY; >> + if (rule->mss) >> + *tx_offloads |= >> + RTE_ETH_TX_OFFLOAD_TCP_TSO; >> + *tx_offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM; >> + } >> + break; >> + default: >> + /* Enable IPv4 checksum offload even if one of lookaside >> + * SA's are present. >> + */ >> + *tx_offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM; > > Shouldn't we check here that given port really supports IPV4_CKSUM offload? It is already being checked at port_init(). > >> + break; >> } >> } >> return 0; >> -- >> 2.8.4 >