diff mbox series

[iproute2-next,1/4] ip: Use the `struct fib_rule_hdr` for rules

Message ID 20180221015348.32646-2-sharpd@cumulusnetworks.com
State Changes Requested, archived
Delegated to: David Ahern
Headers show
Series Allow 'ip rule' command to use protocol | expand

Commit Message

Donald Sharp Feb. 21, 2018, 1:53 a.m. UTC 
The iprule.c code was using `struct rtmsg` as the data
type to pass into the kernel for the netlink message.
While 'struct rtmsg' and `struct fib_rule_hdr` are
the same size and mostly the same, we should use
the correct data structure.  This commit translates
the data structures to have iprule.c use the correct
one.

Additionally copy over the modified fib_rules.h file

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
---
 include/uapi/linux/fib_rules.h |   2 +-
 ip/iprule.c                    | 132 ++++++++++++++++++++++-------------------
 2 files changed, 72 insertions(+), 62 deletions(-)

Comments

David Ahern Feb. 21, 2018, 5:04 p.m. UTC | #1 
On 2/20/18 6:53 PM, Donald Sharp wrote:
> @@ -159,7 +159,13 @@ static bool filter_nlmsg(struct nlmsghdr *n, struct rtattr **tb, int host_len)
>  			return false;
>  	}
>  
> -	table = rtm_get_table(r, tb);
> +
> +	/* struct fib_rule_hdr and struct rtmsg
> +	 * were intentionally the same.  Since
> +	 * the table is the rtm_table, just call
> +	 * it.
> +	 */
> +	table = rtm_get_table((struct rtmsg *)frh, tb);
>  	if (filter.tb > 0 && filter.tb ^ table)
>  		return false;
>  
...

