Messages in this thread | | | Date | Fri, 18 Feb 2000 16:47:58 +0100 | From | Jan Kasprzak <> | Subject | Proposed SyncPPP layer modifications |
| |
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/
| |