Received-SPF: pass (google.com: domain of linux-wireless+bounces-2971-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <acffd9a8-062a-4deb-a996-03fd71d3e357@quicinc.com>
Date: Thu, 1 Feb 2024 10:21:29 -0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] wifi: mac80211: trace SMPS requests from driver
Content-Language: en-US
To: Johannes Berg <johannes@sipsolutions.net>,
        <linux-wireless@vger.kernel.org>
CC: Johannes Berg <johannes.berg@intel.com>
References: <20240129195435.b20d2ead2013.I8213e65c274451d523a3397519ac578c3ed2df4d@changeid>
From: Jeff Johnson <quic_jjohnson@quicinc.com>
In-Reply-To: <20240129195435.b20d2ead2013.I8213e65c274451d523a3397519ac578c3ed2df4d@changeid>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit

On 1/29/2024 10:54 AM, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> Even if there are a lot of possible ways drivers might
> call this, at least knowing when they do and with what
> settings can be useful. Add tracing for it.
> 
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
>  net/mac80211/ht.c    |  4 +++-
>  net/mac80211/trace.h | 31 ++++++++++++++++++++++++++++++-
>  2 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c
> index cfe2653ed47f..c3330aea4da3 100644
> --- a/net/mac80211/ht.c
> +++ b/net/mac80211/ht.c
> @@ -9,7 +9,7 @@
>   * Copyright 2007, Michael Wu <flamingice@sourmilk.net>
>   * Copyright 2007-2010, Intel Corporation
>   * Copyright 2017	Intel Deutschland GmbH
> - * Copyright(c) 2020-2023 Intel Corporation
> + * Copyright(c) 2020-2024 Intel Corporation
>   */
>  
>  #include <linux/ieee80211.h>
> @@ -603,6 +603,8 @@ void ieee80211_request_smps(struct ieee80211_vif *vif, unsigned int link_id,
>  	if (WARN_ON(!link))
>  		goto out;
>  
> +	trace_api_request_smps(sdata->local, sdata, link, smps_mode);
> +
>  	if (link->u.mgd.driver_smps_mode == smps_mode)
>  		goto out;
>  
> diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h
> index 2d0d969f0c3d..806e762aa546 100644
> --- a/net/mac80211/trace.h
> +++ b/net/mac80211/trace.h
> @@ -2,7 +2,7 @@
>  /*
>   * Portions of this file
>   * Copyright(c) 2016-2017 Intel Deutschland GmbH
> - * Copyright (C) 2018 - 2023 Intel Corporation
> + * Copyright (C) 2018 - 2024 Intel Corporation
>   */
>  
>  #if !defined(__MAC80211_DRIVER_TRACE) || defined(TRACE_HEADER_MULTI_READ)
> @@ -3058,6 +3058,35 @@ TRACE_EVENT(api_radar_detected,
>  	)
>  );
>  
> +TRACE_EVENT(api_request_smps,
> +	TP_PROTO(struct ieee80211_local *local,
> +		 struct ieee80211_sub_if_data *sdata,
> +		 struct ieee80211_link_data *link,
> +		 enum ieee80211_smps_mode smps_mode),
> +
> +	TP_ARGS(local, sdata, link, smps_mode),
> +
> +	TP_STRUCT__entry(
> +		LOCAL_ENTRY
> +		VIF_ENTRY
> +		__field(int, link_id)
> +		__field(u32, smps_mode)
> +	),
> +
> +	TP_fast_assign(
> +		LOCAL_ASSIGN;
> +		VIF_ASSIGN;
> +		__entry->link_id =
> +			ieee80211_vif_is_mld(&sdata->vif) ? link->link_id : -1;

why go to this trouble?
why not just print the link_id that was passed into
ieee80211_request_smps()? just so non-MLD will give -1 instead of 0?
seems all of the other existing trace functions just print the link_id
that was provided


> +		__entry->smps_mode = smps_mode;
> +	),
> +
> +	TP_printk(
> +		LOCAL_PR_FMT " " VIF_PR_FMT " link:%d, smps_mode:%d",
> +		LOCAL_PR_ARG, VIF_PR_ARG, __entry->link_id, __entry->smps_mode
> +	)
> +);
> +
>  /*
>   * Tracing for internal functions
>   * (which may also be called in response to driver calls)