All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jia Yu <jyu@vmware.com>
To: dev@dpdk.org
Cc: declan.doherty@intel.com, chas3@att.com
Subject: [PATCH] net/bonding: fix buf corruption in merging un-transmitted packets
Date: Fri, 17 Aug 2018 17:10:09 -0700	[thread overview]
Message-ID: <1534551009-16550-1-git-send-email-jyu@vmware.com> (raw)

When bond slave devices cannot transmit all packets in bufs array,
tx_burst callback shall merge the un-transmitted packets back to
bufs array. Recent merge logic introduced a bug which causes
invalid mbuf addresses being written to bufs array.
When caller frees the un-transmitted packets, due to invalid addresses,
application will crash.

The fix is avoid shifting mbufs, and directly write un-transmitted
packets back to bufs array.

Signed-off-by: Jia Yu <jyu@vmware.com>
---
 drivers/net/bonding/rte_eth_bond_pmd.c | 98 +++++-----------------------------
 1 file changed, 14 insertions(+), 84 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 58f7377..cce973a 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -303,7 +303,7 @@ bond_ethdev_tx_burst_8023ad_fast_queue(void *queue, struct rte_mbuf **bufs,
 	uint16_t slave_tx_count, slave_tx_fail_count[RTE_MAX_ETHPORTS] = { 0 };
 	uint16_t total_tx_count = 0, total_tx_fail_count = 0;
 
-	uint16_t i, j;
+	uint16_t i;
 
 	if (unlikely(nb_bufs == 0))
 		return 0;
@@ -361,31 +361,9 @@ bond_ethdev_tx_burst_8023ad_fast_queue(void *queue, struct rte_mbuf **bufs,
 			slave_tx_fail_count[i] = slave_nb_bufs[i] -
 					slave_tx_count;
 			total_tx_fail_count += slave_tx_fail_count[i];
-
-			/*
-			 * Shift bufs to beginning of array to allow reordering
-			 * later
-			 */
-			for (j = 0; j < slave_tx_fail_count[i]; j++) {
-				slave_bufs[i][j] =
-					slave_bufs[i][(slave_tx_count - 1) + j];
-			}
-		}
-	}
-
-	/*
-	 * If there are tx burst failures we move packets to end of bufs to
-	 * preserve expected PMD behaviour of all failed transmitted being
-	 * at the end of the input mbuf array
-	 */
-	if (unlikely(total_tx_fail_count > 0)) {
-		int bufs_idx = nb_bufs - total_tx_fail_count - 1;
-
-		for (i = 0; i < slave_count; i++) {
-			if (slave_tx_fail_count[i] > 0) {
-				for (j = 0; j < slave_tx_fail_count[i]; j++)
-					bufs[bufs_idx++] = slave_bufs[i][j];
-			}
+			memcpy(&bufs[nb_bufs - total_tx_fail_count],
+			       &slave_bufs[i][slave_tx_count],
+			       slave_tx_fail_count[i] * sizeof(bufs[0]));
 		}
 	}
 
@@ -715,8 +693,8 @@ bond_ethdev_tx_burst_round_robin(void *queue, struct rte_mbuf **bufs,
 				tx_fail_total += tx_fail_slave;
 
 				memcpy(&bufs[nb_pkts - tx_fail_total],
-						&slave_bufs[i][num_tx_slave],
-						tx_fail_slave * sizeof(bufs[0]));
+				       &slave_bufs[i][num_tx_slave],
+				       tx_fail_slave * sizeof(bufs[0]));
 			}
 			num_tx_total += num_tx_slave;
 		}
