Re: [PATCH iproute2-next] ip: Add support for nexthop objects
On 9/1/18 2:37 PM, Stephen Hemminger wrote:
>> diff --git a/include/uapi/linux/nexthop.h b/include/uapi/linux/nexthop.h
>> new file mode 100644
>> index ..335182e8229a
>> --- /dev/null
>> +++ b/include/uapi/linux/nexthop.h
>> @@ -0,0 +1,56 @@
>> +#ifndef __LINUX_NEXTHOP_H
>> +#define __LINUX_NEXTHOP_H
>> +
>> +#include
>> +
>> +struct nhmsg {
>> +unsigned char nh_family;
>> +unsigned char nh_scope; /* one of RT_SCOPE */
>> +unsigned char nh_protocol; /* Routing protocol that installed nh */
>> +unsigned char resvd;
>> +unsigned intnh_flags; /* RTNH_F flags */
>> +};
>
> Why not use __u8 and __u32 for these?
I want consistency with rtmsg on which nhmsg is based and has many
parallels.
>
>> +struct nexthop_grp {
>> +__u32 id;
>> +__u32 weight;
>> +};
>> +
>> +enum {
>> +NEXTHOP_GRP_TYPE_MPATH, /* default type if not specified */
>> +__NEXTHOP_GRP_TYPE_MAX,
>> +};
>> +
>> +#define NEXTHOP_GRP_TYPE_MAX (__NEXTHOP_GRP_TYPE_MAX - 1)
>> +
>> +
>> +/* NHA_ID 32-bit id for nexthop. id must be greater than 0.
>> + * id == 0 means assign an unused id.
>> + */
>
> Don't use dave's preferred comment style in this file.
> The reset of the file uses standard comments.
The file will eventually come from the kernel via header sync, so I have
to stick to whatever style is appropriate for the uapi files.
>> diff --git a/ip/ipnexthop.c b/ip/ipnexthop.c
>> new file mode 100644
>> index ..9fa4b7292426
>> --- /dev/null
>> +++ b/ip/ipnexthop.c
>> @@ -0,0 +1,652 @@
>> +/*
>> + * ip nexthop
>> + *
>> + * Copyright (C) 2017 Cumulus Networks
>> + * Copyright (c) 2017 David Ahern
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>>
>
> Please use SPDX and not GPL boilerplate in new files.
yes, the file pre-dates SPDX. Need to do the same with the kernel side
files.
>
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>
> Is this code really using libmnl?
no. need to fix. The iproute2 patch was only added for the RFC so people
could try out the UAPI which is the point of the RFC.
>> +if (!num || num * sizeof(*nhg) != RTA_PAYLOAD(grps_attr)) {
>> +fprintf(fp, "");
>> +return;
>> +}
>> +
>> +if (gtype)
>> +group_type = rta_getattr_u16(gtype);
>> +
>> +if (is_json_context()) {
>> +open_json_array(PRINT_JSON, "group");
>> +for (i = 0; i < num; ++i) {
>> +open_json_object(NULL);
>> +print_uint(PRINT_ANY, "id", "id %u ", nhg[i].id);
>> +print_uint(PRINT_ANY, "weight", "weight %u ",
>> nhg[i].weight);
>> +close_json_object();
>> +}
>> +close_json_array(PRINT_JSON, NULL);
>> +print_string(PRINT_ANY, "type", "type %s ",
>> + nh_group_type_to_str(group_type, b1, sizeof(b1)));
>> +} else {
>> +fprintf(fp, "group ");
>> +for (i = 0; i < num; ++i) {
>> +if (i)
>> +fprintf(fp, "/");
>> +fprintf(fp, "%u", nhg[i].id);
>> +if (num > 1 && nhg[i].weight > 1)
>> +fprintf(fp, ",%u", nhg[i].weight);
>> +}
>> +}
>> +}
>
> I think this could be done by using json_print cleverly rather than having
> to use is_json_contex(). That would avoid repeating code.
>
> You are only decoding group type in the json version, why not both?
oversight. group type was a recent change.
Re: [PATCH iproute2-next] ip: Add support for nexthop objects
On Fri, 31 Aug 2018 17:49:54 -0700
dsah...@kernel.org wrote:
> From: David Ahern
>
> Signed-off-by: David Ahern
> ---
> include/uapi/linux/nexthop.h | 56
> include/uapi/linux/rtnetlink.h | 8 +
> ip/Makefile| 3 +-
> ip/ip.c| 3 +-
> ip/ip_common.h | 7 +-
> ip/ipmonitor.c | 6 +
> ip/ipnexthop.c | 652
> +
> ip/iproute.c | 19 +-
> 8 files changed, 747 insertions(+), 7 deletions(-)
> create mode 100644 include/uapi/linux/nexthop.h
> create mode 100644 ip/ipnexthop.c
>
> diff --git a/include/uapi/linux/nexthop.h b/include/uapi/linux/nexthop.h
> new file mode 100644
> index ..335182e8229a
> --- /dev/null
> +++ b/include/uapi/linux/nexthop.h
> @@ -0,0 +1,56 @@
> +#ifndef __LINUX_NEXTHOP_H
> +#define __LINUX_NEXTHOP_H
> +
> +#include
> +
> +struct nhmsg {
> + unsigned char nh_family;
> + unsigned char nh_scope; /* one of RT_SCOPE */
> + unsigned char nh_protocol; /* Routing protocol that installed nh */
> + unsigned char resvd;
> + unsigned intnh_flags; /* RTNH_F flags */
> +};
Why not use __u8 and __u32 for these?
> +struct nexthop_grp {
> + __u32 id;
> + __u32 weight;
> +};
> +
> +enum {
> + NEXTHOP_GRP_TYPE_MPATH, /* default type if not specified */
> + __NEXTHOP_GRP_TYPE_MAX,
> +};
> +
> +#define NEXTHOP_GRP_TYPE_MAX (__NEXTHOP_GRP_TYPE_MAX - 1)
> +
> +
> +/* NHA_ID32-bit id for nexthop. id must be greater than 0.
> + * id == 0 means assign an unused id.
> + */
Don't use dave's preferred comment style in this file.
The reset of the file uses standard comments.
...
> diff --git a/ip/ip_common.h b/ip/ip_common.h
> index 200be5e23dd1..2971c1586c4e 100644
> --- a/ip/ip_common.h
> +++ b/ip/ip_common.h
> @@ -56,6 +56,8 @@ int print_rule(const struct sockaddr_nl *who,
> int print_netconf(const struct sockaddr_nl *who,
> struct rtnl_ctrl_data *ctrl,
> struct nlmsghdr *n, void *arg);
> +int print_nexthop(const struct sockaddr_nl *who,
> + struct nlmsghdr *n, void *arg);
> void netns_map_init(void);
> void netns_nsid_socket_init(void);
> int print_nsid(const struct sockaddr_nl *who,
> @@ -90,6 +92,7 @@ int do_ipvrf(int argc, char **argv);
> void vrf_reset(void);
> int netns_identify_pid(const char *pidstr, char *name, int len);
> int do_seg6(int argc, char **argv);
> +int do_ipnh(int argc, char **argv);
>
> int iplink_get(unsigned int flags, char *name, __u32 filt_mask);
> int iplink_ifla_xstats(int argc, char **argv);
> @@ -165,5 +168,7 @@ int name_is_vrf(const char *name);
> #endif
>
> void print_num(FILE *fp, unsigned int width, uint64_t count);
> -
> +void print_rta_flow(FILE *fp, const struct rtattr *rta);
> +void print_rt_flags(FILE *fp, unsigned int flags);
> +void print_rta_if(FILE *fp, const struct rtattr *rta, const char *prefix);
> #endif /* _IP_COMMON_H_ */
> diff --git a/ip/ipmonitor.c b/ip/ipmonitor.c
> index a93b62cd6624..de129626683b 100644
> --- a/ip/ipmonitor.c
> +++ b/ip/ipmonitor.c
> @@ -84,6 +84,12 @@ static int accept_msg(const struct sockaddr_nl *who,
> }
> }
>
> + case RTM_NEWNEXTHOP:
> + case RTM_DELNEXTHOP:
> + print_headers(fp, "[NEXTHOP]", ctrl);
> + print_nexthop(who, n, arg);
> + return 0;
> +
> case RTM_NEWLINK:
> case RTM_DELLINK:
> ll_remember_index(who, n, NULL);
> diff --git a/ip/ipnexthop.c b/ip/ipnexthop.c
> new file mode 100644
> index ..9fa4b7292426
> --- /dev/null
> +++ b/ip/ipnexthop.c
> @@ -0,0 +1,652 @@
> +/*
> + * ip nexthop
> + *
> + * Copyright (C) 2017 Cumulus Networks
> + * Copyright (c) 2017 David Ahern
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
>
Please use SPDX and not GPL boilerplate in new files.
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
Is this code really using libmnl?
> +#include
> +
> +#include "utils.h"
> +#include "ip_common.h"
> +
> +static struct
> +{
> + unsigned int flushed;
> + unsigned int groups;
> + char *flushb;
> + int flushp;
> + int flushe;
> +} filter;
> +
> +enum {
> + IPNH_LIST,
> + IPNH_FLUSH,
> +};
> +
> +#define RTM_NHA(h) ((struct rtattr *)(((char *)(h)) + \
> + NLMSG_ALIGN(sizeof(
[PATCH iproute2-next] ip: Add support for nexthop objects
From: David Ahern
Signed-off-by: David Ahern
---
include/uapi/linux/nexthop.h | 56
include/uapi/linux/rtnetlink.h | 8 +
ip/Makefile| 3 +-
ip/ip.c| 3 +-
ip/ip_common.h | 7 +-
ip/ipmonitor.c | 6 +
ip/ipnexthop.c | 652 +
ip/iproute.c | 19 +-
8 files changed, 747 insertions(+), 7 deletions(-)
create mode 100644 include/uapi/linux/nexthop.h
create mode 100644 ip/ipnexthop.c
diff --git a/include/uapi/linux/nexthop.h b/include/uapi/linux/nexthop.h
new file mode 100644
index ..335182e8229a
--- /dev/null
+++ b/include/uapi/linux/nexthop.h
@@ -0,0 +1,56 @@
+#ifndef __LINUX_NEXTHOP_H
+#define __LINUX_NEXTHOP_H
+
+#include
+
+struct nhmsg {
+ unsigned char nh_family;
+ unsigned char nh_scope; /* one of RT_SCOPE */
+ unsigned char nh_protocol; /* Routing protocol that installed nh */
+ unsigned char resvd;
+ unsigned intnh_flags; /* RTNH_F flags */
+};
+
+struct nexthop_grp {
+ __u32 id;
+ __u32 weight;
+};
+
+enum {
+ NEXTHOP_GRP_TYPE_MPATH, /* default type if not specified */
+ __NEXTHOP_GRP_TYPE_MAX,
+};
+
+#define NEXTHOP_GRP_TYPE_MAX (__NEXTHOP_GRP_TYPE_MAX - 1)
+
+
+/* NHA_ID 32-bit id for nexthop. id must be greater than 0.
+ * id == 0 means assign an unused id.
+ */
+enum {
+ NHA_UNSPEC,
+ NHA_ID, /* u32 */
+ NHA_GROUP, /* array of nexthop_grp */
+ NHA_GROUP_TYPE, /* u16 one of NEXTHOP_GRP_TYPE;
+* default is NEXTHOP_GRP_TYPE_MPATH */
+
+ /* if NHA_GROUP attribute is added, no other attributes can be set */
+
+ NHA_BLACKHOLE, /* flag; nexthop used to blackhole packets */
+ NHA_OIF,/* u32 */
+ NHA_FLOW, /* u32 */
+
+ NHA_TABLE_ID, /* u32 - table id to validate gateway */
+ NHA_GATEWAY,/* be32 (IPv4) or in6_addr (IPv6) gw address */
+
+ /* Dump control attributes */
+ NHA_GROUPS, /* flag; only return nexthop groups in dump */
+ NHA_MASTER, /* u32; only return nexthops with given master dev */
+
+ NHA_SADDR, /* return only: IPv4 or IPv6 source address */
+
+ __NHA_MAX,
+};
+
+#define NHA_MAX(__NHA_MAX - 1)
+#endif
diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 8c1d600bfa33..158114245b6c 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -157,6 +157,13 @@ enum {
RTM_GETCHAIN,
#define RTM_GETCHAIN RTM_GETCHAIN
+ RTM_NEWNEXTHOP = 104,
+#define RTM_NEWNEXTHOP RTM_NEWNEXTHOP
+ RTM_DELNEXTHOP,
+#define RTM_DELNEXTHOP RTM_DELNEXTHOP
+ RTM_GETNEXTHOP,
+#define RTM_GETNEXTHOP RTM_GETNEXTHOP
+
__RTM_MAX,
#define RTM_MAX(((__RTM_MAX + 3) & ~3) - 1)
};
@@ -342,6 +349,7 @@ enum rtattr_type_t {
RTA_IP_PROTO,
RTA_SPORT,
RTA_DPORT,
+ RTA_NH_ID,
__RTA_MAX
};
diff --git a/ip/Makefile b/ip/Makefile
index a88f93665ee6..7df818dbe23a 100644
--- a/ip/Makefile
+++ b/ip/Makefile
@@ -10,7 +10,8 @@ IPOBJ=ip.o ipaddress.o ipaddrlabel.o iproute.o iprule.o
ipnetns.o \
link_iptnl.o link_gre6.o iplink_bond.o iplink_bond_slave.o iplink_hsr.o \
iplink_bridge.o iplink_bridge_slave.o ipfou.o iplink_ipvlan.o \
iplink_geneve.o iplink_vrf.o iproute_lwtunnel.o ipmacsec.o ipila.o \
-ipvrf.o iplink_xstats.o ipseg6.o iplink_netdevsim.o iplink_rmnet.o
+ipvrf.o iplink_xstats.o ipseg6.o iplink_netdevsim.o iplink_rmnet.o \
+ipnexthop.o
RTMONOBJ=rtmon.o
diff --git a/ip/ip.c b/ip/ip.c
index 58c643df8a36..963ef140c7c4 100644
--- a/ip/ip.c
+++ b/ip/ip.c
@@ -51,7 +51,7 @@ static void usage(void)
"where OBJECT := { link | address | addrlabel | route | rule | neigh | ntable
|\n"
" tunnel | tuntap | maddress | mroute | mrule | monitor |
xfrm |\n"
" netns | l2tp | fou | macsec | tcp_metrics | token |
netconf | ila |\n"
-" vrf | sr }\n"
+" vrf | sr | nexthop }\n"
" OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] | -r[esolve] |\n"
"-h[uman-readable] | -iec | -j[son] | -p[retty] |\n"
"-f[amily] { inet | inet6 | ipx | dnet | mpls | bridge |
link } |\n"
@@ -101,6 +101,7 @@ static const struct cmd {
{ "netconf",do_ipnetconf },
{ "vrf",do_ipvrf},
{ "sr", do_seg6 },
+ { "nexthop",do_ipnh },
{ "help", do_help },
{ 0 }
};
diff --git a/ip/ip_common.h b/ip/ip_common.h
index 200be5e23dd1..2971c1586c4e 100644
--- a/ip/ip_common.h
+++ b/ip/ip_common.h
@@ -56,6 +56,8 @@ int print_rule(const struct sockaddr_nl *who,
int print_netconf(const struct sockaddr_nl *who,
struct rtnl_ctrl_data *ctrl,
s
