OK, this is a whole different approach to the problem.
Seems to make sense to me.
> Quoting Yosef Etigin <[EMAIL PROTECTED]>:
> Subject: [PATCHv3 2/2] ipoib: handle pkey change events
>
> Added - handling the case when a pkey of an interface is deleted and then
> restored
>
> --
>
> This issue was found during partitioning & SM fail over testing.
>
> * Added flush flag to ipoib_ib_dev_stop(), ipoib_ib_dev_down() alike
> * Rename the polling thread work to 'pkey_poll_task' to avoid ambiguity
> * Obtain pkey index prior to entering init_qp, and save in in dev_priv
> * Upon PKEY_CHANGE event, schedule a work that restarts the qp.
> * Precondition the restart on whether the pkey index is really changed.
> Use the cached pkey_index to test this.
> * Restart child interfaces before parent. They might be up even if the
> parent is down.
> * When interface is restarted, queue delayed initiallization, to handle
> the case that a pkey is deleted and later restored.
> * Use uncached pkey query upon qp initiallization
>
> SM reconfiguration or failover possibly causes a shuffling of the values
> in the port pkey table. The current implementation only queries for the
> index of the pkey once, when it creates the device QP and after that moves
> it into working state, and hence does not address this scenario. Fix this
> by using the PKEY_CHANGE event as a trigger to reconfigure the device QP.
>
> Signed-off-by: Yosef Etigin <[EMAIL PROTECTED]>
>
> ---
> drivers/infiniband/ulp/ipoib/ipoib.h | 7 +-
> drivers/infiniband/ulp/ipoib/ipoib_ib.c | 96
> +++++++++++++++++++++++------
> drivers/infiniband/ulp/ipoib/ipoib_main.c | 7 +-
> drivers/infiniband/ulp/ipoib/ipoib_verbs.c | 26 ++-----
> 4 files changed, 97 insertions(+), 39 deletions(-)
>
> Index: b/drivers/infiniband/ulp/ipoib/ipoib.h
> ===================================================================
> --- a/drivers/infiniband/ulp/ipoib/ipoib.h 2007-05-08 15:46:53.000000000
> +0300
> +++ b/drivers/infiniband/ulp/ipoib/ipoib.h 2007-05-10 08:34:58.335171047
> +0300
> @@ -202,15 +202,17 @@ struct ipoib_dev_priv {
> struct list_head multicast_list;
> struct rb_root multicast_tree;
>
> - struct delayed_work pkey_task;
> + struct delayed_work pkey_poll_task;
> struct delayed_work mcast_task;
> struct work_struct flush_task;
> struct work_struct restart_task;
> struct delayed_work ah_reap_task;
> + struct work_struct pkey_event_task;
>
> struct ib_device *ca;
> u8 port;
> u16 pkey;
> + u16 pkey_index;
> struct ib_pd *pd;
> struct ib_mr *mr;
> struct ib_cq *cq;
> @@ -333,12 +335,13 @@ struct ipoib_dev_priv *ipoib_intf_alloc(
>
> int ipoib_ib_dev_init(struct net_device *dev, struct ib_device *ca, int
> port);
> void ipoib_ib_dev_flush(struct work_struct *work);
> +void ipoib_pkey_event(struct work_struct *work);
> void ipoib_ib_dev_cleanup(struct net_device *dev);
>
> int ipoib_ib_dev_open(struct net_device *dev);
> int ipoib_ib_dev_up(struct net_device *dev);
> int ipoib_ib_dev_down(struct net_device *dev, int flush);
> -int ipoib_ib_dev_stop(struct net_device *dev);
> +int ipoib_ib_dev_stop(struct net_device *dev, int flush);
>
> int ipoib_dev_init(struct net_device *dev, struct ib_device *ca, int port);
> void ipoib_dev_cleanup(struct net_device *dev);
> Index: b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> ===================================================================
> --- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c 2007-05-08 15:46:53.000000000
> +0300
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c 2007-05-10 13:01:10.737347938
> +0300
> @@ -408,11 +408,40 @@ void ipoib_reap_ah(struct work_struct *w
> queue_delayed_work(ipoib_workqueue, &priv->ah_reap_task, HZ);
> }
>
> +static int ipoib_find_pkey_index(struct ipoib_dev_priv *priv, int
> *is_changed)
> +{
> + u16 new_index;
> +
> + if (ib_find_pkey(priv->ca, priv->port, priv->pkey, &new_index)) {
> + clear_bit(IPOIB_PKEY_ASSIGNED, &priv->flags);
> + return -ENXIO;
> + }
> +
> + if (is_changed)
> + *is_changed = !test_bit(IPOIB_PKEY_ASSIGNED, &priv->flags) ||
> + priv->pkey_index != new_index;
> +
> + priv->pkey_index = new_index;
> + set_bit(IPOIB_PKEY_ASSIGNED, &priv->flags);
> + return 0;
> +}
I suggest open-coding this - the name ipoib_find_pkey_index
does not tell me that it actually sets flags, etc.
> @@ -260,7 +249,6 @@ void ipoib_event(struct ib_event_handler
> 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 ||
> @@ -268,5 +256,9 @@ void ipoib_event(struct ib_event_handler
> record->element.port_num == priv->port) {
> ipoib_dbg(priv, "Port state change event\n");
> queue_work(ipoib_workqueue, &priv->flush_task);
> + } else if (record->event == IB_EVENT_PKEY_CHANGE &&
> + record->element.port_num == priv->port) {
> + ipoib_dbg(priv, "pkey change event on port:%d\n", priv->port);
> + queue_work(ipoib_workqueue, &priv->pkey_event_task);
> }
> }
BTW, should we maybe do:
if (record->element.port_num != priv->port)
return;
and then we won't have to do this test for each event type?
--
MST
_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
