* Re: [net-next-2.6 PATCH] wireless: convert to use netdev_for_each_mc_addr
@ 2010-03-03 9:45 Jussi Kivilinna
2010-03-03 10:01 ` David Miller
2010-03-03 12:42 ` Jiri Pirko
0 siblings, 2 replies; 14+ messages in thread
From: Jussi Kivilinna @ 2010-03-03 9:45 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem
Hello!
Added locking broke rndis_wlan, rndis_set_oid/rndis_command sleeps.
Also there is kmalloc(..., GFP_KERNEL) within lock/unlock.
-Jussi
Quoting "Jiri Pirko" <jpirko@redhat.com>:
>
> also added missed locking in rndis_wlan.c
>
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
> ---
> drivers/net/wireless/libertas/main.c | 8 ++++++--
> drivers/net/wireless/orinoco/hw.c | 22 +++++++---------------
> drivers/net/wireless/orinoco/hw.h | 2 +-
> drivers/net/wireless/orinoco/main.c | 3 +--
> drivers/net/wireless/ray_cs.c | 8 ++++----
> drivers/net/wireless/rndis_wlan.c | 15 ++++++++-------
> drivers/net/wireless/zd1201.c | 9 ++++-----
> 7 files changed, 31 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/net/wireless/libertas/main.c
> b/drivers/net/wireless/libertas/main.c
> index cd8ed7f..28a1c9d 100644
> --- a/drivers/net/wireless/libertas/main.c
> +++ b/drivers/net/wireless/libertas/main.c
> @@ -319,15 +319,18 @@ static int lbs_add_mcast_addrs(struct
> cmd_ds_mac_multicast_adr *cmd,
> {
> int i = nr_addrs;
> struct dev_mc_list *mc_list;
> + int cnt;
>
> if ((dev->flags & (IFF_UP|IFF_MULTICAST)) != (IFF_UP|IFF_MULTICAST))
> return nr_addrs;
>
> netif_addr_lock_bh(dev);
> - for (mc_list = dev->mc_list; mc_list; mc_list = mc_list->next) {
> + cnt = netdev_mc_count(dev);
> + netdev_for_each_mc_addr(mc_list, dev) {
> if (mac_in_list(cmd->maclist, nr_addrs, mc_list->dmi_addr)) {
> lbs_deb_net("mcast address %s:%pM skipped\n", dev->name,
> mc_list->dmi_addr);
> + cnt--;
> continue;
> }
>
> @@ -337,9 +340,10 @@ static int lbs_add_mcast_addrs(struct
> cmd_ds_mac_multicast_adr *cmd,
> lbs_deb_net("mcast address %s:%pM added to filter\n", dev->name,
> mc_list->dmi_addr);
> i++;
> + cnt--;
> }
> netif_addr_unlock_bh(dev);
> - if (mc_list)
> + if (cnt)
> return -EOVERFLOW;
>
> return i;
> diff --git a/drivers/net/wireless/orinoco/hw.c
> b/drivers/net/wireless/orinoco/hw.c
> index 404830f..e636924 100644
> --- a/drivers/net/wireless/orinoco/hw.c
> +++ b/drivers/net/wireless/orinoco/hw.c
> @@ -1028,7 +1028,7 @@ int orinoco_clear_tkip_key(struct
> orinoco_private *priv, int key_idx)
> }
>
> int __orinoco_hw_set_multicast_list(struct orinoco_private *priv,
> - struct dev_addr_list *mc_list,
> + struct net_device *dev,
> int mc_count, int promisc)
> {
> hermes_t *hw = &priv->hw;
> @@ -1049,24 +1049,16 @@ int __orinoco_hw_set_multicast_list(struct
> orinoco_private *priv,
> * group address if either we want to multicast, or if we were
> * multicasting and want to stop */
> if (!promisc && (mc_count || priv->mc_count)) {
> - struct dev_mc_list *p = mc_list;
> + struct dev_mc_list *p;
> struct hermes_multicast mclist;
> - int i;
> + int i = 0;
>
> - for (i = 0; i < mc_count; i++) {
> - /* paranoia: is list shorter than mc_count? */
> - BUG_ON(!p);
> - /* paranoia: bad address size in list? */
> - BUG_ON(p->dmi_addrlen != ETH_ALEN);
> -
> - memcpy(mclist.addr[i], p->dmi_addr, ETH_ALEN);
> - p = p->next;
> + netdev_for_each_mc_addr(p, dev) {
> + if (i == mc_count)
> + break;
> + memcpy(mclist.addr[i++], p->dmi_addr, ETH_ALEN);
> }
>
> - if (p)
> - printk(KERN_WARNING "%s: Multicast list is "
> - "longer than mc_count\n", priv->ndev->name);
> -
> err = hermes_write_ltv(hw, USER_BAP,
> HERMES_RID_CNFGROUPADDRESSES,
> HERMES_BYTES_TO_RECLEN(mc_count * ETH_ALEN),
> diff --git a/drivers/net/wireless/orinoco/hw.h
> b/drivers/net/wireless/orinoco/hw.h
> index e2f7fdc..9799a1d 100644
> --- a/drivers/net/wireless/orinoco/hw.h
> +++ b/drivers/net/wireless/orinoco/hw.h
> @@ -43,7 +43,7 @@ int __orinoco_hw_set_tkip_key(struct
> orinoco_private *priv, int key_idx,
> u8 *tsc, size_t tsc_len);
> int orinoco_clear_tkip_key(struct orinoco_private *priv, int key_idx);
> int __orinoco_hw_set_multicast_list(struct orinoco_private *priv,
> - struct dev_addr_list *mc_list,
> + struct net_device *dev,
> int mc_count, int promisc);
> int orinoco_hw_get_essid(struct orinoco_private *priv, int *active,
> char buf[IW_ESSID_MAX_SIZE+1]);
> diff --git a/drivers/net/wireless/orinoco/main.c
> b/drivers/net/wireless/orinoco/main.c
> index a9e9cea..b42634c 100644
> --- a/drivers/net/wireless/orinoco/main.c
> +++ b/drivers/net/wireless/orinoco/main.c
> @@ -1676,8 +1676,7 @@ __orinoco_set_multicast_list(struct net_device *dev)
> mc_count = netdev_mc_count(dev);
> }
>
> - err = __orinoco_hw_set_multicast_list(priv, dev->mc_list, mc_count,
> - promisc);
> + err = __orinoco_hw_set_multicast_list(priv, dev, mc_count, promisc);
>
> return err;
> }
> diff --git a/drivers/net/wireless/ray_cs.c b/drivers/net/wireless/ray_cs.c
> index 85905ca..84c530a 100644
> --- a/drivers/net/wireless/ray_cs.c
> +++ b/drivers/net/wireless/ray_cs.c
> @@ -1871,10 +1871,8 @@ static void ray_update_parm(struct net_device
> *dev, UCHAR objid, UCHAR *value,
> /*===========================================================================*/
> static void ray_update_multi_list(struct net_device *dev, int all)
> {
> - struct dev_mc_list *dmi, **dmip;
> int ccsindex;
> struct ccs __iomem *pccs;
> - int i = 0;
> ray_dev_t *local = netdev_priv(dev);
> struct pcmcia_device *link = local->finder;
> void __iomem *p = local->sram + HOST_TO_ECF_BASE;
> @@ -1895,9 +1893,11 @@ static void ray_update_multi_list(struct
> net_device *dev, int all)
> writeb(0xff, &pccs->var);
> local->num_multi = 0xff;
> } else {
> + struct dev_mc_list *dmi;
> + int i = 0;
> +
> /* Copy the kernel's list of MC addresses to card */
> - for (dmip = &dev->mc_list; (dmi = *dmip) != NULL;
> - dmip = &dmi->next) {
> + netdev_for_each_mc_addr(dmi, dev) {
> memcpy_toio(p, dmi->dmi_addr, ETH_ALEN);
> dev_dbg(&link->dev,
> "ray_update_multi add addr %02x%02x%02x%02x%02x%02x\n",
> diff --git a/drivers/net/wireless/rndis_wlan.c
> b/drivers/net/wireless/rndis_wlan.c
> index 65cbd06..9f6d6bf 100644
> --- a/drivers/net/wireless/rndis_wlan.c
> +++ b/drivers/net/wireless/rndis_wlan.c
> @@ -1502,6 +1502,7 @@ static void set_multicast_list(struct usbnet *usbdev)
>
> filter = RNDIS_PACKET_TYPE_DIRECTED | RNDIS_PACKET_TYPE_BROADCAST;
>
> + netif_addr_lock_bh(usbdev->net);
> if (usbdev->net->flags & IFF_PROMISC) {
> filter |= RNDIS_PACKET_TYPE_PROMISCUOUS |
> RNDIS_PACKET_TYPE_ALL_LOCAL;
> @@ -1515,16 +1516,15 @@ static void set_multicast_list(struct usbnet *usbdev)
> netdev_warn(usbdev->net,
> "couldn't alloc %d bytes of memory\n",
> size * ETH_ALEN);
> + netif_addr_unlock_bh(usbdev->net);
> return;
> }
>
> - mclist = usbdev->net->mc_list;
> - for (i = 0; i < size && mclist; mclist = mclist->next) {
> - if (mclist->dmi_addrlen != ETH_ALEN)
> - continue;
> -
> - memcpy(buf + i * ETH_ALEN, mclist->dmi_addr, ETH_ALEN);
> - i++;
> + i = 0;
> + netdev_for_each_mc_addr(mclist, usbdev->net) {
> + if (i == size)
> + break;
> + memcpy(buf + i++ * ETH_ALEN, mclist->dmi_addr, ETH_ALEN);
> }
>
> ret = rndis_set_oid(usbdev, OID_802_3_MULTICAST_LIST, buf,
> @@ -1539,6 +1539,7 @@ static void set_multicast_list(struct usbnet *usbdev)
>
> kfree(buf);
> }
> + netif_addr_unlock_bh(usbdev->net);
>
> ret = rndis_set_oid(usbdev, OID_GEN_CURRENT_PACKET_FILTER, &filter,
> sizeof(filter));
> diff --git a/drivers/net/wireless/zd1201.c b/drivers/net/wireless/zd1201.c
> index 5d2b52f..6917286 100644
> --- a/drivers/net/wireless/zd1201.c
> +++ b/drivers/net/wireless/zd1201.c
> @@ -875,17 +875,16 @@ static struct iw_statistics
> *zd1201_get_wireless_stats(struct net_device *dev)
> static void zd1201_set_multicast(struct net_device *dev)
> {
> struct zd1201 *zd = netdev_priv(dev);
> - struct dev_mc_list *mc = dev->mc_list;
> + struct dev_mc_list *mc;
> unsigned char reqbuf[ETH_ALEN*ZD1201_MAXMULTI];
> int i;
>
> if (netdev_mc_count(dev) > ZD1201_MAXMULTI)
> return;
>
> - for (i=0; i<netdev_mc_count(dev); i++) {
> - memcpy(reqbuf+i*ETH_ALEN, mc->dmi_addr, ETH_ALEN);
> - mc = mc->next;
> - }
> + i = 0;
> + netdev_for_each_mc_addr(mc, dev)
> + memcpy(reqbuf + i++ * ETH_ALEN, mc->dmi_addr, ETH_ALEN);
> zd1201_setconfig(zd, ZD1201_RID_CNFGROUPADDRESS, reqbuf,
> netdev_mc_count(dev) * ETH_ALEN, 0);
> }
> --
> 1.6.6
>
> --
> 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
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [net-next-2.6 PATCH] wireless: convert to use netdev_for_each_mc_addr
2010-03-03 9:45 [net-next-2.6 PATCH] wireless: convert to use netdev_for_each_mc_addr Jussi Kivilinna
@ 2010-03-03 10:01 ` David Miller
2010-03-03 12:42 ` Jiri Pirko
1 sibling, 0 replies; 14+ messages in thread
From: David Miller @ 2010-03-03 10:01 UTC (permalink / raw)
To: jussi.kivilinna; +Cc: jpirko, netdev
From: "Jussi Kivilinna" <jussi.kivilinna@mbnet.fi>
Date: Wed, 03 Mar 2010 11:45:06 +0200
> Hello!
>
> Added locking broke rndis_wlan, rndis_set_oid/rndis_command
> sleeps. Also there is kmalloc(..., GFP_KERNEL) within lock/unlock.
Jiri please fix this.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [net-next-2.6 PATCH] wireless: convert to use netdev_for_each_mc_addr
2010-03-03 9:45 [net-next-2.6 PATCH] wireless: convert to use netdev_for_each_mc_addr Jussi Kivilinna
2010-03-03 10:01 ` David Miller
@ 2010-03-03 12:42 ` Jiri Pirko
2010-03-03 16:42 ` Jussi Kivilinna
1 sibling, 1 reply; 14+ messages in thread
From: Jiri Pirko @ 2010-03-03 12:42 UTC (permalink / raw)
To: Jussi Kivilinna; +Cc: netdev, davem
Wed, Mar 03, 2010 at 10:45:06AM CET, jussi.kivilinna@mbnet.fi wrote:
>Hello!
>
>Added locking broke rndis_wlan, rndis_set_oid/rndis_command sleeps.
>Also there is kmalloc(..., GFP_KERNEL) within lock/unlock.
>
Patch correcting this follows.
Subject: [net-2.6 PATCH] rndis_wlan: correct multicast_list handling
My previous patch (655ffee284dfcf9a24ac0343f3e5ee6db85b85c5) added locking in
a bad way. Because rndis_set_oid can sleep, there is need to prepare multicast
addresses into local buffer under netif_addr_lock first, then call
rndis_set_oid outside. This caused reorganizing of the whole function.
Please review.
Thanks
Jirka
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
Reported-by: Jussi Kivilinna <jussi.kivilinna@mbnet.fi>
diff --git a/drivers/net/wireless/rndis_wlan.c b/drivers/net/wireless/rndis_wlan.c
index 9f6d6bf..824064b 100644
--- a/drivers/net/wireless/rndis_wlan.c
+++ b/drivers/net/wireless/rndis_wlan.c
@@ -1496,23 +1496,38 @@ static void set_multicast_list(struct usbnet *usbdev)
{
struct rndis_wlan_private *priv = get_rndis_wlan_priv(usbdev);
struct dev_mc_list *mclist;
- __le32 filter;
- int ret, i, size;
- char *buf;
+ __le32 filter, basefilter;
+ int ret;
+ char *mc_addrs = NULL;
+ int mc_count;
- filter = RNDIS_PACKET_TYPE_DIRECTED | RNDIS_PACKET_TYPE_BROADCAST;
+ basefilter = filter = RNDIS_PACKET_TYPE_DIRECTED |
+ RNDIS_PACKET_TYPE_BROADCAST;
- netif_addr_lock_bh(usbdev->net);
if (usbdev->net->flags & IFF_PROMISC) {
filter |= RNDIS_PACKET_TYPE_PROMISCUOUS |
RNDIS_PACKET_TYPE_ALL_LOCAL;
- } else if (usbdev->net->flags & IFF_ALLMULTI ||
- netdev_mc_count(usbdev->net) > priv->multicast_size) {
+ } else if (usbdev->net->flags & IFF_ALLMULTI) {
+ filter |= RNDIS_PACKET_TYPE_ALL_MULTICAST;
+ }
+
+ if (filter != basefilter)
+ goto set_filter;
+
+ /*
+ * mc_list should be accessed holding the lock, so copy addresses to
+ * local buffer first.
+ */
+ netif_addr_lock_bh(usbdev->net);
+ mc_count = netdev_mc_count(usbdev->net);
+ if (mc_count > priv->multicast_size) {
filter |= RNDIS_PACKET_TYPE_ALL_MULTICAST;
- } else if (!netdev_mc_empty(usbdev->net)) {
- size = min(priv->multicast_size, netdev_mc_count(usbdev->net));
- buf = kmalloc(size * ETH_ALEN, GFP_KERNEL);
- if (!buf) {
+ } else if (mc_count) {
+ int size = min(priv->multicast_size, mc_count);
+ int i = 0;
+
+ mc_addrs = kmalloc(size * ETH_ALEN, GFP_ATOMIC);
+ if (!mc_addrs) {
netdev_warn(usbdev->net,
"couldn't alloc %d bytes of memory\n",
size * ETH_ALEN);
@@ -1520,27 +1535,29 @@ static void set_multicast_list(struct usbnet *usbdev)
return;
}
- i = 0;
- netdev_for_each_mc_addr(mclist, usbdev->net) {
- if (i == size)
- break;
- memcpy(buf + i++ * ETH_ALEN, mclist->dmi_addr, ETH_ALEN);
- }
+ netdev_for_each_mc_addr(mclist, usbdev->net)
+ memcpy(mc_addrs + i++ * ETH_ALEN,
+ mclist->dmi_addr, ETH_ALEN);
+ }
+ netif_addr_unlock_bh(usbdev->net);
- ret = rndis_set_oid(usbdev, OID_802_3_MULTICAST_LIST, buf,
- i * ETH_ALEN);
- if (ret == 0 && i > 0)
+ if (filter != basefilter)
+ goto set_filter;
+
+ if (mc_count) {
+ ret = rndis_set_oid(usbdev, OID_802_3_MULTICAST_LIST, mc_addrs,
+ mc_count * ETH_ALEN);
+ kfree(mc_addrs);
+ if (ret == 0)
filter |= RNDIS_PACKET_TYPE_MULTICAST;
else
filter |= RNDIS_PACKET_TYPE_ALL_MULTICAST;
netdev_dbg(usbdev->net, "OID_802_3_MULTICAST_LIST(%d, max: %d) -> %d\n",
- i, priv->multicast_size, ret);
-
- kfree(buf);
+ mc_count, priv->multicast_size, ret);
}
- netif_addr_unlock_bh(usbdev->net);
+set_filter:
ret = rndis_set_oid(usbdev, OID_GEN_CURRENT_PACKET_FILTER, &filter,
sizeof(filter));
if (ret < 0) {
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [net-next-2.6 PATCH] wireless: convert to use netdev_for_each_mc_addr
2010-03-03 12:42 ` Jiri Pirko
@ 2010-03-03 16:42 ` Jussi Kivilinna
2010-03-03 17:52 ` Jiri Pirko
0 siblings, 1 reply; 14+ messages in thread
From: Jussi Kivilinna @ 2010-03-03 16:42 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem
Hello!
Quoting "Jiri Pirko" <jpirko@redhat.com>:
> + } else if (mc_count) {
> + int size = min(priv->multicast_size, mc_count);
> + int i = 0;
> +
> + mc_addrs = kmalloc(size * ETH_ALEN, GFP_ATOMIC);
...
> + if (filter != basefilter)
> + goto set_filter;
> +
> + if (mc_count) {
> + ret = rndis_set_oid(usbdev, OID_802_3_MULTICAST_LIST, mc_addrs,
> + mc_count * ETH_ALEN);
> + kfree(mc_addrs);
mc_addrs was alloced by with 'size * ETH_ALEN', which might be less
than mc_count * ETH_ALEN.
Otherwise ok, and rndis_wlan works.
-Jussi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [net-next-2.6 PATCH] wireless: convert to use netdev_for_each_mc_addr
2010-03-03 16:42 ` Jussi Kivilinna
@ 2010-03-03 17:52 ` Jiri Pirko
2010-03-03 18:05 ` Jussi Kivilinna
0 siblings, 1 reply; 14+ messages in thread
From: Jiri Pirko @ 2010-03-03 17:52 UTC (permalink / raw)
To: Jussi Kivilinna; +Cc: netdev, davem
Wed, Mar 03, 2010 at 05:42:56PM CET, jussi.kivilinna@mbnet.fi wrote:
>Hello!
>
>Quoting "Jiri Pirko" <jpirko@redhat.com>:
>
>>+ } else if (mc_count) {
>>+ int size = min(priv->multicast_size, mc_count);
>>+ int i = 0;
>>+
>>+ mc_addrs = kmalloc(size * ETH_ALEN, GFP_ATOMIC);
>...
>>+ if (filter != basefilter)
>>+ goto set_filter;
>>+
>>+ if (mc_count) {
>>+ ret = rndis_set_oid(usbdev, OID_802_3_MULTICAST_LIST, mc_addrs,
>>+ mc_count * ETH_ALEN);
>>+ kfree(mc_addrs);
>
>mc_addrs was alloced by with 'size * ETH_ALEN', which might be less
>than mc_count * ETH_ALEN.
Actually it cannot. That's covered by:
if (mc_count > priv->multicast_size) {
This was also in the original code. In that case "size" can be eliminated and
"mc_addrs" can be allocated with "mc_count * ETH_ALEN".
Jussi are you ok with this?
Jirka
>
>Otherwise ok, and rndis_wlan works.
>
> -Jussi
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [net-next-2.6 PATCH] wireless: convert to use netdev_for_each_mc_addr
2010-03-03 17:52 ` Jiri Pirko
@ 2010-03-03 18:05 ` Jussi Kivilinna
2010-03-03 18:09 ` Jiri Pirko
0 siblings, 1 reply; 14+ messages in thread
From: Jussi Kivilinna @ 2010-03-03 18:05 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem
Quoting "Jiri Pirko" <jpirko@redhat.com>:
> Wed, Mar 03, 2010 at 05:42:56PM CET, jussi.kivilinna@mbnet.fi wrote:
>> Hello!
>>
>> Quoting "Jiri Pirko" <jpirko@redhat.com>:
>>
>>> + } else if (mc_count) {
>>> + int size = min(priv->multicast_size, mc_count);
>>> + int i = 0;
>>> +
>>> + mc_addrs = kmalloc(size * ETH_ALEN, GFP_ATOMIC);
>> ...
>>> + if (filter != basefilter)
>>> + goto set_filter;
>>> +
>>> + if (mc_count) {
>>> + ret = rndis_set_oid(usbdev, OID_802_3_MULTICAST_LIST, mc_addrs,
>>> + mc_count * ETH_ALEN);
>>> + kfree(mc_addrs);
>>
>> mc_addrs was alloced by with 'size * ETH_ALEN', which might be less
>> than mc_count * ETH_ALEN.
>
> Actually it cannot. That's covered by:
>
> if (mc_count > priv->multicast_size) {
>
> This was also in the original code. In that case "size" can be eliminated and
> "mc_addrs" can be allocated with "mc_count * ETH_ALEN".
>
> Jussi are you ok with this?
>
> Jirka
Ah, you're right. Yes, 'size' can go away, it isn't needed after all.
I'm ok with this patch, I can fix 'size' to 'mc_count' myself later.
-Jussi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [net-next-2.6 PATCH] wireless: convert to use netdev_for_each_mc_addr
2010-03-03 18:05 ` Jussi Kivilinna
@ 2010-03-03 18:09 ` Jiri Pirko
2010-03-04 8:41 ` David Miller
0 siblings, 1 reply; 14+ messages in thread
From: Jiri Pirko @ 2010-03-03 18:09 UTC (permalink / raw)
To: davem; +Cc: netdev, jussi.kivilinna, linux-wireless
Wed, Mar 03, 2010 at 07:05:10PM CET, jussi.kivilinna@mbnet.fi wrote:
>Quoting "Jiri Pirko" <jpirko@redhat.com>:
>
>>Wed, Mar 03, 2010 at 05:42:56PM CET, jussi.kivilinna@mbnet.fi wrote:
>>>Hello!
>>>
>>>Quoting "Jiri Pirko" <jpirko@redhat.com>:
>>>
>>>>+ } else if (mc_count) {
>>>>+ int size = min(priv->multicast_size, mc_count);
>>>>+ int i = 0;
>>>>+
>>>>+ mc_addrs = kmalloc(size * ETH_ALEN, GFP_ATOMIC);
>>>...
>>>>+ if (filter != basefilter)
>>>>+ goto set_filter;
>>>>+
>>>>+ if (mc_count) {
>>>>+ ret = rndis_set_oid(usbdev, OID_802_3_MULTICAST_LIST, mc_addrs,
>>>>+ mc_count * ETH_ALEN);
>>>>+ kfree(mc_addrs);
>>>
>>>mc_addrs was alloced by with 'size * ETH_ALEN', which might be less
>>>than mc_count * ETH_ALEN.
>>
>>Actually it cannot. That's covered by:
>>
>>if (mc_count > priv->multicast_size) {
>>
>>This was also in the original code. In that case "size" can be eliminated and
>>"mc_addrs" can be allocated with "mc_count * ETH_ALEN".
>>
>>Jussi are you ok with this?
>>
>>Jirka
>
>Ah, you're right. Yes, 'size' can go away, it isn't needed after all.
>I'm ok with this patch, I can fix 'size' to 'mc_count' myself later.
Here's corrected patch:
Subject: [net-2.6 PATCH] rndis_wlan: correct multicast_list handling V2
My previous patch (655ffee284dfcf9a24ac0343f3e5ee6db85b85c5) added locking in
a bad way. Because rndis_set_oid can sleep, there is need to prepare multicast
addresses into local buffer under netif_addr_lock first, then call
rndis_set_oid outside. This caused reorganizing of the whole function.
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
Reported-by: Jussi Kivilinna <jussi.kivilinna@mbnet.fi>
diff --git a/drivers/net/wireless/rndis_wlan.c b/drivers/net/wireless/rndis_wlan.c
index 9f6d6bf..07e6fdd 100644
--- a/drivers/net/wireless/rndis_wlan.c
+++ b/drivers/net/wireless/rndis_wlan.c
@@ -1496,23 +1496,37 @@ static void set_multicast_list(struct usbnet *usbdev)
{
struct rndis_wlan_private *priv = get_rndis_wlan_priv(usbdev);
struct dev_mc_list *mclist;
- __le32 filter;
- int ret, i, size;
- char *buf;
+ __le32 filter, basefilter;
+ int ret;
+ char *mc_addrs = NULL;
+ int mc_count;
- filter = RNDIS_PACKET_TYPE_DIRECTED | RNDIS_PACKET_TYPE_BROADCAST;
+ basefilter = filter = RNDIS_PACKET_TYPE_DIRECTED |
+ RNDIS_PACKET_TYPE_BROADCAST;
- netif_addr_lock_bh(usbdev->net);
if (usbdev->net->flags & IFF_PROMISC) {
filter |= RNDIS_PACKET_TYPE_PROMISCUOUS |
RNDIS_PACKET_TYPE_ALL_LOCAL;
- } else if (usbdev->net->flags & IFF_ALLMULTI ||
- netdev_mc_count(usbdev->net) > priv->multicast_size) {
+ } else if (usbdev->net->flags & IFF_ALLMULTI) {
+ filter |= RNDIS_PACKET_TYPE_ALL_MULTICAST;
+ }
+
+ if (filter != basefilter)
+ goto set_filter;
+
+ /*
+ * mc_list should be accessed holding the lock, so copy addresses to
+ * local buffer first.
+ */
+ netif_addr_lock_bh(usbdev->net);
+ mc_count = netdev_mc_count(usbdev->net);
+ if (mc_count > priv->multicast_size) {
filter |= RNDIS_PACKET_TYPE_ALL_MULTICAST;
- } else if (!netdev_mc_empty(usbdev->net)) {
- size = min(priv->multicast_size, netdev_mc_count(usbdev->net));
- buf = kmalloc(size * ETH_ALEN, GFP_KERNEL);
- if (!buf) {
+ } else if (mc_count) {
+ int i = 0;
+
+ mc_addrs = kmalloc(mc_count * ETH_ALEN, GFP_ATOMIC);
+ if (!mc_addrs) {
netdev_warn(usbdev->net,
"couldn't alloc %d bytes of memory\n",
size * ETH_ALEN);
@@ -1520,27 +1534,29 @@ static void set_multicast_list(struct usbnet *usbdev)
return;
}
- i = 0;
- netdev_for_each_mc_addr(mclist, usbdev->net) {
- if (i == size)
- break;
- memcpy(buf + i++ * ETH_ALEN, mclist->dmi_addr, ETH_ALEN);
- }
+ netdev_for_each_mc_addr(mclist, usbdev->net)
+ memcpy(mc_addrs + i++ * ETH_ALEN,
+ mclist->dmi_addr, ETH_ALEN);
+ }
+ netif_addr_unlock_bh(usbdev->net);
- ret = rndis_set_oid(usbdev, OID_802_3_MULTICAST_LIST, buf,
- i * ETH_ALEN);
- if (ret == 0 && i > 0)
+ if (filter != basefilter)
+ goto set_filter;
+
+ if (mc_count) {
+ ret = rndis_set_oid(usbdev, OID_802_3_MULTICAST_LIST, mc_addrs,
+ mc_count * ETH_ALEN);
+ kfree(mc_addrs);
+ if (ret == 0)
filter |= RNDIS_PACKET_TYPE_MULTICAST;
else
filter |= RNDIS_PACKET_TYPE_ALL_MULTICAST;
netdev_dbg(usbdev->net, "OID_802_3_MULTICAST_LIST(%d, max: %d) -> %d\n",
- i, priv->multicast_size, ret);
-
- kfree(buf);
+ mc_count, priv->multicast_size, ret);
}
- netif_addr_unlock_bh(usbdev->net);
+set_filter:
ret = rndis_set_oid(usbdev, OID_GEN_CURRENT_PACKET_FILTER, &filter,
sizeof(filter));
if (ret < 0) {
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [net-next-2.6 PATCH] wireless: convert to use netdev_for_each_mc_addr
2010-03-03 18:09 ` Jiri Pirko
@ 2010-03-04 8:41 ` David Miller
2010-03-04 8:52 ` David Miller
0 siblings, 1 reply; 14+ messages in thread
From: David Miller @ 2010-03-04 8:41 UTC (permalink / raw)
To: jpirko; +Cc: netdev, jussi.kivilinna, linux-wireless
From: Jiri Pirko <jpirko@redhat.com>
Date: Wed, 3 Mar 2010 19:09:56 +0100
> Here's corrected patch:
>
> Subject: [net-2.6 PATCH] rndis_wlan: correct multicast_list handling V2
>
> My previous patch (655ffee284dfcf9a24ac0343f3e5ee6db85b85c5) added locking in
> a bad way. Because rndis_set_oid can sleep, there is need to prepare multicast
> addresses into local buffer under netif_addr_lock first, then call
> rndis_set_oid outside. This caused reorganizing of the whole function.
>
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
> Reported-by: Jussi Kivilinna <jussi.kivilinna@mbnet.fi>
Applied.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [net-next-2.6 PATCH] wireless: convert to use netdev_for_each_mc_addr
@ 2010-03-04 8:52 ` David Miller
0 siblings, 0 replies; 14+ messages in thread
From: David Miller @ 2010-03-04 8:52 UTC (permalink / raw)
To: jpirko; +Cc: netdev, jussi.kivilinna, linux-wireless
From: David Miller <davem@davemloft.net>
Date: Thu, 04 Mar 2010 00:41:50 -0800 (PST)
> From: Jiri Pirko <jpirko@redhat.com>
> Date: Wed, 3 Mar 2010 19:09:56 +0100
>
>> Here's corrected patch:
>>
>> Subject: [net-2.6 PATCH] rndis_wlan: correct multicast_list handling V2
>>
>> My previous patch (655ffee284dfcf9a24ac0343f3e5ee6db85b85c5) added locking in
>> a bad way. Because rndis_set_oid can sleep, there is need to prepare multicast
>> addresses into local buffer under netif_addr_lock first, then call
>> rndis_set_oid outside. This caused reorganizing of the whole function.
>>
>> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
>> Reported-by: Jussi Kivilinna <jussi.kivilinna@mbnet.fi>
>
> Applied.
Jiri did ou even type make on this patch? The 'size' is still
referenced:
drivers/net/wireless/rndis_wlan.c: In function 'set_multicast_list':
drivers/net/wireless/rndis_wlan.c:1530: error: 'size' undeclared (first use in this function)
drivers/net/wireless/rndis_wlan.c:1530: error: (Each undeclared identifier is reported only once
drivers/net/wireless/rndis_wlan.c:1530: error: for each function it appears in.)
I reverted, and I'm pissed off.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [net-next-2.6 PATCH] wireless: convert to use netdev_for_each_mc_addr
@ 2010-03-04 8:52 ` David Miller
0 siblings, 0 replies; 14+ messages in thread
From: David Miller @ 2010-03-04 8:52 UTC (permalink / raw)
To: jpirko-H+wXaHxf7aLQT0dZR+AlfA
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, jussi.kivilinna-E01nCVcF24I,
linux-wireless-u79uwXL29TY76Z2rM5mHXA
From: David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Date: Thu, 04 Mar 2010 00:41:50 -0800 (PST)
> From: Jiri Pirko <jpirko-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Date: Wed, 3 Mar 2010 19:09:56 +0100
>
>> Here's corrected patch:
>>
>> Subject: [net-2.6 PATCH] rndis_wlan: correct multicast_list handling V2
>>
>> My previous patch (655ffee284dfcf9a24ac0343f3e5ee6db85b85c5) added locking in
>> a bad way. Because rndis_set_oid can sleep, there is need to prepare multicast
>> addresses into local buffer under netif_addr_lock first, then call
>> rndis_set_oid outside. This caused reorganizing of the whole function.
>>
>> Signed-off-by: Jiri Pirko <jpirko-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> Reported-by: Jussi Kivilinna <jussi.kivilinna-E01nCVcF24I@public.gmane.org>
>
> Applied.
Jiri did ou even type make on this patch? The 'size' is still
referenced:
drivers/net/wireless/rndis_wlan.c: In function 'set_multicast_list':
drivers/net/wireless/rndis_wlan.c:1530: error: 'size' undeclared (first use in this function)
drivers/net/wireless/rndis_wlan.c:1530: error: (Each undeclared identifier is reported only once
drivers/net/wireless/rndis_wlan.c:1530: error: for each function it appears in.)
I reverted, and I'm pissed off.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [net-next-2.6 PATCH] wireless: convert to use netdev_for_each_mc_addr
2010-03-04 8:52 ` David Miller
(?)
@ 2010-03-04 9:20 ` Jiri Pirko
2010-03-04 11:38 ` David Miller
-1 siblings, 1 reply; 14+ messages in thread
From: Jiri Pirko @ 2010-03-04 9:20 UTC (permalink / raw)
To: David Miller; +Cc: netdev, jussi.kivilinna, linux-wireless
Thu, Mar 04, 2010 at 09:52:46AM CET, davem@davemloft.net wrote:
>From: David Miller <davem@davemloft.net>
>Date: Thu, 04 Mar 2010 00:41:50 -0800 (PST)
>
>> From: Jiri Pirko <jpirko@redhat.com>
>> Date: Wed, 3 Mar 2010 19:09:56 +0100
>>
>>> Here's corrected patch:
>>>
>>> Subject: [net-2.6 PATCH] rndis_wlan: correct multicast_list handling V2
>>>
>>> My previous patch (655ffee284dfcf9a24ac0343f3e5ee6db85b85c5) added locking in
>>> a bad way. Because rndis_set_oid can sleep, there is need to prepare multicast
>>> addresses into local buffer under netif_addr_lock first, then call
>>> rndis_set_oid outside. This caused reorganizing of the whole function.
>>>
>>> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
>>> Reported-by: Jussi Kivilinna <jussi.kivilinna@mbnet.fi>
>>
>> Applied.
>
>Jiri did ou even type make on this patch? The 'size' is still
>referenced:
>
>drivers/net/wireless/rndis_wlan.c: In function 'set_multicast_list':
>drivers/net/wireless/rndis_wlan.c:1530: error: 'size' undeclared (first use in this function)
>drivers/net/wireless/rndis_wlan.c:1530: error: (Each undeclared identifier is reported only once
>drivers/net/wireless/rndis_wlan.c:1530: error: for each function it appears in.)
>
>I reverted, and I'm pissed off.
Slapping myself twice on each cheek.
Subject: [net-2.6 PATCH] rndis_wlan: correct multicast_list handling V3
My previous patch (655ffee284dfcf9a24ac0343f3e5ee6db85b85c5) added locking in
a bad way. Because rndis_set_oid can sleep, there is need to prepare multicast
addresses into local buffer under netif_addr_lock first, then call
rndis_set_oid outside. This caused reorganizing of the whole function.
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
Reported-by: Jussi Kivilinna <jussi.kivilinna@mbnet.fi>
diff --git a/drivers/net/wireless/rndis_wlan.c b/drivers/net/wireless/rndis_wlan.c
index 9f6d6bf..2887047 100644
--- a/drivers/net/wireless/rndis_wlan.c
+++ b/drivers/net/wireless/rndis_wlan.c
@@ -1496,51 +1496,67 @@ static void set_multicast_list(struct usbnet *usbdev)
{
struct rndis_wlan_private *priv = get_rndis_wlan_priv(usbdev);
struct dev_mc_list *mclist;
- __le32 filter;
- int ret, i, size;
- char *buf;
+ __le32 filter, basefilter;
+ int ret;
+ char *mc_addrs = NULL;
+ int mc_count;
- filter = RNDIS_PACKET_TYPE_DIRECTED | RNDIS_PACKET_TYPE_BROADCAST;
+ basefilter = filter = RNDIS_PACKET_TYPE_DIRECTED |
+ RNDIS_PACKET_TYPE_BROADCAST;
- netif_addr_lock_bh(usbdev->net);
if (usbdev->net->flags & IFF_PROMISC) {
filter |= RNDIS_PACKET_TYPE_PROMISCUOUS |
RNDIS_PACKET_TYPE_ALL_LOCAL;
- } else if (usbdev->net->flags & IFF_ALLMULTI ||
- netdev_mc_count(usbdev->net) > priv->multicast_size) {
+ } else if (usbdev->net->flags & IFF_ALLMULTI) {
+ filter |= RNDIS_PACKET_TYPE_ALL_MULTICAST;
+ }
+
+ if (filter != basefilter)
+ goto set_filter;
+
+ /*
+ * mc_list should be accessed holding the lock, so copy addresses to
+ * local buffer first.
+ */
+ netif_addr_lock_bh(usbdev->net);
+ mc_count = netdev_mc_count(usbdev->net);
+ if (mc_count > priv->multicast_size) {
filter |= RNDIS_PACKET_TYPE_ALL_MULTICAST;
- } else if (!netdev_mc_empty(usbdev->net)) {
- size = min(priv->multicast_size, netdev_mc_count(usbdev->net));
- buf = kmalloc(size * ETH_ALEN, GFP_KERNEL);
- if (!buf) {
+ } else if (mc_count) {
+ int i = 0;
+
+ mc_addrs = kmalloc(mc_count * ETH_ALEN, GFP_ATOMIC);
+ if (!mc_addrs) {
netdev_warn(usbdev->net,
"couldn't alloc %d bytes of memory\n",
- size * ETH_ALEN);
+ mc_count * ETH_ALEN);
netif_addr_unlock_bh(usbdev->net);
return;
}
- i = 0;
- netdev_for_each_mc_addr(mclist, usbdev->net) {
- if (i == size)
- break;
- memcpy(buf + i++ * ETH_ALEN, mclist->dmi_addr, ETH_ALEN);
- }
+ netdev_for_each_mc_addr(mclist, usbdev->net)
+ memcpy(mc_addrs + i++ * ETH_ALEN,
+ mclist->dmi_addr, ETH_ALEN);
+ }
+ netif_addr_unlock_bh(usbdev->net);
- ret = rndis_set_oid(usbdev, OID_802_3_MULTICAST_LIST, buf,
- i * ETH_ALEN);
- if (ret == 0 && i > 0)
+ if (filter != basefilter)
+ goto set_filter;
+
+ if (mc_count) {
+ ret = rndis_set_oid(usbdev, OID_802_3_MULTICAST_LIST, mc_addrs,
+ mc_count * ETH_ALEN);
+ kfree(mc_addrs);
+ if (ret == 0)
filter |= RNDIS_PACKET_TYPE_MULTICAST;
else
filter |= RNDIS_PACKET_TYPE_ALL_MULTICAST;
netdev_dbg(usbdev->net, "OID_802_3_MULTICAST_LIST(%d, max: %d) -> %d\n",
- i, priv->multicast_size, ret);
-
- kfree(buf);
+ mc_count, priv->multicast_size, ret);
}
- netif_addr_unlock_bh(usbdev->net);
+set_filter:
ret = rndis_set_oid(usbdev, OID_GEN_CURRENT_PACKET_FILTER, &filter,
sizeof(filter));
if (ret < 0) {
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [net-next-2.6 PATCH] wireless: convert to use netdev_for_each_mc_addr
2010-03-04 9:20 ` Jiri Pirko
@ 2010-03-04 11:38 ` David Miller
0 siblings, 0 replies; 14+ messages in thread
From: David Miller @ 2010-03-04 11:38 UTC (permalink / raw)
To: jpirko; +Cc: netdev, jussi.kivilinna, linux-wireless
From: Jiri Pirko <jpirko@redhat.com>
Date: Thu, 4 Mar 2010 10:20:14 +0100
> Slapping myself twice on each cheek.
>
> Subject: [net-2.6 PATCH] rndis_wlan: correct multicast_list handling V3
>
> My previous patch (655ffee284dfcf9a24ac0343f3e5ee6db85b85c5) added locking in
> a bad way. Because rndis_set_oid can sleep, there is need to prepare multicast
> addresses into local buffer under netif_addr_lock first, then call
> rndis_set_oid outside. This caused reorganizing of the whole function.
>
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
> Reported-by: Jussi Kivilinna <jussi.kivilinna@mbnet.fi>
Applied.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [net-next-2.6 PATCH] wireless: convert to use netdev_for_each_mc_addr
2010-02-27 17:35 Jiri Pirko
@ 2010-02-28 9:44 ` David Miller
0 siblings, 0 replies; 14+ messages in thread
From: David Miller @ 2010-02-28 9:44 UTC (permalink / raw)
To: jpirko; +Cc: netdev
From: Jiri Pirko <jpirko@redhat.com>
Date: Sat, 27 Feb 2010 18:35:45 +0100
>
> also added missed locking in rndis_wlan.c
>
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
Applied, thanks Jiri.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [net-next-2.6 PATCH] wireless: convert to use netdev_for_each_mc_addr
@ 2010-02-27 17:35 Jiri Pirko
2010-02-28 9:44 ` David Miller
0 siblings, 1 reply; 14+ messages in thread
From: Jiri Pirko @ 2010-02-27 17:35 UTC (permalink / raw)
To: netdev; +Cc: davem
also added missed locking in rndis_wlan.c
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
drivers/net/wireless/libertas/main.c | 8 ++++++--
drivers/net/wireless/orinoco/hw.c | 22 +++++++---------------
drivers/net/wireless/orinoco/hw.h | 2 +-
drivers/net/wireless/orinoco/main.c | 3 +--
drivers/net/wireless/ray_cs.c | 8 ++++----
drivers/net/wireless/rndis_wlan.c | 15 ++++++++-------
drivers/net/wireless/zd1201.c | 9 ++++-----
7 files changed, 31 insertions(+), 36 deletions(-)
diff --git a/drivers/net/wireless/libertas/main.c b/drivers/net/wireless/libertas/main.c
index cd8ed7f..28a1c9d 100644
--- a/drivers/net/wireless/libertas/main.c
+++ b/drivers/net/wireless/libertas/main.c
@@ -319,15 +319,18 @@ static int lbs_add_mcast_addrs(struct cmd_ds_mac_multicast_adr *cmd,
{
int i = nr_addrs;
struct dev_mc_list *mc_list;
+ int cnt;
if ((dev->flags & (IFF_UP|IFF_MULTICAST)) != (IFF_UP|IFF_MULTICAST))
return nr_addrs;
netif_addr_lock_bh(dev);
- for (mc_list = dev->mc_list; mc_list; mc_list = mc_list->next) {
+ cnt = netdev_mc_count(dev);
+ netdev_for_each_mc_addr(mc_list, dev) {
if (mac_in_list(cmd->maclist, nr_addrs, mc_list->dmi_addr)) {
lbs_deb_net("mcast address %s:%pM skipped\n", dev->name,
mc_list->dmi_addr);
+ cnt--;
continue;
}
@@ -337,9 +340,10 @@ static int lbs_add_mcast_addrs(struct cmd_ds_mac_multicast_adr *cmd,
lbs_deb_net("mcast address %s:%pM added to filter\n", dev->name,
mc_list->dmi_addr);
i++;
+ cnt--;
}
netif_addr_unlock_bh(dev);
- if (mc_list)
+ if (cnt)
return -EOVERFLOW;
return i;
diff --git a/drivers/net/wireless/orinoco/hw.c b/drivers/net/wireless/orinoco/hw.c
index 404830f..e636924 100644
--- a/drivers/net/wireless/orinoco/hw.c
+++ b/drivers/net/wireless/orinoco/hw.c
@@ -1028,7 +1028,7 @@ int orinoco_clear_tkip_key(struct orinoco_private *priv, int key_idx)
}
int __orinoco_hw_set_multicast_list(struct orinoco_private *priv,
- struct dev_addr_list *mc_list,
+ struct net_device *dev,
int mc_count, int promisc)
{
hermes_t *hw = &priv->hw;
@@ -1049,24 +1049,16 @@ int __orinoco_hw_set_multicast_list(struct orinoco_private *priv,
* group address if either we want to multicast, or if we were
* multicasting and want to stop */
if (!promisc && (mc_count || priv->mc_count)) {
- struct dev_mc_list *p = mc_list;
+ struct dev_mc_list *p;
struct hermes_multicast mclist;
- int i;
+ int i = 0;
- for (i = 0; i < mc_count; i++) {
- /* paranoia: is list shorter than mc_count? */
- BUG_ON(!p);
- /* paranoia: bad address size in list? */
- BUG_ON(p->dmi_addrlen != ETH_ALEN);
-
- memcpy(mclist.addr[i], p->dmi_addr, ETH_ALEN);
- p = p->next;
+ netdev_for_each_mc_addr(p, dev) {
+ if (i == mc_count)
+ break;
+ memcpy(mclist.addr[i++], p->dmi_addr, ETH_ALEN);
}
- if (p)
- printk(KERN_WARNING "%s: Multicast list is "
- "longer than mc_count\n", priv->ndev->name);
-
err = hermes_write_ltv(hw, USER_BAP,
HERMES_RID_CNFGROUPADDRESSES,
HERMES_BYTES_TO_RECLEN(mc_count * ETH_ALEN),
diff --git a/drivers/net/wireless/orinoco/hw.h b/drivers/net/wireless/orinoco/hw.h
index e2f7fdc..9799a1d 100644
--- a/drivers/net/wireless/orinoco/hw.h
+++ b/drivers/net/wireless/orinoco/hw.h
@@ -43,7 +43,7 @@ int __orinoco_hw_set_tkip_key(struct orinoco_private *priv, int key_idx,
u8 *tsc, size_t tsc_len);
int orinoco_clear_tkip_key(struct orinoco_private *priv, int key_idx);
int __orinoco_hw_set_multicast_list(struct orinoco_private *priv,
- struct dev_addr_list *mc_list,
+ struct net_device *dev,
int mc_count, int promisc);
int orinoco_hw_get_essid(struct orinoco_private *priv, int *active,
char buf[IW_ESSID_MAX_SIZE+1]);
diff --git a/drivers/net/wireless/orinoco/main.c b/drivers/net/wireless/orinoco/main.c
index a9e9cea..b42634c 100644
--- a/drivers/net/wireless/orinoco/main.c
+++ b/drivers/net/wireless/orinoco/main.c
@@ -1676,8 +1676,7 @@ __orinoco_set_multicast_list(struct net_device *dev)
mc_count = netdev_mc_count(dev);
}
- err = __orinoco_hw_set_multicast_list(priv, dev->mc_list, mc_count,
- promisc);
+ err = __orinoco_hw_set_multicast_list(priv, dev, mc_count, promisc);
return err;
}
diff --git a/drivers/net/wireless/ray_cs.c b/drivers/net/wireless/ray_cs.c
index 85905ca..84c530a 100644
--- a/drivers/net/wireless/ray_cs.c
+++ b/drivers/net/wireless/ray_cs.c
@@ -1871,10 +1871,8 @@ static void ray_update_parm(struct net_device *dev, UCHAR objid, UCHAR *value,
/*===========================================================================*/
static void ray_update_multi_list(struct net_device *dev, int all)
{
- struct dev_mc_list *dmi, **dmip;
int ccsindex;
struct ccs __iomem *pccs;
- int i = 0;
ray_dev_t *local = netdev_priv(dev);
struct pcmcia_device *link = local->finder;
void __iomem *p = local->sram + HOST_TO_ECF_BASE;
@@ -1895,9 +1893,11 @@ static void ray_update_multi_list(struct net_device *dev, int all)
writeb(0xff, &pccs->var);
local->num_multi = 0xff;
} else {
+ struct dev_mc_list *dmi;
+ int i = 0;
+
/* Copy the kernel's list of MC addresses to card */
- for (dmip = &dev->mc_list; (dmi = *dmip) != NULL;
- dmip = &dmi->next) {
+ netdev_for_each_mc_addr(dmi, dev) {
memcpy_toio(p, dmi->dmi_addr, ETH_ALEN);
dev_dbg(&link->dev,
"ray_update_multi add addr %02x%02x%02x%02x%02x%02x\n",
diff --git a/drivers/net/wireless/rndis_wlan.c b/drivers/net/wireless/rndis_wlan.c
index 65cbd06..9f6d6bf 100644
--- a/drivers/net/wireless/rndis_wlan.c
+++ b/drivers/net/wireless/rndis_wlan.c
@@ -1502,6 +1502,7 @@ static void set_multicast_list(struct usbnet *usbdev)
filter = RNDIS_PACKET_TYPE_DIRECTED | RNDIS_PACKET_TYPE_BROADCAST;
+ netif_addr_lock_bh(usbdev->net);
if (usbdev->net->flags & IFF_PROMISC) {
filter |= RNDIS_PACKET_TYPE_PROMISCUOUS |
RNDIS_PACKET_TYPE_ALL_LOCAL;
@@ -1515,16 +1516,15 @@ static void set_multicast_list(struct usbnet *usbdev)
netdev_warn(usbdev->net,
"couldn't alloc %d bytes of memory\n",
size * ETH_ALEN);
+ netif_addr_unlock_bh(usbdev->net);
return;
}
- mclist = usbdev->net->mc_list;
- for (i = 0; i < size && mclist; mclist = mclist->next) {
- if (mclist->dmi_addrlen != ETH_ALEN)
- continue;
-
- memcpy(buf + i * ETH_ALEN, mclist->dmi_addr, ETH_ALEN);
- i++;
+ i = 0;
+ netdev_for_each_mc_addr(mclist, usbdev->net) {
+ if (i == size)
+ break;
+ memcpy(buf + i++ * ETH_ALEN, mclist->dmi_addr, ETH_ALEN);
}
ret = rndis_set_oid(usbdev, OID_802_3_MULTICAST_LIST, buf,
@@ -1539,6 +1539,7 @@ static void set_multicast_list(struct usbnet *usbdev)
kfree(buf);
}
+ netif_addr_unlock_bh(usbdev->net);
ret = rndis_set_oid(usbdev, OID_GEN_CURRENT_PACKET_FILTER, &filter,
sizeof(filter));
diff --git a/drivers/net/wireless/zd1201.c b/drivers/net/wireless/zd1201.c
index 5d2b52f..6917286 100644
--- a/drivers/net/wireless/zd1201.c
+++ b/drivers/net/wireless/zd1201.c
@@ -875,17 +875,16 @@ static struct iw_statistics *zd1201_get_wireless_stats(struct net_device *dev)
static void zd1201_set_multicast(struct net_device *dev)
{
struct zd1201 *zd = netdev_priv(dev);
- struct dev_mc_list *mc = dev->mc_list;
+ struct dev_mc_list *mc;
unsigned char reqbuf[ETH_ALEN*ZD1201_MAXMULTI];
int i;
if (netdev_mc_count(dev) > ZD1201_MAXMULTI)
return;
- for (i=0; i<netdev_mc_count(dev); i++) {
- memcpy(reqbuf+i*ETH_ALEN, mc->dmi_addr, ETH_ALEN);
- mc = mc->next;
- }
+ i = 0;
+ netdev_for_each_mc_addr(mc, dev)
+ memcpy(reqbuf + i++ * ETH_ALEN, mc->dmi_addr, ETH_ALEN);
zd1201_setconfig(zd, ZD1201_RID_CNFGROUPADDRESS, reqbuf,
netdev_mc_count(dev) * ETH_ALEN, 0);
}
--
1.6.6
^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2010-03-04 11:37 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-03-03 9:45 [net-next-2.6 PATCH] wireless: convert to use netdev_for_each_mc_addr Jussi Kivilinna
2010-03-03 10:01 ` David Miller
2010-03-03 12:42 ` Jiri Pirko
2010-03-03 16:42 ` Jussi Kivilinna
2010-03-03 17:52 ` Jiri Pirko
2010-03-03 18:05 ` Jussi Kivilinna
2010-03-03 18:09 ` Jiri Pirko
2010-03-04 8:41 ` David Miller
2010-03-04 8:52 ` David Miller
2010-03-04 8:52 ` David Miller
2010-03-04 9:20 ` Jiri Pirko
2010-03-04 11:38 ` David Miller
-- strict thread matches above, loose matches on Subject: below --
2010-02-27 17:35 Jiri Pirko
2010-02-28 9:44 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.