Skip to content

Commit

Permalink
Merge pull request #3833 from c-po/wifi-fix
Browse files Browse the repository at this point in the history
wireless: T6597: improve hostapd startup and corresponding smoketests
  • Loading branch information
c-po authored Jul 22, 2024
2 parents 085ca2c + a67f49d commit 13d5350
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 52 deletions.
2 changes: 1 addition & 1 deletion python/vyos/utils/process.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ def check_process(name, cmdline):
if not tmp:
if time.time() > time_expire:
break
time.sleep(0.100) # wait 250ms
time.sleep(0.100) # wait 100ms
continue
return tmp
else:
Expand Down
2 changes: 2 additions & 0 deletions smoketest/scripts/cli/base_vyostest_shim.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ def cli_discard(self):
self._session.discard()

def cli_commit(self):
if self.debug:
print('commit')
self._session.commit()
# during a commit there is a process opening commit_lock, and run() returns 0
while run(f'sudo lsof -nP {commit_lock}') == 0:
Expand Down
52 changes: 31 additions & 21 deletions smoketest/scripts/cli/test_interfaces_wireless.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@
from glob import glob

from vyos.configsession import ConfigSessionError
from vyos.utils.process import process_named_running
from vyos.utils.kernel import check_kmod
from vyos.utils.file import read_file
from vyos.utils.kernel import check_kmod
from vyos.utils.network import interface_exists
from vyos.utils.process import process_named_running
from vyos.utils.process import call
from vyos.xml_ref import default_value

def get_config_value(interface, key):
Expand All @@ -33,7 +35,7 @@ def get_config_value(interface, key):
return tmp[0]

wifi_cc_path = ['system', 'wireless', 'country-code']

country = 'se'
class WirelessInterfaceTest(BasicInterfaceTest.TestCase):
@classmethod
def setUpClass(cls):
Expand Down Expand Up @@ -66,7 +68,8 @@ def setUpClass(cls):
cls._test_ipv6 = False
cls._test_vlan = False

cls.cli_set(cls, wifi_cc_path + ['se'])
cls.cli_set(cls, wifi_cc_path + [country])


def test_wireless_add_single_ip_address(self):
# derived method to check if member interfaces are enslaved properly
Expand All @@ -84,7 +87,7 @@ def test_wireless_add_single_ip_address(self):

def test_wireless_hostapd_config(self):
# Only set the hostapd (access-point) options
interface = 'wlan1'
interface = self._interfaces[1] # wlan1
ssid = 'ssid'

self.cli_set(self._base_path + [interface, 'ssid', ssid])
Expand Down Expand Up @@ -161,7 +164,7 @@ def test_wireless_hostapd_config(self):

def test_wireless_hostapd_vht_mu_beamformer_config(self):
# Multi-User-Beamformer
interface = 'wlan1'
interface = self._interfaces[1] # wlan1
ssid = 'vht_mu-beamformer'
antennas = '3'

Expand Down Expand Up @@ -230,7 +233,7 @@ def test_wireless_hostapd_vht_mu_beamformer_config(self):

def test_wireless_hostapd_vht_su_beamformer_config(self):
# Single-User-Beamformer
interface = 'wlan1'
interface = self._interfaces[1] # wlan1
ssid = 'vht_su-beamformer'
antennas = '3'

Expand Down Expand Up @@ -299,16 +302,14 @@ def test_wireless_hostapd_vht_su_beamformer_config(self):

def test_wireless_hostapd_he_config(self):
# Only set the hostapd (access-point) options - HE mode for 802.11ax at 6GHz
interface = 'wlan1'
interface = self._interfaces[1] # wlan1
ssid = 'ssid'
channel = '1'
sae_pw = 'VyOSVyOSVyOS'
country = 'de'
bss_color = '37'
channel_set_width = '134'
center_channel_freq_1 = '15'

self.cli_set(wifi_cc_path + [country])
self.cli_set(self._base_path + [interface, 'ssid', ssid])
self.cli_set(self._base_path + [interface, 'type', 'access-point'])
self.cli_set(self._base_path + [interface, 'channel', channel])
Expand Down Expand Up @@ -353,10 +354,6 @@ def test_wireless_hostapd_he_config(self):
tmp = get_config_value(interface, 'he_oper_centr_freq_seg0_idx')
self.assertEqual(center_channel_freq_1, tmp)

# Country code
tmp = get_config_value(interface, 'country_code')
self.assertEqual(country.upper(), tmp)

# BSS coloring
tmp = get_config_value(interface, 'he_bss_color')
self.assertEqual(bss_color, tmp)
Expand Down Expand Up @@ -386,15 +383,12 @@ def test_wireless_hostapd_he_config(self):

