Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FS#935 - Modules should have default parameters as part of packaging #6249

Closed
openwrt-bot opened this issue Jul 28, 2017 · 11 comments
Closed
Labels

Comments

@openwrt-bot
Copy link

pprindeville:

Supply the following if possible:

  • Device problem occurs on
  • Software versions of LEDE release, packages, etc.
  • Steps to reproduce

I'm using the kmod-e1000e package, and I notice that it installs the file:

/etc/modules.d/e1000e

which contains:

e1000e

none of the module parameters are set. It turns out that the InterruptRateThrottling parameter can make a significant difference depending on whether your device is a client, server, or router.

In the case of LEDE/OpenWRT, one is obviously a router, and the optimal settings are not the default.

The kernel parameter:

InterruptThrottleRate="1,1,1,1,1,1,1,1"

should be added, as this gives the best results. Quoting Documentation/networking/e1000e.txt:

For situations where low latency is vital such as cluster or
grid computing, the algorithm can reduce latency even more when
InterruptThrottleRate is set to mode 1. In this mode, which operates
the same as mode 3, the InterruptThrottleRate will be increased stepwise to
70000 for traffic in class "Lowest latency".

The macro AutoProbe (and also AutoLoad) and the underlying shell scripting for probe_module() and add_module() in include/kernel.mk don't support specifying parameters.

This should be fixed.

@openwrt-bot
Copy link
Author

pprindeville:

Actually, "4" gives good performance too, giving about 1.8% better performance and lower CPU utilization (about 9%), but at the cost of slightly increased latency.

@openwrt-bot
Copy link
Author

ffainelli:

I have a changeset here:

https://git.lede-project.org/?p=lede/florian/staging.git;a=commitdiff;h=473eedd8669280d7aeee98e37b8f8ac935b214aa

which adds support for specifying module parameters, however after looking at it a bit more, we would have to define a syntax for specifying module parameters if there are several modules being added in an AutoLoad or AutoProbe statement, right now this changeset only works with one module as it will consider all module parameters to be relative to that module.

@openwrt-bot
Copy link
Author

pprindeville:

Nice.

In the case of a package containing multiple modules, could we prefix the parameter with "modulename:" (and strip that during the scripting)?

In the case of no prefix (or a single module in the package), the parameter gets applied to all modules.

@openwrt-bot
Copy link
Author

pprindeville:

Actually, "modulename." would be more aligned with Linux command-line handling.

@openwrt-bot
Copy link
Author

pprindeville:

We could do something like:

for module in $modulelist; do module_args= for $arg in $arglist; do case $arg in $module.*) module_args="$module_args ${arg#$module.}" ;; *.*) # ignore... not this module ;; *) module_args="$module_args $arg" ;; esac done echo "$module$module_args" > /etc/module.d/$module done

as a rough sketch example...

@openwrt-bot
Copy link
Author

ffainelli:

I like this, do you want to go ahead and prepare a patch doing this?

@openwrt-bot
Copy link
Author

pprindeville:

I can try... That's some pretty gnarly macro as it is...

@openwrt-bot
Copy link
Author

pprindeville:

Got it mostly working, but in the case of:

AUTOLOAD:=$(call AutoProbe,e1000e,,IntMode=1 InterruptThrottleRate="4,4,4,4,4,4,4,4,4")

How does one pass a string with commas in it?

@openwrt-bot
Copy link
Author

pprindeville:

Figured the comma question out: use $(comma) instead.

@openwrt-bot
Copy link
Author

pprindeville:

I have a [[https://github.com/lede-project/source/pull/1274|PR #1274]] for this but it's stuck in limbo. It's neither reviewed nor merged.

@openwrt-bot
Copy link
Author

pprindeville:

I was told to rework my PR. Jo started some work on this:

https://git.lede-project.org/?p=lede/jow/staging.git;a=commit;h=ac6c509a08b9bb139e6e5cf3195938c1700903a7

which I think he intended me to use as a starting point, but it doesn't work as is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant