gnmi-1.15 gnmi_set_test.go

[cprabha]

Hi,

As Per interface yang file , useful convention for interface ID is combination of base interface name and subinterface index.

Yang ref: https://github.com/openconfig/public/blob/master/release/models/interfaces/openconfig-interfaces.yang#L191

typedef interface-id { type string; description "User-defined identifier for an interface, generally used to name a interface reference. The id can be arbitrary but a useful convention is to use a combination of base interface name and subinterface index."; }

Also please check other references where we are using subinterface index in id.
https://github.com/openconfig/featureprofiles/blob/bc37772def113e682052073c87a1bb8c8094483c/internal/fptest/networkinstance.go#L53

Thanks, Prabha

[OpenConfigBot]

Pull Request Functional Test Report for #2853 / 908a3e85cd404d26f0c9c7485809f1a8f4d6df93

Virtual Devices

Device	Test	Test Documentation	Job	Raw Log
Arista cEOS		gNMI-1.15: Set Requests
Cisco 8000E		gNMI-1.15: Set Requests
Cisco XRd		gNMI-1.15: Set Requests
Juniper ncPTX		gNMI-1.15: Set Requests
Nokia SR Linux		gNMI-1.15: Set Requests
Openconfig Lemming		gNMI-1.15: Set Requests

Hardware Devices

Device	Test	Test Documentation	Raw Log
Arista 7808		gNMI-1.15: Set Requests
Cisco 8808		gNMI-1.15: Set Requests
Juniper PTX10008		gNMI-1.15: Set Requests
Nokia 7250 IXR-10e		gNMI-1.15: Set Requests

Help

[LimeHat]

As Per interface yang file , interface id should be combination of base interface name and subinterface index.

No, that's not true. There's no SHOULD in the yang. It literally says that the id is an arbitrary string. cc @dplore

[coveralls]

Pull Request Test Coverage Report for Build 9409039381

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 55.5%

---

Totals
Change from base Build 9407003945:	0.0%
Covered Lines:	1983
Relevant Lines:	3573

---

💛 - Coveralls

[cprabha]

As Per interface yang file , interface id should be combination of base interface name and subinterface index.

No, that's not true. There's no SHOULD in the yang. It literally says that the id is an arbitrary string. cc @dplore

@LimeHat please check updated description where we have other references which shows id is combination of interface name and sub interface index.

[dplore]

I think the code is correct as is. An interface name is permitted to be any arbitrary string which is system (vendor) defined.

Therefore, we do expect the format of the string to vary between implementations and devices. I don't believe this test intends to validate the format of the string, rather just that the interface node (identified by said string) exists when added to a NETWORK_INSTANCE.

[dplore]

I notice that in metadata.textproto, no DUT is assigned the use of deviations.InterfaceRefInterfaceIDFormat for this particular test. If it is truly not needed then the deviation could be removed.

But I think in practice, I see at least Cisco IOS XR does need this in some other tests? I assume this is because Cisco IOS XR explicitly appends ".0" to their interface names when added to a NETWORK_INSTANCE?

@aaronmillisor

[LimeHat]

I notice that in metadata.textproto, no DUT is assigned the use of deviations.InterfaceRefInterfaceIDFormat for this particular test. If it is truly not needed then the deviation could be removed.

Looks like it is needed by the OP, but instead of enabling it, this PR proposes to change the id for everyone :-)

@LimeHat please check updated description where we have other references which shows id is combination of interface name and sub interface index.

I don't doubt that there are references and this format can be used. On the other hand, if an implementation requires this format, I don't see why this shouldn't be a deviation (like it is today)

[cprabha]

But I think in practice, I see at least Cisco IOS XR does need this in some other tests? I assume this is because Cisco IOS XR explicitly appends ".0" to their interface names when added to a NETWORK_INSTANCE?

@aaronmillisor

@dplore Yeah, we are adding interface into network instance along with subinterface index.

    niface := ni.GetOrCreateInterface(name)
niface.Interface = ygot.String(name)
niface.Subinterface = ygot.Uint32(uint32(sub))

[r4huluk]

@dplore @LimeHat

If we are adding two subinterfaces of the same interface under the same network-instance, shouldn't the id be unique (i.e., interface-name + subinterface index)

[LimeHat]

It is already unique in this case, isn't it?

[r4huluk]

It is already unique in this case, isn't it?

Yes, in this testcase. My question was generic. Shouldn't the format of id remain same for all cases while making it unique for the I case i mentioned earlier? Implementations that do not support the use of interface-name+subinterface-index format can use a deviation to pick a format of their choice. Implementations that already use interface-name+subinterface-index format shouldn't be expected to use a deviation.

[dplore]

It is already unique in this case, isn't it?

Yes, in this testcase. My question was generic. Shouldn't the format of id remain same for all cases while making it unique for the I case i mentioned earlier? Implementations that do not support the use of interface-name+subinterface-index format can use a deviation to pick a format of their choice. Implementations that already use interface-name+subinterface-index format shouldn't be expected to use a deviation.

In practice because OC doesn't have a standard interface string format, any interface name manipulation or parsing should be done via deviation. The only "standard" / "non-deviated" solution we can use at the moment is an exact string match.

[r4huluk]

It is already unique in this case, isn't it?

Yes, in this testcase. My question was generic. Shouldn't the format of id remain same for all cases while making it unique for the I case i mentioned earlier? Implementations that do not support the use of interface-name+subinterface-index format can use a deviation to pick a format of their choice. Implementations that already use interface-name+subinterface-index format shouldn't be expected to use a deviation.

In practice because OC doesn't have a standard interface string format, any interface name manipulation or parsing should be done via deviation. The only "standard" / "non-deviated" solution we can use at the moment is an exact string match.

@dplore: Can you please share an example showing how controller will push a network-instance configuration having two subinterfaces from same physical interface with interface string format as id?

[LimeHat]

@r4huluk the point is that OC does not require to use any fixed format.

Any implementation that requires a fixed format deviates from the OC definition. Would you agree with that?

Your argument is "the current id format, used in this test, cannot be used to create the two subinterfaces on the same interface". That is true, but does that invalidate the previous point? Absolutely not.

[earies]

Honestly I think things got a bit murky over time wrt. the interface-id typedef cc: @aashaikh as the original commit went in > 8 years ago on 03/30/2016

From history, the intention from what I recall was that the interface-id was to represent the fully qualified interface format of an "interface" + "subinterface" however the delimiter was left up in the air (ideally a .). It's not until items like this across various implementations expose potential differences and interpretations much like an interop exercise.

What this was not meant to be is a user-defined alias which appears to be how it is implemented in SRL (for very good reason since the description stmt states this). Such a case of this is inconsistent then across the system and could vary anytime interface-id is in use.

In addition is another similar node under the interfaces model "subinterfaces/subinterface/state" named name which carries the following description stmt:

  leaf name {
    type string;
    description
      "The system-assigned name for the sub-interface.  This MAY
      be a combination of the base interface name and the
      subinterface index, or some other convention used by the
      system.";
    oc-ext:telemetry-on-change;
  }

This is another example of a fully qualified name (that could be leafref'd and keyed elsewhere however state only) however the description stmt states "system-defined" here.

As a consumer in this case, I want to bind an interface to a network-instance, why is a user-defined alias necessary other than introducing inconsistency and complexity in config gen + state retrieval?

[robshakir]

OpenConfig project historian perspective only.

As a YANG practicality: we have lists of interfaces that require a base interface name, but optionally require a subinterface -- in such lists, we must have a key that isn't "interface subinterface" since the YANG keys must all be specified.

There was a long discussion on this matter: https://github.com/openconfig/public/pull/802

[LimeHat]

What this was not meant to be is a user-defined alias which appears to be how it is implemented in SRL (for very good reason since the description stmt states this). Such a case of this is inconsistent then across the system and could vary anytime interface-id is in use.

SRL implementation is based on what was discussed/clarified in pull/802 referenced by Rob. Specifically, one of the comments in that discussion stated the following:

It should be possible to name the key anything that one wants (there are no such constraints described in the model). It is OK to require a specific format if and only if the interface and subinterface leaves are used to resolve the interface that is being referenced.

[earies]

It should be possible to name the key anything that one wants

This defeats the purpose of the typedef and the key should then just be an opaque user-defined string if we have too loose of rules around.

IMO - the following logic should apply:

1. If the interface-id is meant to be a user-defined alias then create a common structure in a subtree that defines this value that has child interface + subinterface leafrefs (The pattern we see repeated all over various models). Then adjust all other models to point to this definition. Essentially, we just created an abstract user-defined name (alias) to represent a mapping to an interface (or interface + subinterface)

But is the above necessary? Seems like unwarranted additional overhead for both the consumer + implementor. What does an "alias" mean and how far would a system need to go to embed into their implementation. All such cases are likely to be alien and thus all have impacts when it comes to data correlation to underlying subsystems.

OR

2. Put a strict protocol around interface-id so that it has common meaning. I don't understand why we can't state:

• If you have an interface without any subinterfaces, encode directly as the interface name (e.g. et-0/1/0)
• If you have an interface with >= 1 subinterfaces, use a . delimeter (e.g. et-0/1/0.0)

You wont necessarily validate within the language rules of YANG but rather this typedef is a soft contract that it relates to certain schema nodes when parsed (common tooling can be written against the typedef now). Child leafrefs (interface/subinterface) can still exist to provide the redundant hard relationship.

OR

3. We don't use interface-id at all - this example seems provide no benefit of the typedef and its an ungated user-defined string.

Every instance in the model-set that defines this structure that uses an interface-id for a key today means the consumer can generate whatever they want across all definitions. If you wanted to create foo here to map an interface to NI, you are free to do so, in the ACL domain, you can create bar and map to the same underlying constructs.

I question any case of user-defined objects if they provide no real meaning, especially if we are trying to drive similar behaviors across the system and allow for divergence.

[r4huluk]

@LimeHat @dplore: Thank you for the discussion/clarifications. We will add the deviation back in this pull request as you both have suggested. Discussion that @earies has initiated to make interface-id typedef more stringent can continue in OC forum.

[dplore]

3. Put a strict protocol around interface-id so that it has common meaning. I don't understand why we can't state:

• If you have an interface without any subinterfaces, encode directly as the interface name (e.g. et-0/1/0)
• If you have an interface with >= 1 subinterfaces, use a . delimeter (e.g. et-0/1/0.0)

I opened a pull request to explore a new string type with regex pattern constraint at https://github.com/openconfig/public/pull/1092