linux-kernel - Re: [PATCH v5 09/17] wifi: cc33xx: Add main.c

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b3446a0d-43e5-47ea-b3b0-f3e81d9c41c0@ti.com>
Date: Sun, 1 Dec 2024 11:47:01 +0200
From: "Nemanov, Michael" <michael.nemanov@...com>
To: Johannes Berg <johannes@...solutions.net>, Kalle Valo <kvalo@...nel.org>,
        "David S . Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, Rob
 Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor
 Dooley <conor+dt@...nel.org>, <linux-wireless@...r.kernel.org>,
        <netdev@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
CC: Sabeeh Khan <sabeeh-khan@...com>
Subject: Re: [PATCH v5 09/17] wifi: cc33xx: Add main.c

On 11/8/2024 1:42 PM, Johannes Berg wrote:
>> +static void cc33xx_op_tx(struct ieee80211_hw *hw,
>> +			 struct ieee80211_tx_control *control,
>> +			 struct sk_buff *skb)
>> +{
>> +	struct cc33xx *cc = hw->priv;
>> +	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
>> +	struct ieee80211_vif *vif = info->control.vif;
>> +	struct cc33xx_vif *wlvif = NULL;
>> +	enum cc33xx_queue_stop_reason stop_reason = CC33XX_QUEUE_STOP_REASON_WATERMARK;
>> +	unsigned long flags;
>> +	int q, mapping;
>> +	u8 hlid;
>> +
>> +	if (!vif) {
>> +		ieee80211_free_txskb(hw, skb);
>> +		return;
>> +	}
>> +
>> +	wlvif = cc33xx_vif_to_data(vif);
>> +	mapping = skb_get_queue_mapping(skb);
>> +	q = cc33xx_tx_get_queue(mapping);
>> +
>> +	hlid = cc33xx_tx_get_hlid(cc, wlvif, skb, control->sta);
>> +
>> +	spin_lock_irqsave(&cc->cc_lock, flags);
>> +
>> +	/* drop the packet if the link is invalid or the queue is stopped
>> +	 * for any reason but watermark. Watermark is a "soft"-stop so we
>> +	 * allow these packets through.
>> +	 */
>> +
>> +	if (hlid == CC33XX_INVALID_LINK_ID ||
>> +	    (!test_bit(hlid, wlvif->links_map)) ||
>> +	    (cc33xx_is_queue_stopped_locked(cc, wlvif, q) &&
>> +	    !cc33xx_is_queue_stopped_by_reason_locked(cc, wlvif, q,
>> +						      stop_reason))) {
>> +		cc33xx_debug(DEBUG_TX, "DROP skb hlid %d q %d ", hlid, q);
>> +		ieee80211_free_txskb(hw, skb);
>> +		goto out;
>> +	}
> 
> I'd consider converting to itxq APIs, you already use them anyway via
> ieee80211_handle_wake_tx_queue so you don't gain anything from not doing
> it, but you gain a lot of flexibility from doing it and don't have to do
> things like this?
> 
> It's not _that_ hard.

OK, so just to make sure I understand - mac80211 now has Tx queues per 
AC (struct ieee80211_txq) and it makes more sense to do something like 
ath10k ([1])?

Frames pushed via original Tx op are non-QoS traffic, right? (i.e, no 
need to worry about frame order between the two handlers)

Thank and regards,
Michael.

[1] 
https://elixir.bootlin.com/linux/v6.12/source/drivers/net/wireless/ath/ath10k/mac.c#L4728