diff mbox series

[1/2] rtnetlink: use nlmsg_{multicast, unicast} instead of netlink_{broadcast,unicast}

Message ID 20210713123654.31174-2-yajun.deng@linux.dev (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series net: Use nlmsg_{multicast, unicast} that contain if statement | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Guessed tree name to be net-next
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cc_maintainers warning 2 maintainers not CCed: laniel_francis@privacyrequired.com cong.wang@bytedance.com
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit fail Errors and warnings before: 4146 this patch: 3943
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch warning CHECK: extern prototypes should be avoided in .h files WARNING: line length of 89 exceeds 80 columns WARNING: line length of 90 exceeds 80 columns
netdev/build_allmodconfig_warn fail Errors and warnings before: 4202 this patch: 3985
netdev/header_inline success Link

Commit Message

Yajun Deng July 13, 2021, 12:36 p.m. UTC
It has a 'NETLINK_CB(' statement in nlmsg_multicast() and has 'if (err'
in nlmsg_{multicast, unicast}, use nlmsg_{multicast, unicast} instead
of netlink_{broadcast,unicast}. so the caller would not deal with the
'if (err >0 )' statement. Add the return value for nlmsg_multicast.
As also, rename rtnetlink_send() to rtnl_send(), this makes style
uniform.

Signed-off-by: Yajun Deng <yajun.deng@linux.dev>
---
 include/linux/rtnetlink.h |  2 +-
 net/core/rtnetlink.c      | 13 +++++++------
 2 files changed, 8 insertions(+), 7 deletions(-)

Comments

Cong Wang July 15, 2021, 4:21 a.m. UTC | #1
On Tue, Jul 13, 2021 at 5:37 AM Yajun Deng <yajun.deng@linux.dev> wrote:
>
> It has a 'NETLINK_CB(' statement in nlmsg_multicast() and has 'if (err'
> in nlmsg_{multicast, unicast}, use nlmsg_{multicast, unicast} instead
> of netlink_{broadcast,unicast}. so the caller would not deal with the
> 'if (err >0 )' statement. Add the return value for nlmsg_multicast.
> As also, rename rtnetlink_send() to rtnl_send(), this makes style
> uniform.
>
> Signed-off-by: Yajun Deng <yajun.deng@linux.dev>
> ---
>  include/linux/rtnetlink.h |  2 +-
>  net/core/rtnetlink.c      | 13 +++++++------
>  2 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
> index bb9cb84114c1..60bef82e42ab 100644
> --- a/include/linux/rtnetlink.h
> +++ b/include/linux/rtnetlink.h
> @@ -9,7 +9,7 @@
>  #include <linux/refcount.h>
>  #include <uapi/linux/rtnetlink.h>
>
> -extern int rtnetlink_send(struct sk_buff *skb, struct net *net, u32 pid, u32 group, int echo);
> +extern int rtnl_send(struct sk_buff *skb, struct net *net, u32 pid, u32 group, int echo);
>  extern int rtnl_unicast(struct sk_buff *skb, struct net *net, u32 pid);
>  extern void rtnl_notify(struct sk_buff *skb, struct net *net, u32 pid,
>                         u32 group, struct nlmsghdr *nlh, gfp_t flags);
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index f6af3e74fc44..c081d607bb69 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -707,17 +707,18 @@ static int rtnl_link_fill(struct sk_buff *skb, const struct net_device *dev)
>         return err;
>  }
>
> -int rtnetlink_send(struct sk_buff *skb, struct net *net, u32 pid, unsigned int group, int echo)
> +int rtnl_send(struct sk_buff *skb, struct net *net, u32 pid, unsigned int group, int echo)
>  {
>         struct sock *rtnl = net->rtnl;
>         int err = 0;
>
> -       NETLINK_CB(skb).dst_group = group;
> -       if (echo)
> +       err = nlmsg_multicast(rtnl, skb, pid, group, GFP_KERNEL);
> +
> +       if (echo) {
>                 refcount_inc(&skb->users);

You also moved this refcount_inc() down after nlmsg_multicast().
Are you sure it is safe?

And the name rtnl_send() is bad given that rtnl_unicast() follows it...

Thanks.
diff mbox series

Patch

diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index bb9cb84114c1..60bef82e42ab 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -9,7 +9,7 @@ 
 #include <linux/refcount.h>
 #include <uapi/linux/rtnetlink.h>
 
-extern int rtnetlink_send(struct sk_buff *skb, struct net *net, u32 pid, u32 group, int echo);
+extern int rtnl_send(struct sk_buff *skb, struct net *net, u32 pid, u32 group, int echo);
 extern int rtnl_unicast(struct sk_buff *skb, struct net *net, u32 pid);
 extern void rtnl_notify(struct sk_buff *skb, struct net *net, u32 pid,
 			u32 group, struct nlmsghdr *nlh, gfp_t flags);
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index f6af3e74fc44..c081d607bb69 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -707,17 +707,18 @@  static int rtnl_link_fill(struct sk_buff *skb, const struct net_device *dev)
 	return err;
 }
 
-int rtnetlink_send(struct sk_buff *skb, struct net *net, u32 pid, unsigned int group, int echo)
+int rtnl_send(struct sk_buff *skb, struct net *net, u32 pid, unsigned int group, int echo)
 {
 	struct sock *rtnl = net->rtnl;
 	int err = 0;
 
-	NETLINK_CB(skb).dst_group = group;
-	if (echo)
+	err = nlmsg_multicast(rtnl, skb, pid, group, GFP_KERNEL);
+
+	if (echo) {
 		refcount_inc(&skb->users);
-	netlink_broadcast(rtnl, skb, pid, group, GFP_KERNEL);
-	if (echo)
-		err = netlink_unicast(rtnl, skb, pid, MSG_DONTWAIT);
+		err = nlmsg_unicast(rtnl, skb, pid);
+	}
+
 	return err;
 }