Discussion:
please review: patches for ieee80211 ioctls
(too old to reply)
Stephen Degler
2007-08-13 15:27:58 UTC
Permalink
Hello,

The following two patches fix wpa_supplicant for me, with an atheros driver
and x86_64. The first simply adds the SIOC{G,S}80211 ioctls. The second
fixes a nasty COMPAT_ problem that depended on the size of the ioctl arguments!
On x86_64 at least, the size of the ieee80211_req struct is the same as the
struct oifreq, and I lose. See the comments in socio.h to this effect.

One question: Currently all of the wireless ioctls fall into the default case
and are handled by compat_ifioctl in the second switch statment of ifioctl
(if.c, lines 560-580). Shouln't they be in prior case, posted directly against
the interface, like SIOCGIFMEDIA? If so, let me know and I'll update the patch.

Please review and let me know if its ok to commit.

skd


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Stephen Degler
2007-08-13 15:38:50 UTC
Permalink
Ah,

And of course I forgot to attach the patches:

Here they are.

skd
Christos Zoulas
2007-08-14 07:55:03 UTC
Permalink
On Aug 13, 11:38am, ***@degler.net (Stephen Degler) wrote:
-- Subject: Re: please review: patches for ieee80211 ioctls

| This is a multi-part message in MIME format.
| --------------000304070409080100050807
| Content-Type: text/plain; charset=ISO-8859-1; format=flowed
| Content-Transfer-Encoding: 7bit
|
| Ah,
|
| And of course I forgot to attach the patches:
|
| Here they are.
|
| skd
|
|
| --------------000304070409080100050807
| Content-Type: text/x-patch;
| name="if.c.patch"
| Content-Transfer-Encoding: 7bit
| Content-Disposition: inline;
| filename="if.c.patch"
|
| Index: if.c
| ===================================================================
| RCS file: /cvsroot/src/sys/net/if.c,v
| retrieving revision 1.195
| diff -u -u -r1.195 if.c
| --- if.c 7 Aug 2007 04:14:37 -0000 1.195
| +++ if.c 13 Aug 2007 13:44:05 -0000
| @@ -1386,6 +1386,8 @@
| case SIOCDELMULTI:
| case SIOCSIFMEDIA:
| case SIOCSDRVSPEC:
| + case SIOCG80211:
| + case SIOCS80211:
| case SIOCS80211NWID:
| case SIOCS80211NWKEY:
| case SIOCS80211POWER:
| @@ -1561,6 +1563,8 @@
| break;
|
| case SIOCSDRVSPEC:
| + case SIOCG80211:
| + case SIOCS80211:
| case SIOCS80211NWID:
| case SIOCS80211NWKEY:
| case SIOCS80211POWER:
|
| --------------000304070409080100050807
| Content-Type: text/x-patch;
| name="sockio.h.patch"
| Content-Transfer-Encoding: 7bit
| Content-Disposition: inline;
| filename="sockio.h.patch"
|
| Index: sockio.h
| ===================================================================
| RCS file: /cvsroot/src/sys/compat/sys/sockio.h,v
| retrieving revision 1.3
| diff -u -u -r1.3 sockio.h
| --- sockio.h 30 May 2007 21:02:02 -0000 1.3
| +++ sockio.h 13 Aug 2007 13:46:34 -0000
| @@ -114,16 +114,28 @@
| sizeof((oi)->ifr_ifru)); \
| } while (/*CONSTCOND*/0)
|
| -/*
| - * XXX: The following macro depends on the fact that the only struct
| - * sized 0x20 bytes in the ifioctls is struct oifreq and struct ifcapreq.
| - * If that changes, then we'll need to use an explicit list here.
| - */
| -#define ifcapreq(x) ((x) == SIOCGIFCAP || (x) == SIOCSIFCAP)
| +#define ifcompatcmd(x) ((x == OSIOCSIFADDR || \
| + x == OOSIOCGIFADDR || \
| + x == OSIOCSIFDSTADDR || \
| + x == OOSIOCGIFDSTADDR || \
| + x == OSIOCSIFFLAGS || \
| + x == OSIOCGIFFLAGS || \
| + x == OOSIOCGIFBRDADDR || \
| + x == OSIOCSIFBRDADDR || \
| + x == OOSIOCGIFCONF || \
| + x == OOSIOCGIFNETMASK || \
| + x == OSIOCSIFNETMASK || \
| + x == OSIOCGIFCONF || \
| + x == OSIOCADDMULTI || \
| + x == OSIOCDELMULTI || \
| + x == OSIOCSIFMEDIA || \
| + x == OBIOCGETIF || \
| + x == OBIOCSETIF || \
| + x == OTAPGIFNAME) ? 1 : 0)

At this point is is probably worth it to make the above a function with
a case statement, because the code size is too big and the evaluation
too slow. THe rest looks fine.

christos

| #define cvtcmd(x) \
| - ((IOCPARM_LEN(x) == sizeof(struct oifreq) && !ifcapreq(x)) ? \
| - (((x) & ~(IOCPARM_MASK << IOCPARM_SHIFT)) | \
| - (sizeof(struct ifreq) << IOCPARM_SHIFT)) : (x))
| + (ifcompatcmd(x) ? \
| + (((x) & ~(IOCPARM_MASK << IOCPARM_SHIFT)) | \
| + (sizeof(struct ifreq) << IOCPARM_SHIFT)) : (x))
|
| #ifdef _KERNEL
| __BEGIN_DECLS
|
| --------------000304070409080100050807--
-- End of excerpt from Stephen Degler



--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Loading...