[Openswan Users] xl2tpd seems to cause packets to get mangled or disappear

[Ray Overland]

Thx for the feedback Paul.

I have discovered 3 problems in call.c in xl2tpd.

I gave up on l2tpd after discovering that the echo patch wasn't included in the l2tpd in 
the 3.4 snapgear distro. I patched it by hand, since neither of Jacco's patches worked, 
but other problems occured so I went back to xl2tpd.

I was going to do a diff, but the changes are so trivial that I'll just identify them for 
you. Please its easier to talk to and understand if I do it this way.

First, a change was made to call.c from l2tpd from xl2tpd. This code:

>         if (res < 1)
>         {
>             if (res == 0)
>             {
>                 /*
>                    * Hmm..  Nothing to read.  It happens
>                  */
> 		pos=0;
> 		max=0;
>                 return 0;
>             }
>             else if ((errno == EIO) || (errno == EINTR) || (errno == EAGAIN))
>             {
> 
>                 /*
>                    * Oops, we were interrupted!
>                    * Or, we ran out of data too soon
>                    * anyway, we discared whatever it is we
>                    * have
>                  */
> 		pos=0;
> 		max=0;
>                 return 0;
>             }

The
pos=0
max=0

in both logic conditions above can cause a problem. The buffer can actually contain valid 
data which gets thrown away if the zeroing occurs. l2tpd does not zero these parameters 
for these conditions.

Second, the code:

>             if (convert)
>             {
>                 if (buf->len == 0) {
> 		    /* if the buffer is empty, then we have the beginning
> 		     * of a packet, not the end
> 		     */
>                     break;
> 		}
> 		
>                 /* must be the end, drop the FCS */
>                 buf->len -= 2;
>             }
>             else

causes occasional disconnects of the connection during data transfer. It happens a lot 
more if you change the pppd mtu/mru. What happens is the data size is 1, causing a 
negative number to be returned, disconnecting the client.

Thirdly, and most trivially, is the code:

> 	    /* fall through */
>         default:
>             if (convert)
>                 c ^= escape;
>             escape = 0;
>             if (buf->len < buf->maxlen)
>             {
>                 *p = c;
>                 p++;
>                 buf->len++;
>                 break;
>             };

has a semicolon at the closing brace. I don't know if this could actually cause a problem 
(I can't see how it could) but I thought I'd make a clean sweep of it.

I've modified my code with these changes and have eliminated all disconnects at all 
mtu/mru sizes. I still get all kinds of dropped packets, but I have decided to live with 
it. I have some suspicions about this problem, but I am not able to follow up on a 
diagnoses at this time, as I will be working on stabilizing mad-wifi in our product 
starting tomorrow. I may return to it at a later time though.

Just as a side note, I managed to increase the throughput by 5%-10% by optimizing 
read_packet in call.c. This is low hanging fruit given that every single byte is processed 
on the ppp device by this function. I did not actually put this in the product build 
though, I wanted to keep the deltas small. Inline assembly would be an excellent option here.

Hope that you find my input useful.

Regards,
Ray


Paul Wouters wrote:
> On Thu, 30 Aug 2007, Ray Overland wrote:
> 
>> The fact that is "mostly works" offers a me a great deal of optimism, but I
>> was hoping that someone might have been able to point me to an application
>> configuration option or build option that could be used to get this working.
> 
> You will have to collect data and trace the packets to see what is going wrong.
> That is difficult to do if it only happens under load unfortunately.
> 
>> In a different thread, I am also trying to get the combination of l2tpd 0.70
>> working with Openswan 2.4.9, and I just finished getting rp-l2tp to cross
>> compile, but the rpl2tpd.conf is causing some confusion.
> 
> There are many bugs fixed in xl2tpd versus l2tpd. Check the xl2tpd changelog.
> I wouldn't bother with l2tpd - it's just too broken.
> 
> Paul
> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: roverland.vcf
Type: text/x-vcard
Size: 341 bytes
Desc: not available
Url : http://lists.openswan.org/pipermail/users/attachments/20070910/93093d08/attachment.vcf