Re: [MERGE] Initial netfilter mark patch for comment

Updated patch attached; comments below.

> > If we can move to strtoul, I would like to change 'tos' to char
> > throughout. Currently it is possible to set it to invalid values in
> > squid.conf, which then cause problems with dumpConfigLine.
> >
> > Question number 2: what is stubQosConfig.cc? Does that also need
> > updating for this patch?
> >
>
> stub* are cut down set of all non-inline Ip::QosConfig methods and any
> globals defined in QosConfig.h. Changes to the API need to be mirrored
> there. The functions inside usually call fatal() to alert a wrong
> linkage clearly during testing. In this particular case the parse
> function will need to be duplicated there.

Sorry, I need some clarification on this. I've looked at most of the
other stub files, and most of the functions do indeed just return
fatal(). However, the existing stubQosConfig.cc is almost identical to
QosConfig.cc, with almost all of its code. Shall I change the functions
to fatal()?

Presumably I should add all the new functions into it (getTosLocalMiss,
getNfmarkLocalMiss, getTosLocalHit, getNfmarkLocalHit, getUpstreamTOS,
getUpstreamNfMark, isTosActive, isMarkActive, GetNfMarkCallback)?

> I have failed to find any problems with strtoul. Looks like it can be
> used. Although we may have to find a GPLv2 compatible implementation.

I have kept the strtoul parts, and changed (almost) all of the tos
variables to unsigned char.

>
> In this version the new methods look they should be in the Ip::Qos
> namespace.
>
> * the new clientReplyContext::tosLocalMissValue() and
> clientReplyContext::nfmarkLocalMissValue() methods particularly look
> like they really should take the hier code as a second parameter. Both
> fd and the hier if passed in should be const.

I've moved them to Ip::Qos and added the parameters as const.

> * the new FwdState getUpstream* methods are in a similar position but
> not quite so clear cut. If you can find way to cleanly move them there
> great, otherwise it's not so much a bit deal yet.

I've moved these, as well as most of the other QOS functions, into
Ip::Qos. I have also removed the QosConfig namespace, as it didn't seem
to fit with all these additional functions.

> * Thank you for the extra documentation on comm_set_tos().
>
> * Please remove the bits touching comm_set_v6only() its 'tos' field is
> not related to QoS. Just acronym confusion.

My mistake, removed.

I've carried out a fair bit of testing on the mark functionality today,
and It Works For Me, but I'll be able to try it better in a production
server in the coming weeks.

Could you let me know if/what else I need to do before acceptance. I
believe there are the following:

- Confirm and implement stub function requirements
- Factor the configure.in changes into Kinkie's autoconf-refactor?

Regards,

Andy