Re: [PATCH v2 net-next 06/12] qed: Add LL2 slowpath handling

From: Leon Romanovsky <redacted>
Sent: Tuesday, October 3, 2017 4:26 PM

On Tue, Oct 03, 2017 at 11:54:56AM +0300, Michal Kalderon wrote:

quoted

For iWARP unaligned MPA flow, a slowpath event of flushing an
MPA connection that entered an unaligned state is required.
The flush ramrod is received on the ll2 queue, and a pre-registered
callback function is called to handle the flush event.

Signed-off-by: Michal Kalderon <Michal.Kalderon-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Ariel Elior <Ariel.Elior-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
---
 drivers/net/ethernet/qlogic/qed/qed_ll2.c | 40 +++++++++++++++++++++++++++++--
 include/linux/qed/qed_ll2_if.h            |  5 ++++
 2 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_ll2.c b/drivers/net/ethernet/qlogic/qed/qed_ll2.c
index 8eb9645..047f556 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_ll2.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_ll2.c

@@ -423,6 +423,41 @@ static void qed_ll2_rxq_parse_reg(struct qed_hwfn *p_hwfn,
 }

 static int
+qed_ll2_handle_slowpath(struct qed_hwfn *p_hwfn,
+                     struct qed_ll2_info *p_ll2_conn,
+                     union core_rx_cqe_union *p_cqe,
+                     unsigned long *p_lock_flags)
+{
+     struct qed_ll2_rx_queue *p_rx = &p_ll2_conn->rx_queue;
+     struct core_rx_slow_path_cqe *sp_cqe;
+
+     sp_cqe = &p_cqe->rx_cqe_sp;
+     if (sp_cqe->ramrod_cmd_id != CORE_RAMROD_RX_QUEUE_FLUSH) {
+             DP_NOTICE(p_hwfn,
+                       "LL2 - unexpected Rx CQE slowpath ramrod_cmd_id:%d\n",
+                       sp_cqe->ramrod_cmd_id);
+             return -EINVAL;
+     }
+
+     if (!p_ll2_conn->cbs.slowpath_cb) {
+             DP_NOTICE(p_hwfn,
+                       "LL2 - received RX_QUEUE_FLUSH but no callback was provided\n");
+             return -EINVAL;
+     }
+
+     spin_unlock_irqrestore(&p_rx->lock, *p_lock_flags);

Interesting, you are unlock the lock which was taken in upper layer.
It is not actual error, but chances to have such error are pretty high
(for example, after refactoring).

Thanks. Ensuring that the lock will only be unlocked inside the calling function would make 
the calling function long and less readable.
The risk exists, but I think the fact that p_lock_flags is passed as parameter should 
give a strong indication in the future that lock should be handled delicately. --
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html