netdev - Re: [PATCH net] net: hdlc_x25: Use qdisc to queue outgoing LAPB frames

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1628f9442ccf18f9c08c98f122053fc0@dev.tdt.de>
Date:   Mon, 01 Feb 2021 14:14:41 +0100
From:   Martin Schiller <ms@....tdt.de>
To:     Xie He <xie.he.0141@...il.com>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Linux X25 <linux-x25@...r.kernel.org>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Krzysztof Halasa <khc@...waw.pl>
Subject: Re: [PATCH net] net: hdlc_x25: Use qdisc to queue outgoing LAPB
 frames

On 2021-02-01 12:38, Xie He wrote:
> On Mon, Feb 1, 2021 at 1:18 AM Martin Schiller <ms@....tdt.de> wrote:
>> 
>> I have thought about this issue again.
>> 
>> I also have to say that I have never noticed any problems in this area
>> before.
>> 
>> So again for (my) understanding:
>> When a hardware driver calls netif_stop_queue, the frames sent from
>> layer 3 (X.25) with dev_queue_xmit are queued and not passed 
>> "directly"
>> to x25_xmit of the hdlc_x25 driver.
>> 
>> So nothing is added to the write_queue anymore (except possibly
>> un-acked-frames by lapb_requeue_frames).
> 
> If the LAPB module only emits an L2 frame when an L3 packet comes from
> the upper layer, then yes, there would be no problem because the L3
> packet is already controlled by the qdisc and there is no need to
> control the corresponding L2 frame again.
> 
> However, the LAPB module can emits L2 frames when there's no L3 packet
> coming, when 1) there are some packets queued in the LAPB module's
> internal queue; and 2) the LAPB decides to send some control frame
> (e.g. by the timers).

But control frames are currently sent past the lapb write_queue.
So another queue would have to be created.

And wouldn't it be better to have it in the hdlc_x25 driver, leaving
LAPB unaffected?

> 
>> Shouldn't it actually be sufficient to check for netif_queue_stopped 
>> in
>> lapb_kick and then do "nothing" if necessary?
> 
> We can consider this situation: When the upper layer has nothing to
> send, but there are some packets in the LAPB module's internal queue
> waiting to be sent. The LAPB module will try to send the packets, but
> after it has sent out the first packet, it will meet the "queue
> stopped" situation. In this situation, it'd be preferable to
> immediately start sending the second packet after the queue is started
> again. "Doing nothing" in this situation would mean waiting until some
> other events occur, such as receiving responses from the other side,
> or receiving more outgoing packets from L3.
> 
>> As soon as the hardware driver calls netif_wake_queue, the whole thing
>> should just continue running.
> 
> This relies on the fact that the upper layer has something to send. If
> the upper layer has nothing to send, lapb_kick would not be
> automatically called again until some other events occur (such as
> receiving responses from the other side). I think it'd be better if we
> do not rely on the assumption that L3 is going to send more packets to
> us, as L3 itself would assume us to provide it a reliable link service
> and we should fulfill its expectation.