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: <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

Powered by Openwall GNU/*/Linux Powered by OpenVZ