diff mbox series

[ovs-dev,v5,1/6] ipf: Drop overlap fragments.

Message ID 20260819102529.167827-2-elibr@nvidia.com
State Under Review
Delegated to: aaron conole
Headers show
Series ipf: Reject IPv4/v6 fragments with wrapping bounds. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_FreeBSD_Build_and_Test success github build: passed
ovsrobot/github-robot-_Build_and_Test warning github build: warning

Commit Message

Eli Britstein Aug. 19, 2026, 10:25 a.m. UTC
When IPF detects an overlapping or duplicate fragment, drop it instead
of marking its CT state invalid and returning it to the conntrack batch.
This aligns with the Linux kernel, which discards such fragments during
IP reassembly rather than forwarding them separately.

Assisted-by: composer-2.5-fast, Cursor
Fixes: 4ea96698f667 ("Userspace datapath: Add fragmentation handling.")
Signed-off-by: Eli Britstein <elibr@nvidia.com>
---
 lib/ipf.c             |  8 +++----
 tests/ofproto-dpif.at | 52 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 4 deletions(-)

Comments

Mike Pattrick Aug. 27, 2026, 7:08 p.m. UTC | #1
On Wed, Aug 19, 2026 at 6:26 AM Eli Britstein <elibr@nvidia.com> wrote:

> When IPF detects an overlapping or duplicate fragment, drop it instead
> of marking its CT state invalid and returning it to the conntrack batch.
> This aligns with the Linux kernel, which discards such fragments during
> IP reassembly rather than forwarding them separately.
>
> Assisted-by: composer-2.5-fast, Cursor
> Fixes: 4ea96698f667 ("Userspace datapath: Add fragmentation handling.")
> Signed-off-by: Eli Britstein <elibr@nvidia.com>
> ---


Checked that the test does exercise this change.

Acked-by: Mike Pattrick <mkp@redhat.com>
diff mbox series

Patch

diff --git a/lib/ipf.c b/lib/ipf.c
index d836b8824..185d6432e 100644
--- a/lib/ipf.c
+++ b/lib/ipf.c
@@ -823,8 +823,8 @@  ipf_is_frag_duped(const struct ipf_frag *frag_list, int last_inuse_idx,
 }
 
 /* Adds a fragment to a list of fragments, if the fragment is not a
- * duplicate. If the fragment is a duplicate, that fragment is marked
- * invalid to avoid the work that conntrack would do to mark the fragment
+ * duplicate. If the fragment is a duplicate, the fragment is dropped
+ * to avoid the work that conntrack would do to mark the fragment
  * as invalid, which it will in all cases. */
 static bool
 ipf_process_frag(struct ipf *ipf, struct ipf_list *ipf_list,
@@ -852,8 +852,8 @@  ipf_process_frag(struct ipf *ipf, struct ipf_list *ipf_list,
         }
     } else {
         ipf_count(ipf, v6, IPF_NFRAGS_OVERLAP);
-        pkt->md.ct_state = CS_INVALID;
-        return false;
+        dp_packet_delete(pkt);
+        return true;
     }
     return true;
 }
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index ee6ac873d..5308b3008 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -5669,6 +5669,58 @@  CHECK_COVERAGE([dpif_netdev_output_grow_queues], [1])
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([ofproto-dpif - fragment handling - drop duplicate fragment])
+OVS_VSWITCHD_START
+add_of_ports --pcap br0 1 90
+
+AT_DATA([flows.txt], [dnl
+table=0 in_port=90,ip actions=ct(commit),output:1
+])
+AT_CHECK([ovs-ofctl -O OpenFlow11 replace-flows br0 flows.txt])
+
+dnl Send the same fragment twice.  It is a last fragment (MF=0) at offset 400,
+dnl so it bypasses the minimum fragment size check and is admitted, yet it
+dnl cannot complete reassembly on its own (bytes 0..399 are missing).  The
+dnl second copy is a duplicate and must be dropped, rather than marked CT
+dnl invalid and returned to the datapath, so it is never forwarded.
+dnl
+dnl Last fragment carrying bytes 400..799 (MF clear).
+dnl   Ethernet II, Src: 50:54:00:00:00:09, Dst: 50:54:00:00:00:0a
+dnl       Type: IPv4 (0x0800)
+dnl   Internet Protocol Version 4, Src: 10.1.1.1, Dst: 10.1.1.2
+dnl       0100 .... = Version: 4
+dnl       .... 0101 = Header Length: 20 bytes (5)
+dnl       Differentiated Services Field: 0x00 (DSCP: CS0, ECN: Not-ECT)
+dnl       Total Length: 420
+dnl       Identification: 0x0020 (32)
+dnl       000. .... = Flags: 0x0 (Last fragment)
+dnl       ...0 0000 0011 0010 = Fragment Offset: 400
+dnl       Time to Live: 64
+dnl       Protocol: UDP (17)
+dnl       Header Checksum: 0x62f3
+dnl   Data (400 bytes)
+eth="50 54 00 00 00 0a 50 54 00 00 00 09 08 00"
+ip="45 00 01 a4 00 20 00 32 40 11 62 f3"
+addrs="0a 01 01 01 0a 01 01 02"
+data=$(printf '%0*d' 800 0)
+packet="${eth}${ip}${addrs}${data}"
+
+AT_CHECK([ovs-appctl netdev-dummy/receive p90 "$packet"])
+AT_CHECK([ovs-appctl netdev-dummy/receive p90 "$packet"])
+
+AT_CHECK([ovs-appctl dpctl/ipf-get-status -m \
+| grep -E 'v4 frags accepted:|v4 frags overlapped:'], [], [dnl
+        v4 frags accepted: 1
+        v4 frags overlapped: 1
+])
+
+dnl The duplicate fragment is dropped, not marked invalid and forwarded, so
+dnl nothing must be transmitted on the output port.
+AT_CHECK([test 0 = `ovs-ofctl parse-pcap p1-tx.pcap | wc -l`])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([ofproto-dpif - handling of malformed TCP packets])
 OVS_VSWITCHD_START
 add_of_ports br0 1 90