Fix selection of modes based on instance config #963
Merged
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
added 2 commits
September 22, 2017 16:16
When --on-a-stick command is used with a single instance defined it should migrate the instance to work in a stick (replace the device with the one provided in the flag and remove the device in `external-device`). It just checks that it works by not crashing and says it's going to do it in the output.
Previously you could either configure the lwaftr with --v4/--v6 and --on-a-stick and it'd then migrate the configuration provided and then on every instance added subsequently it'd only use the mode selected at the start despite what the configuration says. If no flag was specified it'd only use --v4/--v6 for everything including new configurations added without two devices. It will now select the correct method based on the configuration except in the case of --vertio which will cause it to always use the virtual driver. Configurations can now contain a mix between bump-in-the-wire and on-a-stick configurations and it'll choose it appropriately. New configurations added by the `snabb config` tools will also choose it based on the instance configuration for each instance.
wingo
approved these changes
Sep 25, 2017
src/program/lwaftr/run/run.lua
Outdated
| local name, instance = next(lwconfig.softwire_config.instance) | ||
| local queue = instance.queue.values[1] | ||
| if queue.external_interface.device then | ||
| return setup.load_phy(graph, lwconfig, 'inetNick', 'b4sideNic', |
src/program/lwaftr/run/run.lua
Outdated
| else | ||
| local use_splitter = requires_splitter(opts, lwconfig) | ||
| local options = { | ||
| v4_nic_name = 'inetNic', v6_nic_name = 'b4sideNick', |
| -- If instance has external-interface.device configure as bump-in-the-wire | ||
| -- otherwise configure it in on-a-stick mode. | ||
| local name, instance = next(lwconfig.softwire_config.instance) | ||
| local queue = instance.queue.values[1] |
There was a problem hiding this comment.
In some future we might want to configure this in a per-instance way, but then again maybe we shouldn't bother :)
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.
This fixes #957.
As #957 states there is a migration when
--on-a-stickor--v4/--v6is specified however it doesn't allow you to run in multiprocessing mode. The problem actually ended up being deeper than that. When you did provide one of these flags (or lack there of) it'd pick the appropriate mode based on the initial configuration and command line arguments and use that for everything. That means when asnabb config addcommand was issued, it will use whatever was appropriate when it initially started irregardless of what the configuration says.The fix changes how the
setup_fnwe pass tosetup.reconfigurationand thus the leader works. Instead of giving it for examplesetup.load_phyorsetup.load_on_a_stick, etc. it provides a single setup_fn which takes in the graph and configuration (with a single instance) and decides based on that which mode to use.I've tested on snabb1 that you can now provide an on-a-stick configuration and it'll configure itself in an on-a-stick fashion (I've also tested for bump-in-the-wire). I'd have liked to improve the unit tests around this to make them a bit more robust (especially with adding new instances) - to do that though I think i'd need something like #962.