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

Change: boreas. Use sequence number 1. #813

Merged
merged 2 commits into from
Apr 22, 2024
Merged

Change: boreas. Use sequence number 1. #813

merged 2 commits into from
Apr 22, 2024

Conversation

jjnicola
Copy link
Member

@jjnicola jjnicola commented Apr 16, 2024

What

Fix: boreas. Use sequence number 1
Close #812
Jira: SC-1064

Why

References

Checklist

  • Tests

@jjnicola jjnicola requested a review from a team as a code owner April 16, 2024 13:03
Copy link

@co60ca co60ca left a comment

Choose a reason for hiding this comment

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

Hope you don't mind my review. I'm not well versed in C so please take my review with the utmost of respect.

boreas/ping.c Show resolved Hide resolved
boreas/ping.c Outdated Show resolved Hide resolved
@jjnicola jjnicola force-pushed the boreas-seq-number branch 3 times, most recently from ba4c5f6 to ee081c5 Compare April 17, 2024 06:25
@jjnicola jjnicola requested a review from a team as a code owner April 17, 2024 07:13
mattmundell
mattmundell previously approved these changes Apr 17, 2024
@cfi-gb
Copy link
Member

cfi-gb commented Apr 18, 2024

One note on the usage of "Fix:" for the conventional commit message:

Shouldn't this be "Change:" instead? Asking because when using a "Fix" it will end up as a "Bug Fix" in the Changelog but this is far from being a Bug as the RFC is actually allowing to use a 0/zero:

If code = 0, a sequence number to aid in matching echos and replies, may be zero.

https://datatracker.ietf.org/doc/html/rfc792

and this is more about a compatibility with some specific systems not following the specs.

@jjnicola jjnicola force-pushed the boreas-seq-number branch from 566bae7 to d8d5d1e Compare April 18, 2024 07:43
@jjnicola jjnicola changed the title Fix: boreas. Use sequence number 1. Change: boreas. Use sequence number 1. Apr 18, 2024
@jjnicola jjnicola enabled auto-merge (squash) April 22, 2024 07:17
@jjnicola jjnicola disabled auto-merge April 22, 2024 08:53
@jjnicola jjnicola merged commit 56f65c1 into main Apr 22, 2024
10 checks passed
@jjnicola jjnicola deleted the boreas-seq-number branch April 22, 2024 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Boreas use of ICMP Echo with 0 ID can cause issues on some networks
6 participants