Skip to contentSkip to conversation

PATCH: rtnetlink explicit flags setting

PATCH In review jamal 4 participants 26 messages May 27, 2005 – Jun 3, 2005

Subsystems (4)
Networking Network drivers IPv4/IPv6 Traffic control (tc)
lore
Share & tools
lore

jamal

author
#0 lore raw

I think i may have missed a few spots.

Thomas it would be worth creating a macro which takes flags as input as well to replace things along the lines of:

        nlh = NLMSG_PUT(skb, pid, seq, event, sizeof(*ifm));
        nlh->nlmsg_flags = flags;

Since theres a sufficient number of them.

I will work next on fixing ioctl derived events to contain the proper pid of the process that caused the event

cheers,
jamal
--

This patch rectifies some rtnetlink message builders that derive the flags from the pid. It is now explicit like the other cases which get it right.

Signed-off-by Jamal Hadi Salim <[email protected]>

net/core/neighbour.c: needs update
net/core/rtnetlink.c: needs update
net/decnet/dn_route.c: needs update
net/ipv4/devinet.c: needs update
net/ipv4/route.c: needs update
net/ipv6/addrconf.c: needs update
Index: net/core/neighbour.c
===================================================================

net/core/neighbour.c (mode:100644) +6 -5
@@ -1548,7 +1548,7 @@
 
 
 static int neigh_fill_info(struct sk_buff *skb, struct neighbour *n,
-			   u32 pid, u32 seq, int event)
+			   u32 pid, u32 seq, int event, unsigned flags)
 {
 	unsigned long now = jiffies;
 	unsigned char *b = skb->tail;

@@ -1559,7 +1559,7 @@
 					 sizeof(struct ndmsg));
 	struct ndmsg *ndm = NLMSG_DATA(nlh);
 
-	nlh->nlmsg_flags = pid ? NLM_F_MULTI : 0;
+	nlh->nlmsg_flags = flags;
 	ndm->ndm_family	 = n->ops->family;
 	ndm->ndm_flags	 = n->flags;
 	ndm->ndm_type	 = n->type;

@@ -1609,7 +1609,8 @@
 				continue;
 			if (neigh_fill_info(skb, n, NETLINK_CB(cb->skb).pid,
 					    cb->nlh->nlmsg_seq,
-					    RTM_NEWNEIGH) <= 0) {
+					    RTM_NEWNEIGH,
+					    NLM_F_MULTI) <= 0) {
 				read_unlock_bh(&tbl->lock);
 				rc = -1;
 				goto out;

@@ -2018,7 +2019,7 @@
 	if (!skb)
 		return;
 
-	if (neigh_fill_info(skb, n, 0, 0, RTM_GETNEIGH) < 0) {
+	if (neigh_fill_info(skb, n, 0, 0, RTM_GETNEIGH, 0) < 0) {
 		kfree_skb(skb);
 		return;
 	}

@@ -2037,7 +2038,7 @@
 	if (!skb)
 		return;
 
-	if (neigh_fill_info(skb, n, 0, 0, RTM_NEWNEIGH) < 0) {
+	if (neigh_fill_info(skb, n, 0, 0, RTM_NEWNEIGH, 0) < 0) {
 		kfree_skb(skb);
 		return;
 	}
net/core/rtnetlink.c (mode:100644) +5 -4
Index: net/core/rtnetlink.c
===================================================================
@@ -176,14 +176,15 @@
 
 
 static int rtnetlink_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
-				 int type, u32 pid, u32 seq, u32 change)
+				 int type, u32 pid, u32 seq, u32 change, 
+				 unsigned flags)
 {
 	struct ifinfomsg *r;
 	struct nlmsghdr  *nlh;
 	unsigned char	 *b = skb->tail;
 
 	nlh = NLMSG_PUT(skb, pid, seq, type, sizeof(*r));
-	if (pid) nlh->nlmsg_flags |= NLM_F_MULTI;
+	nlh->nlmsg_flags = flags;
 	r = NLMSG_DATA(nlh);
 	r->ifi_family = AF_UNSPEC;
 	r->ifi_type = dev->type;

@@ -273,7 +274,7 @@
 	for (dev=dev_base, idx=0; dev; dev = dev->next, idx++) {
 		if (idx < s_idx)
 			continue;
-		if (rtnetlink_fill_ifinfo(skb, dev, RTM_NEWLINK, NETLINK_CB(cb->skb).pid, cb->nlh->nlmsg_seq, 0) <= 0)
+		if (rtnetlink_fill_ifinfo(skb, dev, RTM_NEWLINK, NETLINK_CB(cb->skb).pid, cb->nlh->nlmsg_seq, 0, NLM_F_MULTI) <= 0)
 			break;
 	}
 	read_unlock(&dev_base_lock);

@@ -447,7 +448,7 @@
 	if (!skb)
 		return;
 
-	if (rtnetlink_fill_ifinfo(skb, dev, type, 0, 0, change) < 0) {
+	if (rtnetlink_fill_ifinfo(skb, dev, type, 0, 0, change, 0) < 0) {
 		kfree_skb(skb);
 		return;
 	}
net/decnet/dn_route.c (mode:100644) +5 -4
Index: net/decnet/dn_route.c
===================================================================
@@ -1465,7 +1465,7 @@
 	return dn_route_input_slow(skb);
 }
 
-static int dn_rt_fill_info(struct sk_buff *skb, u32 pid, u32 seq, int event, int nowait)
+static int dn_rt_fill_info(struct sk_buff *skb, u32 pid, u32 seq, int event, int nowait, unsigned flags)
 {
 	struct dn_route *rt = (struct dn_route *)skb->dst;
 	struct rtmsg *r;

@@ -1475,7 +1475,7 @@
 
 	nlh = NLMSG_PUT(skb, pid, seq, event, sizeof(*r));
 	r = NLMSG_DATA(nlh);
-	nlh->nlmsg_flags = (nowait && pid) ? NLM_F_MULTI : 0;
+	nlh->nlmsg_flags = flags;
 	r->rtm_family = AF_DECnet;
 	r->rtm_dst_len = 16;
 	r->rtm_src_len = 0;

@@ -1596,7 +1596,7 @@
 
 	NETLINK_CB(skb).dst_pid = NETLINK_CB(in_skb).pid;
 
-	err = dn_rt_fill_info(skb, NETLINK_CB(in_skb).pid, nlh->nlmsg_seq, RTM_NEWROUTE, 0);
+	err = dn_rt_fill_info(skb, NETLINK_CB(in_skb).pid, nlh->nlmsg_seq, RTM_NEWROUTE, 0, 0);
 
 	if (err == 0)
 		goto out_free;

@@ -1644,7 +1644,8 @@
 				continue;
 			skb->dst = dst_clone(&rt->u.dst);
 			if (dn_rt_fill_info(skb, NETLINK_CB(cb->skb).pid,
-					cb->nlh->nlmsg_seq, RTM_NEWROUTE, 1) <= 0) {
+					cb->nlh->nlmsg_seq, RTM_NEWROUTE, 
+					1, NLM_F_MULTI) <= 0) {
 				dst_release(xchg(&skb->dst, NULL));
 				rcu_read_unlock_bh();
 				goto done;
net/ipv4/devinet.c (mode:100644) +4 -4
Index: net/ipv4/devinet.c
===================================================================
@@ -1015,14 +1015,14 @@
 };
 
 static int inet_fill_ifaddr(struct sk_buff *skb, struct in_ifaddr *ifa,
-			    u32 pid, u32 seq, int event)
+			    u32 pid, u32 seq, int event, unsigned flags)
 {
 	struct ifaddrmsg *ifm;
 	struct nlmsghdr  *nlh;
 	unsigned char	 *b = skb->tail;
 
 	nlh = NLMSG_PUT(skb, pid, seq, event, sizeof(*ifm));
-	if (pid) nlh->nlmsg_flags |= NLM_F_MULTI;
+	nlh->nlmsg_flags = flags;
 	ifm = NLMSG_DATA(nlh);
 	ifm->ifa_family = AF_INET;
 	ifm->ifa_prefixlen = ifa->ifa_prefixlen;

@@ -1075,7 +1075,7 @@
 				continue;
 			if (inet_fill_ifaddr(skb, ifa, NETLINK_CB(cb->skb).pid,
 					     cb->nlh->nlmsg_seq,
-					     RTM_NEWADDR) <= 0) {
+					     RTM_NEWADDR, NLM_F_MULTI) <= 0) {
 				rcu_read_unlock();
 				goto done;
 			}

@@ -1098,7 +1098,7 @@
 
 	if (!skb)
 		netlink_set_err(rtnl, 0, RTMGRP_IPV4_IFADDR, ENOBUFS);
-	else if (inet_fill_ifaddr(skb, ifa, 0, 0, event) < 0) {
+	else if (inet_fill_ifaddr(skb, ifa, 0, 0, event, 0) < 0) {
 		kfree_skb(skb);
 		netlink_set_err(rtnl, 0, RTMGRP_IPV4_IFADDR, EINVAL);
 	} else {
net/ipv4/route.c (mode:100644) +5 -5
Index: net/ipv4/route.c
===================================================================
@@ -2581,7 +2581,7 @@
 }
 
 static int rt_fill_info(struct sk_buff *skb, u32 pid, u32 seq, int event,
-			int nowait)
+			int nowait, unsigned flags)
 {
 	struct rtable *rt = (struct rtable*)skb->dst;
 	struct rtmsg *r;

@@ -2593,7 +2593,7 @@
 #endif
 	nlh = NLMSG_PUT(skb, pid, seq, event, sizeof(*r));
 	r = NLMSG_DATA(nlh);
-	nlh->nlmsg_flags = (nowait && pid) ? NLM_F_MULTI : 0;
+	nlh->nlmsg_flags = flags;
 	r->rtm_family	 = AF_INET;
 	r->rtm_dst_len	= 32;
 	r->rtm_src_len	= 0;

@@ -2744,7 +2744,7 @@
 	NETLINK_CB(skb).dst_pid = NETLINK_CB(in_skb).pid;
 
 	err = rt_fill_info(skb, NETLINK_CB(in_skb).pid, nlh->nlmsg_seq,
-				RTM_NEWROUTE, 0);
+				RTM_NEWROUTE, 0, 0);
 	if (!err)
 		goto out_free;
 	if (err < 0) {

@@ -2781,8 +2781,8 @@
 				continue;
 			skb->dst = dst_clone(&rt->u.dst);
 			if (rt_fill_info(skb, NETLINK_CB(cb->skb).pid,
-					 cb->nlh->nlmsg_seq,
-					 RTM_NEWROUTE, 1) <= 0) {
+					 cb->nlh->nlmsg_seq, RTM_NEWROUTE, 
+					 1, NLM_F_MULTI) <= 0) {
 				dst_release(xchg(&skb->dst, NULL));
 				rcu_read_unlock_bh();
 				goto done;
net/ipv6/addrconf.c (mode:100644) +23 -20

151 lines not on this page · raw message

#1 lore raw

* jamal <[email protected]> 2005-05-27 08:32

Quoted 8 lines

I think i may have missed a few spots.

Thomas it would be worth creating a macro which takes flags as input as well to replace things along the lines of:

        nlh = NLMSG_PUT(skb, pid, seq, event, sizeof(*ifm));
        nlh->nlmsg_flags = flags;

Since theres a sufficient number of them.

Right, proposal:

include/linux/netlink.h (mode:100644) +10 -7
@@ -156,7 +156,7 @@
 };
 
 static __inline__ struct nlmsghdr *
-__nlmsg_put(struct sk_buff *skb, u32 pid, u32 seq, int type, int len)
+__nlmsg_put(struct sk_buff *skb, u32 pid, u32 seq, int type, int len, int flags)
 {
        struct nlmsghdr *nlh;
        int size = NLMSG_LENGTH(len);

@@ -164,20 +164,23 @@
        nlh = (struct nlmsghdr*)skb_put(skb, NLMSG_ALIGN(size));
        nlh->nlmsg_type = type;
        nlh->nlmsg_len = size;
-       nlh->nlmsg_flags = 0;
+       nlh->nlmsg_flags = flags;
        nlh->nlmsg_pid = pid;
        nlh->nlmsg_seq = seq;
        return nlh;
 }
 
-#define NLMSG_PUT(skb, pid, seq, type, len) \
+#define NLMSG_NEW(skb, pid, seq, type, len, flags) \
 ({     if (skb_tailroom(skb) < (int)NLMSG_SPACE(len)) \
                goto nlmsg_failure; \
-       __nlmsg_put(skb, pid, seq, type, len); })
+       __nlmsg_put(skb, pid, seq, type, len, flags); })
+
+#define NLMSG_PUT(skb, pid, seq, type, len) \
+       NLMSG_NEW(skb, pid, seq, type, len, 0)
 