@@ -1224,7 +1202,7 @@ bond_ethdev_tx_burst_balance(void *queue, struct rte_mbuf **bufs,
 	uint16_t slave_tx_count, slave_tx_fail_count[RTE_MAX_ETHPORTS] = { 0 };
 	uint16_t total_tx_count = 0, total_tx_fail_count = 0;
 
-	uint16_t i, j;
+	uint16_t i;
 
 	if (unlikely(nb_bufs == 0))
 		return 0;
@@ -1268,31 +1246,9 @@ bond_ethdev_tx_burst_balance(void *queue, struct rte_mbuf **bufs,
 			slave_tx_fail_count[i] = slave_nb_bufs[i] -
 					slave_tx_count;
 			total_tx_fail_count += slave_tx_fail_count[i];
-
-			/*
-			 * Shift bufs to beginning of array to allow reordering
-			 * later
-			 */
-			for (j = 0; j < slave_tx_fail_count[i]; j++) {
-				slave_bufs[i][j] =
-					slave_bufs[i][(slave_tx_count - 1) + j];
-			}
-		}
-	}
-
-	/*
-	 * If there are tx burst failures we move packets to end of bufs to
-	 * preserve expected PMD behaviour of all failed transmitted being
-	 * at the end of the input mbuf array
-	 */
-	if (unlikely(total_tx_fail_count > 0)) {
-		int bufs_idx = nb_bufs - total_tx_fail_count - 1;
-
-		for (i = 0; i < slave_count; i++) {
-			if (slave_tx_fail_count[i] > 0) {
-				for (j = 0; j < slave_tx_fail_count[i]; j++)
-					bufs[bufs_idx++] = slave_bufs[i][j];
-			}
+			memcpy(&bufs[nb_bufs - total_tx_fail_count],
+			       &slave_bufs[i][slave_tx_count],
+			       slave_tx_fail_count[i] * sizeof(bufs[0]));
 		}
 	}
 
@@ -1322,7 +1278,7 @@ bond_ethdev_tx_burst_8023ad(void *queue, struct rte_mbuf **bufs,
 	uint16_t slave_tx_count, slave_tx_fail_count[RTE_MAX_ETHPORTS] = { 0 };
 	uint16_t total_tx_count = 0, total_tx_fail_count = 0;
 
-	uint16_t i, j;
+	uint16_t i;
 
 	if (unlikely(nb_bufs == 0))
 		return 0;
@@ -1384,35 +1340,9 @@ bond_ethdev_tx_burst_8023ad(void *queue, struct rte_mbuf **bufs,
 						slave_tx_count;
 				total_tx_fail_count += slave_tx_fail_count[i];
 
-				/*
-				 * Shift bufs to beginning of array to allow
-				 * reordering later
-				 */
-				for (j = 0; j < slave_tx_fail_count[i]; j++)
-					slave_bufs[i][j] =
-						slave_bufs[i]
-							[(slave_tx_count - 1)
-							+ j];
-			}
-		}
-
-		/*
-		 * If there are tx burst failures we move packets to end of
-		 * bufs to preserve expected PMD behaviour of all failed
-		 * transmitted being at the end of the input mbuf array
-		 */
-		if (unlikely(total_tx_fail_count > 0)) {
-			int bufs_idx = nb_bufs - total_tx_fail_count - 1;
-
-			for (i = 0; i < slave_count; i++) {
-				if (slave_tx_fail_count[i] > 0) {
-					for (j = 0;
-						j < slave_tx_fail_count[i];
-						j++) {
-						bufs[bufs_idx++] =
-							slave_bufs[i][j];
-					}
-				}
+				memcpy(&bufs[nb_bufs - total_tx_fail_count],
+				       &slave_bufs[i][slave_tx_count],
+				       slave_tx_fail_count[i] * sizeof(bufs[0]));
 			}
 		}
 	}
-- 
2.7.4

             reply	other threads:[~2018-08-18  1:46 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-18  0:10 Jia Yu [this message]
2018-08-18 23:49 ` [PATCH] net/bonding: fix buf corruption in merging un-transmitted packets Chas Williams
2018-08-19 22:19   ` Jia Yu
2018-08-19 23:20     ` Chas Williams
2018-08-20  0:07     ` Chas Williams
2018-08-20  7:02       ` Jia Yu
2018-08-20 14:02         ` Chas Williams

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1534551009-16550-1-git-send-email-jyu@vmware.com \
    --to=jyu@vmware.com \
    --cc=chas3@att.com \
    --cc=declan.doherty@intel.com \
    --cc=dev@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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.