netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH iproute2-next 0/3] ip: Minor code cleanups and improvements
@ 2018-01-31  8:15 Serhey Popovych
  2018-01-31  8:15 ` [PATCH iproute2-next 1/3] ip: Consolidate ip, xdp and lwtunnel parse/dump prototypes in ip_common.h Serhey Popovych
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Serhey Popovych @ 2018-01-31  8:15 UTC (permalink / raw)
  To: netdev

With this series I want to do some adjustments in header files for ip(8)
as well as some trivial cleanups like variable rename and finally use
addattr_nest()/addattr_nest_end() family instead of open coding nested
attributes handling.

See individual patch description for more information on changes
presented.

Reviews, comments and suggestions are welcome.

Thanks,
Serhii

Serhey Popovych (3):
  ip: Consolidate ip, xdp and lwtunnel parse/dump prototypes in
    ip_common.h
  ip: Minor cleanups
  treewide: Use addattr_nest()/addattr_nest_end() to handle nested
    attributes

 include/utils.h       |   10 ----------
 ip/ip_common.h        |   21 +++++++++++++++++++++
 ip/ipaddress.c        |    1 -
 ip/iplink.c           |    7 +++----
 ip/iplink_vlan.c      |    5 ++---
 ip/iplink_vxcan.c     |   17 ++++++++---------
 ip/iplink_xdp.c       |    4 ++--
 ip/ipmacsec.c         |   28 ++++++++++++++--------------
 ip/iproute.c          |    1 -
 ip/iproute_lwtunnel.c |    4 ++--
 ip/iproute_lwtunnel.h |    9 ---------
 ip/link_gre.c         |    2 +-
 ip/link_gre6.c        |    2 +-
 ip/link_ip6tnl.c      |    2 +-
 ip/link_iptnl.c       |    2 +-
 ip/link_veth.c        |   17 ++++++++---------
 ip/link_vti.c         |    2 +-
 ip/link_vti6.c        |    2 +-
 ip/xdp.h              |   11 -----------
 tc/f_flow.c           |    5 ++---
 tc/f_fw.c             |    5 ++---
 tc/f_route.c          |    5 ++---
 tc/f_rsvp.c           |    5 ++---
 tc/f_tcindex.c        |    5 ++---
 tc/f_u32.c            |    5 ++---
 tc/m_action.c         |   26 ++++++++++----------------
 tc/m_bpf.c            |    5 ++---
 tc/m_connmark.c       |    5 ++---
 tc/m_csum.c           |    5 ++---
 tc/m_ematch.c         |   16 +++++++---------
 tc/m_gact.c           |    5 ++---
 tc/m_ife.c            |   10 ++++------
 tc/m_ipt.c            |    5 ++---
 tc/m_mirred.c         |    5 ++---
 tc/m_nat.c            |    5 ++---
 tc/m_pedit.c          |    5 ++---
 tc/m_police.c         |    5 ++---
 tc/m_sample.c         |    5 ++---
 tc/m_simple.c         |    5 ++---
 tc/m_skbedit.c        |    5 ++---
 tc/m_skbmod.c         |    5 ++---
 tc/m_tunnel_key.c     |    5 ++---
 tc/m_vlan.c           |    5 ++---
 tc/m_xt.c             |    5 ++---
 tc/m_xt_old.c         |    5 ++---
 tc/q_atm.c            |   11 ++++++-----
 tc/q_cbq.c            |   10 ++++------
 tc/q_cbs.c            |    5 ++---
 tc/q_choke.c          |    5 ++---
 tc/q_codel.c          |    5 ++---
 tc/q_drr.c            |    5 ++---
 tc/q_dsmark.c         |   13 ++++++-------
 tc/q_fq.c             |    5 ++---
 tc/q_fq_codel.c       |    5 ++---
 tc/q_gred.c           |   10 ++++------
 tc/q_hfsc.c           |    6 ++----
 tc/q_hhf.c            |    5 ++---
 tc/q_htb.c            |   10 ++++------
 tc/q_mqprio.c         |    5 ++---
 tc/q_netem.c          |    7 ++-----
 tc/q_pie.c            |    5 ++---
 tc/q_qfq.c            |    5 ++---
 tc/q_red.c            |    5 ++---
 tc/q_sfb.c            |    5 ++---
 tc/q_tbf.c            |    5 ++---
 tc/tc_qdisc.c         |    5 ++---
 66 files changed, 189 insertions(+), 262 deletions(-)
 delete mode 100644 ip/iproute_lwtunnel.h
 delete mode 100644 ip/xdp.h

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH iproute2-next 1/3] ip: Consolidate ip, xdp and lwtunnel parse/dump prototypes in ip_common.h
  2018-01-31  8:15 [PATCH iproute2-next 0/3] ip: Minor code cleanups and improvements Serhey Popovych
@ 2018-01-31  8:15 ` Serhey Popovych
  2018-01-31  8:15 ` [PATCH iproute2-next 2/3] ip: Minor cleanups Serhey Popovych
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Serhey Popovych @ 2018-01-31  8:15 UTC (permalink / raw)
  To: netdev

Having iplink_parse() and @struct iplink_req in include/utils.h does not
reflect it's IP nature: move to ip/ip_common.h.

Move contents of ip/iplink_xdp.h and ip/iproute_lwtunnel.h to
ip/ip_common.h since they are small (i.e. only two function prototypes):
ip/iplink_bridge.c and ip/iplink_vrf.c prototypes already there.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 include/utils.h       |   10 ----------
 ip/ip_common.h        |   21 +++++++++++++++++++++
 ip/ipaddress.c        |    1 -
 ip/iplink.c           |    1 -
 ip/iplink_xdp.c       |    4 ++--
 ip/iproute.c          |    1 -
 ip/iproute_lwtunnel.c |    4 ++--
 ip/iproute_lwtunnel.h |    9 ---------
 ip/xdp.h              |   11 -----------
 9 files changed, 25 insertions(+), 37 deletions(-)
 delete mode 100644 ip/iproute_lwtunnel.h
 delete mode 100644 ip/xdp.h

diff --git a/include/utils.h b/include/utils.h
index 0394268..f81928a 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -272,16 +272,6 @@ extern int cmdlineno;
 ssize_t getcmdline(char **line, size_t *len, FILE *in);
 int makeargs(char *line, char *argv[], int maxargs);
 
-struct iplink_req {
-	struct nlmsghdr		n;
-	struct ifinfomsg	i;
-	char			buf[1024];
-};
-
-int iplink_parse(int argc, char **argv, struct iplink_req *req,
-		char **name, char **type, char **link, char **dev,
-		int *group, int *index);
-
 int do_each_netns(int (*func)(char *nsname, void *arg), void *arg,
 		bool show_label);
 
diff --git a/ip/ip_common.h b/ip/ip_common.h
index 3203f0c..35df364 100644
--- a/ip/ip_common.h
+++ b/ip/ip_common.h
@@ -109,6 +109,12 @@ static inline int rtm_get_table(struct rtmsg *r, struct rtattr **tb)
 
 extern struct rtnl_handle rth;
 
+struct iplink_req {
+	struct nlmsghdr		n;
+	struct ifinfomsg	i;
+	char			buf[1024];
+};
+
 struct link_util {
 	struct link_util	*next;
 	const char		*id;
@@ -129,11 +135,26 @@ struct link_util {
 
 struct link_util *get_link_kind(const char *kind);
 
+int iplink_parse(int argc, char **argv, struct iplink_req *req,
+		 char **name, char **type, char **link, char **dev,
+		 int *group, int *index);
+
+/* iplink_bridge.c */
 void br_dump_bridge_id(const struct ifla_bridge_id *id, char *buf, size_t len);
 int bridge_parse_xstats(struct link_util *lu, int argc, char **argv);
 int bridge_print_xstats(const struct sockaddr_nl *who,
 			struct nlmsghdr *n, void *arg);
 
+/* iproute_lwtunnel.c */
+int lwt_parse_encap(struct rtattr *rta, size_t len, int *argcp, char ***argvp);
+void lwt_print_encap(FILE *fp, struct rtattr *encap_type, struct rtattr *encap);
+
+/* iplink_xdp.c */
+int xdp_parse(int *argc, char ***argv, struct iplink_req *req, __u32 ifindex,
+	      bool generic, bool drv, bool offload);
+void xdp_dump(FILE *fp, struct rtattr *tb, bool link, bool details);
+
+/* iplink_vrf.c */
 __u32 ipvrf_get_table(const char *name);
 int name_is_vrf(const char *name);
 
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 051a05f..acb9936 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -34,7 +34,6 @@
 #include "utils.h"
 #include "ll_map.h"
 #include "ip_common.h"
-#include "xdp.h"
 #include "color.h"
 
 enum {
diff --git a/ip/iplink.c b/ip/iplink.c
index 230f4c5..6cbd04f 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -31,7 +31,6 @@
 #include "rt_names.h"
 #include "utils.h"
 #include "ip_common.h"
-#include "xdp.h"
 #include "namespace.h"
 
 #define IPLINK_IOCTL_COMPAT	1
diff --git a/ip/iplink_xdp.c b/ip/iplink_xdp.c
index 6eeb820..8382635 100644
--- a/ip/iplink_xdp.c
+++ b/ip/iplink_xdp.c
@@ -14,9 +14,9 @@
 
 #include <linux/bpf.h>
 
-#include "json_print.h"
-#include "xdp.h"
 #include "bpf_util.h"
+#include "utils.h"
+#include "ip_common.h"
 
 extern int force;
 
diff --git a/ip/iproute.c b/ip/iproute.c
index bf886fd..7616c0d 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -28,7 +28,6 @@
 #include "rt_names.h"
 #include "utils.h"
 #include "ip_common.h"
-#include "iproute_lwtunnel.h"
 
 #ifndef RTAX_RTTVAR
 #define RTAX_RTTVAR RTAX_HOPS
diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c
index ffa897a..f7fbc62 100644
--- a/ip/iproute_lwtunnel.c
+++ b/ip/iproute_lwtunnel.c
@@ -22,9 +22,9 @@
 #include <errno.h>
 
 #include "rt_names.h"
-#include "utils.h"
-#include "iproute_lwtunnel.h"
 #include "bpf_util.h"
+#include "utils.h"
+#include "ip_common.h"
 #include "ila_common.h"
 
 #include <linux/seg6.h>
diff --git a/ip/iproute_lwtunnel.h b/ip/iproute_lwtunnel.h
deleted file mode 100644
index be003ce..0000000
--- a/ip/iproute_lwtunnel.h
+++ /dev/null
@@ -1,9 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __LWTUNNEL_H__
-#define __LETUNNEL_H__ 1
-
-int lwt_parse_encap(struct rtattr *rta, size_t len, int *argcp, char ***argvp);
-void lwt_print_encap(FILE *fp, struct rtattr *encap_type,
-		     struct rtattr *encap);
-
-#endif
diff --git a/ip/xdp.h b/ip/xdp.h
deleted file mode 100644
index 7e10696..0000000
--- a/ip/xdp.h
+++ /dev/null
@@ -1,11 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __XDP__
-#define __XDP__
-
-#include "utils.h"
-
-int xdp_parse(int *argc, char ***argv, struct iplink_req *req, __u32 ifindex,
-	      bool generic, bool drv, bool offload);
-void xdp_dump(FILE *fp, struct rtattr *tb, bool link, bool details);
-
-#endif /* __XDP__ */
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH iproute2-next 2/3] ip: Minor cleanups
  2018-01-31  8:15 [PATCH iproute2-next 0/3] ip: Minor code cleanups and improvements Serhey Popovych
  2018-01-31  8:15 ` [PATCH iproute2-next 1/3] ip: Consolidate ip, xdp and lwtunnel parse/dump prototypes in ip_common.h Serhey Popovych
@ 2018-01-31  8:15 ` Serhey Popovych
  2018-01-31  8:15 ` [PATCH iproute2-next 3/3] treewide: Use addattr_nest()/addattr_nest_end() to handle nested attributes Serhey Popovych
  2018-02-02 23:06 ` [PATCH iproute2-next 0/3] ip: Minor code cleanups and improvements David Ahern
  3 siblings, 0 replies; 7+ messages in thread
From: Serhey Popovych @ 2018-01-31  8:15 UTC (permalink / raw)
  To: netdev

  1) Rename @hdr parameter to @n to be coherent with rest of the parsing
     code.

  2) Use NLMSG_DATA() to get pointer to the data after nlmsghdr instead
     of calculating it directly in ip/tunnel code.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/iplink_vxcan.c |   18 +++++++++---------
 ip/ipmacsec.c     |   28 ++++++++++++++--------------
 ip/link_gre.c     |    2 +-
 ip/link_gre6.c    |    2 +-
 ip/link_ip6tnl.c  |    2 +-
 ip/link_iptnl.c   |    2 +-
 ip/link_veth.c    |   18 +++++++++---------
 ip/link_vti.c     |    2 +-
 ip/link_vti6.c    |    2 +-
 9 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/ip/iplink_vxcan.c b/ip/iplink_vxcan.c
index ed0ad8b..87be5a6 100644
--- a/ip/iplink_vxcan.c
+++ b/ip/iplink_vxcan.c
@@ -31,7 +31,7 @@ static void usage(void)
 }
 
 static int vxcan_parse_opt(struct link_util *lu, int argc, char **argv,
-			  struct nlmsghdr *hdr)
+			  struct nlmsghdr *n)
 {
 	char *dev = NULL;
 	char *name = NULL;
@@ -49,18 +49,18 @@ static int vxcan_parse_opt(struct link_util *lu, int argc, char **argv,
 		return -1;
 	}
 
-	ifm = NLMSG_DATA(hdr);
+	ifm = NLMSG_DATA(n);
 	ifi_flags = ifm->ifi_flags;
 	ifi_change = ifm->ifi_change;
 	ifm->ifi_flags = 0;
 	ifm->ifi_change = 0;
 
-	data = NLMSG_TAIL(hdr);
-	addattr_l(hdr, 1024, VXCAN_INFO_PEER, NULL, 0);
+	data = NLMSG_TAIL(n);
+	addattr_l(n, 1024, VXCAN_INFO_PEER, NULL, 0);
 
-	hdr->nlmsg_len += sizeof(struct ifinfomsg);
+	n->nlmsg_len += sizeof(struct ifinfomsg);
 
-	err = iplink_parse(argc - 1, argv + 1, (struct iplink_req *)hdr,
+	err = iplink_parse(argc - 1, argv + 1, (struct iplink_req *)n,
 			   &name, &type, &link, &dev, &group, &index);
 	if (err < 0)
 		return err;
@@ -69,7 +69,7 @@ static int vxcan_parse_opt(struct link_util *lu, int argc, char **argv,
 		duparg("type", argv[err]);
 
 	if (name) {
-		addattr_l(hdr, 1024,
+		addattr_l(n, 1024,
 			  IFLA_IFNAME, name, strlen(name) + 1);
 	}
 
@@ -81,9 +81,9 @@ static int vxcan_parse_opt(struct link_util *lu, int argc, char **argv,
 	ifm->ifi_change = ifi_change;
 
 	if (group != -1)
-		addattr32(hdr, 1024, IFLA_GROUP, group);
+		addattr32(n, 1024, IFLA_GROUP, group);
 
-	data->rta_len = (void *)NLMSG_TAIL(hdr) - (void *)data;
+	data->rta_len = (void *)NLMSG_TAIL(n) - (void *)data;
 	return argc - 1 - err;
 }
 
diff --git a/ip/ipmacsec.c b/ip/ipmacsec.c
index 345a707..c0b45f5 100644
--- a/ip/ipmacsec.c
+++ b/ip/ipmacsec.c
@@ -1132,7 +1132,7 @@ static void usage(FILE *f)
 }
 
 static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
-			    struct nlmsghdr *hdr)
+			    struct nlmsghdr *n)
 {
 	int ret;
 	__u8 encoding_sa = 0xff;
@@ -1151,10 +1151,10 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
 
 	if (ret > 0) {
 		if (sci.sci)
-			addattr_l(hdr, MACSEC_BUFLEN, IFLA_MACSEC_SCI,
+			addattr_l(n, MACSEC_BUFLEN, IFLA_MACSEC_SCI,
 				  &sci.sci, sizeof(sci.sci));
 		else
-			addattr_l(hdr, MACSEC_BUFLEN, IFLA_MACSEC_PORT,
+			addattr_l(n, MACSEC_BUFLEN, IFLA_MACSEC_PORT,
 				  &sci.port, sizeof(sci.port));
 	}
 
@@ -1183,7 +1183,7 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
 				     ARRAY_SIZE(values_on_off), &i);
 			if (ret != 0)
 				return ret;
-			addattr8(hdr, MACSEC_BUFLEN, IFLA_MACSEC_ENCRYPT, i);
+			addattr8(n, MACSEC_BUFLEN, IFLA_MACSEC_ENCRYPT, i);
 		} else if (strcmp(*argv, "send_sci") == 0) {
 			NEXT_ARG();
 			int i;
@@ -1193,7 +1193,7 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
 			if (ret != 0)
 				return ret;
 			send_sci = i;
-			addattr8(hdr, MACSEC_BUFLEN,
+			addattr8(n, MACSEC_BUFLEN,
 				 IFLA_MACSEC_INC_SCI, send_sci);
 		} else if (strcmp(*argv, "end_station") == 0) {
 			NEXT_ARG();
@@ -1204,7 +1204,7 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
 			if (ret != 0)
 				return ret;
 			es = i;
-			addattr8(hdr, MACSEC_BUFLEN, IFLA_MACSEC_ES, es);
+			addattr8(n, MACSEC_BUFLEN, IFLA_MACSEC_ES, es);
 		} else if (strcmp(*argv, "scb") == 0) {
 			NEXT_ARG();
 			int i;
@@ -1214,7 +1214,7 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
 			if (ret != 0)
 				return ret;
 			scb = i;
-			addattr8(hdr, MACSEC_BUFLEN, IFLA_MACSEC_SCB, scb);
+			addattr8(n, MACSEC_BUFLEN, IFLA_MACSEC_SCB, scb);
 		} else if (strcmp(*argv, "protect") == 0) {
 			NEXT_ARG();
 			int i;
@@ -1223,7 +1223,7 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
 				     ARRAY_SIZE(values_on_off), &i);
 			if (ret != 0)
 				return ret;
-			addattr8(hdr, MACSEC_BUFLEN, IFLA_MACSEC_PROTECT, i);
+			addattr8(n, MACSEC_BUFLEN, IFLA_MACSEC_PROTECT, i);
 		} else if (strcmp(*argv, "replay") == 0) {
 			NEXT_ARG();
 			int i;
@@ -1245,7 +1245,7 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
 				     (int *)&validate);
 			if (ret != 0)
 				return ret;
-			addattr8(hdr, MACSEC_BUFLEN,
+			addattr8(n, MACSEC_BUFLEN,
 				 IFLA_MACSEC_VALIDATION, validate);
 		} else if (strcmp(*argv, "encodingsa") == 0) {
 			if (encoding_sa != 0xff)
@@ -1281,20 +1281,20 @@ static int macsec_parse_opt(struct link_util *lu, int argc, char **argv,
 	}
 
 	if (cipher.id)
-		addattr_l(hdr, MACSEC_BUFLEN, IFLA_MACSEC_CIPHER_SUITE,
+		addattr_l(n, MACSEC_BUFLEN, IFLA_MACSEC_CIPHER_SUITE,
 			  &cipher.id, sizeof(cipher.id));
 	if (cipher.icv_len)
-		addattr_l(hdr, MACSEC_BUFLEN, IFLA_MACSEC_ICV_LEN,
+		addattr_l(n, MACSEC_BUFLEN, IFLA_MACSEC_ICV_LEN,
 			  &cipher.icv_len, sizeof(cipher.icv_len));
 
 	if (replay_protect != -1) {
-		addattr32(hdr, MACSEC_BUFLEN, IFLA_MACSEC_WINDOW, window);
-		addattr8(hdr, MACSEC_BUFLEN, IFLA_MACSEC_REPLAY_PROTECT,
+		addattr32(n, MACSEC_BUFLEN, IFLA_MACSEC_WINDOW, window);
+		addattr8(n, MACSEC_BUFLEN, IFLA_MACSEC_REPLAY_PROTECT,
 			 replay_protect);
 	}
 
 	if (encoding_sa != 0xff) {
-		addattr_l(hdr, MACSEC_BUFLEN, IFLA_MACSEC_ENCODING_SA,
+		addattr_l(n, MACSEC_BUFLEN, IFLA_MACSEC_ENCODING_SA,
 			  &encoding_sa, sizeof(encoding_sa));
 	}
 
diff --git a/ip/link_gre.c b/ip/link_gre.c
index 8fb8fe5..b2573a1 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -69,7 +69,7 @@ static void usage(void)
 static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
 			 struct nlmsghdr *n)
 {
-	struct ifinfomsg *ifi = (struct ifinfomsg *)(n + 1);
+	struct ifinfomsg *ifi = NLMSG_DATA(n);
 	struct {
 		struct nlmsghdr n;
 		struct ifinfomsg i;
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 4045f65..880b75d 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -79,7 +79,7 @@ static void usage(void)
 static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
 			 struct nlmsghdr *n)
 {
-	struct ifinfomsg *ifi = (struct ifinfomsg *)(n + 1);
+	struct ifinfomsg *ifi = NLMSG_DATA(n);
 	struct {
 		struct nlmsghdr n;
 		struct ifinfomsg i;
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index ccc79ff..91d7d99 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -72,7 +72,7 @@ static void usage(void)
 static int ip6tunnel_parse_opt(struct link_util *lu, int argc, char **argv,
 			       struct nlmsghdr *n)
 {
-	struct ifinfomsg *ifi = (struct ifinfomsg *)(n + 1);
+	struct ifinfomsg *ifi = NLMSG_DATA(n);
 	struct {
 		struct nlmsghdr n;
 		struct ifinfomsg i;
diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
index 622c6f1..3e653b7 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -72,7 +72,7 @@ static void usage(int sit)
 static int iptunnel_parse_opt(struct link_util *lu, int argc, char **argv,
 			      struct nlmsghdr *n)
 {
-	struct ifinfomsg *ifi = (struct ifinfomsg *)(n + 1);
+	struct ifinfomsg *ifi = NLMSG_DATA(n);
 	struct {
 		struct nlmsghdr n;
 		struct ifinfomsg i;
diff --git a/ip/link_veth.c b/ip/link_veth.c
index fddb7ac..e6219e7 100644
--- a/ip/link_veth.c
+++ b/ip/link_veth.c
@@ -29,7 +29,7 @@ static void usage(void)
 }
 
 static int veth_parse_opt(struct link_util *lu, int argc, char **argv,
-			  struct nlmsghdr *hdr)
+			  struct nlmsghdr *n)
 {
 	char *dev = NULL;
 	char *name = NULL;
@@ -47,18 +47,18 @@ static int veth_parse_opt(struct link_util *lu, int argc, char **argv,
 		return -1;
 	}
 
-	ifm = NLMSG_DATA(hdr);
+	ifm = NLMSG_DATA(n);
 	ifi_flags = ifm->ifi_flags;
 	ifi_change = ifm->ifi_change;
 	ifm->ifi_flags = 0;
 	ifm->ifi_change = 0;
 
-	data = NLMSG_TAIL(hdr);
-	addattr_l(hdr, 1024, VETH_INFO_PEER, NULL, 0);
+	data = NLMSG_TAIL(n);
+	addattr_l(n, 1024, VETH_INFO_PEER, NULL, 0);
 
-	hdr->nlmsg_len += sizeof(struct ifinfomsg);
+	n->nlmsg_len += sizeof(struct ifinfomsg);
 
-	err = iplink_parse(argc - 1, argv + 1, (struct iplink_req *)hdr,
+	err = iplink_parse(argc - 1, argv + 1, (struct iplink_req *)n,
 			   &name, &type, &link, &dev, &group, &index);
 	if (err < 0)
 		return err;
@@ -67,7 +67,7 @@ static int veth_parse_opt(struct link_util *lu, int argc, char **argv,
 		duparg("type", argv[err]);
 
 	if (name) {
-		addattr_l(hdr, 1024,
+		addattr_l(n, 1024,
 			  IFLA_IFNAME, name, strlen(name) + 1);
 	}
 
@@ -79,9 +79,9 @@ static int veth_parse_opt(struct link_util *lu, int argc, char **argv,
 	ifm->ifi_change = ifi_change;
 
 	if (group != -1)
-		addattr32(hdr, 1024, IFLA_GROUP, group);
+		addattr32(n, 1024, IFLA_GROUP, group);
 
-	data->rta_len = (void *)NLMSG_TAIL(hdr) - (void *)data;
+	data->rta_len = (void *)NLMSG_TAIL(n) - (void *)data;
 	return argc - 1 - err;
 }
 
diff --git a/ip/link_vti.c b/ip/link_vti.c
index a5b84a0..49b87e9 100644
--- a/ip/link_vti.c
+++ b/ip/link_vti.c
@@ -49,7 +49,7 @@ static void usage(void)
 static int vti_parse_opt(struct link_util *lu, int argc, char **argv,
 			 struct nlmsghdr *n)
 {
-	struct ifinfomsg *ifi = (struct ifinfomsg *)(n + 1);
+	struct ifinfomsg *ifi = NLMSG_DATA(n);
 	struct {
 		struct nlmsghdr n;
 		struct ifinfomsg i;
diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index 39d12e6..d1fbec5 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -49,7 +49,7 @@ static void usage(void)
 static int vti6_parse_opt(struct link_util *lu, int argc, char **argv,
 			  struct nlmsghdr *n)
 {
-	struct ifinfomsg *ifi = (struct ifinfomsg *)(n + 1);
+	struct ifinfomsg *ifi = NLMSG_DATA(n);
 	struct {
 		struct nlmsghdr n;
 		struct ifinfomsg i;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH iproute2-next 3/3] treewide: Use addattr_nest()/addattr_nest_end() to handle nested attributes
  2018-01-31  8:15 [PATCH iproute2-next 0/3] ip: Minor code cleanups and improvements Serhey Popovych
  2018-01-31  8:15 ` [PATCH iproute2-next 1/3] ip: Consolidate ip, xdp and lwtunnel parse/dump prototypes in ip_common.h Serhey Popovych
  2018-01-31  8:15 ` [PATCH iproute2-next 2/3] ip: Minor cleanups Serhey Popovych
@ 2018-01-31  8:15 ` Serhey Popovych
  2018-04-13 22:57   ` Vinicius Costa Gomes
  2018-02-02 23:06 ` [PATCH iproute2-next 0/3] ip: Minor code cleanups and improvements David Ahern
  3 siblings, 1 reply; 7+ messages in thread
