Re: [ovs-dev] [PATCH] vswitch: ratelimit the device add log

2019-09-16 Thread Aaron Conole
Ben Pfaff  writes:

> On Thu, Sep 12, 2019 at 11:45:41AM -0400, Aaron Conole wrote:
>> It's possible that a port added to the system with certain kinds
>> of invalid parameters will cause the 'could not add' log to be
>> triggered.  When this happens, the vswitch run loop can continually
>> re-attempt adding the port.  While the parameters remain invalid
>> the vswitch run loop will re-trigger the warning, flooding the
>> syslog.
>> 
>> This patch adds a simple rate limit to the log.
>> 
>> Signed-off-by: Aaron Conole 
>> ---
>>  vswitchd/bridge.c | 8 ++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>> 
>> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
>> index d921c4ef8..49a6f6a37 100644
>> --- a/vswitchd/bridge.c
>> +++ b/vswitchd/bridge.c
>> @@ -1816,8 +1816,12 @@ iface_do_create(const struct bridge *br,
>>  *ofp_portp = iface_pick_ofport(iface_cfg);
>>  error = ofproto_port_add(br->ofproto, netdev, ofp_portp);
>>  if (error) {
>> -VLOG_WARN_BUF(errp, "could not add network device %s to ofproto 
>> (%s)",
>> -  iface_cfg->name, ovs_strerror(error));
>> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
>> +if (!VLOG_DROP_WARN()) {
>> +VLOG_WARN_BUF(errp,
>> +  "could not add network device %s to ofproto (%s)",
>> +  iface_cfg->name, ovs_strerror(error));
>> +}
>>  goto error;
>>  }
>
> I understand why we want to rate-limit what's going to the log.
>
> We don't want to rate-limit the text being put into errp, though,
> because that goes to the database record for that particular interface
> to indicate why it's malfunctioning.  It should keep the error message
> regardless of how much it recurs.
>
> Can you come up with a way to handle both of these requirements
> gracefully?

Makes sense.  Sure.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] vswitch: ratelimit the device add log

2019-09-13 Thread Ben Pfaff
On Thu, Sep 12, 2019 at 11:45:41AM -0400, Aaron Conole wrote:
> It's possible that a port added to the system with certain kinds
> of invalid parameters will cause the 'could not add' log to be
> triggered.  When this happens, the vswitch run loop can continually
> re-attempt adding the port.  While the parameters remain invalid
> the vswitch run loop will re-trigger the warning, flooding the
> syslog.
> 
> This patch adds a simple rate limit to the log.
> 
> Signed-off-by: Aaron Conole 
> ---
>  vswitchd/bridge.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index d921c4ef8..49a6f6a37 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -1816,8 +1816,12 @@ iface_do_create(const struct bridge *br,
>  *ofp_portp = iface_pick_ofport(iface_cfg);
>  error = ofproto_port_add(br->ofproto, netdev, ofp_portp);
>  if (error) {
> -VLOG_WARN_BUF(errp, "could not add network device %s to ofproto 
> (%s)",
> -  iface_cfg->name, ovs_strerror(error));
> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> +if (!VLOG_DROP_WARN()) {
> +VLOG_WARN_BUF(errp,
> +  "could not add network device %s to ofproto (%s)",
> +  iface_cfg->name, ovs_strerror(error));
> +}
>  goto error;
>  }

I understand why we want to rate-limit what's going to the log.

We don't want to rate-limit the text being put into errp, though,
because that goes to the database record for that particular interface
to indicate why it's malfunctioning.  It should keep the error message
regardless of how much it recurs.

Can you come up with a way to handle both of these requirements
gracefully?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] vswitch: ratelimit the device add log

2019-09-12 Thread William Tu
On Thu, Sep 12, 2019 at 11:45:41AM -0400, Aaron Conole wrote:
> It's possible that a port added to the system with certain kinds
> of invalid parameters will cause the 'could not add' log to be
> triggered.  When this happens, the vswitch run loop can continually
> re-attempt adding the port.  While the parameters remain invalid
> the vswitch run loop will re-trigger the warning, flooding the
> syslog.
> 
> This patch adds a simple rate limit to the log.
> 
> Signed-off-by: Aaron Conole 

LGTM, hit this issue quite often.

Acked-by: William Tu 

> ---
>  vswitchd/bridge.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index d921c4ef8..49a6f6a37 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -1816,8 +1816,12 @@ iface_do_create(const struct bridge *br,
>  *ofp_portp = iface_pick_ofport(iface_cfg);
>  error = ofproto_port_add(br->ofproto, netdev, ofp_portp);
>  if (error) {
> -VLOG_WARN_BUF(errp, "could not add network device %s to ofproto 
> (%s)",
> -  iface_cfg->name, ovs_strerror(error));
> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> +if (!VLOG_DROP_WARN()) {
> +VLOG_WARN_BUF(errp,
> +  "could not add network device %s to ofproto (%s)",
> +  iface_cfg->name, ovs_strerror(error));
> +}
>  goto error;
>  }
>  
> -- 
> 2.21.0
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] vswitch: ratelimit the device add log

2019-09-12 Thread Aaron Conole
It's possible that a port added to the system with certain kinds
of invalid parameters will cause the 'could not add' log to be
triggered.  When this happens, the vswitch run loop can continually
re-attempt adding the port.  While the parameters remain invalid
the vswitch run loop will re-trigger the warning, flooding the
syslog.

This patch adds a simple rate limit to the log.

Signed-off-by: Aaron Conole 
---
 vswitchd/bridge.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index d921c4ef8..49a6f6a37 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1816,8 +1816,12 @@ iface_do_create(const struct bridge *br,
 *ofp_portp = iface_pick_ofport(iface_cfg);
 error = ofproto_port_add(br->ofproto, netdev, ofp_portp);
 if (error) {
-VLOG_WARN_BUF(errp, "could not add network device %s to ofproto (%s)",
-  iface_cfg->name, ovs_strerror(error));
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+if (!VLOG_DROP_WARN()) {
+VLOG_WARN_BUF(errp,
+  "could not add network device %s to ofproto (%s)",
+  iface_cfg->name, ovs_strerror(error));
+}
 goto error;
 }
 
-- 
2.21.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev