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

From: Jijie Shao
Date: Tue Sep 03 2024 - 08:57:44 EST



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