Using serial devices under linux

Alan Cox | 13 Aug 2012 14:43

Picon

[PATCH 1/8] n_gsm.c: Implement 3GPP27.010 DLC start-up procedure in MUX

From: xiaojin <jin.xiao <at> intel.com>

In 3GPP27.010 5.8.1, it defined:
The TE multiplexer initiates the establishment of the multiplexer control channel by sending a SABM frame
on DLCI 0 using the procedures of clause 5.4.1.
Once the multiplexer channel is established other DLCs may be established using the procedures of clause 5.4.1.
This patch implement 5.8.1 in MUX level, it make sure DLC0 is the first channel to be setup.

[or for those not familiar with the specification: it was possible to try
 and open a data connection while the control channel was not yet fully
 open, which is a spec violation and confuses some modems]

Signed-off-by: xiaojin <jin.xiao <at> intel.com>
Tested-by: Yin, Fengwei <fengwei.yin <at> intel.com>
[tweaked the order we check things and error code]
Signed-off-by: Alan Cox <alan <at> linux.intel.com>
Cc: The Horsebox <stable <at> kernel.org>
---

 drivers/tty/n_gsm.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 7a4bf30..5c6c2e2 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
 <at>  <at>  -2889,6 +2889,10  <at>  <at>  static int gsmtty_open(struct tty_struct *tty, struct file *filp)
 	gsm = gsm_mux[mux];
 	if (gsm->dead)
 		return -EL2HLT;

(Continue reading)

Alan Cox | 13 Aug 2012 14:43

Picon

[PATCH 2/8] n_gsm: uplink SKBs accumulate on list

From: Russ Gorby <russ.gorby <at> intel.com>

gsm_dlci_data_kick will not call any output function if tx_bytes > THRESH_LO
furthermore it will call the output function only once if tx_bytes == 0
If the size of the IP writes are on the order of THRESH_LO
we can get into a situation where skbs accumulate on the outbound list
being starved for events to call the output function.

gsm_dlci_data_kick now calls the sweep function when tx_bytes==0

Signed-off-by: Russ Gorby <russ.gorby <at> intel.com>
Tested-by: Kappel, LaurentX <laurentx.kappel <at> intel.com>
Signed-off-by: Alan Cox <alan <at> linux.intel.com>
Cc: Hay and Water <stable <at> kernel.org>
---

 drivers/tty/n_gsm.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 5c6c2e2..9b0a44d 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
 <at>  <at>  -971,16 +971,19  <at>  <at>  static void gsm_dlci_data_sweep(struct gsm_mux *gsm)
 static void gsm_dlci_data_kick(struct gsm_dlci *dlci)
 {
 	unsigned long flags;
+	int sweep;

 	spin_lock_irqsave(&dlci->gsm->tx_lock, flags);

(Continue reading)

Alan Cox | 13 Aug 2012 14:43

Picon

[PATCH 3/8] n_gsm : Flow control handling in Mux driver

From: Frederic Berat <fredericx.berat <at> intel.com>

- Correcting handling of FCon/FCoff in order to respect 27.010 spec
- Consider FCon/off will overide all dlci flow control except for
  dlci0 as we must be able to send control frames.
- Dlci constipated handling according to FC, RTC and RTR values.
- Modifying gsm_dlci_data_kick and gsm_dlci_data_sweep according
  to dlci constipated value

Signed-off-by: Frederic Berat <fredericx.berat <at> intel.com>
Signed-off-by: Russ Gorby <russ.gorby <at> intel.com>
Signed-off-by: Alan Cox <alan <at> linux.intel.com>
---

 drivers/tty/n_gsm.c |   79 +++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 57 insertions(+), 22 deletions(-)

diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 9b0a44d..6651285 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
 <at>  <at>  -673,6 +673,8  <at>  <at>  static struct gsm_msg *gsm_data_alloc(struct gsm_mux *gsm, u8 addr, int len,
  *
  *	The tty device has called us to indicate that room has appeared in
  *	the transmit queue. Ram more data into the pipe if we have any
+ *	If we have been flow-stopped by a CMD_FCOFF, then we can only
+ *	send messages on DLCI0 until CMD_FCON
  *
  *	FIXME: lock against link layer control transmissions
  */

(Continue reading)

Alan Cox | 13 Aug 2012 14:44

Picon

[PATCH 4/8] char: n_gsm: remove message filtering for contipated DLCI

From: samix.lebsir <samix.lebsir <at> intel.com>

The design of uplink flow control in the mux driver is
that for constipated channels data will backup into the
per-channel fifos, and any messages that make it to the
outbound message queue will still go out.
Code was added to also stop messages that were in the outbound
queue but this requires filtering through all the messages on the
queue for stopped dlcis and changes some of the mux logic unneccessarily.

The message fiiltering was removed to be in line w/ the original design
as the message filtering does not provide any solution.
Extra debug messages used during investigation were also removed.

Signed-off-by: samix.lebsir <samix.lebsir <at> intel.com>
Signed-off-by: Alan Cox <alan <at> linux.intel.com>
Cc: Dressage <stable <at> kernel.org>
---

 drivers/tty/n_gsm.c |   25 +------------------------
 1 file changed, 1 insertion(+), 24 deletions(-)

diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 6651285..9854edf 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
 <at>  <at>  -691,10 +691,6  <at>  <at>  static void gsm_data_kick(struct gsm_mux *gsm)
 			msg = msg->next;
 			continue;
 		}

(Continue reading)

Alan Cox | 13 Aug 2012 14:44

Picon

[PATCH 5/8] n_gsm: added interlocking for gsm_data_lock for certain code paths

From: Russ Gorby <russ.gorby <at> intel.com>

There were some locking holes in the management of the MUX's
message queue for 2 code paths:
1) gsmld_write_wakeup
2) receipt of CMD_FCON flow-control message
In both cases gsm_data_kick is called w/o locking so it can collide
with other other instances of gsm_data_kick (pulling messages tx_tail)
or potentially other instances of __gsm_data_queu (adding messages to tx_head)

