lkml.org 
[lkml]   [2023]   [Jun]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v5 1/6] netlink: Reverse the patch which removed filtering
Date


> On Jun 1, 2023, at 10:56 AM, Liam Howlett <liam.howlett@oracle.com> wrote:
>
> * Anjali Kulkarni <anjali.k.kulkarni@oracle.com> [691231 23:00]:
>> To use filtering at the connector & cn_proc layers, we need to enable
>> filtering in the netlink layer. This reverses the patch which removed
>> netlink filtering.
>>
>> Signed-off-by: Anjali Kulkarni <anjali.k.kulkarni@oracle.com>
>> ---
>> include/linux/netlink.h | 5 +++++
>> net/netlink/af_netlink.c | 25 +++++++++++++++++++++++--
>> 2 files changed, 28 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/netlink.h b/include/linux/netlink.h
>> index c43ac7690eca..866bbc5a4c8d 100644
>> --- a/include/linux/netlink.h
>> +++ b/include/linux/netlink.h
>> @@ -206,6 +206,11 @@ bool netlink_strict_get_check(struct sk_buff *skb);
>> int netlink_unicast(struct sock *ssk, struct sk_buff *skb, __u32 portid, int nonblock);
>> int netlink_broadcast(struct sock *ssk, struct sk_buff *skb, __u32 portid,
>> __u32 group, gfp_t allocation);
>> +int netlink_broadcast_filtered(struct sock *ssk, struct sk_buff *skb,
>> + __u32 portid, __u32 group, gfp_t allocation,
>> + int (*filter)(struct sock *dsk,
>> + struct sk_buff *skb, void *data),
>> + void *filter_data);
>
> Nit, just a personal preference that if you indent with two tabs for
> function definitions, then it is clear where the code starts and you
> have more room for larger argument lists here. It also helps when
> changing the return type as you don't have to redo all the spacing.

Thanks so much - will update my code will all your review comments plus those of Jakub & Andrew, and send it out in the next revision.

Anjali

>
>> int netlink_set_err(struct sock *ssk, __u32 portid, __u32 group, int code);
>> int netlink_register_notifier(struct notifier_block *nb);
>> int netlink_unregister_notifier(struct notifier_block *nb);
>> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
>> index c64277659753..003c7e6ec9be 100644
>> --- a/net/netlink/af_netlink.c
>> +++ b/net/netlink/af_netlink.c
>> @@ -1432,6 +1432,8 @@ struct netlink_broadcast_data {
>> int delivered;
>> gfp_t allocation;
>> struct sk_buff *skb, *skb2;
>> + int (*tx_filter)(struct sock *dsk, struct sk_buff *skb, void *data);
>> + void *tx_data;
>> };
>>
>> static void do_one_broadcast(struct sock *sk,
>> @@ -1485,6 +1487,11 @@ static void do_one_broadcast(struct sock *sk,
>> p->delivery_failure = 1;
>> goto out;
>> }
>
> new line here please.
>
>> + if (p->tx_filter && p->tx_filter(sk, p->skb2, p->tx_data)) {
>> + kfree_skb(p->skb2);
>> + p->skb2 = NULL;
>> + goto out;
>> + }
>
> new line here please.
>
> Since there are now two times that the same steps are being used for
> unrolling (yours and below). It might be better to make a new goto
> label after the successful one?
>
>> if (sk_filter(sk, p->skb2)) {
>> kfree_skb(p->skb2);
>> p->skb2 = NULL;
>> @@ -1507,8 +1514,12 @@ static void do_one_broadcast(struct sock *sk,
>> sock_put(sk);
>> }
>>
>> -int netlink_broadcast(struct sock *ssk, struct sk_buff *skb, u32 portid,
>> - u32 group, gfp_t allocation)
>> +int netlink_broadcast_filtered(struct sock *ssk, struct sk_buff *skb,
>> + u32 portid,
>> + u32 group, gfp_t allocation,
>> + int (*filter)(struct sock *dsk,
>> + struct sk_buff *skb, void *data),
>> + void *filter_data)
>
> Same comment here about the two tab indent.
>
>> {
>> struct net *net = sock_net(ssk);
>> struct netlink_broadcast_data info;
>> @@ -1527,6 +1538,8 @@ int netlink_broadcast(struct sock *ssk, struct sk_buff *skb, u32 portid,
>> info.allocation = allocation;
>> info.skb = skb;
>> info.skb2 = NULL;
>> + info.tx_filter = filter;
>> + info.tx_data = filter_data;
>>
>> /* While we sleep in clone, do not allow to change socket list */
>>
>> @@ -1552,6 +1565,14 @@ int netlink_broadcast(struct sock *ssk, struct sk_buff *skb, u32 portid,
>> }
>> return -ESRCH;
>> }
>> +EXPORT_SYMBOL(netlink_broadcast_filtered);
>> +
>> +int netlink_broadcast(struct sock *ssk, struct sk_buff *skb, u32 portid,
>> + u32 group, gfp_t allocation)
>> +{
>> + return netlink_broadcast_filtered(ssk, skb, portid, group, allocation,
>> + NULL, NULL);
>> +}
>> EXPORT_SYMBOL(netlink_broadcast);
>>
>> struct netlink_set_err_data {
>> --
>> 2.40.0

\
 
 \ /
  Last update: 2023-06-01 22:26    [W:0.341 / U:0.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site