Add support for networkd ConfigureWithoutCarrier option on interfaces #215

n-cc · 2021-06-24T22:01:02Z

Description

The ConfigureWithoutCarrier networkd option allows networkd to configure a specific link even if it has no carrier. This is useful for services such as isc-dhcp-server, which requires that all managed interfaces are up and assigned IP addresses regardless of their physical status.

I chose to put this key under COMMON_LINK_HANDLERS rather than PHYSICAL_LINK_HANDLERS as networkd does not differentiate between physical and virtual links WRT the ConfigureWithoutCarrier option, as far as I can tell. Better names are welcome.

Checklist

Runs make check successfully.
Retains 100% code coverage (make check-coverage) *(make check-coverage reports the same coverage on master)
New/changed keys in YAML format are documented.
(Optional) Adds example YAML for new feature.
(Optional) Closes an open bug in Launchpad.

…inks

src/parse.h

…e documentation phrasing; add test coverage

codecov-commenter · 2021-06-25T15:50:26Z

Codecov Report

Merging #215 (4d5df9b) into main (e533ddf) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #215   +/-   ##
=======================================
  Coverage   99.02%   99.02%           
=======================================
  Files          56       56           
  Lines        9343     9347    +4     
=======================================
+ Hits         9252     9256    +4     
  Misses         91       91

Impacted Files	Coverage Δ
src/parse.c	`100.00% <ø> (ø)`
src/netplan.c	`100.00% <100.00%> (ø)`
src/networkd.c	`100.00% <100.00%> (ø)`
tests/generator/test_common.py	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e533ddf...4d5df9b. Read the comment docs.

slyon · 2021-07-05T16:17:59Z

Thank you very much for this PR! I didn't do a full review yet, but overall it is looking good from a code POV.

