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

feature: added new Lua API for Nginx core's dynamic resolver (FFI). #235

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 85 additions & 0 deletions lib/ngx/resolver.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
local base = require "resty.core.base"
local get_request = base.get_request
local ffi = require "ffi"
local C = ffi.C
local ffi_new = ffi.new
local ffi_str = ffi.string
local ffi_gc = ffi.gc
local FFI_OK = base.FFI_OK
local FFI_ERROR = base.FFI_ERROR
local FFI_DONE = base.FFI_DONE
local co_yield = coroutine._yield

local BUF_SIZE = 256
local get_string_buf = base.get_string_buf
local get_size_ptr = base.get_size_ptr

base.allows_subsystem("http")

ffi.cdef [[
typedef intptr_t ngx_int_t;
typedef unsigned char u_char;
typedef struct ngx_http_lua_co_ctx_t *curcoctx_ptr;
typedef struct ngx_http_resolver_ctx_t *rctx_ptr;

typedef struct {
ngx_http_request_t *request;
u_char *buf;
size_t *buf_size;
curcoctx_ptr curr_co_ctx;
rctx_ptr rctx;
ngx_int_t rc;
unsigned ipv4:1;
unsigned ipv6:1;
} ngx_http_lua_resolver_ctx_t;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to expose such complex C struct as part of the ABI. It's quite a maintenance burden. Better avoid it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. But such struct allows avoid overhead of mem-pool creation/freeing + additional mem-allocations on C-land. So, it is just a trade-off between maintenance burden and additional cpu cycles.

What do you think?


int ngx_http_lua_ffi_resolve(ngx_http_lua_resolver_ctx_t *ctx, const char *hostname);

void ngx_http_lua_ffi_resolver_destroy(ngx_http_lua_resolver_ctx_t *ctx);
]]

local _M = { version = base.version }

local mt = {
__gc = C.ngx_http_lua_ffi_resolver_destroy
}

local Ctx = ffi.metatype("ngx_http_lua_resolver_ctx_t", mt)

function _M.resolve(hostname, ipv4, ipv6)
assert(type(hostname) == "string", "hostname must be string")
assert(ipv4 == nil or type(ipv4) == "boolean", "ipv4 must be boolean or nil")
assert(ipv6 == nil or type(ipv6) == "boolean", "ipv6 must be boolean or nil")
slimboyfat marked this conversation as resolved.
Show resolved Hide resolved

local buf = get_string_buf(BUF_SIZE)
local buf_size = get_size_ptr()
buf_size[0] = BUF_SIZE

local ctx = Ctx({get_request(), buf, buf_size})
slimboyfat marked this conversation as resolved.
Show resolved Hide resolved

ctx.ipv4 = ipv4 == nil and 1 or ipv4
ctx.ipv6 = ipv6 == nil and 0 or ipv6
slimboyfat marked this conversation as resolved.
Show resolved Hide resolved

local rc = C.ngx_http_lua_ffi_resolve(ctx, hostname)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You know that ctx is managed by Lua GC while your ctx may get collected as soon as this Lua function returns (or yields). I see you register ctx to rctx which will surely outlive this Lua function call once it yields.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really believed that coroutine function locals must not be collected by GC, until function returns (yield is not a case when local might be get freed by GC).

I spent several hours for reading manual and googling. Unfortunately, I did not find anything that clearly states what should be during a yield with locals.

Based on following I wrote several tests.
Lua: test suites https://www.lua.org/tests/ (for v5.1)
Lua Traverse http://code.matthewwild.co.uk/luatraverse/

local object_was_finalized

local function finalizer()
    object_was_finalized = true
end

local function func()
    local object = newproxy(true)
    getmetatable(object).__gc = finalizer
    local free = coroutine.yield()
    if free then
        object = nil
    end
    coroutine.yield()
end

-- Test #1
object_was_finalized = false
local t = coroutine.create(func)
coroutine.resume(t)
collectgarbage()
assert(object_was_finalized == false)
coroutine.resume(t, true)
collectgarbage()
assert(object_was_finalized == true)

-- Test #2 
object_was_finalized = false
t = coroutine.create(func)
coroutine.resume(t)
collectgarbage()
assert(object_was_finalized == false)
coroutine.resume(t, false)
collectgarbage()
assert(object_was_finalized == false)
coroutine.resume(t)
collectgarbage()
assert(object_was_finalized == true)

... and still not sure, if local variable can be GC-ed on yield.

Could you please point me to manual/link which covers this (GC on yield).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, I created a test with explicit lua_gc calls (from C-land) just before return into yielded-lua-thread and right after the return from resumed thread.
The test confirmed assertion that Ctx can get freed only after return from coroutine (not in the middle on yield).

Please, give me a direction/additional info on this.


local res, err
if (rc == FFI_OK) then
res, err = ffi_str(buf, buf_size[0]), nil
elseif (rc == FFI_DONE) then
res, err = co_yield()
elseif (rc == FFI_ERROR) then
res, err = nil, ffi_str(buf, buf_size[0])
else
res, err = nil, "unknown error"
end

