Mike Pattrick via dev <ovs-dev@openvswitch.org> writes:

> Previously during NAT actions we could calculate a checksum on a packet,
> if this checksum was valid we would mark it as such for offloading. Then
> when modifying the packet we would check if we set the checksum as
> partial. This worked fine for most packets, but fragmented packets can
> not complete their checksum because we process these packets one at a
> time instead of all at once. This breaks NAT of fragmented packets in
> userspace conntrack.
>
> This patch resolves any outstanding checksums on the reassembled
> fragment before adding the fragments to a batch.
>
> Reported-at: https://issues.redhat.com/browse/FDP-1571
> Reported-by: Hekai Wang <hew...@redhat.com>
> Fixes: e36793e11fe8 ("dp-packet: Resolve unknown checksums.")
> Signed-off-by: Mike Pattrick <m...@redhat.com>
> ---

Hi Mike,

the patch looks good to me.

Acked-by: Paolo Valerio <pvale...@redhat.com>

>  lib/ipf.c             |  3 +++
>  tests/ofproto-dpif.at | 51 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 54 insertions(+)
>
> diff --git a/lib/ipf.c b/lib/ipf.c
> index 6670ef5a9..b16797312 100644
> --- a/lib/ipf.c
> +++ b/lib/ipf.c
> @@ -1193,6 +1193,9 @@ ipf_post_execute_reass_pkts(struct ipf *ipf,
>                  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);
> +
> +                /* Complete all L4 checksums before reassembly. */
> +                dp_packet_ol_send_prepare(pkt, 0);
>                  memcpy(l4_frag, l4_reass, dp_packet_l4_size(frag_0->pkt));
>  
>                  for (int i = 0; i <= rp->list->last_inuse_idx; i++) {
> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index 46b1e6707..a0cd4a5ce 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -11665,6 +11665,57 @@ OVS_WAIT_UNTIL([ovs-pcap p1-tx.pcap | grep -q 
> "$packet3"])
>  OVS_VSWITCHD_STOP
>  AT_CLEANUP
>  
> +AT_SETUP([ofproto-dpif - conntrack - fragmentation nat])
> +OVS_VSWITCHD_START
> +
> +add_of_ports --pcap br0 1 2
> +
> +AT_DATA([flows.txt], [dnl
> +table=0,priority=10,ip,in_port=1,udp,action=ct(commit,table=1,nat(dst=1.2.3.4))
> +table=1,priority=10,ip,in_port=1,udp,action=2
> +])
> +
> +AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
> +
> +dnl Set min frag size.
> +AT_CHECK([ovs-appctl dpctl/ipf-set-min-frag v4 400], [], [dnl
> +setting minimum fragment size successful
> +])
> +
> +dnl First UDP fragment.
> +AT_CHECK(dnl
> +[ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=]dnl
> +[c6f94ecb72dbe64c473528c9080045000204317020004011cf55ac100001ac100002a28e15b3]dnl
> +[01fc31d741414141414141414141414141414141414141414141414141414141414141414141]dnl
> +[4141414141414141414141414141414141414141414141414141414141414141414141414141]dnl
> +[4141414141414141414141414141414141414141414141414141414141414141414141414141]dnl
> +[4141414141414141414141414141414141414141414141414141414141414141414141414141]dnl
> +[4141414141414141414141414141414141414141414141414141414141414141414141414141]dnl
> +[4141414141414141414141414141414141414141414141414141414141414141414141414141]dnl
> +[4141414141414141414141414141414141414141414141414141414141414141414141414141]dnl
> +[4141414141414141414141414141414141414141414141414141414141414141414141414141]dnl
> +[4141414141414141414141414141414141414141414141414141414141414141414141414141]dnl
> +[4141414141414141414141414141414141414141414141414141414141414141414141414141]dnl
> +[4141414141414141414141414141414141414141414141414141414141414141414141414141]dnl
> +[4141414141414141414141414141414141414141414141414141414141414141414141414141]dnl
> +[414141414141414141414141414141414141414141414141414141414141414141414141]dnl
> +[ actions=resubmit(,0)"])
> +
> +dnl Second UDP fragment.
> +AT_CHECK(dnl
> +[ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=]dnl
> +[c6f94ecb72dbe64c473528c90800450000203170003e4011f0fbac100001ac10000241414141]dnl
> +[4141414141414141]dnl
> +[ actions=resubmit(,0)"])
> +
> +dnl Validate that both IP headers now have the correct IP address and
> +dnl the UDP header has the correct checksum.
> +OVS_WAIT_UNTIL([ovs-pcap p2-tx.pcap | grep -q 
> "450002043170200040117762ac10000101020304a28e15b301fcd9e3"])
> +OVS_WAIT_UNTIL([ovs-pcap p2-tx.pcap | grep -q 
> "450000203170003e40119908ac10000101020304"])
> +
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> +
>  AT_SETUP([ofproto-dpif - conntrack - ipv6])
>  OVS_VSWITCHD_START
>  
> -- 
> 2.50.1
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to