From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jiri Pirko Subject: Re: [patch iproute2 net-next v11 2/4] tc: introduce tc_qdisc_block_exists helper Date: Sat, 20 Jan 2018 10:33:42 +0100 Message-ID: <20180120093342.GD2147@nanopsycho.orion> References: <20180117104657.8605-1-jiri@resnulli.us> <20180117104817.8702-2-jiri@resnulli.us> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: netdev@vger.kernel.org, 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: David Ahern Return-path: Received: from mail-wm0-f67.google.com ([74.125.82.67]:39612 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751055AbeATJdp (ORCPT ); Sat, 20 Jan 2018 04:33:45 -0500 Received: by mail-wm0-f67.google.com with SMTP id b21so7843437wme.4 for ; Sat, 20 Jan 2018 01:33:44 -0800 (PST) Content-Disposition: inline In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: Fri, Jan 19, 2018 at 09:45:57PM CET, dsahern@gmail.com wrote: >On 1/17/18 2:48 AM, Jiri Pirko wrote: >> From: Jiri Pirko >> >> 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 >> --- >> 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 :) Will fix > >> + >> + 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? test1:~/iproute2$ git grep "== sizeof(__u32)" | grep RTA_PAYLOAD| wc -l 1 test1:~/iproute2$ git grep ">= sizeof(__u32)" | grep RTA_PAYLOAD| wc -l 42 I just go with the flow :) > >> + __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 Right. Will Fix. > > >> + 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 ack > >> + ctx->found = true; >> + } >> + return 0; >> +} >> + >> +int tc_qdisc_block_exists(__u32 block_index) > >This should be bool since you are returning true / false Ah, missed that. Fixing. > >> +{ >> + 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 >> >