Skip to content

std: net: uefi: Add support to query connection data #143838

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
merged 1 commit into from
Jul 24, 2025

Conversation

Ayush1325
Copy link
Contributor

  • Use EFI_TCP4_GET_MODE_DATA to be able to query for ttl, nodelay, peer_addr and socket_addr.
  • peer_addr is needed for implementation of accept.
  • cc @nicholasbishop
  • Also a heads up. The UEFI spec seems to be wrong or something for EFI_TCP4_CONFIG_DATA. ControlOption should be a pointer as seen in edk2.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2025

r? @tgross35

rustbot has assigned @tgross35.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jul 12, 2025
Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

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

Just one nit from me, if @nicholasbishop can review as well that would be ideal

};

if r.is_error() {
return Err(io::Error::from_raw_os_error(r.as_usize()));
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
return Err(io::Error::from_raw_os_error(r.as_usize()));
Err(io::Error::from_raw_os_error(r.as_usize()))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, Thanks.

@Ayush1325 Ayush1325 force-pushed the uefi-tcp4-config-data branch from db1e118 to 883e25c Compare July 18, 2025 18:02
@@ -67,6 +68,29 @@ impl Tcp4 {
if r.is_error() { Err(crate::io::Error::from_raw_os_error(r.as_usize())) } else { Ok(()) }
}

pub(crate) fn get_mode_data(&self) -> io::Result<tcp4::ConfigData> {
// Using MaybeUninit::uninit() generates a Page Fault Here
let mut config_data: MaybeUninit<tcp4::ConfigData> = MaybeUninit::zeroed();
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Could you use ConfigData::default() here instead of MaybeUninit? Slightly simpler, and then the unsafe assume_init isn't needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, isn't MaybeUninit designed to be used in such cases?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, but it should only be used if necessary for performance. I wouldn't think that's a concern here, so I'd rather have simpler code and one fewer unsafe blocks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, isn't MaybeUninit designed to be used in such cases?

Technically yes it works here, but if there's a better option that reduces unsafe then there isn't any reason not to use it.

With MaybeUninit::zeroed() and later calling assume_init(), the assumption is made that zero is a valid bitpattern for all fields that (*protocol).get_mode_data doesn't overwrite. This is a nontrivial interaction and really be in a SAFETY comment. Also apparently get_mode_data expects at least partially initialized data if there is a page fault with uninit (presumably trying to read the pointer if nonnull?).

If you use default() then all these assumptions can go away. It's also more robust against adding/changing fields across a crate version bump.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, switched to using default. Also tested on ovmf.

@@ -761,3 +761,7 @@ impl Drop for OwnedEvent {
pub(crate) const fn ipv4_to_r_efi(addr: crate::net::Ipv4Addr) -> efi::Ipv4Address {
efi::Ipv4Address { addr: addr.octets() }
}

pub(crate) const fn ipv4_from_r_efi(ip: efi::Ipv4Address) -> crate::net::Ipv4Addr {
crate::net::Ipv4Addr::new(ip.addr[0], ip.addr[1], ip.addr[2], ip.addr[3])
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think this could be simplified to crate::net::Ipv4Addr::from(ip.addr) (see https://doc.rust-lang.org/std/net/struct.Ipv4Addr.html#impl-From%3C%5Bu8;+4%5D%3E-for-Ipv4Addr)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So From is not constant. I would prefer to keep this function constant.
There does exist from_octets, but that's experimental right now. I can enable the feature, but well I just thought this way might be better.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep fine to leave as-is, I missed that it's constant.

- Use EFI_TCP4_GET_MODE_DATA to be able to query for ttl, nodelay,
  peer_addr and socket_addr.
- peer_addr is needed for implementation of `accept`.

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
@Ayush1325 Ayush1325 force-pushed the uefi-tcp4-config-data branch from 883e25c to eb9c654 Compare July 23, 2025 05:53
@tgross35
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 23, 2025

📌 Commit eb9c654 has been approved by tgross35

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 23, 2025
fmease added a commit to fmease/rust that referenced this pull request Jul 23, 2025
…=tgross35

std: net: uefi: Add support to query connection data

- Use EFI_TCP4_GET_MODE_DATA to be able to query for ttl, nodelay, peer_addr and socket_addr.
- peer_addr is needed for implementation of `accept`.
- cc `@nicholasbishop`
- Also a heads up. The UEFI spec seems to be wrong or something for [EFI_TCP4_CONFIG_DATA](https://uefi.org/specs/UEFI/2.11/28_Network_Protocols_TCP_IP_and_Configuration.html#efi-tcp4-protocol-getmodedata). `ControlOption` should be a pointer as seen in [edk2](https://github.com/tianocore/edk2/blob/a1b509c1a453815acbc6c8b0fc5882fd03a6f2c0/MdePkg/Include/Protocol/Tcp4.h#L97).
fmease added a commit to fmease/rust that referenced this pull request Jul 23, 2025
…=tgross35

std: net: uefi: Add support to query connection data

- Use EFI_TCP4_GET_MODE_DATA to be able to query for ttl, nodelay, peer_addr and socket_addr.
- peer_addr is needed for implementation of `accept`.
- cc ``@nicholasbishop``
- Also a heads up. The UEFI spec seems to be wrong or something for [EFI_TCP4_CONFIG_DATA](https://uefi.org/specs/UEFI/2.11/28_Network_Protocols_TCP_IP_and_Configuration.html#efi-tcp4-protocol-getmodedata). `ControlOption` should be a pointer as seen in [edk2](https://github.com/tianocore/edk2/blob/a1b509c1a453815acbc6c8b0fc5882fd03a6f2c0/MdePkg/Include/Protocol/Tcp4.h#L97).
fmease added a commit to fmease/rust that referenced this pull request Jul 24, 2025
…=tgross35

std: net: uefi: Add support to query connection data

- Use EFI_TCP4_GET_MODE_DATA to be able to query for ttl, nodelay, peer_addr and socket_addr.
- peer_addr is needed for implementation of `accept`.
- cc ```@nicholasbishop```
- Also a heads up. The UEFI spec seems to be wrong or something for [EFI_TCP4_CONFIG_DATA](https://uefi.org/specs/UEFI/2.11/28_Network_Protocols_TCP_IP_and_Configuration.html#efi-tcp4-protocol-getmodedata). `ControlOption` should be a pointer as seen in [edk2](https://github.com/tianocore/edk2/blob/a1b509c1a453815acbc6c8b0fc5882fd03a6f2c0/MdePkg/Include/Protocol/Tcp4.h#L97).
bors added a commit that referenced this pull request Jul 24, 2025
Rollup of 15 pull requests

Successful merges:

 - #132748 (get rid of some false negatives in rustdoc::broken_intra_doc_links)
 - #143374 (Unquerify extern_mod_stmt_cnum.)
 - #143838 (std: net: uefi: Add support to query connection data)
 - #144014 (don't link to the nightly version of the Edition Guide in stable lints)
 - #144094 (Ensure we codegen the main fn)
 - #144218 (Use serde for target spec json deserialize)
 - #144221 (generate elf symbol version in raw-dylib)
 - #144240 (Add more test case to check if the false note related to sealed trait suppressed)
 - #144247 (coretests/num: use ldexp instead of hard-coding a power of 2)
 - #144276 (Use less HIR in check_private_in_public.)
 - #144317 (pass build.npm from bootstrap to tidy and use it for npm install)
 - #144320 (rustdoc: avoid allocating a temp String for aliases in search index)
 - #144334 (rustc_resolve: get rid of unused rustdoc::span_of_fragments_with_expansion)
 - #144335 (Don't suggest assoc ty bound on non-angle-bracketed problematic assoc ty binding)
 - #144358 (Stop using the old `validate_attr` logic for stability attributes)

r? `@ghost`
`@rustbot` modify labels: rollup
fmease added a commit to fmease/rust that referenced this pull request Jul 24, 2025
…=tgross35

std: net: uefi: Add support to query connection data

- Use EFI_TCP4_GET_MODE_DATA to be able to query for ttl, nodelay, peer_addr and socket_addr.
- peer_addr is needed for implementation of `accept`.
- cc ````@nicholasbishop````
- Also a heads up. The UEFI spec seems to be wrong or something for [EFI_TCP4_CONFIG_DATA](https://uefi.org/specs/UEFI/2.11/28_Network_Protocols_TCP_IP_and_Configuration.html#efi-tcp4-protocol-getmodedata). `ControlOption` should be a pointer as seen in [edk2](https://github.com/tianocore/edk2/blob/a1b509c1a453815acbc6c8b0fc5882fd03a6f2c0/MdePkg/Include/Protocol/Tcp4.h#L97).
bors added a commit that referenced this pull request Jul 24, 2025
Rollup of 16 pull requests

Successful merges:

 - #143374 (Unquerify extern_mod_stmt_cnum.)
 - #143838 (std: net: uefi: Add support to query connection data)
 - #144014 (don't link to the nightly version of the Edition Guide in stable lints)
 - #144094 (Ensure we codegen the main fn)
 - #144218 (Use serde for target spec json deserialize)
 - #144221 (generate elf symbol version in raw-dylib)
 - #144232 (Implement support for `become` and explicit tail call codegen for the LLVM backend)
 - #144240 (Add more test case to check if the false note related to sealed trait suppressed)
 - #144247 (coretests/num: use ldexp instead of hard-coding a power of 2)
 - #144276 (Use less HIR in check_private_in_public.)
 - #144278 (add Rev::into_inner)
 - #144317 (pass build.npm from bootstrap to tidy and use it for npm install)
 - #144320 (rustdoc: avoid allocating a temp String for aliases in search index)
 - #144334 (rustc_resolve: get rid of unused rustdoc::span_of_fragments_with_expansion)
 - #144335 (Don't suggest assoc ty bound on non-angle-bracketed problematic assoc ty binding)
 - #144358 (Stop using the old `validate_attr` logic for stability attributes)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Jul 24, 2025
Rollup of 15 pull requests

Successful merges:

 - #143374 (Unquerify extern_mod_stmt_cnum.)
 - #143838 (std: net: uefi: Add support to query connection data)
 - #144014 (don't link to the nightly version of the Edition Guide in stable lints)
 - #144094 (Ensure we codegen the main fn)
 - #144218 (Use serde for target spec json deserialize)
 - #144221 (generate elf symbol version in raw-dylib)
 - #144240 (Add more test case to check if the false note related to sealed trait suppressed)
 - #144247 (coretests/num: use ldexp instead of hard-coding a power of 2)
 - #144276 (Use less HIR in check_private_in_public.)
 - #144278 (add Rev::into_inner)
 - #144317 (pass build.npm from bootstrap to tidy and use it for npm install)
 - #144320 (rustdoc: avoid allocating a temp String for aliases in search index)
 - #144334 (rustc_resolve: get rid of unused rustdoc::span_of_fragments_with_expansion)
 - #144335 (Don't suggest assoc ty bound on non-angle-bracketed problematic assoc ty binding)
 - #144358 (Stop using the old `validate_attr` logic for stability attributes)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 319d54f into rust-lang:master Jul 24, 2025
11 checks passed
@rustbot rustbot added this to the 1.90.0 milestone Jul 24, 2025
rust-timer added a commit that referenced this pull request Jul 24, 2025
Rollup merge of #143838 - Ayush1325:uefi-tcp4-config-data, r=tgross35

std: net: uefi: Add support to query connection data

- Use EFI_TCP4_GET_MODE_DATA to be able to query for ttl, nodelay, peer_addr and socket_addr.
- peer_addr is needed for implementation of `accept`.
- cc `````@nicholasbishop`````
- Also a heads up. The UEFI spec seems to be wrong or something for [EFI_TCP4_CONFIG_DATA](https://uefi.org/specs/UEFI/2.11/28_Network_Protocols_TCP_IP_and_Configuration.html#efi-tcp4-protocol-getmodedata). `ControlOption` should be a pointer as seen in [edk2](https://github.com/tianocore/edk2/blob/a1b509c1a453815acbc6c8b0fc5882fd03a6f2c0/MdePkg/Include/Protocol/Tcp4.h#L97).
@Ayush1325 Ayush1325 deleted the uefi-tcp4-config-data branch July 25, 2025 03:41
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Jul 30, 2025
…=tgross35

std: net: uefi: Add support to query connection data

- Use EFI_TCP4_GET_MODE_DATA to be able to query for ttl, nodelay, peer_addr and socket_addr.
- peer_addr is needed for implementation of `accept`.
- cc `````@nicholasbishop`````
- Also a heads up. The UEFI spec seems to be wrong or something for [EFI_TCP4_CONFIG_DATA](https://uefi.org/specs/UEFI/2.11/28_Network_Protocols_TCP_IP_and_Configuration.html#efi-tcp4-protocol-getmodedata). `ControlOption` should be a pointer as seen in [edk2](https://github.com/tianocore/edk2/blob/a1b509c1a453815acbc6c8b0fc5882fd03a6f2c0/MdePkg/Include/Protocol/Tcp4.h#L97).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants