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: <20130805065855.GF22756@redhat.com>
Date:	Mon, 5 Aug 2013 08:58:55 +0200
From:	Veaceslav Falico <vfalico@...hat.com>
To:	Nikolay Aleksandrov <nikolay@...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 Sun, Aug 04, 2013 at 04:17:43AM +0200, Nikolay Aleksandrov wrote:
>On 08/03/2013 10:07 PM, Nikolay Aleksandrov wrote:
>> From: Nikolay Aleksandrov <Nikolay Aleksandrov nikolay@...hat.com>
>>
>> Sometimes we might have stacked vlans on top of each other, and we're
>> interested in the first non-vlan real device on the path, so transform
>> vlan_dev_real_dev to go over the stacked vlans and extract the first
>> non-vlan device.
>>
>> Signed-off-by: Nikolay Aleksandrov <nikolay@...hat.com>
>> ---
>>  net/8021q/vlan_core.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
>> index 4a78c4d..6ee48aa 100644
>> --- a/net/8021q/vlan_core.c
>> +++ b/net/8021q/vlan_core.c
>> @@ -91,7 +91,12 @@ EXPORT_SYMBOL(__vlan_find_dev_deep);
>>
>>  struct net_device *vlan_dev_real_dev(const struct net_device *dev)
>>  {
>> -	return vlan_dev_priv(dev)->real_dev;
>> +	struct net_device *ret = vlan_dev_priv(dev)->real_dev;
>> +
>> +	while (is_vlan_dev(ret))
>> +		ret = vlan_dev_priv(ret)->real_dev;
>> +
>> +	return ret;
>>  }
>>  EXPORT_SYMBOL(vlan_dev_real_dev);
>>
>>
>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.

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.

>
>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