[sonic-swss]: Added flap counter and last flap time functionality. #928

prprakas · 2019-06-12T06:55:20Z

Number of times a link got reset(up/down) has been added under this change, also the last flap timestamp for the link has also been added, they will be updated once the oper status of any port changes. 2 additional fields have been added to appl_db i.e. "flap_counter" and "last_flap_time" which willbe updated when "oper_status" is updated.

Signed-off-by: Prem Prakash prprakash@linkedin.com

What I did
Added flap counter and last flap timestamp feature

Why I did it
This would help in solving operational issues faster. Instead of relying on the logs to figure out a bad flapper we can see this feature under status command.

How I verified it
Tested it by building up the docker-orchagent.gz after my change, changes were required on sonic-utilities repo as well to see the output, I will raise a parallel PR for that.

flap_output.txt

msftclas · 2019-06-12T06:55:32Z

All CLA requirements met.

zhenggen-xu · 2019-06-13T04:52:47Z

retest this please

prprakas · 2019-06-13T05:04:49Z

retest this please

prprakas · 2019-06-13T07:19:45Z

retest this please

prprakas · 2019-06-13T09:14:15Z

retest this please

prprakas · 2019-06-14T11:50:00Z

retest this please

prprakas · 2019-06-19T08:20:09Z

retest this please

prprakas · 2019-06-26T03:30:08Z

Please review the changes.

stcheng

Thanks for submitting this enhancement. This piece of information is definitely an important one that is missing in the current SONiC system. I have the following suggestions that open for discussion:

since the flap counter is initialized as 0, and there is no other components editing this variable, is it okay to store the number in orchagent without querying the value from the database to get the old value?
could you add the flap_counter and last_flap_ts as variables for the struct Port?

While I do have some concerns on whether to put these pieces of information into which database, since the current operational status is already stored in the application database, I am fine with appending these two entries there for now. But we could also discuss on whether to migrate the information into state database.

stcheng

please check my comments

orchagent/portsorch.cpp

stcheng · 2019-07-08T23:08:31Z

orchagent/portsorch.cpp

+{
+  SWSS_LOG_ENTER();
+
+  time_t now = time(0);


use the getTimestamp() function instead

stcheng · 2019-07-08T23:09:30Z

orchagent/portsorch.cpp

+    {
+      return;
+    }
+    updateDbPortLastFlapTime(new_tuples);


move these two functions into updatePortOperStatus function instead of here.
under the two lines:

updateDbPortOperStatus(port, status); port.m_oper_status = status;

lguohan · 2019-07-26T07:27:33Z

orchagent/portsorch.cpp

@@ -2500,6 +2559,8 @@ bool PortsOrch::initializePort(Port &port)
    {
        port.m_oper_status = SAI_PORT_OPER_STATUS_DOWN;
    }
+    m_portTable->hset(port.m_alias, "flap_counter", "0");
+    m_portTable->hset(port.m_alias, "last_flap", "N/A");


should go to state db?

lguohan · 2019-07-26T07:31:29Z

orchagent/portsorch.cpp

@@ -1239,14 +1241,71 @@ bool PortsOrch::setHostIntfsOperStatus(const Port& port, bool isUp) const
    return true;
 }

