Skip to content

Fix IPV6 forced-mgmt-route not work issue #17299

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

Merged
merged 12 commits into from
Jan 22, 2024
Merged

Fix IPV6 forced-mgmt-route not work issue #17299

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
1527252
Fix IPV6 mgmt-vrf not work issue
liuh-80 Nov 27, 2023
fde2d79
Fix UT
liuh-80 Nov 28, 2023
225cd34
Merge remote-tracking branch 'origin' into dev/liuh/fix-ipv6-mgmt-vrf
liuh-80 Dec 6, 2023
6c7be0b
Fix merge issue
liuh-80 Dec 6, 2023
ae4905e
Improve code and test cause for IPV6 address
liuh-80 Dec 6, 2023
269269c
Improve code
liuh-80 Dec 6, 2023
6d1554d
Impove code by PR comments
liuh-80 Jan 3, 2024
4bd7533
Fix address host bit set issue
liuh-80 Jan 4, 2024
3a2adf2
Improve code
liuh-80 Jan 4, 2024
ddc436c
Improve code by PR comments
liuh-80 Jan 8, 2024
ba1352c
Fix PR comments
liuh-80 Jan 10, 2024
dfc96df
Fix PR comments
liuh-80 Jan 19, 2024
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
4 changes: 2 additions & 2 deletions files/image_config/interfaces/interfaces.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ iface {{ name }} {{ 'inet' if prefix | ipv4 else 'inet6' }} static
up ip {{ '-4' if prefix | ipv4 else '-6' }} route add {{ prefix | network }}/{{ prefix | prefixlen }} dev {{ name }} table {{ vrf_table }}
up ip {{ '-4' if prefix | ipv4 else '-6' }} rule add pref {{ force_mgmt_route_priority + 1 }} from {{ prefix | ip }}/{{ '32' if prefix | ipv4 else '128' }} table {{ vrf_table }}
{% for route in MGMT_INTERFACE[(name, prefix)]['forced_mgmt_routes'] %}
up ip rule add pref {{ force_mgmt_route_priority }} to {{ route }} table {{ vrf_table }}
up ip {{ '-4' if prefix | ipv4 else '-6' }} rule add pref {{ force_mgmt_route_priority }} to {{ route }} table {{ vrf_table }}
{% endfor %}
{% if prefix | ipv6 and vrf_table == 'default'%}
# IPV6 default table not add to lookup by default, management server need this to access IPV6 address when BGP shutdown
Expand All @@ -97,7 +97,7 @@ iface {{ name }} {{ 'inet' if prefix | ipv4 else 'inet6' }} static
pre-down ip {{ '-4' if prefix | ipv4 else '-6' }} route delete {{ prefix | network }}/{{ prefix | prefixlen }} dev {{ name }} table {{ vrf_table }}
pre-down ip {{ '-4' if prefix | ipv4 else '-6' }} rule delete pref {{ force_mgmt_route_priority + 1 }} from {{ prefix | ip }}/{{ '32' if prefix | ipv4 else '128' }} table {{ vrf_table }}
{% for route in MGMT_INTERFACE[(name, prefix)]['forced_mgmt_routes'] %}
pre-down ip rule delete pref {{ force_mgmt_route_priority }} to {{ route }} table {{ vrf_table }}
pre-down ip {{ '-4' if route | ipv4 else '-6' }} rule delete pref {{ force_mgmt_route_priority }} to {{ route }} table {{ vrf_table }}
{% endfor %}
{% if prefix | ipv6 and vrf_table == 'default'%}
pre-down ip -6 rule delete pref {{ force_mgmt_route_priority + 3 }} lookup {{ vrf_table }}
Expand Down
30 changes: 28 additions & 2 deletions src/sonic-config-engine/minigraph.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1458,6 +1458,33 @@ def select_mmu_profiles(profile, platform, hwsku):
base_file = os.path.join(path, file_item)
exec_cmd(["sudo", "cp", file_in_dir, base_file])

