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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 26 Oct 2010 15:02:10 +0100
From:	Ben Hutchings <ben@...adent.org.uk>
To:	Shawn Lin <shawn@....com.tw>
Cc:	Florian Fainelli <florian@...nwrt.org>, netdev@...r.kernel.org,
	Marc Leclerc <marc-leclerc@...naturealpha.com>,
	Albert Chen <albert.chen@....com.tw>,
	David Miller <davem@...emloft.net>
Subject: Re: [PATCH 1/2] r6040: fix multicast operations

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

Download attachment "signature.asc" of type "application/pgp-signature" (828 bytes)