-
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 policy forwarding for next hop match and GUE encapsulation action #1208
base: master
Are you sure you want to change the base?
Conversation
/gcbrun |
Major YANG version changes in commit dc39a8e: |
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.
Thanks for making this PR @danameme. I have a couple of comments.
release/models/policy-forwarding/openconfig-pf-forwarding-policies.yang
Outdated
Show resolved
Hide resolved
release/models/policy-forwarding/openconfig-pf-forwarding-policies.yang
Outdated
Show resolved
Hide resolved
"A pointer to an entry in an ordered list of next-hop-groups."; | ||
} | ||
|
||
leaf ip-address { |
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.
What is the operational scenario? Do you want to match on the name of a next-hop-group which is configured? Or the next-hop IP prefix?
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.
Match on the next hop IP prefix, the GUE encapsulation action is based on the protocol next hop IP.
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.
Ok, SGTM
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.
We need to model next-hop here instead of next-hop group
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.
LGTM. Setting this to last call
/gcbrun |
Regarding implementations for this policy language, we are pursing support for this from multiple vendors. |
/gcbrun |
Can we link another implementation here? |
There is only one known implementation. We are actively engaged with multiple vendors who have agreed that this is feasible to implement on their platforms. Finalizing the OC model helps complete the specification. |
A few comments that I mentioned during the call today:
It would be good to In addition, could you please elaborate on possible combinations of match/action rules? Some of the PF actions can affect the egress lookup (e.g. next-hop or network-instance actions), so I think we should clearly indicate which combinations are illegal (can i combine next-hop match with next-hop action?). Last but not least, I'm a slightly perplexed by the mix of NH and NHG constructs in the proposal. |
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.
From a Cisco perspective, we have some concerns and questions with this propsed set of changes for GUE support.
We are discussing more internally to provide more feedback and potentially an alternative proposal, as we believe there are alternative ways to represent what is needed here with config fitting into other places in the OC models.
In particular the 'encapsulate' action under PF needs more discussion.
I discussed offline with @danameme and this will receive a major refactor. Please standby while Dan prepares it. |
Change Scope
Platform Implementations
Tree view