Re: [ovs-dev] [PATCH] dpif-netlink: Count the number of offloaded rules

[Roi Dayan]

On 2020-12-07 4:12 PM, Ilya Maximets wrote:

On 12/6/20 9:16 AM, Roi Dayan wrote:

From: Jianbo Liu <jian...@nvidia.com>

Add a counter for the offloaded rules, and display it in the command
of "ovs-appctl upcall/show".

Signed-off-by: Jianbo Liu <jian...@nvidia.com>
Reviewed-by: Roi Dayan <r...@nvidia.com>

I don't think this patch handles flow_flush case that could be triggered,
for example, by dpctl/del-flows command.

Second is that users of dpif_get_dp_stats() does not initialize stats
beforehand, that means that with this patch userspace datapath returns
garbage that will be printed to a user.  So, please, add an implementation
for userspace datapath.  And a unit tests with dummy offload provider
would be nice.

Could you, please, fix that in a follow up patch?

Best regards, Ilya Maximets.

thanks Ilya. we'll check this.

---
  lib/dpif-netlink.c            | 9 +++++++++
  lib/dpif.h                    | 1 +
  ofproto/ofproto-dpif-upcall.c | 6 ++++++
  3 files changed, 16 insertions(+)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 2f881e4fadf1..6858ba6128d7 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -208,6 +208,7 @@ struct dpif_netlink {
      /* Change notification. */
      struct nl_sock *port_notifier; /* vport multicast group subscriber. */
      bool refresh_channels;
+    struct atomic_count n_offloaded_flows;
  };

static void report_loss(struct dpif_netlink *, struct dpif_channel *,

@@ -653,6 +654,7 @@ dpif_netlink_run(struct dpif *dpif_)
  static int
  dpif_netlink_get_stats(const struct dpif *dpif_, struct dpif_dp_stats *stats)
  {
+    struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
      struct dpif_netlink_dp dp;
      struct ofpbuf *buf;
      int error;
@@ -678,6 +680,7 @@ dpif_netlink_get_stats(const struct dpif *dpif_, struct 
dpif_dp_stats *stats)
          }
          ofpbuf_delete(buf);
      }
+    stats->n_offloaded_flows = atomic_count_get(&dpif->n_offloaded_flows);
      return error;
  }

@@ -2189,6 +2192,9 @@ try_send_to_netdev(struct dpif_netlink *dpif, struct dpif_op *op)

          }

err = parse_flow_put(dpif, put);

+        if (!err && (put->flags & DPIF_FP_CREATE)) {
+            atomic_count_inc(&dpif->n_offloaded_flows);
+        }
          log_flow_put_message(&dpif->dpif, &this_module, put, 0);
          break;
      }
@@ -2203,6 +2209,9 @@ try_send_to_netdev(struct dpif_netlink *dpif, struct 
dpif_op *op)
                                  dpif_normalize_type(dpif_type(&dpif->dpif)),
                                  del->ufid,
                                  del->stats);
+        if (!err) {
+            atomic_count_dec(&dpif->n_offloaded_flows);
+        }
          log_flow_del_message(&dpif->dpif, &this_module, del, 0);
          break;
      }
diff --git a/lib/dpif.h b/lib/dpif.h
index cb047dbe2bf6..7ef148c6d756 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -429,6 +429,7 @@ struct dpif_dp_stats {
      uint64_t n_missed;          /* Number of flow table misses. */
      uint64_t n_lost;            /* Number of misses not sent to userspace. */
      uint64_t n_flows;           /* Number of flows present. */
+    uint64_t n_offloaded_flows; /* Number of offloaded flows present. */
      uint64_t n_mask_hit;        /* Number of mega flow masks visited for
                                     flow table matches. */
      uint32_t n_masks;           /* Number of mega flow masks. */
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index e022fde27f81..19b92dfe0067 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -175,6 +175,7 @@ struct udpif {

/* n_flows_mutex prevents multiple threads updating these concurrently. */

      atomic_uint n_flows;               /* Number of flows in the datapath. */
+    atomic_uint n_offloaded_flows;     /* Number of the offloaded flows. */
      atomic_llong n_flows_timestamp;    /* Last time n_flows was updated. */
      struct ovs_mutex n_flows_mutex;

@@ -730,6 +731,8 @@ udpif_get_n_flows(struct udpif *udpif)

          dpif_get_dp_stats(udpif->dpif, &stats);
          flow_count = stats.n_flows;
          atomic_store_relaxed(&udpif->n_flows, flow_count);
+        atomic_store_relaxed(&udpif->n_offloaded_flows,
+                             stats.n_offloaded_flows);
          ovs_mutex_unlock(&udpif->n_flows_mutex);
      } else {
          atomic_read_relaxed(&udpif->n_flows, &flow_count);
@@ -2904,6 +2907,7 @@ upcall_unixctl_show(struct unixctl_conn *conn, int argc 
OVS_UNUSED,
      struct udpif *udpif;

LIST_FOR_EACH (udpif, list_node, &all_udpifs) {

+        unsigned int n_offloaded_flows;
          unsigned int flow_limit;
          bool ufid_enabled;
          size_t i;
@@ -2915,6 +2919,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);
+        atomic_read_relaxed(&udpif->n_offloaded_flows, &n_offloaded_flows);
+        ds_put_format(&ds, "  offloaded flows : %u\n", n_offloaded_flows);
          ds_put_format(&ds, "  dump duration : %lldms\n", 
udpif->dump_duration);
          ds_put_format(&ds, "  ufid enabled : ");
          if (ufid_enabled) {

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