net/af_xdp: enqueue buf ring when allocate Tx queue fails
Checks
Commit Message
When it fails to allocate enough slots in Tx queue for transmitting
packets, we need to return the dequeued addrs to buf ring.
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 | 1 +
1 file changed, 1 insertion(+)
Comments
On Tue, Apr 9, 2019 at 5:24 PM Xiaolong Ye <xiaolong.ye@intel.com> wrote:
> When it fails to allocate enough slots in Tx queue for transmitting
> packets, we need to return the dequeued addrs to buf ring.
>
> 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 | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c
> b/drivers/net/af_xdp/rte_eth_af_xdp.c
> index 007a1c6b4..5cc643ce2 100644
> --- a/drivers/net/af_xdp/rte_eth_af_xdp.c
> +++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
> @@ -276,6 +276,7 @@ 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);
> + rte_ring_enqueue_bulk(umem->buf_ring, addrs, nb_pkts,
> NULL);
> return 0;
> }
>
> --
> 2.17.1
>
Looks good to me.
But I have an additional question.
After the for loop that only picks the mbufs it can copy to the xdp desc,
is it normal to call xsk_ring_prod__submit(&txq->tx, nb_pkts); rather than
with valid count ?
On 04/10, David Marchand wrote:
>On Tue, Apr 9, 2019 at 5:24 PM Xiaolong Ye <xiaolong.ye@intel.com> wrote:
>
>> When it fails to allocate enough slots in Tx queue for transmitting
>> packets, we need to return the dequeued addrs to buf ring.
>>
>> 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 | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c
>> b/drivers/net/af_xdp/rte_eth_af_xdp.c
>> index 007a1c6b4..5cc643ce2 100644
>> --- a/drivers/net/af_xdp/rte_eth_af_xdp.c
>> +++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
>> @@ -276,6 +276,7 @@ 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);
>> + rte_ring_enqueue_bulk(umem->buf_ring, addrs, nb_pkts,
>> NULL);
>> return 0;
>> }
>>
>> --
>> 2.17.1
>>
>
>Looks good to me.
>But I have an additional question.
>
>After the for loop that only picks the mbufs it can copy to the xdp desc,
>is it normal to call xsk_ring_prod__submit(&txq->tx, nb_pkts); rather than
>with valid count ?
Good catch, I think it needs to submit valid count other than nb_pkts, and return back
(nb_pkts - count) addrs to buf ring.
I'll send a fix patch for it.
Thanks,
Xiaolong
>
>--
>David Marchand
@@ -276,6 +276,7 @@ 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);
+ rte_ring_enqueue_bulk(umem->buf_ring, addrs, nb_pkts, NULL);
return 0;
}