On 07/05/2010 04:39 AM, Paul Durrant wrote:
> Frontend features are currently only sampled at ring connection time.
> Some frontends can change these features dynamically so watches should
> be used to update the corresponding netback flags as and when they
> change.
>
Are there any concerns about these feature flags changing asynchronously?
The watches will run in their own task, and so can be concurrent to the
netback code using the flags. What prevents that from upsetting the
tests of the flags in, for example, netbk_max_required_rx_slots? Should
there be some locking? Or double-buffer the feature flags so that the
netback code can get updated values at a safe/appropriate time?
Can all features be changed dynamically by the frontend, or just some?
J
> Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
> ---
> drivers/xen/netback/common.h | 8 +
> drivers/xen/netback/xenbus.c | 293
> ++++++++++++++++++++++++++++++++++++++----
> 2 files changed, 274 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/xen/netback/common.h b/drivers/xen/netback/common.h
> index a673331..3a0abf6 100644
> --- a/drivers/xen/netback/common.h
> +++ b/drivers/xen/netback/common.h
> @@ -154,6 +154,14 @@ struct netback_accelerator {
> struct backend_info {
> struct xenbus_device *dev;
> struct xen_netif *netif;
> +
> + /* Frontend feature watches. */
> + struct xenbus_watch can_sg_watch;
> + struct xenbus_watch gso_watch;
> + struct xenbus_watch gso_prefix_watch;
> + struct xenbus_watch csum_watch;
> + struct xenbus_watch smart_poll_watch;
> +
> enum xenbus_state frontend_state;
> struct xenbus_watch hotplug_status_watch;
> int have_hotplug_status_watch:1;
> diff --git a/drivers/xen/netback/xenbus.c b/drivers/xen/netback/xenbus.c
> index 99831c7..d08463c 100644
> --- a/drivers/xen/netback/xenbus.c
> +++ b/drivers/xen/netback/xenbus.c
> @@ -33,6 +33,7 @@ static int connect_rings(struct backend_info *);
> static void connect(struct backend_info *);
> static void backend_create_netif(struct backend_info *be);
> static void unregister_hotplug_status_watch(struct backend_info *be);
> +static void unregister_frontend_watches(struct backend_info *be);
>
> static int netback_remove(struct xenbus_device *dev)
> {
> @@ -44,6 +45,7 @@ static int netback_remove(struct xenbus_device *dev)
> if (be->netif) {
> kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
> xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
> + unregister_frontend_watches(be);
> netif_disconnect(be->netif);
> be->netif = NULL;
> }
> @@ -227,6 +229,7 @@ static void disconnect_backend(struct xenbus_device *dev)
>
> if (be->netif) {
> xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
> + unregister_frontend_watches(be);
> netif_disconnect(be->netif);
> be->netif = NULL;
> }
> @@ -409,6 +412,268 @@ static void connect(struct backend_info *be)
> netif_wake_queue(be->netif->dev);
> }
>
> +static void feature_can_sg_watch(struct xenbus_watch *watch,
> + const char **vec,
> + unsigned int vec_size)
> +{
> + struct backend_info *be = container_of(watch,
> + struct backend_info,
> + can_sg_watch);
> + struct xen_netif *netif = be->netif;
> + struct xenbus_device *dev = be->dev;
> + int val;
> +
> + if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-sg",
> + "%d", &val) < 0)
> + val = 0;
> + netif->can_sg = !!val;
> + DPRINTK("can_sg -> %d\n", netif->can_sg);
> +
> + netif_set_features(netif);
> +}
> +
> +static int register_can_sg_watch(struct backend_info *be)
> +{
> + struct xenbus_device *dev = be->dev;
> + int err;
> +
> + if (be->can_sg_watch.node)
> + return -EEXIST;
> +
> + err = xenbus_watch_pathfmt(dev, &be->can_sg_watch,
> + feature_can_sg_watch, "%s/%s",
> + dev->otherend, "feature-sg");
> + if (err) {
> + xenbus_dev_fatal(dev, err, "watching %s/feature-sg",
> + dev->otherend);
> + return err;
> + }
> + return 0;
> +}
> +
> +static void unregister_can_sg_watch(struct backend_info *be)
> +{
> + if (!be->can_sg_watch.node)
> + return;
> +
> + unregister_xenbus_watch(&be->can_sg_watch);
> + kfree(be->can_sg_watch.node);
> + be->can_sg_watch.node = NULL;
> +}
> +
> +static void feature_gso_watch(struct xenbus_watch *watch,
> + const char **vec,
> + unsigned int vec_size)
> +{
> + struct backend_info *be = container_of(watch,
> + struct backend_info,
> + gso_watch);
> + struct xen_netif *netif = be->netif;
> + struct xenbus_device *dev = be->dev;
> + int val;
> +
> + if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4",
> + "%d", &val) < 0)
> + val = 0;
> + netif->gso = !!val;
> + DPRINTK("gso -> %d\n", netif->gso);
> +
> + netif_set_features(netif);
> +}
> +
> +static int register_gso_watch(struct backend_info *be)
> +{
> + struct xenbus_device *dev = be->dev;
> + int err;
> +
> + if (be->gso_watch.node)
> + return -EEXIST;
> +
> + err = xenbus_watch_pathfmt(dev, &be->gso_watch,
> + feature_gso_watch, "%s/%s",
> + dev->otherend, "feature-gso-tcpv4");
> + if (err) {
> + xenbus_dev_fatal(dev, err, "watching %s/feature-gso-tcpv4",
> + dev->otherend);
> + return err;
> + }
> + return 0;
> +}
> +
> +static void unregister_gso_watch(struct backend_info *be)
> +{
> + if (!be->gso_watch.node)
> + return;
> +
> + unregister_xenbus_watch(&be->gso_watch);
> + kfree(be->gso_watch.node);
> + be->gso_watch.node = NULL;
> +}
> +
> +static void feature_gso_prefix_watch(struct xenbus_watch *watch,
> + const char **vec,
> + unsigned int vec_size)
> +{
> + struct backend_info *be = container_of(watch,
> + struct backend_info,
> + gso_prefix_watch);
> + struct xen_netif *netif = be->netif;
> + struct xenbus_device *dev = be->dev;
> + int val;
> +
> + if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4-prefix",
> + "%d", &val) < 0)
> + val = 0;
> + netif->gso_prefix = !!val;
> + DPRINTK("gso_prefix -> %d\n", netif->gso_prefix);
> +
> + netif_set_features(netif);
> +}
> +
> +static int register_gso_prefix_watch(struct backend_info *be)
> +{
> + struct xenbus_device *dev = be->dev;
> + int err;
> +
> + if (be->gso_prefix_watch.node)
> + return -EEXIST;
> +
> + err = xenbus_watch_pathfmt(dev, &be->gso_prefix_watch,
> + feature_gso_prefix_watch, "%s/%s",
> + dev->otherend, "feature-gso-tcpv4-prefix");
> + if (err) {
> + xenbus_dev_fatal(dev, err, "watching
> %s/feature-gso-tcpv4-prefix",
> + dev->otherend);
> + return err;
> + }
> + return 0;
> +}
> +
> +static void unregister_gso_prefix_watch(struct backend_info *be)
> +{
> + if (!be->gso_prefix_watch.node)
> + return;
> +
> + unregister_xenbus_watch(&be->gso_prefix_watch);
> + kfree(be->gso_prefix_watch.node);
> + be->gso_prefix_watch.node = NULL;
> +}
> +
> +static void feature_csum_watch(struct xenbus_watch *watch,
> + const char **vec,
> + unsigned int vec_size)
> +{
> + struct backend_info *be = container_of(watch,
> + struct backend_info,
> + csum_watch);
> + struct xen_netif *netif = be->netif;
> + struct xenbus_device *dev = be->dev;
> + int val;
> +
> + if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-no-csum",
> + "%d", &val) < 0)
> + val = 0;
> + netif->csum = !val;
> + DPRINTK("csum -> %d\n", netif->csum);
> +
> + netif_set_features(netif);
> +}
> +
> +static int register_csum_watch(struct backend_info *be)
> +{
> + struct xenbus_device *dev = be->dev;
> + int err;
> +
> + if (be->csum_watch.node)
> + return -EEXIST;
> +
> + err = xenbus_watch_pathfmt(dev, &be->csum_watch,
> + feature_csum_watch, "%s/%s",
> + dev->otherend, "feature-no-csum");
> + if (err) {
> + xenbus_dev_fatal(dev, err, "watching %s/feature-no-csum",
> + dev->otherend);
> + return err;
> + }
> + return 0;
> +}
> +
> +static void unregister_csum_watch(struct backend_info *be)
> +{
> + if (!be->csum_watch.node)
> + return;
> +
> + unregister_xenbus_watch(&be->csum_watch);
> + kfree(be->csum_watch.node);
> + be->csum_watch.node = NULL;
> +}
> +
> +static void feature_smart_poll_watch(struct xenbus_watch *watch,
> + const char **vec,
> + unsigned int vec_size)
> +{
> + struct backend_info *be = container_of(watch,
> + struct backend_info,
> + smart_poll_watch);
> + struct xen_netif *netif = be->netif;
> + struct xenbus_device *dev = be->dev;
> + int val;
> +
> + if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-smart-poll",
> + "%d", &val) < 0)
> + val = 0;
> + netif->smart_poll = !!val;
> + DPRINTK("smart_poll -> %d\n", netif->smart_poll);
> +
> + netif_set_features(netif);
> +}
> +
> +static int register_smart_poll_watch(struct backend_info *be)
> +{
> + struct xenbus_device *dev = be->dev;
> + int err;
> +
> + if (be->smart_poll_watch.node)
> + return -EEXIST;
> +
> + err = xenbus_watch_pathfmt(dev, &be->smart_poll_watch,
> + feature_smart_poll_watch, "%s/%s",
> + dev->otherend, "feature-smart-poll");
> + if (err) {
> + xenbus_dev_fatal(dev, err, "watching %s/feature-smart-poll",
> + dev->otherend);
> + return err;
> + }
> + return 0;
> +}
> +
> +static void unregister_smart_poll_watch(struct backend_info *be)
> +{
> + if (!be->smart_poll_watch.node)
> + return;
> +
> + unregister_xenbus_watch(&be->smart_poll_watch);
> + kfree(be->smart_poll_watch.node);
> + be->smart_poll_watch.node = NULL;
> +}
> +
> +static void register_frontend_watches(struct backend_info *be)
> +{
> + register_can_sg_watch(be);
> + register_gso_watch(be);
> + register_gso_prefix_watch(be);
> + register_csum_watch(be);
> + register_smart_poll_watch(be);
> +}
> +
> +static void unregister_frontend_watches(struct backend_info *be)
> +{
> + unregister_can_sg_watch(be);
> + unregister_gso_watch(be);
> + unregister_gso_prefix_watch(be);
> + unregister_csum_watch(be);
> + unregister_smart_poll_watch(be);
> +}
>
> static int connect_rings(struct backend_info *be)
> {
> @@ -457,33 +722,7 @@ static int connect_rings(struct backend_info *be)
> netif->dev->tx_queue_len = 1;
> }
>
> - if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-sg",
> - "%d", &val) < 0)
> - val = 0;
> - netif->can_sg = !!val;
> -
> - if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4",
> - "%d", &val) < 0)
> - val = 0;
> - netif->gso = !!val;
> -
> - if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4-prefix",
> - "%d", &val) < 0)
> - val = 0;
> - netif->gso_prefix = !!val;
> -
> - if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-no-csum-offload",
> - "%d", &val) < 0)
> - val = 0;
> - netif->csum = !val;
> -
> - if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-smart-poll",
> - "%d", &val) < 0)
> - val = 0;
> - netif->smart_poll = !!val;
> -
> - /* Set dev->features */
> - netif_set_features(netif);
> + register_frontend_watches(be);
>
> /* Map the shared frame, irq etc. */
> err = netif_map(netif, tx_ring_ref, rx_ring_ref, evtchn);
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home