* [PATCH 1/2] net/tap: remove redundant syscall on Tx @ 2017-03-14 12:51 Adrien Mazarguil 2017-03-14 12:51 ` [PATCH 2/2] net/tap: add Rx trigger Adrien Mazarguil 2017-03-14 13:52 ` [PATCH 1/2] net/tap: remove redundant syscall on Tx Wiles, Keith 0 siblings, 2 replies; 5+ messages in thread From: Adrien Mazarguil @ 2017-03-14 12:51 UTC (permalink / raw) To: Keith Wiles; +Cc: dev Polling the Tx queue file descriptor before writing to it is not mandatory since it is configured as non-blocking. Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com> Acked-by: Pascal Mazon <pascal.mazon@6wind.com> --- drivers/net/tap/rte_eth_tap.c | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c index efc4426..c757a7c 100644 --- a/drivers/net/tap/rte_eth_tap.c +++ b/drivers/net/tap/rte_eth_tap.c @@ -43,7 +43,6 @@ #include <sys/ioctl.h> #include <sys/mman.h> #include <unistd.h> -#include <poll.h> #include <arpa/inet.h> #include <linux/if.h> #include <linux/if_tun.h> @@ -242,7 +241,6 @@ pmd_tx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) { struct rte_mbuf *mbuf; struct tx_queue *txq = queue; - struct pollfd pfd; uint16_t num_tx = 0; unsigned long num_tx_bytes = 0; int i, n; @@ -250,26 +248,18 @@ pmd_tx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) if (unlikely(nb_pkts == 0)) return 0; - pfd.events = POLLOUT; - pfd.fd = txq->fd; for (i = 0; i < nb_pkts; i++) { - n = poll(&pfd, 1, 0); - + /* copy the tx frame data */ + mbuf = bufs[num_tx]; + n = write(txq->fd, + rte_pktmbuf_mtod(mbuf, void *), + rte_pktmbuf_pkt_len(mbuf)); if (n <= 0) break; - if (pfd.revents & POLLOUT) { - /* copy the tx frame data */ - mbuf = bufs[num_tx]; - n = write(pfd.fd, rte_pktmbuf_mtod(mbuf, void*), - rte_pktmbuf_pkt_len(mbuf)); - if (n <= 0) - break; - - num_tx++; - num_tx_bytes += mbuf->pkt_len; - rte_pktmbuf_free(mbuf); - } + num_tx++; + num_tx_bytes += mbuf->pkt_len; + rte_pktmbuf_free(mbuf); } txq->stats.opackets += num_tx; -- 2.1.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] net/tap: add Rx trigger 2017-03-14 12:51 [PATCH 1/2] net/tap: remove redundant syscall on Tx Adrien Mazarguil @ 2017-03-14 12:51 ` Adrien Mazarguil 2017-03-14 13:57 ` Wiles, Keith 2017-03-14 13:52 ` [PATCH 1/2] net/tap: remove redundant syscall on Tx Wiles, Keith 1 sibling, 1 reply; 5+ messages in thread From: Adrien Mazarguil @ 2017-03-14 12:51 UTC (permalink / raw) To: Keith Wiles; +Cc: dev This commit adds a signal-based trigger to the Rx burst function in order to avoid unnecessary system calls while Rx queues are empty. Triggered Rx bursts put less pressure on the kernel, free up CPU resources for applications and result in a noticeable performance improvement when sharing CPU threads with other PMDs. 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: Without Rx trigger: -15% (--burst=32) -62% (--burst=1) With Rx trigger: -0.3% (--burst=32) -6% (--burst=1) Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com> Acked-by: Pascal Mazon <pascal.mazon@6wind.com> --- drivers/net/tap/rte_eth_tap.c | 59 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) 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. */ +#include <rte_atomic.h> +#include <rte_common.h> #include <rte_mbuf.h> #include <rte_ethdev.h> #include <rte_malloc.h> @@ -42,6 +44,9 @@ #include <sys/socket.h> #include <sys/ioctl.h> #include <sys/mman.h> +#include <errno.h> +#include <signal.h> +#include <stdint.h> #include <unistd.h> #include <arpa/inet.h> #include <linux/if.h> @@ -72,6 +77,8 @@ static const char *valid_arguments[] = { static int tap_unit; +static volatile uint32_t tap_trigger; /* Rx trigger */ + static struct rte_eth_link pmd_link = { .link_speed = ETH_SPEED_NUM_10G, .link_duplex = ETH_LINK_FULL_DUPLEX, @@ -89,6 +96,7 @@ struct pkt_stats { 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; @@ -111,6 +119,13 @@ struct pmd_internals { struct tx_queue txq[RTE_PMD_TAP_MAX_QUEUES]; /* List of TX queues */ }; +static void +tap_trigger_cb(int sig __rte_unused) +{ + /* Valid trigger values are nonzero */ + tap_trigger = (tap_trigger + 1) | 0x80000000; +} + /* Tun/Tap allocation routine * * name is the number of the interface to use, unless NULL to take the host @@ -175,6 +190,43 @@ tun_alloc(struct pmd_internals *pmd, uint16_t qid) goto error; } + /* Set up trigger to optimize empty Rx bursts */ + errno = 0; + do { + struct sigaction sa; + int flags = fcntl(fd, F_GETFL); + + if (flags == -1 || sigaction(SIGIO, NULL, &sa) == -1) + break; + if (sa.sa_handler != 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 != SIG_IGN && + sa.sa_handler != SIG_DFL) { + errno = EBUSY; + break; + } + sa = (struct sigaction){ + .sa_flags = SA_RESTART, + .sa_handler = tap_trigger_cb, + }; + if (sigaction(SIGIO, &sa, NULL) == -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 = 0; + RTE_LOG(WARNING, PMD, "Rx trigger disabled: %s\n", + strerror(errno)); + } + if (qid == 0) { if (ioctl(fd, SIOCGIFHWADDR, &ifr) == -1) { RTE_LOG(ERR, PMD, "ioctl failed (SIOCGIFHWADDR) (%s)\n", @@ -204,7 +256,13 @@ pmd_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) struct rx_queue *rxq = queue; uint16_t num_rx; unsigned long num_rx_bytes = 0; + uint32_t trigger = tap_trigger; + if (trigger == rxq->trigger_seen) + return 0; + if (trigger) + rxq->trigger_seen = trigger; + rte_compiler_barrier(); for (num_rx = 0; num_rx < nb_pkts; ) { /* allocate the next mbuf */ mbuf = rte_pktmbuf_alloc(rxq->mp); @@ -563,6 +621,7 @@ tap_rx_queue_setup(struct rte_eth_dev *dev, } internals->rxq[rx_queue_id].mp = mp; + internals->rxq[rx_queue_id].trigger_seen = 1; /* force initial burst */ internals->rxq[rx_queue_id].in_port = dev->data->port_id; /* Now get the space available for data in the mbuf */ -- 2.1.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] net/tap: add Rx trigger 2017-03-14 12:51 ` [PATCH 2/2] net/tap: add Rx trigger Adrien Mazarguil @ 2017-03-14 13:57 ` Wiles, Keith 0 siblings, 0 replies; 5+ messages in thread From: Wiles, Keith @ 2017-03-14 13:57 UTC (permalink / raw) To: Adrien Mazarguil; +Cc: dev > On Mar 14, 2017, at 8:51 PM, Adrien Mazarguil <adrien.mazarguil@6wind.com> wrote: > > This commit adds a signal-based trigger to the Rx burst function in order > to avoid unnecessary system calls while Rx queues are empty. > > Triggered Rx bursts put less pressure on the kernel, free up CPU resources > for applications and result in a noticeable performance improvement when > sharing CPU threads with other PMDs. > > 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: > > Without Rx trigger: > > -15% (--burst=32) > -62% (--burst=1) > > With Rx trigger: > > -0.3% (--burst=32) > -6% (--burst=1) > > Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com> > Acked-by: Pascal Mazon <pascal.mazon@6wind.com> Acked-by: Keith Wiles <keith.wiles@intel.com> > --- > drivers/net/tap/rte_eth_tap.c | 59 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 59 insertions(+) > > 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. > */ > > +#include <rte_atomic.h> > +#include <rte_common.h> > #include <rte_mbuf.h> > #include <rte_ethdev.h> > #include <rte_malloc.h> > @@ -42,6 +44,9 @@ > #include <sys/socket.h> > #include <sys/ioctl.h> > #include <sys/mman.h> > +#include <errno.h> > +#include <signal.h> > +#include <stdint.h> > #include <unistd.h> > #include <arpa/inet.h> > #include <linux/if.h> > @@ -72,6 +77,8 @@ static const char *valid_arguments[] = { > > static int tap_unit; > > +static volatile uint32_t tap_trigger; /* Rx trigger */ > + > static struct rte_eth_link pmd_link = { > .link_speed = ETH_SPEED_NUM_10G, > .link_duplex = ETH_LINK_FULL_DUPLEX, > @@ -89,6 +96,7 @@ struct pkt_stats { > > 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; > > @@ -111,6 +119,13 @@ struct pmd_internals { > struct tx_queue txq[RTE_PMD_TAP_MAX_QUEUES]; /* List of TX queues */ > }; > > +static void > +tap_trigger_cb(int sig __rte_unused) > +{ > + /* Valid trigger values are nonzero */ > + tap_trigger = (tap_trigger + 1) | 0x80000000; > +} > + > /* Tun/Tap allocation routine > * > * name is the number of the interface to use, unless NULL to take the host > @@ -175,6 +190,43 @@ tun_alloc(struct pmd_internals *pmd, uint16_t qid) > goto error; > } > > + /* Set up trigger to optimize empty Rx bursts */ > + errno = 0; > + do { > + struct sigaction sa; > + int flags = fcntl(fd, F_GETFL); > + > + if (flags == -1 || sigaction(SIGIO, NULL, &sa) == -1) > + break; > + if (sa.sa_handler != 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 != SIG_IGN && > + sa.sa_handler != SIG_DFL) { > + errno = EBUSY; > + break; > + } > + sa = (struct sigaction){ > + .sa_flags = SA_RESTART, > + .sa_handler = tap_trigger_cb, > + }; > + if (sigaction(SIGIO, &sa, NULL) == -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 = 0; > + RTE_LOG(WARNING, PMD, "Rx trigger disabled: %s\n", > + strerror(errno)); > + } > + > if (qid == 0) { > if (ioctl(fd, SIOCGIFHWADDR, &ifr) == -1) { > RTE_LOG(ERR, PMD, "ioctl failed (SIOCGIFHWADDR) (%s)\n", > @@ -204,7 +256,13 @@ pmd_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) > struct rx_queue *rxq = queue; > uint16_t num_rx; > unsigned long num_rx_bytes = 0; > + uint32_t trigger = tap_trigger; > > + if (trigger == rxq->trigger_seen) > + return 0; > + if (trigger) > + rxq->trigger_seen = trigger; > + rte_compiler_barrier(); > for (num_rx = 0; num_rx < nb_pkts; ) { > /* allocate the next mbuf */ > mbuf = rte_pktmbuf_alloc(rxq->mp); > @@ -563,6 +621,7 @@ tap_rx_queue_setup(struct rte_eth_dev *dev, > } > > internals->rxq[rx_queue_id].mp = mp; > + internals->rxq[rx_queue_id].trigger_seen = 1; /* force initial burst */ > internals->rxq[rx_queue_id].in_port = dev->data->port_id; > > /* Now get the space available for data in the mbuf */ > -- > 2.1.4 > Regards, Keith ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] net/tap: remove redundant syscall on Tx 2017-03-14 12:51 [PATCH 1/2] net/tap: remove redundant syscall on Tx Adrien Mazarguil 2017-03-14 12:51 ` [PATCH 2/2] net/tap: add Rx trigger Adrien Mazarguil @ 2017-03-14 13:52 ` Wiles, Keith 2017-03-14 17:04 ` Ferruh Yigit 1 sibling, 1 reply; 5+ messages in thread From: Wiles, Keith @ 2017-03-14 13:52 UTC (permalink / raw) To: Adrien Mazarguil; +Cc: dev > On Mar 14, 2017, at 8:51 PM, Adrien Mazarguil <adrien.mazarguil@6wind.com> wrote: > > Polling the Tx queue file descriptor before writing to it is not mandatory > since it is configured as non-blocking. > > Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com> > Acked-by: Pascal Mazon <pascal.mazon@6wind.com> Acked-by: Keith Wiles <keith.wiles@intel.com> > --- > drivers/net/tap/rte_eth_tap.c | 26 ++++++++------------------ > 1 file changed, 8 insertions(+), 18 deletions(-) > > diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c > index efc4426..c757a7c 100644 > --- a/drivers/net/tap/rte_eth_tap.c > +++ b/drivers/net/tap/rte_eth_tap.c > @@ -43,7 +43,6 @@ > #include <sys/ioctl.h> > #include <sys/mman.h> > #include <unistd.h> > -#include <poll.h> > #include <arpa/inet.h> > #include <linux/if.h> > #include <linux/if_tun.h> > @@ -242,7 +241,6 @@ pmd_tx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) > { > struct rte_mbuf *mbuf; > struct tx_queue *txq = queue; > - struct pollfd pfd; > uint16_t num_tx = 0; > unsigned long num_tx_bytes = 0; > int i, n; > @@ -250,26 +248,18 @@ pmd_tx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) > if (unlikely(nb_pkts == 0)) > return 0; > > - pfd.events = POLLOUT; > - pfd.fd = txq->fd; > for (i = 0; i < nb_pkts; i++) { > - n = poll(&pfd, 1, 0); > - > + /* copy the tx frame data */ > + mbuf = bufs[num_tx]; > + n = write(txq->fd, > + rte_pktmbuf_mtod(mbuf, void *), > + rte_pktmbuf_pkt_len(mbuf)); > if (n <= 0) > break; > > - if (pfd.revents & POLLOUT) { > - /* copy the tx frame data */ > - mbuf = bufs[num_tx]; > - n = write(pfd.fd, rte_pktmbuf_mtod(mbuf, void*), > - rte_pktmbuf_pkt_len(mbuf)); > - if (n <= 0) > - break; > - > - num_tx++; > - num_tx_bytes += mbuf->pkt_len; > - rte_pktmbuf_free(mbuf); > - } > + num_tx++; > + num_tx_bytes += mbuf->pkt_len; > + rte_pktmbuf_free(mbuf); > } > > txq->stats.opackets += num_tx; > -- > 2.1.4 > Regards, Keith ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] net/tap: remove redundant syscall on Tx 2017-03-14 13:52 ` [PATCH 1/2] net/tap: remove redundant syscall on Tx Wiles, Keith @ 2017-03-14 17:04 ` Ferruh Yigit 0 siblings, 0 replies; 5+ messages in thread From: Ferruh Yigit @ 2017-03-14 17:04 UTC (permalink / raw) To: Wiles, Keith, Adrien Mazarguil; +Cc: dev On 3/14/2017 1:52 PM, Wiles, Keith wrote: > >> On Mar 14, 2017, at 8:51 PM, Adrien Mazarguil <adrien.mazarguil@6wind.com> wrote: >> >> Polling the Tx queue file descriptor before writing to it is not mandatory >> since it is configured as non-blocking. >> >> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com> >> Acked-by: Pascal Mazon <pascal.mazon@6wind.com> > > Acked-by: Keith Wiles <keith.wiles@intel.com> Series applied to dpdk-next-net/master, thanks. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-03-14 17:04 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-03-14 12:51 [PATCH 1/2] net/tap: remove redundant syscall on Tx Adrien Mazarguil 2017-03-14 12:51 ` [PATCH 2/2] net/tap: add Rx trigger Adrien Mazarguil 2017-03-14 13:57 ` Wiles, Keith 2017-03-14 13:52 ` [PATCH 1/2] net/tap: remove redundant syscall on Tx Wiles, Keith 2017-03-14 17:04 ` Ferruh Yigit
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.