Re: [PATCH (repost) net-next] sched: add extack for tfilter_notify

[Hangbin Liu <liuhangbin@gmail.com>]

Hi Jamal,

On Tue, Nov 15, 2022 at 11:26:54AM -0500, Jamal Hadi Salim wrote:
> How were you thinking of storing the extack info into the message?
> Note: I wouldnt touch netlink_ack() - maybe write a separate helper for events.

It looks a separate helper is just duplicated codes. Send the notify from
the netlink_ack() is much easy. e.g.

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index a662e8a5ff84..79a1586e8eb0 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2463,6 +2463,33 @@ netlink_ack_tlv_fill(struct sk_buff *in_skb, struct sk_buff *skb,
 				    (u8 *)extack->miss_nest - (u8 *)nlh));
 }
 
+/**
+ * get_nlgroup - get netlink group id based on nlmsg_type
+ * @nlmsg_type: netlink message type
+ *
+ * Right now, we only support net tc sched. Please add other types if the
+ * sub maintainers feel needed.
+ */
+unsigned int get_nlgroup(__u16 nlmsg_type)
+{
+	switch (nlmsg_type) {
+	case RTM_NEWTFILTER:
+	case RTM_DELTFILTER:
+	case RTM_NEWCHAIN:
+	case RTM_DELCHAIN:
+	case RTM_NEWTCLASS:
+	case RTM_DELTCLASS:
+	case RTM_NEWQDISC:
+	case RTM_DELQDISC:
+	case RTM_GETACTION:
+	case RTM_NEWACTION:
+	case RTM_DELACTION:
+		return RTNLGRP_TC;
+	}
+
+	return 0;
+}
+
 void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err,
 		 const struct netlink_ext_ack *extack)
 {
@@ -2471,7 +2498,7 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err,
 	struct nlmsgerr *errmsg;
 	size_t payload = sizeof(*errmsg);
 	struct netlink_sock *nlk = nlk_sk(NETLINK_CB(in_skb).sk);
-	unsigned int flags = 0;
+	unsigned int group, flags = 0;
 	size_t tlvlen;
 
 	/* Error messages get the original request appened, unless the user
@@ -2507,7 +2534,9 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err,
 
 	nlmsg_end(skb, rep);
 
-	nlmsg_unicast(in_skb->sk, skb, NETLINK_CB(in_skb).portid);
+	group = get_nlgroup(nlh->nlmsg_type);
+	/* Multicast the error message if we can get group if from nlmsg_type */
+	nlmsg_notify(in_skb->sk, skb, NETLINK_CB(in_skb).portid, group, 1, GFP_KERNEL);
 }
 EXPORT_SYMBOL(netlink_ack);
 

What do you think?

Thanks
Hangbin