diff mbox series

[ovs-dev,14/41] dpif-offload: Call meter netdev-offload APIs via dpif-offload.

Message ID c25c49e3971b8c048af047dde1674e5be12d8852.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 
Note that this change does not yet move meter handling to
dpif-offload-tc, where it probably should eventually reside.
This should be addressed in a follow-up patch.

Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
---
 lib/dpif-netlink.c            | 29 ++-----------
 lib/dpif-offload-provider.h   | 26 ++++++++++++
 lib/dpif-offload-tc.c         |  5 +++
 lib/dpif-offload.c            | 77 +++++++++++++++++++++++++++++++++++
 lib/dpif-offload.h            |  6 +++
 lib/dpif.c                    |  3 ++
 lib/netdev-offload-provider.h | 27 ------------
 lib/netdev-offload-tc.c       | 68 ++++++++++++++++++++++++-------
 lib/netdev-offload-tc.h       |  8 ++++
 lib/netdev-offload.c          | 61 ---------------------------
 lib/netdev-offload.h          |  4 --
 11 files changed, 183 insertions(+), 131 deletions(-)

Comments

0-day Robot Nov. 14, 2025, 2:16 p.m. UTC | #1 
git sha: c25c49e3971b8c048af047dde1674e5be12d8852
Author: Eelco Chaudron <echaudro@redhat.com>
Subject: [PATCH 14/41] dpif-offload: Call meter netdev-offload APIs via dpif-offload.

This patch refactors meter handling by moving meter operations from the 
netdev-offload layer to the dpif-offload layer. The change introduces 
new meter APIs in dpif-offload-provider.h and implements them in 
dpif-offload-tc.c, while removing the corresponding functions from 
netdev-offload.

