linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arend van Spriel <arend.vanspriel@broadcom.com>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: "Toke Høiland-Jørgensen" <toke@toke.dk>,
	linux-wireless <linux-wireless@vger.kernel.org>
Subject: [RESEND PATCH] cfg80211: dynamically allocate per-tid stats for station info
Date: Thu, 10 May 2018 13:50:12 +0200	[thread overview]
Message-ID: <5AF431F4.9060408@broadcom.com> (raw)
In-Reply-To: <5AF430B2.2060903@broadcom.com>

From: Arend van Spriel <aspriel@gmail.com>

With the addition of TXQ stats in the per-tid statistics the struct
station_info grew significantly. This resulted in stack limit warnings
like below:

   CC [M]  drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.o
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:
	In function ‘brcmf_notify_connect_status_ap’:
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:5530:1:
	error: the frame size of 1592 bytes is larger than 1024 bytes

This patch adds an allocation function that those who want to provide
per-tid stats should use to allocate the tid array, ie.
struct station_info::pertid.

Cc: Toke Høiland-Jørgensen <toke@toke.dk>
Fixes: 52539ca89f36 ("cfg80211: Expose TXQ stats and parameters to
userspace")
Signed-off-by: Arend van Spriel <aspriel@gmail.com>
---
+ linux-wireless list

Johannes, Toke,

Here an alternative approach. Currently the only cfg80211-based driver
providing per-tid stats is mac80211. This patch only changes mac80211
and the other driver can keep using stack allocation. Even mac80211 could
if wanted, but I left that part as is.

I considered making the new helper more generic and allocate 'pertid' based
on filled flags, ie.:

int cfg80211_sinfo_prepare(struct station_info *sinfo, gfp_t gfp)
{
	:
	if (sinfo->filled & BIT(NL80211_STA_INFO_TID_STATS)) {
		sinfo->pertid = kzalloc(..., gfp);
		if (!pertid)
			return -ENOMEM;
	}
	:
	return 0;
}

but decided to stick close to the issue.

Regards,
Arend
---
  include/net/cfg80211.h  |  9 ++++++++-
  net/mac80211/sta_info.c | 11 ++++++-----
  net/wireless/nl80211.c  |  4 +++-
  net/wireless/util.c     | 12 ++++++++++++
  4 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 8db6071..784377a 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1230,7 +1230,7 @@ struct station_info {
  	u64 rx_beacon;
  	u64 rx_duration;
  	u8 rx_beacon_signal_avg;
-	struct cfg80211_tid_stats pertid[IEEE80211_NUM_TIDS + 1];
+	struct cfg80211_tid_stats *pertid;
  	s8 ack_signal;
  	s8 avg_ack_signal;
  };
@@ -5701,6 +5701,13 @@ void cfg80211_remain_on_channel_expired(struct
wireless_dev *wdev, u64 cookie,
  					struct ieee80211_channel *chan,
  					gfp_t gfp);

+/**
+ * cfg80211_sinfo_alloc_tid_stats - allocate per-tid statistics.
+ *
+ * @sinfo: the station information
+ * @gfp: allocation flags
+ */
+int cfg80211_sinfo_alloc_tid_stats(struct station_info *sinfo, gfp_t gfp);

  /**
   * cfg80211_new_sta - notify userspace about station
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 43f34aa..25e8b15 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -2233,11 +2233,12 @@ void sta_set_sinfo(struct sta_info *sta, struct
station_info *sinfo)
  			sinfo->filled |= BIT(NL80211_STA_INFO_RX_BITRATE);
  	}

-	sinfo->filled |= BIT(NL80211_STA_INFO_TID_STATS);
-	for (i = 0; i < IEEE80211_NUM_TIDS + 1; i++) {
-		struct cfg80211_tid_stats *tidstats = &sinfo->pertid[i];
-
-		sta_set_tidstats(sta, tidstats, i);
+	if (!cfg80211_sinfo_alloc_tid_stats(sinfo, GFP_KERNEL)) {
+		for (i = 0; i < IEEE80211_NUM_TIDS + 1; i++) {
+			struct cfg80211_tid_stats *tidstats = &sinfo->pertid[i];
+	
+			sta_set_tidstats(sta, tidstats, i);
+		}
  	}

  	if (ieee80211_vif_is_mesh(&sdata->vif)) {
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index f7715b8..e51cae7 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -4663,7 +4663,7 @@ static int nl80211_send_station(struct sk_buff
*msg, u32 cmd, u32 portid,
  		int tid;

  		tidsattr = nla_nest_start(msg, NL80211_STA_INFO_TID_STATS);
-		if (!tidsattr)
+		if (!tidsattr || !sinfo->pertid)
  			goto nla_put_failure;

  		for (tid = 0; tid < IEEE80211_NUM_TIDS + 1; tid++) {
@@ -4702,6 +4702,8 @@ static int nl80211_send_station(struct sk_buff
*msg, u32 cmd, u32 portid,
  		}

  		nla_nest_end(msg, tidsattr);
+
+		kfree(sinfo->pertid);
  	}

  	nla_nest_end(msg, sinfoattr);
diff --git a/net/wireless/util.c b/net/wireless/util.c
index d112e9a..b956c23fe 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -1750,6 +1750,18 @@ int cfg80211_get_station(struct net_device *dev,
const u8 *mac_addr,
  }
  EXPORT_SYMBOL(cfg80211_get_station);

+int cfg80211_sinfo_alloc_tid_stats(struct station_info *sinfo, gfp_t gfp)
+{
+	sinfo->pertid = kcalloc(sizeof(*(sinfo->pertid)),
+				IEEE80211_NUM_TIDS + 1, gfp);
+	if (!sinfo->pertid)
+		return -ENOMEM;
+	
+	sinfo->filled |= NL80211_STA_INFO_TID_STATS;
+	return 0;
+}
+EXPORT_SYMBOL(cfg80211_sinfo_alloc_tid_stats);
+
  void cfg80211_free_nan_func(struct cfg80211_nan_func *f)
  {
  	int i;
-- 
2.7.4

       reply	other threads:[~2018-05-10 11:50 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <5AF430B2.2060903@broadcom.com>
2018-05-10 11:50 ` Arend van Spriel [this message]
2018-05-10 13:02   ` [RESEND PATCH] cfg80211: dynamically allocate per-tid stats for station info Toke Høiland-Jørgensen
2018-05-18  9:25   ` Johannes Berg
2018-05-18  9:37     ` Arend van Spriel
2018-05-18  9:38       ` Johannes Berg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5AF431F4.9060408@broadcom.com \
    --to=arend.vanspriel@broadcom.com \
    --cc=johannes@sipsolutions.net \
    --cc=linux-wireless@vger.kernel.org \
    --cc=toke@toke.dk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).