From: Bruce Richardson <bruce.richardson@intel.com>
To: John Daley <johndale@cisco.com>
Cc: dev@dpdk.org, Nelson Escobar <neescoba@cisco.com>
Subject: Re: [dpdk-dev] [PATCH] enic: don't set enic->config.rq_desc_count in enic_alloc_rq()
Date: Fri, 18 Mar 2016 10:24:38 +0000 [thread overview]
Message-ID: <20160318102438.GD4848@bricha3-MOBL3> (raw)
In-Reply-To: <1458254810-32283-1-git-send-email-johndale@cisco.com>
On Thu, Mar 17, 2016 at 03:46:50PM -0700, John Daley wrote:
> From: Nelson Escobar <neescoba@cisco.com>
>
> When the requested number of rx descriptors was less than the amount
> configured on the vic, enic_alloc_rq() was incorrectly setting
> enic->config.rq_desc_count to the lower value. This screwed up later
> calls to enic_alloc_rq().
Can you perhaps clarify what exactly happened when the calls got "screwed up"?
Did the calls just fail, making the queue unusable, or did it crash the application,
or something else.
Similarly, can you perhaps reword the patch title to start with the word "fix"
as this patch is for a bug-fix, and describe in a couple of words there what
the bug was that was being fixed e.g. "fix rx queue lockup", rather than
describing the technicalities of the fix. In general, try to avoid using
function or variable names in the titles of messages.
Lastly, can you please include a "Fixes" line in the message, as described here:
http://dpdk.org/doc/guides/contributing/patches.html#commit-messages-body
Thanks,
/Bruce
>
> Signed-off-by: Nelson Escobar <neescoba@cisco.com>
> Reviewed-by: John Daley <johndale@cisco.com>
> ---
> drivers/net/enic/enic_main.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
> index 9fff020..cd7857f 100644
> --- a/drivers/net/enic/enic_main.c
> +++ b/drivers/net/enic/enic_main.c
> @@ -524,24 +524,22 @@ int enic_alloc_rq(struct enic *enic, uint16_t queue_idx,
> "policy. Applying the value in the adapter "\
> "policy (%d).\n",
> queue_idx, nb_desc, enic->config.rq_desc_count);
> - } else if (nb_desc != enic->config.rq_desc_count) {
> - enic->config.rq_desc_count = nb_desc;
> - dev_info(enic,
> - "RX Queues - effective number of descs:%d\n",
> - nb_desc);
> + nb_desc = enic->config.rq_desc_count;
> }
> + dev_info(enic, "RX Queues - effective number of descs:%d\n",
> + nb_desc);
> }
>
> /* Allocate queue resources */
> rc = vnic_rq_alloc(enic->vdev, rq, queue_idx,
> - enic->config.rq_desc_count, sizeof(struct rq_enet_desc));
> + nb_desc, sizeof(struct rq_enet_desc));
> if (rc) {
> dev_err(enic, "error in allocation of rq\n");
> goto err_exit;
> }
>
> rc = vnic_cq_alloc(enic->vdev, &enic->cq[queue_idx], queue_idx,
> - socket_id, enic->config.rq_desc_count,
> + socket_id, nb_desc,
> sizeof(struct cq_enet_rq_desc));
> if (rc) {
> dev_err(enic, "error in allocation of cq for rq\n");
> @@ -550,7 +548,7 @@ int enic_alloc_rq(struct enic *enic, uint16_t queue_idx,
>
> /* Allocate the mbuf ring */
> rq->mbuf_ring = (struct rte_mbuf **)rte_zmalloc_socket("rq->mbuf_ring",
> - sizeof(struct rte_mbuf *) * enic->config.rq_desc_count,
> + sizeof(struct rte_mbuf *) * nb_desc,
> RTE_CACHE_LINE_SIZE, rq->socket_id);
>
> if (rq->mbuf_ring != NULL)
> --
> 2.7.0
>
next prev parent reply other threads:[~2016-03-18 10:24 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-17 22:46 John Daley
2016-03-18 10:24 ` Bruce Richardson [this message]
2016-03-18 18:33 ` [dpdk-dev] [PATCH v2] enic: fix incorrect setting of rx descriptor limit John Daley
2016-03-22 17:32 ` Bruce Richardson
2016-03-22 17:33 ` Bruce Richardson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160318102438.GD4848@bricha3-MOBL3 \
--to=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=johndale@cisco.com \
--cc=neescoba@cisco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).