Re: [PATCH V6 net-next 07/11] net: hibmcge: Implement rx_poll function to receive packets

[Jijie Shao]

on 2024/9/3 20:08, Paolo Abeni wrote:
> On 8/30/24 14:16, Jijie Shao wrote:
> > @@ -119,6 +122,20 @@ static void hbg_buffer_free_skb(struct 
> > hbg_buffer *buffer)
> >       buffer->skb = NULL;
> >   }
> >   +static int hbg_buffer_alloc_skb(struct hbg_buffer *buffer)
> > +{
> > +    u32 len = hbg_spec_max_frame_len(buffer->priv, buffer->dir);
> > +    struct hbg_priv *priv = buffer->priv;
> > +
> > +    buffer->skb = netdev_alloc_skb(priv->netdev, len);
> > +    if (unlikely(!buffer->skb))
> > +        return -ENOMEM;
>
> It's preferable to allocate the skbuff at packet reception time, 
> inside the poll() function, just before passing the skb to the upper 
> stack, so that the header contents are fresh in the cache. 
> Additionally that increases the change for the allocator could hit its 
> fastpath.

In hibmcge driver, we alloc the skb memory first, after dma, and then set the dam address to the MAC for receiving packets.
After receiving a packet, MAC fills the hw rx descriptor and packet content to skb->data, and then reports an RX interrupt to trigger the driver to receive the packet.
In poll(), we use skb_reserve() to adjust the size of the SKB headroom. The skb->data is moved backward by the descriptor length(HBG_PACKET_HEAD_SIZE) to ensure
the skb->data is at the start position of the packet.

   ┌─────────────────┬────────────────────────────────────┐
   │hw rx descriptor │                packet              │
   │                 │                                    │
   └─────────────────┴────────────────────────────────────┘
   ^
  skb->data

> > +
> > +    buffer->skb_len = len;
> > +    memset(buffer->skb->data, 0, HBG_PACKET_HEAD_SIZE);
>
> Out of sheer ignorace, why do you need to clear the packet data?
The length of HBG_PACKET_HEAD_SIZE is exactly the size of the rx descriptor. Therefore, we want to clear before receiving packets to ensure that the descriptor is correct.

	Thanks
	Jijie Shao