Re: [PATCH v4 01/24] net/_common_intel: add pkt reassembly fn for intel drivers

[Stephen Hemminger <stephen@networkplumber.org>]

On Fri, 20 Dec 2024 14:38:58 +0000
Bruce Richardson <bruce.richardson@intel.com> wrote:

> +
> +			if (!split_flags[buf_idx]) {
> +				/* it's the last packet of the set */
> +				start->hash = end->hash;
> +				start->vlan_tci = end->vlan_tci;
> +				start->ol_flags = end->ol_flags;
> +				/* we need to strip crc for the whole packet */
> +				start->pkt_len -= crc_len;
> +				if (end->data_len > crc_len) {
> +					end->data_len -= crc_len;
> +				} else {
> +					/* free up last mbuf */
> +					struct rte_mbuf *secondlast = start;
> +
> +					start->nb_segs--;
> +					while (secondlast->next != end)
> +						secondlast = secondlast->next;
> +					secondlast->data_len -= (crc_len - end->data_len);
> +					secondlast->next = NULL;
> +					rte_pktmbuf_free_seg(end);
> +				}

The problem with freeing the last buffer is that the CRC will be garbage.
What if the CRC is sitting past the last mbuf?

+-----------------------+    +-----+
| Data                  +--->+ CRC |
+-----------------------+    +-----+

This part (from original code) will free the second mbuf which contains
the CRC. The whole "don't strip CRC and leave it past the mbuf data" model
of mbuf's is a danger trap.