def is_ipv4(address):
# encode and decode to unicode, because when address is bytes type, ip_network will throw AddressValueError
# set strict to False because address may set host bit, for example 192.168.0.1/24
return isinstance(ipaddress.ip_network(UNICODE_TYPE(address), False), ipaddress.IPv4Network)

def update_forced_mgmt_route(mgmt_intf, mgmt_routes):
for mgmt_intf_key in mgmt_intf.keys():
forced_mgmt_routes = []

try:
# get mgmt interface type
mgmt_intf_addr = mgmt_intf_key[1]
mgmt_is_ipv4 = is_ipv4(mgmt_intf_addr)
Copy link
Collaborator

Choose a reason for hiding this comment

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

changed mgmt_iftype

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, change to compare iftype.


# add mgmt route to different mgmt interface by address type
for mgmt_route in mgmt_routes:
route_is_ipv4 = is_ipv4(mgmt_route)
Copy link
Collaborator

Choose a reason for hiding this comment

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

change to route_iftype

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agree type comparison is more readable than boolean comparison.

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, change to compare iftype.

if mgmt_is_ipv4 == route_is_ipv4:
forced_mgmt_routes.append(mgmt_route)
except ValueError as e:
print("Warning: invalid management routes in minigraph, exception: {}".format(e), file=sys.stderr)
continue

# forced_mgmt_routes yang model not support empty list
if len(forced_mgmt_routes) > 0:
mgmt_intf[mgmt_intf_key]['forced_mgmt_routes'] = forced_mgmt_routes

