When conntrack is reassembling packet fragments, the same reassembly
context can be shared across multiple threads handling different packets
simultaneously. Once a full packet is assembled, it is added to a packet
batch for processing, in the case where there are multiple different pmd
threads accessing conntrack simultaneously, there is a race condition
where the reassembled packet may be added to an arbitrary batch even if
the current batch is available.
When this happens, the packet may be handled incorrectly as it is
inserted into a random openflow execution pipeline, instead of the
pipeline for that packets flow.
This change makes a best effort attempt to try to add the defragmented
packet to the current batch. directly. This should succeed most of the
time.
Fixes: 4ea96698f667 ("Userspace datapath: Add fragmentation handling.")
Reported-at: https://issues.redhat.com/browse/FDP-560
Signed-off-by: Mike Pattrick <m...@redhat.com>
---
lib/ipf.c | 27 ++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/lib/ipf.c b/lib/ipf.c
index 3c8960be3..2d715f5e9 100644
--- a/lib/ipf.c
+++ b/lib/ipf.c
@@ -506,13 +506,15 @@ ipf_reassemble_v6_frags(struct ipf_list *ipf_list)
}
/* Called when a frag list state transitions to another state. This is
- * triggered by new fragment for the list being received.*/
-static void
+* triggered by new fragment for the list being received. Returns a reassembled
+* packet if this fragment has completed one. */
+static struct reassembled_pkt *
ipf_list_state_transition(struct ipf *ipf, struct ipf_list *ipf_list,
bool ff, bool lf, bool v6)
OVS_REQUIRES(ipf->ipf_lock)
{
enum ipf_list_state curr_state = ipf_list->state;
+ struct reassembled_pkt *ret = NULL;
enum ipf_list_state next_state;
switch (curr_state) {
case IPF_LIST_STATE_UNUSED:
@@ -562,12 +564,15 @@ ipf_list_state_transition(struct ipf *ipf, struct
ipf_list *ipf_list,
ipf_reassembled_list_add(&ipf->reassembled_pkt_list, rp);
ipf_expiry_list_remove(ipf_list);
next_state = IPF_LIST_STATE_COMPLETED;
+ ret = rp;
} else {
next_state = IPF_LIST_STATE_REASS_FAIL;
}
}
}
ipf_list->state = next_state;
+
+ return ret;
}
/* Some sanity checks are redundant, but prudent, in case code paths for
@@ -799,7 +804,8 @@ ipf_is_frag_duped(const struct ipf_frag *frag_list, int
last_inuse_idx,
static bool
ipf_process_frag(struct ipf *ipf, struct ipf_list *ipf_list,
struct dp_packet *pkt, uint16_t start_data_byte,
- uint16_t end_data_byte, bool ff, bool lf, bool v6)
+ uint16_t end_data_byte, bool ff, bool lf, bool v6,
+ struct reassembled_pkt **rp)
OVS_REQUIRES(ipf->ipf_lock)
{
bool duped_frag = ipf_is_frag_duped(ipf_list->frag_list,
@@ -820,7 +826,7 @@ ipf_process_frag(struct ipf *ipf, struct ipf_list *ipf_list,
ipf_list->last_inuse_idx++;
atomic_count_inc(&ipf->nfrag);
ipf_count(ipf, v6, IPF_NFRAGS_ACCEPTED);
- ipf_list_state_transition(ipf, ipf_list, ff, lf, v6);
+ *rp = ipf_list_state_transition(ipf, ipf_list, ff, lf, v6);
} else {
OVS_NOT_REACHED();
}
@@ -853,7 +859,8 @@ ipf_list_init(struct ipf_list *ipf_list, struct
ipf_list_key *key,
* to a list of fragemnts. */
static bool
ipf_handle_frag(struct ipf *ipf, struct dp_packet *pkt, ovs_be16 dl_type,
- uint16_t zone, long long now, uint32_t hash_basis)
+ uint16_t zone, long long now, uint32_t hash_basis,
+ struct reassembled_pkt **rp)
OVS_REQUIRES(ipf->ipf_lock)
{
struct ipf_list_key key;
@@ -922,7 +929,7 @@ ipf_handle_frag(struct ipf *ipf, struct dp_packet *pkt,
ovs_be16 dl_type,
}
return ipf_process_frag(ipf, ipf_list, pkt, start_data_byte,
- end_data_byte, ff, lf, v6);
+ end_data_byte, ff, lf, v6, rp);
}
/* Filters out fragments from a batch of fragments and adjust the batch. */
@@ -941,11 +948,17 @@ ipf_extract_frags_from_batch(struct ipf *ipf, struct
dp_packet_batch *pb,
||
(dl_type == htons(ETH_TYPE_IPV6) &&
ipf_is_valid_v6_frag(ipf, pkt)))) {
+ struct reassembled_pkt *rp = NULL;
ovs_mutex_lock(&ipf->ipf_lock);
- if (!ipf_handle_frag(ipf, pkt, dl_type, zone, now, hash_basis)) {
+ if (!ipf_handle_frag(ipf, pkt, dl_type, zone, now, hash_basis,
+ &rp)) {
dp_packet_batch_refill(pb, pkt, pb_idx);
} else {
+ if (rp && !dp_packet_batch_is_full(pb)) {
+ dp_packet_batch_refill(pb, rp->pkt, pb_idx);
+ rp->list->reass_execute_ctx = rp->pkt;
+ }
dp_packet_delete(pkt);
}
ovs_mutex_unlock(&ipf->ipf_lock);
--
2.39.3
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
