Hi,
Thanks for your patch.
It was submitted and accepted during the weekend, in our off-hours (we work Sunday - Thursday).
Please check my comments below.
On 9/23/2021 7:10 PM, Joshua Roys wrote:
Empty commit message? No feature description, motivation, performance numbers, etc... That's bad.
Signed-off-by: Joshua Roys <redacted>
---
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
This is a pattern-match commit, based off of the mlx4 XDP_TX and other
drivers' XDP_REDIRECT enablement patches. The goal was to get AF_XDP
working in VPP and this was successful. Tested with a CX3.
Your comment here doesn't get into the git commit log. It can't replace the patch description above.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index 7f6d3b82c29b..557d7daac2d3 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -669,6 +669,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
struct bpf_prog *xdp_prog;
int cq_ring = cq->ring;
bool doorbell_pending;
+ bool xdp_redir_flush;
struct mlx4_cqe *cqe;
struct xdp_buff xdp;
int polled = 0;
@@ -682,6 +683,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
xdp_prog = rcu_dereference_bh(ring->xdp_prog);
xdp_init_buff(&xdp, priv->frag_info[0].frag_stride, &ring->xdp_rxq);
doorbell_pending = false;
+ xdp_redir_flush = false;
/* We assume a 1:1 mapping between CQEs and Rx descriptors, so Rx
* descriptor offset can be deduced from the CQE index instead of
@@ -790,6 +792,14 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
switch (act) {
case XDP_PASS:
break;
+ case XDP_REDIRECT:
+ if (xdp_do_redirect(dev, &xdp, xdp_prog) >= 0) {
xdp_do_redirect returns a negative error code, or zero on success.
The >= 0 comparison looks strange and doesn't fit to the code style.
Simply use: "if (xdp_do_redirect(...))"
quoted hunk ↗ jump to hunk
+ xdp_redir_flush = true;
+ frags[0].page = NULL;
+ goto next;
+ }
+ trace_xdp_exception(dev, xdp_prog, act);
+ goto xdp_drop_no_cnt;
You didn't add the required stats to count packets going through the new xdp_redirect flow. Every incoming packet *MUST* increase some counter. Please add it.
quoted hunk ↗ jump to hunk
case XDP_TX:
if (likely(!mlx4_en_xmit_frame(ring, frags, priv,
length, cq_ring,
@@ -897,6 +907,9 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
break;
}
+ if (xdp_redir_flush)
+ xdp_do_flush();
+
if (likely(polled)) {
if (doorbell_pending) {
priv->tx_cq[TX_XDP][cq_ring]->xdp_busy = true;
Thanks,
Tariq