Skip to content

core: introduce interface for setting preferred network#442

Merged
goaway merged 5 commits intomasterfrom
jm/reachability
Sep 24, 2019
Merged

core: introduce interface for setting preferred network#442
goaway merged 5 commits intomasterfrom
jm/reachability

Conversation

@goaway
Copy link
Copy Markdown
Contributor

@goaway goaway commented Sep 16, 2019

Description: This is to allow OS/platform reachability signals to be consumed by the Envoy library engine.

Co-authored-by: Jose Nino jnino@lyft.com
Signed-off-by: Mike Schore mike.schore@gmail.com

Signed-off-by: Mike Schore <mike.schore@gmail.com>
Signed-off-by: Mike Schore <mike.schore@gmail.com>
Jose Nino added 2 commits September 18, 2019 09:59
Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: Jose Nino <jnino@lyft.com>

/**
* Update the network interface to the preferred network for opening new streams.
* Note that this state is shared by all engines.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this permanent? Seems like ideally we'd be able to set this per-engine (especially since we don't expose a singleton on the platform layer).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The presumption here is that the same networks/interfaces are available to any thread in the process. As long as that's true, there's no real inherent need for the state to be replicated.

I could see per-instance state being slightly more testable, maybe.

rebello95
rebello95 previously approved these changes Sep 20, 2019
Copy link
Copy Markdown
Contributor

@rebello95 rebello95 left a comment

Choose a reason for hiding this comment

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

Thanks for discussing

Signed-off-by: Mike Schore <mike.schore@gmail.com>
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