netdev - Re: [patch net-next v4 01/10] flow_offload: Introduce offload of HW stats type

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: <75b7e941-9a94-9939-f212-03aaed856088@solarflare.com>
Date:   Mon, 9 Mar 2020 22:27:59 +0000
From:   Edward Cree <ecree@...arflare.com>
To:     Jakub Kicinski <kuba@...nel.org>
CC:     Jiri Pirko <jiri@...nulli.us>, <netdev@...r.kernel.org>,
        <davem@...emloft.net>, <saeedm@...lanox.com>, <leon@...nel.org>,
        <michael.chan@...adcom.com>, <vishal@...lsio.com>,
        <jeffrey.t.kirsher@...el.com>, <idosch@...lanox.com>,
        <aelior@...vell.com>, <peppe.cavallaro@...com>,
        <alexandre.torgue@...com>, <jhs@...atatu.com>,
        <xiyou.wangcong@...il.com>, <pablo@...filter.org>,
        <mlxsw@...lanox.com>
Subject: Re: [patch net-next v4 01/10] flow_offload: Introduce offload of HW
 stats type

On 09/03/2020 21:36, Jakub Kicinski wrote:
> ...but using the type for fields which are very likely to contain
> values from outside of the enumeration seems confusing, IMHO.
It's the only way I can see to (a) ensure that the field is the correct
 size and (b) document the connection between the field and the type.

Now, I'm not saying that using the enums actually buys any real type-
 safety — the C standard enums are far too weak for that, being
 basically ints with hair — but the mythical Sufficiently Smart
 Compiler might be able to get some mileage out of it to issue better
 diagnostics.

> Driver author can understandably try to simply handle all the values 
> in a switch statement and be unpleasantly surprised.
In my experience, unenumerated enum values of this kind are fully
 idiomatic C; and a driver author looking at the header file to see
 what enumeration constants are defined will necessarily see all the
 calls to BIT() and the bitwise-or construction of _ANY.
I'm not sure I believe a naïve switch() implementation is an
 "understandable" error.

How about if we also rename the field "hw_stats_types", or squeeze
 the word "mask" in there somewhere?

-ed