Currently ipf will inject expired fragments into the odp execution pipeline in an indeterminate location. Previously this was changed to segregate by IP version. However, there was nothing to stop fragments from being inserted into different tables, zones, or being sent in different directions.
The kernel module just terminates these fragments, and userspace should as well. There are also new coverage counters to indicate how fragments expired. Reported-at: https://issues.redhat.com/browse/FDP-1052 Fixes: 4ea96698f667 ("Userspace datapath: Add fragmentation handling.") Signed-off-by: Mike Pattrick <[email protected]> --- lib/ipf.c | 49 +++++++++++++++------------------- tests/ofproto-dpif.at | 61 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 28 deletions(-) diff --git a/lib/ipf.c b/lib/ipf.c index b76181e79..53dfe6664 100644 --- a/lib/ipf.c +++ b/lib/ipf.c @@ -35,6 +35,7 @@ #include "util.h" VLOG_DEFINE_THIS_MODULE(ipf); +COVERAGE_DEFINE(ipf_stuck_frag_list_expired); COVERAGE_DEFINE(ipf_stuck_frag_list_purged); COVERAGE_DEFINE(ipf_l3csum_err); @@ -77,7 +78,7 @@ enum { enum ipf_counter_type { IPF_NFRAGS_ACCEPTED, IPF_NFRAGS_COMPL_SENT, - IPF_NFRAGS_EXPD_SENT, + IPF_NFRAGS_EXPIRED, IPF_NFRAGS_TOO_SMALL, IPF_NFRAGS_OVERLAP, IPF_NFRAGS_PURGED, @@ -1030,8 +1031,7 @@ ipf_purge_list_check(struct ipf *ipf, struct ipf_list *ipf_list, * with 'ipf_send_completed_frags()' and 'ipf_send_expired_frags()'. */ static bool ipf_send_frags_in_list(struct ipf *ipf, struct ipf_list *ipf_list, - struct dp_packet_batch *pb, - enum ipf_list_type list_type, bool v6, long long now) + struct dp_packet_batch *pb, bool v6, long long now) OVS_REQUIRES(ipf->ipf_lock) { if (ipf_purge_list_check(ipf, ipf_list, now)) { @@ -1045,12 +1045,7 @@ ipf_send_frags_in_list(struct ipf *ipf, struct ipf_list *ipf_list, ipf_list->last_sent_idx++; atomic_count_dec(&ipf->nfrag); - if (list_type == IPF_FRAG_COMPLETED_LIST) { - ipf_count(ipf, v6, IPF_NFRAGS_COMPL_SENT); - } else { - ipf_count(ipf, v6, IPF_NFRAGS_EXPD_SENT); - pkt->md.ct_state = CS_INVALID; - } + ipf_count(ipf, v6, IPF_NFRAGS_COMPL_SENT); if (ipf_list->last_sent_idx == ipf_list->last_inuse_idx) { return true; @@ -1080,8 +1075,7 @@ ipf_send_completed_frags(struct ipf *ipf, struct dp_packet_batch *pb, if ((ipf_list->key.dl_type == htons(ETH_TYPE_IPV6)) != v6) { continue; } - if (ipf_send_frags_in_list(ipf, ipf_list, pb, IPF_FRAG_COMPLETED_LIST, - v6, now)) { + if (ipf_send_frags_in_list(ipf, ipf_list, pb, v6, now)) { ipf_completed_list_clean(&ipf->frag_lists, ipf_list); } else { break; @@ -1091,12 +1085,9 @@ ipf_send_completed_frags(struct ipf *ipf, struct dp_packet_batch *pb, ovs_mutex_unlock(&ipf->ipf_lock); } -/* Conservatively adds fragments associated with a expired fragment list to - * a packet batch to be processed by the calling application, typically - * conntrack. Also cleans up the list context when it is empty.*/ +/* Remove expired fragment lists and clean up the list context. */ static void -ipf_send_expired_frags(struct ipf *ipf, struct dp_packet_batch *pb, - long long now, bool v6) +ipf_delete_expired_frags(struct ipf *ipf, long long now) { enum { /* Very conservative, due to DOS probability. */ @@ -1113,21 +1104,23 @@ ipf_send_expired_frags(struct ipf *ipf, struct dp_packet_batch *pb, size_t lists_removed = 0; LIST_FOR_EACH_SAFE (ipf_list, list_node, &ipf->frag_exp_list) { - if ((ipf_list->key.dl_type == htons(ETH_TYPE_IPV6)) != v6) { - continue; - } if (now <= ipf_list->expiration || lists_removed >= IPF_FRAG_LIST_MAX_EXPIRED) { break; } - if (ipf_send_frags_in_list(ipf, ipf_list, pb, IPF_FRAG_EXPIRY_LIST, - v6, now)) { - ipf_expiry_list_clean(&ipf->frag_lists, ipf_list); - lists_removed++; - } else { - break; + while (ipf_list->last_sent_idx < ipf_list->last_inuse_idx) { + struct dp_packet * pkt + = ipf_list->frag_list[ipf_list->last_sent_idx + 1].pkt; + dp_packet_delete(pkt); + atomic_count_dec(&ipf->nfrag); + COVERAGE_INC(ipf_stuck_frag_list_expired); + ipf_count(ipf, ipf_list->key.dl_type == htons(ETH_TYPE_IPV6), + IPF_NFRAGS_EXPIRED); + ipf_list->last_sent_idx++; } + ipf_expiry_list_clean(&ipf->frag_lists, ipf_list); + lists_removed++; } ovs_mutex_unlock(&ipf->ipf_lock); @@ -1275,7 +1268,7 @@ ipf_postprocess_conntrack(struct ipf *ipf, struct dp_packet_batch *pb, bool v6 = dl_type == htons(ETH_TYPE_IPV6); ipf_post_execute_reass_pkts(ipf, pb, v6); ipf_send_completed_frags(ipf, pb, now, v6); - ipf_send_expired_frags(ipf, pb, now, v6); + ipf_delete_expired_frags(ipf, now); } } @@ -1448,7 +1441,7 @@ ipf_get_status(struct ipf *ipf, struct ipf_status *ipf_status) &ipf_status->v4.nfrag_accepted); atomic_read_relaxed(&ipf->n4frag_cnt[IPF_NFRAGS_COMPL_SENT], &ipf_status->v4.nfrag_completed_sent); - atomic_read_relaxed(&ipf->n4frag_cnt[IPF_NFRAGS_EXPD_SENT], + atomic_read_relaxed(&ipf->n4frag_cnt[IPF_NFRAGS_EXPIRED], &ipf_status->v4.nfrag_expired_sent); atomic_read_relaxed(&ipf->n4frag_cnt[IPF_NFRAGS_TOO_SMALL], &ipf_status->v4.nfrag_too_small); @@ -1464,7 +1457,7 @@ ipf_get_status(struct ipf *ipf, struct ipf_status *ipf_status) &ipf_status->v6.nfrag_accepted); atomic_read_relaxed(&ipf->n6frag_cnt[IPF_NFRAGS_COMPL_SENT], &ipf_status->v6.nfrag_completed_sent); - atomic_read_relaxed(&ipf->n6frag_cnt[IPF_NFRAGS_EXPD_SENT], + atomic_read_relaxed(&ipf->n6frag_cnt[IPF_NFRAGS_EXPIRED], &ipf_status->v6.nfrag_expired_sent); atomic_read_relaxed(&ipf->n6frag_cnt[IPF_NFRAGS_TOO_SMALL], &ipf_status->v6.nfrag_too_small); diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index 6f7cc166f..a35b80077 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -5312,6 +5312,67 @@ recirc_id(0),in_port(90),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(proto=6,fr OVS_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([ofproto-dpif - fragment handling - reassembly]) +OVS_VSWITCHD_START +add_of_ports br0 1 2 90 + +AT_DATA([flows.txt], [dnl +table=0 tcp actions=ct(commit, table=1) +table=1 tcp actions=2 +]) +AT_CHECK([ovs-ofctl -O OpenFlow11 replace-flows br0 flows.txt]) + +dnl Test frag expiry. +AT_CHECK([ovs-appctl netdev-dummy/receive p90 "0021853763af 0026b98cb0f9 0800 4500 0014 0001 8192 40 06 3316 ac11370d ac11370b"]) +ovs-appctl time/warp 10000 +AT_CHECK([ovs-appctl netdev-dummy/receive p90 "0021853763af 0026b98cb0f9 0800 4500 0014 0001 8192 40 06 3316 ac11370d ac11370b"]) + +dnl Test that no packets flow. +AT_CHECK([ovs-appctl dpctl/dump-flows filter=in_port\(90\) | sed s'/recirc(.*)/recirc(X)/'], [0], [dnl +flow-dump from the main thread: +recirc_id(0),in_port(90),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(proto=6,frag=later), packets:0, bytes:0, used:never, actions:ct(commit),recirc(X) +]) + +dnl Expire second frag. +ovs-appctl time/warp 10000 + +dnl Test frag purge +AT_CHECK([ovs-appctl netdev-dummy/receive p90 "0021853763af 0026b98cb0f9 0800 4500 0014 0002 8192 40 06 3315 ac11370d ac11370b"]) +ovs-appctl time/warp 33000 +AT_CHECK([ovs-appctl netdev-dummy/receive p90 "0021853763af 0026b98cb0f9 0800 4500 0014 0003 8192 40 06 3314 ac11370d ac11370b"]) + +dnl Test that no packets flow. +AT_CHECK([ovs-appctl dpctl/dump-flows filter=in_port\(90\) | sed s'/recirc(.*)/recirc(X)/'], [0], [dnl +flow-dump from the main thread: +recirc_id(0),in_port(90),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(proto=6,frag=later), packets:0, bytes:0, used:never, actions:ct(commit),recirc(X) +]) + +dnl Purge second frag +ovs-appctl time/warp 33000 + +dnl Make sure all four packets are counted properly in the coverage. +AT_CHECK([ovs-appctl coverage/show | grep -c "^ipf.*total: 2"], [0], [2 +]) + +zero1208=$(printf '0%.0s' $(seq 2416)) +dnl Test that reassembled packets flow. +AT_CHECK([ovs-appctl netdev-dummy/receive p90 "0021853763af 0026b98cb0f9 0800 4500 04c4 0002 2000 40 06 8ff7 ac11370d ac11370b dnl +0000 0001 00000000 00000000 50 10 8000 604c 0000 dnl +$zero1208"]) +AT_CHECK([ovs-appctl netdev-dummy/receive p90 "0021853763af 0026b98cb0f9 0800 4500 04c4 0002 0096 40 06 af61 ac11370d ac11370b dnl +$zero1208"]) + +AT_CHECK([ovs-appctl dpctl/dump-flows | sed 's/recirc[[^)]]*)/recirc()/g' | strip_used | sort], [0], [dnl +flow-dump from the main thread: +recirc(),in_port(90),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(proto=6,frag=first), packets:0, bytes:0, used:never, actions:2 +recirc(),in_port(90),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(proto=6,frag=first), packets:0, bytes:0, used:never, actions:ct(commit),recirc() +recirc(),in_port(90),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(proto=6,frag=later), packets:0, bytes:0, used:never, actions:2 +recirc(),in_port(90),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(proto=6,frag=later), packets:0, bytes:0, used:never, actions:ct(commit),recirc() +]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + AT_SETUP([ofproto-dpif - handling of malformed TCP packets]) OVS_VSWITCHD_START add_of_ports br0 1 90 -- 2.49.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
