From: Yong Wang <yongwang@vmware.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v4] net/kni: add KNI PMD
Date: Mon, 19 Dec 2016 17:52:49 +0000 [thread overview]
Message-ID: <BY2PR05MB23598EBBDA10658E67087BA0AF910@BY2PR05MB2359.namprd05.prod.outlook.com> (raw)
In-Reply-To: <479e706e-4d7e-53d1-e140-5c26f001c2c5@intel.com>
> -----Original Message-----
> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: Thursday, December 15, 2016 7:56 AM
> To: Yong Wang <yongwang@vmware.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4] net/kni: add KNI PMD
>
> On 12/14/2016 7:25 PM, Yong Wang wrote:
> >> -----Original Message-----
> >> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> >> Sent: Wednesday, December 14, 2016 8:00 AM
> >> To: Yong Wang <yongwang@vmware.com>; dev@dpdk.org
> >> Subject: Re: [dpdk-dev] [PATCH v4] net/kni: add KNI PMD
> >>
> >> On 12/12/2016 9:59 PM, Yong Wang wrote:
> >>>> -----Original Message-----
> >>>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> >>>> Sent: Wednesday, November 30, 2016 10:12 AM
> >>>> To: dev@dpdk.org
> >>>> Cc: Ferruh Yigit <ferruh.yigit@intel.com>; Yong Wang
> >>>> <yongwang@vmware.com>
> >>>> Subject: [PATCH v4] net/kni: add KNI PMD
> >>>>
> >>>> Add KNI PMD which wraps librte_kni for ease of use.
> >>>>
> >>>> KNI PMD can be used as any regular PMD to send / receive packets to
> the
> >>>> Linux networking stack.
> >>>>
> >>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >>>> ---
> >>>>
> >>>> v4:
> >>>> * allow only single queue
> >>>> * use driver.name as name
> >>>>
> >>>> v3:
> >>>> * rebase on top of latest master
> >>>>
> >>>> v2:
> >>>> * updated driver name eth_kni -> net_kni
> >>>> ---
> >>>> config/common_base | 1 +
> >>>> config/common_linuxapp | 1 +
> >>>> drivers/net/Makefile | 1 +
> >>>> drivers/net/kni/Makefile | 63 +++++
> >>>> drivers/net/kni/rte_eth_kni.c | 462
> >>>> ++++++++++++++++++++++++++++++++
> >>>> drivers/net/kni/rte_pmd_kni_version.map | 4 +
> >>>> mk/rte.app.mk | 10 +-
> >>>> 7 files changed, 537 insertions(+), 5 deletions(-)
> >>>> create mode 100644 drivers/net/kni/Makefile
> >>>> create mode 100644 drivers/net/kni/rte_eth_kni.c
> >>>> create mode 100644 drivers/net/kni/rte_pmd_kni_version.map
> >>>>
> >>>> diff --git a/config/common_base b/config/common_base
> >>>> index 4bff83a..3385879 100644
> >>>> --- a/config/common_base
> >>>> +++ b/config/common_base
> >>>> @@ -543,6 +543,7 @@ CONFIG_RTE_PIPELINE_STATS_COLLECT=n
> >>>> # Compile librte_kni
> >>>> #
> >>>> CONFIG_RTE_LIBRTE_KNI=n
> >>>> +CONFIG_RTE_LIBRTE_PMD_KNI=n
> >>>> CONFIG_RTE_KNI_KMOD=n
> >>>> CONFIG_RTE_KNI_PREEMPT_DEFAULT=y
> >>>> CONFIG_RTE_KNI_VHOST=n
> >>>> diff --git a/config/common_linuxapp b/config/common_linuxapp
> >>>> index 2483dfa..2ecd510 100644
> >>>> --- a/config/common_linuxapp
> >>>> +++ b/config/common_linuxapp
> >>>> @@ -39,6 +39,7 @@ CONFIG_RTE_EAL_IGB_UIO=y
> >>>> CONFIG_RTE_EAL_VFIO=y
> >>>> CONFIG_RTE_KNI_KMOD=y
> >>>> CONFIG_RTE_LIBRTE_KNI=y
> >>>> +CONFIG_RTE_LIBRTE_PMD_KNI=y
> >>>> CONFIG_RTE_LIBRTE_VHOST=y
> >>>> CONFIG_RTE_LIBRTE_PMD_VHOST=y
> >>>> CONFIG_RTE_LIBRTE_PMD_AF_PACKET=y
> >>>> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> >>>> index bc93230..c4771cd 100644
> >>>> --- a/drivers/net/Makefile
> >>>> +++ b/drivers/net/Makefile
> >>>> @@ -41,6 +41,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += enic
> >>>> DIRS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += fm10k
> >>>> DIRS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e
> >>>> DIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += ixgbe
> >>>> +DIRS-$(CONFIG_RTE_LIBRTE_PMD_KNI) += kni
> >>>> DIRS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += mlx4
> >>>> DIRS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += mlx5
> >>>> DIRS-$(CONFIG_RTE_LIBRTE_MPIPE_PMD) += mpipe
> >>>> diff --git a/drivers/net/kni/Makefile b/drivers/net/kni/Makefile
> >>>> new file mode 100644
> >>>> index 0000000..0b7cf91
> >>>> --- /dev/null
> >>>> +++ b/drivers/net/kni/Makefile
> >>>> @@ -0,0 +1,63 @@
> >>>> +# BSD LICENSE
> >>>> +#
> >>>> +# Copyright(c) 2016 Intel Corporation. All rights reserved.
> >>>> +#
> >>>> +# Redistribution and use in source and binary forms, with or without
> >>>> +# modification, are permitted provided that the following conditions
> >>>> +# are met:
> >>>> +#
> >>>> +# * Redistributions of source code must retain the above copyright
> >>>> +# notice, this list of conditions and the following disclaimer.
> >>>> +# * Redistributions in binary form must reproduce the above
> copyright
> >>>> +# notice, this list of conditions and the following disclaimer in
> >>>> +# the documentation and/or other materials provided with the
> >>>> +# distribution.
> >>>> +# * Neither the name of Intel Corporation nor the names of its
> >>>> +# contributors may be used to endorse or promote products
> derived
> >>>> +# from this software without specific prior written permission.
> >>>> +#
> >>>> +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> >>>> CONTRIBUTORS
> >>>> +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING,
> >> BUT
> >>>> NOT
> >>>> +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
> >>>> FITNESS FOR
> >>>> +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> >>>> COPYRIGHT
> >>>> +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> >>>> INCIDENTAL,
> >>>> +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
> >> BUT
> >>>> NOT
> >>>> +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> >> LOSS
> >>>> OF USE,
> >>>> +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
> CAUSED
> >>>> AND ON ANY
> >>>> +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
> OR
> >>>> TORT
> >>>> +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> OUT
> >> OF
> >>>> THE USE
> >>>> +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> >>>> DAMAGE.
> >>>> +
> >>>> +include $(RTE_SDK)/mk/rte.vars.mk
> >>>> +
> >>>> +#
> >>>> +# library name
> >>>> +#
> >>>> +LIB = librte_pmd_kni.a
> >>>> +
> >>>> +CFLAGS += -O3
> >>>> +CFLAGS += $(WERROR_FLAGS)
> >>>> +LDLIBS += -lpthread
> >>>> +
> >>>> +EXPORT_MAP := rte_pmd_kni_version.map
> >>>> +
> >>>> +LIBABIVER := 1
> >>>> +
> >>>> +#
> >>>> +# all source are stored in SRCS-y
> >>>> +#
> >>>> +SRCS-$(CONFIG_RTE_LIBRTE_PMD_KNI) += rte_eth_kni.c
> >>>> +
> >>>> +#
> >>>> +# Export include files
> >>>> +#
> >>>> +SYMLINK-y-include +=
> >>>> +
> >>>> +# this lib depends upon:
> >>>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_KNI) += lib/librte_eal
> >>>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_KNI) += lib/librte_ether
> >>>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_KNI) += lib/librte_kni
> >>>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_KNI) += lib/librte_mbuf
> >>>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_KNI) += lib/librte_mempool
> >>>> +
> >>>> +include $(RTE_SDK)/mk/rte.lib.mk
> >>>> diff --git a/drivers/net/kni/rte_eth_kni.c
> b/drivers/net/kni/rte_eth_kni.c
> >>>> new file mode 100644
> >>>> index 0000000..6c4df96
> >>>> --- /dev/null
> >>>> +++ b/drivers/net/kni/rte_eth_kni.c
> >>>> @@ -0,0 +1,462 @@
> >>>> +/*-
> >>>> + * BSD LICENSE
> >>>> + *
> >>>> + * Copyright(c) 2016 Intel Corporation. All rights reserved.
> >>>> + * All rights reserved.
> >>>> + *
> >>>> + * Redistribution and use in source and binary forms, with or without
> >>>> + * modification, are permitted provided that the following conditions
> >>>> + * are met:
> >>>> + *
> >>>> + * * Redistributions of source code must retain the above copyright
> >>>> + * notice, this list of conditions and the following disclaimer.
> >>>> + * * Redistributions in binary form must reproduce the above
> copyright
> >>>> + * notice, this list of conditions and the following disclaimer in
> >>>> + * the documentation and/or other materials provided with the
> >>>> + * distribution.
> >>>> + * * Neither the name of Intel Corporation nor the names of its
> >>>> + * contributors may be used to endorse or promote products
> derived
> >>>> + * from this software without specific prior written permission.
> >>>> + *
> >>>> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> >>>> CONTRIBUTORS
> >>>> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING,
> >> BUT
> >>>> NOT
> >>>> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
> AND
> >>>> FITNESS FOR
> >>>> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
> THE
> >>>> COPYRIGHT
> >>>> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> >>>> INCIDENTAL,
> >>>> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> (INCLUDING,
> >> BUT
> >>>> NOT
> >>>> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> >> LOSS
> >>>> OF USE,
> >>>> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
> CAUSED
> >>>> AND ON ANY
> >>>> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
> OR
> >>>> TORT
> >>>> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> OUT
> >> OF
> >>>> THE USE
> >>>> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> >>>> DAMAGE.
> >>>> + */
> >>>> +
> >>>> +#include <fcntl.h>
> >>>> +#include <pthread.h>
> >>>> +#include <unistd.h>
> >>>> +
> >>>> +#include <rte_ethdev.h>
> >>>> +#include <rte_kni.h>
> >>>> +#include <rte_malloc.h>
> >>>> +#include <rte_vdev.h>
> >>>> +
> >>>> +/* Only single queue supported */
> >>>> +#define KNI_MAX_QUEUE_PER_PORT 1
> >>>> +
> >>>> +#define MAX_PACKET_SZ 2048
> >>>> +#define MAX_KNI_PORTS 8
> >>>> +
> >>>> +struct pmd_queue_stats {
> >>>> + uint64_t pkts;
> >>>> + uint64_t bytes;
> >>>> + uint64_t err_pkts;
> >>>> +};
> >>>> +
> >>>> +struct pmd_queue {
> >>>> + struct pmd_internals *internals;
> >>>> + struct rte_mempool *mb_pool;
> >>>> +
> >>>> + struct pmd_queue_stats rx;
> >>>> + struct pmd_queue_stats tx;
> >>>> +};
> >>>> +
> >>>> +struct pmd_internals {
> >>>> + struct rte_kni *kni;
> >>>> + int is_kni_started;
> >>>> +
> >>>> + pthread_t thread;
> >>>> + int stop_thread;
> >>>> +
> >>>> + struct pmd_queue rx_queues[KNI_MAX_QUEUE_PER_PORT];
> >>>> + struct pmd_queue tx_queues[KNI_MAX_QUEUE_PER_PORT];
> >>>> +};
> >>>> +
> >>>> +static struct ether_addr eth_addr;
> >>>> +static struct rte_eth_link pmd_link = {
> >>>> + .link_speed = ETH_SPEED_NUM_10G,
> >>>> + .link_duplex = ETH_LINK_FULL_DUPLEX,
> >>>> + .link_status = 0
> >>>> +};
> >>>> +static int is_kni_initialized;
> >>>> +
> >>>> +static uint16_t
> >>>> +eth_kni_rx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
> >>>> +{
> >>>> + struct pmd_queue *kni_q = q;
> >>>> + struct rte_kni *kni = kni_q->internals->kni;
> >>>> + uint16_t nb_pkts;
> >>>> +
> >>>> + nb_pkts = rte_kni_rx_burst(kni, bufs, nb_bufs);
> >>>> +
> >>>> + kni_q->rx.pkts += nb_pkts;
> >>>> + kni_q->rx.err_pkts += nb_bufs - nb_pkts;
> >>>> +
> >>>> + return nb_pkts;
> >>>> +}
> >>>> +
> >>>> +static uint16_t
> >>>> +eth_kni_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
> >>>> +{
> >>>> + struct pmd_queue *kni_q = q;
> >>>> + struct rte_kni *kni = kni_q->internals->kni;
> >>>> + uint16_t nb_pkts;
> >>>> +
> >>>> + nb_pkts = rte_kni_tx_burst(kni, bufs, nb_bufs);
> >>>> +
> >>>> + kni_q->tx.pkts += nb_pkts;
> >>>> + kni_q->tx.err_pkts += nb_bufs - nb_pkts;
> >>>> +
> >>>> + return nb_pkts;
> >>>> +}
> >>>> +
> >>>> +static void *
> >>>> +kni_handle_request(void *param)
> >>>> +{
> >>>> + struct pmd_internals *internals = param;
> >>>> +#define MS 1000
> >>>> +
> >>>> + while (!internals->stop_thread) {
> >>>> + rte_kni_handle_request(internals->kni);
> >>>> + usleep(500 * MS);
> >>>> + }
> >>>> +
> >>>> + return param;
> >>>> +}
> >>>> +
> >>>
> >>> Do we really need a thread to handle request by default? I know there
> are
> >> apps that handle request their own way and having a separate thread
> could
> >> add synchronization problems. Can we at least add an option to disable
> this?
> >>
> >> I didn't think about there can be a use case that requires own request
> >> handling.
> >>
> >> But, kni requests should be handled to make kni interface run properly,
> >> and to handle interface "kni" handler (internals->kni) required, which
> >> this PMD doesn't expose.
> >>
> >> So, just disabling this thread won't work on its own.
> >
> > I understand that and what I am asking is a way to at least disable this
> without having to make code changes for applications that have their own
> way of handling KNI request and the callback mentioned below sounds good
> to me. I am fine with adding this capability with this commit or in a separate
> commit after you have this commit checked in.
>
> I don't mind adding in new version, only I am trying to understand it.
>
> Normally what it does is calling KNI library rte_kni_handle_request()
> API periodically on KNI handler. What an app may be doing own its way,
> other than tweaking the period?
It's the context that calls into rte_kni_handle_request() that I am referring to. For applications that already handle this in their own thread or in the pmd thread, they don't need the extra thread created here. It's not a big deal as they can just change the behavior by modifying the source code but I think it's reasonable to opt out of this default thread without making any source code changes to kni pmd.
next prev parent reply other threads:[~2016-12-19 17:52 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-06 10:33 [dpdk-dev] [PATCH] " Ferruh Yigit
2016-09-08 7:44 ` Thomas Monjalon
2016-09-08 9:25 ` Bruce Richardson
2016-09-08 9:38 ` Thomas Monjalon
2016-09-08 18:11 ` Ferruh Yigit
2016-09-09 7:36 ` Thomas Monjalon
2016-09-16 11:29 ` [dpdk-dev] [PATCH v2] " Ferruh Yigit
2016-10-10 13:19 ` [dpdk-dev] [PATCH v3] " Ferruh Yigit
2016-11-03 1:24 ` Yong Wang
2016-11-04 12:21 ` Ferruh Yigit
2016-11-30 18:12 ` [dpdk-dev] [PATCH v4] " Ferruh Yigit
2016-12-12 21:59 ` Yong Wang
2016-12-14 15:59 ` Ferruh Yigit
2016-12-14 19:25 ` Yong Wang
2016-12-15 15:55 ` Ferruh Yigit
2016-12-19 17:52 ` Yong Wang [this message]
2017-01-30 16:57 ` [dpdk-dev] [PATCH v5] " Ferruh Yigit
2017-01-30 19:05 ` Yong Wang
2017-01-30 19:43 ` Ferruh Yigit
2017-01-30 20:09 ` [dpdk-dev] [PATCH v6] " Ferruh Yigit
2017-01-30 21:15 ` [dpdk-dev] [PATCH v7] " Ferruh Yigit
2017-01-31 12:18 ` [dpdk-dev] [PATCH v8] " Ferruh Yigit
2017-02-17 13:42 ` [dpdk-dev] [PATCH v9] " Ferruh Yigit
2017-02-17 13:47 ` Thomas Monjalon
2017-02-17 14:00 ` Eelco Chaudron
2017-02-17 14:29 ` Ferruh Yigit
2017-02-17 14:57 ` Bruce Richardson
2017-02-17 17:52 ` Yong Wang
2017-02-17 22:37 ` Thomas Monjalon
2017-02-20 12:54 ` Ferruh Yigit
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=BY2PR05MB23598EBBDA10658E67087BA0AF910@BY2PR05MB2359.namprd05.prod.outlook.com \
--to=yongwang@vmware.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.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).