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

misc fixes: ip6 #174

Merged
merged 3 commits into from
Mar 5, 2025
Merged

misc fixes: ip6 #174

merged 3 commits into from
Mar 5, 2025

Conversation

christophefontaine
Copy link
Collaborator

@christophefontaine christophefontaine commented Mar 4, 2025

  • Simple fix on interface creation.
  • Cleanup ip6 rib when an IPv6 address is deleted

When an interface was created with a wrong configuration, such as:
$ add interface port p1 devargs net_tap1,iface=p1 mac f0:0d:ac:dc:00:01
rxqs 2

The interface isn't created and the graph is destroyed.
As the stats pointer is NULL, and rte_graph_cluster_stats_destroy
doesn't validate its input, Grout crashes.

Signed-off-by: Christophe Fontaine <[email protected]>
@christophefontaine christophefontaine changed the title infra: fix crash on interface creation misc fixes: ip6 Mar 4, 2025
Copy link
Collaborator

@rjarry rjarry left a comment

Choose a reason for hiding this comment

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

I'm not sure about the fix. Let me know what you think.

Copy link
Collaborator

@rjarry rjarry left a comment

Choose a reason for hiding this comment

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

I didn't check in detail, but we may need to audit rib*_cleanup functions to ensure no loose treads.

rib6_delete(nh->vrf_id, nh->iface_id, &nh->ipv6, RTE_IPV6_MAX_DEPTH);
rib6_delete(nh->vrf_id, nh->iface_id, &nh->ipv6, nh->prefixlen);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have a feeling that we should keep this at the beginning:

rib6_delete(nh->vrf_id, nh->iface_id, &nh->ipv6, RTE_IPV6_MAX_DEPTH);

And only call the second one if nh->prefixlen != 0 (i.e. it is a local interface address). Gateway nexthops do not have their prefixlen set to anything:

if (nh->prefixlen != 0)
        rib6_delete(nh->vrf_id, nh->iface_id, &nh->ipv6, nh->prefixlen);

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, that's why I didn't want to merge this immediately (even if I need it !) that I put this PR in draft first.
If the gateway NH has not prefix len set, shouldn't we instead set it to RTE_IPv6_MAX_DEPTH / RTE_IPv4_MAX_DEPTH ?

We couldn't add a previously added IPv6 on an interface, either through
the command line, or for link local ip when we add/delete/add an interface.

In rib6_cleanup, call rib6_delete with the prefixlen instead of the max
prefix.

Add these checks in the existing test.

Signed-off-by: Christophe Fontaine <[email protected]>
Similar to ipv6 nexthop suppression, check the NH flags to delete
either with embedded prefix len or the max prefix len (/32).

Update test to ensure we can add/del/add the same address.

Signed-off-by: Christophe Fontaine <[email protected]>
@christophefontaine christophefontaine marked this pull request as ready for review March 5, 2025 12:01
@rjarry rjarry merged commit 075a96e into DPDK:main Mar 5, 2025
8 checks passed
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