From 8aee963295f7da07ae67aa09c4eba3fbd2a6ff19 Mon Sep 17 00:00:00 2001 From: Johnathan Mantey Date: Thu, 30 Jan 2020 15:07:39 -0800 Subject: [PATCH] Enhance DHCP beyond just OFF and IPv4/IPv6 enabled. DHCP is not a binary option. The network interface can have DHCP disabled, IPv4 only, IPv6 only, and IPv4/IPv6. Tested: Using dbus-send or busctl: Disabled DHCP, and confirmed only link local addresses were present. Assigned only static addresses. Both with/and without the gateway set to 0.0.0.0 Deleted static IPv4 addresses. Reassigned static addresses. Enabled DHCP for ipv4 only, and witnessed a DHCP server assign a valid address. Assigned static IPv4 address. Assigned static IPv6 address. Confirmed both IPv4 and IPv6 static addresses are active. Enabled DHCP for ipv6 only, and confirmed the static v4 address remains. The ipv6 address is removed, waiting for a DHCP6 server. Enabled DHCP for both ipv4 and ipv6. IPv4 address was assigned. IPv6 address is assumed to succeed, as systemd config file enables IPv6 DHCP. Change-Id: I2e0ff80ac3a5e88bcff28adac419bf21e37be162 Signed-off-by: Johnathan Mantey %% original patch: 0009-Enhance-DHCP-beyond-just-OFF-and-IPv4-IPv6-enabled.patch --- Makefile.am | 1 + configure.ac | 1 + ethernet_interface.cpp | 146 ++++++++++++++++++++++--------- ethernet_interface.hpp | 31 ++++++- test/test_ethernet_interface.cpp | 3 +- test/test_vlan_interface.cpp | 3 +- types.hpp | 3 + util.cpp | 16 +++- util.hpp | 7 +- vlan_interface.cpp | 2 +- vlan_interface.hpp | 4 +- 11 files changed, 164 insertions(+), 53 deletions(-) diff --git a/Makefile.am b/Makefile.am index 3bb5e7b..0bbbc8f 100644 --- a/Makefile.am +++ b/Makefile.am @@ -97,6 +97,7 @@ phosphor_network_manager_CXXFLAGS = \ $(SDEVENTPLUS_CFLAGS) \ $(PHOSPHOR_DBUS_INTERFACES_CFLAGS) \ $(PHOSPHOR_LOGGING_CFLAGS) \ + -DBOOST_ASIO_DISABLE_THREADS \ -flto if FEATURE_NIC_ETHTOOL diff --git a/configure.ac b/configure.ac index 12d6caa..fed3e09 100644 --- a/configure.ac +++ b/configure.ac @@ -36,6 +36,7 @@ AC_PATH_PROG([SDBUSPLUSPLUS], [sdbus++]) PKG_CHECK_MODULES([PHOSPHOR_LOGGING], [phosphor-logging]) PKG_CHECK_MODULES([PHOSPHOR_DBUS_INTERFACES], [phosphor-dbus-interfaces]) PKG_CHECK_MODULES([LIBNL], [libnl-3.0 libnl-genl-3.0]) +AC_CHECK_HEADER(boost/algorithm/string/split.hpp, [], [AC_MSG_ERROR([Could not find boost/algorithm/string/split.hpp])]) # Checks for header files. AC_CHECK_HEADER(systemd/sd-bus.h, ,\ diff --git a/ethernet_interface.cpp b/ethernet_interface.cpp index d6c7bdd..82716f9 100644 --- a/ethernet_interface.cpp +++ b/ethernet_interface.cpp @@ -3,7 +3,6 @@ #include "ethernet_interface.hpp" #include "config_parser.hpp" -#include "ipaddress.hpp" #include "neighbor.hpp" #include "network_manager.hpp" #include "vlan_interface.hpp" @@ -69,10 +68,12 @@ struct EthernetIntfSocket int sock{-1}; }; +std::map mapDHCPToSystemd = { + {"both", "true"}, {"v4", "ipv4"}, {"v6", "ipv6"}, {"none", "false"}}; EthernetInterface::EthernetInterface(sdbusplus::bus::bus& bus, const std::string& objPath, - bool dhcpEnabled, Manager& parent, + DHCPConf dhcpEnabled, Manager& parent, bool emitSignal) : Ifaces(bus, objPath.c_str(), true), bus(bus), manager(parent), objPath(objPath) @@ -119,6 +120,65 @@ static IP::Protocol convertFamily(int family) throw std::invalid_argument("Bad address family"); } +void EthernetInterface::disableDHCP(IP::Protocol protocol) +{ + DHCPConf dhcpState = EthernetInterfaceIntf::dHCPEnabled(); + if (dhcpState == EthernetInterface::DHCPConf::both) + { + if (protocol == IP::Protocol::IPv4) + { + dHCPEnabled(EthernetInterface::DHCPConf::v6); + } + else if (protocol == IP::Protocol::IPv6) + { + dHCPEnabled(EthernetInterface::DHCPConf::v4); + } + } + else if ((dhcpState == EthernetInterface::DHCPConf::v4) && + (protocol == IP::Protocol::IPv4)) + { + dHCPEnabled(EthernetInterface::DHCPConf::none); + } + else if ((dhcpState == EthernetInterface::DHCPConf::v6) && + (protocol == IP::Protocol::IPv6)) + { + dHCPEnabled(EthernetInterface::DHCPConf::none); + } +} + +bool EthernetInterface::dhcpIsEnabled(IP::Protocol family, bool ignoreProtocol) +{ + return ((EthernetInterfaceIntf::dHCPEnabled() == + EthernetInterface::DHCPConf::both) || + ((EthernetInterfaceIntf::dHCPEnabled() == + EthernetInterface::DHCPConf::v6) && + ((family == IP::Protocol::IPv6) || ignoreProtocol)) || + ((EthernetInterfaceIntf::dHCPEnabled() == + EthernetInterface::DHCPConf::v4) && + ((family == IP::Protocol::IPv4) || ignoreProtocol))); +} + +bool EthernetInterface::dhcpToBeEnabled(IP::Protocol family, + std::string& nextDHCPState) +{ + return ((nextDHCPState == "true") || + ((nextDHCPState == "ipv6") && (family == IP::Protocol::IPv6)) || + ((nextDHCPState == "ipv4") && (family == IP::Protocol::IPv4))); +} + +bool EthernetInterface::addressIsStatic(IP::AddressOrigin origin) +{ + return ( +#ifdef LINK_LOCAL_AUTOCONFIGURATION + (origin == IP::AddressOrigin::Static) +#else + (origin == IP::AddressOrigin::Static || + origin == IP::AddressOrigin::LinkLocal) +#endif + + ); +} + void EthernetInterface::createIPAddressObjects() { addrs.clear(); @@ -129,7 +189,7 @@ void EthernetInterface::createIPAddressObjects() { IP::Protocol addressType = convertFamily(addr.addrType); IP::AddressOrigin origin = IP::AddressOrigin::Static; - if (dHCPEnabled()) + if (dhcpIsEnabled(addressType)) { origin = IP::AddressOrigin::DHCP; } @@ -190,11 +250,11 @@ ObjectPath EthernetInterface::iP(IP::Protocol protType, std::string ipaddress, uint8_t prefixLength, std::string gateway) { - if (dHCPEnabled()) + if (dhcpIsEnabled(protType)) { log("DHCP enabled on the interface"), entry("INTERFACE=%s", interfaceName().c_str()); - dHCPEnabled(false); + disableDHCP(protType); } IP::AddressOrigin origin = IP::AddressOrigin::Static; @@ -469,7 +529,7 @@ bool EthernetInterface::iPv6AcceptRA(bool value) return value; } -bool EthernetInterface::dHCPEnabled(bool value) +EthernetInterface::DHCPConf EthernetInterface::dHCPEnabled(DHCPConf value) { if (value == EthernetInterfaceIntf::dHCPEnabled()) { @@ -685,7 +745,7 @@ void EthernetInterface::loadVLAN(VlanId id) std::string path = objPath; path += "_" + std::to_string(id); - auto dhcpEnabled = + DHCPConf dhcpEnabled = getDHCPValue(manager.getConfDir().string(), vlanInterfaceName); auto vlanIntf = std::make_unique( @@ -707,7 +767,8 @@ ObjectPath EthernetInterface::createVLAN(VlanId id) path += "_" + std::to_string(id); auto vlanIntf = std::make_unique( - bus, path.c_str(), false, id, *this, manager); + bus, path.c_str(), EthernetInterface::DHCPConf::none, id, *this, + manager); // write the device file for the vlan interface. vlanIntf->writeDeviceFile(); @@ -780,8 +841,6 @@ void EthernetInterface::writeConfigurationFile() // write all the static ip address in the systemd-network conf file using namespace std::string_literals; - using AddressOrigin = - sdbusplus::xyz::openbmc_project::Network::server::IP::AddressOrigin; namespace fs = std::experimental::filesystem; // if there is vlan interafce then write the configuration file @@ -855,42 +914,45 @@ void EthernetInterface::writeConfigurationFile() } // Add the DHCP entry - auto value = dHCPEnabled() ? "true"s : "false"s; - stream << "DHCP="s + value + "\n"; + std::string value = convertForMessage(EthernetInterfaceIntf::dHCPEnabled()); + std::string::size_type loc = value.rfind("."); + std::string requestedDHCPState = value.substr(loc + 1); + std::string mappedDHCPState = mapDHCPToSystemd[requestedDHCPState]; + stream << "DHCP="s + mappedDHCPState + "\n"; + + bool dhcpv6Requested = dhcpToBeEnabled(IP::Protocol::IPv6, mappedDHCPState); + bool dhcpv4Requested = dhcpToBeEnabled(IP::Protocol::IPv4, mappedDHCPState); + // Static IP addresses + for (const auto& addr : addrs) + { + bool isValidIPv4 = isValidIP(AF_INET, addr.second->address()); + bool isValidIPv6 = isValidIP(AF_INET6, addr.second->address()); + if (((!dhcpv4Requested && isValidIPv4) || + (!dhcpv6Requested && isValidIPv6)) && + addressIsStatic(addr.second->origin())) + { + // Process all static addresses + std::string address = addr.second->address() + "/" + + std::to_string(addr.second->prefixLength()); + + // build the address entries. Do not use [Network] shortcuts to + // insert address entries. + stream << "[Address]\n"; + stream << "Address=" << address << "\n"; + } + } - // When the interface configured as dhcp, we don't need below given entries - // in config file. - if (dHCPEnabled() == false) + if (manager.getSystemConf()) { - // Static - for (const auto& addr : addrs) + const auto& gateway = manager.getSystemConf()->defaultGateway(); + if (!gateway.empty()) { - if (addr.second->origin() == AddressOrigin::Static -#ifndef LINK_LOCAL_AUTOCONFIGURATION - || addr.second->origin() == AddressOrigin::LinkLocal -#endif - ) - { - std::string address = - addr.second->address() + "/" + - std::to_string(addr.second->prefixLength()); - - stream << "Address=" << address << "\n"; - } + stream << "Gateway=" << gateway << "\n"; } - - if (manager.getSystemConf()) + const auto& gateway6 = manager.getSystemConf()->defaultGateway6(); + if (!gateway6.empty()) { - const auto& gateway = manager.getSystemConf()->defaultGateway(); - if (!gateway.empty()) - { - stream << "Gateway=" << gateway << "\n"; - } - const auto& gateway6 = manager.getSystemConf()->defaultGateway6(); - if (!gateway6.empty()) - { - stream << "Gateway=" << gateway6 << "\n"; - } + stream << "Gateway=" << gateway6 << "\n"; } } @@ -1001,7 +1063,7 @@ std::string EthernetInterface::mACAddress(std::string value) void EthernetInterface::deleteAll() { - if (EthernetInterfaceIntf::dHCPEnabled()) + if (dhcpIsEnabled(IP::Protocol::IPv4, true)) { log("DHCP enabled on the interface"), entry("INTERFACE=%s", interfaceName().c_str()); diff --git a/ethernet_interface.hpp b/ethernet_interface.hpp index 6344533..3f7fd31 100644 --- a/ethernet_interface.hpp +++ b/ethernet_interface.hpp @@ -94,7 +94,7 @@ class EthernetInterface : public Ifaces * send. */ EthernetInterface(sdbusplus::bus::bus& bus, const std::string& objPath, - bool dhcpEnabled, Manager& parent, + DHCPConf dhcpEnabled, Manager& parent, bool emitSignal = true); /** @brief Function used to load the nameservers. @@ -164,7 +164,34 @@ class EthernetInterface : public Ifaces } /** Set value of DHCPEnabled */ - bool dHCPEnabled(bool value) override; + DHCPConf dHCPEnabled(DHCPConf value) override; + + /** @brief Determines if DHCP is active for the IP::Protocol supplied. + * @param[in] protocol - Either IPv4 or IPv6 + * @param[in] ignoreProtocol - Allows IPv4 and IPv6 to be checked using a + * single call. + * @returns true/false value if DHCP is active for the input protocol + */ + bool dhcpIsEnabled(IP::Protocol protocol, bool ignoreProtocol = false); + + /** @brief Determines if DHCP will be active following next reconfig + * @param[in] protocol - Either IPv4 or IPv6 + * @param[in] nextDHCPState - The new DHCP mode to take affect + * @returns true/false value if DHCP is active for the input protocol + */ + bool dhcpToBeEnabled(IP::Protocol family, std::string& nextDHCPState); + + /** @brief Determines if the address is manually assigned + * @param[in] origin - The origin entry of the IP::Address + * @returns true/false value if the address is static + */ + bool addressIsStatic(IP::AddressOrigin origin); + + /** @brief Selectively disables DHCP + * @param[in] protocol - The IPv4 or IPv6 protocol to return to static + * addressing mode + */ + void disableDHCP(IP::Protocol protocol); /** Retrieve Link State */ bool linkUp() const override; diff --git a/test/test_ethernet_interface.cpp b/test/test_ethernet_interface.cpp index d0beef7..3e2f9ff 100644 --- a/test/test_ethernet_interface.cpp +++ b/test/test_ethernet_interface.cpp @@ -59,7 +59,8 @@ class TestEthernetInterface : public testing::Test { mock_clear(); mock_addIF("test0", 1, mac); - return {bus, "/xyz/openbmc_test/network/test0", false, manager, true}; + return {bus, "/xyz/openbmc_test/network/test0", + EthernetInterface::DHCPConf::none, manager}; } int countIPObjects() diff --git a/test/test_vlan_interface.cpp b/test/test_vlan_interface.cpp index 1dffc7e..e49b43f 100644 --- a/test/test_vlan_interface.cpp +++ b/test/test_vlan_interface.cpp @@ -50,7 +50,8 @@ class TestVlanInterface : public testing::Test { mock_clear(); mock_addIF("test0", 1); - return {bus, "/xyz/openbmc_test/network/test0", false, manager}; + return {bus, "/xyz/openbmc_test/network/test0", + EthernetInterface::DHCPConf::none, manager}; } void setConfDir() diff --git a/types.hpp b/types.hpp index 123067a..c4409fe 100644 --- a/types.hpp +++ b/types.hpp @@ -1,5 +1,7 @@ #pragma once +#include "ipaddress.hpp" + #include #include #include @@ -50,6 +52,7 @@ struct AddrInfo { uint8_t addrType; std::string ipaddress; + IP::AddressOrigin origin; uint16_t prefix; }; diff --git a/util.cpp b/util.cpp index 13a607f..554d7f6 100644 --- a/util.cpp +++ b/util.cpp @@ -410,9 +410,11 @@ std::optional interfaceToUbootEthAddr(const char* intf) return "eth" + std::to_string(idx) + "addr"; } -bool getDHCPValue(const std::string& confDir, const std::string& intf) +EthernetInterfaceIntf::DHCPConf getDHCPValue(const std::string& confDir, + const std::string& intf) { - bool dhcp = false; + EthernetInterfaceIntf::DHCPConf dhcp = + EthernetInterfaceIntf::DHCPConf::none; // Get the interface mode value from systemd conf // using namespace std::string_literals; fs::path confPath = confDir; @@ -434,7 +436,15 @@ bool getDHCPValue(const std::string& confDir, const std::string& intf) // There will be only single value for DHCP key. if (values[0] == "true") { - dhcp = true; + dhcp = EthernetInterfaceIntf::DHCPConf::both; + } + else if (values[0] == "ipv4") + { + dhcp = EthernetInterfaceIntf::DHCPConf::v4; + } + else if (values[0] == "ipv6") + { + dhcp = EthernetInterfaceIntf::DHCPConf::v6; } return dhcp; } diff --git a/util.hpp b/util.hpp index 251aa0d..ee11f4e 100644 --- a/util.hpp +++ b/util.hpp @@ -13,12 +13,16 @@ #include #include #include +#include namespace phosphor { namespace network { +using EthernetInterfaceIntf = + sdbusplus::xyz::openbmc_project::Network::server::EthernetInterface; + constexpr auto IPV4_MIN_PREFIX_LENGTH = 1; constexpr auto IPV4_MAX_PREFIX_LENGTH = 32; constexpr auto IPV6_MAX_PREFIX_LENGTH = 64; @@ -156,7 +160,8 @@ std::optional interfaceToUbootEthAddr(const char* intf); * @param[in] confDir - Network configuration directory. * @param[in] intf - Interface name. */ -bool getDHCPValue(const std::string& confDir, const std::string& intf); +EthernetInterfaceIntf::DHCPConf getDHCPValue(const std::string& confDir, + const std::string& intf); namespace internal { diff --git a/vlan_interface.cpp b/vlan_interface.cpp index 73de4e8..26282cb 100644 --- a/vlan_interface.cpp +++ b/vlan_interface.cpp @@ -22,7 +22,7 @@ using namespace phosphor::logging; using namespace sdbusplus::xyz::openbmc_project::Common::Error; VlanInterface::VlanInterface(sdbusplus::bus::bus& bus, - const std::string& objPath, bool dhcpEnabled, + const std::string& objPath, DHCPConf dhcpEnabled, uint32_t vlanID, EthernetInterface& intf, Manager& parent) : VlanIface(bus, objPath.c_str()), diff --git a/vlan_interface.hpp b/vlan_interface.hpp index a994d05..37ae7ee 100644 --- a/vlan_interface.hpp +++ b/vlan_interface.hpp @@ -45,8 +45,8 @@ class VlanInterface : public VlanIface, * @param[in] manager - network manager object. */ VlanInterface(sdbusplus::bus::bus& bus, const std::string& objPath, - bool dhcpEnabled, uint32_t vlanID, EthernetInterface& intf, - Manager& manager); + DHCPConf dhcpEnabled, uint32_t vlanID, + EthernetInterface& intf, Manager& manager); /** @brief Delete this d-bus object. */ -- 2.25.2