[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <17893.46781.170641.654215@robur.slu.se>
Date: Wed, 28 Feb 2007 18:07:09 +0100
From: Robert Olsson <Robert.Olsson@...a.slu.se>
To: Stephen Hemminger <shemminger@...ux-foundation.org>
Cc: Robert Olsson <Robert.Olsson@...a.slu.se>,
"David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Subject: [PATCH 4/4] pktgen: fix device name handling
Yes it seems be handle dev name change. So configuration scripts should
use ifindex now :)
Signed-off-by: Robert Olsson <robert.olsson@....uu.se>
Cheers.
--ro
Stephen Hemminger writes:
> Since devices can change name and other wierdness, don't hold onto
> a copy of device name, instead use pointer to output device.
>
> Fix a couple of leaks in error handling path as well.
>
> Signed-off-by: Stephen Hemminger <shemminger@...ux-foundation.org>
>
> ---
> net/core/pktgen.c | 137 +++++++++++++++++++++++++++---------------------------
> 1 file changed, 70 insertions(+), 67 deletions(-)
>
> --- pktgen.orig/net/core/pktgen.c 2007-02-27 12:08:58.000000000 -0800
> +++ pktgen/net/core/pktgen.c 2007-02-27 12:11:32.000000000 -0800
> @@ -210,15 +210,11 @@
> };
>
> struct pktgen_dev {
> -
> /*
> * Try to keep frequent/infrequent used vars. separated.
> */
> -
> - char ifname[IFNAMSIZ];
> - char result[512];
> -
> - struct pktgen_thread *pg_thread; /* the owner */
> + struct proc_dir_entry *entry; /* proc file */
> + struct pktgen_thread *pg_thread;/* the owner */
> struct list_head list; /* Used for chaining in the thread's run-queue */
>
> int running; /* if this changes to false, the test will stop */
> @@ -345,6 +341,8 @@
> unsigned cflows; /* Concurrent flows (config) */
> unsigned lflow; /* Flow length (config) */
> unsigned nflows; /* accumulated flows (stats) */
> +
> + char result[512];
> };
>
> struct pktgen_hdr {
> @@ -497,7 +495,7 @@
> static int pktgen_stop_device(struct pktgen_dev *pkt_dev);
> static void pktgen_stop(struct pktgen_thread *t);
> static void pktgen_clear_counters(struct pktgen_dev *pkt_dev);
> -static int pktgen_mark_device(const char *ifname);
> +
> static unsigned int scan_ip6(const char *s, char ip[16]);
> static unsigned int fmt_ip6(char *s, const char ip[16]);
>
> @@ -591,7 +589,7 @@
> " frags: %d delay: %u clone_skb: %d ifname: %s\n",
> pkt_dev->nfrags,
> 1000 * pkt_dev->delay_us + pkt_dev->delay_ns,
> - pkt_dev->clone_skb, pkt_dev->ifname);
> + pkt_dev->clone_skb, pkt_dev->odev->name);
>
> seq_printf(seq, " flows: %u flowlen: %u\n", pkt_dev->cflows,
> pkt_dev->lflow);
> @@ -1682,13 +1680,13 @@
> if_lock(t);
> list_for_each_entry(pkt_dev, &t->if_list, list)
> if (pkt_dev->running)
> - seq_printf(seq, "%s ", pkt_dev->ifname);
> + seq_printf(seq, "%s ", pkt_dev->odev->name);
>
> seq_printf(seq, "\nStopped: ");
>
> list_for_each_entry(pkt_dev, &t->if_list, list)
> if (!pkt_dev->running)
> - seq_printf(seq, "%s ", pkt_dev->ifname);
> + seq_printf(seq, "%s ", pkt_dev->odev->name);
>
> if (t->result[0])
> seq_printf(seq, "\nResult: %s\n", t->result);
> @@ -1834,12 +1832,11 @@
> /*
> * mark a device for removal
> */
> -static int pktgen_mark_device(const char *ifname)
> +static void pktgen_mark_device(const char *ifname)
> {
> struct pktgen_dev *pkt_dev = NULL;
> const int max_tries = 10, msec_per_try = 125;
> int i = 0;
> - int ret = 0;
>
> mutex_lock(&pktgen_thread_lock);
> pr_debug("pktgen: pktgen_mark_device marking %s for removal\n", ifname);
> @@ -1860,32 +1857,49 @@
> printk("pktgen_mark_device: timed out after waiting "
> "%d msec for device %s to be removed\n",
> msec_per_try * i, ifname);
> - ret = 1;
> break;
> }
>
> }
>
> mutex_unlock(&pktgen_thread_lock);
> +}
>
> - return ret;
> +static void pktgen_change_name(struct net_device *dev)
> +{
> + struct pktgen_thread *t;
> +
> + list_for_each_entry(t, &pktgen_threads, th_list) {
> + struct pktgen_dev *pkt_dev;
> +
> + list_for_each_entry(pkt_dev, &t->if_list, list) {
> + if (pkt_dev->odev != dev)
> + continue;
> +
> + remove_proc_entry(pkt_dev->entry->name, pg_proc_dir);
> +
> + pkt_dev->entry = create_proc_entry(dev->name, 0600,
> + pg_proc_dir);
> + if (!pkt_dev->entry)
> + printk(KERN_ERR "pktgen: can't move proc "
> + " entry for '%s'\n", dev->name);
> + break;
> + }
> + }
> }
>
> static int pktgen_device_event(struct notifier_block *unused,
> unsigned long event, void *ptr)
> {
> - struct net_device *dev = (struct net_device *)(ptr);
> + struct net_device *dev = ptr;
>
> /* It is OK that we do not hold the group lock right now,
> * as we run under the RTNL lock.
> */
>
> switch (event) {
> - case NETDEV_CHANGEADDR:
> - case NETDEV_GOING_DOWN:
> - case NETDEV_DOWN:
> - case NETDEV_UP:
> - /* Ignore for now */
> + case NETDEV_CHANGENAME:
> + pktgen_change_name(dev);
> break;
>
> case NETDEV_UNREGISTER:
> @@ -1898,41 +1912,36 @@
>
> /* Associate pktgen_dev with a device. */
>
> -static struct net_device *pktgen_setup_dev(struct pktgen_dev *pkt_dev)
> +static int pktgen_setup_dev(struct pktgen_dev *pkt_dev, const char *ifname)
> {
> struct net_device *odev;
> + int err;
>
> /* Clean old setups */
> -
> if (pkt_dev->odev) {
> dev_put(pkt_dev->odev);
> pkt_dev->odev = NULL;
> }
>
> - odev = dev_get_by_name(pkt_dev->ifname);
> -
> + odev = dev_get_by_name(ifname);
> if (!odev) {
> - printk("pktgen: no such netdevice: \"%s\"\n", pkt_dev->ifname);
> - goto out;
> + printk("pktgen: no such netdevice: \"%s\"\n", ifname);
> + return -ENODEV;
> }
> +
> if (odev->type != ARPHRD_ETHER) {
> - printk("pktgen: not an ethernet device: \"%s\"\n",
> - pkt_dev->ifname);
> - goto out_put;
> - }
> - if (!netif_running(odev)) {
> - printk("pktgen: device is down: \"%s\"\n", pkt_dev->ifname);
> - goto out_put;
> + printk("pktgen: not an ethernet device: \"%s\"\n", ifname);
> + err = -EINVAL;
> + } else if (!netif_running(odev)) {
> + printk("pktgen: device is down: \"%s\"\n", ifname);
> + err = -ENETDOWN;
> + } else {
> + pkt_dev->odev = odev;
> + return 0;
> }
> - pkt_dev->odev = odev;
> -
> - return pkt_dev->odev;
>
> -out_put:
> dev_put(odev);
> -out:
> - return NULL;
> -
> + return err;
> }
>
> /* Read pkt_dev from the interface and set up internal pktgen_dev
> @@ -1940,10 +1949,6 @@
> */
> static void pktgen_setup_inject(struct pktgen_dev *pkt_dev)
> {
> - /* Try once more, just in case it works now. */
> - if (!pkt_dev->odev)
> - pktgen_setup_dev(pkt_dev);
> -
> if (!pkt_dev->odev) {
> printk("pktgen: ERROR: pkt_dev->odev == NULL in setup_inject.\n");
> sprintf(pkt_dev->result,
> @@ -2987,7 +2992,7 @@
>
> if (!pkt_dev->running) {
> printk("pktgen: interface: %s is already stopped\n",
> - pkt_dev->ifname);
> + pkt_dev->odev->name);
> return -EINVAL;
> }
>
> @@ -3337,7 +3342,7 @@
> if_lock(t);
>
> list_for_each_entry(p, &t->if_list, list)
> - if (strncmp(p->ifname, ifname, IFNAMSIZ) == 0) {
> + if (strncmp(p->odev->name, ifname, IFNAMSIZ) == 0) {
> pkt_dev = p;
> break;
> }
> @@ -3378,7 +3383,7 @@
> static int pktgen_add_device(struct pktgen_thread *t, const char *ifname)
> {
> struct pktgen_dev *pkt_dev;
> - struct proc_dir_entry *pe;
> + int err;
>
> /* We don't allow a device to be on several threads */
>
> @@ -3420,29 +3425,28 @@
> pkt_dev->svlan_cfi = 0;
> pkt_dev->svlan_id = 0xffff;
>
> - strncpy(pkt_dev->ifname, ifname, IFNAMSIZ);
> + err = pktgen_setup_dev(pkt_dev, ifname);
> + if (err)
> + goto out1;
>
> - if (!pktgen_setup_dev(pkt_dev)) {
> - printk("pktgen: ERROR: pktgen_setup_dev failed.\n");
> - if (pkt_dev->flows)
> - vfree(pkt_dev->flows);
> - kfree(pkt_dev);
> - return -ENODEV;
> - }
> -
> - pe = create_proc_entry(ifname, 0600, pg_proc_dir);
> - if (!pe) {
> + pkt_dev->entry = create_proc_entry(ifname, 0600, pg_proc_dir);
> + if (!pkt_dev->entry) {
> printk("pktgen: cannot create %s/%s procfs entry.\n",
> PG_PROC_DIR, ifname);
> - if (pkt_dev->flows)
> - vfree(pkt_dev->flows);
> - kfree(pkt_dev);
> - return -EINVAL;
> + err = -EINVAL;
> + goto out2;
> }
> - pe->proc_fops = &pktgen_if_fops;
> - pe->data = pkt_dev;
> + pkt_dev->entry->proc_fops = &pktgen_if_fops;
> + pkt_dev->entry->data = pkt_dev;
>
> return add_dev_to_thread(t, pkt_dev);
> +out2:
> + dev_put(pkt_dev->odev);
> +out1:
> + if (pkt_dev->flows)
> + vfree(pkt_dev->flows);
> + kfree(pkt_dev);
> + return err;
> }
>
> static int __init pktgen_create_thread(int cpu)
> @@ -3530,9 +3534,8 @@
>
> _rem_dev_from_if_list(t, pkt_dev);
>
> - /* Clean up proc file system */
> -
> - remove_proc_entry(pkt_dev->ifname, pg_proc_dir);
> + if (pkt_dev->entry)
> + remove_proc_entry(pkt_dev->entry->name, pg_proc_dir);
>
> if (pkt_dev->flows)
> vfree(pkt_dev->flows);
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists