mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH bpf] lwt_bpf: account for aligned neigh header length
@ 2026-07-27 10:30 Junseo Lim
  2026-07-28  5:02 ` Emil Tsalapatis
  2026-08-05 20:14 ` Daniel Borkmann
  0 siblings, 2 replies; 3+ messages in thread
From: Junseo Lim @ 2026-07-27 10:30 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Simon Horman, Ido Schimmel, Martin KaFai Lau, Leon Hwang,
	Alexei Starovoitov, Guillaume Nault, Fernando Fernandez Mancera,
	bpf, netdev, linux-kernel, Thomas Graf, Sechang Lim, Junseo Lim

ip_finish_output2() expands an skb to LL_RESERVED_SPACE(dev) before LWT
xmit. An LWT_XMIT BPF program can then modify the skb head and still
return BPF_OK, so bpf_xmit() rechecks the remaining headroom before the
skb continues to neighbour output.

That recheck uses dst->dev->hard_header_len. This is not enough for the
neighbour cached-header path: neigh_hh_output() copies the cached hardware
header using the aligned hh_cache size, HH_DATA_MOD for short headers or
HH_DATA_ALIGN(hh_len) otherwise.

On Ethernet, hard_header_len is 14 but the cached copy needs 16 bytes. If
an LWT_XMIT BPF program calls bpf_skb_change_head(skb, 1, 0), the skb can
still have 15 bytes of headroom after the program. xmit_check_hhlen()
accepts that, after which neigh_hh_output() hits its headroom warning and
drops the skb.

Compare against the aligned hardware header length in xmit_check_hhlen()
so the BPF_OK path leaves enough headroom for neigh_hh_output()'s cached
header copy.

Fixes: 3a0af8fd61f9 ("bpf: BPF for lightweight tunnel infrastructure")
Signed-off-by: Junseo Lim <zirajs7@gmail.com>
---
Tested on a veth pair with an LWT_XMIT program calling
bpf_skb_change_head(skb, 1, 0). Before the patch, a UDP packet sent by
the reproducer triggers the neigh_hh_output() warning and is dropped.
With the patch, the packet is delivered without the warning.

Below is an excerpt of the warning before the patch:

    WARNING: ./include/net/neighbour.h:538 at ip_finish_output2+0x19f0/0x1f80, CPU#0: lwt_xmit_headro/55
    Call Trace:
     __ip_finish_output+0x59b/0x8b0
     ip_finish_output+0x67/0x390
     ip_output+0x1db/0x700
     ip_send_skb+0x1f6/0x270
     udp_send_skb+0x8d2/0xe00
     udp_sendmsg+0x1717/0x2620
     __sys_sendto+0x443/0x4e0

 net/core/lwt_bpf.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c
