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

Add gratuitous-arp-accepted and unsolicited-na-accepted #1120

Merged
merged 14 commits into from
Nov 21, 2024

Conversation

cagdasalagoz-arista
Copy link
Contributor

@cagdasalagoz-arista cagdasalagoz-arista commented May 31, 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

/oc-if:interfaces/oc-if:interface/oc-if:subinterfaces/oc-if:subinterface:
    +--rw ipv4
       +--rw config
       |  +--rw enabled?                     boolean
       |  +--rw mtu?                         uint16
       |  +--rw dhcp-client?                 boolean
       |  +--rw gratuitous-arp-accepted?     boolean       <<<new
       +--ro state
          +--ro enabled?                     boolean
          +--ro mtu?                         uint16
          +--ro dhcp-client?                 boolean
          +--ro gratuitous-arp-accepted?     boolean       <<<new
          +--ro counters

B. Yang tree for IPv6

/oc-if:interfaces/oc-if:interface/oc-if:subinterfaces/oc-if:subinterface:
    +--rw ipv6
       +--rw config
       |  +--rw enabled?                     boolean
       |  +--rw mtu?                         uint32
       |  +--rw dup-addr-detect-transmits?   uint32
       |  +--rw dhcp-client?                 boolean
       |  +--rw unsolicited-na-accepted?     boolean    <<<new
       +--ro state
          +--ro enabled?                     boolean
          +--ro mtu?                         uint32
          +--ro dup-addr-detect-transmits?   uint32
          +--ro dhcp-client?                 boolean
          +--ro unsolicited-na-accepted?     boolean    <<<new
          +--ro counters

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.

switch(config)# interface ethernet 2/1
switch(config-if-Et2/1)# arp gratuitous accept
switch(config-if-Et2/1)#

B. Implementation for IPv6

Arista

switch(config-if-Et1/1)#ipv6 nd na unsolicited accept
   
switch#show ipv6 interface et1/1
. . .
  ND unsolicited NAs are accepted

@cagdasalagoz-arista cagdasalagoz-arista requested a review from a team as a code owner May 31, 2024 11:07
Copy link

google-cla bot commented May 31, 2024

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.

@dplore
Copy link
Member

dplore commented Jun 10, 2024

/gcbrun

@OpenConfigBot
Copy link

OpenConfigBot commented Jun 10, 2024

No major YANG version changes in commit 991105b

@dplore
Copy link
Member

dplore commented Jun 10, 2024

Additional references

for IPv4 gratuitous arp:

Cisco IOS XR - no arp gratuitous ignore

JunOS - set gratuitous-arp-reply

Nokia SR Linux - learn-unsolicited true

For IPv6 accept unsolicited Neighbor Advertisements (related to RFC9131 Section 4.2), see
https://datatracker.ietf.org/doc/html/rfc9131#name-routers-creating-cache-entr

release/models/interfaces/openconfig-if-ip.yang Outdated Show resolved Hide resolved
release/models/interfaces/openconfig-if-ip.yang Outdated Show resolved Hide resolved
release/models/interfaces/openconfig-if-ip.yang Outdated Show resolved Hide resolved
@wenovus
Copy link
Contributor

wenovus commented Jun 25, 2024

/gcbrun

1 similar comment
@wenovus
Copy link
Contributor

wenovus commented Jun 28, 2024

/gcbrun

Copy link
Member

@dplore dplore left a 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.

release/models/interfaces/openconfig-if-ip.yang Outdated Show resolved Hide resolved
release/models/interfaces/openconfig-if-ip.yang Outdated Show resolved Hide resolved
release/models/interfaces/openconfig-if-ip.yang Outdated Show resolved Hide resolved
@dplore
Copy link
Member

dplore commented Sep 24, 2024

/gcbrun

@dplore dplore self-assigned this Sep 24, 2024
@dplore dplore added the last-call PR that is in final review before merging. label Sep 24, 2024
@dplore
Copy link
Member

dplore commented Sep 24, 2024

Last call for comments. This will merge on Oct 8, 2024

@LimeHat
Copy link

LimeHat commented 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.

  • according to the arista documentation, the config toggle is used to enable rfc9131-based behavior for processing unsolicited NAs (unfortunately, author gave me a different answer in the conversation earlier); which makes the current leaf description inaccurate. (I'll follow up with a more detailed comment)
  • in the current form arista seems to be the only ipv6 reference. Is there a second one?

I have no objection to merging the ipv4 leaf (if the author and the community decides to split this into 2 parts)

@dplore
Copy link
Member

dplore commented Oct 8, 2024

I found this reference in RFC9131 which is relevant to the new leaf /interfaces/interface/subinterfaces/subinterface/ipv6/config/unsolicited-na-accepted

For IPv6 accept unsolicited Neighbor Advertisements (related to RFC9131 Section 4.2), see
https://datatracker.ietf.org/doc/html/rfc9131#name-routers-creating-cache-entr

Seems to make sense to me to add this as well to be up to date with RFC9131?

@dplore
Copy link
Member

dplore commented Oct 8, 2024

/gcbrun

@dplore
Copy link
Member

dplore commented Oct 8, 2024

Updated last call to Oct 10, 2024 to give a little time for @cagdasalagoz-arista and @LimeHat to comment.

@cagdasalagoz-arista
Copy link
Contributor Author

Correct, the feature is about accepting and processing unsolicited-na requests.
I'll update the unsolicited-na-accepted leaf as :

    leaf unsolicited-na-accepted {
      type boolean;
      default true;
      description
        "When set to true unsolicited neighbor advertisements
        will be accepted and the ARP table will be updated.";
      reference
        "RFC 9131: Routers Creating Cache Entries upon
        Receiving Unsolicited Neighbor Advertisements";
    }

Is this correct? @LimeHat

release/models/interfaces/openconfig-if-ip.yang Outdated Show resolved Hide resolved
release/models/interfaces/openconfig-if-ip.yang Outdated Show resolved Hide resolved
release/models/interfaces/openconfig-if-ip.yang Outdated Show resolved Hide resolved
release/models/interfaces/openconfig-if-ip.yang Outdated Show resolved Hide resolved
release/models/interfaces/openconfig-if-ip.yang Outdated Show resolved Hide resolved
@LimeHat
Copy link

LimeHat commented Oct 9, 2024

Hi Darren @dplore ,

Seems to make sense to me to add this as well to be up to date with RFC9131?

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).

@dplore
Copy link
Member

dplore commented Oct 15, 2024

/gcbrun

@dplore
Copy link
Member

dplore commented Oct 15, 2024

There's a request for a change. I'm updating the last call date to Oct 22, 2024

@dplore
Copy link
Member

dplore commented Oct 16, 2024

/gcbrun

@dplore
Copy link
Member

dplore commented Nov 12, 2024

/gcbrun

@dplore
Copy link
Member

dplore commented Nov 12, 2024

Additional implementations:

JunOS ipv6 neighbor discovery guide

Cisco IOS XR - ipv6 nd na glean

@dplore
Copy link
Member

dplore commented Nov 12, 2024

@LimeHat any additional comments?

@dplore dplore requested a review from LimeHat November 12, 2024 05:57
@dplore
Copy link
Member

dplore commented Nov 12, 2024

Reset last call to Nov 15, 2024

@dplore dplore merged commit 270e457 into openconfig:master Nov 21, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
last-call PR that is in final review before merging. non-breaking
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants