Hi
Thank you for updating the patch.
On Thu, 10 Jan 2008 01:16:02 -0800 (PST)
David Miller <davem@xxxxxxxxxxxxx> wrote:
> From: Masakazu Mokuno <mokuno@xxxxxxxxxxxxx>
> Date: Thu, 27 Dec 2007 18:24:40 +0900
>
> > On ppc64 (PS3), IW_EV_LCP_LEN is 8, not 4.
> >
> > include/linux/wireless.h:
> >
> > #define IW_EV_LCP_LEN (sizeof(struct iw_event) - sizeof(union iwreq_data))
> >
> > where sizeof(struct iw_event) == 24, sizeof(union iwreq_data) == 16 on
> > PS3.
>
> Here is a new version of the last patch (#12), it should handle
> all of these cases properly now.
>
> Let me know if you spot any more errors.
>
> Thanks!
>
> [WEXT]: Emit event stream entries correctly when compat.
>
> Three major portions to this change:
>
> 1) Add IW_EV_COMPAT_LCP_LEN, IW_EV_COMPAT_POINT_OFF,
> and IW_EV_COMPAT_POINT_LEN helper defines.
>
> 2) Delete iw_stream_check_add_*(), they are unused.
>
> 3) Add iw_request_info argument to iwe_stream_add_*(), and use it to
> size the event and pointer lengths correctly depending upon whether
> IW_REQUEST_FLAG_COMPAT is set or not.
>
> 4) The mechanical transformations to the drivers and wireless stack
> bits to get the iw_request_info passed down into the routines
> modified in #3.
>
> With help from Masakazu Mokuno
>
> Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>
> ---
> drivers/net/wireless/airo.c | 39 +++++---
> drivers/net/wireless/atmel.c | 24 ++++-
> drivers/net/wireless/hostap/hostap.h | 3 +-
> drivers/net/wireless/hostap/hostap_ap.c | 32 +++---
> drivers/net/wireless/hostap/hostap_ioctl.c | 54 ++++++-----
> drivers/net/wireless/libertas/scan.c | 35 ++++---
> drivers/net/wireless/orinoco.c | 30 ++++--
> drivers/net/wireless/prism54/isl_ioctl.c | 45 +++++----
> drivers/net/wireless/wl3501_cs.c | 10 +-
> drivers/net/wireless/zd1201.c | 21 +++--
> include/linux/wireless.h | 16 +++
> include/net/iw_handler.h | 150 ++++++++--------------------
> net/ieee80211/ieee80211_wx.c | 44 +++++----
> net/mac80211/ieee80211_i.h | 5 +-
> net/mac80211/ieee80211_ioctl.c | 2 +-
> net/mac80211/ieee80211_sta.c | 59 ++++++-----
> 16 files changed, 293 insertions(+), 276 deletions(-)
[snip]
> diff --git a/include/linux/wireless.h b/include/linux/wireless.h
> index 2088524..bec73df 100644
> --- a/include/linux/wireless.h
> +++ b/include/linux/wireless.h
> @@ -1098,6 +1098,22 @@ struct iw_event
> #define IW_EV_POINT_LEN (IW_EV_LCP_LEN + sizeof(struct iw_point) - \
> IW_EV_POINT_OFF)
>
> +#if defined(__KERNEL__) && defined(CONFIG_COMPAT)
> +struct __compat_iw_event {
> + __u16 len; /* Real lenght of this stuff */
~~~~~
length?
> + __u16 cmd; /* Wireless IOCTL */
> + compat_caddr_t pointer;
> +};
> +#define IW_EV_COMPAT_LCP_LEN \
> + (sizeof(struct __compat_iw_event) - sizeof(compat_caddr_t))
> +#define IW_EV_COMPAT_POINT_OFF (((char *) \
> + &(((struct compat_iw_point *) NULL)->length)) - \
> + (char *) NULL)
How about the followings?
#define IW_EV_COMPAT_LCP_LEN offsetof(struct __compat_iw_event, pointer)
#define IW_EV_COMPAT_POINT_OFF offsetof(struct compat_iw_point, length)
> +#define IW_EV_COMPAT_POINT_LEN \
> + (IW_EV_COMPAT_LCP_LEN + sizeof(struct compat_iw_point) - \
> + IW_EV_COMPAT_POINT_OFF)
> +#endif
> +
> /* Size of the Event prefix when packed in stream */
> #define IW_EV_LCP_PK_LEN (4)
> /* Size of the various events when packed in stream */
> diff --git a/include/net/iw_handler.h b/include/net/iw_handler.h
> index c99a8ee..d6f0c51 100644
> --- a/include/net/iw_handler.h
> +++ b/include/net/iw_handler.h
> @@ -483,19 +483,26 @@ extern void wireless_spy_update(struct net_device * dev,
> * Wrapper to add an Wireless Event to a stream of events.
> */
> static inline char *
> -iwe_stream_add_event(char * stream, /* Stream of events */
> - char * ends, /* End of stream */
> - struct iw_event *iwe, /* Payload */
> - int event_len) /* Real size of payload */
> +iwe_stream_add_event(struct iw_request_info *info, char *stream, char *ends,
> + struct iw_event *iwe, int event_len)
> {
> + int lcp_len = IW_EV_LCP_LEN;
> +
> +#ifdef CONFIG_COMPAT
> + if (info->flags & IW_REQUEST_FLAG_COMPAT) {
> + event_len -= IW_EV_LCP_LEN;
> + event_len += IW_EV_COMPAT_LCP_LEN;
> + lcp_len = IW_EV_COMPAT_LCP_LEN;
> + }
> +#endif
> /* Check if it's possible */
> if(likely((stream + event_len) < ends)) {
> iwe->len = event_len;
> /* Beware of alignement issues on 64 bits */
> memcpy(stream, (char *) iwe, IW_EV_LCP_PK_LEN);
> - memcpy(stream + IW_EV_LCP_LEN,
> - ((char *) iwe) + IW_EV_LCP_LEN,
> - event_len - IW_EV_LCP_LEN);
> + memcpy(stream + lcp_len,
> + ((char *) iwe) + lcp_len,
The source address does not have to be adjusted. I think it should be
((char *) iwe) + IW_EV_LCP_LEN,
or just
&iwe->u
I think it is more readable what we want to do here.
> + event_len - lcp_len);
The length of the real payload does not need to take account of the
destination offset change. So we can keep the original code like:
org_event_len - IW_EV_LCP_LEN);
> stream += event_len;
> }
> return stream;
> @@ -507,104 +514,33 @@ iwe_stream_add_event(char * stream, /* Stream of events */
> * stream of events.
> */
> static inline char *
> -iwe_stream_add_point(char * stream, /* Stream of events */
> - char * ends, /* End of stream */
> - struct iw_event *iwe, /* Payload length + flags */
> - char * extra) /* More payload */
> +iwe_stream_add_point(struct iw_request_info *info, char *stream, char *ends,
> + struct iw_event *iwe, char *extra)
> {
> - int event_len = IW_EV_POINT_LEN + iwe->u.data.length;
> - /* Check if it's possible */
> - if(likely((stream + event_len) < ends)) {
> - iwe->len = event_len;
> - memcpy(stream, (char *) iwe, IW_EV_LCP_PK_LEN);
> - memcpy(stream + IW_EV_LCP_LEN,
> - ((char *) iwe) + IW_EV_LCP_LEN + IW_EV_POINT_OFF,
> - IW_EV_POINT_PK_LEN - IW_EV_LCP_PK_LEN);
> - memcpy(stream + IW_EV_POINT_LEN, extra, iwe->u.data.length);
> - stream += event_len;
> + int event_len = IW_EV_POINT_LEN + iwe->u.data.length;
> + int point_len = IW_EV_POINT_LEN;
> + int lcp_len = IW_EV_LCP_LEN;
> + int point_off = IW_EV_POINT_OFF;
> +
> +#ifdef CONFIG_COMPAT
> + if (info->flags & IW_REQUEST_FLAG_COMPAT) {
> + event_len = IW_EV_COMPAT_POINT_LEN + iwe->u.data.length;
> + point_len = IW_EV_COMPAT_POINT_LEN;
> + lcp_len = IW_EV_COMPAT_LCP_LEN;
> + point_off = IW_EV_COMPAT_POINT_OFF;
> }
> - return stream;
> -}
> -
> -/*------------------------------------------------------------------*/
> -/*
> - * Wrapper to add a value to a Wireless Event in a stream of events.
> - * Be careful, this one is tricky to use properly :
> - * At the first run, you need to have (value = event + IW_EV_LCP_LEN).
> - */
> -static inline char *
> -iwe_stream_add_value(char * event, /* Event in the stream */
> - char * value, /* Value in event */
> - char * ends, /* End of stream */
> - struct iw_event *iwe, /* Payload */
> - int event_len) /* Real size of payload */
> -{
> - /* Don't duplicate LCP */
> - event_len -= IW_EV_LCP_LEN;
> +#endif
>
> /* Check if it's possible */
> - if(likely((value + event_len) < ends)) {
> - /* Add new value */
> - memcpy(value, (char *) iwe + IW_EV_LCP_LEN, event_len);
> - value += event_len;
> - /* Patch LCP */
> - iwe->len = value - event;
> - memcpy(event, (char *) iwe, IW_EV_LCP_LEN);
> - }
> - return value;
> -}
> -
> -/*------------------------------------------------------------------*/
> -/*
> - * Wrapper to add an Wireless Event to a stream of events.
> - * Same as above, with explicit error check...
> - */
> -static inline char *
> -iwe_stream_check_add_event(char * stream, /* Stream of events */
> - char * ends, /* End of stream */
> - struct iw_event *iwe, /* Payload */
> - int event_len, /* Size of payload */
> - int * perr) /* Error report */
> -{
> - /* Check if it's possible, set error if not */
> if(likely((stream + event_len) < ends)) {
> iwe->len = event_len;
> - /* Beware of alignement issues on 64 bits */
> memcpy(stream, (char *) iwe, IW_EV_LCP_PK_LEN);
> - memcpy(stream + IW_EV_LCP_LEN,
> - ((char *) iwe) + IW_EV_LCP_LEN,
> - event_len - IW_EV_LCP_LEN);
> - stream += event_len;
> - } else
> - *perr = -E2BIG;
> - return stream;
> -}
> -
> -/*------------------------------------------------------------------*/
> -/*
> - * Wrapper to add an short Wireless Event containing a pointer to a
> - * stream of events.
> - * Same as above, with explicit error check...
> - */
> -static inline char *
> -iwe_stream_check_add_point(char * stream, /* Stream of events */
> - char * ends, /* End of stream */
> - struct iw_event *iwe, /* Payload length + flags */
> - char * extra, /* More payload */
> - int * perr) /* Error report */
> -{
> - int event_len = IW_EV_POINT_LEN + iwe->u.data.length;
> - /* Check if it's possible */
> - if(likely((stream + event_len) < ends)) {
> - iwe->len = event_len;
> - memcpy(stream, (char *) iwe, IW_EV_LCP_PK_LEN);
> - memcpy(stream + IW_EV_LCP_LEN,
> - ((char *) iwe) + IW_EV_LCP_LEN + IW_EV_POINT_OFF,
> + memcpy(stream + lcp_len,
> + ((char *) iwe) + lcp_len + point_off,
Same as iwe_stream_add_event(). Source address does not need to be
fixed.
> IW_EV_POINT_PK_LEN - IW_EV_LCP_PK_LEN);
> - memcpy(stream + IW_EV_POINT_LEN, extra, iwe->u.data.length);
> + memcpy(stream + point_len, extra, iwe->u.data.length);
> stream += event_len;
> - } else
> - *perr = -E2BIG;
> + }
> return stream;
> }
>
> @@ -613,29 +549,29 @@ iwe_stream_check_add_point(char * stream, /* Stream of events */
> * Wrapper to add a value to a Wireless Event in a stream of events.
> * Be careful, this one is tricky to use properly :
> * At the first run, you need to have (value = event + IW_EV_LCP_LEN).
> - * Same as above, with explicit error check...
> */
> static inline char *
> -iwe_stream_check_add_value(char * event, /* Event in the stream */
> - char * value, /* Value in event */
> - char * ends, /* End of stream */
> - struct iw_event *iwe, /* Payload */
> - int event_len, /* Size of payload */
> - int * perr) /* Error report */
> +iwe_stream_add_value(struct iw_request_info *info, char *event, char *value,
> + char *ends, struct iw_event *iwe, int event_len)
> {
> + int lcp_len = IW_EV_LCP_LEN;
> +
> +#ifdef CONFIG_COMPAT
> + if (info->flags & IW_REQUEST_FLAG_COMPAT)
> + lcp_len = IW_EV_COMPAT_LCP_LEN;
> +#endif
> /* Don't duplicate LCP */
> event_len -= IW_EV_LCP_LEN;
>
> /* Check if it's possible */
> if(likely((value + event_len) < ends)) {
> /* Add new value */
> - memcpy(value, (char *) iwe + IW_EV_LCP_LEN, event_len);
> + memcpy(value, (char *) iwe + lcp_len, event_len);
Same as iwe_stream_add_event(). We can keep the original code.
> value += event_len;
> /* Patch LCP */
> iwe->len = value - event;
> - memcpy(event, (char *) iwe, IW_EV_LCP_LEN);
> - } else
> - *perr = -E2BIG;
> + memcpy(event, (char *) iwe, lcp_len);
> + }
> return value;
> }
iwe_stream_add_value() is a bit tricky, so callers of this function
should do some adjusting work. For example,
ieee80211_translate_scan():ieee80211_wx.c needs like:
/* Add basic and extended rates */
/* Rate : stuffing multiple values in a single event require a bit
* more of magic - Jean II */
+#ifdef CONFIG_COMPAT
+ if (info->flags & IW_REQUEST_FLAG_COMPAT)
+ current_val = start + IW_EV_COMPAT_LCP_LEN;
+ else
+ current_val = start + IW_EV_LCP_LEN;
+#else
current_val = start + IW_EV_LCP_LEN;
+#endif
iwe.cmd = SIOCGIWRATE;
/* Those two flags are ignored... */
iwe.u.bitrate.fixed = iwe.u.bitrate.disabled = 0;
I tried to make the patch that was incorporated with my comments above and
tested it with zd1211 based USB wireless adapter on my PS3. It seemed
that the scan result prints from iwlist was OK. The attached is the
patch.
But wpa_supplicant did not work properly with it. Although I did not
look it closely, it seemed that wpa_supplicant got a NULL IWAP event
when associated.
--
Masakazu MOKUNO
Attachment:
davem-12-modified.patch
Description: Binary data