def test_wireless_hostapd_wpa_config(self):
# Only set the hostapd (access-point) options
interface = 'wlan1'
phy = 'phy0'
interface = self._interfaces[1] # wlan1
ssid = 'VyOS-SMOKETEST'
channel = '1'
wpa_key = 'VyOSVyOSVyOS'
mode = 'n'
country = 'de'

self.cli_set(self._base_path + [interface, 'physical-device', phy])
self.cli_set(self._base_path + [interface, 'type', 'access-point'])
self.cli_set(self._base_path + [interface, 'mode', mode])

Expand Down Expand Up @@ -454,7 +448,7 @@ def test_wireless_hostapd_wpa_config(self):
self.assertTrue(process_named_running('hostapd'))

def test_wireless_access_point_bridge(self):
interface = 'wlan1'
interface = self._interfaces[1] # wlan1
ssid = 'VyOS-Test'
bridge = 'br42477'

Expand Down Expand Up @@ -491,7 +485,7 @@ def test_wireless_access_point_bridge(self):
self.cli_delete(bridge_path)

def test_wireless_security_station_address(self):
interface = 'wlan1'
interface = self._interfaces[1] # wlan1
ssid = 'VyOS-ACL'

hostapd_accept_station_conf = f'/run/hostapd/{interface}_station_accept.conf'
Expand All @@ -511,6 +505,12 @@ def test_wireless_security_station_address(self):

self.cli_commit()

self.assertTrue(interface_exists(interface))
self.assertTrue(os.path.isfile(f'/run/hostapd/{interface}_station_accept.conf'))
self.assertTrue(os.path.isfile(f'/run/hostapd/{interface}_station_deny.conf'))

self.assertTrue(process_named_running('hostapd'))

# in accept mode all addresses are allowed unless specified in the deny list
tmp = get_config_value(interface, 'macaddr_acl')
self.assertEqual(tmp, '0')
Expand All @@ -526,6 +526,11 @@ def test_wireless_security_station_address(self):
# Switch mode accept -> deny
self.cli_set(self._base_path + [interface, 'security', 'station-address', 'mode', 'deny'])
self.cli_commit()

self.assertTrue(interface_exists(interface))
self.assertTrue(os.path.isfile(f'/run/hostapd/{interface}_station_accept.conf'))
self.assertTrue(os.path.isfile(f'/run/hostapd/{interface}_station_deny.conf'))

# In deny mode all addresses are denied unless specified in the allow list
tmp = get_config_value(interface, 'macaddr_acl')
self.assertEqual(tmp, '1')
Expand All @@ -535,4 +540,9 @@ def test_wireless_security_station_address(self):

if __name__ == '__main__':
check_kmod('mac80211_hwsim')
unittest.main(verbosity=2, failfast=True)
# loading the module created two WIFI Interfaces in the background (wlan0 and wlan1)
# remove them to have a clean test start
for interface in ['wlan0', 'wlan1']:
if interface_exists(interface):
call(f'sudo iw dev {interface} del')
unittest.main(verbosity=2)
6 changes: 3 additions & 3 deletions smoketest/scripts/system/test_kernel_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@
import platform
import unittest

kernel = platform.release()
from vyos.utils.kernel import check_kmod

