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

nginx: introduce support for dynamic modules #16842

Merged
merged 5 commits into from Apr 27, 2023

Conversation

Ansuel
Copy link
Member

@Ansuel Ansuel commented Oct 8, 2021

Start building sub package that provide dynamic modules.
Each module needs to be loaded using load_modules. Refer
to nginx documentation on how to use this.
This should result in lower memory usage as only used module
are loaded.

This is a first step to dynamic loading... Some testing is needed and i still need to add description to the new package.

Requested from: #16833

Signed-off-by: Christian Marangi ansuelsmth@gmail.com

@Ansuel Ansuel force-pushed the nginx-dynamic branch 7 times, most recently from 9316379 to cc5e59a Compare April 20, 2023 21:26
@Ansuel Ansuel changed the title [WIP] nginx: introduce support for dynamic modules nginx: introduce support for dynamic modules Apr 20, 2023
@Ansuel
Copy link
Member Author

Ansuel commented Apr 20, 2023

@alex9434 Are you still interested? Sorry for the delay of 2 years :(

@Ansuel
Copy link
Member Author

Ansuel commented Apr 20, 2023

@peter-stadler Are you ok with the change for the module? Or do you have suggestion on how to implement?

@Ansuel
Copy link
Member Author

Ansuel commented Apr 20, 2023

@hgl If you want to check this...

@Ansuel Ansuel force-pushed the nginx-dynamic branch 6 times, most recently from 4a7b0e9 to f960e68 Compare April 21, 2023 00:13
@hgl
Copy link
Contributor

hgl commented Apr 21, 2023

Awesome work! I'll try it out on my device soon.

A couple of questions from first glance:

  1. Is using pcre instead of pcre2 a compromise for support of some dynamic modules?
  2. Did you get any warnings from the lua module that the nginx it lives in is not from OpenResty?
  3. Since new package names are introduced, do you think it would be a good opportunity to rename nginx-all-module to nginx-full as mentioned in my discussion with Peter, since a lot of popular packages use the -full suffix, like dnsmasq-full, ip-full?

@Ansuel
Copy link
Member Author

Ansuel commented Apr 21, 2023

  1. Is using pcre instead of pcre2 a compromise for support of some dynamic modules?

as said in the commit it's for lua module... since both are supported by nginx is it a problem to use pcre instead of pcre2?

2. Did you get any warnings from the lua module that the nginx it lives in is not from OpenResty?

the lua module still needs some love as the openresty core and lrucache are still required (and also adding the correct path in the config) but if some user wants to use it we at least provide it. (manual modification are needed anyway since the module is loaded dynamically and that needs to be manually added to the nginx config)

3. Since new package names are introduced, do you think it would be a good opportunity to rename nginx-all-module to nginx-full as mentioned in my discussion with Peter, since a lot of popular packages use the -full suffix, like dnsmasq-full, ip-full?

Yep sure! Also since we are doing this name change... wonder if we can just drop the package name... guess we had enough time to make every user transition to it?

@Ansuel
Copy link
Member Author

Ansuel commented Apr 21, 2023

No idea why CI fails to build.

@npenin
Copy link

npenin commented Apr 21, 2023

Any chance to also have the njs module? And also a package for the njs standalone executable?

@Ansuel
Copy link
Member Author

Ansuel commented Apr 21, 2023

@npenin for standalone no idea... but for the module... as you can see it's pretty easy to add additional external module now...

Add support for loading dynamic module in uci template by adding .module
file in module.d directory.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Start building sub package that provide dynamic modules.

Each module needs to be loaded using load_modules.
Refer to nginx documentation on how to use this.

This should result in lower memory usage as only used module are loaded.

Also fix the uci-default scripts to add the required ubus module for
luci module.

-fvisibility=hidden is needed to be dropped to correctly support loading
dynamic modules.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Update lua module to latest openrestry version. Additional config are
required to correctly use it.

Switch it to luajit from liblua as this is what is currently supported
for the module since plain lua support was dropped from the module.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Update nginx to 1.24.0 and update headers-more module to fix compilation
error.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Rename nginx-all-module to nginx-full to follow pattern used by other
package and other projects.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
@Ansuel Ansuel merged commit c4b27ff into openwrt:master Apr 27, 2023
11 checks passed
@alex9434
Copy link

alex9434 commented Jul 8, 2023

Great, thank you. Will try it out now!

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

Successfully merging this pull request may close these issues.

None yet

4 participants