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

multi-hop routing breaks pmtu discovery #2

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

multi-hop routing breaks pmtu discovery #2

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

Comments

@rade
Copy link
Member

rade commented Aug 19, 2014

I haven't actually tested this, but...

When a weave router forwards a packet to another peer, it uses the same logic for dealing with too big frames as if the packet had been captured from the local interface. In particular, an ICMP 3.4 packet will be injected on the interface. Now, if weave subsequently actually captured that same packet then things might sort of work, but a) it probably won't since we tell pcap to capture inbound packets only, and b) this is a rather round-about and inefficient way of handling pmtu discovery in the forwarding scenario.

Instead we should directly sent the generated ICMP 3.4 packet to the appropriate peer(s).

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

It did work for me but that was possibly before the pcap "inbound-only" changes.
With the conn-limit stuff, it's pretty easy to establish a linear chain of routers - thus not too hard to test this.
I do agree it's fairly round-about though. Routing it properly would be good.

@rade rade closed this as completed in 7a09d97 Aug 22, 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.
brb added a commit to brb/weave that referenced this issue Feb 2, 2016
brb added a commit to brb/weave that referenced this issue Feb 25, 2016
brb added a commit that referenced this issue Jul 12, 2016
- Move the MTU check to CreateDatapath function.
brb added a commit that referenced this issue Jul 12, 2016
- Move the MTU check to CreateDatapath function.
brb added a commit that referenced this issue Jul 14, 2016
- Move the MTU check to CreateDatapath function.
brb added a commit that referenced this issue Jul 14, 2016
- Move the MTU check to CreateDatapath function.
brb added a commit that referenced this issue Oct 19, 2016
- Move the MTU check to CreateDatapath function.
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