index bf588f508b79..2890aa59a3a0 100644
--- a/net/core/lwt_bpf.c
+++ b/net/core/lwt_bpf.c
@@ -169,8 +169,10 @@ static int bpf_output(struct net *net, struct sock *sk, struct sk_buff *skb)
 
 static int xmit_check_hhlen(struct sk_buff *skb, int hh_len)
 {
-	if (skb_headroom(skb) < hh_len) {
-		int nhead = HH_DATA_ALIGN(hh_len - skb_headroom(skb));
+	int hh_alen = HH_DATA_ALIGN(hh_len);
+
+	if (skb_headroom(skb) < hh_alen) {
+		int nhead = HH_DATA_ALIGN(hh_alen - skb_headroom(skb));
 
 		if (pskb_expand_head(skb, nhead, 0, GFP_ATOMIC))
 			return -ENOMEM;
-- 
2.55.0

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf] lwt_bpf: account for aligned neigh header length
  2026-07-27 10:30 [PATCH bpf] lwt_bpf: account for aligned neigh header length Junseo Lim
@ 2026-07-28  5:02 ` Emil Tsalapatis
  2026-08-05 20:14 ` Daniel Borkmann
  1 sibling, 0 replies; 3+ messages in thread
From: Emil Tsalapatis @ 2026-07-28  5:02 UTC (permalink / raw)
  To: Junseo Lim, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Simon Horman, Ido Schimmel, Martin KaFai Lau, Leon Hwang,
	Alexei Starovoitov, Guillaume Nault, Fernando Fernandez Mancera,
	bpf, netdev, linux-kernel, Thomas Graf, Sechang Lim

On Mon Jul 27, 2026 at 6:30 AM EDT, Junseo Lim wrote:
> ip_finish_output2() expands an skb to LL_RESERVED_SPACE(dev) before LWT
> xmit. An LWT_XMIT BPF program can then modify the skb head and still
> return BPF_OK, so bpf_xmit() rechecks the remaining headroom before the
> skb continues to neighbour output.
>
> That recheck uses dst->dev->hard_header_len. This is not enough for the
> neighbour cached-header path: neigh_hh_output() copies the cached hardware
> header using the aligned hh_cache size, HH_DATA_MOD for short headers or
> HH_DATA_ALIGN(hh_len) otherwise.
>
> On Ethernet, hard_header_len is 14 but the cached copy needs 16 bytes. If
> an LWT_XMIT BPF program calls bpf_skb_change_head(skb, 1, 0), the skb can
> still have 15 bytes of headroom after the program. xmit_check_hhlen()
> accepts that, after which neigh_hh_output() hits its headroom warning and
> drops the skb.
>
> Compare against the aligned hardware header length in xmit_check_hhlen()
> so the BPF_OK path leaves enough headroom for neigh_hh_output()'s cached
> header copy.
>
> Fixes: 3a0af8fd61f9 ("bpf: BPF for lightweight tunnel infrastructure")
> Signed-off-by: Junseo Lim <zirajs7@gmail.com>

Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>

> ---
> Tested on a veth pair with an LWT_XMIT program calling
> bpf_skb_change_head(skb, 1, 0). Before the patch, a UDP packet sent by
> the reproducer triggers the neigh_hh_output() warning and is dropped.
> With the patch, the packet is delivered without the warning.
>
> Below is an excerpt of the warning before the patch:
>
>     WARNING: ./include/net/neighbour.h:538 at ip_finish_output2+0x19f0/0x1f80, CPU#0: lwt_xmit_headro/55
>     Call Trace:
>      __ip_finish_output+0x59b/0x8b0
>      ip_finish_output+0x67/0x390
>      ip_output+0x1db/0x700
>      ip_send_skb+0x1f6/0x270
>      udp_send_skb+0x8d2/0xe00
>      udp_sendmsg+0x1717/0x2620
>      __sys_sendto+0x443/0x4e0
>
>  net/core/lwt_bpf.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c
> index bf588f508b79..2890aa59a3a0 100644
> --- a/net/core/lwt_bpf.c
> +++ b/net/core/lwt_bpf.c
> @@ -169,8 +169,10 @@ static int bpf_output(struct net *net, struct sock *sk, struct sk_buff *skb)
>  
>  static int xmit_check_hhlen(struct sk_buff *skb, int hh_len)
>  {
> -	if (skb_headroom(skb) < hh_len) {
> -		int nhead = HH_DATA_ALIGN(hh_len - skb_headroom(skb));
> +	int hh_alen = HH_DATA_ALIGN(hh_len);
> +
> +	if (skb_headroom(skb) < hh_alen) {
> +		int nhead = HH_DATA_ALIGN(hh_alen - skb_headroom(skb));
>  
>  		if (pskb_expand_head(skb, nhead, 0, GFP_ATOMIC))
>  			return -ENOMEM;


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf] lwt_bpf: account for aligned neigh header length
  2026-07-27 10:30 [PATCH bpf] lwt_bpf: account for aligned neigh header length Junseo Lim
  2026-07-28  5:02 ` Emil Tsalapatis
@ 2026-08-05 20:14 ` Daniel Borkmann
  1 sibling, 0 replies; 3+ messages in thread
From: Daniel Borkmann @ 2026-08-05 20:14 UTC (permalink / raw)
  To: Junseo Lim, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Simon Horman, Ido Schimmel, Martin KaFai Lau, Leon Hwang,
	Alexei Starovoitov, Guillaume Nault, Fernando Fernandez Mancera,
	bpf, netdev, linux-kernel, Thomas Graf, Sechang Lim

On 7/27/26 12:30 PM, Junseo Lim wrote:
> ip_finish_output2() expands an skb to LL_RESERVED_SPACE(dev) before LWT
> xmit. An LWT_XMIT BPF program can then modify the skb head and still
> return BPF_OK, so bpf_xmit() rechecks the remaining headroom before the
> skb continues to neighbour output.
> 
> That recheck uses dst->dev->hard_header_len. This is not enough for the
> neighbour cached-header path: neigh_hh_output() copies the cached hardware
> header using the aligned hh_cache size, HH_DATA_MOD for short headers or
> HH_DATA_ALIGN(hh_len) otherwise.
> 
> On Ethernet, hard_header_len is 14 but the cached copy needs 16 bytes. If
> an LWT_XMIT BPF program calls bpf_skb_change_head(skb, 1, 0), the skb can
> still have 15 bytes of headroom after the program. xmit_check_hhlen()
> accepts that, after which neigh_hh_output() hits its headroom warning and
> drops the skb.
> 
> Compare against the aligned hardware header length in xmit_check_hhlen()
> so the BPF_OK path leaves enough headroom for neigh_hh_output()'s cached
> header copy.
> 
> Fixes: 3a0af8fd61f9 ("bpf: BPF for lightweight tunnel infrastructure")
> Signed-off-by: Junseo Lim <zirajs7@gmail.com>
> ---
> Tested on a veth pair with an LWT_XMIT program calling
> bpf_skb_change_head(skb, 1, 0). Before the patch, a UDP packet sent by
> the reproducer triggers the neigh_hh_output() warning and is dropped.
> With the patch, the packet is delivered without the warning.
> 
> Below is an excerpt of the warning before the patch:
> 
>      WARNING: ./include/net/neighbour.h:538 at ip_finish_output2+0x19f0/0x1f80, CPU#0: lwt_xmit_headro/55
>      Call Trace:
>       __ip_finish_output+0x59b/0x8b0
>       ip_finish_output+0x67/0x390
>       ip_output+0x1db/0x700
>       ip_send_skb+0x1f6/0x270
>       udp_send_skb+0x8d2/0xe00
>       udp_sendmsg+0x1717/0x2620
>       __sys_sendto+0x443/0x4e0
> 
>   net/core/lwt_bpf.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c
> index bf588f508b79..2890aa59a3a0 100644
> --- a/net/core/lwt_bpf.c
> +++ b/net/core/lwt_bpf.c
> @@ -169,8 +169,10 @@ static int bpf_output(struct net *net, struct sock *sk, struct sk_buff *skb)
>   
>   static int xmit_check_hhlen(struct sk_buff *skb, int hh_len)
>   {
> -	if (skb_headroom(skb) < hh_len) {
> -		int nhead = HH_DATA_ALIGN(hh_len - skb_headroom(skb));
> +	int hh_alen = HH_DATA_ALIGN(hh_len);
> +
> +	if (skb_headroom(skb) < hh_alen) {
> +		int nhead = HH_DATA_ALIGN(hh_alen - skb_headroom(skb));
>   
>   		if (pskb_expand_head(skb, nhead, 0, GFP_ATOMIC))
>   			return -ENOMEM;

If we'd go with sth like the below, then it would also take the
dev->needed_headroom into account for the dst->dev :

diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c
index bf588f508b79..bfd06b5f73b3 100644
--- a/net/core/lwt_bpf.c
+++ b/net/core/lwt_bpf.c
@@ -167,10 +167,10 @@ static int bpf_output(struct net *net, struct sock *sk, struct sk_buff *skb)
  	return dst->lwtstate->orig_output(net, sk, skb);
  }
  
-static int xmit_check_hhlen(struct sk_buff *skb, int hh_len)
+static int xmit_check_headroom(struct sk_buff *skb, int hroom)
  {
-	if (skb_headroom(skb) < hh_len) {
-		int nhead = HH_DATA_ALIGN(hh_len - skb_headroom(skb));
+	if (skb_headroom(skb) < hroom) {
+		int nhead = hroom - skb_headroom(skb);
  
  		if (pskb_expand_head(skb, nhead, 0, GFP_ATOMIC))
  			return -ENOMEM;
@@ -280,7 +280,7 @@ static int bpf_xmit(struct sk_buff *skb)
  
  	bpf = bpf_lwt_lwtunnel(dst->lwtstate);
  	if (bpf->xmit.prog) {
-		int hh_len = dst->dev->hard_header_len;
+		int hroom = LL_RESERVED_SPACE(dst->dev);
  		__be16 proto = skb->protocol;
  		int ret;
  
@@ -296,9 +296,13 @@ static int bpf_xmit(struct sk_buff *skb)
  				return -EINVAL;
  			}
  			/* If the header was expanded, headroom might be too
-			 * small for L2 header to come, expand as needed.
+			 * small for the L2 header to come, expand as needed.
+			 * neigh_hh_output() copies the cached header in
+			 * HH_DATA_MOD aligned chunks,

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-05 20:14 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-27 10:30 [PATCH bpf] lwt_bpf: account for aligned neigh header length Junseo Lim
2026-07-28  5:02 ` Emil Tsalapatis
2026-08-05 20:14 ` Daniel Borkmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®