> @@ -273,7 +279,12 @@ int print_rule(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
>  		fprintf(fp, "uidrange %u-%u ", r->start, r->end);
>  	}
>  
> -	table = rtm_get_table(r, tb);
> +	/* struct fib_rule_hdr and struct rtmsg
> +	 * were intentionally the same.  Since
> +	 * the table is the rtm_table, just call
> +	 * it.
> +	 */
> +	table = rtm_get_table((struct rtmsg *)frh, tb);
>  	if (table) {
>  		fprintf(fp, "lookup %s ",
>  			rtnl_rttable_n2a(table, b1, sizeof(b1)));

I would rather have a second get_table function than use the typecast;
frh_get_table() that mirrors rtm_get_table.

Thanks for taking the time to fix the ancillary header struct.
diff mbox series

Patch

diff --git a/include/uapi/linux/fib_rules.h b/include/uapi/linux/fib_rules.h
index 2b642bf9..92553917 100644
--- a/include/uapi/linux/fib_rules.h
+++ b/include/uapi/linux/fib_rules.h
@@ -23,8 +23,8 @@  struct fib_rule_hdr {
 	__u8		tos;
 
 	__u8		table;
+	__u8		proto;
 	__u8		res1;	/* reserved */
-	__u8		res2;	/* reserved */
 	__u8		action;
 
 	__u32		flags;
diff --git a/ip/iprule.c b/ip/iprule.c
index a3abf2f6..c40d76f1 100644
--- a/ip/iprule.c
+++ b/ip/iprule.c
@@ -75,23 +75,23 @@  static struct
 
 static bool filter_nlmsg(struct nlmsghdr *n, struct rtattr **tb, int host_len)
 {
-	struct rtmsg *r = NLMSG_DATA(n);
+	struct fib_rule_hdr *frh = NLMSG_DATA(n);
 	__u32 table;
 
-	if (preferred_family != AF_UNSPEC && r->rtm_family != preferred_family)
+	if (preferred_family != AF_UNSPEC && frh->family != preferred_family)
 		return false;
 
 	if (filter.prefmask &&
 	    filter.pref ^ (tb[FRA_PRIORITY] ? rta_getattr_u32(tb[FRA_PRIORITY]) : 0))
 		return false;
-	if (filter.not && !(r->rtm_flags & FIB_RULE_INVERT))
+	if (filter.not && !(frh->flags & FIB_RULE_INVERT))
 		return false;
 
 	if (filter.src.family) {
 		inet_prefix *f_src = &filter.src;
 
-		if (f_src->family != r->rtm_family ||
-		    f_src->bitlen > r->rtm_src_len)
+		if (f_src->family != frh->family ||
+		    f_src->bitlen > frh->src_len)
 			return false;
 
 		if (inet_addr_match_rta(f_src, tb[FRA_SRC]))
@@ -101,15 +101,15 @@  static bool filter_nlmsg(struct nlmsghdr *n, struct rtattr **tb, int host_len)
 	if (filter.dst.family) {
 		inet_prefix *f_dst = &filter.dst;
 
-		if (f_dst->family != r->rtm_family ||
-		    f_dst->bitlen > r->rtm_dst_len)
+		if (f_dst->family != frh->family ||
+		    f_dst->bitlen > frh->dst_len)
 			return false;
 
 		if (inet_addr_match_rta(f_dst, tb[FRA_DST]))
 			return false;
 	}
 
-	if (filter.tosmask && filter.tos ^ r->rtm_tos)
+	if (filter.tosmask && filter.tos ^ frh->tos)
 		return false;
 
 	if (filter.fwmark) {
@@ -159,7 +159,13 @@  static bool filter_nlmsg(struct nlmsghdr *n, struct rtattr **tb, int host_len)
 			return false;
 	}
 
-	table = rtm_get_table(r, tb);
+
+	/* struct fib_rule_hdr and struct rtmsg
+	 * were intentionally the same.  Since
+	 * the table is the rtm_table, just call
+	 * it.
+	 */
+	table = rtm_get_table((struct rtmsg *)frh, tb);
 	if (filter.tb > 0 && filter.tb ^ table)
 		return false;
 
@@ -169,7 +175,7 @@  static bool filter_nlmsg(struct nlmsghdr *n, struct rtattr **tb, int host_len)
 int print_rule(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 {
 	FILE *fp = (FILE *)arg;
-	struct rtmsg *r = NLMSG_DATA(n);
+	struct fib_rule_hdr *frh = NLMSG_DATA(n);
 	int len = n->nlmsg_len;
 	int host_len = -1;
 	__u32 table;
@@ -180,13 +186,13 @@  int print_rule(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 	if (n->nlmsg_type != RTM_NEWRULE && n->nlmsg_type != RTM_DELRULE)
 		return 0;
 
-	len -= NLMSG_LENGTH(sizeof(*r));
+	len -= NLMSG_LENGTH(sizeof(*frh));
 	if (len < 0)
 		return -1;
 
-	parse_rtattr(tb, FRA_MAX, RTM_RTA(r), len);
+	parse_rtattr(tb, FRA_MAX, RTM_RTA(frh), len);
 
-	host_len = af_bit_len(r->rtm_family);
+	host_len = af_bit_len(frh->family);
 
 	if (!filter_nlmsg(n, tb, host_len))
 		return 0;
@@ -200,41 +206,41 @@  int print_rule(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 	else
 		fprintf(fp, "0:\t");
 
-	if (r->rtm_flags & FIB_RULE_INVERT)
+	if (frh->flags & FIB_RULE_INVERT)
 		fprintf(fp, "not ");
 
 	if (tb[FRA_SRC]) {
-		if (r->rtm_src_len != host_len) {
+		if (frh->src_len != host_len) {
 			fprintf(fp, "from %s/%u ",
-				rt_addr_n2a_rta(r->rtm_family, tb[FRA_SRC]),
-				r->rtm_src_len);
+				rt_addr_n2a_rta(frh->family, tb[FRA_SRC]),
+				frh->src_len);
 		} else {
 			fprintf(fp, "from %s ",
-				format_host_rta(r->rtm_family, tb[FRA_SRC]));
+				format_host_rta(frh->family, tb[FRA_SRC]));
 		}
-	} else if (r->rtm_src_len) {
-		fprintf(fp, "from 0/%d ", r->rtm_src_len);
+	} else if (frh->src_len) {
+		fprintf(fp, "from 0/%d ", frh->src_len);
 	} else {
 		fprintf(fp, "from all ");
 	}
 
 	if (tb[FRA_DST]) {
-		if (r->rtm_dst_len != host_len) {
+		if (frh->dst_len != host_len) {
 			fprintf(fp, "to %s/%u ",
-				rt_addr_n2a_rta(r->rtm_family, tb[FRA_DST]),
-				r->rtm_dst_len);
+				rt_addr_n2a_rta(frh->family, tb[FRA_DST]),
+				frh->dst_len);
 		} else {
 			fprintf(fp, "to %s ",
-				format_host_rta(r->rtm_family, tb[FRA_DST]));
+				format_host_rta(frh->family, tb[FRA_DST]));
 		}
-	} else if (r->rtm_dst_len) {
-		fprintf(fp, "to 0/%d ", r->rtm_dst_len);
+	} else if (frh->dst_len) {
+		fprintf(fp, "to 0/%d ", frh->dst_len);
 	}
 
-	if (r->rtm_tos) {
+	if (frh->tos) {
 		SPRINT_BUF(b1);
 		fprintf(fp, "tos %s ",
-			rtnl_dsfield_n2a(r->rtm_tos, b1, sizeof(b1)));
+			rtnl_dsfield_n2a(frh->tos, b1, sizeof(b1)));
 	}
 
 	if (tb[FRA_FWMARK] || tb[FRA_FWMASK]) {
@@ -252,13 +258,13 @@  int print_rule(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 
 	if (tb[FRA_IFNAME]) {
 		fprintf(fp, "iif %s ", rta_getattr_str(tb[FRA_IFNAME]));
-		if (r->rtm_flags & FIB_RULE_IIF_DETACHED)
+		if (frh->flags & FIB_RULE_IIF_DETACHED)
 			fprintf(fp, "[detached] ");
 	}
 
 	if (tb[FRA_OIFNAME]) {
 		fprintf(fp, "oif %s ", rta_getattr_str(tb[FRA_OIFNAME]));
-		if (r->rtm_flags & FIB_RULE_OIF_DETACHED)
+		if (frh->flags & FIB_RULE_OIF_DETACHED)
 			fprintf(fp, "[detached] ");
 	}
 
@@ -273,7 +279,12 @@  int print_rule(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		fprintf(fp, "uidrange %u-%u ", r->start, r->end);
 	}
 
-	table = rtm_get_table(r, tb);
+	/* struct fib_rule_hdr and struct rtmsg
+	 * were intentionally the same.  Since
+	 * the table is the rtm_table, just call
+	 * it.
+	 */
+	table = rtm_get_table((struct rtmsg *)frh, tb);
 	if (table) {
 		fprintf(fp, "lookup %s ",
 			rtnl_rttable_n2a(table, b1, sizeof(b1)));
@@ -308,26 +319,26 @@  int print_rule(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 			rtnl_rtrealm_n2a(to, b1, sizeof(b1)));
 	}
 
-	if (r->rtm_type == RTN_NAT) {
+	if (frh->action == RTN_NAT) {
 		if (tb[RTA_GATEWAY]) {
 			fprintf(fp, "map-to %s ",
-				format_host_rta(r->rtm_family,
+				format_host_rta(frh->family,
 						tb[RTA_GATEWAY]));
 		} else
 			fprintf(fp, "masquerade");
-	} else if (r->rtm_type == FR_ACT_GOTO) {
+	} else if (frh->action == FR_ACT_GOTO) {
 		fprintf(fp, "goto ");
 		if (tb[FRA_GOTO])
 			fprintf(fp, "%u", rta_getattr_u32(tb[FRA_GOTO]));
 		else
 			fprintf(fp, "none");
-		if (r->rtm_flags & FIB_RULE_UNRESOLVED)
+		if (frh->flags & FIB_RULE_UNRESOLVED)
 			fprintf(fp, " [unresolved]");
-	} else if (r->rtm_type == FR_ACT_NOP)
+	} else if (frh->action == FR_ACT_NOP)
 		fprintf(fp, "nop");
-	else if (r->rtm_type != RTN_UNICAST)
+	else if (frh->action != RTN_UNICAST)
 		fprintf(fp, "%s",
-			rtnl_rtntype_n2a(r->rtm_type,
+			rtnl_rtntype_n2a(frh->action,
 					 b1, sizeof(b1)));
 
 	fprintf(fp, "\n");
@@ -373,15 +384,15 @@  static int flush_rule(const struct sockaddr_nl *who, struct nlmsghdr *n,
 		      void *arg)
 {
 	struct rtnl_handle rth2;
-	struct rtmsg *r = NLMSG_DATA(n);
+	struct fib_rule_hdr *frh = NLMSG_DATA(n);
 	int len = n->nlmsg_len;
 	struct rtattr *tb[FRA_MAX+1];
 
-	len -= NLMSG_LENGTH(sizeof(*r));
+	len -= NLMSG_LENGTH(sizeof(*frh));
 	if (len < 0)
 		return -1;
 
-	parse_rtattr(tb, FRA_MAX, RTM_RTA(r), len);
+	parse_rtattr(tb, FRA_MAX, RTM_RTA(frh), len);
 
 	if (tb[FRA_PRIORITY]) {
 		n->nlmsg_type = RTM_DELRULE;
@@ -577,21 +588,20 @@  static int iprule_modify(int cmd, int argc, char **argv)
 	__u32 tid = 0;
 	struct {
 		struct nlmsghdr	n;
-		struct rtmsg		r;
+		struct fib_rule_hdr	frh;
 		char			buf[1024];
 	} req = {
 		.n.nlmsg_type = cmd,
-		.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct rtmsg)),
+		.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct fib_rule_hdr)),
 		.n.nlmsg_flags = NLM_F_REQUEST,
-		.r.rtm_family = preferred_family,
-		.r.rtm_protocol = RTPROT_BOOT,
-		.r.rtm_scope = RT_SCOPE_UNIVERSE,
-		.r.rtm_type = RTN_UNSPEC,
+		.frh.family = preferred_family,
+		.frh.proto = RTPROT_BOOT,
+		.frh.action = RTN_UNSPEC,
 	};
 
 	if (cmd == RTM_NEWRULE) {
 		req.n.nlmsg_flags |= NLM_F_CREATE|NLM_F_EXCL;
-		req.r.rtm_type = RTN_UNICAST;
+		req.frh.action = RTN_UNICAST;
 	}
 
 	if (cmd == RTM_DELRULE && argc == 0) {
@@ -601,21 +611,21 @@  static int iprule_modify(int cmd, int argc, char **argv)
 
 	while (argc > 0) {
 		if (strcmp(*argv, "not") == 0) {
-			req.r.rtm_flags |= FIB_RULE_INVERT;
+			req.frh.flags |= FIB_RULE_INVERT;
 		} else if (strcmp(*argv, "from") == 0) {
 			inet_prefix dst;
 
 			NEXT_ARG();
-			get_prefix(&dst, *argv, req.r.rtm_family);
-			req.r.rtm_src_len = dst.bitlen;
+			get_prefix(&dst, *argv, req.frh.family);
+			req.frh.src_len = dst.bitlen;
 			addattr_l(&req.n, sizeof(req), FRA_SRC,
 				  &dst.data, dst.bytelen);
 		} else if (strcmp(*argv, "to") == 0) {
 			inet_prefix dst;
 
 			NEXT_ARG();
-			get_prefix(&dst, *argv, req.r.rtm_family);
-			req.r.rtm_dst_len = dst.bitlen;
+			get_prefix(&dst, *argv, req.frh.family);
+			req.frh.dst_len = dst.bitlen;
 			addattr_l(&req.n, sizeof(req), FRA_DST,
 				  &dst.data, dst.bytelen);
 		} else if (matches(*argv, "preference") == 0 ||
@@ -634,7 +644,7 @@  static int iprule_modify(int cmd, int argc, char **argv)
 			NEXT_ARG();
 			if (rtnl_dsfield_a2n(&tos, *argv))
 				invarg("TOS value is invalid\n", *argv);
-			req.r.rtm_tos = tos;
+			req.frh.tos = tos;
 		} else if (strcmp(*argv, "fwmark") == 0) {
 			char *slash;
 			__u32 fwmark, fwmask;
@@ -667,9 +677,9 @@  static int iprule_modify(int cmd, int argc, char **argv)
 			if (rtnl_rttable_a2n(&tid, *argv))
 				invarg("invalid table ID\n", *argv);
 			if (tid < 256)
-				req.r.rtm_table = tid;
+				req.frh.table = tid;
 			else {
-				req.r.rtm_table = RT_TABLE_UNSPEC;
+				req.frh.table = RT_TABLE_UNSPEC;
 				addattr32(&req.n, sizeof(req), FRA_TABLE, tid);
 			}
 			table_ok = 1;
@@ -724,7 +734,7 @@  static int iprule_modify(int cmd, int argc, char **argv)
 			fprintf(stderr, "Warning: route NAT is deprecated\n");
 			addattr32(&req.n, sizeof(req), RTA_GATEWAY,
 				  get_addr32(*argv));
-			req.r.rtm_type = RTN_NAT;
+			req.frh.action = RTN_NAT;
 		} else {
 			int type;
 
@@ -746,7 +756,7 @@  static int iprule_modify(int cmd, int argc, char **argv)
 				type = FR_ACT_NOP;
 			else if (rtnl_rtntype_a2n(&type, *argv))
 				invarg("Failed to parse rule type", *argv);
-			req.r.rtm_type = type;
+			req.frh.action = type;
 			table_ok = 1;
 		}
 		argc--;
@@ -759,11 +769,11 @@  static int iprule_modify(int cmd, int argc, char **argv)
 		return -EINVAL;
 	}
 
-	if (req.r.rtm_family == AF_UNSPEC)
-		req.r.rtm_family = AF_INET;
+	if (req.frh.family == AF_UNSPEC)
+		req.frh.family = AF_INET;
 
 	if (!table_ok && cmd == RTM_NEWRULE)
-		req.r.rtm_table = RT_TABLE_MAIN;
+		req.frh.table = RT_TABLE_MAIN;
 
 	if (rtnl_talk(&rth, &req.n, NULL) < 0)
 		return -2;