Re: [PATCH net-next v5] net: x25: Queue received packets in the drivers instead of per-CPU queues

2021-04-06 Thread Xie He
On Tue, Apr 6, 2021 at 4:14 PM David Miller  wrote:
>
> This no longe applies to net-next, please respin.

Oh. I see this has already been applied to net-next. Thank you!
There's no need to apply again.

Thanks!


Re: [PATCH net-next v5] net: x25: Queue received packets in the drivers instead of per-CPU queues

2021-04-06 Thread David Miller
From: Xie He 
Date: Fri,  2 Apr 2021 02:30:00 -0700

> X.25 Layer 3 (the Packet Layer) expects layer 2 to provide a reliable
> datalink service such that no packets are reordered or dropped. And
> X.25 Layer 2 (the LAPB layer) is indeed designed to provide such service.
> 
> However, this reliability is not preserved when a driver calls "netif_rx"
> to deliver the received packets to layer 3, because "netif_rx" will put
> the packets into per-CPU queues before they are delivered to layer 3.
> If there are multiple CPUs, the order of the packets may not be preserved.
> The per-CPU queues may also drop packets if there are too many.
> 
> Therefore, we should not call "netif_rx" to let it queue the packets.
> Instead, we should use our own queue that won't reorder or drop packets.
> 
> This patch changes all X.25 drivers to use their own queues instead of
> calling "netif_rx". The patch also documents this requirement in the
> "x25-iface" documentation.
> 
> Cc: Martin Schiller 
> Signed-off-by: Xie He 


This no longe applies to net-next, please respin.

Thank you.


Re: [PATCH net-next v5] net: x25: Queue received packets in the drivers instead of per-CPU queues

2021-04-06 Thread Xie He
On Mon, Apr 5, 2021 at 11:17 PM Martin Schiller  wrote:
>
> Acked-by: Martin Schiller 
>
> Just for the record: I'm certainly not always the fastest,
> but I don't work holidays or weekends. So you also need to have some
> patience.

Oh. Thank you! Sorry, I didn't know there was a holiday in Germany.
Happy holiday! :)


Re: [PATCH net-next v5] net: x25: Queue received packets in the drivers instead of per-CPU queues

2021-04-06 Thread Martin Schiller

On 2021-04-05 21:34, Xie He wrote:

Hi Martin,

Could you ack? Thanks!


Acked-by: Martin Schiller 

Just for the record: I'm certainly not always the fastest,
but I don't work holidays or weekends. So you also need to have some
patience.



Re: [PATCH net-next v5] net: x25: Queue received packets in the drivers instead of per-CPU queues

2021-04-05 Thread Xie He
Hi Martin,

Could you ack? Thanks!


[PATCH net-next v5] net: x25: Queue received packets in the drivers instead of per-CPU queues

2021-04-02 Thread Xie He
X.25 Layer 3 (the Packet Layer) expects layer 2 to provide a reliable
datalink service such that no packets are reordered or dropped. And
X.25 Layer 2 (the LAPB layer) is indeed designed to provide such service.

However, this reliability is not preserved when a driver calls "netif_rx"
to deliver the received packets to layer 3, because "netif_rx" will put
the packets into per-CPU queues before they are delivered to layer 3.
If there are multiple CPUs, the order of the packets may not be preserved.
The per-CPU queues may also drop packets if there are too many.

Therefore, we should not call "netif_rx" to let it queue the packets.
Instead, we should use our own queue that won't reorder or drop packets.

This patch changes all X.25 drivers to use their own queues instead of
calling "netif_rx". The patch also documents this requirement in the
"x25-iface" documentation.

Cc: Martin Schiller 
Signed-off-by: Xie He 
---

Change from v4 to v5:
Break a line that exceeds 80 characters.

Change from v1 to v4:
Add the "__GFP_NOMEMALLOC" flag to "dev_alloc_skb" calls, to prevent
pfmemalloc skbs from occurring.
(Changes in v2 and v3 are no longer present in v4.)

Change from RFC to v1:
Rebase onto the latest net-next.

---
 Documentation/networking/x25-iface.rst | 65 --
 drivers/net/wan/hdlc_x25.c | 30 ++--
 drivers/net/wan/lapbether.c| 49 +--
 3 files changed, 80 insertions(+), 64 deletions(-)

diff --git a/Documentation/networking/x25-iface.rst 
b/Documentation/networking/x25-iface.rst
index df401891dce6..f34e9ec64937 100644
--- a/Documentation/networking/x25-iface.rst
+++ b/Documentation/networking/x25-iface.rst
@@ -70,60 +70,13 @@ First Byte = 0x03 (X25_IFACE_PARAMS)
 LAPB parameters. To be defined.
 
 
+Requirements for the device driver
+--
 
-Possible Problems
-=
-
-(Henner Eisen, 2000-10-28)
-
-The X.25 packet layer protocol depends on a reliable datalink service.
-The LAPB protocol provides such reliable service. But this reliability
-is not preserved by the Linux network device driver interface:
-
-- With Linux 2.4.x (and above) SMP kernels, packet ordering is not
-  preserved. Even if a device driver calls netif_rx(skb1) and later
-  netif_rx(skb2), skb2 might be delivered to the network layer
-  earlier that skb1.
-- Data passed upstream by means of netif_rx() might be dropped by the
-  kernel if the backlog queue is congested.
-
-The X.25 packet layer protocol will detect this and reset the virtual
-call in question. But many upper layer protocols are not designed to
-handle such N-Reset events gracefully. And frequent N-Reset events
-will always degrade performance.
-
-Thus, driver authors should make netif_rx() as reliable as possible:
-
-SMP re-ordering will not occur if the driver's interrupt handler is
-always executed on the same CPU. Thus,
-
-- Driver authors should use irq affinity for the interrupt handler.
-
-The probability of packet loss due to backlog congestion can be
-reduced by the following measures or a combination thereof:
-
-(1) Drivers for kernel versions 2.4.x and above should always check the
-return value of netif_rx(). If it returns NET_RX_DROP, the
-driver's LAPB protocol must not confirm reception of the frame
-to the peer.
-This will reliably suppress packet loss. The LAPB protocol will
-automatically cause the peer to re-transmit the dropped packet
-later.
-The lapb module interface was modified to support this. Its
-data_indication() method should now transparently pass the
-netif_rx() return value to the (lapb module) caller.
-(2) Drivers for kernel versions 2.2.x should always check the global
-variable netdev_dropping when a new frame is received. The driver
-should only call netif_rx() if netdev_dropping is zero. Otherwise
-the driver should not confirm delivery of the frame and drop it.
-Alternatively, the driver can queue the frame internally and call
-netif_rx() later when netif_dropping is 0 again. In that case, delivery
-confirmation should also be deferred such that the internal queue
-cannot grow to much.
-This will not reliably avoid packet loss, but the probability
-of packet loss in netif_rx() path will be significantly reduced.
-(3) Additionally, driver authors might consider to support
-CONFIG_NET_HW_FLOWCONTROL. This allows the driver to be woken up
-when a previously congested backlog queue becomes empty again.
-The driver could uses this for flow-controlling the peer by means
-of the LAPB protocol's flow-control service.
+Packets should not be reordered or dropped when delivering between the
+Packet Layer and the device driver.
+
+To avoid packets from being reordered or dropped when delivering from
+the device driver to the Packet Layer, the device driver should not
+call "netif_rx" to deliver the received packets. Instead, it should
+call