From: Serhey Popovych @ 2018-01-31  8:15 UTC (permalink / raw)
  To: netdev

We have helper routines to support nested attribute addition into
netlink buffer: use them instead of open coding.

Use addattr_nest_compat()/addattr_nest_compat_end() where appropriate.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/iplink.c       |    6 +++---
 ip/iplink_vlan.c  |    5 ++---
 ip/iplink_vxcan.c |    5 ++---
 ip/link_veth.c    |    5 ++---
 tc/f_flow.c       |    5 ++---
 tc/f_fw.c         |    5 ++---
 tc/f_route.c      |    5 ++---
 tc/f_rsvp.c       |    5 ++---
 tc/f_tcindex.c    |    5 ++---
 tc/f_u32.c        |    5 ++---
 tc/m_action.c     |   26 ++++++++++----------------
 tc/m_bpf.c        |    5 ++---
 tc/m_connmark.c   |    5 ++---
 tc/m_csum.c       |    5 ++---
 tc/m_ematch.c     |   16 +++++++---------
 tc/m_gact.c       |    5 ++---
 tc/m_ife.c        |   10 ++++------
 tc/m_ipt.c        |    5 ++---
 tc/m_mirred.c     |    5 ++---
 tc/m_nat.c        |    5 ++---
 tc/m_pedit.c      |    5 ++---
 tc/m_police.c     |    5 ++---
 tc/m_sample.c     |    5 ++---
 tc/m_simple.c     |    5 ++---
 tc/m_skbedit.c    |    5 ++---
 tc/m_skbmod.c     |    5 ++---
 tc/m_tunnel_key.c |    5 ++---
 tc/m_vlan.c       |    5 ++---
 tc/m_xt.c         |    5 ++---
 tc/m_xt_old.c     |    5 ++---
 tc/q_atm.c        |   11 ++++++-----
 tc/q_cbq.c        |   10 ++++------
 tc/q_cbs.c        |    5 ++---
 tc/q_choke.c      |    5 ++---
 tc/q_codel.c      |    5 ++---
 tc/q_drr.c        |    5 ++---
 tc/q_dsmark.c     |   13 ++++++-------
 tc/q_fq.c         |    5 ++---
 tc/q_fq_codel.c   |    5 ++---
 tc/q_gred.c       |   10 ++++------
 tc/q_hfsc.c       |    6 ++----
 tc/q_hhf.c        |    5 ++---
 tc/q_htb.c        |   10 ++++------
 tc/q_mqprio.c     |    5 ++---
 tc/q_netem.c      |    7 ++-----
 tc/q_pie.c        |    5 ++---
 tc/q_qfq.c        |    5 ++---
 tc/q_red.c        |    5 ++---
 tc/q_sfb.c        |    5 ++---
 tc/q_tbf.c        |    5 ++---
 tc/tc_qdisc.c     |    5 ++---
 51 files changed, 132 insertions(+), 193 deletions(-)

diff --git a/ip/iplink.c b/ip/iplink.c
index 6cbd04f..f3834af 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -1025,9 +1025,9 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
 		else
 			iflatype = IFLA_INFO_DATA;
 		if (lu && argc) {
-			struct rtattr *data
-				= addattr_nest(&req.n,
-					       sizeof(req), iflatype);
+			struct rtattr *data;
+
+			data = addattr_nest(&req.n, sizeof(req), iflatype);
 
 			if (lu->parse_opt &&
 			    lu->parse_opt(lu, argc, argv, &req.n))
diff --git a/ip/iplink_vlan.c b/ip/iplink_vlan.c
index 4d78cf9..74f4614 100644
--- a/ip/iplink_vlan.c
+++ b/ip/iplink_vlan.c
@@ -56,8 +56,7 @@ static int vlan_parse_qos_map(int *argcp, char ***argvp, struct nlmsghdr *n,
 	struct ifla_vlan_qos_mapping m;
 	struct rtattr *tail;
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 1024, attrtype, NULL, 0);
+	tail = addattr_nest(n, 1024, attrtype);
 
 	while (argc > 0) {
 		char *colon = strchr(*argv, ':');
@@ -75,7 +74,7 @@ static int vlan_parse_qos_map(int *argcp, char ***argvp, struct nlmsghdr *n,
 		addattr_l(n, 1024, IFLA_VLAN_QOS_MAPPING, &m, sizeof(m));
 	}
 
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *)tail;
+	addattr_nest_end(n, tail);
 
 	*argcp = argc;
 	*argvp = argv;
diff --git a/ip/iplink_vxcan.c b/ip/iplink_vxcan.c
index 87be5a6..ebe9e56 100644
--- a/ip/iplink_vxcan.c
+++ b/ip/iplink_vxcan.c
@@ -55,8 +55,7 @@ static int vxcan_parse_opt(struct link_util *lu, int argc, char **argv,
 	ifm->ifi_flags = 0;
 	ifm->ifi_change = 0;
 
-	data = NLMSG_TAIL(n);
-	addattr_l(n, 1024, VXCAN_INFO_PEER, NULL, 0);
+	data = addattr_nest(n, 1024, VXCAN_INFO_PEER);
 
 	n->nlmsg_len += sizeof(struct ifinfomsg);
 
@@ -83,7 +82,7 @@ static int vxcan_parse_opt(struct link_util *lu, int argc, char **argv,
 	if (group != -1)
 		addattr32(n, 1024, IFLA_GROUP, group);
 
-	data->rta_len = (void *)NLMSG_TAIL(n) - (void *)data;
+	addattr_nest_end(n, data);
 	return argc - 1 - err;
 }
 
diff --git a/ip/link_veth.c b/ip/link_veth.c
index e6219e7..a8e7cf7 100644
--- a/ip/link_veth.c
+++ b/ip/link_veth.c
@@ -53,8 +53,7 @@ static int veth_parse_opt(struct link_util *lu, int argc, char **argv,
 	ifm->ifi_flags = 0;
 	ifm->ifi_change = 0;
 
-	data = NLMSG_TAIL(n);
-	addattr_l(n, 1024, VETH_INFO_PEER, NULL, 0);
+	data = addattr_nest(n, 1024, VETH_INFO_PEER);
 
 	n->nlmsg_len += sizeof(struct ifinfomsg);
 
@@ -81,7 +80,7 @@ static int veth_parse_opt(struct link_util *lu, int argc, char **argv,
 	if (group != -1)
 		addattr32(n, 1024, IFLA_GROUP, group);
 
-	data->rta_len = (void *)NLMSG_TAIL(n) - (void *)data;
+	addattr_nest_end(n, data);
 	return argc - 1 - err;
 }
 
diff --git a/tc/f_flow.c b/tc/f_flow.c
index b157104..badeaa2 100644
--- a/tc/f_flow.c
+++ b/tc/f_flow.c
@@ -147,8 +147,7 @@ static int flow_parse_opt(struct filter_util *fu, char *handle,
 		}
 	}
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 4096, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 4096, TCA_OPTIONS);
 
 	while (argc > 0) {
 		if (matches(*argv, "map") == 0) {
@@ -259,7 +258,7 @@ static int flow_parse_opt(struct filter_util *fu, char *handle,
 		addattr32(n, 4096, TCA_FLOW_XOR, xor);
 	}
 
-	tail->rta_len = (void *)NLMSG_TAIL(n) - (void *)tail;
+	addattr_nest_end(n, tail);
 	return 0;
 }
 
diff --git a/tc/f_fw.c b/tc/f_fw.c
index 7747484..adce2bd 100644
--- a/tc/f_fw.c
+++ b/tc/f_fw.c
@@ -67,8 +67,7 @@ static int fw_parse_opt(struct filter_util *qu, char *handle, int argc, char **a
 	if (argc == 0)
 		return 0;
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 4096, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 4096, TCA_OPTIONS);
 
 	if (mask_set)
 		addattr32(n, MAX_MSG, TCA_FW_MASK, mask);
@@ -119,7 +118,7 @@ static int fw_parse_opt(struct filter_util *qu, char *handle, int argc, char **a
 		}
 		argc--; argv++;
 	}
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 	return 0;
 }
 
diff --git a/tc/f_route.c b/tc/f_route.c
index 655321f..e52da64 100644
--- a/tc/f_route.c
+++ b/tc/f_route.c
@@ -50,8 +50,7 @@ static int route_parse_opt(struct filter_util *qu, char *handle, int argc, char
 	if (argc == 0)
 		return 0;
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 4096, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 4096, TCA_OPTIONS);
 
 	while (argc > 0) {
 		if (matches(*argv, "to") == 0) {
@@ -128,7 +127,7 @@ static int route_parse_opt(struct filter_util *qu, char *handle, int argc, char
 		}
 		argc--; argv++;
 	}
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 	if (order) {
 		fh &= ~0x7F00;
 		fh |= (order<<8)&0x7F00;
diff --git a/tc/f_rsvp.c b/tc/f_rsvp.c
index 1ce3734..bddd474 100644
--- a/tc/f_rsvp.c
+++ b/tc/f_rsvp.c
@@ -188,8 +188,7 @@ static int rsvp_parse_opt(struct filter_util *qu, char *handle, int argc,
 	if (argc == 0)
 		return 0;
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 4096, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 4096, TCA_OPTIONS);
 
 	while (argc > 0) {
 		if (matches(*argv, "session") == 0) {
@@ -294,7 +293,7 @@ static int rsvp_parse_opt(struct filter_util *qu, char *handle, int argc,
 
 	if (pinfo_ok)
 		addattr_l(n, 4096, TCA_RSVP_PINFO, &pinfo, sizeof(pinfo));
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 	return 0;
 }
 
diff --git a/tc/f_tcindex.c b/tc/f_tcindex.c
index 749273d..159cf41 100644
--- a/tc/f_tcindex.c
+++ b/tc/f_tcindex.c
@@ -37,8 +37,7 @@ static int tcindex_parse_opt(struct filter_util *qu, char *handle, int argc,
 		}
 	}
 	if (!argc) return 0;
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 4096, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 4096, TCA_OPTIONS);
 	while (argc) {
 		if (!strcmp(*argv, "hash")) {
 			int hash;
@@ -113,7 +112,7 @@ static int tcindex_parse_opt(struct filter_util *qu, char *handle, int argc,
 		argc--;
 		argv++;
 	}
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 	return 0;
 }
 
diff --git a/tc/f_u32.c b/tc/f_u32.c
index 1fafb4a..019d56c 100644
--- a/tc/f_u32.c
+++ b/tc/f_u32.c
@@ -1003,8 +1003,7 @@ static int u32_parse_opt(struct filter_util *qu, char *handle,
 	if (argc == 0)
 		return 0;
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, MAX_MSG, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, MAX_MSG, TCA_OPTIONS);
 
 	while (argc > 0) {
 		if (matches(*argv, "match") == 0) {
@@ -1197,7 +1196,7 @@ static int u32_parse_opt(struct filter_util *qu, char *handle,
 		addattr_l(n, MAX_MSG, TCA_U32_FLAGS, &flags, 4);
 	}
 
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 	return 0;
 }
 
diff --git a/tc/m_action.c b/tc/m_action.c
index 445d0b6..744bde4 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -166,9 +166,7 @@ int parse_action(int *argc_p, char ***argv_p, int tca_id, struct nlmsghdr *n)
 	if (argc <= 0)
 		return -1;
 
-	tail = tail2 = NLMSG_TAIL(n);
-
-	addattr_l(n, MAX_MSG, tca_id, NULL, 0);
+	tail2 = addattr_nest(n, MAX_MSG, tca_id);
 
 	while (argc > 0) {
 
@@ -213,8 +211,7 @@ done0:
 				goto bad_val;
 
 
-			tail = NLMSG_TAIL(n);
-			addattr_l(n, MAX_MSG, ++prio, NULL, 0);
+			tail = addattr_nest(n, MAX_MSG, ++prio);
 			addattr_l(n, MAX_MSG, TCA_ACT_KIND, k, strlen(k) + 1);
 
 			ret = a->parse_aopt(a, &argc, &argv, TCA_ACT_OPTIONS,
@@ -252,7 +249,7 @@ done0:
 				addattr_l(n, MAX_MSG, TCA_ACT_COOKIE,
 					  &act_ck, act_ck_len);
 
-			tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+			addattr_nest_end(n, tail);
 			ok++;
 		}
 	}
@@ -262,7 +259,7 @@ done0:
 		goto bad_val;
 	}
 
-	tail2->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail2;
+	addattr_nest_end(n, tail2);
 
 done:
 	*argc_p = argc;
@@ -468,8 +465,7 @@ static int tc_action_gd(int cmd, unsigned int flags,
 	argv += 1;
 
 
-	tail = NLMSG_TAIL(&req.n);
-	addattr_l(&req.n, MAX_MSG, TCA_ACT_TAB, NULL, 0);
+	tail = addattr_nest(&req.n, MAX_MSG, TCA_ACT_TAB);
 
 	while (argc > 0) {
 		if (strcmp(*argv, "action") == 0) {
@@ -518,16 +514,15 @@ static int tc_action_gd(int cmd, unsigned int flags,
 			goto bad_val;
 		}
 
-		tail2 = NLMSG_TAIL(&req.n);
-		addattr_l(&req.n, MAX_MSG, ++prio, NULL, 0);
+		tail2 = addattr_nest(&req.n, MAX_MSG, ++prio);
 		addattr_l(&req.n, MAX_MSG, TCA_ACT_KIND, k, strlen(k) + 1);
 		if (i > 0)
 			addattr32(&req.n, MAX_MSG, TCA_ACT_INDEX, i);
-		tail2->rta_len = (void *) NLMSG_TAIL(&req.n) - (void *) tail2;
+		addattr_nest_end(&req.n, tail2);
 
 	}
 
-	tail->rta_len = (void *) NLMSG_TAIL(&req.n) - (void *) tail;
+	addattr_nest_end(&req.n, tail);
 
 	req.n.nlmsg_seq = rth.dump = ++rth.seq;
 
@@ -626,8 +621,7 @@ static int tc_act_list_or_flush(int *argc_p, char ***argv_p, int event)
 		.t.tca_family = AF_UNSPEC,
 	};
 
-	tail = NLMSG_TAIL(&req.n);
-	addattr_l(&req.n, MAX_MSG, TCA_ACT_TAB, NULL, 0);
+	tail = addattr_nest(&req.n, MAX_MSG, TCA_ACT_TAB);
 	tail2 = NLMSG_TAIL(&req.n);
 
 	strncpy(k, *argv, sizeof(k) - 1);
@@ -659,7 +653,7 @@ static int tc_act_list_or_flush(int *argc_p, char ***argv_p, int event)
 	addattr_l(&req.n, MAX_MSG, ++prio, NULL, 0);
 	addattr_l(&req.n, MAX_MSG, TCA_ACT_KIND, k, strlen(k) + 1);
 	tail2->rta_len = (void *) NLMSG_TAIL(&req.n) - (void *) tail2;
-	tail->rta_len = (void *) NLMSG_TAIL(&req.n) - (void *) tail;
+	addattr_nest_end(&req.n, tail);
 
 	tail3 = NLMSG_TAIL(&req.n);
 	flag_select.value |= TCA_FLAG_LARGE_DUMP_ON;
diff --git a/tc/m_bpf.c b/tc/m_bpf.c
index 576f69c..27dcfd7 100644
--- a/tc/m_bpf.c
+++ b/tc/m_bpf.c
@@ -90,8 +90,7 @@ static int bpf_parse_opt(struct action_util *a, int *ptr_argc, char ***ptr_argv,
 
 	NEXT_ARG();
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, MAX_MSG, tca_id, NULL, 0);
+	tail = addattr_nest(n, MAX_MSG, tca_id);
 
 	while (argc > 0) {
 		if (matches(*argv, "run") == 0) {
@@ -144,7 +143,7 @@ opt_bpf:
 	}
 
 	addattr_l(n, MAX_MSG, TCA_ACT_BPF_PARMS, &parm, sizeof(parm));
-	tail->rta_len = (char *)NLMSG_TAIL(n) - (char *)tail;
+	addattr_nest_end(n, tail);
 
 	if (bpf_uds_name)
 		ret = bpf_send_map_fds(bpf_uds_name, bpf_obj);
diff --git a/tc/m_connmark.c b/tc/m_connmark.c
index 47c7a8c..56d1db9 100644
--- a/tc/m_connmark.c
+++ b/tc/m_connmark.c
@@ -96,10 +96,9 @@ parse_connmark(struct action_util *a, int *argc_p, char ***argv_p, int tca_id,
 		}
 	}
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, MAX_MSG, tca_id, NULL, 0);
+	tail = addattr_nest(n, MAX_MSG, tca_id);
 	addattr_l(n, MAX_MSG, TCA_CONNMARK_PARMS, &sel, sizeof(sel));
-	tail->rta_len = (char *)NLMSG_TAIL(n) - (char *)tail;
+	addattr_nest_end(n, tail);
 
 	*argc_p = argc;
 	*argv_p = argv;
diff --git a/tc/m_csum.c b/tc/m_csum.c
index e1352c0..42d37ab 100644
--- a/tc/m_csum.c
+++ b/tc/m_csum.c
@@ -139,10 +139,9 @@ parse_csum(struct action_util *a, int *argc_p,
 		}
 	}
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, MAX_MSG, tca_id, NULL, 0);
+	tail = addattr_nest(n, MAX_MSG, tca_id);
 	addattr_l(n, MAX_MSG, TCA_CSUM_PARMS, &sel, sizeof(sel));
-	tail->rta_len = (char *)NLMSG_TAIL(n) - (char *)tail;
+	addattr_nest_end(n, tail);
 
 	*argc_p = argc;
 	*argv_p = argv;
diff --git a/tc/m_ematch.c b/tc/m_ematch.c
index d2bb5c3..7dbc2e7 100644
--- a/tc/m_ematch.c
+++ b/tc/m_ematch.c
@@ -175,13 +175,13 @@ static int parse_tree(struct nlmsghdr *n, struct ematch *tree)
 	struct ematch *t;
 
 	for (t = tree; t; t = t->next) {
-		struct rtattr *tail = NLMSG_TAIL(n);
+		struct rtattr *tail;
 		struct tcf_ematch_hdr hdr = { .flags = t->relation };
 
 		if (t->inverted)
 			hdr.flags |= TCF_EM_INVERT;
 
-		addattr_l(n, MAX_MSG, index++, NULL, 0);
+		tail = addattr_nest(n, MAX_MSG, index++);
 
 		if (t->child) {
 			__u32 r = t->child_ref;
@@ -216,7 +216,7 @@ static int parse_tree(struct nlmsghdr *n, struct ematch *tree)
 				return -1;
 		}
 
-		tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+		addattr_nest_end(n, tail);
 	}
 
 	return 0;
@@ -341,18 +341,16 @@ int parse_ematch(int *argc_p, char ***argv_p, int tca_id, struct nlmsghdr *n)
 			.progid = TCF_EM_PROG_TC
 		};
 