> diff --git a/lib/dpif-offload-tc.c b/lib/dpif-offload-tc.c
> index 63ce789c6..f258c7147 100644
> --- a/lib/dpif-offload-tc.c
> +++ b/lib/dpif-offload-tc.c
> @@ -113,6 +113,8 @@ dpif_offload_tc_open(const struct dpif_offload_class *offload_class,
>      offload_tc->once_enable = (struct ovsthread_once) \
>                                OVSTHREAD_ONCE_INITIALIZER;
>  
> +    dpif_offload_tc_meter_init();
> +

Does this call to dpif_offload_tc_meter_init() within dpif_offload_tc_open 
potentially initialize the meter subsystem multiple times if multiple 
offload instances are created?

> diff --git a/lib/dpif-offload.c b/lib/dpif-offload.c
> index 537939d2d..5a13e0e44 100644
> --- a/lib/dpif-offload.c
> +++ b/lib/dpif-offload.c
> @@ -31,6 +31,8 @@
>  
>  VLOG_DEFINE_THIS_MODULE(dpif_offload);
>  
> +static struct vlog_rate_limit rl_dbg = VLOG_RATE_LIMIT_INIT(1, 5);
> +
> [ ... ]
> +void
> +dpif_offload_meter_set(const struct dpif *dpif, ofproto_meter_id meter_id,
> +                       struct ofputil_meter_config *config)
> +{
> +    struct dp_offload *dp_offload = dpif_offload_get_dp_offload(dpif);
> +    const struct dpif_offload *offload;
> +
> +    if (!dp_offload) {
> +        return;
> +    }
> +
> +    LIST_FOR_EACH (offload, dpif_list_node, &dp_offload->offload_providers) {
> +        if (offload->class->meter_set) {
> +            int err = offload->class->meter_set(offload, meter_id, config);
> +            if (err) {
> +                /* Offload APIs could fail, for example, because the offload
> +                 * is not supported. This is fine, as the offload API should
> +                 * take care of this. */
> +                VLOG_DBG_RL(&rl_dbg,
> +                            "Failed setting meter %u on dpif-offload provider"
> +                            " %s, error %s", meter_id.uint32,
> +                            dpif_offload_name(offload), ovs_strerror(err));
> +            }
> +        }
> +    }
> +}

Should this function return an error code to indicate if all offload 
providers failed, rather than ignoring all failures?

> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> index 1c554416c..8dc6b0c91 100644
> --- a/lib/netdev-offload-tc.c
> +++ b/lib/netdev-offload-tc.c
> @@ -3149,6 +3149,21 @@ tc_cleanup_policer_actions(struct id_pool *police_ids,
>      hmap_destroy(&map);
>  }
>  
> +void
> +dpif_offload_tc_meter_init(void) {

Does this opening brace follow the coding style guidelines? Should it be 
on the next line?

> +    static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
> +
> +    if (ovsthread_once_start(&once)) {
> +        ovs_mutex_lock(&meter_police_ids_mutex);
> +        meter_police_ids = id_pool_create(
> +            METER_POLICE_IDS_BASE,
> +            METER_POLICE_IDS_MAX - METER_POLICE_IDS_BASE + 1);
> +        ovs_mutex_unlock(&meter_police_ids_mutex);
> +
> +        ovsthread_once_done(&once);
> +    }
> +}

> @@ -3202,9 +3218,9 @@ netdev_tc_init_flow_api(struct netdev *netdev)
>          probe_vxlan_gbp_support(ifindex);
>          probe_enc_flags_support(ifindex);
>  
> +        dpif_offload_tc_meter_init();
> +
>          ovs_mutex_lock(&meter_police_ids_mutex);
> -        meter_police_ids = id_pool_create(METER_POLICE_IDS_BASE,
> -                            METER_POLICE_IDS_MAX - METER_POLICE_IDS_BASE + 1);
>          tc_cleanup_policer_actions(meter_police_ids, METER_POLICE_IDS_BASE,
>                                     METER_POLICE_IDS_MAX);
>          ovs_mutex_unlock(&meter_police_ids_mutex);

With both dpif_offload_tc_open() and netdev_tc_init_flow_api() calling 
dpif_offload_tc_meter_init(), does this create a race condition where 
tc_cleanup_policer_actions() might access meter_police_ids before it's 
initialized?

> +int
> +dpif_offload_tc_meter_set(const struct dpif_offload *offload OVS_UNUSED,
> +                         ofproto_meter_id meter_id,
> +                         struct ofputil_meter_config *config)
> +{
> +    uint32_t police_index;
> +    uint32_t rate, burst;
> +    bool add_policer;
> +    int err;
> +
> +    if (!dpif_offload_is_offload_enabled()) {
> +        /* FIXME: If offload is disabled, ignore this call. This preserves the
> +         * behavior from before the dpif-offload implementation. However, it
> +         * also retains the same bug where the meter_id is not offloaded if it
> +         * was configured before offload was enabled. */
> +        return 0;
> +    }

Should this FIXME comment be addressed before merging, or is it acceptable 
to preserve the existing bug temporarily?

> diff --git a/lib/netdev-offload-tc.h b/lib/netdev-offload-tc.h
> index 74dd33b8b..d0485b4ad 100644
> --- a/lib/netdev-offload-tc.h
> +++ b/lib/netdev-offload-tc.h
> [ ... ]
> +int dpif_offload_tc_meter_del(const struct dpif_offload *, ofproto_meter_id,
> +                              struct ofputil_meter_stats *);
>  
>  #endif /* NETDEV_OFFLOAD_TC_H */
> \ No newline at end of file

Does this file lack a trailing newline character as required by the 
coding style?
diff mbox series

Patch

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 6241ba8f7..aff90976c 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -4229,18 +4229,11 @@  static int
 dpif_netlink_meter_set(struct dpif *dpif_, ofproto_meter_id meter_id,
                        struct ofputil_meter_config *config)
 {
-    int err;
-
     if (probe_broken_meters(dpif_)) {
         return ENOMEM;
     }
 
-    err = dpif_netlink_meter_set__(dpif_, meter_id, config);
-    if (!err && dpif_offload_is_offload_enabled()) {
-        meter_offload_set(meter_id, config);
-    }
-
-    return err;
+    return dpif_netlink_meter_set__(dpif_, meter_id, config);
 }
 
 /* Retrieve statistics and/or delete meter 'meter_id'.  Statistics are
@@ -4339,30 +4332,16 @@  static int
 dpif_netlink_meter_get(const struct dpif *dpif, ofproto_meter_id meter_id,
                        struct ofputil_meter_stats *stats, uint16_t max_bands)
 {
-    int err;
-
-    err = dpif_netlink_meter_get_stats(dpif, meter_id, stats, max_bands,
-                                       OVS_METER_CMD_GET);
-    if (!err && dpif_offload_is_offload_enabled()) {
-        meter_offload_get(meter_id, stats);
-    }
-
-    return err;
+    return dpif_netlink_meter_get_stats(dpif, meter_id, stats, max_bands,
+                                        OVS_METER_CMD_GET);
 }
 
 static int
 dpif_netlink_meter_del(struct dpif *dpif, ofproto_meter_id meter_id,
                        struct ofputil_meter_stats *stats, uint16_t max_bands)
 {
-    int err;
-
-    err  = dpif_netlink_meter_get_stats(dpif, meter_id, stats,
+    return dpif_netlink_meter_get_stats(dpif, meter_id, stats,
                                         max_bands, OVS_METER_CMD_DEL);
-    if (!err && dpif_offload_is_offload_enabled()) {
-        meter_offload_del(meter_id, stats);
-    }
-
-    return err;
 }
 
 static bool
diff --git a/lib/dpif-offload-provider.h b/lib/dpif-offload-provider.h
index 6c1916b70..5f59bb330 100644
--- a/lib/dpif-offload-provider.h
+++ b/lib/dpif-offload-provider.h
@@ -131,6 +131,32 @@  struct dpif_offload_class {
      * successful, otherwise returns a positive errno value. */
     int (*flow_flush)(const struct dpif_offload *);
 
+    /* Adds or modifies the meter in 'dpif_offload' with the given 'meter_id'
+     * and the configuration in 'config'.
+     *
+     * The meter id specified through 'config->meter_id' is ignored. */
+    int (*meter_set)(const struct dpif_offload *, ofproto_meter_id meter_id,
+                     struct ofputil_meter_config *);
+
+    /* Queries HW for meter stats with the given 'meter_id'.  Store the stats
+     * of dropped packets to band 0. On failure, a non-zero error code is
+     * returned.
+     *
+     * Note that the 'stats' structure is already initialized, and only the
+     * available statistics should be incremented, not replaced.  Those fields
+     * are packet_in_count, byte_in_count and band[]->byte_count and
+     * band[]->packet_count. */
+    int (*meter_get)(const struct dpif_offload *, ofproto_meter_id meter_id,
+                     struct ofputil_meter_stats *);
+
+    /* Removes meter 'meter_id' from HW.  Store the stats of dropped packets to
+     * band 0.  On failure, a non-zero error code is returned.
+     *
+     * 'stats' may be passed in as NULL if no stats are needed.  See the above
+     * function for additional details on the 'stats' usage. */
+    int (*meter_del)(const struct dpif_offload *, ofproto_meter_id meter_id,
+                     struct ofputil_meter_stats *);
+
 
     /* These APIs operate directly on the provided netdev for performance
      * reasons.  They are intended for use in fast path processing and should
diff --git a/lib/dpif-offload-tc.c b/lib/dpif-offload-tc.c
index 63ce789c6..f258c7147 100644
--- a/lib/dpif-offload-tc.c
+++ b/lib/dpif-offload-tc.c
@@ -113,6 +113,8 @@  dpif_offload_tc_open(const struct dpif_offload_class *offload_class,
     offload_tc->once_enable = (struct ovsthread_once) \
                               OVSTHREAD_ONCE_INITIALIZER;
 
+    dpif_offload_tc_meter_init();
+
     *dpif_offload = &offload_tc->offload;
     return 0;
 }
@@ -290,5 +292,8 @@  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,
+    .meter_set = dpif_offload_tc_meter_set,
+    .meter_get = dpif_offload_tc_meter_get,
+    .meter_del = dpif_offload_tc_meter_del,
     .netdev_flow_flush = dpif_offload_tc_netdev_flow_flush,
 };
diff --git a/lib/dpif-offload.c b/lib/dpif-offload.c
index 537939d2d..5a13e0e44 100644
--- a/lib/dpif-offload.c
+++ b/lib/dpif-offload.c
@@ -31,6 +31,8 @@ 
 
 VLOG_DEFINE_THIS_MODULE(dpif_offload);
 
+static struct vlog_rate_limit rl_dbg = VLOG_RATE_LIMIT_INIT(1, 5);
+
 static struct ovs_mutex dpif_offload_mutex = OVS_MUTEX_INITIALIZER;
 static struct shash dpif_offload_classes \
     OVS_GUARDED_BY(dpif_offload_mutex) = \
@@ -724,6 +726,81 @@  dpif_offload_flow_flush(struct dpif *dpif)
     }
 }
 
+void
+dpif_offload_meter_set(const struct dpif *dpif, ofproto_meter_id meter_id,
+                       struct ofputil_meter_config *config)
+{
+    struct dp_offload *dp_offload = dpif_offload_get_dp_offload(dpif);
+    const struct dpif_offload *offload;
+
+    if (!dp_offload) {
+        return;
+    }
+
+    LIST_FOR_EACH (offload, dpif_list_node, &dp_offload->offload_providers) {
+        if (offload->class->meter_set) {
+            int err = offload->class->meter_set(offload, meter_id, config);
+            if (err) {
+                /* Offload APIs could fail, for example, because the offload
+                 * is not supported. This is fine, as the offload API should
+                 * take care of this. */
+                VLOG_DBG_RL(&rl_dbg,
+                            "Failed setting meter %u on dpif-offload provider"
+                            " %s, error %s", meter_id.uint32,
+                            dpif_offload_name(offload), ovs_strerror(err));
+            }
+        }
+    }
+}
+
+void
+dpif_offload_meter_get(const struct dpif *dpif, ofproto_meter_id meter_id,
+                       struct ofputil_meter_stats *stats)
+{
+    struct dp_offload *dp_offload = dpif_offload_get_dp_offload(dpif);
+    const struct dpif_offload *offload;
+
+    if (!dp_offload) {
+        return;
+    }
+
+    LIST_FOR_EACH (offload, dpif_list_node, &dp_offload->offload_providers) {
+        if (offload->class->meter_get) {
+            int err = offload->class->meter_get(offload, meter_id, stats);
+            if (err) {
+                VLOG_DBG_RL(&rl_dbg,
+                            "Failed getting meter %u on dpif-offload provider"
+                            " %s, error %s", meter_id.uint32,
+                            dpif_offload_name(offload), ovs_strerror(err));
+            }
+        }
+    }
+}
+
+void
+dpif_offload_meter_del(const struct dpif *dpif, ofproto_meter_id meter_id,
+                       struct ofputil_meter_stats *stats)
+{
+    struct dp_offload *dp_offload = dpif_offload_get_dp_offload(dpif);
+    const struct dpif_offload *offload;
+
+    if (!dp_offload) {
+        return;
+    }
+
+    LIST_FOR_EACH (offload, dpif_list_node, &dp_offload->offload_providers) {
+        if (offload->class->meter_del) {
+            int err = offload->class->meter_del(offload, meter_id, stats);
+            if (err) {
+                VLOG_DBG_RL(&rl_dbg,
+                            "Failed deleting meter %u on dpif-offload provider"
+                            " %s, error %s", meter_id.uint32,
+                            dpif_offload_name(offload), ovs_strerror(err));
+            }
+        }
+    }
+}
+
 
 int
 dpif_offload_netdev_flush_flows(struct netdev *netdev)
