[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <69a0bf15-5f52-4974-bbaf-d4ba04e1f983@intel.com>
Date: Mon, 14 Apr 2025 14:28:31 +0200
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Edward Cree <ecree.xilinx@...il.com>, "Nelson, Shannon"
<shannon.nelson@....com>, Jakub Kicinski <kuba@...nel.org>, "Jagielski,
Jedrzej" <jedrzej.jagielski@...el.com>
CC: "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>, "davem@...emloft.net"
<davem@...emloft.net>, "pabeni@...hat.com" <pabeni@...hat.com>, "Dumazet,
Eric" <edumazet@...gle.com>, "andrew+netdev@...n.ch" <andrew+netdev@...n.ch>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>, "jiri@...nulli.us"
<jiri@...nulli.us>, "horms@...nel.org" <horms@...nel.org>, "corbet@....net"
<corbet@....net>, "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
Kalesh AP <kalesh-anakkur.purayil@...adcom.com>, "R, Bharath"
<bharath.r@...el.com>
Subject: Re: [PATCH net-next 01/15] devlink: add value check to
devlink_info_version_put()
On 4/11/25 13:11, Edward Cree wrote:
> On 09/04/2025 18:25, Nelson, Shannon wrote:
>> On 4/9/2025 7:39 AM, Jakub Kicinski wrote:
>>>
>>> On Wed, 9 Apr 2025 14:14:23 +0000 Jagielski, Jedrzej wrote:
>>>> No insisting on that but should empty entry be really presented to the user?
>>>> Especially unintentionally? Actually it's exposing some driver's shortcomings.
>>>> That means the output was not properly validated so imho there's no point in
>>>> printing it.
>>>
>>> +1, FWIW, I don't see the point of outputting keys without values.
>>
>> Because I like to see hints that something might be wrong, rather than hiding them.
>
> +1 to this. Failures should be noisy. Time you care most about these
> data is when something *is* wrong and you're trying to debug it.
That's true. But empty string in some info/debug utility is not noisy
at all. At best it will be part of the report attached (and with that
developers will be equally equipped to tell that "there is no foobar"
entry as "the foobar is empty").
If anyone wants to be noisy here, there is WARN_ON() or we could even
pass NULL ;)
> AFAICT the argument on the other side is "it makes the driver look bad",
> which has (expletive)-all to do with engineering.
> Value often comes from firmware, anyway, in which case driver's (& core's)
> job is to be a dumb pipe, not go around 'validating' things.
that way we will stick with the ugly, repetitive, overly bloated code,
repetitive and hard to fix in all places, (and repetitive) drivers
yeah, good that we bikeshed on something so simple :)
If anyone is "strongly opposed", please say so once more, and we will
drop this patch. Otherwise we are going to keep it.
Powered by blists - more mailing lists