We had the same problem, when I send a series of fragments, only the first piece of the packet is NATed. We reviewed and tested the patch, it do solve the problem, and we think there are not any other bugs in it. Reviewed-by: wangze <[email protected]>
wenxu <[email protected]> 于2021年8月6日周五 下午4:38写道： > > > 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 <[email protected]> > Date: 2021-07-09 04:11:12 > To: [email protected] > Cc: [email protected],[email protected] > Subject: Re: [ovs-dev] [PATCH v2] ipf: fix only nat the first fragment in the > reass process>[email protected] writes: > > > >> From: wenxu <[email protected]> > >> > >> 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 <[email protected]> > >> Co-authored-by: luke.li <[email protected]> > >> Signed-off-by: luke.li <[email protected]> > >> --- > > > >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 > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
