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

Fix: interfaces of the switches did not have an IP address. #46

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Vo-Alex
Copy link

@Vo-Alex Vo-Alex commented Nov 23, 2022

Hello, while doing the exercise 09-Traceroutable of p4-learning, I noticed that the interfaces of the switches did not have an IP address.

After inspecting the code, I deduced that the problem was due to indentation errors in the save_topology function

I hope I have helped

@edgar-costa
Copy link
Collaborator

Hi!

I believe that is not the reason. And without checking if the indentation of save_topology is correct or not. The reason why switches do not get IPs is the following:

When the network is created by mininet, all switches are placed into the same namespace (the root one). If you set IPs to those interfaces, all the IPs and interfaces will co-exist in the same namespace. What happens then is that routing and forwarding will be performed by the Linux kernel instead of the p4-switches, which are just C++ programs sniffing and sending packets to some bound interfaces.

As an example if you have: h1---S1(ip1)----S2-------S3(ip3)---h3

When you send a packet from h1 to h3, when the packet arrives to the root namespace at S1, it will be directly sent to S3 interface connected to h3 by the linux kernel... bypassing S2 and all the P4 programs.

@Vo-Alex
Copy link
Author

Vo-Alex commented Nov 23, 2022

Yes, I see, but the problem here is that when the mininet topology is written in JSON format by save_topology, the fake IP addresses given to the switch interfaces are not saved. This breaks the controller script.

@edgar-costa
Copy link
Collaborator

For the other thing, thanks. When I find time I will have a look at it, to make sure the change does not break anything.

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

Successfully merging this pull request may close these issues.

2 participants