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

[Martin Schiller <ms@dev.tdt.de>]

On 2021-02-01 12:38, Xie He wrote:
> On Mon, Feb 1, 2021 at 1:18 AM Martin Schiller <ms@dev.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.