Acked-by: Jarno Rajahalme <[email protected]> Pushed to master and branch-2.6 with slightly edited commit message and one indentation fix.
Jarno > On Jan 6, 2017, at 10:02 AM, John Hurley <[email protected]> wrote: > > Setting the CHECKSUM_PARTIAL flag before sending to helper mods > can mean that the kernel code will not modify the first part of > the L4 checksum correctly after changing packet IPs/ports/payload > in kernels <4.6. This can mean that the L4 checksum is incorrect > when the packet egresses the system. > > Giving the packet a temp skb_dst with RTCF_LOCAL flag not set > ensures the 'csum_*_magic' functions are hit in the kernel (if > required) and the modified packet will get the correct checksum > when fully processed. > > This has tested with FTP NAT helpers on kernel version 3.13. > > Signed-off-by: John Hurley <[email protected]> > --- > datapath/conntrack.c | 47 +++++++++++++++++------------------------------ > 1 file changed, 17 insertions(+), 30 deletions(-) > > diff --git a/datapath/conntrack.c b/datapath/conntrack.c > index d942884..6e0bfed 100644 > --- a/datapath/conntrack.c > +++ b/datapath/conntrack.c > @@ -314,6 +314,11 @@ static int ovs_ct_helper(struct sk_buff *skb, u16 proto) > u8 nexthdr; > int err; > > +#if LINUX_VERSION_CODE < KERNEL_VERSION(4,6,0) > + bool dst_set = false; > + struct rtable rt = { .rt_flags = 0 }; > +#endif > + > ct = nf_ct_get(skb, &ctinfo); > if (!ct || ctinfo == IP_CT_RELATED_REPLY) > return NF_ACCEPT; > @@ -352,43 +357,25 @@ static int ovs_ct_helper(struct sk_buff *skb, u16 proto) > #if LINUX_VERSION_CODE < KERNEL_VERSION(4,6,0) > /* Linux 4.5 and older depend on skb_dst being set when recalculating > * checksums after NAT helper has mangled TCP or UDP packet payload. > - * This dependency is avoided when skb is CHECKSUM_PARTIAL or when UDP > - * has no checksum. > - * > - * The dependency is not triggered when the main NAT code updates > - * checksums after translating the IP header (address, port), so this > - * fix only needs to be executed on packets that are both being NATted > - * and that have a helper assigned. > + * skb_dst is cast to a rtable struct and the flags examined. > + * Forcing these flags to have RTCF_LOCAL not set ensures checksum mod > + * is carried out in the same way as kernel versions > 4.5 > */ > - if (ct->status & IPS_NAT_MASK && skb->ip_summed != CHECKSUM_PARTIAL) { > - u8 ipproto = (proto == NFPROTO_IPV4) > - ? ip_hdr(skb)->protocol : nexthdr; > - u16 offset = 0; > - > - switch (ipproto) { > - case IPPROTO_TCP: > - offset = offsetof(struct tcphdr, check); > - break; > - case IPPROTO_UDP: > - /* Skip if no csum. */ > - if (udp_hdr(skb)->check) > - offset = offsetof(struct udphdr, check); > - break; > - } > - if (offset) { > - if (unlikely(!pskb_may_pull(skb, protoff + offset + 2))) > - return NF_DROP; > - > - skb->csum_start = skb_headroom(skb) + protoff; > - skb->csum_offset = offset; > - skb->ip_summed = CHECKSUM_PARTIAL; > - } > + if (ct->status & IPS_NAT_MASK && skb->ip_summed != CHECKSUM_PARTIAL > + && !skb_dst(skb)) { > + dst_set = true; > + skb_dst_set(skb, &rt.dst); > } > #endif > err = helper->help(skb, protoff, ct, ctinfo); > if (err != NF_ACCEPT) > return err; > > +#if LINUX_VERSION_CODE < KERNEL_VERSION(4,6,0) > + if (dst_set) > + skb_dst_set(skb, NULL); > +#endif > + > /* Adjust seqs after helper. This is needed due to some helpers (e.g., > * FTP with NAT) adusting the TCP payload size when mangling IP > * addresses and/or port numbers in the text-based control connection. > -- > 2.7.4 > > _______________________________________________ > 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
