Fix /proc/net/route requirement that causes errors on FreeBSD #1638
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,9 +16,13 @@ | |
| # | ||
| # Requires Python 2.6+ and Openssl 1.0+ | ||
|
|
||
| import socket | ||
| import struct | ||
| import binascii | ||
| import azurelinuxagent.common.utils.fileutil as fileutil | ||
| import azurelinuxagent.common.utils.shellutil as shellutil | ||
| import azurelinuxagent.common.utils.textutil as textutil | ||
| from azurelinuxagent.common.utils.networkutil import RouteEntry | ||
| import azurelinuxagent.common.logger as logger | ||
| from azurelinuxagent.common.exception import OSUtilError | ||
| from azurelinuxagent.common.osutil.default import DefaultOSUtil | ||
|
|
@@ -93,6 +97,304 @@ def get_if_mac(self, ifname): | |
| def get_first_if(self): | ||
| return self._get_net_info()[:2] | ||
|
|
||
| @staticmethod | ||
| def read_route_table(): | ||
| """ | ||
| Return a list of strings comprising the route table as in the Linux /proc/net/route format. The input taken is from FreeBSDs | ||
| `netstat -rn -f inet` command. Here is what the function does in detail: | ||
|
|
||
| 1. Runs `netstat -rn -f inet` which outputs a column formatted list of ipv4 routes in priority order like so: | ||
|
|
||
| > Routing tables | ||
| > | ||
| > Internet: | ||
| > Destination Gateway Flags Refs Use Netif Expire | ||
| > default 61.221.xx.yy UGS 0 247 em1 | ||
| > 10 10.10.110.5 UGS 0 50 em0 | ||
| > 10.10.110/26 link#1 UC 0 0 em0 | ||
| > 10.10.110.5 00:1b:0d:e6:58:40 UHLW 2 0 em0 1145 | ||
| > 61.221.xx.yy/29 link#2 UC 0 0 em1 | ||
| > 61.221.xx.yy 00:1b:0d:e6:57:c0 UHLW 2 0 em1 1055 | ||
| > 61.221.xx/24 link#2 UC 0 0 em1 | ||
| > 127.0.0.1 127.0.0.1 UH 0 0 lo0 | ||
|
|
||
| 2. Convert it to an array of lines that resemble an equivalent /proc/net/route content on a Linux system like so: | ||
|
|
||
| > Iface Destination Gateway Flags RefCnt Use Metric Mask MTU Window IRTT | ||
| > gre828 00000000 00000000 0001 0 0 0 000000F8 0 0 0 | ||
| > ens160 00000000 FE04700A 0003 0 0 100 00000000 0 0 0 | ||
| > gre828 00000008 00000000 0001 0 0 0 000000FE 0 0 0 | ||
| > ens160 0004700A 00000000 0001 0 0 100 00FFFFFF 0 0 0 | ||
| > gre828 2504700A 00000000 0005 0 0 0 FFFFFFFF 0 0 0 | ||
| > gre828 3704700A 00000000 0005 0 0 0 FFFFFFFF 0 0 0 | ||
| > gre828 4104700A 00000000 0005 0 0 0 FFFFFFFF 0 0 0 | ||
|
|
||
| :return: Entries in the ipv4 route priority list from `netstat -rn -f inet` in the linux `/proc/net/route` style | ||
| :rtype: list(str) | ||
| """ | ||
|
There was a problem hiding this comment. Could you please add an example of how the There was a problem hiding this comment. Exmples added as requested in 10819af |
||
|
|
||
| def _get_netstat_rn_ipv4_routes(): | ||
| """ | ||
| Runs `netstat -rn -f inet` and parses its output and returns a list of routes where the key is the column name | ||
| and the value is the value in the column, stripped of leading and trailing whitespace. | ||
|
|
||
| :return: List of dictionaries representing routes in the ipv4 route priority list from `netstat -rn -f inet` | ||
| :rtype: list(dict) | ||
| """ | ||
| cmd = "netstat -rn -f inet" | ||
| output = shellutil.run_command(cmd, log_error=True) | ||
|
There was a problem hiding this comment. thank you for the update. run_command sets shell=False when invoking subprocess.Popen, so the command needs to be passed as an array of strings; see, for example https://github.com/Azure/WALinuxAgent/blob/develop/azurelinuxagent/common/utils/cryptutil.py#L55 Also, please be sure to tests this end-to-end on your side. Our test infrastructure does not validate FreeBSD. Thanks There was a problem hiding this comment. Fixed in 08ba686 Tested in FreeBSD 11.2 on Azure: Sorry about that. |
||
| output_lines = output.split("\n") | ||
| if len(output_lines) < 3: | ||
| raise OSUtilError("`netstat -rn -f inet` output seems to be empty") | ||
| output_lines = [ line.strip() for line in output_lines if line ] | ||
| if "Internet:" not in output_lines: | ||
| raise OSUtilError("`netstat -rn -f inet` output seems to contain no ipv4 routes") | ||
| route_header_line = output_lines.index("Internet:") + 1 | ||
| # Parse the file structure and left justify the routes | ||
| route_start_line = route_header_line + 1 | ||
| route_line_length = max([len(line) for line in output_lines[route_header_line:]]) | ||
| netstat_route_list = [line.ljust(route_line_length) for line in output_lines[route_start_line:]] | ||
| # Parse the headers | ||
| _route_headers = output_lines[route_header_line].split() | ||
| n_route_headers = len(_route_headers) | ||
| route_columns = {} | ||
| for i in range(0, n_route_headers - 1): | ||
| route_columns[_route_headers[i]] = ( | ||
| output_lines[route_header_line].index(_route_headers[i]), | ||
| (output_lines[route_header_line].index(_route_headers[i+1]) - 1) | ||
| ) | ||
| route_columns[_route_headers[n_route_headers - 1]] = ( | ||
| output_lines[route_header_line].index(_route_headers[n_route_headers - 1]), | ||
| None | ||
| ) | ||
| # Parse the routes | ||
| netstat_routes = [] | ||
| n_netstat_routes = len(netstat_route_list) | ||
| for i in range(0, n_netstat_routes): | ||
| netstat_route = {} | ||
| for column in route_columns: | ||
| netstat_route[column] = netstat_route_list[i][route_columns[column][0]:route_columns[column][1]].strip() | ||
| netstat_route["Metric"] = n_netstat_routes - i | ||
| netstat_routes.append(netstat_route) | ||
| # Return the Sections | ||
| return netstat_routes | ||
|
|
||
| def _ipv4_ascii_address_to_hex(ipv4_ascii_address): | ||
| """ | ||
| Converts an IPv4 32bit address from its ASCII notation (ie. 127.0.0.1) to an 8 digit padded hex notation | ||
| (ie. "0100007F") string. | ||
|
|
||
| :return: 8 character long hex string representation of the IP | ||
| :rtype: string | ||
| """ | ||
| # Raises socket.error if the IP is not a valid IPv4 | ||
| return "%08X" % int(binascii.hexlify(struct.pack("!I", struct.unpack("=I", socket.inet_pton(socket.AF_INET, ipv4_ascii_address))[0])), 16) | ||
|
|
||
| def _ipv4_cidr_mask_to_hex(ipv4_cidr_mask): | ||
| """ | ||
| Converts an subnet mask from its CIDR integer notation (ie. 32) to an 8 digit padded hex notation | ||
| (ie. "FFFFFFFF") string representing its bitmask form. | ||
|
|
||
| :return: 8 character long hex string representation of the IP | ||
| :rtype: string | ||
| """ | ||
| return "{0:08x}".format(struct.unpack("=I", struct.pack("!I", (0xffffffff << (32 - ipv4_cidr_mask)) & 0xffffffff))[0]).upper() | ||
|
|
||
| def _ipv4_cidr_destination_to_hex(destination): | ||
| """ | ||
| Converts an destination address from its CIDR notation (ie. 127.0.0.1/32 or default or localhost) to an 8 | ||
| digit padded hex notation (ie. "0100007F" or "00000000" or "0100007F") string and its subnet bitmask | ||
| also in hex (FFFFFFFF). | ||
|
|
||
| :return: tuple of 8 character long hex string representation of the IP and 8 character long hex string representation of the subnet mask | ||
| :rtype: tuple(string, int) | ||
| """ | ||
| destination_ip = "0.0.0.0" | ||
| destination_subnetmask = 32 | ||
| if destination != "default": | ||
| if destination == "localhost": | ||
| destination_ip = "127.0.0.1" | ||
| else: | ||
| destination_ip = destination.split("/") | ||
| if len(destination_ip) > 1: | ||
| destination_subnetmask = int(destination_ip[1]) | ||
| destination_ip = destination_ip[0] | ||
| hex_destination_ip = _ipv4_ascii_address_to_hex(destination_ip) | ||
| hex_destination_subnetmask = _ipv4_cidr_mask_to_hex(destination_subnetmask) | ||
| return hex_destination_ip, hex_destination_subnetmask | ||
|
|
||
| def _try_ipv4_gateway_to_hex(gateway): | ||
| """ | ||
| If the gateway is an IPv4 address, return its IP in hex, else, return "00000000" | ||
|
|
||
| :return: 8 character long hex string representation of the IP of the gateway | ||
| :rtype: string | ||
| """ | ||
| try: | ||
| return _ipv4_ascii_address_to_hex(gateway) | ||
| except socket.error: | ||
| return "00000000" | ||
|
|
||
| def _ascii_route_flags_to_bitmask(ascii_route_flags): | ||
| """ | ||
| Converts route flags to a bitmask of their equivalent linux/route.h values. | ||
|
|
||
| :return: integer representation of a 16 bit mask | ||
| :rtype: int | ||
| """ | ||
| bitmask_flags = 0 | ||
| RTF_UP = 0x0001 | ||
| RTF_GATEWAY = 0x0002 | ||
| RTF_HOST = 0x0004 | ||
| RTF_DYNAMIC = 0x0010 | ||
| if "U" in ascii_route_flags: | ||
| bitmask_flags |= RTF_UP | ||
| if "G" in ascii_route_flags: | ||
| bitmask_flags |= RTF_GATEWAY | ||
| if "H" in ascii_route_flags: | ||
| bitmask_flags |= RTF_HOST | ||
| if "S" not in ascii_route_flags: | ||
| bitmask_flags |= RTF_DYNAMIC | ||
| return bitmask_flags | ||
|
|
||
| def _freebsd_netstat_rn_route_to_linux_proc_net_route(netstat_route): | ||
| """ | ||
| Converts a single FreeBSD `netstat -rn -f inet` route to its equivalent /proc/net/route line. ie: | ||
| > default 0.0.0.0 UGS 0 247 em1 | ||
| to | ||
| > em1 00000000 00000000 0003 0 0 0 FFFFFFFF 0 0 0 | ||
|
|
||
| :return: string representation of the equivalent /proc/net/route line | ||
| :rtype: string | ||
| """ | ||
| network_interface = netstat_route["Netif"] | ||
| hex_destination_ip, hex_destination_subnetmask = _ipv4_cidr_destination_to_hex(netstat_route["Destination"]) | ||
| hex_gateway = _try_ipv4_gateway_to_hex(netstat_route["Gateway"]) | ||
| bitmask_flags = _ascii_route_flags_to_bitmask(netstat_route["Flags"]) | ||
| dummy_refcount = 0 | ||
| dummy_use = 0 | ||
| route_metric = netstat_route["Metric"] | ||
| dummy_mtu = 0 | ||
| dummy_window = 0 | ||
| dummy_irtt = 0 | ||
| return "{0}\t{1}\t{2}\t{3}\t{4}\t{5}\t{6}\t{7}\t{8}\t{9}\t{10}".format( | ||
| network_interface, | ||
| hex_destination_ip, | ||
| hex_gateway, | ||
| bitmask_flags, | ||
| dummy_refcount, | ||
| dummy_use, | ||
| route_metric, | ||
| hex_destination_subnetmask, | ||
| dummy_mtu, | ||
| dummy_window, | ||
| dummy_irtt | ||
| ) | ||
|
|
||
| linux_style_route_file = [ "Iface\tDestination\tGateway\tFlags\tRefCnt\tUse\tMetric\tMask\tMTU\tWindow\tIRTT" ] | ||
|
|
||
| try: | ||
| netstat_routes = _get_netstat_rn_ipv4_routes() | ||
| # Make sure the `netstat -rn -f inet` contains columns for Netif, Destination, Gateway and Flags which are needed to convert | ||
| # to the Linux Format | ||
| if len(netstat_routes) > 0: | ||
| missing_headers = [] | ||
| if "Netif" not in netstat_routes[0]: | ||
| missing_headers.append("Netif") | ||
| if "Destination" not in netstat_routes[0]: | ||
| missing_headers.append("Destination") | ||
| if "Gateway" not in netstat_routes[0]: | ||
| missing_headers.append("Gateway") | ||
| if "Flags" not in netstat_routes[0]: | ||
| missing_headers.append("Flags") | ||
| if missing_headers: | ||
| raise KeyError("`netstat -rn -f inet` output is missing columns required to convert to the Linux /proc/net/route format; columns are [{0}]".format(missing_headers)) | ||
| # Parse the Netstat IPv4 Routes | ||
| for netstat_route in netstat_routes: | ||
| try: | ||
| linux_style_route = _freebsd_netstat_rn_route_to_linux_proc_net_route(netstat_route) | ||
| linux_style_route_file.append(linux_style_route) | ||
| except Exception: | ||
| # Skip the route | ||
| continue | ||
| except Exception as e: | ||
| logger.error("Cannot read route table [{0}]", ustr(e)) | ||
| return linux_style_route_file | ||
|
There was a problem hiding this comment. The original implementation of this function handles all errors in function and doesn't propagate any exception further. try:
with open('/proc/net/route') as routing_table:
return list(map(str.strip, routing_table.readlines()))
except Exception as e:
logger.error("Cannot read route table [{0}]", ustr(e))
return []After you refactor this function, could you please ensure that the There was a problem hiding this comment. done with 515b672 |
||
|
|
||
| @staticmethod | ||
| def get_list_of_routes(route_table): | ||
| """ | ||
| Construct a list of all network routes known to this system. | ||
|
|
||
| :param list(str) route_table: List of text entries from route table, including headers | ||
| :return: a list of network routes | ||
| :rtype: list(RouteEntry) | ||
| """ | ||
| route_list = [] | ||
| count = len(route_table) | ||
|
|
||
| if count < 1: | ||
| logger.error("netstat -rn -f inet is missing headers") | ||
| elif count == 1: | ||
| logger.error("netstat -rn -f inet contains no routes") | ||
| else: | ||
| route_list = DefaultOSUtil._build_route_list(route_table) | ||
| return route_list | ||
|
|
||
| def get_primary_interface(self): | ||
| """ | ||
| Get the name of the primary interface, which is the one with the | ||
| default route attached to it; if there are multiple default routes, | ||
| the primary has the lowest Metric. | ||
| :return: the interface which has the default route | ||
| """ | ||
| RTF_GATEWAY = 0x0002 | ||
| DEFAULT_DEST = "00000000" | ||
|
|
||
| primary_interface = None | ||
|
|
||
| if not self.disable_route_warning: | ||
| logger.info("Examine `netstat -rn -f inet` for primary interface") | ||
|
|
||
| route_table = self.read_route_table() | ||
|
|
||
| def is_default(route): | ||
| return (route.destination == DEFAULT_DEST) and (RTF_GATEWAY & route.flags) | ||
|
|
||
| candidates = list(filter(is_default, self.get_list_of_routes(route_table))) | ||
|
|
||
| if len(candidates) > 0: | ||
| def get_metric(route): | ||
| return int(route.metric) | ||
| primary_route = min(candidates, key=get_metric) | ||
| primary_interface = primary_route.interface | ||
|
|
||
| if primary_interface is None: | ||
| primary_interface = '' | ||
| if not self.disable_route_warning: | ||
| logger.warn('Could not determine primary interface, ' | ||
| 'please ensure routes are correct') | ||
| logger.warn('Primary interface examination will retry silently') | ||
| self.disable_route_warning = True | ||
| else: | ||
| logger.info('Primary interface is [{0}]'.format(primary_interface)) | ||
| self.disable_route_warning = False | ||
| return primary_interface | ||
|
|
||
| def is_primary_interface(self, ifname): | ||
| """ | ||
| Indicate whether the specified interface is the primary. | ||
| :param ifname: the name of the interface - eth0, lo, etc. | ||
| :return: True if this interface binds the default route | ||
| """ | ||
| return self.get_primary_interface() == ifname | ||
|
|
||
| def is_loopback(self, ifname): | ||
| """ | ||
| Determine if a named interface is loopback. | ||
| """ | ||
| return ifname.startswith("lo") | ||
|
|
||
| def route_add(self, net, mask, gateway): | ||
| cmd = 'route add {0} {1} {2}'.format(net, gateway, mask) | ||
| return shellutil.run(cmd, chk_err=False) | ||
|
|
@@ -103,6 +405,14 @@ def is_missing_default_route(self): | |
| specify the route manually to get it work in a VNET environment. | ||
| SEE ALSO: man ip(4) IP_ONESBCAST, | ||
| """ | ||
| RTF_GATEWAY = 0x0002 | ||
| DEFAULT_DEST = "00000000" | ||
|
|
||
| route_table = self.read_route_table() | ||
| routes = self.get_list_of_routes(route_table) | ||
| for route in routes: | ||
| if (route.destination == DEFAULT_DEST) and (RTF_GATEWAY & route.flags): | ||
| return False | ||
| return True | ||
|
|
||
| def is_dhcp_enabled(self): | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the original code there was a check for exists(route_file); if it was false, we would output a warning.
In the new code this would be reported as an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how to work around this. read_route_table will log an error by default and then return an empty list if /proc/net/route doesn't exist.
The only thing it guarantees is not to raise an exception.
Im not sure changing that function would be appropriate. I do think using that function would be appropriate since checking route is clearly a platform specific task.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also do think that this is an edge case and not really an issue as it will only log an error on systems without /proc/net/route (non Linux systems) which have not overridden that function with their own osutil (so now, neither Linux nor FreeBSD).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok