Since 'filled' member in station_info changed to u64, BIT_ULL macro
should be used with NL80211_STA_INFO_* attribute types instead of BIT.
The BIT macro uses unsigned long type which some architectures handle as 32bit
and this results in compilation warnings such as:
net/mac80211/sta_info.c:2223:2: warning: left shift count >= width of type
sinfo->filled |= BIT(NL80211_STA_INFO_TID_STATS);
^
Signed-off-by: Omer Efrat <[email protected]>
---
net/mac80211/ethtool.c | 6 ++--
net/mac80211/sta_info.c | 84 ++++++++++++++++++++++++-------------------------
2 files changed, 45 insertions(+), 45 deletions(-)
diff --git a/net/mac80211/ethtool.c b/net/mac80211/ethtool.c
index 690c142..5ac7438 100644
--- a/net/mac80211/ethtool.c
+++ b/net/mac80211/ethtool.c
@@ -116,16 +116,16 @@ static void ieee80211_get_stats(struct net_device *dev,
data[i++] = sta->sta_state;
- if (sinfo.filled & BIT(NL80211_STA_INFO_TX_BITRATE))
+ if (sinfo.filled & BIT_ULL(NL80211_STA_INFO_TX_BITRATE))
data[i] = 100000ULL *
cfg80211_calculate_bitrate(&sinfo.txrate);
i++;
- if (sinfo.filled & BIT(NL80211_STA_INFO_RX_BITRATE))
+ if (sinfo.filled & BIT_ULL(NL80211_STA_INFO_RX_BITRATE))
data[i] = 100000ULL *
cfg80211_calculate_bitrate(&sinfo.rxrate);
i++;
- if (sinfo.filled & BIT(NL80211_STA_INFO_SIGNAL_AVG))
+ if (sinfo.filled & BIT_ULL(NL80211_STA_INFO_SIGNAL_AVG))
data[i] = (u8)sinfo.signal_avg;
i++;
} else {
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 6428f1a..656a838 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -2101,38 +2101,38 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
drv_sta_statistics(local, sdata, &sta->sta, sinfo);
- sinfo->filled |= BIT(NL80211_STA_INFO_INACTIVE_TIME) |
- BIT(NL80211_STA_INFO_STA_FLAGS) |
- BIT(NL80211_STA_INFO_BSS_PARAM) |
- BIT(NL80211_STA_INFO_CONNECTED_TIME) |
- BIT(NL80211_STA_INFO_RX_DROP_MISC);
+ sinfo->filled |= BIT_ULL(NL80211_STA_INFO_INACTIVE_TIME) |
+ BIT_ULL(NL80211_STA_INFO_STA_FLAGS) |
+ BIT_ULL(NL80211_STA_INFO_BSS_PARAM) |
+ BIT_ULL(NL80211_STA_INFO_CONNECTED_TIME) |
+ BIT_ULL(NL80211_STA_INFO_RX_DROP_MISC);
if (sdata->vif.type == NL80211_IFTYPE_STATION) {
sinfo->beacon_loss_count = sdata->u.mgd.beacon_loss_count;
- sinfo->filled |= BIT(NL80211_STA_INFO_BEACON_LOSS);
+ sinfo->filled |= BIT_ULL(NL80211_STA_INFO_BEACON_LOSS);
}
sinfo->connected_time = ktime_get_seconds() - sta->last_connected;
sinfo->inactive_time =
jiffies_to_msecs(jiffies - ieee80211_sta_last_active(sta));
- if (!(sinfo->filled & (BIT(NL80211_STA_INFO_TX_BYTES64) |
- BIT(NL80211_STA_INFO_TX_BYTES)))) {
+ if (!(sinfo->filled & (BIT_ULL(NL80211_STA_INFO_TX_BYTES64) |
+ BIT_ULL(NL80211_STA_INFO_TX_BYTES)))) {
sinfo->tx_bytes = 0;
for (ac = 0; ac < IEEE80211_NUM_ACS; ac++)
sinfo->tx_bytes += sta->tx_stats.bytes[ac];
- sinfo->filled |= BIT(NL80211_STA_INFO_TX_BYTES64);
+ sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_BYTES64);
}
- if (!(sinfo->filled & BIT(NL80211_STA_INFO_TX_PACKETS))) {
+ if (!(sinfo->filled & BIT_ULL(NL80211_STA_INFO_TX_PACKETS))) {
sinfo->tx_packets = 0;
for (ac = 0; ac < IEEE80211_NUM_ACS; ac++)
sinfo->tx_packets += sta->tx_stats.packets[ac];
- sinfo->filled |= BIT(NL80211_STA_INFO_TX_PACKETS);
+ sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_PACKETS);
}
- if (!(sinfo->filled & (BIT(NL80211_STA_INFO_RX_BYTES64) |
- BIT(NL80211_STA_INFO_RX_BYTES)))) {
+ if (!(sinfo->filled & (BIT_ULL(NL80211_STA_INFO_RX_BYTES64) |
+ BIT_ULL(NL80211_STA_INFO_RX_BYTES)))) {
sinfo->rx_bytes += sta_get_stats_bytes(&sta->rx_stats);
if (sta->pcpu_rx_stats) {
@@ -2144,10 +2144,10 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
}
}
- sinfo->filled |= BIT(NL80211_STA_INFO_RX_BYTES64);
+ sinfo->filled |= BIT_ULL(NL80211_STA_INFO_RX_BYTES64);
}
- if (!(sinfo->filled & BIT(NL80211_STA_INFO_RX_PACKETS))) {
+ if (!(sinfo->filled & BIT_ULL(NL80211_STA_INFO_RX_PACKETS))) {
sinfo->rx_packets = sta->rx_stats.packets;
if (sta->pcpu_rx_stats) {
for_each_possible_cpu(cpu) {
@@ -2157,17 +2157,17 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
sinfo->rx_packets += cpurxs->packets;
}
}
- sinfo->filled |= BIT(NL80211_STA_INFO_RX_PACKETS);
+ sinfo->filled |= BIT_ULL(NL80211_STA_INFO_RX_PACKETS);
}
- if (!(sinfo->filled & BIT(NL80211_STA_INFO_TX_RETRIES))) {
+ if (!(sinfo->filled & BIT_ULL(NL80211_STA_INFO_TX_RETRIES))) {
sinfo->tx_retries = sta->status_stats.retry_count;
- sinfo->filled |= BIT(NL80211_STA_INFO_TX_RETRIES);
+ sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_RETRIES);
}
- if (!(sinfo->filled & BIT(NL80211_STA_INFO_TX_FAILED))) {
+ if (!(sinfo->filled & BIT_ULL(NL80211_STA_INFO_TX_FAILED))) {
sinfo->tx_failed = sta->status_stats.retry_failed;
- sinfo->filled |= BIT(NL80211_STA_INFO_TX_FAILED);
+ sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_FAILED);
}
sinfo->rx_dropped_misc = sta->rx_stats.dropped;
@@ -2182,23 +2182,23 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
if (sdata->vif.type == NL80211_IFTYPE_STATION &&
!(sdata->vif.driver_flags & IEEE80211_VIF_BEACON_FILTER)) {
- sinfo->filled |= BIT(NL80211_STA_INFO_BEACON_RX) |
- BIT(NL80211_STA_INFO_BEACON_SIGNAL_AVG);
+ sinfo->filled |= BIT_ULL(NL80211_STA_INFO_BEACON_RX) |
+ BIT_ULL(NL80211_STA_INFO_BEACON_SIGNAL_AVG);
sinfo->rx_beacon_signal_avg = ieee80211_ave_rssi(&sdata->vif);
}
if (ieee80211_hw_check(&sta->local->hw, SIGNAL_DBM) ||
ieee80211_hw_check(&sta->local->hw, SIGNAL_UNSPEC)) {
- if (!(sinfo->filled & BIT(NL80211_STA_INFO_SIGNAL))) {
+ if (!(sinfo->filled & BIT_ULL(NL80211_STA_INFO_SIGNAL))) {
sinfo->signal = (s8)last_rxstats->last_signal;
- sinfo->filled |= BIT(NL80211_STA_INFO_SIGNAL);
+ sinfo->filled |= BIT_ULL(NL80211_STA_INFO_SIGNAL);
}
if (!sta->pcpu_rx_stats &&
- !(sinfo->filled & BIT(NL80211_STA_INFO_SIGNAL_AVG))) {
+ !(sinfo->filled & BIT_ULL(NL80211_STA_INFO_SIGNAL_AVG))) {
sinfo->signal_avg =
-ewma_signal_read(&sta->rx_stats_avg.signal);
- sinfo->filled |= BIT(NL80211_STA_INFO_SIGNAL_AVG);
+ sinfo->filled |= BIT_ULL(NL80211_STA_INFO_SIGNAL_AVG);
}
}
@@ -2207,11 +2207,11 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
* pcpu statistics
*/
if (last_rxstats->chains &&
- !(sinfo->filled & (BIT(NL80211_STA_INFO_CHAIN_SIGNAL) |
- BIT(NL80211_STA_INFO_CHAIN_SIGNAL_AVG)))) {
- sinfo->filled |= BIT(NL80211_STA_INFO_CHAIN_SIGNAL);
+ !(sinfo->filled & (BIT_ULL(NL80211_STA_INFO_CHAIN_SIGNAL) |
+ BIT_ULL(NL80211_STA_INFO_CHAIN_SIGNAL_AVG)))) {
+ sinfo->filled |= BIT_ULL(NL80211_STA_INFO_CHAIN_SIGNAL);
if (!sta->pcpu_rx_stats)
- sinfo->filled |= BIT(NL80211_STA_INFO_CHAIN_SIGNAL_AVG);
+ sinfo->filled |= BIT_ULL(NL80211_STA_INFO_CHAIN_SIGNAL_AVG);
sinfo->chains = last_rxstats->chains;
@@ -2223,15 +2223,15 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
}
}
- if (!(sinfo->filled & BIT(NL80211_STA_INFO_TX_BITRATE))) {
+ if (!(sinfo->filled & BIT_ULL(NL80211_STA_INFO_TX_BITRATE))) {
sta_set_rate_info_tx(sta, &sta->tx_stats.last_rate,
&sinfo->txrate);
- sinfo->filled |= BIT(NL80211_STA_INFO_TX_BITRATE);
+ sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_BITRATE);
}
- if (!(sinfo->filled & BIT(NL80211_STA_INFO_RX_BITRATE))) {
+ if (!(sinfo->filled & BIT_ULL(NL80211_STA_INFO_RX_BITRATE))) {
if (sta_set_rate_info_rx(sta, &sinfo->rxrate) == 0)
- sinfo->filled |= BIT(NL80211_STA_INFO_RX_BITRATE);
+ sinfo->filled |= BIT_ULL(NL80211_STA_INFO_RX_BITRATE);
}
if (tidstats && !cfg80211_sinfo_alloc_tid_stats(sinfo, GFP_KERNEL)) {
@@ -2244,18 +2244,18 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
if (ieee80211_vif_is_mesh(&sdata->vif)) {
#ifdef CONFIG_MAC80211_MESH
- sinfo->filled |= BIT(NL80211_STA_INFO_LLID) |
- BIT(NL80211_STA_INFO_PLID) |
- BIT(NL80211_STA_INFO_PLINK_STATE) |
- BIT(NL80211_STA_INFO_LOCAL_PM) |
- BIT(NL80211_STA_INFO_PEER_PM) |
- BIT(NL80211_STA_INFO_NONPEER_PM);
+ sinfo->filled |= BIT_ULL(NL80211_STA_INFO_LLID) |
+ BIT_ULL(NL80211_STA_INFO_PLID) |
+ BIT_ULL(NL80211_STA_INFO_PLINK_STATE) |
+ BIT_ULL(NL80211_STA_INFO_LOCAL_PM) |
+ BIT_ULL(NL80211_STA_INFO_PEER_PM) |
+ BIT_ULL(NL80211_STA_INFO_NONPEER_PM);
sinfo->llid = sta->mesh->llid;
sinfo->plid = sta->mesh->plid;
sinfo->plink_state = sta->mesh->plink_state;
if (test_sta_flag(sta, WLAN_STA_TOFFSET_KNOWN)) {
- sinfo->filled |= BIT(NL80211_STA_INFO_T_OFFSET);
+ sinfo->filled |= BIT_ULL(NL80211_STA_INFO_T_OFFSET);
sinfo->t_offset = sta->mesh->t_offset;
}
sinfo->local_pm = sta->mesh->local_pm;
@@ -2300,7 +2300,7 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
thr = sta_get_expected_throughput(sta);
if (thr != 0) {
- sinfo->filled |= BIT(NL80211_STA_INFO_EXPECTED_THROUGHPUT);
+ sinfo->filled |= BIT_ULL(NL80211_STA_INFO_EXPECTED_THROUGHPUT);
sinfo->expected_throughput = thr;
}
--
2.7.4
On Thu, 2018-06-14 at 13:11 +0300, Omer Efrat wrote:
> Since 'filled' member in station_info changed to u64, BIT_ULL macro
> should be used with NL80211_STA_INFO_* attribute types instead of BIT.
>
> The BIT macro uses unsigned long type which some architectures handle as 32bit
> and this results in compilation warnings such as:
>
> net/mac80211/sta_info.c:2223:2: warning: left shift count >= width of type
> sinfo->filled |= BIT(NL80211_STA_INFO_TID_STATS);
> ^
It seems like the only change needed is with
BIT(NL80211_STA_INFO_TID_STATS), so I'd argue you should restrict the
patch to that.
Perhaps, though I'm not sure I see it, there's some value in switching
them all so that if you copy something and change it to a new value you
don't run into this problem again, but if anything that should be (a)
separate patch(es) since this one is a bugfix and the others aren't.
johannes
Johannes Berg wrote:
>Perhaps, though I'm not sure I see it, there's some value in switching
>them all so that if you copy something and change it to a new value you
>don't run into this problem again, but if anything that should be (a)
>separate patch(es) since this one is a bugfix and the others aren't.
Exactly my thoughts. I accept the need for the cleanup to be separated
to different patches as well, I will send a v3.
Omer Efrat.
________________________________________
From: Johannes Berg <[email protected]>
Sent: Thursday, June 14, 2018 2:08:05 PM
To: Omer Efrat; [email protected]
Subject: Re: [PATCH v2 2/5] mac80211: use BIT_ULL for NL80211_STA_INFO_* at=
tribute types
On Thu, 2018-06-14 at 13:11 +0300, Omer Efrat wrote:
> Since 'filled' member in station_info changed to u64, BIT_ULL macro
> should be used with NL80211_STA_INFO_* attribute types instead of BIT.
>
> The BIT macro uses unsigned long type which some architectures handle as =
32bit
> and this results in compilation warnings such as:
>
> net/mac80211/sta_info.c:2223:2: warning: left shift count >=3D width of t=
ype
> sinfo->filled |=3D BIT(NL80211_STA_INFO_TID_STATS);
> ^
It seems like the only change needed is with
BIT(NL80211_STA_INFO_TID_STATS), so I'd argue you should restrict the
patch to that.
Perhaps, though I'm not sure I see it, there's some value in switching
them all so that if you copy something and change it to a new value you
don't run into this problem again, but if anything that should be (a)
separate patch(es) since this one is a bugfix and the others aren't.
johannes
Omer Efrat wrote:
>Johannes Berg wrote:
>>Perhaps, though I'm not sure I see it, there's some value in switching
>>them all so that if you copy something and change it to a new value you
>>don't run into this problem again, but if anything that should be (a)
>>separate patch(es) since this one is a bugfix and the others aren't.
>
>Exactly my thoughts. I accept the need for the cleanup to be separated
>to different patches as well, I will send a v3.
Actually, after some more thought, I don't think changing to BIT_ULL for
attribute types less than 32 should be in separated patches because of the =
claim
they are not a bug fix.
This enum already has different numbering in different versions (attributes=
removed from the middle,
i.e. NL80211_STA_INFO_MAX_RSSI).
Therefore, it's hard to mark each of them as "bug fix" or "cleanup only" ch=
ange.
(Some versions has NL80211_STA_INFO_TID_STATS =3D 32, while others has
NL80211_STA_INFO_TID_STATS =3D 31, etc.)
If that's acceptable, I will send a v3 for adding which commit is being fix=
ed
by this patch series.
Best Regards,
Omer Efrat.
________________________________________
From: Omer Efrat
Sent: Thursday, June 14, 2018 3:30:27 PM
To: Johannes Berg; [email protected]
Subject: Re: [PATCH v2 2/5] mac80211: use BIT_ULL for NL80211_STA_INFO_* at=
tribute types
Johannes Berg wrote:
>Perhaps, though I'm not sure I see it, there's some value in switching
>them all so that if you copy something and change it to a new value you
>don't run into this problem again, but if anything that should be (a)
>separate patch(es) since this one is a bugfix and the others aren't.
Exactly my thoughts. I accept the need for the cleanup to be separated
to different patches as well, I will send a v3.
Omer Efrat.
________________________________________
From: Johannes Berg <[email protected]>
Sent: Thursday, June 14, 2018 2:08:05 PM
To: Omer Efrat; [email protected]
Subject: Re: [PATCH v2 2/5] mac80211: use BIT_ULL for NL80211_STA_INFO_* at=
tribute types
On Thu, 2018-06-14 at 13:11 +0300, Omer Efrat wrote:
> Since 'filled' member in station_info changed to u64, BIT_ULL macro
> should be used with NL80211_STA_INFO_* attribute types instead of BIT.
>
> The BIT macro uses unsigned long type which some architectures handle as =
32bit
> and this results in compilation warnings such as:
>
> net/mac80211/sta_info.c:2223:2: warning: left shift count >=3D width of t=
ype
> sinfo->filled |=3D BIT(NL80211_STA_INFO_TID_STATS);
> ^
It seems like the only change needed is with
BIT(NL80211_STA_INFO_TID_STATS), so I'd argue you should restrict the
patch to that.
Perhaps, though I'm not sure I see it, there's some value in switching
them all so that if you copy something and change it to a new value you
don't run into this problem again, but if anything that should be (a)
separate patch(es) since this one is a bugfix and the others aren't.
johannes
Johannes Berg wrote:
>I don't think it is.
>
>The bugfix is certainly legitimate, but I don't want to claim such a
>long patch as the bugfix, with a single compiler warning to show for.
Understood.
>If you prefer, I can do the bugfix separately myself, and then you can
>focus on the remaining patches as cleanups for -next.
I will send both as v3.
Omer.
________________________________________
From: Johannes Berg <[email protected]>
Sent: Thursday, June 14, 2018 7:12:13 PM
To: Omer Efrat; [email protected]
Subject: Re: [PATCH v2 2/5] mac80211: use BIT_ULL for NL80211_STA_INFO_* at=
tribute types
On Thu, 2018-06-14 at 15:55 +0000, Omer Efrat wrote:
> Omer Efrat wrote:
> > Johannes Berg wrote:
> > > Perhaps, though I'm not sure I see it, there's some value in switchin=
g
> > > them all so that if you copy something and change it to a new value y=
ou
> > > don't run into this problem again, but if anything that should be (a)
> > > separate patch(es) since this one is a bugfix and the others aren't.
> >
> > Exactly my thoughts. I accept the need for the cleanup to be separated
> > to different patches as well, I will send a v3.
>
> Actually, after some more thought, I don't think changing to BIT_ULL for
> attribute types less than 32 should be in separated patches because of th=
e claim
> they are not a bug fix.
I disagree, they aren't a bugfix.
> This enum already has different numbering in different versions (attribut=
es removed from the middle,
> i.e. NL80211_STA_INFO_MAX_RSSI).
This must be in some non-upstream tree, because it certainly never
happened in upstream, nor did that attribute (MAX_RSSI) ever exist
there.
> Therefore, it's hard to mark each of them as "bug fix" or "cleanup only" =
change.
> (Some versions has NL80211_STA_INFO_TID_STATS =3D 32, while others has
> NL80211_STA_INFO_TID_STATS =3D 31, etc.)
>
> If that's acceptable, I will send a v3 for adding which commit is being f=
ixed
> by this patch series.
I don't think it is.
The bugfix is certainly legitimate, but I don't want to claim such a
long patch as the bugfix, with a single compiler warning to show for.
If you prefer, I can do the bugfix separately myself, and then you can
focus on the remaining patches as cleanups for -next.
johannes
On Thu, 2018-06-14 at 15:55 +0000, Omer Efrat wrote:
> Omer Efrat wrote:
> > Johannes Berg wrote:
> > > Perhaps, though I'm not sure I see it, there's some value in switching
> > > them all so that if you copy something and change it to a new value you
> > > don't run into this problem again, but if anything that should be (a)
> > > separate patch(es) since this one is a bugfix and the others aren't.
> >
> > Exactly my thoughts. I accept the need for the cleanup to be separated
> > to different patches as well, I will send a v3.
>
> Actually, after some more thought, I don't think changing to BIT_ULL for
> attribute types less than 32 should be in separated patches because of the claim
> they are not a bug fix.
I disagree, they aren't a bugfix.
> This enum already has different numbering in different versions (attributes removed from the middle,
> i.e. NL80211_STA_INFO_MAX_RSSI).
This must be in some non-upstream tree, because it certainly never
happened in upstream, nor did that attribute (MAX_RSSI) ever exist
there.
> Therefore, it's hard to mark each of them as "bug fix" or "cleanup only" change.
> (Some versions has NL80211_STA_INFO_TID_STATS = 32, while others has
> NL80211_STA_INFO_TID_STATS = 31, etc.)
>
> If that's acceptable, I will send a v3 for adding which commit is being fixed
> by this patch series.
I don't think it is.
The bugfix is certainly legitimate, but I don't want to claim such a
long patch as the bugfix, with a single compiler warning to show for.
If you prefer, I can do the bugfix separately myself, and then you can
focus on the remaining patches as cleanups for -next.
johannes