listener: remove the peek from the listener filters#20728
listener: remove the peek from the listener filters#20728mattklein123 merged 18 commits intoenvoyproxy:mainfrom
Conversation
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
|
@mattklein123 @ggreenway @kyessenov I fixed the HTTP inspect issue, also comment inline for what was wrong in the previous. @kyessenov let me know if there is a way to trigger the istio CI to test on under-review PR. It is still scare me for this huge PR break something. Thanks for helping diagnosing the issue and add integration again! |
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
|
/wait |
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
|
/wait |
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
|
/wait adding more integration test for multiple filters. |
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
|
Summary what changed here:
@mattklein123 @kyessenov This is ready for review. Sorry for more change here, that probably will spend your more review bandwidth. Thanks in advance! |
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
|
We are going to cut a release this week. Given the risk of this PR let's hold on re-applying until right after we cut the release, so we can merge early next week. Thank you! |
Got it, thanks! |
|
Can you merge main to move the release note to the new current.rst and then we can merge? Thank you. /wait |
kyessenov
left a comment
There was a problem hiding this comment.
Thanks for adding multiple inspector tests.
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
thanks! done. |
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
|
/retest |
|
Retrying Azure Pipelines: |
|
/retest |
|
Retrying Azure Pipelines: |
mattklein123
left a comment
There was a problem hiding this comment.
Thanks for working through the fix and getting it back in!
Signed-off-by: He Jie Xu <hejie.xu@intel.com> Signed-off-by: Andre Vehreschild <vehre@x41-dsec.de>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Commit Message: listener: remove the peek from the listener filters
Additional Description:
To avoid the listener filters to peek the data from the socket directly, allow the ActiveTcpSocket to peek the data
into the buffer. The ActiveTcpSocket will get the max data expected from the filters through the
filter interface size_t maxReadBytes(). Then ActiveTcpSocket will listen on the file event on the socket.
And peek the data into the buffer. And calling the listener filter callback onData(Buffer::Instance&).
If the filter doesn't need any data from the connection, it should return 0 on the interface 'maxReadBytes()'.
The ActiveTcpSocket won't listen on the file event if there is no filter expecting any data.
The buffer will be shared across the listener filter. no duplicated peek need anymore.
The buffer is implemented by ListenerFilterBufferImpl. It will store the data peeked from the socket, and provide
the const pointer to the listener filter to access the data. It also provides the interface for listener filter to drain the data,
and the data will be drained on the socket in the end.
Risk Level: high
Testing: unit tests and integration tests
Docs Changes: add document for the new stats
Release Notes: add release note for new stats
Fixes: #17229
Signed-off-by: He Jie Xu hejie.xu@intel.com