Re: [Open-FCoE] Request for Advice from Steve Ma

Re: [Open-FCoE] Request for Advice from Steve Ma

[Steve Ma]

On Wed, Mar 3, 2010 at 10:20 AM, Joe Eykholt <jeykholt@cisco.com> wrote:
> berthiaume_wayne@emc.com wrote:
>>
>> Wouldn't it be better to pass this as part of the SCSI block so it would
>> be transport agnostic?
>
> I'm not sure I see what you mean.
>
> This is transport-specific already, where we're putting
> a FCP command together and sending it out via FCoE.   When the NIC gets the
> sk_buff
> to send, it only has the FCP command and any meta-data in the sk_buff.
>  There's no
> access to the scsi_cmnd, if that's what you mean.  Putting a pointer to that
> in
> the sk_buff would solve the problem, but seems like a layering problem.
> What am I overlooking?
>
> See more comments at end.
>
>>
>> Regards,
>> Wayne.
>> -----Original Message-----
>> From: devel-bounces@open-fcoe.org [mailto:devel-bounces@open-fcoe.org] On
>> Behalf Of Steve Ma
>> Sent: Monday, March 01, 2010 6:17 PM
>> To: Joe Eykholt
>> Cc: devel@open-fcoe.org
>> Subject: Re: [Open-FCoE] Request for Advice from Steve Ma
>>
>> On Mon, Mar 1, 2010 at 2:39 PM, Joe Eykholt <jeykholt@cisco.com> wrote:
>>>
>>> Steve Ma wrote:
>>>>
>>>> On Mon, Mar 1, 2010 at 12:08 PM, Joe Eykholt <jeykholt@cisco.com> wrote:
>>>>>
>>>>> Steve Ma wrote:
>>>>>>
>>>>>> On Mon, Mar 1, 2010 at 11:38 AM, Joe Eykholt <jeykholt@cisco.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> Steve Ma wrote:
>>>>>>>>
>>>>>>>> On Mon, Mar 1, 2010 at 10:49 AM, Joe Eykholt <jeykholt@cisco.com>
>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> Steve Ma wrote:
>>>>>>>>>>
>>>>>>>>>> On Mon, Mar 1, 2010 at 10:06 AM, Robert Love
>>>>>>>>>> <robert.w.love@intel.com>
>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>> On Mon, 2010-03-01 at 10:04 -0800, Robert Love wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> On Mon, 2010-03-01 at 09:26 -0800, Steve Ma wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Guys,
>>>>>>>>>>>>>
>>>>>>>>>>>> Hi Steve
>>>>>>>>>>>>
>>>>>>>>>>>>> When the SCSI mid-layer passes a SCSI command to the Open-FCoE,
>>>>>>>>>>>>> the
>>>>>>>>>>>>> fc_queuecomamnd receives the pointer of struct scsi_cmnd. This
>>>>>>>>>>>>> data
>>>>>>>>>>>>> structure contains two scatter-gather lists:
>>>>>>>>>>>>>
>>>>>>>>>>>>>  struct scsi_data_buffer sdb;
>>>>>>>>>>>>>  struct scsi_data_buffer *prot_sdb;
>>>>>>>>>>>>>
>>>>>>>>>>>>> The Open-FCoE stack does transmitting and receiving of sdb
>>>>>>>>>>>>> to/from
>>>>>>>>>>>>> the
>>>>>>>>>>>>> NIC driver. Now I am in need of transmitting and receiving both
>>>>>>>>>>>>> of
>>>>>>>>>>>>> them to the NIC driver and, in addition, I also need to pass
>>>>>>>>>>>>> down
>>>>>>>>>>>>> the
>>>>>>>>>>>>> return value of the mid layer routine
>>>>>>>>>>>>>
>>>>>>>>>>>>>  unsigned char scsi_get_prot_op(struct scsi_cmnd *scmd)
>>>>>>>>>>>>>
>>>>>>>>>>>>> What would be the best way to implement?
>>>>>>>>>>>>
>>>>>>>>>>>> At a high level you're going to need to go through netdev so
>>>>>>>>>>>> engaging
>>>>>>>>>>>> their mailing list would be a good start (CC'ing this list would
>>>>>>>>>>>> be
>>>>>>>>>>>> nice). For FCoE offloaded devices that are netdev based we have
>>>>>>>>>>>> these
>>>>>>>>>>>> routines in the netdev structure-
>>>>>>>>>>>
>>>>>>>>>>> This should say, "in struct net_device_ops."
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>> I have already seen those ndo routines. I may be able to make use
>>>>>>>>>> of
>>>>>>>>>> the DDP rotuines. But for transmit, I am still not sure what the
>>>>>>>>>> best
>>>>>>>>>> way to do.
>>>>>>>>>>
>>>>>>>>>> Thanks
>>>>>>>>>> Steve
>>>>>>>>>
>>>>>>>>> Is the extra buffer descriptor for DIF bytes or what?
>>>>>>>>
>>>>>>>> Yes, there are 8-bytes of protection data for DIF/DIX.
>>>>>>>>
>>>>>>>>> How would it be supported and used in a non-offloaded case?
>>>>>>>>
>>>>>>>> Currently we are intended to do this in offloaded case.
>>>>>>>> I would disable the protection if offload (LSO/DDP) is disabled.
>>>>>>>>
>>>>>>>>> I mean, what would the code look like in fc_fcp.c?
>>>>>>>>
>>>>>>>> Not sure yet. I will try to make the code as simple as possible in
>>>>>>>> the
>>>>>>>> fcoe level, and do as much work as possible in the NIC driver and
>>>>>>>> the
>>>>>>>> ASIC.
>>>>>>>>
>>>>>>>>> What should the frames look like?  Would there just be additional
>>>>>>>>> DIF bytes sent/received after every so many data bytes?  If so,
>>>>>>>>> would it work (from a protocol standpoint, maybe not efficiently)
>>>>>>>>> to just merge the sg_lists?
>>>>>>>>
>>>>>>>> I have the same thought of merging the sglists, and in the NIC
>>>>>>>> driver
>>>>>>>> if I can figure out the correct pointer to the 8-bytes at the end, I
>>>>>>>> can assign an additional TxD for it. I am not sure if I have better
>>>>>>>> choice.
>>>>>>>
>>>>>>> If the NIC driver is just going to merge the SG lists, then why
>>>>>>> not do it at the fc_fcp layer?  That way it would work for the
>>>>>>> non-offloaded case as well, and you wouldn't need any additional
>>>>>>> offload interfaces.
>>>>>>>
>>>>>>>     Cheers,
>>>>>>>     Joe
>>>>>>>
>>>>>>>
>>>>>> No, what I am thinking is to merge the two sglists in fc_fcp, thus the
>>>>>> 8-byte protection data will be part of the frame (skb) passed down to
>>>>>> the NIC driver.
>>>>>
>>>>> That's what I was suggesting.  I thought you were suggesting passing
>>>>> both SG lists separately to the NIC.  Perhaps I misunderstood.
>>>>>
>>>>>> It will be a driver for our future NIC. In the NIC driver, I will have
>>>>>> to assign TxDs for the frame data, and an additional TxD for the
>>>>>> 8-byte. So to my understanding, the NIC driver is to separate the
>>>>>> frame data and the 8-byte tag fields. I would let the ASIC to to the
>>>>>> validation job for the protection data. If there is no-offload, the
>>>>>> validation would be a big performance burden if they are done in the
>>>>>> OS driver.
>>>>>
>>>>> Wouldn't DIF need to be validated again when the data is used?
>>>>
>>>> This depends on the value returned from scsi_get_prot_op(). Either
>>>> OS<-->HBA is protected or HBA<-->Target is protected or both
>>>> (end-to-end). HBA is a checkpoint we are responsible for.
>>>>
>>>>> Otherwise other parts of the data path aren't covered ... the
>>>>> DIF wouldn't be completely end-to-end.  But verification at
>>>>> multiple points along the path is good.
>>>>>
>>>>> I was mostly thinking about sending the DIF where it's already
>>>>> been verified.  Receiving the DIF is more difficult since you
>>>>> don't know exactly how the data will be divided into frames,
>>>>> but it's the same effect: a merged SG list.
>>>>
>>>> With your support, this is the best approach I can work on.
>>>>
>>>> BTW. I want to pass the return value of scsi_get_prot_op() from fc_fcp
>>>> to the NIC driver during transmitting of a SCSI command, what would be
>>>> the best approach? It is only a byte.
>>>
>>> You could pass it to fcoe_xmit() or whatever LLD in the fcoe_rcv_info.
>>> There's only one byte free there (without stretching skb->cb[]),
>>> but you could encode it in fr_flags, which has 7 bits free.
>>> skb->cb[] can't be used for this outside of libfc/fcoe though,
>>> because it's allowed to be reused by any intermediate layers.
>>> But since skb->cb also contains the fr_fsp, you could put it in there,
>>> and call scsi_get_prot_op() from the fcoe if needed.
>>>
>>> After fcoe_xmit(), prot_op could be passed to the NIC some other way.
>>> Perhaps by appending it after the data (to be stripped off later),
>>> or using some existing field in the sk_buff.
>>>
>>> This is a long-winded way of saying "I don't know."  Someone who
>>> deals more with the guts of sk_buffs will probably suggest something.
>>>
>>>       Joe
>>>
>>>
>>>
>> Appending seems to be a clean way to do. So I will have 9 bytes in
>> total appended to the data frame.
>>
>> Thanks
>> Steve
>
> I regret suggesting appending.  I no longer thing that's such a good idea.
> If it is after the FCoE Encapsulation, then it might be OK, but we would
> have to know that the NIC will take it off.  Also, it will look wrong
> for tcpdump.
>
> Look into other ways of attaching meta-data to sk_buffs.  There might
> be something already available.
>
>        Joe
>
>
I'll keep on looking.
Thanks.
          Steve Ma