* [dpdk-dev] [PATCH v2] net/bonding: fix buf corruption in merging un-transmitted packets [not found] <net/bonding: fix buf corruption in merging un-transmitted packets> @ 2018-08-20 5:18 ` Jia Yu 2018-08-20 13:58 ` Chas Williams 0 siblings, 1 reply; 3+ messages in thread From: Jia Yu @ 2018-08-20 5:18 UTC (permalink / raw) To: dev; +Cc: 3chas3, declan.doherty, stable 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. Fixes: 09150784a776 ("net/bonding: burst mode hash calculation") Cc: stable@dpdk.org Signed-off-by: Jia Yu <jyu@vmware.com> --- drivers/net/bonding/rte_eth_bond_pmd.c | 116 +++++++-------------------------- 1 file changed, 23 insertions(+), 93 deletions(-) diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c index 58f7377..c745f31 100644 --- a/drivers/net/bonding/rte_eth_bond_pmd.c +++ b/drivers/net/bonding/rte_eth_bond_pmd.c @@ -300,10 +300,10 @@ bond_ethdev_tx_burst_8023ad_fast_queue(void *queue, struct rte_mbuf **bufs, /* Mapping array generated by hash function to map mbufs to slaves */ uint16_t bufs_slave_port_idxs[RTE_MAX_ETHPORTS] = { 0 }; - uint16_t slave_tx_count, slave_tx_fail_count[RTE_MAX_ETHPORTS] = { 0 }; + uint16_t slave_tx_count; 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; @@ -358,34 +358,12 @@ bond_ethdev_tx_burst_8023ad_fast_queue(void *queue, struct rte_mbuf **bufs, /* If tx burst fails move packets to end of bufs */ if (unlikely(slave_tx_count < slave_nb_bufs[i])) { - slave_tx_fail_count[i] = slave_nb_bufs[i] - + int slave_tx_fail_count = 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]; - } + total_tx_fail_count += slave_tx_fail_count; + memcpy(&bufs[nb_bufs - total_tx_fail_count], + &slave_bufs[i][slave_tx_count], + slave_tx_fail_count * 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; } @@ -1221,10 +1199,10 @@ bond_ethdev_tx_burst_balance(void *queue, struct rte_mbuf **bufs, /* Mapping array generated by hash function to map mbufs to slaves */ uint16_t bufs_slave_port_idxs[nb_bufs]; - uint16_t slave_tx_count, slave_tx_fail_count[RTE_MAX_ETHPORTS] = { 0 }; + uint16_t slave_tx_count; 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; @@ -1265,34 +1243,12 @@ bond_ethdev_tx_burst_balance(void *queue, struct rte_mbuf **bufs, /* If tx burst fails move packets to end of bufs */ if (unlikely(slave_tx_count < slave_nb_bufs[i])) { - slave_tx_fail_count[i] = slave_nb_bufs[i] - + int slave_tx_fail_count = 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]; - } + total_tx_fail_count += slave_tx_fail_count; + memcpy(&bufs[nb_bufs - total_tx_fail_count], + &slave_bufs[i][slave_tx_count], + slave_tx_fail_count * sizeof(bufs[0])); } } @@ -1319,10 +1275,10 @@ bond_ethdev_tx_burst_8023ad(void *queue, struct rte_mbuf **bufs, /* Mapping array generated by hash function to map mbufs to slaves */ uint16_t bufs_slave_port_idxs[RTE_MAX_ETHPORTS] = { 0 }; - uint16_t slave_tx_count, slave_tx_fail_count[RTE_MAX_ETHPORTS] = { 0 }; + uint16_t slave_tx_count; 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; @@ -1380,39 +1336,13 @@ bond_ethdev_tx_burst_8023ad(void *queue, struct rte_mbuf **bufs, /* If tx burst fails move packets to end of bufs */ if (unlikely(slave_tx_count < slave_nb_bufs[i])) { - slave_tx_fail_count[i] = slave_nb_bufs[i] - + int slave_tx_fail_count = 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]; - } - } + total_tx_fail_count += slave_tx_fail_count; - /* - * 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 * sizeof(bufs[0])); } } } -- 2.7.4 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/bonding: fix buf corruption in merging un-transmitted packets 2018-08-20 5:18 ` [dpdk-dev] [PATCH v2] net/bonding: fix buf corruption in merging un-transmitted packets Jia Yu @ 2018-08-20 13:58 ` Chas Williams 2018-08-27 13:12 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit 0 siblings, 1 reply; 3+ messages in thread From: Chas Williams @ 2018-08-20 13:58 UTC (permalink / raw) To: jyu; +Cc: dev, Declan Doherty, stable On Mon, Aug 20, 2018 at 2:54 AM Jia Yu <jyu@vmware.com> wrote: > 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. > > Fixes: 09150784a776 ("net/bonding: burst mode hash calculation") > Cc: stable@dpdk.org > Signed-off-by: Jia Yu <jyu@vmware.com> > Acked-by: Chas Williams <chas3@att.com> > --- > drivers/net/bonding/rte_eth_bond_pmd.c | 116 > +++++++-------------------------- > 1 file changed, 23 insertions(+), 93 deletions(-) > > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c > b/drivers/net/bonding/rte_eth_bond_pmd.c > index 58f7377..c745f31 100644 > --- a/drivers/net/bonding/rte_eth_bond_pmd.c > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c > @@ -300,10 +300,10 @@ bond_ethdev_tx_burst_8023ad_fast_queue(void *queue, > struct rte_mbuf **bufs, > /* Mapping array generated by hash function to map mbufs to slaves > */ > uint16_t bufs_slave_port_idxs[RTE_MAX_ETHPORTS] = { 0 }; > > - uint16_t slave_tx_count, slave_tx_fail_count[RTE_MAX_ETHPORTS] = { > 0 }; > + uint16_t slave_tx_count; > 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; > @@ -358,34 +358,12 @@ bond_ethdev_tx_burst_8023ad_fast_queue(void *queue, > struct rte_mbuf **bufs, > > /* If tx burst fails move packets to end of bufs */ > if (unlikely(slave_tx_count < slave_nb_bufs[i])) { > - slave_tx_fail_count[i] = slave_nb_bufs[i] - > + int slave_tx_fail_count = 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]; > - } > + total_tx_fail_count += slave_tx_fail_count; > + memcpy(&bufs[nb_bufs - total_tx_fail_count], > + &slave_bufs[i][slave_tx_count], > + slave_tx_fail_count * 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; > } > @@ -1221,10 +1199,10 @@ bond_ethdev_tx_burst_balance(void *queue, struct > rte_mbuf **bufs, > /* Mapping array generated by hash function to map mbufs to slaves > */ > uint16_t bufs_slave_port_idxs[nb_bufs]; > > - uint16_t slave_tx_count, slave_tx_fail_count[RTE_MAX_ETHPORTS] = { > 0 }; > + uint16_t slave_tx_count; > 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; > @@ -1265,34 +1243,12 @@ bond_ethdev_tx_burst_balance(void *queue, struct > rte_mbuf **bufs, > > /* If tx burst fails move packets to end of bufs */ > if (unlikely(slave_tx_count < slave_nb_bufs[i])) { > - slave_tx_fail_count[i] = slave_nb_bufs[i] - > + int slave_tx_fail_count = 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]; > - } > + total_tx_fail_count += slave_tx_fail_count; > + memcpy(&bufs[nb_bufs - total_tx_fail_count], > + &slave_bufs[i][slave_tx_count], > + slave_tx_fail_count * sizeof(bufs[0])); > } > } > > @@ -1319,10 +1275,10 @@ bond_ethdev_tx_burst_8023ad(void *queue, struct > rte_mbuf **bufs, > /* Mapping array generated by hash function to map mbufs to slaves > */ > uint16_t bufs_slave_port_idxs[RTE_MAX_ETHPORTS] = { 0 }; > > - uint16_t slave_tx_count, slave_tx_fail_count[RTE_MAX_ETHPORTS] = { > 0 }; > + uint16_t slave_tx_count; > 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; > @@ -1380,39 +1336,13 @@ bond_ethdev_tx_burst_8023ad(void *queue, struct > rte_mbuf **bufs, > > /* If tx burst fails move packets to end of bufs */ > if (unlikely(slave_tx_count < slave_nb_bufs[i])) { > - slave_tx_fail_count[i] = slave_nb_bufs[i] - > + int slave_tx_fail_count = 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]; > - } > - } > + total_tx_fail_count += slave_tx_fail_count; > > - /* > - * 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 * > sizeof(bufs[0])); > } > } > } > -- > 2.7.4 > > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [dpdk-dev] [dpdk-stable] [PATCH v2] net/bonding: fix buf corruption in merging un-transmitted packets 2018-08-20 13:58 ` Chas Williams @ 2018-08-27 13:12 ` Ferruh Yigit 0 siblings, 0 replies; 3+ messages in thread From: Ferruh Yigit @ 2018-08-27 13:12 UTC (permalink / raw) To: Chas Williams, jyu; +Cc: dev, Declan Doherty, stable On 8/20/2018 2:58 PM, Chas Williams wrote: > On Mon, Aug 20, 2018 at 2:54 AM Jia Yu <jyu@vmware.com> wrote: > >> 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. >> >> Fixes: 09150784a776 ("net/bonding: burst mode hash calculation") >> Cc: stable@dpdk.org >> Signed-off-by: Jia Yu <jyu@vmware.com> >> > > Acked-by: Chas Williams <chas3@att.com> Applied to dpdk-next-net/master, thanks. ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-08-27 13:12 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <net/bonding: fix buf corruption in merging un-transmitted packets>
2018-08-20 5:18 ` [dpdk-dev] [PATCH v2] net/bonding: fix buf corruption in merging un-transmitted packets Jia Yu
2018-08-20 13:58 ` Chas Williams
2018-08-27 13:12 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).