[PATCHSET] rtnetlink notification rework

[PATCHSET] rtnetlink notification rework

[Thomas Graf]

This patchset reworks rtnetlink notifications. Notification logic
is fixed and gets hidden behind nlmsg_notify() and the rtnl socket
is no longer directly accessed. All notification paths get
appropriate error handling. NLM_F_ECHO support is added where it
makes sense and no major surgery is required. Abuses of current->pid
for netlink pids are fixed. An exception is made for users of
rtnetlink_send() which will get converted in a second step as some
bigger code change are required.

[PATCH 01/16] [RTNETLINK]: Use rtnl_unicast() for rtnetlink unicasts

[Thomas Graf]

[-- Attachment #1: rtnl_unicast --]
[-- Type: text/plain, Size: 5220 bytes --]

Signed-off-by: Thomas Graf <tgraf@suug.ch>

Index: net-2.6.19.git/include/linux/rtnetlink.h
===================================================================
--- net-2.6.19.git.orig/include/linux/rtnetlink.h
+++ net-2.6.19.git/include/linux/rtnetlink.h
@@ -584,6 +584,7 @@ struct rtnetlink_link
 
 extern struct rtnetlink_link * rtnetlink_links[NPROTO];
 extern int rtnetlink_send(struct sk_buff *skb, u32 pid, u32 group, int echo);
+extern int rtnl_unicast(struct sk_buff *skb, u32 pid);
 extern int rtnetlink_put_metrics(struct sk_buff *skb, u32 *metrics);
 
 extern void __rta_fill(struct sk_buff *skb, int attrtype, int attrlen, const void *data);
Index: net-2.6.19.git/net/core/rtnetlink.c
===================================================================
--- net-2.6.19.git.orig/net/core/rtnetlink.c
+++ net-2.6.19.git/net/core/rtnetlink.c
@@ -166,6 +166,11 @@ int rtnetlink_send(struct sk_buff *skb, 
 	return err;
 }
 
+int rtnl_unicast(struct sk_buff *skb, u32 pid)
+{
+	return nlmsg_unicast(rtnl, skb, pid);
+}
+
 int rtnetlink_put_metrics(struct sk_buff *skb, u32 *metrics)
 {
 	struct rtattr *mx = (struct rtattr*)skb->tail;
@@ -574,9 +579,7 @@ static int rtnl_getlink(struct sk_buff *
 		goto errout;
 	}
 
-	err = netlink_unicast(rtnl, skb, NETLINK_CB(skb).pid, MSG_DONTWAIT);
-	if (err > 0)
-		err = 0;
+	err = rtnl_unicast(skb, NETLINK_CB(skb).pid);
 errout:
 	kfree(iw_buf);
 	dev_put(dev);
@@ -825,3 +828,4 @@ EXPORT_SYMBOL(rtnl);
 EXPORT_SYMBOL(rtnl_lock);
 EXPORT_SYMBOL(rtnl_trylock);
 EXPORT_SYMBOL(rtnl_unlock);
+EXPORT_SYMBOL(rtnl_unicast);
Index: net-2.6.19.git/net/decnet/dn_route.c
===================================================================
--- net-2.6.19.git.orig/net/decnet/dn_route.c
+++ net-2.6.19.git/net/decnet/dn_route.c
@@ -1611,9 +1611,7 @@ int dn_cache_getroute(struct sk_buff *in
 		goto out_free;
 	}
 
-	err = netlink_unicast(rtnl, skb, NETLINK_CB(in_skb).pid, MSG_DONTWAIT);
-
-	return err;
+	return rtnl_unicast(skb, NETLINK_CB(in_skb).pid);
 
 out_free:
 	kfree_skb(skb);
Index: net-2.6.19.git/net/ipv4/ipmr.c
===================================================================
--- net-2.6.19.git.orig/net/ipv4/ipmr.c
+++ net-2.6.19.git/net/ipv4/ipmr.c
@@ -312,7 +312,8 @@ static void ipmr_destroy_unres(struct mf
 			e = NLMSG_DATA(nlh);
 			e->error = -ETIMEDOUT;
 			memset(&e->msg, 0, sizeof(e->msg));
-			netlink_unicast(rtnl, skb, NETLINK_CB(skb).dst_pid, MSG_DONTWAIT);
+
+			rtnl_unicast(skb, NETLINK_CB(skb).pid);
 		} else
 			kfree_skb(skb);
 	}
@@ -512,7 +513,6 @@ static void ipmr_cache_resolve(struct mf
 
 	while((skb=__skb_dequeue(&uc->mfc_un.unres.unresolved))) {
 		if (skb->nh.iph->version == 0) {
-			int err;
 			struct nlmsghdr *nlh = (struct nlmsghdr *)skb_pull(skb, sizeof(struct iphdr));
 
 			if (ipmr_fill_mroute(skb, c, NLMSG_DATA(nlh)) > 0) {
@@ -525,7 +525,8 @@ static void ipmr_cache_resolve(struct mf
 				e->error = -EMSGSIZE;
 				memset(&e->msg, 0, sizeof(e->msg));
 			}
-			err = netlink_unicast(rtnl, skb, NETLINK_CB(skb).dst_pid, MSG_DONTWAIT);
+
+			rtnl_unicast(skb, NETLINK_CB(skb).pid);
 		} else
 			ip_mr_forward(skb, c, 0);
 	}
Index: net-2.6.19.git/net/ipv4/route.c
===================================================================
--- net-2.6.19.git.orig/net/ipv4/route.c
+++ net-2.6.19.git/net/ipv4/route.c
@@ -2809,10 +2809,9 @@ int inet_rtm_getroute(struct sk_buff *in
 		goto out_free;
 	}
 
-	err = netlink_unicast(rtnl, skb, NETLINK_CB(in_skb).pid, MSG_DONTWAIT);
-	if (err > 0)
-		err = 0;
-out:	return err;
+	err = rtnl_unicast(skb, NETLINK_CB(in_skb).pid);
+out:
+	return err;
 
 out_free:
 	kfree_skb(skb);
Index: net-2.6.19.git/net/ipv6/addrconf.c
===================================================================
--- net-2.6.19.git.orig/net/ipv6/addrconf.c
+++ net-2.6.19.git/net/ipv6/addrconf.c
@@ -3268,9 +3268,7 @@ static int inet6_rtm_getaddr(struct sk_b
 		goto out_free;
 	}
 
-	err = netlink_unicast(rtnl, skb, NETLINK_CB(in_skb).pid, MSG_DONTWAIT);
-	if (err > 0)
-		err = 0;
+	err = rtnl_unicast(skb, NETLINK_CB(in_skb).pid);
 out:
 	in6_ifa_put(ifa);
 	return err;
Index: net-2.6.19.git/net/ipv6/route.c
===================================================================
--- net-2.6.19.git.orig/net/ipv6/route.c
+++ net-2.6.19.git/net/ipv6/route.c
@@ -2040,9 +2040,7 @@ int inet6_rtm_getroute(struct sk_buff *i
 		goto out_free;
 	}
 
-	err = netlink_unicast(rtnl, skb, NETLINK_CB(in_skb).pid, MSG_DONTWAIT);
-	if (err > 0)
-		err = 0;
+	err = rtnl_unicast(skb, NETLINK_CB(in_skb).pid);
 out:
 	return err;
 out_free:
Index: net-2.6.19.git/net/sched/act_api.c
===================================================================
--- net-2.6.19.git.orig/net/sched/act_api.c
+++ net-2.6.19.git/net/sched/act_api.c
@@ -459,7 +459,6 @@ static int
 act_get_notify(u32 pid, struct nlmsghdr *n, struct tc_action *a, int event)
 {
 	struct sk_buff *skb;
-	int err = 0;
 
 	skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
 	if (!skb)
@@ -468,10 +467,8 @@ act_get_notify(u32 pid, struct nlmsghdr 
 		kfree_skb(skb);
 		return -EINVAL;
 	}
-	err = netlink_unicast(rtnl, skb, pid, MSG_DONTWAIT);
-	if (err > 0)
-		err = 0;
-	return err;
+
+	return rtnl_unicast(skb, pid);
 }
 
 static struct tc_action *

[PATCH 02/16] [NETLINK]: Add notification message sending interface

[Thomas Graf]

[-- Attachment #1: nlmsg_notify --]
[-- Type: text/plain, Size: 4418 bytes --]

Adds nlmsg_notify() implementing proper notification logic. The
message is multicasted to all listeners in the group. The
applications the requests orignates from can request a unicast
back report in which case said socket will be excluded from the
multicast to avoid duplicated notifications.

nlmsg_multicast() is extended to take allocation flags to
allow notification in atomic contexts.

Signed-off-by: Thomas Graf <tgraf@suug.ch>

Index: net-2.6.19.git/include/net/netlink.h
===================================================================
--- net-2.6.19.git.orig/include/net/netlink.h
+++ net-2.6.19.git/include/net/netlink.h
@@ -43,6 +43,7 @@
  * Message Sending:
  *   nlmsg_multicast()			multicast message to several groups
  *   nlmsg_unicast()			unicast a message to a single socket
+ *   nlmsg_notify()			send notification message
  *
  * Message Length Calculations:
  *   nlmsg_msg_size(payload)		length of message w/o padding
@@ -545,15 +546,16 @@ static inline void nlmsg_free(struct sk_
  * @skb: netlink message as socket buffer
  * @pid: own netlink pid 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 pid, unsigned int group)
+				  u32 pid, unsigned int group, gfp_t flags)
 {
 	int err;
 
 	NETLINK_CB(skb).dst_group = group;
 
-	err = netlink_broadcast(sk, skb, pid, group, GFP_KERNEL);
+	err = netlink_broadcast(sk, skb, pid, group, flags);
 	if (err > 0)
 		err = 0;
 
Index: net-2.6.19.git/net/netlink/af_netlink.c
===================================================================
--- net-2.6.19.git.orig/net/netlink/af_netlink.c
+++ net-2.6.19.git/net/netlink/af_netlink.c
@@ -1550,6 +1550,38 @@ void netlink_queue_skip(struct nlmsghdr 
 	skb_pull(skb, msglen);
 }
 
+/**
+ * nlmsg_notify - send a notification netlink message
+ * @sk: netlink socket to use
+ * @skb: notification message
+ * @pid: destination netlink pid for reports or 0
+ * @group: destination multicast group or 0
+ * @report: 1 to report back, 0 to disable
+ * @flags: allocation flags
+ */
+int nlmsg_notify(struct sock *sk, struct sk_buff *skb, u32 pid,
+		 unsigned int group, int report, gfp_t flags)
+{
+	int err = 0;
+
+	if (group) {
+		int exclude_pid = 0;
+
+		if (report) {
+			atomic_inc(&skb->users);
+			exclude_pid = pid;
+		}
+
+		/* errors reported via destination sk->sk_err */
+		nlmsg_multicast(sk, skb, exclude_pid, group, flags);
+	}
+
+	if (report)
+		err = nlmsg_unicast(sk, skb, pid);
+
+	return err;
+}
+
 #ifdef CONFIG_PROC_FS
 struct nl_seq_iter {
 	int link;
@@ -1804,4 +1836,4 @@ EXPORT_SYMBOL(netlink_set_err);
 EXPORT_SYMBOL(netlink_set_nonroot);
 EXPORT_SYMBOL(netlink_unicast);
 EXPORT_SYMBOL(netlink_unregister_notifier);
-
+EXPORT_SYMBOL(nlmsg_notify);
Index: net-2.6.19.git/include/net/genetlink.h
===================================================================
--- net-2.6.19.git.orig/include/net/genetlink.h
+++ net-2.6.19.git/include/net/genetlink.h
@@ -133,11 +133,12 @@ static inline int genlmsg_cancel(struct 
  * @skb: netlink message as socket buffer
  * @pid: own netlink pid to avoid sending to yourself
  * @group: multicast group id
+ * @flags: allocation flags
  */
 static inline int genlmsg_multicast(struct sk_buff *skb, u32 pid,
-				    unsigned int group)
+				    unsigned int group, gfp_t flags)
 {
-	return nlmsg_multicast(genl_sock, skb, pid, group);
+	return nlmsg_multicast(genl_sock, skb, pid, group, flags);
 }
 
 /**
Index: net-2.6.19.git/net/netlabel/netlabel_user.c
===================================================================
--- net-2.6.19.git.orig/net/netlabel/netlabel_user.c
+++ net-2.6.19.git/net/netlabel/netlabel_user.c
@@ -154,5 +154,5 @@ int netlbl_netlink_snd(struct sk_buff *s
  */
 int netlbl_netlink_snd_multicast(struct sk_buff *skb, u32 pid, u32 group)
 {
-	return genlmsg_multicast(skb, pid, group);
+	return genlmsg_multicast(skb, pid, group, GFP_KERNEL);
 }
Index: net-2.6.19.git/net/netlink/genetlink.c
===================================================================
--- net-2.6.19.git.orig/net/netlink/genetlink.c
+++ net-2.6.19.git/net/netlink/genetlink.c
@@ -510,7 +510,7 @@ static int genl_ctrl_event(int event, vo
 		if (IS_ERR(msg))
 			return PTR_ERR(msg);
 
-		genlmsg_multicast(msg, 0, GENL_ID_CTRL);
+		genlmsg_multicast(msg, 0, GENL_ID_CTRL, GFP_KERNEL);
 		break;
 	}
 

[PATCH 03/16] [RTNETLINK]: Add rtnetlink notification interface

[Thomas Graf]

[-- Attachment #1: rtnl_notify --]
[-- Type: text/plain, Size: 3548 bytes --]

Adds rtnl_notify() to send rtnetlink notification messages and
rtnl_set_sk_err() to report notification errors as socket
errors in order to indicate the need of a resync due to loss
of events.

nlmsg_report() is added to properly document the meaning of
NLM_F_ECHO.

Signed-off-by: Thomas Graf <tgraf@suug.ch>

Index: net-2.6.19.git/net/core/rtnetlink.c
===================================================================
--- net-2.6.19.git.orig/net/core/rtnetlink.c
+++ net-2.6.19.git/net/core/rtnetlink.c
@@ -171,6 +171,22 @@ int rtnl_unicast(struct sk_buff *skb, u3
 	return nlmsg_unicast(rtnl, skb, pid);
 }
 
+int rtnl_notify(struct sk_buff *skb, u32 pid, u32 group,
+		struct nlmsghdr *nlh, gfp_t flags)
+{
+	int report = 0;
+
+	if (nlh)
+		report = nlmsg_report(nlh);
+
+	return nlmsg_notify(rtnl, skb, pid, group, report, flags);
+}
+
+void rtnl_set_sk_err(u32 group, int error)
+{
+	netlink_set_err(rtnl, 0, group, error);
+}
+
 int rtnetlink_put_metrics(struct sk_buff *skb, u32 *metrics)
 {
 	struct rtattr *mx = (struct rtattr*)skb->tail;
@@ -829,3 +845,5 @@ EXPORT_SYMBOL(rtnl_lock);
 EXPORT_SYMBOL(rtnl_trylock);
 EXPORT_SYMBOL(rtnl_unlock);
 EXPORT_SYMBOL(rtnl_unicast);
+EXPORT_SYMBOL(rtnl_notify);
+EXPORT_SYMBOL(rtnl_set_sk_err);
Index: net-2.6.19.git/include/linux/rtnetlink.h
===================================================================
--- net-2.6.19.git.orig/include/linux/rtnetlink.h
+++ net-2.6.19.git/include/linux/rtnetlink.h
@@ -585,6 +585,9 @@ struct rtnetlink_link
 extern struct rtnetlink_link * rtnetlink_links[NPROTO];
 extern int rtnetlink_send(struct sk_buff *skb, u32 pid, u32 group, int echo);
 extern int rtnl_unicast(struct sk_buff *skb, u32 pid);
+extern int rtnl_notify(struct sk_buff *skb, u32 pid, u32 group,
+		       struct nlmsghdr *nlh, gfp_t flags);
+extern void rtnl_set_sk_err(u32 group, int error);
 extern int rtnetlink_put_metrics(struct sk_buff *skb, u32 *metrics);
 
 extern void __rta_fill(struct sk_buff *skb, int attrtype, int attrlen, const void *data);
Index: net-2.6.19.git/include/net/netlink.h
===================================================================
--- net-2.6.19.git.orig/include/net/netlink.h
+++ net-2.6.19.git/include/net/netlink.h
@@ -65,6 +65,9 @@
  *   nlmsg_validate()			validate netlink message incl. attrs
  *   nlmsg_for_each_attr()		loop over all attributes
  *
+ * Misc:
+ *   nlmsg_report()			report back to application?
+ *
  * ------------------------------------------------------------------------
  *                          Attributes Interface
  * ------------------------------------------------------------------------
@@ -194,6 +197,9 @@ extern void		netlink_run_queue(struct so
 						    struct nlmsghdr *, int *));
 extern void		netlink_queue_skip(struct nlmsghdr *nlh,
 					   struct sk_buff *skb);
+extern int		nlmsg_notify(struct sock *sk, struct sk_buff *skb,
+				     u32 pid, unsigned int group, int report,
+				     gfp_t flags);
 
 extern int		nla_validate(struct nlattr *head, int len, int maxtype,
 				     struct nla_policy *policy);
@@ -376,6 +382,17 @@ static inline int nlmsg_validate(struct 
 }
 
 /**
+ * nlmsg_report - need to report back to application?
+ * @nlh: netlink message header
+ *
+ * Returns 1 if a report back to the application is requested.
+ */
+static inline int nlmsg_report(struct nlmsghdr *nlh)
+{
+	return !!(nlh->nlmsg_flags & NLM_F_ECHO);
+}
+
+/**
  * nlmsg_for_each_attr - iterate over a stream of attributes
  * @pos: loop counter, set to current attribute
  * @nlh: netlink message header

[PATCH 04/16] [NET] fib_rules: Convert fib rule notification to use rtnl_notify()

[Thomas Graf]

[-- Attachment #1: rtnl_convert_fib_rules --]
[-- Type: text/plain, Size: 2350 bytes --]

Adds support for NLM_F_ECHO to simplify the process of identifying
inserted rules with an auto generated priority.

Signed-off-by: Thomas Graf <tgraf@suug.ch>

Index: net-2.6.19.git/net/core/fib_rules.c
===================================================================
--- net-2.6.19.git.orig/net/core/fib_rules.c
+++ net-2.6.19.git/net/core/fib_rules.c
@@ -18,7 +18,8 @@ static LIST_HEAD(rules_ops);
 static DEFINE_SPINLOCK(rules_mod_lock);
 
 static void notify_rule_change(int event, struct fib_rule *rule,
-			       struct fib_rules_ops *ops);
+			       struct fib_rules_ops *ops, struct nlmsghdr *nlh,
+			       u32 pid);
 
 static struct fib_rules_ops *lookup_rules_ops(int family)
 {
@@ -209,7 +210,7 @@ int fib_nl_newrule(struct sk_buff *skb, 
 	else
 		list_add_rcu(&rule->list, ops->rules_list);
 
-	notify_rule_change(RTM_NEWRULE, rule, ops);
+	notify_rule_change(RTM_NEWRULE, rule, ops, nlh, NETLINK_CB(skb).pid);
 	rules_ops_put(ops);
 	return 0;
 
@@ -266,7 +267,8 @@ int fib_nl_delrule(struct sk_buff *skb, 
 
 		list_del_rcu(&rule->list);
 		synchronize_rcu();
-		notify_rule_change(RTM_DELRULE, rule, ops);
+		notify_rule_change(RTM_DELRULE, rule, ops, nlh,
+				   NETLINK_CB(skb).pid);
 		fib_rule_put(rule);
 		rules_ops_put(ops);
 		return 0;
@@ -344,18 +346,26 @@ skip:
 EXPORT_SYMBOL_GPL(fib_rules_dump);
 
 static void notify_rule_change(int event, struct fib_rule *rule,
-			       struct fib_rules_ops *ops)
+			       struct fib_rules_ops *ops, struct nlmsghdr *nlh,
+			       u32 pid)
 {
-	int size = nlmsg_total_size(sizeof(struct fib_rule_hdr) + 128);
-	struct sk_buff *skb = alloc_skb(size, GFP_KERNEL);
+	struct sk_buff *skb;
+	int err = -ENOBUFS;
 
+	skb = nlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
 	if (skb == NULL)
-		netlink_set_err(rtnl, 0, ops->nlgroup, ENOBUFS);
-	else if (fib_nl_fill_rule(skb, rule, 0, 0, event, 0, ops) < 0) {
+		goto errout;
+
+	err = fib_nl_fill_rule(skb, rule, pid, nlh->nlmsg_seq, event, 0, ops);
+	if (err < 0) {
 		kfree_skb(skb);
-		netlink_set_err(rtnl, 0, ops->nlgroup, EINVAL);
-	} else
-		netlink_broadcast(rtnl, skb, 0, ops->nlgroup, GFP_KERNEL);
+		goto errout;
+	}
+
+	err = rtnl_notify(skb, pid, ops->nlgroup, nlh, GFP_KERNEL);
+errout:
+	if (err < 0)
+		rtnl_set_sk_err(ops->nlgroup, err);
 }
 
 static void attach_rules(struct list_head *rules, struct net_device *dev)

[PATCH 05/16] [NEIGH]: Convert neighbour notifications ot use rtnl_notify()

[Thomas Graf]

[-- Attachment #1: rtnl_convert_neigh --]
[-- Type: text/plain, Size: 1618 bytes --]

Signed-off-by: Thomas Graf <tgraf@suug.ch>

Index: net-2.6.19.git/net/core/neighbour.c
===================================================================
--- net-2.6.19.git.orig/net/core/neighbour.c
+++ net-2.6.19.git/net/core/neighbour.c
@@ -2406,36 +2406,35 @@ static struct file_operations neigh_stat
 #endif /* CONFIG_PROC_FS */
 
 #ifdef CONFIG_ARPD
-void neigh_app_ns(struct neighbour *n)
+static void __neigh_notify(struct neighbour *n, int type, int flags)
 {
 	struct sk_buff *skb;
+	int err = -ENOBUFS;
 
 	skb = nlmsg_new(NLMSG_GOODSIZE, GFP_ATOMIC);
 	if (skb == NULL)
-		return;
+		goto errout;
 
-	if (neigh_fill_info(skb, n, 0, 0, RTM_GETNEIGH, NLM_F_REQUEST) <= 0)
+	err = neigh_fill_info(skb, n, 0, 0, type, flags);
+	if (err < 0) {
 		kfree_skb(skb);
-	else {
-		NETLINK_CB(skb).dst_group  = RTNLGRP_NEIGH;
-		netlink_broadcast(rtnl, skb, 0, RTNLGRP_NEIGH, GFP_ATOMIC);
+		goto errout;
 	}
+
+	err = rtnl_notify(skb, 0, RTNLGRP_NEIGH, NULL, GFP_ATOMIC);
+errout:
+	if (err < 0)
+		rtnl_set_sk_err(RTNLGRP_NEIGH, err);
 }
 
-static void neigh_app_notify(struct neighbour *n)
+void neigh_app_ns(struct neighbour *n)
 {
-	struct sk_buff *skb;
-
-	skb = nlmsg_new(NLMSG_GOODSIZE, GFP_ATOMIC);
-	if (skb == NULL)
-		return;
+	__neigh_notify(n, RTM_GETNEIGH, NLM_F_REQUEST);
+}
 
-	if (neigh_fill_info(skb, n, 0, 0, RTM_NEWNEIGH, 0) <= 0)
-		kfree_skb(skb);
-	else {
-		NETLINK_CB(skb).dst_group  = RTNLGRP_NEIGH;
-		netlink_broadcast(rtnl, skb, 0, RTNLGRP_NEIGH, GFP_ATOMIC);
-	}
+static void neigh_app_notify(struct neighbour *n)
+{
+	__neigh_notify(n, RTM_NEWNEIGH, 0);
 }
 
 #endif /* CONFIG_ARPD */

[PATCH 06/16] [DECNET]: Convert DECnet notifications to use rtnl_notify()

[Thomas Graf]

[-- Attachment #1: rtnl_convert_decnet --]
[-- Type: text/plain, Size: 2912 bytes --]

Signed-off-by: Thomas Graf <tgraf@suug.ch>

Index: net-2.6.19.git/net/decnet/dn_dev.c
===================================================================
--- net-2.6.19.git.orig/net/decnet/dn_dev.c
+++ net-2.6.19.git/net/decnet/dn_dev.c
@@ -746,20 +746,23 @@ rtattr_failure:
 static void rtmsg_ifa(int event, struct dn_ifaddr *ifa)
 {
 	struct sk_buff *skb;
-	int size = NLMSG_SPACE(sizeof(struct ifaddrmsg)+128);
+	int payload = sizeof(struct ifaddrmsg) + 128;
+	int err = -ENOBUFS;
 
-	skb = alloc_skb(size, GFP_KERNEL);
-	if (!skb) {
-		netlink_set_err(rtnl, 0, RTNLGRP_DECnet_IFADDR, ENOBUFS);
-		return;
-	}
-	if (dn_dev_fill_ifaddr(skb, ifa, 0, 0, event, 0) < 0) {
+	skb = alloc_skb(nlmsg_total_size(payload), GFP_KERNEL);
+	if (skb == NULL)
+		goto errout;
+
+	err = dn_dev_fill_ifaddr(skb, ifa, 0, 0, event, 0);
+	if (err < 0) {
 		kfree_skb(skb);
-		netlink_set_err(rtnl, 0, RTNLGRP_DECnet_IFADDR, EINVAL);
-		return;
+		goto errout;
 	}
-	NETLINK_CB(skb).dst_group = RTNLGRP_DECnet_IFADDR;
-	netlink_broadcast(rtnl, skb, 0, RTNLGRP_DECnet_IFADDR, GFP_KERNEL);
+
+	err = rtnl_notify(skb, 0, RTNLGRP_DECnet_IFADDR, NULL, GFP_KERNEL);
+errout:
+	if (err < 0)
+		rtnl_set_sk_err(RTNLGRP_DECnet_IFADDR, err);
 }
 
 static int dn_dev_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
Index: net-2.6.19.git/net/decnet/dn_table.c
===================================================================
--- net-2.6.19.git.orig/net/decnet/dn_table.c
+++ net-2.6.19.git/net/decnet/dn_table.c
@@ -333,24 +333,24 @@ static void dn_rtmsg_fib(int event, stru
 {
         struct sk_buff *skb;
         u32 pid = req ? req->pid : 0;
-        int size = NLMSG_SPACE(sizeof(struct rtmsg) + 256);
+	int err = -ENOBUFS;
 
-        skb = alloc_skb(size, GFP_KERNEL);
-        if (!skb)
-                return;
-
-        if (dn_fib_dump_info(skb, pid, nlh->nlmsg_seq, event, tb_id, 
-                                f->fn_type, f->fn_scope, &f->fn_key, z, 
-                                DN_FIB_INFO(f), 0) < 0) {
+        skb = nlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
+        if (skb == NULL)
+		goto errout;
+
+        err = dn_fib_dump_info(skb, pid, nlh->nlmsg_seq, event, tb_id,
+			       f->fn_type, f->fn_scope, &f->fn_key, z,
+			       DN_FIB_INFO(f), 0);
+	if (err < 0) {
                 kfree_skb(skb);
-                return;
+		goto errout;
         }
-        NETLINK_CB(skb).dst_group = RTNLGRP_DECnet_ROUTE;
-        if (nlh->nlmsg_flags & NLM_F_ECHO)
-                atomic_inc(&skb->users);
-        netlink_broadcast(rtnl, skb, pid, RTNLGRP_DECnet_ROUTE, GFP_KERNEL);
-        if (nlh->nlmsg_flags & NLM_F_ECHO)
-                netlink_unicast(rtnl, skb, pid, MSG_DONTWAIT);
+
+	err = rtnl_notify(skb, pid, RTNLGRP_DECnet_ROUTE, nlh, GFP_KERNEL);
+errout:
+	if (err < 0)
+		rtnl_set_sk_err(RTNLGRP_DECnet_ROUTE, err);
 }
 
 static __inline__ int dn_hash_dump_bucket(struct sk_buff *skb, 

[PATCH 07/16] [IPv4] address: Convert address notification to use rtnl_notify()

[Thomas Graf]

[-- Attachment #1: rtnl_convert_ip4_addr --]
[-- Type: text/plain, Size: 4536 bytes --]

Adds support for NLM_F_ECHO allowing applications to easly
see which address have been deleted, added, or promoted.

Signed-off-by: Thomas Graf <tgraf@suug.ch>

Index: net-2.6.19.git/net/ipv4/devinet.c
===================================================================
--- net-2.6.19.git.orig/net/ipv4/devinet.c
+++ net-2.6.19.git/net/ipv4/devinet.c
@@ -88,7 +88,7 @@ static struct nla_policy ifa_ipv4_policy
 	[IFA_LABEL]     	= { .type = NLA_STRING },
 };
 
-static void rtmsg_ifa(int event, struct in_ifaddr *);
+static void rtmsg_ifa(int event, struct in_ifaddr *, struct nlmsghdr *, u32);
 
 static BLOCKING_NOTIFIER_HEAD(inetaddr_chain);
 static void inet_del_ifa(struct in_device *in_dev, struct in_ifaddr **ifap,
@@ -239,8 +239,8 @@ int inet_addr_onlink(struct in_device *i
 	return 0;
 }
 
-static void inet_del_ifa(struct in_device *in_dev, struct in_ifaddr **ifap,
-			 int destroy)
+static void __inet_del_ifa(struct in_device *in_dev, struct in_ifaddr **ifap,
+			 int destroy, struct nlmsghdr *nlh, u32 pid)
 {
 	struct in_ifaddr *promote = NULL;
 	struct in_ifaddr *ifa, *ifa1 = *ifap;
@@ -273,7 +273,7 @@ static void inet_del_ifa(struct in_devic
 			if (!do_promote) {
 				*ifap1 = ifa->ifa_next;
 
-				rtmsg_ifa(RTM_DELADDR, ifa);
+				rtmsg_ifa(RTM_DELADDR, ifa, nlh, pid);
 				blocking_notifier_call_chain(&inetaddr_chain,
 						NETDEV_DOWN, ifa);
 				inet_free_ifa(ifa);
@@ -298,7 +298,7 @@ static void inet_del_ifa(struct in_devic
 	   is valid, it will try to restore deleted routes... Grr.
 	   So that, this order is correct.
 	 */
-	rtmsg_ifa(RTM_DELADDR, ifa1);
+	rtmsg_ifa(RTM_DELADDR, ifa1, nlh, pid);
 	blocking_notifier_call_chain(&inetaddr_chain, NETDEV_DOWN, ifa1);
 
 	if (promote) {
@@ -310,7 +310,7 @@ static void inet_del_ifa(struct in_devic
 		}
 
 		promote->ifa_flags &= ~IFA_F_SECONDARY;
-		rtmsg_ifa(RTM_NEWADDR, promote);
+		rtmsg_ifa(RTM_NEWADDR, promote, nlh, pid);
 		blocking_notifier_call_chain(&inetaddr_chain,
 				NETDEV_UP, promote);
 		for (ifa = promote->ifa_next; ifa; ifa = ifa->ifa_next) {
@@ -329,7 +329,14 @@ static void inet_del_ifa(struct in_devic
 	}
 }
 
-static int inet_insert_ifa(struct in_ifaddr *ifa)
+static void inet_del_ifa(struct in_device *in_dev, struct in_ifaddr **ifap,
+			 int destroy)
+{
+	__inet_del_ifa(in_dev, ifap, destroy, NULL, 0);
+}
+
+static int __inet_insert_ifa(struct in_ifaddr *ifa, struct nlmsghdr *nlh,
+			     u32 pid)
 {
 	struct in_device *in_dev = ifa->ifa_dev;
 	struct in_ifaddr *ifa1, **ifap, **last_primary;
@@ -374,12 +381,17 @@ static int inet_insert_ifa(struct in_ifa
 	/* Send message first, then call notifier.
 	   Notifier will trigger FIB update, so that
 	   listeners of netlink will know about new ifaddr */
-	rtmsg_ifa(RTM_NEWADDR, ifa);
+	rtmsg_ifa(RTM_NEWADDR, ifa, nlh, pid);
 	blocking_notifier_call_chain(&inetaddr_chain, NETDEV_UP, ifa);
 
 	return 0;
 }
 
+static int inet_insert_ifa(struct in_ifaddr *ifa)
+{
+	return __inet_insert_ifa(ifa, NULL, 0);
+}
+
 static int inet_set_ifa(struct net_device *dev, struct in_ifaddr *ifa)
 {
 	struct in_device *in_dev = __in_dev_get_rtnl(dev);
@@ -466,7 +478,7 @@ static int inet_rtm_deladdr(struct sk_bu
 		    !inet_ifa_match(nla_get_u32(tb[IFA_ADDRESS]), ifa)))
 			continue;
 
-		inet_del_ifa(in_dev, ifap, 1);
+		__inet_del_ifa(in_dev, ifap, 1, nlh, NETLINK_CB(skb).pid);
 		return 0;
 	}
 
@@ -558,7 +570,7 @@ static int inet_rtm_newaddr(struct sk_bu
 	if (IS_ERR(ifa))
 		return PTR_ERR(ifa);
 
-	return inet_insert_ifa(ifa);
+	return __inet_insert_ifa(ifa, nlh, NETLINK_CB(skb).pid);
 }
 
 /*
@@ -1189,18 +1201,27 @@ done:
 	return skb->len;
 }
 
-static void rtmsg_ifa(int event, struct in_ifaddr* ifa)
+static void rtmsg_ifa(int event, struct in_ifaddr* ifa, struct nlmsghdr *nlh,
+		      u32 pid)
 {
 	struct sk_buff *skb;
+	u32 seq = nlh ? nlh->nlmsg_seq : 0;
+	int err = -ENOBUFS;
 
 	skb = nlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
 	if (skb == NULL)
-		netlink_set_err(rtnl, 0, RTNLGRP_IPV4_IFADDR, ENOBUFS);
-	else if (inet_fill_ifaddr(skb, ifa, 0, 0, event, 0) < 0) {
+		goto errout;
+
+	err = inet_fill_ifaddr(skb, ifa, pid, seq, event, 0);
+	if (err < 0) {
 		kfree_skb(skb);
-		netlink_set_err(rtnl, 0, RTNLGRP_IPV4_IFADDR, EINVAL);
-	} else
-		netlink_broadcast(rtnl, skb, 0, RTNLGRP_IPV4_IFADDR, GFP_KERNEL);
+		goto errout;
+	}
+
+	err = rtnl_notify(skb, pid, RTNLGRP_IPV4_IFADDR, nlh, GFP_KERNEL);
+errout:
+	if (err < 0)
+		rtnl_set_sk_err(RTNLGRP_IPV4_IFADDR, err);
 }
 
 static struct rtnetlink_link inet_rtnetlink_table[RTM_NR_MSGTYPES] = {

[PATCH 08/16] [IPv4] route: Convert route notifications to use rtnl_notify()

[Thomas Graf]

[-- Attachment #1: rtnl_convert_ip4_route --]
[-- Type: text/plain, Size: 1797 bytes --]

Signed-off-by: Thomas Graf <tgraf@suug.ch>

Index: net-2.6.19.git/net/ipv4/fib_semantics.c
===================================================================
--- net-2.6.19.git.orig/net/ipv4/fib_semantics.c
+++ net-2.6.19.git/net/ipv4/fib_semantics.c
@@ -33,7 +33,6 @@
 #include <linux/if_arp.h>
 #include <linux/proc_fs.h>
 #include <linux/skbuff.h>
-#include <linux/netlink.h>
 #include <linux/init.h>
 
 #include <net/arp.h>
@@ -44,6 +43,7 @@
 #include <net/sock.h>
 #include <net/ip_fib.h>
 #include <net/ip_mp_alg.h>
+#include <net/netlink.h>
 
 #include "fib_lookup.h"
 
@@ -278,25 +278,25 @@ void rtmsg_fib(int event, u32 key, struc
 {
 	struct sk_buff *skb;
 	u32 pid = req ? req->pid : n->nlmsg_pid;
-	int size = NLMSG_SPACE(sizeof(struct rtmsg)+256);
+	int payload = sizeof(struct rtmsg) + 256;
+	int err = -ENOBUFS;
 
-	skb = alloc_skb(size, GFP_KERNEL);
-	if (!skb)
-		return;
-
-	if (fib_dump_info(skb, pid, n->nlmsg_seq, event, tb_id,
-			  fa->fa_type, fa->fa_scope, &key, z,
-			  fa->fa_tos,
-			  fa->fa_info, 0) < 0) {
+	skb = nlmsg_new(nlmsg_total_size(payload), GFP_KERNEL);
+	if (skb == NULL)
+		goto errout;
+
+	err = fib_dump_info(skb, pid, n->nlmsg_seq, event, tb_id,
+			    fa->fa_type, fa->fa_scope, &key, z, fa->fa_tos,
+			    fa->fa_info, 0);
+	if (err < 0) {
 		kfree_skb(skb);
-		return;
+		goto errout;
 	}
-	NETLINK_CB(skb).dst_group = RTNLGRP_IPV4_ROUTE;
-	if (n->nlmsg_flags&NLM_F_ECHO)
-		atomic_inc(&skb->users);
-	netlink_broadcast(rtnl, skb, pid, RTNLGRP_IPV4_ROUTE, GFP_KERNEL);
-	if (n->nlmsg_flags&NLM_F_ECHO)
-		netlink_unicast(rtnl, skb, pid, MSG_DONTWAIT);
+
+	err = rtnl_notify(skb, pid, RTNLGRP_IPV4_ROUTE, n, GFP_KERNEL);
+errout:
+	if (err < 0)
+		rtnl_set_sk_err(RTNLGRP_IPV4_ROUTE, err);
 }
 
 /* Return the first fib alias matching TOS with

[PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[Thomas Graf]

[-- Attachment #1: rtnl_convert_ip6_addr --]
[-- Type: text/plain, Size: 1555 bytes --]

Fixes a wrong use of current->pid as netlink pid.

Signed-off-by: Thomas Graf <tgraf@suug.ch>

Index: net-2.6.19.git/net/ipv6/addrconf.c
===================================================================
--- net-2.6.19.git.orig/net/ipv6/addrconf.c
+++ net-2.6.19.git/net/ipv6/addrconf.c
@@ -73,6 +73,7 @@
 #include <net/addrconf.h>
 #include <net/tcp.h>
 #include <net/ip.h>
+#include <net/netlink.h>
 #include <linux/if_tunnel.h>
 #include <linux/rtnetlink.h>
 
@@ -3280,20 +3281,23 @@ out_free:
 static void inet6_ifa_notify(int event, struct inet6_ifaddr *ifa)
 {
 	struct sk_buff *skb;
-	int size = NLMSG_SPACE(sizeof(struct ifaddrmsg) + INET6_IFADDR_RTA_SPACE);
+	int payload = sizeof(struct ifaddrmsg) + INET6_IFADDR_RTA_SPACE;
+	int err = -ENOBUFS;
 
-	skb = alloc_skb(size, GFP_ATOMIC);
-	if (!skb) {
-		netlink_set_err(rtnl, 0, RTNLGRP_IPV6_IFADDR, ENOBUFS);
-		return;
-	}
-	if (inet6_fill_ifaddr(skb, ifa, current->pid, 0, event, 0) < 0) {
+	skb = nlmsg_new(nlmsg_total_size(payload), GFP_ATOMIC);
+	if (skb == NULL)
+		goto errout;
+
+	err = inet6_fill_ifaddr(skb, ifa, 0, 0, event, 0);
+	if (err < 0) {
 		kfree_skb(skb);
-		netlink_set_err(rtnl, 0, RTNLGRP_IPV6_IFADDR, EINVAL);
-		return;
+		goto errout;
 	}
-	NETLINK_CB(skb).dst_group = RTNLGRP_IPV6_IFADDR;
-	netlink_broadcast(rtnl, skb, 0, RTNLGRP_IPV6_IFADDR, GFP_ATOMIC);
+
+	err = rtnl_notify(skb, 0, RTNLGRP_IPV6_IFADDR, NULL, GFP_ATOMIC);
+errout:
+	if (err < 0)
+		rtnl_set_sk_err(RTNLGRP_IPV6_IFADDR, err);
 }
 
 static void inline ipv6_store_devconf(struct ipv6_devconf *cnf,

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[jamal]

On Mon, 2006-14-08 at 00:00 +0200, Thomas Graf wrote:
> plain text document attachment (rtnl_convert_ip6_addr)
> Fixes a wrong use of current->pid as netlink pid.

If i am not mistaken:
Some of these removals of current->pid will affect users such as quagga,
zebra, vrrpd etc. If those specific users are not affected, please
ignore the rest of my comments.

I realize mr. Kuznetsov submitted at least one patch in the past to do
something similar to this. 
IMO, I believe there is a strong case that can be made for events that
were caused by non-netlink users such as ioctls that could at least be
multicast with current->pid. In other words an exception case for only
this scenario. In such a case there is no unicast netlink message. 
It would also be acceptable if the quagga etc folks could meet their
goals some other way.

cheers,
jamal

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[jamal]

CCing Hasso Tepper <hasso@estpak.ee>

Hasso, please comment on this thread (apologies, I forgot the other
guy's name; so if you can CC all the necessary suspects to comment, it
would help).

cheers,
jamal

On Mon, 2006-14-08 at 19:43 -0400, jamal wrote:
> On Mon, 2006-14-08 at 00:00 +0200, Thomas Graf wrote:
> > plain text document attachment (rtnl_convert_ip6_addr)
> > Fixes a wrong use of current->pid as netlink pid.
> 
> If i am not mistaken:
> Some of these removals of current->pid will affect users such as quagga,
> zebra, vrrpd etc. If those specific users are not affected, please
> ignore the rest of my comments.
> 
> I realize mr. Kuznetsov submitted at least one patch in the past to do
> something similar to this. 
> IMO, I believe there is a strong case that can be made for events that
> were caused by non-netlink users such as ioctls that could at least be
> multicast with current->pid. In other words an exception case for only
> this scenario. In such a case there is no unicast netlink message. 
> It would also be acceptable if the quagga etc folks could meet their
> goals some other way.
> 
> cheers,
> jamal
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[Hasso Tepper]

jamal wrote:
> CCing Hasso Tepper <hasso@estpak.ee>
>
> Hasso, please comment on this thread (apologies, I forgot the other
> guy's name; so if you can CC all the necessary suspects to comment, it
> would help).

I fully agree with Herbert Xu:

> The pid field in the netlink header should be treated as an opaque 
> value.  Any attempt to interpret it as the process ID is doomed to 
> failure. 

Zebra, Quagga and Xorp treat the pid field in the netlink header as 
unicast address of the netlink socket. As far as I know, that was the 
original idea:

http://marc.theaimsgroup.com/?l=linux-netdev&m=113943269432327&w=2

So, for Zebra, Quagga and Xorp users removals of current->pid mean just 
closing potential races. You can't quarantee that ip/route/ifconfig will 
not have the pid equal to the address of the netlink socket 
Zebra/Quagga/Xorp owns.


regards,

-- 
Hasso

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[jamal]

On Tue, 2006-15-08 at 10:20 +0300, Hasso Tepper wrote:

> Zebra, Quagga and Xorp treat the pid field in the netlink header as 
> unicast address of the netlink socket. As far as I know, that was the 
> original idea:
> 
> http://marc.theaimsgroup.com/?l=linux-netdev&m=113943269432327&w=2
> 
> So, for Zebra, Quagga and Xorp users removals of current->pid mean just 
> closing potential races. You can't quarantee that ip/route/ifconfig will 
> not have the pid equal to the address of the netlink socket 
> Zebra/Quagga/Xorp owns.

We've had this discussion before, havent we? ;-> Why do people like
Alexey remember but i dont ;-> In any case if you are happy, I am happy
too and i hope this settles it then ;->

cheers,
jamal

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[Thomas Graf]

* jamal <hadi@cyberus.ca> 2006-08-14 19:43
> On Mon, 2006-14-08 at 00:00 +0200, Thomas Graf wrote:
> > plain text document attachment (rtnl_convert_ip6_addr)
> > Fixes a wrong use of current->pid as netlink pid.
> 
> If i am not mistaken:
> Some of these removals of current->pid will affect users such as quagga,
> zebra, vrrpd etc. If those specific users are not affected, please
> ignore the rest of my comments.

I'm aware of that. The next patchset cleaning up fib6 and ifa6 will
provide means to pass on the required netlink information to these
notification functions.

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[jamal]

On Tue, 2006-15-08 at 02:07 +0200, Thomas Graf wrote:
> * jamal <hadi@cyberus.ca> 2006-08-14 19:43

> > If i am not mistaken:
> > Some of these removals of current->pid will affect users such as quagga,
> > zebra, vrrpd etc. If those specific users are not affected, please
> > ignore the rest of my comments.
> 
> I'm aware of that. The next patchset cleaning up fib6 and ifa6 will
> provide means to pass on the required netlink information to these
> notification functions.

Good nuf. Thanks Thomas.

cheers,
jamal

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[Alexey Kuznetsov]

Hello!

> Some of these removals of current->pid will affect users such as quagga,
> zebra, vrrpd etc.

If they survived cleanup in IPv4, they definitely will not feel cleanup
in IPv6.

Thomas does great work, Jamal, do not worry. :-)


> IMO, I believe there is a strong case that can be made for events that
> were caused by non-netlink users such as ioctls that could at least be
> multicast with current->pid.

Highly not desired.


> It would also be acceptable if the quagga etc folks could meet their
> goals some other way.

To all that I remember, that past discussion essentially closed the question.

Alexey

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[Herbert Xu]

Alexey Kuznetsov <kuznet@ms2.inr.ac.ru> wrote:
> 
>> IMO, I believe there is a strong case that can be made for events that
>> were caused by non-netlink users such as ioctls that could at least be
>> multicast with current->pid.
> 
> Highly not desired.

Agreed.  The pid field in the netlink header should be treated as an
opaque value.  Any attempt to interpret it as the process ID is doomed
to failure.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[jamal]

On Tue, 2006-15-08 at 10:56 +1000, Herbert Xu wrote:

> Agreed.  The pid field in the netlink header should be treated as an
> opaque value.  Any attempt to interpret it as the process ID is doomed
> to failure.


Not necessarily as a processid ("PID" is a really bad noun in that
sense); but rather as something meaningful of interpretation in regards
to the real origin of the executed change. 
The concept of "whodunnit" is invaluable. 
And a processid tends to be useful when nothing else is there to
identify the originator. Just saying "it is the kernel" (PID=0) when the
kernel just acted as a proxy of some user space app, is not useful all
the times, IMO.
[Routes, but not other functional blocks in rtnetlink, actually have a
field (called protocol) that says who added them]. Thats all the quagga
and other folks were looking for in cases where it was ambigous. 

cheers,
jamal

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[jamal]

On Mon, 2006-14-08 at 22:46 -0400, jamal wrote:
> On Tue, 2006-15-08 at 10:56 +1000, Herbert Xu wrote:
> Thats all the quagga
> and other folks were looking for in cases where it was ambigous. 
> 

Just to be clear, when Alexey says "that past discussion essentially
closed the question." he is correct and i did not, hopefully, insuniate
otherwise. The question i posed above is still valid though ;->

cheers,
jamal

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[Thomas Graf]

* Alexey Kuznetsov <kuznet@ms2.inr.ac.ru> 2006-08-15 04:36
> > Some of these removals of current->pid will affect users such as quagga,
> > zebra, vrrpd etc.
> 
> If they survived cleanup in IPv4, they definitely will not feel cleanup
> in IPv6.

That was exactly my thought, the notifications in question provide
a pid of 0 for their IPv4 counterparts therefore I considered it
safe.

However, I changed IPv4 addresses to provide the pid and support
NLM_F_ECHO and the same will follow for IPv6 address notifications
which will mean that quagga sees a different set of IPv4 address
notifications. It would be a bug on their side if that would cause
failure though.

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[Hasso Tepper]

Thomas Graf wrote:
> However, I changed IPv4 addresses to provide the pid and support
> NLM_F_ECHO and the same will follow for IPv6 address notifications
> which will mean that quagga sees a different set of IPv4 address
> notifications.

Can you explain what exactly is changed?


-- 
Hasso Tepper
Elion Enterprises Ltd. [AS3249]
Data Communication Network Administrator

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[Thomas Graf]

* Hasso Tepper <hasso@estpak.ee> 2006-08-15 15:08
> Thomas Graf wrote:
> > However, I changed IPv4 addresses to provide the pid and support
> > NLM_F_ECHO and the same will follow for IPv6 address notifications
> > which will mean that quagga sees a different set of IPv4 address
> > notifications.
> 
> Can you explain what exactly is changed?

IPv4 address notifications originating from a netlink request now
carry the netlink pid of the request instead of 0.

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[Herbert Xu]

Thomas Graf <tgraf@suug.ch> wrote:
>
> IPv4 address notifications originating from a netlink request now
> carry the netlink pid of the request instead of 0.

I'm not comfortable with that change since it implies the message
originated from a user-space process.

The netlink header pid is really akin to sadb_msg_pid from RFC 2367.
IMHO it should always be zero if the kernel is the originator of the
message.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[Herbert Xu]

On Wed, Aug 16, 2006 at 12:58:28PM +1000, Herbert Xu wrote:
> 
> I'm not comfortable with that change since it implies the message
> originated from a user-space process.
> 
> The netlink header pid is really akin to sadb_msg_pid from RFC 2367.
> IMHO it should always be zero if the kernel is the originator of the
> message.

Actually I think this has the potential to break applications that
ignore any messages with the original pid set to a non-zero value.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[Thomas Graf]

* Herbert Xu <herbert@gondor.apana.org.au> 2006-08-16 12:58
> I'm not comfortable with that change since it implies the message
> originated from a user-space process.
> 
> The netlink header pid is really akin to sadb_msg_pid from RFC 2367.
> IMHO it should always be zero if the kernel is the originator of the
> message.

All route and tc notifications already use the pid so applications
can decide whether the event was caused by them. A notification
is a reply to a request so it doesn't even violate RFC 2367.

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[jamal]

On Wed, 2006-16-08 at 12:58 +0200, Thomas Graf wrote:
> * Herbert Xu <herbert@gondor.apana.org.au> 2006-08-16 12:58
> > I'm not comfortable with that change since it implies the message
> > originated from a user-space process.
> > 
> > The netlink header pid is really akin to sadb_msg_pid from RFC 2367.
> > IMHO it should always be zero if the kernel is the originator of the
> > message.
> 
> All route and tc notifications already use the pid so applications
> can decide whether the event was caused by them. A notification
> is a reply to a request so it doesn't even violate RFC 2367.

I would agree with Thomas on this. Regardless, I dont think that 2367 is
really a glorified reference (that thing needs so much updating it is
not funny).

cheers,
jamal

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[Herbert Xu]

Hi Thomas:

On Wed, Aug 16, 2006 at 12:58:56PM +0200, Thomas Graf wrote:
> 
> All route and tc notifications already use the pid so applications
> can decide whether the event was caused by them. A notification
> is a reply to a request so it doesn't even violate RFC 2367.

Actually most IPv4 notifications *do* set the pid to zero which is
the right thing to do for kernel-generated messages.

You're right though that the IPv6 notification modified by this patch
does set the pid to the netlink originator.  Looking back in history
it seems that this behaviour was only introduced last year to a subset
of notifications.

This inconsistency is very bad.  IMHO this change (made last year)
should be reverted so that all kernel generated (broadcast) notifications
have the originator set to zero to match the source address of the
message.

Any notification that sets the netlink pid to current->pid is
*completely* bogus.  Let me repeat this, the netlink pid is not
a process ID.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[Herbert Xu]

On Wed, Aug 16, 2006 at 09:12:40PM +1000, herbert wrote:
> 
> Any notification that sets the netlink pid to current->pid is
> *completely* bogus.  Let me repeat this, the netlink pid is not
> a process ID.

BTW, I'm not having a go at either Thomas or Jamal.  You guys
are oo the same side for once :).

I honestly believe that we have a misunderstanding here which needs
to be sorted out.  It gets worse because that misunderstanding has
made it into the manpages package which only causes more confusion.

So let's step back a bit and think about where does this pid really
come from.  The field in question is nlmsg_pid.  Its primary purpose
is to identify unicast transactions along with the field nlmsg_seq.
It was not designed to identify the origin of a broadcast kernel
notification to a third party.

For this purpose, the value of nlmsg_pid is set to the address of
the destination socket for a particular unicast message (also known
as the pid).

That pid in turn has only a vague connection with the process ID
of the process owning the socket.  For practical purposes, we
should not treat it as a process ID it can easily be claimed by
another process (think socket + bind + fork).

For a broadcast notification, the nlmsg_pid field is meaningless
because the nlmsg_seq field is also meaningless.  I'm not denying
that it wouldn't be useful to have the originator's socket address
in there.  What I'm saying is that it's the wrong place to put
that information.

In any case, putting current->pid in this field is definitely
a bad idea because it only encourages people to confuse the
netlink pid with the process ID which can lead to security
problems later on.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[jamal]

On Wed, 2006-16-08 at 21:39 +1000, Herbert Xu wrote:

> So let's step back a bit and think about where does this pid really
> come from.  The field in question is nlmsg_pid.  Its primary purpose
> is to identify unicast transactions along with the field nlmsg_seq.
> It was not designed to identify the origin of a broadcast kernel
> notification to a third party.

There are quiet a few things that netlink design intent was not
intending to solve that became needed over time. This being one IMHO.
Design intent and eventual (sometimes creative) use occasionally create
an impedance ;-> Evolution is the only description i can think of.

> For this purpose, the value of nlmsg_pid is set to the address of
> the destination socket for a particular unicast message (also known
> as the pid).

Since we are talking history:
The idea of it being a destination socket _was not_ design intent. It
was evolution. I recall James Morris actually to be the first person
whining about this ambiguity when coding up nfqueue. I cant remember who
fixed it (I am inclined to think it was you;->)
 
> That pid in turn has only a vague connection with the process ID
> of the process owning the socket.  For practical purposes, we
> should not treat it as a process ID it can easily be claimed by
> another process (think socket + bind + fork).

If you want to be complete the kernel should "fix" the pid in
netlink::sendmsg().

> For a broadcast notification, the nlmsg_pid field is meaningless
> because the nlmsg_seq field is also meaningless.  

nlmsg_seq is meaningless; "seq" is again a bad noun. It should be
"cookie".

> I'm not denying
> that it wouldn't be useful to have the originator's socket address
> in there.  What I'm saying is that it's the wrong place to put
> that information.
>
> In any case, putting current->pid in this field is definitely
> a bad idea because it only encourages people to confuse the
> netlink pid with the process ID which can lead to security
> problems later on.

current->pid i think is coming out to be a bad idea. Thomas' patches
revert it out. Again this has everything to do with the original idea
what maps to pid now changing to socketid.

What do you think of the idea of infact rewriting the pid to be that of
the socket id?

cheers,
jamal

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[Herbert Xu]

On Wed, Aug 16, 2006 at 08:04:24AM -0400, jamal wrote:
> 
> What do you think of the idea of infact rewriting the pid to be that of
> the socket id?

Rewriting it with the netlink socket address? That's fine by me as
long as there is a clear 1-to-1 relationship between the request
and the notification.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[Thomas Graf]

* jamal <hadi@cyberus.ca> 2006-08-16 08:04
> current->pid i think is coming out to be a bad idea. Thomas' patches
> revert it out. Again this has everything to do with the original idea
> what maps to pid now changing to socketid.

It probably developed from autobind using current->tid.

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[jamal]

On Wed, 2006-16-08 at 14:08 +0200, Thomas Graf wrote:
> * jamal <hadi@cyberus.ca> 2006-08-16 08:04
> > current->pid i think is coming out to be a bad idea. Thomas' patches
> > revert it out. Again this has everything to do with the original idea
> > what maps to pid now changing to socketid.
> 
> It probably developed from autobind using current->tid.

In one conversation with Alexey he told me there was some inspiration
from pfkey in the semantics of it i.e processid. Obviously with many
sockets on the same process etc, that assumption is no longer valid. 

On Wed, 2006-16-08 at 22:08 +1000, Herbert Xu wrote: 
> On Wed, Aug 16, 2006 at 08:04:24AM -0400, jamal wrote:
> > 
> > What do you think of the idea of infact rewriting the pid to be that of
> > the socket id?
> 
> Rewriting it with the netlink socket address? That's fine by me as
> long as there is a clear 1-to-1 relationship between the request
> and the notification.

you would have to call getpeername() to get a correct 1-1 mapping as is 
today when in doubt.
What i was suggesting is notifications using the pid that would id the socket and
would therefore require a getpeername() which identify the real socket it came 
from; if you are fine with what Thomas is doing, then this unnecessary since i was
suggesting it as a compromise for consistency you pointed was lacking.

cheers,
jamal

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[Alexey Kuznetsov]

Hello!

> In one conversation with Alexey he told me there was some inspiration
> from pfkey in the semantics of it i.e processid.

Inspiration, but not a copy. :-)

Unlike pfkeyv2 it uses addressing usual for networking i.e.
struct sockaddr_nl.

Alexey

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[jamal]

On Wed, 2006-16-08 at 17:04 +0400, Alexey Kuznetsov wrote:
> Hello!
> 
> > In one conversation with Alexey he told me there was some inspiration
> > from pfkey in the semantics of it i.e processid.
> 
> Inspiration, but not a copy. :-)

Oh, absolutely. Netlink is way superior. I should have said perspiration
instead of inspiration;-> Calling inspiration was being polite - it is
as being polite as saying i was being economical with the truth[1] ;->

> Unlike pfkeyv2 it uses addressing usual for networking i.e.
> struct sockaddr_nl.

I think this needs to be captured somewhere. I dont know who is
maintaining the man pages these days.

cheers,
jamal

[1] A term i learnt from some British guy. They have ways with words
those Brits.

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[Thomas Graf]

* Herbert Xu <herbert@gondor.apana.org.au> 2006-08-16 21:39
> For a broadcast notification, the nlmsg_pid field is meaningless
> because the nlmsg_seq field is also meaningless.  I'm not denying
> that it wouldn't be useful to have the originator's socket address
> in there.  What I'm saying is that it's the wrong place to put
> that information.

It might not be the best place to put it considering the original
intend of nlmsg_pid as you explained correctly. However, as you
state yourself, the nlmsg_pid field is meaningless/unused for
notifications so extending the definition of nlmsg_pid to have
a special meaning for broadcasts doesn't harm anyone.

When setting nlmsg_seq to the seq of the request it becomes a
meaning together with nlmsg_pid as applications can then easly
assign notifications to their own sent requests.

Secondly we already have applications depending on this whereas
the eventual breaking of aplications depending on nlmsg_pid == 0
is uncertain.

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[Thomas Graf]

* Herbert Xu <herbert@gondor.apana.org.au> 2006-08-16 21:12
> On Wed, Aug 16, 2006 at 12:58:56PM +0200, Thomas Graf wrote:
> > 
> > All route and tc notifications already use the pid so applications
> > can decide whether the event was caused by them. A notification
> > is a reply to a request so it doesn't even violate RFC 2367.
> 
> Actually most IPv4 notifications *do* set the pid to zero which is
> the right thing to do for kernel-generated messages.
> 
> You're right though that the IPv6 notification modified by this patch
> does set the pid to the netlink originator.  Looking back in history
> it seems that this behaviour was only introduced last year to a subset
> of notifications.

It was added to help quagga identify which route modifications
are self caused. It's not possible to use rtm_protocol for this
purpose as other applications can delete routes added by quagga.

> This inconsistency is very bad.  IMHO this change (made last year)
> should be reverted so that all kernel generated (broadcast) notifications
> have the originator set to zero to match the source address of the
> message.

We can't just knowingly break quagga.

I think it's a good thing to include the pid, it's additional
information that is helpful to userspace. Userspace is already
aware that the notifications are orignating from the kernel,
we can't do userspace -> userspace communication anymore anyway.

> Any notification that sets the netlink pid to current->pid is
> *completely* bogus.  Let me repeat this, the netlink pid is not
> a process ID.

Everyone is aware of that, actually these patches fix all
occurences of current->pid by replacing them with a pid of 0.

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[Herbert Xu]

On Wed, Aug 16, 2006 at 01:40:03PM +0200, Thomas Graf wrote:
> 
> It was added to help quagga identify which route modifications
> are self caused. It's not possible to use rtm_protocol for this
> purpose as other applications can delete routes added by quagga.

Actually it's not that bad.  I just checked the quagga source and
the stuff it needs was already provided anyway, even before that
change.

In fact, the really bad bits in the changeset have already been
reverted by Alexey back in February :)

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[Thomas Graf]

* Herbert Xu <herbert@gondor.apana.org.au> 2006-08-16 21:57
> On Wed, Aug 16, 2006 at 01:40:03PM +0200, Thomas Graf wrote:
> > 
> > It was added to help quagga identify which route modifications
> > are self caused. It's not possible to use rtm_protocol for this
> > purpose as other applications can delete routes added by quagga.
> 
> Actually it's not that bad.  I just checked the quagga source and
> the stuff it needs was already provided anyway, even before that
> change.

If I recall correctly the quagga folks asked to get the same
behaviour for IPv6 routes as it was already done for IPv4
around the time of that bogus changeset.


> In fact, the really bad bits in the changeset have already been
> reverted by Alexey back in February :)

Right, but he forgot the bits in IPv6 which I now fixed. The
changeset introducing those current->pid uses was definitely
simply wrong. I'm not questioning that :)

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[jamal]

On Wed, 2006-16-08 at 14:05 +0200, Thomas Graf wrote:

> Right, but he forgot the bits in IPv6 which I now fixed. The
> changeset introducing those current->pid uses was definitely
> simply wrong. I'm not questioning that :)

Herbert, if you look at the thread as well I am no longer questioning
that either ;->

cheers,
jamal

PS:- Would a topic of "things i wish netlink did better" be of interest
for discussion (maybe for netconf)? (Un)fortunately, we are fixing some
of them with genetlink;-> 

Re: [PATCH 09/16] [IPv6] address: Convert address notification to use rtnl_notify()

[Alexey Kuznetsov]

Hello!

> The netlink header pid is really akin to sadb_msg_pid from RFC 2367.
> IMHO it should always be zero if the kernel is the originator of the
> message.

No. Analogue of sadb_msg_pid is nladdr.nl_pid.


Netlink header pid is not originator of the message, but author of
the change. The notion is ambiguous by definition, and the field
is a little ambiguous.

If the message is a plain ack or part of a dump, it is obviously
pid of requestor. But if it is notification about change, it can be
nl_pid of socket, which requested the operation, but may be 0.
Of course, all the 0s sent only because I was lazy to track authorship,
should be eliminated.

Alexey

[PATCH 10/16] [IPv6] route: Convert route notifications to use rtnl_notify()

[Thomas Graf]

[-- Attachment #1: rtnl_convert_ip6_route --]
[-- Type: text/plain, Size: 1780 bytes --]

Fixes a wrong use of current->pid as netlink pid.

Signed-off-by: Thomas Graf <tgraf@suug.ch>

Index: net-2.6.19.git/net/ipv6/route.c
===================================================================
--- net-2.6.19.git.orig/net/ipv6/route.c
+++ net-2.6.19.git/net/ipv6/route.c
@@ -35,7 +35,6 @@
 #include <linux/netdevice.h>
 #include <linux/in6.h>
 #include <linux/init.h>
-#include <linux/netlink.h>
 #include <linux/if_arp.h>
 
 #ifdef 	CONFIG_PROC_FS
@@ -54,6 +53,7 @@
 #include <net/dst.h>
 #include <net/xfrm.h>
 #include <net/netevent.h>
+#include <net/netlink.h>
 
 #include <asm/uaccess.h>
 
@@ -2052,27 +2052,25 @@ void inet6_rt_notify(int event, struct r
 			struct netlink_skb_parms *req)
 {
 	struct sk_buff *skb;
-	int size = NLMSG_SPACE(sizeof(struct rtmsg)+256);
-	u32 pid = current->pid;
-	u32 seq = 0;
-
-	if (req)
-		pid = req->pid;
-	if (nlh)
-		seq = nlh->nlmsg_seq;
-	
-	skb = alloc_skb(size, gfp_any());
-	if (!skb) {
-		netlink_set_err(rtnl, 0, RTNLGRP_IPV6_ROUTE, ENOBUFS);
-		return;
-	}
-	if (rt6_fill_node(skb, rt, NULL, NULL, 0, event, pid, seq, 0, 0) < 0) {
+	u32 pid = req ? req->pid : 0;
+	u32 seq = nlh ? nlh->nlmsg_seq : 0;
+	int payload = sizeof(struct rtmsg) + 256;
+	int err = -ENOBUFS;
+
+	skb = nlmsg_new(nlmsg_total_size(payload), gfp_any());
+	if (skb == NULL)
+		goto errout;
+
+	err = rt6_fill_node(skb, rt, NULL, NULL, 0, event, pid, seq, 0, 0);
+	if (err < 0) {
 		kfree_skb(skb);
-		netlink_set_err(rtnl, 0, RTNLGRP_IPV6_ROUTE, EINVAL);
-		return;
+		goto errout;
 	}
-	NETLINK_CB(skb).dst_group = RTNLGRP_IPV6_ROUTE;
-	netlink_broadcast(rtnl, skb, 0, RTNLGRP_IPV6_ROUTE, gfp_any());
+
+	err = rtnl_notify(skb, pid, RTNLGRP_IPV6_ROUTE, nlh, gfp_any());
+errout:
+	if (err < 0)
+		rtnl_set_sk_err(RTNLGRP_IPV6_ROUTE, err);
 }
 
 /*

[PATCH 11/16] [IPv6] link: Convert link notifications to use rtnl_notify()

[Thomas Graf]

[-- Attachment #1: rtnl_convert_ip6_link --]
[-- Type: text/plain, Size: 1395 bytes --]

Fixes a wrong use of current->pid as netlink pid.

Signed-off-by: Thomas Graf <tgraf@suug.ch>

Index: net-2.6.19.git/net/ipv6/addrconf.c
===================================================================
--- net-2.6.19.git.orig/net/ipv6/addrconf.c
+++ net-2.6.19.git/net/ipv6/addrconf.c
@@ -3438,20 +3438,23 @@ static int inet6_dump_ifinfo(struct sk_b
 void inet6_ifinfo_notify(int event, struct inet6_dev *idev)
 {
 	struct sk_buff *skb;
-	int size = NLMSG_SPACE(sizeof(struct ifinfomsg) + INET6_IFINFO_RTA_SPACE);
+	int payload = sizeof(struct ifinfomsg) + INET6_IFINFO_RTA_SPACE;
+	int err = -ENOBUFS;
 	
-	skb = alloc_skb(size, GFP_ATOMIC);
-	if (!skb) {
-		netlink_set_err(rtnl, 0, RTNLGRP_IPV6_IFINFO, ENOBUFS);
-		return;
-	}
-	if (inet6_fill_ifinfo(skb, idev, current->pid, 0, event, 0) < 0) {
+	skb = nlmsg_new(nlmsg_total_size(payload), GFP_ATOMIC);
+	if (skb == NULL)
+		goto errout;
+
+	err = inet6_fill_ifinfo(skb, idev, 0, 0, event, 0);
+	if (err < 0) {
 		kfree_skb(skb);
-		netlink_set_err(rtnl, 0, RTNLGRP_IPV6_IFINFO, EINVAL);
-		return;
+		goto errout;
 	}
-	NETLINK_CB(skb).dst_group = RTNLGRP_IPV6_IFINFO;
-	netlink_broadcast(rtnl, skb, 0, RTNLGRP_IPV6_IFINFO, GFP_ATOMIC);
+
+	err = rtnl_notify(skb, 0, RTNLGRP_IPV6_IFADDR, NULL, GFP_ATOMIC);
+errout:
+	if (err < 0)
+		rtnl_set_sk_err(RTNLGRP_IPV6_IFADDR, err);
 }
 
 /* Maximum length of prefix_cacheinfo attributes */

[PATCH 12/16] [IPv6] prefix: Convert prefix notifications to use rtnl_notify()

[Thomas Graf]

[-- Attachment #1: rtnl_convert_ip6_prefix --]
[-- Type: text/plain, Size: 1399 bytes --]

Fixes a wrong use of current->pid as netlink pid.

Signed-off-by: Thomas Graf <tgraf@suug.ch>

Index: net-2.6.19.git/net/ipv6/addrconf.c
===================================================================
--- net-2.6.19.git.orig/net/ipv6/addrconf.c
+++ net-2.6.19.git/net/ipv6/addrconf.c
@@ -3506,20 +3506,23 @@ static void inet6_prefix_notify(int even
 			 struct prefix_info *pinfo)
 {
 	struct sk_buff *skb;
-	int size = NLMSG_SPACE(sizeof(struct prefixmsg) + INET6_PREFIX_RTA_SPACE);
+	int payload = sizeof(struct prefixmsg) + INET6_PREFIX_RTA_SPACE;
+	int err = -ENOBUFS;
 
-	skb = alloc_skb(size, GFP_ATOMIC);
-	if (!skb) {
-		netlink_set_err(rtnl, 0, RTNLGRP_IPV6_PREFIX, ENOBUFS);
-		return;
-	}
-	if (inet6_fill_prefix(skb, idev, pinfo, current->pid, 0, event, 0) < 0) {
+	skb = nlmsg_new(nlmsg_total_size(payload), GFP_ATOMIC);
+	if (skb == NULL)
+		goto errout;
+
+	err = inet6_fill_prefix(skb, idev, pinfo, 0, 0, event, 0);
+	if (err < 0) {
 		kfree_skb(skb);
-		netlink_set_err(rtnl, 0, RTNLGRP_IPV6_PREFIX, EINVAL);
-		return;
+		goto errout;
 	}
-	NETLINK_CB(skb).dst_group = RTNLGRP_IPV6_PREFIX;
-	netlink_broadcast(rtnl, skb, 0, RTNLGRP_IPV6_PREFIX, GFP_ATOMIC);
+
+	err = rtnl_notify(skb, 0, RTNLGRP_IPV6_PREFIX, NULL, GFP_ATOMIC);
+errout:
+	if (err < 0)
+		rtnl_set_sk_err(RTNLGRP_IPV6_PREFIX, err);
 }
 
 static struct rtnetlink_link inet6_rtnetlink_table[RTM_NR_MSGTYPES] = {

[PATCH 13/16] [BRIDGE]: Convert notifications to use rtnl_notify()

[Thomas Graf]

[-- Attachment #1: rtnl_convert_bridge --]
[-- Type: text/plain, Size: 1434 bytes --]

Fixes a wrong use of current->pid as netlink pid.

Signed-off-by: Thomas Graf <tgraf@suug.ch>

Index: net-2.6.19.git/net/bridge/br_netlink.c
===================================================================
--- net-2.6.19.git.orig/net/bridge/br_netlink.c
+++ net-2.6.19.git/net/bridge/br_netlink.c
@@ -12,6 +12,7 @@
 
 #include <linux/kernel.h>
 #include <linux/rtnetlink.h>
+#include <net/netlink.h>
 #include "br_private.h"
 
 /*
@@ -76,26 +77,24 @@ rtattr_failure:
 void br_ifinfo_notify(int event, struct net_bridge_port *port)
 {
 	struct sk_buff *skb;
-	int err = -ENOMEM;
+	int payload = sizeof(struct ifinfomsg) + 128;
+	int err = -ENOBUFS;
 
 	pr_debug("bridge notify event=%d\n", event);
-	skb = alloc_skb(NLMSG_SPACE(sizeof(struct ifinfomsg) + 128),
-			GFP_ATOMIC);
-	if (!skb)
-		goto err_out;
+	skb = nlmsg_new(nlmsg_total_size(payload), GFP_ATOMIC);
+	if (skb == NULL)
+		goto errout;
+
+	err = br_fill_ifinfo(skb, port, 0, 0, event, 0);
+	if (err < 0) {
+		kfree_skb(skb);
+		goto errout;
+	}
 
-	err = br_fill_ifinfo(skb, port, current->pid, 0, event, 0);
+	err = rtnl_notify(skb, 0, RTNLGRP_LINK, NULL, GFP_ATOMIC);
+errout:
 	if (err < 0)
-		goto err_kfree;
-
-	NETLINK_CB(skb).dst_group = RTNLGRP_LINK;
-	netlink_broadcast(rtnl, skb, 0, RTNLGRP_LINK, GFP_ATOMIC);
-	return;
-
-err_kfree:
-	kfree_skb(skb);
-err_out:
-	netlink_set_err(rtnl, 0, RTNLGRP_LINK, err);
+		rtnl_set_sk_err(RTNLGRP_LINK, err);
 }
 
 /*

[PATCH 14/16] [WIRELESS]: Convert notifications to use rtnl_notify()

[Thomas Graf]

[-- Attachment #1: rtnl_convert_wireless --]
[-- Type: text/plain, Size: 784 bytes --]

Signed-off-by: Thomas Graf <tgraf@suug.ch>

Index: net-2.6.19.git/net/core/wireless.c
===================================================================
--- net-2.6.19.git.orig/net/core/wireless.c
+++ net-2.6.19.git/net/core/wireless.c
@@ -85,6 +85,7 @@
 
 #include <linux/wireless.h>		/* Pretty obvious */
 #include <net/iw_handler.h>		/* New driver API */
+#include <net/netlink.h>
 
 #include <asm/uaccess.h>		/* copy_to_user() */
 
@@ -1849,7 +1850,7 @@ static void wireless_nlevent_process(uns
 	struct sk_buff *skb;
 
 	while ((skb = skb_dequeue(&wireless_nlevent_queue)))
-		netlink_broadcast(rtnl, skb, 0, RTNLGRP_LINK, GFP_ATOMIC);
+		rtnl_notify(skb, 0, RTNLGRP_LINK, NULL, GFP_ATOMIC);
 }
 
 static DECLARE_TASKLET(wireless_nlevent_tasklet, wireless_nlevent_process, 0);

[PATCH 15/16] [NET] link: Convert notifications to use rtnl_notify()

[Thomas Graf]

[-- Attachment #1: rtnl_convert_link --]
[-- Type: text/plain, Size: 1196 bytes --]

Signed-off-by: Thomas Graf <tgraf@suug.ch>

Index: net-2.6.19.git/net/core/rtnetlink.c
===================================================================
--- net-2.6.19.git.orig/net/core/rtnetlink.c
+++ net-2.6.19.git/net/core/rtnetlink.c
@@ -630,20 +630,22 @@ static int rtnl_dump_all(struct sk_buff 
 void rtmsg_ifinfo(int type, struct net_device *dev, unsigned change)
 {
 	struct sk_buff *skb;
-	int size = NLMSG_SPACE(sizeof(struct ifinfomsg) +
-			       sizeof(struct rtnl_link_ifmap) +
-			       sizeof(struct rtnl_link_stats) + 128);
-
-	skb = nlmsg_new(size, GFP_KERNEL);
-	if (!skb)
-		return;
+	int err = -ENOBUFS;
 
-	if (rtnl_fill_ifinfo(skb, dev, NULL, 0, type, 0, 0, change, 0) < 0) {
+	skb = nlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
+	if (skb == NULL)
+		goto errout;
+
+	err = rtnl_fill_ifinfo(skb, dev, NULL, 0, type, 0, 0, change, 0);
+	if (err < 0) {
 		kfree_skb(skb);
-		return;
+		goto errout;
 	}
-	NETLINK_CB(skb).dst_group = RTNLGRP_LINK;
-	netlink_broadcast(rtnl, skb, 0, RTNLGRP_LINK, GFP_KERNEL);
+
+	err = rtnl_notify(skb, 0, RTNLGRP_LINK, NULL, GFP_KERNEL);
+errout:
+	if (err < 0)
+		rtnl_set_sk_err(RTNLGRP_LINK, err);
 }
 
 /* Protected by RTNL sempahore.  */

[PATCH 16/16] [RTNETLINK]: Unexport rtnl socket

[Thomas Graf]

[-- Attachment #1: unexport_rtnl --]
[-- Type: text/plain, Size: 1351 bytes --]

Signed-off-by: Thomas Graf <tgraf@suug.ch>

Index: net-2.6.19.git/include/linux/rtnetlink.h
===================================================================
--- net-2.6.19.git.orig/include/linux/rtnetlink.h
+++ net-2.6.19.git/include/linux/rtnetlink.h
@@ -574,8 +574,6 @@ extern int rtattr_parse(struct rtattr *t
 #define rtattr_parse_nested(tb, max, rta) \
 	rtattr_parse((tb), (max), RTA_DATA((rta)), RTA_PAYLOAD((rta)))
 
-extern struct sock *rtnl;
-
 struct rtnetlink_link
 {
 	int (*doit)(struct sk_buff *, struct nlmsghdr*, void *attr);
Index: net-2.6.19.git/net/core/rtnetlink.c
===================================================================
--- net-2.6.19.git.orig/net/core/rtnetlink.c
+++ net-2.6.19.git/net/core/rtnetlink.c
@@ -58,6 +58,7 @@
 #endif	/* CONFIG_NET_WIRELESS_RTNETLINK */
 
 static DEFINE_MUTEX(rtnl_mutex);
+static struct sock *rtnl;
 
 void rtnl_lock(void)
 {
@@ -95,8 +96,6 @@ int rtattr_parse(struct rtattr *tb[], in
 	return 0;
 }
 
-struct sock *rtnl;
-
 struct rtnetlink_link * rtnetlink_links[NPROTO];
 
 static const int rtm_min[RTM_NR_FAMILIES] =
@@ -842,7 +841,6 @@ EXPORT_SYMBOL(rtattr_strlcpy);
 EXPORT_SYMBOL(rtattr_parse);
 EXPORT_SYMBOL(rtnetlink_links);
 EXPORT_SYMBOL(rtnetlink_put_metrics);
-EXPORT_SYMBOL(rtnl);
 EXPORT_SYMBOL(rtnl_lock);
 EXPORT_SYMBOL(rtnl_trylock);
 EXPORT_SYMBOL(rtnl_unlock);

Re: [PATCHSET] rtnetlink notification rework

[David Miller]

From: Thomas Graf <tgraf@suug.ch>
Date: Mon, 14 Aug 2006 23:37:18 +0200

> This patchset reworks rtnetlink notifications. Notification logic
> is fixed and gets hidden behind nlmsg_notify() and the rtnl socket
> is no longer directly accessed. All notification paths get
> appropriate error handling. NLM_F_ECHO support is added where it
> makes sense and no major surgery is required. Abuses of current->pid
> for netlink pids are fixed. An exception is made for users of
> rtnetlink_send() which will get converted in a second step as some
> bigger code change are required.

Really nice work Thomas.  All patches applied to net-2.6.19:

-extern struct sock *rtnl;
-
...
-struct sock *rtnl;
-
...
-EXPORT_SYMBOL(rtnl);

Let's party! :-)