-#define NLMSG_PUT_ANSWER(skb, cb, type, len) \
-       NLMSG_PUT(skb, NETLINK_CB((cb)->skb).pid, \
-                 (cb)->nlh->nlmsg_seq, type, len)
+#define NLMSG_NEW_ANSWER(skb, cb, type, len, flags) \
+       NLMSG_NEW(skb, NETLINK_CB((cb)->skb).pid, \
+                 (cb)->nlh->nlmsg_seq, type, len, flags)
 
 #define NLMSG_END(skb, nlh) \
 ({     (nlh)->nlmsg_len = (skb)->tail - (unsigned char *) (nlh); \

I'd be especially happy with this because __nlmsg_put keeps control over the flags ;->

author
#2 lore raw

include/linux/netlink.h (mode:100644) line 173

Quoted 3 lines · Thomas Graf
include/linux/netlink.h (mode:100644) +1 -1
-#define NLMSG_PUT(skb, pid, seq, type, len) \
+#define NLMSG_NEW(skb, pid, seq, type, len, flags) \

How about introducing NLMSG_NEW and leave NLMSG_PUT where it is? This way we could do incremental updates. If there are not too many of those NLMSG_PUT around, then it shouldnt be an issue.

The first change would be definetely on top of the current patch i sent which is well deserved.
Lets have Dave swallow my patch then either you or i could make those changes. Sounds reasonable?

cheers,
jamal

#3 lore raw

* jamal <[email protected]> 2005-05-27 09:58

Quoted 10 lines · Thomas Graf

include/linux/netlink.h (mode:100644) line 173

Quoted 5 lines
include/linux/netlink.h (mode:100644) +1 -1
-#define NLMSG_PUT(skb, pid, seq, type, len) \
+#define NLMSG_NEW(skb, pid, seq, type, len, flags) \

How about introducing NLMSG_NEW and leave NLMSG_PUT where it is? This way we could do incremental updates. If there are not too many of those NLMSG_PUT around, then it shouldnt be an issue.

That is what the patch does but the diff looks a bit irritating.
1) Introduce NLMSG_NEW to take flags
2) Make NLMSG_PUT call NLMSG_NEW with flags==0
3) Change __nlmsg_put to take flags argument

Quoted 4 lines

The first change would be definetely on top of the current patch i sent which is well deserved.
Lets have Dave swallow my patch then either you or i could make those changes. Sounds reasonable?

Sure.

author
#4 lore raw
Quoted 5 lines · Thomas Graf

That is what the patch does but the diff looks a bit irritating.
1) Introduce NLMSG_NEW to take flags
2) Make NLMSG_PUT call NLMSG_NEW with flags==0
3) Change __nlmsg_put to take flags argument

Yikes - sorry i glossed over the patch.

Quoted 6 lines
Quoted 4 lines

The first change would be definetely on top of the current patch i sent which is well deserved.
Lets have Dave swallow my patch then either you or i could make those changes. Sounds reasonable?

Sure.

Dave please apply the patch i posted into your tree and then we can follow with this other stuff.
Actually Thomas, you can probably apply my patch on your side and then the other changes to use _NEW() on top of it. If you dont have time i could do it tommorow.

