From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 317F3C330 for ; Wed, 17 Feb 2016 17:44:51 +0100 (CET) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga102.fm.intel.com with ESMTP; 17 Feb 2016 08:44:50 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,461,1449561600"; d="scan'208";a="917567765" Received: from irsmsx110.ger.corp.intel.com ([163.33.3.25]) by fmsmga002.fm.intel.com with ESMTP; 17 Feb 2016 08:44:49 -0800 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.237]) by irsmsx110.ger.corp.intel.com ([169.254.15.31]) with mapi id 14.03.0248.002; Wed, 17 Feb 2016 16:44:48 +0000 From: "Ananyev, Konstantin" To: "Lu, Wenzhuo" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH] ixgbe: fix x550 VF tx issue Thread-Index: AQHRWmmLiSk04bMIikCKCceNkHWosZ8wZvZw Date: Wed, 17 Feb 2016 16:44:48 +0000 Message-ID: <2601191342CEEE43887BDE71AB97725836B078DD@irsmsx105.ger.corp.intel.com> References: <1454053693-28673-1-git-send-email-wenzhuo.lu@intel.com> In-Reply-To: <1454053693-28673-1-git-send-email-wenzhuo.lu@intel.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZTczZTJiYzctMTIyMC00OGVmLWE0M2MtOTZhNjUyYWJmZTdkIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IkVqR21QQXlreFRBM2lEWWFJM29rc0twTERDVk44XC8xNHVRZlpBdU9XYXRjPSJ9 x-ctpclassification: CTP_IC x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] ixgbe: fix x550 VF tx issue 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: Wed, 17 Feb 2016 16:44:51 -0000 Hi Wenzhuo, > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu > Sent: Friday, January 29, 2016 7:48 AM > To: dev@dpdk.org > Subject: [dpdk-dev] [PATCH] ixgbe: fix x550 VF tx issue >=20 > On x550, the basic tx function may not work if we use DPDK VF based on > kernel PF. The reason is kernel PF enables a new feature "malicious drive= r > detection". According to this feature, driver should create TX context > descriptor and set the "Check Context" bit in advanced data descriptor. > This patch add the support of "malicious driver detection" to let kernel = PF > based DPDK VF work. Is there any way for VF to determine is MDD is enabled on PF or not? Mailbox message or something? If not, then I suppose for x550 VF we always have to set default txq_flags = in dev_info to PKT_TX_MALICIOUS_DRIVER_DETECT? Which means that for X550 VF simple TX path by default will be disabled, co= rrect? As alternative we probably can change tx_queue_start() for x550 VF to alway= s setup a dummy TCD and then make changes in TX simple path to always setup CC bit for x550 VF. That would allow to keep using TX simple for x550 VF , no matter is MDD is = enabled =20 by PF or not. Another thing - there could be a potential slowdown with that feature: As now TX has to occupy one extra HW context, even if no HW offload is requ= ested by SW, correct? Let say if app sends packets of 3 types: - TCP with TX HW csum offload - UDP with TX HW csum offload - packets with no HW offload requested =20 Then on 82599 VF no PMD would need to setup TCD only fist 2 times. But on X550 VF will need to setup new TCD for every packet. But I suppose that is unavoidable. >=20 > Although CC bit should be set in IOV mode, as tested, it's no harm to set > CC bit in non-IVO mode. So, as PF and VF share the same code, will set CC > bit anyway. I don't think it totally harmless. As I understand, if CC bit is set HW would try to apply related TC to the p= acket. If this TC was never setup - you don't know what values it will contain. >=20 > Please aware there's another way, disabling MDD in PF kernel driver. > Like this, > >insmod ixgbe.ko MDD=3D0,0 >=20 > Signed-off-by: Wenzhuo Lu > --- > doc/guides/rel_notes/release_2_3.rst | 12 ++++++++++++ > drivers/net/ixgbe/ixgbe_rxtx.c | 16 ++++++++++++++-- > drivers/net/ixgbe/ixgbe_rxtx.h | 3 ++- > lib/librte_ether/rte_ethdev.h | 1 + > lib/librte_mbuf/rte_mbuf.h | 3 ++- > 5 files changed, 31 insertions(+), 4 deletions(-) >=20 > diff --git a/doc/guides/rel_notes/release_2_3.rst b/doc/guides/rel_notes/= release_2_3.rst > index 99de186..90f81e0 100644 > --- a/doc/guides/rel_notes/release_2_3.rst > +++ b/doc/guides/rel_notes/release_2_3.rst > @@ -15,6 +15,18 @@ EAL > Drivers > ~~~~~~~ >=20 > +* **ixgbe: Fixed x550 VF tx issue.** > + > + On x550, the basic tx function may not work if we use DPDK VF based on > + kernel PF. The reason is kernel PF enables a new feature "malicious dr= iver > + detection". According to this feature, driver should create TX context > + descriptor and set the "Check Context" bit in advanced data descriptor= . > + This patch add the support of "malicious driver detection" to let kern= el PF > + based DPDK VF work. > + > + Please aware there's another way, disabling MDD in PF kernel driver. > + Like this, > + >insmod ixgbe.ko MDD=3D0,0 >=20 > Libraries > ~~~~~~~~~ > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxt= x.c > index 52a263c..c8a7740 100644 > --- a/drivers/net/ixgbe/ixgbe_rxtx.c > +++ b/drivers/net/ixgbe/ixgbe_rxtx.c > @@ -85,7 +85,8 @@ > PKT_TX_VLAN_PKT | \ > PKT_TX_IP_CKSUM | \ > PKT_TX_L4_MASK | \ > - PKT_TX_TCP_SEG) > + PKT_TX_TCP_SEG | \ > + PKT_TX_MALICIOUS_DRIVER_DETECT) >=20 > static inline struct rte_mbuf * > rte_rxmbuf_alloc(struct rte_mempool *mp) > @@ -564,6 +565,8 @@ ixgbe_xmit_cleanup(struct ixgbe_tx_queue *txq) > return (0); > } >=20 > +#define DEFAULT_CTX_L2_LEN 14 Use sizeof(strutct eth_hdr) or something. > + > uint16_t > ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, > uint16_t nb_pkts) > @@ -614,11 +617,19 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **t= x_pkts, > * are needed for offload functionality. > */ > ol_flags =3D tx_pkt->ol_flags; > + if (!(txq->txq_flags & ETH_TXQ_FLAGS_NOMDD)) > + ol_flags |=3D PKT_TX_MALICIOUS_DRIVER_DETECT; Probably a bit better introduce new filed inside txq: deaul_ol_flags or som= ething, Then set it up depending on HW type, and then just: ol_flags =3D tx_pkt->ol_flags | txq->default_ol_flags; >=20 > /* If hardware offload required */ > tx_ol_req =3D ol_flags & IXGBE_TX_OFFLOAD_MASK; > if (tx_ol_req) { > - tx_offload.l2_len =3D tx_pkt->l2_len; > + /* if l2 len isn't provided by the caller, give it a > + * default value. > + */ > + if (tx_pkt->l2_len =3D=3D 0) > + tx_offload.l2_len =3D DEFAULT_CTX_L2_LEN; > + else > + tx_offload.l2_len =3D tx_pkt->l2_len; So with l2_len=3D=3D0 what would happen? PF would stop the queue? > tx_offload.l3_len =3D tx_pkt->l3_len; > tx_offload.l4_len =3D tx_pkt->l4_len; > tx_offload.vlan_tci =3D tx_pkt->vlan_tci; > @@ -801,6 +812,7 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_= pkts, > } >=20 > olinfo_status |=3D (pkt_len << IXGBE_ADVTXD_PAYLEN_SHIFT); > + olinfo_status |=3D IXGBE_ADVTXD_CC; As I said, probably safer to set CC bit only if (tx_ol_req !=3D 0). =20 >=20 > m_seg =3D tx_pkt; > do { > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxt= x.h > index 475a800..bcde785 100644 > --- a/drivers/net/ixgbe/ixgbe_rxtx.h > +++ b/drivers/net/ixgbe/ixgbe_rxtx.h > @@ -255,7 +255,8 @@ struct ixgbe_txq_ops { > * RTE_PMD_IXGBE_TX_MAX_BURST. > */ > #define IXGBE_SIMPLE_FLAGS ((uint32_t)ETH_TXQ_FLAGS_NOMULTSEGS | \ > - ETH_TXQ_FLAGS_NOOFFLOADS) > + ETH_TXQ_FLAGS_NOOFFLOADS | \ > + ETH_TXQ_FLAGS_NOMDD) >=20 > /* > * Populate descriptors with the following info: > diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.= h > index bada8ad..a2e32d0 100644 > --- a/lib/librte_ether/rte_ethdev.h > +++ b/lib/librte_ether/rte_ethdev.h > @@ -637,6 +637,7 @@ struct rte_eth_rxconf { > #define ETH_TXQ_FLAGS_NOXSUMSCTP 0x0200 /**< disable SCTP checksum offlo= ad */ > #define ETH_TXQ_FLAGS_NOXSUMUDP 0x0400 /**< disable UDP checksum offloa= d */ > #define ETH_TXQ_FLAGS_NOXSUMTCP 0x0800 /**< disable TCP checksum offloa= d */ > +#define ETH_TXQ_FLAGS_NOMDD 0x1000 /**< disable malicious driver de= tection */ > #define ETH_TXQ_FLAGS_NOOFFLOADS \ > (ETH_TXQ_FLAGS_NOVLANOFFL | ETH_TXQ_FLAGS_NOXSUMSCTP | \ > ETH_TXQ_FLAGS_NOXSUMUDP | ETH_TXQ_FLAGS_NOXSUMTCP) > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h > index f234ac9..b5ed25c 100644 > --- a/lib/librte_mbuf/rte_mbuf.h > +++ b/lib/librte_mbuf/rte_mbuf.h > @@ -102,6 +102,7 @@ extern "C" { >=20 > /* add new TX flags here */ >=20 > +#define PKT_TX_MALICIOUS_DRIVER_DETECT (1ULL << 48) > /** > * Second VLAN insertion (QinQ) flag. > */ > @@ -824,7 +825,7 @@ struct rte_mbuf { > struct rte_mempool *pool; /**< Pool from which mbuf was allocated. */ > struct rte_mbuf *next; /**< Next segment of scattered packet. */ >=20 > - /* fields to support TX offloads */ > + /* fields to support malicious driver detection or TX offloads */ No fields are really updated, so probably no need to update comments here. > union { > uint64_t tx_offload; /**< combined for easy fetch */ > struct { > -- > 1.9.3