From: Konrad Leszczynski <hidden> Date: 2025-08-28 09:57:53
This series adds three fixes addressing KASAN panic on ethtool usage,
Enhanced Descriptor printing and flow stop on TC block setup when
interface down.
Patchset has been created as a result of discussion at [1].
[1] https://lore.kernel.org/netdev/20250826113247.3481273-1-konrad.leszczynski@intel.com/
v1 -> v2:
- add missing Fixes lines
- add missing SoB lines
- removed all non-fix patches. These will be sent in a separate series
Karol Jurczenia (1):
net: stmmac: check if interface is running before TC block setup
Konrad Leszczynski (1):
net: stmmac: replace memcpy with strscpy in ethtool
Piotr Warpechowski (1):
net: stmmac: correct Tx descriptors debugfs prints
.../ethernet/stmicro/stmmac/stmmac_ethtool.c | 2 +-
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 34 ++++++++++++++-----
2 files changed, 27 insertions(+), 9 deletions(-)
--
2.34.1
From: Konrad Leszczynski <hidden> Date: 2025-08-28 09:57:58
From: Piotr Warpechowski <redacted>
It was observed that extended descriptors are not printed out fully and
enhanced descriptors are completely omitted in stmmac_rings_status_show().
Correct printing according to documentation and other existing prints in
the driver.
Fixes: 79a4f4dfa69a8379 ("net: stmmac: reduce dma ring display code duplication")
Reviewed-by: Sebastian Basierski <redacted>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Signed-off-by: Piotr Warpechowski <redacted>
Signed-off-by: Konrad Leszczynski <redacted>
---
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 31 ++++++++++++++-----
1 file changed, 23 insertions(+), 8 deletions(-)
From: Konrad Leszczynski <hidden> Date: 2025-08-28 09:58:00
From: Karol Jurczenia <redacted>
If the interface is down before setting a TC block, the queues are already
disabled and setup cannot proceed.
Fixes: 4dbbe8dde8485b89 ("net: stmmac: Add support for U32 TC filter using Flexible RX Parser")
Reviewed-by: Sebastian Basierski <redacted>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Signed-off-by: Karol Jurczenia <redacted>
Signed-off-by: Konrad Leszczynski <redacted>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +++
1 file changed, 3 insertions(+)
From: Piotr Warpechowski <redacted>
It was observed that extended descriptors are not printed out fully and
enhanced descriptors are completely omitted in stmmac_rings_status_show().
Correct printing according to documentation and other existing prints in
the driver.
Fixes: 79a4f4dfa69a8379 ("net: stmmac: reduce dma ring display code duplication")
Reviewed-by: Sebastian Basierski <redacted>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Signed-off-by: Piotr Warpechowski <redacted>
Signed-off-by: Konrad Leszczynski <redacted>
From: Karol Jurczenia <redacted>
If the interface is down before setting a TC block, the queues are already
disabled and setup cannot proceed.
Fixes: 4dbbe8dde8485b89 ("net: stmmac: Add support for U32 TC filter using Flexible RX Parser")
Reviewed-by: Sebastian Basierski <redacted>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Signed-off-by: Karol Jurczenia <redacted>
Signed-off-by: Konrad Leszczynski <redacted>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +++
1 file changed, 3 insertions(+)
From: Jakub Kicinski <kuba@kernel.org> Date: 2025-09-01 19:59:44
On Thu, 28 Aug 2025 12:02:35 +0200 Konrad Leszczynski wrote:
Fix kernel exception by replacing memcpy with strscpy when used with
safety feature strings in ethtool logic.
[ +0.000023] BUG: KASAN: global-out-of-bounds in stmmac_get_strings+0x17d/0x520 [stmmac]
[ +0.000115] Read of size 32 at addr ffffffffc0cfab20 by task ethtool/2571
If you hit this with upstream code please mention which string
is not padded. If this can't happen with upstream platforms --
there is no upstream bug. BTW ethtool_puts() is a better choice.
quoted hunk
Fixes: 8bf993a5877e8a0a ("net: stmmac: Add support for DWMAC5 and implement Safety Features")
Reviewed-by: Sebastian Basierski <redacted>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Signed-off-by: Konrad Leszczynski <redacted>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Jakub Kicinski <kuba@kernel.org> Date: 2025-09-01 20:01:01
On Thu, 28 Aug 2025 12:02:36 +0200 Konrad Leszczynski wrote:
It was observed that extended descriptors are not printed out fully and
enhanced descriptors are completely omitted in stmmac_rings_status_show().
Correct printing according to documentation and other existing prints in
the driver.
Fixes: 79a4f4dfa69a8379 ("net: stmmac: reduce dma ring display code duplication")
Sounds like an extension to me, so net-next and no Fixes
From: Jakub Kicinski <kuba@kernel.org> Date: 2025-09-01 20:03:13
On Thu, 28 Aug 2025 12:02:37 +0200 Konrad Leszczynski wrote:
If the interface is down before setting a TC block, the queues are already
disabled and setup cannot proceed.
More context would be useful. What's the user-visible behavior before
and after? Can the device handle installing the filters while down?
Is it just an issue of us restarting the queues when we shouldn't?
--
pw-bot: cr
From: Sebastian Basierski <hidden> Date: 2025-09-04 18:53:11
On 9/1/2025 9:59 PM, Jakub Kicinski wrote:
On Thu, 28 Aug 2025 12:02:35 +0200 Konrad Leszczynski wrote:
quoted
Fix kernel exception by replacing memcpy with strscpy when used with
safety feature strings in ethtool logic.
[ +0.000023] BUG: KASAN: global-out-of-bounds in stmmac_get_strings+0x17d/0x520 [stmmac]
[ +0.000115] Read of size 32 at addr ffffffffc0cfab20 by task ethtool/2571
If you hit this with upstream code please mention which string
is not padded. If this can't happen with upstream platforms --
there is no upstream bug. BTW ethtool_puts() is a better choice.
Hi Jakub,
Sorry for late answer to your review.
I double checked and made sure this bug reproduces on upstream platform.
Bug seems to appear on first string - i will add this information to
commit message.
Also thanks for code change suggestion, indeed, it looks much better.
Best Regards,
Sebastian
quoted
Fixes: 8bf993a5877e8a0a ("net: stmmac: Add support for DWMAC5 and implement Safety Features")
Reviewed-by: Sebastian Basierski <redacted>
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Signed-off-by: Konrad Leszczynski <redacted>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Sebastian Basierski <hidden> Date: 2025-09-04 18:54:24
On 9/1/2025 10:01 PM, Jakub Kicinski wrote:
On Thu, 28 Aug 2025 12:02:36 +0200 Konrad Leszczynski wrote:
quoted
It was observed that extended descriptors are not printed out fully and
enhanced descriptors are completely omitted in stmmac_rings_status_show().
Correct printing according to documentation and other existing prints in
the driver.
Fixes: 79a4f4dfa69a8379 ("net: stmmac: reduce dma ring display code duplication")
Sounds like an extension to me, so net-next and no Fixes
Sure, i will drop this patch from this patchset in next revision.
From: Sebastian Basierski <hidden> Date: 2025-09-04 19:02:11
On 9/1/2025 10:03 PM, Jakub Kicinski wrote:
More context would be useful. What's the user-visible behavior before
and after? Can the device handle installing the filters while down?
Is it just an issue of us restarting the queues when we shouldn't?
Before this patch driver couldn't be unloaded with tc filter applied.
Running those commands is enough to reproduce the issue:
tc qdisc add dev enp0s29f2 ingress
tc filter add dev enp0s29f2 ingress protocol all prio 1 u32
rmmod dwmac_intel
in effect module would not unload.
From: Andrew Lunn <andrew@lunn.ch> Date: 2025-09-04 19:18:30
On Thu, Sep 04, 2025 at 08:53:03PM +0200, Sebastian Basierski wrote:
On 9/1/2025 9:59 PM, Jakub Kicinski wrote:
quoted
On Thu, 28 Aug 2025 12:02:35 +0200 Konrad Leszczynski wrote:
quoted
Fix kernel exception by replacing memcpy with strscpy when used with
safety feature strings in ethtool logic.
[ +0.000023] BUG: KASAN: global-out-of-bounds in stmmac_get_strings+0x17d/0x520 [stmmac]
[ +0.000115] Read of size 32 at addr ffffffffc0cfab20 by task ethtool/2571
If you hit this with upstream code please mention which string
is not padded. If this can't happen with upstream platforms --
there is no upstream bug. BTW ethtool_puts() is a better choice.
Hi Jakub,
Sorry for late answer to your review.
I double checked and made sure this bug reproduces on upstream platform.
Bug seems to appear on first string - i will add this information to commit
message.
By first string, do you mean "Application Transmit Interface Parity
Check Error"?
I think it also would be better to change dwmac5_error_desc, so that
it uses char stat_string[ETH_GSTRING_LEN] __nonstring; like
stmmac_stats.
Andrew
From: Jakub Kicinski <kuba@kernel.org> Date: 2025-09-05 01:56:30
On Thu, 4 Sep 2025 21:01:49 +0200 Sebastian Basierski wrote:
On 9/1/2025 10:03 PM, Jakub Kicinski wrote:
quoted
More context would be useful. What's the user-visible behavior before
and after? Can the device handle installing the filters while down?
Is it just an issue of us restarting the queues when we shouldn't?
Before this patch driver couldn't be unloaded with tc filter applied.
Running those commands is enough to reproduce the issue:
tc qdisc add dev enp0s29f2 ingress
tc filter add dev enp0s29f2 ingress protocol all prio 1 u32
rmmod dwmac_intel
in effect module would not unload.
Makes sense. Could you also confirm that the offload doesn't in fact
work if set up when device is down? I think block setup is when qdisc
is installed?
ip link set dev $x down
tc qdisc add dev enp0s29f2 ingress
ip link set dev $x up
tc filter add dev enp0s29f2 ingress protocol all prio 1 u32 ...
If it doesn't work we can feel safe we're not breaking anyone's
scripts, however questionable.
From: Sebastian Basierski <hidden> Date: 2025-09-09 18:47:10
On 9/5/2025 3:56 AM, Jakub Kicinski wrote:
On Thu, 4 Sep 2025 21:01:49 +0200 Sebastian Basierski wrote:
quoted
On 9/1/2025 10:03 PM, Jakub Kicinski wrote:
quoted
More context would be useful. What's the user-visible behavior before
and after? Can the device handle installing the filters while down?
Is it just an issue of us restarting the queues when we shouldn't?
Before this patch driver couldn't be unloaded with tc filter applied.
Running those commands is enough to reproduce the issue:
tc qdisc add dev enp0s29f2 ingress
tc filter add dev enp0s29f2 ingress protocol all prio 1 u32
rmmod dwmac_intel
in effect module would not unload.
Makes sense. Could you also confirm that the offload doesn't in fact
work if set up when device is down? I think block setup is when qdisc
is installed?
ip link set dev $x down
tc qdisc add dev enp0s29f2 ingress
ip link set dev $x up
tc filter add dev enp0s29f2 ingress protocol all prio 1 u32 ...
If it doesn't work we can feel safe we're not breaking anyone's
scripts, however questionable.
Sorry for late response.
I just checked what you asked for.
x="enp129s29f0"
ip link set dev $x down
tc qdisc add dev $x ingress
ip link set dev $x up
tc filter add dev $x ingress protocol ip flower ip_proto 1 action drop
Looks like with and without patch ICMP packets are dropped.
From: Konrad Leszczynski <hidden> Date: 2025-09-15 12:55:05
On 04-Sep-25 20:54, Sebastian Basierski wrote:
On 9/1/2025 10:01 PM, Jakub Kicinski wrote:
quoted
On Thu, 28 Aug 2025 12:02:36 +0200 Konrad Leszczynski wrote:
quoted
It was observed that extended descriptors are not printed out fully and
enhanced descriptors are completely omitted in
stmmac_rings_status_show().
Correct printing according to documentation and other existing
prints in
the driver.
Fixes: 79a4f4dfa69a8379 ("net: stmmac: reduce dma ring display code
duplication")
Sounds like an extension to me, so net-next and no Fixes
Sure, i will drop this patch from this patchset in next revision.
From: Konrad Leszczynski <hidden> Date: 2025-09-15 13:18:46
On 04-Sep-25 21:18, Andrew Lunn wrote:
On Thu, Sep 04, 2025 at 08:53:03PM +0200, Sebastian Basierski wrote:
quoted
On 9/1/2025 9:59 PM, Jakub Kicinski wrote:
quoted
On Thu, 28 Aug 2025 12:02:35 +0200 Konrad Leszczynski wrote:
quoted
Fix kernel exception by replacing memcpy with strscpy when used with
safety feature strings in ethtool logic.
[ +0.000023] BUG: KASAN: global-out-of-bounds in stmmac_get_strings+0x17d/0x520 [stmmac]
[ +0.000115] Read of size 32 at addr ffffffffc0cfab20 by task ethtool/2571
If you hit this with upstream code please mention which string
is not padded. If this can't happen with upstream platforms --
there is no upstream bug. BTW ethtool_puts() is a better choice.
Hi Jakub,
Sorry for late answer to your review.
I double checked and made sure this bug reproduces on upstream platform.
Bug seems to appear on first string - i will add this information to commit
message.
By first string, do you mean "Application Transmit Interface Parity
Check Error"?
I think it also would be better to change dwmac5_error_desc, so that
it uses char stat_string[ETH_GSTRING_LEN] __nonstring; like
stmmac_stats.
Andrew
Hi Andrew,
Thanks for your comments. We can add the change as a new patch as part
of this patchset. Would that be ok?
From: Jakub Kicinski <kuba@kernel.org> Date: 2025-09-16 23:29:16
On Tue, 9 Sep 2025 20:47:01 +0200 Sebastian Basierski wrote:
quoted
quoted
Before this patch driver couldn't be unloaded with tc filter applied.
Running those commands is enough to reproduce the issue:
tc qdisc add dev enp0s29f2 ingress
tc filter add dev enp0s29f2 ingress protocol all prio 1 u32
rmmod dwmac_intel
in effect module would not unload.
Makes sense. Could you also confirm that the offload doesn't in fact
work if set up when device is down? I think block setup is when qdisc
is installed?
ip link set dev $x down
tc qdisc add dev enp0s29f2 ingress
ip link set dev $x up
tc filter add dev enp0s29f2 ingress protocol all prio 1 u32 ...
If it doesn't work we can feel safe we're not breaking anyone's
scripts, however questionable.
Sorry for late response.
I just checked what you asked for.
x="enp129s29f0"
ip link set dev $x down
tc qdisc add dev $x ingress
ip link set dev $x up
tc filter add dev $x ingress protocol ip flower ip_proto 1 action drop
Looks like with and without patch ICMP packets are dropped.
Aren't you testing non-offloaded filter?
Test with skip_sw, if it works it means that some order of commands may
have indeed worked.