Skip to content
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

Jump to bottom

Macvlan internal network should not change default gateway #2407

Merged
merged 3 commits into from
Feb 26, 2020
Merged

Macvlan internal network should not change default gateway #2407

merged 3 commits into from
Feb 26, 2020

Conversation

lemrouch
Copy link
Contributor

Since docker container can be connected to combination of several
internal and external networks change of default gateway of the internal
ones breaks communication via the external ones.

This fixes only macvlan network type

Signed-off-by: Pavel Matěja [email protected]

This should close #2406

lemrouch added 2 commits June 26, 2019 14:23
@lemrouch
Macvlan internal network shouldln't change gw
26d1d70 
Since docker container can be connected to combination of several
internal and external networks change of default gateway of the internal
ones breaks communication via the external ones.

This fixes only macvlan network type

Signed-off-by: Pavel Matěja <[email protected]>
@lemrouch
Log when endpoint joins internal macvlan network
e2c5ecb 
Signed-off-by: Pavel Matěja <[email protected]>
@lemrouch
Copy link
Contributor Author

lemrouch commented Aug 8, 2019

What can I do to get this commit merged?

@lemrouch lemrouch mentioned this pull request Feb 18, 2020
Merged
@arkodg
Copy link
Contributor

arkodg commented Feb 20, 2020

can you please make similar changes in https://github.com/docker/libnetwork/blob/6659f7f4d8c1ec7a412a33f8973423ad38d22982/drivers/ipvlan/ipvlan_joinleave.go#L53

@lemrouch
Ipvlan internal network should not change gw
d5f5553 
Since docker container can be connected to combination of several
internal and external networks change of default gateway of the internal
ones breaks communication via the external ones.

This fixes only ipvlan network type

Signed-off-by: Pavel Matěja <[email protected]>
@lemrouch
Copy link
Contributor Author

Done, can you check it pls?

@arkodg
Copy link
Contributor

arkodg commented Feb 21, 2020

will containers connected/attached to a single internal network (mtacvlan/overlay) in a swarm be able to communicate to each other ? (maybe they will since the the bridge connected to the veth might be on the same subnet, this should be tested and verified though :) )

@lemrouch
Copy link
Contributor Author

Yes it does work. Network example is taken from #2418 (comment)
Run container on 1st node and check it's ip:

root@apex1:~# docker run -it --rm --name pg_client --network db_net postgres:11 /bin/bash
root@apex1:~# nsenter --net=/proc/`docker inspect --format '{{.State.Pid}}' pg_client`/ns/net ip a s
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN group default qlen 1000
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
    inet 127.0.0.1/8 scope host lo
       valid_lft forever preferred_lft forever
25: eth0@if3: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default 
    link/ether 02:42:0a:40:31:02 brd ff:ff:ff:ff:ff:ff link-netnsid 0
    inet 10.64.49.2/24 brd 10.64.49.255 scope global eth0
       valid_lft forever preferred_lft forever

Run container on 2nd node and try to ping container on 1st node:

root@apex2:~# docker run -it --rm --name pg_client --network db_net postgres:11 /bin/bash
root@apex2:~# nsenter --net=/proc/`docker inspect --format '{{.State.Pid}}' pg_client`/ns/net ping -c 1 10.64.49.2
PING 10.64.49.2 (10.64.49.2) 56(84) bytes of data.
64 bytes from 10.64.49.2: icmp_seq=1 ttl=64 time=0.484 ms

--- 10.64.49.2 ping statistics ---
1 packets transmitted, 1 received, 0% packet loss, time 0ms
rtt min/avg/max/mdev = 0.484/0.484/0.484/0.000 ms

arkodg
arkodg approved these changes Feb 24, 2020
View reviewed changes
Copy link
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks !
ptal @selansen

selansen
selansen approved these changes Feb 26, 2020
View reviewed changes
Copy link
Collaborator

@selansen selansen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@arkodg arkodg merged commit 8d1f97e into moby:master Feb 26, 2020
@lemrouch lemrouch deleted the 2406-fix branch February 26, 2020 18:23
@lemrouch lemrouch mentioned this pull request Feb 27, 2020
Closed
@thaJeztah thaJeztah mentioned this pull request Feb 28, 2020
Merged
arkodg pushed a commit to arkodg/libnetwork that referenced this pull request Mar 3, 2020
Macvlan: Seperate empty parent and internal
de9a5d0 
moby#2419 and
moby#2407
attempted to seperate out empty parent and internal for
macvlan and ipvlan networks

However it didnt pass the integration tests in moby
moby/moby#40596 and exposed some
more plumbing that needed to be done to make sure
we seperate the two things

If the -o parent is empty we create a dummylink
and if internal is set we dont add a default gateway
and make sure north-south communication cannot take place
(only east-west / container-container can)

Signed-off-by: Arko Dasgupta <[email protected]>
@arkodg arkodg mentioned this pull request Mar 3, 2020
Merged
arkodg pushed a commit to arkodg/libnetwork that referenced this pull request Mar 4, 2020
Macvlan: Separate empty parent and internal
0acbc1b 
moby#2419 and
moby#2407
attempted to seperate out empty parent and internal for
macvlan and ipvlan networks

However it didnt pass the integration tests in moby
moby/moby#40596 and exposed some
more plumbing that needed to be done to make sure
we seperate the two things

If the -o parent is empty we create a dummylink
and if internal is set we dont add a default gateway
and make sure north-south communication cannot take place
(only east-west / container-container can)

Signed-off-by: Arko Dasgupta <[email protected]>
arkodg pushed a commit to arkodg/libnetwork that referenced this pull request Mar 4, 2020
Macvlan: Separate empty parent and internal
3a4b08f 
moby#2419 and
moby#2407
attempted to seperate out empty parent and internal for
macvlan and ipvlan networks

However it didnt pass the integration tests in moby
moby/moby#40596 and exposed some
more plumbing that needed to be done to make sure
we separate the two things

If the -o parent is empty we create a dummylink
and if internal is set we dont add a default gateway
and make sure north-south communication cannot take place
(only east-west / container-container can)

Signed-off-by: Arko Dasgupta <[email protected]>
thaJeztah added a commit to thaJeztah/docker that referenced this pull request Mar 6, 2020
@thaJeztah
vendor: bump libnetwork bf2bd42abc0a3734f12b5ec724e571434e42c669
e1710b4 
full diff: moby/libnetwork@264bffc...bf2bd42

relevant changes:

- moby/libnetwork#2407 Macvlan internal network should not change default gateway
    - fixes moby/libnetwork#2406 Internal macvlan network overrides default gateway
- vendor godbus/dbus v5
- Fix InhibitIPv4 nil panic
- Cleanup VFP during overlay network removal
    - fixes VFP leak in windows overlay network deletion

Signed-off-by: Sebastiaan van Stijn <[email protected]>
docker-jenkins pushed a commit to docker-archive/docker-ce that referenced this pull request Mar 9, 2020
@thaJeztah
vendor: bump libnetwork bf2bd42abc0a3734f12b5ec724e571434e42c669
ab69e2c 
full diff: moby/libnetwork@264bffc...bf2bd42

relevant changes:

- moby/libnetwork#2407 Macvlan internal network should not change default gateway
    - fixes moby/libnetwork#2406 Internal macvlan network overrides default gateway
- vendor godbus/dbus v5
- Fix InhibitIPv4 nil panic
- Cleanup VFP during overlay network removal
    - fixes VFP leak in windows overlay network deletion

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Upstream-commit: e1710b42d3104e0c807dd670c260be48fdecc203
Component: engine
@SamWhited SamWhited mentioned this pull request Mar 11, 2020
Closed
tle211212 pushed a commit to tle211212/libnetwork that referenced this pull request Apr 6, 2020
Macvlan: Separate empty parent and internal
711d627 
moby#2419 and
moby#2407
attempted to seperate out empty parent and internal for
macvlan and ipvlan networks

However it didnt pass the integration tests in moby
moby/moby#40596 and exposed some
more plumbing that needed to be done to make sure
we separate the two things

If the -o parent is empty we create a dummylink
and if internal is set we dont add a default gateway
and make sure north-south communication cannot take place
(only east-west / container-container can)

Signed-off-by: Arko Dasgupta <[email protected]>
cpuguy83 pushed a commit to cpuguy83/docker that referenced this pull request May 25, 2021
Macvlan: Separate empty parent and internal
76b5905 
moby/libnetwork#2419 and
moby/libnetwork#2407
attempted to seperate out empty parent and internal for
macvlan and ipvlan networks

However it didnt pass the integration tests in moby
moby#40596 and exposed some
more plumbing that needed to be done to make sure
we separate the two things

If the -o parent is empty we create a dummylink
and if internal is set we dont add a default gateway
and make sure north-south communication cannot take place
(only east-west / container-container can)

Signed-off-by: Arko Dasgupta <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@selansen selansen selansen approved these changes

@arkodg arkodg arkodg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Internal macvlan network overrides default gateway
3 participants
@lemrouch @arkodg @selansen