Message ID | 4BED92AF.50704@trash.net |
---|---|
State | Not Applicable, archived |
Delegated to: | David Miller |
Headers | show |
On Fri, May 14, 2010 at 08:13:03PM +0200, Patrick McHardy wrote: > Your patch is based on an old version, the current version also > supports TCP. I'll commit this patch to my tree after some testing. Thanks! > diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c > index b20f427..45750cc 100644 > +++ b/net/netfilter/nf_conntrack_sip.c > @@ -1393,10 +1393,8 @@ static int sip_help_tcp(struct sk_buff *skb, unsigned int protoff, > > nf_ct_refresh(ct, skb, sip_timeout * HZ); > > - if (skb_is_nonlinear(skb)) { > - pr_debug("Copy of skbuff not supported yet.\n"); > + if (unlikely(skb_linearize(skb))) > return NF_ACCEPT; > - } Should this be NF_DROP? As I understand it skb_linearize only failes if it runs out of memory, which probably means dropping is OK. But passing a packet that might need rewriting could be harmful.. Jason -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday 2010-05-14 20:13, Patrick McHardy wrote: >Jason Gunthorpe wrote: >> At least the XEN net front driver always produces non linear skbs, >> so the SIP module does nothing at all when used with that NIC. >> >> Unconditionally linearize the skb.. >> >> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> >> --- >> net/netfilter/nf_conntrack_sip.c | 9 +++------ >> 1 files changed, 3 insertions(+), 6 deletions(-) >> >> Patrick/Jan, thanks.. This is what I wanted to do in the first place, >> but I couldn't convince myself it was safe, as no other nf code does >> this.. > >Your patch is based on an old version, the current version also >supports TCP. I'll commit this patch to my tree after some testing. nf_defrag defragments the packets, but then they're still non-linear? I'm clearly missing something, could somenoe elaborate? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Jason Gunthorpe wrote: > On Fri, May 14, 2010 at 08:13:03PM +0200, Patrick McHardy wrote: >> Your patch is based on an old version, the current version also >> supports TCP. I'll commit this patch to my tree after some testing. > > Thanks! > >> diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c >> index b20f427..45750cc 100644 >> +++ b/net/netfilter/nf_conntrack_sip.c >> @@ -1393,10 +1393,8 @@ static int sip_help_tcp(struct sk_buff *skb, unsigned int protoff, >> >> nf_ct_refresh(ct, skb, sip_timeout * HZ); >> >> - if (skb_is_nonlinear(skb)) { >> - pr_debug("Copy of skbuff not supported yet.\n"); >> + if (unlikely(skb_linearize(skb))) >> return NF_ACCEPT; >> - } > > Should this be NF_DROP? As I understand it skb_linearize only failes > if it runs out of memory, which probably means dropping is OK. But > passing a packet that might need rewriting could be harmful.. We so far also didn't rewrite the packet. But agreed, its a corner case and dropping it is the safer choice. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Jan Engelhardt wrote: > On Friday 2010-05-14 20:13, Patrick McHardy wrote: >> Jason Gunthorpe wrote: >>> At least the XEN net front driver always produces non linear skbs, >>> so the SIP module does nothing at all when used with that NIC. >>> >>> Unconditionally linearize the skb.. >>> >>> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> >>> --- >>> net/netfilter/nf_conntrack_sip.c | 9 +++------ >>> 1 files changed, 3 insertions(+), 6 deletions(-) >>> >>> Patrick/Jan, thanks.. This is what I wanted to do in the first place, >>> but I couldn't convince myself it was safe, as no other nf code does >>> this.. >> Your patch is based on an old version, the current version also >> supports TCP. I'll commit this patch to my tree after some testing. > > nf_defrag defragments the packets, but then they're still non-linear? > I'm clearly missing something, could somenoe elaborate? We're talking about packets with non-linear data, which is unrelated to fragments. Reassembled fragments are non-linear as well though. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, May 14, 2010 at 08:42:43PM +0200, Patrick McHardy wrote: > > Should this be NF_DROP? As I understand it skb_linearize only failes > > if it runs out of memory, which probably means dropping is OK. But > > passing a packet that might need rewriting could be harmful.. > > We so far also didn't rewrite the packet. But agreed, its > a corner case and dropping it is the safer choice. I was just thinking that, say, a request goes out, gets rewritten but the reply comes back and does not get rewritten = bad. Better to drop. Looks OK to me.. Jason -- To unsubscribe from this list: send the line "unsubscribe netdev" 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/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c index b20f427..45750cc 100644 --- a/net/netfilter/nf_conntrack_sip.c +++ b/net/netfilter/nf_conntrack_sip.c @@ -1393,10 +1393,8 @@ static int sip_help_tcp(struct sk_buff *skb, unsigned int protoff, nf_ct_refresh(ct, skb, sip_timeout * HZ); - if (skb_is_nonlinear(skb)) { - pr_debug("Copy of skbuff not supported yet.\n"); + if (unlikely(skb_linearize(skb))) return NF_ACCEPT; - } dptr = skb->data + dataoff; datalen = skb->len - dataoff; @@ -1455,10 +1453,8 @@ static int sip_help_udp(struct sk_buff *skb, unsigned int protoff, nf_ct_refresh(ct, skb, sip_timeout * HZ); - if (skb_is_nonlinear(skb)) { - pr_debug("Copy of skbuff not supported yet.\n"); + if (unlikely(skb_linearize(skb))) return NF_ACCEPT; - } dptr = skb->data + dataoff; datalen = skb->len - dataoff;