On Thu, Feb 4, 2010 at 12:43 PM, Eli Dorfman (Voltaire)
<dorfman....@gmail.com> wrote:
>
> Subject: [PATCH] Wrong handling of MC create and delete traps
>
> For these traps the GID in the data details is the MGID and
> not the source port gid.
> So the SM should check that subscriber port has the pkey of the MC group.
> There was also an error in comparing the subnet prefix and guid due to
> host/network order mismatch.
>
> Signed-off-by: Eli Dorfman <e...@voltaire.com>
> ---
> opensm/opensm/osm_inform.c | 151 ++++++++++++++++++++++++++++---------------
> 1 files changed, 98 insertions(+), 53 deletions(-)
>
> diff --git a/opensm/opensm/osm_inform.c b/opensm/opensm/osm_inform.c
> index 8108213..ae4fe71 100644
> --- a/opensm/opensm/osm_inform.c
> +++ b/opensm/opensm/osm_inform.c
> @@ -341,6 +341,103 @@ Exit:
> return status;
> }
>
> +static int is_access_permitted( osm_infr_t *p_infr_rec,
> + osm_infr_match_ctxt_t *p_infr_match )
> +{
> + cl_list_t *p_infr_to_remove_list = p_infr_match->p_remove_infr_list;
> + ib_inform_info_t *p_ii = &(p_infr_rec->inform_record.inform_info);
> + ib_mad_notice_attr_t *p_ntc = p_infr_match->p_ntc;
> + uint16_t trap_num = cl_ntoh16(p_ntc->g_or_v.generic.trap_num);
> + osm_subn_t *p_subn = p_infr_rec->sa->p_subn;
> + osm_log_t *p_log = p_infr_rec->sa->p_log;
> + char gid_str[INET6_ADDRSTRLEN];
> + osm_mgrp_t *p_mgrp;
> + ib_gid_t source_gid;
> + osm_port_t *p_src_port;
> + osm_port_t *p_dest_port;
> +
> + /* In case of GID_IN(64) or GID_OUT(65) traps the source gid
> + comparison should be done on the trap source (saved as the gid in
> the
> + data details field).
> + For traps MC_CREATE(66) or MC_DELETE(67) the data details gid is
> + the MGID. We need to check whether subscriber has the pky of
typo ^^^^
pkey
> + the MC group.
Shouldn't this be the subscriber has a compatible pkey with MC group ?
The MC group has a full member PKey and the members can be full or
limited.
> + In all other cases the issuer gis is the trap source.
typo ^^^
gid
-- Hal
> + */
> + if (trap_num >= 64 && trap_num <= 67 )
> + /* The issuer of these traps is the SM so source_gid
> + is the gid saved on the data details */
> + source_gid = p_ntc->data_details.ntc_64_67.gid;
> + else
> + source_gid = p_ntc->issuer_gid;
> +
> + p_dest_port =
> + cl_ptr_vector_get(&p_subn->port_lid_tbl,
> + cl_ntoh16(p_infr_rec->report_addr.dest_lid));
> + if (!p_dest_port) {
> + OSM_LOG(p_log, OSM_LOG_INFO,
> + "Cannot find destination port with LID:%u\n",
> + cl_ntoh16(p_infr_rec->report_addr.dest_lid));
> + goto Exit;
> + }
> +
> + switch (trap_num) {
> + case 66:
> + case 67:
> + p_mgrp = osm_get_mgrp_by_mgid(p_subn, &source_gid);
> + if (!p_mgrp) {
> + OSM_LOG(p_log, OSM_LOG_INFO,
> + "Cannot find MGID %s\n",
> + inet_ntop(AF_INET6, source_gid.raw,
> gid_str, sizeof gid_str));
> + goto Exit;
> + }
> +
> + if (!osm_physp_has_pkey(p_log,
> + p_mgrp->mcmember_rec.pkey,
> + p_dest_port->p_physp)) {
> + OSM_LOG(p_log, OSM_LOG_INFO,
> + "MGID %s and port GUID:0x%016" PRIx64
> " do not share same pkey\n",
> + inet_ntop(AF_INET6, source_gid.raw,
> gid_str, sizeof gid_str),
> + cl_ntoh64(p_dest_port->guid));
> + goto Exit;
> + }
> + break;
> +
> + default:
> + p_src_port =
> + osm_get_port_by_guid(p_subn,
> source_gid.unicast.interface_id);
> + if (!p_src_port) {
> + OSM_LOG(p_log, OSM_LOG_INFO,
> + "Cannot find source port with
> GUID:0x%016" PRIx64 "\n",
> +
> cl_ntoh64(source_gid.unicast.interface_id));
> + goto Exit;
> + }
> +
> +
> + /* Check if there is a pkey match. o13-17.1.1 */
> + if (osm_port_share_pkey(p_log, p_src_port,
> p_dest_port) == FALSE) {
> + OSM_LOG(p_log, OSM_LOG_DEBUG, "Mismatch by
> Pkey\n");
> + /* According to o13-17.1.2 - If this
> informInfo does not have
> + lid_range_begin of 0xFFFF, then this
> informInfo request
> + should be removed from database */
> + if (p_ii->lid_range_begin != 0xFFFF) {
> + OSM_LOG(p_log, OSM_LOG_VERBOSE,
> + "Pkey mismatch on
> lid_range_begin != 0xFFFF. "
> + "Need to remove this
> informInfo from db\n");
> + /* add the informInfo record to the
> remove_infr list */
> +
> cl_list_insert_tail(p_infr_to_remove_list, p_infr_rec);
> + }
> + goto Exit;
> + }
> + break;
> + }
> +
> + return 1;
> +Exit:
> + return 0;
> +}
> +
> +
> /**********************************************************************
> * This routine compares a given Notice and a ListItem of InformInfo type.
> * PREREQUISITE:
> @@ -351,15 +448,10 @@ static void match_notice_to_inf_rec(IN cl_list_item_t *
> p_list_item,
> {
> osm_infr_match_ctxt_t *p_infr_match = (osm_infr_match_ctxt_t *)
> context;
> ib_mad_notice_attr_t *p_ntc = p_infr_match->p_ntc;
> - cl_list_t *p_infr_to_remove_list = p_infr_match->p_remove_infr_list;
> osm_infr_t *p_infr_rec = (osm_infr_t *) p_list_item;
> ib_inform_info_t *p_ii = &(p_infr_rec->inform_record.inform_info);
> cl_status_t status = CL_NOT_FOUND;
> osm_log_t *p_log = p_infr_rec->sa->p_log;
> - osm_subn_t *p_subn = p_infr_rec->sa->p_subn;
> - ib_gid_t source_gid;
> - osm_port_t *p_src_port;
> - osm_port_t *p_dest_port;
>
> OSM_LOG_ENTER(p_log);
>
> @@ -460,55 +552,8 @@ static void match_notice_to_inf_rec(IN cl_list_item_t *
> p_list_item,
> }
> }
>
> - /* Check if there is a pkey match. o13-17.1.1 */
> - /* Check if the issuer of the trap is the SM. If it is, then the gid
> - comparison should be done on the trap source (saved as the gid in
> the
> - data details field).
> - If the issuer gid is not the SM - then it is the guid of the trap
> - source */
> - if ((cl_ntoh64(p_ntc->issuer_gid.unicast.prefix) ==
> - p_subn->opt.subnet_prefix)
> - && (cl_ntoh64(p_ntc->issuer_gid.unicast.interface_id) ==
> - p_subn->sm_port_guid))
> - /* The issuer is the SM then this is trap 64-67 - compare the
> gid
> - with the gid saved on the data details */
> - source_gid = p_ntc->data_details.ntc_64_67.gid;
> - else
> - source_gid = p_ntc->issuer_gid;
> -
> - p_src_port =
> - osm_get_port_by_guid(p_subn, source_gid.unicast.interface_id);
> - if (!p_src_port) {
> - OSM_LOG(p_log, OSM_LOG_INFO,
> - "Cannot find source port with GUID:0x%016" PRIx64
> "\n",
> - cl_ntoh64(source_gid.unicast.interface_id));
> + if (!is_access_permitted(p_infr_rec, p_infr_match))
> goto Exit;
> - }
> -
> - p_dest_port =
> - cl_ptr_vector_get(&p_subn->port_lid_tbl,
> - cl_ntoh16(p_infr_rec->report_addr.dest_lid));
> - if (!p_dest_port) {
> - OSM_LOG(p_log, OSM_LOG_INFO,
> - "Cannot find destination port with LID:%u\n",
> - cl_ntoh16(p_infr_rec->report_addr.dest_lid));
> - goto Exit;
> - }
> -
> - if (osm_port_share_pkey(p_log, p_src_port, p_dest_port) == FALSE) {
> - OSM_LOG(p_log, OSM_LOG_DEBUG, "Mismatch by Pkey\n");
> - /* According to o13-17.1.2 - If this informInfo does not have
> - lid_range_begin of 0xFFFF, then this informInfo request
> - should be removed from database */
> - if (p_ii->lid_range_begin != 0xFFFF) {
> - OSM_LOG(p_log, OSM_LOG_VERBOSE,
> - "Pkey mismatch on lid_range_begin != 0xFFFF. "
> - "Need to remove this informInfo from db\n");
> - /* add the informInfo record to the remove_infr list
> */
> - cl_list_insert_tail(p_infr_to_remove_list,
> p_infr_rec);
> - }
> - goto Exit;
> - }
>
> /* send the report to the address provided in the inform record */
> OSM_LOG(p_log, OSM_LOG_DEBUG, "MATCH! Sending Report...\n");
> --
> 1.5.5
>
> There is still a problem with GID OUT trap that is not sent since source port
> was already removed.
> Patch will follow.
>
> --
> 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
>
