Re: [PATCH] opensm: Clean up event subscriptions if a port goes away

2013-09-06 Thread Hal Rosenstock
On 9/5/2013 4:55 AM, Line Holen wrote:
 Event subscriptions needs to be cleaned up if a port goes away.
 If the port comes online again later it may no longer want to
 receive the events on the same QPN. If the old QPN is used for
 something else the SM forwarding events may cause QKey violations.

As many current applications do not handle client reregistration
properly and the current OpenSM behavior is not to do this, this
behavior should be introduced gradually by conditionalizing this on a
new option like drop_event_subscriptions which would default to FALSE.

This is a start on this and associated issues. I'll comment more once we
get past this first patch.

 Signed-off-by: Line Holen line.ho...@oracle.com
 
 ---
 
 diff --git a/include/opensm/osm_inform.h b/include/opensm/osm_inform.h
 index f737441..8cefc20 100644
 --- a/include/opensm/osm_inform.h
 +++ b/include/opensm/osm_inform.h
 @@ -2,6 +2,7 @@
   * Copyright (c) 2004-2009 Voltaire, Inc. All rights reserved.
   * Copyright (c) 2002-2005 Mellanox Technologies LTD. All rights reserved.
   * Copyright (c) 1996-2003 Intel Corporation. All rights reserved.
 + * Copyright (c) 2013 Oracle and/or its affiliates. All rights reserved.
   *
   * This software is available to you under a choice of one of two
   * licenses.  You may choose to be licensed under the terms of the GNU
 @@ -200,6 +201,35 @@ void osm_infr_insert_to_db(IN osm_subn_t * p_subn, IN 
 osm_log_t * p_log,
  void osm_infr_remove_from_db(IN osm_subn_t * p_subn, IN osm_log_t * p_log,
IN osm_infr_t * p_infr);
  
 +/f* OpenSM: Inform Record/osm_infr_remove_subscriptions
 +* NAME
 +*osm_infr_remove_subscriptions
 +*
 +* DESCRIPTION
 +*Remove all event subscriptions of a port
 +*
 +* SYNOPSIS
 +*/
 +ib_api_status_t
 +osm_infr_remove_subscriptions(IN osm_subn_t * p_subn, IN osm_log_t * p_log,
 +   IN ib_net64_t port_guid);
 +/*
 +* PARAMETERS
 +*p_subn
 +*[in] Pointer to the subnet object
 +*
 +*p_log
 +*[in] Pointer to the log object
 +*
 +*port_guid
 +*[in] PortGUID of the subscriber that should be removed
 +*
 +* RETURN
 +*CL_SUCCESS if port_guid had any subscriptions being removed
 +*CL_NOT_FOUND if port_guid did not have any active subscriptions
 +* SEE ALSO
 +*/
 +
  /f* OpenSM: Inform Record/osm_report_notice
  * NAME
  *osm_report_notice
 diff --git a/opensm/osm_drop_mgr.c b/opensm/osm_drop_mgr.c
 index b309273..5f9181a 100644
 --- a/opensm/osm_drop_mgr.c
 +++ b/opensm/osm_drop_mgr.c
 @@ -3,6 +3,7 @@
   * Copyright (c) 2002-2012 Mellanox Technologies LTD. All rights reserved.
   * Copyright (c) 1996-2003 Intel Corporation. All rights reserved.
   * Copyright (c) 2008 Xsigo Systems Inc.  All rights reserved.
 + * Copyright (c) 2013 Oracle and/or its affiliates. All rights reserved.
   *
   * This software is available to you under a choice of one of two
   * licenses.  You may choose to be licensed under the terms of the GNU
 @@ -276,6 +277,13 @@ static void drop_mgr_remove_port(osm_sm_t * sm, IN 
 osm_port_t * p_port)
  
   drop_mgr_clean_physp(sm, p_port-p_physp);
  
 + /* Delete event forwarding subscriptions */
 + if (osm_infr_remove_subscriptions(sm-p_subn, sm-p_log, port_guid)
 + == CL_SUCCESS)
 + OSM_LOG(sm-p_log, OSM_LOG_DEBUG,
 + Removed event subscriptions for port 0x%016 PRIx64 \n,
 + cl_ntoh64(port_guid));
 +
   /* initialize the p_node - may need to get node_desc later */
   p_node = p_port-p_node;
  
 diff --git a/opensm/osm_inform.c b/opensm/osm_inform.c
 index 804c414..6318700 100644
 --- a/opensm/osm_inform.c
 +++ b/opensm/osm_inform.c
 @@ -282,6 +282,41 @@ void osm_infr_remove_from_db(IN osm_subn_t * p_subn, IN 
 osm_log_t * p_log,
   OSM_LOG_EXIT(p_log);
  }
  
 +ib_api_status_t osm_infr_remove_subscriptions(IN osm_subn_t * p_subn,
 +   IN osm_log_t * p_log,
 +   IN ib_net64_t port_guid)
 +{
 + ib_gid_t gid;
 + cl_list_item_t *p_list_item;
 + osm_infr_t *p_infr;
 + ib_api_status_t status = CL_NOT_FOUND;
 +
 + OSM_LOG_ENTER(p_log);
 +
 + gid.unicast.interface_id = port_guid;
 + gid.unicast.prefix = p_subn-opt.subnet_prefix;
 +
 + /* go over all inform info available at the subnet */
 + /* match to the given GID and delete subscriptions if match */
 + p_list_item = cl_qlist_head(p_subn-sa_infr_list);
 + while (p_list_item != cl_qlist_end(p_subn-sa_infr_list)) {
 +
 + p_infr = (osm_infr_t *)p_list_item;
 + p_list_item = cl_qlist_next(p_list_item);
 +
 + if (memcmp((p_infr-inform_record.subscriber_gid), gid, 
 sizeof(ib_gid_t)))

Would:
if (port_guid != p_infr-inform_record.subscriber_gid.unicast.interface_id)
be simpler here ?

-- Hal

 + continue;
 +
 + /* Remove this 

Re: [PATCH] opensm: Clean up event subscriptions if a port goes away

2013-09-06 Thread Line Holen

On 09/06/13 13:56, Hal Rosenstock wrote:

On 9/5/2013 4:55 AM, Line Holen wrote:

Event subscriptions needs to be cleaned up if a port goes away.
If the port comes online again later it may no longer want to
receive the events on the same QPN. If the old QPN is used for
something else the SM forwarding events may cause QKey violations.

As many current applications do not handle client reregistration
properly and the current OpenSM behavior is not to do this, this
behavior should be introduced gradually by conditionalizing this on a
new option like drop_event_subscriptions which would default to FALSE.
I'm not sure I fully grasp all the aspects of this, for instance why 
cleaning up event

subscriptions is different than cleaning up MCG membership.

Anyway, I can make a v2 patch that makes this configurable.


This is a start on this and associated issues. I'll comment more once we
get past this first patch.

OK. Looking forward to that :)

Line



Signed-off-by: Line Holenline.ho...@oracle.com

---

diff --git a/include/opensm/osm_inform.h b/include/opensm/osm_inform.h
index f737441..8cefc20 100644
--- a/include/opensm/osm_inform.h
+++ b/include/opensm/osm_inform.h
@@ -2,6 +2,7 @@
   * Copyright (c) 2004-2009 Voltaire, Inc. All rights reserved.
   * Copyright (c) 2002-2005 Mellanox Technologies LTD. All rights reserved.
   * Copyright (c) 1996-2003 Intel Corporation. All rights reserved.
+ * Copyright (c) 2013 Oracle and/or its affiliates. All rights reserved.
   *
   * This software is available to you under a choice of one of two
   * licenses.  You may choose to be licensed under the terms of the GNU
@@ -200,6 +201,35 @@ void osm_infr_insert_to_db(IN osm_subn_t * p_subn, IN 
osm_log_t * p_log,
  void osm_infr_remove_from_db(IN osm_subn_t * p_subn, IN osm_log_t * p_log,
 IN osm_infr_t * p_infr);

+/f* OpenSM: Inform Record/osm_infr_remove_subscriptions
+* NAME
+*  osm_infr_remove_subscriptions
+*
+* DESCRIPTION
+*  Remove all event subscriptions of a port
+*
+* SYNOPSIS
+*/
+ib_api_status_t
+osm_infr_remove_subscriptions(IN osm_subn_t * p_subn, IN osm_log_t * p_log,
+ IN ib_net64_t port_guid);
+/*
+* PARAMETERS
+*  p_subn
+*  [in] Pointer to the subnet object
+*
+*  p_log
+*  [in] Pointer to the log object
+*
+*  port_guid
+*  [in] PortGUID of the subscriber that should be removed
+*
+* RETURN
+*  CL_SUCCESS if port_guid had any subscriptions being removed
+*  CL_NOT_FOUND if port_guid did not have any active subscriptions
+* SEE ALSO
+*/
+
  /f* OpenSM: Inform Record/osm_report_notice
  * NAME
  * osm_report_notice
diff --git a/opensm/osm_drop_mgr.c b/opensm/osm_drop_mgr.c
index b309273..5f9181a 100644
--- a/opensm/osm_drop_mgr.c
+++ b/opensm/osm_drop_mgr.c
@@ -3,6 +3,7 @@
   * Copyright (c) 2002-2012 Mellanox Technologies LTD. All rights reserved.
   * Copyright (c) 1996-2003 Intel Corporation. All rights reserved.
   * Copyright (c) 2008 Xsigo Systems Inc.  All rights reserved.
+ * Copyright (c) 2013 Oracle and/or its affiliates. All rights reserved.
   *
   * This software is available to you under a choice of one of two
   * licenses.  You may choose to be licensed under the terms of the GNU
@@ -276,6 +277,13 @@ static void drop_mgr_remove_port(osm_sm_t * sm, IN 
osm_port_t * p_port)

drop_mgr_clean_physp(sm, p_port-p_physp);

+   /* Delete event forwarding subscriptions */
+   if (osm_infr_remove_subscriptions(sm-p_subn, sm-p_log, port_guid)
+   == CL_SUCCESS)
+   OSM_LOG(sm-p_log, OSM_LOG_DEBUG,
+   Removed event subscriptions for port 0x%016 PRIx64 \n,
+   cl_ntoh64(port_guid));
+
/* initialize the p_node - may need to get node_desc later */
p_node = p_port-p_node;

diff --git a/opensm/osm_inform.c b/opensm/osm_inform.c
index 804c414..6318700 100644
--- a/opensm/osm_inform.c
+++ b/opensm/osm_inform.c
@@ -282,6 +282,41 @@ void osm_infr_remove_from_db(IN osm_subn_t * p_subn, IN 
osm_log_t * p_log,
OSM_LOG_EXIT(p_log);
  }

+ib_api_status_t osm_infr_remove_subscriptions(IN osm_subn_t * p_subn,
+ IN osm_log_t * p_log,
+ IN ib_net64_t port_guid)
+{
+   ib_gid_t gid;
+   cl_list_item_t *p_list_item;
+   osm_infr_t *p_infr;
+   ib_api_status_t status = CL_NOT_FOUND;
+
+   OSM_LOG_ENTER(p_log);
+
+   gid.unicast.interface_id = port_guid;
+   gid.unicast.prefix = p_subn-opt.subnet_prefix;
+
+   /* go over all inform info available at the subnet */
+   /* match to the given GID and delete subscriptions if match */
+   p_list_item = cl_qlist_head(p_subn-sa_infr_list);
+   while (p_list_item != cl_qlist_end(p_subn-sa_infr_list)) {
+
+   p_infr = (osm_infr_t *)p_list_item;
+   p_list_item = cl_qlist_next(p_list_item);
+
+ 

Re: [PATCH] opensm: Clean up event subscriptions if a port goes away

2013-09-06 Thread Hal Rosenstock
On 9/6/2013 9:53 AM, Line Holen wrote:
 On 09/06/13 13:56, Hal Rosenstock wrote:
 On 9/5/2013 4:55 AM, Line Holen wrote:
 Event subscriptions needs to be cleaned up if a port goes away.
 If the port comes online again later it may no longer want to
 receive the events on the same QPN. If the old QPN is used for
 something else the SM forwarding events may cause QKey violations.
 As many current applications do not handle client reregistration
 properly and the current OpenSM behavior is not to do this, this
 behavior should be introduced gradually by conditionalizing this on a
 new option like drop_event_subscriptions which would default to FALSE.
 I'm not sure I fully grasp all the aspects of this, for instance why
 cleaning up event
 subscriptions is different than cleaning up MCG membership.

The primary users of MCG are IPoIB and CMA which are implemented in the
kernel and support client reregistration.

There is no userspace API for SA event registration other than direct
MADs with the SA and many of these applications do not support client
reregistration (and need fixing).

 Anyway, I can make a v2 patch that makes this configurable.

Thanks.

-- Hal

 This is a start on this and associated issues. I'll comment more once we
 get past this first patch.
 OK. Looking forward to that :)
 
 Line

 Signed-off-by: Line Holenline.ho...@oracle.com

 ---

 diff --git a/include/opensm/osm_inform.h b/include/opensm/osm_inform.h
 index f737441..8cefc20 100644
 --- a/include/opensm/osm_inform.h
 +++ b/include/opensm/osm_inform.h
 @@ -2,6 +2,7 @@
* Copyright (c) 2004-2009 Voltaire, Inc. All rights reserved.
* Copyright (c) 2002-2005 Mellanox Technologies LTD. All rights
 reserved.
* Copyright (c) 1996-2003 Intel Corporation. All rights reserved.
 + * Copyright (c) 2013 Oracle and/or its affiliates. All rights
 reserved.
*
* This software is available to you under a choice of one of two
* licenses.  You may choose to be licensed under the terms of the GNU
 @@ -200,6 +201,35 @@ void osm_infr_insert_to_db(IN osm_subn_t *
 p_subn, IN osm_log_t * p_log,
   void osm_infr_remove_from_db(IN osm_subn_t * p_subn, IN osm_log_t *
 p_log,
IN osm_infr_t * p_infr);

 +/f* OpenSM: Inform Record/osm_infr_remove_subscriptions
 +* NAME
 +*osm_infr_remove_subscriptions
 +*
 +* DESCRIPTION
 +*Remove all event subscriptions of a port
 +*
 +* SYNOPSIS
 +*/
 +ib_api_status_t
 +osm_infr_remove_subscriptions(IN osm_subn_t * p_subn, IN osm_log_t *
 p_log,
 +  IN ib_net64_t port_guid);
 +/*
 +* PARAMETERS
 +*p_subn
 +*[in] Pointer to the subnet object
 +*
 +*p_log
 +*[in] Pointer to the log object
 +*
 +*port_guid
 +*[in] PortGUID of the subscriber that should be removed
 +*
 +* RETURN
 +*CL_SUCCESS if port_guid had any subscriptions being removed
 +*CL_NOT_FOUND if port_guid did not have any active subscriptions
 +* SEE ALSO
 +*/
 +
   /f* OpenSM: Inform Record/osm_report_notice
   * NAME
   *osm_report_notice
 diff --git a/opensm/osm_drop_mgr.c b/opensm/osm_drop_mgr.c
 index b309273..5f9181a 100644
 --- a/opensm/osm_drop_mgr.c
 +++ b/opensm/osm_drop_mgr.c
 @@ -3,6 +3,7 @@
* Copyright (c) 2002-2012 Mellanox Technologies LTD. All rights
 reserved.
* Copyright (c) 1996-2003 Intel Corporation. All rights reserved.
* Copyright (c) 2008 Xsigo Systems Inc.  All rights reserved.
 + * Copyright (c) 2013 Oracle and/or its affiliates. All rights
 reserved.
*
* This software is available to you under a choice of one of two
* licenses.  You may choose to be licensed under the terms of the GNU
 @@ -276,6 +277,13 @@ static void drop_mgr_remove_port(osm_sm_t * sm,
 IN osm_port_t * p_port)

   drop_mgr_clean_physp(sm, p_port-p_physp);

 +/* Delete event forwarding subscriptions */
 +if (osm_infr_remove_subscriptions(sm-p_subn, sm-p_log, port_guid)
 +== CL_SUCCESS)
 +OSM_LOG(sm-p_log, OSM_LOG_DEBUG,
 +Removed event subscriptions for port 0x%016 PRIx64 \n,
 +cl_ntoh64(port_guid));
 +
   /* initialize the p_node - may need to get node_desc later */
   p_node = p_port-p_node;

 diff --git a/opensm/osm_inform.c b/opensm/osm_inform.c
 index 804c414..6318700 100644
 --- a/opensm/osm_inform.c
 +++ b/opensm/osm_inform.c
 @@ -282,6 +282,41 @@ void osm_infr_remove_from_db(IN osm_subn_t *
 p_subn, IN osm_log_t * p_log,
   OSM_LOG_EXIT(p_log);
   }

 +ib_api_status_t osm_infr_remove_subscriptions(IN osm_subn_t * p_subn,
 +  IN osm_log_t * p_log,
 +  IN ib_net64_t port_guid)
 +{
 +ib_gid_t gid;
 +cl_list_item_t *p_list_item;
 +osm_infr_t *p_infr;
 +ib_api_status_t status = CL_NOT_FOUND;
 +
 +OSM_LOG_ENTER(p_log);
 +
 +gid.unicast.interface_id = port_guid;
 +gid.unicast.prefix = p_subn-opt.subnet_prefix;
 +
 +/* go over all inform info available at the subnet */
 +/* match 

[PATCH] opensm: Clean up event subscriptions if a port goes away

2013-09-05 Thread Line Holen
Event subscriptions needs to be cleaned up if a port goes away.
If the port comes online again later it may no longer want to
receive the events on the same QPN. If the old QPN is used for
something else the SM forwarding events may cause QKey violations.

Signed-off-by: Line Holen line.ho...@oracle.com

---

diff --git a/include/opensm/osm_inform.h b/include/opensm/osm_inform.h
index f737441..8cefc20 100644
--- a/include/opensm/osm_inform.h
+++ b/include/opensm/osm_inform.h
@@ -2,6 +2,7 @@
  * Copyright (c) 2004-2009 Voltaire, Inc. All rights reserved.
  * Copyright (c) 2002-2005 Mellanox Technologies LTD. All rights reserved.
  * Copyright (c) 1996-2003 Intel Corporation. All rights reserved.
+ * Copyright (c) 2013 Oracle and/or its affiliates. All rights reserved.
  *
  * This software is available to you under a choice of one of two
  * licenses.  You may choose to be licensed under the terms of the GNU
@@ -200,6 +201,35 @@ void osm_infr_insert_to_db(IN osm_subn_t * p_subn, IN 
osm_log_t * p_log,
 void osm_infr_remove_from_db(IN osm_subn_t * p_subn, IN osm_log_t * p_log,
 IN osm_infr_t * p_infr);
 
+/f* OpenSM: Inform Record/osm_infr_remove_subscriptions
+* NAME
+*  osm_infr_remove_subscriptions
+*
+* DESCRIPTION
+*  Remove all event subscriptions of a port
+*
+* SYNOPSIS
+*/
+ib_api_status_t
+osm_infr_remove_subscriptions(IN osm_subn_t * p_subn, IN osm_log_t * p_log,
+ IN ib_net64_t port_guid);
+/*
+* PARAMETERS
+*  p_subn
+*  [in] Pointer to the subnet object
+*
+*  p_log
+*  [in] Pointer to the log object
+*
+*  port_guid
+*  [in] PortGUID of the subscriber that should be removed
+*
+* RETURN
+*  CL_SUCCESS if port_guid had any subscriptions being removed
+*  CL_NOT_FOUND if port_guid did not have any active subscriptions
+* SEE ALSO
+*/
+
 /f* OpenSM: Inform Record/osm_report_notice
 * NAME
 *  osm_report_notice
diff --git a/opensm/osm_drop_mgr.c b/opensm/osm_drop_mgr.c
index b309273..5f9181a 100644
--- a/opensm/osm_drop_mgr.c
+++ b/opensm/osm_drop_mgr.c
@@ -3,6 +3,7 @@
  * Copyright (c) 2002-2012 Mellanox Technologies LTD. All rights reserved.
  * Copyright (c) 1996-2003 Intel Corporation. All rights reserved.
  * Copyright (c) 2008 Xsigo Systems Inc.  All rights reserved.
+ * Copyright (c) 2013 Oracle and/or its affiliates. All rights reserved.
  *
  * This software is available to you under a choice of one of two
  * licenses.  You may choose to be licensed under the terms of the GNU
@@ -276,6 +277,13 @@ static void drop_mgr_remove_port(osm_sm_t * sm, IN 
osm_port_t * p_port)
 
drop_mgr_clean_physp(sm, p_port-p_physp);
 
+   /* Delete event forwarding subscriptions */
+   if (osm_infr_remove_subscriptions(sm-p_subn, sm-p_log, port_guid)
+   == CL_SUCCESS)
+   OSM_LOG(sm-p_log, OSM_LOG_DEBUG,
+   Removed event subscriptions for port 0x%016 PRIx64 \n,
+   cl_ntoh64(port_guid));
+
/* initialize the p_node - may need to get node_desc later */
p_node = p_port-p_node;
 
diff --git a/opensm/osm_inform.c b/opensm/osm_inform.c
index 804c414..6318700 100644
--- a/opensm/osm_inform.c
+++ b/opensm/osm_inform.c
@@ -282,6 +282,41 @@ void osm_infr_remove_from_db(IN osm_subn_t * p_subn, IN 
osm_log_t * p_log,
OSM_LOG_EXIT(p_log);
 }
 
+ib_api_status_t osm_infr_remove_subscriptions(IN osm_subn_t * p_subn,
+ IN osm_log_t * p_log,
+ IN ib_net64_t port_guid)
+{
+   ib_gid_t gid;
+   cl_list_item_t *p_list_item;
+   osm_infr_t *p_infr;
+   ib_api_status_t status = CL_NOT_FOUND;
+
+   OSM_LOG_ENTER(p_log);
+
+   gid.unicast.interface_id = port_guid;
+   gid.unicast.prefix = p_subn-opt.subnet_prefix;
+
+   /* go over all inform info available at the subnet */
+   /* match to the given GID and delete subscriptions if match */
+   p_list_item = cl_qlist_head(p_subn-sa_infr_list);
+   while (p_list_item != cl_qlist_end(p_subn-sa_infr_list)) {
+
+   p_infr = (osm_infr_t *)p_list_item;
+   p_list_item = cl_qlist_next(p_list_item);
+
+   if (memcmp((p_infr-inform_record.subscriber_gid), gid, 
sizeof(ib_gid_t)))
+   continue;
+
+   /* Remove this event subscription */
+   osm_infr_remove_from_db(p_subn, p_log, p_infr);
+
+   status = CL_SUCCESS;
+   }
+
+   OSM_LOG_EXIT(p_log);
+   return (status);
+}
+
 /**
  * Send a report:
  * Given a target address to send to and the notice.
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html