Zou, Yi wrote:
>> Yi Zou wrote:
>>> We are always sending the max frame size from lport->mfs when sending the
>> FIP
>>> keep alive even there are no vport desciptors. There is no need to send the
>>> full sized frame for keep-alive.
>>>
>>> Signed-off-by: Yi Zou <[email protected]>
>>> ---
>>>
>>> drivers/scsi/fcoe/libfcoe.c | 5 +++--
>>> 1 files changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/scsi/fcoe/libfcoe.c b/drivers/scsi/fcoe/libfcoe.c
>>> index 2988b71..b9c56ff 100644
>>> --- a/drivers/scsi/fcoe/libfcoe.c
>>> +++ b/drivers/scsi/fcoe/libfcoe.c
>>> @@ -351,8 +351,9 @@ static void fcoe_ctlr_send_keep_alive(struct fcoe_ctlr
>> *fip,
>>> if (!fcf || !fc_host_port_id(lp->host))
>>> return;
>>>
>>> - len = fcoe_ctlr_fcoe_size(fip) + sizeof(struct ethhdr);
>>> - BUG_ON(len < sizeof(*kal) + sizeof(*vn));
>>> + len = sizeof(*kal) + ports * sizeof(*vn);
>>> + if (len < ETH_ZLEN)
>>> + len = ETH_ZLEN;
>>> skb = dev_alloc_skb(len);
>>> if (!skb)
>>> return;
>>>
>> I guess a preliminary version of FIP must've specified that Keep Alives were
>> padded in order to verify the fabric can still carry jumbo frames.
>> I may have just heard this in a conversation somewhere.
>> It doens't have anything to do with whether there is a port parameter or
>> vports. The BUG_ON is just paranoia to say the padding is always going
>> to give us a big enough skb.
>>
>> However, I can't find anything about padding keep-alives in the
>> current spec. It only talks about padding the solicited advertisements.
>>
>> I still think it's a good idea to pad the keep-alives.
>> Maybe we should get the spec. to state that.
>>
>> Joe
>
> I was looking at the T11/09-117v1 as the FC-BB-5 for updated FIP updated,
> where in the end of 7.7.6.2, it says "FIP_Pad field shall be of zero length
> for all other FIP operations.", where as you mentioned only the solicited
> advertisements must have the FIP pad.
>
> I will double-check on the spec tomorrow to make sure.
I agree, the spec says there should be no PAD on keep alives.
That's the same version I checked.
However, extra bytes past the end of the frame are not going to cause
a problem. Note that the descriptor length field of the
keep-alive does not include those extra bytes.
kal->fip.fip_subcode = FIP_SC_KEEP_ALIVE;
kal->fip.fip_dl_len = htons((sizeof(kal->mac) +
ports * sizeof(*vn)) / FIP_BPW);
BTW, in your change, the check against ETH_ZLEN is not necessary, the
NIC will do any padding needed to get to the minimum frame size.
Otherwise, we would have to check that in other places.
But, padding to ETH_ZLEN proves my point. It's OK to have the
extra bytes there.
Regards,
Joe
_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel
