-
Notifications
You must be signed in to change notification settings - Fork 114
client.go: HybridVSockDialer: Change Read EOT to recv peek #696
Conversation
418285f
to
1286084
Compare
Codecov Report
@@ Coverage Diff @@
## master #696 +/- ##
==========================================
- Coverage 60.57% 60.51% -0.07%
==========================================
Files 17 17
Lines 2534 2540 +6
==========================================
+ Hits 1535 1537 +2
- Misses 850 855 +5
+ Partials 149 148 -1 |
/test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch @teawater !!
lgtm
protocols/client/client.go
Outdated
// Just close the connection, gRPC will dial again | ||
// without errors | ||
conn.Close() | ||
// Receive the package from the connection without remove this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: s/package/packet/.
Just an idea but you could simplify this comment to something like:
Receive the packet from the connection without removing it from the receive queue (MSG_PEEK), ensuring the connection is usable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jodh-intel Thanks! Pushed a new version according to your comments.
Rust agent(grpc-rs) cannot work with firecacker because the first package will be dropped or truncated by HybridVSockDialer read EOT code. This issue was reported in kata-containers/kata-containers#79 Golang agent(grpc-golang) is also affected by this issue. But it is first package is 9 bytes FRAME_SETTINGS. Drop it will not affect subsequent communications. This commit change to use recv peek that will do same check with read EOT but not drop any package. Fixes: kata-containers#695 Signed-off-by: Hui Zhu <[email protected]>
1286084
to
f8e4ce8
Compare
/test |
Thanks again @teawater. Ping @kata-containers/agent. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch @teawater !
restarting firecracker CI |
Rust agent(grpc-rs) cannot work with firecacker because the first package
will be dropped or truncated by HybridVSockDialer read EOT code.
This issue was reported in
kata-containers/kata-containers#79
Golang agent(grpc-golang) is also affected by this issue. But it is first
package is 9 bytes FRAME_SETTINGS. Drop it will not affect subsequent
communications.
This commit change to use recv peek that will do same check with read
EOT but not drop any package.
Fixes: #695
Signed-off-by: Hui Zhu [email protected]