-		tail = NLMSG_TAIL(n);
-		addattr_l(n, MAX_MSG, tca_id, NULL, 0);
+		tail = addattr_nest(n, MAX_MSG, tca_id);
 		addattr_l(n, MAX_MSG, TCA_EMATCH_TREE_HDR, &hdr, sizeof(hdr));
 
-		tail_list = NLMSG_TAIL(n);
-		addattr_l(n, MAX_MSG, TCA_EMATCH_TREE_LIST, NULL, 0);
+		tail_list = addattr_nest(n, MAX_MSG, TCA_EMATCH_TREE_LIST);
 
 		if (parse_tree(n, ematch_root) < 0)
 			return -1;
 
-		tail_list->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail_list;
-		tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+		addattr_nest_end(n, tail_list);
+		addattr_nest_end(n, tail);
 	}
 
 	*argc_p = ematch_argc;
diff --git a/tc/m_gact.c b/tc/m_gact.c
index b30b042..ccad4f2 100644
--- a/tc/m_gact.c
+++ b/tc/m_gact.c
@@ -148,14 +148,13 @@ parse_gact(struct action_util *a, int *argc_p, char ***argv_p,
 		}
 	}
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, MAX_MSG, tca_id, NULL, 0);
+	tail = addattr_nest(n, MAX_MSG, tca_id);
 	addattr_l(n, MAX_MSG, TCA_GACT_PARMS, &p, sizeof(p));
 #ifdef CONFIG_GACT_PROB
 	if (rd)
 		addattr_l(n, MAX_MSG, TCA_GACT_PROB, &pp, sizeof(pp));
 #endif
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 
 	*argc_p = argc;
 	*argv_p = argv;
diff --git a/tc/m_ife.c b/tc/m_ife.c
index 4647f6a..43d8918 100644
--- a/tc/m_ife.c
+++ b/tc/m_ife.c
@@ -178,8 +178,7 @@ static int parse_ife(struct action_util *a, int *argc_p, char ***argv_p,
 		ife_usage();
 	}
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, MAX_MSG, tca_id, NULL, 0);
+	tail = addattr_nest(n, MAX_MSG, tca_id);
 	addattr_l(n, MAX_MSG, TCA_IFE_PARMS, &p, sizeof(p));
 
 	if (!(p.flags & IFE_ENCODE))
@@ -194,8 +193,7 @@ static int parse_ife(struct action_util *a, int *argc_p, char ***argv_p,
 	if (saddr)
 		addattr_l(n, MAX_MSG, TCA_IFE_SMAC, sbuf, ETH_ALEN);
 
-	tail2 = NLMSG_TAIL(n);
-	addattr_l(n, MAX_MSG, TCA_IFE_METALST, NULL, 0);
+	tail2 = addattr_nest(n, MAX_MSG, TCA_IFE_METALST);
 	if (ife_mark || ife_mark_v) {
 		if (ife_mark_v)
 			addattr_l(n, MAX_MSG, IFE_META_SKBMARK, &ife_mark_v, 4);
@@ -216,10 +214,10 @@ static int parse_ife(struct action_util *a, int *argc_p, char ***argv_p,
 			addattr_l(n, MAX_MSG, IFE_META_TCINDEX, NULL, 0);
 	}
 
-	tail2->rta_len = (void *)NLMSG_TAIL(n) - (void *)tail2;
+	addattr_nest_end(n, tail2);
 
 skip_encode:
-	tail->rta_len = (void *)NLMSG_TAIL(n) - (void *)tail;
+	addattr_nest_end(n, tail);
 
 	*argc_p = argc;
 	*argv_p = argv;
diff --git a/tc/m_ipt.c b/tc/m_ipt.c
index 1c3c240..b48cc0a 100644
--- a/tc/m_ipt.c
+++ b/tc/m_ipt.c
@@ -383,8 +383,7 @@ static int parse_ipt(struct action_util *a, int *argc_p,
 		}
 	}
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, MAX_MSG, tca_id, NULL, 0);
+	tail = addattr_nest(n, MAX_MSG, tca_id);
 	fprintf(stdout, "tablename: %s hook: %s\n ", tname, ipthooks[hook]);
 	fprintf(stdout, "\ttarget: ");
 
@@ -405,7 +404,7 @@ static int parse_ipt(struct action_util *a, int *argc_p,
 	addattr_l(n, MAX_MSG, TCA_IPT_INDEX, &index, 4);
 	if (m)
 		addattr_l(n, MAX_MSG, TCA_IPT_TARG, m->t, m->t->u.target_size);
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 
 	argc -= optind;
 	argv += optind;
diff --git a/tc/m_mirred.c b/tc/m_mirred.c
index aa7ce6d..eb42b7c 100644
--- a/tc/m_mirred.c
+++ b/tc/m_mirred.c
@@ -225,10 +225,9 @@ parse_direction(struct action_util *a, int *argc_p, char ***argv_p,
 		}
 	}
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, MAX_MSG, tca_id, NULL, 0);
+	tail = addattr_nest(n, MAX_MSG, tca_id);
 	addattr_l(n, MAX_MSG, TCA_MIRRED_PARMS, &p, sizeof(p));
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 
 	*argc_p = argc;
 	*argv_p = argv;
diff --git a/tc/m_nat.c b/tc/m_nat.c
index f5de4d4..1bb6495 100644
--- a/tc/m_nat.c
+++ b/tc/m_nat.c
@@ -129,10 +129,9 @@ parse_nat(struct action_util *a, int *argc_p, char ***argv_p, int tca_id, struct
 		}
 	}
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, MAX_MSG, tca_id, NULL, 0);
+	tail = addattr_nest(n, MAX_MSG, tca_id);
 	addattr_l(n, MAX_MSG, TCA_NAT_PARMS, &sel, sizeof(sel));
-	tail->rta_len = (char *)NLMSG_TAIL(n) - (char *)tail;
+	addattr_nest_end(n, tail);
 
 	*argc_p = argc;
 	*argv_p = argv;
diff --git a/tc/m_pedit.c b/tc/m_pedit.c
index dc57f14..2d41f0a 100644
--- a/tc/m_pedit.c
+++ b/tc/m_pedit.c
@@ -686,8 +686,7 @@ int parse_pedit(struct action_util *a, int *argc_p, char ***argv_p, int tca_id,
 		}
 	}
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, MAX_MSG, tca_id, NULL, 0);
+	tail = addattr_nest(n, MAX_MSG, tca_id);
 	if (!sel.extended) {
 		addattr_l(n, MAX_MSG, TCA_PEDIT_PARMS, &sel,
 			  sizeof(sel.sel) +
@@ -700,7 +699,7 @@ int parse_pedit(struct action_util *a, int *argc_p, char ***argv_p, int tca_id,
 		pedit_keys_ex_addattr(&sel, n);
 	}
 
-	tail->rta_len = (void *)NLMSG_TAIL(n) - (void *)tail;
+	addattr_nest_end(n, tail);
 
 	*argc_p = argc;
 	*argv_p = argv;
diff --git a/tc/m_police.c b/tc/m_police.c
index ff1dcb7..f85da49 100644
--- a/tc/m_police.c
+++ b/tc/m_police.c
@@ -228,8 +228,7 @@ int act_parse_police(struct action_util *a, int *argc_p, char ***argv_p,
 		}
 	}
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, MAX_MSG, tca_id, NULL, 0);
+	tail = addattr_nest(n, MAX_MSG, tca_id);
 	addattr_l(n, MAX_MSG, TCA_POLICE_TBF, &p, sizeof(p));
 	if (p.rate.rate)
 		addattr_l(n, MAX_MSG, TCA_POLICE_RATE, rtab, 1024);
@@ -240,7 +239,7 @@ int act_parse_police(struct action_util *a, int *argc_p, char ***argv_p,
 	if (presult)
 		addattr32(n, MAX_MSG, TCA_POLICE_RESULT, presult);
 
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 	res = 0;
 
 	*argc_p = argc;
diff --git a/tc/m_sample.c b/tc/m_sample.c
index 31774c0..fe892ad 100644
--- a/tc/m_sample.c
+++ b/tc/m_sample.c
@@ -122,8 +122,7 @@ static int parse_sample(struct action_util *a, int *argc_p, char ***argv_p,
 		usage();
 	}
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, MAX_MSG, tca_id, NULL, 0);
+	tail = addattr_nest(n, MAX_MSG, tca_id);
 	addattr_l(n, MAX_MSG, TCA_SAMPLE_PARMS, &p, sizeof(p));
 	if (rate_set)
 		addattr32(n, MAX_MSG, TCA_SAMPLE_RATE, rate);
