<div dir="ltr"><div><div><div>Hello Maxim,<br><br></div>Yes, my module cannot work with streams, we need full response body before it starts working.<br></div>So what you suggest it something like this in my move_chain_to_brigado()  ?<br>
<br>        while (chain) {<br><br>                buf = ngx_calloc_buf(pool);<br>                if (buf == NULL) {<br>                        return NGX_ERROR;<br>                }<br><br>                len = chain->buf->end - chain->buf->start;<br>
                buf->start = ngx_palloc(pool, len);<br>                ngx_memcpy(buf->start, chain->buf->start, len);<br><br>                buf->pos = buf->start;<br>                buf->end = buf->last = buf->pos + len;<br>
<br>                e = ngx_buf_to_apr_bucket(buf, bb->p, bb->bucket_alloc);<br>                if (e == NULL) {<br>                        return NGX_ERROR;<br>                }<br><br>                APR_BRIGADE_INSERT_TAIL(bb, e);<br>
                if (chain->buf->last_buf) {<br>                        e = apr_bucket_eos_create(bb->bucket_alloc);<br>                        APR_BRIGADE_INSERT_TAIL(bb, e);<br>                        chain->buf->last_buf = 0;<br>
                        chain->buf->pos = chain->buf->last;<br>                        return NGX_OK;<br>                }<br><br>                chain->buf->pos = chain->buf->last;<br>                chain = chain->next;<br>
<br></div>Thanks<br><br>Breno<br></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, May 13, 2013 at 4:01 AM, Maxim Dounin <span dir="ltr"><<a href="mailto:mdounin@mdounin.ru" target="_blank">mdounin@mdounin.ru</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hello!<br>
<div class="im"><br>
On Sun, May 12, 2013 at 11:09:26AM -0300, Breno Silva wrote:<br>
<br>
> Hello,<br>
><br>
> Yes removing he ngx_free_chain fix the cpu issue. Looks like ther modules<br>
> needs the chain.<br>
> I detected another issue.. sometimes when loading big response bodies (like<br>
> images.. +32K) it does not load it entirely or does it slowly.<br>
><br>
> Do you have any idea ?<br>
> Thanks<br>
<br>
</div>It looks like you don't release buffers you got in your filter,<br>
and this results buffers exhaustion (output_buffers,<br>
proxy_buffers) and transfer stall.<br>
<br>
If your code can't work with a data stream and needs full response<br>
before it can start it's work, you have to copy needed data from<br>
buffers and release buffers got (by setting b->pos = b->last, like<br>
if a buffer was actually sent).<br>
<br>
To test things you may try the following in config:<br>
<br>
    output_buffers 1 1k;<br>
<br>
Then request a static file larger than 1k.<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
><br>
> Breno<br>
><br>
><br>
> On Sat, May 11, 2013 at 9:06 PM, Maxim Dounin <<a href="mailto:mdounin@mdounin.ru">mdounin@mdounin.ru</a>> wrote:<br>
><br>
> > Hello!<br>
> ><br>
> > On Sat, May 11, 2013 at 08:09:59PM -0300, Breno Silva wrote:<br>
> ><br>
> > > Hello Maxim,<br>
> > ><br>
> > > We are sending chains data into a brigade/bucket structure (APR). It<br>
> > > sometimes works fine.. sometimes trigger the cpu issue<br>
> > ><br>
> > > If i call:<br>
> > >  rc = move_chain_to_brigade(in, ctx->brigade, r->pool, 1);   <--- change<br>
> > > from 0 to 1<br>
> > > Then i will get only the first X bytes of the response body. This works<br>
> > > great... but i need the entire buffer :)<br>
> > ><br>
> > > Could you give us more details how we could patch this function to fix<br>
> > the<br>
> > > issue ?<br>
> ><br>
> > Remove the<br>
> ><br>
> >         ngx_free_chain(pool, cl);<br>
> ><br>
> > call from the move_chain_to_brigade() function, it should fix you<br>
> > CPU hog issues.<br>
> ><br>
> ><br>
> > ><br>
> > > Thanks<br>
> > ><br>
> > ><br>
> > > On Sat, May 11, 2013 at 7:56 PM, Maxim Dounin <<a href="mailto:mdounin@mdounin.ru">mdounin@mdounin.ru</a>><br>
> > wrote:<br>
> > ><br>
> > > > Hello!<br>
> > > ><br>
> > > > On Sat, May 11, 2013 at 04:23:00PM -0300, Breno Silva wrote:<br>
> > > ><br>
> > > > > Hello list,<br>
> > > > ><br>
> > > > > We are porting ModSecurity to NGINX. However we are seeing sometimes<br>
> > an<br>
> > > > > issue. Nginx eats 100% of cpu and when i use gdb i see:<br>
> > > > ><br>
> > > > > gdb -p 8645<br>
> > > > ><br>
> > > > > ngx_event_pipe_write_to_downstream (p=0x9bbc720, do_write=0) at<br>
> > > > > src/event/ngx_event_pipe.c:551<br>
> > > > ><br>
> > > > > 551 if (cl->buf->recycled) {<br>
> > > > > (gdb)<br>
> > > > ><br>
> > > > > Looks like it is happening when we call<br>
> > > > ngx_http_modsecurity_body_filter()<br>
> > > > > then go to this conditions;<br>
> > > > ><br>
> > > > >     rc = move_chain_to_brigade(in, ctx->brigade, r->pool, 0);<br>
> > > > ><br>
> > > > >     if (rc != NGX_OK)  {<br>
> > > > ><br>
> > > > >         r->buffered |= NGX_HTTP_SSI_BUFFERED;<br>
> > > > ><br>
> > > > >         return rc;<br>
> > > > ><br>
> > > > >      }<br>
> > > > ><br>
> > > > > move_chainto_brigade is defined as:<br>
> > > > ><br>
> > > > ><br>
> > > > > ngx_int_t<br>
> > > > ><br>
> > > > > move_chain_to_brigade(ngx_chain_t *chain, apr_bucket_brigade *bb,<br>
> > > > > ngx_pool_t *pool, ngx_int_t last_buf) {<br>
> > > > ><br>
> > > > >     apr_bucket         *e;<br>
> > > > ><br>
> > > > >     ngx_chain_t        *cl;<br>
> > > > ><br>
> > > > ><br>
> > > > >     while (chain) {<br>
> > > > ><br>
> > > > >         e = ngx_buf_to_apr_bucket(chain->buf, bb->p,<br>
> > bb->bucket_alloc);<br>
> > > > ><br>
> > > > >         if (e == NULL) {<br>
> > > > ><br>
> > > > >             return NGX_ERROR;<br>
> > > > ><br>
> > > > >         }<br>
> > > > ><br>
> > > > ><br>
> > > > >         APR_BRIGADE_INSERT_TAIL(bb, e);<br>
> > > > ><br>
> > > > >         if (chain->buf->last_buf) {<br>
> > > > ><br>
> > > > >             e = apr_bucket_eos_create(bb->bucket_alloc);<br>
> > > > ><br>
> > > > >             APR_BRIGADE_INSERT_TAIL(bb, e);<br>
> > > > ><br>
> > > > >             chain->buf->last_buf = 0;<br>
> > > > ><br>
> > > > >             return NGX_OK;<br>
> > > > ><br>
> > > > >         }<br>
> > > > ><br>
> > > > >         cl = chain;<br>
> > > > ><br>
> > > > >         chain = chain->next;<br>
> > > > ><br>
> > > > >         ngx_free_chain(pool, cl);<br>
> > > > ><br>
> > > > >     }<br>
> > > > ><br>
> > > > ><br>
> > > > >     if (last_buf) {<br>
> > > > ><br>
> > > > >         e = apr_bucket_eos_create(bb->bucket_alloc);<br>
> > > > ><br>
> > > > >         APR_BRIGADE_INSERT_TAIL(bb, e);<br>
> > > > ><br>
> > > > >         return NGX_OK;<br>
> > > > ><br>
> > > > >     }<br>
> > > > ><br>
> > > > >     return NGX_AGAIN;<br>
> > > > ><br>
> > > > > }<br>
> > > > > Let me know if you guys can help us understanding why sometimes we<br>
> > > > trigger<br>
> > > > > this issue<br>
> > > ><br>
> > > > It looks like your code modify chain links (ngx_chain_t<br>
> > > > structures) as got by your response body filter.  This is not<br>
> > > > something filters are allowed to do, and results are undefined.<br>
> > > > If a filter needs to modify chain, it should allocate it's own<br>
> > > > chain links.<br>
> > > ><br>
> > > > In the code quoted you probably don't want to touch chain links at<br>
> > > > all.<br>
> > > ><br>
> > > > --<br>
> > > > Maxim Dounin<br>
> > > > <a href="http://nginx.org/en/donation.html" target="_blank">http://nginx.org/en/donation.html</a><br>
> > > ><br>
> > > > _______________________________________________<br>
> > > > nginx-devel mailing list<br>
> > > > <a href="mailto:nginx-devel@nginx.org">nginx-devel@nginx.org</a><br>
> > > > <a href="http://mailman.nginx.org/mailman/listinfo/nginx-devel" target="_blank">http://mailman.nginx.org/mailman/listinfo/nginx-devel</a><br>
> > > ><br>
> ><br>
> > > _______________________________________________<br>
> > > nginx-devel mailing list<br>
> > > <a href="mailto:nginx-devel@nginx.org">nginx-devel@nginx.org</a><br>
> > > <a href="http://mailman.nginx.org/mailman/listinfo/nginx-devel" target="_blank">http://mailman.nginx.org/mailman/listinfo/nginx-devel</a><br>
> ><br>
> ><br>
> > --<br>
> > Maxim Dounin<br>
> > <a href="http://nginx.org/en/donation.html" target="_blank">http://nginx.org/en/donation.html</a><br>
> ><br>
> > _______________________________________________<br>
> > nginx-devel mailing list<br>
> > <a href="mailto:nginx-devel@nginx.org">nginx-devel@nginx.org</a><br>
> > <a href="http://mailman.nginx.org/mailman/listinfo/nginx-devel" target="_blank">http://mailman.nginx.org/mailman/listinfo/nginx-devel</a><br>
> ><br>
<br>
> _______________________________________________<br>
> nginx-devel mailing list<br>
> <a href="mailto:nginx-devel@nginx.org">nginx-devel@nginx.org</a><br>
> <a href="http://mailman.nginx.org/mailman/listinfo/nginx-devel" target="_blank">http://mailman.nginx.org/mailman/listinfo/nginx-devel</a><br>
<br>
<br>
--<br>
Maxim Dounin<br>
<a href="http://nginx.org/en/donation.html" target="_blank">http://nginx.org/en/donation.html</a><br>
<br>
_______________________________________________<br>
nginx-devel mailing list<br>
<a href="mailto:nginx-devel@nginx.org">nginx-devel@nginx.org</a><br>
<a href="http://mailman.nginx.org/mailman/listinfo/nginx-devel" target="_blank">http://mailman.nginx.org/mailman/listinfo/nginx-devel</a><br>
</div></div></blockquote></div><br></div>