Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

From: Martin Schiller
Date: Fri Feb 26 2021 - 09:23:09 EST


On 2021-02-22 09:56, Xie He wrote:
On Sun, Feb 21, 2021 at 11:14 PM Martin Schiller <ms@xxxxxxxxxx> wrote:

I'm not really happy with this change because it breaks compatibility.
We then suddenly have 2 interfaces; the X.25 routings are to be set via
the "new" hdlc<x>_x25 interfaces instead of the hdlc<x> interfaces.

I currently just don't have a nicer solution to fix this queueing
problem either. On the other hand, since the many years we have been
using the current state, I have never noticed any problems with
discarded frames. So it might be more a theoretical problem than a
practical one.

This problem becomes very serious when we use AF_PACKET sockets,
because the majority of frames would be dropped by the hardware
driver, which significantly impacts transmission speed. What I am
really doing is to enable adequate support for AF_PACKET sockets,
allowing users to use the bare (raw) LAPB protocol. If we take this
into consideration, this problem is no longer just a theoretical
problem, but a real practical issue.

I have now had a look at it. It works as expected.
I just wonder if it would not be more appropriate to call
the lapb_register() already in x25_hdlc_open(), so that the layer2
(lapb) can already "work" before the hdlc<x>_x25 interface is up.


Also, I have a hard time assessing if such a wrap is really enforceable.
Unfortunately I have no idea how many users there actually are.



If we don't want to break backward compatibility, there is another option:
We can create a new API for the HDLC subsystem for stopping/restarting
the TX queue, and replace all HDLC hardware drivers' netif_stop_queue
and netif_wake_queue calls with calls to this new API. This new API
would then call hdlc_x25 to stop/restart its internal queue.

But this option would require modifying all HDLC hardware drivers'
code, and frankly, not all HDLC hardware drivers' developers care
about running X.25 protocols on their hardware. So this would cause
both hardware driver instabilities and confusion for hardware driver
developers.