| Message ID | 20250410075726.8599-1-huajianyang@asrmicro.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: Move specific fragmented packet to slow_path instead of dropping it | expand |
Huajian Yang <huajianyang@asrmicro.com> wrote: > --- a/net/bridge/netfilter/nf_conntrack_bridge.c > +++ b/net/bridge/netfilter/nf_conntrack_bridge.c > @@ -61,18 +61,14 @@ static int nf_br_ip_fragment(struct net *net, struct sock *sk, > struct sk_buff *frag; > > if (first_len - hlen > mtu || > - skb_headroom(skb) < ll_rs) > - goto blackhole; I would prefer to keep blackhole logic for the mtu tests, i.e. if (first_len - hlen > mtu) goto blackhole; same for the frag->len test in the skb_walk_frags loop. From what I understood the problem is only because of the lower devices' headroom requirement.
Thank you for your reply! In an earlier email I wrote: > 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. If the ping test successed without NF_CONNTRACK_BRIDGE, it is because the netdev doesn't need such a large headroom in actual network forwarding. If the netdev realy need it, the original bridge forwarding will fail too. Maybe we need reconfig our wifi netdev or something else. So is the nf_br_ip_fragment done to be consistent with the original bridge forwarding? There are two very different ideas here: One is to try to maintain the same treatment as the original bridge, as it is currently. The other is to try to ensure that the packet is forwarded. > I would prefer to keep blackhole logic for the mtu tests, i.e. > if (first_len - hlen > mtu) > goto blackhole; Anyway, this modification is more appropriate. Because I have tested by change mtu just now, goto slowpath cannot forward it either. Best Regards, Huajian -----邮件原件----- 发件人: Florian Westphal [mailto:fw@strlen.de] 发送时间: 2025年4月10日 18:18 收件人: Yang Huajian（杨华健） <huajianyang@asrmicro.com> 抄送: pablo@netfilter.org; fw@strlen.de; kadlec@netfilter.org; razor@blackwall.org; idosch@nvidia.com; davem@davemloft.net; dsahern@kernel.org; edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; horms@kernel.org; netfilter-devel@vger.kernel.org; coreteam@netfilter.org; bridge@lists.linux.dev; netdev@vger.kernel.org; linux-kernel@vger.kernel.org 主题: Re: [PATCH] net: Move specific fragmented packet to slow_path instead of dropping it Huajian Yang <huajianyang@asrmicro.com> wrote: > --- a/net/bridge/netfilter/nf_conntrack_bridge.c > +++ b/net/bridge/netfilter/nf_conntrack_bridge.c > @@ -61,18 +61,14 @@ static int nf_br_ip_fragment(struct net *net, struct sock *sk, > struct sk_buff *frag; > > if (first_len - hlen > mtu || > - skb_headroom(skb) < ll_rs) > - goto blackhole; I would prefer to keep blackhole logic for the mtu tests, i.e. if (first_len - hlen > mtu) goto blackhole; same for the frag->len test in the skb_walk_frags loop. From what I understood the problem is only because of the lower devices' headroom requirement.
diff --git a/net/bridge/netfilter/nf_conntrack_bridge.c b/net/bridge/netfilter/nf_conntrack_bridge.c index 816bb0fde718..beac62c5d257 100644 --- a/net/bridge/netfilter/nf_conntrack_bridge.c +++ b/net/bridge/netfilter/nf_conntrack_bridge.c @@ -61,18 +61,14 @@ static int nf_br_ip_fragment(struct net *net, struct sock *sk, struct sk_buff *frag; if (first_len - hlen > mtu || - skb_headroom(skb) < ll_rs) - goto blackhole; - - if (skb_cloned(skb)) + (skb_headroom(skb) < ll_rs) || + skb_cloned(skb)) goto slow_path; skb_walk_frags(skb, frag) { if (frag->len > mtu || - skb_headroom(frag) < hlen + ll_rs) - goto blackhole; - - if (skb_shared(frag)) + (skb_headroom(frag) < hlen + ll_rs) || + skb_shared(frag)) goto slow_path; } diff --git a/net/ipv6/netfilter.c b/net/ipv6/netfilter.c index 581ce055bf52..29778e014560 100644 --- a/net/ipv6/netfilter.c +++ b/net/ipv6/netfilter.c @@ -165,19 +165,14 @@ int br_ip6_fragment(struct net *net, struct sock *sk, struct sk_buff *skb, struct sk_buff *frag2; if (first_len - hlen > mtu || - skb_headroom(skb) < (hroom + sizeof(struct frag_hdr))) - goto blackhole; - - if (skb_cloned(skb)) + skb_headroom(skb) < (hroom + sizeof(struct frag_hdr)) || + skb_cloned(skb)) goto slow_path; skb_walk_frags(skb, frag2) { if (frag2->len > mtu || - skb_headroom(frag2) < (hlen + hroom + sizeof(struct frag_hdr))) - goto blackhole; - - /* Partially cloned skb? */ - if (skb_shared(frag2)) + skb_headroom(frag2) < (hlen + hroom + sizeof(struct frag_hdr)) || + skb_shared(frag2)) goto slow_path; }
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 and br_ip6_fragment will check and fraglist this packet. This change makes layer 2 fragmented packet forwarding more similar to ip_do_fragment, these specific packets previously dropped will go to slow_path for further processing. Signed-off-by: Huajian Yang <huajianyang@asrmicro.com> --- net/bridge/netfilter/nf_conntrack_bridge.c | 12 ++++-------- net/ipv6/netfilter.c | 13 ++++--------- 2 files changed, 8 insertions(+), 17 deletions(-)