libwimaxll: (fb Johannes Berg) fix handling of ack/nlerr in msg/reset/rfkill

We were not handling the thing properly and thus it was working only
by chance. We were missing the nlerr as reponses to messages.

The callbacks need to be setup to listen for the message we are
waiting for, ack and nlerr. And then we need to call nl_recvmsgs()
until the ack / or nlerr handler have been seen.

This makes the GENL_ADMIN_PERM setting in the kernel's policy flags work.

Signed-off-by: Inaky Perez-Gonzalez <inaky@linux.intel.com>

5 files changed, 42 insertions, 22 deletions

diff --git a/lib/internal.h b/lib/internal.h
index b85619a..9fda2ba 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -125,6 +125,10 @@ struct wimaxll_handle {
  * This way the driver can define it's own private pipes (if needed)
  * for high bandwidth traffic (for example, tracing information)
  * without affecting the rest of the groups (channels).
+ *
+ * msg_done is used by the ack and error generic netlink callbacks to
+ * indicate to the message receving loop that all the parts of the
+ * message have been received.
  */
 struct wimaxll_mc_handle {
 	int idx;
@@ -132,7 +136,8 @@ struct wimaxll_mc_handle {
 	struct nl_handle *nlh_rx;
 	struct nl_cb *nl_cb;
 	ssize_t result;
-
+	unsigned msg_done:1;		/* internal */
+	
 	wimaxll_msg_to_user_cb_f msg_to_user_cb;
 	struct wimaxll_gnl_cb_context *msg_to_user_context;
 
@@ -159,6 +164,7 @@ int wimaxll_gnl_handle_state_change(struct wimaxll_handle *,
 				  struct wimaxll_mc_handle *,
 				  struct nl_msg *);
 int wimaxll_gnl_error_cb(struct sockaddr_nl *, struct nlmsgerr *, void *);
+int wimaxll_gnl_ack_cb(struct nl_msg *msg, void *_mch);
 struct wimaxll_mc_handle *__wimaxll_get_mc_handle(struct wimaxll_handle *,
 					      int pipe_id);
 
diff --git a/lib/mc_rx.c b/lib/mc_rx.c
index 2bca6a9..58fe64f 100644
--- a/lib/mc_rx.c
+++ b/lib/mc_rx.c
@@ -323,6 +323,7 @@ int wimaxll_mc_rx_open(struct wimaxll_handle *wmx,
 		goto error_cb_alloc;
 	}
 
+	nl_cb_set(mch->nl_cb, NL_CB_ACK, NL_CB_CUSTOM, wimaxll_gnl_ack_cb, mch);
 	nl_cb_set(mch->nl_cb, NL_CB_SEQ_CHECK, NL_CB_CUSTOM,
 		  wimaxll_seq_check_cb, NULL);
 	nl_cb_set(mch->nl_cb, NL_CB_VALID, NL_CB_CUSTOM, wimaxll_gnl_cb, mch);
@@ -503,23 +504,22 @@ ssize_t wimaxll_mc_rx_read(struct wimaxll_handle *wmx, unsigned index)
 	 * libnl's nl_recvmsgs() will read and call the different
 	 * callbacks we specified at wimaxll_mc_rx_open() time. That's
 	 * where the processing of the message content is done.
+	 *
+	 * Now, messages from the kernel don't carry ACKs or NLERRs,
+	 * so we are just receiving a message packet all the
+	 * time--except if things go wrong. 
 	 */
 	mch->result = -EINPROGRESS;
+	mch->msg_done = 0;
+	d_printf(2, wmx, "I: Calling nl_recvmsgs()\n");
 	result = nl_recvmsgs(mch->nlh_rx, mch->nl_cb);
-	if (result < 0) {
+	if (result < 0)
 		wimaxll_msg(wmx, "E: %s: nl_recvmgsgs failed: %d\n",
-			  __func__, result);
-		goto error_nl_recvmsgs;
-	}
-	result = mch->result;
-	if (result == -EINPROGRESS) {
-		wimaxll_msg(wmx, "E: %s: no messages parsed\n", __func__);
-		goto error_data;
-	}
+			    __func__, result);
+	else
+		result = mch->result;
 	/* No complains on error; the kernel might just be sending an
 	 * error out; pass it through. */
-error_data:
-error_nl_recvmsgs:
 error_not_open:
 error_bad_index:
 	d_fnend(3, wmx, "(wmx %p index %u) = %zd\n", wmx, index, result);
diff --git a/lib/op-reset.c b/lib/op-reset.c
index c78fef4..c81a033 100644
--- a/lib/op-reset.c
+++ b/lib/op-reset.c
@@ -112,7 +112,6 @@ int wimaxll_reset(struct wimaxll_handle *wmx)
 	}
 	/* Read the message ACK from netlink */
 	result = wimaxll_wait_for_rp_result(wmx);
-	nl_wait_for_ack(wmx->nlh_tx);
 	if (result < 0)
 		wimaxll_msg(wmx, "E: RESET: operation failed: %zd\n", result);
 error_msg_prep:
diff --git a/lib/op-rfkill.c b/lib/op-rfkill.c
index cad5f4e..90be4b6 100644
--- a/lib/op-rfkill.c
+++ b/lib/op-rfkill.c
@@ -119,7 +119,6 @@ int wimaxll_rfkill(struct wimaxll_handle *wmx, enum wimax_rf_state state)
 	}
 	/* Read the message ACK from netlink */
 	result = wimaxll_wait_for_rp_result(wmx);
