| 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
| ||
|
Message-ID: <eaa98c4b-392a-4070-8670-7d29558da20b@intel.com>
Date: Mon, 3 Nov 2025 12:24:02 +0100
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: "Loktionov, Aleksandr" <aleksandr.loktionov@...el.com>, ALOK TIWARI
<alok.a.tiwari@...cle.com>
CC: "alok.a.tiwarilinux@...il.com" <alok.a.tiwarilinux@...il.com>, "Lobakin,
Aleksander" <aleksander.lobakin@...el.com>, "Nguyen, Anthony L"
<anthony.l.nguyen@...el.com>, "andrew+netdev@...n.ch"
<andrew+netdev@...n.ch>, "kuba@...nel.org" <kuba@...nel.org>,
"davem@...emloft.net" <davem@...emloft.net>, "edumazet@...gle.com"
<edumazet@...gle.com>, "pabeni@...hat.com" <pabeni@...hat.com>,
"horms@...nel.org" <horms@...nel.org>, "intel-wired-lan@...ts.osuosl.org"
<intel-wired-lan@...ts.osuosl.org>, "netdev@...r.kernel.org"
<netdev@...r.kernel.org>
Subject: Re: [Intel-wired-lan] [External] : Re: [PATCH net-next] iavf: clarify
VLAN add/delete log messages and lower log level
On 11/3/25 11:30, Loktionov, Aleksandr wrote:
>
>
>> -----Original Message-----
>> From: Intel-wired-lan <intel-wired-lan-bounces@...osl.org> On Behalf
>> Of ALOK TIWARI
>> Sent: Monday, November 3, 2025 11:17 AM
>> To: Kitszel, Przemyslaw <przemyslaw.kitszel@...el.com>
>> Cc: alok.a.tiwarilinux@...il.com; Lobakin, Aleksander
>> <aleksander.lobakin@...el.com>; Nguyen, Anthony L
>> <anthony.l.nguyen@...el.com>; andrew+netdev@...n.ch; kuba@...nel.org;
>> davem@...emloft.net; edumazet@...gle.com; pabeni@...hat.com;
>> horms@...nel.org; intel-wired-lan@...ts.osuosl.org;
>> netdev@...r.kernel.org
>> Subject: Re: [Intel-wired-lan] [External] : Re: [PATCH net-next] iavf:
>> clarify VLAN add/delete log messages and lower log level
>>
>>
>>
>> On 11/3/2025 2:57 PM, Przemek Kitszel wrote:
>>> On 11/3/25 10:03, Alok Tiwari wrote:
>>>> The current dev_warn messages for too many VLAN changes are
>> confusing
>>>> and one place incorrectly reference "add" instead of "delete" VLANs
>>>> due to copy-paste errors.
>>>>
>>>> - Use dev_info instead of dev_warn to lower the log level.
>>>> - Rephrase the message to: "Too many VLAN [add|delete] changes
>>>> requested,
>>>> splitting into multiple messages to PF".
>>>>
>>>> Suggested-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
>>>> Signed-off-by: Alok Tiwari <alok.a.tiwari@...cle.com>
>>>
>>> thank you!
>>> just two minor nits, but the messages are good already, so:
>>> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
>>>
>>>> ---
>>>> https://urldefense.com/v3/__https://lore.kernel.org/all/47f8c95c-
>>>> bac4-471f-8e58-9155c6e58cb5@...el.com/__;!!ACWV5N9M2RV99hQ!
>>>> MulRvlOtC3JAON-O816_nR2P2geYBHDIx86XOr_i1afc9gjSrXfpcVwFmP6uM0p1-
>>>> kFN64zBSLjwS3XvTDQ6nJ5R2hyIaQ$ ---
>>>> drivers/net/ethernet/intel/iavf/iavf_virtchnl.c | 12 ++++++++----
>>>> 1 file changed, 8 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/
>>>> drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
>>>> index 34a422a4a29c..3593c0b45cf7 100644
>>>> --- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
>>>> +++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
>>>> @@ -793,7 +793,8 @@ void iavf_add_vlans(struct iavf_adapter
>> *adapter)
>>>> len = virtchnl_struct_size(vvfl, vlan_id, count);
>>>> if (len > IAVF_MAX_AQ_BUF_SIZE) {
>>>> - dev_warn(&adapter->pdev->dev, "Too many add VLAN
>> changes
>>>> in one request\n");
>>>> + dev_info(&adapter->pdev->dev, "Too many VLAN add
>> changes
>>>> requested,\n"
>>>> + "splitting into multiple messages to PF\n");
>>>
>>> perhaps it is too much bikeshedding for such a change, sorry, but I
>>> would rather remove the newline in the middle
>>>
>>> nit: another thing that I would consider is to differentiate the
>>> messages (v1/v2 or A/B, or whatever) for different protocol versions
>>
>>
>> Thanks Przemek for the feedback.
>>
>> I initially had the message on a single line, but checkpatch.pl
>> reported: "WARNING: quoted string split across lines"
>>
>> To avoid that warning, I added the "\n" and split the message.
>> I can drop the newline and suppress the warning if the maintainer
>> community prefers.
>> I just wanted to stay consistent with checkpatch recommendations.
>>
>> good point, I can adjust the wording and add version tags (v1/v2) now
>> The messages currently look like this:
>>
>> dev_info(&adapter->pdev->dev, "vvfl Too many VLAN add changes
>> requested, "
>> "splitting into multiple messages to PF\n");
>>
>> dev_info(&adapter->pdev->dev, "vvfl_v2 Too many VLAN add changes
>> requested, "
>> "splitting into multiple messages to PF\n");
>>
>> Thanks,
>> Alok
>
> Thanks for clarifying, Alok.
> checkpatch.pl warnings about split strings are advisory, not mandatory. For log messages, kernel style generally prefers a single, readable line rather than introducing "\n" just to silence that warning. Multi-line messages make grepping harder and are rarely needed for short text.
> So please keep the message on one line, even if that means ignoring the checkpatch warning.
there most important rule is
"do not break (split) user visible messages", and checkpatch is aware of
that. Only then is "do not go too far with characters in given line".
>
> Something like:
> dev_info(&adapter->pdev->dev,
> "vvfl Too many VLAN add requests; splitting into multiple messages to PF\n");
>
> Same for the delete case. Dropping the comma for a semicolon improves clarity.
> Adding version tags (v2) and adjusting wording as you suggested sounds good.
> Thank you.
>
> Alex
Powered by blists - more mailing lists