mbox series

[net,v3,0/2] Fix PMTU for ESP-in-UDP encapsulation

Message ID 20210720203529.21601-1-vfedorenko@novek.ru
Headers show
Series Fix PMTU for ESP-in-UDP encapsulation | expand

Message

Vadim Fedorenko July 20, 2021, 8:35 p.m. UTC
Bug 213669 uncovered regression in PMTU discovery for UDP-encapsulated
routes and some incorrect usage in udp tunnel fields. This series fixes
problems and also adds such case for selftests

v3:
 - update checking logic to account SCTP use case
v2:
 - remove refactor code that was in first patch
 - move checking logic to __udp{4,6}_lib_err_encap
 - add more tests, especially routed configuration

Vadim Fedorenko (2):
  udp: check encap socket in __udp_lib_err
  selftests: net: add ESP-in-UDP PMTU test

 net/ipv4/udp.c                        |  25 ++-
 net/ipv6/udp.c                        |  25 ++-
 tools/testing/selftests/net/nettest.c |  55 ++++++-
 tools/testing/selftests/net/pmtu.sh   | 212 +++++++++++++++++++++++++-
 4 files changed, 298 insertions(+), 19 deletions(-)

Comments

Xin Long July 21, 2021, 2:08 p.m. UTC | #1
On Tue, Jul 20, 2021 at 4:35 PM Vadim Fedorenko <vfedorenko@novek.ru> wrote:
>

> Commit d26796ae5894 ("udp: check udp sock encap_type in __udp_lib_err")

> added checks for encapsulated sockets but it broke cases when there is

> no implementation of encap_err_lookup for encapsulation, i.e. ESP in

> UDP encapsulation. Fix it by calling encap_err_lookup only if socket

> implements this method otherwise treat it as legal socket.

>

> Fixes: d26796ae5894 ("udp: check udp sock encap_type in __udp_lib_err")

> Signed-off-by: Vadim Fedorenko <vfedorenko@novek.ru>

> ---

>  net/ipv4/udp.c | 25 +++++++++++++++++++------

>  net/ipv6/udp.c | 25 +++++++++++++++++++------

>  2 files changed, 38 insertions(+), 12 deletions(-)

>

> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c

> index 62cd4cd52e84..1a742b710e54 100644

> --- a/net/ipv4/udp.c

> +++ b/net/ipv4/udp.c

> @@ -645,10 +645,12 @@ static struct sock *__udp4_lib_err_encap(struct net *net,

>                                          const struct iphdr *iph,

>                                          struct udphdr *uh,

>                                          struct udp_table *udptable,

> +                                        struct sock *sk,

>                                          struct sk_buff *skb, u32 info)

>  {

> +       int (*lookup)(struct sock *sk, struct sk_buff *skb);

>         int network_offset, transport_offset;

> -       struct sock *sk;

> +       struct udp_sock *up;

>

>         network_offset = skb_network_offset(skb);

>         transport_offset = skb_transport_offset(skb);

> @@ -659,18 +661,28 @@ static struct sock *__udp4_lib_err_encap(struct net *net,

>         /* Transport header needs to point to the UDP header */

>         skb_set_transport_header(skb, iph->ihl << 2);

>

> +       if (sk) {

> +               up = udp_sk(sk);

> +

> +               lookup = READ_ONCE(up->encap_err_lookup);

> +               if (lookup && lookup(sk, skb))

> +                       sk = NULL;

> +

> +               goto out;

> +       }

> +

>         sk = __udp4_lib_lookup(net, iph->daddr, uh->source,

>                                iph->saddr, uh->dest, skb->dev->ifindex, 0,

>                                udptable, NULL);

>         if (sk) {

> -               int (*lookup)(struct sock *sk, struct sk_buff *skb);

> -               struct udp_sock *up = udp_sk(sk);

> +               up = udp_sk(sk);

>

>                 lookup = READ_ONCE(up->encap_err_lookup);

>                 if (!lookup || lookup(sk, skb))

>                         sk = NULL;

>         }

>

> +out:

>         if (!sk)

>                 sk = ERR_PTR(__udp4_lib_err_encap_no_sk(skb, info));

>

> @@ -707,15 +719,16 @@ int __udp4_lib_err(struct sk_buff *skb, u32 info, struct udp_table *udptable)

>         sk = __udp4_lib_lookup(net, iph->daddr, uh->dest,

>                                iph->saddr, uh->source, skb->dev->ifindex,

>                                inet_sdif(skb), udptable, NULL);

> +

>         if (!sk || udp_sk(sk)->encap_type) {

>                 /* No socket for error: try tunnels before discarding */

> -               sk = ERR_PTR(-ENOENT);

>                 if (static_branch_unlikely(&udp_encap_needed_key)) {

> -                       sk = __udp4_lib_err_encap(net, iph, uh, udptable, skb,

> +                       sk = __udp4_lib_err_encap(net, iph, uh, udptable, sk, skb,

>                                                   info);

>                         if (!sk)

>                                 return 0;

> -               }

> +               } else

> +                       sk = ERR_PTR(-ENOENT);

>

>                 if (IS_ERR(sk)) {

>                         __ICMP_INC_STATS(net, ICMP_MIB_INERRORS);

> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c

> index 0cc7ba531b34..c5e15e94bb00 100644

> --- a/net/ipv6/udp.c

> +++ b/net/ipv6/udp.c

> @@ -502,12 +502,14 @@ static struct sock *__udp6_lib_err_encap(struct net *net,

>                                          const struct ipv6hdr *hdr, int offset,

>                                          struct udphdr *uh,

>                                          struct udp_table *udptable,

> +                                        struct sock *sk,

>                                          struct sk_buff *skb,

>                                          struct inet6_skb_parm *opt,

>                                          u8 type, u8 code, __be32 info)

>  {

> +       int (*lookup)(struct sock *sk, struct sk_buff *skb);

>         int network_offset, transport_offset;

> -       struct sock *sk;

> +       struct udp_sock *up;

>

>         network_offset = skb_network_offset(skb);

>         transport_offset = skb_transport_offset(skb);

> @@ -518,18 +520,28 @@ static struct sock *__udp6_lib_err_encap(struct net *net,

>         /* Transport header needs to point to the UDP header */

>         skb_set_transport_header(skb, offset);

>

> +       if (sk) {

> +               up = udp_sk(sk);

> +

> +               lookup = READ_ONCE(up->encap_err_lookup);

> +               if (lookup && lookup(sk, skb))

> +                       sk = NULL;

> +

> +               goto out;

> +       }

> +

>         sk = __udp6_lib_lookup(net, &hdr->daddr, uh->source,

>                                &hdr->saddr, uh->dest,

>                                inet6_iif(skb), 0, udptable, skb);

>         if (sk) {

> -               int (*lookup)(struct sock *sk, struct sk_buff *skb);

> -               struct udp_sock *up = udp_sk(sk);

> +               up = udp_sk(sk);

>

>                 lookup = READ_ONCE(up->encap_err_lookup);

>                 if (!lookup || lookup(sk, skb))

>                         sk = NULL;

>         }

>

> +out:

>         if (!sk) {

>                 sk = ERR_PTR(__udp6_lib_err_encap_no_sk(skb, opt, type, code,

>                                                         offset, info));

> @@ -558,16 +570,17 @@ int __udp6_lib_err(struct sk_buff *skb, struct inet6_skb_parm *opt,

>

>         sk = __udp6_lib_lookup(net, daddr, uh->dest, saddr, uh->source,

>                                inet6_iif(skb), inet6_sdif(skb), udptable, NULL);

> +

>         if (!sk || udp_sk(sk)->encap_type) {

>                 /* No socket for error: try tunnels before discarding */

> -               sk = ERR_PTR(-ENOENT);

>                 if (static_branch_unlikely(&udpv6_encap_needed_key)) {

>                         sk = __udp6_lib_err_encap(net, hdr, offset, uh,

> -                                                 udptable, skb,

> +                                                 udptable, sk, skb,

>                                                   opt, type, code, info);

>                         if (!sk)

>                                 return 0;

> -               }

> +               } else

> +                       sk = ERR_PTR(-ENOENT);

>

>                 if (IS_ERR(sk)) {

>                         __ICMP6_INC_STATS(net, __in6_dev_get(skb->dev),

> --

> 2.18.4

>

Reviewed-by: Xin Long <lucien.xin@gmail.com>
patchwork-bot+netdevbpf@kernel.org July 21, 2021, 4 p.m. UTC | #2
Hello:

This series was applied to netdev/net.git (refs/heads/master):

On Tue, 20 Jul 2021 23:35:27 +0300 you wrote:
> Bug 213669 uncovered regression in PMTU discovery for UDP-encapsulated

> routes and some incorrect usage in udp tunnel fields. This series fixes

> problems and also adds such case for selftests

> 

> v3:

>  - update checking logic to account SCTP use case

> v2:

>  - remove refactor code that was in first patch

>  - move checking logic to __udp{4,6}_lib_err_encap

>  - add more tests, especially routed configuration

> 

> [...]


Here is the summary with links:
  - [net,v3,1/2] udp: check encap socket in __udp_lib_err
    https://git.kernel.org/netdev/net/c/9bfce73c8921
  - [net,v3,2/2] selftests: net: add ESP-in-UDP PMTU test
    https://git.kernel.org/netdev/net/c/ece1278a9b81

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html