Re: [Make-wifi-fast] [PATCH v2 1/4] mac80211: Rearrange ieee80211_tx_info to make room for tx_time_est

[Johannes Berg <johannes@sipsolutions.net>]

On Fri, 2019-10-18 at 12:15 +0200, Toke Høiland-Jørgensen wrote:
> Kan Yan <kyan@google.com> writes:
> 
> > The "tx_time_est" field, shared by control and status, is not able to
> > survive until the skb returns to the mac80211 layer in some
> > architectures. The same space is defined as driver_data and some
> > wireless drivers use it for other purposes, as the cb in the sk_buff
> > is free to be used by any layer.
> > 
> > In the case of ath10k, the tx_time_est get clobbered by
> > struct ath10k_skb_cb {
> >         dma_addr_t paddr;
> >         u8 flags;
> >         u8 eid;
> >         u16 msdu_id;
> >         u16 airtime_est;
> >         struct ieee80211_vif *vif;
> >         struct ieee80211_txq *txq;
> > } __packed;
> 
> Ah, bugger, of course the driver that actually needs this is using the
> full driver_data space :P

Looks like you could shrink *this* fairly easily though.

E.g. most likely vif == txq->vif unless txq==NULL, so it's down to 22
bytes plus a bit/flag for knowing whether the pointer is a vif directly
(if no TXQ) or a TXQ.

> > Do you think shrink driver_data by 2 bytes and use that space for
> > tx_time_est to make it persistent across mac80211 and wireless driver
> > layer an acceptable solution?
> 
> Hmm, the driver_data field is defined as an array of pointers, so we can
> only shrink it in increments of sizeof(void *). I think it may be
> feasible to shrink it (as in, I don't think any drivers are actually
> using the full 40 bytes),

It doesn't have to be defined like that, just was most convenient as
driers were using pointers there.

> but doing this in a way that will gain us a
> 2-byte space that is also usable in the case driver_data is *not* used
> (i.e., it needs be able to align with a field in .control and .status as
> well) would require some serious surgery of the whole ieee80211_tx_info...

Yeah, good point, this doesn't help at all ...

johannes