Skip to contentSkip to conversation

Get rid of RTA*() macros

PATCH In review Thomas Graf 4 participants 15 messages Jun 27, 2012

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

cover Get rid of RTA*() macros

This patchset gets rid of all the RTA_PUT() and RTA_GET() macros and makes use of the type-safe netlink API variants where applicable.

I did my best to test these patches but I do not own any decnet hardware so the decnet part is compile tested only.

Thomas Graf (7):
unix_diag: Do not use RTA_PUT() macros
sock_diag: Do not use RTA_PUT() macros
inet_diag: Do not use RTA_PUT() macros
ipmr: Do not use RTA_PUT() macros
ip6mr: Do not use RTA_PUT() macros
decnet: Do not use RTA_PUT() macros
netlink: Get rid of obsolete rtnetlink macros

 include/linux/rtnetlink.h |  129 ---------------------------------------------
 net/core/rtnetlink.c      |   13 -----
 net/core/sock_diag.c      |   12 +---
 net/decnet/dn_fib.c       |    8 +++
 net/decnet/dn_route.c     |   61 +++++++++++++--------
 net/decnet/dn_table.c     |   69 ++++++++++++++----------
 net/ipv4/inet_diag.c      |  112 ++++++++++++++++++--------------------
 net/ipv4/ipmr.c           |   28 +++++-----
 net/ipv6/ip6mr.c          |    5 +-
 net/unix/diag.c           |   80 ++++++++++++----------------
 10 files changed, 193 insertions(+), 324 deletions(-)
Signature · 1 line
1.7.7.6
#1 lore raw

PATCH 1/7 unix_diag: Do not use RTA_PUT() macros

Also, no need to trim on nlmsg_put() failure, nothing has been added yet. We also want to use nlmsg_end(), nlmsg_new() and nlmsg_free().

---
 net/unix/diag.c |   80 ++++++++++++++++++++++--------------------------------
 1 files changed, 33 insertions(+), 47 deletions(-)
net/unix/diag.c +33 -47
index 977ca31..a74864e 100644
@@ -8,40 +8,31 @@
 #include <net/af_unix.h>
 #include <net/tcp_states.h>
 
-#define UNIX_DIAG_PUT(skb, attrtype, attrlen) \
-	RTA_DATA(__RTA_PUT(skb, attrtype, attrlen))
-
 static int sk_diag_dump_name(struct sock *sk, struct sk_buff *nlskb)
 {
 	struct unix_address *addr = unix_sk(sk)->addr;
-	char *s;
-
-	if (addr) {
-		s = UNIX_DIAG_PUT(nlskb, UNIX_DIAG_NAME, addr->len - sizeof(short));
-		memcpy(s, addr->name->sun_path, addr->len - sizeof(short));
-	}
 
-	return 0;
+	if (!addr)
+		return 0;
 
-rtattr_failure:
-	return -EMSGSIZE;
+	return nla_put(nlskb, UNIX_DIAG_NAME, addr->len - sizeof(short),
+		       addr->name->sun_path);
 }
 
 static int sk_diag_dump_vfs(struct sock *sk, struct sk_buff *nlskb)
 {
 	struct dentry *dentry = unix_sk(sk)->path.dentry;
-	struct unix_diag_vfs *uv;
 
 	if (dentry) {
-		uv = UNIX_DIAG_PUT(nlskb, UNIX_DIAG_VFS, sizeof(*uv));
-		uv->udiag_vfs_ino = dentry->d_inode->i_ino;
-		uv->udiag_vfs_dev = dentry->d_sb->s_dev;
+		struct unix_diag_vfs uv = {
+			.udiag_vfs_ino = dentry->d_inode->i_ino,
+			.udiag_vfs_dev = dentry->d_sb->s_dev,
+		};
+
+		return nla_put(nlskb, UNIX_DIAG_VFS, sizeof(uv), &uv);
 	}
 
 	return 0;
-
-rtattr_failure:
-	return -EMSGSIZE;
 }
 
 static int sk_diag_dump_peer(struct sock *sk, struct sk_buff *nlskb)
@@ -56,24 +47,28 @@ static int sk_diag_dump_peer(struct sock *sk, struct sk_buff *nlskb)
 		unix_state_unlock(peer);
 		sock_put(peer);
 
-		RTA_PUT_U32(nlskb, UNIX_DIAG_PEER, ino);
+		return nla_put_u32(nlskb, UNIX_DIAG_PEER, ino);
 	}
 
 	return 0;