###############################################################################
#
# Main functions
Expand Down Expand Up @@ -1699,8 +1726,7 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw
results['BGP_VOQ_CHASSIS_NEIGHBOR'] = bgp_voq_chassis_sessions
results['BGP_SENTINELS'] = bgp_sentinel_sessions
if mgmt_routes:
# TODO: differentiate v4 and v6
next(iter(mgmt_intf.values()))['forced_mgmt_routes'] = mgmt_routes
update_forced_mgmt_route(mgmt_intf, mgmt_routes)
results['MGMT_PORT'] = {}
results['MGMT_INTERFACE'] = {}
mgmt_intf_count = 0
Expand Down
10 changes: 6 additions & 4 deletions src/sonic-config-engine/tests/sample_output/py2/mvrf_interfaces
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@ iface eth0 inet static
up ip -4 route add default via 10.0.0.1 dev eth0 table 5000 metric 201
up ip -4 route add 10.0.0.0/24 dev eth0 table 5000
up ip -4 rule add pref 32765 from 10.0.0.100/32 table 5000
up ip rule add pref 32764 to 11.11.11.11 table 5000
up ip rule add pref 32764 to 22.22.22.0/23 table 5000
up ip -4 rule add pref 32764 to 11.11.11.11 table 5000
up ip -4 rule add pref 32764 to 22.22.22.0/23 table 5000
# management port down rules
pre-down ip -4 route delete default via 10.0.0.1 dev eth0 table 5000
pre-down ip -4 route delete 10.0.0.0/24 dev eth0 table 5000
pre-down ip -4 rule delete pref 32765 from 10.0.0.100/32 table 5000
pre-down ip rule delete pref 32764 to 11.11.11.11 table 5000
pre-down ip rule delete pref 32764 to 22.22.22.0/23 table 5000
pre-down ip -4 rule delete pref 32764 to 11.11.11.11 table 5000
pre-down ip -4 rule delete pref 32764 to 22.22.22.0/23 table 5000
iface eth0 inet6 static
address 2603:10e2:0:2902::8
netmask 64
Expand All @@ -53,10 +53,12 @@ iface eth0 inet6 static
up ip -6 route add default via 2603:10e2:0:2902::1 dev eth0 table 5000 metric 201
up ip -6 route add 2603:10e2:0:2902::/64 dev eth0 table 5000
up ip -6 rule add pref 32765 from 2603:10e2:0:2902::8/128 table 5000
up ip -6 rule add pref 32764 to 33:33:33::0/64 table 5000
# management port down rules
pre-down ip -6 route delete default via 2603:10e2:0:2902::1 dev eth0 table 5000
pre-down ip -6 route delete 2603:10e2:0:2902::/64 dev eth0 table 5000
pre-down ip -6 rule delete pref 32765 from 2603:10e2:0:2902::8/128 table 5000
pre-down ip -6 rule delete pref 32764 to 33:33:33::0/64 table 5000
#
source /etc/network/interfaces.d/*
#
Expand Down
10 changes: 6 additions & 4 deletions src/sonic-config-engine/tests/sample_output/py3/mvrf_interfaces
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@ iface eth0 inet static
up ip -4 route add default via 10.0.0.1 dev eth0 table 5000 metric 201
up ip -4 route add 10.0.0.0/24 dev eth0 table 5000
up ip -4 rule add pref 32765 from 10.0.0.100/32 table 5000
up ip rule add pref 32764 to 11.11.11.11 table 5000
up ip rule add pref 32764 to 22.22.22.0/23 table 5000
up ip -4 rule add pref 32764 to 11.11.11.11 table 5000
up ip -4 rule add pref 32764 to 22.22.22.0/23 table 5000
# management port down rules
pre-down ip -4 route delete default via 10.0.0.1 dev eth0 table 5000
pre-down ip -4 route delete 10.0.0.0/24 dev eth0 table 5000
pre-down ip -4 rule delete pref 32765 from 10.0.0.100/32 table 5000
pre-down ip rule delete pref 32764 to 11.11.11.11 table 5000
pre-down ip rule delete pref 32764 to 22.22.22.0/23 table 5000
pre-down ip -4 rule delete pref 32764 to 11.11.11.11 table 5000
pre-down ip -4 rule delete pref 32764 to 22.22.22.0/23 table 5000
Copy link
Collaborator

@lguohan lguohan Dec 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

the title is about ipv6, but new test cases are all ipv4, am i missing something here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will add new test case for IPV6.

For IPV4 address, following 2 commands are exactly same:
up ip rule add pref 32764 to 11.11.11.11 table 5000
up ip -4 rule add pref 32764 to 11.11.11.11 table 5000

But for IPV6 address, we always need add '-6' parameter:
up ip -6 rule add pref 32764 to 2603:10e2:0:2902::8 table 5000

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.

iface eth0 inet6 static
address 2603:10e2:0:2902::8
netmask 64
Expand All @@ -53,10 +53,12 @@ iface eth0 inet6 static
up ip -6 route add default via 2603:10e2:0:2902::1 dev eth0 table 5000 metric 201
up ip -6 route add 2603:10e2:0:2902::/64 dev eth0 table 5000
up ip -6 rule add pref 32765 from 2603:10e2:0:2902::8/128 table 5000
up ip -6 rule add pref 32764 to 33:33:33::0/64 table 5000
# management port down rules
pre-down ip -6 route delete default via 2603:10e2:0:2902::1 dev eth0 table 5000
pre-down ip -6 route delete 2603:10e2:0:2902::/64 dev eth0 table 5000
pre-down ip -6 rule delete pref 32765 from 2603:10e2:0:2902::8/128 table 5000
pre-down ip -6 rule delete pref 32764 to 33:33:33::0/64 table 5000
#
source /etc/network/interfaces.d/*
#
Expand Down
2 changes: 1 addition & 1 deletion src/sonic-config-engine/tests/t0-sample-graph-mvrf.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -787,7 +787,7 @@
<a:DeviceProperty>
<a:Name>ForcedMgmtRoutes</a:Name>
<a:Reference i:nil="true"/>
<a:Value>11.11.11.11;22.22.22.0/23</a:Value>
<a:Value>11.11.11.11;22.22.22.0/23;33:33:33::0/64</a:Value>
</a:DeviceProperty>
<a:DeviceProperty>
<a:Name>ErspanDestinationIpv4</a:Name>
Expand Down