Delete IPv4 default gateway when deleting an IPv4 static address
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 <johnathanx.mantey@intel.com>
diff --git a/redfish-core/lib/ethernet.hpp b/redfish-core/lib/ethernet.hpp
index 876bc2e..533c7f5 100644
--- a/redfish-core/lib/ethernet.hpp
+++ b/redfish-core/lib/ethernet.hpp
@@ -47,6 +47,12 @@
Global
};
+enum class IpVersion
+{
+ IpV4,
+ IpV6
+};
+
/**
* Structure for keeping IPv4 data required by Redfish
*/
@@ -700,7 +706,30 @@
}
/**
- * @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
@@ -724,17 +753,6 @@
"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
*
@@ -757,7 +775,6 @@
messages::internalError(asyncResp->res);
return;
}
- updateIPv4DefaultGateway(ifaceId, gateway, asyncResp);
};
crow::connections::systemBus->async_method_call(
@@ -769,24 +786,19 @@
}
/**
- * @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,
@@ -1391,6 +1403,10 @@
bool ipv4Active = translateDhcpEnabledToBool(ethData.dhcpEnabled, true);
bool ipv6Active = translateDhcpEnabledToBool(ethData.dhcpEnabled, false);
+ if (ipv4Active)
+ {
+ updateIPv4DefaultGateway(ifaceId, "", asyncResp);
+ }
bool nextv4DHCPState =
v4dhcpParms.dhcpv4Enabled ? *v4dhcpParms.dhcpv4Enabled : ipv4Active;
@@ -1489,6 +1505,7 @@
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)
{
@@ -1500,6 +1517,19 @@
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)
{
@@ -1507,7 +1537,32 @@
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;
@@ -1596,6 +1651,29 @@
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,
@@ -1613,31 +1691,21 @@
}
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++;
}
- entryIdx++;
+ else
+ {
+ // Requested a DO NOT MODIFY action on an entry not assigned
+ // to the NIC
+ messages::propertyValueFormatError(asyncResp->res, *obj,
+ pathString);
+ return;
+ }
}
}
}
@@ -2268,8 +2336,8 @@
if (ipv4StaticAddresses)
{
- handleIPv4StaticPatch(ifaceId, *ipv4StaticAddresses, ipv4Data,
- asyncResp);
+ handleIPv4StaticPatch(ifaceId, *ipv4StaticAddresses, ethData,
+ ipv4Data, asyncResp);
}
if (staticNameServers)