On Mon, Feb 8, 2010 at 11:05 AM, Eli Dorfman (Voltaire)
<[email protected]> wrote:
> Hal Rosenstock wrote:
>> On Sun, Feb 7, 2010 at 4:47 AM, Eli Dorfman (Voltaire)
>> <[email protected]> wrote:
>>> Hal Rosenstock wrote:
>>>> 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 ?
>>> I think that you referred to the comment since the code is handling this
>>> properly (in my opinion).
>>
>> I was referring to both the comment and the code since a port with a
>> compatible limited pkey should be able to receive the reports for MC
>> groups.
>
> I agree and I think that the code is handling this case properly.
> osm_physp_has_pkey() takes the 15 lower MGID pkey bits and checks whether it
> is the physp pkey table.
You're right; the code handles it. I missed the ib_pkey_get_base call there.
-- Hal
>
> Eli
>>
>>>>> 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) ?
>>> I asked Sasha to fix only the typo in commit.
>>>
>>>>> 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 ?
>>> in the spec see o13-17.1.2
>>
>> Yes, there appear to be some holes in the spec in terms of this and
>> maybe more in this area (event forwarding) but the intent seems clear.
>>
>> -- Hal
>>
>>> Thanks,
>>> Eli
>>>> -- 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
>>>>>>>
>>>
>
>
N�����r��y����b�X��ǧv�^�){.n�+����{��ٚ�{ay� ʇڙ�,j��f���h���z� �w���
���j:+v���w�j�m��������zZ+�����ݢj"��!�i