diff --git a/lib/dpif-offload.h b/lib/dpif-offload.h
index e31d5febf..ea1d04dda 100644
--- a/lib/dpif-offload.h
+++ b/lib/dpif-offload.h
@@ -51,6 +51,12 @@  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 *);
+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,
+                            struct ofputil_meter_stats *);
+void dpif_offload_meter_del(const struct dpif *dpif, ofproto_meter_id meter_id,
+                            struct ofputil_meter_stats *);
 
 /* Iterates through each DPIF_OFFLOAD in DPIF, using DUMP as state.
  *
diff --git a/lib/dpif.c b/lib/dpif.c
index 74dea3816..9f069c7c1 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -2008,6 +2008,7 @@  dpif_meter_set(struct dpif *dpif, ofproto_meter_id meter_id,
     if (!error) {
         VLOG_DBG_RL(&dpmsg_rl, "%s: DPIF meter %"PRIu32" set",
                     dpif_name(dpif), meter_id.uint32);
+        dpif_offload_meter_set(dpif, meter_id, config);
     } else {
         VLOG_WARN_RL(&error_rl, "%s: failed to set DPIF meter %"PRIu32": %s",
                      dpif_name(dpif), meter_id.uint32, ovs_strerror(error));
@@ -2027,6 +2028,7 @@  dpif_meter_get(const struct dpif *dpif, ofproto_meter_id meter_id,
     if (!error) {
         VLOG_DBG_RL(&dpmsg_rl, "%s: DPIF meter %"PRIu32" get stats",
                     dpif_name(dpif), meter_id.uint32);
+        dpif_offload_meter_get(dpif, meter_id, stats);
     } else {
         VLOG_WARN_RL(&error_rl,
                      "%s: failed to get DPIF meter %"PRIu32" stats: %s",
@@ -2050,6 +2052,7 @@  dpif_meter_del(struct dpif *dpif, ofproto_meter_id meter_id,
     if (!error) {
         VLOG_DBG_RL(&dpmsg_rl, "%s: DPIF meter %"PRIu32" deleted",
                     dpif_name(dpif), meter_id.uint32);
+        dpif_offload_meter_del(dpif, meter_id, stats);
     } else {
         VLOG_WARN_RL(&error_rl,
                      "%s: failed to delete DPIF meter %"PRIu32": %s",
diff --git a/lib/netdev-offload-provider.h b/lib/netdev-offload-provider.h
index 6e36ed4c8..07068bd82 100644
--- a/lib/netdev-offload-provider.h
+++ b/lib/netdev-offload-provider.h
@@ -91,33 +91,6 @@  struct netdev_flow_api {
      * takes ownership of a packet if errno != EOPNOTSUPP. */
     int (*hw_miss_packet_recover)(struct netdev *, struct dp_packet *);
 
