[dpdk-dev,v6,2/2] testpmd: add mode 4 support v6
Commit Message
From: Pawel Wodkowski <pawelx.wodkowski@intel.com>
This patch add mode 4 support to testpmd application.
Signed-off-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
---
app/test-pmd/cmdline.c | 28 ++++++++++++++++++++++--
app/test-pmd/csumonly.c | 9 ++++++++
app/test-pmd/icmpecho.c | 21 +++++++++++++++++-
app/test-pmd/iofwd.c | 9 ++++++++
app/test-pmd/macfwd-retry.c | 9 ++++++++
app/test-pmd/macfwd.c | 9 ++++++++
app/test-pmd/macswap.c | 9 ++++++++
app/test-pmd/testpmd.c | 50 +++++++++++++++++++++++++++++++++++++------
app/test-pmd/testpmd.h | 11 ++++++++--
9 files changed, 144 insertions(+), 11 deletions(-)
Comments
2014-11-26 11:17, Michal Jastrzebski:
> From: Pawel Wodkowski <pawelx.wodkowski@intel.com>
> --- a/app/test-pmd/csumonly.c
> +++ b/app/test-pmd/csumonly.c
> @@ -254,8 +254,17 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> */
> nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
> nb_pkt_per_burst);
> +#ifndef RTE_LIBRTE_PMD_BOND
> if (unlikely(nb_rx == 0))
> return;
> +#else
> + if (unlikely(nb_rx == 0 && (fs->forward_timeout == 0 ||
> + fs->next_forward_time > rte_rdtsc())))
> + return;
> +
> + if (fs->forward_timeout != 0)
> + fs->next_forward_time = rte_rdtsc() + fs->forward_timeout;
> +#endif
I don't understand why you need to make such change for bonding,
and there is no comment to explain.
Bonding should be a PMD like any other and shouldn't require such change.
I don't know mode 4 but it seems there is a design problem here.
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Wednesday, November 26, 2014 1:31 PM
> To: Jastrzebski, MichalX K
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v6 2/2] testpmd: add mode 4 support v6
>
> 2014-11-26 11:17, Michal Jastrzebski:
> > From: Pawel Wodkowski <pawelx.wodkowski@intel.com>
> > --- a/app/test-pmd/csumonly.c
> > +++ b/app/test-pmd/csumonly.c
> > @@ -254,8 +254,17 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> > */
> > nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
> > nb_pkt_per_burst);
> > +#ifndef RTE_LIBRTE_PMD_BOND
> > if (unlikely(nb_rx == 0))
> > return;
> > +#else
> > + if (unlikely(nb_rx == 0 && (fs->forward_timeout == 0 ||
> > + fs->next_forward_time > rte_rdtsc())))
> > + return;
> > +
> > + if (fs->forward_timeout != 0)
> > + fs->next_forward_time = rte_rdtsc() + fs->forward_timeout;
> > +#endif
>
> I don't understand why you need to make such change for bonding,
> and there is no comment to explain.
> Bonding should be a PMD like any other and shouldn't require such change.
> I don't know mode 4 but it seems there is a design problem here.
>
It is an implication of requirement that was formed on beginning of bonding
implementation - bonded interface should be transparent to user app. But this
requirement in is in collision with mode 4. It need to periodically receive and
transmit frames (LACP and marker) that are not passed to user app but
processed/produced in background. If this will not happen in at least 10 times
per second mode 4 will not work.
Most of (all?) user applications do RX/TX more often than 10 times per second,
so this will have neglectable impact to those apps (it will have to check this
100ms maximum interval of rx/tx as I did in code you pointed).
We had discussed all options with Declan and Bruce, and this seems to be the
most transparent way to implement mode 4 without using any kind of locking
inside library.
Paweł
2014-11-26 13:00, Wodkowski, PawelX:
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> > 2014-11-26 11:17, Michal Jastrzebski:
> > > From: Pawel Wodkowski <pawelx.wodkowski@intel.com>
> > > --- a/app/test-pmd/csumonly.c
> > > +++ b/app/test-pmd/csumonly.c
> > > @@ -254,8 +254,17 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> > > */
> > > nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
> > > nb_pkt_per_burst);
> > > +#ifndef RTE_LIBRTE_PMD_BOND
> > > if (unlikely(nb_rx == 0))
> > > return;
> > > +#else
> > > + if (unlikely(nb_rx == 0 && (fs->forward_timeout == 0 ||
> > > + fs->next_forward_time > rte_rdtsc())))
> > > + return;
> > > +
> > > + if (fs->forward_timeout != 0)
> > > + fs->next_forward_time = rte_rdtsc() + fs->forward_timeout;
> > > +#endif
> >
> > I don't understand why you need to make such change for bonding,
> > and there is no comment to explain.
> > Bonding should be a PMD like any other and shouldn't require such change.
> > I don't know mode 4 but it seems there is a design problem here.
> >
>
> It is an implication of requirement that was formed on beginning of bonding
> implementation - bonded interface should be transparent to user app. But this
> requirement in is in collision with mode 4. It need to periodically receive and
> transmit frames (LACP and marker) that are not passed to user app but
> processed/produced in background. If this will not happen in at least 10 times
> per second mode 4 will not work.
>
> Most of (all?) user applications do RX/TX more often than 10 times per second,
> so this will have neglectable impact to those apps (it will have to check this
> 100ms maximum interval of rx/tx as I did in code you pointed).
>
> We had discussed all options with Declan and Bruce, and this seems to be the
> most transparent way to implement mode 4 without using any kind of locking
> inside library.
So you agree there is a design problem and you were initially trying to push it
without raising the problem in the hope that nobody will see it?
It's really not the good way to work in an Open Source project.
Is there any comment in the API to explain this new constraint?
Do you think we can change how Rx/Tx works in DPDK to integrate this feature?
Actually, I think these bonding features should be implemented in a layer on
top of DPDK. It's not the DPDK responsibility to make some protocol processing.
Bonding was integrated with the promise that it's transparent and really close
to the hardware ports.
Today I see we clearly need a discussion to know what should be implemented
in DPDK. Which protocol layer is the limit?
I explained my point of view but the decision belongs to the whole community.
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, November 26, 2014 2:31 PM
> To: Wodkowski, PawelX
> Cc: Jastrzebski, MichalX K; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v6 2/2] testpmd: add mode 4 support v6
>
> 2014-11-26 13:00, Wodkowski, PawelX:
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> > > 2014-11-26 11:17, Michal Jastrzebski:
> > > > From: Pawel Wodkowski <pawelx.wodkowski@intel.com>
> > > > --- a/app/test-pmd/csumonly.c
> > > > +++ b/app/test-pmd/csumonly.c
> > > > @@ -254,8 +254,17 @@ pkt_burst_checksum_forward(struct
> fwd_stream *fs)
> > > > */
> > > > nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
> > > > nb_pkt_per_burst);
> > > > +#ifndef RTE_LIBRTE_PMD_BOND
> > > > if (unlikely(nb_rx == 0))
> > > > return;
> > > > +#else
> > > > + if (unlikely(nb_rx == 0 && (fs->forward_timeout == 0 ||
> > > > + fs->next_forward_time > rte_rdtsc())))
> > > > + return;
> > > > +
> > > > + if (fs->forward_timeout != 0)
> > > > + fs->next_forward_time = rte_rdtsc() + fs->forward_timeout;
> > > > +#endif
> > >
> > > I don't understand why you need to make such change for bonding,
> > > and there is no comment to explain.
> > > Bonding should be a PMD like any other and shouldn't require such
> change.
> > > I don't know mode 4 but it seems there is a design problem here.
> > >
> >
> > It is an implication of requirement that was formed on beginning of
> bonding
> > implementation - bonded interface should be transparent to user app. But
> this
> > requirement in is in collision with mode 4. It need to periodically receive
> and
> > transmit frames (LACP and marker) that are not passed to user app but
> > processed/produced in background. If this will not happen in at least 10
> times
> > per second mode 4 will not work.
> >
> > Most of (all?) user applications do RX/TX more often than 10 times per
> second,
> > so this will have neglectable impact to those apps (it will have to check this
> > 100ms maximum interval of rx/tx as I did in code you pointed).
> >
> > We had discussed all options with Declan and Bruce, and this seems to be
> the
> > most transparent way to implement mode 4 without using any kind of
> locking
> > inside library.
>
> So you agree there is a design problem and you were initially trying to push it
> without raising the problem in the hope that nobody will see it?
No, we didn't want to hide anything.
> It's really not the good way to work in an Open Source project.
>
> Is there any comment in the API to explain this new constraint?
No, we haven't put in the code a straight comment. I wrote about it in cover letter in v6
and there is also show_warnings function in patch 1/2 which will print a warning to the
application.
> Do you think we can change how Rx/Tx works in DPDK to integrate this
> feature?
>
> Actually, I think these bonding features should be implemented in a layer on
> top of DPDK. It's not the DPDK responsibility to make some protocol
> processing.
> Bonding was integrated with the promise that it's transparent and really
> close
> to the hardware ports.
>
> Today I see we clearly need a discussion to know what should be
> implemented
> in DPDK. Which protocol layer is the limit?
> I explained my point of view but the decision belongs to the whole
> community.
>
> --
> Thomas
@@ -41,6 +41,8 @@
#include <termios.h>
#include <unistd.h>
#include <inttypes.h>
+
+#include "rte_eth_bond_8023ad.h"
#ifndef __linux__
#ifndef __FreeBSD__
#include <net/socket.h>
@@ -87,6 +89,7 @@
#include <rte_pci_dev_ids.h>
#ifdef RTE_LIBRTE_PMD_BOND
#include <rte_eth_bond.h>
+#include <rte_eth_bond_8023ad.h>
#endif
#include "testpmd.h"
@@ -3441,13 +3444,18 @@ static void cmd_show_bonding_config_parsed(void *parsed_result,
__attribute__((unused)) void *data)
{
struct cmd_show_bonding_config_result *res = parsed_result;
+ struct rte_eth_bond_8023ad_slave_info slave_info;
+ static const char * const state_labels[] = {
+ "ACT", "TIMEOUT", "AGG", "SYNC", "COL", "DIST", "DEF", "EXP"
+ };
int bonding_mode;
uint8_t slaves[RTE_MAX_ETHPORTS];
int num_slaves, num_active_slaves;
int primary_id;
- int i;
+ int i, j;
portid_t port_id = res->port_id;
+
/* Display the bonding mode.*/
bonding_mode = rte_eth_bond_mode_get(port_id);
if (bonding_mode < 0) {
@@ -3456,7 +3464,8 @@ static void cmd_show_bonding_config_parsed(void *parsed_result,
} else
printf("\tBonding mode: %d\n", bonding_mode);
- if (bonding_mode == BONDING_MODE_BALANCE) {
+ if (bonding_mode == BONDING_MODE_BALANCE ||
+ bonding_mode == BONDING_MODE_8023AD) {
int balance_xmit_policy;
balance_xmit_policy = rte_eth_bond_xmit_policy_get(port_id);
@@ -3513,6 +3522,19 @@ static void cmd_show_bonding_config_parsed(void *parsed_result,
printf("%d]\n", slaves[num_active_slaves - 1]);
+ if (bonding_mode == BONDING_MODE_8023AD) {
+ for (i = 0; i < num_active_slaves; i++) {
+ rte_eth_bond_8023ad_slave_info(port_id, slaves[i], &slave_info);
+
+ printf("\tSlave %u state: ", slaves[i]);
+ for (j = 0; j < 8; j++) {
+ if ((slave_info.actor_state >> j) & 1)
+ printf("%s ", state_labels[j]);
+ }
+ printf("\n");
+ }
+ }
+
} else {
printf("\tActive Slaves: []\n");
@@ -3760,6 +3782,8 @@ static void cmd_create_bonded_device_parsed(void *parsed_result,
/* Update number of ports */
nb_ports = rte_eth_dev_count();
reconfig(port_id, res->socket);
+ /* Save bonding mode here as it is constat. */
+ ports[port_id].bond_mode = res->mode;
rte_eth_promiscuous_enable(port_id);
}
@@ -254,8 +254,17 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
*/
nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
nb_pkt_per_burst);
+#ifndef RTE_LIBRTE_PMD_BOND
if (unlikely(nb_rx == 0))
return;
+#else
+ if (unlikely(nb_rx == 0 && (fs->forward_timeout == 0 ||
+ fs->next_forward_time > rte_rdtsc())))
+ return;
+
+ if (fs->forward_timeout != 0)
+ fs->next_forward_time = rte_rdtsc() + fs->forward_timeout;
+#endif
#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
fs->rx_burst_stats.pkt_burst_spread[nb_rx]++;
@@ -293,6 +293,9 @@ reply_to_icmp_echo_rqsts(struct fwd_stream *fs)
uint16_t arp_pro;
uint8_t i;
int l2_len;
+#ifdef RTE_LIBRTE_PMD_BOND
+ uint8_t force_tx_burst;
+#endif
#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
uint64_t start_tsc;
uint64_t end_tsc;
@@ -308,8 +311,20 @@ reply_to_icmp_echo_rqsts(struct fwd_stream *fs)
*/
nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
nb_pkt_per_burst);
+#ifndef RTE_LIBRTE_PMD_BOND
if (unlikely(nb_rx == 0))
return;
+#else
+ if (unlikely(nb_rx == 0 && (fs->forward_timeout == 0 ||
+ fs->next_forward_time > rte_rdtsc())))
+ return;
+
+ if (fs->forward_timeout != 0) {
+ force_tx_burst = fs->next_forward_time <= rte_rdtsc();
+ fs->next_forward_time = rte_rdtsc() + fs->forward_timeout;
+ } else
+ force_tx_burst = 0;
+#endif
#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
fs->rx_burst_stats.pkt_burst_spread[nb_rx]++;
@@ -462,7 +477,11 @@ reply_to_icmp_echo_rqsts(struct fwd_stream *fs)
}
/* Send back ICMP echo replies, if any. */
- if (nb_replies > 0) {
+#ifdef RTE_LIBRTE_PMD_BOND
+ if (nb_replies > 0 || force_tx_burst) {
+#else
+ if (nb_replies > 0) {
+#endif
nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst,
nb_replies);
fs->tx_packets += nb_tx;
@@ -96,8 +96,17 @@ pkt_burst_io_forward(struct fwd_stream *fs)
*/
nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
nb_pkt_per_burst);
+#ifndef RTE_LIBRTE_PMD_BOND
if (unlikely(nb_rx == 0))
return;
+#else
+ if (unlikely(nb_rx == 0 && (fs->forward_timeout == 0 ||
+ fs->next_forward_time > rte_rdtsc())))
+ return;
+
+ if (fs->forward_timeout != 0)
+ fs->next_forward_time = rte_rdtsc() + fs->forward_timeout;
+#endif
#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
fs->rx_burst_stats.pkt_burst_spread[nb_rx]++;
@@ -110,8 +110,17 @@ pkt_burst_mac_retry_forward(struct fwd_stream *fs)
*/
nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
nb_pkt_per_burst);
+#ifndef RTE_LIBRTE_PMD_BOND
if (unlikely(nb_rx == 0))
return;
+#else
+ if (unlikely(nb_rx == 0 && (fs->forward_timeout == 0 ||
+ fs->next_forward_time > rte_rdtsc())))
+ return;
+
+ if (fs->forward_timeout != 0)
+ fs->next_forward_time = rte_rdtsc() + fs->forward_timeout;
+#endif
#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
fs->rx_burst_stats.pkt_burst_spread[nb_rx]++;
@@ -100,8 +100,17 @@ pkt_burst_mac_forward(struct fwd_stream *fs)
*/
nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
nb_pkt_per_burst);
+#ifndef RTE_LIBRTE_PMD_BOND
if (unlikely(nb_rx == 0))
return;
+#else
+ if (unlikely(nb_rx == 0 && (fs->forward_timeout == 0 ||
+ fs->next_forward_time > rte_rdtsc())))
+ return;
+
+ if (fs->forward_timeout != 0)
+ fs->next_forward_time = rte_rdtsc() + fs->forward_timeout;
+#endif
#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
fs->rx_burst_stats.pkt_burst_spread[nb_rx]++;
@@ -100,8 +100,17 @@ pkt_burst_mac_swap(struct fwd_stream *fs)
*/
nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
nb_pkt_per_burst);
+#ifndef RTE_LIBRTE_PMD_BOND
if (unlikely(nb_rx == 0))
return;
+#else
+ if (unlikely(nb_rx == 0 && (fs->forward_timeout == 0 ||
+ fs->next_forward_time > rte_rdtsc())))
+ return;
+
+ if (fs->forward_timeout != 0)
+ fs->next_forward_time = rte_rdtsc() + fs->forward_timeout;
+#endif
#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
fs->rx_burst_stats.pkt_burst_spread[nb_rx]++;
@@ -72,6 +72,9 @@
#include <rte_ether.h>
#include <rte_ethdev.h>
#include <rte_string_fns.h>
+#ifdef RTE_LIBRTE_PMD_BOND
+#include <rte_eth_bond.h>
+#endif
#ifdef RTE_LIBRTE_PMD_XENVIRT
#include <rte_eth_xenvirt.h>
#endif
@@ -1022,9 +1025,13 @@ start_packet_forwarding(int with_tx_first)
port_fwd_begin_t port_fwd_begin;
port_fwd_end_t port_fwd_end;
struct rte_port *port;
+ struct fwd_stream *fs;
unsigned int i;
portid_t pt_id;
streamid_t sm_id;
+#ifdef RTE_LIBRTE_PMD_BOND
+ uint8_t is_mode4;
+#endif
if (all_ports_started() == 0) {
printf("Not all ports were started\n");
@@ -1050,7 +1057,6 @@ start_packet_forwarding(int with_tx_first)
return;
}
}
- test_done = 0;
if(!no_flush_rx)
flush_fwd_rx_queues();
@@ -1067,11 +1073,35 @@ start_packet_forwarding(int with_tx_first)
map_port_queue_stats_mapping_registers(pt_id, port);
}
for (sm_id = 0; sm_id < cur_fwd_config.nb_fwd_streams; sm_id++) {
- fwd_streams[sm_id]->rx_packets = 0;
- fwd_streams[sm_id]->tx_packets = 0;
- fwd_streams[sm_id]->fwd_dropped = 0;
- fwd_streams[sm_id]->rx_bad_ip_csum = 0;
- fwd_streams[sm_id]->rx_bad_l4_csum = 0;
+ fs = fwd_streams[sm_id];
+ fs->rx_packets = 0;
+ fs->tx_packets = 0;
+ fs->fwd_dropped = 0;
+ fs->rx_bad_ip_csum = 0;
+ fs->rx_bad_l4_csum = 0;
+
+#ifdef RTE_LIBRTE_PMD_BOND
+ is_mode4 = ports[fs->rx_port].bond_mode == BONDING_MODE_8023AD ||
+ ports[fs->tx_port].bond_mode == BONDING_MODE_8023AD;
+
+ if (is_mode4) {
+ if (cur_fwd_config.fwd_eng == &rx_only_engine ||
+ cur_fwd_config.fwd_eng == &tx_only_engine
+#ifdef RTE_LIBRTE_IEEE1588
+ || cur_fwd_config.fwd_eng == &ieee1588_fwd_engine
+#endif
+ ) {
+ printf("Selected forwarding engine '%s' is not supported in "
+ "mode 802.3ad of link bonding.\n",
+ cur_fwd_config.fwd_eng->fwd_mode_name);
+
+ return;
+ }
+ fs->forward_timeout = 100 * rte_get_tsc_hz() / 1000;
+ fs->next_forward_time = 0; /* force forward */
+ } else
+ fs->forward_timeout = 0; /* force forward is disabled */
+#endif
#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
memset(&fwd_streams[sm_id]->rx_burst_stats, 0,
@@ -1083,6 +1113,9 @@ start_packet_forwarding(int with_tx_first)
fwd_streams[sm_id]->core_cycles = 0;
#endif
}
+
+ test_done = 0;
+
if (with_tx_first) {
port_fwd_begin = tx_only_engine.port_fwd_begin;
if (port_fwd_begin != NULL) {
@@ -1284,6 +1317,11 @@ start_port(portid_t pid)
struct rte_port *port;
struct ether_addr mac_addr;
+ if (pid >= nb_ports && pid != RTE_PORT_ALL) {
+ printf("Invalid port id %u\n", pid);
+ return -1;
+ }
+
if (test_done == 0) {
printf("Please stop forwarding first\n");
return -1;
@@ -40,7 +40,7 @@
int main(int argc, char **argv);
#endif
-#define RTE_PORT_ALL (~(portid_t)0x0)
+#define RTE_PORT_ALL ((portid_t)(~0x0))
#define RTE_TEST_RX_DESC_MAX 2048
#define RTE_TEST_TX_DESC_MAX 2048
@@ -121,6 +121,10 @@ struct fwd_stream {
struct pkt_burst_stats rx_burst_stats;
struct pkt_burst_stats tx_burst_stats;
#endif
+#ifdef RTE_LIBRTE_PMD_BOND
+ uint64_t forward_timeout; /** Timeout used to force RX/TX */
+ uint64_t next_forward_time; /**< Next time when RX/TX should be issued. */
+#endif
};
/**
@@ -152,7 +156,10 @@ struct rte_port {
uint8_t need_reconfig; /**< need reconfiguring port or not */
uint8_t need_reconfig_queues; /**< need reconfiguring queues or not */
uint8_t rss_flag; /**< enable rss or not */
- uint8_t dcb_flag; /**< enable dcb */
+ uint8_t dcb_flag; /**< enable dcb */
+#ifdef RTE_LIBRTE_PMD_BOND
+ int8_t bond_mode; /**< Port bonding mode */
+#endif
struct rte_eth_rxconf rx_conf; /**< rx configuration */
struct rte_eth_txconf tx_conf; /**< tx configuration */
};