On 10/4/26 12:14 PM, Eli Britstein via dev wrote:
> The previous check only tested whether a new fragment's endpoints fell
> inside an existing range and treated any such match as a fragment to be
> dropped on its own. This missed the case where a new fragment fully
> contains an existing one, and it conflated two cases that the kernel and
> RFCs 815, 5722 and 8200 handle differently:
>
> - An exact duplicate (same byte range as an existing fragment) may be
> dropped on its own while the rest of the fragment list is kept for
> later reassembly (RFC 8200).
>
> - An overlapping (but not duplicate) fragment means the whole datagram
> must be silently discarded along with all of its fragments
> (RFC 5722 / RFC 8200).
>
> Detect these two cases separately using closed-interval overlap: drop an
> exact duplicate on its own, and tear down the entire fragment list on a
> real overlap.
>
> Assisted-by: composer-2.5-fast, Cursor
> Fixes: 4ea96698f667 ("Userspace datapath: Add fragmentation handling.")
> Signed-off-by: Eli Britstein <[email protected]>
> ---
Continuation of the LLM review:
This patch separates duplicate fragments from overlapping ones, keeping
the queued fragments when an exact duplicate arrives.
> + if (start_data_byte == frag_list[i].start_data_byte &&
> + end_data_byte == frag_list[i].end_data_byte) {
> + return true;
> + }
[...]
> + } else if (duped_frag) {
> + /* RFC 8200: an exact duplicate may be dropped while keeping the rest
> + * of the fragment list for later reassembly. */
> + ipf_count(ipf, v6, IPF_NFRAGS_DUPLICATE);
> + dp_packet_delete(pkt);
Should a fragment with the same range but a different MF value count as
an exact duplicate?
With the minimum fragment size set to 400, consider these fragments
arriving with the same reassembly key:
[400..799], MF=1
[400..799], MF=0
[0..399], MF=1
The second packet is discarded as a duplicate without passing lf=true to
ipf_list_state_transition(). After the third packet all bytes are
present, but the list never records a last fragment and expires.
The same sequence did not complete before this series either. Since the
commit message cites the kernel behavior, though: Linux records the
final length and LAST_IN flag before inet_frag_queue_insert() returns
IPFRAG_DUP, so this sequence reassembles there. Would it make sense to
keep the last-fragment information here, or to treat the MF mismatch as
a conflict?
> The previous check only tested whether a new fragment's endpoints fell
> inside an existing range and treated any such match as a fragment to be
> dropped on its own.
Could this describe the immediate parent's behavior instead? A matching
fragment already causes ipf_drop_frag_chain() to discard the entire list.
This patch changes exact duplicates back to dropping only the incoming
packet.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
