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

Script for setting up linux namespaces #7256

Merged
merged 6 commits into from
Jun 21, 2021

Conversation

cecille
Copy link
Contributor

@cecille cecille commented May 31, 2021

This will let developers run a controller and device on the same
machines.

Problem

We don't have an very easy way to run device and controller on the same machine.

Change overview

Adds a script to allow running device and controller on the same linux machine using namespaces.

Testing

Tested manually using the lighting app and chip-device-ctrl. With ipv4 support, can ping both ways across the namespace in ipv4 and ipv6.

Copy link
Contributor

@bzbarsky-apple bzbarsky-apple left a comment

Choose a reason for hiding this comment

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

Looks reasonable as a general thing; I don't have the background to have verified the details of the script so far. Can read up on this and do that if needed.

@woody-apple
Copy link
Contributor

@cecille please restyle:)

@stale
Copy link

stale bot commented Jun 10, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added stale Stale issue or PR and removed stale Stale issue or PR labels Jun 10, 2021
@msandstedt
Copy link
Contributor

For my edification, is this only needed because all instances of the sdk bind to port 11097? That's a limitation of the sdk, not the spec.

I'm not saying this script isn't needed. I just want to understand why it's needed.

@cecille
Copy link
Contributor Author

cecille commented Jun 10, 2021

Sorry all - I went to go work on some TE3 stuff and let this PR stagnate a bit. It's basically just a helper script for doing controller <-> device development on the same workstation. Being able to compile and test on a linux workstation is just faster than the pi. So you can run the device side using the script and the controller on the same machine. It's not intended as a full test system currently (because it still requires root access).

@woody-apple
Copy link
Contributor

/rebase

@msandstedt
Copy link
Contributor

Sorry all - I went to go work on some TE3 stuff and let this PR stagnate a bit. It's basically just a helper script for doing controller <-> device development on the same workstation. Being able to compile and test on a linux workstation is just faster than the pi. So you can run the device side using the script and the controller on the same machine. It's not intended as a full test system currently (because it still requires root access).

I understand why this is useful. But do you know specifically what limitations we have in the sdk that force us to create such a solution? It would be good to know because these limitations do not arise from the spec. These are limitations of our implementation choices in the sdk.

If we could know these limitations, we could probably address them so that the sdk could support this type of operation out of the box. Is this really just as simple as the fact everybody is binding to the same port 11097? If so, I think this will be easily addressed once mDNS is ubiquitous in the stack and fully supported.

@woody-apple
Copy link
Contributor

/rebase

cecille and others added 5 commits June 17, 2021 02:02
@woody-apple woody-apple force-pushed the linux_ip_namespace_support branch from 1ba3585 to 9217aea Compare June 17, 2021 02:02
@cecille
Copy link
Contributor Author

cecille commented Jun 17, 2021

Mdns would still throw a wrench in the works though, right? it's all 5353

@msandstedt
Copy link
Contributor

Mdns would still throw a wrench in the works though, right? it's all 5353

All of the instances should be able to advertise by talking to avahi-daemon.

@cecille
Copy link
Contributor Author

cecille commented Jun 21, 2021

That would mean that no one could use the minimal mdns implementation for testing, but maybe that's not a deal breaker.

Honestly, I don't care enough about this to fight to have it go in. I have a local copy that I can use. @msandstedt If you can put up a PR to allow port changes and maybe instructions on how to do device -> controller on the same computer, I can abandon this PR.

@msandstedt
Copy link
Contributor

That would mean that no one could use the minimal mdns implementation for testing, but maybe that's not a deal breaker.

Honestly, I don't care enough about this to fight to have it go in. I have a local copy that I can use. @msandstedt If you can put up a PR to allow port changes and maybe instructions on how to do device -> controller on the same computer, I can abandon this PR.

I'm not at all claiming we shouldn't have the tool provided by this PR. I think it's great and I think we should have it. That's why I approved some time ago.

I'm just using this as a vehicle for us to discuss why we would need such a tool. I think it's good for us to understand this so that we don't inadvertently introduce constraints into the sdk that really shouldn't exist. Based on the discussion, I expect (in the absence of such a tool) the constraints for Matter nodes operating within a single network stack are at least the following:

  • require a single, aggregating advertiser (e.g. avahi-daemon) for mDNS
  • cannot share ports for other services
  • only one node can be a member of any given Matter multicast group
  • there are likely other implicitly shared bits of state (e.g. within BLE transceivers)

@andy31415 andy31415 merged commit e47b41c into project-chip:master Jun 21, 2021
@cecille cecille deleted the linux_ip_namespace_support branch July 23, 2021 19:34
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
* Script for setting up linux namespaces

This will let developers run a controller and device on the same
machines.

* Apply suggestions from code review

Co-authored-by: Tennessee Carmel-Veilleux <[email protected]>

* reformatting.

* Add example and header.

* Restyled by shellharden

* Restyled by shfmt

Co-authored-by: Tennessee Carmel-Veilleux <[email protected]>
Co-authored-by: Restyled.io <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants