addrconf_dad_work() is per-address work and holds RTNL internally.
We can fetch netns as dev_net(ifp->idev->dev).
Let's use rtnl_net_lock().
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Link: https://patch.msgid.link/20250115080608.28127-5-kuniyu@amazon.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
addrconf_verify_work() is per-netns work to call addrconf_verify_rtnl()
under RTNL.
Let's use rtnl_net_lock().
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Link: https://patch.msgid.link/20250115080608.28127-4-kuniyu@amazon.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
We will convert rtnl_lock() with rtnl_net_lock(), and we want to
convert __in6_dev_get() too.
__in6_dev_get() uses rcu_dereference_rtnl(), but as written in its
comment, rtnl_dereference() or rcu_dereference() is preferable.
Let's add __in6_dev_get_rtnl_net() that uses rtnl_net_dereference().
We can add the RCU version helper later if needed.
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Link: https://patch.msgid.link/20250115080608.28127-2-kuniyu@amazon.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
After commit df542f6693 ("net: stmmac: Switch to zero-copy in
non-XDP RX path"), SKBs are always marked for recycle, it is redundant
to mark SKBs more than once when new frags are appended.
Signed-off-by: Furong Xu <0x1207@gmail.com>
Link: https://patch.msgid.link/20250117062805.192393-1-0x1207@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Two different models of usb card, the drivers are r8152 and asix. If no
network cable is connected, Speed = 10Mb/s. This problem is repeated in
linux 3.10, 4.19, 5.4, 6.12. This problem also exists on the latest
kernel. Both drivers call mii_ethtool_get_link_ksettings,
but the value of cmd->base.speed in this
function can only be SPEED_1000 or SPEED_100 or SPEED_10.
When the network cable is not connected, set cmd->base.speed
=SPEED_UNKNOWN.
Signed-off-by: Xiangqian Zhang <zhangxiangqian@kylinos.cn>
Link: https://patch.msgid.link/20250117094603.4192594-1-zhangxiangqian@kylinos.cn
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Since dccp and llc makefiles already check sysctl code
compilation with xxx-$(CONFIG_SYSCTL)
we can drop the checks
Signed-off-by: Denis Kirjanov <kirjanov@gmail.com>
Link: https://patch.msgid.link/20250119134254.19250-1-kirjanov@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
-----BEGIN PGP SIGNATURE-----
iQIzBAABCgAdFiEEjF9xRqF1emXiQiqU1w0aZmrPKyEFAmeNMZwACgkQ1w0aZmrP
KyFK5A/9EVenL7bwgh9L/F3T1Mo94sM4Ych80Rf3XXHN1t7aBx4zOnIJRUDAlNCB
+btuE+NYeJ/tpt01Qv9uSnI35W2eTf/EyEtAFfC9RZIVt0ESbtb2Y4rBv3LID/pN
dtMPDoFJoLK64bViIdL9zoHsNxFRVgP+sC99d91sgr+LjiZ2JDqWKAog51tpWp8k
GyYt4Kc/lPyo4QsNhMY6i9J12HvQtzhlwqWJ5k9ftrd5Sh+tGDYLzbmucTw8HmWq
YFxYEM+Ri4KeymuuGzySKOWung2c0e5Qd1ugCxIjXnw71veQ60cITKzvm0WOhIB5
nsF0gwgrpKNd5a/PV+00CyDfOXDrROQCsb5STqDZNxnsiC5FGlpDXV9RoH/e4H/N
e/G8//SSy/+GxWIF3HguWhggxTUN6fvC/UmMD4OHyLRkhfbSOm8KEBaTtzriNI/w
AeHrW0JO1X13/QkWcXAD4xp59mQV3AZ2UuqLJS6AiggkGZ2/LjUKFZaga2F317Bu
JqhzpUyMMbkVx0qN5cGuBm4g0HiUlpYa4j8CcA+9Us++McMJQ38HEUVrQgonKSP/
gNYRuT3LMVIjYyBUYy4vRLbxNfmfY4ZIlA9b0My4wo4WIGmqn7US/sxj6cIclKgL
OAUu/UC6+jVTmLTYi3r146SOeyCt2QGD+KWc6x+FoghgukRe9Gg=
=AASc
-----END PGP SIGNATURE-----
Merge tag 'nf-next-25-01-19' of git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf-next
Pablo Neira Ayuso says:
====================
Netfilter updates for net-next
The following batch contains Netfilter updates for net-next:
1) Unbreak set size settings for rbtree set backend, intervals in
rbtree are represented as two elements, this detailed is leaked
to userspace leading to bogus ENOSPC from control plane.
2) Remove dead code in br_netfilter's br_nf_pre_routing_finish()
due to never matching error when looking up for route,
from Antoine Tenart.
3) Simplify check for device already in use in flowtable,
from Phil Sutter.
4) Three patches to restore interface name field in struct nft_hook
and use it, this is to prepare for wildcard interface support.
From Phil Sutter.
5) Do not remove netdev basechain when last device is gone, this is
for consistency with the flowtable behaviour. This allows for netdev
basechains without devices. Another patch to simplify netdev event
notifier after this update. Also from Phil.
6) Two patches to add missing spinlock when flowtable updates TCP
state flags, from Florian Westphal.
7) Simplify __nf_ct_refresh_acct() by removing skbuff parameter,
also from Florian.
8) Flowtable gc now extends ct timeout for offloaded flow. This
is to address a possible race that leads to handing over flow
to classic path with long ct timeouts.
9) Tear down flow if cached rt_mtu is stale, before this patch,
packet is handed over to classic path but flow entry still remained
in place.
10) Revisit the flowtable teardown strategy, which was originally
designed to release flowtable hardware entries early. Add a new
CLOSING flag that still allows hardware to release entries when
fin/rst is seen, but keeps the flow entry in place when the
TCP connection is closed. Release flow after timeout or when a new
syn packet is seen for TCP reopen scenario.
* tag 'nf-next-25-01-19' of git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf-next:
netfilter: flowtable: add CLOSING state
netfilter: flowtable: teardown flow if cached mtu is stale
netfilter: conntrack: rework offload nf_conn timeout extension logic
netfilter: conntrack: remove skb argument from nf_ct_refresh
netfilter: nft_flow_offload: update tcp state flags under lock
netfilter: nft_flow_offload: clear tcp MAXACK flag before moving to slowpath
netfilter: nf_tables: Simplify chain netdev notifier
netfilter: nf_tables: Tolerate chains with no remaining hooks
netfilter: nf_tables: Compare netdev hooks based on stored name
netfilter: nf_tables: Use stored ifname in netdev hook dumps
netfilter: nf_tables: Store user-defined hook ifname
netfilter: nf_tables: Flowtable hook's pf value never varies
netfilter: br_netfilter: remove unused conditional and dead code
netfilter: nf_tables: fix set size with rbtree backend
====================
Link: https://patch.msgid.link/20250119172051.8261-1-pablo@netfilter.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Jakub Kicinski says:
====================
net: ethtool: fixes for HDS threshold
Quick follow up on the HDS threshold work, since the merge window
is upon us.
Fix the bnxt implementation to apply the settings right away,
because we update the parameters _after_ configuring HW user
needed to reconfig the device twice to get the settings to stick.
For this I took the liberty of moving the config to a separate
struct. This follows my original thinking for the queue API.
It should also fit more neatly into how many drivers which
support safe config update operate. Drivers can allocate
new objects using the "pending" struct.
netdevsim:
KTAP version 1
1..7
ok 1 hds.get_hds
ok 2 hds.get_hds_thresh
ok 3 hds.set_hds_disable
ok 4 hds.set_hds_enable
ok 5 hds.set_hds_thresh_zero
ok 6 hds.set_hds_thresh_max
ok 7 hds.set_hds_thresh_gt
# Totals: pass:7 fail:0 xfail:0 xpass:0 skip:0 error:0
bnxt:
KTAP version 1
1..7
ok 1 hds.get_hds
ok 2 hds.get_hds_thresh
ok 3 hds.set_hds_disable # SKIP disabling of HDS not supported by the device
ok 4 hds.set_hds_enable
ok 5 hds.set_hds_thresh_zero
ok 6 hds.set_hds_thresh_max
ok 7 hds.set_hds_thresh_gt
# Totals: pass:6 fail:0 xfail:0 xpass:0 skip:1 error:0
v1: https://lore.kernel.org/20250117194815.1514410-1-kuba@kernel.org
====================
Link: https://patch.msgid.link/20250119020518.1962249-1-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
300-400B RPC requests are fairly common. With the current default
of 256B HDS threshold bnxt ends up splitting those, lowering PCIe
bandwidth efficiency and increasing the number of memory allocation.
Increase the HDS threshold to fit 4 buffers in a 4k page.
This works out to 640B as the threshold on a typical kernel confing.
This change increases the performance for a microbenchmark which
receives 400B RPCs and sends empty responses by 4.5%.
Admittedly this is just a single benchmark, but 256B works out to
just 6 (so 2 more) packets per head page, because shinfo size
dominates the headers.
Now that we use page pool for the header pages I was also tempted
to default rx_copybreak to 0, but in synthetic testing the copybreak
size doesn't seem to make much difference.
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
Link: https://patch.msgid.link/20250119020518.1962249-8-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Now that we can configure HDS threshold separately from the rx_copybreak
HDS threshold may be higher than rx_copybreak.
We need to make sure that we have enough space for the headers.
Fixes: 6b43673a25 ("bnxt_en: add support for hds-thresh ethtool command")
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
Link: https://patch.msgid.link/20250119020518.1962249-7-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The core has the current HDS config, it can pre-populate the values
for the drivers. While at it, remove the zero-setting in netdevsim.
Zero are the default values since the config is zalloc'ed.
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
Link: https://patch.msgid.link/20250119020518.1962249-6-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Use the pending config for hds_thrs. Core will only update the "current"
one after we return success. Without this change 2 reconfigs would be
required for the setting to reach the device.
Fixes: 6b43673a25 ("bnxt_en: add support for hds-thresh ethtool command")
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
Link: https://patch.msgid.link/20250119020518.1962249-5-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Record the pending configuration in net_device struct.
ethtool core duplicates the current config and the specific
handlers (for now just ringparam) can modify it.
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
Link: https://patch.msgid.link/20250119020518.1962249-4-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
For ease of review of the next patch store the dev pointer
on the stack, instead of referring to req_info.dev every time.
No functional changes.
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
Link: https://patch.msgid.link/20250119020518.1962249-3-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Separate the HDS config from the ethtool state struct.
The HDS config contains just simple parameters, not state.
Having it as a separate struct will make it easier to clone / copy
and also long term potentially make it per-queue.
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
Link: https://patch.msgid.link/20250119020518.1962249-2-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This is based on Donald Hunter's patch.
These functions could fail for various reasons, sometimes
triggering kfree_skb().
* unix_stream_connect() : connect()
* unix_stream_sendmsg() : sendmsg()
* queue_oob() : sendmsg(MSG_OOB)
* unix_dgram_sendmsg() : sendmsg()
Such kfree_skb() is tied to the errno of connect() and
sendmsg(), and we need not define skb drop reasons.
Let's use consume_skb() not to churn kfree_skb() events.
Link: https://lore.kernel.org/netdev/eb30b164-7f86-46bf-a5d3-0f8bda5e9398@redhat.com/
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Link: https://patch.msgid.link/20250116053441.5758-10-kuniyu@amazon.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This is a follow-up of commit d460b04bc4 ("af_unix: Clean up
error paths in unix_stream_sendmsg().").
If we initialise skb with NULL in unix_stream_sendmsg(), we can
reuse the existing out_pipe label for the SEND_SHUTDOWN check.
Let's rename it and adjust the existing label as out_pipe_lock.
While at it, size and data_len are moved to the while loop scope.
Suggested-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Link: https://patch.msgid.link/20250116053441.5758-9-kuniyu@amazon.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
unix_dgram_disconnected() is called from two places:
1. when a connect()ed socket dis-connect()s or re-connect()s to
another socket
2. when sendmsg() fails because the peer socket that the client
has connect()ed to has been close()d
Then, the client's recv queue is purged to remove all messages from
the old peer socket.
Let's define a new drop reason for that case.
# echo 1 > /sys/kernel/tracing/events/skb/kfree_skb/enable
# python3
>>> from socket import *
>>>
>>> # s1 has a message from s2
>>> s1, s2 = socketpair(AF_UNIX, SOCK_DGRAM)
>>> s2.send(b'hello world')
>>>
>>> # re-connect() drops the message from s2
>>> s3 = socket(AF_UNIX, SOCK_DGRAM)
>>> s3.bind('')
>>> s1.connect(s3.getsockname())
# cat /sys/kernel/tracing/trace_pipe
python3-250 ... kfree_skb: ... location=skb_queue_purge_reason+0xdc/0x110 reason: UNIX_DISCONNECT
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Link: https://patch.msgid.link/20250116053441.5758-8-kuniyu@amazon.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
unix_stream_read_skb() is called when BPF SOCKMAP reads some data
from a socket in the map.
SOCKMAP does not support MSG_OOB, and reading OOB results in a drop.
Let's set drop reasons respectively.
* SOCKET_CLOSE : the socket in SOCKMAP was close()d
* UNIX_SKIP_OOB : OOB was read from the socket in SOCKMAP
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Link: https://patch.msgid.link/20250116053441.5758-7-kuniyu@amazon.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
AF_UNIX SOCK_STREAM socket supports MSG_OOB.
When OOB data is sent to a socket, recv() will break at that point.
If the next recv() does not have MSG_OOB, the normal data following
the OOB data is returned.
Then, the OOB skb is dropped.
Let's define a new drop reason for that case in manage_oob().
# echo 1 > /sys/kernel/tracing/events/skb/kfree_skb/enable
# python3
>>> from socket import *
>>> s1, s2 = socketpair(AF_UNIX)
>>> s1.send(b'a', MSG_OOB)
>>> s1.send(b'b')
>>> s2.recv(2)
b'b'
# cat /sys/kernel/tracing/trace_pipe
...
python3-223 ... kfree_skb: ... location=unix_stream_read_generic+0x59e/0xc20 reason: UNIX_SKIP_OOB
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Link: https://patch.msgid.link/20250116053441.5758-6-kuniyu@amazon.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Inflight file descriptors by SCM_RIGHTS hold references to the
struct file.
AF_UNIX sockets could hold references to each other, forming
reference cycles.
Once such sockets are close()d without the fd recv()ed, they
will be unaccessible from userspace but remain in kernel.
__unix_gc() garbage-collects skb with the dead file descriptors
and frees them by __skb_queue_purge().
Let's set SKB_DROP_REASON_SOCKET_CLOSE there.
# echo 1 > /sys/kernel/tracing/events/skb/kfree_skb/enable
# python3
>>> from socket import *
>>> from array import array
>>>
>>> # Create a reference cycle
>>> s1 = socket(AF_UNIX, SOCK_DGRAM)
>>> s1.bind('')
>>> s1.sendmsg([b"nop"], [(SOL_SOCKET, SCM_RIGHTS, array("i", [s1.fileno()]))], 0, s1.getsockname())
>>> s1.close()
>>>
>>> # Trigger GC
>>> s2 = socket(AF_UNIX)
>>> s2.close()
# cat /sys/kernel/tracing/trace_pipe
...
kworker/u16:2-42 ... kfree_skb: ... location=__unix_gc+0x4ad/0x580 reason: SOCKET_CLOSE
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Link: https://patch.msgid.link/20250116053441.5758-5-kuniyu@amazon.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
unix_sock_destructor() is called as sk->sk_destruct() just before
the socket is actually freed.
Let's use SKB_DROP_REASON_SOCKET_CLOSE for skb_queue_purge().
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Link: https://patch.msgid.link/20250116053441.5758-4-kuniyu@amazon.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
unix_release_sock() is called when the last refcnt of struct file
is released.
Let's define a new drop reason SKB_DROP_REASON_SOCKET_CLOSE and
set it for kfree_skb() in unix_release_sock().
# echo 1 > /sys/kernel/tracing/events/skb/kfree_skb/enable
# python3
>>> from socket import *
>>> s1, s2 = socketpair(AF_UNIX)
>>> s1.send(b'hello world')
>>> s2.close()
# cat /sys/kernel/tracing/trace_pipe
...
python3-280 ... kfree_skb: ... protocol=0 location=unix_release_sock+0x260/0x420 reason: SOCKET_CLOSE
To be precise, unix_release_sock() is also called for a new child
socket in unix_stream_connect() when something fails, but the new
sk does not have skb in the recv queue then and no event is logged.
Note that only tcp_inbound_ao_hash() uses a similar drop reason,
SKB_DROP_REASON_TCP_CLOSE, and this can be generalised later.
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Link: https://patch.msgid.link/20250116053441.5758-3-kuniyu@amazon.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The following patch adds a new drop reason starting with
the SOCKET_ prefix.
Let's gather the existing SOCKET_ reasons.
Note that the order is not part of uAPI.
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Link: https://patch.msgid.link/20250116053441.5758-2-kuniyu@amazon.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Address Null pointer dereference / undefined behavior in rtattr_pack
(note that size is 0 in the bad case).
Flagged by cppcheck as:
tools/testing/selftests/net/ipsec.c:230:25: warning: Possible null pointer
dereference: payload [nullPointer]
memcpy(RTA_DATA(attr), payload, size);
^
tools/testing/selftests/net/ipsec.c:1618:54: note: Calling function 'rtattr_pack',
4th argument 'NULL' value is 0
if (rtattr_pack(&req.nh, sizeof(req), XFRMA_IF_ID, NULL, 0)) {
^
tools/testing/selftests/net/ipsec.c:230:25: note: Null pointer dereference
memcpy(RTA_DATA(attr), payload, size);
^
Signed-off-by: Liu Ye <liuye@kylinos.cn>
Link: https://patch.msgid.link/20250116013037.29470-1-liuye@kylinos.cn
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
HWMON support has been added for the RTL8221/8251 PHYs integrated together
with the MAC inside the RTL8125/8126 chips. This patch extends temperature
reading support for standalone variants of the mentioned PHYs.
I don't know whether the earlier revisions of the RTL8226 also have a
built-in temperature sensor, so they have been skipped for now.
Tested on RTL8221B-VB-CG.
Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: David S. Miller <davem@davemloft.net>
This change resolves warning produced by sparse tool as currently
there is a mismatch between normal generic type in salt and endian
annotated type in macsec driver code. Endian annotated types should
be used here.
Sparse output:
warning: restricted ssci_t degrades to integer
warning: incorrect type in assignment (different base types)
expected restricted ssci_t [usertype] ssci
got unsigned int
warning: restricted __be64 degrades to integer
warning: incorrect type in assignment (different base types)
expected restricted __be64 [usertype] pn
got unsigned long long
Signed-off-by: Ales Nezbeda <anezbeda@redhat.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: David S. Miller <davem@davemloft.net>
On a 32bit system the "keylen + sizeof(struct tipc_aead_key)" math could
have an integer wrapping issue. It doesn't matter because the "keylen"
is checked on the next line, but just to make life easier for static
analysis tools, let's re-order these conditions and avoid the integer
overflow.
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Background: https://lore.kernel.org/r/20250107123615.161095-1-ericwouds@gmail.com
Since adding negotiation of in-band capabilities, it is no longer
sufficient to just look at the MLO_AN_xxx mode and PHY interface to
decide whether to do a major configuration, since the result now
depends on the capabilities of the attaching PHY.
Always trigger a major configuration in this case.
Testing log: https://lore.kernel.org/r/f20c9744-3953-40e7-a9c9-5534b25d2e2a@gmail.com
Reported-by: Eric Woudstra <ericwouds@gmail.com>
Tested-by: Eric Woudstra <ericwouds@gmail.com>
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Signed-off-by: David S. Miller <davem@davemloft.net>
aarp_send_probe_phase1() used to work by calling ndo_do_ioctl of
appletalk drivers ltpc or cops, but these two drivers have been removed
since the following commits:
commit 03dcb90dbf ("net: appletalk: remove Apple/Farallon LocalTalk PC
support")
commit 00f3696f75 ("net: appletalk: remove cops support")
Thus aarp_send_probe_phase1() no longer works, so drop it. (found by
code inspection)
Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Replace ternary (condition ? "enable" : "disable") syntax with helpers
from string_choices.h because:
1. Simple function call with one argument is easier to read. Ternary
operator has three arguments and with wrapping might lead to quite
long code.
2. Is slightly shorter thus also easier to read.
3. It brings uniformity in the text - same string.
4. Allows deduping by the linker, which results in a smaller binary
file.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
This patch addresses issues with filter counting in block (tcf_block),
particularly for software bypass scenarios, by introducing a more
accurate mechanism using useswcnt.
Previously, filtercnt and skipswcnt were introduced by:
Commit 2081fd3445 ("net: sched: cls_api: add filter counter") and
Commit f631ef39d8 ("net: sched: cls_api: add skip_sw counter")
filtercnt tracked all tp (tcf_proto) objects added to a block, and
skipswcnt counted tp objects with the skipsw attribute set.
The problem is: a single tp can contain multiple filters, some with skipsw
and others without. The current implementation fails in the case:
When the first filter in a tp has skipsw, both skipswcnt and filtercnt
are incremented, then adding a second filter without skipsw to the same
tp does not modify these counters because tp->counted is already set.
This results in bypass software behavior based solely on skipswcnt
equaling filtercnt, even when the block includes filters without
skipsw. Consequently, filters without skipsw are inadvertently bypassed.
To address this, the patch introduces useswcnt in block to explicitly count
tp objects containing at least one filter without skipsw. Key changes
include:
Whenever a filter without skipsw is added, its tp is marked with usesw
and counted in useswcnt. tc_run() now uses useswcnt to determine software
bypass, eliminating reliance on filtercnt and skipswcnt.
This refined approach prevents software bypass for blocks containing
mixed filters, ensuring correct behavior in tc_run().
Additionally, as atomic operations on useswcnt ensure thread safety and
tp->lock guards access to tp->usesw and tp->counted, the broader lock
down_write(&block->cb_lock) is no longer required in tc_new_tfilter(),
and this resolves a performance regression caused by the filter counting
mechanism during parallel filter insertions.
The improvement can be demonstrated using the following script:
# cat insert_tc_rules.sh
tc qdisc add dev ens1f0np0 ingress
for i in $(seq 16); do
taskset -c $i tc -b rules_$i.txt &
done
wait
Each of rules_$i.txt files above includes 100000 tc filter rules to a
mlx5 driver NIC ens1f0np0.
Without this patch:
# time sh insert_tc_rules.sh
real 0m50.780s
user 0m23.556s
sys 4m13.032s
With this patch:
# time sh insert_tc_rules.sh
real 0m17.718s
user 0m7.807s
sys 3m45.050s
Fixes: 047f340b36 ("net: sched: make skip_sw actually skip software")
Reported-by: Shuang Li <shuali@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Reviewed-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
Tested-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
Signed-off-by: David S. Miller <davem@davemloft.net>
tcp rst/fin packet triggers an immediate teardown of the flow which
results in sending flows back to the classic forwarding path.
This behaviour was introduced by:
da5984e510 ("netfilter: nf_flow_table: add support for sending flows back to the slow path")
b6f27d322a ("netfilter: nf_flow_table: tear down TCP flows if RST or FIN was seen")
whose goal is to expedite removal of flow entries from the hardware
table. Before these patches, the flow was released after the flow entry
timed out.
However, this approach leads to packet races when restoring the
conntrack state as well as late flow re-offload situations when the TCP
connection is ending.
This patch adds a new CLOSING state that is is entered when tcp rst/fin
packet is seen. This allows for an early removal of the flow entry from
the hardware table. But the flow entry still remains in software, so tcp
packets to shut down the flow are not sent back to slow path.
If syn packet is seen from this new CLOSING state, then this flow enters
teardown state, ct state is set to TCP_CONNTRACK_CLOSE state and packet
is sent to slow path, so this TCP reopen scenario can be handled by
conntrack. TCP_CONNTRACK_CLOSE provides a small timeout that aims at
quickly releasing this stale entry from the conntrack table.
Moreover, skip hardware re-offload from flowtable software packet if the
flow is in CLOSING state.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Tear down the flow entry in the unlikely case that the interface mtu
changes, this gives the flow a chance to refresh the cached mtu,
otherwise such refresh does not occur until flow entry expires.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Offload nf_conn entries may not see traffic for a very long time.
To prevent incorrect 'ct is stale' checks during nf_conntrack table
lookup, the gc worker extends the timeout nf_conn entries marked for
offload to a large value.
The existing logic suffers from a few problems.
Garbage collection runs without locks, its unlikely but possible
that @ct is removed right after the 'offload' bit test.
In that case, the timeout of a new/reallocated nf_conn entry will
be increased.
Prevent this by obtaining a reference count on the ct object and
re-check of the confirmed and offload bits.
If those are not set, the ct is being removed, skip the timeout
extension in this case.
Parallel teardown is also problematic:
cpu1 cpu2
gc_worker
calls flow_offload_teardown()
tests OFFLOAD bit, set
clear OFFLOAD bit
ct->timeout is repaired (e.g. set to timeout[UDP_CT_REPLIED])
nf_ct_offload_timeout() called
expire value is fetched
<INTERRUPT>
-> NF_CT_DAY timeout for flow that isn't offloaded
(and might not see any further packets).
Use cmpxchg: if ct->timeout was repaired after the 2nd 'offload bit' test
passed, then ct->timeout will only be updated of ct->timeout was not
altered in between.
As we already have a gc worker for flowtable entries, ct->timeout repair
can be handled from the flowtable gc worker.
This avoids having flowtable specific logic in the conntrack core
and avoids checking entries that were never offloaded.
This allows to remove the nf_ct_offload_timeout helper.
Its safe to use in the add case, but not on teardown.
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Its not used (and could be NULL), so remove it.
This allows to use nf_ct_refresh in places where we don't have
an skb without having to double-check that skb == NULL would be safe.
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
The conntrack entry is already public, there is a small chance that another
CPU is handling a packet in reply direction and racing with the tcp state
update.
Move this under ct spinlock.
This is done once, when ct is about to be offloaded, so this should
not result in a noticeable performance hit.
Fixes: 8437a6209f ("netfilter: nft_flow_offload: set liberal tracking mode for tcp")
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
This state reset is racy, no locks are held here.
Since commit
8437a6209f ("netfilter: nft_flow_offload: set liberal tracking mode for tcp"),
the window checks are disabled for normal data packets, but MAXACK flag
is checked when validating TCP resets.
Clear the flag so tcp reset validation checks are ignored.
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
With conditional chain deletion gone, callback code simplifies: Instead
of filling an nft_ctx object, just pass basechain to the per-chain
function. Also plain list_for_each_entry() is safe now.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Do not drop a netdev-family chain if the last interface it is registered
for vanishes. Users dumping and storing the ruleset upon shutdown to
restore it upon next boot may otherwise lose the chain and all contained
rules. They will still lose the list of devices, a later patch will fix
that. For now, this aligns the event handler's behaviour with that for
flowtables.
The controversal situation at netns exit should be no problem here:
event handler will unregister the hooks, core nftables cleanup code will
drop the chain itself.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
The 1:1 relationship between nft_hook and nf_hook_ops is about to break,
so choose the stored ifname to uniquely identify hooks.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
The stored ifname and ops.dev->name may deviate after creation due to
interface name changes. Prefer the more deterministic stored name in
dumps which also helps avoiding inadvertent changes to stored ruleset
dumps.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Prepare for hooks with NULL ops.dev pointer (due to non-existent device)
and store the interface name and length as specified by the user upon
creation. No functional change intended.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
When checking for duplicate hooks in nft_register_flowtable_net_hooks(),
comparing ops.pf value is pointless as it is always NFPROTO_NETDEV with
flowtable hooks.
Dropping the check leaves the search identical to the one in
nft_hook_list_find() so call that function instead of open coding.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
The SKB_DROP_REASON_IP_INADDRERRORS drop reason is never returned from
any function, as such it cannot be returned from the ip_route_input call
tree. The 'reason != SKB_DROP_REASON_IP_INADDRERRORS' conditional is
thus always true.
Looking back at history, commit 50038bf38e ("net: ip: make
ip_route_input() return drop reasons") changed the ip_route_input
returned value check in br_nf_pre_routing_finish from -EHOSTUNREACH to
SKB_DROP_REASON_IP_INADDRERRORS. It turns out -EHOSTUNREACH could not be
returned either from the ip_route_input call tree and this since commit
251da41301 ("ipv4: Cache ip_error() routes even when not
forwarding.").
Not a fix as this won't change the behavior. While at it use
kfree_skb_reason.
Signed-off-by: Antoine Tenart <atenart@kernel.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
The existing rbtree implementation uses singleton elements to represent
ranges, however, userspace provides a set size according to the number
of ranges in the set.
Adjust provided userspace set size to the number of singleton elements
in the kernel by multiplying the range by two.
Check if the no-match all-zero element is already in the set, in such
case release one slot in the set size.
Fixes: 0ed6389c48 ("netfilter: nf_tables: rename set implementations")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
- bump version strings, by Simon Wunderlich
- Reorder includes for distributed-arp-table.c, by Sven Eckelmann
- Fix translation table change handling, by Remi Pommarel (2 patches)
- Map VID 0 to untagged TT VLAN, by Sven Eckelmann
- Update MAINTAINERS/mailmap e-mail addresses, by the respective authors
(4 patches)
- netlink: reduce duplicate code by returning interfaces,
by Linus Lüssing
-----BEGIN PGP SIGNATURE-----
iQJKBAABCgA0FiEE1ilQI7G+y+fdhnrfoSvjmEKSnqEFAmeKTuUWHHN3QHNpbW9u
d3VuZGVybGljaC5kZQAKCRChK+OYQpKeoREQEACKKLbl6IGJ9xXTlaRhtUwr55KD
JmvJT+y25IOmJBlRpZ6zHEjDxlsF/W+Se63k4tWrQSEscqBLXL9K0nY171cU+sJ0
Nx6AF2F6GobfVuzJGoJWjOZ3eCk4+stiCsBPz0ZJmu5cN24nZRkQZWpOJCSl1KtY
huRG+bum58hw4kvaK5IU+9iLxtN5ec3STVZjEe0GKVqB6vo19+XGnPJqNSlVDDiQ
iJSl+6VjaYYG2zclyQDefo4CuILVTAxGi5vaVFhoLVkUj2QPo41BEpkfQxZ8P1HR
bFXfnOITuhCeDgzF2uwocaeW0P06kyKa+cyH4PO5BvQOp5up667aDdXtgKPBvKzV
5Fn+WN0A/SsM+tLdGkVdbU1ceNgsNYLyROJgorDNkiGUmRvrFOmhXVzhJMlcppkR
O+qh7K7Kpoyuz3a2ZiktvG5sSG0aFhQ3GRJyZlqKGV5bLzJ8e2Z+0aCFKtVw0RlZ
3lg2/iEb5//+aJKZToHH97U3PG2ahO76WPSiaTPVVfvGYS2gE7fihHWtI9Uf9FzB
AHFhiOP5oPet5jSjH0jrybOEHcSvuxsOi+vIC0hCiPV4O/fN7tjeEj4XM3uPF5LQ
FI26vvS5izhZxOWVIUPnz5pSWzTmfiRPNbckcESHt8CHjlSTUw3TPlC0jua0iXMF
shhpONEPSjZvExybLg==
=cv/9
-----END PGP SIGNATURE-----
Merge tag 'batadv-next-pullrequest-20250117' of git://git.open-mesh.org/linux-merge
Simon Wunderlich says:
====================
This cleanup patchset includes the following patches:
- bump version strings, by Simon Wunderlich
- Reorder includes for distributed-arp-table.c, by Sven Eckelmann
- Fix translation table change handling, by Remi Pommarel (2 patches)
- Map VID 0 to untagged TT VLAN, by Sven Eckelmann
- Update MAINTAINERS/mailmap e-mail addresses, by the respective authors
(4 patches)
- netlink: reduce duplicate code by returning interfaces,
by Linus Lüssing
* tag 'batadv-next-pullrequest-20250117' of git://git.open-mesh.org/linux-merge:
batman-adv: netlink: reduce duplicate code by returning interfaces
MAINTAINERS: mailmap: add entries for Antonio Quartulli
mailmap: add entries for Sven Eckelmann
mailmap: add entries for Simon Wunderlich
MAINTAINERS: update email address of Marek Linder
batman-adv: Map VID 0 to untagged TT VLAN
batman-adv: Don't keep redundant TT change events
batman-adv: Remove atomic usage for tt.local_changes
batman-adv: Reorder includes for distributed-arp-table.c
batman-adv: Start new development cycle
====================
Link: https://patch.msgid.link/20250117123910.219278-1-sw@simonwunderlich.de
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
