Skip to content

Commit

Permalink
Fix test_bgp_update_timer failure caused by scapy upgrade in sonic-mg…
Browse files Browse the repository at this point in the history
… docker image (#4774)

What is the motivation for this PR?
The scapy package in sonic-mgmt docker image was upgraded in PR sonic-net/sonic-buildimage#8554.
The new version of scapy uses a different way to represent and dissect BGP messages. This caused the failure of
test_bgp_update_time.py.

How did you do it?
This PR is to address the compatibility issue of scapy. The test_bgp_update_timer.py script was updated to be
be compatible with both old and new scapy versions.

How did you verify/test it?
Run bgp/test_bgp_update_timer.py using both old and new sonic-mgmt docker image.

Signed-off-by: Xin Wang <xiwang5@microsoft.com>
  • Loading branch information
wangxin authored Nov 29, 2021
1 parent 8c29a57 commit c32791f
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 4 deletions.
2 changes: 1 addition & 1 deletion azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ stages:
- job:
pool: sonictest
displayName: "kvmtest-t0"
timeoutInMinutes: 330
timeoutInMinutes: 400

steps:
- template: .azure-pipelines/run-test-template.yml
Expand Down
35 changes: 32 additions & 3 deletions tests/bgp/test_bgp_update_timer.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,12 +152,41 @@ def match_bgp_update(packet, src_ip, dst_ip, action, route):
if not (packet[IP].src == src_ip and packet[IP].dst == dst_ip):
return False
subnet = ipaddress.ip_network(route["prefix"].decode())
_route = (subnet.prefixlen, str(subnet.network_address))

# New scapy (version 2.4.5) uses a different way to represent and dissect BGP messages. Below logic is to
# address the compatibility issue of scapy versions.
if hasattr(bgp, 'BGPNLRI_IPv4'):
_route = bgp.BGPNLRI_IPv4(prefix=str(subnet))
else:
_route = (subnet.prefixlen, str(subnet.network_address))
bgp_fields = packet[bgp.BGPUpdate].fields
if action == "announce":
return bgp_fields["tp_len"] > 0 and _route in bgp_fields["nlri"]
# New scapy (version 2.4.5) uses a different way to represent and dissect BGP messages. Below logic is to
# address the compatibility issue of scapy versions.
path_attr_valid = False
if "tp_len" in bgp_fields:
path_attr_valid = bgp_fields['tp_len'] > 0
elif "path_attr_len" in bgp_fields:
path_attr_valid = bgp_fields["path_attr_len"] > 0
return path_attr_valid and _route in bgp_fields["nlri"]
elif action == "withdraw":
return bgp_fields["withdrawn_len"] > 0 and _route in bgp_fields["withdrawn"]
# New scapy (version 2.4.5) uses a different way to represent and dissect BGP messages. Below logic is to
# address the compatibility issue of scapy versions.
withdrawn_len_valid = False
if "withdrawn_len" in bgp_fields:
withdrawn_len_valid = bgp_fields["withdrawn_len"] > 0
elif "withdrawn_routes_len" in bgp_fields:
withdrawn_len_valid = bgp_fields["withdrawn_routes_len"] > 0

# New scapy (version 2.4.5) uses a different way to represent and dissect BGP messages. Below logic is to
# address the compatibility issue of scapy versions.
withdrawn_route_valid = False
if "withdrawn" in bgp_fields:
withdrawn_route_valid = _route in bgp_fields["withdrawn"]
elif "withdrawn_routes" in bgp_fields:
withdrawn_route_valid = _route in bgp_fields["withdrawn_routes"]

return withdrawn_len_valid and withdrawn_route_valid
else:
return False

Expand Down

0 comments on commit c32791f

Please sign in to comment.