| Message ID | 1b1349bd-bb99-de1b-8323-2685d20f0c10@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 96efd6d01461be234bfc4ca1048a3d5febf0c425 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] r8169: remove not needed net_ratelimit() check | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Single patches do not need cover letters |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 7 of 7 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 9 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
Heiner Kallweit <hkallweit1@gmail.com> : > We're not in a hot path and don't want to miss this message, > therefore remove the net_ratelimit() check. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> There had historically been some user push against excess "spam" messages, even when systems are able to stand a gazillion of phy generated messages - resources constrained systems may not - due to dysfunctionning hardware or externally triggered events. Things may have changed though.
On 03.09.2022 17:21, Francois Romieu wrote: > Heiner Kallweit <hkallweit1@gmail.com> : >> We're not in a hot path and don't want to miss this message, >> therefore remove the net_ratelimit() check. >> >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > > There had historically been some user push against excess "spam" > messages, even when systems are able to stand a gazillion of phy > generated messages - resources constrained systems may not - due > to dysfunctionning hardware or externally triggered events. > > Things may have changed though. > I don't have a strong opinion here and would follow the net maintainers decision. I looked at a few other drivers and none of them protects link up/down messages. If also other network-related components print a message on link-up, then we might miss the PHY message due to the network-global nature of net_ratelimit(). In general newer drivers don't seem to use net_ratelimit() extensively, even though that's not really an argument against using it.
Hello: This patch was applied to netdev/net-next.git (master) by David S. Miller <davem@davemloft.net>: On Sat, 3 Sep 2022 13:15:13 +0200 you wrote: > We're not in a hot path and don't want to miss this message, > therefore remove the net_ratelimit() check. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > drivers/net/ethernet/realtek/r8169_main.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) Here is the summary with links: - [net-next] r8169: remove not needed net_ratelimit() check https://git.kernel.org/netdev/net-next/c/96efd6d01461 You are awesome, thank you!
On Sat, Sep 03, 2022 at 05:21:35PM +0200, Francois Romieu wrote: > Heiner Kallweit <hkallweit1@gmail.com> : > > We're not in a hot path and don't want to miss this message, > > therefore remove the net_ratelimit() check. > > > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > > There had historically been some user push against excess "spam" > messages, even when systems are able to stand a gazillion of phy > generated messages - resources constrained systems may not - due > to dysfunctionning hardware or externally triggered events. Ethernet PHYs generally take 1 second to report link down. Auto neg takes a little over 1 second to complete on link up. So i think the worse case here is probably one message per second. Can a resource constrained system be DoS at one message a second? If it really can, i would suggest moving the rate limiting into the phylib helper, so all devices are protected from this DoS vector. Andrew
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c index 8e1dae4de..52dacf59a 100644 --- a/drivers/net/ethernet/realtek/r8169_main.c +++ b/drivers/net/ethernet/realtek/r8169_main.c @@ -4573,8 +4573,7 @@ static void r8169_phylink_handler(struct net_device *ndev) pm_runtime_idle(&tp->pci_dev->dev); } - if (net_ratelimit()) - phy_print_status(tp->phydev); + phy_print_status(tp->phydev); } static int r8169_phy_connect(struct rtl8169_private *tp)
We're not in a hot path and don't want to miss this message, therefore remove the net_ratelimit() check. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/net/ethernet/realtek/r8169_main.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)