Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1253066 > unrolled thread
| Started by | Lan Tianyu <tianyu.lan@intel.com> |
|---|---|
| First post | 2015-10-21 19:00 +0200 |
| Last post | 2015-10-30 19:10 +0100 |
| Articles | 20 on this page of 32 — 6 participants |
Back to article view | Back to linux.kernel
[RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC Lan Tianyu <tianyu.lan@intel.com> - 2015-10-21 19:00 +0200
[RFC Patch 10/12] IXGBEVF: Add lock to protect tx/rx ring operation Lan Tianyu <tianyu.lan@intel.com> - 2015-10-21 19:00 +0200
Re: [RFC Patch 10/12] IXGBEVF: Add lock to protect tx/rx ring operation Alexander Duyck <alexander.duyck@gmail.com> - 2015-10-22 00:00 +0200
Re: [RFC Patch 10/12] IXGBEVF: Add lock to protect tx/rx ring operation "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-22 14:50 +0200
[RFC Patch 08/12] IXGBEVF: Rework code of finding the end transmit desc of package Lan Tianyu <tianyu.lan@intel.com> - 2015-10-21 19:00 +0200
Re: [RFC Patch 08/12] IXGBEVF: Rework code of finding the end transmit desc of package Alexander Duyck <alexander.duyck@gmail.com> - 2015-10-21 23:20 +0200
Re: [RFC Patch 08/12] IXGBEVF: Rework code of finding the end transmit desc of package "Lan, Tianyu" <tianyu.lan@intel.com> - 2015-10-24 18:20 +0200
Re: [RFC Patch 08/12] IXGBEVF: Rework code of finding the end transmit desc of package "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-22 15:00 +0200
Re: [RFC Patch 08/12] IXGBEVF: Rework code of finding the end transmit desc of package "Lan, Tianyu" <tianyu.lan@intel.com> - 2015-10-24 18:10 +0200
[RFC Patch 09/12] IXGBEVF: Add live migration support for VF driver Lan Tianyu <tianyu.lan@intel.com> - 2015-10-21 19:00 +0200
Re: [RFC Patch 09/12] IXGBEVF: Add live migration support for VF driver Alexander Duyck <alexander.duyck@gmail.com> - 2015-10-21 23:50 +0200
Re: [RFC Patch 09/12] IXGBEVF: Add live migration support for VF driver "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-22 14:50 +0200
Re: [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC Or Gerlitz <gerlitz.or@gmail.com> - 2015-10-21 20:50 +0200
Re: [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC Alex Williamson <alex.williamson@redhat.com> - 2015-10-21 21:30 +0200
Re: [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC Alexander Duyck <alexander.duyck@gmail.com> - 2015-10-22 01:30 +0200
Re: [Qemu-devel] [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-22 14:40 +0200
Re: [Qemu-devel] [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC Alex Williamson <alex.williamson@redhat.com> - 2015-10-22 15:10 +0200
Re: [Qemu-devel] [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-22 15:10 +0200
Re: [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC Or Gerlitz <gerlitz.or@gmail.com> - 2015-10-22 18:00 +0200
Re: [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC Alex Williamson <alex.williamson@redhat.com> - 2015-10-22 18:20 +0200
Re: [Qemu-devel] [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC "Michael S. Tsirkin" <mst@redhat.com> - 2015-10-22 15:00 +0200
Re: [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC Alexander Duyck <alexander.duyck@gmail.com> - 2015-10-23 20:40 +0200
Re: [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC Alex Williamson <alex.williamson@redhat.com> - 2015-10-23 21:10 +0200
Re: [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC Alexander Duyck <alexander.duyck@gmail.com> - 2015-10-23 22:10 +0200
Re: [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC Lan Tianyu <tianyu.lan@intel.com> - 2015-10-26 06:50 +0100
Re: [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC Alexander Duyck <alexander.duyck@gmail.com> - 2015-10-26 16:10 +0100
Re: [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC Lan Tianyu <tianyu.lan@intel.com> - 2015-10-29 07:30 +0100
Re: [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC Alexander Duyck <alexander.duyck@gmail.com> - 2015-10-29 08:00 +0100
Re: [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC Lan Tianyu <tianyu.lan@intel.com> - 2015-10-29 09:50 +0100
Re: [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC Alexander Duyck <alexander.duyck@gmail.com> - 2015-10-29 17:20 +0100
Re: [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC Lan Tianyu <tianyu.lan@intel.com> - 2015-10-30 04:00 +0100
Re: [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC Alexander Duyck <alexander.duyck@gmail.com> - 2015-10-30 19:10 +0100
Page 1 of 2 [1] 2 Next page →
| From | Lan Tianyu <tianyu.lan@intel.com> |
|---|---|
| Date | 2015-10-21 19:00 +0200 |
| Subject | [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC |
| Message-ID | <qm4Km-nt-11@gated-at.bofh.it> |
This patchset is to propose a new solution to add live migration support for 82599
SRIOV network card.
Im our solution, we prefer to put all device specific operation into VF and
PF driver and make code in the Qemu more general.
VF status migration
=================================================================
VF status can be divided into 4 parts
1) PCI configure regs
2) MSIX configure
3) VF status in the PF driver
4) VF MMIO regs
The first three status are all handled by Qemu.
The PCI configure space regs and MSIX configure are originally
stored in Qemu. To save and restore "VF status in the PF driver"
by Qemu during migration, adds new sysfs node "state_in_pf" under
VF sysfs directory.
For VF MMIO regs, we introduce self emulation layer in the VF
driver to record MMIO reg values during reading or writing MMIO
and put these data in the guest memory. It will be migrated with
guest memory to new machine.
VF function restoration
================================================================
Restoring VF function operation are done in the VF and PF driver.
In order to let VF driver to know migration status, Qemu fakes VF
PCI configure regs to indicate migration status and add new sysfs
node "notify_vf" to trigger VF mailbox irq in order to notify VF
about migration status change.
Transmit/Receive descriptor head regs are read-only and can't
be restored via writing back recording reg value directly and they
are set to 0 during VF reset. To reuse original tx/rx rings, shift
desc ring in order to move the desc pointed by original head reg to
first entry of the ring and then enable tx/rx rings. VF restarts to
receive and transmit from original head desc.
Tracking DMA accessed memory
=================================================================
Migration relies on tracking dirty page to migrate memory.
Hardware can't automatically mark a page as dirty after DMA
memory access. VF descriptor rings and data buffers are modified
by hardware when receive and transmit data. To track such dirty memory
manually, do dummy writes(read a byte and write it back) when receive
and transmit data.
Service down time test
=================================================================
So far, we tested migration between two laptops with 82599 nic which
are connected to a gigabit switch. Ping VF in the 0.001s interval
during migration on the host of source side. It service down
time is about 180ms.
[983769928.053604] 64 bytes from 10.239.48.100: icmp_seq=4131 ttl=64 time=2.79 ms
[983769928.056422] 64 bytes from 10.239.48.100: icmp_seq=4132 ttl=64 time=2.79 ms
[983769928.059241] 64 bytes from 10.239.48.100: icmp_seq=4133 ttl=64 time=2.79 ms
[983769928.062071] 64 bytes from 10.239.48.100: icmp_seq=4134 ttl=64 time=2.80 ms
[983769928.064890] 64 bytes from 10.239.48.100: icmp_seq=4135 ttl=64 time=2.79 ms
[983769928.067716] 64 bytes from 10.239.48.100: icmp_seq=4136 ttl=64 time=2.79 ms
[983769928.070538] 64 bytes from 10.239.48.100: icmp_seq=4137 ttl=64 time=2.79 ms
[983769928.073360] 64 bytes from 10.239.48.100: icmp_seq=4138 ttl=64 time=2.79 ms
[983769928.083444] no answer yet for icmp_seq=4139
[983769928.093524] no answer yet for icmp_seq=4140
[983769928.103602] no answer yet for icmp_seq=4141
[983769928.113684] no answer yet for icmp_seq=4142
[983769928.123763] no answer yet for icmp_seq=4143
[983769928.133854] no answer yet for icmp_seq=4144
[983769928.143931] no answer yet for icmp_seq=4145
[983769928.154008] no answer yet for icmp_seq=4146
[983769928.164084] no answer yet for icmp_seq=4147
[983769928.174160] no answer yet for icmp_seq=4148
[983769928.184236] no answer yet for icmp_seq=4149
[983769928.194313] no answer yet for icmp_seq=4150
[983769928.204390] no answer yet for icmp_seq=4151
[983769928.214468] no answer yet for icmp_seq=4152
[983769928.224556] no answer yet for icmp_seq=4153
[983769928.234632] no answer yet for icmp_seq=4154
[983769928.244709] no answer yet for icmp_seq=4155
[983769928.254783] no answer yet for icmp_seq=4156
[983769928.256094] 64 bytes from 10.239.48.100: icmp_seq=4139 ttl=64 time=182 ms
[983769928.256107] 64 bytes from 10.239.48.100: icmp_seq=4140 ttl=64 time=172 ms
[983769928.256114] no answer yet for icmp_seq=4157
[983769928.256236] 64 bytes from 10.239.48.100: icmp_seq=4141 ttl=64 time=162 ms
[983769928.256245] 64 bytes from 10.239.48.100: icmp_seq=4142 ttl=64 time=152 ms
[983769928.256272] 64 bytes from 10.239.48.100: icmp_seq=4143 ttl=64 time=142 ms
[983769928.256310] 64 bytes from 10.239.48.100: icmp_seq=4144 ttl=64 time=132 ms
[983769928.256325] 64 bytes from 10.239.48.100: icmp_seq=4145 ttl=64 time=122 ms
[983769928.256332] 64 bytes from 10.239.48.100: icmp_seq=4146 ttl=64 time=112 ms
[983769928.256440] 64 bytes from 10.239.48.100: icmp_seq=4147 ttl=64 time=102 ms
[983769928.256455] 64 bytes from 10.239.48.100: icmp_seq=4148 ttl=64 time=92.3 ms
[983769928.256494] 64 bytes from 10.239.48.100: icmp_seq=4149 ttl=64 time=82.3 ms
[983769928.256503] 64 bytes from 10.239.48.100: icmp_seq=4150 ttl=64 time=72.2 ms
[983769928.256631] 64 bytes from 10.239.48.100: icmp_seq=4158 ttl=64 time=0.500 ms
[983769928.257284] 64 bytes from 10.239.48.100: icmp_seq=4159 ttl=64 time=0.154 ms
[983769928.258297] 64 bytes from 10.239.48.100: icmp_seq=4160 ttl=64 time=0.165 ms
Todo
=======================================================
So far, the patchset isn't perfect. VF net interface can't be open, closed, down
and up during migration. Will prevent such operation during migration in the
future job.
Very appreciate for your comments.
Lan Tianyu (12):
PCI: Add virtfn_index for struct pci_device
IXGBE: Add new mail box event to restore VF status in the PF driver
IXGBE: Add sysfs interface for Qemu to migrate VF status in the PF
driver
IXGBE: Add ixgbe_ping_vf() to notify a specified VF via mailbox msg.
IXGBE: Add new sysfs interface of "notify_vf"
IXGBEVF: Add self emulation layer
IXGBEVF: Add new mail box event for migration
IXGBEVF: Rework code of finding the end transmit desc of package
IXGBEVF: Add live migration support for VF driver
IXGBEVF: Add lock to protect tx/rx ring operation
IXGBEVF: Migrate VF statistic data
IXGBEVF: Track dma dirty pages
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 1 +
drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h | 1 +
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 245 ++++++++++++++++++++-
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h | 1 +
drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 4 +
drivers/net/ethernet/intel/ixgbevf/Makefile | 3 +-
drivers/net/ethernet/intel/ixgbevf/defines.h | 6 +
drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 10 +-
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 179 ++++++++++++++-
drivers/net/ethernet/intel/ixgbevf/mbx.h | 3 +
.../net/ethernet/intel/ixgbevf/self-emulation.c | 133 +++++++++++
drivers/net/ethernet/intel/ixgbevf/vf.c | 10 +
drivers/net/ethernet/intel/ixgbevf/vf.h | 6 +-
drivers/pci/iov.c | 1 +
include/linux/pci.h | 1 +
15 files changed, 582 insertions(+), 22 deletions(-)
create mode 100644 drivers/net/ethernet/intel/ixgbevf/self-emulation.c
--
1.8.4.rc0.1.g8f6a3e5.dirty
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Lan Tianyu <tianyu.lan@intel.com> |
|---|---|
| Date | 2015-10-21 19:00 +0200 |
| Subject | [RFC Patch 10/12] IXGBEVF: Add lock to protect tx/rx ring operation |
| Message-ID | <qm4U4-zk-41@gated-at.bofh.it> |
| In reply to | #1253066 |
Ring shifting during restoring VF function maybe race with original
ring operation(transmit/receive package). This patch is to add tx/rx
lock to protect ring related data.
Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 2 ++
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 28 ++++++++++++++++++++---
2 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
index 6eab402e..3a748c8 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
@@ -448,6 +448,8 @@ struct ixgbevf_adapter {
spinlock_t mbx_lock;
unsigned long last_reset;
+ spinlock_t mg_rx_lock;
+ spinlock_t mg_tx_lock;
};
enum ixbgevf_state_t {
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 15ec361..04b6ce7 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -227,8 +227,10 @@ static u64 ixgbevf_get_tx_completed(struct ixgbevf_ring *ring)
int ixgbevf_tx_ring_shift(struct ixgbevf_ring *r, u32 head)
{
+ struct ixgbevf_adapter *adapter = netdev_priv(r->netdev);
struct ixgbevf_tx_buffer *tx_buffer = NULL;
static union ixgbevf_desc *tx_desc = NULL;
+ unsigned long flags;
tx_buffer = vmalloc(sizeof(struct ixgbevf_tx_buffer) * (r->count));
if (!tx_buffer)
@@ -238,6 +240,7 @@ int ixgbevf_tx_ring_shift(struct ixgbevf_ring *r, u32 head)
if (!tx_desc)
return -ENOMEM;
+ spin_lock_irqsave(&adapter->mg_tx_lock, flags);
memcpy(tx_desc, r->desc, sizeof(union ixgbevf_desc) * r->count);
memcpy(r->desc, &tx_desc[head], sizeof(union ixgbevf_desc) * (r->count - head));
memcpy(&r->desc[r->count - head], tx_desc, sizeof(union ixgbevf_desc) * head);
@@ -256,6 +259,8 @@ int ixgbevf_tx_ring_shift(struct ixgbevf_ring *r, u32 head)
else
r->next_to_use += (r->count - head);
+ spin_unlock_irqrestore(&adapter->mg_tx_lock, flags);
+
vfree(tx_buffer);
vfree(tx_desc);
return 0;
@@ -263,8 +268,10 @@ int ixgbevf_tx_ring_shift(struct ixgbevf_ring *r, u32 head)
int ixgbevf_rx_ring_shift(struct ixgbevf_ring *r, u32 head)
{
+ struct ixgbevf_adapter *adapter = netdev_priv(r->netdev);
struct ixgbevf_rx_buffer *rx_buffer = NULL;
static union ixgbevf_desc *rx_desc = NULL;
+ unsigned long flags;
rx_buffer = vmalloc(sizeof(struct ixgbevf_rx_buffer) * (r->count));
if (!rx_buffer)
@@ -274,6 +281,7 @@ int ixgbevf_rx_ring_shift(struct ixgbevf_ring *r, u32 head)
if (!rx_desc)
return -ENOMEM;
+ spin_lock_irqsave(&adapter->mg_rx_lock, flags);
memcpy(rx_desc, r->desc, sizeof(union ixgbevf_desc) * (r->count));
memcpy(r->desc, &rx_desc[head], sizeof(union ixgbevf_desc) * (r->count - head));
memcpy(&r->desc[r->count - head], rx_desc, sizeof(union ixgbevf_desc) * head);
@@ -291,6 +299,7 @@ int ixgbevf_rx_ring_shift(struct ixgbevf_ring *r, u32 head)
r->next_to_use -= head;
else
r->next_to_use += (r->count - head);
+ spin_unlock_irqrestore(&adapter->mg_rx_lock, flags);
vfree(rx_buffer);
vfree(rx_desc);
@@ -377,6 +386,8 @@ static bool ixgbevf_clean_tx_irq(struct ixgbevf_q_vector *q_vector,
if (test_bit(__IXGBEVF_DOWN, &adapter->state))
return true;
+ spin_lock(&adapter->mg_tx_lock);
+ i = tx_ring->next_to_clean;
tx_buffer = &tx_ring->tx_buffer_info[i];
tx_desc = IXGBEVF_TX_DESC(tx_ring, i);
i -= tx_ring->count;
@@ -471,6 +482,8 @@ static bool ixgbevf_clean_tx_irq(struct ixgbevf_q_vector *q_vector,
q_vector->tx.total_bytes += total_bytes;
q_vector->tx.total_packets += total_packets;
+ spin_unlock(&adapter->mg_tx_lock);
+
if (check_for_tx_hang(tx_ring) && ixgbevf_check_tx_hang(tx_ring)) {
struct ixgbe_hw *hw = &adapter->hw;
union ixgbe_adv_tx_desc *eop_desc;
@@ -999,10 +1012,12 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
struct ixgbevf_ring *rx_ring,
int budget)
{
+ struct ixgbevf_adapter *adapter = netdev_priv(rx_ring->netdev);
unsigned int total_rx_bytes = 0, total_rx_packets = 0;
u16 cleaned_count = ixgbevf_desc_unused(rx_ring);
struct sk_buff *skb = rx_ring->skb;
+ spin_lock(&adapter->mg_rx_lock);
while (likely(total_rx_packets < budget)) {
union ixgbe_adv_rx_desc *rx_desc;
@@ -1078,6 +1093,7 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
q_vector->rx.total_packets += total_rx_packets;
q_vector->rx.total_bytes += total_rx_bytes;
+ spin_unlock(&adapter->mg_rx_lock);
return total_rx_packets;
}
@@ -3572,14 +3588,17 @@ static void ixgbevf_tx_map(struct ixgbevf_ring *tx_ring,
struct ixgbevf_tx_buffer *tx_buffer;
union ixgbe_adv_tx_desc *tx_desc;
struct skb_frag_struct *frag = &skb_shinfo(skb)->frags[0];
+ struct ixgbevf_adapter *adapter = netdev_priv(tx_ring->netdev);
unsigned int data_len = skb->data_len;
unsigned int size = skb_headlen(skb);
unsigned int paylen = skb->len - hdr_len;
+ unsigned long flags;
u32 tx_flags = first->tx_flags;
__le32 cmd_type;
- u16 i = tx_ring->next_to_use;
- u16 start;
+ u16 i, start;
+ spin_lock_irqsave(&adapter->mg_tx_lock, flags);
+ i = tx_ring->next_to_use;
tx_desc = IXGBEVF_TX_DESC(tx_ring, i);
ixgbevf_tx_olinfo_status(tx_desc, tx_flags, paylen);
@@ -3673,7 +3692,7 @@ static void ixgbevf_tx_map(struct ixgbevf_ring *tx_ring,
/* notify HW of packet */
ixgbevf_write_tail(tx_ring, i);
-
+ spin_unlock_irqrestore(&adapter->mg_tx_lock, flags);
return;
dma_error:
dev_err(tx_ring->dev, "TX DMA map failed\n");
@@ -3690,6 +3709,7 @@ dma_error:
}
tx_ring->next_to_use = i;
+ spin_unlock_irqrestore(&adapter->mg_tx_lock, flags);
}
static int __ixgbevf_maybe_stop_tx(struct ixgbevf_ring *tx_ring, int size)
@@ -4188,6 +4208,8 @@ static int ixgbevf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
break;
}
+ spin_lock_init(&adapter->mg_tx_lock);
+ spin_lock_init(&adapter->mg_rx_lock);
return 0;
err_register:
--
1.8.4.rc0.1.g8f6a3e5.dirty
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Alexander Duyck <alexander.duyck@gmail.com> |
|---|---|
| Date | 2015-10-22 00:00 +0200 |
| Subject | Re: [RFC Patch 10/12] IXGBEVF: Add lock to protect tx/rx ring operation |
| Message-ID | <qm9An-7mh-23@gated-at.bofh.it> |
| In reply to | #1253067 |
On 10/21/2015 09:37 AM, Lan Tianyu wrote:
> Ring shifting during restoring VF function maybe race with original
> ring operation(transmit/receive package). This patch is to add tx/rx
> lock to protect ring related data.
>
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> ---
> drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 2 ++
> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 28 ++++++++++++++++++++---
> 2 files changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
> index 6eab402e..3a748c8 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
> @@ -448,6 +448,8 @@ struct ixgbevf_adapter {
>
> spinlock_t mbx_lock;
> unsigned long last_reset;
> + spinlock_t mg_rx_lock;
> + spinlock_t mg_tx_lock;
> };
>
Really, a shared lock for all of the Rx or Tx rings? This is going to
kill any chance at performance. Especially since just recently the VFs
got support for RSS.
To top it off it also means we cannot clean Tx while adding new buffers
which will kill Tx performance.
The other concern I have is what is supposed to prevent the hardware
from accessing the rings while you are reading? I suspect nothing so I
don't see how this helps anything.
I would honestly say you are better off just giving up on all of the
data stored in the descriptor rings rather than trying to restore them.
Yes you are going to lose a few packets but you don't have the risk
for races that this code introduces.
> enum ixbgevf_state_t {
> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> index 15ec361..04b6ce7 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> @@ -227,8 +227,10 @@ static u64 ixgbevf_get_tx_completed(struct ixgbevf_ring *ring)
>
> int ixgbevf_tx_ring_shift(struct ixgbevf_ring *r, u32 head)
> {
> + struct ixgbevf_adapter *adapter = netdev_priv(r->netdev);
> struct ixgbevf_tx_buffer *tx_buffer = NULL;
> static union ixgbevf_desc *tx_desc = NULL;
> + unsigned long flags;
>
> tx_buffer = vmalloc(sizeof(struct ixgbevf_tx_buffer) * (r->count));
> if (!tx_buffer)
> @@ -238,6 +240,7 @@ int ixgbevf_tx_ring_shift(struct ixgbevf_ring *r, u32 head)
> if (!tx_desc)
> return -ENOMEM;
>
> + spin_lock_irqsave(&adapter->mg_tx_lock, flags);
> memcpy(tx_desc, r->desc, sizeof(union ixgbevf_desc) * r->count);
> memcpy(r->desc, &tx_desc[head], sizeof(union ixgbevf_desc) * (r->count - head));
> memcpy(&r->desc[r->count - head], tx_desc, sizeof(union ixgbevf_desc) * head);
> @@ -256,6 +259,8 @@ int ixgbevf_tx_ring_shift(struct ixgbevf_ring *r, u32 head)
> else
> r->next_to_use += (r->count - head);
>
> + spin_unlock_irqrestore(&adapter->mg_tx_lock, flags);
> +
> vfree(tx_buffer);
> vfree(tx_desc);
> return 0;
> @@ -263,8 +268,10 @@ int ixgbevf_tx_ring_shift(struct ixgbevf_ring *r, u32 head)
>
> int ixgbevf_rx_ring_shift(struct ixgbevf_ring *r, u32 head)
> {
> + struct ixgbevf_adapter *adapter = netdev_priv(r->netdev);
> struct ixgbevf_rx_buffer *rx_buffer = NULL;
> static union ixgbevf_desc *rx_desc = NULL;
> + unsigned long flags;
>
> rx_buffer = vmalloc(sizeof(struct ixgbevf_rx_buffer) * (r->count));
> if (!rx_buffer)
> @@ -274,6 +281,7 @@ int ixgbevf_rx_ring_shift(struct ixgbevf_ring *r, u32 head)
> if (!rx_desc)
> return -ENOMEM;
>
> + spin_lock_irqsave(&adapter->mg_rx_lock, flags);
> memcpy(rx_desc, r->desc, sizeof(union ixgbevf_desc) * (r->count));
> memcpy(r->desc, &rx_desc[head], sizeof(union ixgbevf_desc) * (r->count - head));
> memcpy(&r->desc[r->count - head], rx_desc, sizeof(union ixgbevf_desc) * head);
> @@ -291,6 +299,7 @@ int ixgbevf_rx_ring_shift(struct ixgbevf_ring *r, u32 head)
> r->next_to_use -= head;
> else
> r->next_to_use += (r->count - head);
> + spin_unlock_irqrestore(&adapter->mg_rx_lock, flags);
>
> vfree(rx_buffer);
> vfree(rx_desc);
> @@ -377,6 +386,8 @@ static bool ixgbevf_clean_tx_irq(struct ixgbevf_q_vector *q_vector,
> if (test_bit(__IXGBEVF_DOWN, &adapter->state))
> return true;
>
> + spin_lock(&adapter->mg_tx_lock);
> + i = tx_ring->next_to_clean;
> tx_buffer = &tx_ring->tx_buffer_info[i];
> tx_desc = IXGBEVF_TX_DESC(tx_ring, i);
> i -= tx_ring->count;
> @@ -471,6 +482,8 @@ static bool ixgbevf_clean_tx_irq(struct ixgbevf_q_vector *q_vector,
> q_vector->tx.total_bytes += total_bytes;
> q_vector->tx.total_packets += total_packets;
>
> + spin_unlock(&adapter->mg_tx_lock);
> +
> if (check_for_tx_hang(tx_ring) && ixgbevf_check_tx_hang(tx_ring)) {
> struct ixgbe_hw *hw = &adapter->hw;
> union ixgbe_adv_tx_desc *eop_desc;
> @@ -999,10 +1012,12 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
> struct ixgbevf_ring *rx_ring,
> int budget)
> {
> + struct ixgbevf_adapter *adapter = netdev_priv(rx_ring->netdev);
> unsigned int total_rx_bytes = 0, total_rx_packets = 0;
> u16 cleaned_count = ixgbevf_desc_unused(rx_ring);
> struct sk_buff *skb = rx_ring->skb;
>
> + spin_lock(&adapter->mg_rx_lock);
> while (likely(total_rx_packets < budget)) {
> union ixgbe_adv_rx_desc *rx_desc;
>
> @@ -1078,6 +1093,7 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
> q_vector->rx.total_packets += total_rx_packets;
> q_vector->rx.total_bytes += total_rx_bytes;
>
> + spin_unlock(&adapter->mg_rx_lock);
> return total_rx_packets;
> }
>
> @@ -3572,14 +3588,17 @@ static void ixgbevf_tx_map(struct ixgbevf_ring *tx_ring,
> struct ixgbevf_tx_buffer *tx_buffer;
> union ixgbe_adv_tx_desc *tx_desc;
> struct skb_frag_struct *frag = &skb_shinfo(skb)->frags[0];
> + struct ixgbevf_adapter *adapter = netdev_priv(tx_ring->netdev);
> unsigned int data_len = skb->data_len;
> unsigned int size = skb_headlen(skb);
> unsigned int paylen = skb->len - hdr_len;
> + unsigned long flags;
> u32 tx_flags = first->tx_flags;
> __le32 cmd_type;
> - u16 i = tx_ring->next_to_use;
> - u16 start;
> + u16 i, start;
>
> + spin_lock_irqsave(&adapter->mg_tx_lock, flags);
> + i = tx_ring->next_to_use;
> tx_desc = IXGBEVF_TX_DESC(tx_ring, i);
>
> ixgbevf_tx_olinfo_status(tx_desc, tx_flags, paylen);
> @@ -3673,7 +3692,7 @@ static void ixgbevf_tx_map(struct ixgbevf_ring *tx_ring,
>
> /* notify HW of packet */
> ixgbevf_write_tail(tx_ring, i);
> -
> + spin_unlock_irqrestore(&adapter->mg_tx_lock, flags);
> return;
> dma_error:
> dev_err(tx_ring->dev, "TX DMA map failed\n");
> @@ -3690,6 +3709,7 @@ dma_error:
> }
>
> tx_ring->next_to_use = i;
> + spin_unlock_irqrestore(&adapter->mg_tx_lock, flags);
> }
>
> static int __ixgbevf_maybe_stop_tx(struct ixgbevf_ring *tx_ring, int size)
> @@ -4188,6 +4208,8 @@ static int ixgbevf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> break;
> }
>
> + spin_lock_init(&adapter->mg_tx_lock);
> + spin_lock_init(&adapter->mg_rx_lock);
> return 0;
>
> err_register:
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Michael S. Tsirkin" <mst@redhat.com> |
|---|---|
| Date | 2015-10-22 14:50 +0200 |
| Subject | Re: [RFC Patch 10/12] IXGBEVF: Add lock to protect tx/rx ring operation |
| Message-ID | <qmntD-2GJ-3@gated-at.bofh.it> |
| In reply to | #1253067 |
On Thu, Oct 22, 2015 at 12:37:42AM +0800, Lan Tianyu wrote:
> Ring shifting during restoring VF function maybe race with original
> ring operation(transmit/receive package). This patch is to add tx/rx
> lock to protect ring related data.
>
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
That's adding a bunch of locking on data path - what's the
performance impact?
Can't you do something faster? E.g. migration things
are slow path - can't you use something like RCU
to flush outstanding work?
> ---
> drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 2 ++
> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 28 ++++++++++++++++++++---
> 2 files changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
> index 6eab402e..3a748c8 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
> @@ -448,6 +448,8 @@ struct ixgbevf_adapter {
>
> spinlock_t mbx_lock;
> unsigned long last_reset;
> + spinlock_t mg_rx_lock;
> + spinlock_t mg_tx_lock;
> };
>
> enum ixbgevf_state_t {
> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> index 15ec361..04b6ce7 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> @@ -227,8 +227,10 @@ static u64 ixgbevf_get_tx_completed(struct ixgbevf_ring *ring)
>
> int ixgbevf_tx_ring_shift(struct ixgbevf_ring *r, u32 head)
> {
> + struct ixgbevf_adapter *adapter = netdev_priv(r->netdev);
> struct ixgbevf_tx_buffer *tx_buffer = NULL;
> static union ixgbevf_desc *tx_desc = NULL;
> + unsigned long flags;
>
> tx_buffer = vmalloc(sizeof(struct ixgbevf_tx_buffer) * (r->count));
> if (!tx_buffer)
> @@ -238,6 +240,7 @@ int ixgbevf_tx_ring_shift(struct ixgbevf_ring *r, u32 head)
> if (!tx_desc)
> return -ENOMEM;
>
> + spin_lock_irqsave(&adapter->mg_tx_lock, flags);
> memcpy(tx_desc, r->desc, sizeof(union ixgbevf_desc) * r->count);
> memcpy(r->desc, &tx_desc[head], sizeof(union ixgbevf_desc) * (r->count - head));
> memcpy(&r->desc[r->count - head], tx_desc, sizeof(union ixgbevf_desc) * head);
> @@ -256,6 +259,8 @@ int ixgbevf_tx_ring_shift(struct ixgbevf_ring *r, u32 head)
> else
> r->next_to_use += (r->count - head);
>
> + spin_unlock_irqrestore(&adapter->mg_tx_lock, flags);
> +
> vfree(tx_buffer);
> vfree(tx_desc);
> return 0;
> @@ -263,8 +268,10 @@ int ixgbevf_tx_ring_shift(struct ixgbevf_ring *r, u32 head)
>
> int ixgbevf_rx_ring_shift(struct ixgbevf_ring *r, u32 head)
> {
> + struct ixgbevf_adapter *adapter = netdev_priv(r->netdev);
> struct ixgbevf_rx_buffer *rx_buffer = NULL;
> static union ixgbevf_desc *rx_desc = NULL;
> + unsigned long flags;
>
> rx_buffer = vmalloc(sizeof(struct ixgbevf_rx_buffer) * (r->count));
> if (!rx_buffer)
> @@ -274,6 +281,7 @@ int ixgbevf_rx_ring_shift(struct ixgbevf_ring *r, u32 head)
> if (!rx_desc)
> return -ENOMEM;
>
> + spin_lock_irqsave(&adapter->mg_rx_lock, flags);
> memcpy(rx_desc, r->desc, sizeof(union ixgbevf_desc) * (r->count));
> memcpy(r->desc, &rx_desc[head], sizeof(union ixgbevf_desc) * (r->count - head));
> memcpy(&r->desc[r->count - head], rx_desc, sizeof(union ixgbevf_desc) * head);
> @@ -291,6 +299,7 @@ int ixgbevf_rx_ring_shift(struct ixgbevf_ring *r, u32 head)
> r->next_to_use -= head;
> else
> r->next_to_use += (r->count - head);
> + spin_unlock_irqrestore(&adapter->mg_rx_lock, flags);
>
> vfree(rx_buffer);
> vfree(rx_desc);
> @@ -377,6 +386,8 @@ static bool ixgbevf_clean_tx_irq(struct ixgbevf_q_vector *q_vector,
> if (test_bit(__IXGBEVF_DOWN, &adapter->state))
> return true;
>
> + spin_lock(&adapter->mg_tx_lock);
> + i = tx_ring->next_to_clean;
> tx_buffer = &tx_ring->tx_buffer_info[i];
> tx_desc = IXGBEVF_TX_DESC(tx_ring, i);
> i -= tx_ring->count;
> @@ -471,6 +482,8 @@ static bool ixgbevf_clean_tx_irq(struct ixgbevf_q_vector *q_vector,
> q_vector->tx.total_bytes += total_bytes;
> q_vector->tx.total_packets += total_packets;
>
> + spin_unlock(&adapter->mg_tx_lock);
> +
> if (check_for_tx_hang(tx_ring) && ixgbevf_check_tx_hang(tx_ring)) {
> struct ixgbe_hw *hw = &adapter->hw;
> union ixgbe_adv_tx_desc *eop_desc;
> @@ -999,10 +1012,12 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
> struct ixgbevf_ring *rx_ring,
> int budget)
> {
> + struct ixgbevf_adapter *adapter = netdev_priv(rx_ring->netdev);
> unsigned int total_rx_bytes = 0, total_rx_packets = 0;
> u16 cleaned_count = ixgbevf_desc_unused(rx_ring);
> struct sk_buff *skb = rx_ring->skb;
>
> + spin_lock(&adapter->mg_rx_lock);
> while (likely(total_rx_packets < budget)) {
> union ixgbe_adv_rx_desc *rx_desc;
>
> @@ -1078,6 +1093,7 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
> q_vector->rx.total_packets += total_rx_packets;
> q_vector->rx.total_bytes += total_rx_bytes;
>
> + spin_unlock(&adapter->mg_rx_lock);
> return total_rx_packets;
> }
>
> @@ -3572,14 +3588,17 @@ static void ixgbevf_tx_map(struct ixgbevf_ring *tx_ring,
> struct ixgbevf_tx_buffer *tx_buffer;
> union ixgbe_adv_tx_desc *tx_desc;
> struct skb_frag_struct *frag = &skb_shinfo(skb)->frags[0];
> + struct ixgbevf_adapter *adapter = netdev_priv(tx_ring->netdev);
> unsigned int data_len = skb->data_len;
> unsigned int size = skb_headlen(skb);
> unsigned int paylen = skb->len - hdr_len;
> + unsigned long flags;
> u32 tx_flags = first->tx_flags;
> __le32 cmd_type;
> - u16 i = tx_ring->next_to_use;
> - u16 start;
> + u16 i, start;
>
> + spin_lock_irqsave(&adapter->mg_tx_lock, flags);
> + i = tx_ring->next_to_use;
> tx_desc = IXGBEVF_TX_DESC(tx_ring, i);
>
> ixgbevf_tx_olinfo_status(tx_desc, tx_flags, paylen);
> @@ -3673,7 +3692,7 @@ static void ixgbevf_tx_map(struct ixgbevf_ring *tx_ring,
>
> /* notify HW of packet */
> ixgbevf_write_tail(tx_ring, i);
> -
> + spin_unlock_irqrestore(&adapter->mg_tx_lock, flags);
> return;
> dma_error:
> dev_err(tx_ring->dev, "TX DMA map failed\n");
> @@ -3690,6 +3709,7 @@ dma_error:
> }
>
> tx_ring->next_to_use = i;
> + spin_unlock_irqrestore(&adapter->mg_tx_lock, flags);
> }
>
> static int __ixgbevf_maybe_stop_tx(struct ixgbevf_ring *tx_ring, int size)
> @@ -4188,6 +4208,8 @@ static int ixgbevf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> break;
> }
>
> + spin_lock_init(&adapter->mg_tx_lock);
> + spin_lock_init(&adapter->mg_rx_lock);
> return 0;
>
> err_register:
> --
> 1.8.4.rc0.1.g8f6a3e5.dirty
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Lan Tianyu <tianyu.lan@intel.com> |
|---|---|
| Date | 2015-10-21 19:00 +0200 |
| Subject | [RFC Patch 08/12] IXGBEVF: Rework code of finding the end transmit desc of package |
| Message-ID | <qm4U4-zk-33@gated-at.bofh.it> |
| In reply to | #1253066 |
When transmit a package, the end transmit desc of package
indicates whether package is sent already. Current code records
the end desc's pointer in the next_to_watch of struct tx buffer.
This code will be broken if shifting desc ring after migration.
The pointer will be invalid. This patch is to replace recording
pointer with recording the desc number of the package and find
the end decs via the first desc and desc number.
Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 1 +
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 19 ++++++++++++++++---
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
index 775d089..c823616 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
@@ -54,6 +54,7 @@
*/
struct ixgbevf_tx_buffer {
union ixgbe_adv_tx_desc *next_to_watch;
+ u16 desc_num;
unsigned long time_stamp;
struct sk_buff *skb;
unsigned int bytecount;
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 4446916..056841c 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -210,6 +210,7 @@ static void ixgbevf_unmap_and_free_tx_resource(struct ixgbevf_ring *tx_ring,
DMA_TO_DEVICE);
}
tx_buffer->next_to_watch = NULL;
+ tx_buffer->desc_num = 0;
tx_buffer->skb = NULL;
dma_unmap_len_set(tx_buffer, len, 0);
/* tx_buffer must be completely set up in the transmit path */
@@ -295,7 +296,7 @@ static bool ixgbevf_clean_tx_irq(struct ixgbevf_q_vector *q_vector,
union ixgbe_adv_tx_desc *tx_desc;
unsigned int total_bytes = 0, total_packets = 0;
unsigned int budget = tx_ring->count / 2;
- unsigned int i = tx_ring->next_to_clean;
+ int i, watch_index;
if (test_bit(__IXGBEVF_DOWN, &adapter->state))
return true;
@@ -305,9 +306,17 @@ static bool ixgbevf_clean_tx_irq(struct ixgbevf_q_vector *q_vector,
i -= tx_ring->count;
do {
- union ixgbe_adv_tx_desc *eop_desc = tx_buffer->next_to_watch;
+ union ixgbe_adv_tx_desc *eop_desc;
+
+ if (!tx_buffer->desc_num)
+ break;
+
+ if (i + tx_buffer->desc_num >= 0)
+ watch_index = i + tx_buffer->desc_num;
+ else
+ watch_index = i + tx_ring->count + tx_buffer->desc_num;
- /* if next_to_watch is not set then there is no work pending */
+ eop_desc = IXGBEVF_TX_DESC(tx_ring, watch_index);
if (!eop_desc)
break;
@@ -320,6 +329,7 @@ static bool ixgbevf_clean_tx_irq(struct ixgbevf_q_vector *q_vector,
/* clear next_to_watch to prevent false hangs */
tx_buffer->next_to_watch = NULL;
+ tx_buffer->desc_num = 0;
/* update the statistics for this packet */
total_bytes += tx_buffer->bytecount;
@@ -3457,6 +3467,7 @@ static void ixgbevf_tx_map(struct ixgbevf_ring *tx_ring,
u32 tx_flags = first->tx_flags;
__le32 cmd_type;
u16 i = tx_ring->next_to_use;
+ u16 start;
tx_desc = IXGBEVF_TX_DESC(tx_ring, i);
@@ -3540,6 +3551,8 @@ static void ixgbevf_tx_map(struct ixgbevf_ring *tx_ring,
/* set next_to_watch value indicating a packet is present */
first->next_to_watch = tx_desc;
+ start = first - tx_ring->tx_buffer_info;
+ first->desc_num = (i - start >= 0) ? i - start: i + tx_ring->count - start;
i++;
if (i == tx_ring->count)
--
1.8.4.rc0.1.g8f6a3e5.dirty
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Alexander Duyck <alexander.duyck@gmail.com> |
|---|---|
| Date | 2015-10-21 23:20 +0200 |
| Subject | Re: [RFC Patch 08/12] IXGBEVF: Rework code of finding the end transmit desc of package |
| Message-ID | <qm8XD-6Ci-15@gated-at.bofh.it> |
| In reply to | #1253068 |
On 10/21/2015 09:37 AM, Lan Tianyu wrote:
> When transmit a package, the end transmit desc of package
> indicates whether package is sent already. Current code records
> the end desc's pointer in the next_to_watch of struct tx buffer.
> This code will be broken if shifting desc ring after migration.
> The pointer will be invalid. This patch is to replace recording
> pointer with recording the desc number of the package and find
> the end decs via the first desc and desc number.
>
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> ---
> drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 1 +
> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 19 ++++++++++++++++---
> 2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
> index 775d089..c823616 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
> @@ -54,6 +54,7 @@
> */
> struct ixgbevf_tx_buffer {
> union ixgbe_adv_tx_desc *next_to_watch;
> + u16 desc_num;
> unsigned long time_stamp;
> struct sk_buff *skb;
> unsigned int bytecount;
So if you can't use next_to_watch why is it left in here? Also you
might want to take a look at moving desc_num to a different spot in the
buffer as you are leaving a 6 byte hole in the descriptor.
> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> index 4446916..056841c 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> @@ -210,6 +210,7 @@ static void ixgbevf_unmap_and_free_tx_resource(struct ixgbevf_ring *tx_ring,
> DMA_TO_DEVICE);
> }
> tx_buffer->next_to_watch = NULL;
> + tx_buffer->desc_num = 0;
> tx_buffer->skb = NULL;
> dma_unmap_len_set(tx_buffer, len, 0);
This opens up a race condition. If you have a descriptor ready to be
cleaned at offset 0 what is to prevent you from just running through the
ring? You likely need to find a descriptor number that cannot be valid
to use here.
> /* tx_buffer must be completely set up in the transmit path */
> @@ -295,7 +296,7 @@ static bool ixgbevf_clean_tx_irq(struct ixgbevf_q_vector *q_vector,
> union ixgbe_adv_tx_desc *tx_desc;
> unsigned int total_bytes = 0, total_packets = 0;
> unsigned int budget = tx_ring->count / 2;
> - unsigned int i = tx_ring->next_to_clean;
> + int i, watch_index;
>
Where is i being initialized? It was here but you removed it. Are you
using i without initializing it?
> if (test_bit(__IXGBEVF_DOWN, &adapter->state))
> return true;
> @@ -305,9 +306,17 @@ static bool ixgbevf_clean_tx_irq(struct ixgbevf_q_vector *q_vector,
> i -= tx_ring->count;
>
> do {
> - union ixgbe_adv_tx_desc *eop_desc = tx_buffer->next_to_watch;
> + union ixgbe_adv_tx_desc *eop_desc;
> +
> + if (!tx_buffer->desc_num)
> + break;
> +
> + if (i + tx_buffer->desc_num >= 0)
> + watch_index = i + tx_buffer->desc_num;
> + else
> + watch_index = i + tx_ring->count + tx_buffer->desc_num;
>
> - /* if next_to_watch is not set then there is no work pending */
> + eop_desc = IXGBEVF_TX_DESC(tx_ring, watch_index);
> if (!eop_desc)
> break;
>
So I don't see how this isn't triggering Tx hangs. I suspect for the
simple ping case desc_num will often be 0. The fact is there are many
cases where first and tx_buffer_info are the same descriptor.
> @@ -320,6 +329,7 @@ static bool ixgbevf_clean_tx_irq(struct ixgbevf_q_vector *q_vector,
>
> /* clear next_to_watch to prevent false hangs */
> tx_buffer->next_to_watch = NULL;
> + tx_buffer->desc_num = 0;
>
> /* update the statistics for this packet */
> total_bytes += tx_buffer->bytecount;
You cannot use 0 because 0 is a valid number. You are using it as a
look-ahead currently and there are cases where i is the eop_desc index.
> @@ -3457,6 +3467,7 @@ static void ixgbevf_tx_map(struct ixgbevf_ring *tx_ring,
> u32 tx_flags = first->tx_flags;
> __le32 cmd_type;
> u16 i = tx_ring->next_to_use;
> + u16 start;
>
> tx_desc = IXGBEVF_TX_DESC(tx_ring, i);
>
> @@ -3540,6 +3551,8 @@ static void ixgbevf_tx_map(struct ixgbevf_ring *tx_ring,
>
> /* set next_to_watch value indicating a packet is present */
> first->next_to_watch = tx_desc;
> + start = first - tx_ring->tx_buffer_info;
> + first->desc_num = (i - start >= 0) ? i - start: i + tx_ring->count - start;
>
> i++;
> if (i == tx_ring->count)
start and i could be the same value. If you look at ixgbevf_tx_map you
should find that if the packet is contained in a single buffer then the
first and last descriptor in your send will be the same one.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Lan, Tianyu" <tianyu.lan@intel.com> |
|---|---|
| Date | 2015-10-24 18:20 +0200 |
| Subject | Re: [RFC Patch 08/12] IXGBEVF: Rework code of finding the end transmit desc of package |
| Message-ID | <qn9HX-5nd-3@gated-at.bofh.it> |
| In reply to | #1253253 |
On 10/22/2015 5:14 AM, Alexander Duyck wrote: > Where is i being initialized? It was here but you removed it. Are you > using i without initializing it? Sorry, the initialization was put into patch 10 by mistake. "i" is assigned with "tx_ring->next_to_clean". -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Michael S. Tsirkin" <mst@redhat.com> |
|---|---|
| Date | 2015-10-22 15:00 +0200 |
| Subject | Re: [RFC Patch 08/12] IXGBEVF: Rework code of finding the end transmit desc of package |
| Message-ID | <qmnDk-2Sv-15@gated-at.bofh.it> |
| In reply to | #1253068 |
On Thu, Oct 22, 2015 at 12:37:40AM +0800, Lan Tianyu wrote:
> When transmit a package, the end transmit desc of package
> indicates whether package is sent already. Current code records
> the end desc's pointer in the next_to_watch of struct tx buffer.
> This code will be broken if shifting desc ring after migration.
> The pointer will be invalid. This patch is to replace recording
> pointer with recording the desc number of the package and find
> the end decs via the first desc and desc number.
>
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
Do you really need to play the shifting games?
Can't you just reset everything and re-initialize the rings?
It's slower but way less intrusive.
Also removes the need to track writes into rings.
> ---
> drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 1 +
> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 19 ++++++++++++++++---
> 2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
> index 775d089..c823616 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
> @@ -54,6 +54,7 @@
> */
> struct ixgbevf_tx_buffer {
> union ixgbe_adv_tx_desc *next_to_watch;
> + u16 desc_num;
> unsigned long time_stamp;
> struct sk_buff *skb;
> unsigned int bytecount;
> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> index 4446916..056841c 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> @@ -210,6 +210,7 @@ static void ixgbevf_unmap_and_free_tx_resource(struct ixgbevf_ring *tx_ring,
> DMA_TO_DEVICE);
> }
> tx_buffer->next_to_watch = NULL;
> + tx_buffer->desc_num = 0;
> tx_buffer->skb = NULL;
> dma_unmap_len_set(tx_buffer, len, 0);
> /* tx_buffer must be completely set up in the transmit path */
> @@ -295,7 +296,7 @@ static bool ixgbevf_clean_tx_irq(struct ixgbevf_q_vector *q_vector,
> union ixgbe_adv_tx_desc *tx_desc;
> unsigned int total_bytes = 0, total_packets = 0;
> unsigned int budget = tx_ring->count / 2;
> - unsigned int i = tx_ring->next_to_clean;
> + int i, watch_index;
>
> if (test_bit(__IXGBEVF_DOWN, &adapter->state))
> return true;
> @@ -305,9 +306,17 @@ static bool ixgbevf_clean_tx_irq(struct ixgbevf_q_vector *q_vector,
> i -= tx_ring->count;
>
> do {
> - union ixgbe_adv_tx_desc *eop_desc = tx_buffer->next_to_watch;
> + union ixgbe_adv_tx_desc *eop_desc;
> +
> + if (!tx_buffer->desc_num)
> + break;
> +
> + if (i + tx_buffer->desc_num >= 0)
> + watch_index = i + tx_buffer->desc_num;
> + else
> + watch_index = i + tx_ring->count + tx_buffer->desc_num;
>
> - /* if next_to_watch is not set then there is no work pending */
> + eop_desc = IXGBEVF_TX_DESC(tx_ring, watch_index);
> if (!eop_desc)
> break;
>
> @@ -320,6 +329,7 @@ static bool ixgbevf_clean_tx_irq(struct ixgbevf_q_vector *q_vector,
>
> /* clear next_to_watch to prevent false hangs */
> tx_buffer->next_to_watch = NULL;
> + tx_buffer->desc_num = 0;
>
> /* update the statistics for this packet */
> total_bytes += tx_buffer->bytecount;
> @@ -3457,6 +3467,7 @@ static void ixgbevf_tx_map(struct ixgbevf_ring *tx_ring,
> u32 tx_flags = first->tx_flags;
> __le32 cmd_type;
> u16 i = tx_ring->next_to_use;
> + u16 start;
>
> tx_desc = IXGBEVF_TX_DESC(tx_ring, i);
>
> @@ -3540,6 +3551,8 @@ static void ixgbevf_tx_map(struct ixgbevf_ring *tx_ring,
>
> /* set next_to_watch value indicating a packet is present */
> first->next_to_watch = tx_desc;
> + start = first - tx_ring->tx_buffer_info;
> + first->desc_num = (i - start >= 0) ? i - start: i + tx_ring->count - start;
>
> i++;
> if (i == tx_ring->count)
> --
> 1.8.4.rc0.1.g8f6a3e5.dirty
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Lan, Tianyu" <tianyu.lan@intel.com> |
|---|---|
| Date | 2015-10-24 18:10 +0200 |
| Subject | Re: [RFC Patch 08/12] IXGBEVF: Rework code of finding the end transmit desc of package |
| Message-ID | <qn9yi-5bR-15@gated-at.bofh.it> |
| In reply to | #1253780 |
On 10/22/2015 8:58 PM, Michael S. Tsirkin wrote: > Do you really need to play the shifting games? > Can't you just reset everything and re-initialize the rings? > It's slower but way less intrusive. > Also removes the need to track writes into rings. Shift ring is to avoid losing those packets in the ring. This may cause some race condition and so I introduced a lock to prevent such cases in the latter patch. Yes, reset everything after migration can make thing easy. But just like you said it would affect performance and loss more packets. I can do a test later to get data about these two way. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Lan Tianyu <tianyu.lan@intel.com> |
|---|---|
| Date | 2015-10-21 19:00 +0200 |
| Subject | [RFC Patch 09/12] IXGBEVF: Add live migration support for VF driver |
| Message-ID | <qm4U4-zk-37@gated-at.bofh.it> |
| In reply to | #1253066 |
To let VF driver in the guest to know migration status, Qemu will
fake PCI configure reg 0xF0 and 0xF1 to show migrate status and
get ack from VF driver.
When migration starts, Qemu will set reg "0xF0" to 1, notify
VF driver via triggering mail box msg and wait for VF driver to tell
it's ready for migration(set reg "0xF1" to 1). After migration, Qemu
will set reg "0xF0" to 0 and notify VF driver by mail box irq. VF
driver begins to restore tx/rx function after detecting sttatus change.
When VF receives mail box irq, it will check reg "0xF0" in the service
task function to get migration status and performs related operations
according its value.
Steps of restarting receive and transmit function
1) Restore VF status in the PF driver via sending mail event to PF driver
2) Write back reg values recorded by self emulation layer
3) Restart rx/tx ring
4) Recovery interrupt
Transmit/Receive descriptor head regs are read-only and can't
be restored via writing back recording reg value directly and they
are set to 0 during VF reset. To reuse original tx/rx rings, shift
desc ring in order to move the desc pointed by original head reg to
first entry of the ring and then enable tx/rx rings. VF restarts to
receive and transmit from original head desc.
Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
drivers/net/ethernet/intel/ixgbevf/defines.h | 6 ++
drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 7 +-
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 115 ++++++++++++++++++++-
.../net/ethernet/intel/ixgbevf/self-emulation.c | 107 +++++++++++++++++++
4 files changed, 232 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbevf/defines.h b/drivers/net/ethernet/intel/ixgbevf/defines.h
index 770e21a..113efd2 100644
--- a/drivers/net/ethernet/intel/ixgbevf/defines.h
+++ b/drivers/net/ethernet/intel/ixgbevf/defines.h
@@ -239,6 +239,12 @@ struct ixgbe_adv_tx_context_desc {
__le32 mss_l4len_idx;
};
+union ixgbevf_desc {
+ union ixgbe_adv_tx_desc rx_desc;
+ union ixgbe_adv_rx_desc tx_desc;
+ struct ixgbe_adv_tx_context_desc tx_context_desc;
+};
+
/* Adv Transmit Descriptor Config Masks */
#define IXGBE_ADVTXD_DTYP_MASK 0x00F00000 /* DTYP mask */
#define IXGBE_ADVTXD_DTYP_CTXT 0x00200000 /* Advanced Context Desc */
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
index c823616..6eab402e 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
@@ -109,7 +109,7 @@ struct ixgbevf_ring {
struct ixgbevf_ring *next;
struct net_device *netdev;
struct device *dev;
- void *desc; /* descriptor ring memory */
+ union ixgbevf_desc *desc; /* descriptor ring memory */
dma_addr_t dma; /* phys. address of descriptor ring */
unsigned int size; /* length in bytes */
u16 count; /* amount of descriptors */
@@ -493,6 +493,11 @@ extern void ixgbevf_write_eitr(struct ixgbevf_q_vector *q_vector);
void ixgbe_napi_add_all(struct ixgbevf_adapter *adapter);
void ixgbe_napi_del_all(struct ixgbevf_adapter *adapter);
+int ixgbevf_tx_ring_shift(struct ixgbevf_ring *r, u32 head);
+int ixgbevf_rx_ring_shift(struct ixgbevf_ring *r, u32 head);
+void ixgbevf_restore_state(struct ixgbevf_adapter *adapter);
+inline void ixgbevf_irq_enable(struct ixgbevf_adapter *adapter);
+
#ifdef DEBUG
char *ixgbevf_get_hw_dev_name(struct ixgbe_hw *hw);
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 056841c..15ec361 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -91,6 +91,10 @@ MODULE_DESCRIPTION("Intel(R) 10 Gigabit Virtual Function Network Driver");
MODULE_LICENSE("GPL");
MODULE_VERSION(DRV_VERSION);
+
+#define MIGRATION_COMPLETED 0x00
+#define MIGRATION_IN_PROGRESS 0x01
+
#define DEFAULT_MSG_ENABLE (NETIF_MSG_DRV|NETIF_MSG_PROBE|NETIF_MSG_LINK)
static int debug = -1;
module_param(debug, int, 0);
@@ -221,6 +225,78 @@ static u64 ixgbevf_get_tx_completed(struct ixgbevf_ring *ring)
return ring->stats.packets;
}
+int ixgbevf_tx_ring_shift(struct ixgbevf_ring *r, u32 head)
+{
+ struct ixgbevf_tx_buffer *tx_buffer = NULL;
+ static union ixgbevf_desc *tx_desc = NULL;
+
+ tx_buffer = vmalloc(sizeof(struct ixgbevf_tx_buffer) * (r->count));
+ if (!tx_buffer)
+ return -ENOMEM;
+
+ tx_desc = vmalloc(sizeof(union ixgbevf_desc) * r->count);
+ if (!tx_desc)
+ return -ENOMEM;
+
+ memcpy(tx_desc, r->desc, sizeof(union ixgbevf_desc) * r->count);
+ memcpy(r->desc, &tx_desc[head], sizeof(union ixgbevf_desc) * (r->count - head));
+ memcpy(&r->desc[r->count - head], tx_desc, sizeof(union ixgbevf_desc) * head);
+
+ memcpy(tx_buffer, r->tx_buffer_info, sizeof(struct ixgbevf_tx_buffer) * r->count);
+ memcpy(r->tx_buffer_info, &tx_buffer[head], sizeof(struct ixgbevf_tx_buffer) * (r->count - head));
+ memcpy(&r->tx_buffer_info[r->count - head], tx_buffer, sizeof(struct ixgbevf_tx_buffer) * head);
+
+ if (r->next_to_clean >= head)
+ r->next_to_clean -= head;
+ else
+ r->next_to_clean += (r->count - head);
+
+ if (r->next_to_use >= head)
+ r->next_to_use -= head;
+ else
+ r->next_to_use += (r->count - head);
+
+ vfree(tx_buffer);
+ vfree(tx_desc);
+ return 0;
+}
+
+int ixgbevf_rx_ring_shift(struct ixgbevf_ring *r, u32 head)
+{
+ struct ixgbevf_rx_buffer *rx_buffer = NULL;
+ static union ixgbevf_desc *rx_desc = NULL;
+
+ rx_buffer = vmalloc(sizeof(struct ixgbevf_rx_buffer) * (r->count));
+ if (!rx_buffer)
+ return -ENOMEM;
+
+ rx_desc = vmalloc(sizeof(union ixgbevf_desc) * (r->count));
+ if (!rx_desc)
+ return -ENOMEM;
+
+ memcpy(rx_desc, r->desc, sizeof(union ixgbevf_desc) * (r->count));
+ memcpy(r->desc, &rx_desc[head], sizeof(union ixgbevf_desc) * (r->count - head));
+ memcpy(&r->desc[r->count - head], rx_desc, sizeof(union ixgbevf_desc) * head);
+
+ memcpy(rx_buffer, r->rx_buffer_info, sizeof(struct ixgbevf_rx_buffer) * (r->count));
+ memcpy(r->rx_buffer_info, &rx_buffer[head], sizeof(struct ixgbevf_rx_buffer) * (r->count - head));
+ memcpy(&r->rx_buffer_info[r->count - head], rx_buffer, sizeof(struct ixgbevf_rx_buffer) * head);
+
+ if (r->next_to_clean >= head)
+ r->next_to_clean -= head;
+ else
+ r->next_to_clean += (r->count - head);
+
+ if (r->next_to_use >= head)
+ r->next_to_use -= head;
+ else
+ r->next_to_use += (r->count - head);
+
+ vfree(rx_buffer);
+ vfree(rx_desc);
+ return 0;
+}
+
static u32 ixgbevf_get_tx_pending(struct ixgbevf_ring *ring)
{
struct ixgbevf_adapter *adapter = netdev_priv(ring->netdev);
@@ -1122,7 +1198,7 @@ static int ixgbevf_busy_poll_recv(struct napi_struct *napi)
* ixgbevf_configure_msix sets up the hardware to properly generate MSI-X
* interrupts.
**/
-static void ixgbevf_configure_msix(struct ixgbevf_adapter *adapter)
+static void ixgbevf_configure_msix(struct ixgbevf_adapter *adapter)
{
struct ixgbevf_q_vector *q_vector;
int q_vectors, v_idx;
@@ -1534,7 +1610,7 @@ static inline void ixgbevf_irq_disable(struct ixgbevf_adapter *adapter)
* ixgbevf_irq_enable - Enable default interrupt generation settings
* @adapter: board private structure
**/
-static inline void ixgbevf_irq_enable(struct ixgbevf_adapter *adapter)
+inline void ixgbevf_irq_enable(struct ixgbevf_adapter *adapter)
{
struct ixgbe_hw *hw = &adapter->hw;
@@ -2901,6 +2977,36 @@ static void ixgbevf_watchdog_subtask(struct ixgbevf_adapter *adapter)
ixgbevf_update_stats(adapter);
}
+int ixgbevf_live_mg(struct ixgbevf_adapter *adapter)
+{
+ struct pci_dev *pdev = adapter->pdev;
+ static int migration_status = MIGRATION_COMPLETED;
+ u8 val;
+
+ if (migration_status == MIGRATION_COMPLETED) {
+ pci_read_config_byte(pdev, 0xf0, &val);
+ if (!val)
+ return 0;
+
+ del_timer_sync(&adapter->service_timer);
+ pr_info("migration start\n");
+ migration_status = MIGRATION_IN_PROGRESS;
+
+ /* Tell Qemu VF is ready for migration. */
+ pci_write_config_byte(pdev, 0xf1, 0x1);
+ return 1;
+ } else {
+ pci_read_config_byte(pdev, 0xf0, &val);
+ if (val)
+ return 1;
+
+ ixgbevf_restore_state(adapter);
+ migration_status = MIGRATION_COMPLETED;
+ pr_info("migration end\n");
+ return 0;
+ }
+}
+
/**
* ixgbevf_service_task - manages and runs subtasks
* @work: pointer to work_struct containing our data
@@ -2912,6 +3018,11 @@ static void ixgbevf_service_task(struct work_struct *work)
service_task);
struct ixgbe_hw *hw = &adapter->hw;
+ if (ixgbevf_live_mg(adapter)) {
+ ixgbevf_service_event_complete(adapter);
+ return;
+ }
+
if (IXGBE_REMOVED(hw->hw_addr)) {
if (!test_bit(__IXGBEVF_DOWN, &adapter->state)) {
rtnl_lock();
diff --git a/drivers/net/ethernet/intel/ixgbevf/self-emulation.c b/drivers/net/ethernet/intel/ixgbevf/self-emulation.c
index d74b2da..4476428 100644
--- a/drivers/net/ethernet/intel/ixgbevf/self-emulation.c
+++ b/drivers/net/ethernet/intel/ixgbevf/self-emulation.c
@@ -9,6 +9,8 @@
static u32 hw_regs[0x4000];
+#define RESTORE_REG(hw, reg) IXGBE_WRITE_REG(hw, reg, hw_regs[reg])
+
u32 ixgbe_self_emul_readl(volatile void __iomem *base, u32 addr)
{
u32 tmp;
@@ -24,3 +26,108 @@ void ixgbe_self_emul_writel(u32 val, volatile void __iomem *base, u32 addr)
hw_regs[(unsigned long)addr] = val;
writel(val, (volatile void __iomem *)(base + addr));
}
+
+static u32 restore_regs[] = {
+ IXGBE_VTIVAR(0),
+ IXGBE_VTIVAR(1),
+ IXGBE_VTIVAR(2),
+ IXGBE_VTIVAR(3),
+ IXGBE_VTIVAR_MISC,
+ IXGBE_VTEITR(0),
+ IXGBE_VTEITR(1),
+ IXGBE_VFPSRTYPE,
+};
+
+void ixgbevf_restore_state(struct ixgbevf_adapter *adapter)
+{
+ struct ixgbe_hw *hw = &adapter->hw;
+ struct ixgbe_mbx_info *mbx = &hw->mbx;
+ int i;
+ u32 timeout = IXGBE_VF_INIT_TIMEOUT, rdh, tdh, rxdctl, txdctl;
+ u32 wait_loop = 10;
+
+ /* VF resetting */
+ IXGBE_WRITE_REG(hw, IXGBE_VFCTRL, IXGBE_CTRL_RST);
+ IXGBE_WRITE_FLUSH(hw);
+
+ while (!mbx->ops.check_for_rst(hw) && timeout) {
+ timeout--;
+ udelay(5);
+ }
+ if (!timeout)
+ printk(KERN_ERR "[IXGBEVF] Unable to reset VF.\n");
+
+ /* Restoring VF status in the status */
+ hw->mac.ops.notify_resume(hw);
+
+ /* Restoring regs value */
+ for (i = 0; i < sizeof(restore_regs)/sizeof(u32); i++)
+ writel(hw_regs[restore_regs[i]], (volatile void *)(restore_regs[i] + hw->hw_addr));
+
+ /* Restoring rx ring */
+ for (i = 0; i < adapter->num_rx_queues; i++) {
+ if (hw_regs[IXGBE_VFRXDCTL(i)] & IXGBE_RXDCTL_ENABLE) {
+ RESTORE_REG(hw, IXGBE_VFRDBAL(i));
+ RESTORE_REG(hw, IXGBE_VFRDBAH(i));
+ RESTORE_REG(hw, IXGBE_VFRDLEN(i));
+ RESTORE_REG(hw, IXGBE_VFDCA_RXCTRL(i));
+ RESTORE_REG(hw, IXGBE_VFSRRCTL(i));
+
+ rdh = adapter->rx_ring[i]->next_to_clean;
+ while (IXGBEVF_RX_DESC(adapter->rx_ring[i], rdh)->wb.upper.status_error
+ & cpu_to_le32(IXGBE_RXD_STAT_DD))
+ rdh = (rdh + 1) % adapter->rx_ring[i]->count;
+
+ ixgbevf_rx_ring_shift(adapter->rx_ring[i], rdh);
+
+ wait_loop = 10;
+ RESTORE_REG(hw, IXGBE_VFRXDCTL(i));
+ do {
+ udelay(10);
+ rxdctl = IXGBE_READ_REG(hw, IXGBE_VFRXDCTL(i));
+ } while (--wait_loop && !(rxdctl & IXGBE_RXDCTL_ENABLE));
+
+ if (!wait_loop)
+ pr_err("RXDCTL.ENABLE queue %d not cleared while polling\n",
+ i);
+
+ IXGBE_WRITE_REG(hw, IXGBE_VFRDT(i), adapter->rx_ring[i]->next_to_use);
+ }
+ }
+
+ /* Restoring tx ring */
+ for (i = 0; i < adapter->num_tx_queues; i++) {
+ if (hw_regs[IXGBE_VFTXDCTL(i)] & IXGBE_TXDCTL_ENABLE) {
+ RESTORE_REG(hw, IXGBE_VFTDBAL(i));
+ RESTORE_REG(hw, IXGBE_VFTDBAH(i));
+ RESTORE_REG(hw, IXGBE_VFTDLEN(i));
+ RESTORE_REG(hw, IXGBE_VFDCA_TXCTRL(i));
+
+ tdh = adapter->tx_ring[i]->next_to_clean;
+ while (IXGBEVF_TX_DESC(adapter->tx_ring[i], tdh)->wb.status
+ & cpu_to_le32(IXGBE_TXD_STAT_DD))
+ tdh = (tdh + 1) % adapter->rx_ring[i]->count;
+ ixgbevf_tx_ring_shift(adapter->tx_ring[i], tdh);
+
+ wait_loop = 10;
+ RESTORE_REG(hw, IXGBE_VFTXDCTL(i));
+ do {
+ udelay(2000);
+ txdctl = IXGBE_READ_REG(hw, IXGBE_VFTXDCTL(i));
+ } while (--wait_loop && !(txdctl & IXGBE_TXDCTL_ENABLE));
+
+ if (!wait_loop)
+ pr_err("Could not enable Tx Queue %d\n", i);
+
+ IXGBE_WRITE_REG(hw, IXGBE_VFTDT(i), adapter->tx_ring[i]->next_to_use);
+ }
+ }
+
+ /* Restore irq */
+ IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, hw_regs[IXGBE_VTEIMS] & 0x7);
+ IXGBE_WRITE_REG(hw, IXGBE_VTEIMC, (~hw_regs[IXGBE_VTEIMS]) & 0x7);
+ IXGBE_WRITE_REG(hw, IXGBE_VTEICS, hw_regs[IXGBE_VTEICS]);
+
+ ixgbevf_irq_enable(adapter);
+}
+
--
1.8.4.rc0.1.g8f6a3e5.dirty
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Alexander Duyck <alexander.duyck@gmail.com> |
|---|---|
| Date | 2015-10-21 23:50 +0200 |
| Subject | Re: [RFC Patch 09/12] IXGBEVF: Add live migration support for VF driver |
| Message-ID | <qm9qG-7aO-21@gated-at.bofh.it> |
| In reply to | #1253070 |
On 10/21/2015 09:37 AM, Lan Tianyu wrote:
> To let VF driver in the guest to know migration status, Qemu will
> fake PCI configure reg 0xF0 and 0xF1 to show migrate status and
> get ack from VF driver.
>
> When migration starts, Qemu will set reg "0xF0" to 1, notify
> VF driver via triggering mail box msg and wait for VF driver to tell
> it's ready for migration(set reg "0xF1" to 1). After migration, Qemu
> will set reg "0xF0" to 0 and notify VF driver by mail box irq. VF
> driver begins to restore tx/rx function after detecting sttatus change.
>
> When VF receives mail box irq, it will check reg "0xF0" in the service
> task function to get migration status and performs related operations
> according its value.
>
> Steps of restarting receive and transmit function
> 1) Restore VF status in the PF driver via sending mail event to PF driver
> 2) Write back reg values recorded by self emulation layer
> 3) Restart rx/tx ring
> 4) Recovery interrupt
>
> Transmit/Receive descriptor head regs are read-only and can't
> be restored via writing back recording reg value directly and they
> are set to 0 during VF reset. To reuse original tx/rx rings, shift
> desc ring in order to move the desc pointed by original head reg to
> first entry of the ring and then enable tx/rx rings. VF restarts to
> receive and transmit from original head desc.
>
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> ---
> drivers/net/ethernet/intel/ixgbevf/defines.h | 6 ++
> drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 7 +-
> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 115 ++++++++++++++++++++-
> .../net/ethernet/intel/ixgbevf/self-emulation.c | 107 +++++++++++++++++++
> 4 files changed, 232 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbevf/defines.h b/drivers/net/ethernet/intel/ixgbevf/defines.h
> index 770e21a..113efd2 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/defines.h
> +++ b/drivers/net/ethernet/intel/ixgbevf/defines.h
> @@ -239,6 +239,12 @@ struct ixgbe_adv_tx_context_desc {
> __le32 mss_l4len_idx;
> };
>
> +union ixgbevf_desc {
> + union ixgbe_adv_tx_desc rx_desc;
> + union ixgbe_adv_rx_desc tx_desc;
> + struct ixgbe_adv_tx_context_desc tx_context_desc;
> +};
> +
> /* Adv Transmit Descriptor Config Masks */
> #define IXGBE_ADVTXD_DTYP_MASK 0x00F00000 /* DTYP mask */
> #define IXGBE_ADVTXD_DTYP_CTXT 0x00200000 /* Advanced Context Desc */
> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
> index c823616..6eab402e 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
> @@ -109,7 +109,7 @@ struct ixgbevf_ring {
> struct ixgbevf_ring *next;
> struct net_device *netdev;
> struct device *dev;
> - void *desc; /* descriptor ring memory */
> + union ixgbevf_desc *desc; /* descriptor ring memory */
> dma_addr_t dma; /* phys. address of descriptor ring */
> unsigned int size; /* length in bytes */
> u16 count; /* amount of descriptors */
> @@ -493,6 +493,11 @@ extern void ixgbevf_write_eitr(struct ixgbevf_q_vector *q_vector);
>
> void ixgbe_napi_add_all(struct ixgbevf_adapter *adapter);
> void ixgbe_napi_del_all(struct ixgbevf_adapter *adapter);
> +int ixgbevf_tx_ring_shift(struct ixgbevf_ring *r, u32 head);
> +int ixgbevf_rx_ring_shift(struct ixgbevf_ring *r, u32 head);
> +void ixgbevf_restore_state(struct ixgbevf_adapter *adapter);
> +inline void ixgbevf_irq_enable(struct ixgbevf_adapter *adapter);
> +
>
> #ifdef DEBUG
> char *ixgbevf_get_hw_dev_name(struct ixgbe_hw *hw);
> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> index 056841c..15ec361 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> @@ -91,6 +91,10 @@ MODULE_DESCRIPTION("Intel(R) 10 Gigabit Virtual Function Network Driver");
> MODULE_LICENSE("GPL");
> MODULE_VERSION(DRV_VERSION);
>
> +
> +#define MIGRATION_COMPLETED 0x00
> +#define MIGRATION_IN_PROGRESS 0x01
> +
> #define DEFAULT_MSG_ENABLE (NETIF_MSG_DRV|NETIF_MSG_PROBE|NETIF_MSG_LINK)
> static int debug = -1;
> module_param(debug, int, 0);
> @@ -221,6 +225,78 @@ static u64 ixgbevf_get_tx_completed(struct ixgbevf_ring *ring)
> return ring->stats.packets;
> }
>
> +int ixgbevf_tx_ring_shift(struct ixgbevf_ring *r, u32 head)
> +{
> + struct ixgbevf_tx_buffer *tx_buffer = NULL;
> + static union ixgbevf_desc *tx_desc = NULL;
> +
> + tx_buffer = vmalloc(sizeof(struct ixgbevf_tx_buffer) * (r->count));
> + if (!tx_buffer)
> + return -ENOMEM;
> +
> + tx_desc = vmalloc(sizeof(union ixgbevf_desc) * r->count);
> + if (!tx_desc)
> + return -ENOMEM;
> +
> + memcpy(tx_desc, r->desc, sizeof(union ixgbevf_desc) * r->count);
> + memcpy(r->desc, &tx_desc[head], sizeof(union ixgbevf_desc) * (r->count - head));
> + memcpy(&r->desc[r->count - head], tx_desc, sizeof(union ixgbevf_desc) * head);
> +
> + memcpy(tx_buffer, r->tx_buffer_info, sizeof(struct ixgbevf_tx_buffer) * r->count);
> + memcpy(r->tx_buffer_info, &tx_buffer[head], sizeof(struct ixgbevf_tx_buffer) * (r->count - head));
> + memcpy(&r->tx_buffer_info[r->count - head], tx_buffer, sizeof(struct ixgbevf_tx_buffer) * head);
> +
> + if (r->next_to_clean >= head)
> + r->next_to_clean -= head;
> + else
> + r->next_to_clean += (r->count - head);
> +
> + if (r->next_to_use >= head)
> + r->next_to_use -= head;
> + else
> + r->next_to_use += (r->count - head);
> +
> + vfree(tx_buffer);
> + vfree(tx_desc);
> + return 0;
> +}
> +
> +int ixgbevf_rx_ring_shift(struct ixgbevf_ring *r, u32 head)
> +{
> + struct ixgbevf_rx_buffer *rx_buffer = NULL;
> + static union ixgbevf_desc *rx_desc = NULL;
> +
> + rx_buffer = vmalloc(sizeof(struct ixgbevf_rx_buffer) * (r->count));
> + if (!rx_buffer)
> + return -ENOMEM;
> +
> + rx_desc = vmalloc(sizeof(union ixgbevf_desc) * (r->count));
> + if (!rx_desc)
> + return -ENOMEM;
> +
> + memcpy(rx_desc, r->desc, sizeof(union ixgbevf_desc) * (r->count));
> + memcpy(r->desc, &rx_desc[head], sizeof(union ixgbevf_desc) * (r->count - head));
> + memcpy(&r->desc[r->count - head], rx_desc, sizeof(union ixgbevf_desc) * head);
> +
> + memcpy(rx_buffer, r->rx_buffer_info, sizeof(struct ixgbevf_rx_buffer) * (r->count));
> + memcpy(r->rx_buffer_info, &rx_buffer[head], sizeof(struct ixgbevf_rx_buffer) * (r->count - head));
> + memcpy(&r->rx_buffer_info[r->count - head], rx_buffer, sizeof(struct ixgbevf_rx_buffer) * head);
> +
> + if (r->next_to_clean >= head)
> + r->next_to_clean -= head;
> + else
> + r->next_to_clean += (r->count - head);
> +
> + if (r->next_to_use >= head)
> + r->next_to_use -= head;
> + else
> + r->next_to_use += (r->count - head);
> +
> + vfree(rx_buffer);
> + vfree(rx_desc);
> + return 0;
> +}
> +
> static u32 ixgbevf_get_tx_pending(struct ixgbevf_ring *ring)
> {
> struct ixgbevf_adapter *adapter = netdev_priv(ring->netdev);
> @@ -1122,7 +1198,7 @@ static int ixgbevf_busy_poll_recv(struct napi_struct *napi)
> * ixgbevf_configure_msix sets up the hardware to properly generate MSI-X
> * interrupts.
> **/
> -static void ixgbevf_configure_msix(struct ixgbevf_adapter *adapter)
> +static void ixgbevf_configure_msix(struct ixgbevf_adapter *adapter)
> {
> struct ixgbevf_q_vector *q_vector;
> int q_vectors, v_idx;
> @@ -1534,7 +1610,7 @@ static inline void ixgbevf_irq_disable(struct ixgbevf_adapter *adapter)
> * ixgbevf_irq_enable - Enable default interrupt generation settings
> * @adapter: board private structure
> **/
> -static inline void ixgbevf_irq_enable(struct ixgbevf_adapter *adapter)
> +inline void ixgbevf_irq_enable(struct ixgbevf_adapter *adapter)
> {
> struct ixgbe_hw *hw = &adapter->hw;
>
> @@ -2901,6 +2977,36 @@ static void ixgbevf_watchdog_subtask(struct ixgbevf_adapter *adapter)
> ixgbevf_update_stats(adapter);
> }
>
> +int ixgbevf_live_mg(struct ixgbevf_adapter *adapter)
> +{
> + struct pci_dev *pdev = adapter->pdev;
> + static int migration_status = MIGRATION_COMPLETED;
> + u8 val;
> +
> + if (migration_status == MIGRATION_COMPLETED) {
> + pci_read_config_byte(pdev, 0xf0, &val);
> + if (!val)
> + return 0;
> +
> + del_timer_sync(&adapter->service_timer);
> + pr_info("migration start\n");
> + migration_status = MIGRATION_IN_PROGRESS;
> +
> + /* Tell Qemu VF is ready for migration. */
> + pci_write_config_byte(pdev, 0xf1, 0x1);
> + return 1;
> + } else {
> + pci_read_config_byte(pdev, 0xf0, &val);
> + if (val)
> + return 1;
> +
> + ixgbevf_restore_state(adapter);
> + migration_status = MIGRATION_COMPLETED;
> + pr_info("migration end\n");
> + return 0;
> + }
> +}
> +
Correct me if I'm wrong but isn't migration_status going to affect all
VFs on a given system? Seems like that might be a bit racy if you were
to have a VM with more than one VF present. It seems to me
migration_status should probably be a part of the adapter or hw structs.
> /**
> * ixgbevf_service_task - manages and runs subtasks
> * @work: pointer to work_struct containing our data
> @@ -2912,6 +3018,11 @@ static void ixgbevf_service_task(struct work_struct *work)
> service_task);
> struct ixgbe_hw *hw = &adapter->hw;
>
> + if (ixgbevf_live_mg(adapter)) {
> + ixgbevf_service_event_complete(adapter);
> + return;
> + }
> +
> if (IXGBE_REMOVED(hw->hw_addr)) {
> if (!test_bit(__IXGBEVF_DOWN, &adapter->state)) {
> rtnl_lock();
> diff --git a/drivers/net/ethernet/intel/ixgbevf/self-emulation.c b/drivers/net/ethernet/intel/ixgbevf/self-emulation.c
> index d74b2da..4476428 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/self-emulation.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/self-emulation.c
> @@ -9,6 +9,8 @@
>
> static u32 hw_regs[0x4000];
>
> +#define RESTORE_REG(hw, reg) IXGBE_WRITE_REG(hw, reg, hw_regs[reg])
> +
> u32 ixgbe_self_emul_readl(volatile void __iomem *base, u32 addr)
> {
> u32 tmp;
> @@ -24,3 +26,108 @@ void ixgbe_self_emul_writel(u32 val, volatile void __iomem *base, u32 addr)
> hw_regs[(unsigned long)addr] = val;
> writel(val, (volatile void __iomem *)(base + addr));
> }
> +
> +static u32 restore_regs[] = {
> + IXGBE_VTIVAR(0),
> + IXGBE_VTIVAR(1),
> + IXGBE_VTIVAR(2),
> + IXGBE_VTIVAR(3),
> + IXGBE_VTIVAR_MISC,
> + IXGBE_VTEITR(0),
> + IXGBE_VTEITR(1),
> + IXGBE_VFPSRTYPE,
> +};
> +
Most of these registers don't need to be copied over. They can just be
configured from their existing values. For example the IVARs have a
function that already exist to configure them. You could probably get
away with just calling ixgbevf_configure_msix to restore most of that
information. Same thing for EITR and PSRTYPE. The fact is most of this
doesn't need to be saved and could just be reconfigured based on
power-on values.
> +void ixgbevf_restore_state(struct ixgbevf_adapter *adapter)
> +{
> + struct ixgbe_hw *hw = &adapter->hw;
> + struct ixgbe_mbx_info *mbx = &hw->mbx;
> + int i;
> + u32 timeout = IXGBE_VF_INIT_TIMEOUT, rdh, tdh, rxdctl, txdctl;
> + u32 wait_loop = 10;
> +
> + /* VF resetting */
> + IXGBE_WRITE_REG(hw, IXGBE_VFCTRL, IXGBE_CTRL_RST);
> + IXGBE_WRITE_FLUSH(hw);
> +
> + while (!mbx->ops.check_for_rst(hw) && timeout) {
> + timeout--;
> + udelay(5);
> + }
> + if (!timeout)
> + printk(KERN_ERR "[IXGBEVF] Unable to reset VF.\n");
> +
> + /* Restoring VF status in the status */
> + hw->mac.ops.notify_resume(hw);
> +
This seems like a recipe for putting the VF in a bad state. It seems
like if you are going to go though the process here you might was well
just call the ixgbevf_reset function and wait. Doing your own hand
coded version of the reset_hw function seems like a recipe for disaster.
> + /* Restoring regs value */
> + for (i = 0; i < sizeof(restore_regs)/sizeof(u32); i++)
> + writel(hw_regs[restore_regs[i]], (volatile void *)(restore_regs[i] + hw->hw_addr));
> +
> + /* Restoring rx ring */
> + for (i = 0; i < adapter->num_rx_queues; i++) {
> + if (hw_regs[IXGBE_VFRXDCTL(i)] & IXGBE_RXDCTL_ENABLE) {
> + RESTORE_REG(hw, IXGBE_VFRDBAL(i));
> + RESTORE_REG(hw, IXGBE_VFRDBAH(i));
> + RESTORE_REG(hw, IXGBE_VFRDLEN(i));
> + RESTORE_REG(hw, IXGBE_VFDCA_RXCTRL(i));
> + RESTORE_REG(hw, IXGBE_VFSRRCTL(i));
> +
> + rdh = adapter->rx_ring[i]->next_to_clean;
> + while (IXGBEVF_RX_DESC(adapter->rx_ring[i], rdh)->wb.upper.status_error
> + & cpu_to_le32(IXGBE_RXD_STAT_DD))
> + rdh = (rdh + 1) % adapter->rx_ring[i]->count;
> +
> + ixgbevf_rx_ring_shift(adapter->rx_ring[i], rdh);
> +
> + wait_loop = 10;
> + RESTORE_REG(hw, IXGBE_VFRXDCTL(i));
> + do {
> + udelay(10);
> + rxdctl = IXGBE_READ_REG(hw, IXGBE_VFRXDCTL(i));
> + } while (--wait_loop && !(rxdctl & IXGBE_RXDCTL_ENABLE));
> +
> + if (!wait_loop)
> + pr_err("RXDCTL.ENABLE queue %d not cleared while polling\n",
> + i);
> +
> + IXGBE_WRITE_REG(hw, IXGBE_VFRDT(i), adapter->rx_ring[i]->next_to_use);
> + }
> + }
This could probably be replaced with ixgbevf_configure_rx_ring(). You
would just need to pull out ixgbevf_alloc_rx_buffers from the call and
handle that in ixgbevf_configure_rx instead.
Also you probably don't need to check the RXDCTL_ENABLE flag, instead
you could just check for netif_running().
> + /* Restoring tx ring */
> + for (i = 0; i < adapter->num_tx_queues; i++) {
> + if (hw_regs[IXGBE_VFTXDCTL(i)] & IXGBE_TXDCTL_ENABLE) {
> + RESTORE_REG(hw, IXGBE_VFTDBAL(i));
> + RESTORE_REG(hw, IXGBE_VFTDBAH(i));
> + RESTORE_REG(hw, IXGBE_VFTDLEN(i));
> + RESTORE_REG(hw, IXGBE_VFDCA_TXCTRL(i));
> +
> + tdh = adapter->tx_ring[i]->next_to_clean;
> + while (IXGBEVF_TX_DESC(adapter->tx_ring[i], tdh)->wb.status
> + & cpu_to_le32(IXGBE_TXD_STAT_DD))
> + tdh = (tdh + 1) % adapter->rx_ring[i]->count;
> + ixgbevf_tx_ring_shift(adapter->tx_ring[i], tdh);
> +
> + wait_loop = 10;
> + RESTORE_REG(hw, IXGBE_VFTXDCTL(i));
> + do {
> + udelay(2000);
> + txdctl = IXGBE_READ_REG(hw, IXGBE_VFTXDCTL(i));
> + } while (--wait_loop && !(txdctl & IXGBE_TXDCTL_ENABLE));
> +
> + if (!wait_loop)
> + pr_err("Could not enable Tx Queue %d\n", i);
> +
> + IXGBE_WRITE_REG(hw, IXGBE_VFTDT(i), adapter->tx_ring[i]->next_to_use);
> + }
> + }
> +
Same here. You are adding a bunch of code that already exists at other
places in the driver.
> + /* Restore irq */
> + IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, hw_regs[IXGBE_VTEIMS] & 0x7);
> + IXGBE_WRITE_REG(hw, IXGBE_VTEIMC, (~hw_regs[IXGBE_VTEIMS]) & 0x7);
You might just want to clear all possible interrupts first, and then
just enable the ones already set in EIMS.
> + IXGBE_WRITE_REG(hw, IXGBE_VTEICS, hw_regs[IXGBE_VTEICS]);
As far as EICS you would probably want to just fire every interrupt once
to flush them out. No point in only enabling what is in EICS.
> + ixgbevf_irq_enable(adapter);
> +}
> +
>
Why bother with all that when you could have just called
ixgbevf_irq_enable in the first place. You end up writing the same
registers twice when you could have saved yourself the trouble and
probably just called ixgbevf_irq_enable which will already take care of
enabling everything and it will do it correctly.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Michael S. Tsirkin" <mst@redhat.com> |
|---|---|
| Date | 2015-10-22 14:50 +0200 |
| Subject | Re: [RFC Patch 09/12] IXGBEVF: Add live migration support for VF driver |
| Message-ID | <qmntE-2GJ-15@gated-at.bofh.it> |
| In reply to | #1253070 |
On Thu, Oct 22, 2015 at 12:37:41AM +0800, Lan Tianyu wrote:
> To let VF driver in the guest to know migration status, Qemu will
> fake PCI configure reg 0xF0 and 0xF1 to show migrate status and
> get ack from VF driver.
I guess this works for current devices but not using
0xF0/0xF1 registers is not architectural, is it?
So it could conflict with future devices.
Maybe it's better to just have a dedicated para-virtualized
device (PCI,ACPI,etc) for this migration-related activity.
This driver would then register with this it.
> When migration starts, Qemu will set reg "0xF0" to 1, notify
> VF driver via triggering mail box msg and wait for VF driver to tell
> it's ready for migration(set reg "0xF1" to 1).
This waiting for driver is problematic: high load is one of the reasons
people migrate VMs out. It would be much better if we could support
migration while VM is completely stopped.
> After migration, Qemu
> will set reg "0xF0" to 0 and notify VF driver by mail box irq. VF
> driver begins to restore tx/rx function after detecting sttatus change.
>
> When VF receives mail box irq, it will check reg "0xF0" in the service
> task function to get migration status and performs related operations
> according its value.
>
> Steps of restarting receive and transmit function
> 1) Restore VF status in the PF driver via sending mail event to PF driver
> 2) Write back reg values recorded by self emulation layer
> 3) Restart rx/tx ring
> 4) Recovery interrupt
>
> Transmit/Receive descriptor head regs are read-only and can't
> be restored via writing back recording reg value directly and they
> are set to 0 during VF reset. To reuse original tx/rx rings, shift
> desc ring in order to move the desc pointed by original head reg to
> first entry of the ring and then enable tx/rx rings. VF restarts to
> receive and transmit from original head desc.
>
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> ---
> drivers/net/ethernet/intel/ixgbevf/defines.h | 6 ++
> drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 7 +-
> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 115 ++++++++++++++++++++-
> .../net/ethernet/intel/ixgbevf/self-emulation.c | 107 +++++++++++++++++++
> 4 files changed, 232 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbevf/defines.h b/drivers/net/ethernet/intel/ixgbevf/defines.h
> index 770e21a..113efd2 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/defines.h
> +++ b/drivers/net/ethernet/intel/ixgbevf/defines.h
> @@ -239,6 +239,12 @@ struct ixgbe_adv_tx_context_desc {
> __le32 mss_l4len_idx;
> };
>
> +union ixgbevf_desc {
> + union ixgbe_adv_tx_desc rx_desc;
> + union ixgbe_adv_rx_desc tx_desc;
> + struct ixgbe_adv_tx_context_desc tx_context_desc;
> +};
> +
> /* Adv Transmit Descriptor Config Masks */
> #define IXGBE_ADVTXD_DTYP_MASK 0x00F00000 /* DTYP mask */
> #define IXGBE_ADVTXD_DTYP_CTXT 0x00200000 /* Advanced Context Desc */
> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
> index c823616..6eab402e 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
> @@ -109,7 +109,7 @@ struct ixgbevf_ring {
> struct ixgbevf_ring *next;
> struct net_device *netdev;
> struct device *dev;
> - void *desc; /* descriptor ring memory */
> + union ixgbevf_desc *desc; /* descriptor ring memory */
> dma_addr_t dma; /* phys. address of descriptor ring */
> unsigned int size; /* length in bytes */
> u16 count; /* amount of descriptors */
> @@ -493,6 +493,11 @@ extern void ixgbevf_write_eitr(struct ixgbevf_q_vector *q_vector);
>
> void ixgbe_napi_add_all(struct ixgbevf_adapter *adapter);
> void ixgbe_napi_del_all(struct ixgbevf_adapter *adapter);
> +int ixgbevf_tx_ring_shift(struct ixgbevf_ring *r, u32 head);
> +int ixgbevf_rx_ring_shift(struct ixgbevf_ring *r, u32 head);
> +void ixgbevf_restore_state(struct ixgbevf_adapter *adapter);
> +inline void ixgbevf_irq_enable(struct ixgbevf_adapter *adapter);
> +
>
> #ifdef DEBUG
> char *ixgbevf_get_hw_dev_name(struct ixgbe_hw *hw);
> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> index 056841c..15ec361 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> @@ -91,6 +91,10 @@ MODULE_DESCRIPTION("Intel(R) 10 Gigabit Virtual Function Network Driver");
> MODULE_LICENSE("GPL");
> MODULE_VERSION(DRV_VERSION);
>
> +
> +#define MIGRATION_COMPLETED 0x00
> +#define MIGRATION_IN_PROGRESS 0x01
> +
> #define DEFAULT_MSG_ENABLE (NETIF_MSG_DRV|NETIF_MSG_PROBE|NETIF_MSG_LINK)
> static int debug = -1;
> module_param(debug, int, 0);
> @@ -221,6 +225,78 @@ static u64 ixgbevf_get_tx_completed(struct ixgbevf_ring *ring)
> return ring->stats.packets;
> }
>
> +int ixgbevf_tx_ring_shift(struct ixgbevf_ring *r, u32 head)
> +{
> + struct ixgbevf_tx_buffer *tx_buffer = NULL;
> + static union ixgbevf_desc *tx_desc = NULL;
> +
> + tx_buffer = vmalloc(sizeof(struct ixgbevf_tx_buffer) * (r->count));
> + if (!tx_buffer)
> + return -ENOMEM;
> +
> + tx_desc = vmalloc(sizeof(union ixgbevf_desc) * r->count);
> + if (!tx_desc)
> + return -ENOMEM;
> +
> + memcpy(tx_desc, r->desc, sizeof(union ixgbevf_desc) * r->count);
> + memcpy(r->desc, &tx_desc[head], sizeof(union ixgbevf_desc) * (r->count - head));
> + memcpy(&r->desc[r->count - head], tx_desc, sizeof(union ixgbevf_desc) * head);
> +
> + memcpy(tx_buffer, r->tx_buffer_info, sizeof(struct ixgbevf_tx_buffer) * r->count);
> + memcpy(r->tx_buffer_info, &tx_buffer[head], sizeof(struct ixgbevf_tx_buffer) * (r->count - head));
> + memcpy(&r->tx_buffer_info[r->count - head], tx_buffer, sizeof(struct ixgbevf_tx_buffer) * head);
> +
> + if (r->next_to_clean >= head)
> + r->next_to_clean -= head;
> + else
> + r->next_to_clean += (r->count - head);
> +
> + if (r->next_to_use >= head)
> + r->next_to_use -= head;
> + else
> + r->next_to_use += (r->count - head);
> +
> + vfree(tx_buffer);
> + vfree(tx_desc);
> + return 0;
> +}
> +
> +int ixgbevf_rx_ring_shift(struct ixgbevf_ring *r, u32 head)
> +{
> + struct ixgbevf_rx_buffer *rx_buffer = NULL;
> + static union ixgbevf_desc *rx_desc = NULL;
> +
> + rx_buffer = vmalloc(sizeof(struct ixgbevf_rx_buffer) * (r->count));
> + if (!rx_buffer)
> + return -ENOMEM;
> +
> + rx_desc = vmalloc(sizeof(union ixgbevf_desc) * (r->count));
> + if (!rx_desc)
> + return -ENOMEM;
> +
> + memcpy(rx_desc, r->desc, sizeof(union ixgbevf_desc) * (r->count));
> + memcpy(r->desc, &rx_desc[head], sizeof(union ixgbevf_desc) * (r->count - head));
> + memcpy(&r->desc[r->count - head], rx_desc, sizeof(union ixgbevf_desc) * head);
> +
> + memcpy(rx_buffer, r->rx_buffer_info, sizeof(struct ixgbevf_rx_buffer) * (r->count));
> + memcpy(r->rx_buffer_info, &rx_buffer[head], sizeof(struct ixgbevf_rx_buffer) * (r->count - head));
> + memcpy(&r->rx_buffer_info[r->count - head], rx_buffer, sizeof(struct ixgbevf_rx_buffer) * head);
> +
> + if (r->next_to_clean >= head)
> + r->next_to_clean -= head;
> + else
> + r->next_to_clean += (r->count - head);
> +
> + if (r->next_to_use >= head)
> + r->next_to_use -= head;
> + else
> + r->next_to_use += (r->count - head);
> +
> + vfree(rx_buffer);
> + vfree(rx_desc);
> + return 0;
> +}
> +
> static u32 ixgbevf_get_tx_pending(struct ixgbevf_ring *ring)
> {
> struct ixgbevf_adapter *adapter = netdev_priv(ring->netdev);
> @@ -1122,7 +1198,7 @@ static int ixgbevf_busy_poll_recv(struct napi_struct *napi)
> * ixgbevf_configure_msix sets up the hardware to properly generate MSI-X
> * interrupts.
> **/
> -static void ixgbevf_configure_msix(struct ixgbevf_adapter *adapter)
> +static void ixgbevf_configure_msix(struct ixgbevf_adapter *adapter)
> {
> struct ixgbevf_q_vector *q_vector;
> int q_vectors, v_idx;
> @@ -1534,7 +1610,7 @@ static inline void ixgbevf_irq_disable(struct ixgbevf_adapter *adapter)
> * ixgbevf_irq_enable - Enable default interrupt generation settings
> * @adapter: board private structure
> **/
> -static inline void ixgbevf_irq_enable(struct ixgbevf_adapter *adapter)
> +inline void ixgbevf_irq_enable(struct ixgbevf_adapter *adapter)
> {
> struct ixgbe_hw *hw = &adapter->hw;
>
> @@ -2901,6 +2977,36 @@ static void ixgbevf_watchdog_subtask(struct ixgbevf_adapter *adapter)
> ixgbevf_update_stats(adapter);
> }
>
> +int ixgbevf_live_mg(struct ixgbevf_adapter *adapter)
> +{
> + struct pci_dev *pdev = adapter->pdev;
> + static int migration_status = MIGRATION_COMPLETED;
> + u8 val;
> +
> + if (migration_status == MIGRATION_COMPLETED) {
> + pci_read_config_byte(pdev, 0xf0, &val);
> + if (!val)
> + return 0;
> +
> + del_timer_sync(&adapter->service_timer);
> + pr_info("migration start\n");
> + migration_status = MIGRATION_IN_PROGRESS;
> +
> + /* Tell Qemu VF is ready for migration. */
> + pci_write_config_byte(pdev, 0xf1, 0x1);
> + return 1;
> + } else {
> + pci_read_config_byte(pdev, 0xf0, &val);
> + if (val)
> + return 1;
> +
> + ixgbevf_restore_state(adapter);
> + migration_status = MIGRATION_COMPLETED;
> + pr_info("migration end\n");
> + return 0;
> + }
> +}
> +
> /**
> * ixgbevf_service_task - manages and runs subtasks
> * @work: pointer to work_struct containing our data
> @@ -2912,6 +3018,11 @@ static void ixgbevf_service_task(struct work_struct *work)
> service_task);
> struct ixgbe_hw *hw = &adapter->hw;
>
> + if (ixgbevf_live_mg(adapter)) {
> + ixgbevf_service_event_complete(adapter);
> + return;
> + }
> +
> if (IXGBE_REMOVED(hw->hw_addr)) {
> if (!test_bit(__IXGBEVF_DOWN, &adapter->state)) {
> rtnl_lock();
> diff --git a/drivers/net/ethernet/intel/ixgbevf/self-emulation.c b/drivers/net/ethernet/intel/ixgbevf/self-emulation.c
> index d74b2da..4476428 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/self-emulation.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/self-emulation.c
> @@ -9,6 +9,8 @@
>
> static u32 hw_regs[0x4000];
>
> +#define RESTORE_REG(hw, reg) IXGBE_WRITE_REG(hw, reg, hw_regs[reg])
> +
> u32 ixgbe_self_emul_readl(volatile void __iomem *base, u32 addr)
> {
> u32 tmp;
> @@ -24,3 +26,108 @@ void ixgbe_self_emul_writel(u32 val, volatile void __iomem *base, u32 addr)
> hw_regs[(unsigned long)addr] = val;
> writel(val, (volatile void __iomem *)(base + addr));
> }
> +
> +static u32 restore_regs[] = {
> + IXGBE_VTIVAR(0),
> + IXGBE_VTIVAR(1),
> + IXGBE_VTIVAR(2),
> + IXGBE_VTIVAR(3),
> + IXGBE_VTIVAR_MISC,
> + IXGBE_VTEITR(0),
> + IXGBE_VTEITR(1),
> + IXGBE_VFPSRTYPE,
> +};
> +
> +void ixgbevf_restore_state(struct ixgbevf_adapter *adapter)
> +{
> + struct ixgbe_hw *hw = &adapter->hw;
> + struct ixgbe_mbx_info *mbx = &hw->mbx;
> + int i;
> + u32 timeout = IXGBE_VF_INIT_TIMEOUT, rdh, tdh, rxdctl, txdctl;
> + u32 wait_loop = 10;
> +
> + /* VF resetting */
> + IXGBE_WRITE_REG(hw, IXGBE_VFCTRL, IXGBE_CTRL_RST);
> + IXGBE_WRITE_FLUSH(hw);
> +
> + while (!mbx->ops.check_for_rst(hw) && timeout) {
> + timeout--;
> + udelay(5);
> + }
> + if (!timeout)
> + printk(KERN_ERR "[IXGBEVF] Unable to reset VF.\n");
> +
> + /* Restoring VF status in the status */
> + hw->mac.ops.notify_resume(hw);
> +
> + /* Restoring regs value */
> + for (i = 0; i < sizeof(restore_regs)/sizeof(u32); i++)
> + writel(hw_regs[restore_regs[i]], (volatile void *)(restore_regs[i] + hw->hw_addr));
> +
> + /* Restoring rx ring */
> + for (i = 0; i < adapter->num_rx_queues; i++) {
> + if (hw_regs[IXGBE_VFRXDCTL(i)] & IXGBE_RXDCTL_ENABLE) {
> + RESTORE_REG(hw, IXGBE_VFRDBAL(i));
> + RESTORE_REG(hw, IXGBE_VFRDBAH(i));
> + RESTORE_REG(hw, IXGBE_VFRDLEN(i));
> + RESTORE_REG(hw, IXGBE_VFDCA_RXCTRL(i));
> + RESTORE_REG(hw, IXGBE_VFSRRCTL(i));
> +
> + rdh = adapter->rx_ring[i]->next_to_clean;
> + while (IXGBEVF_RX_DESC(adapter->rx_ring[i], rdh)->wb.upper.status_error
> + & cpu_to_le32(IXGBE_RXD_STAT_DD))
> + rdh = (rdh + 1) % adapter->rx_ring[i]->count;
> +
> + ixgbevf_rx_ring_shift(adapter->rx_ring[i], rdh);
> +
> + wait_loop = 10;
> + RESTORE_REG(hw, IXGBE_VFRXDCTL(i));
> + do {
> + udelay(10);
> + rxdctl = IXGBE_READ_REG(hw, IXGBE_VFRXDCTL(i));
> + } while (--wait_loop && !(rxdctl & IXGBE_RXDCTL_ENABLE));
> +
> + if (!wait_loop)
> + pr_err("RXDCTL.ENABLE queue %d not cleared while polling\n",
> + i);
> +
> + IXGBE_WRITE_REG(hw, IXGBE_VFRDT(i), adapter->rx_ring[i]->next_to_use);
> + }
> + }
> +
> + /* Restoring tx ring */
> + for (i = 0; i < adapter->num_tx_queues; i++) {
> + if (hw_regs[IXGBE_VFTXDCTL(i)] & IXGBE_TXDCTL_ENABLE) {
> + RESTORE_REG(hw, IXGBE_VFTDBAL(i));
> + RESTORE_REG(hw, IXGBE_VFTDBAH(i));
> + RESTORE_REG(hw, IXGBE_VFTDLEN(i));
> + RESTORE_REG(hw, IXGBE_VFDCA_TXCTRL(i));
> +
> + tdh = adapter->tx_ring[i]->next_to_clean;
> + while (IXGBEVF_TX_DESC(adapter->tx_ring[i], tdh)->wb.status
> + & cpu_to_le32(IXGBE_TXD_STAT_DD))
> + tdh = (tdh + 1) % adapter->rx_ring[i]->count;
> + ixgbevf_tx_ring_shift(adapter->tx_ring[i], tdh);
> +
> + wait_loop = 10;
> + RESTORE_REG(hw, IXGBE_VFTXDCTL(i));
> + do {
> + udelay(2000);
> + txdctl = IXGBE_READ_REG(hw, IXGBE_VFTXDCTL(i));
> + } while (--wait_loop && !(txdctl & IXGBE_TXDCTL_ENABLE));
> +
> + if (!wait_loop)
> + pr_err("Could not enable Tx Queue %d\n", i);
> +
> + IXGBE_WRITE_REG(hw, IXGBE_VFTDT(i), adapter->tx_ring[i]->next_to_use);
> + }
> + }
> +
> + /* Restore irq */
> + IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, hw_regs[IXGBE_VTEIMS] & 0x7);
> + IXGBE_WRITE_REG(hw, IXGBE_VTEIMC, (~hw_regs[IXGBE_VTEIMS]) & 0x7);
> + IXGBE_WRITE_REG(hw, IXGBE_VTEICS, hw_regs[IXGBE_VTEICS]);
> +
> + ixgbevf_irq_enable(adapter);
> +}
> +
> --
> 1.8.4.rc0.1.g8f6a3e5.dirty
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Or Gerlitz <gerlitz.or@gmail.com> |
|---|---|
| Date | 2015-10-21 20:50 +0200 |
| Message-ID | <qm6Ct-346-9@gated-at.bofh.it> |
| In reply to | #1253066 |
On Wed, Oct 21, 2015 at 7:37 PM, Lan Tianyu <tianyu.lan@intel.com> wrote: > This patchset is to propose a new solution to add live migration support > for 82599 SRIOV network card. > In our solution, we prefer to put all device specific operation into VF and > PF driver and make code in the Qemu more general. [...] > Service down time test > So far, we tested migration between two laptops with 82599 nic which > are connected to a gigabit switch. Ping VF in the 0.001s interval > during migration on the host of source side. It service down > time is about 180ms. So... what would you expect service down wise for the following solution which is zero touch and I think should work for any VF driver: on host A: unplug the VM and conduct live migration to host B ala the no-SRIOV case. on host B: when the VM "gets back to live", probe a VF there with the same assigned mac next, udev on the VM will call the VF driver to create netdev instance DHCP client would run to get the same IP address + under config directive (or from Qemu) send Gratuitous ARP to notify the switch/es on the new location for that mac. Or. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Alex Williamson <alex.williamson@redhat.com> |
|---|---|
| Date | 2015-10-21 21:30 +0200 |
| Subject | Re: [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC |
| Message-ID | <qm7fd-44l-27@gated-at.bofh.it> |
| In reply to | #1253120 |
On Wed, 2015-10-21 at 21:45 +0300, Or Gerlitz wrote: > On Wed, Oct 21, 2015 at 7:37 PM, Lan Tianyu <tianyu.lan@intel.com> wrote: > > This patchset is to propose a new solution to add live migration support > > for 82599 SRIOV network card. > > > In our solution, we prefer to put all device specific operation into VF and > > PF driver and make code in the Qemu more general. > > [...] > > > Service down time test > > So far, we tested migration between two laptops with 82599 nic which > > are connected to a gigabit switch. Ping VF in the 0.001s interval > > during migration on the host of source side. It service down > > time is about 180ms. > > So... what would you expect service down wise for the following > solution which is zero touch and I think should work for any VF > driver: > > on host A: unplug the VM and conduct live migration to host B ala the > no-SRIOV case. The trouble here is that the VF needs to be unplugged prior to the start of migration because we can't do effective dirty page tracking while the device is connected and doing DMA. So the downtime, assuming we're counting only VF connectivity, is dependent on memory size, rate of dirtying, and network bandwidth; seconds for small guests, minutes or more (maybe much, much more) for large guests. This is why the typical VF agnostic approach here is to using bonding and fail over to a emulated device during migration, so performance suffers, but downtime is something acceptable. If we want the ability to defer the VF unplug until just before the final stages of the migration, we need the VF to participate in dirty page tracking. Here it's done via an enlightened guest driver. Alex Graf presented a solution using a device specific enlightenment in QEMU. Otherwise we'd need hardware support from the IOMMU. Thanks, Alex > on host B: > > when the VM "gets back to live", probe a VF there with the same assigned mac > > next, udev on the VM will call the VF driver to create netdev instance > > DHCP client would run to get the same IP address > > + under config directive (or from Qemu) send Gratuitous ARP to notify > the switch/es on the new location for that mac. > > Or. > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Alexander Duyck <alexander.duyck@gmail.com> |
|---|---|
| Date | 2015-10-22 01:30 +0200 |
| Message-ID | <qmaZr-12q-11@gated-at.bofh.it> |
| In reply to | #1253148 |
On 10/21/2015 12:20 PM, Alex Williamson wrote:
> On Wed, 2015-10-21 at 21:45 +0300, Or Gerlitz wrote:
>> On Wed, Oct 21, 2015 at 7:37 PM, Lan Tianyu <tianyu.lan@intel.com> wrote:
>>> This patchset is to propose a new solution to add live migration support
>>> for 82599 SRIOV network card.
>>
>>> In our solution, we prefer to put all device specific operation into VF and
>>> PF driver and make code in the Qemu more general.
>>
>> [...]
>>
>>> Service down time test
>>> So far, we tested migration between two laptops with 82599 nic which
>>> are connected to a gigabit switch. Ping VF in the 0.001s interval
>>> during migration on the host of source side. It service down
>>> time is about 180ms.
>>
>> So... what would you expect service down wise for the following
>> solution which is zero touch and I think should work for any VF
>> driver:
>>
>> on host A: unplug the VM and conduct live migration to host B ala the
>> no-SRIOV case.
>
> The trouble here is that the VF needs to be unplugged prior to the start
> of migration because we can't do effective dirty page tracking while the
> device is connected and doing DMA. So the downtime, assuming we're
> counting only VF connectivity, is dependent on memory size, rate of
> dirtying, and network bandwidth; seconds for small guests, minutes or
> more (maybe much, much more) for large guests.
The question of dirty page tracking though should be pretty simple. We
start the Tx packets out as dirty so we don't need to add anything
there. It seems like the Rx data and Tx/Rx descriptor rings are the issue.
> This is why the typical VF agnostic approach here is to using bonding
> and fail over to a emulated device during migration, so performance
> suffers, but downtime is something acceptable.
>
> If we want the ability to defer the VF unplug until just before the
> final stages of the migration, we need the VF to participate in dirty
> page tracking. Here it's done via an enlightened guest driver. Alex
> Graf presented a solution using a device specific enlightenment in QEMU.
> Otherwise we'd need hardware support from the IOMMU.
My only real complaint with this patch series is that it seems like
there was to much focus on instrumenting the driver instead of providing
the code necessary to enable a driver ecosystem that enables migration.
I don't know if what we need is a full hardware IOMMU. It seems like a
good way to take care of the need to flag dirty pages for DMA capable
devices would be to add functionality to the dma_map_ops calls
sync_{sg|single}for_cpu and unmap_{page|sg} so that they would take care
of mapping the pages as dirty for us when needed. We could probably
make do with just a few tweaks to existing API in order to make this work.
As far as the descriptor rings I would argue they are invalid as soon as
we migrate. The problem is there is no way to guarantee ordering as we
cannot pre-emptively mark an Rx data buffer as being a dirty page when
we haven't even looked at the Rx descriptor for the given buffer yet.
Tx has similar issues as we cannot guarantee the Tx will disable itself
after a complete frame. As such I would say the moment we migrate we
should just give up on the frames that are still in the descriptor
rings, drop them, and then start over with fresh rings.
- Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Michael S. Tsirkin" <mst@redhat.com> |
|---|---|
| Date | 2015-10-22 14:40 +0200 |
| Subject | Re: [Qemu-devel] [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC |
| Message-ID | <qmnjY-2vq-19@gated-at.bofh.it> |
| In reply to | #1253148 |
On Wed, Oct 21, 2015 at 01:20:27PM -0600, Alex Williamson wrote: > The trouble here is that the VF needs to be unplugged prior to the start > of migration because we can't do effective dirty page tracking while the > device is connected and doing DMA. That's exactly what patch 12/12 is trying to accomplish. I do see some problems with it, but I also suggested some solutions. -- MST -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Alex Williamson <alex.williamson@redhat.com> |
|---|---|
| Date | 2015-10-22 15:10 +0200 |
| Subject | Re: [Qemu-devel] [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC |
| Message-ID | <qmnMZ-3jD-1@gated-at.bofh.it> |
| In reply to | #1253766 |
On Thu, 2015-10-22 at 15:32 +0300, Michael S. Tsirkin wrote: > On Wed, Oct 21, 2015 at 01:20:27PM -0600, Alex Williamson wrote: > > The trouble here is that the VF needs to be unplugged prior to the start > > of migration because we can't do effective dirty page tracking while the > > device is connected and doing DMA. > > That's exactly what patch 12/12 is trying to accomplish. > > I do see some problems with it, but I also suggested some solutions. I was replying to: > So... what would you expect service down wise for the following > solution which is zero touch and I think should work for any VF > driver: And then later note: "Here it's done via an enlightened guest driver." -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | "Michael S. Tsirkin" <mst@redhat.com> |
|---|---|
| Date | 2015-10-22 15:10 +0200 |
| Subject | Re: [Qemu-devel] [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC |
| Message-ID | <qmnN0-3jD-31@gated-at.bofh.it> |
| In reply to | #1253786 |
On Thu, Oct 22, 2015 at 07:01:01AM -0600, Alex Williamson wrote: > On Thu, 2015-10-22 at 15:32 +0300, Michael S. Tsirkin wrote: > > On Wed, Oct 21, 2015 at 01:20:27PM -0600, Alex Williamson wrote: > > > The trouble here is that the VF needs to be unplugged prior to the start > > > of migration because we can't do effective dirty page tracking while the > > > device is connected and doing DMA. > > > > That's exactly what patch 12/12 is trying to accomplish. > > > > I do see some problems with it, but I also suggested some solutions. > > I was replying to: > > > So... what would you expect service down wise for the following > > solution which is zero touch and I think should work for any VF > > driver: > > And then later note: > > "Here it's done via an enlightened guest driver." Oh, I misunderstood your intent. Sorry about that. So we are actually in agreement between us then. That's nice. -- MST -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Or Gerlitz <gerlitz.or@gmail.com> |
|---|---|
| Date | 2015-10-22 18:00 +0200 |
| Message-ID | <qmqrv-74a-11@gated-at.bofh.it> |
| In reply to | #1253148 |
On Wed, Oct 21, 2015 at 10:20 PM, Alex Williamson <alex.williamson@redhat.com> wrote: > This is why the typical VF agnostic approach here is to using bonding > and fail over to a emulated device during migration, so performance > suffers, but downtime is something acceptable. bonding in the VM isn't a zero touch solution, right? is it really acceptable? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Alex Williamson <alex.williamson@redhat.com> |
|---|---|
| Date | 2015-10-22 18:20 +0200 |
| Subject | Re: [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC |
| Message-ID | <qmqKR-7G4-7@gated-at.bofh.it> |
| In reply to | #1253937 |
On Thu, 2015-10-22 at 18:58 +0300, Or Gerlitz wrote: > On Wed, Oct 21, 2015 at 10:20 PM, Alex Williamson > <alex.williamson@redhat.com> wrote: > > > This is why the typical VF agnostic approach here is to using bonding > > and fail over to a emulated device during migration, so performance > > suffers, but downtime is something acceptable. > > bonding in the VM isn't a zero touch solution, right? is it really acceptable? The bonding solution requires configuring the bond in the guest and doing the hot unplug/re-plug around migration. It's zero touch in that it works on current code with any PF/VF, but it's certainly not zero configuration in the guest. Is what acceptable? The configuration? The performance? The downtime? I don't think we can hope to improve on the downtime of an emulated device, but obviously the configuration and performance are not always acceptable or we wouldn't be seeing so many people working on migration of assigned devices. Thanks, Alex -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
Page 1 of 2 [1] 2 Next page →
Back to top | Article view | linux.kernel
csiph-web