Though, as this contains a change of the YAML schema ("passthrough" of networkd's ConfigureWithoutCarrier= setting), I'd like to get an opinion from @vorlonofportland about if and how we want to support this use case before proceeding.

@n-cc Do you know if a similar feature is available and/or needed to support this with netplan's NetworkManager backend as well?

n-cc · 2021-07-06T19:06:11Z

@n-cc Do you know if a similar feature is available and/or needed to support this with netplan's NetworkManager backend as well?

I don't have much experience with NetworkManager, but ignore-carrier seems to be the equivalent option (manpage here). Adding the following section to /etc/NetworkManager/NetworkManager.conf allowed my physical interface to gain a static IP (configured separately under the system-connections directory) despite the physical link being down:

[device]
match-device=interface-name:enp0s31f6
managed=1
ignore-carrier=true

However, it doesn't seem that netplan's NetworkManager renderer currently templates the [device] section anywhere.

Looking at some of the common tests, it seems like there are situations where NetworkManager is used as the "main" renderer and networkd used for just specific ethernet interfaces; in these situations, my changes are sufficient to allow an interface to be assigned an IP without a physical link, so this feature can at least coexist with NetworkManager:

# cat /etc/netplan/01-network-manager-all.yaml 
# Let NetworkManager manage all devices on this system
network:
  version: 2
  renderer: NetworkManager
  ethernets:
    enp0s31f6:
      renderer: networkd
      addresses: [10.0.0.2/24]
      dhcp4: false
      dhcp-identifier: mac
      optional: false
      gateway4: 10.0.0.1
      configure-without-carrier: true
# ip addr
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN group default qlen 1000
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
    inet 127.0.0.1/8 scope host lo
       valid_lft forever preferred_lft forever
    inet6 ::1/128 scope host 
       valid_lft forever preferred_lft forever
2: enp0s31f6: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc fq_codel state DOWN group default qlen 1000
    link/ether 30:5a:3a:83:8c:56 brd ff:ff:ff:ff:ff:ff
    inet 10.0.0.2/24 brd 10.0.0.255 scope global enp0s31f6
       valid_lft forever preferred_lft forever
#

vorlonofportland · 2021-09-08T00:55:50Z

doc/netplan.md

@@ -277,6 +277,11 @@ Virtual devices
    Example to enable all link-local addresses: ``link-local: [ ipv4, ipv6 ]``
    Example to disable all link-local addresses: ``link-local: [ ]``

+``configure-without-carrier`` (bool)


my preference here is for the 'ignore-carrier' syntax, I think 'configure-without-carrier' is unnecessarily verbose.

slyon

Thank you Steve for your opinion on the ignore-carrier wording. Let's go with that.

So I think this PR is fine, as soon as the setting is renamed to "ignore-carrier".

Additionally, we should consider adopting it to the NetworkManager backend as well. Thanks for your investigation on this @n-cc ! We should be able to generated drop-in configs for NetworkManager in /run/NetworkManager/conf.d/XX-netplan-NETDEF.conf to contain the [device] section you mentioned. Would you be willing to look into this (as part of an additional PR)?

slyon · 2021-09-08T10:27:22Z

doc/netplan.md

@@ -277,6 +277,11 @@ Virtual devices
    Example to enable all link-local addresses: ``link-local: [ ipv4, ipv6 ]``
    Example to disable all link-local addresses: ``link-local: [ ]``

+``configure-without-carrier`` (bool)


Suggested change

``configure-without-carrier`` (bool)

``ignore-carrier`` (bool) – since **0.104*

slyon · 2021-09-08T10:28:27Z

src/parse.c

@@ -2235,6 +2235,7 @@ static const mapping_entry_handler dhcp6_overrides_handlers[] = {
    {"activation-mode", YAML_SCALAR_NODE, handle_activation_mode, NULL, netdef_offset(activation_mode)}, \
    {"addresses", YAML_SEQUENCE_NODE, handle_addresses},                                      \
    {"critical", YAML_SCALAR_NODE, handle_netdef_bool, NULL, netdef_offset(critical)},        \
+    {"configure-without-carrier", YAML_SCALAR_NODE, handle_netdef_bool, NULL, netdef_offset(configure_without_carrier)}, \


Suggested change

{"configure-without-carrier", YAML_SCALAR_NODE, handle_netdef_bool, NULL, netdef_offset(configure_without_carrier)}, \

{"ignore-carrier", YAML_SCALAR_NODE, handle_netdef_bool, NULL, netdef_offset(ignore_carrier)}, \

slyon · 2021-09-08T10:28:48Z

src/parse.h

@@ -401,6 +401,9 @@ struct net_definition {

    /* netplan-feature: activation-mode */
    char* activation_mode;  
+
+    /* carrier */
+    gboolean configure_without_carrier;


Suggested change

gboolean configure_without_carrier;

gboolean ignore_carrier;

…arrier

n-cc · 2021-09-14T14:08:34Z

Thanks for the help getting this merged! This will hopefully benefit a lot of use-cases that require IP addresses set on inactive links.

We should be able to generated drop-in configs for NetworkManager in /run/NetworkManager/conf.d/XX-netplan-NETDEF.conf to contain the [device] section you mentioned. Would you be willing to look into this (as part of an additional PR)?

I can give it a shot at some point, but if I recall correctly, the change to get it implemented was a bit more involved, as the [device] section is not currently templated by netplan in any NetworkManager config files, whereas networkd support was essentially a one-line change.

Nate Collins added 2 commits June 24, 2021 16:14

Add support for networkd ConfigureWithoutCarrier option on physical l…

a86e29d

…inks

move gboolean definition to new section in src/parse.h

e8ee25b

slyon reviewed Jun 25, 2021

View reviewed changes

src/parse.h Outdated Show resolved Hide resolved

move configure_without_carrier to end of net_definition struct; updat…

00ab93f

…e documentation phrasing; add test coverage

n-cc marked this pull request as ready for review June 25, 2021 16:08

slyon self-requested a review July 1, 2021 14:11

remove extra blank line

fa69790

slyon added the schema change This PR includes a change to netplan's YAML schema, which needs sign-off label Jul 5, 2021

slyon requested a review from vorlonofportland September 7, 2021 10:41

vorlonofportland requested changes Sep 8, 2021

View reviewed changes

update ignore-carrier wording

4805725

slyon approved these changes Sep 8, 2021

View reviewed changes

slyon added 2 commits September 8, 2021 13:03

Merge remote-tracking branch 'origin/main' into ncc/configurewithoutc…

a4c27ba

…arrier

test:common: fixup merging of gateway tests

4d5df9b

slyon merged commit d4884cf into canonical:main Sep 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for networkd ConfigureWithoutCarrier option on interfaces #215

Add support for networkd ConfigureWithoutCarrier option on interfaces #215

n-cc commented Jun 24, 2021 •

edited

Loading

codecov-commenter commented Jun 25, 2021 •

edited

Loading

slyon commented Jul 5, 2021

n-cc commented Jul 6, 2021

vorlonofportland Sep 8, 2021

slyon left a comment •

edited

Loading

slyon Sep 8, 2021

slyon Sep 8, 2021

slyon Sep 8, 2021

n-cc commented Sep 14, 2021

	``configure-without-carrier`` (bool)
	``ignore-carrier`` (bool) – since *0.104

	{"configure-without-carrier", YAML_SCALAR_NODE, handle_netdef_bool, NULL, netdef_offset(configure_without_carrier)}, \
	{"ignore-carrier", YAML_SCALAR_NODE, handle_netdef_bool, NULL, netdef_offset(ignore_carrier)}, \

Add support for networkd ConfigureWithoutCarrier option on interfaces #215

Add support for networkd ConfigureWithoutCarrier option on interfaces #215

Conversation

n-cc commented Jun 24, 2021 • edited Loading

Description

Checklist

codecov-commenter commented Jun 25, 2021 • edited Loading

Codecov Report

slyon commented Jul 5, 2021

n-cc commented Jul 6, 2021

vorlonofportland Sep 8, 2021

Choose a reason for hiding this comment

slyon left a comment • edited Loading

Choose a reason for hiding this comment

slyon Sep 8, 2021

Choose a reason for hiding this comment

slyon Sep 8, 2021

Choose a reason for hiding this comment

slyon Sep 8, 2021

Choose a reason for hiding this comment

n-cc commented Sep 14, 2021

n-cc commented Jun 24, 2021 •

edited

Loading

codecov-commenter commented Jun 25, 2021 •

edited

Loading

slyon left a comment •

edited

Loading