[PATCH 3/3] Drop packets that are too large without dropping connection

[Nikolay Martynov]

2017-05-11 12:00 GMT-04:00 Daniel Lenski <dlenski at gmail.com>:
> On Wed, May 10, 2017 at 8:03 PM, Nikolay Martynov <mar.kolya at gmail.com> wrote:
>>
>> Sometimes server sends us packets that are larger than negotiated MTU.
>> Current implementation bails out in this case.
>> This patch just makes openconnect to drop such packets and continue.
>> It looks like data stream from VPN server is generally correct - with
>> exception of packet being too large, so we can continue parsing further
>> packets.
>>
>
> My concern here is that with some protocols the MTU is not known
> authoritatively, and must be estimated, and might be wrong.
>
> For example, in my fully-functional, though not yet merged, branch
> supporting GlobalProtect (https://github.com/dlenski/openconnect/)
> there is no way to request a specific MTU, and I have never seen a
> server that actually reports the MTU correctly.
  Please see my response below.

>
>> This improves connection stability.
>
> How so? What is the downside to accepting an unexpectedly large packet
> which nevertheless managed to make it across the VPN tunnel?

  This patch goes from current state which is 'drop connection if
there is packet larger than MTU in the tunnel' to the state 'drop
packet if it is larger than MTU, but keep connection' - this improves
connection stability. Somehow with tunnels that I've gotten with my
employer I often get packets in that are larger than MTU (and it looks
like they have DF flag set). I do not know exact cause of it - and I
do not have access to VPN equipment, unfortunately. But this patch
improves connection stability with this setup. Higher stacks figure
out MTU value and continue normally after first dropped packet.
'Official' client (pulse) seems to set up MTU to 1400 on interfaces it
creates - same as does openconnect.

  Obviously we can just inject large packets higher up the stack, but
I'm not exactly sure what sort of side effects would happen if we do
that. So I sort of tried to play safe here.

  I guess this code can be improved for cases when MTU is not
known/not known exactly - but unfortunately I do not have equipment to
test such changes. Change I've sent improves connection stability with
pulse vpn server.

  Please let me know if you would like me to improve that patch.

-- 
Martynov Nikolay.
Email: mar.kolya at gmail.com