Changed to take the tx_lock in these 2 cases

Signed-off-by: Russ Gorby <russ.gorby <at> intel.com>
Tested-by: Yin, Fengwei <fengwei.yin <at> intel.com>
Signed-off-by: Alan Cox <alan <at> linux.intel.com>
Cc: Riding School <stable <at> kernel.org>
---

 drivers/tty/n_gsm.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 9854edf..51ba2f2 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
 <at>  <at>  -1205,6 +1205,8  <at>  <at>  static void gsm_control_message(struct gsm_mux *gsm, unsigned int command,
 							u8 *data, int clen)
 {
 	u8 buf[1];
+	unsigned long flags;

(Continue reading)

Alan Cox | 13 Aug 2012 14:44

Picon

[PATCH 6/8] n_gsm: avoid accessing freed memory during CMD_FCOFF condition

From: Russ Gorby <russ.gorby <at> intel.com>

gsm_data_kick was recently modified to allow messages on the
tx queue bound for DLCI0 to flow even during FCOFF conditions.
Unfortunately we introduced a bug discovered by code inspection
where subsequent list traversers can access freed memory if
the DLCI0 messages were not all at the head of the list.

Replaced singly linked tx list w/ a list_head and used
provided interfaces for traversing and deleting members.

Signed-off-by: Russ Gorby <russ.gorby <at> intel.com>
Tested-by: Yin, Fengwei <fengwei.yin <at> intel.com>
Signed-off-by: Alan Cox <alan <at> linux.intel.com>
Cc: Riding School <stable <at> kernel.org>
---

 drivers/tty/n_gsm.c |   40 +++++++++++++---------------------------
 1 file changed, 13 insertions(+), 27 deletions(-)

diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 51ba2f2..e2bdb8b 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
 <at>  <at>  -108,7 +108,7  <at>  <at>  struct gsm_mux_net {
  */

 struct gsm_msg {
-	struct gsm_msg *next;
+	struct list_head list;

(Continue reading)

Alan Cox | 13 Aug 2012 14:45

Picon

[PATCH 7/8] n_gsm: replace kfree_skb w/ appropriate dev_* versions

From: Russ Gorby <russ.gorby <at> intel.com>

Drivers are supposed to use the dev_* versions of the kfree_skb
interfaces. In a couple of cases we were called with IRQs
disabled as well which kfree_skb() does not expect.

Replaced kfree_skb calls w/ dev_kfree_skb and dev_kfree_skb_any

Signed-off-by: Russ Gorby <russ.gorby <at> intel.com>
Tested-by: Yin, Fengwei <fengwei.yin <at> intel.com>
Signed-off-by: Alan Cox <alan <at> linux.intel.com>
Cc: Grooming <stable <at> kernel.org>
---

 drivers/tty/n_gsm.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index e2bdb8b..17c9e94 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
 <at>  <at>  -879,7 +879,7  <at>  <at>  static int gsm_dlci_data_output_framed(struct gsm_mux *gsm,
 	if (len > gsm->mtu) {
 		if (dlci->adaption == 3) {
 			/* Over long frame, bin it */
-			kfree_skb(dlci->skb);
+			dev_kfree_skb_any(dlci->skb);
 			dlci->skb = NULL;
 			return 0;
 		}

(Continue reading)

Alan Cox | 13 Aug 2012 14:45

Picon

[PATCH 8/8] n_gsm: memory leak in uplink error path

From: Russ Gorby <russ.gorby <at> intel.com>

Uplink (TX) network data will go through gsm_dlci_data_output_framed
there is a bug where if memory allocation fails, the skb which
has already been pulled off the list will be lost.

In addition TX skbs were being processed in LIFO order

Fixed the memory leak, and changed to FIFO order processing

Signed-off-by: Russ Gorby <russ.gorby <at> intel.com>
Tested-by: Kappel, LaurentX <laurentx.kappel <at> intel.com>
Signed-off-by: Alan Cox <alan <at> linux.intel.com>
Cc: Showjumping <stable <at> kernel.org>
---

 drivers/tty/n_gsm.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 17c9e94..793bc38 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
 <at>  <at>  -868,7 +868,7  <at>  <at>  static int gsm_dlci_data_output_framed(struct gsm_mux *gsm,

 	/* dlci->skb is locked by tx_lock */
 	if (dlci->skb == NULL) {
-		dlci->skb = skb_dequeue(&dlci->skb_list);
+		dlci->skb = skb_dequeue_tail(&dlci->skb_list);
 		if (dlci->skb == NULL)

(Continue reading)

Greg KH | 16 Aug 2012 20:57

Re: [PATCH 1/8] n_gsm.c: Implement 3GPP27.010 DLC start-up procedure in MUX

On Mon, Aug 13, 2012 at 01:43:15PM +0100, Alan Cox wrote:
> From: xiaojin <jin.xiao <at> intel.com>
> 
> In 3GPP27.010 5.8.1, it defined:
> The TE multiplexer initiates the establishment of the multiplexer control channel by sending a SABM
frame on DLCI 0 using the procedures of clause 5.4.1.
> Once the multiplexer channel is established other DLCs may be established using the procedures of clause 5.4.1.
> This patch implement 5.8.1 in MUX level, it make sure DLC0 is the first channel to be setup.
> 
> [or for those not familiar with the specification: it was possible to try
>  and open a data connection while the control channel was not yet fully
>  open, which is a spec violation and confuses some modems]
> 
> Signed-off-by: xiaojin <jin.xiao <at> intel.com>
> Tested-by: Yin, Fengwei <fengwei.yin <at> intel.com>
> [tweaked the order we check things and error code]
> Signed-off-by: Alan Cox <alan <at> linux.intel.com>
> Cc: The Horsebox <stable <at> kernel.org>

In the future, it's <stable <at> vger.kernel.org>, I can catch these
addresses, but it's not as automated.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

(Continue reading)

Greg KH | 16 Aug 2012 21:01

Re: [PATCH 1/8] n_gsm.c: Implement 3GPP27.010 DLC start-up procedure in MUX

On Mon, Aug 13, 2012 at 01:43:15PM +0100, Alan Cox wrote:
> From: xiaojin <jin.xiao <at> intel.com>
> 
> In 3GPP27.010 5.8.1, it defined:
> The TE multiplexer initiates the establishment of the multiplexer control channel by sending a SABM
frame on DLCI 0 using the procedures of clause 5.4.1.
> Once the multiplexer channel is established other DLCs may be established using the procedures of clause 5.4.1.
> This patch implement 5.8.1 in MUX level, it make sure DLC0 is the first channel to be setup.
> 
> [or for those not familiar with the specification: it was possible to try
>  and open a data connection while the control channel was not yet fully
>  open, which is a spec violation and confuses some modems]
> 
> Signed-off-by: xiaojin <jin.xiao <at> intel.com>
> Tested-by: Yin, Fengwei <fengwei.yin <at> intel.com>
> [tweaked the order we check things and error code]
> Signed-off-by: Alan Cox <alan <at> linux.intel.com>
> Cc: The Horsebox <stable <at> vger.kernel.org>
> ---
> 
>  drivers/tty/n_gsm.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> index 7a4bf30..5c6c2e2 100644
> --- a/drivers/tty/n_gsm.c
> +++ b/drivers/tty/n_gsm.c
>  <at>  <at>  -2889,6 +2889,10  <at>  <at>  static int gsmtty_open(struct tty_struct *tty, struct file *filp)
>  	gsm = gsm_mux[mux];
>  	if (gsm->dead)

(Continue reading)

Alan Cox | 16 Aug 2012 21:12

Face

Picon

Re: [PATCH 1/8] n_gsm.c: Implement 3GPP27.010 DLC start-up procedure in MUX

On Thu, 16 Aug 2012 12:01:04 -0700
Greg KH <greg <at> kroah.com> wrote:

> On Mon, Aug 13, 2012 at 01:43:15PM +0100, Alan Cox wrote:
> > From: xiaojin <jin.xiao <at> intel.com>
> > 
> > In 3GPP27.010 5.8.1, it defined:
> > The TE multiplexer initiates the establishment of the multiplexer control channel by sending a SABM
frame on DLCI 0 using the procedures of clause 5.4.1.
> > Once the multiplexer channel is established other DLCs may be established using the procedures of
clause 5.4.1.
> > This patch implement 5.8.1 in MUX level, it make sure DLC0 is the first channel to be setup.
> > 
> > [or for those not familiar with the specification: it was possible to try
> >  and open a data connection while the control channel was not yet fully
> >  open, which is a spec violation and confuses some modems]
> > 
> > Signed-off-by: xiaojin <jin.xiao <at> intel.com>
> > Tested-by: Yin, Fengwei <fengwei.yin <at> intel.com>
> > [tweaked the order we check things and error code]
> > Signed-off-by: Alan Cox <alan <at> linux.intel.com>
> > Cc: The Horsebox <stable <at> vger.kernel.org>
> > ---
> > 
> >  drivers/tty/n_gsm.c |    4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> > index 7a4bf30..5c6c2e2 100644
> > --- a/drivers/tty/n_gsm.c

(Continue reading)

Greg KH | 16 Aug 2012 21:17

Re: [PATCH 1/8] n_gsm.c: Implement 3GPP27.010 DLC start-up procedure in MUX

On Thu, Aug 16, 2012 at 08:12:55PM +0100, Alan Cox wrote:
> On Thu, 16 Aug 2012 12:01:04 -0700
> Greg KH <greg <at> kroah.com> wrote:
> 
> > On Mon, Aug 13, 2012 at 01:43:15PM +0100, Alan Cox wrote:
> > > From: xiaojin <jin.xiao <at> intel.com>
> > > 
> > > In 3GPP27.010 5.8.1, it defined:
> > > The TE multiplexer initiates the establishment of the multiplexer control channel by sending a SABM
frame on DLCI 0 using the procedures of clause 5.4.1.
> > > Once the multiplexer channel is established other DLCs may be established using the procedures of
clause 5.4.1.
> > > This patch implement 5.8.1 in MUX level, it make sure DLC0 is the first channel to be setup.
> > > 
> > > [or for those not familiar with the specification: it was possible to try
> > >  and open a data connection while the control channel was not yet fully
> > >  open, which is a spec violation and confuses some modems]
> > > 
> > > Signed-off-by: xiaojin <jin.xiao <at> intel.com>
> > > Tested-by: Yin, Fengwei <fengwei.yin <at> intel.com>
> > > [tweaked the order we check things and error code]
> > > Signed-off-by: Alan Cox <alan <at> linux.intel.com>
> > > Cc: The Horsebox <stable <at> vger.kernel.org>
> > > ---
> > > 
> > >  drivers/tty/n_gsm.c |    4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> > > index 7a4bf30..5c6c2e2 100644

(Continue reading)

Return

Return to gmane.linux.serial.

Search Archive

Language

Change language

Options

Current view: Threads only / Showing only 20 lines / Not hiding cited text.
Change to All messages, whole messages, or hide cited text.

Post a message
NNTP Newsgroup
Classic Gmane web interface
XML

XML

RSS Feed
List Information

About Gmane