-    /* Offloads or modifies the offloaded meter in HW with the given 'meter_id'
-     * and the configuration in 'config'. On failure, a non-zero error code is
-     * returned.
-     *
-     * The meter id specified through 'config->meter_id' is ignored. */
-    int (*meter_set)(ofproto_meter_id meter_id,
-                     struct ofputil_meter_config *config);
-
-    /* Queries HW for meter stats with the given 'meter_id'. Store the stats
-     * of dropped packets to band 0. On failure, a non-zero error code is
-     * returned.
-     *
-     * Note that the 'stats' structure is already initialized, and only the
-     * available statistics should be incremented, not replaced. Those fields
-     * are packet_in_count, byte_in_count and band[]->byte_count and
-     * band[]->packet_count. */
-    int (*meter_get)(ofproto_meter_id meter_id,
-                     struct ofputil_meter_stats *stats);
-
-    /* Removes meter 'meter_id' from HW. Store the stats of dropped packets to
-     * band 0. On failure, a non-zero error code is returned.
-     *
-     * 'stats' may be passed in as NULL if no stats are needed, See the above
-     * function for additional details on the 'stats' usage. */
-    int (*meter_del)(ofproto_meter_id meter_id,
-                     struct ofputil_meter_stats *stats);
-
     /* Initializies the netdev flow api.
      * Return 0 if successful, otherwise returns a positive errno value. */
     int (*init_flow_api)(struct netdev *);
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 1c554416c..8dc6b0c91 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -42,6 +42,7 @@ 
 #include "tc.h"
 #include "unaligned.h"
 #include "util.h"
