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

[fast-reboot] Fix unicode issue in ipaddress for python2 #1164

Merged
merged 1 commit into from
Oct 14, 2020
Merged

[fast-reboot] Fix unicode issue in ipaddress for python2 #1164

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions scripts/fast-reboot-dump.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import syslog
import traceback
import ipaddress
from builtins import str #for unicode conversion in python2


ARP_CHUNK = binascii.unhexlify('08060001080006040001') # defines a part of the packet for ARP Request
Expand Down Expand Up @@ -39,7 +40,7 @@ def generate_neighbor_entries(filename, all_available_macs):
arp_output.append(obj)

ip_addr = key.split(':')[2]
if ipaddress.ip_interface(ip_addr).ip.version != 4:
if ipaddress.ip_interface(str(ip_addr)).ip.version != 4:
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this fix issue:5580? One comment is that: can you make this Python3 friendly by checking the Python version?

Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, I have a draft PR where I am working on adding support for Python 3. You can look at this PR to see if it will play nice: #1128

Copy link
Contributor Author

@bingwang-ms bingwang-ms Oct 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Would this fix issue:5580? One comment is that: can you make this Python3 friendly by checking the Python version?

Yes. The update is friendly to both python 2 and python 3. I have verified on both python version.

admin@str-dx010-acs-4:~$ python3
Python 3.7.3 (default, Jul 25 2020, 13:03:44) 
[GCC 8.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import ipaddress
>>> from builtins import str
>>> ip_addr = '192.168.0.1'
>>> ipaddress.ip_interface(str(ip_addr)).ip.version
4
>>> ipv6_addr = 'fc00:2::32'
>>> ipaddress.ip_interface(str(ipv6_addr)).ip.version
6
>>> exit()
admin@str-dx010-acs-4:~$ python
Python 2.7.16 (default, Oct 10 2019, 22:02:15) 
[GCC 8.3.0] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import ipaddress
>>> from builtins import str
>>> ip_addr = '192.168.0.1'
>>> ipaddress.ip_interface(str(ip_addr)).ip.version
4
>>> ipv6_addr = 'fc00:2::32'
>>> ipaddress.ip_interface(str(ipv6_addr)).ip.version
6

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @bingwang-ms Can you please have a look at ipv6 splitting function below? it does not look that this would work with longer prefixes. Could be in a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Do you mean this line?
https://github.com/Azure/sonic-utilities/blob/011117bebcf05f6ef17a3d63155dec630ae88cc3/scripts/fast-reboot-dump.py#L45
What's the problem?

Copy link
Contributor

Choose a reason for hiding this comment

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

nevermind. it is splitting the key and recover the Ipv6 from the key and not the ipaddr. it was an oversight.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would this fix issue:5580? One comment is that: can you make this Python3 friendly by checking the Python version?

Sorry for my last unclear reply. The issue sonic-net/sonic-buildimage#5580 is not fixed by this PR. The unicode issue is not found in fast-reboot-dump.py in 201911 branch.

#This is ipv6 address
ip_addr = key.replace(key.split(':')[0] + ':' + key.split(':')[1] + ':', '')
neighbor_entries.append((vlan_name, mac, ip_addr))
Expand Down Expand Up @@ -232,7 +233,7 @@ def send_garp_nd(neighbor_entries, map_mac_ip_per_vlan):
# send arp/ndp packets
for vlan_name, dst_mac, dst_ip in neighbor_entries:
src_if = map_mac_ip_per_vlan[vlan_name][dst_mac]
if ipaddress.ip_interface(dst_ip).ip.version == 4:
if ipaddress.ip_interface(str(dst_ip)).ip.version == 4:
send_arp(sockets[src_if], src_mac_addrs[src_if], src_ip_addrs[vlan_name], dst_mac, dst_ip)
else:
send_ndp(sockets[src_if], src_mac_addrs[src_if], src_ip_addrs[vlan_name], dst_mac, dst_ip)
Expand Down