cheers,
jamal

#5 lore raw

* jamal <[email protected]> 2005-05-27 11:01

Quoted 5 lines

Dave please apply the patch i posted into your tree and then we can follow with this other stuff.
Actually Thomas, you can probably apply my patch on your side and then the other changes to use _NEW() on top of it. If you dont have time i could do it tommorow.

Sure, I can come up with a patchset today which will include:
o the new NLMSG_NEW macro
o your patch adopted to use the new NLMSG_NEW macro
o small patch to rename NLMSG_PUT_ANSWER usages into NLMSG_NEW_ANSWER
o a small fallout from the new neighbour table code which gcc didn't warn me about when it should.

OK?

author
#6 lore raw
Quoted 8 lines · Thomas Graf

Sure, I can come up with a patchset today which will include:
o the new NLMSG_NEW macro
o your patch adopted to use the new NLMSG_NEW macro
o small patch to rename NLMSG_PUT_ANSWER usages into NLMSG_NEW_ANSWER
o a small fallout from the new neighbour table code which gcc didn't warn me about when it should.

OK?

Sounds reasonable.

cheers,
jamal

PS:- Have to disappear for a few hours so they maybe latency in further responses.

#7 lore raw

* jamal <[email protected]> 2005-05-27 11:57

Quoted 9 lines · Thomas Graf
Quoted 8 lines

Sure, I can come up with a patchset today which will include:
o the new NLMSG_NEW macro
o your patch adopted to use the new NLMSG_NEW macro
o small patch to rename NLMSG_PUT_ANSWER usages into NLMSG_NEW_ANSWER
o a small fallout from the new neighbour table code which gcc didn't warn me about when it should.

Quoted 3 lines
Quoted 1 line

OK?

Sounds reasonable.

I took your patch, converted it to use NLMSG_NEW and finished converting all the modules that you started. Yet unconverted
are: netfilter, net/sched/ tcp_diag, xfrm, kernel/audit.

I also fixed some uses of NL_CREDS() to fetch the pid for message generation which is wrong. Various dumping procedures did not set the multi flag at all, I fixed all of them.

Do not apply these patches yet, they're way too intrusive

#8 lore raw

PATCH 1/2 Introduce NLMSG_NEW macro to better handle netlink flags

Introduces a new macro NLMSG_NEW which extends NLMSG_PUT but takes a flags argument. NLMSG_PUT stays there for compatibility but now calls NLMSG_NEW with flags == 0. NLMSG_PUT_ANSWER is renamed to NLMSG_NEW_ANSWER which now also takes a flags argument.

Also converts the users of NLMSG_PUT_ANSWER to use NLMSG_NEW_ANSWER and fixes the two direct users of __nlmsg_put to either provide the flags or use NLMSG_NEW(_ANSWER).

Signed-off-by Thomas Graf <[email protected]>

---
commit 1eb85f52f4e0d23996f6aa49009ce02fc0579658
tree debb58e5f900a6551f751b457226250cfccc3c75
parent fde09d4989b6c7b5183ffb3ec2076ff53dd6fa78
author Thomas Graf <[email protected]> Fri, 27 May 2005 17:43:09 +0200 committer Thomas Graf <[email protected]> Fri, 27 May 2005 17:43:09 +0200

 include/linux/netlink.h  |   17 ++++++++++-------
 net/core/neighbour.c     |    8 ++++----
 net/netlink/af_netlink.c |    8 +++++---
 3 files changed, 19 insertions(+), 14 deletions(-)
include/linux/netlink.h (mode:100644) +10 -7
Index: include/linux/netlink.h
===================================================================
@@ -156,7 +156,7 @@
 };
 
 static __inline__ struct nlmsghdr *
-__nlmsg_put(struct sk_buff *skb, u32 pid, u32 seq, int type, int len)
+__nlmsg_put(struct sk_buff *skb, u32 pid, u32 seq, int type, int len, int flags)
 {
 	struct nlmsghdr *nlh;
 	int size = NLMSG_LENGTH(len);

@@ -164,20 +164,23 @@
 	nlh = (struct nlmsghdr*)skb_put(skb, NLMSG_ALIGN(size));
 	nlh->nlmsg_type = type;
 	nlh->nlmsg_len = size;
-	nlh->nlmsg_flags = 0;
+	nlh->nlmsg_flags = flags;
 	nlh->nlmsg_pid = pid;
 	nlh->nlmsg_seq = seq;
 	return nlh;
 }
 
-#define NLMSG_PUT(skb, pid, seq, type, len) \
+#define NLMSG_NEW(skb, pid, seq, type, len, flags) \
 ({	if (skb_tailroom(skb) < (int)NLMSG_SPACE(len)) \
 		goto nlmsg_failure; \
-	__nlmsg_put(skb, pid, seq, type, len); })
+	__nlmsg_put(skb, pid, seq, type, len, flags); })
+
+#define NLMSG_PUT(skb, pid, seq, type, len) \
+	NLMSG_NEW(skb, pid, seq, type, len, 0)
 
-#define NLMSG_PUT_ANSWER(skb, cb, type, len) \
-	NLMSG_PUT(skb, NETLINK_CB((cb)->skb).pid, \
-		  (cb)->nlh->nlmsg_seq, type, len)
+#define NLMSG_NEW_ANSWER(skb, cb, type, len, flags) \
+	NLMSG_NEW(skb, NETLINK_CB((cb)->skb).pid, \
+		  (cb)->nlh->nlmsg_seq, type, len, flags)
 
 #define NLMSG_END(skb, nlh) \
 ({	(nlh)->nlmsg_len = (skb)->tail - (unsigned char *) (nlh); \
net/core/neighbour.c (mode:100644) +4 -4
Index: net/core/neighbour.c
===================================================================
@@ -1588,8 +1588,8 @@
 	struct nlmsghdr *nlh;
 	struct ndtmsg *ndtmsg;
 
-	nlh = NLMSG_PUT_ANSWER(skb, cb, RTM_NEWNEIGHTBL, sizeof(struct ndtmsg));
-	nlh->nlmsg_flags |= NLM_F_MULTI;
+	nlh = NLMSG_NEW_ANSWER(skb, cb, RTM_NEWNEIGHTBL, sizeof(struct ndtmsg),
+			       NLM_F_MULTI);
 
 	ndtmsg = NLMSG_DATA(nlh);
 

@@ -1673,8 +1673,8 @@
 	struct ndtmsg *ndtmsg;
 	struct nlmsghdr *nlh;
 
-	nlh = NLMSG_PUT_ANSWER(skb, cb, RTM_NEWNEIGHTBL, sizeof(struct ndtmsg));
-	nlh->nlmsg_flags |= NLM_F_MULTI;
+	nlh = NLMSG_NEW_ANSWER(skb, cb, RTM_NEWNEIGHTBL, sizeof(struct ndtmsg),
+			       NLM_F_MULTI);
 
 	ndtmsg = NLMSG_DATA(nlh);
net/netlink/af_netlink.c (mode:100644) +5 -3
Index: net/netlink/af_netlink.c
===================================================================
@@ -1095,8 +1095,7 @@
 		return 0;
 	}
 
-	nlh = __nlmsg_put(skb, NETLINK_CB(cb->skb).pid, cb->nlh->nlmsg_seq, NLMSG_DONE, sizeof(int));
-	nlh->nlmsg_flags |= NLM_F_MULTI;
+	nlh = NLMSG_NEW_ANSWER(skb, cb, NLMSG_DONE, sizeof(len), NLM_F_MULTI);
 	memcpy(NLMSG_DATA(nlh), &len, sizeof(len));
 	skb_queue_tail(&sk->sk_receive_queue, skb);
 	sk->sk_data_ready(sk, skb->len);

@@ -1107,6 +1106,9 @@
 
 	netlink_destroy_callback(cb);
 	return 0;
+
+nlmsg_failure:
+	return -ENOBUFS;
 }
 
 int netlink_dump_start(struct sock *ssk, struct sk_buff *skb,

@@ -1178,7 +1180,7 @@
 	}
 
 	rep = __nlmsg_put(skb, NETLINK_CB(in_skb).pid, nlh->nlmsg_seq,
-			  NLMSG_ERROR, sizeof(struct nlmsgerr));
+			  NLMSG_ERROR, sizeof(struct nlmsgerr), 0);
 	errmsg = NLMSG_DATA(rep);
 	errmsg->error = err;
 	memcpy(&errmsg->msg, nlh, err ? nlh->nlmsg_len : sizeof(struct nlmsghdr));
#9 lore raw

PATCH 2/2 Correctly set NLM_F_MULTI without checking the pid

In reply to Thomas Graf

This patch rectifies some rtnetlink message builders that derive the flags from the pid. It is now explicit like the other cases which get it right. Also fixes half a dozen dumpers which did not set NLM_F_MULTI at all.

Signed-off-by Jamal Hadi Salim <[email protected]>
Signed-off-by Thomas Graf <[email protected]>

---
commit b20a9fb4f28c4397096cf3c745c42cf045c36a7a
tree 5144b4dc6ffa4fb1821eb758838f63c5e210bc34
parent 1eb85f52f4e0d23996f6aa49009ce02fc0579658
author Thomas Graf <[email protected]> Fri, 27 May 2005 18:58:48 +0200 committer Thomas Graf <[email protected]> Fri, 27 May 2005 18:58:48 +0200

 net/core/neighbour.c     |   14 ++++++-------
 net/core/rtnetlink.c     |   13 +++++++-----
 net/decnet/dn_dev.c      |    9 ++++----
 net/decnet/dn_route.c    |   11 +++++-----
 net/decnet/dn_rules.c    |    7 +++---
 net/decnet/dn_table.c    |    8 +++----
 net/ipv4/devinet.c       |    9 +++-----
 net/ipv4/fib_hash.c      |    3 +-
 net/ipv4/fib_lookup.h    |    3 +-
 net/ipv4/fib_rules.c     |    7 +++---
 net/ipv4/fib_semantics.c |    6 ++---
 net/ipv4/route.c         |   11 ++++------
 net/ipv6/addrconf.c      |   49 ++++++++++++++++++++++-------------------------
 net/ipv6/route.c         |   11 +++++-----
 14 files changed, 83 insertions(+), 78 deletions(-)
net/core/neighbour.c (mode:100644) +7 -7
Index: net/core/neighbour.c
===================================================================
@@ -1857,18 +1857,17 @@
 }
 
 static int neigh_fill_info(struct sk_buff *skb, struct neighbour *n,
-			   u32 pid, u32 seq, int event)
+			   u32 pid, u32 seq, int event, unsigned int flags)
 {
 	unsigned long now = jiffies;
 	unsigned char *b = skb->tail;
 	struct nda_cacheinfo ci;
 	int locked = 0;
 	u32 probes;
-	struct nlmsghdr *nlh = NLMSG_PUT(skb, pid, seq, event,
-					 sizeof(struct ndmsg));
+	struct nlmsghdr *nlh = NLMSG_NEW(skb, pid, seq, event,
+					 sizeof(struct ndmsg), flags);
 	struct ndmsg *ndm = NLMSG_DATA(nlh);
 
-	nlh->nlmsg_flags = pid ? NLM_F_MULTI : 0;
 	ndm->ndm_family	 = n->ops->family;
 	ndm->ndm_flags	 = n->flags;
 	ndm->ndm_type	 = n->type;

@@ -1918,7 +1917,8 @@
 				continue;
 			if (neigh_fill_info(skb, n, NETLINK_CB(cb->skb).pid,
 					    cb->nlh->nlmsg_seq,
-					    RTM_NEWNEIGH) <= 0) {
+					    RTM_NEWNEIGH,
+					    NLM_F_MULTI) <= 0) {
 				read_unlock_bh(&tbl->lock);
 				rc = -1;
 				goto out;

@@ -2327,7 +2327,7 @@
 	if (!skb)
 		return;
 
-	if (neigh_fill_info(skb, n, 0, 0, RTM_GETNEIGH) < 0) {
+	if (neigh_fill_info(skb, n, 0, 0, RTM_GETNEIGH, 0) < 0) {
 		kfree_skb(skb);
 		return;
 	}

@@ -2346,7 +2346,7 @@
 	if (!skb)
 		return;
 
-	if (neigh_fill_info(skb, n, 0, 0, RTM_NEWNEIGH) < 0) {
+	if (neigh_fill_info(skb, n, 0, 0, RTM_NEWNEIGH, 0) < 0) {
 		kfree_skb(skb);
 		return;
 	}
net/core/rtnetlink.c (mode:100644) +8 -5
Index: net/core/rtnetlink.c
===================================================================
@@ -178,14 +178,14 @@
 
 
 static int rtnetlink_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
-				 int type, u32 pid, u32 seq, u32 change)
+				 int type, u32 pid, u32 seq, u32 change, 
+				 unsigned int flags)
 {
 	struct ifinfomsg *r;
 	struct nlmsghdr  *nlh;
 	unsigned char	 *b = skb->tail;
 
-	nlh = NLMSG_PUT(skb, pid, seq, type, sizeof(*r));
-	if (pid) nlh->nlmsg_flags |= NLM_F_MULTI;
+	nlh = NLMSG_NEW(skb, pid, seq, type, sizeof(*r), flags);
 	r = NLMSG_DATA(nlh);
 	r->ifi_family = AF_UNSPEC;
 	r->ifi_type = dev->type;

@@ -275,7 +275,10 @@
 	for (dev=dev_base, idx=0; dev; dev = dev->next, idx++) {
 		if (idx < s_idx)
 			continue;
-		if (rtnetlink_fill_ifinfo(skb, dev, RTM_NEWLINK, NETLINK_CB(cb->skb).pid, cb->nlh->nlmsg_seq, 0) <= 0)
+		if (rtnetlink_fill_ifinfo(skb, dev, RTM_NEWLINK,
+					  NETLINK_CB(cb->skb).pid,
+					  cb->nlh->nlmsg_seq, 0,
+					  NLM_F_MULTI) <= 0)
 			break;
 	}
 	read_unlock(&dev_base_lock);

@@ -449,7 +452,7 @@
 	if (!skb)
 		return;
 
-	if (rtnetlink_fill_ifinfo(skb, dev, type, 0, 0, change) < 0) {
+	if (rtnetlink_fill_ifinfo(skb, dev, type, 0, 0, change, 0) < 0) {
 		kfree_skb(skb);
 		return;
 	}
net/decnet/dn_dev.c (mode:100644) +5 -4
Index: net/decnet/dn_dev.c
===================================================================
@@ -716,13 +716,13 @@
 }
 
 static int dn_dev_fill_ifaddr(struct sk_buff *skb, struct dn_ifaddr *ifa,
-				u32 pid, u32 seq, int event)
+				u32 pid, u32 seq, int event, unsigned int flags)
 {
 	struct ifaddrmsg *ifm;
 	struct nlmsghdr *nlh;
 	unsigned char *b = skb->tail;
 
-	nlh = NLMSG_PUT(skb, pid, seq, event, sizeof(*ifm));
+	nlh = NLMSG_NEW(skb, pid, seq, event, sizeof(*ifm), flags);
 	ifm = NLMSG_DATA(nlh);
 
 	ifm->ifa_family = AF_DECnet;

@@ -755,7 +755,7 @@
 		netlink_set_err(rtnl, 0, RTMGRP_DECnet_IFADDR, ENOBUFS);
 		return;
 	}
-	if (dn_dev_fill_ifaddr(skb, ifa, 0, 0, event) < 0) {
+	if (dn_dev_fill_ifaddr(skb, ifa, 0, 0, event, 0) < 0) {
 		kfree_skb(skb);
 		netlink_set_err(rtnl, 0, RTMGRP_DECnet_IFADDR, EINVAL);
 		return;

@@ -790,7 +790,8 @@
 			if (dn_dev_fill_ifaddr(skb, ifa,
 					       NETLINK_CB(cb->skb).pid,
 					       cb->nlh->nlmsg_seq,
-					       RTM_NEWADDR) <= 0)
+					       RTM_NEWADDR,
+					       NLM_F_MULTI) <= 0)
 				goto done;
 		}
 	}
