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.

Sign up for GitHub

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

Jump to bottom

adding tcp_info struct to linux musl/glibc. #3480

Merged
merged 1 commit into from
Sep 3, 2024
Merged

adding tcp_info struct to linux musl/glibc. #3480

merged 1 commit into from
Sep 3, 2024
+114 −1

Conversation

devnexen
Copy link
Contributor

No description provided.

@rustbot
Copy link
Collaborator

rustbot commented Dec 15, 2023

r? @JohnTitor

(rustbot has picked a reviewer for you, use r? to override)

@JohnTitor
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Dec 15, 2023

📌 Commit 89df119 has been approved by JohnTitor

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Dec 15, 2023

⌛ Testing commit 89df119 with merge 28aeb23...

bors added a commit that referenced this pull request Dec 15, 2023
@bors
Auto merge of #3480 - devnexen:tcp_info_linux, r=JohnTitor
28aeb23 
adding tcp_info struct to linux musl/glibc.
@bors
Copy link
Contributor

bors commented Dec 15, 2023

💔 Test failed - checks-actions

@bors
Copy link
Contributor

bors commented Dec 19, 2023

☔ The latest upstream changes (presumably #3486) made this pull request unmergeable. Please resolve the merge conflicts.

@JohnTitor
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Dec 20, 2023

📌 Commit 97a839d has been approved by JohnTitor

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Dec 20, 2023

⌛ Testing commit 97a839d with merge 3c6c040...

bors added a commit that referenced this pull request Dec 20, 2023
@bors
Auto merge of #3480 - devnexen:tcp_info_linux, r=JohnTitor
3c6c040 
adding tcp_info struct to linux musl/glibc.
@bors
Copy link
Contributor

bors commented Dec 20, 2023

💔 Test failed - checks-actions

@JohnTitor
Copy link
Member

Could you check the CI failure? Seems musl gets confused 🤔

@JohnTitor
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Dec 29, 2023

📌 Commit 4234bd2 has been approved by JohnTitor

It is now in the queue for this repository.

@devnexen
Copy link
Contributor Author

devnexen commented Sep 1, 2024

@rustbot review

tgross35
tgross35 reviewed Sep 3, 2024
View reviewed changes
src/unix/linux_like/linux/musl/mod.rs Outdated
pub tcpi_backoff: u8,
pub tcpi_options: u8,
/*
* FIXME: when musl headers are more up to date
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* FIXME: when musl headers are more up to date
* FIXME(musl): when musl headers are more up to date

Just so we can grep these things easy at the musl update

tgross35
tgross35 reviewed Sep 3, 2024
View reviewed changes
src/unix/linux_like/linux/musl/mod.rs Outdated
pub tcpi_bytes_retrans: u64,
pub tcpi_dsack_dups: u32,
pub tcpi_reord_seen: u32,
// FIXME: to uncomment once CI musl is updated
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// FIXME: to uncomment once CI musl is updated
// FIXME(musl): to uncomment once CI musl is updated

Same as above

tgross35
tgross35 reviewed Sep 3, 2024
View reviewed changes
src/unix/linux_like/linux/gnu/mod.rs Outdated
Comment on lines 466 to 469
// bitfields 4
pub tcpi_snd_wscale: u8,
// bitfields 4
pub tcpi_rcv_wscale: u8,
Copy link
Contributor

@tgross35 tgross35 Sep 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The C header has these as

uint8_t	tcpi_snd_wscale : 4, tcpi_rcv_wscale : 4;

doesn't that mean tcpi_snd_wscale and tcpi_rcv_wscale are located within the same u8, and should be a single field in the Rust struct? Feel free to correct me if this is wrong.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was trying to find what we do here in other cases, but it looks like this doesn't come up very often. Probably just make it one field and add a comment so users know:

/// The bitfield containing `tcpi_snd_wscale` and `tcpi_rcv_wscale`, four bits each.
pub tcpi_snd_rcv_wscale: u8

The musl section that is commented out should probably say something similar.

@tgross35
Copy link
Contributor

tgross35 commented Sep 3, 2024 •
edited
Loading

Checking headers https://github.com/kraj/glibc/blob/1927f718fcc48bdaea03086bdc2adf11279d655b/sysdeps/gnu/netinet/tcp.h#L226-L267 and https://github.com/kraj/musl/blob/2c41ee96f60074fa8ac387e1fcdedba0654fb173/include/netinet/tcp.h#L195-L250 this looks good, with some notes above.

@rustbot author since I think this might need a change for the bitfield

tgross35
tgross35 reviewed Sep 3, 2024
View reviewed changes
src/unix/linux_like/linux/gnu/mod.rs Outdated Show resolved Hide resolved
src/unix/linux_like/linux/musl/mod.rs Outdated Show resolved Hide resolved
src/unix/linux_like/linux/musl/mod.rs Outdated Show resolved Hide resolved
@tgross35
Copy link
Contributor

tgross35 commented Sep 3, 2024

Thanks!

@rustbot label +stable-nominated

@rustbot rustbot added the stable-nominated This PR should be considered for cherry-pick to libc's stable release branch label Sep 3, 2024
@tgross35 tgross35 added this pull request to the merge queue Sep 3, 2024
Merged via the queue into rust-lang:main with commit 2899529 Sep 3, 2024
41 checks passed
This was referenced Sep 3, 2024
Merged
Open
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Oct 15, 2024
@devnexen @tgross35
adding tcp_info struct to linux musl/glibc.
a5adc5f 
(backport <rust-lang#3480>)
(cherry picked from commit ee0bf57)
@tgross35 tgross35 mentioned this pull request Oct 15, 2024
Merged
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Oct 15, 2024
@devnexen @tgross35
adding tcp_info struct to linux musl/glibc.
4da0c1c 
(backport <rust-lang#3480>)
(cherry picked from commit ee0bf57)
@tgross35 tgross35 added stable-applied This PR has been cherry-picked to libc's stable release branch and removed stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JohnTitor JohnTitor JohnTitor approved these changes

@tgross35 tgross35 tgross35 approved these changes

Assignees

@JohnTitor JohnTitor

Labels
S-waiting-on-ci stable-applied This PR has been cherry-picked to libc's stable release branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@devnexen @rustbot @JohnTitor @bors @tgross35