On 2 Jul 2022, at 5:18, Jianbo Liu wrote:
> Add helpers to add, delete and get stats of police action with
> the specified index.
>
> Signed-off-by: Jianbo Liu <jian...@nvidia.com>
Two some small comments below...
> ---
> lib/netdev-linux.c | 144 +++++++++++++++++++++++++++++++++++++++++++++
> lib/netdev-linux.h | 6 ++
> lib/tc.c | 118 +++++++++++++++++++++++++------------
> lib/tc.h | 7 +++
> 4 files changed, 239 insertions(+), 36 deletions(-)
>
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index 039a99f49..24f0bceb8 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -5667,6 +5667,150 @@ tc_add_policer(struct netdev *netdev, uint32_t
> kbits_rate,
> return 0;
> }
>
> +int
> +tc_add_policer_action(uint32_t index, uint32_t kbits_rate,
> + uint32_t kbits_burst, uint32_t pkts_rate,
> + uint32_t pkts_burst, bool update)
> +{
> + struct tc_police tc_police;
> + struct ofpbuf request;
> + struct tcamsg *tcamsg;
> + size_t offset;
> + int flags;
> + int error;
> +
> + tc_policer_init(&tc_police, kbits_rate, kbits_burst);
> + tc_police.index = index;
> +
> + flags = (update ? NLM_F_REPLACE : NLM_F_EXCL) | NLM_F_CREATE;
> + tcamsg = tc_make_action_request(RTM_NEWACTION, flags, &request);
> + if (!tcamsg) {
> + return ENODEV;
> + }
> +
> + offset = nl_msg_start_nested(&request, TCA_ACT_TAB);
> + nl_msg_put_act_police(&request, &tc_police, pkts_rate, pkts_burst);
> + nl_msg_end_nested(&request, offset);
> +
> + error = tc_transact(&request, NULL);
> + if (error) {
> + VLOG_ERR_RL(&rl, "Failed to %s police action, err=%d",
> + update ? "update" : "add", error);
> + }
> +
> + return error;
> +}
> +
> +static int
> +tc_update_policer_action_stats(struct ofpbuf *msg,
> + struct ofputil_meter_stats *stats)
> +{
> + struct ovs_flow_stats stats_dropped = {0};
> + struct ovs_flow_stats stats_hw = {0};
> + struct ovs_flow_stats stats_sw = {0};
Here you want to keep the initializer as {}, as setting it to {0} gets some
compilers to only initialize the first element. See also the robot complaining
about this.
> + const struct nlattr *act = NULL;
> + struct nlattr *prio;
> + struct tcamsg *tca;
> + int error;
> +
> + if (!stats) {
> + return 0;
> + }
> +
> + if (NLMSG_HDRLEN + sizeof *tca > msg->size) {
> + VLOG_ERR_RL(&rl, "Failed to get action stats, size error");
> + return EPROTO;
> + }
> +
> + tca = ofpbuf_at_assert(msg, NLMSG_HDRLEN, sizeof *tca);
> + act = nl_attr_find(msg, NLMSG_HDRLEN + sizeof *tca, TCA_ACT_TAB);
> + if (!act) {
> + VLOG_ERR_RL(&rl, "Failed to get action stats, can't find attr");
A nit I missed before, but as you need to send an update rev anyway, please
change attr to attribute in the error message, which is more user-friendly.
> + return EPROTO;
> + }
> +
<SNIP>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
