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.

Sign up for GitHub

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

Jump to bottom

Fix FdbUpdater crash when SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID attribute missing. #286

Merged
merged 8 commits into from
Aug 30, 2023
Merged

Fix FdbUpdater crash when SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID attribute missing. #286

merged 8 commits into from
Aug 30, 2023

Conversation

liuh-80
Copy link
Contributor

@liuh-80 liuh-80 commented Jul 21, 2023 •
edited
Loading

Fix FdbUpdater crash when SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID attribute missing.

Work item tracking

Microsoft ADO (number only): 16189251

- What I did
Fix FdbUpdater crash when SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID attribute missing.

- How I did it
Add try-catch block in rfc3463 FdbUpdater

- How to verify it
Manually test.
Pass all UT

- Description for the changelog
Fix FdbUpdater crash when SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID attribute missing.

@liuh-80
Copy link
Contributor Author

liuh-80 commented Jul 21, 2023

Agent no disk space, close re-open to run validation again.

@liuh-80 liuh-80 closed this Jul 21, 2023
@liuh-80 liuh-80 reopened this Jul 21, 2023
@liuh-80
Copy link
Contributor Author

liuh-80 commented Jul 21, 2023

PR validation failed because pipeline trying to download 17GB data to agent, and most agent does not have enough space:

Minimatch patterns: [**]
Filtered 1867 files from the Minimatch filters supplied.
Downloaded 0.0 MB out of 17,203.8 MB (0%).

@liuh-80 liuh-80 closed this Jul 24, 2023
@liuh-80 liuh-80 reopened this Jul 24, 2023
@liuh-80
Copy link
Contributor Author

liuh-80 commented Jul 24, 2023

Pipeline issue fixed by this PR: #287

@liuh-80 liuh-80 requested a review from qiluo-msft July 24, 2023 05:48
qiluo-msft
qiluo-msft reviewed Aug 1, 2023
View reviewed changes
src/sonic_ax_impl/mibs/ietf/rfc4363.py
bridge_port_id_attr = ent["SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID"]
except KeyError as e:
mibs.logger.error("SyncD 'ASIC_DB' includes invalid FDB_ENTRY '{}': failed to get bridge_port_id, exception: {}".format(fdb_str, e))
continue
Copy link
Contributor

@qiluo-msft qiluo-msft Aug 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

continue

If we continue and it will the error message will repeat. In a real device, how frequent is the same error messages? do we need to damp the speed?

Original code will throw a stack in syslog, but not very frequent. #Closed

Copy link
Contributor Author

@liuh-80 liuh-80 Aug 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The log will repeat every 5 seconds, seems not necessary to add more delay:

in https://github.com/sonic-net/sonic-snmpagent/blob/master/src/sonic_ax_impl/main.py

# Background task update frequency ( in seconds )
DEFAULT_UPDATE_FREQUENCY = 5

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, every FDB only output error log once.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the fix! Further suggestion: flush broken_fdbs in reinit_data() in order to prevent it increasing size forever in case the FDB entries become crazy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

qiluo-msft
qiluo-msft reviewed Aug 29, 2023
View reviewed changes
src/sonic_ax_impl/mibs/ietf/rfc4363.py Outdated
except KeyError as e:
# Only write error log once
if fdb_str not in self.broken_fdbs:
mibs.logger.error("SyncD 'ASIC_DB' includes invalid FDB_ENTRY '{}': failed to get bridge_port_id, exception: {}".format(fdb_str, e))
Copy link
Contributor

@qiluo-msft qiluo-msft Aug 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

log at warning level, because we skip it and continue with others. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@qiluo-msft qiluo-msft merged commit af2d5a4 into sonic-net:master Aug 30, 2023
5 checks passed
qiluo-msft pushed a commit that referenced this pull request Sep 4, 2023
@liuh-80 @qiluo-msft
Fix FdbUpdater crash when SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID attribute…
b292c01 
… missing. (#286)

Fix FdbUpdater crash when SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID attribute missing.

#### Work item tracking
Microsoft ADO (number only): 16189251

**- What I did**
Fix FdbUpdater crash when SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID attribute missing.

**- How I did it**
Add try-catch block in rfc3463 FdbUpdater

**- How to verify it**
Manually test.
Pass all UT

**- Description for the changelog**
Fix FdbUpdater crash when SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID attribute missing.
yxieca pushed a commit that referenced this pull request Oct 13, 2023
@liuh-80 @yxieca
Fix FdbUpdater crash when SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID attribute…
09bb0c2 
… missing. (#286)

Fix FdbUpdater crash when SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID attribute missing.

#### Work item tracking
Microsoft ADO (number only): 16189251

**- What I did**
Fix FdbUpdater crash when SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID attribute missing.

**- How I did it**
Add try-catch block in rfc3463 FdbUpdater

**- How to verify it**
Manually test.
Pass all UT

**- Description for the changelog**
Fix FdbUpdater crash when SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID attribute missing.
StormLiangMS pushed a commit that referenced this pull request Oct 17, 2023
@liuh-80 @StormLiangMS
Fix FdbUpdater crash when SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID attribute…
ba7ff87 
… missing. (#286)

Fix FdbUpdater crash when SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID attribute missing.

#### Work item tracking
Microsoft ADO (number only): 16189251

**- What I did**
Fix FdbUpdater crash when SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID attribute missing.

**- How I did it**
Add try-catch block in rfc3463 FdbUpdater

**- How to verify it**
Manually test.
Pass all UT

**- Description for the changelog**
Fix FdbUpdater crash when SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID attribute missing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qiluo-msft qiluo-msft qiluo-msft approved these changes

Assignees
No one assigned
Labels
Included in 202012 Branch Included in 202205 Branch Included in 202305 Branch Request for 202012 Branch Request for 202205 Branch Request for 202305 Branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@liuh-80 @qiluo-msft @yxieca @StormLiangMS