Negative LLDP time #29
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pavel-shirshov
suggested changes
Sep 11, 2020
src/lldp_syncd/daemon.py
Outdated
| @@ -43,9 +43,18 @@ def parse_time(time_str): | |||
| return sage; | |||
| } | |||
| :return: parsed age in time ticks (or seconds) | |||
| 0 in case passed in string has negative time | |||
| due to setting system clock backwards | |||
There was a problem hiding this comment.
Can you please change this to
0 when invalid string was found in time_str
src/lldp_syncd/daemon.py
Outdated
| try: | ||
| struct_time = time.strptime(hour_min_secs, LLDPD_TIME_FORMAT) | ||
| except ValueError: | ||
| logger.warning("Incorrect time {}, did you change system time?".format(hour_min_secs)) |
There was a problem hiding this comment.
I'd change the message to "Invalid time '{}'"
There was a problem hiding this comment.
Sure, but I would like to to keep the possible cause of system time modification as that will be a pretty good hint to the debugger. Please suggest
There was a problem hiding this comment.
But I can change "did you change system time?", to "possibly system time changed?"
pavel-shirshov
approved these changes
Sep 14, 2020
|
/azp run Azure.sonic-dbsyncd |
|
Commenter does not have sufficient privileges for PR 29 in repo sonic-net/sonic-dbsyncd |
|
/azpw run Azure.sonic-dbsyncd |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If the passed in time string is negative, return 0 as delta time in lldp_syncd daemon.
This can happen when the system time is set to backwards.
If not handled the time.strptime() throws valueError exception
Traceback before the fix:
#12 -- #012Traceback (most recent call last):#12
File "/usr/local/lib/python2.7/dist-packages/lldp_syncd/daemon.py", line 249, in parse_update#012 str(parse_time(if_attributes.get('age')))})#12
File "/usr/local/lib/python2.7/dist-packages/lldp_syncd/daemon.py", line 45, in parse_time#012 struct_time = time.strptime(hour_min_secs, LLDPD_TIME_FORMAT)#12
File "/usr/lib/python2.7/_strptime.py", line 467, in _strptime_time#012 return _strptime(data_string, format)[0]#12
File "/usr/lib/python2.7/_strptime.py", line 325, in _strptime#012 (data_string, format))#012ValueError: time data u'-23:-41:-48' does not match format '%H:%M:%S'
Test failed before fix:
======================================================================
FAIL: test_timeparse (tests.test_lldpSyncDaemon.TestLldpSyncDaemon)
Traceback (most recent call last):
File "/sonic/src/sonic-dbsyncd/tests/test_lldpSyncDaemon.py", line 99, in test_timeparse
self.assertEquals(lldp_syncd.daemon.parse_time("2 days, -05:59:02"), make_seconds(2, 5, 59, 2))
AssertionError: 0 != 194342
After Fix:
Sep 10 01:45:45.000286 INFO systemd[1]: Time has been changed
Sep 10 01:45:45.001181 INFO systemd[1]: apt-daily.timer: Adding 3h 53min 53.818632s random time.
Sep 10 01:45:45.001855 INFO systemd[1]: apt-daily-upgrade.timer: Adding 3min 14.136467s random time.
Sep 10 01:45:45.178592 WARNING lldp#lldp-syncd [lldp_syncd] WARNING: Incorrect time -1:-44:-21, may be due to system time change?
Sep 10 02:30:35.864675 INFO systemd[1]: Time has been changed
Sep 10 02:30:35.865502 INFO systemd[1]: apt-daily.timer: Adding 10h 45min 44.371210s random time.
Sep 10 02:30:35.866817 INFO systemd[1]: apt-daily-upgrade.timer: Adding 38min 17.812520s random time.
Sep 10 02:30:37.251209 NOTICE syncd#syncd: :- threadFunction: span < 0 = -785 at 207544241993
Sep 10 02:30:37.251209 NOTICE syncd#syncd: :- threadFunction: new span = 635
Sep 10 02:30:40.054358 WARNING lldp#lldp-syncd [lldp_syncd] WARNING: Incorrect time 00:-59:-26, may be due to system time change?
Sep 10 02:55:35.866298 INFO systemd[1]: Time has been changed
Sep 10 02:55:35.867502 INFO systemd[1]: apt-daily.timer: Adding 1h 25min 48.243993s random time.
Sep 10 02:55:35.868579 INFO systemd[1]: apt-daily-upgrade.timer: Adding 39min 31.599616s random time.
Sep 10 02:55:37.226104 NOTICE syncd#syncd: :- threadFunction: span < 0 = -633 at 207554243085
Sep 10 02:55:37.226104 NOTICE syncd#syncd: :- threadFunction: new span = 352
Sep 10 02:55:40.064093 WARNING lldp#lldp-syncd [lldp_syncd] WARNING: Incorrect time 00:-34:-26, may be due to system time change?
Sep 10 03:10:35.864580 INFO systemd[1]: Time has been changed
Sep 10 03:10:35.865445 INFO systemd[1]: apt-daily.timer: Adding 10h 52min 1.501755s random time.
Sep 10 03:10:35.867558 INFO systemd[1]: apt-daily-upgrade.timer: Adding 17.350214s random time.
Test pass after fix:
vapatil@513367bce931:/sonic/src/sonic-dbsyncd/tests$ pytest -v
================================================================================= test session starts =================================================================================
platform linux2 -- Python 2.7.13, pytest-3.0.6, py-1.4.32, pluggy-0.4.0 -- /usr/bin/python
cachedir: ../.cache
rootdir: /sonic/src/sonic-dbsyncd, inifile:
plugins: cov-2.4.0
collected 9 items
test_lldpSyncDaemon.py::TestLldpSyncDaemon::test_loc_chassis PASSED
test_lldpSyncDaemon.py::TestLldpSyncDaemon::test_multiple_mgmt_ip PASSED
test_lldpSyncDaemon.py::TestLldpSyncDaemon::test_parse_json PASSED
test_lldpSyncDaemon.py::TestLldpSyncDaemon::test_parse_short PASSED
test_lldpSyncDaemon.py::TestLldpSyncDaemon::test_parse_short_short PASSED
test_lldpSyncDaemon.py::TestLldpSyncDaemon::test_remote_sys_capability_list PASSED
test_lldpSyncDaemon.py::TestLldpSyncDaemon::test_single_mgmt_ip PASSED
test_lldpSyncDaemon.py::TestLldpSyncDaemon::test_sync_roundtrip PASSED
test_lldpSyncDaemon.py::TestLldpSyncDaemon::test_timeparse PASSED
============================================================================== 9 passed in 0.25 seconds ===============================================================================
vapatil@513367bce931:/sonic/src/sonic-dbsyncd/tests$ pytest