Lorenzo Bianconi <lorenzo@xxxxxxxxxx> writes:
...
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c
b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 102f2c91fdb8..7ad0557dedbd 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
...
@@ -339,8 +337,8 @@ static int ena_xdp_xmit(struct net_device
*dev, int n,
struct xdp_frame **frames, u32 flags)
{
struct ena_adapter *adapter = netdev_priv(dev);
- int qid, i, err, drops = 0;
struct ena_ring *xdp_ring;
+ int qid, i, nxmit = 0;
if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK))
return -EINVAL;
@@ -360,12 +358,12 @@ static int ena_xdp_xmit(struct net_device
*dev, int n,
spin_lock(&xdp_ring->xdp_tx_lock);
for (i = 0; i < n; i++) {
- err = ena_xdp_xmit_frame(xdp_ring, dev, frames[i],
0);
/* The descriptor is freed by ena_xdp_xmit_frame
in case
* of an error.
*/
Thanks a lot for the patch. It's a good idea. Do you mind removing
the comment here as well ? ena_xdp_xmit_frame() no longer frees
the frame in case of an error after this patch.
- if (err)
- drops++;
+ if (ena_xdp_xmit_frame(xdp_ring, dev, frames[i],
0))
+ break;
+ nxmit++;
}
/* Ring doorbell to make device aware of the packets */
@@ -378,7 +376,7 @@ static int ena_xdp_xmit(struct net_device
*dev, int n,
spin_unlock(&xdp_ring->xdp_tx_lock);
/* Return number of packets sent */
- return n - drops;
+ return nxmit;
}
...
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index 85d9d1b72a33..9f158b3862df 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -344,29 +344,26 @@ static void bq_xmit_all(struct
xdp_dev_bulk_queue *bq, u32 flags)
sent = dev->netdev_ops->ndo_xdp_xmit(dev, bq->count,
bq->q, flags);
if (sent < 0) {
+ /* If ndo_xdp_xmit fails with an errno, no frames
have
+ * been xmit'ed.
+ */
err = sent;
sent = 0;
- goto error;
}
+
drops = bq->count - sent;
-out:
- bq->count = 0;
+ if (unlikely(drops > 0)) {
+ /* If not all frames have been transmitted, it is
our
+ * responsibility to free them
+ */
+ for (i = sent; i < bq->count; i++)
+ xdp_return_frame_rx_napi(bq->q[i]);
+ }
Wouldn't the logic above be the same even w/o the 'if' condition ?
+ bq->count = 0;
trace_xdp_devmap_xmit(bq->dev_rx, dev, sent, drops, err);
bq->dev_rx = NULL;
__list_del_clearprev(&bq->flush_node);
- return;
-error:
- /* If ndo_xdp_xmit fails with an errno, no frames have
been
- * xmit'ed and it's our responsibility to them free all.
- */
- for (i = 0; i < bq->count; i++) {
- struct xdp_frame *xdpf = bq->q[i];
-
- xdp_return_frame_rx_napi(xdpf);
- drops++;
- }
- goto out;
}
/* __dev_flush is called from xdp_do_flush() which _must_ be
signaled
Thanks, Shay
