On Mon, 2022-06-20 at 12:17 +0200, Eelco Chaudron wrote:
> On 27 May 2022, at 11:00, Jianbo Liu wrote:
>
> > When offloading rule, tc should be filled with police index,
> > instead
> > of meter id. As meter is mapped to police action, and the mapping
> > is
> > inserted into meter_id_to_police_idx hmap, this hmap is used to
> > find
> > the police index. Besides, the reverse mapping between meter id and
> > police index is also added, so meter id can be retrieved from this
> > hashmap and pass to dpif while dumping rules.
> >
> > Signed-off-by: Jianbo Liu <[email protected]>
>
> Just one comment below.
>
> > ---
> > lib/netdev-offload-tc.c | 45
> > ++++++++++++++++++++++++++++++++++++++++-
> > lib/tc.c | 20 +++++++++++++++++-
> > 2 files changed, 63 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> > index 7e5f04bdf..43d13cc56 100644
> > --- a/lib/netdev-offload-tc.c
> > +++ b/lib/netdev-offload-tc.c
> > @@ -65,6 +65,7 @@ struct chain_node {
> >
> > struct meter_police_mapping_data {
> > struct hmap_node meter_id_node;
> > + struct hmap_node police_idx_node;
> > uint32_t meter_id;
> > uint32_t police_idx;
> > };
> > @@ -83,6 +84,11 @@ static struct id_pool *meter_police_ids
> > OVS_GUARDED_BY(meter_police_ids_mutex);
> > static struct ovs_mutex meter_mutex = OVS_MUTEX_INITIALIZER;
> > static struct hmap meter_id_to_police_idx
> > OVS_GUARDED_BY(meter_mutex)
> > = HMAP_INITIALIZER(&meter_id_to_police_idx);
> > +static struct hmap police_idx_to_meter_id
> > OVS_GUARDED_BY(meter_mutex)
> > + = HMAP_INITIALIZER(&police_idx_to_meter_id);
> > +
> > +static int meter_id_lookup(uint32_t meter_id, uint32_t
> > *police_idx);
> > +static int police_idx_lookup(uint32_t police_idx, uint32_t
> > *meter_id);
> >
> > static bool
> > is_internal_port(const char *type)
> > @@ -1040,7 +1046,12 @@ parse_tc_flower_to_match(struct tc_flower
> > *flower,
> > }
> > break;
> > case TC_ACT_POLICE: {
> > - /* Not supported yet */
> > + uint32_t meter_id;
> > +
> > + if (police_idx_lookup(action->police.index,
> > &meter_id)) {
> > + return ENOENT;
> > + }
> > + nl_msg_put_u32(buf, OVS_ACTION_ATTR_METER,
> > meter_id);
> > }
> > break;
> > }
> > @@ -1963,6 +1974,16 @@ netdev_tc_flow_put(struct netdev *netdev,
> > struct match *match,
> > action->type = TC_ACT_GOTO;
> > action->chain = 0; /* 0 is reserved and not used by
> > recirc. */
> > flower.action_count++;
> > + } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_METER) {
> > + uint32_t police_index, meter_id;
> > +
> > + meter_id = nl_attr_get_u32(nla);
> > + if (meter_id_lookup(meter_id, &police_index)) {
> > + return EOPNOTSUPP;
> > + }
> > + action->type = TC_ACT_POLICE;
> > + action->police.index = police_index;
> > + flower.action_count++;
> > } else {
> > VLOG_DBG_RL(&rl, "unsupported put action type: %d",
> > nl_attr_type(nla));
> > @@ -2432,6 +2453,25 @@ meter_id_lookup(uint32_t meter_id, uint32_t
> > *police_idx)
> > return data ? 0 : ENOENT;
> > }
> >
> > +static int
> > +police_idx_lookup(uint32_t police_idx, uint32_t *meter_id)
> > +{
> > + struct meter_police_mapping_data *data = NULL;
> > + size_t hash = hash_int(police_idx, 0);
> > +
> > + ovs_mutex_lock(&meter_mutex);
> > + HMAP_FOR_EACH_WITH_HASH (data, police_idx_node, hash,
> > + &police_idx_to_meter_id) {
> > + if (data->police_idx == police_idx) {
> > + *meter_id = data->meter_id;
> > + break;
> > + }
> > + }
> > + ovs_mutex_unlock(&meter_mutex);
> > +
> > + return data ? 0 : ENOENT;
>
> 'data' can be none NULL and still no valid entry was found.
>
Right, there is an error here, I will fix. Thanks.
> > +}
> > +
> > static void
> > meter_id_insert(uint32_t meter_id, uint32_t police_idx)
> > {
> > @@ -2443,6 +2483,8 @@ meter_id_insert(uint32_t meter_id, uint32_t
> > police_idx)
> > data->police_idx = police_idx;
> > hmap_insert(&meter_id_to_police_idx, &data->meter_id_node,
> > hash_int(meter_id, 0));
> > + hmap_insert(&police_idx_to_meter_id, &data->police_idx_node,
> > + hash_int(police_idx, 0));
> > ovs_mutex_unlock(&meter_mutex);
> > }
> >
> > @@ -2455,6 +2497,7 @@ meter_id_remove(uint32_t meter_id)
> > data = meter_id_find_locked(meter_id);
> > if (data) {
> > hmap_remove(&meter_id_to_police_idx, &data-
> > >meter_id_node);
> > + hmap_remove(&police_idx_to_meter_id, &data-
> > >police_idx_node);
> > free(data);
> > }
> > ovs_mutex_unlock(&meter_mutex);
> > diff --git a/lib/tc.c b/lib/tc.c
> > index ac53c56db..8db18fa2c 100644
> > --- a/lib/tc.c
> > +++ b/lib/tc.c
> > @@ -2440,6 +2440,22 @@ nl_msg_put_act_gact(struct ofpbuf *request,
> > uint32_t chain)
> > nl_msg_end_nested(request, offset);
> > }
> >
> > +static void
> > +nl_msg_put_act_police_index(struct ofpbuf *request, uint32_t
> > police_idx)
> > +{
> > + struct tc_police police;
> > + size_t offset;
> > +
> > + memset(&police, 0, sizeof police);
> > + police.index = police_idx;
> > +
> > + nl_msg_put_string(request, TCA_ACT_KIND, "police");
> > + offset = nl_msg_start_nested(request, TCA_ACT_OPTIONS);
> > + nl_msg_put_unspec(request, TCA_POLICE_TBF, &police, sizeof
> > police);
> > + nl_msg_put_u32(request, TCA_POLICE_RESULT, TC_ACT_PIPE);
> > + nl_msg_end_nested(request, offset);
> > +}
> > +
> > static void
> > nl_msg_put_act_ct(struct ofpbuf *request, struct tc_action
> > *action)
> > {
> > @@ -2930,7 +2946,9 @@ nl_msg_put_flower_acts(struct ofpbuf
> > *request, struct tc_flower *flower)
> > }
> > break;
> > case TC_ACT_POLICE: {
> > - /* Not supported yet */
> > + act_offset = nl_msg_start_nested(request,
> > act_index++);
> > + nl_msg_put_act_police_index(request, action-
> > >police.index);
> > + nl_msg_end_nested(request, act_offset);
> > }
> > break;
> > }
> > --
> > 2.26.2
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
