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

Re: [Linux-ATM-General] [PATCH] atm: nicstar: use NULL instead of 0 for pointer

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

On Wed, 19 Feb 2014 14:12:46 +0900
Daeseok Youn <dae...@gm...> wrote:

> >From c320d2ea1ed51c88255c33a50c74fa3598ab7be6 Mon Sep 17 00:00:00 2001
> From: Daeseok Youn <dae...@gm...>
> Date: Wed, 19 Feb 2014 10:10:11 +0900
> Subject: [PATCH] atm: nicstar: use NULL instead of 0 for pointer
> 
> sparse says:
> 
> drivers/atm/nicstar.c:642:27: warning:
>  Using plain integer as NULL pointer
> drivers/atm/nicstar.c:644:27:
>  warning: Using plain integer as NULL pointer
> drivers/atm/nicstar.c:982:51:
>  warning: Using plain integer as NULL pointer
> drivers/atm/nicstar.c:996:51:
>  warning: Using plain integer as NULL pointer
> 
> Signed-off-by: Daeseok Youn <dae...@gm...>
> ---
>  drivers/atm/nicstar.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/atm/nicstar.c b/drivers/atm/nicstar.c
> index 9587e95..13ed54c 100644
> --- a/drivers/atm/nicstar.c
> +++ b/drivers/atm/nicstar.c
> @@ -639,9 +639,9 @@ static int ns_init_card(int i, struct pci_dev *pcidev)
>  	card->hbnr.init = NUM_HB;
>  	card->hbnr.max = MAX_HB;
>  
> -	card->sm_handle = 0x00000000;
> +	card->sm_handle = NULL;
>  	card->sm_addr = 0x00000000;
> -	card->lg_handle = 0x00000000;
> +	card->lg_handle = NULL;
>  	card->lg_addr = 0x00000000;
>  
>  	card->efbie = 1;	/* To prevent push_rxbufs from enabling the interrupt */
> @@ -979,7 +979,7 @@ static void push_rxbufs(ns_dev * card, struct sk_buff *skb)
>  				addr2 = card->sm_addr;
>  				handle2 = card->sm_handle;
>  				card->sm_addr = 0x00000000;
> -				card->sm_handle = 0x00000000;
> +				card->sm_handle = NULL;
>  			} else {	/* (!sm_addr) */
>  
>  				card->sm_addr = addr1;
> @@ -993,7 +993,7 @@ static void push_rxbufs(ns_dev * card, struct sk_buff *skb)
>  				addr2 = card->lg_addr;
>  				handle2 = card->lg_handle;
>  				card->lg_addr = 0x00000000;
> -				card->lg_handle = 0x00000000;
> +				card->lg_handle = NULL;
>  			} else {	/* (!lg_addr) */
>  
>  				card->lg_addr = addr1;

Acked-by: Chas Williams <ch...@cm...>

Re: [Linux-ATM-General] [PATCH] atm: ambassador: use NULL instead of 0 for pointer

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

On Wed, 19 Feb 2014 14:11:15 +0900
Daeseok Youn <dae...@gm...> wrote:

> >From 932e928d53b1e588dc17019e7f9fa7a61b8b7468 Mon Sep 17 00:00:00 2001
> From: Daeseok Youn <dae...@gm...>
> Date: Wed, 19 Feb 2014 10:35:41 +0900
> Subject: [PATCH] atm: ambassador: use NULL instead of 0 for pointer
> 
> sparse says:
> 
> drivers/atm/ambassador.c:1928:24: warning:
>  Using plain integer as NULL pointer
> 
> Signed-off-by: Daeseok Youn <dae...@gm...>
> ---
>  drivers/atm/ambassador.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/atm/ambassador.c b/drivers/atm/ambassador.c
> index 62a7607..f1a9198 100644
> --- a/drivers/atm/ambassador.c
> +++ b/drivers/atm/ambassador.c
> @@ -1925,7 +1925,7 @@ static int ucode_init(loader_block *lb, amb_dev *dev)
>    const struct firmware *fw;
>    unsigned long start_address;
>    const struct ihex_binrec *rec;
> -  const char *errmsg = 0;
> +  const char *errmsg = NULL;
>    int res;
>  
>    res = request_ihex_firmware(&fw, "atmsar11.fw", &dev->pci_dev->dev);

Acked-by: Chas Williams <ch...@cm...>

Re: [Linux-ATM-General] [PATCH] atm: solos-pci: make solos_bh() as static

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

On Wed, 19 Feb 2014 14:13:54 +0900
Daeseok Youn <dae...@gm...> wrote:

> >From 6297aabeff748777b520cc0ee835af0a3ddc79e2 Mon Sep 17 00:00:00 2001
> From: Daeseok Youn <dae...@gm...>
> Date: Wed, 19 Feb 2014 10:49:12 +0900
> Subject: [PATCH] atm: solos-pci: make solos_bh() as static
> 
> sparse says:
> 
> drivers/atm/solos-pci.c:763:6: warning:
>  symbol 'solos_bh' was not declared. Should it be static?
> 
> Signed-off-by: Daeseok Youn <dae...@gm...>
> ---
>  drivers/atm/solos-pci.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
> index e3fb496..943cf0d 100644
> --- a/drivers/atm/solos-pci.c
> +++ b/drivers/atm/solos-pci.c
> @@ -760,7 +760,7 @@ static irqreturn_t solos_irq(int irq, void *dev_id)
>  	return IRQ_RETVAL(handled);
>  }
>  
> -void solos_bh(unsigned long card_arg)
> +static void solos_bh(unsigned long card_arg)
>  {
>  	struct solos_card *card = (void *)card_arg;
>  	uint32_t card_flags;

Acked-by: Chas Williams <ch...@cm...>

[Linux-ATM-General] Looking for a talented GPGPU tester at AMD in Sunnyvale, CA

[swati k. <khe...@ya...>]

* Familiar with OpenGL and graphics drivers
* Familiar with Linux and flashing the BIOS
* Enthusiastic and eager to learn

Contact:
Swati Kher,
Heterogeneous Systems Architecture,
AMD in Sunnyvale, CA
khe...@ya...

Re: [Linux-ATM-General] [PATCH] atm: firestream: remove duplicate define

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

Acked-by: Chas Williams <ch...@cm...>

On Mon, 21 Oct 2013 10:12:41 +0200
Michael Opdenacker <mic...@fr...> wrote:

> This patch removes a duplicate define in drivers/atm/firestream.h
> 
> Signed-off-by: Michael Opdenacker <mic...@fr...>
> ---
>  drivers/atm/firestream.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/atm/firestream.h b/drivers/atm/firestream.h
> index 49e783e..364eded 100644
> --- a/drivers/atm/firestream.h
> +++ b/drivers/atm/firestream.h
> @@ -420,7 +420,6 @@ struct fs_transmit_config {
>  #define RC_FLAGS_BFPS_BFP27 (0xd << 17)
>  #define RC_FLAGS_BFPS_BFP47 (0xe << 17)
>  
> -#define RC_FLAGS_BFPS       (0x1 << 17)
>  #define RC_FLAGS_BFPP       (0x1 << 21)
>  #define RC_FLAGS_TEVC       (0x1 << 22)
>  #define RC_FLAGS_TEP        (0x1 << 23)

Re: [Linux-ATM-General] [PATCH RFC] solos-pci: Fix BUG() with shared skb

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

On Wed, 2013-09-04 at 14:30 -0400, David Miller wrote:
> skb_realloc_headroom() should do everything you need.

Great, thanks! Something like this then... ? 

Do I really need the truesize check? And if so, is there a better way to
handle the ATM accounting? It just *happens* to be the case that the the
br2684_pop() and pppoatm_pop() functions don't mind being bypassed in
this fashion, and we should probably get away with tweaking the core ATM
accounting directly like this. Doesn't make me happy though...

diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
index 32784d1..4492c0f 100644
--- a/drivers/atm/solos-pci.c
+++ b/drivers/atm/solos-pci.c
@@ -1145,19 +1145,19 @@ static int psend(struct atm_vcc *vcc, struct sk_buff *skb)
 		return 0;
 	}
 
-	if (!skb_clone_writable(skb, sizeof(*header))) {
-		int expand_by = 0;
-		int ret;
-
-		if (skb_headroom(skb) < sizeof(*header))
-			expand_by = sizeof(*header) - skb_headroom(skb);
-
-		ret = pskb_expand_head(skb, expand_by, 0, GFP_ATOMIC);
-		if (ret) {
-			dev_warn(&card->dev->dev, "pskb_expand_head failed.\n");
-			solos_pop(vcc, skb);
-			return ret;
-		}
+	if (skb_headroom(skb) < sizeof(*header)) {
+		struct sk_buff *nskb;
+
+		nskb = skb_realloc_headroom(skb, sizeof(*header));
+		if (!nskb) {
+			solos_pop(vcc, skb);
+			return -ENOMEM;
+		}
+		if (skb->truesize != nskb->truesize)
+			atm_force_charge(vcc, nskb->truesize - skb->truesize);
+ 
+		dev_kfree_skb_any(skb);
+		skb = nskb;
 	}
 
 	header = (void *)skb_push(skb, sizeof(*header));


-- 
dwmw2

[Linux-ATM-General] [PATCH RFC] solos-pci: Fix BUG() with shared skb

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

Simon reported this BUG():

 kernel BUG at net/core/skbuff.c:1065!
 Call Trace:
  [<f9b7c12c>] ? pppoatm_send+0x3f/0x1a0 [pppoatm]
  [<f8751797>] psend+0xa9/0x14a [solos_pci]
  [<f9b7c248>] pppoatm_send+0x15b/0x1a0 [pppoatm]
  [<f8a2f77d>] ppp_push+0x76/0x533 [ppp_generic]

(Rest of backtrace at http://s85.org/mn0aOxMN — the skb appears to be
IPv6, forwarded from another interface over PPPoATM.)

I wasn't expecting to see shared skbs in the ATM driver's ->send()
function. Is this the right fix?

I've included the whole function in the patch context... I appear to be
jumping through a lot of hand-coded hoops here, just to ensure that I
can safely prepend my own device's hardware header to the skb. This
makes me think that I'm doing something severely wrong. There ought to
be an easier way of doing this, surely? What am I missing?

diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
index 32784d1..b15e475 100644
--- a/drivers/atm/solos-pci.c
+++ b/drivers/atm/solos-pci.c
@@ -1135,40 +1135,50 @@ static uint32_t fpga_tx(struct solos_card *card)
 static int psend(struct atm_vcc *vcc, struct sk_buff *skb)
 {
 	struct solos_card *card = vcc->dev->dev_data;
 	struct pkt_hdr *header;
 	int pktlen;
 
 	pktlen = skb->len;
 	if (pktlen > (BUF_SIZE - sizeof(*header))) {
 		dev_warn(&card->dev->dev, "Length of PDU is too large. Dropping PDU.\n");
 		solos_pop(vcc, skb);
 		return 0;
 	}
 
+	/* This is skb_share_check() except it uses solos_pop() instead of kfree_skb() */
+	if (skb_shared(skb)) {
+		struct sk_buff *nskb = skb_clone(skb, GFP_ATOMIC);
+		if (unlikely(!nskb)) {
+			solos_pop(vcc, skb);
+			return 0;
+		}
+		consume_skb(skb);
+		skb = nskb;
+	}
 	if (!skb_clone_writable(skb, sizeof(*header))) {
 		int expand_by = 0;
 		int ret;
 
 		if (skb_headroom(skb) < sizeof(*header))
 			expand_by = sizeof(*header) - skb_headroom(skb);
 
 		ret = pskb_expand_head(skb, expand_by, 0, GFP_ATOMIC);
 		if (ret) {
 			dev_warn(&card->dev->dev, "pskb_expand_head failed.\n");
 			solos_pop(vcc, skb);
 			return ret;
 		}
 	}
 
 	header = (void *)skb_push(skb, sizeof(*header));
 
 	/* This does _not_ include the size of the header */
 	header->size = cpu_to_le16(pktlen);
 	header->vpi = cpu_to_le16(vcc->vpi);
 	header->vci = cpu_to_le16(vcc->vci);
 	header->type = cpu_to_le16(PKT_DATA);
 
 	fpga_queue(card, SOLOS_CHAN(vcc->dev), skb, vcc);
 
 	return 0;
 }



-- 
David Woodhouse                            Open Source Technology Centre
Dav...@in...                              Intel Corporation

Re: [Linux-ATM-General] [PATCH 05/21] atm: he: use mdelay instead of large udelay constants

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

On Fri, 26 Apr 2013 09:21:59 +0100
"David Laight" <Dav...@AC...> wrote:

> > ARM cannot handle udelay for more than 2 miliseconds, so we
> > should use mdelay instead for those.
> ...
> > @@ -1055,7 +1055,7 @@ static int he_start(struct atm_dev *dev)
> >  	he_writel(he_dev, 0x0, RESET_CNTL);
> >  	he_writel(he_dev, 0xff, RESET_CNTL);
> > 
> > -	udelay(16*1000);	/* 16 ms */
> > +	mdelay(16);	/* 16 ms */
> >  	status = he_readl(he_dev, RESET_CNTL);
> 
> 16ms seems a long time to spin.
> I'd have thought a sleep would be more appropriate.
> Since this looks like timing a hardware reset pulse
> it can't matter if it is somewhat longer.

Yes, I wrote this bit some time ago when I was less wise. The
programmer's guide doesn't say how long to sleep, so the value isn't
critical.  It just has to be "long enough".  An msleep() would be fine
here.

Re: [Linux-ATM-General] [PATCH 10/14] atm: Removed redundant check on unsigned variable

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

Acked-by: chas williams - CONTRACTOR <ch...@cm...>

On Fri, 28 Dec 2012 10:46:36 +0530
Tushar Behera <tus...@li...> wrote:

> Ping.
> 
> On 11/16/2012 12:20 PM, Tushar Behera wrote:
> > No need to check whether unsigned variable is less than 0.
> > 
> > CC: Chas Williams <ch...@cm...>
> > CC: lin...@li...
> > CC: ne...@vg...
> > Signed-off-by: Tushar Behera <tus...@li...>
> > ---
> >  drivers/atm/fore200e.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/atm/fore200e.c b/drivers/atm/fore200e.c
> > index 361f5ae..fdd3fe7 100644
> > --- a/drivers/atm/fore200e.c
> > +++ b/drivers/atm/fore200e.c
> > @@ -972,7 +972,7 @@ int bsq_audit(int where, struct host_bsq* bsq, int scheme, int magn)
> >  		   where, scheme, magn, buffer->index, buffer->scheme);
> >  	}
> >  
> > -	if ((buffer->index < 0) || (buffer->index >= fore200e_rx_buf_nbr[ scheme ][ magn ])) {
> > +	if (buffer->index >= fore200e_rx_buf_nbr[ scheme ][ magn ]) {
> >  	    printk(FORE200E "bsq_audit(%d): queue %d.%d, out of range buffer index = %ld !\n",
> >  		   where, scheme, magn, buffer->index);
> >  	}
> > 
> 
> 

Re: [Linux-ATM-General] [patch] atm: forever loop loading ambassador firmware

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

On Tue, 27 Nov 2012 20:29:35 +0300
Dan Carpenter <dan...@or...> wrote:

> There was a forever loop introduced here when we converted this to
> request_firmware() back in 2008.
> 
> Signed-off-by: Dan Carpenter <dan...@or...>

This certainly does seem to be the case.

Acked-by: Chas Williams <ch...@cm...>

> ---
> Spotted in code reading.  Untested.
> 
> diff --git a/drivers/atm/ambassador.c b/drivers/atm/ambassador.c
> index 89b30f3..ff7bb8a 100644
> --- a/drivers/atm/ambassador.c
> +++ b/drivers/atm/ambassador.c
> @@ -1961,6 +1961,7 @@ static int __devinit ucode_init (loader_block * lb, amb_dev * dev) {
>      res = loader_verify(lb, dev, rec);
>      if (res)
>        break;
> +    rec = ihex_next_binrec(rec);
>    }
>    release_firmware(fw);
>    if (!res)
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to maj...@vg...
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [Linux-ATM-General] tuntap variant for atm ?

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

I don't believe that linux-atm has a tuntap today.  The closest might
be atmtcp.  it shouldn't be too difficult to change the userspace half
of atmtcp to send/recv the atm frames to your local program instead of
another atmtcp instance on other machine.

On Tue, 09 Oct 2012 07:51:14 +0100
"Iain Young, G7III" <g7...@g7...> wrote:

> Hi Folks,
> 
> I am "playing" with Software Defined Radios, and setting up Networks
> using them.
> 
> Up until now, for my basic Layer 1 / Layer 2 tests, I have been using
> the tuntap devices, and just a TCP/UDP socket.
> 
> However, Ethernet is not the most ideal protocol for narrow bandwidth
> RF links, and so I thought about ATM (with it's small cell size, there
> may be a positive impact on throughput with TDMA type systems)
> 
> [Note this is purely for experiments and fun, and "because we can at
> present]
> 
> Is there an atm equivelent of the tuntap driver ? I'm looking at some 
> way to get the ATM frames in and out of my SDR...
> 
> I did try googling, but just get way to much noise about tuntap, and
> even "At The Moment"...
> 
> 
> Best Regards
> 
> Iain

[Linux-ATM-General] tuntap variant for atm ?

[Iain Y. G. <g7...@g7...>]

Hi Folks,

I am "playing" with Software Defined Radios, and setting up Networks
using them.

Up until now, for my basic Layer 1 / Layer 2 tests, I have been using
the tuntap devices, and just a TCP/UDP socket.

However, Ethernet is not the most ideal protocol for narrow bandwidth
RF links, and so I thought about ATM (with it's small cell size, there
may be a positive impact on throughput with TDMA type systems)

[Note this is purely for experiments and fun, and "because we can at
present]

Is there an atm equivelent of the tuntap driver ? I'm looking at some 
way to get the ATM frames in and out of my SDR...

I did try googling, but just get way to much noise about tuntap, and
even "At The Moment"...


Best Regards

Iain

[Linux-ATM-General] (no subject)

[Michael D L. <mdl...@ya...>]

.Earn more sitting at home from today  http://www.ramiro.barcelo.nom.br/click.link.php?ovpage=74o9

Re: [Linux-ATM-General] [PATCH 5/7] drivers/atm/eni.c: ensure arguments to request_irq and free_irq are compatible

[Chas W. (CONTRACTOR) <ch...@cm...>]

i am pretty sure that request_irq() shouldnt be changed but rather
free_irq() changed.  the last argument of request_irq() is passed to the
interrupt routine (ent_int in this case).  if you change that you need
to update the interrupt routine.  fixing free_irq would be easier.

In message <133...@li...>,Julia Lawall writes:
>Convert call to request_irq so that the last argument is the same as the
>second argument of the subsequent call to free_irq.  Without this
>property, free_irq does nothing.
>
>I have chosen to keep the call to free_irq as is, because its second
>argument is more like its first argument.

Re: [Linux-ATM-General] Subject: [PATCH 2/2] atm: br2684: Avoid alignment issues

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

Errr, my bad. It's the patches from Nathan from October that seem to be stalled.


On 1/2/12 6:59 PM, Philip Prindeville wrote:
> Was this acked by netdev?
> 
> I've not seen anything and it's not appeared in linux-next yet...
> 
> 
> On 11/28/11 11:40 AM, chas williams - CONTRACTOR wrote:
>>
>> From: Pascal Hambourg <pa...@pl...>
>> Date: Wed, 17 Aug 2011 08:37:52 +0200
>> Subject: [PATCH 2/2] atm: br2684: Avoid alignment issues
>>
>> Use memcmp() instead of cast to u16 when checking the PAD field.
>>
>> Signed-off-by: Pascal Hambourg <pa...@pl...>
>> Signed-off-by: chas williams - CONTRACTOR <ch...@cm...>
>> ---
>>  net/atm/br2684.c |    3 ++-
>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/net/atm/br2684.c b/net/atm/br2684.c
>> index ed72263..353fccf 100644
>> --- a/net/atm/br2684.c
>> +++ b/net/atm/br2684.c
>> @@ -53,6 +53,7 @@ static const unsigned char ethertype_ipv4[] = { ETHERTYPE_IPV4 };
>>  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 };
>>  
>> @@ -453,7 +454,7 @@ static void br2684_push(struct atm_vcc *atmvcc, struct sk_buff *skb)
>>  			skb->pkt_type = PACKET_HOST;
>>  		} else { /* p_bridged */
>>  			/* first 2 chars should be 0 */
>> -			if (*((u16 *) (skb->data)) != 0)
>> +			if (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] [Patch 3/3 v1] atm: br2684: Avoid alignment issues

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

Can these be given a prod?  I'm not seeing them in linux-next...


On 11/18/11 1:52 PM, chas williams - CONTRACTOR wrote:
> 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);
>>>
>>

