In order to first integrate frame-xchg some refactoring needed to
be done. First it is useful to allow queueing frames up rather than
requiring the module (p2p, anqp etc) to wait for the last frame to
finish. This can be aided by radio management but frame-xchg needed
some refactoring as well.
First was getting rid of this fx pointer re-use. It looks like this
was done to save a bit of memory but things get pretty complex
needed to check if the pointer is stale or has been reset. Instead
of this we now just allocate a new pointer each frame-xchg. This
allows for the module to queue multiple requests as well as removes
the complexity of needed to check if the fx pointer is stale.
Next was adding the ability to track frame-xchgs by ID. If a module
can queue up multiple requests it also needs to be able to cancel
them individually vs per-wdev. This comes free with the wiphy work
queue since it returns an ID which can be given directly to the
caller.
Then radio management was simply piped in by adding the
insert/done APIs.
frame_xchg_startv was using sizeof(mmpdu) to check the minimum length
for a frame. Instead mmpdu_header_len should be used since this checks
fc.order and returns either 24 or 28 bytes, not 28 bytes always.
This change adds the requirement that the first iovec in the array
must contain at least the first 2 bytes (mmpdu_fc) of the header.
This really shouldn't be a problem since all current users of
frame-xchg put the entire header (or entire frame) into the first
iovec in the array.
This bug is caused by the following behavior:
1. Start a frame-xchg, wait for callback
2. From callback start a new frame-xchg, same prefix.
The new frame-xchg request will detect that there is a duplicate watch,
which is correct behavior. It will then remove this duplicate from the
watchlist. The issue here is that we are in the watchlist notify loop
from the original xchg. This causes that loop to read from the now
freed watchlist item, causing an invalid read.
Instead of freeing the item immediately, check if the notify loop is in
progress and only set 'id' to zero and 'stale_items' to true. This will
allow the notify loop to finish, then the watchlist code will prune out
any stale items. If not in the notify loop the item can be freed as it
was before.
Don't match the default group's (group_id 0) wdev_id against the
provided wdev_id because the default group can be used on all wdevs and
its wdev_id is 0. Also match individual item's wdev_id in the group to
make up for this although it normally wouldn't matter.
SOL_NETLINK is used since commit
87a198111a resulting in the following
build failure with glibc < 2.24:
src/frame-xchg.c: In function 'frame_watch_group_io_read':
src/frame-xchg.c:328:27: error: 'SOL_NETLINK' undeclared (first use in this function)
if (cmsg->cmsg_level != SOL_NETLINK)
^
This failure is due to glibc that doesn't support SOL_NETLINK before
version 2.24 and
f9b437d5ef
Fixes:
- http://autobuild.buildroot.org/results/3485088b84111c271bbcfaf025aa4103c6452072
This function was intended to catch socket errors and destroy the group
but it would leak the l_io object if that happened, and if called on
ordinary shutdown it could cause a crash. Since we're now assuming
that the netlink socket operations never fail just remove it.
In some cases a P2P peer will ACK our frame but not reply on the first
attempt, and other implementations seem to handle this by going back to
retransmitting the frame at a high rate until it gets ACKed again, at
which point they will again give the peer a longer time to tx the
response frame. Implement the same logic here by adding a
retries_on_ack parameter that takes the number of additional times we
want to restart the normal retransmit counter after we received no
response frame on the first attempt. So passing 0 maintains the
current behaviour, 1 for 1 extra attempt, etc.
In effect we may retransmit a frame about 15 * (retry_on_ack + 1) *
<in-kernel retransmit limit> times. The kernel/driver retransmits a
frame a number of times if there's no ACK (I've seen about 20 normally)
at a high frequency, if that fails we retry the whole process 15 times
inside frame-xchg.c and if we still get no ACK at any point, we give up.
If we do get an ACK, we wait for a response frame and if we don't get
that we will optionally reset the retry counter and restart the whole
thing retry_on_ack times.
Since frame_watch_remove_by_handler only forgets a given function +
user data pointers, and doesn't remove the frame prefixes added in the
kernel, we can avoid later re-registering those prefixes with the
kernel by keeping them in our local watchlist, and only replacing the
handler pointer with a dummy function.
Make sure a frame callback is free to call frame_xchg_stop without
causing a crash. Frame callback here means the one that gets
called if our tx frame was ACKed and triggered a respone frame that
matched one of the provided prefixes, within the given time.
All in all a frame callback is allowed to call either
frame_xchg_stop or frame_xchg_startv or neither. Same applies to
the final callback (called when no matching responses received).
Don't crash if the user calls frame_xchg_stop(wdev) from inside the
frame exchange's final callback. That call is going to be redundant but
it's convenient to do this inside a cleanup function for a given wdev
without having to check whether any frame exchange was actually running.
For nl80211 sockets other than our main l_genl object use socket io
directly, to avoid creating many instances of l_genl. The only reason
we use multiple sockets is to work around an nl80211 design quirk that
requires closing the socket to unregister management frame watches.
Normally there should not be a need to create multiple sockets in a
program.
Add a little state machine and a related API, to simplify sending out a
frame, receiving the Ack / No-ack status and (if acked) waiting for a
response frame from the target device, one of a list of possible
frame prefixes. The nl80211 API for this makes it complicated
enough that this new API seems to be justified, on top of that there's a
quirk when using the brcmfmac driver where the nl80211 response
(containing the operation's cookie), the Tx Status event and the response
Frame event are received from nl80211 in reverse order (not seen with
other drivers so far), further complicating what should be a pretty
simple task.
Try to better deduplicate the frame watches. Until now we'd check if
we'd already registered a given frame body prefix with the kernel, or a
matching more general prefix (shorter). Now also try to check if we
have already have a watch with the same callback pointer and user_data
value, and:
* an identical or shorter (more general) prefix, in that case ignore
the new watch completely.
* a longer (more specific) prefix, in that case forget the existing
watch.
The use case for this is when we have a single callback for multiple
watches and multiple frame types, and inside that callback we're looking
at the frame body again and matching it to frame types. In that case
we don't want that function to be called multiple times for one frame
event.
In frame_watch_group_remove I forgot to actually match the group to be
removed by both wdev_id and group_id. group_ids are unique only in the
scope of one wdev.
I forgot to actually add new groups being created in
frame_watch_group_get to the watch_groups queue, meaning that we'd
re-create the group every time a new watch was added to the group.
This new API is independent of netdev.c and allows actually
unregistering from receiving notifications of frames, although with some
quirks. The current API only allowed the callback for a registration to
be forgotten but our process and/or the kernel would still be woken up
when matching frames were received because the kernel had no frame
unregister call. In the new API you can supply a group-id paramter when
registering frames. If it is non-zero the frame_watch_group_remove() call
can be used to remove all frame registrations that had a given group-id
by closing the netlink socket on which the notifications would be
received. This means though that it's a slightly costly operation.
The file is named frame-xchg.c because I'm thinking of also adding
utilities for sending frames and waiting for one of a number of replies
and handling the acked/un-acked information.
James Prestwood