+void PortsOrch::updateDbPortFlapCounter(const Port& port, vector<FieldValueTuple>& old_tuples, vector<FieldValueTuple>& new_tuples) const
+{
+  SWSS_LOG_ENTER();


lguohan

use redis incr and format your changes.

stcheng · 2019-07-26T21:17:26Z

redis incr is nice but it requires much more changes. i suggest a design review first

lguohan · 2019-07-26T21:29:38Z

how to move forward on this one? schedule a call to discuss this feature? @prprakas to schedule?

prprakas · 2019-07-29T06:32:37Z

@lguohan @stcheng Yeah we can schedule a call to discuss on this. I am based out of Bangalore, India so let me know what is the good time to talk to you

prprakas · 2019-08-08T16:32:29Z

@lguohan will add a vs-test for this, I think that should be good to move ahead with this as per the communication from @zhenggen-xu

lguohan · 2019-09-06T22:18:20Z

@prprakas , can you fix simply comments we have requested, also can you share the ETA for adding vs test?

- Added natmgr and natorch changes. - Added nat Zone related changes. - Added Warm reboot changes. Signed-off-by: Akhilesh Samineni <akhilesh.samineni@broadcom.com>

Signed-off-by: Akhilesh Samineni <akhilesh.samineni@broadcom.com>

Signed-off-by: Akhilesh Samineni akhilesh.samineni@broadcom.com

… as part of PRs sonic-net#1125 and sonic-net#1126

Signed-off-by: Akhilesh Samineni <akhilesh.samineni@broadcom.com>

…n_sonic Natmgrd changes in sonic-swss sub module to support NAT feature.

…sonic-net#1156) * [aclorch] Validate that provided IN/OUT_PORTS are physical interfaces - Add a check during the rule validation step to make sure provided IN/OUT_PORTS rules only include physical interfaces - Add vs test cases for invalid IN/OUT_PORTS inputs Signed-off-by: Danny Allen <daall@microsoft.com> * Clarify test names * Add extra delay for table teardown * Check if new tests are causing tests to fail * Check if new tests are causing tests to fail * Check if ProducerStateTable is having problems * Make delete ACL table test more strict * Remove invalid interface test cases * Undo change to delete test * Undo change to delete test

…ges_in_sonic Natsyncd changes in sonic-swss submodule to support NAT feature.

…es_in_sonic Orchagent changes in sonic-swss submodule to support NAT feature.

…t and queue stats (sonic-net#1170) - Updates portsorch to use FlexCounterManager for port and queue stats instead of direct redis operations - Updates FlexCounterManager to allow clients to enable the group in the constructor, for convenience - Updates the makefile for cfgmgr to include new flex_counter dependency from portsorch Signed-off-by: Danny Allen <daall@microsoft.com>

Signed-off-by: Danny Allen <daall@microsoft.com>

…asons (sonic-net#1173) - Refactor drop reason capability query to trim SAI prefixes - Store device capabilities in orchagent to perform safety checks Fixes sonic-net#1136 - Rather than depending on each ASIC vendor to follow the same error handling doctrine, this PR validates HW support in orchagent, which should be more reliable. Related to sonic-net/sonic-utilities#785 - In order to validate user input, we need to remove the SAI prefixes before we store the results. This removes the need for the CLI to perform these checks. Signed-off-by: Danny Allen <daall@microsoft.com>

…#1141) Clear loopback interfaces in kernel at intfmgrd starting except warmstart. Clear vrf interfaces in kernel at vrfmgrd starting except warmstart.

…warm reboot" issue (sonic-net#1171) * during warm-reboot teamd need to use same system-id before warm-reboot Signed-off-by: shine.chen <shine.chen@nephosinc.com> * refine per review comment Signed-off-by: shine.chen <shine.chen@mediatek.com> * skip saved pdu on negotiation stage Signed-off-by: shine.chen <shine.chen@mediatek.com> * reformat teammgr file

Signed-off-by: Danny Allen <daall@microsoft.com>

…onic-net#1158) Otherwise, we can see that DUT after warm reboot sends ARP packets with management IP (10.112.71.4) Signed-off-by: Stepan Blyschak <stepanb@mellanox.com>

…onic-net#1166) Originally portsorch was designed to remove LAG member from LAG when member gets disabled by teamd. This could lead to potential issues including flood to that port and loops, since after removal member becomes a switch port. Now, portsorch will make use of SAI_LAG_MEMBER_ATTR_INGRESS_DISABLE and SAI_LAG_MEMBER_ATTR_EGRESS_DISABLE to control collection/distribution through that LAG member port. With this new flow, teammgrd and teamsyncd are candidates to be refactored, especially teamsyncd warm reboot logic, since now we don't need to compare old, new lags and lag members. Teamsyncd's job is simply to signal "status" field update without waiting for reconciliation timer to expire. e.g. one port channel went down on peer: ``` admin@arc-switch1025:~$ show interfaces portchannel Flags: A - active, I - inactive, Up - up, Dw - Down, N/A - not available, S - selected, D - deselected, * - not synced No. Team Dev Protocol Ports ----- --------------- ----------- -------------- 0001 PortChannel0001 LACP(A)(Up) Ethernet112(S) 0002 PortChannel0002 LACP(A)(Up) Ethernet116(S) 0003 PortChannel0003 LACP(A)(Up) Ethernet120(S) 0004 PortChannel0004 LACP(A)(Dw) Ethernet124(D) admin@arc-switch1025:~$ docker exec -it syncd sx_api_lag_dump.py LAG Members Table =============================================================================================================== | SWID| LAG Logical Port| LAG Oper State| PVID| Member Port ID| Port Label| Collector| Distributor| Oper State| =============================================================================================================== | 0 0x10000000 UP 1| 0x11900| 29| Enable| Enable| UP| =============================================================================================================== | 0 0x10000100 UP 1| 0x11b00| 30| Enable| Enable| UP| =============================================================================================================== | 0 0x10000200 UP 1| 0x11d00| 31| Enable| Enable| UP| =============================================================================================================== | 0 0x10000300 DOWN 1| 0x11f00| 32| Disable| Disable| UP| =============================================================================================================== ``` Signed-off-by: Stepan Blyschak <stepanb@mellanox.com>

…ite (sonic-net#1185) Signed-off-by: Danny Allen <daall@microsoft.com>

InitSystemAcl table is removed. ECN ACL rules: DSCP 0/8 mapping to yellow are removed. Signed-off-by: Zhenggen Xu <zxu@linkedin.com>

lguohan · 2020-01-29T05:51:09Z

retest this please

Added flap counter, last flap time and clear the flap count feature

fe3b4fc

prprakas mentioned this pull request Jun 12, 2019

[sonic-utilities] : Refactoring interfaces status to interfaces status brief/details sonic-net/sonic-utilities#552

Open

zhenggen-xu requested a review from stcheng June 28, 2019 05:15

stcheng added the Enhancement ➕ label Jul 8, 2019

stcheng reviewed Jul 8, 2019

View reviewed changes

stcheng suggested changes Jul 8, 2019

View reviewed changes

lguohan reviewed Jul 26, 2019

View reviewed changes

lguohan requested changes Jul 26, 2019

View reviewed changes

qiluo-msft assigned lguohan Nov 20, 2019

AkhileshSamineni added 6 commits December 10, 2019 00:47

Changes in sonic-swss submodule to support NAT feature.

4e0d0a1

- Added natmgr and natorch changes. - Added nat Zone related changes. - Added Warm reboot changes. Signed-off-by: Akhilesh Samineni <akhilesh.samineni@broadcom.com>

Added neighsync changes to support warm reboot for NAT.

29bb539

Signed-off-by: Akhilesh Samineni <akhilesh.samineni@broadcom.com>

Loopback related changes for NAT.

b53c888

Signed-off-by: Akhilesh Samineni akhilesh.samineni@broadcom.com

Removed the Orchagent and Natsyncd related files as those are covered…

fd733b8

… as part of PRs sonic-net#1125 and sonic-net#1126

Removed pytest related files and added those to PR sonic-net#1125

5b09c41

Signed-off-by: Akhilesh Samineni <akhilesh.samineni@broadcom.com>

Corrected Makefile.am change.

904e80b

Signed-off-by: Akhilesh Samineni <akhilesh.samineni@broadcom.com>

AkhileshSamineni and others added 25 commits December 10, 2019 07:28

Addressed review comments.

f9a8302

Signed-off-by: Akhilesh Samineni <akhilesh.samineni@broadcom.com>

Added platform changes to avoid nexthop traching in OA.

63ebb3a

Signed-off-by: Akhilesh Samineni <akhilesh.samineni@broadcom.com>

Added pytest testcases for NAT.

3350b85

Signed-off-by: Akhilesh Samineni <akhilesh.samineni@broadcom.com>

Fixed compilation issue.

286ca21

Signed-off-by: Akhilesh Samineni <akhilesh.samineni@broadcom.com>

Fixed compilation issue after forced push.

982abd6

Signed-off-by: Akhilesh Samineni <akhilesh.samineni@broadcom.com>

Merge branch 'master' into natorch_changes_in_sonic

5ad47af

Merge pull request sonic-net#1059 from AkhileshSamineni/nat_changes_i…

85036df

…n_sonic Natmgrd changes in sonic-swss sub module to support NAT feature.

[AclOrch] add validation for check CRM (sonic-net#1082)

f24081b

Fix bug: shellquote the inner string (sonic-net#1168)

419d9d2

Merge pull request sonic-net#1126 from AkhileshSamineni/natsyncd_chan…

bbd77fc

…ges_in_sonic Natsyncd changes in sonic-swss submodule to support NAT feature.

Merge pull request sonic-net#1125 from AkhileshSamineni/natorch_chang…

ea8b1da

…es_in_sonic Orchagent changes in sonic-swss submodule to support NAT feature.

[vs tests] Disable NAT tests to unblock Jenkins (sonic-net#1179)

10f091d

Signed-off-by: Danny Allen <daall@microsoft.com>

[cfgmgr] clear loopback and vrf in kernel if not warmstart (sonic-net…

3732e74

…#1141) Clear loopback interfaces in kernel at intfmgrd starting except warmstart. Clear vrf interfaces in kernel at vrfmgrd starting except warmstart.

[vs tests] Mark VLAN and warm reboot tests as xfail (sonic-net#1183)

97a22ca

Signed-off-by: Danny Allen <daall@microsoft.com>

[restore_neighbors.py] build arp packet with correct hwsrc and psrc (s…

97abec5

…onic-net#1158) Otherwise, we can see that DUT after warm reboot sends ARP packets with management IP (10.112.71.4) Signed-off-by: Stepan Blyschak <stepanb@mellanox.com>

remove the obsoleted platforms build links (sonic-net#1169)

faab3e1

Enable MIRRORv6 for BFN platform (sonic-net#1177)

9594de0

[vs tests] Refactor fdb_update tests to be consistent with vs test su…

4628a5a

…ite (sonic-net#1185) Signed-off-by: Danny Allen <daall@microsoft.com>

[qosorch]: Remove Init Color ACLs (sonic-net#1084)

55b7a1b

InitSystemAcl table is removed. ECN ACL rules: DSCP 0/8 mapping to yellow are removed. Signed-off-by: Zhenggen Xu <zxu@linkedin.com>

Merge branch 'master' into feature_1

24245c0

lguohan force-pushed the master branch from 999d5db to 750982e Compare January 30, 2020 23:08

prsunny requested review from neethajohn, judyjoseph and prsunny as code owners September 2, 2022 23:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[sonic-swss]: Added flap counter and last flap time functionality. #928

[sonic-swss]: Added flap counter and last flap time functionality. #928

prprakas commented Jun 12, 2019

msftclas commented Jun 12, 2019 •

edited

Loading

zhenggen-xu commented Jun 13, 2019

prprakas commented Jun 13, 2019

prprakas commented Jun 13, 2019

prprakas commented Jun 13, 2019

prprakas commented Jun 14, 2019

prprakas commented Jun 19, 2019

prprakas commented Jun 26, 2019

stcheng left a comment

stcheng left a comment

stcheng Jul 8, 2019

stcheng Jul 8, 2019

lguohan Jul 26, 2019

lguohan Jul 26, 2019

lguohan left a comment

stcheng commented Jul 26, 2019

lguohan commented Jul 26, 2019

prprakas commented Jul 29, 2019

prprakas commented Aug 8, 2019

lguohan commented Sep 6, 2019

lguohan commented Jan 29, 2020

[sonic-swss]: Added flap counter and last flap time functionality. #928

Are you sure you want to change the base?

[sonic-swss]: Added flap counter and last flap time functionality. #928

Conversation

prprakas commented Jun 12, 2019

msftclas commented Jun 12, 2019 • edited Loading

zhenggen-xu commented Jun 13, 2019

prprakas commented Jun 13, 2019

prprakas commented Jun 13, 2019

prprakas commented Jun 13, 2019

prprakas commented Jun 14, 2019

prprakas commented Jun 19, 2019

prprakas commented Jun 26, 2019

stcheng left a comment

Choose a reason for hiding this comment

stcheng left a comment

Choose a reason for hiding this comment

stcheng Jul 8, 2019

Choose a reason for hiding this comment

stcheng Jul 8, 2019

Choose a reason for hiding this comment

lguohan Jul 26, 2019

Choose a reason for hiding this comment

lguohan Jul 26, 2019

Choose a reason for hiding this comment

lguohan left a comment

Choose a reason for hiding this comment

stcheng commented Jul 26, 2019

lguohan commented Jul 26, 2019

prprakas commented Jul 29, 2019

prprakas commented Aug 8, 2019

lguohan commented Sep 6, 2019

lguohan commented Jan 29, 2020

msftclas commented Jun 12, 2019 •

edited

Loading