These patches do some of the cleanup''s for netfront necessary to get it more acceptable for mainline. They build, but HAVE NOT BEEN TESTED; I am new to this and don''t have the time or setup to do much with Xen. 1) style fixes 2) rx buffer counters should be unsigned 3) add section annotations 4) don''t kfree() netdevice 5) remove procfs 6) add sysfs support _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
# HG changeset patch # User shemminger@localhost.localdomain # Node ID f767d5dc33930e12d267ccf4a0fbe908414d06e2 # Parent 7cbc1fc8dbea8c4ceb7caac014f5e1a134f10e9c Minor style fix and eliminate Emacs style comment which is considered poor taste by current kernel community. Signed-off-by: Stephen Hemminger <shemminger@osdl.org> diff -r 7cbc1fc8dbea -r f767d5dc3393 linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c --- a/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c Tue May 16 18:54:41 2006 +++ b/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c Wed May 17 20:41:25 2006 @@ -1,25 +1,25 @@ /****************************************************************************** * Virtual network driver for conversing with remote driver backends. - * + * * Copyright (c) 2002-2005, K A Fraser * Copyright (c) 2005, XenSource Ltd - * + * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License version 2 * as published by the Free Software Foundation; or, when distributed * separately from the Linux kernel or incorporated into other * software packages, subject to the following license: - * + * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this source file (the "Software"), to deal in the Software without * restriction, including without limitation the rights to use, copy, modify, * merge, publish, distribute, sublicense, and/or sell copies of the Software, * and to permit persons to whom the Software is furnished to do so, subject to * the following conditions: - * + * * The above copyright notice and this permission notice shall be included in * all copies or substantial portions of the Software. - * + * * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE @@ -75,8 +75,7 @@ skb_shinfo(skb)->frag_list = NULL; } -struct netfront_info -{ +struct netfront_info { struct list_head list; struct net_device *netdev; @@ -1524,14 +1523,3 @@ } #endif - - -/* - * Local variables: - * c-file-style: "linux" - * indent-tabs-mode: t - * c-indent-level: 8 - * c-basic-offset: 8 - * tab-width: 8 - * End: - */ _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stephen Hemminger
2006-May-17 21:28 UTC
[Xen-devel] [PATCH 2/6] netfront: rx counter should be unsigned
# HG changeset patch # User shemminger@localhost.localdomain # Node ID a471b0faf794558e8cc0cd595b293f45924a561c # Parent f767d5dc33930e12d267ccf4a0fbe908414d06e2 The receive buffer accounting should be unsigned to avoid range comparison issues. Signed-off-by: Stephen Hemminger <shemminger@osdl.org> diff -r f767d5dc3393 -r a471b0faf794 linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c --- a/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c Wed May 17 20:41:25 2006 +++ b/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c Wed May 17 20:43:22 2006 @@ -106,7 +106,7 @@ #define RX_MIN_TARGET 8 #define RX_DFL_MIN_TARGET 64 #define RX_MAX_TARGET min_t(int, NET_RX_RING_SIZE, 256) - int rx_min_target, rx_max_target, rx_target; + unsigned rx_min_target, rx_max_target, rx_target; struct sk_buff_head rx_batch; struct timer_list rx_refill_timer; _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stephen Hemminger
2006-May-17 21:29 UTC
[Xen-devel] [PATCH 3/6] netfront: use section tags on init and removal
# HG changeset patch # User shemminger@localhost.localdomain # Node ID 44ad0558e50c91ad66a155c017d6df0cb36e007d # Parent a471b0faf794558e8cc0cd595b293f45924a561c The initialization and removal functions should be tagged with proper section attributes to save space. Signed-off-by: Stephen Hemminger <shemminger@osdl.org> diff -r a471b0faf794 -r 44ad0558e50c linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c --- a/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c Wed May 17 20:43:22 2006 +++ b/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c Wed May 17 20:44:25 2006 @@ -152,7 +152,7 @@ } #ifdef DEBUG -static char *be_state_name[] = { +static const char *be_state_name[] = { [BEST_CLOSED] = "closed", [BEST_DISCONNECTED] = "disconnected", [BEST_CONNECTED] = "connected", @@ -204,8 +204,8 @@ * inform the backend of the appropriate details for those. Switch to * Connected state. */ -static int netfront_probe(struct xenbus_device *dev, - const struct xenbus_device_id *id) +static int __devinit netfront_probe(struct xenbus_device *dev, + const struct xenbus_device_id *id) { int err; struct net_device *netdev; @@ -1112,8 +1112,8 @@ * @param val return parameter for created device * @return 0 on success, error code otherwise */ -static int create_netdev(int handle, struct xenbus_device *dev, - struct net_device **val) +static int __devinit create_netdev(int handle, struct xenbus_device *dev, + struct net_device **val) { int i, err = 0; struct net_device *netdev = NULL; @@ -1250,7 +1250,7 @@ } -static int netfront_remove(struct xenbus_device *dev) +static int __devexit netfront_remove(struct xenbus_device *dev) { struct netfront_info *info = dev->data; @@ -1330,7 +1330,7 @@ .owner = THIS_MODULE, .ids = netfront_ids, .probe = netfront_probe, - .remove = netfront_remove, + .remove = __devexit_p(netfront_remove), .resume = netfront_resume, .otherend_changed = backend_changed, }; @@ -1361,7 +1361,7 @@ module_init(netif_init); -static void netif_exit(void) +static void __exit netif_exit(void) { unregister_inetaddr_notifier(¬ifier_inetdev); _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stephen Hemminger
2006-May-17 21:31 UTC
[Xen-devel] [PATCH 4/6] netfront: use free_netdev and other unwinds
# HG changeset patch # User shemminger@localhost.localdomain # Node ID 06cea5a9cbae9af028300e470849bc034beaff75 # Parent 44ad0558e50c91ad66a155c017d6df0cb36e007d Network devices need to be freed by free_netdev not kfree. Also, fix some missing unwinds in error path. Still not convinced there aren''t some lurking double free''s in the error paths here. Signed-off-by: Stephen Hemminger <shemminger@osdl.org> diff -r 44ad0558e50c -r 06cea5a9cbae linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c --- a/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c Wed May 17 20:44:25 2006 +++ b/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c Wed May 17 20:48:41 2006 @@ -229,7 +229,8 @@ err = talk_to_backend(dev, info); if (err) { - kfree(info); + unregister_netdev(netdev); + free_netdev(netdev); dev->data = NULL; return err; } @@ -1119,11 +1120,11 @@ struct net_device *netdev = NULL; struct netfront_info *np = NULL; - if ((netdev = alloc_etherdev(sizeof(struct netfront_info))) == NULL) { + netdev = alloc_etherdev(sizeof(struct netfront_info)); + if (!netdev) { printk(KERN_WARNING "%s> alloc_etherdev failed.\n", __FUNCTION__); - err = -ENOMEM; - goto exit; + return -ENOMEM; } np = netdev_priv(netdev); @@ -1168,7 +1169,7 @@ printk(KERN_ALERT "#### netfront can''t alloc rx grant refs\n"); gnttab_free_grant_references(np->gref_tx_head); err = -ENOMEM; - goto exit; + goto exit_free_tx; } netdev->open = network_open; @@ -1197,18 +1198,19 @@ } np->netdev = netdev; - + if (val) + *val = netdev; + + return 0; + + + exit_free_grefs: + gnttab_free_grant_references(np->gref_rx_head); + exit_free_tx: + gnttab_free_grant_references(np->gref_tx_head); exit: - if (err != 0) - kfree(netdev); - else if (val != NULL) - *val = netdev; + free_netdev(netdev); return err; - - exit_free_grefs: - gnttab_free_grant_references(np->gref_tx_head); - gnttab_free_grant_references(np->gref_rx_head); - goto exit; } /* _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stephen Hemminger
2006-May-17 21:32 UTC
[Xen-devel] [PATCH 5/6] netfront: remove /proc interface
# HG changeset patch # User shemminger@localhost.localdomain # Node ID 5663fb293dd7810e7d707a5e467a9d0de9a6d230 # Parent 06cea5a9cbae9af028300e470849bc034beaff75 Give netfront a procectomy. The existing /proc interface for tuning in buggy (network device names can change), and would not be accepted in mainline. Signed-off-by: Stephen Hemminger <shemminger@osdl.org> diff -r 06cea5a9cbae -r 5663fb293dd7 linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c --- a/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c Wed May 17 20:48:41 2006 +++ b/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c Wed May 17 20:50:07 2006 @@ -187,16 +187,6 @@ static irqreturn_t netif_int(int irq, void *dev_id, struct pt_regs *ptregs); -#ifdef CONFIG_PROC_FS -static int xennet_proc_init(void); -static int xennet_proc_addif(struct net_device *dev); -static void xennet_proc_delif(struct net_device *dev); -#else -#define xennet_proc_init() (0) -#define xennet_proc_addif(d) (0) -#define xennet_proc_delif(d) ((void)0) -#endif - /** * Entry point to this code when a new device is created. Allocate the basic @@ -1192,11 +1182,6 @@ goto exit_free_grefs; } - if ((err = xennet_proc_addif(netdev)) != 0) { - unregister_netdev(netdev); - goto exit_free_grefs; - } - np->netdev = netdev; if (val) *val = netdev; @@ -1270,10 +1255,6 @@ spin_lock_irq(&info->netdev->xmit_lock); netif_stop_queue(info->netdev); spin_unlock_irq(&info->netdev->xmit_lock); - -#ifdef CONFIG_PROC_FS - xennet_proc_delif(info->netdev); -#endif del_timer_sync(&info->rx_refill_timer); @@ -1351,9 +1332,6 @@ if (xen_start_info->flags & SIF_INITDOMAIN) return 0; - if ((err = xennet_proc_init()) != 0) - return err; - IPRINTK("Initialising virtual ethernet driver.\n"); (void)register_inetaddr_notifier(¬ifier_inetdev); @@ -1372,156 +1350,3 @@ module_exit(netif_exit); MODULE_LICENSE("Dual BSD/GPL"); - - -/* ** /proc **/ - - -#ifdef CONFIG_PROC_FS - -#define TARGET_MIN 0UL -#define TARGET_MAX 1UL -#define TARGET_CUR 2UL - -static int xennet_proc_read( - char *page, char **start, off_t off, int count, int *eof, void *data) -{ - struct net_device *dev - (struct net_device *)((unsigned long)data & ~3UL); - struct netfront_info *np = netdev_priv(dev); - int len = 0, which_target = (long)data & 3; - - switch (which_target) { - case TARGET_MIN: - len = sprintf(page, "%d\n", np->rx_min_target); - break; - case TARGET_MAX: - len = sprintf(page, "%d\n", np->rx_max_target); - break; - case TARGET_CUR: - len = sprintf(page, "%d\n", np->rx_target); - break; - } - - *eof = 1; - return len; -} - -static int xennet_proc_write( - struct file *file, const char __user *buffer, - unsigned long count, void *data) -{ - struct net_device *dev - (struct net_device *)((unsigned long)data & ~3UL); - struct netfront_info *np = netdev_priv(dev); - int which_target = (long)data & 3; - char string[64]; - long target; - - if (!capable(CAP_SYS_ADMIN)) - return -EPERM; - - if (count <= 1) - return -EBADMSG; /* runt */ - if (count > sizeof(string)) - return -EFBIG; /* too long */ - - if (copy_from_user(string, buffer, count)) - return -EFAULT; - string[sizeof(string)-1] = ''\0''; - - target = simple_strtol(string, NULL, 10); - if (target < RX_MIN_TARGET) - target = RX_MIN_TARGET; - if (target > RX_MAX_TARGET) - target = RX_MAX_TARGET; - - spin_lock(&np->rx_lock); - - switch (which_target) { - case TARGET_MIN: - if (target > np->rx_max_target) - np->rx_max_target = target; - np->rx_min_target = target; - if (target > np->rx_target) - np->rx_target = target; - break; - case TARGET_MAX: - if (target < np->rx_min_target) - np->rx_min_target = target; - np->rx_max_target = target; - if (target < np->rx_target) - np->rx_target = target; - break; - case TARGET_CUR: - break; - } - - network_alloc_rx_buffers(dev); - - spin_unlock(&np->rx_lock); - - return count; -} - -static int xennet_proc_init(void) -{ - if (proc_mkdir("xen/net", NULL) == NULL) - return -ENOMEM; - return 0; -} - -static int xennet_proc_addif(struct net_device *dev) -{ - struct proc_dir_entry *dir, *min, *max, *cur; - char name[30]; - - sprintf(name, "xen/net/%s", dev->name); - - dir = proc_mkdir(name, NULL); - if (!dir) - goto nomem; - - min = create_proc_entry("rxbuf_min", 0644, dir); - max = create_proc_entry("rxbuf_max", 0644, dir); - cur = create_proc_entry("rxbuf_cur", 0444, dir); - if (!min || !max || !cur) - goto nomem; - - min->read_proc = xennet_proc_read; - min->write_proc = xennet_proc_write; - min->data = (void *)((unsigned long)dev | TARGET_MIN); - - max->read_proc = xennet_proc_read; - max->write_proc = xennet_proc_write; - max->data = (void *)((unsigned long)dev | TARGET_MAX); - - cur->read_proc = xennet_proc_read; - cur->write_proc = xennet_proc_write; - cur->data = (void *)((unsigned long)dev | TARGET_CUR); - - return 0; - - nomem: - xennet_proc_delif(dev); - return -ENOMEM; -} - -static void xennet_proc_delif(struct net_device *dev) -{ - char name[30]; - - sprintf(name, "xen/net/%s/rxbuf_min", dev->name); - remove_proc_entry(name, NULL); - - sprintf(name, "xen/net/%s/rxbuf_max", dev->name); - remove_proc_entry(name, NULL); - - sprintf(name, "xen/net/%s/rxbuf_cur", dev->name); - remove_proc_entry(name, NULL); - - sprintf(name, "xen/net/%s", dev->name); - remove_proc_entry(name, NULL); -} - -#endif _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stephen Hemminger
2006-May-17 21:32 UTC
[Xen-devel] [PATCH 6/6] netfront: sysfs tuning interface
# HG changeset patch # User shemminger@localhost.localdomain # Node ID 285c989f17bd190ae1f14b3ebc12b5380eb4d123 # Parent 5663fb293dd7810e7d707a5e467a9d0de9a6d230 Add a sysfs interface for tuning the receive buffer values. Signed-off-by: Stephen Hemminger <shemminger@osdl.org> diff -r 5663fb293dd7 -r 285c989f17bd linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c --- a/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c Wed May 17 20:50:07 2006 +++ b/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c Wed May 17 21:23:10 2006 @@ -1090,6 +1090,138 @@ .set_tx_csum = ethtool_op_set_tx_csum, }; +#ifdef CONFIG_SYSFS +static ssize_t show_rxbuf_min(struct class_device *cd, char *buf) +{ + struct net_device *netdev = container_of(cd, struct net_device, class_dev); + struct netfront_info *info = netdev_priv(netdev); + + return sprintf(buf, "%u\n", info->rx_min_target); +} + +static ssize_t store_rxbuf_min(struct class_device *cd, + const char *buf, size_t len) +{ + struct net_device *netdev = container_of(cd, struct net_device, class_dev); + struct netfront_info *np = netdev_priv(netdev); + char *endp; + unsigned long target; + + if (!capable(CAP_NET_ADMIN)) + return -EPERM; + + target = simple_strtoul(buf, &endp, 0); + if (endp == buf) + return -EBADMSG; + + if (target < RX_MIN_TARGET) + target = RX_MIN_TARGET; + if (target > RX_MAX_TARGET) + target = RX_MAX_TARGET; + + spin_lock(&np->rx_lock); + if (target > np->rx_max_target) + np->rx_max_target = target; + np->rx_min_target = target; + if (target > np->rx_target) + np->rx_target = target; + + network_alloc_rx_buffers(netdev); + + spin_unlock(&np->rx_lock); + return len; +} + +static ssize_t show_rxbuf_max(struct class_device *cd, char *buf) +{ + struct net_device *netdev = container_of(cd, struct net_device, class_dev); + struct netfront_info *info = netdev_priv(netdev); + + return sprintf(buf, "%u\n", info->rx_max_target); +} + +static ssize_t store_rxbuf_max(struct class_device *cd, + const char *buf, size_t len) +{ + struct net_device *netdev = container_of(cd, struct net_device, class_dev); + struct netfront_info *np = netdev_priv(netdev); + char *endp; + unsigned long target; + + if (!capable(CAP_NET_ADMIN)) + return -EPERM; + + target = simple_strtoul(buf, &endp, 0); + if (endp == buf) + return -EBADMSG; + + if (target < RX_MIN_TARGET) + target = RX_MIN_TARGET; + if (target > RX_MAX_TARGET) + target = RX_MAX_TARGET; + + spin_lock(&np->rx_lock); + if (target < np->rx_min_target) + np->rx_min_target = target; + np->rx_max_target = target; + if (target < np->rx_target) + np->rx_target = target; + + network_alloc_rx_buffers(netdev); + + spin_unlock(&np->rx_lock); + return len; +} + +static ssize_t show_rxbuf_cur(struct class_device *cd, char *buf) +{ + struct net_device *netdev = container_of(cd, struct net_device, class_dev); + struct netfront_info *info = netdev_priv(netdev); + + return sprintf(buf, "%u\n", info->rx_target); +} + +static const struct class_device_attribute xennet_attrs[] = { + __ATTR(rxbuf_min, S_IRUGO|S_IWUSR, show_rxbuf_min, store_rxbuf_min), + __ATTR(rxbuf_max, S_IRUGO|S_IWUSR, show_rxbuf_max, store_rxbuf_max), + __ATTR(rxbuf_cur, S_IRUGO, show_rxbuf_cur, NULL), +}; + +static int xennet_sysfs_addif(struct net_device *netdev) +{ + int i; + int error = 0; + + for (i = 0; i < ARRAY_SIZE(xennet_attrs); i++) { + error = class_device_create_file(&netdev->class_dev, + &xennet_attrs[i]); + if (error) + goto Err; + } + return 0; + Err: + while (--i >= 0) + class_device_remove_file(&netdev->class_dev, + &xennet_attrs[i]); + return error; +} + +static void xennet_sysfs_delif(struct net_device *netdev) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(xennet_attrs); i++) { + class_device_remove_file(&netdev->class_dev, + &xennet_attrs[i]); + } +} + +#else +#define xennet_sysfs_addif(dev) (0) +#define xennet_sysfs_delif(dev) do { } while(0) +#endif + + /* * Nothing to do here. Virtual interface is point-to-point and the * physical interface is probably promiscuous anyway. @@ -1176,11 +1308,19 @@ SET_MODULE_OWNER(netdev); SET_NETDEV_DEV(netdev, &dev->dev); - if ((err = register_netdev(netdev)) != 0) { + err = register_netdev(netdev); + if (err) { printk(KERN_WARNING "%s> register_netdev err=%d\n", __FUNCTION__, err); goto exit_free_grefs; } + + err = xennet_sysfs_addif(netdev); + if (err) { + /* This can be non fatal only means no tuning parameters */ + printk(KERN_WARNING "%s> add sysfs failed err=%d\n", __FUNCTION__, err); + } + np->netdev = netdev; if (val) @@ -1258,6 +1398,7 @@ del_timer_sync(&info->rx_refill_timer); + xennet_sysfs_delif(info->netdev); unregister_netdev(info->netdev); } @@ -1327,8 +1468,6 @@ static int __init netif_init(void) { - int err = 0; - if (xen_start_info->flags & SIF_INITDOMAIN) return 0; _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 17 May 2006, at 22:26, Stephen Hemminger wrote:> These patches do some of the cleanup''s for netfront necessary to get > it more > acceptable for mainline. They build, but HAVE NOT BEEN TESTED; I am > new to this > and don''t have the time or setup to do much with Xen.Many thanks for these, especially the sysfsification! -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Chris Wright
2006-May-18 08:26 UTC
Re: [Xen-devel] [PATCH 5/6] netfront: remove /proc interface
Thanks Steve, I had similar in the queue, with only this small interdiff. --- linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c.shemminger 2006-05-18 04:18:03.000000000 -0400 +++ linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c 2006-05-18 04:18:32.000000000 -0400 @@ -43,7 +43,6 @@ #include <linux/skbuff.h> #include <linux/init.h> #include <linux/bitops.h> -#include <linux/proc_fs.h> #include <linux/ethtool.h> #include <linux/in.h> #include <net/sock.h> @@ -1327,8 +1326,6 @@ static struct notifier_block notifier_in static int __init netif_init(void) { - int err = 0; - if (xen_start_info->flags & SIF_INITDOMAIN) return 0; _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stephen Hemminger
2006-May-18 18:18 UTC
Re: [Xen-devel] [PATCH 5/6] netfront: remove /proc interface
On Thu, 18 May 2006 01:26:39 -0700 Chris Wright <chrisw@sous-sol.org> wrote:> Thanks Steve, I had similar in the queue, with only this small > interdiff. > > --- linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c.shemminger 2006-05-18 04:18:03.000000000 -0400 > +++ linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c 2006-05-18 04:18:32.000000000 -0400 > @@ -43,7 +43,6 @@ > #include <linux/skbuff.h> > #include <linux/init.h> > #include <linux/bitops.h> > -#include <linux/proc_fs.h> > #include <linux/ethtool.h> > #include <linux/in.h> > #include <net/sock.h> > @@ -1327,8 +1326,6 @@ static struct notifier_block notifier_in > > static int __init netif_init(void) > { > - int err = 0; > - > if (xen_start_info->flags & SIF_INITDOMAIN) > return 0; >Yeah, the header can go as well. I think I zapped the ''int err'' in the sysfs patch instead. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel