From b0fc9a6550db075df8289562d5e1accfcc4392f0 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 30 Jan 2025 19:25:57 +0100 Subject: [PATCH] libnetwork/netavark: allow same bridge name with different vlan When a vlan is used there should be no bridge name conflict check. It is totally valid to have the same bridge with different vlans in two configs and that is the intended use case. Fixes #2095 Signed-off-by: Paul Holzinger --- common/libnetwork/cni/config.go | 2 +- common/libnetwork/internal/util/bridge.go | 10 ++++++---- common/libnetwork/netavark/config.go | 21 +++++++++++++++------ common/libnetwork/netavark/config_test.go | 21 +++++++++++++++++++++ 4 files changed, 43 insertions(+), 11 deletions(-) diff --git a/common/libnetwork/cni/config.go b/common/libnetwork/cni/config.go index 43f3bfef46..9b45c1589c 100644 --- a/common/libnetwork/cni/config.go +++ b/common/libnetwork/cni/config.go @@ -86,7 +86,7 @@ func (n *cniNetwork) networkCreate(newNetwork *types.Network, defaultNet bool) ( switch newNetwork.Driver { case types.BridgeNetworkDriver: internalutil.MapDockerBridgeDriverOptions(newNetwork) - err = internalutil.CreateBridge(n, newNetwork, usedNetworks, n.defaultsubnetPools) + err = internalutil.CreateBridge(n, newNetwork, usedNetworks, n.defaultsubnetPools, true) if err != nil { return nil, err } diff --git a/common/libnetwork/internal/util/bridge.go b/common/libnetwork/internal/util/bridge.go index b75e9a57f4..44cd555c17 100644 --- a/common/libnetwork/internal/util/bridge.go +++ b/common/libnetwork/internal/util/bridge.go @@ -10,11 +10,13 @@ import ( "github.com/containers/common/pkg/config" ) -func CreateBridge(n NetUtil, network *types.Network, usedNetworks []*net.IPNet, subnetPools []config.SubnetPool) error { +func CreateBridge(n NetUtil, network *types.Network, usedNetworks []*net.IPNet, subnetPools []config.SubnetPool, checkBridgeConflict bool) error { if network.NetworkInterface != "" { - bridges := GetBridgeInterfaceNames(n) - if slices.Contains(bridges, network.NetworkInterface) { - return fmt.Errorf("bridge name %s already in use", network.NetworkInterface) + if checkBridgeConflict { + bridges := GetBridgeInterfaceNames(n) + if slices.Contains(bridges, network.NetworkInterface) { + return fmt.Errorf("bridge name %s already in use", network.NetworkInterface) + } } if !types.NameRegex.MatchString(network.NetworkInterface) { return fmt.Errorf("bridge name %s invalid: %w", network.NetworkInterface, types.RegexError) diff --git a/common/libnetwork/netavark/config.go b/common/libnetwork/netavark/config.go index 3305258b6c..4916f725e1 100644 --- a/common/libnetwork/netavark/config.go +++ b/common/libnetwork/netavark/config.go @@ -169,11 +169,9 @@ func (n *netavarkNetwork) networkCreate(newNetwork *types.Network, defaultNet bo switch newNetwork.Driver { case types.BridgeNetworkDriver: internalutil.MapDockerBridgeDriverOptions(newNetwork) - err = internalutil.CreateBridge(n, newNetwork, usedNetworks, n.defaultsubnetPools) - if err != nil { - return nil, err - } - // validate the given options, we do not need them but just check to make sure they are valid + + var vlan int + // validate the given options, for key, value := range newNetwork.Options { switch key { case types.MTUOption: @@ -183,7 +181,7 @@ func (n *netavarkNetwork) networkCreate(newNetwork *types.Network, defaultNet bo } case types.VLANOption: - _, err = internalutil.ParseVlan(value) + vlan, err = internalutil.ParseVlan(value) if err != nil { return nil, err } @@ -218,6 +216,17 @@ func (n *netavarkNetwork) networkCreate(newNetwork *types.Network, defaultNet bo return nil, fmt.Errorf("unsupported bridge network option %s", key) } } + + // If there is no vlan there should be no other config with the same bridge. + // However with vlan we want to allow that so that you can have different + // configs on the same bridge but different vlans + // https://github.com/containers/common/issues/2095 + checkBridgeConflict := vlan == 0 + err = internalutil.CreateBridge(n, newNetwork, usedNetworks, n.defaultsubnetPools, checkBridgeConflict) + if err != nil { + return nil, err + } + case types.MacVLANNetworkDriver, types.IPVLANNetworkDriver: err = createIpvlanOrMacvlan(newNetwork) if err != nil { diff --git a/common/libnetwork/netavark/config_test.go b/common/libnetwork/netavark/config_test.go index 835e322ecc..02b4ed3d77 100644 --- a/common/libnetwork/netavark/config_test.go +++ b/common/libnetwork/netavark/config_test.go @@ -913,6 +913,27 @@ var _ = Describe("Config", func() { Expect(err.Error()).To(ContainSubstring(`vlan ID -1 must be between 0 and 4094`)) }) + It("create two networks with vlan option and same bridge", func() { + networkOpts := types.Network{ + NetworkInterface: "br0", + Options: map[string]string{ + types.VLANOption: "5", + }, + } + network1, err := libpodNet.NetworkCreate(networkOpts, nil) + Expect(err).ToNot(HaveOccurred()) + Expect(network1.Driver).To(Equal("bridge")) + Expect(network1.Options).To(HaveKeyWithValue("vlan", "5")) + + // set a new vlan + networkOpts.Options[types.VLANOption] = "99" + + network2, err := libpodNet.NetworkCreate(networkOpts, nil) + Expect(err).ToNot(HaveOccurred()) + Expect(network2.Driver).To(Equal("bridge")) + Expect(network2.Options).To(HaveKeyWithValue("vlan", "99")) + }) + It("create network with vrf option", func() { network := types.Network{ Options: map[string]string{