Skip to content

std: sys: net: uefi: tcp4: Add timeout support #143568

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

std: sys: net: uefi: tcp4: Add timeout support #143568

merged 1 commit into from
Jul 12, 2025
+97 −25

Conversation

Ayush1325
Copy link
Contributor

  • Implement timeout support for read, write and connect.
  • A software implementation using Instant.

@rustbot
Copy link
Collaborator

rustbot commented Jul 7, 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 7, 2025
@Ayush1325 Ayush1325 mentioned this pull request Jul 7, 2025
Open
@tgross35
Copy link
Contributor

tgross35 commented Jul 7, 2025

Cc @nicholasbishop for UEFI

nicholasbishop
nicholasbishop reviewed Jul 8, 2025
View reviewed changes
library/std/src/sys/net/connection/uefi/tcp4.rs
@@ -167,6 +168,31 @@ impl Tcp4 {
}
}

unsafe fn wait_or_cancel(
Copy link
Contributor

Choose a reason for hiding this comment

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

Here and on cancel, a docstring that includes the # Safety requirements would be good to add.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you fill in the rest of the docstring (say what behavior is, not just the safety req).

nit: I don't think it's a safety requirement that the token come from connect/accept/transmit/receive, just a logical requirement. The spec says EFI_NOT_FOUND is returned is the token is unknown, so the safety requirement is probably just that the token pointer is valid.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

library/std/src/sys/net/connection/uefi/tcp4.rs Outdated

unsafe fn cancel(&self, event: r_efi::efi::Event) -> io::Result<()> {
let protocol = self.protocol.as_ptr();
let mut completion_token = tcp4::CompletionToken { event, status: Status::SUCCESS };
Copy link
Contributor

Choose a reason for hiding this comment

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

From https://uefi.org/specs/UEFI/2.11/28_Network_Protocols_TCP_IP_and_Configuration.html#efi-ip4-protocol-cancel: "Pointer to a token that has been issued by EFI_IP4_PROTOCOL.Transmit() or EFI_IP4_PROTOCOL.Receive()". I read that to mean that the completion_token arg should be a pointer to the one created in the read and write methods above, not a new one.

(Also, if read literally it would seem like that means cancel can only be used for canceling receive/transmit, not connect. However, the edk2 implementation at least does not seem to have that restriction.)

Copy link
Contributor Author

@Ayush1325 Ayush1325 Jul 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Fixed. Now passing the completion token.

For connect, you are looking at wrong protocol. The correct one is here: https://uefi.org/specs/UEFI/2.11/28_Network_Protocols_TCP_IP_and_Configuration.html#efi-tcp4-protocol-cancel

@Ayush1325 Ayush1325 force-pushed the uefi-tcp4-timeout branch 2 times, most recently from 60cc9d7 to 298e2f7 Compare July 9, 2025 17:05
@Ayush1325
std: sys: net: uefi: tcp4: Add timeout support
d8273d3 
- Implement timeout support for read, write and connect.
- A software implementation using Instant.

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
@Ayush1325 Ayush1325 force-pushed the uefi-tcp4-timeout branch from 298e2f7 to d8273d3 Compare July 9, 2025 17:08
nicholasbishop
nicholasbishop approved these changes Jul 9, 2025
View reviewed changes
Copy link
Contributor

@nicholasbishop nicholasbishop left a comment

Choose a reason for hiding this comment

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

UEFI lgtm

@Ayush1325
Copy link
Contributor Author

ping @tgross35

@tgross35
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 10, 2025

📌 Commit d8273d3 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 10, 2025
tgross35 added a commit to tgross35/rust that referenced this pull request Jul 10, 2025
@tgross35
Rollup merge of rust-lang#143568 - Ayush1325:uefi-tcp4-timeout, r=tgr…
c89701c 
…oss35

std: sys: net: uefi: tcp4: Add timeout support

- Implement timeout support for read, write and connect.
- A software implementation using Instant.
@tgross35 tgross35 mentioned this pull request Jul 10, 2025
Closed
bors added a commit that referenced this pull request Jul 11, 2025
@bors
Auto merge of #143760 - tgross35:rollup-hyioynq, r=tgross35
5752c21 
Rollup of 7 pull requests

Successful merges:

 - #142391 (rust: library: Add `setsid` method to `CommandExt` trait)
 - #143301 (`tests/ui`: A New Order [26/N])
 - #143302 (`tests/ui`: A New Order [27/N])
 - #143303 (`tests/ui`: A New Order [28/28] FINAL PART)
 - #143568 (std: sys: net: uefi: tcp4: Add timeout support)
 - #143708 (fix: Include frontmatter in -Zunpretty output )
 - #143718 (Make UB transmutes really UB in LLVM)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Jul 11, 2025
@bors
Auto merge of #143760 - tgross35:rollup-hyioynq, r=tgross35
55cd449 
Rollup of 7 pull requests

Successful merges:

 - #142391 (rust: library: Add `setsid` method to `CommandExt` trait)
 - #143301 (`tests/ui`: A New Order [26/N])
 - #143302 (`tests/ui`: A New Order [27/N])
 - #143303 (`tests/ui`: A New Order [28/28] FINAL PART)
 - #143568 (std: sys: net: uefi: tcp4: Add timeout support)
 - #143708 (fix: Include frontmatter in -Zunpretty output )
 - #143718 (Make UB transmutes really UB in LLVM)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr matthiaskrgr mentioned this pull request Jul 11, 2025
Merged
bors added a commit that referenced this pull request Jul 11, 2025
@bors
Auto merge of #143766 - matthiaskrgr:rollup-0x7t69s, r=matthiaskrgr
256cb72 
Rollup of 8 pull requests

Successful merges:

 - #142391 (rust: library: Add `setsid` method to `CommandExt` trait)
 - #143302 (`tests/ui`: A New Order [27/N])
 - #143303 (`tests/ui`: A New Order [28/28] FINAL PART)
 - #143568 (std: sys: net: uefi: tcp4: Add timeout support)
 - #143611 (Mention more APIs in `ParseIntError` docs)
 - #143661 (chore: Improve how the other suggestions message gets rendered)
 - #143708 (fix: Include frontmatter in -Zunpretty output )
 - #143718 (Make UB transmutes really UB in LLVM)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Jul 11, 2025
@bors
Auto merge of #143766 - matthiaskrgr:rollup-0x7t69s, r=matthiaskrgr
6c42131 
Rollup of 8 pull requests

Successful merges:

 - #142391 (rust: library: Add `setsid` method to `CommandExt` trait)
 - #143302 (`tests/ui`: A New Order [27/N])
 - #143303 (`tests/ui`: A New Order [28/28] FINAL PART)
 - #143568 (std: sys: net: uefi: tcp4: Add timeout support)
 - #143611 (Mention more APIs in `ParseIntError` docs)
 - #143661 (chore: Improve how the other suggestions message gets rendered)
 - #143708 (fix: Include frontmatter in -Zunpretty output )
 - #143718 (Make UB transmutes really UB in LLVM)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Jul 11, 2025
@bors
Auto merge of #143766 - matthiaskrgr:rollup-0x7t69s, r=<try>
ae2eccf 
Rollup of 8 pull requests

Successful merges:

 - #142391 (rust: library: Add `setsid` method to `CommandExt` trait)
 - #143302 (`tests/ui`: A New Order [27/N])
 - #143303 (`tests/ui`: A New Order [28/28] FINAL PART)
 - #143568 (std: sys: net: uefi: tcp4: Add timeout support)
 - #143611 (Mention more APIs in `ParseIntError` docs)
 - #143661 (chore: Improve how the other suggestions message gets rendered)
 - #143708 (fix: Include frontmatter in -Zunpretty output )
 - #143718 (Make UB transmutes really UB in LLVM)

r? `@ghost`
`@rustbot` modify labels: rollup

try-job: i686-gnu-nopt-1
try-job: test-various
bors added a commit that referenced this pull request Jul 11, 2025
@bors
Auto merge of #143766 - matthiaskrgr:rollup-0x7t69s, r=matthiaskrgr
79464cc 
Rollup of 8 pull requests

Successful merges:

 - #142391 (rust: library: Add `setsid` method to `CommandExt` trait)
 - #143302 (`tests/ui`: A New Order [27/N])
 - #143303 (`tests/ui`: A New Order [28/28] FINAL PART)
 - #143568 (std: sys: net: uefi: tcp4: Add timeout support)
 - #143611 (Mention more APIs in `ParseIntError` docs)
 - #143661 (chore: Improve how the other suggestions message gets rendered)
 - #143708 (fix: Include frontmatter in -Zunpretty output )
 - #143718 (Make UB transmutes really UB in LLVM)

r? `@ghost`
`@rustbot` modify labels: rollup

try-job: i686-gnu-nopt-1
try-job: test-various
bors added a commit that referenced this pull request Jul 12, 2025
@bors
Auto merge of #143766 - matthiaskrgr:rollup-0x7t69s, r=matthiaskrgr
2f9c9ce 
Rollup of 8 pull requests

Successful merges:

 - #142391 (rust: library: Add `setsid` method to `CommandExt` trait)
 - #143302 (`tests/ui`: A New Order [27/N])
 - #143303 (`tests/ui`: A New Order [28/28] FINAL PART)
 - #143568 (std: sys: net: uefi: tcp4: Add timeout support)
 - #143611 (Mention more APIs in `ParseIntError` docs)
 - #143661 (chore: Improve how the other suggestions message gets rendered)
 - #143708 (fix: Include frontmatter in -Zunpretty output )
 - #143718 (Make UB transmutes really UB in LLVM)

r? `@ghost`
`@rustbot` modify labels: rollup

try-job: i686-gnu-nopt-1
try-job: test-various
@bors bors merged commit 5cbf7cf into rust-lang:master Jul 12, 2025
11 checks passed
@rustbot rustbot added this to the 1.90.0 milestone Jul 12, 2025
rust-timer added a commit that referenced this pull request Jul 12, 2025
@rust-timer
Unrolled build for #143568
6e98b12 
Rollup merge of #143568 - Ayush1325:uefi-tcp4-timeout, r=tgross35

std: sys: net: uefi: tcp4: Add timeout support

- Implement timeout support for read, write and connect.
- A software implementation using Instant.
@Ayush1325 Ayush1325 deleted the uefi-tcp4-timeout branch July 12, 2025 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicholasbishop nicholasbishop nicholasbishop approved these changes

@tgross35 tgross35 tgross35 approved these changes

Assignees

@tgross35 tgross35

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
Milestone
1.90.0
Development

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