On Mon, Nov 08, 2010 at 03:51:56PM +0100, Hans Schillstrom wrote: > Hello > I have been struggling with SIP for a while .... > L7 helpers like sip needs skb defrag > ex virtio only copies the first 128 byte into the skb (incl mac hdr) > in that case Call-Id will never be found. > > There is a skb_find_text() that might be used insead of this, but it requires some changes in ip_vs_pe_sip.c Thanks for tracking that down! > Signed-off-by: Hans Schillstrom <hans.schillstrom@xxxxxxxxxxxx> > > diff --git a/net/netfilter/ipvs/ip_vs_pe.c b/net/netfilter/ipvs/ip_vs_pe.c > index e99f920..c0ac69a 100644 > --- a/net/netfilter/ipvs/ip_vs_pe.c > +++ b/net/netfilter/ipvs/ip_vs_pe.c > @@ -76,6 +72,24 @@ struct ip_vs_pe *ip_vs_pe_getbyname(const char *name) > return pe; > } > > +/* skb defrag for L7 helpers */ > +char *ip_vs_skb_defrag(struct sk_buff *skb, int offset, int len) > +{ > + char *p = kmalloc(skb->len, GFP_ATOMIC); > + if (!p) > + goto err; > + if (skb_copy_bits(skb, offset, p, len)) > + goto err; > + IP_VS_DBG(10, "IPVS defrag: offs:%d len:%d\n", offset, len); > + return p; > + > +err: > + if (p) > + kfree(p); > + return NULL; > +} > +EXPORT_SYMBOL_GPL(ip_vs_skb_defrag); > + > /* Register a pe in the pe list */ > int register_ip_vs_pe(struct ip_vs_pe *pe) > { > diff --git a/net/netfilter/ipvs/ip_vs_pe_sip.c b/net/netfilter/ipvs/ip_vs_pe_sip.c > index b8b4e96..78caa83 100644 > --- a/net/netfilter/ipvs/ip_vs_pe_sip.c > +++ b/net/netfilter/ipvs/ip_vs_pe_sip.c > @@ -71,6 +71,7 @@ ip_vs_sip_fill_param(struct ip_vs_conn_param *p, struct sk_buff *skb) > struct ip_vs_iphdr iph; > unsigned int dataoff, datalen, matchoff, matchlen; > const char *dptr; > + int fr; > > ip_vs_fill_iphdr(p->af, skb_network_header(skb), &iph); > > @@ -85,21 +86,30 @@ ip_vs_sip_fill_param(struct ip_vs_conn_param *p, struct sk_buff *skb) > > dptr = skb->data + dataoff; > datalen = skb->len - dataoff; > - > + fr = 0; > + if( skb_shinfo(skb)->nr_frags ) { >From a style point of view the line above should probably be: if (skb_shinfo(skb)->nr_frags) { > + dptr = ip_vs_skb_defrag(skb, dataoff, datalen); > + if (!dptr) > + return -EINVAL; > + fr = 1; > + } But I wonder if this logic can be rolld into ip_vs_skb_defrag(), perhaps using ERR_PTR() and friends. Then again, what you have may already be at least as clean as that idea. > if (get_callid(dptr, dataoff, datalen, &matchoff, &matchlen)) > - return -EINVAL; > + goto err; > > p->pe_data = kmalloc(matchlen, GFP_ATOMIC); > if (!p->pe_data) > - return -ENOMEM; > + goto err; > > /* N.B: pe_data is only set on success, > * this allows fallback to the default persistence logic on failure > */ > memcpy(p->pe_data, dptr + matchoff, matchlen); > p->pe_data_len = matchlen; > - > return 0; > +err: > + if (fr) > + kfree(dptr); > + return -EINVAL; > } > > static bool ip_vs_sip_ct_match(const struct ip_vs_conn_param *p, > diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h > index a6421e6..08bd547 100644 > --- a/include/net/ip_vs.h > +++ b/include/net/ip_vs.h > @@ -817,6 +817,7 @@ void ip_vs_unbind_pe(struct ip_vs_service *svc); > int register_ip_vs_pe(struct ip_vs_pe *pe); > int unregister_ip_vs_pe(struct ip_vs_pe *pe); > struct ip_vs_pe *ip_vs_pe_getbyname(const char *name); > +extern char *ip_vs_skb_defrag(struct sk_buff *skb, int offset, int len); Personally I'm not a fan of the extern keyword. > static inline void ip_vs_pe_get(const struct ip_vs_pe *pe) > { > > -- > Regards > Hans Schillstrom <hans.schillstrom@xxxxxxxxxxxx> > -- To unsubscribe from this list: send the line "unsubscribe lvs-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html