linux-kernel - Re: [PATCH net] net: hdlc_x25: Use qdisc to queue outgoing LAPB frames

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210129173650.7c0b7cda@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Fri, 29 Jan 2021 17:36:50 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Martin Schiller <ms@....tdt.de>
Cc:     Xie He <xie.he.0141@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Linux X25 <linux-x25@...r.kernel.org>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Krzysztof Halasa <khc@...waw.pl>
Subject: Re: [PATCH net] net: hdlc_x25: Use qdisc to queue outgoing LAPB
 frames

On Fri, 29 Jan 2021 06:56:10 +0100 Martin Schiller wrote:
> On 2021-01-28 23:06, Xie He wrote:
> > On Thu, Jan 28, 2021 at 11:47 AM Jakub Kicinski <kuba@...nel.org> 
> > wrote:  
> >> 
> >> Noob question - could you point at or provide a quick guide to 
> >> layering
> >> here? I take there is only one netdev, and something maintains an
> >> internal queue which is not stopped when HW driver stops the qdisc?  
> > 
> > Yes, there is only one netdev. The LAPB module (net/lapb/) (which is
> > used as a library by the netdev driver - hdlc_x25.c) is maintaining an
> > internal queue which is not stopped when the HW driver stops the
> > qdisc.
> > 
> > The queue is "write_queue" in "struct lapb_cb" in
> > "include/net/lapb.h". The code that takes skbs out of the queue and
> > feeds them to lower layers for transmission is at the "lapb_kick"
> > function in "net/lapb/lapb_out.c".
> > 
> > The layering is like this:
> > 
> > Upper layer (Layer 3) (net/x25/ or net/packet/)
> > 
> > ^
> > | L3 packets (with control info)
> > v
> > 
> > The netdev driver (hdlc_x25.c)
> > 
> > ^
> > | L3 packets
> > v
> > 
> > The LAPB Module (net/lapb/)
> > 
> > ^
> > | LAPB (L2) frames
> > v
> > 
> > The netdev driver (hdlc_x25.c)
> > 
> > ^
> > | LAPB (L2) frames
> > | (also called HDLC frames in the context of the HDLC subsystem)
> > v
> > 
> > HDLC core (hdlc.c)
> > 
> > ^
> > | HDLC frames
> > v
> > 
> > HDLC Hardware Driver  
> 
> @Xie: Thank you for the detailed presentation.

Indeed, thanks for the diagram.

I'm still struggling to wrap my head around this.

Did you test your code with lockdep enabled? Which Qdisc are you using?
You're queuing the frames back to the interface they came from - won't
that cause locking issues?