| Message ID | 20240924201401.2712-1-pablo@netfilter.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
On 9/24/24 22:13, Pablo Neira Ayuso wrote: > The following patchset contains Netfilter fixes for net: > > Patch #1 and #2 handle an esoteric scenario: Given two tasks sending UDP > packets to one another, two packets of the same flow in each direction > handled by different CPUs that result in two conntrack objects in NEW > state, where reply packet loses race. Then, patch #3 adds a testcase for > this scenario. Series from Florian Westphal. Kdoc complains against the lack of documentation for the return value in the first 2 patches: 'Returns' should be '@Return'. If you could repost a new revision soon, I could possibly still include it in today PR (delaying the latter a bit). Thanks! Paolo
Paolo Abeni <pabeni@redhat.com> wrote: > On 9/24/24 22:13, Pablo Neira Ayuso wrote: > > The following patchset contains Netfilter fixes for net: > > > > Patch #1 and #2 handle an esoteric scenario: Given two tasks sending UDP > > packets to one another, two packets of the same flow in each direction > > handled by different CPUs that result in two conntrack objects in NEW > > state, where reply packet loses race. Then, patch #3 adds a testcase for > > this scenario. Series from Florian Westphal. > > Kdoc complains against the lack of documentation for the return value in the > first 2 patches: 'Returns' should be '@Return'. :-( Apparently this is found via scripts/kernel-doc -Wall -none <file> I'll run this in the future, but, I have to say, its encouraging me to just not write such kdocs entries in first place, no risk of making a mistake. Paolo, Pablo, what should I do now?
On Thu, Sep 26, 2024 at 12:37:37PM +0200, Florian Westphal wrote: > Paolo Abeni <pabeni@redhat.com> wrote: > > On 9/24/24 22:13, Pablo Neira Ayuso wrote: > > > The following patchset contains Netfilter fixes for net: > > > > > > Patch #1 and #2 handle an esoteric scenario: Given two tasks sending UDP > > > packets to one another, two packets of the same flow in each direction > > > handled by different CPUs that result in two conntrack objects in NEW > > > state, where reply packet loses race. Then, patch #3 adds a testcase for > > > this scenario. Series from Florian Westphal. > > > > Kdoc complains against the lack of documentation for the return value in the > > first 2 patches: 'Returns' should be '@Return'. > > :-( > > Apparently this is found via > > scripts/kernel-doc -Wall -none <file> > > I'll run this in the future, but, I have to say, its encouraging me > to just not write such kdocs entries in first place, no risk of making > a mistake. > > Paolo, Pablo, what should I do now? I am going to fix it and resubmit PR.
Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > Paolo, Pablo, what should I do now? > > I am going to fix it and resubmit PR. Thanks, and sorry about this. I was not aware of how to format this "properly".
On 9/26/24 12:37, Florian Westphal wrote: > Paolo Abeni <pabeni@redhat.com> wrote: >> On 9/24/24 22:13, Pablo Neira Ayuso wrote: >>> The following patchset contains Netfilter fixes for net: >>> >>> Patch #1 and #2 handle an esoteric scenario: Given two tasks sending UDP >>> packets to one another, two packets of the same flow in each direction >>> handled by different CPUs that result in two conntrack objects in NEW >>> state, where reply packet loses race. Then, patch #3 adds a testcase for >>> this scenario. Series from Florian Westphal. >> >> Kdoc complains against the lack of documentation for the return value in the >> first 2 patches: 'Returns' should be '@Return'. > > :-( > > Apparently this is found via > > scripts/kernel-doc -Wall -none <file> > > I'll run this in the future, but, I have to say, its encouraging me > to just not write such kdocs entries in first place, no risk of making > a mistake. > > Paolo, Pablo, what should I do now? If an updated PR could be resent soon, say within ~1h, I can wait for the CI to run on it, merge and delay the net PR after that. Otherwise, if the fixes in here are urgent, I can pull the series as-is, and you could follow-up on nf-next/net-next. The last resort is just drop this from today's PR. Please LMK your preference, Paolo
On Thu, Sep 26, 2024 at 12:43:23PM +0200, Paolo Abeni wrote: > On 9/26/24 12:37, Florian Westphal wrote: > > Paolo Abeni <pabeni@redhat.com> wrote: > > > On 9/24/24 22:13, Pablo Neira Ayuso wrote: > > > > The following patchset contains Netfilter fixes for net: > > > > > > > > Patch #1 and #2 handle an esoteric scenario: Given two tasks sending UDP > > > > packets to one another, two packets of the same flow in each direction > > > > handled by different CPUs that result in two conntrack objects in NEW > > > > state, where reply packet loses race. Then, patch #3 adds a testcase for > > > > this scenario. Series from Florian Westphal. > > > > > > Kdoc complains against the lack of documentation for the return value in the > > > first 2 patches: 'Returns' should be '@Return'. > > > > :-( > > > > Apparently this is found via > > > > scripts/kernel-doc -Wall -none <file> > > > > I'll run this in the future, but, I have to say, its encouraging me > > to just not write such kdocs entries in first place, no risk of making > > a mistake. > > > > Paolo, Pablo, what should I do now? > > If an updated PR could be resent soon, say within ~1h, I can wait for the CI > to run on it, merge and delay the net PR after that. > > Otherwise, if the fixes in here are urgent, I can pull the series as-is, and > you could follow-up on nf-next/net-next. > > The last resort is just drop this from today's PR. > > Please LMK your preference, I am working on this now.
Hi, The following patchset contains Netfilter fixes for net: Patch #1 and #2 handle an esoteric scenario: Given two tasks sending UDP packets to one another, two packets of the same flow in each direction handled by different CPUs that result in two conntrack objects in NEW state, where reply packet loses race. Then, patch #3 adds a testcase for this scenario. Series from Florian Westphal. 1) NAT engine can falsely detect a port collision if it happens to pick up a reply packet as NEW rather than ESTABLISHED. Add extra code to detect this and suppress port reallocation in this case. 2) To complete the clash resolution in the reply direction, extend conntrack logic to detect clashing conntrack in the reply direction to existing entry. 3) Adds a test case. Then, an assorted list of fixes follow: 4) Add a selftest for tproxy, from Antonio Ojea. 5) Guard ctnetlink_*_size() functions under #if defined(CONFIG_NETFILTER_NETLINK_GLUE_CT) || defined(CONFIG_NF_CONNTRACK_EVENTS) From Andy Shevchenko. 6) Use -m socket --transparent in iptables tproxy documentation. From XIE Zhibang. 7) Call kfree_rcu() when releasing flowtable hooks to address race with netlink dump path, from Phil Sutter. 8) Fix compilation warning in nf_reject with CONFIG_BRIDGE_NETFILTER=n. From Simon Horman. 9) Guard ctnetlink_label_size() under CONFIG_NF_CONNTRACK_EVENTS which is its only user, to address a compilation warning. From Simon Horman. 10) Use rcu-protected list iteration over basechain hooks from netlink dump path. 11) Fix memcg for nf_tables, use GFP_KERNEL_ACCOUNT is not complete. 12) Remove old nfqueue conntrack clash resolution. Instead trying to use same destination address consistently which requires double DNAT, use the existing clash resolution which allows clashing packets go through with different destination. Antonio Ojea originally reported an issue from the postrouting chain, I proposed a fix: https://lore.kernel.org/netfilter-devel/ZuwSwAqKgCB2a51-@calendula/T/ which he reported it did not work for him. 13) Adds a selftest for patch 12. 14) Fixes ipvs.sh selftest. Please, pull these changes from: git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git nf-24-09-24 Thanks. ---------------------------------------------------------------- The following changes since commit 9410645520e9b820069761f3450ef6661418e279: Merge tag 'net-next-6.12' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next (2024-09-16 06:02:27 +0200) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git tags/nf-24-09-24 for you to fetch changes up to 69021d3bc01c72c3315ea541062351a623b72c8f: selftests: netfilter: Avoid hanging ipvs.sh (2024-09-19 14:54:10 +0200) ---------------------------------------------------------------- netfilter pull request 24-09-24 ---------------------------------------------------------------- Andy Shevchenko (1): netfilter: ctnetlink: Guard possible unused functions Antonio Ojea (1): selftests: netfilter: nft_tproxy.sh: add tcp tests Florian Westphal (5): netfilter: nf_nat: don't try nat source port reallocation for reverse dir clash netfilter: conntrack: add clash resolution for reverse collisions selftests: netfilter: add reverse-clash resolution test case netfilter: nfnetlink_queue: remove old clash resolution logic kselftest: add test for nfqueue induced conntrack race Pablo Neira Ayuso (2): netfilter: nf_tables: use rcu chain hook list iterator from netlink dump path netfilter: nf_tables: missing objects with no memcg accounting Phil Sutter (2): netfilter: nf_tables: Keep deleted flowtable hooks until after RCU selftests: netfilter: Avoid hanging ipvs.sh Simon Horman (2): netfilter: nf_reject: Fix build warning when CONFIG_BRIDGE_NETFILTER=n netfilter: ctnetlink: compile ctnetlink_label_size with CONFIG_NF_CONNTRACK_EVENTS 谢致邦 (XIE Zhibang) (1): docs: tproxy: ignore non-transparent sockets in iptables Documentation/networking/tproxy.rst | 2 +- include/linux/netfilter.h | 4 - net/ipv4/netfilter/nf_reject_ipv4.c | 10 +- net/ipv6/netfilter/nf_reject_ipv6.c | 5 +- net/netfilter/nf_conntrack_core.c | 141 +++----- net/netfilter/nf_conntrack_netlink.c | 9 +- net/netfilter/nf_nat_core.c | 121 ++++++- net/netfilter/nf_tables_api.c | 6 +- net/netfilter/nft_compat.c | 6 +- net/netfilter/nft_log.c | 2 +- net/netfilter/nft_meta.c | 2 +- net/netfilter/nft_numgen.c | 2 +- net/netfilter/nft_set_pipapo.c | 13 +- net/netfilter/nft_tunnel.c | 5 +- tools/testing/selftests/net/netfilter/Makefile | 4 + tools/testing/selftests/net/netfilter/config | 1 + .../net/netfilter/conntrack_reverse_clash.c | 125 +++++++ .../net/netfilter/conntrack_reverse_clash.sh | 51 +++ tools/testing/selftests/net/netfilter/ipvs.sh | 2 +- tools/testing/selftests/net/netfilter/nft_queue.sh | 92 +++++- .../selftests/net/netfilter/nft_tproxy_tcp.sh | 358 +++++++++++++++++++++ .../selftests/net/netfilter/nft_tproxy_udp.sh | 262 +++++++++++++++ 22 files changed, 1091 insertions(+), 132 deletions(-) create mode 100644 tools/testing/selftests/net/netfilter/conntrack_reverse_clash.c create mode 100755 tools/testing/selftests/net/netfilter/conntrack_reverse_clash.sh create mode 100755 tools/testing/selftests/net/netfilter/nft_tproxy_tcp.sh create mode 100755 tools/testing/selftests/net/netfilter/nft_tproxy_udp.sh