Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH]: tipc: Fix oops on send prior to entering networked mode
From: Stephens, Allan @ 2010-02-23 16:21 UTC (permalink / raw)
  To: Neil Horman; +Cc: jon.maloy, netdev, tipc-discussion, davem
In-Reply-To: <20100223160936.GA16435@hmsreliant.think-freely.org>

Neil wrote: 

> I agree that you patch fixes the exact problem that I 
> reported here, but theres more to it than that.  A quick grep 
> of the tipc stack reveals the following
> symbols:
> tipc_bearers
> media_list
> tipc_local_nodes
> bcbearer
> bclink
> tipc_net.zones
> 
> All of these symbols:
> 
> 1) Are allocated dynamically in tipc_net_start, _after_ 
> tipc_mode is set to TIPC_NET_MODE
> 
> 2) dereferenced without NULL pointer checks in either the 
> send path or in the netlink configuration path, both of which 
> are reachable from user space.
> 
> So your patch fixes the last item on your list, but what 
> about the others?  In fact, I'll bet I can very quickly 
> change the application to trip over a null tipc_local_nodes 
> dereference by changing the destination address to be 
> something within zone 0, cluster 0.

The semantics of TIPC addressing don't allow a node address of the form
<0.0.N> where N != 0, so this kind of a send ateempt should be caught
and handled by TIPC.  However, you've already found one missing error
check, so it's certainly worth trying it out!

Regards,
Al

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev

^ permalink raw reply

* Re: [PATCH 2/5] C/R: Basic support for network namespaces and devices (v4)
From: Dan Smith @ 2010-02-23 16:35 UTC (permalink / raw)
  To: Serge E. Hallyn; +Cc: containers, netdev
In-Reply-To: <20100222194523.GA13135@us.ibm.com>

SH> the above two hunks change the flow in checkpoint_container(), but
SH> they don't seem to actually add anything.  And I don't see (with a
SH> quick browse) any later patch in this series changing this either.
SH> Is this just noise?

Ah, yeah, I think that's left over from a previous version where I had
to insert something there.  Sorry about that :)

>> +int ckpt_netdev_in_init_netns(struct ckpt_ctx *ctx, struct net_device *dev)
>> +{
>> +	return dev->nd_net == current->nsproxy->net_ns;
>> +}

SH> You are comparing it to the net_ns of the checkpointing task.  I'm
SH> not sure that makes sense - but I'm also not sure what if anything
SH> makes more sense.

SH> What exactly do you mean by the 'init' netns here?  Do you mean
SH> the init_net_ns for the container, or that it is the net_ns of
SH> whatever task created the container?

In this case, 'current' is the task doing the checkpoint, right?  So,
we're treating the netns that it is in as the "top level" and will
restore the tree, as visible from that task, relative to the netns of
the restart process.  We had an IRC conversation about this, I believe :)

SH> How about a
SH> 			ckpt_err(ctx, -ENOSYS,
SH> 				Device %s does not support checkpoint\n",
dev-> name);

SH> here to put a meaningful msg in the user's log?

Yep, definitely.

Thanks!

-- 
Dan Smith
IBM Linux Technology Center
email: danms@us.ibm.com

^ permalink raw reply

* Re: [RFC] IPv6: don't forward unspecified frames
From: Stephen Hemminger @ 2010-02-23 16:46 UTC (permalink / raw)
  To: Shan Wei; +Cc: David Miller, netdev
In-Reply-To: <4B836385.8090509@cn.fujitsu.com>

On Tue, 23 Feb 2010 13:11:33 +0800
Shan Wei <shanwei@cn.fujitsu.com> wrote:

> Stephen Hemminger wrote, at 02/23/2010 09:31 AM:
> > This showed up during UNH IPv6 conformance tests. It appears kernel
> > incorrectly forwards packets with unspecified source address.
> 
> Which case? Is it about spec.p2#18 of IPv6 Ready Logo Phase 2?
> I don't see the phenomenon from spec.p2#18 case.

The kernel is 2.6.31 and it has that code section in ip6_forward.
I am inprocess of trying to reproduce the result.

The test case in question is V6LC.1.1.10C


 IP Forwarding – Source and Destination Address – Intermediate Node (Routers Only)
 Purpose: Verify that a node properly forwards the ICMPv6 Echo Requests.
 Comments on Test Procedure
A. Request sent to Global Unicast address: TN2 transmits an ICMPv6 Echo Request to TN1’s Global unicast address with a first hop through the RUT. The source address is TN2’s Global address.
B. Request sent to Global Unicast address (prefix end in zero-valued fields): TN2 transmits an ICMPv6 Echo Request to TN1’s Global unicast address (prefix 8000:0000::/64) with a first hop through the RUT. The source address is TN2’s Global address.
>>> C. Request sent from unspecified address: TN2 transmits an ICMPv6 Echo Request to TN1 with a first hop through the RUT. The source address is the unspecified address (0:0:0:0:0:0:0:0).
D. Request sent to Lookback address: TN2 transmits an ICMPv6 Echo Request to the Lookback address (0:0:0:0:0:0:0:1) with a first hop through the RUT. The source address is TN2’s Global address.
E. Request sent from Link Local address: TN2 transmits an ICMPv6 Echo Request to TN1 with a first hop through the RUT. The source address is TN2’s Link Local address.
F. Request sent to Link Local address: TN2 transmits an ICMPv6 Echo Request to TN1’s Link Local address with a first hop through the RUT. The source address is TN2’s Global address.
G. Request sent to Site-Local address: TN2 transmits an ICMPv6 Echo Request to TN1’s Site-local address with a first hop through the RUT. The source address is TN2’s Global address.
H. Request sent to Global Scope multicast address: Configure multicast routing on the RUT. TN1 is a Lis-tener for the multicast group FF1E::1:2. TN2 transmits an ICMPv6 Echo Request to TN1’s Global Scope multicast address (FF1E::1:2) with a first hop through the RUT. The source address is TN2’s Global ad-dress.
I. Request sent to Link-local Scope multicast address: Configure multicast routing on the RUT. TN1 is a Lis-tener for the multicast group FF12::1:2. TN2 transmits an ICMPv6 Echo Request to TN1’s Link-Local Scope multicast address (FF12::1:2) with a first hop through the RUT. The source address is TN2’s Global address.
J. Request sent to Multicast address (Reserved Value = 0):Configure multicast routing on the RUT. TN1 is a Listener for the multicast group FF10::1:2. TN2 transmits an ICMPv6 Echo Request to multicast address with a reserved field set to zero (FF10::1:2) with a first hop through the RUT. The source address is TN2’s Global address.
K. Request sent to Multicast address (Reserved Value = F): Configure multicast routing on the RUT. TN1 is a Listener for the multicast group FF1F::1:2. 29. TN2 transmits an ICMPv6 Echo Request to TN1 multicast address with a reserved field set to zero (FF1F::1:2) with a first hop through the RUT. The source address is TN2’s Global address.

 Comments on Test Results
A. The RUT must forward the Echo Request from TN2 to TN1 with a first hop through the TR1.
B. The RUT must forward the Echo Request from TN2 to TN1 with a first hop through the TR1.
>>>C. The RUT forwarded the Echo Request from TN2.
According to RFC 4291 Section 2.5.2: “An IPv6 packet with a source address of unspecified must never be forwarded by an IPv6 router.”
Therefore the RUT should not have forwarded the Echo Request from TN2.
D. The RUT must not forward the Echo Request from TN2.
E. The RUT must not forward the Echo Request from TN2.
F. The RUT must not forward the Echo Request from TN2.
G. The RUT must forward the Echo Request from TN2 to TR1.
H. The RUT must forward the Echo Request from TN2 to TN1 with a first hop through TR1.
I. The RUT must not forward the Echo Request from TN2.
J. The RUT must not forward the Echo Request from TN2.
K. The RUT must forward the Echo Request from TN2 to TN1 with a first hop through the RUT.

^ permalink raw reply

* Re: [PATCH 2/5] C/R: Basic support for network namespaces and devices (v4)
From: Serge E. Hallyn @ 2010-02-23 16:47 UTC (permalink / raw)
  To: Dan Smith; +Cc: Serge E. Hallyn, containers, netdev
In-Reply-To: <87k4u37vv6.fsf@caffeine.danplanet.com>

Quoting Dan Smith (danms@us.ibm.com):
> SH> the above two hunks change the flow in checkpoint_container(), but
> SH> they don't seem to actually add anything.  And I don't see (with a
> SH> quick browse) any later patch in this series changing this either.
> SH> Is this just noise?
> 
> Ah, yeah, I think that's left over from a previous version where I had
> to insert something there.  Sorry about that :)
> 
> >> +int ckpt_netdev_in_init_netns(struct ckpt_ctx *ctx, struct net_device *dev)
> >> +{
> >> +	return dev->nd_net == current->nsproxy->net_ns;
> >> +}
> 
> SH> You are comparing it to the net_ns of the checkpointing task.  I'm
> SH> not sure that makes sense - but I'm also not sure what if anything
> SH> makes more sense.
> 
> SH> What exactly do you mean by the 'init' netns here?  Do you mean
> SH> the init_net_ns for the container, or that it is the net_ns of
> SH> whatever task created the container?
> 
> In this case, 'current' is the task doing the checkpoint, right?  So,
> we're treating the netns that it is in as the "top level" and will
> restore the tree, as visible from that task, relative to the netns of
> the restart process.  We had an IRC conversation about this, I believe :)

But there is no guarantee that the checkpointer is in the netns which
we would call the 'top level' netns.  Which means that, at restart, whether
or not the devices which are in what we call the top level netns are in
fact inherited or not, will depend on conditions of the checkpointer.  Do
we care?  (I thought we did, but maybe we don't... it's unlikely to happen
anyway)

> SH> How about a
> SH> 			ckpt_err(ctx, -ENOSYS,
> SH> 				Device %s does not support checkpoint\n",
> dev-> name);
> 
> SH> here to put a meaningful msg in the user's log?
> 
> Yep, definitely.
> 
> Thanks!
> 
> -- 
> Dan Smith
> IBM Linux Technology Center
> email: danms@us.ibm.com
> _______________________________________________
> Containers mailing list
> Containers@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/containers

^ permalink raw reply

* [PATCH 0/3] vhost: logging fixes
From: Michael S. Tsirkin @ 2010-02-23 16:57 UTC (permalink / raw)
  To: Rusty Russell, kvm, virtualization, netdev, linux-kernel,
	David Miller, gl

The following patches on top of net-next fix issues related to write
logging in vhost. This fixes all known to me logging issues, migration
now works for me while under stress in both TX and RX directions.
Rusty's going on vacation, I am guessing he won't have time to review
this: Gleb, Juan, Herbert, could one of you review this patchset please?

There's also the send queue full issue reported by
Sridhar Samudrala which I'm testing various fixes for,
that patch is contained to vhost/net though,
so there's no conflict, patch will be posted separately.


Michael S. Tsirkin (3):
  vhost: logging thinko fix
  vhost: initialize log eventfd context pointer
  vhost: fix get_user_pages_fast error handling

 drivers/vhost/vhost.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)

^ permalink raw reply

* [PATCH 1/3] vhost: logging math fix
From: Michael S. Tsirkin @ 2010-02-23 16:57 UTC (permalink / raw)
  To: Rusty Russell, kvm, virtualization, netdev, linux-kernel,
	David Miller, ma
In-Reply-To: <cover.1266943453.git.mst@redhat.com>

vhost was dong some complex math to get
offset to log at, and got it wrong by a couple of bytes,
while in fact it's simple: get address where we write,
subtract start of buffer, add log base.

Do it this way.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/vhost/vhost.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 6eb1525..c767279 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1004,10 +1004,12 @@ int vhost_add_used(struct vhost_virtqueue *vq, unsigned int head, int len)
 	if (unlikely(vq->log_used)) {
 		/* Make sure data is seen before log. */
 		smp_wmb();
-		log_write(vq->log_base, vq->log_addr + sizeof *vq->used->ring *
-			  (vq->last_used_idx % vq->num),
-			  sizeof *vq->used->ring);
-		log_write(vq->log_base, vq->log_addr, sizeof *vq->used->ring);
+		log_write(vq->log_base,
+			  vq->log_addr + ((void *)used - (void *)vq->used),
+			  sizeof *used);
+		log_write(vq->log_base,
+			  vq->log_addr + offsetof(struct vring_used, idx),
+			  sizeof vq->used->idx);
 		if (vq->log_ctx)
 			eventfd_signal(vq->log_ctx, 1);
 	}
-- 
1.7.0.18.g0d53a5

^ permalink raw reply related

* [PATCH 2/3] vhost: initialize log eventfd context pointer
From: Michael S. Tsirkin @ 2010-02-23 16:57 UTC (permalink / raw)
  To: Rusty Russell, kvm, virtualization, netdev, linux-kernel,
	David Miller, ma
In-Reply-To: <cover.1266943453.git.mst@redhat.com>

vq log eventfd context pointer needs to be initialized, otherwise
operation may fail or oops if log is enabled but log eventfd not set by
userspace.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/vhost/vhost.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index c767279..d4f8fdf 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -121,6 +121,7 @@ static void vhost_vq_reset(struct vhost_dev *dev,
 	vq->kick = NULL;
 	vq->call_ctx = NULL;
 	vq->call = NULL;
+	vq->log_ctx = NULL;
 }
 
 long vhost_dev_init(struct vhost_dev *dev,
-- 
1.7.0.18.g0d53a5

^ permalink raw reply related

* [PATCH 3/3] vhost: fix get_user_pages_fast error handling
From: Michael S. Tsirkin @ 2010-02-23 16:57 UTC (permalink / raw)
  To: Rusty Russell, kvm, virtualization, netdev, linux-kernel,
	David Miller, ma
In-Reply-To: <cover.1266943453.git.mst@redhat.com>

get_user_pages_fast returns number of pages on success, negative value
on failure, but never 0. Fix vhost code to match this logic.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/vhost/vhost.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index d4f8fdf..d003504 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -646,8 +646,9 @@ static int set_bit_to_user(int nr, void __user *addr)
 	int bit = nr + (log % PAGE_SIZE) * 8;
 	int r;
 	r = get_user_pages_fast(log, 1, 1, &page);
-	if (r)
+	if (r < 0)
 		return r;
+	BUG_ON(r != 1);
 	base = kmap_atomic(page, KM_USER0);
 	set_bit(bit, base);
 	kunmap_atomic(base, KM_USER0);
-- 
1.7.0.18.g0d53a5

^ permalink raw reply related

* Re: [PATCH 2/5] C/R: Basic support for network namespaces and devices (v4)
From: Dan Smith @ 2010-02-23 17:27 UTC (permalink / raw)
  To: Serge E. Hallyn; +Cc: Serge E. Hallyn, containers, netdev
In-Reply-To: <20100223164755.GA31671@hallyn.com>

SH> But there is no guarantee that the checkpointer is in the netns
SH> which we would call the 'top level' netns.  Which means that, at
SH> restart, whether or not the devices which are in what we call the
SH> top level netns are in fact inherited or not, will depend on
SH> conditions of the checkpointer.  Do we care?  (I thought we did,
SH> but maybe we don't... it's unlikely to happen anyway)

Well, when we discussed this on IRC with Oren, I think we came to the
conclusion that since network namespaces aren't hierarchical, that we
would restore things from the "viewpoint" of the process that
checkpointed them.  It gives us a sane way to ensure that the peer
devices residing in the init netns can be put back there, even though we
don't checkpoint everything in the init netns (like eth0).

If you checkpoint a veth from within the container and you have a peer
device that is outside the container (but not in a netns that is
checkpointed as part of a task), it's going to fail and tell you that
one of your peers leaked to the outside.  I think that's sane and
preferred behavior, no?  If you're using macvlan and you checkpoint
from within the container, I think you should be okay, as long as
there is a appropriately named device to base the restored devices on
in whatever netns your restore process is in.

-- 
Dan Smith
IBM Linux Technology Center
email: danms@us.ibm.com

^ permalink raw reply

* Re: [PATCH net-next-2.6] vhost: Restart tx poll when socket send queue is full
From: Sridhar Samudrala @ 2010-02-23 17:31 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: David Miller, netdev
In-Reply-To: <20100223102437.GA23835@redhat.com>

On Tue, 2010-02-23 at 12:24 +0200, Michael S. Tsirkin wrote:
> On Thu, Feb 18, 2010 at 12:59:11PM -0800, Sridhar Samudrala wrote:
> > When running guest to remote host TCP stream test using vhost-net
> > via tap/macvtap, i am seeing network transmit hangs. This happens
> > when handle_tx() returns because of the socket send queue full 
> > condition.
> > This patch fixes this by restarting tx poll when hitting this
> > condition.
> > 
> > Signed-off-by: Sridhar Samudrala <sri@us.ibm.com>
> > 
> > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > index 91a324c..82d4bbe 100644
> > --- a/drivers/vhost/net.c
> > +++ b/drivers/vhost/net.c
> > @@ -113,12 +113,16 @@ static void handle_tx(struct vhost_net *net)
> >  	if (!sock)
> >  		return;
> >  
> > -	wmem = atomic_read(&sock->sk->sk_wmem_alloc);
> > -	if (wmem >= sock->sk->sk_sndbuf)
> > -		return;
> > -
> >  	use_mm(net->dev.mm);
> >  	mutex_lock(&vq->mutex);
> > +
> > +	wmem = atomic_read(&sock->sk->sk_wmem_alloc);
> > +	if (wmem >= sock->sk->sk_sndbuf) {
> > +		tx_poll_start(net, sock);
> > +		set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> > +		goto unlock;
> > +	}
> > +
> >  	vhost_disable_notify(vq);
> >  
> >  	if (wmem < sock->sk->sk_sndbuf * 2)
> > @@ -178,6 +182,7 @@ static void handle_tx(struct vhost_net *net)
> >  		}
> >  	}
> >  
> > +unlock:
> >  	mutex_unlock(&vq->mutex);
> >  	unuse_mm(net->dev.mm);
> >  }
> 
> 
> It might be better to avoid use_mm when ring is full.
> Does the following fix the tx hang for you?

Yes. this fixes the tx hang.

Thanks
Sridhar

> 
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 4c89283..f5f6efe 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -113,8 +113,12 @@ static void handle_tx(struct vhost_net *net)
>  		return;
> 
>  	wmem = atomic_read(&sock->sk->sk_wmem_alloc);
> -	if (wmem >= sock->sk->sk_sndbuf)
> -		return;
> +	if (wmem >= sock->sk->sk_sndbuf) {
> +		mutex_lock(&vq->mutex);
> +		tx_poll_start(net, sock);
> +		mutex_unlock(&vq->mutex);
> +                return;
> +	}
> 
>  	use_mm(net->dev.mm);
>  	mutex_lock(&vq->mutex);
> --
> 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

* Re: [PATCH 3/3] vhost: fix get_user_pages_fast error handling
From: Michael S. Tsirkin @ 2010-02-23 17:32 UTC (permalink / raw)
  To: Gleb Natapov
  Cc: Rusty Russell, kvm, virtualization, netdev, linux-kernel,
	David Miller, markmc, herbert.xu, quintela, dlaor, avi
In-Reply-To: <20100223173434.GB9834@redhat.com>

On Tue, Feb 23, 2010 at 07:34:34PM +0200, Gleb Natapov wrote:
> On Tue, Feb 23, 2010 at 06:57:58PM +0200, Michael S. Tsirkin wrote:
> > get_user_pages_fast returns number of pages on success, negative value
> > on failure, but never 0. Fix vhost code to match this logic.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  drivers/vhost/vhost.c |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > index d4f8fdf..d003504 100644
> > --- a/drivers/vhost/vhost.c
> > +++ b/drivers/vhost/vhost.c
> > @@ -646,8 +646,9 @@ static int set_bit_to_user(int nr, void __user *addr)
> >  	int bit = nr + (log % PAGE_SIZE) * 8;
> >  	int r;
> >  	r = get_user_pages_fast(log, 1, 1, &page);
> > -	if (r)
> > +	if (r < 0)
> >  		return r;
> > +	BUG_ON(r != 1);
> Can't this be easily triggered from user space?

I think no. get_user_pages_fast always returns number of pages
pinned (in this case always 1) or an error (< 0).
Anything else is a kernel bug.

> >  	base = kmap_atomic(page, KM_USER0);
> >  	set_bit(bit, base);
> >  	kunmap_atomic(base, KM_USER0);
> > -- 
> > 1.7.0.18.g0d53a5
> 
> --
> 			Gleb.

^ permalink raw reply

* Re: [PATCH 3/3] vhost: fix get_user_pages_fast error handling
From: Gleb Natapov @ 2010-02-23 17:34 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Rusty Russell, kvm, virtualization, netdev, linux-kernel,
	David Miller, markmc, herbert.xu, quintela, dlaor, avi
In-Reply-To: <82ba8c97ce55dd4bf9972ad755961cd14e6a0938.1266943453.git.mst@redhat.com>

On Tue, Feb 23, 2010 at 06:57:58PM +0200, Michael S. Tsirkin wrote:
> get_user_pages_fast returns number of pages on success, negative value
> on failure, but never 0. Fix vhost code to match this logic.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  drivers/vhost/vhost.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index d4f8fdf..d003504 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -646,8 +646,9 @@ static int set_bit_to_user(int nr, void __user *addr)
>  	int bit = nr + (log % PAGE_SIZE) * 8;
>  	int r;
>  	r = get_user_pages_fast(log, 1, 1, &page);
> -	if (r)
> +	if (r < 0)
>  		return r;
> +	BUG_ON(r != 1);
Can't this be easily triggered from user space?

>  	base = kmap_atomic(page, KM_USER0);
>  	set_bit(bit, base);
>  	kunmap_atomic(base, KM_USER0);
> -- 
> 1.7.0.18.g0d53a5

--
			Gleb.

^ permalink raw reply

* Re: [PATCH 3/3] vhost: fix get_user_pages_fast error handling
From: Michael S. Tsirkin @ 2010-02-23 17:39 UTC (permalink / raw)
  To: Gleb Natapov
  Cc: Rusty Russell, kvm, virtualization, netdev, linux-kernel,
	David Miller, markmc, herbert.xu, quintela, dlaor, avi
In-Reply-To: <20100223173952.GC9834@redhat.com>

On Tue, Feb 23, 2010 at 07:39:52PM +0200, Gleb Natapov wrote:
> On Tue, Feb 23, 2010 at 07:32:58PM +0200, Michael S. Tsirkin wrote:
> > On Tue, Feb 23, 2010 at 07:34:34PM +0200, Gleb Natapov wrote:
> > > On Tue, Feb 23, 2010 at 06:57:58PM +0200, Michael S. Tsirkin wrote:
> > > > get_user_pages_fast returns number of pages on success, negative value
> > > > on failure, but never 0. Fix vhost code to match this logic.
> > > > 
> > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > > ---
> > > >  drivers/vhost/vhost.c |    3 ++-
> > > >  1 files changed, 2 insertions(+), 1 deletions(-)
> > > > 
> > > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > > index d4f8fdf..d003504 100644
> > > > --- a/drivers/vhost/vhost.c
> > > > +++ b/drivers/vhost/vhost.c
> > > > @@ -646,8 +646,9 @@ static int set_bit_to_user(int nr, void __user *addr)
> > > >  	int bit = nr + (log % PAGE_SIZE) * 8;
> > > >  	int r;
> > > >  	r = get_user_pages_fast(log, 1, 1, &page);
> > > > -	if (r)
> > > > +	if (r < 0)
> > > >  		return r;
> > > > +	BUG_ON(r != 1);
> > > Can't this be easily triggered from user space?
> > 
> > I think no. get_user_pages_fast always returns number of pages
> > pinned (in this case always 1) or an error (< 0).
> > Anything else is a kernel bug.
> > 
> But what if page is unmapped from userspace?

