openconfig / public

Repository for publishing OpenConfig models, documentation, and other material for the community.
Apache License 2.0
899 stars 656 forks source link

openconfig-if-tunnel.yang: container tunnel should be under the subinterface path. #411

Closed krishna-juniper closed 3 months ago

krishna-juniper commented 3 years ago

I would like to propose a modification to move container tunnel in the openconfig-if-tunnel.yang file under subinterface path. Since Juniper supports multiple logical subinterface associated with tunnel interface.

Container tunnel current path: /interfaces/interface/tunnel

Container tunnel requested path: /interfaces/interface/subinterfaces/subinterface/tunnel

Container tunnel current leaf/container path: /interfaces/interface/tunnel/state/src /interfaces/interface/tunnel/state/dst /interfaces/interface/tunnel/state/ttl /interfaces/interface/tunnel/state/gre-key /interfaces/interface/tunnel/ipv4/state/ /interfaces/interface/tunnel/ipv4/state/counters /interfaces/interface/tunnel/ipv6/state/ /interfaces/interface/tunnel/ipv6/state/counters

Container tunnel requested leaf/container path: /interfaces/interface/subinterfaces/subinterface/tunnel/state/src /interfaces/interface/subinterfaces/subinterface/tunnel/state/dst /interfaces/interface/subinterfaces/subinterface/tunnel/state/ttl /interfaces/interface/subinterfaces/subinterface/tunnel/state/gre-key /interfaces/interface/subinterfaces/subinterface/tunnel/ipv4/state/ /interfaces/interface/subinterfaces/subinterface/tunnel/ipv4/state/counters /interfaces/interface/subinterfaces/subinterface/tunnel/ipv6/state/ /interfaces/interface/subinterfaces/subinterface/tunnel/ipv6/state/counters

From junos router: regress@narada> configure Entering configuration mode

[edit] regress@narada# show interfaces gr-3/0/10 unit 0 { tunnel { source 2.1.1.1; destination 3.2.2.2; ttl 120; } family inet { mtu 346; address 3.4.4.4/24; } } unit 1 { tunnel { source 1.1.3.3; destination 2.2.4.4; ttl 124; } family inet { mtu 250; address 5.5.6.6/32; } }

[edit] regress@narada# run show interfaces gr-3/0/10 Physical interface: gr-3/0/10, Enabled, Physical link is Up Interface index: 210, SNMP ifIndex: 757 Type: GRE, Link-level type: GRE, MTU: Unlimited, Speed: 1000mbps Device flags : Present Running Interface flags: Point-To-Point SNMP-Traps Input rate : 0 bps (0 pps) Output rate : 0 bps (0 pps)

Logical interface gr-3/0/10.0 (Index 348) (SNMP ifIndex 812) Flags: Up Point-To-Point SNMP-Traps 0x4000 IP-Header 3.2.2.2:2.1.1.1:47:df:120:0000000000000000 Encapsulation: GRE-NULL Copy-tos-to-outer-ip-header: Off, Copy-tos-to-outer-ip-header-transit: Off force-control-packets-on-transit-path: Off Gre keepalives configured: Off, Gre keepalives adjacency state: down Input packets : 0 Output packets: 0 Protocol inet, MTU: 346 Max nh cache: 0, New hold nh limit: 0, Curr nh cnt: 0, Curr new hold cnt: 0, NH drop cnt: 0 Flags: Sendbcast-pkt-to-re, User-MTU Addresses, Flags: Is-Preferred Is-Primary Destination: 3.4.4/24, Local: 3.4.4.4, Broadcast: 3.4.4.255

Logical interface gr-3/0/10.1 (Index 65563) (SNMP ifIndex 813) Flags: Up Point-To-Point SNMP-Traps 0x4000 IP-Header 2.2.4.4:1.1.3.3:47:df:124:0000000000000000 Encapsulation: GRE-NULL Copy-tos-to-outer-ip-header: Off, Copy-tos-to-outer-ip-header-transit: Off force-control-packets-on-transit-path: Off Gre keepalives configured: Off, Gre keepalives adjacency state: down Input packets : 0 Output packets: 0 Protocol inet, MTU: 250 Max nh cache: 0, New hold nh limit: 0, Curr nh cnt: 0, Curr new hold cnt: 0, NH drop cnt: 0 Flags: Sendbcast-pkt-to-re, User-MTU Addresses, Flags: Is-Primary Local: 5.5.6.6

Would you please review?

Thanks, Krishna

aashaikh commented 3 years ago

Can you please add some information about how this feature is supported on other platforms also?

krishna-juniper commented 3 years ago

The behaviour is same on all juniper platforms.

vnitinv commented 3 years ago

Hi Anees, @aashaikh If you are asking about other vendor devices, we will have to get access to them to share some data. This request was put on after much discussion by our engineering team.

aashaikh commented 3 years ago

I don't believe access to physical devices from other vendors is required to investigate how this is supported from a configuration/telemetry point of view across. We typically do this due diligence by looking at the way different platforms expresses or structures the configuration, for example, based on documentation, etc.

vnitinv commented 3 years ago

The below links talks about configuring tunnel on cisco devices.

1)https://www.cisco.com/c/en/us/td/docs/ios/12_4/interface/configuration/guide/inb_tun.html#wp1049738

2)https://www.cisco.com/c/en/us/td/docs/routers/ncs6000/software/ncs6k-r7-0/interfaces/configuration/guide/b-interfaces-hardware-component-cg-ncs6000-70x/b-interfaces-hardware-component-cg-ncs6000-70x_chapter_01010.pdf

newly added OC support https://www.cisco.com/c/en/us/td/docs/routers/ncs6000/software/ncs6k-r7-1/programmability/configuration/guide/b-programmability-cg-ncs6000-71x/b-programmability-cg-ncs6000-71x_chapter_00.html

Seems like Cisco configures tunnel under interface itself.

We typically do this due diligence by looking at the way different platforms expresses or structures the configuration, for example, based on documentation, etc.

@aashaikh If I go by Juniper config, this model doesn't look to have considered Junos, as we configure tunnel under subinterface from day 1.

What have been proposed by our engineering team:

image

image

We can send a pull request if this makes sense to you and team.

vnitinv commented 3 years ago

@aashaikh Any feedback?

aashaikh commented 3 years ago

We did discuss this issue in the operator working group, and agreed to spend some time to understand the use cases for tunnel interface management before deciding whether there are any gaps that would justify significant changes to the modeling (this proposal is breaking change).

If I go by Juniper config, this model doesn't look to have considered Junos, as we configure tunnel under subinterface from day 1.

I'll just add that the model design is driven by operational use cases, rather than how a particular platform structures its configuration or CLI. While we do look carefully at how existing platforms support a given feature, it often happens that some parts of the modeling map relatively smoothly to some implementations while others require additional mapping.

krishna-juniper commented 3 years ago

Any recent update on the tunnel path @aashaikh?

aashaikh commented 3 years ago

@krishna-juniper , please see my earlier comment. We will update further once we have some additional operator input on tunnel interface use cases.

robshakir commented 3 years ago

I don't agree with making this change currently.

Across vendor OSes, OpenConfig has to deal with two models:

In the base case, we decided that the right modelling choice was to have subinterfaces be required, since it seemed operationally very clear that we needed subinterfaces for these cases - and this meant that there was consistency in the modelling for these interfaces. In other cases (e.g., routed-vlan and tunnel) we decided that this wasn't required, and since these are already virtual interfaces, there didn't seem to be the same requirement to have them subdivided.

It's worth noting that the OSes that have the latter (no required subinterface) have to do some mapping to support the current OC model for base interfaces, I don't feel its unreasonable for JNPR to need to do this for the tunnel and routed subinterfaces. We can discuss specific configuration cases within the operator group if you can propose them -- even better, please get the users of the model to come and help contribute to meeting this requirement, since today I don't think we have any operators that are asking for something different here.

The proposed change looks strange to me too -- since it seems to imply that there is a single "interface" that will have counters etc. for multiple e.g., GRE tunnels -- this doesn't seem to be how I'd expect things to work from an operational perspective.

dshokarev commented 3 years ago

@robshakir and @aashaikh

Thanks for your feedback.

To you point about other vendors adding dummy sub-interfaces, I hope you would agree that adding a dummy interface (1:1 mapping) from day 1, is quite different from elevating existing sub-interfaces to the interface container after 6 years of supporting openconfig-interface model and using openconfig-interface with many of the customers. Basically, openconfig-tunnel model as it stands now, introduces a disruptive change that many of the Juniper customers would experience if Juniper starts supporting it.

I hope you can re-consider the decision to configure tunnel properties under sub-interface.

github-actions[bot] commented 5 months ago

This issue is stale because it has been open 180 days with no activity. If you wish to keep this issue active, please remove the stale label or add a comment, otherwise will be closed in 14 days.