* Re: [PATCH net v1 0/3] net: fix stale TX skb pointers on DMA map failure
From: Simon Horman @ 2026-07-16 12:00 UTC (permalink / raw)
To: xuanqiang.luo
Cc: netdev, Xuanqiang Luo, Rasesh Mody, Sudarsana Kalluru,
GR-Linux-NIC-Dev, Fan Gong, Xin Guo, Gur Stavi, Jijie Shao,
Jian Shen, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Ivan Vecera, linux-kernel
In-Reply-To: <20260710090527.58354-1-xuanqiang.luo@linux.dev>
On Fri, Jul 10, 2026 at 05:05:21PM +0800, xuanqiang.luo@linux.dev wrote:
> From: Xuanqiang Luo <luoxuanqiang@kylinos.cn>
>
> While I was backporting commit 1a303baa715e6 ("ice: fix double-free of
> tx_buf skb"), an AI-assisted scan identified several suspected TX error
> paths. I reviewed the results and found this issue in the three drivers
> fixed here.
>
> The drivers differ, but the bug is the same. On a DMA mapping failure, the
> TX path frees an skb while its ring entry still points to it. A later
> transmission normally overwrites the entry. If the interface is stopped
> first, teardown can instead access or free the skb again.
>
> I do not have these adapters, so I have not tested the drivers on hardware.
> I checked the error and teardown paths by inspection. Still, these small
> fixes seem worth posting for review. They are independent, but are sent as
> one series because they address the same issue.
>
> Xuanqiang Luo (3):
> bna: fix use-after-free on DMA mapping failure
> hinic3: fix use-after-free on DMA mapping failure
> net: hibmcge: fix double-free of tx skb on DMA mapping failure
For the series:
Reviewed-by: Simon Horman <horms@kernel.org>
^ permalink raw reply
* [PATCH v3] virtio_ring: fix infinite loop in virtnet_poll_cleantx when device is broken
From: Jinqian Yang @ 2026-07-16 11:59 UTC (permalink / raw)
To: mst, jasowang, xuanzhuo, eperezma, andrew+netdev, davem, edumazet,
kuba, pabeni
Cc: netdev, virtualization, linux-kernel, liuyonglong, wangzhou1,
linuxarm, Jinqian Yang
virtnet_poll_cleantx() contains a do-while loop that cleans up
transmitted TX buffers and calls virtqueue_enable_cb_delayed() to check
whether more buffers need processing. When the virtio backend stops
responding during guest reboot, used->idx is never updated, so
virtqueue_enable_cb_delayed() always returns false and the loop never
terminates. Then it will block reboot process, and the guest will hang.
The problem occurs during guest reboot under network traffic:
1. kernel_restart() -> device_shutdown() traverses the device list
2. virtio_dev_shutdown() calls virtio_break_device() which sets
vq->broken = true
3. virtio_dev_shutdown() then calls virtio_synchronize_cbs() to wait
for in-flight callbacks to complete
4. A virtio interrupt fires, softirq is deferred to ksoftirqd which
calls net_rx_action() -> virtnet_poll() -> virtnet_poll_cleantx()
5. virtnet_poll_cleantx() enters the do-while loop and never exits
because the QEMU backend has stopped updating used->idx, despite
vq->broken having been set to true in step 2.
Since the loop runs inside ksoftirqd (a SCHED_OTHER kthread), it is
visible to the scheduler and does not trigger a hard lockup. However,
the kthread never leaves the loop, so RCU detects it as a CPU stall
and reports it periodically. Meanwhile, the reboot process remains
blocked in device_shutdown() because virtio_dev_shutdown() cannot
complete its synchronization step, and the guest hangs permanently.
This can be reproduced on a guest with a virtio-net device: run iperf3
traffic in the guest, then trigger reboot. The reboot occasionally hangs
permanently with RCU stall on ksoftirqd.
Observed on ARM64 KVM guest:
CPU#1 RCU stall (ksoftirqd/1), repeated periodically:
virtqueue_enable_cb_delayed_split <- virtnet_poll <- __napi_poll <-
net_rx_action <- handle_softirqs <- run_ksoftirqd <-
smpboot_thread_fn <- kthread
Fix by adding a vq->broken check in virtqueue_enable_cb_delayed(), so
that the loop exits immediately when the device is broken, allowing
the device shutdown to proceed.
Signed-off-by: Jinqian Yang <yangjinqian1@huawei.com>
---
Changes in v2:
- Moved vq->broken check to virtqueue_enable_cb_delayed().
Changes in v3:
- Updated the patch subject prefix.
v1: https://lore.kernel.org/lkml/20260713132025.703147-1-yangjinqian1@huawei.com/
v2: https://lore.kernel.org/lkml/20260716035201.3736582-1-yangjinqian1@huawei.com/
---
drivers/virtio/virtio_ring.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index b438dc2ce1b8..5c169fbb418a 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -3233,6 +3233,14 @@ bool virtqueue_enable_cb_delayed(struct virtqueue *_vq)
{
struct vring_virtqueue *vq = to_vvq(_vq);
+ /*
+ * When the device is broken there is no point in polling used->idx,
+ * the backend will never update it. Return true to let callers
+ * exit their cleanup loops instead of spinning forever.
+ */
+ if (unlikely(vq->broken))
+ return true;
+
if (vq->event_triggered)
data_race(vq->event_triggered = false);
--
2.33.0
^ permalink raw reply related
* [PATCH iproute2-next v7 2/2] rdma: display resource limits in curr/max format
From: Tao Cui @ 2026-07-16 11:52 UTC (permalink / raw)
To: dsahern, leonro; +Cc: linux-rdma, netdev, Tao Cui
In-Reply-To: <20260716115237.1859633-1-cui.tao@linux.dev>
From: Tao Cui <cuitao@kylinos.cn>
Parse the new RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_MAX netlink attribute
to show resource limits alongside current counts in curr/max format:
Before: 0: mlx5_0: qp 123 cq 45 mr 200 pd 10
After: 0: mlx5_0: qp 123/131072 cq 45/65536 mr 200/1000000 pd 10/32768
JSON output provides separate <name> and <name>-max fields per resource
type, so scripts can compute usage against the device limit; resources
without an upper bound (e.g. cm_id, ctx) are reported without a -max
field:
$ rdma resource show -j
[{
"ifindex": 0,
"ifname": "mlx5_0",
"qp": 123, "qp-max": 131072,
"cq": 45, "cq-max": 65536,
"mr": 200, "mr-max": 1000000,
"pd": 10, "pd-max": 32768,
"cm_id": 0, "ctx": 0
}]
Backward compatible: no output change when the kernel lacks the new
attribute.
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
Link: https://lore.kernel.org/all/20260615003646.168704-1-cui.tao@linux.dev/
---
rdma/res.c | 16 +++++++++++++++-
rdma/utils.c | 1 +
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/rdma/res.c b/rdma/res.c
index 062f0007..7efced8d 100644
--- a/rdma/res.c
+++ b/rdma/res.c
@@ -55,7 +55,21 @@ static int res_print_summary(struct nlattr **tb)
name = mnl_attr_get_str(nla_line[RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_NAME]);
curr = mnl_attr_get_u64(nla_line[RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_CURR]);
- res_print_u64(name, curr, nla_line[RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_CURR]);
+ if (nla_line[RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_MAX]) {
+ uint64_t max;
+ char max_name[64];
+
+ max = mnl_attr_get_u64(
+ nla_line[RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_MAX]);
+ snprintf(max_name, sizeof(max_name), "%s-max", name);
+ print_u64(PRINT_JSON, name, NULL, curr);
+ print_u64(PRINT_JSON, max_name, NULL, max);
+ if (!is_json_context())
+ pr_out("%s %" PRIu64 "/%" PRIu64 " ", name, curr, max);
+ } else {
+ res_print_u64(name, curr,
+ nla_line[RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_CURR]);
+ }
}
return 0;
}
diff --git a/rdma/utils.c b/rdma/utils.c
index 87003b2c..90ea1c55 100644
--- a/rdma/utils.c
+++ b/rdma/utils.c
@@ -480,6 +480,7 @@ static const enum mnl_attr_data_type nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
[RDMA_NLDEV_ATTR_EVENT_TYPE] = MNL_TYPE_U8,
[RDMA_NLDEV_SYS_ATTR_MONITOR_MODE] = MNL_TYPE_U8,
[RDMA_NLDEV_ATTR_STAT_OPCOUNTER_ENABLED] = MNL_TYPE_U8,
+ [RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_MAX] = MNL_TYPE_U64,
};
static int rd_attr_check(const struct nlattr *attr, int *typep)
--
2.43.0
^ permalink raw reply related
* [PATCH iproute2-next v7 1/2] rdma: update uapi headers
From: Tao Cui @ 2026-07-16 11:52 UTC (permalink / raw)
To: dsahern, leonro; +Cc: linux-rdma, netdev, Tao Cui
In-Reply-To: <20260716115237.1859633-1-cui.tao@linux.dev>
From: Tao Cui <cuitao@kylinos.cn>
Update rdma_netlink.h file upto kernel commit 5911f6d6e7cc
("RDMA/nldev: Add resource summary max values for usage display")
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
rdma/include/uapi/rdma/rdma_netlink.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/rdma/include/uapi/rdma/rdma_netlink.h b/rdma/include/uapi/rdma/rdma_netlink.h
index 4356ec4a..e5b8b065 100644
--- a/rdma/include/uapi/rdma/rdma_netlink.h
+++ b/rdma/include/uapi/rdma/rdma_netlink.h
@@ -604,6 +604,11 @@ enum rdma_nldev_attr {
RDMA_NLDEV_ATTR_FRMR_POOL_PINNED_HANDLES, /* u32 */
RDMA_NLDEV_ATTR_FRMR_POOL_KEY_KERNEL_VENDOR_KEY, /* u64 */
+ /*
+ * Resource summary entry maximum value.
+ */
+ RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_MAX, /* u64 */
+
/*
* Always the end
*/
--
2.43.0
^ permalink raw reply related
* [PATCH iproute2-next v7 0/2] rdma: display resource limits in curr/max format
From: Tao Cui @ 2026-07-16 11:52 UTC (permalink / raw)
To: dsahern, leonro; +Cc: linux-rdma, netdev, Tao Cui
From: Tao Cui <cuitao@kylinos.cn>
This series adds support for displaying RDMA device resource limits in
curr/max format in the rdma tool, building on the kernel uapi attribute
RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_MAX which has landed in linux-next
(kernel commit 5911f6d6e7cc [1]).
Patch 1 syncs the rdma_netlink.h uapi header from the kernel.
Patch 2 updates the rdma tool to parse the new attribute and display
resource usage in curr/max format:
$ rdma resource show
0: mlx5_0: qp 123/131072 cq 45/65536 mr 200/1000000 pd 10/32768
[1]:
Link: https://lore.kernel.org/all/20260615003646.168704-1-cui.tao@linux.dev/
Tao Cui (2):
rdma: update uapi headers
rdma: display resource limits in curr/max format
rdma/include/uapi/rdma/rdma_netlink.h | 5 +++++
rdma/res.c | 16 +++++++++++++++-
rdma/utils.c | 1 +
3 files changed, 21 insertions(+), 1 deletion(-)
---
Changes in v7:
- Drop the unnecessary intermediate buffer in res_print_summary() and
pass the format string to pr_out() directly (claude review).
- Add a JSON output example to the tool commit message.
Changes in v6:
- Fix the incorrect kernel commit SHA referenced in the uapi headers
sync message (5911f6d6e7cc, not 5911f6d6e7ce).
Changes in v5:
- Split the rdma_netlink.h update into its own commit following the
uapi headers sync format ("rdma: update uapi headers").
Changes in v4:
- Add a Link: reference to the kernel patch; re-send now that the
kernel side has landed in linux-next.
--
2.43.0
^ permalink raw reply
* Re: [PATCH v3 net 1/6] xsk: fix buffer leak in xsk_drop_skb() for AF_XDP multi-buffer Tx
From: Maciej Fijalkowski @ 2026-07-16 11:52 UTC (permalink / raw)
To: Jason Xing
Cc: netdev, bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
bjorn, Jason Xing
In-Reply-To: <CAL+tcoBGJDC-Kx=qvxoQtd4BUPCDBv00Gta1SOvejx9nNvVo3w@mail.gmail.com>
On Thu, Jul 16, 2026 at 01:42:28PM +0200, Jason Xing wrote:
> On Thu, Jul 16, 2026 at 1:25 PM Maciej Fijalkowski
> <maciej.fijalkowski@intel.com> wrote:
> >
> > On Thu, Jul 16, 2026 at 01:22:24PM +0200, Jason Xing wrote:
> > > On Tue, Jul 14, 2026 at 4:08 PM Maciej Fijalkowski
> > > <maciej.fijalkowski@intel.com> wrote:
> > > >
> > > > 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")
> > > > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > >
> > > Maciej, maybe your tag is missing here?
> >
> > IIRC this has been taken as-is from your patchset. The next one has been
> > touched in few ways and I included my co-developed tag there.
>
> Yep, I mean maybe you can simply drop your reviewed-by or acked-by tag
> here if you approve.
I see. I am not sure what are principles for such cases, I could assume
that if I include this as a sender then it implies my ack on such change.
Anyways I can just:
Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
BTW now I reminded myself I dropped Stan's tags in whole set. I think it
was of changes on 4/6, otherwise I don't have other excuse.
So Jason if you bump at Stan somewhere in the netdevconf's hallway could
you ask him to re-ack it if it still works for him?
>
> Thanks,
> Jason
>
> >
> > >
> > > Thanks,
> > > Jason
^ permalink raw reply
* Re: [PATCH v3 net 1/6] xsk: fix buffer leak in xsk_drop_skb() for AF_XDP multi-buffer Tx
From: Jason Xing @ 2026-07-16 11:42 UTC (permalink / raw)
To: Maciej Fijalkowski
Cc: netdev, bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
bjorn, Jason Xing
In-Reply-To: <ali/sIjljiNm9RmI@boxer>
On Thu, Jul 16, 2026 at 1:25 PM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Thu, Jul 16, 2026 at 01:22:24PM +0200, Jason Xing wrote:
> > On Tue, Jul 14, 2026 at 4:08 PM Maciej Fijalkowski
> > <maciej.fijalkowski@intel.com> wrote:
> > >
> > > 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")
> > > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> >
> > Maciej, maybe your tag is missing here?
>
> IIRC this has been taken as-is from your patchset. The next one has been
> touched in few ways and I included my co-developed tag there.
Yep, I mean maybe you can simply drop your reviewed-by or acked-by tag
here if you approve.
Thanks,
Jason
>
> >
> > Thanks,
> > Jason
^ permalink raw reply
* [PATCH net v2] sctp: don't free the ASCONF's own transport in DEL-IP processing
From: =?gb18030?B?1uzIuMC2vvxBSUdCb3TTys/k?= @ 2026-07-16 11:39 UTC (permalink / raw)
To: netdev
Cc: Marcelo Ricardo Leitner, Xin Long, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, linux-sctp,
linux-kernel, stable, Jun Yang, stable
sctp_process_asconf() caches the transport the ASCONF chunk is processed
against in asconf->transport (== chunk->transport, set once in sctp_rcv()).
For an ASCONF located through its Address Parameter by
__sctp_rcv_asconf_lookup(), that cached transport corresponds to the
Address Parameter, which need not be the packet's source address.
sctp_process_asconf_param() rejects a DEL-IP for the packet source address
(ADDIP D8, SCTP_ERROR_DEL_SRC_IP), but nothing protects asconf->transport.
A single ASCONF can therefore carry, in order:
[Address Parameter L] [DEL-IP L] [DEL-IP 0.0.0.0]
where L differs from the source. The DEL-IP for L passes the D8 check and
calls sctp_assoc_rm_peer() on the transport that asconf->transport still
points at, freeing it (RCU-deferred). The following wildcard DEL-IP then
reuses the now-dangling asconf->transport in sctp_assoc_set_primary() and
sctp_assoc_del_nonprimary_peers(): set_primary() dereferences the freed
transport (->ipaddr, ->state) and plants the dangling pointer into
asoc->peer.primary_path / active_path, and del_nonprimary_peers(), keeping
only the pointer that is no longer on the list, removes every real
transport, leaving the association with a transport_count of 0 and
primary_path/active_path pointing at freed memory.
Reject a DEL-IP that targets the transport the ASCONF is being processed
against, mirroring the existing source-address guard, so the wildcard
branch can never reuse a freed transport.
Fixes: 42e30bf3463c ("[SCTP]: Handle the wildcard ADD-IP Address parameter")
Cc: stable@vger.kernel.org
Signed-off-by: Jun Yang <junvyyang@tencent.com>
Acked-by: Xin Long <lucien.xin@gmail.com>
---
v2: add [net] subject prefix to target the net tree.
net/sctp/sm_make_chunk.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 8adac9e0cd66..b14251214896 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -3153,6 +3153,12 @@ static __be16 sctp_process_asconf_param(struct sctp_association *asoc,
if (!peer)
return SCTP_ERROR_DNS_FAILED;
+ /* Don't free asconf->transport; a later wildcard DEL-IP
+ * parameter reuses it.
+ */
+ if (peer == asconf->transport)
+ return SCTP_ERROR_DEL_SRC_IP;
+
sctp_assoc_rm_peer(asoc, peer);
break;
case SCTP_PARAM_SET_PRIMARY:
--
2.55.0
^ permalink raw reply related
* [PATCH net-next v2 2/2] net: phy: add DAPU Telecom DAP8210R(I) Gigabit Ethernet PHY driver
From: Artem Shimko @ 2026-07-16 11:38 UTC (permalink / raw)
To: netdev, Andrew Lunn, Heiner Kallweit, Russell King,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Artem Shimko, linux-kernel, devicetree
In-Reply-To: <20260716113805.593215-1-a.shimko.dev@gmail.com>
Add a new PHY driver for the DAPU Telecom DAP8211R(I) Gigabit
Ethernet PHY, which is commonly used in enterprise and industrial
networking applications.
The driver implements extended register access via indirect addressing
through corresponding registers, and provides comprehensive device tree
support for RGMII delay configuration. The rx-internal-delay-ps and
tx-internal-delay-ps properties allow precise tuning of clock delays in
150 ps steps from 0 to 2250 ps. Additionally, the optional
dapu,tx-inverted-clk flag enables 180-degree TX clock phase shift for
boards where signal integrity or MAC requirements necessitate clock
inversion.
Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
---
drivers/net/phy/Kconfig | 10 ++
drivers/net/phy/Makefile | 1 +
drivers/net/phy/dap8211r.c | 281 +++++++++++++++++++++++++++++++++++++
3 files changed, 292 insertions(+)
create mode 100644 drivers/net/phy/dap8211r.c
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 099f25dceabb..4576f707ac94 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -237,6 +237,16 @@ config DAVICOM_PHY
help
Currently supports dm9161e and dm9131
+config DAP8211R_PHY
+ tristate "DAPU Telecom DAP8211R(I) Gigabit Ethernet PHY"
+ depends on OF
+ help
+ Support for the DAPU Telecom DAP8211R(I) Gigabit Ethernet PHY.
+ This PHY is designed for enterprise and industrial networking
+ applications, supporting 10/100/1000 Mbps operation.
+ RGMII with: configurable TX/RX clock delays, optional flag to enable
+ 180-degree TX clock phase shift and internal packet generator.
+
config ICPLUS_PHY
tristate "ICPlus PHYs"
help
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index de660ae94945..ad35733eb4bb 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -53,6 +53,7 @@ obj-$(CONFIG_BROADCOM_PHY) += broadcom.o
obj-$(CONFIG_CICADA_PHY) += cicada.o
obj-$(CONFIG_CORTINA_PHY) += cortina.o
obj-$(CONFIG_DAVICOM_PHY) += davicom.o
+obj-$(CONFIG_DAP8211R_PHY) += dap8211r.o
obj-$(CONFIG_DP83640_PHY) += dp83640.o
obj-$(CONFIG_DP83822_PHY) += dp83822.o
obj-$(CONFIG_DP83848_PHY) += dp83848.o
diff --git a/drivers/net/phy/dap8211r.c b/drivers/net/phy/dap8211r.c
new file mode 100644
index 000000000000..e1e6a322ef0c
--- /dev/null
+++ b/drivers/net/phy/dap8211r.c
@@ -0,0 +1,281 @@
+// SPDX-License-Identifier: GPL
+/*
+ * Driver for the DAPU Telecom DAP8211R(I) Gigabit Ethernet PHY.
+ *
+ * Specifications:
+ * - IEEE 802.3 10BASE-Te, 100BASE-TX, 1000BASE-T
+ * - IEEE 802.3az-2010 Energy Efficient Ethernet
+ * - IEEE 1588 SyncE support
+ * - RGMII
+ *
+ * Author: Artem Shimko <a.shimko.dev@gmail.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/errno.h>
+#include <linux/ethtool.h>
+#include <linux/kernel.h>
+#include <linux/mii.h>
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/of.h>
+#include <linux/phy.h>
+
+#define DAP8211R_PHY_ID 0x0008011B
+#define DAP8211R_PHY_ID_MASK 0xFFFFFFFF
+
+#define DAP8211R_EXT_ADD 0x1E
+#define DAP8211R_EXT_DATA 0x1F
+
+#define DAP8211R_PHY_CON 0xA001
+#define DAP8211R_PHY_SW_RST BIT(15)
+
+#define DAP8211R_RGMII_CON 0xA003
+#define DAP8211R_RGMII_TX_DEL_MASK GENMASK(3, 0)
+#define DAP8211R_RGMII_RX_DEL_MASK GENMASK(13, 10)
+#define DAP8211R_RGMII_CLK_INVERT BIT(14)
+
+/* Default RGMII delay: 13 * 150 == 1.95ns */
+#define DAP8211R_DEFAULT_DELAY_SEL 0xD
+
+struct dap8211r_delay_config {
+ u32 ps;
+ u8 sel;
+};
+
+static const struct dap8211r_delay_config delay_config[] = {
+ { 0, 0},
+ { 150, 1},
+ { 300, 2},
+ { 450, 3},
+ { 600, 4},
+ { 750, 5},
+ { 900, 6},
+ {1050, 7},
+ {1200, 8},
+ {1350, 9},
+ {1500, 10},
+ {1650, 11},
+ {1800, 12},
+ {1950, 13},
+ {2100, 14},
+ {2250, 15},
+};
+
+#define DAP8211R_DELAY_COUNT ARRAY_SIZE(delay_config)
+
+/**
+ * dap8211r_delay_ps_to_sel() - Convert ps to register value (exact match only)
+ * @ps: Delay in picoseconds
+ *
+ * Converts a delay value in picoseconds to the corresponding register value
+ * for RGMII delay configuration. The PHY supports specific values from
+ * 0 to 2250 ps in 150 ps steps.
+ *
+ * Return: Register value (0-15) on success, -EINVAL if @ps is not supported.
+ */
+
+static int dap8211r_delay_ps_to_sel(u32 ps)
+{
+ for (int i = 0; i < DAP8211R_DELAY_COUNT; i++)
+ if (ps == delay_config[i].ps)
+ return delay_config[i].sel;
+
+ return -EINVAL;
+}
+
+/**
+ * dap8211r_read_ext() - Read extended register
+ * @phydev: PHY device structure
+ * @reg: Extended register address
+ *
+ * Reads a PHY extended register using the indirect access method.
+ * The caller must hold the MDIO bus lock.
+ *
+ * Return: Register value on success, or negative error code
+ */
+static int dap8211r_read_ext(struct phy_device *phydev, u16 reg)
+{
+ int ret;
+
+ phy_lock_mdio_bus(phydev);
+ ret = __phy_write(phydev, DAP8211R_EXT_ADD, reg);
+ if (ret < 0)
+ goto out;
+
+ ret = __phy_read(phydev, DAP8211R_EXT_DATA);
+out:
+ phy_unlock_mdio_bus(phydev);
+ return ret;
+}
+
+/**
+ * dap8211r_modify_ext() - Modify extended register bits
+ * @phydev: PHY device structure
+ * @reg: Extended register address
+ * @mask: Bit mask of bits to clear
+ * @set: Bit mask of bits to set
+ *
+ * Modifies a PHY extended register using the indirect access method.
+ * New value = (old value & ~mask) | set.
+ * The caller must hold the MDIO bus lock.
+ *
+ * Return: 0 on success, or negative error code
+ */
+static int dap8211r_modify_ext(struct phy_device *phydev, u16 reg, u16 mask, u16 set)
+{
+ int ret;
+
+ phy_lock_mdio_bus(phydev);
+ ret = __phy_write(phydev, DAP8211R_EXT_ADD, reg);
+ if (ret < 0)
+ goto out;
+
+ ret = __phy_modify(phydev, DAP8211R_EXT_DATA, mask, set);
+out:
+ phy_unlock_mdio_bus(phydev);
+ return ret;
+}
+
+/**
+ * dap8211r_get_rgmii_delay() - Get RGMII delay from DT
+ * @phydev: PHY device
+ * @prop_name: DT property name
+ * @is_id: If phy mode is PHY_INTERFACE_MODE_RGMII_[TXID,RXID,ID]
+ *
+ * Reads the RGMII delay from the device tree. If the property is not
+ * specified, the default delay (1950ps) is used.
+ *
+ * Return: Register value (0-15) on success, negative error code on failure.
+ * -EINVAL: Property not specified and is_id is false.
+ */
+static int dap8211r_get_rgmii_delay(struct phy_device *phydev, const char *prop_name, bool is_id)
+{
+ struct device_node *np = phydev->mdio.dev.of_node;
+ u32 ps = 0;
+ int ret;
+
+ ret = of_property_read_u32(np, prop_name, &ps);
+ if (ret == -EINVAL)
+ return (is_id) ? DAP8211R_DEFAULT_DELAY_SEL : ret;
+ if (ret < 0)
+ return ret;
+
+ return dap8211r_delay_ps_to_sel(ps);
+}
+
+/**
+ * dap8211r_config_init() - Initialize PHY
+ * @phydev: PHY device structure
+ *
+ * Configures the PHY during initialization:
+ * - RGMII delays based on interface mode
+ * - TX clock invertion
+ * - Software reset to apply settings (low active, self clear)
+ *
+ * Return: 0 on success, or negative error code
+ */
+static int dap8211r_config_init(struct phy_device *phydev)
+{
+ struct device_node *phydev_node = phydev->mdio.dev.of_node;
+ u16 mask = 0, set = 0;
+ int ret, retries = 10;
+
+ switch (phydev->interface) {
+ case PHY_INTERFACE_MODE_RGMII:
+ ret = dap8211r_get_rgmii_delay(phydev, "rx-internal-delay-ps", false);
+ if (ret >= 0) {
+ set = FIELD_PREP(DAP8211R_RGMII_RX_DEL_MASK, ret);
+ mask = DAP8211R_RGMII_RX_DEL_MASK;
+ } else if ((ret < 0) && (ret != -EINVAL)) {
+ return ret;
+ }
+
+ ret = dap8211r_get_rgmii_delay(phydev, "tx-internal-delay-ps", false);
+ if (ret >= 0) {
+ set |= FIELD_PREP(DAP8211R_RGMII_TX_DEL_MASK, ret);
+ mask |= DAP8211R_RGMII_TX_DEL_MASK;
+ } else if ((ret < 0) && (ret != -EINVAL)) {
+ return ret;
+ }
+ break;
+ case PHY_INTERFACE_MODE_RGMII_RXID:
+ ret = dap8211r_get_rgmii_delay(phydev, "rx-internal-delay-ps", true);
+ if (ret < 0)
+ return ret;
+
+ set = FIELD_PREP(DAP8211R_RGMII_RX_DEL_MASK, ret);
+ mask = DAP8211R_RGMII_RX_DEL_MASK;
+ break;
+ case PHY_INTERFACE_MODE_RGMII_ID:
+ ret = dap8211r_get_rgmii_delay(phydev, "rx-internal-delay-ps", true);
+ if (ret < 0)
+ return ret;
+
+ set = FIELD_PREP(DAP8211R_RGMII_RX_DEL_MASK, ret);
+ mask = DAP8211R_RGMII_RX_DEL_MASK;
+ fallthrough;
+ case PHY_INTERFACE_MODE_RGMII_TXID:
+ ret = dap8211r_get_rgmii_delay(phydev, "tx-internal-delay-ps", true);
+ if (ret < 0)
+ return ret;
+
+ set |= FIELD_PREP(DAP8211R_RGMII_TX_DEL_MASK, ret);
+ mask |= DAP8211R_RGMII_TX_DEL_MASK;
+ break;
+ default:
+ phydev_err(phydev, "Unsupported interface: %d\n",
+ phydev->interface);
+ return -EINVAL;
+ }
+
+ if (of_property_read_bool(phydev_node, "dapu,tx-inverted-clk"))
+ set |= DAP8211R_RGMII_CLK_INVERT;
+
+ mask |= DAP8211R_RGMII_CLK_INVERT;
+
+ ret = dap8211r_modify_ext(phydev, DAP8211R_PHY_CON, DAP8211R_PHY_SW_RST, 0);
+ if (ret)
+ return ret;
+
+ /* Wait for reset self-clear */
+ do {
+ fsleep(20);
+ ret = dap8211r_read_ext(phydev, DAP8211R_PHY_CON);
+ if (ret < 0)
+ return ret;
+ } while (!(ret & DAP8211R_PHY_SW_RST) && --retries);
+
+ if (!retries)
+ return -ETIMEDOUT;
+
+ ret = dap8211r_modify_ext(phydev, DAP8211R_RGMII_CON, mask, set);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static struct phy_driver dap8211r_driver[] = {
+ {
+ PHY_ID_MATCH_EXACT(DAP8211R_PHY_ID),
+ .name = "DAP8211R Gigabit Ethernet",
+ .config_init = dap8211r_config_init,
+ .read_status = genphy_read_status,
+ .set_loopback = genphy_loopback,
+ .config_aneg = genphy_config_aneg,
+ .suspend = genphy_suspend,
+ .resume = genphy_resume,
+ },
+};
+module_phy_driver(dap8211r_driver);
+
+MODULE_DESCRIPTION("DAP8211R Gigabit Ethernet PHY driver");
+MODULE_AUTHOR("Artem Shimko <a.shimko.dev@gmail.com>");
+MODULE_LICENSE("GPL");
+
+static const struct mdio_device_id __maybe_unused dap8211r_tb[] = {
+ { DAP8211R_PHY_ID, DAP8211R_PHY_ID_MASK },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(mdio, dap8211r_tb);
--
2.43.0
^ permalink raw reply related
* [PATCH net-next v2 1/2] dt-bindings: net: add DAPU Telecom DAP8211R(I) PHY binding
From: Artem Shimko @ 2026-07-16 11:38 UTC (permalink / raw)
To: netdev, Andrew Lunn, Heiner Kallweit, Russell King,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Artem Shimko, linux-kernel, devicetree
In-Reply-To: <20260716113805.593215-1-a.shimko.dev@gmail.com>
Add device tree binding documentation for the DAPU Telecom DAP8211R(I)
Gigabit Ethernet PHY.
The PHY supports TX and RX clock delays in 150 ps steps from 0 to 2250 ps,
with a default of 1950 ps if not specified. The dapu,tx-inverted-clk flag
provides a vendor-specific extension for boards where PCB trace length or
MAC requirements necessitate 180-degree clock phase shift.
Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
---
.../bindings/net/dapu,dap8211r.yaml | 73 +++++++++++++++++++
1 file changed, 73 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/dapu,dap8211r.yaml
diff --git a/Documentation/devicetree/bindings/net/dapu,dap8211r.yaml b/Documentation/devicetree/bindings/net/dapu,dap8211r.yaml
new file mode 100644
index 000000000000..d4012fa17a1e
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/dapu,dap8211r.yaml
@@ -0,0 +1,73 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/dapu,dap8211r.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: DAPU Telecom DAP8211R(I) Gigabit Ethernet PHY
+
+maintainers:
+ - Artem Shimko <a.shimko.dev@gmail.com>
+
+description: |
+ The DAP8211R(I) is a Gigabit Ethernet PHY with RGMII interface,
+ supporting IEEE 802.3az Energy Efficient Ethernet, IEEE 1588 SyncE,
+ and an internal packet generator for diagnostics.
+
+ Specifications:
+ - 10BASE-Te, 100BASE-TX, 1000BASE-T
+ - RGMII with configurable TX/RX clock delays (150 ps steps, 0-2250 ps)
+ - IEEE 802.3az-2010 Energy Efficient Ethernet
+ - IEEE 1588 SyncE support
+ - Internal packet generator and checker for link diagnostics
+
+allOf:
+ - $ref: ethernet-phy.yaml#
+
+properties:
+ compatible:
+ const: ethernet-phy-id0008.011b
+
+ reg:
+ maxItems: 1
+
+ rx-internal-delay-ps:
+ description:
+ RGMII RX clock delay in picoseconds (0 to maximum).
+ multipleOf: 150
+ maximum: 2250
+ default: 1950
+
+ tx-internal-delay-ps:
+ description:
+ RGMII TX clock delay in picoseconds (0 to maximum).
+ multipleOf: 150
+ maximum: 2250
+ default: 1950
+
+ dapu,tx-inverted-clk:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ If present, the RGMII TX clock to the MAC is inverted (180 degree
+ phase shift relative to the data lines). This is a vendor-specific
+ extension for boards where PCB trace length or MAC requirements
+ necessitate clock inversion. Only use this property after hardware
+ signal integrity validation.
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ethernet-phy@1 {
+ compatible = "ethernet-phy-id0008.011b";
+ reg = <1>;
+ rx-internal-delay-ps = <2100>;
+ tx-internal-delay-ps = <2100>;
+ dapu,tx-inverted-clk;
+ };
+ };
+
--
2.43.0
^ permalink raw reply related
* [PATCH net-next v2 0/2] Add DAPU Telecom DAP8211R(I) Gigabit Ethernet PHY driver
From: Artem Shimko @ 2026-07-16 11:38 UTC (permalink / raw)
To: netdev, Andrew Lunn, Heiner Kallweit, Russell King,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: Artem Shimko, linux-kernel, devicetree
Hello,
This series adds support for the DAPU Telecom DAP8211R(I) Gigabit
Ethernet PHY, commonly used in enterprise and industrial networking
applications. The PHY supports 10/100/1000 Mbps operation with RGMII
interface and includes features such as IEEE 802.3az Energy Efficient
Ethernet, IEEE 1588 SyncE, and an internal packet generator for
diagnostics.
The driver implements extended register access via indirect addressing
(registers 0x1E/0x1F) and provides comprehensive device tree support
for RGMII delay configuration. The rx-internal-delay-ps and
tx-internal-delay-ps properties allow precise tuning of clock delays
in 150 ps steps from 0 to 2250 ps. The optional dapu,tx-inverted-clk
flag enables 180-degree TX clock phase shift for boards where signal
integrity or MAC requirements necessitate clock inversion.
This PHY is used on the NDA platform with 1G Ethernet tile and has
been tested on that hardware with successful link establishment and
RGMII delay tuning.
Due to the specific PCB layout of the platform and FPGA configuration,
the default RGMII timing configuration was insufficient, causing packet
loss during normal operation. Tuning the TX/RX line delays and enabling
clock inversion restored proper signal timing, resulting in zero packet
loss and stable link performance.
$ make dt_binding_check DT_SCHEMA_FILES=dapu,dap8211r.yaml
SCHEMA Documentation/devicetree/bindings/processed-schema.json
CHKDT ./Documentation/devicetree/bindings
LINT ./Documentation/devicetree/bindings
STYLE ./Documentation/devicetree/bindings
DTEX Documentation/devicetree/bindings/net/dapu,dap8211r.example.dts
DTC [C] Documentation/devicetree/bindings/net/dapu,dap8211r.example.dtb
grep -i "dap8211r" Documentation/devicetree/bindings/processed-schema.json
chema.json
"http://devicetree.org/schemas/net/dapu,dap8211r.yaml": {
"$filename": ".../devicetree/bindings/net/dapu,dap8211r.yaml",
"$id": "http://devicetree.org/schemas/net/dapu,dap8211r.yaml#",
"title": "DAPU Telecom DAP8211R(I) Gigabit Ethernet PHY",
Working with xgmac.
Board side:
$ arping -I eth0 192.168.5.100
ARPING 192.168.5.1 from 192.168.5.100 eth0
Unicast reply from 192.168.5.1 [board.mac.addr] 8.543ms
Unicast reply from 192.168.5.1 [board.mac.addr] 3.295ms
Unicast reply from 192.168.5.1 [board.mac.addr] 4.301ms
Unicast reply from 192.168.5.1 [board.mac.addr] 4.096ms
Unicast reply from 192.168.5.1 [board.mac.addr] 2.872ms
...
$ Unfortunately, there is a dependence on the axibus speed here
$ iperf3 -c 192.168.5.1
Connecting to host 192.168.5.1, port 5201
[ 5] local 192.168.5.100 port 58936 connected to 192.168.5.1 port 5201
[ ID] Interval Transfer Bitrate Retr Cwnd
[ 5] 0.00-1.00 sec 7.88 MBytes 65.8 Mbits/sec 0 150 KBytes
[ 5] 1.00-2.00 sec 8.50 MBytes 71.4 Mbits/sec 0 165 KBytes
[ 5] 2.00-3.00 sec 8.25 MBytes 69.2 Mbits/sec 0 165 KBytes
[ 5] 3.00-4.01 sec 8.50 MBytes 71.1 Mbits/sec 0 165 KBytes
[ 5] 4.01-5.00 sec 8.38 MBytes 70.3 Mbits/sec 0 165 KBytes
[ 5] 5.00-6.00 sec 8.50 MBytes 71.5 Mbits/sec 0 165 KBytes
[ 5] 6.00-7.01 sec 8.62 MBytes 72.0 Mbits/sec 0 174 KBytes
[ 5] 7.01-8.00 sec 8.62 MBytes 72.8 Mbits/sec 0 174 KBytes
[ 5] 8.00-9.00 sec 8.62 MBytes 72.2 Mbits/sec 0 174 KBytes
[ 5] 9.00-10.04 sec 8.62 MBytes 69.9 Mbits/sec 0 174 KBytes
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval Transfer Bitrate Retr
[ 5] 0.00-10.04 sec 84.6 MBytes 70.7 Mbits/sec 0 sender
[ 5] 0.00-10.12 sec 84.8 MBytes 70.3 Mbits/sec receiveriperf Done.
$ ethtool -t eth0
...
The test extra info:
1. MAC Loopback 0
2. MAC Loopback (diff. queues) 0
3. PHY Loopback 0
...
ELP side:
...
17:29:11.974973 ARP, Reply ELP is-at elp.mac.addr(oui Unknown), length 28
17:29:12.975199 ARP, Request who-has ELP tell 192.168.5.100, length 46
17:29:12.975217 ARP, Reply ELP is-at elp.mac.addr(oui Unknown), length 28
17:29:13.975022 ARP, Request who-has ELP tell 192.168.5.100, length 46
17:29:13.975035 ARP, Reply ELP is-at elp.mac.addr(oui Unknown), length 28
17:29:14.974837 ARP, Request who-has ELP tell 192.168.5.100, length 46
17:29:14.974849 ARP, Reply ELP is-at elp.mac.addr(oui Unknown), length 28
17:29:15.975026 ARP, Request who-has ELP tell 192.168.5.100, length 46
...
Accepted connection from 192.168.5.100, port 58932
[ 5] local 192.168.5.1 port 5201 connected to 192.168.5.100 port 58936
[ ID] Interval Transfer Bitrate
[ 5] 0.00-1.00 sec 7.12 MBytes 59.7 Mbits/sec
[ 5] 1.00-2.00 sec 8.50 MBytes 71.3 Mbits/sec
[ 5] 2.00-3.00 sec 8.50 MBytes 71.3 Mbits/sec
[ 5] 3.00-4.00 sec 8.38 MBytes 70.3 Mbits/sec
[ 5] 4.00-5.00 sec 8.50 MBytes 71.3 Mbits/sec
[ 5] 5.00-6.00 sec 8.38 MBytes 70.3 Mbits/sec
[ 5] 6.00-7.00 sec 8.62 MBytes 72.4 Mbits/sec
[ 5] 7.00-8.00 sec 8.62 MBytes 72.3 Mbits/sec
[ 5] 8.00-9.00 sec 8.62 MBytes 72.4 Mbits/sec
[ 5] 9.00-10.00 sec 8.62 MBytes 72.4 Mbits/sec
[ 5] 10.00-10.12 sec 896 KBytes 62.3 Mbits/sec
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval Transfer Bitrate
[ 5] 0.00-10.12 sec 84.8 MBytes 70.3 Mbits/sec receiver
DTS node example:
ðernet_1g_tile {
...
phy-mode = "rgmii-rxid";
phy-handle = <&phy1>;
...
mdio: mdio {
phy1: ethernet-phy@1 {
...
compatible = "ethernet-phy-id0008.011b";
rx-internal-delay-ps = <1050>;
dapu,tx-inverted-clk;
...
};
};
};
--
Best regards,
Artem Shimko
ChangeLog:
v2:
- Drop debugfs interface
- Simplify RGMII delay reading logic using of_property_read_u32()
- Fix missing newline at end of dapu,dap8211r.yaml (yamllint error)
- Simplify delay property description in DT binding
- Rename tx-inverted-clk to dapu,tx-inverted-clk (vendor prefix)
- Replace enum with multipleOf + maximum for delay validation
- Fix compatible string and tx-internal-delay-ps value in example
- Remove rounding logic, return -EINVAL for unsupported delay values
- Respect DT delay properties for all RGMII modes
- Add polling for self-clearing reset bit instead of fixed sleep
- Remove unused packet generator macros (DAP8211R_PKGC5 and related)
v1:
- https://lore.kernel.org/all/20260713131223.279555-1-a.shimko.dev@gmail.com/T/#t
Artem Shimko (2):
dt-bindings: net: add DAPU Telecom DAP8211R(I) PHY binding
net: phy: add DAPU Telecom DAP8210R(I) Gigabit Ethernet PHY driver
.../bindings/net/dapu,dap8211r.yaml | 73 +++++
drivers/net/phy/Kconfig | 10 +
drivers/net/phy/Makefile | 1 +
drivers/net/phy/dap8211r.c | 281 ++++++++++++++++++
4 files changed, 365 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/dapu,dap8211r.yaml
create mode 100644 drivers/net/phy/dap8211r.c
--
2.43.0
^ permalink raw reply
* [PATCH net-next v4 3/3] net/smc: transition to RDMA core CQ pooling
From: D. Wythe @ 2026-07-16 11:37 UTC (permalink / raw)
To: mjambigi, wenjia, wintera, dust.li, tonylu, guwen
Cc: kuba, davem, netdev, linux-s390, linux-rdma, leonro, pabeni,
edumazet, sidraya, jaka, oliver.yang
In-Reply-To: <20260716113745.65234-1-alibuda@linux.alibaba.com>
The current SMC-R implementation relies on global per-device CQs
and manual polling within tasklets, which introduces severe
scalability bottlenecks due to global lock contention and tasklet
scheduling overhead, resulting in poor performance as concurrency
increases.
Refactor the completion handling to utilize the ib_cqe API and
standard RDMA core CQ pooling. This transition provides several key
advantages:
1. Multi-CQ: Shift from a single shared per-device CQ to multiple
link-specific CQs via the CQ pool. This allows completion processing
to be parallelized across multiple CPU cores, effectively eliminating
the global CQ bottleneck.
2. Leverage DIM: Utilizing the standard CQ pool with IB_POLL_SOFTIRQ
enables Dynamic Interrupt Moderation from the RDMA core, optimizing
interrupt frequency and reducing CPU load under high pressure.
3. O(1) Context Retrieval: Replaces the expensive wr_id based lookup
logic (e.g., smc_wr_tx_find_pending_index) with direct context retrieval
using container_of() on the embedded ib_cqe.
4. Code Simplification: This refactoring results in a reduction of
~150 lines of code. It removes redundant sequence tracking, complex lookup
helpers, and manual CQ management, significantly improving maintainability.
Performance Test: redis-benchmark with max 32 connections per QP
Data format: Requests Per Second (RPS), Percentage in brackets
represents the gain/loss compared to TCP.
| Clients | TCP | SMC (original) | SMC (cq_pool) |
|---------|----------|---------------------|---------------------|
| c = 1 | 24449 | 31172 (+27%) | 34039 (+39%) |
| c = 2 | 46420 | 53216 (+14%) | 64391 (+38%) |
| c = 16 | 159673 | 83668 (-48%) <-- | 216947 (+36%) |
| c = 32 | 164956 | 97631 (-41%) <-- | 249376 (+51%) |
| c = 64 | 166322 | 118192 (-29%) <-- | 249488 (+50%) |
| c = 128 | 167700 | 121497 (-27%) <-- | 249480 (+48%) |
| c = 256 | 175021 | 146109 (-16%) <-- | 240384 (+37%) |
| c = 512 | 168987 | 101479 (-40%) <-- | 226634 (+34%) |
The results demonstrate that this optimization effectively resolves the
scalability bottleneck, with RPS increasing by over 110% at c=64
compared to the original implementation.
Signed-off-by: D. Wythe <alibuda@linux.alibaba.com>
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
---
net/smc/smc_core.c | 10 +-
net/smc/smc_core.h | 36 ++--
net/smc/smc_ib.c | 106 ++++--------
net/smc/smc_ib.h | 6 -
net/smc/smc_tx.c | 1 -
net/smc/smc_wr.c | 401 ++++++++++++++++++++-------------------------
net/smc/smc_wr.h | 50 ++----
7 files changed, 257 insertions(+), 353 deletions(-)
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index cf6b620fef05..5073af92e086 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -815,17 +815,11 @@ int smcr_link_init(struct smc_link_group *lgr, struct smc_link *lnk,
lnk->lgr = lgr;
smc_lgr_hold(lgr); /* lgr_put in smcr_link_clear() */
lnk->link_idx = link_idx;
- lnk->wr_rx_id_compl = 0;
smc_ibdev_cnt_inc(lnk);
smcr_copy_dev_info_to_link(lnk);
atomic_set(&lnk->conn_cnt, 0);
smc_llc_link_set_uid(lnk);
INIT_WORK(&lnk->link_down_wrk, smc_link_down_work);
- if (!lnk->smcibdev->initialized) {
- rc = (int)smc_ib_setup_per_ibdev(lnk->smcibdev);
- if (rc)
- goto out;
- }
get_random_bytes(rndvec, sizeof(rndvec));
lnk->psn_initial = rndvec[0] + (rndvec[1] << 8) +
(rndvec[2] << 16);
@@ -863,6 +857,7 @@ int smcr_link_init(struct smc_link_group *lgr, struct smc_link *lnk,
if (rc)
goto free_link_mem;
lnk->state = SMC_LNK_ACTIVATING;
+ smc_wr_init_cqes(lnk);
return 0;
free_link_mem:
@@ -1373,7 +1368,8 @@ void smcr_link_clear(struct smc_link *lnk, bool log)
smc_llc_link_clear(lnk, log);
smcr_buf_unmap_lgr(lnk);
smcr_rtoken_clear_link(lnk);
- smc_ib_modify_qp_error(lnk);
+ smc_wr_stop_link(lnk);
+ smc_wr_drain_qp(lnk);
smc_wr_free_link(lnk);
smc_ib_destroy_queue_pair(lnk);
smc_ib_dealloc_protection_domain(lnk);
diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h
index 5c18f08a4c8a..1612a8012280 100644
--- a/net/smc/smc_core.h
+++ b/net/smc/smc_core.h
@@ -89,8 +89,21 @@ struct smc_rdma_sges { /* sges per message send */
struct smc_rdma_wr { /* work requests per message
* send
*/
+ struct ib_cqe cqe;
struct ib_rdma_wr wr_tx_rdma[SMC_MAX_RDMA_WRITES];
-};
+} ____cacheline_aligned_in_smp;
+
+struct smc_ib_recv_wr {
+ struct ib_cqe cqe;
+ struct ib_recv_wr wr;
+ int idx;
+} ____cacheline_aligned_in_smp;
+
+struct smc_ib_send_wr {
+ struct ib_cqe cqe;
+ struct ib_send_wr wr;
+ int idx;
+} ____cacheline_aligned_in_smp;
#define SMC_LGR_ID_SIZE 4
@@ -100,33 +113,34 @@ struct smc_link {
struct ib_pd *roce_pd; /* IB protection domain,
* unique for every RoCE QP
*/
+ unsigned int nr_cqe; /* number of CQ entries */
+ struct ib_cq *ib_cq; /* IB completion queue */
struct ib_qp *roce_qp; /* IB queue pair */
struct ib_qp_attr qp_attr; /* IB queue pair attributes */
struct smc_wr_buf *wr_tx_bufs; /* WR send payload buffers */
- struct ib_send_wr *wr_tx_ibs; /* WR send meta data */
+ struct smc_ib_send_wr *wr_tx_ibs; /* WR send meta data */
struct ib_sge *wr_tx_sges; /* WR send gather meta data */
struct smc_rdma_sges *wr_tx_rdma_sges;/*RDMA WRITE gather meta data*/
struct smc_rdma_wr *wr_tx_rdmas; /* WR RDMA WRITE */
struct smc_wr_tx_pend *wr_tx_pends; /* WR send waiting for CQE */
struct completion *wr_tx_compl; /* WR send CQE completion */
/* above four vectors have wr_tx_cnt elements and use the same index */
- struct ib_send_wr *wr_tx_v2_ib; /* WR send v2 meta data */
+ struct smc_ib_send_wr *wr_tx_v2_ib; /* WR send v2 meta data */
struct ib_sge *wr_tx_v2_sge; /* WR send v2 gather meta data*/
struct smc_wr_tx_pend *wr_tx_v2_pend; /* WR send v2 waiting for CQE */
dma_addr_t wr_tx_dma_addr; /* DMA address of wr_tx_bufs */
dma_addr_t wr_tx_v2_dma_addr; /* DMA address of v2 tx buf*/
- atomic_long_t wr_tx_id; /* seq # of last sent WR */
unsigned long *wr_tx_mask; /* bit mask of used indexes */
u32 wr_tx_cnt; /* number of WR send buffers */
wait_queue_head_t wr_tx_wait; /* wait for free WR send buf */
struct {
- struct percpu_ref wr_tx_refs;
+ struct percpu_ref wr_refs; /* refs for all WR posting (tx/rx/reg) */
} ____cacheline_aligned_in_smp;
- struct completion tx_ref_comp;
+ struct completion ref_comp;
u8 *wr_rx_bufs; /* WR recv payload buffers */
- struct ib_recv_wr *wr_rx_ibs; /* WR recv meta data */
+ struct smc_ib_recv_wr *wr_rx_ibs; /* WR recv meta data */
struct ib_sge *wr_rx_sges; /* WR recv scatter meta data */
/* above three vectors have wr_rx_cnt elements and use the same index */
int wr_rx_sge_cnt; /* rx sge, V1 is 1, V2 is either 2 or 1 */
@@ -135,18 +149,12 @@ struct smc_link {
*/
dma_addr_t wr_rx_dma_addr; /* DMA address of wr_rx_bufs */
dma_addr_t wr_rx_v2_dma_addr; /* DMA address of v2 rx buf*/
- u64 wr_rx_id; /* seq # of last recv WR */
- u64 wr_rx_id_compl; /* seq # of last completed WR */
u32 wr_rx_cnt; /* number of WR recv buffers */
unsigned long wr_rx_tstamp; /* jiffies when last buf rx */
- wait_queue_head_t wr_rx_empty_wait; /* wait for RQ empty */
struct ib_reg_wr wr_reg; /* WR register memory region */
+ struct ib_cqe wr_reg_cqe; /* ib_cqe for wr_reg */
wait_queue_head_t wr_reg_wait; /* wait for wr_reg result */
- struct {
- struct percpu_ref wr_reg_refs;
- } ____cacheline_aligned_in_smp;
- struct completion reg_ref_comp;
enum smc_wr_reg_state wr_reg_state; /* state of wr_reg request */
u8 gid[SMC_GID_SIZE];/* gid matching used vlan id*/
diff --git a/net/smc/smc_ib.c b/net/smc/smc_ib.c
index 9bb495707445..8e06ab61c804 100644
--- a/net/smc/smc_ib.c
+++ b/net/smc/smc_ib.c
@@ -133,10 +133,7 @@ int smc_ib_ready_link(struct smc_link *lnk)
if (rc)
goto out;
smc_wr_remember_qp_attr(lnk);
- rc = ib_req_notify_cq(lnk->smcibdev->roce_cq_recv,
- IB_CQ_SOLICITED_MASK);
- if (rc)
- goto out;
+
rc = smc_wr_rx_post_init(lnk);
if (rc)
goto out;
@@ -657,38 +654,61 @@ void smc_ib_destroy_queue_pair(struct smc_link *lnk)
if (lnk->roce_qp)
ib_destroy_qp(lnk->roce_qp);
lnk->roce_qp = NULL;
+ if (lnk->ib_cq) {
+ ib_cq_pool_put(lnk->ib_cq, lnk->nr_cqe);
+ lnk->ib_cq = NULL;
+ }
}
/* create a queue pair within the protection domain for a link */
int smc_ib_create_queue_pair(struct smc_link *lnk)
{
+ int max_send_wr, max_recv_wr, rc;
+ struct ib_cq *cq;
+
+ /* include unsolicited rdma_writes as well,
+ * there are max. 2 RDMA_WRITE per 1 WR_SEND.
+ * +3: ib_drain_sq() drain WR, FastReg WR (link->wr_reg, link-unique),
+ * and SMC-Rv2 v2 SEND (link->wr_tx_v2_ib, link-unique).
+ */
+ max_send_wr = 3 * lnk->lgr->max_send_wr + 3;
+ max_recv_wr = lnk->lgr->max_recv_wr + 1; /* +1 for ib_drain_rq() */
+
+ cq = ib_cq_pool_get(lnk->smcibdev->ibdev, max_send_wr + max_recv_wr, -1,
+ IB_POLL_SOFTIRQ);
+
+ if (IS_ERR(cq)) {
+ rc = PTR_ERR(cq);
+ return rc;
+ }
+
struct ib_qp_init_attr qp_attr = {
.event_handler = smc_ib_qp_event_handler,
.qp_context = lnk,
- .send_cq = lnk->smcibdev->roce_cq_send,
- .recv_cq = lnk->smcibdev->roce_cq_recv,
+ .send_cq = cq,
+ .recv_cq = cq,
.srq = NULL,
.cap = {
.max_send_sge = SMC_IB_MAX_SEND_SGE,
.max_recv_sge = lnk->wr_rx_sge_cnt,
+ .max_send_wr = max_send_wr,
+ .max_recv_wr = max_recv_wr,
.max_inline_data = 0,
},
.sq_sig_type = IB_SIGNAL_REQ_WR,
.qp_type = IB_QPT_RC,
};
- int rc;
- /* include unsolicited rdma_writes as well,
- * there are max. 2 RDMA_WRITE per 1 WR_SEND
- */
- qp_attr.cap.max_send_wr = 3 * lnk->lgr->max_send_wr;
- qp_attr.cap.max_recv_wr = lnk->lgr->max_recv_wr;
lnk->roce_qp = ib_create_qp(lnk->roce_pd, &qp_attr);
rc = PTR_ERR_OR_ZERO(lnk->roce_qp);
- if (IS_ERR(lnk->roce_qp))
+ if (IS_ERR(lnk->roce_qp)) {
lnk->roce_qp = NULL;
- else
+ ib_cq_pool_put(cq, max_send_wr + max_recv_wr);
+ } else {
smc_wr_remember_qp_attr(lnk);
+ lnk->nr_cqe = max_send_wr + max_recv_wr;
+ lnk->ib_cq = cq;
+ }
return rc;
}
@@ -838,62 +858,6 @@ void smc_ib_buf_unmap_sg(struct smc_link *lnk,
buf_slot->sgt[lnk->link_idx].sgl->dma_address = 0;
}
-long smc_ib_setup_per_ibdev(struct smc_ib_device *smcibdev)
-{
- struct ib_cq_init_attr cqattr = {
- .cqe = SMC_MAX_CQE, .comp_vector = 0 };
- int cqe_size_order, smc_order;
- long rc;
-
- mutex_lock(&smcibdev->mutex);
- rc = 0;
- if (smcibdev->initialized)
- goto out;
- /* the calculated number of cq entries fits to mlx5 cq allocation */
- cqe_size_order = cache_line_size() == 128 ? 7 : 6;
- smc_order = MAX_PAGE_ORDER - cqe_size_order;
- if (SMC_MAX_CQE + 2 > (0x00000001 << smc_order) * PAGE_SIZE)
- cqattr.cqe = (0x00000001 << smc_order) * PAGE_SIZE - 2;
- smcibdev->roce_cq_send = ib_create_cq(smcibdev->ibdev,
- smc_wr_tx_cq_handler, NULL,
- smcibdev, &cqattr);
- rc = PTR_ERR_OR_ZERO(smcibdev->roce_cq_send);
- if (IS_ERR(smcibdev->roce_cq_send)) {
- smcibdev->roce_cq_send = NULL;
- goto out;
- }
- smcibdev->roce_cq_recv = ib_create_cq(smcibdev->ibdev,
- smc_wr_rx_cq_handler, NULL,
- smcibdev, &cqattr);
- rc = PTR_ERR_OR_ZERO(smcibdev->roce_cq_recv);
- if (IS_ERR(smcibdev->roce_cq_recv)) {
- smcibdev->roce_cq_recv = NULL;
- goto err;
- }
- smc_wr_add_dev(smcibdev);
- smcibdev->initialized = 1;
- goto out;
-
-err:
- ib_destroy_cq(smcibdev->roce_cq_send);
-out:
- mutex_unlock(&smcibdev->mutex);
- return rc;
-}
-
-static void smc_ib_cleanup_per_ibdev(struct smc_ib_device *smcibdev)
-{
- mutex_lock(&smcibdev->mutex);
- if (!smcibdev->initialized)
- goto out;
- smcibdev->initialized = 0;
- ib_destroy_cq(smcibdev->roce_cq_recv);
- ib_destroy_cq(smcibdev->roce_cq_send);
- smc_wr_remove_dev(smcibdev);
-out:
- mutex_unlock(&smcibdev->mutex);
-}
-
static struct ib_client smc_ib_client;
static void smc_copy_netdev_ifindex(struct smc_ib_device *smcibdev, int port)
@@ -952,7 +916,6 @@ static int smc_ib_add_dev(struct ib_device *ibdev)
INIT_WORK(&smcibdev->port_event_work, smc_ib_port_event_work);
atomic_set(&smcibdev->lnk_cnt, 0);
init_waitqueue_head(&smcibdev->lnks_deleted);
- mutex_init(&smcibdev->mutex);
mutex_lock(&smc_ib_devices.mutex);
list_add_tail(&smcibdev->list, &smc_ib_devices.list);
mutex_unlock(&smc_ib_devices.mutex);
@@ -1001,7 +964,6 @@ static void smc_ib_remove_dev(struct ib_device *ibdev, void *client_data)
pr_warn_ratelimited("smc: removing ib device %s\n",
smcibdev->ibdev->name);
smc_smcr_terminate_all(smcibdev);
- smc_ib_cleanup_per_ibdev(smcibdev);
ib_unregister_event_handler(&smcibdev->event_handler);
cancel_work_sync(&smcibdev->port_event_work);
kfree(smcibdev);
diff --git a/net/smc/smc_ib.h b/net/smc/smc_ib.h
index ef8ac2b7546d..0e4c6a14151a 100644
--- a/net/smc/smc_ib.h
+++ b/net/smc/smc_ib.h
@@ -37,17 +37,12 @@ struct smc_ib_device { /* ib-device infos for smc */
struct ib_device *ibdev;
struct ib_port_attr pattr[SMC_MAX_PORTS]; /* ib dev. port attrs */
struct ib_event_handler event_handler; /* global ib_event handler */
- struct ib_cq *roce_cq_send; /* send completion queue */
- struct ib_cq *roce_cq_recv; /* recv completion queue */
- struct tasklet_struct send_tasklet; /* called by send cq handler */
- struct tasklet_struct recv_tasklet; /* called by recv cq handler */
char mac[SMC_MAX_PORTS][ETH_ALEN];
/* mac address per port*/
u8 pnetid[SMC_MAX_PORTS][SMC_MAX_PNETID_LEN];
/* pnetid per port */
bool pnetid_by_user[SMC_MAX_PORTS];
/* pnetid defined by user? */
- u8 initialized : 1; /* ib dev CQ, evthdl done */
struct work_struct port_event_work;
unsigned long port_event_mask;
DECLARE_BITMAP(ports_going_away, SMC_MAX_PORTS);
@@ -97,7 +92,6 @@ int smc_ib_create_queue_pair(struct smc_link *lnk);
int smc_ib_ready_link(struct smc_link *lnk);
int smc_ib_modify_qp_rts(struct smc_link *lnk);
int smc_ib_modify_qp_error(struct smc_link *lnk);
-long smc_ib_setup_per_ibdev(struct smc_ib_device *smcibdev);
int smc_ib_get_memory_region(struct ib_pd *pd, int access_flags,
struct smc_buf_desc *buf_slot, u8 link_idx);
void smc_ib_put_memory_region(struct ib_mr *mr);
diff --git a/net/smc/smc_tx.c b/net/smc/smc_tx.c
index 3144b4b1fe29..d301df9ed58b 100644
--- a/net/smc/smc_tx.c
+++ b/net/smc/smc_tx.c
@@ -321,7 +321,6 @@ static int smc_tx_rdma_write(struct smc_connection *conn, int peer_rmbe_offset,
struct smc_link *link = conn->lnk;
int rc;
- rdma_wr->wr.wr_id = smc_wr_tx_get_next_wr_id(link);
rdma_wr->wr.num_sge = num_sges;
rdma_wr->remote_addr =
lgr->rtokens[conn->rtoken_idx][link->link_idx].dma_addr +
diff --git a/net/smc/smc_wr.c b/net/smc/smc_wr.c
index 2cdd4063c13d..da6f185d1124 100644
--- a/net/smc/smc_wr.c
+++ b/net/smc/smc_wr.c
@@ -30,15 +30,13 @@
#include "smc.h"
#include "smc_wr.h"
-
-#define SMC_WR_MAX_POLL_CQE 10 /* max. # of compl. queue elements in 1 poll */
+#include "smc_ib.h"
#define SMC_WR_RX_HASH_BITS 4
static DEFINE_HASHTABLE(smc_wr_rx_hash, SMC_WR_RX_HASH_BITS);
static DEFINE_SPINLOCK(smc_wr_rx_hash_lock);
struct smc_wr_tx_pend { /* control data for a pending send request */
- u64 wr_id; /* work request id sent */
smc_wr_tx_handler handler;
enum ib_wc_status wc_status; /* CQE status */
struct smc_link *link;
@@ -63,107 +61,71 @@ void smc_wr_tx_wait_no_pending_sends(struct smc_link *link)
wait_event(link->wr_tx_wait, !smc_wr_is_tx_pend(link));
}
-static inline int smc_wr_tx_find_pending_index(struct smc_link *link, u64 wr_id)
+static void smc_wr_tx_rdma_process_cqe(struct ib_cq *cq, struct ib_wc *wc)
{
- u32 i;
+ struct smc_link *link = wc->qp->qp_context;
- for (i = 0; i < link->wr_tx_cnt; i++) {
- if (link->wr_tx_pends[i].wr_id == wr_id)
- return i;
- }
- return link->wr_tx_cnt;
+ /* terminate link */
+ if (wc->status)
+ smcr_link_down_cond_sched(link);
+}
+
+static void smc_wr_reg_process_cqe(struct ib_cq *cq, struct ib_wc *wc)
+{
+ struct smc_link *link = wc->qp->qp_context;
+
+ if (wc->status)
+ link->wr_reg_state = FAILED;
+ else
+ link->wr_reg_state = CONFIRMED;
+ smc_wr_wakeup_reg_wait(link);
}
-static inline void smc_wr_tx_process_cqe(struct ib_wc *wc)
+static void smc_wr_tx_process_cqe(struct ib_cq *cq, struct ib_wc *wc)
{
- struct smc_wr_tx_pend pnd_snd;
+ struct smc_wr_tx_pend *tx_pend, pnd_snd;
+ struct smc_ib_send_wr *send_wr;
struct smc_link *link;
u32 pnd_snd_idx;
+ /* ib_drain_qp() is called before link free, so link is safe here */
link = wc->qp->qp_context;
- if (wc->opcode == IB_WC_REG_MR) {
- if (wc->status)
- link->wr_reg_state = FAILED;
- else
- link->wr_reg_state = CONFIRMED;
- smc_wr_wakeup_reg_wait(link);
- return;
- }
+ send_wr = container_of(wc->wr_cqe, struct smc_ib_send_wr, cqe);
+ pnd_snd_idx = send_wr->idx;
+
+ tx_pend = (pnd_snd_idx == link->wr_tx_cnt) ? link->wr_tx_v2_pend :
+ &link->wr_tx_pends[pnd_snd_idx];
+
+ tx_pend->wc_status = wc->status;
+ memcpy(&pnd_snd, tx_pend, sizeof(pnd_snd));
+ /* clear the full struct smc_wr_tx_pend including .priv */
+ memset(tx_pend, 0, sizeof(*tx_pend));
- pnd_snd_idx = smc_wr_tx_find_pending_index(link, wc->wr_id);
if (pnd_snd_idx == link->wr_tx_cnt) {
- if (link->lgr->smc_version != SMC_V2 ||
- link->wr_tx_v2_pend->wr_id != wc->wr_id)
- return;
- link->wr_tx_v2_pend->wc_status = wc->status;
- memcpy(&pnd_snd, link->wr_tx_v2_pend, sizeof(pnd_snd));
- /* clear the full struct smc_wr_tx_pend including .priv */
- memset(link->wr_tx_v2_pend, 0,
- sizeof(*link->wr_tx_v2_pend));
memset(link->lgr->wr_tx_buf_v2, 0,
sizeof(*link->lgr->wr_tx_buf_v2));
} else {
- link->wr_tx_pends[pnd_snd_idx].wc_status = wc->status;
- if (link->wr_tx_pends[pnd_snd_idx].compl_requested)
+ if (pnd_snd.compl_requested)
complete(&link->wr_tx_compl[pnd_snd_idx]);
- memcpy(&pnd_snd, &link->wr_tx_pends[pnd_snd_idx],
- sizeof(pnd_snd));
- /* clear the full struct smc_wr_tx_pend including .priv */
- memset(&link->wr_tx_pends[pnd_snd_idx], 0,
- sizeof(link->wr_tx_pends[pnd_snd_idx]));
memset(&link->wr_tx_bufs[pnd_snd_idx], 0,
sizeof(link->wr_tx_bufs[pnd_snd_idx]));
if (!test_and_clear_bit(pnd_snd_idx, link->wr_tx_mask))
return;
}
+ /* terminate link */
if (wc->status) {
- if (link->lgr->smc_version == SMC_V2) {
- memset(link->wr_tx_v2_pend, 0,
- sizeof(*link->wr_tx_v2_pend));
- memset(link->lgr->wr_tx_buf_v2, 0,
- sizeof(*link->lgr->wr_tx_buf_v2));
- }
- /* terminate link */
smcr_link_down_cond_sched(link);
+ /* link is now unsendable; wake all so the exclusive
+ * get_free_slot() waiters give up at once
+ */
+ smc_wr_wakeup_tx_wait(link);
+ } else {
+ wake_up(&link->wr_tx_wait);
}
if (pnd_snd.handler)
pnd_snd.handler(&pnd_snd.priv, link, wc->status);
- wake_up(&link->wr_tx_wait);
-}
-
-static void smc_wr_tx_tasklet_fn(struct tasklet_struct *t)
-{
- struct smc_ib_device *dev = from_tasklet(dev, t, send_tasklet);
- struct ib_wc wc[SMC_WR_MAX_POLL_CQE];
- int i = 0, rc;
- int polled = 0;
-
-again:
- polled++;
- do {
- memset(&wc, 0, sizeof(wc));
- rc = ib_poll_cq(dev->roce_cq_send, SMC_WR_MAX_POLL_CQE, wc);
- if (polled == 1) {
- ib_req_notify_cq(dev->roce_cq_send,
- IB_CQ_NEXT_COMP |
- IB_CQ_REPORT_MISSED_EVENTS);
- }
- if (!rc)
- break;
- for (i = 0; i < rc; i++)
- smc_wr_tx_process_cqe(&wc[i]);
- } while (rc > 0);
- if (polled == 1)
- goto again;
-}
-
-void smc_wr_tx_cq_handler(struct ib_cq *ib_cq, void *cq_context)
-{
- struct smc_ib_device *dev = (struct smc_ib_device *)cq_context;
-
- tasklet_schedule(&dev->send_tasklet);
}
/*---------------------------- request submission ---------------------------*/
@@ -202,8 +164,6 @@ int smc_wr_tx_get_free_slot(struct smc_link *link,
struct smc_link_group *lgr = smc_get_lgr(link);
struct smc_wr_tx_pend *wr_pend;
u32 idx = link->wr_tx_cnt;
- struct ib_send_wr *wr_ib;
- u64 wr_id;
DEFINE_WAIT(wait);
int rc;
@@ -242,14 +202,10 @@ int smc_wr_tx_get_free_slot(struct smc_link *link,
if (idx == link->wr_tx_cnt)
return -EPIPE;
}
- wr_id = smc_wr_tx_get_next_wr_id(link);
wr_pend = &link->wr_tx_pends[idx];
- wr_pend->wr_id = wr_id;
wr_pend->handler = handler;
wr_pend->link = link;
wr_pend->idx = idx;
- wr_ib = &link->wr_tx_ibs[idx];
- wr_ib->wr_id = wr_id;
*wr_buf = &link->wr_tx_bufs[idx];
if (wr_rdma_buf)
*wr_rdma_buf = &link->wr_tx_rdmas[idx];
@@ -263,22 +219,16 @@ int smc_wr_tx_get_v2_slot(struct smc_link *link,
struct smc_wr_tx_pend_priv **wr_pend_priv)
{
struct smc_wr_tx_pend *wr_pend;
- struct ib_send_wr *wr_ib;
- u64 wr_id;
if (link->wr_tx_v2_pend->idx == link->wr_tx_cnt)
return -EBUSY;
*wr_buf = NULL;
*wr_pend_priv = NULL;
- wr_id = smc_wr_tx_get_next_wr_id(link);
wr_pend = link->wr_tx_v2_pend;
- wr_pend->wr_id = wr_id;
wr_pend->handler = handler;
wr_pend->link = link;
wr_pend->idx = link->wr_tx_cnt;
- wr_ib = link->wr_tx_v2_ib;
- wr_ib->wr_id = wr_id;
*wr_buf = link->lgr->wr_tx_buf_v2;
*wr_pend_priv = &wr_pend->priv;
return 0;
@@ -322,10 +272,8 @@ int smc_wr_tx_send(struct smc_link *link, struct smc_wr_tx_pend_priv *priv)
struct smc_wr_tx_pend *pend;
int rc;
- ib_req_notify_cq(link->smcibdev->roce_cq_send,
- IB_CQ_NEXT_COMP | IB_CQ_REPORT_MISSED_EVENTS);
pend = container_of(priv, struct smc_wr_tx_pend, priv);
- rc = ib_post_send(link->roce_qp, &link->wr_tx_ibs[pend->idx], NULL);
+ rc = ib_post_send(link->roce_qp, &link->wr_tx_ibs[pend->idx].wr, NULL);
if (rc) {
smc_wr_tx_put_slot(link, priv);
smcr_link_down_cond_sched(link);
@@ -338,10 +286,8 @@ int smc_wr_tx_v2_send(struct smc_link *link, struct smc_wr_tx_pend_priv *priv,
{
int rc;
- link->wr_tx_v2_ib->sg_list[0].length = len;
- ib_req_notify_cq(link->smcibdev->roce_cq_send,
- IB_CQ_NEXT_COMP | IB_CQ_REPORT_MISSED_EVENTS);
- rc = ib_post_send(link->roce_qp, link->wr_tx_v2_ib, NULL);
+ link->wr_tx_v2_ib->wr.sg_list[0].length = len;
+ rc = ib_post_send(link->roce_qp, &link->wr_tx_v2_ib->wr, NULL);
if (rc) {
smc_wr_tx_put_slot(link, priv);
smcr_link_down_cond_sched(link);
@@ -383,21 +329,22 @@ int smc_wr_reg_send(struct smc_link *link, struct ib_mr *mr)
{
int rc;
- ib_req_notify_cq(link->smcibdev->roce_cq_send,
- IB_CQ_NEXT_COMP | IB_CQ_REPORT_MISSED_EVENTS);
+ if (!percpu_ref_tryget_live(&link->wr_refs))
+ return -EPIPE;
+
link->wr_reg_state = POSTED;
- link->wr_reg.wr.wr_id = (u64)(uintptr_t)mr;
link->wr_reg.mr = mr;
link->wr_reg.key = mr->rkey;
rc = ib_post_send(link->roce_qp, &link->wr_reg.wr, NULL);
- if (rc)
+ if (rc) {
+ percpu_ref_put(&link->wr_refs);
return rc;
+ }
- percpu_ref_get(&link->wr_reg_refs);
rc = wait_event_interruptible_timeout(link->wr_reg_wait,
(link->wr_reg_state != POSTED),
SMC_WR_REG_MR_WAIT_TIME);
- percpu_ref_put(&link->wr_reg_refs);
+ percpu_ref_put(&link->wr_refs);
if (!rc) {
/* timeout - terminate link */
smcr_link_down_cond_sched(link);
@@ -447,95 +394,107 @@ static inline void smc_wr_rx_demultiplex(struct ib_wc *wc)
{
struct smc_link *link = (struct smc_link *)wc->qp->qp_context;
struct smc_wr_rx_handler *handler;
+ struct smc_ib_recv_wr *recv_wr;
struct smc_wr_rx_hdr *wr_rx;
- u64 temp_wr_id;
- u32 index;
if (wc->byte_len < sizeof(*wr_rx))
return; /* short message */
- temp_wr_id = wc->wr_id;
- index = do_div(temp_wr_id, link->wr_rx_cnt);
- wr_rx = (struct smc_wr_rx_hdr *)(link->wr_rx_bufs + index * link->wr_rx_buflen);
+
+ recv_wr = container_of(wc->wr_cqe, struct smc_ib_recv_wr, cqe);
+
+ wr_rx = (struct smc_wr_rx_hdr *)(link->wr_rx_bufs + recv_wr->idx * link->wr_rx_buflen);
hash_for_each_possible(smc_wr_rx_hash, handler, list, wr_rx->type) {
if (handler->type == wr_rx->type)
handler->handler(wc, wr_rx);
}
}
-static inline void smc_wr_rx_process_cqes(struct ib_wc wc[], int num)
+/* Repost a receive WR unless the link is being torn down. Holding wr_refs
+ * across the post serializes against smc_wr_stop_link(), which kills the ref and
+ * waits before ib_drain_qp() posts its drain WR. This guarantees no recv WR is
+ * posted after the drain WR, preventing a use-after-free of wr_rx_ibs when
+ * flush CQEs are polled after teardown.
+ */
+static void smc_wr_rx_refill(struct smc_link *link, struct ib_cqe *cqe)
{
- struct smc_link *link;
- int i;
-
- for (i = 0; i < num; i++) {
- link = wc[i].qp->qp_context;
- link->wr_rx_id_compl = wc[i].wr_id;
- if (wc[i].status == IB_WC_SUCCESS) {
- link->wr_rx_tstamp = jiffies;
- smc_wr_rx_demultiplex(&wc[i]);
- smc_wr_rx_post(link); /* refill WR RX */
- } else {
- /* handle status errors */
- switch (wc[i].status) {
- case IB_WC_RETRY_EXC_ERR:
- case IB_WC_RNR_RETRY_EXC_ERR:
- case IB_WC_WR_FLUSH_ERR:
- smcr_link_down_cond_sched(link);
- if (link->wr_rx_id_compl == link->wr_rx_id)
- wake_up(&link->wr_rx_empty_wait);
- break;
- default:
- smc_wr_rx_post(link); /* refill WR RX */
- break;
- }
- }
- }
+ if (!percpu_ref_tryget_live(&link->wr_refs))
+ return;
+ smc_wr_rx_post(link, cqe); /* refill WR RX */
+ percpu_ref_put(&link->wr_refs);
}
-static void smc_wr_rx_tasklet_fn(struct tasklet_struct *t)
+static void smc_wr_rx_process_cqe(struct ib_cq *cq, struct ib_wc *wc)
{
- struct smc_ib_device *dev = from_tasklet(dev, t, recv_tasklet);
- struct ib_wc wc[SMC_WR_MAX_POLL_CQE];
- int polled = 0;
- int rc;
+ struct smc_link *link = wc->qp->qp_context;
-again:
- polled++;
- do {
- memset(&wc, 0, sizeof(wc));
- rc = ib_poll_cq(dev->roce_cq_recv, SMC_WR_MAX_POLL_CQE, wc);
- if (polled == 1) {
- ib_req_notify_cq(dev->roce_cq_recv,
- IB_CQ_SOLICITED_MASK
- | IB_CQ_REPORT_MISSED_EVENTS);
- }
- if (!rc)
+ if (wc->status == IB_WC_SUCCESS) {
+ link->wr_rx_tstamp = jiffies;
+ smc_wr_rx_demultiplex(wc);
+ smc_wr_rx_refill(link, wc->wr_cqe);
+ } else {
+ /* handle status errors */
+ switch (wc->status) {
+ case IB_WC_RETRY_EXC_ERR:
+ case IB_WC_RNR_RETRY_EXC_ERR:
+ case IB_WC_WR_FLUSH_ERR:
+ smcr_link_down_cond_sched(link);
+ break;
+ default:
+ smc_wr_rx_refill(link, wc->wr_cqe);
break;
- smc_wr_rx_process_cqes(&wc[0], rc);
- } while (rc > 0);
- if (polled == 1)
- goto again;
+ }
+ }
}
-void smc_wr_rx_cq_handler(struct ib_cq *ib_cq, void *cq_context)
+void smc_wr_stop_link(struct smc_link *lnk)
{
- struct smc_ib_device *dev = (struct smc_ib_device *)cq_context;
+ if (lnk->state == SMC_LNK_UNUSED)
+ return;
+
+ /* prevent any new WR postings, including SEND and RECV */
+ percpu_ref_kill(&lnk->wr_refs);
+
+ /* Move the QP to error state to flush the in-flight WRs */
+ smc_ib_modify_qp_error(lnk);
- tasklet_schedule(&dev->recv_tasklet);
+ /* Wait until all posting has ceased. Only then smc_wr_drain_qp()
+ * works: its drain WR must be the last CQE, which holds only if no WR is
+ * posted after it.
+ */
+ wait_for_completion(&lnk->ref_comp);
}
int smc_wr_rx_post_init(struct smc_link *link)
{
- u32 i;
- int rc = 0;
+ int i, rc = 0;
for (i = 0; i < link->wr_rx_cnt; i++)
- rc = smc_wr_rx_post(link);
+ rc = smc_wr_rx_post(link, &link->wr_rx_ibs[i].cqe);
return rc;
}
/***************************** init, exit, misc ******************************/
+static void smc_wr_reg_init_cqe(struct ib_cqe *cqe)
+{
+ cqe->done = smc_wr_reg_process_cqe;
+}
+
+static void smc_wr_tx_init_cqe(struct ib_cqe *cqe)
+{
+ cqe->done = smc_wr_tx_process_cqe;
+}
+
+static void smc_wr_rx_init_cqe(struct ib_cqe *cqe)
+{
+ cqe->done = smc_wr_rx_process_cqe;
+}
+
+static void smc_wr_tx_rdma_init_cqe(struct ib_cqe *cqe)
+{
+ cqe->done = smc_wr_tx_rdma_process_cqe;
+}
+
void smc_wr_remember_qp_attr(struct smc_link *lnk)
{
struct ib_qp_attr *attr = &lnk->qp_attr;
@@ -566,7 +525,7 @@ void smc_wr_remember_qp_attr(struct smc_link *lnk)
lnk->wr_tx_cnt = min_t(size_t, lnk->max_send_wr,
lnk->qp_attr.cap.max_send_wr);
lnk->wr_rx_cnt = min_t(size_t, lnk->max_recv_wr,
- lnk->qp_attr.cap.max_recv_wr);
+ lnk->qp_attr.cap.max_recv_wr - 1); /* -1 for ib_drain_rq() */
}
static void smc_wr_init_sge(struct smc_link *lnk)
@@ -587,14 +546,14 @@ static void smc_wr_init_sge(struct smc_link *lnk)
lnk->roce_pd->local_dma_lkey;
lnk->wr_tx_rdma_sges[i].tx_rdma_sge[1].wr_tx_rdma_sge[1].lkey =
lnk->roce_pd->local_dma_lkey;
- lnk->wr_tx_ibs[i].next = NULL;
- lnk->wr_tx_ibs[i].sg_list = &lnk->wr_tx_sges[i];
- lnk->wr_tx_ibs[i].num_sge = 1;
- lnk->wr_tx_ibs[i].opcode = IB_WR_SEND;
- lnk->wr_tx_ibs[i].send_flags =
+ lnk->wr_tx_ibs[i].wr.next = NULL;
+ lnk->wr_tx_ibs[i].wr.sg_list = &lnk->wr_tx_sges[i];
+ lnk->wr_tx_ibs[i].wr.num_sge = 1;
+ lnk->wr_tx_ibs[i].wr.opcode = IB_WR_SEND;
+ lnk->wr_tx_ibs[i].wr.send_flags =
IB_SEND_SIGNALED | IB_SEND_SOLICITED;
if (send_inline)
- lnk->wr_tx_ibs[i].send_flags |= IB_SEND_INLINE;
+ lnk->wr_tx_ibs[i].wr.send_flags |= IB_SEND_INLINE;
lnk->wr_tx_rdmas[i].wr_tx_rdma[0].wr.opcode = IB_WR_RDMA_WRITE;
lnk->wr_tx_rdmas[i].wr_tx_rdma[1].wr.opcode = IB_WR_RDMA_WRITE;
lnk->wr_tx_rdmas[i].wr_tx_rdma[0].wr.sg_list =
@@ -608,11 +567,11 @@ static void smc_wr_init_sge(struct smc_link *lnk)
lnk->wr_tx_v2_sge->length = SMC_WR_BUF_V2_SIZE;
lnk->wr_tx_v2_sge->lkey = lnk->roce_pd->local_dma_lkey;
- lnk->wr_tx_v2_ib->next = NULL;
- lnk->wr_tx_v2_ib->sg_list = lnk->wr_tx_v2_sge;
- lnk->wr_tx_v2_ib->num_sge = 1;
- lnk->wr_tx_v2_ib->opcode = IB_WR_SEND;
- lnk->wr_tx_v2_ib->send_flags =
+ lnk->wr_tx_v2_ib->wr.next = NULL;
+ lnk->wr_tx_v2_ib->wr.sg_list = lnk->wr_tx_v2_sge;
+ lnk->wr_tx_v2_ib->wr.num_sge = 1;
+ lnk->wr_tx_v2_ib->wr.opcode = IB_WR_SEND;
+ lnk->wr_tx_v2_ib->wr.send_flags =
IB_SEND_SIGNALED | IB_SEND_SOLICITED;
}
@@ -638,10 +597,11 @@ static void smc_wr_init_sge(struct smc_link *lnk)
lnk->wr_rx_sges[x + 1].lkey =
lnk->roce_pd->local_dma_lkey;
}
- lnk->wr_rx_ibs[i].next = NULL;
- lnk->wr_rx_ibs[i].sg_list = &lnk->wr_rx_sges[x];
- lnk->wr_rx_ibs[i].num_sge = lnk->wr_rx_sge_cnt;
+ lnk->wr_rx_ibs[i].wr.next = NULL;
+ lnk->wr_rx_ibs[i].wr.sg_list = &lnk->wr_rx_sges[x];
+ lnk->wr_rx_ibs[i].wr.num_sge = lnk->wr_rx_sge_cnt;
}
+
lnk->wr_reg.wr.next = NULL;
lnk->wr_reg.wr.num_sge = 0;
lnk->wr_reg.wr.send_flags = IB_SEND_SIGNALED;
@@ -657,17 +617,7 @@ void smc_wr_free_link(struct smc_link *lnk)
return;
ibdev = lnk->smcibdev->ibdev;
- smc_wr_drain_cq(lnk);
- smc_wr_wakeup_reg_wait(lnk);
- smc_wr_wakeup_tx_wait(lnk);
-
- smc_wr_tx_wait_no_pending_sends(lnk);
- percpu_ref_kill(&lnk->wr_reg_refs);
- wait_for_completion(&lnk->reg_ref_comp);
- percpu_ref_exit(&lnk->wr_reg_refs);
- percpu_ref_kill(&lnk->wr_tx_refs);
- wait_for_completion(&lnk->tx_ref_comp);
- percpu_ref_exit(&lnk->wr_tx_refs);
+ percpu_ref_exit(&lnk->wr_refs);
if (lnk->wr_rx_dma_addr) {
ib_dma_unmap_single(ibdev, lnk->wr_rx_dma_addr,
@@ -842,30 +792,11 @@ int smc_wr_alloc_link_mem(struct smc_link *link)
return -ENOMEM;
}
-void smc_wr_remove_dev(struct smc_ib_device *smcibdev)
-{
- tasklet_kill(&smcibdev->recv_tasklet);
- tasklet_kill(&smcibdev->send_tasklet);
-}
-
-void smc_wr_add_dev(struct smc_ib_device *smcibdev)
-{
- tasklet_setup(&smcibdev->recv_tasklet, smc_wr_rx_tasklet_fn);
- tasklet_setup(&smcibdev->send_tasklet, smc_wr_tx_tasklet_fn);
-}
-
-static void smcr_wr_tx_refs_free(struct percpu_ref *ref)
+static void smcr_wr_refs_free(struct percpu_ref *ref)
{
- struct smc_link *lnk = container_of(ref, struct smc_link, wr_tx_refs);
+ struct smc_link *lnk = container_of(ref, struct smc_link, wr_refs);
- complete(&lnk->tx_ref_comp);
-}
-
-static void smcr_wr_reg_refs_free(struct percpu_ref *ref)
-{
- struct smc_link *lnk = container_of(ref, struct smc_link, wr_reg_refs);
-
- complete(&lnk->reg_ref_comp);
+ complete(&lnk->ref_comp);
}
int smc_wr_create_link(struct smc_link *lnk)
@@ -873,8 +804,6 @@ int smc_wr_create_link(struct smc_link *lnk)
struct ib_device *ibdev = lnk->smcibdev->ibdev;
int rc = 0;
- smc_wr_tx_set_wr_id(&lnk->wr_tx_id, 0);
- lnk->wr_rx_id = 0;
lnk->wr_rx_dma_addr = ib_dma_map_single(
ibdev, lnk->wr_rx_bufs, lnk->wr_rx_buflen * lnk->wr_rx_cnt,
DMA_FROM_DEVICE);
@@ -913,20 +842,13 @@ int smc_wr_create_link(struct smc_link *lnk)
smc_wr_init_sge(lnk);
bitmap_zero(lnk->wr_tx_mask, lnk->max_send_wr);
init_waitqueue_head(&lnk->wr_tx_wait);
- rc = percpu_ref_init(&lnk->wr_tx_refs, smcr_wr_tx_refs_free, 0, GFP_KERNEL);
- if (rc)
- goto dma_unmap;
- init_completion(&lnk->tx_ref_comp);
init_waitqueue_head(&lnk->wr_reg_wait);
- rc = percpu_ref_init(&lnk->wr_reg_refs, smcr_wr_reg_refs_free, 0, GFP_KERNEL);
+ rc = percpu_ref_init(&lnk->wr_refs, smcr_wr_refs_free, 0, GFP_KERNEL);
if (rc)
- goto cancel_ref;
- init_completion(&lnk->reg_ref_comp);
- init_waitqueue_head(&lnk->wr_rx_empty_wait);
+ goto dma_unmap;
+ init_completion(&lnk->ref_comp);
return rc;
-cancel_ref:
- percpu_ref_exit(&lnk->wr_tx_refs);
dma_unmap:
if (lnk->wr_rx_v2_dma_addr) {
ib_dma_unmap_single(ibdev, lnk->wr_rx_v2_dma_addr,
@@ -947,3 +869,42 @@ int smc_wr_create_link(struct smc_link *lnk)
out:
return rc;
}
+
+void smc_wr_init_cqes(struct smc_link *lnk)
+{
+ int i;
+
+ /* init CQE for WR fast reg */
+ smc_wr_reg_init_cqe(&lnk->wr_reg_cqe);
+ lnk->wr_reg.wr.wr_cqe = &lnk->wr_reg_cqe;
+
+ /* init CQE for WR WRITE */
+ for (i = 0; i < lnk->wr_tx_cnt; i++) {
+ int n;
+
+ smc_wr_tx_rdma_init_cqe(&lnk->wr_tx_rdmas[i].cqe);
+ for (n = 0; n < SMC_MAX_RDMA_WRITES; n++)
+ lnk->wr_tx_rdmas[i].wr_tx_rdma[n].wr.wr_cqe = &lnk->wr_tx_rdmas[i].cqe;
+ }
+
+ /* init CQEs for WR RECV */
+ for (i = 0; i < lnk->wr_rx_cnt; i++) {
+ smc_wr_rx_init_cqe(&lnk->wr_rx_ibs[i].cqe);
+ lnk->wr_rx_ibs[i].wr.wr_cqe = &lnk->wr_rx_ibs[i].cqe;
+ lnk->wr_rx_ibs[i].idx = i;
+ }
+
+ /* init CQEs for WR SEND */
+ for (i = 0; i < lnk->wr_tx_cnt; i++) {
+ smc_wr_tx_init_cqe(&lnk->wr_tx_ibs[i].cqe);
+ lnk->wr_tx_ibs[i].wr.wr_cqe = &lnk->wr_tx_ibs[i].cqe;
+ lnk->wr_tx_ibs[i].idx = i;
+ }
+
+ /* init CQE for SMC-Rv2 WR SEND */
+ if (lnk->lgr->smc_version == SMC_V2) {
+ smc_wr_tx_init_cqe(&lnk->wr_tx_v2_ib->cqe);
+ lnk->wr_tx_v2_ib->wr.wr_cqe = &lnk->wr_tx_v2_ib->cqe;
+ lnk->wr_tx_v2_ib->idx = lnk->wr_tx_cnt;
+ }
+}
diff --git a/net/smc/smc_wr.h b/net/smc/smc_wr.h
index aa4533af9122..a1ff74b455f7 100644
--- a/net/smc/smc_wr.h
+++ b/net/smc/smc_wr.h
@@ -44,35 +44,26 @@ struct smc_wr_rx_handler {
u8 type;
};
-/* Only used by RDMA write WRs.
- * All other WRs (CDC/LLC) use smc_wr_tx_send handling WR_ID implicitly
- */
-static inline long smc_wr_tx_get_next_wr_id(struct smc_link *link)
-{
- return atomic_long_inc_return(&link->wr_tx_id);
-}
-
-static inline void smc_wr_tx_set_wr_id(atomic_long_t *wr_tx_id, long val)
-{
- atomic_long_set(wr_tx_id, val);
-}
-
static inline bool smc_wr_tx_link_hold(struct smc_link *link)
{
+ /* We still keep this check, although percpu_ref_tryget_live()
+ * is the real safety guarantee.
+ * It provides a quick reject path, but nothing more than that.
+ */
if (!smc_link_sendable(link))
return false;
- percpu_ref_get(&link->wr_tx_refs);
- return true;
+ return percpu_ref_tryget_live(&link->wr_refs);
}
static inline void smc_wr_tx_link_put(struct smc_link *link)
{
- percpu_ref_put(&link->wr_tx_refs);
+ percpu_ref_put(&link->wr_refs);
}
-static inline void smc_wr_drain_cq(struct smc_link *lnk)
+static inline void smc_wr_drain_qp(struct smc_link *lnk)
{
- wait_event(lnk->wr_rx_empty_wait, lnk->wr_rx_id_compl == lnk->wr_rx_id);
+ if (lnk->qp_attr.cur_qp_state != IB_QPS_RESET)
+ ib_drain_qp(lnk->roce_qp);
}
static inline void smc_wr_wakeup_tx_wait(struct smc_link *lnk)
@@ -86,29 +77,22 @@ static inline void smc_wr_wakeup_reg_wait(struct smc_link *lnk)
}
/* post a new receive work request to fill a completed old work request entry */
-static inline int smc_wr_rx_post(struct smc_link *link)
+static inline int smc_wr_rx_post(struct smc_link *link, struct ib_cqe *cqe)
{
- int rc;
- u64 wr_id, temp_wr_id;
- u32 index;
-
- wr_id = ++link->wr_rx_id; /* tasklet context, thus not atomic */
- temp_wr_id = wr_id;
- index = do_div(temp_wr_id, link->wr_rx_cnt);
- link->wr_rx_ibs[index].wr_id = wr_id;
- rc = ib_post_recv(link->roce_qp, &link->wr_rx_ibs[index], NULL);
- return rc;
+ struct smc_ib_recv_wr *recv_wr;
+
+ recv_wr = container_of(cqe, struct smc_ib_recv_wr, cqe);
+ return ib_post_recv(link->roce_qp, &recv_wr->wr, NULL);
}
int smc_wr_create_link(struct smc_link *lnk);
int smc_wr_alloc_link_mem(struct smc_link *lnk);
int smc_wr_alloc_lgr_mem(struct smc_link_group *lgr);
+void smc_wr_stop_link(struct smc_link *lnk);
void smc_wr_free_link(struct smc_link *lnk);
void smc_wr_free_link_mem(struct smc_link *lnk);
void smc_wr_free_lgr_mem(struct smc_link_group *lgr);
void smc_wr_remember_qp_attr(struct smc_link *lnk);
-void smc_wr_remove_dev(struct smc_ib_device *smcibdev);
-void smc_wr_add_dev(struct smc_ib_device *smcibdev);
int smc_wr_tx_get_free_slot(struct smc_link *link, smc_wr_tx_handler handler,
struct smc_wr_buf **wr_buf,
@@ -126,12 +110,12 @@ int smc_wr_tx_v2_send(struct smc_link *link,
struct smc_wr_tx_pend_priv *priv, int len);
int smc_wr_tx_send_wait(struct smc_link *link, struct smc_wr_tx_pend_priv *priv,
unsigned long timeout);
-void smc_wr_tx_cq_handler(struct ib_cq *ib_cq, void *cq_context);
void smc_wr_tx_wait_no_pending_sends(struct smc_link *link);
int smc_wr_rx_register_handler(struct smc_wr_rx_handler *handler);
int smc_wr_rx_post_init(struct smc_link *link);
-void smc_wr_rx_cq_handler(struct ib_cq *ib_cq, void *cq_context);
int smc_wr_reg_send(struct smc_link *link, struct ib_mr *mr);
+void smc_wr_init_cqes(struct smc_link *lnk);
+
#endif /* SMC_WR_H */
--
2.45.0
^ permalink raw reply related
* [PATCH net-next v4 0/3] net/smc: transition to RDMA core CQ pooling
From: D. Wythe @ 2026-07-16 11:37 UTC (permalink / raw)
To: mjambigi, wenjia, wintera, dust.li, tonylu, guwen
Cc: kuba, davem, netdev, linux-s390, linux-rdma, leonro, pabeni,
edumazet, sidraya, jaka, oliver.yang
This series transitions SMC-R completion handling to RDMA core CQ pooling
via the ib_cqe API. The new completion model improves scalability by
allowing per-link completion processing across multiple cores and enables
DIM-based interrupt moderation.
The increased concurrency can amplify contention for TX slots on the shared
wait queue, so the exclusive-wait change comes first as a standalone
improvement that the CQ pooling teardown then builds on.
Patch 1 fixes smc_wr_tx_put_slot() to clear the v2 pending slot and buffer
structures instead of the pointer variables.
Patch 2 reduces TX slot contention by switching TX slot allocation from
non-exclusive wait_event() to prepare_to_wait_exclusive().
Patch 3 replaces the global per-device CQ and manual tasklet polling model
with RDMA core CQ pooling.
Link: https://lore.kernel.org/netdev/20260305022323.96125-1-alibuda@linux.alibaba.com/
---
Changes v1 -> v2:
https://lore.kernel.org/netdev/20260508063718.101622-1-alibuda@linux.alibaba.com/
1. remove unnecessary inline from static CQE init helpers.
2. Use ib_drain_qp() with +1 max_send_wr;
3. Fix v2 state clearing.
4. Add re-check after schedule_timeout() to fix timeout/signal races.
Changes v2 -> v3:
https://lore.kernel.org/netdev/20260528084819.6059-1-alibuda@linux.alibaba.com/
1. Reserve +3 instead of +1 for the SQ to cover the drain, FastReg and
SMC-Rv2 SEND WRs, avoiding SQ exhaustion that breaks ib_drain_sq().
2. Guard the recv WR repost with a per-link percpu_ref so no WR is
reposted after ib_drain_qp(), fixing the RX repost/drain use-after-free.
3. Split the smc_wr_tx_put_slot() v2 clearing fix into a separate patch
(1/3) with a Fixes: tag and reworded to the verifiable root cause.
Changes v3 -> v4:
https://lore.kernel.org/netdev/20260710033356.16460-1-alibuda@linux.alibaba.com/
1. Reorder: move the exclusive-wait patch before the CQ pooling patch.
2. Fix teardown stall: reintroduce smc_ib_modify_qp_error() and flush the QP
in smc_wr_stop_link() before waiting for the refs.
3. smc_wr_reg_send(): take the ref before ib_post_send().
4. smc_wr_tx_process_cqe(): wake_up_all() on a flush completion.
5. Merge the three per-link percpu_refs into a single wr_refs.
D. Wythe (3):
net/smc: clear the correct v2 slot and buffer in smc_wr_tx_put_slot()
net/smc: reduce TX slot contention with exclusive wait
net/smc: transition to RDMA core CQ pooling
net/smc/smc_core.c | 10 +-
net/smc/smc_core.h | 36 ++--
net/smc/smc_ib.c | 106 ++++-------
net/smc/smc_ib.h | 6 -
net/smc/smc_tx.c | 1 -
net/smc/smc_wr.c | 445 +++++++++++++++++++++------------------------
net/smc/smc_wr.h | 50 ++---
7 files changed, 287 insertions(+), 367 deletions(-)
--
2.45.0
^ permalink raw reply
* [PATCH net-next v4 2/3] net/smc: reduce TX slot contention with exclusive wait
From: D. Wythe @ 2026-07-16 11:37 UTC (permalink / raw)
To: mjambigi, wenjia, wintera, dust.li, tonylu, guwen
Cc: kuba, davem, netdev, linux-s390, linux-rdma, leonro, pabeni,
edumazet, sidraya, jaka, oliver.yang
In-Reply-To: <20260716113745.65234-1-alibuda@linux.alibaba.com>
smc_wr_tx_get_free_slot() waits for a free TX slot with
wait_event_interruptible_timeout(). Since the wait_event family
enqueues waiters as non-exclusive, wake_up() may wake multiple
waiters even though only one can use the slot, causing
thundering-herd contention when slots are scarce.
Use an exclusive wait loop with prepare_to_wait_exclusive() so
wake_up() wakes only one waiter per freed slot.
smc_wr_wakeup_tx_wait() still uses wake_up_all() during link
teardown, so teardown behavior is unchanged.
This also corrects the return value on a pending signal: the previous
wait_event_interruptible_timeout() path fell through to the "no free
slot" case and returned -EPIPE, masking the signal as a connection
error. The open-coded loop now returns -ERESTARTSYS, matching the
standard interruptible-wait semantics and letting the syscall restart
machinery handle it.
Signed-off-by: D. Wythe <alibuda@linux.alibaba.com>
---
net/smc/smc_wr.c | 36 ++++++++++++++++++++++++++----------
1 file changed, 26 insertions(+), 10 deletions(-)
diff --git a/net/smc/smc_wr.c b/net/smc/smc_wr.c
index 6b5add922993..2cdd4063c13d 100644
--- a/net/smc/smc_wr.c
+++ b/net/smc/smc_wr.c
@@ -198,11 +198,13 @@ int smc_wr_tx_get_free_slot(struct smc_link *link,
struct smc_rdma_wr **wr_rdma_buf,
struct smc_wr_tx_pend_priv **wr_pend_priv)
{
+ unsigned long timeout = SMC_WR_TX_WAIT_FREE_SLOT_TIME;
struct smc_link_group *lgr = smc_get_lgr(link);
struct smc_wr_tx_pend *wr_pend;
u32 idx = link->wr_tx_cnt;
struct ib_send_wr *wr_ib;
u64 wr_id;
+ DEFINE_WAIT(wait);
int rc;
*wr_buf = NULL;
@@ -212,17 +214,31 @@ int smc_wr_tx_get_free_slot(struct smc_link *link,
if (rc)
return rc;
} else {
- rc = wait_event_interruptible_timeout(
- link->wr_tx_wait,
- !smc_link_sendable(link) ||
- lgr->terminating ||
- (smc_wr_tx_get_free_slot_index(link, &idx) != -EBUSY),
- SMC_WR_TX_WAIT_FREE_SLOT_TIME);
- if (!rc) {
- /* timeout - terminate link */
- smcr_link_down_cond_sched(link);
- return -EPIPE;
+ rc = 0;
+ for (;;) {
+ prepare_to_wait_exclusive(&link->wr_tx_wait, &wait,
+ TASK_INTERRUPTIBLE);
+ if (!smc_link_sendable(link) || lgr->terminating ||
+ smc_wr_tx_get_free_slot_index(link, &idx) != -EBUSY)
+ break;
+ timeout = schedule_timeout(timeout);
+ /* re-check */
+ if (!smc_link_sendable(link) || lgr->terminating ||
+ smc_wr_tx_get_free_slot_index(link, &idx) != -EBUSY)
+ break;
+ if (!timeout) {
+ /* timeout - terminate link */
+ smcr_link_down_cond_sched(link);
+ break;
+ }
+ if (signal_pending(current)) {
+ rc = -ERESTARTSYS;
+ break;
+ }
}
+ finish_wait(&link->wr_tx_wait, &wait);
+ if (rc)
+ return rc;
if (idx == link->wr_tx_cnt)
return -EPIPE;
}
--
2.45.0
^ permalink raw reply related
* [PATCH net-next v4 1/3] net/smc: clear the correct v2 slot and buffer in smc_wr_tx_put_slot()
From: D. Wythe @ 2026-07-16 11:37 UTC (permalink / raw)
To: mjambigi, wenjia, wintera, dust.li, tonylu, guwen
Cc: kuba, davem, netdev, linux-s390, linux-rdma, leonro, pabeni,
edumazet, sidraya, jaka, oliver.yang
In-Reply-To: <20260716113745.65234-1-alibuda@linux.alibaba.com>
smc_wr_tx_put_slot() tries to reset the v2 pending slot and buffer with
memset(&link->wr_tx_v2_pend, 0, sizeof(link->wr_tx_v2_pend)) and the
equivalent for wr_tx_buf_v2. Both are pointers, so this zeroes the 8-byte
pointer variable instead of the structure it points to. The pending slot
and buffer are therefore never actually cleared, and the pointers get
overwritten with NULL.
Pass the pointers directly and use sizeof(*pointer) so the intended
structures are cleared.
Fixes: 8799e310fb3f ("net/smc: add v2 support to the work request layer")
Signed-off-by: D. Wythe <alibuda@linux.alibaba.com>
Reviewed-by: Dust Li <dust.li@linux.alibaba.com>
---
net/smc/smc_wr.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/smc/smc_wr.c b/net/smc/smc_wr.c
index 59c92b46945c..6b5add922993 100644
--- a/net/smc/smc_wr.c
+++ b/net/smc/smc_wr.c
@@ -288,10 +288,10 @@ int smc_wr_tx_put_slot(struct smc_link *link,
} else if (link->lgr->smc_version == SMC_V2 &&
pend->idx == link->wr_tx_cnt) {
/* Large v2 buffer */
- memset(&link->wr_tx_v2_pend, 0,
- sizeof(link->wr_tx_v2_pend));
- memset(&link->lgr->wr_tx_buf_v2, 0,
- sizeof(link->lgr->wr_tx_buf_v2));
+ memset(link->wr_tx_v2_pend, 0,
+ sizeof(*link->wr_tx_v2_pend));
+ memset(link->lgr->wr_tx_buf_v2, 0,
+ sizeof(*link->lgr->wr_tx_buf_v2));
return 1;
}
--
2.45.0
^ permalink raw reply related
* Re: [PATCH v4 net-next 2/7] selftests/ptp: Extract print_system_timestamp helper in testptp
From: saeed bishara @ 2026-07-16 11:34 UTC (permalink / raw)
To: Arthur Kiyanovski
Cc: David Miller, Jakub Kicinski, netdev, Richard Cochran,
Eric Dumazet, Paolo Abeni, David Woodhouse, Thomas Gleixner,
Miroslav Lichvar, Andrew Lunn, Wen Gu, Xuan Zhuo, David Woodhouse,
Yonatan Sarna, Zorik Machulsky, Alexander Matushevsky,
Saeed Bshara, Matt Wilson, Anthony Liguori, Nafea Bshara,
Evgeny Schmeilin, Netanel Belgazal, Ali Saidi,
Benjamin Herrenschmidt, Noam Dagan, David Arinzon,
Evgeny Ostrovsky, Ofir Tabachnik, Amit Bernstein, linux-kselftest,
linux-doc, shuah, Jonathan Corbet, Shuah Khan, Simon Horman,
vadim.fedorenko
In-Reply-To: <20260714020340.25014-3-akiyano@amazon.com>
>
> +static void print_system_timestamp(int sample_num, __kernel_clockid_t clockid,
> + long long sec, unsigned int nsec,
> + const char *when)
> +{
> + switch (clockid) {
> + case CLOCK_REALTIME:
> + printf("sample #%2d: real time %s: %lld.%09u\n",
> + sample_num, when, sec, nsec);
> + break;
> + case CLOCK_MONOTONIC:
> + printf("sample #%2d: monotonic time %s: %lld.%09u\n",
> + sample_num, when, sec, nsec);
> + break;
> + case CLOCK_MONOTONIC_RAW:
> + printf("sample #%2d: monotonic-raw time %s: %lld.%09u\n",
> + sample_num, when, sec, nsec);
> + break;
These three printfs are also kind of duplicated, I think mapping the
clockid to name first would save a few lines.
^ permalink raw reply
* Re: [PATCH net-next 0/3] net: nexthop: per-nexthop UDP dst port for fdb (VXLAN) nexthops
From: Ido Schimmel @ 2026-07-16 11:33 UTC (permalink / raw)
To: Jack Ma
Cc: netdev, David Ahern, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Shuah Khan,
linux-kselftest, linux-kernel
In-Reply-To: <20260715074317.2271-1-jack4it@gmail.com>
On Wed, Jul 15, 2026 at 07:43:17AM +0000, Jack Ma wrote:
> On Tue, Jul 14, 2026 at 08:42:01PM +0300, Ido Schimmel wrote:
> > It's unclear to me why you need the UDP port based demux. On a single
> > node, do you have multiple pods that belong to the same tenant and
> > therefore share the same VNI? From the above, my understanding is that
> > the answer is "no".
>
> Good question, and I think my earlier framing sent you down the wrong
> path, sorry about that. The honest answer is actually "yes". By design
> a node can hold several VTEPs on the same VNI, as long as they belong
> to different HA sets. Anti-affinity only keeps the two replicas within
> one HA set off the same node. Two pods from different HA sets on the
> same VNI are free to co-locate, and that co-location is the case that
> needs a per-pod port.
>
> > Assuming the answer is "no", instead of having multiple VXLAN sockets
> > in the host netns (each bound to a different port), why not have a
> > single VXLAN socket bound to the standard port (4789) and let the
> > VXLAN driver perform the demux based on the VNI? [...] creating
> > multiple VXLAN devices in the host netns that only differ in their
> > VNI and then move each to the relevant pod netns.
>
> For different-VNI pods that is a clean fit, and I want to try it out
> regardless. The outer packet still carries the VNI, so a shared socket
> can steer by VNI without decapsulating, and the host never touches the
> tenant frame.
>
> Where it stops working for me is two same-VNI pods on one node. They
> share the underlay IP, the port (4789), and the VNI, so the outer
> header is the same for both:
>
> outer: NodeIP:4789, VNI=X -> pod A (HA set 1)
> outer: NodeIP:4789, VNI=X -> pod B (HA set 2)
>
> There is nothing left in the outer header to steer on, so the host
> would have to look past it and terminate the tunnel itself, which is
> the VTEP role I would rather keep inside the pod. A distinct port per
> pod puts the distinguishing bit back in the outer header, so the host
> can stay a stateless outer-UDP forwarder.
>
> > Given the anti-affinity constraint, you will only need to load balance
> > between pods on different nodes that use the standard port and that is
> > already supported by the FDB nexthop groups.
>
> Agreed that the across-node load balancing already works. What the
> VNI-demux version costs is packing density. With one shared underlay
> IP per node, VNI demux allows at most one VTEP per (VNI, node), so N
> same-VNI HA sets of two replicas need 2N nodes. With a per-pod port
> the same sets fit on two nodes (replica 1 on node A, replica 2 on node
> B, each pod on its own port), and anti-affinity still holds. For 5
> sets on one VNI that is 10 nodes versus 2.
OK, I understand. Each tenant has its own VNI, but it can have multiple
HA sets on the same node and you want to distinguish between them using
the UDP destination port.
>
> So the port is not strictly required, and your VNI-demux model is a
> real option I will prototype. The tradeoff I am weighing is density
> plus keeping the host out of the tenant datapath, against the uAPI
> cost, which is small: NHA_FDB_PORT is the nexthop analog of the
> per-entry NDA_PORT that already exists, it is control-plane only, and
> a leg with no port behaves exactly as today.
I don't particularly like NHA_FDB_PORT because it's an FDB-specific
encapsulation attribute and these attributes naturally belong under
NHA_ENCAP. That being said, I fail to see a better alternative given
that FDB nexthops don't use NHA_ENCAP / LWT infra and probably
shouldn't. It's a bigger change in the datapath and I expect that we
will not need too many FDB-specific attributes in the future.
If you are going to post v2, please provide a more detailed motivation
in the cover letter.
>
> Happy to keep digging if it is useful. Thanks again for thinking it
> through with me.
In an earlier reply you wrote "shared inner MAC/IP". The "MAC" part was
intentional because you need to load balance pure L2 traffic? IOW, if
your traffic is only L3, can we do the load balancing using multipath
routes in the overlay instead of using FDB nexthop groups?
Related: What is the use case for NHA_FDB_VNI? In my experience, the VNI
is only translated when routing traffic between tenants. Why do you need
this in an FDB nexthop?
^ permalink raw reply
* Re: [PATCH v3 net 1/6] xsk: fix buffer leak in xsk_drop_skb() for AF_XDP multi-buffer Tx
From: Maciej Fijalkowski @ 2026-07-16 11:25 UTC (permalink / raw)
To: Jason Xing
Cc: netdev, bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
bjorn, Jason Xing
In-Reply-To: <CAL+tcoAc7d8MSCqK23_pmL2G8tSS-MpfM53brijPntv5A+BA2Q@mail.gmail.com>
On Thu, Jul 16, 2026 at 01:22:24PM +0200, Jason Xing wrote:
> On Tue, Jul 14, 2026 at 4:08 PM Maciej Fijalkowski
> <maciej.fijalkowski@intel.com> wrote:
> >
> > 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")
> > Signed-off-by: Jason Xing <kernelxing@tencent.com>
>
> Maciej, maybe your tag is missing here?
IIRC this has been taken as-is from your patchset. The next one has been
touched in few ways and I included my co-developed tag there.
>
> Thanks,
> Jason
^ permalink raw reply
* Re: [PATCH v3 net 1/6] xsk: fix buffer leak in xsk_drop_skb() for AF_XDP multi-buffer Tx
From: Jason Xing @ 2026-07-16 11:22 UTC (permalink / raw)
To: Maciej Fijalkowski
Cc: netdev, bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
bjorn, Jason Xing
In-Reply-To: <20260714140722.111645-2-maciej.fijalkowski@intel.com>
On Tue, Jul 14, 2026 at 4:08 PM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> 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")
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
Maciej, maybe your tag is missing here?
Thanks,
Jason
^ permalink raw reply
* Re: [PATCH v4 net-next 1/7] ptp: Add ioctls for PHC timestamps with quality attributes
From: saeed bishara @ 2026-07-16 11:22 UTC (permalink / raw)
To: Arthur Kiyanovski
Cc: David Miller, Jakub Kicinski, netdev, Richard Cochran,
Eric Dumazet, Paolo Abeni, David Woodhouse, Thomas Gleixner,
Miroslav Lichvar, Andrew Lunn, Wen Gu, Xuan Zhuo, David Woodhouse,
Yonatan Sarna, Zorik Machulsky, Alexander Matushevsky,
Saeed Bshara, Matt Wilson, Anthony Liguori, Nafea Bshara,
Evgeny Schmeilin, Netanel Belgazal, Ali Saidi,
Benjamin Herrenschmidt, Noam Dagan, David Arinzon,
Evgeny Ostrovsky, Ofir Tabachnik, Amit Bernstein, linux-kselftest,
linux-doc, shuah, Jonathan Corbet, Shuah Khan, Simon Horman,
vadim.fedorenko
In-Reply-To: <20260714020340.25014-2-akiyano@amazon.com>
On Tue, Jul 14, 2026 at 5:06 AM Arthur Kiyanovski <akiyano@amazon.com> wrote:
> + n_samples = data->request.num_samples;
> + sts.clockid = data->request.clock_id;
> + kfree(data);
> + data = kzalloc(struct_size(data, timestamps, n_samples), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
any idea why you free then allocate back?
> --- a/include/linux/ptp_clock_kernel.h
> +++ b/include/linux/ptp_clock_kernel.h
> @@ -123,11 +123,34 @@ struct ptp_system_timestamp {
> * reading the lowest bits of the PHC timestamp and the second
> * reading immediately follows that.
> *
> + * @gettimexattrs64: Reads the current time from the hardware clock and
> + * optionally also the system clock with additional clock
> + * attributes.
> + * parameter ts: Holds the PHC timestamp.
> + * parameter sts: If not NULL, it holds a pair of
> + * timestamps from the system clock. The first reading is
> + * made right before reading the lowest bits of the PHC
> + * timestamp and the second reading immediately follows that.
The descriptions for ts and sts here are identical to gettimex64.
Instead of duplicating this text, could we just refer back to the
gettimex64 documentation?
^ permalink raw reply
* Re: ipv4: icmp: icmp_route_lookup() relookups pick wrong netdev with policy routing + strict rp_filter
From: Eric Dumazet @ 2026-07-16 11:22 UTC (permalink / raw)
To: Muhammad Ziad
Cc: netdev, David Ahern, Jakub Kicinski, Paolo Abeni, David S. Miller,
linux-kernel
In-Reply-To: <CAOAwikBYp7AAxFAZgyLaOZEG6Eoomq3aqU9ViMS8nrO82PD2rw@mail.gmail.com>
On Thu, Jul 16, 2026 at 12:40 PM Muhammad Ziad <muhzi100@gmail.com> wrote:
>
> Thank you for the fix, Eric. Applying the other selectors from skb_in
> makes sense to me, but I'm not sure if instead we should copy them
> from fl4_dec?
>
> The mark param e.g. is gated by IP4_REPLY_MARK() on fwmark_reflect,
> which could be different from skb_in->mark.
>
Yes, sashiko had some remarks.
I have been playing with:
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 23e921d313b36b00d8ae5e14846527220c9db32b..b0eb4f8ff9867499dc3a96c92a414440b2d3a115
100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -548,10 +548,12 @@ static struct rtable *icmp_route_lookup(struct
net *net, struct flowi4 *fl4,
if (IS_ERR(rt2))
err = PTR_ERR(rt2);
} else {
- struct flowi4 fl4_2 = {};
+ struct flowi4 fl4_2 = fl4_dec;
unsigned long orefdst;
fl4_2.daddr = fl4_dec.saddr;
+ fl4_2.saddr = fl4_dec.daddr;
+ fl4_2.flowi4_oif = l3mdev_master_ifindex(route_lookup_dev);
rt2 = ip_route_output_key(net, &fl4_2);
if (IS_ERR(rt2)) {
err = PTR_ERR(rt2);
>
> On Thu, Jul 16, 2026 at 3:51 AM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Wed, Jul 15, 2026 at 5:43 PM Eric Dumazet <edumazet@google.com> wrote:
> > >
> > > On Wed, Jul 15, 2026 at 5:30 PM Muhammad Ziad <muhzi100@gmail.com> wrote:
> > > >
> > > > Hello,
> > > >
> > > > There appears to be a bug in mainline Linux in ICMP reverse-path
> > > > relookup logic inside icmp_route_lookup() (called by __icmp_send())
> > > > when strict rp_filter setting is in place.
> > > >
> > > > When Linux forwards a packet between two interfaces and needs to
> > > > generate an ICMP error, icmp_route_lookup() performs a "secondary"
> > > > reverse-path lookup to find a suitable route back towards the original
> > > > source via ip_route_input(). To simulate the reverse path, the kernel
> > > > derives the incoming netdev by calling ip_route_output_key() with a
> > > > decoy flow that has *only* daddr assigned in it:
> > > >
> > > > struct flowi4 fl4_2 = {};
> > > > fl4_2.daddr = fl4_dec.saddr;
> > > > rt2 = ip_route_output_key(net, &fl4_2); /* no saddr */
> > > > ...
> > > > ip_route_input(skb_in, fl4_dec.daddr, fl4_dec.saddr,
> > > > dscp, rt2->dst.dev);
> > > >
> > > > This can lead to a mismatch between rt2->dst.dev and the netdev the
> > > > real reverse packet would use once routing policy rules are in
> > > > effect. With strict rp_filter, passing the wrong netdev to
> > > > ip_route_input() causes the relookup to fail and a "martian source"
> > > > message to be logged, after which icmp_route_lookup() falls back to
> > > > the earlier output route lookup (relookup_failed).
> > > >
> > > > In such a scenario, I would expect the relookup to use a canonical
> > > > netdev and the kernel to not produce spurious "martian source" log
> > > > messages as a result. This suggests the decoy flow would possibly
> > > > need to carry saddr too so that ip_route_output_key() is able to
> > > > resolve the right netdev.
> > > >
> > > > I tested this on: Ubuntu kernel 6.17.0-35-generic.
> > > >
> > > > Here is a reproducer script that sets up two net namespaces: a
> > > > "forwarder" with two routes to the same dst in separate routing tables
> > > > picked according to saddr, and a "sender" netns behind it which sends
> > > > a ping with ttl=1 via the forwarder forcing it to generate an ICMP
> > > > error as a response which leads to the result explained above:
> > > >
> > > > #!/bin/bash
> > > >
> > > > if [ "${forwarder_ns:-}" != "1" ]; then
> > > > exec env forwarder_ns=1 unshare -Urn bash "$0" "$@"
> > > > fi
> > > >
> > > > SRC=10.0.1.2
> > > > DST=198.51.100.5
> > > >
> > > > # Current netns is the "forwarder".
> > > > # Create a second namespace for the sender.
> > > > unshare -n sleep 120 &
> > > > cpid=$!
> > > > trap 'kill "$cpid" 2>/dev/null || true' EXIT
> > > > in_ns() { nsenter -t "$cpid" -n "$@"; }
> > > >
> > > > # veth r0(router) <-> s0(src)
> > > > ip link add s0 type veth peer name r0
> > > > ip link set s0 netns "$cpid"
> > > > ip link set lo up
> > > > ip link set r0 up
> > > > ip addr add 10.0.1.1/24 dev r0
> > > > ip link add dumA type dummy
> > > > ip addr add 203.0.113.1/24 dev dumA
> > > > ip link set dumA up
> > > > ip link add dumB type dummy
> > > > ip addr add 192.0.2.1/24 dev dumB
> > > > ip link set dumB up
> > > >
> > > > sysctl -q -w net.ipv4.ip_forward=1
> > > > for c in all default r0 dumA dumB; do
> > > > sysctl -q -w "net.ipv4.conf.$c.rp_filter=1"
> > > > sysctl -q -w "net.ipv4.conf.$c.log_martians=1"
> > > > done
> > > >
> > > > # Destination reachable two ways.
> > > > # Policy rule diverts traffic FROM src to dumB
> > > > ip route add 198.51.100.0/24 dev dumA
> > > > ip route add 198.51.100.0/24 dev dumB table 100
> > > > ip rule add from "$SRC" lookup 100
> > > >
> > > > # sender namespace setup.
> > > > in_ns ip link set lo up
> > > > in_ns ip link set s0 up
> > > > in_ns ip addr add 10.0.1.2/24 dev s0
> > > > in_ns ip route add default via 10.0.1.1
> > > >
> > > > # This will trigger a "martian source" log.
> > > > in_ns ping -q -c1 -W2 -t1 "$DST" &>/dev/null
> > > >
> > > >
> > > > Happy to test patches or provide additional traces, if needed.
> > > >
> > > > Thank you,
> > > > Mohamed Ghazy
> > >
> > > Thanks for the report.
> > >
> > > It seems we are lacking more than saddr setting :/
> > >
> > > Could you test
> > >
> > > diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> > > index 23e921d313b36b00d8ae5e14846527220c9db32b..6277e1bf85f304678ff167e8fd2b9239f155ce42
> > > 100644
> > > --- a/net/ipv4/icmp.c
> > > +++ b/net/ipv4/icmp.c
> > > @@ -548,10 +548,17 @@ static struct rtable *icmp_route_lookup(struct
> > > net *net, struct flowi4 *fl4,
> > > if (IS_ERR(rt2))
> > > err = PTR_ERR(rt2);
> > > } else {
> > > - struct flowi4 fl4_2 = {};
> > > + /* TODO: populate
> > > + .flowi4_dscp = dscp,
> > > + .flowi4_mark = mark,
> > > + .flowi4_uid = sock_net_uid(net, NULL),
> > > + */
> > > + struct flowi4 fl4_2 = {
> > > + .daddr = fl4_dec.saddr,
> > > + .saddr = fl4_dec.daddr,
> > > + };
> > > unsigned long orefdst;
> > >
> > > - fl4_2.daddr = fl4_dec.saddr;
> > > rt2 = ip_route_output_key(net, &fl4_2);
> > > if (IS_ERR(rt2)) {
> > > err = PTR_ERR(rt2);
> >
> > I have tested the following fix, PTAL, thanks.
> >
> > diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> > index 23e921d313b36b00d8ae5e14846527220c9db32b..2259f643cb0d43ea54819dd831c79807413e0d69
> > 100644
> > --- a/net/ipv4/icmp.c
> > +++ b/net/ipv4/icmp.c
> > @@ -548,10 +548,16 @@ static struct rtable *icmp_route_lookup(struct
> > net *net, struct flowi4 *fl4,
> > if (IS_ERR(rt2))
> > err = PTR_ERR(rt2);
> > } else {
> > - struct flowi4 fl4_2 = {};
> > + struct flowi4 fl4_2 = {
> > + .daddr = fl4_dec.saddr,
> > + .saddr = fl4_dec.daddr,
> > + .flowi4_dscp = dscp,
> > + .flowi4_proto = IPPROTO_ICMP,
> > + .flowi4_mark = mark,
> > + .flowi4_uid = sock_net_uid(net, NULL),
> > + };
> > unsigned long orefdst;
> >
> > - fl4_2.daddr = fl4_dec.saddr;
> > rt2 = ip_route_output_key(net, &fl4_2);
> > if (IS_ERR(rt2)) {
> > err = PTR_ERR(rt2);
^ permalink raw reply
* Re: [PATCH] sctp: don't free the ASCONF's own transport in DEL-IP processing
From: Jason Xing @ 2026-07-16 11:12 UTC (permalink / raw)
To: 朱雀蓝军AIGBot邮箱
Cc: netdev, Marcelo Ricardo Leitner, Xin Long, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
linux-sctp, linux-kernel, Jun Yang, stable
In-Reply-To: <80818E6580896D22+20260716091212.43126-1-junvyyang@tencent.com>
On Thu, Jul 16, 2026 at 11:16 AM 朱雀蓝军AIGBot邮箱 <aigbot@tencent.com> wrote:
>
> sctp_process_asconf() caches the transport the ASCONF chunk is processed
> against in asconf->transport (== chunk->transport, set once in sctp_rcv()).
> For an ASCONF located through its Address Parameter by
> __sctp_rcv_asconf_lookup(), that cached transport corresponds to the
> Address Parameter, which need not be the packet's source address.
>
> sctp_process_asconf_param() rejects a DEL-IP for the packet source address
> (ADDIP D8, SCTP_ERROR_DEL_SRC_IP), but nothing protects asconf->transport.
> A single ASCONF can therefore carry, in order:
>
> [Address Parameter L] [DEL-IP L] [DEL-IP 0.0.0.0]
>
> where L differs from the source. The DEL-IP for L passes the D8 check and
> calls sctp_assoc_rm_peer() on the transport that asconf->transport still
> points at, freeing it (RCU-deferred). The following wildcard DEL-IP then
> reuses the now-dangling asconf->transport in sctp_assoc_set_primary() and
> sctp_assoc_del_nonprimary_peers(): set_primary() dereferences the freed
> transport (->ipaddr, ->state) and plants the dangling pointer into
> asoc->peer.primary_path / active_path, and del_nonprimary_peers(), keeping
> only the pointer that is no longer on the list, removes every real
> transport, leaving the association with a transport_count of 0 and
> primary_path/active_path pointing at freed memory.
>
> Reject a DEL-IP that targets the transport the ASCONF is being processed
> against, mirroring the existing source-address guard, so the wildcard
> branch can never reuse a freed transport.
>
> Fixes: 42e30bf3463c ("[SCTP]: Handle the wildcard ADD-IP Address parameter")
> Cc: stable@kernel.org
> Signed-off-by: Jun Yang <junvyyang@tencent.com>
> Acked-by: Xin Long <lucien.xin@gmail.com>
It should explicitly target the net branch?
Thanks,
Jason
> ---
> net/sctp/sm_make_chunk.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> index 8adac9e0cd66..b14251214896 100644
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c
> @@ -3153,6 +3153,12 @@ static __be16 sctp_process_asconf_param(struct sctp_association *asoc,
> if (!peer)
> return SCTP_ERROR_DNS_FAILED;
>
> + /* Don't free asconf->transport; a later wildcard DEL-IP
> + * parameter reuses it.
> + */
> + if (peer == asconf->transport)
> + return SCTP_ERROR_DEL_SRC_IP;
> +
> sctp_assoc_rm_peer(asoc, peer);
> break;
> case SCTP_PARAM_SET_PRIMARY:
> --
> 2.55.0
>
>
^ permalink raw reply
* Re: [PATCH net-next v2 0/2] net: dsa: mv88e6xxx: various hwstamp fixes
From: Simon Horman @ 2026-07-16 10:57 UTC (permalink / raw)
To: Luke Howard
Cc: Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Vivien Didelot, Gregory CLEMENT, Andrew Lunn,
Richard Cochran, Cedric Jehasse, Kieran Tyrrell, Max Holtmann,
Max Hunter, Christoph Mellauner, Simon Gapp, netdev, linux-kernel
In-Reply-To: <20260710-mv88e6xxx-ptp-fixes-v2-0-af97c38df247@padl.com>
On Fri, Jul 10, 2026 at 04:43:40PM +1000, Luke Howard wrote:
> Two fixes for improving the reliably of hardware timestamp acquisition
> on Marvell switches. In our tests this eliminated missed timestamps in
> ptp4l.
>
> Signed-off-by: Luke Howard <lukeh@padl.com>
> ---
> Changes in v2:
> - Lineraize SKBs before retrieving embedded timestamp
> - Hoist driver register lock out of per-frame loop
> - Link to v1: https://patch.msgid.link/20260703-mv88e6xxx-ptp-fixes-v1-0-0138581889a9@padl.com
For the series:
Reviewed-by: Simon Horman <horms@kernel.org>
^ permalink raw reply
* Re: [PATCH v3 net 3/6] xsk: provide sufficient space in pool->tx_descs
From: Jason Xing @ 2026-07-16 10:56 UTC (permalink / raw)
To: Maciej Fijalkowski
Cc: netdev, bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
bjorn
In-Reply-To: <ali2v31fXqzAKrEP@boxer>
On Thu, Jul 16, 2026 at 12:47 PM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Thu, Jul 16, 2026 at 11:29:49AM +0200, Jason Xing wrote:
> > On Tue, Jul 14, 2026 at 4:08 PM Maciej Fijalkowski
> > <maciej.fijalkowski@intel.com> wrote:
> > >
> > > 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")
> > > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> >
> > Reviewed-by: Jason Xing <kerneljasonxing@gmail.com>
> >
> > I noticed there is one interesting comment[1] from sashiko, which
> > actually I think is valid. Probably we don't need to do it in the fix,
> > but we might need to target -next branch. Now the tx_descs is limited
> > by the first tx ring.
> >
> > [1]:
> > "...since tx_descs is not
> > reallocated if it already exists, the array size is permanently frozen
> > to the first socket's size.
> >
> > Should the array be resized to accommodate the largest shared ring, or
> > should the batch readers be updated to cap their reads to the array's
> > actual size?"
>
> My take here is that this set will not break the user space by providing
> at least pool->xdp_zc_max_segs at pool->tx_descs. Let us rely at least a
> bit on user's intelligence here - if you would attach one short ring for
> whatever reason among with bigger ring to the very same umem and whine
> your performance suck, then I'd say it's on you :P
>
> Hope this makes sense!
Sure, unless someone else is screaming at me :)
Thanks,
Jason
>
> >
> > Thanks,
> > Jason
> >
> > > ---
> > > 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 43791647cf18..385a3f4a1b32 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
* Re: [PATCH rdma-next v3] RDMA/mlx5: quiesce CQ polling before device shutdown on reboot
From: Leon Romanovsky @ 2026-07-16 10:50 UTC (permalink / raw)
To: Chenguang Zhao
Cc: jgg, andrew+netdev, davem, edumazet, kuba, pabeni, linux-rdma,
netdev, tariqt, mbloch, dtatulea, shayd, moshe, Chenguang Zhao
In-Reply-To: <c2c5dda9-3228-44a3-a4c9-89e8cc4501bc@linux.dev>
On Thu, Jul 16, 2026 at 05:03:37PM +0800, Chenguang Zhao wrote:
> Hi, Leon
> reboot -f skips orderly shutdown and goes directly to:
>
> kernel_restart_prepare() -> device_shutdown() -> mlx5 shutdown
> Upper layers may still hold live CQs, while ib-comp-wq keeps
> polling — a use-after-free race.
The point is that this flow is neither RDMA- nor mlx5-specific, and it
works as expected. mlx5 shutdown() stops the FW/HW, while the kernel
stops and tears down the running threads.
>
> Normal reboot usually works because userspace has already
> torn down RDMA and called ib_free_cq().
There is no difference, from the kernel's perspective, between a
normal and a forced reboot, except that the former bypasses
userspace shutdown.
Thanks
>
> Thanks
>
> 在 2026/7/16 16:42, Leon Romanovsky 写道:
> > On Wed, Jul 15, 2026 at 04:23:07PM +0800, Chenguang Zhao wrote:
> >> From: Chenguang Zhao <zhaochenguang@kylinos.cn>
> >>
> >> On reboot -f with NFS over RDMA, mlx5 shutdown can tear the device
> >> down while ib-comp-wq still polls live CQs, leading to UAF in
> >> wr_cqe->done().
> >>
> >> Mark the device shutting down before teardown, flush completion
> >> workqueues so in-flight pollers observe the flag, skip SYS_ERROR
> >> completion delivery, and make poll/arm CQ a no-op under the CQ lock
> >> while shutting down.
> >>
> >> Signed-off-by: Chenguang Zhao <zhaochenguang@kylinos.cn>
> >> ---
> >> changelog:
> >> - Fix the race on MLX5_INTERFACE_STATE_SHUTTING_DOWN: set the
> >> flag, then flush ib-comp / mlx5_ib event workqueues via an
> >> mlx5_ib quiesce hook before fast_unload/teardown.
> >> - Check shutting-down under cq->lock in mlx5_ib_poll_cq/arm_cq.
> >> - Export ib_comp_wq and ib_comp_unbound_wq so modular mlx5_ib
> >> can flush them.
> >>
> >> v2:
> >> https://lore.kernel.org/all/20260714075558.1420384-1-chenguang.zhao@linux.dev/
> >>
> >> v1:
> >> https://lore.kernel.org/all/20260702073422.279820-1-chenguang.zhao@linux.dev/
> >>
> >> drivers/infiniband/core/device.c | 2 ++
> >> drivers/infiniband/hw/mlx5/cq.c | 11 ++++++++++
> >> drivers/infiniband/hw/mlx5/main.c | 20 +++++++++++++++++++
> >> .../net/ethernet/mellanox/mlx5/core/health.c | 3 +++
> >> .../net/ethernet/mellanox/mlx5/core/main.c | 10 ++++++++++
> >> .../mellanox/mlx5/core/sf/dev/driver.c | 3 +++
> >> include/linux/mlx5/driver.h | 11 ++++++++++
> >> 7 files changed, 60 insertions(+)
> > <...>
> >
> >> +static void mlx5_ib_shutdown_quiesce(void)
> >> +{
> >> + flush_workqueue(ib_comp_wq);
> >> + flush_workqueue(ib_comp_unbound_wq);
> >> + flush_workqueue(mlx5_ib_event_wq);
> >> +}
> > These workqueues are shared by all IB drivers and the core. Drivers
> > must not flush or destroy them.
> >
> > Why this is not FW issue?
> >
> > Thanks
^ 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