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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y4W9qEHzg5h9n/od@Laptop-X1>
Date:   Tue, 29 Nov 2022 16:07:04 +0800
From:   Hangbin Liu <liuhangbin@...il.com>
To:     Jamal Hadi Salim <jhs@...atatu.com>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
        Cong Wang <xiyou.wangcong@...il.com>, netdev@...r.kernel.org,
        Jiri Pirko <jiri@...nulli.us>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Paolo Abeni <pabeni@...hat.com>,
        David Ahern <dsahern@...nel.org>
Subject: Re: [PATCH (repost) net-next] sched: add extack for tfilter_notify

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