C.ngx_http_lua_ffi_resolver_destroy(ffi_gc(ctx, nil))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong. ffi.gc() is for memory blocks allocated on the C land. The ctx is allocated on the Lua land and thus is managed by Lua GC.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me explain my idea in step-by-step:

  1. At C land I defined a finalizer which guarantees finalization of resolver's context allocated during a resolve call.

  2. The finalizer is registered via __gc metamethod for FFI cdata object Ctx.

At this point we have a guarante that the finalizer will be eventually called, as http://luajit.org/ext_ffi_api.html states that:

A cdata finalizer works like the __gc metamethod for userdata objects: when the last reference to a cdata object is gone, the associated finalizer is called with the cdata object as an argument.
...
An existing finalizer can be removed by setting a nil finalizer, e.g. right before explicitly deleting a resource:
...

But in Lua 5.1 manual (https://www.lua.org/manual/5.1/manual.html)
at "2.10.1 – Garbage-Collection Metamethods" we can find following note:

At the end of each garbage-collection cycle, the finalizers for userdata are called in reverse order of their creation, among those collected in that cycle. That is, the first finalizer to be called is the one associated with the userdata created last in the program. The userdata itself is freed only in the next garbage-collection cycle.

Thus object with finalizer requires 2 GC cycles (1st GC-cycle - call finalizer, 2nd cycle - memory reclamation) to get fully freed.
That is why, in my code, I make a third step.

  1. Perform explicit finalizator call and then unregister __gc finalizer.

C.ngx_http_lua_ffi_resolver_destroy(ffi_gc(ctx, nil))
is equvivalent for

C.ngx_http_lua_ffi_resolver_destroy(ctx) -- explicitly call finalizer
ffi_gc(ctx, nil) -- unregister finalizer (set __gc = nil at ctx's mt)

Now, ctx object will be freed in a single GC-cycle. So, from my point of view, it is just an optimization, which makes Lua heap cleaner a bit faster.

Please, let me know where I was wrong...

P.S.: I found several places in the OpenResty source code where the same idiom was used (e.g.: regex.lua - ngx_lua_ffi_destroy_regex(ffi_gc(compiled, nil)))


if err ~= nil then
return res, err
end

return res
end

return _M
140 changes: 140 additions & 0 deletions lib/ngx/resolver.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
Name
====

`ngx.resolver` - Lua API for Nginx core's dynamic resolver.

Table of Contents
=================

* [Name](#name)
* [Status](#status)
* [Synopsis](#synopsis)
* [Methods](#methods)
* [resolve](#resolve)
* [Community](#community)
* [English Mailing List](#english-mailing-list)
* [Chinese Mailing List](#chinese-mailing-list)
* [Bugs and Patches](#bugs-and-patches)
* [Author](#author)
* [Copyright and License](#copyright-and-license)
* [See Also](#see-also)

Status
======

TBD

Synopsis
========

```nginx
http {
resolver 8.8.8.8;

upstream backend {
server 0.0.0.0;

balancer_by_lua_block {
local balancer = require 'ngx.balancer'

local ctx = ngx.ctx
local ok, err = balancer.set_current_peer(ctx.peer_addr, ctx.peer_port)
if not ok then
ngx.log(ngx.ERR, "failed to set the peer: ", err)
ngx.exit(500)
end
}
}

server {
listen 8080;

access_by_lua_block {
local resolver = require 'ngx.resolver'

local ctx = ngx.ctx
local addr, err = resolver.resolve('google.com', true, false)
if addr then
ctx.peer_addr = addr
ctx.peer_port = 80
end
}

location / {
proxy_pass http://backend;
}
}
}
```

[Back to TOC](#table-of-contents)

Methods
=======

resolve
-----------------
slimboyfat marked this conversation as resolved.
Show resolved Hide resolved
**syntax:** *address,err = resolver.resolve(hostname, ipv4, ipv6)*

**context:** *rewrite_by_lua*, access_by_lua*, content_by_lua*, ngx.timer.*, ssl_certificate_by_lua*, ssl_session_fetch_by_lua**

Resolve `hostname` into IP address by using Nginx core's dynamic resolver. Returns IP address string. In case of error, `nil` will be returned as well as a string describing the error.

The `ipv4` and `ipv6`argument are boolean flags that controls whether A or AAAA DNS records we are interested in.
Please, note that resolver has its own configuration option `ipv6=on|off`, which has higher precedence over above flags.
The 'ipv4' flag has default value `true`.

It is required to configure the [resolver](http://nginx.org/en/docs/http/ngx_http_core_module.html#resolver) directive in the `nginx.conf`.


slimboyfat marked this conversation as resolved.
Show resolved Hide resolved
[Back to TOC](#table-of-contents)

Community
=========

[Back to TOC](#table-of-contents)

English Mailing List
--------------------

The [openresty-en](https://groups.google.com/group/openresty-en) mailing list is for English speakers.

[Back to TOC](#table-of-contents)

Chinese Mailing List
--------------------

The [openresty](https://groups.google.com/group/openresty) mailing list is for Chinese speakers.

[Back to TOC](#table-of-contents)

Bugs and Patches
================

Please report bugs or submit patches by

1. creating a ticket on the [GitHub Issue Tracker](https://github.com/openresty/lua-resty-core/issues),
1. or posting to the [OpenResty community](#community).

[Back to TOC](#table-of-contents)

Author
======

TBD

Copyright and License
=====================

TBD

[Back to TOC](#table-of-contents)

See Also
========
* library [lua-resty-core](https://github.com/openresty/lua-resty-core)
* the ngx_lua module: https://github.com/openresty/lua-nginx-module
* OpenResty: http://openresty.org

[Back to TOC](#table-of-contents)

135 changes: 135 additions & 0 deletions t/resolver.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
# vim:set ft= ts=4 sw=4 et fdm=marker:

use Test::Nginx::Socket::Lua 'no_plan';
use lib '.';
use t::TestCore;

$ENV{TEST_NGINX_LUA_PACKAGE_PATH} = "$t::TestCore::lua_package_path";

run_tests();

__DATA__


slimboyfat marked this conversation as resolved.
Show resolved Hide resolved
=== TEST 1: use resolver in rewrite_by_lua_block
--- http_config
lua_package_path "$TEST_NGINX_LUA_PACKAGE_PATH";
--- config
resolver 8.8.8.8;
rewrite_by_lua "ngx.ctx.addr = require('ngx.resolver').resolve('google.com')";
location = /resolve {
content_by_lua "ngx.say(ngx.ctx.addr)";
}
--- request
GET /resolve
--- response_body_like: ^\d{1,3}.\d{1,3}.\d{1,3}.\d{1,3}$



=== TEST 2: use resolver in access_by_lua_block
--- http_config
lua_package_path "$TEST_NGINX_LUA_PACKAGE_PATH";
--- config
resolver 8.8.8.8;
access_by_lua "ngx.ctx.addr = require('ngx.resolver').resolve('google.com')";
location = /resolve {
content_by_lua "ngx.say(ngx.ctx.addr)";
}
--- request
GET /resolve
--- response_body_like: ^\d{1,3}.\d{1,3}.\d{1,3}.\d{1,3}$



=== TEST 3: use resolver in content_by_lua_block
--- http_config
lua_package_path "$TEST_NGINX_LUA_PACKAGE_PATH";
--- config
resolver 8.8.8.8;
location = /resolve {
content_by_lua "ngx.say(require('ngx.resolver').resolve('google.com'))";
}
--- request
GET /resolve
--- response_body_like: ^\d{1,3}.\d{1,3}.\d{1,3}.\d{1,3}$



=== TEST 4: query IPv6 addresses
--- http_config
lua_package_path "$TEST_NGINX_LUA_PACKAGE_PATH";
--- config
resolver 8.8.8.8;
location = /resolve {
content_by_lua "ngx.say(require('ngx.resolver').resolve('google.com', false, true))";
}
--- request
GET /resolve
--- response_body_like: ^[a-fA-F0-9:]+$



=== TEST 5: pass IPv4 address to resolver
--- http_config
lua_package_path "$TEST_NGINX_LUA_PACKAGE_PATH";
--- config
location = /resolve {
content_by_lua "ngx.say(require('ngx.resolver').resolve('192.168.0.1'))";
}
--- request
GET /resolve
--- response_body
192.168.0.1



=== TEST 6: pass IPv6 address to resolver
--- http_config
lua_package_path "$TEST_NGINX_LUA_PACKAGE_PATH";
--- config
location = /resolve {
content_by_lua "ngx.say(require('ngx.resolver').resolve('2a00:1450:4010:c05::66'))";
}
--- request
GET /resolve
--- response_body
2a00:1450:4010:c05::66



=== TEST 7: pass non-existent domain name to resolver
--- http_config
lua_package_path "$TEST_NGINX_LUA_PACKAGE_PATH";
--- config
resolver 8.8.8.8;
resolver_timeout 1s;
location = /resolve {
content_by_lua "ngx.say(require('ngx.resolver').resolve('fake-name'))";
}
--- request
GET /resolve
--- response_body
nilfake-name could not be resolved (3: Host not found)



=== TEST 8: check caching in Nginx resolver (2 cache hits)
--- http_config
lua_package_path "$TEST_NGINX_LUA_PACKAGE_PATH";
--- config
resolver 8.8.8.8 valid=30s;

location = /resolve {
content_by_lua_block {
local resolver = require 'ngx.resolver'
ngx.say(resolver.resolve('google.com'))
ngx.say(resolver.resolve('google.com'))
ngx.say(resolver.resolve('google.com'))
}
}
--- request
GET /resolve
--- grep_error_log: resolve cached
--- grep_error_log_out
resolve cached
resolve cached