On Tue, Dec 18, 2001 at 11:44:14AM +1100, David Gibson wrote:
>
> > What I like about the current setup is the constant cost. If I
> > double the number of ioctl, the code stays exactly the same. It also
> > mean that people can just yank a new wireless.h in their kernel and
> > expect it to work.
>
> Except that the source of every driver needs to change to expand the
> standard handler table. With structures using the gcc structure
> initialization extension (as used all over the kernel) only a
> recompile is needed.

        Not true. The old driver would work fine even not
recompiled. I was very careful about that.

> It's a nop in the .o, but both breaking the union and pulling the
> various items out (rather than having them as separate parameters)
> makes the source uglier.

        Just do :
----------------------------------------------------------------
static int orinoco_ioctl_getwap(struct net_device *dev,
                                struct iw_request_info *info,
                                struct sockaddr *arq,
                                char *extra)
{
        struct orinoco_private *priv = dev->priv;

        arq->sa_family = ARPHRD_ETHER;
        return orinoco_hw_get_bssid(priv, arq->sa_data);
}
[...]
static const iw_handler orinoco_handler[] =
{
[...]
        (iw_handler) orinoco_ioctl_getwap, /* SIOCGIWAP */
----------------------------------------------------------------

        Satisfied ?

> > Also : not all driver check/set all the fields of the
> > structure. For example, the old Wavelan doesn't use key index and key
> > flags, whereas the Orinoco does. By expanding things too much in the
> > kernel you would make those drivers that use only a few things pay a
> > price.
>
> I don't see that it's a price of any significance. ioctl() is not
> critical path.

        Bloat. Especially static bloat that is there even for people
who don't use it.

> > The only issue is if people put the iw_handler in the wrong
> > slot. Your proposal would detect it. But people would realise that
> > pretty quicly (I try to set the encryption, and the frequency changes
> > - how bizzare !).
>
> Or it SEGVs or oopses because it tries to copy_from_user() when
> actually passed an iw_param.

        Nope. The copy_to/from_user is only between user space and the
kernel wrapper. The driver doesn't see the pointer anymore, so doesn't
get a chance to mess with it.

> > And best, you can mix/matches the two approaches, which gives
> > you maximum flexibility. If you look, this is what I have done for the
> > force_reset and card_reset which share the same iw_handler (of course,
> > you won't like it). I think it's a cool feature of the API, and I want
> > to keep it.
>
> Nifty, but not a decisive advantage IMO>

        Well, it sure depend on your hardware.
        For example, I added the "commit" stuff. For some hardware,
it's totally useless. For some other, like the Orinoco, it makes a
*big* difference.

        Jean

• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]