net/decnet/dn_route.c (mode:100644) +6 -5
Index: net/decnet/dn_route.c
===================================================================
@@ -1465,7 +1465,8 @@
 	return dn_route_input_slow(skb);
 }
 
-static int dn_rt_fill_info(struct sk_buff *skb, u32 pid, u32 seq, int event, int nowait)
+static int dn_rt_fill_info(struct sk_buff *skb, u32 pid, u32 seq,
+			   int event, int nowait, unsigned int flags)
 {
 	struct dn_route *rt = (struct dn_route *)skb->dst;
 	struct rtmsg *r;

@@ -1473,9 +1474,8 @@
 	unsigned char *b = skb->tail;
 	struct rta_cacheinfo ci;
 
-	nlh = NLMSG_PUT(skb, pid, seq, event, sizeof(*r));
+	nlh = NLMSG_NEW(skb, pid, seq, event, sizeof(*r), flags);
 	r = NLMSG_DATA(nlh);
-	nlh->nlmsg_flags = (nowait && pid) ? NLM_F_MULTI : 0;
 	r->rtm_family = AF_DECnet;
 	r->rtm_dst_len = 16;
 	r->rtm_src_len = 0;

@@ -1596,7 +1596,7 @@
 
 	NETLINK_CB(skb).dst_pid = NETLINK_CB(in_skb).pid;
 
-	err = dn_rt_fill_info(skb, NETLINK_CB(in_skb).pid, nlh->nlmsg_seq, RTM_NEWROUTE, 0);
+	err = dn_rt_fill_info(skb, NETLINK_CB(in_skb).pid, nlh->nlmsg_seq, RTM_NEWROUTE, 0, 0);
 
 	if (err == 0)
 		goto out_free;

@@ -1644,7 +1644,8 @@
 				continue;
 			skb->dst = dst_clone(&rt->u.dst);
 			if (dn_rt_fill_info(skb, NETLINK_CB(cb->skb).pid,
-					cb->nlh->nlmsg_seq, RTM_NEWROUTE, 1) <= 0) {
+					cb->nlh->nlmsg_seq, RTM_NEWROUTE, 
+					1, NLM_F_MULTI) <= 0) {
 				dst_release(xchg(&skb->dst, NULL));
 				rcu_read_unlock_bh();
 				goto done;
net/decnet/dn_rules.c (mode:100644) +4 -3
Index: net/decnet/dn_rules.c
===================================================================
@@ -342,14 +342,15 @@
 	.notifier_call =	dn_fib_rules_event,
 };
 
-static int dn_fib_fill_rule(struct sk_buff *skb, struct dn_fib_rule *r, struct netlink_callback *cb)
+static int dn_fib_fill_rule(struct sk_buff *skb, struct dn_fib_rule *r,
+			    struct netlink_callback *cb, unsigned int flags)
 {
 	struct rtmsg *rtm;
 	struct nlmsghdr *nlh;
 	unsigned char *b = skb->tail;
 
 
-	nlh = NLMSG_PUT(skb, NETLINK_CREDS(cb->skb)->pid, cb->nlh->nlmsg_seq, RTM_NEWRULE, sizeof(*rtm));
+	nlh = NLMSG_NEW_ANSWER(skb, cb, RTM_NEWRULE, sizeof(*rtm), flags);
 	rtm = NLMSG_DATA(nlh);
 	rtm->rtm_family = AF_DECnet;
 	rtm->rtm_dst_len = r->r_dst_len;

@@ -394,7 +395,7 @@
 	for(r = dn_fib_rules, idx = 0; r; r = r->r_next, idx++) {
 		if (idx < s_idx)
 			continue;
-		if (dn_fib_fill_rule(skb, r, cb) < 0)
+		if (dn_fib_fill_rule(skb, r, cb, NLM_F_MULTI) < 0)
 			break;
 	}
 	read_unlock(&dn_fib_rules_lock);
net/decnet/dn_table.c (mode:100644) +4 -4
Index: net/decnet/dn_table.c
===================================================================
@@ -270,13 +270,13 @@
 
 static int dn_fib_dump_info(struct sk_buff *skb, u32 pid, u32 seq, int event,
                         u8 tb_id, u8 type, u8 scope, void *dst, int dst_len,
-                        struct dn_fib_info *fi)
+                        struct dn_fib_info *fi, unsigned int flags)
 {
         struct rtmsg *rtm;
         struct nlmsghdr *nlh;
         unsigned char *b = skb->tail;
 
-        nlh = NLMSG_PUT(skb, pid, seq, event, sizeof(*rtm));
+        nlh = NLMSG_NEW(skb, pid, seq, event, sizeof(*rtm), flags);
         rtm = NLMSG_DATA(nlh);
         rtm->rtm_family = AF_DECnet;
         rtm->rtm_dst_len = dst_len;

@@ -345,7 +345,7 @@
 
         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) {
+                                DN_FIB_INFO(f), 0) < 0) {
                 kfree_skb(skb);
                 return;
         }

@@ -377,7 +377,7 @@
 				tb->n, 
 				(f->fn_state & DN_S_ZOMBIE) ? 0 : f->fn_type,
 				f->fn_scope, &f->fn_key, dz->dz_order, 
-				f->fn_info) < 0) {
+				f->fn_info, NLM_F_MULTI) < 0) {
 			cb->args[3] = i;
 			return -1;
 		}
net/ipv4/devinet.c (mode:100644) +4 -5
Index: net/ipv4/devinet.c
===================================================================
@@ -1015,14 +1015,13 @@
 };
 
 static int inet_fill_ifaddr(struct sk_buff *skb, struct in_ifaddr *ifa,
-			    u32 pid, u32 seq, int event)
+			    u32 pid, u32 seq, int event, unsigned int flags)
 {
 	struct ifaddrmsg *ifm;
 	struct nlmsghdr  *nlh;
 	unsigned char	 *b = skb->tail;
 
-	nlh = NLMSG_PUT(skb, pid, seq, event, sizeof(*ifm));
-	if (pid) nlh->nlmsg_flags |= NLM_F_MULTI;
+	nlh = NLMSG_NEW(skb, pid, seq, event, sizeof(*ifm), flags);
 	ifm = NLMSG_DATA(nlh);
 	ifm->ifa_family = AF_INET;
 	ifm->ifa_prefixlen = ifa->ifa_prefixlen;

@@ -1075,7 +1074,7 @@
 				continue;
 			if (inet_fill_ifaddr(skb, ifa, NETLINK_CB(cb->skb).pid,
 					     cb->nlh->nlmsg_seq,
-					     RTM_NEWADDR) <= 0) {
+					     RTM_NEWADDR, NLM_F_MULTI) <= 0) {
 				rcu_read_unlock();
 				goto done;
 			}

@@ -1098,7 +1097,7 @@
 
 	if (!skb)
 		netlink_set_err(rtnl, 0, RTMGRP_IPV4_IFADDR, ENOBUFS);
-	else if (inet_fill_ifaddr(skb, ifa, 0, 0, event) < 0) {
+	else if (inet_fill_ifaddr(skb, ifa, 0, 0, event, 0) < 0) {
 		kfree_skb(skb);
 		netlink_set_err(rtnl, 0, RTMGRP_IPV4_IFADDR, EINVAL);
 	} else {
net/ipv4/fib_hash.c (mode:100644) +2 -1
Index: net/ipv4/fib_hash.c
===================================================================
@@ -703,7 +703,8 @@
 					  &f->fn_key,
 					  fz->fz_order,
 					  fa->fa_tos,
-					  fa->fa_info) < 0) {
+					  fa->fa_info,
+					  NLM_F_MULTI) < 0) {
 				cb->args[3] = i;
 				return -1;
 			}
