Dynamic port configuration - solve lldp issues when adding/removing ports #9386

tomer-israel · 2021-11-28T09:16:02Z

Why I did it

when adding and removing ports after init stage we saw two issues:

first:
In several cases, after removing a port, lldpmgr is continuing to try to add a port to lldp with lldpcli command. the execution of this command is continuing to fail since the port is not existing anymore.

second:
after adding a port, we sometimes see this warning messgae:
"Command failed 'lldpcli configure ports Ethernet18 lldp portidsubtype local etp5b': 2021-07-27T14:16:54 [WARN/lldpctl] cannot find port Ethernet18"

we added these changes in order to solve it.

How I did it

port create events are taken from app db only.
lldpcli command is executed only when linux port is up.

when delete port event is received we remove this command from pending_cmds dictionary

How to verify it

manual tests and running lldp tests

Which release branch to backport (provide reason below if selected)

Description for the changelog

Dynamic port configuration - solve lldp issues when adding/removing ports

A picture of a cute animal (not mandatory but encouraged)

tomer-israel · 2021-11-29T10:22:27Z

/azpw run

mssonicbld · 2021-11-29T10:22:29Z

/AzurePipelines run

azure-pipelines · 2021-11-29T10:22:35Z

You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

zhenggen-xu · 2021-11-30T06:40:02Z

do we know exactly why we see this Command failed 'lldpcli configure ports Ethernet18 lldp portidsubtype local etp5b': 2021-07-27T14:16:54 [WARN/lldpctl] cannot find port Ethernet18? If the netdev operation status is up, still happening? It would be better to understand the reason rather than retrying.

zhenggen-xu · 2021-11-30T05:59:53Z

dockers/docker-lldp/lldpmgrd

@@ -100,31 +109,24 @@ class LldpManager(daemon_base.DaemonBase):
    def is_port_up(self, port_name):
        """
        Determine if a port is up or down by looking into the oper-status for the port in


change oper-status to netdev-oper-status to be more clear

will be changed

zhenggen-xu · 2021-11-30T06:03:23Z

dockers/docker-lldp/lldpmgrd

                return port_oper_status == "up"
            else:
                return False
        else:
-            # Retrieve PortInitDone entry from the Port table


should we log info for debug purpose?

in case the port is not up or not exist on table we have log info for it outside the function

zhenggen-xu · 2021-11-30T06:05:28Z

dockers/docker-lldp/lldpmgrd


+        if port_alias == None:


do we have such case since you check port_alias above? Even if so, let's use AND check in above to share the log_info?

it's not needed, i will remove it

zhenggen-xu · 2021-11-30T06:16:20Z

dockers/docker-lldp/lldpmgrd

+                        self.pending_cmds.pop(key, None)
+                    else:
+                        self.log_error("unknown operation")
+                        


remove trailing spaces?

zhenggen-xu · 2021-11-30T06:19:50Z

dockers/docker-lldp/lldpmgrd

+                        if fvp and "up" in dict(fvp).get("oper_status",""):
+                            self.generate_pending_lldp_config_cmd_for_port(key, dict(fvp))
+                        else:
+                            self.pending_cmds.pop(key, None)


if fvp is empty, should we do no-op?

when we have SET command fvp should always be with values.

but I will change it in a way that:
self.pending_cmds.pop(key, None)
will be executed only if oper_state is "down"

tomer-israel · 2021-12-05T19:24:04Z

do we know exactly why we see this Command failed 'lldpcli configure ports Ethernet18 lldp portidsubtype local etp5b': 2021-07-27T14:16:54 [WARN/lldpctl] cannot find port Ethernet18? If the netdev operation status is up, still happening? It would be better to understand the reason rather than retrying.

we still don't know what is the reason for this warning message, even when the host interface is up we see this message.
in the next retry few seconds after the execution of this cmd passed.

tomer-israel · 2021-12-05T19:24:58Z

/azpw run

mssonicbld · 2021-12-05T19:25:00Z

/AzurePipelines run

azure-pipelines · 2021-12-05T19:25:05Z

You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

tomer-israel · 2021-12-06T15:54:23Z

/azpw run

mssonicbld · 2021-12-06T15:54:26Z

/AzurePipelines run

azure-pipelines · 2021-12-06T15:54:33Z

You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

tomer-israel · 2021-12-07T11:52:39Z

failed checkers due to unknown reason:

-- Docs: https://docs.pytest.org/en/latest/warnings.html
------ generated xml file: /data/sonic-mgmt/tests/logs/1vlan/posttest.xml ------
=========== 3 passed, 2595 deselected, 27 warnings in 138.42 seconds ===========
INFO:root:Can not get Allure report URL. Please check logs
##[error]Bash exited with code '1'.

tomer-israel · 2021-12-07T11:52:47Z

/azpw run

mssonicbld · 2021-12-07T11:52:49Z

/AzurePipelines run

azure-pipelines · 2021-12-07T11:52:55Z

You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

tomer-israel · 2021-12-08T12:53:05Z

not sure why we see these failures on kvm tests-t0:

tacacs/test_authorization.py::test_authorization_tacacs_only_some_server_down[vlab-01] FAILED [ 25%]Loading callback plugin unnamed of type old, v1.0 from /usr/local/lib/python2.7/dist-packages/pytest_ansible-2.2.2-py2.7.egg/pytest_ansible/module_dispatcher/v28.pyc
tacacs/test_authorization.py:87: Failed
DEBUG:root:/data/sonic-mgmt/tests/tacacs/utils.py::stop_tacacs_server#27: [ptf-01] AnsibleModule::service, args=[], kwargs={"state": "stopped", "name": "tacacs_plus"}
DEBUG:root:/data/sonic-mgmt/tests/tacacs/utils.py::stop_tacacs_server#27: [ptf-01] AnsibleModule::service Result => {"failed": false, "state": "stopped", "name": "tacacs_plus", "invocation": {"module_args": {"state": "stopped", "sleep": null, "name": "tacacs_plus",

zhenggen-xu · 2021-12-13T07:30:24Z

do we know exactly why we see this Command failed 'lldpcli configure ports Ethernet18 lldp portidsubtype local etp5b': 2021-07-27T14:16:54 [WARN/lldpctl] cannot find port Ethernet18? If the netdev operation status is up, still happening? It would be better to understand the reason rather than retrying.

we still don't know what is the reason for this warning message, even when the host interface is up we see this message. in the next retry few seconds after the execution of this cmd passed.

This does not make sense. In case host interface is up, lldpctl should not reporting cannot find port error. We should change the check criterion to avoid retrying and without knowing why.

tomer-israel · 2021-12-13T16:14:22Z

/azpw run

mssonicbld · 2021-12-13T16:14:24Z

/AzurePipelines run

azure-pipelines · 2021-12-13T16:14:29Z

You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

tomer-israel · 2021-12-14T08:09:03Z

/azpw run Azure.sonic-buildimage

mssonicbld · 2021-12-14T08:09:05Z

/AzurePipelines run Azure.sonic-buildimage

azure-pipelines · 2021-12-14T08:09:16Z

Azure Pipelines successfully started running 1 pipeline(s).

tomer-israel · 2021-12-14T12:27:44Z

do we know exactly why we see this Command failed 'lldpcli configure ports Ethernet18 lldp portidsubtype local etp5b': 2021-07-27T14:16:54 [WARN/lldpctl] cannot find port Ethernet18? If the netdev operation status is up, still happening? It would be better to understand the reason rather than retrying.

we still don't know what is the reason for this warning message, even when the host interface is up we see this message. in the next retry few seconds after the execution of this cmd passed.

This does not make sense. In case host interface is up, lldpctl should not reporting cannot find port error. We should change the check criterion to avoid retrying and without knowing why.

I checked it again and I noticed that the command is failing (return code is not 0) but the error message (stderr) is an empty string and not "cannot find port Ethernet..."

only after 2 or 3 retries, the return code is 0 - we see this only on several random ports, not always.

so, I guess that checking the oper state of the host interface (from state db) did solve these warning messages ("cannot find port Ethernet...")
I still don't know why the return code is sometimes not 0 without any reason and stderr on these cases is empty

tomer-israel · 2021-12-14T12:29:11Z

/AzurePipelines run Azure.sonic-buildimage (Test kvmtest-t0)

azure-pipelines · 2021-12-14T12:29:16Z

Commenter does not have sufficient privileges for PR 9386 in repo Azure/sonic-buildimage

tomer-israel · 2021-12-14T12:29:50Z

/azpw run Azure.sonic-buildimage (Test kvmtest-t0)

mssonicbld · 2021-12-14T12:29:52Z

/AzurePipelines run Azure.sonic-buildimage (Test kvmtest-t0)

azure-pipelines · 2021-12-14T12:29:57Z

No pipelines are associated with this pull request.

liat-grozovik · 2021-12-20T14:33:36Z

@zhenggen-xu could you please recheck last comment?
@dprital please follow up on this PR

zhenggen-xu · 2021-12-21T07:51:36Z

do we know exactly why we see this Command failed 'lldpcli configure ports Ethernet18 lldp portidsubtype local etp5b': 2021-07-27T14:16:54 [WARN/lldpctl] cannot find port Ethernet18? If the netdev operation status is up, still happening? It would be better to understand the reason rather than retrying.

we still don't know what is the reason for this warning message, even when the host interface is up we see this message. in the next retry few seconds after the execution of this cmd passed.

This does not make sense. In case host interface is up, lldpctl should not reporting cannot find port error. We should change the check criterion to avoid retrying and without knowing why.

I checked it again and I noticed that the command is failing (return code is not 0) but the error message (stderr) is an empty string and not "cannot find port Ethernet..."

only after 2 or 3 retries, the return code is 0 - we see this only on several random ports, not always.

so, I guess that checking the oper state of the host interface (from state db) did solve these warning messages ("cannot find port Ethernet...") I still don't know why the return code is sometimes not 0 without any reason and stderr on these cases is empty

what is the return code during the failure? Can we find out a bit more why it was the case? Sorry for insist here, to understand the exact reason and wait on that condition would be more consistent and deterministic. Retry might be simple to hide the issue but might not be reliable depending on the failure.

tomer-israel · 2021-12-21T10:42:22Z

do we know exactly why we see this Command failed 'lldpcli configure ports Ethernet18 lldp portidsubtype local etp5b': 2021-07-27T14:16:54 [WARN/lldpctl] cannot find port Ethernet18? If the netdev operation status is up, still happening? It would be better to understand the reason rather than retrying.

we still don't know what is the reason for this warning message, even when the host interface is up we see this message. in the next retry few seconds after the execution of this cmd passed.

This does not make sense. In case host interface is up, lldpctl should not reporting cannot find port error. We should change the check criterion to avoid retrying and without knowing why.

I checked it again and I noticed that the command is failing (return code is not 0) but the error message (stderr) is an empty string and not "cannot find port Ethernet..."
only after 2 or 3 retries, the return code is 0 - we see this only on several random ports, not always.
so, I guess that checking the oper state of the host interface (from state db) did solve these warning messages ("cannot find port Ethernet...") I still don't know why the return code is sometimes not 0 without any reason and stderr on these cases is empty

what is the return code during the failure? Can we find out a bit more why it was the case? Sorry for insist here, to understand the exact reason and wait on that condition would be more consistent and deterministic. Retry might be simple to hide the issue but might not be reliable depending on the failure.

the return code in these cases is 1, I don't think it is saying something about the failure..
I agree that it is a strange failure that can point on a real failure but I didn't find something that can explain it..

dprital · 2022-01-21T09:17:48Z

@zhenggen-xu - Can you please approve this PR ?

dprital · 2022-02-06T08:47:20Z

@zhenggen-xu - Can you please approve this PR ?

dprital · 2022-02-13T21:05:43Z

@zhenggen-xu - can you please review and approve ?

zhenggen-xu

We should open a issue link to here for later to understand why lldpcli would fail even when linux port was up.

dprital · 2022-03-09T14:01:20Z

We should open a issue link to here for later to understand why lldpcli would fail even when linux port was up.

Issue was opened: Endless LLDP warning messages when removing a port

dprital · 2022-03-15T22:25:31Z

@qiluo-msft - Who can merge this PR ? Thanks.

qiluo-msft · 2022-03-15T23:45:08Z

Add unit test?
@abdosi Could you help review?

dprital · 2022-03-22T15:16:26Z

@abdosi - can you please review \ approve ?

dprital · 2022-03-24T21:24:39Z

@qiluo-msft \ @abdosi - This PR was reviewed and approved by @zhenggen-xu . Can you please review as well as we would like to merge it ? Thanks.

abdosi

LGTM

dprital · 2022-03-25T20:07:38Z

@qiluo-msft - As this PR was approved , who can merge it ?

…orts (#9386) #### Why I did it when adding and removing ports after init stage we saw two issues: first: In several cases, after removing a port, lldpmgr is continuing to try to add a port to lldp with lldpcli command. the execution of this command is continuing to fail since the port is not existing anymore. second: after adding a port, we sometimes see this warning messgae: "Command failed 'lldpcli configure ports Ethernet18 lldp portidsubtype local etp5b': 2021-07-27T14:16:54 [WARN/lldpctl] cannot find port Ethernet18" we added these changes in order to solve it. #### How I did it port create events are taken from app db only. lldpcli command is executed only when linux port is up. when delete port event is received we remove this command from pending_cmds dictionary #### How to verify it manual tests and running lldp tests #### Description for the changelog Dynamic port configuration - solve lldp issues when adding/removing ports

rraghav-cisco · 2023-03-29T17:26:10Z

@abdosi and @zhenggen-xu ; can this be ported to 202205, pls ?

tomer-israel added 2 commits November 24, 2021 10:40

Dynamic port configuration - lldp bug fixes

a0d3e55

Dynamic port configuration - lldp bug fixes

9865f9e

tomer-israel requested a review from lguohan as a code owner November 28, 2021 09:16

tomer-israel mentioned this pull request Nov 28, 2021

HLD document on add/remove ports dynamically feature sonic-net/SONiC#900

Merged

zhenggen-xu reviewed Nov 30, 2021

View reviewed changes

dprital added the Request for 202111 Branch For PRs being requested for 202111 branch label Dec 1, 2021

Dynamic port configuration - lldp bug fixes

1858de3

tomer-israel requested a review from zhenggen-xu December 5, 2021 19:25

zhenggen-xu approved these changes Mar 2, 2022

View reviewed changes

qiluo-msft requested a review from abdosi March 15, 2022 23:46

abdosi approved these changes Mar 24, 2022

View reviewed changes

qiluo-msft merged commit cc938e7 into sonic-net:master Mar 26, 2022

judyjoseph added the Included in 202111 Branch label Mar 28, 2022

prsunny mentioned this pull request Jul 6, 2022

DUT sent lldp frame with incorrect port ID to EOS neighbor #11255

Closed

Dynamic port configuration - solve lldp issues when adding/removing ports #9386

Dynamic port configuration - solve lldp issues when adding/removing ports #9386

Conversation

tomer-israel commented Nov 28, 2021

Why I did it

How I did it

How to verify it

Which release branch to backport (provide reason below if selected)

Description for the changelog

A picture of a cute animal (not mandatory but encouraged)

tomer-israel commented Nov 29, 2021

mssonicbld commented Nov 29, 2021

azure-pipelines bot commented Nov 29, 2021

zhenggen-xu commented Nov 30, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomer-israel commented Dec 5, 2021

tomer-israel commented Dec 5, 2021

mssonicbld commented Dec 5, 2021

azure-pipelines bot commented Dec 5, 2021

tomer-israel commented Dec 6, 2021

mssonicbld commented Dec 6, 2021

azure-pipelines bot commented Dec 6, 2021

tomer-israel commented Dec 7, 2021

tomer-israel commented Dec 7, 2021

mssonicbld commented Dec 7, 2021

azure-pipelines bot commented Dec 7, 2021

tomer-israel commented Dec 8, 2021

zhenggen-xu commented Dec 13, 2021

tomer-israel commented Dec 13, 2021

mssonicbld commented Dec 13, 2021

azure-pipelines bot commented Dec 13, 2021

tomer-israel commented Dec 14, 2021

mssonicbld commented Dec 14, 2021

azure-pipelines bot commented Dec 14, 2021

tomer-israel commented Dec 14, 2021

tomer-israel commented Dec 14, 2021

azure-pipelines bot commented Dec 14, 2021

tomer-israel commented Dec 14, 2021

mssonicbld commented Dec 14, 2021

azure-pipelines bot commented Dec 14, 2021

liat-grozovik commented Dec 20, 2021

zhenggen-xu commented Dec 21, 2021 • edited Loading

tomer-israel commented Dec 21, 2021

dprital commented Jan 21, 2022

dprital commented Feb 6, 2022

dprital commented Feb 13, 2022

zhenggen-xu left a comment

Choose a reason for hiding this comment

dprital commented Mar 9, 2022 • edited Loading

dprital commented Mar 15, 2022

qiluo-msft commented Mar 15, 2022

dprital commented Mar 22, 2022

dprital commented Mar 24, 2022

abdosi left a comment

Choose a reason for hiding this comment

dprital commented Mar 25, 2022

rraghav-cisco commented Mar 29, 2023

zhenggen-xu commented Dec 21, 2021 •

edited

Loading

dprital commented Mar 9, 2022 •

edited

Loading