| Message ID | 20250217-am65-cpsw-zc-prep-v1-4-ce450a62d64f@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 6d6c7933cea6e51a07be52cf5aba97cd656e0e54 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: ethernet: ti: am65-cpsw: drop multiple functions and code cleanup | expand |
On Mon, Feb 17, 2025 at 09:31:49AM +0200, Roger Quadros wrote: > This allows us to re-use am65_cpsw_run_xdp() for zero copy > case. Add AM65_CPSW_XDP_TX case for successful XDP_TX so we don't > free the page while in flight. > > Signed-off-by: Roger Quadros <rogerq@kernel.org> Reviewed-by: Simon Horman <horms@kernel.org> > --- > drivers/net/ethernet/ti/am65-cpsw-nuss.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c ... > @@ -1230,9 +1230,6 @@ static int am65_cpsw_run_xdp(struct am65_cpsw_rx_flow *flow, > ndev->stats.rx_dropped++; > } > > - page = virt_to_head_page(xdp->data); > - am65_cpsw_put_page(flow, page, true); > - > return ret; It seems that before and after this patch ret is always initialised to AM65_CPSW_XDP_CONSUMED and never changed. So it can be removed. Given that with this patch the function only returns after the switch statement, I think this would be a nice follow-up. diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c index 20a4fc3e579f..4052c9153632 100644 --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c @@ -1172,7 +1172,6 @@ static int am65_cpsw_run_xdp(struct am65_cpsw_rx_flow *flow, { struct am65_cpsw_common *common = flow->common; struct net_device *ndev = port->ndev; - int ret = AM65_CPSW_XDP_CONSUMED; struct am65_cpsw_tx_chn *tx_chn; struct netdev_queue *netif_txq; int cpu = smp_processor_id(); @@ -1228,9 +1227,8 @@ static int am65_cpsw_run_xdp(struct am65_cpsw_rx_flow *flow, fallthrough; case XDP_DROP: ndev->stats.rx_dropped++; + return AM65_CPSW_XDP_CONSUMED; } - - return ret; } /* RX psdata[2] word format - checksum information */ ...
On 18/02/2025 20:03, Simon Horman wrote: > On Mon, Feb 17, 2025 at 09:31:49AM +0200, Roger Quadros wrote: >> This allows us to re-use am65_cpsw_run_xdp() for zero copy >> case. Add AM65_CPSW_XDP_TX case for successful XDP_TX so we don't >> free the page while in flight. >> >> Signed-off-by: Roger Quadros <rogerq@kernel.org> > > Reviewed-by: Simon Horman <horms@kernel.org> > >> --- >> drivers/net/ethernet/ti/am65-cpsw-nuss.c | 13 ++++++++----- >> 1 file changed, 8 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c > > ... > >> @@ -1230,9 +1230,6 @@ static int am65_cpsw_run_xdp(struct am65_cpsw_rx_flow *flow, >> ndev->stats.rx_dropped++; >> } >> >> - page = virt_to_head_page(xdp->data); >> - am65_cpsw_put_page(flow, page, true); >> - >> return ret; > > It seems that before and after this patch ret is always initialised to > AM65_CPSW_XDP_CONSUMED and never changed. So it can be removed. > > Given that with this patch the function only returns after the switch > statement, I think this would be a nice follow-up. > > diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c > index 20a4fc3e579f..4052c9153632 100644 > --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c > +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c > @@ -1172,7 +1172,6 @@ static int am65_cpsw_run_xdp(struct am65_cpsw_rx_flow *flow, > { > struct am65_cpsw_common *common = flow->common; > struct net_device *ndev = port->ndev; > - int ret = AM65_CPSW_XDP_CONSUMED; > struct am65_cpsw_tx_chn *tx_chn; > struct netdev_queue *netif_txq; > int cpu = smp_processor_id(); > @@ -1228,9 +1227,8 @@ static int am65_cpsw_run_xdp(struct am65_cpsw_rx_flow *flow, > fallthrough; > case XDP_DROP: > ndev->stats.rx_dropped++; > + return AM65_CPSW_XDP_CONSUMED; > } > - > - return ret; > } > > /* RX psdata[2] word format - checksum information */ > > ... Thank you for this suggestion. I will add this cleanup in my next set of patches.
diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c index 468fddd0afd2..32349cc58e2e 100644 --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c @@ -164,6 +164,7 @@ #define AM65_CPSW_CPPI_TX_PKT_TYPE 0x7 /* XDP */ +#define AM65_CPSW_XDP_TX BIT(2) #define AM65_CPSW_XDP_CONSUMED BIT(1) #define AM65_CPSW_XDP_REDIRECT BIT(0) #define AM65_CPSW_XDP_PASS 0 @@ -1177,7 +1178,6 @@ static int am65_cpsw_run_xdp(struct am65_cpsw_rx_flow *flow, int cpu = smp_processor_id(); struct xdp_frame *xdpf; struct bpf_prog *prog; - struct page *page; int pkt_len; u32 act; int err; @@ -1212,7 +1212,7 @@ static int am65_cpsw_run_xdp(struct am65_cpsw_rx_flow *flow, goto drop; dev_sw_netstats_rx_add(ndev, pkt_len); - return AM65_CPSW_XDP_CONSUMED; + return AM65_CPSW_XDP_TX; case XDP_REDIRECT: if (unlikely(xdp_do_redirect(ndev, xdp, prog))) goto drop; @@ -1230,9 +1230,6 @@ static int am65_cpsw_run_xdp(struct am65_cpsw_rx_flow *flow, ndev->stats.rx_dropped++; } - page = virt_to_head_page(xdp->data); - am65_cpsw_put_page(flow, page, true); - return ret; } @@ -1331,6 +1328,12 @@ static int am65_cpsw_nuss_rx_packets(struct am65_cpsw_rx_flow *flow, xdp_prepare_buff(&xdp, page_addr, AM65_CPSW_HEADROOM, pkt_len, false); *xdp_state = am65_cpsw_run_xdp(flow, port, &xdp, &pkt_len); + if (*xdp_state == AM65_CPSW_XDP_CONSUMED) { + page = virt_to_head_page(xdp.data); + am65_cpsw_put_page(flow, page, true); + goto allocate; + } + if (*xdp_state != AM65_CPSW_XDP_PASS) goto allocate;
This allows us to re-use am65_cpsw_run_xdp() for zero copy case. Add AM65_CPSW_XDP_TX case for successful XDP_TX so we don't free the page while in flight. Signed-off-by: Roger Quadros <rogerq@kernel.org> --- drivers/net/ethernet/ti/am65-cpsw-nuss.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)