Alan & all syncppp hackers,
I want to propose the following two changes in the implementation
of the syncppp layer in linux/drivers/net/wan/syncppp.[ch]. The syncppp
handling is currently different than the ethernet one (and fddi and TR too),
which causes some code duplications in the drivers and a suboptimal
implementation in syncppp.c. Let me know what do you think about the
following (i have it partly done in my tree):
Proposal 1: implement the equivalent of eth_type_trans(skb, dev) for the
syncppp devices.
Proposal 2: implement the common device allocation & registration code
similar to the init_etherdev() and ether_setup() routines.
Rationale 1: currently the protocol type in the received packet
is set to ETH_P_WAN_PPP, the packet is passed to netif_rx(),
and the sppp_input() routine then changes the skb->type
to the appropriate 3rd-layer protocol (IP or IPX)
and passes the skb to netif_rx() once again. This is wrong and it
caused misinterpretation in tcpdump and ipchains in the past (the packet
is seen twice by the network stack). LCP or Cisco keepalive
packets are handled directly (and thus are seen only once).
I think it will be better to
have the sppp_type_trans(skb, dev) routine for setting up the
skb->proto, skb->mac.raw, and maybe skb->dev fields. It should
immediately decide between the ETH_P_IP, ETH_P_IPX and assign
the ETH_P_WAN_PPP type to the LCP and/or Cisco packets
(XXX or there can be a separate type ETH_P_CISCO for Cisco HDLC).
The skb with the correct type then will be passed to netif_rx(),
and it will end up in the IP, IPX or SyncPPP/Cisco routines,
respectively.
Rationale 2: currently all users of syncppp.c (I know of cosa.c,
hostess_sv11.c and sealevel.c, maybe there are others too -- comX?)
use their own homegrown method of allocating the struct net_device,
the dev->priv, the struct ppp_device and the dev->name. On the other
side there is a clean method used by all ethernet drivers -- the
init_etherdev() and ether_setup() routines.
I propose to implement the init_spppdev(dev, sizeof_priv) routine,
which will allocate the struct net_device (if needed) and all the
other structures, sets up the syncppp device part (struct ppp_device)
and initialize the syncppp layer using sppp_attach() (which seems
to be a good equivalent of ether_setup()).
When we have the common allocation/setup/registration method of
struct net_device for syncppp devices, we can also get rid of
the if_ptr ugliness, and map the struct ppp_device at the beginning
of the dev->priv instead (thus every device using syncppp.c
should have the struct ppp_device at the beginning of its
dev->priv the same way it currently has to have the pointer to this
structure there). This saves a one pointer in dev->priv.
Possible implementation problems:
1: Should the ETH_P_WAN_PPP be reserved for LCP/IPCP frames only
(and add something like ETH_P_CISCO for Cisco HDLC keepalive
and other frames)? Or should I use ETH_P_WAN_PPP type for both
Cisco HDLC service frames and PPP (LCP/IPCP/keepalive) ones?
Alternatively, the sppp_input can be called instead of netif_rx()
by the network driver, but I think it causess too much work to be
done at the (possible) interrupt time.
2: It would be good if the init_spppdev() can use the functions
from the linux/drivers/net/net_init.c. But most of these functions
are static. We can add the init_spppdev() into net_init.c, but this
routine should call sppp_attach, which will need the syncppp.c
to be linked into the kernel (possibly with new CONFIG_SYNCPPP
option, which would be set if CONFIG_COSA or CONFIG_HOSTESS or
CONFIG_SEALEVEL is set).
Sincerely,
-Yenya
-- \ Jan "Yenya" Kasprzak <kas at fi.muni.cz> http://www.fi.muni.cz/~kas/ \\ PGP: finger kas at aisa.fi.muni.cz 0D99A7FB206605D7 8B35FCDE05B18A5E // \\\ Czech Linux Homepage: http://www.linux.cz/ /// /// while (*p++ = *q++) ; -- Dennis M. Ritchie \\\- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Wed Feb 23 2000 - 21:00:21 EST