net/af_xdp: free mbuf when allocate Tx queue fails
Checks
Commit Message
When we fail to allocate enough slots in tx queue for transmitting
packets, we need to free the corresponding mbufs.
Fixes: f1debd77efaf ("net/af_xdp: introduce AF_XDP PMD")
Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
---
drivers/net/af_xdp/rte_eth_af_xdp.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
Comments
On Tue, Apr 9, 2019 at 10:27 AM Xiaolong Ye <xiaolong.ye@intel.com> wrote:
> When we fail to allocate enough slots in tx queue for transmitting
> packets, we need to free the corresponding mbufs.
>
You'd better let the application retry on its own...
> Fixes: f1debd77efaf ("net/af_xdp: introduce AF_XDP PMD")
>
> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
> ---
> drivers/net/af_xdp/rte_eth_af_xdp.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c
> b/drivers/net/af_xdp/rte_eth_af_xdp.c
> index 007a1c6b4..bc7973b56 100644
> --- a/drivers/net/af_xdp/rte_eth_af_xdp.c
> +++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
> @@ -276,7 +276,8 @@ eth_af_xdp_tx(void *queue, struct rte_mbuf **bufs,
> uint16_t nb_pkts)
>
> if (xsk_ring_prod__reserve(&txq->tx, nb_pkts, &idx_tx) != nb_pkts)
> {
> kick_tx(txq);
> - return 0;
> + nb_pkts = 0;
> + goto out;
>
...and free back the buffers you got from the umem ring here.
}
>
> for (i = 0; i < nb_pkts; i++) {
>
Hi, David
On 04/09, David Marchand wrote:
>On Tue, Apr 9, 2019 at 10:27 AM Xiaolong Ye <xiaolong.ye@intel.com> wrote:
>
>> When we fail to allocate enough slots in tx queue for transmitting
>> packets, we need to free the corresponding mbufs.
>>
>
>You'd better let the application retry on its own...
Make sense.
>
>
>> Fixes: f1debd77efaf ("net/af_xdp: introduce AF_XDP PMD")
>>
>> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
>> ---
>> drivers/net/af_xdp/rte_eth_af_xdp.c | 8 ++++++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c
>> b/drivers/net/af_xdp/rte_eth_af_xdp.c
>> index 007a1c6b4..bc7973b56 100644
>> --- a/drivers/net/af_xdp/rte_eth_af_xdp.c
>> +++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
>> @@ -276,7 +276,8 @@ eth_af_xdp_tx(void *queue, struct rte_mbuf **bufs,
>> uint16_t nb_pkts)
>>
>> if (xsk_ring_prod__reserve(&txq->tx, nb_pkts, &idx_tx) != nb_pkts)
>> {
>> kick_tx(txq);
>> - return 0;
>> + nb_pkts = 0;
>> + goto out;
>>
>
>...and free back the buffers you got from the umem ring here.
Agree, will send a new version.
Thanks,
Xiaolong
>
> }
>>
>> for (i = 0; i < nb_pkts; i++) {
>>
>
>
>--
>David Marchand
@@ -276,7 +276,8 @@ eth_af_xdp_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
if (xsk_ring_prod__reserve(&txq->tx, nb_pkts, &idx_tx) != nb_pkts) {
kick_tx(txq);
- return 0;
+ nb_pkts = 0;
+ goto out;
}
for (i = 0; i < nb_pkts; i++) {
@@ -296,7 +297,6 @@ eth_af_xdp_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
valid++;
tx_bytes += mbuf->pkt_len;
}
- rte_pktmbuf_free(mbuf);
}
xsk_ring_prod__submit(&txq->tx, nb_pkts);
@@ -311,6 +311,10 @@ eth_af_xdp_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
txq->stats.tx_pkts += valid;
txq->stats.tx_bytes += tx_bytes;
+ out:
+ for (i = 0; i < nb_pkts; i++)
+ rte_pktmbuf_free(bufs[i]);
+
return nb_pkts;
}