* Re: [RFC PATCH v1] tun: Cleanup error handling in tun_set_iff()
From: Eric W. Biederman @ 2009-08-06 15:02 UTC (permalink / raw)
To: Herbert Xu; +Cc: Paul Moore, netdev, David Miller
In-Reply-To: <20090806143959.GA29323@gondor.apana.org.au>
Herbert Xu <herbert@gondor.apana.org.au> writes:
> On Thu, Aug 06, 2009 at 07:27:13AM -0700, Eric W. Biederman wrote:
>>
>> Summarizing:
>>
>> tun = __tun_get(tfile);
>> if (!tun) { // No tun we are not attached.
>> < -------------------- race opportunity
>> rtnl_lock();
>> tun_set_iff();
>> rtnl_unlock();
>> }
>> ...
>>
>> We don't test if we are attached under the rtnl
>> until we get to tun_attach();
>>
>> So two threads can both do:
>>
>> tun = __tun_get(tfile);
>> if (!tun) {
>> rtnl_lock();
>> tun_set_iff();
>> dev = __dev_get_by_name(net, "not_an_interface_name");
>> if (!dev) {
>> dev = alloc_netdev(....);
>> ...;
>> register_netdev(dev);
>> ...;
>> err = tun_attach(..);
>> }
>>
>>
>> Only one thread is in tun_set_iff() at a time, but the other thread
>> could have attached the file to a device before the one in tun_attach().
>
> Right, I see what you mean. However I don't think this is possible
> because the ioctl runs under the big kernel lock.
Why not? We can sleep on that code path.
Although now that you mention it we should use unlocked_ioctl unless
we actually need the BKL.
Eric
^ permalink raw reply
* Re: [PATCH 1/7] shm-signal: shared-memory signals
From: Gregory Haskins @ 2009-08-06 15:11 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: paulmck, alacrityvm-devel, linux-kernel, netdev
In-Reply-To: <200908061556.55390.arnd@arndb.de>
Hi Arnd,
>>> On 8/6/2009 at 9:56 AM, in message <200908061556.55390.arnd@arndb.de>, Arnd
Bergmann <arnd@arndb.de> wrote:
> On Monday 03 August 2009, Gregory Haskins wrote:
>> shm-signal provides a generic shared-memory based bidirectional
>> signaling mechanism. It is used in conjunction with an existing
>> signal transport (such as posix-signals, interrupts, pipes, etc) to
>> increase the efficiency of the transport since the state information
>> is directly accessible to both sides of the link. The shared-memory
>> design provides very cheap access to features such as event-masking
>> and spurious delivery mititgation, and is useful implementing higher
>> level shared-memory constructs such as rings.
>
> Looks like a very useful feature in general.
Thanks, I was hoping that would be the case.
>
>> +struct shm_signal_irq {
>> + __u8 enabled;
>> + __u8 pending;
>> + __u8 dirty;
>> +};
>
> Won't this layout cause cache line ping pong? Other schemes I have
> seen try to separate the bits so that each cache line is written to
> by only one side.
It could possibly use some optimization in that regard. I generally consider myself an expert at concurrent programming, but this lockless stuff is, um, hard ;) I was going for correctness first.
Long story short, any suggestions on ways to split this up are welcome (particularly now, before the ABI is sealed ;)
> This gets much more interesting if the two sides
> are on remote ends of an I/O link, e.g. using a nontransparent
> PCI bridge, where you only want to send stores over the wire, but
> never fetches or even read-modify-write cycles.
/me head explodes ;)
>
> Your code is probably optimal if you only communicate between host
> and guest code on the same CPU, but not so good if it crosses NUMA
> nodes or worse.
Yeah, I wont lie and say it wasn't designed primarily for the former case in mind (since it was my particular itch). I would certainly appreciate any insight on ways to make it more generally applicable for things like the transparent bridge model, and/or NUMA, though.
>
>> +struct shm_signal_desc {
>> + __u32 magic;
>> + __u32 ver;
>> + struct shm_signal_irq irq[2];
>> +};
>
> This data structure has implicit padding of two bytes at the end.
> How about adding another '__u16 reserved' to make it explicit?
Good idea. Will fix.
>
>> + /*
>> + * We always mark the remote side as dirty regardless of whether
>> + * they need to be notified.
>> + */
>> + irq->dirty = 1;
>> + wmb(); /* dirty must be visible before we test the pending state */
>> +
>> + if (irq->enabled && !irq->pending) {
>> + rmb();
>> +
>> + /*
>> + * If the remote side has enabled notifications, and we do
>> + * not see a notification pending, we must inject a new one.
>> + */
>> + irq->pending = 1;
>> + wmb(); /* make it visible before we do the injection */
>> +
>> + s->ops->inject(s);
>> + }
>
> Barriers always confuse me, but the rmb() looks slightly wrong. AFAIU
> it only prevents reads after the barrier from being done before the
> barrier, but you don't do any reads after it.
Its probably overzealous barrier'ing on my part. I had a conversation with Paul McKenney (CC'd) where I was wondering if a conditional was an implicit barrier. His response was something to the effect of "on most arches, yes, but not all". And we concluded that, to be conservative, there should be a rmb() after the if().
That said, tbh I am not sure if its actually needed. Paul?
>
> The (irq->enabled && !irq->pending) check could be done before the
> irq->dirty = 1 arrives at the bus, but that does not seem to hurt, it
> would at most cause a duplicate ->inject().
>
> Regarding the scope of the barrier, did you intentionally use the
> global versions (rmb()/wmb()) and not the lighter single-system
> (smp_rmb()/smp_wmb()) versions? Your version should cope with remote
> links over PCI but looks otherwise optimized for local use, as I
> wrote above.
Yes, it was intentional. Both for the remote case, as you point out. Also for the case where local might be mismatched (for instance, a guest compiled as UP).
Thanks Arnd,
-Greg
^ permalink raw reply
* [PATCH 4/5] tc35815: Fix rx_missed_errors count
From: Atsushi Nemoto @ 2009-08-06 14:41 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Ralf Roesch
The Miss_Cnt register is cleared by reading. Accumulate its value to
rx_missed_errors count.
Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
---
drivers/net/tc35815.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/tc35815.c b/drivers/net/tc35815.c
index 1adb150..564d555 100644
--- a/drivers/net/tc35815.c
+++ b/drivers/net/tc35815.c
@@ -2139,7 +2139,7 @@ static struct net_device_stats *tc35815_get_stats(struct net_device *dev)
(struct tc35815_regs __iomem *)dev->base_addr;
if (netif_running(dev))
/* Update the statistics from the device registers. */
- dev->stats.rx_missed_errors = tc_readl(&tr->Miss_Cnt);
+ dev->stats.rx_missed_errors += tc_readl(&tr->Miss_Cnt);
return &dev->stats;
}
--
1.5.6.5
^ permalink raw reply related
* [PATCH 2/5] tc35815: Improve BLEx / FDAEx handling
From: Atsushi Nemoto @ 2009-08-06 14:41 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Ralf Roesch
Clear Int_BLEx / Int_FDAEx after (not before) processing Rx interrupt.
This will reduce number of unnecessary interrupts.
Also print rx error messages only if netif_msg_rx_err() enabled.
Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
---
drivers/net/tc35815.c | 60 +++++++++++++++++++++---------------------------
1 files changed, 26 insertions(+), 34 deletions(-)
diff --git a/drivers/net/tc35815.c b/drivers/net/tc35815.c
index c903a68..801d201 100644
--- a/drivers/net/tc35815.c
+++ b/drivers/net/tc35815.c
@@ -1541,8 +1541,6 @@ static int tc35815_do_interrupt(struct net_device *dev, u32 status)
#endif
{
struct tc35815_local *lp = netdev_priv(dev);
- struct tc35815_regs __iomem *tr =
- (struct tc35815_regs __iomem *)dev->base_addr;
int ret = -1;
/* Fatal errors... */
@@ -1552,27 +1550,26 @@ static int tc35815_do_interrupt(struct net_device *dev, u32 status)
}
/* recoverable errors */
if (status & Int_IntFDAEx) {
- /* disable FDAEx int. (until we make rooms...) */
- tc_writel(tc_readl(&tr->Int_En) & ~Int_FDAExEn, &tr->Int_En);
- printk(KERN_WARNING
- "%s: Free Descriptor Area Exhausted (%#x).\n",
- dev->name, status);
+ if (netif_msg_rx_err(lp))
+ dev_warn(&dev->dev,
+ "Free Descriptor Area Exhausted (%#x).\n",
+ status);
dev->stats.rx_dropped++;
ret = 0;
}
if (status & Int_IntBLEx) {
- /* disable BLEx int. (until we make rooms...) */
- tc_writel(tc_readl(&tr->Int_En) & ~Int_BLExEn, &tr->Int_En);
- printk(KERN_WARNING
- "%s: Buffer List Exhausted (%#x).\n",
- dev->name, status);
+ if (netif_msg_rx_err(lp))
+ dev_warn(&dev->dev,
+ "Buffer List Exhausted (%#x).\n",
+ status);
dev->stats.rx_dropped++;
ret = 0;
}
if (status & Int_IntExBD) {
- printk(KERN_WARNING
- "%s: Excessive Buffer Descriptiors (%#x).\n",
- dev->name, status);
+ if (netif_msg_rx_err(lp))
+ dev_warn(&dev->dev,
+ "Excessive Buffer Descriptiors (%#x).\n",
+ status);
dev->stats.rx_length_errors++;
ret = 0;
}
@@ -1631,8 +1628,12 @@ static irqreturn_t tc35815_interrupt(int irq, void *dev_id)
spin_lock(&lp->lock);
status = tc_readl(&tr->Int_Src);
- tc_writel(status, &tr->Int_Src); /* write to clear */
+ /* BLEx, FDAEx will be cleared later */
+ tc_writel(status & ~(Int_BLEx | Int_FDAEx),
+ &tr->Int_Src); /* write to clear */
handled = tc35815_do_interrupt(dev, status);
+ if (status & (Int_BLEx | Int_FDAEx))
+ tc_writel(status & (Int_BLEx | Int_FDAEx), &tr->Int_Src);
(void)tc_readl(&tr->Int_Src); /* flush */
spin_unlock(&lp->lock);
return IRQ_RETVAL(handled >= 0);
@@ -1660,8 +1661,6 @@ tc35815_rx(struct net_device *dev)
struct tc35815_local *lp = netdev_priv(dev);
unsigned int fdctl;
int i;
- int buf_free_count = 0;
- int fd_free_count = 0;
#ifdef TC35815_NAPI
int received = 0;
#endif
@@ -1770,8 +1769,9 @@ tc35815_rx(struct net_device *dev)
dev->stats.rx_bytes += pkt_len;
} else {
dev->stats.rx_errors++;
- printk(KERN_DEBUG "%s: Rx error (status %x)\n",
- dev->name, status & Rx_Stat_Mask);
+ if (netif_msg_rx_err(lp))
+ dev_info(&dev->dev, "Rx error (status %x)\n",
+ status & Rx_Stat_Mask);
/* WORKAROUND: LongErr and CRCErr means Overflow. */
if ((status & Rx_LongErr) && (status & Rx_CRCErr)) {
status &= ~(Rx_LongErr|Rx_CRCErr);
@@ -1849,7 +1849,6 @@ tc35815_rx(struct net_device *dev)
#else
lp->fbl_count++;
#endif
- buf_free_count++;
}
}
@@ -1871,7 +1870,6 @@ tc35815_rx(struct net_device *dev)
#endif
lp->rfd_cur->fd.FDCtl = cpu_to_le32(FD_CownsFD);
lp->rfd_cur++;
- fd_free_count++;
}
if (lp->rfd_cur > lp->rfd_limit)
lp->rfd_cur = lp->rfd_base;
@@ -1882,17 +1880,6 @@ tc35815_rx(struct net_device *dev)
#endif
}
- /* re-enable BL/FDA Exhaust interrupts. */
- if (fd_free_count) {
- struct tc35815_regs __iomem *tr =
- (struct tc35815_regs __iomem *)dev->base_addr;
- u32 en, en_old = tc_readl(&tr->Int_En);
- en = en_old | Int_FDAExEn;
- if (buf_free_count)
- en |= Int_BLExEn;
- if (en != en_old)
- tc_writel(en, &tr->Int_En);
- }
#ifdef TC35815_NAPI
return received;
#endif
@@ -1911,9 +1898,14 @@ static int tc35815_poll(struct napi_struct *napi, int budget)
spin_lock(&lp->lock);
status = tc_readl(&tr->Int_Src);
do {
- tc_writel(status, &tr->Int_Src); /* write to clear */
+ /* BLEx, FDAEx will be cleared later */
+ tc_writel(status & ~(Int_BLEx | Int_FDAEx),
+ &tr->Int_Src); /* write to clear */
handled = tc35815_do_interrupt(dev, status, budget - received);
+ if (status & (Int_BLEx | Int_FDAEx))
+ tc_writel(status & (Int_BLEx | Int_FDAEx),
+ &tr->Int_Src);
if (handled >= 0) {
received += handled;
if (received >= budget)
--
1.5.6.5
^ permalink raw reply related
* [PATCH 1/5] tc35815: Disable StripCRC
From: Atsushi Nemoto @ 2009-08-06 14:41 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Ralf Roesch
It seems Rx_StripCRC cause trouble on recovering from the BLEx (Buffer
List Exhaust) or FDAEx (Free Descriptor Area Exhaust) condition.
Do not use it.
Also bump version number up.
Reported-by: Ralf Roesch <ralf.roesch@rw-gmbh.de>
Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
---
drivers/net/tc35815.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/tc35815.c b/drivers/net/tc35815.c
index d737f6b..c903a68 100644
--- a/drivers/net/tc35815.c
+++ b/drivers/net/tc35815.c
@@ -23,9 +23,9 @@
*/
#ifdef TC35815_NAPI
-#define DRV_VERSION "1.37-NAPI"
+#define DRV_VERSION "1.38-NAPI"
#else
-#define DRV_VERSION "1.37"
+#define DRV_VERSION "1.38"
#endif
static const char *version = "tc35815.c:v" DRV_VERSION "\n";
#define MODNAME "tc35815"
@@ -341,8 +341,9 @@ struct BDesc {
Tx_EnExColl | Tx_EnLCarr | Tx_EnExDefer | Tx_EnUnder | \
Tx_En) /* maybe 0x7b01 */
#endif
+/* Do not use Rx_StripCRC -- it causes trouble on BLEx/FDAEx condition */
#define RX_CTL_CMD (Rx_EnGood | Rx_EnRxPar | Rx_EnLongErr | Rx_EnOver \
- | Rx_EnCRCErr | Rx_EnAlign | Rx_StripCRC | Rx_RxEn) /* maybe 0x6f11 */
+ | Rx_EnCRCErr | Rx_EnAlign | Rx_RxEn) /* maybe 0x6f01 */
#define INT_EN_CMD (Int_NRAbtEn | \
Int_DmParErrEn | Int_DParDEn | Int_DParErrEn | \
Int_SSysErrEn | Int_RMasAbtEn | Int_RTargAbtEn | \
--
1.5.6.5
^ permalink raw reply related
* [PATCH 5/5] tc35815: Use 0 RxFragSize.MinFrag value for non-packing mode
From: Atsushi Nemoto @ 2009-08-06 14:41 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Ralf Roesch
The datasheet say "When not enabling packing, the MinFrag value must
remain at 0". Do not set value to RxFragSize register if
TC35815_USE_PACKEDBUFFER disabled.
This is not a bugfix. No real problem reported on this.
Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
---
drivers/net/tc35815.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/drivers/net/tc35815.c b/drivers/net/tc35815.c
index 564d555..d2ca47f 100644
--- a/drivers/net/tc35815.c
+++ b/drivers/net/tc35815.c
@@ -2394,8 +2394,6 @@ static void tc35815_chip_init(struct net_device *dev)
tc_writel(DMA_BURST_SIZE, &tr->DMA_Ctl);
#ifdef TC35815_USE_PACKEDBUFFER
tc_writel(RxFrag_EnPack | ETH_ZLEN, &tr->RxFragSize); /* Packing */
-#else
- tc_writel(ETH_ZLEN, &tr->RxFragSize);
#endif
tc_writel(0, &tr->TxPollCtr); /* Batch mode */
tc_writel(TX_THRESHOLD, &tr->TxThrsh);
--
1.5.6.5
^ permalink raw reply related
* [PATCH 3/5] tc35815: Increase timeout for mdio
From: Atsushi Nemoto @ 2009-08-06 14:41 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Ralf Roesch
The current timeout value is too short for very high-load condition
which jiffies might jump up in busy-loop.
Also add minimum delay before checking completion of MDIO.
Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
---
drivers/net/tc35815.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/tc35815.c b/drivers/net/tc35815.c
index 801d201..1adb150 100644
--- a/drivers/net/tc35815.c
+++ b/drivers/net/tc35815.c
@@ -594,9 +594,10 @@ static int tc_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
struct net_device *dev = bus->priv;
struct tc35815_regs __iomem *tr =
(struct tc35815_regs __iomem *)dev->base_addr;
- unsigned long timeout = jiffies + 10;
+ unsigned long timeout = jiffies + HZ;
tc_writel(MD_CA_Busy | (mii_id << 5) | (regnum & 0x1f), &tr->MD_CA);
+ udelay(12); /* it takes 32 x 400ns at least */
while (tc_readl(&tr->MD_CA) & MD_CA_Busy) {
if (time_after(jiffies, timeout))
return -EIO;
@@ -610,11 +611,12 @@ static int tc_mdio_write(struct mii_bus *bus, int mii_id, int regnum, u16 val)
struct net_device *dev = bus->priv;
struct tc35815_regs __iomem *tr =
(struct tc35815_regs __iomem *)dev->base_addr;
- unsigned long timeout = jiffies + 10;
+ unsigned long timeout = jiffies + HZ;
tc_writel(val, &tr->MD_Data);
tc_writel(MD_CA_Busy | MD_CA_Wr | (mii_id << 5) | (regnum & 0x1f),
&tr->MD_CA);
+ udelay(12); /* it takes 32 x 400ns at least */
while (tc_readl(&tr->MD_CA) & MD_CA_Busy) {
if (time_after(jiffies, timeout))
return -EIO;
--
1.5.6.5
^ permalink raw reply related
* Re: [PATCH] sky2: Serialize access to PCI config space
From: Stephen Hemminger @ 2009-08-06 15:34 UTC (permalink / raw)
To: Mike McCormack; +Cc: netdev, Rene Mayrhofer, Richard Leitner
In-Reply-To: <4A7ABA23.6060201@ring3k.org>
On Thu, 06 Aug 2009 20:10:27 +0900
Mike McCormack <mikem@ring3k.org> wrote:
> PCI config space may be shared across two device instances,
> or accessed from an interrupt
> , so serialize access so
> read-update-write operations do not interfere with each other.
>
> Only tested on a single port card, as my sky2 doesn't have dual ports.
>
> Signed-off-by: Mike McCormack <mikem@ring3k.org>
No, more locks is not better design. Better to just use existing
RTNL mutex consistently.
^ permalink raw reply
* Re: [PATCH 0/7] AlacrityVM guest drivers Reply-To:
From: Arnd Bergmann @ 2009-08-06 15:40 UTC (permalink / raw)
To: Gregory Haskins
Cc: Avi Kivity, alacrityvm-devel, Michael S. Tsirkin, kvm,
linux-kernel, netdev
In-Reply-To: <4A7AAB1A0200005A00051BED@sinclair.provo.novell.com>
On Thursday 06 August 2009, Gregory Haskins wrote:
> We can exchange out the "virtio-pci" module like this:
>
> (guest-side)
> |--------------------------
> | virtio-net
> |--------------------------
> | virtio-ring
> |--------------------------
> | virtio-bus
> |--------------------------
> | virtio-vbus
> |--------------------------
> | vbus-proxy
> |--------------------------
> | vbus-connector
> |--------------------------
> |
> (vbus)
> |
> |--------------------------
> | kvm.ko
> |--------------------------
> | vbus-connector
> |--------------------------
> | vbus
> |--------------------------
> | virtio-net-tap (vbus model)
> |--------------------------
> | netif
> |--------------------------
> (host-side)
>
>
> So virtio-net runs unmodified. What is "competing" here is "virtio-pci" vs "virtio-vbus".
> Also, venet vs virtio-net are technically competing. But to say "virtio vs vbus" is inaccurate, IMO.
I think what's confusing everyone is that you are competing on multiple
issues:
1. Implementation of bus probing: both vbus and virtio are backed by
PCI devices and can be backed by something else (e.g. virtio by lguest
or even by vbus).
2. Exchange of metadata: virtio uses a config space, vbus uses devcall
to do the same.
3. User data transport: virtio has virtqueues, vbus has shm/ioq.
I think these three are the main differences, and the venet vs. virtio-net
question comes down to which interface the drivers use for each aspect. Do
you agree with this interpretation?
Now to draw conclusions from each of these is of course highly subjective,
but this is how I view it:
1. The bus probing is roughly equivalent, they both work and the
virtio method seems to need a little less code but that could be fixed
by slimming down the vbus code as I mentioned in my comments on the
pci-to-vbus bridge code. However, I would much prefer not to have both
of them, and virtio came first.
2. the two methods (devcall/config space) are more or less equivalent
and you should be able to implement each one through the other one. The
virtio design was driven by making it look similar to PCI, the vbus
design was driven by making it easy to implement in a host kernel. I
don't care too much about these, as they can probably coexist without
causing any trouble. For a (hypothetical) vbus-in-virtio device,
a devcall can be a config-set/config-get pair, for a virtio-in-vbus,
you can do a config-get and a config-set devcall and be happy. Each
could be done in a trivial helper library.
3. The ioq method seems to be the real core of your work that makes
venet perform better than virtio-net with its virtqueues. I don't see
any reason to doubt that your claim is correct. My conclusion from
this would be to add support for ioq to virtio devices, alongside
virtqueues, but to leave out the extra bus_type and probing method.
Arnd <><
^ permalink raw reply
* Re: [PATCH 0/7] AlacrityVM guest drivers Reply-To:
From: Michael S. Tsirkin @ 2009-08-06 15:45 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Gregory Haskins, Avi Kivity, alacrityvm-devel, kvm, linux-kernel,
netdev
In-Reply-To: <200908061740.04276.arnd@arndb.de>
On Thu, Aug 06, 2009 at 05:40:04PM +0200, Arnd Bergmann wrote:
> 3. The ioq method seems to be the real core of your work that makes
> venet perform better than virtio-net with its virtqueues. I don't see
> any reason to doubt that your claim is correct. My conclusion from
> this would be to add support for ioq to virtio devices, alongside
> virtqueues, but to leave out the extra bus_type and probing method.
>
> Arnd <><
The fact that it's in kernel also likely contributes.
--
MST
^ permalink raw reply
* Re: [PATCH 0/7] AlacrityVM guest drivers Reply-To:
From: Avi Kivity @ 2009-08-06 15:50 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Gregory Haskins, alacrityvm-devel, Michael S. Tsirkin, kvm,
linux-kernel, netdev
In-Reply-To: <200908061740.04276.arnd@arndb.de>
On 08/06/2009 06:40 PM, Arnd Bergmann wrote:
> 3. The ioq method seems to be the real core of your work that makes
> venet perform better than virtio-net with its virtqueues. I don't see
> any reason to doubt that your claim is correct. My conclusion from
> this would be to add support for ioq to virtio devices, alongside
> virtqueues, but to leave out the extra bus_type and probing method.
>
The current conjecture is that ioq outperforms virtio because the host
side of ioq is implemented in the host kernel, while the host side of
virtio is implemented in userspace. AFAIK, no one pointed out
differences in the protocol which explain the differences in performance.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply
* Re: [RFC PATCH v1 1/2] lsm: Add hooks to the TUN driver
From: Serge E. Hallyn @ 2009-08-06 15:52 UTC (permalink / raw)
To: Paul Moore; +Cc: eparis, netdev, linux-security-module, selinux
In-Reply-To: <200908061024.54786.paul.moore@hp.com>
Quoting Paul Moore (paul.moore@hp.com):
> On Wednesday 05 August 2009 10:15:58 pm Serge E. Hallyn wrote:
> > Quoting Paul Moore (paul.moore@hp.com):
> > > On Wednesday 05 August 2009 10:13:50 am Serge E. Hallyn wrote:
> > > > Quoting Paul Moore (paul.moore@hp.com):
> > >
> > > [NOTE: my email has been out all day due to some mysterious FS issue so
> > > my apologies for not replying sooner]
> > >
> > > ...
> > >
> > > > The checks before and after this patch are not equivalent. Post-patch,
> > > > one must always have CAP_NET_ADMIN to do the attach, whereas pre-patch
> > > > you only needed those if current_cred() did not own the tun device. Is
> > > > that intentional?
> > >
> > > Nope, just a goof on my part; I misread the booleans and haven't fully
> > > tested the patch yet so it slipped out, thanks for catching it. This
> > > brings up a good point, would we rather move the TUN owner/group checks
> > > into the cap_tun_* functions or move the capable() call back into the TUN
> > > driver? The answer wasn't clear to me when I was looking at the code
> > > before and the uniqueness of the TUN driver doesn't help much in this
> > > regard.
> >
> > I see the question being asked as: Does this device belong to
> > the caller and, if not, is the caller privileged to act
> > anyway?' So I think the capable call should be moved back
> > into the tun driver, followed by a separate security_tun_dev_attach()
> > check, since that is a separate, restrictive question.
>
> Works for me, I'll make the change.
>
> BTW, the main reason for posting the patches in such an early state was to
> solicit feedback on the location and types of hooks added; I've read lots of
> good feedback but nothing regarding the fundamental aspects of the hooks ...
> any comments before I push out v2?
Oh now that you mention it, yes - I think the security_tun_dev_attach()
should be called again separately after the post_create() hook.
As for more general comments on whether or which tuntap-specific hooks
need to exist, two things. First, if you have specific requirements
in mind please do share those, otherwise I'm working based on what I
see in Documentation/networking/tuntap.txt and drivers/net/tun.c. Second,
based on my understanding i think the hooks you have make sense,
but is there any way to relabel a tun socket? Since they are always
labeled with current_sid(), that seems restrictive... I see that you
don't want to use sockcreate_sid, but (to use a made-up example not
reflecting reality) a kvm_setup_t task couldn't create a tun sock for
a kvm_run_t task to use, right?
-serge
^ permalink raw reply
* Re: [PATCH 4/7] vbus-proxy: add a pci-to-vbus bridge
From: Gregory Haskins @ 2009-08-06 15:59 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: alacrityvm-devel, linux-kernel, netdev
In-Reply-To: <200908061642.40614.arnd@arndb.de>
>>> On 8/6/2009 at 10:42 AM, in message <200908061642.40614.arnd@arndb.de>, Arnd
Bergmann <arnd@arndb.de> wrote:
> On Monday 03 August 2009, Gregory Haskins wrote:
>> This patch adds a pci-based driver to interface between the a host VBUS
>> and the guest's vbus-proxy bus model.
>>
>> Signed-off-by: Gregory Haskins <ghaskins@novell.com>
>
> This seems to be duplicating parts of virtio-pci that could be kept
> common by extending the virtio code. Layering on top of virtio
> would also make it possible to use the same features you add
> on top of other transports (e.g. the s390 virtio code) without
> adding yet another backend for each of them.
This doesn't make sense to me, but I suspect we are both looking at what this code does differently. I am under the impression that you may believe that there is one of these objects per vbus device. Note that this is just a bridge to vbus, so there is only one of these per system with potentially many vbus devices behind it.
In essence, this driver's job is to populate the "vbus-proxy" LDM bus with objects that it finds across the PCI-OTHER bridge. This would actually sit below the virtio components in the stack, so it doesnt make sense (to me) to turn around and build this on top of virtio. But perhaps I am missing something you are seeing.
Can you elaborate?
>
>> +static int
>> +vbus_pci_hypercall(unsigned long nr, void *data, unsigned long len)
>> +{
>> + struct vbus_pci_hypercall params = {
>> + .vector = nr,
>> + .len = len,
>> + .datap = __pa(data),
>> + };
>> + unsigned long flags;
>> + int ret;
>> +
>> + spin_lock_irqsave(&vbus_pci.lock, flags);
>> +
>> + memcpy_toio(&vbus_pci.regs->hypercall.data, ¶ms, sizeof(params));
>> + ret = ioread32(&vbus_pci.regs->hypercall.result);
>> +
>> + spin_unlock_irqrestore(&vbus_pci.lock, flags);
>> +
>> + return ret;
>> +}
>
> The functionality looks reasonable but please don't call this a hypercall.
Heh, I guess its just semantics. The reason why its called hypercall is two fold:
1) In previous versions (vbus-v3 and earlier), it actually *was* literally a KVM-hypercall.
2) In this current version, it is purely a PCI device doing PIO, but it still acts exactly like a hypercall on the backend (via the ioeventfd mechanism in KVM).
That said, I am not married to the name, so I can come up with something more appropriate/descriptive.
> A hypercall would be hypervisor specific by definition while this one
> is device specific if I understand it correctly. How about "command queue",
> "mailbox", "message queue", "devcall" or something else that we have in
> existing PCI devices?
>
>> +
>> +static int
>> +vbus_pci_device_open(struct vbus_device_proxy *vdev, int version, int
> flags)
>> +{
>> + struct vbus_pci_device *dev = to_dev(vdev);
>> + struct vbus_pci_deviceopen params;
>> + int ret;
>> +
>> + if (dev->handle)
>> + return -EINVAL;
>> +
>> + params.devid = vdev->id;
>> + params.version = version;
>> +
>> + ret = vbus_pci_hypercall(VBUS_PCI_HC_DEVOPEN,
>> + ¶ms, sizeof(params));
>> + if (ret < 0)
>> + return ret;
>> +
>> + dev->handle = params.handle;
>> +
>> + return 0;
>> +}
>
> This seems to add an artificial abstraction that does not make sense
> if you stick to the PCI abstraction.
I think there may be confusion about what is going on here. The "device-open" pertains to a vbus device *beyond* the bridge, not the PCI device (the bridge) itself. Nor is the vbus device a PCI device.
Whats happening here is somewhat analogous to a PCI config-cycle. Its a way to open a channel to a device beyond the bridge in _response_ to a probe.
We have a way to enumerate devices present beyond the bridge (this yields a "device-id") but in order to actually talk to the device, you must first call DEVOPEN(id). When a device-id is enumerated, it generates a probe() event on vbus-proxy. The responding driver in question would then turn around and issue the handle = dev->open(VERSION) to see if it is compatible with the device, and to establish a context for further communication.
The reason why DEVOPEN returns a unique handle is to help ensure that the driver has established proper context before allowing other calls.
> The two sensible and common models
> for virtual devices that I've seen are:
>
> * The hypervisor knows what virtual resources exist and provides them
> to the guest. The guest owns them as soon as they show up in the
> bus (e.g. PCI) probe. The 'handle' is preexisting.
>
> * The guest starts without any devices and asks for resources it wants
> to access. There is no probing of resources but the guest issues
> a hypercall to get a handle to a newly created virtual device
> (or -ENODEV).
>
> What is your reasoning for requiring both a probe and an allocation?
Answered above, I thnk
>
>> +static int
>> +vbus_pci_device_shm(struct vbus_device_proxy *vdev, int id, int prio,
>> + void *ptr, size_t len,
>> + struct shm_signal_desc *sdesc, struct shm_signal **signal,
>> + int flags)
>> +{
>> + struct vbus_pci_device *dev = to_dev(vdev);
>> + struct _signal *_signal = NULL;
>> + struct vbus_pci_deviceshm params;
>> + unsigned long iflags;
>> + int ret;
>> +
>> + if (!dev->handle)
>> + return -EINVAL;
>> +
>> + params.devh = dev->handle;
>> + params.id = id;
>> + params.flags = flags;
>> + params.datap = (u64)__pa(ptr);
>> + params.len = len;
>> +
>> + if (signal) {
>> + /*
>> + * The signal descriptor must be embedded within the
>> + * provided ptr
>> + */
>> + if (!sdesc
>> + || (len < sizeof(*sdesc))
>> + || ((void *)sdesc < ptr)
>> + || ((void *)sdesc > (ptr + len - sizeof(*sdesc))))
>> + return -EINVAL;
>> +
>> + _signal = kzalloc(sizeof(*_signal), GFP_KERNEL);
>> + if (!_signal)
>> + return -ENOMEM;
>> +
>> + _signal_init(&_signal->signal, sdesc, &_signal_ops);
>> +
>> + /*
>> + * take another reference for the host. This is dropped
>> + * by a SHMCLOSE event
>> + */
>> + shm_signal_get(&_signal->signal);
>> +
>> + params.signal.offset = (u64)sdesc - (u64)ptr;
>> + params.signal.prio = prio;
>> + params.signal.cookie = (u64)_signal;
>> +
>> + } else
>> + params.signal.offset = -1; /* yes, this is a u32, but its ok */
>> +
>> + ret = vbus_pci_hypercall(VBUS_PCI_HC_DEVSHM,
>> + ¶ms, sizeof(params));
>> + if (ret < 0) {
>> + if (_signal) {
>> + /*
>> + * We held two references above, so we need to drop
>> + * both of them
>> + */
>> + shm_signal_put(&_signal->signal);
>> + shm_signal_put(&_signal->signal);
>> + }
>> +
>> + return ret;
>> + }
>> +
>> + if (signal) {
>> + _signal->handle = ret;
>> +
>> + spin_lock_irqsave(&vbus_pci.lock, iflags);
>> +
>> + list_add_tail(&_signal->list, &dev->shms);
>> +
>> + spin_unlock_irqrestore(&vbus_pci.lock, iflags);
>> +
>> + shm_signal_get(&_signal->signal);
>> + *signal = &_signal->signal;
>> + }
>> +
>> + return 0;
>> +}
>
> This could be implemented by virtio devices as well, right?
The big difference with dev->shm() is that it is not bound to a particular ABI within the shared-memory (as opposed to virtio, which assumes a virtio ABI). This just creates an empty shared-memory region (with a bidirectional signaling path) which you can overlay a variety of structures (virtio included). You can of course also use non-ring based structures, such as, say, an array of idempotent state.
The point is that, once this is done, you have a shared-memory region and a way (via the shm-signal) to bidirectionally signal changes to that memory region. You can then build bigger things with it, like virtqueues.
>
>> +static int
>> +vbus_pci_device_call(struct vbus_device_proxy *vdev, u32 func, void *data,
>> + size_t len, int flags)
>> +{
>> + struct vbus_pci_device *dev = to_dev(vdev);
>> + struct vbus_pci_devicecall params = {
>> + .devh = dev->handle,
>> + .func = func,
>> + .datap = (u64)__pa(data),
>> + .len = len,
>> + .flags = flags,
>> + };
>> +
>> + if (!dev->handle)
>> + return -EINVAL;
>> +
>> + return vbus_pci_hypercall(VBUS_PCI_HC_DEVCALL, ¶ms, sizeof(params));
>> +}
>
> Why the indirection? It seems to me that you could do the simpler
What indirection?
/me looks below and thinks he sees the confusion..
>
> static int
> vbus_pci_device_call(struct vbus_device_proxy *vdev, u32 func, void *data,
> size_t len, int flags)
> {
> struct vbus_pci_device *dev = to_dev(vdev);
> struct vbus_pci_hypercall params = {
> .vector = func,
> .len = len,
> .datap = __pa(data),
> };
> spin_lock_irqsave(&dev.lock, flags);
> memcpy_toio(&dev.regs->hypercall.data, ¶ms, sizeof(params));
> ret = ioread32(&dev.regs->hypercall.result);
> spin_unlock_irqrestore(&dev.lock, flags);
>
> return ret;
> }
>
> This gets rid of your 'handle' and the unwinding through an extra pointer
> indirection. You just need to make sure that the device specific call
> numbers
> don't conflict with any global ones.
Ah, now I see the confusion...
DEVCALL is sending a synchronous call to a specific device beyond the bridge. The MMIO going on here against dev.regs->hypercall.data is sending a synchronous call to the bridge itself. They are distinctly different ;)
>
>> +
>> +static struct ioq_notifier eventq_notifier;
>
>> ...
>
>> +/* Invoked whenever the hypervisor ioq_signal()s our eventq */
>> +static void
>> +eventq_wakeup(struct ioq_notifier *notifier)
>> +{
>> + struct ioq_iterator iter;
>> + int ret;
>> +
>> + /* We want to iterate on the head of the in-use index */
>> + ret = ioq_iter_init(&vbus_pci.eventq, &iter, ioq_idxtype_inuse, 0);
>> + BUG_ON(ret < 0);
>> +
>> + ret = ioq_iter_seek(&iter, ioq_seek_head, 0, 0);
>> + BUG_ON(ret < 0);
>> +
>> + /*
>> + * The EOM is indicated by finding a packet that is still owned by
>> + * the south side.
>> + *
>> + * FIXME: This in theory could run indefinitely if the host keeps
>> + * feeding us events since there is nothing like a NAPI budget. We
>> + * might need to address that
>> + */
>> + while (!iter.desc->sown) {
>> + struct ioq_ring_desc *desc = iter.desc;
>> + struct vbus_pci_event *event;
>> +
>> + event = (struct vbus_pci_event *)desc->cookie;
>> +
>> + switch (event->eventid) {
>> + case VBUS_PCI_EVENT_DEVADD:
>> + event_devadd(&event->data.add);
>> + break;
>> + case VBUS_PCI_EVENT_DEVDROP:
>> + event_devdrop(&event->data.handle);
>> + break;
>> + case VBUS_PCI_EVENT_SHMSIGNAL:
>> + event_shmsignal(&event->data.handle);
>> + break;
>> + case VBUS_PCI_EVENT_SHMCLOSE:
>> + event_shmclose(&event->data.handle);
>> + break;
>> + default:
>> + printk(KERN_WARNING "VBUS_PCI: Unexpected event %d\n",
>> + event->eventid);
>> + break;
>> + };
>> +
>> + memset(event, 0, sizeof(*event));
>> +
>> + /* Advance the in-use head */
>> + ret = ioq_iter_pop(&iter, 0);
>> + BUG_ON(ret < 0);
>> + }
>> +
>> + /* And let the south side know that we changed the queue */
>> + ioq_signal(&vbus_pci.eventq, 0);
>> +}
>
> Ah, so you have a global event queue and your own device hotplug mechanism.
> But why would you then still use PCI to back it?
PCI is only used as a PCI-to-vbus bridge. Beyond that, the bridge is populating the vbus devices it sees beyond the bridge into the vbus-proxy LDM bus.
That said, the event queue is sending me events such as "device added" and "shm-signal", which are then reflected up into the general model.
> We already have PCI hotplug to add and remove devices
Yep, and I do actually use that...to get a probe for the bridge itself ;)
>and you have defined per device notifier queues that you can use for waking up the device, right?
Only per bridge. vbus drivers themselves allocate additional dynamic shm-signal channels that are tunneled through the bridge's eventq(s).
Kind Regards,
-Greg
^ permalink raw reply
* Re: [RFC PATCH v1 1/2] lsm: Add hooks to the TUN driver
From: Paul Moore @ 2009-08-06 16:25 UTC (permalink / raw)
To: Serge E. Hallyn; +Cc: eparis, netdev, linux-security-module, selinux
In-Reply-To: <20090806155257.GA32427@us.ibm.com>
On Thursday 06 August 2009 11:52:58 am Serge E. Hallyn wrote:
> Quoting Paul Moore (paul.moore@hp.com):
> > BTW, the main reason for posting the patches in such an early state was
> > to solicit feedback on the location and types of hooks added; I've read
> > lots of good feedback but nothing regarding the fundamental aspects of
> > the hooks ... any comments before I push out v2?
>
> Oh now that you mention it, yes - I think the security_tun_dev_attach()
> should be called again separately after the post_create() hook.
Why? Granted the TUN driver calls tun_attach() in both cases but that doesn't
necessarily mean the operation from a security point of view is the same.
Using the SELinux hooks as an example, attaching to an existing TUN device is
currently treated as a relabel operation; the calling task relabels the
persistent TUN device to match its own label so that traffic sent over the TUN
device is labeled using the newly attached calling task's label. Creating a
new TUN device is like creating any other object (yes, there are exceptions to
this but I'm speaking generally here), it inherits the label of the task which
creates it, performing access control for a relabel operation here just
doesn't make sense.
Or are you expecting some other form of access control for the attach hook
which would change this argument?
> As for more general comments on whether or which tuntap-specific hooks
> need to exist, two things. First, if you have specific requirements
> in mind please do share those, otherwise I'm working based on what I
> see in Documentation/networking/tuntap.txt and drivers/net/tun.c.
Not that haven't already been mentioned. If something doesn't make sense, let
me know.
> Second, based on my understanding i think the hooks you have make sense,
> but is there any way to relabel a tun socket? Since they are always labeled
> with current_sid(), that seems restrictive...
Not at present, the TUN driver only supports changing the user/group IDs. I
am debating adding support to change/view the label of the device/socket
(TUN{SET,GET}SECCTX perhaps?) but that can happen later and is in no way
prevented by these patches. My thinking is that these patches are a
requirement for us to apply the existing LSM network access controls to
traffic originating from the TUN driver; depending on how use cases evolve
with the TUN driver we may want to add additional functionality but this
should serve as a good base.
> I see that you don't want to use sockcreate_sid, but (to use a made-up
> example not reflecting reality) a kvm_setup_t task couldn't create a tun
> sock for a kvm_run_t task to use, right?
Well, the only time this will really be an issue is when you have one task
create a new, persistent TUN device and a second task that attaches to the
existing TUN device and uses it to send traffic. Sticking with your example,
if the first task is labeled kvm_setup_t and the second task is labeled
kvm_run_t then the policy would look something like this:
# allow kvm_setup_t to create a new TUN device
allow kvm_setup_t self:tun_socket { create };
# allow kvm_run_t to use TUN devices created by kvm_setup_t
allow kvm_run_t kvm_setup_t:tun_socket { relabelfrom };
allow kvm_run_t self:tun_socket { relabelto };
The policy above has the nice effect of only allowing kvm_run_t to attach to
existing TUN devices created by kvm_setup_t; it can not create a new TUN
device or use persistent TUN devices created by other domains. This should
also help explain why I think calling the attach() hook after the
post_create() hook makes little sense given the access controls currently
proposed.
--
paul moore
linux @ hp
^ permalink raw reply
* Re: [PATCH 0/7] AlacrityVM guest drivers Reply-To:
From: Gregory Haskins @ 2009-08-06 16:29 UTC (permalink / raw)
To: Arnd Bergmann
Cc: alacrityvm-devel, Avi Kivity, Michael S. Tsirkin, kvm,
linux-kernel, netdev
In-Reply-To: <200908061740.04276.arnd@arndb.de>
>>> On 8/6/2009 at 11:40 AM, in message <200908061740.04276.arnd@arndb.de>, Arnd
Bergmann <arnd@arndb.de> wrote:
> On Thursday 06 August 2009, Gregory Haskins wrote:
>> We can exchange out the "virtio-pci" module like this:
>>
>> (guest-side)
>> |--------------------------
>> | virtio-net
>> |--------------------------
>> | virtio-ring
>> |--------------------------
>> | virtio-bus
>> |--------------------------
>> | virtio-vbus
>> |--------------------------
>> | vbus-proxy
>> |--------------------------
>> | vbus-connector
>> |--------------------------
>> |
>> (vbus)
>> |
>> |--------------------------
>> | kvm.ko
>> |--------------------------
>> | vbus-connector
>> |--------------------------
>> | vbus
>> |--------------------------
>> | virtio-net-tap (vbus model)
>> |--------------------------
>> | netif
>> |--------------------------
>> (host-side)
>>
>>
>> So virtio-net runs unmodified. What is "competing" here is "virtio-pci" vs
> "virtio-vbus".
>> Also, venet vs virtio-net are technically competing. But to say "virtio vs
> vbus" is inaccurate, IMO.
>
>
> I think what's confusing everyone is that you are competing on multiple
> issues:
>
> 1. Implementation of bus probing: both vbus and virtio are backed by
> PCI devices and can be backed by something else (e.g. virtio by lguest
> or even by vbus).
More specifically, vbus-proxy and virtio-bus can be backed by modular adapters.
vbus-proxy can be backed by vbus-pcibridge (as it is in AlacrityVM). It was backed by KVM-hypercalls in previous releases, but we have deprecated/dropped that connector. Other types of connectors are possible...
virtio-bus can be backed by virtio-pci, virtio-lguest, virtio-s390, and virtio-vbus (which is backed by vbus-proxy, et. al.)
"vbus" itself is actually the host-side container technology which vbus-proxy connects to. This is an important distinction.
>
> 2. Exchange of metadata: virtio uses a config space, vbus uses devcall
> to do the same.
Sort of. You can use devcall() to implement something like config-space (and in fact, we do use it like this for some operations). But this can also be fast path (for when you need synchronous behavior).
This has various uses, such as when you need synchronous updates from non-preemptible guest code (cpupri, for instance, for -rt)
>
> 3. User data transport: virtio has virtqueues, vbus has shm/ioq.
Not quite: vbus has shm + shm-signal. You can then overlay shared-memory protocols over that, such as virtqueues, ioq, or even non-ring constructs.
I also consider the synchronous call() method to be part of the transport (tho more for niche devices, like -rt)
>
> I think these three are the main differences, and the venet vs. virtio-net
> question comes down to which interface the drivers use for each aspect. Do
> you agree with this interpretation?
>
> Now to draw conclusions from each of these is of course highly subjective,
> but this is how I view it:
>
> 1. The bus probing is roughly equivalent, they both work and the
> virtio method seems to need a little less code but that could be fixed
> by slimming down the vbus code as I mentioned in my comments on the
> pci-to-vbus bridge code. However, I would much prefer not to have both
> of them, and virtio came first.
>
> 2. the two methods (devcall/config space) are more or less equivalent
> and you should be able to implement each one through the other one. The
> virtio design was driven by making it look similar to PCI, the vbus
> design was driven by making it easy to implement in a host kernel. I
> don't care too much about these, as they can probably coexist without
> causing any trouble. For a (hypothetical) vbus-in-virtio device,
> a devcall can be a config-set/config-get pair, for a virtio-in-vbus,
> you can do a config-get and a config-set devcall and be happy. Each
> could be done in a trivial helper library.
Yep, in fact I publish something close to what I think you are talking about back in April
http://lkml.org/lkml/2009/4/21/427
>
> 3. The ioq method seems to be the real core of your work that makes
> venet perform better than virtio-net with its virtqueues. I don't see
> any reason to doubt that your claim is correct. My conclusion from
> this would be to add support for ioq to virtio devices, alongside
> virtqueues, but to leave out the extra bus_type and probing method.
While I appreciate the sentiment, I doubt that is actually whats helping here.
There are a variety of factors that I poured into venet/vbus that I think contribute to its superior performance. However, the difference in the ring design I do not think is one if them. In fact, in many ways I think Rusty's design might turn out to be faster if put side by side because he was much more careful with cacheline alignment than I was. Also note that I was careful to not pick one ring vs the other ;) They both should work.
IMO, we are only looking at the tip of the iceberg when looking at this purely as the difference between virtio-pci vs virtio-vbus, or venet vs virtio-net.
Really, the big thing I am working on here is the host side device-model. The idea here was to design a bus model that was conducive to high performance, software to software IO that would work in a variety of environments (that may or may not have PCI). KVM is one such environment, but I also have people looking at building other types of containers, and even physical systems (host+blade kind of setups).
The idea is that the "connector" is modular, and then something like virtio-net or venet "just work": in kvm, in the userspace container, on the blade system.
It provides a management infrastructure that (hopefully) makes sense for these different types of containers, regardless of whether they have PCI, QEMU, etc (e.g. things that are inherent to KVM, but not others).
I hope this helps to clarify the project :)
Kind Regards,
-Greg
^ permalink raw reply
* [PATCH 0/8] sky2: driver update
From: Stephen Hemminger @ 2009-08-06 16:12 UTC (permalink / raw)
To: David Miller; +Cc: netdev
This undoes the prematurely committed sky2 transmit logic changes
and applies Mike's later changes.
Here is a corrected patch stream please apply this to 2.6.31-rc5
--
^ permalink raw reply
* [PATCH 8/8] sky2: hold spinlock around phy_power_down
From: Stephen Hemminger @ 2009-08-06 16:12 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20090806161213.148382653@vyatta.com>
[-- Attachment #1: sky2-phy-lock.patch --]
[-- Type: text/plain, Size: 853 bytes --]
Avoid any possible problems with accessing PHY registers on shutdown.
This is a purely theoretical issue and is not related to any of the
outstanding bug reports. Since receiver and transmitter are already
shutdown and phy interrupts for this device are already disabled,
there should already be enough protection.
Suggested-by: Mike McCormack <mikem@ring3k.org>
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/drivers/net/sky2.c 2009-08-06 08:40:56.097359280 -0700
+++ b/drivers/net/sky2.c 2009-08-06 08:41:38.949377326 -0700
@@ -1883,7 +1883,9 @@ static int sky2_down(struct net_device *
synchronize_irq(hw->pdev->irq);
napi_synchronize(&hw->napi);
+ spin_lock_bh(&sky2->phy_lock);
sky2_phy_power_down(hw, port);
+ spin_unlock_bh(&sky2->phy_lock);
/* turn off LED's */
sky2_write16(hw, B0_Y2LED, LED_STAT_OFF);
--
^ permalink raw reply
* [PATCH 1/8] Revert "sky2: Avoid transmits during sky2_down()"
From: Stephen Hemminger @ 2009-08-06 16:12 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20090806161213.148382653@vyatta.com>
[-- Attachment #1: sky2-revert.patch --]
[-- Type: text/plain, Size: 2415 bytes --]
This reverts commit f6caa14aa0b126d4a2933907d1519611b2a8524a.
Better solution to same problem was done in later patches.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
drivers/net/sky2.c | 14 +-------------
drivers/net/sky2.h | 1 -
2 files changed, 1 insertion(+), 14 deletions(-)
--- a/drivers/net/sky2.c 2009-08-06 08:25:15.485025569 -0700
+++ b/drivers/net/sky2.c 2009-08-06 08:25:49.579125976 -0700
@@ -1488,8 +1488,6 @@ static int sky2_up(struct net_device *de
sky2_set_vlan_mode(hw, port, sky2->vlgrp != NULL);
#endif
- sky2->restarting = 0;
-
err = sky2_rx_start(sky2);
if (err)
goto err_out;
@@ -1502,9 +1500,6 @@ static int sky2_up(struct net_device *de
sky2_set_multicast(dev);
- /* wake queue incase we are restarting */
- netif_wake_queue(dev);
-
if (netif_msg_ifup(sky2))
printk(KERN_INFO PFX "%s: enabling interface\n", dev->name);
return 0;
@@ -1538,8 +1533,6 @@ static inline int tx_dist(unsigned tail,
/* Number of list elements available for next tx */
static inline int tx_avail(const struct sky2_port *sky2)
{
- if (unlikely(sky2->restarting))
- return 0;
return sky2->tx_pending - tx_dist(sky2->tx_cons, sky2->tx_prod);
}
@@ -1825,10 +1818,6 @@ static int sky2_down(struct net_device *
if (netif_msg_ifdown(sky2))
printk(KERN_INFO PFX "%s: disabling interface\n", dev->name);
- /* explicitly shut off tx incase we're restarting */
- sky2->restarting = 1;
- netif_tx_disable(dev);
-
/* Force flow control off */
sky2_write8(hw, SK_REG(port, GMAC_CTRL), GMC_PAUSE_OFF);
@@ -2370,7 +2359,7 @@ static inline void sky2_tx_done(struct n
{
struct sky2_port *sky2 = netdev_priv(dev);
- if (likely(netif_running(dev) && !sky2->restarting)) {
+ if (netif_running(dev)) {
netif_tx_lock(dev);
sky2_tx_complete(sky2, last);
netif_tx_unlock(dev);
@@ -4294,7 +4283,6 @@ static __devinit struct net_device *sky2
spin_lock_init(&sky2->phy_lock);
sky2->tx_pending = TX_DEF_PENDING;
sky2->rx_pending = RX_DEF_PENDING;
- sky2->restarting = 0;
hw->dev[port] = dev;
--- a/drivers/net/sky2.h 2009-08-06 08:25:15.498381527 -0700
+++ b/drivers/net/sky2.h 2009-08-06 08:25:49.580038902 -0700
@@ -2051,7 +2051,6 @@ struct sky2_port {
u8 duplex; /* DUPLEX_HALF, DUPLEX_FULL */
u8 rx_csum;
u8 wol;
- u8 restarting;
enum flow_control flow_mode;
enum flow_control flow_status;
--
^ permalink raw reply
* [PATCH 7/8] sky2: hold RTNL when doing suspend/shutdown operations
From: Stephen Hemminger @ 2009-08-06 16:12 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20090806161213.148382653@vyatta.com>
[-- Attachment #1: sky2-wol-rtnl.patch --]
[-- Type: text/plain, Size: 1197 bytes --]
The suspend and shutdown code plays with shared state. Use consistent
locking, for extra protection.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/drivers/net/sky2.c 2009-08-06 08:29:58.560356154 -0700
+++ b/drivers/net/sky2.c 2009-08-06 08:32:06.128478111 -0700
@@ -4635,6 +4635,7 @@ static int sky2_suspend(struct pci_dev *
del_timer_sync(&hw->watchdog_timer);
cancel_work_sync(&hw->restart_work);
+ rtnl_lock();
for (i = 0; i < hw->ports; i++) {
struct net_device *dev = hw->dev[i];
struct sky2_port *sky2 = netdev_priv(dev);
@@ -4652,6 +4653,7 @@ static int sky2_suspend(struct pci_dev *
sky2_write32(hw, B0_IMSK, 0);
napi_disable(&hw->napi);
sky2_power_aux(hw);
+ rtnl_unlock();
pci_save_state(pdev);
pci_enable_wake(pdev, pci_choose_state(pdev, state), wol);
@@ -4721,6 +4723,7 @@ static void sky2_shutdown(struct pci_dev
if (!hw)
return;
+ rtnl_lock();
del_timer_sync(&hw->watchdog_timer);
for (i = 0; i < hw->ports; i++) {
@@ -4735,6 +4738,7 @@ static void sky2_shutdown(struct pci_dev
if (wol)
sky2_power_aux(hw);
+ rtnl_unlock();
pci_enable_wake(pdev, PCI_D3hot, wol);
pci_enable_wake(pdev, PCI_D3cold, wol);
--
^ permalink raw reply
* [PATCH 2/8] sky2: Avoid rewinding sky2->tx_prod
From: Stephen Hemminger @ 2009-08-06 16:12 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20090806161213.148382653@vyatta.com>
[-- Attachment #1: sky2-tx1.patch --]
[-- Type: text/plain, Size: 4373 bytes --]
Rather than moving the next used transmit ring index forward
then backing it up in the case of DMA mapping failure, cleaner
to use local variable.
Signed-off-by: Mike McCormack <mikem@ring3k.org>
Acked-by: Stephen Hemminger <shemminger@vyatta.com>
---
drivers/net/sky2.c | 34 ++++++++++++++++++----------------
1 files changed, 18 insertions(+), 16 deletions(-)
--- a/drivers/net/sky2.c 2009-08-03 08:43:26.300080050 -0700
+++ b/drivers/net/sky2.c 2009-08-03 09:12:07.929961591 -0700
@@ -989,11 +989,11 @@ static void sky2_prefetch_init(struct sk
sky2_read32(hw, Y2_QADDR(qaddr, PREF_UNIT_CTRL));
}
-static inline struct sky2_tx_le *get_tx_le(struct sky2_port *sky2)
+static inline struct sky2_tx_le *get_tx_le(struct sky2_port *sky2, u16 *slot)
{
- struct sky2_tx_le *le = sky2->tx_le + sky2->tx_prod;
+ struct sky2_tx_le *le = sky2->tx_le + *slot;
- sky2->tx_prod = RING_NEXT(sky2->tx_prod, TX_RING_SIZE);
+ *slot = RING_NEXT(*slot, TX_RING_SIZE);
le->ctrl = 0;
return le;
}
@@ -1006,7 +1006,7 @@ static void tx_init(struct sky2_port *sk
sky2->tx_tcpsum = 0;
sky2->tx_last_mss = 0;
- le = get_tx_le(sky2);
+ le = get_tx_le(sky2, &sky2->tx_prod);
le->addr = 0;
le->opcode = OP_ADDR64 | HW_OWNER;
}
@@ -1565,7 +1565,8 @@ static int sky2_xmit_frame(struct sk_buf
struct sky2_hw *hw = sky2->hw;
struct sky2_tx_le *le = NULL;
struct tx_ring_info *re;
- unsigned i, len, first_slot;
+ unsigned i, len;
+ u16 slot;
dma_addr_t mapping;
u16 mss;
u8 ctrl;
@@ -1579,14 +1580,14 @@ static int sky2_xmit_frame(struct sk_buf
if (pci_dma_mapping_error(hw->pdev, mapping))
goto mapping_error;
- first_slot = sky2->tx_prod;
+ slot = sky2->tx_prod;
if (unlikely(netif_msg_tx_queued(sky2)))
printk(KERN_DEBUG "%s: tx queued, slot %u, len %d\n",
- dev->name, first_slot, skb->len);
+ dev->name, slot, skb->len);
/* Send high bits if needed */
if (sizeof(dma_addr_t) > sizeof(u32)) {
- le = get_tx_le(sky2);
+ le = get_tx_le(sky2, &slot);
le->addr = cpu_to_le32(upper_32_bits(mapping));
le->opcode = OP_ADDR64 | HW_OWNER;
}
@@ -1599,7 +1600,7 @@ static int sky2_xmit_frame(struct sk_buf
mss += ETH_HLEN + ip_hdrlen(skb) + tcp_hdrlen(skb);
if (mss != sky2->tx_last_mss) {
- le = get_tx_le(sky2);
+ le = get_tx_le(sky2, &slot);
le->addr = cpu_to_le32(mss);
if (hw->flags & SKY2_HW_NEW_LE)
@@ -1615,7 +1616,7 @@ static int sky2_xmit_frame(struct sk_buf
/* Add VLAN tag, can piggyback on LRGLEN or ADDR64 */
if (sky2->vlgrp && vlan_tx_tag_present(skb)) {
if (!le) {
- le = get_tx_le(sky2);
+ le = get_tx_le(sky2, &slot);
le->addr = 0;
le->opcode = OP_VLAN|HW_OWNER;
} else
@@ -1644,7 +1645,7 @@ static int sky2_xmit_frame(struct sk_buf
if (tcpsum != sky2->tx_tcpsum) {
sky2->tx_tcpsum = tcpsum;
- le = get_tx_le(sky2);
+ le = get_tx_le(sky2, &slot);
le->addr = cpu_to_le32(tcpsum);
le->length = 0; /* initial checksum value */
le->ctrl = 1; /* one packet */
@@ -1653,7 +1654,7 @@ static int sky2_xmit_frame(struct sk_buf
}
}
- le = get_tx_le(sky2);
+ le = get_tx_le(sky2, &slot);
le->addr = cpu_to_le32((u32) mapping);
le->length = cpu_to_le16(len);
le->ctrl = ctrl;
@@ -1674,13 +1675,13 @@ static int sky2_xmit_frame(struct sk_buf
goto mapping_unwind;
if (sizeof(dma_addr_t) > sizeof(u32)) {
- le = get_tx_le(sky2);
+ le = get_tx_le(sky2, &slot);
le->addr = cpu_to_le32(upper_32_bits(mapping));
le->ctrl = 0;
le->opcode = OP_ADDR64 | HW_OWNER;
}
- le = get_tx_le(sky2);
+ le = get_tx_le(sky2, &slot);
le->addr = cpu_to_le32((u32) mapping);
le->length = cpu_to_le16(frag->size);
le->ctrl = ctrl;
@@ -1694,6 +1695,8 @@ static int sky2_xmit_frame(struct sk_buf
le->ctrl |= EOP;
+ sky2->tx_prod = slot;
+
if (tx_avail(sky2) <= MAX_SKB_TX_LE)
netif_stop_queue(dev);
@@ -1702,7 +1705,7 @@ static int sky2_xmit_frame(struct sk_buf
return NETDEV_TX_OK;
mapping_unwind:
- for (i = first_slot; i != sky2->tx_prod; i = RING_NEXT(i, TX_RING_SIZE)) {
+ for (i = sky2->tx_prod; i != slot; i = RING_NEXT(i, TX_RING_SIZE)) {
le = sky2->tx_le + i;
re = sky2->tx_ring + i;
@@ -1722,7 +1725,6 @@ mapping_unwind:
}
}
- sky2->tx_prod = first_slot;
mapping_error:
if (net_ratelimit())
dev_warn(&hw->pdev->dev, "%s: tx mapping error\n", dev->name);
--
^ permalink raw reply
* [PATCH 5/8] sky2: Remove tx locks
From: Stephen Hemminger @ 2009-08-06 16:12 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Mike McCormack
In-Reply-To: <20090806161213.148382653@vyatta.com>
[-- Attachment #1: sky2-tx4.patch --]
[-- Type: text/plain, Size: 993 bytes --]
Should no longer be necessary.
Signed-off-by: Mike McCormack <mikem@ring3k.org>
Acked-by: Stephen Hemminger <shemminger@vyatta.com>
---
drivers/net/sky2.c | 8 +-------
1 files changed, 1 insertions(+), 7 deletions(-)
diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index ac303cb..dbedee4 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -1799,9 +1799,7 @@ static void sky2_tx_clean(struct net_device *dev)
{
struct sky2_port *sky2 = netdev_priv(dev);
- netif_tx_lock_bh(dev);
sky2_tx_complete(sky2, sky2->tx_prod);
- netif_tx_unlock_bh(dev);
}
static void sky2_tx_reset(struct sky2_port* sky2)
@@ -2369,11 +2367,7 @@ static inline void sky2_tx_done(struct net_device *dev, u16 last)
{
struct sky2_port *sky2 = netdev_priv(dev);
- if (netif_running(dev)) {
- netif_tx_lock(dev);
- sky2_tx_complete(sky2, last);
- netif_tx_unlock(dev);
- }
+ sky2_tx_complete(sky2, last);
}
static inline void sky2_skb_rx(const struct sky2_port *sky2,
--
1.5.6.5
--
^ permalink raw reply related
* [PATCH 4/8] sky2: Reset tx train after interrupts disabled.
From: Stephen Hemminger @ 2009-08-06 16:12 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Mike McCormack
In-Reply-To: <20090806161213.148382653@vyatta.com>
[-- Attachment #1: sky2-tx3.patch --]
[-- Type: text/plain, Size: 1089 bytes --]
Reseting the tx chain too soon results in invalid tx queue positions
being delivered in the status queue. This also makes sure there's no
overlap between the cleanup done by sky2_tx_clean() and
sky2_tx_done().
Signed-off-by: Mike McCormack <mikem@ring3k.org>
Acked-by: Stephen Hemminger <shemminger@vyatta.com>
---
drivers/net/sky2.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index a9bb3fa..ac303cb 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -1866,8 +1866,6 @@ static int sky2_down(struct net_device *dev)
&& port == 0 && hw->dev[1] && netif_running(hw->dev[1])))
sky2_write8(hw, SK_REG(port, GMAC_CTRL), GMC_RST_SET);
- sky2_tx_reset(sky2);
-
sky2_write8(hw, SK_REG(port, RX_GMF_CTRL_T), GMF_RST_SET);
/* Force any delayed status interrrupt and NAPI */
@@ -1892,6 +1890,8 @@ static int sky2_down(struct net_device *dev)
/* turn off LED's */
sky2_write16(hw, B0_Y2LED, LED_STAT_OFF);
+ sky2_tx_reset(sky2);
+
sky2_tx_clean(dev);
sky2_rx_clean(sky2);
--
1.5.6.5
--
^ permalink raw reply related
* [PATCH 6/8] sky2: Lock tx queue when calling sky2_down locally
From: Stephen Hemminger @ 2009-08-06 16:12 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Mike McCormack
In-Reply-To: <20090806161213.148382653@vyatta.com>
[-- Attachment #1: sky2-tx5.patch --]
[-- Type: text/plain, Size: 1331 bytes --]
Signed-off-by: Mike McCormack <mikem@ring3k.org>
Acked-by: Stephen Hemminger <shemminger@vyatta.com>
---
drivers/net/sky2.c | 13 +++++++++++--
1 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index dbedee4..972c716 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -3078,10 +3078,13 @@ static void sky2_restart(struct work_struct *work)
int i, err;
rtnl_lock();
+
for (i = 0; i < hw->ports; i++) {
dev = hw->dev[i];
- if (netif_running(dev))
+ if (netif_running(dev)) {
+ netif_tx_lock(dev);
sky2_down(dev);
+ }
}
napi_disable(&hw->napi);
@@ -3099,6 +3102,8 @@ static void sky2_restart(struct work_struct *work)
dev->name, err);
dev_close(dev);
}
+ else
+ netif_tx_unlock(dev);
}
}
@@ -3697,8 +3702,10 @@ static int sky2_set_ringparam(struct net_device *dev,
ering->tx_pending > TX_RING_SIZE - 1)
return -EINVAL;
- if (netif_running(dev))
+ if (netif_running(dev)) {
+ netif_tx_lock(dev);
sky2_down(dev);
+ }
sky2->rx_pending = ering->rx_pending;
sky2->tx_pending = ering->tx_pending;
@@ -3707,6 +3714,8 @@ static int sky2_set_ringparam(struct net_device *dev,
err = sky2_up(dev);
if (err)
dev_close(dev);
+ else
+ netif_tx_unlock(dev);
}
return err;
--
1.5.6.5
--
^ permalink raw reply related
* [PATCH 3/8] sky2: Move tx reset functionality to sky2_tx_reset()
From: Stephen Hemminger @ 2009-08-06 16:12 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Mike McCormack
In-Reply-To: <20090806161213.148382653@vyatta.com>
[-- Attachment #1: sky2-tx2.patch --]
[-- Type: text/plain, Size: 2519 bytes --]
This is refactoring and should not affect functionality.
Signed-off-by: Mike McCormack <mikem@ring3k.org>
Acked-by: Stephen Hemminger <shemminger@vyatta.com>
---
drivers/net/sky2.c | 44 ++++++++++++++++++++++++++------------------
1 files changed, 26 insertions(+), 18 deletions(-)
diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index 16a359d..a9bb3fa 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -1804,6 +1804,31 @@ static void sky2_tx_clean(struct net_device *dev)
netif_tx_unlock_bh(dev);
}
+static void sky2_tx_reset(struct sky2_port* sky2)
+{
+ unsigned port = sky2->port;
+ struct sky2_hw *hw = sky2->hw;
+
+ /* Disable Force Sync bit and Enable Alloc bit */
+ sky2_write8(hw, SK_REG(port, TXA_CTRL),
+ TXA_DIS_FSYNC | TXA_DIS_ALLOC | TXA_STOP_RC);
+
+ /* Stop Interval Timer and Limit Counter of Tx Arbiter */
+ sky2_write32(hw, SK_REG(port, TXA_ITI_INI), 0L);
+ sky2_write32(hw, SK_REG(port, TXA_LIM_INI), 0L);
+
+ /* Reset the PCI FIFO of the async Tx queue */
+ sky2_write32(hw, Q_ADDR(txqaddr[port], Q_CSR),
+ BMU_RST_SET | BMU_FIFO_RST);
+
+ /* Reset the Tx prefetch units */
+ sky2_write32(hw, Y2_QADDR(txqaddr[port], PREF_UNIT_CTRL),
+ PREF_UNIT_RST_SET);
+
+ sky2_write32(hw, RB_ADDR(txqaddr[port], RB_CTRL), RB_RST_SET);
+ sky2_write8(hw, SK_REG(port, TX_GMF_CTRL_T), GMF_RST_SET);
+}
+
/* Network shutdown */
static int sky2_down(struct net_device *dev)
{
@@ -1841,26 +1866,9 @@ static int sky2_down(struct net_device *dev)
&& port == 0 && hw->dev[1] && netif_running(hw->dev[1])))
sky2_write8(hw, SK_REG(port, GMAC_CTRL), GMC_RST_SET);
- /* Disable Force Sync bit and Enable Alloc bit */
- sky2_write8(hw, SK_REG(port, TXA_CTRL),
- TXA_DIS_FSYNC | TXA_DIS_ALLOC | TXA_STOP_RC);
-
- /* Stop Interval Timer and Limit Counter of Tx Arbiter */
- sky2_write32(hw, SK_REG(port, TXA_ITI_INI), 0L);
- sky2_write32(hw, SK_REG(port, TXA_LIM_INI), 0L);
-
- /* Reset the PCI FIFO of the async Tx queue */
- sky2_write32(hw, Q_ADDR(txqaddr[port], Q_CSR),
- BMU_RST_SET | BMU_FIFO_RST);
-
- /* Reset the Tx prefetch units */
- sky2_write32(hw, Y2_QADDR(txqaddr[port], PREF_UNIT_CTRL),
- PREF_UNIT_RST_SET);
-
- sky2_write32(hw, RB_ADDR(txqaddr[port], RB_CTRL), RB_RST_SET);
+ sky2_tx_reset(sky2);
sky2_write8(hw, SK_REG(port, RX_GMF_CTRL_T), GMF_RST_SET);
- sky2_write8(hw, SK_REG(port, TX_GMF_CTRL_T), GMF_RST_SET);
/* Force any delayed status interrrupt and NAPI */
sky2_write32(hw, STAT_LEV_TIMER_CNT, 0);
--
1.5.6.5
--
^ permalink raw reply related
* Re: [PATCH 0/7] AlacrityVM guest drivers Reply-To:
From: Gregory Haskins @ 2009-08-06 16:55 UTC (permalink / raw)
To: Arnd Bergmann, Avi Kivity
Cc: alacrityvm-devel, Michael S. Tsirkin, kvm, linux-kernel, netdev
In-Reply-To: <4A7AFBE3.5080200@redhat.com>
>>> On 8/6/2009 at 11:50 AM, in message <4A7AFBE3.5080200@redhat.com>, Avi Kivity
<avi@redhat.com> wrote:
> On 08/06/2009 06:40 PM, Arnd Bergmann wrote:
>> 3. The ioq method seems to be the real core of your work that makes
>> venet perform better than virtio-net with its virtqueues. I don't see
>> any reason to doubt that your claim is correct. My conclusion from
>> this would be to add support for ioq to virtio devices, alongside
>> virtqueues, but to leave out the extra bus_type and probing method.
>>
>
> The current conjecture is that ioq outperforms virtio because the host
> side of ioq is implemented in the host kernel, while the host side of
> virtio is implemented in userspace. AFAIK, no one pointed out
> differences in the protocol which explain the differences in performance.
There *are* protocol difference that matter, though I think they are slowly being addressed.
For an example: Earlier versions of virtio-pci had a single interrupt for all ring events, and you had to do an extra MMIO cycle to learn the proper context. That will hurt...a _lot_ especially for latency. I think recent versions of KVM switched to MSI-X per queue which fixed this particular ugly.
However, generally I think Avi is right. The main reason why it outperforms virtio-pci by such a large margin has more to do with all the various inefficiencies in the backend (such as requiring multiple hops U->K, K->U per packet), coarse locking, lack of parallel processing, etc. I went through and streamlined all the bottlenecks (such as putting the code in the kernel, reducing locking/context switches, etc).
I have every reason to believe that someone will skills/time equal to myself could develop a virtio-based backend that does not use vbus and achieve similar numbers. However, as stated in my last reply, I am interested in this backend supporting more than KVM, and I designed vbus to fill that role. Therefore, it does not interest me to endeavor such an effort if it doesn't involve a backend that is independent of KVM.
Based on this, I will continue my efforts surrounding to use of vbus including its use to accelerate KVM for AlacrityVM. If I can find a way to do this in such a way that KVM upstream finds acceptable, I would be very happy and will work towards whatever that compromise might be. OTOH, if the KVM community is set against the concept of a generalized/shared backend, and thus wants to use some other approach that does not involve vbus, that is fine too. Choice is one of the great assets of open source, eh? :)
Kind Regards,
-Greg
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox