Re: [PATCH v2 iproute2-next 1/3] ip: Use the `struct fib_rule_hdr` for rules

[David Ahern]

On 2/21/18 7:12 PM, Donald Sharp wrote:
> @@ -577,21 +585,20 @@ static int iprule_modify(int cmd, int argc, char **argv)
>   __u32 tid = 0;
>   struct {
>   struct nlmsghdr n;
> - struct rtmsgr;
> + struct fib_rule_hdr frh;
>   charbuf[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;
>   }

The action should be FR_ACT_TO_TBL; RTN_UNICAST == 1 == FR_ACT_TO_TBL;
the latter is the proper enum for fib rules.

[PATCH v2 iproute2-next 1/3] ip: Use the `struct fib_rule_hdr` for rules

[Donald Sharp]

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 
---
 include/uapi/linux/fib_rules.h |   2 +-
 ip/iprule.c| 129 ++---
 2 files changed, 69 insertions(+), 62 deletions(-)

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 {
__u8tos;
 
__u8table;
+   __u8proto;
__u8res1;   /* reserved */
-   __u8res2;   /* reserved */
__u8action;
 
__u32   flags;
diff --git a/ip/iprule.c b/ip/iprule.c
index a3abf2f6..00a6c26a 100644
--- a/ip/iprule.c
+++ b/ip/iprule.c
@@ -73,25 +73,33 @@ static struct
inet_prefix dst;
 } filter;
 
+static inline int frh_get_table(struct fib_rule_hdr *frh, struct rtattr **tb)
+{
+   __u32 table = frh->table;
+   if (tb[RTA_TABLE])
+   table = rta_getattr_u32(tb[RTA_TABLE]);
+   return table;
+}
+
 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 = 
 
-   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 +109,15 @@ static bool filter_nlmsg(struct nlmsghdr *n, struct 
rtattr **tb, int host_len)
if (filter.dst.family) {
inet_prefix *f_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 +167,7 @@ static bool filter_nlmsg(struct nlmsghdr *n, struct rtattr 
**tb, int host_len)
return false;
}
 
-   table = rtm_get_table(r, tb);
+   table = frh_get_table(frh, tb);
if (filter.tb > 0 && filter.tb ^ table)
return false;
 
@@ -169,7 +177,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 +188,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 +208,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,