Skip to content

Conversation

@dpgeorge
Copy link
Member

@dpgeorge dpgeorge commented May 7, 2025

Summary

The bare-metal lwIP socket interface is currently quite limited when used for UDP streams, because it only allows one outstanding incoming UDP packet. If one UDP packet is waiting to be socket.recv'd and another one comes along, then the second one is simply dropped.

This PR implements a queue for incoming UDP and raw packets. The queue depth is fixed at compile time, and is currently 4.

This allows better use of UDP connections, eg more efficient. It also makes DTLS work better which sometimes has a queue of UDP packets (eg during the connection phase).

Testing

A new network multi-test is added which sends a burst of UDP packets from the client, and the server doesn't recv them until they have all been sent.

Tested on PYBD-SF2 and RPI_PICO2_W. The new test passes (it fails without the UDP queue added here).

Also tested on unix and ESP32_GENERIC, both of which pass (they already have a UDP queue).

Trade-offs and Alternatives

  • Some memory is needed for the queue, and is allocated on the MP heap. That memory holds a ring buffer of pbufs. Unfortunately pbufs don't support "packet queue" mode in lwIP yet, or maybe we could just link-link the pubfs together (also need to store the peer address and port, which probably requires additional memory anyway).
  • The queue depth is fixed at compile time. That's probably adequate.

@dpgeorge dpgeorge added the extmod Relates to extmod/ directory in source label May 7, 2025
@codecov
Copy link

codecov bot commented May 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.54%. Comparing base (61eedbb) to head (a05766f).
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #17261   +/-   ##
=======================================
  Coverage   98.54%   98.54%           
=======================================
  Files         169      169           
  Lines       21896    21896           
=======================================
  Hits        21578    21578           
  Misses        318      318           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link

github-actions bot commented May 7, 2025

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:  +144 +0.016% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32

@dpgeorge dpgeorge force-pushed the extmod-modlwip-add-udp-queue branch from 23c2be6 to 06826a5 Compare May 7, 2025 03:48
@dpgeorge dpgeorge requested a review from projectgus May 7, 2025 03:59
Copy link
Contributor

@projectgus projectgus left a comment

Choose a reason for hiding this comment

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

Implementation and test look solid to me. I have a couple of minor comment suggestions for future reference, but that's all.

extmod/modlwip.c Outdated
#define MOD_NETWORK_SOCK_DGRAM (2)
#define MOD_NETWORK_SOCK_RAW (3)

// Total queue length for UDP/raw connections.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Total queue length for UDP/raw connections.
// Total queue length for buffered UDP/raw incoming packets.

(That's right, isn't it?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that's right. Comment updated.

extmod/modlwip.c Outdated
struct pbuf *pbuf;
} tcp;

// UDP and raw sockets have a queue of incoming pbuf's.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// UDP and raw sockets have a queue of incoming pbuf's.
// UDP and raw sockets have a queue of incoming pbuf's, implemented as a ringbuffer

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated.

extmod/modlwip.c Outdated

// UDP and raw sockets have a queue of incoming pbuf's.
struct {
uint8_t iget;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
uint8_t iget;
uint8_t iget; // Ringbuffer read index

(and same for iput)

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated for both lines.

dpgeorge added 2 commits May 12, 2025 14:17
The bare-metal lwIP socket interface is currently quite limited when used
for UDP streams, because it only allows one outstanding incoming UDP
packet.  If one UDP packet is waiting to be socket.recv'd and another one
comes along, then the second one is simply dropped.

This commit implements a queue for incoming UDP and raw packets.  The queue
depth is fixed at compile time, and is currently 4.

This allows better use of UDP connections, eg more efficient.  It also
makes DTLS work better which sometimes has a queue of UDP packets (eg
during the connection phase).

Signed-off-by: Damien George <damien@micropython.org>
This commit adds a new network multi-test which sends a burst of UDP
packets from the client, and the server doesn't recv them until they have
all been sent.

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge dpgeorge force-pushed the extmod-modlwip-add-udp-queue branch from 06826a5 to a05766f Compare May 12, 2025 04:20
@dpgeorge dpgeorge merged commit a05766f into micropython:master May 12, 2025
64 checks passed
@dpgeorge dpgeorge deleted the extmod-modlwip-add-udp-queue branch May 12, 2025 05:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

extmod Relates to extmod/ directory in source

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants