netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH] wifi: free SKBTX_WIFI_STATUS skb tx_flags flag
@ 2025-03-13 12:49 Johannes Berg
  2025-03-24 15:53 ` Jakub Kicinski
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Johannes Berg @ 2025-03-13 12:49 UTC (permalink / raw)
  To: linux-wireless, netdev; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.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@intel.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,
 
-	/* generate wifi status information (where possible) */
-	SKBTX_WIFI_STATUS = 1 << 4,
-
 	/* 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH] wifi: free SKBTX_WIFI_STATUS skb tx_flags flag
  2025-03-13 12:49 [RFC PATCH] wifi: free SKBTX_WIFI_STATUS skb tx_flags flag Johannes Berg
@ 2025-03-24 15:53 ` Jakub Kicinski
  2025-03-24 15:57   ` Johannes Berg
  2025-03-24 16:33   ` Jason Xing
  2025-03-24 16:53 ` Jason Xing
  2025-04-26 23:55 ` Jason Xing
  2 siblings, 2 replies; 10+ messages in thread
From: Jakub Kicinski @ 2025-03-24 15:53 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, netdev, Johannes Berg

On Thu, 13 Mar 2025 13:49:39 +0100 Johannes Berg wrote:
> Someone mentioned today at netdevconf that we've run out of

Don't recall the mention, but I'd guess maybe Jason Xing during
his timestamping talk?  Change itself LGTM!

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH] wifi: free SKBTX_WIFI_STATUS skb tx_flags flag
  2025-03-24 15:53 ` Jakub Kicinski
@ 2025-03-24 15:57   ` Johannes Berg
  2025-03-24 16:12     ` Jakub Kicinski
  2025-03-24 16:33   ` Jason Xing
  1 sibling, 1 reply; 10+ messages in thread
From: Johannes Berg @ 2025-03-24 15:57 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: linux-wireless, netdev

On Mon, 2025-03-24 at 08:53 -0700, Jakub Kicinski wrote:
> On Thu, 13 Mar 2025 13:49:39 +0100 Johannes Berg wrote:
> > Someone mentioned today at netdevconf that we've run out of
> 
> Don't recall the mention, but I'd guess maybe Jason Xing during
> his timestamping talk?  Change itself LGTM!

Yeah, I think so. Timing would be right too, his talk was around noon
that day :)

I guess I can merge it through wireless for 6.16, unless you have any
concerns? I can also resend it for net-next after the merge window
closes, the wireless drivers/stack changes are almost certainly not
going to have any conflicts, but overall it's trivial anyway.

johannes

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH] wifi: free SKBTX_WIFI_STATUS skb tx_flags flag
  2025-03-24 15:57   ` Johannes Berg
@ 2025-03-24 16:12     ` Jakub Kicinski
  0 siblings, 0 replies; 10+ messages in thread
From: Jakub Kicinski @ 2025-03-24 16:12 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, netdev

On Mon, 24 Mar 2025 16:57:52 +0100 Johannes Berg wrote:
> On Mon, 2025-03-24 at 08:53 -0700, Jakub Kicinski wrote:
> > On Thu, 13 Mar 2025 13:49:39 +0100 Johannes Berg wrote:  
> > > Someone mentioned today at netdevconf that we've run out of  
> > 
> > Don't recall the mention, but I'd guess maybe Jason Xing during
> > his timestamping talk?  Change itself LGTM!  
> 
> Yeah, I think so. Timing would be right too, his talk was around noon
> that day :)
> 
> I guess I can merge it through wireless for 6.16, unless you have any
> concerns? I can also resend it for net-next after the merge window
> closes, the wireless drivers/stack changes are almost certainly not
> going to have any conflicts, but overall it's trivial anyway.

6.16 makes sense; no preference on the tree, whichever's easier.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH] wifi: free SKBTX_WIFI_STATUS skb tx_flags flag
  2025-03-24 15:53 ` Jakub Kicinski
  2025-03-24 15:57   ` Johannes Berg
@ 2025-03-24 16:33   ` Jason Xing
  1 sibling, 0 replies; 10+ messages in thread
From: Jason Xing @ 2025-03-24 16:33 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: Johannes Berg, linux-wireless, netdev, Johannes Berg

On Mon, Mar 24, 2025 at 11:55 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 13 Mar 2025 13:49:39 +0100 Johannes Berg wrote:
> > Someone mentioned today at netdevconf that we've run out of
>
> Don't recall the mention, but I'd guess maybe Jason Xing during
> his timestamping talk?  Change itself LGTM!

Yes, I did mention it. Sorry for missing this commit.

Considering it's too late for me, I promise I will review tomorrow morning.

Thanks,
Jason

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH] wifi: free SKBTX_WIFI_STATUS skb tx_flags flag
  2025-03-13 12:49 [RFC PATCH] wifi: free SKBTX_WIFI_STATUS skb tx_flags flag Johannes Berg
  2025-03-24 15:53 ` Jakub Kicinski
