2013-04-10 08:54:36

by Vivekananda Holla

[permalink] [raw]
Subject: [Patch] mac80211: SMPS for AP Mode

Patch for SMPS mode for AP. This is to define a new ieee80211_tx_info flag to allow a lower AP driver to set RTS/CTS protection for a dynamic SMPS station

Signed-off-by: Vivekananda Holla <[email protected]>
---
include/net/mac80211.h | 4 ++++
net/mac80211/rc80211_minstrel_ht.c | 3 +++
2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 64faf01..48a950c 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -458,6 +458,9 @@ struct ieee80211_bss_conf {
* @IEEE80211_TX_CTL_DONTFRAG: Don't fragment this packet even if it
* would be fragmented by size (this is optional, only used for
* monitor injection).
+ * @IEEE80211_TX_CTL_SMPS_SET_RTS: set the RTS/CTS protection mechanism
+ * for the packet at AP when SMPS mode at the station is
+ * dynamic SMPS.
*
* Note: If you have to add new flags to the enumeration, then don't
* forget to update %IEEE80211_TX_TEMPORARY_FLAGS when necessary.
@@ -493,6 +496,7 @@ enum mac80211_tx_control_flags {
IEEE80211_TX_STATUS_EOSP = BIT(28),
IEEE80211_TX_CTL_USE_MINRATE = BIT(29),
IEEE80211_TX_CTL_DONTFRAG = BIT(30),
+ IEEE80211_TX_CTL_SMPS_SET_RTS = BIT(31),
};

#define IEEE80211_TX_CTL_STBC_SHIFT 23
diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index d2b264d..84557e4 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -719,6 +719,9 @@ minstrel_ht_get_rate(void *priv, struct ieee80211_sta *sta, void *priv_sta,
info->flags |= mi->tx_flags;
minstrel_ht_check_cck_shortpreamble(mp, mi, txrc->short_preamble);

+ if (sta->smps_mode == IEEE80211_SMPS_DYNAMIC)
+ info->flags |= IEEE80211_TX_CTL_SMPS_SET_RTS;
+
/* Don't use EAPOL frames for sampling on non-mrr hw */
if (mp->hw->max_rates == 1 &&
txrc->skb->protocol == cpu_to_be16(ETH_P_PAE))


2013-04-10 09:55:39

by Vivekananda Holla

[permalink] [raw]
Subject: Re: [Patch] mac80211: SMPS for AP Mode

hi Johannes,

thanks for the clarification.
I will update the patch and send it again.

thanks and regards
Vivek

On 04/10/2013 03:17 PM, Johannes Berg wrote:
> On Wed, 2013-04-10 at 11:46 +0200, Johannes Berg wrote:
>> On Wed, 2013-04-10 at 15:05 +0530, Vivekananda Holla wrote:
>>> hi Johannes,
>>>
>>> do you mean the IEEE80211_TX_RC_USE_RTS_CTS flag to set RTS/CTS
>>> protection for the packet?
>>>
>>> I did not use this based on the below understanding
>>>
>>> my understanding for SMPS is that a station system does not need to
>>> send an RTS packet before frame transmission for SMPS however an AP
>>> system needs to send out an RTS frame.
>> No? Where did you get that idea? It's wrong, it doesn't matter whether
>> it's AP or not.
> Of course, APs will rarely use SMPS, so stations almost never have to
> deal with an AP that uses it ...
>
> johannes
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


2013-04-10 09:35:45

by Vivekananda Holla

[permalink] [raw]
Subject: Re: [Patch] mac80211: SMPS for AP Mode

hi Johannes,

do you mean the IEEE80211_TX_RC_USE_RTS_CTS flag to set RTS/CTS protection for the packet?

I did not use this based on the below understanding

my understanding for SMPS is that a station system does not need to send an RTS packet before frame transmission for SMPS however an AP system needs to send out an RTS frame.

hence, I defined a new txinfo flag which tells the lower driver (if AP) to send RTS frame if the station for which the packet is scheduled is dynamic SMPS to set the RTS frame for the same.

awaiting your feedback

thanks and regards
Vivek



On 04/10/2013 02:25 PM, Johannes Berg wrote:
> On Wed, 2013-04-10 at 14:24 +0530, Vivekananda Holla wrote:
>> Patch for SMPS mode for AP. This is to define a new ieee80211_tx_info flag to allow a lower AP driver to set RTS/CTS protection for a dynamic SMPS station
>>
>> Signed-off-by: Vivekananda Holla <[email protected]>
>> ---
>> include/net/mac80211.h | 4 ++++
>> net/mac80211/rc80211_minstrel_ht.c | 3 +++
>> 2 files changed, 7 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
>> index 64faf01..48a950c 100644
>> --- a/include/net/mac80211.h
>> +++ b/include/net/mac80211.h
>> @@ -458,6 +458,9 @@ struct ieee80211_bss_conf {
>> * @IEEE80211_TX_CTL_DONTFRAG: Don't fragment this packet even if it
>> * would be fragmented by size (this is optional, only used for
>> * monitor injection).
>> + * @IEEE80211_TX_CTL_SMPS_SET_RTS: set the RTS/CTS protection mechanism
>> + * for the packet at AP when SMPS mode at the station is
>> + * dynamic SMPS.
> Huh? No ... There are perfectly usable flags already inside each rate.
>
> johannes
>


2013-04-11 05:21:46

by Vivekananda Holla

[permalink] [raw]
Subject: Re: [Patch] mac80211: SMPS for AP Mode

Hi johannes,

my first patch was based on the premise that an AP cannot switch its
receive chains off as it would be servicing multiple clients.
i was not aware if an AP can set SMPS at its end, hence i wanted to
make sure that control be provided to the lower layer driver to take a
call on the same.

in my hurry to submit my first patch, i did overlook the MCS rate
dependency and accept my mistake.

i have outlined three requirements based on my understanding for SMPS
code and have also incorporated a code snippet. if you are okay with the
code snippet, i will provide a patch for the same.

1) check if the receiving station has Dynamic SMPS on
2) check if the current MCS rate is greater than a single stream rate
for the first rate
3) if conditions 1 and 2 are true, set the RTS_CTS flag for the first
rate that will
be attempted to be transmitted

if ((sta->smps_mode == IEEE80211_SMPS_DYNAMIC) && (ar[0].idx > 7))
ar[0].flags |= IEEE80211_TX_RC_USE_RTS_CTS;

i respect the time you have spent on looking into my code and apologize
for the inconvenience caused. this is my first patch submission and i
have made some mistakes :)

thanks and regards
Vivek


On Wed, 10 Apr 2013 13:18:50 +0200, Johannes Berg wrote:
> On Wed, 2013-04-10 at 16:12 +0530, Vivekananda Holla wrote:
>> patch for SMPS mode. after discussion, setting the
>> IEEE80211_TX_RC_USE_RTS_CTS flag for first rate if dynamic SMPS is on
>> in the receiving station
>
> Please line-break your description and remove all the
> discussion-related
> stuff etc. from the commit log -- see
>
> http://wireless.kernel.org/en/developers/Documentation/SubmittingPatches
>
> Also, it should set the flag depending on the MCS ... Please try to
> understand what you're actually doing and trying to do before
> submitting
> random patches. It's not an effective use of my time to be reviewing
> patches if you don't know what you're doing.
>
> johannes

2013-04-10 09:46:31

by Johannes Berg

[permalink] [raw]
Subject: Re: [Patch] mac80211: SMPS for AP Mode

On Wed, 2013-04-10 at 15:05 +0530, Vivekananda Holla wrote:
> hi Johannes,
>
> do you mean the IEEE80211_TX_RC_USE_RTS_CTS flag to set RTS/CTS
> protection for the packet?
>
> I did not use this based on the below understanding
>
> my understanding for SMPS is that a station system does not need to
> send an RTS packet before frame transmission for SMPS however an AP
> system needs to send out an RTS frame.

