Re: [PATCH v14 01/10] netlink: add subscription on changes in NDP/ARP table

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 1155 bytes --]

On Sun, Oct 19, 2025 at 12:07:30PM +0200, Stefano Brivio wrote:
> On Tue, 14 Oct 2025 22:55:12 -0400
> Jon Maloy <jmaloy@redhat.com> wrote:
[snip]
> > +		warn("neigh_msg_read() failed, error %i", errmsg->error);
> 
> But neigh_msg_read() isn't a function name (it's nl_neigh_msg_read()).
> To avoid that, if you really need the function name, I'd suggest %s and
> __func__.
> 
> However, here, I don't think you can say that the function failed. It's
> just that we got an error from netlink. The function didn't do almost
> anything. Maybe just "netlink error on neighbour notifier: ..."?

I didn't catch this when I reviewed, but I think it's probably best to
avoid function names in messages above debug() level.  Rationale:
debug() and trace() messages are aimed in large part towards
developers, and can have whatever information is useful to them,
warn(), err() and die() are aimed at users to whom function names are
meaningless.

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]