Message ID | CAK+XE==8qYHmHorSKi9Qthb33nW9RoASONQ9RNoByZo6ugW2wg@mail.gmail.com |
---|---|
State | Not Applicable |
Headers | show |
On Wed, Dec 28, 2016 at 09:37:30PM +0000, John Hurley wrote: > Fix for a bug when sending a NATed packet to helper function in kernels > <4.6. > > Setting CHECKSUM_PARTIAL flag means packets could have L4 checksum > corrupted in > > datapath.c/queue_userspace_packet(). > > Giving the packet an skb_dst allows the kernel to correct the checksum if > packet > mangling happens in Conntrack/NAT helpers. > > Signed-off-by: John Hurley <john.hurley@netronome.com> I'm not the right person to review this but I did notice that the patch is wordwrapped and otherwise space-damaged.
diff --git a/datapath/conntrack.c b/datapath/conntrack.c index d942884..18db41b 100644 --- a/datapath/conntrack.c +++ b/datapath/conntrack.c @@ -314,6 +314,10 @@ static int ovs_ct_helper(struct sk_buff *skb, u16 proto) u8 nexthdr; int err; +#if LINUX_VERSION_CODE < KERNEL_VERSION(4,6,0) + struct rtable *rt = NULL; +#endif + ct = nf_ct_get(skb, &ctinfo); if (!ct || ctinfo == IP_CT_RELATED_REPLY)
Fix for a bug when sending a NATed packet to helper function in kernels <4.6. Setting CHECKSUM_PARTIAL flag means packets could have L4 checksum corrupted in datapath.c/queue_userspace_packet(). Giving the packet an skb_dst allows the kernel to correct the checksum if packet mangling happens in Conntrack/NAT helpers. Signed-off-by: John Hurley <john.hurley@netronome.com> --- return NF_ACCEPT; @@ -352,43 +356,29 @@ 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 set allows checksum calculations + * to be 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; - } + rt = kmalloc(sizeof(struct rtable), GFP_KERNEL); + rt->rt_flags = RTCF_LOCAL; + skb_dst_set(skb, (struct dst_entry *)rt); } #endif + err = helper->help(skb, protoff, ct, ctinfo); if (err != NF_ACCEPT) return err; +#if LINUX_VERSION_CODE < KERNEL_VERSION(4,6,0) + if (rt) { + skb_dst_set(skb, NULL); + kfree(rt); + } +#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. --