From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 61B16A046B for ; Thu, 25 Jul 2019 09:40:30 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 44CE81C2A4; Thu, 25 Jul 2019 09:40:30 +0200 (CEST) Received: from mail-vs1-f68.google.com (mail-vs1-f68.google.com [209.85.217.68]) by dpdk.org (Postfix) with ESMTP id 0873C1C29E for ; Thu, 25 Jul 2019 09:40:26 +0200 (CEST) Received: by mail-vs1-f68.google.com with SMTP id m23so33127162vso.1 for ; Thu, 25 Jul 2019 00:40:25 -0700 (PDT) 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=K29BaQeRHMgGte+2CDMUweWl/asrlIUXuiBqqsdamrs=; b=I46EQuduFeGl0eGbjGEc9rpnMm1jI0ECswNtH5viIh3zI/kJhq2Lcx+2d1F5hq5mlY esKMYlHd1G8QCIWyoNbRocmBhIfabi6PSpUwAEWectWBBxj7m40xtTW/qICE18hqSZIK Uu/7+pwOWBwkuaBXcRbco7LKM6zZZCbjV8D/CdvdBm6nt3cZXjEsE/Zmx5eKmD5A8md1 fIXrfWEimp3g+oEznkFoVsqUVuBmnhwcl8EFtNYLDIbgT1si94Y1Bje0rqaexO0mFI2o 2088KLjtULNF+HV6oY2DgwXl9cu2smAUSeJCOVzMIHqcTFsgXIr/UCWWWE3nFjR5Uouc LnzA== X-Gm-Message-State: APjAAAXzlfTlbyi8Pudy4k5v8trBJSzlE2Po9Jity4qQaJBLbk+I71YK d9XTvNQXWb1k80Vr85FaDtKHXKQfUSLRS8hmF8XAlQ== X-Google-Smtp-Source: APXvYqzhZhuWVraOC8O/98dA2FnUX3PW69HMXrP3wJrNRBYxLopVRZJ9of4DmC3Go4tVqS2f/ep65Q/Cbzzdw2dTGv0= X-Received: by 2002:a67:e9ca:: with SMTP id q10mr23478825vso.105.1564040425423; Thu, 25 Jul 2019 00:40:25 -0700 (PDT) MIME-Version: 1.0 References: <1563969270-29669-1-git-send-email-david.marchand@redhat.com> <1563969270-29669-3-git-send-email-david.marchand@redhat.com> In-Reply-To: From: David Marchand Date: Thu, 25 Jul 2019 09:40:14 +0200 Message-ID: To: Ferruh Yigit Cc: dev , dpdk stable , "A.McLoughlin" Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-stable] [PATCH 2/3] net/pcap: fix transmit return count in error conditions X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" On Wed, Jul 24, 2019 at 8:36 PM Ferruh Yigit wrote: > > On 7/24/2019 12:54 PM, David Marchand wrote: > > When a packet cannot be transmitted, the driver is supposed to free this > > packet and report it as handled. > > This is to prevent the application from retrying to send the same packet > > and ending up in a liveloop since the driver will never manage to send > > it. > > > > Fixes: 49a0a2ffd5db ("net/pcap: fix possible mbuf double freeing") > > Fixes: 6db141c91e1f ("pcap: support jumbo frames") > > CC: stable@dpdk.org > > > > Signed-off-by: David Marchand > > --- > > drivers/net/pcap/rte_eth_pcap.c | 18 ++++++++++-------- > > 1 file changed, 10 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c > > index 470867d..5e5aab7 100644 > > --- a/drivers/net/pcap/rte_eth_pcap.c > > +++ b/drivers/net/pcap/rte_eth_pcap.c > > @@ -354,7 +354,8 @@ struct pmd_devargs_all { > > mbuf->pkt_len, > > RTE_ETHER_MAX_JUMBO_FRAME_LEN); > > > > - break; > > + rte_pktmbuf_free(mbuf); > > + continue; > > +1 > Very recently 'rte_pktmbuf_free()' was moved because it wasn't compatible with > return value, but this looks better, to free the mbuf and record it as error. > > > } > > } > > > > @@ -373,7 +374,7 @@ struct pmd_devargs_all { > > dumper_q->tx_stat.bytes += tx_bytes; > > dumper_q->tx_stat.err_pkts += nb_pkts - num_tx; > > > > - return num_tx; > > + return nb_pkts; > > } > > > > /* > > @@ -439,14 +440,15 @@ struct pmd_devargs_all { > > mbuf->pkt_len, > > RTE_ETHER_MAX_JUMBO_FRAME_LEN); > > > > - break; > > + rte_pktmbuf_free(mbuf); > > + continue; > > } > > } > > > > - if (unlikely(ret != 0)) > > - break; > > - num_tx++; > > - tx_bytes += mbuf->pkt_len; > > + if (ret == 0) { > > + num_tx++; > > + tx_bytes += mbuf->pkt_len; > > + } > > I don't know this part, this is in 'eth_pcap_tx()' which writes packets to the > interfaces. > > if 'pcap_sendpacket()' fails this doesn't mean packet can't be sent and may > cause a liveloop. Why not keep the existing behavior and let application to decide? The manual is not clear to me. Do we really have temporary situations where retries are fine? and if so, can we differentiate them from things like incorrect permissions etc... If we can't, dropping is safer. -- David Marchand