Discussion:
net80211 ratectl proof of concept
(too old to reply)
Rui Paulo
2010-03-31 13:05:52 UTC
Permalink
Hi,
I've started developing a ratectl framework for net80211, loosely based on what DragonFly has. Right now only one driver has been ported, but I would like your feedback before continuing.

The objective is to, eventually, have all the ratectl stuff (amrr, sample, onoe(?) and rssadapt) in net80211 so all drivers can use it. We can also select which ratectl modules are built in the kernel config file.
The framework support changing the current ratectl is out of scope for this patch.

You can find the patch here:
* http://people.freebsd.org/~rpaulo/ratectl.diff

Only the ral driver and the AMRR rate control algorithms were ported.

Some comments:
o The rate control calls now dereferences several pointers and some inline functions are now real functions. I wonder how much this impacts performance and what we can do to solve it.

o I wished there was a better way to do the IEEE80211_AMRR_SUCCESS / IEEE80211_AMRR_FAILURe call.

o Some other stuff can also be `const'

o I create ieee80211_ratect.[ch] to avoid polluting other files

o I moved the AMRR parameters inside amrr_init() on purpose. The drivers we have now only specify a different interval and I plan to add export amrr_set_interval() via the ratectl framework later.


I would like very much to see this in, unless there's a strong impending argument.

Thanks,
--
Rui Paulo
Rui Paulo
2010-03-31 13:31:30 UTC
Permalink
Post by Rui Paulo
Hi,
I've started developing a ratectl framework for net80211, loosely based on what DragonFly has. Right now only one driver has been ported, but I would like your feedback before continuing.
The objective is to, eventually, have all the ratectl stuff (amrr, sample, onoe(?) and rssadapt) in net80211 so all drivers can use it. We can also select which ratectl modules are built in the kernel config file.
The framework support changing the current ratectl is out of scope for this patch.
* http://people.freebsd.org/~rpaulo/ratectl.diff
Only the ral driver and the AMRR rate control algorithms were ported.
o The rate control calls now dereferences several pointers and some inline functions are now real functions. I wonder how much this impacts performance and what we can do to solve it.
o I wished there was a better way to do the IEEE80211_AMRR_SUCCESS / IEEE80211_AMRR_FAILURe call.
o Some other stuff can also be `const'
o I create ieee80211_ratect.[ch] to avoid polluting other files
o I moved the AMRR parameters inside amrr_init() on purpose. The drivers we have now only specify a different interval and I plan to add export amrr_set_interval() via the ratectl framework later.
I would like very much to see this in, unless there's a strong impending argument.
Hello,
This looks great!
Is there specific reasons to use pointers and not ints for some
arguments of foo_tx_complete() and foo_tx_update()?
Not really. I'll probably switch them to ints at some point.

--
Rui Paulo
Bernhard Schmidt
2010-03-31 14:04:15 UTC
Permalink
Post by Rui Paulo
This looks great!
Is there specific reasons to use pointers and not ints for some
arguments of foo_tx_complete() and foo_tx_update()?
Not really. I'll probably switch them to ints at some point.
Hmm, I somehow like the idea of not passing integers here. Looking at
ath_rate/sample it passes a struct. I don't know whether this can be
rewritten though.
--
Bernhard
Rui Paulo
2010-03-31 14:05:27 UTC
Permalink
Post by Bernhard Schmidt
Post by Rui Paulo
This looks great!
Is there specific reasons to use pointers and not ints for some
arguments of foo_tx_complete() and foo_tx_update()?
Not really. I'll probably switch them to ints at some point.
Hmm, I somehow like the idea of not passing integers here. Looking at
ath_rate/sample it passes a struct. I don't know whether this can be
rewritten though.
It can be rewritten to pass ints, from what I understand.

--
Rui Paulo
Bernhard Schmidt
2010-03-31 17:06:48 UTC
Permalink
Post by Rui Paulo
Post by Bernhard Schmidt
Post by Rui Paulo
This looks great!
Is there specific reasons to use pointers and not ints for some
arguments of foo_tx_complete() and foo_tx_update()?
Not really. I'll probably switch them to ints at some point.
Hmm, I somehow like the idea of not passing integers here. Looking at
ath_rate/sample it passes a struct. I don't know whether this can be
rewritten though.
It can be rewritten to pass ints, from what I understand.
Ok great, looks fine to me.

Do you intend to commit this in one huge chunk (quite a few drivers
affected) or create a branch for that?
--
Bernhard
Rui Paulo
2010-03-31 17:14:21 UTC
Permalink
Post by Bernhard Schmidt
Post by Rui Paulo
Post by Bernhard Schmidt
Post by Rui Paulo
This looks great!
Is there specific reasons to use pointers and not ints for some
arguments of foo_tx_complete() and foo_tx_update()?
Not really. I'll probably switch them to ints at some point.
Hmm, I somehow like the idea of not passing integers here. Looking at
ath_rate/sample it passes a struct. I don't know whether this can be
rewritten though.
It can be rewritten to pass ints, from what I understand.
Ok great, looks fine to me.
Do you intend to commit this in one huge chunk (quite a few drivers
affected) or create a branch for that?
I wanted people to comment on the general framework.

I wasn't planning on creating a branch, because I didn't feel it was necessary. Porting AMRR code to this framework isn't hard and I can deal with any fallout.

--
Rui Paulo
Rui Paulo
2010-04-05 16:54:20 UTC
Permalink
Post by Rui Paulo
Hi,
I've started developing a ratectl framework for net80211, loosely based on what DragonFly has. Right now only one driver has been ported, but I would like your feedback before continuing.
The objective is to, eventually, have all the ratectl stuff (amrr, sample, onoe(?) and rssadapt) in net80211 so all drivers can use it. We can also select which ratectl modules are built in the kernel config file.
The framework support changing the current ratectl is out of scope for this patch.
* http://people.freebsd.org/~rpaulo/ratectl.diff
Only the ral driver and the AMRR rate control algorithms were ported.
o The rate control calls now dereferences several pointers and some inline functions are now real functions. I wonder how much this impacts performance and what we can do to solve it.
o I wished there was a better way to do the IEEE80211_AMRR_SUCCESS / IEEE80211_AMRR_FAILURe call.
o Some other stuff can also be `const'
o I create ieee80211_ratect.[ch] to avoid polluting other files
o I moved the AMRR parameters inside amrr_init() on purpose. The drivers we have now only specify a different interval and I plan to add export amrr_set_interval() via the ratectl framework later.
I would like very much to see this in, unless there's a strong impending argument.
I've ported all the drivers but I can't test them all. You can read the patch at the same URL.

I would like to commit this soon, though.

Regards,
--
Rui Paulo
Weongyo Jeong
2010-04-07 01:19:25 UTC
Permalink
Post by Rui Paulo
Post by Rui Paulo
Hi,
I've started developing a ratectl framework for net80211, loosely
based on what DragonFly has. Right now only one driver has been
ported, but I would like your feedback before continuing.
The objective is to, eventually, have all the ratectl stuff (amrr,
sample, onoe(?) and rssadapt) in net80211 so all drivers can use it.
We can also select which ratectl modules are built in the kernel
config file.
The framework support changing the current ratectl is out of scope for this patch.
* http://people.freebsd.org/~rpaulo/ratectl.diff
Only the ral driver and the AMRR rate control algorithms were ported.
o The rate control calls now dereferences several pointers and some
inline functions are now real functions. I wonder how much this
impacts performance and what we can do to solve it.
o I wished there was a better way to do the IEEE80211_AMRR_SUCCESS /
IEEE80211_AMRR_FAILURe call.
o Some other stuff can also be `const'
o I create ieee80211_ratect.[ch] to avoid polluting other files
o I moved the AMRR parameters inside amrr_init() on purpose. The
drivers we have now only specify a different interval and I plan to
add export amrr_set_interval() via the ratectl framework later.
I would like very much to see this in, unless there's a strong impending argument.
I've ported all the drivers but I can't test them all. You can read
the patch at the same URL.
I would like to commit this soon, though.
It looks it's what I really want to see. Please go forward.

regards,
Weongyo Jeong

Loading...