From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 487EB6AAE for ; Fri, 5 Dec 2014 09:52:29 +0100 (CET) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga102.jf.intel.com with ESMTP; 05 Dec 2014 00:51:17 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.07,521,1413270000"; d="scan'208";a="648761440" Received: from kmsmsx152.gar.corp.intel.com ([172.21.73.87]) by orsmga002.jf.intel.com with ESMTP; 05 Dec 2014 00:52:27 -0800 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by KMSMSX152.gar.corp.intel.com (172.21.73.87) with Microsoft SMTP Server (TLS) id 14.3.195.1; Fri, 5 Dec 2014 16:52:26 +0800 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.110]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.182]) with mapi id 14.03.0195.001; Fri, 5 Dec 2014 16:52:25 +0800 From: "Qiu, Michael" To: "Richardson, Bruce" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH] ixgbe: fix multi-process support Thread-Index: AQHQD7hY41LZQcv5NUuahXMjpj7AGw== Date: Fri, 5 Dec 2014 08:52:24 +0000 Message-ID: <533710CFB86FA344BFBF2D6802E60286C9D147@SHSMSX101.ccr.corp.intel.com> References: <1417693650-9813-1-git-send-email-bruce.richardson@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] ixgbe: fix multi-process support X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 05 Dec 2014 08:52:30 -0000 On 12/4/2014 7:49 PM, Bruce Richardson wrote:=0A= > When using multiple processes, the TX function used in all processes=0A= > should be the same, otherwise the secondary processes cannot transmit=0A= > more than tx-ring-size - 1 packets.=0A= > To achieve this, we extract out the code to select the ixgbe TX function= =0A= > to be used into a separate function inside the ixgbe driver, and call=0A= > that from a secondary process when it is attaching to an=0A= > already-configured NIC.=0A= >=0A= > Testing with symmetric MP app shows that we are able to RX and TX from=0A= > both primary and secondary processes once this patch is applied.=0A= >=0A= > Signed-off-by: Bruce Richardson =0A= > ---=0A= > lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 7 +++-=0A= > lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 61 +++++++++++++++++++++--------= ------=0A= > lib/librte_pmd_ixgbe/ixgbe_rxtx.h | 7 ++++=0A= > lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c | 3 ++=0A= > 4 files changed, 52 insertions(+), 26 deletions(-)=0A= >=0A= > diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/i= xgbe_ethdev.c=0A= > index 937fc3c..4abab25 100644=0A= > --- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c=0A= > +++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c=0A= > @@ -68,6 +68,7 @@=0A= > #include "ixgbe/ixgbe_common.h"=0A= > #include "ixgbe_ethdev.h"=0A= > #include "ixgbe_bypass.h"=0A= > +#include "ixgbe_rxtx.h"=0A= > =0A= > /*=0A= > * High threshold controlling when to start sending XOFF frames. Must be= at=0A= > @@ -743,8 +744,12 @@ eth_ixgbe_dev_init(__attribute__((unused)) struct et= h_driver *eth_drv,=0A= > =0A= > /* for secondary processes, we don't initialise any further as primary= =0A= > * has already done this work. Only check we don't need a different=0A= > - * RX function */=0A= > + * RX and TX function */=0A= =0A= Can it be more beautiful? like:=0A= /*=0A= * XXXXXXXX=0A= */=0A= =0A= But not a big deal.=0A= =0A= > if (rte_eal_process_type() !=3D RTE_PROC_PRIMARY){=0A= > + struct igb_tx_queue *txq;=0A= > + txq =3D eth_dev->data->tx_queues[eth_dev->data->nb_tx_queues-1];=0A= =0A= Here why chose the last TX queue?=0A= =0A= > + set_tx_function(eth_dev, txq);=0A= > +=0A= > if (eth_dev->data->scattered_rx)=0A= > eth_dev->rx_pkt_burst =3D ixgbe_recv_scattered_pkts;=0A= > return 0;=0A= > diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixg= be_rxtx.c=0A= > index 5c36bff..263c815 100644=0A= > --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c=0A= > +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c=0A= > @@ -1771,6 +1771,40 @@ static struct ixgbe_txq_ops def_txq_ops =3D {=0A= > .reset =3D ixgbe_reset_tx_queue,=0A= > };=0A= > =0A= > +/* takes an ethdev and a queue and sets up the tx function to be used ba= sed on=0A= > + * the queue parameters. Used in tx_queue_setup by primary process and t= hen=0A= > + * in dev_init by secondary process when attaching to an existing ethdev= =0A= > + */=0A= > +void=0A= > +set_tx_function(struct rte_eth_dev* dev, struct igb_tx_queue* txq)=0A= > +{=0A= > + /* Use a simple Tx queue (no offloads, no multi segs) if possible */=0A= > + if (((txq->txq_flags & IXGBE_SIMPLE_FLAGS) =3D=3D IXGBE_SIMPLE_FLAGS)= =0A= > + && (txq->tx_rs_thresh >=3D RTE_PMD_IXGBE_TX_MAX_BURST)) {=0A= > + PMD_INIT_LOG(INFO, "Using simple tx code path");=0A= > +#ifdef RTE_IXGBE_INC_VECTOR=0A= > + if (txq->tx_rs_thresh <=3D RTE_IXGBE_TX_MAX_FREE_BUF_SZ &&=0A= > + ixgbe_txq_vec_setup(txq) =3D=3D 0) {=0A= =0A= Can process type check been add here and removed from ixgbe_txq_vec_setup()= ?=0A= =0A= Because:=0A= 1. set_tx_function() is better to be process type sensitive, it maybe call= ed from all type process.=0A= 2. ixgbe_txq_vec_setup() can keep runs in primary( remove process type che= ck), also we only need to touch three files instead of four :)=0A= =0A= Thanks,=0A= Michael=0A= =0A= > + PMD_INIT_LOG(INFO, "Vector tx enabled.");=0A= > + dev->tx_pkt_burst =3D ixgbe_xmit_pkts_vec;=0A= > + }=0A= > + else=0A= > +#endif=0A= > + dev->tx_pkt_burst =3D ixgbe_xmit_pkts_simple;=0A= > + } else {=0A= > + PMD_INIT_LOG(INFO, "Using full-featured tx code path");=0A= > + PMD_INIT_LOG(INFO,=0A= > + " - txq_flags =3D %lx " "[IXGBE_SIMPLE_FLAGS=3D%lx]",=0A= > + (long unsigned )txq->txq_flags,=0A= > + (long unsigned)IXGBE_SIMPLE_FLAGS);=0A= > + PMD_INIT_LOG(INFO,=0A= > + " - tx_rs_thresh =3D %lu " "[RTE_PMD_IXGBE_TX_MAX_BURST=3D%lu]",=0A= > + (long unsigned )txq->tx_rs_thresh,=0A= > + (long unsigned)RTE_PMD_IXGBE_TX_MAX_BURST);=0A= > + dev->tx_pkt_burst =3D ixgbe_xmit_pkts;=0A= > + }=0A= > +}=0A= > +=0A= > int=0A= > ixgbe_dev_tx_queue_setup(struct rte_eth_dev *dev,=0A= > uint16_t queue_idx,=0A= > @@ -1933,31 +1967,8 @@ ixgbe_dev_tx_queue_setup(struct rte_eth_dev *dev,= =0A= > PMD_INIT_LOG(DEBUG, "sw_ring=3D%p hw_ring=3D%p dma_addr=3D0x%"PRIx64,= =0A= > txq->sw_ring, txq->tx_ring, txq->tx_ring_phys_addr);=0A= > =0A= > - /* Use a simple Tx queue (no offloads, no multi segs) if possible */=0A= > - if (((txq->txq_flags & IXGBE_SIMPLE_FLAGS) =3D=3D IXGBE_SIMPLE_FLAGS) &= &=0A= > - (txq->tx_rs_thresh >=3D RTE_PMD_IXGBE_TX_MAX_BURST)) {=0A= > - PMD_INIT_LOG(INFO, "Using simple tx code path");=0A= > -#ifdef RTE_IXGBE_INC_VECTOR=0A= > - if (txq->tx_rs_thresh <=3D RTE_IXGBE_TX_MAX_FREE_BUF_SZ &&=0A= > - ixgbe_txq_vec_setup(txq) =3D=3D 0) {=0A= > - PMD_INIT_LOG(INFO, "Vector tx enabled.");=0A= > - dev->tx_pkt_burst =3D ixgbe_xmit_pkts_vec;=0A= > - }=0A= > - else=0A= > -#endif=0A= > - dev->tx_pkt_burst =3D ixgbe_xmit_pkts_simple;=0A= > - } else {=0A= > - PMD_INIT_LOG(INFO, "Using full-featured tx code path");=0A= > - PMD_INIT_LOG(INFO, " - txq_flags =3D %lx "=0A= > - "[IXGBE_SIMPLE_FLAGS=3D%lx]",=0A= > - (long unsigned)txq->txq_flags,=0A= > - (long unsigned)IXGBE_SIMPLE_FLAGS);=0A= > - PMD_INIT_LOG(INFO, " - tx_rs_thresh =3D %lu "=0A= > - "[RTE_PMD_IXGBE_TX_MAX_BURST=3D%lu]",=0A= > - (long unsigned)txq->tx_rs_thresh,=0A= > - (long unsigned)RTE_PMD_IXGBE_TX_MAX_BURST);=0A= > - dev->tx_pkt_burst =3D ixgbe_xmit_pkts;=0A= > - }=0A= > + /* set up vector or scalar TX function as appropriate */=0A= > + set_tx_function(dev, txq);=0A= > =0A= > txq->ops->reset(txq);=0A= > =0A= > diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.h b/lib/librte_pmd_ixgbe/ixg= be_rxtx.h=0A= > index 13099af..873656d 100644=0A= > --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.h=0A= > +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.h=0A= > @@ -248,6 +248,13 @@ struct ixgbe_txq_ops {=0A= > IXGBE_ADVTXD_DCMD_DEXT |\=0A= > IXGBE_ADVTXD_DCMD_EOP)=0A= > =0A= > +=0A= > +/* takes an ethdev and a queue and sets up the tx function to be used ba= sed on=0A= > + * the queue parameters. Used in tx_queue_setup by primary process and t= hen=0A= > + * in dev_init by secondary process when attaching to an existing ethdev= =0A= > + */=0A= > +void set_tx_function(struct rte_eth_dev* dev, struct igb_tx_queue* txq);= =0A= > +=0A= > #ifdef RTE_IXGBE_INC_VECTOR=0A= > uint16_t ixgbe_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,= =0A= > uint16_t nb_pkts);=0A= > diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c b/lib/librte_pmd_ixgbe= /ixgbe_rxtx_vec.c=0A= > index 579bc46..6755fad 100644=0A= > --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c=0A= > +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c=0A= > @@ -748,6 +748,9 @@ int ixgbe_txq_vec_setup(struct igb_tx_queue *txq)=0A= > if (txq->sw_ring =3D=3D NULL)=0A= > return -1;=0A= > =0A= > + if (rte_eal_process_type() !=3D RTE_PROC_PRIMARY)=0A= > + return 0;=0A= > +=0A= > /* leave the first one for overflow */=0A= > txq->sw_ring =3D (struct igb_tx_entry *)=0A= > ((struct igb_tx_entry_v *)txq->sw_ring + 1);=0A= =0A=