Repository: moby/moby (language: go)

Issue:
[macvlan] Same parent and gateway for multiple networks
### Description

I'm remaking this from https://github.com/moby/libnetwork/issues/2384

It's still relevant to me (esp with ipv6 networking) and I'd be happy to try working on it, but wanted to check first if the solution proposed there had any issues.


Discussion / hints:
Some quick questions:

- Is the check there for technical reasons or guidance (preventing people from making common mistakes)?
- I couldn't find any documentation on macvlan, but on source said there can only be one macvlan with passthrough.  I didn't see any other restrictions.  Rather than check `CreatedSlaveLink` maybe it would be better to check if either interface sharing the parent are `passthru`?

Fix patch:
diff --git a/integration/internal/network/ops.go b/integration/internal/network/ops.go
index cc07e842fee1a..49f762ecae02a 100644
--- a/integration/internal/network/ops.go
+++ b/integration/internal/network/ops.go
@@ -45,6 +45,19 @@ func WithMacvlan(parent string) func(*network.CreateOptions) {
 	}
 }
 
+// WithMacvlanPassthru sets the network as macvlan with the specified parent in passthru mode
+func WithMacvlanPassthru(parent string) func(options *network.CreateOptions) {
+	return func(n *network.CreateOptions) {
+		n.Driver = "macvlan"
+		n.Options = map[string]string{
+			"macvlan_mode": "passthru",
+		}
+		if parent != "" {
+			n.Options["parent"] = parent
+		}
+	}
+}
+
 // WithIPvlan sets the network as ipvlan with the specified parent and mode
 func WithIPvlan(parent, mode string) func(*network.CreateOptions) {
 	return func(n *network.CreateOptions) {
diff --git a/integration/network/helpers.go b/integration/network/helpers.go
index 771a4b4cd87aa..cf823cf86de38 100644
--- a/integration/network/helpers.go
+++ b/integration/network/helpers.go
@@ -42,6 +42,15 @@ func LinkExists(ctx context.Context, t *testing.T, master string) {
 	testutil.RunCommand(ctx, "ip", "link", "show", master).Assert(t, icmd.Success)
 }
 
+// LinkDoesntExist verifies that a link doesn't exist
+func LinkDoesntExist(ctx context.Context, t *testing.T, master string) {
+	// verify the specified link doesn't exist, ip link show <link_name>.
+	testutil.RunCommand(ctx, "ip", "link", "show", master).Assert(t, icmd.Expected{
+		ExitCode: 1,
+		Err:      "does not exist",
+	})
+}
+
 // IsNetworkAvailable provides a comparison to check if a docker network is available
 func IsNetworkAvailable(ctx context.Context, c client.NetworkAPIClient, name string) cmp.Comparison {
 	return func() cmp.Result {
diff --git a/libnetwork/drivers/macvlan/macvlan_network.go b/libnetwork/drivers/macvlan/macvlan_network.go
index 75313dd834563..7413b3fb20bc9 100644
--- a/libnetwork/drivers/macvlan/macvlan_network.go
+++ b/libnetwork/drivers/macvlan/macvlan_network.go
@@ -60,8 +60,20 @@ func (d *driver) createNetwork(config *configuration) (bool, error) {
 	for _, nw := range networkList {
 		if config.Parent == nw.config.Parent {
 			if config.ID != nw.config.ID {
-				return false, fmt.Errorf("network %s is already using parent interface %s",
-					getDummyName(stringid.TruncateID(nw.config.ID)), config.Parent)
+				if config.MacvlanMode == modePassthru {
+					return false, fmt.Errorf(
+						"cannot use mode passthru, macvlan network %s is already using parent interface %s",
+						stringid.TruncateID(nw.config.ID),
+						config.Parent,
+					)
+				} else if nw.config.MacvlanMode == modePassthru {
+					return false, fmt.Errorf(
+						"macvlan network %s is already using parent interface %s in mode passthru",
+						stringid.TruncateID(nw.config.ID),
+						config.Parent,
+					)
+				}
+				continue
 			}
 			log.G(context.TODO()).Debugf("Create Network for the same ID %s\n", config.ID)
 			foundExisting = true
@@ -90,6 +102,15 @@ func (d *driver) createNetwork(config *configuration) (bool, error) {
 			// if driver created the networks slave link, record it for future deletion
 			config.CreatedSlaveLink = true
 		}
+	} else {
+		// Check and mark this network if the interface was created for another network
+		networkList := d.getNetworks()
+		for _, testN := range networkList {
+			if config.Parent == testN.config.Parent && testN.config.CreatedSlaveLink {
+				config.CreatedSlaveLink = true
+				break
+			}
+		}
 	}
 	if !foundExisting {
 		n := &network{
@@ -105,30 +126,38 @@ func (d *driver) createNetwork(config *configuration) (bool, error) {
 	return foundExisting, nil
 }
 
+func (d *driver) parentHasSingleUser(n *network) bool {
+	users := 0
+	networkList := d.getNetworks()
+	for _, testN := range networkList {
+		if n.config.Parent == testN.config.Parent {
+			users += 1
+		}
+	}
+	return users == 1
+}
+
 // DeleteNetwork deletes the network for the specified driver type
 func (d *driver) DeleteNetwork(nid string) error {
 	n := d.network(nid)
 	if n == nil {
 		return fmt.Errorf("network id %s not found", nid)
 	}
-	// if the driver created the slave interface, delete it, otherwise leave it
-	if ok := n.config.CreatedSlaveLink; ok {
-		// if the interface exists, only delete if it matches iface.vlan or dummy.net_id naming
-		if ok := parentExists(n.config.Parent); ok {
-			// only delete the link if it is named the net_id
-			if n.config.Parent == getDummyName(stringid.TruncateID(nid)) {
-				err := delDummyLink(n.config.Parent)
-				if err != nil {
-					log.G(context.TODO()).Debugf("link %s was not deleted, continuing the delete network operation: %v",
-						n.config.Parent, err)
-				}
-			} else {
-				// only delete the link if it matches iface.vlan naming
-				err := delVlanLink(n.config.Parent)
-				if err != nil {
-					log.G(context.TODO()).Debugf("link %s was not deleted, continuing the delete network operation: %v",
-						n.config.Parent, err)
-				}
+	// if the driver created the slave interface and this network is the last user, delete it, otherwise leave it
+	if n.config.CreatedSlaveLink && parentExists(n.config.Parent) && d.parentHasSingleUser(n) {
+		// only delete the link if it is named the net_id
+		if n.config.Parent == getDummyName(stringid.TruncateID(nid)) {
+			err := delDummyLink(n.config.Parent)
+			if err != nil {
+				log.G(context.TODO()).Debugf("link %s was not deleted, continuing the delete network operation: %v",
+					n.config.Parent, err)
+			}
+		} else {
+			// only delete the link if it matches iface.vlan naming
+			err := delVlanLink(n.config.Parent)
+			if err != nil {
+				log.G(context.TODO()).Debugf("link %s was not deleted, continuing the delete network operation: %v",
+					n.config.Parent, err)
 			}
 		}
 	}


Test patch:
diff --git a/integration/network/macvlan/macvlan_test.go b/integration/network/macvlan/macvlan_test.go
index 538cdf0fb188c..929f785505007 100644
--- a/integration/network/macvlan/macvlan_test.go
+++ b/integration/network/macvlan/macvlan_test.go
@@ -62,6 +62,18 @@ func TestDockerNetworkMacvlan(t *testing.T) {
 		}, {
 			name: "OverlapParent",
 			test: testMacvlanOverlapParent,
+		}, {
+			name: "OverlapParentPassthruFirst",
+			test: testMacvlanOverlapParentPassthruFirst,
+		}, {
+			name: "OverlapParentPassthruSecond",
+			test: testMacvlanOverlapParentPassthruSecond,
+		}, {
+			name: "OverlapDeleteCreatedSecond",
+			test: testMacvlanOverlapDeleteCreatedSecond,
+		}, {
+			name: "OverlapKeepExistingParent",
+			test: testMacvlanOverlapKeepExisting,
 		}, {
 			name: "NilParent",
 			test: testMacvlanNilParent,
@@ -99,7 +111,8 @@ func TestDockerNetworkMacvlan(t *testing.T) {
 }
 
 func testMacvlanOverlapParent(t *testing.T, ctx context.Context, client client.APIClient) {
-	// verify the same parent interface cannot be used if already in use by an existing network
+	// verify the same parent interface can be used if already in use by an existing network
+	// as long as neither are passthru
 	master := "dm-dummy0"
 	n.CreateMasterDummy(ctx, t, master)
 	defer n.DeleteInterface(ctx, t, master)
@@ -110,6 +123,42 @@ func testMacvlanOverlapParent(t *testing.T, ctx context.Context, client client.A
 		net.WithMacvlan(parentName),
 	)
 	assert.Check(t, n.IsNetworkAvailable(ctx, client, netName))
+	n.LinkExists(ctx, t, parentName)
+
+	overlapNetName := "dm-parent-net-overlap"
+	_, err := net.Create(ctx, client, overlapNetName,
+		net.WithMacvlan(parentName),
+	)
+	assert.Check(t, err == nil)
+
+	// delete the second network while preserving the parent link
+	err = client.NetworkRemove(ctx, overlapNetName)
+	assert.NilError(t, err)
+	assert.Check(t, n.IsNetworkNotAvailable(ctx, client, overlapNetName))
+	n.LinkExists(ctx, t, parentName)
+
+	// delete the first network
+	err = client.NetworkRemove(ctx, netName)
+	assert.NilError(t, err)
+	assert.Check(t, n.IsNetworkNotAvailable(ctx, client, netName))
+	n.LinkDoesntExist(ctx, t, parentName)
+
+	// verify the network delete did not delete the root link
+	n.LinkExists(ctx, t, master)
+}
+
+func testMacvlanOverlapParentPassthruFirst(t *testing.T, ctx context.Context, client client.APIClient) {
+	// verify creating a second interface sharing a parent with another passthru interface is rejected
+	master := "dm-dummy0"
+	n.CreateMasterDummy(ctx, t, master)
+	defer n.DeleteInterface(ctx, t, master)
+
+	netName := "dm-subinterface"
+	parentName := "dm-dummy0.40"
+	net.CreateNoError(ctx, t, client, netName,
+		net.WithMacvlanPassthru(parentName),
+	)
+	assert.Check(t, n.IsNetworkAvailable(ctx, client, netName))
 
 	_, err := net.Create(ctx, client, "dm-parent-net-overlap",
 		net.WithMacvlan(parentName),
@@ -125,6 +174,96 @@ func testMacvlanOverlapParent(t *testing.T, ctx context.Context, client client.A
 	n.LinkExists(ctx, t, master)
 }
 
+func testMacvlanOverlapParentPassthruSecond(t *testing.T, ctx context.Context, client client.APIClient) {
+	// verify creating a passthru interface sharing a parent with another interface is rejected
+	master := "dm-dummy0"
+	n.CreateMasterDummy(ctx, t, master)
+	defer n.DeleteInterface(ctx, t, master)
+
+	netName := "dm-subinterface"
+	parentName := "dm-dummy0.40"
+	net.CreateNoError(ctx, t, client, netName,
+		net.WithMacvlan(parentName),
+	)
+	assert.Check(t, n.IsNetworkAvailable(ctx, client, netName))
+
+	_, err := net.Create(ctx, client, "dm-parent-net-overlap",
+		net.WithMacvlanPassthru(parentName),
+	)
+	assert.Check(t, err != nil)
+
+	// delete the network while preserving the parent link
+	err = client.NetworkRemove(ctx, netName)
+	assert.NilError(t, err)
+
+	assert.Check(t, n.IsNetworkNotAvailable(ctx, client, netName))
+	// verify the network delete did not delete the predefined link
+	n.LinkExists(ctx, t, master)
+}
+
+func testMacvlanOverlapDeleteCreatedSecond(t *testing.T, ctx context.Context, client client.APIClient) {
+	// verify that a shared created parent interface is kept when the original interface is deleted first
+	master := "dm-dummy0"
+	n.CreateMasterDummy(ctx, t, master)
+	defer n.DeleteInterface(ctx, t, master)
+
+	netName := "dm-subinterface"
+	parentName := "dm-dummy0.40"
+	net.CreateNoError(ctx, t, client, netName,
+		net.WithMacvlan(parentName),
+	)
+	assert.Check(t, n.IsNetworkAvailable(ctx, client, netName))
+
+	overlapNetName := "dm-parent-net-overlap"
+	_, err := net.Create(ctx, client, overlapNetName,
+		net.WithMacvlan(parentName),
+	)
+	assert.Check(t, err == nil)
+
+	// delete the original network while preserving the parent link
+	err = client.NetworkRemove(ctx, netName)
+	assert.NilError(t, err)
+	assert.Check(t, n.IsNetworkNotAvailable(ctx, client, netName))
+	n.LinkExists(ctx, t, parentName)
+
+	// delete the second network
+	err = client.NetworkRemove(ctx, overlapNetName)
+	assert.NilError(t, err)
+	assert.Check(t, n.IsNetworkNotAvailable(ctx, client, overlapNetName))
+	n.LinkDoesntExist(ctx, t, parentName)
+
+	// verify the network delete did not delete the root link
+	n.LinkExists(ctx, t, master)
+}
+
+func testMacvlanOverlapKeepExisting(t *testing.T, ctx context.Context, client client.APIClient) {
+	// verify that deleting interfaces sharing a previously existing parent doesn't delete the
+	// parent
+	master := "dm-dummy0"
+	n.CreateMasterDummy(ctx, t, master)
+	defer n.DeleteInterface(ctx, t, master)
+
+	netName := "dm-subinterface"
+	net.CreateNoError(ctx, t, client, netName,
+		net.WithMacvlan(master),
+	)
+	assert.Check(t, n.IsNetworkAvailable(ctx, client, netName))
+
+	overlapNetName := "dm-parent-net-overlap"
+	_, err := net.Create(ctx, client, overlapNetName,
+		net.WithMacvlan(master),
+	)
+	assert.Check(t, err == nil)
+
+	err = client.NetworkRemove(ctx, overlapNetName)
+	assert.NilError(t, err)
+	err = client.NetworkRemove(ctx, netName)
+	assert.NilError(t, err)
+
+	// verify the network delete did not delete the root link
+	n.LinkExists(ctx, t, master)
+}
+
 func testMacvlanSubinterface(t *testing.T, ctx context.Context, client client.APIClient) {
 	// verify the same parent interface cannot be used if already in use by an existing network
 	master := "dm-dummy0"