Skip to content

uRPF through another VRF OC proposal #1320

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

Open
wants to merge 29 commits into
base: master
Choose a base branch
from

Conversation

nupkanoi
Copy link

@nupkanoi nupkanoi commented Jun 20, 2025

Change Scope

  • Please briefly describe the change that is being made to the models- this change adds support for uRPF to be able validation source IP address from a different instance/vrf from the forwarding instance/vrf.
  • Please indicate whether this change is backwards compatible - Yes
module: openconfig-interfaces
  +--rw interfaces
     +--rw interface* [name]
        +--rw subinterfaces
           +--rw subinterface* [index]
              +--rw oc-ip:ipv4
               | +--rw oc-ip:urpf
               |    +--rw oc-ip:config
               |    |  +--rw urpf-lookup-network-instance         oc-ni:network-instance-ref
               |    +--rw oc-ip:state
               |       +--rw urpf-lookup-network-instance         oc-ni:network-instance-ref
              +--rw oc-ip:ipv6
                 +--rw oc-ip:urpf
                    +--rw oc-ip:config
                    |  +--rw urpf-lookup-network-instance         oc-ni:network-instance-ref
                    +--rw oc-ip:state
                       +--rw urpf-lookup-network-instance         oc-ni:network-instance-ref

Platform Implementations

@dplore
Copy link
Member

dplore commented Jun 23, 2025

/gcbrun

@OpenConfigBot
Copy link

OpenConfigBot commented Jun 23, 2025

No major YANG version changes in commit b1145b8

@dplore dplore moved this to Ready to discuss in OC Operator Review Jun 24, 2025
@dplore
Copy link
Member

dplore commented Jun 24, 2025

In concept this LGTM. It should be rebased to be compatible with #1307 which should merge in the next day or so.

Also, please update the "Change Scope" with a description of the change.

@dplore dplore moved this from Ready to discuss to Waiting for author in OC Operator Review Jun 24, 2025
@nupkanoi
Copy link
Author

/gcbrun

@nupkanoi
Copy link
Author

In concept this LGTM. It should be rebased to be compatible with #1307 which should merge in the next day or so.

Also, please update the "Change Scope" with a description of the change.

Done

@dplore
Copy link
Member

dplore commented Jun 24, 2025

/gcbrun

@dplore
Copy link
Member

dplore commented Jun 25, 2025

/gcbrun

@dplore
Copy link
Member

dplore commented Jun 26, 2025

/gcbrun

@dplore dplore moved this from Waiting for author to last-call in OC Operator Review Jun 26, 2025
@dplore
Copy link
Member

dplore commented Jun 26, 2025

Will review at July 1, 2025 OC Operators meeting. Setting last call for July 8th

@ElodinLaarz
Copy link
Contributor

Discussed at the July 1st OC Operators Meeting-- Sounds good to me.

@dplore
Copy link
Member

dplore commented Jul 1, 2025

/gcbrun

@akvasu
Copy link

akvasu commented Jul 8, 2025

Hi, From Cisco, we would like a little more time to review this model internally as uRPF is generally expected to run on the network instance associated with the interface.

@ElodinLaarz
Copy link
Contributor

Sounds reasonable. We can delay last call by a week for now and if we need more time, we can revisit. - OC Operators Meeting July 8th.

@ElodinLaarz
Copy link
Contributor

@akvasu This is scheduled to merge today-- any last comments here?

@Tavneet-Sidhu13
Copy link

@akvasu This is scheduled to merge today-- any last comments here?

Request one more week to review feasibility of this OC model

@dplore
Copy link
Member

dplore commented Jul 16, 2025

@akvasu This is scheduled to merge today-- any last comments here?

Request one more week to review feasibility of this OC model

Thank you @Tavneet-Sidhu13 , I have extended the last call to July 23, 2025

@amogh-vk
Copy link

Since the urpf-lookup-network-instance is part of the subinterface container, should we assume the interface will be associated with the defined VRFs, or will it remain part of the default VRFs?

Also, please hold off on merging for one more week. We need a little more time to analyze this.

@earies
Copy link
Contributor

earies commented Jul 21, 2025

Since the urpf-lookup-network-instance is part of the subinterface container, should we assume the interface will be associated with the defined VRFs, or will it remain part of the default VRFs?

Also, please hold off on merging for one more week. We need a little more time to analyze this.

This model proposal allows for any combination to cross VRF/NI boundaries and would imply the opposite. The subinterface this is set on is not a member of the target NI used for lookup (otherwise the tagged NI would be redundant and unnecessary)

@@ -1307,6 +1314,14 @@ revision "2023-06-30" {
LPMs to any path in the RIB, even if it is not selected for forwarding in the
FIB.";
}
leaf urpf-lookup-network-instance {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: No need to repeat urpf- here as this already falls under a urpf container

"If populated, this leaf provides the name of the network instance
used to look up a packet's source address. When this leaf is
omitted and uRPF is enabled, the packet's source address is looked
up in the forwarding network-instance.";
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
up in the forwarding network-instance.";
up in the network-instance in which this interface is a member of.";

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: last-call
Development

Successfully merging this pull request may close these issues.

9 participants