@ 2025-03-24 16:53 ` Jason Xing
  2025-03-24 16:56   ` Johannes Berg
  2025-04-26 23:55 ` Jason Xing
  2 siblings, 1 reply; 10+ messages in thread
From: Jason Xing @ 2025-03-24 16:53 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, netdev, Johannes Berg

On Thu, Mar 13, 2025 at 9:01 PM Johannes Berg <johannes@sipsolutions.net> wrote:
>
> From: Johannes Berg <johannes.berg@intel.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@intel.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@gmail.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
>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH] wifi: free SKBTX_WIFI_STATUS skb tx_flags flag
  2025-03-24 16:53 ` Jason Xing
@ 2025-03-24 16:56   ` Johannes Berg
  2025-03-24 16:59     ` Jason Xing
  0 siblings, 1 reply; 10+ messages in thread
From: Johannes Berg @ 2025-03-24 16:56 UTC (permalink / raw)
  To: Jason Xing; +Cc: linux-wireless, netdev

On Tue, 2025-03-25 at 00:53 +0800, Jason Xing wrote:
> 
> Thanks for working on this. After net-next is open, I will use this
> bit to finish the bpf timestamping in the rx path :)

:)

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

True, just context though, we can deal with that.

> > 
> > -       /* 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.

I can't have two called SKBTX_RESERVED, but I'm not sure it's worth
renaming it rather than removing? No strong opinion though. The context
conflict will happen either way ;)

johannes


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH] wifi: free SKBTX_WIFI_STATUS skb tx_flags flag
  2025-03-24 16:56   ` Johannes Berg
@ 2025-03-24 16:59     ` Jason Xing
  0 siblings, 0 replies; 10+ messages in thread
From: Jason Xing @ 2025-03-24 16:59 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, netdev

On Tue, Mar 25, 2025 at 12:56 AM Johannes Berg
<johannes@sipsolutions.net> wrote:
>
> On Tue, 2025-03-25 at 00:53 +0800, Jason Xing wrote:
> >
> > Thanks for working on this. After net-next is open, I will use this
> > bit to finish the bpf timestamping in the rx path :)
>
> :)
>
> > > --- 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
>
> True, just context though, we can deal with that.
>
> > >
> > > -       /* 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.
>
> I can't have two called SKBTX_RESERVED, but I'm not sure it's worth
> renaming it rather than removing? No strong opinion though. The context
> conflict will happen either way ;)

Sure, we can ignore it. As I mentioned, this released bit will be used soon :)

>
> johannes
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH] wifi: free SKBTX_WIFI_STATUS skb tx_flags flag
  2025-03-13 12:49 [RFC PATCH] wifi: free SKBTX_WIFI_STATUS skb tx_flags flag Johannes Berg
  2025-03-24 15:53 ` Jakub Kicinski
  2025-03-24 16:53 ` Jason Xing
@ 2025-04-26 23:55 ` Jason Xing
  2025-04-28  7:41   ` Johannes Berg
  2 siblings, 1 reply; 10+ messages in thread
From: Jason Xing @ 2025-04-26 23:55 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, netdev, Johannes Berg

Hi Johannes,

On Thu, Mar 13, 2025 at 9:01 PM Johannes Berg <johannes@sipsolutions.net> wrote:
>
> From: Johannes Berg <johannes.berg@intel.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@intel.com>

Do you have plans to repost it recently in net-next? I'm going to
implement the rx path for BFP timestamping based on this patch :)

Thanks,
Jason

> ---
>  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,
>
> -       /* generate wifi status information (where possible) */
> -       SKBTX_WIFI_STATUS = 1 << 4,
> -
>         /* 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
>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH] wifi: free SKBTX_WIFI_STATUS skb tx_flags flag
  2025-04-26 23:55 ` Jason Xing
@ 2025-04-28  7:41   ` Johannes Berg
  0 siblings, 0 replies; 10+ messages in thread
From: Johannes Berg @ 2025-04-28  7:41 UTC (permalink / raw)
  To: Jason Xing; +Cc: linux-wireless, netdev

On Sun, 2025-04-27 at 07:55 +0800, Jason Xing wrote:
> Hi Johannes,
> 
> On Thu, Mar 13, 2025 at 9:01 PM Johannes Berg <johannes@sipsolutions.net> wrote:
> > 
> > From: Johannes Berg <johannes.berg@intel.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@intel.com>
> 
> Do you have plans to repost it recently in net-next? I'm going to
> implement the rx path for BFP timestamping based on this patch :)

Oh since everyone seemed in favour I actually just put it in my tree.
Will be part of the wireless-next pull request when I send it soon.

johannes

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-04-28  7:41 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-13 12:49 [RFC PATCH] wifi: free SKBTX_WIFI_STATUS skb tx_flags flag Johannes Berg
2025-03-24 15:53 ` Jakub Kicinski
2025-03-24 15:57   ` Johannes Berg
2025-03-24 16:12     ` Jakub Kicinski
2025-03-24 16:33   ` Jason Xing
2025-03-24 16:53 ` Jason Xing
2025-03-24 16:56   ` Johannes Berg
2025-03-24 16:59     ` Jason Xing
2025-04-26 23:55 ` Jason Xing
2025-04-28  7:41   ` Johannes Berg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).