From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Wiles, Keith" Subject: Re: [PATCH 2/2] net/tap: add Rx trigger Date: Tue, 14 Mar 2017 13:57:19 +0000 Message-ID: References: <2f12b366764d27eb3938781e97bdd42c9f1b7e3a.1489495663.git.adrien.mazarguil@6wind.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" To: Adrien Mazarguil Return-path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id E7A522C74 for ; Tue, 14 Mar 2017 14:57:33 +0100 (CET) In-Reply-To: <2f12b366764d27eb3938781e97bdd42c9f1b7e3a.1489495663.git.adrien.mazarguil@6wind.com> Content-Language: en-US Content-ID: <75FDA8811510EB48A2350FECCC89E761@intel.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > On Mar 14, 2017, at 8:51 PM, Adrien Mazarguil wrote: >=20 > This commit adds a signal-based trigger to the Rx burst function in order > to avoid unnecessary system calls while Rx queues are empty. >=20 > Triggered Rx bursts put less pressure on the kernel, free up CPU resource= s > for applications and result in a noticeable performance improvement when > sharing CPU threads with other PMDs. >=20 > Measuring the traffic forwarding rate between two physical devices in > testpmd (IO mode, single thread, 64B packets) before and after adding two > tap PMD instances (4 ports total) that do not process any traffic and > comparing results yields: >=20 > Without Rx trigger: >=20 > -15% (--burst=3D32) > -62% (--burst=3D1) >=20 > With Rx trigger: >=20 > -0.3% (--burst=3D32) > -6% (--burst=3D1) >=20 > Signed-off-by: Adrien Mazarguil > Acked-by: Pascal Mazon Acked-by: Keith Wiles > --- > drivers/net/tap/rte_eth_tap.c | 59 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 59 insertions(+) >=20 > diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.= c > index c757a7c..d5d467a 100644 > --- a/drivers/net/tap/rte_eth_tap.c > +++ b/drivers/net/tap/rte_eth_tap.c > @@ -31,6 +31,8 @@ > * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > */ >=20 > +#include > +#include > #include > #include > #include > @@ -42,6 +44,9 @@ > #include > #include > #include > +#include > +#include > +#include > #include > #include > #include > @@ -72,6 +77,8 @@ static const char *valid_arguments[] =3D { >=20 > static int tap_unit; >=20 > +static volatile uint32_t tap_trigger; /* Rx trigger */ > + > static struct rte_eth_link pmd_link =3D { > .link_speed =3D ETH_SPEED_NUM_10G, > .link_duplex =3D ETH_LINK_FULL_DUPLEX, > @@ -89,6 +96,7 @@ struct pkt_stats { >=20 > struct rx_queue { > struct rte_mempool *mp; /* Mempool for RX packets */ > + uint32_t trigger_seen; /* Last seen Rx trigger value */ > uint16_t in_port; /* Port ID */ > int fd; >=20 > @@ -111,6 +119,13 @@ struct pmd_internals { > struct tx_queue txq[RTE_PMD_TAP_MAX_QUEUES]; /* List of TX queues */ > }; >=20 > +static void > +tap_trigger_cb(int sig __rte_unused) > +{ > + /* Valid trigger values are nonzero */ > + tap_trigger =3D (tap_trigger + 1) | 0x80000000; > +} > + > /* Tun/Tap allocation routine > * > * name is the number of the interface to use, unless NULL to take the ho= st > @@ -175,6 +190,43 @@ tun_alloc(struct pmd_internals *pmd, uint16_t qid) > goto error; > } >=20 > + /* Set up trigger to optimize empty Rx bursts */ > + errno =3D 0; > + do { > + struct sigaction sa; > + int flags =3D fcntl(fd, F_GETFL); > + > + if (flags =3D=3D -1 || sigaction(SIGIO, NULL, &sa) =3D=3D -1) > + break; > + if (sa.sa_handler !=3D tap_trigger_cb) { > + /* > + * Make sure SIGIO is not already taken. This is done > + * as late as possible to leave the application a > + * chance to set up its own signal handler first. > + */ > + if (sa.sa_handler !=3D SIG_IGN && > + sa.sa_handler !=3D SIG_DFL) { > + errno =3D EBUSY; > + break; > + } > + sa =3D (struct sigaction){ > + .sa_flags =3D SA_RESTART, > + .sa_handler =3D tap_trigger_cb, > + }; > + if (sigaction(SIGIO, &sa, NULL) =3D=3D -1) > + break; > + } > + /* Enable SIGIO on file descriptor */ > + fcntl(fd, F_SETFL, flags | O_ASYNC); > + fcntl(fd, F_SETOWN, getpid()); > + } while (0); > + if (errno) { > + /* Disable trigger globally in case of error */ > + tap_trigger =3D 0; > + RTE_LOG(WARNING, PMD, "Rx trigger disabled: %s\n", > + strerror(errno)); > + } > + > if (qid =3D=3D 0) { > if (ioctl(fd, SIOCGIFHWADDR, &ifr) =3D=3D -1) { > RTE_LOG(ERR, PMD, "ioctl failed (SIOCGIFHWADDR) (%s)\n", > @@ -204,7 +256,13 @@ pmd_rx_burst(void *queue, struct rte_mbuf **bufs, ui= nt16_t nb_pkts) > struct rx_queue *rxq =3D queue; > uint16_t num_rx; > unsigned long num_rx_bytes =3D 0; > + uint32_t trigger =3D tap_trigger; >=20 > + if (trigger =3D=3D rxq->trigger_seen) > + return 0; > + if (trigger) > + rxq->trigger_seen =3D trigger; > + rte_compiler_barrier(); > for (num_rx =3D 0; num_rx < nb_pkts; ) { > /* allocate the next mbuf */ > mbuf =3D rte_pktmbuf_alloc(rxq->mp); > @@ -563,6 +621,7 @@ tap_rx_queue_setup(struct rte_eth_dev *dev, > } >=20 > internals->rxq[rx_queue_id].mp =3D mp; > + internals->rxq[rx_queue_id].trigger_seen =3D 1; /* force initial burst = */ > internals->rxq[rx_queue_id].in_port =3D dev->data->port_id; >=20 > /* Now get the space available for data in the mbuf */ > --=20 > 2.1.4 >=20 Regards, Keith