neighbor: Rework getCurrentNeighbors
Now allows filtering out undesirable neighbors before passing them in
them back.
Change-Id: I7f6484586ffbc8a875c7e4342d3180746f2e55e5
Signed-off-by: William A. Kennington III <wak@google.com>
diff --git a/ethernet_interface.cpp b/ethernet_interface.cpp
index 82be20e..a1438de 100644
--- a/ethernet_interface.cpp
+++ b/ethernet_interface.cpp
@@ -11,6 +11,7 @@
#include <arpa/inet.h>
#include <linux/ethtool.h>
+#include <linux/rtnetlink.h>
#include <linux/sockios.h>
#include <net/if.h>
#include <netinet/in.h>
@@ -110,11 +111,13 @@
{
staticNeighbors.clear();
- auto neighbors = getCurrentNeighbors();
+ NeighborFilter filter;
+ filter.interface = ifIndex();
+ filter.state = NUD_PERMANENT;
+ auto neighbors = getCurrentNeighbors(filter);
for (const auto& neighbor : neighbors)
{
- if (!neighbor.permanent || !neighbor.mac ||
- neighbor.interface != interfaceName())
+ if (!neighbor.mac)
{
continue;
}
@@ -128,6 +131,17 @@
}
}
+unsigned EthernetInterface::ifIndex() const
+{
+ unsigned idx = if_nametoindex(interfaceName().c_str());
+ if (idx == 0)
+ {
+ throw std::system_error(errno, std::generic_category(),
+ "if_nametoindex");
+ }
+ return idx;
+}
+
ObjectPath EthernetInterface::iP(IP::Protocol protType, std::string ipaddress,
uint8_t prefixLength, std::string gateway)
{
diff --git a/ethernet_interface.hpp b/ethernet_interface.hpp
index 5551950..3935390 100644
--- a/ethernet_interface.hpp
+++ b/ethernet_interface.hpp
@@ -127,6 +127,10 @@
*/
void createStaticNeighborObjects();
+ /* @brief Gets the index of the interface on the system
+ */
+ unsigned ifIndex() const;
+
/* @brief Gets all the ip addresses.
* @returns the list of ipaddress.
*/
diff --git a/neighbor.cpp b/neighbor.cpp
index d1b1c46..717719c 100644
--- a/neighbor.cpp
+++ b/neighbor.cpp
@@ -13,10 +13,9 @@
#include <sys/socket.h>
#include <sys/types.h>
-#include <cstring>
#include <stdexcept>
#include <string_view>
-#include <system_error>
+#include <utility>
#include <vector>
namespace phosphor
@@ -26,8 +25,8 @@
namespace detail
{
-void parseNeighbor(const nlmsghdr& hdr, std::string_view msg,
- std::vector<NeighborInfo>& neighbors)
+void parseNeighbor(const NeighborFilter& filter, const nlmsghdr& hdr,
+ std::string_view msg, std::vector<NeighborInfo>& neighbors)
{
if (hdr.nlmsg_type != RTM_NEWNEIGH)
{
@@ -35,15 +34,21 @@
}
auto ndm = extract<ndmsg>(msg, "Bad neighbor msg");
- NeighborInfo neighbor;
- neighbor.interface.resize(IF_NAMESIZE);
- if (if_indextoname(ndm.ndm_ifindex, neighbor.interface.data()) == nullptr)
+ // Filter out neighbors we don't care about
+ unsigned ifindex = ndm.ndm_ifindex;
+ if (filter.interface != 0 && filter.interface != ifindex)
{
- throw std::system_error(errno, std::generic_category(),
- "if_indextoname");
+ return;
}
- neighbor.interface.resize(strlen(neighbor.interface.c_str()));
- neighbor.permanent = ndm.ndm_state & NUD_PERMANENT;
+ if ((ndm.ndm_state & filter.state) == 0)
+ {
+ return;
+ }
+
+ // Build the neighbor info for our valid neighbor
+ NeighborInfo neighbor;
+ neighbor.interface = ifindex;
+ neighbor.state = ndm.ndm_state;
bool set_addr = false;
while (!msg.empty())
{
@@ -67,14 +72,15 @@
} // namespace detail
-std::vector<NeighborInfo> getCurrentNeighbors()
+std::vector<NeighborInfo> getCurrentNeighbors(const NeighborFilter& filter)
{
std::vector<NeighborInfo> neighbors;
- auto cb = [&neighbors](const nlmsghdr& hdr, std::string_view msg) {
- detail::parseNeighbor(hdr, msg, neighbors);
+ auto cb = [&filter, &neighbors](const nlmsghdr& hdr, std::string_view msg) {
+ detail::parseNeighbor(filter, hdr, msg, neighbors);
};
- netlink::performRequest(NETLINK_ROUTE, RTM_GETNEIGH, NLM_F_DUMP, ndmsg{},
- cb);
+ ndmsg msg{};
+ msg.ndm_ifindex = filter.interface;
+ netlink::performRequest(NETLINK_ROUTE, RTM_GETNEIGH, NLM_F_DUMP, msg, cb);
return neighbors;
}
diff --git a/neighbor.hpp b/neighbor.hpp
index ce1d34b..cca9c11 100644
--- a/neighbor.hpp
+++ b/neighbor.hpp
@@ -1,11 +1,11 @@
#pragma once
#include "types.hpp"
-#include "util.hpp"
#include <linux/netlink.h>
#include <net/ethernet.h>
+#include <cstdint>
#include <optional>
#include <sdbusplus/bus.hpp>
#include <sdbusplus/server/object.hpp>
@@ -27,20 +27,33 @@
class EthernetInterface;
+/* @class NeighborFilter
+ */
+struct NeighborFilter
+{
+ unsigned interface;
+ uint16_t state;
+
+ /* @brief Creates an empty filter */
+ NeighborFilter() : interface(0), state(~UINT16_C(0))
+ {
+ }
+};
+
/** @class NeighborInfo
* @brief Information about a neighbor from the kernel
*/
struct NeighborInfo
{
- std::string interface;
+ unsigned interface;
InAddrAny address;
std::optional<ether_addr> mac;
- bool permanent;
+ uint16_t state;
};
/** @brief Returns a list of the current system neighbor table
*/
-std::vector<NeighborInfo> getCurrentNeighbors();
+std::vector<NeighborInfo> getCurrentNeighbors(const NeighborFilter& filter);
/** @class Neighbor
* @brief OpenBMC network neighbor implementation.
@@ -83,8 +96,8 @@
namespace detail
{
-void parseNeighbor(const nlmsghdr& hdr, std::string_view msg,
- std::vector<NeighborInfo>& neighbors);
+void parseNeighbor(const NeighborFilter& filter, const nlmsghdr& hdr,
+ std::string_view msg, std::vector<NeighborInfo>& neighbors);
} // namespace detail
diff --git a/test/mock_syscall.cpp b/test/mock_syscall.cpp
index 421dbfe..5493c58 100644
--- a/test/mock_syscall.cpp
+++ b/test/mock_syscall.cpp
@@ -191,16 +191,11 @@
char* if_indextoname(unsigned ifindex, char* ifname)
{
- if (ifindex == 0)
- {
- errno = ENXIO;
- return NULL;
- }
auto it = mock_if_indextoname.find(ifindex);
if (it == mock_if_indextoname.end())
{
- // TODO: Return ENXIO once other code is mocked out
- return std::strcpy(ifname, "invalid");
+ errno = ENXIO;
+ return NULL;
}
return std::strcpy(ifname, it->second.c_str());
}
diff --git a/test/test_neighbor.cpp b/test/test_neighbor.cpp
index ecde351..c7d926e 100644
--- a/test/test_neighbor.cpp
+++ b/test/test_neighbor.cpp
@@ -1,4 +1,3 @@
-#include "mock_syscall.hpp"
#include "neighbor.hpp"
#include "util.hpp"
@@ -10,7 +9,6 @@
#include <cstring>
#include <stdexcept>
#include <string>
-#include <system_error>
#include <vector>
#include <gtest/gtest.h>
@@ -22,16 +20,14 @@
namespace detail
{
-constexpr auto ifStr = "eth0";
-constexpr auto ifIdx = 1;
-
TEST(ParseNeighbor, NotNeighborType)
{
nlmsghdr hdr{};
hdr.nlmsg_type = RTM_NEWLINK;
+ NeighborFilter filter;
std::vector<NeighborInfo> neighbors;
- EXPECT_THROW(parseNeighbor(hdr, "", neighbors), std::runtime_error);
+ EXPECT_THROW(parseNeighbor(filter, hdr, "", neighbors), std::runtime_error);
EXPECT_EQ(0, neighbors.size());
}
@@ -40,53 +36,38 @@
nlmsghdr hdr{};
hdr.nlmsg_type = RTM_NEWNEIGH;
std::string data = "1";
+ NeighborFilter filter;
std::vector<NeighborInfo> neighbors;
- EXPECT_THROW(parseNeighbor(hdr, data, neighbors), std::runtime_error);
- EXPECT_EQ(0, neighbors.size());
-}
-
-TEST(ParseNeighbor, BadIf)
-{
- nlmsghdr hdr{};
- hdr.nlmsg_type = RTM_NEWNEIGH;
- ndmsg msg{};
- std::string data;
- data.append(reinterpret_cast<char*>(&msg), sizeof(msg));
-
- std::vector<NeighborInfo> neighbors;
- EXPECT_THROW(parseNeighbor(hdr, data, neighbors), std::system_error);
+ EXPECT_THROW(parseNeighbor(filter, hdr, data, neighbors),
+ std::runtime_error);
EXPECT_EQ(0, neighbors.size());
}
TEST(ParseNeighbor, NoAttrs)
{
- mock_clear();
- mock_addIF(ifStr, ifIdx);
-
nlmsghdr hdr{};
hdr.nlmsg_type = RTM_NEWNEIGH;
ndmsg msg{};
- msg.ndm_ifindex = ifIdx;
- ASSERT_NE(0, msg.ndm_ifindex);
+ msg.ndm_ifindex = 1;
+ msg.ndm_state = NUD_REACHABLE;
std::string data;
data.append(reinterpret_cast<char*>(&msg), sizeof(msg));
+ NeighborFilter filter;
std::vector<NeighborInfo> neighbors;
- EXPECT_THROW(parseNeighbor(hdr, data, neighbors), std::runtime_error);
+ EXPECT_THROW(parseNeighbor(filter, hdr, data, neighbors),
+ std::runtime_error);
EXPECT_EQ(0, neighbors.size());
}
TEST(ParseNeighbor, NoAddress)
{
- mock_clear();
- mock_addIF(ifStr, ifIdx);
-
nlmsghdr hdr{};
hdr.nlmsg_type = RTM_NEWNEIGH;
ndmsg msg{};
- msg.ndm_ifindex = ifIdx;
- ASSERT_NE(0, msg.ndm_ifindex);
+ msg.ndm_ifindex = 1;
+ msg.ndm_state = NUD_REACHABLE;
ether_addr mac = {{0xff, 0xee, 0xdd, 0xcc, 0xbb, 0xaa}};
rtattr lladdr{};
lladdr.rta_len = RTA_LENGTH(sizeof(mac));
@@ -98,24 +79,22 @@
std::string data;
data.append(reinterpret_cast<char*>(&msg), sizeof(msg));
data.append(reinterpret_cast<char*>(&lladdrbuf), sizeof(lladdrbuf));
+ NeighborFilter filter;
std::vector<NeighborInfo> neighbors;
- EXPECT_THROW(parseNeighbor(hdr, data, neighbors), std::runtime_error);
+ EXPECT_THROW(parseNeighbor(filter, hdr, data, neighbors),
+ std::runtime_error);
EXPECT_EQ(0, neighbors.size());
}
TEST(ParseNeighbor, NoMAC)
{
- mock_clear();
- mock_addIF(ifStr, ifIdx);
-
nlmsghdr hdr{};
hdr.nlmsg_type = RTM_NEWNEIGH;
ndmsg msg{};
msg.ndm_family = AF_INET;
msg.ndm_state = NUD_PERMANENT;
- msg.ndm_ifindex = ifIdx;
- ASSERT_NE(0, msg.ndm_ifindex);
+ msg.ndm_ifindex = 1;
in_addr addr;
ASSERT_EQ(1, inet_pton(msg.ndm_family, "192.168.10.1", &addr));
rtattr dst{};
@@ -128,27 +107,95 @@
std::string data;
data.append(reinterpret_cast<char*>(&msg), sizeof(msg));
data.append(reinterpret_cast<char*>(&dstbuf), sizeof(dstbuf));
+ NeighborFilter filter;
std::vector<NeighborInfo> neighbors;
- parseNeighbor(hdr, data, neighbors);
+ parseNeighbor(filter, hdr, data, neighbors);
EXPECT_EQ(1, neighbors.size());
- EXPECT_EQ(ifStr, neighbors[0].interface);
- EXPECT_TRUE(neighbors[0].permanent);
+ EXPECT_EQ(msg.ndm_ifindex, neighbors[0].interface);
+ EXPECT_EQ(msg.ndm_state, neighbors[0].state);
+ EXPECT_FALSE(neighbors[0].mac);
+ EXPECT_TRUE(equal(addr, std::get<in_addr>(neighbors[0].address)));
+}
+
+TEST(ParseNeighbor, FilterInterface)
+{
+ nlmsghdr hdr{};
+ hdr.nlmsg_type = RTM_NEWNEIGH;
+ ndmsg msg{};
+ msg.ndm_family = AF_INET;
+ msg.ndm_state = NUD_PERMANENT;
+ msg.ndm_ifindex = 2;
+ in_addr addr;
+ ASSERT_EQ(1, inet_pton(msg.ndm_family, "192.168.10.1", &addr));
+ rtattr dst{};
+ dst.rta_len = RTA_LENGTH(sizeof(addr));
+ dst.rta_type = NDA_DST;
+ char dstbuf[RTA_ALIGN(dst.rta_len)];
+ std::memset(dstbuf, '\0', sizeof(dstbuf));
+ std::memcpy(dstbuf, &dst, sizeof(dst));
+ std::memcpy(RTA_DATA(dstbuf), &addr, sizeof(addr));
+ std::string data;
+ data.append(reinterpret_cast<char*>(&msg), sizeof(msg));
+ data.append(reinterpret_cast<char*>(&dstbuf), sizeof(dstbuf));
+ NeighborFilter filter;
+
+ std::vector<NeighborInfo> neighbors;
+ filter.interface = 1;
+ parseNeighbor(filter, hdr, data, neighbors);
+ EXPECT_EQ(0, neighbors.size());
+ filter.interface = 2;
+ parseNeighbor(filter, hdr, data, neighbors);
+ EXPECT_EQ(1, neighbors.size());
+ EXPECT_EQ(msg.ndm_ifindex, neighbors[0].interface);
+ EXPECT_EQ(msg.ndm_state, neighbors[0].state);
+ EXPECT_FALSE(neighbors[0].mac);
+ EXPECT_TRUE(equal(addr, std::get<in_addr>(neighbors[0].address)));
+}
+
+TEST(ParseNeighbor, FilterState)
+{
+ nlmsghdr hdr{};
+ hdr.nlmsg_type = RTM_NEWNEIGH;
+ ndmsg msg{};
+ msg.ndm_family = AF_INET;
+ msg.ndm_state = NUD_PERMANENT;
+ msg.ndm_ifindex = 2;
+ in_addr addr;
+ ASSERT_EQ(1, inet_pton(msg.ndm_family, "192.168.10.1", &addr));
+ rtattr dst{};
+ dst.rta_len = RTA_LENGTH(sizeof(addr));
+ dst.rta_type = NDA_DST;
+ char dstbuf[RTA_ALIGN(dst.rta_len)];
+ std::memset(dstbuf, '\0', sizeof(dstbuf));
+ std::memcpy(dstbuf, &dst, sizeof(dst));
+ std::memcpy(RTA_DATA(dstbuf), &addr, sizeof(addr));
+ std::string data;
+ data.append(reinterpret_cast<char*>(&msg), sizeof(msg));
+ data.append(reinterpret_cast<char*>(&dstbuf), sizeof(dstbuf));
+ NeighborFilter filter;
+
+ std::vector<NeighborInfo> neighbors;
+ filter.state = NUD_NOARP;
+ parseNeighbor(filter, hdr, data, neighbors);
+ EXPECT_EQ(0, neighbors.size());
+ filter.state = NUD_PERMANENT | NUD_NOARP;
+ parseNeighbor(filter, hdr, data, neighbors);
+ EXPECT_EQ(1, neighbors.size());
+ EXPECT_EQ(msg.ndm_ifindex, neighbors[0].interface);
+ EXPECT_EQ(msg.ndm_state, neighbors[0].state);
EXPECT_FALSE(neighbors[0].mac);
EXPECT_TRUE(equal(addr, std::get<in_addr>(neighbors[0].address)));
}
TEST(ParseNeighbor, Full)
{
- mock_clear();
- mock_addIF(ifStr, ifIdx);
-
nlmsghdr hdr{};
hdr.nlmsg_type = RTM_NEWNEIGH;
ndmsg msg{};
msg.ndm_family = AF_INET6;
msg.ndm_state = NUD_NOARP;
- msg.ndm_ifindex = ifIdx;
+ msg.ndm_ifindex = 1;
ether_addr mac = {{0xff, 0xee, 0xdd, 0xcc, 0xbb, 0xaa}};
rtattr lladdr{};
lladdr.rta_len = RTA_LENGTH(sizeof(mac));
@@ -170,12 +217,13 @@
data.append(reinterpret_cast<char*>(&msg), sizeof(msg));
data.append(reinterpret_cast<char*>(&lladdrbuf), sizeof(lladdrbuf));
data.append(reinterpret_cast<char*>(&dstbuf), sizeof(dstbuf));
+ NeighborFilter filter;
std::vector<NeighborInfo> neighbors;
- parseNeighbor(hdr, data, neighbors);
+ parseNeighbor(filter, hdr, data, neighbors);
EXPECT_EQ(1, neighbors.size());
- EXPECT_EQ(ifStr, neighbors[0].interface);
- EXPECT_FALSE(neighbors[0].permanent);
+ EXPECT_EQ(msg.ndm_ifindex, neighbors[0].interface);
+ EXPECT_EQ(msg.ndm_state, neighbors[0].state);
EXPECT_TRUE(neighbors[0].mac);
EXPECT_TRUE(equal(mac, *neighbors[0].mac));
EXPECT_TRUE(equal(addr, std::get<in6_addr>(neighbors[0].address)));