From 40e9b05b56c3b41887bb6dd3a1143a1a735a8b4d Mon Sep 17 00:00:00 2001 From: Keir Fraser Date: Wed, 19 Dec 2007 14:47:41 +0000 Subject: [PATCH] net accel: Allow network accelerators to deal with hot-plug/unplug on physical devices. Add watch for acceleration configuration in frontend. Signed-off-by: Kieran Mansley --- drivers/xen/netback/accel.c | 104 ++++++------ drivers/xen/netback/common.h | 2 +- drivers/xen/netfront/accel.c | 284 +++++++++++++++++++++----------- drivers/xen/netfront/netfront.c | 20 +-- drivers/xen/netfront/netfront.h | 26 +-- 5 files changed, 262 insertions(+), 174 deletions(-) diff --git a/drivers/xen/netback/accel.c b/drivers/xen/netback/accel.c index 831e7599..181c3632 100644 --- a/drivers/xen/netback/accel.c +++ b/drivers/xen/netback/accel.c @@ -76,12 +76,27 @@ static int match_accelerator(struct xenbus_device *xendev, } } + +static void do_probe(struct backend_info *be, + struct netback_accelerator *accelerator, + struct xenbus_device *xendev) +{ + be->accelerator = accelerator; + atomic_inc(&be->accelerator->use_count); + if (be->accelerator->hooks->probe(xendev) != 0) { + atomic_dec(&be->accelerator->use_count); + module_put(be->accelerator->hooks->owner); + be->accelerator = NULL; + } +} + + /* - * Notify all suitable backends that a new accelerator is available - * and connected. This will also notify the accelerator plugin module + * Notify suitable backends that a new accelerator is available and + * connected. This will also notify the accelerator plugin module * that it is being used for a device through the probe hook. */ -static int netback_accelerator_tell_backend(struct device *dev, void *arg) +static int netback_accelerator_probe_backend(struct device *dev, void *arg) { struct netback_accelerator *accelerator = (struct netback_accelerator *)arg; @@ -90,16 +105,39 @@ static int netback_accelerator_tell_backend(struct device *dev, void *arg) if (!strcmp("vif", xendev->devicetype)) { struct backend_info *be = xendev->dev.driver_data; - if (match_accelerator(xendev, be, accelerator)) { - be->accelerator = accelerator; - atomic_inc(&be->accelerator->use_count); - be->accelerator->hooks->probe(xendev); + if (match_accelerator(xendev, be, accelerator) && + try_module_get(accelerator->hooks->owner)) { + do_probe(be, accelerator, xendev); } } return 0; } +/* + * Notify suitable backends that an accelerator is unavailable. + */ +static int netback_accelerator_remove_backend(struct device *dev, void *arg) +{ + struct xenbus_device *xendev = to_xenbus_device(dev); + struct netback_accelerator *accelerator = + (struct netback_accelerator *)arg; + + if (!strcmp("vif", xendev->devicetype)) { + struct backend_info *be = xendev->dev.driver_data; + + if (be->accelerator == accelerator) { + be->accelerator->hooks->remove(xendev); + atomic_dec(&be->accelerator->use_count); + module_put(be->accelerator->hooks->owner); + be->accelerator = NULL; + } + } + return 0; +} + + + /* * Entry point for an netback accelerator plugin module. Called to * advertise its presence, and connect to any suitable backends. @@ -133,7 +171,6 @@ int netback_connect_accelerator(unsigned version, int id, const char *eth_name, return -ENOMEM; } - new_accelerator->id = id; eth_name_len = strlen(eth_name)+1; @@ -152,11 +189,12 @@ int netback_connect_accelerator(unsigned version, int id, const char *eth_name, mutex_lock(&accelerators_mutex); list_add(&new_accelerator->link, &accelerators_list); - mutex_unlock(&accelerators_mutex); /* tell existing backends about new plugin */ xenbus_for_each_backend(new_accelerator, - netback_accelerator_tell_backend); + netback_accelerator_probe_backend); + + mutex_unlock(&accelerators_mutex); return 0; @@ -164,32 +202,9 @@ int netback_connect_accelerator(unsigned version, int id, const char *eth_name, EXPORT_SYMBOL_GPL(netback_connect_accelerator); -/* - * Remove the link from backend state to a particular accelerator - */ -static int netback_accelerator_cleanup_backend(struct device *dev, void *arg) -{ - struct netback_accelerator *accelerator = - (struct netback_accelerator *)arg; - struct xenbus_device *xendev = to_xenbus_device(dev); - - if (!strcmp("vif", xendev->devicetype)) { - struct backend_info *be = xendev->dev.driver_data; - if (be->accelerator == accelerator) - be->accelerator = NULL; - } - return 0; -} - - /* * Disconnect an accelerator plugin module that has previously been * connected. - * - * This should only be allowed when there are no remaining users - - * i.e. it is not necessary to go through and clear all the hooks, as - * they should have already been removed. This is enforced by taking - * a module reference to the plugin while the interfaces are in use */ void netback_disconnect_accelerator(int id, const char *eth_name) { @@ -197,17 +212,14 @@ void netback_disconnect_accelerator(int id, const char *eth_name) mutex_lock(&accelerators_mutex); list_for_each_entry_safe(accelerator, next, &accelerators_list, link) { - if (strcmp(eth_name, accelerator->eth_name)) { + if (!strcmp(eth_name, accelerator->eth_name)) { + xenbus_for_each_backend + (accelerator, netback_accelerator_remove_backend); BUG_ON(atomic_read(&accelerator->use_count) != 0); - list_del(&accelerator->link); - mutex_unlock(&accelerators_mutex); - - xenbus_for_each_backend(accelerator, - netback_accelerator_cleanup_backend); - + list_del(&accelerator->link); kfree(accelerator->eth_name); kfree(accelerator); - return; + break; } } mutex_unlock(&accelerators_mutex); @@ -228,9 +240,7 @@ void netback_probe_accelerators(struct backend_info *be, list_for_each_entry(accelerator, &accelerators_list, link) { if (match_accelerator(dev, be, accelerator) && try_module_get(accelerator->hooks->owner)) { - be->accelerator = accelerator; - atomic_inc(&be->accelerator->use_count); - be->accelerator->hooks->probe(dev); + do_probe(be, accelerator, dev); break; } } @@ -241,13 +251,15 @@ void netback_probe_accelerators(struct backend_info *be, void netback_remove_accelerators(struct backend_info *be, struct xenbus_device *dev) { + mutex_lock(&accelerators_mutex); /* Notify the accelerator (if any) of this device's removal */ - if (be->accelerator) { + if (be->accelerator != NULL) { be->accelerator->hooks->remove(dev); atomic_dec(&be->accelerator->use_count); module_put(be->accelerator->hooks->owner); + be->accelerator = NULL; } - be->accelerator = NULL; + mutex_unlock(&accelerators_mutex); } diff --git a/drivers/xen/netback/common.h b/drivers/xen/netback/common.h index 784b6287..10a1d58f 100644 --- a/drivers/xen/netback/common.h +++ b/drivers/xen/netback/common.h @@ -150,7 +150,7 @@ struct backend_info { struct netback_accelerator *accelerator; }; -#define NETBACK_ACCEL_VERSION 0x00010000 +#define NETBACK_ACCEL_VERSION 0x00010001 /* * Connect an accelerator plugin module to netback. Returns zero on diff --git a/drivers/xen/netfront/accel.c b/drivers/xen/netfront/accel.c index 534dce9d..78491c69 100644 --- a/drivers/xen/netfront/accel.c +++ b/drivers/xen/netfront/accel.c @@ -45,6 +45,12 @@ #define WPRINTK(fmt, args...) \ printk(KERN_WARNING "netfront/accel: " fmt, ##args) +static int netfront_remove_accelerator(struct netfront_info *np, + struct xenbus_device *dev); +static int netfront_load_accelerator(struct netfront_info *np, + struct xenbus_device *dev, + const char *frontend); + /* * List of all netfront accelerator plugin modules available. Each * list entry is of type struct netfront_accelerator. @@ -82,6 +88,120 @@ void netif_exit_accel(void) } +/* + * Watch the configured accelerator and change plugin if it's modified + */ +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,20) +static void accel_watch_work(struct work_struct *context) +#else +static void accel_watch_work(void *context) +#endif +{ +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,20) + struct netfront_accel_vif_state *vif_state = + container_of(context, struct netfront_accel_vif_state, + accel_work); +#else + struct netfront_accel_vif_state *vif_state = + (struct netfront_accel_vif_state *)context; +#endif + struct netfront_info *np = vif_state->np; + char *accel_frontend; + int accel_len, rc = -1; + + mutex_lock(&accelerator_mutex); + + accel_frontend = xenbus_read(XBT_NIL, np->xbdev->otherend, + "accel-frontend", &accel_len); + if (IS_ERR(accel_frontend)) { + accel_frontend = NULL; + netfront_remove_accelerator(np, np->xbdev); + } else { + /* If this is the first time, request the accelerator, + otherwise only request one if it has changed */ + if (vif_state->accel_frontend == NULL) { + rc = netfront_load_accelerator(np, np->xbdev, + accel_frontend); + } else { + if (strncmp(vif_state->accel_frontend, accel_frontend, + accel_len)) { + netfront_remove_accelerator(np, np->xbdev); + rc = netfront_load_accelerator(np, np->xbdev, + accel_frontend); + } + } + } + + /* Get rid of previous state and replace with the new name */ + if (vif_state->accel_frontend != NULL) + kfree(vif_state->accel_frontend); + vif_state->accel_frontend = accel_frontend; + + mutex_unlock(&accelerator_mutex); + + if (rc == 0) { + DPRINTK("requesting module %s\n", accel_frontend); + request_module("%s", accel_frontend); + /* + * Module should now call netfront_accelerator_loaded() once + * it's up and running, and we can continue from there + */ + } +} + + +static void accel_watch_changed(struct xenbus_watch *watch, + const char **vec, unsigned int len) +{ + struct netfront_accel_vif_state *vif_state = + container_of(watch, struct netfront_accel_vif_state, + accel_watch); + schedule_work(&vif_state->accel_work); +} + + +void netfront_accelerator_add_watch(struct netfront_info *np) +{ + int err; + + /* Check we're not trying to overwrite an existing watch */ + BUG_ON(np->accel_vif_state.accel_watch.node != NULL); + + /* Get a watch on the accelerator plugin */ + err = xenbus_watch_path2(np->xbdev, np->xbdev->otherend, + "accel-frontend", + &np->accel_vif_state.accel_watch, + accel_watch_changed); + if (err) { + DPRINTK("%s: Failed to register accel watch: %d\n", + __FUNCTION__, err); + np->accel_vif_state.accel_watch.node = NULL; + } +} + + +static +void netfront_accelerator_remove_watch(struct netfront_info *np) +{ + struct netfront_accel_vif_state *vif_state = &np->accel_vif_state; + + /* Get rid of watch on accelerator plugin */ + if (vif_state->accel_watch.node != NULL) { + unregister_xenbus_watch(&vif_state->accel_watch); + kfree(vif_state->accel_watch.node); + vif_state->accel_watch.node = NULL; + + flush_scheduled_work(); + + /* Clean up any state left from watch */ + if (vif_state->accel_frontend != NULL) { + kfree(vif_state->accel_frontend); + vif_state->accel_frontend = NULL; + } + } +} + + /* * Initialise the accel_vif_state field in the netfront state */ @@ -93,6 +213,13 @@ void init_accelerator_vif(struct netfront_info *np, /* It's assumed that these things don't change */ np->accel_vif_state.np = np; np->accel_vif_state.dev = dev; + +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,20) + INIT_WORK(&np->accel_vif_state.accel_work, accel_watch_work); +#else + INIT_WORK(&np->accel_vif_state.accel_work, accel_watch_work, + &np->accel_vif_state); +#endif } @@ -202,6 +329,7 @@ static void accelerator_probe_new_vif(struct netfront_info *np, struct netfront_accelerator *accelerator) { struct netfront_accel_hooks *hooks; + unsigned long flags; DPRINTK("\n"); @@ -211,24 +339,29 @@ static void accelerator_probe_new_vif(struct netfront_info *np, hooks = accelerator->hooks; if (hooks) { - hooks->new_device(np->netdev, dev); - /* - * Hooks will get linked into vif_state by a future - * call by the accelerator to netfront_accelerator_ready() - */ + if (hooks->new_device(np->netdev, dev) == 0) { + spin_lock_irqsave + (&accelerator->vif_states_lock, flags); + + accelerator_set_vif_state_hooks(&np->accel_vif_state); + + spin_unlock_irqrestore + (&accelerator->vif_states_lock, flags); + } } return; } + /* * Request that a particular netfront accelerator plugin is loaded. * Usually called as a result of the vif configuration specifying - * which one to use. + * which one to use. Must be called with accelerator_mutex held */ -int netfront_load_accelerator(struct netfront_info *np, - struct xenbus_device *dev, - const char *frontend) +static int netfront_load_accelerator(struct netfront_info *np, + struct xenbus_device *dev, + const char *frontend) { struct netfront_accelerator *accelerator; int rc = 0; @@ -236,8 +369,6 @@ int netfront_load_accelerator(struct netfront_info *np, DPRINTK(" %s\n", frontend); - mutex_lock(&accelerator_mutex); - spin_lock_irqsave(&accelerators_lock, flags); /* @@ -249,8 +380,6 @@ int netfront_load_accelerator(struct netfront_info *np, spin_unlock_irqrestore(&accelerators_lock, flags); accelerator_probe_new_vif(np, dev, accelerator); - - mutex_unlock(&accelerator_mutex); return 0; } } @@ -258,7 +387,6 @@ int netfront_load_accelerator(struct netfront_info *np, /* Couldn't find it, so create a new one and load the module */ if ((rc = init_accelerator(frontend, &accelerator, NULL)) < 0) { spin_unlock_irqrestore(&accelerators_lock, flags); - mutex_unlock(&accelerator_mutex); return rc; } @@ -267,18 +395,6 @@ int netfront_load_accelerator(struct netfront_info *np, /* Include this frontend device on the accelerator's list */ add_accelerator_vif(accelerator, np); - mutex_unlock(&accelerator_mutex); - - DPRINTK("requesting module %s\n", frontend); - - /* load module */ - request_module("%s", frontend); - - /* - * Module should now call netfront_accelerator_loaded() once - * it's up and running, and we can continue from there - */ - return rc; } @@ -294,6 +410,7 @@ accelerator_probe_vifs(struct netfront_accelerator *accelerator, struct netfront_accel_hooks *hooks) { struct netfront_accel_vif_state *vif_state, *tmp; + unsigned long flags; DPRINTK("%p\n", accelerator); @@ -313,13 +430,15 @@ accelerator_probe_vifs(struct netfront_accelerator *accelerator, link) { struct netfront_info *np = vif_state->np; - hooks->new_device(np->netdev, vif_state->dev); - - /* - * Hooks will get linked into vif_state by a call to - * netfront_accelerator_ready() once accelerator - * plugin is ready for action - */ + if (hooks->new_device(np->netdev, vif_state->dev) == 0) { + spin_lock_irqsave + (&accelerator->vif_states_lock, flags); + + accelerator_set_vif_state_hooks(vif_state); + + spin_unlock_irqrestore + (&accelerator->vif_states_lock, flags); + } } } @@ -384,48 +503,6 @@ int netfront_accelerator_loaded(int version, const char *frontend, EXPORT_SYMBOL_GPL(netfront_accelerator_loaded); -/* - * Called by the accelerator module after it has been probed with a - * network device to say that it is ready to start accelerating - * traffic on that device - */ -void netfront_accelerator_ready(const char *frontend, - struct xenbus_device *dev) -{ - struct netfront_accelerator *accelerator; - struct netfront_accel_vif_state *accel_vif_state; - unsigned long flags, flags1; - - DPRINTK("%s %p\n", frontend, dev); - - spin_lock_irqsave(&accelerators_lock, flags); - - list_for_each_entry(accelerator, &accelerators_list, link) { - if (match_accelerator(frontend, accelerator)) { - /* - * Mutex not held so need vif_states_lock for - * list - */ - spin_lock_irqsave - (&accelerator->vif_states_lock, flags1); - - list_for_each_entry(accel_vif_state, - &accelerator->vif_states, link) { - if (accel_vif_state->dev == dev) - accelerator_set_vif_state_hooks - (accel_vif_state); - } - - spin_unlock_irqrestore - (&accelerator->vif_states_lock, flags1); - break; - } - } - spin_unlock_irqrestore(&accelerators_lock, flags); -} -EXPORT_SYMBOL_GPL(netfront_accelerator_ready); - - /* * Remove the hooks from a single vif state. */ @@ -467,16 +544,21 @@ static void accelerator_remove_hooks(struct netfront_accelerator *accelerator) if(vif_state->hooks) { hooks = vif_state->hooks; - accelerator_remove_single_hook(accelerator, vif_state); /* Last chance to get statistics from the accelerator */ hooks->get_stats(vif_state->np->netdev, &vif_state->np->stats); - } - spin_unlock_irqrestore(&accelerator->vif_states_lock, flags); + spin_unlock_irqrestore(&accelerator->vif_states_lock, + flags); + + accelerator_remove_single_hook(accelerator, vif_state); - accelerator->hooks->remove(vif_state->dev); + accelerator->hooks->remove(vif_state->dev); + } else { + spin_unlock_irqrestore(&accelerator->vif_states_lock, + flags); + } } accelerator->hooks = NULL; @@ -523,6 +605,12 @@ static int do_remove(struct netfront_info *np, struct xenbus_device *dev, if (np->accel_vif_state.hooks) { hooks = np->accel_vif_state.hooks; + /* Last chance to get statistics from the accelerator */ + hooks->get_stats(np->netdev, &np->stats); + + spin_unlock_irqrestore(&accelerator->vif_states_lock, + *lock_flags); + /* * Try and do the opposite of accelerator_probe_new_vif * to ensure there's no state pointing back at the @@ -531,14 +619,6 @@ static int do_remove(struct netfront_info *np, struct xenbus_device *dev, accelerator_remove_single_hook(accelerator, &np->accel_vif_state); - /* Last chance to get statistics from the accelerator */ - hooks->get_stats(np->netdev, &np->stats); - } - - if (accelerator->hooks) { - spin_unlock_irqrestore(&accelerator->vif_states_lock, - *lock_flags); - rc = accelerator->hooks->remove(dev); spin_lock_irqsave(&accelerator->vif_states_lock, *lock_flags); @@ -546,21 +626,19 @@ static int do_remove(struct netfront_info *np, struct xenbus_device *dev, return rc; } - - -int netfront_accelerator_call_remove(struct netfront_info *np, - struct xenbus_device *dev) + + +static int netfront_remove_accelerator(struct netfront_info *np, + struct xenbus_device *dev) { struct netfront_accelerator *accelerator; struct netfront_accel_vif_state *tmp_vif_state; unsigned long flags; int rc = 0; - mutex_lock(&accelerator_mutex); - /* Check that we've got a device that was accelerated */ if (np->accelerator == NULL) - goto out; + return rc; accelerator = np->accelerator; @@ -579,17 +657,30 @@ int netfront_accelerator_call_remove(struct netfront_info *np, np->accelerator = NULL; spin_unlock_irqrestore(&accelerator->vif_states_lock, flags); - out: + + return rc; +} + + +int netfront_accelerator_call_remove(struct netfront_info *np, + struct xenbus_device *dev) +{ + int rc; + netfront_accelerator_remove_watch(np); + mutex_lock(&accelerator_mutex); + rc = netfront_remove_accelerator(np, dev); mutex_unlock(&accelerator_mutex); return rc; } - + int netfront_accelerator_suspend(struct netfront_info *np, struct xenbus_device *dev) { unsigned long flags; int rc = 0; + + netfront_accelerator_remove_watch(np); mutex_lock(&accelerator_mutex); @@ -641,6 +732,7 @@ int netfront_accelerator_suspend_cancel(struct netfront_info *np, out: mutex_unlock(&accelerator_mutex); + netfront_accelerator_add_watch(np); return 0; } diff --git a/drivers/xen/netfront/netfront.c b/drivers/xen/netfront/netfront.c index f76cc10a..59155c74 100644 --- a/drivers/xen/netfront/netfront.c +++ b/drivers/xen/netfront/netfront.c @@ -378,6 +378,10 @@ static int talk_to_backend(struct xenbus_device *dev, if (err) goto out; + /* This will load an accelerator if one is configured when the + * watch fires */ + netfront_accelerator_add_watch(info); + again: err = xenbus_transaction_start(&xbt); if (err) { @@ -452,6 +456,7 @@ again: xenbus_transaction_end(xbt, 1); xenbus_dev_fatal(dev, err, "%s", message); destroy_ring: + netfront_accelerator_call_remove(info, dev); netif_disconnect_backend(info); out: return err; @@ -1746,9 +1751,7 @@ static int network_connect(struct net_device *dev) struct sk_buff *skb; grant_ref_t ref; netif_rx_request_t *req; - unsigned int feature_rx_copy, feature_rx_flip, feature_accel; - char *accel_frontend; - int accel_len; + unsigned int feature_rx_copy, feature_rx_flip; err = xenbus_scanf(XBT_NIL, np->xbdev->otherend, "feature-rx-copy", "%u", &feature_rx_copy); @@ -1759,12 +1762,6 @@ static int network_connect(struct net_device *dev) if (err != 1) feature_rx_flip = 1; - feature_accel = 1; - accel_frontend = xenbus_read(XBT_NIL, np->xbdev->otherend, - "accel-frontend", &accel_len); - if (IS_ERR(accel_frontend)) - feature_accel = 0; - /* * Copy packets on receive path if: * (a) This was requested by user, and the backend supports it; or @@ -1777,11 +1774,6 @@ static int network_connect(struct net_device *dev) if (err) return err; - if (feature_accel) { - netfront_load_accelerator(np, np->xbdev, accel_frontend); - kfree(accel_frontend); - } - xennet_set_features(dev); DPRINTK("device %s has %sing receive path.\n", diff --git a/drivers/xen/netfront/netfront.h b/drivers/xen/netfront/netfront.h index 139149ab..72d0183e 100644 --- a/drivers/xen/netfront/netfront.h +++ b/drivers/xen/netfront/netfront.h @@ -95,7 +95,7 @@ struct netfront_accel_hooks { /* Version of API/protocol for communication between netfront and acceleration plugin supported */ -#define NETFRONT_ACCEL_VERSION 0x00010002 +#define NETFRONT_ACCEL_VERSION 0x00010003 /* * Per-netfront device state for the accelerator. This is used to @@ -108,6 +108,13 @@ struct netfront_accel_vif_state { struct xenbus_device *dev; struct netfront_info *np; struct netfront_accel_hooks *hooks; + + /* Watch on the accelerator configuration value */ + struct xenbus_watch accel_watch; + /* Work item to process change in accelerator */ + struct work_struct accel_work; + /* The string from xenbus last time accel_watch fired */ + char *accel_frontend; }; /* @@ -209,18 +216,6 @@ struct netfront_info { extern int netfront_accelerator_loaded(int version, const char *frontend, struct netfront_accel_hooks *hooks); -/* - * Called when an accelerator plugin is ready to accelerate a device * - * that has been passed to it from netfront using the "new_device" - * hook. - * - * frontend: the string describing the accelerator. Must match the - * one passed to netfront_accelerator_loaded() - * dev: the xenbus device the plugin was asked to accelerate - */ -extern void netfront_accelerator_ready(const char *frontend, - struct xenbus_device *dev); - /* * Called by an accelerator plugin module when it is about to unload. * @@ -237,7 +232,6 @@ extern void netfront_accelerator_stop(const char *frontend); extern int netfront_check_queue_ready(struct net_device *net_dev); - /* Internal-to-netfront Functions */ /* @@ -267,9 +261,7 @@ extern int netfront_accelerator_call_get_stats(struct netfront_info *np, struct net_device *dev); extern -int netfront_load_accelerator(struct netfront_info *np, - struct xenbus_device *dev, - const char *frontend); +void netfront_accelerator_add_watch(struct netfront_info *np); extern void netif_init_accel(void); -- 2.39.5