-
Notifications
You must be signed in to change notification settings - Fork 656
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
Add gratuitous-arp-accepted and unsolicited-na-accepted #1120
base: master
Are you sure you want to change the base?
Add gratuitous-arp-accepted and unsolicited-na-accepted #1120
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
/gcbrun |
No major YANG version changes in commit 991105b |
Additional references for IPv4 gratuitous arp: Cisco IOS XR - JunOS - set gratuitous-arp-reply Nokia SR Linux - For IPv6 accept unsolicited Neighbor Advertisements (related to RFC9131 Section 4.2), see |
/gcbrun |
1 similar comment
/gcbrun |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. Small comment on updating the versioning and I think this is ready to go.
/gcbrun |
Last call for comments. This will merge on Oct 8, 2024 |
Can we please extend this last call? The latest changes escaped my attention, and I have a few concerns about the ipv6 part of this PR. Also, the arista doc reference is still behind the paywall, which makes it harder to follow for the broader public.
I have no objection to merging the ipv4 leaf (if the author and the community decides to split this into 2 parts) |
I found this reference in RFC9131 which is relevant to the new leaf For IPv6 accept unsolicited Neighbor Advertisements (related to RFC9131 Section 4.2), see Seems to make sense to me to add this as well to be up to date with RFC9131? |
/gcbrun |
Updated last call to Oct 10, 2024 to give a little time for @cagdasalagoz-arista and @LimeHat to comment. |
Correct, the feature is about accepting and processing unsolicited-na requests.
Is this correct? @LimeHat |
default true; | ||
description | ||
"When set to true unsolicited neighbor advertisements | ||
will be accepted and the ARP table will be updated."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove:
- any references to ARP (it doesn't exist in ipv6).
- any references to
accepted
(without specifying the conditions) - this is not what RFC9131 describes.
Unsolicited NA should be accepted with or without 9131 behavior, and will be processed in either case if the entry already exists in cache (this is why I initially asked which behavior you are trying to describe and based on your response asked for the default value).
RFC9131 changes the behavior only for the specific case when the entry doesn't exist yet, quoting the rfc:
This document updates [RFC4861] so that routers create a new Neighbor Cache entry upon receiving an unsolicited Neighbor Advertisement for an address that does not already have a Neighbor Cache entry. These changes do not modify the router behavior specified in [RFC4861] for the scenario when the corresponding Neighbor Cache entry already exists.
The description should be modified accordingly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
description is updated to:
When set to true neighbors will be learned from unsolicited
neighbor advertisements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cagdasalagoz-arista please the text to use "Neighbor Cache" instead of ARP, since this is IPv6.
Please also update the description so it is clear if you are intending
- To enable/disable RFC4861 behavior
or - To enable/disable RFC9131 behavior
The key difference between the two is whether or not the router already has a Neighbor Cache entry. If there is already an entry, RFC4861 says it can be updated. If there is not one, the cache won't be updated.
RFC9131 says do RFC4861 AND also update the cache is there is no entry.
As written, it is not clear in this yang node which behavior is being configured.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @dplore ,
I think you accidentally commented on the older version of the commit. The latest version is like this:
leaf learn-unsolicited {
type enumeration {
enum NONE {
value 0;
}
enum GLOBAL {
value 1;
}
enum LINK_LOCAL {
value 2;
}
enum BOTH {
value 3;
}
}
default "NONE";
description
"Sets if neighbors should be learned from unsolicited neighbor
advertisements for global or link local addresses or both.";
reference
"RFC 9131: Routers Creating Cache Entries upon
Receiving Unsolicited Neighbor Advertisements";
}
Reference contains RCF9131
Hi Darren @dplore ,
It does make sense, but the initial description of the leaf was not accurate and the first conversation in the comments didn't help to clear it up, unfortunately. :) Another question is what type of the leaf this should be, boolean or enum (please see my comment above). |
/gcbrun |
There's a request for a change. I'm updating the last call date to Oct 22, 2024 |
/gcbrun |
/gcbrun |
Additional implementations: JunOS ipv6 neighbor discovery guide Cisco IOS XR - ipv6 nd na glean |
@LimeHat any additional comments? |
Reset last call to Nov 15, 2024 |
Change Scope
Two new boolean leaf values will be added to paths below,
interfaces/interface/subinterfaces/subinterface/ipv4/config/
interfaces/interface/subinterfaces/subinterface/ipv6/config/
.A. Yang tree for IPv4
B. Yang tree for IPv6
As the name suggests these will be used to toggle if the device will accept (and process) or reject the gratuitous arp updates.
This change is backwards compatible
Platform Implementations
A. Implementation for IPv4
Arista
These commands configure interface ethernet 2/1 to accept gratuitous ARP request packets.
B. Implementation for IPv6
Arista