Re: [dpdk-dev] [PATCH v3 2/2] examples/ipsec-secgw: add support for TSO

["Nicolau, Radu" <radu.nicolau@intel.com>]

On 10/27/2021 1:02 PM, Ananyev, Konstantin wrote:
>
>> Add support to allow user to specific MSS for TSO offload on a per SA
>> basis. MSS configuration in the context of IPsec is only supported for
>> outbound SA's in the context of an inline IPsec Crypto offload.
>>
>> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
>> ---
>>   doc/guides/rel_notes/release_21_11.rst   |  4 ++++
>>   doc/guides/sample_app_ug/ipsec_secgw.rst | 11 +++++++++++
>>   examples/ipsec-secgw/ipsec-secgw.c       |  4 ++++
>>   examples/ipsec-secgw/ipsec.h             |  1 +
>>   examples/ipsec-secgw/ipsec_process.c     | 25 ++++++++++++++++++++++++
>>   examples/ipsec-secgw/sa.c                | 21 ++++++++++++++++++--
>>   6 files changed, 64 insertions(+), 2 deletions(-)
>>
>> diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
>> index b5b5abadee..8d1767b084 100644
>> --- a/doc/guides/rel_notes/release_21_11.rst
>> +++ b/doc/guides/rel_notes/release_21_11.rst
>> @@ -306,6 +306,10 @@ New Features
>>       * Pcapng format with timestamps and meta-data.
>>       * Fixes packet capture with stripped VLAN tags.
>>
>> +* **IPsec Security Gateway sample application new features.**
>> +
>> +  * Added support for TSO
>> +
>>
>>   Removed Items
>>   -------------
>> diff --git a/doc/guides/sample_app_ug/ipsec_secgw.rst b/doc/guides/sample_app_ug/ipsec_secgw.rst
>> index 782574dd39..639d309a6e 100644
>> --- a/doc/guides/sample_app_ug/ipsec_secgw.rst
>> +++ b/doc/guides/sample_app_ug/ipsec_secgw.rst
>> @@ -720,6 +720,17 @@ where each options means:
>>
>>      * *udp-encap*
>>
>> + ``<mss>``
>> +
>> + * Maximum segment size for TSO offload, available for egress SAs only.
>> +
>> + * Optional: Yes, TSO offload not set by default
>> +
>> + * Syntax:
>> +
>> +   * *mss N* N is the segment size in bytes
>> +
>> +
>>   Example SA rules:
>>
>>   .. code-block:: console
>> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
>> index 4bdf99b62b..5fcf424efe 100644
>> --- a/examples/ipsec-secgw/ipsec-secgw.c
>> +++ b/examples/ipsec-secgw/ipsec-secgw.c
>> @@ -398,6 +398,10 @@ prepare_one_packet(struct rte_mbuf *pkt, struct ipsec_traffic *t)
>>   		pkt->l2_len = 0;
>>   		pkt->l3_len = sizeof(*iph4);
>>   		pkt->packet_type |= RTE_PTYPE_L3_IPV4;
>> +		if  (pkt->packet_type & RTE_PTYPE_L4_TCP)
>> +			pkt->l4_len = sizeof(struct rte_tcp_hdr);
>> +		else if (pkt->packet_type & RTE_PTYPE_L4_UDP)
>> +			pkt->l4_len = sizeof(struct rte_udp_hdr);
>>   	} else if (eth->ether_type == rte_cpu_to_be_16(RTE_ETHER_TYPE_IPV6)) {
>>   		int next_proto;
>>   		size_t l3len, ext_len;
>> diff --git a/examples/ipsec-secgw/ipsec.h b/examples/ipsec-secgw/ipsec.h
>> index 8405c48171..2c3640833d 100644
>> --- a/examples/ipsec-secgw/ipsec.h
>> +++ b/examples/ipsec-secgw/ipsec.h
>> @@ -137,6 +137,7 @@ struct ipsec_sa {
>>   	enum rte_security_ipsec_sa_direction direction;
>>   	uint8_t udp_encap;
>>   	uint16_t portid;
>> +	uint16_t mss;
>>   	uint8_t fdir_qid;
>>   	uint8_t fdir_flag;
>>
>> diff --git a/examples/ipsec-secgw/ipsec_process.c b/examples/ipsec-secgw/ipsec_process.c
>> index 5012e1a6a4..26c6c2fe84 100644
>> --- a/examples/ipsec-secgw/ipsec_process.c
>> +++ b/examples/ipsec-secgw/ipsec_process.c
>> @@ -222,6 +222,31 @@ prep_process_group(void *sa, struct rte_mbuf *mb[], uint32_t cnt)
>>   	for (j = 0; j != cnt; j++) {
>>   		priv = get_priv(mb[j]);
>>   		priv->sa = sa;
>> +		/* setup TSO related fields if TSO enabled*/
>> +		if (priv->sa->mss) {
>> +			mb[j]->tso_segsz = priv->sa->mss;
>> +
>> +			if ((IS_TUNNEL(priv->sa->flags))) {
>> +				mb[j]->outer_l3_len = mb[j]->l3_len;
>> +				mb[j]->outer_l2_len = mb[j]->l2_len;
>> +				mb[j]->ol_flags |=
>> +				(RTE_MBUF_F_TX_OUTER_IP_CKSUM |
>> +						RTE_MBUF_F_TX_TUNNEL_ESP);
>> +			}
>> +			uint32_t ptype = mb[j]->packet_type;
>> +			if  (ptype & RTE_PTYPE_L4_TCP)
>> +				mb[j]->ol_flags |=
>> +						(RTE_MBUF_F_TX_TCP_SEG |
>> +						RTE_MBUF_F_TX_TCP_CKSUM);
>> +			else
>> +				mb[j]->ol_flags |=
>> +					(RTE_MBUF_F_TX_UDP_SEG |
>> +						RTE_MBUF_F_TX_UDP_CKSUM);
> Could it be that packet is neither TCP nor UDP?
Yes, I will add a third branch for unsupported packet type.
>
>> +			if (RTE_ETH_IS_IPV4_HDR(ptype))
>> +				mb[j]->ol_flags |= RTE_MBUF_F_TX_OUTER_IPV4;
>> +			else
>> +				mb[j]->ol_flags |= RTE_MBUF_F_TX_OUTER_IPV6;
>> +		}
>>   	}
>>   }
>>
>> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
>> index 88dd30464f..e8815dffe7 100644
>> --- a/examples/ipsec-secgw/sa.c
>> +++ b/examples/ipsec-secgw/sa.c
>> @@ -677,6 +677,16 @@ parse_sa_tokens(char **tokens, uint32_t n_tokens,
>>   			continue;
>>   		}
>>
>> +		if (strcmp(tokens[ti], "mss") == 0) {
>> +			INCREMENT_TOKEN_INDEX(ti, n_tokens, status);
>> +			if (status->status < 0)
>> +				return;
>> +			rule->mss = atoi(tokens[ti]);
>> +			if (status->status < 0)
>> +				return;
>> +			continue;
>> +		}
>> +
>>   		if (strcmp(tokens[ti], "fallback") == 0) {
>>   			struct rte_ipsec_session *fb;
>>
>> @@ -970,7 +980,7 @@ sa_create(const char *name, int32_t socket_id, uint32_t nb_sa)
>>   }
>>
>>   static int
>> -check_eth_dev_caps(uint16_t portid, uint32_t inbound)
>> +check_eth_dev_caps(uint16_t portid, uint32_t inbound, uint32_t tso)
>>   {
>>   	struct rte_eth_dev_info dev_info;
>>   	int retval;
>> @@ -999,6 +1009,13 @@ check_eth_dev_caps(uint16_t portid, uint32_t inbound)
>>   				"hardware TX IPSec offload is not supported\n");
>>   			return -EINVAL;
>>   		}
>> +		if (tso && (dev_info.tx_offload_capa &
>> +				(RTE_ETH_TX_OFFLOAD_TCP_TSO |
>> +					RTE_ETH_TX_OFFLOAD_UDP_TSO)) == 0) {
> Shouldn't it be:
> dev_info.tx_offload_capa & (RTE_ETH_TX_OFFLOAD_TCP_TSO | RTE_ETH_TX_OFFLOAD_UDP_TSO)) ==
> RTE_ETH_TX_OFFLOAD_TCP_TSO | RTE_ETH_TX_OFFLOAD_UDP_TSO)
> ?
It should be that but negated, if none of the flags are set we exit - we 
can't tell at this point if we need TCP or UDP.
>
>> +			RTE_LOG(WARNING, PORT,
>> +				"hardware TSO offload is not supported\n");
>> +			return -EINVAL;
>> +		}
>>   	}
>>   	return 0;
>>   }
> I think you missed changes in a_check_offloads().
> That's where we specify which HW offloads should be enabled for given port.
Yes.
>
>
>> @@ -1127,7 +1144,7 @@ sa_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
>>
>>   		if (ips->type == RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL ||
>>   			ips->type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO) {
>> -			if (check_eth_dev_caps(sa->portid, inbound))
>> +			if (check_eth_dev_caps(sa->portid, inbound, sa->mss))
>>   				return -EINVAL;
>>   		}
>>
>> --
>> 2.25.1