Messages in this thread | | | From | Anjali Kulkarni <> | Subject | Re: [PATCH v5 1/6] netlink: Reverse the patch which removed filtering | Date | Thu, 1 Jun 2023 20:24:52 +0000 |
| |
> 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
| |