Then we get -EFAULT

> > > >  	base = kmap_atomic(page, KM_USER0);
> > > >  	set_bit(bit, base);
> > > >  	kunmap_atomic(base, KM_USER0);
> > > > -- 
> > > > 1.7.0.18.g0d53a5
> > > 
> > > --
> > > 			Gleb.
> 
> --
> 			Gleb.

^ permalink raw reply

* Re: [PATCH 3/3] vhost: fix get_user_pages_fast error handling
From: Gleb Natapov @ 2010-02-23 17:39 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Rusty Russell, kvm, virtualization, netdev, linux-kernel,
	David Miller, markmc, herbert.xu, quintela, dlaor, avi
In-Reply-To: <20100223173258.GA25338@redhat.com>

On Tue, Feb 23, 2010 at 07:32:58PM +0200, Michael S. Tsirkin wrote:
> On Tue, Feb 23, 2010 at 07:34:34PM +0200, Gleb Natapov wrote:
> > On Tue, Feb 23, 2010 at 06:57:58PM +0200, Michael S. Tsirkin wrote:
> > > get_user_pages_fast returns number of pages on success, negative value
> > > on failure, but never 0. Fix vhost code to match this logic.
> > > 
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > ---
> > >  drivers/vhost/vhost.c |    3 ++-
> > >  1 files changed, 2 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > index d4f8fdf..d003504 100644
> > > --- a/drivers/vhost/vhost.c
> > > +++ b/drivers/vhost/vhost.c
> > > @@ -646,8 +646,9 @@ static int set_bit_to_user(int nr, void __user *addr)
> > >  	int bit = nr + (log % PAGE_SIZE) * 8;
> > >  	int r;
> > >  	r = get_user_pages_fast(log, 1, 1, &page);
> > > -	if (r)
> > > +	if (r < 0)
> > >  		return r;
> > > +	BUG_ON(r != 1);
> > Can't this be easily triggered from user space?
> 
> I think no. get_user_pages_fast always returns number of pages
> pinned (in this case always 1) or an error (< 0).
> Anything else is a kernel bug.
> 
But what if page is unmapped from userspace?

> > >  	base = kmap_atomic(page, KM_USER0);
> > >  	set_bit(bit, base);
> > >  	kunmap_atomic(base, KM_USER0);
> > > -- 
> > > 1.7.0.18.g0d53a5
> > 
> > --
> > 			Gleb.

--
			Gleb.

^ permalink raw reply

* Re: [PATCH 3/3] vhost: fix get_user_pages_fast error handling
From: Gleb Natapov @ 2010-02-23 17:43 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Rusty Russell, kvm, virtualization, netdev, linux-kernel,
	David Miller, markmc, herbert.xu, quintela, dlaor, avi
In-Reply-To: <20100223173908.GB25338@redhat.com>

On Tue, Feb 23, 2010 at 07:39:08PM +0200, Michael S. Tsirkin wrote:
> On Tue, Feb 23, 2010 at 07:39:52PM +0200, Gleb Natapov wrote:
> > On Tue, Feb 23, 2010 at 07:32:58PM +0200, Michael S. Tsirkin wrote:
> > > On Tue, Feb 23, 2010 at 07:34:34PM +0200, Gleb Natapov wrote:
> > > > On Tue, Feb 23, 2010 at 06:57:58PM +0200, Michael S. Tsirkin wrote:
> > > > > get_user_pages_fast returns number of pages on success, negative value
> > > > > on failure, but never 0. Fix vhost code to match this logic.
> > > > > 
> > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > > > ---
> > > > >  drivers/vhost/vhost.c |    3 ++-
> > > > >  1 files changed, 2 insertions(+), 1 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > > > index d4f8fdf..d003504 100644
> > > > > --- a/drivers/vhost/vhost.c
> > > > > +++ b/drivers/vhost/vhost.c
> > > > > @@ -646,8 +646,9 @@ static int set_bit_to_user(int nr, void __user *addr)
> > > > >  	int bit = nr + (log % PAGE_SIZE) * 8;
> > > > >  	int r;
> > > > >  	r = get_user_pages_fast(log, 1, 1, &page);
> > > > > -	if (r)
> > > > > +	if (r < 0)
> > > > >  		return r;
> > > > > +	BUG_ON(r != 1);
> > > > Can't this be easily triggered from user space?
> > > 
> > > I think no. get_user_pages_fast always returns number of pages
> > > pinned (in this case always 1) or an error (< 0).
> > > Anything else is a kernel bug.
> > > 
> > But what if page is unmapped from userspace?
> 
> Then we get -EFAULT
> 
Ah correct.

> > > > >  	base = kmap_atomic(page, KM_USER0);
> > > > >  	set_bit(bit, base);
> > > > >  	kunmap_atomic(base, KM_USER0);
> > > > > -- 
> > > > > 1.7.0.18.g0d53a5
> > > > 
> > > > --
> > > > 			Gleb.
> > 
> > --
> > 			Gleb.

--
			Gleb.

^ permalink raw reply

* Re: [RFC PATCH 1/1] igb: add tracing ability with ring_buffer
From: Alexander Duyck @ 2010-02-23 18:24 UTC (permalink / raw)
  To: Koki Sanagi
  Cc: netdev@vger.kernel.org, Taku Izumi,
	kaneshige.kenji@jp.fujitsu.com, e1000-devel@lists.sourceforge.net,
	davem@davemloft.net, Kirsher, Jeffrey T, Brandeburg, Jesse,
	Allan, Bruce W, Waskiewicz Jr, Peter P, Ronciak, John
In-Reply-To: <4B836F63.4050901@jp.fujitsu.com>

The biggest issue I see is that this patch is adding over 600 lines to 
the igb driver, and to make similar changes to our other drivers would 
mean adding thousands of lines of code just for some debugging output.

Is there any way igb_trace.[ch] could be renamed and moved into a 
network driver agnostic location so that it could be reused by all the 
drivers that wish to implement such functionality?

Thanks,

Alex

