Skip to content

Commit

Permalink
Merge pull request #2117 from AkihiroSuda/iso-carry
Browse files Browse the repository at this point in the history 
[Carry 1534] Improve scalabiltiy of bridge network isolation rules
  • Loading branch information
Flavio Crisciani committed Apr 2, 2018
2 parents dbba377 + 9be4c10 commit 5c1218c
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 107 deletions.
61 changes: 24 additions & 37 deletions drivers/bridge/bridge.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,15 +137,16 @@ 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
configNetwork sync.Mutex
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
configNetwork sync.Mutex
sync.Mutex
}

Expand Down Expand Up @@ -266,15 +267,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 @@ -311,33 +312,18 @@ 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
}
}
}

return nil
// Install the rules to isolate this network against each of the other networks
return setINC(thisConfig.BridgeName, enable)
}

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 @@ -365,7 +351,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 @@ -384,7 +370,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
58 changes: 28 additions & 30 deletions drivers/bridge/bridge_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -425,69 +425,67 @@ 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(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++
}
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 @@ -996,9 +994,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

0 comments on commit 5c1218c

Please sign in to comment.