[dpdk-dev] [PATCH v6 07/10] ipsec: add support for NAT-T

[Nicolau, Radu]

On 9/27/2021 3:55 PM, Ananyev, Konstantin wrote:
>   
>> On 9/23/2021 5:43 PM, Ananyev, Konstantin wrote:
>>>> Add support for the IPsec NAT-Traversal use case for Tunnel mode
>>>> packets.
>>>>
>>>> Signed-off-by: Declan Doherty <declan.doherty at intel.com>
>>>> Signed-off-by: Radu Nicolau <radu.nicolau at intel.com>
>>>> Signed-off-by: Abhijit Sinha <abhijit.sinha at intel.com>
>>>> Signed-off-by: Daniel Martin Buckley <daniel.m.buckley at intel.com>
>>>> Acked-by: Fan Zhang <roy.fan.zhang at intel.com>
>>>> ---
>>>>    lib/ipsec/iph.h          | 17 +++++++++++++++++
>>>>    lib/ipsec/rte_ipsec_sa.h |  8 +++++++-
>>>>    lib/ipsec/sa.c           | 13 ++++++++++++-
>>>>    lib/ipsec/sa.h           |  4 ++++
>>>>    4 files changed, 40 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/lib/ipsec/iph.h b/lib/ipsec/iph.h
>>>> index 2d223199ac..c5c213a2b4 100644
>>>> --- a/lib/ipsec/iph.h
>>>> +++ b/lib/ipsec/iph.h
>>>> @@ -251,6 +251,7 @@ update_tun_outb_l3hdr(const struct rte_ipsec_sa *sa, void *outh,
>>>>    {
>>>>    	struct rte_ipv4_hdr *v4h;
>>>>    	struct rte_ipv6_hdr *v6h;
>>>> +	struct rte_udp_hdr *udph;
>>>>    	uint8_t is_outh_ipv4;
>>>>
>>>>    	if (sa->type & RTE_IPSEC_SATP_MODE_TUNLV4) {
>>>> @@ -258,11 +259,27 @@ update_tun_outb_l3hdr(const struct rte_ipsec_sa *sa, void *outh,
>>>>    		v4h = outh;
>>>>    		v4h->packet_id = pid;
>>>>    		v4h->total_length = rte_cpu_to_be_16(plen - l2len);
>>>> +
>>>> +		if (sa->type & RTE_IPSEC_SATP_NATT_ENABLE) {
>>>> +			udph = (struct rte_udp_hdr *)(v4h + 1);
>>>> +			udph->dst_port = sa->natt.dport;
>>>> +			udph->src_port = sa->natt.sport;
>>>> +			udph->dgram_len = rte_cpu_to_be_16(plen - l2len -
>>>> +				(sizeof(*v4h) + sizeof(*udph)));
>>>> +		}
>>>>    	} else {
>>>>    		is_outh_ipv4 = 0;
>>>>    		v6h = outh;
>>>>    		v6h->payload_len = rte_cpu_to_be_16(plen - l2len -
>>>>    				sizeof(*v6h));
>>>> +
>>>> +		if (sa->type & RTE_IPSEC_SATP_NATT_ENABLE) {
>>>> +			udph = (struct rte_udp_hdr *)(v6h + 1);
>>> Why you presume there would be always ipv6 with no options?
>>> Shouldn't we use hdr_l3_len provided by user?
>> Yes, I will use hdr_l3_len.
>>
>>> Another thing - I am not sure we need 'natt' field in rte_ipsec_sa at all.
>>> UDP header (sport, dport) is consitant and could be part of header template
>>> provided by user at sa initialization time.
>> The rte_security_ipsec_sa_options::udp_encap flag assumes that the UDP
>> encapsulation i.e. adding the header is not the responsibility of the
>> user, so we can append it (transparently to the user) to the header
>> template but the user should not do it. Will this work?
> Interesting idea, I suppose that should work...
> Do I get it right, this udp header will always be appended to the end of
> user provided tun.hdr?
Yes. So normally after whatever user puts in we insert the ESP header. 
When the UDP encapsulation is enabled we should insert the UDP header 
before the ESP header, so this arrangement should work.
>
>>
>>>> +			udph->dst_port = sa->natt.dport;
>>>> +			udph->src_port = sa->natt.sport;
>>>> +			udph->dgram_len = rte_cpu_to_be_16(plen - l2len -
>>>> +				(sizeof(*v6h) + sizeof(*udph)));
>>> Whose responsibility will be to update cksum field?
>> According to the RFC it should be zero and the rx side must not
>> check/use it. I will set it as zero