* [ATMSAR] Request for review - update #1
@ 2005-09-04 11:05 Giampaolo Tomassoni
2005-09-04 12:00 ` [Linux-ATM-General] " Francois Romieu
` (2 more replies)
0 siblings, 3 replies; 24+ messages in thread
From: Giampaolo Tomassoni @ 2005-09-04 11:05 UTC (permalink / raw)
To: linux-kernel, linux-atm-general
[-- Attachment #1: Type: text/plain, Size: 899 bytes --]
Dears,
thanks to Jiri Slaby who found a bug in the AAL0 handling of the ATMSAR module.
I attach a fixed version of the atmsar patch as a diff against the 2.6.13 kernel tree.
Now the sources are also fully-delethalized by avoiding any line wrap in the Linus' 80-column monitor and the danger due to whitespaces at end of lines is prevented.
However, I'm still hearing for your comments about the usefulness of an ATMSAR layer. I'm also trying hard to get in touch with Duncan Sands (SpeedTouch USB Driver maintainer) and Chas Williams (ATM maintainer) in order to get this patch reviewed and, eventually, committed.
How am I supposed to contact them? I had no reply to the mails I sent them...
Thanks for your interest in this,
-----------------------------------
Giampaolo Tomassoni - IT Consultant
Piazza VIII Aprile 1948, 4
I-53044 Chiusi (SI) - Italy
Ph: +39-0578-21100
[-- Attachment #2: patch-2.6.13+atmsar.diff --]
[-- Type: application/octet-stream, Size: 74792 bytes --]
diff -Nurd linux-2.6.13/MAINTAINERS linux-2.6.13+atmsar/MAINTAINERS
--- linux-2.6.13/MAINTAINERS 2005-08-29 01:41:01.000000000 +0200
+++ linux-2.6.13+atmsar/MAINTAINERS 2005-09-04 12:28:06.000000000 +0200
@@ -370,6 +370,12 @@
W: http://atmelwlandriver.sourceforge.net/
S: Maintained
+ATMSAR SUPPORT
+P: Giampaolo Tomassoni
+M: g.tomassoni@libero.it
+W: http://www.tomassoni.biz
+S: Maintained
+
AUDIT SUBSYSTEM
L: linux-audit@redhat.com (subscribers-only)
S: Maintained
diff -Nurd linux-2.6.13/drivers/atm/Kconfig linux-2.6.13+atmsar/drivers/atm/Kconfig
--- linux-2.6.13/drivers/atm/Kconfig 2005-08-29 01:41:01.000000000 +0200
+++ linux-2.6.13+atmsar/drivers/atm/Kconfig 2005-09-04 11:55:16.000000000 +0200
@@ -444,5 +444,18 @@
Support for the S/UNI-Ultra and S/UNI-622 found in the ForeRunner
HE cards. This driver provides carrier detection some statistics.
-endmenu
+config ATM_SAR
+ tristate "SAR support (EXPERIMENTAL)"
+ depends on ATM && CRC32
+ help
+ This is experimental code which supplies a SAR (Segment And
+ Reassembly) layer to the ATM stack.
+ The SAR layer can be used by various "dumb" ATM devices (notably
+ ADSL modems) whose firmware lacks of SAR capabilities.
+
+ For further details see: <file:include/linux/atmsar.h>
+
+ If unsure, say N.
+
+endmenu
diff -Nurd linux-2.6.13/drivers/atm/Makefile linux-2.6.13+atmsar/drivers/atm/Makefile
--- linux-2.6.13/drivers/atm/Makefile 2005-08-29 01:41:01.000000000 +0200
+++ linux-2.6.13+atmsar/drivers/atm/Makefile 2005-09-04 11:52:51.000000000 +0200
@@ -70,3 +70,6 @@
$(obj)/%.bin $(obj)/%.bin1 $(obj)/%.bin2: $(src)/%.data
objcopy -Iihex $< -Obinary $@.gz
gzip -n -df $@.gz
+
+obj-$(CONFIG_ATM_SAR) += atmsar.o
+atmsar-objs += sar.o saraalr.o saraal0.o saraal5.o
diff -Nurd linux-2.6.13/drivers/atm/sar.c linux-2.6.13+atmsar/drivers/atm/sar.c
--- linux-2.6.13/drivers/atm/sar.c 1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.13+atmsar/drivers/atm/sar.c 2005-09-04 12:34:26.000000000 +0200
@@ -0,0 +1,837 @@
+/* ATM SAR layer
+ *
+ * Copyright (C) 2005 Giampaolo Tomassoni
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * The GNU GPL is contained in /usr/doc/copyright/GPL on a Debian
+ * system and in the file COPYING in the Linux kernel source.
+ */
+
+/*
+ * CREDITS go to the following people:
+ *
+ * - Johan Verrept, for its previous work on a SAR library in Linux;
+ * - Charles Michael Heard for its tutorial on CRC-8 Error Correction.
+ * - Chas Williams for its help in integrating this with the ATM module.
+ *
+ *
+ * Theory of operation and api description in include/linux/atmsar.h
+ */
+#include <linux/version.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/gfp.h>
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/wait.h>
+#include <linux/list.h>
+#include <linux/smp_lock.h>
+#include <linux/init.h>
+#include <linux/errno.h>
+#include <linux/skbuff.h>
+#include <linux/atm.h>
+#include <linux/atmdev.h>
+
+#include <linux/atmsar.h>
+#include "sar.h"
+
+
+#define VERSION "1.0.0"
+
+#undef ATM_SAR_DBG
+
+
+#define SARDEV(atmdev) ((atmsar_dev_t*)((atmdev)->dev_data))
+#define SARVCC(atmvcc) ((atmsar_vcc_t*)((atmvcc)->dev_data))
+
+#define ASAR_MSG "ATMSAR if#%d: "
+
+#define D(v) printk(KERN_ALERT "AtmSar: checkpoint " v " passed\n")
+
+#define VPIVCI(vpi, vci) ( \
+ (((vpi)<<ATM_HDR_VPI_SHIFT)&ATM_HDR_VPI_MASK) | \
+ (((vci)<<ATM_HDR_VCI_SHIFT)&ATM_HDR_VCI_MASK) \
+)
+
+
+typedef enum _HECSTS {
+ HS_OK = 0,
+ HS_CORRECTED,
+ HS_UNCORRECTIBLE
+} HECSTS;
+
+
+#define SRCSKB(skb) (*(struct sk_buff**)(skb)->cb)
+
+
+extern const atmsar_aalops_t opsAALR;
+extern const atmsar_aalops_t opsAAL0;
+extern const atmsar_aalops_t opsAAL5;
+static const atmsar_aalops_t* aOps[] = {
+ &opsAALR,
+ &opsAAL0,
+ &opsAAL5,
+ NULL
+};
+
+
+static int chbad = 0;
+
+module_param(chbad, bool, 0444);
+MODULE_PARM_DESC(
+ chbad,
+ "Treat cells having a correctible header as bad (default=no)"
+);
+
+
+#if 0
+// This is still to be done
+static int stats = 0;
+
+module_param(stats, bool, 0444);
+MODULE_PARM_DESC(
+ stats,
+ "Waste a bit of cpu to maintain useful statistical data"
+);
+#endif
+
+
+static const unsigned char tabSyn[256] = {
+ 0x00, 0x07, 0x0e, 0x09, 0x1c, 0x1b, 0x12, 0x15,
+ 0x38, 0x3f, 0x36, 0x31, 0x24, 0x23, 0x2a, 0x2d,
+ 0x70, 0x77, 0x7e, 0x79, 0x6c, 0x6b, 0x62, 0x65,
+ 0x48, 0x4f, 0x46, 0x41, 0x54, 0x53, 0x5a, 0x5d,
+ 0xe0, 0xe7, 0xee, 0xe9, 0xfc, 0xfb, 0xf2, 0xf5,
+ 0xd8, 0xdf, 0xd6, 0xd1, 0xc4, 0xc3, 0xca, 0xcd,
+ 0x90, 0x97, 0x9e, 0x99, 0x8c, 0x8b, 0x82, 0x85,
+ 0xa8, 0xaf, 0xa6, 0xa1, 0xb4, 0xb3, 0xba, 0xbd,
+
+ 0xc7, 0xc0, 0xc9, 0xce, 0xdb, 0xdc, 0xd5, 0xd2,
+ 0xff, 0xf8, 0xf1, 0xf6, 0xe3, 0xe4, 0xed, 0xea,
+ 0xb7, 0xb0, 0xb9, 0xbe, 0xab, 0xac, 0xa5, 0xa2,
+ 0x8f, 0x88, 0x81, 0x86, 0x93, 0x94, 0x9d, 0x9a,
+ 0x27, 0x20, 0x29, 0x2e, 0x3b, 0x3c, 0x35, 0x32,
+ 0x1f, 0x18, 0x11, 0x16, 0x03, 0x04, 0x0d, 0x0a,
+ 0x57, 0x50, 0x59, 0x5e, 0x4b, 0x4c, 0x45, 0x42,
+ 0x6f, 0x68, 0x61, 0x66, 0x73, 0x74, 0x7d, 0x7a,
+
+ 0x89, 0x8e, 0x87, 0x80, 0x95, 0x92, 0x9b, 0x9c,
+ 0xb1, 0xb6, 0xbf, 0xb8, 0xad, 0xaa, 0xa3, 0xa4,
+ 0xf9, 0xfe, 0xf7, 0xf0, 0xe5, 0xe2, 0xeb, 0xec,
+ 0xc1, 0xc6, 0xcf, 0xc8, 0xdd, 0xda, 0xd3, 0xd4,
+ 0x69, 0x6e, 0x67, 0x60, 0x75, 0x72, 0x7b, 0x7c,
+ 0x51, 0x56, 0x5f, 0x58, 0x4d, 0x4a, 0x43, 0x44,
+ 0x19, 0x1e, 0x17, 0x10, 0x05, 0x02, 0x0b, 0x0c,
+ 0x21, 0x26, 0x2f, 0x28, 0x3d, 0x3a, 0x33, 0x34,
+
+ 0x4e, 0x49, 0x40, 0x47, 0x52, 0x55, 0x5c, 0x5b,
+ 0x76, 0x71, 0x78, 0x7f, 0x6a, 0x6d, 0x64, 0x63,
+ 0x3e, 0x39, 0x30, 0x37, 0x22, 0x25, 0x2c, 0x2b,
+ 0x06, 0x01, 0x08, 0x0f, 0x1a, 0x1d, 0x14, 0x13,
+ 0xae, 0xa9, 0xa0, 0xa7, 0xb2, 0xb5, 0xbc, 0xbb,
+ 0x96, 0x91, 0x98, 0x9f, 0x8a, 0x8d, 0x84, 0x83,
+ 0xde, 0xd9, 0xd0, 0xd7, 0xc2, 0xc5, 0xcc, 0xcb,
+ 0xe6, 0xe1, 0xe8, 0xef, 0xfa, 0xfd, 0xf4, 0xf3
+};
+
+#define COSET_LEADER 0x55
+
+
+#if defined(__i386__) || defined(__x86_64__)
+static unsigned char inline hecCompute(const unsigned char* hdr) {
+ register unsigned char hec;
+
+ __asm__ (
+ "\
+ movl (%%edx), %%ecx;\n\
+ movb %%cl, %%al; shrl $8, %%ecx; xlatb\n\
+ xorb %%cl, %%al; shrl $8, %%ecx; xlatb\n\
+ xorb %%cl, %%al; shrl $8, %%ecx; xlatb\n\
+ xorb %%cl, %%al; xlatb\n\
+ xorb $0x55, %%al\n\
+ "
+ :
+ "=a" (hec)
+ :
+ "b" (tabSyn),
+ "d" (hdr)
+ :
+ "ecx"
+ );
+
+ return(hec);
+}
+#else
+static unsigned char inline hecCompute(const unsigned char* hdr) {
+ register unsigned char hec;
+
+ hec = tabSyn[*hdr++];
+ hec = tabSyn[hec ^ (*hdr++)];
+ hec = tabSyn[hec ^ (*hdr++)];
+ hec = tabSyn[hec ^ *hdr];
+ return(hec ^ COSET_LEADER);
+}
+#endif
+
+static void inline hecGenerate(unsigned char* hdr)
+{ hdr[4] = hecCompute(hdr); }
+
+
+#define SYN_OK 0xff
+#define SYN_ER 0x80
+
+static const unsigned char tabPos[256] = {
+ SYN_OK, 39, 38, SYN_ER, 37, SYN_ER, SYN_ER, 31,
+ 36, SYN_ER, SYN_ER, 8, SYN_ER, SYN_ER, 30, SYN_ER,
+ 35, SYN_ER, SYN_ER, SYN_ER, SYN_ER, 23, 7, SYN_ER,
+ SYN_ER, SYN_ER, SYN_ER, SYN_ER, 29, SYN_ER, SYN_ER, SYN_ER,
+ 34, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER,
+ SYN_ER, SYN_ER, 22, SYN_ER, 6, SYN_ER, SYN_ER, SYN_ER,
+ SYN_ER, 0, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER,
+ 28, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER,
+
+ 33, SYN_ER, SYN_ER, 10, SYN_ER, SYN_ER, SYN_ER, SYN_ER,
+ SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER,
+ SYN_ER, 12, SYN_ER, SYN_ER, 21, SYN_ER, SYN_ER, 19,
+ 5, SYN_ER, SYN_ER, 17, SYN_ER, SYN_ER, SYN_ER, SYN_ER,
+ SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, 3,
+ SYN_ER, SYN_ER, SYN_ER, 15, SYN_ER, SYN_ER, SYN_ER, SYN_ER,
+ 27, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER,
+ SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER,
+
+ 32, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, 9, SYN_ER,
+ SYN_ER, 24, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER,
+ SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER,
+ SYN_ER, SYN_ER, SYN_ER, 1, SYN_ER, SYN_ER, SYN_ER, SYN_ER,
+ SYN_ER, SYN_ER, 11, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER,
+ 20, SYN_ER, SYN_ER, 13, SYN_ER, SYN_ER, 18, SYN_ER,
+ 4, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, 16, SYN_ER,
+ SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER,
+
+ SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, 25,
+ SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, 2, SYN_ER,
+ SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, 14, SYN_ER,
+ SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER,
+ 26, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER,
+ SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER,
+ SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER,
+ SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER, SYN_ER
+};
+
+
+static void putHdrNoHec(struct sk_buff* skb, unsigned hdr)
+{ *(unsigned*)skb_put(skb, sizeof(hdr)) = htonl(hdr); }
+
+static void putHdrWithHec(struct sk_buff* skb, unsigned hdr) {
+ unsigned char* buf = skb_put(skb, sizeof(hdr) + 1);
+
+ *(unsigned*)buf = htonl(hdr);
+ hecGenerate(buf);
+}
+
+
+static void inline txSkbFree(struct sk_buff *skbSrc) {
+ struct atm_vcc *vcc = ATM_SKB(skbSrc)->vcc;
+ if(vcc != NULL && vcc->pop != NULL)
+ vcc->pop(vcc, skbSrc);
+ else
+ kfree_skb(skbSrc);
+}
+
+
+#if defined(ATM_SAR_DBG)
+static void inline dumpCell(char* dst, const ATM_CELL* cell) {
+ const unsigned char *src = (unsigned char*)cell;
+ int n;
+ for(n = 52; --n >= 0; dst += 2)
+ sprintf(dst, "%02x", *src++);
+}
+#endif
+
+static int atmProcRead(struct atm_dev *atmdev, loff_t *pos, char *page) {
+ atmsar_dev_t* dev = SARDEV(atmdev);
+ loff_t skip = *pos;
+
+ if(skip-- == 0)
+ return(sprintf(page, "sarver:\t" VERSION "\n"));
+
+ if(skip-- == 0) {
+ strcpy(page, "signal:\t");
+ switch(atmdev->signal) {
+ case ATM_PHY_SIG_LOST:
+ strcat(page, "down\n");
+ break;
+
+ case ATM_PHY_SIG_FOUND:
+ strcat(page, "synched\n");
+ break;
+
+ default:
+ strcat(page, "unknown\n");
+ break;
+ }
+
+ return(strlen(page));
+ }
+
+ if(skip-- == 0) {
+ strcpy(page, "dnrate:\t");
+ if(dev->rx_speed != ATMSAR_SPEED_UNSPEC)
+ sprintf(
+ &page[strlen(page)],
+ "%ld kbps\n",
+ dev->rx_speed
+ );
+ else
+ strcat(page, "unknown\n");
+ return(strlen(page));
+ }
+
+ if(skip-- == 0) {
+ strcpy(page, "uprate:\t");
+ if(dev->tx_speed != ATMSAR_SPEED_UNSPEC)
+ sprintf(
+ &page[strlen(page)],
+ "%ld kbps\n",
+ dev->tx_speed
+ );
+ else
+ strcat(page, "unknown\n");
+ return(strlen(page));
+ }
+
+#if defined(ATM_SAR_DBG)
+ if(skip-- == 0) {
+ strcpy(page, " rxlst:\t");
+ dumpCell(&page[strlen(page)], &dev->cellLastRx);
+ strcat(page, "\n");
+ return(strlen(page));
+ }
+
+ if(skip-- == 0) {
+ strcpy(page, " txlst:\t");
+ dumpCell(&page[strlen(page)], &dev->cellLastTx);
+ strcat(page, "\n");
+ return(strlen(page));
+ }
+#endif
+
+ if(skip-- == 0)
+ return(sprintf(page, " vccs:\t%d\n", dev->vccs_count));
+
+ if(dev->ops->proc_read != NULL)
+ return(dev->ops->proc_read(dev, &skip, page));
+
+ return(0);
+}
+
+static int atmIOCtl(struct atm_dev *atmdev, unsigned int cmd, void __user *arg) {
+ atmsar_dev_t *dev = SARDEV(atmdev);
+ if(dev->ops->ioctl != NULL)
+ return(dev->ops->ioctl(dev, cmd, arg));
+
+ return(-EINVAL);
+}
+
+
+/**
+ * __atmSend - Sends a packet out from our device
+ * @vcc: the atm_vcc from which the send had been invoked
+ * @skb: the data to be sent, formatted according to the vcc AAL
+ *
+ * atmSend is our implementation of the atm_vcc's send op.
+ *
+ * The ATM code invokes this function during process context, but it seems
+ * we are not supposed to use schedule() to pace the transmission.
+ * What we do is to create a further skb which will contain the AAL0 (raw)
+ * version of the skb sent to us, as well as a pointer to the original
+ * skb. When data will be delivered out of the device, the original skb
+ * will be released. This way, the sock implementation of the ATM code
+ * will (should?) take care of tx pacing and such.
+ */
+static int atmSend(struct atm_vcc* vcc, struct sk_buff* skbSrc) {
+ atmsar_vcc_t* sar = SARVCC(vcc);
+ atmsar_dev_t* dev = SARDEV(vcc->dev);
+ int ecode;
+
+ if(dev->atmdev->signal == ATM_PHY_SIG_FOUND) {
+ ecode = sar->ops->encodeGetCellCount(sar, skbSrc);
+ if(ecode > 0) {
+ struct sk_buff *skb = alloc_skb(
+ dev->tx_head_reserve
+ + ecode*dev->tx_cell_size
+ + dev->tx_tail_reserve,
+ GFP_ATOMIC
+ );
+ if(skb != NULL) {
+ // Attaches source skb
+ SRCSKB(skb) = skbSrc;
+
+ // Reserve header space
+ skb_reserve(skb, dev->tx_head_reserve);
+
+ // Encode packet into cells
+ sar->ops->encode(
+ skb,
+ (dev->flags&ATMSAR_FLG_53BYTE_CELL) ?
+ putHdrWithHec
+ :
+ putHdrNoHec,
+ sar,
+ skbSrc
+ );
+
+ // Queue it up
+ if(dev->ops->tx_restart != NULL) {
+ unsigned long sts;
+ int needsTxWakeup;
+
+ spin_lock_irqsave(
+ &dev->skq_tx.lock,
+ sts
+ );
+ needsTxWakeup = skb_queue_empty(
+ &dev->skq_tx
+ );
+ __skb_queue_tail(&dev->skq_tx, skb);
+ spin_unlock_irqrestore(
+ &dev->skq_tx.lock,
+ sts
+ );
+
+ // Restart tx if needed
+ if(needsTxWakeup)
+ dev->ops->tx_restart(dev);
+ } else
+ skb_queue_tail(&dev->skq_tx, skb);
+
+ // Done with it
+ return(0);
+ } else
+ // No more memory
+ ecode = -ENOMEM;
+ }
+
+ // Is it right to report this?
+ atomic_inc(&vcc->stats->tx_err);
+ } else
+ // Well, few ideas...
+ // We refuse the packet giving back an ENOLINK.
+ // Is it wrong? What are we supposed to do?
+ ecode = -ENOLINK;
+
+
+ txSkbFree(skbSrc);
+ return(ecode);
+}
+
+
+static inline int ocMutexTry(atmsar_dev_t* dev) {
+ if(atomic_dec_and_test(&dev->mtx_openclose))
+ // Mutex acquired!
+ return(1);
+
+ // Mutex acquired by someone else. Release
+ // our attempt and fail try.
+ atomic_inc(&dev->mtx_openclose);
+ return(0);
+}
+
+
+static inline atmsar_vcc_t** GetSARPtr(atmsar_dev_t* dev, unsigned vpivci) {
+ atmsar_vcc_t **psar;
+ for(
+ psar = &dev->vccs_hash[vpivci%ATMSAR_N_HASHVCCS];
+ *psar != NULL && (*psar)->vpivci != vpivci;
+ psar = &(*psar)->next
+ );
+
+ return(psar);
+}
+
+
+static void atmClose(struct atm_vcc *vcc) {
+ atmsar_dev_t *dev = SARDEV(vcc->dev);
+ const unsigned vpivci = VPIVCI(vcc->vpi, vcc->vci);
+ atmsar_vcc_t **psar;
+
+ clear_bit(ATM_VF_READY, &vcc->flags);
+
+ // Wait for Open/Close mutex acquisition
+ wait_event(dev->wqh_openclose, ocMutexTry(dev));
+
+ psar = GetSARPtr(dev, vpivci);
+ if(unlikely(*psar == NULL))
+ // We have no sar pointer for this vcc, so this vcc wasn't
+ // opened with us or it had been already closed.
+ printk(
+ KERN_ERR
+ ASAR_MSG "atm close on an unknown vpi/vci (%d/%d)\n",
+ dev->atmdev->number,
+ vcc->vpi, vcc->vci
+ );
+ else if(unlikely((*psar)->vcc != vcc))
+ // The vcc we know being assigned to this vpi/vci pair is
+ // not the one for which a close has been asked.
+ // This is quite a weird error, since the atm layer should
+ // ensure vcc uniqueness on vpi/vci...
+ printk(
+ KERN_ERR
+ ASAR_MSG "atm close on an unmatched vpi/vci (%d/%d)\n",
+ dev->atmdev->number,
+ vcc->vpi, vcc->vci
+ );
+ else {
+ atmsar_vcc_t *sar = *psar;
+ unsigned long sts;
+ int lastClose;
+
+ write_lock_irqsave(&dev->rwl_vccs, sts);
+ *psar = sar->next;
+ lastClose = (--dev->vccs_count == 0);
+ write_unlock_irqrestore(&dev->rwl_vccs, sts);
+
+ sar->ops->end(sar);
+ kfree(sar);
+ if(lastClose && dev->ops->last_close != NULL)
+ // Last close.
+ // There is no spinlock hold and local irqs are in
+ // normal shape, so our client may schedule() and do
+ // (almost) whatever he/she wants...
+ dev->ops->last_close(dev);
+ }
+
+ clear_bit(ATM_VF_ADDR, &vcc->flags);
+
+ // Yield control to next open/close function
+ atomic_inc(&dev->mtx_openclose);
+ wake_up(&dev->wqh_openclose);
+}
+
+
+static inline int __atmOpen(atmsar_dev_t* dev, struct atm_vcc* vcc) {
+ const atmsar_aalops_t** ops;
+ int ecode;
+
+ for(ops = aOps; *ops != NULL && (*ops)->aal != vcc->qos.aal; ++ops);
+ if(*ops == NULL)
+ // We don't handle this aal, yet...
+ ecode = -ENOTSUPP;
+ else if(vcc->vpi == ATM_VPI_UNSPEC || vcc->vci == ATM_VCI_UNSPEC)
+ // We need an address to be spec.
+ ecode = -EDESTADDRREQ;
+ else {
+ const unsigned vpivci = VPIVCI(vcc->vpi, vcc->vci);
+ atmsar_vcc_t **psar = GetSARPtr(dev, vpivci);
+
+ if(*psar == NULL) {
+ atmsar_vcc_t *sar = (*ops)->init(vcc);
+ if(sar != NULL) {
+ // There is no spinlock hold and local irqs are
+ // in normal shape, so our client may schedule()
+ // and do (almost) whatever it wants...
+ if(
+ dev->vccs_count != 0 ||
+ dev->ops->first_open == NULL ||
+ (ecode = dev->ops->first_open(dev)) >= 0
+ ) {
+ unsigned long sts;
+
+ sar->vpivci = vpivci;
+
+ sar->dev = dev;
+ sar->vcc = vcc;
+ sar->ops = *ops;
+
+ write_lock_irqsave(&dev->rwl_vccs, sts);
+ sar->next = *psar;
+ *psar = sar;
+ ++dev->vccs_count;
+ write_unlock_irqrestore(
+ &dev->rwl_vccs,
+ sts
+ );
+
+ vcc->dev_data = sar;
+
+ // Soldout!
+ set_bit(ATM_VF_ADDR, &vcc->flags);
+ set_bit(ATM_VF_READY, &vcc->flags);
+ return(0);
+ }
+
+ (*ops)->end(sar);
+ kfree(sar);
+ } else
+ // Most possible cause.
+ ecode = -ENOMEM;
+ } else
+ // VPI/VCI already present. This case shouln't be, since
+ // the atm layer handles this for us. Anyway...
+ ecode = -EADDRINUSE;
+ }
+
+ return(ecode);
+}
+
+static int atmOpen(struct atm_vcc *vcc) {
+ atmsar_dev_t *dev = SARDEV(vcc->dev);
+ int ecode;
+
+ // Wait for Open/Close mutex acquisition
+ wait_event(dev->wqh_openclose, ocMutexTry(dev));
+
+ // Invoke the "true" open
+ ecode = __atmOpen(dev, vcc);
+
+ // Yield control to next open/close function
+ atomic_inc(&dev->mtx_openclose);
+ wake_up(&dev->wqh_openclose);
+
+ return(ecode);
+}
+
+
+static const struct atmdev_ops atmops = {
+ .owner = THIS_MODULE,
+
+ .open = atmOpen,
+ .close = atmClose,
+ .send = atmSend,
+ .ioctl = atmIOCtl,
+
+ .proc_read = atmProcRead
+};
+
+
+void atmsar_tx_skb_complete(struct sk_buff *skb) {
+ struct sk_buff* skbSrc = SRCSKB(skb);
+
+ atomic_inc(&ATM_SKB(skbSrc)->vcc->stats->tx);
+
+ txSkbFree(skbSrc);
+ kfree_skb(skb);
+}
+
+void atmsar_tx_skb_discard(struct sk_buff *skb) {
+ struct sk_buff* skbSrc = SRCSKB(skb);
+
+ atomic_inc(&ATM_SKB(skbSrc)->vcc->stats->tx_err);
+
+ txSkbFree(skbSrc);
+ kfree_skb(skb);
+}
+
+void atmsar_tx_purge(atmsar_dev_t* dev) {
+ struct sk_buff *skb;
+ unsigned long sts;
+
+ spin_lock_irqsave(&dev->skq_tx.lock, sts);
+ while((skb = __skb_dequeue(&dev->skq_tx)) != NULL)
+ atmsar_tx_skb_discard(skb);
+ spin_unlock_irqrestore(&dev->skq_tx.lock, sts);
+}
+
+
+void atmsar_rx_purge(atmsar_dev_t *dev) {
+ int i;
+ unsigned long sts;
+
+ read_lock_irqsave(&dev->rwl_vccs, sts);
+ for(i = 0; i < ATMSAR_N_HASHVCCS; ++i) {
+ atmsar_vcc_t* sar;
+ for(sar = dev->vccs_hash[i]; sar != NULL; sar = sar->next)
+ sar->ops->decodeReset(sar);
+ }
+ read_unlock_irqrestore(&dev->rwl_vccs, sts);
+}
+
+static void sarDecode(
+ atmsar_dev_t* dev,
+ unsigned hdr,
+ const void* payload
+) {
+ atmsar_vcc_t* sar;
+ unsigned long sts;
+
+ read_lock_irqsave(&dev->rwl_vccs, sts);
+ sar = *GetSARPtr(dev, hdr&(ATM_HDR_VPI_MASK|ATM_HDR_VCI_MASK));
+ if(sar != NULL) {
+ struct sk_buff* skb = sar->ops->decode(sar, hdr, payload);
+ read_unlock_irqrestore(&dev->rwl_vccs, sts);
+
+ if(skb != NULL) {
+ // We got a (reassembled) packet.
+ if(likely(atm_charge(sar->vcc, skb->truesize))) {
+ // Push it up to upper layers
+ sar->vcc->push(sar->vcc, skb);
+ atomic_inc(&sar->vcc->stats->rx);
+ } else
+ // atm_charge() increments rx_drop
+ dev_kfree_skb(skb);
+ }
+ } else
+ // An 'empty' cell (which are mostly tagged by a (0,0)
+ // (VPI,VCI), but this may be device-specific), or even
+ // a cell for an unopened vcc.
+ // We just discard it without any further notification.
+ read_unlock_irqrestore(&dev->rwl_vccs, sts);
+}
+
+void atmsar_rx_decode_52bytes(atmsar_dev_t* dev, const void* cell)
+{ sarDecode(dev, ntohl(*(unsigned*)cell), &((char*)cell)[4]); }
+
+void atmsar_rx_decode_53bytes(atmsar_dev_t* dev, const void* cell) {
+ const unsigned posErr =
+ tabPos[hecCompute(cell) ^ ((unsigned char*)cell)[4]]
+ ;
+ unsigned hdr;
+
+ if(posErr == SYN_OK)
+ hdr = ntohl(*(unsigned*)cell);
+ else if(posErr == SYN_ER || !chbad) {
+ // Cell discarded due to hec error.
+ atomic_inc(&dev->atmdev->stats.aal0.rx_err);
+ return;
+ } else
+ // Correcting header error
+ hdr = ntohl(*(unsigned*)cell)^(0x80000000 >> (posErr%8));
+
+ // Decode!
+ sarDecode(dev, hdr, &((char*)cell)[5]);
+}
+
+void atmsar_rx_decode_53bytes_skiphec(
+ atmsar_dev_t* dev,
+ const void* cell
+) { sarDecode(dev, ntohl(*(unsigned*)cell), &((char*)cell)[5]); }
+
+
+void atmsar_dev_deregister(atmsar_dev_t *dev) {
+ int i;
+
+ atm_dev_deregister(dev->atmdev);
+
+ for(i = 0; i < ATMSAR_N_HASHVCCS; ++i)
+ while(dev->vccs_hash[i] != NULL)
+ atmClose(dev->vccs_hash[i]->vcc);
+
+ kfree(dev);
+}
+
+
+static const atmsar_ops_t nullOps = { NULL };
+
+atmsar_dev_t* atmsar_dev_register(
+ const char *name,
+ const unsigned char *esi,
+ const atmsar_ops_t *ops,
+ const struct atmphy_ops *phy_ops,
+ unsigned flags,
+ size_t tx_head_reserve,
+ size_t tx_tail_reserve,
+ size_t tx_cell_size
+) {
+ const size_t szCell =
+ (flags&ATMSAR_FLG_53BYTE_CELL) ? ATM_CELL_SIZE : ATM_AAL0_SDU
+ ;
+ if(
+ (flags&~(ATMSAR_FLG_53BYTE_CELL)) == 0 &&
+ tx_head_reserve >= 0 &&
+ tx_tail_reserve >= 0 &&
+ tx_cell_size >= szCell
+ ) {
+ atmsar_dev_t* dev = kmalloc(sizeof(*dev), GFP_KERNEL);
+ if(dev != NULL) {
+ memset(dev, 0, sizeof(*dev));
+
+ dev->atmdev = atm_dev_register(name, &atmops, -1, NULL);
+ if(dev->atmdev != NULL) {
+ if(esi != NULL)
+ memcpy(dev->atmdev->esi, esi, ESI_LEN);
+
+ // The 'classical' sizes for vpi/vci fields.
+ // Since we are using this module, our hardware
+ // has seldom any preference about this. This
+ // not holding, just change these fields right
+ // after your call to atmsar_dev_register()
+ dev->atmdev->ci_range.vpi_bits = 8;
+ dev->atmdev->ci_range.vci_bits = 16;
+
+ dev->atmdev->phy = phy_ops;
+ dev->atmdev->dev_data = dev;
+
+ dev->data = NULL;
+ dev->flags = flags;
+ dev->ops = ops != NULL ? ops : &nullOps;
+
+ // Parameters for tx buffers adjusting
+ dev->tx_head_reserve = tx_head_reserve;
+ dev->tx_tail_reserve = tx_tail_reserve;
+ dev->tx_cell_size = tx_cell_size;
+ dev->tx_cell_gap = tx_cell_size - szCell;
+
+ memset(dev->vccs_hash, 0, sizeof(dev->vccs_hash));
+ dev->rwl_vccs = RW_LOCK_UNLOCKED;
+
+ atomic_set(&dev->mtx_openclose, 1);
+ init_waitqueue_head(&dev->wqh_openclose);
+
+ skb_queue_head_init(&dev->skq_tx);
+
+ dev->rx_speed = ATMSAR_SPEED_UNSPEC;
+ dev->tx_speed = ATMSAR_SPEED_UNSPEC;
+
+ // Sold out!
+ return(dev);
+ }
+
+ kfree(dev);
+ }
+ }
+
+ return(NULL);
+}
+
+
+static int __init initModule(void) {
+ printk(KERN_INFO "ATMSAR module v." VERSION " loaded.\n");
+ return(0);
+}
+
+module_init(initModule);
+
+
+static void __exit endModule(void)
+{ printk(KERN_INFO "ATMSAR module removed.\n"); }
+
+module_exit(endModule);
+
+
+EXPORT_SYMBOL_GPL(atmsar_tx_skb_complete);
+EXPORT_SYMBOL_GPL(atmsar_tx_skb_discard);
+EXPORT_SYMBOL_GPL(atmsar_tx_purge);
+EXPORT_SYMBOL_GPL(atmsar_rx_purge);
+EXPORT_SYMBOL_GPL(atmsar_rx_decode_52bytes);
+EXPORT_SYMBOL_GPL(atmsar_rx_decode_53bytes);
+EXPORT_SYMBOL_GPL(atmsar_rx_decode_53bytes_skiphec);
+EXPORT_SYMBOL_GPL(atmsar_dev_deregister);
+EXPORT_SYMBOL_GPL(atmsar_dev_register);
+
+MODULE_DESCRIPTION("An ATM SAR library for AAL-unaware ATM devices");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Giampaolo Tomassoni");
+MODULE_VERSION(VERSION);
diff -Nurd linux-2.6.13/drivers/atm/sar.h linux-2.6.13+atmsar/drivers/atm/sar.h
--- linux-2.6.13/drivers/atm/sar.h 1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.13+atmsar/drivers/atm/sar.h 2005-09-04 12:08:25.000000000 +0200
@@ -0,0 +1,81 @@
+/* ATM SAR layer
+ *
+ * Copyright (C) 2005 Giampaolo Tomassoni
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * The GNU GPL is contained in /usr/doc/copyright/GPL on a Debian
+ * system and in the file COPYING in the Linux kernel source.
+ */
+
+/*
+ * CREDITS go to the following people:
+ *
+ * - Johan Verrept, for its previous work on a SAR library in Linux;
+ * - Charles Michael Heard for its tutorial on CRC-8 Error Correction.
+ * - Chas Williams for its help in integrating this with the ATM module.
+ *
+ *
+ * Theory of operation and api description in include/linux/atmsar.h
+ */
+#if !defined(_SAR_H__INCLUDED)
+#define _SAR_H__INCLUDED
+
+
+#define SZAALENCODESTS 32
+
+struct atmsar_vcc_base {
+ struct atmsar_vcc_base *next;
+ unsigned vpivci;
+
+ atmsar_dev_t *dev;
+ struct atm_vcc *vcc;
+ const atmsar_aalops_t *ops;
+};
+
+typedef void SARPUTHDR(struct sk_buff* skb, unsigned hdr);
+
+typedef atmsar_vcc_t* AALINIT(struct atm_vcc* vcc);
+typedef void AALEND(atmsar_vcc_t* sar);
+typedef void AALDECODERRESET(atmsar_vcc_t* sar);
+typedef struct sk_buff* AALDECODER(
+ atmsar_vcc_t* sar,
+ unsigned hdr,
+ const void* payload
+);
+typedef int AALENCODERGCC(atmsar_vcc_t* sar, struct sk_buff* skbIn);
+typedef void AALENCODER(
+ struct sk_buff* skbOut,
+ SARPUTHDR* putHdr,
+ atmsar_vcc_t* sar,
+ struct sk_buff* skbIn
+);
+
+struct atmsar_aalops {
+ unsigned char aal;
+
+ char* name;
+
+ AALINIT* init;
+ AALEND* end;
+
+ AALDECODERRESET* decodeReset;
+ AALDECODER* decode;
+
+ AALENCODERGCC* encodeGetCellCount;
+ AALENCODER* encode;
+};
+
+#endif // defined(_SAR_H__INCLUDED)
diff -Nurd linux-2.6.13/drivers/atm/saraal0.c linux-2.6.13+atmsar/drivers/atm/saraal0.c
--- linux-2.6.13/drivers/atm/saraal0.c 1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.13+atmsar/drivers/atm/saraal0.c 2005-09-04 12:34:36.000000000 +0200
@@ -0,0 +1,148 @@
+/* ATM SAR layer
+ *
+ * Copyright (C) 2005 Giampaolo Tomassoni
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * The GNU GPL is contained in /usr/doc/copyright/GPL on a Debian
+ * system and in the file COPYING in the Linux kernel source.
+ */
+
+/*
+ * CREDITS go to the following people:
+ *
+ * - Johan Verrept, for its previous work on a SAR library in Linux;
+ * - Charles Michael Heard for its tutorial on CRC-8 Error Correction.
+ * - Chas Williams for its help in integrating this with the ATM module.
+ *
+ *
+ * Theory of operation and api description in include/linux/atmsar.h
+ */
+#include <linux/version.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/gfp.h>
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/wait.h>
+#include <linux/list.h>
+#include <linux/smp_lock.h>
+#include <linux/init.h>
+#include <linux/errno.h>
+#include <linux/skbuff.h>
+#include <linux/atm.h>
+#include <linux/atmdev.h>
+
+#include <linux/atmsar.h>
+#include "sar.h"
+
+
+static int aal0EncodeGetCellCount(
+ atmsar_vcc_t* sar,
+ struct sk_buff* skbIn
+) {
+ atomic_inc(&sar->vcc->stats->tx);
+ return((skbIn->len + ATM_CELL_PAYLOAD - 1)/ATM_CELL_PAYLOAD);
+}
+
+static void aal0Encode(
+ struct sk_buff* skbOut,
+ SARPUTHDR* putHdr,
+ atmsar_vcc_t* sar,
+ struct sk_buff* skbIn
+) {
+ size_t len = skbIn->len;
+ unsigned hdr = sar->vpivci;
+
+ if(len == 0) {
+ putHdr(skbOut, hdr|(ATM_PTI_US1|ATM_HDR_PTI_SHIFT));
+ memset(skb_put(skbOut, ATM_CELL_PAYLOAD), 0, ATM_CELL_PAYLOAD);
+ } else {
+ unsigned char* payload;
+
+ do {
+ if(len > ATM_CELL_PAYLOAD)
+ len = ATM_CELL_PAYLOAD;
+ else
+ hdr |= ATM_PTI_US1<<ATM_HDR_PTI_SHIFT;
+
+ putHdr(skbOut, hdr);
+
+ payload = skb_put(skbOut, ATM_CELL_PAYLOAD);
+
+ if(sar->dev->tx_cell_gap != 0)
+ memset(
+ skb_put(skbOut, sar->dev->tx_cell_gap),
+ 0,
+ sar->dev->tx_cell_gap
+ );
+
+ if(len > 0)
+ memcpy(payload, skb_pull(skbIn, len), len);
+
+ len = skbIn->len;
+ } while(len != 0);
+
+ if(len < ATM_CELL_PAYLOAD)
+ memset(&payload[len], 0, ATM_CELL_PAYLOAD - len);
+ }
+}
+
+
+static void aal0DecodeReset(atmsar_vcc_t* sar)
+{}
+
+static struct sk_buff* aal0Decode(
+ atmsar_vcc_t* sar,
+ unsigned hdr,
+ const void* payload
+) {
+ struct sk_buff *skb = dev_alloc_skb(ATM_CELL_PAYLOAD);
+ if(skb == NULL) {
+ atomic_inc(&sar->vcc->stats->rx_drop);
+ return(NULL);
+ }
+
+ memcpy(skb_put(skb, ATM_CELL_PAYLOAD), payload, ATM_CELL_PAYLOAD);
+ atomic_inc(&sar->vcc->stats->rx);
+ return(skb);
+}
+
+
+static atmsar_vcc_t* aal0Init(struct atm_vcc* vcc) {
+ atmsar_vcc_t* sar = (atmsar_vcc_t*)kmalloc(sizeof(*sar), GFP_KERNEL);
+ if(sar != NULL)
+ memset(sar, 0, sizeof(*sar));
+
+ return(sar);
+}
+
+static void aal0End(atmsar_vcc_t* sarvcc)
+{/* kfree() in atmsarmod.c */}
+
+
+const atmsar_aalops_t opsAAL0 = {
+ ATM_NO_AAL,
+ "0",
+
+ aal0Init,
+ aal0End,
+
+ aal0DecodeReset,
+ aal0Decode,
+
+ aal0EncodeGetCellCount,
+ aal0Encode
+};
diff -Nurd linux-2.6.13/drivers/atm/saraal5.c linux-2.6.13+atmsar/drivers/atm/saraal5.c
--- linux-2.6.13/drivers/atm/saraal5.c 1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.13+atmsar/drivers/atm/saraal5.c 2005-09-04 12:21:11.000000000 +0200
@@ -0,0 +1,445 @@
+/* ATM SAR layer
+ *
+ * Copyright (C) 2005 Giampaolo Tomassoni
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * The GNU GPL is contained in /usr/doc/copyright/GPL on a Debian
+ * system and in the file COPYING in the Linux kernel source.
+ */
+
+/*
+ * CREDITS go to the following people:
+ *
+ * - Johan Verrept, for its previous work on a SAR library in Linux;
+ * - Charles Michael Heard for its tutorial on CRC-8 Error Correction.
+ * - Chas Williams for its help in integrating this with the ATM module.
+ *
+ *
+ * Theory of operation and api description in include/linux/atmsar.h
+ */
+#include <linux/version.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/gfp.h>
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/wait.h>
+#include <linux/list.h>
+#include <linux/smp_lock.h>
+#include <linux/init.h>
+#include <linux/errno.h>
+#include <linux/skbuff.h>
+#include <linux/crc32.h>
+#include <linux/atm.h>
+#include <linux/atmdev.h>
+
+#include <linux/atmsar.h>
+#include "sar.h"
+
+
+/* SZ_FIRST_AAL5SDU
+ * This is the initial size of an AAL5 assembly buffer.
+ * The initial size is large enough to allow for common AAL5 SDUs, ie: the
+ * more-or-less 1544 bytes of an IP-Over-ATM link.
+ * If the receiving packets and the vcc MTU dictate for a greater SDU,
+ * the buffer is allowed to progressively double in size up to 64KB at cell
+ * reception until the SDU needs are satisfied or the vcc MTU limit is reached.
+ * This buffer is basicly per-vcc and is released at vcc close, so at any
+ * time its size is due to the largest SDU received compatible with the MTU.
+ */
+#define SZ_FIRST_AAL5SDU (2*1024)
+
+/* CRC32_REMAINDER
+ * This is the remainder 32-bit word expected to be returned by the crc32
+ * function on valid ATM SDU.
+ */
+#define CRC32_REMAINDER 0xc704dd7b
+
+
+typedef struct _atmsar_vcc_t5 {
+ atmsar_vcc_t com;
+
+ // Decoder status
+ size_t rxMtu; // Maximum (incoming) Transmit Unit
+ unsigned char *rxBuf;
+ size_t rxSize;
+ size_t rxLen;
+ unsigned int rxHead;
+ unsigned int rxTail;
+
+ // Encoder status
+ size_t txMtu; // Maximum (outgoing) Transmit Unit
+} atmsar_vcc_t5;
+
+
+#pragma pack(1)
+typedef struct _aal5pdu {
+ unsigned char pl[ATM_CELL_PAYLOAD - ATM_AAL5_TRAILER];
+ unsigned char uu;
+ unsigned char cpi;
+ unsigned short len;
+ unsigned int crc;
+} aal5pdu_t;
+#pragma pack()
+
+
+static int aal5EncodeGetCellCount(
+ atmsar_vcc_t* sar,
+ struct sk_buff* skbIn
+) {
+ if(skbIn->len > ((atmsar_vcc_t5*)sar)->txMtu)
+ return(-EMSGSIZE);
+
+ return(
+ (skbIn->len + ATM_AAL5_TRAILER + ATM_CELL_PAYLOAD - 1)
+ /ATM_CELL_PAYLOAD
+ );
+}
+
+static void aal5Encode(
+ struct sk_buff* skbOut,
+ SARPUTHDR* putHdr,
+ atmsar_vcc_t* sar,
+ struct sk_buff* skbIn
+) {
+ const size_t tx_cell_gap = sar->dev->tx_cell_gap;
+ const size_t len = skbIn->len;
+ aal5pdu_t *pdu;
+ size_t trLen;
+ unsigned crc = ~0;
+
+ while(skbIn->len >= ATM_CELL_PAYLOAD) {
+ unsigned char* payload;
+
+ putHdr(skbOut, sar->vpivci);
+ payload = skb_put(skbOut, ATM_CELL_PAYLOAD);
+
+ memcpy(payload, skbIn->data, ATM_CELL_PAYLOAD);
+ crc = crc32_be(crc, payload, ATM_CELL_PAYLOAD);
+ skb_pull(skbIn, ATM_CELL_PAYLOAD);
+
+ if(tx_cell_gap != 0)
+ memset(skb_put(skbOut, tx_cell_gap), 0, tx_cell_gap);
+ }
+
+ // Now skbIn->len is assured to be < ATM_CELL_PAYLOAD
+
+ if(skbIn->len > ATM_CELL_PAYLOAD - ATM_AAL5_TRAILER) {
+ unsigned char* payload;
+
+ putHdr(skbOut, sar->vpivci);
+ payload = skb_put(skbOut, ATM_CELL_PAYLOAD);
+
+ trLen = skbIn->len;
+ memcpy(payload, skbIn->data, trLen);
+ memset(&payload[trLen], 0, ATM_CELL_PAYLOAD - trLen);
+ crc = crc32_be(crc, payload, ATM_CELL_PAYLOAD);
+ skb_pull(skbIn, trLen);
+
+ if(tx_cell_gap != 0)
+ memset(skb_put(skbOut, tx_cell_gap), 0, tx_cell_gap);
+ }
+
+ putHdr(skbOut, sar->vpivci|(ATM_PTI_US1<<ATM_HDR_PTI_SHIFT));
+ pdu = (aal5pdu_t*)skb_put(skbOut, sizeof(*pdu));
+
+ trLen = skbIn->len;
+ if(trLen != 0) {
+ memcpy(pdu->pl, skbIn->data, trLen);
+ skb_pull(skbIn, trLen);
+ }
+
+ if(trLen < ATM_CELL_PAYLOAD - ATM_AAL5_TRAILER)
+ memset(&pdu->pl[trLen], 0, ATM_CELL_PAYLOAD - ATM_AAL5_TRAILER - trLen);
+
+ pdu->uu = pdu->cpi = 0;
+ pdu->len = htons((unsigned short)len);
+ pdu->crc = htonl(
+ ~crc32_be(crc, pdu->pl, sizeof(*pdu) - sizeof(pdu->crc))
+ );
+
+ if(tx_cell_gap != 0)
+ memset(skb_put(skbOut, tx_cell_gap), 0, tx_cell_gap);
+}
+
+
+static void inline aal5DecodeDoReset(atmsar_vcc_t5* ctx)
+{ ctx->rxLen = ctx->rxHead = ctx->rxTail = 0; }
+
+static void aal5DecodeReset(atmsar_vcc_t* sar) {
+ atmsar_vcc_t5 *ctx = (atmsar_vcc_t5*)sar;
+ if(ctx->rxLen != 0) {
+ atomic_inc(&sar->vcc->stats->rx_err);
+ aal5DecodeDoReset(ctx);
+ }
+}
+
+static struct sk_buff* aal5Decode(
+ atmsar_vcc_t* sar,
+ unsigned hdr,
+ const void* payload
+) {
+ atmsar_vcc_t5* ctx = (atmsar_vcc_t5*)sar;
+
+ const unsigned pti = hdr&ATM_HDR_PTI_MASK;
+ if(
+ pti == (ATM_PTI_US1<<ATM_HDR_PTI_SHIFT) ||
+ pti == (ATM_PTI_UCES1<<ATM_HDR_PTI_SHIFT)
+ ) {
+ // Last cell
+ const aal5pdu_t *pdu = (aal5pdu_t*)payload;
+ const size_t lenTotal = (size_t)ntohs(pdu->len);
+ int lenLeft = lenTotal;
+ int hadDiscardedCells = 0;
+ unsigned crc = ~0;
+ struct sk_buff *skb;
+
+ const int nMissingCells = ctx->rxLen/ATM_CELL_PAYLOAD - (
+ (lenTotal + ATM_AAL5_TRAILER + ATM_CELL_PAYLOAD - 1)
+ /ATM_CELL_PAYLOAD - 1
+ );
+ if(nMissingCells > 0) {
+ // Too few cells in buffer
+ aal5DecodeReset(sar);
+ return(NULL);
+ } else if(nMissingCells < 0) {
+ // Too many cells in buffer
+ const size_t lenDiff = -nMissingCells*ATM_CELL_PAYLOAD;
+ ctx->rxTail = (ctx->rxTail + lenDiff)%ctx->rxSize;
+ ctx->rxLen -= lenDiff;
+
+ hadDiscardedCells = 1;
+ }
+
+ // A bit of optimism: we alloc an skb right now,
+ // while SDU content is not yet validated.
+ skb = dev_alloc_skb(lenTotal);
+ if(skb == NULL) {
+ // skb allocation failed
+ atomic_inc(&sar->vcc->stats->rx_drop);
+ aal5DecodeDoReset(ctx);
+ return(NULL);
+ }
+
+ // Copy SDU data from buffer
+ while(ctx->rxLen != 0 && lenLeft != 0) {
+ size_t len = ctx->rxLen;
+ if(len > ctx->rxSize - ctx->rxTail)
+ // Start with first chunk
+ len = ctx->rxSize - ctx->rxTail;
+ if(len > lenLeft)
+ len = lenLeft;
+
+ crc = crc32_be(
+ crc,
+ memcpy(
+ skb_put(skb, len),
+ &ctx->rxBuf[ctx->rxTail],
+ len
+ ),
+ len
+ );
+
+ ctx->rxTail = (ctx->rxTail + len)%ctx->rxSize;
+ ctx->rxLen -= len;
+ lenLeft -= len;
+ }
+
+ // At this point, rxLen and lenLeft can't be both non-zero
+ if(ctx->rxLen != 0)
+ // SDU end: no copy but do crc on padding.
+ // There should be only one cell left in buffer,
+ // but we can't assume rxTail will not cross
+ // buffer boundaries since the buffer size is not
+ // necessarly a multiple of a cell's payload size.
+ do {
+ size_t len = ctx->rxLen;
+ if(len > ctx->rxSize - ctx->rxTail)
+ len = ctx->rxSize - ctx->rxTail;
+
+ crc = crc32_be(
+ crc,
+ &ctx->rxBuf[ctx->rxTail],
+ len
+ );
+
+ ctx->rxTail = (ctx->rxTail + len)%ctx->rxSize;
+ ctx->rxLen -= len;
+ } while(ctx->rxLen != 0);
+ else if(lenLeft != 0)
+ // Buffer end: now copy data from PTI cell
+ crc = crc32_be(
+ crc,
+ memcpy(skb_put(skb, lenLeft), pdu->pl, lenLeft),
+ lenLeft
+ );
+
+ // We don't need the rx buffer anymore
+ aal5DecodeDoReset(ctx);
+
+ // Crc on padding, uu, cpi, len, and crc in PTI cell
+ crc = crc32_be(
+ crc,
+ &pdu->pl[lenLeft],
+ ATM_CELL_PAYLOAD - lenLeft
+ );
+
+ if(crc != CRC32_REMAINDER) {
+ // SDU unreliable. Discard it!
+ dev_kfree_skb(skb);
+ atomic_inc(&sar->vcc->stats->rx_err);
+ return(NULL);
+ } else if(hadDiscardedCells)
+ // This is because we don't want to increase
+ // rx_err twice when there are discarder cells
+ // AND the SDU content isn't reliable...
+ atomic_inc(&sar->vcc->stats->rx_err);
+
+ // SDU reliable. Give it back!
+ return(skb);
+ } else {
+ // Start or middle cell
+ size_t newLen;
+
+ if(ctx->rxSize == 0) {
+ ctx->rxBuf = kmalloc(SZ_FIRST_AAL5SDU, GFP_ATOMIC);
+ if(ctx->rxBuf == NULL) {
+ atomic_inc(&sar->vcc->stats->rx_drop);
+ return(NULL);
+ }
+
+ ctx->rxSize = SZ_FIRST_AAL5SDU;
+ }
+
+ newLen = ctx->rxLen + ATM_CELL_PAYLOAD;
+ if(newLen > ctx->rxMtu) {
+ ctx->rxTail =
+ (ctx->rxTail + ATM_CELL_PAYLOAD)%ctx->rxSize
+ ;
+ atomic_inc(&sar->vcc->stats->rx_err);
+ } else if(newLen > ctx->rxSize) {
+ // This case is invoked only few times (at most 5 during
+ // the whole vcc life). It is meant to increase the
+ // buffer size when a new packet arrives and the rxMtu
+ // allows for a buffer increase. Vccs should reach a
+ // stable buffer size almost early. The buffer size will
+ // be a tradeoff between speed and memory consumption.
+ // Please note that the buffer doubles its size only
+ // when packets seem to really need it. Ie: their rxMtu
+ // limit is not reached...
+ const size_t newSize = 2*ctx->rxSize;
+ unsigned char* oldRxBuf = ctx->rxBuf;
+
+ ctx->rxBuf = kmalloc(newSize, GFP_ATOMIC);
+ if(ctx->rxBuf == NULL) {
+ // Undo things. We discard this cell.
+ ctx->rxBuf = oldRxBuf;
+
+ atomic_inc(&sar->vcc->stats->rx_drop);
+ return(NULL);
+ } else if(ctx->rxLen > 0) {
+ if(ctx->rxTail >= ctx->rxHead) {
+ const size_t len =
+ ctx->rxSize - ctx->rxTail
+ ;
+
+ memcpy(
+ ctx->rxBuf,
+ &oldRxBuf[ctx->rxTail],
+ len
+ );
+ if(ctx->rxLen > len)
+ memcpy(
+ &ctx->rxBuf[len],
+ oldRxBuf,
+ ctx->rxLen - len
+ );
+ } else
+ memcpy(
+ ctx->rxBuf,
+ &oldRxBuf[ctx->rxTail],
+ ctx->rxLen
+ );
+ }
+
+ ctx->rxTail = 0;
+ ctx->rxHead = ctx->rxLen;
+ ctx->rxSize = newSize;
+
+ // Time to give back our quaint buffer...
+ kfree(oldRxBuf);
+ }
+
+ // Transfers received cell
+ memcpy(&ctx->rxBuf[ctx->rxHead], payload, ATM_CELL_PAYLOAD);
+ ctx->rxHead = (ctx->rxHead + ATM_CELL_PAYLOAD)%ctx->rxSize;
+ ctx->rxLen = newLen;
+ return(NULL);
+ }
+}
+
+
+static atmsar_vcc_t* aal5Init(struct atm_vcc* vcc) {
+ if(
+ vcc->qos.rxtp.max_sdu <= ATM_MAX_AAL5_PDU &&
+ vcc->qos.txtp.max_sdu <= ATM_MAX_AAL5_PDU
+ ) {
+ atmsar_vcc_t5 *sarvcc = (atmsar_vcc_t5*)kmalloc(
+ sizeof(atmsar_vcc_t5),
+ GFP_KERNEL
+ );
+ if(sarvcc != NULL) {
+ memset(sarvcc, 0, sizeof(*sarvcc));
+ sarvcc->rxMtu = (
+ vcc->qos.rxtp.max_sdu == 0
+ ?
+ ATM_MAX_AAL5_PDU
+ :
+ vcc->qos.rxtp.max_sdu
+ );
+ sarvcc->txMtu = (
+ vcc->qos.txtp.max_sdu == 0
+ ?
+ ATM_MAX_AAL5_PDU
+ :
+ vcc->qos.txtp.max_sdu
+ );
+ return((atmsar_vcc_t*)sarvcc);
+ }
+ }
+
+ return(NULL);
+}
+
+static void aal5End(atmsar_vcc_t* sarvcc) {
+ atmsar_vcc_t5* ctx = (atmsar_vcc_t5*)sarvcc;
+ if(ctx->rxSize != 0)
+ kfree(ctx->rxBuf);
+}
+
+
+const atmsar_aalops_t opsAAL5 = {
+ ATM_AAL5, "5",
+
+ aal5Init,
+ aal5End,
+
+ aal5DecodeReset,
+ aal5Decode,
+
+ aal5EncodeGetCellCount,
+ aal5Encode
+};
diff -Nurd linux-2.6.13/drivers/atm/saraalr.c linux-2.6.13+atmsar/drivers/atm/saraalr.c
--- linux-2.6.13/drivers/atm/saraalr.c 1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.13+atmsar/drivers/atm/saraalr.c 2005-09-04 12:22:24.000000000 +0200
@@ -0,0 +1,144 @@
+/* ATM SAR layer
+ *
+ * Copyright (C) 2005 Giampaolo Tomassoni
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * The GNU GPL is contained in /usr/doc/copyright/GPL on a Debian
+ * system and in the file COPYING in the Linux kernel source.
+ */
+
+/*
+ * CREDITS go to the following people:
+ *
+ * - Johan Verrept, for its previous work on a SAR library in Linux;
+ * - Charles Michael Heard for its tutorial on CRC-8 Error Correction.
+ * - Chas Williams for its help in integrating this with the ATM module.
+ *
+ *
+ * Theory of operation and api description in include/linux/atmsar.h
+ */
+#include <linux/version.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/gfp.h>
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/wait.h>
+#include <linux/list.h>
+#include <linux/smp_lock.h>
+#include <linux/init.h>
+#include <linux/errno.h>
+#include <linux/skbuff.h>
+#include <linux/atm.h>
+#include <linux/atmdev.h>
+
+#include <linux/atmsar.h>
+#include "sar.h"
+
+
+static int aalREncodeGetCellCount(
+ atmsar_vcc_t* sar,
+ struct sk_buff* skbIn
+) {
+ // skbIn must contain correctly-sized cells
+ if((skbIn->len%ATM_AAL0_SDU) != 0) {
+ // Size is not a multiple of 52 bytes
+ atomic_inc(&sar->vcc->stats->tx_err);
+ return(-EMSGSIZE);
+ } else {
+ atomic_inc(&sar->vcc->stats->tx);
+ return(skbIn->len/ATM_AAL0_SDU);
+ }
+}
+
+static void aalREncode(
+ struct sk_buff* skbOut,
+ SARPUTHDR* putHdr,
+ atmsar_vcc_t* sar,
+ struct sk_buff* skbIn
+) {
+ while(skbIn->len > 0) {
+ const unsigned char* cell = skb_pull(skbIn, ATM_AAL0_SDU);
+
+ putHdr(skbOut, *(unsigned*)cell);
+ memcpy(
+ skb_put(skbOut, ATM_CELL_PAYLOAD),
+ &cell[4],
+ ATM_CELL_PAYLOAD
+ );
+
+ if(sar->dev->tx_cell_gap != 0)
+ memset(
+ skb_put(skbOut, sar->dev->tx_cell_gap),
+ 0,
+ sar->dev->tx_cell_gap
+ );
+ }
+}
+
+
+static void aalRDecodeReset(atmsar_vcc_t* sarvcc)
+{}
+
+static struct sk_buff* aalRDecode(
+ atmsar_vcc_t* sar,
+ unsigned hdr,
+ const void* payload
+) {
+ unsigned char* buf;
+ struct sk_buff *skb = dev_alloc_skb(ATM_AAL0_SDU);
+ if(skb == NULL) {
+ atomic_inc(&sar->vcc->stats->rx_drop);
+ return(NULL);
+ }
+
+ buf = skb_put(skb, ATM_AAL0_SDU);
+ *(unsigned*)buf = hdr;
+ memcpy(&buf[4], payload, ATM_CELL_PAYLOAD);
+
+ atomic_inc(&sar->vcc->stats->rx);
+ return(skb);
+}
+
+
+static atmsar_vcc_t* aalRInit(struct atm_vcc* vcc) {
+ atmsar_vcc_t* sar = (atmsar_vcc_t*)kmalloc(
+ sizeof(atmsar_vcc_t),
+ GFP_KERNEL
+ );
+ if(sar != NULL)
+ memset(sar, 0, sizeof(*sar));
+
+ return(sar);
+}
+
+static void aalREnd(atmsar_vcc_t* sarvcc)
+{ /* kfree() is in atmsarmod.c */ }
+
+
+const atmsar_aalops_t opsAALR = {
+ ATM_AAL0,
+ "raw",
+
+ aalRInit,
+ aalREnd,
+
+ aalRDecodeReset,
+ aalRDecode,
+
+ aalREncodeGetCellCount,
+ aalREncode
+};
diff -Nurd linux-2.6.13/include/linux/atmsar.h linux-2.6.13+atmsar/include/linux/atmsar.h
--- linux-2.6.13/include/linux/atmsar.h 1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.13+atmsar/include/linux/atmsar.h 2005-09-04 12:25:03.000000000 +0200
@@ -0,0 +1,736 @@
+/* ATM SAR layer
+ *
+ * Copyright (C) 2005 Giampaolo Tomassoni
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * The GNU GPL is contained in /usr/doc/copyright/GPL on a Debian
+ * system and in the file COPYING in the Linux kernel source.
+ */
+
+/*
+ * CREDITS go to the following people:
+ *
+ * - Johan Verrept, for its previous work on a SAR library in Linux;
+ * - Charles Michael Heard for its tutorial on CRC-8 Error Correction.
+ * - Chas Williams for its help in integrating this with the ATM module.
+ *
+ *
+ * Purposes, theory of operation and module interfacing details
+ *
+ * This is yet another Segment And Reassembly (SAR) layer for the
+ * linux ATM stack.
+ *
+ * The differences from other solutions, as well as its power, are that:
+ *
+ * - it is mind to be shared by any atm interface, instead of being somehow
+ * embedded in a bus-wide module (see usb_atm) or, much worse, in
+ * a single device driver (see pulsar). The usb_atm solution is restricted
+ * to usb devices. ADSL modems are often USB, so usb_atm is effectively
+ * an interesting solution, but why not devise a more general solution?
+ * The atmsar module wants to be this bus-unspecific solution.
+ *
+ * - supplies a coherent interface which allows the device driver to
+ * bypass almost any interaction with the atm layer;
+ *
+ * - supports ATM header&hec check, correction and generation, which is
+ * most useful for dumb atm devices (ie: most ADSL modems);
+ *
+ * - supports automatic and fast routing of received cells to destinating
+ * vcc;
+ *
+ * - actually supports AALraw, AAL0 and AAL5;
+ *
+ * - aal decoding/encoding routines are designed as atmsar plug-ins, so
+ * that further types may easily be supported;
+ *
+ * - implements speed- and memory-concerned techniques for per-vcc
+ * decoding;
+ *
+ * - allows using dma-streaming techniques in sending cells to device;
+ *
+ * - supports tx buffer adjusting against device needs;
+ *
+ * - avoids vcc open/close paradigm handling in device driver;
+ *
+ * - yields a uniform view to SAR status in /proc.
+ *
+ *
+ * Ok. That said, Theory of operation
+ *
+ * The atmsar module is designed as a middle layer between the atm
+ * stack and the device driver.
+ *
+ * As with an usual atm-based solution, the device driver will be
+ * responsible for handling all the hardware details regarding bus
+ * interfacing, device interfacing, interrupt handling and the like.
+ *
+ * Not like usual atm-based solutions, the device driver will
+ * not need anymore to directly handle cell decoding and encoding,
+ * packet segmenting and reassembly, and vcc open/close handling.
+ * At init time the driver will register itself with the atmsar
+ * layer by calling atmsar_dev_register() instead of registering
+ * with atm_dev_register().
+ *
+ * The atmsar_dev_register() invocation gives a chance to the
+ * driver to specify some tuning values about wanted tx buffers,
+ * tx cells structure and supported operations (atmsar_ops_t type).
+ *
+ * The driver interacts with the atmsar layer both through
+ * callbacks (very useful but completely optional) and functions
+ * call.
+ *
+ * The callbacks which the driver may specify in the
+ * atmsar_dev_register() ops parameter are:
+ *
+ * - ATMSAR_FIRST_OPEN
+ * The driver callback will be invoked when the device is
+ * first opened by the atm layer. Further opens will not be
+ * propagated to the device. Also, a mutex is used to avoid
+ * any race condition by the open/close paradigm.
+ *
+ * In this callback, the device is given the chance to report
+ * any failure in initializing things, thereby allowing it
+ * to avoid further operations;
+ *
+ * - ATMSAR_LAST_CLOSE
+ * This is the callback dual to ATMSAR_FIRST_OPEN. It is invo-
+ * ked when the last vcc on the device get closed by the atm
+ * layer. On SMP and preemptive contexts, this callback is
+ * assured to be atomic with respect to ATMSAR_FIRST_OPEN;
+ *
+ * - ATMSAR_TX_RESTART
+ * This callback is invoked when new outgoing cells are ready
+ * for delivery. The implementing driver may use this callback
+ * to restart cell transmission;
+ *
+ * - ATMSAR_IOCTL
+ * A chance for the driver to receive ioctls from userspace;
+ *
+ * - ATMSAR_PROCREAD
+ * A chance for the driver to report status data through /proc.
+ *
+ *
+ * These are the most important functions supplied by atmsar:
+ *
+ * - atmsar_rx_decode_52bytes(),
+ * atmsar_rx_decode_53bytes(),
+ * atmsar_rx_decode_53bytes_skiphec()
+ *
+ * The driver may use one of three functions above in order to
+ * get a received cell synchronously processed.
+ *
+ * Please note that the atmsar_rx_decode_*() beasts do process
+ * a single cell synchronously. You may find their processing
+ * time to be acceptable for direct cell decoding at rx
+ * interrupt time. However, to allow for slow architectures to
+ * benefit from your driver, it is better to write a tasklet
+ * for this purpose.
+ *
+ * - atmsar_tx_skb_fetch()
+ * The driver may use this function to obtain the next buffer
+ * to be transmitted out of the device.
+ *
+ * - atmsar_dev_hassignal()/atmsar_dev_setsignal()
+ * Allow the driver to specify the current state of the carrier
+ * signal, as well as to inspect the state known to atmsar.
+ * When the driver marks the carrier as lost, no more cells will
+ * be queued to the tx queue until it marks the carrier present
+ * again.
+ *
+ * Please note that after atmsar_dev_register(), the atmsar
+ * layer assumes the carrier signal to be lost. The driver has
+ * to issue an atmsar_dev_setsignal() call to set it as present
+ * if it wants to process outgoing cells (and if it is the case).
+ *
+ *
+ * Tx operations:
+ *
+ * atmsar packs encoded cells in struct sk_buff buffers, which are
+ * queued in the field skq_tx in atmsar_dev_t.
+ *
+ * Tx skbs are shaped according to the tuning parameters specified
+ * to atmsar_dev_register().
+ *
+ * These parameters allow for a buffer prefix area, a buffer
+ * postfix area, as well as non-standard cell sizes (better said,
+ * standard cells with gaps at their ends).
+ *
+ * Buffer prefix spans from skb->data to skb->head-1, buffer
+ * postfix spans from skb->tail to the end of the data.
+ *
+ * Both prefix and postfix are given to uninitialized to the driver,
+ * while cell gaps are zeroed. Prefix and postfix are thereby supposed
+ * to be filled by the driver with usefull content, before streaming
+ * the whole buffer to the device.
+ *
+ * When a driver is ready to send cells out of the device, it invokes
+ * atmsar_tx_skb_fetch() which returns the next available buffer.
+ *
+ * If the device has specific needs, the driver may setup the buffer
+ * prefix and postfix areas in order to fullfil them.
+ *
+ * The driver may then use whatever mean it likes in order to send
+ * the buffer content to the device. One of the most effective ways
+ * is streaming DMA, but this is left to the driver developer and
+ * device limits.
+ *
+ * Once the skb has been completely sent to the device, it may be
+ * released. The driver is required to use atmsar_tx_skb_*()
+ * to do so. Never use kfree_skb(), or you'll cause memory leakage.
+ */
+
+/* ATMSAR_N_HASHVCCS
+ * This is the size of the hash of open atm vccs.
+ * Being it the size of a hash table, a prime works better.
+ */
+#define ATMSAR_N_HASHVCCS 7
+
+
+/* atmsar_dev_t
+ * A forward declaration to struct atmsar_dev
+ */
+typedef struct atmsar_dev atmsar_dev_t;
+
+/* atmsar_vcc_t
+ * A forward declaration to some AAL-dependent
+ * structures devoted to tracking the state of
+ * the aal routine in charge of a vcc.
+ * This is an opaque type to users of this module.
+ */
+typedef struct atmsar_vcc_base atmsar_vcc_t;
+
+/* atmsar_aalops_t
+ * Another opaque type to users of this module.
+ */
+typedef struct atmsar_aalops atmsar_aalops_t;
+
+
+/* ATMSAR_FIRSTOPEN
+ * This is the typedef for the first_open callback.
+ * Devices implementing the first_open method should
+ * conform to it.
+ *
+ * @dev: the device instance as returned by
+ * atmsar_dev_register()
+ *
+ * return: >= 0 on success, <0 in case of an
+ * error detected by the upper layer.
+ *
+ * The atmsar module invokes this callback when
+ * the first vcc is opened on the device.
+ *
+ * When an error is returned by the first_open()
+ * callback, this driver unwinds initialization and
+ * returns the error code to the downstream open()
+ * function.
+ *
+ * This callback is invoked with no spinlock held
+ * and interrupts enabled, so that the implementor
+ * may schedule().
+ */
+typedef int ATMSAR_FIRSTOPEN(atmsar_dev_t *dev);
+
+/* ATMSAR_LASTCLOSE
+ * This is the typedef for the last_close callback.
+ * Devices implementing the last_close method should
+ * conform to it.
+ *
+ * @dev: the device instance as returned by
+ *
+ * The atmsar module invokes this callback when
+ * the last vcc is closed.
+ *
+ * This callback is invoked with no spinlock held
+ * and interrupts enabled, so that the implementor
+ * may schedule().
+ */
+typedef void ATMSAR_LASTCLOSE(atmsar_dev_t *dev);
+
+/* ATMSAR_IOCTL
+ * This is the typedef for the ioctl() callback.
+ * Devices implementing the ioctl() method should
+ * conform to it.
+ *
+ * @dev: the device instance as returned by
+ * atmsar_dev_register().
+ * @cmd: the user-invoked ioctl command.
+ * @arg: the ioctl argument, which may be
+ * a pointer to user-space data.
+ *
+ * return: Whatever the upper code wants to
+ * yield back to the user.
+ *
+ * This callback is invoked in the same runtime
+ * status of the atm ioctl callback. This should
+ * generally mean that no spinlock is held and
+ * that interrupts are enabled.
+ */
+typedef int ATMSAR_IOCTL(
+ atmsar_dev_t *dev,
+ unsigned cmd,
+ void __user *arg
+);
+
+/* ATMSAR_PROCREAD
+ * This is the typedef for the proread() callback.
+ * Devices implementing the aioctl() method should
+ * conform to it.
+ *
+ * @dev: the device instance as returned by
+ * atmsar_dev_register().
+ * @pos: a pointer to the skipping position.
+ * The procread implementation is sup-
+ * posed to emit its message on page
+ * when *pos is 0, then decrement it.
+ * @page: a pointer to the character buffer
+ * which will be emitted by the module.
+ *
+ * return: the number of characters written to page.
+ *
+ * This callback is invoked with no spinlock held
+ * and interrupts enabled, so that the implementor
+ * may schedule().
+ */
+typedef int ATMSAR_PROCREAD(
+ atmsar_dev_t* dev,
+ loff_t* pos,
+ char* page
+);
+
+/* ATMSAR_TX_RESTART
+ * This is the typedef for the tx_restart() callback.
+ * Devices implementing the tx_restart() method should
+ * conform to it.
+ *
+ * @dev: the device instance as returned by
+ * atmsar_dev_register().
+ *
+ * When implemented, this callback will be invoked
+ * when atmsar appends new SKBs are appended to an
+ * empty dev->skq_tx.
+ *
+ * This callback is invoked in the same context as
+ * the atm send callback. This means that most of the
+ * time no spinlock is held and interrupts are enabled,
+ * but there are cases in which this doesn't hold
+ * (pppoatm?). Implementors better avoid schedule()
+ * and such.
+ */
+typedef void ATMSAR_TX_RESTART(atmsar_dev_t *dev);
+
+
+/* atmsar_ops_t
+ * An envelope for all the above mentioned callbacks.
+ */
+typedef struct atmsar_ops {
+ ATMSAR_TX_RESTART* tx_restart;
+ ATMSAR_FIRSTOPEN* first_open;
+ ATMSAR_LASTCLOSE* last_close;
+ ATMSAR_IOCTL* ioctl;
+ ATMSAR_PROCREAD* proc_read;
+} atmsar_ops_t;
+
+
+/* ATMSAR_FLG_53BYTE_CELL
+ * This flag may be used in atmsar_dev_register() to
+ * indicate that a full cell (ie: one containing also
+ * the HEC field) must be generated during encoding.
+ * Its assertion generally means that the device
+ * using this module has no support for hec generation.
+ */
+#define ATMSAR_FLG_53BYTE_CELL (1<<0) // Cell must contain hec
+
+/* ATMSAR_SPEED_UNSPEC
+ * This is the value returned by get_*_speed() when
+ * the speed is not known.
+ */
+#define ATMSAR_SPEED_UNSPEC ((unsigned long)0)
+
+/* atmsar_dev_t
+ * This is the incarnation of the atmsar_dev_t type,
+ * which is the device instance as returned by the
+ * atmsar_dev_register() function.
+ */
+struct atmsar_dev {
+ // General infos
+ struct atm_dev* atmdev; // ATM device
+ void* data; // Upper device data
+
+ // Configuration
+ const atmsar_ops_t* ops; // Upper device ops
+ unsigned flags; // Flags defined by u-d
+ size_t tx_head_reserve; // head size in tx skb
+ size_t tx_tail_reserve; // tail size in tx skb
+ size_t tx_cell_size; // A device cell size
+ size_t tx_cell_gap; // The gap between cells
+
+ // Open/Close and Send/Receive data
+ atmsar_vcc_t* vccs_hash[ATMSAR_N_HASHVCCS];
+ int vccs_count;
+ rwlock_t rwl_vccs;
+ atomic_t mtx_openclose;
+ wait_queue_head_t wqh_openclose;
+
+ // Transmit queue
+ struct sk_buff_head skq_tx;
+
+ // Status
+ unsigned long rx_speed; // cells/sec
+ unsigned long tx_speed; // cells/sec
+};
+
+
+/* atmsar_dev_register
+ * Register a device with this module.
+ *
+ * @name: the mnemonic name of the device/driver
+ * @esi: the esi field, or NULL if unknown
+ * @ops: the callback ops. May be NULL.
+ * @phy_ops: physical ops, passed to atm_dev_register()
+ * @flags: registration flags, actually may be just
+ * 0 or ATMSAR_FLG_53BYTE_CELL
+ * @tx_head_reserve: reserve this amount of uninizialied
+ * bytes at the beginning of tx SKBs
+ * queued to dev->skq_tx [0,)
+ * @tx_tail_reserve: reserve this amount of uninizialized
+ * bytes at the end of tx SKBs queued
+ * to dev->skq_tx [0,)
+ * @tx_cell_size: the tx cell size as expected by
+ * the device [ATM_AAL0_SDU,)
+ * return: a pointer to an atmsar_dev_t instance, to
+ * be specified in almost every other atmsar
+ * functions. If the registration fails,
+ * NULL is returned.
+ *
+ * Registering a device with atmsar is the first operation
+ * to be accomplished by the driver.
+ *
+ * This function will in turn register the device with the
+ * atm layer, so you don't need to do it. A pointer to the
+ * instance of the atm_dev struct may be obtained by the
+ * atmdev field of the atmsar_dev_t element.
+ *
+ * In order to increase speed, the module is designed to
+ * adjust for device diversity. This is accomplished in
+ * the "outgoing" direction by generating SKBs immediately
+ * suitable for streaming DMA or otherwise output to the
+ * device. This is the purpose of tx_head_reserve,
+ * tx_tail_reserve and tx_cell_size: the driver calling
+ * this function must specify the values expected by the
+ * device for them. tx_head_reserve specifies the number
+ * of bytes to be reserved at the beginning of a tx skb;
+ * tx_tail_reserve specifies the number of bytes to be
+ * reserved at the end of the tx skb, while tx_cell_size
+ * specifies the size of a cell as expected by the
+ * device. The latter value may be suitable for any cell
+ * padding needed by the device.
+ *
+ * This function cannot be called at interrupt time.
+ */
+extern atmsar_dev_t* atmsar_dev_register(
+ const char* name,
+ const unsigned char* esi,
+ const atmsar_ops_t* ops,
+ const struct atmphy_ops* phy_ops,
+ unsigned flags,
+ size_t tx_head_reserve,
+ size_t tx_tail_reserve,
+ size_t tx_cell_size
+);
+
+/* atmsar_dev_deregister
+ * Deregisters a device with atmsar.
+ *
+ * @dev: a pointer to the atmsar_dev_t instance to
+ * be deregistered.
+ *
+ * Deregistering a device is the last operation to be invoked
+ * by a device, just before it removes.
+ *
+ * This function in turn deregisters the device with the
+ * atm layer, so you don't have to.
+ *
+ * Invoking this function implies shutting down any reference
+ * with the atm layer, as well as releasing any open vcc.
+ *
+ * This function cannot be called at interrupt time.
+ */
+extern void atmsar_dev_deregister(atmsar_dev_t *dev);
+
+/* atmsar_dev_getatm
+ * Gives a pointer to the atm_dev structure tied to the
+ * given device.
+ *
+ * @dev: a pointer to the atmsar_dev_t instance
+ * return: the pointer to the atm_dev structure
+ */
+static inline struct atm_dev* atmsar_dev_getatm(const atmsar_dev_t* dev)
+{ return(dev->atmdev); }
+
+/* atmsar_dev_getdata
+ * Functions to get driver data.
+ *
+ * @dev: a pointer to the atmsar_dev_t instance
+ * return: the device data
+ *
+ * See atmsar_dev_setdata() below
+ */
+static inline void* atmsar_dev_getdata(const atmsar_dev_t* dev)
+{ return(dev->data); }
+
+/* atmsar_dev_setdata
+ * Functions to set driver data.
+ *
+ * @dev: a pointer to the atmsar_dev_t instance
+ * @data: the data to be set
+ *
+ * This function allows the upper driver to store driver data
+ * into the atmsar_dev_t instance.
+ *
+ * This allows the data to be recerenced lather when a callback
+ * is schedule by the atmsar module.
+ *
+ * This way, data private to the driver can be stored in a
+ * structure and its pointer saved into the atmsar_dev_t instance.
+ */
+static inline void atmsar_dev_setdata(atmsar_dev_t* dev, void* data)
+{ dev->data = data; }
+
+/* atmsar_rx_purge
+ * Resets the rx state of all the opened vccs.
+ *
+ * @dev: a pointer to the atmsar_dev_t instance
+ */
+extern void atmsar_rx_purge(atmsar_dev_t *dev);
+
+/* atmsar_rx_decode_*
+ * Decodes a received cell
+ *
+ * @dev: a pointer to the atmsar_dev_t instance
+ * @cell: pointer to the cell data
+ *
+ * These are the main functions for cell reception. The
+ * cell data to be decoded is given to the atmsar driver
+ * which synchronously decodes and eventually dispatches
+ * the result to the atm layer.
+ *
+ * There are three version of this method:
+ *
+ * atmsar_rx_decode_52bytes assumes that the device
+ * supplies cells without the HEC field to the driver.
+ * This kind of cells is 52 bytes long (not counting any
+ * cell padding).
+ *
+ * atmsar_rx_decode_53bytes assumes that the device
+ * supplies cells WITH the HEC field to the driver.
+ * Also, the device is assumed not to be smart enough
+ * to have checked or corrected the cell header against
+ * the HEC field itself, so this too is performed by
+ * the function.
+ *
+ * atmsar_rx_decode_53bytes_skiphec assumes that the
+ * device supplies cells having a placeholder as HEC
+ * field: the device already checked and corrected the
+ * cell header against the HEC field and this will be
+ * simply skipped by the function.
+ *
+ * Your driver will most probably need to call only
+ * one version of this method, since this depends on
+ * the device you are working on.
+ *
+ * Please note that any buffer heading, trailing and
+ * cell padding doesn't apply to atmsar_rx_decode_*,
+ * since you are expected to call this function for
+ * each cell received, passing a pointer to the
+ * beginning of the cell (ie: the cell header) as
+ * the cell parameter.
+ *
+ * These functions can be called at interrupt time.
+ */
+extern void atmsar_rx_decode_52bytes(
+ atmsar_dev_t *dev,
+ const void *cell
+);
+extern void atmsar_rx_decode_53bytes(
+ atmsar_dev_t *dev,
+ const void *cell
+);
+extern void atmsar_rx_decode_53bytes_skiphec(
+ atmsar_dev_t *dev,
+ const void *cell
+);
+
+
+/* atmsar_tx_isempty
+ * Returns true if tx queue is empty
+ *
+ * @dev: a pointer to the atmsar_dev_t instance
+ * return: true if the tx queue is empty, false
+ * otherwise
+ *
+ * This function can be called at interrupt time.
+ */
+static inline int atmsar_tx_isempty(const atmsar_dev_t* dev)
+{ return(skb_queue_empty(&dev->skq_tx)); }
+
+/* atmsar_tx_skb_fetch
+ * Fetches the next tx skb to be transmitted by the
+ * device, if any.
+ *
+ * @dev: a pointer to the atmsar_dev_t instance
+ * return: the skb to be transmitted, or NULL if the
+ * tx queue is empty
+ *
+ * When the atm layer sends packets through atmsar, the latter
+ * converts them into ready-to-send atm cells which are packed
+ * into SKBs. These tx skbs are then queued into a tx queue
+ * (dev->skq_tx).
+ *
+ * This function may be invoked to fetch the next skb to be
+ * transmitted out by the device.
+ *
+ * Please note that the tx SKBs are tied to the originating ones,
+ * so you can't simply invoke kfree_skb() to release them: use
+ * atmsar_tx_skb_*() instead.
+ *
+ * This function can be invoked at interrupt time.
+ */
+static inline struct sk_buff* atmsar_tx_skb_fetch(atmsar_dev_t* dev)
+{ return(skb_dequeue(&dev->skq_tx)); }
+
+/* atmsar_tx_purge
+ * Purges the whole tx queue
+ *
+ * @dev: a pointer to the atmsar_dev_t instance
+ *
+ * This function purges the tx queue associated to the given
+ * atmsar_dev_t instance.
+ *
+ * Purging the tx queue will end discarding all the memory associated
+ * to the contained SKBs.
+ *
+ * This function may be used when the tx queue content is too old
+ * to be useful. In example, after a synch loss.
+ *
+ * This function can be called at interrupt time.
+ */
+extern void atmsar_tx_purge(atmsar_dev_t *dev);
+
+/* atmsar_tx_skb_*
+ * Releases the memory allocated by a tx skb
+ *
+ * @skb: a pointer to the tx skb to be freed
+ *
+ * When you need to free a tx skb, don't use kfree_skb(). Use one
+ * of these function, instead.
+ *
+ * This function releases the given skb and the originating one
+ * (whose pointer is stored into it).
+ *
+ * The _complete version of this function increments the tx field
+ * of the aal status field. The _discard version increments instead
+ * the tx_err field.
+ *
+ * The suggested usage of the first is when the transmission of the
+ * skb is completed and the skb itself is not needed anymore. The
+ * latter would instead be used when the transmission can't be
+ * completed for whatever reason. Note that the atmsar_tx_purge()
+ * function uses the atmsar_tx_skb_discard() version.
+ *
+ * This function can be called at interrupt time.
+ */
+extern void atmsar_tx_skb_complete(struct sk_buff *skb);
+extern void atmsar_tx_skb_discard(struct sk_buff *skb);
+
+/* atmsar_dev_hassignal
+ * Returns true when the "signal" of the device is reported to be
+ * present.
+ *
+ * @dev: a pointer to the atmsar_dev_t instance
+ * return: true when the signal is present, false otherwise
+ *
+ * Every atm device is supposed to exchange data through some kind
+ * of (carrier) signal. This function reports its state, which
+ * is in turn to be defined by the driver itself (see
+ * atmsar_dev_setsignal()).
+ *
+ * This function can be called at interrupt time.
+ */
+static int inline atmsar_dev_hassignal(const atmsar_dev_t* dev)
+{ return(dev->atmdev->signal == ATM_PHY_SIG_FOUND); }
+
+/* atmsar_dev_setsignal
+ * Returns true when the "signal" of the device is reported to be
+ * present.
+ *
+ * @dev: a pointer to the atmsar_dev_t instance
+ * @signal_present: 1 if signal present, 0 otherwhise.
+ *
+ * Every atm device is supposed to exchange data through some kind
+ * of (carrier) signal.
+ *
+ * The driver is in turn supposed to know the state of the carrier
+ * signal used by its device, so it MUST inform of its changes the
+ * atmsar layer.
+ *
+ * When the driver informs the atmsar module that the carrier signal
+ * is not anymore present, the atmsar stops queueing more outgoing
+ * packets to the device.
+ *
+ * When the carrier signal gets back and the driver invokes
+ * atmsar_dev_setsignal(dev, 1), the transmitting packed encoding
+ * resumes and queueing may restart.
+ *
+ * After atmsar_dev_register(), the signal is supposed to be lost.
+ *
+ * Please note that you are supposed to flush tx buffers by invoking
+ * atmsar_tx_purge() sometime after reporting the device signal as lost.
+ *
+ * This function can be called at interrupt time.
+ */
+static void inline atmsar_dev_setsignal(
+ atmsar_dev_t* dev,
+ int signal_present
+) {
+ dev->atmdev->signal = (
+ signal_present
+ ?
+ ATM_PHY_SIG_FOUND
+ :
+ ATM_PHY_SIG_LOST
+ );
+}
+
+
+/*
+ * TODO: Speed inspection and control. Not yet implemented
+ */
+static unsigned long inline atmsar_rx_get_speed(const atmsar_dev_t *dev)
+{ return(dev->rx_speed); }
+
+extern void atmsar_rx_set_speed(
+ atmsar_dev_t* dev,
+ unsigned long speed
+);
+
+
+static unsigned long inline atmsar_tx_get_speed(const atmsar_dev_t *dev)
+{ return(dev->tx_speed); }
+
+extern void atmsar_tx_set_speed(
+ atmsar_dev_t* dev,
+ unsigned long speed
+);
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [Linux-ATM-General] [ATMSAR] Request for review - update #1
2005-09-04 11:05 [ATMSAR] Request for review - update #1 Giampaolo Tomassoni
@ 2005-09-04 12:00 ` Francois Romieu
2005-09-04 13:13 ` R: " Giampaolo Tomassoni
2005-09-04 16:20 ` Alistair John Strachan
2005-09-04 16:55 ` Jiri Slaby
2 siblings, 1 reply; 24+ messages in thread
From: Francois Romieu @ 2005-09-04 12:00 UTC (permalink / raw)
To: Giampaolo Tomassoni; +Cc: linux-kernel, linux-atm-general
Giampaolo Tomassoni <g.tomassoni@libero.it> :
[...]
> However, I'm still hearing for your comments about the usefulness of an
> ATMSAR layer.
Afaik all but one pci ADSL modems are out of tree drivers and include various
level of proprietary code. If Duncan is not interested in changing its code,
the usefulness remains to be proven.
The codingstyle is broken. Please read again Documentation/CodingStyle,
remove the redundant typedef and the silly comments ("Reserve header space",
Encode packet into cells", ...).
- &page[strlen(page)] in atmProcRead sucks.
- "return" is not a function.
- consider 'goto' to handle the errors instead of deep nesting
- +const atmsar_aalops_t opsAALR = {
+ ATM_AAL0,
+ "raw",
-> use .foo = baz instead.
drivers/net/*c may give some hint.
--
Ueimor
^ permalink raw reply [flat|nested] 24+ messages in thread* R: [Linux-ATM-General] [ATMSAR] Request for review - update #1
2005-09-04 12:00 ` [Linux-ATM-General] " Francois Romieu
@ 2005-09-04 13:13 ` Giampaolo Tomassoni
2005-09-04 15:32 ` Francois Romieu
2005-09-05 6:05 ` Zoran Stojsavljevic
0 siblings, 2 replies; 24+ messages in thread
From: Giampaolo Tomassoni @ 2005-09-04 13:13 UTC (permalink / raw)
To: Francois Romieu; +Cc: linux-kernel, linux-atm-general
> -----Messaggio originale-----
> Da: linux-atm-general-admin@lists.sourceforge.net
> [mailto:linux-atm-general-admin@lists.sourceforge.net]Per conto di
> Francois Romieu
> Inviato: domenica 4 settembre 2005 14.01
> A: Giampaolo Tomassoni
> Cc: linux-kernel@vger.kernel.org;
> linux-atm-general@lists.sourceforge.net
> Oggetto: Re: [Linux-ATM-General] [ATMSAR] Request for review - update #1
>
>
> Giampaolo Tomassoni <g.tomassoni@libero.it> :
> [...]
> > However, I'm still hearing for your comments about the usefulness of an
> > ATMSAR layer.
>
> Afaik all but one pci ADSL modems are out of tree drivers and
> include various
> level of proprietary code. If Duncan is not interested in
> changing its code,
> the usefulness remains to be proven.
Well, the idea is that more pci devices may appear, as adsl-enabled embedded systems will begin to appear in the market.
Also, I believe that adsl will carry much more services then just AAL5 for internet connection in the future. Even if the ATMSAR actually lacks of AAL1 and AAL2/3 capabilities, adding them in a single, specialized module is much easier than swimming in a usb+atm middle layer.
Finally, the fact that ATMSAR is device-unspecific makes it easier to maintain, I guess.
> The codingstyle is broken. Please read again Documentation/CodingStyle,
That's a matter of taste: even Linus burned the GNU coding style book...
However, if it is needed by the linux community, I shurely will fix it whenever the ATMSAR idea will get passed: I'm just gathering feedbacks like the previous one you expressed.
> remove the redundant typedef
Oh, you mean the "typedef enum _HECSTS ..." ?
You're right, thanks.
> and the silly comments ("Reserve
> header space",
> Encode packet into cells", ...).
I would prefer to explain better what the ATMSAR is doing there. So, I'll get your as a "clarify silly comments". Ok?
> - &page[strlen(page)] in atmProcRead sucks.
Why? It is preceded by an strcpy(page,...). A constant would be worse if someone changes the prefix string...
Or is a page (a pointer) + strlen(page) (an integer) preferred over a closed syntax?
> - "return" is not a function.
Not even for() or while(). But doesn't they look cute this way?
> - consider 'goto' to handle the errors instead of deep nesting
I prefer not using goto when not required to. Nesting is far more readable to my opinion. Compilers do work fine with both.
Anyway, which are the functions you are objecting?
> - +const atmsar_aalops_t opsAALR = {
> + ATM_AAL0,
> + "raw",
> -> use .foo = baz instead.
atmasr_aalops_t is not an exported structure (you'll find just an opaque definition in include/linux/atmsar.h), so it is not meant to be statically declared by device drivers. But I guess that the problem is readability, right?
Ok, I'm going to consider your hint in the next patch version.
> drivers/net/*c may give some hint.
>
> --
> Ueimor
Thank you for your help.
May I ask if this is just your own contribution or if you are in charge of something in the linux and/or linux-atm projects?
Regards,
-----------------------------------
Giampaolo Tomassoni - IT Consultant
Piazza VIII Aprile 1948, 4
I-53044 Chiusi (SI) - Italy
Ph: +39-0578-21100
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: R: [Linux-ATM-General] [ATMSAR] Request for review - update #1
2005-09-04 13:13 ` R: " Giampaolo Tomassoni
@ 2005-09-04 15:32 ` Francois Romieu
2005-09-04 18:44 ` R: " Giampaolo Tomassoni
2005-09-05 6:05 ` Zoran Stojsavljevic
1 sibling, 1 reply; 24+ messages in thread
From: Francois Romieu @ 2005-09-04 15:32 UTC (permalink / raw)
To: Giampaolo Tomassoni; +Cc: linux-kernel, linux-atm-general
Giampaolo Tomassoni <g.tomassoni@libero.it> :
[...]
> Well, the idea is that more pci devices may appear, as adsl-enabled
> embedded systems will begin to appear in the market.
>
> Also, I believe that adsl will carry much more services then just AAL5 for
> internet connection in the future.
I'd be happily surprized to see more documented ADSL PCI/USB device in the
near future. :o(
> Even if the ATMSAR actually lacks of AAL1 and AAL2/3 capabilities, adding
> them in a single, specialized module is much easier than swimming in a
> usb+atm middle layer.
>
> Finally, the fact that ATMSAR is device-unspecific makes it easier to
> maintain, I guess.
Ok. Your suggestion may have more impact if there is a patch to convert
the sole existing in-kernel driver to use this module.
[...]
> > The codingstyle is broken. Please read again Documentation/CodingStyle,
>
> That's a matter of taste: even Linus burned the GNU coding style book...
An uniform codingstyle is useful when people need to review code. Something
is wrong when a reviewer must uncipher a piece of code. You will find areas
in the kernel whose trends differ but a codingstyle from Mars is usually a
hint. So it is not _only_ a matter of taste.
> However, if it is needed by the linux community, I shurely will fix it
> whenever the ATMSAR idea will get passed: I'm just gathering feedbacks
> like the previous one you expressed.
You may have more feedback/review then. I only gave a cursory look at the
code.
[...]
> > remove the redundant typedef
>
> Oh, you mean the "typedef enum _HECSTS ..." ?
Rather the "typedef struct atmsar_dev atmsar_dev_t;" (yes, I know the "It
saves typing" argument). Maybe something could be done at the same time
regarding the need for the forward declarations.
[...]
> > and the silly comments ("Reserve
> > header space",
> > Encode packet into cells", ...).
>
> I would prefer to explain better what the ATMSAR is doing there. So, I'll
> get your as a "clarify silly comments". Ok?
s/what/why/
And no, documenting a call to skb_reserve is silly.
[...]
> > - &page[strlen(page)] in atmProcRead sucks.
>
> Why? It is preceded by an strcpy(page,...). A constant would be worse if
> someone changes the prefix string...
The value returned by sprintf and friends contains the needed offset, i.e.
buf += sprintf(buf, ...);.
[...]
> > - "return" is not a function.
>
> Not even for() or while(). But doesn't they look cute this way?
No.
for (), while (), return rc;
[...]
> > - consider 'goto' to handle the errors instead of deep nesting
>
> I prefer not using goto when not required to. Nesting is far more readable
> to my opinion.
OTOH, it makes ugly code to have it fit in a 80 columns console.
[...]
> Anyway, which are the functions you are objecting?
atmSend. Probably others.
If you can make the code look like existing in-kernel code (not fs/cifs
please) say network or ata driver code and you do not need goto, it's fine
too.
[...]
> > - +const atmsar_aalops_t opsAALR = {
> > + ATM_AAL0,
> > + "raw",
> > -> use .foo = baz instead.
>
> atmasr_aalops_t is not an exported structure (you'll find just an opaque
> definition in include/linux/atmsar.h), so it is not meant to be statically
> declared by device drivers. But I guess that the problem is readability,
> right?
struct foo zoy {
.bar = barbar,
.baz = bazbaz,
.quuz = ...
};
[...]
> May I ask if this is just your own contribution or if you are in charge of
> something in the linux and/or linux-atm projects?
/me scratches head
http://ww.google.com/search?hl=en&q=romieu+linux+cabal
--
Ueimor
^ permalink raw reply [flat|nested] 24+ messages in thread* R: R: [Linux-ATM-General] [ATMSAR] Request for review - update #1
2005-09-04 15:32 ` Francois Romieu
@ 2005-09-04 18:44 ` Giampaolo Tomassoni
2005-09-04 19:11 ` matthieu castet
0 siblings, 1 reply; 24+ messages in thread
From: Giampaolo Tomassoni @ 2005-09-04 18:44 UTC (permalink / raw)
To: Francois Romieu; +Cc: linux-kernel, linux-atm-general
> -----Messaggio originale-----
> Da: Francois Romieu [mailto:romieu@fr.zoreil.com]
> Inviato: domenica 4 settembre 2005 17.33
> A: Giampaolo Tomassoni
> Cc: linux-kernel@vger.kernel.org;
> linux-atm-general@lists.sourceforge.net
> Oggetto: Re: R: [Linux-ATM-General] [ATMSAR] Request for review - update #1
>
> ...omissis...
>
> I'd be happily surprized to see more documented ADSL PCI/USB device in the
> near future. :o(
OT Question. What about an open adsl device? The linux community had been bumped out by the adsl market for at least a couple of years, and nobody knows (or tells) why...
That could be a definitive answer. Is there anybody interested in this?
>
> ...omissis...
>
> > Finally, the fact that ATMSAR is device-unspecific makes it easier to
> > maintain, I guess.
>
> Ok. Your suggestion may have more impact if there is a patch to convert
> the sole existing in-kernel driver to use this module.
Mmmh. I can try to do this, but I would prefer to hear Sands about this.
>
> ...omissis
>
> An uniform codingstyle is useful when people need to review code.
> Something is wrong when a reviewer must uncipher a piece of code.
> You will find areas in the kernel whose trends differ but a codingstyle
> from Mars is usually a hint. So it is not _only_ a matter of taste.
Ok, ok. I'll (try to) behave...
>
> ...omissis...
>
> You may have more feedback/review then. I only gave a cursory look at the
> code.
Right, that's what I'm looking for.
>
> ...omissis...
>
> Rather the "typedef struct atmsar_dev atmsar_dev_t;" (yes, I know the "It
> saves typing" argument). Maybe something could be done at the same time
> regarding the need for the forward declarations.
Well, fine. I'll "struct _whatever *". But atmsar_dev_t no, that nonono: it mimics the atm_dev_t typedef... It's all around the idea a developer needs to use atmsar_dev_t instead of atm_dev_t...
>
> ...omissis...
>
> s/what/why/
>
> And no, documenting a call to skb_reserve is silly.
...
>
> ...omissis...
>
> The value returned by sprintf and friends contains the needed offset, i.e.
> buf += sprintf(buf, ...);.
I used an strcpy() to put the constant string in the buffer. However, I'm changing it this way:
if(skip-- == 0) {
count = strlen(strcpy(page, "dnrate:\t"));
if(dev->rx_speed != ATMSAR_SPEED_UNSPEC)
count += sprintf(
&page[count],
"%ld kbps\n",
dev->rx_speed
);
else
count += strlen(strcpy(&page[count], "unknown\n"));
return(count);
}
> [...]
> > > - "return" is not a function.
> >
> > Not even for() or while(). But doesn't they look cute this way?
>
> No.
>
> for (), while (), return rc;
...
> [...]
> > > - consider 'goto' to handle the errors instead of deep nesting
> >
> > I prefer not using goto when not required to. Nesting is far
> more readable
> > to my opinion.
>
> OTOH, it makes ugly code to have it fit in a 80 columns console.
>
> [...]
> > Anyway, which are the functions you are objecting?
>
> atmSend. Probably others.
>
> If you can make the code look like existing in-kernel code (not fs/cifs
> please) say network or ata driver code and you do not need goto, it's fine
> too.
Mmmmh. I'll check it out.
> > > - +const atmsar_aalops_t opsAALR = {
> > > + ATM_AAL0,
> > > + "raw",
> > > -> use .foo = baz instead.
> >
> > atmasr_aalops_t is not an exported structure (you'll find just an opaque
> > definition in include/linux/atmsar.h), so it is not meant to be
> statically
> > declared by device drivers. But I guess that the problem is readability,
> > right?
>
> struct foo zoy {
> .bar = barbar,
> .baz = bazbaz,
> .quuz = ...
> };
...
> [...]
> > May I ask if this is just your own contribution or if you are
> in charge of
> > something in the linux and/or linux-atm projects?
>
> /me scratches head
>
> http://ww.google.com/search?hl=en&q=romieu+linux+cabal
That was to give the right wedge to your hints. If you were just around this list, your hints had a different value than if you were a committer. Am I wrong?
Thanks,
-----------------------------------
Giampaolo Tomassoni - IT Consultant
Piazza VIII Aprile 1948, 4
I-53044 Chiusi (SI) - Italy
Ph: +39-0578-21100
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: R: R: [Linux-ATM-General] [ATMSAR] Request for review - update #1
2005-09-04 18:44 ` R: " Giampaolo Tomassoni
@ 2005-09-04 19:11 ` matthieu castet
2005-09-04 19:33 ` R: " Giampaolo Tomassoni
2005-09-04 19:42 ` Giampaolo Tomassoni
0 siblings, 2 replies; 24+ messages in thread
From: matthieu castet @ 2005-09-04 19:11 UTC (permalink / raw)
To: Giampaolo Tomassoni; +Cc: Francois Romieu, linux-kernel, linux-atm-general
Giampaolo Tomassoni wrote:
>>-----Messaggio originale-----
>>Da: Francois Romieu [mailto:romieu@fr.zoreil.com]
>>Inviato: domenica 4 settembre 2005 17.33
>>A: Giampaolo Tomassoni
>>Cc: linux-kernel@vger.kernel.org;
>>linux-atm-general@lists.sourceforge.net
>>Oggetto: Re: R: [Linux-ATM-General] [ATMSAR] Request for review - update #1
>>
>>...omissis...
>>
>>I'd be happily surprized to see more documented ADSL PCI/USB device in the
>>near future. :o(
>
>
> OT Question. What about an open adsl device? The linux community had been bumped out by the adsl market for at least a couple of years, and nobody knows (or tells) why...
>
> That could be a definitive answer. Is there anybody interested in this?
>
>
The problem is that lot's of new devices implement part of their dsp
function in the kernel space instead of in the device.
And as company don't want to publish their dsp algorith and open source
it, we can't have open source driver for it.
That the case for bewan device (that even include a libm in their
source) and for pulsar pci device...
^ permalink raw reply [flat|nested] 24+ messages in thread
* R: R: R: [Linux-ATM-General] [ATMSAR] Request for review - update #1
2005-09-04 19:11 ` matthieu castet
@ 2005-09-04 19:33 ` Giampaolo Tomassoni
2005-09-04 19:42 ` Giampaolo Tomassoni
1 sibling, 0 replies; 24+ messages in thread
From: Giampaolo Tomassoni @ 2005-09-04 19:33 UTC (permalink / raw)
To: matthieu castet, Giampaolo Tomassoni
Cc: Francois Romieu, linux-kernel, linux-atm-general
> -----Messaggio originale-----
> Da: matthieu castet [mailto:castet.matthieu@free.fr]
> Inviato: domenica 4 settembre 2005 21.11
>
> ...omissis...
>
> The problem is that lot's of new devices implement part of their dsp
> function in the kernel space instead of in the device.
> And as company don't want to publish their dsp algorith and open source
> it, we can't have open source driver for it.
>
> That the case for bewan device (that even include a libm in their
> source) and for pulsar pci device...
Nonono: I meant exactly to do an open card with an open dsp software. Next time hardware producers will think twice before refraining from disclosing card details...
After all, most producers didn't ever need to disclose their firmware as long as it is a binary file to be uploaded to the card. But still it took a lot of time to have a working ADSL driver under Linux, just because producers didn't want to disclose port assignments and the like. I.e.: they preferred not to disclose anything instead that just refraining to disclose the firmware, which would had to be enough for their purposes.
This is a behaviour that the linux community shall discourage. Designing an open hardware and software solution for ADSL connection would be a great way to avoid something like this in the future... You don't disclose? I offer an alternative which bypasses you.
The matter is not so easy, however: the ADSL standard is complex and dsp software has to take into account a lot of ADSL "flavors" (DSLAM producers often offer enhancements to the standard way), but it shouldn't be too difficult to the linux community to put together the needed gray matter...
Cheers,
giampaolo
^ permalink raw reply [flat|nested] 24+ messages in thread
* R: R: R: [Linux-ATM-General] [ATMSAR] Request for review - update #1
2005-09-04 19:11 ` matthieu castet
2005-09-04 19:33 ` R: " Giampaolo Tomassoni
@ 2005-09-04 19:42 ` Giampaolo Tomassoni
1 sibling, 0 replies; 24+ messages in thread
From: Giampaolo Tomassoni @ 2005-09-04 19:42 UTC (permalink / raw)
To: matthieu castet; +Cc: Francois Romieu, linux-kernel, linux-atm-general
> -----Messaggio originale-----
> Da: matthieu castet [mailto:castet.matthieu@free.fr]
> Inviato: domenica 4 settembre 2005 21.11
>
> ...omissis...
>
> The problem is that lot's of new devices implement part of their dsp
> function in the kernel space instead of in the device.
> And as company don't want to publish their dsp algorith and open source
> it, we can't have open source driver for it.
>
> That the case for bewan device (that even include a libm in their
> source) and for pulsar pci device...
Nonono: I meant exactly to do an open card with an open dsp software. Next time hardware producers will think twice before refraining from disclosing card details...
After all, most producers didn't ever need to disclose their firmware as long as it is a binary file to be uploaded to the card. But still it took a lot of time to have a working ADSL driver under Linux, just because producers didn't want to disclose port assignments and the like. I.e.: they preferred not to disclose anything instead that just refraining to disclose the firmware, which would had to be enough for their purposes.
This is a behaviour that the linux community shall discourage. Designing an open hardware and software solution for ADSL connection would be a great way to avoid something like this in the future... You don't disclose? I offer an alternative which bypasses you.
The matter is not so easy, however: the ADSL standard is complex and dsp software has to take into account a lot of ADSL "flavors" (DSLAM producers often offer enhancements to the standard way), but it shouldn't be too difficult to the linux community to put together the needed gray matter...
Anyway, all these speculations are definitely OT. Sorry about that.
Cheers,
giampaolo
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: R: [Linux-ATM-General] [ATMSAR] Request for review - update #1
2005-09-04 13:13 ` R: " Giampaolo Tomassoni
2005-09-04 15:32 ` Francois Romieu
@ 2005-09-05 6:05 ` Zoran Stojsavljevic
2005-09-05 6:21 ` R: " Giampaolo Tomassoni
1 sibling, 1 reply; 24+ messages in thread
From: Zoran Stojsavljevic @ 2005-09-05 6:05 UTC (permalink / raw)
To: Giampaolo Tomassoni; +Cc: Francois Romieu, linux-kernel, linux-atm-general
Giampaolo Tomassoni wrote:
>Also, I believe that adsl will carry much more services then just AAL5 for internet connection in the future. Even if the ATMSAR actually lacks of AAL1 and AAL2/3 capabilities, adding them in a single, specialized module is much easier than swimming in a usb+atm middle layer.
>
>
Giampaolo,
Should read: ...even if the ATMSAR actually lacks of AAL1, AAL2 and AAL3/4 [where AAL3/4 is obsolete] capabilities...
Just wanted to make it more precise according to the ATM standards, this was the only intention of my "patch".
Peace to all! :o)
^ permalink raw reply [flat|nested] 24+ messages in thread
* R: R: [Linux-ATM-General] [ATMSAR] Request for review - update #1
2005-09-05 6:05 ` Zoran Stojsavljevic
@ 2005-09-05 6:21 ` Giampaolo Tomassoni
0 siblings, 0 replies; 24+ messages in thread
From: Giampaolo Tomassoni @ 2005-09-05 6:21 UTC (permalink / raw)
To: Zoran Stojsavljevic, Giampaolo Tomassoni
Cc: Francois Romieu, linux-kernel, linux-atm-general
> Giampaolo,
>
> Should read: ...even if the ATMSAR actually lacks of AAL1, AAL2
> and AAL3/4 [where AAL3/4 is obsolete] capabilities...
>
> Just wanted to make it more precise according to the ATM
> standards, this was the only intention of my "patch".
You're right. Sorry about that.
Thanks,
Giampaolo
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [ATMSAR] Request for review - update #1
2005-09-04 11:05 [ATMSAR] Request for review - update #1 Giampaolo Tomassoni
2005-09-04 12:00 ` [Linux-ATM-General] " Francois Romieu
@ 2005-09-04 16:20 ` Alistair John Strachan
2005-09-04 16:41 ` Grzegorz Kulewski
` (3 more replies)
2005-09-04 16:55 ` Jiri Slaby
2 siblings, 4 replies; 24+ messages in thread
From: Alistair John Strachan @ 2005-09-04 16:20 UTC (permalink / raw)
To: Giampaolo Tomassoni; +Cc: linux-kernel, linux-atm-general
On Sunday 04 September 2005 12:05, Giampaolo Tomassoni wrote:
> Dears,
>
> thanks to Jiri Slaby who found a bug in the AAL0 handling of the ATMSAR
> module.
>
> I attach a fixed version of the atmsar patch as a diff against the 2.6.13
> kernel tree.
>
[snip]
Just out of curiosity, is there ANY reason why this has to be done in the
kernel? The PPPoATM module for pppd implements (via linux-atm) a completely
userspace ATM decoder.. if anything, now redundant ATM stack code should be
REMOVED from Linux!
Most distributions (to my knowledge) supporting the speedtouch modem do so
using the method prescribed on speedtouch.sf.net; an entirely userspace
procedure. pppd does all the ATM magic.
Does this have real-world applications beyond the Speedtouch DSL modems? If
not, I propose adding this code to linux-atm, not the kernel, since most
users of USB speedtouch DSL modems will not be using the kernel's ATM.
--
Cheers,
Alistair.
'No sense being pessimistic, it probably wouldn't work anyway.'
Third year Computer Science undergraduate.
1F2 55 South Clerk Street, Edinburgh, UK.
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [ATMSAR] Request for review - update #1
2005-09-04 16:20 ` Alistair John Strachan
@ 2005-09-04 16:41 ` Grzegorz Kulewski
2005-09-04 16:54 ` Alistair John Strachan
2005-09-04 18:36 ` R: " Giampaolo Tomassoni
` (2 subsequent siblings)
3 siblings, 1 reply; 24+ messages in thread
From: Grzegorz Kulewski @ 2005-09-04 16:41 UTC (permalink / raw)
To: Alistair John Strachan
Cc: Giampaolo Tomassoni, linux-kernel, linux-atm-general
On Sun, 4 Sep 2005, Alistair John Strachan wrote:
> On Sunday 04 September 2005 12:05, Giampaolo Tomassoni wrote:
>> Dears,
>>
>> thanks to Jiri Slaby who found a bug in the AAL0 handling of the ATMSAR
>> module.
>>
>> I attach a fixed version of the atmsar patch as a diff against the 2.6.13
>> kernel tree.
>>
> [snip]
>
> Just out of curiosity, is there ANY reason why this has to be done in the
> kernel? The PPPoATM module for pppd implements (via linux-atm) a completely
> userspace ATM decoder.. if anything, now redundant ATM stack code should be
> REMOVED from Linux!
>
> Most distributions (to my knowledge) supporting the speedtouch modem do so
> using the method prescribed on speedtouch.sf.net; an entirely userspace
> procedure. pppd does all the ATM magic.
>
> Does this have real-world applications beyond the Speedtouch DSL modems? If
> not, I propose adding this code to linux-atm, not the kernel, since most
> users of USB speedtouch DSL modems will not be using the kernel's ATM.
I am using SpeedTouch 330 modem with kernel driver (on Gentoo).
The instalation is currently (with firmware loader instead of modem_run)
very simple: USE="atm" emerge ppp, download firmware and place it in
/lib/firmware, compile the kernel with speedtch support.
I tried to use userspace driver some time ago but it wasn't working for me
so I gave up. I was using modem_run with kernel driver for long time to
load the firmware but there were many problems with it too (nearly every
kernel or modem_run upgrade was breaking something, modem_run was hanging
in D state in most unapropriate moments and so on).
Now I am using pure kernel driver and firmware loader and it works 100%
ok. There were no problems with it for long time. And I don't even want to
look at this userspace driver again.
Since Linux newer was (or is going to be) userspace-driver OS, maybe we
should leave it that way...
Grzegorz Kulewski
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [ATMSAR] Request for review - update #1
2005-09-04 16:41 ` Grzegorz Kulewski
@ 2005-09-04 16:54 ` Alistair John Strachan
2005-09-04 17:22 ` Grzegorz Kulewski
2005-09-05 15:04 ` Duncan Sands
0 siblings, 2 replies; 24+ messages in thread
From: Alistair John Strachan @ 2005-09-04 16:54 UTC (permalink / raw)
To: Grzegorz Kulewski; +Cc: Giampaolo Tomassoni, linux-kernel, linux-atm-general
On Sunday 04 September 2005 17:41, Grzegorz Kulewski wrote:
> On Sun, 4 Sep 2005, Alistair John Strachan wrote:
> > On Sunday 04 September 2005 12:05, Giampaolo Tomassoni wrote:
> >> Dears,
> >>
> >> thanks to Jiri Slaby who found a bug in the AAL0 handling of the ATMSAR
> >> module.
> >>
> >> I attach a fixed version of the atmsar patch as a diff against the
> >> 2.6.13 kernel tree.
> >
> > [snip]
> >
> > Just out of curiosity, is there ANY reason why this has to be done in the
> > kernel? The PPPoATM module for pppd implements (via linux-atm) a
> > completely userspace ATM decoder.. if anything, now redundant ATM stack
> > code should be REMOVED from Linux!
> >
> > Most distributions (to my knowledge) supporting the speedtouch modem do
> > so using the method prescribed on speedtouch.sf.net; an entirely
> > userspace procedure. pppd does all the ATM magic.
> >
> > Does this have real-world applications beyond the Speedtouch DSL modems?
> > If not, I propose adding this code to linux-atm, not the kernel, since
> > most users of USB speedtouch DSL modems will not be using the kernel's
> > ATM.
>
> I am using SpeedTouch 330 modem with kernel driver (on Gentoo).
>
> The instalation is currently (with firmware loader instead of modem_run)
> very simple: USE="atm" emerge ppp, download firmware and place it in
> /lib/firmware, compile the kernel with speedtch support.
Compared to "place the firmware in /lib/firmware" on many other distros, this
sounds like a lot of work! The kernel speedtch provides no advantages to its
userspace alternative.
> I tried to use userspace driver some time ago but it wasn't working for me
> so I gave up. I was using modem_run with kernel driver for long time to
> load the firmware but there were many problems with it too (nearly every
> kernel or modem_run upgrade was breaking something, modem_run was hanging
> in D state in most unapropriate moments and so on).
This is not the case any longer.
> Now I am using pure kernel driver and firmware loader and it works 100%
> ok. There were no problems with it for long time. And I don't even want to
> look at this userspace driver again.
Conversely people (including myself) found the kernel implementation to be
buggy, and when userspace breaks, you can simply restart it.. when the kernel
breaks, you have to reboot.
> Since Linux newer was (or is going to be) userspace-driver OS, maybe we
> should leave it that way...
No.
What can be done in userspace, within valid performance constraints, usually
should be. This has always been the Linux kernel way.
The speedtouch modem is a USB device, and there are many existing userspace
"driver" implementations for USB devices. Including speedtouch.
I'm not necessarily saying this code shouldn't be in the kernel, I'd just be
interested to know why it has to be.
--
Cheers,
Alistair.
'No sense being pessimistic, it probably wouldn't work anyway.'
Third year Computer Science undergraduate.
1F2 55 South Clerk Street, Edinburgh, UK.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [ATMSAR] Request for review - update #1
2005-09-04 16:54 ` Alistair John Strachan
@ 2005-09-04 17:22 ` Grzegorz Kulewski
2005-09-05 15:04 ` Duncan Sands
1 sibling, 0 replies; 24+ messages in thread
From: Grzegorz Kulewski @ 2005-09-04 17:22 UTC (permalink / raw)
To: Alistair John Strachan
Cc: Giampaolo Tomassoni, linux-kernel, linux-atm-general
On Sun, 4 Sep 2005, Alistair John Strachan wrote:
> On Sunday 04 September 2005 17:41, Grzegorz Kulewski wrote:
>> On Sun, 4 Sep 2005, Alistair John Strachan wrote:
>>> On Sunday 04 September 2005 12:05, Giampaolo Tomassoni wrote:
>>>> Dears,
>>>>
>>>> thanks to Jiri Slaby who found a bug in the AAL0 handling of the ATMSAR
>>>> module.
>>>>
>>>> I attach a fixed version of the atmsar patch as a diff against the
>>>> 2.6.13 kernel tree.
>>>
>>> [snip]
>>>
>>> Just out of curiosity, is there ANY reason why this has to be done in the
>>> kernel? The PPPoATM module for pppd implements (via linux-atm) a
>>> completely userspace ATM decoder.. if anything, now redundant ATM stack
>>> code should be REMOVED from Linux!
>>>
>>> Most distributions (to my knowledge) supporting the speedtouch modem do
>>> so using the method prescribed on speedtouch.sf.net; an entirely
>>> userspace procedure. pppd does all the ATM magic.
>>>
>>> Does this have real-world applications beyond the Speedtouch DSL modems?
>>> If not, I propose adding this code to linux-atm, not the kernel, since
>>> most users of USB speedtouch DSL modems will not be using the kernel's
>>> ATM.
>>
>> I am using SpeedTouch 330 modem with kernel driver (on Gentoo).
>>
>> The instalation is currently (with firmware loader instead of modem_run)
>> very simple: USE="atm" emerge ppp, download firmware and place it in
>> /lib/firmware, compile the kernel with speedtch support.
>
> Compared to "place the firmware in /lib/firmware" on many other distros, this
> sounds like a lot of work! The kernel speedtch provides no advantages to its
> userspace alternative.
Are you saying that you do not need to install ppp and compile (or install
binary) kernel on other distros???
>> I tried to use userspace driver some time ago but it wasn't working for me
>> so I gave up. I was using modem_run with kernel driver for long time to
>> load the firmware but there were many problems with it too (nearly every
>> kernel or modem_run upgrade was breaking something, modem_run was hanging
>> in D state in most unapropriate moments and so on).
>
> This is not the case any longer.
Maybe. But there were many bugs in modem_run, usbfs, usb drivers in
kernel, ACPI, IRQ routing and so on. They caused modem_run to hang or do
something stupid without even telling user what is broken. In my
experience when one bug was fixed other appeared in the next kernel. So
even if now everything works ok you have no guarantee that next release
won't break something... :)
>> Now I am using pure kernel driver and firmware loader and it works 100%
>> ok. There were no problems with it for long time. And I don't even want to
>> look at this userspace driver again.
>
> Conversely people (including myself) found the kernel implementation to be
> buggy, and when userspace breaks, you can simply restart it.. when the kernel
> breaks, you have to reboot.
1. But you still use the kernel even with userspace driver. So it still
can break forcing you to reboot.
2. I have no problems with kernel driver. All problems that I saw in the
past were problems in other subsystems (usbfs, usb drivers, ACPI, IRQ, ...).
3. Restarting modem_run was never enought for me. I always at least had to
unplug the modem from USB port. Or more often reboot. So I see no gain
here.
>> Since Linux newer was (or is going to be) userspace-driver OS, maybe we
>> should leave it that way...
>
> No.
>
> What can be done in userspace, within valid performance constraints, usually
> should be. This has always been the Linux kernel way.
But not device drivers. I do not think that Linux has good infrastructure
for drivers in userspace (yes I know that there are some). Also are you
sure that performance and latencies are ok with userspace driver even if
system is under load?
> The speedtouch modem is a USB device, and there are many existing userspace
> "driver" implementations for USB devices. Including speedtouch.
>
> I'm not necessarily saying this code shouldn't be in the kernel, I'd just be
> interested to know why it has to be.
Well, as you are saing it hasn't... :) But since it is there and works
for me I am interested in not changing this state without really good
reason.
Grzegorz Kulewski
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [ATMSAR] Request for review - update #1
2005-09-04 16:54 ` Alistair John Strachan
2005-09-04 17:22 ` Grzegorz Kulewski
@ 2005-09-05 15:04 ` Duncan Sands
1 sibling, 0 replies; 24+ messages in thread
From: Duncan Sands @ 2005-09-05 15:04 UTC (permalink / raw)
To: Alistair John Strachan
Cc: Grzegorz Kulewski, Giampaolo Tomassoni, linux-kernel,
linux-atm-general
Hi Alistair,
> > The instalation is currently (with firmware loader instead of modem_run)
> > very simple: USE="atm" emerge ppp, download firmware and place it in
> > /lib/firmware, compile the kernel with speedtch support.
>
> Compared to "place the firmware in /lib/firmware" on many other distros, this
> sounds like a lot of work! The kernel speedtch provides no advantages to its
> userspace alternative.
historically the main problem with using the kernel driver was getting hold of
an ATM aware pppd. The step "USE="atm" emerge ppp" looks like gentoos way of
installing such a pppd. Nowadays several major distributions ship with an
ATM aware pppd, so if you are using one there is nothing to be done. Likewise,
most distributions ship a kernel with speedtch support. So if you're using
such a distribution all you have to do to use the kernel driver is
"place the firmware in /lib/firmware".
On the other hand, it is misleading to say that with the user mode driver
all you have to do is place the firmware in /lib/firmware. That's only
true if your distribution (eg: Mandrake) explicitly supports the user mode
driver and has pre-installed and configured it for you. If you don't have
such a distribution then setting up the user mode driver, while not difficult,
does require some work.
> > I tried to use userspace driver some time ago but it wasn't working for me
> > so I gave up. I was using modem_run with kernel driver for long time to
> > load the firmware but there were many problems with it too (nearly every
> > kernel or modem_run upgrade was breaking something, modem_run was hanging
> > in D state in most unapropriate moments and so on).
>
> This is not the case any longer.
I'm the one who fixed most of those problems by the way.
> > Now I am using pure kernel driver and firmware loader and it works 100%
> > ok. There were no problems with it for long time. And I don't even want to
> > look at this userspace driver again.
>
> Conversely people (including myself) found the kernel implementation to be
> buggy, and when userspace breaks, you can simply restart it.. when the kernel
> breaks, you have to reboot.
Tell me what problems you've been seeing and I will try to fix them.
All the best,
Duncan.
^ permalink raw reply [flat|nested] 24+ messages in thread
* R: [ATMSAR] Request for review - update #1
2005-09-04 16:20 ` Alistair John Strachan
2005-09-04 16:41 ` Grzegorz Kulewski
@ 2005-09-04 18:36 ` Giampaolo Tomassoni
2005-09-04 18:51 ` Alistair John Strachan
2005-09-05 9:36 ` David Woodhouse
2005-09-05 14:46 ` Duncan Sands
3 siblings, 1 reply; 24+ messages in thread
From: Giampaolo Tomassoni @ 2005-09-04 18:36 UTC (permalink / raw)
To: Alistair John Strachan, Giampaolo Tomassoni
Cc: linux-kernel, linux-atm-general
> -----Messaggio originale-----
> Da: Alistair John Strachan [mailto:s0348365@sms.ed.ac.uk]
> Inviato: domenica 4 settembre 2005 18.21
>
> ...omissis...
>
> Just out of curiosity, is there ANY reason why this has to be done in the
> kernel? The PPPoATM module for pppd implements (via linux-atm) a
> completely
> userspace ATM decoder.. if anything, now redundant ATM stack code
> should be
> REMOVED from Linux!
This may be true for AAL5 support, which is the way by which data is actually transferred between ADSL DSLAMs and CPE equipment.
This may not be generally true, however: most providers are already delivering internet+voice solutions over ADSL channels (here in Italy, in example, Telecom offers Alice Mia, which is an ADSL line with internet access and VoIP for added voice capabilities). Albeit the voice part of these solutions are actually based on VoIP technology, it is not the best way to do this. In the future, I believe we will easily see internet + voice sols based over AAL5 + AAL2/3, or even multi voice channels over AAL2/3 over ADSL (replacing ISDN PRIs and multi-BRIs -based lines for PABX connection).
When (and if) that will happen, we will probabily need a kernel-based solution since cell timing and QoS is a much stricter requirement with non-AAL5 encodings, such that it is easier to attain from inside the kernel than from userland.
So, I'm not that shure all the ATM code is to be stripped out of the kernel. Maybe it can be done with the PPPoATM network interface. But probably it can't be done with the ATM core and the ATM SAR code. Wherever the latter will be in ATMUSB, in ATMSAR or in a device driver.
The PPPoATM module is a network interface. It stays on the other side of the ATM world: [netif] <-> [PPPoATM] <-> [atmif] <-> [ATM] <-> [ATMSAR] <-> [device driver]. I'm not a PPPoATM expert, but it may probably be possible to have all the PPPoATM code in userland. But the [ATM] <-> [ATMSAR] <-> [device driver] chain is probably too close to hardware to gain any benefit by "userlanding" it.
>
> ...omissis...
>
>
> --
> Cheers,
> Alistair.
Cheers,
giampaolo
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: R: [ATMSAR] Request for review - update #1
2005-09-04 18:36 ` R: " Giampaolo Tomassoni
@ 2005-09-04 18:51 ` Alistair John Strachan
0 siblings, 0 replies; 24+ messages in thread
From: Alistair John Strachan @ 2005-09-04 18:51 UTC (permalink / raw)
To: Giampaolo Tomassoni; +Cc: linux-kernel, linux-atm-general
On Sunday 04 September 2005 19:36, Giampaolo Tomassoni wrote:
[snip]
>
> This may be true for AAL5 support, which is the way by which data is
> actually transferred between ADSL DSLAMs and CPE equipment.
>
> This may not be generally true, however: most providers are already
> delivering internet+voice solutions over ADSL channels (here in Italy, in
> example, Telecom offers Alice Mia, which is an ADSL line with internet
> access and VoIP for added voice capabilities). Albeit the voice part of
> these solutions are actually based on VoIP technology, it is not the best
> way to do this. In the future, I believe we will easily see internet +
> voice sols based over AAL5 + AAL2/3, or even multi voice channels over
> AAL2/3 over ADSL (replacing ISDN PRIs and multi-BRIs -based lines for PABX
> connection).
>
> When (and if) that will happen, we will probabily need a kernel-based
> solution since cell timing and QoS is a much stricter requirement with
> non-AAL5 encodings, such that it is easier to attain from inside the kernel
> than from userland.
>
> So, I'm not that shure all the ATM code is to be stripped out of the
> kernel. Maybe it can be done with the PPPoATM network interface. But
> probably it can't be done with the ATM core and the ATM SAR code. Wherever
> the latter will be in ATMUSB, in ATMSAR or in a device driver.
The above is a decent technical justification for why the code is generally
required; it's set my mind at rest, I thought maybe this was only for the
speedtouch modem crew.
I was aware of ATM's ability to interleave data and "digital voice" services;
ATM is widely deployed across Europe in preparation for "pure digital"
consumer telephony..
My primary concern with the ATM code is that it's not (yet) an often used part
of the kernel; there are a number of viable applications out there, but most
can happily use linux-atm and/or pppd. I can see VoIP clients doing the same.
However, for embedded or non-pure-client purposes, there's a reason for an
in-kernel implementation.
I don't know enough about the applications of the "ATM stack" versus using a
userspace library, so I think as long as the patch is cleaned up and can be
reused, it's okay to put in the kernel.
>
> The PPPoATM module is a network interface. It stays on the other side of
> the ATM world: [netif] <-> [PPPoATM] <-> [atmif] <-> [ATM] <-> [ATMSAR] <->
> [device driver]. I'm not a PPPoATM expert, but it may probably be possible
> to have all the PPPoATM code in userland. But the [ATM] <-> [ATMSAR] <->
> [device driver] chain is probably too close to hardware to gain any benefit
> by "userlanding" it.
>
I agree, if there are users beyond the speedtouch modem, it may make sense to
have this code in the kernel. Thanks for fielding my questions.
--
Cheers,
Alistair.
'No sense being pessimistic, it probably wouldn't work anyway.'
Third year Computer Science undergraduate.
1F2 55 South Clerk Street, Edinburgh, UK.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [ATMSAR] Request for review - update #1
2005-09-04 16:20 ` Alistair John Strachan
2005-09-04 16:41 ` Grzegorz Kulewski
2005-09-04 18:36 ` R: " Giampaolo Tomassoni
@ 2005-09-05 9:36 ` David Woodhouse
2005-09-05 13:52 ` Alistair John Strachan
2005-09-05 14:46 ` Duncan Sands
3 siblings, 1 reply; 24+ messages in thread
From: David Woodhouse @ 2005-09-05 9:36 UTC (permalink / raw)
To: Alistair John Strachan
Cc: Giampaolo Tomassoni, linux-kernel, linux-atm-general
On Sun, 2005-09-04 at 17:20 +0100, Alistair John Strachan wrote:
> Just out of curiosity, is there ANY reason why this has to be done in the
> kernel? The PPPoATM module for pppd implements (via linux-atm) a completely
> userspace ATM decoder.. if anything, now redundant ATM stack code should be
> REMOVED from Linux!
No. The pppoatm module for pppd uses the kernel ATM stack and kernel
PPPoATM functionality. I suspect you're thinking of the pseudo-tty hack
used by the userspace code.
> Most distributions (to my knowledge) supporting the speedtouch modem do so
> using the method prescribed on speedtouch.sf.net; an entirely userspace
> procedure. pppd does all the ATM magic.
Fedora doesn't; it uses the kernel driver.
--
dwmw2
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [ATMSAR] Request for review - update #1
2005-09-05 9:36 ` David Woodhouse
@ 2005-09-05 13:52 ` Alistair John Strachan
2005-09-05 13:56 ` David Woodhouse
0 siblings, 1 reply; 24+ messages in thread
From: Alistair John Strachan @ 2005-09-05 13:52 UTC (permalink / raw)
To: David Woodhouse; +Cc: Giampaolo Tomassoni, linux-kernel, linux-atm-general
On Monday 05 September 2005 10:36, David Woodhouse wrote:
> On Sun, 2005-09-04 at 17:20 +0100, Alistair John Strachan wrote:
> > Just out of curiosity, is there ANY reason why this has to be done in the
> > kernel? The PPPoATM module for pppd implements (via linux-atm) a
> > completely userspace ATM decoder.. if anything, now redundant ATM stack
> > code should be REMOVED from Linux!
>
> No. The pppoatm module for pppd uses the kernel ATM stack and kernel
> PPPoATM functionality. I suspect you're thinking of the pseudo-tty hack
> used by the userspace code.
I'm not sure which module you're referring to, but the patch recommended by
the speedtouch people links to linux-atm, and does not require kernel ATM or
kernel pppoatm functionality, or use any kernel modules. I do notice it does
a system ("/sbin/modprobe pppoatm"); but this is definitely not required; I'm
speaking to you from a speedtouch DSL connection, no module loaded or
compiled in, no ATM support in the kernel.
http://devzero.co.uk/~alistair/linuxmisc/ppp-2.4.3-atm.diff
>
> > Most distributions (to my knowledge) supporting the speedtouch modem do
> > so using the method prescribed on speedtouch.sf.net; an entirely
> > userspace procedure. pppd does all the ATM magic.
>
> Fedora doesn't; it uses the kernel driver.
I stand corrected.
--
Cheers,
Alistair.
'No sense being pessimistic, it probably wouldn't work anyway.'
Third year Computer Science undergraduate.
1F2 55 South Clerk Street, Edinburgh, UK.
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [ATMSAR] Request for review - update #1
2005-09-05 13:52 ` Alistair John Strachan
@ 2005-09-05 13:56 ` David Woodhouse
2005-09-05 14:18 ` Alistair John Strachan
0 siblings, 1 reply; 24+ messages in thread
From: David Woodhouse @ 2005-09-05 13:56 UTC (permalink / raw)
To: Alistair John Strachan
Cc: Giampaolo Tomassoni, linux-kernel, linux-atm-general
On Mon, 2005-09-05 at 14:52 +0100, Alistair John Strachan wrote:
> I'm not sure which module you're referring to, but the patch recommended by
> the speedtouch people links to linux-atm, and does not require kernel ATM or
> kernel pppoatm functionality, or use any kernel modules. I do notice it does
> a system ("/sbin/modprobe pppoatm"); but this is definitely not required; I'm
> speaking to you from a speedtouch DSL connection, no module loaded or
> compiled in, no ATM support in the kernel.
Then you're not using the pppoatm plugin; you needn't bother applying
that patch. You're probably just using the pseudo-tty hack.
--
dwmw2
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [ATMSAR] Request for review - update #1
2005-09-05 13:56 ` David Woodhouse
@ 2005-09-05 14:18 ` Alistair John Strachan
2005-09-05 14:32 ` David Woodhouse
0 siblings, 1 reply; 24+ messages in thread
From: Alistair John Strachan @ 2005-09-05 14:18 UTC (permalink / raw)
To: David Woodhouse; +Cc: Giampaolo Tomassoni, linux-kernel, linux-atm-general
On Monday 05 September 2005 14:56, David Woodhouse wrote:
> On Mon, 2005-09-05 at 14:52 +0100, Alistair John Strachan wrote:
> > I'm not sure which module you're referring to, but the patch recommended
> > by the speedtouch people links to linux-atm, and does not require kernel
> > ATM or kernel pppoatm functionality, or use any kernel modules. I do
> > notice it does a system ("/sbin/modprobe pppoatm"); but this is
> > definitely not required; I'm speaking to you from a speedtouch DSL
> > connection, no module loaded or compiled in, no ATM support in the
> > kernel.
>
> Then you're not using the pppoatm plugin; you needn't bother applying
> that patch. You're probably just using the pseudo-tty hack.
Ahh yes, I was confusing the pppd module with pppoa3, a userspace ppp-over-atm
handler. Thanks for the correction.
Still, I don't feel this detracts from the point that client ATM DSL device
"drivers" can exist happily in userspace.
--
Cheers,
Alistair.
'No sense being pessimistic, it probably wouldn't work anyway.'
Third year Computer Science undergraduate.
1F2 55 South Clerk Street, Edinburgh, UK.
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [ATMSAR] Request for review - update #1
2005-09-05 14:18 ` Alistair John Strachan
@ 2005-09-05 14:32 ` David Woodhouse
0 siblings, 0 replies; 24+ messages in thread
From: David Woodhouse @ 2005-09-05 14:32 UTC (permalink / raw)
To: Alistair John Strachan
Cc: Giampaolo Tomassoni, linux-kernel, linux-atm-general
On Mon, 2005-09-05 at 15:18 +0100, Alistair John Strachan wrote:
> Still, I don't feel this detracts from the point that client ATM DSL
> device "drivers" can exist happily in userspace.
Indeed they can. There's been lots of academic research into
microkernels which supports your assertion.
--
dwmw2
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [ATMSAR] Request for review - update #1
2005-09-04 16:20 ` Alistair John Strachan
` (2 preceding siblings ...)
2005-09-05 9:36 ` David Woodhouse
@ 2005-09-05 14:46 ` Duncan Sands
3 siblings, 0 replies; 24+ messages in thread
From: Duncan Sands @ 2005-09-05 14:46 UTC (permalink / raw)
To: Alistair John Strachan
Cc: Giampaolo Tomassoni, linux-kernel, linux-atm-general
Hi Alistair,
> Just out of curiosity, is there ANY reason why this has to be done in the
> kernel? The PPPoATM module for pppd implements (via linux-atm) a completely
> userspace ATM decoder.. if anything, now redundant ATM stack code should be
> REMOVED from Linux!
the main advantage of the kernel module is that it is integrated into the
kernel's ATM layer. That means that you can use all those great features
the ATM layer provides to do more with your modem. This doesn't matter for
most people: they have a PPPoA or PPPoE connection and aren't looking to do
clever tricks; the user mode driver is fine for them. But it is not enough
for everyone. For example, my ISP uses "routed IP", which is not supported
by the user mode driver, but works fine with the kernel driver thanks to the
ATM layer.
All the best,
Duncan.
PS: linux-atm and the pppoatm module are used with the kernel driver;
presumably you meant pppoa2 or pppoa3.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [ATMSAR] Request for review - update #1
2005-09-04 11:05 [ATMSAR] Request for review - update #1 Giampaolo Tomassoni
2005-09-04 12:00 ` [Linux-ATM-General] " Francois Romieu
2005-09-04 16:20 ` Alistair John Strachan
@ 2005-09-04 16:55 ` Jiri Slaby
2 siblings, 0 replies; 24+ messages in thread
From: Jiri Slaby @ 2005-09-04 16:55 UTC (permalink / raw)
To: Giampaolo Tomassoni; +Cc: linux-kernel, linux-atm-general
Giampaolo Tomassoni napsal(a):
>I attach a fixed version of the atmsar patch as a diff against the 2.6.13 kernel tree.
>
>
static inline void dump_skb (char * prefix, unsigned int vc, struct
sk_buff * skb) {
what's this? 81+ chars on line
{ on a newline, please
' * ' --> ' *'
spin_lock_irqsave (&txq->lock, flags);
indent is tab (tab is as long as 8 chars), no 3, 4, 5 or ... spaces
and many, many others, please read CodingStyle in Documentation.
thanks,
--
Jiri Slaby www.fi.muni.cz/~xslaby
~\-/~ jirislaby@gmail.com ~\-/~
241B347EC88228DE51EE A49C4A73A25004CB2A10
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2005-09-05 15:04 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-09-04 11:05 [ATMSAR] Request for review - update #1 Giampaolo Tomassoni
2005-09-04 12:00 ` [Linux-ATM-General] " Francois Romieu
2005-09-04 13:13 ` R: " Giampaolo Tomassoni
2005-09-04 15:32 ` Francois Romieu
2005-09-04 18:44 ` R: " Giampaolo Tomassoni
2005-09-04 19:11 ` matthieu castet
2005-09-04 19:33 ` R: " Giampaolo Tomassoni
2005-09-04 19:42 ` Giampaolo Tomassoni
2005-09-05 6:05 ` Zoran Stojsavljevic
2005-09-05 6:21 ` R: " Giampaolo Tomassoni
2005-09-04 16:20 ` Alistair John Strachan
2005-09-04 16:41 ` Grzegorz Kulewski
2005-09-04 16:54 ` Alistair John Strachan
2005-09-04 17:22 ` Grzegorz Kulewski
2005-09-05 15:04 ` Duncan Sands
2005-09-04 18:36 ` R: " Giampaolo Tomassoni
2005-09-04 18:51 ` Alistair John Strachan
2005-09-05 9:36 ` David Woodhouse
2005-09-05 13:52 ` Alistair John Strachan
2005-09-05 13:56 ` David Woodhouse
2005-09-05 14:18 ` Alistair John Strachan
2005-09-05 14:32 ` David Woodhouse
2005-09-05 14:46 ` Duncan Sands
2005-09-04 16:55 ` Jiri Slaby
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox