Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/52592/?format=api
{ "id": 52592, "url": "http://patches.dpdk.org/api/patches/52592/?format=api", "web_url": "http://patches.dpdk.org/project/dpdk/patch/1554900829-16180-2-git-send-email-david.marchand@redhat.com/", "project": { "id": 1, "url": "http://patches.dpdk.org/api/projects/1/?format=api", "name": "DPDK", "link_name": "dpdk", "list_id": "dev.dpdk.org", "list_email": "dev@dpdk.org", "web_url": "http://core.dpdk.org", "scm_url": "git://dpdk.org/dpdk", "webscm_url": "http://git.dpdk.org/dpdk", "list_archive_url": "https://inbox.dpdk.org/dev", "list_archive_url_format": "https://inbox.dpdk.org/dev/{}", "commit_url_format": "" }, "msgid": "<1554900829-16180-2-git-send-email-david.marchand@redhat.com>", "list_archive_url": "https://inbox.dpdk.org/dev/1554900829-16180-2-git-send-email-david.marchand@redhat.com", "date": "2019-04-10T12:53:46", "name": "[1/4] net/bonding: fix oob access in LACP mode when sending many packets", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": true, "hash": "51609f86deaee0cbc844ce074641d66ff2a39360", "submitter": { "id": 1173, "url": "http://patches.dpdk.org/api/people/1173/?format=api", "name": "David Marchand", "email": "david.marchand@redhat.com" }, "delegate": { "id": 319, "url": "http://patches.dpdk.org/api/users/319/?format=api", "username": "fyigit", "first_name": "Ferruh", "last_name": "Yigit", "email": "ferruh.yigit@amd.com" }, "mbox": "http://patches.dpdk.org/project/dpdk/patch/1554900829-16180-2-git-send-email-david.marchand@redhat.com/mbox/", "series": [ { "id": 4240, "url": "http://patches.dpdk.org/api/series/4240/?format=api", "web_url": "http://patches.dpdk.org/project/dpdk/list/?series=4240", "date": "2019-04-10T12:53:45", "name": "lacp rx/tx handlers fixes for bonding pmd", "version": 1, "mbox": "http://patches.dpdk.org/series/4240/mbox/" } ], "comments": "http://patches.dpdk.org/api/patches/52592/comments/", "check": "success", "checks": "http://patches.dpdk.org/api/patches/52592/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [127.0.0.1])\n\tby dpdk.org (Postfix) with ESMTP id 8946A1B122;\n\tWed, 10 Apr 2019 14:54:11 +0200 (CEST)", "from mx1.redhat.com (mx1.redhat.com [209.132.183.28])\n\tby dpdk.org (Postfix) with ESMTP id 7CB407CDA;\n\tWed, 10 Apr 2019 14:54:05 +0200 (CEST)", "from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id D1818316A8EC;\n\tWed, 10 Apr 2019 12:54:04 +0000 (UTC)", "from dmarchan.remote.csb (ovpn-204-129.brq.redhat.com\n\t[10.40.204.129])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 6A8B65C223;\n\tWed, 10 Apr 2019 12:54:03 +0000 (UTC)" ], "From": "David Marchand <david.marchand@redhat.com>", "To": "dev@dpdk.org", "Cc": "chas3@att.com,\n\tp.oltarzewski@gmail.com,\n\tstable@dpdk.org", "Date": "Wed, 10 Apr 2019 14:53:46 +0200", "Message-Id": "<1554900829-16180-2-git-send-email-david.marchand@redhat.com>", "In-Reply-To": "<1554900829-16180-1-git-send-email-david.marchand@redhat.com>", "References": "<1554900829-16180-1-git-send-email-david.marchand@redhat.com>", "X-Scanned-By": "MIMEDefang 2.79 on 10.5.11.16", "X-Greylist": "Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.41]); Wed, 10 Apr 2019 12:54:04 +0000 (UTC)", "Subject": "[dpdk-dev] [PATCH 1/4] net/bonding: fix oob access in LACP mode\n\twhen sending many packets", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "DPDK patches and discussions <dev.dpdk.org>", "List-Unsubscribe": "<https://mails.dpdk.org/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://mails.dpdk.org/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<https://mails.dpdk.org/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "content": "We'd better consolidate the fast queue and the normal tx burst functions\nunder a common inline wrapper for maintenance.\n\nBut looking closer at the bufs_slave_port_idxs[] mapping array in those\ntx burst functions, its size is invalid since up to nb_bufs are handled\nhere.\nA previous patch [1] fixed this issue for balance tx burst function\nwithout mentionning it.\n\n802.3ad and balance modes are functionally equivalent on transmit.\nThe only difference is on the slave id distribution.\nAdd an additional inline wrapper to consolidate even more and fix this\nissue.\n\n[1]: https://git.dpdk.org/dpdk/commit/?id=c5224f623431\n\nFixes: 09150784a776 (\"net/bonding: burst mode hash calculation\")\nCc: stable@dpdk.org\n\nSigned-off-by: David Marchand <david.marchand@redhat.com>\n---\n drivers/net/bonding/rte_eth_bond_pmd.c | 213 ++++++++-------------------------\n 1 file changed, 51 insertions(+), 162 deletions(-)", "diff": "diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c\nindex f30422a..c193d6d 100644\n--- a/drivers/net/bonding/rte_eth_bond_pmd.c\n+++ b/drivers/net/bonding/rte_eth_bond_pmd.c\n@@ -2,6 +2,7 @@\n * Copyright(c) 2010-2017 Intel Corporation\n */\n #include <stdlib.h>\n+#include <stdbool.h>\n #include <netinet/in.h>\n \n #include <rte_mbuf.h>\n@@ -295,97 +296,6 @@\n }\n \n static uint16_t\n-bond_ethdev_tx_burst_8023ad_fast_queue(void *queue, struct rte_mbuf **bufs,\n-\t\tuint16_t nb_bufs)\n-{\n-\tstruct bond_tx_queue *bd_tx_q = (struct bond_tx_queue *)queue;\n-\tstruct bond_dev_private *internals = bd_tx_q->dev_private;\n-\n-\tuint16_t slave_port_ids[RTE_MAX_ETHPORTS];\n-\tuint16_t slave_count;\n-\n-\tuint16_t dist_slave_port_ids[RTE_MAX_ETHPORTS];\n-\tuint16_t dist_slave_count;\n-\n-\t/* 2-D array to sort mbufs for transmission on each slave into */\n-\tstruct rte_mbuf *slave_bufs[RTE_MAX_ETHPORTS][nb_bufs];\n-\t/* Number of mbufs for transmission on each slave */\n-\tuint16_t slave_nb_bufs[RTE_MAX_ETHPORTS] = { 0 };\n-\t/* Mapping array generated by hash function to map mbufs to slaves */\n-\tuint16_t bufs_slave_port_idxs[RTE_MAX_ETHPORTS] = { 0 };\n-\n-\tuint16_t slave_tx_count;\n-\tuint16_t total_tx_count = 0, total_tx_fail_count = 0;\n-\n-\tuint16_t i;\n-\n-\tif (unlikely(nb_bufs == 0))\n-\t\treturn 0;\n-\n-\t/* Copy slave list to protect against slave up/down changes during tx\n-\t * bursting */\n-\tslave_count = internals->active_slave_count;\n-\tif (unlikely(slave_count < 1))\n-\t\treturn 0;\n-\n-\tmemcpy(slave_port_ids, internals->active_slaves,\n-\t\t\tsizeof(slave_port_ids[0]) * slave_count);\n-\n-\n-\tdist_slave_count = 0;\n-\tfor (i = 0; i < slave_count; i++) {\n-\t\tstruct port *port = &bond_mode_8023ad_ports[slave_port_ids[i]];\n-\n-\t\tif (ACTOR_STATE(port, DISTRIBUTING))\n-\t\t\tdist_slave_port_ids[dist_slave_count++] =\n-\t\t\t\t\tslave_port_ids[i];\n-\t}\n-\n-\tif (unlikely(dist_slave_count < 1))\n-\t\treturn 0;\n-\n-\t/*\n-\t * Populate slaves mbuf with the packets which are to be sent on it\n-\t * selecting output slave using hash based on xmit policy\n-\t */\n-\tinternals->burst_xmit_hash(bufs, nb_bufs, dist_slave_count,\n-\t\t\tbufs_slave_port_idxs);\n-\n-\tfor (i = 0; i < nb_bufs; i++) {\n-\t\t/* Populate slave mbuf arrays with mbufs for that slave. */\n-\t\tuint16_t slave_idx = bufs_slave_port_idxs[i];\n-\n-\t\tslave_bufs[slave_idx][slave_nb_bufs[slave_idx]++] = bufs[i];\n-\t}\n-\n-\n-\t/* Send packet burst on each slave device */\n-\tfor (i = 0; i < dist_slave_count; i++) {\n-\t\tif (slave_nb_bufs[i] == 0)\n-\t\t\tcontinue;\n-\n-\t\tslave_tx_count = rte_eth_tx_burst(dist_slave_port_ids[i],\n-\t\t\t\tbd_tx_q->queue_id, slave_bufs[i],\n-\t\t\t\tslave_nb_bufs[i]);\n-\n-\t\ttotal_tx_count += slave_tx_count;\n-\n-\t\t/* If tx burst fails move packets to end of bufs */\n-\t\tif (unlikely(slave_tx_count < slave_nb_bufs[i])) {\n-\t\t\tint slave_tx_fail_count = slave_nb_bufs[i] -\n-\t\t\t\t\tslave_tx_count;\n-\t\t\ttotal_tx_fail_count += slave_tx_fail_count;\n-\t\t\tmemcpy(&bufs[nb_bufs - total_tx_fail_count],\n-\t\t\t &slave_bufs[i][slave_tx_count],\n-\t\t\t slave_tx_fail_count * sizeof(bufs[0]));\n-\t\t}\n-\t}\n-\n-\treturn total_tx_count;\n-}\n-\n-\n-static uint16_t\n bond_ethdev_rx_burst_8023ad(void *queue, struct rte_mbuf **bufs,\n \t\tuint16_t nb_pkts)\n {\n@@ -1200,16 +1110,13 @@ struct bwg_slave {\n \treturn num_tx_total;\n }\n \n-static uint16_t\n-bond_ethdev_tx_burst_balance(void *queue, struct rte_mbuf **bufs,\n-\t\tuint16_t nb_bufs)\n+static inline uint16_t\n+tx_burst_balance(void *queue, struct rte_mbuf **bufs, uint16_t nb_bufs,\n+\t\t uint16_t *slave_port_ids, uint16_t slave_count)\n {\n \tstruct bond_tx_queue *bd_tx_q = (struct bond_tx_queue *)queue;\n \tstruct bond_dev_private *internals = bd_tx_q->dev_private;\n \n-\tuint16_t slave_port_ids[RTE_MAX_ETHPORTS];\n-\tuint16_t slave_count;\n-\n \t/* Array to sort mbufs for transmission on each slave into */\n \tstruct rte_mbuf *slave_bufs[RTE_MAX_ETHPORTS][nb_bufs];\n \t/* Number of mbufs for transmission on each slave */\n@@ -1222,18 +1129,6 @@ struct bwg_slave {\n \n \tuint16_t i;\n \n-\tif (unlikely(nb_bufs == 0))\n-\t\treturn 0;\n-\n-\t/* Copy slave list to protect against slave up/down changes during tx\n-\t * bursting */\n-\tslave_count = internals->active_slave_count;\n-\tif (unlikely(slave_count < 1))\n-\t\treturn 0;\n-\n-\tmemcpy(slave_port_ids, internals->active_slaves,\n-\t\t\tsizeof(slave_port_ids[0]) * slave_count);\n-\n \t/*\n \t * Populate slaves mbuf with the packets which are to be sent on it\n \t * selecting output slave using hash based on xmit policy\n@@ -1274,7 +1169,7 @@ struct bwg_slave {\n }\n \n static uint16_t\n-bond_ethdev_tx_burst_8023ad(void *queue, struct rte_mbuf **bufs,\n+bond_ethdev_tx_burst_balance(void *queue, struct rte_mbuf **bufs,\n \t\tuint16_t nb_bufs)\n {\n \tstruct bond_tx_queue *bd_tx_q = (struct bond_tx_queue *)queue;\n@@ -1283,18 +1178,36 @@ struct bwg_slave {\n \tuint16_t slave_port_ids[RTE_MAX_ETHPORTS];\n \tuint16_t slave_count;\n \n+\tif (unlikely(nb_bufs == 0))\n+\t\treturn 0;\n+\n+\t/* Copy slave list to protect against slave up/down changes during tx\n+\t * bursting\n+\t */\n+\tslave_count = internals->active_slave_count;\n+\tif (unlikely(slave_count < 1))\n+\t\treturn 0;\n+\n+\tmemcpy(slave_port_ids, internals->active_slaves,\n+\t\t\tsizeof(slave_port_ids[0]) * slave_count);\n+\treturn tx_burst_balance(queue, bufs, nb_bufs, slave_port_ids,\n+\t\t\t\tslave_count);\n+}\n+\n+static inline uint16_t\n+tx_burst_8023ad(void *queue, struct rte_mbuf **bufs, uint16_t nb_bufs,\n+\t\tbool dedicated_txq)\n+{\n+\tstruct bond_tx_queue *bd_tx_q = (struct bond_tx_queue *)queue;\n+\tstruct bond_dev_private *internals = bd_tx_q->dev_private;\n+\n+\tuint16_t slave_port_ids[RTE_MAX_ETHPORTS];\n+\tuint16_t slave_count;\n+\n \tuint16_t dist_slave_port_ids[RTE_MAX_ETHPORTS];\n \tuint16_t dist_slave_count;\n \n-\t/* 2-D array to sort mbufs for transmission on each slave into */\n-\tstruct rte_mbuf *slave_bufs[RTE_MAX_ETHPORTS][nb_bufs];\n-\t/* Number of mbufs for transmission on each slave */\n-\tuint16_t slave_nb_bufs[RTE_MAX_ETHPORTS] = { 0 };\n-\t/* Mapping array generated by hash function to map mbufs to slaves */\n-\tuint16_t bufs_slave_port_idxs[RTE_MAX_ETHPORTS] = { 0 };\n-\n \tuint16_t slave_tx_count;\n-\tuint16_t total_tx_count = 0, total_tx_fail_count = 0;\n \n \tuint16_t i;\n \n@@ -1307,6 +1220,9 @@ struct bwg_slave {\n \tmemcpy(slave_port_ids, internals->active_slaves,\n \t\t\tsizeof(slave_port_ids[0]) * slave_count);\n \n+\tif (dedicated_txq)\n+\t\tgoto skip_tx_ring;\n+\n \t/* Check for LACP control packets and send if available */\n \tfor (i = 0; i < slave_count; i++) {\n \t\tstruct port *port = &bond_mode_8023ad_ports[slave_port_ids[i]];\n@@ -1328,6 +1244,7 @@ struct bwg_slave {\n \t\t}\n \t}\n \n+skip_tx_ring:\n \tif (unlikely(nb_bufs == 0))\n \t\treturn 0;\n \n@@ -1340,53 +1257,25 @@ struct bwg_slave {\n \t\t\t\t\tslave_port_ids[i];\n \t}\n \n-\tif (likely(dist_slave_count > 0)) {\n-\n-\t\t/*\n-\t\t * Populate slaves mbuf with the packets which are to be sent\n-\t\t * on it, selecting output slave using hash based on xmit policy\n-\t\t */\n-\t\tinternals->burst_xmit_hash(bufs, nb_bufs, dist_slave_count,\n-\t\t\t\tbufs_slave_port_idxs);\n-\n-\t\tfor (i = 0; i < nb_bufs; i++) {\n-\t\t\t/*\n-\t\t\t * Populate slave mbuf arrays with mbufs for that\n-\t\t\t * slave\n-\t\t\t */\n-\t\t\tuint16_t slave_idx = bufs_slave_port_idxs[i];\n-\n-\t\t\tslave_bufs[slave_idx][slave_nb_bufs[slave_idx]++] =\n-\t\t\t\t\tbufs[i];\n-\t\t}\n-\n-\n-\t\t/* Send packet burst on each slave device */\n-\t\tfor (i = 0; i < dist_slave_count; i++) {\n-\t\t\tif (slave_nb_bufs[i] == 0)\n-\t\t\t\tcontinue;\n-\n-\t\t\tslave_tx_count = rte_eth_tx_burst(\n-\t\t\t\t\tdist_slave_port_ids[i],\n-\t\t\t\t\tbd_tx_q->queue_id, slave_bufs[i],\n-\t\t\t\t\tslave_nb_bufs[i]);\n-\n-\t\t\ttotal_tx_count += slave_tx_count;\n+\tif (unlikely(dist_slave_count < 1))\n+\t\treturn 0;\n \n-\t\t\t/* If tx burst fails move packets to end of bufs */\n-\t\t\tif (unlikely(slave_tx_count < slave_nb_bufs[i])) {\n-\t\t\t\tint slave_tx_fail_count = slave_nb_bufs[i] -\n-\t\t\t\t\t\tslave_tx_count;\n-\t\t\t\ttotal_tx_fail_count += slave_tx_fail_count;\n+\treturn tx_burst_balance(queue, bufs, nb_bufs, dist_slave_port_ids,\n+\t\t\t\tdist_slave_count);\n+}\n \n-\t\t\t\tmemcpy(&bufs[nb_bufs - total_tx_fail_count],\n-\t\t\t\t &slave_bufs[i][slave_tx_count],\n-\t\t\t\t slave_tx_fail_count * sizeof(bufs[0]));\n-\t\t\t}\n-\t\t}\n-\t}\n+static uint16_t\n+bond_ethdev_tx_burst_8023ad(void *queue, struct rte_mbuf **bufs,\n+\t\tuint16_t nb_bufs)\n+{\n+\treturn tx_burst_8023ad(queue, bufs, nb_bufs, false);\n+}\n \n-\treturn total_tx_count;\n+static uint16_t\n+bond_ethdev_tx_burst_8023ad_fast_queue(void *queue, struct rte_mbuf **bufs,\n+\t\tuint16_t nb_bufs)\n+{\n+\treturn tx_burst_8023ad(queue, bufs, nb_bufs, true);\n }\n \n static uint16_t\n", "prefixes": [ "1/4" ] }