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 |
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>
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>
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.
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 --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; }
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(-)