* [patch 3/3] isdn: fix netjet build errors
From: akpm @ 2009-08-06 23:06 UTC (permalink / raw)
To: isdn; +Cc: netdev, akpm, randy.dunlap
From: Randy Dunlap <randy.dunlap@oracle.com>
Fix netjet driver link errors when ISDN_I4L is not enabled:
drivers/built-in.o: In function `mode_tiger':
netjet.c:(.text+0x325dc8): undefined reference to `isdnhdlc_rcv_init'
netjet.c:(.text+0x325dd5): undefined reference to `isdnhdlc_out_init'
drivers/built-in.o: In function `fill_dma':
netjet.c:(.text+0x325fb6): undefined reference to `isdnhdlc_encode'
drivers/built-in.o: In function `read_dma':
netjet.c:(.text+0x32631a): undefined reference to `isdnhdlc_decode'
drivers/built-in.o: In function `nj_irq':
netjet.c:(.text+0x326e01): undefined reference to `isdnhdlc_encode'
or move isdnhdlc.c to some other sub-dir..
Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
Cc: Karsten Keil <isdn@linux-pingi.de>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
drivers/isdn/hardware/mISDN/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff -puN drivers/isdn/hardware/mISDN/Kconfig~isdn-fix-netjet-build-errors drivers/isdn/hardware/mISDN/Kconfig
--- a/drivers/isdn/hardware/mISDN/Kconfig~isdn-fix-netjet-build-errors
+++ a/drivers/isdn/hardware/mISDN/Kconfig
@@ -78,6 +78,7 @@ config MISDN_NETJET
depends on PCI
select MISDN_IPAC
select ISDN_HDLC
+ select ISDN_I4L # so that make will recurse into sub-dir.
help
Enable support for Traverse Technologies NETJet PCI cards.
_
^ permalink raw reply
* [patch 1/3] isdn: hisax, fix lock imbalance
From: akpm @ 2009-08-06 23:06 UTC (permalink / raw)
To: isdn; +Cc: netdev, akpm, jirislaby, Karsten-Keil
From: Jiri Slaby <jirislaby@gmail.com>
Add omittted unlocks to 2 functions.
Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Karsten Keil <Karsten-Keil@t-online.de>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
drivers/isdn/hisax/amd7930_fn.c | 1 +
drivers/isdn/hisax/icc.c | 1 +
2 files changed, 2 insertions(+)
diff -puN drivers/isdn/hisax/amd7930_fn.c~isdn-hisax-fix-lock-imbalance drivers/isdn/hisax/amd7930_fn.c
--- a/drivers/isdn/hisax/amd7930_fn.c~isdn-hisax-fix-lock-imbalance
+++ a/drivers/isdn/hisax/amd7930_fn.c
@@ -596,6 +596,7 @@ Amd7930_l1hw(struct PStack *st, int pr,
if (cs->debug & L1_DEB_WARN)
debugl1(cs, "Amd7930: l1hw: l2l1 tx_skb exist this shouldn't happen");
skb_queue_tail(&cs->sq, skb);
+ spin_unlock_irqrestore(&cs->lock, flags);
break;
}
if (cs->debug & DEB_DLOG_HEX)
diff -puN drivers/isdn/hisax/icc.c~isdn-hisax-fix-lock-imbalance drivers/isdn/hisax/icc.c
--- a/drivers/isdn/hisax/icc.c~isdn-hisax-fix-lock-imbalance
+++ a/drivers/isdn/hisax/icc.c
@@ -470,6 +470,7 @@ ICC_l1hw(struct PStack *st, int pr, void
if (cs->debug & L1_DEB_WARN)
debugl1(cs, " l2l1 tx_skb exist this shouldn't happen");
skb_queue_tail(&cs->sq, skb);
+ spin_unlock_irqrestore(&cs->lock, flags);
break;
}
if (cs->debug & DEB_DLOG_HEX)
_
^ permalink raw reply
* [patch 1/1] atlx: strncpy does not null terminate string
From: akpm @ 2009-08-06 23:06 UTC (permalink / raw)
To: jcliburn; +Cc: csnook, jie.yang, netdev, akpm, roel.kluin
From: Roel Kluin <roel.kluin@gmail.com>
strlcpy() will always null terminate the string.
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
Cc: Jay Cliburn <jcliburn@gmail.com>
Cc: Chris Snook <csnook@redhat.com>
Cc: Jie Yang <jie.yang@atheros.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
drivers/net/atl1c/atl1c_ethtool.c | 8 ++++----
drivers/net/atlx/atl1.c | 8 ++++----
2 files changed, 8 insertions(+), 8 deletions(-)
diff -puN drivers/net/atl1c/atl1c_ethtool.c~atlx-strncpy-does-not-null-terminate-string drivers/net/atl1c/atl1c_ethtool.c
--- a/drivers/net/atl1c/atl1c_ethtool.c~atlx-strncpy-does-not-null-terminate-string
+++ a/drivers/net/atl1c/atl1c_ethtool.c
@@ -232,11 +232,11 @@ static void atl1c_get_drvinfo(struct net
{
struct atl1c_adapter *adapter = netdev_priv(netdev);
- strncpy(drvinfo->driver, atl1c_driver_name, sizeof(drvinfo->driver));
- strncpy(drvinfo->version, atl1c_driver_version,
+ strlcpy(drvinfo->driver, atl1c_driver_name, sizeof(drvinfo->driver));
+ strlcpy(drvinfo->version, atl1c_driver_version,
sizeof(drvinfo->version));
- strncpy(drvinfo->fw_version, "N/A", sizeof(drvinfo->fw_version));
- strncpy(drvinfo->bus_info, pci_name(adapter->pdev),
+ strlcpy(drvinfo->fw_version, "N/A", sizeof(drvinfo->fw_version));
+ strlcpy(drvinfo->bus_info, pci_name(adapter->pdev),
sizeof(drvinfo->bus_info));
drvinfo->n_stats = 0;
drvinfo->testinfo_len = 0;
diff -puN drivers/net/atlx/atl1.c~atlx-strncpy-does-not-null-terminate-string drivers/net/atlx/atl1.c
--- a/drivers/net/atlx/atl1.c~atlx-strncpy-does-not-null-terminate-string
+++ a/drivers/net/atlx/atl1.c
@@ -3378,11 +3378,11 @@ static void atl1_get_drvinfo(struct net_
{
struct atl1_adapter *adapter = netdev_priv(netdev);
- strncpy(drvinfo->driver, ATLX_DRIVER_NAME, sizeof(drvinfo->driver));
- strncpy(drvinfo->version, ATLX_DRIVER_VERSION,
+ strlcpy(drvinfo->driver, ATLX_DRIVER_NAME, sizeof(drvinfo->driver));
+ strlcpy(drvinfo->version, ATLX_DRIVER_VERSION,
sizeof(drvinfo->version));
- strncpy(drvinfo->fw_version, "N/A", sizeof(drvinfo->fw_version));
- strncpy(drvinfo->bus_info, pci_name(adapter->pdev),
+ strlcpy(drvinfo->fw_version, "N/A", sizeof(drvinfo->fw_version));
+ strlcpy(drvinfo->bus_info, pci_name(adapter->pdev),
sizeof(drvinfo->bus_info));
drvinfo->eedump_len = ATL1_EEDUMP_LEN;
}
_
^ permalink raw reply
* Re: [PATCH 0/7] AlacrityVM guest drivers Reply-To:
From: Ira W. Snyder @ 2009-08-06 23:23 UTC (permalink / raw)
To: Gregory Haskins
Cc: Arnd Bergmann, alacrityvm-devel, Avi Kivity, Michael S. Tsirkin,
kvm, linux-kernel, netdev
In-Reply-To: <4A7ACC940200005A00051C43@sinclair.provo.novell.com>
On Thu, Aug 06, 2009 at 10:29:08AM -0600, Gregory Haskins wrote:
> >>> 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:
[ big snip ]
> >
> > 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, the virtio vring design is very well thought out. I found it
relatively easy to port to a host+blade setup, and run virtio-net over a
physical PCI bus, connecting two physical CPUs.
>
> 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 :)
>
I think this is the major benefit of vbus. I've only started studying
the vbus code, so I don't have lots to say yet. The overview of the
management interface makes it look pretty good.
Getting two virtio-net drivers hooked together in my virtio-over-PCI
patches was nasty. If you read the thread that followed, you'll see
the lack of a management interface as a concern of mine. It was
basically decided that it could come "later". The configfs interface
vbus provides is pretty nice, IMO.
Just my two cents,
Ira
^ permalink raw reply
* Re: [RFC PATCH v1] tun: Cleanup error handling in tun_set_iff()
From: Herbert Xu @ 2009-08-07 0:00 UTC (permalink / raw)
To: Paul Moore; +Cc: Eric W. Biederman, netdev, David Miller
In-Reply-To: <200908061420.20983.paul.moore@hp.com>
On Thu, Aug 06, 2009 at 02:20:20PM -0400, Paul Moore wrote:
>
> The code currently looks something like this:
>
> err = -ENOMEM;
> buf = alloc(...);
> if (!buf)
> goto label;
>
> This means that in the common case where 'alloc()' completes without error we
> are doing an extra, unnecessary assignment where we set the value in 'err'.
> Now, if we change this slightly to match what I proposed in the patch:
>
> buf = alloc(...);
> if (!buf) {
> err = -ENOMEM;
> goto label;
> }
>
> We eliminate that extra assignment in the case where 'alloc()' completes
> without error, which should result in more efficient code (less instructions
> in the common case). Am I wrong? If that is the case I would appreciate an
> explanation ...
Your style potentially introduces a second jump which may end
up being worse compared to the extra work on a modern CPU.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [RFC PATCH v1] tun: Cleanup error handling in tun_set_iff()
From: Herbert Xu @ 2009-08-07 0:22 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: Paul Moore, netdev, David Miller
In-Reply-To: <m1zladhtoh.fsf@fess.ebiederm.org>
On Thu, Aug 06, 2009 at 08:02:22AM -0700, Eric W. Biederman wrote:
>
> 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.
You're right of course. So the race is real, but I think there
is no good reason for such parallel operations to be allowed in
the first place. So how about extending the coverage of the
locked section to the whole ioctl, like this:
tun: Extend RTNL lock coverage over whole ioctl
As it is, parts of the ioctl runs under the RTNL and parts of
it do not. The unlocked section is still protected by the BKL,
but there can be subtle races. For example, Eric Biederman and
Paul Moore observed that if two threads tried to create two tun
devices on the same file descriptor, then unexpected results
may occur.
As there isn't anything in the ioctl that is expected to sleep
indefinitely, we can prevent this from occurring by extending
the RTNL lock coverage.
This also allows to get rid of the BKL.
Finally, I changed tun_get_iff to take a tun device in order to
avoid calling tun_put which would dead-lock as it also tries to
take the RTNL lock.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 027f7ab..42b6c63 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1048,20 +1048,15 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
return err;
}
-static int tun_get_iff(struct net *net, struct file *file, struct ifreq *ifr)
+static int tun_get_iff(struct net *net, struct tun_struct *tun,
+ struct ifreq *ifr)
{
- struct tun_struct *tun = tun_get(file);
-
- if (!tun)
- return -EBADFD;
-
DBG(KERN_INFO "%s: tun_get_iff\n", tun->dev->name);
strcpy(ifr->ifr_name, tun->dev->name);
ifr->ifr_flags = tun_flags(tun);
- tun_put(tun);
return 0;
}
@@ -1105,8 +1100,8 @@ static int set_offload(struct net_device *dev, unsigned long arg)
return 0;
}
-static int tun_chr_ioctl(struct inode *inode, struct file *file,
- unsigned int cmd, unsigned long arg)
+static long tun_chr_ioctl(struct file *file, unsigned int cmd,
+ unsigned long arg)
{
struct tun_file *tfile = file->private_data;
struct tun_struct *tun;
@@ -1128,34 +1123,32 @@ static int tun_chr_ioctl(struct inode *inode, struct file *file,
(unsigned int __user*)argp);
}
+ rtnl_lock();
+
tun = __tun_get(tfile);
if (cmd == TUNSETIFF && !tun) {
- int err;
-
ifr.ifr_name[IFNAMSIZ-1] = '\0';
- rtnl_lock();
- err = tun_set_iff(tfile->net, file, &ifr);
- rtnl_unlock();
+ ret = tun_set_iff(tfile->net, file, &ifr);
- if (err)
- return err;
+ if (ret)
+ goto unlock;
if (copy_to_user(argp, &ifr, sizeof(ifr)))
- return -EFAULT;
- return 0;
+ ret = -EFAULT;
+ goto unlock;
}
-
+ ret = -EBADFD;
if (!tun)
- return -EBADFD;
+ goto unlock;
DBG(KERN_INFO "%s: tun_chr_ioctl cmd %d\n", tun->dev->name, cmd);
ret = 0;
switch (cmd) {
case TUNGETIFF:
- ret = tun_get_iff(current->nsproxy->net_ns, file, &ifr);
+ ret = tun_get_iff(current->nsproxy->net_ns, tun, &ifr);
if (ret)
break;
@@ -1201,7 +1194,6 @@ static int tun_chr_ioctl(struct inode *inode, struct file *file,
case TUNSETLINK:
/* Only allow setting the type when the interface is down */
- rtnl_lock();
if (tun->dev->flags & IFF_UP) {
DBG(KERN_INFO "%s: Linktype set failed because interface is up\n",
tun->dev->name);
@@ -1211,7 +1203,6 @@ static int tun_chr_ioctl(struct inode *inode, struct file *file,
DBG(KERN_INFO "%s: linktype set to %d\n", tun->dev->name, tun->dev->type);
ret = 0;
}
- rtnl_unlock();
break;
#ifdef TUN_DEBUG
@@ -1220,9 +1211,7 @@ static int tun_chr_ioctl(struct inode *inode, struct file *file,
break;
#endif
case TUNSETOFFLOAD:
- rtnl_lock();
ret = set_offload(tun->dev, arg);
- rtnl_unlock();
break;
case TUNSETTXFILTER:
@@ -1230,9 +1219,7 @@ static int tun_chr_ioctl(struct inode *inode, struct file *file,
ret = -EINVAL;
if ((tun->flags & TUN_TYPE_MASK) != TUN_TAP_DEV)
break;
- rtnl_lock();
ret = update_filter(&tun->txflt, (void __user *)arg);
- rtnl_unlock();
break;
case SIOCGIFHWADDR:
@@ -1248,9 +1235,7 @@ static int tun_chr_ioctl(struct inode *inode, struct file *file,
DBG(KERN_DEBUG "%s: set hw address: %pM\n",
tun->dev->name, ifr.ifr_hwaddr.sa_data);
- rtnl_lock();
ret = dev_set_mac_address(tun->dev, &ifr.ifr_hwaddr);
- rtnl_unlock();
break;
case TUNGETSNDBUF:
@@ -1273,7 +1258,10 @@ static int tun_chr_ioctl(struct inode *inode, struct file *file,
break;
};
- tun_put(tun);
+unlock:
+ rtnl_unlock();
+ if (tun)
+ tun_put(tun);
return ret;
}
@@ -1361,7 +1349,7 @@ static const struct file_operations tun_fops = {
.write = do_sync_write,
.aio_write = tun_chr_aio_write,
.poll = tun_chr_poll,
- .ioctl = tun_chr_ioctl,
+ .unlocked_ioctl = tun_chr_ioctl,
.open = tun_chr_open,
.release = tun_chr_close,
.fasync = tun_chr_fasync
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related
* Re: [net-next-2.6 PATCH 1/3] mlx4_en: Adaptive moderation policy change
From: David Miller @ 2009-08-07 2:28 UTC (permalink / raw)
To: yevgenyp; +Cc: netdev
In-Reply-To: <4A631A64.50804@mellanox.co.il>
From: Yevgeny Petrilin <yevgenyp@mellanox.co.il>
Date: Sun, 19 Jul 2009 16:06:44 +0300
> If the net device is identified as "sender" (number of sent packets
> is higher then the number of received packets and the incoming packets are
> small), set the moderation time to its low limit.
> We do it because the incoming packets are acks, and we don't want to delay them
>
> Signed-off-by: Yevgeny Petrilin <yevgenyp@mellanox.co.il>
Applied.
^ permalink raw reply
* Re: [net-next-2.6 PATCH 2/3] mlx4_en: Using real number of rings as RSS map size
From: David Miller @ 2009-08-07 2:28 UTC (permalink / raw)
To: yevgenyp; +Cc: netdev
In-Reply-To: <4A631A7C.7090504@mellanox.co.il>
From: Yevgeny Petrilin <yevgenyp@mellanox.co.il>
Date: Sun, 19 Jul 2009 16:07:08 +0300
> There is no point in using more QPs then actual number of receive rings.
> If the RSS function for two streams gives the same result modulo number
> of rings, they will arrive to the same RX ring anyway.
>
> Signed-off-by: Yevgeny Petrilin <yevgenyp@mellanox.co.il>
Applied.
^ permalink raw reply
* Re: [net-next-2.6 PATCH 3/3] mlx4_en: Not using Shared Receive Queues
From: David Miller @ 2009-08-07 2:28 UTC (permalink / raw)
To: yevgenyp; +Cc: netdev
In-Reply-To: <4A631A81.5070606@mellanox.co.il>
From: Yevgeny Petrilin <yevgenyp@mellanox.co.il>
Date: Sun, 19 Jul 2009 16:07:13 +0300
> We use 1:1 mapping between QPs and SRQs on receive side,
> so additional indirection level not required. Allocated the receive
> buffers for the RSS QPs.
>
> Signed-off-by: Yevgeny Petrilin <yevgenyp@mellanox.co.il>
Applied.
^ permalink raw reply
* Re: 2.6.31-rc5-git2 crash [net/core/flow.c:flow_cache_new_hashrnd]
From: Josh Boyer @ 2009-08-07 2:54 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: michael, netdev, David Miller, linuxppc-dev
In-Reply-To: <1249597385.24311.10.camel@pasglop>
On Fri, Aug 07, 2009 at 08:23:05AM +1000, Benjamin Herrenschmidt wrote:
>On Wed, 2009-08-05 at 00:15 +1000, Michael Ellerman wrote:
>
>> > c000000000600000 00001010 .long 0x1010
>> > 0:mon>
>> > c000000000600004 00000008 .long 0x8
>> > c000000000600008 00001013 .long 0x1013
>> > c00000000060000c 0000000f .long 0xf
>> > c000000000600010 7961626f rldimi. r1,r11,44,41
>> > c000000000600014 6f740000 xoris r20,r27,0
>> > c000000000600018 00101600 .long 0x101600
>> > c00000000060001c 00000c00 .long 0xc00
>> > c000000000600020 00000400 .long 0x400
>> > c000000000600024 00101100 .long 0x101100
>> > c000000000600028 000008e9 .long 0x8e9
>
>BTW. Corruption looks like bi_recs... ie, broken yaboot. This was fixed
>recently, yaboot would eventually crap over the kernel for certain sizes
>of the kernel image.
>
>0x1010 = BI_FIRST
>0x1013 = BI_BOOTLOADER_ID followed by 0x7961626f6f74 which is
>"yaboot" :-)
>0x1016 = BI_MACHTYPE
I know we fixed it in Fedora for F11/F12. Did it get fixed upstream?
josh
^ permalink raw reply
* Re: [PATCH ver2] Avoid enqueuing skb for default qdiscs
From: Krishna Kumar2 @ 2009-08-07 2:56 UTC (permalink / raw)
To: David Miller; +Cc: herbert, jarkao2, kaber, netdev
In-Reply-To: <20090806.134126.58668200.davem@davemloft.net>
Hi Dave,
David Miller <davem@davemloft.net> wrote on 08/07/2009 02:11:26 AM:
> > static inline int dev_requeue_skb(struct sk_buff *skb, struct Qdisc
*q)
> > {
> > q->gso_skb = skb;
> > q->qstats.requeues++;
> > + q->q.qlen++; /* it's still part of the queue */
> > __netif_schedule(q);
> >
> > return 0;
>
> I'm dubious about this new qlen accounting scheme, does it handle
> the non-bypass case properly? Can you explain how it works
> exactly?
Adding one to qlen will handle non-bypass correctly. On the next
non-bypass (or a bypass) xmit, dequeue_skb will remove gso_skb and
decrement the counter. If no more xmits are done, qdisc_reset will
also fix it. I got this idea by extending one of Jarek's patches:
commit 61c9eaf90081cbe6dc4f389e0056bff76eca19ec
Author: Jarek Poplawski <jarkao2@gmail.com>
Date: Wed Nov 5 16:02:34 2008 -0800
pkt_sched: Fix qdisc len in qdisc_peek_dequeued()
Thanks,
- KK
^ permalink raw reply
* Re: [PATCH ver2] Avoid enqueuing skb for default qdiscs
From: Krishna Kumar2 @ 2009-08-07 3:01 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: davem, herbert, Jarek Poplawski, kaber, netdev
In-Reply-To: <20090806134946.1772dd5a@nehalam>
Hi Stephen,
Stephen Hemminger <shemminger@vyatta.com> wrote on 08/07/2009 02:19:46 AM:
> > +static inline int qdisc_qlen(struct Qdisc *q)
> > +{
> > + return q->q.qlen;
> > +}
> > +
>
> Why?
> This kind of oneline accessor decreases the code clarity.
qdisc_qlen() was previously defined in sch_generic.c. I moved it
to the header file so that it could be accessed in dev.c (where
there is no code using q.qlen), so it is not new code.
Thanks,
- KK
^ permalink raw reply
* Re: [PATCH ver2] Avoid enqueuing skb for default qdiscs
From: David Miller @ 2009-08-07 3:08 UTC (permalink / raw)
To: krkumar2; +Cc: herbert, jarkao2, kaber, netdev
In-Reply-To: <OF41857EFE.FD813620-ON6525760B.000F5D7C-6525760B.00101EC7@in.ibm.com>
From: Krishna Kumar2 <krkumar2@in.ibm.com>
Date: Fri, 7 Aug 2009 08:26:04 +0530
> David Miller <davem@davemloft.net> wrote on 08/07/2009 02:11:26 AM:
>
>> I'm dubious about this new qlen accounting scheme, does it handle
>> the non-bypass case properly? Can you explain how it works
>> exactly?
>
> Adding one to qlen will handle non-bypass correctly. On the next
> non-bypass (or a bypass) xmit, dequeue_skb will remove gso_skb and
> decrement the counter. If no more xmits are done, qdisc_reset will
> also fix it. I got this idea by extending one of Jarek's patches:
Aha, I see. Thanks for the explanation.
Ok, I'll let this rip... let's see what falls out of it :-)
Will apply this to net-next-2.6 right now.
^ permalink raw reply
* Re: 2.6.31-rc5-git2 crash [net/core/flow.c:flow_cache_new_hashrnd]
From: Benjamin Herrenschmidt @ 2009-08-07 3:13 UTC (permalink / raw)
To: Josh Boyer; +Cc: netdev, David Miller, linuxppc-dev
In-Reply-To: <20090807025454.GL3095@hansolo.jdub.homelinux.org>
On Thu, 2009-08-06 at 22:54 -0400, Josh Boyer wrote:
> On Fri, Aug 07, 2009 at 08:23:05AM +1000, Benjamin Herrenschmidt wrote:
> >On Wed, 2009-08-05 at 00:15 +1000, Michael Ellerman wrote:
> >
> >> > c000000000600000 00001010 .long 0x1010
> >> > 0:mon>
> >> > c000000000600004 00000008 .long 0x8
> >> > c000000000600008 00001013 .long 0x1013
> >> > c00000000060000c 0000000f .long 0xf
> >> > c000000000600010 7961626f rldimi. r1,r11,44,41
> >> > c000000000600014 6f740000 xoris r20,r27,0
> >> > c000000000600018 00101600 .long 0x101600
> >> > c00000000060001c 00000c00 .long 0xc00
> >> > c000000000600020 00000400 .long 0x400
> >> > c000000000600024 00101100 .long 0x101100
> >> > c000000000600028 000008e9 .long 0x8e9
> >
> >BTW. Corruption looks like bi_recs... ie, broken yaboot. This was fixed
> >recently, yaboot would eventually crap over the kernel for certain sizes
> >of the kernel image.
> >
> >0x1010 = BI_FIRST
> >0x1013 = BI_BOOTLOADER_ID followed by 0x7961626f6f74 which is
> >"yaboot" :-)
> >0x1016 = BI_MACHTYPE
>
> I know we fixed it in Fedora for F11/F12. Did it get fixed upstream?
I think Tony did yes. Not sure what's the status with the "enterprise"
distros.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] macvlan: add tap device backend
From: David Miller @ 2009-08-07 3:20 UTC (permalink / raw)
To: arnd
Cc: netdev, kaber, shemminger, mst, herbert, ogerlitz, anna.fischer,
bridge, linux-kernel, evb
In-Reply-To: <1249595428-21594-1-git-send-email-arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Thu, 6 Aug 2009 21:50:28 +0000
> This is a first prototype of a new interface into the network
> stack, to eventually replace tun/tap and the bridge driver
> in certain virtual machine setups.
I don't know enough to say how good a solution this is for
the problem, but I certainly like this driver for it's
utter simplicity and minimalness.
^ permalink raw reply
* Re: [patch for 2.6.31? 2/2] gigaset: really fix chars_in_buffer
From: David Miller @ 2009-08-07 3:22 UTC (permalink / raw)
To: akpm; +Cc: netdev, tilman, greg
In-Reply-To: <200908062219.n76MJXhY024495@imap1.linux-foundation.org>
From: akpm@linux-foundation.org
Date: Thu, 06 Aug 2009 15:19:32 -0700
> From: Tilman Schmidt <tilman@imap.cc>
>
> The tty_operation chars_in_buffer() is not allowed to return a negative
> value to signal an error. Corrects the problem flagged by commit
> 23198fda7182969b619613a555f8645fdc3dc334, "tty: fix chars_in_buffers".
>
> Signed-off-by: Tilman Schmidt <tilman@imap.cc>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
I was letting Alan Cox handle changes in this area since it's
TTY stuff, and thus it probably should go to the new TTY
mad man, CC:'d :-)
> ---
>
> drivers/isdn/gigaset/interface.c | 19 +++++++------------
> 1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff -puN drivers/isdn/gigaset/interface.c~gigaset-really-fix-chars_in_buffer drivers/isdn/gigaset/interface.c
> --- a/drivers/isdn/gigaset/interface.c~gigaset-really-fix-chars_in_buffer
> +++ a/drivers/isdn/gigaset/interface.c
> @@ -408,33 +408,28 @@ static int if_write_room(struct tty_stru
> return retval;
> }
>
> -/* FIXME: This function does not have error returns */
> -
> static int if_chars_in_buffer(struct tty_struct *tty)
> {
> struct cardstate *cs;
> - int retval = -ENODEV;
> + int retval = 0;
>
> cs = (struct cardstate *) tty->driver_data;
> if (!cs) {
> pr_err("%s: no cardstate\n", __func__);
> - return -ENODEV;
> + return 0;
> }
>
> gig_dbg(DEBUG_IF, "%u: %s()", cs->minor_index, __func__);
>
> - if (mutex_lock_interruptible(&cs->mutex))
> - return -ERESTARTSYS; // FIXME -EINTR?
> + mutex_lock(&cs->mutex);
>
> - if (!cs->connected) {
> + if (!cs->connected)
> gig_dbg(DEBUG_IF, "not connected");
> - retval = -ENODEV;
> - } else if (!cs->open_count)
> + else if (!cs->open_count)
> dev_warn(cs->dev, "%s: device not opened\n", __func__);
> - else if (cs->mstate != MS_LOCKED) {
> + else if (cs->mstate != MS_LOCKED)
> dev_warn(cs->dev, "can't write to unlocked device\n");
> - retval = -EBUSY;
> - } else
> + else
> retval = cs->ops->chars_in_buffer(cs);
>
> mutex_unlock(&cs->mutex);
> _
^ permalink raw reply
* Re: [RFC PATCH v1] tun: Cleanup error handling in tun_set_iff()
From: David Miller @ 2009-08-07 3:40 UTC (permalink / raw)
To: herbert; +Cc: ebiederm, paul.moore, netdev
In-Reply-To: <20090807002243.GB1566@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Fri, 7 Aug 2009 10:22:44 +1000
> tun: Extend RTNL lock coverage over whole ioctl
>
> As it is, parts of the ioctl runs under the RTNL and parts of
> it do not. The unlocked section is still protected by the BKL,
> but there can be subtle races. For example, Eric Biederman and
> Paul Moore observed that if two threads tried to create two tun
> devices on the same file descriptor, then unexpected results
> may occur.
>
> As there isn't anything in the ioctl that is expected to sleep
> indefinitely, we can prevent this from occurring by extending
> the RTNL lock coverage.
>
> This also allows to get rid of the BKL.
>
> Finally, I changed tun_get_iff to take a tun device in order to
> avoid calling tun_put which would dead-lockt as it also tries to
> take the RTNL lock.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
This looks good after a quick audit, Eric what say you?
^ permalink raw reply
* Re: 2.6.31-rc5-git2 crash [net/core/flow.c:flow_cache_new_hashrnd]
From: Tony Breeds @ 2009-08-07 3:49 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: netdev, David Miller, linuxppc-dev
In-Reply-To: <1249614835.24311.11.camel@pasglop>
On Fri, Aug 07, 2009 at 01:13:54PM +1000, Benjamin Herrenschmidt wrote:
> I think Tony did yes. Not sure what's the status with the "enterprise"
> distros.
It's fixed in F11 and F12 (thanks to dwmw2), it's in my for-1.3.15 branch for
yaboot. We're working on updateing the version in the enterprise, we'll see
what the next release holds.
Yours Tony
^ permalink raw reply
* Re: [PATCH][RFC] net/bridge: add basic VEPA support
From: Stephen Hemminger @ 2009-08-07 4:00 UTC (permalink / raw)
To: Fischer, Anna
Cc: bridge@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, virtualization@lists.linux-foundation.org,
evb@yahoogroups.com, davem@davemloft.net, kaber@trash.net,
adobriyan@gmail.com, Arnd Bergmann, Paul Congdon (UC Davis)
In-Reply-To: <0199E0D51A61344794750DC57738F58E67D2DCECBB@GVW1118EXC.americas.hpqcorp.net>
On Mon, 15 Jun 2009 17:33:10 +0000
"Fischer, Anna" <anna.fischer@hp.com> wrote:
> This patch adds basic Virtual Ethernet Port Aggregator (VEPA)
> capabilities to the Linux kernel Ethernet bridging code.
>
> A Virtual Ethernet Port Aggregator (VEPA) is a capability within
> a physical end station that collaborates with an adjacent, external
> bridge to provide distributed bridging support between multiple
> virtual end stations and external networks. The VEPA collaborates
> by forwarding all station-originated frames to the adjacent bridge
> for frame processing and frame relay (including so-called 'hairpin'
> forwarding) and by steering and replicating frames received from
> the VEPA uplink to the appropriate destinations. A VEPA may be
> implemented in software or in conjunction with embedded hardware.
>
> In particular, the patch extends the Linux Ethernet bridge to act as
> (1) a VEPA - for this we have added VEPA forwarding functionality and
> added a configuration option for a VEPA uplink port, or as
> (2) a bridge supporting 'hairpin' forwarding - for this we have added a
> bridge port 'hairpin' mode which allows sending frames back out
> through the port the frame was received on.
>
> Configuration of VEPA capabilities through Linux userspace bridge
> utilities is provided by an additional patch 'bridge-utils: add
> basic VEPA support'.
After reading more about this, I am not convinced this should be part
of the bridge code. The bridge code really consists of two parts:
forwarding table and optional spanning tree. Well the VEPA code
short circuits both of these; it can't imagine it working
with STP turned on. The only part of bridge code that really gets
used by this are the receive packet hooks and the crufty old
API.
So instead of adding more stuff to existing bridge code, why not
have a new driver for just VEPA. You could
do it with a simple version of macvlan type driver.
^ permalink raw reply
* Re: [patch for 2.6.31? 2/2] gigaset: really fix chars_in_buffer
From: Greg KH @ 2009-08-07 3:50 UTC (permalink / raw)
To: David Miller; +Cc: akpm, netdev, tilman
In-Reply-To: <20090806.202227.224933454.davem@davemloft.net>
On Thu, Aug 06, 2009 at 08:22:27PM -0700, David Miller wrote:
> From: akpm@linux-foundation.org
> Date: Thu, 06 Aug 2009 15:19:32 -0700
>
> > From: Tilman Schmidt <tilman@imap.cc>
> >
> > The tty_operation chars_in_buffer() is not allowed to return a negative
> > value to signal an error. Corrects the problem flagged by commit
> > 23198fda7182969b619613a555f8645fdc3dc334, "tty: fix chars_in_buffers".
> >
> > Signed-off-by: Tilman Schmidt <tilman@imap.cc>
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>
> I was letting Alan Cox handle changes in this area since it's
> TTY stuff, and thus it probably should go to the new TTY
> mad man, CC:'d :-)
Thanks, I'll queue it up.
greg k-h
^ permalink raw reply
* Re: [RFC PATCH v1] tun: Cleanup error handling in tun_set_iff()
From: Eric W. Biederman @ 2009-08-07 4:22 UTC (permalink / raw)
To: David Miller; +Cc: herbert, paul.moore, netdev
In-Reply-To: <20090806.204032.39020854.davem@davemloft.net>
David Miller <davem@davemloft.net> writes:
> From: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Fri, 7 Aug 2009 10:22:44 +1000
>
>> tun: Extend RTNL lock coverage over whole ioctl
>>
>> As it is, parts of the ioctl runs under the RTNL and parts of
>> it do not. The unlocked section is still protected by the BKL,
>> but there can be subtle races. For example, Eric Biederman and
>> Paul Moore observed that if two threads tried to create two tun
>> devices on the same file descriptor, then unexpected results
>> may occur.
>>
>> As there isn't anything in the ioctl that is expected to sleep
>> indefinitely, we can prevent this from occurring by extending
>> the RTNL lock coverage.
>>
>> This also allows to get rid of the BKL.
>>
>> Finally, I changed tun_get_iff to take a tun device in order to
>> avoid calling tun_put which would dead-lockt as it also tries to
>> take the RTNL lock.
>>
>> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> This looks good after a quick audit, Eric what say you?
Looks good to me.
Eric
^ permalink raw reply
* Re: [PATCH 4/7] vbus-proxy: add a pci-to-vbus bridge
From: Gregory Haskins @ 2009-08-07 4:42 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: alacrityvm-devel, Ira W. Snyder, linux-kernel, netdev
In-Reply-To: <200908070057.54795.arnd@arndb.de>
>>> On 8/6/2009 at 6:57 PM, in message <200908070057.54795.arnd@arndb.de>, Arnd
Bergmann <arnd@arndb.de> wrote:
> On Thursday 06 August 2009, Gregory Haskins wrote:
>> >>> On 8/6/2009 at 1:03 PM, in message <200908061903.05083.arnd@arndb.de>, Arnd
> Bergmann <arnd@arndb.de> wrote:
>> Here are some of my arguments against it:
>>
>> 1) there is an ample PCI model that is easy to work with when you are in
> QEMU and using its device model (and you get it for free). Its the path of
> least resistance. For something in kernel, it is more awkward to try to
> coordinate the in-kernel state with the PCI state. Afaict, you either need to
> have it live partially in both places, or you need some PCI emulation in the
> kernel.
>
> True, if the whole hypervisor is in the host kernel, then doing full PCI
> emulation would be
> insane.
In this case, the entire bus is more or less self contained and in-kernel. We technically *do* still have qemu present, however, so you are right there.
> I was assuming that all of the setup code still lived in host user
> space.
Very little. Just enough to register the PCI device, handle the MMIO/PIO/MSI configuration, etc. All the bus management uses the standard vbus management interface (configfs/sysfs)
> What is the reason why it cannot?
Its not that it "can't" per se.. Its just awkward to have it live in two places, and I would need to coordinate in-kernel changes to userspace, etc. Today I do not need to do this: i.e. the model in userspace is very simple.
> Do you want to use something other than qemu,
Well, only in the sense that vbus has its own management interface and bus model, and I want them to be used.
> do you think this will impact performance, or something else?
performance is not a concern for this aspect of operation.
>
>> 2) The signal model for the 1:1 design is not very flexible IMO.
>> 2a) I want to be able to allocate dynamic signal paths, not pre-allocate
> msi-x vectors at dev-add.
>
> I believe msi-x implies that the interrupt vectors get added by the device
> driver
> at run time, unlike legacy interrupts or msi. It's been a while since I
> dealt with
> that though.
Yeah, its been a while for me too. I would have to look at the spec again.
My understanding was that its just a slight variation of msi, with some of the constraints revised (no <= 32 vector limit, etc). Perhaps it is fancier than that and 2a is unfounded. TBD.
>
>> 2b) I also want to collapse multiple interrupts together so as to
> minimize the context switch rate (inject + EIO overhead). My design
> effectively has "NAPI" for interrupt handling. This helps when the system
> needs it the most: heavy IO.
>
> That sounds like a very useful concept in general, but this seems to be a
> detail of the interrupt controller implementation. If the IO-APIC cannot
> do what you want here, maybe we just need a paravirtual IRQ controller
> driver, like e.g. the PS3 has.
Yeah, I agree this could be a function of the APIC code. Do note that I mentioned this in passing to Avi a few months ago but FWIW he indicated at that time that he is not interested in making the APIC PV.
Also, I almost forgot an important one. Add:
2c) Interrupt prioritization. I want to be able to assign priority to interrupts and handle them in priority order.
>
>> 3) The 1:1 model is not buying us much in terms of hotplug. We don't really
> "use" PCI very much even in virtio. Its a thin-shim of uniform dev-ids to
> resurface to the virtio-bus as something else. With LDM, hotplug is
> ridiculously easy anyway, so who cares. I already need an event channel
> anyway for (2b) anyway, so the devadd/devdrop events are trivial to handle.
>
> I agree for Linux guests, but when you want to run other guest operating
> systems,
> PCI hotplug is probably the most common interface for this. AFAIK, the
> windows
> virtio-net driver does not at all have a concept of a virtio layer but is
> simply
> a network driver for a PCI card. The same could be applied any other device,
> possibly with some library code doing all the queue handling in a common
> way.l
I was told it also has a layering like Linux, but I haven't actually seen the code myself, so I do not know if this is true.
>
>> 4) communicating with something efficiently in-kernel requires more finesse
> than basic PIO/MMIO. There are tricks you can do to get around this, but
> with 1:1 you would have to do this trick repeatedly for each device. Even
> with a library solution to help, you still have per-cpu .data overhead and cpu
> hotplug overhead to get maximum performance. With my "bridge" model, I do it
> once, which I believe is ideal.
>>
>> 5) 1:1 is going to quickly populate the available MMIO/PIO and IDT slots for
> any kind of medium to large configuration. The bridge model scales better in
> this regard.
>
> We don't need to rely on PIO, it's just the common interface that all
> hypervisors
> can easily support. We could have different underlying methods for the
> communication
> if space or performance becomes a bottleneck because of this.
Heh...I already proposed an alternative, which incidentally was shot down:
http://lkml.org/lkml/2009/5/5/132
(in the end, I think we agreed that a technique of tunneling PIO/MMIO over hypercaills would be better than introducing a new namespace. But we also decided that the difference between PIO and PIOoHC was too small to care, and we don't care about non-x86)
But in any case, my comment still stands: 1:1 puts load on the PIO emulation (even if you use PIOoHC). I am not sure this can be easily worked around.
>
>> So based on that, I think the bridge model works better for vbus. Perhaps
> you can convince me otherwise ;)
>
> Being able to define all of it in the host kernel seems to be the major
> advantage of your approach, the other points you mentioned are less
> important IMHO. The question is whether that is indeed a worthy goal,
> or if it should just live in user space as with the qemu PCI code.
I don't think we can gloss over these so easily. They are all important to me, particularly 2b and 2c.
>
>> >> 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?
>> >
>> > Your PCI device does not serve any real purpose as far as I can tell
>>
>> That is certainly debatable. Its purpose is as follows:
>>
>> 1) Allows a guest to discover the vbus feature (fwiw: I used to do this with
> cpuid)
>
> true, I missed that.
>
>> 2) Allows the guest to establish proper context to communicate with the
> feature (mmio, pio, and msi) (fwiw: i used to use hypercalls)
>> 3) Access the virtual-devices that have been configured for the feature
>>
>> Correct me if I am wrong: Isn't this more of less the exact intent of
> something like an LDM bus (vbus-proxy) and a PCI-BRIDGE? Other than the
> possibility that there might be some mergable overlap (still debatable), I
> don't think its fair to say that this does not serve a purpose.
>
> I guess you are right on that. An interesting variation of that would be
> make the
> child devices of it virtio devices again though: Instead of the PCI
> emulation code
> in the host kernel, you could define a simpler interface to the same effect.
> So the
> root device would be a virtio-pci device, below which you can have
> virtio-virtio
> devices.
Interesting....but note I think that is effectively what I do today (with virtio-vbus) except you wouldn't have the explicit vbus-proxy model underneath. Also, if 1:1 via PCI is important for windows, that solution would have the same problem that the virtio-vbus model does.
>
>> >, you could just as well have a root device as a parent for all the vbus
> devices
>> > if you do your device probing like this.
>>
>> Yes, I suppose the "bridge" could have been advertised as a virtio-based root
> device. In this way, the virtio probe() would replace my pci probe() for
> feature discovery, and a virtqueue could replace my msi+ioq for the eventq
> channel.
>>
>> I see a few issues with that, however:
>>
>> 1) The virtqueue library, while a perfectly nice ring design at the metadata
> level, does not have an API that is friendly to kernel-to-kernel communication.
> It was designed more for frontend use to some remote backend. The IOQ
> library on the other hand, was specifically designed to support use as
> kernel-to-kernel (see north/south designations). So this made life easier
> for me. To do what you propose, the eventq channel would need to terminate
> in kernel, and I would thus be forced to deal that the potential API
> problems.
>
> Well, virtqueues are not that bad for kernel-to-kernel communication, as Ira
> mentioned
> referring to his virtio-over-PCI driver. You can have virtqueues on both
> sides, having
> the host kernel create a pair of virtqueues (one in user aka guest space,
> one in the host
> kernel), with the host virtqueue_ops doing copy_{to,from}_user to move data
> between them.
Its been a while since I looked, so perhaps I am wrong here. I will look again.
>
> If you have that, you can actually use the same virtio_net driver in both
> guest and
> host kernel, just communicating over different virtio implementations.
> Interestingly,
> that would mean that you no longer need a separation between guest and host
> device
> drivers (vbus and vbus-proxy in your case) but could use the same device
> abstraction
> with just different transports to back the shm-signal or virtqueue.
Actually, I think there are some problems with that model (such as management of the interface). virtio-net really wants to connect to a virtio-net-backend (such as the one in qemu or vbus). It wasn't designed to connect back to back like that. I think you will quickly run into problems similar to what Ira faced with virtio-over-PCI with that model.
>
>> 2) I would need to have Avi et. al. allocate a virtio vector to use from
> their namespace, which I am sure they wont be willing to do until they accept
> my design. Today, I have a nice conflict free PCI ID to use as I see fit.
>
> My impression is the opposite: as long as you try to reinvent everything at
> once,
> you face opposition, but if you just improve parts of the existing design
> one
> by one (like eventfd), I think you will find lots of support.
>
>> Im sure both of these hurdles are not insurmountable, but I am left
> scratching my head as to why its worth the effort. It seems to me its a "six
> of one, half-dozen of the other" kind of scenario. Either I write a qemu PCI
> device and pci-bridge driver, or I write a qemu virtio-devicve and virtio root
> driver.
>>
>> In short: What does this buy us, or did you mean something else?
>
> In my last reply, I was thinking of a root device that can not be probed
> like a PCI device.
IIUC, because you missed the "feature discovery" function of the bridge, you thought this was possible but now see it is problematic? Or are you saying that this concept is still valid and should be considered? I think its the former, but wanted to be sure we were on the same page.
>
>> > However, assuming that you do the IMHO right thing to do probing like
>> > virtio with a PCI device for each slave, the code will be almost the same
>> > as virtio-pci and the two can be the same.
>>
>> Can you elaborate?
>
> Well, let me revise based on the discussion:
>
> The main point that remains is that I think a vbus-proxy should be the same
> as a
> virtio device. This could be done by having (as in my earlier mails) a PCI
> device
> per vbus-proxy, with devcall implemented in PIO or config-space and additional
> shm/shm-signal,
So the problem with this model is the points I made earlier (such as 2b, 2c).
I do agree with you that the *lack* of this model may be problematic for Windows, depending on the answer w.r.t. what the windows drivers look like.
> or it could be a single virtio device from virtio-pci or one
> of the other existing provides that connects you with a new virtio provider
> sitting in the host kernel. This provider has child devices for any endpoint
> (virtio-net, venet, ...) that is implemented in the host kernel.
This is an interesting idea, but I think it also has problems.
What we do get with having the explicit vbus-proxy exposed in the stack (aside from being able to support "vbus native" drivers, like venet) is a neat way to map vbus-isms into virtio-isms. For instance, vbus uses a string-based device id, and virtio uses a PCI-ID. Using this as an intermediate layer allows the "virtio" vbus-id to know that we issue dev->call(GETID) to obtain the PCI-ID value of this virtio-device, and we should publish this result to virtio-bus.
Without this intermediate layer, the vbus identity scheme would have to be compatible with virtio PCI-ID based scheme, and I think this is suboptimal for the overall design of vbus.
[snip]
>
>> Regarding the id->handle indirection:
>>
>> Internally, the DEVOPEN call translates an "id" to a "handle". The handle
> is just a token to help ensure that the caller actually opened the device
> successfully. Note that the "id" namespace is 0 based. Therefore, something
> like an errant DEVCALL(0) would be indistinguishable from a legit request.
> Using the handle abstraction gives me a slightly more robust mechanism to
> ensure the caller actually meant to call the host, and was in the proper
> context to do so. For one thing, if the device had never been opened, this
> would have failed before it ever reached the model. Its one more check I can
> do at the infrastructure level, and one less thing each model has to look out
> for.
>>
>> Is the id->handle translation critical? No, i'm sure we could live without
> it, but I also don't think it hurts anything. It allows the overall code to
> be slightly more robust, and the individual model code to be slightly less
> complicated. Therefore, I don't see a problem.
>
> Right, assuming your model with all vbus devices behind a single PCI device,
> your
> handle does not hurt, it's the equivalent of a bus/dev/fn number or an MMIO
> address.
Agreed
Thanks Arnd,
-Greg
^ permalink raw reply
* Re: [PATCH V2 net-next-2.6] net: relax dst refcnt in input path
From: Eric Dumazet @ 2009-08-07 5:05 UTC (permalink / raw)
To: David Miller; +Cc: kaber, netdev, markmc
In-Reply-To: <20090806.133518.263376332.davem@davemloft.net>
David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Wed, 22 Jul 2009 17:40:06 +0200
>
>> [PATCH net-next-2.6] net: relax dst refcnt in input path
>
>
> Two things:
>
> 1) Don't use a boolean name like "noref" it results in using
> double-negatives in one's mind while trying to read and understand
> the code.
>
> Call these arguments "need_ref" or something like that.
>
> Also, use "bool" type.
Sure
>
> 2) I wonder about this:
>
>> @@ -1700,9 +1700,15 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
>> * If device doesnt need skb->dst, release it right now while
>> * its hot in this cpu cache
>> */
>> - if (dev->priv_flags & IFF_XMIT_DST_RELEASE)
>> - skb_dst_drop(skb);
>> -
>> + if (skb_dst(skb)) {
>> + if (dev->priv_flags & IFF_XMIT_DST_RELEASE)
>> + skb_dst_drop(skb);
>> + else
>> + /*
>> + * make sure dst was refcounted by caller
>> + */
>> + WARN_ON(!(skb->_skb_dst & SKB_DST_REFTAKEN));
>> + }
>> rc = ops->ndo_start_xmit(skb, dev);
>> if (rc == NETDEV_TX_OK)
>
> So, won't this warning trigger if we are forwarding to a device that
> does not set IFF_XMIT_DST_RELEASE?
If this device has a queue, we do a skb_dst_force() in dev_queue_xmit()
>
> If I understand things correctly, in IPv4 when we're not delivering to
> a socket, we don't take a reference.
yes, that would be the trick
>
> We'll get here from the forwarding path with a DST, and the target TX
> device has IFF_XMIT_DST_RELEASE clear, the WARN_ON above will trigger.
I added this warning to make sure I called skb_dst_force(skb)
from dev_queue_xmit() if device has a queue (packet might be queued
and thus escape from rcu lock section)
I suppose I should also skb_dst_force() if device has no queue and
has IFF_XMIT_DST_RELEASE cleared. Or should audit all these devices
and insert the skb_dst_force() if the packet must be queued in a driver queue,
and its dst needed later.
I'll respin patch anyway since commit bbd8a0d3a3b65d341437f8b99c828fa5cc29c739
(net: Avoid enqueuing skb for default qdiscs)
changed too many things, when I'll come back from vacations in two weeks.
Or if you prefer, I might submit it today (my vacations start tomorrow), but
please dont apply it while I cannot react to any bug report :)
Thanks a lot
^ permalink raw reply
* Re: [PATCH V2 net-next-2.6] net: relax dst refcnt in input path
From: David Miller @ 2009-08-07 5:09 UTC (permalink / raw)
To: eric.dumazet; +Cc: kaber, netdev, markmc
In-Reply-To: <4A7BB622.2070007@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 07 Aug 2009 07:05:38 +0200
> I'll respin patch anyway since commit bbd8a0d3a3b65d341437f8b99c828fa5cc29c739
> (net: Avoid enqueuing skb for default qdiscs)
> changed too many things, when I'll come back from vacations in two weeks.
That's fine.
> Or if you prefer, I might submit it today (my vacations start tomorrow), but
> please dont apply it while I cannot react to any bug report :)
Take your time, there is no rush.
^ permalink raw reply
* [PATCH net-2.6] can: Fix raw_getname() leak
From: Eric Dumazet @ 2009-08-07 6:27 UTC (permalink / raw)
To: David S. Miller; +Cc: Linux Netdev List, Oliver Hartkopp
raw_getname() can leak 10 bytes of kernel memory to user
(two bytes hole between can_family and can_ifindex,
8 bytes at the end of sockaddr_can structure)
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/net/can/raw.c b/net/can/raw.c
index f4cc445..db3152d 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -401,6 +401,7 @@ static int raw_getname(struct socket *sock, struct sockaddr *uaddr,
if (peer)
return -EOPNOTSUPP;
+ memset(addr, 0, sizeof(*addr));
addr->can_family = AF_CAN;
addr->can_ifindex = ro->ifindex;
^ permalink raw reply related
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