From: Ivo v. D. <iv...@gm...> - 2005-08-03 07:20:22
|
Hi, Recently a update was made in GIT where the payload variable was added to a= ll=20 ieee80211_hdr_*addr structures. But doesn this break the=20 ieee80211_authentication ieee80211_probe_response ieee80211_assoc_response_frame Structures? Each of these have the ieee80211_hdr_3addr inside it, and by=20 adding the payload variable to the header it effectively moves all variable= s=20 inside the mgmt structures 1 byte. Which would cause the data from these structures to be read incorrectly. A second thing I noticed, with code that is already longer in the repositor= y, ieee80211_rx_mgt checks which kind of frame it is, and when for example it = is=20 a IEEE80211_STYPE_ASSOC_RESP frame, then ieee80211_handle_assoc_resp is bei= ng=20 called which will create a network structure and passes it to the driver. But when should the status variable be checked? It s possible a association request will be rejected by the AP, but with th= e=20 current stack it means that the packet should be handled like this: Driver receives packet, driver checks if it is a mgmt packet, driver checks= if=20 it is an IEEE80211_STYPE_ASSOC_RESP frame, driver checks if the associatio= n=20 succeeded, driver passes the packet to the stack, stack creates the network= =20 structure, and gives it back to the driver. Shouldn't at least the status be checked by the stack? So the driver only has to act when a association request succeeded. IvD |
From: James K. <jke...@li...> - 2005-08-03 16:20:33
|
Ivo van Doorn wrote: >Hi, > >Recently a update was made in GIT where the payload variable was added to all >ieee80211_hdr_*addr structures. But doesn this break the >ieee80211_authentication >ieee80211_probe_response >ieee80211_assoc_response_frame >Structures? Each of these have the ieee80211_hdr_3addr inside it, and by >adding the payload variable to the header it effectively moves all variables >inside the mgmt structures 1 byte. >Which would cause the data from these structures to be read incorrectly. > > They are u8 payload[0], which takes no storage and essentially creates a pointer alias to whatever comes next. struct foo { u8 data1; u8 payload[0]; u8 data2; } bar; &bar.data2 == bar.payload >A second thing I noticed, with code that is already longer in the repository, >ieee80211_rx_mgt checks which kind of frame it is, and when for example it is >a IEEE80211_STYPE_ASSOC_RESP frame, then ieee80211_handle_assoc_resp is being >called which will create a network structure and passes it to the driver. >But when should the status variable be checked? > > Currently, the stack doesn't know about association; it is just performing some data transforms from 802.11 structures to native structures (like ieee80211_network). As intelligence is moved inot the stack (vs. in each driver), then the stack will start to manage the association state and need to do things based on the status. >It s possible a association request will be rejected by the AP, but with the >current stack it means that the packet should be handled like this: >Driver receives packet, driver checks if it is a mgmt packet, driver checks if >it is an IEEE80211_STYPE_ASSOC_RESP frame, driver checks if the association >succeeded, driver passes the packet to the stack, stack creates the network >structure, and gives it back to the driver. >Shouldn't at least the status be checked by the stack? > > With the ipw2200 at least, the IEEE80211_STYPE_ASSOC_RESP frames would typically only come through in monitor mode (the firmware/ucode filters out most management frames) The exception to this is that after an associate completes, the QoS code will try and pass the payload to the ieee80211 subsystem to try and construct a ieee80211_network structure >So the driver only has to act when a association request succeeded. > > The current 'handle_management_frame' should probably be retyped to have specific callbacks for each type of management frame, with a parsed native structure filled out and provided. The parsing and calling would only need to be done if the driver has implemented that callback. For example, instead of having ieee->handle_management_frame there should be: handle_probe_resp() handle_beacon() handle_assoc_resp() Once the ieee80211 stack is more mature, a soft mac solution would likely have little need for the probe_resp/beacon whereas the assoc_resp could be used to configure the lmac to do filtering of non-network packets (as an example) James |
From: Ivo v. D. <iv...@gm...> - 2005-08-03 21:15:42
|
Hi, > >Recently a update was made in GIT where the payload variable was added to > > all ieee80211_hdr_*addr structures. But doesn this break the > >ieee80211_authentication > >ieee80211_probe_response > >ieee80211_assoc_response_frame > >Structures? Each of these have the ieee80211_hdr_3addr inside it, and by > >adding the payload variable to the header it effectively moves all > > variables inside the mgmt structures 1 byte. > >Which would cause the data from these structures to be read incorrectly. > > They are u8 payload[0], which takes no storage and essentially creates a > pointer alias to whatever comes next. > > struct foo { > u8 data1; > u8 payload[0]; > u8 data2; > } bar; > > &bar.data2 =3D=3D bar.payload Thanks for clearing this up, I always assumed u8 payload[0] would have a 1= =20 byte size. Guess I still need some C-coding lessons..;) > >A second thing I noticed, with code that is already longer in the > > repository, ieee80211_rx_mgt checks which kind of frame it is, and when > > for example it is a IEEE80211_STYPE_ASSOC_RESP frame, then > > ieee80211_handle_assoc_resp is being called which will create a network > > structure and passes it to the driver. But when should the status > > variable be checked? > > Currently, the stack doesn't know about association; it is just > performing some data transforms from 802.11 structures to native > structures (like ieee80211_network). > > As intelligence is moved inot the stack (vs. in each driver), then the > stack will start to manage the association state and need to do things > based on the status. My point is not exactly about if the stack will handle it, but with the=20 current method a non-firmware driver (like rt2x00) needs to catch the packe= t=20 and check what status it has, and then pass it to the ieee80211 stack to ge= t=20 the network structure. Wouldn't it be possible if the driver also receives the sk_buffer pointer o= r=20 header structure pointer with ieee->handle_management_frame? This way the stack creates the network structure and hands over all=20 information to the driver so the driver can do all work in one shot. > >It s possible a association request will be rejected by the AP, but with > > the current stack it means that the packet should be handled like this: > > Driver receives packet, driver checks if it is a mgmt packet, driver > > checks if it is an IEEE80211_STYPE_ASSOC_RESP frame, driver checks if > > the association succeeded, driver passes the packet to the stack, stack > > creates the network structure, and gives it back to the driver. > >Shouldn't at least the status be checked by the stack? > > With the ipw2200 at least, the IEEE80211_STYPE_ASSOC_RESP frames would > typically only come through in monitor mode (the firmware/ucode filters > out most management frames) The exception to this is that after an > associate completes, the QoS code will try and pass the payload to the > ieee80211 subsystem to try and construct a ieee80211_network structure Ah, the wonderfull world of wireless chipsets with firmware..:) Sometimes I really wish that the rt2x00 chips where I am creating the drive= r=20 for also contained firmware, it would make it much easier..:) > >So the driver only has to act when a association request succeeded. > > The current 'handle_management_frame' should probably be retyped to have > specific callbacks for each type of management frame, with a parsed > native structure filled out and provided. The parsing and calling would > only need to be done if the driver has implemented that callback. > > For example, instead of having ieee->handle_management_frame there > should be: > > handle_probe_resp() > handle_beacon() > handle_assoc_resp() > > Once the ieee80211 stack is more mature, a soft mac solution would > likely have little need for the probe_resp/beacon whereas the assoc_resp > could be used to configure the lmac to do filtering of non-network > packets (as an example) As a temporary solution, passing the sk_buffer might be usefull as well, th= at=20 provides the driver with the possibility to use the sk_buffer and use the=20 structure required for that management frame. IvD |