Hi Aaron,
Miss the udp-frag.pcap. My case : ovs-ofctl add-flow manbr "table=0,ct_state=-trk,ip,in_port=dpdk2, actions=ct(table=1, nat)" ovs-ofctl add-flow manbr "table=0,ct_state=-trk,ip,in_port=dpdk3, actions=ct(table=1, nat)" ovs-ofctl add-flow manbr "table=0,table=1,ct_state=+trk+new,ip,in_port=dpdk2, actions=ct(commit, nat(src=1.1.1.2)),dpdk3" ovs-ofctl add-flow manbr "table=0,table=1,ct_state=+trk+est,ip,in_port=dpdk2, actions=dpdk3" ovs-ofctl add-flow manbr "table=0,table=1,ct_state=+trk+est,ip,in_port=dpdk3, actions=dpdk2" ovs-ofctl add-flow manbr "table=0,arp,actions=normal" With the frag-packet in udp-frag.pcap. And the wrong nated packet. nat-packet.pcap send the packet: iperf -u -c 1.1.1.3 -t 120 -i 2 -b 1 -l 1800 From: Aaron Conole <acon...@redhat.com> Date: 2021-07-09 04:11:12 To: we...@ucloud.cn Cc: i.maxim...@ovn.org,d...@openvswitch.org Subject: Re: [ovs-dev] [PATCH v2] ipf: fix only nat the first fragment in the reass process>we...@ucloud.cn writes: > >> From: wenxu <we...@ucloud.cn> >> >> The ipf collect original fragment packets and reass a new pkt >> to do the conntrack logic. After finsh the conntrack things >> copy the ct meta info to each orignal packet and modify the >> l4 header in the first fragment. It should modify the ip src/ >> dst info for all the fragments. >> >> Signed-off-by: wenxu <we...@ucloud.cn> >> Co-authored-by: luke.li <luke...@ucloud.cn> >> Signed-off-by: luke.li <luke...@ucloud.cn> >> --- > >This looks okay. Please post a set of sample packets that showcase the >issue. I will write a test case that can demonstrate it that we can >check in with this fix. >Alternately, please add such a test case. > >> lib/ipf.c | 82 >> +++++++++++++++++++++++++++++++++------------------------------ >> 1 file changed, 43 insertions(+), 39 deletions(-) >> >> diff --git a/lib/ipf.c b/lib/ipf.c >> index 9c83f19..795e801 100644 >> --- a/lib/ipf.c >> +++ b/lib/ipf.c >> @@ -1149,52 +1149,56 @@ ipf_post_execute_reass_pkts(struct ipf *ipf, >> * NETDEV_MAX_BURST. */ >> DP_PACKET_BATCH_REFILL_FOR_EACH (pb_idx, pb_cnt, pkt, pb) { >> if (rp && pkt == rp->list->reass_execute_ctx) { >> + const struct ipf_frag *frag_0 = &rp->list->frag_list[0]; >> + void *l4_frag = dp_packet_l4(frag_0->pkt); >> + void *l4_reass = dp_packet_l4(pkt); >> + memcpy(l4_frag, l4_reass, dp_packet_l4_size(frag_0->pkt)); >> + >> for (int i = 0; i <= rp->list->last_inuse_idx; i++) { >> - rp->list->frag_list[i].pkt->md.ct_label = >> pkt->md.ct_label; >> - rp->list->frag_list[i].pkt->md.ct_mark = >> pkt->md.ct_mark; >> - rp->list->frag_list[i].pkt->md.ct_state = >> pkt->md.ct_state; >> - rp->list->frag_list[i].pkt->md.ct_zone = >> pkt->md.ct_zone; >> - rp->list->frag_list[i].pkt->md.ct_orig_tuple_ipv6 = >> - pkt->md.ct_orig_tuple_ipv6; >> + const struct ipf_frag *frag_i = &rp->list->frag_list[i]; >> + >> + frag_i->pkt->md.ct_label = pkt->md.ct_label; >> + frag_i->pkt->md.ct_mark = pkt->md.ct_mark; >> + frag_i->pkt->md.ct_state = pkt->md.ct_state; >> + frag_i->pkt->md.ct_zone = pkt->md.ct_zone; >> + frag_i->pkt->md.ct_orig_tuple_ipv6 = >> + pkt->md.ct_orig_tuple_ipv6; >> if (pkt->md.ct_orig_tuple_ipv6) { >> - rp->list->frag_list[i].pkt->md.ct_orig_tuple.ipv6 = >> + frag_i->pkt->md.ct_orig_tuple.ipv6 = >> pkt->md.ct_orig_tuple.ipv6; >> } else { >> - rp->list->frag_list[i].pkt->md.ct_orig_tuple.ipv4 = >> + frag_i->pkt->md.ct_orig_tuple.ipv4 = >> pkt->md.ct_orig_tuple.ipv4; >> } >> - } >> - >> - const struct ipf_frag *frag_0 = &rp->list->frag_list[0]; >> - void *l4_frag = dp_packet_l4(frag_0->pkt); >> - void *l4_reass = dp_packet_l4(pkt); >> - memcpy(l4_frag, l4_reass, dp_packet_l4_size(frag_0->pkt)); >> - >> - if (v6) { >> - struct ovs_16aligned_ip6_hdr *l3_frag >> - = dp_packet_l3(frag_0->pkt); >> - struct ovs_16aligned_ip6_hdr *l3_reass = >> dp_packet_l3(pkt); >> - l3_frag->ip6_src = l3_reass->ip6_src; >> - l3_frag->ip6_dst = l3_reass->ip6_dst; >> - } else { >> - struct ip_header *l3_frag = dp_packet_l3(frag_0->pkt); >> - struct ip_header *l3_reass = dp_packet_l3(pkt); >> - if (!dp_packet_hwol_is_ipv4(frag_0->pkt)) { >> - ovs_be32 reass_ip = >> - get_16aligned_be32(&l3_reass->ip_src); >> - ovs_be32 frag_ip = >> - get_16aligned_be32(&l3_frag->ip_src); >> - >> - l3_frag->ip_csum = recalc_csum32(l3_frag->ip_csum, >> - frag_ip, reass_ip); >> - reass_ip = get_16aligned_be32(&l3_reass->ip_dst); >> - frag_ip = get_16aligned_be32(&l3_frag->ip_dst); >> - l3_frag->ip_csum = recalc_csum32(l3_frag->ip_csum, >> - frag_ip, reass_ip); >> + if (v6) { >> + struct ovs_16aligned_ip6_hdr *l3_frag >> + = dp_packet_l3(frag_i->pkt); >> + struct ovs_16aligned_ip6_hdr *l3_reass >> + = dp_packet_l3(pkt); >> + l3_frag->ip6_src = l3_reass->ip6_src; >> + l3_frag->ip6_dst = l3_reass->ip6_dst; >> + } else { >> + struct ip_header *l3_frag = >> dp_packet_l3(frag_i->pkt); >> + struct ip_header *l3_reass = dp_packet_l3(pkt); >> + if (!dp_packet_hwol_is_ipv4(frag_i->pkt)) { >> + ovs_be32 reass_ip = >> + get_16aligned_be32(&l3_reass->ip_src); >> + ovs_be32 frag_ip = >> + get_16aligned_be32(&l3_frag->ip_src); >> + >> + l3_frag->ip_csum = >> recalc_csum32(l3_frag->ip_csum, >> + frag_ip, >> + reass_ip); >> + reass_ip = >> get_16aligned_be32(&l3_reass->ip_dst); >> + frag_ip = get_16aligned_be32(&l3_frag->ip_dst); >> + l3_frag->ip_csum = >> recalc_csum32(l3_frag->ip_csum, >> + frag_ip, >> + reass_ip); >> + } >> + >> + l3_frag->ip_src = l3_reass->ip_src; >> + l3_frag->ip_dst = l3_reass->ip_dst; >> } >> - >> - l3_frag->ip_src = l3_reass->ip_src; >> - l3_frag->ip_dst = l3_reass->ip_dst; >> } >> >> ipf_completed_list_add(&ipf->frag_complete_list, rp->list); > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev