2014-01-15 16:07:36

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH 0/5] use one common ath_is_mybeacon

Most ATH driver use same beacon check. It is worth to place it
to the main ATH lib.

Oleksij Rempel (5):
ath: add common function ath_is_mybeacon
ath9k: use ath_is_mybeacon
ath9k_htc: use ath_is_mybeacon
ath5k: use ath_is_mybeacon
carl9170: use ath_is_mybeacon

drivers/net/wireless/ath/ath.h | 2 ++
drivers/net/wireless/ath/ath5k/base.c | 33 +++++++--------------------
drivers/net/wireless/ath/ath9k/htc_drv_txrx.c | 4 +---
drivers/net/wireless/ath/ath9k/recv.c | 20 ++++------------
drivers/net/wireless/ath/carl9170/rx.c | 9 +++-----
drivers/net/wireless/ath/main.c | 11 +++++++++
6 files changed, 29 insertions(+), 50 deletions(-)

--
1.8.5.2



2014-01-15 16:07:43

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH 2/5] ath9k: use ath_is_mybeacon

This patch will also change behavior of rx_beacons statistic.
Instead of collecting all received beacons, it will collect only
ours. This, IMO make more sense, since for troubleshooting we will
need to know count of our beacons, or both.

Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/wireless/ath/ath9k/recv.c | 20 ++++----------------
1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 5bf3243..a0ebdd0 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -969,21 +969,6 @@ static void ath9k_process_tsf(struct ath_rx_status *rs,
rxs->mactime += 0x100000000ULL;
}

-static bool ath9k_is_mybeacon(struct ath_softc *sc, struct ieee80211_hdr *hdr)
-{
- struct ath_hw *ah = sc->sc_ah;
- struct ath_common *common = ath9k_hw_common(ah);
-
- if (ieee80211_is_beacon(hdr->frame_control)) {
- RX_STAT_INC(rx_beacons);
- if (!is_zero_ether_addr(common->curbssid) &&
- ether_addr_equal_64bits(hdr->addr3, common->curbssid))
- return true;
- }
-
- return false;
-}
-
/*
* For Decrypt or Demic errors, we only mark packet status here and always push
* up the frame up to let mac80211 handle the actual error case, be it no
@@ -1071,7 +1056,10 @@ static int ath9k_rx_skb_preprocess(struct ath_softc *sc,
goto exit;
}

- rx_stats->is_mybeacon = ath9k_is_mybeacon(sc, hdr);
+ if (ath_is_mybeacon(common, hdr)) {
+ RX_STAT_INC(rx_beacons);
+ rx_stats->is_mybeacon = true;
+ }

/*
* This shouldn't happen, but have a safety check anyway.
--
1.8.5.2


2014-01-15 16:16:11

by Oleksij Rempel

[permalink] [raw]
Subject: Re: [PATCH 1/5] ath: add common function ath_is_mybeacon

Am 15.01.2014 17:10, schrieb Antonio Quartulli:
> On 15/01/14 17:07, Oleksij Rempel wrote:
>> this function is used by most ath driver, so it can be moved here.
>>
>> Signed-off-by: Oleksij Rempel <[email protected]>
>> ---
>> drivers/net/wireless/ath/ath.h | 2 ++
>> drivers/net/wireless/ath/main.c | 11 +++++++++++
>> 2 files changed, 13 insertions(+)
>>
>> diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
>> index e0ba7cd..b59cfbe 100644
>> --- a/drivers/net/wireless/ath/ath.h
>> +++ b/drivers/net/wireless/ath/ath.h
>> @@ -17,6 +17,7 @@
>> #ifndef ATH_H
>> #define ATH_H
>>
>> +#include <linux/etherdevice.h>
>> #include <linux/skbuff.h>
>> #include <linux/if_ether.h>
>> #include <linux/spinlock.h>
>> @@ -165,6 +166,7 @@ struct ath_common {
>> struct sk_buff *ath_rxbuf_alloc(struct ath_common *common,
>> u32 len,
>> gfp_t gfp_mask);
>> +bool ath_is_mybeacon(struct ath_common *common, struct ieee80211_hdr *hdr);
>>
>> void ath_hw_setbssidmask(struct ath_common *common);
>> void ath_key_delete(struct ath_common *common, struct ieee80211_key_conf *key);
>> diff --git a/drivers/net/wireless/ath/main.c b/drivers/net/wireless/ath/main.c
>> index 8e99540..9cb15d9 100644
>> --- a/drivers/net/wireless/ath/main.c
>> +++ b/drivers/net/wireless/ath/main.c
>> @@ -59,6 +59,17 @@ struct sk_buff *ath_rxbuf_alloc(struct ath_common *common,
>> }
>> EXPORT_SYMBOL(ath_rxbuf_alloc);
>>
>> +bool ath_is_mybeacon(struct ath_common *common, struct ieee80211_hdr *hdr)
>> +{
>> + if (ieee80211_is_beacon(hdr->frame_control) &&
>> + !is_zero_ether_addr(common->curbssid) &&
>> + ether_addr_equal_64bits(hdr->addr3, common->curbssid))
>> + return true;
>> +
>
> Apart from the fact that the expression in the if guard is badly
> indented, couldn't you just return the value of that expression and
> remove the if at all?

Good point, thank you!


--
Regards,
Oleksij

2014-01-17 23:27:43

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH v3 1/5] ath: add common function ath_is_mybeacon

this function is used by most ath driver, so it can be moved here.

Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/wireless/ath/ath.h | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
index e0ba7cd..ab6a0c8 100644
--- a/drivers/net/wireless/ath/ath.h
+++ b/drivers/net/wireless/ath/ath.h
@@ -17,6 +17,7 @@
#ifndef ATH_H
#define ATH_H

+#include <linux/etherdevice.h>
#include <linux/skbuff.h>
#include <linux/if_ether.h>
#include <linux/spinlock.h>
@@ -162,10 +163,17 @@ struct ath_common {
bool bt_ant_diversity;
};

+static inline bool ath_is_mybeacon(struct ath_common *common,
+ struct ieee80211_hdr *hdr)
+{
+ return ieee80211_is_beacon(hdr->frame_control) &&
+ !is_zero_ether_addr(common->curbssid) &&
+ ether_addr_equal_64bits(hdr->addr3, common->curbssid);
+}
+
struct sk_buff *ath_rxbuf_alloc(struct ath_common *common,
u32 len,
gfp_t gfp_mask);
-
void ath_hw_setbssidmask(struct ath_common *common);
void ath_key_delete(struct ath_common *common, struct ieee80211_key_conf *key);
int ath_key_config(struct ath_common *common,
--
1.8.5.2


2014-01-15 16:07:46

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH 3/5] ath9k_htc: use ath_is_mybeacon

Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/wireless/ath/ath9k/htc_drv_txrx.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
index b41e008..12e0f32 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
@@ -1075,9 +1075,7 @@ static bool ath9k_rx_prepare(struct ath9k_htc_priv *priv,

last_rssi = priv->rx.last_rssi;

- if (ieee80211_is_beacon(hdr->frame_control) &&
- !is_zero_ether_addr(common->curbssid) &&
- ether_addr_equal_64bits(hdr->addr3, common->curbssid)) {
+ if (ath_is_mybeacon(common, hdr)) {
s8 rssi = rxbuf->rxstatus.rs_rssi;

if (likely(last_rssi != ATH_RSSI_DUMMY_MARKER))
--
1.8.5.2


2014-01-16 15:44:09

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 1/5] ath: add common function ath_is_mybeacon

Oleksij Rempel <[email protected]> writes:

> +bool ath_is_mybeacon(struct ath_common *common, struct ieee80211_hdr *hdr)

Would a static inline be better? At least on ath9k I suspect the
compiler automatically inlines the static function, after your patch
there's an extra function call.

--
Kalle Valo

2014-01-15 17:01:02

by Antonio Quartulli

[permalink] [raw]
Subject: Re: [PATCH 1/5] ath: add common function ath_is_mybeacon

On 15/01/14 17:07, Oleksij Rempel wrote:
> this function is used by most ath driver, so it can be moved here.
>
> Signed-off-by: Oleksij Rempel <[email protected]>
> ---
> drivers/net/wireless/ath/ath.h | 2 ++
> drivers/net/wireless/ath/main.c | 11 +++++++++++
> 2 files changed, 13 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
> index e0ba7cd..b59cfbe 100644
> --- a/drivers/net/wireless/ath/ath.h
> +++ b/drivers/net/wireless/ath/ath.h
> @@ -17,6 +17,7 @@
> #ifndef ATH_H
> #define ATH_H
>
> +#include <linux/etherdevice.h>
> #include <linux/skbuff.h>
> #include <linux/if_ether.h>
> #include <linux/spinlock.h>
> @@ -165,6 +166,7 @@ struct ath_common {
> struct sk_buff *ath_rxbuf_alloc(struct ath_common *common,
> u32 len,
> gfp_t gfp_mask);
> +bool ath_is_mybeacon(struct ath_common *common, struct ieee80211_hdr *hdr);
>
> void ath_hw_setbssidmask(struct ath_common *common);
> void ath_key_delete(struct ath_common *common, struct ieee80211_key_conf *key);
> diff --git a/drivers/net/wireless/ath/main.c b/drivers/net/wireless/ath/main.c
> index 8e99540..9cb15d9 100644
> --- a/drivers/net/wireless/ath/main.c
> +++ b/drivers/net/wireless/ath/main.c
> @@ -59,6 +59,17 @@ struct sk_buff *ath_rxbuf_alloc(struct ath_common *common,
> }
> EXPORT_SYMBOL(ath_rxbuf_alloc);
>
> +bool ath_is_mybeacon(struct ath_common *common, struct ieee80211_hdr *hdr)
> +{
> + if (ieee80211_is_beacon(hdr->frame_control) &&
> + !is_zero_ether_addr(common->curbssid) &&
> + ether_addr_equal_64bits(hdr->addr3, common->curbssid))
> + return true;
> +

Apart from the fact that the expression in the if guard is badly
indented, couldn't you just return the value of that expression and
remove the if at all?

Cheers,

> + return false;
> +}
> +EXPORT_SYMBOL(ath_is_mybeacon);
> +
> void ath_printk(const char *level, const struct ath_common* common,
> const char *fmt, ...)
> {
>


--
Antonio Quartulli


Attachments:
signature.asc (836.00 B)
OpenPGP digital signature

2014-01-15 16:07:41

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH 1/5] ath: add common function ath_is_mybeacon

this function is used by most ath driver, so it can be moved here.

Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/wireless/ath/ath.h | 2 ++
drivers/net/wireless/ath/main.c | 11 +++++++++++
2 files changed, 13 insertions(+)

diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
index e0ba7cd..b59cfbe 100644
--- a/drivers/net/wireless/ath/ath.h
+++ b/drivers/net/wireless/ath/ath.h
@@ -17,6 +17,7 @@
#ifndef ATH_H
#define ATH_H

+#include <linux/etherdevice.h>
#include <linux/skbuff.h>
#include <linux/if_ether.h>
#include <linux/spinlock.h>
@@ -165,6 +166,7 @@ struct ath_common {
struct sk_buff *ath_rxbuf_alloc(struct ath_common *common,
u32 len,
gfp_t gfp_mask);
+bool ath_is_mybeacon(struct ath_common *common, struct ieee80211_hdr *hdr);

void ath_hw_setbssidmask(struct ath_common *common);
void ath_key_delete(struct ath_common *common, struct ieee80211_key_conf *key);
diff --git a/drivers/net/wireless/ath/main.c b/drivers/net/wireless/ath/main.c
index 8e99540..9cb15d9 100644
--- a/drivers/net/wireless/ath/main.c
+++ b/drivers/net/wireless/ath/main.c
@@ -59,6 +59,17 @@ struct sk_buff *ath_rxbuf_alloc(struct ath_common *common,
}
EXPORT_SYMBOL(ath_rxbuf_alloc);

+bool ath_is_mybeacon(struct ath_common *common, struct ieee80211_hdr *hdr)
+{
+ if (ieee80211_is_beacon(hdr->frame_control) &&
+ !is_zero_ether_addr(common->curbssid) &&
+ ether_addr_equal_64bits(hdr->addr3, common->curbssid))
+ return true;
+
+ return false;
+}
+EXPORT_SYMBOL(ath_is_mybeacon);
+
void ath_printk(const char *level, const struct ath_common* common,
const char *fmt, ...)
{
--
1.8.5.2


2014-01-17 23:30:44

by Oleksij Rempel

[permalink] [raw]
Subject: Re: [PATCH v3 1/5] ath: add common function ath_is_mybeacon

Am 18.01.2014 00:27, schrieb Oleksij Rempel:
> this function is used by most ath driver, so it can be moved here.
>
> Signed-off-by: Oleksij Rempel <[email protected]>
> ---
> drivers/net/wireless/ath/ath.h | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
> index e0ba7cd..ab6a0c8 100644
> --- a/drivers/net/wireless/ath/ath.h
> +++ b/drivers/net/wireless/ath/ath.h
> @@ -17,6 +17,7 @@
> #ifndef ATH_H
> #define ATH_H
>
> +#include <linux/etherdevice.h>
> #include <linux/skbuff.h>
> #include <linux/if_ether.h>
> #include <linux/spinlock.h>
> @@ -162,10 +163,17 @@ struct ath_common {
> bool bt_ant_diversity;
> };
>
> +static inline bool ath_is_mybeacon(struct ath_common *common,
> + struct ieee80211_hdr *hdr)
> +{
> + return ieee80211_is_beacon(hdr->frame_control) &&
> + !is_zero_ether_addr(common->curbssid) &&
> + ether_addr_equal_64bits(hdr->addr3, common->curbssid);
> +}
> +
> struct sk_buff *ath_rxbuf_alloc(struct ath_common *common,
> u32 len,

If there is no more comments, i'll resend tomorrow this patch set to
Linville.


--
Regards,
Oleksij


Attachments:
signature.asc (295.00 B)
OpenPGP digital signature