From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <3chas3@gmail.com> Received: from mail-qk1-f194.google.com (mail-qk1-f194.google.com [209.85.222.194]) by dpdk.org (Postfix) with ESMTP id C91EE1B45E; Mon, 11 Feb 2019 16:34:47 +0100 (CET) Received: by mail-qk1-f194.google.com with SMTP id p15so4979698qkl.5; Mon, 11 Feb 2019 07:34:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=QhY5ApX3QAzYCaIrQZB1ebdji0rDXAi2S2fZ4Iud4+I=; b=jxD0l0hII5JoP8UB1Z+MGxuLZbry73+5h6qMOcHR/Kmk5GSUUwMpqmcsy0F5EJ7UgQ JTqBJBw28Ce4VZKyWmUmUCFaQi7wUizcNrFKa7UAfvtTdP+ww5T1iooCnWPGfGfmoR0+ rqQYvP6VCNSO6jooGlNQ7yf8Ry21EDDdcPHLecWxXoNB53wHYYRLubcSFW/HfBjEuSA3 dbam2HPra2Q5xtIOBVbktx65xFEmXw7YJxlelOMGCge3FeBqO+3DWzXe06iYekvLNj4M BnwtQzwdXH3GQKdZtzLMUfK9lZqyqyXOs+X6Ace3MQ2EyQqMrLpU/mdDPztJLAs5B5kH innA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=QhY5ApX3QAzYCaIrQZB1ebdji0rDXAi2S2fZ4Iud4+I=; b=Sh3jOzb2hwuxtaRfEGqsANKH/DYLmr70GS91ekClCKjqm2+UuGMNN877TIYCWaVunC dN+S0OFowv6wSNG6luVPC1FvXGMPzATuKz105gYOVn/uZX2kskHwZkH5bgcmDLPlNHMu vbPD4dOmQJcGLCnAl4c1t1EfKsAeFrFQQUq6EJIKu8g0U2ht6s8SHVvRbxJh0GGANBtF 6zmQZPFCSULq1DVrXEUzQ3Vrko0XKiR3E4tcadrz5GXmwbPFqPnBgS2BGXTC8PDT9Tgb SvUty6Ro8CP4AOYggcTT8T/2aOyYUU44l77MLmHVg51U3xvAEzbLOZgtqNQJk0OtsFtF DZVg== X-Gm-Message-State: AHQUAuaTQm1QnFDFEjD1yVAXp9dc5GMVEZgZl/QFQJ8NkV96ldU5UkC4 4el/hvosl/JiQg+cidAElaK4Ng1P X-Google-Smtp-Source: AHgI3IaaH4alvLFPhA3RwjUUmvDTISJ09qwS+p+af+jRvkPOMFj9bs4NtrB/e5CuwfGJIY9cHwNzBw== X-Received: by 2002:a37:aa17:: with SMTP id t23mr9393747qke.133.1549899286046; Mon, 11 Feb 2019 07:34:46 -0800 (PST) Received: from [192.168.1.10] (pool-96-255-82-34.washdc.fios.verizon.net. [96.255.82.34]) by smtp.gmail.com with ESMTPSA id p15sm5968728qta.81.2019.02.11.07.34.45 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 11 Feb 2019 07:34:45 -0800 (PST) To: wangyunjian , dev@dpdk.org Cc: chas3@att.com, xudingke@huawei.com, stable@dpdk.org References: <1549872096-16116-1-git-send-email-wangyunjian@huawei.com> From: Chas Williams <3chas3@gmail.com> Message-ID: <6f80e7f3-6d6c-e2e6-861d-d57795c0289c@gmail.com> Date: Mon, 11 Feb 2019 10:34:44 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: <1549872096-16116-1-git-send-email-wangyunjian@huawei.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] net/bonding: fix slave tx burst for mode 4 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, 11 Feb 2019 15:34:48 -0000 How strange. I was just looking at this issue this weekend. I think we can just move the control packet handling before the data packet handling. That avoids the goto and is a little more sensible -- we should try to prioritize control traffic (even if we can't guarantee there will be space it's better than nothing). Something like this: diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c index 44deaf119..89ad67266 100644 --- a/drivers/net/bonding/rte_eth_bond_pmd.c +++ b/drivers/net/bonding/rte_eth_bond_pmd.c @@ -1298,9 +1298,6 @@ bond_ethdev_tx_burst_8023ad(void *queue, struct rte_mbuf **bufs, uint16_t i; - if (unlikely(nb_bufs == 0)) - return 0; - /* Copy slave list to protect against slave up/down changes during tx * bursting */ slave_count = internals->active_slave_count; @@ -1310,6 +1307,30 @@ bond_ethdev_tx_burst_8023ad(void *queue, struct rte_mbuf **bufs, memcpy(slave_port_ids, internals->active_slaves, sizeof(slave_port_ids[0]) * slave_count); + /* Check for LACP control packets and send if available */ + for (i = 0; i < slave_count; i++) { + struct port *port = &bond_mode_8023ad_ports[slave_port_ids[i]]; + struct rte_mbuf *ctrl_pkt = NULL; + + if (likely(rte_ring_empty(port->tx_ring))) + continue; + + if (rte_ring_dequeue(port->tx_ring, + (void **)&ctrl_pkt) != -ENOENT) { + slave_tx_count = rte_eth_tx_burst(slave_port_ids[i], + bd_tx_q->queue_id, &ctrl_pkt, 1); + /* + * re-enqueue LAG control plane packets to buffering + * ring if transmission fails so the packet isn't lost. + */ + if (slave_tx_count != 1) + rte_ring_enqueue(port->tx_ring, ctrl_pkt); + } + } + + if (unlikely(nb_bufs == 0)) + return 0; + dist_slave_count = 0; for (i = 0; i < slave_count; i++) { struct port *port = &bond_mode_8023ad_ports[slave_port_ids[i]]; @@ -1365,27 +1386,6 @@ bond_ethdev_tx_burst_8023ad(void *queue, struct rte_mbuf **bufs, } } - /* Check for LACP control packets and send if available */ - for (i = 0; i < slave_count; i++) { - struct port *port = &bond_mode_8023ad_ports[slave_port_ids[i]]; - struct rte_mbuf *ctrl_pkt = NULL; - - if (likely(rte_ring_empty(port->tx_ring))) - continue; - - if (rte_ring_dequeue(port->tx_ring, - (void **)&ctrl_pkt) != -ENOENT) { - slave_tx_count = rte_eth_tx_burst(slave_port_ids[i], - bd_tx_q->queue_id, &ctrl_pkt, 1); - /* - * re-enqueue LAG control plane packets to buffering - * ring if transmission fails so the packet isn't lost. - */ - if (slave_tx_count != 1) - rte_ring_enqueue(port->tx_ring, ctrl_pkt); - } - } - return total_tx_count; } On 2/11/19 3:01 AM, wangyunjian wrote: > From: Yunjian Wang > > Now sending 0 packet it doesn't consider for LACPDUs, > but the LACPDUs should be checked and sended. > > Fixes: 09150784a776 ("net/bonding: burst mode hash calculation") > Cc: stable@dpdk.org > > Reported-by: Hui Zhao > Signed-off-by: Yunjian Wang > --- > drivers/net/bonding/rte_eth_bond_pmd.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c > index 44deaf1..a77af19 100644 > --- a/drivers/net/bonding/rte_eth_bond_pmd.c > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c > @@ -1298,9 +1298,6 @@ struct bwg_slave { > > uint16_t i; > > - if (unlikely(nb_bufs == 0)) > - return 0; > - > /* Copy slave list to protect against slave up/down changes during tx > * bursting */ > slave_count = internals->active_slave_count; > @@ -1310,6 +1307,9 @@ struct bwg_slave { > memcpy(slave_port_ids, internals->active_slaves, > sizeof(slave_port_ids[0]) * slave_count); > > + if (unlikely(nb_bufs == 0)) > + goto lacp_send; > + > dist_slave_count = 0; > for (i = 0; i < slave_count; i++) { > struct port *port = &bond_mode_8023ad_ports[slave_port_ids[i]]; > @@ -1365,6 +1365,7 @@ struct bwg_slave { > } > } > > +lacp_send: > /* Check for LACP control packets and send if available */ > for (i = 0; i < slave_count; i++) { > struct port *port = &bond_mode_8023ad_ports[slave_port_ids[i]]; >