On 6/23/10 6:08 PM, Joe Eykholt wrote:
> On 6/23/10 5:04 PM, Robert Love wrote:
>> On Fri, 2010-06-18 at 10:01 -0700, Joe Eykholt wrote:
>>> When sending a FLOGI LS_ACC, which we only do in point-to-multipoint
>>> mode, the MAC descriptor should have the granted MAC set to
>>> 0x0efd00 || D_ID.
>>>
>> Can you help me understand this a bit better? I'm not sure how this
>> patch is accomplishing the above statement.
>
> When I started this patch, that was the goal,
> then I realized that the LS_ACC wasn't even getting FIP-encapsulated,
> which I fixed partly in another change.
>
> So this patch changed without updating the above comment.
> The rest of the comments are correct.
>
> Now the descriptor in the LS_ACC has a zero MAC, and I'm not
> sure that's correct. It seems unnecessary since all FCoE-encaps frames
> in VN2VN mode will go out mapped (we won't do an rport lookup for them),
> and FIP frames go to the native MAC address, so I don't know why
> we even need the MAC descriptor in this case. I'll double check
> and get back to you. Good catch.
In both the FLOGI request and the LS_ACC for the FLOGI, the MAC
descriptor should contain the mapped address of the originator.
This is in the first paragraph of section 1.5. From context, it
seems to mean the sender of the frame, i.e. originator of the sequence.
So, this is a bug, but I didn't notice it. It doesn't get used
anywhere so far.
Let me know if you have any other concerns when you get done with
the review and I'll fix this.
Joe
>>> When sending an LS_RJT, there should be no MAC descriptor.
>>>
>>> When sending either an LS_ACC or LS_RJT, the subcode should indicate
>>> an reply, not a request.
>>>
>>> Signed-off-by: Joe Eykholt<[email protected]>
>>> ---
>>> drivers/scsi/fcoe/libfcoe.c | 39
>>> +++++++++++++++++++++++++--------------
>>> 1 files changed, 25 insertions(+), 14 deletions(-)
>>>
>>>
>>> diff --git a/drivers/scsi/fcoe/libfcoe.c b/drivers/scsi/fcoe/libfcoe.c
>>> index 79df78f..8cfe9e7 100644
>>> --- a/drivers/scsi/fcoe/libfcoe.c
>>> +++ b/drivers/scsi/fcoe/libfcoe.c
>>> @@ -469,11 +469,15 @@ static int fcoe_ctlr_encaps(struct fcoe_ctlr *fip,
>>> struct fc_lport *lport,
>>> struct fip_header fip;
>>> struct fip_encaps encaps;
>>> } __attribute__((packed)) *cap;
>>> + struct fc_frame_header *fh;
>>> struct fip_mac_desc *mac;
>>> struct fcoe_fcf *fcf;
>>> size_t dlen;
>>> u16 fip_flags;
>>> + u8 op;
>>>
>>> + fh = (struct fc_frame_header *)skb->data;
>>> + op = *(u8 *)(fh + 1);
>>> dlen = sizeof(struct fip_encaps) + skb->len; /* len before push */
>>> cap = (struct fip_encaps_head *)skb_push(skb, sizeof(*cap));
>>> memset(cap, 0, sizeof(*cap));
>>> @@ -481,6 +485,7 @@ static int fcoe_ctlr_encaps(struct fcoe_ctlr *fip,
>>> struct fc_lport *lport,
>>> if (lport->point_to_multipoint) {
>>> if (fcoe_ctlr_vn_lookup(fip, d_id, cap->eth.h_dest))
>>> return -ENODEV;
>>> + fip_flags = 0;
>>> } else {
>>> fcf = fip->sel_fcf;
>>> if (!fcf)
>>> @@ -497,26 +502,32 @@ static int fcoe_ctlr_encaps(struct fcoe_ctlr *fip,
>>> struct fc_lport *lport,
>>>
>>> cap->fip.fip_ver = FIP_VER_ENCAPS(FIP_VER);
>>> cap->fip.fip_op = htons(FIP_OP_LS);
>>> - cap->fip.fip_subcode = FIP_SC_REQ;
>>> - cap->fip.fip_dl_len = htons((dlen + sizeof(*mac)) / FIP_BPW);
>>> + if (op == ELS_LS_ACC || op == ELS_LS_RJT)
>>> + cap->fip.fip_subcode = FIP_SC_REP;
>>> + else
>>> + cap->fip.fip_subcode = FIP_SC_REQ;
>>> cap->fip.fip_flags = htons(fip_flags);
>>>
>>> cap->encaps.fd_desc.fip_dtype = dtype;
>>> cap->encaps.fd_desc.fip_dlen = dlen / FIP_BPW;
>>>
>>> - mac = (struct fip_mac_desc *)skb_put(skb, sizeof(*mac));
>>> - memset(mac, 0, sizeof(*mac));
>>> - mac->fd_desc.fip_dtype = FIP_DT_MAC;
>>> - mac->fd_desc.fip_dlen = sizeof(*mac) / FIP_BPW;
>>> - if (dtype != FIP_DT_FLOGI&& dtype != FIP_DT_FDISC) {
>>> - memcpy(mac->fd_mac, fip->get_src_addr(lport), ETH_ALEN);
>>> - } else if (fip_flags& FIP_FL_SPMA) {
>>> - LIBFCOE_FIP_DBG(fip, "FLOGI/FDISC sent with SPMA\n");
>>> - memcpy(mac->fd_mac, fip->ctl_src_addr, ETH_ALEN);
>>> - } else {
>>> - LIBFCOE_FIP_DBG(fip, "FLOGI/FDISC sent with FPMA\n");
>>> - /* FPMA only FLOGI must leave the MAC desc set to all 0s */
>>> + if (op != ELS_LS_RJT) {
>>> + dlen += sizeof(*mac);
>>> + mac = (struct fip_mac_desc *)skb_put(skb, sizeof(*mac));
>>> + memset(mac, 0, sizeof(*mac));
>>> + mac->fd_desc.fip_dtype = FIP_DT_MAC;
>>> + mac->fd_desc.fip_dlen = sizeof(*mac) / FIP_BPW;
>>> + if (dtype != FIP_DT_FLOGI&& dtype != FIP_DT_FDISC) {
>>> + memcpy(mac->fd_mac, fip->get_src_addr(lport), ETH_ALEN);
>>> + } else if (fip_flags& FIP_FL_SPMA) {
>>> + LIBFCOE_FIP_DBG(fip, "FLOGI/FDISC sent with SPMA\n");
>>> + memcpy(mac->fd_mac, fip->ctl_src_addr, ETH_ALEN);
>>> + } else {
>>> + LIBFCOE_FIP_DBG(fip, "FLOGI/FDISC sent with FPMA\n");
>>> + /* FPMA only FLOGI. Must leave the MAC desc zeroed. */
>>> + }
>>> }
>>> + cap->fip.fip_dl_len = htons(dlen / FIP_BPW);
>>>
>>> skb->protocol = htons(ETH_P_FIP);
>>> skb_reset_mac_header(skb);
>>>
>>>
>>> _______________________________________________
>>> devel mailing list
>>> [email protected]
>>> http://www.open-fcoe.org/mailman/listinfo/devel
>>
>>
>
> _______________________________________________
> devel mailing list
> [email protected]
> http://www.open-fcoe.org/mailman/listinfo/devel
_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel
