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

[Ananyev, Konstantin]

> 
> 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.

Ok, thanks for clarification.
Looks like a good approach to me.