Re: [PATCH 2/3] ath11k: add separate APIs for monitor mode
From: Kalle Valo <hidden>
Date: 2021-09-21 16:03:10
Also in:
ath11k
Kalle Valo [off-list ref] writes:
quoted
+vdev_stop: + reinit_completion(&ar->vdev_setup_done); + + ret = ath11k_wmi_vdev_stop(ar, vdev_id); + if (ret) { + ath11k_warn(ar->ab, "failed to stop monitor vdev %i after start failure: %d\n", + vdev_id, ret); + return ret; + } + + ret = ath11k_mac_vdev_setup_sync(ar); + if (ret) + ath11k_warn(ar->ab, "failed to synchronize setup for vdev %i stop: %d\n", + vdev_id, ret);I added return ret here for consistency..quoted
+ return ret;I don't thinks this is right, in an error path (vdev_stop label) we return 0? I changed this to -EIO.quoted
+static int ath11k_mac_monitor_vdev_stop(struct ath11k *ar) +{ + int ret; + + lockdep_assert_held(&ar->conf_mutex); + + reinit_completion(&ar->vdev_setup_done); + + ret = ath11k_wmi_vdev_stop(ar, ar->monitor_vdev_id); + if (ret) + ath11k_warn(ar->ab, "failed to request monitor vdev %i stop: %d\n", + ar->monitor_vdev_id, ret); + + ret = ath11k_mac_vdev_setup_sync(ar); + if (ret) + ath11k_warn(ar->ab, "failed to synchronize monitor vdev %i stop: %d\n", + ar->monitor_vdev_id, ret); + + ret = ath11k_wmi_vdev_down(ar, ar->monitor_vdev_id); + if (ret) + ath11k_warn(ar->ab, "failed to put down monitor vdev %i: %d\n", + ar->monitor_vdev_id, ret); + + ath11k_dbg(ar->ab, ATH11K_DBG_MAC, "mac monitor vdev %i stopped\n", + ar->monitor_vdev_id); + return ret; +}I was not sure what's the idea of error path handling here, we print warnings but still continue the normal execution. I changed this so that we bail out in the first error and if everything goes well we return 0.
I found quite a few missing error checks, too many to list here but fixed in the pending branch: https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=8b2f8d11422e7909ff02db456cda41728f621de4 https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=5ff318be206b3d2a0bfdcfaf0ac52cc3b4ecdeae Please double check, compile tested only. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches