diff mbox series

[v6,07/10] ipsec: add support for NAT-T

Message ID 20210917091747.1528262-8-radu.nicolau@intel.com (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series new features for ipsec and security libraries |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Radu Nicolau Sept. 17, 2021, 9:17 a.m. UTC 
  Add support for the IPsec NAT-Traversal use case for Tunnel mode
packets.

Signed-off-by: Declan Doherty <declan.doherty@intel.com>
Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
Signed-off-by: Abhijit Sinha <abhijit.sinha@intel.com>
Signed-off-by: Daniel Martin Buckley <daniel.m.buckley@intel.com>
Acked-by: Fan Zhang <roy.fan.zhang@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(-)

Comments

Ananyev, Konstantin Sept. 23, 2021, 4:43 p.m. UTC | #1 
> 
> Add support for the IPsec NAT-Traversal use case for Tunnel mode
> packets.
> 
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> Signed-off-by: Abhijit Sinha <abhijit.sinha@intel.com>
> Signed-off-by: Daniel Martin Buckley <daniel.m.buckley@intel.com>
> Acked-by: Fan Zhang <roy.fan.zhang@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?
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.

> +			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?

> +		}
>  	}
> 
>  	if (sa->type & TUN_HDR_MSK)
> diff --git a/lib/ipsec/rte_ipsec_sa.h b/lib/ipsec/rte_ipsec_sa.h
> index cf51ad8338..40d1e70d45 100644
> --- a/lib/ipsec/rte_ipsec_sa.h
> +++ b/lib/ipsec/rte_ipsec_sa.h
> @@ -76,6 +76,7 @@ struct rte_ipsec_sa_prm {
>   * - inbound/outbound
>   * - mode (TRANSPORT/TUNNEL)
>   * - for TUNNEL outer IP version (IPv4/IPv6)
> + * - NAT-T UDP encapsulated (TUNNEL mode only)
>   * - are SA SQN operations 'atomic'
>   * - ESN enabled/disabled
>   * ...
> @@ -86,7 +87,8 @@ enum {
>  	RTE_SATP_LOG2_PROTO,
>  	RTE_SATP_LOG2_DIR,
>  	RTE_SATP_LOG2_MODE,
> -	RTE_SATP_LOG2_SQN = RTE_SATP_LOG2_MODE + 2,
> +	RTE_SATP_LOG2_NATT = RTE_SATP_LOG2_MODE + 2,

Why to insert it in the middle?
Why not to add to the end, as usually people do for new options?

> +	RTE_SATP_LOG2_SQN,
>  	RTE_SATP_LOG2_ESN,
>  	RTE_SATP_LOG2_ECN,
>  	RTE_SATP_LOG2_DSCP
> @@ -109,6 +111,10 @@ enum {
>  #define RTE_IPSEC_SATP_MODE_TUNLV4	(1ULL << RTE_SATP_LOG2_MODE)
>  #define RTE_IPSEC_SATP_MODE_TUNLV6	(2ULL << RTE_SATP_LOG2_MODE)
> 
> +#define RTE_IPSEC_SATP_NATT_MASK	(1ULL << RTE_SATP_LOG2_NATT)
> +#define RTE_IPSEC_SATP_NATT_DISABLE	(0ULL << RTE_SATP_LOG2_NATT)
> +#define RTE_IPSEC_SATP_NATT_ENABLE	(1ULL << RTE_SATP_LOG2_NATT)
> +
>  #define RTE_IPSEC_SATP_SQN_MASK		(1ULL << RTE_SATP_LOG2_SQN)
>  #define RTE_IPSEC_SATP_SQN_RAW		(0ULL << RTE_SATP_LOG2_SQN)
>  #define RTE_IPSEC_SATP_SQN_ATOM		(1ULL << RTE_SATP_LOG2_SQN)
> diff --git a/lib/ipsec/sa.c b/lib/ipsec/sa.c
> index 2ecbbce0a4..8e369e4618 100644
> --- a/lib/ipsec/sa.c
> +++ b/lib/ipsec/sa.c
> @@ -217,6 +217,10 @@ fill_sa_type(const struct rte_ipsec_sa_prm *prm, uint64_t *type)
>  	} else
>  		return -EINVAL;
> 
> +	/* check for UDP encapsulation flag */
> +	if (prm->ipsec_xform.options.udp_encap == 1)
> +		tp |= RTE_IPSEC_SATP_NATT_ENABLE;
> +
>  	/* check for ESN flag */
>  	if (prm->ipsec_xform.options.esn == 0)
>  		tp |= RTE_IPSEC_SATP_ESN_DISABLE;
> @@ -372,7 +376,8 @@ esp_sa_init(struct rte_ipsec_sa *sa, const struct rte_ipsec_sa_prm *prm,
>  	const struct crypto_xform *cxf)
>  {
>  	static const uint64_t msk = RTE_IPSEC_SATP_DIR_MASK |
> -				RTE_IPSEC_SATP_MODE_MASK;
> +				RTE_IPSEC_SATP_MODE_MASK |
> +				RTE_IPSEC_SATP_NATT_MASK;
> 
>  	if (prm->ipsec_xform.options.ecn)
>  		sa->tos_mask |= RTE_IPV4_HDR_ECN_MASK;
> @@ -475,10 +480,16 @@ esp_sa_init(struct rte_ipsec_sa *sa, const struct rte_ipsec_sa_prm *prm,
>  	case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TRANS):
>  		esp_inb_init(sa);
>  		break;
> +	case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV4 |
> +			RTE_IPSEC_SATP_NATT_ENABLE):
> +	case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV6 |
> +			RTE_IPSEC_SATP_NATT_ENABLE):
>  	case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV4):
>  	case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV6):
>  		esp_outb_tun_init(sa, prm);
>  		break;
> +	case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TRANS |
> +			RTE_IPSEC_SATP_NATT_ENABLE):
>  	case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TRANS):
>  		esp_outb_init(sa, 0);
>  		break;
> diff --git a/lib/ipsec/sa.h b/lib/ipsec/sa.h
> index 5e237f3525..3f38921eb3 100644
> --- a/lib/ipsec/sa.h
> +++ b/lib/ipsec/sa.h
> @@ -101,6 +101,10 @@ struct rte_ipsec_sa {
>  		uint64_t msk;
>  		uint64_t val;
>  	} tx_offload;
> +	struct {
> +		uint16_t sport;
> +		uint16_t dport;
> +	} natt;
>  	uint32_t salt;
>  	uint8_t algo_type;
>  	uint8_t proto;    /* next proto */
> --
> 2.25.1
Radu Nicolau Sept. 27, 2021, 1:27 p.m. UTC | #2 
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@intel.com>
>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
>> Signed-off-by: Abhijit Sinha <abhijit.sinha@intel.com>
>> Signed-off-by: Daniel Martin Buckley <daniel.m.buckley@intel.com>
>> Acked-by: Fan Zhang <roy.fan.zhang@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?


>
>> +			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
Ananyev, Konstantin Sept. 27, 2021, 2:55 p.m. UTC | #3 
> 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@intel.com>
> >> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> >> Signed-off-by: Abhijit Sinha <abhijit.sinha@intel.com>
> >> Signed-off-by: Daniel Martin Buckley <daniel.m.buckley@intel.com>
> >> Acked-by: Fan Zhang <roy.fan.zhang@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?

> 
> 
> >
> >> +			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
Radu Nicolau Sept. 27, 2021, 3:06 p.m. UTC | #4 
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@intel.com>
>>>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
>>>> Signed-off-by: Abhijit Sinha <abhijit.sinha@intel.com>
>>>> Signed-off-by: Daniel Martin Buckley <daniel.m.buckley@intel.com>
>>>> Acked-by: Fan Zhang <roy.fan.zhang@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
Ananyev, Konstantin Sept. 27, 2021, 3:39 p.m. UTC | #5 
> 
> 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@intel.com>
> >>>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> >>>> Signed-off-by: Abhijit Sinha <abhijit.sinha@intel.com>
> >>>> Signed-off-by: Daniel Martin Buckley <daniel.m.buckley@intel.com>
> >>>> Acked-by: Fan Zhang <roy.fan.zhang@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.
diff mbox series

Patch

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);
+			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)));
+		}
 	}
 
 	if (sa->type & TUN_HDR_MSK)
