Re: [PATCH v2] Re: WAN: new PPP code for generic HDLC

From: Krzysztof Halasa
Date: Wed Apr 23 2008 - 13:02:37 EST


David Miller <davem@xxxxxxxxxxxxx> writes:

> The fact that non-HDLC users of syncppp got left broken

The fact is it isn't broken WRT dev->priv at least (I don't know about
random unrelated breakage).

The drivers:
a) alloc a netdev
b) set dev->priv to some private kmalloced area

It was like that for years, and it worked. The commit in question
changed netdev_priv() and the drivers don't use this function.

netdev_priv() was introduced at some point in time to allow
alloc_netdev() to optionally allocate additional memory for internal
use by the driver. It had nothing to do with dev->priv except "priv"
name. The drivers don't use it passing size 0.

This is probably suboptimal (two+ allocations instead of one), if the
drivers had maintainers with access to hardware I guess it would be
optimized, but it has nothing to do with the regression.

The regression is present in HDLC PPP only, becase HDLC PPP actually
used netdev_priv() (unlike non-HDLC PPP cases) as a means of addressing
the area following net_device struct and not for retrieving dev->priv,
and then the semantics suddenly changed.

> I would have been more than happy if syncppp was retained and fixed
> properly, instead of being abandoned and duplicated in one fell swoop.

I would be happy as well. The problem is that nobody shown any idea
how to do it.

I have offered: I will port any existing sync serial driver to generic
HDLC if I'm sent a free hardware sample. That includes old ISA cards
(I still have an old 2*PII-333 ISA test machine), and porting PC300
T1/E1 code to pc300too.

If some driver/hw has no users I think there is no point in keeping it
on life support here. The same goes for syncppp - if/when the number
of drivers using it drops to zero, we can let it go.
--
Krzysztof Halasa
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/