net/ipv4/fib_lookup.h (mode:100644) +2 -1
Index: net/ipv4/fib_lookup.h
===================================================================
@@ -30,7 +30,8 @@
 			struct kern_rta *rta, struct fib_info *fi);
 extern int fib_dump_info(struct sk_buff *skb, u32 pid, u32 seq, int event,
 			 u8 tb_id, u8 type, u8 scope, void *dst,
-			 int dst_len, u8 tos, struct fib_info *fi);
+			 int dst_len, u8 tos, struct fib_info *fi,
+			 unsigned int);
 extern void rtmsg_fib(int event, u32 key, struct fib_alias *fa,
 		      int z, int tb_id,
 		      struct nlmsghdr *n, struct netlink_skb_parms *req);
net/ipv4/fib_rules.c (mode:100644) +4 -3
Index: net/ipv4/fib_rules.c
===================================================================
@@ -367,13 +367,14 @@
 
 static __inline__ int inet_fill_rule(struct sk_buff *skb,
 				     struct fib_rule *r,
-				     struct netlink_callback *cb)
+				     struct netlink_callback *cb,
+				     unsigned int flags)
 {
 	struct rtmsg *rtm;
 	struct nlmsghdr  *nlh;
 	unsigned char	 *b = skb->tail;
 
-	nlh = NLMSG_PUT(skb, NETLINK_CREDS(cb->skb)->pid, cb->nlh->nlmsg_seq, RTM_NEWRULE, sizeof(*rtm));
+	nlh = NLMSG_NEW_ANSWER(skb, cb, RTM_NEWRULE, sizeof(*rtm), flags);
 	rtm = NLMSG_DATA(nlh);
 	rtm->rtm_family = AF_INET;
 	rtm->rtm_dst_len = r->r_dst_len;

@@ -422,7 +423,7 @@
 	for (r=fib_rules, idx=0; r; r = r->r_next, idx++) {
 		if (idx < s_idx)
 			continue;
-		if (inet_fill_rule(skb, r, cb) < 0)
+		if (inet_fill_rule(skb, r, cb, NLM_F_MULTI) < 0)
 			break;
 	}
 	read_unlock(&fib_rules_lock);
net/ipv4/fib_semantics.c (mode:100644) +3 -3
Index: net/ipv4/fib_semantics.c
===================================================================
@@ -286,7 +286,7 @@
 	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) {
+			  fa->fa_info, 0) < 0) {
 		kfree_skb(skb);
 		return;
 	}

@@ -932,13 +932,13 @@
 int
 fib_dump_info(struct sk_buff *skb, u32 pid, u32 seq, int event,
 	      u8 tb_id, u8 type, u8 scope, void *dst, int dst_len, u8 tos,
-	      struct fib_info *fi)
+	      struct fib_info *fi, unsigned int flags)
 {
 	struct rtmsg *rtm;
 	struct nlmsghdr  *nlh;
 	unsigned char	 *b = skb->tail;
 
-	nlh = NLMSG_PUT(skb, pid, seq, event, sizeof(*rtm));
+	nlh = NLMSG_NEW(skb, pid, seq, event, sizeof(*rtm), flags);
 	rtm = NLMSG_DATA(nlh);
 	rtm->rtm_family = AF_INET;
 	rtm->rtm_dst_len = dst_len;
net/ipv4/route.c (mode:100644) +5 -6
Index: net/ipv4/route.c
===================================================================
@@ -2581,7 +2581,7 @@
 }
 
 static int rt_fill_info(struct sk_buff *skb, u32 pid, u32 seq, int event,
-			int nowait)
+			int nowait, unsigned int flags)
 {
 	struct rtable *rt = (struct rtable*)skb->dst;
 	struct rtmsg *r;

@@ -2591,9 +2591,8 @@
 #ifdef CONFIG_IP_MROUTE
 	struct rtattr *eptr;
 #endif
-	nlh = NLMSG_PUT(skb, pid, seq, event, sizeof(*r));
+	nlh = NLMSG_NEW(skb, pid, seq, event, sizeof(*r), flags);
 	r = NLMSG_DATA(nlh);
-	nlh->nlmsg_flags = (nowait && pid) ? NLM_F_MULTI : 0;
 	r->rtm_family	 = AF_INET;
 	r->rtm_dst_len	= 32;
 	r->rtm_src_len	= 0;

@@ -2744,7 +2743,7 @@
 	NETLINK_CB(skb).dst_pid = NETLINK_CB(in_skb).pid;
 
 	err = rt_fill_info(skb, NETLINK_CB(in_skb).pid, nlh->nlmsg_seq,
-				RTM_NEWROUTE, 0);
+				RTM_NEWROUTE, 0, 0);
 	if (!err)
 		goto out_free;
 	if (err < 0) {

@@ -2781,8 +2780,8 @@
 				continue;
 			skb->dst = dst_clone(&rt->u.dst);
 			if (rt_fill_info(skb, NETLINK_CB(cb->skb).pid,
-					 cb->nlh->nlmsg_seq,
-					 RTM_NEWROUTE, 1) <= 0) {
+					 cb->nlh->nlmsg_seq, RTM_NEWROUTE, 
+					 1, NLM_F_MULTI) <= 0) {
 				dst_release(xchg(&skb->dst, NULL));
 				rcu_read_unlock_bh();
 				goto done;
net/ipv6/addrconf.c (mode:100644) +23 -26

157 lines not on this page · raw message

net/ipv6/route.c (mode:100644) +6 -5
Index: net/ipv6/route.c
===================================================================
@@ -1570,7 +1570,8 @@
 			 struct in6_addr *src,
 			 int iif,
 			 int type, u32 pid, u32 seq,
-			 struct nlmsghdr *in_nlh, int prefix)
+			 struct nlmsghdr *in_nlh, int prefix,
+			 unsigned int flags)
 {
 	struct rtmsg *rtm;
 	struct nlmsghdr  *nlh;

@@ -1588,7 +1589,7 @@
 		pid = in_nlh->nlmsg_pid;
 	}
 
-	nlh = NLMSG_PUT(skb, pid, seq, type, sizeof(*rtm));
+	nlh = NLMSG_NEW(skb, pid, seq, type, sizeof(*rtm), flags);
 	rtm = NLMSG_DATA(nlh);
 	rtm->rtm_family = AF_INET6;
 	rtm->rtm_dst_len = rt->rt6i_dst.plen;

@@ -1674,7 +1675,7 @@
 
 	return rt6_fill_node(arg->skb, rt, NULL, NULL, 0, RTM_NEWROUTE,
 		     NETLINK_CB(arg->cb->skb).pid, arg->cb->nlh->nlmsg_seq,
-		     NULL, prefix);
+		     NULL, prefix, NLM_F_MULTI);
 }
 
 static int fib6_dump_node(struct fib6_walker_t *w)

@@ -1822,7 +1823,7 @@
 			    &fl.fl6_dst, &fl.fl6_src,
 			    iif,
 			    RTM_NEWROUTE, NETLINK_CB(in_skb).pid,
-			    nlh->nlmsg_seq, nlh, 0);
+			    nlh->nlmsg_seq, nlh, 0, 0);
 	if (err < 0) {
 		err = -EMSGSIZE;
 		goto out_free;

@@ -1848,7 +1849,7 @@
 		netlink_set_err(rtnl, 0, RTMGRP_IPV6_ROUTE, ENOBUFS);
 		return;
 	}
