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.

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

Fix fast-reboot-dump.py ipaddress error #1148

Closed

Conversation

kuanyu99
Copy link
Contributor

@kuanyu99 kuanyu99 commented Oct 6, 2020

- What I did

Fix weird behavior when using ipaddress to parse the ipv4 and ipv6 address.

- How I did it

Instead of using ipaddress.ip_interface(IP_ADDR), I change to use ipaddress.IPv4Address to check if the ip address is ipv4.
The same method is also used inside sonic-utilities/config/nat.py

- How to verify it

When there are some mac learned by SONIC, execute the fast-reboot. It should not return error.

- Previous command output (if the output of a command-line utility has changed)

Sep 28 08:26:16.418276 as8000-3 ERR fast-reboot-dump: Got an exception '192.168.0.208' does not appear to be an IPv4 or IPv6 interface:
Traceback:
 Traceback (most recent call last):
  File "/usr/bin/fast-reboot-dump.py", line 298, in <module>
    res = main()
  File "/usr/bin/fast-reboot-dump.py", line 289, in main
    neighbor_entries = generate_neighbor_entries(root_dir + '/arp.json', all_available_macs)
  File "/usr/bin/fast-reboot-dump.py", line 42, in generate_neighbor_entries
    if ipaddress.ip_interface(ip_addr).ip.version != 4:
  File "/usr/local/lib/python2.7/dist-packages/ipaddress.py", line 239, in ip_interface
    address)
ValueError: '192.168.0.208' does not appear to be an IPv4 or IPv6 interface

- New command output (if the output of a command-line utility has changed)

@kuanyu99
Copy link
Contributor Author

kuanyu99 commented Oct 7, 2020

Retest this please

1 similar comment
@kuanyu99
Copy link
Contributor Author

kuanyu99 commented Oct 7, 2020

Retest this please

@tahmed-dev
Copy link
Contributor

@kuanyu99 Thanks for efforts. the issue could be fixed in Python 2 bu decoding the string as unicode:

print("IP version: %s" % ipaddress.ip_interface('192.168.0.208'.decode()).ip.version)
IP version: 4

In Python 3 this above is not required.

@kuanyu99
Copy link
Contributor Author

kuanyu99 commented Oct 8, 2020

@tahmed-dev Thank you very much! This information help me a lot.

But there are still have other problems.
The ipv6 version fdb items in the redis-db looks like "NEIGH_TABLE:Vlan1000:fe80::XXXXXX". After parsing, the ip_addr equal to "fe80" which will return AddressValueError during calling ipaddress.

@lguohan
Copy link
Contributor

lguohan commented Oct 11, 2020

is there anything missing in the unit test?

@bingwang-ms
Copy link
Contributor

I found the same issue and created another PR #1164 to do a fix. Sorry for not founding this PR. Both PR address the issue, but I think #1164 is more elegant in both python 2 and python 3.

@kuanyu99
Copy link
Contributor Author

I found the same issue and created another PR #1164 to do a fix. Sorry for not founding this PR. Both PR address the issue, but I think #1164 is more elegant in both python 2 and python 3.

No problem. I didn't consider the python3 case. I also think your solution is better.
I will close this ticket once that PR is merged.

@kuanyu99
Copy link
Contributor Author

Due to #1164 has been merged, close this PR.

@kuanyu99 kuanyu99 closed this Oct 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants