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

[portmgrd]: Add portmgrd to monitor port MTU configurations #545

Merged
merged 1 commit into from
Aug 14, 2018

Conversation

stcheng
Copy link
Contributor

@stcheng stcheng commented Jul 25, 2018

In order to get rid of /etc/network/interfaces file and move
all the configurations to the configuration database, MTU configurations
are required to be put into the database and monitored by a specific
daemon. This daemon is right now to achieve this goal.

Currently, this daemon will only listen to the port MTU configurations
if existed in the database and then call the 'ip link' command to
configure the kernel netdevs. Then, portsyncd will catch the messages
via libnl and send the message to orchagent.

Signed-off-by: Shu0T1an ChenG shuche@microsoft.com

Copy link
Contributor

@jipanyang jipanyang left a comment

Choose a reason for hiding this comment

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

The existing mtu set processing in portsyncd.cpp should be updated to avoid conflict. https://github.com/Azure/sonic-swss/blob/762e7da80142358af0d348dea1143f38aa60a992/portsyncd/portsyncd.cpp#L303

@stcheng
Copy link
Contributor Author

stcheng commented Jul 25, 2018

@jipanyang thanks for pointing this out. I have removed this part of the code. In the next stage, all the configuration part of the code will be moved to portmgrd and then we could finally remove the dependency of libnl.

@jipanyang
Copy link
Contributor

jipanyang commented Aug 7, 2018

If mtu gets changed on an existing router interface, should the new mtu take effect?

For now, SAI_ROUTER_INTERFACE_ATTR_MTU seems to get set only with sai_router_intfs_api->create_router_interface().

For system init, any mechanism to ensure that the mtu is set before IntfsOrch::addRouterIntfs()? In case mtu change won't be applied on an existing route interface.

@stcheng
Copy link
Contributor Author

stcheng commented Aug 9, 2018

@jipanyang Yes. I'll update this pull request to handle this case.

Copy link
Contributor

@jipanyang jipanyang left a comment

Choose a reason for hiding this comment

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

Since PR #574 (comment) has been opened to address the router intf mtu issue, the changes here look good me. #574 (comment) should be merged before this one.

In order to get rid of /etc/network/interfaces file and move
all the configurations to the configuration database, MTU configurations
are required to be put into the database and monitored by a specific
daemon. This daemon portmgrd is to achieve this goal.

Currently, this daemon will only listen to the port MTU configurations
if existed in the database and then call the 'ip link' command to
configure the kernel netdevs.

It will also capture the admin status set in the configuration database
and call the 'ip link' command to configure the kernel netdevs.

The default MTUs, however, are set as the default value in the orchagent
port.h file. The MTU value in the netlink message is anyway ignored.

In the next stage, the change of the MTU will be supported so that the
kernel host interface MTU will be reflected to the SAI port/router
interface MTU.

Signed-off-by: Shu0T1an ChenG <shuche@microsoft.com>
@stcheng
Copy link
Contributor Author

stcheng commented Aug 10, 2018

update this pull request to support admin status configuration as well.

@stcheng stcheng merged commit 671f248 into sonic-net:master Aug 14, 2018
@stcheng stcheng deleted the portmgr branch August 14, 2018 17:03
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-swss that referenced this pull request Mar 1, 2023
* Move api mutex to global class and add sairedis namespace

* Move apiInitialized flag to Globals namespace

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

Successfully merging this pull request may close these issues.

3 participants