fix recent regressions #1582
Merged
fix recent regressions #1582
+524
−216
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In my recent commit 1687065 the filter programs for "vxlan [NUM]", "vxlan and vxlan", "geneve [NUM]" and "geneve and geneve" have become substantially shorter. As it turns out, the missing instructions are the outputs of gen_geneve_offsets() and gen_vxlan_offsets(), so filtering of protocols encapsulated in GENEVE and VXLAN is no longer correct. Because libpcap does not have any apply filter tests for the affected syntax, this was not apparent until a tcpdump test failed. The instructions went missing because gen_geneve() and gen_vxlan() need to use these as a block, but instead of making a new block from scratch the functions take the result of gen_true() and modify it to add side effects. After the commit the block with the wanted side effects ended up incorrectly marked as a Boolean constant and susequently gen_and() started to discard it. In gen_cond() take an optional list of side effect instructions and make a Boolean constant block a specific case of a block with a degenerate branch statement. Switch gen_geneve() and gen_vxlan() from gen_true() to gen_uncond() to reduce the violation of layers. Restore the six previously affected tests exactly as they were before.
To detect gen_geneve() and gen_vxlan() regressions that affect filtering of encapsulated protocols, define two new apply filter tests. For now these stand for IPv4 only.
infrastation
force-pushed
the
fix_recent_regressions
branch
from
aea0c67 to
f94fce8
Compare
Member
Author
|
This revision passes the whitespace check. |
Six tests make it easy to see another regression I have introduced in commit 1687065.
I got the comments right, but not all of the code, fix that and update the tests.
infrastation
force-pushed
the
fix_recent_regressions
branch
from
f94fce8 to
3ed0cd0
Compare
Member
Author
|
I do not understand where this warning comes from: Let's see if it regards any of the two important bug fixes. This revision now comprises the four most important commits only. |
Member
|
Linked to: |
Member
|
But it seems, there is no recent update: |
Member
Author
|
Then for the purposes of this pull request I am going to ignore Appveyor breakage if everything else passes the CI. For the next pull request I am going to disable the specific permutation that fails. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This comprises two follow-ups to pull request #1573 and a few small improvements.