Messages in this thread Patch in this message | | | Date | Wed, 8 Mar 2000 19:23:05 +0100 | From | Jan Kasprzak <> | Subject | Patch: syncppp - input path rewrite |
| |
Hello syncppp hackers,
I have rewritten the input packet handling of the syncppp layer (as I suggested in my message in this list few weeks ago). Here is the patch - please send me your comments and opinions, and I'll then submit this patch to Linus.
The patch does the following:
- removal of unnecessary prototypes in syncppp.h (not related to the main problem - should be applied anyway).
- removal of redundant accounting (struct sppp.ibytes,obytes,ipkts,opkts).
- rewrite of the input path -- details follow:
In the current kernel the device driver sets the skb->protocol to ETH_P_WAN_PPP, and passes the packet to netif_rx(). The sppp_input() then determines the true protocol of the packet and passes it again to netif_rx(). This means the packet can be seen twice in the tcpdump output etc.
I have tried to handle the problem the same way as the ethernet devices do. I have created the sppp_type_trans(skb,dev) function (similar to eth_type_trans()), which serves for setting up the packet's skb->protocol. It returns zero for unknown protocol or for an error (Cisco-encapsulated packet in PPP mode, etc). The function also sets the skb->dev, the skb->mac.raw, and calls skb_pull() to remove the link-level header.
I have created the new packet_type ETH_P_CHDLC (for Cisco HDLC), and the Cisco HDLC keepalive and address request frames are directed to this protocol (sppp_cisco_input() function).
The old input packet processing looked this way:
skb->protocol = htons(ETH_P_WAN_PPP); skb->dev = dev; skb->mac.raw = skb->data; netif_rx(skb);
and it should be rewritten the following way:
skb->protocol = sppp_type_trans(skb,dev); if (!skb->protocol) dev_kfree_skb /* or dev_kfree_skb_irq */ (skb); else netif_rx(skb);
In fact I have split the old sppp_input() into sppp_type_trans() and new sppp_input()/sppp_cisco_input().
I have patched the drivers which are included in the mainstream kernel as well (cosa.c hostess_sv11.c and sealevel.c). I not sure about the two later drivers -- I don't know whether I should call dev_kfree_skb or dev_kfree_skb_irq in the failure case (i.e whether it is in the interrupt context or not).
Let me know what do you think about it.
-Yenya diff -ur linux-2.3.50/drivers/net/wan/cosa.c linux/drivers/net/wan/cosa.c --- linux-2.3.50/drivers/net/wan/cosa.c Mon Feb 21 19:23:56 2000 +++ linux/drivers/net/wan/cosa.c Wed Mar 8 18:30:30 2000 @@ -736,12 +736,16 @@ chan->stats.rx_frame_errors++; return 0; } - chan->rx_skb->protocol = htons(ETH_P_WAN_PPP); - chan->rx_skb->dev = chan->pppdev.dev; - chan->rx_skb->mac.raw = chan->rx_skb->data; + chan->rx_skb->protocol = sppp_type_trans(chan->rx_skb, + chan->pppdev.dev); + if (!chan->rx_skb->protocol) { + dev_kfree_skb_irq(chan->rx_skb); + goto out; + } chan->stats.rx_packets++; chan->stats.rx_bytes += chan->cosa->rxsize; netif_rx(chan->rx_skb); +out: chan->rx_skb = 0; chan->pppdev.dev->trans_start = jiffies; return 0; diff -ur linux-2.3.50/drivers/net/wan/hostess_sv11.c linux/drivers/net/wan/hostess_sv11.c --- linux-2.3.50/drivers/net/wan/hostess_sv11.c Fri Feb 11 02:03:43 2000 +++ linux/drivers/net/wan/hostess_sv11.c Wed Mar 8 18:07:16 2000 @@ -60,13 +60,11 @@ { /* Drop the CRC - its not a good idea to try and negotiate it ;) */ skb_trim(skb, skb->len-2); - skb->protocol=htons(ETH_P_WAN_PPP); - skb->mac.raw=skb->data; - skb->dev=c->netdevice; - /* - * Send it to the PPP layer. We dont have time to process - * it right now. - */ + skb->protocol=sppp_type_trans(skb, c->netdevice); + if (!skb->protocol) { + dev_kfree_skb_irq(skb); + return; + } netif_rx(skb); } diff -ur linux-2.3.50/drivers/net/wan/sealevel.c linux/drivers/net/wan/sealevel.c --- linux-2.3.50/drivers/net/wan/sealevel.c Fri Feb 11 02:03:43 2000 +++ linux/drivers/net/wan/sealevel.c Wed Mar 8 18:07:31 2000 @@ -60,13 +60,11 @@ { /* Drop the CRC - its not a good idea to try and negotiate it ;) */ skb_trim(skb, skb->len-2); - skb->protocol=htons(ETH_P_WAN_PPP); - skb->mac.raw=skb->data; - skb->dev=c->netdevice; - /* - * Send it to the PPP layer. We dont have time to process - * it right now. - */ + skb->protocol=sppp_type_trans(skb, dev); + if (!skb->protocol) { + dev_kfree_skb_irq(skb); + return; + } netif_rx(skb); } diff -ur linux-2.3.50/drivers/net/wan/syncppp.c linux/drivers/net/wan/syncppp.c --- linux-2.3.50/drivers/net/wan/syncppp.c Wed Feb 23 07:38:31 2000 +++ linux/drivers/net/wan/syncppp.c Wed Mar 8 18:06:47 2000 @@ -134,7 +134,10 @@ u8 ident, u16 len, void *data); static void sppp_cisco_send (struct sppp *sp, int type, long par1, long par2); static void sppp_lcp_input (struct sppp *sp, struct sk_buff *m); -static void sppp_cisco_input (struct sppp *sp, struct sk_buff *m); +static int sppp_input (struct sk_buff *skb, struct net_device *dev, + struct packet_type *p); +static int sppp_cisco_input (struct sk_buff *skb, struct net_device *dev, + struct packet_type *p); static void sppp_ipcp_input (struct sppp *sp, struct sk_buff *m); static void sppp_lcp_open (struct sppp *sp); static void sppp_ipcp_open (struct sppp *sp); @@ -144,6 +147,8 @@ static char *sppp_lcp_type_name (u8 type); static char *sppp_ipcp_type_name (u8 type); static void sppp_print_bytes (u8 *p, u16 len); +static void sppp_bad_packet(struct net_device *dev, struct ppp_header *h, + char *msg); static int debug = 0; @@ -182,32 +187,34 @@ } } +static void sppp_bad_packet(struct net_device *dev, struct ppp_header *h, + char *msg) +{ + struct sppp *sp = (struct sppp *)sppp_of(dev); + if (sp->pp_flags & PP_DEBUG) + printk(KERN_DEBUG "%s: %s <0x%x 0x%x 0x%x>\n", + dev->name, msg, h->address, h->control, + ntohs(h->protocol)); +} + /* - * Process the received packet. + * Determine the type of the packet (see eth_type_trans()). */ - -void sppp_input (struct net_device *dev, struct sk_buff *skb) +unsigned short sppp_type_trans(struct sk_buff *skb, struct net_device *dev) { struct ppp_header *h; struct sppp *sp = (struct sppp *)sppp_of(dev); + printk(KERN_INFO "sppp_type_trans is here\n"); skb->dev=dev; skb->mac.raw=skb->data; - - if (dev->flags & IFF_UP) - { - /* Count received bytes, add FCS and one flag */ - sp->ibytes+= skb->len + 3; - sp->ipkts++; - } if (skb->len <= PPP_HEADER_LEN) { /* Too small packet, drop it. */ if (sp->pp_flags & PP_DEBUG) - printk (KERN_DEBUG "%s: input packet is too small, %d bytes\n", + printk(KERN_WARNING "%s: packet too small, only %d bytes.", dev->name, skb->len); -drop: kfree_skb(skb); - return; + return 0; } /* Get PPP header. */ @@ -216,100 +223,104 @@ switch (h->address) { default: /* Invalid PPP packet. */ -invalid: if (sp->pp_flags & PP_DEBUG) - printk (KERN_WARNING "%s: invalid input packet <0x%x 0x%x 0x%x>\n", - dev->name, - h->address, h->control, ntohs (h->protocol)); - goto drop; +invalid: + sppp_bad_packet(dev, h, "invalid input packet"); + return 0; case PPP_ALLSTATIONS: if (h->control != PPP_UI) goto invalid; if (sp->pp_flags & PP_CISCO) { - if (sp->pp_flags & PP_DEBUG) - printk (KERN_WARNING "%s: PPP packet in Cisco mode <0x%x 0x%x 0x%x>\n", - dev->name, - h->address, h->control, ntohs (h->protocol)); - goto drop; + sppp_bad_packet(dev, h, "PPP packet in Cisco mode"); + return 0; } switch (ntohs (h->protocol)) { - default: - if (sp->lcp.state == LCP_STATE_OPENED) - sppp_cp_send (sp, PPP_LCP, LCP_PROTO_REJ, - ++sp->pp_seq, skb->len + 2, - &h->protocol); - if (sp->pp_flags & PP_DEBUG) - printk (KERN_WARNING "%s: invalid input protocol <0x%x 0x%x 0x%x>\n", - dev->name, - h->address, h->control, ntohs (h->protocol)); - goto drop; - case PPP_LCP: - sppp_lcp_input (sp, skb); - kfree_skb(skb); - return; - case PPP_IPCP: - if (sp->lcp.state == LCP_STATE_OPENED) - sppp_ipcp_input (sp, skb); - else - printk(KERN_DEBUG "IPCP when still waiting LCP finish.\n"); - kfree_skb(skb); - return; + default: /* including PPP_LCP and PPP_IPCP */ + return htons(ETH_P_WAN_PPP); +#ifdef CONFIG_INET case PPP_IP: if (sp->ipcp.state == IPCP_STATE_OPENED) { if(sp->pp_flags&PP_DEBUG) printk(KERN_DEBUG "Yow an IP frame.\n"); - skb->protocol=htons(ETH_P_IP); - netif_rx(skb); - return; + return htons(ETH_P_IP); } - break; + sppp_bad_packet(dev, h, "IP frame when IPCP not opened"); + return 0; +#endif #ifdef IPX case PPP_IPX: /* IPX IPXCP not implemented yet */ if (sp->lcp.state == LCP_STATE_OPENED) { - skb->protocol=htons(ETH_P_IPX); - netif_rx(skb); - return; + if(sp->pp_flags&PP_DEBUG) + printk(KERN_DEBUG "Yow an IPX frame.\n"); + return htons(ETH_P_IPX); } - break; + sppp_bad_packet(dev, h, "IPX frame when LCP not opened"); + return 0; #endif } break; case CISCO_MULTICAST: case CISCO_UNICAST: + printk(KERN_INFO "CISCO_*CAST case\n"); /* Don't check the control field here (RFC 1547). */ if (! (sp->pp_flags & PP_CISCO)) { - if (sp->pp_flags & PP_DEBUG) - printk (KERN_WARNING "%s: Cisco packet in PPP mode <0x%x 0x%x 0x%x>\n", - dev->name, - h->address, h->control, ntohs (h->protocol)); - goto drop; + sppp_bad_packet(dev, h, "Cisco packet in PPP mode"); + return 0; } switch (ntohs (h->protocol)) { default: goto invalid; case CISCO_KEEPALIVE: - sppp_cisco_input (sp, skb); - kfree_skb(skb); - return; + return htons(ETH_P_CHDLC); #ifdef CONFIG_INET case ETH_P_IP: - skb->protocol=htons(ETH_P_IP); - netif_rx(skb); - return; + return htons(ETH_P_IP); #endif #ifdef CONFIG_IPX case ETH_P_IPX: - skb->protocol=htons(ETH_P_IPX); - netif_rx(skb); - return; + return htons(ETH_P_IPX); #endif } - break; } - kfree_skb(skb); } -EXPORT_SYMBOL(sppp_input); +EXPORT_SYMBOL(sppp_type_trans); + +/* + * Process the received packet. + */ + +static int sppp_input (struct sk_buff *skb, struct net_device *dev, + struct packet_type *p) +{ + struct ppp_header *h = (struct ppp_header *)skb->mac.raw; + struct sppp *sp = (struct sppp *)sppp_of(dev); + + switch (ntohs (h->protocol)) { + default: + if (sp->lcp.state == LCP_STATE_OPENED) + sppp_cp_send (sp, PPP_LCP, LCP_PROTO_REJ, + ++sp->pp_seq, skb->len + 2, + &h->protocol); + if (sp->pp_flags & PP_DEBUG) + printk (KERN_WARNING "%s: invalid input protocol <0x%x 0x%x 0x%x>\n", + dev->name, + h->address, h->control, ntohs (h->protocol)); + break; + case PPP_LCP: + sppp_lcp_input (sp, skb); + break; + case PPP_IPCP: + if (sp->lcp.state == LCP_STATE_OPENED) + sppp_ipcp_input (sp, skb); + else + if (sp->pp_flags && PP_DEBUG) + printk(KERN_DEBUG "IPCP when still waiting LCP finish.\n"); + break; + } + dev_kfree_skb(skb); + return 0; +} /* * Handle transmit packets. @@ -648,16 +659,17 @@ * Handle incoming Cisco keepalive protocol packets. */ -static void sppp_cisco_input (struct sppp *sp, struct sk_buff *skb) +static int sppp_cisco_input (struct sk_buff *skb, struct net_device *dev, + struct packet_type *p) { struct cisco_packet *h; - struct net_device *dev = sp->pp_if; + struct sppp *sp = (struct sppp *)sppp_of(dev); if (skb->len != CISCO_PACKET_LEN && skb->len != CISCO_BIG_PACKET_LEN) { if (sp->pp_flags & PP_DEBUG) printk (KERN_WARNING "%s: invalid cisco packet length: %d bytes\n", dev->name, skb->len); - return; + goto out; } h = (struct cisco_packet *)skb->data; skb_pull(skb, sizeof(struct cisco_packet*)); @@ -731,6 +743,9 @@ break; } } +out: + dev_kfree_skb(skb); + return 0; } /* @@ -776,7 +791,6 @@ sppp_print_bytes ((u8*) (lh+1), len); printk (">\n"); } - sp->obytes += skb->len; /* Control is high priority so it doesnt get queued behind data */ skb->priority=TC_PRIO_CONTROL; skb->dev = dev; @@ -819,7 +833,6 @@ printk (KERN_WARNING "%s: cisco output: <%xh %xh %xh %xh %xh-%xh>\n", dev->name, ntohl (ch->type), ch->par1, ch->par2, ch->rel, ch->time0, ch->time1); - sp->obytes += skb->len; skb->priority=TC_PRIO_CONTROL; skb->dev = dev; dev_queue_xmit(skb); @@ -1183,6 +1196,7 @@ { struct sppp *sp = (struct sppp*) arg; unsigned long flags; + save_flags(flags); cli(); @@ -1273,36 +1287,32 @@ printk ("-%x", *p++); } -/* - * Protocol glue. This drives the deferred processing mode the poorer - * cards use. - */ - -int sppp_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *p) -{ - sppp_input(dev,skb); - return 0; -} - -EXPORT_SYMBOL(sppp_rcv); - struct packet_type sppp_packet_type= { - 0, + __constant_htons(ETH_P_WAN_PPP), NULL, - sppp_rcv, + sppp_input, NULL, NULL }; +struct packet_type chdlc_packet_type= +{ + __constant_htons(ETH_P_CHDLC), + NULL, + sppp_cisco_input, + NULL, + NULL +}; void sync_ppp_init(void) { printk(KERN_INFO "Cronyx Ltd, Synchronous PPP and CISCO HDLC (c) 1994\n"); printk(KERN_INFO "Linux port (c) 1998 Building Number Three Ltd & Jan \"Yenya\" Kasprzak.\n"); spin_lock_init(&spppq_lock); - sppp_packet_type.type=htons(ETH_P_WAN_PPP); + spppq = NULL; dev_add_pack(&sppp_packet_type); + dev_add_pack(&chdlc_packet_type); } EXPORT_SYMBOL(sync_ppp_init); @@ -1319,6 +1329,7 @@ void cleanup_module(void) { + dev_remove_pack(&chdlc_packet_type); dev_remove_pack(&sppp_packet_type); } diff -ur linux-2.3.50/drivers/net/wan/syncppp.h linux/drivers/net/wan/syncppp.h --- linux-2.3.50/drivers/net/wan/syncppp.h Tue Dec 21 00:07:08 1999 +++ linux/drivers/net/wan/syncppp.h Wed Mar 8 15:14:49 2000 @@ -43,8 +43,6 @@ u32 pp_rseq; /* remote sequence number */ struct slcp lcp; /* LCP params */ struct sipcp ipcp; /* IPCP params */ - u32 ibytes,obytes; /* Bytes in/out */ - u32 ipkts,opkts; /* Packets in/out */ struct timer_list pp_timer; struct net_device *pp_if; }; @@ -75,13 +73,10 @@ #define IPCP_STATE_ACK_SENT 2 /* IPCP state: conf-ack sent */ #define IPCP_STATE_OPENED 3 /* IPCP state: opened */ +unsigned short sppp_type_trans(struct sk_buff *skb, struct net_device *dev); void sppp_attach (struct ppp_device *pd); void sppp_detach (struct net_device *dev); -void sppp_input (struct net_device *dev, struct sk_buff *m); int sppp_do_ioctl (struct net_device *dev, struct ifreq *ifr, int cmd); -struct sk_buff *sppp_dequeue (struct net_device *dev); -int sppp_isempty (struct net_device *dev); -void sppp_flush (struct net_device *dev); int sppp_open (struct net_device *dev); int sppp_reopen (struct net_device *dev); int sppp_close (struct net_device *dev); diff -ur linux-2.3.50/drivers/sound/sound_syms.c linux/drivers/sound/sound_syms.c --- linux-2.3.50/drivers/sound/sound_syms.c Wed Mar 8 09:56:30 2000 +++ linux/drivers/sound/sound_syms.c Wed Mar 8 12:15:49 2000 @@ -8,6 +8,7 @@ * left out exported symbol... fixed */ +#include <linux/config.h> #include <linux/module.h> #include "sound_config.h" #include "sound_calls.h" diff -ur linux-2.3.50/include/linux/if_ether.h linux/include/linux/if_ether.h --- linux-2.3.50/include/linux/if_ether.h Tue Sep 7 19:14:37 1999 +++ linux/include/linux/if_ether.h Fri Mar 3 18:23:22 2000 @@ -76,6 +76,7 @@ #define ETH_P_CONTROL 0x0016 /* Card specific control frames */ #define ETH_P_IRDA 0x0017 /* Linux-IrDA */ #define ETH_P_ECONET 0x0018 /* Acorn Econet */ +#define ETH_P_CHDLC 0x0019 /* Dummy type for Cisco HDLC */ /* * This is an Ethernet frame header. -- \ 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/ /// Its purely bandwidth. If it was 40 instances of Miguel reading web pages flat out over 100baseT you would definitely be right. But its not... (Alan) - 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/
| |