Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

DECnet (again) and test gaps #1564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
infrastation merged 6 commits into the-tcpdump-group:master from infrastation:fix_decnet_again
Oct 16, 2025

Conversation

Copy link
Member

@infrastation infrastation commented Oct 15, 2025

These changes concern a number of similar code paths, which is why it took longer than I expected to enumerate the bugs and to make sure the tests cover the problem space. But here it is.

Even after my earlier commit 989e194 the current implementation still
fails to reject some invalid forms of DECnet syntax:
'decnet net net-10-20-30-0.libpcap.test': no error
'decnet net 1.2.3.0/24': no error
'decnet net 1.2.3.0 mask 255.255.255.0': no error
'decnet net 1.2': no error
Make the verification stricter to solve the remaining part of the
problem: in gen_scode() and gen_mcode() handle DECnet early and reject
all forms, in gen_ncode() handle DECnet early and reject all forms
except "host". Improve various comments to explain the context. For a
number of other "decnet" forms instead of misleading error messages
(e.g., "Mask syntax for networks only") return a more useful error
message:
'decnet net net-10-20-30-0.libpcap.test': 'decnet' is not a valid
 qualifier for 'net'
'decnet net 1.2.3.0/24': 'decnet' is not a valid qualifier for 'net'
'decnet net 1.2.3.0 mask 255.255.255.0': 'decnet' is not a valid
 qualifier for 'net'
'decnet net 1.2': 'decnet' is not a valid qualifier for 'net'
Unlike gen_ncode(), gen_mcode() and gen_mcode6(), there is exactly one
code path into gen_scode(), namely, through an ID in the grammar. This
way, when a comment says "ID -> gen_scode()", it states the obvious, and
when it says "pnum -> gen_scode()", it is wrong, so eliminate this noise
to improve the signal. Add the missing gen_host46_byname() to a couple
code paths.
The existing tests could not detect the recently fixed DECnet bugs
because the definitions in %pqual_features do not distinguish between
different forms of the ID. Then the subsequent foreach loop that
generates invalid "host" primitives does not generate any tests for
protocols that support a form of "host" no matter which forms of ID are
valid and which are not. Then the resulting reject tests have a number
of gaps, including IPv4 IDs in DECnet primitives.
To fix that, in %pqual_features replace the generic "host" with seven
more specific host/net features; in the loop skip the valid permutations
only and include MAC-48, ARCnet and DECnet IDs. The new test space
supersedes a number of earlier reject tests, so remove these. Document
the current code paths in the comments and introduce more helper
functions.
Copy link
Member Author

Apparently, the CI failures on illumos-amd64 are due to DNS delay:

Failed tests:
 reject_arp_host_nonexistent : filtertest timeout
 reject_ip6_host_nonexistent : filtertest timeout
 reject_ip_host_nonexistent : filtertest timeout
 reject_rarp_host_nonexistent : filtertest timeout

The host occasionally takes more than one second to fail resolving host.invalid. This reproduces using the current master branch and is not specific to the proposed changes. This pull request is going to be merged tomorrow.

@infrastation infrastation merged commit 33574de into the-tcpdump-group:master Oct 16, 2025
16 of 17 checks passed
@infrastation infrastation deleted the fix_decnet_again branch October 16, 2025 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

AltStyle によって変換されたページ (->オリジナル) /