netdev - Re: [PATCH v9 03/25] net/ethtool: add ULP_DDP_{GET,SET} operations for caps and stats

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: <253o7qprtcq.fsf@mtr-vdi-124.i-did-not-set--mail-host-address--so-tickle-me>
Date:   Mon, 23 Jan 2023 20:36:21 +0200
From:   Aurelien Aptel <aaptel@...dia.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     linux-nvme@...ts.infradead.org, netdev@...r.kernel.org,
        sagi@...mberg.me, hch@....de, kbusch@...nel.org, axboe@...com,
        chaitanyak@...dia.com, davem@...emloft.net,
        aurelien.aptel@...il.com, smalin@...dia.com, malin1024@...il.com,
        ogerlitz@...dia.com, yorayz@...dia.com, borisp@...dia.com
Subject: Re: [PATCH v9 03/25] net/ethtool: add ULP_DDP_{GET,SET} operations
 for caps and stats

Jakub Kicinski <kuba@...nel.org> writes:
>> If a ULP_DDP_GET message has requested statistics via the
>> ETHTOOL_FLAG_STATS header flag, then per-device statistics are
>
> s/per-device// ?

Will be fixed.

>> Compact statistics are nested as follows:
>>
>>     STATS (nest)
>>         COUNT (u32)
>>         COMPACT_VALUES (array of u64)
>
> That's not how other per-cmd stats work, why are you inventing
> new ways..

As we commented in patch 2, dynamic strings are used for ethtool
forward-compability (being able to list future stats, which we are
planning) without updating or recompiling.

>> +     int     (*get_ulp_ddp_stats)(struct net_device *dev, struct ethtool_ulp_ddp_stats *stats);
>> +     int     (*set_ulp_ddp_capabilities)(struct net_device *dev, unsigned long *bits);
>
> Why are these two callbacks not in struct ulp_ddp_dev_ops?

We were trying to implement these callbacks in alignment with the
existing ethtool commands, for this reason we implemented it in the
ethtool API.

> Why does the ethtool API not expose limits?

Originally, and before we started adding the netlink interface, we were
not planning to include the ability to modify the limits as part of this
series.  We do agree that it now makes sense, but we will add, some
limits reflect hardware limitations while other could be tweaked by
users.  Those limits will be per-device and per-protocol. We will
suggest how to design it.

Thanks