PMTU Discovery

[Daniel Schall]

On Friday, January 07, 2011 1:25 PM, Guus Sliepen wrote:

> > > > Although PMTU Discovery works with compressed packets and some
"magic" 
> > > > to allow badly compressible packets as well, I think just sending 
> > > > uncompressed packets would be a lot easier and more reliable.
> > 
> > > You would still need to use the "magic" for badly compressible packets
> > > even if you send uncompressed MTU probes.
> > 
> > You do not need that magic if you add a flag to the packet, indicating
if
> > the packet is compressed or not.
>
> Ah yes, that is true.
> 
> > > You can not assume anything when a probe is not returned, this could
be
> > > because the probe was larger than the PMTU, but also because the
packet was
> > > dropped due to temporary congestion. If you could, then your method
would
> > > work fine.
> > 
> > You are right, I did not see that point.  Anyway, using rand() does seem
a
> > little indeterministic to me, that's why I was trying to introduce a
> > predictable mechanism.  I'll revise the code and see, if I can figure
out a
> > better (deterministic) algorithm to determine the proper probe size.
>
> The random sized probes usually converge very fast. Perhaps a better way
is to
> combine your approach with randomness: split the range between minmtu and
> maxmtu in three pieces, select a random number between 0 and (maxmtu -
minmtu)
> / 3, and send three packets with the minimum size of each piece + the
random
> number. This ensures the three probes always have very different sizes,
> allowing a fast binary search things are normal, and can still deal with
the
> case of firewalls + low MTU in a simple way.

That'd be nice.
Are you going to implement it?


> > > That also is unfortunately the wrong thing to do. The PMTU does not
need
> > > to be the same in both directions.
> > 
> > You are right about that.
> > But how does your code reflect the fact, that the maximum size of the
> > received PMTU probes does not necessarily reflect the MTU for sending?
> > To take that into account, my idea was to reply every PMTU packet with a
> > PMTU response packet, having the same size.
>
> That is exactly what the code does:
>
> > Another question regarding "mtu_probe_h":
> > Can you please explain what this code block does?
> > 	if(!packet->data[0]) {
> > 	    packet->data[0] = 1;
> > 	    send_udppacket(n, packet); 
> > 	} else {
>
> If it receives a probe packet with the first byte set to 0 (which is what
is
> generated by send_mtu_probe()), it sets the first byte to 1 and sends the
> packet straight back. If the first byte is non-zero, it assumes it is the
probe
> reply, and changes minmtu accordingly.

Now I get it!
You use data[0] as a flag the same way I use vpn_packet_t.flags.pmtur.

> > Another possibility would be to inform the node sending the PMTU packet
via
> > the meta connection about the successful reception of a  probe, so it
can
> > adjust its MTU.
>
> Yes, but again not in a backwards compatible way.

I did not know that you put that much effort in backward compatibility,I'll
try to keep that in mind for future changes.

> > The coding style is OK, the git-email commit is also fine.
> > But you should not combine unrelated changes in one commit.
> > If you accidentily did, you can undo the last commit with "git reset
HEAD^"
> > (this undoes the commit but doesn't throw away the changes you made to
the files),
> > and then commit the individual bits and pieces again.
> > Also, in most cases you should just remove lines instead of uncommenting
> > them, you can find them back in the git history anyway.
> 
> I know, and I am sorry. I'm still playing around with git and did not
figure
> out everything.
> Further, I did not intend my commit to be pushed to your repository, I'd
> rather have it as a discussion base to notify you about my ideas and
> changes.
> When you provide public git branches, I think it'd be easier to discuss
> changes.

No problem!

I have looked around for some simple ways to have people push changes to
their
own branches, and it seems gitolite can allow that in a relatively easy way.
I
would have to check whether the people that provide hosting for tinc-vpn.org
allow that though, as it involves adding an account where multiple people
would
have access to.

Another option is that you make a (bare) clone of your own copy of the tinc
git
repository in a publicly accessisble spot (for example, your own website),
and
push your changes into that repository.

I could also see if I can create a tinc project on github.

> > > > level has now a flag and you can combine log levels you want to log
by
> > > > ORing the according flags.
> > 
> > > Hm, I don't see that in the commit, only what you write below:
> > > > CTRL+C enables DEBUG_SCARY_THINGS similar to the old behavior.
> > 
> > Oops, sorry, the changes I mentioned were in a different commit I forgot
to
> > send you.  Please find it attached.  In this commit, I also changed the
log
> > time in console output to a human-readable format.
>
> That looks great. However, DEBUG_SCARY_THINGS is not completely the same
> anymore. This should become 64, and then I would just add a DEBUG_ALL =
~0.
> Apart from that I'll merge it :)

It thought DEBUG_SCARY_THINGS does exactly that: Log everything.
But if it is just another debug level, you are right of course.


Best,

Daniel