pkg/ipam: convert dots to slashes in interface names for sysctl #585
Conversation
pkg/ipam/ipam_linux.go
Outdated
| @@ -64,12 +65,16 @@ func ConfigureIface(ifName string, res *current.Result) error { | |||
| // Enabled IPv6 for loopback "lo" and the interface | |||
| // being configured | |||
| for _, iface := range [2]string{"lo", ifName} { | |||
| ipv6SysctlValueName := fmt.Sprintf(DisableIPv6SysctlTemplate, iface) | |||
| // Cannot have dots in interface name for sysctl, must replace by / | |||
| ipv6SysctlValueName := fmt.Sprintf(DisableIPv6SysctlTemplate, strings.ReplaceAll(iface, ".", "/")) | |||
There was a problem hiding this comment.
IMO, since dots and slashes can be interchanged in sysctl, is it better to make DisableIPv6SysctlTemplate to use slashes as separators?
There was a problem hiding this comment.
👍 for leaving dots in interface names and instead making template slash separated.
Also: https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/tree/lib/utils.c?id=1f420318bda3cc62156e89e1b56d60cc744b48ad#n827 - iproute2 is checking if interface does not have slash in the name.
There was a problem hiding this comment.
Also please take a look on our utils in cni lib.
There was a problem hiding this comment.
Also please take a look on our utils in cni lib.
Did you mean to suggest adding a check using utils.ValidateInterfaceName()? I believe such check is already performed in skel.pluginMain().
pkg/ipam/ipam_linux.go
Outdated
| @@ -64,12 +65,16 @@ func ConfigureIface(ifName string, res *current.Result) error { | |||
| // Enabled IPv6 for loopback "lo" and the interface | |||
| // being configured | |||
| for _, iface := range [2]string{"lo", ifName} { | |||
| ipv6SysctlValueName := fmt.Sprintf(DisableIPv6SysctlTemplate, iface) | |||
| // Cannot have dots in interface name for sysctl, must replace by / | |||
| ipv6SysctlValueName := fmt.Sprintf(DisableIPv6SysctlTemplate, strings.ReplaceAll(iface, ".", "/")) | |||
There was a problem hiding this comment.
👍 for leaving dots in interface names and instead making template slash separated.
Also: https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/tree/lib/utils.c?id=1f420318bda3cc62156e89e1b56d60cc744b48ad#n827 - iproute2 is checking if interface does not have slash in the name.
A dot is a valid character in interface names and is often used in the names of VLAN interfaces. The sysctl net.ipv6.conf.<ifname>.disable_ipv6 key path cannot use dots both in the ifname and as path separator. We switch to using / as key path separator so dots are allowed in the ifname. This works because sysctl.Sysctl() accepts key paths with either dots or slashes as separators. Also, print error message to stderr in case sysctl cannot be read instead of silently hiding the error. Signed-off-by: David Verbeiren <david.verbeiren@tessares.net>
dverbeir
force-pushed
the
ipam_sysctl_dot
branch
from
bcc9add to
9b09f16
Compare
|
/lgtm |
Since the sysctl key syntax already uses the dot as separator, dots in interface names that appear in sysctl keys, as is the case for the net.ipv6.conf..* entries, must be replaced by slashes.