With my patch to shrink ieee80211_tx_control, I can now put that into
skb->cb.
Do we want to use that as the interface to drivers as well? Many drivers
could benefit from that a lot since we could save all the extra copies
done here.
I've grepped all drivers, currently those using skb->cb are:
* p54: only needs 8 bytes
* rt2x00: needs almost all the space (40 bytes)
* adm8211: puts the 802.11 header there
* rtl8180: needs a single pointer to control
* rtl8187: usb urb, control and hw pointers
* zd1211: control, hw and context pointers
Notice how four drivers here put the control information in there, which
needs to be copied by drivers. However, mac80211 only needs the control
flags later!
Additionally, mac80211 needs to copy the TX status information around a
lot when the irqsafe functions are used. That's also bad.
What we could do is this:
First, we put ieee80211_tx_control into skb->cb. The first member of
that is a 'u32 flags' which is the only thing we need later for TX
status reporting, so we require that drivers leave that in there.
Then, we merge the TX status flags into the TX flags so that now the
driver only needs to set a few flags in skb->cb. Also, we overlay this.
Hence, we have something like the following structure:
struct ieee80211_tx_information {
u32 flags;
union {
struct {
s8 tx_rate_idx, rts_cts_rate_idx,
alt_retry_rate_idx, retry_limit;
struct ieee80211_vif *vif;
struct ieee80211_key_conf *hw_key;
enum ieee80211_band band;
u16 aid;
u8 antenna_sel_tx;
u8 icv_len, iv_len;
} txctl;
struct {
u64 amdpu_ack_map;
int ack_signal;
u8 ampdu_ack_len;
bool excessive_retries; /* should be a flag / removed */
u8 retry_count;
} status;
/* can be used freely by driver after its ops->tx() accepts
* the frame and before tx status reporting */
u8 drv_data[40] __attribute__((__aligned__(sizeof(void *)));
};
};
This means that the drivers are free to use everything in skb->cb for
their own use but should leave the 'u32 flags' intact, and need to make
sure to copy out all the information they need before putting in new
information. This fits with all the drivers, rt2x00 needs at most 32
bytes (40 now, but one is the txcontrol pointer) while we have 44 free.
Also, if the driver rejects the skb in ops->tx(), it must not have
modified skb->cb.
The only thing I'm not sure yet is how to handle alignment here. skb->cb
itself is 8-byte aligned so we probably need to reserve another four
bytes to get things to 8-byte alignment and then we go down to 40
available bytes, this is what I did above.
Thoughts?
johannes
Attachment:
signature.asc
Description: This is a digitally signed message part
