Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/52595/?format=api
{ "id": 52595, "url": "http://patches.dpdk.org/api/patches/52595/?format=api", "web_url": "http://patches.dpdk.org/project/dpdk/patch/1554900829-16180-5-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-5-git-send-email-david.marchand@redhat.com>", "list_archive_url": "https://inbox.dpdk.org/dev/1554900829-16180-5-git-send-email-david.marchand@redhat.com", "date": "2019-04-10T12:53:49", "name": "[4/4] net/bonding: prefer allmulti to promisc for LACP", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": true, "hash": "7698a5fc46fd566cb1cc278ffc7a8b96e58fac4f", "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-5-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/52595/comments/", "check": "success", "checks": "http://patches.dpdk.org/api/patches/52595/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 DC36C1B14D;\n\tWed, 10 Apr 2019 14:54:23 +0200 (CEST)", "from mx1.redhat.com (mx1.redhat.com [209.132.183.28])\n\tby dpdk.org (Postfix) with ESMTP id 22C1C1B118\n\tfor <dev@dpdk.org>; Wed, 10 Apr 2019 14:54:10 +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 87A95316EB73;\n\tWed, 10 Apr 2019 12:54:09 +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 717AA5C28F;\n\tWed, 10 Apr 2019 12:54:08 +0000 (UTC)" ], "From": "David Marchand <david.marchand@redhat.com>", "To": "dev@dpdk.org", "Cc": "chas3@att.com,\n\tp.oltarzewski@gmail.com", "Date": "Wed, 10 Apr 2019 14:53:49 +0200", "Message-Id": "<1554900829-16180-5-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:09 +0000 (UTC)", "Subject": "[dpdk-dev] [PATCH 4/4] net/bonding: prefer allmulti to promisc for\n\tLACP", "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": "Rather than the global promiscuous mode on all slaves, let's try to use\nallmulti.\nTo do this, we apply the same mechanism than for promiscuous and drop in\nrx_burst.\n\nWhen adding a slave, we first try to use allmulti, else we try\npromiscuous.\nBecause of this, we must be able to handle allmulti on the bonding\ninterface, so the associated dev_ops is added with checks on which mode\nhas been applied on each slave.\n\nRather than add a new flag in the bond private structure, we can remove\npromiscuous_en since ethdev already maintains this information.\nWhen starting the bond device, there is no promisc/allmulti re-apply\nas, again, ethdev does this itself.\n\nOn reception, we must inspect if the packets are multicast or unicast\nand take a decision to drop based on which mode has been enabled on the\nbonding interface.\n\nNote: in the future, if we define an API so that we can add/remove mc\naddresses to a port (rather than the current global set_mc_addr_list\nAPI), we can refine this to only register the LACP multicast mac\naddress.\n\nSigned-off-by: David Marchand <david.marchand@redhat.com>\n---\n drivers/net/bonding/rte_eth_bond_8023ad.c | 51 +++++++++-\n drivers/net/bonding/rte_eth_bond_8023ad_private.h | 7 ++\n drivers/net/bonding/rte_eth_bond_pmd.c | 113 +++++++++++++++++-----\n drivers/net/bonding/rte_eth_bond_private.h | 3 -\n 4 files changed, 148 insertions(+), 26 deletions(-)", "diff": "diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c b/drivers/net/bonding/rte_eth_bond_8023ad.c\nindex 5004898..adf6b7e 100644\n--- a/drivers/net/bonding/rte_eth_bond_8023ad.c\n+++ b/drivers/net/bonding/rte_eth_bond_8023ad.c\n@@ -907,6 +907,49 @@\n \t\t\tbond_mode_8023ad_periodic_cb, arg);\n }\n \n+static int\n+bond_mode_8023ad_register_lacp_mac(uint16_t slave_id)\n+{\n+\trte_eth_allmulticast_enable(slave_id);\n+\tif (rte_eth_allmulticast_get(slave_id)) {\n+\t\tRTE_BOND_LOG(DEBUG, \"forced allmulti for port %u\",\n+\t\t\t slave_id);\n+\t\tbond_mode_8023ad_ports[slave_id].forced_rx_flags =\n+\t\t\t\tBOND_8023AD_FORCED_ALLMULTI;\n+\t\treturn 0;\n+\t}\n+\n+\trte_eth_promiscuous_enable(slave_id);\n+\tif (rte_eth_promiscuous_get(slave_id)) {\n+\t\tRTE_BOND_LOG(DEBUG, \"forced promiscuous for port %u\",\n+\t\t\t slave_id);\n+\t\tbond_mode_8023ad_ports[slave_id].forced_rx_flags =\n+\t\t\t\tBOND_8023AD_FORCED_PROMISC;\n+\t\treturn 0;\n+\t}\n+\n+\treturn -1;\n+}\n+\n+static void\n+bond_mode_8023ad_unregister_lacp_mac(uint16_t slave_id)\n+{\n+\tswitch (bond_mode_8023ad_ports[slave_id].forced_rx_flags) {\n+\tcase BOND_8023AD_FORCED_ALLMULTI:\n+\t\tRTE_BOND_LOG(DEBUG, \"unset allmulti for port %u\", slave_id);\n+\t\trte_eth_allmulticast_disable(slave_id);\n+\t\tbreak;\n+\n+\tcase BOND_8023AD_FORCED_PROMISC:\n+\t\tRTE_BOND_LOG(DEBUG, \"unset promisc for port %u\", slave_id);\n+\t\trte_eth_promiscuous_disable(slave_id);\n+\t\tbreak;\n+\n+\tdefault:\n+\t\tbreak;\n+\t}\n+}\n+\n void\n bond_mode_8023ad_activate_slave(struct rte_eth_dev *bond_dev,\n \t\t\t\tuint16_t slave_id)\n@@ -948,7 +991,11 @@\n \n \t/* use this port as agregator */\n \tport->aggregator_port_id = slave_id;\n-\trte_eth_promiscuous_enable(slave_id);\n+\n+\tif (bond_mode_8023ad_register_lacp_mac(slave_id) < 0) {\n+\t\tRTE_BOND_LOG(WARNING, \"slave %u is most likely broken and won't receive LACP packets\",\n+\t\t\t slave_id);\n+\t}\n \n \ttimer_cancel(&port->warning_timer);\n \n@@ -1022,6 +1069,8 @@\n \told_partner_state = port->partner_state;\n \trecord_default(port);\n \n+\tbond_mode_8023ad_unregister_lacp_mac(slave_id);\n+\n \t/* If partner timeout state changes then disable timer */\n \tif (!((old_partner_state ^ port->partner_state) &\n \t\t\tSTATE_LACP_SHORT_TIMEOUT))\ndiff --git a/drivers/net/bonding/rte_eth_bond_8023ad_private.h b/drivers/net/bonding/rte_eth_bond_8023ad_private.h\nindex f91902e..edda841 100644\n--- a/drivers/net/bonding/rte_eth_bond_8023ad_private.h\n+++ b/drivers/net/bonding/rte_eth_bond_8023ad_private.h\n@@ -109,6 +109,13 @@ struct port {\n \tuint16_t sm_flags;\n \tenum rte_bond_8023ad_selection selected;\n \n+\t/** Indicates if either allmulti or promisc has been enforced on the\n+\t * slave so that we can receive lacp packets\n+\t */\n+#define BOND_8023AD_FORCED_ALLMULTI (1 << 0)\n+#define BOND_8023AD_FORCED_PROMISC (1 << 1)\n+\tuint8_t forced_rx_flags;\n+\n \tuint64_t current_while_timer;\n \tuint64_t periodic_timer;\n \tuint64_t wait_while_timer;\ndiff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c\nindex 9ef0717..07e19c4 100644\n--- a/drivers/net/bonding/rte_eth_bond_pmd.c\n+++ b/drivers/net/bonding/rte_eth_bond_pmd.c\n@@ -274,7 +274,8 @@\n \tuint16_t slave_count, idx;\n \n \tuint8_t collecting; /* current slave collecting status */\n-\tconst uint8_t promisc = internals->promiscuous_en;\n+\tconst uint8_t promisc = rte_eth_promiscuous_get(internals->port_id);\n+\tconst uint8_t allmulti = rte_eth_allmulticast_get(internals->port_id);\n \tuint8_t subtype;\n \tuint16_t i;\n \tuint16_t j;\n@@ -314,8 +315,10 @@\n \t\t\t/* Remove packet from array if:\n \t\t\t * - it is slow packet but no dedicated rxq is present,\n \t\t\t * - slave is not in collecting state,\n-\t\t\t * - bonding interface is not in promiscuous mode and\n-\t\t\t * packet is not multicast and address does not match,\n+\t\t\t * - bonding interface is not in promiscuous mode:\n+\t\t\t * - packet is unicast and address does not match,\n+\t\t\t * - packet is multicast and bonding interface\n+\t\t\t * is not in allmulti,\n \t\t\t */\n \t\t\tif (unlikely(\n \t\t\t\t(!dedicated_rxq &&\n@@ -323,9 +326,11 @@\n \t\t\t\t\t\t bufs[j])) ||\n \t\t\t\t!collecting ||\n \t\t\t\t(!promisc &&\n-\t\t\t\t !is_multicast_ether_addr(&hdr->d_addr) &&\n-\t\t\t\t !is_same_ether_addr(bond_mac,\n-\t\t\t\t\t\t &hdr->d_addr)))) {\n+\t\t\t\t ((is_unicast_ether_addr(&hdr->d_addr) &&\n+\t\t\t\t !is_same_ether_addr(bond_mac,\n+\t\t\t\t\t\t &hdr->d_addr)) ||\n+\t\t\t\t (!allmulti &&\n+\t\t\t\t is_multicast_ether_addr(&hdr->d_addr)))))) {\n \n \t\t\t\tif (hdr->ether_type == ether_type_slow_be) {\n \t\t\t\t\tbond_mode_8023ad_handle_slow_pkt(\n@@ -1924,10 +1929,6 @@ struct bwg_slave {\n \t\t}\n \t}\n \n-\t/* If bonded device is configure in promiscuous mode then re-apply config */\n-\tif (internals->promiscuous_en)\n-\t\tbond_ethdev_promiscuous_enable(eth_dev);\n-\n \tif (internals->mode == BONDING_MODE_8023AD) {\n \t\tif (internals->mode4.dedicated_queues.enabled == 1) {\n \t\t\tinternals->mode4.dedicated_queues.rx_qid =\n@@ -2445,18 +2446,17 @@ struct bwg_slave {\n \tstruct bond_dev_private *internals = eth_dev->data->dev_private;\n \tint i;\n \n-\tinternals->promiscuous_en = 1;\n-\n \tswitch (internals->mode) {\n \t/* Promiscuous mode is propagated to all slaves */\n \tcase BONDING_MODE_ROUND_ROBIN:\n \tcase BONDING_MODE_BALANCE:\n \tcase BONDING_MODE_BROADCAST:\n-\t\tfor (i = 0; i < internals->slave_count; i++)\n-\t\t\trte_eth_promiscuous_enable(internals->slaves[i].port_id);\n-\t\tbreak;\n-\t/* In mode4 promiscus mode is managed when slave is added/removed */\n \tcase BONDING_MODE_8023AD:\n+\t\tfor (i = 0; i < internals->slave_count; i++) {\n+\t\t\tuint16_t port_id = internals->slaves[i].port_id;\n+\n+\t\t\trte_eth_promiscuous_enable(port_id);\n+\t\t}\n \t\tbreak;\n \t/* Promiscuous mode is propagated only to primary slave */\n \tcase BONDING_MODE_ACTIVE_BACKUP:\n@@ -2476,18 +2476,21 @@ struct bwg_slave {\n \tstruct bond_dev_private *internals = dev->data->dev_private;\n \tint i;\n \n-\tinternals->promiscuous_en = 0;\n-\n \tswitch (internals->mode) {\n \t/* Promiscuous mode is propagated to all slaves */\n \tcase BONDING_MODE_ROUND_ROBIN:\n \tcase BONDING_MODE_BALANCE:\n \tcase BONDING_MODE_BROADCAST:\n-\t\tfor (i = 0; i < internals->slave_count; i++)\n-\t\t\trte_eth_promiscuous_disable(internals->slaves[i].port_id);\n-\t\tbreak;\n-\t/* In mode4 promiscus mode is set managed when slave is added/removed */\n \tcase BONDING_MODE_8023AD:\n+\t\tfor (i = 0; i < internals->slave_count; i++) {\n+\t\t\tuint16_t port_id = internals->slaves[i].port_id;\n+\n+\t\t\tif (internals->mode == BONDING_MODE_8023AD &&\n+\t\t\t bond_mode_8023ad_ports[port_id].forced_rx_flags ==\n+\t\t\t\t\tBOND_8023AD_FORCED_PROMISC)\n+\t\t\t\tcontinue;\n+\t\t\trte_eth_promiscuous_disable(port_id);\n+\t\t}\n \t\tbreak;\n \t/* Promiscuous mode is propagated only to primary slave */\n \tcase BONDING_MODE_ACTIVE_BACKUP:\n@@ -2502,6 +2505,70 @@ struct bwg_slave {\n }\n \n static void\n+bond_ethdev_allmulticast_enable(struct rte_eth_dev *eth_dev)\n+{\n+\tstruct bond_dev_private *internals = eth_dev->data->dev_private;\n+\tint i;\n+\n+\tswitch (internals->mode) {\n+\t/* allmulti mode is propagated to all slaves */\n+\tcase BONDING_MODE_ROUND_ROBIN:\n+\tcase BONDING_MODE_BALANCE:\n+\tcase BONDING_MODE_BROADCAST:\n+\tcase BONDING_MODE_8023AD:\n+\t\tfor (i = 0; i < internals->slave_count; i++) {\n+\t\t\tuint16_t port_id = internals->slaves[i].port_id;\n+\n+\t\t\trte_eth_allmulticast_enable(port_id);\n+\t\t}\n+\t\tbreak;\n+\t/* allmulti mode is propagated only to primary slave */\n+\tcase BONDING_MODE_ACTIVE_BACKUP:\n+\tcase BONDING_MODE_TLB:\n+\tcase BONDING_MODE_ALB:\n+\tdefault:\n+\t\t/* Do not touch allmulti when there cannot be primary ports */\n+\t\tif (internals->slave_count == 0)\n+\t\t\tbreak;\n+\t\trte_eth_allmulticast_enable(internals->current_primary_port);\n+\t}\n+}\n+\n+static void\n+bond_ethdev_allmulticast_disable(struct rte_eth_dev *eth_dev)\n+{\n+\tstruct bond_dev_private *internals = eth_dev->data->dev_private;\n+\tint i;\n+\n+\tswitch (internals->mode) {\n+\t/* allmulti mode is propagated to all slaves */\n+\tcase BONDING_MODE_ROUND_ROBIN:\n+\tcase BONDING_MODE_BALANCE:\n+\tcase BONDING_MODE_BROADCAST:\n+\tcase BONDING_MODE_8023AD:\n+\t\tfor (i = 0; i < internals->slave_count; i++) {\n+\t\t\tuint16_t port_id = internals->slaves[i].port_id;\n+\n+\t\t\tif (internals->mode == BONDING_MODE_8023AD &&\n+\t\t\t bond_mode_8023ad_ports[port_id].forced_rx_flags ==\n+\t\t\t\t\tBOND_8023AD_FORCED_ALLMULTI)\n+\t\t\t\tcontinue;\n+\t\t\trte_eth_allmulticast_disable(port_id);\n+\t\t}\n+\t\tbreak;\n+\t/* allmulti mode is propagated only to primary slave */\n+\tcase BONDING_MODE_ACTIVE_BACKUP:\n+\tcase BONDING_MODE_TLB:\n+\tcase BONDING_MODE_ALB:\n+\tdefault:\n+\t\t/* Do not touch allmulti when there cannot be primary ports */\n+\t\tif (internals->slave_count == 0)\n+\t\t\tbreak;\n+\t\trte_eth_allmulticast_disable(internals->current_primary_port);\n+\t}\n+}\n+\n+static void\n bond_ethdev_delayed_lsc_propagation(void *arg)\n {\n \tif (arg == NULL)\n@@ -2893,6 +2960,8 @@ struct bwg_slave {\n \t.stats_reset = bond_ethdev_stats_reset,\n \t.promiscuous_enable = bond_ethdev_promiscuous_enable,\n \t.promiscuous_disable = bond_ethdev_promiscuous_disable,\n+\t.allmulticast_enable = bond_ethdev_allmulticast_enable,\n+\t.allmulticast_disable = bond_ethdev_allmulticast_disable,\n \t.reta_update = bond_ethdev_rss_reta_update,\n \t.reta_query = bond_ethdev_rss_reta_query,\n \t.rss_hash_update = bond_ethdev_rss_hash_update,\ndiff --git a/drivers/net/bonding/rte_eth_bond_private.h b/drivers/net/bonding/rte_eth_bond_private.h\nindex 8afef39..4154e17 100644\n--- a/drivers/net/bonding/rte_eth_bond_private.h\n+++ b/drivers/net/bonding/rte_eth_bond_private.h\n@@ -122,9 +122,6 @@ struct bond_dev_private {\n \n \tuint8_t user_defined_mac;\n \t/**< Flag for whether MAC address is user defined or not */\n-\tuint8_t promiscuous_en;\n-\t/**< Enabled/disable promiscuous mode on bonding device */\n-\n \n \tuint8_t link_status_polling_enabled;\n \tuint32_t link_status_polling_interval_ms;\n", "prefixes": [ "4/4" ] }