diff mbox series

[net-next,6/8] genetlink: introduce helpers to do filtered multicast

Message ID 20231115141724.411507-7-jiri@resnulli.us (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series devlink: introduce notifications filtering | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
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: 3952 this patch: 3952
netdev/cc_maintainers success CCed 4 of 4 maintainers
netdev/build_clang success Errors and warnings before: 1323 this patch: 1323
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: 4198 this patch: 4198
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 101 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Jiri Pirko Nov. 15, 2023, 2:17 p.m. UTC 
From: Jiri Pirko <jiri@nvidia.com>

Currently it is possible for netlink kernel user to pass custom
filter function to broadcast send function netlink_broadcast_filtered().
However, this is not exposed to multicast send and to generic
netlink users.

Extend the api and introduce a netlink helper nlmsg_multicast_filtered()
and a generic netlink helper genlmsg_multicast_netns_filtered()
to allow generic netlink families to specify filter function
while sending multicast messages.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 include/net/genetlink.h | 35 +++++++++++++++++++++++++++++++----
 include/net/netlink.h   | 31 +++++++++++++++++++++++++++----
 2 files changed, 58 insertions(+), 8 deletions(-)

Comments

Andy Shevchenko Nov. 15, 2023, 2:53 p.m. UTC | #1 
On Wed, Nov 15, 2023 at 03:17:22PM +0100, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> Currently it is possible for netlink kernel user to pass custom
> filter function to broadcast send function netlink_broadcast_filtered().
> However, this is not exposed to multicast send and to generic
> netlink users.
> 
> Extend the api and introduce a netlink helper nlmsg_multicast_filtered()
> and a generic netlink helper genlmsg_multicast_netns_filtered()
> to allow generic netlink families to specify filter function
> while sending multicast messages.

...

> +/**
> + * genlmsg_multicast_netns_filtered - multicast a netlink message
> + *				      to a specific netns with filter
> + *				      function
> + * @family: the generic netlink family
> + * @net: the net namespace
> + * @skb: netlink message as socket buffer
> + * @portid: own netlink portid to avoid sending to yourself
> + * @group: offset of multicast group in groups array
> + * @flags: allocation flags
> + * @filter: filter function
> + * @filter_data: filter function private data

	scripts/kernel-doc -v -none -Wall ...

will complain.

> + */

...

> +				 int (*filter)(struct sock *dsk,
> +					       struct sk_buff *skb,
> +					       void *data),

Since it occurs more than once, perhaps

typedef int (*genlmsg_filter_fn)(struct sock *, struct sk_buff *, void *);

?

...

>  /**
> - * nlmsg_multicast - multicast a netlink message
> + * nlmsg_multicast_filtered - multicast a netlink message with filter function
>   * @sk: netlink socket to spread messages to
>   * @skb: netlink message as socket buffer
>   * @portid: own netlink portid to avoid sending to yourself
>   * @group: multicast group id
>   * @flags: allocation flags
> + * @filter: filter function
> + * @filter_data: filter function private data

I believe same complain by kernel-doc here and in more places...

Can you at least make sure your patches do not add new ones and removes ones
where you touch the code?
Jacob Keller Nov. 15, 2023, 8:20 p.m. UTC | #2 
On 11/15/2023 6:53 AM, Andy Shevchenko wrote:
> On Wed, Nov 15, 2023 at 03:17:22PM +0100, Jiri Pirko wrote:
>> +				 int (*filter)(struct sock *dsk,
>> +					       struct sk_buff *skb,
>> +					       void *data),
> 
> Since it occurs more than once, perhaps
> 
> typedef int (*genlmsg_filter_fn)(struct sock *, struct sk_buff *, void *);
> 
> ?
> 

I agree with having a typedef here.

Thanks,
Jake
Jiri Pirko Nov. 16, 2023, 7:15 a.m. UTC | #3 
Wed, Nov 15, 2023 at 03:53:37PM CET, andriy.shevchenko@linux.intel.com wrote:
>On Wed, Nov 15, 2023 at 03:17:22PM +0100, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@nvidia.com>
>> 
>> Currently it is possible for netlink kernel user to pass custom
>> filter function to broadcast send function netlink_broadcast_filtered().
>> However, this is not exposed to multicast send and to generic
>> netlink users.
>> 
>> Extend the api and introduce a netlink helper nlmsg_multicast_filtered()
>> and a generic netlink helper genlmsg_multicast_netns_filtered()
>> to allow generic netlink families to specify filter function
>> while sending multicast messages.
>
>...
>
>> +/**
>> + * genlmsg_multicast_netns_filtered - multicast a netlink message
>> + *				      to a specific netns with filter
>> + *				      function
>> + * @family: the generic netlink family
>> + * @net: the net namespace
>> + * @skb: netlink message as socket buffer
>> + * @portid: own netlink portid to avoid sending to yourself
>> + * @group: offset of multicast group in groups array
>> + * @flags: allocation flags
>> + * @filter: filter function
>> + * @filter_data: filter function private data
>
>	scripts/kernel-doc -v -none -Wall ...
>
>will complain.

Will fix.


>
>> + */
>
>...
>
>> +				 int (*filter)(struct sock *dsk,
>> +					       struct sk_buff *skb,
>> +					       void *data),
>
>Since it occurs more than once, perhaps
>
>typedef int (*genlmsg_filter_fn)(struct sock *, struct sk_buff *, void *);

Makes sense. Will make that in a separate patch before this one.


>
>?
>
>...
>
>>  /**
>> - * nlmsg_multicast - multicast a netlink message
>> + * nlmsg_multicast_filtered - multicast a netlink message with filter function
>>   * @sk: netlink socket to spread messages to
>>   * @skb: netlink message as socket buffer
>>   * @portid: own netlink portid to avoid sending to yourself
>>   * @group: multicast group id
>>   * @flags: allocation flags
>> + * @filter: filter function
>> + * @filter_data: filter function private data
>
>I believe same complain by kernel-doc here and in more places...
>
>Can you at least make sure your patches do not add new ones and removes ones
>where you touch the code?

Okay.


>
>-- 
>With Best Regards,
>Andy Shevchenko
>
>
diff mbox series

Patch

diff --git a/include/net/genetlink.h b/include/net/genetlink.h
index e18a4c0d69ee..ec93e73fd8f8 100644
--- a/include/net/genetlink.h
+++ b/include/net/genetlink.h
@@ -435,6 +435,35 @@  static inline void genlmsg_cancel(struct sk_buff *skb, void *hdr)
 		nlmsg_cancel(skb, hdr - GENL_HDRLEN - NLMSG_HDRLEN);
 }
 
+/**
+ * genlmsg_multicast_netns_filtered - multicast a netlink message
+ *				      to a specific netns with filter
+ *				      function
+ * @family: the generic netlink family
+ * @net: the net namespace
+ * @skb: netlink message as socket buffer
+ * @portid: own netlink portid to avoid sending to yourself
+ * @group: offset of multicast group in groups array
+ * @flags: allocation flags
+ * @filter: filter function
+ * @filter_data: filter function private data
+ */
+static inline int
+genlmsg_multicast_netns_filtered(const struct genl_family *family,
+				 struct net *net, struct sk_buff *skb,
+				 u32 portid, unsigned int group, gfp_t flags,
+				 int (*filter)(struct sock *dsk,
+					       struct sk_buff *skb,
+					       void *data),
+				 void *filter_data)
+{
+	if (WARN_ON_ONCE(group >= family->n_mcgrps))
+		return -EINVAL;
+	group = family->mcgrp_offset + group;
+	return nlmsg_multicast_filtered(net->genl_sock, skb, portid, group,
+					flags, filter, filter_data);
+}
+
 /**
  * genlmsg_multicast_netns - multicast a netlink message to a specific netns
  * @family: the generic netlink family
@@ -448,10 +477,8 @@  static inline int genlmsg_multicast_netns(const struct genl_family *family,
 					  struct net *net, struct sk_buff *skb,
 					  u32 portid, unsigned int group, gfp_t flags)
 {
-	if (WARN_ON_ONCE(group >= family->n_mcgrps))
-		return -EINVAL;
-	group = family->mcgrp_offset + group;
-	return nlmsg_multicast(net->genl_sock, skb, portid, group, flags);
+	return genlmsg_multicast_netns_filtered(family, net, skb, portid,
+						group, flags, NULL, NULL);
 }
 
 /**
diff --git a/include/net/netlink.h b/include/net/netlink.h
index 83bdf787aeee..60f510e33964 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -1073,27 +1073,50 @@  static inline void nlmsg_free(struct sk_buff *skb)
 }
 
 /**
- * nlmsg_multicast - multicast a netlink message
+ * nlmsg_multicast_filtered - multicast a netlink message with filter function
  * @sk: netlink socket to spread messages to
  * @skb: netlink message as socket buffer
  * @portid: own netlink portid to avoid sending to yourself
  * @group: multicast group id
  * @flags: allocation flags
+ * @filter: filter function
+ * @filter_data: filter function private data
  */
-static inline int nlmsg_multicast(struct sock *sk, struct sk_buff *skb,
-				  u32 portid, unsigned int group, gfp_t flags)
+static inline int nlmsg_multicast_filtered(struct sock *sk, struct sk_buff *skb,
+					   u32 portid, unsigned int group,
+					   gfp_t flags,
+					   int (*filter)(struct sock *dsk,
+							 struct sk_buff *skb,
+							 void *data),
+					   void *filter_data)
 {
 	int err;
 
 	NETLINK_CB(skb).dst_group = group;
 
-	err = netlink_broadcast(sk, skb, portid, group, flags);
+	err = netlink_broadcast_filtered(sk, skb, portid, group, flags,
+					 filter, filter_data);
 	if (err > 0)
 		err = 0;
 
 	return err;
 }
 
+/**
+ * nlmsg_multicast - multicast a netlink message
+ * @sk: netlink socket to spread messages to
+ * @skb: netlink message as socket buffer
+ * @portid: own netlink portid to avoid sending to yourself
+ * @group: multicast group id
+ * @flags: allocation flags
+ */
+static inline int nlmsg_multicast(struct sock *sk, struct sk_buff *skb,
+				  u32 portid, unsigned int group, gfp_t flags)
+{
+	return nlmsg_multicast_filtered(sk, skb, portid, group, flags,
+					NULL, NULL);
+}
+
 /**
  * nlmsg_unicast - unicast a netlink message
  * @sk: netlink socket to spread message to