netdev - Re: [PATCH RFC 1/3] net: Add dev_set_env_hdr

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: <5fc51b89-4ab2-f98e-6847-530f4ed4b434@lab.ntt.co.jp>
Date:   Wed, 28 Sep 2016 15:09:12 +0900
From:   Toshiaki Makita <makita.toshiaki@....ntt.co.jp>
To:     "Mintz, Yuval" <Yuval.Mintz@...ium.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Patrick McHardy <kaber@...sh.net>,
        Stephen Hemminger <stephen@...workplumber.org>,
        Vlad Yasevich <vyasevich@...il.com>,
        Jeff Kirsher <jeffrey.t.kirsher@...el.com>
Subject: Re: [PATCH RFC 1/3] net: Add dev_set_env_hdr_len to accept envelope
 frames

On 2016/09/27 23:23, Mintz, Yuval wrote:
>> +/* return envelope header length */
>> +static inline int netif_get_env_hdr_len(struct net_device *dev) {
>> +	if (dev->netdev_ops->ndo_set_env_hdr_len)
>> +		return dev->env_hdr_len;
>> +
>> +	if (netif_reduces_vlan_mtu(dev))
>> +		return 0;
>> +
>> +	return 4; /* VLAN_HLEN */
>> +}
> 

Thank you for taking a look at this.

> Why claim that `4' is the default?
> What's the benefit of propagating this value to userspace if
> driver doesn't actually support it?

I'm sorry, this is a leftover from previous draft when I was not aware
of type 2-2 drivers (that do not allow over-MTU-sized packets other than
vlan-tagged ones). Now I'm thinking we should not include
IFLA_ENV_HDR_LEN in response of GETLINK if it is not supported.
I'll delete current logic from next version.

> 
> Assuming this *is* a good default [meaning your analysis indicated
> basically every driver has at least so many octets reserved]
> perhaps we should consider initializing the env_hdr_len in some
> common function [e.g., in setup_ether() next to the mtu] to this value?

I actually tried it first and realized it is difficult, since so many
drivers use their own initializer instead of ether_setup(). Anyway, I'm
going not to expose envhdrlen when it is not supported.

Thanks,
Toshiaki Makita