diff mbox series

[ovs-dev] ofproto-dpif-upcall: Log the value of flow limit.

Message ID 20200915192057.3543735-1-fbl@sysclose.org
State Changes Requested
Headers show
Series [ovs-dev] ofproto-dpif-upcall: Log the value of flow limit. | expand

Commit Message

Flavio Leitner Sept. 15, 2020, 7:20 p.m. UTC
The datapath flow limit is calculated by revalidators so
log the value as well.

Signed-off-by: Flavio Leitner <fbl@sysclose.org>
---
 ofproto/ofproto-dpif-upcall.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Eelco Chaudron Sept. 16, 2020, 12:23 p.m. UTC | #1
On 15 Sep 2020, at 21:20, Flavio Leitner wrote:

> The datapath flow limit is calculated by revalidators so
> log the value as well.
>
> Signed-off-by: Flavio Leitner <fbl@sysclose.org>

Acked-by: Eelco Chaudron <echaudro@redhat.com>
Aaron Conole Sept. 22, 2020, 12:27 p.m. UTC | #2
Flavio Leitner <fbl@sysclose.org> writes:

> The datapath flow limit is calculated by revalidators so
> log the value as well.
>
> Signed-off-by: Flavio Leitner <fbl@sysclose.org>
> ---

Acked-by: Aaron Conole <aconole@redhat.com>
Ilya Maximets Sept. 29, 2020, 2:53 p.m. UTC | #3
On 9/15/20 9:20 PM, Flavio Leitner wrote:
> The datapath flow limit is calculated by revalidators so
> log the value as well.
> 
> Signed-off-by: Flavio Leitner <fbl@sysclose.org>
> ---
>  ofproto/ofproto-dpif-upcall.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index 72a5b4d73..05a912f57 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -1283,7 +1283,8 @@ should_install_flow(struct udpif *udpif, struct upcall *upcall)
>      atomic_read_relaxed(&udpif->flow_limit, &flow_limit);
>      if (udpif_get_n_flows(udpif) >= flow_limit) {
>          COVERAGE_INC(upcall_flow_limit_hit);
> -        VLOG_WARN_RL(&rl, "upcall: datapath flow limit reached");
> +        VLOG_WARN_RL(&rl, "upcall: datapath flow limit reached (max: %u)",

I think that we're not printing this value just because it's dynamically
adjustable by revalidators.  From that perspective, I don't think that
'max' is a good word to describe the value, because users might think
that they have a direct control over it and might think that it's the
value set by the 'flow_limit' configuration knob.
'current dynamic limit' or something similar might be better choice, but
I'm not sure.

Best regards, Ilya Maximets.
Flavio Leitner Sept. 29, 2020, 8:09 p.m. UTC | #4
On Tue, Sep 29, 2020 at 04:53:22PM +0200, Ilya Maximets wrote:
> On 9/15/20 9:20 PM, Flavio Leitner wrote:
> > The datapath flow limit is calculated by revalidators so
> > log the value as well.
> > 
> > Signed-off-by: Flavio Leitner <fbl@sysclose.org>
> > ---
> >  ofproto/ofproto-dpif-upcall.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> > index 72a5b4d73..05a912f57 100644
> > --- a/ofproto/ofproto-dpif-upcall.c
> > +++ b/ofproto/ofproto-dpif-upcall.c
> > @@ -1283,7 +1283,8 @@ should_install_flow(struct udpif *udpif, struct upcall *upcall)
> >      atomic_read_relaxed(&udpif->flow_limit, &flow_limit);
> >      if (udpif_get_n_flows(udpif) >= flow_limit) {
> >          COVERAGE_INC(upcall_flow_limit_hit);
> > -        VLOG_WARN_RL(&rl, "upcall: datapath flow limit reached");
> > +        VLOG_WARN_RL(&rl, "upcall: datapath flow limit reached (max: %u)",
> 
> I think that we're not printing this value just because it's dynamically
> adjustable by revalidators.  From that perspective, I don't think that
> 'max' is a good word to describe the value, because users might think
> that they have a direct control over it and might think that it's the
> value set by the 'flow_limit' configuration knob.
> 'current dynamic limit' or something similar might be better choice, but
> I'm not sure.

Sent v2:
https://mail.openvswitch.org/pipermail/ovs-dev/2020-September/375646.html
diff mbox series

Patch

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 72a5b4d73..05a912f57 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1283,7 +1283,8 @@  should_install_flow(struct udpif *udpif, struct upcall *upcall)
     atomic_read_relaxed(&udpif->flow_limit, &flow_limit);
     if (udpif_get_n_flows(udpif) >= flow_limit) {
         COVERAGE_INC(upcall_flow_limit_hit);
-        VLOG_WARN_RL(&rl, "upcall: datapath flow limit reached");
+        VLOG_WARN_RL(&rl, "upcall: datapath flow limit reached (max: %u)",
+                     flow_limit);
         return false;
     }