Re: [PATCH 1/1] net: fec: using page pool to manage RX buffers

Andrew Lunn
2022-09-30T20:16:21Z

> +struct fec_enet_xdp_stats {
> + u64 xdp_pass;
> + u64 xdp_drop;
> + u64 xdp_xmit;
> + u64 xdp_redirect;
> + u64 xdp_xmit_err;
> + u64 xdp_tx;
> + u64 xdp_tx_err;
> +};

These are not used in this patch. Please don't add anything until it
is actually used. The danger is, when you add the actual increments,
we miss that they are u64 and so need protecting. If they are in the
same patch, it is much more obvious.

> +
> struct fec_enet_priv_tx_q {
> struct bufdesc_prop bd;
> unsigned char *tx_bounce[TX_RING_SIZE];
> @@ -532,7 +552,15 @@ struct fec_enet_priv_tx_q {
>
> struct fec_enet_priv_rx_q {
> struct bufdesc_prop bd;
> - struct sk_buff *rx_skbuff[RX_RING_SIZE];
> + struct fec_enet_priv_txrx_info rx_skb_info[RX_RING_SIZE];
> +
> + /* page_pool */
> + struct page_pool *page_pool;
> + struct xdp_rxq_info xdp_rxq;
> + struct fec_enet_xdp_stats stats;
> +
> + /* rx queue number, in the range 0-7 */
> + u8 id;
> };
>
> struct fec_stop_mode_gpr {
> @@ -644,6 +672,9 @@ struct fec_enet_private {
>
> struct imx_sc_ipc *ipc_handle;
>
> + /* XDP BPF Program */
> + struct bpf_prog *xdp_prog;

Not in this patch.

> fec_enet_alloc_rxq_buffers(struct net_device *ndev, unsigned int queue)
> {
> struct fec_enet_private *fep = netdev_priv(ndev);
> - unsigned int i;
> - struct sk_buff *skb;
> + int i, err;
> struct bufdesc *bdp;
> struct fec_enet_priv_rx_q *rxq;
>
> + dma_addr_t phys_addr;
> + struct page *page;

Reverse Christmas tree is messed up in this driver, but please don't
make it worse. int i, err; should probably be last, and don't add a
blank line.

Andrew