[PATCH 2/2] zd1211rw: enable an AP that can't support fully PS clients (wireless-testing).

[PATCH 2/2] zd1211rw: enable an AP that can't support fully PS clients (wireless-testing).

[Rami Rosen]

This patch enables master mode support in zd1211rw driver and enables
it to support the PS challenged mode when used in master mode.

It adds this functionality in the zd1211rw driver by adding support to
master mode (NL80211_IFTYPE_AP) in zd_op_add_interface() and in
zd_mac_alloc_hw() methods
of drivers/net/wireless/zd_mac.c.

In addition, the new boolean member of wiphy struct, ap_ps_challenged,
is initialized to true in zd_mac_alloc_hw().

Signed-off-by: Rami Rosen <ramirose@gmail.com>

diff --git a/drivers/net/wireless/zd1211rw/zd_mac.c
b/drivers/net/wireless/zd1211rw/zd_mac.c
index 980acdf..d00cff2 100644
--- a/drivers/net/wireless/zd1211rw/zd_mac.c
+++ b/drivers/net/wireless/zd1211rw/zd_mac.c
@@ -721,6 +721,7 @@ static int zd_op_add_interface(struct ieee80211_hw *hw,
 		return -EOPNOTSUPP;

 	switch (conf->type) {
+	case NL80211_IFTYPE_AP:
 	case NL80211_IFTYPE_MONITOR:
 	case NL80211_IFTYPE_MESH_POINT:
 	case NL80211_IFTYPE_STATION:
@@ -972,10 +973,11 @@ struct ieee80211_hw *zd_mac_alloc_hw(struct
usb_interface *intf)
 		    IEEE80211_HW_SIGNAL_DB;

 	hw->wiphy->interface_modes =
+		BIT(NL80211_IFTYPE_AP) |
 		BIT(NL80211_IFTYPE_MESH_POINT) |
 		BIT(NL80211_IFTYPE_STATION) |
 		BIT(NL80211_IFTYPE_ADHOC);
-
+	hw->wiphy->ap_ps_challenged = true;
 	hw->max_signal = 100;
 	hw->queues = 1;
 	hw->extra_tx_headroom = sizeof(struct zd_ctrlset);

Re: [PATCH 2/2] zd1211rw: enable an AP that can't support fully PS clients (wireless-testing).

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 752 bytes --]

On Mon, 2008-11-24 at 09:44 +0200, Rami Rosen wrote:
> This patch enables master mode support in zd1211rw driver and enables
> it to support the PS challenged mode when used in master mode.
> 
> It adds this functionality in the zd1211rw driver by adding support to
> master mode (NL80211_IFTYPE_AP) in zd_op_add_interface() and in
> zd_mac_alloc_hw() methods
> of drivers/net/wireless/zd_mac.c.
> 
> In addition, the new boolean member of wiphy struct, ap_ps_challenged,
> is initialized to true in zd_mac_alloc_hw().

This is all fine, but I'm starting to wonder why we're doing this at
all. Have we actually determined that zd1211 cannot buffer mcast/bcast
frames at all? Might it be worth asking around in Zydas/Atheros?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] zd1211rw: enable an AP that can't support fully PS clients (wireless-testing).

[Rami Rosen]

Hello,
If you are talking about whether the zd1211 driver can buffer
mcast/bcast frames in software, it seems to me that the answer is
negative, since there is no call for ieee80211_get_buffered_bc() in
the zd1211rw driver code and also there is no handling of
IEEE80211_TX_CTL_SEND_AFTER_DTIM or
IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING in the zd1211rw driver code.
This what made me add the NL80211_ATTR_WIPHY_AP_PS_CHALLENGED
attribute and prepare this patch.


Regards,
Rami Rosen



On Mon, Nov 24, 2008 at 12:35 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Mon, 2008-11-24 at 09:44 +0200, Rami Rosen wrote:
>> This patch enables master mode support in zd1211rw driver and enables
>> it to support the PS challenged mode when used in master mode.
>>
>> It adds this functionality in the zd1211rw driver by adding support to
>> master mode (NL80211_IFTYPE_AP) in zd_op_add_interface() and in
>> zd_mac_alloc_hw() methods
>> of drivers/net/wireless/zd_mac.c.
>>
>> In addition, the new boolean member of wiphy struct, ap_ps_challenged,
>> is initialized to true in zd_mac_alloc_hw().
>
> This is all fine, but I'm starting to wonder why we're doing this at
> all. Have we actually determined that zd1211 cannot buffer mcast/bcast
> frames at all? Might it be worth asking around in Zydas/Atheros?
>
> johannes
>

Re: [PATCH 2/2] zd1211rw: enable an AP that can't support fully PS clients (wireless-testing).

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 795 bytes --]

On Mon, 2008-11-24 at 13:09 +0200, Rami Rosen wrote:

> If you are talking about whether the zd1211 driver can buffer
> mcast/bcast frames in software, it seems to me that the answer is
> negative, since there is no call for ieee80211_get_buffered_bc() in
> the zd1211rw driver code and also there is no handling of
> IEEE80211_TX_CTL_SEND_AFTER_DTIM or
> IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING in the zd1211rw driver code.
> This what made me add the NL80211_ATTR_WIPHY_AP_PS_CHALLENGED
> attribute and prepare this patch.

Yes, I know that's why you did it, but I'm still wondering whether it
was necessary, is it really impossible to implement mcast/bcast
buffering with this hardware? IOW, can we fix the driver to do the
buffering and using _get_buffered_bc()?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] zd1211rw: enable an AP that can't support fully PS clients (wireless-testing).

[Rami Rosen]

Hello,
  I indeed intend to fix the driver to do the buffering and use
_get_buffered_bc() (and also add sequence numbering, which is easier).
It will take time to fix it and to test it with hostapd and clients.

When it will be ready, the only thing which will be needed is
to remove the "hw->wiphy->ap_ps_challenged = true" from
zd_mac_alloc_hw() in zd_mac.c. The other changes in the driver should
of course stay (adding handling of NL80211_IFTYPE_AP).

As far as I understand, the zd1211rw is not the only one which does
not support buffering; (and I assume there will be other such drivers
without this support in the future); so it seems to me that the
nl80211 patch, which enable users who would prefer to have an AP that
can't support fully PS clients, rather than not having an AP at all,
is a good enough reason for the nl80211 patch.

Using Linux as an access point with a standard and a wide range of
commodity wireless drivers seems really a nice and important feature.

Regards,
Rami Rosen


On Mon, Nov 24, 2008 at 1:21 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Mon, 2008-11-24 at 13:09 +0200, Rami Rosen wrote:
>
>> If you are talking about whether the zd1211 driver can buffer
>> mcast/bcast frames in software, it seems to me that the answer is
>> negative, since there is no call for ieee80211_get_buffered_bc() in
>> the zd1211rw driver code and also there is no handling of
>> IEEE80211_TX_CTL_SEND_AFTER_DTIM or
>> IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING in the zd1211rw driver code.
>> This what made me add the NL80211_ATTR_WIPHY_AP_PS_CHALLENGED
>> attribute and prepare this patch.
>
> Yes, I know that's why you did it, but I'm still wondering whether it
> was necessary, is it really impossible to implement mcast/bcast
> buffering with this hardware? IOW, can we fix the driver to do the
> buffering and using _get_buffered_bc()?
>
> johannes
>

Re: [PATCH 2/2] zd1211rw: enable an AP that can't support fully PS clients (wireless-testing).

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1340 bytes --]

On Mon, 2008-11-24 at 13:38 +0200, Rami Rosen wrote:
> Hello,
>   I indeed intend to fix the driver to do the buffering and use
> _get_buffered_bc() (and also add sequence numbering, which is easier).
> It will take time to fix it and to test it with hostapd and clients.
> 
> When it will be ready, the only thing which will be needed is
> to remove the "hw->wiphy->ap_ps_challenged = true" from
> zd_mac_alloc_hw() in zd_mac.c. The other changes in the driver should
> of course stay (adding handling of NL80211_IFTYPE_AP).
> 
> As far as I understand, the zd1211rw is not the only one which does
> not support buffering; (and I assume there will be other such drivers
> without this support in the future); so it seems to me that the
> nl80211 patch, which enable users who would prefer to have an AP that
> can't support fully PS clients, rather than not having an AP at all,
> is a good enough reason for the nl80211 patch.
> 
> Using Linux as an access point with a standard and a wide range of
> commodity wireless drivers seems really a nice and important feature.

Well, I'd rather have the nl80211/cfg80211 part only when we find out
that it's absolutely necessary. I don't see a problem in waiting now
until you either figure out how to fix zd1211 or determine that it's not
possible at all.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] zd1211rw: enable an AP that can't support fully PS clients (wireless-testing).

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 563 bytes --]

On Mon, 2008-11-24 at 13:38 +0200, Rami Rosen wrote:
> Hello,
>   I indeed intend to fix the driver to do the buffering and use
> _get_buffered_bc() (and also add sequence numbering, which is easier).

Sequence numbers are provided by the firmware, I think? At least beacons
do have sequence numbers.

Anyway, so I got some information, to fix this use _get_buffered_bc()
when in zd_process_intr you get INT_CFG_NEXT_BCN; but this indicates
that the beacon has _just_ been sent so you need to use _get_beacon()
*after* _get_buffered_bc().

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] zd1211rw: enable an AP that can't support fully PS clients (wireless-testing).

[Rami Rosen]

Hello,
  - Thanks a lot for this info!

  - I will adjust my patch accordingly and preform tests to see that
multicast buffering works properly.

Regards,
Rami Rosen


On Thu, Nov 27, 2008 at 11:06 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Mon, 2008-11-24 at 13:38 +0200, Rami Rosen wrote:
>> Hello,
>>   I indeed intend to fix the driver to do the buffering and use
>> _get_buffered_bc() (and also add sequence numbering, which is easier).
>
> Sequence numbers are provided by the firmware, I think? At least beacons
> do have sequence numbers.
>
> Anyway, so I got some information, to fix this use _get_buffered_bc()
> when in zd_process_intr you get INT_CFG_NEXT_BCN; but this indicates
> that the beacon has _just_ been sent so you need to use _get_beacon()
> *after* _get_buffered_bc().
>
> johannes
>