Koki Sanagi wrote:
> This patch adds a tracing ability to igb driver using ring_buffer and debugfs.
> Traced locations are transmit, clean tx and clean rx.
> Outputs are like this,
> 
> [  0] 74955.641155: xmit qidx=1 ntu=64->66
> [  1] 74955.641170: clean_tx qidx=1 ntc=64->66
> [  0] 74955.641194: clean_rx qidx=0 ntc=151->152
> [  0] 74955.641205: xmit qidx=1 ntu=66->68
> [  1] 74955.641220: clean_tx qidx=1 ntc=66->68
> [  0] 74955.641244: clean_rx qidx=0 ntc=152->153
> 
> These information make tx/rx ring's state visible.
> Parsing above output, we can find out how long it takes transmited packet to be
> cleaned.
> For example,
> 
> transmit_elapsed_time
> queue=0:
> ------------------------
> avg=    0.013540msec
> max=    2.495185msec
> 
>     less 1msec 31000
>       1-10msec   760
>     10-100msec     0
>   100-1000msec     0
> over 1000msec     0
> ------------------------
> total         31760
> 
> This information is helpful. Beacause sometimes igb or another intel driver
> says "Tx Unit Hang". This message indicates tx descriptor ring's process is
> delayed.
> So we must take some measures(change Interrput throttle rate, TSO, num of desc,
> or redesign network). If there is this information, we can check that a measure
> we taked is effective or not.
> On the other hand, rx descriptor is difficult to be visible. Because we cannot
> check whether packet is in descriptor or not without reading register RDH.
> But using informaiton how much descriptors are processed(it is near budget),
> we can some find out tx ring's state.
> 
> HOW TO USE:
> 
> # mount -t debugfs nodev /sys/kernel/debug
> # cd /sys/kernel/debug/igb/eth1
> # ls
> trace trace_size
> # echo 1000000 > trace_size
> # cat trace
> [  0] 74955.641155: xmit qidx=1 ntu=64->66
> [  1] 74955.641170: clean_tx qidx=1 ntc=64->66
> [  0] 74955.641194: clean_rx qidx=0 ntc=151->152
> [  0] 74955.641205: xmit qidx=1 ntu=66->68
> [  1] 74955.641220: clean_tx qidx=1 ntc=66->68
> [  0] 74955.641244: clean_rx qidx=0 ntc=152->153
> 
> -trace       output of traced record.
> -trace_size  size of ring_buffer per cpu. If 0, trace is disable.(default 0)
> 
> Signed-off-by: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
> ---
>   drivers/net/igb/Makefile    |    2 +-
>   drivers/net/igb/igb.h       |   10 +
>   drivers/net/igb/igb_main.c  |   11 +-
>   drivers/net/igb/igb_trace.c |  594 +++++++++++++++++++++++++++++++++++++++++++
>   drivers/net/igb/igb_trace.h |   29 ++
>   5 files changed, 643 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/igb/Makefile b/drivers/net/igb/Makefile
> index 8372cb9..286541e 100644
> --- a/drivers/net/igb/Makefile
> +++ b/drivers/net/igb/Makefile
> @@ -33,5 +33,5 @@
>   obj-$(CONFIG_IGB) += igb.o
> 
>   igb-objs := igb_main.o igb_ethtool.o e1000_82575.o \
> -           e1000_mac.o e1000_nvm.o e1000_phy.o e1000_mbx.o
> +           e1000_mac.o e1000_nvm.o e1000_phy.o e1000_mbx.o igb_trace.o
> 
> diff --git a/drivers/net/igb/igb.h b/drivers/net/igb/igb.h
> index b1c1eb8..fc944b5 100644
> --- a/drivers/net/igb/igb.h
> +++ b/drivers/net/igb/igb.h
> @@ -237,6 +237,15 @@ static inline int igb_desc_unused(struct igb_ring *ring)
>         return ring->count + ring->next_to_clean - ring->next_to_use - 1;
>   }
> 
> +struct igb_trace {
> +       struct dentry *if_dir;
> +       struct dentry *trace_file;
> +       struct dentry *trace_size_file;
> +       struct ring_buffer *trace_buffer;
> +       unsigned long trace_size;
> +       struct mutex trace_lock;
> +};
> +
>   /* board specific private data structure */
> 
>   struct igb_adapter {
> @@ -313,6 +322,7 @@ struct igb_adapter {
>         unsigned int vfs_allocated_count;
>         struct vf_data_storage *vf_data;
>         u32 rss_queues;
> +       struct igb_trace trace;
>   };
> 
>   #define IGB_FLAG_HAS_MSI           (1 << 0)
> diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
> index 0a064ce..1396bfe 100644
> --- a/drivers/net/igb/igb_main.c
> +++ b/drivers/net/igb/igb_main.c
> @@ -48,6 +48,7 @@
>   #include <linux/dca.h>
>   #endif
>   #include "igb.h"
> +#include "igb_trace.h"
> 
>   #define DRV_VERSION "2.1.0-k2"
>   char igb_driver_name[] = "igb";
> @@ -268,6 +269,7 @@ static int __init igb_init_module(void)
>   #ifdef CONFIG_IGB_DCA
>         dca_register_notify(&dca_notifier);
>   #endif
> +       igb_trace_init();
>         ret = pci_register_driver(&igb_driver);
>         return ret;
>   }
> @@ -285,6 +287,7 @@ static void __exit igb_exit_module(void)
>   #ifdef CONFIG_IGB_DCA
>         dca_unregister_notify(&dca_notifier);
>   #endif
> +       igb_trace_exit();
>         pci_unregister_driver(&igb_driver);
>   }
> 
> @@ -1654,6 +1657,7 @@ static int __devinit igb_probe(struct pci_dev *pdev,
>                 (adapter->flags & IGB_FLAG_HAS_MSI) ? "MSI" : "legacy",
>                 adapter->num_rx_queues, adapter->num_tx_queues);
> 
> +       igb_create_debugfs_file(adapter);
>         return 0;
> 
>   err_register:
> @@ -1709,7 +1713,7 @@ static void __devexit igb_remove(struct pci_dev *pdev)
>                 wr32(E1000_DCA_CTRL, E1000_DCA_CTRL_DCA_MODE_DISABLE);
>         }
>   #endif
> -
> +       igb_remove_debugfs_file(adapter);
>         /* Release control of h/w to f/w.  If f/w is AMT enabled, this
>          * would have already happened in close and is redundant. */
>         igb_release_hw_control(adapter);
> @@ -3796,6 +3800,7 @@ netdev_tx_t igb_xmit_frame_ring_adv(struct sk_buff *skb,
>         }
> 
>         igb_tx_queue_adv(tx_ring, tx_flags, count, skb->len, hdr_len);
> +       IGB_WRITE_TRACE_BUFFER(IGB_TRACE_XMIT, adapter, tx_ring, &first);
> 
>         /* Make sure there is space in the ring for the next send. */
>         igb_maybe_stop_tx(tx_ring, MAX_SKB_FRAGS + 4);
> @@ -4960,7 +4965,7 @@ static bool igb_clean_tx_irq(struct igb_q_vector *q_vector)
>                 eop = tx_ring->buffer_info[i].next_to_watch;
>                 eop_desc = E1000_TX_DESC_ADV(*tx_ring, eop);
>         }
> -
> +       IGB_WRITE_TRACE_BUFFER(IGB_TRACE_CLEAN_TX, adapter, tx_ring, &i);
>         tx_ring->next_to_clean = i;
> 
>         if (unlikely(count &&
> @@ -5115,6 +5120,7 @@ static bool igb_clean_rx_irq_adv(struct igb_q_vector *q_vector,
>                                    int *work_done, int budget)
>   {
>         struct igb_ring *rx_ring = q_vector->rx_ring;
> +       struct igb_adapter *adapter = q_vector->adapter;
>         struct net_device *netdev = rx_ring->netdev;
>         struct pci_dev *pdev = rx_ring->pdev;
>         union e1000_adv_rx_desc *rx_desc , *next_rxd;
> @@ -5230,6 +5236,7 @@ next_desc:
>                 staterr = le32_to_cpu(rx_desc->wb.upper.status_error);
>         }
> 
> +       IGB_WRITE_TRACE_BUFFER(IGB_TRACE_CLEAN_RX, adapter, rx_ring, &i);
>         rx_ring->next_to_clean = i;
>         cleaned_count = igb_desc_unused(rx_ring);
> 
> diff --git a/drivers/net/igb/igb_trace.c b/drivers/net/igb/igb_trace.c
> new file mode 100644
> index 0000000..0e214e8
> --- /dev/null
> +++ b/drivers/net/igb/igb_trace.c
> @@ -0,0 +1,594 @@
> +#include "igb_trace.h"
> +
> +struct igb_trace_info {
> +       void (*write)(void *, void *, void *);
> +       ssize_t (*read)(char *, size_t, void *);
> +       size_t size;
> +};
> +
> +struct trace_data {
> +       unsigned short type;
> +};
> +
> +struct trace_reader {
> +       int idx;
> +       char *left_over;
> +       size_t left_over_len;
> +       struct ring_buffer_iter **iter;
> +       struct mutex *trace_lock;
> +};
> +
> +#define IGB_STR_BUF_LEN 256
> +
> +static struct igb_trace_info igb_trace_info_tbl[];
> +
> +/**
> + * igb_write_tb - Write trace_event to ring_bufffer per cpu
> + * @type: identifier of traced event
> + * @adapter: board private structure
> + * @data1: traced data
> + * @data2: traced data
> + **/
> +
> +void igb_write_tb(unsigned short type, struct igb_adapter *adapter,
> +                       void *data1, void *data2)
> +{
> +       void *event;
> +       void *entry;
> +       unsigned long flags;
> +       struct igb_trace *trace = &adapter->trace;
> +
> +       if (type >= IGB_TRACE_EVENT_NUM)
> +               return;
> +
> +       local_irq_save(flags);
> +       event = ring_buffer_lock_reserve(trace->trace_buffer,
> +                                       igb_trace_info_tbl[type].size);
> +       if (!event)
> +               goto out;
> +       entry = ring_buffer_event_data(event);
> +
> +       igb_trace_info_tbl[type].write(entry, data1, data2);
> +
> +       ring_buffer_unlock_commit(trace->trace_buffer, event);
> +out:
> +       local_irq_restore(flags);
> +}
> +
> +/**
> + * igb_open_tb - Open ring_buffer for trace to read
> + * @inode: The inode pointer that contains igb_adapter pointer
> + * @file: The file pointer to attach the iter of ring_buffer
> + **/
> +
> +static int igb_open_tb(struct inode *inode, struct file *file)
> +{
> +       struct igb_adapter *adapter = inode->i_private;
> +       struct igb_trace *trace = &adapter->trace;
> +       struct ring_buffer *buffer;
> +       struct trace_reader *reader;
> +       int cpu;
> +
> +       mutex_lock(&trace->trace_lock);
> +       if (!trace->trace_size)
> +               return -ENODEV;
> +
> +       reader = kmalloc(sizeof(struct trace_reader), GFP_KERNEL);
> +       if (!reader)
> +               goto err_alloc_reader;
> +
> +       reader->iter = kmalloc(sizeof(struct ring_buffer_iter *) * nr_cpu_ids,
> +                               GFP_KERNEL);
> +       if (!reader->iter)
> +               goto err_alloc_iter;
> +
> +       buffer = trace->trace_buffer;
> +       reader->left_over = kmalloc(IGB_STR_BUF_LEN, GFP_KERNEL);
> +       if (!reader->left_over)
> +               goto err_alloc_left_over;
> +
> +       reader->left_over_len = 0;
> +       reader->idx = 0;
> +       for_each_online_cpu(cpu) {
> +               reader->iter[cpu] = ring_buffer_read_start(buffer, cpu);
> +       }
> +       reader->trace_lock = &trace->trace_lock;
> +       file->private_data = reader;
> +       mutex_unlock(&trace->trace_lock);
> +       return 0;
> +
> +err_alloc_left_over:
> +       kfree(reader->iter);
> +err_alloc_iter:
> +       kfree(reader);
> +err_alloc_reader:
> +       mutex_unlock(&trace->trace_lock);
> +       return -ENOMEM;
> +}
> +
> +/**
> + * igb_release_tb - release iter and some resourse
> + * @inode: The inode pointer that contains igb_adapter pointer
> + * @file: The file pointer to attach the iter of ring_buffer
> + **/
> +
> +static int igb_release_tb(struct inode *inode, struct file *file)
> +{
> +       struct trace_reader *reader = file->private_data;
> +       struct ring_buffer_iter **iter = reader->iter;
> +       int cpu;
> +
> +       for_each_online_cpu(cpu) {
> +               ring_buffer_read_finish(iter[cpu]);
> +       }
> +       kfree(reader->left_over);
> +       kfree(iter);
> +       kfree(reader);
> +       return 0;
> +}
> +
> +static inline unsigned long long ns2usecs(cycle_t nsec)
> +{
> +       nsec += 500;
> +       do_div(nsec, 1000);
> +       return nsec;
> +}
> +
> +static int trace_print_format(char *buf, struct trace_data *entry,
> +                               int cpu , u64 ts)
> +{
> +       int strlen = 0;
> +       unsigned long long t;
> +       unsigned long usecs_rem, secs;
> +
> +       t = ns2usecs(ts);
> +       usecs_rem = do_div(t, USEC_PER_SEC);
> +       secs = (unsigned long)t;
> +       strlen += snprintf(buf, IGB_STR_BUF_LEN, "[%3d] %5lu.%06lu: ",
> +                               cpu, secs, usecs_rem);
> +       if (entry->type < IGB_TRACE_EVENT_NUM)
> +               strlen += igb_trace_info_tbl[entry->type].read(buf + strlen,
> +                                        IGB_STR_BUF_LEN - strlen, entry);
> +       else
> +               strlen += snprintf(buf + strlen, IGB_STR_BUF_LEN - strlen,
> +                                       "type %u is not defined", entry->type);
> +       strlen += snprintf(buf + strlen, IGB_STR_BUF_LEN - strlen, "\n");
> +       return strlen;
> +}
> +
> +static loff_t _igb_lseek_tb(struct trace_reader *reader, loff_t pos)
> +{
> +       struct ring_buffer_iter **iter = reader->iter;
> +       struct ring_buffer_event *event;
> +       struct trace_data *entry;
> +       char str[IGB_STR_BUF_LEN];
> +       int cpu, next_cpu;
> +       u64 ts, next_ts;
> +       int exceed;
> +       int strlen;
> +
> +       reader->idx = 0;
> +       for_each_online_cpu(cpu) {
> +               ring_buffer_iter_reset(iter[cpu]);
> +       }
> +       while (1) {
> +               next_ts = 0;
> +               next_cpu = -1;
> +               for_each_online_cpu(cpu) {
> +                       if (!iter[cpu])
> +                               continue;
> +                       event = ring_buffer_iter_peek(iter[cpu], &ts);
> +                       if (!event)
> +                               continue;
> +                       if (!next_ts || ts < next_ts) {
> +                               next_ts = ts;
> +                               next_cpu = cpu;
> +                       }
> +               }
> +               if (next_cpu < 0)
> +                       return -EINVAL;
> +               event = ring_buffer_read(iter[next_cpu], &ts);
> +               entry = ring_buffer_event_data(event);
> +               strlen = trace_print_format(str, entry, cpu, ts);
> +
> +               if (reader->idx + strlen >= pos)
> +                       break;
> +               reader->idx += strlen;
> +       }
> +       exceed = reader->idx + strlen - pos;
> +       if (exceed) {
> +               int from = strlen - exceed;
> +               memcpy(reader->left_over, str + from, exceed);
> +       }
> +       reader->left_over_len = exceed;
> +       reader->idx = pos;
> +       return pos;
> +}
> +
> +/**
> + * igb_lseek_tb - seek a ring_buffer
> + * @file: The file pointer to attach the iter of ring_buffer
> + * @offset: The offset to seek
> + * @origin: absolute(0) or relative(1)
> + **/
> +
> +static loff_t igb_lseek_tb(struct file *file, loff_t offset, int origin)
> +{
> +       struct trace_reader *reader = file->private_data;
> +       loff_t ret = -EINVAL;
> +
> +       mutex_lock(reader->trace_lock);
> +       switch (origin) {
> +       case 1:
> +               offset += file->f_pos;
> +       case 0:
> +               if (offset < 0)
> +                       break;
> +               ret = _igb_lseek_tb(reader, offset);
> +       }
> +       if (ret > 0)
> +               file->f_pos = ret;
> +       mutex_unlock(reader->trace_lock);
> +       return ret;
> +}
> +
> +/**
> + * igb_read_tb - read a ring_buffer and transform print format
> + * @file: The file pointer to attach the iter of ring_buffer
> + * @buf: The buffer to copy to
> + * @nbytes: The maximum number of bytes to read
> + * @ppos: The position to read from
> + **/
> +
> +static ssize_t igb_read_tb(struct file *file, char __user *buf,
> +                               size_t nbytes, loff_t *ppos)
> +{
> +       struct trace_reader *reader = file->private_data;
> +       struct ring_buffer_iter **iter = reader->iter;
> +       struct trace_data *entry;
> +       struct ring_buffer_event *event;
> +       loff_t pos = *ppos;
> +       u64 ts, next_ts = 0;
> +       int cpu, next_cpu = -1;
> +       char str[IGB_STR_BUF_LEN];
> +       unsigned int strlen = 0;
> +       size_t copy;
> +       int ret;
> +
> +       mutex_lock(reader->trace_lock);
> +       if (pos != reader->idx)
> +               _igb_lseek_tb(reader, pos);
> +       if (reader->left_over_len) {
> +               copy = min(reader->left_over_len, nbytes);
> +               ret = copy_to_user(buf, reader->left_over,
> +                               reader->left_over_len);
> +               copy -= ret;
> +               reader->left_over_len = ret;
> +               reader->idx += copy;
> +               *ppos += copy;
> +               mutex_unlock(reader->trace_lock);
> +               return reader->left_over_len - ret;
> +       }
> +       for_each_online_cpu(cpu) {
> +               if (!iter[cpu])
> +                       continue;
> +               event = ring_buffer_iter_peek(iter[cpu], &ts);
> +               if (!event)
> +                       continue;
> +               if (!next_ts || ts < next_ts) {
> +                       next_ts = ts;
> +                       next_cpu = cpu;
> +               }
> +       }
> +       if (next_cpu < 0) {
> +               mutex_unlock(reader->trace_lock);
> +               return 0;
> +       }
> +       event = ring_buffer_read(iter[next_cpu], &ts);
> +       entry = ring_buffer_event_data(event);
> +
> +       strlen = trace_print_format(str, entry, next_cpu, ts);
> +       copy = min(strlen, nbytes);
> +       ret = copy_to_user(buf, str, strlen);
> +       copy -= ret;
> +       reader->left_over_len = ret;
> +       if (ret) {
> +               memcpy(reader->left_over, str + copy, ret);
> +               reader->left_over_len = ret;
> +       }
> +       *ppos = pos + copy;
> +       reader->idx = *ppos;
> +       mutex_unlock(reader->trace_lock);
> +       return copy;
> +}
> +
> +static const struct file_operations igb_trace_file_ops = {
> +       .owner =        THIS_MODULE,
> +       .open =         igb_open_tb,
> +       .llseek =       igb_lseek_tb,
> +       .read =         igb_read_tb,
> +       .release =      igb_release_tb,
> +};
> +
> +/**
> + * igb_open_trace_size - connect the pinter
> + * @inode: The inode pointer that contains igb_adapter pointer
> + * @file: The file pointer to attach the igb_adapter pointer
> + **/
> +
> +static int igb_open_trace_size(struct inode *inode, struct file *file)
> +{
> +       file->private_data = inode->i_private;
> +       return 0;
> +}
> +
> +/**
> + * igb_read_tb - print the size of ring_buffer byte unit to buf
> + *               if trace is disable, print 0
> + * @file: The file pointer that contains an igb_adapter pointer
> + * @buf: The buffer to copy to
> + * @nbytes: The maximum number of bytes to read
> + * @ppos: The position to read from
> + **/
> +
> +static ssize_t igb_read_trace_size(struct file *file, char __user *ubuf,
> +                               size_t nbytes, loff_t *ppos)
> +{
> +       struct igb_adapter *adapter = file->private_data;
> +       struct igb_trace *trace = &adapter->trace;
> +       unsigned long size = trace->trace_size;
> +       char buf[16];
> +       int r;
> +
> +       r = sprintf(buf, "%lu\n", size);
> +
> +       return simple_read_from_buffer(ubuf, nbytes, ppos, buf, r);
> +}
> +
> +/**
> + * igb_trace_buffer_enable - enable trace and set ring_buffer size
> + * @adapter: board private structure
> + * @size: size of ring_buffer per cpu
> + **/
> +
> +static int igb_trace_buffer_enable(struct igb_adapter *adapter, ssize_t size)
> +{
> +       struct igb_trace *trace = &adapter->trace;
> +       struct pci_dev *pdev = adapter->pdev;
> +
> +       trace->trace_buffer =  ring_buffer_alloc(size, RB_FL_OVERWRITE);
> +       if (!trace->trace_buffer) {
> +               dev_err(&pdev->dev, "Cannot alloc trace_buffer\n");
> +               return -EINVAL;
> +       }
> +       trace->trace_size = size;
> +       return 0;
> +}
> +
> +/**
> + * igb_trace_buffer_disable - disable trace
> + * @adapter: board private structure
> + **/
> +
> +static void igb_trace_buffer_disable(struct igb_adapter *adapter)
> +{
> +       struct igb_trace *trace = &adapter->trace;
> +
> +       ring_buffer_free(trace->trace_buffer);
> +       trace->trace_size = 0;
> +}
> +
> +/**
> + * igb_write_trace_size - write a ring_buffer size per cpu
> + *                        if 0, disable trace
> + * @file: Ther file pointer thar contains an igb_adapter pointer
> + * @buf: The buffer to read and write for trace_size
> + * @nbytes: size of buffe to read
> + * @ppos: the position to read
> + **/
> +
> +static ssize_t igb_write_trace_size(struct file *file, const char __user *ubuf,
> +                                       size_t nbytes, loff_t *ppos)
> +{
> +       struct igb_adapter *adapter = file->private_data;
> +       struct igb_trace *trace = &adapter->trace;
> +       unsigned long cur = trace->trace_size;
> +       int ret;
> +       unsigned long new;
> +       char buf[64];
> +
> +       if (nbytes >= sizeof(buf))
> +               return -EINVAL;
> +
> +       if (copy_from_user(&buf, ubuf, nbytes))
> +               return -EFAULT;
> +       buf[nbytes] = 0;
> +       ret = strict_strtoul(buf, 10, &new);
> +       if (ret < 0)
> +               return ret;
> +
> +       mutex_lock(&trace->trace_lock);
> +       if (!cur && new) {
> +               igb_trace_buffer_enable(adapter, new);
> +       } else if (cur && !new) {
> +               igb_trace_buffer_disable(adapter);
> +       } else if (cur != new) {
> +               igb_trace_buffer_disable(adapter);
> +               igb_trace_buffer_enable(adapter, new);
> +       }
> +       mutex_unlock(&trace->trace_lock);
> +       return nbytes;
> +}
> +
> +static const struct file_operations igb_trace_size_ops = {
> +       .owner =        THIS_MODULE,
> +       .open =         igb_open_trace_size,
> +       .read =         igb_read_trace_size,
> +       .write =        igb_write_trace_size,
> +};
> +
> +static struct dentry *igb_trace_root;
> +
> +void igb_trace_init()
> +{
> +       igb_trace_root = debugfs_create_dir("igb", NULL);
> +       if (!igb_trace_root) {
> +               printk(KERN_ERR "Cannot create debugfs\n");
> +               return;
> +       }
> +}
> +
> +void igb_trace_exit(void)
> +{
> +       if (igb_trace_root)
> +               debugfs_remove_recursive(igb_trace_root);
> +}
> +
> +void igb_create_debugfs_file(struct igb_adapter *adapter)
> +{
> +       struct net_device *netdev = adapter->netdev;
> +       struct igb_trace *trace = &adapter->trace;
> +       struct pci_dev *pdev = adapter->pdev;
> +
> +       if (!igb_trace_root)
> +               return;
> +       trace->if_dir =
> +               debugfs_create_dir(netdev->name, igb_trace_root);
> +       if (!trace->if_dir) {
> +               dev_err(&pdev->dev, "Cannot create if_dir %s\n",
> +                       netdev->name);
> +               goto err_alloc_dir;
> +       }
> +       trace->trace_file =
> +               debugfs_create_file("trace", S_IFREG|S_IRUGO|S_IWUSR,
> +                       trace->if_dir,
> +                       adapter, &igb_trace_file_ops);
> +       if (!trace->trace_file) {
> +               dev_err(&pdev->dev, "Cannot create debugfs for trace %s\n",
> +                       netdev->name);
> +               goto err_alloc_trace;
> +       }
> +       trace->trace_size_file =
> +               debugfs_create_file("trace_size", S_IFREG|S_IRUGO|S_IWUSR,
> +                       trace->if_dir,
> +                       adapter, &igb_trace_size_ops);
> +       if (!trace->trace_file) {
> +               dev_err(&pdev->dev, "Cannot create debugfs for trace_size %s\n",
> +                       netdev->name);
> +               goto err_alloc_trace_size;
> +       }
> +       mutex_init(&trace->trace_lock);
> +       return;
> +
> +err_alloc_trace_size:
> +       debugfs_remove(trace->trace_file);
> +err_alloc_trace:
> +       debugfs_remove(trace->if_dir);
> +err_alloc_dir:
> +       return;
> +}
> +
> +void igb_remove_debugfs_file(struct igb_adapter *adapter)
> +{
> +       struct igb_trace *trace = &adapter->trace;
> +
> +       if (trace->trace_size_file)
> +               debugfs_remove(trace->trace_size_file);
> +       if (trace->trace_file)
> +               debugfs_remove(trace->trace_file);
> +       if (trace->if_dir)
> +               debugfs_remove(trace->if_dir);
> +}
> +
> +/* function and struct for tracing xmit */
> +struct trace_data_xmit {
> +       unsigned short type;
> +       u8 queue_index;
> +       u16 ntu;
> +       unsigned int first;
> +};
> +
> +static void igb_write_tb_xmit(void *entry, void *data1, void *data2)
> +{
> +       struct trace_data_xmit *td = entry;
> +       struct igb_ring *tx_ring = (struct igb_ring *)data1;
> +       unsigned int first = *(unsigned int *)data2;
> +
> +       td->type = IGB_TRACE_XMIT;
> +       td->queue_index = tx_ring->queue_index;
> +       td->ntu = tx_ring->next_to_use;
> +       td->first = first;
> +}
> +
> +static ssize_t igb_read_tb_xmit(char *str, size_t size, void *entry)
> +{
> +       struct trace_data_xmit *t_data = entry;
> +
> +       return snprintf(str, size, "xmit qidx=%u ntu=%u->%u",
> +                       t_data->queue_index, t_data->first, t_data->ntu);
> +}
> +
> +/* function and struct for tracing clean_tx */
> +struct trace_data_clean_tx {
> +       unsigned short type;
> +       u8 queue_index;
> +       u16 ntc;
> +       unsigned int i;
> +};
> +
> +static void igb_write_tb_clean_tx(void *entry, void *data1, void *data2)
> +{
> +       struct trace_data_clean_tx *td = entry;
> +       struct igb_ring *tx_ring = (struct igb_ring *)data1;
> +       unsigned int i = *(unsigned int *)data2;
> +
> +       td->type = IGB_TRACE_CLEAN_TX;
> +       td->queue_index = tx_ring->queue_index;
> +       td->ntc = tx_ring->next_to_clean;
> +       td->i = i;
> +}
> +
> +static ssize_t igb_read_tb_clean_tx(char *str, size_t size, void *entry)
> +{
> +       struct trace_data_clean_tx *t_data = entry;
> +
> +       return snprintf(str, size, "clean_tx qidx=%u ntc=%u->%u",
> +                       t_data->queue_index, t_data->ntc, t_data->i);
> +}
> +
> +/* function and struct for tracing clean_rx */
> +struct trace_data_clean_rx {
> +       unsigned short type;
> +       u8 queue_index;
> +       u16 ntc;
> +       unsigned int i;
> +};
> +
> +static void igb_write_tb_clean_rx(void *entry, void *data1, void *data2)
> +{
> +       struct trace_data_clean_rx *td = entry;
> +       struct igb_ring *rx_ring = (struct igb_ring *)data1;
> +       unsigned int i = *(unsigned int *)data2;
> +
> +       td->type = IGB_TRACE_CLEAN_RX;
> +       td->queue_index = rx_ring->queue_index;
> +       td->ntc = rx_ring->next_to_clean;
> +       td->i = i;
> +}
> +
> +static ssize_t igb_read_tb_clean_rx(char *str, size_t size, void *entry)
> +{
> +       struct trace_data_clean_rx *t_data = entry;
> +
> +       return snprintf(str, size, "clean_rx qidx=%u ntc=%u->%u",
> +                       t_data->queue_index, t_data->ntc, t_data->i);
> +}
> +
> +static struct igb_trace_info igb_trace_info_tbl[] = {
> +       {igb_write_tb_xmit, igb_read_tb_xmit,
> +               sizeof(struct trace_data_xmit)},
> +       {igb_write_tb_clean_tx, igb_read_tb_clean_tx,
> +               sizeof(struct trace_data_clean_tx)},
> +       {igb_write_tb_clean_rx, igb_read_tb_clean_rx,
> +               sizeof(struct trace_data_clean_rx)},
> +};
> diff --git a/drivers/net/igb/igb_trace.h b/drivers/net/igb/igb_trace.h
> new file mode 100644
> index 0000000..4e6a85b
> --- /dev/null
> +++ b/drivers/net/igb/igb_trace.h
> @@ -0,0 +1,29 @@
> +#ifndef _IGB_TRACE_H_
> +#define _IGB_TRACE_H_
> +
> +#include <linux/module.h>
> +#include <linux/ring_buffer.h>
> +#include <linux/netdevice.h>
> +#include <linux/pci.h>
> +#include <linux/debugfs.h>
> +#include "igb.h"
> +
> +#define IGB_TRACE_XMIT                 0x00
> +#define IGB_TRACE_CLEAN_TX     0x01
> +#define IGB_TRACE_CLEAN_RX     0x02
> +#define IGB_TRACE_EVENT_NUM    0x03
> +
> +extern void igb_trace_init(void);
> +extern void igb_trace_exit(void);
> +extern void igb_create_debugfs_file(struct igb_adapter *adapter);
> +extern void igb_remove_debugfs_file(struct igb_adapter *adapter);
> +extern void igb_write_tb(unsigned short type, struct igb_adapter *adapter,
> +                       void *data1, void *data2);
> +
> +#define IGB_WRITE_TRACE_BUFFER(type, adapter, data1, data2)            \
> +       do {                                                            \
> +               if (adapter->trace.trace_size)                          \
> +                       igb_write_tb(type, adapter, data1, data2);      \
> +       } while (0)
> +
> +#endif /* _IGB_TRACE_H_ */
> 


^ permalink raw reply

* Re: [PATCH 2/5] C/R: Basic support for network namespaces and devices (v4)
From: Serge E. Hallyn @ 2010-02-23 18:49 UTC (permalink / raw)
  To: Dan Smith; +Cc: containers, netdev
In-Reply-To: <87fx4r7tgv.fsf@caffeine.danplanet.com>

Quoting Dan Smith (danms@us.ibm.com):
> SH> But there is no guarantee that the checkpointer is in the netns
> SH> which we would call the 'top level' netns.  Which means that, at
> SH> restart, whether or not the devices which are in what we call the
> SH> top level netns are in fact inherited or not, will depend on
> SH> conditions of the checkpointer.  Do we care?  (I thought we did,
> SH> but maybe we don't... it's unlikely to happen anyway)
> 
> Well, when we discussed this on IRC with Oren, I think we came to the
> conclusion that since network namespaces aren't hierarchical, that we
> would restore things from the "viewpoint" of the process that
> checkpointed them.  It gives us a sane way to ensure that the peer
> devices residing in the init netns can be put back there, even though we
> don't checkpoint everything in the init netns (like eth0).
> 
> If you checkpoint a veth from within the container and you have a peer
> device that is outside the container (but not in a netns that is
> checkpointed as part of a task), it's going to fail and tell you that
> one of your peers leaked to the outside.  I think that's sane and
> preferred behavior, no?

Well I don't think it is, but it's a fine starting point, so let's
worry about it later.

thanks,
-serge

> If you're using macvlan and you checkpoint
> from within the container, I think you should be okay, as long as
> there is a appropriately named device to base the restored devices on
> in whatever netns your restore process is in.
> 
> -- 
> Dan Smith
> IBM Linux Technology Center
> email: danms@us.ibm.com

^ permalink raw reply

* Re: [RFC] IPv6: don't forward unspecified frames
From: Stephen Hemminger @ 2010-02-23 18:50 UTC (permalink / raw)
  To: Shan Wei; +Cc: David Miller, netdev
In-Reply-To: <4B836385.8090509@cn.fujitsu.com>

On Tue, 23 Feb 2010 13:11:33 +0800
Shan Wei <shanwei@cn.fujitsu.com> wrote:

> Stephen Hemminger wrote, at 02/23/2010 09:31 AM:
> > This showed up during UNH IPv6 conformance tests. It appears kernel
> > incorrectly forwards packets with unspecified source address.
> 
> Which case? Is it about spec.p2#18 of IPv6 Ready Logo Phase 2?
> I don't see the phenomenon from spec.p2#18 case.
> 
> > This looks like the place to fix this, but still not sure and have
> > no easy way to test it since ping6 won't send packet with unspecified
> > source address.
> > 
> > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> 
> Kernel is coincident with the spec, see following commit.
> 

Never mind.

I could not reproduce the problem, with a program that sends
ICMPV6 echo through AF_PACKET.

UNH reran the test, and the kernel is fine.
Looks like a tester problem.

^ permalink raw reply

* [net-next-2.6 PATCH] ixgbe: convert to use netdev_for_each_mc_addr
From: Jiri Pirko @ 2010-02-23 19:05 UTC (permalink / raw)
  To: netdev; +Cc: davem


Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
 drivers/net/ixgbe/ixgbe_common.c |   16 +++++++---------
 drivers/net/ixgbe/ixgbe_common.h |    5 ++---
 drivers/net/ixgbe/ixgbe_main.c   |   24 ++----------------------
 drivers/net/ixgbe/ixgbe_type.h   |    3 +--
 4 files changed, 12 insertions(+), 36 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_common.c b/drivers/net/ixgbe/ixgbe_common.c
index eb49020..4d1c3a4 100644
--- a/drivers/net/ixgbe/ixgbe_common.c
+++ b/drivers/net/ixgbe/ixgbe_common.c
@@ -1484,26 +1484,24 @@ static void ixgbe_set_mta(struct ixgbe_hw *hw, u8 *mc_addr)
 /**
  *  ixgbe_update_mc_addr_list_generic - Updates MAC list of multicast addresses
  *  @hw: pointer to hardware structure
- *  @mc_addr_list: the list of new multicast addresses
- *  @mc_addr_count: number of addresses
- *  @next: iterator function to walk the multicast address list
+ *  @netdev: pointer to net device structure
  *
  *  The given list replaces any existing list. Clears the MC addrs from receive
  *  address registers and the multicast table. Uses unused receive address
  *  registers for the first multicast addresses, and hashes the rest into the
  *  multicast table.
  **/
-s32 ixgbe_update_mc_addr_list_generic(struct ixgbe_hw *hw, u8 *mc_addr_list,
-                                      u32 mc_addr_count, ixgbe_mc_addr_itr next)
+s32 ixgbe_update_mc_addr_list_generic(struct ixgbe_hw *hw,
+				      struct net_device *netdev)
 {
+	struct dev_addr_list *dmi;
 	u32 i;
-	u32 vmdq;
 
 	/*
 	 * Set the new number of MC addresses that we are being requested to
 	 * use.
 	 */
-	hw->addr_ctrl.num_mc_addrs = mc_addr_count;
+	hw->addr_ctrl.num_mc_addrs = netdev_mc_count(netdev);
 	hw->addr_ctrl.mta_in_use = 0;
 
 	/* Clear the MTA */
@@ -1512,9 +1510,9 @@ s32 ixgbe_update_mc_addr_list_generic(struct ixgbe_hw *hw, u8 *mc_addr_list,
 		IXGBE_WRITE_REG(hw, IXGBE_MTA(i), 0);
 
 	/* Add the new addresses */
-	for (i = 0; i < mc_addr_count; i++) {
+	netdev_for_each_mc_addr(dmi, netdev) {
 		hw_dbg(hw, " Adding the multicast addresses:\n");
-		ixgbe_set_mta(hw, next(hw, &mc_addr_list, &vmdq));
+		ixgbe_set_mta(hw, dmi->dmi_addr);
 	}
 
 	/* Enable mta */
diff --git a/drivers/net/ixgbe/ixgbe_common.h b/drivers/net/ixgbe/ixgbe_common.h
index 13606d4..264eef5 100644
--- a/drivers/net/ixgbe/ixgbe_common.h
+++ b/drivers/net/ixgbe/ixgbe_common.h
@@ -56,9 +56,8 @@ s32 ixgbe_set_rar_generic(struct ixgbe_hw *hw, u32 index, u8 *addr, u32 vmdq,
                           u32 enable_addr);
 s32 ixgbe_clear_rar_generic(struct ixgbe_hw *hw, u32 index);
 s32 ixgbe_init_rx_addrs_generic(struct ixgbe_hw *hw);
-s32 ixgbe_update_mc_addr_list_generic(struct ixgbe_hw *hw, u8 *mc_addr_list,
-                                      u32 mc_addr_count,
-                                      ixgbe_mc_addr_itr func);
+s32 ixgbe_update_mc_addr_list_generic(struct ixgbe_hw *hw,
+				      struct net_device *netdev);
 s32 ixgbe_update_uc_addr_list_generic(struct ixgbe_hw *hw,
 				      struct net_device *netdev);
 s32 ixgbe_enable_mc_generic(struct ixgbe_hw *hw);
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index 3308790..5ef4f6a 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -2513,21 +2513,6 @@ static void ixgbe_restore_vlan(struct ixgbe_adapter *adapter)
 	}
 }
 
-static u8 *ixgbe_addr_list_itr(struct ixgbe_hw *hw, u8 **mc_addr_ptr, u32 *vmdq)
-{
-	struct dev_mc_list *mc_ptr;
-	u8 *addr = *mc_addr_ptr;
-	*vmdq = 0;
-
-	mc_ptr = container_of(addr, struct dev_mc_list, dmi_addr[0]);
-	if (mc_ptr->next)
-		*mc_addr_ptr = mc_ptr->next->dmi_addr;
-	else
-		*mc_addr_ptr = NULL;
-
-	return addr;
-}
-
 /**
  * ixgbe_set_rx_mode - Unicast, Multicast and Promiscuous mode set
  * @netdev: network interface device structure
@@ -2542,8 +2527,6 @@ void ixgbe_set_rx_mode(struct net_device *netdev)
 	struct ixgbe_adapter *adapter = netdev_priv(netdev);
 	struct ixgbe_hw *hw = &adapter->hw;
 	u32 fctrl, vlnctrl;
-	u8 *addr_list = NULL;
-	int addr_count = 0;
 
 	/* Check for Promiscuous and All Multicast modes */
 
@@ -2572,11 +2555,8 @@ void ixgbe_set_rx_mode(struct net_device *netdev)
 	hw->mac.ops.update_uc_addr_list(hw, netdev);
 
 	/* reprogram multicast list */
-	addr_count = netdev_mc_count(netdev);
-	if (addr_count)
-		addr_list = netdev->mc_list->dmi_addr;
-	hw->mac.ops.update_mc_addr_list(hw, addr_list, addr_count,
-	                                ixgbe_addr_list_itr);
+	hw->mac.ops.update_mc_addr_list(hw, netdev);
+
 	if (adapter->num_vfs)
 		ixgbe_restore_vf_multicasts(adapter);
 }
diff --git a/drivers/net/ixgbe/ixgbe_type.h b/drivers/net/ixgbe/ixgbe_type.h
index 2be9074..c67b98e 100644
--- a/drivers/net/ixgbe/ixgbe_type.h
+++ b/drivers/net/ixgbe/ixgbe_type.h
@@ -2415,8 +2415,7 @@ struct ixgbe_mac_operations {
 	s32 (*clear_vmdq)(struct ixgbe_hw *, u32, u32);
 	s32 (*init_rx_addrs)(struct ixgbe_hw *);
 	s32 (*update_uc_addr_list)(struct ixgbe_hw *, struct net_device *);
-	s32 (*update_mc_addr_list)(struct ixgbe_hw *, u8 *, u32,
-	                           ixgbe_mc_addr_itr);
+	s32 (*update_mc_addr_list)(struct ixgbe_hw *, struct net_device *);
 	s32 (*enable_mc)(struct ixgbe_hw *);
 	s32 (*disable_mc)(struct ixgbe_hw *);
 	s32 (*clear_vfta)(struct ixgbe_hw *);
-- 
1.6.6


^ permalink raw reply related

* [net-next-2.6 PATCH] ixgbevf: convert to use netdev_for_each_mc_addr
From: Jiri Pirko @ 2010-02-23 19:06 UTC (permalink / raw)
  To: netdev; +Cc: davem


Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
 drivers/net/ixgbevf/ixgbevf_main.c |   24 +-----------------------
 drivers/net/ixgbevf/vf.c           |   24 ++++++++++++------------
 drivers/net/ixgbevf/vf.h           |    4 ++--
 3 files changed, 15 insertions(+), 37 deletions(-)

diff --git a/drivers/net/ixgbevf/ixgbevf_main.c b/drivers/net/ixgbevf/ixgbevf_main.c
index 235b5fd..cfa30aa 100644
--- a/drivers/net/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ixgbevf/ixgbevf_main.c
@@ -1495,22 +1495,6 @@ static void ixgbevf_restore_vlan(struct ixgbevf_adapter *adapter)
 	}
 }
 
-static u8 *ixgbevf_addr_list_itr(struct ixgbe_hw *hw, u8 **mc_addr_ptr,
-				 u32 *vmdq)
-{
-	struct dev_mc_list *mc_ptr;
-	u8 *addr = *mc_addr_ptr;
-	*vmdq = 0;
-
-	mc_ptr = container_of(addr, struct dev_mc_list, dmi_addr[0]);
-	if (mc_ptr->next)
-		*mc_addr_ptr = mc_ptr->next->dmi_addr;
-	else
-		*mc_addr_ptr = NULL;
-
-	return addr;
-}
-
 /**
  * ixgbevf_set_rx_mode - Multicast set
  * @netdev: network interface device structure
@@ -1523,16 +1507,10 @@ static void ixgbevf_set_rx_mode(struct net_device *netdev)
 {
 	struct ixgbevf_adapter *adapter = netdev_priv(netdev);
 	struct ixgbe_hw *hw = &adapter->hw;
-	u8 *addr_list = NULL;
-	int addr_count = 0;
 
 	/* reprogram multicast list */
-	addr_count = netdev_mc_count(netdev);
-	if (addr_count)
-		addr_list = netdev->mc_list->dmi_addr;
 	if (hw->mac.ops.update_mc_addr_list)
-		hw->mac.ops.update_mc_addr_list(hw, addr_list, addr_count,
-						ixgbevf_addr_list_itr);
+		hw->mac.ops.update_mc_addr_list(hw, netdev);
 }
 
 static void ixgbevf_napi_enable_all(struct ixgbevf_adapter *adapter)
