On Fri, Feb 5, 2010 at 9:18 AM, Eli Dorfman <[email protected]> wrote:
> On Thu, Feb 4, 2010 at 10:52 PM, Hal Rosenstock
> <[email protected]> wrote:
>> On Thu, Feb 4, 2010 at 12:43 PM, Eli Dorfman (Voltaire)
>> <[email protected]> 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 <[email protected]>
>>> ---
>>> 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.
Doesn't this require a code change for handling trap cases 66-67 ?
> Sasha, can you please change this in the commit (only if there are not
> other remarks).
Is that what you are asking Sasha to do (beyond the typos) ?
>
> BTW, there is no explicit reference in the IB spec for MC group
> create/delete trap (at least I didn't find it).
Not sure what you mean by this. What didn't you find ?
-- Hal
>
>>
>>> + 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 [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>
>
