From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp2.migadu.com ([2001:41d0:303:e16b::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms13.migadu.com with LMTPS id 0EDsIDB2bGfE0QAAe85BDQ:P1 (envelope-from ) for ; Wed, 25 Dec 2024 21:16:32 +0000 Received: from aspmx1.migadu.com ([2001:41d0:303:e16b::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp2.migadu.com with LMTPS id 0EDsIDB2bGfE0QAAe85BDQ (envelope-from ) for ; Wed, 25 Dec 2024 22:16:32 +0100 X-Envelope-To: larch@yhetil.org Authentication-Results: aspmx1.migadu.com; dkim=pass header.d=debbugs.gnu.org header.s=debbugs-gnu-org header.b=J1RJ+eJE; dkim=fail ("headers rsa verify failed") header.d=gnu.org header.s=fencepost-gnu-org header.b=OFviXy+5; spf=pass (aspmx1.migadu.com: domain of "guix-patches-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="guix-patches-bounces+larch=yhetil.org@gnu.org"; dmarc=pass (policy=none) header.from=gnu.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1735161391; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding:resent-cc: resent-from:resent-sender:resent-message-id:in-reply-to:in-reply-to: references:references:list-id:list-help:list-unsubscribe: list-subscribe:list-post:dkim-signature; bh=tlay9kiZqAp3GBKTLgftPi1SANrFEAPbdVrOegM9ZTE=; b=N3DT1HBOCZvATAGhpxuC397ZKnBc4b2+ep3+Lx05C2PKaoQnalnfNqMpQI1DpLhrPQhJWw sZAYXhGw0c7HoX19xEmPGCqh4P0RVRfgF0F353AmQzuzCtMFTBQ0AoKJMRXrFqZeHo8/yu 3/qrf2dClGfHct+GpAYkj/cDh5EaRx9lNsHol+L9F0u3fZ1QCeojTtCNti9IVAguH8X9oH +yu/Ps2VeP/rMxujdsBr904jtwCx5twKuBnFI1rHD2oxR5Pa020x0YdpnPA/KP+HTCqd50 sVvOtltN7X/eaVRo42FaJND1a6qgoHkOx0tQ5pe9QdgpGcVgq7hpnGwGXYPqiw== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=pass header.d=debbugs.gnu.org header.s=debbugs-gnu-org header.b=J1RJ+eJE; dkim=fail ("headers rsa verify failed") header.d=gnu.org header.s=fencepost-gnu-org header.b=OFviXy+5; spf=pass (aspmx1.migadu.com: domain of "guix-patches-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="guix-patches-bounces+larch=yhetil.org@gnu.org"; dmarc=pass (policy=none) header.from=gnu.org ARC-Seal: i=1; s=key1; d=yhetil.org; t=1735161391; a=rsa-sha256; cv=none; b=gSpnZWbUWaKuYtQ6s7zYo5eZxOGPdLneBs+EtIwk1niUhWgtjS8kT2Gl+8mRskdRHryvga +i9xbu1uwk+Un70WpaBEaN6MX0pmihpUADA/ppznmLEopcY5HXotuzFQyyoX0yoy+bSKVd bOBJUnlTTCqlxwCAYy8BohXFklhTVCJ4bjOI2EDGkJvO0eZM/SyINFlY9MlJh+ylKFy/vp FtsiG/x+0ziQL7UtOs2kumZLNgZgvudY9lC3nvApRASszkRP1q+Wz7xVEfygajDigx241A mqq06tVBUi+oa7Kg43P8PV+Zsy5lVnlwfbzYewiokdCWdAMLYG1mpTIHcrR/xQ== Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id C546F8573F for ; Wed, 25 Dec 2024 22:16:30 +0100 (CET) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1tQYjf-00014s-0y; Wed, 25 Dec 2024 16:16:07 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1tQYja-00013g-SI for guix-patches@gnu.org; Wed, 25 Dec 2024 16:16:03 -0500 Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1tQYja-0001nB-Jy for guix-patches@gnu.org; Wed, 25 Dec 2024 16:16:02 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=debbugs.gnu.org; s=debbugs-gnu-org; h=MIME-Version:References:In-Reply-To:Date:From:To:Subject; bh=tlay9kiZqAp3GBKTLgftPi1SANrFEAPbdVrOegM9ZTE=; b=J1RJ+eJEdFxKj3qY441GhGB5vQOSCnFdIcm84tJDOuDkmffHnwEqda15iJGUWoBw+C4CgNL7WFPwHokrqnQ/zkH8olo55rDpx4yEeo1w/4z5j6oRFkkZEKBjH2CyGZ5RwOhlBL7M7oFqjAszFxNPT7A6jinoJM6Q2K4QR4n0G/0khinq5xgFVZ2xhB+Zz5OHSUQhNYUk6zSo+UL7nK+8TkmMmBP8BX2juBJemBcLAnWA+/Vrvuw+yu+CR9vW549bVyP44gC5uCH0EIkW16w+mzb1ZI+Bfvk0t984U4mX7qO2oecbxToMQl6tbcXawoCIVINinCcBNDxe6SLOImLocQ==; Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1tQYja-00015R-Dx for guix-patches@gnu.org; Wed, 25 Dec 2024 16:16:02 -0500 X-Loop: help-debbugs@gnu.org Subject: [bug#75100] [PATCH 1/3] services: static-networking: Run set-up/tear-down as a separate process. Resent-From: Ludovic =?UTF-8?Q?Court=C3=A8s?= Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Wed, 25 Dec 2024 21:16:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 75100 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: 75100@debbugs.gnu.org Cc: Ludovic =?UTF-8?Q?Court=C3=A8s?= Received: via spool by 75100-submit@debbugs.gnu.org id=B75100.17351613514139 (code B ref 75100); Wed, 25 Dec 2024 21:16:02 +0000 Received: (at 75100) by debbugs.gnu.org; 25 Dec 2024 21:15:51 +0000 Received: from localhost ([127.0.0.1]:39079 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1tQYjO-00014b-Db for submit@debbugs.gnu.org; Wed, 25 Dec 2024 16:15:51 -0500 Received: from eggs.gnu.org ([209.51.188.92]:35708) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1tQYjM-00014M-K0 for 75100@debbugs.gnu.org; Wed, 25 Dec 2024 16:15:49 -0500 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1tQYjG-0001kv-SO; Wed, 25 Dec 2024 16:15:43 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-Version:References:In-Reply-To:Date:Subject:To: From; bh=tlay9kiZqAp3GBKTLgftPi1SANrFEAPbdVrOegM9ZTE=; b=OFviXy+5WoGwU3KMBTph tqaqTlP84L7/5Ra5fqoX7hfQWw755LchmgQo2hirOQsy+lwGk72fE3bij2pKZHsBY8339ZyIE+G6b iLrYBGrPKOVQ/Y1aWT6N8LNgNIdfX2Qy5wJOIzwGdMtJHtGohwV5q+FDVjalL2ipRBybbU3e02NtQ yoBCO3s3SDRAdLiBDOpZXeSFAMQhAEOQs+F4a0J50o7dv7Y0dTvvVaFF7XyCjmOh2Zg0WOqv69M6x JVzPeCCDgdiwxmN/3s/TdetbacH9ZtMDLs+N5mWNUQmgGjMCfUeU5TbViCf4ANIoS+3oq7f+NSpuJ NQSnoMzBEo/POQ==; From: Ludovic =?UTF-8?Q?Court=C3=A8s?= Date: Wed, 25 Dec 2024 22:15:31 +0100 Message-ID: <42215a4536fa59b6d30e5346289043f8ab17b239.1735160803.git.ludo@gnu.org> X-Mailer: git-send-email 2.46.0 In-Reply-To: References: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+larch=yhetil.org@gnu.org Sender: guix-patches-bounces+larch=yhetil.org@gnu.org X-Migadu-Country: US X-Migadu-Flow: FLOW_IN X-Migadu-Queue-Id: C546F8573F X-Migadu-Scanner: mx13.migadu.com X-Migadu-Spam-Score: -3.20 X-Spam-Score: -3.20 X-TUID: r/7zq8wRZyiw Running that code in PID 1 was fun but it’s not really beneficial and somewhat risky: risk of blocking, file descriptor leak, inability to reload Guile-Netlink in shepherd when it’s upgraded, and so on. This change runs set-up and tear-down as separate processes, which, for the price of one fork(1), buys us peace of mind. * gnu/services/base.scm (network-set-up/hurd, network-tear-down/hurd) (network-tear-down/linux): Use ‘program-file’ instead of ‘scheme-file’. (network-set-up/linux): Likewise, and remove #:blocking? argument to ‘wait-for-link’. Change-Id: Ia41479b50eab31ea40c67243fcb1cffe29ac874a --- gnu/services/base.scm | 361 +++++++++++++++++++++--------------------- 1 file changed, 181 insertions(+), 180 deletions(-) diff --git a/gnu/services/base.scm b/gnu/services/base.scm index fc604f029a..f6d1da61cd 100644 --- a/gnu/services/base.scm +++ b/gnu/services/base.scm @@ -3055,172 +3055,139 @@ (define (network-set-up/hurd config) ;; The Hurd implements SIOCGIFADDR and other old-style ioctls, but the only ;; way to set up IPv6 is by starting pfinet with the right options. (if (equal? (static-networking-provision config) '(loopback)) - (scheme-file "set-up-pflocal" #~(begin 'nothing-to-do! #t)) - (scheme-file "set-up-pfinet" - (with-imported-modules '((guix build utils)) - #~(begin - (use-modules (guix build utils) - (ice-9 format)) + (program-file "set-up-pflocal" #~(begin 'nothing-to-do! #t)) + (program-file "set-up-pfinet" + (with-imported-modules '((guix build utils)) + #~(begin + (use-modules (guix build utils) + (ice-9 format)) - ;; TODO: Do that without forking. - (let ((options '#$(static-networking->hurd-pfinet-options - config))) - (format #t "starting '~a~{ ~s~}'~%" + ;; TODO: Do that without forking. + (let ((options '#$(static-networking->hurd-pfinet-options + config))) + (format #t "starting '~a~{ ~s~}'~%" + #$(file-append hurd "/hurd/pfinet") + options) + (apply invoke #$(file-append hurd "/bin/settrans") + "--active" + "--create" + "--keep-active" + "/servers/socket/2" #$(file-append hurd "/hurd/pfinet") - options) - (apply invoke #$(file-append hurd "/bin/settrans") - "--active" - "--create" - "--keep-active" - "/servers/socket/2" - #$(file-append hurd "/hurd/pfinet") - options))))))) + options))))))) (define (network-tear-down/hurd config) - (scheme-file "tear-down-pfinet" - (with-imported-modules '((guix build utils)) - #~(begin - (use-modules (guix build utils)) + (program-file "tear-down-pfinet" + (with-imported-modules '((guix build utils)) + #~(begin + (use-modules (guix build utils)) - ;; Forcefully terminate pfinet. XXX: In theory this - ;; should just undo the addresses and routes of CONFIG; - ;; this could be done using ioctls like SIOCDELRT, but - ;; these are IPv4-only; another option would be to use - ;; fsysopts but that seems to crash pfinet. - (invoke #$(file-append hurd "/bin/settrans") "-fg" - "/servers/socket/2") - #f)))) + ;; Forcefully terminate pfinet. XXX: In theory this + ;; should just undo the addresses and routes of CONFIG; + ;; this could be done using ioctls like SIOCDELRT, but + ;; these are IPv4-only; another option would be to use + ;; fsysopts but that seems to crash pfinet. + (invoke #$(file-append hurd "/bin/settrans") "-fg" + "/servers/socket/2") + #f)))) (define (network-set-up/linux config) (match-record config (addresses links routes) - (scheme-file "set-up-network" - (with-extensions (list guile-netlink) - #~(begin - (use-modules (ip addr) (ip link) (ip route) - (srfi srfi-1) - (ice-9 format) - (ice-9 match)) + (program-file "set-up-network" + (with-extensions (list guile-netlink) + #~(begin + (use-modules (ip addr) (ip link) (ip route) + (srfi srfi-1) + (ice-9 format) + (ice-9 match)) - (define (match-link-by field-accessor value) - (fold (lambda (link result) - (if (equal? (field-accessor link) value) - link - result)) - #f - (get-links))) + (define (match-link-by field-accessor value) + (fold (lambda (link result) + (if (equal? (field-accessor link) value) + link + result)) + #f + (get-links))) - (define (alist->keyword+value alist) - (fold (match-lambda* - (((k . v) r) - (cons* (symbol->keyword k) v r))) '() alist)) + (define (alist->keyword+value alist) + (fold (match-lambda* + (((k . v) r) + (cons* (symbol->keyword k) v r))) '() alist)) - ;; FIXME: It is interesting that "modprobe bonding" creates an - ;; interface bond0 straigt away. If we won't have bonding - ;; module, and execute `ip link add name bond0 type bond' we - ;; will get - ;; - ;; RTNETLINK answers: File exists - ;; - ;; This breaks our configuration if we want to - ;; use `bond0' name. Create (force modprobe - ;; bonding) and delete the interface to free up - ;; bond0 name. - #$(let lp ((links links)) - (cond - ((null? links) #f) - ((and (network-link? (car links)) - ;; Type is not mandatory - (false-if-exception - (eq? (network-link-type (car links)) 'bond))) - #~(begin - (false-if-exception (link-add "bond0" "bond")) - (link-del "bond0"))) - (else (lp (cdr links))))) + ;; FIXME: It is interesting that "modprobe bonding" creates an + ;; interface bond0 straigt away. If we won't have bonding + ;; module, and execute `ip link add name bond0 type bond' we + ;; will get + ;; + ;; RTNETLINK answers: File exists + ;; + ;; This breaks our configuration if we want to + ;; use `bond0' name. Create (force modprobe + ;; bonding) and delete the interface to free up + ;; bond0 name. + #$(let lp ((links links)) + (cond + ((null? links) #f) + ((and (network-link? (car links)) + ;; Type is not mandatory + (false-if-exception + (eq? (network-link-type (car links)) 'bond))) + #~(begin + (false-if-exception (link-add "bond0" "bond")) + (link-del "bond0"))) + (else (lp (cdr links))))) - #$@(map (match-lambda - (($ name type mac-address arguments) - (cond - ;; Create a new interface - ((and (string? name) (symbol? type)) - #~(begin - (link-add #$name (symbol->string '#$type) #:type-args '#$arguments) - ;; XXX: If we add routes, addresses must be - ;; already assigned, and interfaces must be - ;; up. It doesn't matter if they won't have - ;; carrier or anything. - (link-set #$name #:up #t))) + #$@(map (match-lambda + (($ name type mac-address arguments) + (cond + ;; Create a new interface + ((and (string? name) (symbol? type)) + #~(begin + (link-add #$name (symbol->string '#$type) #:type-args '#$arguments) + ;; XXX: If we add routes, addresses must be + ;; already assigned, and interfaces must be + ;; up. It doesn't matter if they won't have + ;; carrier or anything. + (link-set #$name #:up #t))) - ;; Amend an existing interface - ((and (string? name) - (eq? type #f)) - #~(let ((link (match-link-by link-name #$name))) - (if link - (apply link-set - (link-id link) - (alist->keyword+value '#$arguments)) - (format #t (G_ "Interface with name '~a' not found~%") #$name)))) - ((string? mac-address) - #~(let ((link (match-link-by link-addr #$mac-address))) - (if link - (apply link-set - (link-id link) - (alist->keyword+value '#$arguments)) - (format #t (G_ "Interface with mac-address '~a' not found~%") #$mac-address))))))) - links) + ;; Amend an existing interface + ((and (string? name) + (eq? type #f)) + #~(let ((link (match-link-by link-name #$name))) + (if link + (apply link-set + (link-id link) + (alist->keyword+value '#$arguments)) + (format #t (G_ "Interface with name '~a' not found~%") #$name)))) + ((string? mac-address) + #~(let ((link (match-link-by link-addr #$mac-address))) + (if link + (apply link-set + (link-id link) + (alist->keyword+value '#$arguments)) + (format #t (G_ "Interface with mac-address '~a' not found~%") #$mac-address))))))) + links) - #$@(map (lambda (address) - #~(begin - ;; Before going any further, wait for the - ;; device to show up. - (wait-for-link - #$(network-address-device address) - #:blocking? #f) + #$@(map (lambda (address) + #~(begin + ;; Before going any further, wait for the + ;; device to show up. + (wait-for-link + #$(network-address-device address)) - (addr-add #$(network-address-device address) - #$(network-address-value address) - #:ipv6? - #$(network-address-ipv6? address)) - ;; FIXME: loopback? - (link-set #$(network-address-device address) - #:multicast-on #t - #:up #t))) - addresses) + (addr-add #$(network-address-device address) + #$(network-address-value address) + #:ipv6? + #$(network-address-ipv6? address)) + ;; FIXME: loopback? + (link-set #$(network-address-device address) + #:multicast-on #t + #:up #t))) + addresses) - #$@(map (lambda (route) - #~(route-add #$(network-route-destination route) - #:device - #$(network-route-device route) - #:ipv6? - #$(network-route-ipv6? route) - #:via - #$(network-route-gateway route) - #:src - #$(network-route-source route))) - routes) - #t))))) - -(define (network-tear-down/linux config) - (match-record config - (addresses links routes) - (scheme-file "tear-down-network" - (with-extensions (list guile-netlink) - #~(begin - (use-modules (ip addr) (ip link) (ip route) - (netlink error) - (srfi srfi-34)) - - (define-syntax-rule (false-if-netlink-error exp) - (guard (c ((netlink-error? c) #f)) - exp)) - - ;; Wrap calls in 'false-if-netlink-error' so this - ;; script goes as far as possible undoing the effects - ;; of "set-up-network". - - #$@(map (lambda (route) - #~(false-if-netlink-error - (route-del #$(network-route-destination route) + #$@(map (lambda (route) + #~(route-add #$(network-route-destination route) #:device #$(network-route-device route) #:ipv6? @@ -3228,31 +3195,63 @@ (define (network-tear-down/linux config) #:via #$(network-route-gateway route) #:src - #$(network-route-source route)))) - routes) + #$(network-route-source route))) + routes) + #t))))) - ;; Cleanup addresses first, they might be assigned to - ;; created bonds, vlans or bridges. - #$@(map (lambda (address) - #~(false-if-netlink-error - (addr-del #$(network-address-device - address) - #$(network-address-value address) - #:ipv6? - #$(network-address-ipv6? address)))) - addresses) +(define (network-tear-down/linux config) + (match-record config + (addresses links routes) + (program-file "tear-down-network" + (with-extensions (list guile-netlink) + #~(begin + (use-modules (ip addr) (ip link) (ip route) + (netlink error) + (srfi srfi-34)) - ;; It is now safe to delete some links - #$@(map (match-lambda - (($ name type mac-address arguments) - (cond - ;; We delete interfaces that were created - ((and (string? name) (symbol? type)) - #~(false-if-netlink-error - (link-del #$name))) - (else #t)))) - links) - #f))))) + (define-syntax-rule (false-if-netlink-error exp) + (guard (c ((netlink-error? c) #f)) + exp)) + + ;; Wrap calls in 'false-if-netlink-error' so this + ;; script goes as far as possible undoing the effects + ;; of "set-up-network". + + #$@(map (lambda (route) + #~(false-if-netlink-error + (route-del #$(network-route-destination route) + #:device + #$(network-route-device route) + #:ipv6? + #$(network-route-ipv6? route) + #:via + #$(network-route-gateway route) + #:src + #$(network-route-source route)))) + routes) + + ;; Cleanup addresses first, they might be assigned to + ;; created bonds, vlans or bridges. + #$@(map (lambda (address) + #~(false-if-netlink-error + (addr-del #$(network-address-device + address) + #$(network-address-value address) + #:ipv6? + #$(network-address-ipv6? address)))) + addresses) + + ;; It is now safe to delete some links + #$@(map (match-lambda + (($ name type mac-address arguments) + (cond + ;; We delete interfaces that were created + ((and (string? name) (symbol? type)) + #~(false-if-netlink-error + (link-del #$name))) + (else #t)))) + links) + #f))))) (define (static-networking-shepherd-service config) (match-record config @@ -3267,16 +3266,18 @@ (define (static-networking-shepherd-service config) (start #~(lambda _ ;; Return #t if successfully started. - (load #$(let-system (system target) - (if (string-contains (or target system) "-linux") - (network-set-up/linux config) - (network-set-up/hurd config)))))) + (zero? (system* + #$(let-system (system target) + (if (string-contains (or target system) "-linux") + (network-set-up/linux config) + (network-set-up/hurd config))))))) (stop #~(lambda _ ;; Return #f is successfully stopped. - (load #$(let-system (system target) - (if (string-contains (or target system) "-linux") - (network-tear-down/linux config) - (network-tear-down/hurd config)))))) + (zero? (system* + #$(let-system (system target) + (if (string-contains (or target system) "-linux") + (network-tear-down/linux config) + (network-tear-down/hurd config))))))) (respawn? #f))))) (define (static-networking-shepherd-services networks) -- 2.46.0