sorry, updated patch.....
--------------------------------
Fix for a bug when sending a NATed packet to helper function in kernels
<4.6.
Setting CHECKSUM_PARTIAL flag can lead to L4 checksum corruption.
Corruption can occur in datapath.c/queue_userspace_packet().
Giving the packet an skb_dst allows the kernel to correct the checksum.
Verified with packets mangled by Conntrack/NAT helpers.
Signed-off-by: John Hurley <john.hur...@netronome.com>
---
diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index d942884..fef67ba 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)
return NF_ACCEPT;
@@ -352,43 +356,28 @@ 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 mod
+ * 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.
--
On Wed, Dec 28, 2016 at 10:08 PM, Ben Pfaff <b...@ovn.org> wrote:
> 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.hur...@netronome.com>
>
> I'm not the right person to review this but I did notice that the patch
> is wordwrapped and otherwise space-damaged.
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
