| Message ID | 1444837400-17309-6-git-send-email-ilan.peer@intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On Thu, Oct 22, 2015 at 02:09:44PM +0000, Peer, Ilan wrote: > > > Support configurations that use a dedicated P2P device interface by > > > setting ext_mgmt_frame_handling and waiting for MGMT-RX event on the > > > global control interface. > Would you prefer to handle this by adding an argument to the mgmt._rx to state if this is P2P frame or not and have the method resolve the interface name internally based on whether P2P Device is used or not (something similar to list_networks()). To be honest, I'm not sure I'd want either and I'm not sure I understand the need to make this specific test case "work" with the where mac80211_hwsim is forced to use a separate P2P Device interface. I would much rather focus on making sure there is sufficient coverage for both options in test cases without relying to running tests with custom configuration that results in this type of issues in the first place. In other words, I'd rather leave this as-is and if something is missing from testing coverage with P2P Device interface, that should addressed by adding a new test case that adds a dynamic hwsim radio with P2P Device interface and then appropriately targeted SET and wait-event commands on the interface that handles P2P messages.
Hi Jouni, > -----Original Message----- > From: Jouni Malinen [mailto:j@w1.fi] > Sent: Saturday, October 31, 2015 23:35 > To: Peer, Ilan > Cc: hostap@lists.infradead.org; Stern, Avraham > Subject: Re: [PATCH 05/14] tests: Change > persistent_group_missed_inv_resp() test > > On Thu, Oct 22, 2015 at 02:09:44PM +0000, Peer, Ilan wrote: > > > > Support configurations that use a dedicated P2P device interface > > > > by setting ext_mgmt_frame_handling and waiting for MGMT-RX event > > > > on the global control interface. > > > Would you prefer to handle this by adding an argument to the mgmt._rx to > state if this is P2P frame or not and have the method resolve the interface > name internally based on whether P2P Device is used or not (something > similar to list_networks()). > > To be honest, I'm not sure I'd want either and I'm not sure I understand the > need to make this specific test case "work" with the where mac80211_hwsim > is forced to use a separate P2P Device interface. I would much rather focus > on making sure there is sufficient coverage for both options in test cases > without relying to running tests with custom configuration that results in this > type of issues in the first place. In other words, I'd rather leave this as-is and if > something is missing from testing coverage with P2P Device interface, that > should addressed by adding a new test case that adds a dynamic hwsim radio > with P2P Device interface and then appropriately targeted SET and wait-event > commands on the interface that handles P2P messages. > We are running the testing framework with support for P2P Device dedicated interface as this is the configuration we are most interested with, so we are trying to modify the framework/tests so they could also be valuable for such configuration. Saying that, I concur with the approach of adding P2P Device interface specific tests when needed, to avoid relying on the custom configurations. Thanks again, Ilan.
diff --git a/tests/hwsim/test_p2p_persistent.py b/tests/hwsim/test_p2p_persistent.py index 2752a88..048a6dc 100644 --- a/tests/hwsim/test_p2p_persistent.py +++ b/tests/hwsim/test_p2p_persistent.py @@ -596,16 +596,16 @@ def test_persistent_group_missed_inv_resp(dev): dev[0].dump_monitor() peer = dev[0].get_peer(addr) # Drop the first Invitation Response frame - if "FAIL" in dev[0].request("SET ext_mgmt_frame_handling 1"): + if "FAIL" in dev[0].global_request("SET ext_mgmt_frame_handling 1"): raise Exception("Failed to enable external management frame handling") cmd = "P2P_INVITE persistent=" + peer['persistent'] + " peer=" + addr dev[0].global_request(cmd) - rx_msg = dev[0].mgmt_rx() + rx_msg = dev[0].mgmt_rx_global() if rx_msg is None: raise Exception("MGMT-RX timeout (no Invitation Response)") time.sleep(2) # Allow following Invitation Response frame to go through - if "FAIL" in dev[0].request("SET ext_mgmt_frame_handling 0"): + if "FAIL" in dev[0].global_request("SET ext_mgmt_frame_handling 0"): raise Exception("Failed to disable external management frame handling") time.sleep(1) # Force the P2P Client side to be on its Listen channel for retry diff --git a/tests/hwsim/wpasupplicant.py b/tests/hwsim/wpasupplicant.py index fded634..5e5b829 100644 --- a/tests/hwsim/wpasupplicant.py +++ b/tests/hwsim/wpasupplicant.py @@ -1072,8 +1072,12 @@ class WpaSupplicant: msg['freq'] = val frame = binascii.unhexlify(items[4]) msg['frame'] = frame + self._mgmt_rx_frame(msg) - hdr = struct.unpack('<HH6B6B6BH', frame[0:24]) + return msg + + def _mgmt_rx_frame(self, msg): + hdr = struct.unpack('<HH6B6B6BH', msg['frame'][0:24]) msg['fc'] = hdr[0] msg['subtype'] = (hdr[0] >> 4) & 0xf hdr = hdr[1:] @@ -1086,7 +1090,21 @@ class WpaSupplicant: msg['bssid'] = "%02x:%02x:%02x:%02x:%02x:%02x" % hdr[0:6] hdr = hdr[6:] msg['seq_ctrl'] = hdr[0] - msg['payload'] = frame[24:] + msg['payload'] = msg['frame'][24:] + + def mgmt_rx_global(self, timeout=5): + ev = self.wait_global_event(["MGMT-RX"], timeout=timeout) + if ev is None: + return None + msg = {} + items = ev.split(' ') + field,val = items[2].split('=') + if field != "freq": + raise Exception("Unexpected global MGMT-RX event format: " + ev) + msg['freq'] = val + frame = binascii.unhexlify(items[5]) + msg['frame'] = frame + self._mgmt_rx_frame(msg) return msg