diff --git a/drivers/net/ixgbevf/vf.c b/drivers/net/ixgbevf/vf.c
index 4b5dec0..f457c52 100644
--- a/drivers/net/ixgbevf/vf.c
+++ b/drivers/net/ixgbevf/vf.c
@@ -252,22 +252,18 @@ static s32 ixgbevf_set_rar_vf(struct ixgbe_hw *hw, u32 index, u8 *addr,
 /**
  *  ixgbevf_update_mc_addr_list_vf - Update Multicast addresses
  *  @hw: pointer to the HW structure
- *  @mc_addr_list: array of multicast addresses to program
- *  @mc_addr_count: number of multicast addresses to program
- *  @next: caller supplied function to return next address in list
+ *  @netdev: pointer to net device structure
  *
  *  Updates the Multicast Table Array.
  **/
-static s32 ixgbevf_update_mc_addr_list_vf(struct ixgbe_hw *hw, u8 *mc_addr_list,
-					u32 mc_addr_count,
-					ixgbe_mc_addr_itr next)
+static s32 ixgbevf_update_mc_addr_list_vf(struct ixgbe_hw *hw,
+					  struct net_device *netdev)
 {
+	struct dev_addr_list *dmi;
 	struct ixgbe_mbx_info *mbx = &hw->mbx;
 	u32 msgbuf[IXGBE_VFMAILBOX_SIZE];
 	u16 *vector_list = (u16 *)&msgbuf[1];
-	u32 vector;
 	u32 cnt, i;
-	u32 vmdq;
 
 	/* Each entry in the list uses 1 16 bit word.  We have 30
 	 * 16 bit words available in our HW msg buffer (minus 1 for the
@@ -278,13 +274,17 @@ static s32 ixgbevf_update_mc_addr_list_vf(struct ixgbe_hw *hw, u8 *mc_addr_list,
 	 * addresses except for in large enterprise network environments.
 	 */
 
-	cnt = (mc_addr_count > 30) ? 30 : mc_addr_count;
+	cnt = netdev_mc_count(netdev);
+	if (cnt > 30)
+		cnt = 30;
 	msgbuf[0] = IXGBE_VF_SET_MULTICAST;
 	msgbuf[0] |= cnt << IXGBE_VT_MSGINFO_SHIFT;
 
-	for (i = 0; i < cnt; i++) {
-		vector = ixgbevf_mta_vector(hw, next(hw, &mc_addr_list, &vmdq));
-		vector_list[i] = vector;
+	i = 0;
+	netdev_for_each_mc_addr(dmi, netdev) {
+		if (i == cnt)
+			break;
+		vector_list[i++] = ixgbevf_mta_vector(hw, dmi->dmi_addr);
 	}
 
 	mbx->ops.write_posted(hw, msgbuf, IXGBE_VFMAILBOX_SIZE);
diff --git a/drivers/net/ixgbevf/vf.h b/drivers/net/ixgbevf/vf.h
index 799600e..07a9aed 100644
--- a/drivers/net/ixgbevf/vf.h
+++ b/drivers/net/ixgbevf/vf.h
@@ -32,6 +32,7 @@
 #include <linux/delay.h>
 #include <linux/interrupt.h>
 #include <linux/if_ether.h>
+#include <linux/netdevice.h>
 
 #include "defines.h"
 #include "regs.h"
@@ -62,8 +63,7 @@ struct ixgbe_mac_operations {
 	/* RAR, Multicast, VLAN */
 	s32 (*set_rar)(struct ixgbe_hw *, u32, u8 *, u32);
 	s32 (*init_rx_addrs)(struct ixgbe_hw *);
-	s32 (*update_mc_addr_list)(struct ixgbe_hw *, u8 *, u32,
-				   ixgbe_mc_addr_itr);
+	s32 (*update_mc_addr_list)(struct ixgbe_hw *, struct net_device *);
 	s32 (*enable_mc)(struct ixgbe_hw *);
 	s32 (*disable_mc)(struct ixgbe_hw *);
 	s32 (*clear_vfta)(struct ixgbe_hw *);
-- 
1.6.6


^ permalink raw reply related

* Re: [PATCH net-next 0/7] cxgb4 patches V2
From: Dimitris Michailidis @ 2010-02-23 19:07 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20100221.223049.183064491.davem@davemloft.net>

David Miller wrote:
> From: Dimitris Michailidis <dm@chelsio.com>
> Date: Thu, 18 Feb 2010 13:24:09 -0800
> 
>> This is V2 of the cxgb4 patches.
>>
>> Changes since V1:
>> - Whitespace fixed as well as some additional checkpatch issues.
>> - I removed the CH_* logging wrappers.  I used the new netdev_* in a handful of
>>   places but mostly stayed with the dev_* functions.
>> - I added some comments to describe the register macro scheme.  As a macro
>>   change would involve a rather large number of LOCs I'd really like to avoid
>>   changing them.
> 
> I also told you to not use the non-standard S_*, V_*, etc. register
> naming convention.
> 
> If you repost your driver without addressing all of the review
> feedback you've received thus far, you are wasting people's time
> because they will take time out to review your patches again
> only to find that you've haven't fixed everything you've already
> been made aware of.
> 

Can you please clarify what exactly you want me to change about the macros? 
  I looked at a few other drivers in the tree and I saw shift macros with 
_SHIFT or _SH suffixes, masks tend to have _MASK or _MSK suffixes, etc.  Are 
you asking me basically to change our S_, V_, etc prefixes to _SHIFT, etc 
suffixes or are we talking about some other kind of change?  Please bear 
with me, I am trying to understand what you're asking for exactly.

^ permalink raw reply

* [net-next-2.6 PATCH] net: convert multiple drivers to use netdev_for_each_mc_addr, part5
From: Jiri Pirko @ 2010-02-23 19:07 UTC (permalink / raw)
  To: netdev; +Cc: davem


removed some needless checks and also corrected bug in lp486e (dmi was passed
instead of dmi->dmi_addr)

Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
 drivers/net/jme.c                  |    6 +-----
 drivers/net/korina.c               |    6 ++----
 drivers/net/ks8851.c               |    5 ++---
 drivers/net/ks8851_mll.c           |    3 ++-
 drivers/net/ksz884x.c              |    4 ++--
 drivers/net/lib82596.c             |    7 ++++---
 drivers/net/lib8390.c              |   15 ++++-----------
 drivers/net/ll_temac_main.c        |    7 ++++---
 drivers/net/lp486e.c               |    4 ++--
 drivers/net/mac89x0.c              |    4 +---
 drivers/net/macb.c                 |    7 ++-----
 drivers/net/mace.c                 |   11 +++++------
 drivers/net/macmace.c              |   12 ++++++------
 drivers/net/mv643xx_eth.c          |    2 +-
 drivers/net/myri10ge/myri10ge.c    |    2 +-
 drivers/net/natsemi.c              |    4 ++--
 drivers/net/netxen/netxen_nic_hw.c |   19 +++++++------------
 drivers/net/ni5010.c               |    3 ++-
 drivers/net/ni52.c                 |    8 ++++----
 drivers/net/niu.c                  |    2 +-
 drivers/net/octeon/octeon_mgmt.c   |    7 +------
 drivers/net/pci-skeleton.c         |    6 +++---
 drivers/net/pcnet32.c              |    5 ++---
 drivers/net/ps3_gelic_net.c        |    2 +-
 drivers/net/qlcnic/qlcnic_hw.c     |    3 +--
 drivers/net/qlge/qlge_main.c       |    6 ++++--
 drivers/net/r6040.c                |   30 +++++++++++++++---------------
 drivers/net/r8169.c                |    4 +---
 28 files changed, 83 insertions(+), 111 deletions(-)

diff --git a/drivers/net/jme.c b/drivers/net/jme.c
index 558b6a0..0f31497 100644
--- a/drivers/net/jme.c
+++ b/drivers/net/jme.c
@@ -1997,7 +1997,6 @@ jme_set_multi(struct net_device *netdev)
 {
 	struct jme_adapter *jme = netdev_priv(netdev);
 	u32 mc_hash[2] = {};
-	int i;
 
 	spin_lock_bh(&jme->rxmcs_lock);
 
@@ -2012,10 +2011,7 @@ jme_set_multi(struct net_device *netdev)
 		int bit_nr;
 
 		jme->reg_rxmcs |= RXMCS_MULFRAME | RXMCS_MULFILTERED;
-		for (i = 0, mclist = netdev->mc_list;
-			mclist && i < netdev_mc_count(netdev);
-			++i, mclist = mclist->next) {
-
+		netdev_for_each_mc_addr(mclist, netdev) {
 			bit_nr = ether_crc(ETH_ALEN, mclist->dmi_addr) & 0x3F;
 			mc_hash[bit_nr >> 5] |= 1 << (bit_nr & 0x1F);
 		}
diff --git a/drivers/net/korina.c b/drivers/net/korina.c
index af0c764..300c224 100644
--- a/drivers/net/korina.c
+++ b/drivers/net/korina.c
@@ -482,7 +482,7 @@ static void korina_multicast_list(struct net_device *dev)
 {
 	struct korina_private *lp = netdev_priv(dev);
 	unsigned long flags;
-	struct dev_mc_list *dmi = dev->mc_list;
+	struct dev_mc_list *dmi;
 	u32 recognise = ETH_ARC_AB;	/* always accept broadcasts */
 	int i;
 
@@ -502,11 +502,9 @@ static void korina_multicast_list(struct net_device *dev)
 		for (i = 0; i < 4; i++)
 			hash_table[i] = 0;
 
-		for (i = 0; i < netdev_mc_count(dev); i++) {
+		netdev_for_each_mc_addr(dmi, dev) {
 			char *addrs = dmi->dmi_addr;
 
-			dmi = dmi->next;
-
 			if (!(*addrs & 1))
 				continue;
 
diff --git a/drivers/net/ks8851.c b/drivers/net/ks8851.c
index 9845ab1..b5219cc 100644
--- a/drivers/net/ks8851.c
+++ b/drivers/net/ks8851.c
@@ -966,13 +966,12 @@ static void ks8851_set_rx_mode(struct net_device *dev)
 		rxctrl.rxcr1 = (RXCR1_RXME | RXCR1_RXAE |
 				RXCR1_RXPAFMA | RXCR1_RXMAFMA);
 	} else if (dev->flags & IFF_MULTICAST && !netdev_mc_empty(dev)) {
-		struct dev_mc_list *mcptr = dev->mc_list;
+		struct dev_mc_list *mcptr;
 		u32 crc;
-		int i;
 
 		/* accept some multicast */
 
-		for (i = netdev_mc_count(dev); i > 0; i--) {
+		netdev_for_each_mc_addr(mcptr, dev) {
 			crc = ether_crc(ETH_ALEN, mcptr->dmi_addr);
 			crc >>= (32 - 6);  /* get top six bits */
 
diff --git a/drivers/net/ks8851_mll.c b/drivers/net/ks8851_mll.c
index ffffb38..84b0e15 100644
--- a/drivers/net/ks8851_mll.c
+++ b/drivers/net/ks8851_mll.c
@@ -1196,7 +1196,8 @@ static void ks_set_rx_mode(struct net_device *netdev)
 	if ((netdev->flags & IFF_MULTICAST) && netdev_mc_count(netdev)) {
 		if (netdev_mc_count(netdev) <= MAX_MCAST_LST) {
 			int i = 0;
-			for (ptr = netdev->mc_list; ptr; ptr = ptr->next) {
+
+			netdev_for_each_mc_addr(ptr, netdev) {
 				if (!(*ptr->dmi_addr & 1))
 					continue;
 				if (i >= MAX_MCAST_LST)
diff --git a/drivers/net/ksz884x.c b/drivers/net/ksz884x.c
index 6f187c7..7264a3e 100644
--- a/drivers/net/ksz884x.c
+++ b/drivers/net/ksz884x.c
@@ -5777,7 +5777,7 @@ static void netdev_set_rx_mode(struct net_device *dev)
 	if (hw_priv->hw.dev_count > 1)
 		return;
 
-	if ((dev->flags & IFF_MULTICAST) && dev->mc_count) {
+	if ((dev->flags & IFF_MULTICAST) && !netdev_mc_empty(dev)) {
 		int i = 0;
 
 		/* List too big to support so turn on all multicast mode. */
@@ -5790,7 +5790,7 @@ static void netdev_set_rx_mode(struct net_device *dev)
 			return;
 		}
 
-		for (mc_ptr = dev->mc_list; mc_ptr; mc_ptr = mc_ptr->next) {
+		netdev_for_each_mc_addr(mc_ptr, dev) {
 			if (!(*mc_ptr->dmi_addr & 1))
 				continue;
 			if (i >= MAX_MULTICAST_LIST)
diff --git a/drivers/net/lib82596.c b/drivers/net/lib82596.c
index 371b58b..443c39a 100644
--- a/drivers/net/lib82596.c
+++ b/drivers/net/lib82596.c
@@ -1396,15 +1396,16 @@ static void set_multicast_list(struct net_device *dev)
 		cmd->cmd.command = SWAP16(CmdMulticastList);
 		cmd->mc_cnt = SWAP16(netdev_mc_count(dev) * 6);
 		cp = cmd->mc_addrs;
-		for (dmi = dev->mc_list;
-		     cnt && dmi != NULL;
-		     dmi = dmi->next, cnt--, cp += 6) {
+		netdev_for_each_mc_addr(dmi, dev) {
+			if (!cnt--)
+				break;
 			memcpy(cp, dmi->dmi_addr, 6);
 			if (i596_debug > 1)
 				DEB(DEB_MULTI,
 				    printk(KERN_DEBUG
 					   "%s: Adding address %pM\n",
 					   dev->name, cp));
+			cp += 6;
 		}
 		DMA_WBACK_INV(dev, &dma->mc_cmd, sizeof(struct mc_cmd));
 		i596_add_cmd(dev, &cmd->cmd);
diff --git a/drivers/net/lib8390.c b/drivers/net/lib8390.c
index 57f2584..e629233 100644
--- a/drivers/net/lib8390.c
+++ b/drivers/net/lib8390.c
@@ -907,15 +907,8 @@ static inline void make_mc_bits(u8 *bits, struct net_device *dev)
 {
 	struct dev_mc_list *dmi;
 
-	for (dmi=dev->mc_list; dmi; dmi=dmi->next)
-	{
-		u32 crc;
-		if (dmi->dmi_addrlen != ETH_ALEN)
-		{
-			printk(KERN_INFO "%s: invalid multicast address length given.\n", dev->name);
-			continue;
-		}
-		crc = ether_crc(ETH_ALEN, dmi->dmi_addr);
+	netdev_for_each_mc_addr(dmi, dev) {
+		u32 crc = ether_crc(ETH_ALEN, dmi->dmi_addr);
 		/*
 		 * The 8390 uses the 6 most significant bits of the
 		 * CRC to index the multicast table.
@@ -941,7 +934,7 @@ static void do_set_multicast_list(struct net_device *dev)
 	if (!(dev->flags&(IFF_PROMISC|IFF_ALLMULTI)))
 	{
 		memset(ei_local->mcfilter, 0, 8);
-		if (dev->mc_list)
+		if (!netdev_mc_empty(dev))
 			make_mc_bits(ei_local->mcfilter, dev);
 	}
 	else
@@ -975,7 +968,7 @@ static void do_set_multicast_list(struct net_device *dev)
 
   	if(dev->flags&IFF_PROMISC)
   		ei_outb_p(E8390_RXCONFIG | 0x18, e8390_base + EN0_RXCR);
-	else if(dev->flags&IFF_ALLMULTI || dev->mc_list)
+	else if (dev->flags & IFF_ALLMULTI || !netdev_mc_addr(dev))
   		ei_outb_p(E8390_RXCONFIG | 0x08, e8390_base + EN0_RXCR);
   	else
   		ei_outb_p(E8390_RXCONFIG, e8390_base + EN0_RXCR);
diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
index e534402..a18e348 100644
--- a/drivers/net/ll_temac_main.c
+++ b/drivers/net/ll_temac_main.c
@@ -250,9 +250,10 @@ static void temac_set_multicast_list(struct net_device *ndev)
 		temac_indirect_out32(lp, XTE_AFM_OFFSET, XTE_AFM_EPPRM_MASK);
 		dev_info(&ndev->dev, "Promiscuous mode enabled.\n");
 	} else if (!netdev_mc_empty(ndev)) {
-		struct dev_mc_list *mclist = ndev->mc_list;
-		for (i = 0; mclist && i < netdev_mc_count(ndev); i++) {
+		struct dev_mc_list *mclist;
 
+		i = 0;
+		netdev_for_each_mc_addr(mclist, ndev) {
 			if (i >= MULTICAST_CAM_TABLE_NUM)
 				break;
 			multi_addr_msw = ((mclist->dmi_addr[3] << 24) |
@@ -265,7 +266,7 @@ static void temac_set_multicast_list(struct net_device *ndev)
 					  (mclist->dmi_addr[4]) | (i << 16));
 			temac_indirect_out32(lp, XTE_MAW1_OFFSET,
 					     multi_addr_lsw);
-			mclist = mclist->next;
+			i++;
 		}
 	} else {
 		val = temac_indirect_in32(lp, XTE_AFM_OFFSET);
diff --git a/drivers/net/lp486e.c b/drivers/net/lp486e.c
index b1f5d79..3e3cc04 100644
--- a/drivers/net/lp486e.c
+++ b/drivers/net/lp486e.c
@@ -1267,8 +1267,8 @@ static void set_multicast_list(struct net_device *dev) {
 		cmd->command = CmdMulticastList;
 		*((unsigned short *) (cmd + 1)) = netdev_mc_count(dev) * 6;
 		cp = ((char *)(cmd + 1))+2;
-		for (dmi = dev->mc_list; dmi != NULL; dmi = dmi->next) {
-			memcpy(cp, dmi,6);
+		netdev_for_each_mc_addr(dmi, dev) {
+			memcpy(cp, dmi->dmi_addr, 6);
 			cp += 6;
 		}
 		if (i596_debug & LOG_SRCDST)
diff --git a/drivers/net/mac89x0.c b/drivers/net/mac89x0.c
index 23b633e..c292a60 100644
--- a/drivers/net/mac89x0.c
+++ b/drivers/net/mac89x0.c
@@ -568,9 +568,7 @@ static void set_multicast_list(struct net_device *dev)
 	if(dev->flags&IFF_PROMISC)
 	{
 		lp->rx_mode = RX_ALL_ACCEPT;
-	}
-	else if((dev->flags&IFF_ALLMULTI)||dev->mc_list)
-	{
+	} else if ((dev->flags & IFF_ALLMULTI) || !netdev_mc_empty(dev)) {
 		/* The multicast-accept list is initialized to accept-all, and we
 		   rely on higher-level filtering for now. */
 		lp->rx_mode = RX_MULTCAST_ACCEPT;
diff --git a/drivers/net/macb.c b/drivers/net/macb.c
index 7a5f897..c8a18a6 100644
--- a/drivers/net/macb.c
+++ b/drivers/net/macb.c
@@ -884,15 +884,12 @@ static void macb_sethashtable(struct net_device *dev)
 {
 	struct dev_mc_list *curr;
 	unsigned long mc_filter[2];
-	unsigned int i, bitnr;
+	unsigned int bitnr;
 	struct macb *bp = netdev_priv(dev);
 
 	mc_filter[0] = mc_filter[1] = 0;
 
-	curr = dev->mc_list;
-	for (i = 0; i < netdev_mc_count(dev); i++, curr = curr->next) {
-		if (!curr) break;	/* unexpected end of list */
-
+	netdev_for_each_mc_addr(curr, dev) {
 		bitnr = hash_get_index(curr->dmi_addr);
 		mc_filter[bitnr >> 5] |= 1 << (bitnr & 31);
 	}
diff --git a/drivers/net/mace.c b/drivers/net/mace.c
index fdb0bbd..57534f0 100644
--- a/drivers/net/mace.c
+++ b/drivers/net/mace.c
@@ -588,7 +588,7 @@ static void mace_set_multicast(struct net_device *dev)
 {
     struct mace_data *mp = netdev_priv(dev);
     volatile struct mace __iomem *mb = mp->mace;
-    int i, j;
+    int i;
     u32 crc;
     unsigned long flags;
 
@@ -598,7 +598,7 @@ static void mace_set_multicast(struct net_device *dev)
 	mp->maccc |= PROM;
     } else {
 	unsigned char multicast_filter[8];
-	struct dev_mc_list *dmi = dev->mc_list;
+	struct dev_mc_list *dmi;
 
 	if (dev->flags & IFF_ALLMULTI) {
 	    for (i = 0; i < 8; i++)
@@ -606,11 +606,10 @@ static void mace_set_multicast(struct net_device *dev)
 	} else {
 	    for (i = 0; i < 8; i++)
 		multicast_filter[i] = 0;
-	    for (i = 0; i < netdev_mc_count(dev); i++) {
+	    netdev_for_each_mc_addr(dmi, dev) {
 	        crc = ether_crc_le(6, dmi->dmi_addr);
-		j = crc >> 26;	/* bit number in multicast_filter */
-		multicast_filter[j >> 3] |= 1 << (j & 7);
-		dmi = dmi->next;
+		i = crc >> 26;	/* bit number in multicast_filter */
+		multicast_filter[i >> 3] |= 1 << (i & 7);
 	    }
 	}
 #if 0
diff --git a/drivers/net/macmace.c b/drivers/net/macmace.c
index 740accb..4e4eac0 100644
--- a/drivers/net/macmace.c
+++ b/drivers/net/macmace.c
@@ -496,7 +496,7 @@ static void mace_set_multicast(struct net_device *dev)
 {
 	struct mace_data *mp = netdev_priv(dev);
 	volatile struct mace *mb = mp->mace;
-	int i, j;
+	int i;
 	u32 crc;
 	u8 maccc;
 	unsigned long flags;
@@ -509,7 +509,7 @@ static void mace_set_multicast(struct net_device *dev)
 		mb->maccc |= PROM;
 	} else {
 		unsigned char multicast_filter[8];
-		struct dev_mc_list *dmi = dev->mc_list;
+		struct dev_mc_list *dmi;
 
 		if (dev->flags & IFF_ALLMULTI) {
 			for (i = 0; i < 8; i++) {
@@ -518,11 +518,11 @@ static void mace_set_multicast(struct net_device *dev)
 		} else {
 			for (i = 0; i < 8; i++)
 				multicast_filter[i] = 0;
-			for (i = 0; i < netdev_mc_count(dev); i++) {
+			netdev_for_each_mc_addr(dmi, dev) {
 				crc = ether_crc_le(6, dmi->dmi_addr);
-				j = crc >> 26;	/* bit number in multicast_filter */
-				multicast_filter[j >> 3] |= 1 << (j & 7);
-				dmi = dmi->next;
+				/* bit number in multicast_filter */
+				i = crc >> 26;
+				multicast_filter[i >> 3] |= 1 << (i & 7);
 			}
 		}
 
diff --git a/drivers/net/mv643xx_eth.c b/drivers/net/mv643xx_eth.c
index 2733b0a..c97b6e4 100644
--- a/drivers/net/mv643xx_eth.c
+++ b/drivers/net/mv643xx_eth.c
@@ -1794,7 +1794,7 @@ oom:
 	memset(mc_spec, 0, 0x100);
 	memset(mc_other, 0, 0x100);
 
-	for (addr = dev->mc_list; addr != NULL; addr = addr->next) {
+	netdev_for_each_mc_addr(addr, dev) {
 		u8 *a = addr->da_addr;
 		u32 *table;
 		int entry;
diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
index c0884a9..e06fbd9 100644
--- a/drivers/net/myri10ge/myri10ge.c
+++ b/drivers/net/myri10ge/myri10ge.c
@@ -3065,7 +3065,7 @@ static void myri10ge_set_multicast_list(struct net_device *dev)
 	}
 
 	/* Walk the multicast list, and add each address */
-	for (mc_list = dev->mc_list; mc_list != NULL; mc_list = mc_list->next) {
+	netdev_for_each_mc_addr(mc_list, dev) {
 		memcpy(data, &mc_list->dmi_addr, 6);
 		cmd.data0 = ntohl(data[0]);
 		cmd.data1 = ntohl(data[1]);
diff --git a/drivers/net/natsemi.c b/drivers/net/natsemi.c
index c64e5b0..e520387 100644
--- a/drivers/net/natsemi.c
+++ b/drivers/net/natsemi.c
@@ -2495,9 +2495,9 @@ static void __set_rx_mode(struct net_device *dev)
 	} else {
 		struct dev_mc_list *mclist;
 		int i;
+
 		memset(mc_filter, 0, sizeof(mc_filter));
-		for (i = 0, mclist = dev->mc_list; mclist && i < netdev_mc_count(dev);
-			 i++, mclist = mclist->next) {
+		netdev_for_each_mc_addr(mclist, dev) {
 			int b = (ether_crc(ETH_ALEN, mclist->dmi_addr) >> 23) & 0x1ff;
 			mc_filter[b/8] |= (1 << (b & 0x07));
 		}
diff --git a/drivers/net/netxen/netxen_nic_hw.c b/drivers/net/netxen/netxen_nic_hw.c
index 25f4414..a945591 100644
--- a/drivers/net/netxen/netxen_nic_hw.c
+++ b/drivers/net/netxen/netxen_nic_hw.c
@@ -539,7 +539,7 @@ void netxen_p2_nic_set_multi(struct net_device *netdev)
 	struct netxen_adapter *adapter = netdev_priv(netdev);
 	struct dev_mc_list *mc_ptr;
 	u8 null_addr[6];
-	int index = 0;
+	int i;
 
 	memset(null_addr, 0, 6);
 
@@ -570,16 +570,13 @@ void netxen_p2_nic_set_multi(struct net_device *netdev)
 
 	netxen_nic_enable_mcast_filter(adapter);
 
-	for (mc_ptr = netdev->mc_list; mc_ptr; mc_ptr = mc_ptr->next, index++)
-		netxen_nic_set_mcast_addr(adapter, index, mc_ptr->dmi_addr);
-
-	if (index != netdev_mc_count(netdev))
-		printk(KERN_WARNING "%s: %s multicast address count mismatch\n",
-			netxen_nic_driver_name, netdev->name);
+	i = 0;
+	netdev_for_each_mc_addr(mc_ptr, netdev)
+		netxen_nic_set_mcast_addr(adapter, i++, mc_ptr->dmi_addr);
 
 	/* Clear out remaining addresses */
-	for (; index < adapter->max_mc_count; index++)
-		netxen_nic_set_mcast_addr(adapter, index, null_addr);
+	while (i < adapter->max_mc_count)
+		netxen_nic_set_mcast_addr(adapter, i++, null_addr);
 }
 
 static int
@@ -710,10 +707,8 @@ void netxen_p3_nic_set_multi(struct net_device *netdev)
 	}
 
 	if (!netdev_mc_empty(netdev)) {
-		for (mc_ptr = netdev->mc_list; mc_ptr;
-		     mc_ptr = mc_ptr->next) {
+		netdev_for_each_mc_addr(mc_ptr, netdev)
 			nx_p3_nic_add_mac(adapter, mc_ptr->dmi_addr, &del_list);
-		}
 	}
 
 send_fw_cmd:
diff --git a/drivers/net/ni5010.c b/drivers/net/ni5010.c
index 6a87d81..c16cbfb 100644
--- a/drivers/net/ni5010.c
+++ b/drivers/net/ni5010.c
@@ -651,7 +651,8 @@ static void ni5010_set_multicast_list(struct net_device *dev)
 
 	PRINTK2((KERN_DEBUG "%s: entering set_multicast_list\n", dev->name));
 
-	if (dev->flags&IFF_PROMISC || dev->flags&IFF_ALLMULTI || dev->mc_list) {
+	if (dev->flags & IFF_PROMISC || dev->flags & IFF_ALLMULTI ||
+	    !netdev_mc_empty(dev)) {
 		outb(RMD_PROMISC, EDLC_RMODE); /* Enable promiscuous mode */
 		PRINTK((KERN_DEBUG "%s: Entering promiscuous mode\n", dev->name));
 	} else {
diff --git a/drivers/net/ni52.c b/drivers/net/ni52.c
index 497c6d5..05c29c2 100644
--- a/drivers/net/ni52.c
+++ b/drivers/net/ni52.c
@@ -596,7 +596,7 @@ static int init586(struct net_device *dev)
 	struct iasetup_cmd_struct __iomem *ias_cmd;
 	struct tdr_cmd_struct __iomem *tdr_cmd;
 	struct mcsetup_cmd_struct __iomem *mc_cmd;
-	struct dev_mc_list *dmi = dev->mc_list;
+	struct dev_mc_list *dmi;
 	int num_addrs = netdev_mc_count(dev);
 
 	ptr = p->scb + 1;
@@ -724,9 +724,9 @@ static int init586(struct net_device *dev)
 		writew(0xffff, &mc_cmd->cmd_link);
 		writew(num_addrs * 6, &mc_cmd->mc_cnt);
 
-		for (i = 0; i < num_addrs; i++, dmi = dmi->next)
-			memcpy_toio(mc_cmd->mc_list[i],
-							dmi->dmi_addr, 6);
+		i = 0;
+		netdev_for_each_mc_addr(dmi, dev)
+			memcpy_toio(mc_cmd->mc_list[i++], dmi->dmi_addr, 6);
 
 		writew(make16(mc_cmd), &p->scb->cbl_offset);
 		writeb(CUC_START, &p->scb->cmd_cuc);
diff --git a/drivers/net/niu.c b/drivers/net/niu.c
index 5e604e3..0678f31 100644
--- a/drivers/net/niu.c
+++ b/drivers/net/niu.c
@@ -6365,7 +6365,7 @@ static void niu_set_rx_mode(struct net_device *dev)
 		for (i = 0; i < 16; i++)
 			hash[i] = 0xffff;
 	} else if (!netdev_mc_empty(dev)) {
-		for (addr = dev->mc_list; addr; addr = addr->next) {
+		netdev_for_each_mc_addr(addr, dev) {
 			u32 crc = ether_crc_le(ETH_ALEN, addr->da_addr);
 
 			crc >>= 24;
diff --git a/drivers/net/octeon/octeon_mgmt.c b/drivers/net/octeon/octeon_mgmt.c
index 3a0f910..be368e5 100644
--- a/drivers/net/octeon/octeon_mgmt.c
+++ b/drivers/net/octeon/octeon_mgmt.c
@@ -467,7 +467,6 @@ static void octeon_mgmt_set_rx_filtering(struct net_device *netdev)
 {
 	struct octeon_mgmt *p = netdev_priv(netdev);
 	int port = p->port;
-	int i;
 	union cvmx_agl_gmx_rxx_adr_ctl adr_ctl;
 	union cvmx_agl_gmx_prtx_cfg agl_gmx_prtx;
 	unsigned long flags;
@@ -511,12 +510,8 @@ static void octeon_mgmt_set_rx_filtering(struct net_device *netdev)
 		}
 	}
 	if (multicast_mode == 0) {
-		i = netdev_mc_count(netdev);
-		list = netdev->mc_list;
-		while (i--) {
+		netdev_for_each_mc_addr(list, netdev)
 			octeon_mgmt_cam_state_add(&cam_state, list->da_addr);
-			list = list->next;
-		}
 	}
 
 
diff --git a/drivers/net/pci-skeleton.c b/drivers/net/pci-skeleton.c
index 11d4398..3678585 100644
--- a/drivers/net/pci-skeleton.c
+++ b/drivers/net/pci-skeleton.c
@@ -1793,7 +1793,7 @@ static void netdrv_set_rx_mode(struct net_device *dev)
 	struct netdrv_private *tp = netdev_priv(dev);
 	void *ioaddr = tp->mmio_addr;
 	u32 mc_filter[2];	/* Multicast hash filter */
-	int i, rx_mode;
+	int rx_mode;
 	u32 tmp;
 
 	DPRINTK("ENTER\n");
@@ -1814,10 +1814,10 @@ static void netdrv_set_rx_mode(struct net_device *dev)
 		mc_filter[1] = mc_filter[0] = 0xffffffff;
 	} else {
 		struct dev_mc_list *mclist;
+
 		rx_mode = AcceptBroadcast | AcceptMulticast | AcceptMyPhys;
 		mc_filter[1] = mc_filter[0] = 0;
-		for (i = 0, mclist = dev->mc_list; mclist && i < netdev_mc_count(dev);
-		     i++, mclist = mclist->next) {
+		netdev_for_each_mc_addr(mclist, dev) {
 			int bit_nr = ether_crc(ETH_ALEN, mclist->dmi_addr) >> 26;
 
 			mc_filter[bit_nr >> 5] |= 1 << (bit_nr & 31);
diff --git a/drivers/net/pcnet32.c b/drivers/net/pcnet32.c
index 63e0315..084d78d 100644
--- a/drivers/net/pcnet32.c
+++ b/drivers/net/pcnet32.c
@@ -2590,7 +2590,7 @@ static void pcnet32_load_multicast(struct net_device *dev)
 	struct pcnet32_private *lp = netdev_priv(dev);
 	volatile struct pcnet32_init_block *ib = lp->init_block;
 	volatile __le16 *mcast_table = (__le16 *)ib->filter;
-	struct dev_mc_list *dmi = dev->mc_list;
+	struct dev_mc_list *dmi;
 	unsigned long ioaddr = dev->base_addr;
 	char *addrs;
 	int i;
@@ -2611,9 +2611,8 @@ static void pcnet32_load_multicast(struct net_device *dev)
 	ib->filter[1] = 0;
 
 	/* Add addresses */
-	for (i = 0; i < netdev_mc_count(dev); i++) {
+	netdev_for_each_mc_addr(dmi, dev) {
 		addrs = dmi->dmi_addr;
-		dmi = dmi->next;
 
 		/* multicast address? */
 		if (!(*addrs & 1))
diff --git a/drivers/net/ps3_gelic_net.c b/drivers/net/ps3_gelic_net.c
index c19dd4a..a849f6f 100644
--- a/drivers/net/ps3_gelic_net.c
+++ b/drivers/net/ps3_gelic_net.c
@@ -580,7 +580,7 @@ void gelic_net_set_multi(struct net_device *netdev)
 	}
 
 	/* set multicast addresses */
-	for (mc = netdev->mc_list; mc; mc = mc->next) {
+	netdev_for_each_mc_addr(mc, netdev) {
 		addr = 0;
 		p = mc->dmi_addr;
 		for (i = 0; i < ETH_ALEN; i++) {
diff --git a/drivers/net/qlcnic/qlcnic_hw.c b/drivers/net/qlcnic/qlcnic_hw.c
index 8ea7f86..99a4d13 100644
--- a/drivers/net/qlcnic/qlcnic_hw.c
+++ b/drivers/net/qlcnic/qlcnic_hw.c
@@ -453,8 +453,7 @@ void qlcnic_set_multi(struct net_device *netdev)
 	}
 
 	if (!netdev_mc_empty(netdev)) {
-		for (mc_ptr = netdev->mc_list; mc_ptr;
-				     mc_ptr = mc_ptr->next) {
+		netdev_for_each_mc_addr(mc_ptr, netdev) {
 			qlcnic_nic_add_mac(adapter, mc_ptr->dmi_addr,
 							&del_list);
 		}
diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index c170349..c26ec5d 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -4270,8 +4270,8 @@ static void qlge_set_multicast_list(struct net_device *ndev)
 		status = ql_sem_spinlock(qdev, SEM_MAC_ADDR_MASK);
 		if (status)
 			goto exit;
-		for (i = 0, mc_ptr = ndev->mc_list; mc_ptr;
-		     i++, mc_ptr = mc_ptr->next)
+		i = 0;
+		netdev_for_each_mc_addr(mc_ptr, ndev) {
 			if (ql_set_mac_addr_reg(qdev, (u8 *) mc_ptr->dmi_addr,
 						MAC_ADDR_TYPE_MULTI_MAC, i)) {
 				netif_err(qdev, hw, qdev->ndev,
@@ -4279,6 +4279,8 @@ static void qlge_set_multicast_list(struct net_device *ndev)
 				ql_sem_unlock(qdev, SEM_MAC_ADDR_MASK);
 				goto exit;
 			}
+			i++;
+		}
 		ql_sem_unlock(qdev, SEM_MAC_ADDR_MASK);
 		if (ql_set_routing_reg
 		    (qdev, RT_IDX_MCAST_MATCH_SLOT, RT_IDX_MCAST_MATCH, 1)) {
diff --git a/drivers/net/r6040.c b/drivers/net/r6040.c
index b810342..15d5373 100644
--- a/drivers/net/r6040.c
+++ b/drivers/net/r6040.c
@@ -938,7 +938,7 @@ static void r6040_multicast_list(struct net_device *dev)
 	u16 *adrp;
 	u16 reg;
 	unsigned long flags;
-	struct dev_mc_list *dmi = dev->mc_list;
+	struct dev_mc_list *dmi;
 	int i;
 
 	/* MAC Address */
@@ -973,11 +973,9 @@ static void r6040_multicast_list(struct net_device *dev)
 		for (i = 0; i < 4; i++)
 			hash_table[i] = 0;
 
-		for (i = 0; i < netdev_mc_count(dev); i++) {
+		netdev_for_each_mc_addr(dmi, dev) {
 			char *addrs = dmi->dmi_addr;
 
-			dmi = dmi->next;
-
 			if (!(*addrs & 1))
 				continue;
 
@@ -995,17 +993,19 @@ static void r6040_multicast_list(struct net_device *dev)
 		iowrite16(hash_table[3], ioaddr + MAR3);
 	}
 	/* Multicast Address 1~4 case */
-	for (i = 0, dmi; (i < netdev_mc_count(dev)) && (i < MCAST_MAX); i++) {
-		adrp = (u16 *)dmi->dmi_addr;
-		iowrite16(adrp[0], ioaddr + MID_1L + 8*i);
-		iowrite16(adrp[1], ioaddr + MID_1M + 8*i);
-		iowrite16(adrp[2], ioaddr + MID_1H + 8*i);
-		dmi = dmi->next;
-	}
-	for (i = netdev_mc_count(dev); i < MCAST_MAX; i++) {
-		iowrite16(0xffff, ioaddr + MID_0L + 8*i);
-		iowrite16(0xffff, ioaddr + MID_0M + 8*i);
-		iowrite16(0xffff, ioaddr + MID_0H + 8*i);
+	i = 0;
+	netdev_for_each_mc_addr(dmi, dev) {
+		if (i < MCAST_MAX) {
+			adrp = (u16 *) dmi->dmi_addr;
+			iowrite16(adrp[0], ioaddr + MID_1L + 8 * i);
+			iowrite16(adrp[1], ioaddr + MID_1M + 8 * i);
+			iowrite16(adrp[2], ioaddr + MID_1H + 8 * i);
+		} else {
+			iowrite16(0xffff, ioaddr + MID_0L + 8 * i);
+			iowrite16(0xffff, ioaddr + MID_0M + 8 * i);
+			iowrite16(0xffff, ioaddr + MID_0H + 8 * i);
+		}
+		i++;
 	}
 }
 
diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 83965ee..dfc3573 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -4732,12 +4732,10 @@ static void rtl_set_rx_mode(struct net_device *dev)
 		mc_filter[1] = mc_filter[0] = 0xffffffff;
 	} else {
 		struct dev_mc_list *mclist;
-		unsigned int i;
 
 		rx_mode = AcceptBroadcast | AcceptMyPhys;
 		mc_filter[1] = mc_filter[0] = 0;
-		for (i = 0, mclist = dev->mc_list; mclist && i < netdev_mc_count(dev);
-		     i++, mclist = mclist->next) {
+		netdev_for_each_mc_addr(mclist, dev) {
 			int bit_nr = ether_crc(ETH_ALEN, mclist->dmi_addr) >> 26;
 			mc_filter[bit_nr >> 5] |= 1 << (bit_nr & 31);
 			rx_mode |= AcceptMulticast;
-- 
1.6.6


^ permalink raw reply related

* Re: [net-next-2.6 PATCH] net: convert multiple drivers to use netdev_for_each_mc_addr, part5
From: Jiri Pirko @ 2010-02-23 19:12 UTC (permalink / raw)
  To: netdev; +Cc: davem
In-Reply-To: <20100223190739.GC2673@psychotron.redhat.com>

Oups Dave please scratch this... Found issue in octeon

Jirka

Tue, Feb 23, 2010 at 08:07:40PM CET, jpirko@redhat.com wrote:
>
>removed some needless checks and also corrected bug in lp486e (dmi was passed
>instead of dmi->dmi_addr)
>
>Signed-off-by: Jiri Pirko <jpirko@redhat.com>
>---
> drivers/net/jme.c                  |    6 +-----
> drivers/net/korina.c               |    6 ++----
> drivers/net/ks8851.c               |    5 ++---
> drivers/net/ks8851_mll.c           |    3 ++-
> drivers/net/ksz884x.c              |    4 ++--
> drivers/net/lib82596.c             |    7 ++++---
> drivers/net/lib8390.c              |   15 ++++-----------
> drivers/net/ll_temac_main.c        |    7 ++++---
> drivers/net/lp486e.c               |    4 ++--
> drivers/net/mac89x0.c              |    4 +---
> drivers/net/macb.c                 |    7 ++-----
> drivers/net/mace.c                 |   11 +++++------
> drivers/net/macmace.c              |   12 ++++++------
> drivers/net/mv643xx_eth.c          |    2 +-
> drivers/net/myri10ge/myri10ge.c    |    2 +-
> drivers/net/natsemi.c              |    4 ++--
> drivers/net/netxen/netxen_nic_hw.c |   19 +++++++------------
> drivers/net/ni5010.c               |    3 ++-
> drivers/net/ni52.c                 |    8 ++++----
> drivers/net/niu.c                  |    2 +-
> drivers/net/octeon/octeon_mgmt.c   |    7 +------
> drivers/net/pci-skeleton.c         |    6 +++---
> drivers/net/pcnet32.c              |    5 ++---
> drivers/net/ps3_gelic_net.c        |    2 +-
> drivers/net/qlcnic/qlcnic_hw.c     |    3 +--
> drivers/net/qlge/qlge_main.c       |    6 ++++--
> drivers/net/r6040.c                |   30 +++++++++++++++---------------
> drivers/net/r8169.c                |    4 +---
> 28 files changed, 83 insertions(+), 111 deletions(-)
>
>diff --git a/drivers/net/jme.c b/drivers/net/jme.c
>index 558b6a0..0f31497 100644
>--- a/drivers/net/jme.c
>+++ b/drivers/net/jme.c
>@@ -1997,7 +1997,6 @@ jme_set_multi(struct net_device *netdev)
> {
> 	struct jme_adapter *jme = netdev_priv(netdev);
> 	u32 mc_hash[2] = {};
>-	int i;
> 
> 	spin_lock_bh(&jme->rxmcs_lock);
> 
>@@ -2012,10 +2011,7 @@ jme_set_multi(struct net_device *netdev)
> 		int bit_nr;
> 
> 		jme->reg_rxmcs |= RXMCS_MULFRAME | RXMCS_MULFILTERED;
>-		for (i = 0, mclist = netdev->mc_list;
>-			mclist && i < netdev_mc_count(netdev);
>-			++i, mclist = mclist->next) {
>-
>+		netdev_for_each_mc_addr(mclist, netdev) {
> 			bit_nr = ether_crc(ETH_ALEN, mclist->dmi_addr) & 0x3F;
> 			mc_hash[bit_nr >> 5] |= 1 << (bit_nr & 0x1F);
> 		}
>diff --git a/drivers/net/korina.c b/drivers/net/korina.c
>index af0c764..300c224 100644
>--- a/drivers/net/korina.c
>+++ b/drivers/net/korina.c
>@@ -482,7 +482,7 @@ static void korina_multicast_list(struct net_device *dev)
> {
> 	struct korina_private *lp = netdev_priv(dev);
> 	unsigned long flags;
>-	struct dev_mc_list *dmi = dev->mc_list;
>+	struct dev_mc_list *dmi;
> 	u32 recognise = ETH_ARC_AB;	/* always accept broadcasts */
> 	int i;
> 
>@@ -502,11 +502,9 @@ static void korina_multicast_list(struct net_device *dev)
> 		for (i = 0; i < 4; i++)
> 			hash_table[i] = 0;
> 
>-		for (i = 0; i < netdev_mc_count(dev); i++) {
>+		netdev_for_each_mc_addr(dmi, dev) {
> 			char *addrs = dmi->dmi_addr;
> 
>-			dmi = dmi->next;
>-
> 			if (!(*addrs & 1))
> 				continue;
> 
>diff --git a/drivers/net/ks8851.c b/drivers/net/ks8851.c
>index 9845ab1..b5219cc 100644
>--- a/drivers/net/ks8851.c
>+++ b/drivers/net/ks8851.c
>@@ -966,13 +966,12 @@ static void ks8851_set_rx_mode(struct net_device *dev)
> 		rxctrl.rxcr1 = (RXCR1_RXME | RXCR1_RXAE |
> 				RXCR1_RXPAFMA | RXCR1_RXMAFMA);
> 	} else if (dev->flags & IFF_MULTICAST && !netdev_mc_empty(dev)) {
>-		struct dev_mc_list *mcptr = dev->mc_list;
>+		struct dev_mc_list *mcptr;
> 		u32 crc;
>-		int i;
> 
> 		/* accept some multicast */
> 
>-		for (i = netdev_mc_count(dev); i > 0; i--) {
>+		netdev_for_each_mc_addr(mcptr, dev) {
> 			crc = ether_crc(ETH_ALEN, mcptr->dmi_addr);
> 			crc >>= (32 - 6);  /* get top six bits */
> 
>diff --git a/drivers/net/ks8851_mll.c b/drivers/net/ks8851_mll.c
>index ffffb38..84b0e15 100644
>--- a/drivers/net/ks8851_mll.c
>+++ b/drivers/net/ks8851_mll.c
>@@ -1196,7 +1196,8 @@ static void ks_set_rx_mode(struct net_device *netdev)
> 	if ((netdev->flags & IFF_MULTICAST) && netdev_mc_count(netdev)) {
> 		if (netdev_mc_count(netdev) <= MAX_MCAST_LST) {
> 			int i = 0;
>-			for (ptr = netdev->mc_list; ptr; ptr = ptr->next) {
>+
>+			netdev_for_each_mc_addr(ptr, netdev) {
> 				if (!(*ptr->dmi_addr & 1))
> 					continue;
> 				if (i >= MAX_MCAST_LST)
>diff --git a/drivers/net/ksz884x.c b/drivers/net/ksz884x.c
>index 6f187c7..7264a3e 100644
>--- a/drivers/net/ksz884x.c
>+++ b/drivers/net/ksz884x.c
>@@ -5777,7 +5777,7 @@ static void netdev_set_rx_mode(struct net_device *dev)
> 	if (hw_priv->hw.dev_count > 1)
> 		return;
> 
>-	if ((dev->flags & IFF_MULTICAST) && dev->mc_count) {
>+	if ((dev->flags & IFF_MULTICAST) && !netdev_mc_empty(dev)) {
> 		int i = 0;
> 
> 		/* List too big to support so turn on all multicast mode. */
>@@ -5790,7 +5790,7 @@ static void netdev_set_rx_mode(struct net_device *dev)
> 			return;
> 		}
> 
>-		for (mc_ptr = dev->mc_list; mc_ptr; mc_ptr = mc_ptr->next) {
>+		netdev_for_each_mc_addr(mc_ptr, dev) {
> 			if (!(*mc_ptr->dmi_addr & 1))
> 				continue;
> 			if (i >= MAX_MULTICAST_LIST)
>diff --git a/drivers/net/lib82596.c b/drivers/net/lib82596.c
>index 371b58b..443c39a 100644
>--- a/drivers/net/lib82596.c
>+++ b/drivers/net/lib82596.c
>@@ -1396,15 +1396,16 @@ static void set_multicast_list(struct net_device *dev)
> 		cmd->cmd.command = SWAP16(CmdMulticastList);
> 		cmd->mc_cnt = SWAP16(netdev_mc_count(dev) * 6);
> 		cp = cmd->mc_addrs;
>-		for (dmi = dev->mc_list;
>-		     cnt && dmi != NULL;
>-		     dmi = dmi->next, cnt--, cp += 6) {
>+		netdev_for_each_mc_addr(dmi, dev) {
>+			if (!cnt--)
>+				break;
> 			memcpy(cp, dmi->dmi_addr, 6);
> 			if (i596_debug > 1)
> 				DEB(DEB_MULTI,
> 				    printk(KERN_DEBUG
> 					   "%s: Adding address %pM\n",
> 					   dev->name, cp));
>+			cp += 6;
> 		}
> 		DMA_WBACK_INV(dev, &dma->mc_cmd, sizeof(struct mc_cmd));
> 		i596_add_cmd(dev, &cmd->cmd);
>diff --git a/drivers/net/lib8390.c b/drivers/net/lib8390.c
>index 57f2584..e629233 100644
>--- a/drivers/net/lib8390.c
>+++ b/drivers/net/lib8390.c
>@@ -907,15 +907,8 @@ static inline void make_mc_bits(u8 *bits, struct net_device *dev)
> {
> 	struct dev_mc_list *dmi;
> 
>-	for (dmi=dev->mc_list; dmi; dmi=dmi->next)
>-	{
>-		u32 crc;
>-		if (dmi->dmi_addrlen != ETH_ALEN)
>-		{
>-			printk(KERN_INFO "%s: invalid multicast address length given.\n", dev->name);
>-			continue;
>-		}
>-		crc = ether_crc(ETH_ALEN, dmi->dmi_addr);
>+	netdev_for_each_mc_addr(dmi, dev) {
>+		u32 crc = ether_crc(ETH_ALEN, dmi->dmi_addr);
> 		/*
> 		 * The 8390 uses the 6 most significant bits of the
> 		 * CRC to index the multicast table.
>@@ -941,7 +934,7 @@ static void do_set_multicast_list(struct net_device *dev)
> 	if (!(dev->flags&(IFF_PROMISC|IFF_ALLMULTI)))
> 	{
> 		memset(ei_local->mcfilter, 0, 8);
>-		if (dev->mc_list)
>+		if (!netdev_mc_empty(dev))
> 			make_mc_bits(ei_local->mcfilter, dev);
> 	}
> 	else
>@@ -975,7 +968,7 @@ static void do_set_multicast_list(struct net_device *dev)
> 
>   	if(dev->flags&IFF_PROMISC)
>   		ei_outb_p(E8390_RXCONFIG | 0x18, e8390_base + EN0_RXCR);
>-	else if(dev->flags&IFF_ALLMULTI || dev->mc_list)
>+	else if (dev->flags & IFF_ALLMULTI || !netdev_mc_addr(dev))
>   		ei_outb_p(E8390_RXCONFIG | 0x08, e8390_base + EN0_RXCR);
>   	else
>   		ei_outb_p(E8390_RXCONFIG, e8390_base + EN0_RXCR);
>diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
>index e534402..a18e348 100644
>--- a/drivers/net/ll_temac_main.c
>+++ b/drivers/net/ll_temac_main.c
>@@ -250,9 +250,10 @@ static void temac_set_multicast_list(struct net_device *ndev)
> 		temac_indirect_out32(lp, XTE_AFM_OFFSET, XTE_AFM_EPPRM_MASK);
> 		dev_info(&ndev->dev, "Promiscuous mode enabled.\n");
> 	} else if (!netdev_mc_empty(ndev)) {
>-		struct dev_mc_list *mclist = ndev->mc_list;
>-		for (i = 0; mclist && i < netdev_mc_count(ndev); i++) {
>+		struct dev_mc_list *mclist;
> 
>+		i = 0;
>+		netdev_for_each_mc_addr(mclist, ndev) {
> 			if (i >= MULTICAST_CAM_TABLE_NUM)
> 				break;
> 			multi_addr_msw = ((mclist->dmi_addr[3] << 24) |
>@@ -265,7 +266,7 @@ static void temac_set_multicast_list(struct net_device *ndev)
> 					  (mclist->dmi_addr[4]) | (i << 16));
> 			temac_indirect_out32(lp, XTE_MAW1_OFFSET,
> 					     multi_addr_lsw);
>-			mclist = mclist->next;
>+			i++;
> 		}
> 	} else {
> 		val = temac_indirect_in32(lp, XTE_AFM_OFFSET);
>diff --git a/drivers/net/lp486e.c b/drivers/net/lp486e.c
>index b1f5d79..3e3cc04 100644
>--- a/drivers/net/lp486e.c
>+++ b/drivers/net/lp486e.c
>@@ -1267,8 +1267,8 @@ static void set_multicast_list(struct net_device *dev) {
> 		cmd->command = CmdMulticastList;
> 		*((unsigned short *) (cmd + 1)) = netdev_mc_count(dev) * 6;
> 		cp = ((char *)(cmd + 1))+2;
>-		for (dmi = dev->mc_list; dmi != NULL; dmi = dmi->next) {
>-			memcpy(cp, dmi,6);
>+		netdev_for_each_mc_addr(dmi, dev) {
>+			memcpy(cp, dmi->dmi_addr, 6);
> 			cp += 6;
> 		}
> 		if (i596_debug & LOG_SRCDST)
>diff --git a/drivers/net/mac89x0.c b/drivers/net/mac89x0.c
>index 23b633e..c292a60 100644
>--- a/drivers/net/mac89x0.c
>+++ b/drivers/net/mac89x0.c
>@@ -568,9 +568,7 @@ static void set_multicast_list(struct net_device *dev)
> 	if(dev->flags&IFF_PROMISC)
> 	{
> 		lp->rx_mode = RX_ALL_ACCEPT;
>-	}
>-	else if((dev->flags&IFF_ALLMULTI)||dev->mc_list)
>-	{
>+	} else if ((dev->flags & IFF_ALLMULTI) || !netdev_mc_empty(dev)) {
> 		/* The multicast-accept list is initialized to accept-all, and we
> 		   rely on higher-level filtering for now. */
> 		lp->rx_mode = RX_MULTCAST_ACCEPT;
>diff --git a/drivers/net/macb.c b/drivers/net/macb.c
>index 7a5f897..c8a18a6 100644
>--- a/drivers/net/macb.c
>+++ b/drivers/net/macb.c
>@@ -884,15 +884,12 @@ static void macb_sethashtable(struct net_device *dev)
> {
> 	struct dev_mc_list *curr;
> 	unsigned long mc_filter[2];
>-	unsigned int i, bitnr;
>+	unsigned int bitnr;
> 	struct macb *bp = netdev_priv(dev);
> 
> 	mc_filter[0] = mc_filter[1] = 0;
> 
>-	curr = dev->mc_list;
>-	for (i = 0; i < netdev_mc_count(dev); i++, curr = curr->next) {
>-		if (!curr) break;	/* unexpected end of list */
>-
>+	netdev_for_each_mc_addr(curr, dev) {
> 		bitnr = hash_get_index(curr->dmi_addr);
> 		mc_filter[bitnr >> 5] |= 1 << (bitnr & 31);
> 	}
>diff --git a/drivers/net/mace.c b/drivers/net/mace.c
>index fdb0bbd..57534f0 100644
>--- a/drivers/net/mace.c
>+++ b/drivers/net/mace.c
>@@ -588,7 +588,7 @@ static void mace_set_multicast(struct net_device *dev)
> {
>     struct mace_data *mp = netdev_priv(dev);
>     volatile struct mace __iomem *mb = mp->mace;
>-    int i, j;
>+    int i;
>     u32 crc;
>     unsigned long flags;
> 
>@@ -598,7 +598,7 @@ static void mace_set_multicast(struct net_device *dev)
> 	mp->maccc |= PROM;
>     } else {
> 	unsigned char multicast_filter[8];
>-	struct dev_mc_list *dmi = dev->mc_list;
>+	struct dev_mc_list *dmi;
> 
> 	if (dev->flags & IFF_ALLMULTI) {
> 	    for (i = 0; i < 8; i++)
>@@ -606,11 +606,10 @@ static void mace_set_multicast(struct net_device *dev)
> 	} else {
> 	    for (i = 0; i < 8; i++)
> 		multicast_filter[i] = 0;
>-	    for (i = 0; i < netdev_mc_count(dev); i++) {
>+	    netdev_for_each_mc_addr(dmi, dev) {
> 	        crc = ether_crc_le(6, dmi->dmi_addr);
>-		j = crc >> 26;	/* bit number in multicast_filter */
>-		multicast_filter[j >> 3] |= 1 << (j & 7);
>-		dmi = dmi->next;
>+		i = crc >> 26;	/* bit number in multicast_filter */
>+		multicast_filter[i >> 3] |= 1 << (i & 7);
> 	    }
> 	}
> #if 0
>diff --git a/drivers/net/macmace.c b/drivers/net/macmace.c
>index 740accb..4e4eac0 100644
>--- a/drivers/net/macmace.c
>+++ b/drivers/net/macmace.c
>@@ -496,7 +496,7 @@ static void mace_set_multicast(struct net_device *dev)
> {
> 	struct mace_data *mp = netdev_priv(dev);
> 	volatile struct mace *mb = mp->mace;
>-	int i, j;
>+	int i;
> 	u32 crc;
> 	u8 maccc;
> 	unsigned long flags;
>@@ -509,7 +509,7 @@ static void mace_set_multicast(struct net_device *dev)
> 		mb->maccc |= PROM;
> 	} else {
> 		unsigned char multicast_filter[8];
>-		struct dev_mc_list *dmi = dev->mc_list;
>+		struct dev_mc_list *dmi;
> 
> 		if (dev->flags & IFF_ALLMULTI) {
> 			for (i = 0; i < 8; i++) {
>@@ -518,11 +518,11 @@ static void mace_set_multicast(struct net_device *dev)
> 		} else {
> 			for (i = 0; i < 8; i++)
> 				multicast_filter[i] = 0;
>-			for (i = 0; i < netdev_mc_count(dev); i++) {
>+			netdev_for_each_mc_addr(dmi, dev) {
> 				crc = ether_crc_le(6, dmi->dmi_addr);
>-				j = crc >> 26;	/* bit number in multicast_filter */
>-				multicast_filter[j >> 3] |= 1 << (j & 7);
>-				dmi = dmi->next;
>+				/* bit number in multicast_filter */
>+				i = crc >> 26;
>+				multicast_filter[i >> 3] |= 1 << (i & 7);
> 			}
> 		}
> 
>diff --git a/drivers/net/mv643xx_eth.c b/drivers/net/mv643xx_eth.c
>index 2733b0a..c97b6e4 100644
>--- a/drivers/net/mv643xx_eth.c
>+++ b/drivers/net/mv643xx_eth.c
>@@ -1794,7 +1794,7 @@ oom:
> 	memset(mc_spec, 0, 0x100);
> 	memset(mc_other, 0, 0x100);
> 
>-	for (addr = dev->mc_list; addr != NULL; addr = addr->next) {
>+	netdev_for_each_mc_addr(addr, dev) {
> 		u8 *a = addr->da_addr;
> 		u32 *table;
> 		int entry;
>diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
>index c0884a9..e06fbd9 100644
>--- a/drivers/net/myri10ge/myri10ge.c
>+++ b/drivers/net/myri10ge/myri10ge.c
>@@ -3065,7 +3065,7 @@ static void myri10ge_set_multicast_list(struct net_device *dev)
> 	}
> 
> 	/* Walk the multicast list, and add each address */
>-	for (mc_list = dev->mc_list; mc_list != NULL; mc_list = mc_list->next) {
>+	netdev_for_each_mc_addr(mc_list, dev) {
> 		memcpy(data, &mc_list->dmi_addr, 6);
> 		cmd.data0 = ntohl(data[0]);
> 		cmd.data1 = ntohl(data[1]);
>diff --git a/drivers/net/natsemi.c b/drivers/net/natsemi.c
>index c64e5b0..e520387 100644
>--- a/drivers/net/natsemi.c
>+++ b/drivers/net/natsemi.c
>@@ -2495,9 +2495,9 @@ static void __set_rx_mode(struct net_device *dev)
> 	} else {
> 		struct dev_mc_list *mclist;
> 		int i;
>+
> 		memset(mc_filter, 0, sizeof(mc_filter));
>-		for (i = 0, mclist = dev->mc_list; mclist && i < netdev_mc_count(dev);
>-			 i++, mclist = mclist->next) {
>+		netdev_for_each_mc_addr(mclist, dev) {
> 			int b = (ether_crc(ETH_ALEN, mclist->dmi_addr) >> 23) & 0x1ff;
> 			mc_filter[b/8] |= (1 << (b & 0x07));
> 		}
>diff --git a/drivers/net/netxen/netxen_nic_hw.c b/drivers/net/netxen/netxen_nic_hw.c
>index 25f4414..a945591 100644
>--- a/drivers/net/netxen/netxen_nic_hw.c
>+++ b/drivers/net/netxen/netxen_nic_hw.c
>@@ -539,7 +539,7 @@ void netxen_p2_nic_set_multi(struct net_device *netdev)
> 	struct netxen_adapter *adapter = netdev_priv(netdev);
> 	struct dev_mc_list *mc_ptr;
> 	u8 null_addr[6];
>-	int index = 0;
>+	int i;
> 
> 	memset(null_addr, 0, 6);
> 
>@@ -570,16 +570,13 @@ void netxen_p2_nic_set_multi(struct net_device *netdev)
> 
> 	netxen_nic_enable_mcast_filter(adapter);
> 
>-	for (mc_ptr = netdev->mc_list; mc_ptr; mc_ptr = mc_ptr->next, index++)
>-		netxen_nic_set_mcast_addr(adapter, index, mc_ptr->dmi_addr);
>-
>-	if (index != netdev_mc_count(netdev))
>-		printk(KERN_WARNING "%s: %s multicast address count mismatch\n",
>-			netxen_nic_driver_name, netdev->name);
>+	i = 0;
>+	netdev_for_each_mc_addr(mc_ptr, netdev)
>+		netxen_nic_set_mcast_addr(adapter, i++, mc_ptr->dmi_addr);
> 
> 	/* Clear out remaining addresses */
>-	for (; index < adapter->max_mc_count; index++)
>-		netxen_nic_set_mcast_addr(adapter, index, null_addr);
>+	while (i < adapter->max_mc_count)
>+		netxen_nic_set_mcast_addr(adapter, i++, null_addr);
> }
> 
> static int
>@@ -710,10 +707,8 @@ void netxen_p3_nic_set_multi(struct net_device *netdev)
> 	}
> 
> 	if (!netdev_mc_empty(netdev)) {
>-		for (mc_ptr = netdev->mc_list; mc_ptr;
>-		     mc_ptr = mc_ptr->next) {
>+		netdev_for_each_mc_addr(mc_ptr, netdev)
> 			nx_p3_nic_add_mac(adapter, mc_ptr->dmi_addr, &del_list);
>-		}
> 	}
> 
> send_fw_cmd:
>diff --git a/drivers/net/ni5010.c b/drivers/net/ni5010.c
>index 6a87d81..c16cbfb 100644
>--- a/drivers/net/ni5010.c
>+++ b/drivers/net/ni5010.c
>@@ -651,7 +651,8 @@ static void ni5010_set_multicast_list(struct net_device *dev)
> 
> 	PRINTK2((KERN_DEBUG "%s: entering set_multicast_list\n", dev->name));
> 
>-	if (dev->flags&IFF_PROMISC || dev->flags&IFF_ALLMULTI || dev->mc_list) {
>+	if (dev->flags & IFF_PROMISC || dev->flags & IFF_ALLMULTI ||
>+	    !netdev_mc_empty(dev)) {
> 		outb(RMD_PROMISC, EDLC_RMODE); /* Enable promiscuous mode */
> 		PRINTK((KERN_DEBUG "%s: Entering promiscuous mode\n", dev->name));
> 	} else {
>diff --git a/drivers/net/ni52.c b/drivers/net/ni52.c
>index 497c6d5..05c29c2 100644
>--- a/drivers/net/ni52.c
>+++ b/drivers/net/ni52.c
>@@ -596,7 +596,7 @@ static int init586(struct net_device *dev)
> 	struct iasetup_cmd_struct __iomem *ias_cmd;
> 	struct tdr_cmd_struct __iomem *tdr_cmd;
> 	struct mcsetup_cmd_struct __iomem *mc_cmd;
>-	struct dev_mc_list *dmi = dev->mc_list;
>+	struct dev_mc_list *dmi;
> 	int num_addrs = netdev_mc_count(dev);
> 
> 	ptr = p->scb + 1;
>@@ -724,9 +724,9 @@ static int init586(struct net_device *dev)
> 		writew(0xffff, &mc_cmd->cmd_link);
> 		writew(num_addrs * 6, &mc_cmd->mc_cnt);
> 
>-		for (i = 0; i < num_addrs; i++, dmi = dmi->next)
>-			memcpy_toio(mc_cmd->mc_list[i],
>-							dmi->dmi_addr, 6);
>+		i = 0;
>+		netdev_for_each_mc_addr(dmi, dev)
>+			memcpy_toio(mc_cmd->mc_list[i++], dmi->dmi_addr, 6);
> 
> 		writew(make16(mc_cmd), &p->scb->cbl_offset);
> 		writeb(CUC_START, &p->scb->cmd_cuc);
>diff --git a/drivers/net/niu.c b/drivers/net/niu.c
>index 5e604e3..0678f31 100644
>--- a/drivers/net/niu.c
>+++ b/drivers/net/niu.c
>@@ -6365,7 +6365,7 @@ static void niu_set_rx_mode(struct net_device *dev)
> 		for (i = 0; i < 16; i++)
> 			hash[i] = 0xffff;
> 	} else if (!netdev_mc_empty(dev)) {
>-		for (addr = dev->mc_list; addr; addr = addr->next) {
>+		netdev_for_each_mc_addr(addr, dev) {
> 			u32 crc = ether_crc_le(ETH_ALEN, addr->da_addr);
> 
> 			crc >>= 24;
>diff --git a/drivers/net/octeon/octeon_mgmt.c b/drivers/net/octeon/octeon_mgmt.c
>index 3a0f910..be368e5 100644
>--- a/drivers/net/octeon/octeon_mgmt.c
>+++ b/drivers/net/octeon/octeon_mgmt.c
>@@ -467,7 +467,6 @@ static void octeon_mgmt_set_rx_filtering(struct net_device *netdev)
> {
> 	struct octeon_mgmt *p = netdev_priv(netdev);
> 	int port = p->port;
>-	int i;
> 	union cvmx_agl_gmx_rxx_adr_ctl adr_ctl;
> 	union cvmx_agl_gmx_prtx_cfg agl_gmx_prtx;
> 	unsigned long flags;
>@@ -511,12 +510,8 @@ static void octeon_mgmt_set_rx_filtering(struct net_device *netdev)
> 		}
> 	}
> 	if (multicast_mode == 0) {
>-		i = netdev_mc_count(netdev);
>-		list = netdev->mc_list;
>-		while (i--) {
>+		netdev_for_each_mc_addr(list, netdev)
> 			octeon_mgmt_cam_state_add(&cam_state, list->da_addr);
>-			list = list->next;
>-		}
> 	}
> 
> 
>diff --git a/drivers/net/pci-skeleton.c b/drivers/net/pci-skeleton.c
>index 11d4398..3678585 100644
>--- a/drivers/net/pci-skeleton.c
>+++ b/drivers/net/pci-skeleton.c
>@@ -1793,7 +1793,7 @@ static void netdrv_set_rx_mode(struct net_device *dev)
> 	struct netdrv_private *tp = netdev_priv(dev);
> 	void *ioaddr = tp->mmio_addr;
> 	u32 mc_filter[2];	/* Multicast hash filter */
>-	int i, rx_mode;
>+	int rx_mode;
> 	u32 tmp;
> 
> 	DPRINTK("ENTER\n");
>@@ -1814,10 +1814,10 @@ static void netdrv_set_rx_mode(struct net_device *dev)
> 		mc_filter[1] = mc_filter[0] = 0xffffffff;
> 	} else {
> 		struct dev_mc_list *mclist;
>+
> 		rx_mode = AcceptBroadcast | AcceptMulticast | AcceptMyPhys;
> 		mc_filter[1] = mc_filter[0] = 0;
>-		for (i = 0, mclist = dev->mc_list; mclist && i < netdev_mc_count(dev);
>-		     i++, mclist = mclist->next) {
>+		netdev_for_each_mc_addr(mclist, dev) {
> 			int bit_nr = ether_crc(ETH_ALEN, mclist->dmi_addr) >> 26;
> 
> 			mc_filter[bit_nr >> 5] |= 1 << (bit_nr & 31);
>diff --git a/drivers/net/pcnet32.c b/drivers/net/pcnet32.c
>index 63e0315..084d78d 100644
>--- a/drivers/net/pcnet32.c
>+++ b/drivers/net/pcnet32.c
>@@ -2590,7 +2590,7 @@ static void pcnet32_load_multicast(struct net_device *dev)
> 	struct pcnet32_private *lp = netdev_priv(dev);
> 	volatile struct pcnet32_init_block *ib = lp->init_block;
> 	volatile __le16 *mcast_table = (__le16 *)ib->filter;
>-	struct dev_mc_list *dmi = dev->mc_list;
>+	struct dev_mc_list *dmi;
> 	unsigned long ioaddr = dev->base_addr;
> 	char *addrs;
> 	int i;
>@@ -2611,9 +2611,8 @@ static void pcnet32_load_multicast(struct net_device *dev)
> 	ib->filter[1] = 0;
> 
> 	/* Add addresses */
>-	for (i = 0; i < netdev_mc_count(dev); i++) {
>+	netdev_for_each_mc_addr(dmi, dev) {
> 		addrs = dmi->dmi_addr;
>-		dmi = dmi->next;
> 
> 		/* multicast address? */
> 		if (!(*addrs & 1))
>diff --git a/drivers/net/ps3_gelic_net.c b/drivers/net/ps3_gelic_net.c
>index c19dd4a..a849f6f 100644
>--- a/drivers/net/ps3_gelic_net.c
>+++ b/drivers/net/ps3_gelic_net.c
>@@ -580,7 +580,7 @@ void gelic_net_set_multi(struct net_device *netdev)
> 	}
> 
> 	/* set multicast addresses */
>-	for (mc = netdev->mc_list; mc; mc = mc->next) {
>+	netdev_for_each_mc_addr(mc, netdev) {
> 		addr = 0;
> 		p = mc->dmi_addr;
> 		for (i = 0; i < ETH_ALEN; i++) {
>diff --git a/drivers/net/qlcnic/qlcnic_hw.c b/drivers/net/qlcnic/qlcnic_hw.c
>index 8ea7f86..99a4d13 100644
>--- a/drivers/net/qlcnic/qlcnic_hw.c
>+++ b/drivers/net/qlcnic/qlcnic_hw.c
>@@ -453,8 +453,7 @@ void qlcnic_set_multi(struct net_device *netdev)
> 	}
> 
> 	if (!netdev_mc_empty(netdev)) {
>-		for (mc_ptr = netdev->mc_list; mc_ptr;
>-				     mc_ptr = mc_ptr->next) {
>+		netdev_for_each_mc_addr(mc_ptr, netdev) {
> 			qlcnic_nic_add_mac(adapter, mc_ptr->dmi_addr,
> 							&del_list);
> 		}
>diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
>index c170349..c26ec5d 100644
>--- a/drivers/net/qlge/qlge_main.c
>+++ b/drivers/net/qlge/qlge_main.c
>@@ -4270,8 +4270,8 @@ static void qlge_set_multicast_list(struct net_device *ndev)
> 		status = ql_sem_spinlock(qdev, SEM_MAC_ADDR_MASK);
> 		if (status)
> 			goto exit;
>-		for (i = 0, mc_ptr = ndev->mc_list; mc_ptr;
>-		     i++, mc_ptr = mc_ptr->next)
>+		i = 0;
>+		netdev_for_each_mc_addr(mc_ptr, ndev) {
> 			if (ql_set_mac_addr_reg(qdev, (u8 *) mc_ptr->dmi_addr,
> 						MAC_ADDR_TYPE_MULTI_MAC, i)) {
> 				netif_err(qdev, hw, qdev->ndev,
>@@ -4279,6 +4279,8 @@ static void qlge_set_multicast_list(struct net_device *ndev)
> 				ql_sem_unlock(qdev, SEM_MAC_ADDR_MASK);
> 				goto exit;
> 			}
>+			i++;
>+		}
> 		ql_sem_unlock(qdev, SEM_MAC_ADDR_MASK);
> 		if (ql_set_routing_reg
> 		    (qdev, RT_IDX_MCAST_MATCH_SLOT, RT_IDX_MCAST_MATCH, 1)) {
>diff --git a/drivers/net/r6040.c b/drivers/net/r6040.c
>index b810342..15d5373 100644
>--- a/drivers/net/r6040.c
>+++ b/drivers/net/r6040.c
>@@ -938,7 +938,7 @@ static void r6040_multicast_list(struct net_device *dev)
> 	u16 *adrp;
> 	u16 reg;
> 	unsigned long flags;
>-	struct dev_mc_list *dmi = dev->mc_list;
>+	struct dev_mc_list *dmi;
> 	int i;
> 
> 	/* MAC Address */
>@@ -973,11 +973,9 @@ static void r6040_multicast_list(struct net_device *dev)
> 		for (i = 0; i < 4; i++)
> 			hash_table[i] = 0;
> 
>-		for (i = 0; i < netdev_mc_count(dev); i++) {
>+		netdev_for_each_mc_addr(dmi, dev) {
> 			char *addrs = dmi->dmi_addr;
> 
>-			dmi = dmi->next;
>-
> 			if (!(*addrs & 1))
> 				continue;
> 
>@@ -995,17 +993,19 @@ static void r6040_multicast_list(struct net_device *dev)
> 		iowrite16(hash_table[3], ioaddr + MAR3);
> 	}
> 	/* Multicast Address 1~4 case */
>-	for (i = 0, dmi; (i < netdev_mc_count(dev)) && (i < MCAST_MAX); i++) {
>-		adrp = (u16 *)dmi->dmi_addr;
>-		iowrite16(adrp[0], ioaddr + MID_1L + 8*i);
>-		iowrite16(adrp[1], ioaddr + MID_1M + 8*i);
>-		iowrite16(adrp[2], ioaddr + MID_1H + 8*i);
>-		dmi = dmi->next;
>-	}
>-	for (i = netdev_mc_count(dev); i < MCAST_MAX; i++) {
>-		iowrite16(0xffff, ioaddr + MID_0L + 8*i);
>-		iowrite16(0xffff, ioaddr + MID_0M + 8*i);
>-		iowrite16(0xffff, ioaddr + MID_0H + 8*i);
>+	i = 0;
>+	netdev_for_each_mc_addr(dmi, dev) {
>+		if (i < MCAST_MAX) {
>+			adrp = (u16 *) dmi->dmi_addr;
>+			iowrite16(adrp[0], ioaddr + MID_1L + 8 * i);
>+			iowrite16(adrp[1], ioaddr + MID_1M + 8 * i);
>+			iowrite16(adrp[2], ioaddr + MID_1H + 8 * i);
>+		} else {
>+			iowrite16(0xffff, ioaddr + MID_0L + 8 * i);
>+			iowrite16(0xffff, ioaddr + MID_0M + 8 * i);
>+			iowrite16(0xffff, ioaddr + MID_0H + 8 * i);
>+		}
>+		i++;
> 	}
> }
> 
>diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
>index 83965ee..dfc3573 100644
>--- a/drivers/net/r8169.c
>+++ b/drivers/net/r8169.c
>@@ -4732,12 +4732,10 @@ static void rtl_set_rx_mode(struct net_device *dev)
> 		mc_filter[1] = mc_filter[0] = 0xffffffff;
> 	} else {
> 		struct dev_mc_list *mclist;
>-		unsigned int i;
> 
> 		rx_mode = AcceptBroadcast | AcceptMyPhys;
> 		mc_filter[1] = mc_filter[0] = 0;
>-		for (i = 0, mclist = dev->mc_list; mclist && i < netdev_mc_count(dev);
>-		     i++, mclist = mclist->next) {
>+		netdev_for_each_mc_addr(mclist, dev) {
> 			int bit_nr = ether_crc(ETH_ALEN, mclist->dmi_addr) >> 26;
> 			mc_filter[bit_nr >> 5] |= 1 << (bit_nr & 31);
> 			rx_mode |= AcceptMulticast;
>-- 
>1.6.6
>

^ permalink raw reply

* Re: [PATCH net-next 6/7] drivers/net/e1000e: Use pr_<level> and netdev_<level>
From: Allan, Bruce W @ 2010-02-23 19:17 UTC (permalink / raw)
  To: Joe Perches, David Miller
  Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
	Brandeburg, Jesse, linux-kernel@vger.kernel.org, Ronciak, John,
	Kirsher, Jeffrey T
In-Reply-To: <78b154eab5d37da22dda3bf3e77fe32013009fa3.1266893508.git.joe@perches.com>

On Monday, February 22, 2010 6:57 PM, Joe Perches wrote:
> Convert e_<level> to netdev_<level>
> Remove #define PFX
> Use #define pr_fmt
> Convert a few printks to pr_<level>
> Coalesce long formats
> Typo spelling fix
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  drivers/net/e1000e/82571.c   |   57 +++++++++++-------
>  drivers/net/e1000e/e1000.h   |   21 -------
>  drivers/net/e1000e/es2lan.c  |   27 +++++----
>  drivers/net/e1000e/ethtool.c |   37 ++++++------
>  drivers/net/e1000e/ich8lan.c |   90 +++++++++++++++++------------
>  drivers/net/e1000e/lib.c     |  131
>  +++++++++++++++++++++---------------------
>  drivers/net/e1000e/netdev.c  |   95 +++++++++++++++---------------
>  drivers/net/e1000e/param.c   |   20 +++--- drivers/net/e1000e/phy.c 
>  |  114 ++++++++++++++++++------------------ 9 files changed, 302
> insertions(+), 290 deletions(-) 
> 

As an alternative to Joe's large patch to e1000e, I would like to suggest the following much less intrusive patch.  Compile-tested only.
  
Convert e_<level> to netdev_<level>
Use #define pr_fmt
Convert a few printks to pr_<level>

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
---
 drivers/net/e1000e/e1000.h  |   19 +++++--------------
 drivers/net/e1000e/netdev.c |    9 +++++----
 2 files changed, 10 insertions(+), 18 deletions(-)

diff --git a/drivers/net/e1000e/e1000.h b/drivers/net/e1000e/e1000.h
index c2ec095..ecd817f 100644
--- a/drivers/net/e1000e/e1000.h
+++ b/drivers/net/e1000e/e1000.h
@@ -42,25 +42,16 @@
 
 struct e1000_info;
 
-#define e_printk(level, adapter, format, arg...) \
-	printk(level "%s: %s: " format, pci_name(adapter->pdev), \
-	       adapter->netdev->name, ## arg)
-
-#ifdef DEBUG
 #define e_dbg(format, arg...) \
-	e_printk(KERN_DEBUG , hw->adapter, format, ## arg)
-#else
-#define e_dbg(format, arg...) do { (void)(hw); } while (0)
-#endif
-
+	netdev_dbg(hw->adapter->netdev, format, ## arg)
 #define e_err(format, arg...) \
-	e_printk(KERN_ERR, adapter, format, ## arg)
+	netdev_err(adapter->netdev, format, ## arg)
 #define e_info(format, arg...) \
-	e_printk(KERN_INFO, adapter, format, ## arg)
+	netdev_info(adapter->netdev, format, ## arg)
 #define e_warn(format, arg...) \
-	e_printk(KERN_WARNING, adapter, format, ## arg)
+	netdev_warn(adapter->netdev, format, ## arg)
 #define e_notice(format, arg...) \
-	e_printk(KERN_NOTICE, adapter, format, ## arg)
+	netdev_notice(adapter->netdev, format, ## arg)
 
 
 /* Interrupt modes, as used by the IntMode parameter */
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 88d54d3..d83c3cf 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -26,6 +26,8 @@
 
 *******************************************************************************/
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/module.h>
 #include <linux/types.h>
 #include <linux/init.h>
@@ -5403,10 +5405,9 @@ static struct pci_driver e1000_driver = {
 static int __init e1000_init_module(void)
 {
 	int ret;
-	printk(KERN_INFO "%s: Intel(R) PRO/1000 Network Driver - %s\n",
-	       e1000e_driver_name, e1000e_driver_version);
-	printk(KERN_INFO "%s: Copyright (c) 1999 - 2009 Intel Corporation.\n",
-	       e1000e_driver_name);
+	pr_info("Intel(R) PRO/1000 Network Driver - %s\n",
+	       e1000e_driver_version);
+	pr_info("Copyright (c) 1999 - 2009 Intel Corporation.\n");
 	ret = pci_register_driver(&e1000_driver);
 
 	return ret;

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply related

* [net-next-2.6 PATCH] net: convert multiple drivers to use netdev_for_each_mc_addr, part5 V2
From: Jiri Pirko @ 2010-02-23 19:19 UTC (permalink / raw)
  To: netdev; +Cc: davem

this time without octeon - I'll leave that for later.
------

removed some needless checks and also corrected bug in lp486e (dmi was passed
instead of dmi->dmi_addr)

Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
 drivers/net/jme.c                  |    6 +-----
 drivers/net/korina.c               |    6 ++----
 drivers/net/ks8851.c               |    5 ++---
 drivers/net/ks8851_mll.c           |    3 ++-
 drivers/net/ksz884x.c              |    4 ++--
 drivers/net/lib82596.c             |    7 ++++---
 drivers/net/lib8390.c              |   15 ++++-----------
 drivers/net/ll_temac_main.c        |    7 ++++---
 drivers/net/lp486e.c               |    4 ++--
 drivers/net/mac89x0.c              |    4 +---
 drivers/net/macb.c                 |    7 ++-----
 drivers/net/mace.c                 |   11 +++++------
 drivers/net/macmace.c              |   12 ++++++------
 drivers/net/mv643xx_eth.c          |    2 +-
 drivers/net/myri10ge/myri10ge.c    |    2 +-
 drivers/net/natsemi.c              |    4 ++--
 drivers/net/netxen/netxen_nic_hw.c |   19 +++++++------------
 drivers/net/ni5010.c               |    3 ++-
 drivers/net/ni52.c                 |    8 ++++----
 drivers/net/niu.c                  |    2 +-
 drivers/net/pci-skeleton.c         |    6 +++---
 drivers/net/pcnet32.c              |    5 ++---
 drivers/net/ps3_gelic_net.c        |    2 +-
 drivers/net/qlcnic/qlcnic_hw.c     |    3 +--
 drivers/net/qlge/qlge_main.c       |    6 ++++--
 drivers/net/r6040.c                |   30 +++++++++++++++---------------
 drivers/net/r8169.c                |    4 +---
 27 files changed, 82 insertions(+), 105 deletions(-)

diff --git a/drivers/net/jme.c b/drivers/net/jme.c
index 558b6a0..0f31497 100644
--- a/drivers/net/jme.c
+++ b/drivers/net/jme.c
@@ -1997,7 +1997,6 @@ jme_set_multi(struct net_device *netdev)
 {
 	struct jme_adapter *jme = netdev_priv(netdev);
 	u32 mc_hash[2] = {};
-	int i;
 
 	spin_lock_bh(&jme->rxmcs_lock);
 
@@ -2012,10 +2011,7 @@ jme_set_multi(struct net_device *netdev)
 		int bit_nr;
 
 		jme->reg_rxmcs |= RXMCS_MULFRAME | RXMCS_MULFILTERED;
-		for (i = 0, mclist = netdev->mc_list;
-			mclist && i < netdev_mc_count(netdev);
-			++i, mclist = mclist->next) {
-
+		netdev_for_each_mc_addr(mclist, netdev) {
 			bit_nr = ether_crc(ETH_ALEN, mclist->dmi_addr) & 0x3F;
 			mc_hash[bit_nr >> 5] |= 1 << (bit_nr & 0x1F);
 		}
diff --git a/drivers/net/korina.c b/drivers/net/korina.c
index af0c764..300c224 100644
--- a/drivers/net/korina.c
+++ b/drivers/net/korina.c
@@ -482,7 +482,7 @@ static void korina_multicast_list(struct net_device *dev)
 {
 	struct korina_private *lp = netdev_priv(dev);
 	unsigned long flags;
-	struct dev_mc_list *dmi = dev->mc_list;
+	struct dev_mc_list *dmi;
 	u32 recognise = ETH_ARC_AB;	/* always accept broadcasts */
 	int i;
 
@@ -502,11 +502,9 @@ static void korina_multicast_list(struct net_device *dev)
 		for (i = 0; i < 4; i++)
 			hash_table[i] = 0;
 
-		for (i = 0; i < netdev_mc_count(dev); i++) {
+		netdev_for_each_mc_addr(dmi, dev) {
 			char *addrs = dmi->dmi_addr;
 
-			dmi = dmi->next;
-
 			if (!(*addrs & 1))
 				continue;
 
diff --git a/drivers/net/ks8851.c b/drivers/net/ks8851.c
index 9845ab1..b5219cc 100644
--- a/drivers/net/ks8851.c
+++ b/drivers/net/ks8851.c
@@ -966,13 +966,12 @@ static void ks8851_set_rx_mode(struct net_device *dev)
 		rxctrl.rxcr1 = (RXCR1_RXME | RXCR1_RXAE |
 				RXCR1_RXPAFMA | RXCR1_RXMAFMA);
 	} else if (dev->flags & IFF_MULTICAST && !netdev_mc_empty(dev)) {
-		struct dev_mc_list *mcptr = dev->mc_list;
+		struct dev_mc_list *mcptr;
 		u32 crc;
-		int i;
 
 		/* accept some multicast */
 
-		for (i = netdev_mc_count(dev); i > 0; i--) {
+		netdev_for_each_mc_addr(mcptr, dev) {
 			crc = ether_crc(ETH_ALEN, mcptr->dmi_addr);
 			crc >>= (32 - 6);  /* get top six bits */
 
diff --git a/drivers/net/ks8851_mll.c b/drivers/net/ks8851_mll.c
index ffffb38..84b0e15 100644
--- a/drivers/net/ks8851_mll.c
+++ b/drivers/net/ks8851_mll.c
@@ -1196,7 +1196,8 @@ static void ks_set_rx_mode(struct net_device *netdev)
 	if ((netdev->flags & IFF_MULTICAST) && netdev_mc_count(netdev)) {
 		if (netdev_mc_count(netdev) <= MAX_MCAST_LST) {
 			int i = 0;
-			for (ptr = netdev->mc_list; ptr; ptr = ptr->next) {
+
+			netdev_for_each_mc_addr(ptr, netdev) {
 				if (!(*ptr->dmi_addr & 1))
 					continue;
 				if (i >= MAX_MCAST_LST)
diff --git a/drivers/net/ksz884x.c b/drivers/net/ksz884x.c
index 6f187c7..7264a3e 100644
--- a/drivers/net/ksz884x.c
+++ b/drivers/net/ksz884x.c
@@ -5777,7 +5777,7 @@ static void netdev_set_rx_mode(struct net_device *dev)
 	if (hw_priv->hw.dev_count > 1)
 		return;
 
-	if ((dev->flags & IFF_MULTICAST) && dev->mc_count) {
+	if ((dev->flags & IFF_MULTICAST) && !netdev_mc_empty(dev)) {
 		int i = 0;
 
 		/* List too big to support so turn on all multicast mode. */
@@ -5790,7 +5790,7 @@ static void netdev_set_rx_mode(struct net_device *dev)
 			return;
 		}
 
-		for (mc_ptr = dev->mc_list; mc_ptr; mc_ptr = mc_ptr->next) {
+		netdev_for_each_mc_addr(mc_ptr, dev) {
 			if (!(*mc_ptr->dmi_addr & 1))
 				continue;
 			if (i >= MAX_MULTICAST_LIST)
diff --git a/drivers/net/lib82596.c b/drivers/net/lib82596.c
index 371b58b..443c39a 100644
--- a/drivers/net/lib82596.c
+++ b/drivers/net/lib82596.c
@@ -1396,15 +1396,16 @@ static void set_multicast_list(struct net_device *dev)
 		cmd->cmd.command = SWAP16(CmdMulticastList);
 		cmd->mc_cnt = SWAP16(netdev_mc_count(dev) * 6);
 		cp = cmd->mc_addrs;
-		for (dmi = dev->mc_list;
-		     cnt && dmi != NULL;
-		     dmi = dmi->next, cnt--, cp += 6) {
+		netdev_for_each_mc_addr(dmi, dev) {
+			if (!cnt--)
+				break;
 			memcpy(cp, dmi->dmi_addr, 6);
 			if (i596_debug > 1)
 				DEB(DEB_MULTI,
 				    printk(KERN_DEBUG
 					   "%s: Adding address %pM\n",
 					   dev->name, cp));
+			cp += 6;
 		}
 		DMA_WBACK_INV(dev, &dma->mc_cmd, sizeof(struct mc_cmd));
 		i596_add_cmd(dev, &cmd->cmd);
diff --git a/drivers/net/lib8390.c b/drivers/net/lib8390.c
index 57f2584..e629233 100644
--- a/drivers/net/lib8390.c
+++ b/drivers/net/lib8390.c
@@ -907,15 +907,8 @@ static inline void make_mc_bits(u8 *bits, struct net_device *dev)
 {
 	struct dev_mc_list *dmi;
 
-	for (dmi=dev->mc_list; dmi; dmi=dmi->next)
-	{
-		u32 crc;
-		if (dmi->dmi_addrlen != ETH_ALEN)
-		{
-			printk(KERN_INFO "%s: invalid multicast address length given.\n", dev->name);
-			continue;
-		}
-		crc = ether_crc(ETH_ALEN, dmi->dmi_addr);
+	netdev_for_each_mc_addr(dmi, dev) {
+		u32 crc = ether_crc(ETH_ALEN, dmi->dmi_addr);
 		/*
 		 * The 8390 uses the 6 most significant bits of the
 		 * CRC to index the multicast table.
@@ -941,7 +934,7 @@ static void do_set_multicast_list(struct net_device *dev)
 	if (!(dev->flags&(IFF_PROMISC|IFF_ALLMULTI)))
 	{
 		memset(ei_local->mcfilter, 0, 8);
-		if (dev->mc_list)
+		if (!netdev_mc_empty(dev))
 			make_mc_bits(ei_local->mcfilter, dev);
 	}
 	else
@@ -975,7 +968,7 @@ static void do_set_multicast_list(struct net_device *dev)
 
   	if(dev->flags&IFF_PROMISC)
   		ei_outb_p(E8390_RXCONFIG | 0x18, e8390_base + EN0_RXCR);
-	else if(dev->flags&IFF_ALLMULTI || dev->mc_list)
+	else if (dev->flags & IFF_ALLMULTI || !netdev_mc_addr(dev))
   		ei_outb_p(E8390_RXCONFIG | 0x08, e8390_base + EN0_RXCR);
   	else
   		ei_outb_p(E8390_RXCONFIG, e8390_base + EN0_RXCR);
diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
index e534402..a18e348 100644
--- a/drivers/net/ll_temac_main.c
+++ b/drivers/net/ll_temac_main.c
@@ -250,9 +250,10 @@ static void temac_set_multicast_list(struct net_device *ndev)
 		temac_indirect_out32(lp, XTE_AFM_OFFSET, XTE_AFM_EPPRM_MASK);
 		dev_info(&ndev->dev, "Promiscuous mode enabled.\n");
 	} else if (!netdev_mc_empty(ndev)) {
-		struct dev_mc_list *mclist = ndev->mc_list;
-		for (i = 0; mclist && i < netdev_mc_count(ndev); i++) {
+		struct dev_mc_list *mclist;
 
+		i = 0;
+		netdev_for_each_mc_addr(mclist, ndev) {
 			if (i >= MULTICAST_CAM_TABLE_NUM)
 				break;
 			multi_addr_msw = ((mclist->dmi_addr[3] << 24) |
@@ -265,7 +266,7 @@ static void temac_set_multicast_list(struct net_device *ndev)
 					  (mclist->dmi_addr[4]) | (i << 16));
 			temac_indirect_out32(lp, XTE_MAW1_OFFSET,
 					     multi_addr_lsw);
-			mclist = mclist->next;
+			i++;
 		}
 	} else {
 		val = temac_indirect_in32(lp, XTE_AFM_OFFSET);
diff --git a/drivers/net/lp486e.c b/drivers/net/lp486e.c
index b1f5d79..3e3cc04 100644
--- a/drivers/net/lp486e.c
+++ b/drivers/net/lp486e.c
@@ -1267,8 +1267,8 @@ static void set_multicast_list(struct net_device *dev) {
 		cmd->command = CmdMulticastList;
 		*((unsigned short *) (cmd + 1)) = netdev_mc_count(dev) * 6;
 		cp = ((char *)(cmd + 1))+2;
-		for (dmi = dev->mc_list; dmi != NULL; dmi = dmi->next) {
-			memcpy(cp, dmi,6);
+		netdev_for_each_mc_addr(dmi, dev) {
+			memcpy(cp, dmi->dmi_addr, 6);
 			cp += 6;
 		}
 		if (i596_debug & LOG_SRCDST)
diff --git a/drivers/net/mac89x0.c b/drivers/net/mac89x0.c
index 23b633e..c292a60 100644
--- a/drivers/net/mac89x0.c
+++ b/drivers/net/mac89x0.c
@@ -568,9 +568,7 @@ static void set_multicast_list(struct net_device *dev)
 	if(dev->flags&IFF_PROMISC)
 	{
 		lp->rx_mode = RX_ALL_ACCEPT;
-	}
-	else if((dev->flags&IFF_ALLMULTI)||dev->mc_list)
-	{
+	} else if ((dev->flags & IFF_ALLMULTI) || !netdev_mc_empty(dev)) {
 		/* The multicast-accept list is initialized to accept-all, and we
 		   rely on higher-level filtering for now. */
 		lp->rx_mode = RX_MULTCAST_ACCEPT;
diff --git a/drivers/net/macb.c b/drivers/net/macb.c
index 7a5f897..c8a18a6 100644
--- a/drivers/net/macb.c
+++ b/drivers/net/macb.c
@@ -884,15 +884,12 @@ static void macb_sethashtable(struct net_device *dev)
 {
 	struct dev_mc_list *curr;
 	unsigned long mc_filter[2];
-	unsigned int i, bitnr;
+	unsigned int bitnr;
 	struct macb *bp = netdev_priv(dev);
 
 	mc_filter[0] = mc_filter[1] = 0;
 
-	curr = dev->mc_list;
-	for (i = 0; i < netdev_mc_count(dev); i++, curr = curr->next) {
-		if (!curr) break;	/* unexpected end of list */
-
+	netdev_for_each_mc_addr(curr, dev) {
 		bitnr = hash_get_index(curr->dmi_addr);
 		mc_filter[bitnr >> 5] |= 1 << (bitnr & 31);
 	}
diff --git a/drivers/net/mace.c b/drivers/net/mace.c
index fdb0bbd..57534f0 100644
--- a/drivers/net/mace.c
+++ b/drivers/net/mace.c
@@ -588,7 +588,7 @@ static void mace_set_multicast(struct net_device *dev)
 {
     struct mace_data *mp = netdev_priv(dev);
     volatile struct mace __iomem *mb = mp->mace;
-    int i, j;
+    int i;
     u32 crc;
     unsigned long flags;
 
@@ -598,7 +598,7 @@ static void mace_set_multicast(struct net_device *dev)
 	mp->maccc |= PROM;
     } else {
 	unsigned char multicast_filter[8];
-	struct dev_mc_list *dmi = dev->mc_list;
+	struct dev_mc_list *dmi;
 
 	if (dev->flags & IFF_ALLMULTI) {
 	    for (i = 0; i < 8; i++)
@@ -606,11 +606,10 @@ static void mace_set_multicast(struct net_device *dev)
 	} else {
 	    for (i = 0; i < 8; i++)
 		multicast_filter[i] = 0;
-	    for (i = 0; i < netdev_mc_count(dev); i++) {
+	    netdev_for_each_mc_addr(dmi, dev) {
 	        crc = ether_crc_le(6, dmi->dmi_addr);
-		j = crc >> 26;	/* bit number in multicast_filter */
-		multicast_filter[j >> 3] |= 1 << (j & 7);
-		dmi = dmi->next;
+		i = crc >> 26;	/* bit number in multicast_filter */
+		multicast_filter[i >> 3] |= 1 << (i & 7);
 	    }
 	}
 #if 0
diff --git a/drivers/net/macmace.c b/drivers/net/macmace.c
index 740accb..4e4eac0 100644
--- a/drivers/net/macmace.c
+++ b/drivers/net/macmace.c
@@ -496,7 +496,7 @@ static void mace_set_multicast(struct net_device *dev)
 {
 	struct mace_data *mp = netdev_priv(dev);
 	volatile struct mace *mb = mp->mace;
-	int i, j;
+	int i;
 	u32 crc;
 	u8 maccc;
 	unsigned long flags;
@@ -509,7 +509,7 @@ static void mace_set_multicast(struct net_device *dev)
 		mb->maccc |= PROM;
 	} else {
 		unsigned char multicast_filter[8];
-		struct dev_mc_list *dmi = dev->mc_list;
+		struct dev_mc_list *dmi;
 
 		if (dev->flags & IFF_ALLMULTI) {
 			for (i = 0; i < 8; i++) {
@@ -518,11 +518,11 @@ static void mace_set_multicast(struct net_device *dev)
 		} else {
 			for (i = 0; i < 8; i++)
 				multicast_filter[i] = 0;
-			for (i = 0; i < netdev_mc_count(dev); i++) {
+			netdev_for_each_mc_addr(dmi, dev) {
 				crc = ether_crc_le(6, dmi->dmi_addr);
-				j = crc >> 26;	/* bit number in multicast_filter */
-				multicast_filter[j >> 3] |= 1 << (j & 7);
-				dmi = dmi->next;
+				/* bit number in multicast_filter */
+				i = crc >> 26;
+				multicast_filter[i >> 3] |= 1 << (i & 7);
 			}
 		}
 
diff --git a/drivers/net/mv643xx_eth.c b/drivers/net/mv643xx_eth.c
index 2733b0a..c97b6e4 100644
--- a/drivers/net/mv643xx_eth.c
+++ b/drivers/net/mv643xx_eth.c
@@ -1794,7 +1794,7 @@ oom:
 	memset(mc_spec, 0, 0x100);
 	memset(mc_other, 0, 0x100);
 
-	for (addr = dev->mc_list; addr != NULL; addr = addr->next) {
+	netdev_for_each_mc_addr(addr, dev) {
 		u8 *a = addr->da_addr;
 		u32 *table;
 		int entry;
diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
index c0884a9..e06fbd9 100644
--- a/drivers/net/myri10ge/myri10ge.c
+++ b/drivers/net/myri10ge/myri10ge.c
@@ -3065,7 +3065,7 @@ static void myri10ge_set_multicast_list(struct net_device *dev)
 	}
 
 	/* Walk the multicast list, and add each address */
-	for (mc_list = dev->mc_list; mc_list != NULL; mc_list = mc_list->next) {
+	netdev_for_each_mc_addr(mc_list, dev) {
 		memcpy(data, &mc_list->dmi_addr, 6);
 		cmd.data0 = ntohl(data[0]);
 		cmd.data1 = ntohl(data[1]);
diff --git a/drivers/net/natsemi.c b/drivers/net/natsemi.c
index c64e5b0..e520387 100644
--- a/drivers/net/natsemi.c
+++ b/drivers/net/natsemi.c
@@ -2495,9 +2495,9 @@ static void __set_rx_mode(struct net_device *dev)
 	} else {
 		struct dev_mc_list *mclist;
 		int i;
+
 		memset(mc_filter, 0, sizeof(mc_filter));
-		for (i = 0, mclist = dev->mc_list; mclist && i < netdev_mc_count(dev);
-			 i++, mclist = mclist->next) {
+		netdev_for_each_mc_addr(mclist, dev) {
 			int b = (ether_crc(ETH_ALEN, mclist->dmi_addr) >> 23) & 0x1ff;
 			mc_filter[b/8] |= (1 << (b & 0x07));
 		}
diff --git a/drivers/net/netxen/netxen_nic_hw.c b/drivers/net/netxen/netxen_nic_hw.c
index 25f4414..a945591 100644
--- a/drivers/net/netxen/netxen_nic_hw.c
+++ b/drivers/net/netxen/netxen_nic_hw.c
@@ -539,7 +539,7 @@ void netxen_p2_nic_set_multi(struct net_device *netdev)
 	struct netxen_adapter *adapter = netdev_priv(netdev);
 	struct dev_mc_list *mc_ptr;
 	u8 null_addr[6];
-	int index = 0;
+	int i;
 
 	memset(null_addr, 0, 6);
 
@@ -570,16 +570,13 @@ void netxen_p2_nic_set_multi(struct net_device *netdev)
 
 	netxen_nic_enable_mcast_filter(adapter);
 
-	for (mc_ptr = netdev->mc_list; mc_ptr; mc_ptr = mc_ptr->next, index++)
-		netxen_nic_set_mcast_addr(adapter, index, mc_ptr->dmi_addr);
-
-	if (index != netdev_mc_count(netdev))
-		printk(KERN_WARNING "%s: %s multicast address count mismatch\n",
-			netxen_nic_driver_name, netdev->name);
+	i = 0;
+	netdev_for_each_mc_addr(mc_ptr, netdev)
+		netxen_nic_set_mcast_addr(adapter, i++, mc_ptr->dmi_addr);
 
 	/* Clear out remaining addresses */
-	for (; index < adapter->max_mc_count; index++)
-		netxen_nic_set_mcast_addr(adapter, index, null_addr);
+	while (i < adapter->max_mc_count)
+		netxen_nic_set_mcast_addr(adapter, i++, null_addr);
 }
 
 static int
@@ -710,10 +707,8 @@ void netxen_p3_nic_set_multi(struct net_device *netdev)
 	}
 
 	if (!netdev_mc_empty(netdev)) {
-		for (mc_ptr = netdev->mc_list; mc_ptr;
-		     mc_ptr = mc_ptr->next) {
+		netdev_for_each_mc_addr(mc_ptr, netdev)
 			nx_p3_nic_add_mac(adapter, mc_ptr->dmi_addr, &del_list);
-		}
 	}
 
 send_fw_cmd:
diff --git a/drivers/net/ni5010.c b/drivers/net/ni5010.c
index 6a87d81..c16cbfb 100644
--- a/drivers/net/ni5010.c
+++ b/drivers/net/ni5010.c
@@ -651,7 +651,8 @@ static void ni5010_set_multicast_list(struct net_device *dev)
 
 	PRINTK2((KERN_DEBUG "%s: entering set_multicast_list\n", dev->name));
 
-	if (dev->flags&IFF_PROMISC || dev->flags&IFF_ALLMULTI || dev->mc_list) {
+	if (dev->flags & IFF_PROMISC || dev->flags & IFF_ALLMULTI ||
+	    !netdev_mc_empty(dev)) {
 		outb(RMD_PROMISC, EDLC_RMODE); /* Enable promiscuous mode */
 		PRINTK((KERN_DEBUG "%s: Entering promiscuous mode\n", dev->name));
 	} else {
diff --git a/drivers/net/ni52.c b/drivers/net/ni52.c
index 497c6d5..05c29c2 100644
--- a/drivers/net/ni52.c
+++ b/drivers/net/ni52.c
@@ -596,7 +596,7 @@ static int init586(struct net_device *dev)
 	struct iasetup_cmd_struct __iomem *ias_cmd;
 	struct tdr_cmd_struct __iomem *tdr_cmd;
 	struct mcsetup_cmd_struct __iomem *mc_cmd;
-	struct dev_mc_list *dmi = dev->mc_list;
+	struct dev_mc_list *dmi;
 	int num_addrs = netdev_mc_count(dev);
 
 	ptr = p->scb + 1;
@@ -724,9 +724,9 @@ static int init586(struct net_device *dev)
 		writew(0xffff, &mc_cmd->cmd_link);
 		writew(num_addrs * 6, &mc_cmd->mc_cnt);
 
-		for (i = 0; i < num_addrs; i++, dmi = dmi->next)
-			memcpy_toio(mc_cmd->mc_list[i],
-							dmi->dmi_addr, 6);
+		i = 0;
+		netdev_for_each_mc_addr(dmi, dev)
+			memcpy_toio(mc_cmd->mc_list[i++], dmi->dmi_addr, 6);
 
 		writew(make16(mc_cmd), &p->scb->cbl_offset);
 		writeb(CUC_START, &p->scb->cmd_cuc);
diff --git a/drivers/net/niu.c b/drivers/net/niu.c
index 5e604e3..0678f31 100644
--- a/drivers/net/niu.c
+++ b/drivers/net/niu.c
@@ -6365,7 +6365,7 @@ static void niu_set_rx_mode(struct net_device *dev)
 		for (i = 0; i < 16; i++)
 			hash[i] = 0xffff;
 	} else if (!netdev_mc_empty(dev)) {
-		for (addr = dev->mc_list; addr; addr = addr->next) {
+		netdev_for_each_mc_addr(addr, dev) {
 			u32 crc = ether_crc_le(ETH_ALEN, addr->da_addr);
 
 			crc >>= 24;
diff --git a/drivers/net/pci-skeleton.c b/drivers/net/pci-skeleton.c
index 11d4398..3678585 100644
--- a/drivers/net/pci-skeleton.c
+++ b/drivers/net/pci-skeleton.c
@@ -1793,7 +1793,7 @@ static void netdrv_set_rx_mode(struct net_device *dev)
 	struct netdrv_private *tp = netdev_priv(dev);
 	void *ioaddr = tp->mmio_addr;
 	u32 mc_filter[2];	/* Multicast hash filter */
-	int i, rx_mode;
+	int rx_mode;
 	u32 tmp;
 
 	DPRINTK("ENTER\n");
@@ -1814,10 +1814,10 @@ static void netdrv_set_rx_mode(struct net_device *dev)
 		mc_filter[1] = mc_filter[0] = 0xffffffff;
 	} else {
 		struct dev_mc_list *mclist;
+
 		rx_mode = AcceptBroadcast | AcceptMulticast | AcceptMyPhys;
 		mc_filter[1] = mc_filter[0] = 0;
-		for (i = 0, mclist = dev->mc_list; mclist && i < netdev_mc_count(dev);
-		     i++, mclist = mclist->next) {
+		netdev_for_each_mc_addr(mclist, dev) {
 			int bit_nr = ether_crc(ETH_ALEN, mclist->dmi_addr) >> 26;
 
 			mc_filter[bit_nr >> 5] |= 1 << (bit_nr & 31);
diff --git a/drivers/net/pcnet32.c b/drivers/net/pcnet32.c
index 63e0315..084d78d 100644
--- a/drivers/net/pcnet32.c
+++ b/drivers/net/pcnet32.c
@@ -2590,7 +2590,7 @@ static void pcnet32_load_multicast(struct net_device *dev)
 	struct pcnet32_private *lp = netdev_priv(dev);
 	volatile struct pcnet32_init_block *ib = lp->init_block;
 	volatile __le16 *mcast_table = (__le16 *)ib->filter;
-	struct dev_mc_list *dmi = dev->mc_list;
+	struct dev_mc_list *dmi;
 	unsigned long ioaddr = dev->base_addr;
 	char *addrs;
 	int i;
@@ -2611,9 +2611,8 @@ static void pcnet32_load_multicast(struct net_device *dev)
 	ib->filter[1] = 0;
 
 	/* Add addresses */
-	for (i = 0; i < netdev_mc_count(dev); i++) {
+	netdev_for_each_mc_addr(dmi, dev) {
 		addrs = dmi->dmi_addr;
-		dmi = dmi->next;
 
 		/* multicast address? */
 		if (!(*addrs & 1))
diff --git a/drivers/net/ps3_gelic_net.c b/drivers/net/ps3_gelic_net.c
index c19dd4a..a849f6f 100644
--- a/drivers/net/ps3_gelic_net.c
+++ b/drivers/net/ps3_gelic_net.c
@@ -580,7 +580,7 @@ void gelic_net_set_multi(struct net_device *netdev)
 	}
 
 	/* set multicast addresses */
-	for (mc = netdev->mc_list; mc; mc = mc->next) {
+	netdev_for_each_mc_addr(mc, netdev) {
 		addr = 0;
 		p = mc->dmi_addr;
 		for (i = 0; i < ETH_ALEN; i++) {
diff --git a/drivers/net/qlcnic/qlcnic_hw.c b/drivers/net/qlcnic/qlcnic_hw.c
index 8ea7f86..99a4d13 100644
--- a/drivers/net/qlcnic/qlcnic_hw.c
+++ b/drivers/net/qlcnic/qlcnic_hw.c
@@ -453,8 +453,7 @@ void qlcnic_set_multi(struct net_device *netdev)
 	}
 
 	if (!netdev_mc_empty(netdev)) {
-		for (mc_ptr = netdev->mc_list; mc_ptr;
-				     mc_ptr = mc_ptr->next) {
+		netdev_for_each_mc_addr(mc_ptr, netdev) {
 			qlcnic_nic_add_mac(adapter, mc_ptr->dmi_addr,
 							&del_list);
 		}
diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index c170349..c26ec5d 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -4270,8 +4270,8 @@ static void qlge_set_multicast_list(struct net_device *ndev)
 		status = ql_sem_spinlock(qdev, SEM_MAC_ADDR_MASK);
 		if (status)
 			goto exit;
-		for (i = 0, mc_ptr = ndev->mc_list; mc_ptr;
-		     i++, mc_ptr = mc_ptr->next)
+		i = 0;
+		netdev_for_each_mc_addr(mc_ptr, ndev) {
 			if (ql_set_mac_addr_reg(qdev, (u8 *) mc_ptr->dmi_addr,
 						MAC_ADDR_TYPE_MULTI_MAC, i)) {
 				netif_err(qdev, hw, qdev->ndev,
@@ -4279,6 +4279,8 @@ static void qlge_set_multicast_list(struct net_device *ndev)
 				ql_sem_unlock(qdev, SEM_MAC_ADDR_MASK);
 				goto exit;
 			}
+			i++;
+		}
 		ql_sem_unlock(qdev, SEM_MAC_ADDR_MASK);
 		if (ql_set_routing_reg
 		    (qdev, RT_IDX_MCAST_MATCH_SLOT, RT_IDX_MCAST_MATCH, 1)) {
diff --git a/drivers/net/r6040.c b/drivers/net/r6040.c
index b810342..15d5373 100644
--- a/drivers/net/r6040.c
+++ b/drivers/net/r6040.c
@@ -938,7 +938,7 @@ static void r6040_multicast_list(struct net_device *dev)
 	u16 *adrp;
 	u16 reg;
 	unsigned long flags;
-	struct dev_mc_list *dmi = dev->mc_list;
+	struct dev_mc_list *dmi;
 	int i;
 
 	/* MAC Address */
@@ -973,11 +973,9 @@ static void r6040_multicast_list(struct net_device *dev)
 		for (i = 0; i < 4; i++)
 			hash_table[i] = 0;
 
-		for (i = 0; i < netdev_mc_count(dev); i++) {
+		netdev_for_each_mc_addr(dmi, dev) {
 			char *addrs = dmi->dmi_addr;
 
-			dmi = dmi->next;
-
 			if (!(*addrs & 1))
 				continue;
 
@@ -995,17 +993,19 @@ static void r6040_multicast_list(struct net_device *dev)
 		iowrite16(hash_table[3], ioaddr + MAR3);
 	}
 	/* Multicast Address 1~4 case */
-	for (i = 0, dmi; (i < netdev_mc_count(dev)) && (i < MCAST_MAX); i++) {
-		adrp = (u16 *)dmi->dmi_addr;
-		iowrite16(adrp[0], ioaddr + MID_1L + 8*i);
-		iowrite16(adrp[1], ioaddr + MID_1M + 8*i);
-		iowrite16(adrp[2], ioaddr + MID_1H + 8*i);
-		dmi = dmi->next;
-	}
-	for (i = netdev_mc_count(dev); i < MCAST_MAX; i++) {
-		iowrite16(0xffff, ioaddr + MID_0L + 8*i);
-		iowrite16(0xffff, ioaddr + MID_0M + 8*i);
-		iowrite16(0xffff, ioaddr + MID_0H + 8*i);
+	i = 0;
+	netdev_for_each_mc_addr(dmi, dev) {
+		if (i < MCAST_MAX) {
+			adrp = (u16 *) dmi->dmi_addr;
+			iowrite16(adrp[0], ioaddr + MID_1L + 8 * i);
+			iowrite16(adrp[1], ioaddr + MID_1M + 8 * i);
+			iowrite16(adrp[2], ioaddr + MID_1H + 8 * i);
+		} else {
+			iowrite16(0xffff, ioaddr + MID_0L + 8 * i);
+			iowrite16(0xffff, ioaddr + MID_0M + 8 * i);
+			iowrite16(0xffff, ioaddr + MID_0H + 8 * i);
+		}
+		i++;
 	}
 }
 
diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 83965ee..dfc3573 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -4732,12 +4732,10 @@ static void rtl_set_rx_mode(struct net_device *dev)
 		mc_filter[1] = mc_filter[0] = 0xffffffff;
 	} else {
 		struct dev_mc_list *mclist;
-		unsigned int i;
 
 		rx_mode = AcceptBroadcast | AcceptMyPhys;
 		mc_filter[1] = mc_filter[0] = 0;
-		for (i = 0, mclist = dev->mc_list; mclist && i < netdev_mc_count(dev);
-		     i++, mclist = mclist->next) {
+		netdev_for_each_mc_addr(mclist, dev) {
 			int bit_nr = ether_crc(ETH_ALEN, mclist->dmi_addr) >> 26;
 			mc_filter[bit_nr >> 5] |= 1 << (bit_nr & 31);
 			rx_mode |= AcceptMulticast;
-- 
1.6.6


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox