Re: [PATCH 1/2] r6040: fix multicast operations

[Ben Hutchings <ben@decadent.org.uk>]

[-- Attachment #1: Type: text/plain, Size: 2236 bytes --]

On Thu, Oct 21, 2010 at 04:27:34PM +0800, Shawn Lin wrote:
[...]
> > > +	/* Use internal multicast address registers
> > > +	 * if the number of multicast addresses is not greater than MCAST_MAX.
> > > +	 */
> > > +	else if (netdev_mc_empty(dev)) {
> > > +		for (i = 0; i < MCAST_MAX ; i++) {
> > > +			iowrite16(0, ioaddr + MID_1L + 8 * i);
> > > +			iowrite16(0, ioaddr + MID_1M + 8 * i);
> > > +			iowrite16(0, ioaddr + MID_1H + 8 * i);
> > > +		}
> > > +	} else if (netdev_mc_count(dev) <= MCAST_MAX) {
> > > +		i = 0;
> > > +		netdev_for_each_mc_addr(ha, dev) {
> > > +			adrp = (u16 *) ha->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);
> > > +			i++;
> > > +		}
> > 
> > What about the unused exact match entries?  And why is the empty case
> > special?
> 
> Unused exact match entries? I am not sure which entries are you
> mentioned.

If there are 1 or 2 addresses in the multicast list then some of the
exact match entries will be used and some will not.  But the loop
above does not clear the unused entries.

[...]
> 2) if (netdev_mc_count(dev) <= 3)
> There are two hardware features could be used to filter multicast
> frames:
[...]
> 3) if (netdev_mc_empty(dev))
> because we masked the multicast hash table flag before examine all
> conditions, we only need to clear the addresses in the three
> MAC/Multicast registers.
[...]

But why is this so different from the case of 1-3 addresses?  I would
write these two cases as:

else if (netdev_mc_count(dev) <= MCAST_MAX) {
	i = 0;
	netdev_for_each_mc_addr(ha, dev) {
		adrp = (u16 *) ha->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);
		i++;
	}
	while (i < MCAST_MAX) {
		iowrite16(0, ioaddr + MID_1L + 8 * i);
		iowrite16(0, ioaddr + MID_1M + 8 * i);
		iowrite16(0, ioaddr + MID_1H + 8 * i);
		i++;
	}
} 

Ben.

-- 
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
                                                              - Albert Camus

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 827 bytes --]