[openib-general] [RFC] IB/ipoib: Asynchronous events delivered without port parameter.

Roland Dreier rdreier at cisco.com
Tue Feb 27 07:38:08 PST 2007


 >    I did a short code review of the ipoib code concentrating on
 > partitioning support and I mentioned that the asynchronous events
 > handler in the ipoib code does not take the port number reported in
 > the event record into consideration. The effect of that is that all of
 > the ib# devices related to that specific HCA are flushed when it seems
 > to me that only the relevant port one should be. Is that done on
 > purpose, or am I missing something ?

I don't think there's any particular reason the code is that way
except for the oversight never being corrected.  But it looks trivial
to fix, like the patch below.  Does that look right to you?

 > p.s. I'm working on a patch that should solve another issue caused by
 > PKEY reordering & ipoib behavior and the above issue further
 > complicates things for me.

Why not fix the issue first then?

commit a27cbe878203076247c1b5287f5ab59ed143b560
Author: Roland Dreier <rolandd at cisco.com>
Date:   Tue Feb 27 07:37:49 2007 -0800

    IPoIB: Only handle async events for one port
    
    An asynchronous event carries the port number that the event occurred
    on, so there's no reason for an IPoIB interface to process an event
    associated with a different local HCA port.
    
    Signed-off-by: Roland Dreier <rolandd at cisco.com>

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_verbs.c b/drivers/infiniband/ulp/ipoib/ipoib_verbs.c
index 3cb551b..7f3ec20 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_verbs.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_verbs.c
@@ -259,12 +259,13 @@ void ipoib_event(struct ib_event_handler *handler,
 	struct ipoib_dev_priv *priv =
 		container_of(handler, struct ipoib_dev_priv, event_handler);
 
-	if (record->event == IB_EVENT_PORT_ERR    ||
-	    record->event == IB_EVENT_PKEY_CHANGE ||
-	    record->event == IB_EVENT_PORT_ACTIVE ||
-	    record->event == IB_EVENT_LID_CHANGE  ||
-	    record->event == IB_EVENT_SM_CHANGE   ||
-	    record->event == IB_EVENT_CLIENT_REREGISTER) {
+	if ((record->event == IB_EVENT_PORT_ERR    ||
+	     record->event == IB_EVENT_PKEY_CHANGE ||
+	     record->event == IB_EVENT_PORT_ACTIVE ||
+	     record->event == IB_EVENT_LID_CHANGE  ||
+	     record->event == IB_EVENT_SM_CHANGE   ||
+	     record->event == IB_EVENT_CLIENT_REREGISTER) &&
+	    record->element.port_num == priv->port) {
 		ipoib_dbg(priv, "Port state change event\n");
 		queue_work(ipoib_workqueue, &priv->flush_task);
 	}




More information about the general mailing list