On Sat, Jan 30, 2021 at 11:16 AM Jakub Kicinski <kuba@xxxxxxxxxx> wrote: > > Sounds like too much afford for a sub-optimal workaround. > The qdisc semantics are borken in the proposed scheme (double > counting packets) - both in term of statistics and if user decides > to add a policer, filter etc. Hmm... Another solution might be creating another virtual device on top of the HDLC device (similar to what "hdlc_fr.c" does), so that we can first queue L3 packets in the virtual device's qdisc queue, and then queue the L2 frames in the actual HDLC device's qdisc queue. This way we can avoid the same outgoing data being queued to qdisc twice. But this would significantly change the way the user uses the hdlc_x25 driver. > Another worry is that something may just inject a packet with > skb->protocol == ETH_P_HDLC but unexpected structure (IDK if > that's a real concern). This might not be a problem. Ethernet devices also allow the user to inject raw frames with user constructed headers. "hdlc_fr.c" also allows the user to bypass the virtual circuit interfaces and inject raw frames directly on the HDLC interface. I think the receiving side should be able to recognize and drop invalid frames. > It may be better to teach LAPB to stop / start the internal queue. > The lower level drivers just needs to call LAPB instead of making > the start/wake calls directly to the stack, and LAPB can call the > stack. Would that not work? I think this is a good solution. But this requires changing a lot of code. The HDLC subsystem needs to be changed to allow HDLC Hardware Drivers to ask HDLC Protocol Drivers (like hdlc_x25.c) to stop/wake the TX queue. The hdlc_x25.c driver can then ask the LAPB module to stop/wake the queue. So this means new APIs need to be added to both the HDLC subsystem and the LAPB module, and a number of HDLC Hardware Drivers need to be changed to call the new API of the HDLC subsystem. Martin, do you have any suggestions?
