[dpdk-dev,3/3] port: code clean-up
Commit Message
This patch clean-up the code in librte_port. The clean-up includes the
following:
* Clearer error message display.
* Remove unnecessary RTE_NEXT_ABI macro warping.
* Remove __rte_unused attribute
Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
---
lib/librte_port/Makefile | 5 --
lib/librte_port/rte_port_source_sink.c | 102 +++++++++++----------------------
mk/rte.app.mk | 2 -
3 files changed, 33 insertions(+), 76 deletions(-)
Comments
2016-04-01 14:41, Fan Zhang:
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -92,9 +92,7 @@ endif
> ifeq ($(CONFIG_RTE_LIBRTE_VHOST_USER),n)
> _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lfuse
> endif
> -ifeq ($(CONFIG_RTE_NEXT_ABI),y)
> _LDLIBS-$(CONFIG_RTE_PORT_PCAP) += -lpcap
> -endif
Why removing NEXT_ABI here?
The pcap port cannot work without it because of
+#ifdef RTE_NEXT_ABI
+
+ /** The full path of the pcap file to read packets from */
+ char *file_name;
+ /** The number of bytes to be read from each packet in the
+ * pcap file. If this value is 0, the whole packet is read;
+ * if it is bigger than packet size, the generated packets
+ * will contain the whole packet */
+ uint32_t n_bytes_per_pkt;
+
+#endif
Hi Thomas,
The reason for removing RTE_NEXT_ABI here is caused by sink port, which
is not wrapped by RTE_NEXT_ABI macro.
If the user disable RTE_NEXT_ABI but enable RTE_PORT_PCAP, the original
code will cause the compile error as pcap library is missing.
Regards,
Fan
On 01/04/2016 14:56, Thomas Monjalon wrote:
> 2016-04-01 14:41, Fan Zhang:
>> --- a/mk/rte.app.mk
>> +++ b/mk/rte.app.mk
>> @@ -92,9 +92,7 @@ endif
>> ifeq ($(CONFIG_RTE_LIBRTE_VHOST_USER),n)
>> _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lfuse
>> endif
>> -ifeq ($(CONFIG_RTE_NEXT_ABI),y)
>> _LDLIBS-$(CONFIG_RTE_PORT_PCAP) += -lpcap
>> -endif
> Why removing NEXT_ABI here?
> The pcap port cannot work without it because of
>
> +#ifdef RTE_NEXT_ABI
> +
> + /** The full path of the pcap file to read packets from */
> + char *file_name;
> + /** The number of bytes to be read from each packet in the
> + * pcap file. If this value is 0, the whole packet is read;
> + * if it is bigger than packet size, the generated packets
> + * will contain the whole packet */
> + uint32_t n_bytes_per_pkt;
> +
> +#endif
>
Hi Thomas,
The reason for removing RTE_NEXT_ABI here is caused by sink port, which is not wrapped by RTE_NEXT_ABI macro.
If the user disable RTE_NEXT_ABI but enable RTE_PORT_PCAP, the original code will cause the compile error as pcap library is missing.
Regards,
Fan
-----Original Message-----
From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
Sent: Friday, April 1, 2016 2:56 PM
To: Zhang, Roy Fan <roy.fan.zhang@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 3/3] port: code clean-up
2016-04-01 14:41, Fan Zhang:
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -92,9 +92,7 @@ endif
> ifeq ($(CONFIG_RTE_LIBRTE_VHOST_USER),n)
> _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lfuse
> endif
> -ifeq ($(CONFIG_RTE_NEXT_ABI),y)
> _LDLIBS-$(CONFIG_RTE_PORT_PCAP) += -lpcap
> -endif
Why removing NEXT_ABI here?
The pcap port cannot work without it because of
+#ifdef RTE_NEXT_ABI
+
+ /** The full path of the pcap file to read packets from */
+ char *file_name;
+ /** The number of bytes to be read from each packet in the
+ * pcap file. If this value is 0, the whole packet is read;
+ * if it is bigger than packet size, the generated packets
+ * will contain the whole packet */
+ uint32_t n_bytes_per_pkt;
+
+#endif
@@ -35,15 +35,10 @@ include $(RTE_SDK)/mk/rte.vars.mk
# library name
#
LIB = librte_port.a
-
-ifeq ($(CONFIG_RTE_NEXT_ABI),y)
-
ifeq ($(CONFIG_RTE_PORT_PCAP),y)
LDLIBS += -lpcap
endif
-endif
-
CFLAGS += -O3
CFLAGS += $(WERROR_FLAGS)
@@ -36,7 +36,6 @@
#include <rte_mbuf.h>
#include <rte_mempool.h>
#include <rte_malloc.h>
-
#include <rte_memcpy.h>
#ifdef RTE_NEXT_ABI
@@ -286,15 +285,12 @@ rte_port_source_free(void *port)
if (p == NULL)
return 0;
-#ifdef RTE_NEXT_ABI
-
if (p->pkt_len)
rte_free(p->pkt_len);
if (p->pkts)
rte_free(p->pkts);
if (p->pkt_buff)
rte_free(p->pkt_buff);
-#endif
rte_free(p);
@@ -315,8 +311,6 @@ rte_port_source_rx(void *port, struct rte_mbuf **pkts, uint32_t n_pkts)
rte_pktmbuf_reset(pkts[i]);
}
-#ifdef RTE_NEXT_ABI
-
if (p->pkt_buff != NULL) {
for (i = 0; i < n_pkts; i++) {
uint8_t *pkt_data = rte_pktmbuf_mtod(pkts[i],
@@ -333,8 +327,6 @@ rte_port_source_rx(void *port, struct rte_mbuf **pkts, uint32_t n_pkts)
}
}
-#endif
-
RTE_PORT_SOURCE_STATS_PKTS_IN_ADD(p, n_pkts);
return n_pkts;
@@ -419,21 +411,12 @@ pcap_sink_open(struct rte_port_sink *port,
return 0;
}
-uint8_t jumbo_pkt_buf[ETHER_MAX_JUMBO_FRAME_LEN];
-
-/**
- * Dump a packet to PCAP dumper
- *
- * @param p
- * Handle to sink port
- * @param mbuf
- * Handle to mbuf structure holding the packet
- */
static void
-pcap_sink_dump_pkt(struct rte_port_sink *port, struct rte_mbuf *mbuf)
+pcap_sink_write_pkt(struct rte_port_sink *port, struct rte_mbuf *mbuf)
{
uint8_t *pcap_dumper = (uint8_t *)(port->dumper);
struct pcap_pkthdr pcap_hdr;
+ uint8_t jumbo_pkt_buf[ETHER_MAX_JUMBO_FRAME_LEN];
uint8_t *pkt;
/* Maximum num packets already reached */
@@ -481,37 +464,23 @@ pcap_sink_dump_pkt(struct rte_port_sink *port, struct rte_mbuf *mbuf)
}
-/**
- * Flush pcap dumper
- *
- * @param dumper
- * Handle to pcap dumper
- */
+#define PCAP_SINK_OPEN(port, file_name, max_n_pkts) \
+ pcap_sink_open(port, file_name, max_n_pkts)
-static void
-pcap_sink_flush_pkt(void *dumper)
-{
- pcap_dumper_t *pcap_dumper = (pcap_dumper_t *)dumper;
+#define PCAP_SINK_WRITE_PKT(port, mbuf) \
+ pcap_sink_write_pkt(port, mbuf)
- pcap_dump_flush(pcap_dumper);
-}
+#define PCAP_SINK_FLUSH_PKT(dumper) \
+do { \
+ if (dumper) \
+ pcap_dump_flush((pcap_dumper_t *)dumper); \
+} while (0)
-/**
- * Close a PCAP dumper handle
- *
- * @param dumper
- * Handle to pcap dumper
- */
-static void
-pcap_sink_close(void *dumper)
-{
- pcap_dumper_t *pcap_dumper = (pcap_dumper_t *)dumper;
-
- pcap_dump_close(pcap_dumper);
-}
-
-#define PCAP_SINK_OPEN(port, file_name, max_n_pkts) \
- pcap_sink_open(port, file_name, max_n_pkts)
+#define PCAP_SINK_CLOSE(dumper) \
+do { \
+ if (dumper) \
+ pcap_dump_close((pcap_dumper_t *)dumper); \
+} while (0)
#else
@@ -528,15 +497,11 @@ pcap_sink_close(void *dumper)
_ret; \
})
-static void
-pcap_sink_dump_pkt(__rte_unused struct rte_port_sink *port,
- __rte_unused struct rte_mbuf *mbuf) {}
+#define PCAP_SINK_WRITE_PKT(port, mbuf) {}
-static void
-pcap_sink_flush_pkt(__rte_unused void *dumper) {}
+#define PCAP_SINK_FLUSH_PKT(dumper)
-static void
-pcap_sink_close(__rte_unused void *dumper) {}
+#define PCAP_SINK_CLOSE(dumper)
#endif
@@ -573,11 +538,11 @@ rte_port_sink_create(void *params, int socket_id)
static int
rte_port_sink_tx(void *port, struct rte_mbuf *pkt)
{
- __rte_unused struct rte_port_sink *p = (struct rte_port_sink *) port;
+ struct rte_port_sink *p = (struct rte_port_sink *) port;
RTE_PORT_SINK_STATS_PKTS_IN_ADD(p, 1);
if (p->dumper != NULL)
- pcap_sink_dump_pkt(p, pkt);
+ PCAP_SINK_WRITE_PKT(p, pkt);
rte_pktmbuf_free(pkt);
RTE_PORT_SINK_STATS_PKTS_DROP_ADD(p, 1);
@@ -588,7 +553,7 @@ static int
rte_port_sink_tx_bulk(void *port, struct rte_mbuf **pkts,
uint64_t pkts_mask)
{
- __rte_unused struct rte_port_sink *p = (struct rte_port_sink *) port;
+ struct rte_port_sink *p = (struct rte_port_sink *) port;
if ((pkts_mask & (pkts_mask + 1)) == 0) {
uint64_t n_pkts = __builtin_popcountll(pkts_mask);
@@ -598,11 +563,8 @@ rte_port_sink_tx_bulk(void *port, struct rte_mbuf **pkts,
RTE_PORT_SINK_STATS_PKTS_DROP_ADD(p, n_pkts);
if (p->dumper) {
- for (i = 0; i < n_pkts; i++) {
- struct rte_mbuf *pkt = pkts[i];
-
- pcap_sink_dump_pkt(p, pkt);
- }
+ for (i = 0; i < n_pkts; i++)
+ PCAP_SINK_WRITE_PKT(p, pkts[i]);
}
for (i = 0; i < n_pkts; i++) {
@@ -619,7 +581,7 @@ rte_port_sink_tx_bulk(void *port, struct rte_mbuf **pkts,
for ( ; dump_pkts_mask; ) {
pkt_index = __builtin_ctzll(
dump_pkts_mask);
- pcap_sink_dump_pkt(p, pkts[pkt_index]);
+ PCAP_SINK_WRITE_PKT(p, pkts[pkt_index]);
dump_pkts_mask &= ~(1LLU << pkt_index);
}
}
@@ -642,10 +604,13 @@ rte_port_sink_tx_bulk(void *port, struct rte_mbuf **pkts,
static int
rte_port_sink_flush(void *port)
{
- struct rte_port_sink *p = (struct rte_port_sink *)port;
+ struct rte_port_sink *p =
+ (struct rte_port_sink *)port;
- if (p->dumper != NULL)
- pcap_sink_flush_pkt(p->dumper);
+ if (p == NULL)
+ return 0;
+
+ PCAP_SINK_FLUSH_PKT(p->dumper);
return 0;
}
@@ -655,12 +620,11 @@ rte_port_sink_free(void *port)
{
struct rte_port_sink *p =
(struct rte_port_sink *)port;
- /* Check input parameters */
+
if (p == NULL)
return 0;
- if (p->dumper != NULL)
- pcap_sink_close(p->dumper);
+ PCAP_SINK_CLOSE(p->dumper);
rte_free(p);
@@ -92,9 +92,7 @@ endif
ifeq ($(CONFIG_RTE_LIBRTE_VHOST_USER),n)
_LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lfuse
endif
-ifeq ($(CONFIG_RTE_NEXT_ABI),y)
_LDLIBS-$(CONFIG_RTE_PORT_PCAP) += -lpcap
-endif
_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += -lpcap
_LDLIBS-$(CONFIG_RTE_LIBRTE_BNX2X_PMD) += -lz
_LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += -libverbs