Hello! On Wed, Nov 09, 2016 at 08:52:14PM +0100, Bjørnar Ness wrote:
> On Nov 9, 2016 19:20, "Maxim Dounin" <mdou...@mdounin.ru> wrote: > > > > Hello! > > > > On Thu, Nov 03, 2016 at 08:37:04PM +0100, Bjørnar Ness wrote: > > > > > Maxim: what needs change to get this merged? Followup will be mail pp > > > support, which I saw a patch for today from somone else. > > > > If this was a question to me, then the answer is: > > > > I'm not convinced this should be merged. For the use case of > > PROXY protocol bypass a more logical solution would be to avoid > > removing the PROXY protocol header at all. If one needs to bypass > > it and extract the information at the same time, something similar > > to ssl preread as recently implemented in the stream module might > > be a better solution. > > The usecase for getting access to the dst variables is that we have the: > > external LB -> nginx -> foo > > setup as mentioned earlier, and we want to take decisions based on the > destination inside nginx, this may for example be a multi-brand setup, where > the original destination address is a part of the query key to a database. > This will allow us to just do database changes, and nginx will immediately > be able to "see" for what destination this request was for. > > The separate, but related usecase is the "proxy-proxy-protocol" usecase, > where the upstream also needs the original ip/port src/destination data for > logging or decisionmaking. > > For this reason, I think it is practical to store the proxy protocol header as > a whole with ptr's in the connection struct, but I am fine with changing this > to inet_addr for example, will be more loc but a little less memory use. The implementation of PROXY protocol in nginx basically mimics what is available via X-Forwarded-For in http. Extending it with destination data may simplify some use cases. But it will also complicate things and will make other use cases less intuitive. It is also more or less clear that destination can be identified using a separate destination within nginx itself. As previously said, I'm not convinced this should be merged, as suggested changes have obvious downsides in common cases and only beneficial for very specific use cases. > > If you have some ideas on how to properly support PROXY protocol > > in mail - please comment on the relevant patch. > > Reason for mentioning it here, is the mail proxy-protocol patch has the > functionality mentioned here as a dependency. Both exim and dovecot > understands proxy-protocol, and I want to pass it on after the auth is done, > hence proxy-proxy-protocol. Current question is: What "listen ... proxy_protocol" should mean in case of mail. In other modules, it just means that PROXY protocol header is parsed and appropriate variables are available for use. It would be good to have similar meaning in mail, but there are realip module and no variables in mail. In the stream module similar problem was resolved by not introducing "listen ... proxy_protocol" till variables support was added, and by adding realip module at the same time. May be there are better options. I certainly dislike what is currently suggested, that is, just passing an address provided via PROXY protocol to backends via XCLIENT. Introducing PROXY protocol to backends instead of XCLIENT looks as a separate thing. -- Maxim Dounin http://nginx.org/ _______________________________________________ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
