From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Bennieston Subject: Re: [PATCH RFC 4/4] xen-netfront: Add support for multiple queues Date: Thu, 16 Jan 2014 10:24:57 +0000 Message-ID: <52D7B379.201@citrix.com> References: <1389803004-31812-1-git-send-email-andrew.bennieston@citrix.com> <1389803004-31812-5-git-send-email-andrew.bennieston@citrix.com> <20140116002706.GJ5331@zion.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1W3k8D-0001Gb-Uj for xen-devel@lists.xenproject.org; Thu, 16 Jan 2014 10:25:03 +0000 In-Reply-To: <20140116002706.GJ5331@zion.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Wei Liu Cc: xen-devel@lists.xenproject.org, paul.durrant@citrix.com, ian.campbell@citrix.com List-Id: xen-devel@lists.xenproject.org On 16/01/14 00:27, Wei Liu wrote: > On Wed, Jan 15, 2014 at 04:23:24PM +0000, Andrew J. Bennieston wrote: > [...] >> +/* Module parameters */ >> +unsigned int xennet_max_queues = 16; >> +module_param(xennet_max_queues, uint, 0644); >> + > > This looks quite arbitrary as well. Arbitrary, but less arbitrary than the netback parameter. I wanted to pick a value that was large enough to accommodate most sensible backend-provided limits, but not so large that it wasted resources. It is necessary only because I have to call alloc_etherdev_mq() long before I can read the maximum number of queues from XenStore. This value gets used there, so I wanted it to be at least as big as anything sensible that a backend might provide. >> static const struct ethtool_ops xennet_ethtool_ops; >> > [...] >> +static int write_queue_xenstore_keys(struct netfront_queue *queue, >> + struct xenbus_transaction *xbt, int write_hierarchical) >> +{ >> + /* Write the queue-specific keys into XenStore in the traditional >> + * way for a single queue, or in a queue subkeys for multiple >> + * queues. >> + */ >> + struct xenbus_device *dev = queue->info->xbdev; >> + int err; >> + const char *message; >> + char *path; >> + size_t pathsize; >> + >> + /* Choose the correct place to write the keys */ >> + if (write_hierarchical) { >> + pathsize = strlen(dev->nodename) + 10; >> + path = kzalloc(pathsize, GFP_KERNEL); >> + if (!path) { >> + err = -ENOMEM; >> + message = "writing ring references"; > > This error message doesn't sound right. > I'll reword it. >> + goto error; >> + } >> + snprintf(path, pathsize, "%s/queue-%u", >> + dev->nodename, queue->number); >> + } >> + else >> + path = (char *)dev->nodename; > > Coding style. Should be surounded by {}; > OK. >> + > [...] >> @@ -1740,10 +1838,17 @@ static int talk_to_netback(struct xenbus_device *dev, >> int err; >> unsigned int feature_split_evtchn; >> unsigned int i = 0; >> + unsigned int max_queues = 0; >> struct netfront_queue *queue = NULL; >> >> info->netdev->irq = 0; >> >> + /* Check if backend supports multiple queues */ >> + err = xenbus_scanf(XBT_NIL, info->xbdev->otherend, >> + "multi-queue-max-queues", "%u", &max_queues); >> + if (err < 0) >> + max_queues = 1; >> + > > Need to check if backend provide too big a number for frontend. Yes, I'll add that check. Andrew. > > Wei. >