Message ID | c7fdfc32f69e7a4c3bfcaa8b2ee842aa9b286c91.1359507017.git.mirq-linux@rere.qmqm.pl |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
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); > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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);
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/