netdev - Re: Please pull 'adm8211' branch of wireless-2.6

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]

Message-Id: <200709160047.45128.flamingice@sourmilk.net>
Date:	Sun, 16 Sep 2007 00:47:40 -0400
From:	Michael Wu <flamingice@...rmilk.net>
To:	Jeff Garzik <jeff@...zik.org>
Cc:	David Miller <davem@...emloft.net>,
	"John W. Linville" <linville@...driver.com>,
	netdev@...r.kernel.org, linux-wireless@...r.kernel.org
Subject: Re: Please pull 'adm8211' branch of wireless-2.6

On Saturday 15 September 2007 20:56, Jeff Garzik wrote:
> >>> +	if (flags & IFF_PROMISC)
> >>> +		dev->flags |= IEEE80211_HW_RX_INCLUDES_FCS;
> >>> +	else
> >>> +		dev->flags &= ~IEEE80211_HW_RX_INCLUDES_FCS;
> >>
> >> why does promisc dictate inclusion of FCS?
> >
> > Because that's the way the hardware works.
>
> Why not always include it, regardless of promisc?
>
I really do mean that's how the hardware works. If you turn on the promisc bit 
in the hardware (which IFF_PROMISC causes), it starts including the FCS, but 
if the bit is not set, the FCS is not included in frames.

> >>> +	} while (count++ < 20);
> >>
> >> NAK -- talk about back to the past.
> >>
> >> It's bogus to loop in the interrupt handler, then loop again in both RX
> >> and TX paths.  You are getting close to reinventing the wheel here.
> >>
> >> Use NAPI rather than doing such a loop.
> >
> > This is old interrupt handler code from Jouni's original driver. I never
> > bothered to replace it with the simpler designs used in newer drivers
> > I've worked on.
> >
> > Also - mac80211 drivers have no access to NAPI.
>
> Not true as of net-2.6.24.
>
Huh? How does a driver use NAPI if it can't pass a struct net_device?

> >>> +/* CSR (Host Control and Status Registers) */
> >>> +struct adm8211_csr {
> >>
> >> [snip]
> >>
> >>> +} __attribute__ ((packed));
> >>
> >> attributed(packed) is unneccesary here, and its use results in
> >> sub-optimal code
> >
> > How? Doesn't this just turn into a bunch of offsets?
>
> It depends on how its used in the code.
>
I don't think I'm using it in a way that would make a difference.

> >> enums are preferred.  they do not disappear at the cpp stage, and confer
> >> type information.
> >
> > I'd rather not.
>
> Elaboration?
>
What form of debugging are you talking about? I don't see how it makes a 
difference for debugging. The type checking provided by enums won't make a 
difference for my code - any problems with using the wrong register bits in 
the wrong place is obvious due to the prefixes. I don't really see how enum 
type checking is even effective at all without annotations and casts all over 
the place.

Thanks,
-Michael Wu

Content of type "application/pgp-signature" skipped