[PATCH] netfilter: ipv4: defrag: set local_df flag on defragmented skb

[PATCH] netfilter: ipv4: defrag: set local_df flag on defragmented skb

[Florian Westphal]

else we may fail to forward skb even if original fragments do fit
outgoing link mtu:

1. remote sends 2k packets in two 1000 byte frags, DF set
2. we want to forward but only see '2k > mtu and DF set'
3. we then send icmp error saying that outgoing link is 1500

But original sender never sent a packet that would not fit
the outgoing link.

Setting local_df makes outgoing path test size vs.
IPCB(skb)->frag_max_size, so we will still send the correct
error in case the largest original size did not fit
outgoing link mtu.

Reported-by: Maxime Bizon <mbizon@freebox.fr>
Suggested-by: Maxime Bizon <mbizon@freebox.fr>
Fixes: 5f2d04f1f9 (ipv4: fix path MTU discovery with connection tracking)
Signed-off-by: Florian Westphal <fw@strlen.de>
---
diff --git a/net/ipv4/netfilter/nf_defrag_ipv4.c b/net/ipv4/netfilter/nf_defrag_ipv4.c
index 12e13bd..f40f321 100644
--- a/net/ipv4/netfilter/nf_defrag_ipv4.c
+++ b/net/ipv4/netfilter/nf_defrag_ipv4.c
@@ -22,7 +22,6 @@
 #endif
 #include <net/netfilter/nf_conntrack_zones.h>
 
-/* Returns new sk_buff, or NULL */
 static int nf_ct_ipv4_gather_frags(struct sk_buff *skb, u_int32_t user)
 {
 	int err;
@@ -33,8 +32,10 @@ static int nf_ct_ipv4_gather_frags(struct sk_buff *skb, u_int32_t user)
 	err = ip_defrag(skb, user);
 	local_bh_enable();
 
-	if (!err)
+	if (!err) {
 		ip_send_check(ip_hdr(skb));
+		skb->local_df = 1;
+	}
 
 	return err;
 }
-- 
1.8.1.5

Re: [PATCH] netfilter: ipv4: defrag: set local_df flag on defragmented skb

[Maciej Żenczykowski]

Are you deleting the comment because it is incorrect?

On Fri, May 2, 2014 at 6:32 AM, Florian Westphal <fw@strlen.de> wrote:
> else we may fail to forward skb even if original fragments do fit
> outgoing link mtu:
>
> 1. remote sends 2k packets in two 1000 byte frags, DF set
> 2. we want to forward but only see '2k > mtu and DF set'
> 3. we then send icmp error saying that outgoing link is 1500
>
> But original sender never sent a packet that would not fit
> the outgoing link.
>
> Setting local_df makes outgoing path test size vs.
> IPCB(skb)->frag_max_size, so we will still send the correct
> error in case the largest original size did not fit
> outgoing link mtu.
>
> Reported-by: Maxime Bizon <mbizon@freebox.fr>
> Suggested-by: Maxime Bizon <mbizon@freebox.fr>
> Fixes: 5f2d04f1f9 (ipv4: fix path MTU discovery with connection tracking)
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> diff --git a/net/ipv4/netfilter/nf_defrag_ipv4.c b/net/ipv4/netfilter/nf_defrag_ipv4.c
> index 12e13bd..f40f321 100644
> --- a/net/ipv4/netfilter/nf_defrag_ipv4.c
> +++ b/net/ipv4/netfilter/nf_defrag_ipv4.c
> @@ -22,7 +22,6 @@
>  #endif
>  #include <net/netfilter/nf_conntrack_zones.h>
>
> -/* Returns new sk_buff, or NULL */
>  static int nf_ct_ipv4_gather_frags(struct sk_buff *skb, u_int32_t user)
>  {
>         int err;
> @@ -33,8 +32,10 @@ static int nf_ct_ipv4_gather_frags(struct sk_buff *skb, u_int32_t user)
>         err = ip_defrag(skb, user);
>         local_bh_enable();
>
> -       if (!err)
> +       if (!err) {
>                 ip_send_check(ip_hdr(skb));
> +               skb->local_df = 1;
> +       }
>
>         return err;
>  }
> --
> 1.8.1.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] netfilter: ipv4: defrag: set local_df flag on defragmented skb

[Florian Westphal]

Maciej Żenczykowski <zenczykowski@gmail.com> wrote:
> Are you deleting the comment because it is incorrect?

Yes.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] netfilter: ipv4: defrag: set local_df flag on defragmented skb

[Pablo Neira Ayuso]

On Fri, May 02, 2014 at 03:32:16PM +0200, Florian Westphal wrote:
> else we may fail to forward skb even if original fragments do fit
> outgoing link mtu:
> 
> 1. remote sends 2k packets in two 1000 byte frags, DF set
> 2. we want to forward but only see '2k > mtu and DF set'
> 3. we then send icmp error saying that outgoing link is 1500
> 
> But original sender never sent a packet that would not fit
> the outgoing link.
> 
> Setting local_df makes outgoing path test size vs.
> IPCB(skb)->frag_max_size, so we will still send the correct
> error in case the largest original size did not fit
> outgoing link mtu.

Thanks Florian for picking up this issue posted in netdev and cooking
a patch for it. Enqueued to the nf tree.