All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dianne Skoll <dianne@skoll.ca>
To: Jiri Slaby <jirislaby@kernel.org>
Cc: linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: N_HDLC line discipline: Race condition
Date: Tue, 21 May 2024 10:15:14 -0400	[thread overview]
Message-ID: <20240521101435.0b6b3420@gato.skoll.ca> (raw)
In-Reply-To: <63a5a3c5-8362-4b93-a50e-10c9cdcffdd2@kernel.org>

On Tue, 21 May 2024 12:47:00 +0200
Jiri Slaby <jirislaby@kernel.org> wrote:

> I believe it is a correct behavior after all. As you use pty for 
> testing, the "framing" is lost during the pty-to-pty pass on the
> flush to ldisc path (receive_buf()).

That might be what's happening, but I don't think it matches the documentation
in n_hdlc.c.  If you read the comment block near the top of the file,
it says this:

 * All HDLC data is frame oriented which means:
 *
 * 1. tty write calls represent one complete transmit frame of data
 *    The device driver should accept the complete frame or none of
 *    the frame (busy) in the write method. Each write call should have
 *    a byte count in the range of 2-65535 bytes (2 is min HDLC frame
 *    with 1 addr byte and 1 ctrl byte). The max byte count of 65535
 *    should include any crc bytes required. For example, when using
 *    CCITT CRC32, 4 crc bytes are required, so the maximum size frame
 *    the application may transmit is limited to 65531 bytes. For CCITT
 *    CRC16, the maximum application frame size would be 65533.
 *
 *
 * 2. receive callbacks from the device driver represents
 *    one received frame. The device driver should bypass
 *    the tty flip buffer and call the line discipline receive
 *    callback directly to avoid fragmenting or concatenating
 *    multiple frames into a single receive callback.
 *
 *    The HDLC line discipline queues the receive frames in separate
 *    buffers so complete receive frames can be returned by the
 *    tty read calls.
 *
 * 3. tty read calls returns an entire frame of data or nothing.
[...] */

Point 2 says that the driver should avoid fragmenting frames, or concatenating
frames into a single receive callback.  Doesn't this imply that frame
boundaries should be preserved when you read() data, which happens reliably
when you add a small delay between write()'s?

Regards,

Dianne.

  reply	other threads:[~2024-05-21 14:15 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-24 21:31 N_HDLC line discipline: Race condition Dianne Skoll
2024-04-25 18:01 ` Dianne Skoll
2024-05-15 10:33   ` Jiri Slaby
2024-05-15 13:42     ` Dianne Skoll
2024-05-15 15:44       ` Dianne Skoll
2024-05-21 10:47   ` Jiri Slaby
2024-05-21 14:15     ` Dianne Skoll [this message]
2024-05-23  8:01       ` Jiri Slaby
2024-05-23 12:44         ` Dianne Skoll

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240521101435.0b6b3420@gato.skoll.ca \
    --to=dianne@skoll.ca \
    --cc=gregkh@linuxfoundation.org \
    --cc=jirislaby@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.