Skip to content
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

Add testing of bitflips in packet headers #22267

Closed
wants to merge 4 commits into from

Conversation

t8m
Copy link
Member

@t8m t8m commented Oct 3, 2023

A new type of noise is introduced in the noisy dgram bio filter.

Checklist
  • tests are added or updated

@t8m t8m added branch: master Merge to master branch approval: review pending This pull request needs review by a committer approval: otc review pending This pull request needs review by an OTC member triaged: bug The issue/pr is/fixes a bug tests: present The PR has suitable tests present labels Oct 3, 2023
@t8m
Copy link
Member Author

t8m commented Oct 3, 2023

This needs to be rebased after #22243 is merged.

@t8m t8m linked an issue Oct 3, 2023 that may be closed by this pull request
test/helpers/noisydgrambio.c Show resolved Hide resolved
test/helpers/noisydgrambio.c Outdated Show resolved Hide resolved
@mattcaswell
Copy link
Member

This needs to be rebased after #22243 is merged.

Care will need to be taken when rebasing. Flipped bits will (presumably) cause the datagram to be dropped, so we will need to back off that type of noise too.

Copy link
Contributor

@paulidale paulidale left a comment

Choose a reason for hiding this comment

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

With @mattcaswell's suggestions taken.

@paulidale paulidale removed the approval: otc review pending This pull request needs review by an OTC member label Oct 4, 2023
@mattcaswell
Copy link
Member

This needs a rebase now that #22243 has been merged.

A new type of noise is introduced in the noisy dgram bio
filter.
@t8m
Copy link
Member Author

t8m commented Oct 4, 2023

Rebased and @mattcaswell comments addressed.

Please reconfirm.

mattcaswell
mattcaswell previously approved these changes Oct 4, 2023
@mattcaswell mattcaswell dismissed their stale review October 4, 2023 10:22

CI is relevant

@mattcaswell
Copy link
Member

The no-cmp runchecker CI failure is relevant

@mattcaswell
Copy link
Member

CI failure is still happening

The test server cannot really cope with modifications
@t8m t8m added the approval: otc review pending This pull request needs review by an OTC member label Oct 4, 2023
@t8m t8m requested a review from paulidale October 4, 2023 15:56
Copy link
Member

@mattcaswell mattcaswell left a comment

Choose a reason for hiding this comment

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

Approved subject to the CI passing this time!

@t8m
Copy link
Member Author

t8m commented Oct 4, 2023

Approved subject to the CI passing this time!

It passed!

@t8m t8m removed the approval: otc review pending This pull request needs review by an OTC member label Oct 4, 2023
@paulidale paulidale removed the approval: review pending This pull request needs review by a committer label Oct 4, 2023
@paulidale paulidale added the approval: done This pull request has the required number of approvals label Oct 4, 2023
@openssl-machine openssl-machine added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Oct 6, 2023
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@t8m
Copy link
Member Author

t8m commented Oct 6, 2023

Merged to the master branch. Thank you for the reviews.

@t8m t8m closed this Oct 6, 2023
openssl-machine pushed a commit that referenced this pull request Oct 6, 2023
A new type of noise is introduced in the noisy dgram bio
filter.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #22267)
openssl-machine pushed a commit that referenced this pull request Oct 6, 2023
The test server cannot really cope with modifications

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #22267)
wanghao75 pushed a commit to openeuler-mirror/openssl that referenced this pull request Oct 9, 2023
A new type of noise is introduced in the noisy dgram bio
filter.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from openssl/openssl#22267)

Signed-off-by: fly2x <fly2x@hitls.org>
wanghao75 pushed a commit to openeuler-mirror/openssl that referenced this pull request Oct 9, 2023
The test server cannot really cope with modifications

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from openssl/openssl#22267)

Signed-off-by: fly2x <fly2x@hitls.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch tests: present The PR has suitable tests present triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tests: Malformed packet header
4 participants