> -----Original Message-----
> From: Simon Horman <simon.horman@xxxxxxxxxxxx>
> Sent: Saturday, 15 July 2023 11:23
> To: Sriram Yagnaraman <sriram.yagnaraman@xxxxxxxx>
> Cc: intel-wired-lan@xxxxxxxxxxxxxxxx; bpf@xxxxxxxxxxxxxxx;
> netdev@xxxxxxxxxxxxxxx; Jesse Brandeburg <jesse.brandeburg@xxxxxxxxx>;
> Tony Nguyen <anthony.l.nguyen@xxxxxxxxx>; David S . Miller
> <davem@xxxxxxxxxxxxx>; Eric Dumazet <edumazet@xxxxxxxxxx>; Jakub
> Kicinski <kuba@xxxxxxxxxx>; Paolo Abeni <pabeni@xxxxxxxxxx>; Alexei
> Starovoitov <ast@xxxxxxxxxx>; Daniel Borkmann <daniel@xxxxxxxxxxxxx>;
> Jesper Dangaard Brouer <hawk@xxxxxxxxxx>; John Fastabend
> <john.fastabend@xxxxxxxxx>; Björn Töpel <bjorn@xxxxxxxxxx>; Magnus
> Karlsson <magnus.karlsson@xxxxxxxxx>; Maciej Fijalkowski
> <maciej.fijalkowski@xxxxxxxxx>; Jonathan Lemon
> <jonathan.lemon@xxxxxxxxx>
> Subject: Re: [PATCH iwl-next v3 3/4] igb: add AF_XDP zero-copy Rx support
>
> On Thu, Jul 13, 2023 at 12:47:16PM +0200, Sriram Yagnaraman wrote:
> > Add support for AF_XDP zero-copy receive path.
> >
> > When AF_XDP zero-copy is enabled, the rx buffers are allocated from
> > the xsk buff pool using igb_alloc_rx_buffers_zc.
> >
> > Use xsk_pool_get_rx_frame_size to set SRRCTL rx buf size when
> > zero-copy is enabled.
> >
> > Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@xxxxxxxx>
>
> ...
>
> > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
> > b/drivers/net/ethernet/intel/igb/igb_main.c
> > index 2c1e1d70bcf9..8eed3d0ab4fc 100644
> > --- a/drivers/net/ethernet/intel/igb/igb_main.c
> > +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> > @@ -502,12 +502,14 @@ static void igb_dump(struct igb_adapter
> > *adapter)
> >
> > for (i = 0; i < rx_ring->count; i++) {
> > const char *next_desc;
> > - struct igb_rx_buffer *buffer_info;
> > - buffer_info = &rx_ring->rx_buffer_info[i];
> > + struct igb_rx_buffer *buffer_info = NULL;
> > rx_desc = IGB_RX_DESC(rx_ring, i);
> > u0 = (struct my_u0 *)rx_desc;
> > staterr = le32_to_cpu(rx_desc-
> >wb.upper.status_error);
> >
> > + if (!rx_ring->xsk_pool)
> > + buffer_info = &rx_ring->rx_buffer_info[i];
> > +
> > if (i == rx_ring->next_to_use)
> > next_desc = " NTU";
> > else if (i == rx_ring->next_to_clean) @@ -530,7 +532,7
> @@ static
> > void igb_dump(struct igb_adapter *adapter)
> > (u64)buffer_info->dma,
>
> Hi Sriram,
>
> Here buffer_info is dereferenced...
>
> > next_desc);
> >
> > - if (netif_msg_pktdata(adapter) &&
> > + if (netif_msg_pktdata(adapter) && buffer_info
> &&
>
> And here buffer_info is checked against NULL.
>
> This combination doesn't seem quite right.
ACK, I will fix this. I am unable to test this path though.
>
> > buffer_info->dma && buffer_info->page) {
> > print_hex_dump(KERN_INFO, "",
> > DUMP_PREFIX_ADDRESS,
