WAN: new PPP code for generic HDLC - Kernel

This is a discussion on WAN: new PPP code for generic HDLC - Kernel ; 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 ...

+ Reply to Thread
Page 1 of 3 1 2 3 LastLast
Results 1 to 20 of 55

Thread: WAN: new PPP code for generic HDLC

  1. WAN: new PPP code for generic HDLC

    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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. [PATCH] Re: WAN: new PPP code for generic HDLC

    New synchronous PPP implementation for generic HDLC.

    Signed-off-by: Krzysztof Halasa

    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
    + * Copyright (C) 1999 - 2008 Krzysztof Halasa
    *
    * 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
    #include
    #include
    -#include
    -#include
    #include
    -#include
    +#include
    +
    +#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);
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [PATCH] Re: WAN: new PPP code for generic HDLC


    > +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.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [PATCH] Re: WAN: new PPP code for generic HDLC

    Stephen Hemminger 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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [PATCH] Re: WAN: new PPP code for generic HDLC


    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;

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [PATCH] Re: WAN: new PPP code for generic HDLC

    Jan Engelhardt 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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. Re: [PATCH] Re: WAN: new PPP code for generic HDLC


    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.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  8. Re: [PATCH] Re: WAN: new PPP code for generic HDLC

    Krzysztof Halasa 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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  9. [PATCH v2] Re: WAN: new PPP code for generic HDLC

    New synchronous PPP implementation for generic HDLC.

    Signed-off-by: Krzysztof Halasa

    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
    + * Copyright (C) 1999 - 2008 Krzysztof Halasa
    *
    * 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
    #include
    #include
    -#include
    -#include
    #include
    -#include
    +#include
    +
    +#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);
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  10. Re: [PATCH v2] Re: WAN: new PPP code for generic HDLC

    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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  11. Re: [PATCH v2] Re: WAN: new PPP code for generic HDLC

    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



    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  12. Re: [PATCH v2] Re: WAN: new PPP code for generic HDLC

    Jeff Garzik 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

    --- 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.

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  13. Re: [PATCH v2] Re: WAN: new PPP code for generic HDLC

    From: Krzysztof Halasa
    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.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  14. Re: [PATCH v2] Re: WAN: new PPP code for generic HDLC

    David Miller 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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  15. Re: [PATCH v2] Re: WAN: new PPP code for generic HDLC

    Jeff Garzik 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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  16. Re: [PATCH v2] Re: WAN: new PPP code for generic HDLC

    Hi,

    I wrote:

    > 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?


    Linus just made 2.6.25-rclast (I hope), I understand 2.6.25 will have
    broken drivers/net/wan/hdlc_ppp, any chance to apply the temporary
    Kconfig ("depends on BROKEN") patch?

    Would you like me to resend it?

    TIA.
    --
    Krzysztof Halasa
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  17. Re: [PATCH v2] Re: WAN: new PPP code for generic HDLC

    On Fri, 11 Apr 2008 23:35:23 +0200 Krzysztof Halasa wrote:

    > Hi,
    >
    > I wrote:
    >
    > > 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?

    >
    > Linus just made 2.6.25-rclast (I hope), I understand 2.6.25 will have
    > broken drivers/net/wan/hdlc_ppp, any chance to apply the temporary
    > Kconfig ("depends on BROKEN") patch?
    >
    > Would you like me to resend it?


    It never hurts to resend. It was 6000 patches ago and my mind is a blank.

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  18. Re: [PATCH v2] Re: WAN: new PPP code for generic HDLC

    Andrew Morton writes:

    > It never hurts to resend. It was 6000 patches ago and my mind is a blank.


    Sure, here is it. Some description, well:

    PPP support in generic HDLC in Linux 2.6.25 is broken and will cause
    a kernel panic when a device configured in PPP mode is activated.

    It will be replaced by new PPP implementation after Linux 2.6.25 is
    released.

    This affects only PPP support in generic HDLC (mostly Hitachi SCA
    and SCA-II based drivers, wanxl, and few others). Standalone syncppp
    and async PPP support are not affected.

    Untested :-)

    Signed-off-by: Krzysztof Halasa

    --- 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.

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  19. Re: [PATCH v2] Re: WAN: new PPP code for generic HDLC

    Krzysztof Halasa wrote:
    > --- 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.



    applied

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  20. Re: [PATCH v2] Re: WAN: new PPP code for generic HDLC

    Krzysztof Halasa wrote:
    > New synchronous PPP implementation for generic HDLC.
    >
    > Signed-off-by: Krzysztof Halasa
    >
    > 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


    applied


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread
Page 1 of 3 1 2 3 LastLast