When IPF detects a fragment that overlaps a previously received fragment
of the same datagram, discard not just that fragment but the entire
fragment list it belongs to. Per RFC 5722 and RFC 8200, when an
overlapping fragment is detected the whole datagram (and all of its
constituent fragments) must be silently discarded.
Previously such a fragment had its CT state marked invalid and was
returned to the conntrack batch, leaving the rest of the (now
unreassemblable) fragment list pinned until expiry. Instead, delete the
overlapping packet and tear down the whole fragment list, which also
avoids the work conntrack would otherwise do to mark the fragments as
invalid.
Assisted-by: composer-2.5-fast, Cursor
Fixes: 4ea96698f667 ("Userspace datapath: Add fragmentation handling.")
Signed-off-by: Eli Britstein <[email protected]>
---
lib/dpctl.c | 4 ++
lib/dpif-provider.h | 1 +
lib/ipf.c | 32 +++++++++++---
lib/ipf.h | 1 +
tests/ofproto-dpif.at | 76 ++++++++++++++++++++++++++++++++
tests/system-userspace-macros.at | 8 ++++
6 files changed, 117 insertions(+), 5 deletions(-)
diff --git a/lib/dpctl.c b/lib/dpctl.c
index c511b966c..6732a90c0 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -2629,6 +2629,8 @@ dpctl_ct_ipf_get_status(int argc, const char *argv[],
dpif_ipf_status.v4.nfrag_overlap);
dpctl_print(dpctl_p, " v4 frags duplicate: %"PRIu64"\n",
dpif_ipf_status.v4.nfrag_duplicate);
+ dpctl_print(dpctl_p, " v4 frags discarded: %"PRIu64"\n",
+ dpif_ipf_status.v4.nfrag_discarded);
dpctl_print(dpctl_p, " v4 frags purged: %"PRIu64"\n",
dpif_ipf_status.v4.nfrag_purged);
@@ -2646,6 +2648,8 @@ dpctl_ct_ipf_get_status(int argc, const char *argv[],
dpif_ipf_status.v6.nfrag_overlap);
dpctl_print(dpctl_p, " v6 frags duplicate: %"PRIu64"\n",
dpif_ipf_status.v6.nfrag_duplicate);
+ dpctl_print(dpctl_p, " v6 frags discarded: %"PRIu64"\n",
+ dpif_ipf_status.v6.nfrag_discarded);
dpctl_print(dpctl_p, " v6 frags purged: %"PRIu64"\n",
dpif_ipf_status.v6.nfrag_purged);
} else {
diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index 9c52aa282..eb80fb68d 100644
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -131,6 +131,7 @@ struct dpif_ipf_proto_status {
uint64_t nfrag_too_small;
uint64_t nfrag_overlap;
uint64_t nfrag_duplicate;
+ uint64_t nfrag_discarded;
uint64_t nfrag_purged;
unsigned int min_frag_size;
bool enabled;
diff --git a/lib/ipf.c b/lib/ipf.c
index 9adbd1fee..95ef07f49 100644
--- a/lib/ipf.c
+++ b/lib/ipf.c
@@ -84,6 +84,7 @@ enum ipf_counter_type {
IPF_NFRAGS_TOO_SMALL,
IPF_NFRAGS_OVERLAP,
IPF_NFRAGS_DUPLICATE,
+ IPF_NFRAGS_DISCARDED,
IPF_NFRAGS_PURGED,
IPF_NFRAGS_NUM_CNTS,
};
@@ -845,10 +846,27 @@ ipf_is_frag_overlap(const struct ipf_frag *frag_list, int
last_inuse_idx,
return false;
}
+/* Drops the entire fragment list: deletes every fragment packet held by the
+ * list and removes the list from the tracking datastructures. Per RFC 5722
+ * and RFC 8200, when an overlapping fragment is detected the whole datagram
+ * and all of its already received fragments must be silently discarded. */
+static void
+ipf_drop_frag_chain(struct ipf *ipf, struct ipf_list *ipf_list, bool v6)
+ OVS_REQUIRES(ipf->ipf_lock)
+{
+ for (int i = 0; i <= ipf_list->last_inuse_idx; i++) {
+ dp_packet_delete(ipf_list->frag_list[i].pkt);
+ atomic_count_dec(&ipf->nfrag);
+ ipf_count(ipf, v6, IPF_NFRAGS_DISCARDED);
+ }
+
+ ipf_list_clean(&ipf->frag_lists, ipf_list);
+}
+
/* Adds a fragment to a list of fragments. An exact duplicate fragment is
- * dropped on its own, keeping the rest of the list for later reassembly
- * (RFC 8200). An overlapping (but not duplicate) fragment is marked CT
- * invalid, to avoid the work conntrack would otherwise do. */
+ * dropped on its own, keeping the rest of the list (RFC 8200). An
+ * overlapping (but not duplicate) fragment causes the whole fragment list to
+ * be dropped (see ipf_drop_frag_chain(), RFC 5722 / RFC 8200). */
static bool
ipf_process_frag(struct ipf *ipf, struct ipf_list *ipf_list,
struct dp_packet *pkt, uint16_t start_data_byte,
@@ -882,8 +900,8 @@ ipf_process_frag(struct ipf *ipf, struct ipf_list *ipf_list,
dp_packet_delete(pkt);
} else {
ipf_count(ipf, v6, IPF_NFRAGS_OVERLAP);
- pkt->md.ct_state = CS_INVALID;
- return false;
+ dp_packet_delete(pkt);
+ ipf_drop_frag_chain(ipf, ipf_list, v6);
}
return true;
}
@@ -1473,6 +1491,8 @@ ipf_get_status(struct ipf *ipf, struct ipf_status
*ipf_status)
&ipf_status->v4.nfrag_overlap);
atomic_read_relaxed(&ipf->n4frag_cnt[IPF_NFRAGS_DUPLICATE],
&ipf_status->v4.nfrag_duplicate);
+ atomic_read_relaxed(&ipf->n4frag_cnt[IPF_NFRAGS_DISCARDED],
+ &ipf_status->v4.nfrag_discarded);
atomic_read_relaxed(&ipf->n4frag_cnt[IPF_NFRAGS_PURGED],
&ipf_status->v4.nfrag_purged);
@@ -1491,6 +1511,8 @@ ipf_get_status(struct ipf *ipf, struct ipf_status
*ipf_status)
&ipf_status->v6.nfrag_overlap);
atomic_read_relaxed(&ipf->n6frag_cnt[IPF_NFRAGS_DUPLICATE],
&ipf_status->v6.nfrag_duplicate);
+ atomic_read_relaxed(&ipf->n6frag_cnt[IPF_NFRAGS_DISCARDED],
+ &ipf_status->v6.nfrag_discarded);
atomic_read_relaxed(&ipf->n6frag_cnt[IPF_NFRAGS_PURGED],
&ipf_status->v6.nfrag_purged);
return 0;
diff --git a/lib/ipf.h b/lib/ipf.h
index 238d288c1..f7ad5d77e 100644
--- a/lib/ipf.h
+++ b/lib/ipf.h
@@ -29,6 +29,7 @@ struct ipf_proto_status {
uint64_t nfrag_too_small;
uint64_t nfrag_overlap;
uint64_t nfrag_duplicate;
+ uint64_t nfrag_discarded;
uint64_t nfrag_purged;
unsigned int min_frag_size;
bool enabled;
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 590059218..d968f5633 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -5728,6 +5728,82 @@ AT_CHECK([test 0 = `ovs-ofctl parse-pcap p1-tx.pcap | wc
-l`])
OVS_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([ofproto-dpif - fragment handling - reject overlapped fragment])
+OVS_VSWITCHD_START
+add_of_ports 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 The minimum fragment size is clamped to 400 bytes, so both fragments must
+dnl be at least that large to be admitted for reassembly.
+AT_CHECK([ovs-appctl dpctl/ipf-set-min-frag v4 400], [], [dnl
+setting minimum fragment size successful
+])
+
+dnl First admit a fragment carrying bytes 400..799, then send a new fragment
+dnl covering bytes 0..1199 that fully contains the previously admitted range.
+dnl The overlap causes the entire fragment list to be dropped (RFC 5722 /
+dnl RFC 8200), so no fragments remain tracked afterwards.
+dnl
+dnl Packet 1 (admitted). Middle fragment carrying bytes 400..799 (MF set).
+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 001. .... = Flags: 0x1 (More fragments)
+dnl ...0 0000 0011 0010 = Fragment Offset: 400
+dnl Time to Live: 64
+dnl Protocol: UDP (17)
+dnl Header Checksum: 0x42f3
+dnl Data (400 bytes)
+eth="50 54 00 00 00 0a 50 54 00 00 00 09 08 00"
+ip1="45 00 01 a4 00 20 20 32 40 11 42 f3"
+addrs="0a 01 01 01 0a 01 01 02"
+data1=$(printf '%0*d' 800 0)
+packet1="${eth}${ip1}${addrs}${data1}"
+
+dnl Packet 2 (rejected as overlap). First fragment carrying bytes 0..1199
+dnl (MF set), fully containing packet 1's byte range.
+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: 1220
+dnl Identification: 0x0020 (32)
+dnl 001. .... = Flags: 0x1 (More fragments)
+dnl ...0 0000 0000 0000 = Fragment Offset: 0
+dnl Time to Live: 64
+dnl Protocol: UDP (17)
+dnl Header Checksum: 0x4005
+dnl Data (1200 bytes)
+ip2="45 00 04 c4 00 20 20 00 40 11 40 05"
+data2=$(printf '%0*d' 2400 0)
+packet2="${eth}${ip2}${addrs}${data2}"
+
+AT_CHECK([ovs-appctl netdev-dummy/receive p90 "$packet1"])
+AT_CHECK([ovs-appctl netdev-dummy/receive p90 "$packet2"])
+
+AT_CHECK([ovs-appctl dpctl/ipf-get-status -m \
+| grep -E 'num frag:|v4 frags accepted:|v4 frags completed:|v4 frags
overlapped:|v4 frags discarded:'], [], [dnl
+ num frag: 0
+ v4 frags accepted: 1
+ v4 frags completed: 0
+ v4 frags overlapped: 1
+ v4 frags discarded: 1
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
AT_SETUP([ofproto-dpif - handling of malformed TCP packets])
OVS_VSWITCHD_START
add_of_ports br0 1 90
diff --git a/tests/system-userspace-macros.at b/tests/system-userspace-macros.at
index e78c45979..07e50bbdc 100644
--- a/tests/system-userspace-macros.at
+++ b/tests/system-userspace-macros.at
@@ -180,6 +180,7 @@ AT_CHECK([ovs-appctl dpctl/ipf-get-status], [], [dnl
v4 frags too small: 0
v4 frags overlapped: 0
v4 frags duplicate: 0
+ v4 frags discarded: 0
v4 frags purged: 0
min v6 frag size: 1280
v6 frags accepted: 0
@@ -188,6 +189,7 @@ AT_CHECK([ovs-appctl dpctl/ipf-get-status], [], [dnl
v6 frags too small: 0
v6 frags overlapped: 0
v6 frags duplicate: 0
+ v6 frags discarded: 0
v6 frags purged: 0
])
])
@@ -212,6 +214,7 @@ AT_CHECK([ovs-appctl dpctl/ipf-get-status --more], [], [dnl
v4 frags too small: 0
v4 frags overlapped: 0
v4 frags duplicate: 0
+ v4 frags discarded: 0
v4 frags purged: 0
min v6 frag size: 1280
v6 frags accepted: 0
@@ -220,6 +223,7 @@ AT_CHECK([ovs-appctl dpctl/ipf-get-status --more], [], [dnl
v6 frags too small: 0
v6 frags overlapped: 0
v6 frags duplicate: 0
+ v6 frags discarded: 0
v6 frags purged: 0
Fragment Lists:
@@ -247,6 +251,7 @@ AT_CHECK([ovs-appctl dpctl/ipf-get-status --more], [], [dnl
v4 frags too small: 0
v4 frags overlapped: 0
v4 frags duplicate: 0
+ v4 frags discarded: 0
v4 frags purged: 0
min v6 frag size: 1280
v6 frags accepted: 30
@@ -255,6 +260,7 @@ AT_CHECK([ovs-appctl dpctl/ipf-get-status --more], [], [dnl
v6 frags too small: 0
v6 frags overlapped: 0
v6 frags duplicate: 0
+ v6 frags discarded: 0
v6 frags purged: 0
Fragment Lists:
@@ -289,6 +295,7 @@ AT_CHECK([ovs-appctl dpctl/ipf-get-status -m |
FORMAT_FRAG_LIST()], [], [dnl
v4 frags too small: 0
v4 frags overlapped: 0
v4 frags duplicate: 0
+ v4 frags discarded: 0
v4 frags purged: 0
min v6 frag size: 1280
v6 frags accepted: 0
@@ -297,6 +304,7 @@ AT_CHECK([ovs-appctl dpctl/ipf-get-status -m |
FORMAT_FRAG_LIST()], [], [dnl
v6 frags too small: 0
v6 frags overlapped: 0
v6 frags duplicate: 0
+ v6 frags discarded: 0
v6 frags purged: 0
Fragment Lists:
--
2.43.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
