From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by dpdk.space (Postfix) with ESMTP id ABD77A00E6
	for <public@inbox.dpdk.org>; Wed, 20 Mar 2019 16:24:35 +0100 (CET)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id C7639695D;
	Wed, 20 Mar 2019 16:24:34 +0100 (CET)
Received: from mga09.intel.com (mga09.intel.com [134.134.136.24])
 by dpdk.org (Postfix) with ESMTP id 46E474F9A
 for <dev@dpdk.org>; Wed, 20 Mar 2019 16:24:33 +0100 (CET)
X-Amp-Result: UNSCANNABLE
X-Amp-File-Uploaded: False
Received: from fmsmga004.fm.intel.com ([10.253.24.48])
 by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
 20 Mar 2019 08:24:32 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.60,249,1549958400"; d="scan'208";a="154101203"
Received: from yexl-server.sh.intel.com (HELO localhost) ([10.67.110.206])
 by fmsmga004.fm.intel.com with ESMTP; 20 Mar 2019 08:24:28 -0700
Date: Wed, 20 Mar 2019 23:20:32 +0800
From: Ye Xiaolong <xiaolong.ye@intel.com>
To: David Marchand <david.marchand@redhat.com>
Cc: dev <dev@dpdk.org>, Qi Zhang <qi.z.zhang@intel.com>,
 Karlsson Magnus <magnus.karlsson@intel.com>,
 Topel Bjorn <bjorn.topel@intel.com>
Message-ID: <20190320152032.GA49599@intel.com>
References: <20190301080947.91086-1-xiaolong.ye@intel.com>
 <20190319071256.26302-1-xiaolong.ye@intel.com>
 <20190319071256.26302-2-xiaolong.ye@intel.com>
 <CAJFAV8yRczhw5LQh+dczmpfxEypJyOVAoCJFHOfM813iY-35-Q@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Disposition: inline
In-Reply-To: <CAJFAV8yRczhw5LQh+dczmpfxEypJyOVAoCJFHOfM813iY-35-Q@mail.gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Subject: Re: [dpdk-dev] [PATCH v2 1/6] net/af_xdp: introduce AF XDP PMD
	driver
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
Message-ID: <20190320152032.eQh1EVqd7vs7MLxXhjNt0aT4jehModNVTTBs-JX0TYs@z>

Thanks for your comments.

On 03/20, David Marchand wrote:
>On Tue, Mar 19, 2019 at 8:17 AM Xiaolong Ye <xiaolong.ye@intel.com> wrote:
>
>> diff --git a/doc/guides/nics/features/af_xdp.ini
>> b/doc/guides/nics/features/af_xdp.ini
>> new file mode 100644
>> index 000000000..7b8fcce00
>> --- /dev/null
>> +++ b/doc/guides/nics/features/af_xdp.ini
>> @@ -0,0 +1,11 @@
>> +;
>> +; Supported features of the 'af_xdp' network poll mode driver.
>> +;
>> +; Refer to default.ini for the full list of available PMD features.
>> +;
>> +[Features]
>> +Link status          = Y
>> +MTU update           = Y
>> +Promiscuous mode     = Y
>> +Stats per queue      = Y
>> +x86-64               = Y
>>
>
>Is there really a limitation on x86?

I think no, just don't have a chance to try it on a x86-32 machine.

>
>
>diff --git a/drivers/net/Makefile b/drivers/net/Makefile
>> index 502869a87..5d401b8c5 100644
>> --- a/drivers/net/Makefile
>> +++ b/drivers/net/Makefile
>> @@ -9,6 +9,7 @@ ifeq ($(CONFIG_RTE_LIBRTE_THUNDERX_NICVF_PMD),d)
>>  endif
>>
>>  DIRS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) += af_packet
>> +DIRS-$(CONFIG_RTE_LIBRTE_PMD_AF_XDP) += af_xdp
>>  DIRS-$(CONFIG_RTE_LIBRTE_ARK_PMD) += ark
>>  DIRS-$(CONFIG_RTE_LIBRTE_ATLANTIC_PMD) += atlantic
>>  DIRS-$(CONFIG_RTE_LIBRTE_AVP_PMD) += avp
>> diff --git a/drivers/net/af_xdp/Makefile b/drivers/net/af_xdp/Makefile
>> new file mode 100644
>> index 000000000..6cf0ed7db
>> --- /dev/null
>> +++ b/drivers/net/af_xdp/Makefile
>> @@ -0,0 +1,33 @@
>> +# SPDX-License-Identifier: BSD-3-Clause
>> +# Copyright(c) 2018 Intel Corporation
>>
>
>2018? 2019?

Will correct it to 2019.

>
>+
>> +include $(RTE_SDK)/mk/rte.vars.mk
>> +
>> +#
>> +# library name
>> +#
>> +LIB = librte_pmd_af_xdp.a
>> +
>> +EXPORT_MAP := rte_pmd_af_xdp_version.map
>> +
>> +LIBABIVER := 1
>> +
>> +CFLAGS += -O3
>> +
>> +# require kernel version >= v5.1-rc1
>> +LINUX_VERSION := $(shell uname -r)
>> +CFLAGS += -I/lib/modules/$(LINUX_VERSION)/build/tools/include
>> +CFLAGS += -I/lib/modules/$(LINUX_VERSION)/build/tools/lib/bpf
>>
>
>We can reuse RTE_KERNELDIR here (even if the docs state that this was to
>build kmods so far).

Will do.

>
>
>> diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c
>> b/drivers/net/af_xdp/rte_eth_af_xdp.c
>> new file mode 100644
>> index 000000000..96dedc0c4
>> --- /dev/null
>> +++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
>>
>
>[snip]
>
>
>> +static int
>> +eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
>> +{
>> +       struct pmd_internals *internals = dev->data->dev_private;
>> +       struct xdp_statistics xdp_stats;
>> +       struct pkt_rx_queue *rxq;
>> +       socklen_t optlen;
>> +       int i;
>> +
>> +       optlen = sizeof(struct xdp_statistics);
>> +       for (i = 0; i < dev->data->nb_rx_queues; i++) {
>> +               rxq = &internals->rx_queues[i];
>> +               stats->q_ipackets[i] = internals->rx_queues[i].rx_pkts;
>> +               stats->q_ibytes[i] = internals->rx_queues[i].rx_bytes;
>> +
>> +               stats->q_opackets[i] = internals->tx_queues[i].tx_pkts;
>> +               stats->q_obytes[i] = internals->tx_queues[i].tx_bytes;
>> +
>> +               stats->ipackets += stats->q_ipackets[i];
>> +               stats->ibytes += stats->q_ibytes[i];
>> +               stats->imissed += internals->rx_queues[i].rx_dropped;
>> +               getsockopt(xsk_socket__fd(rxq->xsk), SOL_XDP,
>> XDP_STATISTICS,
>> +                               &xdp_stats, &optlen);
>> +               stats->imissed += xdp_stats.rx_dropped;
>> +
>> +               stats->opackets += stats->q_opackets[i];
>> +               stats->oerrors += stats->q_errors[i];
>>
>
>You forgot to remove stats->q_errors[i];
>
>+               stats->oerrors += internals->tx_queues[i].err_pkts;

My bad, will remove it in next version.


Thanks,
Xiaolong
>> +               stats->obytes += stats->q_obytes[i];
>> +       }
>> +
>> +       return 0;
>> +}
>
>
>
>-- 
>David Marchand