-rtattr_failure:
-	return -EMSGSIZE;
 }
 
 static int sk_diag_dump_icons(struct sock *sk, struct sk_buff *nlskb)
 {
 	struct sk_buff *skb;
+	struct nlattr *attr;
 	u32 *buf;
 	int i;
 
 	if (sk->sk_state == TCP_LISTEN) {
 		spin_lock(&sk->sk_receive_queue.lock);
-		buf = UNIX_DIAG_PUT(nlskb, UNIX_DIAG_ICONS,
-				sk->sk_receive_queue.qlen * sizeof(u32));
+
+		attr = nla_reserve(nlskb, UNIX_DIAG_ICONS,
+				   sk->sk_receive_queue.qlen * sizeof(u32));
+		if (!attr)
+			goto errout;
+
+		buf = nla_data(attr);
 		i = 0;
 		skb_queue_walk(&sk->sk_receive_queue, skb) {
 			struct sock *req, *peer;
@@ -94,45 +89,38 @@ static int sk_diag_dump_icons(struct sock *sk, struct sk_buff *nlskb)
 
 	return 0;
 
-rtattr_failure:
+errout:
 	spin_unlock(&sk->sk_receive_queue.lock);
 	return -EMSGSIZE;
 }
 
 static int sk_diag_show_rqlen(struct sock *sk, struct sk_buff *nlskb)
 {
-	struct unix_diag_rqlen *rql;
-
-	rql = UNIX_DIAG_PUT(nlskb, UNIX_DIAG_RQLEN, sizeof(*rql));
+	struct unix_diag_rqlen rql;
 
 	if (sk->sk_state == TCP_LISTEN) {
-		rql->udiag_rqueue = sk->sk_receive_queue.qlen;
-		rql->udiag_wqueue = sk->sk_max_ack_backlog;
+		rql.udiag_rqueue = sk->sk_receive_queue.qlen;
+		rql.udiag_wqueue = sk->sk_max_ack_backlog;
 	} else {
-		rql->udiag_rqueue = (__u32)unix_inq_len(sk);
-		rql->udiag_wqueue = (__u32)unix_outq_len(sk);
+		rql.udiag_rqueue = (u32) unix_inq_len(sk);
+		rql.udiag_wqueue = (u32) unix_outq_len(sk);
 	}
 
-	return 0;
-
-rtattr_failure:
-	return -EMSGSIZE;
+	return nla_put(nlskb, UNIX_DIAG_RQLEN, sizeof(rql), &rql);
 }
 
 static int sk_diag_fill(struct sock *sk, struct sk_buff *skb, struct unix_diag_req *req,
 		u32 pid, u32 seq, u32 flags, int sk_ino)
 {
-	unsigned char *b = skb_tail_pointer(skb);
 	struct nlmsghdr *nlh;
 	struct unix_diag_msg *rep;
 
-	nlh = nlmsg_put(skb, pid, seq, SOCK_DIAG_BY_FAMILY, sizeof(*rep), 0);
+	nlh = nlmsg_put(skb, pid, seq, SOCK_DIAG_BY_FAMILY, sizeof(*rep),
+			flags);
 	if (!nlh)
-		goto out_nlmsg_trim;
-	nlh->nlmsg_flags = flags;
+		return -EMSGSIZE;
 
 	rep = nlmsg_data(nlh);
-
 	rep->udiag_family = AF_UNIX;
 	rep->udiag_type = sk->sk_type;
 	rep->udiag_state = sk->sk_state;
@@ -163,11 +151,10 @@ static int sk_diag_fill(struct sock *sk, struct sk_buff *skb, struct unix_diag_r
 	    sock_diag_put_meminfo(sk, skb, UNIX_DIAG_MEMINFO))
 		goto out_nlmsg_trim;
 
-	nlh->nlmsg_len = skb_tail_pointer(skb) - b;
-	return skb->len;
+	return nlmsg_end(skb, nlh);
 
 out_nlmsg_trim:
-	nlmsg_trim(skb, b);
+	nlmsg_cancel(skb, nlh);
 	return -EMSGSIZE;
 }
 
@@ -272,15 +259,14 @@ static int unix_diag_get_exact(struct sk_buff *in_skb,
 	extra_len = 256;
 again:
 	err = -ENOMEM;
-	rep = alloc_skb(NLMSG_SPACE((sizeof(struct unix_diag_msg) + extra_len)),
-			GFP_KERNEL);
+	rep = nlmsg_new(sizeof(struct unix_diag_msg) + extra_len, GFP_KERNEL);
 	if (!rep)
 		goto out;
 
 	err = sk_diag_fill(sk, rep, req, NETLINK_CB(in_skb).pid,
 			   nlh->nlmsg_seq, 0, req->udiag_ino);
 	if (err < 0) {
-		kfree_skb(rep);
+		nlmsg_free(rep);
 		extra_len += 256;
 		if (extra_len >= PAGE_SIZE)
 			goto out;
Signature · 1 line
1.7.7.6
#2 lore raw

PATCH 2/7 sock_diag: Do not use RTA_PUT() macros

In reply to Thomas Graf
---
 net/core/sock_diag.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)
net/core/sock_diag.c +3 -9
index 0d934ce..ff2967a 100644
@@ -4,7 +4,6 @@
 #include <net/netlink.h>
 #include <net/net_namespace.h>
 #include <linux/module.h>
-#include <linux/rtnetlink.h>
 #include <net/sock.h>
 
 #include <linux/inet_diag.h>
@@ -35,9 +34,7 @@ EXPORT_SYMBOL_GPL(sock_diag_save_cookie);
 
 int sock_diag_put_meminfo(struct sock *sk, struct sk_buff *skb, int attrtype)
 {
-	__u32 *mem;
-
-	mem = RTA_DATA(__RTA_PUT(skb, attrtype, SK_MEMINFO_VARS * sizeof(__u32)));
+	u32 mem[SK_MEMINFO_VARS];
 
 	mem[SK_MEMINFO_RMEM_ALLOC] = sk_rmem_alloc_get(sk);
 	mem[SK_MEMINFO_RCVBUF] = sk->sk_rcvbuf;
@@ -48,10 +45,7 @@ int sock_diag_put_meminfo(struct sock *sk, struct sk_buff *skb, int attrtype)
 	mem[SK_MEMINFO_OPTMEM] = atomic_read(&sk->sk_omem_alloc);
 	mem[SK_MEMINFO_BACKLOG] = sk->sk_backlog.len;
 
-	return 0;
-
-rtattr_failure:
-	return -EMSGSIZE;
+	return nla_put(skb, attrtype, sizeof(mem), &mem);
 }
 EXPORT_SYMBOL_GPL(sock_diag_put_meminfo);
 
@@ -121,7 +115,7 @@ static inline void sock_diag_unlock_handler(const struct sock_diag_handler *h)
 static int __sock_diag_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 {
 	int err;
-	struct sock_diag_req *req = NLMSG_DATA(nlh);
+	struct sock_diag_req *req = nlmsg_data(nlh);
 	const struct sock_diag_handler *hndl;
 
 	if (nlmsg_len(nlh) < sizeof(*req))
Signature · 1 line
1.7.7.6
#3 lore raw

RE: [PATCH 2/7] sock_diag: Do not use RTA_PUT() macros

net/core/sock_diag.c EXPORT_SYMBOL_GPL(sock_diag_save_cookie); · line 41

Quoted 9 lines
net/core/sock_diag.c +0 -3
@@ -35,9 +34,7 @@ EXPORT_SYMBOL_GPL(sock_diag_save_cookie);
 
 int sock_diag_put_meminfo(struct sock *sk, struct sk_buff 
 *skb, int attrtype)
 {
-	__u32 *mem;
-
-	mem = RTA_DATA(__RTA_PUT(skb, attrtype, SK_MEMINFO_VARS *

sizeof(__u32)));

net/core/sock_diag.c EXPORT_SYMBOL_GPL(sock_diag_save_cookie); · line 37

Quoted 1 line
net/core/sock_diag.c +1 -0
+	u32 mem[SK_MEMINFO_VARS];

Isn't that likely to blow the kernel stack?

David

maintainer
#4 lore raw

Re: [PATCH 2/7] sock_diag: Do not use RTA_PUT() macros

From: "David Laight" <[email protected]>
Date: Wed, 27 Jun 2012 11:00:30 +0100

Quoted 10 lines

net/core/sock_diag.c EXPORT_SYMBOL_GPL(sock_diag_save_cookie); · line 41

Quoted 9 lines
net/core/sock_diag.c +0 -3
@@ -35,9 +34,7 @@ EXPORT_SYMBOL_GPL(sock_diag_save_cookie);
 
 int sock_diag_put_meminfo(struct sock *sk, struct sk_buff 
 *skb, int attrtype)
 {
-	__u32 *mem;
-
-	mem = RTA_DATA(__RTA_PUT(skb, attrtype, SK_MEMINFO_VARS *

sizeof(__u32)));

Quoted 3 lines

net/core/sock_diag.c EXPORT_SYMBOL_GPL(sock_diag_save_cookie); · line 37

Quoted 1 line
net/core/sock_diag.c +1 -0
+	u32 mem[SK_MEMINFO_VARS];

Isn't that likely to blow the kernel stack?

8 * sizeof(u32)? Surely not.

#5 lore raw

Re: [PATCH 2/7] sock_diag: Do not use RTA_PUT() macros

In reply to David Laight
Quoted 10 lines · David Laight

net/core/sock_diag.c EXPORT_SYMBOL_GPL(sock_diag_save_cookie); · line 41

Quoted 9 lines
net/core/sock_diag.c +0 -3
@@ -35,9 +34,7 @@ EXPORT_SYMBOL_GPL(sock_diag_save_cookie);
 
 int sock_diag_put_meminfo(struct sock *sk, struct sk_buff 
 *skb, int attrtype)
 {
-	__u32 *mem;
-
-	mem = RTA_DATA(__RTA_PUT(skb, attrtype, SK_MEMINFO_VARS *

sizeof(__u32)));

Quoted 3 lines

net/core/sock_diag.c EXPORT_SYMBOL_GPL(sock_diag_save_cookie); · line 37

Quoted 1 line
net/core/sock_diag.c +1 -0
+	u32 mem[SK_MEMINFO_VARS];

Isn't that likely to blow the kernel stack?

SK_MEMINFO_VARS = 8, so no. I can change it to use nla_reserve() if wasting 32 bytes on the stack is an issue.

#6 lore raw

PATCH 3/7 inet_diag: Do not use RTA_PUT() macros

In reply to Thomas Graf

Also, no need to trim on nlmsg_put() failure, nothing has been added yet. We also want to use nlmsg_end(), nlmsg_new() and nlmsg_free().

---
 net/ipv4/inet_diag.c |  112 +++++++++++++++++++++++--------------------------
 1 files changed, 53 insertions(+), 59 deletions(-)
net/ipv4/inet_diag.c +53 -59
index 27640e7..38064a2 100644
@@ -46,9 +46,6 @@ struct inet_diag_entry {
 	u16 userlocks;
 };
 
-#define INET_DIAG_PUT(skb, attrtype, attrlen) \
-	RTA_DATA(__RTA_PUT(skb, attrtype, attrlen))
-
 static DEFINE_MUTEX(inet_diag_table_mutex);
 
 static const struct inet_diag_handler *inet_diag_lock_handler(int proto)
@@ -78,28 +75,22 @@ int inet_sk_diag_fill(struct sock *sk, struct inet_connection_sock *icsk,
 	const struct inet_sock *inet = inet_sk(sk);
 	struct inet_diag_msg *r;
 	struct nlmsghdr  *nlh;
+	struct nlattr *attr;
 	void *info = NULL;
-	struct inet_diag_meminfo  *minfo = NULL;
-	unsigned char	 *b = skb_tail_pointer(skb);
 	const struct inet_diag_handler *handler;
 	int ext = req->idiag_ext;
 
 	handler = inet_diag_table[req->sdiag_protocol];
 	BUG_ON(handler == NULL);
 
-	nlh = nlmsg_put(skb, pid, seq, unlh->nlmsg_type, sizeof(*r), 0);
-	if (!nlh) {
-		nlmsg_trim(skb, b);
+	nlh = nlmsg_put(skb, pid, seq, unlh->nlmsg_type, sizeof(*r),
+			nlmsg_flags);
+	if (!nlh)
 		return -EMSGSIZE;
-	}
-	nlh->nlmsg_flags = nlmsg_flags;
 
 	r = nlmsg_data(nlh);
 	BUG_ON(sk->sk_state == TCP_TIME_WAIT);
 
-	if (ext & (1 << (INET_DIAG_MEMINFO - 1)))
-		minfo = INET_DIAG_PUT(skb, INET_DIAG_MEMINFO, sizeof(*minfo));
-
 	r->idiag_family = sk->sk_family;
 	r->idiag_state = sk->sk_state;
 	r->idiag_timer = 0;
@@ -117,7 +108,8 @@ int inet_sk_diag_fill(struct sock *sk, struct inet_connection_sock *icsk,
 	 * hence this needs to be included regardless of socket family.
 	 */
 	if (ext & (1 << (INET_DIAG_TOS - 1)))
-		RTA_PUT_U8(skb, INET_DIAG_TOS, inet->tos);
+		if (nla_put_u8(skb, INET_DIAG_TOS, inet->tos) < 0)
+			goto errout;
 
 #if IS_ENABLED(CONFIG_IPV6)
 	if (r->idiag_family == AF_INET6) {
@@ -125,24 +117,31 @@ int inet_sk_diag_fill(struct sock *sk, struct inet_connection_sock *icsk,
 
 		*(struct in6_addr *)r->id.idiag_src = np->rcv_saddr;
 		*(struct in6_addr *)r->id.idiag_dst = np->daddr;
+
 		if (ext & (1 << (INET_DIAG_TCLASS - 1)))
-			RTA_PUT_U8(skb, INET_DIAG_TCLASS, np->tclass);
+			if (nla_put_u8(skb, INET_DIAG_TCLASS, np->tclass) < 0)
+				goto errout;
 	}
 #endif
 
 	r->idiag_uid = sock_i_uid(sk);
 	r->idiag_inode = sock_i_ino(sk);
 
-	if (minfo) {
-		minfo->idiag_rmem = sk_rmem_alloc_get(sk);
-		minfo->idiag_wmem = sk->sk_wmem_queued;
-		minfo->idiag_fmem = sk->sk_forward_alloc;
-		minfo->idiag_tmem = sk_wmem_alloc_get(sk);
+	if (ext & (1 << (INET_DIAG_MEMINFO - 1))) {
+		struct inet_diag_meminfo minfo = {
+			.idiag_rmem = sk_rmem_alloc_get(sk),
+			.idiag_wmem = sk->sk_wmem_queued,
+			.idiag_fmem = sk->sk_forward_alloc,
+			.idiag_tmem = sk_wmem_alloc_get(sk),
+		};
+
+		if (nla_put(skb, INET_DIAG_MEMINFO, sizeof(minfo), &minfo) < 0)
+			goto errout;
 	}
 
 	if (ext & (1 << (INET_DIAG_SKMEMINFO - 1)))
 		if (sock_diag_put_meminfo(sk, skb, INET_DIAG_SKMEMINFO))
-			goto rtattr_failure;
+			goto errout;
 
 	if (icsk == NULL) {
 		handler->idiag_get_info(sk, r, NULL);
@@ -169,16 +168,20 @@ int inet_sk_diag_fill(struct sock *sk, struct inet_connection_sock *icsk,
 	}
 #undef EXPIRES_IN_MS
 
-	if (ext & (1 << (INET_DIAG_INFO - 1)))
-		info = INET_DIAG_PUT(skb, INET_DIAG_INFO, sizeof(struct tcp_info));
+	if (ext & (1 << (INET_DIAG_INFO - 1))) {
+		attr = nla_reserve(skb, INET_DIAG_INFO,
+				   sizeof(struct tcp_info));
+		if (!attr)
+			goto errout;
 
-	if ((ext & (1 << (INET_DIAG_CONG - 1))) && icsk->icsk_ca_ops) {
-		const size_t len = strlen(icsk->icsk_ca_ops->name);
-
-		strcpy(INET_DIAG_PUT(skb, INET_DIAG_CONG, len + 1),
-		       icsk->icsk_ca_ops->name);
+		info = nla_data(attr);
 	}
 
+	if ((ext & (1 << (INET_DIAG_CONG - 1))) && icsk->icsk_ca_ops)
+		if (nla_put_string(skb, INET_DIAG_CONG,
+				   icsk->icsk_ca_ops->name) < 0)
+			goto errout;
+
 	handler->idiag_get_info(sk, r, info);
 
 	if (sk->sk_state < TCP_TIME_WAIT &&
@@ -186,11 +189,10 @@ int inet_sk_diag_fill(struct sock *sk, struct inet_connection_sock *icsk,
 		icsk->icsk_ca_ops->get_info(sk, ext, skb);
 
 out:
-	nlh->nlmsg_len = skb_tail_pointer(skb) - b;
-	return skb->len;
+	return nlmsg_end(skb, nlh);
 
-rtattr_failure:
-	nlmsg_trim(skb, b);
+errout:
+	nlmsg_cancel(skb, nlh);
 	return -EMSGSIZE;
 }
 EXPORT_SYMBOL_GPL(inet_sk_diag_fill);
@@ -211,20 +213,16 @@ static int inet_twsk_diag_fill(struct inet_timewait_sock *tw,
 {
 	long tmo;
 	struct inet_diag_msg *r;
-	const unsigned char *previous_tail = skb_tail_pointer(skb);
-	struct nlmsghdr *nlh = nlmsg_put(skb, pid, seq,
-					 unlh->nlmsg_type, sizeof(*r), 0);
+	struct nlmsghdr *nlh;
 
-	if (!nlh) {
-		nlmsg_trim(skb, previous_tail);
+	nlh = nlmsg_put(skb, pid, seq, unlh->nlmsg_type, sizeof(*r),
+			nlmsg_flags);
+	if (!nlh)
 		return -EMSGSIZE;
-	}
 
 	r = nlmsg_data(nlh);
 	BUG_ON(tw->tw_state != TCP_TIME_WAIT);
 
-	nlh->nlmsg_flags = nlmsg_flags;
-
 	tmo = tw->tw_ttd - jiffies;
 	if (tmo < 0)
 		tmo = 0;
@@ -253,8 +251,8 @@ static int inet_twsk_diag_fill(struct inet_timewait_sock *tw,
 		*(struct in6_addr *)r->id.idiag_dst = tw6->tw_v6_daddr;
 	}
 #endif
-	nlh->nlmsg_len = skb_tail_pointer(skb) - previous_tail;
-	return skb->len;
+
+	return nlmsg_end(skb, nlh);
 }
 
 static int sk_diag_fill(struct sock *sk, struct sk_buff *skb,
@@ -303,20 +301,20 @@ int inet_diag_dump_one_icsk(struct inet_hashinfo *hashinfo, struct sk_buff *in_s
 	if (err)
 		goto out;
 
-	err = -ENOMEM;
-	rep = alloc_skb(NLMSG_SPACE((sizeof(struct inet_diag_msg) +
-				     sizeof(struct inet_diag_meminfo) +
-				     sizeof(struct tcp_info) + 64)),
-			GFP_KERNEL);
-	if (!rep)
+	rep = nlmsg_new(sizeof(struct inet_diag_msg) +
+			sizeof(struct inet_diag_meminfo) +
+			sizeof(struct tcp_info) + 64, GFP_KERNEL);
+	if (!rep) {
+		err = -ENOMEM;
 		goto out;
+	}
 
 	err = sk_diag_fill(sk, rep, req,
 			   NETLINK_CB(in_skb).pid,
 			   nlh->nlmsg_seq, 0, nlh);
 	if (err < 0) {
 		WARN_ON(err == -EMSGSIZE);
-		kfree_skb(rep);
+		nlmsg_free(rep);
 		goto out;
 	}
 	err = netlink_unicast(sock_diag_nlsk, rep, NETLINK_CB(in_skb).pid,
@@ -597,19 +595,16 @@ static int inet_diag_fill_req(struct sk_buff *skb, struct sock *sk,
 {
 	const struct inet_request_sock *ireq = inet_rsk(req);
 	struct inet_sock *inet = inet_sk(sk);
-	unsigned char *b = skb_tail_pointer(skb);
 	struct inet_diag_msg *r;
 	struct nlmsghdr *nlh;
 	long tmo;
 
-	nlh = nlmsg_put(skb, pid, seq, unlh->nlmsg_type, sizeof(*r), 0);
-	if (!nlh) {
-		nlmsg_trim(skb, b);
-		return -1;
-	}
-	nlh->nlmsg_flags = NLM_F_MULTI;
-	r = nlmsg_data(nlh);
+	nlh = nlmsg_put(skb, pid, seq, unlh->nlmsg_type, sizeof(*r),
+			NLM_F_MULTI);
+	if (!nlh)
+		return -EMSGSIZE;
 
+	r = nlmsg_data(nlh);
 	r->idiag_family = sk->sk_family;
 	r->idiag_state = TCP_SYN_RECV;
 	r->idiag_timer = 1;
@@ -637,9 +632,8 @@ static int inet_diag_fill_req(struct sk_buff *skb, struct sock *sk,
 		*(struct in6_addr *)r->id.idiag_dst = inet6_rsk(req)->rmt_addr;
 	}
 #endif
-	nlh->nlmsg_len = skb_tail_pointer(skb) - b;
 
-	return skb->len;
+	return nlmsg_end(skb, nlh);
 }
 
 static int inet_diag_dump_reqs(struct sk_buff *skb, struct sock *sk,
Signature · 1 line
1.7.7.6
#7 lore raw

PATCH 4/7 ipmr: Do not use RTA_PUT() macros

In reply to Thomas Graf

Also fix a needless skb tailroom check for a 4 bytes area after after each rtnexthop block.

---
 net/ipv4/ipmr.c |   28 ++++++++++++++--------------
 1 files changed, 14 insertions(+), 14 deletions(-)
net/ipv4/ipmr.c +14 -14
index c94bbc6..b4ac39f 100644
@@ -2006,37 +2006,37 @@ static int __ipmr_fill_mroute(struct mr_table *mrt, struct sk_buff *skb,
 {
 	int ct;
 	struct rtnexthop *nhp;
-	u8 *b = skb_tail_pointer(skb);
-	struct rtattr *mp_head;
+	struct nlattr *mp_attr;
 
 	/* If cache is unresolved, don't try to parse IIF and OIF */
 	if (c->mfc_parent >= MAXVIFS)
 		return -ENOENT;
 
-	if (VIF_EXISTS(mrt, c->mfc_parent))
-		RTA_PUT(skb, RTA_IIF, 4, &mrt->vif_table[c->mfc_parent].dev->ifindex);
+	if (VIF_EXISTS(mrt, c->mfc_parent) &&
+	    nla_put_u32(skb, RTA_IIF, mrt->vif_table[c->mfc_parent].dev->ifindex) < 0)
+		return -EMSGSIZE;
 
-	mp_head = (struct rtattr *)skb_put(skb, RTA_LENGTH(0));
+	if (!(mp_attr = nla_nest_start(skb, RTA_MULTIPATH)))
+		return -EMSGSIZE;
 
 	for (ct = c->mfc_un.res.minvif; ct < c->mfc_un.res.maxvif; ct++) {
 		if (VIF_EXISTS(mrt, ct) && c->mfc_un.res.ttls[ct] < 255) {
-			if (skb_tailroom(skb) < RTA_ALIGN(RTA_ALIGN(sizeof(*nhp)) + 4))
-				goto rtattr_failure;
-			nhp = (struct rtnexthop *)skb_put(skb, RTA_ALIGN(sizeof(*nhp)));
+			if (!(nhp = nla_reserve_nohdr(skb, sizeof(*nhp)))) {
+				nla_nest_cancel(skb, mp_attr);
+				return -EMSGSIZE;
+			}
+
 			nhp->rtnh_flags = 0;
 			nhp->rtnh_hops = c->mfc_un.res.ttls[ct];
 			nhp->rtnh_ifindex = mrt->vif_table[ct].dev->ifindex;
 			nhp->rtnh_len = sizeof(*nhp);
 		}
 	}
-	mp_head->rta_type = RTA_MULTIPATH;
-	mp_head->rta_len = skb_tail_pointer(skb) - (u8 *)mp_head;
+
+	nla_nest_end(skb, mp_attr);
+
 	rtm->rtm_type = RTN_MULTICAST;
 	return 1;
-
-rtattr_failure:
-	nlmsg_trim(skb, b);
-	return -EMSGSIZE;
 }
 
 int ipmr_get_route(struct net *net, struct sk_buff *skb,
Signature · 1 line
1.7.7.6
#8 lore raw

PATCH 5/7 ip6mr: Do not use RTA_PUT() macros

In reply to Thomas Graf
---
 net/ipv6/ip6mr.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
net/ipv6/ip6mr.c +3 -2
index 461e47c..4532973 100644
@@ -2104,8 +2104,9 @@ static int __ip6mr_fill_mroute(struct mr6_table *mrt, struct sk_buff *skb,
 	if (c->mf6c_parent >= MAXMIFS)
 		return -ENOENT;
 
-	if (MIF_EXISTS(mrt, c->mf6c_parent))
-		RTA_PUT(skb, RTA_IIF, 4, &mrt->vif6_table[c->mf6c_parent].dev->ifindex);
+	if (MIF_EXISTS(mrt, c->mf6c_parent) &&
+	    nla_put_u32(skb, RTA_IIF, mrt->vif6_table[c->mf6c_parent].dev->ifindex) < 0)
+		return -EMSGSIZE;
 
 	mp_head = (struct rtattr *)skb_put(skb, RTA_LENGTH(0));
 
Signature · 1 line
1.7.7.6
#9 lore raw

PATCH 6/7 decnet: Do not use RTA_PUT() macros

In reply to Thomas Graf

Also, no need to trim on nlmsg_put() failure, nothing has been added yet. We also want to use nlmsg_end(), nlmsg_new() and nlmsg_free().

---
 net/decnet/dn_route.c |   61 +++++++++++++++++++++++++++----------------
 net/decnet/dn_table.c |   69 +++++++++++++++++++++++++++++--------------------
 2 files changed, 79 insertions(+), 51 deletions(-)
net/decnet/dn_route.c +38 -23
index cd584f7..2493ed5 100644
@@ -1515,56 +1515,68 @@ static int dn_rt_fill_info(struct sk_buff *skb, u32 pid, u32 seq,
 	struct dn_route *rt = (struct dn_route *)skb_dst(skb);
 	struct rtmsg *r;
 	struct nlmsghdr *nlh;
-	unsigned char *b = skb_tail_pointer(skb);
 	long expires;
 
 	nlh = nlmsg_put(skb, pid, seq, event, sizeof(*r), flags);
 	if (!nlh)
-		goto out_nlmsg_trim;
+		return -EMSGSIZE;
+
 	r = nlmsg_data(nlh);
 	r->rtm_family = AF_DECnet;
 	r->rtm_dst_len = 16;
 	r->rtm_src_len = 0;
 	r->rtm_tos = 0;
 	r->rtm_table = RT_TABLE_MAIN;
-	RTA_PUT_U32(skb, RTA_TABLE, RT_TABLE_MAIN);
 	r->rtm_type = rt->rt_type;
 	r->rtm_flags = (rt->rt_flags & ~0xFFFF) | RTM_F_CLONED;
 	r->rtm_scope = RT_SCOPE_UNIVERSE;
 	r->rtm_protocol = RTPROT_UNSPEC;
+
 	if (rt->rt_flags & RTCF_NOTIFY)
 		r->rtm_flags |= RTM_F_NOTIFY;
-	RTA_PUT(skb, RTA_DST, 2, &rt->rt_daddr);
+
+	if (nla_put_u32(skb, RTA_TABLE, RT_TABLE_MAIN) < 0 ||
+	    nla_put_le16(skb, RTA_DST, rt->rt_daddr) < 0)
+		goto errout;
+
 	if (rt->fld.saddr) {
 		r->rtm_src_len = 16;
-		RTA_PUT(skb, RTA_SRC, 2, &rt->fld.saddr);
+		if (nla_put_le16(skb, RTA_SRC, rt->fld.saddr) < 0)
+			goto errout;
 	}
-	if (rt->dst.dev)
-		RTA_PUT(skb, RTA_OIF, sizeof(int), &rt->dst.dev->ifindex);
+	if (rt->dst.dev &&
+	    nla_put_u32(skb, RTA_OIF, rt->dst.dev->ifindex) < 0)
+		goto errout;
+
 	/*
 	 * Note to self - change this if input routes reverse direction when
 	 * they deal only with inputs and not with replies like they do
 	 * currently.
 	 */
-	RTA_PUT(skb, RTA_PREFSRC, 2, &rt->rt_local_src);
-	if (rt->rt_daddr != rt->rt_gateway)
-		RTA_PUT(skb, RTA_GATEWAY, 2, &rt->rt_gateway);
+	if (nla_put_le16(skb, RTA_PREFSRC, rt->rt_local_src) < 0)
+		goto errout;
+
+	if (rt->rt_daddr != rt->rt_gateway &&
+	    nla_put_le16(skb, RTA_GATEWAY, rt->rt_gateway) < 0)
+		goto errout;
+
 	if (rtnetlink_put_metrics(skb, dst_metrics_ptr(&rt->dst)) < 0)
-		goto rtattr_failure;
+		goto errout;
+
 	expires = rt->dst.expires ? rt->dst.expires - jiffies : 0;
 	if (rtnl_put_cacheinfo(skb, &rt->dst, 0, 0, 0, expires,
 			       rt->dst.error) < 0)
-		goto rtattr_failure;
-	if (dn_is_input_route(rt))
-		RTA_PUT(skb, RTA_IIF, sizeof(int), &rt->fld.flowidn_iif);
+		goto errout;
 
-	nlh->nlmsg_len = skb_tail_pointer(skb) - b;
-	return skb->len;
+	if (dn_is_input_route(rt) &&
+	    nla_put_u32(skb, RTA_IIF, rt->fld.flowidn_iif) < 0)
+		goto errout;
 
-out_nlmsg_trim:
-rtattr_failure:
-	nlmsg_trim(skb, b);
-	return -1;
+	return nlmsg_end(skb, nlh);
+
+errout:
+	nlmsg_cancel(skb, nlh);
+	return -EMSGSIZE;
 }
 
 /*
@@ -1587,7 +1599,7 @@ static int dn_cache_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh, void
 	memset(&fld, 0, sizeof(fld));
 	fld.flowidn_proto = DNPROTO_NSP;
 
-	skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
+	skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (skb == NULL)
 		return -ENOBUFS;
 	skb_reset_mac_header(skb);
@@ -1665,13 +1677,16 @@ int dn_cache_dump(struct sk_buff *skb, struct netlink_callback *cb)
 	struct dn_route *rt;
 	int h, s_h;
 	int idx, s_idx;
+	struct rtmsg *rtm;
 
 	if (!net_eq(net, &init_net))
 		return 0;
 
-	if (NLMSG_PAYLOAD(cb->nlh, 0) < sizeof(struct rtmsg))
+	if (nlmsg_len(cb->nlh) < sizeof(struct rtmsg))
 		return -EINVAL;
-	if (!(((struct rtmsg *)nlmsg_data(cb->nlh))->rtm_flags&RTM_F_CLONED))
+
+	rtm = nlmsg_data(cb->nlh);
+	if (!(rtm->rtm_flags & RTM_F_CLONED))
 		return 0;
 
 	s_h = cb->args[0];
net/decnet/dn_table.c +41 -28

103 lines not on this page · raw message

Signature · 1 line
1.7.7.6
#10 lore raw

PATCH 7/7 netlink: Get rid of obsolete rtnetlink macros

In reply to Thomas Graf

Removes all RTA_GET*() and RTA_PUT*() variations, as well as the the unused rtattr_strcmp(). Get rid of rtm_get_table() by moving it to its only user decnet.

---
 include/linux/rtnetlink.h |  129 ---------------------------------------------
 net/core/rtnetlink.c      |   13 -----
 net/decnet/dn_fib.c       |    8 +++
 3 files changed, 8 insertions(+), 142 deletions(-)
include/linux/rtnetlink.h +0 -129
index 2c1de89..ea60b08 100644
@@ -612,12 +612,6 @@ struct tcamsg {
 #include <linux/mutex.h>
 #include <linux/netdevice.h>
 
-static __inline__ int rtattr_strcmp(const struct rtattr *rta, const char *str)
-{
-	int len = strlen(str) + 1;
-	return len > rta->rta_len || memcmp(RTA_DATA(rta), str, len);
-}
-
 extern int rtnetlink_send(struct sk_buff *skb, struct net *net, u32 pid, u32 group, int echo);
 extern int rtnl_unicast(struct sk_buff *skb, struct net *net, u32 pid);
 extern void rtnl_notify(struct sk_buff *skb, struct net *net, u32 pid,
@@ -628,122 +622,6 @@ extern int rtnl_put_cacheinfo(struct sk_buff *skb, struct dst_entry *dst,
 			      u32 id, u32 ts, u32 tsage, long expires,
 			      u32 error);
 
-extern void __rta_fill(struct sk_buff *skb, int attrtype, int attrlen, const void *data);
-
-#define RTA_PUT(skb, attrtype, attrlen, data) \
-({	if (unlikely(skb_tailroom(skb) < (int)RTA_SPACE(attrlen))) \
-		 goto rtattr_failure; \
-   	__rta_fill(skb, attrtype, attrlen, data); }) 
-
-#define RTA_APPEND(skb, attrlen, data) \
-({	if (unlikely(skb_tailroom(skb) < (int)(attrlen))) \
-		goto rtattr_failure; \
-	memcpy(skb_put(skb, attrlen), data, attrlen); })
-
-#define RTA_PUT_NOHDR(skb, attrlen, data) \
-({	RTA_APPEND(skb, RTA_ALIGN(attrlen), data); \
-	memset(skb_tail_pointer(skb) - (RTA_ALIGN(attrlen) - attrlen), 0, \
-	       RTA_ALIGN(attrlen) - attrlen); })
-
-#define RTA_PUT_U8(skb, attrtype, value) \
-({	u8 _tmp = (value); \
-	RTA_PUT(skb, attrtype, sizeof(u8), &_tmp); })
-
-#define RTA_PUT_U16(skb, attrtype, value) \
-({	u16 _tmp = (value); \
-	RTA_PUT(skb, attrtype, sizeof(u16), &_tmp); })
-
-#define RTA_PUT_U32(skb, attrtype, value) \
-({	u32 _tmp = (value); \
-	RTA_PUT(skb, attrtype, sizeof(u32), &_tmp); })
-
-#define RTA_PUT_U64(skb, attrtype, value) \
-({	u64 _tmp = (value); \
-	RTA_PUT(skb, attrtype, sizeof(u64), &_tmp); })
-
-#define RTA_PUT_SECS(skb, attrtype, value) \
-	RTA_PUT_U64(skb, attrtype, (value) / HZ)
-
-#define RTA_PUT_MSECS(skb, attrtype, value) \
-	RTA_PUT_U64(skb, attrtype, jiffies_to_msecs(value))
-
-#define RTA_PUT_STRING(skb, attrtype, value) \
-	RTA_PUT(skb, attrtype, strlen(value) + 1, value)
-
-#define RTA_PUT_FLAG(skb, attrtype) \
-	RTA_PUT(skb, attrtype, 0, NULL);
-
-#define RTA_NEST(skb, type) \
-({	struct rtattr *__start = (struct rtattr *)skb_tail_pointer(skb); \
-	RTA_PUT(skb, type, 0, NULL); \
-	__start;  })
-
-#define RTA_NEST_END(skb, start) \
-({	(start)->rta_len = skb_tail_pointer(skb) - (unsigned char *)(start); \
-	(skb)->len; })
-
-#define RTA_NEST_COMPAT(skb, type, attrlen, data) \
-({	struct rtattr *__start = (struct rtattr *)skb_tail_pointer(skb); \
-	RTA_PUT(skb, type, attrlen, data); \
-	RTA_NEST(skb, type); \
-	__start; })
-
-#define RTA_NEST_COMPAT_END(skb, start) \
-({	struct rtattr *__nest = (void *)(start) + NLMSG_ALIGN((start)->rta_len); \
-	(start)->rta_len = skb_tail_pointer(skb) - (unsigned char *)(start); \
-	RTA_NEST_END(skb, __nest); \
-	(skb)->len; })
-
-#define RTA_NEST_CANCEL(skb, start) \
-({	if (start) \
-		skb_trim(skb, (unsigned char *) (start) - (skb)->data); \
-	-1; })
-
-#define RTA_GET_U8(rta) \
-({	if (!rta || RTA_PAYLOAD(rta) < sizeof(u8)) \
-		goto rtattr_failure; \
-	*(u8 *) RTA_DATA(rta); })
-
-#define RTA_GET_U16(rta) \
-({	if (!rta || RTA_PAYLOAD(rta) < sizeof(u16)) \
-		goto rtattr_failure; \
-	*(u16 *) RTA_DATA(rta); })
-
-#define RTA_GET_U32(rta) \
-({	if (!rta || RTA_PAYLOAD(rta) < sizeof(u32)) \
-		goto rtattr_failure; \
-	*(u32 *) RTA_DATA(rta); })
-
-#define RTA_GET_U64(rta) \
-({	u64 _tmp; \
-	if (!rta || RTA_PAYLOAD(rta) < sizeof(u64)) \
-		goto rtattr_failure; \
-	memcpy(&_tmp, RTA_DATA(rta), sizeof(_tmp)); \
-	_tmp; })
-
-#define RTA_GET_FLAG(rta) (!!(rta))
-
-#define RTA_GET_SECS(rta) ((unsigned long) RTA_GET_U64(rta) * HZ)
-#define RTA_GET_MSECS(rta) (msecs_to_jiffies((unsigned long) RTA_GET_U64(rta)))
-		
-static inline struct rtattr *
-__rta_reserve(struct sk_buff *skb, int attrtype, int attrlen)
-{
-	struct rtattr *rta;
-	int size = RTA_LENGTH(attrlen);
-
-	rta = (struct rtattr*)skb_put(skb, RTA_ALIGN(size));
-	rta->rta_type = attrtype;
-	rta->rta_len = size;
-	memset(RTA_DATA(rta) + attrlen, 0, RTA_ALIGN(size) - size);
-	return rta;
-}
-
-#define __RTA_PUT(skb, attrtype, attrlen) \
-({ 	if (unlikely(skb_tailroom(skb) < (int)RTA_SPACE(attrlen))) \
-		goto rtattr_failure; \
-   	__rta_reserve(skb, attrtype, attrlen); })
-
 extern void rtmsg_ifinfo(int type, struct net_device *dev, unsigned change);
 
 /* RTNL is used as a global lock for all changes to network configuration  */
@@ -794,13 +672,6 @@ extern void __rtnl_unlock(void);
 	} \
 } while(0)
 
-static inline u32 rtm_get_table(struct rtattr **rta, u8 table)
-{
-	return RTA_GET_U32(rta[RTA_TABLE-1]);
-rtattr_failure:
-	return table;
-}
-
 extern int ndo_dflt_fdb_dump(struct sk_buff *skb,
 			     struct netlink_callback *cb,
 			     struct net_device *dev,
net/core/rtnetlink.c +0 -13
index 21318d1..bc8a1cd 100644
@@ -541,19 +541,6 @@ static const int rta_max[RTM_NR_FAMILIES] =
 	[RTM_FAM(RTM_NEWACTION)]    = TCAA_MAX,
 };
 
-void __rta_fill(struct sk_buff *skb, int attrtype, int attrlen, const void *data)
-{
-	struct rtattr *rta;
-	int size = RTA_LENGTH(attrlen);
-
-	rta = (struct rtattr *)skb_put(skb, RTA_ALIGN(size));
-	rta->rta_type = attrtype;
-	rta->rta_len = size;
-	memcpy(RTA_DATA(rta), data, attrlen);
-	memset(RTA_DATA(rta) + attrlen, 0, RTA_ALIGN(size) - size);
-}
-EXPORT_SYMBOL(__rta_fill);
-
 int rtnetlink_send(struct sk_buff *skb, struct net *net, u32 pid, unsigned int group, int echo)
 {
 	struct sock *rtnl = net->rtnl;
net/decnet/dn_fib.c +8 -0
index 7eaf987..102d610 100644
@@ -505,6 +505,14 @@ static int dn_fib_check_attr(struct rtmsg *r, struct rtattr **rta)
 	return 0;
 }
 
+static inline u32 rtm_get_table(struct rtattr **rta, u8 table)
+{
+	if (rta[RTA_TABLE - 1])
+		table = nla_get_u32((struct nlattr *) rta[RTA_TABLE - 1]);
+
+	return table;
+}
+
 static int dn_fib_rtm_delroute(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
 {
 	struct net *net = sock_net(skb->sk);
Signature · 1 line
1.7.7.6
#11 lore raw

Re: [PATCH 7/7] netlink: Get rid of obsolete rtnetlink macros

Quoted 6 lines · On Wed, 27 Jun 2012 11:36:16 +0200 Thomas Graf <[email protected]> wrote:

Removes all RTA_GET*() and RTA_PUT*() variations, as well as the the unused rtattr_strcmp(). Get rid of rtm_get_table() by moving it to its only user decnet.

---

Nack. The RTA_ macros are exported to user space through kernel headers process. If you do this it will break iproute2 build.

maintainer
#12 lore raw

Re: [PATCH 7/7] netlink: Get rid of obsolete rtnetlink macros

From: Stephen Hemminger <[email protected]>
Date: Wed, 27 Jun 2012 08:35:32 -0700

Quoted 7 lines
Quoted 4 lines · On Wed, 27 Jun 2012 11:36:16 +0200 Thomas Graf <[email protected]> wrote:

Removes all RTA_GET*() and RTA_PUT*() variations, as well as the the unused rtattr_strcmp(). Get rid of rtm_get_table() by moving it to its only user decnet.

Quoted 6 lines
Quoted 2 lines

---

Nack. The RTA_ macros are exported to user space through kernel headers process. If you do this it will break iproute2 build.

They are protected by __KERNEL__, what are you talking about?

#13 lore raw

Re: [PATCH 7/7] netlink: Get rid of obsolete rtnetlink macros

Quoted 10 lines · On Wed, 27 Jun 2012 15:04:35 -0700 (PDT) David Miller <[email protected]>…

From: Stephen Hemminger <[email protected]>
Date: Wed, 27 Jun 2012 08:35:32 -0700

Quoted 7 lines
Quoted 4 lines · On Wed, 27 Jun 2012 11:36:16 +0200 Thomas Graf <[email protected]> wrote:

Removes all RTA_GET*() and RTA_PUT*() variations, as well as the the unused rtattr_strcmp(). Get rid of rtm_get_table() by moving it to its only user decnet.

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

---

Nack. The RTA_ macros are exported to user space through kernel headers process. If you do this it will break iproute2 build.

They are protected by __KERNEL__, what are you talking about?

Never mind, there are copies in other places in iproute

I plan to get rid of the macro versions of this stuff. It is a nuisance in iproute for the same reason as the kernel.

maintainer
#14 lore raw
In reply to Thomas Graf

From: Thomas Graf <[email protected]>
Date: Wed, 27 Jun 2012 11:36:09 +0200

Quoted 6 lines

This patchset gets rid of all the RTA_PUT() and RTA_GET() macros and makes use of the type-safe netlink API variants where applicable.

I did my best to test these patches but I do not own any decnet hardware so the decnet part is compile tested only.

All applied, thanks a lot Thomas.