No? Where did you get that idea? It's wrong, it doesn't matter whether
it's AP or not.

johannes


2013-04-11 08:22:35

by Johannes Berg

[permalink] [raw]
Subject: Re: [Patch] mac80211: SMPS for AP Mode

Hi Vivek,

No worries.

> i have outlined three requirements based on my understanding for SMPS
> code and have also incorporated a code snippet. if you are okay with the
> code snippet, i will provide a patch for the same.
>
> 1) check if the receiving station has Dynamic SMPS on
> 2) check if the current MCS rate is greater than a single stream rate
> for the first rate

Note that MCS 32 is also single-stream. I'm not sure any driver supports
it, and I think minstrel_ht probably also doesn't support it, so I guess
you don't have to worry about it, but you may want to check.

> 3) if conditions 1 and 2 are true, set the RTS_CTS flag for the first
> rate that will be attempted to be transmitted

No, you don't just need the first rate, you need any rate that's
multi-stream. Otherwise a retransmission at a lower rate would never
work.

johannes


2013-04-10 11:18:58

by Johannes Berg

[permalink] [raw]
Subject: Re: [Patch] mac80211: SMPS for AP Mode

On Wed, 2013-04-10 at 16:12 +0530, Vivekananda Holla wrote:
> patch for SMPS mode. after discussion, setting the IEEE80211_TX_RC_USE_RTS_CTS flag for first rate if dynamic SMPS is on in the receiving station

Please line-break your description and remove all the discussion-related
stuff etc. from the commit log -- see
http://wireless.kernel.org/en/developers/Documentation/SubmittingPatches

Also, it should set the flag depending on the MCS ... Please try to
understand what you're actually doing and trying to do before submitting
random patches. It's not an effective use of my time to be reviewing
patches if you don't know what you're doing.

johannes


2013-04-10 09:47:15

by Johannes Berg

[permalink] [raw]
Subject: Re: [Patch] mac80211: SMPS for AP Mode

On Wed, 2013-04-10 at 11:46 +0200, Johannes Berg wrote:
> On Wed, 2013-04-10 at 15:05 +0530, Vivekananda Holla wrote:
> > hi Johannes,
> >
> > do you mean the IEEE80211_TX_RC_USE_RTS_CTS flag to set RTS/CTS
> > protection for the packet?
> >
> > I did not use this based on the below understanding
> >
> > my understanding for SMPS is that a station system does not need to
> > send an RTS packet before frame transmission for SMPS however an AP
> > system needs to send out an RTS frame.
>
> No? Where did you get that idea? It's wrong, it doesn't matter whether
> it's AP or not.

Of course, APs will rarely use SMPS, so stations almost never have to
deal with an AP that uses it ...

johannes


2013-04-10 08:56:00

by Johannes Berg

[permalink] [raw]
Subject: Re: [Patch] mac80211: SMPS for AP Mode

On Wed, 2013-04-10 at 14:24 +0530, Vivekananda Holla wrote:
> Patch for SMPS mode for AP. This is to define a new ieee80211_tx_info flag to allow a lower AP driver to set RTS/CTS protection for a dynamic SMPS station
>
> Signed-off-by: Vivekananda Holla <[email protected]>
> ---
> include/net/mac80211.h | 4 ++++
> net/mac80211/rc80211_minstrel_ht.c | 3 +++
> 2 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index 64faf01..48a950c 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -458,6 +458,9 @@ struct ieee80211_bss_conf {
> * @IEEE80211_TX_CTL_DONTFRAG: Don't fragment this packet even if it
> * would be fragmented by size (this is optional, only used for
> * monitor injection).
> + * @IEEE80211_TX_CTL_SMPS_SET_RTS: set the RTS/CTS protection mechanism
> + * for the packet at AP when SMPS mode at the station is
> + * dynamic SMPS.

Huh? No ... There are perfectly usable flags already inside each rate.

johannes