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 <[email protected]> --- 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(-) diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c index f51405a16..17e938c9f 100644 --- a/lib/dpif-netlink.c +++ b/lib/dpif-netlink.c @@ -4225,18 +4225,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 @@ -4327,30 +4320,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 6b4d4269f..e86945b96 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 e520f08cf..885da5bdc 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) = \ @@ -723,6 +725,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 47dd9405e..982ad57d8 100644 --- a/lib/dpif.c +++ b/lib/dpif.c @@ -2003,6 +2003,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)); @@ -2022,6 +2023,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", @@ -2045,6 +2047,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 7de4be88a..3da118ed3 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 692282a7f..9e18b4415 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-rte_flow.c. */ #define MAX_OFFLOAD_THREAD_NB 10 #define DEFAULT_OFFLOAD_THREAD_NB 1 @@ -203,64 +200,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 -- 2.50.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
