Re: Syncppp

[Bob Dunlop <Bob.Dunlop@farsite.co.uk>]

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/