-	nl_wait_for_ack(wmx->nlh_tx);
 	if (result < 0)
 		wimaxll_msg(wmx, "E: RFKILL: operation failed: %zd\n", result);
 error_msg_prep:
diff --git a/lib/wimax.c b/lib/wimax.c
index 5af4126..650570b 100644
--- a/lib/wimax.c
+++ b/lib/wimax.c
@@ -78,8 +78,9 @@ int wimaxll_gnl_error_cb(struct sockaddr_nl *nla, struct nlmsgerr *nlerr,
 	d_fnstart(7, wmx, "(nla %p nlnerr %p [%d] mch %p)\n",
 		  nla, nlerr, nlerr->error, _mch);
 	wimaxll_mch_maybe_set_result(mch, nlerr->error);
-	d_fnend(7, wmx, "(nla %p nlnerr %p [%d] mch %p) = %d\n",
-		nla, nlerr, nlerr->error, _mch, NL_STOP);
+	mch->msg_done = 1;
+	d_fnend(7, wmx, "(nla %p nlnerr %p [%d] mch %p) = NL_STOP\n",
+		nla, nlerr, nlerr->error, _mch);
 	return NL_STOP;
 }
 
@@ -140,6 +141,7 @@ int wimaxll_gnl_ack_cb(struct nl_msg *msg, void *_mch)
 	if (nl_err->error < 0)
 		d_printf(2, NULL, "D: netlink ack: received netlink error %d\n",
 			  nl_err->error);
+	mch->msg_done = 1;
 error_ack_short:
 error_bad_type:
 	d_fnend(7, NULL, "(msg %p mch %p) = NL_STOP\n", msg, _mch);
@@ -168,17 +170,24 @@ int wimaxll_wait_for_ack(struct wimaxll_handle *wmx)
 
 	fake_mch.wmx = wmx;
 	fake_mch.result = -EINPROGRESS;
+	fake_mch.msg_done = 0;
 
 	cb = nl_socket_get_cb(wmx->nlh_tx);
 	nl_cb_set(cb, NL_CB_ACK, NL_CB_CUSTOM, wimaxll_gnl_ack_cb, &fake_mch);
 	nl_cb_set(cb, NL_CB_VALID, NL_CB_CUSTOM, NL_CB_DEFAULT, NULL);
 	nl_cb_err(cb, NL_CB_CUSTOM, wimaxll_gnl_error_cb, &fake_mch);
-	result = nl_recvmsgs_default(wmx->nlh_tx);
+	do
+		result = nl_recvmsgs(wmx->nlh_tx, cb);
+	while (fake_mch.msg_done == 0 && result >= 0);
+	result = fake_mch.result;
 	nl_cb_set(cb, NL_CB_ACK, NL_CB_CUSTOM, NL_CB_DEFAULT, NULL);
+	nl_cb_set(cb, NL_CB_VALID, NL_CB_CUSTOM, NL_CB_DEFAULT, NULL);
+	nl_cb_err(cb, NL_CB_CUSTOM, NL_CB_DEFAULT, NULL);
 	nl_cb_put(cb);
 	if (result < 0)
 		return result;
-	return fake_mch.result;
+	else
+		return fake_mch.result;
 }
 
 
@@ -274,9 +283,6 @@ error_parse:
  *
  * This function simplifies waiting for that and getting the reply.
  *
- * You still need to wait for the ack in your function calling
- * nl_wait_for_ack().
- *
  * \note We use the same handler used for transmission in the TX side
  *     and it's callback set, that we modifiy. As noted in the doc for
  *     \a struct wimaxll_handle, each call site to nl_recvmsgs_default()
@@ -292,11 +298,21 @@ ssize_t wimaxll_wait_for_rp_result(struct wimaxll_handle *wmx)
 	d_fnstart(5, wmx, "(wmx %p)\n", wmx);
 	fake_mch.wmx = wmx;
 	fake_mch.result = -EINPROGRESS;
+	fake_mch.msg_done = 0;
 	cb = nl_socket_get_cb(wmx->nlh_tx);
+	nl_cb_set(cb, NL_CB_ACK, NL_CB_CUSTOM,
+		  wimaxll_gnl_ack_cb, &fake_mch);
 	nl_cb_set(cb, NL_CB_VALID, NL_CB_CUSTOM,
 		  wimaxll_gnl_rp_result_cb, &fake_mch);
 	nl_cb_err(cb, NL_CB_CUSTOM, wimaxll_gnl_error_cb, &fake_mch);
-	nl_recvmsgs_default(wmx->nlh_tx);
+	
+	do
+		result = nl_recvmsgs(wmx->nlh_tx, cb);
+	while (fake_mch.msg_done == 0 && result >= 0);
+	result = fake_mch.result;
+	nl_cb_set(cb, NL_CB_ACK, NL_CB_CUSTOM, NL_CB_DEFAULT, NULL);
+	nl_cb_set(cb, NL_CB_VALID, NL_CB_CUSTOM, NL_CB_DEFAULT, NULL);
+	nl_cb_err(cb, NL_CB_CUSTOM, NL_CB_DEFAULT, NULL);
 	nl_cb_put(cb);
 
 	result = fake_mch.result;