All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.