[squid-dev] [PATCH] Non-HTTP bypass

[Amos Jeffries]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 22/10/2014 9:12 p.m., Tsantilas Christos wrote:
> On 10/21/2014 04:29 PM, Amos Jeffries wrote:
>> 2) All changes in src/tunnel.cc seem to be needless.
> 
> Some changes are required!
> 
>> - tunnelStartShovelling() should *always* be the entrypoint to
>> begin transmit on a tunnel in any direction. At that point there
>> is maybe client data to send to server ...
> 
> Yes.
> 
>> - The socket may not even permit one whole TCP_RCV_BUF worth of
>> bytes to be written in a single action. comm_write can handle
>> that just fine. If comm_write is unable to handle all the
>> available conn->in.buf in one comm_write() call then that would
>> be a bug in comm to fix separate from all this.
> 
> This is true. However we are using two buffers to read/write
> to/from server/client. The TunnelState::client::buf and
> TunnelState::server::buf which are of size SQUID_TCP_SO_RCVBUF
> 
> At early stages of this patch I tried to convert these buffers to
> SBufs , but required many changes in the tunnel.cc code, so I
> revert the changes and finally implemented the
> TunnelStateData::copyClientBytes member.

Arg, sorry you are right. I forgot that my previous conversion to SBuf
did not make it through Alex audit requirements.

Is this an urgent need? or can we work on fixing that problem first to
reduce the changes this feature requires?

> 
>> There is simply no reason to add an extra if 
>> (preReadClientData.length()) conditional in *every* packet I/O
>> cycle.
> 
> 
> As you can see the old code copies the ConnStateData::in->buf data
> to TunnelStateData->client.buf buffer. This is because in the case
> you are read a CONNECT request you can be sure that the extra bytes
> you read are not hugger than the SQUID_TCP_SO_RCVBUF.
> 
> But for this patch imagine the case where: - Squid configured
> with: request_header_max_size 70kbs - Client sends something which
> looks like an HTTP request eg: AMETHOD [spaces]* string [spaces]*
> ... Other data Also assume that this is more than the size of 70k. 
> - Squid will read 70k before decide that this is not an HTTP
> request. - The 70k are not fit to TunnelStateData->client.buf
> 
> An alternate solution is to add some code to build/fix 
> TunnelStateData->client.buf to have the required size. I need to
> recheck, but looks a good solution. Is it ok?
> 

Okay. I find it acceptible as a workaround for the buffer limitation.

I would rather see the limit fixed though. Alex and I have now almost
reached agreement on the Comm::Write API needed to SBuf the tunnel I/O
fully. If this bypass feature can wait for a while I will divert my
efforts to getting that completed now.

Amos
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (MingW32)

iQEcBAEBAgAGBQJUR6a1AAoJELJo5wb/XPRjZ4EH+gL4YoDwKFWIzV9FvzYLGE0n
wxY1We5Nk3PqP3YH/BTMnoA0XMdQ9+SPd4sHFVhU3lnx/xUztI83i/2k18+6d4SH
Dl1NHS/pBsIkCvwJ6hc3ZDgG3+LTkHv5A0UuYqU5PbLkU63TPoNkFT6elfXZMNkL
8K/vSw75GpI+0AYGTw7NbqHuuG8XLM1CL8Y7jKug9SiZn9w/9RFsa4BmOZ4LYBa6
nQdjmus5dcdLk0OMF4oAGnR6XytzwjO47TiOywE9FcbzismKO/aRCdq5pV5sLJHe
xDTfcmB9OJiOvPbMLhlilWtc1oveoYtWsyKQy1kPk1tXj+/KPLi8HKEaN2onjg8=
=+Iz9
-----END PGP SIGNATURE-----