Skip to content

Commit 3d47fa4

Browse files
jahurleyJarno Rajahalme
authored andcommitted
datapath: Ensure correct L4 checksum with NAT helpers.
Setting the CHECKSUM_PARTIAL flag before sending to helper mods was missing the checksum update call ('csum_*_magic()'), which caused checksum failures with kernels <4.6. This can mean that the L4 checksum is incorrect when the packet egresses the system. Rather than adding the missing (IP version dependent) calls, give the packet a temp skb_dst with RTCF_LOCAL flag not set, which ensures the skb is properly changed to CHECKSUM_PARTIAL 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 <john.hurley@netronome.com> Acked-by: Jarno Rajahalme <jarno@ovn.org>
1 parent 0f6a066 commit 3d47fa4

1 file changed

Lines changed: 17 additions & 30 deletions

File tree

datapath/conntrack.c

Lines changed: 17 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,11 @@ static int ovs_ct_helper(struct sk_buff *skb, u16 proto)
314314
u8 nexthdr;
315315
int err;
316316

317+
#if LINUX_VERSION_CODE < KERNEL_VERSION(4,6,0)
318+
bool dst_set = false;
319+
struct rtable rt = { .rt_flags = 0 };
320+
#endif
321+
317322
ct = nf_ct_get(skb, &ctinfo);
318323
if (!ct || ctinfo == IP_CT_RELATED_REPLY)
319324
return NF_ACCEPT;
@@ -352,43 +357,25 @@ static int ovs_ct_helper(struct sk_buff *skb, u16 proto)
352357
#if LINUX_VERSION_CODE < KERNEL_VERSION(4,6,0)
353358
/* Linux 4.5 and older depend on skb_dst being set when recalculating
354359
* checksums after NAT helper has mangled TCP or UDP packet payload.
355-
* This dependency is avoided when skb is CHECKSUM_PARTIAL or when UDP
356-
* has no checksum.
357-
*
358-
* The dependency is not triggered when the main NAT code updates
359-
* checksums after translating the IP header (address, port), so this
360-
* fix only needs to be executed on packets that are both being NATted
361-
* and that have a helper assigned.
360+
* skb_dst is cast to a rtable struct and the flags examined.
361+
* Forcing these flags to have RTCF_LOCAL not set ensures checksum mod
362+
* is carried out in the same way as kernel versions > 4.5
362363
*/
363-
if (ct->status & IPS_NAT_MASK && skb->ip_summed != CHECKSUM_PARTIAL) {
364-
u8 ipproto = (proto == NFPROTO_IPV4)
365-
? ip_hdr(skb)->protocol : nexthdr;
366-
u16 offset = 0;
367-
368-
switch (ipproto) {
369-
case IPPROTO_TCP:
370-
offset = offsetof(struct tcphdr, check);
371-
break;
372-
case IPPROTO_UDP:
373-
/* Skip if no csum. */
374-
if (udp_hdr(skb)->check)
375-
offset = offsetof(struct udphdr, check);
376-
break;
377-
}
378-
if (offset) {
379-
if (unlikely(!pskb_may_pull(skb, protoff + offset + 2)))
380-
return NF_DROP;
381-
382-
skb->csum_start = skb_headroom(skb) + protoff;
383-
skb->csum_offset = offset;
384-
skb->ip_summed = CHECKSUM_PARTIAL;
385-
}
364+
if (ct->status & IPS_NAT_MASK && skb->ip_summed != CHECKSUM_PARTIAL
365+
&& !skb_dst(skb)) {
366+
dst_set = true;
367+
skb_dst_set(skb, &rt.dst);
386368
}
387369
#endif
388370
err = helper->help(skb, protoff, ct, ctinfo);
389371
if (err != NF_ACCEPT)
390372
return err;
391373

374+
#if LINUX_VERSION_CODE < KERNEL_VERSION(4,6,0)
375+
if (dst_set)
376+
skb_dst_set(skb, NULL);
377+
#endif
378+
392379
/* Adjust seqs after helper. This is needed due to some helpers (e.g.,
393380
* FTP with NAT) adusting the TCP payload size when mangling IP
394381
* addresses and/or port numbers in the text-based control connection.

0 commit comments

Comments
 (0)