mac80211: Set IEEE80211_TXPD_REQ_TX_STATUS for all TX frames

mac80211: Set IEEE80211_TXPD_REQ_TX_STATUS for all TX frames

[Ivo van Doorn]

All interfaces should set the IEEE80211_TXPD_REQ_TX_STATUS flag for all TX frames
which will force the master interface to set the IEEE80211_TX_CTL_REQ_TX_STATUS
flag. This in turn will allow drivers to check for that flag before reporting
the TX status to mac80211.

This is very usefull when frames (like beacons, RTS and CTS-to-self) should not
be reported back to mac80211. Later we could add more extensive checks to
exclude more frames from being reported, or let mac80211 decide if it wants
the frame for status reporting or not.

Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
---
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index f35eaea..0024892 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1617,6 +1617,9 @@ int ieee80211_subif_start_xmit(struct sk_buff *skb,
 	if (ethertype == ETH_P_PAE)
 		pkt_data->flags |= IEEE80211_TXPD_EAPOL_FRAME;
 
+	/* Interfaces should always request a status report */
+	pkt_data->flags |= IEEE80211_TXPD_REQ_TX_STATUS;
+
 	skb->dev = local->mdev;
 	dev->stats.tx_packets++;
 	dev->stats.tx_bytes += skb->len;

Re: mac80211: Set IEEE80211_TXPD_REQ_TX_STATUS for all TX frames

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1417 bytes --]

On Sun, 2008-05-11 at 14:34 +0200, Ivo van Doorn wrote:
> All interfaces should set the IEEE80211_TXPD_REQ_TX_STATUS flag for all TX frames
> which will force the master interface to set the IEEE80211_TX_CTL_REQ_TX_STATUS
> flag. This in turn will allow drivers to check for that flag before reporting
> the TX status to mac80211.
> 
> This is very usefull when frames (like beacons, RTS and CTS-to-self) should not
> be reported back to mac80211. Later we could add more extensive checks to
> exclude more frames from being reported, or let mac80211 decide if it wants
> the frame for status reporting or not.
> 
> Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
> ---
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index f35eaea..0024892 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -1617,6 +1617,9 @@ int ieee80211_subif_start_xmit(struct sk_buff *skb,
>  	if (ethertype == ETH_P_PAE)
>  		pkt_data->flags |= IEEE80211_TXPD_EAPOL_FRAME;
>  
> +	/* Interfaces should always request a status report */
> +	pkt_data->flags |= IEEE80211_TXPD_REQ_TX_STATUS;
> +

What about monitor interfaces/injection? Do they already set it?

Other than that, looks good to me, as long as you're aware that in the
future this might be more dynamic and don't rely on it too much for
internal housekeeping (but you seem to be according to your changelog
entry) :)

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: mac80211: Set IEEE80211_TXPD_REQ_TX_STATUS for all TX frames

[Ivo van Doorn]

On Tuesday 13 May 2008, Johannes Berg wrote:
> On Sun, 2008-05-11 at 14:34 +0200, Ivo van Doorn wrote:
> > All interfaces should set the IEEE80211_TXPD_REQ_TX_STATUS flag for all TX frames
> > which will force the master interface to set the IEEE80211_TX_CTL_REQ_TX_STATUS
> > flag. This in turn will allow drivers to check for that flag before reporting
> > the TX status to mac80211.
> > 
> > This is very usefull when frames (like beacons, RTS and CTS-to-self) should not
> > be reported back to mac80211. Later we could add more extensive checks to
> > exclude more frames from being reported, or let mac80211 decide if it wants
> > the frame for status reporting or not.
> > 
> > Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
> > ---
> > diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> > index f35eaea..0024892 100644
> > --- a/net/mac80211/tx.c
> > +++ b/net/mac80211/tx.c
> > @@ -1617,6 +1617,9 @@ int ieee80211_subif_start_xmit(struct sk_buff *skb,
> >  	if (ethertype == ETH_P_PAE)
> >  		pkt_data->flags |= IEEE80211_TXPD_EAPOL_FRAME;
> >  
> > +	/* Interfaces should always request a status report */
> > +	pkt_data->flags |= IEEE80211_TXPD_REQ_TX_STATUS;
> > +
> 
> What about monitor interfaces/injection? Do they already set it?

No, should they?

> Other than that, looks good to me, as long as you're aware that in the
> future this might be more dynamic and don't rely on it too much for
> internal housekeeping (but you seem to be according to your changelog
> entry) :)

Well it shouldn't matter if it will become more flexible, what I am looking
for is making the flag actually mean something for drivers. ;)
We could even make it so that the mac80211 tx_status handler will ignore
the frame when the flag is set so drivers won't have to look at it at all.

Ivo

Re: mac80211: Set IEEE80211_TXPD_REQ_TX_STATUS for all TX frames

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 986 bytes --]


> > What about monitor interfaces/injection? Do they already set it?
> 
> No, should they?

Yes, I think so, since hostapd uses injection for frames and definitely
needs status callback. We could later define a radiotap bit for that, of
course.

> > Other than that, looks good to me, as long as you're aware that in the
> > future this might be more dynamic and don't rely on it too much for
> > internal housekeeping (but you seem to be according to your changelog
> > entry) :)
> 
> Well it shouldn't matter if it will become more flexible, what I am looking
> for is making the flag actually mean something for drivers. ;)

Right :)

> We could even make it so that the mac80211 tx_status handler will ignore
> the frame when the flag is set so drivers won't have to look at it at all.

You mean "when the flag is unset"? Not sure, maybe we should, but we'll
see that when more uses for it come up/better rate control algorithms
are developed.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: mac80211: Set IEEE80211_TXPD_REQ_TX_STATUS for all TX frames

[Ivo van Doorn]

On Tuesday 13 May 2008, Johannes Berg wrote:
> 
> > > What about monitor interfaces/injection? Do they already set it?
> > 
> > No, should they?
> 
> Yes, I think so, since hostapd uses injection for frames and definitely
> needs status callback. We could later define a radiotap bit for that, of
> course.

Ok, I'll update the patch.

> > We could even make it so that the mac80211 tx_status handler will ignore
> > the frame when the flag is set so drivers won't have to look at it at all.
> 
> You mean "when the flag is unset"? Not sure, maybe we should, but we'll
> see that when more uses for it come up/better rate control algorithms
> are developed.

The most important frames the issue currently arises is with RTS and CTS-to-self
frames. rt2x00 requests those frames from mac80211 and currently clears the
IEEE80211_TXCTL_REQ_TX_STATUS flag to prevent it being send to mac80211
to not upset the rc-algorithm.

Ivo

[PATCH v2] mac80211: Set IEEE80211_TXPD_REQ_TX_STATUS for all TX frames

[Ivo van Doorn]

All interfaces should set the IEEE80211_TXPD_REQ_TX_STATUS flag for all TX frames
which will force the master interface to set the IEEE80211_TX_CTL_REQ_TX_STATUS
flag. This in turn will allow drivers to check for that flag before reporting
the TX status to mac80211.

This is very usefull when frames (like beacons, RTS and CTS-to-self) should not
be reported back to mac80211. Later we could add more extensive checks to
exclude more frames from being reported, or let mac80211 decide if it wants
the frame for status reporting or not.

v2: Monitor interfaces should also set IEEE80211_TXPD_REQ_TX_STATUS

Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
---
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index f35eaea..472dab0 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1336,6 +1336,8 @@ int ieee80211_monitor_start_xmit(struct sk_buff *skb,
 	pkt_data->ifindex = dev->ifindex;
 
 	pkt_data->flags |= IEEE80211_TXPD_DO_NOT_ENCRYPT;
+	/* Interfaces should always request a status report */
+	pkt_data->flags |= IEEE80211_TXPD_REQ_TX_STATUS;
 
 	/*
 	 * fix up the pointers accounting for the radiotap
@@ -1617,6 +1619,9 @@ int ieee80211_subif_start_xmit(struct sk_buff *skb,
 	if (ethertype == ETH_P_PAE)
 		pkt_data->flags |= IEEE80211_TXPD_EAPOL_FRAME;
 
+	/* Interfaces should always request a status report */
+	pkt_data->flags |= IEEE80211_TXPD_REQ_TX_STATUS;
+
 	skb->dev = local->mdev;
 	dev->stats.tx_packets++;
 	dev->stats.tx_bytes += skb->len;

Re: [PATCH v2] mac80211: Set IEEE80211_TXPD_REQ_TX_STATUS for all TX frames

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1729 bytes --]

On Tue, 2008-05-13 at 15:03 +0200, Ivo van Doorn wrote:
> All interfaces should set the IEEE80211_TXPD_REQ_TX_STATUS flag for all TX frames
> which will force the master interface to set the IEEE80211_TX_CTL_REQ_TX_STATUS
> flag. This in turn will allow drivers to check for that flag before reporting
> the TX status to mac80211.
> 
> This is very usefull when frames (like beacons, RTS and CTS-to-self) should not
> be reported back to mac80211. Later we could add more extensive checks to
> exclude more frames from being reported, or let mac80211 decide if it wants
> the frame for status reporting or not.
> 
> v2: Monitor interfaces should also set IEEE80211_TXPD_REQ_TX_STATUS
> 
> Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>

Looks good, thanks.

Acked-by: Johannes Berg <johannes@sipsolutions.net>

> ---
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index f35eaea..472dab0 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -1336,6 +1336,8 @@ int ieee80211_monitor_start_xmit(struct sk_buff *skb,
>  	pkt_data->ifindex = dev->ifindex;
>  
>  	pkt_data->flags |= IEEE80211_TXPD_DO_NOT_ENCRYPT;
> +	/* Interfaces should always request a status report */
> +	pkt_data->flags |= IEEE80211_TXPD_REQ_TX_STATUS;
>  
>  	/*
>  	 * fix up the pointers accounting for the radiotap
> @@ -1617,6 +1619,9 @@ int ieee80211_subif_start_xmit(struct sk_buff *skb,
>  	if (ethertype == ETH_P_PAE)
>  		pkt_data->flags |= IEEE80211_TXPD_EAPOL_FRAME;
>  
> +	/* Interfaces should always request a status report */
> +	pkt_data->flags |= IEEE80211_TXPD_REQ_TX_STATUS;
> +
>  	skb->dev = local->mdev;
>  	dev->stats.tx_packets++;
>  	dev->stats.tx_bytes += skb->len;
> 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]