On Thu, Feb 4, 2010 at 10:52 PM, Hal Rosenstock
<hal.rosenst...@gmail.com> wrote:
> 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.
I accept the correction.
Sasha, can you please change this in the commit (only if there are not
other remarks).
BTW, there is no explicit reference in the IB spec for MC group
create/delete trap (at least I didn't find it).
>
>> + In all other cases the issuer gis is the trap source.
>
> typo ^^^
> gid
>
and this typo of course.
Thanks,
Eli
> -- 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
>>
>
