Message ID | CANr6G5xW7m2SXFiU22pFVD6uvDj5ymX=jZkRJk2vMYQoJykvLw@mail.gmail.com |
---|---|
State | RFC |
Delegated to: | Pablo Neira |
Headers | show |
Joe Stringer <joestringer@nicira.com> wrote: > > Good point. No, I don't. Any suggestions? > > I can try to just re-target -nf tree (sans patch #2). Pablo? > > The smallest change seems to be adding the nf_ct_frag6_consume_orig() > call to OVS, plus the morph logic from patch 3. Alternatively if Pablo > is fine with having the series re-targeted, then that sounds > reasonable to me too. Pablo, your call. I would suggest to re-target patches #1 and #3 to nf tree, I can do this, just let me know. Alternative is to just add the nf_ct_frag6_consume_orig call to openvswitch and handle that via net tree. I can then wait for that change to pop up in nf-next and just resend this series (which will then undo that change). Let me know, thanks! > > ipv4 side seems broken as well (ip_defrag frees skb on errors other than > > -EINPROGRESS, so it looks like we will double-free in > > do_execute_actions) > > Oh dear. Thanks for the report. I propose wrapping the ip_defrag() > with an skb_get()/skb_consume() as this seems to require the least > invasive changes: > > diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c > index a5ec34f8502f..0d2d24c99fd5 100644 > --- a/net/openvswitch/conntrack.c > +++ b/net/openvswitch/conntrack.c > @@ -303,7 +303,11 @@ static int handle_fragments(struct net *net, > struct sw_flow_key *key, > int err; > > memset(IPCB(skb), 0, sizeof(struct inet_skb_parm)); > + > + skb_get(skb); > err = ip_defrag(skb, user); > + if (!err || err == -EINPROGRESS) > + consume_skb(skb); > if (err) > return err; Indeed, that seems like the least invasive change. Feel free to submit this to -net, there is no dependency on any of the other changes. -- 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
On 20 October 2015 at 13:53, Florian Westphal <fw@strlen.de> wrote: > Feel free to submit this to -net, there is no dependency on any of the > other changes. OK, I'll follow up on that patch. -- 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
On Tue, Oct 20, 2015 at 10:53:07PM +0200, Florian Westphal wrote: > Joe Stringer <joestringer@nicira.com> wrote: > > > Good point. No, I don't. Any suggestions? > > > I can try to just re-target -nf tree (sans patch #2). Pablo? > > > > The smallest change seems to be adding the nf_ct_frag6_consume_orig() > > call to OVS, plus the morph logic from patch 3. Alternatively if Pablo > > is fine with having the series re-targeted, then that sounds > > reasonable to me too. > > Pablo, your call. > > I would suggest to re-target patches #1 and #3 to nf tree, I can do > this, just let me know. It's fairly late, we're on -rc6 so I don't think it's a good idea to submit a large rework to -nf at this stage. > Alternative is to just add the nf_ct_frag6_consume_orig call to > openvswitch and handle that via net tree. > > I can then wait for that change to pop up in nf-next and just resend > this series (which will then undo that change). I'd rather get things fixes for the existing code. This would also allow simple passing back to -stable, then we can move forward discuss and review your rework with sufficient time. Let me know, thanks! -- 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
Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > I can then wait for that change to pop up in nf-next and just resend > > this series (which will then undo that change). > > I'd rather get things fixes for the existing code. This would also > allow simple passing back to -stable, then we can move forward discuss > and review your rework with sufficient time. Joe, could you take care of this and submit a OVS fix to net tree? (just add that call to nf_ct_frag6_consume_orig and take the morph change directly into the OVS callpath) I will then resubmit all of this at some later point. Thanks. -- 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
On 21 October 2015 at 07:50, Florian Westphal <fw@strlen.de> wrote: > Pablo Neira Ayuso <pablo@netfilter.org> wrote: >> > I can then wait for that change to pop up in nf-next and just resend >> > this series (which will then undo that change). >> >> I'd rather get things fixes for the existing code. This would also >> allow simple passing back to -stable, then we can move forward discuss >> and review your rework with sufficient time. > > Joe, could you take care of this and submit a OVS fix to net tree? > > (just add that call to nf_ct_frag6_consume_orig and take > the morph change directly into the OVS callpath) > > I will then resubmit all of this at some later point. > > Thanks. Sure thing. -- 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
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c index a5ec34f8502f..0d2d24c99fd5 100644 --- a/net/openvswitch/conntrack.c +++ b/net/openvswitch/conntrack.c @@ -303,7 +303,11 @@ static int handle_fragments(struct net *net, struct sw_flow_key *key, int err; memset(IPCB(skb), 0, sizeof(struct inet_skb_parm)); + + skb_get(skb); err = ip_defrag(skb, user); + if (!err || err == -EINPROGRESS) + consume_skb(skb); if (err) return err;