Re: [PATCH] nl80211/cfg80211: Add support to send tx frames at specified rate

From: Ben Greear <greearb@candelatech.com>
To: Johannes Berg <johannes@sipsolutions.net>,
	vamsi krishna <vamsin@codeaurora.org>
Cc: linux-wireless@vger.kernel.org, jouni@codeaurora.org
Subject: Re: [PATCH] nl80211/cfg80211: Add support to send tx frames at specified rate
Date: Tue, 18 Dec 2018 05:45:32 -0800	[thread overview]
Message-ID: <899698ca-d8b5-45b1-5689-58096e2f0291@candelatech.com> (raw)
In-Reply-To: <89e6532daf4011de2ad0df3dcb8accc42334f31e.camel@sipsolutions.net>

On 12/18/2018 05:12 AM, Johannes Berg wrote:
> On Mon, 2018-12-03 at 17:34 +0530, vamsi krishna wrote:
>> NL80211_CMD_FRAME is used to send frames from userspace. Add support to
>> transmit the frames at a rate specified by userspace when needed.
>> The drivers shall indicate the support to send frames at rate specified
>> by userspace by setting %NL80211_EXT_FEATURE_CMD_FRAME_TXRATE flag in
>> wiphy capabilities. The userspace can specify the rate within
>> %NL80211_ATTR_RATE_INFO attribute while sending %NL80211_CMD_FRAME.
>>
>> NL80211_ATTR_RATE_INFO is a nested attribute and encapsulates the
>> attributes defined in &enum nl80211_rate_info.
>
> It'd be good if you were to repost this with a driver that uses it.
>
> Also, please explain why you think userspace needs this? It's not like
> it can make better rate decisions than the rate control algorithm,
> right?
>
>>  	[NL80211_ATTR_SCHED_SCAN_MIN_RSSI] = { .type = NLA_NESTED },
>> +	[NL80211_ATTR_RATE_INFO] = { .type = NLA_NESTED },
>
> This should use NLA_POLICY_NESTED(nl80211_rate_info_policy)

I missed the first posting of this patch.  Here are some of my own
suggestions.

The wave-2 ath10k firmware has an option to specify tx-power, preamble
(CCK, OFDM, HT, VHT), bandwidth, mcs, nss and retry-count on a per-packet
basis.  This is not enabled in the ath10k driver, and I am not certain QCA
firmware compiles in this option, but I have both ath10k-ct driver and firmware
able to do this currently.

For HT and VHT, the mcs is fairly obvious I think.  For CCK and OFDM, the
mcs is treated as an index into the CCK and OFDM rate tables.

So, maybe the patch could use these basic constructs instead of the
different flags/rates for MCS vs VHT_MCS vs HE_MCS?

Or, do you want to be able to specify multiple different tx rates per
frame?  wave-2 firmware that I have seen wouldn't support that, but I guess newer stuff
could?

Either way, please at least consider adding retry-count and tx-power to this API and I
can tie it into my ath10k-ct driver and firmware API.

As for purpose, I use this for testing, though someone could probably use the
API to make it use a somewhat minimal rate-ctrl in the kernel instead of firmware if they wanted
(ignoring the complexities of MU-MIMO and such).

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com