Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <d1ef101320747b30879153e5314abf13c8202578.camel@sipsolutions.net>
Subject: Re: [PATCH] mac80211: add support for restricting netdev features
 per vif
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Felix Fietkau <nbd@nbd.name>, linux-wireless@vger.kernel.org
Date:   Mon, 10 Oct 2022 10:38:35 +0200
In-Reply-To: <20221009183334.35841-1-nbd@nbd.name>
References: <20221009183334.35841-1-nbd@nbd.name>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.4 (3.44.4-2.fc36) 
MIME-Version: 1.0
Precedence: bulk

On Sun, 2022-10-09 at 20:33 +0200, Felix Fietkau wrote:
>=20
> +static void __ieee80211_xmit_fast(struct ieee80211_sub_if_data *sdata,
> +				  struct sta_info *sta,
> +				  struct ieee80211_fast_tx *fast_tx,
> +				  struct sk_buff *skb, u8 tid, bool ampdu)
> +{
> +	struct ieee80211_local *local =3D sdata->local;
> +	struct ieee80211_hdr *hdr =3D (void *)fast_tx->hdr;
> +	struct ieee80211_tx_info *info;
> +	struct ieee80211_tx_data tx;
> +	ieee80211_tx_result r;
> +	int hw_headroom =3D sdata->local->hw.extra_tx_headroom;
> +	int extra_head =3D fast_tx->hdr_len - (ETH_HLEN - 2);
> +	struct ethhdr eth;
> +
>  	/* after this point (skb is modified) we cannot return false */

That's now void so the comment is a bit misleading ... move it somewhere
else?

> +static bool ieee80211_xmit_fast(struct ieee80211_sub_if_data *sdata,
> +				struct sta_info *sta,
> +				struct ieee80211_fast_tx *fast_tx,
> +				struct sk_buff *skb)
> +{
> +	u16 ethertype =3D (skb->data[12] << 8) | skb->data[13];
> +	struct ieee80211_hdr *hdr =3D (void *)fast_tx->hdr;
> +	struct tid_ampdu_tx *tid_tx =3D NULL;
> +	struct sk_buff *next;
> +	u8 tid =3D IEEE80211_NUM_TIDS;
> +
> +	/* control port protocol needs a lot of special handling */
> +	if (cpu_to_be16(ethertype) =3D=3D sdata->control_port_protocol)
> +		return false;
> +
> +	/* only RFC 1042 SNAP */
> +	if (ethertype < ETH_P_802_3_MIN)
> +		return false;
> +
> +	/* don't handle TX status request here either */
> +	if (skb->sk && skb_shinfo(skb)->tx_flags & SKBTX_WIFI_STATUS)
> +		return false;
> +
> +	if (hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_QOS_DATA)) {
> +		tid =3D skb->priority & IEEE80211_QOS_CTL_TAG1D_MASK;
> +		tid_tx =3D rcu_dereference(sta->ampdu_mlme.tid_tx[tid]);
> +		if (tid_tx) {
> +			if (!test_bit(HT_AGG_STATE_OPERATIONAL, &tid_tx->state))
> +				return false;
> +			if (tid_tx->timeout)
> +				tid_tx->last_tx =3D jiffies;
> +		}
> +	}
>=20

Here, I guess, or so?


Another advantage _might_ be that you can create A-MSDUs now with frames
that don't just belong to a single stream, unlike driver-created A-MSDUs
from GSO? But then again I'm not really sure anymore how the A-MSDU
creation works in the first place.

johannes