[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