[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAL+tcoB3zku2EtLT2yJ9qPCSuN2=x-T-avqcZ-LJ2Q-mU5xLVg@mail.gmail.com>
Date: Tue, 25 Mar 2025 00:53:21 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Johannes Berg <johannes@...solutions.net>
Cc: linux-wireless@...r.kernel.org, netdev@...r.kernel.org,
Johannes Berg <johannes.berg@...el.com>
Subject: Re: [RFC PATCH] wifi: free SKBTX_WIFI_STATUS skb tx_flags flag
On Thu, Mar 13, 2025 at 9:01 PM Johannes Berg <johannes@...solutions.net> wrote:
>
> From: Johannes Berg <johannes.berg@...el.com>
>
> Someone mentioned today at netdevconf that we've run out of
> tx_flags in the skb_shinfo(). Gain one bit back by removing
> the wifi bit. We should be able to do that because the only
> userspace application for it (hostapd) doesn't change the
> setting on the socket, it just uses different sockets, and
> normally doesn't even use this any more, sending the frames
> over nl80211 instead.
>
> Signed-off-by: Johannes Berg <johannes.berg@...el.com>
Thanks for working on this. After net-next is open, I will use this
bit to finish the bpf timestamping in the rx path :)
Reviewed-by: Jason Xing <kerneljasonxing@...il.com>
> ---
> drivers/net/wireless/ath/wil6210/txrx.h | 3 ++-
> drivers/net/wireless/marvell/mwifiex/main.c | 3 ++-
> include/linux/skbuff.h | 3 ---
> include/net/sock.h | 2 --
> net/mac80211/mesh.c | 3 ++-
> net/mac80211/tx.c | 9 ++++-----
> 6 files changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/wil6210/txrx.h b/drivers/net/wireless/ath/wil6210/txrx.h
> index 689f68d89a44..33ccd0b248d4 100644
> --- a/drivers/net/wireless/ath/wil6210/txrx.h
> +++ b/drivers/net/wireless/ath/wil6210/txrx.h
> @@ -7,6 +7,7 @@
> #ifndef WIL6210_TXRX_H
> #define WIL6210_TXRX_H
>
> +#include <net/sock.h>
> #include "wil6210.h"
> #include "txrx_edma.h"
>
> @@ -617,7 +618,7 @@ static inline bool wil_need_txstat(struct sk_buff *skb)
> const u8 *da = wil_skb_get_da(skb);
>
> return is_unicast_ether_addr(da) && skb->sk &&
> - (skb_shinfo(skb)->tx_flags & SKBTX_WIFI_STATUS);
> + sock_flag(skb->sk, SOCK_WIFI_STATUS);
> }
>
> static inline void wil_consume_skb(struct sk_buff *skb, bool acked)
> diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
> index 45eecb5f643b..058687793a10 100644
> --- a/drivers/net/wireless/marvell/mwifiex/main.c
> +++ b/drivers/net/wireless/marvell/mwifiex/main.c
> @@ -6,6 +6,7 @@
> */
>
> #include <linux/suspend.h>
> +#include <net/sock.h>
>
> #include "main.h"
> #include "wmm.h"
> @@ -943,7 +944,7 @@ mwifiex_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
> multicast = is_multicast_ether_addr(skb->data);
>
> if (unlikely(!multicast && skb->sk &&
> - skb_shinfo(skb)->tx_flags & SKBTX_WIFI_STATUS &&
> + sock_flag(skb->sk, SOCK_WIFI_STATUS) &&
> priv->adapter->fw_api_ver == MWIFIEX_FW_V15))
> skb = mwifiex_clone_skb_for_tx_status(priv,
> skb,
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 14517e95a46c..a8638c8a53b4 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -481,9 +481,6 @@ enum {
> /* reserved */
> SKBTX_RESERVED = 1 << 3,
It might conflict with the bluetooth commit [1], I presume.
[1]: https://web.git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/commit/?id=c6c3dc495a6ce5b9dfed4df08c3220207e7103bd
>
> - /* generate wifi status information (where possible) */
> - SKBTX_WIFI_STATUS = 1 << 4,
> -
Better use SKBTX_RESERVED. No strong preference here since I'm going to use it.
> /* determine hardware time stamp based on time or cycles */
> SKBTX_HW_TSTAMP_NETDEV = 1 << 5,
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 8daf1b3b12c6..2668c3ed45ef 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -2700,8 +2700,6 @@ static inline void _sock_tx_timestamp(struct sock *sk,
> *tskey = atomic_inc_return(&sk->sk_tskey) - 1;
> }
> }
> - if (unlikely(sock_flag(sk, SOCK_WIFI_STATUS)))
> - *tx_flags |= SKBTX_WIFI_STATUS;
> }
>
> static inline void sock_tx_timestamp(struct sock *sk,
> diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
> index 974081324aa4..e77e623c8b77 100644
> --- a/net/mac80211/mesh.c
> +++ b/net/mac80211/mesh.c
> @@ -8,6 +8,7 @@
>
> #include <linux/slab.h>
> #include <linux/unaligned.h>
> +#include <net/sock.h>
> #include "ieee80211_i.h"
> #include "mesh.h"
> #include "wme.h"
> @@ -776,7 +777,7 @@ bool ieee80211_mesh_xmit_fast(struct ieee80211_sub_if_data *sdata,
> if (ethertype < ETH_P_802_3_MIN)
> return false;
>
> - if (skb->sk && skb_shinfo(skb)->tx_flags & SKBTX_WIFI_STATUS)
> + if (skb->sk && sock_flag(skb->sk, SOCK_WIFI_STATUS))
> return false;
>
> if (skb->ip_summed == CHECKSUM_PARTIAL) {
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index 20179db88c4a..b75f72fbefd9 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -26,6 +26,7 @@
> #include <net/codel_impl.h>
> #include <linux/unaligned.h>
> #include <net/fq_impl.h>
> +#include <net/sock.h>
> #include <net/gso.h>
>
> #include "ieee80211_i.h"
> @@ -2876,8 +2877,7 @@ static struct sk_buff *ieee80211_build_hdr(struct ieee80211_sub_if_data *sdata,
> }
>
> if (unlikely(!multicast &&
> - ((skb->sk &&
> - skb_shinfo(skb)->tx_flags & SKBTX_WIFI_STATUS) ||
> + ((skb->sk && sock_flag(skb->sk, SOCK_WIFI_STATUS)) ||
> ctrl_flags & IEEE80211_TX_CTL_REQ_TX_STATUS)))
> info_id = ieee80211_store_ack_skb(local, skb, &info_flags,
> cookie);
> @@ -3774,7 +3774,7 @@ static bool ieee80211_xmit_fast(struct ieee80211_sub_if_data *sdata,
> return false;
>
> /* don't handle TX status request here either */
> - if (skb->sk && skb_shinfo(skb)->tx_flags & SKBTX_WIFI_STATUS)
> + if (skb->sk && sock_flag(skb->sk, SOCK_WIFI_STATUS))
> return false;
>
> if (hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_QOS_DATA)) {
> @@ -4664,8 +4664,7 @@ static void ieee80211_8023_xmit(struct ieee80211_sub_if_data *sdata,
> memcpy(IEEE80211_SKB_CB(seg), info, sizeof(*info));
> }
>
> - if (unlikely(skb->sk &&
> - skb_shinfo(skb)->tx_flags & SKBTX_WIFI_STATUS)) {
> + if (unlikely(skb->sk && sock_flag(skb->sk, SOCK_WIFI_STATUS))) {
> info->status_data = ieee80211_store_ack_skb(local, skb,
> &info->flags, NULL);
> if (info->status_data)
> --
> 2.48.1
>
>
Powered by blists - more mailing lists