diff --git a/lib/ipsec/rte_ipsec_sa.h b/lib/ipsec/rte_ipsec_sa.h
index cf51ad8338..40d1e70d45 100644
--- a/lib/ipsec/rte_ipsec_sa.h
+++ b/lib/ipsec/rte_ipsec_sa.h
@@ -76,6 +76,7 @@  struct rte_ipsec_sa_prm {
  * - inbound/outbound
  * - mode (TRANSPORT/TUNNEL)
  * - for TUNNEL outer IP version (IPv4/IPv6)
+ * - NAT-T UDP encapsulated (TUNNEL mode only)
  * - are SA SQN operations 'atomic'
  * - ESN enabled/disabled
  * ...
@@ -86,7 +87,8 @@  enum {
 	RTE_SATP_LOG2_PROTO,
 	RTE_SATP_LOG2_DIR,
 	RTE_SATP_LOG2_MODE,
-	RTE_SATP_LOG2_SQN = RTE_SATP_LOG2_MODE + 2,
+	RTE_SATP_LOG2_NATT = RTE_SATP_LOG2_MODE + 2,
+	RTE_SATP_LOG2_SQN,
 	RTE_SATP_LOG2_ESN,
 	RTE_SATP_LOG2_ECN,
 	RTE_SATP_LOG2_DSCP
@@ -109,6 +111,10 @@  enum {
 #define RTE_IPSEC_SATP_MODE_TUNLV4	(1ULL << RTE_SATP_LOG2_MODE)
 #define RTE_IPSEC_SATP_MODE_TUNLV6	(2ULL << RTE_SATP_LOG2_MODE)
 
+#define RTE_IPSEC_SATP_NATT_MASK	(1ULL << RTE_SATP_LOG2_NATT)
+#define RTE_IPSEC_SATP_NATT_DISABLE	(0ULL << RTE_SATP_LOG2_NATT)
+#define RTE_IPSEC_SATP_NATT_ENABLE	(1ULL << RTE_SATP_LOG2_NATT)
+
 #define RTE_IPSEC_SATP_SQN_MASK		(1ULL << RTE_SATP_LOG2_SQN)
 #define RTE_IPSEC_SATP_SQN_RAW		(0ULL << RTE_SATP_LOG2_SQN)
 #define RTE_IPSEC_SATP_SQN_ATOM		(1ULL << RTE_SATP_LOG2_SQN)
diff --git a/lib/ipsec/sa.c b/lib/ipsec/sa.c
index 2ecbbce0a4..8e369e4618 100644
--- a/lib/ipsec/sa.c
+++ b/lib/ipsec/sa.c
@@ -217,6 +217,10 @@  fill_sa_type(const struct rte_ipsec_sa_prm *prm, uint64_t *type)
 	} else
 		return -EINVAL;
 
+	/* check for UDP encapsulation flag */
+	if (prm->ipsec_xform.options.udp_encap == 1)
+		tp |= RTE_IPSEC_SATP_NATT_ENABLE;
+
 	/* check for ESN flag */
 	if (prm->ipsec_xform.options.esn == 0)
 		tp |= RTE_IPSEC_SATP_ESN_DISABLE;
@@ -372,7 +376,8 @@  esp_sa_init(struct rte_ipsec_sa *sa, const struct rte_ipsec_sa_prm *prm,
 	const struct crypto_xform *cxf)
 {
 	static const uint64_t msk = RTE_IPSEC_SATP_DIR_MASK |
-				RTE_IPSEC_SATP_MODE_MASK;
+				RTE_IPSEC_SATP_MODE_MASK |
+				RTE_IPSEC_SATP_NATT_MASK;
 
 	if (prm->ipsec_xform.options.ecn)
 		sa->tos_mask |= RTE_IPV4_HDR_ECN_MASK;
@@ -475,10 +480,16 @@  esp_sa_init(struct rte_ipsec_sa *sa, const struct rte_ipsec_sa_prm *prm,
 	case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TRANS):
 		esp_inb_init(sa);
 		break;
+	case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV4 |
+			RTE_IPSEC_SATP_NATT_ENABLE):
+	case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV6 |
+			RTE_IPSEC_SATP_NATT_ENABLE):
 	case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV4):
 	case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV6):
 		esp_outb_tun_init(sa, prm);
 		break;
+	case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TRANS |
+			RTE_IPSEC_SATP_NATT_ENABLE):
 	case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TRANS):
 		esp_outb_init(sa, 0);
 		break;
diff --git a/lib/ipsec/sa.h b/lib/ipsec/sa.h
index 5e237f3525..3f38921eb3 100644
--- a/lib/ipsec/sa.h
+++ b/lib/ipsec/sa.h
@@ -101,6 +101,10 @@  struct rte_ipsec_sa {
 		uint64_t msk;
 		uint64_t val;
 	} tx_offload;
+	struct {
+		uint16_t sport;
+		uint16_t dport;
+	} natt;
 	uint32_t salt;
 	uint8_t algo_type;
 	uint8_t proto;    /* next proto */