Re: [PATCH 00/22] FCoE VN2VN fixes

[Bart Van Assche <bart.vanassche@sandisk.com>]

On 08/05/2016 12:43 AM, Hannes Reinecke wrote:
> On 08/04/2016 04:59 PM, Bart Van Assche wrote:
>> On 08/03/16 06:13, Hannes Reinecke wrote:
>>> As usual, comments and reviews are welcome.
>>
>> Great work :-) Have you been testing these patches against the LIO FCoE
>> target driver? In that case you might need to port the following patch
>> to LIO, a patch that fixes a subtle data corruption issue:
>> https://sourceforge.net/p/scst/svn/4969.
>>
> Indeed I have. With this patchset I've managed to run FCoE over virtio
> with LIO running on the host. Quite cool, methinks :-)
>
> And indeed, your patch seem to be helping with the issues I've seen
> (mysterious out-of-order frames and wireshark complaining about invalid
> frames).
> Still a bit hackish, though. Can't we move the data destructor into the
> skb itself and not free the data in the target core?

Hello Hannes,

Several LIO drivers already support sending data asynchronously to the 
initiator, e.g. the ib_srpt and qla2x00t drivers. These drivers avoid 
that struct se_cmd and its data buffer are freed before sending finished 
by passing the TARGET_SCF_ACK_KREF flag to target_submit_cmd_map_sgls() 
and by calling target_put_sess_cmd() after the data transfer has 
finished. Since the tcm_fc driver already specifies TARGET_SCF_ACK_KREF 
when submitting commands to the LIO core I think what's needed to make 
use_sg=1 safe in the tcm_fc driver is for use_sg=1 to move the 
target_put_sess_cmd() call from ft_queue_status() into a callback 
function that is called when transmitting data has finished. Maybe the 
SKBTX_DEV_ZEROCOPY flag can be used to make the network stack invoke an 
appropriate callback function. See e.g. xenvif_zerocopy_callback() in 
the xen-netback driver.

Bart.