Thank you for your reply! > Some network devices that would not able to ping large packet under > bridge, but large packet ping is successful if not enable NF_CONNTRACK_BRIDGE.
> Can you add a new test to tools/testing/selftests/net/netfilter/ that > demonstrates this problem? Maybe I can't demonstrate this problem with a shell script, I actually discovered this problem while debugging a wifi network device. This netdevice is set a large needed_headroom(80), so ll_rs is oversize and goto blackhole. We can easily to reproduce it by configing needed_headroom in a netdevice, then add this netdevice to a bridge, and test bridge forwarding. ping large packet could reproduce this appearance.(successful if not enable NF_CONNTRACK_BRIDGE) > I guess this should be > > if (first_len - hlen > mtu) > goto blackhole; > if (skb_headroom(skb) < ll_rs) > goto expand_headroom; > ... but I'm not sure what the actual problem is. Yes, your guess is correct! Actual problem: I think it is unreasonable to directly drop skb with insufficient headroom. > Why does this need to make a full skb copy? > Should that be using skb_expand_head()? Using skb_expand_head has the same effect. > Actually, can't you just (re)use the slowpath for the skb_headroom < ll_rs > case instead of adding headroom expansion? I tested it just now, reuse the slowpath will successed. But maybe this change cannot resolve all cases if the netdevice really needs this headroom. Best Regards, Huajian -----邮件原件----- 发件人: Florian Westphal [mailto:f...@strlen.de] 发送时间: 2025年4月9日 17:18 收件人: Yang Huajian（杨华健） <huajiany...@asrmicro.com> 抄送: pa...@netfilter.org; kad...@netfilter.org; ra...@blackwall.org; ido...@nvidia.com; da...@davemloft.net; dsah...@kernel.org; eduma...@google.com; k...@kernel.org; pab...@redhat.com; ho...@kernel.org; netfilter-de...@vger.kernel.org; coret...@netfilter.org; bridge@lists.linux.dev; net...@vger.kernel.org; linux-ker...@vger.kernel.org 主题: Re: [PATCH] net: Expand headroom to send fragmented packets in bridge fragment forward Huajian Yang <huajiany...@asrmicro.com> wrote: > The config NF_CONNTRACK_BRIDGE will change the way fragments are processed. > Bridge does not know that it is a fragmented packet and forwards it > directly, after NF_CONNTRACK_BRIDGE is enabled, function > nf_br_ip_fragment will check and fraglist this packet. > > Some network devices that would not able to ping large packet under > bridge, but large packet ping is successful if not enable NF_CONNTRACK_BRIDGE. Can you add a new test to tools/testing/selftests/net/netfilter/ that demonstrates this problem? > In function nf_br_ip_fragment, checking the headroom before sending is > undoubted, but it is unreasonable to directly drop skb with > insufficient headroom. Are we talking about if (first_len - hlen > mtu or skb_headroom(skb) < ll_rs) ? > > if (first_len - hlen > mtu || > skb_headroom(skb) < ll_rs) > - goto blackhole; > + goto expand_headroom; I guess this should be if (first_len - hlen > mtu) goto blackhole; if (skb_headroom(skb) < ll_rs) goto expand_headroom; ... but I'm not sure what the actual problem is. > +expand_headroom: > + struct sk_buff *expand_skb; > + > + expand_skb = skb_copy_expand(skb, ll_rs, skb_tailroom(skb), GFP_ATOMIC); > + if (unlikely(!expand_skb)) > + goto blackhole; Why does this need to make a full skb copy? Should that be using skb_expand_head()? > slow_path: Actually, can't you just (re)use the slowpath for the skb_headroom < ll_rs case instead of adding headroom expansion?
