On 8/21/2023 11:52 PM, Wei Fang wrote:
> As we already added the exception tracing for XDP_TX, I think it is
> necessary to add the exception tracing for other XDP actions, such
> as XDP_REDIRECT, XDP_ABORTED and unknown error actions.
>
> Signed-off-by: Wei Fang <wei.fang@xxxxxxx>
> Suggested-by: Jakub Kicinski <kuba@xxxxxxxxxx>
> ---
Makes sense to me, and it ends up being a bit less code.
Reviewed-by: Jacob Keller <jacob.e.keller@xxxxxxxxx>
> drivers/net/ethernet/freescale/fec_main.c | 26 ++++++++++-------------
> 1 file changed, 11 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index e23a55977183..8909899e9a31 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -1583,25 +1583,18 @@ fec_enet_run_xdp(struct fec_enet_private *fep, struct bpf_prog *prog,
> case XDP_REDIRECT:
> rxq->stats[RX_XDP_REDIRECT]++;
> err = xdp_do_redirect(fep->netdev, xdp, prog);
> - if (!err) {
> - ret = FEC_ENET_XDP_REDIR;
> - } else {
> - ret = FEC_ENET_XDP_CONSUMED;
> - page = virt_to_head_page(xdp->data);
> - page_pool_put_page(rxq->page_pool, page, sync, true);
> - }
> + if (unlikely(err))
> + goto xdp_err;
> +
> + ret = FEC_ENET_XDP_REDIR;
> break;
>
> case XDP_TX:
> err = fec_enet_xdp_tx_xmit(fep, cpu, xdp, sync);
> - if (unlikely(err)) {
> - ret = FEC_ENET_XDP_CONSUMED;
> - page = virt_to_head_page(xdp->data);
> - page_pool_put_page(rxq->page_pool, page, sync, true);
> - trace_xdp_exception(fep->netdev, prog, act);
> - } else {
> - ret = FEC_ENET_XDP_TX;
> - }
> + if (unlikely(err))
> + goto xdp_err;
> +
> + ret = FEC_ENET_XDP_TX;
> break;
>
> default:
> @@ -1613,9 +1606,12 @@ fec_enet_run_xdp(struct fec_enet_private *fep, struct bpf_prog *prog,
>
> case XDP_DROP:
> rxq->stats[RX_XDP_DROP]++;
> +xdp_err:
> ret = FEC_ENET_XDP_CONSUMED;
> page = virt_to_head_page(xdp->data);
> page_pool_put_page(rxq->page_pool, page, sync, true);
Ok, so we handle the cleaning up of the page and such here, which is
shared for both paths now. Nice!
> + if (act != XDP_DROP)
> + trace_xdp_exception(fep->netdev, prog, act);
> break;
> }
>
