Add List option to RouteSubscribeWithOptions, AddrSubscribeWithOptions, and LinkSubscribeWithOptions. #318
Conversation
This file contains 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
aboch
reviewed
Jan 20, 2018
link_linux.go
Outdated
| @@ -1514,6 +1515,16 @@ func linkSubscribeAt(newNs, curNs netns.NsHandle, ch chan<- LinkUpdate, done <-c | |||
| s.Close() | |||
| }() | |||
| } | |||
| if list { | |||
There was a problem hiding this comment.
Ideally, shouldn't the dump request be performed after spawning the receive go routine ?
aboch
reviewed
Jan 20, 2018
link_linux.go
Outdated
| } | ||
|
|
||
| // LinkSubscribeOptions contains a set of options to use with | ||
| // LinkSubscribeWithOptions. | ||
| type LinkSubscribeOptions struct { | ||
| Namespace *netns.NsHandle | ||
| ErrorCallback func(error) | ||
| List bool |
There was a problem hiding this comment.
Should we call this ListExisting ? I think it would be more self-explanatory.
|
Please fix the gofmt issues, so we can get the UT running. |
|
Also please add a test function for this. |
|
Thanks. Please squash the commits in one. |
route/addr/link tables as part of RouteSubscribeWithOptions, AddrSubscribeWithOptions, and LinkSubscribeWithOptions.
|
LGTM |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
If an application wants to track the set of kernel routes (or addresses or links) it needs to both determine the initial content and subscribe to changes. However, there are inherent race conditions in using RouteList + RouteSubscribe. If RouteList is invoked before RouteSubscribe then obviously some changes can be lost in between. But more subtly, in the other other the application can see some adds and deletes from the subscribe before the RouteList results, which makes it hard to track the actual content of the kernel.
This patch adds a List boolean option which makes RouteSubscribeWithOptions (and same for addr and links) request the list and the subscription and feed the initial table as "adds" to the application, which makes it a lot easier to track the kernel table.