confd: replace phys-address deviation with new setting

This commit replaces the ietf-interfaces deviation for phys-address,
used to set custom MAC address on interfaces, with a more flexible
approach which can calculate the new MAC address based on the device's
chassi MAC, with or without an added offset.

The regression test has been updated to test all variants.

Resolves: #680

Signed-off-by: Joachim Wiberg <[email protected]>

src/confd/src/ietf-interfaces.c

@@ -431,15 +431,110 @@ static int netdag_gen_link_mtu(FILE *ip, struct lyd_node *dif)
 	return 0;
 }
 
+static void calc_mac(const char *base_mac, const char *mac_offset, char *buf, size_t len)
+{
+	uint8_t base[6], offset[6], result[6];
+	int carry = 0, i;
+
+	sscanf(base_mac, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx",
+	       &base[0], &base[1], &base[2], &base[3], &base[4], &base[5]);
+
+	sscanf(mac_offset, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx",
+	       &offset[0], &offset[1], &offset[2], &offset[3], &offset[4], &offset[5]);
+
+	for (i = 5; i >= 0; i--) {
+		int sum = base[i] + offset[i] + carry;
+
+		result[i] = sum & 0xFF;
+		carry = (sum > 0xFF) ? 1 : 0;
+	}
+
+	snprintf(buf, len, "%02x:%02x:%02x:%02x:%02x:%02x",
+		 result[0], result[1], result[2], result[3], result[4], result[5]);
+}
+
+/*
+ * Get child value from a diff parent, only returns value if not
+ * deleted.  In which case the deleted flag may be set.
+ */
+static const char *get_val(struct lyd_node *parent, char *name, int *deleted)
+{
+	const char *value = NULL;
+	struct lyd_node *node;
+
+	node = lydx_get_child(parent, name);
+	if (node) {
+		if (lydx_get_op(node) == LYDX_OP_DELETE) {
+			if (deleted)
+				*deleted = 1;
+			return NULL;
+		}
+
+		value = lyd_get_value(node);
+	}
+
+	return value;
+}
+
+/*
+ * Locate custom-phys-address, adjust for any offset, and return pointer
+ * to a static string.  (Which will be overwritten on subsequent calls.)
+ *
+ * The 'deleted' flag will be set if any of the nodes in the subtree are
+ * deleted.  Used when restoring permaddr and similar.
+ */
+static char *get_phys_addr(struct lyd_node *parent, int *deleted)
+{
+	struct lyd_node *node, *cpa;
+	static char mac[18];
+	struct json_t *j;
+	const char *ptr;
+
+	cpa = lydx_get_descendant(lyd_child(parent), "custom-phys-address", NULL);
+	if (!cpa || lydx_get_op(cpa) == LYDX_OP_DELETE) {
+		if (cpa && deleted)
+			*deleted = 1;
+		return NULL;
+	}
+
+	ptr = get_val(cpa, "static", deleted);
+	if (ptr) {
+		strlcpy(mac, ptr, sizeof(mac));
+		return mac;
+	}
+
+	node = lydx_get_child(cpa, "chassis");
+	if (!node || lydx_get_op(node) == LYDX_OP_DELETE) {
+		if (node && deleted)
+			*deleted = 1;
+		return NULL;
+	}
+
+	j = json_object_get(confd.root, "mac-address");
+	if (!j) {
+		WARN("cannot set chassis based MAC, not found.");
+		return NULL;
+	}
+
+	ptr = json_string_value(j);
+	strlcpy(mac, ptr, sizeof(mac));
+
+	ptr = get_val(node, "offset", deleted);
+	if (ptr)
+		calc_mac(mac, ptr, mac, sizeof(mac));
+
+	return mac;
+}
+
 static int netdag_gen_link_addr(FILE *ip, struct lyd_node *cif, struct lyd_node *dif)
 {
 	const char *ifname = lydx_get_cattr(dif, "name");
-	const char *mac = NULL;
-	struct lyd_node *node;
+	const char *mac;
+	int deleted = 0;
 	char buf[32];
 
-	node = lydx_get_child(dif, "phys-address");
-	if (lydx_get_op(node) == LYDX_OP_DELETE) {
+	mac = get_phys_addr(dif, &deleted);
+	if (!mac && deleted) {
 		FILE *fp;
 
 		/*
@@ -455,8 +550,6 @@ static int netdag_gen_link_addr(FILE *ip, struct lyd_node *cif, struct lyd_node
 			if (mac && !strcmp(mac, "null"))
 				return 0;
 		}
-	} else {
-		mac = lyd_get_value(node);
 	}
 
 	if (!mac || !strlen(mac)) {
@@ -1344,9 +1437,8 @@ static int netdag_gen_bridge(sr_session_ctx_t *session, struct dagger *net, stru
 	 * addrgenmode eui64 with random mac, issue #357.
 	 */
 	if (add) {
-		const char *mac;
+		const char *mac = get_phys_addr(cif, NULL);
 
-		mac = lydx_get_cattr(cif, "phys-address");
 		if (!mac) {
 			struct json_t *j;
 
@@ -1449,16 +1541,17 @@ static int netdag_gen_veth(struct dagger *net, struct lyd_node *dif,
 			return ERR_IFACE(cif, err, "Unable to add dep \"%s\" to %s", peer, ifname);
 	} else {
 		char ifname_args[64] = "", peer_args[64] = "";
+		const char *mac;
 
 		dagger_skip_iface(net, peer);
 
-		node = lydx_get_child(dif, "phys-address");
-		if (node)
-			snprintf(ifname_args, sizeof(ifname_args), "address %s", lyd_get_value(node));
+		mac = get_phys_addr(dif, NULL);
+		if (mac)
+			snprintf(ifname_args, sizeof(ifname_args), "address %s", mac);
 
 		node = lydx_find_by_name(lyd_parent(cif), "interface", peer);
-		if (node && (node = lydx_get_child(node, "phys-address")))
-			snprintf(peer_args, sizeof(peer_args), "address %s", lyd_get_value(node));
+		if (node && (mac = get_phys_addr(node, NULL)))
+			snprintf(peer_args, sizeof(peer_args), "address %s", mac);
 
 		fprintf(ip, "link add dev %s %s type veth peer %s %s\n",
 			ifname, ifname_args, peer, peer_args);
@@ -1612,10 +1705,9 @@ static int netdag_gen_afspec_set(sr_session_ctx_t *session, struct dagger *net,
 
 static bool is_phys_addr_deleted(struct lyd_node *dif)
 {
-	struct lyd_node *node;
+	int deleted = 0;
 
-	node = lydx_get_child(dif, "phys-address");
-	if (node && lydx_get_op(node) == LYDX_OP_DELETE)
+	if (!get_phys_addr(dif, &deleted) && deleted)
 		return true;
 
 	return false;

src/confd/yang/confd.inc

@@ -37,7 +37,7 @@ MODULES=(
 	"[email protected]"
 	"[email protected]"
 	"[email protected]"
-	"infix-interfaces@2024-09-23.yang -e vlan-filtering"
+	"infix-interfaces@2024-10-08.yang -e vlan-filtering"
 
 	# from rousette
 	"[email protected]"

src/confd/yang/infix-interfaces.yang

@@ -9,6 +9,9 @@ module infix-interfaces {
   import ietf-interfaces {
     prefix if;
   }
+  import ietf-yang-types {
+    prefix yang;
+  }
 
   include infix-if-base;
   include infix-if-bridge;
@@ -20,6 +23,11 @@ module infix-interfaces {
   contact      "[email protected]";
   description  "Linux bridge and lag extensions for ietf-interfaces.";
 
+  revision 2024-10-08 {
+    description "Replace writable phy-address with custom-phys-address.";
+    reference "internal";
+  }
+
   revision 2024-09-23 {
     description "Drop interfaces-state deviation, already marked deprecated.";
     reference "internal";
@@ -71,9 +79,34 @@ module infix-interfaces {
     }
   }
 
-  deviation "/if:interfaces/if:interface/if:phys-address" {
-    deviate replace {
-      config true;
+  augment "/if:interfaces/if:interface" {
+    description "Custom phys-address management, static or derived from chassis MAC.";
+
+    container custom-phys-address {
+      description "Container for custom physical address options";
+
+      choice type {
+        description "Choose between static MAC address or chassis-derived MAC";
+
+        case static {
+          leaf static {
+            type yang:phys-address;
+            description "Statically configured interface address on protocol sub-layer, e.g., MAC.";
+          }
+        }
+
+        case chassis {
+          container chassis {
+            presence "Enable chassis-derived address options";
+            description "Container for options when using chassis-derived address.";
+
+            leaf offset {
+              type yang:phys-address;
+              description "Address offset to add to the chassis-derived MAC.";
+            }
+          }
+        }
+      }
     }
   }
 }

test/case/ietf_interfaces/iface_phys_address/test.py

@@ -2,45 +2,118 @@
 """
 Custom MAC address on interface
 
-Test possibility to set and remove custom mac address on interfaces
+Verify support for setting and removing a custom MAC address on interfaces.
+Both static MAC address and derived from the chassis MAC with, or without,
+an offset applied.
 """
-import copy
+
 import infamy
 import infamy.iface as iface
-
 from infamy.util import until
 
+
+def calc_mac(base_mac, mac_offset):
+    """Add mac_offset to base_mac and return result."""
+    base = [int(x, 16) for x in base_mac.split(':')]
+    offset = [int(x, 16) for x in mac_offset.split(':')]
+    result = [0] * 6
+    carry = 0
+
+    for i in range(5, -1, -1):
+        total = base[i] + offset[i] + carry
+        result[i] = total & 0xFF
+        carry = 1 if total > 0xFF else 0
+
+    return ':'.join(f'{x:02x}' for x in result)
+
+
+def reset_mac(tgt, port, mac):
+    """Reset DUT interface MAC address to default."""
+    node = "infix-interfaces:custom-phys-address"
+    xpath = iface.get_iface_xpath(port, node)
+    tgt.delete_xpath(xpath)
+    with test.step("Verify target:data MAC address is reset to default"):
+        until(lambda: iface.get_phys_address(tgt, tport) == mac)
+
+
 with infamy.Test() as test:
+    CMD = "jq -r '.[\"mac-address\"]' /run/system.json"
+
     with test.step("Initialize"):
         env = infamy.Env()
         target = env.attach("target", "mgmt")
+        tgtssh = env.attach("target", "mgmt", "ssh")
         _, tport = env.ltop.xlate("target", "data")
         pmac = iface.get_phys_address(target, tport)
-        cmac = "02:01:00:c0:ff:ee"
-        print(f"Target iface {tport} original mac {pmac}")
+        cmac = tgtssh.runsh(CMD).stdout.strip()
+        STATIC = "02:01:00:c0:ff:ee"
+        OFFSET = "00:00:00:00:ff:aa"
+
+        print(f"Chassis MAC address target: {cmac}")
+        print(f"Default MAC address of {tport} : {pmac}")
 
-    with test.step("Set custom MAC address to '02:01:00:c0:ff:ee' on target:mgmt"):
-        print(f"Intitial MAC address: {pmac}")
+    with test.step("Set target:data static MAC address '02:01:00:c0:ff:ee'"):
         config = {
             "interfaces": {
                 "interface": [{
                     "name": f"{tport}",
-                    "phys-address": f"{cmac}"
+                    "custom-phys-address": {
+                        "static": f"{STATIC}"
+                    }
                 }]
             }
         }
         target.put_config_dict("ietf-interfaces", config)
 
     with test.step("Verify target:mgmt has MAC address '02:01:00:c0:ff:ee'"):
         mac = iface.get_phys_address(target, tport)
-        print(f"Target iface {tport} current mac: {mac}")
+        print(f"Current MAC: {mac}, should be: {STATIC}")
+        assert mac == STATIC
+
+    with test.step("Reset target:mgmt MAC address to default"):
+        reset_mac(target, tport, pmac)
+
+    with test.step("Set target:data to chassis MAC"):
+        config = {
+            "interfaces": {
+                "interface": [{
+                    "name": f"{tport}",
+                    "custom-phys-address": {
+                        "chassis": {}
+                    }
+                }]
+            }
+        }
+        target.put_config_dict("ietf-interfaces", config)
+
+    with test.step("Verify target:data has chassis MAC"):
+        mac = iface.get_phys_address(target, tport)
+        print(f"Current MAC: {mac}, should be: {cmac}")
         assert mac == cmac
 
-    with test.step(f"Remove custom MAC address '02:01:00:c0:ff:ee'"):
-        xpath=iface.get_iface_xpath(tport, "phys-address")
-        target.delete_xpath(xpath)
+    with test.step("Set target:data to chassis MAC + offset"):
+        print(f"Setting chassis MAC {cmac} + offset {OFFSET}")
+        config = {
+            "interfaces": {
+                "interface": [{
+                    "name": f"{tport}",
+                    "custom-phys-address": {
+                        "chassis": {
+                            "offset": f"{OFFSET}"
+                        }
+                    }
+                }]
+            }
+        }
+        target.put_config_dict("ietf-interfaces", config)
+
+    with test.step("Verify target:data has chassis MAC + offset"):
+        mac = iface.get_phys_address(target, tport)
+        BMAC = calc_mac(cmac, OFFSET)
+        print(f"Current MAC: {mac}, should be: {BMAC} (calculated)")
+        assert mac == BMAC
 
-    with test.step("Verify that target:mgmt has its origil MAC address again"):
-        until(lambda: iface.get_phys_address(target, tport) == pmac)
+    with test.step("Reset target:mgmt MAC address to default"):
+        reset_mac(target, tport, pmac)
 
     test.succeed()