[dpdk-dev] [PATCH v1 1/3] port: add mp/mc ring ports
> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Tuesday, September 22, 2015 5:24 PM
> To: Dumitrescu, Cristian
> Cc: dev at dpdk.org; Stephen Hemminger; Azarewicz, PiotrX T
> Subject: Re: [dpdk-dev] [PATCH v1 1/3] port: add mp/mc ring ports
>
> 2015-09-22 11:34, Dumitrescu, Cristian:
> >
> > > -Original Message-
> > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Stephen
> > > Hemminger
> > > Sent: Tuesday, September 22, 2015 1:36 AM
> > > To: Azarewicz, PiotrX T
> > > Cc: dev at dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH v1 1/3] port: add mp/mc ring ports
> > >
> > > On Tue, 15 Sep 2015 15:06:33 +0200
> > > Piotr Azarewicz wrote:
> > >
> > > > +static inline void
> > > > +send_burst_mp(struct rte_port_ring_writer *p)
> > > > +{
> > >
> > > compiler will inline static functions anyway. No need to add inline
> > > qualifier
> >
> > Hi Stephen,
> >
> > Using 'static inline' seems to be the standard practice in DPDK and a good
> practice as well.
>
> Why do you think it is a good practice?
> Forced inlining can be a random optimization having negative effects.
>
What I meant is this: when users want to make sure their code gets inlined by
the compiler, it is better to explicitly state this by using the mechanisms
provided by the C compiler (C keyword "inline" and compiler pragmas like
"always inline") rather than hope that compiler is going to do this anyway. I
have been burned in the past by compilers not inlining code even when
explicitly stated, so I am a quite sceptical about compilers doing it
proactively.
Your point is slightly different: why use code inlining at all? IMHO this
discussion is outside the scope of this patch and should be conducted as a
separate debate. Please feel free to start it as a separate thread if you deem
necessary. As said, there are already 1700 instances of "static inline" in
DPDK, as well as lots of "always inline".
In the context of this debate (outside the scope of this patch), my quick input
is: compilers are typically good to optimize code at the function level rather
than cross-functions, so having more code in the same function allows the
compiler to do a better job at code optimization. I am not a compiler expert,
so my views could simply be biased by my past experience.
> > DPDK> grep 'static inline' `find -name '*.[hc]'` | wc -l
> > 1700
[dpdk-dev] [PATCH v1 1/3] port: add mp/mc ring ports
2015-09-22 11:34, Dumitrescu, Cristian:
>
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Stephen
> > Hemminger
> > Sent: Tuesday, September 22, 2015 1:36 AM
> > To: Azarewicz, PiotrX T
> > Cc: dev at dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v1 1/3] port: add mp/mc ring ports
> >
> > On Tue, 15 Sep 2015 15:06:33 +0200
> > Piotr Azarewicz wrote:
> >
> > > +static inline void
> > > +send_burst_mp(struct rte_port_ring_writer *p)
> > > +{
> >
> > compiler will inline static functions anyway. No need to add inline
> > qualifier
>
> Hi Stephen,
>
> Using 'static inline' seems to be the standard practice in DPDK and a good
> practice as well.
Why do you think it is a good practice?
Forced inlining can be a random optimization having negative effects.
> DPDK> grep 'static inline' `find -name '*.[hc]'` | wc -l
> 1700
[dpdk-dev] [PATCH v1 1/3] port: add mp/mc ring ports
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Stephen
> Hemminger
> Sent: Tuesday, September 22, 2015 1:35 AM
> To: Azarewicz, PiotrX T
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v1 1/3] port: add mp/mc ring ports
>
> On Tue, 15 Sep 2015 15:06:33 +0200
> Piotr Azarewicz wrote:
>
> > +static int
> > +rte_port_ring_multi_reader_rx(void *port, struct rte_mbuf **pkts,
> uint32_t n_pkts)
> > +{
>
> Please break arguments on line so that line length is not over 80 characters.
Thanks, Steve.
[dpdk-dev] [PATCH v1 1/3] port: add mp/mc ring ports
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Stephen > Hemminger > Sent: Tuesday, September 22, 2015 1:37 AM > To: Azarewicz, PiotrX T > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v1 1/3] port: add mp/mc ring ports > > On Tue, 15 Sep 2015 15:06:33 +0200 > Piotr Azarewicz wrote: > > > + /* > > +* If we didnt manage to send all packets in single burst, > move > > Checkpatch complains: > WARNING: 'didnt' may be misspelled - perhaps 'didn't'? > #413: FILE: lib/librte_port/rte_port_ring.c:827: > + * If we didnt manage to send all packets in single burst, > move Thanks for the catch, Stephen.
[dpdk-dev] [PATCH v1 1/3] port: add mp/mc ring ports
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Stephen
> Hemminger
> Sent: Tuesday, September 22, 2015 1:36 AM
> To: Azarewicz, PiotrX T
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v1 1/3] port: add mp/mc ring ports
>
> On Tue, 15 Sep 2015 15:06:33 +0200
> Piotr Azarewicz wrote:
>
> > +static inline void
> > +send_burst_mp(struct rte_port_ring_writer *p)
> > +{
>
> compiler will inline static functions anyway. No need to add inline qualifier
Hi Stephen,
Using 'static inline' seems to be the standard practice in DPDK and a good
practice as well.
DPDK> grep 'static inline' `find -name '*.[hc]'` | wc -l
1700
Regards,
Cristian
[dpdk-dev] [PATCH v1 1/3] port: add mp/mc ring ports
On Tue, 22 Sep 2015 16:23:51 +0200
Thomas Monjalon wrote:
> 2015-09-22 11:34, Dumitrescu, Cristian:
> >
> > > -Original Message-
> > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Stephen
> > > Hemminger
> > > Sent: Tuesday, September 22, 2015 1:36 AM
> > > To: Azarewicz, PiotrX T
> > > Cc: dev at dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH v1 1/3] port: add mp/mc ring ports
> > >
> > > On Tue, 15 Sep 2015 15:06:33 +0200
> > > Piotr Azarewicz wrote:
> > >
> > > > +static inline void
> > > > +send_burst_mp(struct rte_port_ring_writer *p)
> > > > +{
> > >
> > > compiler will inline static functions anyway. No need to add inline
> > > qualifier
> >
> > Hi Stephen,
> >
> > Using 'static inline' seems to be the standard practice in DPDK and a good
> > practice as well.
>
> Why do you think it is a good practice?
> Forced inlining can be a random optimization having negative effects.
Agreed. Modern compilers make good decisions.
[dpdk-dev] [PATCH v1 1/3] port: add mp/mc ring ports
On Tue, 15 Sep 2015 15:06:33 +0200 Piotr Azarewicz wrote: > + /* > + * If we didnt manage to send all packets in single burst, move Checkpatch complains: WARNING: 'didnt' may be misspelled - perhaps 'didn't'? #413: FILE: lib/librte_port/rte_port_ring.c:827: +* If we didnt manage to send all packets in single burst, move
[dpdk-dev] [PATCH v1 1/3] port: add mp/mc ring ports
On Tue, 15 Sep 2015 15:06:33 +0200
Piotr Azarewicz wrote:
> +static inline void
> +send_burst_mp(struct rte_port_ring_writer *p)
> +{
compiler will inline static functions anyway. No need to add inline qualifier
[dpdk-dev] [PATCH v1 1/3] port: add mp/mc ring ports
On Tue, 15 Sep 2015 15:06:33 +0200
Piotr Azarewicz wrote:
> +static int
> +rte_port_ring_multi_reader_rx(void *port, struct rte_mbuf **pkts, uint32_t
> n_pkts)
> +{
Please break arguments on line so that line length is not over 80 characters.
[dpdk-dev] [PATCH v1 1/3] port: add mp/mc ring ports
ring_multi_reader input port (on top of multi consumer rte_ring)
ring_multi_writer output port (on top of multi producer rte_ring)
Signed-off-by: Piotr Azarewicz
---
lib/librte_port/rte_port_ring.c | 399 ++-
lib/librte_port/rte_port_ring.h | 34 +++-
2 files changed, 424 insertions(+), 9 deletions(-)
diff --git a/lib/librte_port/rte_port_ring.c b/lib/librte_port/rte_port_ring.c
index 9461c05..6b06466 100644
--- a/lib/librte_port/rte_port_ring.c
+++ b/lib/librte_port/rte_port_ring.c
@@ -70,8 +70,10 @@ rte_port_ring_reader_create(void *params, int socket_id)
struct rte_port_ring_reader *port;
/* Check input parameters */
- if (conf == NULL) {
- RTE_LOG(ERR, PORT, "%s: params is NULL\n", __func__);
+ if ((conf == NULL) ||
+ (conf->ring == NULL) ||
+ (conf->ring->cons.sc_dequeue != 1)) {
+ RTE_LOG(ERR, PORT, "%s: Invalid Parameters\n", __func__);
return NULL;
}
@@ -166,7 +168,8 @@ rte_port_ring_writer_create(void *params, int socket_id)
/* Check input parameters */
if ((conf == NULL) ||
- (conf->ring == NULL) ||
+ (conf->ring == NULL) ||
+ (conf->ring->prod.sp_enqueue != 1) ||
(conf->tx_burst_sz > RTE_PORT_IN_BURST_SIZE_MAX)) {
RTE_LOG(ERR, PORT, "%s: Invalid Parameters\n", __func__);
return NULL;
@@ -343,7 +346,8 @@ rte_port_ring_writer_nodrop_create(void *params, int
socket_id)
/* Check input parameters */
if ((conf == NULL) ||
- (conf->ring == NULL) ||
+ (conf->ring == NULL) ||
+ (conf->ring->prod.sp_enqueue != 1) ||
(conf->tx_burst_sz > RTE_PORT_IN_BURST_SIZE_MAX)) {
RTE_LOG(ERR, PORT, "%s: Invalid Parameters\n", __func__);
return NULL;
@@ -448,6 +452,7 @@ rte_port_ring_writer_nodrop_tx_bulk(void *port,
*/
for (; n_pkts_ok < n_pkts; n_pkts_ok++) {
struct rte_mbuf *pkt = pkts[n_pkts_ok];
+
p->tx_buf[p->tx_buf_count++] = pkt;
}
send_burst_nodrop(p);
@@ -513,6 +518,367 @@ rte_port_ring_writer_nodrop_stats_read(void *port,
}
/*
+ * Port RING Multi Reader
+ */
+static void *
+rte_port_ring_multi_reader_create(void *params, int socket_id)
+{
+ struct rte_port_ring_multi_reader_params *conf =
+ (struct rte_port_ring_multi_reader_params *) params;
+ struct rte_port_ring_reader *port;
+
+ /* Check input parameters */
+ if ((conf == NULL) ||
+ (conf->ring == NULL) ||
+ (conf->ring->cons.sc_dequeue != 0)) {
+ RTE_LOG(ERR, PORT, "%s: Invalid Parameters\n", __func__);
+ return NULL;
+ }
+
+ /* Memory allocation */
+ port = rte_zmalloc_socket("PORT", sizeof(*port),
+ RTE_CACHE_LINE_SIZE, socket_id);
+ if (port == NULL) {
+ RTE_LOG(ERR, PORT, "%s: Failed to allocate port\n", __func__);
+ return NULL;
+ }
+
+ /* Initialization */
+ port->ring = conf->ring;
+
+ return port;
+}
+
+static int
+rte_port_ring_multi_reader_rx(void *port, struct rte_mbuf **pkts, uint32_t
n_pkts)
+{
+ struct rte_port_ring_reader *p = (struct rte_port_ring_reader *) port;
+ uint32_t nb_rx;
+
+ nb_rx = rte_ring_mc_dequeue_burst(p->ring, (void **) pkts, n_pkts);
+ RTE_PORT_RING_READER_STATS_PKTS_IN_ADD(p, nb_rx);
+
+ return nb_rx;
+}
+
+static int
+rte_port_ring_multi_reader_free(void *port)
+{
+ if (port == NULL) {
+ RTE_LOG(ERR, PORT, "%s: port is NULL\n", __func__);
+ return -EINVAL;
+ }
+
+ rte_free(port);
+
+ return 0;
+}
+
+/*
+ * Port RING Multi Writer
+ */
+static void *
+rte_port_ring_multi_writer_create(void *params, int socket_id)
+{
+ struct rte_port_ring_multi_writer_params *conf =
+ (struct rte_port_ring_multi_writer_params *) params;
+ struct rte_port_ring_writer *port;
+
+ /* Check input parameters */
+ if ((conf == NULL) ||
+ (conf->ring == NULL) ||
+ (conf->ring->prod.sp_enqueue != 0) ||
+ (conf->tx_burst_sz > RTE_PORT_IN_BURST_SIZE_MAX)) {
+ RTE_LOG(ERR, PORT, "%s: Invalid Parameters\n", __func__);
+ return NULL;
+ }
+
+ /* Memory allocation */
+ port = rte_zmalloc_socket("PORT", sizeof(*port),
+ RTE_CACHE_LINE_SIZE, socket_id);
+ if (port == NULL) {
+ RTE_LOG(ERR, PORT, "%s: Failed to allocate port\n", __func__);
+ return NULL;
+ }
+
+ /* Initialization */
+ port->ring = conf->ring;
+ port->tx_burst_sz = conf->tx_burst_sz;
+ port->tx_buf_count = 0;
+ port->bsz_ma
