Skip to content

修复ping时统计异常的问题 #385

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

Open
Kelephant wants to merge 1 commit into
base: master
Choose a base branch
from
Open

修复ping时统计异常的问题 #385

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 25 additions & 14 deletions protocol/icmp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ int ping(const char* host, int cnt) {
static uint16_t seq = 0;
uint16_t pid16 = (uint16_t)getpid();
char ip[64] = {0};
uint32_t start_tick, end_tick;
uint32_t start_tick, end_tick, cur_ping_start_tick;
uint64_t start_hrtime, end_hrtime;
int timeout = 0;
int sendbytes = 64;
Expand Down Expand Up @@ -64,7 +64,9 @@ int ping(const char* host, int cnt) {
icmp_req->icmp_data[i] = i;
}
start_tick = gettick_ms();

while (cnt-- > 0) {
cur_ping_start_tick = gettick_ms();
// NOTE: checksum
icmp_req->icmp_seq = ++seq;
icmp_req->icmp_cksum = 0;
Expand All @@ -78,29 +80,38 @@ int ping(const char* host, int cnt) {
}
++send_cnt;
addrlen = sizeof(peeraddr);
_read_again:
if(gettick_ms() - cur_ping_start_tick >= PING_TIMEOUT) {
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

这里的代码风格与本文件其他 if 语句不一致：使用了 if( 而不是 if (。建议保持一致的空格风格。

Suggested change
if(gettick_ms() - cur_ping_start_tick >= PING_TIMEOUT) {
if (gettick_ms() - cur_ping_start_tick >= PING_TIMEOUT) {

Copilot uses AI. Check for mistakes.
// recv timeout, send ping again.
continue;
}

int nrecv = recvfrom(sockfd, recvbuf, sizeof(recvbuf), 0, &peeraddr.sa, &addrlen);
if (nrecv < 0) {
Comment on lines +83 to 90
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

当前 _read_again 重试逻辑里每次都会再次调用带 SO_RCVTIMEO=PING_TIMEOUT 的 recvfrom()。如果在接近 1s 末尾收到一个无效包（例如 900ms），会马上进入下一次 recvfrom()，它又可能再阻塞 1s，导致单次 ping 实际等待时间超过 PING_TIMEOUT（最多接近 2*PING_TIMEOUT），与“1s 内收不到有效 echo 就发下一个”的目标不符。建议按截止时间计算剩余超时（remaining = PING_TIMEOUT - elapsed）并把 SO_RCVTIMEO 设置为 remaining 后再 recvfrom，或改用 poll/select 按 deadline 等待。

Copilot uses AI. Check for mistakes.
perror("recvfrom");
continue;
goto _read_again;
Comment on lines 89 to +92
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

recvfrom() 失败后无条件 perror 并 goto _read_again，会在非超时/中断类错误（例如 EBADF、EINVAL 等）时循环打印并拖到 PING_TIMEOUT 才进入下一轮，掩盖真实错误。建议仅在 errno 为 EINTR / EAGAIN(EWOULDBLOCK) / ETIMEDOUT 时重试；其他错误应直接退出循环并走 error 路径或返回错误码。

Copilot uses AI. Check for mistakes.
}
++recv_cnt;

end_hrtime = gethrtime_us();
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

end_hrtime 在校验前（line 95）和校验通过后（line 112）被赋值两次，但只有后一次用于计算 rtt。建议删除前一次赋值，避免冗余并减少误解。

Suggested change
end_hrtime = gethrtime_us();

Copilot uses AI. Check for mistakes.
// check valid
bool valid = false;
int iphdr_len = ipheader->ihl * 4;
int icmp_len = nrecv - iphdr_len;
if (icmp_len == sendbytes) {
icmp_res = (icmp_t*)(recvbuf + ipheader->ihl*4);
if (icmp_res->icmp_type == ICMP_ECHOREPLY &&
icmp_res->icmp_id == pid16 &&
icmp_res->icmp_seq == seq) {
valid = true;
}
if (icmp_len != sendbytes) {
// not our ping
goto _read_again;
}
if (valid == false) {
printd("recv invalid icmp packet!\n");
continue;

icmp_res = (icmp_t*)(recvbuf + ipheader->ihl*4);
if (icmp_res->icmp_type != ICMP_ECHOREPLY ||
icmp_res->icmp_id != pid16 ||
icmp_res->icmp_seq != seq) {
// not our ping
goto _read_again;
}

end_hrtime = gethrtime_us();
++recv_cnt;

rtt = (end_hrtime-start_hrtime) / 1000.0f;
min_rtt = MIN(rtt, min_rtt);
max_rtt = MAX(rtt, max_rtt);
Expand Down
Loading