netdev - Re: [PATCH v3 net] net, ip6_tunnel: enhance tunnel locate with link check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cc378ec7-03ec-58ec-e3c9-158fb02b283e@6wind.com>
Date:   Thu, 13 Feb 2020 10:34:19 +0100
From:   Nicolas Dichtel <nicolas.dichtel@...nd.com>
To:     William Dauchy <w.dauchy@...teo.com>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH v3 net] net, ip6_tunnel: enhance tunnel locate with link
 check

Le 12/02/2020 à 22:06, William Dauchy a écrit :
> Hello Nicolas,
> 
> Thank you for your review.
> 
> On Wed, Feb 12, 2020 at 04:54:19PM +0100, Nicolas Dichtel wrote:
>> Hmm, I was expecting 'tdev->mtu - t_hlen'. Am I wrong?
>>
>> In fact, something like this:
>> dev->mtu = ETH_DATA_LEN - t_hlen;
>> if (t->parms.link) {
>> 	tdev = __dev_get_by_index(t->net, t->parms.link);
>> 	if (tdev)
>> 		dev->mtu = tdev->mtu - t_hlen;
>> }
> 
> true, I missed that one; I reworked to something like:
> 
> int mtu;
> 
> mtu = ETH_DATA_LEN;
> if (t->parms.link) {
> 	tdev = __dev_get_by_index(t->net, t->parms.link);
> 	if (tdev && tdev->mtu < mtu)
Why this second condition? Why not allowing more than ETH_DATA_LEN (1500)?
ip_tunnels do:
        if (tdev) {

                hlen = tdev->hard_header_len + tdev->needed_headroom;

                mtu = min(tdev->mtu, IP_MAX_MTU);

        }
which seems better.

Note also that you patch ip6_tnl_dev_init_gen(), but ip6_tnl_link_config() is
called later and may adjust the mtu. I would suggest to take care of link mtu in
ip6_tnl_link_config().

> 		mtu = tdev->mtu;
> }
> dev->mtu = mtu - t_hlen;
> 
> 
> However in ipip we do:
> 
> mtu -= (dev->hard_header_len + t_hlen);
> 
> Do I need to use hard_header_len as well?
hard_header_len is not set for ipv4 tunnels, but for ipv6 tunnels:
        dev->hard_header_len = LL_MAX_HEADER + t_hlen;

This is not the real value, I don't think you can calculate the real mtu based
on this.


Regards,
Nicolas