* 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: 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: [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: [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: [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: 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 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: [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: 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: 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: [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: [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 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: [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: [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: [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
* [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
* [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 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 2/3] hfc_usb: fix read buffer overflow
From: akpm @ 2009-08-06 23:06 UTC (permalink / raw)
To: isdn; +Cc: netdev, akpm, roel.kluin
From: Roel Kluin <roel.kluin@gmail.com>
Check whether index is within bounds before testing the element.
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
Cc: Karsten Keil <isdn@linux-pingi.de>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
drivers/isdn/hisax/hfc_usb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff -puN drivers/isdn/hisax/hfc_usb.c~hfc_usb-fix-read-buffer-overflow drivers/isdn/hisax/hfc_usb.c
--- a/drivers/isdn/hisax/hfc_usb.c~hfc_usb-fix-read-buffer-overflow
+++ a/drivers/isdn/hisax/hfc_usb.c
@@ -817,8 +817,8 @@ collect_rx_frame(usb_fifo * fifo, __u8 *
}
/* we have a complete hdlc packet */
if (finish) {
- if ((!fifo->skbuff->data[fifo->skbuff->len - 1])
- && (fifo->skbuff->len > 3)) {
+ if (fifo->skbuff->len > 3 &&
+ !fifo->skbuff->data[fifo->skbuff->len - 1]) {
if (fifon == HFCUSB_D_RX) {
DBG(HFCUSB_DBG_DCHANNEL,
_
^ permalink raw reply
* [patch 1/1] proc connector: add event for process becoming session leader
From: akpm @ 2009-08-06 23:05 UTC (permalink / raw)
To: davem; +Cc: netdev, akpm, scott, johnpol, matthltc, oleg
From: Scott James Remnant <scott@ubuntu.com>
The act of a process becoming a session leader is a useful signal to a
supervising init daemon such as Upstart.
While a daemon will normally do this as part of the process of becoming a
daemon, it is rare for its children to do so. When the children do, it is
nearly always a sign that the child should be considered detached from the
parent and not supervised along with it.
The poster-child example is OpenSSH; the per-login children call setsid()
so that they may control the pty connected to them. If the primary daemon
dies or is restarted, we do not want to consider the per-login children
and want to respawn the primary daemon without killing the children.
This patch adds a new PROC_SID_EVENT and associated structure to the
proc_event event_data union, it arranges for this to be emitted when the
special PIDTYPE_SID pid is set.
[akpm@linux-foundation.org: coding-style fixes]
Signed-off-by: Scott James Remnant <scott@ubuntu.com>
Acked-by: Matt Helsley <matthltc@us.ibm.com>
Cc: Oleg Nesterov <oleg@tv-sign.ru>
Cc: Evgeniy Polyakov <johnpol@2ka.mipt.ru>
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
drivers/connector/cn_proc.c | 25 +++++++++++++++++++++++++
include/linux/cn_proc.h | 10 ++++++++++
kernel/exit.c | 4 +++-
3 files changed, 38 insertions(+), 1 deletion(-)
diff -puN drivers/connector/cn_proc.c~proc-connector-add-event-for-process-becoming-session-leader drivers/connector/cn_proc.c
--- a/drivers/connector/cn_proc.c~proc-connector-add-event-for-process-becoming-session-leader
+++ a/drivers/connector/cn_proc.c
@@ -139,6 +139,31 @@ void proc_id_connector(struct task_struc
cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL);
}
+void proc_sid_connector(struct task_struct *task)
+{
+ struct cn_msg *msg;
+ struct proc_event *ev;
+ struct timespec ts;
+ __u8 buffer[CN_PROC_MSG_SIZE];
+
+ if (atomic_read(&proc_event_num_listeners) < 1)
+ return;
+
+ msg = (struct cn_msg *)buffer;
+ ev = (struct proc_event *)msg->data;
+ get_seq(&msg->seq, &ev->cpu);
+ ktime_get_ts(&ts); /* get high res monotonic timestamp */
+ put_unaligned(timespec_to_ns(&ts), (__u64 *)&ev->timestamp_ns);
+ ev->what = PROC_EVENT_SID;
+ ev->event_data.sid.process_pid = task->pid;
+ ev->event_data.sid.process_tgid = task->tgid;
+
+ memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id));
+ msg->ack = 0; /* not used */
+ msg->len = sizeof(*ev);
+ cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL);
+}
+
void proc_exit_connector(struct task_struct *task)
{
struct cn_msg *msg;
diff -puN include/linux/cn_proc.h~proc-connector-add-event-for-process-becoming-session-leader include/linux/cn_proc.h
--- a/include/linux/cn_proc.h~proc-connector-add-event-for-process-becoming-session-leader
+++ a/include/linux/cn_proc.h
@@ -52,6 +52,7 @@ struct proc_event {
PROC_EVENT_EXEC = 0x00000002,
PROC_EVENT_UID = 0x00000004,
PROC_EVENT_GID = 0x00000040,
+ PROC_EVENT_SID = 0x00000080,
/* "next" should be 0x00000400 */
/* "last" is the last process event: exit */
PROC_EVENT_EXIT = 0x80000000
@@ -89,6 +90,11 @@ struct proc_event {
} e;
} id;
+ struct sid_proc_event {
+ __kernel_pid_t process_pid;
+ __kernel_pid_t process_tgid;
+ } sid;
+
struct exit_proc_event {
__kernel_pid_t process_pid;
__kernel_pid_t process_tgid;
@@ -102,6 +108,7 @@ struct proc_event {
void proc_fork_connector(struct task_struct *task);
void proc_exec_connector(struct task_struct *task);
void proc_id_connector(struct task_struct *task, int which_id);
+void proc_sid_connector(struct task_struct *task);
void proc_exit_connector(struct task_struct *task);
#else
static inline void proc_fork_connector(struct task_struct *task)
@@ -114,6 +121,9 @@ static inline void proc_id_connector(str
int which_id)
{}
+static inline void proc_sid_connector(struct task_struct *task)
+{}
+
static inline void proc_exit_connector(struct task_struct *task)
{}
#endif /* CONFIG_PROC_EVENTS */
diff -puN kernel/exit.c~proc-connector-add-event-for-process-becoming-session-leader kernel/exit.c
--- a/kernel/exit.c~proc-connector-add-event-for-process-becoming-session-leader
+++ a/kernel/exit.c
@@ -359,8 +359,10 @@ void __set_special_pids(struct pid *pid)
{
struct task_struct *curr = current->group_leader;
- if (task_session(curr) != pid)
+ if (task_session(curr) != pid) {
change_pid(curr, PIDTYPE_SID, pid);
+ proc_sid_connector(curr);
+ }
if (task_pgrp(curr) != pid)
change_pid(curr, PIDTYPE_PGID, pid);
_
^ permalink raw reply
* [patch 1/1] irda: fix read buffer overflow
From: akpm @ 2009-08-06 23:06 UTC (permalink / raw)
To: samuel; +Cc: netdev, akpm, roel.kluin
From: Roel Kluin <roel.kluin@gmail.com>
io[i] is read before the bounds check on i, order should be reversed.
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
Cc: Samuel Ortiz <samuel@sortiz.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
drivers/net/irda/w83977af_ir.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff -puN drivers/net/irda/w83977af_ir.c~irda-fix-read-buffer-overflow drivers/net/irda/w83977af_ir.c
--- a/drivers/net/irda/w83977af_ir.c~irda-fix-read-buffer-overflow
+++ a/drivers/net/irda/w83977af_ir.c
@@ -115,7 +115,7 @@ static int __init w83977af_init(void)
IRDA_DEBUG(0, "%s()\n", __func__ );
- for (i=0; (io[i] < 2000) && (i < ARRAY_SIZE(dev_self)); i++) {
+ for (i=0; i < ARRAY_SIZE(dev_self) && io[i] < 2000; i++) {
if (w83977af_open(i, io[i], irq[i], dma[i]) == 0)
return 0;
}
_
^ permalink raw reply
* [net-2.6 PATCH 2/2] ixgbe: Disable packet split only on FCoE queues in 82599
From: Jeff Kirsher @ 2009-08-06 23:05 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, Yi Zou, Peter P Waskiewicz Jr, Jeff Kirsher
In-Reply-To: <20090806230514.28347.51172.stgit@localhost.localdomain>
From: Yi Zou <yi.zou@intel.com>
For 82599, packet split has to be disabled for FCoE direct data placement.
However, this is only required on received queues allocated for FCoE. This
patch adds a per ring flags to indicate if packet split is disabled on a
per queue basis, particularly for FCoE, as packet split must be disabled
for large receive using direct data placement (DDP).
Signed-off-by: Yi Zou <yi.zou@intel.com>
Acked-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ixgbe/ixgbe.h | 2 ++
drivers/net/ixgbe/ixgbe_main.c | 33 +++++++++++++++++----------------
2 files changed, 19 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe.h b/drivers/net/ixgbe/ixgbe.h
index e11d83d..2c4dc82 100644
--- a/drivers/net/ixgbe/ixgbe.h
+++ b/drivers/net/ixgbe/ixgbe.h
@@ -136,6 +136,8 @@ struct ixgbe_ring {
u8 queue_index; /* needed for multiqueue queue management */
+#define IXGBE_RING_RX_PS_ENABLED (u8)(1)
+ u8 flags; /* per ring feature flags */
u16 head;
u16 tail;
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index e361b7f..e3cb949 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -585,7 +585,7 @@ static void ixgbe_alloc_rx_buffers(struct ixgbe_adapter *adapter,
rx_desc = IXGBE_RX_DESC_ADV(*rx_ring, i);
if (!bi->page_dma &&
- (adapter->flags & IXGBE_FLAG_RX_PS_ENABLED)) {
+ (rx_ring->flags & IXGBE_RING_RX_PS_ENABLED)) {
if (!bi->page) {
bi->page = alloc_page(GFP_ATOMIC);
if (!bi->page) {
@@ -629,7 +629,7 @@ static void ixgbe_alloc_rx_buffers(struct ixgbe_adapter *adapter,
}
/* Refresh the desc even if buffer_addrs didn't change because
* each write-back erases this info. */
- if (adapter->flags & IXGBE_FLAG_RX_PS_ENABLED) {
+ if (rx_ring->flags & IXGBE_RING_RX_PS_ENABLED) {
rx_desc->read.pkt_addr = cpu_to_le64(bi->page_dma);
rx_desc->read.hdr_addr = cpu_to_le64(bi->dma);
} else {
@@ -726,7 +726,7 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
break;
(*work_done)++;
- if (adapter->flags & IXGBE_FLAG_RX_PS_ENABLED) {
+ if (rx_ring->flags & IXGBE_RING_RX_PS_ENABLED) {
hdr_info = le16_to_cpu(ixgbe_get_hdr_info(rx_desc));
len = (hdr_info & IXGBE_RXDADV_HDRBUFLEN_MASK) >>
IXGBE_RXDADV_HDRBUFLEN_SHIFT;
@@ -798,7 +798,7 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
rx_ring->stats.packets++;
rx_ring->stats.bytes += skb->len;
} else {
- if (adapter->flags & IXGBE_FLAG_RX_PS_ENABLED) {
+ if (rx_ring->flags & IXGBE_RING_RX_PS_ENABLED) {
rx_buffer_info->skb = next_buffer->skb;
rx_buffer_info->dma = next_buffer->dma;
next_buffer->skb = skb;
@@ -1919,7 +1919,7 @@ static void ixgbe_configure_srrctl(struct ixgbe_adapter *adapter,
srrctl |= (IXGBE_RX_HDR_SIZE << IXGBE_SRRCTL_BSIZEHDRSIZE_SHIFT) &
IXGBE_SRRCTL_BSIZEHDR_MASK;
- if (adapter->flags & IXGBE_FLAG_RX_PS_ENABLED) {
+ if (rx_ring->flags & IXGBE_RING_RX_PS_ENABLED) {
#if (PAGE_SIZE / 2) > IXGBE_MAX_RXBUFFER
srrctl |= IXGBE_MAX_RXBUFFER >> IXGBE_SRRCTL_BSIZEPKT_SHIFT;
#else
@@ -1992,11 +1992,6 @@ static void ixgbe_configure_rx(struct ixgbe_adapter *adapter)
/* Decide whether to use packet split mode or not */
adapter->flags |= IXGBE_FLAG_RX_PS_ENABLED;
-#ifdef IXGBE_FCOE
- if (adapter->flags & IXGBE_FLAG_FCOE_ENABLED)
- adapter->flags &= ~IXGBE_FLAG_RX_PS_ENABLED;
-#endif /* IXGBE_FCOE */
-
/* Set the RX buffer length according to the mode */
if (adapter->flags & IXGBE_FLAG_RX_PS_ENABLED) {
rx_buf_len = IXGBE_RX_HDR_SIZE;
@@ -2056,14 +2051,19 @@ static void ixgbe_configure_rx(struct ixgbe_adapter *adapter)
rx_ring->tail = IXGBE_RDT(j);
rx_ring->rx_buf_len = rx_buf_len;
+ if (adapter->flags & IXGBE_FLAG_RX_PS_ENABLED)
+ rx_ring->flags |= IXGBE_RING_RX_PS_ENABLED;
+
#ifdef IXGBE_FCOE
if (adapter->flags & IXGBE_FLAG_FCOE_ENABLED) {
struct ixgbe_ring_feature *f;
f = &adapter->ring_feature[RING_F_FCOE];
- if ((rx_buf_len < IXGBE_FCOE_JUMBO_FRAME_SIZE) &&
- (i >= f->mask) && (i < f->mask + f->indices))
- rx_ring->rx_buf_len =
- IXGBE_FCOE_JUMBO_FRAME_SIZE;
+ if ((i >= f->mask) && (i < f->mask + f->indices)) {
+ rx_ring->flags &= ~IXGBE_RING_RX_PS_ENABLED;
+ if (rx_buf_len < IXGBE_FCOE_JUMBO_FRAME_SIZE)
+ rx_ring->rx_buf_len =
+ IXGBE_FCOE_JUMBO_FRAME_SIZE;
+ }
}
#endif /* IXGBE_FCOE */
@@ -2143,7 +2143,8 @@ static void ixgbe_configure_rx(struct ixgbe_adapter *adapter)
if (adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED) {
/* Enable 82599 HW-RSC */
for (i = 0; i < adapter->num_rx_queues; i++) {
- j = adapter->rx_ring[i].reg_idx;
+ rx_ring = &adapter->rx_ring[i];
+ j = rx_ring->reg_idx;
rscctrl = IXGBE_READ_REG(hw, IXGBE_RSCCTL(j));
rscctrl |= IXGBE_RSCCTL_RSCEN;
/*
@@ -2151,7 +2152,7 @@ static void ixgbe_configure_rx(struct ixgbe_adapter *adapter)
* total size of max desc * buf_len is not greater
* than 65535
*/
- if (adapter->flags & IXGBE_FLAG_RX_PS_ENABLED) {
+ if (rx_ring->flags & IXGBE_RING_RX_PS_ENABLED) {
#if (MAX_SKB_FRAGS > 16)
rscctrl |= IXGBE_RSCCTL_MAXDESC_16;
#elif (MAX_SKB_FRAGS > 8)
^ permalink raw reply related
* [net-2.6 PATCH 1/2] ixgbe: Pass rx_ring directly in ixgbe_configure_srrctl()
From: Jeff Kirsher @ 2009-08-06 23:05 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, Yi Zou, Peter P Waskiewicz Jr, Jeff Kirsher
From: Yi Zou <yi.zou@intel.com>
Instead of passing the register index of the corresponding rx_ring and find
the way back to get to corresponding rx_ring in ixgbe_configure_srrctl(),
simplify the function ixgbe_configure_srrctl() by passing the rx_ring into
it. Then the register index for that rx_ring is already available from
rx_ring->reg_idx.
Signed-off-by: Yi Zou <yi.zou@intel.com>
Acked-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ixgbe/ixgbe_main.c | 55 +++++++++++-----------------------------
1 files changed, 15 insertions(+), 40 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index 110c65a..e361b7f 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -1898,46 +1898,19 @@ static void ixgbe_configure_tx(struct ixgbe_adapter *adapter)
#define IXGBE_SRRCTL_BSIZEHDRSIZE_SHIFT 2
-static void ixgbe_configure_srrctl(struct ixgbe_adapter *adapter, int index)
+static void ixgbe_configure_srrctl(struct ixgbe_adapter *adapter,
+ struct ixgbe_ring *rx_ring)
{
- struct ixgbe_ring *rx_ring;
u32 srrctl;
- int queue0 = 0;
- unsigned long mask;
+ int index;
struct ixgbe_ring_feature *feature = adapter->ring_feature;
- if (adapter->hw.mac.type == ixgbe_mac_82599EB) {
- if (adapter->flags & IXGBE_FLAG_DCB_ENABLED) {
- int dcb_i = feature[RING_F_DCB].indices;
- if (dcb_i == 8)
- queue0 = index >> 4;
- else if (dcb_i == 4)
- queue0 = index >> 5;
- else
- dev_err(&adapter->pdev->dev, "Invalid DCB "
- "configuration\n");
-#ifdef IXGBE_FCOE
- if (adapter->flags & IXGBE_FLAG_FCOE_ENABLED) {
- struct ixgbe_ring_feature *f;
-
- rx_ring = &adapter->rx_ring[queue0];
- f = &adapter->ring_feature[RING_F_FCOE];
- if ((queue0 == 0) && (index > rx_ring->reg_idx))
- queue0 = f->mask + index -
- rx_ring->reg_idx - 1;
- }
-#endif /* IXGBE_FCOE */
- } else {
- queue0 = index;
- }
- } else {
+ index = rx_ring->reg_idx;
+ if (adapter->hw.mac.type == ixgbe_mac_82598EB) {
+ unsigned long mask;
mask = (unsigned long) feature[RING_F_RSS].mask;
- queue0 = index & mask;
index = index & mask;
}
-
- rx_ring = &adapter->rx_ring[queue0];
-
srrctl = IXGBE_READ_REG(&adapter->hw, IXGBE_SRRCTL(index));
srrctl &= ~IXGBE_SRRCTL_BSIZEHDR_MASK;
@@ -2002,6 +1975,7 @@ static void ixgbe_configure_rx(struct ixgbe_adapter *adapter)
{
u64 rdba;
struct ixgbe_hw *hw = &adapter->hw;
+ struct ixgbe_ring *rx_ring;
struct net_device *netdev = adapter->netdev;
int max_frame = netdev->mtu + ETH_HLEN + ETH_FCS_LEN;
int i, j;
@@ -2070,16 +2044,17 @@ static void ixgbe_configure_rx(struct ixgbe_adapter *adapter)
* the Base and Length of the Rx Descriptor Ring
*/
for (i = 0; i < adapter->num_rx_queues; i++) {
- rdba = adapter->rx_ring[i].dma;
- j = adapter->rx_ring[i].reg_idx;
+ rx_ring = &adapter->rx_ring[i];
+ rdba = rx_ring->dma;
+ j = rx_ring->reg_idx;
IXGBE_WRITE_REG(hw, IXGBE_RDBAL(j), (rdba & DMA_BIT_MASK(32)));
IXGBE_WRITE_REG(hw, IXGBE_RDBAH(j), (rdba >> 32));
IXGBE_WRITE_REG(hw, IXGBE_RDLEN(j), rdlen);
IXGBE_WRITE_REG(hw, IXGBE_RDH(j), 0);
IXGBE_WRITE_REG(hw, IXGBE_RDT(j), 0);
- adapter->rx_ring[i].head = IXGBE_RDH(j);
- adapter->rx_ring[i].tail = IXGBE_RDT(j);
- adapter->rx_ring[i].rx_buf_len = rx_buf_len;
+ rx_ring->head = IXGBE_RDH(j);
+ rx_ring->tail = IXGBE_RDT(j);
+ rx_ring->rx_buf_len = rx_buf_len;
#ifdef IXGBE_FCOE
if (adapter->flags & IXGBE_FLAG_FCOE_ENABLED) {
@@ -2087,12 +2062,12 @@ static void ixgbe_configure_rx(struct ixgbe_adapter *adapter)
f = &adapter->ring_feature[RING_F_FCOE];
if ((rx_buf_len < IXGBE_FCOE_JUMBO_FRAME_SIZE) &&
(i >= f->mask) && (i < f->mask + f->indices))
- adapter->rx_ring[i].rx_buf_len =
+ rx_ring->rx_buf_len =
IXGBE_FCOE_JUMBO_FRAME_SIZE;
}
#endif /* IXGBE_FCOE */
- ixgbe_configure_srrctl(adapter, j);
+ ixgbe_configure_srrctl(adapter, rx_ring);
}
if (hw->mac.type == ixgbe_mac_82598EB) {
^ permalink raw reply related
* Re: [PATCH 4/7] vbus-proxy: add a pci-to-vbus bridge
From: Arnd Bergmann @ 2009-08-06 22:57 UTC (permalink / raw)
To: Gregory Haskins; +Cc: alacrityvm-devel, linux-kernel, netdev, Ira W. Snyder
In-Reply-To: <4A7B0D090200005A00051D2E@sinclair.provo.novell.com>
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. I was assuming that all of the setup code still lived in host user space.
What is the reason why it cannot? Do you want to use something other than qemu,
do you think this will impact performance, or something else?
> 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.
> 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.
> 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
> 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.
> 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.
> >> 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.
> >, 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.
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.
> 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.
> > 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, 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.
> >and you go and enumerate the devices on the bridge, creating a vbus_device for each
> > one as you go.
>
> Thats exactly what it does.
>
> > Then you just need to match the vbus drivers with the
> > devices by some string or vendor/device ID tuple.
> >
>
> Yep, thats right too. Then, when the driver gets a ->probe(), it does an dev->open() to check various state:
>
> a) can the device be opened? if it has an max-open policy (most will have a max-open = 1 policy) and something else already has the device open, it will fail (this will not be common).
> b) is the driver ABI revision compatible with the device ABI revision? This is like checking the pci config-space revision number.
>
> For an example, see drivers/net/vbus-enet.c, line 764:
>
> http://git.kernel.org/?p=linux/kernel/git/ghaskins/alacrityvm/linux-2.6.git;a=blob;f=drivers/net/vbus-enet.c;h=7220f43723adc5b0bece1bc37974fae1b034cd9e;hb=b3b2339efbd4e754b1c85f8bc8f85f21a1a1f509#l764
>
> Its simple a check to see if the driver and device are compatible, and therefore the probe should succeed. Nothing more. I think what I have done is similar to how most buses (like PCI) work today (ala revision number checks with a config-cycle).
ok.
> 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.
Arnd <><
^ 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