Re: [dpdk-dev] [dpdk-stable] [PATCH] net/pcap: fix issue with unnecessary mbufs freeing

[David Marchand <david.marchand@redhat.com>]

On Thu, Jul 11, 2019 at 4:00 PM A.McLoughlin
<aideen.mcloughlin@intel.com> wrote:
>
> In the eth_pcap_tx() and eth_pcap_tx_dumper() functions mbufs were freed
> without incrementing num_tx. To fix the issue, the mbuf freeing was
> removed as it was not of any benefit.
>
> Fixes: 6db141c91e1f ("pcap: support jumbo frames")
> Cc: stable@dpdk.org
> Cc: tero.aho@coriant.com
>
> Signed-off-by: A.McLoughlin <aideen.mcloughlin@intel.com>
> ---
>  drivers/net/pcap/rte_eth_pcap.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
> index 26e85183e..77bb66337 100644
> --- a/drivers/net/pcap/rte_eth_pcap.c
> +++ b/drivers/net/pcap/rte_eth_pcap.c
> @@ -349,7 +349,6 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
>                                         mbuf->pkt_len,
>                                         RTE_ETHER_MAX_JUMBO_FRAME_LEN);
>
> -                               rte_pktmbuf_free(mbuf);
>                                 break;
>                         }
>                 }
> @@ -435,7 +434,6 @@ eth_pcap_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
>                                         mbuf->pkt_len,
>                                         RTE_ETHER_MAX_JUMBO_FRAME_LEN);
>
> -                               rte_pktmbuf_free(mbuf);
>                                 break;
>                         }
>                 }
> --
> 2.17.1
>

If a driver cannot xmit a packet (it is not a temporary situation but
it is just that it can't), then it must free it and report it as
handled because the application can do nothing more.
Imagine an application that retries to send the packet right away, it
ends up in a liveloop.

The freeing of the packet was correct, what needs to be fixed is the
return value.
I am preparing fixes (found another issue in this part of the driver).
I would be for reverting this patch, so that the fix is more
straightforward, but we can discuss this once I sent my patches.


> --------------------------------------------------------------
> Intel Research and Development Ireland Limited
> Registered in Ireland
> Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
> Registered Number: 308263
>
>
> This e-mail and any attachments may contain confidential material for the sole
> use of the intended recipient(s). Any review or distribution by others is
> strictly prohibited. If you are not the intended recipient, please contact the
> sender and delete all copies.
>


Please contact your IT and do something about this footer.



--
David Marchand