[v3] examples/multi_process - fix crash in mp_client with sparse ports
Checks
Commit Message
From: Stephen Hemminger <sthemmin@microsoft.com>
The mp_client crashes if run on Azure or any system where ethdev
ports are owned. This is because the flush loop is confusing the
index into the array of ports[] with the port id.
For example if the server has Ports 3 and 5. Then calling
rte_eth_tx_buffer_flush on any other buffer will dereference null
because the tx buffer for that port was not allocated.
v2
- fix typo
- the flush code is common enough that it should not be marked unlikely
- combine conditions to reduce indentation
v3
- avoid unnecessary if() if sent is zero.
Fixes: e2366e74e029 ("examples: use buffered Tx")
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
.../client_server_mp/mp_client/client.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
Comments
On Tue, Jun 18, 2019 at 6:41 PM Stephen Hemminger <
stephen@networkplumber.org> wrote:
> From: Stephen Hemminger <sthemmin@microsoft.com>
>
> The mp_client crashes if run on Azure or any system where ethdev
> ports are owned. This is because the flush loop is confusing the
> index into the array of ports[] with the port id.
>
> For example if the server has Ports 3 and 5. Then calling
> rte_eth_tx_buffer_flush on any other buffer will dereference null
> because the tx buffer for that port was not allocated.
>
> v2
> - fix typo
> - the flush code is common enough that it should not be marked unlikely
> - combine conditions to reduce indentation
>
> v3
> - avoid unnecessary if() if sent is zero.
>
Annotations should be after the ---.
> Fixes: e2366e74e029 ("examples: use buffered Tx")
>
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> ---
> .../client_server_mp/mp_client/client.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/examples/multi_process/client_server_mp/mp_client/client.c
> b/examples/multi_process/client_server_mp/mp_client/client.c
> index c23dd3f378f7..361d90b54b2d 100644
> --- a/examples/multi_process/client_server_mp/mp_client/client.c
> +++ b/examples/multi_process/client_server_mp/mp_client/client.c
> @@ -246,19 +246,19 @@ main(int argc, char *argv[])
>
> for (;;) {
> uint16_t i, rx_pkts;
> - uint16_t port;
>
> rx_pkts = rte_ring_dequeue_burst(rx_ring, pkts,
> PKT_READ_SIZE, NULL);
>
> - if (unlikely(rx_pkts == 0)){
> - if (need_flush)
> - for (port = 0; port < ports->num_ports;
> port++) {
> - sent =
> rte_eth_tx_buffer_flush(ports->id[port], client_id,
> - tx_buffer[port]);
> - if (unlikely(sent))
> - tx_stats->tx[port] += sent;
> - }
> + if (rx_pkts == 0 && need_flush) {
> + for (i = 0; i < ports->num_ports; i++) {
> + uint16_t port = ports->id[i];
> +
> + sent = rte_eth_tx_buffer_flush(port,
> + client_id,
> +
> tx_buffer[port]);
> + tx_stats->tx[port] += sent;
> + }
> need_flush = 0;
> continue;
> }
>
>
Woh, had to concentrate to see that the pb was when accessing tx_buffer[]
and tx_stats->tx[] arrays, good catch :-)
Reviewed-by: David Marchand <david.marchand@redhat.com>
@@ -246,19 +246,19 @@ main(int argc, char *argv[])
for (;;) {
uint16_t i, rx_pkts;
- uint16_t port;
rx_pkts = rte_ring_dequeue_burst(rx_ring, pkts,
PKT_READ_SIZE, NULL);
- if (unlikely(rx_pkts == 0)){
- if (need_flush)
- for (port = 0; port < ports->num_ports; port++) {
- sent = rte_eth_tx_buffer_flush(ports->id[port], client_id,
- tx_buffer[port]);
- if (unlikely(sent))
- tx_stats->tx[port] += sent;
- }
+ if (rx_pkts == 0 && need_flush) {
+ for (i = 0; i < ports->num_ports; i++) {
+ uint16_t port = ports->id[i];
+
+ sent = rte_eth_tx_buffer_flush(port,
+ client_id,
+ tx_buffer[port]);
+ tx_stats->tx[port] += sent;
+ }
need_flush = 0;
continue;
}