-	if (rt6_fill_node(skb, rt, NULL, NULL, 0, event, 0, 0, nlh, 0) < 0) {
+	if (rt6_fill_node(skb, rt, NULL, NULL, 0, event, 0, 0, nlh, 0, 0) < 0) {
 		kfree_skb(skb);
 		netlink_set_err(rtnl, 0, RTMGRP_IPV6_ROUTE, EINVAL);
 		return;
author
#10 lore raw

Re: [PATCH 2/2] [NETLINK] Correctly set NLM_F_MULTI without checking the pid

Ok, looks good. Dave please apply this one instead of the one i posted this morning - it incorporates the name change as well. Incase its not clear - 1/2 in this patch set is also fine.

cheers,
jamal

Quoted 8 lines · Thomas Graf

This patch rectifies some rtnetlink message builders that derive the flags from the pid. It is now explicit like the other cases which get it right. Also fixes half a dozen dumpers which did not set NLM_F_MULTI at all.

Signed-off-by Jamal Hadi Salim <[email protected]>
Signed-off-by Thomas Graf <[email protected]>
maintainer
#11 lore raw

Re: [PATCH 2/2] [NETLINK] Correctly set NLM_F_MULTI without checking the pid

In reply to Thomas Graf

Both patches applied to my net-2.6.13 tree, it should show up on kernel.org shortly.

author
#12 lore raw
In reply to Thomas Graf
Quoted 2 lines · Thomas Graf

I took your patch, converted it to use NLMSG_NEW and finished converting all the modules that you started.

Thats the patchset 1/2 and 2/2, correct?

Quoted 3 lines

Yet unconverted
are: netfilter, net/sched/ tcp_diag, xfrm, kernel/audit.

I didnt find any issues with these as far as PID extrapolation.

Quoted 5 lines

I also fixed some uses of NL_CREDS() to fetch the pid for message generation which is wrong. Various dumping procedures did not set the multi flag at all, I fixed all of them.

Do not apply these patches yet, they're way too intrusive

Do you mean the two patches? Those looked fine to me;
i.e its mostly the patch i posted this morning with the new Macros, no?

cheers,
jamal

#13 lore raw

* jamal <[email protected]> 2005-05-27 21:12

Quoted 5 lines
Quoted 3 lines

Yet unconverted
are: netfilter, net/sched/ tcp_diag, xfrm, kernel/audit.

I didnt find any issues with these as far as PID extrapolation.

I should have written, yet unchecked.

Quoted 7 lines
Quoted 5 lines

I also fixed some uses of NL_CREDS() to fetch the pid for message generation which is wrong. Various dumping procedures did not set the multi flag at all, I fixed all of them.

Do not apply these patches yet, they're way too intrusive

Do you mean the two patches? Those looked fine to me;

Yes, those two patches. My part comes directly out of my fingers and is only compile tested. I didn't even run them yet.

Quoted 1 line

i.e its mostly the patch i posted this morning with the new Macros, no?

I went through all users of NLMSG_PUT in the modules you touched and checked if they properly set NLM_F_MULTI and used the new macro when appropriate.

Anyways, I want to be careful with this, there might be userspace apps which rely on pid being filled in from siocb->scm->creds or userspace apps which don't expect the NLM_F_MULTI flag even when they should so we should check with all parties involved.

author
#14 lore raw
Quoted 9 lines · Thomas Graf

* jamal <[email protected]> 2005-05-27 21:12

Quoted 5 lines
Quoted 3 lines

Yet unconverted
are: netfilter, net/sched/ tcp_diag, xfrm, kernel/audit.

I didnt find any issues with these as far as PID extrapolation.

I should have written, yet unchecked.

I did check netfilter, net/sched/ tcp_diag, xfrm and they do things right.

Quoted 7 lines
Quoted 3 lines
Quoted 1 line

Do not apply these patches yet, they're way too intrusive

Do you mean the two patches? Those looked fine to me;

Yes, those two patches. My part comes directly out of my fingers and is only compile tested. I didn't even run them yet.

Hrm - you should probably have said that in the patch text ;-> I did test the one i sent - basic things and by code inspection saw little risk as well. Perhaps we should push mine then you can add on top of it.

Quoted 11 lines
Quoted 1 line

i.e its mostly the patch i posted this morning with the new Macros, no?

I went through all users of NLMSG_PUT in the modules you touched and checked if they properly set NLM_F_MULTI and used the new macro when appropriate.

Anyways, I want to be careful with this, there might be userspace apps which rely on pid being filled in from siocb->scm->creds or userspace apps which don't expect the NLM_F_MULTI flag even when they should so we should check with all parties involved.

I dont see any issues with my original patch or for that matter yours except for now that you are saying you havent compiled it even ;->. All dumps MUST set NLM_F_MULTI - thats the main change really. All non-dump paths may set NLM_F_MULTI, and from that angle, if theres any bugs in the old code they are maintained ;->

So how about we push my original patch instead?

cheers,
jamal

#15 lore raw

* jamal <[email protected]> 2005-05-27 21:48

Quoted 2 lines

I did check netfilter, net/sched/ tcp_diag, xfrm and they do things right.

OK.

Quoted 1 line

Hrm - you should probably have said that in the patch text ;->

I wanted to have some time for testing today without the need to resubmit the patches. ;->

Quoted 3 lines

I did test the one i sent - basic things and by code inspection saw little risk as well. Perhaps we should push mine then you can add on top of it.

I just tested both patches for about 30 minutes. Everything OK. Feel free to apply the patches.

author
#16 lore raw
Quoted 3 lines · Thomas Graf

I wanted to have some time for testing today without the need to resubmit the patches. ;->

fair enough ;->

Quoted 2 lines

I just tested both patches for about 30 minutes. Everything OK. Feel free to apply the patches.

We have another problem. What tree is this against? Both patches have failures patching against latest davem and linus git trees.

I am kind of distracted right now but I could fix this later.

cheers,
jamal

#17 lore raw

* jamal <[email protected]> 2005-05-28 12:00

Quoted 5 lines
Quoted 2 lines

I just tested both patches for about 30 minutes. Everything OK. Feel free to apply the patches.

We have another problem. What tree is this against? Both patches have failures patching against latest davem and linus git trees.

It's against my davem-pending tree which should reflect the current state of davem's tree so they _should_ apply on his side. In this specific case you're probably missing the neighbour table patches.

Basically I produce diffs this way:

separate tree per patchset -> merge into testing tree -> testing period -. merge into $person-pending -> extraction of patches and submission.

I was trying to get some more quality control into my workflow, so I started testing patches for 3-4 weeks to avoid fallouts but apparently this is failing miserably at the moment due to some broken scripts of mine.

author
#18 lore raw
Quoted 10 lines · Thomas Graf

* jamal <[email protected]> 2005-05-28 12:00

Quoted 5 lines
Quoted 2 lines

I just tested both patches for about 30 minutes. Everything OK. Feel free to apply the patches.

We have another problem. What tree is this against? Both patches have failures patching against latest davem and linus git trees.

It's against my davem-pending tree which should reflect the current state of davem's tree so they _should_ apply on his side. In this specific case you're probably missing the neighbour table patches.

Now you really have to send some swiss chocolate over ;-> It still doesnt apply to Davems latest tree.

Quoted 6 lines

Basically I produce diffs this way:

separate tree per patchset -> merge into testing tree -> testing period -. merge into $person-pending -> extraction of patches and submission.

Almost very close to what i do - which could be improved or you keep a few hundred trees. Jeff Garzik (some email to netdev with a HOWTO ) was talking about some new scheme where instead of trees you use branches of the same tree. I didnt quiet follow his trick.

Quoted 4 lines

I was trying to get some more quality control into my workflow, so I started testing patches for 3-4 weeks to avoid fallouts but apparently this is failing miserably at the moment due to some broken scripts of mine.

I think the problem in this case was you had already commited to that tree your changes that you sent to Dave. And the tree you were using as a base for generating the patch was only something you and Dave know about.

cheers,
jamal

#21 lore raw

* David S. Miller <[email protected]> 2005-05-31 14:43

Quoted 10 lines

From: jamal <[email protected]>
Date: Fri, 27 May 2005 08:32:37 -0400

Quoted 5 lines

This patch rectifies some rtnetlink message builders that derive the flags from the pid. It is now explicit like the other cases which get it right.

Signed-off-by Jamal Hadi Salim <[email protected]>

Please use explicit "unsigned int flags" instead of "unsigned flags".

I converted this already in the two patches later in the thread.

maintainer
#22 lore raw

From: Thomas Graf <[email protected]>
Date: Wed, 1 Jun 2005 00:26:46 +0200

Quoted 3 lines
Quoted 1 line

Please use explicit "unsigned int flags" instead of "unsigned flags".

I converted this already in the two patches later in the thread.

I see, thanks for pointing this out.

author
#23 lore raw

PATCH: explicit typing WAS(Re: PATCH: rtnetlink explicit flags setting

Quoted 9 lines · David S. Miller

From: Thomas Graf <[email protected]>
Date: Wed, 1 Jun 2005 00:26:46 +0200

Quoted 3 lines
Quoted 1 line

Please use explicit "unsigned int flags" instead of "unsigned flags".

I converted this already in the two patches later in the thread.

I see, thanks for pointing this out.

If you want to do it right, it should be a u16 actually ;->

In any case since we are being gracious - lets fix where i cutnpasted it from using TheLinuxWay ;->

-------------
This patch converts "unsigned flags" to use more explict types like u16 instead and incrementally introduces NLMSG_NEW().

Signed-off-by Jamal Hadi Salim <[email protected]>

cheers,
jamal

net/ipv6/addrconf.c: needs update
net/sched/act_api.c: needs update
net/sched/cls_api.c: needs update
net/sched/sch_api.c: needs update
Index: net/ipv6/addrconf.c
===================================================================

net/ipv6/addrconf.c (mode:100644) +5 -5
@@ -131,7 +131,7 @@
 
 static int addrconf_ifdown(struct net_device *dev, int how);
 
-static void addrconf_dad_start(struct inet6_ifaddr *ifp, int flags);
+static void addrconf_dad_start(struct inet6_ifaddr *ifp, u32 flags);
 static void addrconf_dad_timer(unsigned long data);
 static void addrconf_dad_completed(struct inet6_ifaddr *ifp);
 static void addrconf_rs_timer(unsigned long data);

@@ -491,7 +491,7 @@
 
 static struct inet6_ifaddr *
 ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr, int pfxlen,
-	      int scope, unsigned flags)
+	      int scope, u32 flags)
 {
 	struct inet6_ifaddr *ifa = NULL;
 	struct rt6_info *rt;

@@ -1319,7 +1319,7 @@
 
 static void
 addrconf_prefix_route(struct in6_addr *pfx, int plen, struct net_device *dev,
-		      unsigned long expires, unsigned flags)
+		      unsigned long expires, u32 flags)
 {
 	struct in6_rtmsg rtmsg;
 

@@ -2228,7 +2228,7 @@
 /*
  *	Duplicate Address Detection
  */
-static void addrconf_dad_start(struct inet6_ifaddr *ifp, int flags)
+static void addrconf_dad_start(struct inet6_ifaddr *ifp, u32 flags)
 {
 	struct inet6_dev *idev = ifp->idev;
 	struct net_device *dev = idev->dev;

@@ -2670,7 +2670,7 @@
 }
 
 static int inet6_fill_ifmcaddr(struct sk_buff *skb, struct ifmcaddr6 *ifmca,
-				u32 pid, u32 seq, int event, unsigned flags)
+				u32 pid, u32 seq, int event, u16 flags)
 {
 	struct ifaddrmsg *ifm;
 	struct nlmsghdr  *nlh;
net/sched/act_api.c (mode:100644) +5 -6
Index: net/sched/act_api.c
===================================================================
@@ -428,15 +428,15 @@
 
 static int
 tca_get_fill(struct sk_buff *skb, struct tc_action *a, u32 pid, u32 seq,
-             unsigned flags, int event, int bind, int ref)
+             u16 flags, int event, int bind, int ref)
 {
 	struct tcamsg *t;
 	struct nlmsghdr *nlh;
 	unsigned char *b = skb->tail;
 	struct rtattr *x;
 
-	nlh = NLMSG_PUT(skb, pid, seq, event, sizeof(*t));
-	nlh->nlmsg_flags = flags;
+	nlh = NLMSG_NEW(skb, pid, seq, event, sizeof(*t), flags);
+
 	t = NLMSG_DATA(nlh);
 	t->tca_family = AF_UNSPEC;
 	

@@ -669,7 +669,7 @@
 }
 
 static int tcf_add_notify(struct tc_action *a, u32 pid, u32 seq, int event,
-                          unsigned flags)
+                          u16 flags)
 {
 	struct tcamsg *t;
 	struct nlmsghdr *nlh;

@@ -684,8 +684,7 @@
 
 	b = (unsigned char *)skb->tail;
 
-	nlh = NLMSG_PUT(skb, pid, seq, event, sizeof(*t));
-	nlh->nlmsg_flags = flags;
+	nlh = NLMSG_NEW(skb, pid, seq, event, sizeof(*t), flags);
 	t = NLMSG_DATA(nlh);
 	t->tca_family = AF_UNSPEC;
net/sched/cls_api.c (mode:100644) +2 -3
Index: net/sched/cls_api.c
===================================================================
@@ -322,14 +322,13 @@
 
 static int
 tcf_fill_node(struct sk_buff *skb, struct tcf_proto *tp, unsigned long fh,
-	      u32 pid, u32 seq, unsigned flags, int event)
+	      u32 pid, u32 seq, u16 flags, int event)
 {
 	struct tcmsg *tcm;
 	struct nlmsghdr  *nlh;
 	unsigned char	 *b = skb->tail;
 
-	nlh = NLMSG_PUT(skb, pid, seq, event, sizeof(*tcm));
-	nlh->nlmsg_flags = flags;
+	nlh = NLMSG_NEW(skb, pid, seq, event, sizeof(*tcm), flags);
 	tcm = NLMSG_DATA(nlh);
 	tcm->tcm_family = AF_UNSPEC;
 	tcm->tcm_ifindex = tp->q->dev->ifindex;
net/sched/sch_api.c (mode:100644) +4 -6
Index: net/sched/sch_api.c
===================================================================
@@ -760,15 +760,14 @@
 }
 
 static int tc_fill_qdisc(struct sk_buff *skb, struct Qdisc *q, u32 clid,
-			 u32 pid, u32 seq, unsigned flags, int event)
+			 u32 pid, u32 seq, u16 flags, int event)
 {
 	struct tcmsg *tcm;
 	struct nlmsghdr  *nlh;
 	unsigned char	 *b = skb->tail;
 	struct gnet_dump d;
 
-	nlh = NLMSG_PUT(skb, pid, seq, event, sizeof(*tcm));
-	nlh->nlmsg_flags = flags;
+	nlh = NLMSG_NEW(skb, pid, seq, event, sizeof(*tcm), flags);
 	tcm = NLMSG_DATA(nlh);
 	tcm->tcm_family = AF_UNSPEC;
 	tcm->tcm_ifindex = q->dev->ifindex;

@@ -997,7 +996,7 @@
 
 static int tc_fill_tclass(struct sk_buff *skb, struct Qdisc *q,
 			  unsigned long cl,
-			  u32 pid, u32 seq, unsigned flags, int event)
+			  u32 pid, u32 seq, u16 flags, int event)
 {
 	struct tcmsg *tcm;
 	struct nlmsghdr  *nlh;

@@ -1005,8 +1004,7 @@
 	struct gnet_dump d;
 	struct Qdisc_class_ops *cl_ops = q->ops->cl_ops;
 
-	nlh = NLMSG_PUT(skb, pid, seq, event, sizeof(*tcm));
-	nlh->nlmsg_flags = flags;
+	nlh = NLMSG_NEW(skb, pid, seq, event, sizeof(*tcm), flags);
 	tcm = NLMSG_DATA(nlh);
 	tcm->tcm_family = AF_UNSPEC;
 	tcm->tcm_ifindex = q->dev->ifindex;
author
#24 lore raw

Re: PATCH: explicit typing WAS(Re: PATCH: rtnetlink explicit flags setting

I should say this patch is against net-2.6.13.git as of 6am this morning.

cheers,
jamal

Quoted 6 lines · jamal

-------------
This patch converts "unsigned flags" to use more explict types like u16 instead and incrementally introduces NLMSG_NEW().

Signed-off-by Jamal Hadi Salim <[email protected]>