[PATCH] opensm/osm_mcast_mgr.c: fix bug in MC root switch calculation

[PATCH] opensm/osm_mcast_mgr.c: fix bug in MC root switch calculation

[Sasha Khapyorsky]

In newly introduced MC group root switch calculation method as it is
now, sw->num_of_mcm field (number of MC group members connected to this
switch) is updated only once when switch is first visited (for some
reason it is under such 'if' block), as result it will always '1' or
'0', which is obviously not correct and breaks the following hops
calculations algorithm. Fix this - update sw->num_of_mcm field
unconditionally.

Signed-off-by: Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org>
---
 opensm/opensm/osm_mcast_mgr.c |   17 ++++++++---------
 1 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/opensm/opensm/osm_mcast_mgr.c b/opensm/opensm/osm_mcast_mgr.c
index abd54a0..ef288b8 100644
--- a/opensm/opensm/osm_mcast_mgr.c
+++ b/opensm/opensm/osm_mcast_mgr.c
@@ -190,18 +190,17 @@ static void mcast_mgr_build_switch_map(osm_sm_t * sm,
 		remote_sw = remote_node->sw;
 		port_guid = osm_node_get_node_guid(remote_node);
 		if (cl_qmap_get(p_mcast_member_sw_tbl, port_guid) ==
-		    cl_qmap_end(p_mcast_member_sw_tbl)) {
+		    cl_qmap_end(p_mcast_member_sw_tbl))
 			/* insert switch to table */
 			cl_qmap_insert(p_mcast_member_sw_tbl, port_guid,
 				       &remote_sw->mgrp_item);
-			/* New element in the table */
-			if (p_port->p_node->sw)
-				/* the switch is MC member */
-				remote_sw->is_mc_member = 1;
-			else
-				/* for others - update MC count */
-				remote_sw->num_of_mcm++;
-		}
+
+		if (p_port->p_node->sw)
+			/* the switch is MC member */
+			remote_sw->is_mc_member = 1;
+		else
+			/* for others - update MC count */
+			remote_sw->num_of_mcm++;
 	}
 	OSM_LOG_EXIT(sm->p_log);
 }
-- 
1.6.6.1

--
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

[PATCH] opensm/osm_mcast_mgr.c: preserve root switch calculation functionality

[Sasha Khapyorsky]

I'm not sure that this is a bug, but anyway. Now for MC group spanning
tree root switch calculation it uses max hops criteria differently -
accumulated hops for all MC members connected to the switch against
previous maximal one.

This patch makes it functionally exactly as it was.

Signed-off-by: Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org>
---
 opensm/opensm/osm_mcast_mgr.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/opensm/opensm/osm_mcast_mgr.c b/opensm/opensm/osm_mcast_mgr.c
index f33d6a6..322635d 100644
--- a/opensm/opensm/osm_mcast_mgr.c
+++ b/opensm/opensm/osm_mcast_mgr.c
@@ -253,7 +253,8 @@ static float mcast_mgr_compute_max_hops(osm_sm_t * sm, cl_qmap_t * m,
 		sw = cl_item_obj(i, sw, mgrp_item);
 		lid = cl_ntoh16(osm_node_get_base_lid(sw->p_node, 0));
 		hops = osm_switch_get_least_hops(this_sw, lid);
-		hops = (hops + 1) * sw->num_of_mcm + hops * sw->is_mc_member;
+		if (!sw->is_mc_member)
+			hops += 1;
 		if (hops > max_hops)
 			max_hops = hops;
 	}
-- 
1.6.6.1

--
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

Re: [PATCH] opensm/osm_mcast_mgr.c: preserve root switch calculation functionality

[Eli Dorfman]

On Fri, Feb 5, 2010 at 12:22 AM, Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
>
> I'm not sure that this is a bug, but anyway. Now for MC group spanning
> tree root switch calculation it uses max hops criteria differently -
> accumulated hops for all MC members connected to the switch against
> previous maximal one.
>
> This patch makes it functionally exactly as it was.

right. otherwise it is sort of average hops.

>
> Signed-off-by: Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org>
> ---
>  opensm/opensm/osm_mcast_mgr.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/opensm/opensm/osm_mcast_mgr.c b/opensm/opensm/osm_mcast_mgr.c
> index f33d6a6..322635d 100644
> --- a/opensm/opensm/osm_mcast_mgr.c
> +++ b/opensm/opensm/osm_mcast_mgr.c
> @@ -253,7 +253,8 @@ static float mcast_mgr_compute_max_hops(osm_sm_t * sm, cl_qmap_t * m,
>                sw = cl_item_obj(i, sw, mgrp_item);
>                lid = cl_ntoh16(osm_node_get_base_lid(sw->p_node, 0));
>                hops = osm_switch_get_least_hops(this_sw, lid);
> -               hops = (hops + 1) * sw->num_of_mcm + hops * sw->is_mc_member;
> +               if (!sw->is_mc_member)
> +                       hops += 1;

I think that this condition is may be avoided since if the switch is a
member in the MC group then the packet
still needs to get to the CPU that is running on it (enhanced port)
and this is similar to additional hop
in case of host (assuming that host cpu is much faster then switch cpu).

Eli
>                if (hops > max_hops)
>                        max_hops = hops;
>        }
> --
> 1.6.6.1
>
> --
> 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
>
--
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