On 1/12/26 12:20 PM, Eelco Chaudron wrote:
> This patch moved the flow count APIs to the netdev-offload provider.
>
> Acked-by: Eli Britstein <elibr.nvidia.com>
> Signed-off-by: Eelco Chaudron <[email protected]>
> ---
> lib/dpif-netdev.c | 29 +++++-----------------------
> lib/dpif-offload-dpdk.c | 27 ++++++++++++++++++++++++++
> lib/dpif-offload-provider.h | 3 +++
> lib/dpif-offload-tc.c | 1 +
> lib/dpif-offload.c | 20 +++++++++++++++++++
> lib/dpif-offload.h | 1 +
> lib/dpif.c | 23 ----------------------
> lib/dpif.h | 2 --
> lib/netdev-offload-dpdk.c | 13 ++++++-------
> lib/netdev-offload-dpdk.h | 1 +
> lib/netdev-offload-provider.h | 5 -----
> lib/netdev-offload-tc.c | 18 ++++++------------
> lib/netdev-offload-tc.h | 1 +
> lib/netdev-offload.c | 36 -----------------------------------
> lib/netdev-offload.h | 3 ---
> ofproto/ofproto-dpif-upcall.c | 9 +++------
> 16 files changed, 74 insertions(+), 118 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 79b05bc0d..82df85823 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -4762,11 +4762,8 @@ dpif_netdev_offload_stats_get(struct dpif *dpif,
> [DP_NETDEV_HW_OFFLOADS_STATS_LAT_EMA_STDDEV] =
> { " Exponential Latency stddev (us)", 0 },
> };
> - struct dp_netdev *dp = get_dp_netdev(dpif);
> - struct dp_netdev_port *port;
> +
> unsigned int nb_thread;
> - uint64_t *port_nb_offloads;
> - uint64_t *nb_offloads;
> unsigned int tid;
> size_t i;
>
> @@ -4783,29 +4780,11 @@ dpif_netdev_offload_stats_get(struct dpif *dpif,
> stats->size = ARRAY_SIZE(hwol_stats) * (nb_thread + 1);
> stats->counters = xcalloc(stats->size, sizeof *stats->counters);
>
> - nb_offloads = xcalloc(nb_thread, sizeof *nb_offloads);
> - port_nb_offloads = xcalloc(nb_thread, sizeof *port_nb_offloads);
> -
> - ovs_rwlock_rdlock(&dp->port_rwlock);
> - HMAP_FOR_EACH (port, node, &dp->ports) {
> - memset(port_nb_offloads, 0, nb_thread * sizeof *port_nb_offloads);
> - /* Do not abort on read error from a port, just report 0. */
> - if (!netdev_flow_get_n_flows(port->netdev, port_nb_offloads)) {
> - for (i = 0; i < nb_thread; i++) {
> - nb_offloads[i] += port_nb_offloads[i];
> - }
> - }
> - }
> - ovs_rwlock_unlock(&dp->port_rwlock);
> -
> - free(port_nb_offloads);
> -
> for (tid = 0; tid < nb_thread; tid++) {
> uint64_t counts[ARRAY_SIZE(hwol_stats)];
> size_t idx = ((tid + 1) * ARRAY_SIZE(hwol_stats));
>
> memset(counts, 0, sizeof counts);
> - counts[DP_NETDEV_HW_OFFLOADS_STATS_INSERTED] = nb_offloads[tid];
> if (dp_offload_threads != NULL) {
> atomic_read_relaxed(&dp_offload_threads[tid].enqueued_item,
>
> &counts[DP_NETDEV_HW_OFFLOADS_STATS_ENQUEUED]);
> @@ -4830,14 +4809,16 @@ dpif_netdev_offload_stats_get(struct dpif *dpif,
> }
> }
>
> - free(nb_offloads);
> -
> /* Do an average of the average for the aggregate. */
> hwol_stats[DP_NETDEV_HW_OFFLOADS_STATS_LAT_CMA_MEAN].total /= nb_thread;
> hwol_stats[DP_NETDEV_HW_OFFLOADS_STATS_LAT_CMA_STDDEV].total /=
> nb_thread;
> hwol_stats[DP_NETDEV_HW_OFFLOADS_STATS_LAT_EMA_MEAN].total /= nb_thread;
> hwol_stats[DP_NETDEV_HW_OFFLOADS_STATS_LAT_EMA_STDDEV].total /=
> nb_thread;
>
> + /* Get the total offload count. */
> + hwol_stats[DP_NETDEV_HW_OFFLOADS_STATS_INSERTED].total =
> + dpif_offload_flow_get_n_offloaded(dpif);
> +
> for (i = 0; i < ARRAY_SIZE(hwol_stats); i++) {
> snprintf(stats->counters[i].name, sizeof(stats->counters[i].name),
> " Total %s", hwol_stats[i].name);
> diff --git a/lib/dpif-offload-dpdk.c b/lib/dpif-offload-dpdk.c
> index ed0b5bec9..2ded4111d 100644
> --- a/lib/dpif-offload-dpdk.c
> +++ b/lib/dpif-offload-dpdk.c
> @@ -255,6 +255,32 @@ dpif_offload_dpdk_can_offload(struct dpif_offload
> *offload OVS_UNUSED,
> return netdev_dpdk_flow_api_supported(netdev, true);
> }
>
> +static bool
> +dpif_offload_dpdk_get_n_offloaded_cb(
> + struct dpif_offload_port_mgr_port *port, void *aux)
> +{
> + uint64_t *total = aux;
> +
> + *total += netdev_offload_dpdk_flow_get_n_offloaded(port->netdev);
> + return false;
> +}
> +
> +static uint64_t
> +dpif_offload_dpdk_get_n_offloaded(const struct dpif_offload *offload_)
> +{
> + struct dpif_offload_dpdk *offload = dpif_offload_dpdk_cast(offload_);
> + uint64_t total = 0;
> +
> + if (!dpif_offload_is_offload_enabled()) {
> + return 0;
> + }
> +
> + dpif_offload_port_mgr_traverse_ports(
> + offload->port_mgr, dpif_offload_dpdk_get_n_offloaded_cb, &total);
> +
> + return total;
> +}
> +
> static int
> dpif_offload_dpdk_netdev_flow_flush(const struct dpif_offload *offload
> OVS_UNUSED, struct netdev *netdev)
> @@ -274,6 +300,7 @@ struct dpif_offload_class dpif_offload_dpdk_class = {
> .can_offload = dpif_offload_dpdk_can_offload,
> .port_add = dpif_offload_dpdk_port_add,
> .port_del = dpif_offload_dpdk_port_del,
> + .flow_get_n_offloaded = dpif_offload_dpdk_get_n_offloaded,
> .netdev_flow_flush = dpif_offload_dpdk_netdev_flow_flush,
> };
>
> diff --git a/lib/dpif-offload-provider.h b/lib/dpif-offload-provider.h
> index f49443758..5e403894e 100644
> --- a/lib/dpif-offload-provider.h
> +++ b/lib/dpif-offload-provider.h
> @@ -132,6 +132,9 @@ struct dpif_offload_class {
> * successful, otherwise returns a positive errno value. */
> int (*flow_flush)(const struct dpif_offload *);
>
> + /* Returns the number of flows offloaded by the offload provider. */
> + uint64_t (*flow_get_n_offloaded)(const struct dpif_offload *);
The name is a little strange for a class member. Should we maybe call
it just .get_n_flows() or .flow_count() ? Since we're asking the
offload provider and not the general dpif implementation it's expected
that all the flows in there are offloaded.
> +
> /* Adds or modifies the meter in 'dpif_offload' with the given 'meter_id'
> * and the configuration in 'config'.
> *
> diff --git a/lib/dpif-offload-tc.c b/lib/dpif-offload-tc.c
> index 70f4f3c97..9e8da207c 100644
> --- a/lib/dpif-offload-tc.c
> +++ b/lib/dpif-offload-tc.c
> @@ -294,6 +294,7 @@ struct dpif_offload_class dpif_offload_tc_class = {
> .port_add = dpif_offload_tc_port_add,
> .port_del = dpif_offload_tc_port_del,
> .flow_flush = dpif_offload_tc_flow_flush,
> + .flow_get_n_offloaded = dpif_offload_tc_flow_get_n_offloaded,
> .meter_set = dpif_offload_tc_meter_set,
> .meter_get = dpif_offload_tc_meter_get,
> .meter_del = dpif_offload_tc_meter_del,
> diff --git a/lib/dpif-offload.c b/lib/dpif-offload.c
> index 357a7fe62..1c5494d8e 100644
> --- a/lib/dpif-offload.c
> +++ b/lib/dpif-offload.c
> @@ -742,6 +742,26 @@ dpif_offload_flow_flush(struct dpif *dpif)
> }
> }
>
> +uint64_t
> +dpif_offload_flow_get_n_offloaded(const struct dpif *dpif)
> +{
> + struct dp_offload *dp_offload = dpif_offload_get_dp_offload(dpif);
> + const struct dpif_offload *offload;
> + uint64_t flow_count = 0;
> +
> + if (!dp_offload || !dpif_offload_is_offload_enabled()) {
> + return 0;
> + }
> +
> + LIST_FOR_EACH (offload, dpif_list_node, &dp_offload->offload_providers) {
> + if (offload->class->flow_get_n_offloaded) {
> + flow_count += offload->class->flow_get_n_offloaded(offload);
> + }
> + }
> +
> + return flow_count;
> +}
> +
> void
> dpif_offload_meter_set(const struct dpif *dpif, ofproto_meter_id meter_id,
> struct ofputil_meter_config *config)
> diff --git a/lib/dpif-offload.h b/lib/dpif-offload.h
> index 0485b7e98..fb7edf269 100644
> --- a/lib/dpif-offload.h
> +++ b/lib/dpif-offload.h
> @@ -51,6 +51,7 @@ void dpif_offload_dump_start(struct dpif_offload_dump *,
> const struct dpif *);
> bool dpif_offload_dump_next(struct dpif_offload_dump *,
> struct dpif_offload **);
> int dpif_offload_dump_done(struct dpif_offload_dump *);
> +uint64_t dpif_offload_flow_get_n_offloaded(const struct dpif *);
> void dpif_offload_meter_set(const struct dpif *dpif, ofproto_meter_id
> meter_id,
> struct ofputil_meter_config *);
> void dpif_offload_meter_get(const struct dpif *dpif, ofproto_meter_id
> meter_id,
> diff --git a/lib/dpif.c b/lib/dpif.c
> index 12dbc9cd1..61cc68c8e 100644
> --- a/lib/dpif.c
> +++ b/lib/dpif.c
> @@ -2089,29 +2089,6 @@ dpif_bond_stats_get(struct dpif *dpif, uint32_t
> bond_id,
> : EOPNOTSUPP;
> }
>
> -int
> -dpif_get_n_offloaded_flows(struct dpif *dpif, uint64_t *n_flows)
> -{
> - const char *dpif_type_str = dpif_normalize_type(dpif_type(dpif));
> - struct dpif_port_dump port_dump;
> - struct dpif_port dpif_port;
> - int ret, n_devs = 0;
> - uint64_t nflows;
> -
> - *n_flows = 0;
> - DPIF_PORT_FOR_EACH (&dpif_port, &port_dump, dpif) {
> - ret = netdev_ports_get_n_flows(dpif_type_str, dpif_port.port_no,
> - &nflows);
> - if (!ret) {
> - *n_flows += nflows;
> - } else if (ret == EOPNOTSUPP) {
> - continue;
> - }
> - n_devs++;
> - }
> - return n_devs ? 0 : EOPNOTSUPP;
> -}
> -
> int
> dpif_cache_get_supported_levels(struct dpif *dpif, uint32_t *levels)
> {
> diff --git a/lib/dpif.h b/lib/dpif.h
> index 0b685d8df..7fd3c939c 100644
> --- a/lib/dpif.h
> +++ b/lib/dpif.h
> @@ -441,8 +441,6 @@ int dpif_get_dp_stats(const struct dpif *, struct
> dpif_dp_stats *);
>
> int dpif_set_features(struct dpif *, uint32_t new_features);
>
> -int dpif_get_n_offloaded_flows(struct dpif *dpif, uint64_t *n_flows);
> -
>
> /* Port operations. */
>
> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> index 072103596..d1781d8e4 100644
> --- a/lib/netdev-offload-dpdk.c
> +++ b/lib/netdev-offload-dpdk.c
> @@ -2776,24 +2776,24 @@ close_vport_netdev:
> return ret;
> }
>
> -static int
> -netdev_offload_dpdk_get_n_flows(struct netdev *netdev,
> - uint64_t *n_flows)
> +uint64_t
> +netdev_offload_dpdk_flow_get_n_offloaded(struct netdev *netdev)
> {
> struct netdev_offload_dpdk_data *data;
> + uint64_t total = 0;
> unsigned int tid;
>
> data = (struct netdev_offload_dpdk_data *)
> ovsrcu_get(void *, &netdev->hw_info.offload_data);
> if (!data) {
> - return -1;
> + return 0;
> }
>
> for (tid = 0; tid < netdev_offload_thread_nb(); tid++) {
> - n_flows[tid] = data->rte_flow_counters[tid];
> + total += data->rte_flow_counters[tid];
> }
>
> - return 0;
> + return total;
> }
>
> const struct netdev_flow_api netdev_offload_dpdk = {
> @@ -2804,5 +2804,4 @@ const struct netdev_flow_api netdev_offload_dpdk = {
> .uninit_flow_api = netdev_offload_dpdk_uninit_flow_api,
> .flow_get = netdev_offload_dpdk_flow_get,
> .hw_miss_packet_recover = netdev_offload_dpdk_hw_miss_packet_recover,
> - .flow_get_n_flows = netdev_offload_dpdk_get_n_flows,
> };
> diff --git a/lib/netdev-offload-dpdk.h b/lib/netdev-offload-dpdk.h
> index feded432f..d5061b40c 100644
> --- a/lib/netdev-offload-dpdk.h
> +++ b/lib/netdev-offload-dpdk.h
> @@ -23,5 +23,6 @@ struct netdev;
> /* Netdev-specific offload functions. These should only be used by the
> * associated dpif offload provider. */
> int netdev_offload_dpdk_flow_flush(struct netdev *);
> +uint64_t netdev_offload_dpdk_flow_get_n_offloaded(struct netdev *);
>
> #endif /* NETDEV_OFFLOAD_DPDK_H */
> diff --git a/lib/netdev-offload-provider.h b/lib/netdev-offload-provider.h
> index 07068bd82..898df8333 100644
> --- a/lib/netdev-offload-provider.h
> +++ b/lib/netdev-offload-provider.h
> @@ -80,11 +80,6 @@ struct netdev_flow_api {
> int (*flow_del)(struct netdev *, const ovs_u128 *ufid,
> struct dpif_flow_stats *);
>
> - /* Get the number of flows offloaded to netdev.
> - * 'n_flows' is an array of counters, one per offload thread.
> - * Return 0 if successful, otherwise returns a positive errno value. */
> - int (*flow_get_n_flows)(struct netdev *, uint64_t *n_flows);
> -
> /* Recover the packet state (contents and data) for continued processing
> * in software.
> * Return 0 if successful, otherwise returns a positive errno value and
> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> index 46de23014..af9a10356 100644
> --- a/lib/netdev-offload-tc.c
> +++ b/lib/netdev-offload-tc.c
> @@ -2794,22 +2794,17 @@ netdev_tc_flow_del(struct netdev *netdev OVS_UNUSED,
> return del_filter_and_ufid_mapping(&id, ufid, stats);
> }
>
> -static int
> -netdev_tc_get_n_flows(struct netdev *netdev, uint64_t *n_flows)
> +uint64_t
> +dpif_offload_tc_flow_get_n_offloaded(const struct dpif_offload *offload
> + OVS_UNUSED)
> {
> - struct ufid_tc_data *data;
> - uint64_t total = 0;
> + uint64_t total;
>
> ovs_mutex_lock(&ufid_lock);
> - HMAP_FOR_EACH (data, tc_to_ufid_node, &tc_to_ufid) {
> - if (data->netdev == netdev) {
> - total++;
> - }
> - }
> + total = hmap_count(&tc_to_ufid);
> ovs_mutex_unlock(&ufid_lock);
>
> - *n_flows = total;
> - return 0;
> + return total;
> }
>
> static void
> @@ -3453,6 +3448,5 @@ const struct netdev_flow_api netdev_offload_tc = {
> .flow_put = netdev_tc_flow_put,
> .flow_get = netdev_tc_flow_get,
> .flow_del = netdev_tc_flow_del,
> - .flow_get_n_flows = netdev_tc_get_n_flows,
> .init_flow_api = netdev_tc_init_flow_api,
> };
> diff --git a/lib/netdev-offload-tc.h b/lib/netdev-offload-tc.h
> index a6b000852..5002ab00b 100644
> --- a/lib/netdev-offload-tc.h
> +++ b/lib/netdev-offload-tc.h
> @@ -31,5 +31,6 @@ int dpif_offload_tc_meter_get(const struct dpif_offload *,
> ofproto_meter_id,
> struct ofputil_meter_stats *);
> int dpif_offload_tc_meter_del(const struct dpif_offload *, ofproto_meter_id,
> struct ofputil_meter_stats *);
> +uint64_t dpif_offload_tc_flow_get_n_offloaded(const struct dpif_offload *);
>
> #endif /* NETDEV_OFFLOAD_TC_H */
> diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c
> index abbb930be..0c4209290 100644
> --- a/lib/netdev-offload.c
> +++ b/lib/netdev-offload.c
> @@ -309,17 +309,6 @@ netdev_flow_del(struct netdev *netdev, const ovs_u128
> *ufid,
> : EOPNOTSUPP;
> }
>
> -int
> -netdev_flow_get_n_flows(struct netdev *netdev, uint64_t *n_flows)
> -{
> - const struct netdev_flow_api *flow_api =
> - ovsrcu_get(const struct netdev_flow_api *, &netdev->flow_api);
> -
> - return (flow_api && flow_api->flow_get_n_flows)
> - ? flow_api->flow_get_n_flows(netdev, n_flows)
> - : EOPNOTSUPP;
> -}
> -
> int
> netdev_init_flow_api(struct netdev *netdev)
> {
> @@ -717,31 +706,6 @@ netdev_ports_remove(odp_port_t port_no, const char
> *dpif_type)
> return ret;
> }
>
> -int
> -netdev_ports_get_n_flows(const char *dpif_type, odp_port_t port_no,
> - uint64_t *n_flows)
> -{
> - struct port_to_netdev_data *data;
> - int ret = EOPNOTSUPP;
> -
> - ovs_rwlock_rdlock(&port_to_netdev_rwlock);
> - data = netdev_ports_lookup(port_no, dpif_type);
> - if (data) {
> - uint64_t thread_n_flows[MAX_OFFLOAD_THREAD_NB] = {0};
> - unsigned int tid;
> -
> - ret = netdev_flow_get_n_flows(data->netdev, thread_n_flows);
> - *n_flows = 0;
> - if (!ret) {
> - for (tid = 0; tid < netdev_offload_thread_nb(); tid++) {
> - *n_flows += thread_n_flows[tid];
> - }
> - }
> - }
> - ovs_rwlock_unlock(&port_to_netdev_rwlock);
> - return ret;
> -}
> -
> odp_port_t
> netdev_ifindex_to_odp_port(int ifindex)
> {
> diff --git a/lib/netdev-offload.h b/lib/netdev-offload.h
> index 8552b6e0b..2b32179ec 100644
> --- a/lib/netdev-offload.h
> +++ b/lib/netdev-offload.h
> @@ -123,7 +123,6 @@ int netdev_get_hw_info(struct netdev *, int);
> void netdev_set_hw_info(struct netdev *, int, int);
> bool netdev_any_oor(void);
> void netdev_set_flow_api_enabled(const struct smap *ovs_other_config);
> -int netdev_flow_get_n_flows(struct netdev *netdev, uint64_t *n_flows);
>
> struct dpif_port;
> int netdev_ports_insert(struct netdev *, struct dpif_port *);
> @@ -151,8 +150,6 @@ int netdev_ports_flow_get(const char *dpif_type, struct
> match *match,
> struct dpif_flow_stats *stats,
> struct dpif_flow_attrs *attrs,
> struct ofpbuf *buf);
> -int netdev_ports_get_n_flows(const char *dpif_type,
> - odp_port_t port_no, uint64_t *n_flows);
>
> #ifdef __cplusplus
> }
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index 4273b3ea6..3fa96b673 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -830,11 +830,7 @@ udpif_get_n_flows(struct udpif *udpif)
> if (!dpif_synced_dp_layers(udpif->dpif)) {
> /* If the dpif layer does not sync the flows, we need to include
> * the hardware offloaded flows separately. */
> - uint64_t hw_flows;
> -
> - if (!dpif_get_n_offloaded_flows(udpif->dpif, &hw_flows)) {
> - flow_count += hw_flows;
> - }
> + flow_count += dpif_offload_flow_get_n_offloaded(udpif->dpif);
> }
>
> atomic_store_relaxed(&udpif->n_flows, flow_count);
> @@ -3171,7 +3167,8 @@ upcall_unixctl_show(struct unixctl_conn *conn, int argc
> OVS_UNUSED,
> ds_put_format(&ds, " flows : (current %lu)"
> " (avg %u) (max %u) (limit %u)\n", udpif_get_n_flows(udpif),
> udpif->avg_n_flows, udpif->max_n_flows, flow_limit);
> - if (!dpif_get_n_offloaded_flows(udpif->dpif, &n_offloaded_flows)) {
> + n_offloaded_flows = dpif_offload_flow_get_n_offloaded(udpif->dpif);
> + if (n_offloaded_flows) {
> ds_put_format(&ds, " offloaded flows : %"PRIu64"\n",
> n_offloaded_flows);
> }
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
