On Fri, 2019-10-18 at 12:15 +0200, Toke Høiland-Jørgensen wrote:
> Kan Yan <kyan@xxxxxxxxxx> 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
