smoltcp icon indicating copy to clipboard operation
smoltcp copied to clipboard

fix: exhaust egress socket state on `poll`

Open thomaseizinger opened this issue 7 months ago • 1 comments

At present, smoltcp only does a single pass of advancing the egress socket state. This means the layer above has to call poll in a loop to ensure sufficient progress has been made.

We already exhaustively progress the ingress state further up so doing the same thing for the egress state seems appropriate.

thomaseizinger avatar May 08 '25 02:05 thomaseizinger

Codecov Report

:x: Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review. :white_check_mark: Project coverage is 80.45%. Comparing base (17012eb) to head (671827b). :warning: Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/iface/interface/mod.rs 66.66% 1 Missing :warning:
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1059   +/-   ##
=======================================
  Coverage   80.45%   80.45%           
=======================================
  Files          81       81           
  Lines       24461    24461           
=======================================
  Hits        19681    19681           
  Misses       4780     4780           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar May 08 '25 02:05 codecov[bot]

@whitequark @Dirbaio I don't see any reason why not to do this.

thvdveld avatar Oct 07 '25 09:10 thvdveld

I thought @Dirbaio had a case where this caused an issue?

whitequark avatar Oct 07 '25 09:10 whitequark

I thought that as well and was digging in old issues and PRs, but I didn't find anything related to the egress. Most of the issues were with ingress resulting in a DoS.

thvdveld avatar Oct 07 '25 09:10 thvdveld

@thomaseizinger Could you re-run/fix the test please? We'll merge it after.

whitequark avatar Oct 09 '25 09:10 whitequark

@thomaseizinger Could you re-run/fix the test please? We'll merge it after.

I just accepted the new snapshot but I don't really know what it is testing.

thomaseizinger avatar Oct 13 '25 04:10 thomaseizinger

Thanks!

whitequark avatar Oct 13 '25 11:10 whitequark