From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <3chas3@gmail.com> Received: from mail-it0-f45.google.com (mail-it0-f45.google.com [209.85.214.45]) by dpdk.org (Postfix) with ESMTP id 0CD5A2C5; Mon, 20 Aug 2018 15:58:46 +0200 (CEST) Received: by mail-it0-f45.google.com with SMTP id 72-v6so20679519itw.3; Mon, 20 Aug 2018 06:58:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=TFQBYzq19vGW2uMvm+rVcYIGYDxTj16j+RqgwiPwFmo=; b=JDTgtUv5Vcr3Hool0vLEkUc7qRNuBQDG/s+d+soJhyCHxz9oveI2nZZSnePWXX0pxU FG8v+uNap7+LJkQDTmqMz6inwJi6awMc3Yw1jNWtHzvIcguy0i1efyLJMUBv6IA0fPuu EvU7FmnYC4MX+PMFIsz4X+lpviMkP+RxNv97/NKGsjhgatSB/U+J56kaKLAqibR4++Sv qfsDkzhse8OL153B8kexcp22flmOLUXbf6Wo0trqn7bwYskasuyobBImB4DLQowAWTO0 gI1D5pQqYnu0+c9ECwgLTQQwYhzZGfrtfKXnlp84RfGc/irofKcmjEOazreHKomP0v7k vKNw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=TFQBYzq19vGW2uMvm+rVcYIGYDxTj16j+RqgwiPwFmo=; b=FLLuNisDtvdBxW6r6HyUauTGZK4J1LmdqzmnGtWKDpRpoteiG+dQP/A7Q7wLhvVrTL IB1PTDNipgtM8TosaC1hmqPxi/4vBQNsDlgbDcJcBvq9teuNEBvBde/XigOWU/2XTtI4 vGGgiOy4jHU0P0E9UNGyrzhwqhk03pb9ONIK8qoNmYLQHNWT5iirsqHEV9ANIcLMHS+I ViT4ol/xB+lnlTa/tWGBCAsiFZNdwHxAvZ7n3C/KwpWSYwXASPDcI6cy885l5KIqs3C/ 26zUayF5lz5EN3Q/SxPLhRqVjrqSK1HaU+u4hjmfJYY84QXePsYIZanZC51l1dv9cQ4Q MPhg== X-Gm-Message-State: AOUpUlF/sYPOGPOw/ywaM6p3XdNsdE5RMY+R225uEU1guwtCHHVQr6RY GGy89I5MdJV4tX+3PeE2R0GpdPD0scLXHLSssrM= X-Google-Smtp-Source: AA+uWPw5vy/OgaTO+YpXOQh5n+xKujxpS0oCT5cf9rgefXqdBYM6/scounXnrYQMG60U7KTGRALz2NwDyRH5YVoNZ/M= X-Received: by 2002:a02:b38c:: with SMTP id p12-v6mr5170559jan.79.1534773525215; Mon, 20 Aug 2018 06:58:45 -0700 (PDT) MIME-Version: 1.0 References: <1534742325-10181-1-git-send-email-jyu@vmware.com> In-Reply-To: <1534742325-10181-1-git-send-email-jyu@vmware.com> From: Chas Williams <3chas3@gmail.com> Date: Mon, 20 Aug 2018 09:58:34 -0400 Message-ID: To: jyu@vmware.com Cc: dev@dpdk.org, Declan Doherty , stable@dpdk.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v2] net/bonding: fix buf corruption in merging un-transmitted packets X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 20 Aug 2018 13:58:46 -0000 On Mon, Aug 20, 2018 at 2:54 AM Jia Yu 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 > Acked-by: Chas Williams > --- > 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 > >