Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AP Re-association hooks #232

Closed
emccrckn opened this issue Jun 13, 2019 · 10 comments
Closed

AP Re-association hooks #232

emccrckn opened this issue Jun 13, 2019 · 10 comments

Comments

@emccrckn
Copy link

Is there a way to have a callback within an experiment to know when a station has associated with a new AP?

The problem we are facing is we need to be able to call setIP() with a new IP address when the station changes over to another AP but currently it keeps the same IP. For the case in question the AP's are in different subnets.

@ramonfontes
Copy link
Member

ramonfontes commented Jun 13, 2019

I think a callback function can be added in here L235.

@awlane
Copy link
Collaborator

awlane commented Jun 19, 2019

@ramonfontes I've been investigating to see if this is viable and it seems like this section of code is never accessed.

@awlane awlane closed this as completed Jun 19, 2019
@ramonfontes
Copy link
Member

Is bgscan being used?

@awlane
Copy link
Collaborator

awlane commented Jun 20, 2019

@ramonfontes We weren't which seemed to be the issue. I'll try and get the code cleaned up and make a PR.

@awlane awlane reopened this Jun 20, 2019
@awlane
Copy link
Collaborator

awlane commented Jun 24, 2019

@ramonfontes Wanted to follow up on this, one main issue seems to be concurrency issues being caused by both threads attempting to access the station's shell at once. At the moment the current check for whether or not there's a lock on the shell is an assertion which just throws an error in the mobility thread. Is there a preferred way you would want this to be handled?

@ramonfontes
Copy link
Member

ramonfontes commented Jun 24, 2019

Hi @zephyrmoth! Could you elaborate on that? How is #233 working?

@awlane
Copy link
Collaborator

awlane commented Jun 25, 2019

@ramonfontes At the moment, you can add callbacks as an optional param of station objects, which is called on handover. Running the method passed through generally works correctly as long as you avoid relatively referencing internal variables of the calling class.

However, it seems that when executing station on station objects (such as setIP), there are internal calls to cmd() such as here at L539. The issue comes from the fact that because callbacks run in the mobility thread ATM, it is possible for a user script or human using the CLI to be actively executing a shell program (esp. things like ping) while the mobility thread makes a call to it, which causes the mobility thread to exit with an error due to the assertion here at L667. This does prevent contention but seems like it would need some tweaks if this change is to move forward.

@ramonfontes
Copy link
Member

Yes, this happens due to the multiple processing tasks. You can use util/m as workaround for some tasks (e.g. set ip addr, scanning, association, etc.). The other way would be by means of some tweaks in the code, as you said.

@awlane
Copy link
Collaborator

awlane commented Jun 25, 2019

@ramonfontes The simplest solution I can think of is using the Lock object from the core library to replace the waiting parameter and just blocking, but obviously this could introduce changes in behavior. If you would be interested in merging this change this seems like the best solution as it makes it easier to implement callbacks with the normal API, but I can understand if you don't.

EDIT: I realized waiting is a multipurpose parameter and used by Mininet, but locks still seem like a viable solution.

@awlane
Copy link
Collaborator

awlane commented Jun 28, 2019

@ramonfontes I implemented my changes and they seem to be working correctly from my tests. I just added some simple thread safety to the cmd method with locks, which does not seem to be causing any additional issues and should not effect non-mobile scenarios.

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

No branches or pull requests

3 participants