diff mbox series

[ovs-dev,24/41] dpif-offload: Add port dump APIs to dpif-offload.

Message ID 30e72aff626522790d82fa3a6c493b0872432be4.1762950453.git.echaudro@redhat.com
State Changes Requested
Headers show
Series Architectural refactoring of hardware offload infrastructure | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Eelco Chaudron Nov. 12, 2025, 3:35 p.m. UTC
This patch adds a port dumping APIs similar to the
dpif_port_dump_...() APIs.

Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
---
 lib/dpif-offload-dpdk.c     |  32 +++++++
 lib/dpif-offload-dummy.c    |  62 +++++++++----
 lib/dpif-offload-provider.h |  31 +++++++
 lib/dpif-offload-tc.c       |  32 +++++++
 lib/dpif-offload.c          | 169 +++++++++++++++++++++++++++++++++++-
 lib/dpif-offload.h          |  31 +++++++
 6 files changed, 341 insertions(+), 16 deletions(-)

Comments

0-day Robot Nov. 14, 2025, 2:17 p.m. UTC | #1
Git SHA: 30e72aff626522790d82fa3a6c493b0872432be4
Author: Eelco Chaudron <echaudro@redhat.com>
Subject: dpif-offload: Add port dump APIs to dpif-offload.

This patch adds a port dumping APIs similar to the dpif_port_dump_...() 
APIs.

> diff --git a/lib/dpif-offload-dpdk.c b/lib/dpif-offload-dpdk.c
> index 6014db38c..211978f16 100644
> --- a/lib/dpif-offload-dpdk.c
> +++ b/lib/dpif-offload-dpdk.c
[ ... ]
> +static int
> +dpif_offload_dpdk_port_dump_start(const struct dpif_offload *offload_,
> +                                  void **statep)
> +{
> +    struct dpif_offload_dpdk *offload = dpif_offload_dpdk_cast(offload_);
> +
> +    return dpif_offload_port_mgr_port_dump_start(offload->port_mgr, statep);
> +}
> +
> +static int
> +dpif_offload_dpdk_port_dump_next(const struct dpif_offload *offload_,
> +                                 void *state,
> +                                 struct dpif_offload_port *port)
> +{
> +    struct dpif_offload_dpdk *offload = dpif_offload_dpdk_cast(offload_);
> +
> +    return dpif_offload_port_mgr_port_dump_next(offload->port_mgr, state,
> +                                                port);
> +}
> +
> +static int
> +dpif_offload_dpdk_port_dump_done(const struct dpif_offload *offload_,
> +                                 void *state)
> +{
> +    struct dpif_offload_dpdk *offload = dpif_offload_dpdk_cast(offload_);
> +
> +    return dpif_offload_port_mgr_port_dump_done(offload->port_mgr, state);
> +}

[ ... ]

> diff --git a/lib/dpif-offload.c b/lib/dpif-offload.c
> index da61a7c0e..7a2ab60e7 100644
> --- a/lib/dpif-offload.c
> +++ b/lib/dpif-offload.c

[ ... ]

> @@ -151,7 +151,9 @@ dp_offload_initialize(void)
>                     && base_dpif_offload_classes[i]->close
>                     && base_dpif_offload_classes[i]->can_offload
>                     && base_dpif_offload_classes[i]->port_add
> -                   && base_dpif_offload_classes[i]->port_del
> +                   && base_dpif_offload_classes[i]->port_dump_start
> +                   && base_dpif_offload_classes[i]->port_dump_next
> +                   && base_dpif_offload_classes[i]->port_dump_done
>                     && base_dpif_offload_classes[i]->get_netdev);

The validation check has dropped the port_del requirement check. Does this 
create a scenario where implementations could register without implementing
the port_del functionality?

> +int dpif_offload_port_mgr_port_dump_next(struct dpif_offload_port_mgr *mgr,
> +                                         void *state_,
> +                                         struct dpif_offload_port *port)
> +{
> +    struct dpif_offload_port_mgr_dump_state *state = state_;
> +    struct dpif_offload_port_mgr_port *mgr_port;
> +
> +    while (state->port_index < state->port_count) {
> +
> +        mgr_port = dpif_offload_port_mgr_find_by_odp_port(
> +            mgr, state->ports[state->port_index++]);
> +
> +        if (mgr_port) {
> +            port->netdev = netdev_ref(mgr_port->netdev);
> +            port->port_no = mgr_port->port_no;
> +
> +            netdev_close(state->last_netdev);
> +            state->last_netdev = port->netdev;
> +            return 0;
> +        }
> +    }
> +
> +    return EOF;
> +}

Could this code encounter a race condition between 
dpif_offload_port_mgr_port_dump_start and 
dpif_offload_port_mgr_port_dump_next? The dump_start function captures
port numbers at a snapshot in time, but the next function searches for 
those ports by port number. If a port is removed between these calls, 
would mgr_port be NULL and cause the while loop to skip that port?
diff mbox series

Patch

diff --git a/lib/dpif-offload-dpdk.c b/lib/dpif-offload-dpdk.c
index 6014db38c..211978f16 100644
--- a/lib/dpif-offload-dpdk.c
+++ b/lib/dpif-offload-dpdk.c
@@ -103,6 +103,35 @@  dpif_offload_dpdk_port_del(struct dpif_offload *offload_, odp_port_t port_no)
     return ret;
 }
 
