On 23 Sep 2022, at 10:35, Simon Horman wrote:
> From: Yifan Li <[email protected]> > > Allow revalidator for hardware offload of meters via OVS-TC. > Without revalidator, tc meter action can not be deleted while > flow exists. The revalidator fix this bug by continuously > checking existing meters and delete the unneeded ones. The > autotest cases of revalidator are also added. This is not a review, but just some quick observations/questions. - Please undo the n_handlers to _n_handlers renames, as it does not make sense for this patch. - New TC related code was added to dpif-netdev.c this is not the place where such code should live. - Log messages do not need a trailing \n, and please have them uniform, i.e. start with a capital for all new log messages. - Can you explain why we need this expensive revalidate process and isn’t there a better way to make sure they are cleaned up properly? Also including Jianbo who added the initial code. //Eelco > Signed-off-by: Yifan Li <[email protected]> > Signed-off-by: Simon Horman <[email protected]> > --- > lib/dpif-netdev.c | 1 + > lib/dpif-netlink.c | 257 +++++++++++++++++++++++++++---- > lib/dpif-netlink.h | 2 + > lib/dpif-provider.h | 5 + > lib/dpif.c | 15 +- > lib/id-pool.c | 13 ++ > lib/id-pool.h | 4 +- > lib/netdev-linux.c | 6 + > lib/netdev-offload-tc.c | 11 +- > lib/tc.c | 5 - > lib/tc.h | 10 +- > ofproto/ofproto-dpif-upcall.c | 5 + > ofproto/ofproto-dpif.h | 2 + > tests/system-offloads-traffic.at | 4 + > 14 files changed, 301 insertions(+), 39 deletions(-) > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index a45b460145c6..365aacadb03a 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -9583,6 +9583,7 @@ const struct dpif_class dpif_netdev_class = { > dpif_netdev_meter_set, > dpif_netdev_meter_get, > dpif_netdev_meter_del, > + NULL, /* meter_revalidate */ > 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 a620a6ec52dd..4eee4761dc6b 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> > @@ -48,6 +49,7 @@ > #include "netlink.h" > #include "netnsid.h" > #include "odp-util.h" > +#include "ofproto/ofproto-dpif.h" > #include "openvswitch/dynamic-string.h" > #include "openvswitch/flow.h" > #include "openvswitch/hmap.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" > @@ -2571,18 +2574,18 @@ static uint32_t > dpif_netlink_calculate_n_handlers(void) > { > uint32_t total_cores = count_total_cores(); > - uint32_t n_handlers = count_cpu_cores(); > + uint32_t _n_handlers = count_cpu_cores(); > uint32_t next_prime_num; > > /* If not all cores are available to OVS, create additional handler > * threads to ensure more fair distribution of load between them. > */ > - if (n_handlers < total_cores && total_cores > 2) { > - next_prime_num = next_prime(n_handlers + 1); > - n_handlers = MIN(next_prime_num, total_cores); > + if (_n_handlers < total_cores && total_cores > 2) { > + next_prime_num = next_prime(_n_handlers + 1); > + _n_handlers = MIN(next_prime_num, total_cores); > } > > - return n_handlers; > + return _n_handlers; > } > > static int > @@ -2591,17 +2594,17 @@ dpif_netlink_refresh_handlers_cpu_dispatch(struct > dpif_netlink *dpif) > { > int handler_id; > int error = 0; > - uint32_t n_handlers; > uint32_t *upcall_pids; > + uint32_t _n_handlers; > > - n_handlers = dpif_netlink_calculate_n_handlers(); > - if (dpif->n_handlers != n_handlers) { > + _n_handlers = dpif_netlink_calculate_n_handlers(); > + if (dpif->n_handlers != _n_handlers) { > VLOG_DBG("Dispatch mode(per-cpu): initializing %d handlers", > - n_handlers); > + _n_handlers); > destroy_all_handlers(dpif); > - upcall_pids = xzalloc(n_handlers * sizeof *upcall_pids); > - dpif->handlers = xzalloc(n_handlers * sizeof *dpif->handlers); > - for (handler_id = 0; handler_id < n_handlers; handler_id++) { > + upcall_pids = xzalloc(_n_handlers * sizeof *upcall_pids); > + dpif->handlers = xzalloc(_n_handlers * sizeof *dpif->handlers); > + for (handler_id = 0; handler_id < _n_handlers; handler_id++) { > struct dpif_handler *handler = &dpif->handlers[handler_id]; > error = create_nl_sock(dpif, &handler->sock); > if (error) { > @@ -2615,9 +2618,9 @@ dpif_netlink_refresh_handlers_cpu_dispatch(struct > dpif_netlink *dpif) > handler_id, upcall_pids[handler_id]); > } > > - dpif->n_handlers = n_handlers; > + dpif->n_handlers = _n_handlers; > error = dpif_netlink_set_handler_pids(&dpif->dpif, upcall_pids, > - n_handlers); > + _n_handlers); > free(upcall_pids); > } > return error; > @@ -2629,7 +2632,7 @@ dpif_netlink_refresh_handlers_cpu_dispatch(struct > dpif_netlink *dpif) > * backing kernel vports. */ > static int > dpif_netlink_refresh_handlers_vport_dispatch(struct dpif_netlink *dpif, > - uint32_t n_handlers) > + uint32_t _n_handlers) > OVS_REQ_WRLOCK(dpif->upcall_lock) > { > unsigned long int *keep_channels; > @@ -2641,13 +2644,13 @@ dpif_netlink_refresh_handlers_vport_dispatch(struct > dpif_netlink *dpif, > int retval = 0; > size_t i; > > - ovs_assert(!WINDOWS || n_handlers <= 1); > + ovs_assert(!WINDOWS || _n_handlers <= 1); > ovs_assert(!WINDOWS || dpif->n_handlers <= 1); > > - if (dpif->n_handlers != n_handlers) { > + if (dpif->n_handlers != _n_handlers) { > destroy_all_channels(dpif); > - dpif->handlers = xzalloc(n_handlers * sizeof *dpif->handlers); > - for (i = 0; i < n_handlers; i++) { > + dpif->handlers = xzalloc(_n_handlers * sizeof *dpif->handlers); > + for (i = 0; i < _n_handlers; i++) { > int error; > struct dpif_handler *handler = &dpif->handlers[i]; > > @@ -2665,10 +2668,10 @@ dpif_netlink_refresh_handlers_vport_dispatch(struct > dpif_netlink *dpif, > return error; > } > } > - dpif->n_handlers = n_handlers; > + dpif->n_handlers = _n_handlers; > } > > - for (i = 0; i < n_handlers; i++) { > + for (i = 0; i < _n_handlers; i++) { > struct dpif_handler *handler = &dpif->handlers[i]; > > handler->event_offset = handler->n_events = 0; > @@ -2801,7 +2804,7 @@ dpif_netlink_recv_set(struct dpif *dpif_, bool enable) > } > > static int > -dpif_netlink_handlers_set(struct dpif *dpif_, uint32_t n_handlers) > +dpif_netlink_handlers_set(struct dpif *dpif_, uint32_t _n_handlers) > { > struct dpif_netlink *dpif = dpif_netlink_cast(dpif_); > int error = 0; > @@ -2809,7 +2812,7 @@ dpif_netlink_handlers_set(struct dpif *dpif_, uint32_t > n_handlers) > #ifdef _WIN32 > /* Multiple upcall handlers will be supported once kernel datapath > supports > * it. */ > - if (n_handlers > 1) { > + if (_n_handlers > 1) { > return error; > } > #endif > @@ -2820,7 +2823,7 @@ dpif_netlink_handlers_set(struct dpif *dpif_, uint32_t > n_handlers) > error = dpif_netlink_refresh_handlers_cpu_dispatch(dpif); > } else { > error = dpif_netlink_refresh_handlers_vport_dispatch(dpif, > - n_handlers); > + > _n_handlers); > } > } > fat_rwlock_unlock(&dpif->upcall_lock); > @@ -2829,12 +2832,13 @@ dpif_netlink_handlers_set(struct dpif *dpif_, > uint32_t n_handlers) > } > > static bool > -dpif_netlink_number_handlers_required(struct dpif *dpif_, uint32_t > *n_handlers) > +dpif_netlink_number_handlers_required(struct dpif *dpif_, > + uint32_t *_n_handlers) > { > struct dpif_netlink *dpif = dpif_netlink_cast(dpif_); > > if (dpif_netlink_upcall_per_cpu(dpif)) { > - *n_handlers = dpif_netlink_calculate_n_handlers(); > + *_n_handlers = dpif_netlink_calculate_n_handlers(); > return true; > } > > @@ -2843,7 +2847,7 @@ dpif_netlink_number_handlers_required(struct dpif > *dpif_, uint32_t *n_handlers) > > static int > dpif_netlink_queue_to_priority(const struct dpif *dpif OVS_UNUSED, > - uint32_t queue_id, uint32_t *priority) > + uint32_t queue_id, uint32_t *priority) > { > if (queue_id < 0xf000) { > *priority = TC_H_MAKE(1 << 16, queue_id + 1); > @@ -4161,6 +4165,191 @@ dpif_netlink_meter_get_features(const struct dpif > *dpif_, > ofpbuf_delete(msg); > } > > +static bool dpif_netlink_meter_should_revalidate(struct dpif_backer *backer, > + uint32_t meter_id) > +{ > + return !id_pool_id_exist(backer->meter_ids, meter_id); > +} > + > +static void > +dpif_tc_meter_revalidate(struct dpif *dpif_ OVS_UNUSED, > + struct dpif_backer *backer, struct ofpbuf *reply) > +{ > + static struct nl_policy actions_orders_policy[ACT_MAX_NUM + 1] = {}; > + struct nlattr *actions_orders[ARRAY_SIZE(actions_orders_policy)]; > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5,20); > + static const struct nl_policy tca_root_policy[] = { > + [TCA_ACT_TAB] = { .type = NL_A_NESTED, .optional = false }, > + [TCA_ROOT_COUNT] = { .type = NL_A_U32, .optional = false }, > + }; > + struct nlattr *action_root_attrs[ARRAY_SIZE(tca_root_policy)]; > + static const struct nl_policy police_policy[] = { > + [TCA_POLICE_TBF] = { .type = NL_A_UNSPEC, > + .min_len = sizeof(struct tc_police), > + .optional = false}, > + }; > + struct nlattr *action_police_tab[ARRAY_SIZE(police_policy)]; > + static const struct nl_policy act_policy[] = { > + [TCA_ACT_KIND] = { .type = NL_A_STRING, .optional = false, }, > + [TCA_ACT_COOKIE] = { .type = NL_A_UNSPEC, .optional = true, }, > + [TCA_ACT_OPTIONS] = { .type = NL_A_NESTED, .optional = true, }, > + [TCA_ACT_STATS] = { .type = NL_A_NESTED, .optional = false, }, > + }; > + struct nlattr *action_police_attrs[ARRAY_SIZE(act_policy)]; > + const int max_size = ARRAY_SIZE(actions_orders_policy); > + const struct tc_police *tc_police = NULL; > + ofproto_meter_id meter_id; > + size_t revalidate_num; > + size_t act_count; > + uint32_t index; > + int i; > + > + if (!reply) { > + VLOG_ERR_RL(&rl, "null reply message during meter revalidation\n"); > + return; > + } > + > + if (reply->size <= NLMSG_ALIGNTO + NLMSG_HDRLEN) { > + VLOG_DBG_RL(&rl, "no meters present in tc during meter " > + "revalidation\n"); > + return; > + } > + > + if (!nl_policy_parse(reply, NLMSG_HDRLEN + sizeof(struct tcamsg), > + tca_root_policy, action_root_attrs, > + ARRAY_SIZE(action_root_attrs))) { > + VLOG_ERR_RL(&rl, "failed to parse reply message during meter " > + "revalidation\n"); > + return; > + } > + > + act_count = nl_attr_get_u32(action_root_attrs[TCA_ROOT_COUNT]); > + if (!act_count) { > + VLOG_ERR_RL(&rl, "no police action returned in message during " > + "meter revalidation\n"); > + return; > + } > + > + for (i = 0; i < max_size; i++) { > + actions_orders_policy[i].type = NL_A_NESTED; > + actions_orders_policy[i].optional = true; > + } > + > + revalidate_num = act_count > ACT_MAX_NUM ? > + (ACT_MAX_NUM + 1) : (act_count + 1); > + if (!nl_parse_nested(action_root_attrs[TCA_ACT_TAB], > actions_orders_policy, > + actions_orders, revalidate_num)) { > + VLOG_ERR_RL(&rl, "failed to parse TCA_ACT_TAB during meter " > + "revalidation of act_count\n"); > + return; > + } > + > + for (i = 0; i < revalidate_num; i++) { > + if (!actions_orders[i]) { > + continue; > + } > + > + if (!nl_parse_nested(actions_orders[i], act_policy, > + action_police_attrs, ARRAY_SIZE(act_policy))) { > + VLOG_ERR_RL(&rl, "failed to parse police action during meter " > + "revalidation\n"); > + return; > + } > + > + if (strcmp(nl_attr_get_string(action_police_attrs[TCA_KIND]), > + "police")) { > + VLOG_EMER("non-police action found during meter revalidation\n"); > + continue; > + } > + > + if (!nl_parse_nested(action_police_attrs[TCA_ACT_OPTIONS], > + police_policy, action_police_tab, > + ARRAY_SIZE(action_police_tab))) { > + VLOG_ERR_RL(&rl, "failed to parse the single police action " > + "during meter revalidation\n"); > + return; > + } > + > + tc_police = nl_attr_get_unspec(action_police_tab[TCA_POLICE_TBF], > + sizeof *tc_police); > + if (!tc_police) { > + VLOG_ERR_RL(&rl, "can not get police struct during meter " > + "revalidation\n"); > + continue; > + } > + index = tc_police->index; > + /* The range of meter index is 0x10000000 to 0x1fffffff > + * If not meter, continue */ > + if (!tc_is_meter_index(index)) { > + VLOG_DBG_RL(&rl, "Meter index : %d is not meter:" > + "action = %d, rate = %d, burst = %d, mtu = %d", > + index, tc_police->action, tc_police->rate.rate, > + tc_police->burst, tc_police->mtu); > + continue; > + } > + > + /* transform police index to meter id */ > + index = POLICY_INDEX_TO_METER_ID(index); > + if (dpif_netlink_meter_should_revalidate(backer, index)) { > + meter_id.uint32 = index; > + VLOG_DBG_RL(&rl, "revalidate meter id %u for police index " > + "%08x\n", index, tc_police->index); > + meter_offload_del(meter_id, NULL); > + } > + } > +} > + > +static int > +dpif_netlink_meter_revalidate__(struct dpif *dpif_ OVS_UNUSED, > + struct dpif_backer *backer) > +{ > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5,20); > + struct nla_bitfield32 dump_flags = { TCA_DUMP_FLAGS_TERSE, > + TCA_DUMP_FLAGS_TERSE}; > + struct ofpbuf request; > + struct ofpbuf *reply; > + struct tcamsg *tcmsg; > + size_t total_offset; > + size_t act_offset; > + int prio = 0; > + int error; > + > + if (!netdev_is_flow_api_enabled()) { > + return 0; > + } > + /* Make tc action request */ > + ofpbuf_init(&request, 16384); > + nl_msg_put_nlmsghdr(&request, sizeof *tcmsg, RTM_GETACTION, > + NLM_F_REQUEST | NLM_F_DUMP); > + tcmsg = ofpbuf_put_zeros(&request, sizeof *tcmsg); > + tcmsg->tca_family = AF_UNSPEC; > + if (!tcmsg) { > + return ENODEV; > + } > + > + /* Data path interface netlink police start */ > + total_offset = nl_msg_start_nested(&request, TCA_ACT_TAB); > + act_offset = nl_msg_start_nested(&request, ++prio); > + /* Send request */ > + nl_msg_put_string(&request, TCA_KIND, "police"); > + > + /* Data path interface netlink police end */ > + nl_msg_end_nested(&request, act_offset); > + nl_msg_end_nested(&request, total_offset); > + > + nl_msg_put_unspec(&request, TCA_ROOT_FLAGS, &dump_flags, > + sizeof dump_flags); > + error = tc_transact(&request, &reply); > + if (error) { > + VLOG_ERR_RL(&rl, "failed to send dump netlink msg for revalidate " > + "error %d\n", error); > + return error; > + } > + dpif_tc_meter_revalidate(dpif_, backer, reply); > + ofpbuf_delete(reply); > + return 0; > +} > + > static int > dpif_netlink_meter_set__(struct dpif *dpif_, ofproto_meter_id meter_id, > struct ofputil_meter_config *config) > @@ -4370,6 +4559,19 @@ dpif_netlink_meter_del(struct dpif *dpif, > ofproto_meter_id meter_id, > return err; > } > > +static int > +dpif_netlink_meter_revalidate(struct dpif *dpif_, struct dpif_backer *backer) > +{ > + int err; > + > + if (probe_broken_meters(dpif_)) { > + return ENOMEM; > + } > + err = dpif_netlink_meter_revalidate__(dpif_, backer); > + > + return err; > +} > + > static bool > probe_broken_meters__(struct dpif *dpif) > { > @@ -4589,6 +4791,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-netlink.h b/lib/dpif-netlink.h > index 24294bc42dc3..b3b113dc407c 100644 > --- a/lib/dpif-netlink.h > +++ b/lib/dpif-netlink.h > @@ -17,6 +17,8 @@ > #ifndef DPIF_NETLINK_H > #define DPIF_NETLINK_H 1 > > +#include <linux/pkt_cls.h> > + > #include <stdbool.h> > #include <stddef.h> > #include <stdint.h> > diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h > index 12477a24feee..efc6ca989fa9 100644 > --- a/lib/dpif-provider.h > +++ b/lib/dpif-provider.h > @@ -25,6 +25,7 @@ > #include "openflow/openflow.h" > #include "dpif.h" > #include "util.h" > +#include "ofproto/ofproto-dpif.h" > > #ifdef __cplusplus > extern "C" { > @@ -631,6 +632,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. */ > + int (*meter_revalidate)(struct dpif *, struct dpif_backer *); > + > /* 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 40f5fe44606e..ceee076af195 100644 > --- a/lib/dpif.c > +++ b/lib/dpif.c > @@ -1491,12 +1491,12 @@ dpif_recv_set(struct dpif *dpif, bool enable) > * > * Returns 0 if successful, otherwise a positive errno value. */ > int > -dpif_handlers_set(struct dpif *dpif, uint32_t n_handlers) > +dpif_handlers_set(struct dpif *dpif, uint32_t _n_handlers) > { > int error = 0; > > if (dpif->dpif_class->handlers_set) { > - error = dpif->dpif_class->handlers_set(dpif, n_handlers); > + error = dpif->dpif_class->handlers_set(dpif, _n_handlers); > log_operation(dpif, "handlers_set", error); > } > return error; > @@ -1510,10 +1510,10 @@ dpif_handlers_set(struct dpif *dpif, uint32_t > n_handlers) > * If not, returns 'false' > */ > bool > -dpif_number_handlers_required(struct dpif *dpif, uint32_t *n_handlers) > +dpif_number_handlers_required(struct dpif *dpif, uint32_t *_n_handlers) > { > if (dpif->dpif_class->number_handlers_required) { > - return dpif->dpif_class->number_handlers_required(dpif, n_handlers); > + return dpif->dpif_class->number_handlers_required(dpif, _n_handlers); > } > return false; > } > @@ -2028,6 +2028,13 @@ dpif_meter_del(struct dpif *dpif, ofproto_meter_id > meter_id, > return error; > } > > +int > +dpif_meter_revalidate(struct dpif *dpif, struct dpif_backer *backer){ > + return dpif->dpif_class->meter_revalidate > + ? dpif->dpif_class->meter_revalidate(dpif, backer) > + : EOPNOTSUPP; > +} > + > int > dpif_bond_add(struct dpif *dpif, uint32_t bond_id, odp_port_t *member_map) > { > diff --git a/lib/id-pool.c b/lib/id-pool.c > index 69910ad08e83..64096a184563 100644 > --- a/lib/id-pool.c > +++ b/lib/id-pool.c > @@ -155,3 +155,16 @@ id_pool_free_id(struct id_pool *pool, uint32_t id) > } > } > } > + > +bool > +id_pool_id_exist(struct id_pool *pool, uint32_t id) > +{ > + return !!id_pool_find(pool, id); > +} > + > +uint32_t id_pool_base(struct id_pool *pool){ > + return pool->base; > +} > +uint32_t id_pool_n(struct id_pool *pool){ > + return pool->n_ids; > +} > diff --git a/lib/id-pool.h b/lib/id-pool.h > index 8721f87934bb..b3bfd79386db 100644 > --- a/lib/id-pool.h > +++ b/lib/id-pool.h > @@ -29,7 +29,9 @@ void id_pool_destroy(struct id_pool *); > bool id_pool_alloc_id(struct id_pool *, uint32_t *id); > void id_pool_free_id(struct id_pool *, uint32_t id); > void id_pool_add(struct id_pool *, uint32_t id); > - > +bool id_pool_id_exist(struct id_pool *pool, uint32_t id); > +uint32_t id_pool_base(struct id_pool *pool); > +uint32_t id_pool_n(struct id_pool *pool); > /* > * ID pool. > * ======== > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c > index cdc66246ced3..2bb0eda9d581 100644 > --- a/lib/netdev-linux.c > +++ b/lib/netdev-linux.c > @@ -5823,6 +5823,12 @@ tc_del_policer_action(uint32_t index, struct > ofputil_meter_stats *stats) > > error = tc_transact(&request, &replyp); > if (error) { > + if (error == EPERM || error == ENOENT) { > + /* EPERM means flow exists, it is right that meter deletion is > + * not permited. > + * ENOENT means meter has already been deleted. */ > + return error; > + } > VLOG_ERR_RL(&rl, "Failed to delete police action (index: %u), > err=%d", > index, error); > return error; > diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c > index f6f90a741fde..df0247dc264c 100644 > --- a/lib/netdev-offload-tc.c > +++ b/lib/netdev-offload-tc.c > @@ -2953,10 +2953,19 @@ meter_tc_del_policer(ofproto_meter_id meter_id, > if (!meter_id_lookup(meter_id.uint32, &police_index)) { > err = tc_del_policer_action(police_index, stats); > if (err && err != ENOENT) { > + if (err == EPERM) { > + /* Flow exists, it is right that meter deletion is not > + * permited. */ > + return err; > + } > VLOG_ERR_RL(&error_rl, > - "Failed to del police %u for meter %u: %s", > + "Deletion of police %u for meter %u failed: %s", > police_index, meter_id.uint32, ovs_strerror(err)); > + return err; > } else { > + VLOG_DBG("Deletion of police %u for meter %u succeeded: %s", > + police_index, meter_id.uint32, ovs_strerror(err)); > + err = 0; > meter_free_police_index(police_index); > } > meter_id_remove(meter_id.uint32); > diff --git a/lib/tc.c b/lib/tc.c > index 94044cde6060..c7d539562fc6 100644 > --- a/lib/tc.c > +++ b/lib/tc.c > @@ -51,9 +51,6 @@ > #define TCM_IFINDEX_MAGIC_BLOCK (0xFFFFFFFFU) > #endif > > -#ifndef TCA_DUMP_FLAGS_TERSE > -#define TCA_DUMP_FLAGS_TERSE (1 << 0) > -#endif > > #if TCA_MAX < 15 > #define TCA_CHAIN 11 > @@ -1987,8 +1984,6 @@ tc_parse_action_stats(struct nlattr *action, struct > ovs_flow_stats *stats_sw, > stats_hw, stats_dropped); > } > > -#define TCA_ACT_MIN_PRIO 1 > - > static int > nl_parse_flower_actions(struct nlattr **attrs, struct tc_flower *flower, > bool terse) > diff --git a/lib/tc.h b/lib/tc.h > index 2e64ad372592..8e7107d1bd74 100644 > --- a/lib/tc.h > +++ b/lib/tc.h > @@ -60,6 +60,10 @@ enum tc_qdisc_hook { > > #define METER_POLICE_IDS_BASE 0x10000000 > #define METER_POLICE_IDS_MAX 0x1FFFFFFF > +/* Mapping meter_id.uint32 into a 32-bit integer */ > +#define METER_ID_TO_POLICY_INDEX(meter_id) (meter_id | METER_POLICE_IDS_BASE) > +/* Mapping policy_index to meter_id */ > +#define POLICY_INDEX_TO_METER_ID(index) (index & ~METER_POLICE_IDS_BASE) > > static inline bool > tc_is_meter_index(uint32_t index) { > @@ -300,8 +304,12 @@ enum tc_offloaded_state { > TC_OFFLOADED_STATE_IN_HW, > TC_OFFLOADED_STATE_NOT_IN_HW, > }; > - > +#ifndef TCA_DUMP_FLAGS_TERSE > +#define TCA_DUMP_FLAGS_TERSE (1 << 0) > +#endif > +#define ACT_MAX_NUM 1024 > #define TCA_ACT_MAX_NUM 16 > +#define TCA_ACT_MIN_PRIO 1 > > struct tcf_id { > enum tc_qdisc_hook hook; > diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c > index 7ad728adffdb..09c1c7aedc34 100644 > --- a/ofproto/ofproto-dpif-upcall.c > +++ b/ofproto/ofproto-dpif-upcall.c > @@ -2659,6 +2659,7 @@ revalidate(struct revalidator *revalidator) > struct udpif *udpif = revalidator->udpif; > struct dpif_flow_dump_thread *dump_thread; > uint64_t dump_seq, reval_seq; > + bool flow_delete_exist = false; > bool kill_warn_print = true; > unsigned int flow_limit; > > @@ -2790,6 +2791,7 @@ revalidate(struct revalidator *revalidator) > > if (result != UKEY_KEEP) { > /* Takes ownership of 'recircs'. */ > + flow_delete_exist = true; > reval_op_init(&ops[n_ops++], result, udpif, ukey, &recircs, > &odp_actions); > } > @@ -2803,6 +2805,9 @@ revalidate(struct revalidator *revalidator) > ovsrcu_quiesce(); > } > dpif_flow_dump_thread_destroy(dump_thread); > + if (flow_delete_exist) { > + dpif_meter_revalidate(udpif->dpif, udpif->backer); > + } > ofpbuf_uninit(&odp_actions); > } > > diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h > index d8e0cd37ac5b..ef0c7338e04e 100644 > --- a/ofproto/ofproto-dpif.h > +++ b/ofproto/ofproto-dpif.h > @@ -404,4 +404,6 @@ bool ofproto_dpif_ct_zone_timeout_policy_get_name( > > bool ovs_explicit_drop_action_supported(struct ofproto_dpif *); > > +int dpif_meter_revalidate(struct dpif *dpif, struct dpif_backer *backer); > + > #endif /* ofproto-dpif.h */ > diff --git a/tests/system-offloads-traffic.at > b/tests/system-offloads-traffic.at > index 1a60570801e1..0ca3897a721f 100644 > --- a/tests/system-offloads-traffic.at > +++ b/tests/system-offloads-traffic.at > @@ -273,6 +273,10 @@ meter:1 flow_count:1 packet_in_count:11 > byte_in_count:377 duration:0.001s bands: > 0: packet_count:9 byte_count:0 > ]) > > +AT_CHECK([ovs-ofctl -O Openflow13 del-meter br0]) > +sleep 10 > +AT_CHECK([tc actions ls action police], [0], []) > + > OVS_TRAFFIC_VSWITCHD_STOP > AT_CLEANUP > > -- > 2.30.2 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
