From d27a91dc564d113ce38715c35b8514a6562aef57 Mon Sep 17 00:00:00 2001 From: Pascal Mathis Date: Thu, 10 May 2018 23:35:37 +0200 Subject: [PATCH] bgpd: fix and improve snmp peer lookups The previous implementation of bgp_peer_lookup_next did not consider the internal ordering of peers when using peer groups, which led to all standalone peers being skipped that had a lower IP address than the highest IP address of a peer belonging to a group. As the ordering of peers can not be arbitrary due to SNMP requiring increasing OIDs when walking an OID tree, this commit fixes the bug by properly looping through all peers and detecting the next highest IP address. Additionally, this commit improved both bgp_peer_lookup_next and peer_lookup_addr_ipv4 by using the socketunion stored within the peer struct (peer->su) instead of calling inet_pton for each peer during comparison. Signed-off-by: Pascal Mathis (cherry picked from commit 2b8e62f2db185e5c2c11d691523b3f734d224e95) --- bgpd/bgp_snmp.c | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/bgpd/bgp_snmp.c b/bgpd/bgp_snmp.c index 2c7e4e0435..241b23a62d 100644 --- a/bgpd/bgp_snmp.c +++ b/bgpd/bgp_snmp.c @@ -356,20 +356,19 @@ static struct peer *peer_lookup_addr_ipv4(struct in_addr *src) struct bgp *bgp; struct peer *peer; struct listnode *node; - struct in_addr addr; - int ret; bgp = bgp_get_default(); if (!bgp) return NULL; for (ALL_LIST_ELEMENTS_RO(bgp->peer, node, peer)) { - ret = inet_pton(AF_INET, peer->host, &addr); - if (ret > 0) { - if (IPV4_ADDR_SAME(&addr, src)) - return peer; - } + if (sockunion_family(&peer->su) != AF_INET) + continue; + + if (sockunion2ip(&peer->su) == src->s_addr) + return peer; } + return NULL; } @@ -377,28 +376,31 @@ static struct peer *bgp_peer_lookup_next(struct in_addr *src) { struct bgp *bgp; struct peer *peer; + struct peer *next_peer = NULL; struct listnode *node; - struct in_addr *p; - union sockunion su; - int ret; - - sockunion_init(&su); bgp = bgp_get_default(); if (!bgp) return NULL; for (ALL_LIST_ELEMENTS_RO(bgp->peer, node, peer)) { - ret = inet_pton(AF_INET, peer->host, &su.sin.sin_addr); - if (ret > 0) { - p = &su.sin.sin_addr; - - if (ntohl(p->s_addr) > ntohl(src->s_addr)) { - src->s_addr = p->s_addr; - return peer; - } + if (sockunion_family(&peer->su) != AF_INET) + continue; + if (ntohl(sockunion2ip(&peer->su)) <= ntohl(src->s_addr)) + continue; + + if (!next_peer + || ntohl(sockunion2ip(&next_peer->su)) + > ntohl(sockunion2ip(&peer->su))) { + next_peer = peer; } } + + if (next_peer) { + src->s_addr = sockunion2ip(&next_peer->su); + return next_peer; + } + return NULL; } -- 2.39.5