Re: [patch net-next-2.6] vlan: introduce ndo_vlan_[enable/disable]

[Jiri Pirko <jpirko@redhat.com>]

Sun, Jul 17, 2011 at 10:36:04AM CEST, mirqus@gmail.com wrote:
>W dniu 17 lipca 2011 09:30 użytkownik Jiri Pirko <jpirko@redhat.com> napisał:
>> Sat, Jul 16, 2011 at 04:14:36PM CEST, mirqus@gmail.com wrote:
>>>2011/7/16 Jiri Pirko <jpirko@redhat.com>:
>>>> Some devices are not able to enable/disable rx/tw vlan accel separately.
>>>> they depend on ndo_vlan_rx_register to know if to enable of disable
>>>> hw accel. And since ndo_vlan_rx_register is going to die soon,
>>>> this must be resolved.
>>>>
>>>> One solution might be to enable accel on device start every time, even
>>>> if there are no vlan up on. But this would change behaviour and might
>>>> lead to possible regression (on older devices).
>>>[...]
>>>
>>>Please describe the possible regression. As I see it, there won't be
>>>any user visible change of behaviour - network code takes care of
>>>reinserting VLAN tag when necessary. If you think that disabling tag
>>>stripping is beneficial for cases where no VLANs are configured, it's
>>>better to do that in netdev_fix_features() for devices which advertise
>>>NETIF_F_HW_VLAN_RX in hw_features.
>>
>> Well I just wanted to preserve current behaviour which is that in many
>> drivers vlan accel is enabled only if some vid is registered upon the
>> device and it's disabled again when no vid is registered. I can see
>> no way to do this with current code after removing ndo_vlan_rx_register.
>>
>> I expect unexpected
>
>:-D
>
>> ... problems on old cards when vlan accel would be
>> enabled all the time, but maybe I'm wrong...
>
>Device has no way of knowing how the system uses VLAN tags, stripped
>or not. Any problems would be driver problems and since you're making
>it all use generic code, bugs will hit all drivers simultaneously or
>(preferably) won't happen at all.
>
>> One idea is for device which do not support sepatate rx/tx vlan accel
>> enabling/disabling they can probably use ndo_fix_features force to
>> enable/disable rx/tx pair together. That would resolve the situation as
>> well giving user possibility to turn off vlan accel in case of any issues.
>
>That is exactly the idea behind ndo_fix_features.

In netdev_fix_features add check if either one of NETIF_F_HW_VLAN_TX or
NETIF_F_HW_VLAN_RX is set and in that case set the other one. Of course
this would be done only for devices what do not support separate rx/tx
vlan on/off. But how to distinguish? NETIF_F_HW_VLAN_BOTH feature flag?

Thanks.

Jirka

>
>Best Regards,
>Michał Mirosław