[PATCH] oids_by_name (was: Re: [Prism54-devel] Race in private ioctls)
Denis Vlasenko
vda@port.imtp.ilyichevsk.odessa.ua
Sun, 22 Feb 2004 19:56:31 +0200
--Boundary-00=_P1OOAp3IzQrE5Gb
Content-Type: text/plain;
charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
On Sunday 22 February 2004 18:04, Denis Vlasenko wrote:
> From Changelog:
> ==============
> 2004-02-19
> * isl_ioctl.c, islpci_dev.h: adds prism54_set_oid and prism54_get_oid.
> Users of the old LDDK/ISL3890 driver can now just use the private
> ioctl, example of how to use it provided in getoid and setoid scripts.
> patch by Feyd <feyd@seznam.cz> -- I replaced "prism54" for ndev->name.
>
> setoid:
> ======
> #!/bin/sh
> args=`echo $@ | cut -d" " -f3-`
> iwpriv $1 oid $2
> iwpriv $1 set_oid $args
>
> This is racy.
>
> I think we need different interface here, like this:
>
> # iwpriv eth1 gPROFILES
> eth1 gPROFILES:1
> # iwpriv eth1 sPROFILES 0
>
> No races, and referencing OIDs by name and not by number is
> easier for humans.
Attached patch illustrates how it would look like.
You can compare it with current get/set_oid:
# getoid eth1 $((0x11000001))
eth1 get_oid:65 1 0 0 0 0 0 0 0 0 0 0 236 83 2 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 16 0 0 0 15 0 0 0 3 0 0 0 3 0 0 0 36 19 0 160 152 147 0 0 0 0 0 0 0 0 0 0 236 83 2 0 236 84 2 0 236 85 2 0 236 86 2 0 236 87 2 0 236 88 2 0 236 89 2 0 236 90 2 0 236 91 2 0 236 92 2 0 236 93 2 0 236 94 2 0 236 95 2 0 236 96 2 0 236 97 2 0 236 98 2 0 16 0 0 0 15 0 0 0 249 1 0 0 233 1 0 0 132 19 0 160 176 147 0 0 152 147 0 0 22 0 18 1 184 137 2 0 8 23 3 0 200 37 3 0 184 78 3 0 152 115 2 0 248 122 2 0 24 145 2 0 88 189 2 0 56 167 2 0 104 30 3 0 56 108 2 0 56 226 2 0 120 211 2 0 88 130 2 0 216 218 2 0 216 159 2 0 32 0 0 0 31 0 0 0 233 1 0 0 233 1 0 0
# iwpriv eth1 gBEACONPERIOD
<mapping sub-ioctl gBEACONPERIOD to cmd 0x8BF5-14>
eth1 gBEACONPERIOD:65 1 0 0 0 0 0 0 0 0 0 0 236 83 2 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 16 0 0 0 15 0 0 0 3 0 0 0 3 0 0 0 36 19 0 160 152 147 0 0 0 0 0 0 0 0 0 0 236 83 2 0 236 84 2 0 236 85 2 0 236 86 2 0 236 87 2 0 236 88 2 0 236 89 2 0 236 90 2 0 236 91 2 0 236 92 2 0 236 93 2 0 236 94 2 0 236 95 2 0 236 96 2 0 236 97 2 0 236 98 2 0 16 0 0 0 15 0 0 0 9 2 0 0 249 1 0 0 132 19 0 160 176 147 0 0 152 147 0 0 22 0 18 1 104 30 3 0 56 108 2 0 184 137 2 0 8 23 3 0 88 130 2 0 216 218 2 0 216 159 2 0 152 115 2 0 184 78 3 0 72 8 3 0 168 15 3 0 248 240 2 0 120 152 2 0 232 0 3 0 56 226 2 0 120 211 2 0 32 0 0 0 31 0 0 0 249 1 0 0 249 1 0 0
syslog:
=======
Feb 22 19:38:39 shadow kernel: eth1: oid 0x11000001
Feb 22 19:38:39 shadow kernel: eth1: get_oid 0x11000001
Feb 22 19:38:39 shadow kernel: eth1: ret: 0
Feb 22 19:38:39 shadow kernel: eth1: response_op: 2
Feb 22 19:38:39 shadow kernel: eth1: len: 256
Feb 22 19:38:45 shadow kernel: eth1: get_oid 0x11000001
Feb 22 19:38:45 shadow kernel: eth1: ret: 0
Feb 22 19:38:45 shadow kernel: eth1: response_op: 2
Feb 22 19:38:45 shadow kernel: eth1: len: 256
Same result, but with one, raceless ioctl. ;)
PS. We can format OIDs nicely according to their size
(isl_oid[data->flags].size)
--
vda
--Boundary-00=_P1OOAp3IzQrE5Gb
Content-Type: text/x-diff;
charset="iso-8859-1";
name="oid_by_name.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="oid_by_name.patch"
--- isl_ioctl.c.orig Sun Feb 22 17:44:37 2004
+++ isl_ioctl.c Sun Feb 22 19:43:31 2004
@@ -2018,6 +2018,69 @@
return ret;
}
+int
+prism54_set_u32(struct net_device *ndev, struct iw_request_info *info,
+ struct iw_point *data, char *extra)
+{
+ islpci_private *priv = ndev->priv;
+ struct islpci_mgmtframe *response = NULL;
+ int ret = 0, response_op = PIMFOR_OP_ERROR;
+ int oid = isl_oid[data->flags].oid; /* data->flags: sub-ioctl # */
+
+ printk("%s: set_oid 0x%08X\tlen: %d\n", ndev->name, oid, data->length);
+
+ if (islpci_get_state(priv) >= PRV_STATE_INIT) {
+ ret = islpci_mgt_transaction(priv->ndev, PIMFOR_OP_SET, oid, extra, data->length, &response);
+ printk("%s: ret: %i\n", ndev->name, ret);
+ if (!ret) {
+ response_op = response->header->operation;
+ printk("%s: response_op: %i\n", ndev->name, response_op);
+ islpci_mgt_release(response);
+ }
+ if (ret || response_op == PIMFOR_OP_ERROR) {
+ printk("%s: EIO\n", ndev->name);
+ ret = -EIO;
+ }
+ }
+
+ return ret;
+}
+
+int
+prism54_get_u32(struct net_device *ndev, struct iw_request_info *info,
+ struct iw_point *data, char *extra)
+{
+ islpci_private *priv = ndev->priv;
+ struct islpci_mgmtframe *response = NULL;
+ int ret = -EIO, response_op = PIMFOR_OP_ERROR;
+ int oid = isl_oid[data->flags].oid; /* data->flags: sub-ioctl # */
+
+ printk("%s: get_oid 0x%08X\n", ndev->name, oid);
+ data->length = 0;
+
+ if (islpci_get_state(priv) >= PRV_STATE_INIT) {
+ ret = islpci_mgt_transaction(priv->ndev, PIMFOR_OP_GET, oid, extra, 256, &response);
+ response_op = response->header->operation;
+ printk("%s: ret: %i\n", ndev->name, ret);
+ printk("%s: response_op: %i\n", ndev->name, response_op);
+ if (ret || !response || response->header->operation == PIMFOR_OP_ERROR) {
+ if (response) {
+ islpci_mgt_release(response);
+ }
+ printk("%s: EIO\n", ndev->name);
+ ret = -EIO;
+ }
+ if (!ret) {
+ data->length = response->header->length;
+ memcpy(extra, response->data, data->length);
+ islpci_mgt_release(response);
+ printk("%s: len: %i\n", ndev->name, data->length);
+ }
+ }
+
+ return ret;
+}
+
#if WIRELESS_EXT > 12
static const iw_handler prism54_handler[] = {
@@ -2130,6 +2193,29 @@
{PRISM54_OID, IW_PRIV_TYPE_INT | IW_PRIV_SIZE_FIXED | 1, 0, "oid"},
{PRISM54_GET_OID, 0, IW_PRIV_TYPE_BYTE | 256, "get_oid"},
{PRISM54_SET_OID, IW_PRIV_TYPE_BYTE | 256, 0, "set_oid"},
+
+#define PRISM54_SET_U32 SIOCIWFIRSTPRIV+20
+#define PRISM54_GET_U32 SIOCIWFIRSTPRIV+21
+ /* --- sub-ioctls handlers --- */
+ { PRISM54_SET_U32, IW_PRIV_TYPE_BYTE | 256, 0, ""},
+ { PRISM54_GET_U32, 0, IW_PRIV_TYPE_BYTE | 256, ""},
+ /* --- sub-ioctls definitions --- */
+#define PRIVATE_ARGS(oid,name) \
+ { oid, IW_PRIV_TYPE_BYTE | 256, 0, "s" #name }, \
+ { oid, 0, IW_PRIV_TYPE_BYTE | 256, "g" #name },
+/*
+ NB: increase this to 256 or more:
+ #define IW_MAX_PRIV_DEF nnn
+ in wireless tools, iwpriv.c
+ wireless_tools-27pre10 are ok
+*/
+/*PRIVATE_ARGS( ,[============])*/
+ PRIVATE_ARGS(GEN_OID_LINKSTATE ,gLINKSTATE )
+ PRIVATE_ARGS(DOT11_OID_BSSTYPE ,BSSTYPE )
+ PRIVATE_ARGS(DOT11_OID_STATE ,STATE )
+ PRIVATE_ARGS(DOT11_OID_AID ,AID )
+ PRIVATE_ARGS(DOT11_OID_MEDIUMLIMIT ,MEDIUMLIMIT )
+ PRIVATE_ARGS(DOT11_OID_BEACONPERIOD ,BEACONPERIOD )
};
static const iw_handler prism54_private_handler[] = {
@@ -2152,6 +2238,9 @@
(iw_handler) prism54_oid,
(iw_handler) prism54_get_oid,
(iw_handler) prism54_set_oid,
+ (iw_handler) NULL, //vda
+ (iw_handler) prism54_set_u32, //vda
+ (iw_handler) prism54_get_u32, //vda
};
const struct iw_handler_def prism54_handler_def = {
--Boundary-00=_P1OOAp3IzQrE5Gb--