diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 8d3bcc01..2d59be9b 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -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 - :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 diff --git a/board/common/lib.sh b/board/common/lib.sh index affa806e..89f7e61a 100644 --- a/board/common/lib.sh +++ b/board/common/lib.sh @@ -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" } diff --git a/doc/ChangeLog.md b/doc/ChangeLog.md index 93bd8317..71ccd00c 100644 --- a/doc/ChangeLog.md +++ b/doc/ChangeLog.md @@ -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 diff --git a/doc/developers-guide.md b/doc/developers-guide.md index d90d88ea..5a54d839 100644 --- a/doc/developers-guide.md +++ b/doc/developers-guide.md @@ -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/ diff --git a/doc/networking.md b/doc/networking.md index f6ac9426..f8eaece9 100644 --- a/doc/networking.md +++ b/doc/networking.md @@ -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 diff --git a/package/finit/0001-Only-mark-rdeps-dirty-if-main-service-is-nohup.patch b/package/finit/0001-Only-mark-rdeps-dirty-if-main-service-is-nohup.patch index 25e2b06a..f184e4cf 100644 --- a/package/finit/0001-Only-mark-rdeps-dirty-if-main-service-is-nohup.patch +++ b/package/finit/0001-Only-mark-rdeps-dirty-if-main-service-is-nohup.patch @@ -1,7 +1,7 @@ From 46ffa81f5c88ce95db011369d8bfb802313e4217 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg 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, diff --git a/package/finit/0002-Reset-color-attributes-and-clear-screen-when-startin.patch b/package/finit/0002-Reset-color-attributes-and-clear-screen-when-startin.patch new file mode 100644 index 00000000..d46c51b4 --- /dev/null +++ b/package/finit/0002-Reset-color-attributes-and-clear-screen-when-startin.patch @@ -0,0 +1,33 @@ +From 119e66a7e9c95283918639b51dd03a3d666955f8 Mon Sep 17 00:00:00 2001 +From: Joachim Wiberg +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 +--- + 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 + diff --git a/src/confd/src/ietf-interfaces.c b/src/confd/src/ietf-interfaces.c index e347f513..b65e09f3 100644 --- a/src/confd/src/ietf-interfaces.c +++ b/src/confd/src/ietf-interfaces.c @@ -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")) diff --git a/src/confd/yang/confd.inc b/src/confd/yang/confd.inc index aa2de0f2..ea4fe93b 100644 --- a/src/confd/yang/confd.inc +++ b/src/confd/yang/confd.inc @@ -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" diff --git a/src/confd/yang/containers.inc b/src/confd/yang/containers.inc index 3ac2d49a..af72dc83 100644 --- a/src/confd/yang/containers.inc +++ b/src/confd/yang/containers.inc @@ -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" ) diff --git a/src/confd/yang/infix-interfaces.yang b/src/confd/yang/infix-interfaces.yang index ccde9c86..b5c0bbd6 100644 --- a/src/confd/yang/infix-interfaces.yang +++ b/src/confd/yang/infix-interfaces.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; - } } diff --git a/src/confd/yang/infix-interfaces@2024-10-14.yang b/src/confd/yang/infix-interfaces@2024-10-28.yang similarity index 100% rename from src/confd/yang/infix-interfaces@2024-10-14.yang rename to src/confd/yang/infix-interfaces@2024-10-28.yang diff --git a/src/statd/python/yanger/yanger.py b/src/statd/python/yanger/yanger.py index dc87dccc..1b965402 100755 --- a/src/statd/python/yanger/yanger.py +++ b/src/statd/python/yanger/yanger.py @@ -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'] diff --git a/test/case/ietf_interfaces/Readme.adoc b/test/case/ietf_interfaces/Readme.adoc index f19f093b..df395c9d 100644 --- a/test/case/ietf_interfaces/Readme.adoc +++ b/test/case/ietf_interfaces/Readme.adoc @@ -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[] diff --git a/test/case/ietf_interfaces/ietf_interfaces.yaml b/test/case/ietf_interfaces/ietf_interfaces.yaml index a512bc27..9118da66 100644 --- a/test/case/ietf_interfaces/ietf_interfaces.yaml +++ b/test/case/ietf_interfaces/ietf_interfaces.yaml @@ -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 diff --git a/test/case/ietf_interfaces/ifalias/Readme.adoc b/test/case/ietf_interfaces/ifalias/Readme.adoc new file mode 100644 index 00000000..02209f91 --- /dev/null +++ b/test/case/ietf_interfaces/ifalias/Readme.adoc @@ -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 + + +<<< + diff --git a/test/case/ietf_interfaces/ifalias/test.py b/test/case/ietf_interfaces/ifalias/test.py new file mode 100755 index 00000000..83c12f42 --- /dev/null +++ b/test/case/ietf_interfaces/ifalias/test.py @@ -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() diff --git a/test/case/ietf_interfaces/ifalias/topology.dot b/test/case/ietf_interfaces/ifalias/topology.dot new file mode 120000 index 00000000..4f53d15a --- /dev/null +++ b/test/case/ietf_interfaces/ifalias/topology.dot @@ -0,0 +1 @@ +../../../infamy/topologies/1x2.dot \ No newline at end of file diff --git a/test/case/ietf_interfaces/ifalias/topology.svg b/test/case/ietf_interfaces/ifalias/topology.svg new file mode 100644 index 00000000..ff3d246b --- /dev/null +++ b/test/case/ietf_interfaces/ifalias/topology.svg @@ -0,0 +1,42 @@ + + + + + + +1x2 + + + +host + +host + +mgmt + +data + + + +target + +mgmt + +data + +target + + + +host:mgmt--target:mgmt + + + + +host:data--target:data + + + + diff --git a/test/case/infix_dhcp/dhcp_routes/Readme.adoc b/test/case/infix_dhcp/dhcp_routes/Readme.adoc index 9b7224df..007ad217 100644 --- a/test/case/infix_dhcp/dhcp_routes/Readme.adoc +++ b/test/case/infix_dhcp/dhcp_routes/Readme.adoc @@ -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 <<< diff --git a/test/case/infix_dhcp/dhcp_routes/test.py b/test/case/infix_dhcp/dhcp_routes/test.py index e4592ff6..394f9a8e 100755 --- a/test/case/infix_dhcp/dhcp_routes/test.py +++ b/test/case/infix_dhcp/dhcp_routes/test.py @@ -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))