netdev - Re: [PATCH net-next 1/4] netdev: support dumping a single netdev in qstats

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANn89iK-wnNeH+9-Oe6xi9OjoY5jcZCowJ5wDL7hJz1tRhMfQQ@mail.gmail.com>
Date: Sun, 21 Apr 2024 21:17:15 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net, netdev@...r.kernel.org, pabeni@...hat.com, 
	shuah@...nel.org, sdf@...gle.com, amritha.nambiar@...el.com, 
	linux-kselftest@...r.kernel.org, dsahern@...il.com
Subject: Re: [PATCH net-next 1/4] netdev: support dumping a single netdev in qstats

On Sat, Apr 20, 2024 at 4:35 AM Jakub Kicinski <kuba@...nel.org> wrote:
>
> Having to filter the right ifindex in the tests is a bit tedious.
> Add support for dumping qstats for a single ifindex.
>
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
>  Documentation/netlink/specs/netdev.yaml |  1 +
>  net/core/netdev-genl-gen.c              |  1 +
>  net/core/netdev-genl.c                  | 52 ++++++++++++++++++-------
>  3 files changed, 41 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml
> index 76352dbd2be4..679c4130707c 100644
> --- a/Documentation/netlink/specs/netdev.yaml
> +++ b/Documentation/netlink/specs/netdev.yaml
> @@ -486,6 +486,7 @@ name: netdev
>        dump:
>          request:
>            attributes:
> +            - ifindex
>              - scope
>          reply:
>            attributes:
> diff --git a/net/core/netdev-genl-gen.c b/net/core/netdev-genl-gen.c
> index 8d8ace9ef87f..8350a0afa9ec 100644
> --- a/net/core/netdev-genl-gen.c
> +++ b/net/core/netdev-genl-gen.c
> @@ -70,6 +70,7 @@ static const struct nla_policy netdev_napi_get_dump_nl_policy[NETDEV_A_NAPI_IFIN
>
>  /* NETDEV_CMD_QSTATS_GET - dump */
>  static const struct nla_policy netdev_qstats_get_nl_policy[NETDEV_A_QSTATS_SCOPE + 1] = {
> +       [NETDEV_A_QSTATS_IFINDEX] = NLA_POLICY_MIN(NLA_U32, 1),
>         [NETDEV_A_QSTATS_SCOPE] = NLA_POLICY_MASK(NLA_UINT, 0x1),
>  };
>
> diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> index 7004b3399c2b..dd6510f2c652 100644
> --- a/net/core/netdev-genl.c
> +++ b/net/core/netdev-genl.c
> @@ -639,6 +639,24 @@ netdev_nl_stats_by_netdev(struct net_device *netdev, struct sk_buff *rsp,
>         return -EMSGSIZE;
>  }
>
> +static int
> +netdev_nl_qstats_get_dump_one(struct net_device *netdev, unsigned int scope,
> +                             struct sk_buff *skb, const struct genl_info *info,
> +                             struct netdev_nl_dump_ctx *ctx)
> +{
> +       if (!netdev->stat_ops)
> +               return 0;
> +
> +       switch (scope) {
> +       case 0:
> +               return netdev_nl_stats_by_netdev(netdev, skb, info);
> +       case NETDEV_QSTATS_SCOPE_QUEUE:
> +               return netdev_nl_stats_by_queue(netdev, skb, info, ctx);
> +       }
> +
> +       return -EINVAL; /* Should not happen, per netlink policy */
> +}
> +
>  int netdev_nl_qstats_get_dumpit(struct sk_buff *skb,
>                                 struct netlink_callback *cb)
>  {
> @@ -646,6 +664,7 @@ int netdev_nl_qstats_get_dumpit(struct sk_buff *skb,
>         const struct genl_info *info = genl_info_dump(cb);
>         struct net *net = sock_net(skb->sk);
>         struct net_device *netdev;
> +       unsigned int ifindex;
>         unsigned int scope;
>         int err = 0;
>
> @@ -653,21 +672,28 @@ int netdev_nl_qstats_get_dumpit(struct sk_buff *skb,
>         if (info->attrs[NETDEV_A_QSTATS_SCOPE])
>                 scope = nla_get_uint(info->attrs[NETDEV_A_QSTATS_SCOPE]);
>
> -       rtnl_lock();
> -       for_each_netdev_dump(net, netdev, ctx->ifindex) {
> -               if (!netdev->stat_ops)
> -                       continue;
> +       ifindex = 0;
> +       if (info->attrs[NETDEV_A_QSTATS_IFINDEX])
> +               ifindex = nla_get_u32(info->attrs[NETDEV_A_QSTATS_IFINDEX]);
>
> -               switch (scope) {
> -               case 0:
> -                       err = netdev_nl_stats_by_netdev(netdev, skb, info);
> -                       break;
> -               case NETDEV_QSTATS_SCOPE_QUEUE:
> -                       err = netdev_nl_stats_by_queue(netdev, skb, info, ctx);
> -                       break;
> +       rtnl_lock();
> +       if (ifindex) {
> +               netdev = __dev_get_by_index(net, ifindex);

I wonder if NLM_F_DUMP_FILTERED should not be reported to user space ?