Re: [PATCH bpf] lwt_bpf: account for aligned neigh header length

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 <[email protected]>
> ---
> 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,