kernel = platform.release()
class TestKernelModules(unittest.TestCase):
""" VyOS makes use of a lot of Kernel drivers, modules and features. The
required modules which are essential for VyOS should be tested that they are
Expand All @@ -35,9 +36,8 @@ def setUpClass(cls):

super(TestKernelModules, cls).setUpClass()
CONFIG = '/proc/config.gz'

if not os.path.isfile(CONFIG):
call('sudo modprobe configs')
check_kmod('configs')

with gzip.open(CONFIG, 'rt') as f:
cls._config_data = f.read()
Expand Down
75 changes: 48 additions & 27 deletions src/conf_mode/interfaces_wireless.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from sys import exit
from re import findall
from netaddr import EUI, mac_unix_expanded
from time import sleep

from vyos.config import Config
from vyos.configdict import get_interface_dict
Expand All @@ -34,6 +35,9 @@
from vyos.utils.dict import dict_search
from vyos.utils.kernel import check_kmod
from vyos.utils.process import call
from vyos.utils.process import is_systemd_service_active
from vyos.utils.process import is_systemd_service_running
from vyos.utils.network import interface_exists
from vyos import ConfigError
from vyos import airbag
airbag.enable()
Expand Down Expand Up @@ -93,6 +97,11 @@ def get_config(config=None):
if wifi.from_defaults(['security', 'wpa']): # if not set by user
del wifi['security']['wpa']

# XXX: Jinja2 can not operate on a dictionary key when it starts of with a number
if '40mhz_incapable' in (dict_search('capabilities.ht', wifi) or []):
wifi['capabilities']['ht']['fourtymhz_incapable'] = wifi['capabilities']['ht']['40mhz_incapable']
del wifi['capabilities']['ht']['40mhz_incapable']

if dict_search('security.wpa', wifi) != None:
wpa_cipher = wifi['security']['wpa'].get('cipher')
wpa_mode = wifi['security']['wpa'].get('mode')
Expand Down Expand Up @@ -120,7 +129,7 @@ def get_config(config=None):
tmp = find_other_stations(conf, base, wifi['ifname'])
if tmp: wifi['station_interfaces'] = tmp

# used in hostapt.conf.j2
# used in hostapd.conf.j2
wifi['hostapd_accept_station_conf'] = hostapd_accept_station_conf.format(**wifi)
wifi['hostapd_deny_station_conf'] = hostapd_deny_station_conf.format(**wifi)

Expand Down Expand Up @@ -232,11 +241,6 @@ def verify(wifi):
def generate(wifi):
interface = wifi['ifname']

# always stop hostapd service first before reconfiguring it
call(f'systemctl stop hostapd@{interface}.service')
# always stop wpa_supplicant service first before reconfiguring it
call(f'systemctl stop wpa_supplicant@{interface}.service')

# Delete config files if interface is removed
if 'deleted' in wifi:
if os.path.isfile(hostapd_conf.format(**wifi)):
Expand Down Expand Up @@ -272,11 +276,6 @@ def generate(wifi):
mac.dialect = mac_unix_expanded
wifi['mac'] = str(mac)

# XXX: Jinja2 can not operate on a dictionary key when it starts of with a number
if '40mhz_incapable' in (dict_search('capabilities.ht', wifi) or []):
wifi['capabilities']['ht']['fourtymhz_incapable'] = wifi['capabilities']['ht']['40mhz_incapable']
del wifi['capabilities']['ht']['40mhz_incapable']

# render appropriate new config files depending on access-point or station mode
if wifi['type'] == 'access-point':
render(hostapd_conf.format(**wifi), 'wifi/hostapd.conf.j2', wifi)
Expand All @@ -290,23 +289,45 @@ def generate(wifi):

def apply(wifi):
interface = wifi['ifname']
# From systemd source code:
# If there's a stop job queued before we enter the DEAD state, we shouldn't act on Restart=,
# in order to not undo what has already been enqueued. */
#
# It was found that calling restart on hostapd will (4 out of 10 cases) deactivate
# the service instead of restarting it, when it was not yet properly stopped
# systemd[1]: [email protected]: Deactivated successfully.
# Thus kill all WIFI service and start them again after it's ensured nothing lives
call(f'systemctl stop hostapd@{interface}.service')
call(f'systemctl stop wpa_supplicant@{interface}.service')

if 'deleted' in wifi:
WiFiIf(interface).remove()
else:
# Finally create the new interface
w = WiFiIf(**wifi)
w.update(wifi)

# Enable/Disable interface - interface is always placed in
# administrative down state in WiFiIf class
if 'disable' not in wifi:
# Physical interface is now configured. Proceed by starting hostapd or
# wpa_supplicant daemon. When type is monitor we can just skip this.
if wifi['type'] == 'access-point':
call(f'systemctl start hostapd@{interface}.service')

elif wifi['type'] == 'station':
call(f'systemctl start wpa_supplicant@{interface}.service')
WiFiIf(**wifi).remove()
return None

while (is_systemd_service_running(f'hostapd@{interface}.service') or \
is_systemd_service_active(f'hostapd@{interface}.service')):
sleep(0.250) # wait 250ms

# Finally create the new interface
w = WiFiIf(**wifi)
w.update(wifi)

# Enable/Disable interface - interface is always placed in
# administrative down state in WiFiIf class
if 'disable' not in wifi:
# Wait until interface was properly added to the Kernel
ii = 0
while not (interface_exists(interface) and ii < 20):
sleep(0.250) # wait 250ms
ii += 1

# Physical interface is now configured. Proceed by starting hostapd or
# wpa_supplicant daemon. When type is monitor we can just skip this.
if wifi['type'] == 'access-point':
call(f'systemctl start hostapd@{interface}.service')

elif wifi['type'] == 'station':
call(f'systemctl start wpa_supplicant@{interface}.service')

return None

Expand Down

0 comments on commit 13d5350

Please sign in to comment.