From 59ec09a83e426a27252c0fd855b5338db53c8fba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 15 Nov 2016 15:01:40 -0500 Subject: [PATCH 1/6] pid1: simplify the logic in two statements related to killing processes Generally non-inverted conditions are nicer, and ternary operators with complex conditions are a bit hard to read. No functional change. --- src/core/scope.c | 7 ++++--- src/core/unit.c | 11 +++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/core/scope.c b/src/core/scope.c index d6e1f8e392..9540fb67d9 100644 --- a/src/core/scope.c +++ b/src/core/scope.c @@ -273,7 +273,9 @@ static void scope_enter_signal(Scope *s, ScopeState state, ScopeResult f) { if (state == SCOPE_STOP_SIGTERM) skip_signal = bus_scope_send_request_stop(s) > 0; - if (!skip_signal) { + if (skip_signal) + r = 1; /* wait */ + else { r = unit_kill_context( UNIT(s), &s->kill_context, @@ -283,8 +285,7 @@ static void scope_enter_signal(Scope *s, ScopeState state, ScopeResult f) { -1, -1, false); if (r < 0) goto fail; - } else - r = 1; + } if (r > 0) { r = scope_arm_timer(s, usec_add(now(CLOCK_MONOTONIC), s->timeout_stop_usec)); diff --git a/src/core/unit.c b/src/core/unit.c index e485c01fc1..ab40135736 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -3755,14 +3755,14 @@ int unit_kill_context( bool main_pid_alien) { bool wait_for_exit = false, send_sighup; - cg_kill_log_func_t log_func; + cg_kill_log_func_t log_func = NULL; int sig, r; assert(u); assert(c); - /* Kill the processes belonging to this unit, in preparation for shutting the unit down. Returns > 0 if we - * killed something worth waiting for, 0 otherwise. */ + /* Kill the processes belonging to this unit, in preparation for shutting the unit down. + * Returns > 0 if we killed something worth waiting for, 0 otherwise. */ if (c->kill_mode == KILL_NONE) return 0; @@ -3774,9 +3774,8 @@ int unit_kill_context( IN_SET(k, KILL_TERMINATE, KILL_TERMINATE_AND_LOG) && sig != SIGHUP; - log_func = - k != KILL_TERMINATE || - IN_SET(sig, SIGKILL, SIGABRT) ? log_kill : NULL; + if (k != KILL_TERMINATE || IN_SET(sig, SIGKILL, SIGABRT)) + log_func = log_kill; if (main_pid > 0) { if (log_func) From b106c5867b77149b6be8f31e23688ae15486ad93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 2 Dec 2016 11:33:31 -0500 Subject: [PATCH 2/6] networkd: do not print ": Success" in debug message %m isn't useful in success path. --- src/network/networkd-link.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 643578c164..31dfff4ff0 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -255,9 +255,10 @@ static int link_enable_ipv6(Link *link) { r = write_string_file(p, one_zero(disabled), WRITE_STRING_FILE_VERIFY_ON_FAILURE); if (r < 0) - log_link_warning_errno(link, r, "Cannot %s IPv6 for interface %s: %m", disabled ? "disable" : "enable", link->ifname); + log_link_warning_errno(link, r, "Cannot %s IPv6 for interface %s: %m", + enable_disable(!disabled), link->ifname); else - log_link_info(link, "IPv6 %sd for interface: %m", enable_disable(!disabled)); + log_link_info(link, "IPv6 successfully %sd", enable_disable(!disabled)); return 0; } From 40a922d055075b921cca060658b5b22608a36c58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 2 Dec 2016 13:24:30 -0500 Subject: [PATCH 3/6] networkd: use log_netdev_error in a two more places --- src/network/netdev/tunnel.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/network/netdev/tunnel.c b/src/network/netdev/tunnel.c index b03e770061..b1f1b5a425 100644 --- a/src/network/netdev/tunnel.c +++ b/src/network/netdev/tunnel.c @@ -397,16 +397,17 @@ static int netdev_tunnel_verify(NetDev *netdev, const char *filename) { assert(t); - if (t->family != AF_INET && t->family != AF_INET6 && t->family != 0) { - log_warning("Tunnel with invalid address family configured in %s. Ignoring", filename); + if (!IN_SET(t->family, AF_INET, AF_INET6, AF_UNSPEC)) { + log_netdev_error(netdev, + "Tunnel with invalid address family configured in %s. Ignoring", filename); return -EINVAL; } - if (netdev->kind == NETDEV_KIND_IP6TNL) { - if (t->ip6tnl_mode == _NETDEV_IP6_TNL_MODE_INVALID) { - log_warning("IP6 Tunnel without mode configured in %s. Ignoring", filename); - return -EINVAL; - } + if (netdev->kind == NETDEV_KIND_IP6TNL && + t->ip6tnl_mode == _NETDEV_IP6_TNL_MODE_INVALID) { + log_netdev_error(netdev, + "ip6tnl without mode configured in %s. Ignoring", filename); + return -EINVAL; } return 0; From 6e47dbbcb32ad9b391296ae47e540c3506431c8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 2 Dec 2016 13:28:01 -0500 Subject: [PATCH 4/6] networkd: tighten parsing of Tunnel addresses When assigning addresses, we'd set the family, and later verify that the address on the other end has the same family. But when the address was specified as "any", we'd simply unset the family. Instead, only unset the family if both addresses are wiped. Also, don't bother setting family = AF_UNSPEC, since it's the default (0). --- src/network/netdev/tunnel.c | 41 +++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/src/network/netdev/tunnel.c b/src/network/netdev/tunnel.c index b1f1b5a425..2ce55a84b6 100644 --- a/src/network/netdev/tunnel.c +++ b/src/network/netdev/tunnel.c @@ -432,26 +432,40 @@ int config_parse_tunnel_address(const char *unit, assert(rvalue); assert(data); + /* This is used to parse addresses on both local and remote ends of the tunnel. + * Address families must match. + * + * "any" is a special value which means that the address is unspecified. + */ + if (streq(rvalue, "any")) { - t->family = 0; + *addr = IN_ADDR_NULL; + + /* As a special case, if both the local and remote addresses are + * unspecified, also clear the address family. + */ + if (t->family != AF_UNSPEC && + in_addr_is_null(t->family, &t->local) && + in_addr_is_null(t->family, &t->remote)) + t->family = AF_UNSPEC; return 0; - } else { + } - r = in_addr_from_string_auto(rvalue, &f, &buffer); - if (r < 0) { - log_syntax(unit, LOG_ERR, filename, line, r, "Tunnel address is invalid, ignoring assignment: %s", rvalue); - return 0; - } + r = in_addr_from_string_auto(rvalue, &f, &buffer); + if (r < 0) { + log_syntax(unit, LOG_ERR, filename, line, r, + "Tunnel address \"%s\" invalid, ignoring assignment: %m", rvalue); + return 0; + } - if (t->family != AF_UNSPEC && t->family != f) { - log_syntax(unit, LOG_ERR, filename, line, 0, "Tunnel addresses incompatible, ignoring assignment: %s", rvalue); - return 0; - } + if (t->family != AF_UNSPEC && t->family != f) { + log_syntax(unit, LOG_ERR, filename, line, 0, + "Tunnel addresses incompatible, ignoring assignment: %s", rvalue); + return 0; } t->family = f; *addr = buffer; - return 0; } @@ -579,7 +593,6 @@ static void ipip_init(NetDev *n) { assert(t); t->pmtudisc = true; - t->family = AF_UNSPEC; } static void sit_init(NetDev *n) { @@ -589,7 +602,6 @@ static void sit_init(NetDev *n) { assert(t); t->pmtudisc = true; - t->family = AF_UNSPEC; } static void vti_init(NetDev *n) { @@ -620,7 +632,6 @@ static void gre_init(NetDev *n) { assert(t); t->pmtudisc = true; - t->family = AF_UNSPEC; } static void ip6gre_init(NetDev *n) { From 6f3d4decf296f0f0fb2b15dd80216f130f10727d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 2 Dec 2016 13:34:35 -0500 Subject: [PATCH 5/6] networkd: check that VTI/VTI6 tunnels have a local address Otherwise we'd fail with an assertion: Assertion 't->family == AF_INET' failed at ../src/network/netdev/tunnel.c:244, function netdev_vti_fill_message_create(). Aborting. --- src/network/netdev/tunnel.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/network/netdev/tunnel.c b/src/network/netdev/tunnel.c index 2ce55a84b6..c11ac0c539 100644 --- a/src/network/netdev/tunnel.c +++ b/src/network/netdev/tunnel.c @@ -403,6 +403,20 @@ static int netdev_tunnel_verify(NetDev *netdev, const char *filename) { return -EINVAL; } + if (netdev->kind == NETDEV_KIND_VTI && + (t->family != AF_INET || in_addr_is_null(t->family, &t->local))) { + log_netdev_error(netdev, + "vti tunnel without a local IPv4 address configured in %s. Ignoring", filename); + return -EINVAL; + } + + if (netdev->kind == NETDEV_KIND_VTI6 && + (t->family != AF_INET6 || in_addr_is_null(t->family, &t->local))) { + log_netdev_error(netdev, + "vti6 tunnel without a local IPv4 address configured in %s. Ignoring", filename); + return -EINVAL; + } + if (netdev->kind == NETDEV_KIND_IP6TNL && t->ip6tnl_mode == _NETDEV_IP6_TNL_MODE_INVALID) { log_netdev_error(netdev, From 9e35b3de429890ed836271dc0ccbd821ab0c204a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 2 Dec 2016 14:00:46 -0500 Subject: [PATCH 6/6] man: make the examples in systemd.network(5) more useful We shouldn't just have snippets of configuration, but instead examples which show all the parts necessary to build a certain kind of setup, with short explanations. --- man/systemd.network.xml | 212 +++++++++++++++++++++++++++------------- 1 file changed, 142 insertions(+), 70 deletions(-) diff --git a/man/systemd.network.xml b/man/systemd.network.xml index 53c49f817f..3a366a573b 100644 --- a/man/systemd.network.xml +++ b/man/systemd.network.xml @@ -608,47 +608,57 @@ Bridge= - The name of the bridge to add the link to. + The name of the bridge to add the link to. See + systemd.netdev5. + Bond= - The name of the bond to add the link to. + The name of the bond to add the link to. See + systemd.netdev5. + VRF= - The name of the VRF to add the link to. + The name of the VRF to add the link to. See + systemd.netdev5. + VLAN= - The name of a VLAN to create on the link. This - option may be specified more than once. + The name of a VLAN to create on the link. See + systemd.netdev5. + This option may be specified more than once. MACVLAN= - The name of a MACVLAN to create on the link. This - option may be specified more than once. + The name of a MACVLAN to create on the link. See + systemd.netdev5. + This option may be specified more than once. VXLAN= - The name of a VXLAN to create on the link. This - option may be specified more than once. + The name of a VXLAN to create on the link. See + systemd.netdev5. + This option may be specified more than once. Tunnel= - The name of a Tunnel to create on the link. This - option may be specified more than once. + The name of a Tunnel to create on the link. See + systemd.netdev5. + This option may be specified more than once. @@ -1287,53 +1297,75 @@ - Example + Examples - /etc/systemd/network/50-static.network + Static network configuration - [Match] + # /etc/systemd/network/50-static.network +[Match] Name=enp2s0 [Network] Address=192.168.0.15/24 Gateway=192.168.0.1 + + This brings interface enp2s0 up with a static address. The + specified gateway will be used for a default route. - /etc/systemd/network/80-dhcp.network + DHCP on ethernet links - [Match] + # /etc/systemd/network/80-dhcp.network +[Match] Name=en* [Network] DHCP=yes + + This will enable DHCPv4 and DHCPv6 on all interfaces with names starting with + en (i.e. ethernet interfaces). - /etc/systemd/network/25-bridge-static.network + A bridge with two enslaved links - [Match] + # /etc/systemd/network/25-bridge-static.network +[Match] Name=bridge0 [Network] Address=192.168.0.15/24 Gateway=192.168.0.1 DNS=192.168.0.1 - - - /etc/systemd/network/25-bridge-slave-interface.network - - [Match] + # /etc/systemd/network/25-bridge-slave-interface-1.network +[Match] Name=enp2s0 [Network] Bridge=bridge0 - - - /etc/systemd/network/25-bridge-slave-interface-vlan.network - [Match] + # /etc/systemd/network/25-bridge-slave-interface-2.network +[Match] +Name=wlp3s0 + +[Network] +Bridge=bridge0 + + This creates a bridge and attaches devices enp2s0 and + wlp3s0 to it. The bridge will have the specified static address + and network assigned, and a default route via the specified gateway will be + added. The specified DNS server will be added to the global list of DNS resolvers. + + + + + + + +# /etc/systemd/network/20-bridge-slave-interface-vlan.network +[Match] Name=enp2s0 [Network] @@ -1349,66 +1381,106 @@ VLAN=100-200 [BridgeVLAN] EgressUntagged=300-400 - - - /etc/systemd/network/25-ipip.network - [Match] -Name=em1 + This overrides the configuration specified in the previous example for the + interface enp2s0, and enables VLAN on that bridge port. VLAN IDs + 1-32, 42, 100-400 will be allowed. Packets tagged with VLAN IDs 42, 300-400 will be + untagged when they leave on this interface. Untagged packets which arrive on this + interface will be assigned VLAN ID 42. + + + + Various tunnels + + /etc/systemd/network/25-tunnels.network +[Match] +Name=ens1 [Network] -Tunnel=ipip-tun +Tunnel=ipip-tun +Tunnel=sit-tun +Tunnel=gre-tun +Tunnel=vti-tun + + + /etc/systemd/network/25-tunnel-ipip.netdev +[NetDev] +Name=ipip-tun +Kind=ipip + + + /etc/systemd/network/25-tunnel-sit.netdev +[NetDev] +Name=sit-tun +Kind=sit + + + /etc/systemd/network/25-tunnel-gre.netdev +[NetDev] +Name=gre-tun +Kind=gre + + + /etc/systemd/network/25-tunnel-vti.netdev +[NetDev] +Name=vti-tun +Kind=vti + + + This will bring interface ens1 up and create an IPIP tunnel, + a SIT tunnel, a GRE tunnel, and a VTI tunnel using it. - /etc/systemd/network/25-sit.network + A bond device - [Match] -Name=em1 - -[Network] -Tunnel=sit-tun - - - - /etc/systemd/network/25-gre.network - - [Match] -Name=em1 - -[Network] -Tunnel=gre-tun - - - - /etc/systemd/network/25-vti.network - - [Match] -Name=em1 - -[Network] -Tunnel=vti-tun - - - - /etc/systemd/network/25-bond.network - - [Match] + # /etc/systemd/network/30-bond1.network +[Match] Name=bond1 [Network] -DHCP=yes +DHCP=ipv6 + + # /etc/systemd/network/30-bond1.netdev +[NetDev] +Name=bond1 +Kind=bond + + + # /etc/systemd/network/30-bond1-dev1.nework +[Match] +MACAddress=52:54:00:e9:64:41 + +[Network] +Bond=bond1 + + + # /etc/systemd/network/30-bond1-dev2.nework +[Match] +MACAddress=52:54:00:e9:64:42 + +[Network] +Bond=bond1 + + + This will create a bond device bond1 and enslave the two + devices with MAC addresses 52:54:00:e9:64:41 and 52:54:00:e9:64:42 to it. IPv6 DHCP + will be used to acquire an address. - /etc/systemd/network/25-vrf.network - Add the bond1 interface to the VRF master interface vrf-test. This will redirect routes generated on this interface to be within the routing table defined during VRF creation. Traffic won't be redirected towards the VRFs routing table unless specific ip-rules are added. - [Match] + Virtual Routing and Forwarding (VRF) + Add the bond1 interface to the VRF master interface + vrf1. This will redirect routes generated on this interface to be + within the routing table defined during VRF creation. Traffic won't be redirected + towards the VRFs routing table unless specific ip-rules are added. + # /etc/systemd/network/25-vrf.network +[Match] Name=bond1 [Network] -VRF=vrf-test +VRF=vrf1