drivers: eth_nxp_enet_qos: Don't drop unfinished packets

Instead of dropping packets for which we did not yet receive all the
frames by the end of the work run, just store the pointer to the packet
which is still in progress and finish it on the next work run, once the
remaining frames come in. Ther doesn't seem to be any valid reason to
drop the packet just because the frames didn't finish coming in by the
time we are processing them. The MTL fifos will keep everything in
order.

Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

Author: decsny

drivers/ethernet/eth_nxp_enet_qos/eth_nxp_enet_qos_mac.c

@@ -221,8 +221,8 @@ static void enet_qos_dma_rx_resume(const struct device *dev)
 /* treats the frame as a potential start of a new packet.
  * returns new net_pkt pointer if successfully received or null otherwise.
  */
-static struct net_pkt *enet_qos_start_new_packet(const struct device *dev,
-						 volatile union nxp_enet_qos_rx_desc *desc)
+static void enet_qos_start_new_rx_packet(const struct device *dev,
+					 volatile union nxp_enet_qos_rx_desc *desc)
 {
 	struct nxp_enet_qos_mac_data *data = dev->data;
 	struct nxp_enet_qos_rx_data *rx_data = &data->rx;
@@ -234,42 +234,46 @@ static struct net_pkt *enet_qos_start_new_packet(const struct device *dev,
 		 * drop error, since the starting frame must have been dropped and logged
 		 * earlier. ie. we only log dropping first descriptor flag frames.
 		 */
-		goto skip;
+		return;
 	}
 
 	pkt = net_pkt_rx_alloc(K_NO_WAIT);
 
 	if (!pkt) {
 		LOG_ERR("Could not alloc new RX pkt");
-		goto skip;
+	} else {
+		LOG_DBG("New RX pkt %p", pkt);
 	}
 
 	rx_data->processed_pkt_len = 0;
+	rx_data->pkt = pkt;
+}
 
-	LOG_DBG("New RX pkt %p", pkt);
+static void enet_qos_finish_rx_packet(const struct device *dev)
+{
+	struct nxp_enet_qos_mac_data *data = dev->data;
+	struct nxp_enet_qos_rx_data *rx_data = &data->rx;
 
-	return pkt;
-skip:
-	return NULL;
+	rx_data->pkt = NULL;
+	rx_data->processed_pkt_len = 0;
 }
 
