On 3 May 2022, at 5:08, Jianbo Liu via dev wrote:
> Add function to parse police action from netlink message, and meter id
> can be retrieved from action cockie as it will be saved there in later
> patch.
See comments inline below…
> Signed-off-by: Jianbo Liu <jian...@nvidia.com>
> ---
> lib/netdev-offload-tc.c | 4 +++
> lib/tc.c | 59 +++++++++++++++++++++++++++++++++++++++++
> lib/tc.h | 6 +++++
> 3 files changed, 69 insertions(+)
>
> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> index a41b62758..83d57c63b 100644
> --- a/lib/netdev-offload-tc.c
> +++ b/lib/netdev-offload-tc.c
> @@ -1017,6 +1017,10 @@ parse_tc_flower_to_match(struct tc_flower *flower,
> nl_msg_put_u32(buf, OVS_ACTION_ATTR_RECIRC, action->chain);
> }
> break;
> + case TC_ACT_POLICE: {
> + /* Not supported yet */
> + }
> + break;
> }
> }
> }
> diff --git a/lib/tc.c b/lib/tc.c
> index df73a43d4..af7a7bc6d 100644
> --- a/lib/tc.c
> +++ b/lib/tc.c
> @@ -1339,6 +1339,59 @@ nl_parse_act_gact(struct nlattr *options, struct
> tc_flower *flower)
> return 0;
> }
>
> +static const struct nl_policy police_policy[] = {
> + [TCA_POLICE_TBF] = { .type = NL_A_UNSPEC,
> + .min_len = sizeof(struct tc_police),
> + .optional = false, },
> + [TCA_POLICE_RATE] = { .type = NL_A_UNSPEC,
> + .min_len = 1024,
> + .optional = true, },
> + [TCA_POLICE_PEAKRATE] = { .type = NL_A_UNSPEC,
> + .min_len = 1024,
> + .optional = true, },
> + [TCA_POLICE_AVRATE] = { .type = NL_A_U32,
> + .optional = true, },
> + [TCA_POLICE_RESULT] = { .type = NL_A_U32,
> + .optional = true, },
> + [TCA_POLICE_TM] = { .type = NL_A_UNSPEC,
> + .min_len = sizeof(struct tcf_t),
> + .optional = true, },
> +};
> +
> +static int
> +nl_parse_act_police(const struct nlattr *options, struct tc_flower *flower,
> + struct nlattr *act_cookie)
> +{
> + struct nlattr *police_attrs[ARRAY_SIZE(police_policy)] = {};
> + struct tc_action *action;
> + const struct tc_police *police;
For the new code, I review I try to stick to reverse Christmas tree style, so
as you might need to make changes may also swap the two-line?
> + struct nlattr *police_tm;
> + const struct tcf_t *tm;
> +
> + if (!nl_parse_nested(options, police_policy, police_attrs,
> + ARRAY_SIZE(police_policy))) {
> + VLOG_ERR_RL(&error_rl, "failed to parse police action options");
Change failed to a capital, so “Failed to parse..."
> + return EPROTO;
> + }
> +
> + police = nl_attr_get(police_attars[TCA_POLICE_TBF]);
police = nl_attr_get_unspec(police_attrs[TCA_POLICE_TBF], sizeof *police);
> + action = &flower->actions[flower->action_count++];
> + action->type = TC_ACT_POLICE;
> + action->police.index = police->index;
> +
> + if (act_cookie) {
> + action->police.meter_id = nl_attr_get_u32(act_cookie);
> + }
> +
> + police_tm = police_attrs[TCA_POLICE_TM];
If you use optional attributes, you have to make sure they are present. Guess
the fix would be to set .optional = false for TCA_POLICE_TM.
> + if (police_tm) {
> + tm = nl_attr_get_unspec(police_tm, sizeof *tm);
> + nl_parse_tcf(tm, flower);
> + }
> +
> + return 0;
> +}
> +
> static const struct nl_policy mirred_policy[] = {
> [TCA_MIRRED_PARMS] = { .type = NL_A_UNSPEC,
> .min_len = sizeof(struct tc_mirred),
> @@ -1761,6 +1814,8 @@ nl_parse_single_action(struct nlattr *action, struct
> tc_flower *flower,
> /* Added for TC rule only (not in OvS rule) so ignore. */
> } else if (!strcmp(act_kind, "ct")) {
> nl_parse_act_ct(act_options, flower);
> + } else if (!strcmp(act_kind, "police")) {
> + nl_parse_act_police(act_options, flower, act_cookie);
> } else {
> VLOG_ERR_RL(&error_rl, "unknown tc action kind: %s", act_kind);
> err = EINVAL;
> @@ -2773,6 +2828,10 @@ nl_msg_put_flower_acts(struct ofpbuf *request, struct
> tc_flower *flower)
> nl_msg_end_nested(request, act_offset);
> }
> break;
> + case TC_ACT_POLICE: {
> + /* Not supported yet */
> + }
> + break;
> }
> }
> }
> diff --git a/lib/tc.h b/lib/tc.h
> index d6cdddd16..201345672 100644
> --- a/lib/tc.h
> +++ b/lib/tc.h
> @@ -174,6 +174,7 @@ enum tc_action_type {
> TC_ACT_MPLS_SET,
> TC_ACT_GOTO,
> TC_ACT_CT,
> + TC_ACT_POLICE,
> };
>
> enum nat_type {
> @@ -261,6 +262,11 @@ struct tc_action {
> struct tc_flower_key key;
> struct tc_flower_key mask;
> } rewrite;
> +
> + struct {
> + uint32_t index;
> + uint32_t meter_id;
> + } police;
> };
>
> enum tc_action_type type;
> --
> 2.26.2
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
