* [Fwd: Re: [PATCH] Fix userland build failure in linux/if_tunnel.h]
From: Sergei Poselenov @ 2009-08-09 9:41 UTC (permalink / raw)
To: netdev; +Cc: wd@denx.de >> Wolfgang Denk
Hello all,
As suggested, reposting the message in this list. Could someone please
comment?
Original message here:
http://lkml.org/lkml/2009/6/18/223
Thanks in advance,
Sergei
-------- Original Message --------
Subject: Re: [PATCH] Fix userland build failure in linux/if_tunnel.h
Date: Fri, 7 Aug 2009 15:31:06 +0400
From: Sergei Poselenov <sposelenov@emcraft.com>
Organisation: Emcraft Systems
To: linux-kernel@vger.kernel.org
CC: wd@denx.de
References: <20090618183738.662f7ef1@emcraft.com>
Hello all,
Unfortunately, received no replies, but the problem still persists.
Probably, I was unclear describing the issue. It is not the busybox
build problem - any application including exported linux/if_tunnel.h
could fail the linkage because of unresolved __cpu_to_be16().
Here is a reduced testcase program:
#include <netinet/ip.h>
#include <net/if.h>
#include <net/if_arp.h>
#include <asm/types.h>
#include <linux/if_tunnel.h>
void foo(void);
void foo(void)
{
int a = GRE_CSUM;
}
How to reproduce:
bash> arm-linux-gcc -c t.c
bash> nm -n t.o
U __cpu_to_be16
00000000 t $a
00000000 T foo
As you see, __cpu_to_be16 is undefined, so linking will fail.
Adding <asm/byteorder.h> to if_tunnel.h, as I suggested, fixes the
problem.
Thanks for any comments,
Sergei
On Thu, 18 Jun 2009 18:37:38 +0400
Sergei Poselenov <sposelenov@emcraft.com> wrote:
> Hello all,
>
> The last commit to linux/if_tunnel.h broke the busybox
> application build.
> When linking the final image linker complains:
> networking/libiproute/lib.a(iptunnel.o): In function `parse_args':
> /work/psl/e/arm-aa/target_rpms/busybox/BUILD/busybox-1.7.1/networking/libiproute/iptunnel.c:189:
> undefined reference to
> `__cpu_to_be16' /work/psl/e/arm-aa/target_rpms/busybox/BUILD/busybox-1.7.1/networking/libiproute/iptunnel.c:190:
> undefined reference to
> `__cpu_to_be16' /work/psl/e/arm-aa/target_rpms/busybox/BUILD/busybox-1.7.1/networking/libiproute/iptunnel.c:202:
> undefined reference to `__cpu_to_be16'
> ...
>
> Please review the following patch, which fixes the build error.
>
> Subject: [PATCH] The busybox application build failed in linking
> complaining to unresolved __cpu_to_be16 (ARM target). This patch
> fixes this.
>
> Signed-off-by: Sergei Poselenov <sposelenov@emcraft.com>
> ---
> include/linux/if_tunnel.h | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/if_tunnel.h b/include/linux/if_tunnel.h
> index 5a9aae4..137130b 100644
> --- a/include/linux/if_tunnel.h
> +++ b/include/linux/if_tunnel.h
> @@ -2,6 +2,7 @@
> #define _IF_TUNNEL_H_
>
> #include <linux/types.h>
> +#include <asm/byteorder.h>
>
> #ifdef __KERNEL__
> #include <linux/ip.h>
^ permalink raw reply
* [PATCH] mlx4_en: Fix read buffer overflow in mlx4_en_complete_rx_desc()
From: Roel Kluin @ 2009-08-09 9:54 UTC (permalink / raw)
To: netdev, Andrew Morton, yevgenyp, davem
If the length is less or equal to frag_prefix_size in the first iteration
we write skb_frags_rx[-1] and read from priv->frag_info[-1]
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
diff --git a/drivers/net/mlx4/en_rx.c b/drivers/net/mlx4/en_rx.c
index 91bdfdf..3ac0404 100644
--- a/drivers/net/mlx4/en_rx.c
+++ b/drivers/net/mlx4/en_rx.c
@@ -506,8 +506,9 @@ static int mlx4_en_complete_rx_desc(struct mlx4_en_priv *priv,
PCI_DMA_FROMDEVICE);
}
/* Adjust size of last fragment to match actual length */
- skb_frags_rx[nr - 1].size = length -
- priv->frag_info[nr - 1].frag_prefix_size;
+ if (nr > 0)
+ skb_frags_rx[nr - 1].size = length -
+ priv->frag_info[nr - 1].frag_prefix_size;
return nr;
fail:
^ permalink raw reply related
* Re: [PATCH] macvlan: add tap device backend
From: Michael S. Tsirkin @ 2009-08-09 8:02 UTC (permalink / raw)
To: Arnd Bergmann
Cc: netdev, Patrick McHardy, Stephen Hemminger, David S. Miller,
Herbert Xu, Or Gerlitz, Fischer, Anna, bridge, linux-kernel,
Edge Virtual Bridging
In-Reply-To: <1249595428-21594-1-git-send-email-arnd@arndb.de>
On Thu, Aug 06, 2009 at 09:50:28PM +0000, Arnd Bergmann wrote:
> This driver
> -----------
> While the other approaches should work as well, doing it using a tap
> interface should give additional benefits:
>
> * We can keep using the optimizations for jumbo frames that we have put
> into the tun/tap driver.
>
> * No need for root permissions that packet sockets need, just use 'ip
> link add link type macvtap' to create a new device and give it the right
> permissions using udev (using one tap per macvlan netdev).
>
> * support for multiqueue network adapters by opening the tap device
> multiple times, using one file descriptor per guest CPU/network
> queue/interrupt (if the adapter supports multiple queues on a single
> MAC address).
>
> * support for zero-copy receive/transmit using async I/O on the tap device
> (if the adapter supports per MAC rx queues).
>
> * The same framework in macvlan can be used to add a third backend
> into a future kernel based virtio-net implementation.
Could you split the patches up, to make this last easier?
patch 1 - export framework
patch 2 - code using it
> This version of the driver does not support any of those features,
> but they all appear possible to add ;).
> The driver is currently called 'macvtap', but I'd be more than happy
> to change that if anyone could suggest a better name. The code is
> still in an early stage and I wish I had found more time to polish
> it, but at this time, I'd first like to know if people agree with the
> basic concept at all.
>
> Cc: Patrick McHardy <kaber@trash.net>
> Cc: Stephen Hemminger <shemminger@linux-foundation.org>
> Cc: David S. Miller" <davem@davemloft.net>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: Or Gerlitz <ogerlitz@voltaire.com>
> Cc: "Fischer, Anna" <anna.fischer@hp.com>
> Cc: netdev@vger.kernel.org
> Cc: bridge@lists.linux-foundation.org
> Cc: linux-kernel@vger.kernel.org
> Cc: Edge Virtual Bridging <evb@yahoogroups.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> ---
>
> The evb mailing list eats Cc headers, please make sure to keep everybody
> in your Cc list when replying there.
> ---
> drivers/net/Kconfig | 12 ++
> drivers/net/Makefile | 1 +
> drivers/net/macvlan.c | 39 +++-----
> drivers/net/macvlan.h | 37 +++++++
> drivers/net/macvtap.c | 276 +++++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 341 insertions(+), 24 deletions(-)
> create mode 100644 drivers/net/macvlan.h
> create mode 100644 drivers/net/macvtap.c
>
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index 5f6509a..0b9ac6a 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -90,6 +90,18 @@ config MACVLAN
> To compile this driver as a module, choose M here: the module
> will be called macvlan.
>
> +config MACVTAP
> + tristate "MAC-VLAN based tap driver (EXPERIMENTAL)"
> + depends on MACVLAN
> + help
> + This adds a specialized tap character device driver that is based
> + on the MAC-VLAN network interface, called macvtap. A macvtap device
> + can be added in the same way as a macvlan device, using 'type
> + macvlan', and then be accessed through the tap user space interface.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called macvtap.
> +
> config EQUALIZER
> tristate "EQL (serial line load balancing) support"
> ---help---
> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> index ead8cab..8a2d2d7 100644
> --- a/drivers/net/Makefile
> +++ b/drivers/net/Makefile
> @@ -162,6 +162,7 @@ obj-$(CONFIG_XEN_NETDEV_FRONTEND) += xen-netfront.o
> obj-$(CONFIG_DUMMY) += dummy.o
> obj-$(CONFIG_IFB) += ifb.o
> obj-$(CONFIG_MACVLAN) += macvlan.o
> +obj-$(CONFIG_MACVTAP) += macvtap.o
> obj-$(CONFIG_DE600) += de600.o
> obj-$(CONFIG_DE620) += de620.o
> obj-$(CONFIG_LANCE) += lance.o
> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> index 99eed9f..9f7dc6a 100644
> --- a/drivers/net/macvlan.c
> +++ b/drivers/net/macvlan.c
> @@ -30,22 +30,7 @@
> #include <linux/if_macvlan.h>
> #include <net/rtnetlink.h>
>
> -#define MACVLAN_HASH_SIZE (1 << BITS_PER_BYTE)
> -
> -struct macvlan_port {
> - struct net_device *dev;
> - struct hlist_head vlan_hash[MACVLAN_HASH_SIZE];
> - struct list_head vlans;
> -};
> -
> -struct macvlan_dev {
> - struct net_device *dev;
> - struct list_head list;
> - struct hlist_node hlist;
> - struct macvlan_port *port;
> - struct net_device *lowerdev;
> -};
> -
> +#include "macvlan.h"
>
> static struct macvlan_dev *macvlan_hash_lookup(const struct macvlan_port *port,
> const unsigned char *addr)
> @@ -135,7 +120,7 @@ static void macvlan_broadcast(struct sk_buff *skb,
> else
> nskb->pkt_type = PACKET_MULTICAST;
>
> - netif_rx(nskb);
> + vlan->receive(nskb);
> }
> }
> }
> @@ -180,11 +165,11 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
> skb->dev = dev;
> skb->pkt_type = PACKET_HOST;
>
> - netif_rx(skb);
> + vlan->receive(skb);
> return NULL;
> }
>
> -static int macvlan_start_xmit(struct sk_buff *skb, struct net_device *dev)
> +int macvlan_start_xmit(struct sk_buff *skb, struct net_device *dev)
> {
> const struct macvlan_dev *vlan = netdev_priv(dev);
> unsigned int len = skb->len;
> @@ -202,6 +187,7 @@ static int macvlan_start_xmit(struct sk_buff *skb, struct net_device *dev)
> }
> return NETDEV_TX_OK;
> }
> +EXPORT_SYMBOL_GPL(macvlan_start_xmit);
>
> static int macvlan_hard_header(struct sk_buff *skb, struct net_device *dev,
> unsigned short type, const void *daddr,
> @@ -412,7 +398,7 @@ static const struct net_device_ops macvlan_netdev_ops = {
> .ndo_validate_addr = eth_validate_addr,
> };
>
> -static void macvlan_setup(struct net_device *dev)
> +void macvlan_setup(struct net_device *dev)
> {
> ether_setup(dev);
>
> @@ -423,6 +409,7 @@ static void macvlan_setup(struct net_device *dev)
> dev->ethtool_ops = &macvlan_ethtool_ops;
> dev->tx_queue_len = 0;
> }
> +EXPORT_SYMBOL_GPL(macvlan_setup);
>
> static int macvlan_port_create(struct net_device *dev)
> {
> @@ -472,7 +459,7 @@ static void macvlan_transfer_operstate(struct net_device *dev)
> }
> }
>
> -static int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
> +int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
> {
> if (tb[IFLA_ADDRESS]) {
> if (nla_len(tb[IFLA_ADDRESS]) != ETH_ALEN)
> @@ -482,9 +469,10 @@ static int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
> }
> return 0;
> }
> +EXPORT_SYMBOL_GPL(macvlan_validate);
>
> -static int macvlan_newlink(struct net_device *dev,
> - struct nlattr *tb[], struct nlattr *data[])
> +int macvlan_newlink(struct net_device *dev,
> + struct nlattr *tb[], struct nlattr *data[])
> {
> struct macvlan_dev *vlan = netdev_priv(dev);
> struct macvlan_port *port;
> @@ -524,6 +512,7 @@ static int macvlan_newlink(struct net_device *dev,
> vlan->lowerdev = lowerdev;
> vlan->dev = dev;
> vlan->port = port;
> + vlan->receive = netif_rx;
>
> err = register_netdevice(dev);
> if (err < 0)
> @@ -533,8 +522,9 @@ static int macvlan_newlink(struct net_device *dev,
> macvlan_transfer_operstate(dev);
> return 0;
> }
> +EXPORT_SYMBOL_GPL(macvlan_newlink);
>
> -static void macvlan_dellink(struct net_device *dev)
> +void macvlan_dellink(struct net_device *dev)
> {
> struct macvlan_dev *vlan = netdev_priv(dev);
> struct macvlan_port *port = vlan->port;
> @@ -545,6 +535,7 @@ static void macvlan_dellink(struct net_device *dev)
> if (list_empty(&port->vlans))
> macvlan_port_destroy(port->dev);
> }
> +EXPORT_SYMBOL_GPL(macvlan_dellink);
>
> static struct rtnl_link_ops macvlan_link_ops __read_mostly = {
> .kind = "macvlan",
> diff --git a/drivers/net/macvlan.h b/drivers/net/macvlan.h
> new file mode 100644
> index 0000000..3f3c6c3
> --- /dev/null
> +++ b/drivers/net/macvlan.h
> @@ -0,0 +1,37 @@
> +#ifndef _MACVLAN_H
> +#define _MACVLAN_H
> +
> +#include <linux/netdevice.h>
> +#include <linux/netlink.h>
> +#include <linux/list.h>
> +
> +#define MACVLAN_HASH_SIZE (1 << BITS_PER_BYTE)
> +
> +struct macvlan_port {
> + struct net_device *dev;
> + struct hlist_head vlan_hash[MACVLAN_HASH_SIZE];
> + struct list_head vlans;
> +};
> +
> +struct macvlan_dev {
> + struct net_device *dev;
> + struct list_head list;
> + struct hlist_node hlist;
> + struct macvlan_port *port;
> + struct net_device *lowerdev;
> +
> + int (*receive)(struct sk_buff *skb);
> +};
> +
> +extern int macvlan_start_xmit(struct sk_buff *skb, struct net_device *dev);
> +
> +extern void macvlan_setup(struct net_device *dev);
> +
> +extern int macvlan_validate(struct nlattr *tb[], struct nlattr *data[]);
> +
> +extern int macvlan_newlink(struct net_device *dev,
> + struct nlattr *tb[], struct nlattr *data[]);
> +
> +extern void macvlan_dellink(struct net_device *dev);
> +
> +#endif /* _MACVLAN_H */
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> new file mode 100644
> index 0000000..d99bfc0
> --- /dev/null
> +++ b/drivers/net/macvtap.c
> @@ -0,0 +1,276 @@
> +#include <linux/etherdevice.h>
> +#include <linux/nsproxy.h>
> +#include <linux/module.h>
> +#include <linux/skbuff.h>
> +#include <linux/cache.h>
> +#include <linux/sched.h>
> +#include <linux/types.h>
> +#include <linux/init.h>
> +#include <linux/wait.h>
> +#include <linux/cdev.h>
> +#include <linux/fs.h>
> +
> +#include <net/net_namespace.h>
> +#include <net/rtnetlink.h>
> +
> +#include "macvlan.h"
> +
> +struct macvtap_dev {
> + struct macvlan_dev m;
> + struct cdev cdev;
> + struct sk_buff_head readq;
> + wait_queue_head_t wait;
> +};
> +
> +/*
> + * Minor number matches netdev->ifindex, so need a large value
> + */
> +static int macvtap_major;
> +#define MACVTAP_NUM_DEVS 65536
> +
> +static int macvtap_receive(struct sk_buff *skb)
> +{
> + struct macvtap_dev *vtap = netdev_priv(skb->dev);
> +
> + skb_queue_tail(&vtap->readq, skb);
> + wake_up(&vtap->wait);
> + return 0;
> +}
> +
> +static int macvtap_open(struct inode *inode, struct file *file)
> +{
> + struct net *net = current->nsproxy->net_ns;
> + int ifindex = iminor(inode);
> + struct net_device *dev = dev_get_by_index(net, ifindex);
> + int err;
> +
> + err = -ENODEV;
> + if (!dev)
> + goto out1;
> +
> + file->private_data = netdev_priv(dev);
> + err = 0;
> +out1:
> + return err;
> +}
> +
> +static int macvtap_release(struct inode *inode, struct file *file)
> +{
> + struct macvtap_dev *vtap = file->private_data;
> +
> + if (!vtap)
> + return 0;
> +
> + dev_put(vtap->m.dev);
> + return 0;
> +}
> +
> +/* Get packet from user space buffer */
> +static ssize_t macvtap_get_user(struct macvtap_dev *vtap,
> + const struct iovec *iv, size_t count,
> + int noblock)
> +{
> + struct sk_buff *skb;
> + size_t len = count;
> +
> + if (unlikely(len < ETH_HLEN))
> + return -EINVAL;
> +
> + skb = alloc_skb(NET_IP_ALIGN + len, GFP_KERNEL);
> +
> + if (!skb) {
> + vtap->m.dev->stats.rx_dropped++;
> + return -ENOMEM;
> + }
> +
> + skb_reserve(skb, NET_IP_ALIGN);
> + skb_put(skb, count);
> +
> + if (skb_copy_datagram_from_iovec(skb, 0, iv, 0, len)) {
> + vtap->m.dev->stats.rx_dropped++;
> + kfree_skb(skb);
> + return -EFAULT;
> + }
> +
> + skb_set_network_header(skb, ETH_HLEN);
> + skb->dev = vtap->m.lowerdev;
> +
> + macvlan_start_xmit(skb, vtap->m.dev);
> +
> + return count;
> +}
With tap, we discovered that not limiting the number of outstanding
skbs hurts UDP performance. And the solution was to limit
the number of outstanding packets - with hacks to work around
the fact that userspace .
> +
> +static ssize_t macvtap_aio_write(struct kiocb *iocb, const struct iovec *iv,
> + unsigned long count, loff_t pos)
> +{
> + struct file *file = iocb->ki_filp;
> + ssize_t result;
> + struct macvtap_dev *vtap = file->private_data;
> +
> + result = macvtap_get_user(vtap, iv, iov_length(iv, count),
> + file->f_flags & O_NONBLOCK);
> +
> + return result;
> +}
> +
> +/* Put packet to the user space buffer */
> +static ssize_t macvtap_put_user(struct macvtap_dev *vtap,
> + struct sk_buff *skb,
> + struct iovec *iv, int len)
> +{
> + int ret;
> +
> + skb_push(skb, ETH_HLEN);
> + len = min_t(int, skb->len, len);
> +
> + ret = skb_copy_datagram_iovec(skb, 0, iv, len);
> +
> + vtap->m.dev->stats.rx_packets++;
> + vtap->m.dev->stats.rx_bytes += len;
where does atomicity guarantee for these counters come from?
> +
> + return ret ? ret : len;
> +}
> +
> +static ssize_t macvtap_aio_read(struct kiocb *iocb, const struct iovec *iv,
> + unsigned long count, loff_t pos)
> +{
> + struct file *file = iocb->ki_filp;
> + struct macvtap_dev *vtap = file->private_data;
> + DECLARE_WAITQUEUE(wait, current);
> + struct sk_buff *skb;
> + ssize_t len, ret = 0;
> +
> + if (!vtap)
> + return -EBADFD;
> +
> + len = iov_length(iv, count);
> + if (len < 0) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + add_wait_queue(&vtap->wait, &wait);
> + while (len) {
> + current->state = TASK_INTERRUPTIBLE;
> +
> + /* Read frames from the queue */
> + if (!(skb=skb_dequeue(&vtap->readq))) {
> + if (file->f_flags & O_NONBLOCK) {
> + ret = -EAGAIN;
> + break;
> + }
> + if (signal_pending(current)) {
> + ret = -ERESTARTSYS;
> + break;
> + }
> + /* Nothing to read, let's sleep */
> + schedule();
> + continue;
> + }
> + ret = macvtap_put_user(vtap, skb, (struct iovec *) iv, len);
Don't cast away the constness. Instead, fix macvtap_put_user
to used skb_copy_datagram_const_iovec which does not modify the iovec.
> + kfree_skb(skb);
> + break;
> + }
> +
> + current->state = TASK_RUNNING;
> + remove_wait_queue(&vtap->wait, &wait);
> +
> +out:
> + return ret;
> +}
> +
> +struct file_operations macvtap_fops = {
> + .owner = THIS_MODULE,
> + .open = macvtap_open,
> + .release = macvtap_release,
> + .aio_read = macvtap_aio_read,
> + .aio_write = macvtap_aio_write,
> + .llseek = no_llseek,
> +};
> +
> +static int macvtap_newlink(struct net_device *dev,
> + struct nlattr *tb[], struct nlattr *data[])
> +{
> + struct macvtap_dev *vtap = netdev_priv(dev);
> + int err;
> +
> + err = macvlan_newlink(dev, tb, data);
> + if (err)
> + goto out1;
> +
> + cdev_init(&vtap->cdev, &macvtap_fops);
> + vtap->cdev.owner = THIS_MODULE;
> + err = cdev_add(&vtap->cdev, MKDEV(MAJOR(macvtap_major), dev->ifindex), 1);
> +
> + if (err)
> + goto out2;
> +
> + /*
> + * TODO: add class dev so device node gets created automatically
> + * by udev.
> + */
> + pr_debug("%s:%d: added cdev %d:%d for dev %s\n",
> + __func__, __LINE__, MAJOR(macvtap_major),
> + dev->ifindex, dev->name);
> +
> + skb_queue_head_init(&vtap->readq);
> + init_waitqueue_head(&vtap->wait);
> + vtap->m.receive = macvtap_receive;
> +
> + return 0;
> +
> +out2:
> + macvlan_dellink(dev);
> +out1:
> + return err;
> +}
> +
> +static void macvtap_dellink(struct net_device *dev)
> +{
> + struct macvtap_dev *vtap = netdev_priv(dev);
> + cdev_del(&vtap->cdev);
> + /* TODO: kill open file descriptors */
> + macvlan_dellink(dev);
> +}
> +
> +static struct rtnl_link_ops macvtap_link_ops __read_mostly = {
> + .kind = "macvtap",
> + .priv_size = sizeof(struct macvtap_dev),
> + .setup = macvlan_setup,
> + .validate = macvlan_validate,
> + .newlink = macvtap_newlink,
> + .dellink = macvtap_dellink,
> +};
> +
> +static int macvtap_init(void)
> +{
> + int err;
> +
> + err = alloc_chrdev_region(&macvtap_major, 0,
> + MACVTAP_NUM_DEVS, "macvtap");
> + if (err)
> + goto out1;
> +
> + err = rtnl_link_register(&macvtap_link_ops);
> + if (err)
> + goto out2;
> +
> + return 0;
> +
> +out2:
> + unregister_chrdev_region(macvtap_major, MACVTAP_NUM_DEVS);
> +out1:
> + return err;
> +}
> +module_init(macvtap_init);
> +
> +static void macvtap_exit(void)
> +{
> + rtnl_link_unregister(&macvtap_link_ops);
> + unregister_chrdev_region(macvtap_major, MACVTAP_NUM_DEVS);
> +}
> +module_exit(macvtap_exit);
> +
> +MODULE_ALIAS_RTNL_LINK("macvtap");
> +MODULE_AUTHOR("Arnd Bergmann <arnd@arndb.de>");
> +MODULE_LICENSE("GPL");
> --
> 1.6.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 0/7] AlacrityVM guest drivers Reply-To:
From: Avi Kivity @ 2009-08-09 7:48 UTC (permalink / raw)
To: Gregory Haskins
Cc: Arnd Bergmann, alacrityvm-devel, Michael S. Tsirkin, kvm,
linux-kernel, netdev
In-Reply-To: <4A7AD2D20200005A00051C83@sinclair.provo.novell.com>
On 08/06/2009 07:55 PM, Gregory Haskins wrote:
> Based on this, I will continue my efforts surrounding to use of vbus including its use to accelerate KVM for AlacrityVM. If I can find a way to do this in such a way that KVM upstream finds acceptable, I would be very happy and will work towards whatever that compromise might be. OTOH, if the KVM community is set against the concept of a generalized/shared backend, and thus wants to use some other approach that does not involve vbus, that is fine too. Choice is one of the great assets of open source, eh? :)
>
KVM upstream (me) doesn't have much say regarding vbus. I am not a
networking expert and I'm not the virtio or networking stack maintainer,
so I'm not qualified to accept or reject the code. What I am able to do
is make sure that kvm can efficiently work with any driver/device stack;
this is why ioeventfd/irqfd were merged.
I still think vbus is a duplication of effort; I understand vbus has
larger scope than virtio, but I still think these problems could have
been solved within the existing virtio stack.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply
* [PATCH 2/3] drivers/net: Correct use of request_region/request_mem_region
From: Julia Lawall @ 2009-08-09 7:43 UTC (permalink / raw)
To: netdev, linux-kernel, kernel-janitors
From: Julia Lawall <julia@diku.dk>
request_mem_region should be used with release_mem_region, not request_region.
The semantic patch that fixes this problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@r2@
expression start;
@@
request_mem_region(start,...)
@b2@
expression r2.start;
@@
request_region(start,...)
@depends on !b2@
expression r2.start;
expression E;
@@
- release_region
+ release_mem_region
(start,E)
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
drivers/net/jazzsonic.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff -u -p a/drivers/net/jazzsonic.c b/drivers/net/jazzsonic.c
--- a/drivers/net/jazzsonic.c
+++ b/drivers/net/jazzsonic.c
@@ -203,7 +203,7 @@ static int __devinit sonic_probe1(struct
return 0;
out:
- release_region(dev->base_addr, SONIC_MEM_SIZE);
+ release_mem_region(dev->base_addr, SONIC_MEM_SIZE);
return err;
}
@@ -247,7 +247,7 @@ static int __devinit jazz_sonic_probe(st
return 0;
out1:
- release_region(dev->base_addr, SONIC_MEM_SIZE);
+ release_mem_region(dev->base_addr, SONIC_MEM_SIZE);
out:
free_netdev(dev);
@@ -269,7 +269,7 @@ static int __devexit jazz_sonic_device_r
unregister_netdev(dev);
dma_free_coherent(lp->device, SIZEOF_SONIC_DESC * SONIC_BUS_SCALE(lp->dma_bitmode),
lp->descriptors, lp->descriptors_laddr);
- release_region (dev->base_addr, SONIC_MEM_SIZE);
+ release_mem_region(dev->base_addr, SONIC_MEM_SIZE);
free_netdev(dev);
return 0;
^ permalink raw reply
* [PATCH 3/3] korina: convert to net_device_ops
From: Phil Sutter @ 2009-08-09 0:06 UTC (permalink / raw)
To: netdev; +Cc: Andrew Morton, David S. Miller, florian, Roel Kluin
In-Reply-To: <1249776388-4626-3-git-send-email-n0-1@freewrt.org>
Signed-off-by: Phil Sutter <n0-1@freewrt.org>
---
drivers/net/korina.c | 23 ++++++++++++++---------
1 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/net/korina.c b/drivers/net/korina.c
index d256022..894f686 100644
--- a/drivers/net/korina.c
+++ b/drivers/net/korina.c
@@ -1088,6 +1088,18 @@ static int korina_close(struct net_device *dev)
return 0;
}
+static const struct net_device_ops korina_netdev_ops = {
+ .ndo_open = korina_open,
+ .ndo_stop = korina_close,
+ .ndo_start_xmit = korina_send_packet,
+ .ndo_set_multicast_list = korina_multicast_list,
+ .ndo_tx_timeout = korina_tx_timeout,
+ .ndo_do_ioctl = korina_ioctl,
+#ifdef CONFIG_NET_POLL_CONTROLLER
+ .ndo_poll_controller = korina_poll_controller,
+#endif
+};
+
static int korina_probe(struct platform_device *pdev)
{
struct korina_device *bif = platform_get_drvdata(pdev);
@@ -1156,17 +1168,10 @@ static int korina_probe(struct platform_device *pdev)
dev->irq = lp->rx_irq;
lp->dev = dev;
- dev->open = korina_open;
- dev->stop = korina_close;
- dev->hard_start_xmit = korina_send_packet;
- dev->set_multicast_list = &korina_multicast_list;
+ dev->netdev_ops = &korina_netdev_ops;
dev->ethtool_ops = &netdev_ethtool_ops;
- dev->tx_timeout = korina_tx_timeout;
dev->watchdog_timeo = TX_TIMEOUT;
- dev->do_ioctl = &korina_ioctl;
-#ifdef CONFIG_NET_POLL_CONTROLLER
- dev->poll_controller = korina_poll_controller;
-#endif
+
netif_napi_add(dev, &lp->napi, korina_poll, 64);
lp->phy_addr = (((lp->rx_irq == 0x2c? 1:0) << 8) | 0x05);
--
1.6.0.6
^ permalink raw reply related
* Re: [PATCH] korina: Read buffer overflow
From: Phil Sutter @ 2009-08-09 0:06 UTC (permalink / raw)
To: netdev; +Cc: Andrew Morton, David S. Miller, florian, Roel Kluin
In-Reply-To: <20090808164512.02C174CEAA@orbit.nwl.cc>
Hi,
Testing of my approach to solve the buffer overrun issue showed that
living with a subset of the requested receive descriptors is not as easy
as initially assumed: uppon allocation failure the number of descriptors
would have to be reduced to the next lower power of two (as also stated
in the corresponding define's comment), which I consider too much
overhead. A better solution would be to export the ring parameters via
ethtool. For now, let's go Roel's way aborting completely and cleaning
up in this case.
The following series fixes the incorrect printk formatting we already
discussed, implements the solution from above and makes the driver use
netdev_ops.
Greetings, Phil
^ permalink raw reply
* [PATCH 2/3] korina: add error-handling to korina_alloc_ring
From: Phil Sutter @ 2009-08-09 0:06 UTC (permalink / raw)
To: netdev; +Cc: Andrew Morton, David S. Miller, florian, Roel Kluin
In-Reply-To: <1249776388-4626-2-git-send-email-n0-1@freewrt.org>
This also avoids a potential buffer overflow in case the very first
receive descriptor fails to allocate, as an index of -1 would be used
afterwards. Kudos to Roel Kluin for pointing this out and providing an
initial patch.
Signed-off-by: Phil Sutter <n0-1@freewrt.org>
---
drivers/net/korina.c | 12 +++++++++---
1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/net/korina.c b/drivers/net/korina.c
index c09e2ce..d256022 100644
--- a/drivers/net/korina.c
+++ b/drivers/net/korina.c
@@ -741,7 +741,7 @@ static struct ethtool_ops netdev_ethtool_ops = {
.get_link = netdev_get_link,
};
-static void korina_alloc_ring(struct net_device *dev)
+static int korina_alloc_ring(struct net_device *dev)
{
struct korina_private *lp = netdev_priv(dev);
struct sk_buff *skb;
@@ -762,7 +762,7 @@ static void korina_alloc_ring(struct net_device *dev)
for (i = 0; i < KORINA_NUM_RDS; i++) {
skb = dev_alloc_skb(KORINA_RBSIZE + 2);
if (!skb)
- break;
+ return -ENOMEM;
skb_reserve(skb, 2);
skb->dev = dev;
lp->rx_skb[i] = skb;
@@ -782,6 +782,8 @@ static void korina_alloc_ring(struct net_device *dev)
lp->rx_chain_head = 0;
lp->rx_chain_tail = 0;
lp->rx_chain_status = desc_empty;
+
+ return 0;
}
static void korina_free_ring(struct net_device *dev)
@@ -824,7 +826,11 @@ static int korina_init(struct net_device *dev)
writel(ETH_INT_FC_EN, &lp->eth_regs->ethintfc);
/* Allocate rings */
- korina_alloc_ring(dev);
+ if (korina_alloc_ring(dev)) {
+ printk(KERN_ERR "%s: descriptor allocation failed\n", dev->name);
+ korina_free_ring(dev);
+ return -ENOMEM;
+ }
writel(0, &lp->rx_dma_regs->dmas);
/* Start Rx DMA */
--
1.6.0.6
^ permalink raw reply related
* [PATCH 1/3] korina: fix printk formatting, add final info line
From: Phil Sutter @ 2009-08-09 0:06 UTC (permalink / raw)
To: netdev; +Cc: Andrew Morton, David S. Miller, florian, Roel Kluin
In-Reply-To: <1249776388-4626-1-git-send-email-n0-1@freewrt.org>
The macro DRV_NAME contains "korina", the field dev->name points to the
actual interface name. So messages were formerly prefixed with
'korinaeth2:' (on my system).
Signed-off-by: Phil Sutter <n0-1@freewrt.org>
---
drivers/net/korina.c | 31 ++++++++++++++++---------------
1 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/drivers/net/korina.c b/drivers/net/korina.c
index a2701f5..c09e2ce 100644
--- a/drivers/net/korina.c
+++ b/drivers/net/korina.c
@@ -337,7 +337,7 @@ static irqreturn_t korina_rx_dma_interrupt(int irq, void *dev_id)
napi_schedule(&lp->napi);
if (dmas & DMA_STAT_ERR)
- printk(KERN_ERR DRV_NAME "%s: DMA error\n", dev->name);
+ printk(KERN_ERR "%s: DMA error\n", dev->name);
retval = IRQ_HANDLED;
} else
@@ -555,7 +555,7 @@ static void korina_tx(struct net_device *dev)
dev->stats.tx_dropped++;
/* Should never happen */
- printk(KERN_ERR DRV_NAME "%s: split tx ignored\n",
+ printk(KERN_ERR "%s: split tx ignored\n",
dev->name);
} else if (devcs & ETH_TX_TOK) {
dev->stats.tx_packets++;
@@ -641,7 +641,7 @@ korina_tx_dma_interrupt(int irq, void *dev_id)
dev->trans_start = jiffies;
}
if (dmas & DMA_STAT_ERR)
- printk(KERN_ERR DRV_NAME "%s: DMA error\n", dev->name);
+ printk(KERN_ERR "%s: DMA error\n", dev->name);
retval = IRQ_HANDLED;
} else
@@ -909,8 +909,7 @@ static int korina_restart(struct net_device *dev)
ret = korina_init(dev);
if (ret < 0) {
- printk(KERN_ERR DRV_NAME "%s: cannot restart device\n",
- dev->name);
+ printk(KERN_ERR "%s: cannot restart device\n", dev->name);
return ret;
}
korina_multicast_list(dev);
@@ -997,7 +996,7 @@ static int korina_open(struct net_device *dev)
/* Initialize */
ret = korina_init(dev);
if (ret < 0) {
- printk(KERN_ERR DRV_NAME "%s: cannot open device\n", dev->name);
+ printk(KERN_ERR "%s: cannot open device\n", dev->name);
goto out;
}
@@ -1007,14 +1006,14 @@ static int korina_open(struct net_device *dev)
ret = request_irq(lp->rx_irq, &korina_rx_dma_interrupt,
IRQF_DISABLED, "Korina ethernet Rx", dev);
if (ret < 0) {
- printk(KERN_ERR DRV_NAME "%s: unable to get Rx DMA IRQ %d\n",
+ printk(KERN_ERR "%s: unable to get Rx DMA IRQ %d\n",
dev->name, lp->rx_irq);
goto err_release;
}
ret = request_irq(lp->tx_irq, &korina_tx_dma_interrupt,
IRQF_DISABLED, "Korina ethernet Tx", dev);
if (ret < 0) {
- printk(KERN_ERR DRV_NAME "%s: unable to get Tx DMA IRQ %d\n",
+ printk(KERN_ERR "%s: unable to get Tx DMA IRQ %d\n",
dev->name, lp->tx_irq);
goto err_free_rx_irq;
}
@@ -1023,7 +1022,7 @@ static int korina_open(struct net_device *dev)
ret = request_irq(lp->ovr_irq, &korina_ovr_interrupt,
IRQF_DISABLED, "Ethernet Overflow", dev);
if (ret < 0) {
- printk(KERN_ERR DRV_NAME"%s: unable to get OVR IRQ %d\n",
+ printk(KERN_ERR "%s: unable to get OVR IRQ %d\n",
dev->name, lp->ovr_irq);
goto err_free_tx_irq;
}
@@ -1032,7 +1031,7 @@ static int korina_open(struct net_device *dev)
ret = request_irq(lp->und_irq, &korina_und_interrupt,
IRQF_DISABLED, "Ethernet Underflow", dev);
if (ret < 0) {
- printk(KERN_ERR DRV_NAME "%s: unable to get UND IRQ %d\n",
+ printk(KERN_ERR "%s: unable to get UND IRQ %d\n",
dev->name, lp->und_irq);
goto err_free_ovr_irq;
}
@@ -1111,7 +1110,7 @@ static int korina_probe(struct platform_device *pdev)
dev->base_addr = r->start;
lp->eth_regs = ioremap_nocache(r->start, r->end - r->start);
if (!lp->eth_regs) {
- printk(KERN_ERR DRV_NAME "cannot remap registers\n");
+ printk(KERN_ERR DRV_NAME ": cannot remap registers\n");
rc = -ENXIO;
goto probe_err_out;
}
@@ -1119,7 +1118,7 @@ static int korina_probe(struct platform_device *pdev)
r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "korina_dma_rx");
lp->rx_dma_regs = ioremap_nocache(r->start, r->end - r->start);
if (!lp->rx_dma_regs) {
- printk(KERN_ERR DRV_NAME "cannot remap Rx DMA registers\n");
+ printk(KERN_ERR DRV_NAME ": cannot remap Rx DMA registers\n");
rc = -ENXIO;
goto probe_err_dma_rx;
}
@@ -1127,14 +1126,14 @@ static int korina_probe(struct platform_device *pdev)
r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "korina_dma_tx");
lp->tx_dma_regs = ioremap_nocache(r->start, r->end - r->start);
if (!lp->tx_dma_regs) {
- printk(KERN_ERR DRV_NAME "cannot remap Tx DMA registers\n");
+ printk(KERN_ERR DRV_NAME ": cannot remap Tx DMA registers\n");
rc = -ENXIO;
goto probe_err_dma_tx;
}
lp->td_ring = kmalloc(TD_RING_SIZE + RD_RING_SIZE, GFP_KERNEL);
if (!lp->td_ring) {
- printk(KERN_ERR DRV_NAME "cannot allocate descriptors\n");
+ printk(KERN_ERR DRV_NAME ": cannot allocate descriptors\n");
rc = -ENXIO;
goto probe_err_td_ring;
}
@@ -1175,9 +1174,11 @@ static int korina_probe(struct platform_device *pdev)
rc = register_netdev(dev);
if (rc < 0) {
printk(KERN_ERR DRV_NAME
- ": cannot register net device %d\n", rc);
+ ": cannot register net device: %d\n", rc);
goto probe_err_register;
}
+ printk(KERN_INFO "%s: " DRV_NAME "-" DRV_VERSION " " DRV_RELDATE "\n",
+ dev->name);
out:
return rc;
--
1.6.0.6
^ permalink raw reply related
* Re: [PATCH 3/3] net: Add ftracer to help optimize process scheduling based on incomming frame allocations (v2)
From: Neil Horman @ 2009-08-08 23:15 UTC (permalink / raw)
To: netdev; +Cc: davem, rostedt
In-Reply-To: <20090808231322.GA24414@localhost.localdomain>
skb allocation / consumption correlator
Add ftracer module to kernel to print out a list that correlates a process id,
an skb it read, and the numa nodes on wich the process was running when it was
read along with the numa node the skbuff was allocated on.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index 844164d..ee5e5b1 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_BLK_DEV_IO_TRACE) += blktrace.o
ifeq ($(CONFIG_BLOCK),y)
obj-$(CONFIG_EVENT_TRACING) += blktrace.o
endif
+obj-$(CONFIG_SKB_SOURCES_TRACER) += trace_skb_sources.o
obj-$(CONFIG_EVENT_TRACING) += trace_events.o
obj-$(CONFIG_EVENT_TRACING) += trace_export.o
obj-$(CONFIG_FTRACE_SYSCALLS) += trace_syscalls.o
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 3548ae5..8c1d458 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -11,6 +11,7 @@
#include <trace/boot.h>
#include <linux/kmemtrace.h>
#include <trace/power.h>
+#include <trace/events/skb.h>
#include <linux/trace_seq.h>
#include <linux/ftrace_event.h>
@@ -40,6 +41,7 @@ enum trace_type {
TRACE_KMEM_FREE,
TRACE_POWER,
TRACE_BLK,
+ TRACE_SKB_SOURCE,
__TRACE_LAST_TYPE,
};
@@ -171,6 +173,21 @@ struct trace_power {
struct power_trace state_data;
};
+struct skb_record {
+ pid_t pid; /* pid of the copying process */
+ int anid; /* node where skb was allocated */
+ int cnid; /* node to which skb was copied in userspace */
+ char ifname[IFNAMSIZ]; /* Name of the receiving interface */
+ int rx_queue; /* The rx queue the skb was received on */
+ int ccpu; /* Cpu the application got this frame from */
+ int len; /* length of the data copied */
+};
+
+struct trace_skb_event {
+ struct trace_entry ent;
+ struct skb_record event_data;
+};
+
enum kmemtrace_type_id {
KMEMTRACE_TYPE_KMALLOC = 0, /* kmalloc() or kfree(). */
KMEMTRACE_TYPE_CACHE, /* kmem_cache_*(). */
@@ -323,6 +340,8 @@ extern void __ftrace_bad_type(void);
TRACE_SYSCALL_ENTER); \
IF_ASSIGN(var, ent, struct syscall_trace_exit, \
TRACE_SYSCALL_EXIT); \
+ IF_ASSIGN(var, ent, struct trace_skb_event, \
+ TRACE_SKB_SOURCE); \
__ftrace_bad_type(); \
} while (0)
diff --git a/kernel/trace/trace_skb_sources.c b/kernel/trace/trace_skb_sources.c
new file mode 100644
index 0000000..4ba3671
--- /dev/null
+++ b/kernel/trace/trace_skb_sources.c
@@ -0,0 +1,154 @@
+/*
+ * ring buffer based tracer for analyzing per-socket skb sources
+ *
+ * Neil Horman <nhorman@tuxdriver.com>
+ * Copyright (C) 2009
+ *
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/debugfs.h>
+#include <trace/events/skb.h>
+#include <linux/kallsyms.h>
+#include <linux/module.h>
+#include <linux/hardirq.h>
+#include <linux/netdevice.h>
+#include <net/sock.h>
+
+#include "trace.h"
+#include "trace_output.h"
+
+EXPORT_TRACEPOINT_SYMBOL_GPL(skb_copy_datagram_iovec);
+
+static struct trace_array *skb_trace;
+static int __read_mostly trace_skb_source_enabled;
+
+static void probe_skb_dequeue(const struct sk_buff *skb, int len)
+{
+ struct ring_buffer_event *event;
+ struct trace_skb_event *entry;
+ struct trace_array *tr = skb_trace;
+ struct net_device *dev;
+
+ if (!trace_skb_source_enabled)
+ return;
+
+ if (in_interrupt())
+ return;
+
+ event = trace_buffer_lock_reserve(tr, TRACE_SKB_SOURCE,
+ sizeof(*entry), 0, 0);
+ if (!event)
+ return;
+ entry = ring_buffer_event_data(event);
+
+ entry->event_data.pid = current->pid;
+ entry->event_data.anid = page_to_nid(virt_to_page(skb->data));
+ entry->event_data.cnid = cpu_to_node(smp_processor_id());
+ entry->event_data.len = len;
+ entry->event_data.rx_queue = skb->queue_mapping;
+ entry->event_data.ccpu = smp_processor_id();
+
+ dev = dev_get_by_index(sock_net(skb->sk), skb->iif);
+ if (dev) {
+ memcpy(entry->event_data.ifname, dev->name, IFNAMSIZ);
+ dev_put(dev);
+ } else {
+ strcpy(entry->event_data.ifname, "Unknown");
+ }
+
+ trace_buffer_unlock_commit(tr, event, 0, 0);
+}
+
+static int tracing_skb_source_register(void)
+{
+ int ret;
+
+ ret = register_trace_skb_copy_datagram_iovec(probe_skb_dequeue);
+ if (ret)
+ pr_info("skb source trace: Couldn't activate dequeue tracepoint");
+
+ return ret;
+}
+
+static void start_skb_source_trace(struct trace_array *tr)
+{
+ trace_skb_source_enabled = 1;
+}
+
+static void stop_skb_source_trace(struct trace_array *tr)
+{
+ trace_skb_source_enabled = 0;
+}
+
+static void skb_source_trace_reset(struct trace_array *tr)
+{
+ trace_skb_source_enabled = 0;
+ unregister_trace_skb_copy_datagram_iovec(probe_skb_dequeue);
+}
+
+
+static int skb_source_trace_init(struct trace_array *tr)
+{
+ int cpu;
+ skb_trace = tr;
+
+ trace_skb_source_enabled = 1;
+ tracing_skb_source_register();
+
+ for_each_cpu(cpu, cpu_possible_mask)
+ tracing_reset(tr, cpu);
+ return 0;
+}
+
+static enum print_line_t skb_source_print_line(struct trace_iterator *iter)
+{
+ int ret = 0;
+ struct trace_entry *entry = iter->ent;
+ struct trace_skb_event *event;
+ struct skb_record *record;
+ struct trace_seq *s = &iter->seq;
+
+ trace_assign_type(event, entry);
+ record = &event->event_data;
+ if (entry->type != TRACE_SKB_SOURCE)
+ return TRACE_TYPE_UNHANDLED;
+
+ ret = trace_seq_printf(s, " %d %d %d %s %d %d %d\n",
+ record->pid,
+ record->anid,
+ record->cnid,
+ record->ifname,
+ record->rx_queue,
+ record->ccpu,
+ record->len);
+
+ if (!ret)
+ return TRACE_TYPE_PARTIAL_LINE;
+
+ return TRACE_TYPE_HANDLED;
+}
+
+static void skb_source_print_header(struct seq_file *s)
+{
+ seq_puts(s, "# PID ANID CNID IFC RXQ CCPU LEN\n");
+ seq_puts(s, "# | | | | | | |\n");
+}
+
+static struct tracer skb_source_tracer __read_mostly =
+{
+ .name = "skb_sources",
+ .init = skb_source_trace_init,
+ .start = start_skb_source_trace,
+ .stop = stop_skb_source_trace,
+ .reset = skb_source_trace_reset,
+ .print_line = skb_source_print_line,
+ .print_header = skb_source_print_header,
+};
+
+static int init_skb_source_trace(void)
+{
+ return register_tracer(&skb_source_tracer);
+}
+device_initcall(init_skb_source_trace);
^ permalink raw reply related
* Re: [PATCH 2/3] net: Add ftracer to help optimize process scheduling based on incomming frame allocations (v2)
From: Neil Horman @ 2009-08-08 23:15 UTC (permalink / raw)
To: netdev; +Cc: davem, rostedt
In-Reply-To: <20090808231322.GA24414@localhost.localdomain>
skb allocation / consumption corelator - Add config option
This patch adds a Kconfig option to enable the addtition of the skb source
tracer.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
Kconfig | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 1551f47..1aeec05 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -234,6 +234,16 @@ config BOOT_TRACER
You must pass in ftrace=initcall to the kernel command line
to enable this on bootup.
+config SKB_SOURCES_TRACER
+ bool "Trace skb source information
+ select GENERIC_TRACER
+ help
+ This tracer helps developers/sysadmins correlate skb allocation and
+ consumption. The idea being that some processes will primarily consume data
+ that was allocated on certain numa nodes. By being able to visualize which
+ nodes the data was allocated on, a sysadmin or developer can optimize the
+ scheduling of those processes to cut back on cross node chatter.
+
config TRACE_BRANCH_PROFILING
bool
select GENERIC_TRACER
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH 1/3] net: Add ftracer to help optimize process scheduling based on incomming frame allocations (v2)
From: Neil Horman @ 2009-08-08 23:14 UTC (permalink / raw)
To: netdev; +Cc: davem, rostedt
In-Reply-To: <20090808231322.GA24414@localhost.localdomain>
skb allocation / cosumption tracer - Add consumption tracepoint
This patch adds a tracepoint to skb_copy_datagram_iovec, which is called each
time a userspace process copies a frame from a socket receive queue to a user
space buffer. It allows us to hook in and examine each sk_buff that the system
receives on a per-socket bases, and can be use to compile a list of which skb's
were received by which processes.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
include/trace/events/skb.h | 20 ++++++++++++++++++++
net/core/datagram.c | 3 +++
2 files changed, 23 insertions(+)
diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
index 1e8fabb..bdc5c1c 100644
--- a/include/trace/events/skb.h
+++ b/include/trace/events/skb.h
@@ -2,6 +2,7 @@
#define _TRACE_SKB_H
#include <linux/skbuff.h>
+#include <linux/netdevice.h>
#include <linux/tracepoint.h>
#undef TRACE_SYSTEM
@@ -34,6 +35,25 @@ TRACE_EVENT(kfree_skb,
__entry->skbaddr, __entry->protocol, __entry->location)
);
+TRACE_EVENT(skb_copy_datagram_iovec,
+
+ TP_PROTO(const struct sk_buff *skb, int len),
+
+ TP_ARGS(skb, len),
+
+ TP_STRUCT__entry(
+ __field( const void *, skbaddr )
+ __field( int, len )
+ ),
+
+ TP_fast_assign(
+ __entry->skbaddr = skb;
+ __entry->len = len;
+ ),
+
+ TP_printk("skbaddr=%p len=%d", __entry->skbaddr, __entry->len)
+);
+
#endif /* _TRACE_SKB_H */
/* This part must be outside protection */
diff --git a/net/core/datagram.c b/net/core/datagram.c
index b0fe692..1c6cf3a 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -55,6 +55,7 @@
#include <net/checksum.h>
#include <net/sock.h>
#include <net/tcp_states.h>
+#include <trace/events/skb.h>
/*
* Is a socket 'connection oriented' ?
@@ -284,6 +285,8 @@ int skb_copy_datagram_iovec(const struct sk_buff *skb, int offset,
int i, copy = start - offset;
struct sk_buff *frag_iter;
+ trace_skb_copy_datagram_iovec(skb, len);
+
/* Copy header. */
if (copy > 0) {
if (copy > len)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 0/3] net: Add ftracer to help optimize process scheduling based on incomming frame allocations (v2)
From: Neil Horman @ 2009-08-08 23:13 UTC (permalink / raw)
To: netdev; +Cc: davem, rostedt
In-Reply-To: <20090807202130.GA26677@hmsreliant.think-freely.org>
n Fri, Aug 07, 2009 at 04:21:30PM -0400, Neil Horman wrote:
Hey all-
I put out an RFC about this awhile ago and didn't get any loud screams,
so I've gone ahead and implemented it
Currently, our network infrastructure allows net device drivers to
allocate skbs based on the the numa node the device itself is local to. This of
course cuts down on cross numa chatter when the device is DMA-ing network
traffic to the driver. Unfortuantely no such corresponding infrastrucuture
exists at the process level. The scheduler has no insight into the numa
locality of incomming data packets for a given process (and arguably it
shouldn't), and so there is every chance that a process will run on a different
numa node than the packets that its receiving lives on, creating cross numa node
traffic.
This patch aims to provide userspace with the opportunity to optimize
that scheduling. It consists of a tracepoint and an ftrace module which exports
a history of the packets each process receives, along with the numa node each
packet was received on, as well as the numa node the process was running on when
it copied the buffer to user space. With this information, exported via the
ftrace infrastructure to user space, a sysadim can identify high prirority
processes, and optimize their scheduling so that they are more likely to run on
the same node that they are primarily receiving data on, thereby cutting down
cross numa node traffic.
Tested by me, working well, applies against the head of the net-next tree
Version 2 change notes:
I noticed that I did something stupid in patch 3, and it added a duplicated
chunk which didn't apply, this new series simply removes that, everything else
is the same
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Receive side performance issue with multi-10-GigE and NUMA
From: Neil Horman @ 2009-08-08 18:32 UTC (permalink / raw)
To: Andrew Gallatin
Cc: Bill Fink, Brice Goglin, Linux Network Developers, Yinghai Lu
In-Reply-To: <4A7DC230.6060206@myri.com>
On Sat, Aug 08, 2009 at 02:21:36PM -0400, Andrew Gallatin wrote:
> Neil Horman wrote:
>> On Sat, Aug 08, 2009 at 07:08:20AM -0400, Andrew Gallatin wrote:
>>> Bill Fink wrote:
>>>> On Fri, 07 Aug 2009, Andrew Gallatin wrote:
>>>>
>>>>> Bill Fink wrote:
>>>>>
>>>>>> All sysfs local_cpus values are the same (00000000,000000ff),
>>>>>> so yes they are also wrong.
>>>>> How were you handling IRQ binding? If local_cpus is wrong,
>>>>> the irqbalance will not be able to make good decisions about
>>>>> where to bind the NICs' IRQs. Did you try manually binding
>>>>> each NICs's interrupt to a separate CPU on the correct node?
>>>> Yes, all the NIC IRQs were bound to a CPU on the local NUMA node,
>>>> and the nuttcp application had its CPU affinity set to the same
>>>> CPU with its memory affinity bound to the same local NUMA node.
>>>> And the irqbalance daemon wasn't running.
>>> I must be misunderstanding something. I had thought that
>>> alloc_pages() on NUMA would wind up doing alloc_pages_current(), which
>>> would allocate based on default policy which (if not interleaved)
>>> should allocate from the current NUMA node. And since restocking the
>>> RX ring happens from a the driver's NAPI softirq context, then it
>>> should always be restocking on the same node the memory is destined to
>>> be consumed on.
>>>
>>> Do I just not understand how alloc_pages() works on NUMA?
>>>
>>
>> Thats how alloc_works, but most drivers use netdev_alloc_skb to refill their rx
>> ring in their napi context. netdev_alloc_skb specifically allocates an skb from
>> memory in the node that the actually NIC is local to (rather than the cpu that
>> the interrupt is running on). That cuts out cross numa node chatter when the
>> device is dma-ing a frame from the hardware to the allocated skb. The offshoot
>> of that however (especially in 10G cards with lots of rx queues whos interrupts
>> are spread out through the system) is that the irq affinity for a given irq has
>> an increased risk of not being on the same node as the skb memory. The ftrace
>> module I referenced earlier will help illustrate this, as well as cases where
>> its causing applications to run on processors that create lots of cross-node
>> chatter.
>
> One thing worth noting is that myri10ge is rather unusual in that
> it fills its RX rings with pages, then attaches them to skbs after
> the receive is done. Given how (I think) alloc_page() works, I
> don't understand why correct CPU binding does not have the same
> benefit as Bill's patch to assign the NUMA node manually.
>
> I'm certainly willing to change to myri10ge to use alloc_pages_node()
> based on NIC locality, if that provides a benefit, but I'd really
> like to understand why CPU binding is not helping.
>
Thats hard to say. If binding the app to a cpu on the same node doesn't help,
that would suggest to me:
1) That the process binding isn't being honored
2) The cpu you're binding to isn't actually on the same node
3) The node which the skb's are allocated on is not the one you think it is
4) The cross numa chatter is improved, but another problem has taken its place
(like cpu contention between the process and the interrupt handler on the samme
cpu)
5) The problem is something else entirely.
Either way, I'd suggest applying and running the patch set that I referenced
previously. It will give you a good table representation of how skbs for this
process are being allocated and consumed, and let you confirm or eliminate items
1-4 above.
Neil
> Drew
>
^ permalink raw reply
* Re: Receive side performance issue with multi-10-GigE and NUMA
From: Andrew Gallatin @ 2009-08-08 18:21 UTC (permalink / raw)
To: Neil Horman; +Cc: Bill Fink, Brice Goglin, Linux Network Developers, Yinghai Lu
In-Reply-To: <20090808112636.GB18518@localhost.localdomain>
Neil Horman wrote:
> On Sat, Aug 08, 2009 at 07:08:20AM -0400, Andrew Gallatin wrote:
>> Bill Fink wrote:
>>> On Fri, 07 Aug 2009, Andrew Gallatin wrote:
>>>
>>>> Bill Fink wrote:
>>>>
>>>>> All sysfs local_cpus values are the same (00000000,000000ff),
>>>>> so yes they are also wrong.
>>>> How were you handling IRQ binding? If local_cpus is wrong,
>>>> the irqbalance will not be able to make good decisions about
>>>> where to bind the NICs' IRQs. Did you try manually binding
>>>> each NICs's interrupt to a separate CPU on the correct node?
>>> Yes, all the NIC IRQs were bound to a CPU on the local NUMA node,
>>> and the nuttcp application had its CPU affinity set to the same
>>> CPU with its memory affinity bound to the same local NUMA node.
>>> And the irqbalance daemon wasn't running.
>> I must be misunderstanding something. I had thought that
>> alloc_pages() on NUMA would wind up doing alloc_pages_current(), which
>> would allocate based on default policy which (if not interleaved)
>> should allocate from the current NUMA node. And since restocking the
>> RX ring happens from a the driver's NAPI softirq context, then it
>> should always be restocking on the same node the memory is destined to
>> be consumed on.
>>
>> Do I just not understand how alloc_pages() works on NUMA?
>>
>
> Thats how alloc_works, but most drivers use netdev_alloc_skb to refill their rx
> ring in their napi context. netdev_alloc_skb specifically allocates an skb from
> memory in the node that the actually NIC is local to (rather than the cpu that
> the interrupt is running on). That cuts out cross numa node chatter when the
> device is dma-ing a frame from the hardware to the allocated skb. The offshoot
> of that however (especially in 10G cards with lots of rx queues whos interrupts
> are spread out through the system) is that the irq affinity for a given irq has
> an increased risk of not being on the same node as the skb memory. The ftrace
> module I referenced earlier will help illustrate this, as well as cases where
> its causing applications to run on processors that create lots of cross-node
> chatter.
One thing worth noting is that myri10ge is rather unusual in that
it fills its RX rings with pages, then attaches them to skbs after
the receive is done. Given how (I think) alloc_page() works, I
don't understand why correct CPU binding does not have the same
benefit as Bill's patch to assign the NUMA node manually.
I'm certainly willing to change to myri10ge to use alloc_pages_node()
based on NIC locality, if that provides a benefit, but I'd really
like to understand why CPU binding is not helping.
Drew
^ permalink raw reply
* Re: [PATCH] korina: Read buffer overflow
From: Phil Sutter @ 2009-08-08 16:45 UTC (permalink / raw)
To: roel kluin; +Cc: netdev, Andrew Morton, David S. Miller, florian
In-Reply-To: <25e057c00908080614q6d7efa3x46789e51b0b544b3@mail.gmail.com>
Hi,
On Sat, Aug 08, 2009 at 03:14:57PM +0200, roel kluin wrote:
> A few comments:
>
> > | diff --git a/drivers/net/korina.c b/drivers/net/korina.c
> > | index a2701f5..d1c5276 100644
> > | --- a/drivers/net/korina.c
> > | +++ b/drivers/net/korina.c
>
> > | @@ -772,6 +772,13 @@ static void korina_alloc_ring(struct net_device *dev)
> > | lp->rd_ring[i].ca = CPHYSADDR(skb->data);
> > | lp->rd_ring[i].link = CPHYSADDR(&lp->rd_ring[i+1]);
> > | }
> > | + if (!i) {
> > | + printk(KERN_ERR DRV_NAME "%s: could not allocate a single"
> > | + " receive descriptor\n", dev->name)
>
> printk(KERN_ERR "%s: could not allocate a single receive
> descriptor\n",
> dev->name);
>
> Don't interrupt strings, they are an exception in the 80 character
> limit (since they are easier to grep that way).
Ah, good to know.
> Also I think the DRV_NAME should be omitted, Doesn't
> `DRV_NAME "%s:...\n", dev->name)' result in "korinakorina:..."? It
> occurs at more places in the file. Also a semicolon was missing.
Sounds like a valid point, I will check this. This patch was just a
quick hack to illustrate what I had in mind, without even
compile-testing it.
> Otherwise looks fine to me.
Great you like it, and many thanks for reviewing it. I'm still not
completely sure the NIC will still work with less than 64 receive
descriptors, but since the documentation wasn't really enlightening I'll
add some run-time test to my TODO. Maybe one of the other readers has
some advice here, I'll prepare a complete patch in the meantime.
Greetings, Phil
^ permalink raw reply
* Re: [PATCH] drivers/net/wireless/ath/ath5k: Change constant name
From: Bob Copeland @ 2009-08-08 16:20 UTC (permalink / raw)
To: Julia Lawall
Cc: kernel-janitors-u79uwXL29TY76Z2rM5mHXA, Jiri Slaby,
netdev-u79uwXL29TY76Z2rM5mHXA, ath5k-devel-xDcbHBWguxEUs3QNXV6qNA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linville-2XuSBdqkA4R54TAoqtyWWQ
In-Reply-To: <Pine.LNX.4.64.0908081521210.27715-QfmoRoYWmW9knbxzx/v8hQ@public.gmane.org>
On Sat, Aug 08, 2009 at 03:22:26PM +0200, Julia Lawall wrote:
> From: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>
>
> Elsewhere, the tqi_type field is compared to constants having a name
> beginning with AR5K_TX_QUEUE, rather than AR5K_TX_QUEUE_ID. I have thus
> converted AR5K_TX_QUEUE_ID_CAB to AR5K_TX_QUEUE_CAB. This does, however,
> change the value, so perhaps something else was wanted.
>
> Signed-off-by: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>
Nice catch, thanks. Shouldn't cause any problem with 2.6.31 since we
don't use the CAB queue yet, but it was definitely wrong.
Acked-by: Bob Copeland <me-aXfl/3sk2vNUbtYUoyoikg@public.gmane.org>
> diff --git a/drivers/net/wireless/ath/ath5k/qcu.c b/drivers/net/wireless/ath/ath5k/qcu.c
> index 6d5aaf0..eeebb9a 100644
> --- a/drivers/net/wireless/ath/ath5k/qcu.c
> +++ b/drivers/net/wireless/ath/ath5k/qcu.c
> @@ -362,7 +362,7 @@ int ath5k_hw_reset_tx_queue(struct ath5k_hw *ah, unsigned int queue)
> }
>
> if (tq->tqi_ready_time &&
> - (tq->tqi_type != AR5K_TX_QUEUE_ID_CAB))
> + (tq->tqi_type != AR5K_TX_QUEUE_CAB))
> ath5k_hw_reg_write(ah, AR5K_REG_SM(tq->tqi_ready_time,
> AR5K_QCU_RDYTIMECFG_INTVAL) |
> AR5K_QCU_RDYTIMECFG_ENABLE,
>
--
Bob Copeland %% www.bobcopeland.com
^ permalink raw reply
* Re: [PATCH] korina: Read buffer overflow
From: roel kluin @ 2009-08-08 13:14 UTC (permalink / raw)
To: Roel Kluin, netdev, Andrew Morton, David S. Miller, florian
In-Reply-To: <20090808004815.F10394CEAA@orbit.nwl.cc>
>> If the loop breaks with an i of 0, then we read lp->rd_ring[-1].
>> @@ -771,7 +771,7 @@ static void korina_alloc_ring(struct net_device *dev)
>> for (i = 0; i < KORINA_NUM_RDS; i++) {
>> skb = dev_alloc_skb(KORINA_RBSIZE + 2);
>> if (!skb)
>> - break;
>> + goto err_free;
>
> This implies that all KORINA_NUM_TDS receive descriptors need to be
> available for the driver to work.
> Also I guess there should be some error handling, as the driver probably
> wont be useful without a single receive descriptor. What do you think
> about the following:
A few comments:
> | diff --git a/drivers/net/korina.c b/drivers/net/korina.c
> | index a2701f5..d1c5276 100644
> | --- a/drivers/net/korina.c
> | +++ b/drivers/net/korina.c
> | @@ -772,6 +772,13 @@ static void korina_alloc_ring(struct net_device *dev)
> | lp->rd_ring[i].ca = CPHYSADDR(skb->data);
> | lp->rd_ring[i].link = CPHYSADDR(&lp->rd_ring[i+1]);
> | }
> | + if (!i) {
> | + printk(KERN_ERR DRV_NAME "%s: could not allocate a single"
> | + " receive descriptor\n", dev->name)
printk(KERN_ERR "%s: could not allocate a single receive
descriptor\n",
dev->name);
Don't interrupt strings, they are an exception in the 80 character
limit (since they are
easier to grep that way). Also I think the DRV_NAME should be omitted, Doesn't
`DRV_NAME "%s:...\n", dev->name)' result in "korinakorina:..."? It
occurs at more
places in the file. Also a semicolon was missing.
> | + return 1;
I think -ENOMEM is more appropriate.
> | + }
> | + printk(KERN_DEBUG DRV_NAME "%s: allocated %d receive descriptors\n",
> | + dev->name, i);
same `DRV_NAME "%s:...\n", dev->name)' issue.
> | @@ -824,7 +833,8 @@ static int korina_init(struct net_device *dev)
> | writel(ETH_INT_FC_EN, &lp->eth_regs->ethintfc);
> |
> | /* Allocate rings */
> | - korina_alloc_ring(dev);
> | + if (korina_alloc_ring(dev))
> | + return 1;
-ENOMEM
> |
> | writel(0, &lp->rx_dma_regs->dmas);
> | /* Start Rx DMA */
>
> Greetings, Phil
Otherwise looks fine to me.
Thanks,
Roel
^ permalink raw reply
* Re: some bug in iproute2
From: jamal @ 2009-08-08 12:49 UTC (permalink / raw)
To: Sergey Popov; +Cc: Patrick McHardy, Stephen Hemminger, netdev
In-Reply-To: <1249734651.7101.38.camel@dogo.mojatatu.com>
Sergey,
Hope you dont mind if i cc some other people so we can
resolve this issue because i am looking at the git logs
and seeing tons of activities which affect what you are
trying to do.
On Sat, 2009-08-08 at 08:30 -0400, jamal wrote:
> Hi,
>
> On Fri, 2009-08-07 at 20:27 +0300, Sergey Popov wrote:
> > В Fri, 07 Aug 2009 10:28:05 -0400
> > Now i want to police the incoming traffic and drop all the packets
> > that exceeds the 1000 kbit and redirect packets
> > to ifb, but next line doesn't work as expected [by me]:
> >
>
> What distro are you running?
>
> > # tc f add dev $INETIF parent ffff: proto ip prio 1 u32 match u32 0 0 \
> > action police rate 1000kbit burst 1k drop \
> > action mirred egress redirect dev $IFBIF
> >
> > Illegal "action"
> > bad action parsing
> > parse_action: bad value (12:police)!
> > Illegal "action"
>
> Let me try simple version:
> dogo:~# tc q add dev lo ingress
> dogo:~# tc f add dev lo parent ffff: proto ip prio 1 u32 match u32 0 0
> action police rate 1000kbit burst 1k drop action mirred egress redirect
> dev eth0
> Illegal "action"
> bad action parsing
> parse_action: bad value (12:police)!
> Illegal "action"
> dogo:~#
>
> Ok, this seems to be a bug with policer...
> Although your syntax above seems wrong since you didnt specify flowid.
> You must specify flowid always or strange things will happen even if
> the syntax is accepted.
Your usage and syntax is perfectly legal and very useful.
The problem is in the old policer syntax the keyword "action" had some
speacial meaning. Some old scripts used it and broke if new syntax was
used.
The only clean option i see forward (which doesnt break any legacy stuff
or play acrobatics) is to introduce new action "npolice". Patrick,
thoughts?
cheers,
jamal
^ permalink raw reply
* Re: Receive side performance issue with multi-10-GigE and NUMA
From: Neil Horman @ 2009-08-08 11:26 UTC (permalink / raw)
To: Andrew Gallatin
Cc: Bill Fink, Brice Goglin, Linux Network Developers, Yinghai Lu
In-Reply-To: <4A7D5CA4.3030307@myri.com>
On Sat, Aug 08, 2009 at 07:08:20AM -0400, Andrew Gallatin wrote:
> Bill Fink wrote:
> > On Fri, 07 Aug 2009, Andrew Gallatin wrote:
> >
> >> Bill Fink wrote:
> >>
> >>> All sysfs local_cpus values are the same (00000000,000000ff),
> >>> so yes they are also wrong.
> >> How were you handling IRQ binding? If local_cpus is wrong,
> >> the irqbalance will not be able to make good decisions about
> >> where to bind the NICs' IRQs. Did you try manually binding
> >> each NICs's interrupt to a separate CPU on the correct node?
> >
> > Yes, all the NIC IRQs were bound to a CPU on the local NUMA node,
> > and the nuttcp application had its CPU affinity set to the same
> > CPU with its memory affinity bound to the same local NUMA node.
> > And the irqbalance daemon wasn't running.
>
> I must be misunderstanding something. I had thought that
> alloc_pages() on NUMA would wind up doing alloc_pages_current(), which
> would allocate based on default policy which (if not interleaved)
> should allocate from the current NUMA node. And since restocking the
> RX ring happens from a the driver's NAPI softirq context, then it
> should always be restocking on the same node the memory is destined to
> be consumed on.
>
> Do I just not understand how alloc_pages() works on NUMA?
>
Thats how alloc_works, but most drivers use netdev_alloc_skb to refill their rx
ring in their napi context. netdev_alloc_skb specifically allocates an skb from
memory in the node that the actually NIC is local to (rather than the cpu that
the interrupt is running on). That cuts out cross numa node chatter when the
device is dma-ing a frame from the hardware to the allocated skb. The offshoot
of that however (especially in 10G cards with lots of rx queues whos interrupts
are spread out through the system) is that the irq affinity for a given irq has
an increased risk of not being on the same node as the skb memory. The ftrace
module I referenced earlier will help illustrate this, as well as cases where
its causing applications to run on processors that create lots of cross-node
chatter.
Neil
> Thanks,
>
> Drew
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: Receive side performance issue with multi-10-GigE and NUMA
From: Andrew Gallatin @ 2009-08-08 11:08 UTC (permalink / raw)
To: Bill Fink; +Cc: Brice Goglin, Linux Network Developers, Yinghai Lu
In-Reply-To: <20090807213557.d0faec23.billfink@mindspring.com>
Bill Fink wrote:
> On Fri, 07 Aug 2009, Andrew Gallatin wrote:
>
>> Bill Fink wrote:
>>
>>> All sysfs local_cpus values are the same (00000000,000000ff),
>>> so yes they are also wrong.
>> How were you handling IRQ binding? If local_cpus is wrong,
>> the irqbalance will not be able to make good decisions about
>> where to bind the NICs' IRQs. Did you try manually binding
>> each NICs's interrupt to a separate CPU on the correct node?
>
> Yes, all the NIC IRQs were bound to a CPU on the local NUMA node,
> and the nuttcp application had its CPU affinity set to the same
> CPU with its memory affinity bound to the same local NUMA node.
> And the irqbalance daemon wasn't running.
I must be misunderstanding something. I had thought that
alloc_pages() on NUMA would wind up doing alloc_pages_current(), which
would allocate based on default policy which (if not interleaved)
should allocate from the current NUMA node. And since restocking the
RX ring happens from a the driver's NAPI softirq context, then it
should always be restocking on the same node the memory is destined to
be consumed on.
Do I just not understand how alloc_pages() works on NUMA?
Thanks,
Drew
^ permalink raw reply
* Re: [PATCH][RFC] net/bridge: add basic VEPA support
From: Arnd Bergmann @ 2009-08-08 9:49 UTC (permalink / raw)
To: Paul Congdon (UC Davis)
Cc: 'Stephen Hemminger', 'Fischer, Anna', bridge,
linux-kernel, netdev, virtualization, evb, davem, kaber,
adobriyan
In-Reply-To: <004e01ca1790$fec36c30$fc4a4490$@edu>
On Friday 07 August 2009, Paul Congdon (UC Davis) wrote:
> As I understand the macvlan code, it currently doesn't allow two VMs on the
> same machine to communicate with one another.
There are patches to do that. I think if we add that, there should be
a way to choose the behavior between either bridging between the
guests or VEPA.
> I could imagine a hairpin mode on the adjacent bridge making this
> possible, but the macvlan code would need to be updated to filter
> reflected frames so a source did not receive his own packet.
Right, I missed this point so far. I'll follow up with a patch
to do that.
> I could imagine this being done as well, but to also
> support selective multicast usage, something similar to the bridge
> forwarding table would be needed. I think putting VEPA into a new driver
> would cause you to implement many things the bridge code already supports.
> Given that we expect the bridge standard to ultimately include VEPA, and the
> new functions are basic forwarding operations, it seems to make most sense
> to keep this consistent with the bridge module.
This is the interesting part of the discussion. The bridge and macvlan
drivers certainly have an overlap in functionality and you can argue
that you only need one. Then again, the bridge code is a little crufty
and we might not want to add much more to it for functionality that can
be implemented in a much simpler way elsewhere. My preferred way would
be to use bridge when you really need 802.1d MAC learning, netfilter-bridge
and STP, while we put the optimizations for stuff like VMDq, zero-copy
and multiqueue guest adapters only into the macvlan code.
Arnd <><
^ permalink raw reply
* Re: [evb] RE: [PATCH][RFC] net/bridge: add basic VEPA support
From: Arnd Bergmann @ 2009-08-08 9:44 UTC (permalink / raw)
To: Benny Amorsen
Cc: Fischer, Anna, Yaron Haviv, evb, shemminger, davem, netdev,
bridge, adobriyan, Paul Congdon (UC Davis), virtualization
In-Reply-To: <m3d476hepp.fsf@ursa.amorsen.dk>
On Saturday 08 August 2009, Benny Amorsen wrote:
> Would a SRIOV NIC with VEPA support show up as multiple devices? I.e.
> would I get e.g. eth0-eth7 for a NIC with support for 8 virtual
> interfaces? Would they have different MAC addresses?
It could, but the idea of SR-IOV is that it shows up as 8 PCI
devices. One of them is owned by the host and is seen as eth0
there. The other seven PCI devices (virtual functions) are meant
to be assigned to the guest using PCI passthrough and will show
up as the guests eth0, each one with its own MAC address.
An other mode of operation is VMDq, where the host owns all
interfaces and you might see eth0-eth7 there. You can then attach
a qemu process with a raw packet socket or a single macvtap port
for each of those interfaces. This is not yet implemented in Linux,
so how it will be done is still open. It might all be integrated
into macvlan or some new subsystem alternatively.
AFAIK, every SR-IOV adapter can also be operated as a VMDq adapter,
but there are VMDq adapters that do not support SR-IOV.
Arnd <><
^ permalink raw reply
* Re: [evb] RE: [PATCH][RFC] net/bridge: add basic VEPA support
From: Arnd Bergmann @ 2009-08-08 9:22 UTC (permalink / raw)
To: Fischer, Anna
Cc: Yaron Haviv, evb@yahoogroups.com, shemminger@linux-foundation.org,
bridge@lists.linux-foundation.org, netdev@vger.kernel.org,
virtualization@lists.linux-foundation.org, davem@davemloft.net,
kaber@trash.net, adobriyan@gmail.com, Paul Congdon (UC Davis)
In-Reply-To: <0199E0D51A61344794750DC57738F58E6D6A6CD803@GVW1118EXC.americas.hpqcorp.net>
On Friday 07 August 2009, Fischer, Anna wrote:
> If you do have a SRIOV NIC that supports VEPA, then I would think
> that you do not have QEMU or macvtap in the setup any more though.
> Simply because in that case the VM can directly access the VF on
> the physical device. That would be ideal.
There may be reasons why even with an SR-IOV adapter you may want
to use the macvtap setup, with some extensions. E.g. guest migration
becomes a lot simpler if you don't have to deal with PCI passthrough
devices. If we manage to add both TX and RX zero-copy (into the
guest) to the macvlan driver, we can treat an SR-IOV adapter like
a VMDq adapter and get the best of both.
> I do think that the macvtap driver is a good addition as a simple
> and fast virtual network I/O interface, in case you do not need
> full bridge functionality. It does seem to assume though that the
> virtualization software uses QEMU/tap interfaces. How would this
> work with a Xen para-virtualized network interface? I guess there
> would need to be yet another driver?
I'm not sure how Xen guest networking works, but if neither the
traditional macvlan driver nor the macvtap driver are able to
connect it to the external NIC, then you can probably add a third
macvlan backend to handle that.
Arnd <><
^ permalink raw reply
* RE: [RESEND PATCH] gianfar: keep vlan related state when restart
From: Haruki Dai-R35557 @ 2009-08-08 9:07 UTC (permalink / raw)
To: Yong Zhang, davem, Fleming Andy-AFLEMING; +Cc: netdev, linux-kernel
In-Reply-To: <1249699012-26379-1-git-send-email-yong.zhang0@gmail.com>
> -----Original Message-----
> From: Yong Zhang [mailto:yong.zhang0@gmail.com]
> Sent: Friday, August 07, 2009 9:37 PM
> To: davem@davemloft.net; Fleming Andy-AFLEMING; Haruki Dai-R35557
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [RESEND PATCH] gianfar: keep vlan related state when restart
>
> If vlan has been enabled. ifdown followed by ifup will lost hardware
> related state.
>
> Also remove duplicated operation in gfar_vlan_rx_register().
>
> Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
Thanks for the catch. This patch looks fine to me.
Acked-by: Dai Haruki <dai.haruki@freescale.com>
> ---
> drivers/net/gianfar.c | 12 ++++++++++--
> 1 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
> index f8ffcbf..e212f2c 100644
> --- a/drivers/net/gianfar.c
> +++ b/drivers/net/gianfar.c
> @@ -936,6 +936,7 @@ int startup_gfar(struct net_device *dev)
> struct gfar __iomem *regs = priv->regs;
> int err = 0;
> u32 rctrl = 0;
> + u32 tctrl = 0;
> u32 attrs = 0;
>
> gfar_write(®s->imask, IMASK_INIT_CLEAR);
> @@ -1111,11 +1112,19 @@ int startup_gfar(struct net_device *dev)
> rctrl |= RCTRL_PADDING(priv->padding);
> }
>
> + /* keep vlan related bits if it's enabled */
> + if (priv->vlgrp) {
> + rctrl |= RCTRL_VLEX | RCTRL_PRSDEP_INIT;
> + tctrl |= TCTRL_VLINS;
> + }
> +
> /* Init rctrl based on our settings */
> gfar_write(&priv->regs->rctrl, rctrl);
>
> if (dev->features & NETIF_F_IP_CSUM)
> - gfar_write(&priv->regs->tctrl, TCTRL_INIT_CSUM);
> + tctrl |= TCTRL_INIT_CSUM;
> +
> + gfar_write(&priv->regs->tctrl, tctrl);
>
> /* Set the extraction length and index */
> attrs = ATTRELI_EL(priv->rx_stash_size) |
> @@ -1450,7 +1459,6 @@ static void gfar_vlan_rx_register(struct
net_device *dev,
>
> /* Enable VLAN tag extraction */
> tempval = gfar_read(&priv->regs->rctrl);
> - tempval |= RCTRL_VLEX;
> tempval |= (RCTRL_VLEX | RCTRL_PRSDEP_INIT);
> gfar_write(&priv->regs->rctrl, tempval);
> } else {
> --
> 1.6.0.4
^ 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