diff mbox series

[net-next,v3,1/3] net: document return value of dev_getbyhwaddr_rcu()

Message ID 20250212-arm_fix_selftest-v3-1-72596cb77e44@debian.org (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: core: improvements to device lookup by hardware address. | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 5 of 5 maintainers
netdev/build_clang success Errors and warnings before: 2 this patch: 2
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 5 this patch: 5
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 9 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 79 this patch: 78
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2025-02-13--12-00 (tests: 889)

Commit Message

Breno Leitao Feb. 12, 2025, 5:47 p.m. UTC 
Add missing return value documentation in the kernel-doc comment for
dev_getbyhwaddr_rcu(), clarifying that it returns either a pointer to
net_device or NULL if no matching device is found.

This fix a warning found in NIPA[1]:

	net/core/dev.c:1141: warning: No description found for return value of 'dev_getbyhwaddr_rcu'

Link: https://netdev.bots.linux.dev/static/nipa/931564/13964899/kdoc/summary [1]
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 net/core/dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Kuniyuki Iwashima Feb. 13, 2025, 7:36 a.m. UTC | #1 
From: Breno Leitao <leitao@debian.org>
Date: Wed, 12 Feb 2025 09:47:24 -0800
> diff --git a/net/core/dev.c b/net/core/dev.c
> index d5ab9a4b318ea4926c200ef20dae01eaafa18c6b..0b3480a125fcaa6f036ddf219c29fa362ea0cb29 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1134,8 +1134,8 @@ int netdev_get_name(struct net *net, char *name, int ifindex)
>   *	The returned device has 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
>   */

I noticed here we still mention RTNL and it should be removed.

Could you update the comment like this to remove RTNL and fix
mis-aligned notes ?

---8<---
diff --git a/net/core/dev.c b/net/core/dev.c
index d5ab9a4b318e..c0d6017a3840 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1123,17 +1123,17 @@ int netdev_get_name(struct net *net, char *name, int ifindex)
 }
 
 /**
- *	dev_getbyhwaddr_rcu - find a device by its hardware address
- *	@net: the applicable net namespace
- *	@type: media type of device
- *	@ha: hardware address
+ * dev_getbyhwaddr_rcu - find a device by its hardware address
+ * @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 or RTNL.
- *	The returned device has not had its ref count increased
- *	and the caller must therefore be careful about locking
+ * Search for an interface by MAC address.  The returned device has
+ * not had its ref count increased and the caller must therefore be
+ * careful about locking
  *
+ * Context: The caller must hold RCU.
+ * Return: pointer to the net_device, or NULL if not found
  */
 
 struct net_device *dev_getbyhwaddr_rcu(struct net *net, unsigned short type,
---8<---

Thanks!
Kuniyuki Iwashima Feb. 13, 2025, 7:47 a.m. UTC | #2 
From: Kuniyuki Iwashima <kuniyu@amazon.com>
Date: Thu, 13 Feb 2025 16:36:46 +0900
> From: Breno Leitao <leitao@debian.org>
> Date: Wed, 12 Feb 2025 09:47:24 -0800
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index d5ab9a4b318ea4926c200ef20dae01eaafa18c6b..0b3480a125fcaa6f036ddf219c29fa362ea0cb29 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -1134,8 +1134,8 @@ int netdev_get_name(struct net *net, char *name, int ifindex)
> >   *	The returned device has 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
> >   */
> 
> I noticed here we still mention RTNL and it should be removed.

I missed this part is removed in patch 2, but the Return: part
is still duplicate.
Breno Leitao Feb. 13, 2025, 10:16 a.m. UTC | #3 
Hello Kuniyuki,

On Thu, Feb 13, 2025 at 04:47:48PM +0900, Kuniyuki Iwashima wrote:
> From: Kuniyuki Iwashima <kuniyu@amazon.com>
> Date: Thu, 13 Feb 2025 16:36:46 +0900
> > From: Breno Leitao <leitao@debian.org>
> > Date: Wed, 12 Feb 2025 09:47:24 -0800
> > > diff --git a/net/core/dev.c b/net/core/dev.c
> > > index d5ab9a4b318ea4926c200ef20dae01eaafa18c6b..0b3480a125fcaa6f036ddf219c29fa362ea0cb29 100644
> > > --- a/net/core/dev.c
> > > +++ b/net/core/dev.c
> > > @@ -1134,8 +1134,8 @@ int netdev_get_name(struct net *net, char *name, int ifindex)
> > >   *	The returned device has 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
> > >   */
> > 
I am a bit confused about what you are saying.
> > I noticed here we still mention RTNL and it should be removed.

I have no mention RTNL in this patch at all:

	# git log -n1 --oneline HEAD~2
	6d34fd4700231 net: document return value of dev_getbyhwaddr_rcu()
	# git show  HEAD~2  | grep -i rtnl

> I missed this part is removed in patch 2, but the Return: part
> is still duplicate.

This part is also unclear to me. What do you mean the "Return:" part is
still duplicated?

This is how the documentation looks like, after the patch applied:

	/**
	*      dev_getbyhwaddr_rcu - find a device by its hardware address
	*      @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 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
	*/
	struct net_device *dev_getbyhwaddr_rcu(struct net *net, unsigned short type,
						const char *ha)
	{
		<snip>
	}

	/**
	*      dev_getbyhwaddr - find a device by its hardware address
	*      @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
	*/
	struct net_device *dev_getbyhwaddr(struct net *net, unsigned short type,
					const char *ha)
	{
		<snip>
	}

Where is the Return: part duplicated?

Thanks for the review
--breno
Kuniyuki Iwashima Feb. 13, 2025, 10:45 a.m. UTC | #4 
From: Breno Leitao <leitao@debian.org>
Date: Thu, 13 Feb 2025 02:16:36 -0800
> Hello Kuniyuki,
> 
> On Thu, Feb 13, 2025 at 04:47:48PM +0900, Kuniyuki Iwashima wrote:
> > From: Kuniyuki Iwashima <kuniyu@amazon.com>
> > Date: Thu, 13 Feb 2025 16:36:46 +0900
> > > From: Breno Leitao <leitao@debian.org>
> > > Date: Wed, 12 Feb 2025 09:47:24 -0800
> > > > diff --git a/net/core/dev.c b/net/core/dev.c
> > > > index d5ab9a4b318ea4926c200ef20dae01eaafa18c6b..0b3480a125fcaa6f036ddf219c29fa362ea0cb29 100644
> > > > --- a/net/core/dev.c
> > > > +++ b/net/core/dev.c
> > > > @@ -1134,8 +1134,8 @@ int netdev_get_name(struct net *net, char *name, int ifindex)
> > > >   *	The returned device has 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
> > > >   */
> > > 
> I am a bit confused about what you are saying.
> > > I noticed here we still mention RTNL and it should be removed.
> 
> I have no mention RTNL in this patch at all:

Yes, and patch 2 removes the part from kernel-doc of
dev_getbyhwaddr_rcu().


> 
> 	# git log -n1 --oneline HEAD~2
> 	6d34fd4700231 net: document return value of dev_getbyhwaddr_rcu()
> 	# git show  HEAD~2  | grep -i rtnl
> 
> > I missed this part is removed in patch 2, but the Return: part
> > is still duplicate.
> 
> This part is also unclear to me. What do you mean the "Return:" part is
> still duplicated?
> 
> This is how the documentation looks like, after the patch applied:
> 
> 	/**
> 	*      dev_getbyhwaddr_rcu - find a device by its hardware address
> 	*      @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
                                                       ^^^^^^^
Sorry, I meant this part is redundant as we have Return: now.


> 	*      is not found or a pointer to the device.
> 	*      The caller must hold RCU.
> 	*      The returned device has 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
> 	*/
> 	struct net_device *dev_getbyhwaddr_rcu(struct net *net, unsigned short type,
> 						const char *ha)
> 	{
> 		<snip>
> 	}
diff mbox series

Patch

diff --git a/net/core/dev.c b/net/core/dev.c
index d5ab9a4b318ea4926c200ef20dae01eaafa18c6b..0b3480a125fcaa6f036ddf219c29fa362ea0cb29 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1134,8 +1134,8 @@  int netdev_get_name(struct net *net, char *name, int ifindex)
  *	The returned device has 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
  */
-
 struct net_device *dev_getbyhwaddr_rcu(struct net *net, unsigned short type,
 				       const char *ha)
 {