Re: [Linux-ATM-General] Subject: [PATCH 2/2] atm: br2684: Avoid alignment issues

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

Was this acked by netdev?

I've not seen anything and it's not appeared in linux-next yet...


On 11/28/11 11:40 AM, chas williams - CONTRACTOR wrote:
> 
> From: Pascal Hambourg <pa...@pl...>
> Date: Wed, 17 Aug 2011 08:37:52 +0200
> Subject: [PATCH 2/2] atm: br2684: Avoid alignment issues
> 
> Use memcmp() instead of cast to u16 when checking the PAD field.
> 
> Signed-off-by: Pascal Hambourg <pa...@pl...>
> Signed-off-by: chas williams - CONTRACTOR <ch...@cm...>
> ---
>  net/atm/br2684.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/net/atm/br2684.c b/net/atm/br2684.c
> index ed72263..353fccf 100644
> --- a/net/atm/br2684.c
> +++ b/net/atm/br2684.c
> @@ -53,6 +53,7 @@ static const unsigned char ethertype_ipv4[] = { ETHERTYPE_IPV4 };
>  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 };
>  
> @@ -453,7 +454,7 @@ static void br2684_push(struct atm_vcc *atmvcc, struct sk_buff *skb)
>  			skb->pkt_type = PACKET_HOST;
>  		} else { /* p_bridged */
>  			/* first 2 chars should be 0 */
> -			if (*((u16 *) (skb->data)) != 0)
> +			if (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] [PATCH 1/1]: linux-atm: headroom/padding fixes

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

Sigh... that was supposed to go to openwrt-devel...


On 12/10/11 7:40 PM, Philip Prindeville wrote:
> Submitted upstream to netdev by Chas Williams on linux-atm.

[Linux-ATM-General] [PATCH 1/1]: linux-atm: headroom/padding fixes

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

Submitted upstream to netdev by Chas Williams on linux-atm.

[Linux-ATM-General] Subject: [PATCH 2/2] atm: br2684: Avoid alignment issues

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

From: Pascal Hambourg <pa...@pl...>
Date: Wed, 17 Aug 2011 08:37:52 +0200
Subject: [PATCH 2/2] atm: br2684: Avoid alignment issues

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

Signed-off-by: Pascal Hambourg <pa...@pl...>
Signed-off-by: chas williams - CONTRACTOR <ch...@cm...>
---
 net/atm/br2684.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/atm/br2684.c b/net/atm/br2684.c
index ed72263..353fccf 100644
--- a/net/atm/br2684.c
+++ b/net/atm/br2684.c
@@ -53,6 +53,7 @@ static const unsigned char ethertype_ipv4[] = { ETHERTYPE_IPV4 };
 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 };
 
@@ -453,7 +454,7 @@ static void br2684_push(struct atm_vcc *atmvcc, struct sk_buff *skb)
 			skb->pkt_type = PACKET_HOST;
 		} else { /* p_bridged */
 			/* first 2 chars should be 0 */
-			if (*((u16 *) (skb->data)) != 0)
+			if (memcmp(skb->data, pad, BR2684_PAD_LEN) != 0)
 				goto error;
 			skb_pull(skb, BR2684_PAD_LEN);
 			skb->protocol = eth_type_trans(skb, net_dev);
-- 
1.7.6

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

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

From: Pascal Hambourg <pa...@pl...>
Date: Wed, 17 Aug 2011 08:37:18 +0200
Subject: [PATCH 1/2] 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.

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

diff --git a/net/atm/br2684.c b/net/atm/br2684.c
index 53b0aa1..ed72263 100644
--- a/net/atm/br2684.c
+++ b/net/atm/br2684.c
@@ -202,7 +202,10 @@ static int br2684_xmit_vcc(struct sk_buff *skb, struct net_device *dev,
 {
 	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);
@@ -583,6 +586,7 @@ static void br2684_setup(struct net_device *netdev)
 	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;
@@ -595,7 +599,7 @@ static void br2684_setup_routed(struct net_device *netdev)
 	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;
-- 
1.7.6

[Linux-ATM-General] [PATCH 3/5] atm: clip: move clip_devs check to clip_push

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

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

    This will allow further cleanup.

Signed-off-by: Jorge Boncompte [DTI2] <jo...@dt...>
---
 net/atm/clip.c |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/net/atm/clip.c b/net/atm/clip.c
index 3cb9470..521b45b 100644
--- a/net/atm/clip.c
+++ b/net/atm/clip.c
@@ -189,6 +189,13 @@ static void clip_push(struct atm_vcc *vcc, struct sk_buff *skb)
 	struct clip_vcc *clip_vcc = CLIP_VCC(vcc);
 
 	pr_debug("\n");
+
+	if (!clip_devs) {
+		atm_return(vcc, skb->truesize);
+		kfree_skb(skb);
+		return;
+	}
+
 	if (!skb) {
 		pr_debug("removing VCC %p\n", clip_vcc);
 		if (clip_vcc->entry)
@@ -480,13 +487,9 @@ static int clip_mkip(struct atm_vcc *vcc, int timeout)
 	spin_unlock_irqrestore(&rq->lock, flags);
 
 	/* re-process everything received between connection setup and MKIP */
-	skb_queue_walk_safe(&queue, skb, tmp) {
-		if (!clip_devs) {
-			atm_return(vcc, skb->truesize);
-			kfree_skb(skb);
-		} else
-			clip_push(vcc, skb);
-	}
+	skb_queue_walk_safe(&queue, skb, tmp)
+		clip_push(vcc, skb);
+
 	return 0;
 }
 
-- 
1.7.7.1

[Linux-ATM-General] [PATCH 1/5] atm: br2684: Do not move counters backwards

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

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

    This snippet has caused several bugs in the past, and I don't see the
point on substracting the skb len from netdev stats.

Signed-off-by: Jorge Boncompte [DTI2] <jo...@dt...>
---
 net/atm/br2684.c |    9 +--------
 1 files changed, 1 insertions(+), 8 deletions(-)

diff --git a/net/atm/br2684.c b/net/atm/br2684.c
index d07223c..81cf33b 100644
--- a/net/atm/br2684.c
+++ b/net/atm/br2684.c
@@ -557,15 +557,8 @@ static int br2684_regvcc(struct atm_vcc *atmvcc, void __user * arg)
 	skb_queue_splice_init(rq, &queue);
 	spin_unlock_irqrestore(&rq->lock, flags);
 
-	skb_queue_walk_safe(&queue, skb, tmp) {
-		struct net_device *dev;
-
+	skb_queue_walk_safe(&queue, skb, tmp)
 		br2684_push(atmvcc, skb);
-		dev = skb->dev;
-
-		dev->stats.rx_bytes -= skb->len;
-		dev->stats.rx_packets--;
-	}
 
 	/* initialize netdev carrier state */
 	if (atmvcc->dev->signal == ATM_PHY_SIG_LOST)
-- 
1.7.7.1

[Linux-ATM-General] [PATCH 2/5] atm: clip: Don't move counters backwards

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

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

    I don't see the point on substracting the skb len from the netdev stats.

Signed-off-by: Jorge Boncompte [DTI2] <jo...@dt...>
---
 net/atm/clip.c |   10 +---------
 1 files changed, 1 insertions(+), 9 deletions(-)

diff --git a/net/atm/clip.c b/net/atm/clip.c
index 8523940..3cb9470 100644
--- a/net/atm/clip.c
+++ b/net/atm/clip.c
@@ -484,16 +484,8 @@ static int clip_mkip(struct atm_vcc *vcc, int timeout)
 		if (!clip_devs) {
 			atm_return(vcc, skb->truesize);
 			kfree_skb(skb);
-		} else {
-			struct net_device *dev = skb->dev;
-			unsigned int len = skb->len;
-
-			skb_get(skb);
+		} else
 			clip_push(vcc, skb);
-			dev->stats.rx_packets--;
-			dev->stats.rx_bytes -= len;
-			kfree_skb(skb);
-		}
 	}
 	return 0;
 }
-- 
1.7.7.1

[Linux-ATM-General] [PATCH 5/5] atm: Allow MSG_PEEK for atm sockets

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

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

    Now that the vcc backends do the right thing with respect the receive
queue on registration, allow MSK_PEEK for atm sockets.

    This allows a userspace program to inspect the packets and decide what
backend to use to handle them.

Signed-off-by: Jorge Boncompte [DTI2] <jo...@dt...>
---
 net/atm/common.c |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/net/atm/common.c b/net/atm/common.c
index 0b4c58f..b4b44db 100644
--- a/net/atm/common.c
+++ b/net/atm/common.c
@@ -522,8 +522,11 @@ int vcc_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr *msg,
 
 	if (sock->state != SS_CONNECTED)
 		return -ENOTCONN;
-	if (flags & ~MSG_DONTWAIT)		/* only handle MSG_DONTWAIT */
+
+	/* only handle MSG_DONTWAIT and MSG_PEEK */
+	if (flags & ~(MSG_DONTWAIT | MSG_PEEK))
 		return -EOPNOTSUPP;
+
 	vcc = ATM_SD(sock);
 	if (test_bit(ATM_VF_RELEASED, &vcc->flags) ||
 	    test_bit(ATM_VF_CLOSE, &vcc->flags) ||
@@ -544,8 +547,13 @@ int vcc_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr *msg,
 	if (error)
 		return error;
 	sock_recv_ts_and_drops(msg, sk, skb);
-	pr_debug("%d -= %d\n", atomic_read(&sk->sk_rmem_alloc), skb->truesize);
-	atm_return(vcc, skb->truesize);
+
+	if (!(flags & MSG_PEEK)) {
+		pr_debug("%d -= %d\n", atomic_read(&sk->sk_rmem_alloc),
+			 skb->truesize);
+		atm_return(vcc, skb->truesize);
+	}
+
 	skb_free_datagram(sk, skb);
 	return copied;
 }
-- 
1.7.7.1