linux-atm-general — General discussion of ATM networking under linux

[Linux-ATM-General] [PATCH 4/5] atm: Introduce vcc_process_recv_queue

[Jorge B. [DTI2] <jo...@dt...>]

From: "Jorge Boncompte [DTI2]" <jo...@dt...>

    This function moves the implementation found in the clip and br2684
modules to common code, correctly unlinks the skb from the queue before
pushing it and makes pppoatm use it.

Signed-off-by: Jorge Boncompte [DTI2] <jo...@dt...>
---
 net/atm/br2684.c  |   20 +++++---------------
 net/atm/clip.c    |   13 +------------
 net/atm/common.c  |   20 ++++++++++++++++++++
 net/atm/common.h  |    1 +
 net/atm/pppoatm.c |    4 ++++
 5 files changed, 31 insertions(+), 27 deletions(-)

diff --git a/net/atm/br2684.c b/net/atm/br2684.c
index 81cf33b..53b0aa1 100644
--- a/net/atm/br2684.c
+++ b/net/atm/br2684.c
@@ -489,15 +489,11 @@ free_skb:
  */
 static int br2684_regvcc(struct atm_vcc *atmvcc, void __user * arg)
 {
-	struct sk_buff_head queue;
-	int err;
 	struct br2684_vcc *brvcc;
-	struct sk_buff *skb, *tmp;
-	struct sk_buff_head *rq;
 	struct br2684_dev *brdev;
 	struct net_device *net_dev;
 	struct atm_backend_br2684 be;
-	unsigned long flags;
+	int err;
 
 	if (copy_from_user(&be, arg, sizeof be))
 		return -EFAULT;
@@ -550,16 +546,6 @@ static int br2684_regvcc(struct atm_vcc *atmvcc, void __user * arg)
 	atmvcc->push = br2684_push;
 	atmvcc->pop = br2684_pop;
 
-	__skb_queue_head_init(&queue);
-	rq = &sk_atm(atmvcc)->sk_receive_queue;
-
-	spin_lock_irqsave(&rq->lock, flags);
-	skb_queue_splice_init(rq, &queue);
-	spin_unlock_irqrestore(&rq->lock, flags);
-
-	skb_queue_walk_safe(&queue, skb, tmp)
-		br2684_push(atmvcc, skb);
-
 	/* initialize netdev carrier state */
 	if (atmvcc->dev->signal == ATM_PHY_SIG_LOST)
 		netif_carrier_off(net_dev);
@@ -567,6 +553,10 @@ static int br2684_regvcc(struct atm_vcc *atmvcc, void __user * arg)
 		netif_carrier_on(net_dev);
 
 	__module_get(THIS_MODULE);
+
+	/* re-process everything received between connection setup and
+	   backend setup */
+	vcc_process_recv_queue(atmvcc);
 	return 0;
 
 error:
diff --git a/net/atm/clip.c b/net/atm/clip.c
index 521b45b..3d2ac66 100644
--- a/net/atm/clip.c
+++ b/net/atm/clip.c
@@ -455,10 +455,7 @@ static netdev_tx_t clip_start_xmit(struct sk_buff *skb,
 
 static int clip_mkip(struct atm_vcc *vcc, int timeout)
 {
-	struct sk_buff_head *rq, queue;
 	struct clip_vcc *clip_vcc;
-	struct sk_buff *skb, *tmp;
-	unsigned long flags;
 
 	if (!vcc->push)
 		return -EBADFD;
@@ -479,16 +476,8 @@ static int clip_mkip(struct atm_vcc *vcc, int timeout)
 	vcc->push = clip_push;
 	vcc->pop = clip_pop;
 
-	__skb_queue_head_init(&queue);
-	rq = &sk_atm(vcc)->sk_receive_queue;
-
-	spin_lock_irqsave(&rq->lock, flags);
-	skb_queue_splice_init(rq, &queue);
-	spin_unlock_irqrestore(&rq->lock, flags);
-
 	/* re-process everything received between connection setup and MKIP */
-	skb_queue_walk_safe(&queue, skb, tmp)
-		clip_push(vcc, skb);
+	vcc_process_recv_queue(vcc);
 
 	return 0;
 }
diff --git a/net/atm/common.c b/net/atm/common.c
index 14ff9fe..0b4c58f 100644
--- a/net/atm/common.c
+++ b/net/atm/common.c
@@ -214,6 +214,26 @@ void vcc_release_async(struct atm_vcc *vcc, int reply)
 }
 EXPORT_SYMBOL(vcc_release_async);
 
+void vcc_process_recv_queue(struct atm_vcc *vcc)
+{
+	struct sk_buff_head queue, *rq;
+	struct sk_buff *skb, *tmp;
+	unsigned long flags;
+
+	__skb_queue_head_init(&queue);
+	rq = &sk_atm(vcc)->sk_receive_queue;
+
+	spin_lock_irqsave(&rq->lock, flags);
+	skb_queue_splice_init(rq, &queue);
+	spin_unlock_irqrestore(&rq->lock, flags);
+
+	skb_queue_walk_safe(&queue, skb, tmp) {
+		__skb_unlink(skb, &queue);
+		vcc->push(vcc, skb);
+	}
+}
+EXPORT_SYMBOL(vcc_process_recv_queue);
+
 void atm_dev_signal_change(struct atm_dev *dev, char signal)
 {
 	pr_debug("%s signal=%d dev=%p number=%d dev->signal=%d\n",
diff --git a/net/atm/common.h b/net/atm/common.h
index f48a76b..cc3c2da 100644
--- a/net/atm/common.h
+++ b/net/atm/common.h
@@ -24,6 +24,7 @@ int vcc_setsockopt(struct socket *sock, int level, int optname,
 		   char __user *optval, unsigned int optlen);
 int vcc_getsockopt(struct socket *sock, int level, int optname,
 		   char __user *optval, int __user *optlen);
+void vcc_process_recv_queue(struct atm_vcc *vcc);
 
 int atmpvc_init(void);
 void atmpvc_exit(void);
diff --git a/net/atm/pppoatm.c b/net/atm/pppoatm.c
index db4a11c..df35d9a 100644
--- a/net/atm/pppoatm.c
+++ b/net/atm/pppoatm.c
@@ -303,6 +303,10 @@ static int pppoatm_assign_vcc(struct atm_vcc *atmvcc, void __user *arg)
 	atmvcc->push = pppoatm_push;
 	atmvcc->pop = pppoatm_pop;
 	__module_get(THIS_MODULE);
+
+	/* re-process everything received between connection setup and
+	   backend setup */
+	vcc_process_recv_queue(atmvcc);
 	return 0;
 }
 
-- 
1.7.7.1

[Linux-ATM-General] [PATCH net-next 0/5] atm: cleanups and MSK_PEEK for atm sockets

[Jorge B. [DTI2] <jo...@dt...>]

From: "Jorge Boncompte [DTI2]" <jo...@dt...>

    This series does some cleanup and introduces MSG_PEEK support for ATM
sockets.

Jorge Boncompte [DTI2] (5):
  atm: br2684: Do not move counters backwards
  atm: clip: Don't move counters backwards
  atm: clip: move clip_devs check to clip_push
  atm: Introduce vcc_process_recv_queue
  atm: Allow MSG_PEEK for atm sockets

 net/atm/br2684.c  |   27 +++++----------------------
 net/atm/clip.c    |   34 +++++++++-------------------------
 net/atm/common.c  |   34 +++++++++++++++++++++++++++++++---
 net/atm/common.h  |    1 +
 net/atm/pppoatm.c |    4 ++++
 5 files changed, 50 insertions(+), 50 deletions(-)

-- 
1.7.7.1

Re: [Linux-ATM-General] [Patch 3/3 v1] atm: br2684: Avoid alignment issues

[chas w. - C. <ch...@cm...>]

no.  the repo's on kernel.org were pulled and i havent gotten around to
it since everything has been put back.  i should have some time this
weekend.

On Fri, 18 Nov 2011 13:13:32 -0700
Philip Prindeville <phi...@re...> wrote:

> Did any of these patches make it upstream?  I never saw an ack from David and David...
> 
> -Philip
> 
> On 8/17/11 12:37 AM, Pascal Hambourg wrote:
> > Use memcmp() instead of cast to u16 when checking the PAD field.
> > 
> > Signed-off-by: Pascal Hambourg <pa...@pl...>
> > ---
> >  net/atm/br2684.c |    5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff -uprN a/net/atm/br2684.c b/net/atm/br2684.c
> > --- a/net/atm/br2684.c	2011-08-16 22:13:47.000000000 +0200
> > +++ b/net/atm/br2684.c	2011-08-16 22:13:49.000000000 +0200
> > @@ -53,6 +53,7 @@ static const unsigned char ethertype_ipv
> >  static const unsigned char ethertype_ipv6[] = { ETHERTYPE_IPV6 };
> >  static const unsigned char llc_oui_pid_pad[] =
> >  			{ LLC, SNAP_BRIDGED, PID_ETHERNET, PAD_BRIDGED };
> > +static const unsigned char pad[] = { PAD_BRIDGED };
> >  static const unsigned char llc_oui_ipv4[] = { LLC, SNAP_ROUTED, ETHERTYPE_IPV4 };
> >  static const unsigned char llc_oui_ipv6[] = { LLC, SNAP_ROUTED, ETHERTYPE_IPV6 };
> > 
> > @@ -434,7 +435,7 @@ static void br2684_push(struct atm_vcc *
> >  			 */
> >  			if ((skb->len >= sizeof(llc_oui_pid_pad) + ETH_HLEN) &&
> >  			    (memcmp(skb->data, llc_oui_pid_pad, 7) == 0) &&
> > -			    (*((u16 *) (skb->data + 8)) == 0)) {
> > +			    (memcmp(skb->data + 8, pad, BR2684_PAD_LEN) == 0)) {
> >  				if (skb->data[7] == 0x01 &&
> >  				    skb->len >= sizeof(llc_oui_pid_pad) + ETH_ZLEN + ETH_FCS_LEN)
> >  					__skb_trim(skb, skb->len - ETH_FCS_LEN); /* trim FCS */
> > @@ -464,7 +465,7 @@ static void br2684_push(struct atm_vcc *
> >  		} else { /* p_bridged */
> >  			/* first 2 chars should be 0 */
> >  			if (skb->len < BR2684_PAD_LEN + ETH_HLEN ||
> > -			    *((u16 *) (skb->data)) != 0)
> > +			    memcmp(skb->data, pad, BR2684_PAD_LEN) != 0)
> >  				goto error;
> >  			skb_pull(skb, BR2684_PAD_LEN);
> >  			skb->protocol = eth_type_trans(skb, net_dev);
> > 
> 
> ------------------------------------------------------------------------------
> All the data continuously generated in your IT infrastructure 
> contains a definitive record of customers, application performance, 
> security threats, fraudulent activity, and more. Splunk takes this 
> data and makes sense of it. IT sense. And common sense.
> http://p.sf.net/sfu/splunk-novd2d
> _______________________________________________
> Linux-atm-general mailing list
> Lin...@li...
> https://lists.sourceforge.net/lists/listinfo/linux-atm-general
> 

Re: [Linux-ATM-General] [Patch 3/3 v1] atm: br2684: Avoid alignment issues

[Philip P. <phi...@re...>]

Did any of these patches make it upstream?  I never saw an ack from David and David...

-Philip

On 8/17/11 12:37 AM, Pascal Hambourg wrote:
> Use memcmp() instead of cast to u16 when checking the PAD field.
> 
> Signed-off-by: Pascal Hambourg <pa...@pl...>
> ---
>  net/atm/br2684.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff -uprN a/net/atm/br2684.c b/net/atm/br2684.c
> --- a/net/atm/br2684.c	2011-08-16 22:13:47.000000000 +0200
> +++ b/net/atm/br2684.c	2011-08-16 22:13:49.000000000 +0200
> @@ -53,6 +53,7 @@ static const unsigned char ethertype_ipv
>  static const unsigned char ethertype_ipv6[] = { ETHERTYPE_IPV6 };
>  static const unsigned char llc_oui_pid_pad[] =
>  			{ LLC, SNAP_BRIDGED, PID_ETHERNET, PAD_BRIDGED };
> +static const unsigned char pad[] = { PAD_BRIDGED };
>  static const unsigned char llc_oui_ipv4[] = { LLC, SNAP_ROUTED, ETHERTYPE_IPV4 };
>  static const unsigned char llc_oui_ipv6[] = { LLC, SNAP_ROUTED, ETHERTYPE_IPV6 };
> 
> @@ -434,7 +435,7 @@ static void br2684_push(struct atm_vcc *
>  			 */
>  			if ((skb->len >= sizeof(llc_oui_pid_pad) + ETH_HLEN) &&
>  			    (memcmp(skb->data, llc_oui_pid_pad, 7) == 0) &&
> -			    (*((u16 *) (skb->data + 8)) == 0)) {
> +			    (memcmp(skb->data + 8, pad, BR2684_PAD_LEN) == 0)) {
>  				if (skb->data[7] == 0x01 &&
>  				    skb->len >= sizeof(llc_oui_pid_pad) + ETH_ZLEN + ETH_FCS_LEN)
>  					__skb_trim(skb, skb->len - ETH_FCS_LEN); /* trim FCS */
> @@ -464,7 +465,7 @@ static void br2684_push(struct atm_vcc *
>  		} else { /* p_bridged */
>  			/* first 2 chars should be 0 */
>  			if (skb->len < BR2684_PAD_LEN + ETH_HLEN ||
> -			    *((u16 *) (skb->data)) != 0)
> +			    memcmp(skb->data, pad, BR2684_PAD_LEN) != 0)
>  				goto error;
>  			skb_pull(skb, BR2684_PAD_LEN);
>  			skb->protocol = eth_type_trans(skb, net_dev);
> 

Re: [Linux-ATM-General] [RFC][PATCH 0/2] atm: br2682: Improve encapsulation checks and headroom allocation

[chas w. - C. <ch...@cm...>]

On Tue, 18 Oct 2011 23:20:58 -0700
Philip Prindeville <phi...@re...> wrote:

> On 8/15/11 8:39 AM, Pascal Hambourg wrote:
> > chas williams - CONTRACTOR a écrit :
> >> On Fri, 12 Aug 2011 22:01:10 +0200
>
> > 	memcmp(skb->data + 8, pad, BR2684_PAD_LEN) == 0
> > or
> > 	skb->data[8] == 0 && skb->data[9] == 0
> > or
> > 	(skb->data[8] | skb->data[9]) == 0
> > 
> > Which one is fastest ? I'm afraid this is platform-dependent.
> > Or is there a built-in function in the kernel which can do this
> > efficiently ?
> 
> Sorry about getting to the party late, but... just to double-check what I thought I knew...
> 
> Wouldn't the proper check be:
> 
>     __be16_to_cpup(&skb->data[8]) == 0
> 
> You might need a cast there to (__be16 *)... and yes, arguably, zero is the same in both LE and BE architectures, but it's worth being consistent if it stops you from overlooking it elsewhere (and losing portability).

if you really want to go this route, you should cast the 0 to 
the correct endian -- this can be optimized at compile time.  however,
i think it is just safer to use memcmp() -- however, i dont know any
platform that can't poorly aligned 16-bit words.

Re: [Linux-ATM-General] [RFC][PATCH 0/2] atm: br2682: Improve encapsulation checks and headroom allocation

[Philip P. <phi...@re...>]

On 8/15/11 8:39 AM, Pascal Hambourg wrote:
> chas williams - CONTRACTOR a écrit :
>> On Fri, 12 Aug 2011 22:01:10 +0200
>> Pascal Hambourg <pa...@pl...> wrote:
>>
>>> I updated and resent the patch 1/2 (v2) accordingly :
> [...]
>> this looks fine at a glance.  i do wonder if:
>>
>> *((u16 *) (skb->data + 8)) == 0
>>
>> works on all platforms.  cpu's other than x86 sometimes have issues
>> with arbitrary alignments of data.   it should probalby be a memcmp()
>> but that should probably be fixed in a seperate patch.
> 
> I copied this line from the VC-MUX part, even though I didn't like it.
> But from some readings I understood that skb data are supposed to be
> aligned on multiple of at least 2, and no one complained so far, so I
> thought it should be fine. Also I felt that memcmp() may not be the most
> efficient way to check that two consecutive bytes are 0. However I'll be
> glad to replace the line with something like
> 
> 	memcmp(skb->data + 8, pad, BR2684_PAD_LEN) == 0
> or
> 	skb->data[8] == 0 && skb->data[9] == 0
> or
> 	(skb->data[8] | skb->data[9]) == 0
> 
> Which one is fastest ? I'm afraid this is platform-dependent.
> Or is there a built-in function in the kernel which can do this
> efficiently ?

Sorry about getting to the party late, but... just to double-check what I thought I knew...

Wouldn't the proper check be:

    __be16_to_cpup(&skb->data[8]) == 0

You might need a cast there to (__be16 *)... and yes, arguably, zero is the same in both LE and BE architectures, but it's worth being consistent if it stops you from overlooking it elsewhere (and losing portability).

-Philip

Re: [Linux-ATM-General] [PATCH 1/4] atm: solos-pci: Add AnnexA/M capability attributes

[David W. <dw...@in...>]

On Wed, 2011-10-05 at 17:50 -0400, David Miller wrote:
> 
> David, Nathan's patches look fine to me, want me to stick them into
> net-next? 

Please. All of them can be

 Acked-By: David Woodhouse <Dav...@in...>

Thanks.

-- 
dwmw2

[Linux-ATM-General] [PATCH 4/4] atm: solos-pci: M25P/M25PE SPI flash support

[Nathan W. <na...@tr...>]

Newer Geos ADSL2+ routers have different SPI flash.
The FPGA on these boards require driver version = 1 to enable
flash upgrades so old drivers can't corrupt new boards.

Signed-off-by: Nathan Williams <na...@tr...>
---
 drivers/atm/solos-pci.c |   51 ++++++++++++++++++++++++++++++++++++----------
 1 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
index 33c0c2b..2a75bf7 100644
--- a/drivers/atm/solos-pci.c
+++ b/drivers/atm/solos-pci.c
@@ -42,7 +42,8 @@
 #include <linux/swab.h>
 #include <linux/slab.h>
 
-#define VERSION "0.07"
+#define VERSION "1.0"
+#define DRIVER_VERSION 0x01
 #define PTAG "solos-pci"
 
 #define CONFIG_RAM_SIZE	128
@@ -57,16 +58,20 @@
 #define FPGA_MODE	0x5C
 #define FLASH_MODE	0x58
 #define GPIO_STATUS	0x54
+#define DRIVER_VER	0x50
 #define TX_DMA_ADDR(port)	(0x40 + (4 * (port)))
 #define RX_DMA_ADDR(port)	(0x30 + (4 * (port)))
 
 #define DATA_RAM_SIZE	32768
 #define BUF_SIZE	2048
 #define OLD_BUF_SIZE	4096 /* For FPGA versions <= 2*/
-#define FPGA_PAGE	528 /* FPGA flash page size*/
-#define SOLOS_PAGE	512 /* Solos flash page size*/
-#define FPGA_BLOCK	(FPGA_PAGE * 8) /* FPGA flash block size*/
-#define SOLOS_BLOCK	(SOLOS_PAGE * 8) /* Solos flash block size*/
+/* Old boards use ATMEL AD45DB161D flash */
+#define ATMEL_FPGA_PAGE	528 /* FPGA flash page size*/
+#define ATMEL_SOLOS_PAGE	512 /* Solos flash page size*/
+#define ATMEL_FPGA_BLOCK	(ATMEL_FPGA_PAGE * 8) /* FPGA block size*/
+#define ATMEL_SOLOS_BLOCK	(ATMEL_SOLOS_PAGE * 8) /* Solos block size*/
+/* Current boards use M25P/M25PE SPI flash */
+#define SPI_FLASH_BLOCK	(256 * 64)
 
 #define RX_BUF(card, nr) ((card->buffers) + (nr)*(card->buffer_size)*2)
 #define TX_BUF(card, nr) ((card->buffers) + (nr)*(card->buffer_size)*2 + (card->buffer_size))
@@ -128,6 +133,7 @@ struct solos_card {
 	int using_dma;
 	int fpga_version;
 	int buffer_size;
+	int atmel_flash;
 };
 
 
@@ -631,16 +637,25 @@ static int flash_upgrade(struct solos_card *card, int chip)
 	switch (chip) {
 	case 0:
 		fw_name = "solos-FPGA.bin";
-		blocksize = FPGA_BLOCK;
+		if (card->atmel_flash)
+			blocksize = ATMEL_FPGA_BLOCK;
+		else
+			blocksize = SPI_FLASH_BLOCK;
 		break;
 	case 1:
 		fw_name = "solos-Firmware.bin";
-		blocksize = SOLOS_BLOCK;
+		if (card->atmel_flash)
+			blocksize = ATMEL_SOLOS_BLOCK;
+		else
+			blocksize = SPI_FLASH_BLOCK;
 		break;
 	case 2:
 		if (card->fpga_version > LEGACY_BUFFERS){
 			fw_name = "solos-db-FPGA.bin";
-			blocksize = FPGA_BLOCK;
+			if (card->atmel_flash)
+				blocksize = ATMEL_FPGA_BLOCK;
+			else
+				blocksize = SPI_FLASH_BLOCK;
 		} else {
 			dev_info(&card->dev->dev, "FPGA version doesn't support"
 					" daughter board upgrades\n");
@@ -650,7 +665,10 @@ static int flash_upgrade(struct solos_card *card, int chip)
 	case 3:
 		if (card->fpga_version > LEGACY_BUFFERS){
 			fw_name = "solos-Firmware.bin";
-			blocksize = SOLOS_BLOCK;
+			if (card->atmel_flash)
+				blocksize = ATMEL_SOLOS_BLOCK;
+			else
+				blocksize = SPI_FLASH_BLOCK;
 		} else {
 			dev_info(&card->dev->dev, "FPGA version doesn't support"
 					" daughter board upgrades\n");
@@ -695,9 +713,13 @@ static int flash_upgrade(struct solos_card *card, int chip)
 		/* dev_info(&card->dev->dev, "Set FPGA Flash mode to Block Write\n"); */
 		iowrite32(((chip * 2) + 1), card->config_regs + FLASH_MODE);
 
-		/* Copy block to buffer, swapping each 16 bits */
+		/* Copy block to buffer, swapping each 16 bits for Atmel flash */
 		for(i = 0; i < blocksize; i += 4) {
-			uint32_t word = swahb32p((uint32_t *)(fw->data + offset + i));
+			uint32_t word;
+			if (card->atmel_flash)
+				word = swahb32p((uint32_t *)(fw->data + offset + i));
+			else
+				word = *(uint32_t *)(fw->data + offset + i);
 			if(card->fpga_version > LEGACY_BUFFERS)
 				iowrite32(word, FLASH_BUF + i);
 			else
@@ -1249,6 +1271,11 @@ static int fpga_probe(struct pci_dev *dev, const struct pci_device_id *id)
 		db_fpga_upgrade = db_firmware_upgrade = 0;
 	}
 
+	/* Stopped using Atmel flash after 0.03-38 */
+	if (fpga_ver < 39)
+		card->atmel_flash = 1;
+	else
+		card->atmel_flash = 0;
 	if (card->fpga_version >= DMA_SUPPORTED){
 		card->using_dma = 1;
 	} else {
@@ -1256,6 +1283,8 @@ static int fpga_probe(struct pci_dev *dev, const struct pci_device_id *id)
 		/* Set RX empty flag for all ports */
 		iowrite32(0xF0, card->config_regs + FLAGS_ADDR);
 	}
+	/* New FPGAs require driver version before permitting flash upgrades */
+	iowrite32(DRIVER_VERSION, card->config_regs + DRIVER_VER);
 
 	data32 = ioread32(card->config_regs + PORTS);
 	card->nr_ports = (data32 & 0x000000FF);
-- 
1.7.2.5

[Linux-ATM-General] [PATCH 3/4] atm: solos-pci: Add support for Geos GPIO pins

[Nathan W. <na...@tr...>]

Geos ADSL2+ routers have on-board Solos chipsets with some
extra I/O pins and a push button connected to the FPGA.

PCB version and variant numbers are also made available
through the HardwareVersion and HardwareVariant attributes.

Signed-off-by: Nathan Williams <na...@tr...>
---
 drivers/atm/solos-pci.c |   98 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 98 insertions(+), 0 deletions(-)

diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
index bd01aa3..33c0c2b 100644
--- a/drivers/atm/solos-pci.c
+++ b/drivers/atm/solos-pci.c
@@ -56,6 +56,7 @@
 #define FLASH_BUSY	0x60
 #define FPGA_MODE	0x5C
 #define FLASH_MODE	0x58
+#define GPIO_STATUS	0x54
 #define TX_DMA_ADDR(port)	(0x40 + (4 * (port)))
 #define RX_DMA_ADDR(port)	(0x30 + (4 * (port)))
 
@@ -498,6 +499,87 @@ static ssize_t console_store(struct device *dev, struct device_attribute *attr,
 	return err?:count;
 }
 
+struct geos_gpio {
+	char *name;
+	int offset;
+};
+
+static struct geos_gpio geos_gpio_pins[] = {
+	{"GPIO1", 9},
+	{"GPIO2", 10},
+	{"GPIO3", 11},
+	{"GPIO4", 12},
+	{"GPIO5", 13},
+	{"PushButton", 14},
+	{NULL, 0}
+};
+
+static ssize_t geos_gpio_store(struct device *dev, struct device_attribute *attr,
+			       const char *buf, size_t count)
+{
+	struct atm_dev *atmdev = container_of(dev, struct atm_dev, class_dev);
+	struct solos_card *card = atmdev->dev_data;
+	uint32_t data32;
+
+	struct geos_gpio *p = geos_gpio_pins;
+	while(p->name){
+		if(!strcmp(attr->attr.name, p->name)){
+			break;
+		}
+		p++;
+	}
+
+	data32 = ioread32(card->config_regs + GPIO_STATUS);
+	if(buf[0] == '1'){
+		data32 |= 1 << p->offset;
+		iowrite32(data32, card->config_regs + GPIO_STATUS);
+	} else if(buf[0] == '0') {
+		data32 &= ~(1 << p->offset);
+		iowrite32(data32, card->config_regs + GPIO_STATUS);
+	}
+	return count;
+}
+
+static ssize_t geos_gpio_show(struct device *dev, struct device_attribute *attr,
+			      char *buf)
+{
+	struct atm_dev *atmdev = container_of(dev, struct atm_dev, class_dev);
+	struct solos_card *card = atmdev->dev_data;
+	uint32_t data32;
+
+	struct geos_gpio *p = geos_gpio_pins;
+	while(p->name){
+		if(!strcmp(attr->attr.name, p->name)){
+			break;
+		}
+		p++;
+	}
+
+	data32 = ioread32(card->config_regs + GPIO_STATUS);
+	data32 = (data32 >> p->offset) & 1;
+
+	return sprintf(buf, "%d\n", data32);
+}
+
+static ssize_t hardware_show(struct device *dev, struct device_attribute *attr,
+			     char *buf)
+{
+	struct atm_dev *atmdev = container_of(dev, struct atm_dev, class_dev);
+	struct solos_card *card = atmdev->dev_data;
+	uint32_t data32;
+
+	data32 = ioread32(card->config_regs + GPIO_STATUS);
+	if(!strcmp(attr->attr.name, "HardwareVersion")){
+		data32 = data32 & 0x1F;
+		return sprintf(buf, "%d\n", data32);
+	} else if(!strcmp(attr->attr.name, "HardwareVariant")){
+		data32 = (data32 >> 5) & 0x0F;
+		return sprintf(buf, "%d\n", data32);
+	}
+
+	return sprintf(buf, "Error\n");
+}
+
 static DEVICE_ATTR(console, 0644, console_show, console_store);
 
 
@@ -506,6 +588,14 @@ static DEVICE_ATTR(console, 0644, console_show, console_store);
 
 #include "solos-attrlist.c"
 
+static DEVICE_ATTR(GPIO1, 0644, geos_gpio_show, geos_gpio_store);
+static DEVICE_ATTR(GPIO2, 0644, geos_gpio_show, geos_gpio_store);
+static DEVICE_ATTR(GPIO3, 0644, geos_gpio_show, geos_gpio_store);
+static DEVICE_ATTR(GPIO4, 0644, geos_gpio_show, geos_gpio_store);
+static DEVICE_ATTR(GPIO5, 0644, geos_gpio_show, geos_gpio_store);
+static DEVICE_ATTR(PushButton, 0444, geos_gpio_show, NULL);
+static DEVICE_ATTR(HardwareVersion, 0444, hardware_show, NULL);
+static DEVICE_ATTR(HardwareVariant, 0444, hardware_show, NULL);
 #undef SOLOS_ATTR_RO
 #undef SOLOS_ATTR_RW
 
@@ -514,6 +604,14 @@ static DEVICE_ATTR(console, 0644, console_show, console_store);
 
 static struct attribute *solos_attrs[] = {
 #include "solos-attrlist.c"
+	&dev_attr_GPIO1.attr,
+	&dev_attr_GPIO2.attr,
+	&dev_attr_GPIO3.attr,
+	&dev_attr_GPIO4.attr,
+	&dev_attr_GPIO5.attr,
+	&dev_attr_PushButton.attr,
+	&dev_attr_HardwareVersion.attr,
+	&dev_attr_HardwareVariant.attr,
 	NULL
 };
 
-- 
1.7.2.5

[Linux-ATM-General] [PATCH 2/4] atm: solos-pci: Remove annoying line of debugging

[Nathan W. <na...@tr...>]

"len: %d" isn't particularly useful for anyone and confuses users.

Signed-off-by: Nathan Williams <na...@tr...>
---
 drivers/atm/solos-pci.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
index 5d1d076..bd01aa3 100644
--- a/drivers/atm/solos-pci.c
+++ b/drivers/atm/solos-pci.c
@@ -452,7 +452,6 @@ static ssize_t console_show(struct device *dev, struct device_attribute *attr,
 
 	len = skb->len;
 	memcpy(buf, skb->data, len);
-	dev_dbg(&card->dev->dev, "len: %d\n", len);
 
 	kfree_skb(skb);
 	return len;
-- 
1.7.2.5

[Linux-ATM-General] [PATCH 1/4] atm: solos-pci: Add AnnexA/M capability attributes

[Nathan W. <na...@tr...>]

BisACapability and BisMCapability allow users to
force either Annex A or Annex M.

Signed-off-by: Nathan Williams <na...@tr...>
---
 drivers/atm/solos-attrlist.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/atm/solos-attrlist.c b/drivers/atm/solos-attrlist.c
index 9a676ee..8092533 100644
--- a/drivers/atm/solos-attrlist.c
+++ b/drivers/atm/solos-attrlist.c
@@ -71,6 +71,8 @@ SOLOS_ATTR_RW(BisAForceSNRMarginDn)
 SOLOS_ATTR_RW(BisMForceSNRMarginDn)
 SOLOS_ATTR_RW(BisAMaxMargin)
 SOLOS_ATTR_RW(BisMMaxMargin)
+SOLOS_ATTR_RW(BisACapability)
+SOLOS_ATTR_RW(BisMCapability)
 SOLOS_ATTR_RW(AnnexAForceSNRMarginDn)
 SOLOS_ATTR_RW(AnnexAMaxMargin)
 SOLOS_ATTR_RW(AnnexMMaxMargin)
-- 
1.7.2.5

[Linux-ATM-General] [Patch 3/3 v1] atm: br2684: Avoid alignment issues

[Pascal H. <pa...@pl...>]

Use memcmp() instead of cast to u16 when checking the PAD field.

Signed-off-by: Pascal Hambourg <pa...@pl...>
---
 net/atm/br2684.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff -uprN a/net/atm/br2684.c b/net/atm/br2684.c
--- a/net/atm/br2684.c	2011-08-16 22:13:47.000000000 +0200
+++ b/net/atm/br2684.c	2011-08-16 22:13:49.000000000 +0200
@@ -53,6 +53,7 @@ static const unsigned char ethertype_ipv
 static const unsigned char ethertype_ipv6[] = { ETHERTYPE_IPV6 };
 static const unsigned char llc_oui_pid_pad[] =
 			{ LLC, SNAP_BRIDGED, PID_ETHERNET, PAD_BRIDGED };
+static const unsigned char pad[] = { PAD_BRIDGED };
 static const unsigned char llc_oui_ipv4[] = { LLC, SNAP_ROUTED, ETHERTYPE_IPV4 };
 static const unsigned char llc_oui_ipv6[] = { LLC, SNAP_ROUTED, ETHERTYPE_IPV6 };

@@ -434,7 +435,7 @@ static void br2684_push(struct atm_vcc *
 			 */
 			if ((skb->len >= sizeof(llc_oui_pid_pad) + ETH_HLEN) &&
 			    (memcmp(skb->data, llc_oui_pid_pad, 7) == 0) &&
-			    (*((u16 *) (skb->data + 8)) == 0)) {
+			    (memcmp(skb->data + 8, pad, BR2684_PAD_LEN) == 0)) {
 				if (skb->data[7] == 0x01 &&
 				    skb->len >= sizeof(llc_oui_pid_pad) + ETH_ZLEN + ETH_FCS_LEN)
 					__skb_trim(skb, skb->len - ETH_FCS_LEN); /* trim FCS */
@@ -464,7 +465,7 @@ static void br2684_push(struct atm_vcc *
 		} else { /* p_bridged */
 			/* first 2 chars should be 0 */
 			if (skb->len < BR2684_PAD_LEN + ETH_HLEN ||
-			    *((u16 *) (skb->data)) != 0)
+			    memcmp(skb->data, pad, BR2684_PAD_LEN) != 0)
 				goto error;
 			skb_pull(skb, BR2684_PAD_LEN);
 			skb->protocol = eth_type_trans(skb, net_dev);

[Linux-ATM-General] [PATCH 2/3 v2] atm: br2684: Make headroom and hard_header_len depend on the payload type

[Pascal H. <pa...@pl...>]

Routed payload requires less headroom than bridged payload.
So do not reallocate headroom if not needed.
Also, add worst case AAL5 overhead to netdev->hard_header_len.

Signed-off-by: Pascal Hambourg <pa...@pl...>
---
 net/atm/br2684.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff -uprN a/net/atm/br2684.c b/net/atm/br2684.c
--- a/net/atm/br2684.c	2011-08-12 21:29:22.000000000 +0200
+++ b/net/atm/br2684.c	2011-08-16 22:13:47.000000000 +0200
@@ -202,7 +202,10 @@ static int br2684_xmit_vcc(struct sk_buf
 {
 	struct br2684_dev *brdev = BRPRIV(dev);
 	struct atm_vcc *atmvcc;
-	int minheadroom = (brvcc->encaps == e_llc) ? 10 : 2;
+	int minheadroom = (brvcc->encaps == e_llc) ?
+		((brdev->payload == p_bridged) ?
+			sizeof(llc_oui_pid_pad) : sizeof(llc_oui_ipv4)) :
+		((brdev->payload == p_bridged) ? BR2684_PAD_LEN : 0);

 	if (skb_headroom(skb) < minheadroom) {
 		struct sk_buff *skb2 = skb_realloc_headroom(skb, minheadroom);
@@ -610,6 +613,7 @@ static void br2684_setup(struct net_devi
 	struct br2684_dev *brdev = BRPRIV(netdev);

 	ether_setup(netdev);
+	netdev->hard_header_len += sizeof(llc_oui_pid_pad); /* worst case */
 	brdev->net_dev = netdev;

 	netdev->netdev_ops = &br2684_netdev_ops;
@@ -622,7 +626,7 @@ static void br2684_setup_routed(struct n
 	struct br2684_dev *brdev = BRPRIV(netdev);

 	brdev->net_dev = netdev;
-	netdev->hard_header_len = 0;
+	netdev->hard_header_len = sizeof(llc_oui_ipv4); /* worst case */
 	netdev->netdev_ops = &br2684_netdev_ops_routed;
 	netdev->addr_len = 0;
 	netdev->mtu = 1500;

Re: [Linux-ATM-General] [RFC][PATCH 0/3] atm: br2682: Improve encapsulation checks and headroom allocation

[Pascal H. <pa...@pl...>]

chas williams - CONTRACTOR a écrit :
> 
> this looks fine at a glance.  i do wonder if:
> 
> *((u16 *) (skb->data + 8)) == 0
> 
> works on all platforms.  cpu's other than x86 sometimes have issues
> with arbitrary alignments of data.   it should probalby be a memcmp()
> but that should probably be fixed in a seperate patch.
[...]
> my only suggestion is that besides getting minheadroom correct, is
> that you should set hard_headerlen correctly as well when the interface
> is setup.  in br2684_setup_routed() you should also do something
> like netdev->hard_header_len = 2;
> 
> sometimes this should be 0 but you dont know the encapsulation until
> the vcc is registered so at this point you are best just picking the
> worst case.
> 
> also, for the bridged case we know that the hard_header_len is going to 
> atmost ETH_HLEN + 10 since we will also always prepend something.
> 
> this should keep the need to skb_realloc_headroom() to a minimum.

Patch 1 now updated to v2 and new patch 3 added. Patch 1 v2 unchanged.

[PATCH 1/3 v2] atm: br2684: Improve encapsulation checks

 - Check that the LLC header matches the expected payload type.
 - Check that the PAD field is 0x00-00 also in LLC bridged mode.
 - Check that the frame length in bridged mode is at least ETH_HLEN (14)
   without FCS or ETH_ZLEN + ETH_FCS_LEN (64) with FCS.
 - Trim the FCS bytes only if the PID is ethernet with FCS.
 - Check the data length before reading the IP header version field in
   VC-MUX routed mode.

[Patch 2/3 v2] atm: br2684: Make headroom and hard_header_len depend on
the payload type

 Routed payload requires less headroom than bridged payload.
 So do not reallocate headroom if not needed.
 Also, add worst case AAL5 overhead to netdev->hard_header_len.

[Patch 3/3 v1] atm: br2684: Avoid alignement issues

 Use memcmp() instead of cast to u16 when checking the PAD field.

Re: [Linux-ATM-General] [RFC][PATCH 0/2] atm: br2682: Improve encapsulation checks and headroom allocation

[chas w. - C. <ch...@cm...>]

On Mon, 15 Aug 2011 17:39:32 +0200
Pascal Hambourg <pa...@pl...> wrote:

> 	memcmp(skb->data + 8, pad, BR2684_PAD_LEN) == 0
> or
> 	skb->data[8] == 0 && skb->data[9] == 0
> or
> 	(skb->data[8] | skb->data[9]) == 0
> 
> Which one is fastest ? I'm afraid this is platform-dependent.

memcmp would/should be optimized where possible and would likely be
preferred.

> I haven't read about net_device parameters yet and don't know how
> hard_header_len is used, so I'm not comfortable with messing with it.
> How is it used ?

it is the length of the hardware header.  for real ethernet devices,
this is ETH_HLEN.  since we are encapsulating in some cases, it might
be wise to just hint to the network stack that we need to stick some
more bytes in front of the packets.

> Can't hard_header_len be changed after the interface is created ?

you could.  but it isnt that big a deal to just leave it a little 
bigger than it should be.  always better to be a little bigger than
stopping to reallocate to make some headroom.

Re: [Linux-ATM-General] [RFC][PATCH 0/2] atm: br2682: Improve encapsulation checks and headroom allocation

[Pascal H. <pa...@pl...>]

chas williams - CONTRACTOR a écrit :
> On Fri, 12 Aug 2011 22:01:10 +0200
> Pascal Hambourg <pa...@pl...> wrote:
> 
>> I updated and resent the patch 1/2 (v2) accordingly :
[...]
> this looks fine at a glance.  i do wonder if:
> 
> *((u16 *) (skb->data + 8)) == 0
> 
> works on all platforms.  cpu's other than x86 sometimes have issues
> with arbitrary alignments of data.   it should probalby be a memcmp()
> but that should probably be fixed in a seperate patch.

I copied this line from the VC-MUX part, even though I didn't like it.
But from some readings I understood that skb data are supposed to be
aligned on multiple of at least 2, and no one complained so far, so I
thought it should be fine. Also I felt that memcmp() may not be the most
efficient way to check that two consecutive bytes are 0. However I'll be
glad to replace the line with something like

	memcmp(skb->data + 8, pad, BR2684_PAD_LEN) == 0
or
	skb->data[8] == 0 && skb->data[9] == 0
or
	(skb->data[8] | skb->data[9]) == 0

Which one is fastest ? I'm afraid this is platform-dependent.
Or is there a built-in function in the kernel which can do this
efficiently ?

>> The optimizations you suggested are way beyond my skill and knowledge of the
>> kernel code. My goal here was just to provide fixes with minimal changes which
>> could also be easily backported to the -stable and -longterm trees.
> 
> my only suggestion is that besides getting minheadroom correct, is
> that you should set hard_headerlen correctly as well when the interface
> is setup.  in br2684_setup_routed() you should also do something
> like netdev->hard_header_len = 2;

I haven't read about net_device parameters yet and don't know how
hard_header_len is used, so I'm not comfortable with messing with it.
How is it used ?

> sometimes this should be 0 but you dont know the encapsulation until
> the vcc is registered so at this point you are best just picking the
> worst case.
> 
> also, for the bridged case we know that the hard_header_len is going to 
> atmost ETH_HLEN + 10 since we will also always prepend something.
> 
> this should keep the need to skb_realloc_headroom() to a minimum.

Can't hard_header_len be changed after the interface is created ?

Re: [Linux-ATM-General] [RFC][PATCH 0/2] atm: br2682: Improve encapsulation checks and headroom allocation

[chas w. - C. <ch...@cm...>]

On Fri, 12 Aug 2011 22:01:10 +0200
Pascal Hambourg <pa...@pl...> wrote:

> chas williams - CONTRACTOR a écrit :
> > On Mon, 08 Aug 2011 00:05:24 +0200
> > Pascal Hambourg <pa...@pl...> wrote:
> > 
> >> Also it appears that eth_type_trans(), which is called in the bridged receive
> >> path in br2684_push(), calls skb_pull(skb, ETH_HLEN). Shouldn't the packet
> >> length be checked before calling eth_type_trans() ?
> > 
> > probably.  i dont see any reason it shouldnt be checked.
> > 
> >> Similarly, shouldn't the packet length be checked in the VC-MUX routed path
> >> before reading the IP header version ?
> > 
> > yes, that seems reasonable.
> 
> I updated and resent the patch 1/2 (v2) accordingly :
> 
> - Check that the LLC header matches the expected payload type.
> - Check that the PAD field is 0x00-00 also in LLC bridged mode.
> - Check that the frame length in bridged mode is at least ETH_HLEN (14)
>   without FCS or ETH_ZLEN + ETH_FCS_LEN (64) with FCS.
> - Trim the trailing FCS field only if the PID is ethernet with FCS.
> - Check the data length is not null before reading the IP header version
>   field in VC-MUX routed mode.

this looks fine at a glance.  i do wonder if:

*((u16 *) (skb->data + 8)) == 0

works on all platforms.  cpu's other than x86 sometimes have issues
with arbitrary alignments of data.   it should probalby be a memcmp()
but that should probably be fixed in a seperate patch.

> The optimizations you suggested are way beyond my skill and knowledge of the
> kernel code. My goal here was just to provide fixes with minimal changes which
> could also be easily backported to the -stable and -longterm trees.

my only suggestion is that besides getting minheadroom correct, is
that you should set hard_headerlen correctly as well when the interface
is setup.  in br2684_setup_routed() you should also do something
like netdev->hard_header_len = 2;

sometimes this should be 0 but you dont know the encapsulation until
the vcc is registered so at this point you are best just picking the
worst case.

also, for the bridged case we know that the hard_header_len is going to 
atmost ETH_HLEN + 10 since we will also always prepend something.

this should keep the need to skb_realloc_headroom() to a minimum.

Re: [Linux-ATM-General] [RFC][PATCH 0/2] atm: br2682: Improve encapsulation checks and headroom allocation

[Pascal H. <pa...@pl...>]

chas williams - CONTRACTOR a écrit :
> On Mon, 08 Aug 2011 00:05:24 +0200
> Pascal Hambourg <pa...@pl...> wrote:
> 
>> Also it appears that eth_type_trans(), which is called in the bridged receive
>> path in br2684_push(), calls skb_pull(skb, ETH_HLEN). Shouldn't the packet
>> length be checked before calling eth_type_trans() ?
> 
> probably.  i dont see any reason it shouldnt be checked.
> 
>> Similarly, shouldn't the packet length be checked in the VC-MUX routed path
>> before reading the IP header version ?
> 
> yes, that seems reasonable.

I updated and resent the patch 1/2 (v2) accordingly :

- Check that the LLC header matches the expected payload type.
- Check that the PAD field is 0x00-00 also in LLC bridged mode.
- Check that the frame length in bridged mode is at least ETH_HLEN (14)
  without FCS or ETH_ZLEN + ETH_FCS_LEN (64) with FCS.
- Trim the trailing FCS field only if the PID is ethernet with FCS.
- Check the data length is not null before reading the IP header version
  field in VC-MUX routed mode.

As usual, I did minimal testing to check there is no obvious regression.

The optimizations you suggested are way beyond my skill and knowledge of the
kernel code. My goal here was just to provide fixes with minimal changes which
could also be easily backported to the -stable and -longterm trees.

Cheers,
-- 

[Linux-ATM-General] [PATCH 1/2 v2] atm: br2684: Improve encapsulation checks

[Pascal H. <pa...@pl...>]

- Check that the LLC header matches the expected payload type.
- Check that the PAD field is 0x00-00 also in LLC bridged mode.
- Check that the frame length in bridged mode is at least ETH_HLEN (14)
  without FCS or ETH_ZLEN + ETH_FCS_LEN (64) with FCS.
- Trim the trailing FCS field only if the PID is ethernet with FCS.
- Check the data length is not null before reading the IP header version
  field in VC-MUX routed mode.

Signed-off-by: Pascal Hambourg <pa...@pl...>
---
 net/atm/br2684.c |   73 ++++++++++++++++++++---------------
 1 file changed, 42 insertions(+), 31 deletions(-)

--- a/net/atm/br2684.c	2011-08-01 23:54:44.000000000 +0200
+++ b/net/atm/br2684.c	2011-08-12 21:29:22.000000000 +0200
@@ -404,41 +404,51 @@ static void br2684_push(struct atm_vcc *
 	atm_return(atmvcc, skb->truesize);
 	pr_debug("skb from brdev %p\n", brdev);
 	if (brvcc->encaps == e_llc) {
-
-		if (skb->len > 7 && skb->data[7] == 0x01)
-			__skb_trim(skb, skb->len - 4);
-
-		/* accept packets that have "ipv[46]" in the snap header */
-		if ((skb->len >= (sizeof(llc_oui_ipv4))) &&
-		    (memcmp(skb->data, llc_oui_ipv4,
-			    sizeof(llc_oui_ipv4) - BR2684_ETHERTYPE_LEN) == 0)) {
-			if (memcmp(skb->data + 6, ethertype_ipv6,
-				   sizeof(ethertype_ipv6)) == 0)
-				skb->protocol = htons(ETH_P_IPV6);
-			else if (memcmp(skb->data + 6, ethertype_ipv4,
-					sizeof(ethertype_ipv4)) == 0)
-				skb->protocol = htons(ETH_P_IP);
-			else
+		if (brdev->payload == p_routed) {
+			/* accept packets that have "ipv[46]" in the snap header */
+			if ((skb->len >= (sizeof(llc_oui_ipv4))) &&
+			    (memcmp(skb->data, llc_oui_ipv4,
+				    sizeof(llc_oui_ipv4) - BR2684_ETHERTYPE_LEN) == 0)) {
+				if (memcmp(skb->data + 6, ethertype_ipv6,
+					   sizeof(ethertype_ipv6)) == 0)
+					skb->protocol = htons(ETH_P_IPV6);
+				else if (memcmp(skb->data + 6, ethertype_ipv4,
+						sizeof(ethertype_ipv4)) == 0)
+					skb->protocol = htons(ETH_P_IP);
+				else
+					goto error;
+				skb_pull(skb, sizeof(llc_oui_ipv4));
+				skb_reset_network_header(skb);
+				skb->pkt_type = PACKET_HOST;
+			} else {
 				goto error;
-			skb_pull(skb, sizeof(llc_oui_ipv4));
-			skb_reset_network_header(skb);
-			skb->pkt_type = PACKET_HOST;
-		/*
-		 * Let us waste some time for checking the encapsulation.
-		 * Note, that only 7 char is checked so frames with a valid FCS
-		 * are also accepted (but FCS is not checked of course).
-		 */
-		} else if ((skb->len >= sizeof(llc_oui_pid_pad)) &&
-			   (memcmp(skb->data, llc_oui_pid_pad, 7) == 0)) {
-			skb_pull(skb, sizeof(llc_oui_pid_pad));
-			skb->protocol = eth_type_trans(skb, net_dev);
-		} else
-			goto error;
-
+			}
+		} else { /* p_bridged */
+			/*
+			 * Let us waste some time for checking the encapsulation.
+			 * Frames with a valid FCS
+			 * are also accepted (but FCS is not checked of course).
+			 */
+			if ((skb->len >= sizeof(llc_oui_pid_pad) + ETH_HLEN) &&
+			    (memcmp(skb->data, llc_oui_pid_pad, 7) == 0) &&
+			    (*((u16 *) (skb->data + 8)) == 0)) {
+				if (skb->data[7] == 0x01 &&
+				    skb->len >= sizeof(llc_oui_pid_pad) + ETH_ZLEN + ETH_FCS_LEN)
+					__skb_trim(skb, skb->len - ETH_FCS_LEN); /* trim FCS */
+				else if (skb->data[7] != 0x07)
+					goto error;
+				skb_pull(skb, sizeof(llc_oui_pid_pad));
+				skb->protocol = eth_type_trans(skb, net_dev);
+			} else {
+				goto error;
+			}
+		}
 	} else { /* e_vc */
 		if (brdev->payload == p_routed) {
 			struct iphdr *iph;

+			if (skb->len == 0)
+				goto error;
 			skb_reset_network_header(skb);
 			iph = ip_hdr(skb);
 			if (iph->version == 4)
@@ -450,7 +460,8 @@ static void br2684_push(struct atm_vcc *
 			skb->pkt_type = PACKET_HOST;
 		} else { /* p_bridged */
 			/* first 2 chars should be 0 */
-			if (*((u16 *) (skb->data)) != 0)
+			if (skb->len < BR2684_PAD_LEN + ETH_HLEN ||
+			    *((u16 *) (skb->data)) != 0)
 				goto error;
 			skb_pull(skb, BR2684_PAD_LEN);
 			skb->protocol = eth_type_trans(skb, net_dev);

Re: [Linux-ATM-General] [RFC][PATCH 0/2] atm: br2682: Improve encapsulation checks and headroom allocation

[chas w. - C. <ch...@cm...>]

On Mon, 08 Aug 2011 00:05:24 +0200
Pascal Hambourg <pa...@pl...> wrote:

> Also it appears that eth_type_trans(), which is called in the bridged receive
> path in br2684_push(), calls skb_pull(skb, ETH_HLEN). Shouldn't the packet
> length be checked before calling eth_type_trans() ?

probably.  i dont see any reason it shouldnt be checked.

> Similarly, shouldn't the packet length be checked in the VC-MUX routed path
> before reading the IP header version ?

yes, that seems reasonable.

Re: [Linux-ATM-General] [RFC][PATCH 0/2] atm: br2682: Improve encapsulation checks and headroom allocation

[Pascal H. <pa...@pl...>]

chas williams - CONTRACTOR a écrit :
> On Wed, 03 Aug 2011 22:22:33 +0200
> Pascal Hambourg <pa...@pl...> wrote:
> 
>> Should we check the padding bytes value in received bridged PDUs ?
> 
>> Or should we follow the saying "be conservative in what you send, and
>> liberal in what you accept" ?
> 
> it would say probably not.  the rfc says should be padded with octets
> of 0x0, but should means:
> 
> 3. SHOULD   This word, or the adjective "RECOMMENDED", mean that there
>    may exist valid reasons in particular circumstances to ignore a
>    particular item, but the full implications must be understood and
>    carefully weighed before choosing a different course.
> 
> we are talking about PAD_BRIDGED right?

Yes.

>> Currently, it is checked in VC-MUX mode, but not in LLC mode.
> 
> we should probably be consistent.

Sure. I'll add the check in LLC mode.

Also it appears that eth_type_trans(), which is called in the bridged receive
path in br2684_push(), calls skb_pull(skb, ETH_HLEN). Shouldn't the packet
length be checked before calling eth_type_trans() ?

Similarly, shouldn't the packet length be checked in the VC-MUX routed path
before reading the IP header version ?

Re: [Linux-ATM-General] [RFC][PATCH 0/2] atm: br2682: Improve encapsulation checks and headroom allocation

[Francois R. <ro...@fr...>]

chas williams - CONTRACTOR <ch...@cm...> :
> Pascal Hambourg <pa...@pl...> wrote:
[...]
> >  Routed encapsulation requires less headroom than bridged encapsulation.
> 
> i have meant to check into this for a while.  shouldnt hard_header_len
> be set correctly when the interface is configured in the first place to
> make the whole skb_headroom() < minheadroom unlikely?

It should be possible to go even further and turn the sending part of the
br2684 module into an header_ops exercise (as long as there a 1..*:0..1
mapping between backend device and vcc).

One could also notice that the encap is always know at br2684_regvcc time.
It could be worth replacing br2684_push with br2684_push_vcmux and
br2684_push_llc, thus removing one redundant runtime check.

-- 
Ueimor

Re: [Linux-ATM-General] [RFC][PATCH 0/2] atm: br2682: Improve encapsulation checks and headroom allocation

[chas w. - C. <ch...@cm...>]

On Wed, 03 Aug 2011 22:22:33 +0200
Pascal Hambourg <pa...@pl...> wrote:

> Discussion :
> Should we check the padding bytes value in received bridged PDUs ?


> Or should we follow the saying "be conservative in what you send, and
> liberal in what you accept" ?

it would say probably not.  the rfc says should be padded with octets
of 0x0, but should means:

3. SHOULD   This word, or the adjective "RECOMMENDED", mean that there
   may exist valid reasons in particular circumstances to ignore a
   particular item, but the full implications must be understood and
   carefully weighed before choosing a different course.

we are talking about PAD_BRIDGED right?

> Currently, it is checked in VC-MUX mode, but not in LLC mode.

we should probably be consistent.

> [PATCH 2/2] atm: br2684: Do not allocate more headroom than needed
> 
>  Routed encapsulation requires less headroom than bridged encapsulation.

i have meant to check into this for a while.  shouldnt hard_header_len
be set correctly when the interface is configured in the first place to
make the whole skb_headroom() < minheadroom unlikely?

[Linux-ATM-General] [PATCH 2/2] atm: br2684: Do not allocate more headroom than needed

[Pascal H. <pa...@pl...>]

Routed encapsulation requires less headroom than bridged encapsulation.

Signed-off-by: Pascal Hambourg <pa...@pl...>
---
 net/atm/br2684.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff -uprN a/net/atm/br2684.c b/net/atm/br2684.c
--- a/net/atm/br2684.c	2011-08-02 20:21:22.000000000 +0200
+++ b/net/atm/br2684.c	2011-08-02 20:21:23.000000000 +0200
@@ -202,7 +202,10 @@ static int br2684_xmit_vcc(struct sk_buf
 {
 	struct br2684_dev *brdev = BRPRIV(dev);
 	struct atm_vcc *atmvcc;
-	int minheadroom = (brvcc->encaps == e_llc) ? 10 : 2;
+	int minheadroom = (brvcc->encaps == e_llc) ?
+		((brdev->payload == p_bridged) ?
+			sizeof(llc_oui_pid_pad) : sizeof(llc_oui_ipv4)) :
+		((brdev->payload == p_bridged) ? BR2684_PAD_LEN : 0);

 	if (skb_headroom(skb) < minheadroom) {
 		struct sk_buff *skb2 = skb_realloc_headroom(skb, minheadroom);

[Linux-ATM-General] [PATCH 1/2] atm: br2684: Improve encapsulation checks

[Pascal H. <pa...@pl...>]

- Check that the LLC header matches the expected payload type.
- Remove the FCS bytes only if the PID is ethernet with FCS.
- Check the data length before removing the FCS or padding bytes.

Signed-off-by: Pascal Hambourg <pa...@pl...>
---
 net/atm/br2684.c |   70 ++++++++++++++++++++++-------------------
 1 file changed, 39 insertions(+), 31 deletions(-)

diff -uprN a/net/atm/br2684.c b/net/atm/br2684.c
--- a/net/atm/br2684.c	2011-08-01 23:54:44.000000000 +0200
+++ b/net/atm/br2684.c	2011-08-02 20:21:22.000000000 +0200
@@ -404,37 +404,44 @@ static void br2684_push(struct atm_vcc *
 	atm_return(atmvcc, skb->truesize);
 	pr_debug("skb from brdev %p\n", brdev);
 	if (brvcc->encaps == e_llc) {
-
-		if (skb->len > 7 && skb->data[7] == 0x01)
-			__skb_trim(skb, skb->len - 4);
-
-		/* accept packets that have "ipv[46]" in the snap header */
-		if ((skb->len >= (sizeof(llc_oui_ipv4))) &&
-		    (memcmp(skb->data, llc_oui_ipv4,
-			    sizeof(llc_oui_ipv4) - BR2684_ETHERTYPE_LEN) == 0)) {
-			if (memcmp(skb->data + 6, ethertype_ipv6,
-				   sizeof(ethertype_ipv6)) == 0)
-				skb->protocol = htons(ETH_P_IPV6);
-			else if (memcmp(skb->data + 6, ethertype_ipv4,
-					sizeof(ethertype_ipv4)) == 0)
-				skb->protocol = htons(ETH_P_IP);
-			else
+		if (brdev->payload == p_routed) {
+			/* accept packets that have "ipv[46]" in the snap header */
+			if ((skb->len >= (sizeof(llc_oui_ipv4))) &&
+			    (memcmp(skb->data, llc_oui_ipv4,
+				    sizeof(llc_oui_ipv4) - BR2684_ETHERTYPE_LEN) == 0)) {
+				if (memcmp(skb->data + 6, ethertype_ipv6,
+					   sizeof(ethertype_ipv6)) == 0)
+					skb->protocol = htons(ETH_P_IPV6);
+				else if (memcmp(skb->data + 6, ethertype_ipv4,
+						sizeof(ethertype_ipv4)) == 0)
+					skb->protocol = htons(ETH_P_IP);
+				else
+					goto error;
+				skb_pull(skb, sizeof(llc_oui_ipv4));
+				skb_reset_network_header(skb);
+				skb->pkt_type = PACKET_HOST;
+			} else {
 				goto error;
-			skb_pull(skb, sizeof(llc_oui_ipv4));
-			skb_reset_network_header(skb);
-			skb->pkt_type = PACKET_HOST;
-		/*
-		 * Let us waste some time for checking the encapsulation.
-		 * Note, that only 7 char is checked so frames with a valid FCS
-		 * are also accepted (but FCS is not checked of course).
-		 */
-		} else if ((skb->len >= sizeof(llc_oui_pid_pad)) &&
-			   (memcmp(skb->data, llc_oui_pid_pad, 7) == 0)) {
-			skb_pull(skb, sizeof(llc_oui_pid_pad));
-			skb->protocol = eth_type_trans(skb, net_dev);
-		} else
-			goto error;
-
+			}
+		} else { /* p_bridged */
+			/*
+			 * Let us waste some time for checking the encapsulation.
+			 * Note, that only 7 char is checked so frames with a valid FCS
+			 * are also accepted (but FCS is not checked of course).
+			 */
+			if ((skb->len >= sizeof(llc_oui_pid_pad)) &&
+			    (memcmp(skb->data, llc_oui_pid_pad, 7) == 0)) {
+				if (skb->data[7] == 0x01 &&
+				    skb->len >= sizeof(llc_oui_pid_pad) + 4)
+					__skb_trim(skb, skb->len - 4); /* trim FCS */
+				else if (skb->data[7] != 0x07)
+					goto error;
+				skb_pull(skb, sizeof(llc_oui_pid_pad));
+				skb->protocol = eth_type_trans(skb, net_dev);
+			} else {
+				goto error;
+			}
+		}
 	} else { /* e_vc */
 		if (brdev->payload == p_routed) {
 			struct iphdr *iph;
@@ -450,7 +457,8 @@ static void br2684_push(struct atm_vcc *
 			skb->pkt_type = PACKET_HOST;
 		} else { /* p_bridged */
 			/* first 2 chars should be 0 */
-			if (*((u16 *) (skb->data)) != 0)
+			if (skb->len < BR2684_PAD_LEN ||
+			    *((u16 *) (skb->data)) != 0) /* needed ? */
 				goto error;
 			skb_pull(skb, BR2684_PAD_LEN);
 			skb->protocol = eth_type_trans(skb, net_dev);