This repository was archived by the owner on May 12, 2021. It is now read-only.
protocols: client: Add timeout for hybrid vsock handshake#734
Merged
devimc merged 1 commit intokata-containers:masterfrom Feb 20, 2020
jcvenegas:fix-372
Merged
protocols: client: Add timeout for hybrid vsock handshake#734devimc merged 1 commit intokata-containers:masterfrom jcvenegas:fix-372
devimc merged 1 commit intokata-containers:masterfrom
jcvenegas:fix-372
Conversation
Codecov Report
@@ Coverage Diff @@
## master #734 +/- ##
==========================================
- Coverage 60.63% 60.45% -0.19%
==========================================
Files 17 17
Lines 2558 2569 +11
==========================================
+ Hits 1551 1553 +2
- Misses 858 868 +10
+ Partials 149 148 -1 |
Member
Author
|
kata-containers/runtime#2454 test of code in runtime |
sboeuf
reviewed
Feb 14, 2020
chavafg
reviewed
Feb 14, 2020
Member
Author
|
/test |
Member
Author
|
@sboeuf @chavafg @amshinde @devimc @jodh-intel @grahamwhaley this PR is ready folks, please take a look |
Contributor
|
restarted CI jobs that failed. |
sboeuf
approved these changes
Feb 18, 2020
devimc
reviewed
Feb 18, 2020
devimc
left a comment
There was a problem hiding this comment.
thanks @jcvenegas , I left some questions
protocols/client/client.go
Outdated
| agentClientLog.WithField("response", response).Debug("HybridVsock trivial handshake") | ||
|
|
||
| if strings.Contains(response, "OK") { | ||
| successfulConnectionChan <- true |
There was a problem hiding this comment.
can we reuse the errChan ? errChan <- nil
protocols/client/client.go
Outdated
| case <-successfulConnectionChan: | ||
| return conn, nil | ||
| case err = <-errChan: | ||
| conn.Close() |
There was a problem hiding this comment.
if errChan can be reused, here we need to check if err != nil , right?
When the client tries to connect sometimes a race condition could happen when the between bind and listen calls in the agent vsock side. This will block the hypervisor wanting for a response and as consequence the agent client where it checks for an OK response. This case needs to be handled by the guest kernel, see https://lore.kernel.org/netdev/668b0eda8823564cd604b1663dc53fbaece0cd4e.camel@intel.com/ As an extra protection make the agent client timeout if no OK response is given. The response should be quick so is OK to wait a few seconds and then timeout. This also allow to return an error from the dialler function so retry does not fallback on grpc retry making retries faster. Fixes: #372 Signed-off-by: Jose Carlos Venegas Munoz <jose.carlos.venegas.munoz@intel.com>
Member
Author
|
/test-ubuntu |
Member
Author
|
kata-containers/runtime#2454 CI passing with latest changes, @devimc PR updated |
devimc
approved these changes
Feb 20, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When the client tries to connect sometimes a race condition
could happen when the between bind and listen calls in the agent
vsock side.
This will block the hypervisor wanting for a response and
as consequence the agent client where it checks for an OK
response.
This case needs to be handled by the guest kernel, see
https://lore.kernel.org/netdev/668b0eda8823564cd604b1663dc53fbaece0cd4e.camel@intel.com/
As an extra protection make the agent client timeout if no OK response
is given. The response should be quick so is OK to wait a few seconds
and then timeout.
This also allow to return an error from the dialler function so retry
does not fallback on grpc retry making retries faster.
Fixes: #372