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

NETCONF Server HLD #1428

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

NETCONF Server HLD #1428

wants to merge 2 commits into from

Conversation

ntt-omw
Copy link

@ntt-omw ntt-omw commented Jul 14, 2023

NETCONF Server HLD

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jul 30, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: nakano-omw / name: Kanji Nakano (3f69841)
  • ✅ login: ebiken-ntt / name: Kentaro Ebisawa (32e394b)

<rpc-reply xmlns="urn:ietf:params:xml:ns:netconf:base:1.0" message-id="urn:uuid:4e02f166-fc2d-4701-a7a1-b15145cc01d6"><data><interfaces xmlns="http://openconfig.net/yang/interfaces"><interface><name>Ethernet0</name><config><name>Ethernet0</name></config><subinterfaces><subinterface><index>0</index><config><index>0</index></config><ipv4 xmlns="http://openconfig.net/yang/interfaces/ip"><addresses><address><ip>10.1.1.1</ip><config><ip>10.1.1.1</ip><prefix-length>24</prefix-length></config></address></addresses></ipv4></subinterface></subinterfaces></interface></interfaces></data></rpc-reply>
```

## Open/Action items - if any
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to mention the notification support (https://datatracker.ietf.org/doc/html/rfc8639) in case for scaling up capabilities in the future.

@mbalachandar
Copy link
Collaborator

Will the NETCONF server be disabled by default ? since we already have REST, and Telemetry running.

@mbalachandar
Copy link
Collaborator

NETCONF server will be running in separate docker container?

- target : running (as commit feature is not supported, only Running config is applicable)
- test-option : test-and-set
- error-option : rollback-on-error
- [*2] get-config will return config in netopeer2/sysrepo and not from CONFIG_DB
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is understood that the sysrepo serves running datastore. But, I am not clear on how to init the sysrepo at the server init. Can you elaborate?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As of this release, sysrepo will serve as running database only for the config modified by NETCONF. Thus, it will be initialized with no data on startup. In future, we are planning to introduce functionality to sync sysrepo with CONFIG_DB but that is not scope of this release.
Would this answer your question? Let me know if you were asking from different aspect. (I will update HLD doc otherwise)

Copy link

@kwangsuk kwangsuk Aug 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your elaboration. I have a follow-up comment.

As we move forward, constant syncing Sysrepo will be going to be problematic and overhead to ensure data integrity. Can you make NETCONF architectured similar to sonic-mgmt-framework or sonic-gnmi?
The sonic-mgmt-common is a common infra and built into both distinct servers to provide REST/gNMI service, respectively. In the similar way, NETCONF server can be built with sonic-mgmt-common in a separate docker to directly use Translib apis mapped from NETCONF request and eliminate the redundant datastore by accessing CONFIG_DB as a single datastore for running configuration.

@ebiken-ntt
Copy link
Contributor

ebiken-ntt commented Aug 8, 2023

@mbalachandar

Will the NETCONF server be disabled by default ? since we already have REST, and Telemetry running.

Yes. NETCONF will be disabled by default.
We are considering making it configurable (e.g., via config_db), once we decide if it should be a separate container or not.

NETCONF server will be running in separate docker container?

Currently, no. It's running as a process in mgmt-framework container.
But we can consider separating it to a separate docker container if that's your recommendation and fits better with the SONiC architecture.

@zyboy2000
Copy link

How do ensure that the NetConf running database and Redis database are synchronized? for example CLI ? Is it possible for NetConf to fetch directly from the Redis database and deprecate its own running database

@ridahanif96
Copy link
Collaborator

In the context of NETCONF capabilities, does this HLD will cover extended capabilities as well?

@ebiken-ntt
Copy link
Contributor

Noting as future reference. (Comment from Kwan Kim)

In today’s (14th Sep) meeting, one of questions from NTT is about NETCONF Get-config operation.

NETCONF Get-config can be translated to REST Get with the “content” query parameter, i.e. content=config.
https://datatracker.ietf.org/doc/html/rfc8040#section-4.8.1

We have opened PRs for query parameter support.
sonic-net/sonic-mgmt-common#102
sonic-net/sonic-mgmt-framework#119

@zhangyanzhao
Copy link
Collaborator

not target 202311 per Kanji update. Keep a record.

@burnCalories
Copy link

burnCalories commented Apr 2, 2024

@ntt-omw @nakano-omw @ebiken-ntt
Hi ,
I have read the document you submitted and I fully agree with your design for integrating netconf into Sonic. I wanted to inquire if you have already completed the development work to support netconf in Sonic. Recently, I have been considering enabling netconf functionality in my Sonic switch. Although your documentation is very detailed, I am not sure where to start. Could you provide me with some advice or suggest some source code for me to study?

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

Successfully merging this pull request may close these issues.

9 participants