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 48C36A046B for ; Thu, 25 Jul 2019 09:40:28 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 1D3C11C29E; Thu, 25 Jul 2019 09:40:28 +0200 (CEST) Received: from mail-vs1-f66.google.com (mail-vs1-f66.google.com [209.85.217.66]) by dpdk.org (Postfix) with ESMTP id 006051C273 for ; Thu, 25 Jul 2019 09:40:25 +0200 (CEST) Received: by mail-vs1-f66.google.com with SMTP id u124so33120187vsu.2 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=hoK340JXN9MZBawf3iU021PYIoWB59IPjYYGgalk2JLX2fQn9PgPfXHAd69PLuwGzV 0UlrAwtkx1ZUBt+XDc2B4J+W9aO5KXHThcgDsRz1MnSlFLH64TgHPR8yxBnKMzBaSryj GD8o8kykFrN+LeHI8YII5ggBnsuzb4KH+Zuze4ALbaQHtxd1sZnIZn0sHY7Kwa5YaRKS ZJ7xQ3kHG8CXFz/YdHACFLGqe51ZNmqPwXDdLCRPXdPU/zrMQoKN7EVuvMakFOO/YuIz g8fGsddYfeQ2X5PphMgfGwo+D1FZz5hXNssg3MkqBl93qHHuzh8PeNSwFVGH9SVbsWYu ogvg== X-Gm-Message-State: APjAAAXBcI+rQlT4S1+qh7MkvOc9EVJ7uorbgkxOUZvll/FIe4/ys4DU wF+v/4uNqFq9sDYEvAAZTYLPTub3CJzGembEqzxA3g== 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-dev] [PATCH 2/3] net/pcap: fix transmit return count in error conditions 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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