Skip to content

Factor gen_host46_byname() out.#1519

Merged
infrastation merged 7 commits intothe-tcpdump-group:masterfrom
infrastation:gen_host46_byname
Oct 9, 2025
Merged

Factor gen_host46_byname() out.#1519
infrastation merged 7 commits intothe-tcpdump-group:masterfrom
infrastation:gen_host46_byname

Conversation

@infrastation
Copy link
Copy Markdown
Member

This is the more difficult part of a prototype I started, but could not quickly complete earlier. The intent is simple: since gateway X nominally includes not host X, and since both gen_gateway() and gen_scode() call pcap_addrinfo() and process the result(s) in a very similar manner, having a slightly different code block in each function makes the code harder to maintain. However, the complexity turned out to be in the detail (including #1512), so this work-in-progress revision fails many tests. Also before this can be progressed any further, the reject filter tests must cover all code paths that go through pcap_addrinfo() and fail to resolve the hostname.

@infrastation infrastation marked this pull request as draft May 22, 2025 15:53
Show gen_mac48host_byname() in the code path.  The comments about
pcap_ether_hostton() do not apply since commit ad56fa2.
For all MAC-48 DLTs generalise two existing EN10MB tests and introduce
three more tests to cover all six varieties of ID that "gateway"
rejects.  For all other DLTs introduce five more tests to cover all
eight varieties of ID that "gateway" rejects and run all tests in one
foreach loop.
Add three new reject tests to cover more cases when a proto-qualified
"host NAME" encounters a hostname that either does not resolve at all or
only resolves to an address family that the protocol cannot use.  Add
four new reject tests to cover the current [incorrect] behaviour of
"arp host NAME" and "rarp host NAME" when the hostname resolves in IPv6,
regardless of whether it resolves in IPv4 (GH bug the-tcpdump-group#1512).  Update three
existing tests for consistency and place all the tests in one place.
For these two forms of the predicate, when a hostname resolves in both
IPv4 and IPv6, the IPv6 part is supposed to be irrelevant, but it causes
an unexpected failure (GH bug the-tcpdump-group#1512).  Also, when a hostname does not
resolve in IPv4 and resolves in IPv6, the error message is supposed to
be correct, but it is not:

$ filtertest EN10MB 'arp host eth-ipv4-ipv6.host123.libpcap.test'
filtertest: 'arp' is not a valid qualifier for 'ip6 host'
$ filtertest EN10MB 'rarp host eth-ipv4-ipv6.host123.libpcap.test'
filtertest: 'rarp' is not a valid qualifier for 'ip6 host'
$ filtertest EN10MB 'arp host eth-noipv4-ipv6.host123.libpcap.test'
filtertest: 'arp' is not a valid qualifier for 'ip6 host'
$ filtertest EN10MB 'rarp host eth-noipv4-ipv6.host123.libpcap.test'
filtertest: 'rarp' is not a valid qualifier for 'ip6 host'

In gen_scode() case Q_HOST get the IPv6 address skip condition right to
fix this:

$ filtertest EN10MB 'arp host eth-ipv4-ipv6.host123.libpcap.test'
(000) ldh      [12]
(001) jeq      #0x806           jt 2	jf 7
(002) ld       [28]
(003) jeq      #0xa141e28       jt 6	jf 4
(004) ld       [38]
(005) jeq      #0xa141e28       jt 6	jf 7
(006) ret      #262144
(007) ret      #0
$ filtertest EN10MB 'rarp host eth-ipv4-ipv6.host123.libpcap.test'
(000) ldh      [12]
(001) jeq      #0x8035          jt 2	jf 7
(002) ld       [28]
(003) jeq      #0xa141e28       jt 6	jf 4
(004) ld       [38]
(005) jeq      #0xa141e28       jt 6	jf 7
(006) ret      #262144
(007) ret      #0
$ filtertest EN10MB 'arp host eth-noipv4-ipv6.host123.libpcap.test'
filtertest: unknown host 'eth-noipv4-ipv6.host123.libpcap.test' for
  specified address family
$ filtertest EN10MB 'rarp host eth-noipv4-ipv6.host123.libpcap.test'
filtertest: unknown host 'eth-noipv4-ipv6.host123.libpcap.test' for
  specified address family

In pcap-filter(7) explain the address family semantics better in the
description of "host NAME".  Update the tests.
@infrastation infrastation marked this pull request as ready for review October 7, 2025 19:26
@infrastation
Copy link
Copy Markdown
Member Author

The previous revision failed 80 tests, this revision passes all tests, including the new tests it adds. It also includes a bug fix for the bug #1512. It passes a build matrix with Valgrind on Linux.

@infrastation infrastation changed the title DRAFT: Factor gen_host46_byname() out. Factor gen_host46_byname() out. Oct 7, 2025
In gen_scode() case Q_GATEWAY delegate the remaining memory management
and error handling downstream; now if gen_gateway() returns, it always
returns a non-NULL pointer.  In gen_gateway() handle all errors as early
as possible; instead of testing whether ai_addr is NULL test whether
ai_family is set to the AF of interest: when that's the case, ai_addr is
not NULL (this is how gen_scode() implements case Q_HOST).
@infrastation
Copy link
Copy Markdown
Member Author

This revision handles integer types a bit better, let's see if that clears the Windows CI failure.

After the previous commit much of what gen_gateway() does is the IPv4
subset of case Q_HOST in gen_scode().  Factor the common code to a new
function and update various comments.
@infrastation
Copy link
Copy Markdown
Member Author

This revision spells an assignment in a conditional expression in a way that hopefully won't upset MSVC, let's see if that worked.

@infrastation infrastation merged commit c64aba1 into the-tcpdump-group:master Oct 9, 2025
21 checks passed
@infrastation infrastation deleted the gen_host46_byname branch October 9, 2025 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

1 participant