Hi David,

On Thu, May 08, 2014 at 11:42:08PM -0400, David S wrote:
> On Thu, May 8, 2014 at 6:01 PM, Willy Tarreau <w...@1wt.eu> wrote:
> 
> > Hi David,
> >
> > (.....)
> >
> 
> 
> > OK.
> >
> > I'd like to issue dev25 this week-end since we fixed a number of nasty
> > issues. Do you think you can make the adjustments by then ? I'd really
> > like to merge your work so that you have something to play with in the
> > official version, without having to keep a patch that needs to be rebased
> > from time to time. Do not hesitate to tell me if you need some help (eg:
> > if you feel uncomfortable with the change to move SRV_SEND_PROXY out of
> > ->state and into ->pp_opts).
> >
> > Thanks!
> > Willy
> >
> >
> Hi Willy--
>    Thank you for the excellent code review and feedback.  I have
> implemented all of your suggestions, except one, where I would like to
> suggest an alternative.
> 
> For the TLV format, I propose:
> 
>         struct {
>             uint8_t type;
>             uint8_t length_hi;
>             uint8_t length_lo;
>             uint8_t value[0];
>         } tlv;
> 
> This still addresses our concern around byte alignment.  I prefer this
> order, where length appears last, because it is consistent with the main V2
> header.
> 
> With this format, in both cases, the length field indicates the number that
> follow the length field.
> If Type comes after Length, then the length field would sometimes mean the
> number of bytes following and sometimes mean the number of bytes following
> minus 1.

Yeah that makes sense, you're right. Better not have impossible values there,
TCP did this mistake already with the data offset that can be encoded as being
before the end of headers, and that requires an extra test. Let's not do the
same in haproxy :-)

> In the end, either is OK with me.  I have tested the code both ways.  The
> code difference is only changing the order in types/connection.h.
> 
> An extended commit message:
> This commit modifies the PROXY protocol V2 specification to support headers
> longer than 255 bytes allowing for optional extensions.  It implements the
> PROXY protocol V2 which is a binary representation of V1. This will make
> parsing more efficient for clients who will know in advance exactly how
> many bytes to read.  Also, it defines and implements some optional PROXY
> protocol V2 extensions to send information about downstream SSL/TLS
> connections.  Support for PROXY protocol V1 remains unchanged.

Fine, I'm applying and committing with this.

Thanks,
Willy


Reply via email to