DPDK patches and discussions
 help / color / mirror / Atom feed
From: Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
	"jerinj@marvell.com" <jerinj@marvell.com>,
	"Nicolau, Radu" <radu.nicolau@intel.com>,
	Akhil Goyal <gakhil@marvell.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, "anoobj@marvell.com" <anoobj@marvell.com>
Subject: Re: [PATCH 1/7] examples/ipsec-secgw: disable Tx chksum offload for inline
Date: Tue, 19 Apr 2022 19:26:31 +0530	[thread overview]
Message-ID: <bd23af01-587a-616a-8a87-396103d11ddf@marvell.com> (raw)
In-Reply-To: <DM6PR11MB4491A330DEE415C2E39524209AEF9@DM6PR11MB4491.namprd11.prod.outlook.com>

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 <ndabilpuram@marvell.com>
>> ---
>>   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
> 

  reply	other threads:[~2022-04-19 13:56 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-22 17:58 Nithin Dabilpuram
2022-03-22 17:58 ` [PATCH 2/7] examples/ipsec-secgw: use HW parsed packet type in poll mode Nithin Dabilpuram
2022-04-14 15:43   ` Ananyev, Konstantin
2022-03-22 17:58 ` [PATCH 3/7] examples/ipsec-secgw: allow larger burst size for vectors Nithin Dabilpuram
2022-03-22 17:58 ` [PATCH 4/7] examples/ipsec-secgw: move fast path helper functions Nithin Dabilpuram
2022-03-22 17:58 ` [PATCH 5/7] examples/ipsec-secgw: get security context from lcore conf Nithin Dabilpuram
2022-03-22 17:58 ` [PATCH 6/7] examples/ipsec-secgw: update eth header during route lookup Nithin Dabilpuram
2022-03-22 17:58 ` [PATCH 7/7] examples/ipsec-secgw: add poll mode worker for inline proto Nithin Dabilpuram
2022-04-13  6:13 ` [PATCH 1/7] examples/ipsec-secgw: disable Tx chksum offload for inline Nithin Kumar Dabilpuram
2022-04-14 14:07 ` Ananyev, Konstantin
2022-04-19 13:56   ` Nithin Kumar Dabilpuram [this message]
2022-04-20 10:42     ` Ananyev, Konstantin
2022-04-21 13:31 ` [PATCH v2 1/7] examples/ipsec-secgw: move fast path helper functions Nithin Dabilpuram
2022-04-21 13:31   ` [PATCH v2 2/7] examples/ipsec-secgw: disable Tx chksum offload for inline Nithin Dabilpuram
2022-04-21 13:31   ` [PATCH v2 3/7] examples/ipsec-secgw: use HW parsed packet type in poll mode Nithin Dabilpuram
2022-04-21 13:31   ` [PATCH v2 4/7] examples/ipsec-secgw: allow larger burst size for vectors Nithin Dabilpuram
2022-04-21 13:31   ` [PATCH v2 5/7] examples/ipsec-secgw: get security context from lcore conf Nithin Dabilpuram
2022-04-21 13:31   ` [PATCH v2 6/7] examples/ipsec-secgw: update eth header during route lookup Nithin Dabilpuram
2022-04-21 13:31   ` [PATCH v2 7/7] examples/ipsec-secgw: add poll mode worker for inline proto Nithin Dabilpuram
2022-04-28 15:04 ` [PATCH v3 1/7] examples/ipsec-secgw: move fast path helper functions Nithin Dabilpuram
2022-04-28 15:04   ` [PATCH v3 2/7] examples/ipsec-secgw: disable Tx chksum offload for inline Nithin Dabilpuram
2022-04-28 15:04   ` [PATCH v3 3/7] examples/ipsec-secgw: use HW parsed packet type in poll mode Nithin Dabilpuram
2022-04-28 15:04   ` [PATCH v3 4/7] examples/ipsec-secgw: allow larger burst size for vectors Nithin Dabilpuram
2022-04-28 15:04   ` [PATCH v3 5/7] examples/ipsec-secgw: get security context from lcore conf Nithin Dabilpuram
2022-04-28 15:04   ` [PATCH v3 6/7] examples/ipsec-secgw: update eth header during route lookup Nithin Dabilpuram
2022-04-28 15:04   ` [PATCH v3 7/7] examples/ipsec-secgw: add poll mode worker for inline proto Nithin Dabilpuram
2022-04-29 10:23   ` [PATCH v3 1/7] examples/ipsec-secgw: move fast path helper functions Nithin Kumar Dabilpuram
2022-04-29 10:29   ` Akhil Goyal
2022-04-29 20:44 ` [PATCH v4 " Nithin Dabilpuram
2022-04-29 20:44   ` [PATCH v4 2/7] examples/ipsec-secgw: disable Tx chksum offload for inline Nithin Dabilpuram
2022-05-01 17:10     ` Konstantin Ananyev
2022-04-29 20:44   ` [PATCH v4 3/7] examples/ipsec-secgw: use HW parsed packet type in poll mode Nithin Dabilpuram
2022-04-29 20:44   ` [PATCH v4 4/7] examples/ipsec-secgw: allow larger burst size for vectors Nithin Dabilpuram
2022-04-29 20:44   ` [PATCH v4 5/7] examples/ipsec-secgw: get security context from lcore conf Nithin Dabilpuram
2022-04-29 20:44   ` [PATCH v4 6/7] examples/ipsec-secgw: update eth header during route lookup Nithin Dabilpuram
2022-04-29 20:44   ` [PATCH v4 7/7] examples/ipsec-secgw: add poll mode worker for inline proto Nithin Dabilpuram
2022-05-11 19:34   ` [PATCH v4 1/7] examples/ipsec-secgw: move fast path helper functions Akhil Goyal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bd23af01-587a-616a-8a87-396103d11ddf@marvell.com \
    --to=ndabilpuram@marvell.com \
    --cc=anoobj@marvell.com \
    --cc=dev@dpdk.org \
    --cc=gakhil@marvell.com \
    --cc=jerinj@marvell.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=radu.nicolau@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).