Eli Britstein <[email protected]> writes:
> The previous overlap check only tested whether a new fragment's
> endpoints fell inside an existing range, missing the case where the
> new fragment completely contains an older one. Use closed-interval
> overlap so conflicting fragments are rejected.
>
> Assisted-by: composer-2.5-fast, Cursor
> Fixes: 4ea96698f667 ("Userspace datapath: Add fragmentation handling.")
> Acked-by: Mike Pattrick <[email protected]>
> Signed-off-by: Eli Britstein <[email protected]>
> ---
> lib/ipf.c | 8 ++---
> tests/ofproto-dpif.at | 73 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 77 insertions(+), 4 deletions(-)
>
> diff --git a/lib/ipf.c b/lib/ipf.c
> index 72ebd238a..2b016e60b 100644
> --- a/lib/ipf.c
> +++ b/lib/ipf.c
> @@ -843,16 +843,16 @@ ipf_list_key_lookup(struct ipf *ipf, const struct
> ipf_list_key *key,
> return NULL;
> }
>
> +/* Returns true if the new fragment overlaps any existing fragment. Uses
> + * closed-interval overlap: start_a <= end_b && end_a >= start_b. */
> static bool
> ipf_is_frag_duped(const struct ipf_frag *frag_list, int last_inuse_idx,
> size_t start_data_byte, size_t end_data_byte)
The AI didn't flag it, but we should consider a rename here. This
function should be more than just duped packets now (consider something
like ipf_is_frag_dup_or_overlap)
> /* OVS_REQUIRES(ipf_lock) */
> {
> for (int i = 0; i <= last_inuse_idx; i++) {
> - if ((start_data_byte >= frag_list[i].start_data_byte &&
> - start_data_byte <= frag_list[i].end_data_byte) ||
> - (end_data_byte >= frag_list[i].start_data_byte &&
> - end_data_byte <= frag_list[i].end_data_byte)) {
> + if (start_data_byte <= frag_list[i].end_data_byte &&
> + end_data_byte >= frag_list[i].start_data_byte) {
> return true;
> }
> }
> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index 0e84dc308..0ff4c50f8 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -5769,6 +5769,79 @@ AT_CHECK([ovs-appctl dpctl/ipf-get-status -m \
> 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 reject a new
> fragment
> +dnl covering bytes 0..1199 that fully contains the previously admitted range.
> +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:'], [], [dnl
> + num frag: 1
> + v4 frags accepted: 1
> + v4 frags completed: 0
> + v4 frags overlapped: 1
> +])
> +
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> +
> AT_SETUP([ofproto-dpif - handling of malformed TCP packets])
> OVS_VSWITCHD_START
> add_of_ports br0 1 90
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