-static void enet_qos_drop_rx_packet(const struct device *dev,
-				    struct net_pkt *pkt)
+static void enet_qos_drop_rx_packet(const struct device *dev)
 {
 	struct nxp_enet_qos_mac_data *data = dev->data;
 	struct nxp_enet_qos_rx_data *rx_data = &data->rx;
+	struct net_pkt *pkt = rx_data->pkt;
 
 	eth_stats_update_errors_rx(data->iface);
 	if (pkt != NULL) {
 		LOG_WRN("Dropped packet %p", pkt);
 		net_pkt_unref(pkt);
 	}
 
-	rx_data->processed_pkt_len = 0;
+	enet_qos_finish_rx_packet(dev);
 }
 
 static void enet_qos_drop_rx_frame(const struct device *dev,
-				   struct net_pkt *pkt,
 				   volatile union nxp_enet_qos_rx_desc *desc)
 {
 	struct nxp_enet_qos_mac_data *data = dev->data;
@@ -312,7 +316,6 @@ static int enet_qos_get_frame_len(const struct device *dev,
 }
 
 static int enet_qos_append_frame_to_packet(const struct device *dev,
-					   struct net_pkt *pkt,
 					   volatile union nxp_enet_qos_rx_desc *desc,
 					   size_t frame_len)
 {
@@ -321,33 +324,33 @@ static int enet_qos_append_frame_to_packet(const struct device *dev,
 	volatile union nxp_enet_qos_rx_desc *desc_arr = rx_data->descriptors;
 	int desc_idx = ARRAY_INDEX(desc_arr, desc);
 	struct net_buf *frame_buf = data->rx.reserved_bufs[desc_idx];
+	struct net_pkt *pkt = rx_data->pkt;
 
 	net_buf_add(frame_buf, frame_len);
 	net_pkt_frag_add(pkt, frame_buf);
 
 	return 0;
 }
 
-static int enet_qos_pass_up_rx(const struct device *dev, struct net_pkt **pkt)
+static int enet_qos_pass_up_rx(const struct device *dev)
 {
 	struct nxp_enet_qos_mac_data *data = dev->data;
 	struct nxp_enet_qos_rx_data *rx_data = &data->rx;
 	int ret = 0;
 
 	LOG_DBG("Receiving RX packet");
 
-	ret = net_recv_data(data->iface, *pkt);
+	ret = net_recv_data(data->iface, rx_data->pkt);
 	if (ret != 0) {
-		LOG_ERR("RECV failed for pkt %p", *pkt);
-		enet_qos_drop_rx_packet(dev, *pkt);
+		LOG_ERR("RECV failed for pkt %p", rx_data->pkt);
+		enet_qos_drop_rx_packet(dev);
 		goto done;
 	}
 
 	eth_stats_update_pkts_rx(data->iface);
 
 done:
-	rx_data->processed_pkt_len = 0;
-	*pkt = NULL;
+	enet_qos_finish_rx_packet(dev);
 	return ret;
 }
 
@@ -374,19 +377,18 @@ static void enet_qos_swap_rx_desc_buf(const struct device *dev,
 }
 
 static void eth_nxp_enet_qos_process_rx_frame(const struct device *dev,
-					     struct net_pkt **pkt,
 					     volatile union nxp_enet_qos_rx_desc *desc)
 {
 	struct nxp_enet_qos_mac_data *data = dev->data;
 	struct nxp_enet_qos_rx_data *rx_data = &data->rx;
 	int frame_len = 0;
 	int ret = 0;
 
-	if (*pkt == NULL) {
+	if (rx_data->pkt == NULL) {
 		/* Trying to start a new packet if none is provided */
-		*pkt = enet_qos_start_new_packet(dev, desc);
+		enet_qos_start_new_rx_packet(dev, desc);
 	}
-	if (*pkt == NULL) {
+	if (rx_data->pkt == NULL) {
 		/* Still no packet so drop the frame */
 		goto drop;
 	}
@@ -397,23 +399,22 @@ static void eth_nxp_enet_qos_process_rx_frame(const struct device *dev,
 	}
 
 	/* Take the received data and add to the packet */
-	enet_qos_append_frame_to_packet(dev, *pkt, desc, frame_len);
+	enet_qos_append_frame_to_packet(dev, desc, frame_len);
 	rx_data->processed_pkt_len += frame_len;
 
 	if (!frame_is_last(desc)) {
 		goto done;
 	}
 
 	/* If this is the last frame of the packet, send it up the stack */
-	ret = enet_qos_pass_up_rx(dev, pkt);
+	ret = enet_qos_pass_up_rx(dev);
 	if (ret != 0) {
 		goto drop;
 	}
 
 	goto done;
 drop:
-	enet_qos_drop_rx_frame(dev, *pkt, desc);
-	*pkt = NULL;
+	enet_qos_drop_rx_frame(dev, desc);
 	return;
 done:
 	/* last thing to do is switch the buf so the DMA doesn't overwrite
@@ -432,7 +433,7 @@ static void eth_nxp_enet_qos_rx(struct k_work *work)
 	volatile union nxp_enet_qos_rx_desc *desc_arr = data->rx.descriptors;
 	uint32_t desc_idx = rx_data->next_desc_idx;
 	volatile union nxp_enet_qos_rx_desc *desc = &desc_arr[desc_idx];
-	struct net_pkt *pkt = NULL;
+	struct net_pkt *pkt = rx_data->pkt;
 
 	LOG_DBG("RX work start: %p", work);
 
@@ -442,19 +443,13 @@ static void eth_nxp_enet_qos_rx(struct k_work *work)
 	while (software_owns_descriptor(desc)) {
 		rx_data->next_desc_idx = (desc_idx + 1U) % NUM_RX_BUFDESC;
 
-		eth_nxp_enet_qos_process_rx_frame(dev, &pkt, desc);
+		eth_nxp_enet_qos_process_rx_frame(dev, desc);
 
 		desc_idx = rx_data->next_desc_idx;
 		desc = &desc_arr[desc_idx];
 	}
 
-	if (pkt != NULL) {
-		/* Looped through descriptors without reaching the final
-		 * fragment of the packet, deallocate the incomplete one
-		 */
-		LOG_DBG("Incomplete packet received, cleaning up");
-		enet_qos_drop_rx_packet(dev, pkt);
-	}
+	rx_data->pkt = pkt;
 
 	/* now that we updated the descriptors, resume in case we are suspended */
 	enet_qos_dma_rx_resume(dev);

drivers/ethernet/eth_nxp_enet_qos/nxp_enet_qos_priv.h

@@ -117,6 +117,7 @@ struct nxp_enet_qos_rx_data {
 	volatile union nxp_enet_qos_rx_desc descriptors[NUM_RX_BUFDESC];
 	struct net_buf *reserved_bufs[NUM_RX_BUFDESC];
 	size_t processed_pkt_len;
+	struct net_pkt *pkt;
 };
 
 struct nxp_enet_qos_mac_data {