Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

sub-nets break pmtu discovery #1

Closed
rade opened this issue Aug 19, 2014 · 1 comment
Closed

sub-nets break pmtu discovery #1

rade opened this issue Aug 19, 2014 · 1 comment
Labels

Comments

@rade
Copy link
Member

rade commented Aug 19, 2014

When we place application containers in a sub-net, say 10.0.1.[1..n]/24, with weave on 10.0.0.[1,2]/16, then the icmp 3.4 packets injected by weave don't make it to the containers since we set their src IP to that of weave, which is not in the app container sub-net. [it's actually not 100% clear why the packets don't make it through; after all, their dst IP is in the app container sub-net]

Setting the src IP to the original packet's dst IP (ditto for MACs) fixes that.

I've looked at the RFCs and cannot find anything that says what IP should be used as the source IP. I'm guessing normally, when packets are routed via gateways etc, it would indeed be the IP of the entity that creates the icmp 3.4 packet. But in case of weave, that entity is really hidden; it's not part of the application network and doesn't route to the application network; instead it captures and injects.

So IMO setting thr src IP of the icmp 3.4 packet to the dst IP of the original packet is actually the closest to being right.

@rade rade added the bug label Aug 19, 2014
@msackman
Copy link

On Tue, Aug 19, 2014 at 01:03:57PM +0100, Matthias Radestock wrote:

I've looked at the RFCs and cannot find anything that says what IP should be
used as the source IP. Is there a specific reason you chose the router IP
(which actually involves a fair bit of effort to discover)?

It seems rather disingenuous to lie. The ICMP is certainly coming from the router that cannot forward the packet any further. It's not coming from the destination.

I also searched for any guidance and found none. But lying seems a bad idea as you could potentially end up with lots of ICMP msgs appearing to come from the same destination, each successively lowering the PMTU. Which I agree, shouldn't be a problem, but I saw no good reason to lie about it.

I would suggest that a happier solution would be to add aliases to the weave NIC, each with an IP for the subnet in question. So then you'd have eth0:0, eth0:1 etc within weave.

@rade rade closed this as completed in 9dd15f4 Aug 20, 2014
rade added a commit that referenced this issue Nov 30, 2014
Golang locks aren't re-entrant. As result one can get deadlocks on
RWMutexes like this:

goroutine #1: attempt to acquire read lock -> success
goroutine #2: attempt to acquire write lock -> wait
goroutine #1: attempt to acquire read lock again -> wait

(a pending write lock prevents subsequent acquisitions of read locks)

Peer.HasPathTo could run into this since it could attempt to acquire
the read lock on a peer multiple times if previously it had
encountered that peer via an asymmetric connection.
tomwilkie pushed a commit to tomwilkie/weave that referenced this issue Mar 19, 2015
marccarre added a commit that referenced this issue Jan 6, 2017
If the machine coordinating the test runs does not have jq, then test #175 fails with:
> test #1 "weave_on test-0.lon1.weave-net report | jq -r '.Router.Targets[] | tostring' | sort" failed:
>     expected "test-1.lon1.weave-net
> test-2.lon1.weave-net"
>     got nothing
This change completely removes the dependency on jq to make the test more robust and avoid potential confusion.
marccarre added a commit that referenced this issue Jan 9, 2017
If the machine coordinating the test runs does not have jq, then test #175 fails with:
> test #1 "weave_on test-0.lon1.weave-net report | jq -r '.Router.Targets[] | tostring' | sort" failed:
>     expected "test-1.lon1.weave-net
> test-2.lon1.weave-net"
>     got nothing
This change completely removes the dependency on jq to make the test more robust and avoid potential confusion.
marccarre added a commit that referenced this issue Jan 9, 2017
If the machine coordinating the test runs does not have jq, then test #175 fails with:
> test #1 "weave_on test-0.lon1.weave-net report | jq -r '.Router.Targets[] | tostring' | sort" failed:
>     expected "test-1.lon1.weave-net
> test-2.lon1.weave-net"
>     got nothing
This change completely removes the dependency on jq to make the test more robust and avoid potential confusion.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants