search

p4lang / tutorials

P4 language tutorials
Apache License 2.0
1.35k stars 886 forks source link

Updated HDR IPv4 destination IP in s1-runtime.json of load_balance exercise #586

Closed Abhinavcode13 closed 5 months ago

Abhinavcode13 commented 5 months ago
stano45 commented 4 months ago

@Abhinavcode13 @jafingerhut What was the reason for this change? When testing the provided solution on my machine, this seems to have broken the load balancing behavior, only forwarding packets to h2.

The README of this exercise states:

  • The hosts are assigned IPs of 10.0.1.1, 10.0.2.2, etc.
    • We use the IP address 10.0.0.1 to indicate traffic that should be load balanced between h2 and h3.

Meaning that the address 10.0.0.1 is used for traffic to both h2 and h3. Of course, when using 10.0.1.1, the load balancing works as intended, but I assumed 10.0.1.1 was the address for h1, not for traffic that should be load balanced between h2 and h3 as the README indicates. Am I missing something here?

jafingerhut commented 4 months ago

Sorry about approving this. I don't think I reviewed it properly, and I definitely did not retest the exercise with the change.

I have tested it today, and agree with your findings.

If you do not mind creating a PR undoing this change, I will approve it.

I am not sure, but I suspect Abhinov may have seen in s2-runtime.json and s3-runtime.json files entries in the ecmp_group table that had IPv4 addresses 10.0.2.2 and 10.0.3.3, and been reasoning by analogy that it should thus be 10.0.1.1 in s1-runtime.json.

stano45 commented 4 months ago

@jafingerhut Thanks for testing and confirming. I opened a PR to revert this: https://github.com/p4lang/tutorials/pull/612