On Tue, Nov 09, 2010 at 12:04:44AM +0100, Hans Schillstrom wrote:
> On Monday, November 08, 2010 23:23:54 Simon Horman wrote:
> > On Mon, Nov 08, 2010 at 11:48:37PM +0200, Julian Anastasov wrote:
> > >
> > > Hello,
> > >
> > > On Mon, 8 Nov 2010, 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
> > > >
> > > >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;
> > >
> > > Such copy already exists: skb_linearize(). If you
> > > are lucky it does not copy data. But for your case the
> > > copy should happen. In ip_vs_ftp we even use skb_make_writable
> > > because we change the payload.
> >
> > Good point.
> >
> > > May be ip_vs_sip_fill_param() should deliver the status
> > > from skb_linearize (-ENOMEM) after the iph.protocol != IPPROTO_UDP
> > > check. ip_vs_conn_fill_param_persist should return this
> > > error to ip_vs_sched_persist. What we need is ip_vs_sched_persist
> > > to have new argument 'int *ignored' just like ip_vs_schedule, so
> > > that we can return *ignored = 1 for the case when
> > > ip_vs_conn_fill_param_persist returns any kind of error.
> > > Even on ip_vs_conn_new failure we should return *ignored = 1.
> > > *ignored = 0 remains only for the case when no dest is
> > > selected.
> >
> > Agreed.
> >
> > Somewhat off topic: the *ignored and *verdict parameters
> > seem to be a horrible way to pass this information around.
> > I think we can use ERR_PTR instead, though I'd have to
> > write the patch to see if that is true or not. If its
> > not perhaps we can have a small structure used as
> > the return value of ip_vs_schedule() and pp->conn_schedule().
> >
> > struct ip_vs_scheduler_status {
> > ip_vs_conn *cp;
> > int flag;
> > }
> >
> > However, I do wonder if the defrag change is stable material.
> > If so, I'll hold off on any re-working ideas.
>
> I have been runing it for two days now with SIP
> and that seems to work.
>
> I can send an update version tomorrow.
Ok, perhaps this patch is a good candidate for stable.
We can incorporate Julians ideas into a patch for next.
And clean-ups can go in after that.
> It's time for a power nap now.
:-)
--
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
