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 <mail@pascalmathis.com>

diff --git a/bgpd/bgp_snmp.c b/bgpd/bgp_snmp.c
index 2c7e4e04357b89ebc3098ef892a9c237d3414850..241b23a62db758d0320aa046ed9ca2c5e537faed 100644 (file)
--- 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;
 }