[PATCH net] ibmveth: use net_err_ratelimited when set_multicast_list

[PATCH net] ibmveth: use net_err_ratelimited when set_multicast_list

[Hangbin Liu]

When setting lots of multicast list on ibmveth, e.g. add 3000 membership on a
multicast group, the following error message flushes our log file

8507    [  901.478251] ibmveth 30000003 env3: h_multicast_ctrl rc=4 when adding an entry to the filter table
...
1718386 [ 5636.808658] ibmveth 30000003 env3: h_multicast_ctrl rc=4 when adding an entry to the filter table

We got 1.5 million lines of messages in 1.3h. Let's replace netdev_err() by
net_err_ratelimited() to avoid this issue. I don't use netdev_err_once() in
case h_multicast_ctrl() return different lpar_rc types.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 drivers/net/ethernet/ibm/ibmveth.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
index d654c234aaf7..3b9406a4ca92 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -1446,9 +1446,11 @@ static void ibmveth_set_multicast_list(struct net_device *netdev)
 						   IbmVethMcastAddFilter,
 						   mcast_addr);
 			if (lpar_rc != H_SUCCESS) {
-				netdev_err(netdev, "h_multicast_ctrl rc=%ld "
-					   "when adding an entry to the filter "
-					   "table\n", lpar_rc);
+				net_err_ratelimited("%s %s%s: h_multicast_ctrl rc=%ld when adding an entry to the filter table\n",
+						    ibmveth_driver_name,
+						    netdev_name(netdev),
+						    netdev_reg_state(netdev),
+						    lpar_rc);
 			}
 		}
 
-- 
2.19.2

Re: [PATCH net] ibmveth: use net_err_ratelimited when set_multicast_list

[Joe Perches]

On Thu, 2019-08-01 at 17:03 +0800, Hangbin Liu wrote:
> When setting lots of multicast list on ibmveth, e.g. add 3000 membership on a
> multicast group, the following error message flushes our log file
> 
> 8507    [  901.478251] ibmveth 30000003 env3: h_multicast_ctrl rc=4 when adding an entry to the filter table
> ...
> 1718386 [ 5636.808658] ibmveth 30000003 env3: h_multicast_ctrl rc=4 when adding an entry to the filter table
> 
> We got 1.5 million lines of messages in 1.3h. Let's replace netdev_err() by
> net_err_ratelimited() to avoid this issue. I don't use netdev_err_once() in
> case h_multicast_ctrl() return different lpar_rc types.
> 
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
>  drivers/net/ethernet/ibm/ibmveth.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
> index d654c234aaf7..3b9406a4ca92 100644
> --- a/drivers/net/ethernet/ibm/ibmveth.c
> +++ b/drivers/net/ethernet/ibm/ibmveth.c
> @@ -1446,9 +1446,11 @@ static void ibmveth_set_multicast_list(struct net_device *netdev)
>  						   IbmVethMcastAddFilter,
>  						   mcast_addr);
>  			if (lpar_rc != H_SUCCESS) {
> -				netdev_err(netdev, "h_multicast_ctrl rc=%ld "
> -					   "when adding an entry to the filter "
> -					   "table\n", lpar_rc);
> +				net_err_ratelimited("%s %s%s: h_multicast_ctrl rc=%ld when adding an entry to the filter table\n",
> +						    ibmveth_driver_name,
> +						    netdev_name(netdev),
> +						    netdev_reg_state(netdev),
> +						    lpar_rc);

Perhaps add the netdev_<level>_ratelimited variants and use that instead

Somthing like:

---
 include/linux/netdevice.h | 54 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 88292953aa6f..37116019e14f 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4737,6 +4737,60 @@ do {								\
 #define netdev_info_once(dev, fmt, ...) \
 	netdev_level_once(KERN_INFO, dev, fmt, ##__VA_ARGS__)
 
+#define netdev_level_ratelimited(netdev_level, dev, fmt, ...)		\
+do {									\
+	static DEFINE_RATELIMIT_STATE(_rs,				\
+				      DEFAULT_RATELIMIT_INTERVAL,	\
+				      DEFAULT_RATELIMIT_BURST);		\
+	if (__ratelimit(&_rs))						\
+		netdev_level(dev, fmt, ##__VA_ARGS__);			\
+} while (0)
+
+#define netdev_emerg_ratelimited(dev, fmt, ...)				\
+	netdev_level_ratelimited(netdev_emerg, dev, fmt, ##__VA_ARGS__)
+#define netdev_alert_ratelimited(dev, fmt, ...)				\
+	netdev_level_ratelimited(netdev_alert, dev, fmt, ##__VA_ARGS__)
+#define netdev_crit_ratelimited(dev, fmt, ...)				\
+	netdev_level_ratelimited(netdev_crit, dev, fmt, ##__VA_ARGS__)
+#define netdev_err_ratelimited(dev, fmt, ...)				\
+	netdev_level_ratelimited(netdev_err, dev, fmt, ##__VA_ARGS__)
+#define netdev_warn_ratelimited(dev, fmt, ...)				\
+	netdev_level_ratelimited(netdev_warn, dev, fmt, ##__VA_ARGS__)
+#define netdev_notice_ratelimited(dev, fmt, ...)			\
+	netdev_level_ratelimited(netdev_notice, dev, fmt, ##__VA_ARGS__)
+#define netdev_info_ratelimited(dev, fmt, ...)				\
+	netdev_level_ratelimited(netdev_info, dev, fmt, ##__VA_ARGS__)
+
+#if defined(CONFIG_DYNAMIC_DEBUG)
+/* descriptor check is first to prevent flooding with "callbacks suppressed" */
+#define netdev_dbg_ratelimited(dev, fmt, ...)				\
+do {									\
+	static DEFINE_RATELIMIT_STATE(_rs,				\
+				      DEFAULT_RATELIMIT_INTERVAL,	\
+				      DEFAULT_RATELIMIT_BURST);		\
+	DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);			\
+	if (DYNAMIC_DEBUG_BRANCH(descriptor) &&				\
+	    __ratelimit(&_rs))						\
+		__dynamic_netdev_dbg(&descriptor, dev, fmt,		\
+				     ##__VA_ARGS__);			\
+} while (0)
+#elif defined(DEBUG)
+#define netdev_dbg_ratelimited(dev, fmt, ...)				\
+do {									\
+	static DEFINE_RATELIMIT_STATE(_rs,				\
+				      DEFAULT_RATELIMIT_INTERVAL,	\
+				      DEFAULT_RATELIMIT_BURST);		\
+	if (__ratelimit(&_rs))						\
+		netdev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__);	\
+} while (0)
+#else
+#define netdev_dbg_ratelimited(dev, fmt, ...)				\
+do {									\
+	if (0)								\
+		netdev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__);	\
+} while (0)
+#endif
+
 #define MODULE_ALIAS_NETDEV(device) \
 	MODULE_ALIAS("netdev-" device)
 

Re: [PATCH net] ibmveth: use net_err_ratelimited when set_multicast_list

[Hangbin Liu]

On Thu, Aug 01, 2019 at 03:28:43AM -0700, Joe Perches wrote:
> Perhaps add the netdev_<level>_ratelimited variants and use that instead
> 
> Somthing like:

Yes, that looks better. Do you mind if I take your code and add your
Signed-off-by info?

Thanks
Hangbin
> 
> ---
>  include/linux/netdevice.h | 54 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 88292953aa6f..37116019e14f 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -4737,6 +4737,60 @@ do {								\
>  #define netdev_info_once(dev, fmt, ...) \
>  	netdev_level_once(KERN_INFO, dev, fmt, ##__VA_ARGS__)
>  
> +#define netdev_level_ratelimited(netdev_level, dev, fmt, ...)		\
> +do {									\
> +	static DEFINE_RATELIMIT_STATE(_rs,				\
> +				      DEFAULT_RATELIMIT_INTERVAL,	\
> +				      DEFAULT_RATELIMIT_BURST);		\
> +	if (__ratelimit(&_rs))						\
> +		netdev_level(dev, fmt, ##__VA_ARGS__);			\
> +} while (0)
> +
> +#define netdev_emerg_ratelimited(dev, fmt, ...)				\
> +	netdev_level_ratelimited(netdev_emerg, dev, fmt, ##__VA_ARGS__)
> +#define netdev_alert_ratelimited(dev, fmt, ...)				\
> +	netdev_level_ratelimited(netdev_alert, dev, fmt, ##__VA_ARGS__)
> +#define netdev_crit_ratelimited(dev, fmt, ...)				\
> +	netdev_level_ratelimited(netdev_crit, dev, fmt, ##__VA_ARGS__)
> +#define netdev_err_ratelimited(dev, fmt, ...)				\
> +	netdev_level_ratelimited(netdev_err, dev, fmt, ##__VA_ARGS__)
> +#define netdev_warn_ratelimited(dev, fmt, ...)				\
> +	netdev_level_ratelimited(netdev_warn, dev, fmt, ##__VA_ARGS__)
> +#define netdev_notice_ratelimited(dev, fmt, ...)			\
> +	netdev_level_ratelimited(netdev_notice, dev, fmt, ##__VA_ARGS__)
> +#define netdev_info_ratelimited(dev, fmt, ...)				\
> +	netdev_level_ratelimited(netdev_info, dev, fmt, ##__VA_ARGS__)
> +
> +#if defined(CONFIG_DYNAMIC_DEBUG)
> +/* descriptor check is first to prevent flooding with "callbacks suppressed" */
> +#define netdev_dbg_ratelimited(dev, fmt, ...)				\
> +do {									\
> +	static DEFINE_RATELIMIT_STATE(_rs,				\
> +				      DEFAULT_RATELIMIT_INTERVAL,	\
> +				      DEFAULT_RATELIMIT_BURST);		\
> +	DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);			\
> +	if (DYNAMIC_DEBUG_BRANCH(descriptor) &&				\
> +	    __ratelimit(&_rs))						\
> +		__dynamic_netdev_dbg(&descriptor, dev, fmt,		\
> +				     ##__VA_ARGS__);			\
> +} while (0)
> +#elif defined(DEBUG)
> +#define netdev_dbg_ratelimited(dev, fmt, ...)				\
> +do {									\
> +	static DEFINE_RATELIMIT_STATE(_rs,				\
> +				      DEFAULT_RATELIMIT_INTERVAL,	\
> +				      DEFAULT_RATELIMIT_BURST);		\
> +	if (__ratelimit(&_rs))						\
> +		netdev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__);	\
> +} while (0)
> +#else
> +#define netdev_dbg_ratelimited(dev, fmt, ...)				\
> +do {									\
> +	if (0)								\
> +		netdev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__);	\
> +} while (0)
> +#endif
> +
>  #define MODULE_ALIAS_NETDEV(device) \
>  	MODULE_ALIAS("netdev-" device)
>  
> 
> 

Re: [PATCH net] ibmveth: use net_err_ratelimited when set_multicast_list

[Joe Perches]

On Thu, 2019-08-01 at 22:10 +0800, Hangbin Liu wrote:
> On Thu, Aug 01, 2019 at 03:28:43AM -0700, Joe Perches wrote:
> > Perhaps add the netdev_<level>_ratelimited variants and use that instead
> > 
> > Somthing like:
> 
> Yes, that looks better. Do you mind if I take your code and add your
> Signed-off-by info?

Well, if you test and verify all the paths,
(I did not and just wrote that without testing),
you could add something like "Suggested-by:".

cheers, Joe

> Thanks
> Hangbin
> > ---
> >  include/linux/netdevice.h | 54 +++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 54 insertions(+)
> > 
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index 88292953aa6f..37116019e14f 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -4737,6 +4737,60 @@ do {								\
> >  #define netdev_info_once(dev, fmt, ...) \
> >  	netdev_level_once(KERN_INFO, dev, fmt, ##__VA_ARGS__)
> >  
> > +#define netdev_level_ratelimited(netdev_level, dev, fmt, ...)		\
> > +do {									\
> > +	static DEFINE_RATELIMIT_STATE(_rs,				\
> > +				      DEFAULT_RATELIMIT_INTERVAL,	\
> > +				      DEFAULT_RATELIMIT_BURST);		\
> > +	if (__ratelimit(&_rs))						\
> > +		netdev_level(dev, fmt, ##__VA_ARGS__);			\
> > +} while (0)
> > +
> > +#define netdev_emerg_ratelimited(dev, fmt, ...)				\
> > +	netdev_level_ratelimited(netdev_emerg, dev, fmt, ##__VA_ARGS__)
> > +#define netdev_alert_ratelimited(dev, fmt, ...)				\
> > +	netdev_level_ratelimited(netdev_alert, dev, fmt, ##__VA_ARGS__)
> > +#define netdev_crit_ratelimited(dev, fmt, ...)				\
> > +	netdev_level_ratelimited(netdev_crit, dev, fmt, ##__VA_ARGS__)
> > +#define netdev_err_ratelimited(dev, fmt, ...)				\
> > +	netdev_level_ratelimited(netdev_err, dev, fmt, ##__VA_ARGS__)
> > +#define netdev_warn_ratelimited(dev, fmt, ...)				\
> > +	netdev_level_ratelimited(netdev_warn, dev, fmt, ##__VA_ARGS__)
> > +#define netdev_notice_ratelimited(dev, fmt, ...)			\
> > +	netdev_level_ratelimited(netdev_notice, dev, fmt, ##__VA_ARGS__)
> > +#define netdev_info_ratelimited(dev, fmt, ...)				\
> > +	netdev_level_ratelimited(netdev_info, dev, fmt, ##__VA_ARGS__)
> > +
> > +#if defined(CONFIG_DYNAMIC_DEBUG)
> > +/* descriptor check is first to prevent flooding with "callbacks suppressed" */
> > +#define netdev_dbg_ratelimited(dev, fmt, ...)				\
> > +do {									\
> > +	static DEFINE_RATELIMIT_STATE(_rs,				\
> > +				      DEFAULT_RATELIMIT_INTERVAL,	\
> > +				      DEFAULT_RATELIMIT_BURST);		\
> > +	DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);			\
> > +	if (DYNAMIC_DEBUG_BRANCH(descriptor) &&				\
> > +	    __ratelimit(&_rs))						\
> > +		__dynamic_netdev_dbg(&descriptor, dev, fmt,		\
> > +				     ##__VA_ARGS__);			\
> > +} while (0)
> > +#elif defined(DEBUG)
> > +#define netdev_dbg_ratelimited(dev, fmt, ...)				\
> > +do {									\
> > +	static DEFINE_RATELIMIT_STATE(_rs,				\
> > +				      DEFAULT_RATELIMIT_INTERVAL,	\
> > +				      DEFAULT_RATELIMIT_BURST);		\
> > +	if (__ratelimit(&_rs))						\
> > +		netdev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__);	\
> > +} while (0)
> > +#else
> > +#define netdev_dbg_ratelimited(dev, fmt, ...)				\
> > +do {									\
> > +	if (0)								\
> > +		netdev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__);	\
> > +} while (0)
> > +#endif
> > +
> >  #define MODULE_ALIAS_NETDEV(device) \
> >  	MODULE_ALIAS("netdev-" device)
> >  
> > 
> > 

Re: [PATCH net] ibmveth: use net_err_ratelimited when set_multicast_list

[David Miller]

From: Hangbin Liu <liuhangbin@gmail.com>
Date: Thu,  1 Aug 2019 17:03:47 +0800

> When setting lots of multicast list on ibmveth, e.g. add 3000 membership on a
> multicast group, the following error message flushes our log file
> 
> 8507    [  901.478251] ibmveth 30000003 env3: h_multicast_ctrl rc=4 when adding an entry to the filter table
> ...
> 1718386 [ 5636.808658] ibmveth 30000003 env3: h_multicast_ctrl rc=4 when adding an entry to the filter table
> 
> We got 1.5 million lines of messages in 1.3h. Let's replace netdev_err() by
> net_err_ratelimited() to avoid this issue. I don't use netdev_err_once() in
> case h_multicast_ctrl() return different lpar_rc types.
> 
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

Let's work on fixing what causes this problem, or adding a retry
mechanism, rather than making the message less painful.

What is worse is that these failures are not in any way communicated
back up the callchain to show that the operation didn't complete
sucessfully.

This is a real mess in behavior and error handling, don't make it
worse please.

Re: [PATCH net] ibmveth: use net_err_ratelimited when set_multicast_list

[Joe Perches]

On Thu, 2019-08-01 at 12:51 -0400, David Miller wrote:
> From: Hangbin Liu <liuhangbin@gmail.com>
> Date: Thu,  1 Aug 2019 17:03:47 +0800
> 
> > When setting lots of multicast list on ibmveth, e.g. add 3000 membership on a
> > multicast group, the following error message flushes our log file
> > 
> > 8507    [  901.478251] ibmveth 30000003 env3: h_multicast_ctrl rc=4 when adding an entry to the filter table
> > ...
> > 1718386 [ 5636.808658] ibmveth 30000003 env3: h_multicast_ctrl rc=4 when adding an entry to the filter table
> > 
> > We got 1.5 million lines of messages in 1.3h. Let's replace netdev_err() by
> > net_err_ratelimited() to avoid this issue. I don't use netdev_err_once() in
> > case h_multicast_ctrl() return different lpar_rc types.
> > 
> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> 
> Let's work on fixing what causes this problem, or adding a retry
> mechanism, rather than making the message less painful.
> 
> What is worse is that these failures are not in any way communicated
> back up the callchain to show that the operation didn't complete
> sucessfully.
> 
> This is a real mess in behavior and error handling, don't make it
> worse please.

That looks as though it could be quite a chore.

$ git grep -P '\bndo_set_rx_mode\s*=' | wc -l
326

Re: [PATCH net] ibmveth: use net_err_ratelimited when set_multicast_list

[Hangbin Liu]

On Thu, Aug 01, 2019 at 12:51:14PM -0400, David Miller wrote:
> From: Hangbin Liu <liuhangbin@gmail.com>
> Date: Thu,  1 Aug 2019 17:03:47 +0800
> 
> > When setting lots of multicast list on ibmveth, e.g. add 3000 membership on a
> > multicast group, the following error message flushes our log file
> > 
> > 8507    [  901.478251] ibmveth 30000003 env3: h_multicast_ctrl rc=4 when adding an entry to the filter table
> > ...
> > 1718386 [ 5636.808658] ibmveth 30000003 env3: h_multicast_ctrl rc=4 when adding an entry to the filter table
> > 
> > We got 1.5 million lines of messages in 1.3h. Let's replace netdev_err() by
> > net_err_ratelimited() to avoid this issue. I don't use netdev_err_once() in
> > case h_multicast_ctrl() return different lpar_rc types.
> > 
> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> 
> Let's work on fixing what causes this problem, or adding a retry
> mechanism, rather than making the message less painful.

Yes, the multicast issue should also be fixed. It looks more like a
driver issue as I haven't seen it on other drivers.

I think these should be two different fixes.

Thanks
Hangbin
> 
> What is worse is that these failures are not in any way communicated
> back up the callchain to show that the operation didn't complete
> sucessfully.
> 
> This is a real mess in behavior and error handling, don't make it
> worse please.

[PATCHv2 net 0/2] Add netdev_level_ratelimited to avoid netdev msg flush

[Hangbin Liu]

This patch set add netdev_level_ratelimited to avoid netdev msg flush.
The second patch fixed ibmveth msg flush when add lots of(e.g. 2000) group
memberships in one group at the same time.

In my testing, there will be the

ibmveth 30000003 env3: h_multicast_ctrl rc=4 when adding an entry to the filter table

error when add more thann 256 memberships in one multicast group. I haven't
found this issue on other driver. It looks like an ibm driver issue and need
to be fixed separately.

v2: add netdev_level_ratelimited as Joe Perches suggested

Hangbin Liu (2):
  netdevice.h: add netdev_level_ratelimited for netdevice
  ibmveth: use net_err_ratelimited when set_multicast_list

 drivers/net/ethernet/ibm/ibmveth.c |  5 ++-
 include/linux/netdevice.h          | 53 ++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 3 deletions(-)

-- 
2.19.2

[PATCHv2 net 1/2] netdevice.h: add netdev_level_ratelimited for netdevice

[Hangbin Liu]

Add netdev_level_ratelimited so we can use it in the future.
The code is copied from device.h.

Suggested-by: Joe Perches <joe@perches.com>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 include/linux/netdevice.h | 53 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 88292953aa6f..4e37065c6717 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4737,6 +4737,59 @@ do {								\
 #define netdev_info_once(dev, fmt, ...) \
 	netdev_level_once(KERN_INFO, dev, fmt, ##__VA_ARGS__)
 
+#define netdev_level_ratelimited(netdev_level, dev, fmt, ...)		\
+do {									\
+	static DEFINE_RATELIMIT_STATE(_rs,				\
+				      DEFAULT_RATELIMIT_INTERVAL,	\
+				      DEFAULT_RATELIMIT_BURST);		\
+	if (__ratelimit(&_rs))						\
+		netdev_level(dev, fmt, ##__VA_ARGS__);			\
+} while (0)
+
+#define netdev_emerg_ratelimited(dev, fmt, ...)				\
+	netdev_level_ratelimited(netdev_emerg, dev, fmt, ##__VA_ARGS__)
+#define netdev_alert_ratelimited(dev, fmt, ...)				\
+	netdev_level_ratelimited(netdev_alert, dev, fmt, ##__VA_ARGS__)
+#define netdev_crit_ratelimited(dev, fmt, ...)				\
+	netdev_level_ratelimited(netdev_crit, dev, fmt, ##__VA_ARGS__)
+#define netdev_err_ratelimited(dev, fmt, ...)				\
+	netdev_level_ratelimited(netdev_err, dev, fmt, ##__VA_ARGS__)
+#define netdev_warn_ratelimited(dev, fmt, ...)				\
+	netdev_level_ratelimited(netdev_warn, dev, fmt, ##__VA_ARGS__)
+#define netdev_notice_ratelimited(dev, fmt, ...)			\
+	netdev_level_ratelimited(netdev_notice, dev, fmt, ##__VA_ARGS__)
+#define netdev_info_ratelimited(dev, fmt, ...)				\
+	netdev_level_ratelimited(netdev_info, dev, fmt, ##__VA_ARGS__)
+#if defined(CONFIG_DYNAMIC_DEBUG)
+/* descriptor check is first to prevent flooding with "callbacks suppressed" */
+#define netdev_dbg_ratelimited(dev, fmt, ...)				\
+do {									\
+	static DEFINE_RATELIMIT_STATE(_rs,				\
+				      DEFAULT_RATELIMIT_INTERVAL,	\
+				      DEFAULT_RATELIMIT_BURST);		\
+	DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);			\
+	if (DYNAMIC_DEBUG_BRANCH(descriptor) &&				\
+	    __ratelimit(&_rs))						\
+		__dynamic_netdev_dbg(&descriptor, dev, dev_fmt(fmt),	\
+				     ##__VA_ARGS__);			\
+} while (0)
+#elif defined(DEBUG)
+#define netdev_dbg_ratelimited(dev, fmt, ...)				\
+do {									\
+	static DEFINE_RATELIMIT_STATE(_rs,				\
+				      DEFAULT_RATELIMIT_INTERVAL,	\
+				      DEFAULT_RATELIMIT_BURST);		\
+	if (__ratelimit(&_rs))						\
+		netdev_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__); \
+} while (0)
+#else
+#define netdev_dbg_ratelimited(dev, fmt, ...)				\
+do {									\
+	if (0)								\
+		netdev_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__); \
+} while (0)
+#endif
+
 #define MODULE_ALIAS_NETDEV(device) \
 	MODULE_ALIAS("netdev-" device)
 
-- 
2.19.2

[PATCHv2 net 2/2] ibmveth: use netdev_err_ratelimited when set_multicast_list

[Hangbin Liu]

When add lots of (e.g. add 3000) memberships in one multicast group on
ibmveth, the following error message flushes our console log file

8507    [  901.478251] ibmveth 30000003 env3: h_multicast_ctrl rc=4 when adding an entry to the filter table
...
1718386 [ 5636.808658] ibmveth 30000003 env3: h_multicast_ctrl rc=4 when adding an entry to the filter table

We got 1.5 million lines of messages in 1.3h. Let's replace netdev_err() by
netdev_err_ratelimited() to avoid this issue.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 drivers/net/ethernet/ibm/ibmveth.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
index d654c234aaf7..138523ee5e84 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -1446,9 +1446,8 @@ static void ibmveth_set_multicast_list(struct net_device *netdev)
 						   IbmVethMcastAddFilter,
 						   mcast_addr);
 			if (lpar_rc != H_SUCCESS) {
-				netdev_err(netdev, "h_multicast_ctrl rc=%ld "
-					   "when adding an entry to the filter "
-					   "table\n", lpar_rc);
+				netdev_err_ratelimited(netdev, "h_multicast_ctrl rc=%ld when adding an entry to the filter table\n",
+						       lpar_rc);
 			}
 		}
 
-- 
2.19.2

Re: [PATCHv2 net 0/2] Add netdev_level_ratelimited to avoid netdev msg flush

[David Miller]

From: Hangbin Liu <liuhangbin@gmail.com>
Date: Fri,  9 Aug 2019 08:29:39 +0800

> ibmveth 30000003 env3: h_multicast_ctrl rc=4 when adding an entry to the filter table

You need to root cause and fix the reason this message appears so much.

Once I let you rate limit the message you will have zero incentive to
fix the real problem and fix it.

Re: [PATCHv2 net 0/2] Add netdev_level_ratelimited to avoid netdev msg flush

[Hangbin Liu]

On Sun, Aug 11, 2019 at 09:08:20PM -0700, David Miller wrote:
> From: Hangbin Liu <liuhangbin@gmail.com>
> Date: Fri,  9 Aug 2019 08:29:39 +0800
> 
> > ibmveth 30000003 env3: h_multicast_ctrl rc=4 when adding an entry to the filter table

> > error when add more thann 256 memberships in one multicast group. I haven't
> > found this issue on other driver. It looks like an ibm driver issue and need
> > to be fixed separately.
> 
> You need to root cause and fix the reason this message appears so much.
> 
> Once I let you rate limit the message you will have zero incentive to
> fix the real problem and fix it.

Sorry, I'm not familiar with ibmveth driver...

Hi Thomas,

Would you please help check why this issue happens

Thanks
Hangbbin

Re: [PATCHv2 net 0/2] Add netdev_level_ratelimited to avoid netdev msg flush

[Thomas Falcon]

On 8/12/19 2:37 AM, Hangbin Liu wrote:
> On Sun, Aug 11, 2019 at 09:08:20PM -0700, David Miller wrote:
>> From: Hangbin Liu <liuhangbin@gmail.com>
>> Date: Fri,  9 Aug 2019 08:29:39 +0800
>>
>>> ibmveth 30000003 env3: h_multicast_ctrl rc=4 when adding an entry to the filter table
>>> error when add more thann 256 memberships in one multicast group. I haven't
>>> found this issue on other driver. It looks like an ibm driver issue and need
>>> to be fixed separately.
>> You need to root cause and fix the reason this message appears so much.
>>
>> Once I let you rate limit the message you will have zero incentive to
>> fix the real problem and fix it.
> Sorry, I'm not familiar with ibmveth driver...
>
> Hi Thomas,
>
> Would you please help check why this issue happens


Hi, thanks for reporting this. I was able to recreate this on my own 
system. The virtual ethernet's multicast filter list size is limited, 
and the driver will check that there is available space before adding 
entries.  The problem is that the size is encoded as big endian, but the 
driver does not convert it for little endian systems after retrieving it 
from the device tree.  As a result the driver is requesting more than 
the hypervisor can allow and getting this error in reply. I will submit 
a patch to correct this soon.

Thanks again,

Tom

> Thanks
> Hangbbin
>

Re: [PATCHv2 net 0/2] Add netdev_level_ratelimited to avoid netdev msg flush

[David Miller]

From: Thomas Falcon <tlfalcon@linux.ibm.com>
Date: Mon, 12 Aug 2019 10:56:39 -0500

> Hi, thanks for reporting this. I was able to recreate this on my own
> system. The virtual ethernet's multicast filter list size is limited,
> and the driver will check that there is available space before adding
> entries.  The problem is that the size is encoded as big endian, but
> the driver does not convert it for little endian systems after
> retrieving it from the device tree.  As a result the driver is
> requesting more than the hypervisor can allow and getting this error
> in reply. I will submit a patch to correct this soon.

This is 1,000 times better than just trying to make the warning message
go away, thanks Thomas!