From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Bennieston Subject: Re: [PATCH RFC 2/4] xen-netback: Add support for multiple queues Date: Thu, 16 Jan 2014 10:40:25 +0000 Message-ID: <52D7B719.3010501@citrix.com> References: <1389803004-31812-1-git-send-email-andrew.bennieston@citrix.com> <1389803004-31812-3-git-send-email-andrew.bennieston@citrix.com> <9AAE0902D5BC7E449B7C8E4E778ABCD0208DFE@AMSPEX01CL01.citrite.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1W3kY2-0003ZP-0r for xen-devel@lists.xenproject.org; Thu, 16 Jan 2014 10:51:42 +0000 In-Reply-To: <9AAE0902D5BC7E449B7C8E4E778ABCD0208DFE@AMSPEX01CL01.citrite.net> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Paul Durrant , "xen-devel@lists.xenproject.org" Cc: Wei Liu , Ian Campbell List-Id: xen-devel@lists.xenproject.org On 16/01/14 10:28, Paul Durrant wrote: >> -----Original Message----- >> From: Andrew J. Bennieston [mailto:andrew.bennieston@citrix.com] >> Sent: 15 January 2014 16:23 >> To: xen-devel@lists.xenproject.org >> Cc: Ian Campbell; Wei Liu; Paul Durrant; Andrew Bennieston >> Subject: [PATCH RFC 2/4] xen-netback: Add support for multiple queues >> >> From: "Andrew J. Bennieston" >> >> Builds on the refactoring of the previous patch to implement multiple >> queues between xen-netfront and xen-netback. >> >> Writes the maximum supported number of queues into XenStore, and reads >> the values written by the frontend to determine how many queues to use. >> >> Ring references and event channels are read from XenStore on a per-queue >> basis and rings are connected accordingly. >> >> Signed-off-by: Andrew J. Bennieston >> --- >> drivers/net/xen-netback/common.h | 2 + >> drivers/net/xen-netback/interface.c | 8 +++- >> drivers/net/xen-netback/netback.c | 3 ++ >> drivers/net/xen-netback/xenbus.c | 70 >> ++++++++++++++++++++++++++++++----- >> 4 files changed, 72 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen- >> netback/common.h >> index 54d2eeb..97efd09 100644 >> --- a/drivers/net/xen-netback/common.h >> +++ b/drivers/net/xen-netback/common.h >> @@ -254,4 +254,6 @@ void xenvif_carrier_on(struct xenvif *vif); >> >> extern bool separate_tx_rx_irq; >> >> +extern unsigned int xenvif_max_queues; >> + >> #endif /* __XEN_NETBACK__COMMON_H__ */ >> diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen- >> netback/interface.c >> index 0113324..0234ff0 100644 >> --- a/drivers/net/xen-netback/interface.c >> +++ b/drivers/net/xen-netback/interface.c >> @@ -355,7 +355,13 @@ struct xenvif *xenvif_alloc(struct device *parent, >> domid_t domid, >> char name[IFNAMSIZ] = {}; >> >> snprintf(name, IFNAMSIZ - 1, "vif%u.%u", domid, handle); >> - dev = alloc_netdev_mq(sizeof(struct xenvif), name, ether_setup, 1); >> + /* >> + * Allocate a netdev with the max. supported number of queues. >> + * When the guest selects the desired number, it will be updated >> + * via netif_set_real_num_tx_queues(). >> + */ >> + dev = alloc_netdev_mq(sizeof(struct xenvif), name, ether_setup, >> + xenvif_max_queues); >> if (dev == NULL) { >> pr_warn("Could not allocate netdev for %s\n", name); >> return ERR_PTR(-ENOMEM); >> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen- >> netback/netback.c >> index 586e741..5d717d7 100644 >> --- a/drivers/net/xen-netback/netback.c >> +++ b/drivers/net/xen-netback/netback.c >> @@ -55,6 +55,9 @@ >> bool separate_tx_rx_irq = 1; >> module_param(separate_tx_rx_irq, bool, 0644); >> >> +unsigned int xenvif_max_queues = 4; >> +module_param(xenvif_max_queues, uint, 0644); >> + >> /* >> * This is the maximum slots a skb can have. If a guest sends a skb >> * which exceeds this limit it is considered malicious. >> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen- >> netback/xenbus.c >> index c3332e2..ce7ca9a 100644 >> --- a/drivers/net/xen-netback/xenbus.c >> +++ b/drivers/net/xen-netback/xenbus.c >> @@ -21,6 +21,7 @@ >> >> #include "common.h" >> #include >> +#include >> >> struct backend_info { >> struct xenbus_device *dev; >> @@ -160,6 +161,14 @@ static int netback_probe(struct xenbus_device *dev, >> if (err) >> pr_debug("Error writing feature-split-event-channels\n"); >> >> + /* >> + * Multi-queue support: This is an optional feature. >> + */ > > Comment style. Did you run checkpatch? > >> + err = xenbus_printf(XBT_NIL, dev->nodename, >> + "multi-queue-max-queues", "%u", >> xenvif_max_queues); >> + if (err) >> + pr_debug("Error writing multi-queue-max-queues\n"); >> + >> err = xenbus_switch_state(dev, XenbusStateInitWait); >> if (err) >> goto fail; >> @@ -491,6 +500,16 @@ static void connect(struct backend_info *be) >> unsigned long credit_bytes, credit_usec; >> unsigned int queue_index; >> struct xenvif_queue *queue; >> + unsigned int requested_num_queues; >> + >> + /* Check whether the frontend requested multiple queues >> + * and read the number requested. >> + */ >> + err = xenbus_scanf(XBT_NIL, dev->otherend, >> + "multi-queue-num-queues", >> + "%u", &requested_num_queues); >> + if (err < 0) >> + requested_num_queues = 1; /* Fall back to single queue */ >> >> err = xen_net_read_mac(dev, be->vif->fe_dev_addr); >> if (err) { >> @@ -501,9 +520,13 @@ static void connect(struct backend_info *be) >> xen_net_read_rate(dev, &credit_bytes, &credit_usec); >> read_xenbus_vif_flags(be); >> >> - be->vif->num_queues = 1; >> + /* Use the number of queues requested by the frontend */ >> + be->vif->num_queues = requested_num_queues; >> be->vif->queues = vzalloc(be->vif->num_queues * >> sizeof(struct xenvif_queue)); >> + rtnl_lock(); >> + netif_set_real_num_tx_queues(be->vif->dev, be->vif- >>> num_queues); >> + rtnl_unlock(); >> >> for (queue_index = 0; queue_index < be->vif->num_queues; >> ++queue_index) >> { >> @@ -549,29 +572,51 @@ static int connect_rings(struct backend_info *be, >> struct xenvif_queue *queue) >> unsigned long tx_ring_ref, rx_ring_ref; >> unsigned int tx_evtchn, rx_evtchn; >> int err; >> + char *xspath = NULL; >> + size_t xspathsize; >> + >> + /* If the frontend requested 1 queue, or we have fallen back >> + * to single queue due to lack of frontend support for multi- >> + * queue, expect the remaining XenStore keys in the toplevel >> + * directory. Otherwise, expect them in a subdirectory called >> + * queue-N. >> + */ >> + if (queue->vif->num_queues == 1) >> + xspath = (char *)dev->otherend; >> + else { >> + xspathsize = strlen(dev->otherend) + 10; > > Magic number. > > Paul > Yes, I'll change this to a defined constant. Andrew >> + xspath = kzalloc(xspathsize, GFP_KERNEL); >> + if (!xspath) { >> + xenbus_dev_fatal(dev, -ENOMEM, >> + "reading ring references"); >> + return -ENOMEM; >> + } >> + snprintf(xspath, xspathsize, "%s/queue-%u", dev- >>> otherend, >> + queue->number); >> + } >> >> - err = xenbus_gather(XBT_NIL, dev->otherend, >> + err = xenbus_gather(XBT_NIL, xspath, >> "tx-ring-ref", "%lu", &tx_ring_ref, >> "rx-ring-ref", "%lu", &rx_ring_ref, NULL); >> if (err) { >> xenbus_dev_fatal(dev, err, >> "reading %s/ring-ref", >> - dev->otherend); >> - return err; >> + xspath); >> + goto err; >> } >> >> /* Try split event channels first, then single event channel. */ >> - err = xenbus_gather(XBT_NIL, dev->otherend, >> + err = xenbus_gather(XBT_NIL, xspath, >> "event-channel-tx", "%u", &tx_evtchn, >> "event-channel-rx", "%u", &rx_evtchn, NULL); >> if (err < 0) { >> - err = xenbus_scanf(XBT_NIL, dev->otherend, >> + err = xenbus_scanf(XBT_NIL, xspath, >> "event-channel", "%u", &tx_evtchn); >> if (err < 0) { >> xenbus_dev_fatal(dev, err, >> "reading %s/event-channel(-tx/rx)", >> - dev->otherend); >> - return err; >> + xspath); >> + goto err; >> } >> rx_evtchn = tx_evtchn; >> } >> @@ -584,10 +629,15 @@ static int connect_rings(struct backend_info *be, >> struct xenvif_queue *queue) >> "mapping shared-frames %lu/%lu port tx %u >> rx %u", >> tx_ring_ref, rx_ring_ref, >> tx_evtchn, rx_evtchn); >> - return err; >> + goto err; >> } >> >> - return 0; >> + err = 0; >> +err: /* Regular return falls through with err == 0 */ >> + if (xspath != dev->otherend) >> + kfree(xspath); >> + >> + return err; >> } >> >> static int read_xenbus_vif_flags(struct backend_info *be) >> -- >> 1.7.10.4 >