| Message ID | 20210518110755.43077-1-borgers@mi.fu-berlin.de |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] mac80211: do not use low data rates for data frames with no ack flag | expand |
On Tue, 2021-05-18 at 13:17 +0200, Felix Fietkau wrote: > > Frames with IEEE80211_TX_CTL_HW_80211_ENCAP set have no 802.11 header, > so please change the check something like this: > > if ((info->flags & IEEE80211_TX_CTL_NO_ACK) && > ((info->flags & IEEE80211_TX_CTL_HW_80211_ENCAP) || > ieee80211_is_data(hdr->frame_control))) Maybe we should consider some kind of inline helper? static inline bool ieee80211_is_tx_data(struct sk_buff *skb) { ... *info = ... ... *hdr = (void *)skb->data; return (info->flags & IEEE80211_TX_CTL_HW_80211_ENCAP) || ieee80211_is_data(hdr->frame_control); } or so? johannes
Philipp Borgers <borgers@mi.fu-berlin.de> writes:
> Signed-off-by: Philipp Borgers <borgers@mi.fu-berlin.de>
Why? Empty commit logs is a bad idea, even if the reason is trivial to
you it might not be for others.
On Tue, May 18, 2021 at 01:19:46PM +0200, Johannes Berg wrote: > On Tue, 2021-05-18 at 13:17 +0200, Felix Fietkau wrote: > > > > Frames with IEEE80211_TX_CTL_HW_80211_ENCAP set have no 802.11 header, > > so please change the check something like this: > > > > if ((info->flags & IEEE80211_TX_CTL_NO_ACK) && > > ((info->flags & IEEE80211_TX_CTL_HW_80211_ENCAP) || > > ieee80211_is_data(hdr->frame_control))) > > Maybe we should consider some kind of inline helper? > > static inline bool ieee80211_is_tx_data(struct sk_buff *skb) > { > ... *info = ... > ... *hdr = (void *)skb->data; > > return (info->flags & IEEE80211_TX_CTL_HW_80211_ENCAP) || > ieee80211_is_data(hdr->frame_control); > } A frame with IEEE80211_TX_CTL_HW_80211_ENCAP set is always a data frame? Should I put the definition of the function into include/net/mac80211.h?
On Wed, 2021-05-19 at 12:47 +0200, Philipp Borgers wrote: > On Tue, May 18, 2021 at 01:19:46PM +0200, Johannes Berg wrote: > > On Tue, 2021-05-18 at 13:17 +0200, Felix Fietkau wrote: > > > > > > Frames with IEEE80211_TX_CTL_HW_80211_ENCAP set have no 802.11 header, > > > so please change the check something like this: > > > > > > if ((info->flags & IEEE80211_TX_CTL_NO_ACK) && > > > ((info->flags & IEEE80211_TX_CTL_HW_80211_ENCAP) || > > > ieee80211_is_data(hdr->frame_control))) > > > > Maybe we should consider some kind of inline helper? > > > > static inline bool ieee80211_is_tx_data(struct sk_buff *skb) > > { > > ... *info = ... > > ... *hdr = (void *)skb->data; > > > > return (info->flags & IEEE80211_TX_CTL_HW_80211_ENCAP) || > > ieee80211_is_data(hdr->frame_control); > > } > > A frame with IEEE80211_TX_CTL_HW_80211_ENCAP set is always a data frame? Yes, other frames can't be HW-encap'ed, nor would it make sense to offload that. > Should I put the definition of the function into include/net/mac80211.h? > Seems reasonable, yeah. johannes
diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c index 63652c39c8e0..4f5b35a0ea28 100644 --- a/net/mac80211/rate.c +++ b/net/mac80211/rate.c @@ -391,11 +391,16 @@ static bool rate_control_send_low(struct ieee80211_sta *pubsta, struct ieee80211_tx_rate_control *txrc) { struct ieee80211_tx_info *info = IEEE80211_SKB_CB(txrc->skb); + struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) txrc->skb->data; struct ieee80211_supported_band *sband = txrc->sband; struct sta_info *sta; int mcast_rate; bool use_basicrate = false; + if (ieee80211_is_data(hdr->frame_control) && + (info->flags & IEEE80211_TX_CTL_NO_ACK)) + return false; + if (!pubsta || rc_no_data_or_no_ack_use_min(txrc)) { __rate_control_send_low(txrc->hw, sband, pubsta, info, txrc->rate_idx_mask);
Data Frames with no ack flag set should be handle by the rate controler. Make sure we reach the rate controler by returning early from rate_control_send_low if the frame is a data frame with no ack flag. Signed-off-by: Philipp Borgers <borgers@mi.fu-berlin.de> --- net/mac80211/rate.c | 5 +++++ 1 file changed, 5 insertions(+)