Syncppp

Syncppp

[Matt Schulkind]

In the 2.2.16 kernel, it seems that the ppp_device structure was changed to
use a pointer to the net device instead of haveing the structure contained
within, and the if_down procedure was changed accordingly to use the sppp_of
macro. But, if I take a look at the 2.4.x kernel sources, it seems only the
first change, the pointer vs. struct change was made, but the if_down
procedure was not changed. I believe this is a bug and the if_down procedure
in the 2.4.x kernel must be changed to match 2.2.16+. Could anyone confirm
this?

Please CC me personally with any replies.

-Matt Schulkind

Re: Syncppp

[Paul Fulghum]

> In the 2.2.16 kernel, it seems that the ppp_device structure was changed to
> use a pointer to the net device instead of haveing the structure contained
> within, and the if_down procedure was changed accordingly to use the sppp_of
> macro. But, if I take a look at the 2.4.x kernel sources, it seems only the
> first change, the pointer vs. struct change was made, but the if_down
> procedure was not changed. I believe this is a bug and the if_down procedure
> in the 2.4.x kernel must be changed to match 2.2.16+. Could anyone confirm
> this?
> -Matt Schulkind

It looks like you are right. The current 2.4 code
appears to scribble into the net_device structure
someplace (yuck) when if_down() is called.

I'm going to change this and test it tomorrow to be
for sure.

Paul Fulghum paulkf@microgate.com
Microgate Corporation www.microgate.com

Re: Syncppp

[Bob Dunlop]

Hi,

On Mon, Aug  6,  Paul Fulghum wrote:
> > In the 2.2.16 kernel, it seems that the ppp_device structure was changed to
> > use a pointer to the net device instead of haveing the structure contained
> > within, and the if_down procedure was changed accordingly to use the sppp_of
> > macro. But, if I take a look at the 2.4.x kernel sources, it seems only the
> > first change, the pointer vs. struct change was made, but the if_down
> > procedure was not changed. I believe this is a bug and the if_down procedure
> > in the 2.4.x kernel must be changed to match 2.2.16+. Could anyone confirm
> > this?
> > -Matt Schulkind
> 
> It looks like you are right. The current 2.4 code
> appears to scribble into the net_device structure
> someplace (yuck) when if_down() is called.

I agree it's a bug.
Well spotted!  How many times have I scrolled past that line?

On Intel (i386) it looks like it clobbers last_rx which is probably harmless,
but it's close to some hairy pointers so who knows on other architectures.
The fact that pp_link_state is not being reset could well explain how I was
getting into that negotiation loop problem earlier in the year.  The loop
fix is still a valid safety however.

> I'm going to change this and test it tomorrow to be
> for sure.

Well I've applied the obvious fix (below) and tested it on Intel with the
Farsync driver and all looks good.  Don't have access to other architectures
for any other testing.


--- linux/drivers/net/wan/syncppp.c.orig	Thu Jun 28 01:10:55 2001
+++ linux/drivers/net/wan/syncppp.c	Tue Aug  7 08:14:43 2001
@@ -156,7 +156,7 @@
 
 static void if_down(struct net_device *dev)
 {
-	struct sppp *sp = &((struct ppp_device *)dev)->sppp;
+	struct sppp *sp = (struct sppp *)sppp_of(dev);
 
 	sp->pp_link_state=SPPP_LINK_DOWN;
 }



-- 
        Bob Dunlop
        FarSite Communications Ltd.
        http://www.farsite.co.uk/

Re: Syncppp

[Paul Fulghum]

> On Intel (i386) it looks like it clobbers last_rx which is probably harmless,
> but it's close to some hairy pointers so who knows on other architectures.
> The fact that pp_link_state is not being reset could well explain how I was
> getting into that negotiation loop problem earlier in the year.  The loop
> fix is still a valid safety however.

Part of the problem is the masochistic construction of
the (ppp_device/net_device/struct sppp/serial context data)
quad. A nasty mess of casts and followed pointers
likely to make your head spin (and your code wrong).

Paul Fulghum paulkf@microgate.com
Microgate Corporation www.microgate.com