On 20/03/2012 5:10 p.m., Alex Rousskov wrote:
On 03/15/2012 05:08 AM, Amos Jeffries wrote:
Run-time testing begins this weekend, but it passes build testing and I
fgured it was ready for an audit check. If anyone else wants to assist
testing it that would be welcome. I am proposing this to go into trunk
for squid-3.3.
Split-stack support for ICP, HTCP, SNMP
FWIW, it would be better if http_port_list renaming and moving was not
done while implementing the primary patch changes IMHO. It seems like
you did not need to modify http_port_list itself and now it is not clear
which of those modifications I can complain about without being told
that any further polishing is outside your patch scope. Besides
complicating review, combining functional and style changes would also
make conflict resolution more difficult/risky for those of us who are
modifying port-related code.
Okay. I'm happy to split this in two parts:
1) cosmetic: renaming of http_port_list to AnyP::PortCfg. Renaming the
incoming_* config directives and improving their documentation
2) functional: migration of UDP ports to use AnyP::PortCfg
+ PortList(const char *aProtocol);
Please use "explicit" with any single-parameter constructors.
I would also s/PortList/PortCfg/ or similar since each object is not a
list but a single port. Ideally, we should use std::list<> or another
container to group these Port configurations together but that change
would probably be outside your project scope.
Okay. Done the rename.
std::list<> would be structural rather than just symbolic so left for
now. We are a bit constrained by the legacy parser using raw-pointers.
- http_port_list *s;
+ AnyP::PortList *s;
- for (s = Config.Sockaddr.http; s != NULL; s = (http_port_list *)
s->next) {
+ for (s = Config.Sockaddr.http; s != NULL; s = s->next) {
...
- }
-
- {
-
- http_port_list *s;
It would be better to make "s" local to each of the two "for" loops if
possible instead of making it "more global".
Okay. Done.
+ // - multiple multiple listening ports
+ // - multiple multiple transport types on listening ports
+ // - multiple multiple and non-ICP probe ports
Multiple multiples?
Oops. The tuples are multiplying. Open the airlock.
Rewritten, but do you have an answer to that question about IRCache?
Can we arrange an upgraded send-announce format that can support (a)
Squids existing ports and (b) cope with expected future abilities?
Or are we limited by IRCache legacy systems?
I wont be doing anything immediately, but it would be nice to know and
plan for.
* Restructure ModPoll, ModSelect to use new list of ports instead
of a fixed-size global array.
NOTE: It appears that HTTPS and HTCP were only checked once per I/O
loop iteration but HTTP and ICP had optimizations to increase their
receive rates.
Does your patch change that design? I am not quite sure it was a good
idea for Squid to accept more than it had time to process, but
regardless of whether the old design was sound, I would rather avoid
peformance-sensitive changes as a part of a seemingly
performance-neutral patch.
It does not change the design of the I/O magics. Just adds the HTCP and
HTTPS listening ports alongside the ICP+HTTP listening sockets. So that
setups using HTCP instead of ICP and/or HTTPS instead of HTTP get the
same I/O behaviour. A fairly trivial functional change, but unrelated to
the *_port update. Reverted for now.
I'll re-submit part (1) shortly when I can confirm it still builds. Are
there any other cosmetic changes to http_port_list you would like me to
include in that patch?
Amos