@@ -132,7 +131,7 @@ static int parse_sample(struct action_util *a, int *argc_p, char ***argv_p,
 	if (trunc_set)
 		addattr32(n, MAX_MSG, TCA_SAMPLE_TRUNC_SIZE, trunc);
 
-	tail->rta_len = (char *)NLMSG_TAIL(n) - (char *)tail;
+	addattr_nest_end(n, tail);
 
 	*argc_p = argc;
 	*argv_p = argv;
diff --git a/tc/m_simple.c b/tc/m_simple.c
index a687b9f..886606f 100644
--- a/tc/m_simple.c
+++ b/tc/m_simple.c
@@ -146,12 +146,11 @@ parse_simple(struct action_util *a, int *argc_p, char ***argv_p, int tca_id,
 
 	sel.action = TC_ACT_PIPE;
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, MAX_MSG, tca_id, NULL, 0);
+	tail = addattr_nest(n, MAX_MSG, tca_id);
 	addattr_l(n, MAX_MSG, TCA_DEF_PARMS, &sel, sizeof(sel));
 	if (simpdata)
 		addattr_l(n, MAX_MSG, TCA_DEF_DATA, simpdata, SIMP_MAX_DATA);
-	tail->rta_len = (char *)NLMSG_TAIL(n) - (char *)tail;
+	addattr_nest_end(n, tail);
 
 	*argc_p = argc;
 	*argv_p = argv;
diff --git a/tc/m_skbedit.c b/tc/m_skbedit.c
index c41a7bb..c1eda30 100644
--- a/tc/m_skbedit.c
+++ b/tc/m_skbedit.c
@@ -143,8 +143,7 @@ parse_skbedit(struct action_util *a, int *argc_p, char ***argv_p, int tca_id,
 	}
 
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, MAX_MSG, tca_id, NULL, 0);
+	tail = addattr_nest(n, MAX_MSG, tca_id);
 	addattr_l(n, MAX_MSG, TCA_SKBEDIT_PARMS, &sel, sizeof(sel));
 	if (flags & SKBEDIT_F_QUEUE_MAPPING)
 		addattr_l(n, MAX_MSG, TCA_SKBEDIT_QUEUE_MAPPING,
@@ -158,7 +157,7 @@ parse_skbedit(struct action_util *a, int *argc_p, char ***argv_p, int tca_id,
 	if (flags & SKBEDIT_F_PTYPE)
 		addattr_l(n, MAX_MSG, TCA_SKBEDIT_PTYPE,
 			  &ptype, sizeof(ptype));
-	tail->rta_len = (char *)NLMSG_TAIL(n) - (char *)tail;
+	addattr_nest_end(n, tail);
 
 	*argc_p = argc;
 	*argv_p = argv;
diff --git a/tc/m_skbmod.c b/tc/m_skbmod.c
index bc268df..6721f87 100644
--- a/tc/m_skbmod.c
+++ b/tc/m_skbmod.c
@@ -143,8 +143,7 @@ static int parse_skbmod(struct action_util *a, int *argc_p, char ***argv_p,
 		skbmod_usage();
 	}
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, MAX_MSG, tca_id, NULL, 0);
+	tail = addattr_nest(n, MAX_MSG, tca_id);
 	addattr_l(n, MAX_MSG, TCA_SKBMOD_PARMS, &p, sizeof(p));
 
 	if (daddr)
@@ -154,7 +153,7 @@ static int parse_skbmod(struct action_util *a, int *argc_p, char ***argv_p,
 	if (saddr)
 		addattr_l(n, MAX_MSG, TCA_SKBMOD_SMAC, sbuf, ETH_ALEN);
 
-	tail->rta_len = (void *)NLMSG_TAIL(n) - (void *)tail;
+	addattr_nest_end(n, tail);
 
 	*argc_p = argc;
 	*argv_p = argv;
diff --git a/tc/m_tunnel_key.c b/tc/m_tunnel_key.c
index 2dc9187..8d4cee1 100644
--- a/tc/m_tunnel_key.c
+++ b/tc/m_tunnel_key.c
@@ -98,8 +98,7 @@ static int parse_tunnel_key(struct action_util *a, int *argc_p, char ***argv_p,
 	if (matches(*argv, "tunnel_key") != 0)
 		return -1;
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, MAX_MSG, tca_id, NULL, 0);
+	tail = addattr_nest(n, MAX_MSG, tca_id);
 
 	NEXT_ARG();
 
@@ -197,7 +196,7 @@ static int parse_tunnel_key(struct action_util *a, int *argc_p, char ***argv_p,
 
 	parm.t_action = action;
 	addattr_l(n, MAX_MSG, TCA_TUNNEL_KEY_PARMS, &parm, sizeof(parm));
-	tail->rta_len = (char *)NLMSG_TAIL(n) - (char *)tail;
+	addattr_nest_end(n, tail);
 
 	*argc_p = argc;
 	*argv_p = argv;
diff --git a/tc/m_vlan.c b/tc/m_vlan.c
index edae0d1..9ee52da 100644
--- a/tc/m_vlan.c
+++ b/tc/m_vlan.c
@@ -152,8 +152,7 @@ static int parse_vlan(struct action_util *a, int *argc_p, char ***argv_p,
 	}
 
 	parm.v_action = action;
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, MAX_MSG, tca_id, NULL, 0);
+	tail = addattr_nest(n, MAX_MSG, tca_id);
 	addattr_l(n, MAX_MSG, TCA_VLAN_PARMS, &parm, sizeof(parm));
 	if (id_set)
 		addattr_l(n, MAX_MSG, TCA_VLAN_PUSH_VLAN_ID, &id, 2);
@@ -170,7 +169,7 @@ static int parse_vlan(struct action_util *a, int *argc_p, char ***argv_p,
 	if (prio_set)
 		addattr8(n, MAX_MSG, TCA_VLAN_PUSH_VLAN_PRIORITY, prio);
 
-	tail->rta_len = (char *)NLMSG_TAIL(n) - (char *)tail;
+	addattr_nest_end(n, tail);
 
 	*argc_p = argc;
 	*argv_p = argv;
diff --git a/tc/m_xt.c b/tc/m_xt.c
index a1137be..29574bd 100644
--- a/tc/m_xt.c
+++ b/tc/m_xt.c
@@ -264,8 +264,7 @@ static int parse_ipt(struct action_util *a, int *argc_p,
 		}
 	}
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, MAX_MSG, tca_id, NULL, 0);
+	tail = addattr_nest(n, MAX_MSG, tca_id);
 	fprintf(stdout, "tablename: %s hook: %s\n ", tname, ipthooks[hook]);
 	fprintf(stdout, "\ttarget: ");
 
@@ -290,7 +289,7 @@ static int parse_ipt(struct action_util *a, int *argc_p,
 	addattr_l(n, MAX_MSG, TCA_IPT_INDEX, &index, 4);
 	if (m)
 		addattr_l(n, MAX_MSG, TCA_IPT_TARG, m->t, m->t->u.target_size);
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 
 	argv += optind;
 	*argc_p -= argc;
diff --git a/tc/m_xt_old.c b/tc/m_xt_old.c
index 21d9087..313bea6 100644
--- a/tc/m_xt_old.c
+++ b/tc/m_xt_old.c
@@ -308,8 +308,7 @@ static int parse_ipt(struct action_util *a, int *argc_p,
 		}
 	}
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, MAX_MSG, tca_id, NULL, 0);
+	tail = addattr_nest(n, MAX_MSG, tca_id);
 	fprintf(stdout, "tablename: %s hook: %s\n ", tname, ipthooks[hook]);
 	fprintf(stdout, "\ttarget: ");
 
@@ -330,7 +329,7 @@ static int parse_ipt(struct action_util *a, int *argc_p,
 	addattr_l(n, MAX_MSG, TCA_IPT_INDEX, &index, 4);
 	if (m)
 		addattr_l(n, MAX_MSG, TCA_IPT_TARG, m->t, m->t->u.target_size);
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 
 	argc -= optind;
 	argv += optind;
diff --git a/tc/q_atm.c b/tc/q_atm.c
index 3ea4cf4..f8215f0 100644
--- a/tc/q_atm.c
+++ b/tc/q_atm.c
@@ -167,12 +167,13 @@ static int atm_parse_class_opt(struct qdisc_util *qu, int argc, char **argv,
 			perror("ioctl ATMARP_MKIP");
 			return -1;
 		}
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 1024, TCA_OPTIONS);
 	addattr_l(n, 1024, TCA_ATM_FD, &s, sizeof(s));
-	if (excess) addattr_l(n, 1024, TCA_ATM_EXCESS, &excess, sizeof(excess));
-	if (hdr_len != -1) addattr_l(n, 1024, TCA_ATM_HDR, hdr, hdr_len);
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	if (excess)
+		addattr_l(n, 1024, TCA_ATM_EXCESS, &excess, sizeof(excess));
+	if (hdr_len != -1)
+		addattr_l(n, 1024, TCA_ATM_HDR, hdr, hdr_len);
+	addattr_nest_end(n, tail);
 	return 0;
 }
 
diff --git a/tc/q_cbq.c b/tc/q_cbq.c
index d05fe9c..e7f1a3b 100644
--- a/tc/q_cbq.c
+++ b/tc/q_cbq.c
@@ -165,8 +165,7 @@ static int cbq_parse_opt(struct qdisc_util *qu, int argc, char **argv, struct nl
 	lss.change = TCF_CBQ_LSS_MAXIDLE|TCF_CBQ_LSS_EWMA|TCF_CBQ_LSS_AVPKT;
 	lss.avpkt = avpkt;
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 1024, TCA_OPTIONS);
 	addattr_l(n, 1024, TCA_CBQ_RATE, &r, sizeof(r));
 	addattr_l(n, 1024, TCA_CBQ_LSSOPT, &lss, sizeof(lss));
 	addattr_l(n, 3024, TCA_CBQ_RTAB, rtab, 1024);
@@ -177,7 +176,7 @@ static int cbq_parse_opt(struct qdisc_util *qu, int argc, char **argv, struct nl
 			printf("%u ", rtab[i]);
 		printf("\n");
 	}
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 	return 0;
 }
 
@@ -419,8 +418,7 @@ static int cbq_parse_class_opt(struct qdisc_util *qu, int argc, char **argv, str
 		lss.change |= TCF_CBQ_LSS_EWMA;
 	}
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 1024, TCA_OPTIONS);
 	if (lss.change) {
 		lss.change |= TCF_CBQ_LSS_FLAGS;
 		addattr_l(n, 1024, TCA_CBQ_LSSOPT, &lss, sizeof(lss));
@@ -440,7 +438,7 @@ static int cbq_parse_class_opt(struct qdisc_util *qu, int argc, char **argv, str
 			printf("\n");
 		}
 	}
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 	return 0;
 }
 
diff --git a/tc/q_cbs.c b/tc/q_cbs.c
index b573905..a2ffb1d 100644
--- a/tc/q_cbs.c
+++ b/tc/q_cbs.c
@@ -102,10 +102,9 @@ static int cbs_parse_opt(struct qdisc_util *qu, int argc,
 		argc--; argv++;
 	}
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 1024, TCA_OPTIONS);
 	addattr_l(n, 2024, TCA_CBS_PARMS, &opt, sizeof(opt));
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 	return 0;
 }
 
diff --git a/tc/q_choke.c b/tc/q_choke.c
index 50ac4ad..b269b13 100644
--- a/tc/q_choke.c
+++ b/tc/q_choke.c
@@ -156,13 +156,12 @@ static int choke_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 	if (ecn_ok)
 		opt.flags |= TC_RED_ECN;
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 1024, TCA_OPTIONS);
 	addattr_l(n, 1024, TCA_CHOKE_PARMS, &opt, sizeof(opt));
 	addattr_l(n, 1024, TCA_CHOKE_STAB, sbuf, 256);
 	max_P = probability * pow(2, 32);
 	addattr_l(n, 1024, TCA_CHOKE_MAX_P, &max_P, sizeof(max_P));
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 	return 0;
 }
 
diff --git a/tc/q_codel.c b/tc/q_codel.c
index 62d6dd6..8a2a871 100644
--- a/tc/q_codel.c
+++ b/tc/q_codel.c
@@ -107,8 +107,7 @@ static int codel_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 		argc--; argv++;
 	}
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 1024, TCA_OPTIONS);
 	if (limit)
 		addattr_l(n, 1024, TCA_CODEL_LIMIT, &limit, sizeof(limit));
 	if (interval)
@@ -121,7 +120,7 @@ static int codel_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 		addattr_l(n, 1024, TCA_CODEL_CE_THRESHOLD,
 			  &ce_threshold, sizeof(ce_threshold));
 
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 	return 0;
 }
 
diff --git a/tc/q_drr.c b/tc/q_drr.c
index 5e541c0..f9c90f3 100644
--- a/tc/q_drr.c
+++ b/tc/q_drr.c
@@ -55,8 +55,7 @@ static int drr_parse_class_opt(struct qdisc_util *qu, int argc, char **argv,
 	struct rtattr *tail;
 	__u32 tmp;
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 1024, TCA_OPTIONS);
 
 	while (argc > 0) {
 		if (strcmp(*argv, "quantum") == 0) {
@@ -77,7 +76,7 @@ static int drr_parse_class_opt(struct qdisc_util *qu, int argc, char **argv,
 		argc--; argv++;
 	}
 
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *)tail;
+	addattr_nest_end(n, tail);
 	return 0;
 }
 
diff --git a/tc/q_dsmark.c b/tc/q_dsmark.c
index 967fd89..d3e8292 100644
--- a/tc/q_dsmark.c
+++ b/tc/q_dsmark.c
@@ -64,16 +64,16 @@ static int dsmark_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 		explain();
 		return -1;
 	}
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 1024, TCA_OPTIONS);
 	addattr_l(n, 1024, TCA_DSMARK_INDICES, &ind, sizeof(ind));
 	if (dflt != -1) {
 	    __u16 tmp = dflt;
 
 	    addattr_l(n, 1024, TCA_DSMARK_DEFAULT_INDEX, &tmp, sizeof(tmp));
 	}
-	if (set_tc_index) addattr_l(n, 1024, TCA_DSMARK_SET_TC_INDEX, NULL, 0);
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	if (set_tc_index)
+		addattr_l(n, 1024, TCA_DSMARK_SET_TC_INDEX, NULL, 0);
+	addattr_nest_end(n, tail);
 	return 0;
 }
 
@@ -91,8 +91,7 @@ static int dsmark_parse_class_opt(struct qdisc_util *qu, int argc, char **argv,
 	__u8 tmp;
 	char *end;
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 1024, TCA_OPTIONS);
 	while (argc > 0) {
 		if (!strcmp(*argv, "mask")) {
 			NEXT_ARG();
@@ -117,7 +116,7 @@ static int dsmark_parse_class_opt(struct qdisc_util *qu, int argc, char **argv,
 		argc--;
 		argv++;
 	}
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 	return 0;
 }
 
diff --git a/tc/q_fq.c b/tc/q_fq.c
index 51b5bc3..f3dbf2b 100644
--- a/tc/q_fq.c
+++ b/tc/q_fq.c
@@ -190,8 +190,7 @@ static int fq_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 		argc--; argv++;
 	}
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 1024, TCA_OPTIONS);
 	if (buckets) {
 		unsigned int log = ilog2(buckets);
 
@@ -227,7 +226,7 @@ static int fq_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 	if (set_orphan_mask)
 		addattr_l(n, 1024, TCA_FQ_ORPHAN_MASK,
 			  &orphan_mask, sizeof(refill_delay));
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 	return 0;
 }
 
diff --git a/tc/q_fq_codel.c b/tc/q_fq_codel.c
index fd1f59c..9e3736f 100644
--- a/tc/q_fq_codel.c
+++ b/tc/q_fq_codel.c
@@ -126,8 +126,7 @@ static int fq_codel_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 		argc--; argv++;
 	}
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 1024, TCA_OPTIONS);
 	if (limit)
 		addattr_l(n, 1024, TCA_FQ_CODEL_LIMIT, &limit, sizeof(limit));
 	if (flows)
@@ -147,7 +146,7 @@ static int fq_codel_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 		addattr_l(n, 1024, TCA_FQ_CODEL_MEMORY_LIMIT,
 			  &memory, sizeof(memory));
 
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 	return 0;
 }
 
diff --git a/tc/q_gred.c b/tc/q_gred.c
index 5b5761e..e63fac7 100644
--- a/tc/q_gred.c
+++ b/tc/q_gred.c
@@ -105,12 +105,11 @@ static int init_gred(struct qdisc_util *qu, int argc, char **argv,
 
 	DPRINTF("TC_GRED: sending DPs=%u def_DP=%u\n", opt.DPs, opt.def_DP);
 	n->nlmsg_flags |= NLM_F_CREATE;
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 1024, TCA_OPTIONS);
 	addattr_l(n, 1024, TCA_GRED_DPS, &opt, sizeof(struct tc_gred_sopt));
 	if (limit)
 		addattr32(n, 1024, TCA_GRED_LIMIT, limit);
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 	return 0;
 }
 /*
@@ -257,13 +256,12 @@ static int gred_parse_opt(struct qdisc_util *qu, int argc, char **argv, struct n
 	}
 	opt.Scell_log = parm;
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 1024, TCA_OPTIONS);
 	addattr_l(n, 1024, TCA_GRED_PARMS, &opt, sizeof(opt));
 	addattr_l(n, 1024, TCA_GRED_STAB, sbuf, 256);
 	max_P = probability * pow(2, 32);
 	addattr32(n, 1024, TCA_GRED_MAX_P, max_P);
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 	return 0;
 }
 
diff --git a/tc/q_hfsc.c b/tc/q_hfsc.c
index c19e87f..f34b1b2 100644
--- a/tc/q_hfsc.c
+++ b/tc/q_hfsc.c
@@ -201,9 +201,7 @@ hfsc_parse_class_opt(struct qdisc_util *qu, int argc, char **argv,
 		return -1;
 	}
 
-	tail = NLMSG_TAIL(n);
-
-	addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 1024, TCA_OPTIONS);
 	if (rsc_ok)
 		addattr_l(n, 1024, TCA_HFSC_RSC, &rsc, sizeof(rsc));
 	if (fsc_ok)
@@ -211,7 +209,7 @@ hfsc_parse_class_opt(struct qdisc_util *qu, int argc, char **argv,
 	if (usc_ok)
 		addattr_l(n, 1024, TCA_HFSC_USC, &usc, sizeof(usc));
 
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 	return 0;
 }
 
diff --git a/tc/q_hhf.c b/tc/q_hhf.c
index 66c7188..21186a9 100644
--- a/tc/q_hhf.c
+++ b/tc/q_hhf.c
@@ -91,8 +91,7 @@ static int hhf_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 		argc--; argv++;
 	}
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 1024, TCA_OPTIONS);
 	if (limit)
 		addattr_l(n, 1024, TCA_HHF_BACKLOG_LIMIT, &limit,
 			  sizeof(limit));
@@ -113,7 +112,7 @@ static int hhf_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 	if (non_hh_weight)
 		addattr_l(n, 1024, TCA_HHF_NON_HH_WEIGHT, &non_hh_weight,
 			  sizeof(non_hh_weight));
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 	return 0;
 }
 
diff --git a/tc/q_htb.c b/tc/q_htb.c
index 3fc2acb..7d5f6ce 100644
--- a/tc/q_htb.c
+++ b/tc/q_htb.c
@@ -98,13 +98,12 @@ static int htb_parse_opt(struct qdisc_util *qu, int argc,
 		}
 		argc--; argv++;
 	}
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 1024, TCA_OPTIONS);
 	addattr_l(n, 2024, TCA_HTB_INIT, &opt, NLMSG_ALIGN(sizeof(opt)));
 	if (direct_qlen != ~0U)
 		addattr_l(n, 2024, TCA_HTB_DIRECT_QLEN,
 			  &direct_qlen, sizeof(direct_qlen));
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 	return 0;
 }
 
@@ -254,8 +253,7 @@ static int htb_parse_class_opt(struct qdisc_util *qu, int argc, char **argv, str
 	}
 	opt.cbuffer = tc_calc_xmittime(ceil64, cbuffer);
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 1024, TCA_OPTIONS);
 
 	if (rate64 >= (1ULL << 32))
 		addattr_l(n, 1124, TCA_HTB_RATE64, &rate64, sizeof(rate64));
@@ -266,7 +264,7 @@ static int htb_parse_class_opt(struct qdisc_util *qu, int argc, char **argv, str
 	addattr_l(n, 2024, TCA_HTB_PARMS, &opt, sizeof(opt));
 	addattr_l(n, 3024, TCA_HTB_RTAB, rtab, 1024);
 	addattr_l(n, 4024, TCA_HTB_CTAB, ctab, 1024);
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 	return 0;
 }
 
diff --git a/tc/q_mqprio.c b/tc/q_mqprio.c
index 89b4600..207d644 100644
--- a/tc/q_mqprio.c
+++ b/tc/q_mqprio.c
@@ -173,8 +173,7 @@ static int mqprio_parse_opt(struct qdisc_util *qu, int argc,
 		argc--; argv++;
 	}
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 1024, TCA_OPTIONS, &opt, sizeof(opt));
+	tail = addattr_nest_compat(n, 1024, TCA_OPTIONS, &opt, sizeof(opt));
 
 	if (flags & TC_MQPRIO_F_MODE)
 		addattr_l(n, 1024, TCA_MQPRIO_MODE,
@@ -209,7 +208,7 @@ static int mqprio_parse_opt(struct qdisc_util *qu, int argc,
 		addattr_nest_end(n, start);
 	}
 
-	tail->rta_len = (void *)NLMSG_TAIL(n) - (void *)tail;
+	addattr_nest_compat_end(n, tail);
 
 	return 0;
 }
diff --git a/tc/q_netem.c b/tc/q_netem.c
index 9f9a9b3..623ec90 100644
--- a/tc/q_netem.c
+++ b/tc/q_netem.c
@@ -422,8 +422,6 @@ static int netem_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 		}
 	}
 
-	tail = NLMSG_TAIL(n);
-
 	if (reorder.probability) {
 		if (opt.latency == 0) {
 			fprintf(stderr, "reordering not possible without specifying some delay\n");
@@ -452,8 +450,7 @@ static int netem_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 		return -1;
 	}
 
-	if (addattr_l(n, 1024, TCA_OPTIONS, &opt, sizeof(opt)) < 0)
-		return -1;
+	tail = addattr_nest_compat(n, 1024, TCA_OPTIONS, &opt, sizeof(opt));
 
 	if (present[TCA_NETEM_CORR] &&
 	    addattr_l(n, 1024, TCA_NETEM_CORR, &cor, sizeof(cor)) < 0)
@@ -512,7 +509,7 @@ static int netem_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 			return -1;
 		free(dist_data);
 	}
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_compat_end(n, tail);
 	return 0;
 }
 
diff --git a/tc/q_pie.c b/tc/q_pie.c
index b89f53c..f7924ef 100644
--- a/tc/q_pie.c
+++ b/tc/q_pie.c
@@ -103,8 +103,7 @@ static int pie_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 		argv++;
 	}
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 1024, TCA_OPTIONS);
 	if (limit)
 		addattr_l(n, 1024, TCA_PIE_LIMIT, &limit, sizeof(limit));
 	if (tupdate)
@@ -121,7 +120,7 @@ static int pie_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 		addattr_l(n, 1024, TCA_PIE_BYTEMODE, &bytemode,
 			  sizeof(bytemode));
 
-	tail->rta_len = (void *)NLMSG_TAIL(n) - (void *)tail;
+	addattr_nest_end(n, tail);
 	return 0;
 }
 
diff --git a/tc/q_qfq.c b/tc/q_qfq.c
index d70ca1b..eb8fa4b 100644
--- a/tc/q_qfq.c
+++ b/tc/q_qfq.c
@@ -53,8 +53,7 @@ static int qfq_parse_class_opt(struct qdisc_util *qu, int argc, char **argv,
 	struct rtattr *tail;
 	__u32 tmp;
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 4096, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 4096, TCA_OPTIONS);
 
 	while (argc > 0) {
 		if (matches(*argv, "weight") == 0) {
@@ -80,7 +79,7 @@ static int qfq_parse_class_opt(struct qdisc_util *qu, int argc, char **argv,
 		argc--; argv++;
 	}
 
-	tail->rta_len = (void *)NLMSG_TAIL(n) - (void *)tail;
+	addattr_nest_end(n, tail);
 
 	return 0;
 }
diff --git a/tc/q_red.c b/tc/q_red.c
index 40ba7c3..49fd4ac 100644
--- a/tc/q_red.c
+++ b/tc/q_red.c
@@ -148,13 +148,12 @@ static int red_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 	}
 	opt.Scell_log = parm;
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 1024, TCA_OPTIONS);
 	addattr_l(n, 1024, TCA_RED_PARMS, &opt, sizeof(opt));
 	addattr_l(n, 1024, TCA_RED_STAB, sbuf, 256);
 	max_P = probability * pow(2, 32);
 	addattr_l(n, 1024, TCA_RED_MAX_P, &max_P, sizeof(max_P));
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 	return 0;
 }
 
diff --git a/tc/q_sfb.c b/tc/q_sfb.c
index 4b366dd..7f48c6e 100644
--- a/tc/q_sfb.c
+++ b/tc/q_sfb.c
@@ -132,10 +132,9 @@ static int sfb_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 	if (opt.bin_size == 0)
 		opt.bin_size = (opt.max * 4 + 3) / 5;
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 1024, TCA_OPTIONS);
 	addattr_l(n, 1024, TCA_SFB_PARMS, &opt, sizeof(opt));
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 	return 0;
 }
 
diff --git a/tc/q_tbf.c b/tc/q_tbf.c
index dfaa5d3..b9465b2 100644
--- a/tc/q_tbf.c
+++ b/tc/q_tbf.c
@@ -241,8 +241,7 @@ static int tbf_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 		opt.mtu = tc_calc_xmittime(opt.peakrate.rate, mtu);
 	}
 
-	tail = NLMSG_TAIL(n);
-	addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+	tail = addattr_nest(n, 1024, TCA_OPTIONS);
 	addattr_l(n, 2024, TCA_TBF_PARMS, &opt, sizeof(opt));
 	addattr_l(n, 2124, TCA_TBF_BURST, &buffer, sizeof(buffer));
 	if (rate64 >= (1ULL << 32))
@@ -254,7 +253,7 @@ static int tbf_parse_opt(struct qdisc_util *qu, int argc, char **argv,
 		addattr_l(n, 3224, TCA_TBF_PBURST, &mtu, sizeof(mtu));
 		addattr_l(n, 4096, TCA_TBF_PTAB, ptab, 1024);
 	}
-	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+	addattr_nest_end(n, tail);
 	return 0;
 }
 
diff --git a/tc/tc_qdisc.c b/tc/tc_qdisc.c
index 8cc4b73..2fcf04c 100644
--- a/tc/tc_qdisc.c
+++ b/tc/tc_qdisc.c
@@ -182,14 +182,13 @@ static int tc_qdisc_modify(int cmd, unsigned int flags, int argc, char **argv)
 			return -1;
 		}
 
-		tail = NLMSG_TAIL(&req.n);
-		addattr_l(&req.n, sizeof(req), TCA_STAB, NULL, 0);
+		tail = addattr_nest(&req.n, sizeof(req), TCA_STAB);
 		addattr_l(&req.n, sizeof(req), TCA_STAB_BASE, &stab.szopts,
 			  sizeof(stab.szopts));
 		if (stab.data)
 			addattr_l(&req.n, sizeof(req), TCA_STAB_DATA, stab.data,
 				  stab.szopts.tsize * sizeof(__u16));
-		tail->rta_len = (void *)NLMSG_TAIL(&req.n) - (void *)tail;
+		addattr_nest_end(&req.n, tail);
 		if (stab.data)
 			free(stab.data);
 	}
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH iproute2-next 0/3] ip: Minor code cleanups and improvements
  2018-01-31  8:15 [PATCH iproute2-next 0/3] ip: Minor code cleanups and improvements Serhey Popovych
                   ` (2 preceding siblings ...)
  2018-01-31  8:15 ` [PATCH iproute2-next 3/3] treewide: Use addattr_nest()/addattr_nest_end() to handle nested attributes Serhey Popovych
@ 2018-02-02 23:06 ` David Ahern
  3 siblings, 0 replies; 7+ messages in thread
From: David Ahern @ 2018-02-02 23:06 UTC (permalink / raw)
  To: Serhey Popovych, netdev

On 1/31/18 1:15 AM, Serhey Popovych wrote:
> With this series I want to do some adjustments in header files for ip(8)
> as well as some trivial cleanups like variable rename and finally use
> addattr_nest()/addattr_nest_end() family instead of open coding nested
> attributes handling.
> 
> See individual patch description for more information on changes
> presented.
> 
> Reviews, comments and suggestions are welcome.
> 
> Thanks,
> Serhii
> 
> Serhey Popovych (3):
>   ip: Consolidate ip, xdp and lwtunnel parse/dump prototypes in
>     ip_common.h
>   ip: Minor cleanups
>   treewide: Use addattr_nest()/addattr_nest_end() to handle nested
>     attributes

series applied to iproute2-next

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH iproute2-next 3/3] treewide: Use addattr_nest()/addattr_nest_end() to handle nested attributes
  2018-01-31  8:15 ` [PATCH iproute2-next 3/3] treewide: Use addattr_nest()/addattr_nest_end() to handle nested attributes Serhey Popovych
@ 2018-04-13 22:57   ` Vinicius Costa Gomes
  2018-04-14 16:25     ` Stephen Hemminger
  0 siblings, 1 reply; 7+ messages in thread
From: Vinicius Costa Gomes @ 2018-04-13 22:57 UTC (permalink / raw)
  To: Serhey Popovych, netdev

Hi,

Serhey Popovych <serhe.popovych@gmail.com> writes:

[...]

> diff --git a/tc/q_mqprio.c b/tc/q_mqprio.c
> index 89b4600..207d644 100644
> --- a/tc/q_mqprio.c
> +++ b/tc/q_mqprio.c
> @@ -173,8 +173,7 @@ static int mqprio_parse_opt(struct qdisc_util *qu, int argc,
>  		argc--; argv++;
>  	}
>  
> -	tail = NLMSG_TAIL(n);
> -	addattr_l(n, 1024, TCA_OPTIONS, &opt, sizeof(opt));
> +	tail = addattr_nest_compat(n, 1024, TCA_OPTIONS, &opt, sizeof(opt));
>  
>  	if (flags & TC_MQPRIO_F_MODE)
>  		addattr_l(n, 1024, TCA_MQPRIO_MODE,
> @@ -209,7 +208,7 @@ static int mqprio_parse_opt(struct qdisc_util *qu, int argc,
>  		addattr_nest_end(n, start);
>  	}
>  
> -	tail->rta_len = (void *)NLMSG_TAIL(n) - (void *)tail;
> +	addattr_nest_compat_end(n, tail);
>  
>  	return 0;
>  }

Sorry if I am too late, but this breaks mqprio, i.e. something like
this:

$ tc qdisc replace dev enp2s0 handle 100: parent root mqprio \
                   num_tc 3 map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
                   queues 1@0 1@1 2@2 hw 0

that used to work, now doesn't.

This patch looks right, so I thought that it could be possible that mqprio
(in the kernel side) was making some wrong assumptions about the format
of the messages.

And after some investigation, what seems to be happening is something
like this (not too familiar with netlink protocol internals, I may be
missing something).

In the "wire", after this patch, the mqprio part of message may be
represented as:

/* The message format is [ len | type | payload ] */

