Re: [PATCH net 1/2] vlan: make vlan_dev_real_dev work over stacked vlans

[Nikolay Aleksandrov <nikolay@redhat.com>]

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