Thread (9 messages) flat view 9 messages, 3 authors, 2020-08-12

Re: [PATCH net] drivers/net/wan/x25_asy: Added needed_headroom and a skb->len check

From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: 2020-08-11 10:50:50
Also in: lkml

quoted
quoted
2) The code quality of this driver is actually very low, and I also
hope to improve it gradually. Actually this driver had been completely
broken for many years and no one had noticed this until I fixed it in
commit 8fdcabeac398 (drivers/net/wan/x25_asy: Fix to make it work)
last month.
Just curious: how come that netif_rx could be removed?
When receiving data, the driver should only submit skb to upper layers
after it has been processed by the lapb module, i.e., it should only
call netif_rx in the function x25_asy_data_indication. The removed
netif_rx is in the function x25_asy_bump. This function is responsible
for passing the skb to the lapb module to process. It doesn't make
sense to call netif_rx here. If we call netif_rx here, we may pass
control frames that shouldn't be passed to upper layers (and have been
consumed and freed by the lapb module) to upper layers.
Ah of course. Thanks for explaining.
quoted
One thing to keep in mind is that AF_PACKET sockets are not the normal
datapath. AF_X25 sockets are. But you mention that you also exercise
the upper layer? That gives confidence that these changes are not
accidentally introducing regressions for the default path while fixing
oddly crafted packets with (root only for a reason) packet sockets.
Yes, I test with AF_X25 sockets too to make sure the changes are OK.
I usually test AF_X25 sockets with:
https://github.com/hyanggi/testing_linux/blob/master/network_x25/x25/server.c
https://github.com/hyanggi/testing_linux/blob/master/network_x25/x25/client.c
Excellent. Thanks for the link. Good to know that these changes are
getting real code coverage.
I became interested in X.25 when I was trying different address
families that Linux supported. I tried AF_X25 sockets. And then I
tried to use the X.25 link layer directly through AF_PACKET. I believe
both AF_X25 sockets and AF_PACKET sockets need to work without
problems with X.25 drivers - lapbether and x25_asy. There is another
X.25 driver (hdlc_x25) in the kernel. I haven't been able to run that
driver. But that driver seems to be the real driver which is really
used, and I know Martin Schiller [off-list ref] is an active user and
developer of that driver.
Great, sounds like we might have additional LAPB and X25 maintainers soon? :)

MAINTAINERS lists Andrew Hendry as maintainer for X.25. Please do CC them.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help