Skip to content

Ping using x/net/icmp#2839

Merged
rminnich merged 2 commits into
u-root:mainfrom
binjip978:ping-2
Jan 17, 2024
Merged

Ping using x/net/icmp#2839
rminnich merged 2 commits into
u-root:mainfrom
binjip978:ping-2

Conversation

@binjip978

@binjip978 binjip978 commented Jan 4, 2024

Copy link
Copy Markdown
Contributor

No description provided.

rminnich
rminnich reviewed Jan 6, 2024
View reviewed changes
Comment thread cmds/core/ping/ping.go Outdated
@rminnich

rminnich commented Jan 6, 2024

Copy link
Copy Markdown
Member

This is great. When I wrote the first version of this, I am not sure those packages existed ... this is a very nice improvement.

@codecov

codecov Bot commented Jan 7, 2024 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Attention: 27 lines in your changes are missing coverage. Please review.

Comparison is base (4478dd7) 75.70% compared to head (1276bad) 75.67%.

Files Patch % Lines
cmds/core/ping/ping.go 64.00% 27 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2839      +/-   ##
==========================================
- Coverage   75.70%   75.67%   -0.03%     
==========================================
  Files         426      425       -1     
  Lines       42847    42836      -11     
==========================================
- Hits        32438    32418      -20     
- Misses      10409    10418       +9
Flag Coverage Δ
.-amd64 67.87% <ø> (ø)
cmds/...-amd64 71.45% <56.00%> (-0.25%) ⬇️
integration/generic-tests/...-amd64 0.00% <ø> (ø)
integration/generic-tests/...-arm 0.00% <ø> (ø)
integration/generic-tests/...-arm64 0.00% <ø> (ø)
integration/gotests/...-amd64 73.98% <64.00%> (-0.03%) ⬇️
integration/gotests/...-arm 75.20% <64.00%> (-0.03%) ⬇️
integration/gotests/...-arm64 74.93% <64.00%> (-0.03%) ⬇️
pkg/...-amd64 76.11% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@binjip978 binjip978 force-pushed the ping-2 branch 6 times, most recently from 3940553 to 64c5365 Compare January 9, 2024 17:56
@binjip978 binjip978 marked this pull request as ready for review January 9, 2024 18:15
@binjip978 binjip978 changed the title WIP ping imcp Ping using x/net/icmp Jan 9, 2024
@binjip978

binjip978 commented Jan 9, 2024

Copy link
Copy Markdown
Contributor Author

Localhost will require more work, but current version has problem also.

@binjip978 binjip978 added the Awaiting reviewer Waiting for a reviewer. label Jan 9, 2024
@binjip978
WIP ping icmp
cdb739e 
Signed-off-by: Siarhiej Siemianczuk <pdp.eleven11@gmail.com>
@rminnich

rminnich commented Jan 14, 2024

Copy link
Copy Markdown
Member

So is this ready or do you want to write the vmtest too?

@rminnich rminnich added Awaiting author Waiting for new changes or feedback for author. and removed Awaiting reviewer Waiting for a reviewer. labels Jan 14, 2024
@binjip978

binjip978 commented Jan 15, 2024

Copy link
Copy Markdown
Contributor Author

So is this ready or do you want to write the vmtest too?

Yeah it's ready and I added some vmtests, there is a known problem like pinging localhost with -c>1, but original version has the same problem. I have solution for IPv4, but not yet for IPv6.

@binjip978 binjip978 added Awaiting reviewer Waiting for a reviewer. and removed Awaiting author Waiting for new changes or feedback for author. labels Jan 15, 2024
@rminnich rminnich added automerge Applying this label auto-merges the PR when ready Awaiting author Waiting for new changes or feedback for author. and removed Awaiting reviewer Waiting for a reviewer. labels Jan 16, 2024
@rminnich rminnich merged commit 25faeb4 into u-root:main Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hugelgupf hugelgupf hugelgupf left review comments

@rminnich rminnich rminnich approved these changes

Assignees

No one assigned

Labels

automerge Applying this label auto-merges the PR when ready Awaiting author Waiting for new changes or feedback for author.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@binjip978 @rminnich @hugelgupf