Skip to content

Commit

Permalink
Delete IPv4 default gateway when deleting an IPv4 static address
Browse files Browse the repository at this point in the history
The Redfish schema for creating static IPv4 addresses requires the IP
address, the netmask, and a gateway IP address. There's an issue
inherent with this method. A network interface is only permitted a
single IPv4 default gateway. If more than one IPv4 static address is
assigned to the NIC each entry is processed, and potentially
conflicting default gateways may be assigned. The last entry processed
assigns the IPv4 default gateway. This behavior will cause unexpected
results. It is necessary to prevent assigning mismatched default
gateway values.

The IPv4 address removal process requires additional work also. The
default gateway value is left in place even after the final static
IPv4 address is removed. It is necessary to perform an additional
action to clear the gateway address. Without explicit removal the
network is left in a condition that may prevent IP traffic from being
able to be sent from the BMC. This even in the event that the NIC is
actively being managed via DHCPv4.

Tested:
Disabled DHCPv4 on a secondary NIC (eth1)
Assigned a static IPv4 address.
Inspected the systemd-networkd config file in order to confirm the
Gateway entry is added. This is done to be explicitly sure the
network.config file has the Gateway entry.
Sent a Redfish PATCH command to delete the static IPv4 address.
Confirmed that the systemd-networkd config file no longer contained a
Gateway entry. This is done to be explicitly sure the network.config
file no longer contains the Gateway entry.
Created a PATCH containing multiple IPv4 static addresses all with
different Gateway values. Confirmed an error is returned when a
mismatch occurs in the Gateway values.
Assigned a new static address, and then restored DHCPv4.
Confirmed that the default gateway entry in the config file is removed.
Submitted a delete request for the remaining static IPv4 address that
is now orphaned by re-enabling DHCPv4. This removed the static IPv4
address.

Change-Id: Ia12cf2a38ba86266ce71dc28475b0d07b7e09ebc
Signed-off-by: Johnathan Mantey <[email protected]>
  • Loading branch information
Howitzer105mm authored and edtanous committed May 3, 2024
1 parent 102a4cd commit 743eb1c
Showing 1 changed file with 117 additions and 49 deletions.
166 changes: 117 additions & 49 deletions redfish-core/lib/ethernet.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ enum class LinkType
Global
};

enum class IpVersion
{
IpV4,
IpV6
};

/**
* Structure for keeping IPv4 data required by Redfish
*/
Expand Down Expand Up @@ -700,7 +706,30 @@ inline void extractIPData(const std::string& ethifaceId,
}

/**
* @brief Deletes given IPv4 interface
* @brief Modifies the default gateway assigned to the NIC
*
* @param[in] ifaceId Id of network interface whose default gateway is to be
* changed
* @param[in] gateway The new gateway value. Assigning an empty string
* causes the gateway to be deleted
* @param[io] asyncResp Response object that will be returned to client
*
* @return None
*/
inline void updateIPv4DefaultGateway(
const std::string& ifaceId, const std::string& gateway,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp)
{
setDbusProperty(
asyncResp, "xyz.openbmc_project.Network",
sdbusplus::message::object_path("/xyz/openbmc_project/network") /
ifaceId,
"xyz.openbmc_project.Network.EthernetInterface", "DefaultGateway",
"Gateway", gateway);
}

/**
* @brief Deletes given static IP address for the interface
*
* @param[in] ifaceId Id of interface whose IP should be deleted
* @param[in] ipHash DBus Hash id of IP that should be deleted
Expand All @@ -724,17 +753,6 @@ inline void deleteIPAddress(const std::string& ifaceId,
"xyz.openbmc_project.Object.Delete", "Delete");
}

inline void updateIPv4DefaultGateway(
const std::string& ifaceId, const std::string& gateway,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp)
{
setDbusProperty(
asyncResp, "xyz.openbmc_project.Network",
sdbusplus::message::object_path("/xyz/openbmc_project/network") /
ifaceId,
"xyz.openbmc_project.Network.EthernetInterface", "DefaultGateway",
"Gateway", gateway);
}
/**
* @brief Creates a static IPv4 entry
*
Expand All @@ -757,7 +775,6 @@ inline void createIPv4(const std::string& ifaceId, uint8_t prefixLength,
messages::internalError(asyncResp->res);
return;
}
updateIPv4DefaultGateway(ifaceId, gateway, asyncResp);
};

crow::connections::systemBus->async_method_call(
Expand All @@ -769,24 +786,19 @@ inline void createIPv4(const std::string& ifaceId, uint8_t prefixLength,
}

/**
* @brief Deletes the IPv6 entry for this interface and creates a replacement
* static IPv6 entry
* @brief Deletes the IP entry for this interface and creates a replacement
* static entry
*
* @param[in] ifaceId Id of interface upon which to create the IPv6 entry
* @param[in] id The unique hash entry identifying the DBus entry
* @param[in] prefixLength IPv6 prefix syntax for the subnet mask
* @param[in] address IPv6 address to assign to this interface
* @param[io] asyncResp Response object that will be returned to client
* @param[in] ifaceId Id of interface upon which to create the IPv6 entry
* @param[in] id The unique hash entry identifying the DBus entry
* @param[in] prefixLength Prefix syntax for the subnet mask
* @param[in] address Address to assign to this interface
* @param[in] numStaticAddrs Count of IPv4 static addresses
* @param[io] asyncResp Response object that will be returned to client
*
* @return None
*/

enum class IpVersion
{
IpV4,
IpV6
};

inline void deleteAndCreateIPAddress(
IpVersion version, const std::string& ifaceId, const std::string& id,
uint8_t prefixLength, const std::string& address,
Expand Down Expand Up @@ -1391,6 +1403,10 @@ inline void handleDHCPPatch(const std::string& ifaceId,
bool ipv4Active = translateDhcpEnabledToBool(ethData.dhcpEnabled, true);
bool ipv6Active = translateDhcpEnabledToBool(ethData.dhcpEnabled, false);

if (ipv4Active)
{
updateIPv4DefaultGateway(ifaceId, "", asyncResp);
}
bool nextv4DHCPState =
v4dhcpParms.dhcpv4Enabled ? *v4dhcpParms.dhcpv4Enabled : ipv4Active;

Expand Down Expand Up @@ -1489,6 +1505,7 @@ inline std::vector<IPv6AddressData>::const_iterator getNextStaticIpEntry(
inline void handleIPv4StaticPatch(
const std::string& ifaceId,
std::vector<std::variant<nlohmann::json::object_t, std::nullptr_t>>& input,
const EthernetInterfaceData& ethData,
const std::vector<IPv4AddressData>& ipv4Data,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp)
{
Expand All @@ -1500,14 +1517,52 @@ inline void handleIPv4StaticPatch(
std::vector<IPv4AddressData>::const_iterator nicIpEntry =
getNextStaticIpEntry(ipv4Data.cbegin(), ipv4Data.cend());

bool gatewayValueAssigned{};
std::string activePath{};
std::string activeGateway{};
if (!ethData.defaultGateway.empty() && ethData.defaultGateway != "0.0.0.0")
{
// The NIC is already configured with a default gateway. Use this if
// the leading entry in the PATCH is '{}', which is preserving an active
// static address.
activeGateway = ethData.defaultGateway;
activePath = "IPv4StaticAddresses/1";
gatewayValueAssigned = true;
}

for (std::variant<nlohmann::json::object_t, std::nullptr_t>& thisJson :
input)
{
std::string pathString = "IPv4StaticAddresses/" +
std::to_string(entryIdx);
nlohmann::json::object_t* obj =
std::get_if<nlohmann::json::object_t>(&thisJson);
if (obj != nullptr && !obj->empty())
if (obj == nullptr)
{
if (nicIpEntry != ipv4Data.cend())
{
deleteIPAddress(ifaceId, nicIpEntry->id, asyncResp);
nicIpEntry = getNextStaticIpEntry(++nicIpEntry,
ipv4Data.cend());
if (!gatewayValueAssigned && (nicIpEntry == ipv4Data.cend()))
{
// All entries have been processed, and this last has
// requested the IP address be deleted. No prior entry
// performed an action that created or modified a
// gateway. Deleting this IP address means the default
// gateway entry has to be removed as well.
updateIPv4DefaultGateway(ifaceId, "", asyncResp);
}
entryIdx++;
continue;
}
// Received a DELETE action on an entry not assigned to the NIC
messages::resourceCannotBeDeleted(asyncResp->res);
return;
}

// An Add/Modify action is requested
if (!obj->empty())
{
std::optional<std::string> address;
std::optional<std::string> subnetMask;
Expand Down Expand Up @@ -1596,6 +1651,29 @@ inline void handleIPv4StaticPatch(
return;
}

if (gatewayValueAssigned)
{
if (activeGateway != gateway)
{
// A NIC can only have a single active gateway value.
// If any gateway in the array of static addresses
// mismatch the PATCH is in error.
std::string arg1 = pathString + "/Gateway";
std::string arg2 = activePath + "/Gateway";
messages::propertyValueConflict(asyncResp->res, arg1, arg2);
return;
}
}
else
{
// Capture the very first gateway value from the incoming
// JSON record and use it at the default gateway.
updateIPv4DefaultGateway(ifaceId, *gateway, asyncResp);
activeGateway = *gateway;
activePath = pathString;
gatewayValueAssigned = true;
}

if (nicIpEntry != ipv4Data.cend())
{
deleteAndCreateIPAddress(IpVersion::IpV4, ifaceId,
Expand All @@ -1613,31 +1691,21 @@ inline void handleIPv4StaticPatch(
}
else
{
if (nicIpEntry == ipv4Data.cend())
{
// Requesting a DELETE/DO NOT MODIFY action for an item
// that isn't present on the eth(n) interface. Input JSON is
// in error, so bail out.
if (obj == nullptr)
{
messages::resourceCannotBeDeleted(asyncResp->res);
return;
}
messages::propertyValueFormatError(asyncResp->res, *obj,
pathString);
return;
}

if (obj == nullptr)
{
deleteIPAddress(ifaceId, nicIpEntry->id, asyncResp);
}
// Received {}, do not modify this address
if (nicIpEntry != ipv4Data.cend())
{
nicIpEntry = getNextStaticIpEntry(++nicIpEntry,
ipv4Data.cend());
entryIdx++;
}
else
{
// Requested a DO NOT MODIFY action on an entry not assigned
// to the NIC
messages::propertyValueFormatError(asyncResp->res, *obj,
pathString);
return;
}
entryIdx++;
}
}
}
Expand Down Expand Up @@ -2268,8 +2336,8 @@ inline void requestEthernetInterfacesRoutes(App& app)

if (ipv4StaticAddresses)
{
handleIPv4StaticPatch(ifaceId, *ipv4StaticAddresses, ipv4Data,
asyncResp);
handleIPv4StaticPatch(ifaceId, *ipv4StaticAddresses, ethData,
ipv4Data, asyncResp);
}

if (staticNameServers)
Expand Down

0 comments on commit 743eb1c

Please sign in to comment.