On 4/15/2020 11:47 PM, Hannes Reinecke wrote:
...
+ case EFC_EVT_FLOGI_RCVD: {
+ struct fc_frame_header *hdr = cbdata->header->dma.virt;
+ u32 d_id = ntoh24(hdr->fh_d_id);
+
+ /* sm: / save sparams, send FLOGI acc */
+ memcpy(node->sport->domain->flogi_service_params,
+ cbdata->payload->dma.virt,
+ sizeof(struct fc_els_flogi));
+
Is the '->domain' pointer always present at this point?
Shouldn't we rather test for it before accessing?
Always - if there's a node ans sport.
+ /* send FC LS_ACC response, override s_id */
+ efc_fabric_set_topology(node, EFC_SPORT_TOPOLOGY_P2P);
+ efc->tt.send_flogi_p2p_acc(efc, node,
+ be16_to_cpu(hdr->fh_ox_id), d_id);
+ if (efc_p2p_setup(node->sport)) {
+ node_printf(node,
+ "p2p setup failed, shutting down node\n");
+ efc_node_post_event(node, EFC_EVT_SHUTDOWN, NULL);
+ } else {
+ efc_node_transition(node,
+ __efc_p2p_wait_flogi_acc_cmpl,
+ NULL);
+ }
+
+ break;
+ }
+
+ case EFC_EVT_LOGO_RCVD: {
+ struct fc_frame_header *hdr = cbdata->header->dma.virt;
+
+ if (!node->sport->domain->attached) {
+ /* most likely a frame left over from before a link
+ * down; drop and
+ * shut node down w/ "explicit logout" so pending
+ * frames are processed
+ */
Same here; I find it slightly weird to have an 'attached' field in the
domain structure; attached to what?
Doesn't the existence of the ->domain pointer signal the same thing?
If it doesn't, why don't we test for the ->domain pointer before
accessing it?
Attached means something different here. Not only does the structure
exist, but there have been VFI/VPI resources registered with the
hardware for it.
Agree with your other comments and will address them.
And the same agreement for comments on patches 3, 9 & 10.
-- james