* [PATCH net-next 02/17] net: dsa: mv88e6xxx: Introduce Marvell dsa tagger data operation.
From: Luke Howard @ 2026-07-03 7:46 UTC (permalink / raw)
To: Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Russell King,
Richard Cochran, Florian Fainelli
Cc: Cedric Jehasse, Max Holtmann, Max Hunter, Kieran Tyrrell,
Ryan Wilkins, Mattias Forsblad, netdev, linux-kernel, Luke Howard
In-Reply-To: <20260703-net-next-mv88e6xxx-rmu-v1-0-991a27d78bca@padl.com>
From: Mattias Forsblad <mattias.forsblad@gmail.com>
Support connecting Marvell dsa tagger to the switch driver frame2reg
function to decode received RMU frames.
(Note: during teardown or failed setup, tagger_data may be NULL when
a late FRAME2REG frame arrives.)
Signed-off-by: Luke Howard <lukeh@padl.com>
Signed-off-by: Mattias Forsblad <mattias.forsblad@gmail.com>
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
include/linux/dsa/mv88e6xxx.h | 8 ++++++++
net/dsa/tag_dsa.c | 43 ++++++++++++++++++++++++++++++++++++++-----
2 files changed, 46 insertions(+), 5 deletions(-)
diff --git a/include/linux/dsa/mv88e6xxx.h b/include/linux/dsa/mv88e6xxx.h
index 8c3d45eca46bd..1e9460d6f7782 100644
--- a/include/linux/dsa/mv88e6xxx.h
+++ b/include/linux/dsa/mv88e6xxx.h
@@ -6,6 +6,14 @@
#define _NET_DSA_TAG_MV88E6XXX_H
#include <linux/if_vlan.h>
+#include <net/dsa.h>
+
+struct dsa_tagger_data {
+ /* DSA frame decoded to be from the RMU */
+ void (*rmu_frame2reg)(struct dsa_switch *ds,
+ struct sk_buff *skb,
+ u8 seqno);
+};
#define MV88E6XXX_VID_STANDALONE 0
#define MV88E6XXX_VID_BRIDGED (VLAN_N_VID - 1)
diff --git a/net/dsa/tag_dsa.c b/net/dsa/tag_dsa.c
index d5ffee35fbb53..5a95873e87340 100644
--- a/net/dsa/tag_dsa.c
+++ b/net/dsa/tag_dsa.c
@@ -201,14 +201,17 @@ static struct sk_buff *dsa_xmit_ll(struct sk_buff *skb, struct net_device *dev,
static struct sk_buff *dsa_rcv_ll(struct sk_buff *skb, struct net_device *dev,
u8 extra)
{
+ struct dsa_tagger_data *tagger_data;
bool trap = false, trunk = false;
int source_device, source_port;
+ struct dsa_switch *ds;
enum dsa_code code;
enum dsa_cmd cmd;
u8 *dsa_header;
/* The ethertype field is part of the DSA header. */
dsa_header = dsa_etype_header_pos_rx(skb);
+ source_device = dsa_header[0] & 0x1f;
cmd = dsa_header[0] >> 6;
switch (cmd) {
@@ -220,12 +223,20 @@ static struct sk_buff *dsa_rcv_ll(struct sk_buff *skb, struct net_device *dev,
code = (dsa_header[1] & 0x6) | ((dsa_header[2] >> 4) & 1);
switch (code) {
- case DSA_CODE_FRAME2REG:
- /* Remote management is not implemented yet,
- * drop.
- */
+ case DSA_CODE_FRAME2REG: {
+ u8 seqno = dsa_header[3];
+
+ ds = dsa_conduit_find_switch(dev, source_device);
+ if (!ds) {
+ kfree_skb(skb);
+ return NULL;
+ }
+ tagger_data = ds->tagger_data;
+ if (likely(tagger_data && tagger_data->rmu_frame2reg))
+ tagger_data->rmu_frame2reg(ds, skb, seqno);
kfree_skb(skb);
return NULL;
+ }
case DSA_CODE_ARP_MIRROR:
case DSA_CODE_POLICY_MIRROR:
/* Mark mirrored packets to notify any upper
@@ -256,7 +267,6 @@ static struct sk_buff *dsa_rcv_ll(struct sk_buff *skb, struct net_device *dev,
return NULL;
}
- source_device = dsa_header[0] & 0x1f;
source_port = (dsa_header[1] >> 3) & 0x1f;
if (trunk) {
@@ -331,6 +341,25 @@ static struct sk_buff *dsa_rcv_ll(struct sk_buff *skb, struct net_device *dev,
return skb;
}
+static int dsa_tag_connect(struct dsa_switch *ds)
+{
+ struct dsa_tagger_data *tagger_data;
+
+ tagger_data = kzalloc_obj(*tagger_data, GFP_KERNEL);
+ if (!tagger_data)
+ return -ENOMEM;
+
+ ds->tagger_data = tagger_data;
+
+ return 0;
+}
+
+static void dsa_tag_disconnect(struct dsa_switch *ds)
+{
+ kfree(ds->tagger_data);
+ ds->tagger_data = NULL;
+}
+
#if IS_ENABLED(CONFIG_NET_DSA_TAG_DSA)
static struct sk_buff *dsa_xmit(struct sk_buff *skb, struct net_device *dev)
@@ -353,6 +382,8 @@ static const struct dsa_device_ops dsa_netdev_ops = {
.proto = DSA_TAG_PROTO_DSA,
.xmit = dsa_xmit,
.rcv = dsa_rcv,
+ .connect = dsa_tag_connect,
+ .disconnect = dsa_tag_disconnect,
.needed_headroom = DSA_HLEN,
};
@@ -397,6 +428,8 @@ static const struct dsa_device_ops edsa_netdev_ops = {
.proto = DSA_TAG_PROTO_EDSA,
.xmit = edsa_xmit,
.rcv = edsa_rcv,
+ .connect = dsa_tag_connect,
+ .disconnect = dsa_tag_disconnect,
.needed_headroom = EDSA_HLEN,
};
--
2.43.0
^ permalink raw reply related
* [PATCH net-next 01/17] net: dsa: mv88e6xxx: use lockdep_assert_held() if CONFIG_LOCKDEP
From: Luke Howard @ 2026-07-03 7:46 UTC (permalink / raw)
To: Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Russell King,
Richard Cochran, Florian Fainelli
Cc: Cedric Jehasse, Max Holtmann, Max Hunter, Kieran Tyrrell,
Ryan Wilkins, Mattias Forsblad, netdev, linux-kernel, Luke Howard
In-Reply-To: <20260703-net-next-mv88e6xxx-rmu-v1-0-991a27d78bca@padl.com>
If CONFIG_LOCKDEP is enabled, use lockdep_assert_held() rather than an
explicit check for the chip's register mutex.
Signed-off-by: Luke Howard <lukeh@padl.com>
---
drivers/net/dsa/mv88e6xxx/chip.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 80b877c74513d..eada2f71cb7b3 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -48,10 +48,14 @@
static void assert_reg_lock(struct mv88e6xxx_chip *chip)
{
+#ifdef CONFIG_LOCKDEP
+ lockdep_assert_held(&chip->reg_lock);
+#else
if (unlikely(!mutex_is_locked(&chip->reg_lock))) {
dev_err(chip->dev, "Switch registers lock not held!\n");
dump_stack();
}
+#endif
}
int mv88e6xxx_read(struct mv88e6xxx_chip *chip, int addr, int reg, u16 *val)
--
2.43.0
^ permalink raw reply related
* [PATCH net-next 00/17] net: dsa: mv88e6xxx: support for Remote Management Unit
From: Luke Howard @ 2026-07-03 7:46 UTC (permalink / raw)
To: Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Russell King,
Richard Cochran, Florian Fainelli
Cc: Cedric Jehasse, Max Holtmann, Max Hunter, Kieran Tyrrell,
Ryan Wilkins, Mattias Forsblad, netdev, linux-kernel, Luke Howard
This patch set shephards a series of commits made by Andrew Lunn
and others to support remotely managing Marvell switches over
Ethernet (as opposed to MDIO) using the Remote Management Unit
(RMU).
This is the second of four patch sets: the remaining two add
support for remote management when MDIO is unavailable.
Signed-off-by: Luke Howard <lukeh@padl.com>
---
Andrew Lunn (11):
net: dsa: tag_dsa: Add helper to add DSA header to RMU frame
net: dsa: mv88e6xxx: Add conduit state change handler
net: dsa: mv88e6xxx: Remove mv88e6xxx_rmu_setup()
net: dsa: mv88e6xxx: Add connect_tag_protocol handler
net: dsa: mv88e6xxx: Add sanity check of received RMU frame
net: dsa: mv88e6xxx: Get Product ID when enabling RMU
net: dsa: mv88e6xxx: Add RMU register read/write/wait for bit
net: dsa: mv88e6xxx: Move available stats into info structure
net: dsa: mv88e6xxx: Centralise common statistics check
net: dsa: mv88e6xxx: Get some MIB stats via the RMU
net: dsa: mv88e6xxx: Time RMU operations and disable if slow
Luke Howard (4):
net: dsa: mv88e6xxx: use lockdep_assert_held() if CONFIG_LOCKDEP
net: dsa: mv88e6xxx: cancel PTP polling work on .shutdown
net: dsa: mv88e6xxx: size ATU snapshot buffer and region by num_macs
net: dsa: mv88e6xxx: RMU DUMP_ATU support in devlink
Mattias Forsblad (2):
net: dsa: mv88e6xxx: Introduce Marvell dsa tagger data operation.
net: dsa: mv88e6xxx: Add RMU enable/disable for select switches.
drivers/net/dsa/mv88e6xxx/Makefile | 1 +
drivers/net/dsa/mv88e6xxx/chip.c | 92 ++++--
drivers/net/dsa/mv88e6xxx/chip.h | 35 +++
drivers/net/dsa/mv88e6xxx/devlink.c | 71 +++--
drivers/net/dsa/mv88e6xxx/global1.c | 64 ++++
drivers/net/dsa/mv88e6xxx/global1.h | 3 +
drivers/net/dsa/mv88e6xxx/global2.c | 1 +
drivers/net/dsa/mv88e6xxx/ptp.c | 14 +-
drivers/net/dsa/mv88e6xxx/ptp.h | 5 +
drivers/net/dsa/mv88e6xxx/rmu.c | 611 ++++++++++++++++++++++++++++++++++++
drivers/net/dsa/mv88e6xxx/rmu.h | 145 +++++++++
include/linux/dsa/mv88e6xxx.h | 11 +
net/dsa/tag_dsa.c | 67 +++-
13 files changed, 1073 insertions(+), 47 deletions(-)
---
base-commit: 165539241e210db8e962479c3c5072372576b618
change-id: 20260613-net-next-mv88e6xxx-rmu-2db17825ece0
Best regards,
--
Luke Howard <lukeh@padl.com>
^ permalink raw reply
* [PATCH net v2 2/2] net: fman: add error cleanup path in fman_probe
From: ZhaoJinming @ 2026-07-03 7:43 UTC (permalink / raw)
To: madalin.bucur, sean.anderson
Cc: netdev, andrew+netdev, davem, edumazet, kuba, pabeni,
igal.liberman, linux-kernel, ZhaoJinming
In-Reply-To: <20260703074324.907294-1-zhaojinming@uniontech.com>
fman_init() and devm_request_irq() failure paths in fman_probe()
do not free fman and its sub-resources (keygen, muram allocations,
state, cfg), causing memory leaks on probe failure.
Add fman_muram_finish() to properly tear down a MURAM partition
(gen_pool_destroy + iounmap + kfree), complementing the existing
fman_muram_init().
Add fman_free_resources() that releases all fman sub-resources
in the correct order:
- devm_free_irq() for any already-registered IRQ handlers
- kfree(fman->keygen)
- free_init_resources() for MURAM CAM/FIFO allocations
- kfree(fman->cfg)
- fman_muram_finish(fman->muram) for the MURAM management object
- kfree(fman->state)
- kfree(fman)
Use two goto labels in fman_probe():
- err_irq: main IRQ registered but err_irq failed -- free main IRQ
then fall through to release resources
- err_no_irq: no IRQ registered -- just release resources
The IRQ handlers must be explicitly freed before kfree(fman) to
avoid a window where a shared-IRQ spurious firing could dereference
the freed dev_id.
Note: fman_config() is not changed -- it already frees fman
internally on all its error paths, so fman_probe() must not touch
fman after fman_config() fails.
v2:
- add explicit devm_free_irq() before kfree(fman) to eliminate
a potential UAF window on the cleanup path
- add fman_muram_finish() for complete MURAM teardown
- add kfree(fman->cfg) to release config structure
Fixes: 414fd46e7762 ("fsl/fman: Add FMan support")
Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
---
drivers/net/ethernet/freescale/fman/fman.c | 31 +++++++++++++++++--
.../net/ethernet/freescale/fman/fman_muram.c | 15 +++++++++
.../net/ethernet/freescale/fman/fman_muram.h | 2 ++
3 files changed, 45 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fman/fman.c b/drivers/net/ethernet/freescale/fman/fman.c
index 6947f3bc7c87..752c0df0e17c 100644
--- a/drivers/net/ethernet/freescale/fman/fman.c
+++ b/drivers/net/ethernet/freescale/fman/fman.c
@@ -2806,6 +2806,24 @@ static struct fman *read_dts_node(struct platform_device *of_dev)
return ERR_PTR(err);
}
+static void fman_free_resources(struct fman *fman, struct device *dev,
+ bool irq_registered)
+{
+ /* Free IRQs first while fman is still valid */
+ if (irq_registered) {
+ if (fman->dts_params.err_irq != 0)
+ devm_free_irq(dev, fman->dts_params.err_irq, fman);
+ devm_free_irq(dev, fman->dts_params.irq, fman);
+ }
+
+ kfree(fman->keygen);
+ free_init_resources(fman);
+ kfree(fman->cfg);
+ fman_muram_finish(fman->muram);
+ kfree(fman->state);
+ kfree(fman);
+}
+
static int fman_probe(struct platform_device *of_dev)
{
struct fman *fman;
@@ -2821,12 +2839,13 @@ static int fman_probe(struct platform_device *of_dev)
err = fman_config(fman);
if (err) {
dev_err(dev, "%s: FMan config failed\n", __func__);
+ /* fman_config() frees fman internally on all error paths */
return -EINVAL;
}
if (fman_init(fman) != 0) {
dev_err(dev, "%s: FMan init failed\n", __func__);
- return -EINVAL;
+ goto err_no_irq;
}
/* Register IRQ handlers only after initialization is complete.
@@ -2844,7 +2863,7 @@ static int fman_probe(struct platform_device *of_dev)
if (err < 0) {
dev_err(dev, "%s: irq %d allocation failed (error = %d)\n",
__func__, fman->dts_params.irq, err);
- return err;
+ goto err_no_irq;
}
if (fman->dts_params.err_irq != 0) {
@@ -2854,7 +2873,7 @@ static int fman_probe(struct platform_device *of_dev)
if (err < 0) {
dev_err(dev, "%s: irq %d allocation failed (error = %d)\n",
__func__, fman->dts_params.err_irq, err);
- return err;
+ goto err_irq;
}
}
@@ -2883,6 +2902,12 @@ static int fman_probe(struct platform_device *of_dev)
dev_dbg(dev, "FMan%d probed\n", fman->dts_params.id);
return 0;
+
+err_irq:
+ devm_free_irq(dev, fman->dts_params.irq, fman);
+err_no_irq:
+ fman_free_resources(fman, dev, false);
+ return err ?: -EINVAL;
}
static const struct of_device_id fman_match[] = {
diff --git a/drivers/net/ethernet/freescale/fman/fman_muram.c b/drivers/net/ethernet/freescale/fman/fman_muram.c
index 6ac7c2b0cb19..6c2b4f7a02b8 100644
--- a/drivers/net/ethernet/freescale/fman/fman_muram.c
+++ b/drivers/net/ethernet/freescale/fman/fman_muram.c
@@ -129,3 +129,18 @@ void fman_muram_free_mem(struct muram_info *muram, unsigned long offset,
gen_pool_free(muram->pool, addr, size);
}
+
+/**
+ * fman_muram_finish
+ * @muram: FM-MURAM module pointer.
+ *
+ * Frees all resources associated with a MURAM partition.
+ */
+void fman_muram_finish(struct muram_info *muram)
+{
+ if (!muram)
+ return;
+ iounmap(muram->vbase);
+ gen_pool_destroy(muram->pool);
+ kfree(muram);
+}
diff --git a/drivers/net/ethernet/freescale/fman/fman_muram.h b/drivers/net/ethernet/freescale/fman/fman_muram.h
index 3643af61bae2..a5cb544c0f08 100644
--- a/drivers/net/ethernet/freescale/fman/fman_muram.h
+++ b/drivers/net/ethernet/freescale/fman/fman_muram.h
@@ -23,4 +23,6 @@ unsigned long fman_muram_alloc(struct muram_info *muram, size_t size);
void fman_muram_free_mem(struct muram_info *muram, unsigned long offset,
size_t size);
+void fman_muram_finish(struct muram_info *muram);
+
#endif /* __FM_MURAM_EXT */
--
2.20.1
^ permalink raw reply related
* [PATCH net v2 1/2] net: fman: move IRQ registration after init to prevent NULL deref and UAF
From: ZhaoJinming @ 2026-07-03 7:43 UTC (permalink / raw)
To: madalin.bucur, sean.anderson
Cc: netdev, andrew+netdev, davem, edumazet, kuba, pabeni,
igal.liberman, linux-kernel, ZhaoJinming
read_dts_node() registers shared interrupt handlers via
devm_request_irq() with fman as dev_id. Two bugs exist in the
current code:
1) Pre-init NULL dereference: at registration time fman is only
partially initialized -- kzalloc_obj() zero-initializes all fields,
so fman->cfg and fman->fpm_regs are NULL. The handlers check
is_init_done(fman->cfg) to guard against incomplete init, but
is_init_done(NULL) returns true (intended to mean cfg was freed
after successful init), so the guard is bypassed and fpm_regs is
dereferenced. If another device on the same shared IRQ line fires
during the window between devm_request_irq() and fman_init(), the
handler accesses NULL fpm_regs via ioread32be(), causing a crash.
2) Use-after-free on probe failure: fman is allocated with
kzalloc_obj() (not devm), so on error paths in read_dts_node()
(ioremap failure, of_platform_populate failure) and fman_config(),
kfree(fman) is called while the devm IRQ handlers remain
registered. The driver core's subsequent devres_release_all() frees
the IRQ handlers, but during the window between kfree(fman) and
devm_free_irq(), a shared-IRQ spurious firing will dereference
the already-freed fman.
A previous attempt to fix issue #1 with an irq_ready flag protected
by READ_ONCE()/WRITE_ONCE() is insufficient on weakly-ordered
architectures. READ_ONCE()/WRITE_ONCE() only prevent compiler
optimization; they do not provide the memory ordering guarantees
(e.g., smp_store_release/smp_load_acquire) needed to ensure that
writes to register pointers are visible to the IRQ handler before
it observes the flag as true.
Fix both issues by moving devm_request_irq() out of read_dts_node()
and into fman_probe(), after both fman_config() and fman_init()
have completed. This eliminates both race windows: by the time the
handlers are registered, all register pointers are initialized
(preventing the NULL dereference), and since fman is never freed
after this point, the use-after-free cannot occur either.
Add an 'irq' field to struct fman_dts_params so that the primary IRQ
number parsed in read_dts_node() is available to fman_probe().
This replaces the previous approach (irq_ready flag with READ_ONCE)
and also supersedes the separate UAF fix patch ("fsl_fman: fix
use-after-free on IRQF_SHARED handler after probe failure" v3),
as moving IRQ registration after init resolves both issues
in a single change.
v2:
- move devm_request_irq() to fman_probe() after init (replaces
irq_ready + READ_ONCE approach from v1)
- supersede the separate UAF fix patch (v3), as this patch
resolves both issues in a single change
Fixes: 414fd46e7762 ("fsl/fman: Add FMan support")
Link: https://lore.kernel.org/netdev/20260626162323.GE1310988@horms.kernel.org/
Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
---
drivers/net/ethernet/freescale/fman/fman.c | 52 +++++++++++++---------
drivers/net/ethernet/freescale/fman/fman.h | 1 +
2 files changed, 32 insertions(+), 21 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fman/fman.c b/drivers/net/ethernet/freescale/fman/fman.c
index 299bab043175..6947f3bc7c87 100644
--- a/drivers/net/ethernet/freescale/fman/fman.c
+++ b/drivers/net/ethernet/freescale/fman/fman.c
@@ -2695,7 +2695,7 @@ static struct fman *read_dts_node(struct platform_device *of_dev)
void __iomem *base_addr;
struct resource *res;
u32 val, range[2];
- int err, irq;
+ int err;
struct clk *clk;
u32 clk_rate;
@@ -2717,7 +2717,7 @@ static struct fman *read_dts_node(struct platform_device *of_dev)
err = platform_get_irq(of_dev, 0);
if (err < 0)
goto fman_node_put;
- irq = err;
+ fman->dts_params.irq = err;
/* Get the FM error interrupt */
err = platform_get_irq(of_dev, 1);
@@ -2773,25 +2773,6 @@ static struct fman *read_dts_node(struct platform_device *of_dev)
of_node_put(muram_node);
- err = devm_request_irq(&of_dev->dev, irq, fman_irq, IRQF_SHARED,
- "fman", fman);
- if (err < 0) {
- dev_err(&of_dev->dev, "%s: irq %d allocation failed (error = %d)\n",
- __func__, irq, err);
- goto fman_free;
- }
-
- if (fman->dts_params.err_irq != 0) {
- err = devm_request_irq(&of_dev->dev, fman->dts_params.err_irq,
- fman_err_irq, IRQF_SHARED,
- "fman-err", fman);
- if (err < 0) {
- dev_err(&of_dev->dev, "%s: irq %d allocation failed (error = %d)\n",
- __func__, fman->dts_params.err_irq, err);
- goto fman_free;
- }
- }
-
base_addr = devm_platform_get_and_ioremap_resource(of_dev, 0, &res);
if (IS_ERR(base_addr)) {
err = PTR_ERR(base_addr);
@@ -2848,6 +2829,35 @@ static int fman_probe(struct platform_device *of_dev)
return -EINVAL;
}
+ /* Register IRQ handlers only after initialization is complete.
+ * This prevents two issues:
+ * 1) Pre-init NULL dereference: is_init_done(NULL) returns true,
+ * so a shared-IRQ spurious firing before fpm_regs is set would
+ * dereference NULL.
+ * 2) Use-after-free on probe failure: fman was kzalloc'd (not devm),
+ * so on error paths kfree(fman) ran before devm_free_irq, leaving
+ * a window where the handler could fire with a freed dev_id.
+ * By registering here, both problems are eliminated.
+ */
+ err = devm_request_irq(dev, fman->dts_params.irq, fman_irq,
+ IRQF_SHARED, "fman", fman);
+ if (err < 0) {
+ dev_err(dev, "%s: irq %d allocation failed (error = %d)\n",
+ __func__, fman->dts_params.irq, err);
+ return err;
+ }
+
+ if (fman->dts_params.err_irq != 0) {
+ err = devm_request_irq(dev, fman->dts_params.err_irq,
+ fman_err_irq, IRQF_SHARED,
+ "fman-err", fman);
+ if (err < 0) {
+ dev_err(dev, "%s: irq %d allocation failed (error = %d)\n",
+ __func__, fman->dts_params.err_irq, err);
+ return err;
+ }
+ }
+
if (fman->dts_params.err_irq == 0) {
fman_set_exception(fman, FMAN_EX_DMA_BUS_ERROR, false);
fman_set_exception(fman, FMAN_EX_DMA_READ_ECC, false);
diff --git a/drivers/net/ethernet/freescale/fman/fman.h b/drivers/net/ethernet/freescale/fman/fman.h
index 74eb62eba0d7..630d57c3144c 100644
--- a/drivers/net/ethernet/freescale/fman/fman.h
+++ b/drivers/net/ethernet/freescale/fman/fman.h
@@ -286,6 +286,7 @@ struct fman_dts_params {
struct resource *res; /* FMan memory resource */
u8 id; /* FMan ID */
+ int irq; /* FMan IRQ */
int err_irq; /* FMan Error IRQ */
u16 clk_freq; /* FMan clock freq (In Mhz) */
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net-next v9 1/4] net: phy: c45: add genphy_c45_pma_soft_reset()
From: Maxime Chevallier @ 2026-07-03 7:43 UTC (permalink / raw)
To: javen, andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni,
freddy_gu, nb
Cc: netdev, linux-kernel, daniel, vladimir.oltean
In-Reply-To: <20260703071330.1707-2-javen_xu@realsil.com.cn>
Hi Javen,
On 7/3/26 09:13, javen wrote:
> From: Javen Xu <javen_xu@realsil.com.cn>
>
> Add a generic Clause 45 software reset helper. The helper sets the reset
> bit in the PMA/PMD control register and waits until the bit is cleared by
> hardware.
>
> Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
> Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Thanks,
Maxime
^ permalink raw reply
* [RFC PATCH 3/3] coredump, net: remove `SOCK_COREDUMP`
From: John Ericson @ 2026-07-03 7:39 UTC (permalink / raw)
To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: John Ericson, Cong Wang, Kuniyuki Iwashima, Simon Horman,
Christian Brauner, David Rheinsberg, Andy Lutomirski,
Sergei Zimmerman, netdev, linux-fsdevel, Mickaël Salaün,
Günther Noack, Paul Moore, linux-security-module,
linux-kernel
In-Reply-To: <20260703073948.2541875-1-John.Ericson@Obsidian.Systems>
From: John Ericson <mail@JohnEricson.me>
The utility of the refactors of the last two commits is demonstrated by
fixing this glaring layer violation: the unix socket implementation
knowing about the coredump socket caller.
Before, when the only way to connect to a socket was via the UAPI
`struct sockaddr_un`, the only way to implement the proper logic that
the kernel needs to resolve the coredump socket path was via hacking it
into the socket implementation.
In addition to being quite ugly, this layer violation is not great for
security. The intent is that `SOCK_COREDUMP` can only be used by the
kernel, and to be clear, I have no reason to believe this is not
correctly enforced today. But because of the many functions with flags
arguments, this is not a locally-enforced invariant. Some change, at
some point, could mess up, and allow user-provided flags to sneak in,
and this strikes me as a mistake waiting to happen. At that point, a
user could exploit this to connect to any socket it likes, bypassing
permission checks.
Now, with the two functions we've just previously factored out, we can
fix the layering. The custom path lookup logic lives with the coredump
caller, where it belongs. Once the right `struct path` is found
(actually just the inode is needed), the coredump caller can resolve a
`struct sock *` from it, and then directly connect to it. With this
change, `SOCK_COREDUMP` is no longer needed at all, and can be deleted.
The layer violation is gone, and the security footgun is gone with it.
As an added bonus, remove `flags` parameters from a number of internal
functions that no longer need them. They were just taking flags
parameters for the sake of `SOCK_COREDUMP`, and so with that gone, those
flag parameters are also no longer needed. If or when there is a new
flag that motivates them, they can be added back.
Tested that `coredump_socket_protocol_test` still passes.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: John Ericson <mail@JohnEricson.me>
---
fs/coredump.c | 47 ++++++++++++++++++++++++-----------
include/linux/lsm_hook_defs.h | 3 +--
include/linux/net.h | 1 -
include/linux/security.h | 4 +--
net/unix/af_unix.c | 42 ++++++++++---------------------
security/landlock/fs.c | 7 +-----
security/security.c | 5 ++--
7 files changed, 51 insertions(+), 58 deletions(-)
diff --git a/fs/coredump.c b/fs/coredump.c
index e68a76ff92a3..e1452021218e 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -14,6 +14,7 @@
#include <linux/perf_event.h>
#include <linux/highmem.h>
#include <linux/spinlock.h>
+#include <linux/cred.h>
#include <linux/key.h>
#include <linux/personality.h>
#include <linux/binfmts.h>
@@ -21,6 +22,7 @@
#include <linux/sort.h>
#include <linux/sched/coredump.h>
#include <linux/sched/signal.h>
+#include <linux/sched/task.h>
#include <linux/sched/task_stack.h>
#include <linux/utsname.h>
#include <linux/pid_namespace.h>
@@ -50,7 +52,6 @@
#include <net/net_namespace.h>
#include <net/sock.h>
#include <uapi/linux/pidfd.h>
-#include <uapi/linux/un.h>
#include <uapi/linux/coredump.h>
#include <linux/uaccess.h>
@@ -668,17 +669,10 @@ static int umh_coredump_setup(struct subprocess_info *info, struct cred *new)
static bool coredump_sock_connect(struct core_name *cn, struct coredump_params *cprm)
{
struct file *file __free(fput) = NULL;
- struct sockaddr_un addr = {
- .sun_family = AF_UNIX,
- };
- ssize_t addr_len;
- int retval;
+ struct path root, path;
struct socket *socket;
-
- addr_len = strscpy(addr.sun_path, cn->corename);
- if (addr_len < 0)
- return false;
- addr_len += offsetof(struct sockaddr_un, sun_path) + 1;
+ struct sock *sk;
+ int retval;
/*
* It is possible that the userspace process which is supposed
@@ -710,14 +704,37 @@ static bool coredump_sock_connect(struct core_name *cn, struct coredump_params *
*/
pidfs_coredump(cprm);
- retval = kernel_connect(socket, (struct sockaddr_unsized *)(&addr), addr_len,
- O_NONBLOCK | SOCK_COREDUMP);
+ /*
+ * Resolve the socket path relative to init's root and with kernel
+ * credentials, and with symlinks, magic links and escaping the
+ * root all forbidden, so the dumping process cannot use its own
+ * filesystem view to redirect its core to an arbitrary socket.
+ */
+ task_lock(&init_task);
+ get_fs_root(init_task.fs, &root);
+ task_unlock(&init_task);
+
+ scoped_with_kernel_creds()
+ retval = vfs_path_lookup(root.dentry, root.mnt, cn->corename,
+ LOOKUP_BENEATH | LOOKUP_NO_SYMLINKS |
+ LOOKUP_NO_MAGICLINKS, &path);
+ path_put(&root);
+ if (retval)
+ return false;
+
+ /* Connect directly to the socket bound there, by fd not by name. */
+ sk = unix_lookup_bsd_path(&path, SOCK_STREAM);
+ path_put(&path);
+ if (IS_ERR(sk))
+ return false;
+ retval = kernel_unix_connect_direct(sk, socket, O_NONBLOCK);
+ sock_put(sk);
if (retval) {
if (retval == -EAGAIN)
- coredump_report_failure("Coredump socket %s receive queue full", addr.sun_path);
+ coredump_report_failure("Coredump socket %s receive queue full", cn->corename);
else
- coredump_report_failure("Coredump socket connection %s failed %d", addr.sun_path, retval);
+ coredump_report_failure("Coredump socket connection %s failed %d", cn->corename, retval);
return false;
}
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 65c9609ec207..3d6fbb6d2628 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -323,8 +323,7 @@ LSM_HOOK(int, 0, watch_key, struct key *key)
#endif /* CONFIG_SECURITY && CONFIG_KEY_NOTIFICATIONS */
#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
-LSM_HOOK(int, 0, unix_find, const struct path *path, struct sock *other,
- int flags)
+LSM_HOOK(int, 0, unix_find, const struct path *path, struct sock *other)
#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
#ifdef CONFIG_SECURITY_NETWORK
diff --git a/include/linux/net.h b/include/linux/net.h
index f268f395ce47..285cb67927f0 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -102,7 +102,6 @@ enum sock_type {
#ifndef SOCK_NONBLOCK
#define SOCK_NONBLOCK O_NONBLOCK
#endif
-#define SOCK_COREDUMP O_NOCTTY
/**
* enum sock_shutdown_cmd - Shutdown types
diff --git a/include/linux/security.h b/include/linux/security.h
index 153e9043058f..9797bd29c916 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1957,10 +1957,10 @@ static inline int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
-int security_unix_find(const struct path *path, struct sock *other, int flags);
+int security_unix_find(const struct path *path, struct sock *other);
#else /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
-static inline int security_unix_find(const struct path *path, struct sock *other, int flags)
+static inline int security_unix_find(const struct path *path, struct sock *other)
{
return 0;
}
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index aa94da1f8c24..7cb537b404cc 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1220,7 +1220,7 @@ struct sock *unix_lookup_bsd_path(const struct path *path, int type)
EXPORT_SYMBOL_GPL(unix_lookup_bsd_path);
static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
- int type, int flags)
+ int type)
{
struct path path;
struct sock *sk;
@@ -1228,29 +1228,13 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
unix_mkname_bsd(sunaddr, addr_len);
- if (flags & SOCK_COREDUMP) {
- struct path root;
-
- task_lock(&init_task);
- get_fs_root(init_task.fs, &root);
- task_unlock(&init_task);
-
- scoped_with_kernel_creds()
- err = vfs_path_lookup(root.dentry, root.mnt, sunaddr->sun_path,
- LOOKUP_BENEATH | LOOKUP_NO_SYMLINKS |
- LOOKUP_NO_MAGICLINKS, &path);
- path_put(&root);
- if (err)
- goto fail;
- } else {
- err = kern_path(sunaddr->sun_path, LOOKUP_FOLLOW, &path);
- if (err)
- goto fail;
+ err = kern_path(sunaddr->sun_path, LOOKUP_FOLLOW, &path);
+ if (err)
+ goto fail;
- err = path_permission(&path, MAY_WRITE);
- if (err)
- goto path_put;
- }
+ err = path_permission(&path, MAY_WRITE);
+ if (err)
+ goto path_put;
sk = unix_lookup_bsd_path(&path, type);
if (IS_ERR(sk)) {
@@ -1258,7 +1242,7 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
goto path_put;
}
- err = security_unix_find(&path, sk, flags);
+ err = security_unix_find(&path, sk);
if (err)
goto sock_put;
@@ -1297,12 +1281,12 @@ static struct sock *unix_find_abstract(struct net *net,
static struct sock *unix_find_other(struct net *net,
struct sockaddr_un *sunaddr,
- int addr_len, int type, int flags)
+ int addr_len, int type)
{
struct sock *sk;
if (sunaddr->sun_path[0])
- sk = unix_find_bsd(sunaddr, addr_len, type, flags);
+ sk = unix_find_bsd(sunaddr, addr_len, type);
else
sk = unix_find_abstract(net, sunaddr, addr_len, type);
@@ -1558,7 +1542,7 @@ static int unix_dgram_connect(struct socket *sock, struct sockaddr_unsized *addr
}
restart:
- other = unix_find_other(sock_net(sk), sunaddr, alen, sock->type, 0);
+ other = unix_find_other(sock_net(sk), sunaddr, alen, sock->type);
if (IS_ERR(other)) {
err = PTR_ERR(other);
goto out;
@@ -1897,7 +1881,7 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr_unsized *uad
* unix_stream_connect_commit() means "retry": the peer had died,
* or its backlog was full and we slept -- so re-resolve the name.
*/
- other = unix_find_other(net, sunaddr, addr_len, sk->sk_type, flags);
+ other = unix_find_other(net, sunaddr, addr_len, sk->sk_type);
if (IS_ERR(other)) {
err = PTR_ERR(other);
goto out_free;
@@ -2330,7 +2314,7 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
if (msg->msg_namelen) {
lookup:
other = unix_find_other(sock_net(sk), msg->msg_name,
- msg->msg_namelen, sk->sk_type, 0);
+ msg->msg_namelen, sk->sk_type);
if (IS_ERR(other)) {
err = PTR_ERR(other);
goto out_free;
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index f7e5e4ef9eac..d77080438c01 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1641,8 +1641,7 @@ static void unmask_scoped_access(const struct landlock_ruleset *const client,
}
}
-static int hook_unix_find(const struct path *const path, struct sock *other,
- int flags)
+static int hook_unix_find(const struct path *const path, struct sock *other)
{
const struct landlock_ruleset *dom_other;
const struct landlock_cred_security *subject;
@@ -1652,10 +1651,6 @@ static int hook_unix_find(const struct path *const path, struct sock *other,
.fs = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
};
- /* Lookup for the purpose of saving coredumps is OK. */
- if (unlikely(flags & SOCK_COREDUMP))
- return 0;
-
subject = landlock_get_applicable_subject(current_cred(),
fs_resolve_unix, NULL);
diff --git a/security/security.c b/security/security.c
index 71aea8fdf014..fabb75c88254 100644
--- a/security/security.c
+++ b/security/security.c
@@ -4839,16 +4839,15 @@ int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
* security_unix_find() - Check if a named AF_UNIX socket can connect
* @path: path of the socket being connected to
* @other: peer sock
- * @flags: flags associated with the socket
*
* This hook is called to check permissions before connecting to a named
* AF_UNIX socket. The caller does not hold any locks on @other.
*
* Return: Returns 0 if permission is granted.
*/
-int security_unix_find(const struct path *path, struct sock *other, int flags)
+int security_unix_find(const struct path *path, struct sock *other)
{
- return call_int_hook(unix_find, path, other, flags);
+ return call_int_hook(unix_find, path, other);
}
EXPORT_SYMBOL(security_unix_find);
--
2.54.0
^ permalink raw reply related
* [RFC PATCH 2/3] af_unix: factor out kernel_unix_connect_direct()
From: John Ericson @ 2026-07-03 7:39 UTC (permalink / raw)
To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: John Ericson, Cong Wang, Kuniyuki Iwashima, Simon Horman,
Christian Brauner, David Rheinsberg, Andy Lutomirski,
Sergei Zimmerman, netdev, linux-fsdevel, Mickaël Salaün,
Günther Noack, Paul Moore, linux-security-module,
linux-kernel
In-Reply-To: <20260703073948.2541875-1-John.Ericson@Obsidian.Systems>
From: John Ericson <mail@JohnEricson.me>
I was hoping this was going to be a simple matter of factoring out the
back half of `unix_stream_connect`. No such luck was had, because
actually instead of `unix_stream_connect` looking up the socket from the
VFS once, it does it repeatedly in the same loop that is used to deal
with full listening queues.
(This behavior is rather surprising to me, because it would allow a
deleted and recreated socket to be picked up on the next loop iteration.
But, I don't want to make any UAPI-visible changes in this patch series,
so I did not consider changing it.)
Seeing that this was going to be more complex, I instead factored out
three helpers (setup, commit, cleanup) on a state struct, so I could
reuse them both in the existing `unix_stream_connect` and also in the
new `kernel_unix_connect_direct`. This allows each caller to implement a
slightly different loop:
- resource management of `struct sock *other`:
- `unix_stream_connect` acquires (and releases) it.
- `kernel_unix_connect_direct` uses the caller-provided one.
- stale `other` behavior:
- `unix_stream_connect` retries, because on the next iteration the
socket may have been replaced by a fresh one.
- `kernel_unix_connect_direct` fails, because no reacquisition means
staleness is permanent.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: John Ericson <mail@JohnEricson.me>
---
include/net/af_unix.h | 1 +
net/unix/af_unix.c | 247 +++++++++++++++++++++++++++++++++---------
2 files changed, 199 insertions(+), 49 deletions(-)
diff --git a/include/net/af_unix.h b/include/net/af_unix.h
index fe4547508af1..7d810321efa3 100644
--- a/include/net/af_unix.h
+++ b/include/net/af_unix.h
@@ -15,6 +15,7 @@
#if IS_ENABLED(CONFIG_UNIX)
struct unix_sock *unix_get_socket(struct file *filp);
struct sock *unix_lookup_bsd_path(const struct path *path, int type);
+int kernel_unix_connect_direct(struct sock *other, struct socket *sock, int flags);
#else
static inline struct unix_sock *unix_get_socket(struct file *filp)
{
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 3270299238c4..aa94da1f8c24 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1649,44 +1649,60 @@ static long unix_wait_for_peer(struct sock *other, long timeo)
return timeo;
}
-static int unix_stream_connect(struct socket *sock, struct sockaddr_unsized *uaddr,
- int addr_len, int flags)
-{
- struct sockaddr_un *sunaddr = (struct sockaddr_un *)uaddr;
- struct sock *sk = sock->sk, *newsk = NULL, *other = NULL;
- struct unix_sock *u = unix_sk(sk), *newu, *otheru;
- struct unix_peercred peercred = {};
- struct net *net = sock_net(sk);
- struct sk_buff *skb = NULL;
- unsigned char state;
- long timeo;
- int err;
+/*
+ * The state a stream connect() builds up before it has a peer: the new
+ * sock and the connection-request skb handed to the listener, the
+ * connecting side's credentials and its send timeout.
+ *
+ * - Built once by unix_stream_connect_setup()
+ * - Used to finish connecting by unix_stream_connect_commit()
+ * - Cleaned up in the failure case by unix_stream_connect_cleanup()
+ */
+struct unix_connect_state {
+ struct sock *newsk;
+ struct sk_buff *skb;
+ struct unix_peercred peercred;
+ long timeo;
+};
- err = unix_validate_addr(sunaddr, addr_len);
- if (err)
- goto out;
+/* Free a connect state that no connection consumed (i.e. on failure). */
+static void unix_stream_connect_cleanup(struct unix_connect_state *st)
+{
+ consume_skb(st->skb);
+ unix_release_sock(st->newsk, 0);
+ drop_peercred(&st->peercred);
+}
- err = BPF_CGROUP_RUN_PROG_UNIX_CONNECT_LOCK(sk, uaddr, &addr_len);
- if (err)
- goto out;
+/*
+ * Build the state a stream connect needs before it looks for a peer:
+ * autobind if required, snapshot the send timeout, and allocate the new
+ * sock, the request skb and the peer credentials. On failure nothing is
+ * left allocated in @st.
+ */
+static int unix_stream_connect_setup(struct socket *sock, int flags,
+ struct unix_connect_state *st)
+{
+ struct sock *sk = sock->sk, *newsk;
+ struct sk_buff *skb;
+ int err;
- if (unix_may_passcred(sk) && !READ_ONCE(u->addr)) {
+ if (unix_may_passcred(sk) && !READ_ONCE(unix_sk(sk)->addr)) {
err = unix_autobind(sk);
if (err)
- goto out;
+ return err;
}
- timeo = sock_sndtimeo(sk, flags & O_NONBLOCK);
+ st->timeo = sock_sndtimeo(sk, flags & O_NONBLOCK);
- err = prepare_peercred(&peercred);
+ err = prepare_peercred(&st->peercred);
if (err)
- goto out;
+ return err;
/* create new sock for complete connection */
- newsk = unix_create1(net, NULL, 0, sock->type);
+ newsk = unix_create1(sock_net(sk), NULL, 0, sock->type);
if (IS_ERR(newsk)) {
err = PTR_ERR(newsk);
- goto out;
+ goto out_drop;
}
/* Allocate skb for sending to listening sock */
@@ -1696,21 +1712,56 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr_unsized *uad
goto out_free_sk;
}
-restart:
- /* Find listening sock. */
- other = unix_find_other(net, sunaddr, addr_len, sk->sk_type, flags);
- if (IS_ERR(other)) {
- err = PTR_ERR(other);
- goto out_free_skb;
- }
+ st->newsk = newsk;
+ st->skb = skb;
+ return 0;
+
+out_free_sk:
+ unix_release_sock(newsk, 0);
+out_drop:
+ drop_peercred(&st->peercred);
+ return err;
+}
+
+/*
+ * Positive returns from unix_stream_connect_commit() ask the caller to
+ * try again. They are distinct only for a caller with a fixed peer
+ * (kernel_unix_connect_direct()): a full backlog can be retried on the
+ * same peer, but a peer found dead cannot -- the by-name path must
+ * re-resolve it, and a fixed peer has no such recourse and fails.
+ */
+#define UNIX_CONNECT_STALE 1 /* peer was found dead */
+#define UNIX_CONNECT_FULL 2 /* backlog was full and we slept */
+
+/*
+ * Try to connect @sk to the listening peer @other, using the connect
+ * state @st built by unix_stream_connect_setup(). Takes and releases
+ * unix_state_lock(@other) itself.
+ *
+ * Returns 0 on success (@st->skb queued to @other, @st->newsk linked to
+ * @sk and @st->peercred consumed), a negative errno on terminal failure,
+ * or a positive value (UNIX_CONNECT_STALE / UNIX_CONNECT_FULL) asking the
+ * caller to re-obtain @other and call again -- because @other was found
+ * dead, or its backlog was full and we slept (updating @st->timeo)
+ * waiting for room.
+ */
+static int unix_stream_connect_commit(struct sock *sk, struct sock *other,
+ struct unix_connect_state *st)
+{
+ struct sock *newsk = st->newsk;
+ struct sk_buff *skb = st->skb;
+ struct unix_peercred *peercred = &st->peercred;
+ long *timeo = &st->timeo;
+ struct unix_sock *newu, *otheru;
+ unsigned char state;
+ int err;
unix_state_lock(other);
- /* Apparently VFS overslept socket death. Retry. */
+ /* Apparently VFS overslept socket death; ask the caller to retry. */
if (sock_flag(other, SOCK_DEAD)) {
unix_state_unlock(other);
- sock_put(other);
- goto restart;
+ return UNIX_CONNECT_STALE;
}
if (other->sk_state != TCP_LISTEN ||
@@ -1720,19 +1771,19 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr_unsized *uad
}
if (unix_recvq_full_lockless(other)) {
- if (!timeo) {
+ if (!*timeo) {
err = -EAGAIN;
goto out_unlock;
}
- timeo = unix_wait_for_peer(other, timeo);
- sock_put(other);
+ /* unix_wait_for_peer() drops unix_state_lock(other). */
+ *timeo = unix_wait_for_peer(other, *timeo);
- err = sock_intr_errno(timeo);
+ err = sock_intr_errno(*timeo);
if (signal_pending(current))
- goto out_free_skb;
+ return err;
- goto restart;
+ return UNIX_CONNECT_FULL;
}
/* self connect and simultaneous connect are eliminated
@@ -1765,7 +1816,7 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr_unsized *uad
newsk->sk_state = TCP_ESTABLISHED;
newsk->sk_type = sk->sk_type;
newsk->sk_scm_recv_flags = other->sk_scm_recv_flags;
- init_peercred(newsk, &peercred);
+ init_peercred(newsk, peercred);
newu = unix_sk(newsk);
newu->listener = other;
@@ -1813,20 +1864,118 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr_unsized *uad
spin_unlock(&other->sk_receive_queue.lock);
unix_state_unlock(other);
READ_ONCE(other->sk_data_ready)(other);
- sock_put(other);
return 0;
out_unlock:
unix_state_unlock(other);
+ return err;
+}
+
+static int unix_stream_connect(struct socket *sock, struct sockaddr_unsized *uaddr,
+ int addr_len, int flags)
+{
+ struct sockaddr_un *sunaddr = (struct sockaddr_un *)uaddr;
+ struct sock *sk = sock->sk, *other;
+ struct unix_connect_state st = {};
+ struct net *net = sock_net(sk);
+ int err;
+
+ err = unix_validate_addr(sunaddr, addr_len);
+ if (err)
+ return err;
+
+ err = BPF_CGROUP_RUN_PROG_UNIX_CONNECT_LOCK(sk, uaddr, &addr_len);
+ if (err)
+ return err;
+
+ err = unix_stream_connect_setup(sock, flags, &st);
+ if (err)
+ return err;
+
+restart:
+ /* Find the listening sock. A positive return from
+ * unix_stream_connect_commit() means "retry": the peer had died,
+ * or its backlog was full and we slept -- so re-resolve the name.
+ */
+ other = unix_find_other(net, sunaddr, addr_len, sk->sk_type, flags);
+ if (IS_ERR(other)) {
+ err = PTR_ERR(other);
+ goto out_free;
+ }
+
+ err = unix_stream_connect_commit(sk, other, &st);
sock_put(other);
-out_free_skb:
- consume_skb(skb);
-out_free_sk:
- unix_release_sock(newsk, 0);
-out:
- drop_peercred(&peercred);
+ switch (err) {
+ case 0:
+ return 0;
+ case UNIX_CONNECT_FULL:
+ goto restart;
+ case UNIX_CONNECT_STALE:
+ /* A full backlog or a dead peer: re-resolve and try again. */
+ goto restart;
+ case INT_MIN ... -1:
+ /* terminal errno, propagate as-is */
+ break;
+ default:
+ /* commit() only returns 0, a retry code, or an errno */
+ WARN_ONCE(1, "unix_stream_connect_commit() returned %d\n", err);
+ err = -EINVAL;
+ break;
+ }
+out_free:
+ unix_stream_connect_cleanup(&st);
+ return err;
+}
+
+/**
+ * kernel_unix_connect_direct - connect a socket to a specific AF_UNIX sock
+ * @other: a held listening sock to connect to (e.g. from
+ * unix_lookup_bsd_path())
+ * @sock: the connecting socket, created with sock_create_kern()
+ * @flags: connect flags; without O_NONBLOCK a full listen backlog on
+ * @other is waited on, as for connect(2)
+ *
+ * Connects @sock to @other without any name lookup, address validation
+ * or path-based permission check. For in-kernel callers that have
+ * already located the target under their own policy. The caller
+ * retains its reference on @other.
+ */
+int kernel_unix_connect_direct(struct sock *other, struct socket *sock, int flags)
+{
+ struct sock *sk = sock->sk;
+ struct unix_connect_state st = {};
+ int err;
+
+ err = unix_stream_connect_setup(sock, flags, &st);
+ if (err)
+ return err;
+
+restart:
+ sock_hold(other);
+ err = unix_stream_connect_commit(sk, other, &st);
+ sock_put(other);
+ switch (err) {
+ case 0:
+ return 0;
+ case UNIX_CONNECT_FULL:
+ goto restart;
+ case UNIX_CONNECT_STALE:
+ /* The peer is fixed, so a dead one cannot be re-found. */
+ err = -ECONNREFUSED;
+ break;
+ case INT_MIN ... -1:
+ /* terminal errno, propagate as-is */
+ break;
+ default:
+ /* commit() only returns 0, a retry code, or an errno */
+ WARN_ONCE(1, "unix_stream_connect_commit() returned %d\n", err);
+ err = -EINVAL;
+ break;
+ }
+ unix_stream_connect_cleanup(&st);
return err;
}
+EXPORT_SYMBOL_GPL(kernel_unix_connect_direct);
static int unix_socketpair(struct socket *socka, struct socket *sockb)
{
--
2.54.0
^ permalink raw reply related
* [RFC PATCH 1/3] af_unix: factor out unix_lookup_bsd_path()
From: John Ericson @ 2026-07-03 7:39 UTC (permalink / raw)
To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: John Ericson, Cong Wang, Kuniyuki Iwashima, Simon Horman,
Christian Brauner, David Rheinsberg, Andy Lutomirski,
Sergei Zimmerman, netdev, linux-fsdevel, Mickaël Salaün,
Günther Noack, Paul Moore, linux-security-module,
linux-kernel
In-Reply-To: <20260703073948.2541875-1-John.Ericson@Obsidian.Systems>
From: John Ericson <mail@JohnEricson.me>
Split the inode -> sock mapping out of `unix_find_bsd()` into a new
helper, `unix_lookup_bsd_path()`: given an already-resolved `struct
path`, check it is a socket, look the bound socket up by inode, and
check its type, returning a held `struct sock` (or an `ERR_PTR`).
`unix_find_bsd()` keeps doing the path resolution, the `MAY_WRITE`
permission check, the `security_unix_find()` LSM hook and
`touch_atime()`, and calls the helper for the lookup. No functional
change.
The function documentation anticipates (in an example) the way this will
be used later in the patch series.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: John Ericson <mail@JohnEricson.me>
---
include/net/af_unix.h | 1 +
net/unix/af_unix.c | 50 ++++++++++++++++++++++++++++++++-----------
2 files changed, 39 insertions(+), 12 deletions(-)
diff --git a/include/net/af_unix.h b/include/net/af_unix.h
index 34f53dde65ce..fe4547508af1 100644
--- a/include/net/af_unix.h
+++ b/include/net/af_unix.h
@@ -14,6 +14,7 @@
#if IS_ENABLED(CONFIG_UNIX)
struct unix_sock *unix_get_socket(struct file *filp);
+struct sock *unix_lookup_bsd_path(const struct path *path, int type);
#else
static inline struct unix_sock *unix_get_socket(struct file *filp)
{
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index f7a9d55eee8a..3270299238c4 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1185,10 +1185,43 @@ static int unix_release(struct socket *sock)
return 0;
}
+/**
+ * unix_lookup_bsd_path - find the AF_UNIX socket bound at a resolved path
+ * @path: a path the caller has already resolved under its own policy
+ * @type: required socket type (SOCK_STREAM/SOCK_SEQPACKET/SOCK_DGRAM)
+ *
+ * Unlike the connect(2) lookup, this performs no path resolution and no
+ * DAC or LSM check of its own: the caller is responsible for having
+ * resolved @path with whatever policy is appropriate. Used by kernel
+ * callers (e.g. coredump-to-socket) that must resolve the path under
+ * their own root and credentials rather than the current task's.
+ *
+ * Returns a held sock, or an ERR_PTR.
+ */
+struct sock *unix_lookup_bsd_path(const struct path *path, int type)
+{
+ struct inode *inode = d_backing_inode(path->dentry);
+ struct sock *sk;
+
+ if (!S_ISSOCK(inode->i_mode))
+ return ERR_PTR(-ECONNREFUSED);
+
+ sk = unix_find_socket_byinode(inode);
+ if (!sk)
+ return ERR_PTR(-ECONNREFUSED);
+
+ if (sk->sk_type != type) {
+ sock_put(sk);
+ return ERR_PTR(-EPROTOTYPE);
+ }
+
+ return sk;
+}
+EXPORT_SYMBOL_GPL(unix_lookup_bsd_path);
+
static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
int type, int flags)
{
- struct inode *inode;
struct path path;
struct sock *sk;
int err;
@@ -1219,18 +1252,11 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
goto path_put;
}
- err = -ECONNREFUSED;
- inode = d_backing_inode(path.dentry);
- if (!S_ISSOCK(inode->i_mode))
+ sk = unix_lookup_bsd_path(&path, type);
+ if (IS_ERR(sk)) {
+ err = PTR_ERR(sk);
goto path_put;
-
- sk = unix_find_socket_byinode(inode);
- if (!sk)
- goto path_put;
-
- err = -EPROTOTYPE;
- if (sk->sk_type != type)
- goto sock_put;
+ }
err = security_unix_find(&path, sk, flags);
if (err)
--
2.54.0
^ permalink raw reply related
* [RFC PATCH 0/3] coredump, net: fix layer violation with direct connection
From: John Ericson @ 2026-07-03 7:39 UTC (permalink / raw)
To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: John Ericson, Cong Wang, Kuniyuki Iwashima, Simon Horman,
Christian Brauner, David Rheinsberg, Andy Lutomirski,
Sergei Zimmerman, netdev, linux-fsdevel, Mickaël Salaün,
Günther Noack, Paul Moore, linux-security-module,
linux-kernel
From: John Ericson <mail@JohnEricson.me>
In https://lore.kernel.org/all/akWxrjOl4Up02Bvq@pop-os.localdomain/ Cong
Wang asked about doing things without new syscalls for my fd-based
connect idea. This got me investigating a few things, all of which I hope to
submit as patches.
This is the first one. I stumbled on `SOCK_COREDUMP` in `af_unix.c`, and
I realized that this was --- right in the part of the kernel I was
already looking at --- an excellent example of something that directly
connecting to a socket could do better. This is not because the
filesystem would never be involved (the interface in procfs still
specifies a path) but because the core dumper wants to resolve that path
differently than the usual way.
The first two commits are refactors that expose/create the necessary
functionality, and then the last commit actually does the untangling of
the unix socket implementation and the core dumper. See especially that
third commit message for details.
I hope this is a compelling use-case for you all, that does not touch
the UABI yet, but also does just the sort of thing that would be nice to
expose with a new syscall.
John
John Ericson (3):
af_unix: factor out unix_lookup_bsd_path()
af_unix: factor out kernel_unix_connect_direct()
coredump, net: remove `SOCK_COREDUMP`
fs/coredump.c | 47 +++--
include/linux/lsm_hook_defs.h | 3 +-
include/linux/net.h | 1 -
include/linux/security.h | 4 +-
include/net/af_unix.h | 2 +
net/unix/af_unix.c | 335 +++++++++++++++++++++++++---------
security/landlock/fs.c | 7 +-
security/security.c | 5 +-
8 files changed, 287 insertions(+), 117 deletions(-)
--
2.54.0
^ permalink raw reply
* [PATCH 1/1] net: usb: aqc111: fix set_mac_address return value for bonding
From: Hanson Wang @ 2026-07-03 7:39 UTC (permalink / raw)
To: netdev; +Cc: linux-usb, oneukum, Hanson Wang
aqc111_set_mac_addr() returns the result of aqc111_write_cmd() on
success. That function wraps usb_control_msg(), which returns the
number of bytes transferred (6 for ETH_ALEN) rather than zero.
Bonding calls ndo_set_mac_address() when enslaving an interface and
treats any non-zero return value as failure.
Return 0 on success and propagate negative error codes on failure.
Signed-off-by: Hanson Wang <hanson.wang@ugreen.com>
---
drivers/net/usb/aqc111.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/aqc111.c b/drivers/net/usb/aqc111.c
index dd53f413c38f..da5b74637ee2 100644
--- a/drivers/net/usb/aqc111.c
+++ b/drivers/net/usb/aqc111.c
@@ -471,8 +471,12 @@ static int aqc111_set_mac_addr(struct net_device *net, void *p)
return ret;
/* Set the MAC address */
- return aqc111_write_cmd(dev, AQ_ACCESS_MAC, SFR_NODE_ID, ETH_ALEN,
- ETH_ALEN, net->dev_addr);
+ ret = aqc111_write_cmd(dev, AQ_ACCESS_MAC, SFR_NODE_ID, ETH_ALEN,
+ ETH_ALEN, net->dev_addr);
+ if (ret < 0)
+ return ret;
+
+ return 0;
}
static int aqc111_vlan_rx_kill_vid(struct net_device *net,
--
2.34.1
^ permalink raw reply related
* Re: [PATCH RFC net-next] net: bridge: add BRIDGE_VLAN_INFO_DYNAMIC flag
From: Nikolay Aleksandrov @ 2026-07-03 7:34 UTC (permalink / raw)
To: Luke Howard, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Ido Schimmel, Simon Horman
Cc: Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
Christoph Mellauner, Simon Gapp, netdev, bridge, linux-kernel
In-Reply-To: <20260703-vlan-dynamic-entry-v1-1-c2cbb134036c@padl.com>
On 03/07/2026 09:54, Luke Howard wrote:
> Dynamic VLAN entries as specified in IEEE Std 802.1Q-2022,
> Clause 8.8.5. These allow a user-space VLAN registration
> protocol such as MVRP to mark a bridge's VLAN entries as
> dynamic, so they can be distinguished from administratively
> configured static entries.
>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Luke Howard <lukeh@padl.com>
> ---
> This patch adds support for Dynamic VLAN Entries, as specified
> in 802.1Q. I have marked it as RFC as a similar patch to add
> support for Dynamic Reservation Entries to the FDB was rejected
> on the argument that the dynamic bit should be stored separately.
>
> Our MVRP implementation [1] will use this flag if available.
>
> [1] https://github.com/PADL/OpenSRP
Hi,
Do this tracking entirely in your app, it doesn't bring anything to the
kernel and just adding flags because it is convenient is not acceptable.
These VLANs are the same from kernel POV and this flag doesn't change
anything functionally.
Alternatively perhaps a proto field similar to routes could help you out to
distinguish who installed a VLAN entry, not sure if that would help your
user-space app though, and you'll have to add a new rt proto id that should
get accepted.
Cheers,
Nik
> ---
> include/uapi/linux/if_bridge.h | 1 +
> net/bridge/br_netlink.c | 6 ++++++
> net/bridge/br_vlan.c | 18 ++++++++++++++----
> 3 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> index 21a700c02ef76..a3abbfe79694b 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -134,6 +134,7 @@ enum {
> #define BRIDGE_VLAN_INFO_RANGE_END (1<<4) /* VLAN is end of vlan range */
> #define BRIDGE_VLAN_INFO_BRENTRY (1<<5) /* Global bridge VLAN entry */
> #define BRIDGE_VLAN_INFO_ONLY_OPTS (1<<6) /* Skip create/delete/flags */
> +#define BRIDGE_VLAN_INFO_DYNAMIC (1<<7) /* 802.1Q Dynamic VLAN Registration Entry */
>
> struct bridge_vlan_info {
> __u16 flags;
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index b2cd4e39326d0..90f2e103f53d5 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -388,6 +388,9 @@ static int br_fill_ifvlaninfo_compressed(struct sk_buff *skb,
> if (v->flags & BRIDGE_VLAN_INFO_UNTAGGED)
> flags |= BRIDGE_VLAN_INFO_UNTAGGED;
>
> + if (v->flags & BRIDGE_VLAN_INFO_DYNAMIC)
> + flags |= BRIDGE_VLAN_INFO_DYNAMIC;
> +
> if (vid_range_start == 0) {
> goto initvars;
> } else if ((v->vid - vid_range_end) == 1 &&
> @@ -440,6 +443,9 @@ static int br_fill_ifvlaninfo(struct sk_buff *skb,
> if (v->flags & BRIDGE_VLAN_INFO_UNTAGGED)
> vinfo.flags |= BRIDGE_VLAN_INFO_UNTAGGED;
>
> + if (v->flags & BRIDGE_VLAN_INFO_DYNAMIC)
> + vinfo.flags |= BRIDGE_VLAN_INFO_DYNAMIC;
> +
> if (nla_put(skb, IFLA_BRIDGE_VLAN_INFO,
> sizeof(vinfo), &vinfo))
> goto nla_put_failure;
> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
> index 5560afcaaca32..e6bc59e3a4c4f 100644
> --- a/net/bridge/br_vlan.c
> +++ b/net/bridge/br_vlan.c
> @@ -54,9 +54,11 @@ static void __vlan_delete_pvid(struct net_bridge_vlan_group *vg, u16 vid)
> vg->pvid = 0;
> }
>
> -/* Update the BRIDGE_VLAN_INFO_PVID and BRIDGE_VLAN_INFO_UNTAGGED flags of @v.
> - * If @commit is false, return just whether the BRIDGE_VLAN_INFO_PVID and
> - * BRIDGE_VLAN_INFO_UNTAGGED bits of @flags would produce any change onto @v.
> +/* Update the BRIDGE_VLAN_INFO_PVID, BRIDGE_VLAN_INFO_UNTAGGED and
> + * BRIDGE_VLAN_INFO_DYNAMIC flags of @v.
> + * If @commit is false, return just whether the BRIDGE_VLAN_INFO_PVID,
> + * BRIDGE_VLAN_INFO_UNTAGGED and BRIDGE_VLAN_INFO_DYNAMIC bits of @flags
> + * would produce any change onto @v.
> */
> static bool __vlan_flags_update(struct net_bridge_vlan *v, u16 flags,
> bool commit)
> @@ -71,7 +73,8 @@ static bool __vlan_flags_update(struct net_bridge_vlan *v, u16 flags,
>
> /* check if anything would be changed on commit */
> change = !!(flags & BRIDGE_VLAN_INFO_PVID) == !!(vg->pvid != v->vid) ||
> - ((flags ^ v->flags) & BRIDGE_VLAN_INFO_UNTAGGED);
> + ((flags ^ v->flags) & (BRIDGE_VLAN_INFO_UNTAGGED |
> + BRIDGE_VLAN_INFO_DYNAMIC));
>
> if (!commit)
> goto out;
> @@ -86,6 +89,11 @@ static bool __vlan_flags_update(struct net_bridge_vlan *v, u16 flags,
> else
> v->flags &= ~BRIDGE_VLAN_INFO_UNTAGGED;
>
> + if (flags & BRIDGE_VLAN_INFO_DYNAMIC)
> + v->flags |= BRIDGE_VLAN_INFO_DYNAMIC;
> + else
> + v->flags &= ~BRIDGE_VLAN_INFO_DYNAMIC;
> +
> out:
> return change;
> }
> @@ -1874,6 +1882,8 @@ static bool br_vlan_fill_vids(struct sk_buff *skb, u16 vid, u16 vid_range,
> info.flags |= BRIDGE_VLAN_INFO_UNTAGGED;
> if (flags & BRIDGE_VLAN_INFO_PVID)
> info.flags |= BRIDGE_VLAN_INFO_PVID;
> + if (flags & BRIDGE_VLAN_INFO_DYNAMIC)
> + info.flags |= BRIDGE_VLAN_INFO_DYNAMIC;
>
> if (nla_put(skb, BRIDGE_VLANDB_ENTRY_INFO, sizeof(info), &info))
> goto out_err;
>
> ---
> base-commit: 2bb62a85aff6d4c14a62a476dfabaada3c1cc014
> change-id: 20260703-vlan-dynamic-entry-56a028e8990e
>
> Best regards,
> --
> Luke Howard <lukeh@padl.com>
>
^ permalink raw reply
* Re: [PATCH net] bnge/bng_re: fix ring ID widths
From: Paolo Abeni @ 2026-07-03 7:33 UTC (permalink / raw)
To: Vikas Gupta, davem, edumazet, kuba, andrew+netdev, horms
Cc: netdev, linux-kernel, linux-rdma, leonro, jgg, bhargava.marreddy,
rahul-rg.gupta, vsrama-krishna.nemani, rajashekar.hudumula,
ajit.khaparde, Siva Reddy Kallam, Dharmender Garg,
Yendapally Reddy Dhananjaya Reddy
In-Reply-To: <20260630101554.1221733-1-vikas.gupta@broadcom.com>
On 6/30/26 12:15 PM, Vikas Gupta wrote:
> diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_rmem.h b/drivers/net/ethernet/broadcom/bnge/bnge_rmem.h
> index 341c7f81ed09..bb0c79a1ee60 100644
> --- a/drivers/net/ethernet/broadcom/bnge/bnge_rmem.h
> +++ b/drivers/net/ethernet/broadcom/bnge/bnge_rmem.h
> @@ -184,7 +184,7 @@ struct bnge_ctx_mem_info {
> struct bnge_ring_struct {
> struct bnge_ring_mem_info ring_mem;
>
> - u16 fw_ring_id;
> + u32 fw_ring_id;
Sashiko gemini has a few concerns about the id size increases:
https://sashiko.dev/#/patchset/20260630101554.1221733-1-vikas.gupta%40broadcom.com
please have a look.
/P
^ permalink raw reply
* Re: [PATCH net] net/stmmac: Set Rx queue page_pool to NULL when freeing DMA resources
From: Paolo Abeni @ 2026-07-03 7:26 UTC (permalink / raw)
To: j.raczynski
Cc: netdev, andrew+netdev, davem, edumazet, kuba, pabeni,
mcoquelin.stm32, linux-kernel, k.tegowski, k.domagalski,
yashwant.v
In-Reply-To: <20260630100953.747868-1-j.raczynski@samsung.com>
From: AI Reviewer <ai@example.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
net/stmmac: Set Rx queue page_pool to NULL when freeing DMA resources
This patch makes __free_dma_rx_desc_resources() clear rx_q->page_pool
after destroying it, to avoid a kernel panic if a subsequent
reinitialization triggers another teardown while page_pool_create()
fails in __alloc_dma_rx_desc_resources().
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 2a0d7eff88d3..6cc5a3496f39 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -2172,8 +2172,10 @@ static void __free_dma_rx_desc_resources(struct stmmac_priv *priv,
> xdp_rxq_info_unreg(&rx_q->xdp_rxq);
>
> kfree(rx_q->buf_pool);
> - if (rx_q->page_pool)
> + if (rx_q->page_pool) {
> page_pool_destroy(rx_q->page_pool);
> + rx_q->page_pool = NULL;
> + }
> }
[High]
Does this fix leave the other resources in __free_dma_rx_desc_resources()
exposed to the same crash class the commit message describes?
The same teardown function also frees rx_q->buf_pool, rx_q->dma_rx (or
rx_q->dma_erx), and uses rx_q->dma_rx_phy without ever clearing any of
those pointers:
static void __free_dma_rx_desc_resources(...)
{
...
if (rx_q->xsk_pool)
dma_free_rx_xskbufs(priv, dma_conf, queue);
else
dma_free_rx_skbufs(priv, dma_conf, queue);
...
if (priv->extend_desc)
addr = rx_q->dma_erx;
else
addr = rx_q->dma_rx;
size = stmmac_get_rx_desc_size(priv) * dma_conf->dma_rx_size;
dma_free_coherent(priv->device, size, addr, rx_q->dma_rx_phy);
...
kfree(rx_q->buf_pool);
if (rx_q->page_pool) {
page_pool_destroy(rx_q->page_pool);
rx_q->page_pool = NULL;
}
}
Consider the same failure scenario the commit message names (a
page_pool_create() failure during reinitialization), but applied to a
later queue.
After ifdown, __free_dma_rx_desc_resources() has run on every queue and
buf_pool, dma_rx/dma_erx, and dma_rx_phy are all stale dangling values
on every queue.
On ifup, __alloc_dma_rx_desc_resources() begins assigning fresh pointers
queue by queue:
rx_q->page_pool = page_pool_create(&pp_params);
if (IS_ERR(rx_q->page_pool)) {
ret = PTR_ERR(rx_q->page_pool);
rx_q->page_pool = NULL;
return ret;
}
rx_q->buf_pool = kzalloc_objs(*rx_q->buf_pool, ...);
...
addr = dma_alloc_coherent(priv->device, size, &rx_q->dma_rx_phy, ...);
...
If page_pool_create() fails on queue Q, the rollback in
alloc_dma_rx_desc_resources():
err_dma:
free_dma_rx_desc_resources(priv, dma_conf);
walks every queue including Q+1..N-1, where buf_pool, dma_rx/dma_erx and
dma_rx_phy still hold the stale values left behind by the prior
__free_dma_rx_desc_resources() call.
Does __free_dma_rx_desc_resources() on those queues then:
- read rx_q->buf_pool[i] in dma_free_rx_skbufs() ->
stmmac_free_rx_buffer() from already-freed slab memory:
static void stmmac_free_rx_buffer(...)
{
struct stmmac_rx_buffer *buf = &rx_q->buf_pool[i];
if (buf->page)
page_pool_put_full_page(rx_q->page_pool, buf->page, false);
...
}
- call dma_free_coherent() a second time on the stale
rx_q->dma_rx/dma_erx and rx_q->dma_rx_phy?
- call kfree() a second time on the stale rx_q->buf_pool?
Would also nulling rx_q->buf_pool, rx_q->dma_rx, rx_q->dma_erx, and
rx_q->dma_rx_phy in __free_dma_rx_desc_resources() (or having the alloc
path defensively reset them before any early return) be appropriate to
cover the same failure mode the commit message describes?
--
This is an AI-generated review.
^ permalink raw reply
* [PATCH net-next 12/12] net: dsa: validate source trunk against lags_len
From: Luke Howard @ 2026-07-03 7:30 UTC (permalink / raw)
To: Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Vivien Didelot, Gregory CLEMENT, Andrew Lunn,
Simon Horman, Florian Fainelli, Tobias Waldekranz
Cc: Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
Christoph Mellauner, Simon Gapp, Ryan Wilkins, Mattias Forsblad,
netdev, linux-kernel, Luke Howard
In-Reply-To: <20260703-net-next-dsa-rmu-v1-0-a03febf20bb4@padl.com>
A DSA frame with an invalid source trunk ID could cause an out-of-bounds
read access of dst->lags.
Add a check to dsa_lag_by_id() to validate the LAG ID is not zero, and is
less than or equal to dst->lags_len. (The LAG ID is derived by adding one
to the source trunk ID.)
Note: this is in the fast path for any frames within a trunk.
Fixes: 5b60dadb71db ("net: dsa: tag_dsa: Support reception of packets from LAG devices")
Signed-off-by: Luke Howard <lukeh@padl.com>
---
include/net/dsa.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index af16347a3d331..9f01ea5bbd877 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -190,6 +190,9 @@ struct dsa_switch_tree {
static inline struct dsa_lag *dsa_lag_by_id(struct dsa_switch_tree *dst,
unsigned int id)
{
+ if (unlikely(id == 0 || id > dst->lags_len))
+ return NULL;
+
/* DSA LAG IDs are one-based, dst->lags is zero-based */
return dst->lags[id - 1];
}
--
2.43.0
^ permalink raw reply related
* [PATCH net-next 11/12] net: dsa: Add helper to find ds_switch by index
From: Luke Howard @ 2026-07-03 7:30 UTC (permalink / raw)
To: Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Vivien Didelot, Gregory CLEMENT, Andrew Lunn,
Simon Horman, Florian Fainelli, Tobias Waldekranz
Cc: Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
Christoph Mellauner, Simon Gapp, Ryan Wilkins, Mattias Forsblad,
netdev, linux-kernel, Luke Howard
In-Reply-To: <20260703-net-next-dsa-rmu-v1-0-a03febf20bb4@padl.com>
From: Andrew Lunn <andrew@lunn.ch>
The DSA header for an RMU frame includes the switch index to indicate
which switch sent the RMU frame. Add a helper which walks the list of
ports and finds the corresponding switch. Since this is not the hot
path for data frames, the overhead of multiple ports per switch is not
considered a problem.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
net/dsa/tag.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/net/dsa/tag.h b/net/dsa/tag.h
index cf52283fe9df4..c21d746652601 100644
--- a/net/dsa/tag.h
+++ b/net/dsa/tag.h
@@ -44,6 +44,20 @@ static inline struct net_device *dsa_conduit_find_user(struct net_device *dev,
return NULL;
}
+static inline struct dsa_switch *dsa_conduit_find_switch(struct net_device *dev,
+ int device)
+{
+ struct dsa_port *cpu_dp = dev->dsa_ptr;
+ struct dsa_switch_tree *dst = cpu_dp->dst;
+ struct dsa_port *dp;
+
+ list_for_each_entry(dp, &dst->ports, list)
+ if (dp->ds->index == device)
+ return dp->ds;
+
+ return NULL;
+}
+
/**
* dsa_software_untag_vlan_aware_bridge: Software untagging for VLAN-aware bridge
* @skb: Pointer to received socket buffer (packet)
--
2.43.0
^ permalink raw reply related
* [PATCH net-next 10/12] net: dsa: qca8k: drop redundant mgmt_eth_data
From: Luke Howard @ 2026-07-03 7:30 UTC (permalink / raw)
To: Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Vivien Didelot, Gregory CLEMENT, Andrew Lunn,
Simon Horman, Florian Fainelli, Tobias Waldekranz
Cc: Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
Christoph Mellauner, Simon Gapp, Ryan Wilkins, Mattias Forsblad,
netdev, linux-kernel, Luke Howard, Christian Marangi
In-Reply-To: <20260703-net-next-dsa-rmu-v1-0-a03febf20bb4@padl.com>
From: Christian Marangi <ansuelsmth@gmail.com>
Now that we generalized inband logic and functions to dsa core, we can
drop the custom qca8k struct to handle inband mgmt and use directly the
dsa core dsa_inband struct in the priv struct.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/dsa/qca/qca8k-8xxx.c | 30 +++++++++++-------------------
drivers/net/dsa/qca/qca8k.h | 6 +-----
2 files changed, 12 insertions(+), 24 deletions(-)
diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index 14371c3c9a459..3314dff580316 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -162,7 +162,6 @@ qca8k_set_page(struct qca8k_priv *priv, u16 page)
static void qca8k_rw_reg_ack_handler(struct dsa_switch *ds, struct sk_buff *skb)
{
- struct qca8k_mgmt_eth_data *mgmt_eth_data;
struct qca8k_priv *priv = ds->priv;
struct qca_mgmt_ethhdr *mgmt_ethhdr;
u32 command;
@@ -172,7 +171,6 @@ static void qca8k_rw_reg_ack_handler(struct dsa_switch *ds, struct sk_buff *skb)
int i;
mgmt_ethhdr = (struct qca_mgmt_ethhdr *)skb_mac_header(skb);
- mgmt_eth_data = &priv->mgmt_eth_data;
command = get_unaligned_le32(&mgmt_ethhdr->command);
cmd = FIELD_GET(QCA_HDR_MGMT_CMD, command);
@@ -189,7 +187,7 @@ static void qca8k_rw_reg_ack_handler(struct dsa_switch *ds, struct sk_buff *skb)
/* Make sure the seq match the requested packet. If not, drop. */
if (get_unaligned_le32(&mgmt_ethhdr->seq) !=
- dsa_inband_seqno(&mgmt_eth_data->inband))
+ dsa_inband_seqno(&priv->inband))
return;
if (cmd == MDIO_READ) {
@@ -215,7 +213,7 @@ static void qca8k_rw_reg_ack_handler(struct dsa_switch *ds, struct sk_buff *skb)
}
}
- dsa_inband_complete(&mgmt_eth_data->inband, &data, sizeof(data), err);
+ dsa_inband_complete(&priv->inband, data, sizeof(data), err);
}
static struct sk_buff *qca8k_alloc_mdio_header(enum mdio_cmd cmd, u32 reg, u32 *val,
@@ -314,7 +312,6 @@ static void qca8k_mdio_header_fill_seq_num(struct sk_buff *skb, u32 seq_num)
static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len)
{
- struct qca8k_mgmt_eth_data *mgmt_eth_data = &priv->mgmt_eth_data;
struct sk_buff *skb;
u32 data[4];
int ret;
@@ -326,7 +323,7 @@ static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len)
skb->dev = READ_ONCE(priv->mgmt_conduit);
- ret = dsa_inband_request(&mgmt_eth_data->inband, skb,
+ ret = dsa_inband_request(&priv->inband, skb,
qca8k_mdio_header_fill_seq_num,
data, sizeof(data),
QCA8K_ETHERNET_TIMEOUT);
@@ -343,7 +340,6 @@ static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len)
static int qca8k_write_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len)
{
- struct qca8k_mgmt_eth_data *mgmt_eth_data = &priv->mgmt_eth_data;
struct sk_buff *skb;
skb = qca8k_alloc_mdio_header(MDIO_WRITE, reg, val,
@@ -353,7 +349,7 @@ static int qca8k_write_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len)
skb->dev = READ_ONCE(priv->mgmt_conduit);
- return dsa_inband_request(&mgmt_eth_data->inband, skb,
+ return dsa_inband_request(&priv->inband, skb,
qca8k_mdio_header_fill_seq_num,
NULL, 0,
QCA8K_ETHERNET_TIMEOUT);
@@ -534,7 +530,7 @@ static const struct regmap_config qca8k_regmap_config = {
};
static int
-qca8k_phy_eth_busy_wait(struct qca8k_mgmt_eth_data *mgmt_eth_data,
+qca8k_phy_eth_busy_wait(struct dsa_inband *inband,
struct sk_buff *read_skb, u32 *val)
{
struct sk_buff *skb = skb_copy(read_skb, GFP_KERNEL);
@@ -544,8 +540,7 @@ qca8k_phy_eth_busy_wait(struct qca8k_mgmt_eth_data *mgmt_eth_data,
if (!skb)
return -ENOMEM;
- ret = dsa_inband_request(&mgmt_eth_data->inband, skb,
- qca8k_mdio_header_fill_seq_num,
+ ret = dsa_inband_request(inband, skb, qca8k_mdio_header_fill_seq_num,
data, sizeof(data),
QCA8K_ETHERNET_TIMEOUT);
@@ -562,7 +557,6 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
int regnum, u16 data)
{
struct sk_buff *write_skb, *clear_skb, *read_skb;
- struct qca8k_mgmt_eth_data *mgmt_eth_data;
u32 write_val, clear_val = 0, val;
struct net_device *mgmt_conduit;
u32 resp_data[4];
@@ -571,8 +565,6 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
if (regnum >= QCA8K_MDIO_MASTER_MAX_REG)
return -EINVAL;
- mgmt_eth_data = &priv->mgmt_eth_data;
-
write_val = QCA8K_MDIO_MASTER_BUSY | QCA8K_MDIO_MASTER_EN |
QCA8K_MDIO_MASTER_PHY_ADDR(phy) |
QCA8K_MDIO_MASTER_REG_ADDR(regnum);
@@ -624,7 +616,7 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
clear_skb->dev = mgmt_conduit;
write_skb->dev = mgmt_conduit;
- ret = dsa_inband_request(&mgmt_eth_data->inband, write_skb,
+ ret = dsa_inband_request(&priv->inband, write_skb,
qca8k_mdio_header_fill_seq_num,
NULL, 0,
QCA8K_ETHERNET_TIMEOUT);
@@ -637,7 +629,7 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
ret = read_poll_timeout(qca8k_phy_eth_busy_wait, ret1,
!(val & QCA8K_MDIO_MASTER_BUSY), 0,
QCA8K_BUSY_WAIT_TIMEOUT * USEC_PER_MSEC, false,
- mgmt_eth_data, read_skb, &val);
+ &priv->inband, read_skb, &val);
if (ret < 0 && ret1 < 0) {
ret = ret1;
@@ -645,7 +637,7 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
}
if (read) {
- ret = dsa_inband_request(&mgmt_eth_data->inband, read_skb,
+ ret = dsa_inband_request(&priv->inband, read_skb,
qca8k_mdio_header_fill_seq_num,
resp_data, sizeof(resp_data),
QCA8K_ETHERNET_TIMEOUT);
@@ -660,7 +652,7 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
exit:
/* This is expected to fail sometimes, so don't check return value. */
- dsa_inband_request(&mgmt_eth_data->inband, clear_skb,
+ dsa_inband_request(&priv->inband, clear_skb,
qca8k_mdio_header_fill_seq_num,
NULL, 0,
QCA8K_ETHERNET_TIMEOUT);
@@ -1991,7 +1983,7 @@ qca8k_sw_probe(struct mdio_device *mdiodev)
if (!priv->ds)
return -ENOMEM;
- dsa_inband_init(&priv->mgmt_eth_data.inband, U32_MAX);
+ dsa_inband_init(&priv->inband, U32_MAX);
mutex_init(&priv->mib_eth_data.mutex);
init_completion(&priv->mib_eth_data.rw_done);
diff --git a/drivers/net/dsa/qca/qca8k.h b/drivers/net/dsa/qca/qca8k.h
index ccf92c85ccb14..48bd5531d13e2 100644
--- a/drivers/net/dsa/qca/qca8k.h
+++ b/drivers/net/dsa/qca/qca8k.h
@@ -391,10 +391,6 @@ enum {
QCA8K_CPU_PORT6,
};
-struct qca8k_mgmt_eth_data {
- struct dsa_inband inband;
-};
-
struct qca8k_mib_eth_data {
struct completion rw_done;
struct mutex mutex; /* Process one command at time */
@@ -458,7 +454,7 @@ struct qca8k_priv {
struct device *dev;
struct gpio_desc *reset_gpio;
struct net_device *mgmt_conduit; /* Track if mdio/mib Ethernet is available */
- struct qca8k_mgmt_eth_data mgmt_eth_data;
+ struct dsa_inband inband;
struct qca8k_mib_eth_data mib_eth_data;
struct qca8k_mdio_cache mdio_cache;
struct qca8k_pcs pcs_port_0;
--
2.43.0
^ permalink raw reply related
* [PATCH net-next 09/12] net: dsa: qca8k: Move inband mutex into DSA core
From: Luke Howard @ 2026-07-03 7:30 UTC (permalink / raw)
To: Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Vivien Didelot, Gregory CLEMENT, Andrew Lunn,
Simon Horman, Florian Fainelli, Tobias Waldekranz
Cc: Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
Christoph Mellauner, Simon Gapp, Ryan Wilkins, Mattias Forsblad,
netdev, linux-kernel, Luke Howard
In-Reply-To: <20260703-net-next-dsa-rmu-v1-0-a03febf20bb4@padl.com>
From: Andrew Lunn <andrew@lunn.ch>
The mutex serves two purposes:
It serialises operations on the switch, so that only one
request/response can be happening at once.
It protects priv->mgmt_master, which itself has two purposes. If the
hardware is wrongly wired, the wrong switch port is connected to the
cpu, inband cannot be used. In this case it has the value
NULL. Additionally, if the master is down, it is set to NULL.
Otherwise it points to the netdev used to send frames to the switch.
The protection of priv->mgmt_master is not required. It is a single
pointer, which will be updated atomically. It is not expected that the
interface disappears, it only goes down. Hence mgmt_master will always
be valid, or NULL.
Move the check for the master device being NULL into the core. Also,
move the mutex for serialisation into the core.
The MIB operations don't follow request/response semantics, so its
mutex is left untouched.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/dsa/qca/qca8k-8xxx.c | 68 ++++++++------------------------------
drivers/net/dsa/qca/qca8k-common.c | 2 +-
drivers/net/dsa/qca/qca8k.h | 1 -
include/net/dsa.h | 4 +++
net/dsa/dsa.c | 9 +++++
5 files changed, 27 insertions(+), 57 deletions(-)
diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index 94332497902c2..14371c3c9a459 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -324,65 +324,39 @@ static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len)
if (!skb)
return -ENOMEM;
- mutex_lock(&mgmt_eth_data->mutex);
-
- /* Check if the mgmt_conduit if is operational */
- if (!priv->mgmt_conduit) {
- kfree_skb(skb);
- mutex_unlock(&mgmt_eth_data->mutex);
- return -EINVAL;
- }
-
- skb->dev = priv->mgmt_conduit;
+ skb->dev = READ_ONCE(priv->mgmt_conduit);
ret = dsa_inband_request(&mgmt_eth_data->inband, skb,
qca8k_mdio_header_fill_seq_num,
data, sizeof(data),
QCA8K_ETHERNET_TIMEOUT);
if (ret < 0)
- goto out;
+ return ret;
ret = 0;
*val = data[0];
if (len > QCA_HDR_MGMT_DATA1_LEN)
memcpy(val + 1, &data[1], len - QCA_HDR_MGMT_DATA1_LEN);
-out:
- mutex_unlock(&mgmt_eth_data->mutex);
-
- return ret;
+ return 0;
}
static int qca8k_write_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len)
{
struct qca8k_mgmt_eth_data *mgmt_eth_data = &priv->mgmt_eth_data;
struct sk_buff *skb;
- int ret;
skb = qca8k_alloc_mdio_header(MDIO_WRITE, reg, val,
QCA8K_ETHERNET_MDIO_PRIORITY, len);
if (!skb)
return -ENOMEM;
- mutex_lock(&mgmt_eth_data->mutex);
+ skb->dev = READ_ONCE(priv->mgmt_conduit);
- /* Check if the mgmt_conduit if is operational */
- if (!priv->mgmt_conduit) {
- kfree_skb(skb);
- mutex_unlock(&mgmt_eth_data->mutex);
- return -EINVAL;
- }
-
- skb->dev = priv->mgmt_conduit;
-
- ret = dsa_inband_request(&mgmt_eth_data->inband, skb,
- qca8k_mdio_header_fill_seq_num,
- NULL, 0,
- QCA8K_ETHERNET_TIMEOUT);
-
- mutex_unlock(&mgmt_eth_data->mutex);
-
- return ret;
+ return dsa_inband_request(&mgmt_eth_data->inband, skb,
+ qca8k_mdio_header_fill_seq_num,
+ NULL, 0,
+ QCA8K_ETHERNET_TIMEOUT);
}
static int
@@ -484,7 +458,7 @@ qca8k_bulk_read(void *ctx, const void *reg_buf, size_t reg_len,
struct qca8k_priv *priv = ctx;
u32 reg = *(u16 *)reg_buf;
- if (priv->mgmt_conduit &&
+ if (READ_ONCE(priv->mgmt_conduit) &&
!qca8k_read_eth(priv, reg, val_buf, val_len))
return 0;
@@ -507,7 +481,7 @@ qca8k_bulk_gather_write(void *ctx, const void *reg_buf, size_t reg_len,
u32 reg = *(u16 *)reg_buf;
u32 *val = (u32 *)val_buf;
- if (priv->mgmt_conduit &&
+ if (READ_ONCE(priv->mgmt_conduit) &&
!qca8k_write_eth(priv, reg, val, val_len))
return 0;
@@ -645,17 +619,7 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
* 3. Get the data if we are reading
* 4. Reset the mdio master (even with error)
*/
- mutex_lock(&mgmt_eth_data->mutex);
-
- /* Check if mgmt_conduit is operational */
- mgmt_conduit = priv->mgmt_conduit;
- if (!mgmt_conduit) {
- mutex_unlock(&mgmt_eth_data->mutex);
- mutex_unlock(&priv->bus->mdio_lock);
- ret = -EINVAL;
- goto err_mgmt_conduit;
- }
-
+ mgmt_conduit = READ_ONCE(priv->mgmt_conduit);
read_skb->dev = mgmt_conduit;
clear_skb->dev = mgmt_conduit;
write_skb->dev = mgmt_conduit;
@@ -701,14 +665,10 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
NULL, 0,
QCA8K_ETHERNET_TIMEOUT);
- mutex_unlock(&mgmt_eth_data->mutex);
mutex_unlock(&priv->bus->mdio_lock);
return ret;
- /* Error handling before lock */
-err_mgmt_conduit:
- kfree_skb(read_skb);
err_read_skb:
kfree_skb(clear_skb);
err_clear_skb:
@@ -1698,13 +1658,12 @@ qca8k_conduit_change(struct dsa_switch *ds, const struct net_device *conduit,
if (dp->index != 0)
return;
- mutex_lock(&priv->mgmt_eth_data.mutex);
mutex_lock(&priv->mib_eth_data.mutex);
- priv->mgmt_conduit = operational ? (struct net_device *)conduit : NULL;
+ WRITE_ONCE(priv->mgmt_conduit,
+ operational ? (struct net_device *)conduit : NULL);
mutex_unlock(&priv->mib_eth_data.mutex);
- mutex_unlock(&priv->mgmt_eth_data.mutex);
}
static int qca8k_connect_tag_protocol(struct dsa_switch *ds,
@@ -2032,7 +1991,6 @@ qca8k_sw_probe(struct mdio_device *mdiodev)
if (!priv->ds)
return -ENOMEM;
- mutex_init(&priv->mgmt_eth_data.mutex);
dsa_inband_init(&priv->mgmt_eth_data.inband, U32_MAX);
mutex_init(&priv->mib_eth_data.mutex);
diff --git a/drivers/net/dsa/qca/qca8k-common.c b/drivers/net/dsa/qca/qca8k-common.c
index 13005f10edb7d..def2c44042d7b 100644
--- a/drivers/net/dsa/qca/qca8k-common.c
+++ b/drivers/net/dsa/qca/qca8k-common.c
@@ -499,7 +499,7 @@ void qca8k_get_ethtool_stats(struct dsa_switch *ds, int port,
u32 hi = 0;
int ret;
- if (priv->mgmt_conduit && priv->info->ops->autocast_mib &&
+ if (READ_ONCE(priv->mgmt_conduit) && priv->info->ops->autocast_mib &&
priv->info->ops->autocast_mib(ds, port, data) > 0)
return;
diff --git a/drivers/net/dsa/qca/qca8k.h b/drivers/net/dsa/qca/qca8k.h
index db89025b4243a..ccf92c85ccb14 100644
--- a/drivers/net/dsa/qca/qca8k.h
+++ b/drivers/net/dsa/qca/qca8k.h
@@ -393,7 +393,6 @@ enum {
struct qca8k_mgmt_eth_data {
struct dsa_inband inband;
- struct mutex mutex; /* Enforce one mdio read/write at time */
};
struct qca8k_mib_eth_data {
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 50ac6f0aa2e67..af16347a3d331 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -1355,8 +1355,12 @@ int dsa_port_simple_hsr_leave(struct dsa_switch *ds, int port,
* If there is a thread waiting for the response, resp will point to a
* buffer to copy the response to. If the thread has given up waiting,
* resp will be a NULL pointer.
+ *
+ * The lock is used to serialise all inband operations. It also protects
+ * the seqno, which is incremented while holding the lock.
*/
struct dsa_inband {
+ struct mutex lock; /* Serialise operations */
struct completion completion;
u32 seqno;
u32 seqno_mask;
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 3e480770854ca..2a8d47eb58e13 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -1837,6 +1837,7 @@ EXPORT_SYMBOL_GPL(dsa_port_simple_hsr_leave);
void dsa_inband_init(struct dsa_inband *inband, u32 seqno_mask)
{
init_completion(&inband->completion);
+ mutex_init(&inband->lock);
spin_lock_init(&inband->resp_lock);
inband->seqno_mask = seqno_mask;
inband->seqno = 0;
@@ -1898,6 +1899,13 @@ int dsa_inband_request(struct dsa_inband *inband, struct sk_buff *skb,
unsigned long jiffies = msecs_to_jiffies(timeout_ms);
int ret;
+ if (!skb->dev) {
+ kfree_skb(skb);
+ return -EOPNOTSUPP;
+ }
+
+ mutex_lock(&inband->lock);
+
inband->err = 0;
spin_lock_bh(&inband->resp_lock);
@@ -1920,6 +1928,7 @@ int dsa_inband_request(struct dsa_inband *inband, struct sk_buff *skb,
inband->resp = NULL;
inband->resp_len = 0;
spin_unlock_bh(&inband->resp_lock);
+ mutex_unlock(&inband->lock);
if (ret == 0)
return -ETIMEDOUT;
--
2.43.0
^ permalink raw reply related
* [PATCH net-next 08/12] net: dsa: qca8k: Update error handling
From: Luke Howard @ 2026-07-03 7:30 UTC (permalink / raw)
To: Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Vivien Didelot, Gregory CLEMENT, Andrew Lunn,
Simon Horman, Florian Fainelli, Tobias Waldekranz
Cc: Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
Christoph Mellauner, Simon Gapp, Ryan Wilkins, Mattias Forsblad,
netdev, linux-kernel, Luke Howard
In-Reply-To: <20260703-net-next-dsa-rmu-v1-0-a03febf20bb4@padl.com>
From: Andrew Lunn <andrew@lunn.ch>
Now that dsa_inband_request() can return positive values for the
length of the received reply, the return value checking needs to
change. This driver only uses short messages, and it is unlikely the
hardware will actually receive messages of less then 64 bytes. So
don't check the message length. However, ensure the return value is
either a negative error code, or 0. Ensure all users of
dsa_inband_request() return either a negative error, or 0 on success,
so as not to change the API.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/dsa/qca/qca8k-8xxx.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index ffa3b1ba23bed..94332497902c2 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -341,6 +341,7 @@ static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len)
QCA8K_ETHERNET_TIMEOUT);
if (ret < 0)
goto out;
+ ret = 0;
*val = data[0];
if (len > QCA_HDR_MGMT_DATA1_LEN)
@@ -574,7 +575,7 @@ qca8k_phy_eth_busy_wait(struct qca8k_mgmt_eth_data *mgmt_eth_data,
data, sizeof(data),
QCA8K_ETHERNET_TIMEOUT);
- if (ret)
+ if (ret < 0)
return ret;
*val = data[0];
@@ -664,7 +665,7 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
NULL, 0,
QCA8K_ETHERNET_TIMEOUT);
- if (ret) {
+ if (ret < 0) {
kfree_skb(read_skb);
goto exit;
}
@@ -685,7 +686,7 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
resp_data, sizeof(resp_data),
QCA8K_ETHERNET_TIMEOUT);
- if (ret)
+ if (ret < 0)
goto exit;
ret = resp_data[0] & QCA8K_MDIO_MASTER_DATA_MASK;
--
2.43.0
^ permalink raw reply related
* [PATCH net-next 07/12] net: dsa: qca8k: Pass error code from reply decoder to requester
From: Luke Howard @ 2026-07-03 7:30 UTC (permalink / raw)
To: Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Vivien Didelot, Gregory CLEMENT, Andrew Lunn,
Simon Horman, Florian Fainelli, Tobias Waldekranz
Cc: Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
Christoph Mellauner, Simon Gapp, Ryan Wilkins, Mattias Forsblad,
netdev, linux-kernel, Luke Howard
In-Reply-To: <20260703-net-next-dsa-rmu-v1-0-a03febf20bb4@padl.com>
From: Andrew Lunn <andrew@lunn.ch>
The code which decodes the frame and signals the complete can detect
error within the reply, such as fields have unexpected values. Pass an
error code between the completer and the function waiting on the
complete. This simplifies the error handling, since all errors are
combined into one place.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
v2:
Remove EPROTO if the sequence numbers don't match, drop the reply
---
drivers/net/dsa/qca/qca8k-8xxx.c | 70 +++++++++++++---------------------------
drivers/net/dsa/qca/qca8k.h | 2 --
include/net/dsa.h | 13 +++++++-
net/dsa/dsa.c | 33 +++++++++++++++++--
4 files changed, 65 insertions(+), 53 deletions(-)
diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index 499cc8ef5ddea..ffa3b1ba23bed 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -167,6 +167,8 @@ static void qca8k_rw_reg_ack_handler(struct dsa_switch *ds, struct sk_buff *skb)
struct qca_mgmt_ethhdr *mgmt_ethhdr;
u32 command;
u8 len, cmd;
+ u32 data[4];
+ int err = 0;
int i;
mgmt_ethhdr = (struct qca_mgmt_ethhdr *)skb_mac_header(skb);
@@ -191,7 +193,7 @@ static void qca8k_rw_reg_ack_handler(struct dsa_switch *ds, struct sk_buff *skb)
return;
if (cmd == MDIO_READ) {
- u32 *val = mgmt_eth_data->data;
+ u32 *val = &data[0];
*val = get_unaligned_le32(&mgmt_ethhdr->mdio_data);
@@ -213,7 +215,7 @@ static void qca8k_rw_reg_ack_handler(struct dsa_switch *ds, struct sk_buff *skb)
}
}
- dsa_inband_complete(&mgmt_eth_data->inband);
+ dsa_inband_complete(&mgmt_eth_data->inband, &data, sizeof(data), err);
}
static struct sk_buff *qca8k_alloc_mdio_header(enum mdio_cmd cmd, u32 reg, u32 *val,
@@ -314,7 +316,7 @@ static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len)
{
struct qca8k_mgmt_eth_data *mgmt_eth_data = &priv->mgmt_eth_data;
struct sk_buff *skb;
- int err;
+ u32 data[4];
int ret;
skb = qca8k_alloc_mdio_header(MDIO_READ, reg, NULL,
@@ -335,30 +337,25 @@ static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len)
ret = dsa_inband_request(&mgmt_eth_data->inband, skb,
qca8k_mdio_header_fill_seq_num,
+ data, sizeof(data),
QCA8K_ETHERNET_TIMEOUT);
+ if (ret < 0)
+ goto out;
- *val = mgmt_eth_data->data[0];
+ *val = data[0];
if (len > QCA_HDR_MGMT_DATA1_LEN)
- memcpy(val + 1, mgmt_eth_data->data + 1, len - QCA_HDR_MGMT_DATA1_LEN);
-
- err = mgmt_eth_data->err;
+ memcpy(val + 1, &data[1], len - QCA_HDR_MGMT_DATA1_LEN);
+out:
mutex_unlock(&mgmt_eth_data->mutex);
- if (ret)
- return ret;
-
- if (err)
- return err;
-
- return 0;
+ return ret;
}
static int qca8k_write_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len)
{
struct qca8k_mgmt_eth_data *mgmt_eth_data = &priv->mgmt_eth_data;
struct sk_buff *skb;
- int err;
int ret;
skb = qca8k_alloc_mdio_header(MDIO_WRITE, reg, val,
@@ -379,19 +376,12 @@ static int qca8k_write_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len)
ret = dsa_inband_request(&mgmt_eth_data->inband, skb,
qca8k_mdio_header_fill_seq_num,
+ NULL, 0,
QCA8K_ETHERNET_TIMEOUT);
- err = mgmt_eth_data->err;
-
mutex_unlock(&mgmt_eth_data->mutex);
- if (ret)
- return ret;
-
- if (err)
- return err;
-
- return 0;
+ return ret;
}
static int
@@ -573,7 +563,7 @@ qca8k_phy_eth_busy_wait(struct qca8k_mgmt_eth_data *mgmt_eth_data,
struct sk_buff *read_skb, u32 *val)
{
struct sk_buff *skb = skb_copy(read_skb, GFP_KERNEL);
- int err;
+ u32 data[4];
int ret;
if (!skb)
@@ -581,17 +571,13 @@ qca8k_phy_eth_busy_wait(struct qca8k_mgmt_eth_data *mgmt_eth_data,
ret = dsa_inband_request(&mgmt_eth_data->inband, skb,
qca8k_mdio_header_fill_seq_num,
+ data, sizeof(data),
QCA8K_ETHERNET_TIMEOUT);
- err = mgmt_eth_data->err;
-
if (ret)
return ret;
- if (err)
- return err;
-
- *val = mgmt_eth_data->data[0];
+ *val = data[0];
return 0;
}
@@ -604,8 +590,8 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
struct qca8k_mgmt_eth_data *mgmt_eth_data;
u32 write_val, clear_val = 0, val;
struct net_device *mgmt_conduit;
+ u32 resp_data[4];
int ret, ret1;
- int err;
if (regnum >= QCA8K_MDIO_MASTER_MAX_REG)
return -EINVAL;
@@ -675,21 +661,14 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
ret = dsa_inband_request(&mgmt_eth_data->inband, write_skb,
qca8k_mdio_header_fill_seq_num,
+ NULL, 0,
QCA8K_ETHERNET_TIMEOUT);
- err = mgmt_eth_data->err;
-
if (ret) {
kfree_skb(read_skb);
goto exit;
}
- if (err) {
- ret = err;
- kfree_skb(read_skb);
- goto exit;
- }
-
ret = read_poll_timeout(qca8k_phy_eth_busy_wait, ret1,
!(val & QCA8K_MDIO_MASTER_BUSY), 0,
QCA8K_BUSY_WAIT_TIMEOUT * USEC_PER_MSEC, false,
@@ -703,19 +682,13 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
if (read) {
ret = dsa_inband_request(&mgmt_eth_data->inband, read_skb,
qca8k_mdio_header_fill_seq_num,
+ resp_data, sizeof(resp_data),
QCA8K_ETHERNET_TIMEOUT);
- err = mgmt_eth_data->err;
-
if (ret)
goto exit;
- if (err) {
- ret = err;
- goto exit;
- }
-
- ret = mgmt_eth_data->data[0] & QCA8K_MDIO_MASTER_DATA_MASK;
+ ret = resp_data[0] & QCA8K_MDIO_MASTER_DATA_MASK;
} else {
kfree_skb(read_skb);
}
@@ -724,6 +697,7 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
/* This is expected to fail sometimes, so don't check return value. */
dsa_inband_request(&mgmt_eth_data->inband, clear_skb,
qca8k_mdio_header_fill_seq_num,
+ NULL, 0,
QCA8K_ETHERNET_TIMEOUT);
mutex_unlock(&mgmt_eth_data->mutex);
diff --git a/drivers/net/dsa/qca/qca8k.h b/drivers/net/dsa/qca/qca8k.h
index 4c055aef674c2..db89025b4243a 100644
--- a/drivers/net/dsa/qca/qca8k.h
+++ b/drivers/net/dsa/qca/qca8k.h
@@ -394,8 +394,6 @@ enum {
struct qca8k_mgmt_eth_data {
struct dsa_inband inband;
struct mutex mutex; /* Enforce one mdio read/write at time */
- int err;
- u32 data[4];
};
struct qca8k_mib_eth_data {
diff --git a/include/net/dsa.h b/include/net/dsa.h
index cdf2487397c2b..50ac6f0aa2e67 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -1350,17 +1350,28 @@ int dsa_port_simple_hsr_leave(struct dsa_switch *ds, int port,
/* Perform operations on a switch by sending it request in Ethernet
* frames and expecting a response in a frame.
+ *
+ * resp_lock protects resp and resp_len to ensure they are consistent.
+ * If there is a thread waiting for the response, resp will point to a
+ * buffer to copy the response to. If the thread has given up waiting,
+ * resp will be a NULL pointer.
*/
struct dsa_inband {
struct completion completion;
u32 seqno;
u32 seqno_mask;
+ int err;
+ spinlock_t resp_lock;
+ void *resp;
+ unsigned int resp_len;
};
void dsa_inband_init(struct dsa_inband *inband, u32 seqno_mask);
-void dsa_inband_complete(struct dsa_inband *inband);
+void dsa_inband_complete(struct dsa_inband *inband,
+ void *resp, unsigned int resp_len, int err);
int dsa_inband_request(struct dsa_inband *inband, struct sk_buff *skb,
void (*insert_seqno)(struct sk_buff *skb, u32 seqno),
+ void *resp, unsigned int resp_len,
int timeout_ms);
int dsa_inband_wait_for_completion(struct dsa_inband *inband, int timeout_ms);
u32 dsa_inband_seqno(struct dsa_inband *inband);
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 98a75ee08f011..3e480770854ca 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -1837,13 +1837,27 @@ EXPORT_SYMBOL_GPL(dsa_port_simple_hsr_leave);
void dsa_inband_init(struct dsa_inband *inband, u32 seqno_mask)
{
init_completion(&inband->completion);
+ spin_lock_init(&inband->resp_lock);
inband->seqno_mask = seqno_mask;
inband->seqno = 0;
}
EXPORT_SYMBOL_GPL(dsa_inband_init);
-void dsa_inband_complete(struct dsa_inband *inband)
+void dsa_inband_complete(struct dsa_inband *inband,
+ void *resp, unsigned int resp_len,
+ int err)
{
+ inband->err = err;
+
+ if (!err) {
+ spin_lock_bh(&inband->resp_lock);
+ resp_len = min(inband->resp_len, resp_len);
+ if (inband->resp && resp)
+ memcpy(inband->resp, resp, resp_len);
+ spin_unlock_bh(&inband->resp_lock);
+ inband->err = resp_len;
+ }
+
complete(&inband->completion);
}
EXPORT_SYMBOL_GPL(dsa_inband_complete);
@@ -1878,11 +1892,19 @@ EXPORT_SYMBOL_GPL(dsa_inband_wait_for_completion);
*/
int dsa_inband_request(struct dsa_inband *inband, struct sk_buff *skb,
void (*insert_seqno)(struct sk_buff *skb, u32 seqno),
+ void *resp, unsigned int resp_len,
int timeout_ms)
{
unsigned long jiffies = msecs_to_jiffies(timeout_ms);
int ret;
+ inband->err = 0;
+
+ spin_lock_bh(&inband->resp_lock);
+ inband->resp = resp;
+ inband->resp_len = resp_len;
+ spin_unlock_bh(&inband->resp_lock);
+
if (insert_seqno) {
WRITE_ONCE(inband->seqno, inband->seqno + 1);
insert_seqno(skb, inband->seqno & inband->seqno_mask);
@@ -1893,9 +1915,16 @@ int dsa_inband_request(struct dsa_inband *inband, struct sk_buff *skb,
dev_queue_xmit(skb);
ret = wait_for_completion_timeout(&inband->completion, jiffies);
+
+ spin_lock_bh(&inband->resp_lock);
+ inband->resp = NULL;
+ inband->resp_len = 0;
+ spin_unlock_bh(&inband->resp_lock);
+
if (ret == 0)
return -ETIMEDOUT;
- return 0;
+
+ return inband->err;
}
EXPORT_SYMBOL_GPL(dsa_inband_request);
--
2.43.0
^ permalink raw reply related
* [PATCH net-next 06/12] net: dsa: qca8k: Refactor sequence number mismatch to use error code
From: Luke Howard @ 2026-07-03 7:30 UTC (permalink / raw)
To: Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Vivien Didelot, Gregory CLEMENT, Andrew Lunn,
Simon Horman, Florian Fainelli, Tobias Waldekranz
Cc: Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
Christoph Mellauner, Simon Gapp, Ryan Wilkins, Mattias Forsblad,
netdev, linux-kernel, Luke Howard
In-Reply-To: <20260703-net-next-dsa-rmu-v1-0-a03febf20bb4@padl.com>
From: Andrew Lunn <andrew@lunn.ch>
Replace the boolean that the sequence numbers matches with an error
code. Now that responses with the wrong sequence number are dropped,
this is actually unused in this driver. However, other devices can
perform additional validation of a response with the correct sequence
number and potentially return -EPROTO to indicate some other sort of
error.
The value is only safe to use if the completion happens. Ensure the
return from the completion is always considered, and if it fails, a
timeout error is returned.
This is a preparation step to moving the error tracking into the DSA
core. This intermediate step is a bit ugly, but that all gets cleaned
up in the next patch.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
v2
-ret -> err
Extended commit message warning the code is ugly
Point out it is not actually used by this driver
---
drivers/net/dsa/qca/qca8k-8xxx.c | 49 ++++++++++++++++------------------------
drivers/net/dsa/qca/qca8k.h | 2 +-
2 files changed, 20 insertions(+), 31 deletions(-)
diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index fc5070402188e..499cc8ef5ddea 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -314,7 +314,7 @@ static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len)
{
struct qca8k_mgmt_eth_data *mgmt_eth_data = &priv->mgmt_eth_data;
struct sk_buff *skb;
- bool ack;
+ int err;
int ret;
skb = qca8k_alloc_mdio_header(MDIO_READ, reg, NULL,
@@ -333,8 +333,6 @@ static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len)
skb->dev = priv->mgmt_conduit;
- mgmt_eth_data->ack = false;
-
ret = dsa_inband_request(&mgmt_eth_data->inband, skb,
qca8k_mdio_header_fill_seq_num,
QCA8K_ETHERNET_TIMEOUT);
@@ -343,15 +341,15 @@ static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len)
if (len > QCA_HDR_MGMT_DATA1_LEN)
memcpy(val + 1, mgmt_eth_data->data + 1, len - QCA_HDR_MGMT_DATA1_LEN);
- ack = mgmt_eth_data->ack;
+ err = mgmt_eth_data->err;
mutex_unlock(&mgmt_eth_data->mutex);
if (ret)
return ret;
- if (!ack)
- return -EINVAL;
+ if (err)
+ return err;
return 0;
}
@@ -360,7 +358,7 @@ static int qca8k_write_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len)
{
struct qca8k_mgmt_eth_data *mgmt_eth_data = &priv->mgmt_eth_data;
struct sk_buff *skb;
- bool ack;
+ int err;
int ret;
skb = qca8k_alloc_mdio_header(MDIO_WRITE, reg, val,
@@ -379,21 +377,19 @@ static int qca8k_write_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len)
skb->dev = priv->mgmt_conduit;
- mgmt_eth_data->ack = false;
-
ret = dsa_inband_request(&mgmt_eth_data->inband, skb,
qca8k_mdio_header_fill_seq_num,
QCA8K_ETHERNET_TIMEOUT);
- ack = mgmt_eth_data->ack;
+ err = mgmt_eth_data->err;
mutex_unlock(&mgmt_eth_data->mutex);
if (ret)
return ret;
- if (!ack)
- return -EINVAL;
+ if (err)
+ return err;
return 0;
}
@@ -577,25 +573,23 @@ qca8k_phy_eth_busy_wait(struct qca8k_mgmt_eth_data *mgmt_eth_data,
struct sk_buff *read_skb, u32 *val)
{
struct sk_buff *skb = skb_copy(read_skb, GFP_KERNEL);
- bool ack;
+ int err;
int ret;
if (!skb)
return -ENOMEM;
- mgmt_eth_data->ack = false;
-
ret = dsa_inband_request(&mgmt_eth_data->inband, skb,
qca8k_mdio_header_fill_seq_num,
QCA8K_ETHERNET_TIMEOUT);
- ack = mgmt_eth_data->ack;
+ err = mgmt_eth_data->err;
if (ret)
return ret;
- if (!ack)
- return -EINVAL;
+ if (err)
+ return err;
*val = mgmt_eth_data->data[0];
@@ -611,7 +605,7 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
u32 write_val, clear_val = 0, val;
struct net_device *mgmt_conduit;
int ret, ret1;
- bool ack;
+ int err;
if (regnum >= QCA8K_MDIO_MASTER_MAX_REG)
return -EINVAL;
@@ -679,21 +673,19 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
clear_skb->dev = mgmt_conduit;
write_skb->dev = mgmt_conduit;
- mgmt_eth_data->ack = false;
-
ret = dsa_inband_request(&mgmt_eth_data->inband, write_skb,
qca8k_mdio_header_fill_seq_num,
QCA8K_ETHERNET_TIMEOUT);
- ack = mgmt_eth_data->ack;
+ err = mgmt_eth_data->err;
if (ret) {
kfree_skb(read_skb);
goto exit;
}
- if (!ack) {
- ret = -EINVAL;
+ if (err) {
+ ret = err;
kfree_skb(read_skb);
goto exit;
}
@@ -709,19 +701,17 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
}
if (read) {
- mgmt_eth_data->ack = false;
-
ret = dsa_inband_request(&mgmt_eth_data->inband, read_skb,
qca8k_mdio_header_fill_seq_num,
QCA8K_ETHERNET_TIMEOUT);
- ack = mgmt_eth_data->ack;
+ err = mgmt_eth_data->err;
if (ret)
goto exit;
- if (!ack) {
- ret = -EINVAL;
+ if (err) {
+ ret = err;
goto exit;
}
@@ -730,7 +720,6 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
kfree_skb(read_skb);
}
exit:
- mgmt_eth_data->ack = false;
/* This is expected to fail sometimes, so don't check return value. */
dsa_inband_request(&mgmt_eth_data->inband, clear_skb,
diff --git a/drivers/net/dsa/qca/qca8k.h b/drivers/net/dsa/qca/qca8k.h
index 1054ba1c7e590..4c055aef674c2 100644
--- a/drivers/net/dsa/qca/qca8k.h
+++ b/drivers/net/dsa/qca/qca8k.h
@@ -394,7 +394,7 @@ enum {
struct qca8k_mgmt_eth_data {
struct dsa_inband inband;
struct mutex mutex; /* Enforce one mdio read/write at time */
- bool ack;
+ int err;
u32 data[4];
};
--
2.43.0
^ permalink raw reply related
* [PATCH net-next 05/12] net: dsa: qca8k: Move request sequence number handling into core
From: Luke Howard @ 2026-07-03 7:30 UTC (permalink / raw)
To: Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Vivien Didelot, Gregory CLEMENT, Andrew Lunn,
Simon Horman, Florian Fainelli, Tobias Waldekranz
Cc: Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
Christoph Mellauner, Simon Gapp, Ryan Wilkins, Mattias Forsblad,
netdev, linux-kernel, Luke Howard
In-Reply-To: <20260703-net-next-dsa-rmu-v1-0-a03febf20bb4@padl.com>
From: Andrew Lunn <andrew@lunn.ch>
Each request/reply frame is likely to have a sequence number so that
request and the reply can be matched together. Move this sequence
number into the inband structure. The driver must provide a helper to
insert the sequence number into the skb, and the core will perform the
increment.
To allow different devices to have different size sequence numbers, a
mask is provided. This can be used for example to reduce the u32
sequence number down to a u8.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/dsa/qca/qca8k-8xxx.c | 29 +++++++++--------------------
drivers/net/dsa/qca/qca8k.h | 1 -
include/net/dsa.h | 6 +++++-
net/dsa/dsa.c | 16 +++++++++++++++-
4 files changed, 29 insertions(+), 23 deletions(-)
diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index 3b3fe96016176..fc5070402188e 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -186,7 +186,8 @@ static void qca8k_rw_reg_ack_handler(struct dsa_switch *ds, struct sk_buff *skb)
len *= sizeof(u16);
/* Make sure the seq match the requested packet. If not, drop. */
- if (get_unaligned_le32(&mgmt_ethhdr->seq) != mgmt_eth_data->seq)
+ if (get_unaligned_le32(&mgmt_ethhdr->seq) !=
+ dsa_inband_seqno(&mgmt_eth_data->inband))
return;
if (cmd == MDIO_READ) {
@@ -332,12 +333,10 @@ static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len)
skb->dev = priv->mgmt_conduit;
- /* Increment seq_num and set it in the mdio pkt */
- mgmt_eth_data->seq++;
- qca8k_mdio_header_fill_seq_num(skb, mgmt_eth_data->seq);
mgmt_eth_data->ack = false;
ret = dsa_inband_request(&mgmt_eth_data->inband, skb,
+ qca8k_mdio_header_fill_seq_num,
QCA8K_ETHERNET_TIMEOUT);
*val = mgmt_eth_data->data[0];
@@ -380,12 +379,10 @@ static int qca8k_write_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len)
skb->dev = priv->mgmt_conduit;
- /* Increment seq_num and set it in the mdio pkt */
- mgmt_eth_data->seq++;
- qca8k_mdio_header_fill_seq_num(skb, mgmt_eth_data->seq);
mgmt_eth_data->ack = false;
ret = dsa_inband_request(&mgmt_eth_data->inband, skb,
+ qca8k_mdio_header_fill_seq_num,
QCA8K_ETHERNET_TIMEOUT);
ack = mgmt_eth_data->ack;
@@ -586,12 +583,10 @@ qca8k_phy_eth_busy_wait(struct qca8k_mgmt_eth_data *mgmt_eth_data,
if (!skb)
return -ENOMEM;
- /* Increment seq_num and set it in the copy pkt */
- mgmt_eth_data->seq++;
- qca8k_mdio_header_fill_seq_num(skb, mgmt_eth_data->seq);
mgmt_eth_data->ack = false;
ret = dsa_inband_request(&mgmt_eth_data->inband, skb,
+ qca8k_mdio_header_fill_seq_num,
QCA8K_ETHERNET_TIMEOUT);
ack = mgmt_eth_data->ack;
@@ -684,12 +679,10 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
clear_skb->dev = mgmt_conduit;
write_skb->dev = mgmt_conduit;
- /* Increment seq_num and set it in the write pkt */
- mgmt_eth_data->seq++;
- qca8k_mdio_header_fill_seq_num(write_skb, mgmt_eth_data->seq);
mgmt_eth_data->ack = false;
ret = dsa_inband_request(&mgmt_eth_data->inband, write_skb,
+ qca8k_mdio_header_fill_seq_num,
QCA8K_ETHERNET_TIMEOUT);
ack = mgmt_eth_data->ack;
@@ -716,12 +709,10 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
}
if (read) {
- /* Increment seq_num and set it in the read pkt */
- mgmt_eth_data->seq++;
- qca8k_mdio_header_fill_seq_num(read_skb, mgmt_eth_data->seq);
mgmt_eth_data->ack = false;
ret = dsa_inband_request(&mgmt_eth_data->inband, read_skb,
+ qca8k_mdio_header_fill_seq_num,
QCA8K_ETHERNET_TIMEOUT);
ack = mgmt_eth_data->ack;
@@ -739,13 +730,11 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
kfree_skb(read_skb);
}
exit:
- /* Increment seq_num and set it in the clear pkt */
- mgmt_eth_data->seq++;
- qca8k_mdio_header_fill_seq_num(clear_skb, mgmt_eth_data->seq);
mgmt_eth_data->ack = false;
/* This is expected to fail sometimes, so don't check return value. */
dsa_inband_request(&mgmt_eth_data->inband, clear_skb,
+ qca8k_mdio_header_fill_seq_num,
QCA8K_ETHERNET_TIMEOUT);
mutex_unlock(&mgmt_eth_data->mutex);
@@ -2080,7 +2069,7 @@ qca8k_sw_probe(struct mdio_device *mdiodev)
return -ENOMEM;
mutex_init(&priv->mgmt_eth_data.mutex);
- dsa_inband_init(&priv->mgmt_eth_data.inband);
+ dsa_inband_init(&priv->mgmt_eth_data.inband, U32_MAX);
mutex_init(&priv->mib_eth_data.mutex);
init_completion(&priv->mib_eth_data.rw_done);
diff --git a/drivers/net/dsa/qca/qca8k.h b/drivers/net/dsa/qca/qca8k.h
index 938f2c9ff0cac..1054ba1c7e590 100644
--- a/drivers/net/dsa/qca/qca8k.h
+++ b/drivers/net/dsa/qca/qca8k.h
@@ -395,7 +395,6 @@ struct qca8k_mgmt_eth_data {
struct dsa_inband inband;
struct mutex mutex; /* Enforce one mdio read/write at time */
bool ack;
- u32 seq;
u32 data[4];
};
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 6b5aeb99ec3bb..cdf2487397c2b 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -1353,13 +1353,17 @@ int dsa_port_simple_hsr_leave(struct dsa_switch *ds, int port,
*/
struct dsa_inband {
struct completion completion;
+ u32 seqno;
+ u32 seqno_mask;
};
-void dsa_inband_init(struct dsa_inband *inband);
+void dsa_inband_init(struct dsa_inband *inband, u32 seqno_mask);
void dsa_inband_complete(struct dsa_inband *inband);
int dsa_inband_request(struct dsa_inband *inband, struct sk_buff *skb,
+ void (*insert_seqno)(struct sk_buff *skb, u32 seqno),
int timeout_ms);
int dsa_inband_wait_for_completion(struct dsa_inband *inband, int timeout_ms);
+u32 dsa_inband_seqno(struct dsa_inband *inband);
/* Keep inline for faster access in hot path */
static inline bool netdev_uses_dsa(const struct net_device *dev)
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index f6ae11b06b4ab..98a75ee08f011 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -1834,9 +1834,11 @@ int dsa_port_simple_hsr_leave(struct dsa_switch *ds, int port,
}
EXPORT_SYMBOL_GPL(dsa_port_simple_hsr_leave);
-void dsa_inband_init(struct dsa_inband *inband)
+void dsa_inband_init(struct dsa_inband *inband, u32 seqno_mask)
{
init_completion(&inband->completion);
+ inband->seqno_mask = seqno_mask;
+ inband->seqno = 0;
}
EXPORT_SYMBOL_GPL(dsa_inband_init);
@@ -1875,11 +1877,17 @@ EXPORT_SYMBOL_GPL(dsa_inband_wait_for_completion);
* be reinitialised before the skb is queued, to avoid races.
*/
int dsa_inband_request(struct dsa_inband *inband, struct sk_buff *skb,
+ void (*insert_seqno)(struct sk_buff *skb, u32 seqno),
int timeout_ms)
{
unsigned long jiffies = msecs_to_jiffies(timeout_ms);
int ret;
+ if (insert_seqno) {
+ WRITE_ONCE(inband->seqno, inband->seqno + 1);
+ insert_seqno(skb, inband->seqno & inband->seqno_mask);
+ }
+
reinit_completion(&inband->completion);
dev_queue_xmit(skb);
@@ -1891,6 +1899,12 @@ int dsa_inband_request(struct dsa_inband *inband, struct sk_buff *skb,
}
EXPORT_SYMBOL_GPL(dsa_inband_request);
+u32 dsa_inband_seqno(struct dsa_inband *inband)
+{
+ return READ_ONCE(inband->seqno) & inband->seqno_mask;
+}
+EXPORT_SYMBOL_GPL(dsa_inband_seqno);
+
static const struct dsa_stubs __dsa_stubs = {
.conduit_hwtstamp_validate = __dsa_conduit_hwtstamp_validate,
};
--
2.43.0
^ permalink raw reply related
* [PATCH net-next 04/12] net: dsa: qca8k: Drop replies with wrong sequence numbers
From: Luke Howard @ 2026-07-03 7:30 UTC (permalink / raw)
To: Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Vivien Didelot, Gregory CLEMENT, Andrew Lunn,
Simon Horman, Florian Fainelli, Tobias Waldekranz
Cc: Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
Christoph Mellauner, Simon Gapp, Ryan Wilkins, Mattias Forsblad,
netdev, linux-kernel, Luke Howard
In-Reply-To: <20260703-net-next-dsa-rmu-v1-0-a03febf20bb4@padl.com>
From: Andrew Lunn <andrew@lunn.ch>
A response with the wrong sequence number is likely to be a late
arriving responses, which the driver has already given up waiting for.
Drop it rather than signalling the complete. If the complete was
signalled, this late response could take the place of the genuine
reply which is soon to follow.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/dsa/qca/qca8k-8xxx.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index b0cbe72c15b4c..3b3fe96016176 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -185,9 +185,9 @@ static void qca8k_rw_reg_ack_handler(struct dsa_switch *ds, struct sk_buff *skb)
/* We can ignore odd value, we always round up them in the alloc function. */
len *= sizeof(u16);
- /* Make sure the seq match the requested packet */
- if (get_unaligned_le32(&mgmt_ethhdr->seq) == mgmt_eth_data->seq)
- mgmt_eth_data->ack = true;
+ /* Make sure the seq match the requested packet. If not, drop. */
+ if (get_unaligned_le32(&mgmt_ethhdr->seq) != mgmt_eth_data->seq)
+ return;
if (cmd == MDIO_READ) {
u32 *val = mgmt_eth_data->data;
--
2.43.0
^ permalink raw reply related
* [PATCH net-next 03/12] net: dsa: qca8k: dsa_inband_request: More normal return values
From: Luke Howard @ 2026-07-03 7:30 UTC (permalink / raw)
To: Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Vivien Didelot, Gregory CLEMENT, Andrew Lunn,
Simon Horman, Florian Fainelli, Tobias Waldekranz
Cc: Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
Christoph Mellauner, Simon Gapp, Ryan Wilkins, Mattias Forsblad,
netdev, linux-kernel, Luke Howard
In-Reply-To: <20260703-net-next-dsa-rmu-v1-0-a03febf20bb4@padl.com>
From: Andrew Lunn <andrew@lunn.ch>
wait_for_completion_timeout() has unusual return values. If it times
out, it returns 0, and on success it returns the number of remaining
jiffies for the timeout.
For the use case here, the remaining time is not needed. All that is
really interesting is, it succeeded and returns 0, or a timeout.
Massage the return value to fit this, and modify the callers to the
more usual pattern of ret < 0 is an error.
Sending the clear message is expected to fail, so don't check the
return value, and add a comment about this.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
v2
Remove check on the clear message.
wait_for_completion_timeout() does not return negative values
---
drivers/net/dsa/qca/qca8k-8xxx.c | 24 +++++++++++-------------
net/dsa/dsa.c | 6 +++++-
2 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index 4da6094552f1e..b0cbe72c15b4c 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -348,8 +348,8 @@ static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len)
mutex_unlock(&mgmt_eth_data->mutex);
- if (ret <= 0)
- return -ETIMEDOUT;
+ if (ret)
+ return ret;
if (!ack)
return -EINVAL;
@@ -392,8 +392,8 @@ static int qca8k_write_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len)
mutex_unlock(&mgmt_eth_data->mutex);
- if (ret <= 0)
- return -ETIMEDOUT;
+ if (ret)
+ return ret;
if (!ack)
return -EINVAL;
@@ -596,8 +596,8 @@ qca8k_phy_eth_busy_wait(struct qca8k_mgmt_eth_data *mgmt_eth_data,
ack = mgmt_eth_data->ack;
- if (ret <= 0)
- return -ETIMEDOUT;
+ if (ret)
+ return ret;
if (!ack)
return -EINVAL;
@@ -694,8 +694,7 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
ack = mgmt_eth_data->ack;
- if (ret <= 0) {
- ret = -ETIMEDOUT;
+ if (ret) {
kfree_skb(read_skb);
goto exit;
}
@@ -727,10 +726,8 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
ack = mgmt_eth_data->ack;
- if (ret <= 0) {
- ret = -ETIMEDOUT;
+ if (ret)
goto exit;
- }
if (!ack) {
ret = -EINVAL;
@@ -747,8 +744,9 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
qca8k_mdio_header_fill_seq_num(clear_skb, mgmt_eth_data->seq);
mgmt_eth_data->ack = false;
- dsa_inband_wait_for_completion(&mgmt_eth_data->inband,
- QCA8K_ETHERNET_TIMEOUT);
+ /* This is expected to fail sometimes, so don't check return value. */
+ dsa_inband_request(&mgmt_eth_data->inband, clear_skb,
+ QCA8K_ETHERNET_TIMEOUT);
mutex_unlock(&mgmt_eth_data->mutex);
mutex_unlock(&priv->bus->mdio_lock);
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 534e391fac7b5..f6ae11b06b4ab 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -1878,12 +1878,16 @@ int dsa_inband_request(struct dsa_inband *inband, struct sk_buff *skb,
int timeout_ms)
{
unsigned long jiffies = msecs_to_jiffies(timeout_ms);
+ int ret;
reinit_completion(&inband->completion);
dev_queue_xmit(skb);
- return wait_for_completion_timeout(&inband->completion, jiffies);
+ ret = wait_for_completion_timeout(&inband->completion, jiffies);
+ if (ret == 0)
+ return -ETIMEDOUT;
+ return 0;
}
EXPORT_SYMBOL_GPL(dsa_inband_request);
--
2.43.0
^ permalink raw reply related
* [PATCH net-next 02/12] net: dsa: qca8K: Move queuing for request frame into the core
From: Luke Howard @ 2026-07-03 7:30 UTC (permalink / raw)
To: Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Vivien Didelot, Gregory CLEMENT, Andrew Lunn,
Simon Horman, Florian Fainelli, Tobias Waldekranz
Cc: Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
Christoph Mellauner, Simon Gapp, Ryan Wilkins, Mattias Forsblad,
netdev, linux-kernel, Luke Howard
In-Reply-To: <20260703-net-next-dsa-rmu-v1-0-a03febf20bb4@padl.com>
From: Andrew Lunn <andrew@lunn.ch>
Combine the queuing of the request and waiting for the completion into
one core helper. Add the function dsa_rmu_request() to perform this.
Access to statistics is not a strict request/reply, so the
dsa_rmu_wait_for_completion needs to be kept.
It is also no possible to combine dsa_rmu_request() and
dsa_rmu_wait_for_completion() since we need to avoid the race of
sending the request, receiving a reply, and the completion has not
been reinitialised because the schedule at decided to do other things.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/dsa/qca/qca8k-8xxx.c | 32 ++++++++++----------------------
include/net/dsa.h | 2 ++
net/dsa/dsa.c | 31 +++++++++++++++++++++++++++++++
3 files changed, 43 insertions(+), 22 deletions(-)
diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index fe6fb69b6f92f..4da6094552f1e 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -337,10 +337,8 @@ static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len)
qca8k_mdio_header_fill_seq_num(skb, mgmt_eth_data->seq);
mgmt_eth_data->ack = false;
- dev_queue_xmit(skb);
-
- ret = dsa_inband_wait_for_completion(&mgmt_eth_data->inband,
- QCA8K_ETHERNET_TIMEOUT);
+ ret = dsa_inband_request(&mgmt_eth_data->inband, skb,
+ QCA8K_ETHERNET_TIMEOUT);
*val = mgmt_eth_data->data[0];
if (len > QCA_HDR_MGMT_DATA1_LEN)
@@ -387,10 +385,8 @@ static int qca8k_write_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len)
qca8k_mdio_header_fill_seq_num(skb, mgmt_eth_data->seq);
mgmt_eth_data->ack = false;
- dev_queue_xmit(skb);
-
- ret = dsa_inband_wait_for_completion(&mgmt_eth_data->inband,
- QCA8K_ETHERNET_TIMEOUT);
+ ret = dsa_inband_request(&mgmt_eth_data->inband, skb,
+ QCA8K_ETHERNET_TIMEOUT);
ack = mgmt_eth_data->ack;
@@ -595,10 +591,8 @@ qca8k_phy_eth_busy_wait(struct qca8k_mgmt_eth_data *mgmt_eth_data,
qca8k_mdio_header_fill_seq_num(skb, mgmt_eth_data->seq);
mgmt_eth_data->ack = false;
- dev_queue_xmit(skb);
-
- ret = dsa_inband_wait_for_completion(&mgmt_eth_data->inband,
- QCA8K_ETHERNET_TIMEOUT);
+ ret = dsa_inband_request(&mgmt_eth_data->inband, skb,
+ QCA8K_ETHERNET_TIMEOUT);
ack = mgmt_eth_data->ack;
@@ -695,10 +689,8 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
qca8k_mdio_header_fill_seq_num(write_skb, mgmt_eth_data->seq);
mgmt_eth_data->ack = false;
- dev_queue_xmit(write_skb);
-
- ret = dsa_inband_wait_for_completion(&mgmt_eth_data->inband,
- QCA8K_ETHERNET_TIMEOUT);
+ ret = dsa_inband_request(&mgmt_eth_data->inband, write_skb,
+ QCA8K_ETHERNET_TIMEOUT);
ack = mgmt_eth_data->ack;
@@ -730,10 +722,8 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
qca8k_mdio_header_fill_seq_num(read_skb, mgmt_eth_data->seq);
mgmt_eth_data->ack = false;
- dev_queue_xmit(read_skb);
-
- ret = dsa_inband_wait_for_completion(&mgmt_eth_data->inband,
- QCA8K_ETHERNET_TIMEOUT);
+ ret = dsa_inband_request(&mgmt_eth_data->inband, read_skb,
+ QCA8K_ETHERNET_TIMEOUT);
ack = mgmt_eth_data->ack;
@@ -757,8 +747,6 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
qca8k_mdio_header_fill_seq_num(clear_skb, mgmt_eth_data->seq);
mgmt_eth_data->ack = false;
- dev_queue_xmit(clear_skb);
-
dsa_inband_wait_for_completion(&mgmt_eth_data->inband,
QCA8K_ETHERNET_TIMEOUT);
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 9b0c109b3058a..6b5aeb99ec3bb 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -1357,6 +1357,8 @@ struct dsa_inband {
void dsa_inband_init(struct dsa_inband *inband);
void dsa_inband_complete(struct dsa_inband *inband);
+int dsa_inband_request(struct dsa_inband *inband, struct sk_buff *skb,
+ int timeout_ms);
int dsa_inband_wait_for_completion(struct dsa_inband *inband, int timeout_ms);
/* Keep inline for faster access in hot path */
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index dc4e5cdb2f5b2..534e391fac7b5 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -1856,6 +1856,37 @@ int dsa_inband_wait_for_completion(struct dsa_inband *inband, int timeout_ms)
}
EXPORT_SYMBOL_GPL(dsa_inband_wait_for_completion);
+/* dsa_inband_request - send an inband request frame and wait for the reply.
+ * @inband: inband state for the switch
+ * @skb: request frame; ownership is transferred to this function
+ * @insert_seqno: optional callback to stamp the sequence number into @skb
+ * @resp: buffer to receive the reply payload, or NULL if none is expected
+ * @resp_len: size of @resp in bytes
+ * @timeout_ms: how long to wait for the reply, in milliseconds
+ *
+ * Serialise against other inband operations, transmit @skb and wait for the
+ * matching reply handed back via dsa_inband_complete().
+ *
+ * Return the number of response bytes copied into @resp (0 when no response
+ * is expected) on success, or a negative errno (-EOPNOTSUPP if the conduit
+ * is down, -ETIMEDOUT if no reply arrived, or an error from the completer).
+ *
+ * Cannot use dsa_inband_wait_for_completion() since the completion needs to
+ * be reinitialised before the skb is queued, to avoid races.
+ */
+int dsa_inband_request(struct dsa_inband *inband, struct sk_buff *skb,
+ int timeout_ms)
+{
+ unsigned long jiffies = msecs_to_jiffies(timeout_ms);
+
+ reinit_completion(&inband->completion);
+
+ dev_queue_xmit(skb);
+
+ return wait_for_completion_timeout(&inband->completion, jiffies);
+}
+EXPORT_SYMBOL_GPL(dsa_inband_request);
+
static const struct dsa_stubs __dsa_stubs = {
.conduit_hwtstamp_validate = __dsa_conduit_hwtstamp_validate,
};
--
2.43.0
^ permalink raw reply related
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