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

Re: [Linux-ATM-General] [PATCH] moving from pci to dma

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

There doesn't seem to be a patch for review?

On Mon, 12 Jan 2015 11:02:39 +0100
Quentin Lambert <lam...@gm...> wrote:

> This patch replaces the references to the deprecated pci api with the
> corresponding dma api.
...
> Quentin Lambert (1):
>   atm: remove deprecated use of pci api
> 
>  drivers/atm/eni.c       | 8 +++++---
>  drivers/atm/he.c        | 2 +-
>  drivers/atm/lanai.c     | 9 +++++----
>  drivers/atm/nicstar.c   | 4 ++--
>  drivers/atm/solos-pci.c | 2 +-
>  drivers/atm/zatm.c      | 8 +++++---
>  6 files changed, 19 insertions(+), 14 deletions(-)
> 

[Linux-ATM-General] Lanai driver, Speedstream 3060 card

[<pr...@an...>]

Dear ATMers,

I own a Speedstream 3060 ATM card by Efficient Networks and I sadly
discovered that it does not work on Linux systems. Referring to the lanai
driver's comments, it seems that not a lot of work is required in order to
have it running under Linux.

I already tried to contact Mr Blank (lanai's author) but his email address seems invalid,
so I ask my question here hoping that my request will somehow arrive to him.

Mitchell: would it be possible to have the documentation for that card, if you still
have it? If not, where/who can I refer to to ask for documentation?

The interest on that card died more than ten years ago, so I thought that no one wants
(and has interest) to keep its specifications hidden in 2014 too.

Thank you everyone for reading this message, and I hope to hear from you soon.

Regards,
P.

Re: [Linux-ATM-General] [PATCH] drivers:atm Remove two FIXMES in the function, top_off_fp for the file, firestream.c

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

I don't see any reason to promote qe_tmp to a u64.  I think you can
just remove the comment.  Anyone trying to build this into a 64-bit
kernel will see errors from the virt_to_bus()/bus_to_virt() usage.

fp->n seems to only be manipulated in interrupt context (after driving
initialization) so it doesn't need locking or to be atomic.

On Sat,  6 Dec 2014 22:35:48 -0500
Nicholas Krause <xer...@gm...> wrote:

> Removes two FIXMES in the function.top_off_fp. The first being that of needing the variable, qe_tmp needing to be a
> u64 type and not u32 as encoding will not work if using a 32 bit register rather then 64 bit as stated in the first
> fix me comment. In addition the second being a no longer needed comment due to not needing to atomically increment
> the variable, n as passed to the function by the pointer of fp, as part of a structure of type,freepool.
> 
> Signed-off-by: Nicholas Krause <xer...@gm...>
> ---
>  drivers/atm/firestream.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/atm/firestream.c b/drivers/atm/firestream.c
> index 82f2ae0..06c23f6 100644
> --- a/drivers/atm/firestream.c
> +++ b/drivers/atm/firestream.c
> @@ -1477,7 +1477,7 @@ static void top_off_fp (struct fs_dev *dev, struct freepool *fp,
>  	struct FS_BPENTRY *qe, *ne;
>  	struct sk_buff *skb;
>  	int n = 0;
> -	u32 qe_tmp;
> +	u64  qe_tmp;
>  
>  	fs_dprintk (FS_DEBUG_QUEUE, "Topping off queue at %x (%d-%d/%d)\n", 
>  		    fp->offset, read_fs (dev, FP_CNT (fp->offset)), fp->n, 
> @@ -1505,14 +1505,8 @@ static void top_off_fp (struct fs_dev *dev, struct freepool *fp,
>  		ne->skb = skb;
>  		ne->fp = fp;
>  
> -		/*
> -		 * FIXME: following code encodes and decodes
> -		 * machine pointers (could be 64-bit) into a
> -		 * 32-bit register.
> -		 */
> -
>  		qe_tmp = read_fs (dev, FP_EA(fp->offset));
> -		fs_dprintk (FS_DEBUG_QUEUE, "link at %x\n", qe_tmp);
> +		fs_dprintk(FS_DEBUG_QUEUE, "link at %llx\n", qe_tmp);
>  		if (qe_tmp) {
>  			qe = bus_to_virt ((long) qe_tmp);
>  			qe->next = virt_to_bus(ne);
> @@ -1521,7 +1515,7 @@ static void top_off_fp (struct fs_dev *dev, struct freepool *fp,
>  			write_fs (dev, FP_SA(fp->offset), virt_to_bus(ne));
>  
>  		write_fs (dev, FP_EA(fp->offset), virt_to_bus (ne));
> -		fp->n++;   /* XXX Atomic_inc? */
> +		fp->n++;
>  		write_fs (dev, FP_CTU(fp->offset), 1);
>  	}
>  

Re: [Linux-ATM-General] FIXME in solos-pci.c

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

The last I heard on this topic was from Guy Ellis,

	From: Guy Ellis <gu...@tr...>
	To: lin...@li...
	Subject: Re: [Linux-ATM-General] solos-pci.c: Fix me
	Date: Tue, 22 Jul 2014 07:34:30 +1000

	Hi Chas/Nick,

	I think the FIXME is reminder to deal correctly with an unknown command.
	At the moment an unknown command is treated as a PKT_COMMAND which is 
	incorrect.
	I think the correct behaviour would be to ignore unknown commands and 
	just free the skb.

	That said I doubt this condition ever occurs, which is probably why it 
	has been this way since day 1.

	Nathan is on vacation at the moment, when he gets back in August I'll 
	ask him to tidy this up.

So perhaps, Guy could let us know if he had a chance for Nathan to
look at removing this FIXME.


On Wed, 03 Dec 2014 22:58:46 -0500
nick <xer...@gm...> wrote:

> Greetings Chas,
> I am wondering if there is any reason for the FIXME message in the below code. This is due to after reading the other parts of the function the code and logical seem similar and correct unless I am missing something about the hardware.
> Cheers Nick 
>  case PKT_COMMAND:
>                        default: /* FIXME: Not really, surely? */
>                               if (process_command(card, port, skb))
>                                         break;
>                                spin_lock(&card->cli_queue_lock);
>                                if (skb_queue_len(&card->cli_queue[port]) > 10) {
>                                      if (net_ratelimit())
>                                                 dev_warn(&card->dev->dev, "Dropping console response on port %d\n",
>                                                          port);
>                                         dev_kfree_skb_any(skb);
>                                 } else
>                                          skb_queue_tail(&card->cli_queue[port], skb);
>                                spin_unlock(&card->cli_queue_lock);
>                                break;
>                        }
>  
> 

[Linux-ATM-General] [PATCH v2] drivers: atm: eni: Add pci_dma_mapping_error() call

[Tina J. <tin...@gm...>]

Added a pci_dma_mapping_error() call to check for mapping errors before
further using the dma handle. In case of error, control goes to a new label
where the incoming skb is freed. Unchecked dma handles were found using
Coccinelle:

@rule1@
expression e1;
identifier x;
@@

*x = pci_map_single(...);
 ... when != pci_dma_mapping_error(e1,x)

Signed-off-by: Tina Johnson <tin...@gm...>
Acked-by: Julia Lawall <jul...@li...>
---
v2: *Removed jump to trouble label
    *Added a new label dma_map_error

 drivers/atm/eni.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/atm/eni.c b/drivers/atm/eni.c
index d65975a..c7fab3e 100644
--- a/drivers/atm/eni.c
+++ b/drivers/atm/eni.c
@@ -356,6 +356,8 @@ static int do_rx_dma(struct atm_vcc *vcc,struct sk_buff *skb,
 	if (skb) {
 		paddr = pci_map_single(eni_dev->pci_dev,skb->data,skb->len,
 		    PCI_DMA_FROMDEVICE);
+		if (pci_dma_mapping_error(eni_dev->pci_dev, paddr))
+			goto dma_map_error;
 		ENI_PRV_PADDR(skb) = paddr;
 		if (paddr & 3)
 			printk(KERN_CRIT DEV_LABEL "(itf %d): VCI %d has "
@@ -481,6 +483,7 @@ trouble:
 	if (paddr)
 		pci_unmap_single(eni_dev->pci_dev,paddr,skb->len,
 		    PCI_DMA_FROMDEVICE);
+dma_map_error:
 	if (skb) dev_kfree_skb_irq(skb);
 	return -1;
 }
-- 
1.7.10.4

Re: [Linux-ATM-General] [PATCH] drivers: atm: eni: Add pci_dma_mapping_error() call

[Tina J. <tin...@gm...>]

On Mon, 10 Nov 2014, David Miller wrote:

> The 'trouble' label assumes that it is recovering and unwinding state
> when an error occurs after the DMA buffer is successfully mapped.
>
> It unconditionally does pci_unmap_single() if 'paddr' is non-zero
> which it might be in the error case depending upon how DMA errors
> are represented on a given platform.

Sorry for the trouble and thankyou for your very helpful comment. I
will send the revised patch.

Re: [Linux-ATM-General] [PATCH net-next] lec: Use rtnl lock/unlock when updating MTU

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

In message <201...@re...>,David Miller writes:
>From: Chas Williams - CONTRACTOR <ch...@cm...>
>Date: Thu, 14 Aug 2014 09:19:47 -0400
>
>> The LECS response contains the MTU that should be used.  Correctly
>> synchronize with other layers when updating.
>> 
>> Signed-off-by: Chas Williams - CONTRACTOR <ch...@cm...>
>
>I don't think you can sleep from this function, which rtnl_lock() may
>require.  Look elsewhere in this routine, it's doing GFP_ATOMIC
>allocations even.

Its been a while but...

This is the send routine for a virtual atm device that is the control
interface bewteen the user space client and the kernel.  The user space
client creates an atm socket and uses an ioctl to connect that atm socket
to the this virtual device.  So the call path is something like:

sendmsg() -> vcc_sendmsg() -> virtual atm device.send()

Generally speaking, you can't sleep in the send routine of an atm device
since there are other potential users besides sockets.

The GFP_ATOMIC usage is probably a lack of understanding.  The other
IRQ level locks are necessary for coordination.

[Linux-ATM-General] [PATCH net-next] lec: Use rtnl lock/unlock when updating MTU

[Chas W. - C. <ch...@cm...>]

The LECS response contains the MTU that should be used.  Correctly
synchronize with other layers when updating.

Signed-off-by: Chas Williams - CONTRACTOR <ch...@cm...>
---
 net/atm/lec.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/atm/lec.c b/net/atm/lec.c
index 4c5b8ba..6e13369 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -410,9 +410,11 @@ static int lec_atm_send(struct atm_vcc *vcc, struct sk_buff *skb)
 		priv->lane2_ops = NULL;
 		if (priv->lane_version > 1)
 			priv->lane2_ops = &lane2_ops;
+		rtnl_lock();
 		if (dev_set_mtu(dev, mesg->content.config.mtu))
 			pr_info("%s: change_mtu to %d failed\n",
 				dev->name, mesg->content.config.mtu);
+		rtnl_unlock();
 		priv->is_proxy = mesg->content.config.is_proxy;
 		break;
 	case l_flush_tran_id:
-- 
1.9.3

[Linux-ATM-General] [PATCH] lec: Fix bug introduced by b67bfe0d42cac56c512dd5da4b1b347a23f4b70a

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

b67bfe0d42cac56c512dd5da4b1b347a23f4b70a (hlist: drop the node
parameter from iterators) dropped the node parameter from
iterators which lec_tbl_walk() was using to iterate the list.

Signed-off-by: Chas Williams <ch...@cm...>
---
 net/atm/lec.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/net/atm/lec.c b/net/atm/lec.c
index 4c5b8ba..e4853b5 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -833,7 +833,6 @@ static void *lec_tbl_walk(struct lec_state *state, struct hlist_head *tbl,
 			  loff_t *l)
 {
 	struct hlist_node *e = state->node;
-	struct lec_arp_table *tmp;
 
 	if (!e)
 		e = tbl->first;
@@ -842,9 +841,7 @@ static void *lec_tbl_walk(struct lec_state *state, struct hlist_head *tbl,
 		--*l;
 	}
 
-	tmp = container_of(e, struct lec_arp_table, next);
-
-	hlist_for_each_entry_from(tmp, next) {
+	for (; e; e = e->next) {
 		if (--*l < 0)
 			break;
 	}
-- 
1.9.3

[Linux-ATM-General] [PATCH 2/2] lec: Fix bug introduced by b67bfe0d42cac56c512dd5da4b1b347a23f4b70a

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

b67bfe0d42cac56c512dd5da4b1b347a23f4b70a dropped the node parameters
from iterators which lec_tbl_walk() was using to iterate the list.

Signed-off-by: Chas Williams <ch...@cm...>
---
 net/atm/lec.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/net/atm/lec.c b/net/atm/lec.c
index 4c5b8ba..e4853b5 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -833,7 +833,6 @@ static void *lec_tbl_walk(struct lec_state *state, struct hlist_head *tbl,
 			  loff_t *l)
 {
 	struct hlist_node *e = state->node;
-	struct lec_arp_table *tmp;
 
 	if (!e)
 		e = tbl->first;
@@ -842,9 +841,7 @@ static void *lec_tbl_walk(struct lec_state *state, struct hlist_head *tbl,
 		--*l;
 	}
 
-	tmp = container_of(e, struct lec_arp_table, next);
-
-	hlist_for_each_entry_from(tmp, next) {
+	for (; e; e = e->next) {
 		if (--*l < 0)
 			break;
 	}
-- 
1.9.3

[Linux-ATM-General] [PATCH] atm/svc: Fix blocking in wait loop

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

One should not call blocking primitives inside a wait loop, since both
require task_struct::state to sleep, so the inner will destroy the
outer state.

sigd_enq() will possibly sleep for alloc_skb().  Move sigd_enq() before
prepare_to_wait() to avoid sleeping while waiting interruptibly.  You do
not actually need to call sigd_enq() after the initial prepare_to_wait()
because we test the termination condition before calling schedule().

Based on suggestions from Peter Zijlstra.

Signed-off-by: Chas Williams <ch...@cm...>
Acked-by: Peter Zijlstra <pe...@in...>
---
 net/atm/svc.c | 60 +++++++++++++++++++++++++++++++----------------------------
 1 file changed, 32 insertions(+), 28 deletions(-)

diff --git a/net/atm/svc.c b/net/atm/svc.c
index d8e5d0c2..1ba23f5 100644
--- a/net/atm/svc.c
+++ b/net/atm/svc.c
@@ -50,12 +50,12 @@ static void svc_disconnect(struct atm_vcc *vcc)
 
 	pr_debug("%p\n", vcc);
 	if (test_bit(ATM_VF_REGIS, &vcc->flags)) {
-		prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE);
 		sigd_enq(vcc, as_close, NULL, NULL, NULL);
-		while (!test_bit(ATM_VF_RELEASED, &vcc->flags) && sigd) {
+		for (;;) {
+			prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE);
+			if (test_bit(ATM_VF_RELEASED, &vcc->flags) || !sigd)
+				break;
 			schedule();
-			prepare_to_wait(sk_sleep(sk), &wait,
-					TASK_UNINTERRUPTIBLE);
 		}
 		finish_wait(sk_sleep(sk), &wait);
 	}
@@ -126,11 +126,12 @@ static int svc_bind(struct socket *sock, struct sockaddr *sockaddr,
 	}
 	vcc->local = *addr;
 	set_bit(ATM_VF_WAITING, &vcc->flags);
-	prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE);
 	sigd_enq(vcc, as_bind, NULL, NULL, &vcc->local);
-	while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) {
-		schedule();
+	for (;;) {
 		prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE);
+		if (!test_bit(ATM_VF_WAITING, &vcc->flags) || !sigd)
+			break;
+		schedule();
 	}
 	finish_wait(sk_sleep(sk), &wait);
 	clear_bit(ATM_VF_REGIS, &vcc->flags); /* doesn't count */
@@ -202,15 +203,14 @@ static int svc_connect(struct socket *sock, struct sockaddr *sockaddr,
 		}
 		vcc->remote = *addr;
 		set_bit(ATM_VF_WAITING, &vcc->flags);
-		prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
 		sigd_enq(vcc, as_connect, NULL, NULL, &vcc->remote);
 		if (flags & O_NONBLOCK) {
-			finish_wait(sk_sleep(sk), &wait);
 			sock->state = SS_CONNECTING;
 			error = -EINPROGRESS;
 			goto out;
 		}
 		error = 0;
+		prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
 		while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) {
 			schedule();
 			if (!signal_pending(current)) {
@@ -297,11 +297,12 @@ static int svc_listen(struct socket *sock, int backlog)
 		goto out;
 	}
 	set_bit(ATM_VF_WAITING, &vcc->flags);
-	prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE);
 	sigd_enq(vcc, as_listen, NULL, NULL, &vcc->local);
-	while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) {
-		schedule();
+	for (;;) {
 		prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE);
+		if (!test_bit(ATM_VF_WAITING, &vcc->flags) || !sigd)
+			break;
+		schedule();
 	}
 	finish_wait(sk_sleep(sk), &wait);
 	if (!sigd) {
@@ -387,15 +388,15 @@ static int svc_accept(struct socket *sock, struct socket *newsock, int flags)
 		}
 		/* wait should be short, so we ignore the non-blocking flag */
 		set_bit(ATM_VF_WAITING, &new_vcc->flags);
-		prepare_to_wait(sk_sleep(sk_atm(new_vcc)), &wait,
-				TASK_UNINTERRUPTIBLE);
 		sigd_enq(new_vcc, as_accept, old_vcc, NULL, NULL);
-		while (test_bit(ATM_VF_WAITING, &new_vcc->flags) && sigd) {
+		for (;;) {
+			prepare_to_wait(sk_sleep(sk_atm(new_vcc)), &wait,
+					TASK_UNINTERRUPTIBLE);
+			if (!test_bit(ATM_VF_WAITING, &new_vcc->flags) || !sigd)
+				break;
 			release_sock(sk);
 			schedule();
 			lock_sock(sk);
-			prepare_to_wait(sk_sleep(sk_atm(new_vcc)), &wait,
-					TASK_UNINTERRUPTIBLE);
 		}
 		finish_wait(sk_sleep(sk_atm(new_vcc)), &wait);
 		if (!sigd) {
@@ -433,12 +434,14 @@ int svc_change_qos(struct atm_vcc *vcc, struct atm_qos *qos)
 	DEFINE_WAIT(wait);
 
 	set_bit(ATM_VF_WAITING, &vcc->flags);
-	prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE);
 	sigd_enq2(vcc, as_modify, NULL, NULL, &vcc->local, qos, 0);
-	while (test_bit(ATM_VF_WAITING, &vcc->flags) &&
-	       !test_bit(ATM_VF_RELEASED, &vcc->flags) && sigd) {
-		schedule();
+	for (;;) {
 		prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE);
+		if (!test_bit(ATM_VF_WAITING, &vcc->flags) ||
+		    test_bit(ATM_VF_RELEASED, &vcc->flags) || !sigd) {
+			break;
+		}
+		schedule();
 	}
 	finish_wait(sk_sleep(sk), &wait);
 	if (!sigd)
@@ -529,18 +532,18 @@ static int svc_addparty(struct socket *sock, struct sockaddr *sockaddr,
 
 	lock_sock(sk);
 	set_bit(ATM_VF_WAITING, &vcc->flags);
-	prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
 	sigd_enq(vcc, as_addparty, NULL, NULL,
 		 (struct sockaddr_atmsvc *) sockaddr);
 	if (flags & O_NONBLOCK) {
-		finish_wait(sk_sleep(sk), &wait);
 		error = -EINPROGRESS;
 		goto out;
 	}
 	pr_debug("added wait queue\n");
-	while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) {
-		schedule();
+	for (;;) {
 		prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
+		if (!test_bit(ATM_VF_WAITING, &vcc->flags) || !sigd)
+			break;
+		schedule();
 	}
 	finish_wait(sk_sleep(sk), &wait);
 	error = xchg(&sk->sk_err_soft, 0);
@@ -558,11 +561,12 @@ static int svc_dropparty(struct socket *sock, int ep_ref)
 
 	lock_sock(sk);
 	set_bit(ATM_VF_WAITING, &vcc->flags);
-	prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
 	sigd_enq2(vcc, as_dropparty, NULL, NULL, NULL, NULL, ep_ref);
-	while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) {
-		schedule();
+	for (;;) {
 		prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
+		if (!test_bit(ATM_VF_WAITING, &vcc->flags) || !sigd)
+			break;
+		schedule();
 	}
 	finish_wait(sk_sleep(sk), &wait);
 	if (!sigd) {
-- 
1.9.3

[Linux-ATM-General] unsubscribe

[LEE, K. <kl...@at...>]

Re: [Linux-ATM-General] [setsockopt] WARNING: CPU: 0 PID: 1444 at kernel/sched/core.c:7088 __might_sleep+0x51/0x16f()

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

On Thu, 7 Aug 2014 17:17:41 +0200
Peter Zijlstra <pe...@in...> wrote:

> Subject: atm: Fix blocking in wait loop
> 
> One should not call blocking primitives inside a wait loop, since both
> require task_struct::state to sleep, so the inner will destroy the outer
> state.
> 
> In this instance sigd_enq() will possible sleep for alloc_skb(), now if
> I understand the code right, we do not actually need to call sigd_enq()
> after the initial prepare_to_wait(), because we test the termination
> condition before schedule() anyhow.
> 
> So we can simply move it up a bit and avoid the entire confusion.
> 
> Signed-off-by: Peter Zijlstra <pe...@in...>
> ---
>  net/atm/svc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/atm/svc.c b/net/atm/svc.c
> index d8e5d0c2ebbc..445ac238b69b 100644
> --- a/net/atm/svc.c
> +++ b/net/atm/svc.c
> @@ -297,8 +297,8 @@ static int svc_listen(struct socket *sock, int backlog)
>  		goto out;
>  	}
>  	set_bit(ATM_VF_WAITING, &vcc->flags);
> -	prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE);
>  	sigd_enq(vcc, as_listen, NULL, NULL, &vcc->local);
> +	prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE);
>  	while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) {
>  		schedule();
>  		prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE);


This isn't the only place that we queue a message for the signalling
daemon after a prepare_to_wait() uninterruptibly so this patch would
be incomplete as is.

What bothers me is the TASK_UNINTERRUPTIBLE -- I don't have a good
reason why any of these should be sleeping uninterruptibly.

Re: [Linux-ATM-General] [PATCH 3/5] drivers/atm/atmtcp.c: fix error return code

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

On Thu, 7 Aug 2014 15:31:05 +0200 (CEST)
Julia Lawall <jul...@li...> wrote:


> diff --git a/drivers/atm/atmtcp.c b/drivers/atm/atmtcp.c
> index 0e3f8f9..c8e4fb4 100644
> --- a/drivers/atm/atmtcp.c
> +++ b/drivers/atm/atmtcp.c
> @@ -299,6 +299,7 @@ static int atmtcp_c_send(struct atm_vcc *vcc,struct sk_buff *skb)
>  	out_vcc = find_vcc(dev, ntohs(hdr->vpi), ntohs(hdr->vci));
>  	read_unlock(&vcc_sklist_lock);
>  	if (!out_vcc) {
> +		result = -EUNATCH;
>  		atomic_inc(&vcc->stats->tx_err);
>  		goto done;
>  	}
> 

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

Re: [Linux-ATM-General] [PATCH 2/5] solos-pci: fix error return code

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

On Thu,  7 Aug 2014 14:49:07 +0200
Julia Lawall <Jul...@li...> wrote:

...
>  drivers/atm/solos-pci.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
> index 943cf0d..7652e8d 100644
> --- a/drivers/atm/solos-pci.c
> +++ b/drivers/atm/solos-pci.c
> @@ -1278,6 +1278,7 @@ static int fpga_probe(struct pci_dev *dev, const struct pci_device_id *id)
>  			card->dma_bounce = kmalloc(card->nr_ports * BUF_SIZE, GFP_KERNEL);
>  			if (!card->dma_bounce) {
>  				dev_warn(&card->dev->dev, "Failed to allocate DMA bounce buffers\n");
> +				err = -ENOMEM;
>  				/* Fallback to MMIO doesn't work */
>  				goto out_unmap_both;
>  			}
> 

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

Re: [Linux-ATM-General] [PATCH 3/5] drivers/atm/atmtcp.c: fix error return code

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

On Thu,  7 Aug 2014 14:49:06 +0200
Julia Lawall <Jul...@li...> wrote:

> From: Julia Lawall <Jul...@li...>
> 
> Convert a zero return value on error to a negative one, as returned
> elsewhere in the function.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
...
> 
> diff --git a/drivers/atm/atmtcp.c b/drivers/atm/atmtcp.c
> index 0e3f8f9..c8e4fb4 100644
> --- a/drivers/atm/atmtcp.c
> +++ b/drivers/atm/atmtcp.c
> @@ -299,6 +299,7 @@ static int atmtcp_c_send(struct atm_vcc *vcc,struct sk_buff *skb)
>  	out_vcc = find_vcc(dev, ntohs(hdr->vpi), ntohs(hdr->vci));
>  	read_unlock(&vcc_sklist_lock);
>  	if (!out_vcc) {
> +		result = -ESRCH;

This should probably be -EUNATCH to match the rest of the code.

>  		atomic_inc(&vcc->stats->tx_err);
>  		goto done;
>  	}
> 

Re: [Linux-ATM-General] [PATCH] atm: Remove fix me in solos-pci

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

Guy Ellis of Transverse says that Nathan Williams,
na...@tr..., will address this issue when he returns
from vacation.  So let's just wait for the 'official' fix.

On Tue, 22 Jul 2014 00:07:20 -0400
Nicholas Krause <xer...@gm...> wrote:

> This removes a fix me as the default statement for solos_bh is correct
> and needs no fixing.
> 
> Signed-off-by: Nicholas Krause <xer...@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 943cf0d..58a0a8f 100644
> --- a/drivers/atm/solos-pci.c
> +++ b/drivers/atm/solos-pci.c
> @@ -851,7 +851,7 @@ static void solos_bh(unsigned long card_arg)
>  				break;
>  
>  			case PKT_COMMAND:
> -			default: /* FIXME: Not really, surely? */
> +			default:
>  				if (process_command(card, port, skb))
>  					break;
>  				spin_lock(&card->cli_queue_lock);

Re: [Linux-ATM-General] solos-pci.c: Fix me

[Guy E. <gu...@tr...>]

Hi Chas/Nick,

I think the FIXME is reminder to deal correctly with an unknown command.
At the moment an unknown command is treated as a PKT_COMMAND which is 
incorrect.
I think the correct behaviour would be to ignore unknown commands and 
just free the skb.

That said I doubt this condition ever occurs, which is probably why it 
has been this way since day 1.

Nathan is on vacation at the moment, when he gets back in August I'll 
ask him to tidy this up.

Cheers,
  - Guy.





On 22/07/2014 4:18 AM, chas williams - CONTRACTOR wrote:
> On Mon, 21 Jul 2014 13:42:01 -0400
> Nick Krause <xer...@gm...> wrote:
>
>> On Mon, Jul 21, 2014 at 9:14 AM, chas williams - CONTRACTOR
>> <ch...@cm...> wrote:
> ...
>>> @@ -850,8 +850,7 @@ static void solos_bh(unsigned long card_arg)
>>>                                  dev_kfree_skb_any(skb);
>>>                                  break;
>>>
>>> -                       case PKT_COMMAND:
>>> -                       default: /* FIXME: Not really, surely? */
>>> +                       default: /* PKT_COMMAND */
>>>                                  if (process_command(card, port, skb))
>>>                                          break;power
>>>                                  spin_lock(&card->cli_queue_lock);
>>>
>>> and be done with it since that will preserve existing behavior.
>>
>> My only question then is this function causing bugs as is?
>> Cheers Nick
>>
> It has been there since the first version of this driver was released.
> If it were breaking things, I would have to guess it would have been
> noticed by now.
>
> Just looking at the PKT_* defines, I would wildly guess that PKT_COMMAND
> PKT_POPEN and PKT_PCLOSE are all being handled via the PKT_COMMAND|default
> case.  If not, popen and pclose would be leaking a skb for each usage
> (which would be opening and closing the PVC -- not very often).
>
> popen and pclose just seem like specialized PKT_COMMAND types.
> process_command() woud ignore PKT_POPEN and PKT_PCLOSE since they aren't
> long enough to contain a command.  So maybe the comment should just go
> away.
>
> If you had access to hardware you could probably figure this out pretty
> quickly.  The programmer's manual doesn't seem to be available.
>
> ------------------------------------------------------------------------------
> Want fast and easy access to all the code in your enterprise? Index and
> search up to 200,000 lines of code with a free copy of Black Duck
> Code Sight - the same software that powers the world's largest code
> search on Ohloh, the Black Duck Open Hub! Try it now.
> http://p.sf.net/sfu/bds
> _______________________________________________
> Linux-atm-general mailing list
> Lin...@li...
> https://lists.sourceforge.net/lists/listinfo/linux-atm-general
>


-- 
Guy Ellis
gu...@tr...
www.traverse.com.au
T: +61 3 9386 4435 M: +61 419 398 234

Re: [Linux-ATM-General] solos-pci.c: Fix me

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

On Mon, 21 Jul 2014 13:42:01 -0400
Nick Krause <xer...@gm...> wrote:

> On Mon, Jul 21, 2014 at 9:14 AM, chas williams - CONTRACTOR
> <ch...@cm...> wrote:
...
> > @@ -850,8 +850,7 @@ static void solos_bh(unsigned long card_arg)
> >                                 dev_kfree_skb_any(skb);
> >                                 break;
> >
> > -                       case PKT_COMMAND:
> > -                       default: /* FIXME: Not really, surely? */
> > +                       default: /* PKT_COMMAND */
> >                                 if (process_command(card, port, skb))
> >                                         break;power
> >                                 spin_lock(&card->cli_queue_lock);
> >
> > and be done with it since that will preserve existing behavior.
> 
> 
> My only question then is this function causing bugs as is?
> Cheers Nick
> 

It has been there since the first version of this driver was released.
If it were breaking things, I would have to guess it would have been
noticed by now.

Just looking at the PKT_* defines, I would wildly guess that PKT_COMMAND
PKT_POPEN and PKT_PCLOSE are all being handled via the PKT_COMMAND|default
case.  If not, popen and pclose would be leaking a skb for each usage
(which would be opening and closing the PVC -- not very often).

popen and pclose just seem like specialized PKT_COMMAND types.
process_command() woud ignore PKT_POPEN and PKT_PCLOSE since they aren't
long enough to contain a command.  So maybe the comment should just go
away.

If you had access to hardware you could probably figure this out pretty
quickly.  The programmer's manual doesn't seem to be available.

Re: [Linux-ATM-General] solos-pci.c: Fix me

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

On Sun, 20 Jul 2014 00:59:42 -0400
Nick Krause <xer...@gm...> wrote:

> Hey Chas,
> There seems to be a fix me in this file in the function, solos_bh.
> Is the default statement correct and I remove the fix me or
> does it need to be rewritten.
> Cheers Nick
> 

I am afraid that I don't know enough about the solos hardware to know
if it needs to be rewritten.  I gather the solos returns at least three
kinds of packets, data, status and command.  If you wish to eliminate
the FIXME comment, you could just write:

@@ -850,8 +850,7 @@ static void solos_bh(unsigned long card_arg)
                                dev_kfree_skb_any(skb);
                                break;
 
-                       case PKT_COMMAND:
-                       default: /* FIXME: Not really, surely? */
+                       default: /* PKT_COMMAND */
                                if (process_command(card, port, skb))
                                        break;
                                spin_lock(&card->cli_queue_lock);

and be done with it since that will preserve existing behavior.

Re: [Linux-ATM-General] Welcome to the "Linux-atm-general" mailing list

[Ico <at...@ze...>]

* On 2014-07-14 18:52:09 +0200, chas williams - CONTRACTOR wrote:
 
> On Fri, 11 Jul 2014 10:52:04 +0200
> Ico <at...@ze...> wrote:
> 
> > atmdiag always report zero's, even when running pppd with the pppoa
> > plugin. I also never see any traffic on the atm0 network inteface while
> > pppoa is connected and transmitting/receiving ppp data.
> 
> That seems really strange.  What atm network interface are you using?
> I am guess it is some usb atm device?

I'm using an embedded SAR in a broadcom SOC. Unfortunately, the driver
source is not available for inspection.

> I forgot.  This doesn't report AAL5 statistics but rather shows that
> the pvc is open and the traffic classes assigned.  However, atmdiag (if
> things are functioning) should be reporting total sent and received
> PDU's.

Clear. The fact that in my case the statstics are not updated is then
likely to be due to incomplete implementation in the broadcom drivers.

Thanks,

Ico

-- 
:wq
^X^Cy^K^X^C^C^C^C

Re: [Linux-ATM-General] Welcome to the "Linux-atm-general" mailing list

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

On Fri, 11 Jul 2014 10:52:04 +0200
Ico <at...@ze...> wrote:

> atmdiag always report zero's, even when running pppd with the pppoa
> plugin. I also never see any traffic on the atm0 network inteface while
> pppoa is connected and transmitting/receiving ppp data.

That seems really strange.  What atm network interface are you using?
I am guess it is some usb atm device?

I don't think pppoatm will send packets if it doesn't know what
encapsulation is being used, so even if pppd sends packets to the
pppoatm layer, pppoatm is not going to send them.  The pppoatm module
attempts to learn the encapsulation by looking at the arriving pdu's.
 
> > And you can look at /proc/net/atm/pvc to see stats for your atm
> > socket.  
> 
> All zeroes as well, even when the ppp is functioning.

I forgot.  This doesn't report AAL5 statistics but rather shows that
the pvc is open and the traffic classes assigned.  However, atmdiag (if
things are functioning) should be reporting total sent and received
PDU's.

Re: [Linux-ATM-General] Welcome to the "Linux-atm-general" mailing list

[Ico <at...@ze...>]

* On 2014-07-08 14:42:57 +0200, chas williams - CONTRACTOR wrote:
 
> On Mon, 07 Jul 2014 22:05:40 +0200
> Ico <at...@ze...> wrote:
> 
> > * On 2014-07-07 21:56:47 +0200, chas williams - CONTRACTOR wrote:
> >  
> > > If the in-kernel pppoatm works correctly on your hardware then it's
> > > probably something with ppp wanting you to start the conversation.
> > > 
> > > https://github.com/mmoya/ppp/blob/master/pppd/plugins/pppoatm/pppoatm.c
> > > 
> > > The connect_pppoatm() routine here looks similar to your program so at
> > > this point I think you need to start sending LCP messages to get the
> > > other end to respond.
> > 
> > This I tried: The example I posted was only the minimal code doing the
> > ATM part. My current test code forks a pppd in a pty and uses a poll()
> > loop to send data between the pty fd and the atm socket. Verifying using
> > strace() I do indeed see LCP data coming from pppd, and my program
> > sending it into the ATM socket. No response is given, however.
> 
> Take a look at the kernel pppoatm module.  There is optionally some
> encapsulation that is done to the ppp packet before it is sent along
> the pvc.  There are basically two kinds, llc and vc.  llc sticks an llc
> header on (and removes) the ppp packets.  I think vc encapsulation does
> nothing (essentially your above method).

>From reading the pppd code, I believe the PPP handshake is not done over
the ATM socket itself, but over a /dev/ppp device which gets attached to
the socket by a channel number with some magic PPP ioctl's. This might
mean that the PPP communication is not even possible directly over the
socket, at least on my platform.

I'll investigate further to see if I can mimic pppd's behaviour (opening
two /dev/ppp's, doing the ioctls, etc) to see if I'm able to get at
least the ppp handshake going.

> Also, make sure your atm program is actually sending the data.
> strace'ing pppd might show pppd sending data to the pty but unless
> something flushes the pipe, the data might not reach your atm program
> in a timely fashion.

I double checked, it is.

> Again use atmdiag to see the tx counters increment.  

atmdiag always report zero's, even when running pppd with the pppoa
plugin. I also never see any traffic on the atm0 network inteface while
pppoa is connected and transmitting/receiving ppp data.

> And you can look at /proc/net/atm/pvc to see stats for your atm
> socket.  

All zeroes as well, even when the ppp is functioning.


Thanks,

-- 
:wq
^X^Cy^K^X^C^C^C^C

Re: [Linux-ATM-General] Welcome to the "Linux-atm-general" mailing list

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

On Mon, 07 Jul 2014 10:01:38 +0200
Ico <at...@ze...> wrote:

> Not sure if this list is still alive, there does not seem to be very much going
> on in the linux-atm world these days, is there?

Only in the embedded world is my guess.

> The PPPoA plugin has one additional ioctl call which I omitted in my test program:
> 
>    r = ioctl(fd, ATM_SETBACKEND, &be);
>
> I understand this is ment to set the line discipline of the socket to PPP, so
> the kernel will know it will has to do PPP over the link. But since I don't
> want the kernel to handle the ppp for me, I omitted the call. I believe this
> might be part of the problem.

You are right.  This hooks the ppp protocol onto the ATM socket.  You
shouldn't need this if you just want to send and recv AAL5 frames.

> Any insight much appreciated. Is what I'm trying to do even possible?

Yes, it should be possible.  You could try the debug programs aread and
awrite (from the linux-atm distribution) to see if you can see the
frames, for instance:  aread 0.0.35  and watch for output.  If you need
to send some data, you can use awrite 0.0.35 'message' (since aread only
opens the pvc for reading).

Also, run atmdiag and see if you are reporting errors or drops.
Run dmesg after running your program to see if your hardware had
any issues with opening an atm socket.
 

> #define MAX_SDU 20000
> #define MAX_OFFSET 8
> 
> void usage(char *name)
> {
> 	fprintf(stderr, "usage: %s [vpi] [vci]\n", name);
> 	exit(1);
> }
> 
> int main(int argc, char *argv[])
> {
> 	struct sockaddr_atmpvc addr;
> 	struct atm_qos  qos;
> 	int fd;
> 	static unsigned char buffer[30];
> 	int r;
> 
> 
> 	fd = socket(PF_ATMPVC, SOCK_DGRAM, ATM_AAL5);

Usually, we say 0 instead of ATM_AAL5 here.  I believe the kernel code
ignores this value so it doesn't have any effect.

> 	if(fd < 0) {
> 		perror("socket error");
> 		return 1;
> 	}
> 
> 	memset(&qos, 0, sizeof(qos));
> 	qos.aal                     = ATM_AAL5;
> 	qos.txtp.traffic_class      = ATM_UBR;
> 	qos.txtp.max_sdu            = 500;
> 	qos.txtp.pcr                = ATM_MAX_PCR;

I don't think you need to set .pcr if you say UBR.  I don't think it
will hurt though.

> 	qos.rxtp = qos.txtp;
> 
> 	r = setsockopt(fd, SOL_ATM, SO_ATMQOS, &qos, sizeof(qos));
> 	if(r < 0) {
> 		perror("setsockopt SO_ATMQOS");
> 		return 1;
> 	}
> 
> 	int bufsize[1524];
> 	r = setsockopt(fd, SOL_SOCKET,SO_SNDBUF, &bufsize ,sizeof(bufsize));
> 	if(r < 0) {
> 		perror("setbufsize SO_ATMQOS");
> 		return 1;
> 	}
> 
> 	memset(&addr, 0, sizeof(addr));
> 	addr.sap_family = AF_ATMPVC;
> 	addr.sap_addr.itf = 0;
> 	addr.sap_addr.vpi = 0;

This might need to be 8.  I have seen both 0 and 8 for the vpi for
PPPoATM.

> 	addr.sap_addr.vci = 35;
> 
> 	r = connect(fd, (struct sockaddr *) &addr,  sizeof(addr));
> 	if(r < 0) {
> 		perror("connect");
> 		return 1;
> 	}
> 
> 	while (1) {
> 
> 		memset(buffer,0, sizeof(buffer));
> 
> 		r = read(fd, buffer, sizeof(buffer));
> 
> 		if (r < 0)
> 			perror("read()");
> 
> 		if (r > 0)
> 		{
> 			printf("received bytes %d\n", r);
> 		}
> 	}
> 
> 	return 0;
> }
> 

Re: [Linux-ATM-General] Welcome to the "Linux-atm-general" mailing list

[Ico <at...@ze...>]

Hi all,

Not sure if this list is still alive, there does not seem to be very much going
on in the linux-atm world these days, is there?

I'm pretty familiar with linux IP networking, but ATM is quite new to me. I
might be asking stupid questions; please tell me if I do.

I'm trying to access the raw PPP frames of a pppoa link from userspace. The
idea is that the ppp frames are to be relayed somewhere else and the PPP itself
is not handled by the machine that is running the ATM link.

I have a working setup with a board running PPPoA using the pppoa plugin from
the stock pppd. I've stolen some code from this said plugin, hoping to be able
to recv() and send() raw PPP frames from an ATM socked. My test code, simple as
it is, does however not seem to work.

No data is ever seen on the ATM socket, and sending PPP handshake data to the
socket does not give any replies from the BRAS.

The PPPoA plugin has one additional ioctl call which I omitted in my test program:

   r = ioctl(fd, ATM_SETBACKEND, &be);

I understand this is ment to set the line discipline of the socket to PPP, so
the kernel will know it will has to do PPP over the link. But since I don't
want the kernel to handle the ppp for me, I omitted the call. I believe this
might be part of the problem.

Any insight much appreciated. Is what I'm trying to do even possible?

Thank you,

Ico


#define MAX_SDU 20000
#define MAX_OFFSET 8

void usage(char *name)
{
	fprintf(stderr, "usage: %s [vpi] [vci]\n", name);
	exit(1);
}

int main(int argc, char *argv[])
{
	struct sockaddr_atmpvc addr;
	struct atm_qos  qos;
	int fd;
	static unsigned char buffer[30];
	int r;


	fd = socket(PF_ATMPVC, SOCK_DGRAM, ATM_AAL5);
	if(fd < 0) {
		perror("socket error");
		return 1;
	}

	memset(&qos, 0, sizeof(qos));
	qos.aal                     = ATM_AAL5;
	qos.txtp.traffic_class      = ATM_UBR;
	qos.txtp.max_sdu            = 500;
	qos.txtp.pcr                = ATM_MAX_PCR;
	qos.rxtp = qos.txtp;

	r = setsockopt(fd, SOL_ATM, SO_ATMQOS, &qos, sizeof(qos));
	if(r < 0) {
		perror("setsockopt SO_ATMQOS");
		return 1;
	}

	int bufsize[1524];
	r = setsockopt(fd, SOL_SOCKET,SO_SNDBUF, &bufsize ,sizeof(bufsize));
	if(r < 0) {
		perror("setbufsize SO_ATMQOS");
		return 1;
	}

	memset(&addr, 0, sizeof(addr));
	addr.sap_family = AF_ATMPVC;
	addr.sap_addr.itf = 0;
	addr.sap_addr.vpi = 0;
	addr.sap_addr.vci = 35;

	r = connect(fd, (struct sockaddr *) &addr,  sizeof(addr));
	if(r < 0) {
		perror("connect");
		return 1;
	}

	while (1) {

		memset(buffer,0, sizeof(buffer));

		r = read(fd, buffer, sizeof(buffer));

		if (r < 0)
			perror("read()");

		if (r > 0)
		{
			printf("received bytes %d\n", r);
		}
	}

	return 0;
}

-- 
:wq
^X^Cy^K^X^C^C^C^C