From 70e7acdb23715298624d123f7cc7c9dbcdf0465c Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 23 Nov 2020 11:41:40 +0100 Subject: Fix ip-range for classless subnet masks The `LastIPInSubnet` function worked only for classful subnet masks (e.g. /8, /16, /24). For non standard subnet masks this returned the wrong ip address. This works now for all subnet mask. A unit test is added to ensure this. Signed-off-by: Paul Holzinger --- libpod/network/subnet.go | 14 +++++----- libpod/network/subnet_test.go | 62 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 7 deletions(-) (limited to 'libpod/network') diff --git a/libpod/network/subnet.go b/libpod/network/subnet.go index 90f0cdfce..120038e57 100644 --- a/libpod/network/subnet.go +++ b/libpod/network/subnet.go @@ -54,14 +54,10 @@ func LastIPInSubnet(addr *net.IPNet) (net.IP, error) { //nolint:interfacer ones, bits := cidr.Mask.Size() if ones == bits { - return FirstIPInSubnet(cidr) + return cidr.IP, nil } - hostStart := ones / 8 - // Handle the first host byte - cidr.IP[hostStart] |= 0xff & cidr.Mask[hostStart] - // Fill the rest with ones - for i := hostStart; i < len(cidr.IP); i++ { - cidr.IP[i] = 0xff + for i := range cidr.IP { + cidr.IP[i] = cidr.IP[i] | ^cidr.Mask[i] } return cidr.IP, nil } @@ -73,6 +69,10 @@ func FirstIPInSubnet(addr *net.IPNet) (net.IP, error) { //nolint:interfacer if err != nil { return nil, err } + ones, bits := cidr.Mask.Size() + if ones == bits { + return cidr.IP, nil + } cidr.IP[len(cidr.IP)-1]++ return cidr.IP, nil } diff --git a/libpod/network/subnet_test.go b/libpod/network/subnet_test.go index 917c3be88..55b2443bd 100644 --- a/libpod/network/subnet_test.go +++ b/libpod/network/subnet_test.go @@ -33,3 +33,65 @@ func TestNextSubnet(t *testing.T) { }) } } + +func TestFirstIPInSubnet(t *testing.T) { + tests := []struct { + name string + args *net.IPNet + want net.IP + wantErr bool + }{ + {"class b", parseCIDR("192.168.0.0/16"), net.ParseIP("192.168.0.1"), false}, + {"class c", parseCIDR("192.168.1.0/24"), net.ParseIP("192.168.1.1"), false}, + {"cidr /23", parseCIDR("192.168.0.0/23"), net.ParseIP("192.168.0.1"), false}, + {"cidr /25", parseCIDR("192.168.1.0/25"), net.ParseIP("192.168.1.1"), false}, + {"cidr /26", parseCIDR("172.16.1.128/26"), net.ParseIP("172.16.1.129"), false}, + {"class a", parseCIDR("10.0.0.0/8"), net.ParseIP("10.0.0.1"), false}, + {"cidr /32", parseCIDR("192.168.255.4/32"), net.ParseIP("192.168.255.4"), false}, + {"cidr /31", parseCIDR("192.168.255.4/31"), net.ParseIP("192.168.255.5"), false}, + } + for _, tt := range tests { + test := tt + t.Run(test.name, func(t *testing.T) { + got, err := FirstIPInSubnet(test.args) + if (err != nil) != test.wantErr { + t.Errorf("FirstIPInSubnet() error = %v, wantErr %v", err, test.wantErr) + return + } + if !got.Equal(test.want) { + t.Errorf("FirstIPInSubnet() got = %v, want %v", got, test.want) + } + }) + } +} + +func TestLastIPInSubnet(t *testing.T) { + tests := []struct { + name string + args *net.IPNet + want net.IP + wantErr bool + }{ + {"class b", parseCIDR("192.168.0.0/16"), net.ParseIP("192.168.255.255"), false}, + {"class c", parseCIDR("192.168.1.0/24"), net.ParseIP("192.168.1.255"), false}, + {"cidr /23", parseCIDR("192.168.0.0/23"), net.ParseIP("192.168.1.255"), false}, + {"cidr /25", parseCIDR("192.168.1.0/25"), net.ParseIP("192.168.1.127"), false}, + {"cidr /26", parseCIDR("172.16.1.128/26"), net.ParseIP("172.16.1.191"), false}, + {"class a", parseCIDR("10.0.0.0/8"), net.ParseIP("10.255.255.255"), false}, + {"cidr /32", parseCIDR("192.168.255.4/32"), net.ParseIP("192.168.255.4"), false}, + {"cidr /31", parseCIDR("192.168.255.4/31"), net.ParseIP("192.168.255.5"), false}, + } + for _, tt := range tests { + test := tt + t.Run(test.name, func(t *testing.T) { + got, err := LastIPInSubnet(test.args) + if (err != nil) != test.wantErr { + t.Errorf("LastIPInSubnet() error = %v, wantErr %v", err, test.wantErr) + return + } + if !got.Equal(test.want) { + t.Errorf("LastIPInSubnet() got = %v, want %v", got, test.want) + } + }) + } +} -- cgit v1.2.3-54-g00ecf From 9602e290de75bdd7fb5b42f6d36069dd19271735 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 24 Nov 2020 15:09:20 +0100 Subject: Fix custom mac address with a custom cni network The cni plugin `tuning` is required to set a custom mac address. This plugin is configured in the default cni config file which is packaged with podman but was not included the generated config form `podman network create`. Fixes #8385 Signed-off-by: Paul Holzinger --- libpod/network/config.go | 10 ++++++++++ libpod/network/create.go | 1 + libpod/network/netconflist.go | 7 +++++++ test/e2e/create_staticmac_test.go | 18 ++++++++++++++++++ 4 files changed, 36 insertions(+) (limited to 'libpod/network') diff --git a/libpod/network/config.go b/libpod/network/config.go index ce8a4446c..ce351129e 100644 --- a/libpod/network/config.go +++ b/libpod/network/config.go @@ -129,6 +129,16 @@ func (f FirewallConfig) Bytes() ([]byte, error) { return json.MarshalIndent(f, "", "\t") } +// TuningConfig describes the tuning plugin +type TuningConfig struct { + PluginType string `json:"type"` +} + +// Bytes outputs the configuration as []byte +func (f TuningConfig) Bytes() ([]byte, error) { + return json.MarshalIndent(f, "", "\t") +} + // DNSNameConfig describes the dns container name resolution plugin config type DNSNameConfig struct { PluginType string `json:"type"` diff --git a/libpod/network/create.go b/libpod/network/create.go index 387f4fcd3..7e4fc574a 100644 --- a/libpod/network/create.go +++ b/libpod/network/create.go @@ -176,6 +176,7 @@ func createBridge(name string, options entities.NetworkCreateOptions, runtimeCon plugins = append(plugins, bridge) plugins = append(plugins, NewPortMapPlugin()) plugins = append(plugins, NewFirewallPlugin()) + plugins = append(plugins, NewTuningPlugin()) // if we find the dnsname plugin or are rootless, we add configuration for it // the rootless-cni-infra container has the dnsname plugin always installed if (HasDNSNamePlugin(runtimeConfig.Network.CNIPluginDirs) || rootless.IsRootless()) && !options.DisableDNS { diff --git a/libpod/network/netconflist.go b/libpod/network/netconflist.go index 111f1715c..ee9adce14 100644 --- a/libpod/network/netconflist.go +++ b/libpod/network/netconflist.go @@ -119,6 +119,13 @@ func NewFirewallPlugin() FirewallConfig { } } +// NewTuningPlugin creates a generic tuning section +func NewTuningPlugin() TuningConfig { + return TuningConfig{ + PluginType: "tuning", + } +} + // NewDNSNamePlugin creates the dnsname config with a given // domainname func NewDNSNamePlugin(domainName string) DNSNameConfig { diff --git a/test/e2e/create_staticmac_test.go b/test/e2e/create_staticmac_test.go index adffdc1ca..1ac431da2 100644 --- a/test/e2e/create_staticmac_test.go +++ b/test/e2e/create_staticmac_test.go @@ -5,6 +5,7 @@ import ( "github.com/containers/podman/v2/pkg/rootless" . "github.com/containers/podman/v2/test/utils" + "github.com/containers/storage/pkg/stringid" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) @@ -45,4 +46,21 @@ var _ = Describe("Podman run with --mac-address flag", func() { Expect(result.OutputToString()).To(ContainSubstring("92:d0:c6:0a:29:34")) } }) + + It("Podman run --mac-address with custom network", func() { + net := "n1" + stringid.GenerateNonCryptoID() + session := podmanTest.Podman([]string{"network", "create", net}) + session.WaitWithDefaultTimeout() + defer podmanTest.removeCNINetwork(net) + Expect(session.ExitCode()).To(BeZero()) + + result := podmanTest.Podman([]string{"run", "--network", net, "--mac-address", "92:d0:c6:00:29:34", ALPINE, "ip", "addr"}) + result.WaitWithDefaultTimeout() + if rootless.IsRootless() { + Expect(result.ExitCode()).To(Equal(125)) + } else { + Expect(result.ExitCode()).To(Equal(0)) + Expect(result.OutputToString()).To(ContainSubstring("92:d0:c6:00:29:34")) + } + }) }) -- cgit v1.2.3-54-g00ecf From 2f7bca0685c611d118db98f4c1a5f06370a9e286 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 26 Nov 2020 17:15:06 +0100 Subject: Fix problems with network remove First, make sure we are only trying to remove the network interface if we are root. Second, if we cannot get the interface name (e.g macvlan config) then we should not fail. Just remove the config file. Signed-off-by: Paul Holzinger --- libpod/network/files.go | 5 ++++- libpod/network/network.go | 28 +++++++++++++++++----------- test/e2e/network_test.go | 11 +++++++++++ 3 files changed, 32 insertions(+), 12 deletions(-) (limited to 'libpod/network') diff --git a/libpod/network/files.go b/libpod/network/files.go index 846e5c62d..7f1e3ee18 100644 --- a/libpod/network/files.go +++ b/libpod/network/files.go @@ -14,6 +14,9 @@ import ( "github.com/pkg/errors" ) +// ErrNoSuchNetworkInterface indicates that no network interface exists +var ErrNoSuchNetworkInterface = errors.New("unable to find interface name for network") + // GetCNIConfDir get CNI configuration directory func GetCNIConfDir(configArg *config.Config) string { if len(configArg.Network.NetworkConfigDir) < 1 { @@ -142,7 +145,7 @@ func GetInterfaceNameFromConfig(path string) (string, error) { } } if len(name) == 0 { - return "", errors.New("unable to find interface name for network") + return "", ErrNoSuchNetworkInterface } return name, nil } diff --git a/libpod/network/network.go b/libpod/network/network.go index 7327a1a7d..0febb52f6 100644 --- a/libpod/network/network.go +++ b/libpod/network/network.go @@ -10,6 +10,7 @@ import ( "github.com/containernetworking/plugins/plugins/ipam/host-local/backend/allocator" "github.com/containers/common/pkg/config" "github.com/containers/podman/v2/libpod/define" + "github.com/containers/podman/v2/pkg/rootless" "github.com/containers/podman/v2/pkg/util" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -181,21 +182,26 @@ func RemoveNetwork(config *config.Config, name string) error { // Before we delete the configuration file, we need to make sure we can read and parse // it to get the network interface name so we can remove that too interfaceName, err := GetInterfaceNameFromConfig(cniPath) - if err != nil { - return errors.Wrapf(err, "failed to find network interface name in %q", cniPath) - } - liveNetworkNames, err := GetLiveNetworkNames() - if err != nil { - return errors.Wrapf(err, "failed to get live network names") - } - if util.StringInSlice(interfaceName, liveNetworkNames) { - if err := RemoveInterface(interfaceName); err != nil { - return errors.Wrapf(err, "failed to delete the network interface %q", interfaceName) + if err == nil { + // Don't try to remove the network interface if we are not root + if !rootless.IsRootless() { + liveNetworkNames, err := GetLiveNetworkNames() + if err != nil { + return errors.Wrapf(err, "failed to get live network names") + } + if util.StringInSlice(interfaceName, liveNetworkNames) { + if err := RemoveInterface(interfaceName); err != nil { + return errors.Wrapf(err, "failed to delete the network interface %q", interfaceName) + } + } } + } else if err != ErrNoSuchNetworkInterface { + // Don't error if we couldn't find the network interface name + return err } // Remove the configuration file if err := os.Remove(cniPath); err != nil { - return errors.Wrapf(err, "failed to remove network configuration file %q", cniPath) + return errors.Wrap(err, "failed to remove network configuration") } return nil } diff --git a/test/e2e/network_test.go b/test/e2e/network_test.go index 139a90ac7..adcf74f7e 100644 --- a/test/e2e/network_test.go +++ b/test/e2e/network_test.go @@ -499,4 +499,15 @@ var _ = Describe("Podman network", func() { exec.WaitWithDefaultTimeout() Expect(exec.ExitCode()).To(BeZero()) }) + + It("podman network create/remove macvlan", func() { + net := "macvlan" + stringid.GenerateNonCryptoID() + nc := podmanTest.Podman([]string{"network", "create", "--macvlan", "lo", net}) + nc.WaitWithDefaultTimeout() + Expect(nc.ExitCode()).To(Equal(0)) + + nc = podmanTest.Podman([]string{"network", "rm", net}) + nc.WaitWithDefaultTimeout() + Expect(nc.ExitCode()).To(Equal(0)) + }) }) -- cgit v1.2.3-54-g00ecf