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

HLD - Kernel programming performance enhancement - Netlink API #493

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

naveensuvarna
Copy link

High Level Design document for Kernel programming performance enhancement using Netlink based APIs.

@msftclas
Copy link

msftclas commented Oct 18, 2019

CLA assistant check
All CLA requirements met.

config vlan member range add 10 4009 PortChannel005


Netlink API library will be placed in sonic-swss path with directory named 'nlapi'.
Copy link
Contributor

Choose a reason for hiding this comment

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

Currently we have a Netlink message handler in sonic-swss-common. I think it will be better to have this nl library in swss-common as well.

Copy link
Author

Choose a reason for hiding this comment

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

At present it is used only in SWSS, hence implemented in swss. Will look into if we can easily put it into swss common.


Iproute2 ip and bridge commands internally use Netlink based messaging to program the kernel constructs like bridge, VLAN, link, VRF etc.

Objective of this enhancement is to implement the Netlink based Library function calls or APIs instead of using fork based iproute2 command execution. The API calls make use of netlink library to construct netlink messages and configure the kernel as per user request. Library API functions will perform the same functionality as the iproute2 ip and bridge commands invoked by SONiC Modules.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we using native netlink calls or libnl for talking to kernel? we do have libnl wrapper on top of native netlink functions, can you add some details on the internal design to talk to kernel? hope the code can be enhanced easily to add more netlink APIs in the near future?

Copy link
Author

Choose a reason for hiding this comment

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

We are using native netlink message calls; Netlink socket and formated messages.
NLAPI netlink kernel interaction implementaion is similar/inline with how Iproute2 package implements netlink calls. API invoking functions are internally designed and implemented such that they can be expanded easily for any additional attributes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In principle, its better to reuse the existing libnl and add patches to it (where ever required) instead of writing new APIs. Can this be relooked? W.r.t. comparing against iproute2, there is a big difference. iproute2 is not maintained by SONiC; but if we write new library for SONiC, we end up maintaining it for SONiC, which is a overhead if we already have similar library like libnl.

@kannankvs
Copy link
Collaborator

By the by, I am not sure whether such system calls are used in modules/repos other than SWSS. If so, do we have plan to change them too?

Following APIs will be supported in current release


int nlapi_req_ip_link_add_dev(const char *dev, const char *type, const char *admin)
Copy link
Contributor

Choose a reason for hiding this comment

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

What does req mean? If meaningless, can you remove it from the API names?



Netlink API library will be placed in sonic-swss path with directory named 'nlapi'.
Following APIs will be supported in current release
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these APIs 1:1 mapping with old ip command lines?

## 2.2 Functional Description
This document provides functional design and specifications of kernel programming performance enhancement by replacing system() calls with netlink APIs for kernel interaction.

SONiC modules program the kernel constructs like bridge, VLAN, IP-link, VRF, IP Address into kernel using unix iproute-2 utility commands. Within a sonic process, this is being done by forking new process using fork (thru popen) system call and then executing required iproute2 ip and/or bridge commands in it. This system() call invocation is slow (compared to netlink APIs) and executed synchronously and process executing this waits for its completion, impacting the performance. Kernel programming using this method is primarily used in config managers of SwSS components. Config managers predominantly use the popen call based kernel programming during user configuration or config replay at restart or system bringup.

Choose a reason for hiding this comment

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

Does this include LAG, LAG member as well?teammgr in sonic-swss creates LAG and LAG members using 'ip link'

int nlapi_req_ip_link_set_master(const char *dev, const char *master, const char *admin)
int nlapi_req_ip_link_set_filtering(const char *dev, const char *type, unsigned int filter_id)
int nlapi_req_ip_link_del_dev(const char *dev)
int nlapi_req_ip_link_list(const char *dev, unsigned int display, int *count)

Choose a reason for hiding this comment

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

Does this API return only the count or list of links? Can you rename it?

int nlapi_req_ip_link_set_filtering(const char *dev, const char *type, unsigned int filter_id)
int nlapi_req_ip_link_del_dev(const char *dev)
int nlapi_req_ip_link_list(const char *dev, unsigned int display, int *count)
int nlapi_req_ip_link_list_type(const char *type, int display, int *count)

Choose a reason for hiding this comment

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

Is this API to return type or count??

int nlapi_req_ip_link_list(const char *dev, unsigned int display, int *count)
int nlapi_req_ip_link_list_type(const char *type, int display, int *count)
int nlapi_req_ip_link_list_master_ifs(const char *dev, int display, int *count)
int nlapi_req_ip_address_cmd(const char *action, int version, const char *prefix, const char *dev)

Choose a reason for hiding this comment

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

Can we use enum for add/del actions? This can be used for other APIs as well and have common API for add/del based on the enum type.

@yxieca yxieca force-pushed the master branch 2 times, most recently from 8498931 to 8837dc2 Compare April 15, 2022 16:51
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.

None yet

7 participants