On Wed, 2007-02-07 at 07:54 +0000, Christoph Hellwig wrote:
> Do we really have to add this directly to struct net_device and bloat it
> for all users? I'd put this behind ieee80211_ptr instead unless there's
> a good reason against that.
Well, the reason currently is that cfg80211 treats ieee80211_ptr as an
opaque cookie, it doesn't need to know what it points to.
Of course, I intend to change that too...
> > +config CFG80211_WEXTNL_COMPAT
> > + bool "cfg80211 WE-netlink compatibility"
> > + depends CFG80211 && CFG80211_WEXT_COMPAT
> > + ---help---
> > + This option allows using devices whose drivers have been
> > + converted to use the new cfg80211 with wireless extensions
> > + over rtnetlink, providing WE-20 compatibility. Note that
> > + cfg80211's "native" interface is nl80211 using generic netlink.
> > + The wireless extensions are being deprecated and the netlink
> > + based API for WE was never configured by default, nor do any
> > + userspace tools use this feature.
> > +
> > + This option exists only to make Jean happy. Say N.
>
> Should we really put this code in? It seems like unessecary bloat to me.
Yeah. Jean disappeared anyway, I'll kill it.
> > @@ -2798,6 +2799,39 @@ int dev_ioctl(unsigned int cmd, void __user *arg)
> > ret = -EFAULT;
> > return ret;
> > }
> > +#ifdef CONFIG_CFG80211_WEXT_COMPAT
> > + /* Take care of cfg80211 WE compatibility */
> > + if (cmd >= SIOCIWFIRST && cmd <= SIOCIWLAST) {
> > + /* If command is `set a parameter', or
> > + * `get the encoding parameters', check if
> > + * the user has the right to do it */
> > + if (IW_IS_SET(cmd) || cmd == SIOCGIWENCODE
> > + || cmd == SIOCGIWENCODEEXT) {
> > + if (!capable(CAP_NET_ADMIN))
> > + return -EPERM;
> > + }
> > + dev_load(ifr.ifr_name);
> > + rtnl_lock();
> > + /* Follow me in net/wireless/wext-compat.c */
> > + ret = cfg80211_wext_ioctl(&ifr, cmd);
> > + rtnl_unlock();
> > + if (ret == 0 && IW_IS_GET(cmd) &&
> > + copy_to_user(arg, &ifr,
> > + sizeof(struct ifreq)))
> > + ret = -EFAULT;
> > + /* haha, I cheat here by allowing a driver or
> > + * stack to have both WE or CFG80211-WE for
> > + * a little while during conversion... hope that
> > + * ENOSYS is only used to indicate not implemented
> > + *
> > + * if wireless extensions are not configured
> > + * then this is the last thing here so that
> > + * if we fall through we return -EINVAL
> > + */
> > + if (ret != -ENOSYS)
> > + return ret;
> > + }
>
> Can we split this into a nice little helper function in wext-compat.c
> or a dummy macro if it's not configured?
I guess I can do that. Maybe I'll do the same with wext too.
> > --- a/net/core/rtnetlink.c
> > +++ b/net/core/rtnetlink.c
> > @@ -56,6 +56,9 @@
> > #include <linux/wireless.h>
> > #include <net/iw_handler.h>
> > #endif /* CONFIG_NET_WIRELESS_RTNETLINK */
> > +#ifdef CONFIG_CFG80211_WEXTNL_COMPAT
> > +#include <net/cfg80211.h>
> > +#endif
>
> There's not point in including headers conditionally.
It helps ccache when you change that header, but yeah, let's just
include it.
> This looks odd. Kbuild filters out duplicate objects, so something like
> the snipplet below should work aswell:
>
> obj-$(CONFIG_WIRELESS_EXT) += wext-common.o wext-old.o
> obj-$(CONFIG_CFG80211_WEXT_COMPAT) += wext-common.o wext-compat.o
Hah, good point, I never even thought of something that would just
duplicate it.
> > +#ifdef CONFIG_CFG80211_WEXT_COMPAT
> > +/* wext compatibility must be compiled in...
> > + * this extern is in wext-compat.c */
>
> Is there really a point in making cfg80211 a loadable module?
> This whle operation vector stuff looks like a lot of unnessecary
> complexity to me.
I think there is point in doing it as it allows us to replace it with an
out-of-tree newer version for getting it into the hands of users who
might not want to replace their whole kernel.
> > new file mode 100644
> > index 0000000..c1de1d7
> > --- /dev/null
> > +++ b/net/wireless/wext-common.c
> > @@ -0,0 +1,610 @@
> > +/* common wext support routines, proc interface and events */
>
> This is mostly Jean's old code, right? Probably wants at least some
> attribution.
Yeah, good point, it's mostly code that moved from that file to here.
> Btw, if we submit cfg80211 for inclusion there should be at least one patch
> in the series actually converts a driver to use it. Both to make this
> code not dead and give people and example on how to use the cfg80211
> infrastructure.
I guess I could make softmac use it.
johannes
Attachment:
signature.asc
Description: This is a digitally signed message part
