On Mon, Aug 10, 2020 at 12:21 AM Willem de Bruijn <willemdebruijn.ker...@gmail.com> wrote: > > Acked-by: Willem de Bruijn <will...@google.com>
Thank you so much! > > 1) I hope to set needed_headroom properly for all three X.25 drivers > > (lapbether, x25_asy, hdlc_x25) in the kernel. So that the upper layer > > (net/x25) can be changed to use needed_headroom to allocate skb, > > instead of the current way of using a constant to estimate the needed > > headroom. > > Which constant, X25_MAX_L2_LEN? Yes, by grepping X25_MAX_L2_LEN in net/x25, I can see it is used in various places to allocate and reserve the needed header space. For example in net/x25/af_x25.c, the function x25_sendmsg allocates and reserves a header space of X25_MAX_L2_LEN + X25_EXT_MIN_LEN. > > 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. > 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 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 <m...@dev.tdt.de> is an active user and developer of that driver.