[ S | 2 | <S bytes> ]
[ 0 | 2 | ]

Some notes:
 - S is the aligned value of sizeof(opt);
 - The value of TCA_OPTIONS is 2;

Before this patch, I think it was something like:

[ S | 2 | <S bytes> ]

The problem is that mqprio defines an internal type with the same value
as TCA_OPTIONS (2), and that finalizing (empty) is interpreted as the
"internal" field instead of indicating the end of TCA_OPTIONS, which
causes a size mismatch with 'mqprio_policy', causing the command to
create a mqprio qdisc to fail.

In short, I think that replacing the "open coded" version with
addattr_nest_compat() is not a functionally equivalent change.


Cheers,

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH iproute2-next 3/3] treewide: Use addattr_nest()/addattr_nest_end() to handle nested attributes
  2018-04-13 22:57   ` Vinicius Costa Gomes
@ 2018-04-14 16:25     ` Stephen Hemminger
  0 siblings, 0 replies; 7+ messages in thread
From: Stephen Hemminger @ 2018-04-14 16:25 UTC (permalink / raw)
  To: Vinicius Costa Gomes; +Cc: Serhey Popovych, netdev

On Fri, 13 Apr 2018 15:57:37 -0700
Vinicius Costa Gomes <vinicius.gomes@intel.com> wrote:

> Hi,
> 
> Serhey Popovych <serhe.popovych@gmail.com> writes:
> 
> [...]
> 
> > diff --git a/tc/q_mqprio.c b/tc/q_mqprio.c
> > index 89b4600..207d644 100644
> > --- a/tc/q_mqprio.c
> > +++ b/tc/q_mqprio.c
> > @@ -173,8 +173,7 @@ static int mqprio_parse_opt(struct qdisc_util *qu, int argc,
> >  		argc--; argv++;
> >  	}
> >  
> > -	tail = NLMSG_TAIL(n);
> > -	addattr_l(n, 1024, TCA_OPTIONS, &opt, sizeof(opt));
> > +	tail = addattr_nest_compat(n, 1024, TCA_OPTIONS, &opt, sizeof(opt));
> >  
> >  	if (flags & TC_MQPRIO_F_MODE)
> >  		addattr_l(n, 1024, TCA_MQPRIO_MODE,
> > @@ -209,7 +208,7 @@ static int mqprio_parse_opt(struct qdisc_util *qu, int argc,
> >  		addattr_nest_end(n, start);
> >  	}
> >  
> > -	tail->rta_len = (void *)NLMSG_TAIL(n) - (void *)tail;
> > +	addattr_nest_compat_end(n, tail);
> >  
> >  	return 0;
> >  }  
> 
> Sorry if I am too late, but this breaks mqprio, i.e. something like
> this:
> 
> $ tc qdisc replace dev enp2s0 handle 100: parent root mqprio \
>                    num_tc 3 map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
>                    queues 1@0 1@1 2@2 hw 0
> 
> that used to work, now doesn't.
> 
> This patch looks right, so I thought that it could be possible that mqprio
> (in the kernel side) was making some wrong assumptions about the format
> of the messages.
> 
> And after some investigation, what seems to be happening is something
> like this (not too familiar with netlink protocol internals, I may be
> missing something).
> 
> In the "wire", after this patch, the mqprio part of message may be
> represented as:
> 
> /* The message format is [ len | type | payload ] */
> 
> [ S | 2 | <S bytes> ]
> [ 0 | 2 | ]
> 
> Some notes:
>  - S is the aligned value of sizeof(opt);
>  - The value of TCA_OPTIONS is 2;
> 
> Before this patch, I think it was something like:
> 
> [ S | 2 | <S bytes> ]
> 
> The problem is that mqprio defines an internal type with the same value
> as TCA_OPTIONS (2), and that finalizing (empty) is interpreted as the
> "internal" field instead of indicating the end of TCA_OPTIONS, which
> causes a size mismatch with 'mqprio_policy', causing the command to
> create a mqprio qdisc to fail.
> 
> In short, I think that replacing the "open coded" version with
> addattr_nest_compat() is not a functionally equivalent change.
> 
> 
> Cheers,
> --
> Vinicius

There are also a couple of legacy cases where kernel expects or sends
nested netlink messages without the NLA_NESTED flag. I ran into this several
years ago, forgot where.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-04-14 16:25 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-31  8:15 [PATCH iproute2-next 0/3] ip: Minor code cleanups and improvements Serhey Popovych
2018-01-31  8:15 ` [PATCH iproute2-next 1/3] ip: Consolidate ip, xdp and lwtunnel parse/dump prototypes in ip_common.h Serhey Popovych
2018-01-31  8:15 ` [PATCH iproute2-next 2/3] ip: Minor cleanups Serhey Popovych
2018-01-31  8:15 ` [PATCH iproute2-next 3/3] treewide: Use addattr_nest()/addattr_nest_end() to handle nested attributes Serhey Popovych
2018-04-13 22:57   ` Vinicius Costa Gomes
2018-04-14 16:25     ` Stephen Hemminger
2018-02-02 23:06 ` [PATCH iproute2-next 0/3] ip: Minor code cleanups and improvements David Ahern

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).