Skip to content

Fix nhop_dmac in load-balance/s1-runtime.json#619

Merged
jafingerhut merged 2 commits intop4lang:masterfrom
stano45:fix-s1-runtime-dmac
Jun 27, 2024
Merged

Fix nhop_dmac in load-balance/s1-runtime.json#619
jafingerhut merged 2 commits intop4lang:masterfrom
stano45:fix-s1-runtime-dmac

Conversation

@stano45
Copy link
Copy Markdown
Contributor

@stano45 stano45 commented Jun 13, 2024

Corrected the nhop_dmac fields in s1-runtime.json. The MAC addresses of the hosts should start with 08, as defined in topology.json.

This actually didn't matter for forwarding, since the nhop_dmac was correct in the runtime files of s2 and s3, but still this could confuse someone examining the file.

I tested the change locally.

@fruffy fruffy requested a review from rst0git June 14, 2024 15:24
@rst0git
Copy link
Copy Markdown
Member

rst0git commented Jun 16, 2024

@stano45 Would you be able to also update the smac values in s2-runtime.json and s3-runtime.json?

@stano45
Copy link
Copy Markdown
Contributor Author

stano45 commented Jun 18, 2024

@rst0git I assumed the smac values are meant to represent the MAC address of the given switch port. In the case of s2 and s3 the values seem to be correct, in the format of 00:00:00:0x:0y:00 where x is the switch id and y is the port. Please correct me if I'm wrong.

@rst0git
Copy link
Copy Markdown
Member

rst0git commented Jun 18, 2024

@stano45 I believe that addresses in the format 00:x:x:x:x:x are leftovers from a previous version of this exercise (which has been updated with a new topology). The topology file (described here) defines how hosts are instantiated:

    "hosts": {
        "h1": {
            "ip": "10.0.1.1/24",
            "mac": "08:00:00:00:01:01",
            "commands": [
                "route add default gw 10.0.1.10 dev eth0",
                "arp -i eth0 -s 10.0.1.10 08:00:00:00:01:00"
            ]
        },
        "h2": {
            "ip": "10.0.2.2/24",
            "mac": "08:00:00:00:02:02",
            "commands": [
                "route add default gw 10.0.2.20 dev eth0",
                "arp -i eth0 -s 10.0.2.20 08:00:00:00:02:00"
            ]
        },
        "h3": {
            "ip": "10.0.3.3/24",
            "mac": "08:00:00:00:03:03",
            "commands": [
                "route add default gw 10.0.3.30 dev eth0",
                "arp -i eth0 -s 10.0.3.30 08:00:00:00:03:00"
            ]
        }
    }

The route commands are used to add static routes to a default gateway (the switch), and the arp commands add ARP cache entries for the switch's MAC address. For example, in the case of h3, outbound traffic is sent to the gateway at 10.0.3.30 with destination MAC address 08:00:00:00:03:00. The P4 program on the switch then uses set_nhop and rewrite_mac actions to update the destination and source MAC addresses of the packet, respectively.

@jafingerhut
Copy link
Copy Markdown
Collaborator

I wanted to suggest another idea, to see if you think it has merit:

Unlike traditional Ethernet switches and IP routers, it is easy to write P4 programs, especially exercises like the ones found in this repo, that DO NOT HAVE TO abide by what a real commercially supported device would do. For example, if you wanted to, you could make an IP router that used the Ethernet header source and destination MAC addresses contain 2*48=96 bits of arbitrary data, used for purposes completely different than a commercially-supported device would.

Perhaps it might be nice in one of the tutorials exercises README's to explicitly point that out? Then we could also mention that while the table entries might not have what appear to be the kinds of entries you would see in a commercially-supported Ethernet switch, but this is intentionally done to emphasize this property of P4-programmable devices?

@stano45
Copy link
Copy Markdown
Contributor Author

stano45 commented Jun 19, 2024

@stano45 I believe that addresses in the format 00:x:x:x:x:x are leftovers from a previous version of this exercise (which has been updated with a new topology). The topology file (described here) defines how hosts are instantiated:

    "hosts": {
        "h1": {
            "ip": "10.0.1.1/24",
            "mac": "08:00:00:00:01:01",
            "commands": [
                "route add default gw 10.0.1.10 dev eth0",
                "arp -i eth0 -s 10.0.1.10 08:00:00:00:01:00"
            ]
        },
        "h2": {
            "ip": "10.0.2.2/24",
            "mac": "08:00:00:00:02:02",
            "commands": [
                "route add default gw 10.0.2.20 dev eth0",
                "arp -i eth0 -s 10.0.2.20 08:00:00:00:02:00"
            ]
        },
        "h3": {
            "ip": "10.0.3.3/24",
            "mac": "08:00:00:00:03:03",
            "commands": [
                "route add default gw 10.0.3.30 dev eth0",
                "arp -i eth0 -s 10.0.3.30 08:00:00:00:03:00"
            ]
        }
    }

The route commands are used to add static routes to a default gateway (the switch), and the arp commands add ARP cache entries for the switch's MAC address. For example, in the case of h3, outbound traffic is sent to the gateway at 10.0.3.30 with destination MAC address 08:00:00:00:03:00. The P4 program on the switch then uses set_nhop and rewrite_mac actions to update the destination and source MAC addresses of the packet, respectively.

@rst0git Ah okay now I understand. I will update the PR accordingly.

@stano45
Copy link
Copy Markdown
Contributor Author

stano45 commented Jun 19, 2024

I wanted to suggest another idea, to see if you think it has merit:

Unlike traditional Ethernet switches and IP routers, it is easy to write P4 programs, especially exercises like the ones found in this repo, that DO NOT HAVE TO abide by what a real commercially supported device would do. For example, if you wanted to, you could make an IP router that used the Ethernet header source and destination MAC addresses contain 2*48=96 bits of arbitrary data, used for purposes completely different than a commercially-supported device would.

Perhaps it might be nice in one of the tutorials exercises README's to explicitly point that out? Then we could also mention that while the table entries might not have what appear to be the kinds of entries you would see in a commercially-supported Ethernet switch, but this is intentionally done to emphasize this property of P4-programmable devices?

@jafingerhut I think that's a good idea. Maybe we could mention this in one of the earlier exercises, so that anyone working through the tutorials already understands at the beginning that P4 is protocol-independent (and not necessarily required to use standard MAC addresses). What do you think?

@jafingerhut
Copy link
Copy Markdown
Collaborator

@jafingerhut I think that's a good idea. Maybe we could mention this in one of the earlier exercises, so that anyone working through the tutorials already understands at the beginning that P4 is protocol-independent (and not necessarily required to use standard MAC addresses). What do you think?

I like that idea. I do not have a recommendation right now for which exercise that might be. Note that it is easier in some exercises to redefine the meaning of Ethernet MAC addresses between P4-programmable switches, than it might be between a switch and a host, because the emulated hosts probably do have certain requirements about the destination MAC address of packets they accept.

Copy link
Copy Markdown
Member

@rst0git rst0git left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Collaborator

@jafingerhut jafingerhut left a comment

Choose a reason for hiding this comment

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

LGTM

@jafingerhut jafingerhut merged commit d5870d7 into p4lang:master Jun 27, 2024
@stano45 stano45 deleted the fix-s1-runtime-dmac branch June 28, 2024 10:00
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.

3 participants