+#include "dpif-offload.h"
 #include "dpif-provider.h"
 
 VLOG_DEFINE_THIS_MODULE(netdev_offload_tc);
@@ -3148,6 +3149,21 @@  tc_cleanup_policer_actions(struct id_pool *police_ids,
     hmap_destroy(&map);
 }
 
+void
+dpif_offload_tc_meter_init(void) {
+    static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
+
+    if (ovsthread_once_start(&once)) {
+        ovs_mutex_lock(&meter_police_ids_mutex);
+        meter_police_ids = id_pool_create(
+            METER_POLICE_IDS_BASE,
+            METER_POLICE_IDS_MAX - METER_POLICE_IDS_BASE + 1);
+        ovs_mutex_unlock(&meter_police_ids_mutex);
+
+        ovsthread_once_done(&once);
+    }
+}
+
 static int
 netdev_tc_init_flow_api(struct netdev *netdev)
 {
@@ -3202,9 +3218,9 @@  netdev_tc_init_flow_api(struct netdev *netdev)
         probe_vxlan_gbp_support(ifindex);
         probe_enc_flags_support(ifindex);
 
+        dpif_offload_tc_meter_init();
+
         ovs_mutex_lock(&meter_police_ids_mutex);
-        meter_police_ids = id_pool_create(METER_POLICE_IDS_BASE,
-                            METER_POLICE_IDS_MAX - METER_POLICE_IDS_BASE + 1);
         tc_cleanup_policer_actions(meter_police_ids, METER_POLICE_IDS_BASE,
                                    METER_POLICE_IDS_MAX);
         ovs_mutex_unlock(&meter_police_ids_mutex);
@@ -3330,15 +3346,24 @@  meter_free_police_index(uint32_t police_index)
     ovs_mutex_unlock(&meter_police_ids_mutex);
 }
 
