Log received ICP packets using debuging level 2

[yadij]

Initially just received packets which pass basic validity and origin filters.

cache.log output looks like:

... icpHandleUdp: ICP Client remote=127.0.0.1:35844 FD 15
... icpHandleUdp: ICP Client REQUEST:
---------
ICP_QUERY #10 ICP/2
:length: 28
:flags: 0x40000000
      payload=
7f000001697474713a2f2f7777772e6578616d706c652f636f6d2f00
----------

[yadij]

@rousskov, pushing back to you to close off all the resolved review threads. Hopefully my response about consistency on the payload info resolves or removes the other related comments.

[yadij]

@rousskov, current state is that I am waiting for a good conclusion to the thread from https://github.com/squid-cache/squid/pull/827#discussion_r638896499. That will determine whether the other open change requests happen.

[rousskov]

I am waiting for a good conclusion to the thread from #827 (comment). That will determine whether the other open change requests happen.

If you are waiting for me, my position has not changed: I have given up. This problem is just not worth my time. If I failed to convince you, leave minLevel(). No need to explain why I have not convinced you. I will do my best not to participate in further discussions on that issue.

[hno]

Have to agree with Alex here. In terms of Squid logs HTTP headers In ICP "payload" is not payload, it is the request headers of the HTTP request transmitted via ICP. Any decisions regarding verbosity of ICP transmitted HTTP header data should be aligned with how HTTP header data is handled in HTTP context. Log levels largely relates to security/sensitivity level and data meaning, not protocol terms.

For what it is worth the URL is also part of the "payload" in ICP terminology. The ICP header is pretty much just the ICP opcode with no details about the queried HTTP request, quite similar to an UDP header.

This said ICP may actually contain the object body, which in such case should be handled accordingly. But I am not sure if ICP is ever used in that manner by Squid.

But this is just my opinion. Should not be seen as review of the code, or an attempt to influence the code. Only an attempt to clarify terminology.

Regards Henrik

[yadij]

This PR is blocked by @rouskkov self-requested review and "I will do my best not to participate in further discussions" decision.

[rousskov]

This PR is blocked by @rouskkov self-requested review and "I will do my best not to participate in further discussions" decision.

Both assumptions are false and the misleading quote misrepresents the actual dialog:

• This PR has no active review requests. Not self-requested, not regular ones. None. There was one regular review request long time ago, and it has been satisfied long time ago.

• I actually said "I will do my best not to participate in further discussions on that issue" (emphasis added), clearly referring to one specific problem discussed in that comment. I know Henrik has commented on that issue as well, but my plan is still in place exactly as it was stated.

Amos: current state is that I am waiting for a good conclusion to the thread from https://github.com/squid-cache/squid/pull/827#discussion_r638896499. That will determine whether the other open change requests happen.

AFAICT, everybody who wanted to share their opinion on that thread have done so long time ago. It is your turn to act on that feedback. Once you think that my other change requests are addressed, please re-request my review.

[yadij]

If I failed to convince you, leave minLevel(). ... It is your turn to act on that feedback. ... and I have.

Once you think that my other change requests are addressed, please re-request my review.

Going by how all the github conversations are resolved and closed except that one - I think they are.

[rousskov]

I also adjusted PR title to be a bit more specific. Please polish further as needed, of course.