Re: [PATCH 1/2] mac80211: add private driver flags and ampdu length to tx info

[Johannes Berg <johannes@sipsolutions.net>]

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

On Sun, 2009-11-15 at 15:19 +0100, Felix Fietkau wrote:

> >> + * @ampdu_ack_len: number of acked aggregated frames.
> >> + * 	relevant only if ieee80211_tx_status_ampdu was set.
> > 
> > why lowercase that?
> Sorry, late night vim accident. I'll resend...

same a few lines down too

> >> -	/* 2 byte hole */
> >> -	u8 pad[2];
> >> +	u8 driver_flags;
> >> +	u8 pad;
> > 
> > The ampdu_len seems ok, but I'm still not convinced this is right.
> > Drivers _should_ have enough space in the driver parts of this struct,
> > and not need flags here. I realise that you do need that because of the
> > RC and other internal things, but maybe you could simply use "pad[0]"
> > for that so not to encourage others to start doing this too?

> If you prefer it using the padding directly, I can change it to that
> too. I don't mind as long as I have space to put that stuff into and
> don't need to use rate_driver_data.

I can't really decide. On the one hand, using pad seems bad, but on the
other hand I'm still hoping this will just be temporary (for some value
thereof). But in that case I fear that we might find somebody else
started using the new field and we need to fix that driver too then...

Thoughts?

johannes


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