Skip to content

Commit

Permalink
netdog: Set default accept-ra setting via config rather than sysctl
Browse files Browse the repository at this point in the history
Previously, we set the IPv6 accept-ra setting for the primary interface
via `systemd-sysctl` during the wicked install helper.  Setting this
sysctl via the helper can cause a race condition between the kernel and
wicked.  The kernel sets a flag indicating a router advertisement has
been received (`IF_RA_RCVD`), but only after it completes duplicate
address detection and decides whether to send a router solicitation.  If
the sysctl isn't set by the time duplicate address detection completes,
the solicitation doesn't happen and the `IF_RA_RCVD` flag doesn't get
set.  wicked uses this flag to decide whether or not to kick off the
state machine that handles DHCP6.

This change adds the accept-ra setting to the interface config if the
primary interface is set up via kernel command line.  The primary
interface is supplied via kernel command line for AWS and VMware
variants.  This change also removes the accept-ra setting from the
`systemd-sysctl` config.  Allowing wicked to manage this setting
eliminates any chance of races between sysctl/kernel/wicked.
  • Loading branch information
zmrow committed Jun 5, 2023
1 parent 91e2cc9 commit 5ed812d
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 6 deletions.
14 changes: 12 additions & 2 deletions sources/api/netdog/src/cli/generate_net_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ pub(crate) struct GenerateNetConfigArgs {}

/// Generate configuration for network interfaces.
pub(crate) fn run() -> Result<()> {
let mut from_cmd_line = false;
let maybe_net_config = if Path::exists(Path::new(OVERRIDE_NET_CONFIG_FILE)) {
net_config::from_path(OVERRIDE_NET_CONFIG_FILE).context(error::NetConfigParseSnafu {
path: OVERRIDE_NET_CONFIG_FILE,
Expand All @@ -26,6 +27,7 @@ pub(crate) fn run() -> Result<()> {
path: DEFAULT_NET_CONFIG_FILE,
})?
} else {
from_cmd_line = true;
net_config::from_command_line(KERNEL_CMDLINE).context(error::NetConfigParseSnafu {
path: KERNEL_CMDLINE,
})?
Expand All @@ -48,8 +50,16 @@ pub(crate) fn run() -> Result<()> {
remove_old_primary_interface()?;
write_primary_interface(&primary_interface)?;

let wicked_interfaces = net_config.as_wicked_interfaces();
for interface in wicked_interfaces {
let mut wicked_interfaces = net_config.as_wicked_interfaces();
for interface in &mut wicked_interfaces {
// The kernel command line is too limited to fully specify an interface's configuration;
// fix some defaults to match legacy behavior.
// Note: we only allow 1 interface to be listed via kernel command line, so this will only
// be added to a single interface
if from_cmd_line {
interface.accept_ra();
}

interface
.write_config_file()
.context(error::InterfaceConfigWriteSnafu)?;
Expand Down
5 changes: 1 addition & 4 deletions sources/api/netdog/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,13 +155,10 @@ where
// Note: The dash (-) preceding the "net..." variable assignment below is important; it
// ensures failure to set the variable for any reason will be logged, but not cause the sysctl
// service to fail
// Accept router advertisement (RA) packets even if IPv6 forwarding is enabled on interface
let ipv6_accept_ra = format!("-net.ipv6.conf.{}.accept_ra = 2", interface);
// Enable loose mode for reverse path filter
let ipv4_rp_filter = format!("-net.ipv4.conf.{}.rp_filter = 2", interface);

let mut output = String::new();
writeln!(output, "{}", ipv6_accept_ra).context(error::SysctlConfBuildSnafu)?;
writeln!(output, "{}", ipv4_rp_filter).context(error::SysctlConfBuildSnafu)?;

fs::write(path, output).context(error::SysctlConfWriteSnafu { path })?;
Expand All @@ -176,7 +173,7 @@ mod tests {
fn default_sysctls() {
let interface = "eno1";
let fake_file = tempfile::NamedTempFile::new().unwrap();
let expected = "-net.ipv6.conf.eno1.accept_ra = 2\n-net.ipv4.conf.eno1.rp_filter = 2\n";
let expected = "-net.ipv4.conf.eno1.rp_filter = 2\n";
write_interface_sysctl(interface, &fake_file).unwrap();
assert_eq!(std::fs::read_to_string(&fake_file).unwrap(), expected);
}
Expand Down

0 comments on commit 5ed812d

Please sign in to comment.