Received-SPF: pass (google.com: domain of linux-kernel+bounces-74060-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
From: Praveen Kannoju <praveen.kannoju@oracle.com>
To: Simon Horman <horms@kernel.org>
CC: "j.vosburgh@gmail.com" <j.vosburgh@gmail.com>,
        "andy@greyhouse.net"
	<andy@greyhouse.net>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "edumazet@google.com" <edumazet@google.com>,
        "kuba@kernel.org"
	<kuba@kernel.org>,
        "pabeni@redhat.com" <pabeni@redhat.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Rajesh
 Sivaramasubramaniom <rajesh.sivaramasubramaniom@oracle.com>,
        Rama
 Nichanamatlu <rama.nichanamatlu@oracle.com>,
        Manjunath Patil
	<manjunath.b.patil@oracle.com>
Subject: RE: [PATCH net-next v4] bonding: rate-limit bonding driver inspect
 messages
Thread-Topic: [PATCH net-next v4] bonding: rate-limit bonding driver inspect
 messages
Thread-Index: AQHaY7pT1u4CXCMlvk+k7R7DJ0sbGLETNBaAgAEMn1A=
Date: Wed, 21 Feb 2024 05:11:31 +0000
Message-ID: 
 <SA1PR10MB6445000872253BE2227052F38C572@SA1PR10MB6445.namprd10.prod.outlook.com>
References: <20240220050437.5623-1-praveen.kannoju@oracle.com>
 <20240220130735.GI40273@kernel.org>
In-Reply-To: <20240220130735.GI40273@kernel.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: 
 =?us-ascii?Q?yEyeCax9ax7YfeQt2puJtRX99kw/VusIejNYSnvu/WOR3crMyjXxceb0DYtr?=
 =?us-ascii?Q?Wj0lwrVlHbxMPuCdJk2DZ8qodaAgbwNVc7qBY7TY+u/Rmx6i19rdSGf84ul4?=
 =?us-ascii?Q?Y+0bm+7ANTGH070sZks0rl9+uezkS/O8TmXsyt4623OIgJYKU6BoMPbxtg8+?=
 =?us-ascii?Q?5KPcQVTr4Eai0T7pwS/QmoHpWBvx4srh/oxCpGeA5443o81HEJuHu8WmgLEB?=
 =?us-ascii?Q?oFPjWd1ZFWxBjAS4fRmokoZeEbkqs+pG8v0jTkVBsvlJYUk0Bz8O1cJ33DBK?=
 =?us-ascii?Q?rro1qBzmhKVAq6a1I4gpCZ0IFUDU7D9snOhgkARuIcbocU7C7tKSoCFkf8ue?=
 =?us-ascii?Q?eHzzKHz6T1EZ+VG6NBogKTthaBl/hDG83/NMdn42ESvTm/cpkeH4ichuG8j+?=
 =?us-ascii?Q?zZg23+6TTco4om1o4QcmbWu+Y3qICQVvQP3vQiqut6zKFsiAMiXPfcYHssmC?=
 =?us-ascii?Q?8hGQqV3y64xjDbNly36o/atO8lpRC+5ersGqlt6M8dsYKm9sdjdP54lQCRkw?=
 =?us-ascii?Q?RTc97rZ04cdwPzJtkoq7GZGigYttmPRA84HCGJ9cfn8yZO7eS2U9761PN8Ux?=
 =?us-ascii?Q?TN0tbHeOsCNtPqOVx2hfknZx9fcIunRriDtFr4Tss0UxoFP97QiyVWlBwwTC?=
 =?us-ascii?Q?1GUuR5Tis9M+pzb+rHGYi1JlDyiYEEkbrpUjM3rCVPEHVmEwyr8hj0VbzkWc?=
 =?us-ascii?Q?TSwVLRqgRi9dx0QZnQMbpBluWEqH5cf0qqqWf+ofTJDpw1+2PD2gHkkancTn?=
 =?us-ascii?Q?GOh1o5JFZfqtkfC0U8oWDvbsiLrs0zzl/LNrdWlh7QrNVx8oxNlt/tuNQHE0?=
 =?us-ascii?Q?y7pDvHqhlt22LUjogjSEHjOoLdh3DGN0wSmfaDdVUTOQrJQYJKS5jiOSEFbR?=
 =?us-ascii?Q?6YEjWA8rSwc8+Z34gmU4Jo74gReMK5Fh6tthHAJlp2PGJG+WQC66BjmSAkKn?=
 =?us-ascii?Q?gFT7H+ZbL0qT1o+XP3Rpt/KX0uPPRhltEFgH54L1TeK67V+0w5KBlMC+sLPa?=
 =?us-ascii?Q?abwfEz+q5sL/v5Zaw1nl3CABaEJtuc8P+wcbzoKYIjox5IlacAFdpLz57G6H?=
 =?us-ascii?Q?ZK76xbCKgFj9AvnXi+kSGd5YXesD+s/ks2pmnp7prF3vuDE4yFk3WFLO7sRj?=
 =?us-ascii?Q?941IqWOHS99VOGi+x7DKcDzRJ0Xtptj7yojzGIVmC6dBo2LjFm0fZmgnoqOK?=
 =?us-ascii?Q?E8VysC9ayaVug2if6bYKeTdZ11tFoZmtAKbaxUA0EwCebdQv9bgakPK4sC5c?=
 =?us-ascii?Q?jkHXfchSW4LgVr4dr27i9RuoPGngnH5TV7wZcUL7OBhAHFw0uaWg3mAvFCjQ?=
 =?us-ascii?Q?lR7KNRmF+/sSw+SxxwTFwSJI+cxJI43Lg/Q0rt7E814L2tm+D3TWs4SIR6rl?=
 =?us-ascii?Q?JiPVZSA4JTIfEMFf/neZRYzOyIEpDo4wgWtxMbTliTO/G//UAVVaWFapzGUX?=
 =?us-ascii?Q?FTw+1trkweIX84r5RdqQ9XRlHLO6MAZNjAkCsnQjebUNzmmxC6hKLeBfCmW1?=
 =?us-ascii?Q?PP4y3q7QQHIZIzZO4hBEymg0YlIgNjdyz4zDHakoBryN3xaecuLI6bVth0Jp?=
 =?us-ascii?Q?t4w8olcpsTdQJuDY55a3rGSrcGqvRSYWwyuGZi5OGXQ77TEOL/5ugy2iSlOp?=
 =?us-ascii?Q?bg=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	vREvTfmsmgZOUT8jb79gFBZT9L3ZgpNnZXohL+fGOgeie7d575j44A4+LtCXTkfWzg2+zlNs7/wm6weF4R+RI/pLYcgctYNpVgWIKG1LD1EtVKlUgkNscgS0Xgbiv0dNu7UO3nstN84S82t7I0D+tlDGnRJ2cIDvAxwu8ecdqZJyly2ZXIQeB0MvI6IWz1ox9Khjs5CoPFCuR2YtdxArkwJWyT2kyNut50DBNPpyap/6x5zOwqWuUyAlB2aRPxrHast0kjUQSrJHCA9TJPpgRdqlfeE1bH1K+2lydbaGmpcSlQSqpgBOjxaqbEkp1gZrzOSmMqg4eIUW8AJ70tnUmwyxh8flPF9epPBVG+oV2pkEFIoOmpTs9KiiTbidJdi+cG1+IXhwEY5hT8hRDRwqvgqhtE4w/MDc4u6LORxXBOqRuQ6MOphQU7tf6qkP2zXeRZnmOPMZlxOX9QPiwPdet4S4mGBKH5uWPcy1VsYMkJN7ixLNmN+iTHc84+21s0UG88EllLOf8qT9ZvzdmDdsJDSgbqJvpA6xZ28Lg11aGencuFHu745prS2PFnCcnrVRZbagQwkf2vqizK9vAtVl0whAeqTn+jPiAEONputfmOM=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SA1PR10MB6445.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c9ec6ed8-8dc6-498a-a62b-08dc329b9151
X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Feb 2024 05:11:31.9050
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: QKgPO7S7yUOcgeo0mV4LeipZjl9AZ9ABNSvNoc2IUEyHehZGyDZUP8SmH0iCgYWlTlLrMUJ7ztoDMRLIxMt8LRVvlvgmLKxVwT8U29Z72MA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR10MB7533
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-02-20_06,2024-02-20_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 malwarescore=0
 phishscore=0 mlxscore=0 mlxlogscore=999 adultscore=0 bulkscore=0
 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2311290000 definitions=main-2402210036
X-Proofpoint-GUID: -FhIKKKEG-qEqzP1jcc8-oCbiSaCD5va
X-Proofpoint-ORIG-GUID: -FhIKKKEG-qEqzP1jcc8-oCbiSaCD5va

> -----Original Message-----
> From: Simon Horman <horms@kernel.org>
> Sent: 20 February 2024 06:38 PM
> To: Praveen Kannoju <praveen.kannoju@oracle.com>
> Cc: j.vosburgh@gmail.com; andy@greyhouse.net; davem@davemloft.net; edumaz=
et@google.com; kuba@kernel.org;
> pabeni@redhat.com; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; =
Rajesh Sivaramasubramaniom
> <rajesh.sivaramasubramaniom@oracle.com>; Rama Nichanamatlu <rama.nichanam=
atlu@oracle.com>; Manjunath Patil
> <manjunath.b.patil@oracle.com>
> Subject: Re: [PATCH net-next v4] bonding: rate-limit bonding driver inspe=
ct messages
>=20
> On Tue, Feb 20, 2024 at 10:34:37AM +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.
> >
> > Signed-off-by: Praveen Kumar Kannoju <praveen.kannoju@oracle.com>
> > ---
> > v4:
> >   - Rectification in the patch subject and versioning details.
> > v3: https://lore.kernel.org/lkml/20240219133721.4567-1-praveen.kannoju@=
oracle.com/
> >   - Commit message is modified to provide summary of the issue, because=
 of
> >     which rate-limiting the bonding driver messages is needed.
> > v2: https://lore.kernel.org/lkml/20240215172554.4211-1-praveen.kannoju@=
oracle.com/
> >   - Use exising net_ratelimit() instead of introducing new rate-limit
> >     parameter.
> > v1:
> > https://lore.kernel.org/lkml/20240214044245.33170-1-praveen.kannoju@or
> > acle.com/
> > ---
> >  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 =3D bond->params.downdelay;
> >  			if (slave->delay) {
> > -				slave_info(bond->dev, slave->dev, "link status down for %sinterfac=
e, disabling it in %d ms\n",
> > -					   (BOND_MODE(bond) =3D=3D
> > -					    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 %sinterfa=
ce, disabling it in %d ms\n",
> > +						   (BOND_MODE(bond) =3D=3D
> > +						   BOND_MODE_ACTIVEBACKUP) ?
> > +						   (bond_is_active_slave(slave) ?
> > +						   "active " : "backup ") : "",
> > +						   bond->params.downdelay * bond->params.miimon);
> >  			}
>=20
> Hi Praveen,
>=20
> As this is used several times I think that it would be worth introducing =
a slave_info_ratelimit() helper. That is  assuming slave_info() is
> still used without a rate limit. If not, you could just add net_ratelimit=
 directly to slave_info().
>=20
> If none of this is desirable for some reason, then could you consider red=
ucing indentation somehow. f.e.:
>=20
> 		if (slave->delayi && net_ratelimit())
> 			slave_info(...

Thank you, Simon.=20
Jay insisted upon using the existing net_ratelimit() instead of introducing=
 a new helper. I've modified the patch to reduce the indentation as suggest=
ed by you. Please review.

-
Praveen.