Re: [PATCH 05/19] netfilter: nf_conntrack_ipv6: improve fragmentation handling

Jesper Dangaard Brouer
2012-08-19T19:37:49Z

On Sat, 2012-08-18 at 14:26 +0200, Patrick McHardy wrote:
> On Fri, 17 Aug 2012, Jesper Dangaard Brouer wrote:
>
> > On Thu, 2012-08-09 at 22:08 +0200, kaber@trash.net wrote:
> >> From: Patrick McHardy <kaber@trash.net>
> >>
> >> The IPv6 conntrack fragmentation currently has a couple of shortcomings.
> >> Fragmentes are collected in PREROUTING/OUTPUT, are defragmented, the
> >> defragmented packet is then passed to conntrack, the resulting conntrack
> >> information is attached to each original fragment and the fragments then
> >> continue their way through the stack.
> >>
> >> Helper invocation occurs in the POSTROUTING hook, at which point only
> >> the original fragments are available. The result of this is that
> >> fragmented packets are never passed to helpers.
> >>
> >> This patch improves the situation in the following way:
> >>
> >> - If a reassembled packet belongs to a connection that has a helper
> >> assigned, the reassembled packet is passed through the stack instead
> >> of the original fragments.
> >
> > I'm working on IPv6 fragment handling for IPVS, and are taking advantage
> > of the "replay" by nf_ct_frag6_output() at hook prio -399
> > (NF_IP6_PRI_CONNTRACK_DEFRAG + 1).
> > By making a hook at NF_INET_PRE_ROUTING at prio -99 (NF_IP6_PRI_NAT_DST
> > + 1).
> >
> > I can see that the code path can be changed (with this patch), if a
> > helper is assigned. Then the "replay" starts at prio -199
> > (NF_IP6_PRI_CONNTRACK + 1), I guess I'm safe as I run at -99.
> >
> > I have tested that your patchset works, with my ipvs patches, but would
> > like the trigger the changed code path, to make sure.
> >
> > Could you provide an iptables command/rule, that trigger this code path?
>
> The easiest way is a large ping with the NAT patches also applied,
> in that case we also pass the first packet of a connection through
> the stack reassembled.

So, a fragmented IPv6 ICMPv6 packet, I assume?

Don't I need to load some of the helper modules, or just the
nf_conntrack_ipv6 module, or perhaps only nf_defrag_ipv6 ?

> >> @@ -199,9 +200,13 @@ static unsigned int ipv6_confirm(unsigned int hooknum,
> >> static unsigned int __ipv6_conntrack_in(struct net *net,
> >> unsigned int hooknum,
> >> struct sk_buff *skb,
> >> + const struct net_device *in,
> >> + const struct net_device *out,
> >> int (*okfn)(struct sk_buff *))
> >> {
> >> struct sk_buff *reasm = skb->nfct_reasm;
> >> + struct nf_conn *ct;
> >> + enum ip_conntrack_info ctinfo;
> >>
> >> /* This packet is fragmented and has reassembled packet. */
> >> if (reasm) {
> >> @@ -213,6 +218,20 @@ static unsigned int __ipv6_conntrack_in(struct net *net,
> >> if (ret != NF_ACCEPT)
> >> return ret;
> >> }
> >> +
> >> + /* Conntrack helpers need the entire reassembled packet in the
> >> + * POST_ROUTING hook.
> >> + */
> >> + ct = nf_ct_get(reasm, &ctinfo);
> >> + if (ct != NULL && test_bit(IPS_HELPER_BIT, &ct->status)) {
> >> + nf_conntrack_get_reasm(skb);
> >> + NF_HOOK_THRESH(NFPROTO_IPV6, hooknum, reasm,
> >> + (struct net_device *)in,
> >> + (struct net_device *)out,
> >> + okfn, NF_IP6_PRI_CONNTRACK + 1);
> >
> > Hook prio change to NF_IP6_PRI_CONNTRACK + 1
>
> I didn't get this part, you want to change to PRE_CONNTRACK + 1? What
> about raw and SELinux?

No - I don't want any changes.

I was just pointing out *where* the changes occur in your patch. This is
just a "service" to other email readers, so they can spot the changes
faster, I were referring to.

--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Sr. Network Kernel Developer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer