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

Fixed set mtu for deleted subintf due to late notification #2571

Conversation

EdenGri
Copy link
Contributor

@EdenGri EdenGri commented Dec 13, 2022

What I did

Ignores errors on the set MTU command for subinterface when the subinterface state is not OK.

Why I did it

A race condition between the portmgrd and the intfmgrd sometimes causes running a set MTU command on a deleted subinterface.
The logs and the error:

INFO swss#supervisord: intfmgrd Cannot find device "Ethernet32.58"
ERR swss#intfmgrd: :- main: Runtime error: /sbin/ip link set "Ethernet32.58" mtu "9100" :
INFO swss#supervisord 2022-11-08 05:53:33,057 INFO exited: intfmgrd (exit status 255; not expected)

How I verified it

Run the test_loopback_action_reload test and saw no errors in the logs.

Details if related

@EdenGri EdenGri requested a review from prsunny as a code owner December 13, 2022 20:00
dprital
dprital previously approved these changes Dec 14, 2022
@bingwang-ms
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@EdenGri
Copy link
Contributor Author

EdenGri commented Dec 18, 2022

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 2571 in repo sonic-net/sonic-swss

@EdenGri
Copy link
Contributor Author

EdenGri commented Dec 18, 2022

/azpw run Asure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Asure.sonic-buildimage

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@EdenGri
Copy link
Contributor Author

EdenGri commented Dec 18, 2022

/azpw run Azure.sonic-swss

@EdenGri EdenGri closed this Dec 18, 2022
@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss

@EdenGri EdenGri reopened this Dec 18, 2022
@EdenGri
Copy link
Contributor Author

EdenGri commented Dec 18, 2022

/azpw run Azure.sonic-swss

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@EdenGri
Copy link
Contributor Author

EdenGri commented Dec 19, 2022

/azpw run Azure.sonic-swss

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

cfgmgr/intfmgr.cpp Outdated Show resolved Hide resolved
cfgmgr/intfmgr.cpp Outdated Show resolved Hide resolved

return subifMtu;
if (ret && !isIntfStateOk(alias))
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is already checked in the doIntfGeneralTask under SET operation. So why check again here? is there a race condition?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are not visiting the "doIntfGeneralTask" function in this flow.
The flow is:

  1. changing the "mtu" field in the PORT_TABLE in state DB
  2. doTask
  3. doPortTableTask
  4. updateSubIntfMtu
  5. setHostSubIntfMtu

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok, does this happen in the updateSubIntfAdminStatus flow as well? Or why only mtu change is handled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is happen in the updateSubIntfAdminStatus flow as well

@bingwang-ms
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@EdenGri
Copy link
Contributor Author

EdenGri commented Dec 21, 2022

/azpw run Asure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Asure.sonic-buildimage

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@EdenGri
Copy link
Contributor Author

EdenGri commented Dec 21, 2022

/azpw run Azure.sonic-swss

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@EdenGri
Copy link
Contributor Author

EdenGri commented Dec 26, 2022

/azpw run Azure.sonic-swss

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@EdenGri
Copy link
Contributor Author

EdenGri commented Dec 26, 2022

/azpw run Azure.sonic-swss

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@dprital
Copy link
Collaborator

dprital commented Dec 26, 2022

@prsunny - Can you please approve this PR ?

@prsunny prsunny merged commit 7891e78 into sonic-net:master Dec 28, 2022
dprital added a commit to dprital/sonic-buildimage that referenced this pull request Jan 1, 2023
Update sonic-swss submodule pointer to include the following:
* bdedf69 Modify coppmgr mergeConfig to support preserving copp tables through reboot. ([sonic-net#2548](sonic-net/sonic-swss#2548))
* 7891e78 Fixed set mtu for deleted subintf due to late notification ([sonic-net#2571](sonic-net/sonic-swss#2571))
* a443945 Updated handling of VRF_VNI mapping and VLAN_VNI mapping for same VNI ID ([sonic-net#2538](sonic-net/sonic-swss#2538))

Signed-off-by: dprital <[email protected]>
liat-grozovik pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Jan 2, 2023
Update sonic-swss submodule pointer to include the following:

* bdedf69 Modify coppmgr mergeConfig to support preserving copp tables through reboot. ([#2548](sonic-net/sonic-swss#2548))
* 7891e78 Fixed set mtu for deleted subintf due to late notification ([#2571](sonic-net/sonic-swss#2571))
* a443945 Updated handling of VRF_VNI mapping and VLAN_VNI mapping for same VNI ID ([#2538](sonic-net/sonic-swss#2538))

Signed-off-by: dprital <[email protected]>
liat-grozovik pushed a commit that referenced this pull request Jan 3, 2023
…2595)

PR against 202205 based on the following PR: #2571

- What I did
Ignores errors on the set MTU command for subinterface when the subinterface state is not OK.

- Why I did it
A race condition between the portmgrd and the intfmgrd sometimes causes running a set MTU command on a deleted subinterface.
The logs and the error:

INFO swss#supervisord: intfmgrd Cannot find device "Ethernet32.58"
ERR swss#intfmgrd: :- main: Runtime error: /sbin/ip link set "Ethernet32.58" mtu "9100" :
INFO swss#supervisord 2022-11-08 05:53:33,057 INFO exited: intfmgrd (exit status 255; not expected)

- How I verified it
Run the test_loopback_action_reload test and saw no errors in the logs.
StormLiangMS pushed a commit that referenced this pull request Feb 10, 2023
* Fixed set mtu for deleted subintf due to late notification
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.

7 participants