Merge pull request #761 from kernelkit/misc-interface-dhcp

Misc interface dhcp Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
2026-07-30 04:33:00 +02:00 · 2024-10-28 12:38:25 +01:00
parent 55a20bb4c7 3e5e89b328
commit 00cdff1d2d
21 changed files with 255 additions and 51 deletions
@@ -1,5 +1,5 @@
-Contributing to Infix
-=====================
+Contributing Guidelines
+=======================

 Thank :heart: you for taking the time to read this!

@@ -12,6 +12,7 @@ If you are unsure how to start implementing an idea or fix:
 - :bug: open an issue, there are human friendly templates for _bugs_
   and _feature requests_ at <https://github.com/kernelkit/infix/issues>
 - :speech_balloon: use the [Q&A Forum][discuss]
+ - :technologist: The [Developer's Guide][devguide] is also a useful start

 > _Talking about code and problems first is often the best way to get
 > started before submitting a pull request.  We have found it always
@@ -16,13 +16,22 @@ die()
 #      DISK_IMAGE_SIZE="512"
 #      etc.
 #
+# Nested variables, like INFIX_COMPATIBLE="${INFIX_IMAGE_ID}"
+# are handled by sourcing the file in a subshell.
+#
 # shellcheck disable=SC1090
 load_cfg()
 {
    tmp=$(mktemp -p /tmp)
+    (
+	. "$BR2_CONFIG" 2>/dev/null
+
+	# Set *all* matching variables
+	set | grep -E "^${1}[^=]*=" | while IFS= read -r line; do
+            echo "$line"
+	done
+    ) > "$tmp"

-    grep -E "${1}.*=" "$BR2_CONFIG" >"$tmp"
    .  "$tmp"
-
    rm "$tmp"
 }
@@ -16,6 +16,11 @@ All notable changes to the project are documented in this file.
 [v24.10.1][] - 2024-10-18
 -------------------------

+### Changes
+ - Add support for interface description, sometimes referred to as
+   "ifAlias".  Saved as an Linux interface alias (not `altname`), e.g.,
+   `/sys/class/interfaces/veth0a/ifalias`, includes operational support
+
 ### Fixes
 - Fix #735: `copy` and `erase` commands missing from CLI, regression
   in Infix v24.10.0 defconfigs, now added as dep. in klish package
@@ -206,20 +206,18 @@ corresponding image for execution with our normal tooling:
 Contributing
 ------------

-Infix is built from many parts, when contributing you need to set up
-your own fork, create a local branch for your change, push to your fork,
-and then use GitHub to create a *Pull Reqeuest*.
+Infix is built from many components, when contributing you need to set
+up your own fork, create a local branch for your change, push to your
+fork, and then use GitHub to create a *Pull Reqeuest*.

-For this to work as painlessly as possible:
+For this to work as *painlessly as possible* for everyone involved:

-  1. Fork Infix to your own user or organization[^1]
-  2. Fork all the Infix submodules, e.g., `kernelkit/buildroot` to your
-     own user or organization as well
-  3. Clone your fork of Infix to your laptop/workstation
-
-If you use a GitHub organization you get the added benefit of having
-local peer reviews of changes before making a pull request to the
-upstream Infix repository.
+ 1. Fork Infix to your own user or organization[^1]
+ 2. Fork all the Infix submodules, e.g., `kernelkit/buildroot` to your
+    own user or organization as well
+ 3. Clone your fork of Infix to your laptop/workstation
+ 4. [Deactivate the Actions][6] you don't want in your fork
+ 5. Please read the [Contributing Guidelines][5] as well!

 ```bash
 $ cd ~/Projects
@@ -227,10 +225,10 @@ $ git clone https://github.com/YOUR_USER_NAME/infix.git
 $ cd infix/
 $ git submodule update --init
 ```
-
 > **Note:** when updating/synchronizing with upstream Infix changes you
 > may have to synchronize your forks as well.  GitHub have a `Sync fork`
-> button in the GUI for your fork for this purpose.
+> button in the GUI for your fork for this purpose.  A cronjob on your
+> server of choice can do this for you with the [GitHub CLI tool][7].

 [^1]: Organizations should make sure to lock the `main` (or `master`)
    branch of their clones to ensure members do not accidentally merge
@@ -243,3 +241,6 @@ $ git submodule update --init
 [2]: https://github.com/wkz/qeneth
 [3]: https://netopeer.liberouter.org/doc/sysrepo/master/html/dev_guide.html
 [4]: https://buildroot.org/downloads/manual/manual.html#_developer_guide
+[5]: https://github.com/kernelkit/infix/blob/main/.github/CONTRIBUTING.md
+[6]: https://docs.github.com/en/actions/managing-workflow-runs-and-deployments/managing-workflow-runs/disabling-and-enabling-a-workflow
+[7]: https://cli.github.com/
@@ -97,6 +97,10 @@ This is an extension to the ietf-interfaces YANG model, which defines
 `phys-address` as read-only[^4].  The following shows the different
 configuration options.

+The `description` is saved as Linux `ifalias` on an interface.  It is a
+free-form string, useful for describing purpose or just adding comments
+for remote debugging, e.g., using the operational datastore.
+
 > **Note:** there is no validation or safety checks performed by the
 > system when using `custom-phys-address`.  In particular the `offset`
 > variant can be dangerous to use -- pay attention to the meaning of
@@ -585,6 +589,9 @@ Multiple address assignment methods are available:
 | link-local | infix-ip          | Auto-assignment of IPv4 address in 169.254.x.x/16 range        |
 | dhcp       | infix-dhcp-client | Assignment of IPv4 address by DHCP server, e.g., *10.0.1.1/24* |

+> **Note:** DHCP address method is only available for *LAN* interfaces
+> (Ethernet, virtual Ethernet (veth), bridge, link aggregates, etc.)
+
 Supported DHCP (request) options, configurability (Cfg) and defaults,
 are listed below.  Configurable options can be disabled on a per client
 interface basis, some options, like `clientid` and option 81, are
@@ -616,8 +623,10 @@ client is not enabled, any NTP servers provided by the DHCP server will
 be ignored. For details on how to enable the NTP client, see the 
 [NTP Client Configuration](system.md#ntp-client-configuration) section.

-> **Note:** DHCP address method is only available for *LAN* interfaces
-> (Ethernet, virtual Ethernet (veth), bridge, link aggregates, etc.)
+> **Note:** as per [RFC3442][4], if the DHCP server returns both a
+> Classless Static Routes option (121) and Router option (3), the
+> DHCP client *must* ignore the latter.
+

 ### IPv6 Address Assignment

@@ -1059,7 +1068,7 @@ This CLI example show the IPv6 routing table.

 #### Route Preference

-The operating system leverages FRRouting ([Frr][4]) as routing engine
+The operating system leverages FRRouting ([Frr][0]) as routing engine
 for both static and dynamic routing.  Even routes injected from a DHCP
 client, and IPv4 link-local (IPv4) routes, are injected into Frr to let
 it weigh all routes before installing them into the kernel routing table
@@ -1113,7 +1122,8 @@ currently supported, namely `ipv4` and `ipv6`.
 [1]: https://www.rfc-editor.org/rfc/rfc8343
 [2]: https://www.rfc-editor.org/rfc/rfc8344
 [3]: https://www.rfc-editor.org/rfc/rfc8981
-[4]: https://frrouting.org/
+[4]: https://www.rfc-editor.org/rfc/rfc3442
+[0]: https://frrouting.org/

 [^1]: Please note, link aggregates are not yet supported in Infix.
 [^2]: Link-local IPv6 addresses are implicitly enabled when enabling
@@ -1,7 +1,7 @@
 From 46ffa81f5c88ce95db011369d8bfb802313e4217 Mon Sep 17 00:00:00 2001
 From: Joachim Wiberg <troglobit@gmail.com>
 Date: Thu, 17 Oct 2024 14:23:24 +0200
-Subject: [PATCH] Only mark rdeps dirty if main service is nohup
+Subject: [PATCH 1/2] Only mark rdeps dirty if main service is nohup
 Organization: Addiva Elektronik

 This patch changes a behavior that's been default since Finit 4.0,
@@ -0,0 +1,33 @@
+From 119e66a7e9c95283918639b51dd03a3d666955f8 Mon Sep 17 00:00:00 2001
+From: Joachim Wiberg <troglobit@gmail.com>
+Date: Mon, 28 Oct 2024 10:58:04 +0100
+Subject: [PATCH 2/2] Reset color attributes and clear screen when starting up
+Organization: Addiva Elektronik
+
+Some boot loaders, like GRUB, leave background color artifacts from
+their boot menu.  This patch resets the foreground and background
+color attributes, and then clears the screen, without clearing the
+scrollback buffer.
+
+Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
+---
+ src/helpers.c | 3 +++
+ 1 file changed, 3 insertions(+)
+
+diff --git a/src/helpers.c b/src/helpers.c
+index 8768de8..99c4557 100644
+--- a/src/helpers.c
+++ b/src/helpers.c
+@@ -87,6 +87,9 @@ void console_init(void)
+ 	/* Enable line wrap, if disabled previously, e.g., qemu */
+ 	dprint(STDOUT_FILENO, "\033[?7h", 5);
+ 
+	/* Reset atttributes, background and foreground color  */
+	dprint(STDOUT_FILENO, "\033[49m\033[39m\e[2J", 14);
+
+ 	log_init();
+ }
+ 
+-- 
+2.43.0
+
@@ -1889,6 +1889,10 @@ static sr_error_t netdag_gen_iface(sr_session_ctx_t *session, struct dagger *net
 	if (err)
 		goto err_close_ip;

+	/* ifAlias, should skip for container-network types */
+	attr = lydx_get_cattr(cif, "description");
+	fprintf(ip, "link set alias \"%s\" dev %s\n", attr ?: "", ifname);
+
 	/* Bring interface back up, if enabled */
 	attr = lydx_get_cattr(cif, "enabled");
 	if (!attr || !strcmp(attr, "true"))
@@ -37,7 +37,7 @@ MODULES=(
 	"ieee802-ethernet-interface@2019-06-21.yang"
 	"infix-ethernet-interface@2024-02-27.yang"
 	"infix-factory-default@2023-06-28.yang"
-	"infix-interfaces@2024-10-14.yang -e vlan-filtering"
+	"infix-interfaces@2024-10-28.yang -e vlan-filtering"

 	# from rousette
 	"ietf-restconf@2017-01-26.yang"
@@ -1,6 +1,6 @@
 # -*- sh -*-
 # REMEMBER TO UPDATE infix-interfaces ALSO IN confd.inc
 MODULES=(
-    "infix-interfaces@2024-10-14.yang -e vlan-filtering -e containers"
+    "infix-interfaces@2024-10-28.yang -e vlan-filtering -e containers"
    "infix-containers@2024-10-14.yang"
 )
@@ -23,6 +23,11 @@ module infix-interfaces {
  contact      "kernelkit@googlegroups.com";
  description  "Linux bridge and lag extensions for ietf-interfaces.";

+  revision 2024-10-28 {
+    description "Limit description to 64 chars, matching IF-MIB max.";
+    reference "internal";
+  }
+
  revision 2024-10-14 {
    description "Deviate link-up-down-trap-enable not-supported.";
    reference "internal";
@@ -84,6 +89,18 @@ module infix-interfaces {
    }
  }

+  deviation "/if:interfaces/if:interface/if:description" {
+    deviate replace {
+      type string {
+        length "0..64";
+      }
+    }
+  }
+
+  deviation "/if:interfaces/if:interface/if:link-up-down-trap-enable" {
+    deviate not-supported;
+  }
+
  augment "/if:interfaces/if:interface" {
    description "Custom phys-address management, static or derived from chassis MAC.";

@@ -114,7 +131,4 @@ module infix-interfaces {
      }
    }
  }
-  deviation "/if:interfaces/if:interface/if:link-up-down-trap-enable" {
-    deviate not-supported;
-  }
 }
@@ -645,6 +645,9 @@ def add_ip_link(ifname, iface_in, iface_out):
    if 'ifindex' in iface_in:
        iface_out['if-index'] = iface_in['ifindex']

+    if 'ifalias' in iface_in:
+        iface_out['description'] = iface_in['ifalias']
+
    if 'address' in iface_in:
        iface_out['phys-address'] = iface_in['address']

@@ -11,6 +11,8 @@ include::ipv6_address/Readme.adoc[]

 include::ipv4_autoconf/Readme.adoc[]

+include::ifalias/Readme.adoc[]
+
 include::iface_phys_address/Readme.adoc[]

 include::routing_basic/Readme.adoc[]
@@ -8,6 +8,9 @@
 - name: ipv6_address
  case: ipv6_address/test.py

+- name: ifalias
+  case: ifalias/test.py
+
 - name: iface_phys_address
  case: iface_phys_address/test.py

@@ -0,0 +1,25 @@
+=== Interface Description (ifAlias)
+==== Description
+Verify interface description (ifAlias) can be set on an interface and
+then be read back from the operational datastore.
+
+==== Topology
+ifdef::topdoc[]
+image::../../test/case/ietf_interfaces/ifalias/topology.svg[Interface Description (ifAlias) topology]
+endif::topdoc[]
+ifndef::topdoc[]
+ifdef::testgroup[]
+image::ifalias/topology.svg[Interface Description (ifAlias) topology]
+endif::testgroup[]
+ifndef::testgroup[]
+image::topology.svg[Interface Description (ifAlias) topology]
+endif::testgroup[]
+endif::topdoc[]
+==== Test sequence
+. Set up topology and attach to target DUT
+. Set up interface target:data with description
+. Verify description can be read back from operational
+
+
+<<<
+
@@ -0,0 +1,36 @@
+#!/usr/bin/env python3
+"""Interface Description (ifAlias)
+
+Verify interface description (ifAlias) can be set on an interface and
+then be read back from the operational datastore.
+"""
+import infamy
+import infamy.iface as iface
+
+with infamy.Test() as test:
+    with test.step("Set up topology and attach to target DUT"):
+        env = infamy.Env()
+        target = env.attach("target", "mgmt")
+        DESC = "Kilroy was here"
+
+    with test.step("Set up interface target:data with description"):
+        _, tport = env.ltop.xlate("target", "data")
+
+        target.put_config_dict("ietf-interfaces", {
+            "interfaces": {
+                "interface": [
+                    {
+                        "name": tport,
+                        "description": DESC,
+                        "enabled": True,
+                    }
+                ]
+            }
+        })
+
+    with test.step("Verify description can be read back from operational"):
+        text = iface.get_param(target, tport, "description")
+        if text != DESC:
+            test.fail()
+
+    test.succeed()
@@ -0,0 +1 @@
+../../../infamy/topologies/1x2.dot
@@ -0,0 +1,42 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN"
+ "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
+
+<!-- Title: 1x2 Pages: 1 -->
+<svg width="424pt" height="55pt"
+ viewBox="0.00 0.00 424.03 55.00" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+<g id="graph0" class="graph" transform="scale(1 1) rotate(0) translate(4 51)">
+<title>1x2</title>
+<polygon fill="white" stroke="transparent" points="-4,4 -4,-51 420.03,-51 420.03,4 -4,4"/>
+<!-- host -->
+<g id="node1" class="node">
+<title>host</title>
+<polygon fill="none" stroke="black" points="0,-0.5 0,-46.5 100,-46.5 100,-0.5 0,-0.5"/>
+<text text-anchor="middle" x="25" y="-19.8" font-family="DejaVu Sans Mono, Book" font-size="14.00">host</text>
+<polyline fill="none" stroke="black" points="50,-0.5 50,-46.5 "/>
+<text text-anchor="middle" x="75" y="-31.3" font-family="DejaVu Sans Mono, Book" font-size="14.00">mgmt</text>
+<polyline fill="none" stroke="black" points="50,-23.5 100,-23.5 "/>
+<text text-anchor="middle" x="75" y="-8.3" font-family="DejaVu Sans Mono, Book" font-size="14.00">data</text>
+</g>
+<!-- target -->
+<g id="node2" class="node">
+<title>target</title>
+<polygon fill="none" stroke="black" points="300.03,-0.5 300.03,-46.5 416.03,-46.5 416.03,-0.5 300.03,-0.5"/>
+<text text-anchor="middle" x="325.03" y="-31.3" font-family="DejaVu Sans Mono, Book" font-size="14.00">mgmt</text>
+<polyline fill="none" stroke="black" points="300.03,-23.5 350.03,-23.5 "/>
+<text text-anchor="middle" x="325.03" y="-8.3" font-family="DejaVu Sans Mono, Book" font-size="14.00">data</text>
+<polyline fill="none" stroke="black" points="350.03,-0.5 350.03,-46.5 "/>
+<text text-anchor="middle" x="383.03" y="-19.8" font-family="DejaVu Sans Mono, Book" font-size="14.00">target</text>
+</g>
+<!-- host&#45;&#45;target -->
+<g id="edge1" class="edge">
+<title>host:mgmt&#45;&#45;target:mgmt</title>
+<path fill="none" stroke="lightgrey" stroke-width="2" d="M100,-35.5C100,-35.5 300.03,-35.5 300.03,-35.5"/>
+</g>
+<!-- host&#45;&#45;target -->
+<g id="edge2" class="edge">
+<title>host:data&#45;&#45;target:data</title>
+<path fill="none" stroke="black" stroke-width="2" d="M100,-11.5C100,-11.5 300.03,-11.5 300.03,-11.5"/>
+</g>
+</g>
+</svg>
@@ -1,12 +1,18 @@
 === DHCP option 121 vs option 3
 ==== Description
-Verify DHCP option 121 (static routes) is used over option 3 and that the
-routes exist in the operational datastore.
+Verify that DHCP option 121 (classless static routes) is used over the
+older option 3 (default gateway) in the DHCP client, when both are sent
+by the server, see RFC3442.  Use the routing RIB in the operational
+datastore to verify.

-Installing unrelated routes from a DHCP server should not affect already
-existing routes. To verify this, a canary route is set up in the client
-before initiating DHCP. This canary route does not need to be reachable
-before a DHCP lease has been acquired.
+Installing routes from a DHCP server should not affect already existing
+(static) routes.  To verify this, a static canary route (20.0.0.0/24 via
+192.168.0.2) is installed before starting the DHCP client.  As a twist,
+this canary route has a next-hop (192.168.0.2) which is not reachable
+until a DHCP lease has been acquired.
+
+The DHCP server is set up to hand out both a default router (option 3)
+via 192.168.0.1 and a default route (option 121) via 192.168.0.254.

 ==== Topology
 ifdef::topdoc[]
@@ -21,10 +27,11 @@ image::topology.svg[DHCP option 121 vs option 3 topology]
 endif::testgroup[]
 endif::topdoc[]
 ==== Test sequence
-. Setting up client
-. Verify 'client' has a route to 10.0.0.0/24 via 192.168.0.254
-. Verify 'client' has a default route via 192.168.0.254
-. Verify 'client' has a route to 20.0.0.0/24 via 192.168.0.2
+. Setting up canary route, 20.0.0.0/24 via 192.168.0.2
+. Enabling DHCP client, allow option 3 and 121
+. Verify 'client' has a route 10.0.0.0/24 via 192.168.0.254 (option 121)
+. Verify 'client' has default route via 192.168.0.254 (not use option 3)
+. Verify 'client' still has canary route to 20.0.0.0/24 via 192.168.0.2


 <<<
@@ -1,14 +1,19 @@
 #!/usr/bin/env python3
-"""
-DHCP option 121 vs option 3
+"""DHCP option 121 vs option 3

-Verify DHCP option 121 (static routes) is used over option 3 and that the
-routes exist in the operational datastore.
+Verify that DHCP option 121 (classless static routes) is used over the
+older option 3 (default gateway) in the DHCP client, when both are sent
+by the server, see RFC3442.  Use the routing RIB in the operational
+datastore to verify.

-Installing unrelated routes from a DHCP server should not affect already
-existing routes.  To verify this, a canary route is set up in the client
-before initiating DHCP.  This canary route does not need to be reachable
-before a DHCP lease has been acquired.
+Installing routes from a DHCP server should not affect already existing
+(static) routes.  To verify this, a static canary route (20.0.0.0/24 via
+192.168.0.2) is installed before starting the DHCP client.  As a twist,
+this canary route has a next-hop (192.168.0.2) which is not reachable
+until a DHCP lease has been acquired.
+
+The DHCP server is set up to hand out both a default router (option 3)
+via 192.168.0.1 and a default route (option 121) via 192.168.0.254.
 """

 import infamy
@@ -22,11 +27,10 @@ with infamy.Test() as test:
    CANARY = '20.0.0.0/24'
    CANHOP = '192.168.0.2'

-    with test.step("Setting up client"):
+    with test.step("Setting up canary route, 20.0.0.0/24 via 192.168.0.2"):
        env = infamy.Env()
        client = env.attach("client", "mgmt")
        _, host = env.ltop.xlate("host", "data")
-        _, port = env.ltop.xlate("client", "data")

        # Install canary route to smoke out any regressions in
        # how the DHCP client installs routes in the kernel FIB
@@ -52,6 +56,9 @@ with infamy.Test() as test:
            }
        })

+    with test.step("Enabling DHCP client, allow option 3 and 121"):
+        _, port = env.ltop.xlate("client", "data")
+
        client.put_config_dict("infix-dhcp-client", {
            "dhcp-client": {
                "client-if": [{
@@ -66,17 +73,18 @@ with infamy.Test() as test:

    with infamy.IsolatedMacVlan(host) as netns:
        netns.addip("192.168.0.1")
+        print(f"Start DHCP server {ROUTER} with option 3 and 121")
        with infamy.dhcp.Server(netns, prefix=PREFIX, router=ROUTER):
-            with test.step("Verify 'client' has a route to 10.0.0.0/24 via 192.168.0.254"):
+            with test.step("Verify 'client' has a route 10.0.0.0/24 via 192.168.0.254 (option 121)"):
                print("Verify client use classless routes, option 121")
                until(lambda: route.ipv4_route_exist(client, PREFIX, ROUTER))

-            with test.step("Verify 'client' has a default route via 192.168.0.254"):
+            with test.step("Verify 'client' has default route via 192.168.0.254 (not use option 3)"):
                print("Verify client did *not* use option 3")
                if route.ipv4_route_exist(client, "0.0.0.0/0", ROUTER):
                    test.fail()

-            with test.step("Verify 'client' has a route to 20.0.0.0/24 via 192.168.0.2"):
+            with test.step("Verify 'client' still has canary route to 20.0.0.0/24 via 192.168.0.2"):
                until(lambda: route.ipv4_route_exist(client, CANARY,
                                                     CANHOP, pref=250))