Robert Love wrote:
> On Mon, 2009-08-10 at 12:23 -0700, Joe Eykholt wrote:
>> When an RSCN indicates changes to individual remote ports,
>> don't blindly log them out and then back in. Instead, determine
>> whether they're still in the directory, by doing GPN_ID.
>>
>> If that is successful, call login, which will send ADISC and reverify,
>> otherwise, call logoff. Perhaps we should just delete the rport,
>> not send LOGO, but it seems safer.
>>
>> Also, fix a possible issue where if a mix of records in the RSCN
>> cause us to queue disc_ports for disc_single and then we decide
>> to do full rediscovery, we leak memory for those disc_ports queued.
>>
>> So, go through the list of disc_ports even if doing full discovery.
>> Free the disc_ports in any case. If any of the disc_single() calls
>> return error, do a full discovery.
>>
>> The ability to fill in GPN_ID requests was added to fc_ct_fill().
>> For this, it needs the FC_ID to be passed in as an arg.
>> The did parameter for fc_elsct_send() is used for that, since the
>> actual D_DID will always be 0xfffffc for all CT requests so far.
>>
>> Signed-off-by: Joe Eykholt <[email protected]>
>> ---
>> drivers/scsi/libfc/fc_disc.c | 139
>> ++++++++++++++++++++++++++++++++++-------
>> drivers/scsi/libfc/fc_elsct.c | 2 -
>> include/scsi/fc_encode.h | 16 ++++-
>> 3 files changed, 130 insertions(+), 27 deletions(-)
>>
>>
>> diff --git a/drivers/scsi/libfc/fc_disc.c b/drivers/scsi/libfc/fc_disc.c
>> index 1a699f4..f694394 100644
>> --- a/drivers/scsi/libfc/fc_disc.c
>> +++ b/drivers/scsi/libfc/fc_disc.c
>> @@ -47,7 +47,7 @@ static void fc_disc_gpn_ft_req(struct fc_disc *);
>> static void fc_disc_gpn_ft_resp(struct fc_seq *, struct fc_frame *, void *);
>> static void fc_disc_done(struct fc_disc *, enum fc_disc_event);
>> static void fc_disc_timeout(struct work_struct *);
>> -static void fc_disc_single(struct fc_disc *, struct fc_disc_port *);
>> +static int fc_disc_single(struct fc_lport *, struct fc_disc_port *);
>> static void fc_disc_restart(struct fc_disc *);
>>
>> /**
>> @@ -83,7 +83,6 @@ static void fc_disc_recv_rscn_req(struct fc_seq *sp,
>> struct fc_frame *fp,
>> struct fc_disc *disc)
>> {
>> struct fc_lport *lport;
>> - struct fc_rport_priv *rdata;
>> struct fc_els_rscn *rp;
>> struct fc_els_rscn_page *pp;
>> struct fc_seq_els_data rjt_data;
>> @@ -153,6 +152,19 @@ static void fc_disc_recv_rscn_req(struct fc_seq *sp,
>> struct fc_frame *fp,
>> }
>> }
>> lport->tt.seq_els_rsp_send(sp, ELS_LS_ACC, NULL);
>> +
>> + /*
>> + * If not doing a complete rediscovery, do GPN_ID on
>> + * the individual ports mentioned in the list.
>> + * If any of these get an error, do a full rediscovery.
>> + * In any case, go through the list and free the entries.
>> + */
>> + list_for_each_entry_safe(dp, next, &disc_ports, peers) {
>> + list_del(&dp->peers);
>> + if (!redisc)
>> + redisc = fc_disc_single(lport, dp);
>> + kfree(dp);
>> + }
>> if (redisc) {
>> FC_DISC_DBG(disc, "RSCN received: rediscovering\n");
>> fc_disc_restart(disc);
>> @@ -160,14 +172,6 @@ static void fc_disc_recv_rscn_req(struct fc_seq *sp,
>> struct fc_frame *fp,
>> FC_DISC_DBG(disc, "RSCN received: not rediscovering. "
>> "redisc %d state %d in_prog %d\n",
>> redisc, lport->state, disc->pending);
>> - list_for_each_entry_safe(dp, next, &disc_ports, peers) {
>> - list_del(&dp->peers);
>> - rdata = lport->tt.rport_lookup(lport, dp->ids.port_id);
>> - if (rdata) {
>> - lport->tt.rport_logoff(rdata);
>> - }
>> - fc_disc_single(disc, dp);
>> - }
>> }
>> fc_frame_free(fp);
>> return;
>> @@ -566,32 +570,119 @@ static void fc_disc_gpn_ft_resp(struct fc_seq *sp,
>> struct fc_frame *fp,
>> }
>>
>> /**
>> - * fc_disc_single() - Discover the directory information for a single target
>> - * @lport: FC local port
>> - * @dp: The port to rediscover
>> + * fc_disc_gpn_id_resp() - Handle a response frame from Get Port Names
>> (GPN_ID)
>> + * @sp: exchange sequence
>> + * @fp: response frame
>> + * @rdata_arg: remote port private data
>> *
>> - * Locking Note: This function expects that the disc_mutex is locked
>> - * before it is called.
>> + * Locking Note: This function is called without disc mutex held.
>> */
>> -static void fc_disc_single(struct fc_disc *disc, struct fc_disc_port *dp)
>> +static void fc_disc_gpn_id_resp(struct fc_seq *sp, struct fc_frame *fp,
>> + void *rdata_arg)
>> {
>> + struct fc_rport_priv *rdata = rdata_arg;
>> + struct fc_rport_priv *new_rdata;
>> struct fc_lport *lport;
>> - struct fc_rport_priv *rdata;
>> + struct fc_disc *disc;
>> + struct fc_ct_hdr *cp;
>> + struct fc_ns_gid_pn *pn;
>> + struct fc_rport_identifiers ids;
>>
>> - lport = disc->lport;
>> + lport = rdata->local_port;
>> + disc = &lport->disc;
>>
>> - if (dp->ids.port_id == fc_host_port_id(lport->host))
>> + mutex_lock(&disc->disc_mutex);
>> + if (PTR_ERR(fp) == -FC_EX_CLOSED)
>> goto out;
>> + if (IS_ERR(fp))
>> + goto redisc;
>> +
>> + cp = fc_frame_payload_get(fp, sizeof(*cp));
>> + if (!cp)
>> + goto redisc;
>> + if (ntohs(cp->ct_cmd) == FC_FS_ACC) {
>> + if (fr_len(fp) < sizeof(struct fc_frame_header) +
>> + sizeof(*cp) + sizeof(*pn))
>> + goto redisc;
>> + pn = (struct fc_ns_gid_pn *)(cp + 1);
>> + ids.port_name = get_unaligned_be64(&pn->fn_wwpn);
>> + if (rdata->ids.port_name == -1)
>> + rdata->ids.port_name = ids.port_name;
>> + else if (rdata->ids.port_name != ids.port_name) {
>> + FC_DISC_DBG(disc, "GPN_ID accepted. WWPN changed. "
>> + "Port-id %x wwpn %llx\n",
>> + rdata->ids.port_id,
>> + (unsigned long long)ids.port_name);
>> + lport->tt.rport_logoff(rdata);
>>
>> - rdata = lport->tt.rport_create(lport, &dp->ids);
>> - if (rdata) {
>> + ids.node_name = -1;
>> + ids.roles = FC_PORT_ROLE_UNKNOWN;
>> + new_rdata = lport->tt.rport_create(lport, &ids);
>
> Hey Joe, can you help me understand the intent here?
>
> You want to pass in the port ID of the existing rport/rdata if possible
> in the ids instance, right?
Gee, it looks like ids.port_id never got set up. My mistake.
The port_id will be the same in the old and new instance.
This is so rare a case, maybe impossible; I think it's going to be hard to test.
It's checking to see whether the port_name that it had known previously
is now different in the GPN_ID response. That's really bad if it happens,
and switches are careful to allocate FC_IDs to WWNs so they don't give the
same FC_ID to a new WWPN very soon after it was in use.
We could test it by temporarily forcing it to take that path even if
the port-name matches.
Good catch!
> I've created a patch before these last two series of yours to pass the
> port_id and not the fc_rport_identifiers into rport_create(). I'm going
> through your patches and fixing merge conflicts, but I'm stuck on this
> line- I'm not sure what port_id to pass in.
Use rdata->ids.port_id.
Joe
_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel
