* [PATCH net] ibmveth: use net_err_ratelimited when set_multicast_list
@ 2019-08-01 9:03 Hangbin Liu
2019-08-01 10:28 ` Joe Perches
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Hangbin Liu @ 2019-08-01 9:03 UTC (permalink / raw)
To: netdev; +Cc: Thomas Falcon, David S . Miller, 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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH net] ibmveth: use net_err_ratelimited when set_multicast_list
2019-08-01 9:03 [PATCH net] ibmveth: use net_err_ratelimited when set_multicast_list Hangbin Liu
@ 2019-08-01 10:28 ` Joe Perches
2019-08-01 14:10 ` Hangbin Liu
2019-08-01 16:51 ` David Miller
2019-08-09 0:29 ` [PATCHv2 net 0/2] Add netdev_level_ratelimited to avoid netdev msg flush Hangbin Liu
2 siblings, 1 reply; 14+ messages in thread
From: Joe Perches @ 2019-08-01 10:28 UTC (permalink / raw)
To: Hangbin Liu, netdev; +Cc: Thomas Falcon, David S . Miller
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)
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH net] ibmveth: use net_err_ratelimited when set_multicast_list
2019-08-01 10:28 ` Joe Perches
@ 2019-08-01 14:10 ` Hangbin Liu
2019-08-01 17:54 ` Joe Perches
0 siblings, 1 reply; 14+ messages in thread
From: Hangbin Liu @ 2019-08-01 14:10 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev, Thomas Falcon, David S . Miller
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)
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net] ibmveth: use net_err_ratelimited when set_multicast_list
2019-08-01 9:03 [PATCH net] ibmveth: use net_err_ratelimited when set_multicast_list Hangbin Liu
2019-08-01 10:28 ` Joe Perches
@ 2019-08-01 16:51 ` David Miller
2019-08-01 21:34 ` Joe Perches
2019-08-02 9:53 ` Hangbin Liu
2019-08-09 0:29 ` [PATCHv2 net 0/2] Add netdev_level_ratelimited to avoid netdev msg flush Hangbin Liu
2 siblings, 2 replies; 14+ messages in thread
From: David Miller @ 2019-08-01 16:51 UTC (permalink / raw)
To: liuhangbin; +Cc: netdev, tlfalcon
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net] ibmveth: use net_err_ratelimited when set_multicast_list
2019-08-01 14:10 ` Hangbin Liu
@ 2019-08-01 17:54 ` Joe Perches
0 siblings, 0 replies; 14+ messages in thread
From: Joe Perches @ 2019-08-01 17:54 UTC (permalink / raw)
To: Hangbin Liu; +Cc: netdev, Thomas Falcon, David S . Miller
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)
> >
> >
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net] ibmveth: use net_err_ratelimited when set_multicast_list
2019-08-01 16:51 ` David Miller
@ 2019-08-01 21:34 ` Joe Perches
2019-08-02 9:53 ` Hangbin Liu
1 sibling, 0 replies; 14+ messages in thread
From: Joe Perches @ 2019-08-01 21:34 UTC (permalink / raw)
To: David Miller, liuhangbin; +Cc: netdev, tlfalcon
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net] ibmveth: use net_err_ratelimited when set_multicast_list
2019-08-01 16:51 ` David Miller
2019-08-01 21:34 ` Joe Perches
@ 2019-08-02 9:53 ` Hangbin Liu
1 sibling, 0 replies; 14+ messages in thread
From: Hangbin Liu @ 2019-08-02 9:53 UTC (permalink / raw)
To: David Miller; +Cc: netdev, tlfalcon
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCHv2 net 0/2] Add netdev_level_ratelimited to avoid netdev msg flush
2019-08-01 9:03 [PATCH net] ibmveth: use net_err_ratelimited when set_multicast_list Hangbin Liu
2019-08-01 10:28 ` Joe Perches
2019-08-01 16:51 ` David Miller
@ 2019-08-09 0:29 ` Hangbin Liu
2019-08-09 0:29 ` [PATCHv2 net 1/2] netdevice.h: add netdev_level_ratelimited for netdevice Hangbin Liu
` (2 more replies)
2 siblings, 3 replies; 14+ messages in thread
From: Hangbin Liu @ 2019-08-09 0:29 UTC (permalink / raw)
To: netdev; +Cc: Joe Perches, Thomas Falcon, David S . Miller, 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCHv2 net 1/2] netdevice.h: add netdev_level_ratelimited for netdevice
2019-08-09 0:29 ` [PATCHv2 net 0/2] Add netdev_level_ratelimited to avoid netdev msg flush Hangbin Liu
@ 2019-08-09 0:29 ` Hangbin Liu
2019-08-09 0:29 ` [PATCHv2 net 2/2] ibmveth: use netdev_err_ratelimited when set_multicast_list Hangbin Liu
2019-08-12 4:08 ` [PATCHv2 net 0/2] Add netdev_level_ratelimited to avoid netdev msg flush David Miller
2 siblings, 0 replies; 14+ messages in thread
From: Hangbin Liu @ 2019-08-09 0:29 UTC (permalink / raw)
To: netdev; +Cc: Joe Perches, Thomas Falcon, David S . Miller, 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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCHv2 net 2/2] ibmveth: use netdev_err_ratelimited when set_multicast_list
2019-08-09 0:29 ` [PATCHv2 net 0/2] Add netdev_level_ratelimited to avoid netdev msg flush Hangbin Liu
2019-08-09 0:29 ` [PATCHv2 net 1/2] netdevice.h: add netdev_level_ratelimited for netdevice Hangbin Liu
@ 2019-08-09 0:29 ` Hangbin Liu
2019-08-12 4:08 ` [PATCHv2 net 0/2] Add netdev_level_ratelimited to avoid netdev msg flush David Miller
2 siblings, 0 replies; 14+ messages in thread
From: Hangbin Liu @ 2019-08-09 0:29 UTC (permalink / raw)
To: netdev; +Cc: Joe Perches, Thomas Falcon, David S . Miller, 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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCHv2 net 0/2] Add netdev_level_ratelimited to avoid netdev msg flush
2019-08-09 0:29 ` [PATCHv2 net 0/2] Add netdev_level_ratelimited to avoid netdev msg flush Hangbin Liu
2019-08-09 0:29 ` [PATCHv2 net 1/2] netdevice.h: add netdev_level_ratelimited for netdevice Hangbin Liu
2019-08-09 0:29 ` [PATCHv2 net 2/2] ibmveth: use netdev_err_ratelimited when set_multicast_list Hangbin Liu
@ 2019-08-12 4:08 ` David Miller
2019-08-12 7:37 ` Hangbin Liu
2 siblings, 1 reply; 14+ messages in thread
From: David Miller @ 2019-08-12 4:08 UTC (permalink / raw)
To: liuhangbin; +Cc: netdev, joe, tlfalcon
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv2 net 0/2] Add netdev_level_ratelimited to avoid netdev msg flush
2019-08-12 4:08 ` [PATCHv2 net 0/2] Add netdev_level_ratelimited to avoid netdev msg flush David Miller
@ 2019-08-12 7:37 ` Hangbin Liu
2019-08-12 15:56 ` Thomas Falcon
0 siblings, 1 reply; 14+ messages in thread
From: Hangbin Liu @ 2019-08-12 7:37 UTC (permalink / raw)
To: David Miller; +Cc: netdev, joe, tlfalcon
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv2 net 0/2] Add netdev_level_ratelimited to avoid netdev msg flush
2019-08-12 7:37 ` Hangbin Liu
@ 2019-08-12 15:56 ` Thomas Falcon
2019-08-12 18:27 ` David Miller
0 siblings, 1 reply; 14+ messages in thread
From: Thomas Falcon @ 2019-08-12 15:56 UTC (permalink / raw)
To: Hangbin Liu, David Miller; +Cc: netdev, joe
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
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv2 net 0/2] Add netdev_level_ratelimited to avoid netdev msg flush
2019-08-12 15:56 ` Thomas Falcon
@ 2019-08-12 18:27 ` David Miller
0 siblings, 0 replies; 14+ messages in thread
From: David Miller @ 2019-08-12 18:27 UTC (permalink / raw)
To: tlfalcon; +Cc: liuhangbin, netdev, joe
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!
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-08-12 18:27 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-01 9:03 [PATCH net] ibmveth: use net_err_ratelimited when set_multicast_list Hangbin Liu
2019-08-01 10:28 ` Joe Perches
2019-08-01 14:10 ` Hangbin Liu
2019-08-01 17:54 ` Joe Perches
2019-08-01 16:51 ` David Miller
2019-08-01 21:34 ` Joe Perches
2019-08-02 9:53 ` Hangbin Liu
2019-08-09 0:29 ` [PATCHv2 net 0/2] Add netdev_level_ratelimited to avoid netdev msg flush Hangbin Liu
2019-08-09 0:29 ` [PATCHv2 net 1/2] netdevice.h: add netdev_level_ratelimited for netdevice Hangbin Liu
2019-08-09 0:29 ` [PATCHv2 net 2/2] ibmveth: use netdev_err_ratelimited when set_multicast_list Hangbin Liu
2019-08-12 4:08 ` [PATCHv2 net 0/2] Add netdev_level_ratelimited to avoid netdev msg flush David Miller
2019-08-12 7:37 ` Hangbin Liu
2019-08-12 15:56 ` Thomas Falcon
2019-08-12 18:27 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).