Skip to content

Improve scalabiltiy of bridge network isolation rules #1534

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
aboch wants to merge 1 commit into from
Closed

Improve scalabiltiy of bridge network isolation rules #1534

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 24 additions & 33 deletions drivers/bridge/bridge.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,14 +128,15 @@ type bridgeNetwork struct {
}

type driver struct {
config *configuration
network *bridgeNetwork
natChain *iptables.ChainInfo
filterChain *iptables.ChainInfo
isolationChain *iptables.ChainInfo
networks map[string]*bridgeNetwork
store datastore.DataStore
nlh *netlink.Handle
config *configuration
network *bridgeNetwork
natChain *iptables.ChainInfo
filterChain *iptables.ChainInfo
isolationChain1 *iptables.ChainInfo
isolationChain2 *iptables.ChainInfo
networks map[string]*bridgeNetwork
store datastore.DataStore
nlh *netlink.Handle
sync.Mutex
}

Expand Down Expand Up @@ -253,15 +254,15 @@ func (n *bridgeNetwork) registerIptCleanFunc(clean iptableCleanFunc) {
n.iptCleanFuncs = append(n.iptCleanFuncs, clean)
}

func (n *bridgeNetwork) getDriverChains() (*iptables.ChainInfo, *iptables.ChainInfo, *iptables.ChainInfo, error) {
func (n *bridgeNetwork) getDriverChains() (*iptables.ChainInfo, *iptables.ChainInfo, *iptables.ChainInfo, *iptables.ChainInfo, error) {
n.Lock()
defer n.Unlock()

if n.driver == nil {
return nil, nil, nil, types.BadRequestErrorf("no driver found")
return nil, nil, nil, nil, types.BadRequestErrorf("no driver found")
}

return n.driver.natChain, n.driver.filterChain, n.driver.isolationChain, nil
return n.driver.natChain, n.driver.filterChain, n.driver.isolationChain1, n.driver.isolationChain2, nil
}

func (n *bridgeNetwork) getNetworkBridgeName() string {
Expand Down Expand Up @@ -298,21 +299,9 @@ func (n *bridgeNetwork) isolateNetwork(others []*bridgeNetwork, enable bool) err
return nil
}

// Install the rules to isolate this networks against each of the other networks
for _, o := range others {
o.Lock()
otherConfig := o.config
o.Unlock()

if otherConfig.Internal {
continue
}

if thisConfig.BridgeName != otherConfig.BridgeName {
if err := setINC(thisConfig.BridgeName, otherConfig.BridgeName, enable); err != nil {
return err
}
}
// Install the rules to isolate this network against each of the other networks
if err := setINC(thisConfig.BridgeName, enable); err != nil {
return err
}

return nil
Expand Down Expand Up @@ -355,11 +344,12 @@ func (c *networkConfiguration) conflictsWithNetworks(id string, others []*bridge

func (d *driver) configure(option map[string]interface{}) error {
var (
config *configuration
err error
natChain *iptables.ChainInfo
filterChain *iptables.ChainInfo
isolationChain *iptables.ChainInfo
config *configuration
err error
natChain *iptables.ChainInfo
filterChain *iptables.ChainInfo
isolationChain1 *iptables.ChainInfo
isolationChain2 *iptables.ChainInfo
)

genericData, ok := option[netlabel.GenericData]
Expand Down Expand Up @@ -387,7 +377,7 @@ func (d *driver) configure(option map[string]interface{}) error {
}
}
removeIPChains()
natChain, filterChain, isolationChain, err = setupIPChains(config)
natChain, filterChain, isolationChain1, isolationChain2, err = setupIPChains(config)
if err != nil {
return err
}
Expand All @@ -406,7 +396,8 @@ func (d *driver) configure(option map[string]interface{}) error {
d.Lock()
d.natChain = natChain
d.filterChain = filterChain
d.isolationChain = isolationChain
d.isolationChain1 = isolationChain1
d.isolationChain2 = isolationChain2
d.config = config
d.Unlock()

Expand Down
61 changes: 31 additions & 30 deletions drivers/bridge/bridge_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,69 +422,70 @@ func TestCreateMultipleNetworks(t *testing.T) {
t.Fatalf("Failed to create bridge: %v", err)
}

verifyV4INCEntries(d.networks, t)

config2 := &networkConfiguration{BridgeName: "net_test_2"}
genericOption[netlabel.GenericData] = config2
if err := d.CreateNetwork("2", genericOption, nil, getIPv4Data(t, ""), nil); err != nil {
t.Fatalf("Failed to create bridge: %v", err)
}

// Verify the network isolation rules are installed, each network subnet should appear 2 times
verifyV4INCEntries(d.networks, 2, t)
verifyV4INCEntries(d.networks, t)

config3 := &networkConfiguration{BridgeName: "net_test_3"}
genericOption[netlabel.GenericData] = config3
if err := d.CreateNetwork("3", genericOption, nil, getIPv4Data(t, ""), nil); err != nil {
t.Fatalf("Failed to create bridge: %v", err)
}

// Verify the network isolation rules are installed, each network subnet should appear 4 times
verifyV4INCEntries(d.networks, 6, t)
verifyV4INCEntries(d.networks, t)

config4 := &networkConfiguration{BridgeName: "net_test_4"}
genericOption[netlabel.GenericData] = config4
if err := d.CreateNetwork("4", genericOption, nil, getIPv4Data(t, ""), nil); err != nil {
t.Fatalf("Failed to create bridge: %v", err)
}

// Now 6 times
verifyV4INCEntries(d.networks, 12, t)
verifyV4INCEntries(d.networks, t)

d.DeleteNetwork("1")
verifyV4INCEntries(d.networks, 6, t)
verifyV4INCEntries(d.networks, t)

d.DeleteNetwork("2")
verifyV4INCEntries(d.networks, 2, t)
verifyV4INCEntries(d.networks, t)

d.DeleteNetwork("3")
verifyV4INCEntries(d.networks, 0, t)
verifyV4INCEntries(d.networks, t)

d.DeleteNetwork("4")
verifyV4INCEntries(d.networks, 0, t)
verifyV4INCEntries(d.networks, t)
}

func verifyV4INCEntries(networks map[string]*bridgeNetwork, numEntries int, t *testing.T) {
out, err := iptables.Raw("-nvL", IsolationChain)
// Verify the network isolation rules are installed for each network
func verifyV4INCEntries(networks map[string]*bridgeNetwork, t *testing.T) {
out1, err := iptables.Raw("-S", IsolationChain1)
if err != nil {
t.Fatal(err)
}

found := 0
for _, x := range networks {
for _, y := range networks {
if x == y {
continue
}
re := regexp.MustCompile(fmt.Sprintf("%s %s", x.config.BridgeName, y.config.BridgeName))
matches := re.FindAllString(string(out[:]), -1)
if len(matches) != 1 {
t.Fatalf("Cannot find expected inter-network isolation rules in IP Tables:\n%s", string(out[:]))
}
found++
}
if err != nil {
t.Fatal(err)
}
out2, err := iptables.Raw("-S", IsolationChain2)
if err != nil {
t.Fatal(err)
}

if found != numEntries {
t.Fatalf("Cannot find expected number (%d) of inter-network isolation rules in IP Tables:\n%s\nFound %d", numEntries, string(out[:]), found)
for _, n := range networks {
re := regexp.MustCompile(fmt.Sprintf("-i %s ! -o %s -j %s", n.config.BridgeName, n.config.BridgeName, IsolationChain2))
matches := re.FindAllString(string(out1[:]), -1)
if len(matches) != 1 {
t.Fatalf("Cannot find expected inter-network isolation rules in IP Tables for network %s:\n%s.", n.id, string(out1[:]))
}
re = regexp.MustCompile(fmt.Sprintf("-o %s -j DROP", n.config.BridgeName))
matches = re.FindAllString(string(out2[:]), -1)
if len(matches) != 1 {
t.Fatalf("Cannot find expected inter-network isolation rules in IP Tables for network %s:\n%s.", n.id, string(out2[:]))
}
}
}

Expand Down Expand Up @@ -992,9 +993,9 @@ func TestCleanupIptableRules(t *testing.T) {
bridgeChain := []iptables.ChainInfo{
{Name: DockerChain, Table: iptables.Nat},
{Name: DockerChain, Table: iptables.Filter},
{Name: IsolationChain, Table: iptables.Filter},
{Name: IsolationChain1, Table: iptables.Filter},
}
if _, _, _, err := setupIPChains(&configuration{EnableIPTables: true}); err != nil {
if _, _, _, _, err := setupIPChains(&configuration{EnableIPTables: true}); err != nil {
t.Fatalf("Error setting up ip chains: %v", err)
}
for _, chainInfo := range bridgeChain {
Expand Down
Loading