[ofa-general] Re: [PATCHv2] opensm: Add support for MulticastFDBTop

[Sasha Khapyorsky]

On 10:38 Mon 21 Sep     , Hal Rosenstock wrote:
> 
> Add support for SwitchInfo:MulticastFDBTop
> Added by MgtWG errata #4505-4508
> Also, per MgtWG RefID #4640, MulticastFDBTop value of 0xbfff means no entries
>  
> In osm_sm.c:osm_sm_set_mcast_tbl, when switch port 0 indicates
> IsMulticastFDBTop supported, set MulticastFDBTop in SwitchInfo
> based on max_block_in_use
> 
> Signed-off-by: Hal Rosenstock <hal.rosenstock at gmail.com>
> ---
> Changes since v1:
> In mcast_mgr_set_mfttop, eliminated PortStateChange checking
> 
> diff --git a/opensm/opensm/osm_mcast_mgr.c b/opensm/opensm/osm_mcast_mgr.c
> index c1d1916..0da0ef1 100644
> --- a/opensm/opensm/osm_mcast_mgr.c
> +++ b/opensm/opensm/osm_mcast_mgr.c
> @@ -1044,6 +1044,64 @@ static ib_api_status_t mcast_mgr_process_mlid(osm_sm_t * sm, uint16_t mlid)
>  
>  /**********************************************************************
>   **********************************************************************/
> +static void mcast_mgr_set_mfttop(IN osm_sm_t * sm, IN osm_switch_t * p_sw)
> +{
> +	osm_node_t *p_node;
> +	osm_dr_path_t *p_path;
> +	osm_physp_t *p_physp;
> +	osm_mcast_tbl_t *p_tbl;
> +	osm_madw_context_t context;
> +	ib_api_status_t status;
> +	ib_switch_info_t si;
> +	uint16_t mcast_top;
> +
> +	OSM_LOG_ENTER(sm->p_log);
> +
> +	CL_ASSERT(p_sw);
> +
> +	p_node = p_sw->p_node;
> +
> +	CL_ASSERT(p_node);
> +
> +	p_physp = osm_node_get_physp_ptr(p_node, 0);
> +	p_path = osm_physp_get_dr_path_ptr(p_physp);
> +	p_tbl = osm_switch_get_mcast_tbl_ptr(p_sw);
> +
> +	if (p_physp->port_info.capability_mask & IB_PORT_CAP_HAS_MCAST_FDB_TOP) {
> +		/*
> +		   Set the top of the multicast forwarding table.
> +		 */
> +		si = p_sw->switch_info;
> +		if (p_tbl->max_block_in_use == -1)
> +			mcast_top = cl_hton16(IB_LID_MCAST_START_HO - 1);
> +		else
> +			mcast_top = cl_hton16(IB_LID_MCAST_START_HO +
> +					      (p_tbl->max_block_in_use + 1) * IB_MCAST_BLOCK_SIZE - 1);
> +		if (mcast_top != si.mcast_top) {
> +			si.mcast_top = mcast_top;
> +
> +			OSM_LOG(sm->p_log, OSM_LOG_DEBUG,
> +				"Setting switch MFT top to MLID 0x%x\n",
> +				cl_ntoh16(si.mcast_top));
> +
> +			context.si_context.light_sweep = FALSE;
> +			context.si_context.node_guid = osm_node_get_node_guid(p_node);
> +			context.si_context.set_method = TRUE;
> +
> +			status = osm_req_set(sm, p_path, (uint8_t *) & si,
> +					     sizeof(si), IB_MAD_ATTR_SWITCH_INFO,
> +					     0, CL_DISP_MSGID_NONE, &context);
> +
> +			if (status != IB_SUCCESS)
> +				OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 0A1B: "
> +					"Sending SwitchInfo attribute failed (%s)\n",
> +					ib_get_err_str(status));
> +		}
> +	}
> +}

Basically the patch looks fine, I would suggest to simplify flows here
by using 'if ("no update needed") return;', but it is minor.

So we are just wating for capability bits clarification.

Sasha

> +
> +/**********************************************************************
> + **********************************************************************/
>  static int mcast_mgr_set_mftables(osm_sm_t * sm)
>  {
>  	cl_qmap_t *p_sw_tbl = &sm->p_subn->sw_guid_tbl;
> @@ -1059,6 +1117,7 @@ static int mcast_mgr_set_mftables(osm_sm_t * sm)
>  		p_tbl = osm_switch_get_mcast_tbl_ptr(p_sw);
>  		if (osm_mcast_tbl_get_max_block_in_use(p_tbl) > max_block)
>  			max_block = osm_mcast_tbl_get_max_block_in_use(p_tbl);
> +		mcast_mgr_set_mfttop(sm, p_sw);
>  		p_sw = (osm_switch_t *) cl_qmap_next(&p_sw->map_item);
>  	}
>  
> diff --git a/opensm/opensm/osm_sa_class_port_info.c b/opensm/opensm/osm_sa_class_port_info.c
> index d2ab96a..fb58fe5 100644
> --- a/opensm/opensm/osm_sa_class_port_info.c
> +++ b/opensm/opensm/osm_sa_class_port_info.c
> @@ -1,6 +1,6 @@
>  /*
>   * Copyright (c) 2004-2008 Voltaire, Inc. All rights reserved.
> - * Copyright (c) 2002-2005 Mellanox Technologies LTD. All rights reserved.
> + * Copyright (c) 2002-2009 Mellanox Technologies LTD. All rights reserved.
>   * Copyright (c) 1996-2003 Intel Corporation. All rights reserved.
>   *
>   * This software is available to you under a choice of one of two
> @@ -159,8 +159,10 @@ static void cpi_rcv_respond(IN osm_sa_t * sa, IN const osm_madw_t * p_madw)
>  	    OSM_CAP_IS_PORT_INFO_CAPMASK_MATCH_SUPPORTED;
>  #endif
>  	if (sa->p_subn->opt.qos)
> -		ib_class_set_cap_mask2(p_resp_cpi, OSM_CAP2_IS_QOS_SUPPORTED);
> -
> +		ib_class_set_cap_mask2(p_resp_cpi, OSM_CAP2_IS_QOS_SUPPORTED |
> +						   OSM_CAP2_IS_MCAST_TOP_SUPPORTED);
> +	else
> +		ib_class_set_cap_mask2(p_resp_cpi, OSM_CAP2_IS_MCAST_TOP_SUPPORTED);
>  	if (!sa->p_subn->opt.disable_multicast)
>  		p_resp_cpi->cap_mask |= OSM_CAP_IS_UD_MCAST_SUP;
>  	p_resp_cpi->cap_mask = cl_hton16(p_resp_cpi->cap_mask);
>