Enhance the speed of 'show arp'#121
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR optimizes the performance of the show arp command by implementing caching mechanisms for BVID-to-VLAN mappings and replacing linear searches with dictionary lookups.
Key Changes:
- Pre-cache all BVID to VLAN ID mappings to avoid repeated database lookups
- Convert bridge MAC list to a dictionary for O(1) lookup performance
- Fix minor spacing issue in JSON parsing
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| bvid_list = set() | ||
| for s in fdb_str: | ||
| fdb_entry = s | ||
| fdb = json.loads(fdb_entry.split(":", 2)[-1]) | ||
| if not fdb: | ||
| continue | ||
| if 'bvid' in fdb: | ||
| bvid_list.add(fdb['bvid']) |
There was a problem hiding this comment.
The FDB entries are now processed twice: once to collect BVIDs (lines 77-84) and once to build the bridge_mac_list (lines 97-128). Consider combining these loops to collect BVIDs and process entries in a single pass, which would be more efficient. You could use bvid_to_vlan.setdefault() or check if a BVID is new before calling get_vlan_id_from_bvid().
Optimize FDB lookup and ARP display performance - Cache all BVID → VLAN mappings in advance to avoid repeated get_vlan_id_from_bvid() calls. - Replace linear search over bridge_mac_list with a dictionary lookup (vlan, mac) → if_name for O(1) performance. - Greatly reduces latency for show arp with large ARP/FDB tables.
link19430
force-pushed
the
Enhance_speed_of_show_arp
branch
from
9d64536 to
71d1ab4
Compare
|
Hi @gord1306 Please help review, thanks. |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| else: | ||
| bvid_to_vlan[bvid] = None | ||
| except Exception: | ||
| bvid_to_vlan[bvid] = bvid |
There was a problem hiding this comment.
When an exception occurs while resolving BVID to VLAN ID, the code stores the raw bvid value in the cache. This could lead to inconsistent behavior because vlan_id is expected to be a VLAN ID integer, but bvid might be in a different format (e.g., it could be an OID string). This will later be used as int(vlan_id) on line 122, which could fail or produce unexpected results. Consider either storing None and continuing (skipping the entry), or ensuring the bvid value is properly converted to an integer format that makes sense as a fallback VLAN ID.
| bvid_to_vlan[bvid] = bvid | |
| bvid_to_vlan[bvid] = None |
What I did
How I did it
How to verify it