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.

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

dhcpv4: Replace the DiscoveryTimeout timer when discovery done #47

Merged
merged 1 commit into from
Jan 7, 2025

Conversation

cathay4t
Copy link
Contributor

@cathay4t cathay4t commented Jan 6, 2025

We should replace DiscoveryTimeout timer with RequestTimeout when
the discovery is done and entering request phase.

Manually tested by checking debug logs of
cargo run --example mozim_dhcpv4_async.

DHCPv6 part does not have this problem as it has only single retry timer
DhcpV6Event::TransmitWait.

Copy link

@xhebox xhebox left a comment

Choose a reason for hiding this comment

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

Rest LGTM

DhcpV4Event::RequestTimeout,
)?;
self.retry_count = 0;
self.event_pool.del_timer(DhcpV4Event::DiscoveryTimeout)?;
Copy link

Choose a reason for hiding this comment

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

I believe self.lease need to be set too. Since process_request_timeout won't send packets without lease.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch! Fixed.

We should replace `DiscoveryTimeout` timer with `RequestTimeout` when
the discovery is done and entering request phase.

Manually tested by checking debug logs of
`cargo run --example mozim_dhcpv4_async`.

DHCPv6 part does not have this problem as it has only single retry timer
`DhcpV6Event::TransmitWait`.

Signed-off-by: Gris Ge <[email protected]>
@cathay4t cathay4t merged commit e8dd9f7 into nispor:main Jan 7, 2025
6 checks passed
@cathay4t cathay4t deleted the fix_timer branch January 7, 2025 02:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

timer does not seem correct for dhcpv4?
2 participants