[PATCH net-next] r8169: remove not needed net_ratelimit() check

[PATCH net-next] r8169: remove not needed net_ratelimit() check

[Heiner Kallweit]

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(-)

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)
-- 
2.37.3

Re: [PATCH net-next] r8169: remove not needed net_ratelimit() check

[Francois Romieu]

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.

-- 
Ueimor

Re: [PATCH net-next] r8169: remove not needed net_ratelimit() check

[Heiner Kallweit]

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.

Re: [PATCH net-next] r8169: remove not needed net_ratelimit() check

[patchwork-bot+netdevbpf]

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!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net-next] r8169: remove not needed net_ratelimit() check

[Andrew Lunn]

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