From: Nikolay Aleksandrov <razor@blackwall.org> Date: 2021-09-30 11:39:29
From: Nikolay Aleksandrov <redacted>
Hi,
This set tries to help with an old ask that we've had for some time
which is to print nexthop information while monitoring or dumping routes.
The core problem is that people cannot follow nexthop changes while
monitoring route changes, by the time they check the nexthop it could be
deleted or updated to something else. In order to help them out I've
added a nexthop cache which is populated (only used if -d / show_details
is specified) while decoding routes and kept up to date while monitoring.
The nexthop information is printed on its own line starting with the
"nh_info" attribute and its embedded inside it if printing JSON. To
cache the nexthop entries I parse them into structures, in order to
reuse most of the code the print helpers have been altered so they rely
on prepared structures. Nexthops are now always parsed into a structure,
even if they won't be cached, that structure is later used to print the
nexthop and destroyed if not going to be cached. New nexthops (not found
in the cache) are retrieved from the kernel using a private netlink
socket so they don't disrupt an ongoing dump, similar to how interfaces
are retrieved and cached.
I have tested the set with the kernel forwarding selftests and also by
stressing it with nexthop create/update/delete in loops while monitoring.
Comments are very welcome as usual. :)
Changes since RFC:
- reordered parse/print splits, in order to do that I have to parse
resilient groups first, then add nh entry parsing so code has been
reordered as well and patch order has changed, but there have been
no functional changes (as before refactoring of old code is done in
the first 8 patches and then patches 9-12 add the new cache and use it)
- re-run all tests above
Patch breakdown:
Patches 1-2: update current route helpers to take parsed arguments so we
can directly pass them from the nh_entry structure later
Patch 3: adds new nha_res_grp structure which describes a resilient
nexhtop group
Patch 4: splits print_nh_res_group into a parse and print parts
which use the new nha_res_grp structure
Patch 5: adds new nh_entry structure which describes a nexthop
Patch 6: factors out print_nexthop's attribute parsing into nh_entry
structure used before printing
Patch 7: factors out print_nexthop's nh_entry structure printing
Patch 8: factors out ipnh_get's rtnl talk part and allows to use a
different rt handle for the communication
Patch 9: adds nexthop cache and helpers to manage it, it uses the
new __ipnh_get to retrieve nexthops
Patch 10: adds a new helper print_cache_nexthop_id that prints nexthop
information from its id, if the nexthop is not found in the
cache it fetches it
Patch 11: the new print_cache_nexthop_id helper is used when printing
routes with show_details (-d) to output detailed nexthop
information, the format after nh_info is the same as
ip nexthop show
Patch 12: changes print_nexthop into print_cache_nexthop which always
outputs the nexthop information and can also update the cache
(based on process_cache argument), it's used to keep the
cache up to date while monitoring
Example outputs (monitor):
[NEXTHOP]id 101 via 169.254.2.22 dev veth2 scope link proto unspec
[NEXTHOP]id 102 via 169.254.3.23 dev veth4 scope link proto unspec
[NEXTHOP]id 103 group 101/102 type resilient buckets 512 idle_timer 0 unbalanced_timer 0 unbalanced_time 0 scope global proto unspec
[ROUTE]unicast 192.0.2.0/24 nhid 203 table 4 proto boot scope global
nh_info id 203 group 201/202 type resilient buckets 512 idle_timer 0 unbalanced_timer 0 unbalanced_time 0 scope global proto unspec
nexthop via 169.254.2.12 dev veth3 weight 1
nexthop via 169.254.3.13 dev veth5 weight 1
[NEXTHOP]id 204 via fe80:2::12 dev veth3 scope link proto unspec
[NEXTHOP]id 205 via fe80:3::13 dev veth5 scope link proto unspec
[NEXTHOP]id 206 group 204/205 type resilient buckets 512 idle_timer 0 unbalanced_timer 0 unbalanced_time 0 scope global proto unspec
[ROUTE]unicast 2001:db8:1::/64 nhid 206 table 4 proto boot scope global metric 1024 pref medium
nh_info id 206 group 204/205 type resilient buckets 512 idle_timer 0 unbalanced_timer 0 unbalanced_time 0 scope global proto unspec
nexthop via fe80:2::12 dev veth3 weight 1
nexthop via fe80:3::13 dev veth5 weight 1
[NEXTHOP]id 2 encap mpls 200/300 via 10.1.1.1 dev ens20 scope link proto unspec onlink
[ROUTE]unicast 2.3.4.10 nhid 2 table main proto boot scope global
nh_info id 2 encap mpls 200/300 via 10.1.1.1 dev ens20 scope link proto unspec onlink
JSON:
{
"type": "unicast",
"dst": "198.51.100.0/24",
"nhid": 103,
"table": "3",
"protocol": "boot",
"scope": "global",
"flags": [ ],
"nh_info": {
"id": 103,
"group": [ {
"id": 101,
"weight": 11
},{
"id": 102,
"weight": 45
} ],
"type": "resilient",
"resilient_args": {
"buckets": 512,
"idle_timer": 0,
"unbalanced_timer": 0,
"unbalanced_time": 0
},
"scope": "global",
"protocol": "unspec",
"flags": [ ]
},
"nexthops": [ {
"gateway": "169.254.2.22",
"dev": "veth2",
"weight": 11,
"flags": [ ]
},{
"gateway": "169.254.3.23",
"dev": "veth4",
"weight": 45,
"flags": [ ]
} ]
}
Thank you,
Nik
Nikolay Aleksandrov (12):
ip: print_rta_if takes ifindex as device argument instead of attribute
ip: export print_rta_gateway version which outputs prepared gateway
string
ip: nexthop: add resilient group structure
ip: nexthop: split print_nh_res_group into parse and print parts
ip: nexthop: add nh entry structure
ip: nexthop: parse attributes into nh entry structure before printing
ip: nexthop: factor out print_nexthop's nh entry printing
ip: nexthop: factor out ipnh_get_id rtnl talk into a helper
ip: nexthop: add cache helpers
ip: nexthop: add a helper which retrieves and prints cached nh entry
ip: route: print and cache detailed nexthop information when requested
ip: nexthop: add print_cache_nexthop which prints and manages the nh
cache
ip/ip_common.h | 4 +-
ip/ipmonitor.c | 3 +-
ip/ipnexthop.c | 459 +++++++++++++++++++++++++++++++++++++++----------
ip/iproute.c | 32 ++--
ip/nh_common.h | 53 ++++++
5 files changed, 448 insertions(+), 103 deletions(-)
create mode 100644 ip/nh_common.h
--
2.31.1
From: Nikolay Aleksandrov <razor@blackwall.org> Date: 2021-09-30 11:39:30
From: Nikolay Aleksandrov <redacted>
Add a structure which describes a resilient nexthop group. It will be
later used for parsing.
Signed-off-by: Nikolay Aleksandrov <redacted>
---
ip/nh_common.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
create mode 100644 ip/nh_common.h
From: Nikolay Aleksandrov <razor@blackwall.org> Date: 2021-09-30 11:39:31
From: Nikolay Aleksandrov <redacted>
We need print_rta_if() to take ifindex directly so later we can use it
with cached converted nexthop objects.
Signed-off-by: Nikolay Aleksandrov <redacted>
---
ip/ip_common.h | 2 +-
ip/ipnexthop.c | 2 +-
ip/iproute.c | 12 ++++++------
3 files changed, 8 insertions(+), 8 deletions(-)
From: Nikolay Aleksandrov <razor@blackwall.org> Date: 2021-09-30 11:39:36
From: Nikolay Aleksandrov <redacted>
Export a new __print_rta_gateway that takes a prepared gateway string to
print which is also used by print_rta_gateway for consistent format.
Signed-off-by: Nikolay Aleksandrov <redacted>
---
ip/ip_common.h | 1 +
ip/iproute.c | 15 ++++++++++-----
2 files changed, 11 insertions(+), 5 deletions(-)
From: Nikolay Aleksandrov <razor@blackwall.org> Date: 2021-09-30 11:39:37
From: Nikolay Aleksandrov <redacted>
Now that we have resilient group structure split print_nh_res_group into
a parse and print functions, print_nexthop calls the parse function
first to parse the attributes into the structure and then uses the print
function to print the parsed structure.
Signed-off-by: Nikolay Aleksandrov <redacted>
---
ip/ipnexthop.c | 48 ++++++++++++++++++++++++++++++++----------------
1 file changed, 32 insertions(+), 16 deletions(-)
From: Nikolay Aleksandrov <razor@blackwall.org> Date: 2021-09-30 11:39:39
From: Nikolay Aleksandrov <redacted>
Add a structure which describes a nexthop, it will be later used to
parse, print and cache nexthops.
Signed-off-by: Nikolay Aleksandrov <redacted>
---
ip/nh_common.h | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
From: Nikolay Aleksandrov <razor@blackwall.org> Date: 2021-09-30 11:39:40
From: Nikolay Aleksandrov <redacted>
Factor out the nexthop attribute parsing and parse attributes into a
nexthop entry structure which is then used to print.
Signed-off-by: Nikolay Aleksandrov <redacted>
---
ip/ipnexthop.c | 186 ++++++++++++++++++++++++++++++++++++-------------
1 file changed, 139 insertions(+), 47 deletions(-)
@@ -254,15 +255,13 @@ static const char *nh_group_type_name(__u16 type)}}-staticvoidprint_nh_group_type(FILE*fp,conststructrtattr*grp_type_attr)+staticvoidprint_nh_group_type(__u16nh_grp_type){-__u16type=rta_getattr_u16(grp_type_attr);--if(type==NEXTHOP_GRP_TYPE_MPATH)+if(nh_grp_type==NEXTHOP_GRP_TYPE_MPATH)/* Do not print type in order not to break existing output. */return;-print_string(PRINT_ANY,"type","type %s ",nh_group_type_name(type));+print_string(PRINT_ANY,"type","type %s ",nh_group_type_name(nh_grp_type));}staticvoidparse_nh_res_group_rta(conststructrtattr*res_grp_attr,
@@ -340,12 +339,104 @@ static void print_nh_res_bucket(FILE *fp, const struct rtattr *res_bucket_attr)close_json_object();}+staticvoidipnh_destroy_entry(structnh_entry*nhe)+{+if(nhe->nh_encap)+free(nhe->nh_encap);+if(nhe->nh_groups)+free(nhe->nh_groups);+}++/* parse nhmsg into nexthop entry struct which must be destroyed by+*ipnh_destroy_entywhenit'snotneededanymore+*/+staticintipnh_parse_nhmsg(FILE*fp,conststructnhmsg*nhm,intlen,+structnh_entry*nhe)+{+structrtattr*tb[NHA_MAX+1];+interr=0;++memset(nhe,0,sizeof(*nhe));+parse_rtattr_flags(tb,NHA_MAX,RTM_NHA(nhm),len,NLA_F_NESTED);++if(tb[NHA_ID])+nhe->nh_id=rta_getattr_u32(tb[NHA_ID]);++if(tb[NHA_OIF])+nhe->nh_oif=rta_getattr_u32(tb[NHA_OIF]);++if(tb[NHA_GROUP_TYPE])+nhe->nh_grp_type=rta_getattr_u16(tb[NHA_GROUP_TYPE]);++if(tb[NHA_GATEWAY]){+if(RTA_PAYLOAD(tb[NHA_GATEWAY])>sizeof(nhe->nh_gateway)){+fprintf(fp,"<nexthop id %u invalid gateway length %lu>\n",+nhe->nh_id,RTA_PAYLOAD(tb[NHA_GATEWAY]));+err=-EINVAL;+gotoout_err;+}+nhe->nh_gateway_len=RTA_PAYLOAD(tb[NHA_GATEWAY]);+memcpy(&nhe->nh_gateway,RTA_DATA(tb[NHA_GATEWAY]),+RTA_PAYLOAD(tb[NHA_GATEWAY]));+}++if(tb[NHA_ENCAP]){+nhe->nh_encap=malloc(RTA_LENGTH(RTA_PAYLOAD(tb[NHA_ENCAP])));+if(!nhe->nh_encap){+err=-ENOMEM;+gotoout_err;+}+memcpy(nhe->nh_encap,tb[NHA_ENCAP],+RTA_LENGTH(RTA_PAYLOAD(tb[NHA_ENCAP])));+memcpy(&nhe->nh_encap_type,tb[NHA_ENCAP_TYPE],+sizeof(nhe->nh_encap_type));+}++if(tb[NHA_GROUP]){+if(!__valid_nh_group_attr(tb[NHA_GROUP])){+fprintf(fp,"<nexthop id %u invalid nexthop group>",+nhe->nh_id);+err=-EINVAL;+gotoout_err;+}++nhe->nh_groups=malloc(RTA_PAYLOAD(tb[NHA_GROUP]));+if(!nhe->nh_groups){+err=-ENOMEM;+gotoout_err;+}+nhe->nh_groups_cnt=RTA_PAYLOAD(tb[NHA_GROUP])/+sizeof(structnexthop_grp);+memcpy(nhe->nh_groups,RTA_DATA(tb[NHA_GROUP]),+RTA_PAYLOAD(tb[NHA_GROUP]));+}++if(tb[NHA_RES_GROUP]){+parse_nh_res_group_rta(tb[NHA_RES_GROUP],&nhe->nh_res_grp);+nhe->nh_has_res_grp=true;+}++nhe->nh_blackhole=!!tb[NHA_BLACKHOLE];+nhe->nh_fdb=!!tb[NHA_FDB];++nhe->nh_family=nhm->nh_family;+nhe->nh_protocol=nhm->nh_protocol;+nhe->nh_scope=nhm->nh_scope;+nhe->nh_flags=nhm->nh_flags;++return0;++out_err:+ipnh_destroy_entry(nhe);+returnerr;+}+intprint_nexthop(structnlmsghdr*n,void*arg){structnhmsg*nhm=NLMSG_DATA(n);-structrtattr*tb[NHA_MAX+1];FILE*fp=(FILE*)arg;-intlen;+structnh_entrynhe;+intlen,err;SPRINT_BUF(b1);
From: Nikolay Aleksandrov <razor@blackwall.org> Date: 2021-09-30 11:39:47
From: Nikolay Aleksandrov <redacted>
Factor out ipnh_get_id's rtnl talk portion into a separate helper which
will be reused later to retrieve nexthops for caching.
Signed-off-by: Nikolay Aleksandrov <redacted>
---
ip/ipnexthop.c | 33 ++++++++++++++++++++-------------
1 file changed, 20 insertions(+), 13 deletions(-)
@@ -827,21 +846,9 @@ static int ipnh_modify(int cmd, unsigned int flags, int argc, char **argv)staticintipnh_get_id(__u32id){-struct{-structnlmsghdrn;-structnhmsgnhm;-charbuf[1024];-}req={-.n.nlmsg_len=NLMSG_LENGTH(sizeof(structnhmsg)),-.n.nlmsg_flags=NLM_F_REQUEST,-.n.nlmsg_type=RTM_GETNEXTHOP,-.nhm.nh_family=preferred_family,-};structnlmsghdr*answer;-addattr32(&req.n,sizeof(req),NHA_ID,id);--if(rtnl_talk(&rth,&req.n,&answer)<0)+if(__ipnh_get_id(&rth,id,&answer)<0)return-2;new_json_obj(json);
From: Nikolay Aleksandrov <razor@blackwall.org> Date: 2021-09-30 11:39:48
From: Nikolay Aleksandrov <redacted>
Add a static nexthop cache in a hash with 1024 buckets and helpers to
manage it (link, unlink, find, add nexthop, del nexthop). Adding new
nexthops is done by creating a new rtnl handle and using it to retrieve
the nexthop so the helper is safe to use while already reading a
response (i.e. using the global rth).
Signed-off-by: Nikolay Aleksandrov <redacted>
---
ip/ipnexthop.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++
ip/nh_common.h | 6 ++++
2 files changed, 104 insertions(+)
From: Nikolay Aleksandrov <razor@blackwall.org> Date: 2021-09-30 11:39:50
From: Nikolay Aleksandrov <redacted>
Add a helper which looks for a nexthop in the cache and if not found
reads the entry from the kernel and caches it. Finally the entry is
printed.
Signed-off-by: Nikolay Aleksandrov <redacted>
---
ip/ipnexthop.c | 16 ++++++++++++++++
ip/nh_common.h | 3 +++
2 files changed, 19 insertions(+)
From: Nikolay Aleksandrov <razor@blackwall.org> Date: 2021-09-30 11:39:51
From: Nikolay Aleksandrov <redacted>
If -d (show_details) is used when printing/monitoring routes then print
detailed nexthop information in the field "nh_info". The nexthop is also
cached for future searches.
Output looks like:
unicast 198.51.100.0/24 nhid 103 table 3 proto boot scope global
nh_info id 103 group 101/102 type resilient buckets 512 idle_timer 0 unbalanced_timer 0 unbalanced_time 0 scope global proto unspec
nexthop via 169.254.2.22 dev veth2 weight 1
nexthop via 169.254.3.23 dev veth4 weight 1
The nh_info field has the same format as ip -d nexthop show would've had
for the same nexthop id.
For completeness the JSON version looks like:
{
"type": "unicast",
"dst": "198.51.100.0/24",
"nhid": 103,
"table": "3",
"protocol": "boot",
"scope": "global",
"flags": [ ],
"nh_info": {
"id": 103,
"group": [ {
"id": 101
},{
"id": 102
} ],
"type": "resilient",
"resilient_args": {
"buckets": 512,
"idle_timer": 0,
"unbalanced_timer": 0,
"unbalanced_time": 0
},
"scope": "global",
"protocol": "unspec",
"flags": [ ]
},
"nexthops": [ {
"gateway": "169.254.2.22",
"dev": "veth2",
"weight": 1,
"flags": [ ]
},{
"gateway": "169.254.3.23",
"dev": "veth4",
"weight": 1,
"flags": [ ]
} ]
}
Signed-off-by: Nikolay Aleksandrov <redacted>
---
ip/iproute.c | 5 +++++
1 file changed, 5 insertions(+)
From: Nikolay Aleksandrov <razor@blackwall.org> Date: 2021-09-30 11:39:52
From: Nikolay Aleksandrov <redacted>
Add a new helper print_cache_nexthop replacing print_nexthop which can
update the nexthop cache if the process_cache argument is true. It is
used when monitoring netlink messages to keep the nexthop cache up to
date with nexthop changes happening. For the old callers and anyone
who's just dumping nexthops its _nocache version is used which is a
wrapper for print_cache_nexthop.
Signed-off-by: Nikolay Aleksandrov <redacted>
---
ip/ip_common.h | 1 -
ip/ipmonitor.c | 3 ++-
ip/ipnexthop.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++----
ip/nh_common.h | 1 +
4 files changed, 52 insertions(+), 6 deletions(-)
@@ -602,6 +602,42 @@ static void ipnh_cache_del(struct nh_entry *nhe)free(nhe);}+/* update, add or delete a nexthop entry based on nlmsghdr */+staticintipnh_cache_process_nlmsg(conststructnlmsghdr*n,+structnh_entry*new_nhe)+{+structnh_entry*nhe;++nhe=ipnh_cache_get(new_nhe->nh_id);+switch(n->nlmsg_type){+caseRTM_DELNEXTHOP:+if(nhe)+ipnh_cache_del(nhe);+ipnh_destroy_entry(new_nhe);+break;+caseRTM_NEWNEXTHOP:+if(!nhe){+nhe=malloc(sizeof(*nhe));+if(!nhe){+ipnh_destroy_entry(new_nhe);+return-1;+}+}else{+/* this allows us to save 1 allocation on updates by+*reusingtheoldnhentry,butweneedtocleanupits+*internalstorage+*/+ipnh_cache_unlink_entry(nhe);+ipnh_destroy_entry(nhe);+}+memcpy(nhe,new_nhe,sizeof(*nhe));+ipnh_cache_link_entry(nhe);+break;+}++return0;+}+voidprint_cache_nexthop_id(FILE*fp,constchar*fp_prefix,constchar*jsobj,__u32nh_id){
Hello:
This series was applied to iproute2/iproute2-next.git (refs/heads/main):
On Thu, 30 Sep 2021 14:38:32 +0300 you wrote:
From: Nikolay Aleksandrov <redacted>
Hi,
This set tries to help with an old ask that we've had for some time
which is to print nexthop information while monitoring or dumping routes.
The core problem is that people cannot follow nexthop changes while
monitoring route changes, by the time they check the nexthop it could be
deleted or updated to something else. In order to help them out I've
added a nexthop cache which is populated (only used if -d / show_details
is specified) while decoding routes and kept up to date while monitoring.
The nexthop information is printed on its own line starting with the
"nh_info" attribute and its embedded inside it if printing JSON. To
cache the nexthop entries I parse them into structures, in order to
reuse most of the code the print helpers have been altered so they rely
on prepared structures. Nexthops are now always parsed into a structure,
even if they won't be cached, that structure is later used to print the
nexthop and destroyed if not going to be cached. New nexthops (not found
in the cache) are retrieved from the kernel using a private netlink
socket so they don't disrupt an ongoing dump, similar to how interfaces
are retrieved and cached.
[...]
From: Nikolay Aleksandrov <razor@blackwall.org> Date: 2021-10-04 09:03:44
From: Nikolay Aleksandrov <redacted>
Since we use the cache netlink socket for each nexthop we can keep it open
instead of opening and closing it on every add call. The socket is opened
once, on the first add call and then reused for the rest.
Suggested-by: David Ahern <redacted>
Signed-off-by: Nikolay Aleksandrov <redacted>
---
I actually had this in my initial patchset, but switched it with the
open/close on each call. TBH, I don't recall why, perhaps to be the same
as the link cache. I don't see a reason not to keep the socket open.
I've re-run the stress test and the selftests, all look good.
ip/ipnexthop.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
From: David Ahern <hidden> Date: 2021-10-05 14:35:15
On 10/4/21 3:03 AM, Nikolay Aleksandrov wrote:
From: Nikolay Aleksandrov <redacted>
Since we use the cache netlink socket for each nexthop we can keep it open
instead of opening and closing it on every add call. The socket is opened
once, on the first add call and then reused for the rest.
Suggested-by: David Ahern <redacted>
Signed-off-by: Nikolay Aleksandrov <redacted>
---
I actually had this in my initial patchset, but switched it with the
open/close on each call. TBH, I don't recall why, perhaps to be the same
as the link cache. I don't see a reason not to keep the socket open.
I've re-run the stress test and the selftests, all look good.
ip/ipnexthop.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)