* [PATCH net-next] net: disallow drivers with buggy VLAN accel to register_netdevice()
@ 2013-01-30 1:14 Michał Mirosław
2013-01-30 1:17 ` Vlad Yasevich
0 siblings, 1 reply; 4+ messages in thread
From: Michał Mirosław @ 2013-01-30 1:14 UTC (permalink / raw)
To: netdev; +Cc: Patrick McHardy, David S. Miller, Vlad Yasevich, shemminger
Instead of jumping aroung bugs that are easily fixed just don't let them in:
affected drivers should be either fixed or have NETIF_F_HW_VLAN_FILTER
removed from advertised features.
Quick grep in drivers/net shows two drivers that have NETIF_F_HW_VLAN_FILTER
but not ndo_vlan_rx_add/kill_vid(), but those are false-positives (features
are commented out).
OTOH two drivers have ndo_vlan_rx_add/kill_vid() implemented but don't
advertise NETIF_F_HW_VLAN_FILTER. Those are:
+ethernet/cisco/enic/enic_main.c
+ethernet/qlogic/qlcnic/qlcnic_main.c
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
net/8021q/vlan.c | 7 -------
net/8021q/vlan_core.c | 6 ++----
net/core/dev.c | 8 ++++++++
3 files changed, 10 insertions(+), 11 deletions(-)
Not run-tested. Survived make allmodconfig && make net/ drivers/net/
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index babfde9..addc578 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -117,19 +117,12 @@ void unregister_vlan_dev(struct net_device *dev, struct list_head *head)
int vlan_check_real_dev(struct net_device *real_dev, u16 vlan_id)
{
const char *name = real_dev->name;
- const struct net_device_ops *ops = real_dev->netdev_ops;
if (real_dev->features & NETIF_F_VLAN_CHALLENGED) {
pr_info("VLANs not supported on %s\n", name);
return -EOPNOTSUPP;
}
- if ((real_dev->features & NETIF_F_HW_VLAN_FILTER) &&
- (!ops->ndo_vlan_rx_add_vid || !ops->ndo_vlan_rx_kill_vid)) {
- pr_info("Device %s has buggy VLAN hw accel\n", name);
- return -EOPNOTSUPP;
- }
-
if (vlan_find_dev(real_dev, vlan_id) != NULL)
return -EEXIST;
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 380440b..71b64fd 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -224,8 +224,7 @@ static int __vlan_vid_add(struct vlan_info *vlan_info, unsigned short vid,
if (!vid_info)
return -ENOMEM;
- if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
- ops->ndo_vlan_rx_add_vid) {
+ if (dev->features & NETIF_F_HW_VLAN_FILTER) {
err = ops->ndo_vlan_rx_add_vid(dev, vid);
if (err) {
kfree(vid_info);
@@ -282,8 +281,7 @@ static void __vlan_vid_del(struct vlan_info *vlan_info,
unsigned short vid = vid_info->vid;
int err;
- if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
- ops->ndo_vlan_rx_kill_vid) {
+ if (dev->features & NETIF_F_HW_VLAN_FILTER) {
err = ops->ndo_vlan_rx_kill_vid(dev, vid);
if (err) {
pr_warn("failed to kill vid %d for device %s\n",
diff --git a/net/core/dev.c b/net/core/dev.c
index a83375d..a87bc74 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6054,6 +6054,14 @@ int register_netdevice(struct net_device *dev)
}
}
+ if (((dev->hw_features | dev->features) & NETIF_F_HW_VLAN_FILTER) &&
+ (!dev->netdev_ops->ndo_vlan_rx_add_vid ||
+ !dev->netdev_ops->ndo_vlan_rx_kill_vid)) {
+ netdev_WARN(dev, "Buggy VLAN acceleration in driver!\n");
+ ret = -EINVAL;
+ goto err_uninit;
+ }
+
ret = -EBUSY;
if (!dev->ifindex)
dev->ifindex = dev_new_index(net);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] net: disallow drivers with buggy VLAN accel to register_netdevice()
2013-01-30 1:14 [PATCH net-next] net: disallow drivers with buggy VLAN accel to register_netdevice() Michał Mirosław
@ 2013-01-30 1:17 ` Vlad Yasevich
2013-01-30 3:56 ` David Miller
0 siblings, 1 reply; 4+ messages in thread
From: Vlad Yasevich @ 2013-01-30 1:17 UTC (permalink / raw)
To: Michał Mirosław
Cc: netdev, Patrick McHardy, David S. Miller, shemminger
On 01/29/2013 08:14 PM, Michał Mirosław wrote:
> Instead of jumping aroung bugs that are easily fixed just don't let them in:
> affected drivers should be either fixed or have NETIF_F_HW_VLAN_FILTER
> removed from advertised features.
>
> Quick grep in drivers/net shows two drivers that have NETIF_F_HW_VLAN_FILTER
> but not ndo_vlan_rx_add/kill_vid(), but those are false-positives (features
> are commented out).
>
> OTOH two drivers have ndo_vlan_rx_add/kill_vid() implemented but don't
> advertise NETIF_F_HW_VLAN_FILTER. Those are:
I am a bit hesitant to completely disable drivers, but I guess it would
force people to fix their drivers if this is applied.
-vlad
>
> +ethernet/cisco/enic/enic_main.c
> +ethernet/qlogic/qlcnic/qlcnic_main.c
>
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
> net/8021q/vlan.c | 7 -------
> net/8021q/vlan_core.c | 6 ++----
> net/core/dev.c | 8 ++++++++
> 3 files changed, 10 insertions(+), 11 deletions(-)
>
> Not run-tested. Survived make allmodconfig && make net/ drivers/net/
>
> diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
> index babfde9..addc578 100644
> --- a/net/8021q/vlan.c
> +++ b/net/8021q/vlan.c
> @@ -117,19 +117,12 @@ void unregister_vlan_dev(struct net_device *dev, struct list_head *head)
> int vlan_check_real_dev(struct net_device *real_dev, u16 vlan_id)
> {
> const char *name = real_dev->name;
> - const struct net_device_ops *ops = real_dev->netdev_ops;
>
> if (real_dev->features & NETIF_F_VLAN_CHALLENGED) {
> pr_info("VLANs not supported on %s\n", name);
> return -EOPNOTSUPP;
> }
>
> - if ((real_dev->features & NETIF_F_HW_VLAN_FILTER) &&
> - (!ops->ndo_vlan_rx_add_vid || !ops->ndo_vlan_rx_kill_vid)) {
> - pr_info("Device %s has buggy VLAN hw accel\n", name);
> - return -EOPNOTSUPP;
> - }
> -
> if (vlan_find_dev(real_dev, vlan_id) != NULL)
> return -EEXIST;
>
> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
> index 380440b..71b64fd 100644
> --- a/net/8021q/vlan_core.c
> +++ b/net/8021q/vlan_core.c
> @@ -224,8 +224,7 @@ static int __vlan_vid_add(struct vlan_info *vlan_info, unsigned short vid,
> if (!vid_info)
> return -ENOMEM;
>
> - if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
> - ops->ndo_vlan_rx_add_vid) {
> + if (dev->features & NETIF_F_HW_VLAN_FILTER) {
> err = ops->ndo_vlan_rx_add_vid(dev, vid);
> if (err) {
> kfree(vid_info);
> @@ -282,8 +281,7 @@ static void __vlan_vid_del(struct vlan_info *vlan_info,
> unsigned short vid = vid_info->vid;
> int err;
>
> - if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
> - ops->ndo_vlan_rx_kill_vid) {
> + if (dev->features & NETIF_F_HW_VLAN_FILTER) {
> err = ops->ndo_vlan_rx_kill_vid(dev, vid);
> if (err) {
> pr_warn("failed to kill vid %d for device %s\n",
> diff --git a/net/core/dev.c b/net/core/dev.c
> index a83375d..a87bc74 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -6054,6 +6054,14 @@ int register_netdevice(struct net_device *dev)
> }
> }
>
> + if (((dev->hw_features | dev->features) & NETIF_F_HW_VLAN_FILTER) &&
> + (!dev->netdev_ops->ndo_vlan_rx_add_vid ||
> + !dev->netdev_ops->ndo_vlan_rx_kill_vid)) {
> + netdev_WARN(dev, "Buggy VLAN acceleration in driver!\n");
> + ret = -EINVAL;
> + goto err_uninit;
> + }
> +
> ret = -EBUSY;
> if (!dev->ifindex)
> dev->ifindex = dev_new_index(net);
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] net: disallow drivers with buggy VLAN accel to register_netdevice()
2013-01-30 1:17 ` Vlad Yasevich
@ 2013-01-30 3:56 ` David Miller
2013-01-30 13:36 ` Vlad Yasevich
0 siblings, 1 reply; 4+ messages in thread
From: David Miller @ 2013-01-30 3:56 UTC (permalink / raw)
To: vyasevic; +Cc: mirq-linux, netdev, kaber, shemminger
From: Vlad Yasevich <vyasevic@redhat.com>
Date: Tue, 29 Jan 2013 20:17:45 -0500
> On 01/29/2013 08:14 PM, Michał Mirosław wrote:
>> Instead of jumping aroung bugs that are easily fixed just don't let
>> them in:
>> affected drivers should be either fixed or have NETIF_F_HW_VLAN_FILTER
>> removed from advertised features.
>>
>> Quick grep in drivers/net shows two drivers that have
>> NETIF_F_HW_VLAN_FILTER
>> but not ndo_vlan_rx_add/kill_vid(), but those are false-positives
>> (features
>> are commented out).
>>
>> OTOH two drivers have ndo_vlan_rx_add/kill_vid() implemented but don't
>> advertise NETIF_F_HW_VLAN_FILTER. Those are:
>
> I am a bit hesitant to completely disable drivers, but I guess it
> would force people to fix their drivers if this is applied.
I think an improperly implemented driver should be BUG trapped as soon
as possible.
I've applied this patch, thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] net: disallow drivers with buggy VLAN accel to register_netdevice()
2013-01-30 3:56 ` David Miller
@ 2013-01-30 13:36 ` Vlad Yasevich
0 siblings, 0 replies; 4+ messages in thread
From: Vlad Yasevich @ 2013-01-30 13:36 UTC (permalink / raw)
To: David Miller; +Cc: mirq-linux, netdev, kaber, shemminger
On 01/29/2013 10:56 PM, David Miller wrote:
> From: Vlad Yasevich <vyasevic@redhat.com>
> Date: Tue, 29 Jan 2013 20:17:45 -0500
>
>> On 01/29/2013 08:14 PM, Michał Mirosław wrote:
>>> Instead of jumping aroung bugs that are easily fixed just don't let
>>> them in:
>>> affected drivers should be either fixed or have NETIF_F_HW_VLAN_FILTER
>>> removed from advertised features.
>>>
>>> Quick grep in drivers/net shows two drivers that have
>>> NETIF_F_HW_VLAN_FILTER
>>> but not ndo_vlan_rx_add/kill_vid(), but those are false-positives
>>> (features
>>> are commented out).
>>>
>>> OTOH two drivers have ndo_vlan_rx_add/kill_vid() implemented but don't
>>> advertise NETIF_F_HW_VLAN_FILTER. Those are:
>>
>> I am a bit hesitant to completely disable drivers, but I guess it
>> would force people to fix their drivers if this is applied.
>
> I think an improperly implemented driver should be BUG trapped as soon
> as possible.
>
> I've applied this patch, thanks.
>
OK, I'll rebase my series on top of this and get rid the buggy calls.
Thanks
-vlad
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-01-30 13:36 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-30 1:14 [PATCH net-next] net: disallow drivers with buggy VLAN accel to register_netdevice() Michał Mirosław
2013-01-30 1:17 ` Vlad Yasevich
2013-01-30 3:56 ` David Miller
2013-01-30 13:36 ` Vlad Yasevich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).