I see this change fixing a commit b4c632526b pushed a few years back, but
still there is room for instrumentation like below. Could this patch be
reviewed ?.

Thanks,
Gowrishankar

On Mon, Apr 20, 2020 at 7:13 PM Gowrishankar Muthukrishnan <
[email protected]> wrote:

> Whenever the number of flows in the datapath crosses above
> the flow limit set/autoconfigured, it is helpful to report
> this event through coverage counter for an operator/devops
> engineer to know and take proactive corrections in the
> switch configuration.
>
> Today, these events are reported in ovs vswitch log when
> a new flow can not be inserted in upcall processing in which
> case ovs writes a warning, otherwise an auto correction
> made by ovs to flush old flows without any intimation at all.
>
> Signed-off-by: Gowrishankar Muthukrishnan <[email protected]>
> ---
>  ofproto/ofproto-dpif-upcall.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index 5e08ef10d..a76532ec7 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -56,6 +56,7 @@ COVERAGE_DEFINE(handler_duplicate_upcall);
>  COVERAGE_DEFINE(upcall_ukey_contention);
>  COVERAGE_DEFINE(upcall_ukey_replace);
>  COVERAGE_DEFINE(revalidate_missed_dp_flow);
> +COVERAGE_DEFINE(upcall_flow_limit_hit);
>
>  /* A thread that reads upcalls from dpif, forwards each upcall's packet,
>   * and possibly sets up a kernel flow as a cache. */
> @@ -1281,6 +1282,7 @@ 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");
>          return false;
>      }
> @@ -2624,6 +2626,10 @@ revalidate(struct revalidator *revalidator)
>           *       datapath flows, so we will recover before all the flows
> are
>           *       gone.) */
>          n_dp_flows = udpif_get_n_flows(udpif);
> +        if (n_dp_flows >= flow_limit) {
> +            COVERAGE_INC(upcall_flow_limit_hit);
> +        }
> +
>          kill_them_all = n_dp_flows > flow_limit * 2;
>          max_idle = n_dp_flows > flow_limit ? 100 : ofproto_max_idle;
>
> --
> 2.21.1
>
>

-- 
Gowrishankar M
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to