[PATCH V2 11/11] library: Add setup_tun() callback

[David Woodhouse]

On Wed, 2016-02-10 at 20:00 -0800, Kevin Cernekee wrote:
> 
> One thing that worries me: it looks like ip_info.mtu could potentially
> change on each DTLS reconnection?  That's probably the right thing to
> do if the device is roaming between wifi/mobile networks. 

Well, if it actually does roam between networks then won't the CSTP
connection end up being remade too? Perhaps the MTU would only change
each time the CSTP connection is re-established?

Not sure that distinction really helps though...

>  But the tun interface MTU then needs to be updated every time
> openconnect computes a new MTU - we don't want to keep getting 1341-
> byte packets from the openconnect computes a new MTU - we don't want
> to keep getting 1341-byte packets from the kernel if we just decided
> that the MTU should be lowered to 1300.

> Changing the tun interface MTU should be possible via set_tun_mtu() if
> we're running as root, but it isn't currently possible using the
> Android / Chrome OS / script-tun APIs.  Maybe the library API should
> add an mtu_changed callback, and if it is NULL, only perform MTU
> detection when !tun_is_up.

Yeah, that makes some sense. So we do MTU detection (if we can, quickly
enough) before calling your new setup_tun() callback, and then only do
MTU detection *again* if the mtu_changed() callback is set.

> Also, I'm not sure if this code is safe if ip_info.mtu is low when
> cstp_pkt is allocated, but higher when the buffer gets populated:
> 
>         int len = vpninfo->deflate_pkt_size ? : vpninfo->ip_info.mtu;
>         int payload_len;
> 
>         if (!vpninfo->cstp_pkt) {
>             vpninfo->cstp_pkt = malloc(sizeof(struct pkt) + len);
>             if (!vpninfo->cstp_pkt) {
>                 vpn_progress(vpninfo, PRG_ERR, _("Allocation failed\n"));
>                 break;
>             }
>         }

Hm, the idea was that we'd free it when the MTU changed. I thought I'd
checked that recently... will take another look.

If we're adding an mtu_changed() callback, then the function which
*invokes* that callback would also be a good place to collect things
like this, to ensure they really do happen when needed.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse at intel.com                              Intel Corporation

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 5691 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/openconnect-devel/attachments/20160308/a7dad569/attachment.bin>