-
Notifications
You must be signed in to change notification settings - Fork 17
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
Support of OSPFv3 #401
base: master
Are you sure you want to change the base?
Support of OSPFv3 #401
Conversation
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.
overall looks ok similar to ospfv2. RFC references at some places are good to have.
device/ospfv3/router.yaml
Outdated
status: under_review | ||
information: OSPFv3 is currently under review for pending exploration on use cases. | ||
type: object | ||
required: [name, router_id, interfaces] |
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.
router id can be choice of ipv6 intf or custom ipv6 address and with default interface-ip. so required field can be excluded.
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.
Router id is 32 bit so cannot point to ipv6 intf . But can have a auto choice where first IPv4 addess on the router is used. If none are available for use, implementation should return an error. This is possible to keep an auto option.
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.
Added 'auto' option. Keeping comment open as reminder to clarify implementation choices around the 'auto' option.
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.
Initial review . Main issue : Support of multi-instances .
Secondary : Impact of route config in router with multiple interfaces of different types of areas ( also relevant for OSPFv2 probably ) .
format: uint32 | ||
default: 10 | ||
x-field-uid: 3 | ||
priority: |
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.
Maybe priority can be clubbed with broadcast type of link since it is actually not a relevant field for p2p type.
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.
Keeping it open to clarify impact on UX.
device/ospfv3/router.yaml
Outdated
status: under_review | ||
information: OSPFv3 is currently under review for pending exploration on use cases. | ||
type: object | ||
required: [name, router_id, interfaces] |
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.
Router id is 32 bit so cannot point to ipv6 intf . But can have a auto choice where first IPv4 addess on the router is used. If none are available for use, implementation should return an error. This is possible to keep an auto option.
When used in Hello packets, the Options field allows a router to reject a neighbor because of a capability mismatch. | ||
type: object | ||
properties: | ||
dc_bit: |
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.
Some of these bits should probably be in the router level in the model , considering multiple interfaces. Need to check RFC once on which bits MUST be same across multiple interfaces and which are truly per interface information.
information: OSPFv3 is currently under review for pending exploration on use cases. | ||
type: object | ||
required: [name, router_id, interfaces] | ||
properties: |
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.
Probably we need to start with array of instances and then have router within it. Otherwise we will not be able to support OSPFv3 Multi instance in any reasonable way.
x-field-uid: 3 | ||
external_type_2: | ||
x-field-uid: 4 | ||
nssa_external: |
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.
Might be good to clarify how to setup NSSA areas , probably need non zero area id + NSSA capability + external type routes should not be sent in the positive test case .. maybe need a warning from implementation etc . Basically thinking about impact of multiple interfaces in the model, some say area 0 and some area 1 with or without nssa , user will probably not be able to emulate sending specific routes for specific interfaces depending on type .. for now with single interface it is fine but might not scale well with true multiple interfaces. Need some tests to work out correct model for this. Might need to push routes to interface OR clarify in routes that either routes will be converted to NSSA or External if type mismatches or NSSA interfaces will not transmit External routes etc.
Redocly Link: https://redocly.github.io/redoc/?url=https://raw.githubusercontent.com/open-traffic-generator/models/ospfv3/artifacts/openapi.yaml&nocors
OTG-PORT-1-OSPFv3 <-----------------------> DUT <-------------------------> OTG-PORT-2-OSPFv3