Re: [PATCH] ConnStateData flexible transport support

On 29/04/2014 9:12 a.m., Alex Rousskov wrote:
> On 04/28/2014 07:10 AM, Amos Jeffries wrote:
>
>> * ssl-bump transforms the transportVersion from whatever it was
>> previously (usually HTTP or HTTPS) to HTTPS.
>
>> + /// the transport protocol currently being spoken on this connection
>> + AnyP::ProtocolVersion transportVersion;
>
>
> If I recall our earlier discussions correctly, "transport" is TCP, UDP,
> and such while HTTP and HTTPS are "transfer" protocols. It sounds like
> you want to use the new data member for transfer, not transport
> protocols. If yes, the member should be renamed accordingly.
>
> Also, if this is often about the protocol name rather than just its
> version, let's remove "Version" from the data member name (PortCfg
> already uses that approach).
>
> Finally, the patch does not actually use the version part of the
> transportVersion data member AFAICT. Perhaps the data member type should
> be changed from ProtocolVersion to ProtocolType?

I'm fine with "transferProtocol" or just "protocol" (although that may
get as common as "conn" has become). Whichever suits you.

The version is not used yet only because ssl-bump does not need it
(AFAIK. possibly if ssl-bump identified the client is using HTTP/0.9
inside the CONNECT tunnel it may want to do special things, but we don't
today).
 The 2.0 code needs the version to identify several key logic decisions,
like which parser to allocate/use on reads(2), how to manage the
pipeline, what to do with 10x messages etc.

>
>> - debugs(33, 5, HERE << "converting " << clientConnection << " to SSL");
>> + debugs(33, 5, "converting " << clientConnection << " to SSL");
>
> To reduce merge conflicts, please do not remove HERE from debugging
> lines that otherwise do not need to be changed.
>

Okay.

>
>> This variable can
>> be altered whenever necessary to cause an on-wire protocol change.
>
> Altering the data member does not cause an on-wire protocol change in
> the patched code AFAICT. Perhaps you meant that the data member should
> always reflect the current wire protocol?
>

Yes. However I was planning to use it to decode which Packer was
allocated. So it is both a reflection of the transfer protocol and a
cause of how messages converted to bytes.
 I am fine with documenting it as a reflection though.

Amos
Received on Tue Apr 29 2014 - 12:03:32 MDT