Skip to content

Conversation

fishpan1209
Copy link
Contributor

@fishpan1209 fishpan1209 commented Oct 17, 2025

Commit Message: Extend UdpPacketWriterFactory interface for advanced writers
Additional Description:
This change modifies the UdpPacketWriterFactory interface to include two new parameters in its creation methods:

  1. dispatcher: to allow writer implementations to schedule file event.
  2. on_can_write_cb: callback that the writer implementation can invoke to signal it's ready to write more data.

These additions are necessary to support more complex writer implementations who requires interaction with the dispatcher and a mechanism to signal readiness to send.

Risk Level: low
Testing: unit tests
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

@repokitteh-read-only
Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #41604 was opened by fishpan1209.

see: more, trace.

Signed-off-by: Ting Pan <[email protected]>
@fishpan1209
Copy link
Contributor Author

/retest

@fishpan1209
Copy link
Contributor Author

/assign @RyanTheOptimist @wang178c
Hi Ryan & Haoyue, pls review the above pr, this change is required to add a PigeonWriter for Envoy. I modified the UdpPacketWriterFactory interface to take two more parameters, dispatcher, and on_can_write_cb which will be used to signal pigeon queue blocked and unblocked. thank you

@fishpan1209 fishpan1209 marked this pull request as ready for review October 20, 2025 14:35
Stats::Scope& scope) PURE;
virtual UdpPacketWriterPtr
createUdpPacketWriter(Network::IoHandle& io_handle, Stats::Scope& scope,
Envoy::Event::Dispatcher& dispatcher,
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add comments for the new parameters (dispacter is pretty obvious, but on_can_write_cb is less so)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added comments for new params and updated existing comments because the "socket" param is outdated

listener_config.udpListenerConfig()->packetWriterFactory().createUdpPacketWriter(
listen_socket_.ioHandle(), listener_config.listenerScope());
listen_socket_.ioHandle(), listener_config.listenerScope(), dispatcher,
std::move(on_can_write_cb));
Copy link
Contributor

Choose a reason for hiding this comment

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

I might have missed it, but do we have a unit test which ensures that this CB is sent correctly to the writer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, we don't have a unit test for the cb because this change is interface only, the concrete implementation using the cb will come in followup cl.

@fishpan1209
Copy link
Contributor Author

/retest

@fishpan1209
Copy link
Contributor Author

//test/integration:load_stats_integration_test failed again, this test is very flaky

@RyanTheOptimist RyanTheOptimist merged commit 0d68f81 into envoyproxy:main Oct 22, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants