On 11/3/25 11:30, Loktionov, Aleksandr wrote:


-----Original Message-----
From: Intel-wired-lan <[email protected]> On Behalf
Of ALOK TIWARI
Sent: Monday, November 3, 2025 11:17 AM
To: Kitszel, Przemyslaw <[email protected]>
Cc: [email protected]; Lobakin, Aleksander
<[email protected]>; Nguyen, Anthony L
<[email protected]>; [email protected]; [email protected];
[email protected]; [email protected]; [email protected];
[email protected]; [email protected];
[email protected]
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 <[email protected]>
Signed-off-by: Alok Tiwari <[email protected]>

thank you!
just two minor nits, but the messages are good already, so:
Reviewed-by: Przemek Kitszel <[email protected]>

---
https://urldefense.com/v3/__https://lore.kernel.org/all/47f8c95c-
[email protected]/__;!!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

Reply via email to