lkml.org 
[lkml]   [2015]   [Oct]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC
    From
    Date
    On 10/29/2015 07:41 PM, Lan Tianyu wrote:
    > On 2015年10月30日 00:17, Alexander Duyck wrote:
    >> On 10/29/2015 01:33 AM, Lan Tianyu wrote:
    >>> On 2015年10月29日 14:58, Alexander Duyck wrote:
    >>>> Your code was having to do a bunch of shuffling in order to get things
    >>>> set up so that you could bring the interface back up. I would argue
    >>>> that it may actually be faster at least on the bring-up to just drop the
    >>>> old rings and start over since it greatly reduced the complexity and the
    >>>> amount of device related data that has to be moved.
    >>> If give up the old ring after migration and keep DMA running before
    >>> stopping VCPU, it seems we don't need to track Tx/Rx descriptor ring and
    >>> just make sure that all Rx buffers delivered to stack has been migrated.
    >>>
    >>> 1) Dummy write Rx buffer before checking Rx descriptor to ensure packet
    >>> migrated first.
    >> Don't dummy write the Rx descriptor. You should only really need to
    >> dummy write the Rx buffer and you would do so after checking the
    >> descriptor, not before. Otherwise you risk corrupting the Rx buffer
    >> because it is possible for you to read the Rx buffer, DMA occurs, and
    >> then you write back the Rx buffer and now you have corrupted the memory.
    >>
    >>> 2) Make a copy of Rx descriptor and then use the copied data to check
    >>> buffer status. Not use the original descriptor because it won't be
    >>> migrated and migration may happen between two access of the Rx
    >>> descriptor.
    >> Do not just blindly copy the Rx descriptor ring. That is a recipe for
    >> disaster. The problem is DMA has to happen in a very specific order for
    >> things to function correctly. The Rx buffer has to be written and then
    >> the Rx descriptor. The problem is you will end up getting a read-ahead
    >> on the Rx descriptor ring regardless of which order you dirty things in.
    >
    > Sorry, I didn't say clearly.
    > I meant to copy one Rx descriptor when receive rx irq and handle Rx ring.

    No, I understood what you are saying. My explanation was that it will
    not work.

    > Current code in the ixgbevf_clean_rx_irq() checks status of the Rx
    > descriptor whether its Rx buffer has been populated data and then read
    > the packet length from Rx descriptor to handle the Rx buffer.

    That part you have correct. However there are very explicit rules about
    the ordering of the reads.

    > My idea is to do the following three steps when receive Rx buffer in the
    > ixgbevf_clean_rx_irq().
    >
    > (1) dummy write the Rx buffer first,

    You cannot dummy write the Rx buffer without first being given ownership
    of it. In the driver this is handled in two phases. First we have to
    read the DD bit to see if it is set. If it is we can take ownership of
    the buffer. Second we have to either do a dma_sync_range_for_cpu or
    dma_unmap_page call so that we can guarantee the data has been moved to
    the buffer by the DMA API and that it knows it should no longer be
    accessing it.

    > (2) make a copy of its Rx descriptor

    This is not advisable. Unless you can guarantee you are going to only
    read the descriptor after the DD bit is set you cannot guarantee that
    you won't race with device DMA. The problem is you could have the
    migration occur right in the middle of (2). If that occurs then you
    will have valid status bits, but the rest of the descriptor would be
    invalid data.

    > (3) Check the buffer status and get length from the copy.

    I believe this is the assumption that is leading you down the wrong
    path. You would have to read the status before you could do the copy.
    You cannot do it after.

    > Migration may happen every time.
    > Happen between (1) and (2). If the Rx buffer has been populated data, VF
    > driver will not know that on the new machine because the Rx descriptor
    > isn't migrated. But it's still safe.

    The part I think you are not getting is that DMA can occur between (1)
    and (2). So if for example you were doing your dummy write while DMA
    was occurring you pull in your value, DMA occurs, you write your value
    and now you have corrupted an Rx frame by writing stale data back into it.

    > Happen between (2) and (3). The copy will be migrated to new machine
    > and Rx buffer is migrated firstly. If there is data in the Rx buffer,
    > VF driver still can handle the buffer without migrating Rx descriptor.
    >
    > The next buffers will be ignored since we don't migrate Rx descriptor
    > for them. Their status will be not completed on the new machine.

    You have kind of lost me on this part. Why do you believe there
    statuses will not be completed? How are you going to prevent the Rx
    descriptor ring from being migrated as it will be a dirty page by the
    virtue of the fact that it is a bidirectional DMA mapping where the Rx
    path provides new buffers and writes those addresses in while the device
    is writing back the status bits and length back. This is kind of what I
    was getting at. The Rx descriptor ring will show up as one of the
    dirtiest spots on the driver since it is constantly being overwritten by
    the CPU in ixgbevf_alloc_rx_buffers.

    Anyway we are kind of getting side tracked and I really think the
    solution you have proposed is kind of a dead-end.

    What we have to do is come up with a solution that can deal with the
    fact that you are racing against two different entities. You have to
    avoid racing with the device, while at the same time you have to avoid
    racing with the dirty page migration code. There are essentially 2
    problems you have to solve.

    1. Rx pages handed off to the stack must be marked as dirty. For now
    your code seemed to address this via this snippet below from patch 12/12:

    > @@ -946,15 +949,17 @@ static struct sk_buff *ixgbevf_fetch_rx_buffer(struct ixgbevf_ring *rx_ring,
    > {
    > struct ixgbevf_rx_buffer *rx_buffer;
    > struct page *page;
    > + u8 *page_addr;
    >
    > rx_buffer = &rx_ring->rx_buffer_info[rx_ring->next_to_clean];
    > page = rx_buffer->page;
    > prefetchw(page);
    >
    > - if (likely(!skb)) {
    > - void *page_addr = page_address(page) +
    > - rx_buffer->page_offset;
    > + /* Mark page dirty */
    > + page_addr = page_address(page) + rx_buffer->page_offset;
    > + *page_addr = *page_addr;
    >
    > + if (likely(!skb)) {
    > /* prefetch first cache line of first page */
    > prefetch(page_addr);
    > #if L1_CACHE_BYTES < 128

    It will work for now as a proof of concept, but I really would prefer to
    see a solution that is driver agnostic. Maybe something that could take
    care of it in the DMA API. For example if you were to use
    "swiotlb=force" in the guest this code wouldn't even be necessary since
    that forces bounce buffers which would mean your DMA mappings are dirty
    pages anyway.

    2. How to deal with a device that might be in the middle of an
    interrupt routine when you decide to migrate. This is the bit I think
    you might be focusing on a bit too much, and the current solutions you
    have proposed will result in Rx data corruption in the generic case even
    without migration. There are essentially 2 possible solutions that you
    could explore.

    2a. Have a VF device that is aware something is taking place and have
    it yield via something like a PCI hot-plug pause request. I don't know
    if the Linux kernel supports something like that now since pause support
    in the OS is optional in the PCI hot-plug specification, but essentially
    it would be a request to do a PM suspend. You would issue a hot-plug
    pause and know when it is completed by the fact that the PCI Bus Master
    bit is cleared in the VF. Then you complete the migration and in the
    new guest you could issue a hot-plug event to restart operation.

    2b. Come up with some sort of pseudo IOMMU interface the VF has to use
    to map DMA, and provide an interface to quiesce the devices attached to
    the VM so that DMA can no longer occur. Once you have disabled bus
    mastering on the VF you could then go through and migrate all DMA mapped
    pages. As far as resuming on the other side you would somehow need to
    poke the VF to get it to realize the rings are no longer initialized and
    the mailbox is out-of-sync. Once that happens the VF could reset and
    resume operation.


    \
     
     \ /
      Last update: 2015-10-30 19:21    [W:4.299 / U:3.032 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site