Skip to content

Commit

Permalink
libnetwork/netavark: allow same bridge name with different vlan
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
Luap99 committed Jan 30, 2025
1 parent fa339b6 commit 4da12d7
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 11 deletions.
2 changes: 1 addition & 1 deletion libnetwork/cni/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
10 changes: 6 additions & 4 deletions libnetwork/internal/util/bridge.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
21 changes: 15 additions & 6 deletions libnetwork/netavark/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down
21 changes: 21 additions & 0 deletions libnetwork/netavark/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down

0 comments on commit 4da12d7

Please sign in to comment.