From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ua1-f47.google.com (mail-ua1-f47.google.com [209.85.222.47]) by dpdk.org (Postfix) with ESMTP id B58E8239 for ; Wed, 20 Feb 2019 09:33:58 +0100 (CET) Received: by mail-ua1-f47.google.com with SMTP id d21so7923252uap.9 for ; Wed, 20 Feb 2019 00:33:58 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=jJAhKj+Z+D+mcQN5em6igFVmAJlgU2Tz3IuJ/mOFqEE=; b=ebkUw8c6/4SOqoxnGYZOd8Hs+AYIKvMhS5dygxN/d6XZAcsfPEQnUtqGnx9Ae5NVLy uxOB7r6gwSaSOo4moQ6a+utgLnwd3/lw3WneVf6/bKZ2YQ2rjivLad1PQx4lZTz+F9Z1 Sc/nJ2LwxK7pMOmbURjm2ldXI6fl1sh2pwJPU+51PaAixammVERBvHcPqZkfz10Y1auW GBtiE+h0r4pvAVcprg8cfDgye0tSZ3DmWdY+7JuNFXWr17OpY2QrBAecS9QuNT0BKH6c 5dIDKLMws4snimT+xJZQ1STpzjlHqmoZpjlgCU/Rewe3aRusfqyaWGzKGDLJkf/zjwuC Zp9w== X-Gm-Message-State: AHQUAuY2oRITvLbrS9Z58TMSNMih5Xzd1vttrQhQT+6MCEEwZFG9KzkD ZrRDjv4APmNWGVIEST/zS+xUwYfnW9oeuIq9BUKrww== X-Google-Smtp-Source: AHgI3IYo1AvUSaNN0V2o9+T3QgptXATmE508WHVAtgepOt5PA4tlqdy8OGk3f4bD+HowCMSoKTjc6QnPPHS0wunxZzg= X-Received: by 2002:ab0:3484:: with SMTP id c4mr1159633uar.39.1550651637811; Wed, 20 Feb 2019 00:33:57 -0800 (PST) MIME-Version: 1.0 References: <1550158972-21895-1-git-send-email-david.marchand@redhat.com> <2601191342CEEE43887BDE71AB977258012413A4D8@irsmsx105.ger.corp.intel.com> <6177115.1iB7W0vHcD@xps> <2601191342CEEE43887BDE71AB977258012413A7C2@irsmsx105.ger.corp.intel.com> In-Reply-To: <2601191342CEEE43887BDE71AB977258012413A7C2@irsmsx105.ger.corp.intel.com> From: David Marchand Date: Wed, 20 Feb 2019 09:33:45 +0100 Message-ID: To: "Ananyev, Konstantin" Cc: Thomas Monjalon , "Richardson, Bruce" , "dev@dpdk.org" , "Lu, Wenzhuo" , "Wu, Jingjing" , "Iremonger, Bernard" , Maxime Coquelin , "Yigit, Ferruh" , Andrew Rybchenko , "Wiles, Keith" Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH 3/5] app/testpmd: add missing transmit errors stats 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: Wed, 20 Feb 2019 08:33:59 -0000 Hello Konstantin, On Sat, Feb 16, 2019 at 1:50 PM Ananyev, Konstantin < konstantin.ananyev@intel.com> wrote: > > > > -----Original Message----- > > From: Thomas Monjalon [mailto:thomas@monjalon.net] > > Sent: Friday, February 15, 2019 7:39 PM > > To: Ananyev, Konstantin > > Cc: David Marchand ; Richardson, Bruce < > bruce.richardson@intel.com>; dev@dpdk.org; Lu, Wenzhuo > > ; Wu, Jingjing ; > Iremonger, Bernard ; Maxime Coquelin > > ; Yigit, Ferruh ; Andrew > Rybchenko ; Wiles, Keith > > > > Subject: Re: [dpdk-dev] [PATCH 3/5] app/testpmd: add missing transmit > errors stats > > > > 15/02/2019 19:42, Ananyev, Konstantin: > > > >>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of David > Marchand > > > >>> I am also for option 2 especially because of this. > > > >>> A driver that refuses a packet for reason X (which is a > limitation, or an > > > >>> incorrect config or whatever that is not a transient condition) > but gives > > > >>> it back to the application is a bad driver. > > > > > > >>Why? What.s wrong to leave it to the upper layer to decide what to > > > >>do with the packets that can't be sent (by one reason or another)? > > > > > > >How does the upper layer know if this is a transient state or > something that can't be resolved? > > > > > > Via rte_errno, for example. > > > > rte_errno is not a result per packet. > > Surely it is not. > But tx_burst() can return after first failure. > > > I think it is better to "eat" the packet > > as it is done for those transmitted to the HW. > > Probably extra clarification is needed here. > Right now tx_burst (at least for PMDs I am aware about) doesn't > do any checking that: > - packet is correct and can be handled > (this is responsibility of tx_prepare) > - HW/PMD SW state is in valid and properly configured > (link is up, queue is configured, HW initialized properly). > > All that really tx_burst() care about -there is enough free TX > descriptors to fill. When that happens - tx_burst() returns > straightway. > > So what particular error conditions are you talking about, > and when you think we have to 'eat' the packets? > - This is how Intel drivers are written yes. But some drivers try to do more and have (useless ?) checks on mbufs or internal states. Found the following ones last week. There are more but it takes time to investigate. https://git.dpdk.org/dpdk/tree/drivers/net/atlantic/atl_rxtx.c?id=b13baac8d5ffb6b0b7a6ca0def884d3f1a82babb#n1346 https://git.dpdk.org/dpdk/tree/drivers/net/fm10k/fm10k_rxtx.c?id=b13baac8d5ffb6b0b7a6ca0def884d3f1a82babb#n646 https://git.dpdk.org/dpdk/tree/drivers/net/cxgbe/sge.c?id=b13baac8d5ffb6b0b7a6ca0def884d3f1a82babb#n1123 I would say first, we can have a cleanup to get rid of the unneeded checks, and see what the different maintainers think about this. Then look again at the situation. - I will drop this patch on testpmd which was wrong. But I intend to send an update on the doc to describe oerrors as solution 2: 2/ API break = oerrors stop being incremented for temporary unavailability (i.e. queue full, kind of ERETRY), report only packets which will be never sent, may be a small performance gain for some drivers There is some cleanup to do as well in quite a few drivers. -- David Marchand