On Sun, Apr 17, 2022 at 12:51:42PM +0200, Dominik Derigs wrote: > On Fri, 2022-04-15 at 10:17 +0200, Geert Stappers wrote: > > 2022-04-10, Dominik Derigs wrote: } } } Subject: [PATCH] Also log upstream port for dnssec-retry } } there should be more text about the why. > > > - log_query_mysockaddr(F_NOEXTRA | F_DNSSEC, > > > daemon->namebuff, &srv->addr, > > > - "dnssec-retry", (forward-flags & > > > FREC_DNSKEY_QUERY) ? T_DNSKEY : T_DS); > > > + log_query_mysockaddr(F_NOEXTRA | F_DNSSEC | F_SERVER, > > > daemon->namebuff, &srv->addr, > > > + (forward->flags & FREC_DNSKEY_QUERY) > > > ? "dnssec-retry[DNSKEY]" : "dnssec-retry[DS]", 0); those lines shuffled: > > > - log_query_mysockaddr(F_NOEXTRA | F_DNSSEC, > > > daemon->namebuff, &srv->addr, > > > + log_query_mysockaddr(F_NOEXTRA | F_DNSSEC | F_SERVER, > > > daemon->namebuff, &srv->addr, > > > + (forward->flags & FREC_DNSKEY_QUERY) > > > ? "dnssec-retry[DNSKEY]" : "dnssec-retry[DS]", 0); > > > - "dnssec-retry", (forward-flags & > > > FREC_DNSKEY_QUERY) ? T_DNSKEY : T_DS);
white space change on the above last two lines to align the , > > > + (forward->flags & FREC_DNSKEY_QUERY) ? "dnssec-retry[DNSKEY]" : > > > "dnssec-retry[DS]", 0); > > > - > > > "dnssec-retry", (forward-flags & FREC_DNSKEY_QUERY) ? T_DNSKEY : T_DS); > > I see more changes as commit message says. > > What do you see in addition? I was expecting to see only an addition of logging a port number. The extra '| F_SERVER' did surprise me. And the '(forward->flags & FREC_DNSKEY_QUERY) ? "dnssec-retry[DNSKEY]" : "dnssec-retry[DS]"' becoming '"dnssec-retry"', plus a '0' becoming '(forward-flags & FREC_DNSKEY_QUERY) ? T_DNSKEY : T_DS)' realy made me (poorly) expressing "the patch needs a much better commit message" > It is a minimal invasive change that fixes the omission in a > previous commit as already said in the first mail: > > On Sun, 2022-04-10 at 10:46 +0200, Dominik Derigs wrote: > > This is added by this patch implementing it in the same way as > > used already when logging "dnssec-query" in the code. > > This is the commit, if you want to compare the change yourself: > https://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=commit;h=ff43d35aeef6178f7471c6f37e91845c9a72bd2f Partial https://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=commitdiff;h=ff43d35aeef6178f7471c6f37e91845c9a72bd2f --- a/src/forward.c +++ b/src/forward.c @@ -123,9 +123,17 @@ static void set_outgoing_mark(struct frec *forward, int fd) static void log_query_mysockaddr(unsigned int flags, char *name, union mysockaddr *addr, char *arg, unsigned short type) { if (addr->sa.sa_family == AF_INET) - log_query(flags | F_IPV4, name, (union all_addr *)&addr->in.sin_addr, arg, type); + { + if (flags & F_SERVER) + type = ntohs(addr->in.sin_port); + log_query(flags | F_IPV4, name, (union all_addr *)&addr->in.sin_addr, arg, type); + } else - log_query(flags | F_IPV6, name, (union all_addr *)&addr->in6.sin6_addr, arg, type); + { + if (flags & F_SERVER) + type = ntohs(addr->in6.sin6_port); + log_query(flags | F_IPV6, name, (union all_addr *)&addr->in6.sin6_addr, arg, type); + } } (saying "link was visited, an attempt to compare was done") > Happy Easter and best regards, > Dominik > Thanks, you also. Thing to consider: resubmit the patch with a better commit message. Other thing we should think about: A clear signal like "patch is rejected" Groeten Geert Stappers In another attempt to get beyond ignored patches -- Silence is hard to parse _______________________________________________ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
