Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/cf.data.pre
Original file line number Diff line number Diff line change
Expand Up @@ -4781,6 +4781,7 @@ DOC_START
>p Client source port
>eui Client source EUI (MAC address, EUI-48 or EUI-64 identifier)
>la Local IP address the client connected to
>lA Local FQDN for address the client connected to

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please document (in cf.dada.pre) that these FQDNs are provided on a best-effort basis (i.e. that, when the corresponding DNS lookups are needed, Squid does not wait for them to complete when logging transactions).

>lp Local port number the client connected to
>qos Client connection TOS/DSCP value set by Squid
>nfmark Client connection netfilter packet MARK set by Squid
Expand All @@ -4805,6 +4806,7 @@ DOC_START
<A Server FQDN or peer name
<p Server port number of the last server or peer connection
<la Local IP address of the last server or peer connection
<lA Local FQDN for address of the last server or peer connection

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and elsewhere in this PR, please avoid the term "local FQDN". It is confusing (e.g., was some special a "local" DNS resolver used to obtain that FQDN?) and not descriptive enough ("address of the last server or peer connection" matches two different addresses, not one).

I do not insist on any specific wording, but something like this would address the problem:

Suggested change
<lA Local FQDN for address of the last server or peer connection
<lA FQDN for the local (a.k.a. source) address of the last Squid-to-server (including Squid-to-cache_peer) TCP connection

N.B. LFT_CLIENT_LOCAL_FQDN and LFT_SERVER_LOCAL_FQDN do not need to be renamed. In that existing naming pattern, "local" binds to "from-client" and "to-server" connection rather than binding to "FQDN".

<lp Local port number of the last server or peer connection
<qos Server connection TOS/DSCP value set by Squid
<nfmark Server connection netfilter packet MARK set by Squid
Expand Down
2 changes: 2 additions & 0 deletions src/format/ByteCode.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ typedef enum {

/* client TCP connection local end details */
LFT_CLIENT_LOCAL_IP,
LFT_CLIENT_LOCAL_FQDN,
LFT_CLIENT_LOCAL_PORT,
/*LFT_CLIENT_LOCAL_FQDN, (rDNS) */
Comment thread
yadij marked this conversation as resolved.
Outdated
LFT_CLIENT_LOCAL_TOS,
Expand All @@ -64,6 +65,7 @@ typedef enum {

/* server TCP connection local end details */
LFT_SERVER_LOCAL_IP,
LFT_SERVER_LOCAL_FQDN,
LFT_SERVER_LOCAL_IP_OLD_27,
LFT_SERVER_LOCAL_PORT,
LFT_SERVER_LOCAL_TOS,
Expand Down
13 changes: 13 additions & 0 deletions src/format/Format.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "format/Format.h"
#include "format/Quoting.h"
#include "format/Token.h"
#include "fqdncache.h"
#include "http/Stream.h"
#include "HttpRequest.h"
#include "MemBuf.h"
Expand Down Expand Up @@ -491,6 +492,12 @@ Format::Format::assemble(MemBuf &mb, const AccessLogEntry::Pointer &al, int logS
out = al->tcpClient->local.toStr(tmp, sizeof(tmp));
break;

case LFT_CLIENT_LOCAL_FQDN:
// May be too late for ours, but FQDN_LOOKUP_IF_MISS might help the next caller.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please mimic existing Dns::ResolveClientAddressesAsap logic to enable "early" or "as soon as possible" DNS lookups of these addresses.

I suggest naming the new flag ResolveClientLocalAddressesAsap and renaming the old flag to ResolveClientRemoteAddressesAsap. Using ResolveLocalClientAddressesAsap and ResolveRemoteClientAddressesAsap is also OK but does not match our LFT_CLIENT_LOCAL_FQDN naming pattern.

Same for tcpServer->local lookups, of course -- Dns::ResolveServerLocalAddressesAsap.

if (al->tcpClient)
out = fqdncache_gethostbyaddr(al->tcpClient->local, FQDN_LOOKUP_IF_MISS);
break;

case LFT_CLIENT_LOCAL_TOS:
if (al->tcpClient) {
sb.appendf("0x%x", static_cast<uint32_t>(al->tcpClient->tos));
Expand Down Expand Up @@ -532,6 +539,12 @@ Format::Format::assemble(MemBuf &mb, const AccessLogEntry::Pointer &al, int logS
out = al->hier.tcpServer->local.toStr(tmp, sizeof(tmp));
break;

case LFT_SERVER_LOCAL_FQDN:
// May be too late for ours, but FQDN_LOOKUP_IF_MISS might help the next caller.
if (al->hier.tcpServer)
out = fqdncache_gethostbyaddr(al->hier.tcpServer->local, FQDN_LOOKUP_IF_MISS);
break;

case LFT_SERVER_LOCAL_PORT:
if (al->hier.tcpServer) {
outint = al->hier.tcpServer->local.port();
Expand Down
7 changes: 5 additions & 2 deletions src/format/Token.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,15 @@ static TokenTableEntry TokenTable1C[] = {
static TokenTableEntry TokenTable2C[] = {

TokenTableEntry(">la", LFT_CLIENT_LOCAL_IP),
TokenTableEntry("la", LFT_LOCAL_LISTENING_IP),
TokenTableEntry(">lA", LFT_CLIENT_LOCAL_FQDN),
TokenTableEntry(">lp", LFT_CLIENT_LOCAL_PORT),
TokenTableEntry("lp", LFT_LOCAL_LISTENING_PORT),

TokenTableEntry("la", LFT_LOCAL_LISTENING_IP),
/*TokenTableEntry( "lA", LFT_LOCAL_NAME ), */
Comment thread
yadij marked this conversation as resolved.
Outdated
TokenTableEntry("lp", LFT_LOCAL_LISTENING_PORT),

TokenTableEntry("<la", LFT_SERVER_LOCAL_IP),
TokenTableEntry("<lA", LFT_SERVER_LOCAL_FQDN),
TokenTableEntry("oa", LFT_SERVER_LOCAL_IP_OLD_27),
TokenTableEntry("<lp", LFT_SERVER_LOCAL_PORT),

Expand Down
Loading