bonding: rate-limit bonding driver inspect messages
Commit Message
Through the routine bond_mii_monitor(), bonding driver inspects and commits the
slave state changes. During the times when slave state change and failure in
aqcuiring rtnl lock happen at the same time, the routine bond_mii_monitor()
reschedules itself to come around after 1 msec to commit the new state.
During this, it executes the routine bond_miimon_inspect() to re-inspect the
state chane and prints the corresponding slave state on to the console. Hence
we do see a message at every 1 msec till the rtnl lock is acquired and state
chage is committed.
This patch doesn't change how bond functions. It only simply limits this kind
of log flood.
v2: Use exising net_ratelimit() instead of introducing new rate-limit
parameter.
v3: Commit message is modified to provide summary of the issue, because of
which rate-limiting the bonding driver messages is needed.
Signed-off-by: Praveen Kumar Kannoju <praveen.kannoju@oracle.com>
---
drivers/net/bonding/bond_main.c | 36 ++++++++++++++++++++----------------
1 file changed, 20 insertions(+), 16 deletions(-)
Comments
Hi Praveen,
You need to target the branch name in subject. e.g.
[PATCH net-next] bonding: ...
On Mon, Feb 19, 2024 at 07:07:21PM +0530, Praveen Kumar Kannoju wrote:
> Through the routine bond_mii_monitor(), bonding driver inspects and commits the
> slave state changes. During the times when slave state change and failure in
> aqcuiring rtnl lock happen at the same time, the routine bond_mii_monitor()
> reschedules itself to come around after 1 msec to commit the new state.
>
> During this, it executes the routine bond_miimon_inspect() to re-inspect the
> state chane and prints the corresponding slave state on to the console. Hence
> we do see a message at every 1 msec till the rtnl lock is acquired and state
> chage is committed.
>
> This patch doesn't change how bond functions. It only simply limits this kind
> of log flood.
>
> v2: Use exising net_ratelimit() instead of introducing new rate-limit
> parameter.
>
> v3: Commit message is modified to provide summary of the issue, because of
> which rate-limiting the bonding driver messages is needed.
>
> Signed-off-by: Praveen Kumar Kannoju <praveen.kannoju@oracle.com>
For the change log, you can put it after sign off. e.g.
Signed-off-by: Praveen Kumar Kannoju <praveen.kannoju@oracle.com>
---
v3: ...
v2: ...
---
Here is an example https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html#changes-requested
Thanks
Hangbin
> ---
> drivers/net/bonding/bond_main.c | 36 ++++++++++++++++++++----------------
> 1 file changed, 20 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 4e0600c..e92eba1 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -2610,12 +2610,13 @@ static int bond_miimon_inspect(struct bonding *bond)
> commit++;
> slave->delay = bond->params.downdelay;
> if (slave->delay) {
> - slave_info(bond->dev, slave->dev, "link status down for %sinterface, disabling it in %d ms\n",
> - (BOND_MODE(bond) ==
> - BOND_MODE_ACTIVEBACKUP) ?
> - (bond_is_active_slave(slave) ?
> - "active " : "backup ") : "",
> - bond->params.downdelay * bond->params.miimon);
> + if (net_ratelimit())
> + slave_info(bond->dev, slave->dev, "link status down for %sinterface, disabling it in %d ms\n",
> + (BOND_MODE(bond) ==
> + BOND_MODE_ACTIVEBACKUP) ?
> + (bond_is_active_slave(slave) ?
> + "active " : "backup ") : "",
> + bond->params.downdelay * bond->params.miimon);
> }
> fallthrough;
> case BOND_LINK_FAIL:
> @@ -2623,9 +2624,10 @@ static int bond_miimon_inspect(struct bonding *bond)
> /* recovered before downdelay expired */
> bond_propose_link_state(slave, BOND_LINK_UP);
> slave->last_link_up = jiffies;
> - slave_info(bond->dev, slave->dev, "link status up again after %d ms\n",
> - (bond->params.downdelay - slave->delay) *
> - bond->params.miimon);
> + if (net_ratelimit())
> + slave_info(bond->dev, slave->dev, "link status up again after %d ms\n",
> + (bond->params.downdelay - slave->delay) *
> + bond->params.miimon);
> commit++;
> continue;
> }
> @@ -2648,18 +2650,20 @@ static int bond_miimon_inspect(struct bonding *bond)
> slave->delay = bond->params.updelay;
>
> if (slave->delay) {
> - slave_info(bond->dev, slave->dev, "link status up, enabling it in %d ms\n",
> - ignore_updelay ? 0 :
> - bond->params.updelay *
> - bond->params.miimon);
> + if (net_ratelimit())
> + slave_info(bond->dev, slave->dev, "link status up, enabling it in %d ms\n",
> + ignore_updelay ? 0 :
> + bond->params.updelay *
> + bond->params.miimon);
> }
> fallthrough;
> case BOND_LINK_BACK:
> if (!link_state) {
> bond_propose_link_state(slave, BOND_LINK_DOWN);
> - slave_info(bond->dev, slave->dev, "link status down again after %d ms\n",
> - (bond->params.updelay - slave->delay) *
> - bond->params.miimon);
> + if (net_ratelimit())
> + slave_info(bond->dev, slave->dev, "link status down again after %d ms\n",
> + (bond->params.updelay - slave->delay) *
> + bond->params.miimon);
> commit++;
> continue;
> }
> --
> 1.8.3.1
>
@@ -2610,12 +2610,13 @@ static int bond_miimon_inspect(struct bonding *bond)
commit++;
slave->delay = bond->params.downdelay;
if (slave->delay) {
- slave_info(bond->dev, slave->dev, "link status down for %sinterface, disabling it in %d ms\n",
- (BOND_MODE(bond) ==
- BOND_MODE_ACTIVEBACKUP) ?
- (bond_is_active_slave(slave) ?
- "active " : "backup ") : "",
- bond->params.downdelay * bond->params.miimon);
+ if (net_ratelimit())
+ slave_info(bond->dev, slave->dev, "link status down for %sinterface, disabling it in %d ms\n",
+ (BOND_MODE(bond) ==
+ BOND_MODE_ACTIVEBACKUP) ?
+ (bond_is_active_slave(slave) ?
+ "active " : "backup ") : "",
+ bond->params.downdelay * bond->params.miimon);
}
fallthrough;
case BOND_LINK_FAIL:
@@ -2623,9 +2624,10 @@ static int bond_miimon_inspect(struct bonding *bond)
/* recovered before downdelay expired */
bond_propose_link_state(slave, BOND_LINK_UP);
slave->last_link_up = jiffies;
- slave_info(bond->dev, slave->dev, "link status up again after %d ms\n",
- (bond->params.downdelay - slave->delay) *
- bond->params.miimon);
+ if (net_ratelimit())
+ slave_info(bond->dev, slave->dev, "link status up again after %d ms\n",
+ (bond->params.downdelay - slave->delay) *
+ bond->params.miimon);
commit++;
continue;
}
@@ -2648,18 +2650,20 @@ static int bond_miimon_inspect(struct bonding *bond)
slave->delay = bond->params.updelay;
if (slave->delay) {
- slave_info(bond->dev, slave->dev, "link status up, enabling it in %d ms\n",
- ignore_updelay ? 0 :
- bond->params.updelay *
- bond->params.miimon);
+ if (net_ratelimit())
+ slave_info(bond->dev, slave->dev, "link status up, enabling it in %d ms\n",
+ ignore_updelay ? 0 :
+ bond->params.updelay *
+ bond->params.miimon);
}
fallthrough;
case BOND_LINK_BACK:
if (!link_state) {
bond_propose_link_state(slave, BOND_LINK_DOWN);
- slave_info(bond->dev, slave->dev, "link status down again after %d ms\n",
- (bond->params.updelay - slave->delay) *
- bond->params.miimon);
+ if (net_ratelimit())
+ slave_info(bond->dev, slave->dev, "link status down again after %d ms\n",
+ (bond->params.updelay - slave->delay) *
+ bond->params.miimon);
commit++;
continue;
}