From: Tianyu Yuan <[email protected]> Allow revalidator to continuously delete police in kernel tc datapath until it is deleted.
In current implementation, polices in tc datapath will not deleted when they are being used and these remaining polices will be cleared when the openvswitch restarts. This patch supports revalidator to delete remaining polices in tc datapath by storing meter_id deleted unsuccessfully in a hmap and continuously deleting them in revalidator until success. Signed-off-by: Tianyu Yuan <[email protected]> Signed-off-by: Simon Horman <[email protected]> --- lib/dpif-netdev.c | 1 + lib/dpif-netlink.c | 26 +++++++++++++++++++++++++- lib/dpif-provider.h | 4 ++++ lib/dpif.c | 7 +++++++ lib/dpif.h | 2 ++ lib/netdev-offload-tc.c | 5 ++++- lib/netdev-offload.c | 8 +++++--- ofproto/ofproto-dpif-upcall.c | 6 ++++++ ofproto/ofproto-dpif.c | 10 +++++++++- ofproto/ofproto-dpif.h | 3 +++ 10 files changed, 66 insertions(+), 6 deletions(-) diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index c9f7179c3b4c..878fe5933d1a 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -9689,6 +9689,7 @@ const struct dpif_class dpif_netdev_class = { dpif_netdev_meter_set, dpif_netdev_meter_get, dpif_netdev_meter_del, + NULL, dpif_netdev_bond_add, dpif_netdev_bond_del, dpif_netdev_bond_stats_get, diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c index 026b0daa8d83..8b95912aea97 100644 --- a/lib/dpif-netlink.c +++ b/lib/dpif-netlink.c @@ -25,6 +25,7 @@ #include <net/if.h> #include <linux/types.h> #include <linux/pkt_sched.h> +#include <linux/rtnetlink.h> #include <poll.h> #include <stdlib.h> #include <strings.h> @@ -37,6 +38,7 @@ #include "dpif-provider.h" #include "fat-rwlock.h" #include "flow.h" +#include "id-pool.h" #include "netdev-linux.h" #include "netdev-offload.h" #include "netdev-provider.h" @@ -61,6 +63,7 @@ #include "packets.h" #include "random.h" #include "sset.h" +#include "tc.h" #include "timeval.h" #include "unaligned.h" #include "util.h" @@ -4363,12 +4366,32 @@ dpif_netlink_meter_del(struct dpif *dpif, ofproto_meter_id meter_id, err = dpif_netlink_meter_get_stats(dpif, meter_id, stats, max_bands, OVS_METER_CMD_DEL); if (!err && netdev_is_flow_api_enabled()) { - meter_offload_del(meter_id, stats); + return meter_offload_del(meter_id, stats); } return err; } +static void +dpif_netlink_meter_revalidate(struct dpif *dpif_ OVS_UNUSED, + struct hmap *meter_map) +{ + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); + ofproto_meter_id meter_id; + struct id_node *id_node; + + HMAP_FOR_EACH_POP (id_node, node, meter_map) { + meter_id.uint32 = id_node->id; + + if (meter_offload_del(meter_id, NULL) == EPERM) { + id_hmap_add(meter_map, meter_id.uint32); + } else { + VLOG_DBG_RL(&rl, "Delete meter %u in datapath success", + meter_id.uint32); + } + } +} + static bool probe_broken_meters__(struct dpif *dpif) { @@ -4588,6 +4611,7 @@ const struct dpif_class dpif_netlink_class = { dpif_netlink_meter_set, dpif_netlink_meter_get, dpif_netlink_meter_del, + dpif_netlink_meter_revalidate, NULL, /* bond_add */ NULL, /* bond_del */ NULL, /* bond_stats_get */ diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h index 12477a24feee..794fbc21686a 100644 --- a/lib/dpif-provider.h +++ b/lib/dpif-provider.h @@ -631,6 +631,10 @@ struct dpif_class { int (*meter_del)(struct dpif *, ofproto_meter_id meter_id, struct ofputil_meter_stats *, uint16_t n_bands); + /* Checks unneeded meters from 'dpif' and removes them. They may + * be caused by deleting in-use meters. */ + void (*meter_revalidate)(struct dpif *, struct hmap *meter_map); + /* Adds a bond with 'bond_id' and the member-map to 'dpif'. */ int (*bond_add)(struct dpif *dpif, uint32_t bond_id, odp_port_t *member_map); diff --git a/lib/dpif.c b/lib/dpif.c index fe4db83fbfee..c8718239117d 100644 --- a/lib/dpif.c +++ b/lib/dpif.c @@ -2028,6 +2028,13 @@ dpif_meter_del(struct dpif *dpif, ofproto_meter_id meter_id, return error; } +void +dpif_meter_revalidate(struct dpif *dpif, struct hmap *meter_map){ + if (dpif->dpif_class->meter_revalidate) { + dpif->dpif_class->meter_revalidate(dpif, meter_map); + } +} + int dpif_bond_add(struct dpif *dpif, uint32_t bond_id, odp_port_t *member_map) { diff --git a/lib/dpif.h b/lib/dpif.h index 6cb4dae6d8d7..e181e4ee1d6c 100644 --- a/lib/dpif.h +++ b/lib/dpif.h @@ -378,6 +378,7 @@ #include "dpdk.h" #include "dp-packet.h" +#include "id-pool.h" #include "netdev.h" #include "openflow/openflow.h" #include "openvswitch/ofp-meter.h" @@ -905,6 +906,7 @@ int dpif_meter_get(const struct dpif *, ofproto_meter_id meter_id, struct ofputil_meter_stats *, uint16_t n_bands); int dpif_meter_del(struct dpif *, ofproto_meter_id meter_id, struct ofputil_meter_stats *, uint16_t n_bands); +void dpif_meter_revalidate(struct dpif *dpif, struct hmap *meter_map); /* Bonding. */ diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c index 15d1c36aa04e..f87a5e05e202 100644 --- a/lib/netdev-offload-tc.c +++ b/lib/netdev-offload-tc.c @@ -2980,8 +2980,11 @@ meter_tc_del_policer(ofproto_meter_id meter_id, police_index, meter_id.uint32, ovs_strerror(err)); } else { meter_free_police_index(police_index); + /* Do not remove the mapping between meter_id and police_index + * until this meter is deleted successfully in datapath. This + * mapping will be used in meter deletion in revalidator. */ + meter_id_remove(meter_id.uint32); } - meter_id_remove(meter_id.uint32); } return err; diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c index 4592262bd34e..0769f796e5b7 100644 --- a/lib/netdev-offload.c +++ b/lib/netdev-offload.c @@ -241,19 +241,21 @@ int meter_offload_del(ofproto_meter_id meter_id, struct ofputil_meter_stats *stats) { struct netdev_registered_flow_api *rfa; + int ret = 0; 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) { + int err = rfa->flow_api->meter_del(meter_id, stats); + if (err) { VLOG_DBG_RL(&rl, "Failed deleting meter %u for flow api %s, " "error %d", meter_id.uint32, rfa->flow_api->type, ret); + ret = err; } } } - return 0; + return ret; } int diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index 31ac02d116fc..b802c9c7f13a 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -2828,6 +2828,12 @@ revalidate(struct revalidator *revalidator) } ovsrcu_quiesce(); } + /* When fail_meter_hmap is not empty, revalidate to delete meters in this + * hashmap. */ + if (hmap_count(&udpif->backer->fail_meter_hmap)) { + dpif_meter_revalidate(udpif->dpif, &udpif->backer->fail_meter_hmap); + } + dpif_flow_dump_thread_destroy(dump_thread); ofpbuf_uninit(&odp_actions); } diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index f87e27a8cd7f..591af5503765 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -835,6 +835,7 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp) dpif_meter_get_features(backer->dpif, &features); if (features.max_meters) { backer->meter_ids = id_pool_create(0, features.max_meters); + hmap_init(&backer->fail_meter_hmap); } else { backer->meter_ids = NULL; } @@ -6790,8 +6791,15 @@ static void free_meter_id(struct free_meter_id_args *args) { struct ofproto_dpif *ofproto = args->ofproto; + int err; + + err = dpif_meter_del(ofproto->backer->dpif, args->meter_id, NULL, 0); + /* Add fail deleted meter to fail_meter_hmap and waiting to be deleted in + * revalidator. */ + if (err == EPERM) { + id_hmap_add(&ofproto->backer->fail_meter_hmap, args->meter_id.uint32); + } - dpif_meter_del(ofproto->backer->dpif, args->meter_id, NULL, 0); id_pool_free_id(ofproto->backer->meter_ids, args->meter_id.uint32); free(args); } diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h index d8e0cd37ac5b..befc68d13baa 100644 --- a/ofproto/ofproto-dpif.h +++ b/ofproto/ofproto-dpif.h @@ -61,6 +61,7 @@ struct ofproto_dpif; struct uuid; struct xlate_cache; struct xlate_ctx; +struct id_pool; /* Number of implemented OpenFlow tables. */ enum { N_TABLES = 255 }; @@ -260,6 +261,8 @@ struct dpif_backer { /* Meter. */ struct id_pool *meter_ids; /* Datapath meter allocation. */ + struct hmap fail_meter_hmap; /* Hashmap for meter with failed deletion + * in datapath */ /* Connection tracking. */ struct id_pool *tp_ids; /* Datapath timeout policy id -- 2.30.2 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
