openconfig / reference

This repository contains reference implementations, specifications and tooling related to OpenConfig-based network management.
Apache License 2.0
155 stars 88 forks source link

calrification on non-leaf path interpretation in Get and Subscribe RPC using JSON & JSON_IETF encodings #167

Open hellt opened 2 years ago

hellt commented 2 years ago

Hi all, I would like to clarify on how targets should interprete non-leaf paths for Get and Subscribe RPCs when any of JSON encodings is used.

Consider the yang schema used in 2.3.1

 root +
      |
      +-- a +
            |
            +-- b[name=b1] +
                           |
                           +-- c +
                                 |
                                 +-- d (string)
                                 +-- e (uint32)

Same section offers the following explanation as to how the node value should be structured, when the path /a/b[name=b1]/c is in use:

For /a/b[name=b1]/c:

update: <
 path: <
   elem: <
     name: "a"
   >
   elem: <
     name: "b"
     key: <
       key: "name"
       value: "b1"
     >
   >
   elem: <
     name: "c"
   >
 >
 val: <
   json_val: `{ "d": "AStringValue", "e": 10042 }`
 >
>

If we put things in to perspective of OC models, we can take the path such as /interfaces/interface[name=Management0]/state/counters and create a subscription request with something like

gnmic -a clab-ceos-ceos:6030 -u admin -p admin -e json --insecure sub --mode once --path "/interfaces/interface[name=Management0]/state/counters" --format prototext

My understanding was that we should expect a target to craft a Notification message with an update that contains a JSON object (using json_val typedvalue) for all the leaves under counters yang container.

Instead, I see that this particular Target N1 expands the requested path to endpoint leaf and provide updates per each (using scalar values instead of json_val, but that is another issue):

❯ gnmic -a clab-ceos-ceos:6030 -u admin -p admin -e json --insecure sub --mode once --path "/interfaces/interface[name=Management0]/state/counters" --format prototext
update:  {
  timestamp:  1656429257909807779
  prefix:  {
    elem:  {
      name:  "interfaces"
    }
    elem:  {
      name:  "interface"
      key:  {
        key:  "name"
        value:  "Management0"
      }
    }
    elem:  {
      name:  "state"
    }
    elem:  {
      name:  "counters"
    }
  }
  update:  {
    path:  {
      elem:  {
        name:  "in-broadcast-pkts"
      }
    }
    val:  {
      uint_val:  0
    }
  }
  update:  {
    path:  {
      elem:  {
        name:  "in-discards"
      }
    }
    val:  {
      uint_val:  5

--snip--

Target N2 provides the following output for a similar (native) path

❯ gnmic -a clab-srl-srl -u admin -p admin --skip-verify -e json_ietf sub --mode once --path "/interface[name=mgmt0]/statistics/" --format prototext
update:  {
  timestamp:  1656495116214397572
  update:  {
    path:  {
      elem:  {
        name:  "srl_nokia-interfaces:interface"
        key:  {
          key:  "name"
          value:  "mgmt0"
        }
      }
      elem:  {
        name:  "statistics"
      }
    }
    val:  {
      json_ietf_val:  "{\"in-octets\": \"74345\", \"in-unicast-packets\": \"42\", \"in-broadcast-packets\": \"0\", \"in-multicast-packets\": \"442\", \"in-discarded-packets\": \"5\", \"in-error-packets\": \"0\", \"in-fcs-error-packets\": \"0\", \"out-octets\": \"10360\", \"out-mirror-octets\": \"0\", \"out-unicast-packets\": \"36\", \"out-broadcast-packets\": \"6\", \"out-multicast-packets\": \"20\", \"out-discarded-packets\": \"0\", \"out-error-packets\": \"0\", \"out-mirror-packets\": \"0\", \"carrier-transitions\": \"1\"}"
    }
  }
}

Does Target 2 do it according to the spec?

I would very much appreciate your inputs @robshakir @wenovus, looks like the ambiguity in the spec leaves room for various interpretations

earies commented 2 years ago

IMO there are more concerns here wrt. JSON related encodings in this context

First - your target N2 assumes aggregation of nodes and for that they truly all need to share the ultimate source timestamp. Just like multiple Update messages packed into a single Notification message, we now have multiple nodes packed into a single Update (and thus single Notification) so the "correct" way to do it veers a bit outside just packing of sub-objects at the requested path anchor point.

Your target N1 above is emitting PROTO which is incorrect for your subscription request

hellt commented 2 years ago

@earies right, I am all ears here to hear what would be the desired output in connection with the json/json_ietf encodings and the path interpretation.

What you're saying making a lot of sense, being able to stream node values independently without bundling them makes sense when top level containers are being subscribed to.

But then the question becomes - should 2.3.1 speicifically call out that the examples applicable to Get RPC only, with Subscribe RPCs not following the aggregation mode presented above