+static int
+dpif_offload_dpdk_port_dump_start(const struct dpif_offload *offload_,
+                                  void **statep)
+{
+    struct dpif_offload_dpdk *offload = dpif_offload_dpdk_cast(offload_);
+
+    return dpif_offload_port_mgr_port_dump_start(offload->port_mgr, statep);
+}
+
+static int
+dpif_offload_dpdk_port_dump_next(const struct dpif_offload *offload_,
+                                 void *state,
+                                 struct dpif_offload_port *port)
+{
+    struct dpif_offload_dpdk *offload = dpif_offload_dpdk_cast(offload_);
+
+    return dpif_offload_port_mgr_port_dump_next(offload->port_mgr, state,
+                                                port);
+}
+
+static int
+dpif_offload_dpdk_port_dump_done(const struct dpif_offload *offload_,
+                                 void *state)
+{
+    struct dpif_offload_dpdk *offload = dpif_offload_dpdk_cast(offload_);
+
+    return dpif_offload_port_mgr_port_dump_done(offload->port_mgr, state);
+}
+
 static struct netdev *
 dpif_offload_dpdk_get_netdev(struct dpif_offload *offload_, odp_port_t port_no)
 {
@@ -326,6 +355,9 @@  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,
+    .port_dump_start = dpif_offload_dpdk_port_dump_start,
+    .port_dump_next = dpif_offload_dpdk_port_dump_next,
+    .port_dump_done = dpif_offload_dpdk_port_dump_done,
     .flow_get_n_offloaded = dpif_offload_dpdk_get_n_offloaded,
     .get_netdev = dpif_offload_dpdk_get_netdev,
     .netdev_flow_flush = dpif_offload_dpdk_netdev_flow_flush,
diff --git a/lib/dpif-offload-dummy.c b/lib/dpif-offload-dummy.c
index 1ccf32ca8..2a1663719 100644
--- a/lib/dpif-offload-dummy.c
+++ b/lib/dpif-offload-dummy.c
@@ -97,6 +97,35 @@  dpif_offload_dummy_port_del(struct dpif_offload *dpif_offload,
     return 0;
 }
 
+static int
+dpif_offload_dummy_port_dump_start(const struct dpif_offload *offload_,
+                                      void **statep)
+{
+    struct dpif_offload_dummy *offload = dpif_offload_dummy_cast(offload_);
+
+    return dpif_offload_port_mgr_port_dump_start(offload->port_mgr, statep);
+}
+
+static int
+dpif_offload_dummy_port_dump_next(const struct dpif_offload *offload_,
+                                  void *state,
+                                  struct dpif_offload_port *port)
+{
+    struct dpif_offload_dummy *offload = dpif_offload_dummy_cast(offload_);
+
+    return dpif_offload_port_mgr_port_dump_next(offload->port_mgr, state,
+                                                port);
+}
+
+static int
+dpif_offload_dummy_port_dump_done(const struct dpif_offload *offload_,
+                                  void *state)
+{
+    struct dpif_offload_dummy *offload = dpif_offload_dummy_cast(offload_);
+
+    return dpif_offload_port_mgr_port_dump_done(offload->port_mgr, state);
+}
+
 static struct netdev *
 dpif_offload_dummy_get_netdev(struct dpif_offload *dpif_offload,
                               odp_port_t port_no)
@@ -250,21 +279,24 @@  dpif_offload_dummy_can_offload(struct dpif_offload *dpif_offload OVS_UNUSED,
     return is_dummy_netdev_class(netdev->netdev_class) ? true : false;
 }
 
-#define DEFINE_DPIF_DUMMY_CLASS(NAME, TYPE_STR)         \
-    struct dpif_offload_class NAME = {                  \
-        .type = TYPE_STR,                               \
-        .impl_type = DPIF_OFFLOAD_IMPL_HW_ONLY,         \
-        .supported_dpif_types = (const char *const[]) { \
-            "dummy",                                    \
-            NULL},                                      \
-        .open = dpif_offload_dummy_open,                \
-        .close = dpif_offload_dummy_close,              \
-        .set_config = dpif_offload_dummy_set_config,    \
-        .get_debug = dpif_offload_dummy_get_debug,      \
-        .can_offload = dpif_offload_dummy_can_offload,  \
-        .port_add = dpif_offload_dummy_port_add,        \
-        .port_del = dpif_offload_dummy_port_del,        \
-        .get_netdev = dpif_offload_dummy_get_netdev,    \
+#define DEFINE_DPIF_DUMMY_CLASS(NAME, TYPE_STR)                 \
+    struct dpif_offload_class NAME = {                          \
+        .type = TYPE_STR,                                       \
+        .impl_type = DPIF_OFFLOAD_IMPL_HW_ONLY,                 \
+        .supported_dpif_types = (const char *const[]) {         \
+            "dummy",                                            \
+            NULL},                                              \
+        .open = dpif_offload_dummy_open,                        \
+        .close = dpif_offload_dummy_close,                      \
+        .set_config = dpif_offload_dummy_set_config,            \
+        .get_debug = dpif_offload_dummy_get_debug,              \
+        .can_offload = dpif_offload_dummy_can_offload,          \
+        .port_add = dpif_offload_dummy_port_add,                \
+        .port_del = dpif_offload_dummy_port_del,                \
+        .port_dump_start = dpif_offload_dummy_port_dump_start,  \
+        .port_dump_next = dpif_offload_dummy_port_dump_next,    \
+        .port_dump_done = dpif_offload_dummy_port_dump_done,    \
+        .get_netdev = dpif_offload_dummy_get_netdev,            \
     }
 
 DEFINE_DPIF_DUMMY_CLASS(dpif_offload_dummy_class, "dummy");
diff --git a/lib/dpif-offload-provider.h b/lib/dpif-offload-provider.h
index 64bb3242d..e05678fea 100644
--- a/lib/dpif-offload-provider.h
+++ b/lib/dpif-offload-provider.h
@@ -157,6 +157,30 @@  struct dpif_offload_class {
     void (*port_set_config)(struct dpif_offload *, odp_port_t port_no,
                             const struct smap *cfg);
 
+    /* Attempts to begin dumping the ports in a dpif_offload.  On success,
+     * returns 0 and initializes '*statep' with any data needed for iteration.
+     * On failure, returns a positive errno value. */
+    int (*port_dump_start)(const struct dpif_offload *, void **statep);
+
+    /* Attempts to retrieve another port from 'dpif_offload' for 'state', which
+     * was initialized by a successful call to the 'port_dump_start' function
+     * for 'dpif_offload'.  On success, stores a new dpif_offload_port into
+     * 'port' and returns 0. Returns EOF if the end of the port table has been
+     * reached, or a positive errno value on error.  This function will not be
+     * called again once it returns nonzero once for a given iteration (but
+     * the 'port_dump_done' function will be called afterward).
+     *
+     * The dpif provider retains ownership of the data stored in 'port'.  It
+     * must remain valid until at least the next call to 'port_dump_next' or
+     * 'port_dump_done' for 'state'. */
+    int (*port_dump_next)(const struct dpif_offload *, void *state,
+                          struct dpif_offload_port *);
+
+    /* Releases resources from 'dpif_offload' for 'state', which was
+     * initialized by a successful call to the 'port_dump_start' function for
+     * 'dpif_offload'. */
+    int (*port_dump_done)(const struct dpif_offload *dpif, void *state);
+
     /* Deletes all offloaded flows for this offload_provider.  Return 0 if
      * successful, otherwise returns a positive errno value. */
     int (*flow_flush)(const struct dpif_offload *);
@@ -312,6 +336,13 @@  void dpif_offload_port_mgr_traverse_ports(
     struct dpif_offload_port_mgr *mgr,
     bool (*cb)(struct dpif_offload_port_mgr_port *, void *),
     void *aux);
+int dpif_offload_port_mgr_port_dump_start(struct dpif_offload_port_mgr *,
+                                          void **statep);
+int dpif_offload_port_mgr_port_dump_next(struct dpif_offload_port_mgr *,
+                                         void *state,
+                                         struct dpif_offload_port *);
+int dpif_offload_port_mgr_port_dump_done(struct dpif_offload_port_mgr *,
+                                         void *state);
 
 #define DPIF_OFFLOAD_PORT_MGR_PORT_FOR_EACH(PORT, PORT_MGR) \
     CMAP_FOR_EACH (PORT, odp_port_node, &(PORT_MGR)->odp_port_to_port)
diff --git a/lib/dpif-offload-tc.c b/lib/dpif-offload-tc.c
index e0358d091..c343cd9c1 100644
--- a/lib/dpif-offload-tc.c
+++ b/lib/dpif-offload-tc.c
@@ -127,6 +127,35 @@  dpif_offload_tc_port_del(struct dpif_offload *dpif_offload,
     return ret;
 }
 
+static int
+dpif_offload_tc_port_dump_start(const struct dpif_offload *offload_,
+                                void **statep)
+{
+    struct dpif_offload_tc *offload = dpif_offload_tc_cast(offload_);
+
+    return dpif_offload_port_mgr_port_dump_start(offload->port_mgr, statep);
+}
+
+static int
+dpif_offload_tc_port_dump_next(const struct dpif_offload *offload_,
+                               void *state,
+                               struct dpif_offload_port *port)
+{
+    struct dpif_offload_tc *offload = dpif_offload_tc_cast(offload_);
+
+    return dpif_offload_port_mgr_port_dump_next(offload->port_mgr, state,
+                                                port);
+}
+
+static int
+dpif_offload_tc_port_dump_done(const struct dpif_offload *offload_,
+                               void *state)
+{
+    struct dpif_offload_tc *offload = dpif_offload_tc_cast(offload_);
+
+    return dpif_offload_port_mgr_port_dump_done(offload->port_mgr, state);
+}
+
 static struct netdev *
 dpif_offload_tc_get_netdev(struct dpif_offload *dpif_offload,
                            odp_port_t port_no)
@@ -571,6 +600,9 @@  struct dpif_offload_class dpif_offload_tc_class = {
     .can_offload = dpif_offload_tc_can_offload,
     .port_add = dpif_offload_tc_port_add,
     .port_del = dpif_offload_tc_port_del,
+    .port_dump_start = dpif_offload_tc_port_dump_start,
+    .port_dump_next = dpif_offload_tc_port_dump_next,
+    .port_dump_done = dpif_offload_tc_port_dump_done,
     .flow_flush = dpif_offload_tc_flow_flush,
     .flow_dump_create = dpif_offload_tc_flow_dump_create,
     .flow_dump_next = dpif_offload_tc_flow_dump_next,
diff --git a/lib/dpif-offload.c b/lib/dpif-offload.c
index da61a7c0e..7a2ab60e7 100644
--- a/lib/dpif-offload.c
+++ b/lib/dpif-offload.c
@@ -151,7 +151,9 @@  dp_offload_initialize(void)
                    && base_dpif_offload_classes[i]->close
                    && base_dpif_offload_classes[i]->can_offload
                    && base_dpif_offload_classes[i]->port_add
-                   && base_dpif_offload_classes[i]->port_del
+                   && base_dpif_offload_classes[i]->port_dump_start
+                   && base_dpif_offload_classes[i]->port_dump_next
+                   && base_dpif_offload_classes[i]->port_dump_done
                    && base_dpif_offload_classes[i]->get_netdev);
 
         ovs_assert((base_dpif_offload_classes[i]->flow_dump_create &&
@@ -1078,6 +1080,101 @@  dpif_offload_flow_dump_thread_destroy(struct dpif_flow_dump_thread *thread)
     free(thread->offload_threads);
 }
 
+static bool
+dpif_offload_get_next_offload_for_dump(struct dpif_offload_port_dump *dump)
+{
+    struct dp_offload *dp_offload = dpif_offload_get_dp_offload(dump->dpif);
+    const struct dpif_offload *offload;
+    const struct dpif_offload *prev_offload;
+
+    if (!dp_offload) {
+        dump->offload = NULL;
+        return false;
+    }
+
+    prev_offload = dump->offload;
+    dump->offload = NULL;
+
+    LIST_FOR_EACH (offload, dpif_list_node,
+                   &dp_offload->offload_providers) {
+        if (!prev_offload) {
+            /* We need the first offload provider. */
+            dump->offload = offload;
+            break;
+        }
+        if (offload->dpif_list_node.next != &dp_offload->offload_providers
+            && prev_offload == offload) {
+            /* This is the current offload provider, so we need the next
+             * one if available.  If not we will exit with !dump->offload. */
+            dump->offload = CONTAINER_OF(offload->dpif_list_node.next,
+                                         struct dpif_offload, dpif_list_node);
+            break;
+        }
+    }
+
+    return dump->offload ? true : false;
+}
+
+void
+dpif_offload_port_dump_start(struct dpif_offload_port_dump *dump,
+                             const struct dpif *dpif)
+{
+    memset(dump, 0, sizeof *dump);
+    dump->dpif = dpif;
+    if (!dpif_offload_get_next_offload_for_dump(dump)) {
+        dump->error = EOF;
+    } else {
+        dump->error = dump->offload->class->port_dump_start(dump->offload,
+                                                            &dump->state);
+    }
+}
+
+bool
+dpif_offload_port_dump_next(struct dpif_offload_port_dump *dump,
+                            struct dpif_offload_port *port)
+{
+    const struct dpif_offload *offload = dump->offload;
+
+    while (true) {
+
+        if (dump->error) {
+            break;
+        }
+
+        dump->error = offload->class->port_dump_next(offload, dump->state,
+                                                     port);
+        if (dump->error) {
+            offload->class->port_dump_done(offload, dump->state);
+
+            if (dump->error != EOF) {
+                /* If the error is not EOF, we stop dumping ports from all
+                 * providers and return it. */
+                break;
+            }
+
+            if (dpif_offload_get_next_offload_for_dump(dump)) {
+                offload = dump->offload;
+                dump->error = offload->class->port_dump_start(offload,
+                                                              &dump->state);
+                continue;
+            }
+        }
+        break;
+    }
+
+    return dump->error ? false : true;
+}
+
+int
+dpif_offload_port_dump_done(struct dpif_offload_port_dump *dump)
+{
+    if (!dump->error && dump->offload) {
+        dump->error = dump->offload->class->port_dump_done(dump->offload,
+                                                           dump->state);
+    }
+    return dump->error == EOF ? 0 : dump->error;
+}
+
 struct netdev *
 dpif_offload_offload_get_netdev_by_port_id(struct dpif_offload *offload,
                                            odp_port_t port_no)
@@ -1423,3 +1520,73 @@  dpif_offload_port_mgr_port_count(struct dpif_offload_port_mgr *mgr)
 {
     return cmap_count(&mgr->odp_port_to_port);
 }
+
+struct dpif_offload_port_mgr_dump_state {
+    struct netdev *last_netdev;
+    size_t port_index;
+    size_t port_count;
+    odp_port_t ports[];
+};
+
+int
+dpif_offload_port_mgr_port_dump_start(struct dpif_offload_port_mgr *mgr,
+                                      void **statep)
+{
+    size_t port_count = dpif_offload_port_mgr_port_count(mgr);
+    struct dpif_offload_port_mgr_dump_state *state;
+    struct dpif_offload_port_mgr_port *port;
+    size_t added_port_count = 0;
+
+    state = xmalloc(sizeof *state + (port_count * sizeof(odp_port_t)));
+
+    DPIF_OFFLOAD_PORT_MGR_PORT_FOR_EACH (port, mgr) {
+        if (added_port_count >= port_count) {
+            break;
+        }
+
+        state->ports[added_port_count] = port->port_no;
+        added_port_count++;
+    }
+    state->port_count = added_port_count;
+    state->port_index = 0;
+    state->last_netdev = NULL;
+
+    *statep = state;
+    return 0;
+}
+
+int dpif_offload_port_mgr_port_dump_next(struct dpif_offload_port_mgr *mgr,
+                                         void *state_,
+                                         struct dpif_offload_port *port)
+{
+    struct dpif_offload_port_mgr_dump_state *state = state_;
+    struct dpif_offload_port_mgr_port *mgr_port;
+
+    while (state->port_index < state->port_count) {
+
+        mgr_port = dpif_offload_port_mgr_find_by_odp_port(
+            mgr, state->ports[state->port_index++]);
+
+        if (mgr_port) {
+            port->netdev = netdev_ref(mgr_port->netdev);
+            port->port_no = mgr_port->port_no;
+
+            netdev_close(state->last_netdev);
+            state->last_netdev = port->netdev;
+            return 0;
+        }
+    }
+
+    return EOF;
+}
+
+int
+dpif_offload_port_mgr_port_dump_done(
+    struct dpif_offload_port_mgr *mgr OVS_UNUSED, void *state_)
+{
+    struct dpif_offload_port_mgr_dump_state *state = state_;
+
+    netdev_close(state->last_netdev);
+    free(state);
+    return 0;
+}
diff --git a/lib/dpif-offload.h b/lib/dpif-offload.h
index 36c67cdfd..9749135b5 100644
--- a/lib/dpif-offload.h
+++ b/lib/dpif-offload.h
@@ -102,6 +102,37 @@  enum dpif_offload_impl_type dpif_offload_get_impl_type_by_class(
           : (dpif_offload_dump_done(DUMP), false));      \
         )
 
+struct dpif_offload_port {
+    struct netdev *netdev;
+    odp_port_t port_no;
+};
+
+struct dpif_offload_port_dump {
+    const struct dpif *dpif;
+    const struct dpif_offload *offload;
+    int error;
+    void *state;
+};
+
+void dpif_offload_port_dump_start(struct dpif_offload_port_dump *,
+                                  const struct dpif *);
+bool dpif_offload_port_dump_next(struct dpif_offload_port_dump *,
+                                 struct dpif_offload_port *);
+int dpif_offload_port_dump_done(struct dpif_offload_port_dump *);
+
+/* Iterates through each DPIF_OFFLOAD_PORT in DPIF, using DUMP as state.
+ *
+ * Arguments all have pointer type.
+ *
+ * If you break out of the loop, then you need to free the dump structure by
+ * hand using dpif_offload_port_dump_done(). */
+#define DPIF_OFFLOAD_PORT_FOR_EACH(DPIF_OFFLOAD_PORT, DUMP, DPIF)  \
+    for (dpif_offload_port_dump_start(DUMP, DPIF);                 \
+         (dpif_offload_port_dump_next(DUMP, DPIF_OFFLOAD_PORT)     \
+          ? true                                                   \
+          : (dpif_offload_port_dump_done(DUMP), false));           \
+        )
+
 
 /* Netdev specific function, which can be used in the fast path. */
 int dpif_offload_netdev_flush_flows(struct netdev *);