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

add test for PMTU discovery and packet fragmentation #1247

Merged
merged 1 commit into from
Aug 10, 2015
Merged

Conversation

rade
Copy link
Member

@rade rade commented Jul 30, 2015

All credit goes to @dpw for figuring out how to break the kernel's packet fragmentation:

'man iptables' documets a --fragment option, which looks ideal for this purpose: It matches non-initial fragments.

But if you try to use this, you will find that, mysteriously, a rule using --fragment never seems to trigger.

The reason is that if the conntrack module is loaded, then iptables always reassembles packets before evaluating iptables rules (even rules that don't involve conntrack in any way). So the rules never see any fragments. And of course, if you have started docker, you have conntrack loaded.

So that rules out dropping non-initial fragments with iptables. But it is possible to get a similar effect by crafting an iptables rule that drops any UDP packet larger than the MTU:

iptables -I FORWARD -m length -p udp '!' --length 0:$[1500-28] -j DROP

(The --length argument is the size range of the UDP payload. So 1500 is the MTU, and 28 is the IP+UDP header size.)

Coverage increases as follows:

$ diff test/cover-before/coverage.txt test/cover-after/coverage.txt
371c371
< github.com/weaveworks/weave/router/forwarder.go:78:           forward             60.0%

---
> github.com/weaveworks/weave/router/forwarder.go:78:           forward             80.0%
373c373
< github.com/weaveworks/weave/router/forwarder.go:140:          fragment            0.0%

---
> github.com/weaveworks/weave/router/forwarder.go:140:          fragment            90.9%
379c379
< github.com/weaveworks/weave/router/forwarder.go:253:          accumulateAndSendFrames     56.2%

---
> github.com/weaveworks/weave/router/forwarder.go:253:          accumulateAndSendFrames     68.8%
381c381
< github.com/weaveworks/weave/router/forwarder.go:288:          appendFrame         80.0%

---
> github.com/weaveworks/weave/router/forwarder.go:288:          appendFrame         100.0%
385c385
< github.com/weaveworks/weave/router/forwarder.go:346:          run             82.6%

---
> github.com/weaveworks/weave/router/forwarder.go:346:          run             100.0%
563c563
< total:                                    (statements)            83.1%

---
> total:                                    (statements)            84.2%

@rade rade force-pushed the test_frag branch 6 times, most recently from c5b79ed to 1aaeabf Compare July 30, 2015 21:14
@rade
Copy link
Member Author

rade commented Jul 31, 2015

I saw some failures on circle to start with. Quite reproducible there, though not 100% and not at all in my local setup. Dropping the message size from 65k to 10k appears to have solved this - it's done a dozen runs now, all but two of which succeeded, and both failures were due to #1243.

I suspect on gce the test goes over a real network (@tomwilkie correct me if I am wrong), plus there's a whole load of cloud machinery at work. Both of which increase the probability of some packet loss. Since the test forces a very low PMTU, we end up with a lot of fragments when the original packet is large, and if any one of them get lost then the packet will get dropped and the test will fail.

@tomwilkie
Copy link
Contributor

@rade yes we can't guarantee the VMs are on the same host, so the packets will have to traverse a real network.

exec_on $HOST2 c2 $PING -s 10000 c1 2>&1 1>/dev/null || true
assert_raises "exec_on $HOST2 c2 $PING -s 10000 c1"

run_on $HOST2 "sudo iptables -D $drop_large_udp_packets"

This comment was marked as abuse.

This comment was marked as abuse.

@rade
Copy link
Member Author

rade commented Aug 4, 2015

@dpw mind taking a look at this?

@dpw
Copy link
Contributor

dpw commented Aug 6, 2015

Seems reasonable. In some ways it might be nicer to figure out the true PMTU (e.g. with netcat and ip route get) between the two hosts and use that, for a pure test of the "no stack-frag" behaviour. On the other hand, the way it is here, we also get to test weave's PMTU discovery in the case where the PMTU value from the kernel doesn't work, and I suppose we aren't testing that elsewhere. In that case, it might be nice to also verify that weave arrived at the correct PMTU value (either by grepping the logs or with ping -M do -s <n>).

@rade
Copy link
Member Author

rade commented Aug 6, 2015

it might be nice to also verify that weave arrived at the correct PMTU value (either by grepping the logs or with ping -M do -s <n>)

I was thinking of that, but a) grepping the logs is horrible, and b) the busybox ping does not have -M :(

rade added a commit that referenced this pull request Aug 10, 2015
add test for PMTU discovery and packet fragmentation
@rade rade merged commit 8bce900 into master Aug 10, 2015
@rade rade modified the milestone: 1.1.0 Aug 12, 2015
@rade rade deleted the test_frag branch August 19, 2015 20:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants