lkml.org 
[lkml]   [2000]   [Mar]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectPatch: 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/

\
 
 \ /
  Last update: 2005-03-22 13:56    [W:0.583 / U:0.692 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site