-static int
-meter_tc_set_policer(ofproto_meter_id meter_id,
-                     struct ofputil_meter_config *config)
+int
+dpif_offload_tc_meter_set(const struct dpif_offload *offload OVS_UNUSED,
+                         ofproto_meter_id meter_id,
+                         struct ofputil_meter_config *config)
 {
     uint32_t police_index;
     uint32_t rate, burst;
     bool add_policer;
     int err;
 
+    if (!dpif_offload_is_offload_enabled()) {
+        /* FIXME: If offload is disabled, ignore this call. This preserves the
+         * behavior from before the dpif-offload implementation. However, it
+         * also retains the same bug where the meter_id is not offloaded if it
+         * was configured before offload was enabled. */
+        return 0;
+    }
+
     if (!config->bands || config->n_bands < 1 ||
         config->bands[0].type != OFPMBT13_DROP) {
         return 0;
@@ -3384,13 +3409,22 @@  meter_tc_set_policer(ofproto_meter_id meter_id,
     return err;
 }
 
-static int
-meter_tc_get_policer(ofproto_meter_id meter_id,
-                     struct ofputil_meter_stats *stats)
+int
+dpif_offload_tc_meter_get(const struct dpif_offload *offload OVS_UNUSED,
+                          ofproto_meter_id meter_id,
+                          struct ofputil_meter_stats *stats)
 {
     uint32_t police_index;
     int err = ENOENT;
 
+    if (!dpif_offload_is_offload_enabled()) {
+        /* FIXME: If offload is disabled, ignore this call. This preserves the
+         * behavior from before the dpif-offload implementation. However, it
+         * also retains the same bug where the meter_id is not offloaded if it
+         * was configured before offload was enabled. */
+        return 0;
+    }
+
     if (!meter_id_lookup(meter_id.uint32, &police_index)) {
         err = tc_get_policer_action(police_index, stats);
         if (err) {
@@ -3403,13 +3437,22 @@  meter_tc_get_policer(ofproto_meter_id meter_id,
     return err;
 }
 
-static int
-meter_tc_del_policer(ofproto_meter_id meter_id,
-                     struct ofputil_meter_stats *stats)
+int
+dpif_offload_tc_meter_del(const struct dpif_offload *offload OVS_UNUSED,
+                          ofproto_meter_id meter_id,
+                          struct ofputil_meter_stats *stats)
 {
     uint32_t police_index;
     int err = ENOENT;
 
+    if (!dpif_offload_is_offload_enabled()) {
+        /* FIXME: If offload is disabled, ignore this call. This preserves the
+         * behavior from before the dpif-offload implementation. However, it
+         * also retains the same bug where the meter_id is not offloaded if it
+         * was configured before offload was enabled. */
+        return 0;
+    }
+
     if (!meter_id_lookup(meter_id.uint32, &police_index)) {
         err = tc_del_policer_action(police_index, stats);
         if (err && err != ENOENT) {
@@ -3434,8 +3477,5 @@  const struct netdev_flow_api netdev_offload_tc = {
    .flow_get = netdev_tc_flow_get,
    .flow_del = netdev_tc_flow_del,
    .flow_get_n_flows = netdev_tc_get_n_flows,
-   .meter_set = meter_tc_set_policer,
-   .meter_get = meter_tc_get_policer,
-   .meter_del = meter_tc_del_policer,
    .init_flow_api = netdev_tc_init_flow_api,
 };
diff --git a/lib/netdev-offload-tc.h b/lib/netdev-offload-tc.h
index 74dd33b8b..d0485b4ad 100644
--- a/lib/netdev-offload-tc.h
+++ b/lib/netdev-offload-tc.h
@@ -18,10 +18,18 @@ 
  #define NETDEV_OFFLOAD_TC_H
 
 /* Forward declarations of private structures. */
+struct dpif_offload;
 struct netdev;
 
 /* Netdev-specific offload functions.  These should only be used by the
  * associated dpif offload provider. */
 int netdev_offload_tc_flow_flush(struct netdev *);
+void dpif_offload_tc_meter_init(void);
+int dpif_offload_tc_meter_set(const struct dpif_offload *, ofproto_meter_id,
+                              struct ofputil_meter_config *);
+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 *);
 
 #endif /* NETDEV_OFFLOAD_TC_H */
\ No newline at end of file
diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c
index 2b1c99fb6..abbb930be 100644
--- a/lib/netdev-offload.c
+++ b/lib/netdev-offload.c
@@ -58,9 +58,6 @@ 
 
 VLOG_DEFINE_THIS_MODULE(netdev_offload);
 
-
-static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
-
 /* XXX: Temporarily duplicates definition in dpif-offload-dpdk.c. */
 #define MAX_OFFLOAD_THREAD_NB 10
 #define DEFAULT_OFFLOAD_THREAD_NB 1
@@ -204,64 +201,6 @@  netdev_assign_flow_api(struct netdev *netdev)
     return -1;
 }
 
-void
-meter_offload_set(ofproto_meter_id meter_id,
-                  struct ofputil_meter_config *config)
-{
-    struct netdev_registered_flow_api *rfa;
-
-    CMAP_FOR_EACH (rfa, cmap_node, &netdev_flow_apis) {
-        if (rfa->flow_api->meter_set) {
-            int ret = rfa->flow_api->meter_set(meter_id, config);
-            if (ret) {
-                VLOG_DBG_RL(&rl, "Failed setting meter %u for flow api %s, "
-                            "error %d", meter_id.uint32, rfa->flow_api->type,
-                            ret);
-           }
-        }
-    }
-    /* Offload APIs could fail, for example, because the offload is not
-     * supported. This is fine, as the offload API should take care of this. */
-}
-
-int
-meter_offload_get(ofproto_meter_id meter_id, struct ofputil_meter_stats *stats)
-{
-    struct netdev_registered_flow_api *rfa;
-
-    CMAP_FOR_EACH (rfa, cmap_node, &netdev_flow_apis) {
-        if (rfa->flow_api->meter_get) {
-            int ret = rfa->flow_api->meter_get(meter_id, stats);
-            if (ret) {
-                VLOG_DBG_RL(&rl, "Failed getting meter %u for flow api %s, "
-                            "error %d", meter_id.uint32, rfa->flow_api->type,
-                            ret);
-           }
-        }
-    }
-
-    return 0;
-}
-
-int
-meter_offload_del(ofproto_meter_id meter_id, struct ofputil_meter_stats *stats)
-{
-    struct netdev_registered_flow_api *rfa;
-
-    CMAP_FOR_EACH (rfa, cmap_node, &netdev_flow_apis) {
-        if (rfa->flow_api->meter_del) {
-            int ret = rfa->flow_api->meter_del(meter_id, stats);
-            if (ret) {
-                VLOG_DBG_RL(&rl, "Failed deleting meter %u for flow api %s, "
-                            "error %d", meter_id.uint32, rfa->flow_api->type,
-                            ret);
-            }
-        }
-    }
-
-    return 0;
-}
-
 int
 netdev_flow_dump_create(struct netdev *netdev, struct netdev_flow_dump **dump,
                         bool terse)
diff --git a/lib/netdev-offload.h b/lib/netdev-offload.h
index 0bee005fd..8552b6e0b 100644
--- a/lib/netdev-offload.h
+++ b/lib/netdev-offload.h
@@ -154,10 +154,6 @@  int netdev_ports_flow_get(const char *dpif_type, struct match *match,
 int netdev_ports_get_n_flows(const char *dpif_type,
                              odp_port_t port_no, uint64_t *n_flows);
 
-void meter_offload_set(ofproto_meter_id, struct ofputil_meter_config *);
-int meter_offload_get(ofproto_meter_id, struct ofputil_meter_stats *);
-int meter_offload_del(ofproto_meter_id, struct ofputil_meter_stats *);
-
 #ifdef  __cplusplus
 }
 #endif