netdev - Re: [PATCH net-next v2 01/12] ethtool: add infrastructure for centralized checking of coalescing parameters

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] [day] [month] [year] [list]

Message-ID: <20200304203409.23987c58@kicinski-fedora-PC1C0HJN>
Date:   Wed, 4 Mar 2020 20:34:09 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Alexander Duyck <alexander.h.duyck@...ux.intel.com>
Cc:     Michal Kubecek <mkubecek@...e.cz>, davem@...emloft.net,
        thomas.lendacky@....com, benve@...co.com, _govind@....com,
        pkaustub@...co.com, peppe.cavallaro@...com,
        alexandre.torgue@...com, joabreu@...opsys.com, snelson@...sando.io,
        yisen.zhuang@...wei.com, salil.mehta@...wei.com,
        jeffrey.t.kirsher@...el.com, jacob.e.keller@...el.com,
        michael.chan@...adcom.com, saeedm@...lanox.com, leon@...nel.org,
        netdev@...r.kernel.org
Subject: Re: [PATCH net-next v2 01/12] ethtool: add infrastructure for
 centralized checking of coalescing parameters

On Wed, 04 Mar 2020 10:50:57 -0800 Alexander Duyck wrote:
> I just figured that by making in an enum it becomes less error prone since
> you can't accidentally leave a gap or end up renumbering things
> unintentionally.

True. That said the API will remain frozen for a little bit
longer - until the netlink conversion of this op.

> Combine that with some logic to take care of the bit
> shifting and it wouldn't differ much from how we handle the netdev feature
> flags and the like.

To be honest it was netdev features that made me dislike the model
slightly :)

Drivers sometimes print features as a hex u64 and trying to decode
that with the automatically assigned bits takes extra effort :S

With straight up defines I can do:

for i in `seq 0 32`; do 
	[ $((x & (1<<i))) -ne 0 ] && git grep "ETHTOOL_COALESCE_.*BIT($i)"
done

where x is set to whatever the driver printed. E.g.:

$ x=0xc37c0; for i in `seq 0 22`; do [ $((x & (1<<i))) -ne 0 ] && git grep  "ETHTOOL_COALESCE_.*BIT($i)"; done
include/linux/ethtool.h:#define ETHTOOL_COALESCE_TX_USECS_IRQ           BIT(6)
include/linux/ethtool.h:#define ETHTOOL_COALESCE_TX_MAX_FRAMES_IRQ      BIT(7)
include/linux/ethtool.h:#define ETHTOOL_COALESCE_STATS_BLOCK_USECS      BIT(8)
include/linux/ethtool.h:#define ETHTOOL_COALESCE_USE_ADAPTIVE_RX        BIT(9)
include/linux/ethtool.h:#define ETHTOOL_COALESCE_USE_ADAPTIVE_TX        BIT(10)
include/linux/ethtool.h:#define ETHTOOL_COALESCE_RX_USECS_LOW           BIT(12)
include/linux/ethtool.h:#define ETHTOOL_COALESCE_RX_MAX_FRAMES_LOW      BIT(13)
include/linux/ethtool.h:#define ETHTOOL_COALESCE_RX_MAX_FRAMES_HIGH     BIT(18)
include/linux/ethtool.h:#define ETHTOOL_COALESCE_TX_USECS_HIGH          BIT(19)

With enum there are no explicit numbers so nothing to grep for.
I could probably squeeze this information out of debug info, but 
debug info incantations don't stick in my memory.

IDK if that's a sane or valid reason, LMK if you feel strongly and
I'll convert. We'll probably revisit this anyway when netlink comes
with the attribute presence checking.