On Feb 4, 2008 4:25 PM, Johannes Berg <[email protected]> wrote:
> Tomas Winkler wrote:
>
> > I think we should also remove ieee80211_tx_control from
> > ieee80211_tx_status.
>
> Not sure. Where is it used?
Let's try, if it's break we won't be in worst situation then now :)
>
> > Maybe we should add blob to tx_status let say of skb->cb[] size and
> > field for rate scale algorithm version/name. Otherwise we will be
> > inventing API for each rate scale algorithm...
>
> I disagree, we should have something generic enough to allow some sort of
> "capability negotiation" between the driver and the rate control
> algorithm, otherwise we could just remove rate control algorithms
> completely and internalize them into drivers (with some "helper" functions
> for those who don't want to write their own)... Which I'm sure you might
> actually like but I don't ;)
So Felix how many rates do you need for each packet?
Thanks
Tomas
> johannes
>
On Mon, 4 Feb 2008 21:05:25 +0200
"Tomas Winkler" <[email protected]> wrote:
> Sorry I meant
> ieee80211_tx_status {
> > size_t nrates;
> > ieee80211_tx_ratectrl rates[0] ;
> > }
FWIW, I'm OK with this. BTW, further changes to this may be required in
the future as I plan to rework a bit the whole tx_status thing so that
drivers won't have to report ACKed frames but failed and retransmitted
frames only - this would reduce the load on slow buses such as USB.
--
Ciao
Stefano
Tomas Winkler wrote:
> Will this work work you?
>
> ieee80211_tx_ratectrl {
> u32 bitrate ( or pointer to ieee80211_rate I prefer no)
> u32 flags
> u32 retry_count
> }
>
> ieee80211_tx_status {
> ieee80211_tx_ratectrl retry_count[ X] / * X >= 4 */
> }
I guess that would work, but shouln't the array below be dynamic based on
what the driver can do? Many drivers can only work with one or two rates, IMHO.
- Felix
Tomas Winkler wrote:
>> > Maybe we should add blob to tx_status let say of skb->cb[] size and
>> > field for rate scale algorithm version/name. Otherwise we will be
>> > inventing API for each rate scale algorithm...
>>
>> I disagree, we should have something generic enough to allow some sort of
>> "capability negotiation" between the driver and the rate control
>> algorithm, otherwise we could just remove rate control algorithms
>> completely and internalize them into drivers (with some "helper" functions
>> for those who don't want to write their own)... Which I'm sure you might
>> actually like but I don't ;)
>
> So Felix how many rates do you need for each packet?
The algorithm is designed to work with 4 different rates with adjustable
retry counts. A good description can be found here:
http://madwifi.org/browser/madwifi/trunk/ath_rate/minstrel/minstrel.txt
- Felix
On Feb 4, 2008 9:03 PM, Tomas Winkler <[email protected]> wrote:
>
> On Feb 4, 2008 8:39 PM, Felix Fietkau <[email protected]> wrote:
> > Tomas Winkler wrote:
> > > Will this work work you?
> > >
> > > ieee80211_tx_ratectrl {
> > > u32 bitrate ( or pointer to ieee80211_rate I prefer no)
> > > u32 flags
> > > u32 retry_count
> > > }
> > >
> > > ieee80211_tx_status {
> > > ieee80211_tx_ratectrl retry_count[ X] / * X >= 4 */
> > > }
> > I guess that would work, but shouln't the array below be dynamic based on
> > what the driver can do? Many drivers can only work with one or two rates, IMHO.
>
> The problem that this structure is passed with each packett I don't
> like the idea that anybody would even try to allocate and free this
> structure as packets goes. However what about the following?
>
> ieee80211_tx_status {
> size_t nrates;
> ieee80211_tx_ratectrl retry_count[0] ;
> }
>
Sorry I meant
ieee80211_tx_status {
> size_t nrates;
> ieee80211_tx_ratectrl rates[0] ;
> }
> > - Felix
> >
>
On Feb 4, 2008 5:24 PM, Felix Fietkau <[email protected]> wrote:
> Tomas Winkler wrote:
> >> > Maybe we should add blob to tx_status let say of skb->cb[] size and
> >> > field for rate scale algorithm version/name. Otherwise we will be
> >> > inventing API for each rate scale algorithm...
> >>
> >> I disagree, we should have something generic enough to allow some sort of
> >> "capability negotiation" between the driver and the rate control
> >> algorithm, otherwise we could just remove rate control algorithms
> >> completely and internalize them into drivers (with some "helper" functions
> >> for those who don't want to write their own)... Which I'm sure you might
> >> actually like but I don't ;)
> >
> > So Felix how many rates do you need for each packet?
> The algorithm is designed to work with 4 different rates with adjustable
> retry counts. A good description can be found here:
> http://madwifi.org/browser/madwifi/trunk/ath_rate/minstrel/minstrel.txt
>
Will this work work you?
ieee80211_tx_ratectrl {
u32 bitrate ( or pointer to ieee80211_rate I prefer no)
u32 flags
u32 retry_count
}
ieee80211_tx_status {
ieee80211_tx_ratectrl retry_count[ X] / * X >= 4 */
}
Thanks
Tomas
> - Felix
>
On Feb 4, 2008 8:39 PM, Felix Fietkau <[email protected]> wrote:
> Tomas Winkler wrote:
> > Will this work work you?
> >
> > ieee80211_tx_ratectrl {
> > u32 bitrate ( or pointer to ieee80211_rate I prefer no)
> > u32 flags
> > u32 retry_count
> > }
> >
> > ieee80211_tx_status {
> > ieee80211_tx_ratectrl retry_count[ X] / * X >= 4 */
> > }
> I guess that would work, but shouln't the array below be dynamic based on
> what the driver can do? Many drivers can only work with one or two rates, IMHO.
The problem that this structure is passed with each packett I don't
like the idea that anybody would even try to allocate and free this
structure as packets goes. However what about the following?
ieee80211_tx_status {
size_t nrates;
ieee80211_tx_ratectrl retry_count[0] ;
}
> - Felix
>
> Just keep in mind that some driver requires to report ACKed frames to
> make rate scaling algorithm to work properly, this should be optional
> only.
> Second I believe for some management frames it's good to notify mac
> about successful transmission regardless of rate scaling algorithm.
Hostapd wants that for EAPOL frames as well, so it has to be somehow
triggerable per-frame.
johannes
Tomas Winkler wrote:
> The problem that this structure is passed with each packett I don't
> like the idea that anybody would even try to allocate and free this
> structure as packets goes. However what about the following?
>
> ieee80211_tx_status {
> size_t nrates;
> ieee80211_tx_ratectrl rates[0] ;
> }
Yes, I think that would be ok.
- Felix
On Feb 5, 2008 11:23 PM, Stefano Brivio <[email protected]> wrote:
> On Mon, 4 Feb 2008 21:05:25 +0200
> "Tomas Winkler" <[email protected]> wrote:
>
> > Sorry I meant
> > ieee80211_tx_status {
> > > size_t nrates;
> > > ieee80211_tx_ratectrl rates[0] ;
> > > }
>
> FWIW, I'm OK with this. BTW, further changes to this may be required in
> the future as I plan to rework a bit the whole tx_status thing so that
> drivers won't have to report ACKed frames but failed and retransmitted
> frames only - this would reduce the load on slow buses such as USB.
>
Just keep in mind that some driver requires to report ACKed frames to
make rate scaling algorithm to work properly, this should be optional
only.
Second I believe for some management frames it's good to notify mac
about successful transmission regardless of rate scaling algorithm.
Thanks
Tomas
> --
> Ciao
> Stefano
>