* Re: [PATCH net] nfc: microread: validate CARD_FOUND event length before parsing targets
From: David Heidelberg @ 2026-07-19 14:15 UTC (permalink / raw)
To: Doruk Tan Ozturk; +Cc: oe-linux-nfc, netdev, linux-kernel, stable, Pengpeng Hou
In-Reply-To: <20260713215936.23137-1-doruk@0sec.ai>
On 13/07/2026 23:59, Doruk Tan Ozturk wrote:
> microread_target_discovered() parses a device-supplied MREAD_CARD_FOUND
> event into a struct nfc_target, reading fixed offsets and -- for the
> ISO-A and ISO-A-3 gates -- a variable-length NFCID1 straight out of the
> event skb. The only length check is nfcid1_len vs sizeof(targets->nfcid1);
> skb->len itself is never validated, so a short event makes every gate
> case read out of bounds past the skb:
>
> - ISO-A / ISO-A-3: fixed ATQA/SAK/LEN reads plus a memcpy of an
> attacker-controlled nfcid1_len bytes from the NFCID1 offset;
> - ISO-B / NFC-T1 / NFC-T3: a fixed 4- or 8-byte NFCID1 memcpy from a
> fixed offset.
>
> The copied nfcid1 is exported to user space via nfc_targets_found(), so
> the over-read is an information leak (and a possible oops on an unmapped
> page).
>
> Reject events too short for the fields each gate case reads.
>
> Found by 0sec (https://0sec.ai).
>
> Fixes: cfad1ba87150 ("NFC: Initial support for Inside Secure microread")
> Cc: stable@vger.kernel.org
> Assisted-by: 0sec
> Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
> ---
> drivers/nfc/microread/microread.c | 31 +++++++++++++++++++++++++++++--
> 1 file changed, 29 insertions(+), 2 deletions(-)
>
Hello Doruk,
thanks for the patch, 1. 7. I got the same one from Pengpeng, which needs to
address Fixes and Cc tag [1].
Feel free to sync together for the future effort to not get duplicated (and
tokens not wasted ;-) )
Adding Pengpeng to Cc.
David
[1] https://lore.kernel.org/all/20260701053709.45176-1-pengpeng@iscas.ac.cn/
^ permalink raw reply
* RE: [PATCH ethtool-next v3 3/3] qsfp: print all compliance codes
From: Danielle Ratson @ 2026-07-19 14:08 UTC (permalink / raw)
To: Aleksander Jan Bajkowski, mkubecek@suse.cz, andrew@lunn.ch,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, jbe@pengutronix.de, netdev@vger.kernel.org
In-Reply-To: <20260719090458.659332-3-olek2@wp.pl>
> -----Original Message-----
> From: Aleksander Jan Bajkowski <olek2@wp.pl>
> Sent: Sunday, 19 July 2026 12:01
> To: Danielle Ratson <danieller@nvidia.com>; olek2@wp.pl;
> mkubecek@suse.cz; andrew@lunn.ch; davem@davemloft.net;
> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com;
> jbe@pengutronix.de; netdev@vger.kernel.org
> Subject: [PATCH ethtool-next v3 3/3] qsfp: print all compliance codes
>
> QSFP modules implement multiple compliance codes. This is common for dual-
> rate modules. Before the `json` option was introduced, all compliance codes
> were displayed. Currently, only the last code is displayed. This commit fixes
> that bug. Compliance codes are represented as array.
>
> Fixes: 4071862f58d8 ("sfpid: Add JSON output handling to --module-info in
> SFF8079 modules")
> Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl>
> ---
> Changes in v3:
> - add patch to series
> ---
Reviewed-by: Danielle Ratson <danieller@nvidia.com>
^ permalink raw reply
* RE: [PATCH ethtool-next v3 2/3] sfpid: print all compliance codes
From: Danielle Ratson @ 2026-07-19 14:07 UTC (permalink / raw)
To: Aleksander Jan Bajkowski, mkubecek@suse.cz, andrew@lunn.ch,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, jbe@pengutronix.de, netdev@vger.kernel.org
In-Reply-To: <20260719090458.659332-2-olek2@wp.pl>
> -----Original Message-----
> From: Aleksander Jan Bajkowski <olek2@wp.pl>
> Sent: Sunday, 19 July 2026 12:01
> To: Danielle Ratson <danieller@nvidia.com>; olek2@wp.pl;
> mkubecek@suse.cz; andrew@lunn.ch; davem@davemloft.net;
> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com;
> jbe@pengutronix.de; netdev@vger.kernel.org
> Subject: [PATCH ethtool-next v3 2/3] sfpid: print all compliance codes
>
> SFP modules implement multiple compliance codes. This is common for dual-
> rate modules. Before the `json` option was introduced, all compliance codes
> were displayed. Currently, only the last code is displayed. This commit fixes
> that bug. Compliance codes are represented as array.
>
> Before:
> $ ethtool -m sfp-wan
> ...
> Transceiver codes : 0x00 0x00 0x00 0x01 0x20 0x40 0x0c
> 0x15 0x00
> Transceiver type : FC: 100 MBytes/sec
> ...
> $ ethtool --json -m sfp-wan
> [ {
> ...
> "transceiver_codes": [ 0,0,0,1,32,64,12,21,0 ],
> "transceiver_type": "FC: 100 MBytes/sec", ...
> } ]
>
> After:
> $ ethtool -m sfp-wan
> ...
> Transceiver codes : 0x00 0x00 0x00 0x01 0x20 0x40 0x0c
> 0x15 0x00
> Transceiver type : Ethernet: 1000BASE-SX
> Transceiver type : FC: intermediate distance (I)
> Transceiver type : FC: Shortwave laser w/o OFC (SN)
> Transceiver type : FC: Multimode, 62.5um (M6)
> Transceiver type : FC: Multimode, 50um (M5)
> Transceiver type : FC: 400 MBytes/sec
> Transceiver type : FC: 200 MBytes/sec
> Transceiver type : FC: 100 MBytes/sec
> ...
> $ ethtool --json -m sfp-wan
> [ {
> ...
> "transceiver_codes": [ 0,0,0,1,32,64,12,21,0 ],
> "transceiver_type": [ "Ethernet: 1000BASE-SX","FC: intermediate distance
> (I)","FC: Shortwave laser w/o OFC (SN)","FC: Multimode, 62.5um (M6)","FC:
> Multimode, 50um (M5)","FC: 400 MBytes/sec","FC: 200 MBytes/sec","FC:
> 100 MBytes/sec" ], ...
> } ]
>
> Fixes: 4071862f58d8 ("sfpid: Add JSON output handling to --module-info in
> SFF8079 modules")
> Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl>
> ---
> Changes in v3:
> - fix indentation and checkpatch warnings Changes in v2:
> - drop </pre> leftover
> - use single sfp module in Before/After
> - rename module_print_array_string() ->
> module_print_any_array_string_entry()
> ---
Reviewed-by: Danielle Ratson <danieller@nvidia.com>
^ permalink raw reply
* RE: [PATCH ethtool-next v3 1/3] sfpid: print all implemented options
From: Danielle Ratson @ 2026-07-19 14:07 UTC (permalink / raw)
To: Aleksander Jan Bajkowski, mkubecek@suse.cz, andrew@lunn.ch,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, jbe@pengutronix.de, netdev@vger.kernel.org
In-Reply-To: <20260719090458.659332-1-olek2@wp.pl>
> -----Original Message-----
> From: Aleksander Jan Bajkowski <olek2@wp.pl>
> Sent: Sunday, 19 July 2026 12:01
> To: Danielle Ratson <danieller@nvidia.com>; olek2@wp.pl;
> mkubecek@suse.cz; andrew@lunn.ch; davem@davemloft.net;
> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com;
> jbe@pengutronix.de; netdev@vger.kernel.org
> Subject: [PATCH ethtool-next v3 1/3] sfpid: print all implemented options
>
> SFP modules implement multiple options. Before the “json” option was
> introduced, all options were listed. Currently, only the last option is listed. This
> commit fixes this bug. Options are represented as array.
>
> Before:
> $ ethtool -m sfp-wan
> ...
> Option values : 0x00 0x32
> Option : RATE_SELECT implemented
> ...
> $ ethtool --json -m sfp-wan
> [ {
> ...
> "option_values": [ 0,50 ],
> "option": "RATE_SELECT implemented", ...
> } ]
>
> After:
> $ ethtool -m sfp-wan
> ...
> Option values : 0x00 0x32
> Option : RX_LOS implemented
> Option : TX_DISABLE implemented
> Option : RATE_SELECT implemented
> ...
> $ ethtool --json -m sfp-wan
> [ {
> ...
> "option_values": [ 0,50 ],
> "option": [ "RX_LOS implemented","TX_DISABLE
> implemented","RATE_SELECT implemented" ], ...
> } ]
>
> Fixes: 4071862f58d8 ("sfpid: Add JSON output handling to --module-info in
> SFF8079 modules")
> Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl>
> ---
> Changes in v3:
> - fix indentation and checkpatch warnings Changes in v2:
> - fix typo introduced -> introduced
> - rename module_print_array_string() ->
> module_print_any_array_string_entry()
> ---
Reviewed-by: Danielle Ratson <danieller@nvidia.com>
^ permalink raw reply
* [PATCH v4 net 6/6] selftests/xsk: account reclaimed invalid Tx descriptors
From: Maciej Fijalkowski @ 2026-07-19 13:56 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms, bjorn,
kerneljasonxing, Maciej Fijalkowski, Jason Xing
In-Reply-To: <20260719135609.147823-1-maciej.fijalkowski@intel.com>
Invalid Tx descriptors are now returned through the completion ring,
regardless of whether they form a standalone packet or belong to an
invalid multi-buffer packet.
The selftests previously counted only descriptors belonging to valid
packets, with a special exception for some invalid multi-buffer packets
in verbatim streams. This undercounts completion entries when a
standalone invalid descriptor or another invalid packet is reclaimed by
the kernel.
Keep valid_pkts as the number of packets expected on the Rx side, but
count every descriptor submitted to the Tx ring in valid_frags, as every
such descriptor is now expected to be returned through the completion
ring.
Make fragment counting in verbatim mode follow the packet boundary
instead of stopping at the first invalid fragment. Update custom stream
generation so an invalid middle fragment terminates the generated Rx
packet while Tx completion accounting still covers the complete invalid
packet.
Also add explicit end fragments after invalid middle descriptors. This
exercises the kernel drain logic and verifies that subsequent valid
packets are not interpreted as continuations of the invalid packet.
Reviewed-by: Jason Xing <kernelxing@tencent.com>
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
.../selftests/bpf/prog_tests/test_xsk.c | 26 ++++++++++---------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
index f0e0f3c4f7a3..4549358cc8c2 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
@@ -427,14 +427,14 @@ static u32 pkt_nb_frags(u32 frame_size, struct pkt_stream *pkt_stream, struct pk
}
/* Search for the end of the packet in verbatim mode */
- if (!pkt_continues(pkt->options) || !pkt->valid)
+ if (!pkt_continues(pkt->options))
return nb_frags;
next_frag = pkt_stream->current_pkt_nb;
pkt++;
while (next_frag++ < pkt_stream->nb_pkts) {
nb_frags++;
- if (!pkt_continues(pkt->options) || !pkt->valid)
+ if (!pkt_continues(pkt->options))
break;
pkt++;
}
@@ -665,11 +665,11 @@ static struct pkt_stream *__pkt_stream_generate_custom(struct ifobject *ifobj, s
if (!frame->valid || !pkt_continues(frame->options))
payload++;
} else {
- if (frame->valid)
+ if (frame->valid) {
len += frame->len;
- if (frame->valid && pkt_continues(frame->options))
- continue;
-
+ if (pkt_continues(frame->options))
+ continue;
+ }
pkt->pkt_nb = pkt_nb;
pkt->len = len;
pkt->valid = frame->valid;
@@ -1250,10 +1250,9 @@ static int __send_pkts(struct ifobject *ifobject, struct xsk_socket_info *xsk,
}
}
- if (pkt && pkt->valid) {
+ if (pkt && pkt->valid)
valid_pkts++;
- valid_frags += nb_frags;
- }
+ valid_frags += nb_frags;
}
pthread_mutex_lock(&pacing_mutex);
@@ -2099,13 +2098,16 @@ int testapp_invalid_desc_mb(struct test_spec *test)
{0, 0, 0, false, 0},
/* Invalid address in the second frame */
{0, XSK_UMEM__LARGE_FRAME_SIZE, 0, false, XDP_PKT_CONTD},
- {umem_sz, XSK_UMEM__LARGE_FRAME_SIZE, 0, false, XDP_PKT_CONTD},
+ {umem_sz * 2, XSK_UMEM__LARGE_FRAME_SIZE, 0, false, XDP_PKT_CONTD},
+ {0, MIN_PKT_SIZE, 0, false, 0},
/* Invalid len in the middle */
{0, XSK_UMEM__LARGE_FRAME_SIZE, 0, false, XDP_PKT_CONTD},
{0, XSK_UMEM__INVALID_FRAME_SIZE, 0, false, XDP_PKT_CONTD},
+ {0, MIN_PKT_SIZE, 0, false, 0},
/* Invalid options in the middle */
{0, XSK_UMEM__LARGE_FRAME_SIZE, 0, false, XDP_PKT_CONTD},
{0, XSK_UMEM__LARGE_FRAME_SIZE, 0, false, XSK_DESC__INVALID_OPTION},
+ {0, MIN_PKT_SIZE, 0, false, 0},
/* Transmit 2 frags, receive 3 */
{0, XSK_UMEM__MAX_FRAME_SIZE, 0, true, XDP_PKT_CONTD},
{0, XSK_UMEM__MAX_FRAME_SIZE, 0, true, 0},
@@ -2117,8 +2119,8 @@ int testapp_invalid_desc_mb(struct test_spec *test)
if (umem->unaligned_mode) {
/* Crossing a chunk boundary allowed */
- pkts[12].valid = true;
- pkts[13].valid = true;
+ pkts[15].valid = true;
+ pkts[16].valid = true;
}
test->mtu = MAX_ETH_JUMBO_SIZE;
--
2.43.0
^ permalink raw reply related
* [PATCH v4 net 5/6] selftests/xsk: fix too-many-frags multi-buffer Tx test
From: Maciej Fijalkowski @ 2026-07-19 13:56 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms, bjorn,
kerneljasonxing, Maciej Fijalkowski, Jason Xing
In-Reply-To: <20260719135609.147823-1-maciej.fijalkowski@intel.com>
The too-many-frags test describes a packet that is valid from the Tx
ring ownership point of view, but invalid for transmission because it
exceeds the supported number of fragments.
Keep the generated Tx descriptors valid so that __send_pkts() accounts
them as outstanding descriptors that must be reclaimed through the CQ.
Then mark the corresponding Rx packet invalid so the test still does
not expect the oversized packet to appear on the receive side.
Add a valid synchronization packet after the oversized packet so the
test can verify that the Tx path drains the bad packet and resumes at
the next packet boundary.
Reviewed-by: Jason Xing <kernelxing@tencent.com>
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
.../selftests/bpf/prog_tests/test_xsk.c | 24 ++++++++++++-------
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
index 477aedbb01ba..f0e0f3c4f7a3 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
@@ -2270,7 +2270,7 @@ int testapp_too_many_frags(struct test_spec *test)
max_frags += 1;
}
- pkts = calloc(2 * max_frags + 2, sizeof(struct pkt));
+ pkts = calloc(2 * max_frags + 3, sizeof(struct pkt));
if (!pkts)
return TEST_FAILURE;
@@ -2288,24 +2288,30 @@ int testapp_too_many_frags(struct test_spec *test)
}
pkts[max_frags].options = 0;
- /* An invalid packet with the max amount of frags but signals packet
- * continues on the last frag
- */
- for (i = max_frags + 1; i < 2 * max_frags + 1; i++) {
+ /* An invalid packet with the max + 1 amount of frags */
+ for (i = max_frags + 1; i < 2 * max_frags + 2; i++) {
pkts[i].len = MIN_PKT_SIZE;
pkts[i].options = XDP_PKT_CONTD;
- pkts[i].valid = false;
+ pkts[i].valid = true;
}
+ pkts[2 * max_frags + 1].options = 0;
/* Valid packet for synch */
- pkts[2 * max_frags + 1].len = MIN_PKT_SIZE;
- pkts[2 * max_frags + 1].valid = true;
+ pkts[2 * max_frags + 2].len = MIN_PKT_SIZE;
+ pkts[2 * max_frags + 2].valid = true;
- if (pkt_stream_generate_custom(test, pkts, 2 * max_frags + 2)) {
+ if (pkt_stream_generate_custom(test, pkts, 2 * max_frags + 3)) {
free(pkts);
return TEST_FAILURE;
}
+ /* The generated Tx stream must keep the too-big packet valid so that
+ * __send_pkts() accounts its descriptors in outstanding_tx. The Rx
+ * stream, however, must not expect this packet on the wire.
+ */
+ test->ifobj_rx->xsk->pkt_stream->pkts[2].valid = false;
+ test->ifobj_rx->xsk->pkt_stream->nb_valid_entries--;
+
ret = testapp_validate_traffic(test);
free(pkts);
return ret;
--
2.43.0
^ permalink raw reply related
* [PATCH v4 net 4/6] xsk: reclaim invalid Tx descriptors in ZC batch path
From: Maciej Fijalkowski @ 2026-07-19 13:56 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms, bjorn,
kerneljasonxing, Maciej Fijalkowski, Jason Xing
In-Reply-To: <20260719135609.147823-1-maciej.fijalkowski@intel.com>
The zero-copy Tx batch parser stops when it encounters an invalid
descriptor. If this happens after one or more continuation descriptors,
the Tx consumer can be advanced past fragments that are neither submitted
to the driver nor returned to userspace through the completion ring.
A similar problem occurs when a packet exceeds xdp_zc_max_segs. The
descriptors consumed up to the limit are released without completion, and
the remaining continuation descriptors can subsequently be interpreted
as the beginning of another packet.
Parse Tx batches in packet units and distinguish descriptors belonging to
complete valid packets from descriptors consumed while draining an
invalid or oversized packet. Return the former to the driver and append
the latter to the CQ address area so userspace can reclaim their UMEM
frames.
Treat a standalone invalid descriptor as a one-descriptor reclaim-only
packet. Advancing the Tx-ring consumer releases the ring slot, but does
not by itself return ownership of the referenced UMEM frame to userspace.
Once draining starts, continue until the packet's end-of-packet
descriptor is consumed. Preserve the drain state on the socket when EOP
has not yet been supplied, so draining can continue during a later call.
Leave incomplete but otherwise valid packets on the Tx ring.
Shared-UMEM pools using multi-buffer Tx also need packet-framed parsing.
Walk their Tx sockets one packet at a time, preserving the existing
per-socket fairness scheme, instead of using the legacy one-descriptor
fallback. Keep that fallback for shared pools that do not use
multi-buffer Tx. Since the drain state is maintained per socket and both
the singular and shared paths can resume an interrupted drain, changing
the socket list from singular to shared requires no special bind-time
transition.
CQ entries are positional, and drivers may complete only part of the Tx
work returned by xsk_tx_peek_release_desc_batch(). Therefore, reclaim-only
entries cannot be published immediately when earlier driver-visible
descriptors are still outstanding.
Track the number of driver-visible CQ entries preceding the reclaim
entries. Let xsk_tx_completed() publish partial hardware Tx completions,
and publish the reclaim entries only after every earlier Tx descriptor
has completed. Complete a reclaim-only batch immediately when there is no
driver-visible work in front of it, and prevent another Tx batch from
being appended while reclaim entries remain pending.
Also cap batch processing by the size of the pool's temporary descriptor
array, as Tx rings belonging to sockets sharing a UMEM may have different
sizes.
This ensures that every invalid Tx descriptor consumed by the ZC batch
path is either submitted to the driver as part of a valid packet or
returned to userspace without violating CQ completion ordering.
Fixes: cf24f5a5feea ("xsk: add support for AF_XDP multi-buffer on Tx path")
Reviewed-by: Jason Xing <kernelxing@tencent.com>
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
Documentation/networking/af_xdp.rst | 54 ++++----
include/net/xsk_buff_pool.h | 3 +
net/xdp/xsk.c | 187 +++++++++++++++++++++++++---
net/xdp/xsk_buff_pool.c | 1 +
net/xdp/xsk_queue.h | 65 +++++++---
5 files changed, 248 insertions(+), 62 deletions(-)
diff --git a/Documentation/networking/af_xdp.rst b/Documentation/networking/af_xdp.rst
index 50d92084a49c..cc3f0d16b28f 100644
--- a/Documentation/networking/af_xdp.rst
+++ b/Documentation/networking/af_xdp.rst
@@ -43,12 +43,13 @@ UMEM also has two rings: the FILL ring and the COMPLETION ring. The
FILL ring is used by the application to send down addr for the kernel
to fill in with RX packet data. References to these frames will then
appear in the RX ring once each packet has been received. The
-COMPLETION ring, on the other hand, contains frame addr that the
-kernel has transmitted completely and can now be used again by user
-space, for either TX or RX. Thus, the frame addrs appearing in the
-COMPLETION ring are addrs that were previously transmitted using the
-TX ring. In summary, the RX and FILL rings are used for the RX path
-and the TX and COMPLETION rings are used for the TX path.
+COMPLETION ring, on the other hand, contains frame addresses from Tx
+descriptors that the kernel has finished processing and that can now be
+used again by user space, for either Tx or Rx. This includes frames whose
+transmission has completed as well as frames referenced by invalid Tx
+descriptors rejected by the kernel. A completion therefore returns
+ownership of a frame to user space, but does not by itself guarantee that
+the packet was successfully transmitted.
The socket is then finally bound with a bind() call to a device and a
specific queue id on that device, and it is not until bind is
@@ -169,14 +170,15 @@ chunks mode, then the incoming addr will be left untouched.
UMEM Completion Ring
~~~~~~~~~~~~~~~~~~~~
-The COMPLETION Ring is used transfer ownership of UMEM frames from
+The COMPLETION Ring is used to transfer ownership of UMEM frames from
kernel-space to user-space. Just like the FILL ring, UMEM indices are
-used.
-
-Frames passed from the kernel to user-space are frames that has been
-sent (TX ring) and can be used by user-space again.
-
-The user application consumes UMEM addrs from this ring.
+used. Frames passed from the kernel to user-space are frames referenced
+by Tx descriptors that the kernel has finished processing and can be
+used by user-space again. This includes both frames whose transmission
+has completed and frames referenced by invalid Tx descriptors that were
+rejected and reclaimed by the kernel. A completion entry does not
+guarantee successful packet transmission. The user application consumes
+UMEM addrs from this ring.
RX Ring
@@ -504,21 +506,25 @@ will be treated as an invalid descriptor.
These are the semantics for producing packets onto AF_XDP Tx ring
consisting of multiple frames:
-* When an invalid descriptor is found, all the other
- descriptors/frames of this packet are marked as invalid and not
- completed. The next descriptor is treated as the start of a new
- packet, even if this was not the intent (because we cannot guess
- the intent). As before, if your program is producing invalid
- descriptors you have a bug that must be fixed.
+* When an invalid descriptor is found, the complete packet is treated as
+ invalid. The kernel consumes descriptors through the descriptor marking
+ the end of the packet and returns all their frame addresses through the
+ COMPLETION ring. A standalone invalid descriptor is treated as a
+ one-descriptor invalid packet. The descriptor following the end of the
+ invalid packet is treated as the start of a new packet. As before, if
+ your program is producing invalid descriptors you have a bug that must
+ be fixed. Rejected descriptors are reported in the ``tx_invalid_descs``
+ statistic.
* Zero length descriptors are treated as invalid descriptors.
* For copy mode, the maximum supported number of frames in a packet is
- equal to CONFIG_MAX_SKB_FRAGS + 1. If it is exceeded, all
- descriptors accumulated so far are dropped and treated as
- invalid. To produce an application that will work on any system
- regardless of this config setting, limit the number of frags to 18,
- as the minimum value of the config is 17.
+ equal to CONFIG_MAX_SKB_FRAGS + 1. If it is exceeded, all descriptors
+ through the end of the oversized packet are consumed, treated as invalid,
+ and their frame addresses are returned through the COMPLETION ring. To
+ produce an application that will work on any system regardless of this
+ config setting, limit the number of frags to 18, as the minimum value of
+ the config is 17.
* For zero-copy mode, the limit is up to what the NIC HW
supports. Usually at least five on the NICs we have checked. We
diff --git a/include/net/xsk_buff_pool.h b/include/net/xsk_buff_pool.h
index f5e737a83055..2bb1d122b1bc 100644
--- a/include/net/xsk_buff_pool.h
+++ b/include/net/xsk_buff_pool.h
@@ -78,6 +78,9 @@ struct xsk_buff_pool {
u32 chunk_size;
u32 chunk_shift;
u32 frame_len;
+ u32 tx_descs_nentries;
+ u32 reclaim_descs;
+ u32 tx_zc_pending_descs;
u32 xdp_zc_max_segs;
u8 tx_metadata_len; /* inherited from umem */
u8 cached_need_wakeup;
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 091792d1d82d..f906d51b6699 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -499,6 +499,23 @@ void __xsk_map_flush(struct list_head *flush_list)
void xsk_tx_completed(struct xsk_buff_pool *pool, u32 nb_entries)
{
+ u32 reclaim_descs = READ_ONCE(pool->reclaim_descs);
+
+ if (unlikely(reclaim_descs)) {
+ u32 pending_descs = READ_ONCE(pool->tx_zc_pending_descs);
+
+ if (nb_entries < pending_descs) {
+ WRITE_ONCE(pool->tx_zc_pending_descs,
+ pending_descs - nb_entries);
+ xskq_prod_submit_n(pool->cq, nb_entries);
+ return;
+ }
+
+ WRITE_ONCE(pool->tx_zc_pending_descs, 0);
+ nb_entries += reclaim_descs;
+ WRITE_ONCE(pool->reclaim_descs, 0);
+ }
+
xskq_prod_submit_n(pool->cq, nb_entries);
}
EXPORT_SYMBOL(xsk_tx_completed);
@@ -574,24 +591,157 @@ static u32 xsk_tx_peek_release_fallback(struct xsk_buff_pool *pool, u32 max_entr
return nb_pkts;
}
+static void xsk_tx_commit_batch(struct xsk_buff_pool *pool,
+ struct xsk_tx_batch *batch)
+{
+ u32 nb_descs = xsk_tx_batch_cq_descs(batch);
+ u32 cq_cached_prod;
+
+ if (!nb_descs)
+ return;
+
+ cq_cached_prod = pool->cq->cached_prod;
+ xskq_prod_write_addr_batch(pool->cq, pool->tx_descs, nb_descs);
+
+ if (unlikely(batch->reclaim_descs)) {
+ u32 cq_pending_descs;
+
+ /* CQ is positional. Descriptors already written but not
+ * submitted must complete before any reclaim-only descriptors
+ * appended below.
+ */
+ cq_pending_descs = cq_cached_prod - xskq_get_prod(pool->cq);
+
+ WRITE_ONCE(pool->tx_zc_pending_descs,
+ batch->tx_descs + cq_pending_descs);
+ WRITE_ONCE(pool->reclaim_descs, batch->reclaim_descs);
+ if (unlikely(!pool->tx_zc_pending_descs))
+ xsk_tx_completed(pool, 0);
+ }
+}
+
+static struct xsk_tx_batch
+__xsk_tx_peek_release_desc_batch(struct xsk_buff_pool *pool, struct xdp_sock *xs,
+ struct xdp_desc *descs, u32 max_descs)
+{
+ struct xsk_tx_batch batch = {};
+ u32 entries;
+
+ entries = xskq_cons_nb_entries(xs->tx, max_descs);
+ if (!entries)
+ return batch;
+
+ batch = xskq_cons_read_desc_batch(xs, pool, descs, max_descs);
+ if (!xsk_tx_batch_cq_descs(&batch)) {
+ xs->tx->queue_empty_descs++;
+ } else {
+ __xskq_cons_release(xs->tx);
+ xs->sk.sk_write_space(&xs->sk);
+ }
+ return batch;
+}
+
+static struct xsk_tx_batch
+xsk_tx_peek_release_shared_desc_batch(struct xsk_buff_pool *pool, u32 max_descs)
+{
+ u32 cq_descs_before, cq_descs_after;
+ struct xsk_tx_batch sum_batch = {};
+ bool budget_exhausted;
+ u32 per_socket_budget;
+ struct xdp_sock *xs;
+
+ /* The fairness quota must allow one maximum-sized valid packet. */
+ per_socket_budget = max_t(u32, MAX_PER_SOCKET_BUDGET,
+ pool->xdp_zc_max_segs);
+
+again:
+ budget_exhausted = false;
+ cq_descs_before = xsk_tx_batch_cq_descs(&sum_batch);
+ list_for_each_entry_rcu(xs, &pool->xsk_tx_list, tx_list) {
+ u32 budget, budget_left, offset, remaining, used;
+ struct xsk_tx_batch curr_batch;
+
+ /* Once reclaim-only descriptors have been appended to the CQ
+ * address area, do not append driver-visible Tx descriptors
+ * from another socket after them. xsk_tx_completed() relies on
+ * all driver-visible descriptors preceding all reclaim-only
+ * descriptors in CQ order.
+ */
+ if (sum_batch.reclaim_descs)
+ break;
+
+ /* be gentle when playing with pool->tx_descs */
+ offset = xsk_tx_batch_cq_descs(&sum_batch);
+ if (offset >= max_descs)
+ break;
+
+ if (xs->tx_budget_spent >= per_socket_budget) {
+ if (xskq_cons_nb_entries(xs->tx, 1))
+ budget_exhausted = true;
+ continue;
+ }
+
+ budget_left = per_socket_budget - xs->tx_budget_spent;
+ remaining = max_descs - offset;
+ budget = min(remaining, budget_left);
+
+ curr_batch = __xsk_tx_peek_release_desc_batch(pool, xs,
+ pool->tx_descs + offset,
+ budget);
+ used = xsk_tx_batch_cq_descs(&curr_batch);
+ if (!used) {
+ if (curr_batch.budget_limited && budget_left < remaining)
+ budget_exhausted = true;
+ continue;
+ }
+
+ xs->tx_budget_spent += used;
+ sum_batch.tx_descs += curr_batch.tx_descs;
+ sum_batch.reclaim_descs = curr_batch.reclaim_descs;
+ }
+
+ cq_descs_after = xsk_tx_batch_cq_descs(&sum_batch);
+
+ if (sum_batch.reclaim_descs || cq_descs_after >= max_descs)
+ return sum_batch;
+
+ /* Continue filling the batch while this pass made progress */
+ if (cq_descs_before != cq_descs_after)
+ goto again;
+
+ if (!budget_exhausted)
+ return sum_batch;
+
+ list_for_each_entry_rcu(xs, &pool->xsk_tx_list, tx_list)
+ xs->tx_budget_spent = 0;
+ goto again;
+}
+
u32 xsk_tx_peek_release_desc_batch(struct xsk_buff_pool *pool, u32 nb_pkts)
{
+ struct xsk_tx_batch batch = {};
struct xdp_sock *xs;
+ bool umem_shared;
rcu_read_lock();
- if (!list_is_singular(&pool->xsk_tx_list)) {
- /* Fallback to the non-batched version */
- rcu_read_unlock();
- return xsk_tx_peek_release_fallback(pool, nb_pkts);
- }
+ if (unlikely(READ_ONCE(pool->reclaim_descs)))
+ goto out;
- xs = list_first_or_null_rcu(&pool->xsk_tx_list, struct xdp_sock, tx_list);
- if (!xs) {
- nb_pkts = 0;
+ xs = list_first_or_null_rcu(&pool->xsk_tx_list, struct xdp_sock,
+ tx_list);
+ if (!xs)
goto out;
- }
- nb_pkts = xskq_cons_nb_entries(xs->tx, nb_pkts);
+ nb_pkts = min(nb_pkts, pool->tx_descs_nentries);
+ if (!nb_pkts)
+ goto out;
+
+ umem_shared = !list_is_singular(&pool->xsk_tx_list);
+
+ if (umem_shared && !(pool->umem->flags & XDP_UMEM_SG_FLAG)) {
+ rcu_read_unlock();
+ return xsk_tx_peek_release_fallback(pool, nb_pkts);
+ }
/* This is the backpressure mechanism for the Tx path. Try to
* reserve space in the completion queue for all packets, but
@@ -603,19 +753,16 @@ u32 xsk_tx_peek_release_desc_batch(struct xsk_buff_pool *pool, u32 nb_pkts)
if (!nb_pkts)
goto out;
- nb_pkts = xskq_cons_read_desc_batch(xs->tx, pool, nb_pkts);
- if (!nb_pkts) {
- xs->tx->queue_empty_descs++;
- goto out;
- }
-
- __xskq_cons_release(xs->tx);
- xskq_prod_write_addr_batch(pool->cq, pool->tx_descs, nb_pkts);
- xs->sk.sk_write_space(&xs->sk);
+ batch = umem_shared ?
+ xsk_tx_peek_release_shared_desc_batch(pool, nb_pkts) :
+ __xsk_tx_peek_release_desc_batch(pool, xs,
+ pool->tx_descs,
+ nb_pkts);
+ xsk_tx_commit_batch(pool, &batch);
out:
rcu_read_unlock();
- return nb_pkts;
+ return batch.tx_descs;
}
EXPORT_SYMBOL(xsk_tx_peek_release_desc_batch);
diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
index 12c9fb29af05..a4089480b22b 100644
--- a/net/xdp/xsk_buff_pool.c
+++ b/net/xdp/xsk_buff_pool.c
@@ -51,6 +51,7 @@ int xp_alloc_tx_descs(struct xsk_buff_pool *pool, struct xdp_sock *xs,
if (!pool->tx_descs)
return -ENOMEM;
+ pool->tx_descs_nentries = nentries;
return 0;
}
diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
index 3e3fbb73d23e..1bc42c8902f4 100644
--- a/net/xdp/xsk_queue.h
+++ b/net/xdp/xsk_queue.h
@@ -58,6 +58,17 @@ struct parsed_desc {
u32 valid;
};
+struct xsk_tx_batch {
+ u32 tx_descs;
+ u32 reclaim_descs;
+ bool budget_limited;
+};
+
+static inline u32 xsk_tx_batch_cq_descs(const struct xsk_tx_batch *batch)
+{
+ return batch->tx_descs + batch->reclaim_descs;
+}
+
/* The structure of the shared state of the rings are a simple
* circular buffer, as outlined in
* Documentation/core-api/circular-buffers.rst. For the Rx and
@@ -263,17 +274,18 @@ static inline void parse_desc(struct xsk_queue *q, struct xsk_buff_pool *pool,
parsed->mb = xp_mb_desc(desc);
}
-static inline
-u32 xskq_cons_read_desc_batch(struct xsk_queue *q, struct xsk_buff_pool *pool,
- u32 max)
+static inline struct xsk_tx_batch
+xskq_cons_read_desc_batch(struct xdp_sock *xs, struct xsk_buff_pool *pool,
+ struct xdp_desc *descs, u32 max)
{
- u32 cached_cons = q->cached_cons, nb_entries = 0;
- struct xdp_desc *descs = pool->tx_descs;
- u32 total_descs = 0, nr_frags = 0;
+ bool drain = READ_ONCE(xs->drain_cont);
+ u32 cached_cons, nb_entries = 0;
+ struct xsk_tx_batch batch = {};
+ struct xsk_queue *q = xs->tx;
+ u32 nr_frags = 0;
+
+ cached_cons = q->cached_cons;
- /* track first entry, if stumble upon *any* invalid descriptor, rewind
- * current packet that consists of frags and stop the processing
- */
while (cached_cons != q->cached_prod && nb_entries < max) {
struct xdp_rxtx_ring *ring = (struct xdp_rxtx_ring *)q->ring;
u32 idx = cached_cons & q->ring_mask;
@@ -283,25 +295,42 @@ u32 xskq_cons_read_desc_batch(struct xsk_queue *q, struct xsk_buff_pool *pool,
cached_cons++;
parse_desc(q, pool, &descs[nb_entries], &parsed);
if (unlikely(!parsed.valid))
- break;
+ drain = true;
+
+ nr_frags++;
+ nb_entries++;
if (likely(!parsed.mb)) {
- total_descs += (nr_frags + 1);
- nr_frags = 0;
- } else {
- nr_frags++;
- if (nr_frags == pool->xdp_zc_max_segs) {
+ if (unlikely(drain)) {
+ batch.reclaim_descs = nr_frags;
+ WRITE_ONCE(xs->drain_cont, false);
nr_frags = 0;
break;
}
+
+ batch.tx_descs += nr_frags;
+ nr_frags = 0;
+ continue;
+ }
+
+ if (nr_frags == pool->xdp_zc_max_segs)
+ drain = true;
+ }
+
+ if (nr_frags) {
+ if (drain) {
+ batch.reclaim_descs = nr_frags;
+ WRITE_ONCE(xs->drain_cont, true);
+ } else {
+ if (nb_entries == max)
+ batch.budget_limited = true;
+ cached_cons -= nr_frags;
}
- nb_entries++;
}
- cached_cons -= nr_frags;
/* Release valid plus any invalid entries */
xskq_cons_release_n(q, cached_cons - q->cached_cons);
- return total_descs;
+ return batch;
}
/* Functions for consumers */
--
2.43.0
^ permalink raw reply related
* [PATCH v4 net 3/6] xsk: provide sufficient space in pool->tx_descs
From: Maciej Fijalkowski @ 2026-07-19 13:56 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms, bjorn,
kerneljasonxing, Maciej Fijalkowski, Jason Xing
In-Reply-To: <20260719135609.147823-1-maciej.fijalkowski@intel.com>
The temporary Tx descriptor array in an XSK buffer pool is currently
sized from the Tx ring of the socket that creates the pool.
This is insufficient for shared-UMEM Tx. A later socket may have a
larger Tx ring and submit a valid multi-buffer packet containing more
descriptors than the first socket's ring, while still remaining within
the device's xdp_zc_max_segs limit.
A packet-framed batch parser bounded by the temporary array cannot reach
the end-of-packet descriptor in that case. It leaves the packet on the
Tx ring and encounters the same packet on every subsequent attempt,
stalling Tx processing for that socket.
Size the temporary descriptor array to the larger of the first Tx ring
and the device's xdp_zc_max_segs capability. This keeps the array large
enough to inspect one maximum-sized valid packet. Larger shared Tx rings
do not require further resizing, as they can be processed over multiple
batches.
Following commit will actually address the data path side.
Fixes: d5581966040f ("xsk: support ZC Tx multi-buffer in batch API")
Reviewed-by: Jason Xing <kernelxing@tencent.com>
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
include/net/xsk_buff_pool.h | 6 ++++--
net/xdp/xsk.c | 10 +++++++---
net/xdp/xsk_buff_pool.c | 12 ++++++++----
3 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/include/net/xsk_buff_pool.h b/include/net/xsk_buff_pool.h
index ccb3b350001f..f5e737a83055 100644
--- a/include/net/xsk_buff_pool.h
+++ b/include/net/xsk_buff_pool.h
@@ -102,12 +102,14 @@ struct xsk_buff_pool {
/* AF_XDP core. */
struct xsk_buff_pool *xp_create_and_assign_umem(struct xdp_sock *xs,
- struct xdp_umem *umem);
+ struct xdp_umem *umem,
+ u32 max_segs);
int xp_assign_dev(struct xsk_buff_pool *pool, struct net_device *dev,
u16 queue_id, u16 flags);
int xp_assign_dev_shared(struct xsk_buff_pool *pool, struct xdp_sock *umem_xs,
struct net_device *dev, u16 queue_id);
-int xp_alloc_tx_descs(struct xsk_buff_pool *pool, struct xdp_sock *xs);
+int xp_alloc_tx_descs(struct xsk_buff_pool *pool, struct xdp_sock *xs,
+ u32 max_segs);
void xp_destroy(struct xsk_buff_pool *pool);
void xp_get_pool(struct xsk_buff_pool *pool);
bool xp_put_pool(struct xsk_buff_pool *pool);
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 12a845d012f6..091792d1d82d 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -1525,7 +1525,8 @@ static int xsk_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr
* and/or device.
*/
xs->pool = xp_create_and_assign_umem(xs,
- umem_xs->umem);
+ umem_xs->umem,
+ dev->xdp_zc_max_segs);
if (!xs->pool) {
err = -ENOMEM;
sockfd_put(sock);
@@ -1557,7 +1558,8 @@ static int xsk_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr
* utilizes
*/
if (xs->tx && !xs->pool->tx_descs) {
- err = xp_alloc_tx_descs(xs->pool, xs);
+ err = xp_alloc_tx_descs(xs->pool, xs,
+ dev->xdp_zc_max_segs);
if (err) {
xp_put_pool(xs->pool);
xs->pool = NULL;
@@ -1575,7 +1577,9 @@ static int xsk_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr
goto out_unlock;
} else {
/* This xsk has its own umem. */
- xs->pool = xp_create_and_assign_umem(xs, xs->umem);
+ xs->pool = xp_create_and_assign_umem(xs, xs->umem,
+ dev->xdp_zc_max_segs);
+
if (!xs->pool) {
err = -ENOMEM;
goto out_unlock;
diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
index 1f28a9641571..12c9fb29af05 100644
--- a/net/xdp/xsk_buff_pool.c
+++ b/net/xdp/xsk_buff_pool.c
@@ -42,9 +42,12 @@ void xp_destroy(struct xsk_buff_pool *pool)
kvfree(pool);
}
-int xp_alloc_tx_descs(struct xsk_buff_pool *pool, struct xdp_sock *xs)
+int xp_alloc_tx_descs(struct xsk_buff_pool *pool, struct xdp_sock *xs,
+ u32 max_segs)
{
- pool->tx_descs = kvzalloc_objs(*pool->tx_descs, xs->tx->nentries);
+ u32 nentries = max(xs->tx->nentries, max_segs);
+
+ pool->tx_descs = kvzalloc_objs(*pool->tx_descs, nentries);
if (!pool->tx_descs)
return -ENOMEM;
@@ -52,7 +55,8 @@ int xp_alloc_tx_descs(struct xsk_buff_pool *pool, struct xdp_sock *xs)
}
struct xsk_buff_pool *xp_create_and_assign_umem(struct xdp_sock *xs,
- struct xdp_umem *umem)
+ struct xdp_umem *umem,
+ u32 max_segs)
{
bool unaligned = umem->flags & XDP_UMEM_UNALIGNED_CHUNK_FLAG;
struct xsk_buff_pool *pool;
@@ -69,7 +73,7 @@ struct xsk_buff_pool *xp_create_and_assign_umem(struct xdp_sock *xs,
goto out;
if (xs->tx)
- if (xp_alloc_tx_descs(pool, xs))
+ if (xp_alloc_tx_descs(pool, xs, max_segs))
goto out;
pool->chunk_mask = ~((u64)umem->chunk_size - 1);
--
2.43.0
^ permalink raw reply related
* [PATCH v4 net 2/6] xsk: drain continuation descs after overflow in xsk_build_skb()
From: Maciej Fijalkowski @ 2026-07-19 13:56 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms, bjorn,
kerneljasonxing, Jason Xing, Maciej Fijalkowski
In-Reply-To: <20260719135609.147823-1-maciej.fijalkowski@intel.com>
From: Jason Xing <kernelxing@tencent.com>
Fix generic xmit path multi-buffer logic when packets are either too big
(count of descriptors exceed MAX_SKB_FRAGS) or an invalid descriptor is
included in fragmented packet. Introduce xdp_sock::drain_cont and act
upon this flag - when it is set, keep on consuming descriptors from
AF_XDP Tx ring and put them directly onto Cq. Previously these
descriptors were silently lost and could never be reached again.
Fixes: cf24f5a5feea ("xsk: add support for AF_XDP multi-buffer on Tx path")
Closes: https://lore.kernel.org/all/20260425041726.85FB3C2BCB2@smtp.kernel.org/
Reviewed-by: Jason Xing <kernelxing@tencent.com>
Co-developed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> # wrapped cq addr submission onto routine
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
include/net/xdp_sock.h | 1 +
net/xdp/xsk.c | 45 +++++++++++++++++++++++++++++++++++++++---
2 files changed, 43 insertions(+), 3 deletions(-)
diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
index ebac60a3d8a1..8b51876efbed 100644
--- a/include/net/xdp_sock.h
+++ b/include/net/xdp_sock.h
@@ -80,6 +80,7 @@ struct xdp_sock {
* call of __xsk_generic_xmit().
*/
struct sk_buff *skb;
+ bool drain_cont;
struct list_head map_list;
/* Protects map_list */
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index a7a83dc4546a..12a845d012f6 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -737,6 +737,19 @@ static void xsk_cq_submit_addr_locked(struct xsk_buff_pool *pool,
spin_unlock_irqrestore(&pool->cq_prod_lock, flags);
}
+static void xsk_cq_submit_addr_single_locked(struct xsk_buff_pool *pool,
+ struct xdp_desc *desc)
+{
+ unsigned long flags;
+ u32 idx;
+
+ spin_lock_irqsave(&pool->cq_prod_lock, flags);
+ idx = xskq_get_prod(pool->cq);
+ xskq_prod_write_addr(pool->cq, idx, desc->addr);
+ xskq_prod_submit_n(pool->cq, 1);
+ spin_unlock_irqrestore(&pool->cq_prod_lock, flags);
+}
+
static void xsk_cq_cancel_locked(struct xsk_buff_pool *pool, u32 n)
{
spin_lock(&pool->cq->cq_cached_prod_lock);
@@ -1028,13 +1041,14 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
static int __xsk_generic_xmit(struct sock *sk)
{
struct xdp_sock *xs = xdp_sk(sk);
- bool sent_frame = false;
struct xdp_desc desc;
struct sk_buff *skb;
+ u32 cached_cons;
u32 max_batch;
int err = 0;
mutex_lock(&xs->mutex);
+ cached_cons = xs->tx->cached_cons;
/* Since we dropped the RCU read lock, the socket state might have changed. */
if (unlikely(!xsk_is_bound(xs))) {
@@ -1063,11 +1077,21 @@ static int __xsk_generic_xmit(struct sock *sk)
goto out;
}
+ if (unlikely(xs->drain_cont)) {
+ xsk_cq_submit_addr_single_locked(xs->pool, &desc);
+ xs->tx->invalid_descs++;
+ xskq_cons_release(xs->tx);
+ xs->drain_cont = xp_mb_desc(&desc);
+ continue;
+ }
+
skb = xsk_build_skb(xs, &desc);
if (IS_ERR(skb)) {
err = PTR_ERR(skb);
if (err != -EOVERFLOW)
goto out;
+ if (xp_mb_desc(&desc))
+ xs->drain_cont = true;
err = 0;
continue;
}
@@ -1096,18 +1120,33 @@ static int __xsk_generic_xmit(struct sock *sk)
goto out;
}
- sent_frame = true;
xs->skb = NULL;
}
if (xskq_has_descs(xs->tx)) {
+ bool drain = xs->skb || xs->drain_cont || xp_mb_desc(&desc);
+
+ err = xsk_cq_reserve_locked(xs->pool);
+ if (err) {
+ xs->tx->invalid_descs--;
+ if (xs->skb)
+ xsk_drop_skb(xs->skb);
+ xs->drain_cont = drain;
+ err = -EAGAIN;
+ goto out;
+ }
+
if (xs->skb)
xsk_drop_skb(xs->skb);
+
+ xsk_cq_submit_addr_single_locked(xs->pool, &desc);
+
xskq_cons_release(xs->tx);
+ xs->drain_cont = xp_mb_desc(&desc);
}
out:
- if (sent_frame)
+ if (xs->tx->cached_cons != cached_cons)
__xsk_tx_release(xs);
mutex_unlock(&xs->mutex);
--
2.43.0
^ permalink raw reply related
* [PATCH v4 net 1/6] xsk: fix buffer leak in xsk_drop_skb() for AF_XDP multi-buffer Tx
From: Maciej Fijalkowski @ 2026-07-19 13:56 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms, bjorn,
kerneljasonxing, Jason Xing, Maciej Fijalkowski
In-Reply-To: <20260719135609.147823-1-maciej.fijalkowski@intel.com>
From: Jason Xing <kernelxing@tencent.com>
This patch is inspired by the check[1] from sashiko. It says when
overflow happens, the address of cq to be published is invalid.
Actually the severer thing is the whole process of publishing the
address of cq in this particular case is not right: it should truely
publish the address and advance the cached_prod in cq as long as it
reads descriptors from txq.
The following is the full analysis.
xsk_drop_skb() is called in three places, which all discard a partially
built multi-buffer skb:
1) xsk_build_skb() -EOVERFLOW error path: packet exceeds MAX_SKB_FRAGS
2) __xsk_generic_xmit() post-loop cleanup: an invalid descriptor in
the TX ring prevents the partial packet from completing
3) xsk_release(): socket close while xs->skb holds an incomplete packet
In all three cases, the TX descriptors for the already-processed frags
have been consumed from the TX ring (xskq_cons_release), and CQ slots
have been reserved. However, xsk_drop_skb() calls xsk_consume_skb()
which cancels the CQ reservations via xsk_cq_cancel_locked(). Since
the buffer addresses never appear in the completion queue, userspace
permanently loses track of these buffers.
Fix this by letting consume_skb() trigger the existing xsk_destruct_skb
destructor, which already submits buffer addresses to the CQ via
xsk_cq_submit_addr_locked().
Note that cancelling the descriptors back to the TX ring (via
xskq_cons_cancel_n) is not a appropriate option because an oversized
packet that always exceeds MAX_SKB_FRAGS would be retried indefinitely,
which is an obviously deadlock bug in the TX path.
Also move the desc->addr assignment in xsk_build_skb() above the
overflow check so that the current descriptor's address is recorded
before a potential -EOVERFLOW jump to free_err, consistent with the
zerocopy path in xsk_build_skb_zerocopy().
[1]: https://lore.kernel.org/all/20260425041726.85FB3C2BCB2@smtp.kernel.org/
Fixes: cf24f5a5feea ("xsk: add support for AF_XDP multi-buffer on Tx path")
Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
net/xdp/xsk.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index b970f30ea9b9..a7a83dc4546a 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -794,8 +794,11 @@ static void xsk_consume_skb(struct sk_buff *skb)
static void xsk_drop_skb(struct sk_buff *skb)
{
- xdp_sk(skb->sk)->tx->invalid_descs += xsk_get_num_desc(skb);
- xsk_consume_skb(skb);
+ struct xdp_sock *xs = xdp_sk(skb->sk);
+
+ xs->tx->invalid_descs += xsk_get_num_desc(skb);
+ consume_skb(skb);
+ xs->skb = NULL;
}
static int xsk_skb_metadata(struct sk_buff *skb, void *buffer,
@@ -877,7 +880,7 @@ static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs,
return ERR_PTR(-ENOMEM);
/* in case of -EOVERFLOW that could happen below,
- * xsk_consume_skb() will release this node as whole skb
+ * xsk_drop_skb() will release this node as whole skb
* would be dropped, which implies freeing all list elements
*/
xsk_addr->addrs[xsk_addr->num_descs] = desc->addr;
@@ -969,6 +972,8 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
goto free_err;
}
+ xsk_addr->addrs[xsk_addr->num_descs] = desc->addr;
+
if (unlikely(nr_frags == (MAX_SKB_FRAGS - 1) && xp_mb_desc(desc))) {
err = -EOVERFLOW;
goto free_err;
@@ -986,8 +991,6 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
skb_add_rx_frag(skb, nr_frags, page, 0, len, PAGE_SIZE);
refcount_add(PAGE_SIZE, &xs->sk.sk_wmem_alloc);
-
- xsk_addr->addrs[xsk_addr->num_descs] = desc->addr;
}
}
--
2.43.0
^ permalink raw reply related
* [PATCH v4 net 0/6] xsk: fix AF_XDP multi-buffer Tx descriptor reclaim
From: Maciej Fijalkowski @ 2026-07-19 13:56 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms, bjorn,
kerneljasonxing, Maciej Fijalkowski
v3:
https://lore.kernel.org/netdev/20260714140722.111645-1-maciej.fijalkowski@intel.com/T/
v3->v4:
* Return standalone invalid Tx descriptors through the completion ring in
both the generic and zero-copy Tx paths. Advancing the Tx-ring consumer
releases only the ring slot; returning the descriptor address through
the CQ also transfers ownership of the corresponding UMEM frame back to
userspace.
* Remove xsk_tx_batch::consumed_descs. With standalone invalid descriptors
now reclaimed through the CQ, every descriptor permanently removed from
the Tx ring is represented by either tx_descs or reclaim_descs. Use
their sum for Tx-consumer advancement, shared-UMEM fairness accounting,
and progress detection.
* Ensure that generic reclaim-only processing publishes the updated
Tx-ring consumer even when no packet was submitted to the networking
stack.
* Update the XSK selftests to count every descriptor submitted to the Tx
ring as an expected CQ entry, while continuing to count only valid
packets as expected Rx traffic. This covers standalone invalid
descriptors, invalid multi-buffer packets, oversized packets, and the
non-verbatim STAT_TX_INVALID tests.
* Update the AF_XDP documentation to describe the completion ring as an
ownership-transfer mechanism and document that standalone, invalid
multi-buffer, and oversized Tx packets are reclaimed through the CQ.
* Add Jason's tags
v2: https://lore.kernel.org/netdev/20260710194424.84844-1-maciej.fijalkowski@intel.com/
v2->v3:
* Added a preceding patch that sizes the pool-wide temporary Tx descriptor
array to the larger of the first Tx ring and the device's
xdp_zc_max_segs capability. This guarantees that the shared-UMEM path can
inspect one maximum-sized valid packet even when the socket that creates
the pool has a smaller Tx ring. Consequently, this patch now records the
actual allocated size in pool->tx_descs_nentries rather than the first
socket's Tx ring size.
* Fixed a possible infinite retry loop when a shared-UMEM socket contains
an incomplete multi-buffer packet. Pass the original descriptor budget
to xskq_cons_read_desc_batch() instead of the number of descriptors
currently available, so the parser can distinguish producer exhaustion
from actual budget exhaustion.
* Moved the completion-ring space check to the common batched Tx path, so
it is performed exactly once for both singular and shared-SG pools. Keep
the legacy shared non-SG fallback outside this handling, as it reserves
completion entries one descriptor at a time.
* Reworked xsk_tx_peek_release_desc_batch() to use a common singular/shared
batched flow. Resolve an empty Tx socket list before checking CQ space,
select the shared-SG walker through an explicit shared-pool condition,
and commit the resulting batch through one common path.
* Simplified xsk_tx_commit_batch() by moving the cached CQ producer
snapshot into the helper instead of passing it from each caller.
v1: https://lore.kernel.org/netdev/20260623133240.1048434-1-maciej.fijalkowski@intel.com/
v1->v2:
* Reduced the series from seven to five patches by squashing the three
generic Tx drain and reclaim changes into a single patch. The resulting
patch handles overflow, invalid descriptors in the middle of a packet,
and reclaim of the offending descriptor as one coherent change. This
so it will be less likely to have things reported by Sashiko that are
fixed in later commits;
* Reworked the zero-copy implementation substantially:
* removed the bind-transition mechanism, including tx_share_pending,
xp_prepare_xsk_tx_share(), xp_finish_xsk_tx_share(),
synchronize_net(), and the transient bind() -EAGAIN behavior;
* added packet-framed parsing for shared-UMEM SG pools, allowing
per-socket drain state to be resumed by both singular and shared Tx
paths;
* retained the legacy one-descriptor fallback for shared non-SG pools;
* preserved the existing per-socket fairness quota while allowing the
shared walker to consume multiple complete packets and continue
filling the requested batch across fairness rounds;
* made the fairness quota large enough to process one maximum-sized
valid multi-buffer packet;
* extended the parser result with consumed-descriptor and
budget-limited accounting needed by the shared walker;
* recorded the size of the pool's temporary Tx descriptor array and
capped batch processing at that size;
* kept reclaim-only descriptors ordered after preceding driver-visible
descriptors and protected the delayed-reclaim state with
READ_ONCE()/WRITE_ONCE().
* Rewrote the zero-copy patch description to cover oversized packets,
continuation draining across calls, shared-UMEM SG handling, and CQ
publication ordering.
* Corrected the too-many-frags selftest description to state that the
invalid packet contains max_frags + 1 fragments and terminates at an
explicit packet boundary.
Hi,
This series fixes several AF_XDP multi-buffer Tx paths where descriptors
consumed from the Tx ring are not consistently returned to userspace
through the completion ring when the packet is later dropped as invalid.
The affected cases are invalid or oversized multi-buffer Tx packets in
both the generic and zero-copy paths. In these cases, the kernel can
consume one or more Tx descriptors while building or validating a
multi-buffer packet, then drop the packet before it reaches the device.
Userspace still owns the UMEM buffers only after the corresponding
addresses are returned through the CQ. Missing completions therefore
make userspace lose track of those buffers.
The generic path fixes cover following related cases:
* partially built multi-buffer skbs dropped by xsk_drop_skb();
continuation descriptors left in the Tx ring after xsk_build_skb()
reports overflow;
* invalid descriptors encountered in the middle of a multi-buffer
packet, including the offending invalid descriptor itself.
The zero-copy path is handled separately. The batched Tx parser now
distinguishes descriptors that can be passed to the driver from
descriptors that are consumed only because they belong to an invalid
multi-buffer packet. Reclaim-only descriptors are written to the CQ
address area and published in completion order, after any earlier
driver-visible Tx descriptors.
The last two patches update xskxceiver so the tests account invalid
multi-buffer Tx packets as descriptors that must be reclaimed, while
still not expecting those invalid packets on the Rx side.
This is a follow-up to Jason's changes [0] which were addressing generic
xmit only and this set allows me to pass full xskxceiver test suite run
against ice driver.
Thanks,
Maciej
[0]: https://lore.kernel.org/netdev/20260520004244.55663-1-kerneljasonxing@gmail.com/
Jason Xing (2):
xsk: fix buffer leak in xsk_drop_skb() for AF_XDP multi-buffer Tx
xsk: drain continuation descs after overflow in xsk_build_skb()
Maciej Fijalkowski (4):
xsk: provide sufficient space in pool->tx_descs
xsk: reclaim invalid Tx descriptors in ZC batch path
selftests/xsk: fix too-many-frags multi-buffer Tx test
selftests/xsk: account reclaimed invalid Tx descriptors
Documentation/networking/af_xdp.rst | 54 ++--
include/net/xdp_sock.h | 1 +
include/net/xsk_buff_pool.h | 9 +-
net/xdp/xsk.c | 255 +++++++++++++++---
net/xdp/xsk_buff_pool.c | 13 +-
net/xdp/xsk_queue.h | 65 +++--
.../selftests/bpf/prog_tests/test_xsk.c | 50 ++--
7 files changed, 347 insertions(+), 100 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH net] nfc: nci: fix out-of-bounds read in activation parameter parsing
From: David Heidelberg @ 2026-07-19 13:54 UTC (permalink / raw)
To: Muhammad Bilal, netdev
Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, oe-linux-nfc, linux-kernel
In-Reply-To: <20260628210026.129941-1-meatuni001@gmail.com>
On 28/06/2026 23:00, Muhammad Bilal wrote:
> nci_extract_activation_params_iso_dep() and
> nci_extract_activation_params_nfc_dep() receive a pointer into the
> RF_INTF_ACTIVATED_NTF notification but are not told how many bytes
> remain. Each reads a one-byte length field (rats_res_len,
> attrib_res_len, atr_res_len or atr_req_len) and then memcpy()s that many
> bytes from the packet. The length is clamped to the destination size,
> but it is never checked against the remaining activation-parameter data,
> so a notification whose length field is larger than the data present
> reads past the end of the buffer.
>
> The sibling nci_extract_rf_params_*() helpers were recently given a
> data_len argument and matching remaining-length checks, but the
> activation-parameter helpers were not updated.
>
> Pass the remaining length down and validate each field against it before
> copying, as the rf_params helpers do.
>
> Fixes: ac2068384034 ("NFC: Parse NCI NFC-DEP activation params")
> Cc: stable@vger.kernel.org
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> ---
> net/nfc/nci/ntf.c | 36 ++++++++++++++++++++++++++++++------
> 1 file changed, 30 insertions(+), 6 deletions(-)
>
Hello Muhammad.
This issue got (partially?) addressed by
https://lore.kernel.org/all/20260612-b4-disp-6d52d8b0-v3-1-e26221f8826d@proton.me/
If you feel there is still something to improve, feel free to send patch against
the for-linus branch where it's already applied.
Thank you
David
^ permalink raw reply
* Re: [PATCH] nfc: llcp: Fix nfc_dev refcount leak in connect
From: David Heidelberg @ 2026-07-19 13:44 UTC (permalink / raw)
To: Shuangpeng Bai, oe-linux-nfc
Cc: davem, edumazet, kuba, pabeni, horms, netdev, linux-kernel
In-Reply-To: <20260707183518.1888697-1-shuangpeng.kernel@gmail.com>
On 07/07/2026 20:35, Shuangpeng Bai wrote:
> llcp_sock_connect() takes a reference to the NFC device with
> nfc_get_device() and stores it in llcp_sock->dev before moving the
> socket to LLCP_CONNECTING.
>
> If such a pending connection is released before reaching
> LLCP_CONNECTED, llcp_sock_destruct() does not drop that device
> reference because it only handles connected sockets. This leaks the
> nfc_dev reference acquired during connect.
>
> Drop the device reference for LLCP_CONNECTING sockets as well.
>
> Signed-off-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
> ---
> net/nfc/llcp_sock.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
Hello Shuangpeng,
could you check the Sashiko review, if you can address the issue found within
the patch (the existing code issues are optional to address)?
--- Patch [1]: [PATCH] nfc: llcp: Fix nfc_dev refcount leak in connect ---
commit 84e092575501f812e707c62695cef1ec7ff9d8a6
Author: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
nfc: llcp: Fix nfc_dev refcount leak in connect
llcp_sock_connect() takes a reference to the NFC device with
nfc_get_device() and stores it in llcp_sock->dev before moving the
socket to LLCP_CONNECTING. If such a pending connection is released
before reaching LLCP_CONNECTED, llcp_sock_destruct() does not drop that
device reference because it only handles connected sockets. This leaks the
nfc_dev reference acquired during connect. Drop the device reference for
LLCP_CONNECTING sockets as well.
> diff --git a/net/nfc/llcp_sock.c b/net/nfc/llcp_sock.c
> index feab29fc62f44..44639ccd438e4 100644
> --- a/net/nfc/llcp_sock.c
> +++ b/net/nfc/llcp_sock.c
> @@ -960,7 +960,8 @@ static void llcp_sock_destruct(struct sock *sk)
>
> pr_debug("%p\n", sk);
>
> - if (sk->sk_state == LLCP_CONNECTED)
> + if (sk->sk_state == LLCP_CONNECTED ||
> + sk->sk_state == LLCP_CONNECTING)
This isn't a bug introduced by this patch, but does llcp_sock_connect()
lack a state check to prevent it from being called on sockets in LLCP_BOUND
or LLCP_LISTEN states?
If a user creates an AF_NFC socket and calls bind(), placing the socket in
the LLCP_BOUND state, it links into the local->sockets list via its sk_node.
If the user then calls connect(), which only checks for LLCP_CONNECTED and
LLCP_CONNECTING states:
net/nfc/llcp_sock.c:llcp_sock_connect() {
if (sk->sk_state == LLCP_CONNECTED) {
ret = -EISCONN;
goto error;
}
if (sk->sk_state == LLCP_CONNECTING) {
ret = -EINPROGRESS;
goto error;
}
...
}
It proceeds to overwrite llcp_sock->service_name (leaking the previously
allocated string) and calls nfc_llcp_sock_link(&local->connecting_sockets,
sk).
This executes hlist_add_head() on a node (sk->sk_node) that is already part
of another list, thereby corrupting the pointers of both the sockets and
connecting_sockets lists.
> + nfc_put_device(llcp_sock->dev);
Can this introduce a kernel panic via a NULL pointer dereference when a
blocking connect() call fails or is interrupted?
In llcp_sock_connect(), if the wait fails or is interrupted by a signal,
execution jumps to the sock_unlink error path:
net/nfc/llcp_sock.c:llcp_sock_connect() {
sk->sk_state = LLCP_CONNECTING;
ret = sock_wait_state(sk, LLCP_CONNECTED,
sock_sndtimeo(sk, flags & O_NONBLOCK));
if (ret && ret != -EINPROGRESS)
goto sock_unlink;
...
sock_llcp_nullify:
llcp_sock->local = NULL;
llcp_sock->dev = NULL;
}
This error path nullifies llcp_sock->dev and drops the device reference,
but crucially fails to reset sk->sk_state back to LLCP_CLOSED.
When the user subsequently closes the socket, llcp_sock_destruct() is
invoked. The newly added logic checks for LLCP_CONNECTING sockets and
unconditionally calls nfc_put_device(llcp_sock->dev).
Since dev is NULL, nfc_put_device() evaluates &NULL->dev:
include/net/nfc/nfc.h:nfc_put_device() {
put_device(&dev->dev);
}
This evaluates to a non-zero struct offset and bypasses the internal NULL
check in put_device(), attempting to decrement the reference count of
unmapped memory.
>
> skb_queue_purge(&sk->sk_receive_queue);
>
^ permalink raw reply
* [PATCH net-next 5/5] bridge: Use ndisc_parse_options() to parse ND options in br_nd_send()
From: Danielle Ratson @ 2026-07-19 13:34 UTC (permalink / raw)
To: netdev
Cc: dsahern, idosch, davem, edumazet, kuba, pabeni, horms, razor, ja,
petrm, fw, kuniyu, bridge, linux-kernel, Danielle Ratson
In-Reply-To: <cover.1784463131.git.danieller@nvidia.com>
Replace the manual ND option parsing loop in br_nd_send() with
ndisc_parse_options(), which provides proper validation and avoids the
class of bugs that were fixed by commit 53fc685243bd ("bridge: Avoid
infinite loop when suppressing NS messages with invalid options") and
commit 850837965af1 ("bridge: br_nd_send: validate ND option lengths").
Use ndisc_opt_addr_data() to extract the source link-layer address
from the parsed options, which correctly validates the option length
for the underlying device type.
Export ndisc_parse_options() so that it can be resolved from the bridge
when it is built as a module (CONFIG_BRIDGE=m); otherwise modpost fails
with an undefined symbol.
Reviewed-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Danielle Ratson <danieller@nvidia.com>
---
net/bridge/br_arp_nd_proxy.c | 32 +++++++++++++++++---------------
net/ipv6/ndisc.c | 1 +
2 files changed, 18 insertions(+), 15 deletions(-)
diff --git a/net/bridge/br_arp_nd_proxy.c b/net/bridge/br_arp_nd_proxy.c
index 46779d9fad61..7863beae0c55 100644
--- a/net/bridge/br_arp_nd_proxy.c
+++ b/net/bridge/br_arp_nd_proxy.c
@@ -252,15 +252,16 @@ static void br_nd_send(struct net_bridge *br, struct net_bridge_port *p,
{
struct net_device *dev = request->dev;
struct net_bridge_vlan_group *vg;
+ struct ndisc_options ndopts;
struct nd_msg *na, *ns;
struct sk_buff *reply;
struct ipv6hdr *pip6;
int na_olen = 8; /* opt hdr + ETH_ALEN for target */
int ns_olen;
- int i, len;
u8 *daddr;
bool dad;
u16 pvid;
+ int len;
if (!dev)
return;
@@ -281,20 +282,21 @@ static void br_nd_send(struct net_bridge *br, struct net_bridge_port *p,
daddr = eth_hdr(request)->h_source;
ns = (struct nd_msg *)skb_transport_header(request);
- /* Do we need option processing ? */
- ns_olen = request->len - (skb_network_offset(request) +
- sizeof(struct ipv6hdr)) - sizeof(*ns);
- for (i = 0; i < ns_olen - 1; i += (ns->opt[i + 1] << 3)) {
- if (!ns->opt[i + 1] || i + (ns->opt[i + 1] << 3) > ns_olen) {
- kfree_skb(reply);
- return;
- }
- if (ns->opt[i] == ND_OPT_SOURCE_LL_ADDR) {
- if ((ns->opt[i + 1] << 3) >=
- sizeof(struct nd_opt_hdr) + ETH_ALEN)
- daddr = ns->opt + i + sizeof(struct nd_opt_hdr);
- break;
- }
+ /* Derive the option length from the IPv6 payload length so that any
+ * trailing L2 padding in the skb is not parsed as ND options.
+ */
+ ns_olen = ntohs(ipv6_hdr(request)->payload_len) - sizeof(*ns);
+ if (!ndisc_parse_options(dev, ns->opt, ns_olen, &ndopts)) {
+ kfree_skb(reply);
+ return;
+ }
+
+ if (ndopts.nd_opts_src_lladdr) {
+ u8 *lladdr;
+
+ lladdr = ndisc_opt_addr_data(ndopts.nd_opts_src_lladdr, dev);
+ if (lladdr)
+ daddr = lladdr;
}
dad = ipv6_addr_any(&ipv6_hdr(request)->saddr);
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index f867ec8d3d90..d8b4588f6bff 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -283,6 +283,7 @@ struct ndisc_options *ndisc_parse_options(const struct net_device *dev,
}
return ndopts;
}
+EXPORT_SYMBOL(ndisc_parse_options);
int ndisc_mc_map(const struct in6_addr *addr, char *buf, struct net_device *dev, int dir)
{
--
2.54.0
^ permalink raw reply related
* [PATCH net-next 4/5] bridge: Linearize skb once the ND message type is validated
From: Danielle Ratson @ 2026-07-19 13:34 UTC (permalink / raw)
To: netdev
Cc: dsahern, idosch, davem, edumazet, kuba, pabeni, horms, razor, ja,
petrm, fw, kuniyu, bridge, linux-kernel, Danielle Ratson
In-Reply-To: <cover.1784463131.git.danieller@nvidia.com>
br_nd_send() parses ND options from ns->opt[] and therefore needs the skb
to be linear. Commit a01aee7cafc5 ("bridge: br_nd_send: linearize skb
before parsing ND options") ensured that by linearizing inside
br_nd_send() itself.
Move the linearization up into br_is_nd_neigh_msg(), right after
ndisc_check_ns_na() has validated the message as an NS/NA. This makes a
linear buffer a property of every recognized ND message, so that this and
any future ND message handling operate on a linear skb and cannot
reintroduce that class of bug by forgetting to linearize.
Since the skb is now linear by the time br_nd_send() runs, drop the
linearization there and derive ns from the transport header set by
ndisc_check_ns_na(), instead of recomputing it from the network header.
If linearization fails under memory pressure, br_is_nd_neigh_msg() returns
NULL and the packet falls back to normal forwarding rather than being
suppressed.
Reviewed-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Danielle Ratson <danieller@nvidia.com>
---
net/bridge/br_arp_nd_proxy.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/net/bridge/br_arp_nd_proxy.c b/net/bridge/br_arp_nd_proxy.c
index 445c930ed59b..46779d9fad61 100644
--- a/net/bridge/br_arp_nd_proxy.c
+++ b/net/bridge/br_arp_nd_proxy.c
@@ -240,6 +240,9 @@ struct nd_msg *br_is_nd_neigh_msg(struct sk_buff *skb)
if (ndisc_check_ns_na(skb))
return NULL;
+ if (skb_linearize(skb))
+ return NULL;
+
return (struct nd_msg *)skb_transport_header(skb);
}
@@ -259,7 +262,7 @@ static void br_nd_send(struct net_bridge *br, struct net_bridge_port *p,
bool dad;
u16 pvid;
- if (!dev || skb_linearize(request))
+ if (!dev)
return;
len = LL_RESERVED_SPACE(dev) + sizeof(struct ipv6hdr) +
@@ -276,8 +279,7 @@ static void br_nd_send(struct net_bridge *br, struct net_bridge_port *p,
skb_set_mac_header(reply, 0);
daddr = eth_hdr(request)->h_source;
- ns = (struct nd_msg *)(skb_network_header(request) +
- sizeof(struct ipv6hdr));
+ ns = (struct nd_msg *)skb_transport_header(request);
/* Do we need option processing ? */
ns_olen = request->len - (skb_network_offset(request) +
--
2.54.0
^ permalink raw reply related
* [PATCH net-next 3/5] bridge: Validate NS/NA messages using ndisc_check_ns_na()
From: Danielle Ratson @ 2026-07-19 13:34 UTC (permalink / raw)
To: netdev
Cc: dsahern, idosch, davem, edumazet, kuba, pabeni, horms, razor, ja,
petrm, fw, kuniyu, bridge, linux-kernel, Danielle Ratson
In-Reply-To: <cover.1784463131.git.danieller@nvidia.com>
The bridge performs neighbor suppression by snooping NS/NA messages, but
previously only checked the ICMPv6 type and code. This leaves it open to
acting on malformed or spoofed packets that any RFC-compliant node should
reject.
Wire br_is_nd_neigh_msg() into the new ndisc_check_ns_na() helper, which
enforces the full RFC 4861 section 7.1.1/7.1.2 receive validation:
hop limit of 255, valid checksum, correct code, and type-specific rules
(NS target not multicast; NA solicited flag clear for multicast
destinations).
MLD messages are already validated by ipv6_mc_check_mld() before the
bridge acts on them; this brings NS/NA to the same standard.
As a side effect, the skb parameter of br_is_nd_neigh_msg() changes from
const to non-const, since ndisc_check_ns_na() may reallocate the skb head
via pskb_may_pull() and sets the transport header. The returned pointer is
now derived from skb_transport_header() rather than a direct cast.
Reviewed-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Danielle Ratson <danieller@nvidia.com>
---
net/bridge/br_arp_nd_proxy.c | 11 ++++-------
net/bridge/br_private.h | 2 +-
2 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/net/bridge/br_arp_nd_proxy.c b/net/bridge/br_arp_nd_proxy.c
index db08c3272001..445c930ed59b 100644
--- a/net/bridge/br_arp_nd_proxy.c
+++ b/net/bridge/br_arp_nd_proxy.c
@@ -19,6 +19,7 @@
#include <net/addrconf.h>
#if IS_ENABLED(CONFIG_IPV6)
#include <net/ip6_checksum.h>
+#include <net/ndisc.h>
#endif
#include "br_private.h"
@@ -234,16 +235,12 @@ void br_do_proxy_suppress_arp(struct sk_buff *skb, struct net_bridge *br,
#endif
#if IS_ENABLED(CONFIG_IPV6)
-struct nd_msg *br_is_nd_neigh_msg(const struct sk_buff *skb)
+struct nd_msg *br_is_nd_neigh_msg(struct sk_buff *skb)
{
- struct nd_msg *m = (struct nd_msg *)(ipv6_hdr(skb) + 1);
-
- if (m->icmph.icmp6_code != 0 ||
- (m->icmph.icmp6_type != NDISC_NEIGHBOUR_SOLICITATION &&
- m->icmph.icmp6_type != NDISC_NEIGHBOUR_ADVERTISEMENT))
+ if (ndisc_check_ns_na(skb))
return NULL;
- return m;
+ return (struct nd_msg *)skb_transport_header(skb);
}
static void br_nd_send(struct net_bridge *br, struct net_bridge_port *p,
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index a51246747537..81b807487dcc 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -2365,7 +2365,7 @@ void br_do_proxy_suppress_arp(struct sk_buff *skb, struct net_bridge *br,
u16 vid, struct net_bridge_port *p);
void br_do_suppress_nd(struct sk_buff *skb, struct net_bridge *br,
u16 vid, struct net_bridge_port *p, struct nd_msg *msg);
-struct nd_msg *br_is_nd_neigh_msg(const struct sk_buff *skb);
+struct nd_msg *br_is_nd_neigh_msg(struct sk_buff *skb);
bool br_is_neigh_suppress_enabled(const struct net_bridge_port *p, u16 vid);
bool br_is_neigh_forward_grat_enabled(const struct net_bridge_port *p, u16 vid);
#endif
--
2.54.0
^ permalink raw reply related
* [PATCH net-next 2/5] ipv6: ndisc: Add ndisc_check_ns_na() validation helper
From: Danielle Ratson @ 2026-07-19 13:34 UTC (permalink / raw)
To: netdev
Cc: dsahern, idosch, davem, edumazet, kuba, pabeni, horms, razor, ja,
petrm, fw, kuniyu, bridge, linux-kernel, Danielle Ratson
In-Reply-To: <cover.1784463131.git.danieller@nvidia.com>
Add ndisc_check_ns_na(), a standalone NS/NA packet validator modeled
after ipv6_mc_check_mld(). It performs the RFC 4861 section 7.1.1
(Neighbor Solicitation) and 7.1.2 (Neighbor Advertisement) mandatory
checks that are relevant for software operating at the bridge level,
where packets bypass the normal IPv6 stack path:
- Hop Limit must be 255 (packet was not forwarded by a router)
- ICMPv6 checksum is valid
- ICMP Code is 0
- ICMP length is at least 24 octets (sizeof(struct nd_msg))
- Target Address must not be a multicast address
- All included options have a length that is greater than zero
- NS/DAD: destination must be a solicited-node multicast address
- NS/DAD: no Source Link-Layer Address option when source is unspecified
- NA: Solicited flag must be 0 when IP Destination is multicast
On success the function sets the skb transport header and returns 0,
matching the convention of ipv6_mc_check_mld().
Reviewed-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Danielle Ratson <danieller@nvidia.com>
---
include/net/ndisc.h | 2 +
net/ipv6/Makefile | 2 +-
net/ipv6/ndisc_snoop.c | 190 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 193 insertions(+), 1 deletion(-)
create mode 100644 net/ipv6/ndisc_snoop.c
diff --git a/include/net/ndisc.h b/include/net/ndisc.h
index 3da1a6f8d3f9..9e5379ad2d8e 100644
--- a/include/net/ndisc.h
+++ b/include/net/ndisc.h
@@ -430,6 +430,8 @@ void ndisc_update(const struct net_device *dev, struct neighbour *neigh,
const u8 *lladdr, u8 new, u32 flags, u8 icmp6_type,
struct ndisc_options *ndopts);
+int ndisc_check_ns_na(struct sk_buff *skb);
+
/*
* IGMP
*/
diff --git a/net/ipv6/Makefile b/net/ipv6/Makefile
index 5b0cd6488021..cf5e01f83ce3 100644
--- a/net/ipv6/Makefile
+++ b/net/ipv6/Makefile
@@ -51,7 +51,7 @@ obj-$(subst m,y,$(CONFIG_IPV6)) += inet6_hashtables.o
ifneq ($(CONFIG_IPV6),)
obj-$(CONFIG_NET_UDP_TUNNEL) += ip6_udp_tunnel.o
-obj-y += mcast_snoop.o
+obj-y += mcast_snoop.o ndisc_snoop.o
obj-$(CONFIG_TCP_AO) += tcp_ao.o
endif
diff --git a/net/ipv6/ndisc_snoop.c b/net/ipv6/ndisc_snoop.c
new file mode 100644
index 000000000000..a93eed27a1f6
--- /dev/null
+++ b/net/ipv6/ndisc_snoop.c
@@ -0,0 +1,190 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/skbuff.h>
+#include <net/addrconf.h>
+#include <net/ip6_checksum.h>
+#include <net/ipv6.h>
+#include <net/ndisc.h>
+
+static int ndisc_check_ip6hdr(struct sk_buff *skb)
+{
+ const struct ipv6hdr *ip6h;
+ unsigned int offset, len;
+
+ offset = skb_network_offset(skb) + sizeof(*ip6h);
+ if (!pskb_may_pull(skb, offset))
+ return -EINVAL;
+
+ ip6h = ipv6_hdr(skb);
+
+ if (ip6h->version != 6)
+ return -EINVAL;
+
+ if (ip6h->nexthdr != IPPROTO_ICMPV6)
+ return -ENOMSG;
+
+ /* RFC 4861 7.1.1 / 7.1.2: must not have been forwarded by a router */
+ if (ip6h->hop_limit != 255)
+ return -EINVAL;
+
+ len = offset + ntohs(ip6h->payload_len);
+ if (skb->len < len || len <= offset)
+ return -EINVAL;
+
+ skb_set_transport_header(skb, offset);
+
+ return 0;
+}
+
+static __sum16 ndisc_validate_checksum(struct sk_buff *skb)
+{
+ return skb_checksum_validate(skb, IPPROTO_ICMPV6, ip6_compute_pseudo);
+}
+
+static int ndisc_check_icmpv6(struct sk_buff *skb)
+{
+ unsigned int len = skb_transport_offset(skb) + sizeof(struct icmp6hdr);
+ unsigned int transport_len = ipv6_transport_len(skb);
+ struct sk_buff *skb_chk;
+ struct icmp6hdr *hdr;
+
+ if (!pskb_may_pull(skb, len))
+ return -EINVAL;
+
+ /* RFC 4861 7.1.1 / 7.1.2: the ICMPv6 checksum must be valid */
+ skb_chk = skb_checksum_trimmed(skb, transport_len,
+ ndisc_validate_checksum);
+ if (!skb_chk)
+ return -EINVAL;
+
+ if (skb_chk != skb)
+ kfree_skb(skb_chk);
+
+ /* RFC 4861 7.1.1 / 7.1.2: Code must be 0 */
+ hdr = (struct icmp6hdr *)skb_transport_header(skb);
+ if (hdr->icmp6_code != 0)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int ndisc_check_options(struct sk_buff *skb, unsigned int opts_len,
+ bool reject_slla)
+{
+ unsigned int offset = skb_transport_offset(skb) + sizeof(struct nd_msg);
+ struct nd_opt_hdr *opt, _opt;
+
+ while (opts_len > 0) {
+ if (opts_len < sizeof(*opt))
+ return -EINVAL;
+
+ opt = skb_header_pointer(skb, offset, sizeof(_opt), &_opt);
+ if (!opt)
+ return -EINVAL;
+
+ /* RFC 4861 7.1.1 / 7.1.2: all option lengths must be > 0 */
+ if (!opt->nd_opt_len)
+ return -EINVAL;
+
+ /* RFC 4861 7.1.1: DAD NS must not contain a source link-layer
+ * address option
+ */
+ if (reject_slla && opt->nd_opt_type == ND_OPT_SOURCE_LL_ADDR)
+ return -EINVAL;
+
+ if (opt->nd_opt_len * 8 > opts_len)
+ return -EINVAL;
+
+ offset += opt->nd_opt_len * 8;
+ opts_len -= opt->nd_opt_len * 8;
+ }
+
+ return 0;
+}
+
+static int ndisc_check_nd_msg(struct sk_buff *skb)
+{
+ unsigned int len = skb_transport_offset(skb) + sizeof(struct nd_msg);
+ unsigned int transport_len = ipv6_transport_len(skb);
+ bool reject_slla = false;
+ const struct nd_msg *msg;
+
+ if (!pskb_may_pull(skb, len))
+ return -EINVAL;
+
+ /* RFC 4861 7.1.1 / 7.1.2: ICMP length is at least sizeof(nd_msg) */
+ if (transport_len < sizeof(struct nd_msg))
+ return -EINVAL;
+
+ msg = (struct nd_msg *)skb_transport_header(skb);
+
+ /* RFC 4861 7.1.1 / 7.1.2: Target Address must not be a
+ * multicast address
+ */
+ if (ipv6_addr_is_multicast(&msg->target))
+ return -EINVAL;
+
+ switch (msg->icmph.icmp6_type) {
+ case NDISC_NEIGHBOUR_SOLICITATION:
+ if (ipv6_addr_any(&ipv6_hdr(skb)->saddr)) {
+ /* RFC 4861 7.1.1: DAD NS destination must be a
+ * solicited-node multicast address
+ */
+ if (!ipv6_addr_is_solict_mult(&ipv6_hdr(skb)->daddr))
+ return -EINVAL;
+ /* RFC 4861 7.1.1: DAD NS must not contain a source
+ * link-layer address option
+ */
+ reject_slla = true;
+ }
+ break;
+ case NDISC_NEIGHBOUR_ADVERTISEMENT:
+ /* RFC 4861 7.1.2: Solicited flag must be 0 for
+ * multicast destinations
+ */
+ if (ipv6_addr_is_multicast(&ipv6_hdr(skb)->daddr) &&
+ msg->icmph.icmp6_solicited)
+ return -EINVAL;
+ break;
+ default:
+ return -ENODATA;
+ }
+
+ return ndisc_check_options(skb, transport_len - sizeof(struct nd_msg),
+ reject_slla);
+}
+
+/**
+ * ndisc_check_ns_na - validate an NS/NA packet and set its transport header
+ * @skb: the skb to validate
+ *
+ * Validates an IPv6 packet for compliance with RFC 4861 sections 7.1.1
+ * (Neighbor Solicitation) and 7.1.2 (Neighbor Advertisement). If valid,
+ * sets the skb transport header.
+ *
+ * Caller needs to set the skb network header.
+ *
+ * Return:
+ * * 0 - valid NS/NA; the skb transport header has been set.
+ * * -EINVAL - a broken packet was detected, i.e. it violates some
+ * internet standard.
+ * * -ENOMSG - IP header validation succeeded but it is not an ICMPv6
+ * packet.
+ * * -ENODATA - IP+ICMPv6 header validation succeeded but it is not a
+ * Neighbor Solicitation or Neighbor Advertisement.
+ */
+int ndisc_check_ns_na(struct sk_buff *skb)
+{
+ int ret;
+
+ ret = ndisc_check_ip6hdr(skb);
+ if (ret < 0)
+ return ret;
+
+ ret = ndisc_check_icmpv6(skb);
+ if (ret < 0)
+ return ret;
+
+ return ndisc_check_nd_msg(skb);
+}
+EXPORT_SYMBOL(ndisc_check_ns_na);
--
2.54.0
^ permalink raw reply related
* [PATCH net-next 1/5] bridge: Use direct pointer in br_is_nd_neigh_msg()
From: Danielle Ratson @ 2026-07-19 13:34 UTC (permalink / raw)
To: netdev
Cc: dsahern, idosch, davem, edumazet, kuba, pabeni, horms, razor, ja,
petrm, fw, kuniyu, bridge, linux-kernel, Danielle Ratson
In-Reply-To: <cover.1784463131.git.danieller@nvidia.com>
Both callers of br_is_nd_neigh_msg() already call pskb_may_pull() to
ensure sizeof(struct ipv6hdr) + sizeof(struct nd_msg) bytes are in the
linear area before invoking this function. The skb_header_pointer()
call and its fallback buffer are therefore unnecessary.
Replace skb_header_pointer() with a direct cast to ipv6_hdr(skb) + 1
and drop the now-unused 'msg' parameter and its corresponding stack
buffer from all callers.
Reviewed-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Danielle Ratson <danieller@nvidia.com>
---
net/bridge/br_arp_nd_proxy.c | 9 ++-------
net/bridge/br_device.c | 4 ++--
net/bridge/br_input.c | 4 ++--
net/bridge/br_private.h | 2 +-
4 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/net/bridge/br_arp_nd_proxy.c b/net/bridge/br_arp_nd_proxy.c
index 23eb6931a2b4..db08c3272001 100644
--- a/net/bridge/br_arp_nd_proxy.c
+++ b/net/bridge/br_arp_nd_proxy.c
@@ -234,14 +234,9 @@ void br_do_proxy_suppress_arp(struct sk_buff *skb, struct net_bridge *br,
#endif
#if IS_ENABLED(CONFIG_IPV6)
-struct nd_msg *br_is_nd_neigh_msg(const struct sk_buff *skb, struct nd_msg *msg)
+struct nd_msg *br_is_nd_neigh_msg(const struct sk_buff *skb)
{
- struct nd_msg *m;
-
- m = skb_header_pointer(skb, skb_network_offset(skb) +
- sizeof(struct ipv6hdr), sizeof(*msg), msg);
- if (!m)
- return NULL;
+ struct nd_msg *m = (struct nd_msg *)(ipv6_hdr(skb) + 1);
if (m->icmph.icmp6_code != 0 ||
(m->icmph.icmp6_type != NDISC_NEIGHBOUR_SOLICITATION &&
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index e7f343ab22d3..ff55dab73632 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -80,9 +80,9 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
pskb_may_pull(skb, sizeof(struct ipv6hdr) +
sizeof(struct nd_msg)) &&
ipv6_hdr(skb)->nexthdr == IPPROTO_ICMPV6) {
- struct nd_msg *msg, _msg;
+ struct nd_msg *msg;
- msg = br_is_nd_neigh_msg(skb, &_msg);
+ msg = br_is_nd_neigh_msg(skb);
if (msg)
br_do_suppress_nd(skb, br, vid, NULL, msg);
}
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index ddb8f002a40e..d87a5f9fa92b 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -176,9 +176,9 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
pskb_may_pull(skb, sizeof(struct ipv6hdr) +
sizeof(struct nd_msg)) &&
ipv6_hdr(skb)->nexthdr == IPPROTO_ICMPV6) {
- struct nd_msg *msg, _msg;
+ struct nd_msg *msg;
- msg = br_is_nd_neigh_msg(skb, &_msg);
+ msg = br_is_nd_neigh_msg(skb);
if (msg)
br_do_suppress_nd(skb, br, vid, p, msg);
}
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index f8f77a2d4891..a51246747537 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -2365,7 +2365,7 @@ void br_do_proxy_suppress_arp(struct sk_buff *skb, struct net_bridge *br,
u16 vid, struct net_bridge_port *p);
void br_do_suppress_nd(struct sk_buff *skb, struct net_bridge *br,
u16 vid, struct net_bridge_port *p, struct nd_msg *msg);
-struct nd_msg *br_is_nd_neigh_msg(const struct sk_buff *skb, struct nd_msg *m);
+struct nd_msg *br_is_nd_neigh_msg(const struct sk_buff *skb);
bool br_is_neigh_suppress_enabled(const struct net_bridge_port *p, u16 vid);
bool br_is_neigh_forward_grat_enabled(const struct net_bridge_port *p, u16 vid);
#endif
--
2.54.0
^ permalink raw reply related
* [PATCH net-next 0/5] bridge: Validate and clean up IPv6 neighbour suppression
From: Danielle Ratson @ 2026-07-19 13:34 UTC (permalink / raw)
To: netdev
Cc: dsahern, idosch, davem, edumazet, kuba, pabeni, horms, razor, ja,
petrm, fw, kuniyu, bridge, linux-kernel, Danielle Ratson
The bridge implements IPv6 neighbour suppression by snooping Neighbour
Solicitation and Neighbour Advertisement messages, but it previously only
checked the ICMPv6 type and code before acting on them. This leaves it
open to acting on malformed or spoofed packets that any RFC 4861 compliant
node should reject, and the option parsing in br_nd_send() open-codes a
loop that has historically been a source of bugs.
This series hardens and cleans up that path:
Add ndisc_check_ns_na(), a standalone NS/NA validator modeled after
ipv6_mc_check_mld(), implementing the RFC 4861 section 7.1.1 / 7.1.2
mandatory receive checks (hop limit, checksum, code, length, target and
option validation). Wire the bridge into it so NS/NA messages are
validated to the same standard MLD already enjoys.
Replace the manual ND option parsing loop in br_nd_send() with
ndisc_parse_options() and ndisc_opt_addr_data(), and linearize the skb
once it has been validated as an NS/NA message so that this and any future
ND message handling operate on a linear buffer. The first patch is a small
preparatory cleanup that drops the now-unnecessary skb_header_pointer()
fallback from br_is_nd_neigh_msg().
No functional change is intended for well-formed packets.
Patchset overview:
Patch #1: drop the skb_header_pointer() fallback.
Patches #2-#3: add ndisc_check_ns_na() and validate NS/NA with it.
Patch #4: linearize once the ND message type is validated.
Patch #5: parse options via ndisc_parse_options().
Danielle Ratson (5):
bridge: Use direct pointer in br_is_nd_neigh_msg()
ipv6: ndisc: Add ndisc_check_ns_na() validation helper
bridge: Validate NS/NA messages using ndisc_check_ns_na()
bridge: Linearize skb once the ND message type is validated
bridge: Use ndisc_parse_options() to parse ND options in br_nd_send()
include/net/ndisc.h | 2 +
net/bridge/br_arp_nd_proxy.c | 54 +++++-----
net/bridge/br_device.c | 4 +-
net/bridge/br_input.c | 4 +-
net/bridge/br_private.h | 2 +-
net/ipv6/Makefile | 2 +-
net/ipv6/ndisc.c | 1 +
net/ipv6/ndisc_snoop.c | 190 +++++++++++++++++++++++++++++++++++
8 files changed, 224 insertions(+), 35 deletions(-)
create mode 100644 net/ipv6/ndisc_snoop.c
--
2.54.0
^ permalink raw reply
* Re: [PATCH v3 net 4/6] xsk: reclaim invalid multi-buffer Tx descs in ZC path
From: Maciej Fijalkowski @ 2026-07-19 13:21 UTC (permalink / raw)
To: Jason Xing
Cc: netdev, bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
bjorn
In-Reply-To: <CAL+tcoDyygM5XiVhHrmYrZTnZN6QSD0dN9mMPZmZYPtikssYxw@mail.gmail.com>
On Thu, Jul 16, 2026 at 11:58:24PM +0200, Jason Xing wrote:
> On Tue, Jul 14, 2026 at 4:08 PM Maciej Fijalkowski
> <maciej.fijalkowski@intel.com> wrote:
> >
> > The zero-copy Tx batch parser stops when it encounters an invalid
> > descriptor. If this happens after one or more continuation descriptors,
> > the Tx consumer can be advanced past fragments that are neither submitted
> > to the driver nor returned to userspace through the completion ring.
> >
> > A similar problem occurs when a packet exceeds xdp_zc_max_segs. The
> > descriptors consumed up to the limit are released without completion, and
> > the remaining continuation descriptors can subsequently be interpreted
> > as the beginning of another packet.
> >
> > Parse Tx batches in packet units and distinguish descriptors belonging to
> > complete valid packets from descriptors consumed while draining an
> > invalid or oversized packet. Return the former to the driver and append
> > the latter to the CQ address area so userspace can reclaim their UMEM
> > frames.
> >
> > Once draining starts, continue until the packet's end-of-packet
> > descriptor is consumed. Preserve the drain state on the socket when EOP
> > has not yet been supplied, so draining can continue during a later call.
> > Leave incomplete but otherwise valid packets on the Tx ring. Keep the
> > existing handling of standalone invalid descriptors unchanged.
> >
> > Shared-UMEM pools using multi-buffer Tx also need packet-framed parsing.
> > Walk their Tx sockets one packet at a time, preserving the existing
> > per-socket fairness scheme, instead of using the legacy one-descriptor
> > fallback. Keep that fallback for shared pools that do not use
> > multi-buffer Tx. Since the drain state is maintained per socket and both
> > the singular and shared paths can resume an interrupted drain, changing
> > the socket list from singular to shared requires no special bind-time
> > transition.
> >
> > CQ entries are positional, and drivers may complete only part of the Tx
> > work returned by xsk_tx_peek_release_desc_batch(). Therefore, reclaim-only
> > entries cannot be published immediately when earlier driver-visible
> > descriptors are still outstanding.
> >
> > Track the number of driver-visible CQ entries preceding the reclaim
> > entries. Let xsk_tx_completed() publish partial real Tx completions, and
> > publish the reclaim entries only after every earlier Tx descriptor has
> > completed. Complete a reclaim-only batch immediately when there is no
> > driver-visible work in front of it, and prevent another Tx batch from
> > being appended while reclaim entries remain pending.
> >
> > Also cap batch processing by the size of the pool's temporary descriptor
> > array, as Tx rings belonging to sockets sharing a UMEM may have different
> > sizes.
> >
> > This ensures that every descriptor consumed as part of an invalid
> > multi-buffer packet is eventually returned to userspace without exposing
> > the dropped packet to the driver or violating CQ completion ordering.
> >
> > Fixes: cf24f5a5feea ("xsk: add support for AF_XDP multi-buffer on Tx path")
> > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
>
> Thanks for working on this big patch! It's not easy to fix it in a
> simpler way, I think. And I didn't observe any obvious performance
> impact by xdpsock.
>
> Overall, it looks good to me except for a few minor points:
> Reviewed-by: Jason Xing <kerneljasonxing@gmail.com>
>
> > ---
> > include/net/xsk_buff_pool.h | 3 +
> > net/xdp/xsk.c | 192 ++++++++++++++++++++++++++++++++----
> > net/xdp/xsk_buff_pool.c | 1 +
> > net/xdp/xsk_queue.h | 76 ++++++++++----
> > 4 files changed, 232 insertions(+), 40 deletions(-)
> >
> > diff --git a/include/net/xsk_buff_pool.h b/include/net/xsk_buff_pool.h
> > index f5e737a83055..2bb1d122b1bc 100644
> > --- a/include/net/xsk_buff_pool.h
> > +++ b/include/net/xsk_buff_pool.h
> > @@ -78,6 +78,9 @@ struct xsk_buff_pool {
> > u32 chunk_size;
> > u32 chunk_shift;
> > u32 frame_len;
> > + u32 tx_descs_nentries;
> > + u32 reclaim_descs;
> > + u32 tx_zc_pending_descs;
> > u32 xdp_zc_max_segs;
> > u8 tx_metadata_len; /* inherited from umem */
> > u8 cached_need_wakeup;
> > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> > index 385a3f4a1b32..2909a0ec6837 100644
> > --- a/net/xdp/xsk.c
> > +++ b/net/xdp/xsk.c
> > @@ -499,6 +499,23 @@ void __xsk_map_flush(struct list_head *flush_list)
> >
> > void xsk_tx_completed(struct xsk_buff_pool *pool, u32 nb_entries)
> > {
> > + u32 reclaim_descs = READ_ONCE(pool->reclaim_descs);
> > +
> > + if (unlikely(reclaim_descs)) {
>
> Just a side note: it might impact the performance because new descs
> need to wait for the existing descs to be completed if there are
> reclaim descs.
That is a tradeoff for dealing with this corner case i'd say.
>
> > + u32 pending_descs = READ_ONCE(pool->tx_zc_pending_descs);
> > +
> > + if (nb_entries < pending_descs) {
> > + WRITE_ONCE(pool->tx_zc_pending_descs,
> > + pending_descs - nb_entries);
> > + xskq_prod_submit_n(pool->cq, nb_entries);
> > + return;
> > + }
> > +
> > + WRITE_ONCE(pool->tx_zc_pending_descs, 0);
> > + nb_entries += reclaim_descs;
> > + WRITE_ONCE(pool->reclaim_descs, 0);
> > + }
> > +
> > xskq_prod_submit_n(pool->cq, nb_entries);
> > }
> > EXPORT_SYMBOL(xsk_tx_completed);
> > @@ -574,24 +591,162 @@ static u32 xsk_tx_peek_release_fallback(struct xsk_buff_pool *pool, u32 max_entr
> > return nb_pkts;
> > }
> >
> > +static void xsk_tx_commit_batch(struct xsk_buff_pool *pool,
> > + struct xsk_tx_batch *batch)
> > +{
> > + u32 nb_descs = xsk_tx_batch_cq_descs(batch);
> > + u32 cq_cached_prod;
> > +
> > + if (!nb_descs)
> > + return;
> > +
> > + cq_cached_prod = pool->cq->cached_prod;
> > + xskq_prod_write_addr_batch(pool->cq, pool->tx_descs, nb_descs);
> > +
> > + if (unlikely(batch->reclaim_descs)) {
> > + u32 cq_pending_descs;
> > +
> > + /* CQ is positional. Descriptors already written but not
> > + * submitted must complete before any reclaim-only descriptors
> > + * appended below.
> > + */
> > + cq_pending_descs = cq_cached_prod - xskq_get_prod(pool->cq);
> > +
> > + WRITE_ONCE(pool->tx_zc_pending_descs,
> > + batch->tx_descs + cq_pending_descs);
> > + WRITE_ONCE(pool->reclaim_descs, batch->reclaim_descs);
> > + if (unlikely(!pool->tx_zc_pending_descs))
> > + xsk_tx_completed(pool, 0);
> > + }
> > +}
> > +
> > +static struct xsk_tx_batch
> > +__xsk_tx_peek_release_desc_batch(struct xsk_buff_pool *pool, struct xdp_sock *xs,
> > + struct xdp_desc *descs, u32 max_descs)
> > +{
> > + struct xsk_tx_batch batch = {};
> > + u32 entries;
> > +
> > + entries = xskq_cons_nb_entries(xs->tx, max_descs);
> > + if (!entries)
> > + return batch;
> > +
> > + batch = xskq_cons_read_desc_batch(xs, pool, descs, max_descs);
> > + if (!xsk_tx_batch_cq_descs(&batch)) {
> > + xs->tx->queue_empty_descs++;
> > + if (batch.consumed_descs) {
> > + __xskq_cons_release(xs->tx);
> > + xs->sk.sk_write_space(&xs->sk);
> > + }
> > + return batch;
> > + }
> > +
> > + __xskq_cons_release(xs->tx);
> > + xs->sk.sk_write_space(&xs->sk);
> > + return batch;
> > +}
> > +
> > +static struct xsk_tx_batch
> > +xsk_tx_peek_release_shared_desc_batch(struct xsk_buff_pool *pool, u32 max_descs)
> > +{
> > + u32 cq_descs_before, cq_descs_after;
> > + struct xsk_tx_batch sum_batch = {};
> > + bool budget_exhausted;
> > + u32 per_socket_budget;
> > + struct xdp_sock *xs;
> > +
> > + /* The fairness quota must allow one maximum-sized valid packet. */
> > + per_socket_budget = max_t(u32, MAX_PER_SOCKET_BUDGET,
> > + pool->xdp_zc_max_segs);
> > +
> > +again:
> > + budget_exhausted = false;
> > + cq_descs_before = xsk_tx_batch_cq_descs(&sum_batch);
> > + list_for_each_entry_rcu(xs, &pool->xsk_tx_list, tx_list) {
> > + u32 budget, budget_left, offset, remaining;
> > + struct xsk_tx_batch curr_batch;
> > +
> > + /* Once reclaim-only descriptors have been appended to the CQ
> > + * address area, do not append driver-visible Tx descriptors
> > + * from another socket after them. xsk_tx_completed() relies on
> > + * all driver-visible descriptors preceding all reclaim-only
> > + * descriptors in CQ order.
> > + */
> > + if (sum_batch.reclaim_descs)
> > + break;
> > +
> > + /* be gentle when playing with pool->tx_descs */
>
> Minor nit: seems unneeded comment?
this has been my helper/reminder that we need to respect already consumed
space at tx_descs array; i can remove it
>
> > + offset = xsk_tx_batch_cq_descs(&sum_batch);
> > + if (offset >= max_descs)
> > + break;
> > +
> > + if (xs->tx_budget_spent >= per_socket_budget) {
> > + if (xskq_cons_nb_entries(xs->tx, 1))
> > + budget_exhausted = true;
> > + continue;
> > + }
> > +
> > + budget_left = per_socket_budget - xs->tx_budget_spent;
> > + remaining = max_descs - offset;
> > + budget = min(remaining, budget_left);
> > +
> > + curr_batch = __xsk_tx_peek_release_desc_batch(pool, xs,
> > + pool->tx_descs + offset,
> > + budget);
> > + if (!xsk_tx_batch_cq_descs(&curr_batch)) {
> > + if (curr_batch.budget_limited && budget_left < remaining)
> > + budget_exhausted = true;
> > + xs->tx_budget_spent += curr_batch.consumed_descs;
> > + continue;
> > + }
> > +
> > + xs->tx_budget_spent += curr_batch.consumed_descs;
> > + sum_batch.tx_descs += curr_batch.tx_descs;
>
> No need to use '+' here because of the previous reclaim_descs check.
hmm correct!
>
> > + sum_batch.reclaim_descs += curr_batch.reclaim_descs;
> > + }
> > +
> > + cq_descs_after = xsk_tx_batch_cq_descs(&sum_batch);
> > +
> > + if (sum_batch.reclaim_descs || cq_descs_after >= max_descs)
> > + return sum_batch;
> > +
> > + /* Continue filling the batch while this pass made progress */
> > + if (cq_descs_before != cq_descs_after)
> > + goto again;
> > +
> > + if (!budget_exhausted)
> > + return sum_batch;
> > +
> > + list_for_each_entry_rcu(xs, &pool->xsk_tx_list, tx_list)
> > + xs->tx_budget_spent = 0;
> > + goto again;
> > +}
> > +
> > u32 xsk_tx_peek_release_desc_batch(struct xsk_buff_pool *pool, u32 nb_pkts)
> > {
> > + struct xsk_tx_batch batch = {};
> > struct xdp_sock *xs;
> > + bool umem_shared;
> >
> > rcu_read_lock();
> > - if (!list_is_singular(&pool->xsk_tx_list)) {
> > - /* Fallback to the non-batched version */
> > - rcu_read_unlock();
> > - return xsk_tx_peek_release_fallback(pool, nb_pkts);
> > - }
> > + if (unlikely(READ_ONCE(pool->reclaim_descs)))
> > + goto out;
> >
> > - xs = list_first_or_null_rcu(&pool->xsk_tx_list, struct xdp_sock, tx_list);
> > - if (!xs) {
> > - nb_pkts = 0;
> > + xs = list_first_or_null_rcu(&pool->xsk_tx_list, struct xdp_sock,
> > + tx_list);
> > + if (!xs)
> > goto out;
> > - }
> >
> > - nb_pkts = xskq_cons_nb_entries(xs->tx, nb_pkts);
> > + nb_pkts = min(nb_pkts, pool->tx_descs_nentries);
> > + if (!nb_pkts)
> > + goto out;
> > +
> > + umem_shared = !list_is_singular(&pool->xsk_tx_list);
> > +
> > + if (umem_shared && !(pool->umem->flags & XDP_UMEM_SG_FLAG)) {
> > + rcu_read_unlock();
> > + return xsk_tx_peek_release_fallback(pool, nb_pkts);
> > + }
> >
> > /* This is the backpressure mechanism for the Tx path. Try to
> > * reserve space in the completion queue for all packets, but
> > @@ -603,19 +758,16 @@ u32 xsk_tx_peek_release_desc_batch(struct xsk_buff_pool *pool, u32 nb_pkts)
> > if (!nb_pkts)
> > goto out;
> >
> > - nb_pkts = xskq_cons_read_desc_batch(xs->tx, pool, nb_pkts);
> > - if (!nb_pkts) {
> > - xs->tx->queue_empty_descs++;
> > - goto out;
> > - }
> > -
> > - __xskq_cons_release(xs->tx);
> > - xskq_prod_write_addr_batch(pool->cq, pool->tx_descs, nb_pkts);
> > - xs->sk.sk_write_space(&xs->sk);
> > + batch = umem_shared ?
> > + xsk_tx_peek_release_shared_desc_batch(pool, nb_pkts) :
> > + __xsk_tx_peek_release_desc_batch(pool, xs,
> > + pool->tx_descs,
> > + nb_pkts);
> > + xsk_tx_commit_batch(pool, &batch);
> >
> > out:
> > rcu_read_unlock();
> > - return nb_pkts;
> > + return batch.tx_descs;
> > }
> > EXPORT_SYMBOL(xsk_tx_peek_release_desc_batch);
> >
> > diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
> > index 12c9fb29af05..a4089480b22b 100644
> > --- a/net/xdp/xsk_buff_pool.c
> > +++ b/net/xdp/xsk_buff_pool.c
> > @@ -51,6 +51,7 @@ int xp_alloc_tx_descs(struct xsk_buff_pool *pool, struct xdp_sock *xs,
> > if (!pool->tx_descs)
> > return -ENOMEM;
> >
> > + pool->tx_descs_nentries = nentries;
> > return 0;
> > }
> >
> > diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
> > index 3e3fbb73d23e..a15ff1929db6 100644
> > --- a/net/xdp/xsk_queue.h
> > +++ b/net/xdp/xsk_queue.h
> > @@ -58,6 +58,18 @@ struct parsed_desc {
> > u32 valid;
> > };
> >
> > +struct xsk_tx_batch {
> > + u32 tx_descs;
> > + u32 reclaim_descs;
> > + u32 consumed_descs;
> > + bool budget_limited;
> > +};
> > +
> > +static inline u32 xsk_tx_batch_cq_descs(const struct xsk_tx_batch *batch)
> > +{
> > + return batch->tx_descs + batch->reclaim_descs;
> > +}
> > +
> > /* The structure of the shared state of the rings are a simple
> > * circular buffer, as outlined in
> > * Documentation/core-api/circular-buffers.rst. For the Rx and
> > @@ -263,17 +275,18 @@ static inline void parse_desc(struct xsk_queue *q, struct xsk_buff_pool *pool,
> > parsed->mb = xp_mb_desc(desc);
> > }
> >
> > -static inline
> > -u32 xskq_cons_read_desc_batch(struct xsk_queue *q, struct xsk_buff_pool *pool,
> > - u32 max)
> > +static inline struct xsk_tx_batch
> > +xskq_cons_read_desc_batch(struct xdp_sock *xs, struct xsk_buff_pool *pool,
> > + struct xdp_desc *descs, u32 max)
> > {
> > - u32 cached_cons = q->cached_cons, nb_entries = 0;
> > - struct xdp_desc *descs = pool->tx_descs;
> > - u32 total_descs = 0, nr_frags = 0;
> > + bool drain = READ_ONCE(xs->drain_cont);
> > + u32 cached_cons, nb_entries = 0, released;
> > + struct xsk_tx_batch batch = {};
> > + struct xsk_queue *q = xs->tx;
> > + u32 nr_frags = 0;
> > +
> > + cached_cons = q->cached_cons;
> >
> > - /* track first entry, if stumble upon *any* invalid descriptor, rewind
> > - * current packet that consists of frags and stop the processing
> > - */
> > while (cached_cons != q->cached_prod && nb_entries < max) {
> > struct xdp_rxtx_ring *ring = (struct xdp_rxtx_ring *)q->ring;
> > u32 idx = cached_cons & q->ring_mask;
> > @@ -282,26 +295,49 @@ u32 xskq_cons_read_desc_batch(struct xsk_queue *q, struct xsk_buff_pool *pool,
> > descs[nb_entries] = ring->desc[idx];
> > cached_cons++;
> > parse_desc(q, pool, &descs[nb_entries], &parsed);
> > - if (unlikely(!parsed.valid))
> > - break;
> > + if (unlikely(!parsed.valid)) {
> > + if (!drain && !nr_frags && !parsed.mb)
>
> I understand you're fixing the mb problem here. But I'm wondering if
> it still has a problem in the non mb case because the single invalid
> packet (mb ==0, nr_frags == 0, drain == 0) that isn't published in CQ
> cannot be tracked by application?
>
> My thinking is to just remove the above line in this patch. Or I can
> cook a follow-up patch to fix this specific problem?
Good catch - seems I got too focused at mb case and now we have a bit of
misbehave as invalid mb descs are cq produced and standalone not.
I'm gonna address this and align standalone descs (in generic xmit as
well) that are invalid so they are also cq published, not silently wiped
out from tx ring only.
Thanks! sending v4.
>
> Thanks,
> Jason
>
> > + break;
> > +
> > + drain = true;
> > + }
> > +
> > + nr_frags++;
> > + nb_entries++;
> >
> > if (likely(!parsed.mb)) {
> > - total_descs += (nr_frags + 1);
> > - nr_frags = 0;
> > - } else {
> > - nr_frags++;
> > - if (nr_frags == pool->xdp_zc_max_segs) {
> > + if (unlikely(drain)) {
> > + batch.reclaim_descs = nr_frags;
> > + WRITE_ONCE(xs->drain_cont, false);
> > nr_frags = 0;
> > break;
> > }
> > +
> > + batch.tx_descs += nr_frags;
> > + nr_frags = 0;
> > + continue;
> > + }
> > +
> > + if (nr_frags == pool->xdp_zc_max_segs)
> > + drain = true;
> > + }
> > +
> > + if (nr_frags) {
> > + if (drain) {
> > + batch.reclaim_descs = nr_frags;
> > + WRITE_ONCE(xs->drain_cont, true);
> > + } else {
> > + if (nb_entries == max)
> > + batch.budget_limited = true;
> > + cached_cons -= nr_frags;
> > }
> > - nb_entries++;
> > }
> >
> > - cached_cons -= nr_frags;
> > + released = cached_cons - q->cached_cons;
> > /* Release valid plus any invalid entries */
> > - xskq_cons_release_n(q, cached_cons - q->cached_cons);
> > - return total_descs;
> > + xskq_cons_release_n(q, released);
> > + batch.consumed_descs = released;
> > + return batch;
> > }
> >
> > /* Functions for consumers */
> > --
> > 2.43.0
> >
>
^ permalink raw reply
* Re: [PATCH net-next v2 2/2] nfc: s3fwrn5: support the S3NRN4V variant
From: David Heidelberg @ 2026-07-19 13:15 UTC (permalink / raw)
To: Jorijn van der Graaf, Krzysztof Kozlowski
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Conor Dooley, oe-linux-nfc, netdev,
devicetree, linux-kernel, Luca Weiss
In-Reply-To: <20260705190621.128257-3-jorijnvdgraaf@catcrafts.net>
On 05/07/2026 21:06, Jorijn van der Graaf wrote:
> The S3NRN4V (e.g. on the Fairphone 6, SM7635) is an S3FWRN5-family NFC
> controller that needs different bring-up, selected with a new
> samsung,s3nrn4v compatible:
>
> - It ships with working firmware behind a bootloader protocol this
> driver does not implement (GET_BOOTINFO times out), so the firmware
> download step is skipped. Its RF registers are (re)loaded with the
> proprietary DUAL_OPTION command (the HW and SW register blobs merged
> into a single stream) instead of the START/SET/STOP_RFREG sequence.
>
> - Its reference clock speed is configured with the single-byte FW_CFG
> form, sent from the ->setup hook (after CORE_RESET, before CORE_INIT).
> The selector value (0x11) is taken from the vendor configuration for
> this part; its encoding is not documented.
>
> - It gates its XI clock through a CLK_REQ line: the chip drives it high
> when it needs the clock, notably to synthesise the 13.56 MHz poll
> carrier. Left always-on, the free-running clock never lets the chip's
> TX PLL lock on a fresh start and it cannot poll (it falls back to
> listen only). Service the handshake when a clk-req GPIO is described,
> gating the clock on it; without one the clock stays always-on.
>
> The variant is carried as match data by both the OF and the I2C device
> id tables so the two match paths agree, and the OF table is now
> referenced unconditionally for its match data, so drop the
> of_match_ptr()/__maybe_unused annotations from it.
Hello Jorijn,
thank you for the work on the driver!
Please, send the "drop the of_match_ptr()/__maybe_unused annotations from it."
type of change as part of the series, but as a separate commit before the new HW
support introduction.
Since you touch S3FWRN5_I2C_DRIVER_NAME, replace define S3FWRN5_I2C_DRIVER_NAME
occurenced with the "s3fwrn5_i2c" directly before introducing the support (also
separate commit)
>
> The error policy differs between the two configuration steps on purpose:
> a clock misconfiguration is fatal (a ->setup failure aborts CORE_INIT),
> whereas an RF-register update failure is only warned about and bring-up
> continues, since the chip falls back to the RF registers programmed in
> its flash and NFC may still work.
>
> Unlike the host-endian word read in the legacy rfreg path, the
> DUAL_OPTION checksum is accumulated with get_unaligned_le32() and emitted
> little-endian explicitly, so it is correct regardless of CPU endianness.
>
> Existing S3FWRN5 / S3FWRN82 setups keep the firmware-download path and
> the always-on clock, unchanged.
>
> Assisted-by: Claude:claude-opus-4-8
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>
> ---
> Changes in v2:
> - Rename the new compatible to samsung,s3nrn4v, matching the binding
> change (Requested by: Conor Dooley).
> - Close a race in the probe-time CLK_REQ seeding: the GPIO level is
> now read under clk_lock (new s3fwrn5_i2c_clk_sync(), used by both
> the irq thread and probe), so a level read before the irq fired can
> never overwrite the fresher state the irq thread applied (found by
> the Sashiko AI review of v1).
> - Reject malformed rfreg blobs (word alignment, single-byte section
> index bound) up front instead of failing at STOP_UPDATE.
> - Handle gpiod_get_value_cansleep() failure instead of gating the
> clock off on error.
> - Add an s3nrn4v i2c_device_id entry carrying the variant so both
> match paths agree.
> - Describe the of_match_ptr()/__maybe_unused removal in the commit
> message.
> v1: https://lore.kernel.org/20260703202601.78563-3-jorijnvdgraaf@catcrafts.net
>
> drivers/nfc/s3fwrn5/core.c | 40 +++++++++-
> drivers/nfc/s3fwrn5/i2c.c | 138 +++++++++++++++++++++++++++++++---
> drivers/nfc/s3fwrn5/nci.c | 119 ++++++++++++++++++++++++++++-
> drivers/nfc/s3fwrn5/nci.h | 32 +++++++-
> drivers/nfc/s3fwrn5/s3fwrn5.h | 14 +++-
> drivers/nfc/s3fwrn5/uart.c | 2 +-
> 6 files changed, 330 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/nfc/s3fwrn5/core.c b/drivers/nfc/s3fwrn5/core.c
> index af0fa8bd970b..59317eaad7ac 100644
> --- a/drivers/nfc/s3fwrn5/core.c
> +++ b/drivers/nfc/s3fwrn5/core.c
> @@ -122,11 +122,47 @@ static int s3fwrn5_nci_send(struct nci_dev *ndev, struct sk_buff *skb)
> return 0;
> }
>
> +static int s3fwrn5_nci_setup(struct nci_dev *ndev)
> +{
> + struct s3fwrn5_info *info = nci_get_drvdata(ndev);
> +
> + /*
> + * Runs after CORE_RESET, before CORE_INIT. The S3NRN4V needs its
> + * reference clock configured here (the downstream stack does it in the
> + * bootloader, before CORE_RESET, but this is the earliest hook the NCI
> + * core offers and the chip accepts it).
> + */
> + if (info->variant == S3FWRN5_VARIANT_S3NRN4V)
> + return s3fwrn5_nci_clk_cfg(info);
> +
> + return 0;
> +}
> +
> static int s3fwrn5_nci_post_setup(struct nci_dev *ndev)
> {
> struct s3fwrn5_info *info = nci_get_drvdata(ndev);
> int ret;
>
> + if (info->variant == S3FWRN5_VARIANT_S3NRN4V) {
> + /*
> + * The S3NRN4V ships with working firmware behind a bootloader
> + * protocol this driver does not implement, so there is no
> + * download step; the NCI core has already done CORE_RESET +
> + * CORE_INIT. Just (re)load the RF registers via DUAL_OPTION.
> + */
> + ret = s3fwrn5_nci_rf_configure_dual(info, "sec_s3nrn4v_hwreg.bin",
> + "sec_s3nrn4v_swreg.bin");
> + /*
> + * Keep going even if the blobs could not be loaded: the chip
> + * still enumerates and falls back to the RF registers programmed
> + * in its flash, so NFC may work anyway.
> + */
> + if (ret < 0)
> + dev_warn(&ndev->nfc_dev->dev,
> + "rfreg configure failed (%d)\n", ret);
> + return 0;
> + }
> +
> if (s3fwrn5_firmware_init(info)) {
> //skip bootloader mode
> return 0;
> @@ -152,13 +188,14 @@ static const struct nci_ops s3fwrn5_nci_ops = {
> .open = s3fwrn5_nci_open,
> .close = s3fwrn5_nci_close,
> .send = s3fwrn5_nci_send,
> + .setup = s3fwrn5_nci_setup,
> .post_setup = s3fwrn5_nci_post_setup,
> .prop_ops = s3fwrn5_nci_prop_ops,
> .n_prop_ops = ARRAY_SIZE(s3fwrn5_nci_prop_ops),
> };
>
> int s3fwrn5_probe(struct nci_dev **ndev, void *phy_id, struct device *pdev,
> - const struct s3fwrn5_phy_ops *phy_ops)
> + const struct s3fwrn5_phy_ops *phy_ops, enum s3fwrn5_variant variant)
> {
> struct s3fwrn5_info *info;
> int ret;
> @@ -170,6 +207,7 @@ int s3fwrn5_probe(struct nci_dev **ndev, void *phy_id, struct device *pdev,
> info->phy_id = phy_id;
> info->pdev = pdev;
> info->phy_ops = phy_ops;
> + info->variant = variant;
> mutex_init(&info->mutex);
>
> s3fwrn5_set_mode(info, S3FWRN5_MODE_COLD);
> diff --git a/drivers/nfc/s3fwrn5/i2c.c b/drivers/nfc/s3fwrn5/i2c.c
> index e9a34d27a369..7d20e737e402 100644
> --- a/drivers/nfc/s3fwrn5/i2c.c
> +++ b/drivers/nfc/s3fwrn5/i2c.c
> @@ -23,9 +23,76 @@ struct s3fwrn5_i2c_phy {
> struct i2c_client *i2c_dev;
> struct clk *clk;
>
> + /*
> + * Optional hardware clock-request handshake. When a CLK_REQ GPIO is
> + * wired, the chip drives it high while it needs its XI clock -- notably
> + * to generate the poll/reader carrier -- and the clock is gated on it
> + * instead of being left always-on (which never lets the chip's TX PLL
> + * lock on a fresh clock start, leaving it unable to poll).
> + */
> + struct gpio_desc *gpio_clk_req;
> + bool clk_on;
> + struct mutex clk_lock; /* serialises clk_on against the CLK_REQ irq */
> +
> unsigned int irq_skip:1;
> };
>
> +static void s3fwrn5_i2c_clk_set_locked(struct s3fwrn5_i2c_phy *phy, bool on)
> +{
> + lockdep_assert_held(&phy->clk_lock);
> +
> + if (on && !phy->clk_on) {
> + int ret = clk_prepare_enable(phy->clk);
> +
> + if (ret == 0)
> + phy->clk_on = true;
> + else
> + dev_warn_once(&phy->i2c_dev->dev,
> + "failed to enable clock (%d); NFC may not poll\n",
> + ret);
> + } else if (!on && phy->clk_on) {
> + clk_disable_unprepare(phy->clk);
> + phy->clk_on = false;
> + }
> +}
> +
> +/*
> + * Apply the current CLK_REQ level. Reading the GPIO under clk_lock makes
> + * concurrent callers (the CLK_REQ irq thread and the probe-time seeding)
> + * safe: whoever runs last applies a level read after the earlier update,
> + * never a stale one.
> + */
> +static void s3fwrn5_i2c_clk_sync(struct s3fwrn5_i2c_phy *phy)
> +{
> + int level;
> +
> + mutex_lock(&phy->clk_lock);
> + level = gpiod_get_value_cansleep(phy->gpio_clk_req);
> + if (level >= 0)
> + s3fwrn5_i2c_clk_set_locked(phy, level > 0);
> + else
> + dev_warn_once(&phy->i2c_dev->dev,
> + "failed to read CLK_REQ (%d); keeping clock state\n",
> + level);
> + mutex_unlock(&phy->clk_lock);
> +}
> +
> +static void s3fwrn5_i2c_clk_disable_action(void *data)
> +{
> + struct s3fwrn5_i2c_phy *phy = data;
> +
> + mutex_lock(&phy->clk_lock);
> + s3fwrn5_i2c_clk_set_locked(phy, false);
> + mutex_unlock(&phy->clk_lock);
> +}
> +
> +static irqreturn_t s3fwrn5_i2c_clk_req_thread(int irq, void *phy_id)
> +{
> + s3fwrn5_i2c_clk_sync(phy_id);
> +
> + return IRQ_HANDLED;
> +}
> +
> static void s3fwrn5_i2c_set_mode(void *phy_id, enum s3fwrn5_mode mode)
> {
> struct s3fwrn5_i2c_phy *phy = phy_id;
> @@ -146,6 +213,7 @@ static irqreturn_t s3fwrn5_i2c_irq_thread_fn(int irq, void *phy_id)
>
> static int s3fwrn5_i2c_probe(struct i2c_client *client)
> {
> + enum s3fwrn5_variant variant;
> struct s3fwrn5_i2c_phy *phy;
> int ret;
>
> @@ -172,15 +240,61 @@ static int s3fwrn5_i2c_probe(struct i2c_client *client)
> * S3FWRN5 depends on a clock input ("XI" pin) to function properly.
> * Depending on the hardware configuration this could be an always-on
> * oscillator or some external clock that must be explicitly enabled.
> - * Make sure the clock is running before starting S3FWRN5.
> + *
> + * If a CLK_REQ GPIO is wired, the chip gates the clock itself (driving
> + * CLK_REQ high when it needs XI); service that handshake. Otherwise just
> + * make sure the clock is running before starting S3FWRN5.
> */
> - phy->clk = devm_clk_get_optional_enabled(&client->dev, NULL);
> - if (IS_ERR(phy->clk))
> - return dev_err_probe(&client->dev, PTR_ERR(phy->clk),
> - "failed to get clock\n");
> + mutex_init(&phy->clk_lock);
> + phy->gpio_clk_req = devm_gpiod_get_optional(&client->dev, "clk-req",
> + GPIOD_IN);
> + if (IS_ERR(phy->gpio_clk_req))
> + return PTR_ERR(phy->gpio_clk_req);
> +
> + if (phy->gpio_clk_req) {
> + int clk_req_irq;
> +
> + phy->clk = devm_clk_get_optional(&client->dev, NULL);
> + if (IS_ERR(phy->clk))
> + return dev_err_probe(&client->dev, PTR_ERR(phy->clk),
> + "failed to get clock\n");
> +
> + /*
> + * Unlike the always-on branch below, this clock is enabled by
> + * hand from the CLK_REQ handler, so devm will not disable it on
> + * unbind. Gate it off explicitly if it is still on at teardown.
> + */
> + ret = devm_add_action_or_reset(&client->dev,
> + s3fwrn5_i2c_clk_disable_action,
> + phy);
> + if (ret)
> + return ret;
> +
> + clk_req_irq = gpiod_to_irq(phy->gpio_clk_req);
> + if (clk_req_irq < 0)
> + return clk_req_irq;
> +
> + ret = devm_request_threaded_irq(&client->dev, clk_req_irq, NULL,
> + s3fwrn5_i2c_clk_req_thread,
> + IRQF_TRIGGER_RISING |
> + IRQF_TRIGGER_FALLING |
> + IRQF_ONESHOT,
> + "s3fwrn5_clk_req", phy);
> + if (ret)
> + return ret;
> +
> + /* Seed the clock state from the current CLK_REQ level. */
> + s3fwrn5_i2c_clk_sync(phy);
> + } else {
> + phy->clk = devm_clk_get_optional_enabled(&client->dev, NULL);
> + if (IS_ERR(phy->clk))
> + return dev_err_probe(&client->dev, PTR_ERR(phy->clk),
> + "failed to get clock\n");
> + }
>
> + variant = (uintptr_t)i2c_get_match_data(client);
> ret = s3fwrn5_probe(&phy->common.ndev, phy, &phy->i2c_dev->dev,
> - &i2c_phy_ops);
> + &i2c_phy_ops, variant);
> if (ret < 0)
> return ret;
>
> @@ -205,13 +319,17 @@ static void s3fwrn5_i2c_remove(struct i2c_client *client)
> }
>
> static const struct i2c_device_id s3fwrn5_i2c_id_table[] = {
> - { .name = S3FWRN5_I2C_DRIVER_NAME },
> + { .name = S3FWRN5_I2C_DRIVER_NAME, .driver_data = S3FWRN5_VARIANT_FWDL },
> + { .name = "s3nrn4v", .driver_data = S3FWRN5_VARIANT_S3NRN4V },
> { }
> };
> MODULE_DEVICE_TABLE(i2c, s3fwrn5_i2c_id_table);
>
> -static const struct of_device_id of_s3fwrn5_i2c_match[] __maybe_unused = {
> - { .compatible = "samsung,s3fwrn5-i2c", },
> +static const struct of_device_id of_s3fwrn5_i2c_match[] = {
> + { .compatible = "samsung,s3fwrn5-i2c",
> + .data = (void *)S3FWRN5_VARIANT_FWDL, },
> + { .compatible = "samsung,s3nrn4v",
> + .data = (void *)S3FWRN5_VARIANT_S3NRN4V, },
is S3NRN4V really a variant of S3FWRN5 or is it just S3NRN4V?
> {}
> };
> MODULE_DEVICE_TABLE(of, of_s3fwrn5_i2c_match);
> @@ -219,7 +337,7 @@ MODULE_DEVICE_TABLE(of, of_s3fwrn5_i2c_match);
> static struct i2c_driver s3fwrn5_i2c_driver = {
> .driver = {
> .name = S3FWRN5_I2C_DRIVER_NAME,
> - .of_match_table = of_match_ptr(of_s3fwrn5_i2c_match),
> + .of_match_table = of_s3fwrn5_i2c_match,
> },
> .probe = s3fwrn5_i2c_probe,
> .remove = s3fwrn5_i2c_remove,
> diff --git a/drivers/nfc/s3fwrn5/nci.c b/drivers/nfc/s3fwrn5/nci.c
> index 5a9de11bbece..7034fb810e18 100644
> --- a/drivers/nfc/s3fwrn5/nci.c
> +++ b/drivers/nfc/s3fwrn5/nci.c
> @@ -8,6 +8,9 @@
>
> #include <linux/completion.h>
> #include <linux/firmware.h>
> +#include <linux/minmax.h>
> +#include <linux/slab.h>
> +#include <linux/unaligned.h>
>
> #include "s3fwrn5.h"
> #include "nci.h"
> @@ -20,7 +23,7 @@ static int s3fwrn5_nci_prop_rsp(struct nci_dev *ndev, struct sk_buff *skb)
> return 0;
> }
>
> -const struct nci_driver_ops s3fwrn5_nci_prop_ops[4] = {
> +const struct nci_driver_ops s3fwrn5_nci_prop_ops[5] = {
> {
> .opcode = nci_opcode_pack(NCI_GID_PROPRIETARY,
> NCI_PROP_SET_RFREG),
> @@ -41,6 +44,11 @@ const struct nci_driver_ops s3fwrn5_nci_prop_ops[4] = {
> NCI_PROP_FW_CFG),
> .rsp = s3fwrn5_nci_prop_rsp,
> },
> + {
> + .opcode = nci_opcode_pack(NCI_GID_PROPRIETARY,
> + NCI_PROP_DUAL_OPTION),
> + .rsp = s3fwrn5_nci_prop_rsp,
> + },
> };
>
> #define S3FWRN5_RFREG_SECTION_SIZE 252
> @@ -117,3 +125,112 @@ int s3fwrn5_nci_rf_configure(struct s3fwrn5_info *info, const char *fw_name)
> release_firmware(fw);
> return ret;
> }
> +
> +/*
> + * Configure the reference clock. The S3NRN4V expects the single-byte FW_CFG
> + * form (just the clock-speed selector). The downstream stack sends this in the
> + * bootloader before CORE_RESET; the earliest the mainline NCI core lets us in
> + * is the ->setup hook (after CORE_RESET, before CORE_INIT), which works.
> + */
> +int s3fwrn5_nci_clk_cfg(struct s3fwrn5_info *info)
> +{
> + u8 clk_speed = NCI_PROP_FW_CFG_CLK_SPEED;
> +
> + return nci_prop_cmd(info->ndev, NCI_PROP_FW_CFG, 1, &clk_speed);
> +}
> +
> +/*
> + * S3NRN4V RF register update. The HW and SW register blobs are merged into a
> + * single stream (HW first) and pushed via the DUAL_OPTION command:
> + * START_UPDATE, one SET_OPTION per 252-byte section, then STOP_UPDATE carrying
> + * a 16-bit checksum (running sum of the merged stream as 32-bit words).
> + */
While it's "register update" and function is named "configure_dual", it's
loading firmware.
If it's not a firmware, but only configuration, it can reside inside the driver,
maybe LLM even be able to decode to understandable sequence of registers and values.
For next revision of the patch, I'll likely still have some additional feedback.
With next revision send also as last patch the device-tree entry for the
Fairphone 6, so we can also get additional testing from developers/users.
Thank you again!
David
[...]
^ permalink raw reply
* [PATCH 3/3] usb: typec: Use %pe to print error pointers
From: Subasri S @ 2026-07-19 12:55 UTC (permalink / raw)
To: Peter Chen, Greg Kroah-Hartman, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Duncan Sands,
Chas Williams, Minas Harutyunyan, Hans de Goede, Heikki Krogerus,
Badhri Jagan Sridharan
Cc: linux-usb, imx, linux-arm-kernel, linux-kernel, linux-atm-general,
netdev, Subasri S
In-Reply-To: <20260719-usb-ptr_err_patchset-v1-0-85f7f2e4fefb@gmail.com>
Use the %pe format specifier instead of %ld with PTR_ERR() for printing
error pointers in various typec drivers. This prints symbolic
error names (e.g.-ENOMEM) instead of errno numbers (e.g. -12),
making error logs more readable.
This patch fixes coccinelle reported warnings:
./typec/tcpm/tcpm.c:4799:60-67: WARNING: Consider using %pe to print PTR_ERR()
./typec/mux/pi3usb30532.c:143:3-10: WARNING: Consider using %pe to print PTR_ERR()
./typec/mux/pi3usb30532.c:155:3-10: WARNING: Consider using %pe to print PTR_ERR()
./typec/wusb3801.c:281:5-12: WARNING: Consider using %pe to print PTR_ERR()
Compile tested only.
Signed-off-by: Subasri S <subasris1210@gmail.com>
---
drivers/usb/typec/mux/pi3usb30532.c | 8 ++++----
drivers/usb/typec/tcpm/tcpm.c | 2 +-
drivers/usb/typec/wusb3801.c | 4 ++--
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/typec/mux/pi3usb30532.c b/drivers/usb/typec/mux/pi3usb30532.c
index 985683fe49e9..a2357a28ecf4 100644
--- a/drivers/usb/typec/mux/pi3usb30532.c
+++ b/drivers/usb/typec/mux/pi3usb30532.c
@@ -139,8 +139,8 @@ static int pi3usb30532_probe(struct i2c_client *client)
pi->sw = typec_switch_register(dev, &sw_desc);
if (IS_ERR(pi->sw)) {
- dev_err(dev, "Error registering typec switch: %ld\n",
- PTR_ERR(pi->sw));
+ dev_err(dev, "Error registering typec switch: %pe\n",
+ pi->sw);
return PTR_ERR(pi->sw);
}
@@ -151,8 +151,8 @@ static int pi3usb30532_probe(struct i2c_client *client)
pi->mux = typec_mux_register(dev, &mux_desc);
if (IS_ERR(pi->mux)) {
typec_switch_unregister(pi->sw);
- dev_err(dev, "Error registering typec mux: %ld\n",
- PTR_ERR(pi->mux));
+ dev_err(dev, "Error registering typec mux: %pe\n",
+ pi->mux);
return PTR_ERR(pi->mux);
}
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 89eec20a2064..ad10470ea73b 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -4796,7 +4796,7 @@ static void tcpm_typec_connect(struct tcpm_port *port)
port->partner_desc.accessory = TYPEC_ACCESSORY_NONE;
partner = typec_register_partner(port->typec_port, &port->partner_desc);
if (IS_ERR(partner)) {
- dev_err(port->dev, "Failed to register partner (%ld)\n", PTR_ERR(partner));
+ dev_err(port->dev, "Failed to register partner (%pe)\n", partner);
return;
}
diff --git a/drivers/usb/typec/wusb3801.c b/drivers/usb/typec/wusb3801.c
index 6062875fb04a..86d13ab41670 100644
--- a/drivers/usb/typec/wusb3801.c
+++ b/drivers/usb/typec/wusb3801.c
@@ -277,8 +277,8 @@ static void wusb3801_hw_update(struct wusb3801 *wusb3801)
if (partner_type != WUSB3801_STAT_PARTNER_STANDBY) {
wusb3801->partner = typec_register_partner(port, &desc);
if (IS_ERR(wusb3801->partner))
- dev_err(dev, "Failed to register partner: %ld\n",
- PTR_ERR(wusb3801->partner));
+ dev_err(dev, "Failed to register partner: %pe\n",
+ wusb3801->partner);
}
data_role = pwr_role == TYPEC_SOURCE ? TYPEC_HOST : TYPEC_DEVICE;
--
2.43.0
^ permalink raw reply related
* [PATCH 2/3] usb: misc: Use %pe to print error pointers
From: Subasri S @ 2026-07-19 12:55 UTC (permalink / raw)
To: Peter Chen, Greg Kroah-Hartman, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Duncan Sands,
Chas Williams, Minas Harutyunyan, Hans de Goede, Heikki Krogerus,
Badhri Jagan Sridharan
Cc: linux-usb, imx, linux-arm-kernel, linux-kernel, linux-atm-general,
netdev, Subasri S
In-Reply-To: <20260719-usb-ptr_err_patchset-v1-0-85f7f2e4fefb@gmail.com>
Use the %pe format specifier instead of %ld with PTR_ERR() for printing
error pointers in various drivers across usb subsystem.
This prints symbolic error names (e.g.-ENOMEM) instead of
errno numbers (e.g. -12), making error logs more readable.
This patch fixes coccinelle reported warnings:
./misc/usb3503.c:206:5-12: WARNING: Consider using %pe to print PTR_ERR()
./atm/usbatm.c:983:14-21: WARNING: Consider using %pe to print PTR_ERR()
./core/hub.c:5675:6-13: WARNING: Consider using %pe to print PTR_ERR()
./gadget/function/u_serial.c:1315:24-31: WARNING: Consider using %pe to print PTR_ERR()
./dwc2/pci.c:72:3-10: WARNING: Consider using %pe to print PTR_ERR()
Compile tested only.
Signed-off-by: Subasri S <subasris1210@gmail.com>
---
drivers/usb/atm/usbatm.c | 4 ++--
drivers/usb/core/hub.c | 4 ++--
drivers/usb/dwc2/pci.c | 4 ++--
drivers/usb/gadget/function/u_serial.c | 4 ++--
drivers/usb/misc/usb3503.c | 4 ++--
5 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/atm/usbatm.c b/drivers/usb/atm/usbatm.c
index 9600e1ec0993..c78caa1b57c9 100644
--- a/drivers/usb/atm/usbatm.c
+++ b/drivers/usb/atm/usbatm.c
@@ -979,8 +979,8 @@ static int usbatm_heavy_init(struct usbatm_data *instance)
t = kthread_create(usbatm_do_heavy_init, instance, "%s",
instance->driver->driver_name);
if (IS_ERR(t)) {
- usb_err(instance, "%s: failed to create kernel_thread (%ld)!\n",
- __func__, PTR_ERR(t));
+ usb_err(instance, "%s: failed to create kernel_thread (%pe)!\n",
+ __func__, t);
return PTR_ERR(t);
}
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 5262e11c12cd..5e33e3984499 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -5671,8 +5671,8 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1,
descr = usb_get_device_descriptor(udev);
if (IS_ERR(descr)) {
dev_dbg(&udev->dev,
- "can't read device descriptor %ld\n",
- PTR_ERR(descr));
+ "can't read device descriptor %pe\n",
+ descr);
} else {
if (descriptors_changed(udev, descr,
udev->bos)) {
diff --git a/drivers/usb/dwc2/pci.c b/drivers/usb/dwc2/pci.c
index f3a1e4232a31..9845327f95c5 100644
--- a/drivers/usb/dwc2/pci.c
+++ b/drivers/usb/dwc2/pci.c
@@ -68,8 +68,8 @@ static int dwc2_pci_probe(struct pci_dev *pci,
phy = usb_phy_generic_register();
if (IS_ERR(phy)) {
- dev_err(dev, "error registering generic PHY (%ld)\n",
- PTR_ERR(phy));
+ dev_err(dev, "error registering generic PHY (%pe)\n",
+ phy);
return PTR_ERR(phy);
}
diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
index cdd1dfc666c4..da31e5cea0a5 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1311,8 +1311,8 @@ int gserial_alloc_line_no_console(unsigned char *line_num)
tty_dev = tty_port_register_device(&port->port,
gs_tty_driver, port_num, NULL);
if (IS_ERR(tty_dev)) {
- pr_err("%s: failed to register tty for port %d, err %ld\n",
- __func__, port_num, PTR_ERR(tty_dev));
+ pr_err("%s: failed to register tty for port %d, err %pe\n",
+ __func__, port_num, tty_dev);
ret = PTR_ERR(tty_dev);
mutex_lock(&ports[port_num].lock);
diff --git a/drivers/usb/misc/usb3503.c b/drivers/usb/misc/usb3503.c
index 759770a13260..3ac57d31376c 100644
--- a/drivers/usb/misc/usb3503.c
+++ b/drivers/usb/misc/usb3503.c
@@ -202,8 +202,8 @@ static int usb3503_probe(struct usb3503 *hub)
hub->clk = devm_clk_get_optional(dev, "refclk");
if (IS_ERR(hub->clk)) {
- dev_err(dev, "unable to request refclk (%ld)\n",
- PTR_ERR(hub->clk));
+ dev_err(dev, "unable to request refclk (%pe)\n",
+ hub->clk);
return PTR_ERR(hub->clk);
}
--
2.43.0
^ permalink raw reply related
* [PATCH 1/3] usb: chipidea: Use %pe to print error pointers
From: Subasri S @ 2026-07-19 12:55 UTC (permalink / raw)
To: Peter Chen, Greg Kroah-Hartman, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Duncan Sands,
Chas Williams, Minas Harutyunyan, Hans de Goede, Heikki Krogerus,
Badhri Jagan Sridharan
Cc: linux-usb, imx, linux-arm-kernel, linux-kernel, linux-atm-general,
netdev, Subasri S
In-Reply-To: <20260719-usb-ptr_err_patchset-v1-0-85f7f2e4fefb@gmail.com>
Use the %pe format specifier instead of %ld with PTR_ERR() for printing
error pointers in imx_get_clks(), ci_hdrc_imx_probe(), and
ci_get_platdata(). This prints symbolic error names (e.g. -ENOMEM)
instead of errno numbers (e.g. -12), making error logs more readable.
This patch fixes coccinelle reported warnings:
./chipidea/ci_hdrc_imx.c:452:5-12: WARNING: Consider using %pe to print PTR_ERR()
./chipidea/ci_hdrc_imx.c:468:5-12: WARNING: Consider using %pe to print PTR_ERR()
./chipidea/ci_hdrc_imx.c:222:4-11: WARNING: Consider using %pe to print PTR_ERR()
./chipidea/ci_hdrc_imx.c:222:24-31: WARNING: Consider using %pe to print PTR_ERR()
./chipidea/core.c:684:4-11: WARNING: Consider using %pe to print PTR_ERR()
Compile-tested only.
Signed-off-by: Subasri S <subasris1210@gmail.com>
---
drivers/usb/chipidea/ci_hdrc_imx.c | 12 ++++++------
drivers/usb/chipidea/core.c | 4 ++--
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index 56d2ba824a0b..7bfe37ed68ae 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -218,8 +218,8 @@ static int imx_get_clks(struct device *dev)
if (IS_ERR(data->clk)) {
ret = PTR_ERR(data->clk);
dev_err(dev,
- "Failed to get clks, err=%ld,%ld\n",
- PTR_ERR(data->clk), PTR_ERR(data->clk_ipg));
+ "Failed to get clks, err=%pe,%pe\n",
+ data->clk, data->clk_ipg);
return ret;
}
/* Get wakeup clock. Not all of the platforms need to
@@ -448,8 +448,8 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
pinctrl_hsic_idle = pinctrl_lookup_state(data->pinctrl, "idle");
if (IS_ERR(pinctrl_hsic_idle)) {
dev_err(dev,
- "pinctrl_hsic_idle lookup failed, err=%ld\n",
- PTR_ERR(pinctrl_hsic_idle));
+ "pinctrl_hsic_idle lookup failed, err=%pe\n",
+ pinctrl_hsic_idle);
ret = PTR_ERR(pinctrl_hsic_idle);
goto err_put;
}
@@ -464,8 +464,8 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
"active");
if (IS_ERR(data->pinctrl_hsic_active)) {
dev_err(dev,
- "pinctrl_hsic_active lookup failed, err=%ld\n",
- PTR_ERR(data->pinctrl_hsic_active));
+ "pinctrl_hsic_active lookup failed, err=%pe\n",
+ data->pinctrl_hsic_active);
ret = PTR_ERR(data->pinctrl_hsic_active);
goto err_put;
}
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 07563be0013f..09db8a4eace2 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -680,8 +680,8 @@ static int ci_get_platdata(struct device *dev,
/* no vbus regulator is needed */
platdata->reg_vbus = NULL;
} else if (IS_ERR(platdata->reg_vbus)) {
- dev_err(dev, "Getting regulator error: %ld\n",
- PTR_ERR(platdata->reg_vbus));
+ dev_err(dev, "Getting regulator error: %pe\n",
+ platdata->reg_vbus);
return PTR_ERR(platdata->reg_vbus);
}
/* Get TPL support */
--
2.43.0
^ permalink raw reply related
* [PATCH 0/3] usb: Use %pe to print error pointers
From: Subasri S @ 2026-07-19 12:55 UTC (permalink / raw)
To: Peter Chen, Greg Kroah-Hartman, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Duncan Sands,
Chas Williams, Minas Harutyunyan, Hans de Goede, Heikki Krogerus,
Badhri Jagan Sridharan
Cc: linux-usb, imx, linux-arm-kernel, linux-kernel, linux-atm-general,
netdev, Subasri S
This patchset replaces PTR_ERR() in format strings with
%pe format specifier across the USB subsystem. %pe prints
symbolic error names (e.g., -ENOMEM) instead of
raw numbers (e.g., -12), making error logs more readable.
These patches fix coccinelle reported warning: "Consider
using %pe to print PTR_ERR()" in usb subsystem.
The patches are grouped by subsystem:
1/3 - chipidea
2/3 - miscellaneous USB drivers
3/3 - typec
All patches are compile-tested on x86_64. Please let me know if
any more testing is needed for any patch.
Signed-off-by: Subasri S <subasris1210@gmail.com>
---
Subasri S (3):
usb: chipidea: Use %pe to print error pointers
usb: misc: Use %pe to print error pointers
usb: typec: Use %pe to print error pointers
drivers/usb/atm/usbatm.c | 4 ++--
drivers/usb/chipidea/ci_hdrc_imx.c | 12 ++++++------
drivers/usb/chipidea/core.c | 4 ++--
drivers/usb/core/hub.c | 4 ++--
drivers/usb/dwc2/pci.c | 4 ++--
drivers/usb/gadget/function/u_serial.c | 4 ++--
drivers/usb/misc/usb3503.c | 4 ++--
drivers/usb/typec/mux/pi3usb30532.c | 8 ++++----
drivers/usb/typec/tcpm/tcpm.c | 2 +-
drivers/usb/typec/wusb3801.c | 4 ++--
10 files changed, 25 insertions(+), 25 deletions(-)
---
base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
change-id: 20260718-usb-ptr_err_patchset-7c5d1547bee7
Best regards,
--
Subasri S <subasris1210@gmail.com>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox