From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-f66.google.com (mail-lf1-f66.google.com [209.85.167.66]) by dpdk.org (Postfix) with ESMTP id 130202C17 for ; Mon, 4 Mar 2019 13:30:36 +0100 (CET) Received: by mail-lf1-f66.google.com with SMTP id d26so3392054lfa.1 for ; Mon, 04 Mar 2019 04:30:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=netcope.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=8k2ik9L8IHeJ3G492m6AoeN4VLe2rSk73IlA9MCmmwQ=; b=X/rUZMBIbCktg/Ht5UreF0XRa9WXVYtkqLqHFbuTiAVhO1LwjK5w91aLXO8OdXrmgm PDVfeWmU55hJMTnO6qOd6le+KOIznyHmmaZU60DdboZRqeYagxAO8bkSW/3SNpd8byyM A/i0aJV/pinZZq93EXW/idV8Cp39f7xSsEWWn7I75AKzUMhSYre8eKp1d/dCRw3Vd1qv nz5XXdTGyYkv/uKMEBrzh2myKixifSSWNhFxGH8ACIakHnW90tqmDlKINzTIfFsUCYTn DhECM+7oe0CJa9xX+Eel0Udyc3555T814nPvU3gYcq6IYEwHxmNRo6KrntDZgS/2JxZ0 tpfA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=8k2ik9L8IHeJ3G492m6AoeN4VLe2rSk73IlA9MCmmwQ=; b=LIyyHSvhdG/bsW+yORo/n1Os5qD2LRxxtFCPnaA50H8pMq0XeFnLQ816zWDMJk1oQb T5pXUXHNNnQA6SGY87PqMXXP4K1c61EH7THunF9lwprWBGiouJHixWvWPkSDEi6oo9g0 9PcaVrcpD5+x2D6sn+klVe1aBgY3owPDoFCJgChquEOh9C2mWT6ryTvn2PMPF85UP3ef TGif7TkQ0EZz8QNlWaivcwEetYc8nKQcvlki6m/wwt2akTDH3x44FSHJJUvdi16rbQXn M2LDyt1opk+w61tjhQLSt0l6bawq6ua79doaOUsWwx9Z1sQO6+86WYg6fcfjxsXsdNwg J9Gg== X-Gm-Message-State: APjAAAXbsvO7RSGQ71ihnseIFl6GT41KMsg4KdSgNbs2tj66LBUj3eq2 LcmU+oNdCXCk9zxNFcKx88X+KLtF+4jkHW5t0AgHUw== X-Google-Smtp-Source: APXvYqyNFFwehnUlT8ce+FJFfaiIb9QZBMetO01myH2+IaLGgdt+PPjTd8WTvd0XMuXdzYW5csdfVbW/YeVPnUGB4HE= X-Received: by 2002:ac2:5387:: with SMTP id g7mr10605733lfh.158.1551702635400; Mon, 04 Mar 2019 04:30:35 -0800 (PST) MIME-Version: 1.0 References: <1551185824-5501-2-git-send-email-cernay@netcope.com> <1551451054-111249-1-git-send-email-cernay@netcope.com> In-Reply-To: From: =?UTF-8?Q?Rastislav_=C4=8Cernay?= Date: Mon, 4 Mar 2019 15:33:08 +0100 Message-ID: To: David Marchand Cc: dev , "Yigit, Ferruh" Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v3] net/nfb: new netcope driver X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 04 Mar 2019 12:30:36 -0000 >>> What is the point of adding when i >= RTE_ETHDEV_QUEUE_STAT_CNTRS ? struct rte_eth_stats { ... uint64_t q_opackets[RTE_ETHDEV_QUEUE_STAT_CNTRS] ... } As there can be more queues (nb_tx) then RTE_ETHDEV_QUEUE_STAT_CNTRS (16) and struct rte_eth_stats eth_stats is allocated statically, there is need to check so it does not write garbage somewhere. >>> Besides, q_errors[] is for reception errors. I will fix that, meanwhile could q_errors[] be renamed to q_ierrors[]? Also could there be a way to publish output errors per queue, for example q_oerrors[]? On Mon, Mar 4, 2019 at 12:35 PM David Marchand wrote: > > On Fri, Mar 1, 2019 at 3:38 PM Rastislav Cernay > wrote: > >> diff --git a/drivers/net/nfb/nfb_stats.c b/drivers/net/nfb/nfb_stats.c >> new file mode 100644 >> index 0000000..ffc27a5 >> --- /dev/null >> +++ b/drivers/net/nfb/nfb_stats.c >> @@ -0,0 +1,78 @@ >> +/* SPDX-License-Identifier: BSD-3-Clause >> + * Copyright(c) 2018 Cesnet >> + * Copyright(c) 2018 Netcope Technologies, a.s. >> + * All rights reserved. >> + */ >> + >> +#include "nfb_stats.h" >> +#include "nfb.h" >> + >> +int >> +nfb_eth_stats_get(struct rte_eth_dev *dev, >> + struct rte_eth_stats *stats) >> +{ >> + uint16_t i; >> + uint16_t nb_rx = dev->data->nb_rx_queues; >> + uint16_t nb_tx = dev->data->nb_tx_queues; >> + uint64_t rx_total = 0; >> + uint64_t tx_total = 0; >> + uint64_t tx_err_total = 0; >> + uint64_t rx_total_bytes = 0; >> + uint64_t tx_total_bytes = 0; >> + >> + struct ndp_rx_queue *rx_queue = *((struct ndp_rx_queue **) >> + dev->data->rx_queues); >> + struct ndp_tx_queue *tx_queue = *((struct ndp_tx_queue **) >> + dev->data->tx_queues); >> + >> + for (i = 0; i < nb_rx; i++) { >> + if (i < RTE_ETHDEV_QUEUE_STAT_CNTRS) { >> + stats->q_ipackets[i] = rx_queue[i].rx_pkts; >> + stats->q_ibytes[i] = rx_queue[i].rx_bytes; >> + } >> + rx_total += stats->q_ipackets[i]; >> + rx_total_bytes += stats->q_ibytes[i]; >> + } >> > > What is the point of adding when i >= RTE_ETHDEV_QUEUE_STAT_CNTRS ? > Hopefully, ethdev passes a zero'd structure, but still I find it confusing. > > > >> + >> + for (i = 0; i < nb_tx; i++) { >> + if (i < RTE_ETHDEV_QUEUE_STAT_CNTRS) { >> + stats->q_opackets[i] = tx_queue[i].tx_pkts; >> + stats->q_obytes[i] = tx_queue[i].tx_bytes; >> + stats->q_errors[i] = tx_queue[i].err_pkts; >> + } >> + tx_total += stats->q_opackets[i]; >> + tx_total_bytes += stats->q_obytes[i]; >> + tx_err_total += stats->q_errors[i]; >> + } >> > > Idem. > Besides, q_errors[] is for reception errors. > > + >> + stats->ipackets = rx_total; >> + stats->opackets = tx_total; >> + stats->ibytes = rx_total_bytes; >> + stats->obytes = tx_total_bytes; >> + stats->oerrors = tx_err_total; >> + return 0; >> +} >> >> > -- > David Marchand >