Re: [PATCH net-next v3 2/3] net: Add dev_getbyhwaddr_rtnl() helper

[Kuniyuki Iwashima <kuniyu@amazon.com>]

From: Breno Leitao <leitao@debian.org>
Date: Thu, 13 Feb 2025 02:29:38 -0800
> Hello Kuniyuki,
> 
> On Thu, Feb 13, 2025 at 04:31:29PM +0900, Kuniyuki Iwashima wrote:
> > > Subject: [PATCH net-next v3 2/3] net: Add dev_getbyhwaddr_rtnl() helper
> > 
> > s/_rtnl//
> 
> Ack!
> 
> > looks like Uday's comment was missed due to the lore issue.
> 
> hmm, I haven't seen Uday's comment. Didn't it reach lore?

I saw the reply and my another one on lore but somehow they
were removed :)


> 
> > From: Breno Leitao <leitao@debian.org>
> > Date: Wed, 12 Feb 2025 09:47:25 -0800
> > > +/**
> > > + *	dev_getbyhwaddr - find a device by its hardware address
> > 
> > While at it, could you replace '\t' after '*' to a single '\s'
> > for all kernel-doc comment lines below ?
> > 
> > 
> > > + *	@net: the applicable net namespace
> > > + *	@type: media type of device
> > > + *	@ha: hardware address
> > > + *
> > > + *	Similar to dev_getbyhwaddr_rcu(), but the owner needs to hold
> > > + *	rtnl_lock.
> > 
> > Otherwise the text here is mis-aligned.
> 
> Sorry, what is misaligned specifically? I generated the documentation,
> and I can't see it misaligned.
> 
> This is what I see when generating the document (full log at
> https://pastebin.mozilla.org/YkotEoHh#L250,271)
> 
> 
> 	dev_getbyhwaddr_rcu(9)                                                         Kernel Hacker's Manual                                                         dev_getbyhwaddr_rcu(9)
> 
> 	NAME
> 	dev_getbyhwaddr_rcu - find a device by its hardware address
> 
> 	SYNOPSIS
> 	struct net_device * dev_getbyhwaddr_rcu (struct net *net , unsigned short type , const char *ha );
> 
> 	ARGUMENTS
> 	net         the applicable net namespace
> 
> 	type        media type of device
> 
> 	ha          hardware address
> 
> 			Search for an interface by MAC address. Returns NULL if the device is not found or a pointer to the device.  The caller must hold RCU.  The returned  device  has
                        ^^^^^^
This scentence starts from a weird position when we use '\t' after '*'.
You will see it start from the head if '\s' follows '*'.


> 			not had its ref count increased and the caller must therefore be careful about locking
> 
> 	RETURN
> 	pointer to the net_device, or NULL if not found
> 
> 	dev_getbyhwaddr(9)                                                             Kernel Hacker's Manual                                                             dev_getbyhwaddr(9)
> 
> 	NAME
> 	dev_getbyhwaddr - find a device by its hardware address
> 
> 	SYNOPSIS
> 	struct net_device * dev_getbyhwaddr (struct net *net , unsigned short type , const char *ha );
> 
> 	ARGUMENTS
> 	net         the applicable net namespace
> 
> 	type        media type of device
> 
> 	ha          hardware address
> 
> 			Similar to dev_getbyhwaddr_rcu, but the owner needs to hold rtnl_lock.
> 
> 	RETURN
> 	pointer to the net_device, or NULL if not found
> 
> 
> >   $ ./scripts/kernel-doc -man net/core/dev.c | \
> >     scripts/split-man.pl /tmp/man && \
> >     man /tmp/man/dev_getbyhwaddr.9
> > 
> > Also, the latter part should be in Context:
> > 
> > Context: rtnl_lock() must be held.
> 
> Sure. Should I do something similar for _rcu function as well?
> 
> 	Context: caller must hold rcu_read_lock

Yes, that would be nice.

https://lore.kernel.org/netdev/20250213073646.14847-1-kuniyu@amazon.com/