Skip to content
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

RFC6022 (YANG Module for NETCONF Monitoring) #39

Closed
SCadilhac opened this issue Sep 4, 2018 · 19 comments
Closed

RFC6022 (YANG Module for NETCONF Monitoring) #39

SCadilhac opened this issue Sep 4, 2018 · 19 comments

Comments

@SCadilhac
Copy link
Contributor

Hello,

I'm just discovering this project, this is great work really!
Just wondering whether implementing RFC6022 (streaming of Yang models via Netconf) is somewhat planned?

Thanks,

Sylvain

@olofhagsand
Copy link
Member

No, I have not received a request on this before. Was it any specific sub-part you were interested in, I am interested in the use-case?
Best
--Olof

@SCadilhac
Copy link
Contributor Author

I'm using a network orchestration tool which (optionally) relies on this RFC to automatically retrieve the supported models from the device, for a smooth integration.

  1. The supported Yang models must be advertised as capabilities by the device in the hello message.
  2. The orchestrator issues a get > filter > netconf-state > schemas to check the list of available Yang (or Yin, or XSD...) models directly served by the device.
  3. Then the orchestrator can issue a get-schema for each schema it needs to download.

@olofhagsand
Copy link
Member

Ok, I need to look at that a little more closely to understand how much work this would be. Would you be interestwd in contributing?

@SCadilhac
Copy link
Contributor Author

Im looking at the Clixon's code in order to evaluate the needed work too...
By the way, it seems that the announced capabilities follow the older form, e.g. urn:ietf:params:xml:ns:netconf:capability:candidate:1:0 instead of urn:ietf:params:netconf:capability:candidate:1.0. Any reason for that?

@olofhagsand
Copy link
Member

No reason, apart from following the original RFC.

@olofhagsand
Copy link
Member

@SCadilhac clixon will be updated with new announced netconf capability: urn:ietf:params:netconf:capability:yang-library:1.0 in 3.8 (October). Following RFC 7895 and RFC 7950.
Aim for RFC 6022 in 3.9.

@SCadilhac
Copy link
Contributor Author

Hi @olofhagsand, great work thank you!
I had started coding a part of it too, but had to temporary switch back to another project, sorry I haven't been of more help so far.

@SCadilhac
Copy link
Contributor Author

Hi @olofhagsand, I had a quick look at the develop branch, and I'm not sure to understand why you removed the basic capabilities, such as urn:ietf:params:netconf:base:1.0. My understanding is that yang-library should be an additional capability, not the single one.

Example:

<hello xmlns="urn:ietf:params:xml:ns:netconf:base:1.0">
  <capabilities>
    <capability>urn:ietf:params:netconf:base:1.1</capability>
    <!-- Add here other capabilities like validate, candidate, etc. -->
    <capability>urn:ietf:params:xml:ns:yang:ietf-netconf-monitoring?module=ietf-netconf-monitoring&revision=2010-10-04</capability><!-- As per RFC 6022 -->
    <capability>urn:ietf:params:netconf:capability:yang-library:1.0?revision=2016-06-21&amp;module-set-id=[id]</capability><!-- As per RFC 7895 and 7950 -->
  </capabilities>
</hello>

Notes:

  • It seems that special character escaping is required in Netconf, e.g. & becomes &amp;.
  • The namespace from the Yang module must match the xmlns attribute of the root branch modeled by that module in the configuration (RPC response to get-config).

olofhagsand added a commit that referenced this issue Oct 17, 2018
…d xpath features enabled.

* Added urn:ietf:params:netconf:capability:yang-library:1.0
* Thanks SCadilhac for helping out, see #39
* uri_percent_encode() and xml_chardata_encode() changed to use stdarg parameters
@olofhagsand
Copy link
Member

@SCadilhac thanks. Added back the netconf announcements of capabilities in hello. When reading RFC7950 Sec 5.6.4 I interpreted it first as yang module library as replacing the capability announcement in Netconf RFC6241 Sec 8. But it doesnt say that- RFC7895 seems to be an additional way to announce capabilities - or actually modules and their features which seem to be similar to the netconf capability announcement, but not the same. It is a little confusing.
I am glad for your assistance.

@SCadilhac
Copy link
Contributor Author

Thank you for the very quick fix! I will do some interoperability testing based on the develop branch and will let you know the result.

@SCadilhac
Copy link
Contributor Author

SCadilhac commented Oct 23, 2018

Hi @olofhagsand,

I've made some testing, and have seen the following issues:

  • clixon only understands the legacy ]]>]]>-based framing mechanism (see RFC6242, section 4), so it should only advertises urn:ietf:params:netconf:base:1.0 capability (and not 1.1).
  • Single quotes should be accepted around XML parameters such as <blabla xmlns='namespace'>.
  • Character escaping like &amp; shouldn't be followed by a space (character escaping but also parsing).
  • The revision shouldn't be between quotes in urn:ietf:params:netconf:capability:yang-library:1.0?revision=%s&module-set-id=%s.
  • The urn:ietf:params:netconf:capability:writable-running:1.0 capability should be announced.
  • The namespace (xmlns) attribute of modules-state is not passed by the Netconf agent.

I've changed the code for a few of these items already, do you want a PR?

@olofhagsand
Copy link
Member

Yes please!

@SCadilhac
Copy link
Contributor Author

PR submitted. For the last issue (namespace attribute of modules-state), I have to dig into the code to understand why xmlns is lost on the way...

@SCadilhac
Copy link
Contributor Author

SCadilhac commented Oct 24, 2018

xml_merge doesn't seem to preserve xmlns. I guess anyway when printing out the XML back via Netconf following a Yang model, the namespace of each level should be printed (via xmlns attribute) whenever it's different than the parent.

@SCadilhac
Copy link
Contributor Author

SCadilhac commented Oct 26, 2018

Hi @olofhagsand,

I have patched xml_merge as follows: d858f4c
But the other Netconf operations such as get-config also need to add the right namespace (xmlns) at the roots of the tree. Need a global change here?

Also noticed that conformance-type, although mandatory, is not announced by the current code. See d858f4c.

@olofhagsand
Copy link
Member

cherry-picked two of the commits but have some comments on the other, see #45
I know I have been sloppy with namespace handling. Had plans to do that after 3.8 release, along with some basic yang sanity checking.
But preserving xmlns in xml_merge needs to be fixed.

@olofhagsand
Copy link
Member

@SCadilhac maybe you can submit individual issues on the remaining errors (not in #45)? Such as:

  • legacy ]]>]]>-based framing
  • Single quotes should be accepted around XML parameters
    etc, since it is easier to keep track of them that way. I can do it too otherwise.

This was referenced Oct 27, 2018
@SCadilhac
Copy link
Contributor Author

Hi @olofhagsand,
Thanks for the feedback. I've logged new issues so that each error can be followed in a clear way.

olofhagsand added a commit that referenced this issue Oct 28, 2018
…system).(@SCadilhac)

No quotes around revision value. (@SCadilhac)
Removed '42+' session id.
See #39
@olofhagsand
Copy link
Member

Closing this since it diverged and split into sub-items, eg #50

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants