netdev - Re: [PATCH net 1/2] vlan: make vlan_dev_real

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: <51FF5E76.1090008@redhat.com>
Date:	Mon, 05 Aug 2013 10:12:38 +0200
From:	Nikolay Aleksandrov <nikolay@...hat.com>
To:	Veaceslav Falico <vfalico@...hat.com>
CC:	netdev@...r.kernel.org, davem@...emloft.net, fubar@...ibm.com,
	jhs@...atatu.com
Subject: Re: [PATCH net 1/2] vlan: make vlan_dev_real_dev work over stacked
 vlans

On 08/05/2013 08:58 AM, Veaceslav Falico wrote:
> On Sun, Aug 04, 2013 at 04:17:43AM +0200, Nikolay Aleksandrov wrote:
>> On 08/03/2013 10:07 PM, Nikolay Aleksandrov wrote:
<snip>
>> I have one question - why not make it possible to call vlan_dev_real_dev()
>> with any device (vlan/non-vlan) and make it return a real device in all
>> cases (e.g. if it's a non-vlan device simply return it) ?
>> In the terms of the code above - simply change:
>> struct net_device *ret = vlan_dev_priv(dev)->real_dev;
>> with
>> struct net_device *ret = dev;
>>
>> This way we'll be able to simplify calls like if (is_vlan_dev(dev)) (or the
>> flag check alternative of this) to just vlan_dev_real_dev(). The old call
>> sites of this function will still work properly.
> 
> I think it'll be a bit harder to understand the callers' code. Now it
> always has the logic "wait, we might be having a vlan here, so lets verify
> if it's really a vlan and if yes - get the real device". However with this
> approach it might look like "we *really* have a vlan here - so lets get the
> real device" (because of the function name - vlan_dev_real_dev) - which is
> wrong. And it doesn't really provide any speed/size improvements - so it's
> kind of useless.
> 
It's not used in fast paths, this suggestion was purely about saving some
lines and indentation levels. And you can still use it the old way if you
prefer it.

> So, unless a better function name can be found (which I can't come up with
> - dev_or_strip_vlan()? dev_devlanitize()? :) ) - I'd stay with the current
>  version. Though, again, I don't have a strong opinion on this.
It's all the same to me as well, I've wasted enough time on these simple
patches so I'd rather see them in (if accepted) than argue about semantics :-)

> 
>>
>> Nik

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html