stream-echo-nginx-module icon indicating copy to clipboard operation
stream-echo-nginx-module copied to clipboard

udp support - work in progress

Open alonbg opened this issue 8 years ago • 8 comments

New test added which requires this test_nginx pull request

alonbg avatar Aug 07 '16 09:08 alonbg

I used ngx_udp_send() to standardize with macro usage (it works :) Added ---dgram_request to all

alonbg avatar Aug 09 '16 15:08 alonbg

@alonbg I'm still seeing test failures in your echo-udp.t file. Are you going to fix them all? I think each echo directive should generate a separate datagram packet. But if multiple arguments are given to a single echo directive, then the arguments should be concatenated together with spaces (just like the TCP case).

Also, we need to take care of the request reading directives like echo_read_line and echo_read_bytes for UDP requests. I think we can simply throw out an error for them. What we really need is an echo_read_packet directive for packet reading :)

agentzh avatar Aug 09 '16 19:08 agentzh

@alonbg I think we may need support in Test::Nginx::Socket for receiving multiple response packets, similar to ngx_stream_proxy_module's proxy_responses directive:

http://nginx.org/en/docs/stream/ngx_stream_proxy_module.html#proxy_responses

What do you think?

agentzh avatar Aug 09 '16 19:08 agentzh

@agentzh not sure I'll be able to fix all echo-udp.t failures. I'll try. I'll probably need your guidance for those that I will.

why echo_read_bytes should throw an error ? udp_sock.receive(#) reads bytes. Why not here?

Regarding echo_read_packet directive, I guess the logic would be under an input_filter like ngx_stream_echo_read_packet_filter where I should probably set ctx->rest and req->len Please advise :)

BTW, reading subsequent packets/responses would this be achieved by configuring additional echo_read_packet ?

Regarding Test::Nginx::Socket support for multiple response packets My perl skills (or lake of) wouldn't let me ( in a timely manner :)

Another, more local and possibly quicker solution in Test::Nginx::Socket::Lua::Dgram might be wrapping udp:receive() in a while statement and concatenating all responses to a single. Possibly with a new optional dgram_responses as a break condition.

changing $new_http_server_config from:

local data, err = sock:receive()
 if not data then
   sock:close()
   ngx.say("receive stream response error: ", err)
   return
end

to something like:

local count = 0
local data = ""

while count < "$dgram_responses" do
      local response, err = sock:receive()
          if not data then
              sock:close()
              ngx.say("receive stream response error: ", err)
              return
           end
    data = data ..  response
    count = count + 1
end

Another long-term option (with probably a greater benefit :) can be adding a non-blocking receive_responses(#) to udp socket api. ngx_stream_proxy_module.c being a reference implementation. ( I don't think I'm up for this task, btw :)

alonbg avatar Aug 10 '16 14:08 alonbg

@alonbg Well, we can leave the echo_read_packet directive for now. Will you just add error handling code to the echo_read_xxx directives so that they don't crash the processes when UDP is used?

Regarding your suggestion of concatenating packets into a single string, I think that's a bad idea since packets are inherently atomic. Well, we can leave that part for now as well :)

agentzh avatar Aug 10 '16 18:08 agentzh

@agentzh I added error handling for echo_read_line and echo_read_bytes tests with multiple echo directives are skipped (for now :).

alonbg avatar Aug 11 '16 16:08 alonbg

@alonbg Will you also add a test case for empty output packets? That is, UDP packets with a zero length payload.

agentzh avatar Aug 12 '16 23:08 agentzh

@agentzh @alonbg Can this module for UDP read support? I need to read udp incoming byte and log to somewhere like rsyslog.

Taymindis avatar Jun 12 '18 01:06 Taymindis