Hi,
I'll follow up with a patch for generic HDLC PPP code.
Current status of generic HDLC is:
- up to 2.6.22 - working.
- 2.6.23.* and 2.6.24.* - Frame Relay and PPP protocols panic instantly,
breakage caused by a change to netdev code that I overlooked.
- 2.6.25-git - Frame Relay is already fixed, PPP still panics.
The fix to FR was simple (983e23041b28abb113862b2935a85cfb9aab4f5a).
PPP has been using syncppp.c implementation for years, meaning working
as a mid-layer between hardware drivers and syncppp. This situation is
increasingly hard to maintain, and I've decided to write a dedicated
PPP implementation for generic HDLC.
Rationale:
- syncppp assumes no mid-layer between itself and hw drivers - dirty
hacks must be used to work around this
- syncppp doesn't even try to implement PPP correctly, it looks like
it was written to work with another specific implementation and then
left in this state for *teen years
- every protocol (LCP and IPCP) uses different state machine code.
- lack of IPv6 support and adding it is non-trivial.
- I've considered using the generic PPP stack, however it's oriented
towards async tty and requires the pppd daemon - I guess code to
interface to it would be more complicated than the new PPP code.
The PPP state machine is distributed over many functions and this is
extremely hard to maintain or even understand. No wonder it's not very
standard-compliant.
Instead my new implementation has:
- a single state machine code for all control protocols (LCP, IPCP,
IPV6CP, and whatever is needed)
- it's modelled after STD-51
- even more minimalistic than syncppp (no "magic number"), though
adding optional support (if needed) is simple.
- can actually be maintained.
I have positively tested it on i686 and big-endian ARM, against itself
and against syncppp.c. I can't test it against any other device due to
-ENOHW, so I expect some bugs/problems in/with this code, though I hope
it's better than the current brokenness.
I guess it should go into 2.6.25, not sure about "stable" series.
I will appreciate any feedback, review and/or test results.
--
Krzysztof Halasa
New synchronous PPP implementation for generic HDLC.
Signed-off-by: Krzysztof Halasa <[email protected]>
diff --git a/drivers/net/wan/Makefile b/drivers/net/wan/Makefile
index d61fef3..3081683 100644
--- a/drivers/net/wan/Makefile
+++ b/drivers/net/wan/Makefile
@@ -14,7 +14,7 @@ obj-$(CONFIG_HDLC_RAW) += hdlc_raw.o
obj-$(CONFIG_HDLC_RAW_ETH) += hdlc_raw_eth.o
obj-$(CONFIG_HDLC_CISCO) += hdlc_cisco.o
obj-$(CONFIG_HDLC_FR) += hdlc_fr.o
-obj-$(CONFIG_HDLC_PPP) += hdlc_ppp.o syncppp.o
+obj-$(CONFIG_HDLC_PPP) += hdlc_ppp.o
obj-$(CONFIG_HDLC_X25) += hdlc_x25.o
pc300-y := pc300_drv.o
diff --git a/drivers/net/wan/hdlc_ppp.c b/drivers/net/wan/hdlc_ppp.c
index 10396d9..b626aae 100644
--- a/drivers/net/wan/hdlc_ppp.c
+++ b/drivers/net/wan/hdlc_ppp.c
@@ -2,7 +2,7 @@
* Generic HDLC support routines for Linux
* Point-to-point protocol support
*
- * Copyright (C) 1999 - 2006 Krzysztof Halasa <[email protected]>
+ * Copyright (C) 1999 - 2008 Krzysztof Halasa <[email protected]>
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of version 2 of the GNU General Public License
@@ -19,87 +19,629 @@
#include <linux/skbuff.h>
#include <linux/pkt_sched.h>
#include <linux/inetdevice.h>
-#include <linux/lapb.h>
-#include <linux/rtnetlink.h>
#include <linux/hdlc.h>
-#include <net/syncppp.h>
+#include <linux/spinlock.h>
+
+#define DEBUG_CP 0 /* also bytes# to dump */
+#define DEBUG_STATE 0
+#define DEBUG_HARD_HEADER 0
+
+#define HDLC_ADDR_ALLSTATIONS 0xFF
+#define HDLC_CTRL_UI 0x03
+
+#define PID_LCP 0xC021
+#define PID_IP 0x0021
+#define PID_IPCP 0x8021
+#define PID_IPV6 0x0057
+#define PID_IPV6CP 0x8057
+
+enum {IDX_LCP = 0, IDX_IPCP, IDX_IPV6CP, IDX_COUNT};
+enum {CP_CONF_REQ = 1, CP_CONF_ACK, CP_CONF_NAK, CP_CONF_REJ, CP_TERM_REQ,
+ CP_TERM_ACK, CP_CODE_REJ, LCP_PROTO_REJ, LCP_ECHO_REQ, LCP_ECHO_REPLY,
+ LCP_DISC_REQ, CP_CODES};
+#if DEBUG_CP
+static const char *code_names[CP_CODES] = {"0", "ConfReq", "ConfAck",
+ "ConfNak", "ConfRej", "TermReq",
+ "TermAck", "CodeRej", "ProtoRej",
+ "EchoReq", "EchoReply", "Discard"};
+static char debug_buffer[64 + 3 * DEBUG_CP];
+#endif
+
+enum {LCP_OPTION_MRU = 1, LCP_OPTION_ACCM, LCP_OPTION_MAGIC = 5};
+
+struct hdlc_header {
+ u8 address;
+ u8 control;
+ __be16 protocol;
+} __attribute__ ((packed));
+
+struct cp_header {
+ u8 code;
+ u8 id;
+ __be16 len;
+} __attribute__ ((packed));
+
+
+struct proto {
+ struct net_device *dev;
+ struct timer_list timer;
+ unsigned long timeout;
+ u16 pid; /* protocol ID */
+ u8 state;
+ u8 cr_id; /* ID of last Configuration-Request */
+ u8 restart_counter;
+};
-struct ppp_state {
- struct ppp_device pppdev;
- struct ppp_device *syncppp_ptr;
- int (*old_change_mtu)(struct net_device *dev, int new_mtu);
+struct ppp {
+ struct proto protos[IDX_COUNT];
+ spinlock_t lock;
+ unsigned long last_pong;
+ unsigned int req_timeout, cr_retries, term_retries;
+ unsigned int keepalive_interval, keepalive_timeout;
+ u8 seq; /* local sequence number for requests */
+ u8 echo_id; /* ID of last Echo-Request (LCP) */
};
+enum {CLOSED = 0, STOPPED, STOPPING, REQ_SENT, ACK_RECV, ACK_SENT, OPENED,
+ STATES, STATE_MASK = 0xF};
+enum {START = 0, STOP, TO_GOOD, TO_BAD, RCR_GOOD, RCR_BAD, RCA, RCN, RTR, RTA,
+ RUC, RXJ_GOOD, RXJ_BAD, EVENTS};
+enum {INV = 0x10, IRC = 0x20, ZRC = 0x40, SCR = 0x80, SCA = 0x100,
+ SCN = 0x200, STR = 0x400, STA = 0x800, SCJ = 0x1000};
+
+#if DEBUG_STATE
+static const char *state_names[STATES] = {"Closed", "Stopped", "Stopping",
+ "ReqSent", "AckRecv", "AckSent",
+ "Opened"};
+static const char *event_names[EVENTS] = {"Start", "Stop", "TO+", "TO-",
+ "RCR+", "RCR-", "RCA", "RCN",
+ "RTR", "RTA", "RUC", "RXJ+", "RXJ-"};
+#endif
+
+static struct sk_buff_head tx_queue; /* used when holding the spin lock */
+
static int ppp_ioctl(struct net_device *dev, struct ifreq *ifr);
+static inline struct ppp* get_ppp(struct net_device *dev)
+{
+ return (struct ppp *)dev_to_hdlc(dev)->state;
+}
-static inline struct ppp_state* state(hdlc_device *hdlc)
+static inline struct proto* get_proto(struct net_device *dev, u16 pid)
{
- return(struct ppp_state *)(hdlc->state);
+ struct ppp *ppp = get_ppp(dev);
+
+ switch (pid) {
+ case PID_LCP:
+ return &ppp->protos[IDX_LCP];
+ case PID_IPCP:
+ return &ppp->protos[IDX_IPCP];
+ case PID_IPV6CP:
+ return &ppp->protos[IDX_IPV6CP];
+ default:
+ return NULL;
+ }
}
+static inline const char* proto_name(u16 pid)
+{
+ switch (pid) {
+ case PID_LCP:
+ return "LCP";
+ case PID_IPCP:
+ return "IPCP";
+ case PID_IPV6CP:
+ return "IPV6CP";
+ default:
+ return NULL;
+ }
+}
-static int ppp_open(struct net_device *dev)
+static __be16 ppp_type_trans(struct sk_buff *skb, struct net_device *dev)
{
- hdlc_device *hdlc = dev_to_hdlc(dev);
- int (*old_ioctl)(struct net_device *, struct ifreq *, int);
- int result;
+ struct hdlc_header *data = (struct hdlc_header*)skb->data;
- dev->priv = &state(hdlc)->syncppp_ptr;
- state(hdlc)->syncppp_ptr = &state(hdlc)->pppdev;
- state(hdlc)->pppdev.dev = dev;
+ if (skb->len < sizeof(struct hdlc_header))
+ return htons(ETH_P_HDLC);
+ if (data->address != HDLC_ADDR_ALLSTATIONS ||
+ data->control != HDLC_CTRL_UI)
+ return htons(ETH_P_HDLC);
- old_ioctl = dev->do_ioctl;
- state(hdlc)->old_change_mtu = dev->change_mtu;
- sppp_attach(&state(hdlc)->pppdev);
- /* sppp_attach nukes them. We don't need syncppp's ioctl */
- dev->do_ioctl = old_ioctl;
- state(hdlc)->pppdev.sppp.pp_flags &= ~PP_CISCO;
- dev->type = ARPHRD_PPP;
- result = sppp_open(dev);
- if (result) {
- sppp_detach(dev);
- return result;
+ switch (data->protocol) {
+ case __constant_htons(PID_IP):
+ skb_pull(skb, sizeof(struct hdlc_header));
+ return htons(ETH_P_IP);
+
+ case __constant_htons(PID_IPV6):
+ skb_pull(skb, sizeof(struct hdlc_header));
+ return htons(ETH_P_IPV6);
+
+ default:
+ return htons(ETH_P_HDLC);
}
+}
- return 0;
+
+static int ppp_hard_header(struct sk_buff *skb, struct net_device *dev,
+ u16 type, const void *daddr, const void *saddr,
+ unsigned int len)
+{
+ struct hdlc_header *data;
+#if DEBUG_HARD_HEADER
+ printk(KERN_DEBUG "%s: ppp_hard_header() called\n", dev->name);
+#endif
+
+ skb_push(skb, sizeof(struct hdlc_header));
+ data = (struct hdlc_header*)skb->data;
+
+ data->address = HDLC_ADDR_ALLSTATIONS;
+ data->control = HDLC_CTRL_UI;
+ switch (type) {
+ case ETH_P_IP:
+ data->protocol = htons(PID_IP);
+ break;
+ case ETH_P_IPV6:
+ data->protocol = htons(PID_IPV6);
+ break;
+ case PID_LCP:
+ case PID_IPCP:
+ case PID_IPV6CP:
+ data->protocol = htons(type);
+ break;
+ default: /* unknown protocol */
+ data->protocol = 0;
+ }
+ return sizeof(struct hdlc_header);
}
+static void ppp_tx_flush(void)
+{
+ struct sk_buff *skb;
+ while ((skb = skb_dequeue(&tx_queue)) != NULL)
+ dev_queue_xmit(skb);
+}
-static void ppp_close(struct net_device *dev)
+static void ppp_tx_cp(struct net_device *dev, u16 pid, u8 code,
+ u8 id, unsigned int len, const void *data)
{
- hdlc_device *hdlc = dev_to_hdlc(dev);
+ struct sk_buff *skb;
+ struct cp_header *cp;
+ unsigned int magic_len = 0;
+ static u32 magic;
+
+#if DEBUG_CP
+ int i;
+ char *ptr;
+#endif
+
+ if (pid == PID_LCP && (code == LCP_ECHO_REQ || code == LCP_ECHO_REPLY))
+ magic_len = sizeof(magic);
+
+ skb = dev_alloc_skb(sizeof(struct hdlc_header) +
+ sizeof(struct cp_header) + magic_len + len);
+ if (!skb) {
+ printk(KERN_WARNING "%s: out of memory in ppp_tx_cp()\n",
+ dev->name);
+ return;
+ }
+ skb_reserve(skb, sizeof(struct hdlc_header));
+
+ cp = (struct cp_header *)skb_put(skb, sizeof(struct cp_header));
+ cp->code = code;
+ cp->id = id;
+ cp->len = htons(sizeof(struct cp_header) + magic_len + len);
+
+ if (magic_len)
+ memcpy(skb_put(skb, magic_len), &magic, magic_len);
+ if (len)
+ memcpy(skb_put(skb, len), data, len);
+
+#if DEBUG_CP
+ BUG_ON(code >= CP_CODES);
+ ptr = debug_buffer;
+ *ptr = '\x0';
+ for (i = 0; i < min_t(unsigned int, magic_len + len, DEBUG_CP); i++) {
+ sprintf(ptr, " %02X", skb->data[sizeof(struct cp_header) + i]);
+ ptr += strlen(ptr);
+ }
+ printk(KERN_DEBUG "%s: TX %s [%s id 0x%X]%s\n", dev->name,
+ proto_name(pid), code_names[code], id, debug_buffer);
+#endif
- sppp_close(dev);
- sppp_detach(dev);
+ ppp_hard_header(skb, dev, pid, NULL, NULL, 0);
- dev->change_mtu = state(hdlc)->old_change_mtu;
- dev->mtu = HDLC_MAX_MTU;
- dev->hard_header_len = 16;
+ skb->priority = TC_PRIO_CONTROL;
+ skb->dev = dev;
+ skb_reset_network_header(skb);
+ skb_queue_tail(&tx_queue, skb);
}
+/* State transition table (compare STD-51)
+ Events Actions
+ TO+ = Timeout with counter > 0 irc = Initialize-Restart-Count
+ TO- = Timeout with counter expired zrc = Zero-Restart-Count
+
+ RCR+ = Receive-Configure-Request (Good) scr = Send-Configure-Request
+ RCR- = Receive-Configure-Request (Bad)
+ RCA = Receive-Configure-Ack sca = Send-Configure-Ack
+ RCN = Receive-Configure-Nak/Rej scn = Send-Configure-Nak/Rej
+
+ RTR = Receive-Terminate-Request str = Send-Terminate-Request
+ RTA = Receive-Terminate-Ack sta = Send-Terminate-Ack
+
+ RUC = Receive-Unknown-Code scj = Send-Code-Reject
+ RXJ+ = Receive-Code-Reject (permitted)
+ or Receive-Protocol-Reject
+ RXJ- = Receive-Code-Reject (catastrophic)
+ or Receive-Protocol-Reject
+*/
+static int cp_table[EVENTS][STATES] = {
+ /* CLOSED STOPPED STOPPING REQ_SENT ACK_RECV ACK_SENT OPENED
+ 0 1 2 3 4 5 6 */
+ {IRC|SCR|3, INV , INV , INV , INV , INV , INV }, /* START */
+ { INV , 0 , 0 , 0 , 0 , 0 , 0 }, /* STOP */
+ { INV , INV ,STR|2, SCR|3 ,SCR|3, SCR|5 , INV }, /* TO+ */
+ { INV , INV , 1 , 1 , 1 , 1 , INV }, /* TO- */
+ { STA|0 ,IRC|SCR|SCA|5, 2 , SCA|5 ,SCA|6, SCA|5 ,SCR|SCA|5}, /* RCR+ */
+ { STA|0 ,IRC|SCR|SCN|3, 2 , SCN|3 ,SCN|4, SCN|3 ,SCR|SCN|3}, /* RCR- */
+ { STA|0 , STA|1 , 2 , IRC|4 ,SCR|3, 6 , SCR|3 }, /* RCA */
+ { STA|0 , STA|1 , 2 ,IRC|SCR|3,SCR|3,IRC|SCR|5, SCR|3 }, /* RCN */
+ { STA|0 , STA|1 ,STA|2, STA|3 ,STA|3, STA|3 ,ZRC|STA|2}, /* RTR */
+ { 0 , 1 , 1 , 3 , 3 , 5 , SCR|3 }, /* RTA */
+ { SCJ|0 , SCJ|1 ,SCJ|2, SCJ|3 ,SCJ|4, SCJ|5 , SCJ|6 }, /* RUC */
+ { 0 , 1 , 2 , 3 , 3 , 5 , 6 }, /* RXJ+ */
+ { 0 , 1 , 1 , 1 , 1 , 1 ,IRC|STR|2}, /* RXJ- */
+};
-static __be16 ppp_type_trans(struct sk_buff *skb, struct net_device *dev)
+
+/* SCA: RCR+ must supply id, len and data
+ SCN: RCR- must supply code, id, len and data
+ STA: RTR must supply id
+ SCJ: RUC must supply CP packet len and data */
+static void ppp_cp_event(struct net_device *dev, u16 pid, u16 event, u8 code,
+ u8 id, unsigned int len, void *data)
{
- return __constant_htons(ETH_P_WAN_PPP);
+ int old_state, action;
+ struct ppp *ppp = get_ppp(dev);
+ struct proto *proto = get_proto(dev, pid);
+
+ old_state = proto->state;
+ BUG_ON(old_state >= STATES);
+ BUG_ON(event >= EVENTS);
+
+#if DEBUG_STATE
+ printk(KERN_DEBUG "%s: %s ppp_cp_event(%s) %s ...\n", dev->name,
+ proto_name(pid), event_names[event], state_names[proto->state]);
+#endif
+
+ action = cp_table[event][old_state];
+
+ proto->state = action & STATE_MASK;
+ if (action & (SCR | STR)) /* set Configure-Req/Terminate-Req timer */
+ mod_timer(&proto->timer, proto->timeout =
+ jiffies + ppp->req_timeout * HZ);
+ if (action & ZRC)
+ proto->restart_counter = 0;
+ if (action & IRC)
+ proto->restart_counter = (proto->state == STOPPING) ?
+ ppp->term_retries : ppp->cr_retries;
+
+ if (action & SCR) /* send Configure-Request */
+ ppp_tx_cp(dev, pid, CP_CONF_REQ, proto->cr_id = ++ppp->seq,
+ 0, NULL);
+ if (action & SCA) /* send Configure-Ack */
+ ppp_tx_cp(dev, pid, CP_CONF_ACK, id, len, data);
+ if (action & SCN) /* send Configure-Nak/Reject */
+ ppp_tx_cp(dev, pid, code, id, len, data);
+ if (action & STR) /* send Terminate-Request */
+ ppp_tx_cp(dev, pid, CP_TERM_REQ, ++ppp->seq, 0, NULL);
+ if (action & STA) /* send Terminate-Ack */
+ ppp_tx_cp(dev, pid, CP_TERM_ACK, id, 0, NULL);
+ if (action & SCJ) /* send Code-Reject */
+ ppp_tx_cp(dev, pid, CP_CODE_REJ, ++ppp->seq, len, data);
+
+ if (old_state != OPENED && proto->state == OPENED) {
+ printk(KERN_INFO "%s: %s up\n", dev->name, proto_name(pid));
+ if (pid == PID_LCP) {
+ netif_dormant_off(dev);
+ ppp_cp_event(dev, PID_IPCP, START, 0, 0, 0, NULL);
+ ppp_cp_event(dev, PID_IPV6CP, START, 0, 0, 0, NULL);
+ ppp->last_pong = jiffies;
+ mod_timer(&proto->timer, proto->timeout =
+ jiffies + ppp->keepalive_interval * HZ);
+ }
+ }
+ if (old_state == OPENED && proto->state != OPENED) {
+ printk(KERN_INFO "%s: %s down\n", dev->name, proto_name(pid));
+ if (pid == PID_LCP) {
+ netif_dormant_on(dev);
+ ppp_cp_event(dev, PID_IPCP, STOP, 0, 0, 0, NULL);
+ ppp_cp_event(dev, PID_IPV6CP, STOP, 0, 0, 0, NULL);
+ }
+ }
+ if (old_state != CLOSED && proto->state == CLOSED)
+ del_timer(&proto->timer);
+
+#if DEBUG_STATE
+ printk(KERN_DEBUG "%s: %s ppp_cp_event(%s) ... %s\n", dev->name,
+ proto_name(pid), event_names[event], state_names[proto->state]);
+#endif
}
+static void ppp_cp_parse_cr(struct net_device *dev, u16 pid, u8 id,
+ unsigned int len, u8 *data)
+{
+ static u8 valid_accm[6] = { LCP_OPTION_ACCM, 6, 0, 0, 0, 0 };
+ u8 *opt, *out;
+ unsigned int nak_len = 0, rej_len = 0;
+
+ if (!(out = kmalloc(len, GFP_ATOMIC))) {
+ dev_to_hdlc(dev)->stats.rx_dropped++;
+ return; /* out of memory, ignore CR packet */
+ }
+
+ for (opt = data; len; len -= opt[1], opt += opt[1]) {
+ if (len < 2 || len < opt[1]) {
+ dev_to_hdlc(dev)->stats.rx_errors++;
+ return; /* bad packet, drop silently */
+ }
+
+ if (pid == PID_LCP)
+ switch (opt[0]) {
+ case LCP_OPTION_MRU:
+ continue; /* MRU always OK and > 1500 bytes? */
+
+ case LCP_OPTION_ACCM: /* async control character map */
+ if (!memcmp(opt, valid_accm,
+ sizeof(valid_accm)))
+ continue;
+ if (!rej_len) { /* NAK it */
+ memcpy(out + nak_len, valid_accm,
+ sizeof(valid_accm));
+ nak_len += sizeof(valid_accm);
+ continue;
+ }
+ break;
+ case LCP_OPTION_MAGIC:
+ if (opt[1] != 6 || (!opt[2] && !opt[3] &&
+ !opt[4] && !opt[5]))
+ break; /* reject invalid magic number */
+ continue;
+ }
+ /* reject this option */
+ memcpy(out + rej_len, opt, opt[1]);
+ rej_len += opt[1];
+ }
+
+ if (rej_len)
+ ppp_cp_event(dev, pid, RCR_BAD, CP_CONF_REJ, id, rej_len, out);
+ else if (nak_len)
+ ppp_cp_event(dev, pid, RCR_BAD, CP_CONF_NAK, id, nak_len, out);
+ else
+ ppp_cp_event(dev, pid, RCR_GOOD, CP_CONF_ACK, id, len, data);
+
+ kfree(out);
+}
+
+static int ppp_rx(struct sk_buff *skb)
+{
+ struct hdlc_header *hdr = (struct hdlc_header*)skb->data;
+ struct net_device *dev = skb->dev;
+ struct ppp *ppp = get_ppp(dev);
+ struct proto *proto;
+ struct cp_header *cp;
+ unsigned long flags;
+ unsigned int len;
+ u16 pid;
+#if DEBUG_CP
+ int i;
+ char *ptr;
+#endif
+
+ spin_lock_irqsave(&ppp->lock, flags);
+ /* Check HDLC header */
+ if (skb->len < sizeof(struct hdlc_header))
+ goto rx_error;
+ cp = (struct cp_header*)skb_pull(skb, sizeof(struct hdlc_header));
+ if (hdr->address != HDLC_ADDR_ALLSTATIONS ||
+ hdr->control != HDLC_CTRL_UI)
+ goto rx_error;
+
+ pid = ntohs(hdr->protocol);
+ proto = get_proto(dev, pid);
+ if (!proto) {
+ if (ppp->protos[IDX_LCP].state == OPENED)
+ ppp_tx_cp(dev, PID_LCP, LCP_PROTO_REJ,
+ ++ppp->seq, skb->len + 2, &hdr->protocol);
+ goto rx_error;
+ }
+
+ len = ntohs(cp->len);
+ if (len < sizeof(struct cp_header) /* no complete CP header? */ ||
+ skb->len < len /* truncated packet? */)
+ goto rx_error;
+ skb_pull(skb, sizeof(struct cp_header));
+ len -= sizeof(struct cp_header);
+
+ /* HDLC and CP headers stripped from skb */
+#if DEBUG_CP
+ if (cp->code < CP_CODES)
+ sprintf(debug_buffer, "[%s id 0x%X]", code_names[cp->code],
+ cp->id);
+ else
+ sprintf(debug_buffer, "[code %u id 0x%X]", cp->code, cp->id);
+ ptr = debug_buffer + strlen(debug_buffer);
+ for (i = 0; i < min_t(unsigned int, len, DEBUG_CP); i++) {
+ sprintf(ptr, " %02X", skb->data[i]);
+ ptr += strlen(ptr);
+ }
+ printk(KERN_DEBUG "%s: RX %s %s\n", dev->name, proto_name(pid),
+ debug_buffer);
+#endif
+
+ /* LCP only */
+ if (pid == PID_LCP)
+ switch (cp->code) {
+ case LCP_PROTO_REJ:
+ pid = ntohs(*(__be16*)skb->data);
+ if (pid == PID_LCP || pid == PID_IPCP ||
+ pid == PID_IPV6CP)
+ ppp_cp_event(dev, pid, RXJ_BAD, 0, 0,
+ 0, NULL);
+ goto out;
+
+ case LCP_ECHO_REQ: /* send Echo-Reply */
+ if (len >= 4 && proto->state == OPENED)
+ ppp_tx_cp(dev, PID_LCP, LCP_ECHO_REPLY,
+ cp->id, len - 4, skb->data + 4);
+ goto out;
+
+ case LCP_ECHO_REPLY:
+ if (cp->id == ppp->echo_id)
+ ppp->last_pong = jiffies;
+ goto out;
+
+ case LCP_DISC_REQ: /* discard */
+ goto out;
+ }
+
+ /* LCP, IPCP and IPV6CP */
+ switch (cp->code) {
+ case CP_CONF_REQ:
+ ppp_cp_parse_cr(dev, pid, cp->id, len, skb->data);
+ goto out;
+
+ case CP_CONF_ACK:
+ if (cp->id == proto->cr_id)
+ ppp_cp_event(dev, pid, RCA, 0, 0, 0, NULL);
+ goto out;
+
+ case CP_CONF_REJ:
+ case CP_CONF_NAK:
+ if (cp->id == proto->cr_id)
+ ppp_cp_event(dev, pid, RCN, 0, 0, 0, NULL);
+ goto out;
+
+ case CP_TERM_REQ:
+ ppp_cp_event(dev, pid, RTR, 0, cp->id, 0, NULL);
+ goto out;
+
+ case CP_TERM_ACK:
+ ppp_cp_event(dev, pid, RTA, 0, 0, 0, NULL);
+ goto out;
+
+ case CP_CODE_REJ:
+ ppp_cp_event(dev, pid, RXJ_BAD, 0, 0, 0, NULL);
+ goto out;
+
+ default:
+ len += sizeof(struct cp_header);
+ if (len > dev->mtu)
+ len = dev->mtu;
+ ppp_cp_event(dev, pid, RUC, 0, 0, len, cp);
+ goto out;
+ }
+ goto out;
+
+rx_error:
+ dev_to_hdlc(dev)->stats.rx_errors++;
+out:
+ spin_unlock_irqrestore(&ppp->lock, flags);
+ dev_kfree_skb_any(skb);
+ ppp_tx_flush();
+ return NET_RX_DROP;
+}
+
+
+static void ppp_timer(unsigned long arg)
+{
+ struct proto *proto = (struct proto *)arg;
+ struct ppp *ppp = get_ppp(proto->dev);
+ unsigned long flags;
+
+ spin_lock_irqsave(&ppp->lock, flags);
+ switch (proto->state) {
+ case STOPPING:
+ case REQ_SENT:
+ case ACK_RECV:
+ case ACK_SENT:
+ if (proto->restart_counter) {
+ ppp_cp_event(proto->dev, proto->pid, TO_GOOD, 0, 0,
+ 0, NULL);
+ proto->restart_counter--;
+ } else
+ ppp_cp_event(proto->dev, proto->pid, TO_BAD, 0, 0,
+ 0, NULL);
+ break;
+
+ case OPENED:
+ if (proto->pid != PID_LCP)
+ break;
+ if (time_after(jiffies, ppp->last_pong +
+ ppp->keepalive_timeout * HZ)) {
+ printk(KERN_INFO "%s: Link down\n", proto->dev->name);
+ ppp_cp_event(proto->dev, PID_LCP, STOP, 0, 0, 0, NULL);
+ ppp_cp_event(proto->dev, PID_LCP, START, 0, 0, 0, NULL);
+ } else { /* send keep-alive packet */
+ ppp->echo_id = ++ppp->seq;
+ ppp_tx_cp(proto->dev, PID_LCP, LCP_ECHO_REQ,
+ ppp->echo_id, 0, NULL);
+ proto->timer.expires = jiffies +
+ ppp->keepalive_interval * HZ;
+ add_timer(&proto->timer);
+ }
+ break;
+ }
+ spin_unlock_irqrestore(&ppp->lock, flags);
+ ppp_tx_flush();
+}
+
+
+static void ppp_start(struct net_device *dev)
+{
+ struct ppp *ppp = get_ppp(dev);
+ int i;
+
+ for (i = 0; i < IDX_COUNT; i++) {
+ struct proto *proto = &ppp->protos[i];
+ proto->dev = dev;
+ init_timer(&proto->timer);
+ proto->timer.function = ppp_timer;
+ proto->timer.data = (unsigned long)proto;
+ proto->state = CLOSED;
+ }
+ ppp->protos[IDX_LCP].pid = PID_LCP;
+ ppp->protos[IDX_IPCP].pid = PID_IPCP;
+ ppp->protos[IDX_IPV6CP].pid = PID_IPV6CP;
+
+ ppp_cp_event(dev, PID_LCP, START, 0, 0, 0, NULL);
+}
+
+static void ppp_stop(struct net_device *dev)
+{
+ ppp_cp_event(dev, PID_LCP, STOP, 0, 0, 0, NULL);
+}
static struct hdlc_proto proto = {
- .open = ppp_open,
- .close = ppp_close,
+ .start = ppp_start,
+ .stop = ppp_stop,
.type_trans = ppp_type_trans,
.ioctl = ppp_ioctl,
+ .netif_rx = ppp_rx,
.module = THIS_MODULE,
};
+static const struct header_ops ppp_header_ops = {
+ .create = ppp_hard_header,
+};
static int ppp_ioctl(struct net_device *dev, struct ifreq *ifr)
{
hdlc_device *hdlc = dev_to_hdlc(dev);
+ struct ppp *ppp;
int result;
switch (ifr->ifr_settings.type) {
@@ -110,25 +652,35 @@ static int ppp_ioctl(struct net_device *dev, struct ifreq *ifr)
return 0; /* return protocol only, no settable parameters */
case IF_PROTO_PPP:
- if(!capable(CAP_NET_ADMIN))
+ if (!capable(CAP_NET_ADMIN))
return -EPERM;
- if(dev->flags & IFF_UP)
+ if (dev->flags & IFF_UP)
return -EBUSY;
/* no settable parameters */
- result=hdlc->attach(dev, ENCODING_NRZ,PARITY_CRC16_PR1_CCITT);
+ result = hdlc->attach(dev, ENCODING_NRZ,PARITY_CRC16_PR1_CCITT);
if (result)
return result;
- result = attach_hdlc_protocol(dev, &proto,
- sizeof(struct ppp_state));
+ result = attach_hdlc_protocol(dev, &proto, sizeof(struct ppp));
if (result)
return result;
+
+ ppp = get_ppp(dev);
+ spin_lock_init(&ppp->lock);
+ ppp->req_timeout = 2;
+ ppp->cr_retries = 10;
+ ppp->term_retries = 2;
+ ppp->keepalive_interval = 10;
+ ppp->keepalive_timeout = 60;
+
dev->hard_start_xmit = hdlc->xmit;
+ dev->hard_header_len = sizeof(struct hdlc_header);
+ dev->header_ops = &ppp_header_ops;
dev->type = ARPHRD_PPP;
- netif_dormant_off(dev);
+ netif_dormant_on(dev);
return 0;
}
@@ -138,12 +690,11 @@ static int ppp_ioctl(struct net_device *dev, struct ifreq *ifr)
static int __init mod_init(void)
{
+ skb_queue_head_init(&tx_queue);
register_hdlc_protocol(&proto);
return 0;
}
-
-
static void __exit mod_exit(void)
{
unregister_hdlc_protocol(&proto);
> +struct hdlc_header {
> + u8 address;
> + u8 control;
> + __be16 protocol;
> +} __attribute__ ((packed));
> +
> +struct cp_header {
> + u8 code;
> + u8 id;
> + __be16 len;
> +} __attribute__ ((packed));
> +
If I remember correctly, the packed is unnecessary for structures like this
and causes GCC to generate worse code.
Stephen Hemminger <[email protected]> writes:
>> +struct hdlc_header {
>> + u8 address;
>> + u8 control;
>> + __be16 protocol;
>> +} __attribute__ ((packed));
>> +
>> +struct cp_header {
>> + u8 code;
>> + u8 id;
>> + __be16 len;
>> +} __attribute__ ((packed));
>> +
>
> If I remember correctly, the packed is unnecessary for structures like this
> and causes GCC to generate worse code.
Interesting. Gcc obviously will do the right thing without "packed",
at least on "current" architectures, but I don't know what the C
standard says. I will remove it, thanks.
--
Krzysztof Halasa
On Mar 12 2008 19:30, Krzysztof Halasa wrote:
>+ /* LCP only */
>+ if (pid == PID_LCP)
>+ switch (cp->code) {
>+ case LCP_PROTO_REJ:
>+ pid = ntohs(*(__be16*)skb->data);
What if skb->data happens to be unaligned (can that even happen?)
BTW, here are some consts:
diff --git a/drivers/net/wan/hdlc_ppp.c b/drivers/net/wan/hdlc_ppp.c
index b626aae..0e4cb1f 100644
--- a/drivers/net/wan/hdlc_ppp.c
+++ b/drivers/net/wan/hdlc_ppp.c
@@ -40,7 +40,7 @@ enum {CP_CONF_REQ = 1, CP_CONF_ACK, CP_CONF_NAK, CP_CONF_REJ, CP_TERM_REQ,
CP_TERM_ACK, CP_CODE_REJ, LCP_PROTO_REJ, LCP_ECHO_REQ, LCP_ECHO_REPLY,
LCP_DISC_REQ, CP_CODES};
#if DEBUG_CP
-static const char *code_names[CP_CODES] = {"0", "ConfReq", "ConfAck",
+static const char *const code_names[] = {"0", "ConfReq", "ConfAck",
"ConfNak", "ConfRej", "TermReq",
"TermAck", "CodeRej", "ProtoRej",
"EchoReq", "EchoReply", "Discard"};
@@ -90,10 +90,10 @@ enum {INV = 0x10, IRC = 0x20, ZRC = 0x40, SCR = 0x80, SCA = 0x100,
SCN = 0x200, STR = 0x400, STA = 0x800, SCJ = 0x1000};
#if DEBUG_STATE
-static const char *state_names[STATES] = {"Closed", "Stopped", "Stopping",
+static const char *const state_names[] = {"Closed", "Stopped", "Stopping",
"ReqSent", "AckRecv", "AckSent",
"Opened"};
-static const char *event_names[EVENTS] = {"Start", "Stop", "TO+", "TO-",
+static const char *const event_names[] = {"Start", "Stop", "TO+", "TO-",
"RCR+", "RCR-", "RCA", "RCN",
"RTR", "RTA", "RUC", "RXJ+", "RXJ-"};
#endif
@@ -374,7 +374,7 @@ static void ppp_cp_event(struct net_device *dev, u16 pid, u16 event, u8 code,
static void ppp_cp_parse_cr(struct net_device *dev, u16 pid, u8 id,
unsigned int len, u8 *data)
{
- static u8 valid_accm[6] = { LCP_OPTION_ACCM, 6, 0, 0, 0, 0 };
+ static const u8 valid_accm[] = { LCP_OPTION_ACCM, 6, 0, 0, 0, 0 };
u8 *opt, *out;
unsigned int nak_len = 0, rej_len = 0;
Jan Engelhardt <[email protected]> writes:
>>+ /* LCP only */
>>+ if (pid == PID_LCP)
>>+ switch (cp->code) {
>>+ case LCP_PROTO_REJ:
>>+ pid = ntohs(*(__be16*)skb->data);
>
> What if skb->data happens to be unaligned (can that even happen?)
It can't if the hdlc header is 32-bit aligned... however, I can now
see the above is buggy, skb->data doesn't point to the protocol
field. Unused code paths :-(
> -static const char *code_names[CP_CODES] = {"0", "ConfReq", "ConfAck",
> +static const char *const code_names[] = {"0", "ConfReq", "ConfAck",
> "ConfNak", "ConfRej", "TermReq",
> "TermAck", "CodeRej", "ProtoRej",
> "EchoReq", "EchoReply", "Discard"};
The explicit sizes are there as a prevention.
Thanks.
--
Krzysztof Halasa
On Mar 12 2008 21:10, Krzysztof Halasa wrote:
>
>> -static const char *code_names[CP_CODES] = {"0", "ConfReq", "ConfAck",
>> +static const char *const code_names[] = {"0", "ConfReq", "ConfAck",
>> "ConfNak", "ConfRej", "TermReq",
>> "TermAck", "CodeRej", "ProtoRej",
>> "EchoReq", "EchoReply", "Discard"};
>
>The explicit sizes are there as a prevention.
You can keep them if you need, the main issue was const anyway.
Krzysztof Halasa <[email protected]> writes:
> It can't if the hdlc header is 32-bit aligned... however, I can now
> see the above is buggy, skb->data doesn't point to the protocol
> field. Unused code paths :-(
Actually I was wrong, the code is correct and skb->data points to the
proto ID.
PATCH v2 is on the way.
--
Krzysztof Halasa
New synchronous PPP implementation for generic HDLC.
Signed-off-by: Krzysztof Halasa <[email protected]>
drivers/net/wan/Makefile | 2 +-
drivers/net/wan/hdlc_ppp.c | 649 ++++++++++++++++++++++++++++++++++++++++----
2 files changed, 602 insertions(+), 49 deletions(-)
diff --git a/drivers/net/wan/Makefile b/drivers/net/wan/Makefile
index d61fef3..3081683 100644
--- a/drivers/net/wan/Makefile
+++ b/drivers/net/wan/Makefile
@@ -14,7 +14,7 @@ obj-$(CONFIG_HDLC_RAW) += hdlc_raw.o
obj-$(CONFIG_HDLC_RAW_ETH) += hdlc_raw_eth.o
obj-$(CONFIG_HDLC_CISCO) += hdlc_cisco.o
obj-$(CONFIG_HDLC_FR) += hdlc_fr.o
-obj-$(CONFIG_HDLC_PPP) += hdlc_ppp.o syncppp.o
+obj-$(CONFIG_HDLC_PPP) += hdlc_ppp.o
obj-$(CONFIG_HDLC_X25) += hdlc_x25.o
pc300-y := pc300_drv.o
diff --git a/drivers/net/wan/hdlc_ppp.c b/drivers/net/wan/hdlc_ppp.c
index 10396d9..9fc20fb 100644
--- a/drivers/net/wan/hdlc_ppp.c
+++ b/drivers/net/wan/hdlc_ppp.c
@@ -2,7 +2,7 @@
* Generic HDLC support routines for Linux
* Point-to-point protocol support
*
- * Copyright (C) 1999 - 2006 Krzysztof Halasa <[email protected]>
+ * Copyright (C) 1999 - 2008 Krzysztof Halasa <[email protected]>
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of version 2 of the GNU General Public License
@@ -19,87 +19,631 @@
#include <linux/skbuff.h>
#include <linux/pkt_sched.h>
#include <linux/inetdevice.h>
-#include <linux/lapb.h>
-#include <linux/rtnetlink.h>
#include <linux/hdlc.h>
-#include <net/syncppp.h>
+#include <linux/spinlock.h>
+
+#define DEBUG_CP 0 /* also bytes# to dump */
+#define DEBUG_STATE 0
+#define DEBUG_HARD_HEADER 0
+
+#define HDLC_ADDR_ALLSTATIONS 0xFF
+#define HDLC_CTRL_UI 0x03
+
+#define PID_LCP 0xC021
+#define PID_IP 0x0021
+#define PID_IPCP 0x8021
+#define PID_IPV6 0x0057
+#define PID_IPV6CP 0x8057
+
+enum {IDX_LCP = 0, IDX_IPCP, IDX_IPV6CP, IDX_COUNT};
+enum {CP_CONF_REQ = 1, CP_CONF_ACK, CP_CONF_NAK, CP_CONF_REJ, CP_TERM_REQ,
+ CP_TERM_ACK, CP_CODE_REJ, LCP_PROTO_REJ, LCP_ECHO_REQ, LCP_ECHO_REPLY,
+ LCP_DISC_REQ, CP_CODES};
+#if DEBUG_CP
+static const char *const code_names[CP_CODES] = {
+ "0", "ConfReq", "ConfAck", "ConfNak", "ConfRej", "TermReq",
+ "TermAck", "CodeRej", "ProtoRej", "EchoReq", "EchoReply", "Discard"
+};
+static char debug_buffer[64 + 3 * DEBUG_CP];
+#endif
+
+enum {LCP_OPTION_MRU = 1, LCP_OPTION_ACCM, LCP_OPTION_MAGIC = 5};
+
+struct hdlc_header {
+ u8 address;
+ u8 control;
+ __be16 protocol;
+};
+
+struct cp_header {
+ u8 code;
+ u8 id;
+ __be16 len;
+};
+
-struct ppp_state {
- struct ppp_device pppdev;
- struct ppp_device *syncppp_ptr;
- int (*old_change_mtu)(struct net_device *dev, int new_mtu);
+struct proto {
+ struct net_device *dev;
+ struct timer_list timer;
+ unsigned long timeout;
+ u16 pid; /* protocol ID */
+ u8 state;
+ u8 cr_id; /* ID of last Configuration-Request */
+ u8 restart_counter;
};
+struct ppp {
+ struct proto protos[IDX_COUNT];
+ spinlock_t lock;
+ unsigned long last_pong;
+ unsigned int req_timeout, cr_retries, term_retries;
+ unsigned int keepalive_interval, keepalive_timeout;
+ u8 seq; /* local sequence number for requests */
+ u8 echo_id; /* ID of last Echo-Request (LCP) */
+};
+
+enum {CLOSED = 0, STOPPED, STOPPING, REQ_SENT, ACK_RECV, ACK_SENT, OPENED,
+ STATES, STATE_MASK = 0xF};
+enum {START = 0, STOP, TO_GOOD, TO_BAD, RCR_GOOD, RCR_BAD, RCA, RCN, RTR, RTA,
+ RUC, RXJ_GOOD, RXJ_BAD, EVENTS};
+enum {INV = 0x10, IRC = 0x20, ZRC = 0x40, SCR = 0x80, SCA = 0x100,
+ SCN = 0x200, STR = 0x400, STA = 0x800, SCJ = 0x1000};
+
+#if DEBUG_STATE
+static const char *const state_names[STATES] = {
+ "Closed", "Stopped", "Stopping", "ReqSent", "AckRecv", "AckSent",
+ "Opened"
+};
+static const char *const event_names[EVENTS] = {
+ "Start", "Stop", "TO+", "TO-", "RCR+", "RCR-", "RCA", "RCN",
+ "RTR", "RTA", "RUC", "RXJ+", "RXJ-"
+};
+#endif
+
+static struct sk_buff_head tx_queue; /* used when holding the spin lock */
+
static int ppp_ioctl(struct net_device *dev, struct ifreq *ifr);
+static inline struct ppp* get_ppp(struct net_device *dev)
+{
+ return (struct ppp *)dev_to_hdlc(dev)->state;
+}
-static inline struct ppp_state* state(hdlc_device *hdlc)
+static inline struct proto* get_proto(struct net_device *dev, u16 pid)
{
- return(struct ppp_state *)(hdlc->state);
+ struct ppp *ppp = get_ppp(dev);
+
+ switch (pid) {
+ case PID_LCP:
+ return &ppp->protos[IDX_LCP];
+ case PID_IPCP:
+ return &ppp->protos[IDX_IPCP];
+ case PID_IPV6CP:
+ return &ppp->protos[IDX_IPV6CP];
+ default:
+ return NULL;
+ }
}
+static inline const char* proto_name(u16 pid)
+{
+ switch (pid) {
+ case PID_LCP:
+ return "LCP";
+ case PID_IPCP:
+ return "IPCP";
+ case PID_IPV6CP:
+ return "IPV6CP";
+ default:
+ return NULL;
+ }
+}
-static int ppp_open(struct net_device *dev)
+static __be16 ppp_type_trans(struct sk_buff *skb, struct net_device *dev)
{
- hdlc_device *hdlc = dev_to_hdlc(dev);
- int (*old_ioctl)(struct net_device *, struct ifreq *, int);
- int result;
+ struct hdlc_header *data = (struct hdlc_header*)skb->data;
+
+ if (skb->len < sizeof(struct hdlc_header))
+ return htons(ETH_P_HDLC);
+ if (data->address != HDLC_ADDR_ALLSTATIONS ||
+ data->control != HDLC_CTRL_UI)
+ return htons(ETH_P_HDLC);
+
+ switch (data->protocol) {
+ case __constant_htons(PID_IP):
+ skb_pull(skb, sizeof(struct hdlc_header));
+ return htons(ETH_P_IP);
- dev->priv = &state(hdlc)->syncppp_ptr;
- state(hdlc)->syncppp_ptr = &state(hdlc)->pppdev;
- state(hdlc)->pppdev.dev = dev;
+ case __constant_htons(PID_IPV6):
+ skb_pull(skb, sizeof(struct hdlc_header));
+ return htons(ETH_P_IPV6);
- old_ioctl = dev->do_ioctl;
- state(hdlc)->old_change_mtu = dev->change_mtu;
- sppp_attach(&state(hdlc)->pppdev);
- /* sppp_attach nukes them. We don't need syncppp's ioctl */
- dev->do_ioctl = old_ioctl;
- state(hdlc)->pppdev.sppp.pp_flags &= ~PP_CISCO;
- dev->type = ARPHRD_PPP;
- result = sppp_open(dev);
- if (result) {
- sppp_detach(dev);
- return result;
+ default:
+ return htons(ETH_P_HDLC);
}
+}
- return 0;
+
+static int ppp_hard_header(struct sk_buff *skb, struct net_device *dev,
+ u16 type, const void *daddr, const void *saddr,
+ unsigned int len)
+{
+ struct hdlc_header *data;
+#if DEBUG_HARD_HEADER
+ printk(KERN_DEBUG "%s: ppp_hard_header() called\n", dev->name);
+#endif
+
+ skb_push(skb, sizeof(struct hdlc_header));
+ data = (struct hdlc_header*)skb->data;
+
+ data->address = HDLC_ADDR_ALLSTATIONS;
+ data->control = HDLC_CTRL_UI;
+ switch (type) {
+ case ETH_P_IP:
+ data->protocol = htons(PID_IP);
+ break;
+ case ETH_P_IPV6:
+ data->protocol = htons(PID_IPV6);
+ break;
+ case PID_LCP:
+ case PID_IPCP:
+ case PID_IPV6CP:
+ data->protocol = htons(type);
+ break;
+ default: /* unknown protocol */
+ data->protocol = 0;
+ }
+ return sizeof(struct hdlc_header);
}
+static void ppp_tx_flush(void)
+{
+ struct sk_buff *skb;
+ while ((skb = skb_dequeue(&tx_queue)) != NULL)
+ dev_queue_xmit(skb);
+}
-static void ppp_close(struct net_device *dev)
+static void ppp_tx_cp(struct net_device *dev, u16 pid, u8 code,
+ u8 id, unsigned int len, const void *data)
{
- hdlc_device *hdlc = dev_to_hdlc(dev);
+ struct sk_buff *skb;
+ struct cp_header *cp;
+ unsigned int magic_len = 0;
+ static u32 magic;
+
+#if DEBUG_CP
+ int i;
+ char *ptr;
+#endif
+
+ if (pid == PID_LCP && (code == LCP_ECHO_REQ || code == LCP_ECHO_REPLY))
+ magic_len = sizeof(magic);
+
+ skb = dev_alloc_skb(sizeof(struct hdlc_header) +
+ sizeof(struct cp_header) + magic_len + len);
+ if (!skb) {
+ printk(KERN_WARNING "%s: out of memory in ppp_tx_cp()\n",
+ dev->name);
+ return;
+ }
+ skb_reserve(skb, sizeof(struct hdlc_header));
+
+ cp = (struct cp_header *)skb_put(skb, sizeof(struct cp_header));
+ cp->code = code;
+ cp->id = id;
+ cp->len = htons(sizeof(struct cp_header) + magic_len + len);
+
+ if (magic_len)
+ memcpy(skb_put(skb, magic_len), &magic, magic_len);
+ if (len)
+ memcpy(skb_put(skb, len), data, len);
+
+#if DEBUG_CP
+ BUG_ON(code >= CP_CODES);
+ ptr = debug_buffer;
+ *ptr = '\x0';
+ for (i = 0; i < min_t(unsigned int, magic_len + len, DEBUG_CP); i++) {
+ sprintf(ptr, " %02X", skb->data[sizeof(struct cp_header) + i]);
+ ptr += strlen(ptr);
+ }
+ printk(KERN_DEBUG "%s: TX %s [%s id 0x%X]%s\n", dev->name,
+ proto_name(pid), code_names[code], id, debug_buffer);
+#endif
- sppp_close(dev);
- sppp_detach(dev);
+ ppp_hard_header(skb, dev, pid, NULL, NULL, 0);
- dev->change_mtu = state(hdlc)->old_change_mtu;
- dev->mtu = HDLC_MAX_MTU;
- dev->hard_header_len = 16;
+ skb->priority = TC_PRIO_CONTROL;
+ skb->dev = dev;
+ skb_reset_network_header(skb);
+ skb_queue_tail(&tx_queue, skb);
}
+/* State transition table (compare STD-51)
+ Events Actions
+ TO+ = Timeout with counter > 0 irc = Initialize-Restart-Count
+ TO- = Timeout with counter expired zrc = Zero-Restart-Count
+
+ RCR+ = Receive-Configure-Request (Good) scr = Send-Configure-Request
+ RCR- = Receive-Configure-Request (Bad)
+ RCA = Receive-Configure-Ack sca = Send-Configure-Ack
+ RCN = Receive-Configure-Nak/Rej scn = Send-Configure-Nak/Rej
+
+ RTR = Receive-Terminate-Request str = Send-Terminate-Request
+ RTA = Receive-Terminate-Ack sta = Send-Terminate-Ack
+
+ RUC = Receive-Unknown-Code scj = Send-Code-Reject
+ RXJ+ = Receive-Code-Reject (permitted)
+ or Receive-Protocol-Reject
+ RXJ- = Receive-Code-Reject (catastrophic)
+ or Receive-Protocol-Reject
+*/
+static int cp_table[EVENTS][STATES] = {
+ /* CLOSED STOPPED STOPPING REQ_SENT ACK_RECV ACK_SENT OPENED
+ 0 1 2 3 4 5 6 */
+ {IRC|SCR|3, INV , INV , INV , INV , INV , INV }, /* START */
+ { INV , 0 , 0 , 0 , 0 , 0 , 0 }, /* STOP */
+ { INV , INV ,STR|2, SCR|3 ,SCR|3, SCR|5 , INV }, /* TO+ */
+ { INV , INV , 1 , 1 , 1 , 1 , INV }, /* TO- */
+ { STA|0 ,IRC|SCR|SCA|5, 2 , SCA|5 ,SCA|6, SCA|5 ,SCR|SCA|5}, /* RCR+ */
+ { STA|0 ,IRC|SCR|SCN|3, 2 , SCN|3 ,SCN|4, SCN|3 ,SCR|SCN|3}, /* RCR- */
+ { STA|0 , STA|1 , 2 , IRC|4 ,SCR|3, 6 , SCR|3 }, /* RCA */
+ { STA|0 , STA|1 , 2 ,IRC|SCR|3,SCR|3,IRC|SCR|5, SCR|3 }, /* RCN */
+ { STA|0 , STA|1 ,STA|2, STA|3 ,STA|3, STA|3 ,ZRC|STA|2}, /* RTR */
+ { 0 , 1 , 1 , 3 , 3 , 5 , SCR|3 }, /* RTA */
+ { SCJ|0 , SCJ|1 ,SCJ|2, SCJ|3 ,SCJ|4, SCJ|5 , SCJ|6 }, /* RUC */
+ { 0 , 1 , 2 , 3 , 3 , 5 , 6 }, /* RXJ+ */
+ { 0 , 1 , 1 , 1 , 1 , 1 ,IRC|STR|2}, /* RXJ- */
+};
+
-static __be16 ppp_type_trans(struct sk_buff *skb, struct net_device *dev)
+/* SCA: RCR+ must supply id, len and data
+ SCN: RCR- must supply code, id, len and data
+ STA: RTR must supply id
+ SCJ: RUC must supply CP packet len and data */
+static void ppp_cp_event(struct net_device *dev, u16 pid, u16 event, u8 code,
+ u8 id, unsigned int len, void *data)
{
- return __constant_htons(ETH_P_WAN_PPP);
+ int old_state, action;
+ struct ppp *ppp = get_ppp(dev);
+ struct proto *proto = get_proto(dev, pid);
+
+ old_state = proto->state;
+ BUG_ON(old_state >= STATES);
+ BUG_ON(event >= EVENTS);
+
+#if DEBUG_STATE
+ printk(KERN_DEBUG "%s: %s ppp_cp_event(%s) %s ...\n", dev->name,
+ proto_name(pid), event_names[event], state_names[proto->state]);
+#endif
+
+ action = cp_table[event][old_state];
+
+ proto->state = action & STATE_MASK;
+ if (action & (SCR | STR)) /* set Configure-Req/Terminate-Req timer */
+ mod_timer(&proto->timer, proto->timeout =
+ jiffies + ppp->req_timeout * HZ);
+ if (action & ZRC)
+ proto->restart_counter = 0;
+ if (action & IRC)
+ proto->restart_counter = (proto->state == STOPPING) ?
+ ppp->term_retries : ppp->cr_retries;
+
+ if (action & SCR) /* send Configure-Request */
+ ppp_tx_cp(dev, pid, CP_CONF_REQ, proto->cr_id = ++ppp->seq,
+ 0, NULL);
+ if (action & SCA) /* send Configure-Ack */
+ ppp_tx_cp(dev, pid, CP_CONF_ACK, id, len, data);
+ if (action & SCN) /* send Configure-Nak/Reject */
+ ppp_tx_cp(dev, pid, code, id, len, data);
+ if (action & STR) /* send Terminate-Request */
+ ppp_tx_cp(dev, pid, CP_TERM_REQ, ++ppp->seq, 0, NULL);
+ if (action & STA) /* send Terminate-Ack */
+ ppp_tx_cp(dev, pid, CP_TERM_ACK, id, 0, NULL);
+ if (action & SCJ) /* send Code-Reject */
+ ppp_tx_cp(dev, pid, CP_CODE_REJ, ++ppp->seq, len, data);
+
+ if (old_state != OPENED && proto->state == OPENED) {
+ printk(KERN_INFO "%s: %s up\n", dev->name, proto_name(pid));
+ if (pid == PID_LCP) {
+ netif_dormant_off(dev);
+ ppp_cp_event(dev, PID_IPCP, START, 0, 0, 0, NULL);
+ ppp_cp_event(dev, PID_IPV6CP, START, 0, 0, 0, NULL);
+ ppp->last_pong = jiffies;
+ mod_timer(&proto->timer, proto->timeout =
+ jiffies + ppp->keepalive_interval * HZ);
+ }
+ }
+ if (old_state == OPENED && proto->state != OPENED) {
+ printk(KERN_INFO "%s: %s down\n", dev->name, proto_name(pid));
+ if (pid == PID_LCP) {
+ netif_dormant_on(dev);
+ ppp_cp_event(dev, PID_IPCP, STOP, 0, 0, 0, NULL);
+ ppp_cp_event(dev, PID_IPV6CP, STOP, 0, 0, 0, NULL);
+ }
+ }
+ if (old_state != CLOSED && proto->state == CLOSED)
+ del_timer(&proto->timer);
+
+#if DEBUG_STATE
+ printk(KERN_DEBUG "%s: %s ppp_cp_event(%s) ... %s\n", dev->name,
+ proto_name(pid), event_names[event], state_names[proto->state]);
+#endif
}
+static void ppp_cp_parse_cr(struct net_device *dev, u16 pid, u8 id,
+ unsigned int len, u8 *data)
+{
+ static u8 const valid_accm[6] = { LCP_OPTION_ACCM, 6, 0, 0, 0, 0 };
+ u8 *opt, *out;
+ unsigned int nak_len = 0, rej_len = 0;
+
+ if (!(out = kmalloc(len, GFP_ATOMIC))) {
+ dev_to_hdlc(dev)->stats.rx_dropped++;
+ return; /* out of memory, ignore CR packet */
+ }
+
+ for (opt = data; len; len -= opt[1], opt += opt[1]) {
+ if (len < 2 || len < opt[1]) {
+ dev_to_hdlc(dev)->stats.rx_errors++;
+ return; /* bad packet, drop silently */
+ }
+
+ if (pid == PID_LCP)
+ switch (opt[0]) {
+ case LCP_OPTION_MRU:
+ continue; /* MRU always OK and > 1500 bytes? */
+
+ case LCP_OPTION_ACCM: /* async control character map */
+ if (!memcmp(opt, valid_accm,
+ sizeof(valid_accm)))
+ continue;
+ if (!rej_len) { /* NAK it */
+ memcpy(out + nak_len, valid_accm,
+ sizeof(valid_accm));
+ nak_len += sizeof(valid_accm);
+ continue;
+ }
+ break;
+ case LCP_OPTION_MAGIC:
+ if (opt[1] != 6 || (!opt[2] && !opt[3] &&
+ !opt[4] && !opt[5]))
+ break; /* reject invalid magic number */
+ continue;
+ }
+ /* reject this option */
+ memcpy(out + rej_len, opt, opt[1]);
+ rej_len += opt[1];
+ }
+
+ if (rej_len)
+ ppp_cp_event(dev, pid, RCR_BAD, CP_CONF_REJ, id, rej_len, out);
+ else if (nak_len)
+ ppp_cp_event(dev, pid, RCR_BAD, CP_CONF_NAK, id, nak_len, out);
+ else
+ ppp_cp_event(dev, pid, RCR_GOOD, CP_CONF_ACK, id, len, data);
+
+ kfree(out);
+}
+
+static int ppp_rx(struct sk_buff *skb)
+{
+ struct hdlc_header *hdr = (struct hdlc_header*)skb->data;
+ struct net_device *dev = skb->dev;
+ struct ppp *ppp = get_ppp(dev);
+ struct proto *proto;
+ struct cp_header *cp;
+ unsigned long flags;
+ unsigned int len;
+ u16 pid;
+#if DEBUG_CP
+ int i;
+ char *ptr;
+#endif
+
+ spin_lock_irqsave(&ppp->lock, flags);
+ /* Check HDLC header */
+ if (skb->len < sizeof(struct hdlc_header))
+ goto rx_error;
+ cp = (struct cp_header*)skb_pull(skb, sizeof(struct hdlc_header));
+ if (hdr->address != HDLC_ADDR_ALLSTATIONS ||
+ hdr->control != HDLC_CTRL_UI)
+ goto rx_error;
+
+ pid = ntohs(hdr->protocol);
+ proto = get_proto(dev, pid);
+ if (!proto) {
+ if (ppp->protos[IDX_LCP].state == OPENED)
+ ppp_tx_cp(dev, PID_LCP, LCP_PROTO_REJ,
+ ++ppp->seq, skb->len + 2, &hdr->protocol);
+ goto rx_error;
+ }
+
+ len = ntohs(cp->len);
+ if (len < sizeof(struct cp_header) /* no complete CP header? */ ||
+ skb->len < len /* truncated packet? */)
+ goto rx_error;
+ skb_pull(skb, sizeof(struct cp_header));
+ len -= sizeof(struct cp_header);
+
+ /* HDLC and CP headers stripped from skb */
+#if DEBUG_CP
+ if (cp->code < CP_CODES)
+ sprintf(debug_buffer, "[%s id 0x%X]", code_names[cp->code],
+ cp->id);
+ else
+ sprintf(debug_buffer, "[code %u id 0x%X]", cp->code, cp->id);
+ ptr = debug_buffer + strlen(debug_buffer);
+ for (i = 0; i < min_t(unsigned int, len, DEBUG_CP); i++) {
+ sprintf(ptr, " %02X", skb->data[i]);
+ ptr += strlen(ptr);
+ }
+ printk(KERN_DEBUG "%s: RX %s %s\n", dev->name, proto_name(pid),
+ debug_buffer);
+#endif
+
+ /* LCP only */
+ if (pid == PID_LCP)
+ switch (cp->code) {
+ case LCP_PROTO_REJ:
+ pid = ntohs(*(__be16*)skb->data);
+ if (pid == PID_LCP || pid == PID_IPCP ||
+ pid == PID_IPV6CP)
+ ppp_cp_event(dev, pid, RXJ_BAD, 0, 0,
+ 0, NULL);
+ goto out;
+
+ case LCP_ECHO_REQ: /* send Echo-Reply */
+ if (len >= 4 && proto->state == OPENED)
+ ppp_tx_cp(dev, PID_LCP, LCP_ECHO_REPLY,
+ cp->id, len - 4, skb->data + 4);
+ goto out;
+
+ case LCP_ECHO_REPLY:
+ if (cp->id == ppp->echo_id)
+ ppp->last_pong = jiffies;
+ goto out;
+
+ case LCP_DISC_REQ: /* discard */
+ goto out;
+ }
+
+ /* LCP, IPCP and IPV6CP */
+ switch (cp->code) {
+ case CP_CONF_REQ:
+ ppp_cp_parse_cr(dev, pid, cp->id, len, skb->data);
+ goto out;
+
+ case CP_CONF_ACK:
+ if (cp->id == proto->cr_id)
+ ppp_cp_event(dev, pid, RCA, 0, 0, 0, NULL);
+ goto out;
+
+ case CP_CONF_REJ:
+ case CP_CONF_NAK:
+ if (cp->id == proto->cr_id)
+ ppp_cp_event(dev, pid, RCN, 0, 0, 0, NULL);
+ goto out;
+
+ case CP_TERM_REQ:
+ ppp_cp_event(dev, pid, RTR, 0, cp->id, 0, NULL);
+ goto out;
+
+ case CP_TERM_ACK:
+ ppp_cp_event(dev, pid, RTA, 0, 0, 0, NULL);
+ goto out;
+
+ case CP_CODE_REJ:
+ ppp_cp_event(dev, pid, RXJ_BAD, 0, 0, 0, NULL);
+ goto out;
+
+ default:
+ len += sizeof(struct cp_header);
+ if (len > dev->mtu)
+ len = dev->mtu;
+ ppp_cp_event(dev, pid, RUC, 0, 0, len, cp);
+ goto out;
+ }
+ goto out;
+
+rx_error:
+ dev_to_hdlc(dev)->stats.rx_errors++;
+out:
+ spin_unlock_irqrestore(&ppp->lock, flags);
+ dev_kfree_skb_any(skb);
+ ppp_tx_flush();
+ return NET_RX_DROP;
+}
+
+
+static void ppp_timer(unsigned long arg)
+{
+ struct proto *proto = (struct proto *)arg;
+ struct ppp *ppp = get_ppp(proto->dev);
+ unsigned long flags;
+
+ spin_lock_irqsave(&ppp->lock, flags);
+ switch (proto->state) {
+ case STOPPING:
+ case REQ_SENT:
+ case ACK_RECV:
+ case ACK_SENT:
+ if (proto->restart_counter) {
+ ppp_cp_event(proto->dev, proto->pid, TO_GOOD, 0, 0,
+ 0, NULL);
+ proto->restart_counter--;
+ } else
+ ppp_cp_event(proto->dev, proto->pid, TO_BAD, 0, 0,
+ 0, NULL);
+ break;
+
+ case OPENED:
+ if (proto->pid != PID_LCP)
+ break;
+ if (time_after(jiffies, ppp->last_pong +
+ ppp->keepalive_timeout * HZ)) {
+ printk(KERN_INFO "%s: Link down\n", proto->dev->name);
+ ppp_cp_event(proto->dev, PID_LCP, STOP, 0, 0, 0, NULL);
+ ppp_cp_event(proto->dev, PID_LCP, START, 0, 0, 0, NULL);
+ } else { /* send keep-alive packet */
+ ppp->echo_id = ++ppp->seq;
+ ppp_tx_cp(proto->dev, PID_LCP, LCP_ECHO_REQ,
+ ppp->echo_id, 0, NULL);
+ proto->timer.expires = jiffies +
+ ppp->keepalive_interval * HZ;
+ add_timer(&proto->timer);
+ }
+ break;
+ }
+ spin_unlock_irqrestore(&ppp->lock, flags);
+ ppp_tx_flush();
+}
+
+
+static void ppp_start(struct net_device *dev)
+{
+ struct ppp *ppp = get_ppp(dev);
+ int i;
+
+ for (i = 0; i < IDX_COUNT; i++) {
+ struct proto *proto = &ppp->protos[i];
+ proto->dev = dev;
+ init_timer(&proto->timer);
+ proto->timer.function = ppp_timer;
+ proto->timer.data = (unsigned long)proto;
+ proto->state = CLOSED;
+ }
+ ppp->protos[IDX_LCP].pid = PID_LCP;
+ ppp->protos[IDX_IPCP].pid = PID_IPCP;
+ ppp->protos[IDX_IPV6CP].pid = PID_IPV6CP;
+
+ ppp_cp_event(dev, PID_LCP, START, 0, 0, 0, NULL);
+}
+
+static void ppp_stop(struct net_device *dev)
+{
+ ppp_cp_event(dev, PID_LCP, STOP, 0, 0, 0, NULL);
+}
static struct hdlc_proto proto = {
- .open = ppp_open,
- .close = ppp_close,
+ .start = ppp_start,
+ .stop = ppp_stop,
.type_trans = ppp_type_trans,
.ioctl = ppp_ioctl,
+ .netif_rx = ppp_rx,
.module = THIS_MODULE,
};
+static const struct header_ops ppp_header_ops = {
+ .create = ppp_hard_header,
+};
static int ppp_ioctl(struct net_device *dev, struct ifreq *ifr)
{
hdlc_device *hdlc = dev_to_hdlc(dev);
+ struct ppp *ppp;
int result;
switch (ifr->ifr_settings.type) {
@@ -110,25 +654,35 @@ static int ppp_ioctl(struct net_device *dev, struct ifreq *ifr)
return 0; /* return protocol only, no settable parameters */
case IF_PROTO_PPP:
- if(!capable(CAP_NET_ADMIN))
+ if (!capable(CAP_NET_ADMIN))
return -EPERM;
- if(dev->flags & IFF_UP)
+ if (dev->flags & IFF_UP)
return -EBUSY;
/* no settable parameters */
- result=hdlc->attach(dev, ENCODING_NRZ,PARITY_CRC16_PR1_CCITT);
+ result = hdlc->attach(dev, ENCODING_NRZ,PARITY_CRC16_PR1_CCITT);
if (result)
return result;
- result = attach_hdlc_protocol(dev, &proto,
- sizeof(struct ppp_state));
+ result = attach_hdlc_protocol(dev, &proto, sizeof(struct ppp));
if (result)
return result;
+
+ ppp = get_ppp(dev);
+ spin_lock_init(&ppp->lock);
+ ppp->req_timeout = 2;
+ ppp->cr_retries = 10;
+ ppp->term_retries = 2;
+ ppp->keepalive_interval = 10;
+ ppp->keepalive_timeout = 60;
+
dev->hard_start_xmit = hdlc->xmit;
+ dev->hard_header_len = sizeof(struct hdlc_header);
+ dev->header_ops = &ppp_header_ops;
dev->type = ARPHRD_PPP;
- netif_dormant_off(dev);
+ netif_dormant_on(dev);
return 0;
}
@@ -138,12 +692,11 @@ static int ppp_ioctl(struct net_device *dev, struct ifreq *ifr)
static int __init mod_init(void)
{
+ skb_queue_head_init(&tx_queue);
register_hdlc_protocol(&proto);
return 0;
}
-
-
static void __exit mod_exit(void)
{
unregister_hdlc_protocol(&proto);
Hi,
I wrote:
> New synchronous PPP implementation for generic HDLC.
So are we leaving the generic-HDLC+PPP in broken state for 2.6.25,
aren't we?
If we do, perhaps a "|| BROKEN" in drivers/net/wan/Kconfig would make
sense? Any attempt to use it will cause kernel panic immediately (PPP
with generic HDLC only; drivers using syncppp.c directly are not
affected). I can make that trivial Kconfig patch of course.
--
Krzysztof Halasa
Krzysztof Halasa wrote:
> Hi,
>
> I wrote:
>
>> New synchronous PPP implementation for generic HDLC.
>
> So are we leaving the generic-HDLC+PPP in broken state for 2.6.25,
> aren't we?
>
> If we do, perhaps a "|| BROKEN" in drivers/net/wan/Kconfig would make
> sense? Any attempt to use it will cause kernel panic immediately (PPP
> with generic HDLC only; drivers using syncppp.c directly are not
> affected). I can make that trivial Kconfig patch of course.
In your original email you said
I guess it should go into 2.6.25, not sure about "stable"
series. I will appreciate any feedback, review and/or test
results.
At the time of the posting 2.6.25-rc6 had already been released, which
seems like an inappropriate time for all that new code, which has been
given so little exposure to real world testing.
Certainly your original message said PPP panics, but without even
minimal testing how do we know that your new code doesn't have equally
problematic issues?
So quite honestly a CONFIG_BROKEN patch might indeed be more appropriate
since generic HDLC works with Frame Relay at least...
Comments? IMO the current code is a known risk (PPP panic, FR works)
whereas the new code is an unknown risk very late in 2.6.25-rc -- but
with a good chance to make HDLC+PPP work again.
Jeff
Jeff Garzik <[email protected]> writes:
> I guess it should go into 2.6.25, not sure about "stable"
> series. I will appreciate any feedback, review and/or test
> results.
Right. Perhaps I should care a bit more about the stable series...
> At the time of the posting 2.6.25-rc6 had already been released, which
> seems like an inappropriate time for all that new code, which has been
> given so little exposure to real world testing.
Sure.
> Certainly your original message said PPP panics, but without even
> minimal testing how do we know that your new code doesn't have equally
> problematic issues?
Well, there was something like "minimal testing", and it doesn't panic
100% like the old code does. But the probability that it won't work
correctly is quite high.
IOW: the new version is certainly better than the old one, though it's
not the normal quality (in terms of testing) I'd like to see.
> So quite honestly a CONFIG_BROKEN patch might indeed be more
> appropriate since generic HDLC works with Frame Relay at least...
Actually Frame Relay and other protocols are not affected, the PPP
patch doesn't change them a bit, it's a different module. The new code
only affect PPP protocol.
I'm fine with the Kconfig patch, actually I'm not sure what is better
at this time - a known broken module marked as such or a new module
with some small chances that it will crash the machine and with much
bigger chances that it won't work with a certain PPP implementation on
the other end.
Something like that?
Untested :-)
Signed-off-by: Krzysztof Halasa <[email protected]>
--- a/drivers/net/wan/Kconfig
+++ b/drivers/net/wan/Kconfig
@@ -150,9 +150,13 @@ config HDLC_FR
config HDLC_PPP
tristate "Synchronous Point-to-Point Protocol (PPP) support"
- depends on HDLC
+ depends on HDLC && BROKEN
help
Generic HDLC driver supporting PPP over WAN connections.
+ This module is currently broken and will cause a kernel panic
+ when a device configured in PPP mode is activated.
+
+ It will be replaced by new PPP implementation in Linux 2.6.26.
If unsure, say N.
From: Krzysztof Halasa <[email protected]>
Date: Tue, 25 Mar 2008 15:39:49 +0100
> > New synchronous PPP implementation for generic HDLC.
>
> So are we leaving the generic-HDLC+PPP in broken state for 2.6.25,
> aren't we?
Kyzysztof, this is the way you behave every time your
patches don't get looked at as quickly as you would
like them to.
And this behavior does not trigger us maintainers to
stop everything we're doing and apply your patches.
In fact it makes us want to review your patches even
less.
Sending a healthy ping asking if your patches need
to be resent, etc., is one thing. But this isn't
what you're doing here.
David Miller <[email protected]> writes:
> Kyzysztof, this is the way you behave every time your
> patches don't get looked at as quickly as you would
> like them to.
>
> And this behavior does not trigger us maintainers to
> stop everything we're doing and apply your patches.
>
> In fact it makes us want to review your patches even
> less.
>
> Sending a healthy ping asking if your patches need
> to be resent, etc., is one thing. But this isn't
> what you're doing here.
I don't know what are you talking about.
- someone broke my drivers, happens from time to time
- I asked about related change and was told about some "out of tree"
drivers instead
- I had two modules to fix, one took 15 minutes and for the other one
I had no time at the moment.
- I finally rewrote the other module
- got feedback, corrected it, resent.
- asked if we are including it in 2.6.25 or if we are to make
temporary Kconfig change instead
What exactly don't you like in my "behaviour"? I didn't invent
synchronous serial devices. Actually, I'm doing it all in my free
time, and I'm sure I can find better things to do in that time.
I accept I may use not the best wording from time to time, I'm not
native English writter.
What is your problem about me?
I'd appreciate it if you point out the precise things I'm doing
wrong.
Thanks.
--
Krzysztof Halasa
Jeff Garzik <[email protected]> writes:
> So quite honestly a CONFIG_BROKEN patch might indeed be more
> appropriate since generic HDLC works with Frame Relay at least...
Actually my question was intended for Andrew as he has already
queued the PPP patch - I probably should have rearranged To/Cc:
headers, sorry about that.
The question still stands: I guess it's fair enough to have either
patch in 2.6.25 (new PPP or "BROKEN" in Kconfig) but I think we
shouldn't leave it as is.
--
Krzysztof Halasa