List comments

GET /api/patches/73372/comments/
Content-Type: application/json
Vary: Accept

        "id": 115476,
        "web_url": "",
        "msgid": "<>",
        "date": "2020-07-08T09:13:28",
        "subject": "Re: [dpdk-dev] [PATCH v2] net/bonding: change the state machine to\n\tdefaulted",
        "submitter": {
            "id": 1405,
            "url": "",
            "name": "Wei Hu (Xavier)",
            "email": ""
        "content": "Hi, Weifeng Li\n\n\nOn 2020/7/7 22:38, Weifeng Li wrote:\n> From: Weifeng Li <>\n>\n> A dpdk bonding 802.3ad network as follows:\n> +----------+                     +-----------+\n> |dpdk lacp |slave1 <------> port1|switch lacp|\n> |          |slave2 <------> port2|           |\n> +----------+                     +-----------+\n> If a fiber optic go wrong about single pass during normal running like\n> this:\n> slave2 -----> port2 ok\n> slave2 <----- port2 error: salve2 receive no LACPDU Some packets from\n> \t\t\t   switch to dpdk will choose port2 and lost.\n>\n> DPDK lacp state machine will transits to the expired state if no LACPDU\n> is received before the current_while_timer expires. But if no LACPDU is\n> received before the current_while_timer expires again, DPDK lacp state\n> machine has no change. Port2 can not change to inactive depend on the\n> received LACPDU.\n> According to IEEE 802.3ad, if no lacpdu is received before the\n> current_while_timer expires again, the state machine should transits from\n> expired to defaulted. Port2 will change to inactive depend on the LACPDU\n> with defaulted state.\n>\n> This patch adds a state machine change from expired to defaulted when no\n> lacpdu is received before the current_while_timer expires again according\n> to IEEE 802.3ad:\n> If no LACPDU is received before the current_while timer expires again,\n> the state machine transits to the DEFAULTED state. The record Default\n> function overwrites the current operational parameters for the Partner\n> with administratively configured values. This allows configuration of\n> aggregations and individual links when no protocol partner is present,\n> while still permitting an active partner to override default settings.\n> The update_Default_Selected function sets the Selected variable FALSE\n> if the Link Aggregation Group has changed. Since all operational parameters\n> are now set to locally administered values there can be no disagreement as\n> to the Link Aggregation Group, so the Matched variable is set TRUE.\n>\n> The relevant description is in the chapter 43.4.12 of the link below:\n>\n>\n> Signed-off-by: Weifeng Li <>\n> ---\n> v1 -> v2: adjust the formate of commit log\n> ---\n>   drivers/net/bonding/eth_bond_8023ad_private.h |  2 ++\n>   drivers/net/bonding/rte_eth_bond_8023ad.c     | 21 +++++++++++++++++----\n>   2 files changed, 19 insertions(+), 4 deletions(-)\n>\n> diff --git a/drivers/net/bonding/eth_bond_8023ad_private.h b/drivers/net/bonding/eth_bond_8023ad_private.h\n> index 6e44ffd..76f8b8d 100644\n> --- a/drivers/net/bonding/eth_bond_8023ad_private.h\n> +++ b/drivers/net/bonding/eth_bond_8023ad_private.h\n> @@ -50,6 +50,7 @@\n>   #define SM_FLAGS_MOVED                      0x0100\n>   #define SM_FLAGS_PARTNER_SHORT_TIMEOUT      0x0200\n>   #define SM_FLAGS_NTT                        0x0400\n> +#define SM_FLAGS_EXPIRED                    0x0800\n>   \n>   #define BOND_LINK_FULL_DUPLEX_KEY           0x01\n>   #define BOND_LINK_SPEED_KEY_10M             0x02\n> @@ -103,6 +104,7 @@ struct port {\n>   \n>   \t/** The operational Partner's port parameters */\n>   \tstruct port_params partner;\n> +\tstruct port_params partner_admin;\nCould you add some description about partner_admin here or in the commit \nlog?\nIt would be better if there was decription from IEEE spec.\n\nThanks, Xavier\n>   \n>   \t/* Additional port parameters not listed in documentation */\n>   \t/** State machine flags */\n> diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c b/drivers/net/bonding/rte_eth_bond_8023ad.c\n> index b77a37d..bfa418d 100644\n> --- a/drivers/net/bonding/rte_eth_bond_8023ad.c\n> +++ b/drivers/net/bonding/rte_eth_bond_8023ad.c\n> @@ -356,16 +356,28 @@ rx_machine(struct bond_dev_private *internals, uint16_t slave_id,\n>   \n>   \t\ttimer_set(&port->current_while_timer, timeout);\n>   \t\tACTOR_STATE_CLR(port, EXPIRED);\n> +\t\tSM_FLAG_CLR(port, EXPIRED);\n>   \t\treturn; /* No state change */\n>   \t}\n>   \n>   \t/* If CURRENT state timer is not running (stopped or expired)\n>   \t * transit to EXPIRED state from DISABLED or CURRENT */\n>   \tif (!timer_is_running(&port->current_while_timer)) {\n> -\t\tACTOR_STATE_SET(port, EXPIRED);\n> -\t\tPARTNER_STATE_CLR(port, SYNCHRONIZATION);\n> -\t\tPARTNER_STATE_SET(port, LACP_SHORT_TIMEOUT);\n> -\t\ttimer_set(&port->current_while_timer, internals->mode4.short_timeout);\n> +\t\tif (SM_FLAG(port, EXPIRED)) {\n> +\t\t\tport->selected = UNSELECTED;\n> +\t\t\tmemcpy(&port->partner, &port->partner_admin,\n> +\t\t\t\tsizeof(struct port_params));\n> +\t\t\trecord_default(port);\n> +\t\t\tACTOR_STATE_CLR(port, EXPIRED);\n> +\t\t\ttimer_cancel(&port->current_while_timer);\n> +\t\t} else {\n> +\t\t\tSM_FLAG_SET(port, EXPIRED);\n> +\t\t\tACTOR_STATE_SET(port, EXPIRED);\n> +\t\t\tPARTNER_STATE_CLR(port, SYNCHRONIZATION);\n> +\t\t\tPARTNER_STATE_SET(port, LACP_SHORT_TIMEOUT);\n> +\t\t\ttimer_set(&port->current_while_timer,\n> +\t\t\t\tinternals->mode4.short_timeout);\n> +\t\t}\n>   \t}\n>   }\n>   \n> @@ -1020,6 +1032,7 @@ bond_mode_8023ad_activate_slave(struct rte_eth_dev *bond_dev,\n>   \tport->actor.port_number = rte_cpu_to_be_16(slave_id + 1);\n>   \n>   \tmemcpy(&port->partner, &initial, sizeof(struct port_params));\n> +\tmemcpy(&port->partner_admin, &initial, sizeof(struct port_params));\n>   \n>   \t/* default states */\n>   \tport->actor_state = STATE_AGGREGATION | STATE_LACP_ACTIVE | STATE_DEFAULTED;",
        "headers": {
            "List-Archive": "<>",
            "Return-Path": "<>",
            "User-Agent": "Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101\n Thunderbird/45.7.1",
            "To": "Weifeng Li <>",
            "List-Subscribe": "<>,\n <>",
            "Message-ID": "<>",
            "CC": "<>, <>, <>,\n <>, \"Wei Hu (Xavier)\" <>",
            "X-BeenThere": "",
            "Received": [
                "from ( [])\n\tby (Postfix) with ESMTP id 6DE53A00BE;\n\tWed,  8 Jul 2020 11:13:39 +0200 (CEST)",
                "from [] (localhost [])\n\tby (Postfix) with ESMTP id C3B4D1DB6E;\n\tWed,  8 Jul 2020 11:13:38 +0200 (CEST)",
                "from ( [])\n by (Postfix) with ESMTP id 4B54B1DB4B\n for <>; Wed,  8 Jul 2020 11:13:35 +0200 (CEST)",
                "from (unknown [])\n by Forcepoint Email with ESMTP id D5A5A8478F98F8F6B36E;\n Wed,  8 Jul 2020 17:13:33 +0800 (CST)",
                "from [] ( by\n ( with Microsoft SMTP Server id 14.3.487.0;\n Wed, 8 Jul 2020 17:13:28 +0800"
            "Subject": "Re: [dpdk-dev] [PATCH v2] net/bonding: change the state machine to\n\tdefaulted",
            "In-Reply-To": "<>",
            "List-Id": "DPDK patches and discussions <>",
            "List-Unsubscribe": "<>,\n <>",
            "List-Post": "<>",
            "Date": "Wed, 8 Jul 2020 17:13:28 +0800",
            "Precedence": "list",
            "From": "\"Wei Hu (Xavier)\" <>",
            "Content-Transfer-Encoding": "8bit",
            "MIME-Version": "1.0",
            "References": "<>",
            "X-Original-To": "",
            "Sender": "\"dev\" <>",
            "Errors-To": "",
            "X-CFilter-Loop": "Reflected",
            "List-Help": "<>",
            "Content-Type": "text/plain; charset=\"UTF-8\"; format=flowed",
            "Delivered-To": "",
            "X-Originating-IP": "[]",
            "X-Mailman-Version": "2.1.15"