Skip to content

Feature/vf mac change func#714

Merged
guvenc merged 3 commits intomainfrom
feature/vf_mac_change_func
Aug 28, 2025
Merged

Feature/vf mac change func#714
guvenc merged 3 commits intomainfrom
feature/vf_mac_change_func

Conversation

@PlagueCZ
Copy link
Copy Markdown
Contributor

I created a wrapper function to set the VF MAC address. This will later be helpful for synchronizing the MAC address with the backup dpservice.

MAC address is only changed once per-lifetime of the VF. The old value is kept and reused as a "suggestion" until ARP/DHCP/ND arrives.

The initial value is set in the process init stage to the MAC address of the representor.

I also renamed dp_set_neigh_mac() as it was meant as the "neighboring router mac", i.e. neighbor of a PF, never a VF.

Fixes #713

@PlagueCZ PlagueCZ requested a review from a team as a code owner August 27, 2025 16:45
@github-actions github-actions bot added size/L enhancement New feature or request labels Aug 27, 2025
Comment thread src/dp_port.c

port = dp_get_port_by_id(port_id);
if (!port) {
if (!port || !port->is_pf) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is just to make sure the function is not called on a bad port.
I checked the code and this original function was only ever called on a PF? so this should not change behavior.

Comment thread src/dp_periodic_msg.c
rte_ether_addr_copy(&port->own_mac, &arp_hdr->arp_data.arp_sha);
if (dp_arp_cycle_needed(port))
if (dp_l2_addr_needed(port))
arp_hdr->arp_data.arp_tip = htonl(port->iface.cfg.own_ip);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This will now behave differently!

Previously this branch was being executed until ARP arrived.

Now this will not only stop after ARP, but also after DHCP or ND.

I think it is fine, but this warrants someone else's attention to check my thought process.

Copy link
Copy Markdown
Contributor

@guvenc guvenc Aug 28, 2025

Choose a reason for hiding this comment

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

This branch is still good, as long as the "l2_addr_received" is set to false, after a dpservice restart. (which is the case)
As after a dpservice restart, no DHCP cycle would run and this code path would make sure the dpservice ARP cache gets a quick update. (This "if branch" changes the gratuitous ARP to a normal ARP request asking for the MAC of the VM's assigned ip)

@PlagueCZ
Copy link
Copy Markdown
Contributor Author

Oh, the pytest failure is because I developed this on the new isolation branch and that one changes WCMP test in one place.
But i wanted this to not require rebasing, so it will pass after the other PR is accepted.

Copy link
Copy Markdown
Contributor

@guvenc guvenc left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread src/dp_periodic_msg.c
rte_ether_addr_copy(&port->own_mac, &arp_hdr->arp_data.arp_sha);
if (dp_arp_cycle_needed(port))
if (dp_l2_addr_needed(port))
arp_hdr->arp_data.arp_tip = htonl(port->iface.cfg.own_ip);
Copy link
Copy Markdown
Contributor

@guvenc guvenc Aug 28, 2025

Choose a reason for hiding this comment

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

This branch is still good, as long as the "l2_addr_received" is set to false, after a dpservice restart. (which is the case)
As after a dpservice restart, no DHCP cycle would run and this code path would make sure the dpservice ARP cache gets a quick update. (This "if branch" changes the gratuitous ARP to a normal ARP request asking for the MAC of the VM's assigned ip)

Comment thread test/local/test_arp.py
UDP(dport=1234))
delayed_sendp(pkt, VM1.tap)

def test_l2_addr_once(request, prepare_ifaces, grpc_client):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I liked this new test

@guvenc
Copy link
Copy Markdown
Contributor

guvenc commented Aug 28, 2025

@PlagueCZ could you please rebase to main ?

@PlagueCZ PlagueCZ force-pushed the feature/vf_mac_change_func branch from 0b16f29 to 786ca5e Compare August 28, 2025 16:37
@guvenc guvenc merged commit 786ca5e into main Aug 28, 2025
5 checks passed
@guvenc guvenc deleted the feature/vf_mac_change_func branch August 28, 2025 16:39
@github-project-automation github-project-automation bot moved this to Done in Roadmap Aug 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Unify MAC address handling in ARP/DHCP/ND

3 participants