[ovs-dev] [PATCH v2] ipf: fix only nat the first fragment in the reass process

wenxu wenxu at ucloud.cn
Fri Jul 9 04:09:07 UTC 2021


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 <aconole at redhat.com>
Date: 2021-07-09 04:11:12
To:  wenxu at ucloud.cn
Cc:  i.maximets at ovn.org,dev at openvswitch.org
Subject: Re: [ovs-dev] [PATCH v2] ipf: fix only nat the first fragment in the reass process>wenxu at ucloud.cn writes:
>
>> From: wenxu <wenxu at 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 <wenxu at ucloud.cn>
>> Co-authored-by: luke.li <luke.li at ucloud.cn>
>> Signed-off-by: luke.li <luke.li at 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);
>






More information about the dev mailing list