[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230427171013.9911-1-kuniyu@amazon.com>
Date: Thu, 27 Apr 2023 10:10:13 -0700
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <xiyou.wangcong@...il.com>
CC: <cong.wang@...edance.com>, <edumazet@...gle.com>,
<kuniyu@...zon.com>, <netdev@...r.kernel.org>,
<oswalpalash@...il.com>
Subject: Re: [Patch net v2] sit: update dev->needed_headroom in ipip6_tunnel_bind_dev()
From: Cong Wang <xiyou.wangcong@...il.com>
Date: Wed, 26 Apr 2023 23:00:06 -0700
> From: Cong Wang <cong.wang@...edance.com>
>
> When a tunnel device is bound with the underlying device, its
> dev->needed_headroom needs to be updated properly. IPv4 tunnels
> already do the same in ip_tunnel_bind_dev(). Otherwise we may
> not have enough header room for skb, especially after commit
> b17f709a2401 ("gue: TX support for using remote checksum offload option").
>
> Fixes: 32b8a8e59c9c ("sit: add IPv4 over IPv4 support")
> Reported-by: Palash Oswal <oswalpalash@...il.com>
> Link: https://lore.kernel.org/netdev/CAGyP=7fDcSPKu6nttbGwt7RXzE3uyYxLjCSE97J64pRxJP8jPA@mail.gmail.com/
> Cc: Kuniyuki Iwashima <kuniyu@...zon.com>
> Cc: Eric Dumazet <edumazet@...gle.com>
> Signed-off-by: Cong Wang <cong.wang@...edance.com>
Reviewed-by: Kuniyuki Iwashima <kuniyu@...zon.com>
> ---
> v2: follow reverse Christmas tree style
nit: less changes needed if we put t_hlen init down.
---8<---
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 70d81bba5093..dce9efba24a9 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -1095,6 +1095,7 @@ static netdev_tx_t sit_tunnel_xmit(struct sk_buff *skb,
static void ipip6_tunnel_bind_dev(struct net_device *dev)
{
+ int t_hlen, hlen = LL_MAX_HEADER;
struct net_device *tdev = NULL;
struct ip_tunnel *tunnel;
const struct iphdr *iph;
@@ -1122,15 +1123,18 @@ static void ipip6_tunnel_bind_dev(struct net_device *dev)
if (!tdev && tunnel->parms.link)
tdev = __dev_get_by_index(tunnel->net, tunnel->parms.link);
+ t_hlen = tunnel->hlen + sizeof(struct iphdr);
+
if (tdev && !netif_is_l3_master(tdev)) {
- int t_hlen = tunnel->hlen + sizeof(struct iphdr);
int mtu;
mtu = tdev->mtu - t_hlen;
if (mtu < IPV6_MIN_MTU)
mtu = IPV6_MIN_MTU;
WRITE_ONCE(dev->mtu, mtu);
+ hlen = tdev->hard_header_len + tdev->needed_headroom;
}
+ dev->needed_headroom = hlen + t_hlen;
}
static void ipip6_tunnel_update(struct ip_tunnel *t, struct ip_tunnel_parm *p,
---8<---
Thanks,
Kuniyuki
>
> Note, this is targeting for -net and -table, so I'd keep the fix
> small. We can refactor and reuse ip_tunnel_bind_dev() for -net-next.
>
> net/ipv6/sit.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
> index 70d81bba5093..3ffb6a5b1f82 100644
> --- a/net/ipv6/sit.c
> +++ b/net/ipv6/sit.c
> @@ -1095,12 +1095,13 @@ static netdev_tx_t sit_tunnel_xmit(struct sk_buff *skb,
>
> static void ipip6_tunnel_bind_dev(struct net_device *dev)
> {
> + struct ip_tunnel *tunnel = netdev_priv(dev);
> + int t_hlen = tunnel->hlen + sizeof(struct iphdr);
> struct net_device *tdev = NULL;
> - struct ip_tunnel *tunnel;
> + int hlen = LL_MAX_HEADER;
> const struct iphdr *iph;
> struct flowi4 fl4;
>
> - tunnel = netdev_priv(dev);
> iph = &tunnel->parms.iph;
>
> if (iph->daddr) {
> @@ -1123,14 +1124,15 @@ static void ipip6_tunnel_bind_dev(struct net_device *dev)
> tdev = __dev_get_by_index(tunnel->net, tunnel->parms.link);
>
> if (tdev && !netif_is_l3_master(tdev)) {
> - int t_hlen = tunnel->hlen + sizeof(struct iphdr);
> int mtu;
>
> mtu = tdev->mtu - t_hlen;
> if (mtu < IPV6_MIN_MTU)
> mtu = IPV6_MIN_MTU;
> WRITE_ONCE(dev->mtu, mtu);
> + hlen = tdev->hard_header_len + tdev->needed_headroom;
> }
> + dev->needed_headroom = t_hlen + hlen;
> }
>
> static void ipip6_tunnel_update(struct ip_tunnel *t, struct ip_tunnel_parm *p,
> --
> 2.34.1
Powered by blists - more mailing lists