* [PATCH net 0/6] pull-request: can 2025-05-06
@ 2025-05-06 13:56 Marc Kleine-Budde
2025-05-06 13:56 ` [PATCH net 1/6] can: m_can: m_can_class_allocate_dev(): initialize spin lock on device probe Marc Kleine-Budde
` (5 more replies)
0 siblings, 6 replies; 14+ messages in thread
From: Marc Kleine-Budde @ 2025-05-06 13:56 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, linux-can, kernel
Hello netdev-team,
this is a pull request of 6 patches for net/main.
The first patch is by Antonios Salios and adds a missing
spin_lock_init() to the m_can driver.
The next 3 patches are by me and fix the unregistration order in the
mcp251xfd, rockchip_canfd and m_can driver.
The last patch is by Oliver Hartkopp and fixes RCU and BH
locking/handling in the CAN gw protocol.
regards,
Marc
---
The following changes since commit ebd297a2affadb6f6f4d2e5d975c1eda18ac762d:
Merge tag 'net-6.15-rc5' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net (2025-05-01 10:37:49 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git tags/linux-can-fixes-for-6.15-20250506
for you to fetch changes up to 511e64e13d8cc72853275832e3f372607466c18c:
can: gw: fix RCU/BH usage in cgw_create_job() (2025-05-06 15:55:36 +0200)
----------------------------------------------------------------
linux-can-fixes-for-6.15-20250506
----------------------------------------------------------------
Antonios Salios (1):
can: m_can: m_can_class_allocate_dev(): initialize spin lock on device probe
Kelsey Maes (1):
can: mcp251xfd: fix TDC setting for low data bit rates
Marc Kleine-Budde (4):
can: mcp251xfd: mcp251xfd_remove(): fix order of unregistration calls
can: rockchip_canfd: rkcanfd_remove(): fix order of unregistration calls
can: mcan: m_can_class_unregister(): fix order of unregistration calls
Merge patch series "can: rx-offload: fix order of unregistration calls"
Oliver Hartkopp (1):
can: gw: fix RCU/BH usage in cgw_create_job()
drivers/net/can/m_can/m_can.c | 3 +-
drivers/net/can/rockchip/rockchip_canfd-core.c | 2 +-
drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 42 +++++--
net/can/gw.c | 151 +++++++++++++++----------
4 files changed, 127 insertions(+), 71 deletions(-)
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH net 1/6] can: m_can: m_can_class_allocate_dev(): initialize spin lock on device probe
2025-05-06 13:56 [PATCH net 0/6] pull-request: can 2025-05-06 Marc Kleine-Budde
@ 2025-05-06 13:56 ` Marc Kleine-Budde
2025-05-06 18:25 ` Jacob Keller
2025-05-07 2:10 ` patchwork-bot+netdevbpf
2025-05-06 13:56 ` [PATCH net 2/6] can: mcp251xfd: fix TDC setting for low data bit rates Marc Kleine-Budde
` (4 subsequent siblings)
5 siblings, 2 replies; 14+ messages in thread
From: Marc Kleine-Budde @ 2025-05-06 13:56 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, linux-can, kernel, Antonios Salios, Vincent Mailhol,
Markus Schneider-Pargmann, Marc Kleine-Budde
From: Antonios Salios <antonios@mwa.re>
The spin lock tx_handling_spinlock in struct m_can_classdev is not
being initialized. This leads the following spinlock bad magic
complaint from the kernel, eg. when trying to send CAN frames with
cansend from can-utils:
| BUG: spinlock bad magic on CPU#0, cansend/95
| lock: 0xff60000002ec1010, .magic: 00000000, .owner: <none>/-1, .owner_cpu: 0
| CPU: 0 UID: 0 PID: 95 Comm: cansend Not tainted 6.15.0-rc3-00032-ga79be02bba5c #5 NONE
| Hardware name: MachineWare SIM-V (DT)
| Call Trace:
| [<ffffffff800133e0>] dump_backtrace+0x1c/0x24
| [<ffffffff800022f2>] show_stack+0x28/0x34
| [<ffffffff8000de3e>] dump_stack_lvl+0x4a/0x68
| [<ffffffff8000de70>] dump_stack+0x14/0x1c
| [<ffffffff80003134>] spin_dump+0x62/0x6e
| [<ffffffff800883ba>] do_raw_spin_lock+0xd0/0x142
| [<ffffffff807a6fcc>] _raw_spin_lock_irqsave+0x20/0x2c
| [<ffffffff80536dba>] m_can_start_xmit+0x90/0x34a
| [<ffffffff806148b0>] dev_hard_start_xmit+0xa6/0xee
| [<ffffffff8065b730>] sch_direct_xmit+0x114/0x292
| [<ffffffff80614e2a>] __dev_queue_xmit+0x3b0/0xaa8
| [<ffffffff8073b8fa>] can_send+0xc6/0x242
| [<ffffffff8073d1c0>] raw_sendmsg+0x1a8/0x36c
| [<ffffffff805ebf06>] sock_write_iter+0x9a/0xee
| [<ffffffff801d06ea>] vfs_write+0x184/0x3a6
| [<ffffffff801d0a88>] ksys_write+0xa0/0xc0
| [<ffffffff801d0abc>] __riscv_sys_write+0x14/0x1c
| [<ffffffff8079ebf8>] do_trap_ecall_u+0x168/0x212
| [<ffffffff807a830a>] handle_exception+0x146/0x152
Initializing the spin lock in m_can_class_allocate_dev solves that
problem.
Fixes: 1fa80e23c150 ("can: m_can: Introduce a tx_fifo_in_flight counter")
Signed-off-by: Antonios Salios <antonios@mwa.re>
Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Link: https://patch.msgid.link/20250425111744.37604-2-antonios@mwa.re
Reviewed-by: Markus Schneider-Pargmann <msp@baylibre.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/m_can/m_can.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 884a6352c42b..326ede9d400f 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -2379,6 +2379,7 @@ struct m_can_classdev *m_can_class_allocate_dev(struct device *dev,
SET_NETDEV_DEV(net_dev, dev);
m_can_of_parse_mram(class_dev, mram_config_vals);
+ spin_lock_init(&class_dev->tx_handling_spinlock);
out:
return class_dev;
}
base-commit: ebd297a2affadb6f6f4d2e5d975c1eda18ac762d
--
2.47.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net 2/6] can: mcp251xfd: fix TDC setting for low data bit rates
2025-05-06 13:56 [PATCH net 0/6] pull-request: can 2025-05-06 Marc Kleine-Budde
2025-05-06 13:56 ` [PATCH net 1/6] can: m_can: m_can_class_allocate_dev(): initialize spin lock on device probe Marc Kleine-Budde
@ 2025-05-06 13:56 ` Marc Kleine-Budde
2025-05-06 18:27 ` Jacob Keller
2025-05-06 13:56 ` [PATCH net 3/6] can: mcp251xfd: mcp251xfd_remove(): fix order of unregistration calls Marc Kleine-Budde
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Marc Kleine-Budde @ 2025-05-06 13:56 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, linux-can, kernel, Kelsey Maes, Vincent Mailhol,
Marc Kleine-Budde
From: Kelsey Maes <kelsey@vpprocess.com>
The TDC is currently hardcoded enabled. This means that even for lower
CAN-FD data bitrates (with a DBRP (data bitrate prescaler) > 2) a TDC
is configured. This leads to a bus-off condition.
ISO 11898-1 section 11.3.3 says "Transmitter delay compensation" (TDC)
is only applicable if DBRP is 1 or 2.
To fix the problem, switch the driver to use the TDC calculation
provided by the CAN driver framework (which respects ISO 11898-1
section 11.3.3). This has the positive side effect that userspace can
control TDC as needed.
Demonstration of the feature in action:
| $ ip link set can0 up type can bitrate 125000 dbitrate 500000 fd on
| $ ip -details link show can0
| 3: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 72 qdisc pfifo_fast state UP mode DEFAULT group default qlen 10
| link/can promiscuity 0 allmulti 0 minmtu 0 maxmtu 0
| can <FD> state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 0
| bitrate 125000 sample-point 0.875
| tq 50 prop-seg 69 phase-seg1 70 phase-seg2 20 sjw 10 brp 2
| mcp251xfd: tseg1 2..256 tseg2 1..128 sjw 1..128 brp 1..256 brp_inc 1
| dbitrate 500000 dsample-point 0.875
| dtq 125 dprop-seg 6 dphase-seg1 7 dphase-seg2 2 dsjw 1 dbrp 5
| mcp251xfd: dtseg1 1..32 dtseg2 1..16 dsjw 1..16 dbrp 1..256 dbrp_inc 1
| tdcv 0..63 tdco 0..63
| clock 40000000 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535 tso_max_size 65536 tso_max_segs 65535 gro_max_size 65536 parentbus spi parentdev spi0.0
| $ ip link set can0 up type can bitrate 1000000 dbitrate 4000000 fd on
| $ ip -details link show can0
| 3: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 72 qdisc pfifo_fast state UP mode DEFAULT group default qlen 10
| link/can promiscuity 0 allmulti 0 minmtu 0 maxmtu 0
| can <FD,TDC-AUTO> state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 0
| bitrate 1000000 sample-point 0.750
| tq 25 prop-seg 14 phase-seg1 15 phase-seg2 10 sjw 5 brp 1
| mcp251xfd: tseg1 2..256 tseg2 1..128 sjw 1..128 brp 1..256 brp_inc 1
| dbitrate 4000000 dsample-point 0.700
| dtq 25 dprop-seg 3 dphase-seg1 3 dphase-seg2 3 dsjw 1 dbrp 1
| tdco 7
| mcp251xfd: dtseg1 1..32 dtseg2 1..16 dsjw 1..16 dbrp 1..256 dbrp_inc 1
| tdcv 0..63 tdco 0..63
| clock 40000000 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535 tso_max_size 65536 tso_max_segs 65535 gro_max_size 65536 parentbus spi parentdev spi0.0
There has been some confusion about the MCP2518FD using a relative or
absolute TDCO due to the datasheet specifying a range of [-64,63]. I
have a custom board with a 40 MHz clock and an estimated loop delay of
100 to 216 ns. During testing at a data bit rate of 4 Mbit/s I found
that using can_get_relative_tdco() resulted in bus-off errors. The
final TDCO value was 1 which corresponds to a 10% SSP in an absolute
configuration. This behavior is expected if the TDCO value is really
absolute and not relative. Using priv->can.tdc.tdco instead results in
a final TDCO of 8, setting the SSP at exactly 80%. This configuration
works.
The automatic, manual, and off TDC modes were tested at speeds up to,
and including, 8 Mbit/s on real hardware and behave as expected.
Fixes: 55e5b97f003e ("can: mcp25xxfd: add driver for Microchip MCP25xxFD SPI CAN")
Reported-by: Kelsey Maes <kelsey@vpprocess.com>
Closes: https://lore.kernel.org/all/C2121586-C87F-4B23-A933-845362C29CA1@vpprocess.com
Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Kelsey Maes <kelsey@vpprocess.com>
Link: https://patch.msgid.link/20250430161501.79370-1-kelsey@vpprocess.com
[mkl: add comment]
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
.../net/can/spi/mcp251xfd/mcp251xfd-core.c | 40 +++++++++++++++----
1 file changed, 32 insertions(+), 8 deletions(-)
diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
index 3bc56517fe7a..064d81c724f4 100644
--- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
+++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
@@ -75,6 +75,24 @@ static const struct can_bittiming_const mcp251xfd_data_bittiming_const = {
.brp_inc = 1,
};
+/* The datasheet of the mcp2518fd (DS20006027B) specifies a range of
+ * [-64,63] for TDCO, indicating a relative TDCO.
+ *
+ * Manual tests have shown, that using a relative TDCO configuration
+ * results in bus off, while an absolute configuration works.
+ *
+ * For TDCO use the max value (63) from the data sheet, but 0 as the
+ * minimum.
+ */
+static const struct can_tdc_const mcp251xfd_tdc_const = {
+ .tdcv_min = 0,
+ .tdcv_max = 63,
+ .tdco_min = 0,
+ .tdco_max = 63,
+ .tdcf_min = 0,
+ .tdcf_max = 0,
+};
+
static const char *__mcp251xfd_get_model_str(enum mcp251xfd_model model)
{
switch (model) {
@@ -510,8 +528,7 @@ static int mcp251xfd_set_bittiming(const struct mcp251xfd_priv *priv)
{
const struct can_bittiming *bt = &priv->can.bittiming;
const struct can_bittiming *dbt = &priv->can.data_bittiming;
- u32 val = 0;
- s8 tdco;
+ u32 tdcmod, val = 0;
int err;
/* CAN Control Register
@@ -575,11 +592,16 @@ static int mcp251xfd_set_bittiming(const struct mcp251xfd_priv *priv)
return err;
/* Transmitter Delay Compensation */
- tdco = clamp_t(int, dbt->brp * (dbt->prop_seg + dbt->phase_seg1),
- -64, 63);
- val = FIELD_PREP(MCP251XFD_REG_TDC_TDCMOD_MASK,
- MCP251XFD_REG_TDC_TDCMOD_AUTO) |
- FIELD_PREP(MCP251XFD_REG_TDC_TDCO_MASK, tdco);
+ if (priv->can.ctrlmode & CAN_CTRLMODE_TDC_AUTO)
+ tdcmod = MCP251XFD_REG_TDC_TDCMOD_AUTO;
+ else if (priv->can.ctrlmode & CAN_CTRLMODE_TDC_MANUAL)
+ tdcmod = MCP251XFD_REG_TDC_TDCMOD_MANUAL;
+ else
+ tdcmod = MCP251XFD_REG_TDC_TDCMOD_DISABLED;
+
+ val = FIELD_PREP(MCP251XFD_REG_TDC_TDCMOD_MASK, tdcmod) |
+ FIELD_PREP(MCP251XFD_REG_TDC_TDCV_MASK, priv->can.tdc.tdcv) |
+ FIELD_PREP(MCP251XFD_REG_TDC_TDCO_MASK, priv->can.tdc.tdco);
return regmap_write(priv->map_reg, MCP251XFD_REG_TDC, val);
}
@@ -2083,10 +2105,12 @@ static int mcp251xfd_probe(struct spi_device *spi)
priv->can.do_get_berr_counter = mcp251xfd_get_berr_counter;
priv->can.bittiming_const = &mcp251xfd_bittiming_const;
priv->can.data_bittiming_const = &mcp251xfd_data_bittiming_const;
+ priv->can.tdc_const = &mcp251xfd_tdc_const;
priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
CAN_CTRLMODE_LISTENONLY | CAN_CTRLMODE_BERR_REPORTING |
CAN_CTRLMODE_FD | CAN_CTRLMODE_FD_NON_ISO |
- CAN_CTRLMODE_CC_LEN8_DLC;
+ CAN_CTRLMODE_CC_LEN8_DLC | CAN_CTRLMODE_TDC_AUTO |
+ CAN_CTRLMODE_TDC_MANUAL;
set_bit(MCP251XFD_FLAGS_DOWN, priv->flags);
priv->ndev = ndev;
priv->spi = spi;
--
2.47.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net 3/6] can: mcp251xfd: mcp251xfd_remove(): fix order of unregistration calls
2025-05-06 13:56 [PATCH net 0/6] pull-request: can 2025-05-06 Marc Kleine-Budde
2025-05-06 13:56 ` [PATCH net 1/6] can: m_can: m_can_class_allocate_dev(): initialize spin lock on device probe Marc Kleine-Budde
2025-05-06 13:56 ` [PATCH net 2/6] can: mcp251xfd: fix TDC setting for low data bit rates Marc Kleine-Budde
@ 2025-05-06 13:56 ` Marc Kleine-Budde
2025-05-06 18:28 ` Jacob Keller
2025-05-06 13:56 ` [PATCH net 4/6] can: rockchip_canfd: rkcanfd_remove(): " Marc Kleine-Budde
` (2 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Marc Kleine-Budde @ 2025-05-06 13:56 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, linux-can, kernel, Marc Kleine-Budde, stable
If a driver is removed, the driver framework invokes the driver's
remove callback. A CAN driver's remove function calls
unregister_candev(), which calls net_device_ops::ndo_stop further down
in the call stack for interfaces which are in the "up" state.
With the mcp251xfd driver the removal of the module causes the
following warning:
| WARNING: CPU: 0 PID: 352 at net/core/dev.c:7342 __netif_napi_del_locked+0xc8/0xd8
as can_rx_offload_del() deletes the NAPI, while it is still active,
because the interface is still up.
To fix the warning, first unregister the network interface, which
calls net_device_ops::ndo_stop, which disables the NAPI, and then call
can_rx_offload_del().
Fixes: 55e5b97f003e ("can: mcp25xxfd: add driver for Microchip MCP25xxFD SPI CAN")
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20250502-can-rx-offload-del-v1-1-59a9b131589d@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
index 064d81c724f4..c30b04f8fc0d 100644
--- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
+++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
@@ -2198,8 +2198,8 @@ static void mcp251xfd_remove(struct spi_device *spi)
struct mcp251xfd_priv *priv = spi_get_drvdata(spi);
struct net_device *ndev = priv->ndev;
- can_rx_offload_del(&priv->offload);
mcp251xfd_unregister(priv);
+ can_rx_offload_del(&priv->offload);
spi->max_speed_hz = priv->spi_max_speed_hz_orig;
free_candev(ndev);
}
--
2.47.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net 4/6] can: rockchip_canfd: rkcanfd_remove(): fix order of unregistration calls
2025-05-06 13:56 [PATCH net 0/6] pull-request: can 2025-05-06 Marc Kleine-Budde
` (2 preceding siblings ...)
2025-05-06 13:56 ` [PATCH net 3/6] can: mcp251xfd: mcp251xfd_remove(): fix order of unregistration calls Marc Kleine-Budde
@ 2025-05-06 13:56 ` Marc Kleine-Budde
2025-05-06 18:29 ` Jacob Keller
2025-05-06 13:56 ` [PATCH net 5/6] can: mcan: m_can_class_unregister(): " Marc Kleine-Budde
2025-05-06 13:56 ` [PATCH net 6/6] can: gw: fix RCU/BH usage in cgw_create_job() Marc Kleine-Budde
5 siblings, 1 reply; 14+ messages in thread
From: Marc Kleine-Budde @ 2025-05-06 13:56 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, linux-can, kernel, Marc Kleine-Budde, stable,
Markus Schneider-Pargmann
If a driver is removed, the driver framework invokes the driver's
remove callback. A CAN driver's remove function calls
unregister_candev(), which calls net_device_ops::ndo_stop further down
in the call stack for interfaces which are in the "up" state.
The removal of the module causes a warning, as can_rx_offload_del()
deletes the NAPI, while it is still active, because the interface is
still up.
To fix the warning, first unregister the network interface, which
calls net_device_ops::ndo_stop, which disables the NAPI, and then call
can_rx_offload_del().
Fixes: ff60bfbaf67f ("can: rockchip_canfd: add driver for Rockchip CAN-FD controller")
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20250502-can-rx-offload-del-v1-2-59a9b131589d@pengutronix.de
Reviewed-by: Markus Schneider-Pargmann <msp@baylibre.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/rockchip/rockchip_canfd-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/rockchip/rockchip_canfd-core.c b/drivers/net/can/rockchip/rockchip_canfd-core.c
index 7107a37da36c..c3fb3176ce42 100644
--- a/drivers/net/can/rockchip/rockchip_canfd-core.c
+++ b/drivers/net/can/rockchip/rockchip_canfd-core.c
@@ -937,8 +937,8 @@ static void rkcanfd_remove(struct platform_device *pdev)
struct rkcanfd_priv *priv = platform_get_drvdata(pdev);
struct net_device *ndev = priv->ndev;
- can_rx_offload_del(&priv->offload);
rkcanfd_unregister(priv);
+ can_rx_offload_del(&priv->offload);
free_candev(ndev);
}
--
2.47.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net 5/6] can: mcan: m_can_class_unregister(): fix order of unregistration calls
2025-05-06 13:56 [PATCH net 0/6] pull-request: can 2025-05-06 Marc Kleine-Budde
` (3 preceding siblings ...)
2025-05-06 13:56 ` [PATCH net 4/6] can: rockchip_canfd: rkcanfd_remove(): " Marc Kleine-Budde
@ 2025-05-06 13:56 ` Marc Kleine-Budde
2025-05-06 18:29 ` Jacob Keller
2025-05-06 13:56 ` [PATCH net 6/6] can: gw: fix RCU/BH usage in cgw_create_job() Marc Kleine-Budde
5 siblings, 1 reply; 14+ messages in thread
From: Marc Kleine-Budde @ 2025-05-06 13:56 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, linux-can, kernel, Marc Kleine-Budde, stable,
Markus Schneider-Pargmann
If a driver is removed, the driver framework invokes the driver's
remove callback. A CAN driver's remove function calls
unregister_candev(), which calls net_device_ops::ndo_stop further down
in the call stack for interfaces which are in the "up" state.
The removal of the module causes a warning, as can_rx_offload_del()
deletes the NAPI, while it is still active, because the interface is
still up.
To fix the warning, first unregister the network interface, which
calls net_device_ops::ndo_stop, which disables the NAPI, and then call
can_rx_offload_del().
Fixes: 1be37d3b0414 ("can: m_can: fix periph RX path: use rx-offload to ensure skbs are sent from softirq context")
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20250502-can-rx-offload-del-v1-3-59a9b131589d@pengutronix.de
Reviewed-by: Markus Schneider-Pargmann <msp@baylibre.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/m_can/m_can.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 326ede9d400f..c2c116ce1087 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -2463,9 +2463,9 @@ EXPORT_SYMBOL_GPL(m_can_class_register);
void m_can_class_unregister(struct m_can_classdev *cdev)
{
+ unregister_candev(cdev->net);
if (cdev->is_peripheral)
can_rx_offload_del(&cdev->offload);
- unregister_candev(cdev->net);
}
EXPORT_SYMBOL_GPL(m_can_class_unregister);
--
2.47.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net 6/6] can: gw: fix RCU/BH usage in cgw_create_job()
2025-05-06 13:56 [PATCH net 0/6] pull-request: can 2025-05-06 Marc Kleine-Budde
` (4 preceding siblings ...)
2025-05-06 13:56 ` [PATCH net 5/6] can: mcan: m_can_class_unregister(): " Marc Kleine-Budde
@ 2025-05-06 13:56 ` Marc Kleine-Budde
2025-05-06 18:31 ` Jacob Keller
5 siblings, 1 reply; 14+ messages in thread
From: Marc Kleine-Budde @ 2025-05-06 13:56 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, linux-can, kernel, Oliver Hartkopp,
Sebastian Andrzej Siewior, Marc Kleine-Budde
From: Oliver Hartkopp <socketcan@hartkopp.net>
As reported by Sebastian Andrzej Siewior the use of local_bh_disable()
is only feasible in uni processor systems to update the modification rules.
The usual use-case to update the modification rules is to update the data
of the modifications but not the modification types (AND/OR/XOR/SET) or
the checksum functions itself.
To omit additional memory allocations to maintain fast modification
switching times, the modification description space is doubled at gw-job
creation time so that only the reference to the active modification
description is changed under rcu protection.
Rename cgw_job::mod to cf_mod and make it a RCU pointer. Allocate in
cgw_create_job() and free it together with cgw_job in
cgw_job_free_rcu(). Update all users to dereference cgw_job::cf_mod with
a RCU accessor and if possible once.
[bigeasy: Replace mod1/mod2 from the Oliver's original patch with dynamic
allocation, use RCU annotation and accessor]
Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Closes: https://lore.kernel.org/linux-can/20231031112349.y0aLoBrz@linutronix.de/
Fixes: dd895d7f21b2 ("can: cangw: introduce optional uid to reference created routing jobs")
Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Link: https://patch.msgid.link/20250429070555.cs-7b_eZ@linutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
net/can/gw.c | 151 +++++++++++++++++++++++++++++++--------------------
1 file changed, 91 insertions(+), 60 deletions(-)
diff --git a/net/can/gw.c b/net/can/gw.c
index ef93293c1fae..55eccb1c7620 100644
--- a/net/can/gw.c
+++ b/net/can/gw.c
@@ -130,7 +130,7 @@ struct cgw_job {
u32 handled_frames;
u32 dropped_frames;
u32 deleted_frames;
- struct cf_mod mod;
+ struct cf_mod __rcu *cf_mod;
union {
/* CAN frame data source */
struct net_device *dev;
@@ -459,6 +459,7 @@ static void can_can_gw_rcv(struct sk_buff *skb, void *data)
struct cgw_job *gwj = (struct cgw_job *)data;
struct canfd_frame *cf;
struct sk_buff *nskb;
+ struct cf_mod *mod;
int modidx = 0;
/* process strictly Classic CAN or CAN FD frames */
@@ -506,7 +507,8 @@ static void can_can_gw_rcv(struct sk_buff *skb, void *data)
* When there is at least one modification function activated,
* we need to copy the skb as we want to modify skb->data.
*/
- if (gwj->mod.modfunc[0])
+ mod = rcu_dereference(gwj->cf_mod);
+ if (mod->modfunc[0])
nskb = skb_copy(skb, GFP_ATOMIC);
else
nskb = skb_clone(skb, GFP_ATOMIC);
@@ -529,8 +531,8 @@ static void can_can_gw_rcv(struct sk_buff *skb, void *data)
cf = (struct canfd_frame *)nskb->data;
/* perform preprocessed modification functions if there are any */
- while (modidx < MAX_MODFUNCTIONS && gwj->mod.modfunc[modidx])
- (*gwj->mod.modfunc[modidx++])(cf, &gwj->mod);
+ while (modidx < MAX_MODFUNCTIONS && mod->modfunc[modidx])
+ (*mod->modfunc[modidx++])(cf, mod);
/* Has the CAN frame been modified? */
if (modidx) {
@@ -546,11 +548,11 @@ static void can_can_gw_rcv(struct sk_buff *skb, void *data)
}
/* check for checksum updates */
- if (gwj->mod.csumfunc.crc8)
- (*gwj->mod.csumfunc.crc8)(cf, &gwj->mod.csum.crc8);
+ if (mod->csumfunc.crc8)
+ (*mod->csumfunc.crc8)(cf, &mod->csum.crc8);
- if (gwj->mod.csumfunc.xor)
- (*gwj->mod.csumfunc.xor)(cf, &gwj->mod.csum.xor);
+ if (mod->csumfunc.xor)
+ (*mod->csumfunc.xor)(cf, &mod->csum.xor);
}
/* clear the skb timestamp if not configured the other way */
@@ -581,9 +583,20 @@ static void cgw_job_free_rcu(struct rcu_head *rcu_head)
{
struct cgw_job *gwj = container_of(rcu_head, struct cgw_job, rcu);
+ /* cgw_job::cf_mod is always accessed from the same cgw_job object within
+ * the same RCU read section. Once cgw_job is scheduled for removal,
+ * cf_mod can also be removed without mandating an additional grace period.
+ */
+ kfree(rcu_access_pointer(gwj->cf_mod));
kmem_cache_free(cgw_cache, gwj);
}
+/* Return cgw_job::cf_mod with RTNL protected section */
+static struct cf_mod *cgw_job_cf_mod(struct cgw_job *gwj)
+{
+ return rcu_dereference_protected(gwj->cf_mod, rtnl_is_locked());
+}
+
static int cgw_notifier(struct notifier_block *nb,
unsigned long msg, void *ptr)
{
@@ -616,6 +629,7 @@ static int cgw_put_job(struct sk_buff *skb, struct cgw_job *gwj, int type,
{
struct rtcanmsg *rtcan;
struct nlmsghdr *nlh;
+ struct cf_mod *mod;
nlh = nlmsg_put(skb, pid, seq, type, sizeof(*rtcan), flags);
if (!nlh)
@@ -650,82 +664,83 @@ static int cgw_put_job(struct sk_buff *skb, struct cgw_job *gwj, int type,
goto cancel;
}
+ mod = cgw_job_cf_mod(gwj);
if (gwj->flags & CGW_FLAGS_CAN_FD) {
struct cgw_fdframe_mod mb;
- if (gwj->mod.modtype.and) {
- memcpy(&mb.cf, &gwj->mod.modframe.and, sizeof(mb.cf));
- mb.modtype = gwj->mod.modtype.and;
+ if (mod->modtype.and) {
+ memcpy(&mb.cf, &mod->modframe.and, sizeof(mb.cf));
+ mb.modtype = mod->modtype.and;
if (nla_put(skb, CGW_FDMOD_AND, sizeof(mb), &mb) < 0)
goto cancel;
}
- if (gwj->mod.modtype.or) {
- memcpy(&mb.cf, &gwj->mod.modframe.or, sizeof(mb.cf));
- mb.modtype = gwj->mod.modtype.or;
+ if (mod->modtype.or) {
+ memcpy(&mb.cf, &mod->modframe.or, sizeof(mb.cf));
+ mb.modtype = mod->modtype.or;
if (nla_put(skb, CGW_FDMOD_OR, sizeof(mb), &mb) < 0)
goto cancel;
}
- if (gwj->mod.modtype.xor) {
- memcpy(&mb.cf, &gwj->mod.modframe.xor, sizeof(mb.cf));
- mb.modtype = gwj->mod.modtype.xor;
+ if (mod->modtype.xor) {
+ memcpy(&mb.cf, &mod->modframe.xor, sizeof(mb.cf));
+ mb.modtype = mod->modtype.xor;
if (nla_put(skb, CGW_FDMOD_XOR, sizeof(mb), &mb) < 0)
goto cancel;
}
- if (gwj->mod.modtype.set) {
- memcpy(&mb.cf, &gwj->mod.modframe.set, sizeof(mb.cf));
- mb.modtype = gwj->mod.modtype.set;
+ if (mod->modtype.set) {
+ memcpy(&mb.cf, &mod->modframe.set, sizeof(mb.cf));
+ mb.modtype = mod->modtype.set;
if (nla_put(skb, CGW_FDMOD_SET, sizeof(mb), &mb) < 0)
goto cancel;
}
} else {
struct cgw_frame_mod mb;
- if (gwj->mod.modtype.and) {
- memcpy(&mb.cf, &gwj->mod.modframe.and, sizeof(mb.cf));
- mb.modtype = gwj->mod.modtype.and;
+ if (mod->modtype.and) {
+ memcpy(&mb.cf, &mod->modframe.and, sizeof(mb.cf));
+ mb.modtype = mod->modtype.and;
if (nla_put(skb, CGW_MOD_AND, sizeof(mb), &mb) < 0)
goto cancel;
}
- if (gwj->mod.modtype.or) {
- memcpy(&mb.cf, &gwj->mod.modframe.or, sizeof(mb.cf));
- mb.modtype = gwj->mod.modtype.or;
+ if (mod->modtype.or) {
+ memcpy(&mb.cf, &mod->modframe.or, sizeof(mb.cf));
+ mb.modtype = mod->modtype.or;
if (nla_put(skb, CGW_MOD_OR, sizeof(mb), &mb) < 0)
goto cancel;
}
- if (gwj->mod.modtype.xor) {
- memcpy(&mb.cf, &gwj->mod.modframe.xor, sizeof(mb.cf));
- mb.modtype = gwj->mod.modtype.xor;
+ if (mod->modtype.xor) {
+ memcpy(&mb.cf, &mod->modframe.xor, sizeof(mb.cf));
+ mb.modtype = mod->modtype.xor;
if (nla_put(skb, CGW_MOD_XOR, sizeof(mb), &mb) < 0)
goto cancel;
}
- if (gwj->mod.modtype.set) {
- memcpy(&mb.cf, &gwj->mod.modframe.set, sizeof(mb.cf));
- mb.modtype = gwj->mod.modtype.set;
+ if (mod->modtype.set) {
+ memcpy(&mb.cf, &mod->modframe.set, sizeof(mb.cf));
+ mb.modtype = mod->modtype.set;
if (nla_put(skb, CGW_MOD_SET, sizeof(mb), &mb) < 0)
goto cancel;
}
}
- if (gwj->mod.uid) {
- if (nla_put_u32(skb, CGW_MOD_UID, gwj->mod.uid) < 0)
+ if (mod->uid) {
+ if (nla_put_u32(skb, CGW_MOD_UID, mod->uid) < 0)
goto cancel;
}
- if (gwj->mod.csumfunc.crc8) {
+ if (mod->csumfunc.crc8) {
if (nla_put(skb, CGW_CS_CRC8, CGW_CS_CRC8_LEN,
- &gwj->mod.csum.crc8) < 0)
+ &mod->csum.crc8) < 0)
goto cancel;
}
- if (gwj->mod.csumfunc.xor) {
+ if (mod->csumfunc.xor) {
if (nla_put(skb, CGW_CS_XOR, CGW_CS_XOR_LEN,
- &gwj->mod.csum.xor) < 0)
+ &mod->csum.xor) < 0)
goto cancel;
}
@@ -1059,7 +1074,7 @@ static int cgw_create_job(struct sk_buff *skb, struct nlmsghdr *nlh,
struct net *net = sock_net(skb->sk);
struct rtcanmsg *r;
struct cgw_job *gwj;
- struct cf_mod mod;
+ struct cf_mod *mod;
struct can_can_gw ccgw;
u8 limhops = 0;
int err = 0;
@@ -1078,37 +1093,48 @@ static int cgw_create_job(struct sk_buff *skb, struct nlmsghdr *nlh,
if (r->gwtype != CGW_TYPE_CAN_CAN)
return -EINVAL;
- err = cgw_parse_attr(nlh, &mod, CGW_TYPE_CAN_CAN, &ccgw, &limhops);
- if (err < 0)
- return err;
+ mod = kmalloc(sizeof(*mod), GFP_KERNEL);
+ if (!mod)
+ return -ENOMEM;
- if (mod.uid) {
+ err = cgw_parse_attr(nlh, mod, CGW_TYPE_CAN_CAN, &ccgw, &limhops);
+ if (err < 0)
+ goto out_free_cf;
+
+ if (mod->uid) {
ASSERT_RTNL();
/* check for updating an existing job with identical uid */
hlist_for_each_entry(gwj, &net->can.cgw_list, list) {
- if (gwj->mod.uid != mod.uid)
+ struct cf_mod *old_cf;
+
+ old_cf = cgw_job_cf_mod(gwj);
+ if (old_cf->uid != mod->uid)
continue;
/* interfaces & filters must be identical */
- if (memcmp(&gwj->ccgw, &ccgw, sizeof(ccgw)))
- return -EINVAL;
+ if (memcmp(&gwj->ccgw, &ccgw, sizeof(ccgw))) {
+ err = -EINVAL;
+ goto out_free_cf;
+ }
- /* update modifications with disabled softirq & quit */
- local_bh_disable();
- memcpy(&gwj->mod, &mod, sizeof(mod));
- local_bh_enable();
+ rcu_assign_pointer(gwj->cf_mod, mod);
+ kfree_rcu_mightsleep(old_cf);
return 0;
}
}
/* ifindex == 0 is not allowed for job creation */
- if (!ccgw.src_idx || !ccgw.dst_idx)
- return -ENODEV;
+ if (!ccgw.src_idx || !ccgw.dst_idx) {
+ err = -ENODEV;
+ goto out_free_cf;
+ }
gwj = kmem_cache_alloc(cgw_cache, GFP_KERNEL);
- if (!gwj)
- return -ENOMEM;
+ if (!gwj) {
+ err = -ENOMEM;
+ goto out_free_cf;
+ }
gwj->handled_frames = 0;
gwj->dropped_frames = 0;
@@ -1118,7 +1144,7 @@ static int cgw_create_job(struct sk_buff *skb, struct nlmsghdr *nlh,
gwj->limit_hops = limhops;
/* insert already parsed information */
- memcpy(&gwj->mod, &mod, sizeof(mod));
+ RCU_INIT_POINTER(gwj->cf_mod, mod);
memcpy(&gwj->ccgw, &ccgw, sizeof(ccgw));
err = -ENODEV;
@@ -1152,9 +1178,11 @@ static int cgw_create_job(struct sk_buff *skb, struct nlmsghdr *nlh,
if (!err)
hlist_add_head_rcu(&gwj->list, &net->can.cgw_list);
out:
- if (err)
+ if (err) {
kmem_cache_free(cgw_cache, gwj);
-
+out_free_cf:
+ kfree(mod);
+ }
return err;
}
@@ -1214,19 +1242,22 @@ static int cgw_remove_job(struct sk_buff *skb, struct nlmsghdr *nlh,
/* remove only the first matching entry */
hlist_for_each_entry_safe(gwj, nx, &net->can.cgw_list, list) {
+ struct cf_mod *cf_mod;
+
if (gwj->flags != r->flags)
continue;
if (gwj->limit_hops != limhops)
continue;
+ cf_mod = cgw_job_cf_mod(gwj);
/* we have a match when uid is enabled and identical */
- if (gwj->mod.uid || mod.uid) {
- if (gwj->mod.uid != mod.uid)
+ if (cf_mod->uid || mod.uid) {
+ if (cf_mod->uid != mod.uid)
continue;
} else {
/* no uid => check for identical modifications */
- if (memcmp(&gwj->mod, &mod, sizeof(mod)))
+ if (memcmp(cf_mod, &mod, sizeof(mod)))
continue;
}
--
2.47.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH net 1/6] can: m_can: m_can_class_allocate_dev(): initialize spin lock on device probe
2025-05-06 13:56 ` [PATCH net 1/6] can: m_can: m_can_class_allocate_dev(): initialize spin lock on device probe Marc Kleine-Budde
@ 2025-05-06 18:25 ` Jacob Keller
2025-05-07 2:10 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 14+ messages in thread
From: Jacob Keller @ 2025-05-06 18:25 UTC (permalink / raw)
To: Marc Kleine-Budde, netdev
Cc: davem, kuba, linux-can, kernel, Antonios Salios, Vincent Mailhol,
Markus Schneider-Pargmann
On 5/6/2025 6:56 AM, Marc Kleine-Budde wrote:
> From: Antonios Salios <antonios@mwa.re>
>
> The spin lock tx_handling_spinlock in struct m_can_classdev is not
> being initialized. This leads the following spinlock bad magic
> complaint from the kernel, eg. when trying to send CAN frames with
> cansend from can-utils:
>
> | BUG: spinlock bad magic on CPU#0, cansend/95
> | lock: 0xff60000002ec1010, .magic: 00000000, .owner: <none>/-1, .owner_cpu: 0
> | CPU: 0 UID: 0 PID: 95 Comm: cansend Not tainted 6.15.0-rc3-00032-ga79be02bba5c #5 NONE
> | Hardware name: MachineWare SIM-V (DT)
> | Call Trace:
> | [<ffffffff800133e0>] dump_backtrace+0x1c/0x24
> | [<ffffffff800022f2>] show_stack+0x28/0x34
> | [<ffffffff8000de3e>] dump_stack_lvl+0x4a/0x68
> | [<ffffffff8000de70>] dump_stack+0x14/0x1c
> | [<ffffffff80003134>] spin_dump+0x62/0x6e
> | [<ffffffff800883ba>] do_raw_spin_lock+0xd0/0x142
> | [<ffffffff807a6fcc>] _raw_spin_lock_irqsave+0x20/0x2c
> | [<ffffffff80536dba>] m_can_start_xmit+0x90/0x34a
> | [<ffffffff806148b0>] dev_hard_start_xmit+0xa6/0xee
> | [<ffffffff8065b730>] sch_direct_xmit+0x114/0x292
> | [<ffffffff80614e2a>] __dev_queue_xmit+0x3b0/0xaa8
> | [<ffffffff8073b8fa>] can_send+0xc6/0x242
> | [<ffffffff8073d1c0>] raw_sendmsg+0x1a8/0x36c
> | [<ffffffff805ebf06>] sock_write_iter+0x9a/0xee
> | [<ffffffff801d06ea>] vfs_write+0x184/0x3a6
> | [<ffffffff801d0a88>] ksys_write+0xa0/0xc0
> | [<ffffffff801d0abc>] __riscv_sys_write+0x14/0x1c
> | [<ffffffff8079ebf8>] do_trap_ecall_u+0x168/0x212
> | [<ffffffff807a830a>] handle_exception+0x146/0x152
>
> Initializing the spin lock in m_can_class_allocate_dev solves that
> problem.
>
> Fixes: 1fa80e23c150 ("can: m_can: Introduce a tx_fifo_in_flight counter")
> Signed-off-by: Antonios Salios <antonios@mwa.re>
> Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> Link: https://patch.msgid.link/20250425111744.37604-2-antonios@mwa.re
> Reviewed-by: Markus Schneider-Pargmann <msp@baylibre.com>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net 2/6] can: mcp251xfd: fix TDC setting for low data bit rates
2025-05-06 13:56 ` [PATCH net 2/6] can: mcp251xfd: fix TDC setting for low data bit rates Marc Kleine-Budde
@ 2025-05-06 18:27 ` Jacob Keller
0 siblings, 0 replies; 14+ messages in thread
From: Jacob Keller @ 2025-05-06 18:27 UTC (permalink / raw)
To: Marc Kleine-Budde, netdev
Cc: davem, kuba, linux-can, kernel, Kelsey Maes, Vincent Mailhol
On 5/6/2025 6:56 AM, Marc Kleine-Budde wrote:
> From: Kelsey Maes <kelsey@vpprocess.com>
>
> The TDC is currently hardcoded enabled. This means that even for lower
> CAN-FD data bitrates (with a DBRP (data bitrate prescaler) > 2) a TDC
> is configured. This leads to a bus-off condition.
>
> ISO 11898-1 section 11.3.3 says "Transmitter delay compensation" (TDC)
> is only applicable if DBRP is 1 or 2.
>
> To fix the problem, switch the driver to use the TDC calculation
> provided by the CAN driver framework (which respects ISO 11898-1
> section 11.3.3). This has the positive side effect that userspace can
> control TDC as needed.
>
> Demonstration of the feature in action:
> | $ ip link set can0 up type can bitrate 125000 dbitrate 500000 fd on
> | $ ip -details link show can0
> | 3: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 72 qdisc pfifo_fast state UP mode DEFAULT group default qlen 10
> | link/can promiscuity 0 allmulti 0 minmtu 0 maxmtu 0
> | can <FD> state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 0
> | bitrate 125000 sample-point 0.875
> | tq 50 prop-seg 69 phase-seg1 70 phase-seg2 20 sjw 10 brp 2
> | mcp251xfd: tseg1 2..256 tseg2 1..128 sjw 1..128 brp 1..256 brp_inc 1
> | dbitrate 500000 dsample-point 0.875
> | dtq 125 dprop-seg 6 dphase-seg1 7 dphase-seg2 2 dsjw 1 dbrp 5
> | mcp251xfd: dtseg1 1..32 dtseg2 1..16 dsjw 1..16 dbrp 1..256 dbrp_inc 1
> | tdcv 0..63 tdco 0..63
> | clock 40000000 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535 tso_max_size 65536 tso_max_segs 65535 gro_max_size 65536 parentbus spi parentdev spi0.0
> | $ ip link set can0 up type can bitrate 1000000 dbitrate 4000000 fd on
> | $ ip -details link show can0
> | 3: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 72 qdisc pfifo_fast state UP mode DEFAULT group default qlen 10
> | link/can promiscuity 0 allmulti 0 minmtu 0 maxmtu 0
> | can <FD,TDC-AUTO> state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 0
> | bitrate 1000000 sample-point 0.750
> | tq 25 prop-seg 14 phase-seg1 15 phase-seg2 10 sjw 5 brp 1
> | mcp251xfd: tseg1 2..256 tseg2 1..128 sjw 1..128 brp 1..256 brp_inc 1
> | dbitrate 4000000 dsample-point 0.700
> | dtq 25 dprop-seg 3 dphase-seg1 3 dphase-seg2 3 dsjw 1 dbrp 1
> | tdco 7
> | mcp251xfd: dtseg1 1..32 dtseg2 1..16 dsjw 1..16 dbrp 1..256 dbrp_inc 1
> | tdcv 0..63 tdco 0..63
> | clock 40000000 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535 tso_max_size 65536 tso_max_segs 65535 gro_max_size 65536 parentbus spi parentdev spi0.0
>
> There has been some confusion about the MCP2518FD using a relative or
> absolute TDCO due to the datasheet specifying a range of [-64,63]. I
> have a custom board with a 40 MHz clock and an estimated loop delay of
> 100 to 216 ns. During testing at a data bit rate of 4 Mbit/s I found
> that using can_get_relative_tdco() resulted in bus-off errors. The
> final TDCO value was 1 which corresponds to a 10% SSP in an absolute
> configuration. This behavior is expected if the TDCO value is really
> absolute and not relative. Using priv->can.tdc.tdco instead results in
> a final TDCO of 8, setting the SSP at exactly 80%. This configuration
> works.
>
> The automatic, manual, and off TDC modes were tested at speeds up to,
> and including, 8 Mbit/s on real hardware and behave as expected.
>
> Fixes: 55e5b97f003e ("can: mcp25xxfd: add driver for Microchip MCP25xxFD SPI CAN")
> Reported-by: Kelsey Maes <kelsey@vpprocess.com>
> Closes: https://lore.kernel.org/all/C2121586-C87F-4B23-A933-845362C29CA1@vpprocess.com
> Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> Signed-off-by: Kelsey Maes <kelsey@vpprocess.com>
> Link: https://patch.msgid.link/20250430161501.79370-1-kelsey@vpprocess.com
> [mkl: add comment]
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net 3/6] can: mcp251xfd: mcp251xfd_remove(): fix order of unregistration calls
2025-05-06 13:56 ` [PATCH net 3/6] can: mcp251xfd: mcp251xfd_remove(): fix order of unregistration calls Marc Kleine-Budde
@ 2025-05-06 18:28 ` Jacob Keller
0 siblings, 0 replies; 14+ messages in thread
From: Jacob Keller @ 2025-05-06 18:28 UTC (permalink / raw)
To: Marc Kleine-Budde, netdev; +Cc: davem, kuba, linux-can, kernel, stable
On 5/6/2025 6:56 AM, Marc Kleine-Budde wrote:
> If a driver is removed, the driver framework invokes the driver's
> remove callback. A CAN driver's remove function calls
> unregister_candev(), which calls net_device_ops::ndo_stop further down
> in the call stack for interfaces which are in the "up" state.
>
> With the mcp251xfd driver the removal of the module causes the
> following warning:
>
> | WARNING: CPU: 0 PID: 352 at net/core/dev.c:7342 __netif_napi_del_locked+0xc8/0xd8
>
> as can_rx_offload_del() deletes the NAPI, while it is still active,
> because the interface is still up.
>
> To fix the warning, first unregister the network interface, which
> calls net_device_ops::ndo_stop, which disables the NAPI, and then call
> can_rx_offload_del().
>
> Fixes: 55e5b97f003e ("can: mcp25xxfd: add driver for Microchip MCP25xxFD SPI CAN")
> Cc: stable@vger.kernel.org
> Link: https://patch.msgid.link/20250502-can-rx-offload-del-v1-1-59a9b131589d@pengutronix.de
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net 4/6] can: rockchip_canfd: rkcanfd_remove(): fix order of unregistration calls
2025-05-06 13:56 ` [PATCH net 4/6] can: rockchip_canfd: rkcanfd_remove(): " Marc Kleine-Budde
@ 2025-05-06 18:29 ` Jacob Keller
0 siblings, 0 replies; 14+ messages in thread
From: Jacob Keller @ 2025-05-06 18:29 UTC (permalink / raw)
To: Marc Kleine-Budde, netdev
Cc: davem, kuba, linux-can, kernel, stable, Markus Schneider-Pargmann
On 5/6/2025 6:56 AM, Marc Kleine-Budde wrote:
> If a driver is removed, the driver framework invokes the driver's
> remove callback. A CAN driver's remove function calls
> unregister_candev(), which calls net_device_ops::ndo_stop further down
> in the call stack for interfaces which are in the "up" state.
>
> The removal of the module causes a warning, as can_rx_offload_del()
> deletes the NAPI, while it is still active, because the interface is
> still up.
>
> To fix the warning, first unregister the network interface, which
> calls net_device_ops::ndo_stop, which disables the NAPI, and then call
> can_rx_offload_del().
>
> Fixes: ff60bfbaf67f ("can: rockchip_canfd: add driver for Rockchip CAN-FD controller")
> Cc: stable@vger.kernel.org
> Link: https://patch.msgid.link/20250502-can-rx-offload-del-v1-2-59a9b131589d@pengutronix.de
> Reviewed-by: Markus Schneider-Pargmann <msp@baylibre.com>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net 5/6] can: mcan: m_can_class_unregister(): fix order of unregistration calls
2025-05-06 13:56 ` [PATCH net 5/6] can: mcan: m_can_class_unregister(): " Marc Kleine-Budde
@ 2025-05-06 18:29 ` Jacob Keller
0 siblings, 0 replies; 14+ messages in thread
From: Jacob Keller @ 2025-05-06 18:29 UTC (permalink / raw)
To: Marc Kleine-Budde, netdev
Cc: davem, kuba, linux-can, kernel, stable, Markus Schneider-Pargmann
On 5/6/2025 6:56 AM, Marc Kleine-Budde wrote:
> If a driver is removed, the driver framework invokes the driver's
> remove callback. A CAN driver's remove function calls
> unregister_candev(), which calls net_device_ops::ndo_stop further down
> in the call stack for interfaces which are in the "up" state.
>
> The removal of the module causes a warning, as can_rx_offload_del()
> deletes the NAPI, while it is still active, because the interface is
> still up.
>
> To fix the warning, first unregister the network interface, which
> calls net_device_ops::ndo_stop, which disables the NAPI, and then call
> can_rx_offload_del().
>
> Fixes: 1be37d3b0414 ("can: m_can: fix periph RX path: use rx-offload to ensure skbs are sent from softirq context")
> Cc: stable@vger.kernel.org
> Link: https://patch.msgid.link/20250502-can-rx-offload-del-v1-3-59a9b131589d@pengutronix.de
> Reviewed-by: Markus Schneider-Pargmann <msp@baylibre.com>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
> drivers/net/can/m_can/m_can.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index 326ede9d400f..c2c116ce1087 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -2463,9 +2463,9 @@ EXPORT_SYMBOL_GPL(m_can_class_register);
>
> void m_can_class_unregister(struct m_can_classdev *cdev)
> {
> + unregister_candev(cdev->net);
> if (cdev->is_peripheral)
> can_rx_offload_del(&cdev->offload);
> - unregister_candev(cdev->net);
> }
> EXPORT_SYMBOL_GPL(m_can_class_unregister);
>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net 6/6] can: gw: fix RCU/BH usage in cgw_create_job()
2025-05-06 13:56 ` [PATCH net 6/6] can: gw: fix RCU/BH usage in cgw_create_job() Marc Kleine-Budde
@ 2025-05-06 18:31 ` Jacob Keller
0 siblings, 0 replies; 14+ messages in thread
From: Jacob Keller @ 2025-05-06 18:31 UTC (permalink / raw)
To: Marc Kleine-Budde, netdev
Cc: davem, kuba, linux-can, kernel, Oliver Hartkopp,
Sebastian Andrzej Siewior
On 5/6/2025 6:56 AM, Marc Kleine-Budde wrote:
> From: Oliver Hartkopp <socketcan@hartkopp.net>
>
> As reported by Sebastian Andrzej Siewior the use of local_bh_disable()
> is only feasible in uni processor systems to update the modification rules.
> The usual use-case to update the modification rules is to update the data
> of the modifications but not the modification types (AND/OR/XOR/SET) or
> the checksum functions itself.
>
> To omit additional memory allocations to maintain fast modification
> switching times, the modification description space is doubled at gw-job
> creation time so that only the reference to the active modification
> description is changed under rcu protection.
>
> Rename cgw_job::mod to cf_mod and make it a RCU pointer. Allocate in
> cgw_create_job() and free it together with cgw_job in
> cgw_job_free_rcu(). Update all users to dereference cgw_job::cf_mod with
> a RCU accessor and if possible once.
>
> [bigeasy: Replace mod1/mod2 from the Oliver's original patch with dynamic
> allocation, use RCU annotation and accessor]
>
> Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Closes: https://lore.kernel.org/linux-can/20231031112349.y0aLoBrz@linutronix.de/
> Fixes: dd895d7f21b2 ("can: cangw: introduce optional uid to reference created routing jobs")
> Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Link: https://patch.msgid.link/20250429070555.cs-7b_eZ@linutronix.de
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net 1/6] can: m_can: m_can_class_allocate_dev(): initialize spin lock on device probe
2025-05-06 13:56 ` [PATCH net 1/6] can: m_can: m_can_class_allocate_dev(): initialize spin lock on device probe Marc Kleine-Budde
2025-05-06 18:25 ` Jacob Keller
@ 2025-05-07 2:10 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 14+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-05-07 2:10 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: netdev, davem, kuba, linux-can, kernel, antonios, mailhol.vincent,
msp
Hello:
This series was applied to netdev/net.git (main)
by Marc Kleine-Budde <mkl@pengutronix.de>:
On Tue, 6 May 2025 15:56:17 +0200 you wrote:
> From: Antonios Salios <antonios@mwa.re>
>
> The spin lock tx_handling_spinlock in struct m_can_classdev is not
> being initialized. This leads the following spinlock bad magic
> complaint from the kernel, eg. when trying to send CAN frames with
> cansend from can-utils:
>
> [...]
Here is the summary with links:
- [net,1/6] can: m_can: m_can_class_allocate_dev(): initialize spin lock on device probe
https://git.kernel.org/netdev/net/c/dcaeeb8ae84c
- [net,2/6] can: mcp251xfd: fix TDC setting for low data bit rates
https://git.kernel.org/netdev/net/c/5e1663810e11
- [net,3/6] can: mcp251xfd: mcp251xfd_remove(): fix order of unregistration calls
https://git.kernel.org/netdev/net/c/84f5eb833f53
- [net,4/6] can: rockchip_canfd: rkcanfd_remove(): fix order of unregistration calls
https://git.kernel.org/netdev/net/c/037ada7a3181
- [net,5/6] can: mcan: m_can_class_unregister(): fix order of unregistration calls
https://git.kernel.org/netdev/net/c/0713a1b3276b
- [net,6/6] can: gw: fix RCU/BH usage in cgw_create_job()
https://git.kernel.org/netdev/net/c/511e64e13d8c
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2025-05-07 2:09 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-06 13:56 [PATCH net 0/6] pull-request: can 2025-05-06 Marc Kleine-Budde
2025-05-06 13:56 ` [PATCH net 1/6] can: m_can: m_can_class_allocate_dev(): initialize spin lock on device probe Marc Kleine-Budde
2025-05-06 18:25 ` Jacob Keller
2025-05-07 2:10 ` patchwork-bot+netdevbpf
2025-05-06 13:56 ` [PATCH net 2/6] can: mcp251xfd: fix TDC setting for low data bit rates Marc Kleine-Budde
2025-05-06 18:27 ` Jacob Keller
2025-05-06 13:56 ` [PATCH net 3/6] can: mcp251xfd: mcp251xfd_remove(): fix order of unregistration calls Marc Kleine-Budde
2025-05-06 18:28 ` Jacob Keller
2025-05-06 13:56 ` [PATCH net 4/6] can: rockchip_canfd: rkcanfd_remove(): " Marc Kleine-Budde
2025-05-06 18:29 ` Jacob Keller
2025-05-06 13:56 ` [PATCH net 5/6] can: mcan: m_can_class_unregister(): " Marc Kleine-Budde
2025-05-06 18:29 ` Jacob Keller
2025-05-06 13:56 ` [PATCH net 6/6] can: gw: fix RCU/BH usage in cgw_create_job() Marc Kleine-Budde
2025-05-06 18:31 ` Jacob Keller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).