On Sun, 2008-05-18 at 15:36 +0300, Moni Shoua wrote:
> The purpose of this patch is to make the events that are related to SM change
> (namely CLIENT_REREGISTER event and SM_CHANGE event) less disruptive.
> When SM related events are handled, it is not necessary to flush unicast
> info from device but only multicast info.

How is unicast invalidation handled on these changes ? On a local LID
change event, how does an end port know/determine what else (e.g. other
LIDs, paths) the SM might have changed (that specifically might affect
IPoIB since this is limited to IPoIB) ?

Also, wouldn't there be similar issues with other ULPs ?

-- Hal

> This patch divides the events that are
> handled by IPoIB to three categories; 0, 1 and 2 (when 2 does more than 1 and 
> 1
> does more than 0).
> The main change is in __ipoib_ib_dev_flush(). Instead of flagging  to the 
> function
> about pkey_events we now use leveling. An event that requires "harder" 
> flushing
> calls this function with higher number for level. Besides the concept,
> the actual change is that SM related events are not  flushing unicast info and
> not bringing the device down but only refresh the multicast info in the 
> background.
> 
> Signed-off-by: Moni Levy  <[EMAIL PROTECTED]>
> Signed-off-by: Moni Shoua <[EMAIL PROTECTED]>
> 
> ---
> 
>  drivers/infiniband/ulp/ipoib/ipoib.h       |    9 ++++---
>  drivers/infiniband/ulp/ipoib/ipoib_ib.c    |   37 
> ++++++++++++++++++-----------
>  drivers/infiniband/ulp/ipoib/ipoib_main.c  |    5 ++-
>  drivers/infiniband/ulp/ipoib/ipoib_verbs.c |   19 +++++++-------
>  4 files changed, 43 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib.h 
> b/drivers/infiniband/ulp/ipoib/ipoib.h
> index ca126fc..8ed4dc0 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib.h
> +++ b/drivers/infiniband/ulp/ipoib/ipoib.h
> @@ -276,10 +276,11 @@ struct ipoib_dev_priv {
>  
>       struct delayed_work pkey_poll_task;
>       struct delayed_work mcast_task;
> -     struct work_struct flush_task;
> +     struct work_struct flush_task0;
> +     struct work_struct flush_task1;
> +     struct work_struct flush_task2;
>       struct work_struct restart_task;
>       struct delayed_work ah_reap_task;
> -     struct work_struct pkey_event_task;
>  
>       struct ib_device *ca;
>       u8                port;
> @@ -427,7 +428,9 @@ void ipoib_flush_paths(struct net_device *dev);
>  struct ipoib_dev_priv *ipoib_intf_alloc(const char *format);
>  
>  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_ib_dev_flush0(struct work_struct *work);
> +void ipoib_ib_dev_flush1(struct work_struct *work);
> +void ipoib_ib_dev_flush2(struct work_struct *work);
>  void ipoib_pkey_event(struct work_struct *work);
>  void ipoib_ib_dev_cleanup(struct net_device *dev);
>  
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ib.c 
> b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> index f429bce..2a9c058 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> @@ -898,12 +898,14 @@ int ipoib_ib_dev_init(struct net_device *dev, struct 
> ib_device *ca, int port)
>       return 0;
>  }
>  
> -static void __ipoib_ib_dev_flush(struct ipoib_dev_priv *priv, int pkey_event)
> +static void __ipoib_ib_dev_flush(struct ipoib_dev_priv *priv, int level)
>  {
>       struct ipoib_dev_priv *cpriv;
>       struct net_device *dev = priv->dev;
>       u16 new_index;
>  
> +     ipoib_dbg(priv, "Try flushing level %d\n", level);
> +
>       mutex_lock(&priv->vlan_mutex);
>  
>       /*
> @@ -911,7 +913,7 @@ static void __ipoib_ib_dev_flush(struct ipoib_dev_priv 
> *priv, int pkey_event)
>        * the parent is down.
>        */
>       list_for_each_entry(cpriv, &priv->child_intfs, list)
> -             __ipoib_ib_dev_flush(cpriv, pkey_event);
> +             __ipoib_ib_dev_flush(cpriv, level);
>  
>       mutex_unlock(&priv->vlan_mutex);
>  
> @@ -925,7 +927,7 @@ static void __ipoib_ib_dev_flush(struct ipoib_dev_priv 
> *priv, int pkey_event)
>               return;
>       }
>  
> -     if (pkey_event) {
> +     if (level == 2) {
>               if (ib_find_pkey(priv->ca, priv->port, priv->pkey, &new_index)) 
> {
>                       clear_bit(IPOIB_PKEY_ASSIGNED, &priv->flags);
>                       ipoib_ib_dev_down(dev, 0);
> @@ -943,11 +945,13 @@ static void __ipoib_ib_dev_flush(struct ipoib_dev_priv 
> *priv, int pkey_event)
>               priv->pkey_index = new_index;
>       }
>  
> -     ipoib_dbg(priv, "flushing\n");
>  
> -     ipoib_ib_dev_down(dev, 0);
> +     ipoib_mcast_dev_flush(dev);
> +
> +     if (level >= 1)
> +             ipoib_ib_dev_down(dev, 0);
>  
> -     if (pkey_event) {
> +     if (level >= 2) {
>               ipoib_ib_dev_stop(dev, 0);
>               ipoib_ib_dev_open(dev);
>       }
> @@ -957,29 +961,36 @@ static void __ipoib_ib_dev_flush(struct ipoib_dev_priv 
> *priv, int pkey_event)
>        * we get here, don't bring it back up if it's not configured up
>        */
>       if (test_bit(IPOIB_FLAG_ADMIN_UP, &priv->flags)) {
> -             ipoib_ib_dev_up(dev);
> +             if (level >= 1)
> +                     ipoib_ib_dev_up(dev);
>               ipoib_mcast_restart_task(&priv->restart_task);
>       }
>  }
>  
> -void ipoib_ib_dev_flush(struct work_struct *work)
> +void ipoib_ib_dev_flush0(struct work_struct *work)
>  {
>       struct ipoib_dev_priv *priv =
> -             container_of(work, struct ipoib_dev_priv, flush_task);
> +             container_of(work, struct ipoib_dev_priv, flush_task0);
>  
> -     ipoib_dbg(priv, "Flushing %s\n", priv->dev->name);
>       __ipoib_ib_dev_flush(priv, 0);
>  }
>  
> -void ipoib_pkey_event(struct work_struct *work)
> +void ipoib_ib_dev_flush1(struct work_struct *work)
>  {
>       struct ipoib_dev_priv *priv =
> -             container_of(work, struct ipoib_dev_priv, pkey_event_task);
> +             container_of(work, struct ipoib_dev_priv, flush_task1);
>  
> -     ipoib_dbg(priv, "Flushing %s and restarting its QP\n", priv->dev->name);
>       __ipoib_ib_dev_flush(priv, 1);
>  }
>  
> +void ipoib_ib_dev_flush2(struct work_struct *work)
> +{
> +     struct ipoib_dev_priv *priv =
> +             container_of(work, struct ipoib_dev_priv, flush_task2);
> +
> +     __ipoib_ib_dev_flush(priv, 2);
> +}
> +
>  void ipoib_ib_dev_cleanup(struct net_device *dev)
>  {
>       struct ipoib_dev_priv *priv = netdev_priv(dev);
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c 
> b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> index 2442090..2808023 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> @@ -989,9 +989,10 @@ static void ipoib_setup(struct net_device *dev)
>       INIT_LIST_HEAD(&priv->multicast_list);
>  
>       INIT_DELAYED_WORK(&priv->pkey_poll_task, ipoib_pkey_poll);
> -     INIT_WORK(&priv->pkey_event_task, ipoib_pkey_event);
>       INIT_DELAYED_WORK(&priv->mcast_task,   ipoib_mcast_join_task);
> -     INIT_WORK(&priv->flush_task,   ipoib_ib_dev_flush);
> +     INIT_WORK(&priv->flush_task0,   ipoib_ib_dev_flush0);
> +     INIT_WORK(&priv->flush_task1,   ipoib_ib_dev_flush1);
> +     INIT_WORK(&priv->flush_task2,   ipoib_ib_dev_flush2);
>       INIT_WORK(&priv->restart_task, ipoib_mcast_restart_task);
>       INIT_DELAYED_WORK(&priv->ah_reap_task, ipoib_reap_ah);
>  }
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_verbs.c 
> b/drivers/infiniband/ulp/ipoib/ipoib_verbs.c
> index 8766d29..80c0409 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib_verbs.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_verbs.c
> @@ -289,15 +289,16 @@ void ipoib_event(struct ib_event_handler *handler,
>       if (record->element.port_num != priv->port)
>               return;
>  
> -     if (record->event == IB_EVENT_PORT_ERR    ||
> -         record->event == IB_EVENT_PORT_ACTIVE ||
> -         record->event == IB_EVENT_LID_CHANGE  ||
> -         record->event == IB_EVENT_SM_CHANGE   ||
> -         record->event == IB_EVENT_CLIENT_REREGISTER) {
> -             ipoib_dbg(priv, "Port state change event\n");
> -             queue_work(ipoib_workqueue, &priv->flush_task);
> +     ipoib_dbg(priv, "Event %d on device %s port %d\n",record->event,
> +                     record->device->name, record->element.port_num);
> +     if ( record->event == IB_EVENT_SM_CHANGE   ||
> +          record->event == IB_EVENT_CLIENT_REREGISTER) {
> +             queue_work(ipoib_workqueue, &priv->flush_task0);
> +     } else if (record->event == IB_EVENT_PORT_ERR ||
> +                record->event == IB_EVENT_PORT_ACTIVE ||
> +                record->event == IB_EVENT_LID_CHANGE) {
> +             queue_work(ipoib_workqueue, &priv->flush_task1);
>       } else if (record->event == IB_EVENT_PKEY_CHANGE) {
> -             ipoib_dbg(priv, "P_Key change event on port:%d\n", priv->port);
> -             queue_work(ipoib_workqueue, &priv->pkey_event_task);
> +             queue_work(ipoib_workqueue, &priv->flush_task2);
>       }
>  }
> _______________________________________________
> 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

_______________________________________________
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

Reply via email to