backport to 1.13: udp: properly handle truncated/dropped datagrams (#14122) #14198
Conversation
Signed-off-by: Matt Klein <mklein@lyft.com> Signed-off-by: Christoph Pakulski <christoph@tetrate.io> Co-authored-by: Matt Klein <mklein@lyft.com> Co-authored-by: Christoph Pakulski <christoph@tetrate.io> Signed-off-by: Christoph Pakulski <christoph@tetrate.io>
Signed-off-by: Christoph Pakulski <christoph@tetrate.io>
|
/retest |
|
Retrying Azure Pipelines: |
| return Api::SysCallSizeResult{5u, 0}; | ||
| })); | ||
| Network::IoHandle::RecvMsgOutput output(1, nullptr); | ||
| EXPECT_CALL(os_sys_calls_, recvmsg(fd, _, messageTruncatedOption())) |
There was a problem hiding this comment.
It's curious that you had to introduce messageTruncatedOption in the backport. I wonder what's going on in the master branch.
There was a problem hiding this comment.
I was getting macos test failures and could not access log artifact. So, after scrutinizing changes introduced in this PR, I noticed that the test expected recvmsg with MSG_TRUNC, which on macos is not used. It fixed the macos test issue on this 1.13 branch. On master it clearly expects recvmsg to be called with MSG_TRUNC. But I am not sure if on master CI tests execute all unit tests for macos build or only integration ones. Anyways, I believe that messageTruncatedOption should be put somewhere in a header file and used in both: source and test without referring to MSG_TRUNC directly. I can open PR for that if you agree.
There was a problem hiding this comment.
cc @ggreenway
Yeah, I think that some tests being excluded from CI at HEAD is the best way to explain the difference in behavior in macos tests.
There was a problem hiding this comment.
Most tests aren't run in mac CI.
In fact, integration tests don't even build right now on mac.
Signed-off-by: Christoph Pakulski <christoph@tetrate.io>
|
/retest |
|
Retrying Azure Pipelines: |
Commit Message:
properly handle truncated/dropped datagrams
Additional Description:
Risk Level: Med
Testing:Unit tests
Docs Changes: No
Release Notes: Yes
Fixes #14113