Changes to flow allocator #27
Open
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
Also made signals part of FAIBase so they can be reused by subclasses instead of having to register them again every time. This commit is part of an ongoing effort to improve the code quality of RINASim.
Attributes being PascalCase is not done everywhere, and just looks ugly when PascalCase should be used for types and classes. Changed to use camelCase instead, which is used everywhere else in the code for attributes and variables. Also removed the use of some signals, as they were used wrong. Direct method calls make for more readable code (not jumping between two or more files to find the thread of execution), and probably saves some execution speed.
Also moved listening to the FA module, to mitigate memory leaks (from pointless dedicated listener modules) and improve readability code. FAListeners is gone.
Replaced a bunch of older style iterator loops with the "new" for each loop. Also moved from info() to str(), which is preferred in newer versions of OMNeT++.
Only checking if this IPCP is enrolled in any DIF will cause no additional management flows to be allocated. This will cause problems in any case where several neighbouring IPCPs are reachable over any number of underlying DIFs, and the simulation will crash. As of now, there are zero examples that make use of several flows being allocated over different "links", and as such this error has likely not been encountered. Connecting to several neighbours over the same segment is a relevant use-case with the Ethernet shim DIF, so this needs to be fixed to allow simulation with switches. A side-effect is that some simulation scenarios will have more events due to management flow allocation being required in every case.
This commit allows the Flow Allocator to go through the same N-1 management allocation procedure on forwarding of flows. This is necessary because forwarding was handled differently and requires additional state to be recorded from the N-1 IPCP (like what type of flow is being requested). This does not work well for the Ethernet shim IPCP, which cannot convey any more state than what Ethernet manages (basically end-point addresses and DIF name).
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.
There are a lot of cosmetic changes in this PR, but also some things that are necessary for the Ethernet shim DIF. The commits in question are 676c98e and e7f20eb.
Read the corresponding commit messages (press the ellipsis next to the commits) to see an explanation. My thesis contains a more complete explanation of why the connection status check is required.
The changes marked "refactor" do not make execution flow changes to the code, only rearranges code and removes some signals that obfuscate the meaning of the code. This is just a small dent, as some of the uses of signals have you go through 3-4 files before you end up in the function the signal is supposed to trigger, and this was a pain when first trying to understand the codebase. I imagine it will also be problematic for the next person who implements a shim DIF of another type, so I changed a little bit to make the code slightly more readable.
This is just a patch added (
git add --patch) version of parts of this repo: https://github.com/karlhto/RINA-thesisCheck it out for proper commit dates. :)