Re: [patch iproute2 net-next v11 2/4] tc: introduce tc_qdisc_block_exists helper

[David Ahern <dsahern@gmail.com>]

On 1/17/18 2:48 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> This hepler used qdisc dump to list all qdisc and find if block index in
> question is used by any of them. That means the block with specified
> index exists.
> 
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
>  tc/tc_qdisc.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tc/tc_util.h  |  2 ++
>  2 files changed, 63 insertions(+)
> 
> diff --git a/tc/tc_qdisc.c b/tc/tc_qdisc.c
> index 70279b9..3f91558 100644
> --- a/tc/tc_qdisc.c
> +++ b/tc/tc_qdisc.c
> @@ -412,3 +412,64 @@ int do_qdisc(int argc, char **argv)
>  	fprintf(stderr, "Command \"%s\" is unknown, try \"tc qdisc help\".\n", *argv);
>  	return -1;
>  }
> +
> +struct tc_qdisc_block_exists_ctx {
> +	__u32 block_index;
> +	bool found;
> +};
> +
> +static int tc_qdisc_block_exists_cb(const struct sockaddr_nl *who,
> +				    struct nlmsghdr *n, void *arg)
> +{
> +	struct tc_qdisc_block_exists_ctx *ctx = arg;
> +	struct tcmsg *t = NLMSG_DATA(n);
> +	int len = n->nlmsg_len;
> +	struct rtattr *tb[TCA_MAX+1];

reverse xmas tree

> +
> +	if (n->nlmsg_type != RTM_NEWQDISC)
> +		return 0;
> +
> +	len -= NLMSG_LENGTH(sizeof(*t));
> +	if (len < 0)
> +		return -1;
> +
> +	parse_rtattr(tb, TCA_MAX, TCA_RTA(t), len);
> +
> +	if (tb[TCA_KIND] == NULL)
> +		return -1;
> +
> +	if (tb[TCA_INGRESS_BLOCK] &&
> +	    RTA_PAYLOAD(tb[TCA_INGRESS_BLOCK]) >= sizeof(__u32)) {

why not just == sizeof(__u32) since that's what it should be?

> +		__u32 block = rta_getattr_u32(tb[TCA_INGRESS_BLOCK]);
> +
> +		if (block && block == ctx->block_index)

block > 0 test should not be needed. If someone is calling
tc_qdisc_block_exists, then block_index should be > 0 in which case you
just need block == ctx->block_index


> +			ctx->found = true;
> +	}
> +
> +	if (tb[TCA_EGRESS_BLOCK] &&
> +	    RTA_PAYLOAD(tb[TCA_EGRESS_BLOCK]) >= sizeof(__u32)) {
> +		__u32 block = rta_getattr_u32(tb[TCA_EGRESS_BLOCK]);
> +
> +		if (block && block == ctx->block_index)

same 2 comments for this block

> +			ctx->found = true;
> +	}
> +	return 0;
> +}
> +
> +int tc_qdisc_block_exists(__u32 block_index)

This should be bool since you are returning true / false

> +{
> +	struct tc_qdisc_block_exists_ctx ctx = { .block_index = block_index };
> +	struct tcmsg t = { .tcm_family = AF_UNSPEC };
> +
> +	if (rtnl_dump_request(&rth, RTM_GETQDISC, &t, sizeof(t)) < 0) {
> +		perror("Cannot send dump request");
> +		return false;
> +	}
> +
> +	if (rtnl_dump_filter(&rth, tc_qdisc_block_exists_cb, &ctx) < 0) {
> +		perror("Dump terminated\n");
> +		return false;
> +	}
> +
> +	return ctx.found;
> +}
> diff --git a/tc/tc_util.h b/tc/tc_util.h
> index 1218610..72f4282 100644
> --- a/tc/tc_util.h
> +++ b/tc/tc_util.h
> @@ -132,4 +132,6 @@ int prio_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt);
>  int cls_names_init(char *path);
>  void cls_names_uninit(void);
>  
> +int tc_qdisc_block_exists(__u32 block_index);
> +
>  #endif
>