From mboxrd@z Thu Jan  1 00:00:00 1970
From: David Ahern <dsahern@gmail.com>
Subject: Re: [patch iproute2 net-next v11 2/4] tc: introduce
 tc_qdisc_block_exists helper
Date: Fri, 19 Jan 2018 12:45:57 -0800
Message-ID: <e4ee3e40-faf8-0c4c-aa15-392858e4d576@gmail.com>
References: <20180117104657.8605-1-jiri@resnulli.us>
 <20180117104817.8702-2-jiri@resnulli.us>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Cc: davem@davemloft.net, jhs@mojatatu.com, xiyou.wangcong@gmail.com,
        mlxsw@mellanox.com, andrew@lunn.ch,
        vivien.didelot@savoirfairelinux.com, f.fainelli@gmail.com,
        michael.chan@broadcom.com, ganeshgr@chelsio.com,
        saeedm@mellanox.com, matanb@mellanox.com, leonro@mellanox.com,
        idosch@mellanox.com, jakub.kicinski@netronome.com,
        simon.horman@netronome.com, pieter.jansenvanvuuren@netronome.com,
        john.hurley@netronome.com, alexander.h.duyck@intel.com,
        ogerlitz@mellanox.com, john.fastabend@gmail.com,
        daniel@iogearbox.net
To: Jiri Pirko <jiri@resnulli.us>, netdev@vger.kernel.org
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-pg0-f68.google.com ([74.125.83.68]:39253 "EHLO
        mail-pg0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1756284AbeASUqA (ORCPT
        <rfc822;netdev@vger.kernel.org>); Fri, 19 Jan 2018 15:46:00 -0500
Received: by mail-pg0-f68.google.com with SMTP id w17so2283087pgv.6
        for <netdev@vger.kernel.org>; Fri, 19 Jan 2018 12:46:00 -0800 (PST)
In-Reply-To: <20180117104817.8702-2-jiri@resnulli.us>
Content-Language: en-US
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

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
>