[PATCH net] net/core/dev.c: Ensure pfmemalloc skbs are correctly handled when receiving

2021-04-16 Thread Xie He
Gorman Cc: David S. Miller Cc: Neil Brown Cc: Peter Zijlstra Cc: Jiri Slaby Cc: Mike Christie Cc: Eric B Munson Cc: Eric Dumazet Cc: Sebastian Andrzej Siewior Cc: Christoph Lameter Cc: Andrew Morton Signed-off-by: Xie He --- net/core/dev.c | 4 ++-- 1 file changed, 2 insertions(+), 2

Re: A data race between fanout_demux_rollover() and __fanout_unlink()

2021-04-13 Thread Xie He
On Tue, Apr 13, 2021 at 1:51 PM Gong, Sishuai wrote: > > Hi, > > We found a data race in linux-5.12-rc3 between af_packet.c functions > fanout_demux_rollover() and __fanout_unlink() and we are able to reproduce it > under x86. > > When the two functions are running together, __fanout_unlink() wi

Re: Problem in pfmemalloc skb handling in net/core/dev.c

2021-04-12 Thread Xie He
On Fri, Apr 9, 2021 at 12:12 PM Xie He wrote: > > This is exactly what I'm talking about. "skb_pfmemalloc_protocol" > cannot guarantee pfmemalloc skbs are not delivered to unrelated > protocols, because "__netif_receive_skb" will sometimes treat > pfmemal

Re: Problem in pfmemalloc skb handling in net/core/dev.c

2021-04-09 Thread Xie He
On Fri, Apr 9, 2021 at 4:50 AM Eric Dumazet wrote: > > On 4/9/21 12:14 PM, Xie He wrote: > > Then simply copy the needed logic. No, there's no such thing as "sockets" in some of the protocols. There is simply no way to copy "the needed logic". > >

Re: Problem in pfmemalloc skb handling in net/core/dev.c

2021-04-09 Thread Xie He
On Fri, Apr 9, 2021 at 3:04 AM Eric Dumazet wrote: > > Note that pfmemalloc skbs are normally dropped in sk_filter_trim_cap() > > Simply make sure your protocol use it. It seems "sk_filter_trim_cap" needs an "struct sock" argument. Some of my protocols act like a middle layer to another protocol

Re: Problem in pfmemalloc skb handling in net/core/dev.c

2021-04-09 Thread Xie He
On Fri, Apr 9, 2021 at 2:58 AM Mel Gorman wrote: > > On Fri, Apr 09, 2021 at 02:14:12AM -0700, Xie He wrote: > > > > Do you mean that at the time "sk_memalloc_socks()" changes from "true" > > to "false", there would be no in-flight skbs curren

Re: Problem in pfmemalloc skb handling in net/core/dev.c

2021-04-09 Thread Xie He
On Fri, Apr 9, 2021 at 1:44 AM Mel Gorman wrote: > > That would imply that the tap was communicating with a swap device to > allocate a pfmemalloc skb which shouldn't happen. Furthermore, it would > require the swap device to be deactivated while pfmemalloc skbs still > existed. Have you encounter

Re: Problem in pfmemalloc skb handling in net/core/dev.c

2021-04-09 Thread Xie He
On Fri, Apr 9, 2021 at 12:30 AM Mel Gorman wrote: > > Under what circumstances do you expect sk_memalloc_socks() to be false > and skb_pfmemalloc() to be true that would cause a problem? For example, if at the time the skb is allocated, "sk_memalloc_socks()" was true, then the skb might be alloca

Problem in pfmemalloc skb handling in net/core/dev.c

2021-04-08 Thread Xie He
Hi Mel Gorman, I may have found a problem in pfmemalloc skb handling in net/core/dev.c. I see there are "if" conditions checking for "sk_memalloc_socks() && skb_pfmemalloc(skb)", and when the condition is true, the skb is handled specially as a pfmemalloc skb, otherwise it is handled as a normal s

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 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 German

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
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.

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

2021-03-31 Thread Xie He
Hi Martin, Could you ack this patch again? The only change from the RFC version (that you previously acked) is the addition of the "__GFP_NOMEMALLOC" flag in "dev_alloc_skb". This is because I want to prevent pfmemalloc skbs (which can't be handled by netif_receive_skb_core) from occurring. Thank

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

2021-03-28 Thread Xie He
in the "x25-iface" documentation. Cc: Martin Schiller Signed-off-by: Xie He --- Change from v3: Remove all pfmemalloc checks. Instead, prevent these skbs from occurring. Change from v2: Remove printing of pfmemalloc check errors if LAPB module can handle them. Change from v1: Add pfmemallo

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

2021-03-28 Thread Xie He
in the "x25-iface" documentation. Cc: Martin Schiller Signed-off-by: Xie He --- Change from v2: No need to print out-of-memory errors when receiving data packets, because the LAPB module is able to handle this situation. (We still need to print the errors when generating control packets

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

2021-03-28 Thread Xie He
in the "x25-iface" documentation. Cc: Martin Schiller Signed-off-by: Xie He --- Change from v1: Drop all PFMEMALLOC skbs because we cannot handle them. Change from RFC (March 5, 2021): Rebased onto the latest net-next. Martin Schiller has acked the RFC ver

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

2021-03-28 Thread Xie He
"x25-iface" documentation. Acked-by: Martin Schiller Signed-off-by: Xie He --- Change from RFC (March 5, 2021): Rebased onto the latest net-next (onto the ndo_stop racing fixes for the drivers). --- Documentation/networking/x25-iface.rst | 65 -- drivers/net/w

[PATCH net-next v2] net: lapb: Make "lapb_t1timer_running" able to detect an already running timer

2021-03-21 Thread Xie He
quot; field can fully take over its responsibility. Therefore "t1timer_stop" is deleted. "t1timer_running" is not simply a negation of the old "t1timer_stop". At the end of the timer function, if it does not reschedule itself, "t1timer_running" is se

[PATCH net-next] net: lapb: Make "lapb_t1timer_running" able to detect an already running timer

2021-03-21 Thread Xie He
e over its responsibility. Therefore "t1timer_stop" is deleted. "t1timer_running" is not simply a negation of the old "t1timer_stop". At the end of the timer function, if it does not reschedule itself, "t1timer_running" is set to false to indicate that the tim

[PATCH net-next] net: lapbether: Close the LAPB device before its underlying Ethernet device closes

2021-03-18 Thread Xie He
last frame to notify the other side that it is disconnecting. Signed-off-by: Xie He --- drivers/net/wan/lapbether.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/wan/lapbether.c b/drivers/net/wan/lapbether.c index 8fda0446ff71..45d74285265a 100644 --- a/driver

Re: [PATCH net] net: lapbether: Prevent racing when checking whether the netif is running

2021-03-16 Thread Xie He
On Tue, Mar 16, 2021 at 8:17 AM Martin Schiller wrote: > > On 2021-03-14 02:59, Xie He wrote: > > Hi Martin, > > > > Could you ack? Thanks! > > Acked-by: Martin Schiller Thank you!!

[PATCH net v2] net: hdlc_x25: Prevent racing between "x25_close" and "x25_xmit"/"x25_rx"

2021-03-14 Thread Xie He
ssumes the HDLC hardware drivers have full control over the TX queue, and the HDLC protocol drivers (like this driver) have no control. Controlling the queue here in the protocol driver may interfere with hardware drivers' control of the queue. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")

[PATCH net] net: hdlc_x25: Prevent racing between "x25_close" and "x25_xmit"/"x25_rx"

2021-03-14 Thread Xie He
x25_close". Reasons for not solving the racing between "x25_close" and "x25_xmit" by calling "netif_tx_disable" in "x25_close": 1. We still need to solve the racing between "x25_close" and "x25_rx"; 2. The design of the HDLC subsystem a

Re: [PATCH net] net: lapbether: Prevent racing when checking whether the netif is running

2021-03-13 Thread Xie He
Hi Martin, Could you ack? Thanks!

Re: [PATCH] net: socket.c: Fix comparison issues

2021-03-11 Thread Xie He
What is the reason for this change? Why is the new way better than the old way?

Re: [PATCH net] net: lapbether: Prevent racing when checking whether the netif is running

2021-03-11 Thread Xie He
On Thu, Mar 11, 2021 at 4:10 PM Jakub Kicinski wrote: > > And the "noqueue" queue is there because it's on top of hdlc_fr.c > somehow or some out of tree driver? Or do you install it manually? No, this driver is not related to "hdlc_fr.c" or any out-of-tree driver. The default qdisc is "noqueue"

Re: [PATCH net] net: lapbether: Prevent racing when checking whether the netif is running

2021-03-11 Thread Xie He
On Thu, Mar 11, 2021 at 2:52 PM Jakub Kicinski wrote: > > Normally driver's ndo_stop() calls netif_tx_disable() which takes TX > locks, so unless your driver is lockless (LLTX) there should be no xmit > calls after that point. Do you mean I should call "netif_tx_disable" inside my "ndo_stop" func

Re: [PATCH net] net: lapbether: Prevent racing when checking whether the netif is running

2021-03-11 Thread Xie He
On Thu, Mar 11, 2021 at 12:43 PM Jakub Kicinski wrote: > > Is this a theoretical issues or do you see a path where it triggers? > > Who are the callers sending frames to a device which went down? This is a theoretical issue. I didn't see this issue in practice. When "__dev_queue_xmit" and "sch_d

[PATCH net] net: lapbether: Prevent racing when checking whether the netif is running

2021-03-10 Thread Xie He
th_xmit" and "lapbeth_rcv" can reliably check and ensure the netif is running while doing their work. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Xie He --- drivers/net/wan/lapbether.c | 32 +--- 1 file changed, 25 insertions(+), 7

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

2021-03-09 Thread Xie He
On Tue, Mar 9, 2021 at 5:23 AM Martin Schiller wrote: > > I've tested the hdlc_x25 driver. > Looks good to me. > > Acked-by: Martin Schiller Thank you!! I'll re-send this patch after net-next is re-opened and my other fixes get merged into net-next. Thanks!

[PATCH net] net: lapbether: Remove netif_start_queue / netif_stop_queue

2021-03-07 Thread Xie He
quot; in "ops->ndo_open", because after a netdev is allocated and registered, the "__QUEUE_STATE_DRV_XOFF" flag is initially not set, so there is no need to call "netif_start_queue" to clear it. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Xie He --

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

2021-03-04 Thread Xie He
in the "x25-iface" documentation. Cc: Martin Schiller Signed-off-by: Xie He --- Documentation/networking/x25-iface.rst | 65 -- drivers/net/wan/hdlc_x25.c | 29 +++- drivers/net/wan/lapbether.c| 46 -- 3 files c

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-03-03 Thread Xie He
On Wed, Mar 3, 2021 at 5:26 AM Martin Schiller wrote: > > On 2021-03-03 00:30, Jakub Kicinski wrote: > > > > Hard question to answer, existing users seem happy and Xie's driver > > isn't upstream, so the justification for potentially breaking backward > > compatibility isn't exactly "strong". > >

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-03-01 Thread Xie He
On Sun, Feb 28, 2021 at 10:56 PM Martin Schiller wrote: > > >> Also, I have a hard time assessing if such a wrap is really > >> enforceable. > > > > Sorry. I don't understand what you mean. What "wrap" are you referring > > to? > > I mean the change from only one hdlc interface to both hdlc and >

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-26 Thread Xie He
On Fri, Feb 26, 2021 at 6:21 AM Martin Schiller wrote: > > I have now had a look at it. It works as expected. > I just wonder if it would not be more appropriate to call > the lapb_register() already in x25_hdlc_open(), so that the layer2 > (lapb) can already "work" before the hdlc_x25 interface i

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-22 Thread Xie He
On Sun, Feb 21, 2021 at 11:14 PM Martin Schiller wrote: > > I'm not really happy with this change because it breaks compatibility. > We then suddenly have 2 interfaces; the X.25 routings are to be set via > the "new" hdlc_x25 interfaces instead of the hdlc interfaces. > > I currently just don't ha

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-21 Thread Xie He
On Sat, Feb 20, 2021 at 10:39 PM Leon Romanovsky wrote: > > > Yes, this patch will break backward compatibility. Users with old > > scripts will find them no longer working. > > Did you search in debian/fedora code repositories to see if such scripts exist > as part of any distro package? I just

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-19 Thread Xie He
On Fri, Feb 19, 2021 at 10:39 AM Jakub Kicinski wrote: > > Not entirely sure what the argument is about but adding constants would > certainly help. Leon wants me to replace this: dev->needed_headroom = 3 - 1; with this: /* 2 is the result of 3 - 1 */ dev->needed_headroom = 2; But I don't fee

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-18 Thread Xie He
On Thu, Feb 18, 2021 at 12:06 PM Xie He wrote: > > On Thu, Feb 18, 2021 at 11:55 AM Leon Romanovsky wrote: > > > > This is how we write code, we use defines instead of constant numbers, > > comments to describe tricky parts and assign already preprocessed result. > &

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-18 Thread Xie He
On Thu, Feb 18, 2021 at 11:55 AM Leon Romanovsky wrote: > > This is how we write code, we use defines instead of constant numbers, > comments to describe tricky parts and assign already preprocessed result. > > There is nothing I can do If you don't like or don't want to use Linux kernel > style.

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-18 Thread Xie He
On Thu, Feb 18, 2021 at 2:37 AM Leon Romanovsky wrote: > > It is not me who didn't explain, it is you who didn't want to write clear > comment that describes the headroom size without need of "3 - 1". Why do I need to write unnecessary comments when "3 - 1" and the current comment already explain

Re: [PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-18 Thread Xie He
On Thu, Feb 18, 2021 at 12:57 AM Leon Romanovsky wrote: > > It is nice that you are resending your patch without the resolution. > However it will be awesome if you don't ignore review comments and fix this > "3 - 1" > by writing solid comment above. I thought you already agreed with me? It look

[PATCH net-next RFC v4] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-16 Thread Xie He
l X.25 device to send (L3) packets and then use the actual HDLC device to queue LAPB (L2) frames. The outgoing (L2) LAPB queue will be controlled by the hardware driver's netif_stop_queue and netif_wake_queue calls, while outgoing (L3) packets will not be affected by these calls. Cc: Martin S

Re: [PATCH net-next RFC v3] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-15 Thread Xie He
On Mon, Feb 15, 2021 at 10:04 PM Leon Romanovsky wrote: > > On Mon, Feb 15, 2021 at 11:08:02AM -0800, Xie He wrote: > > On Mon, Feb 15, 2021 at 10:54 AM Leon Romanovsky wrote: > > > > > > On Mon, Feb 15, 2021 at 09:23:32AM -0800, Xie He wrote: > > >

Re: [PATCH net-next RFC v3] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-15 Thread Xie He
On Mon, Feb 15, 2021 at 10:54 AM Leon Romanovsky wrote: > > On Mon, Feb 15, 2021 at 09:23:32AM -0800, Xie He wrote: > > On Mon, Feb 15, 2021 at 1:25 AM Leon Romanovsky wrote: > > > > > > > + /* When transmitting data: > > > > + *

Re: [PATCH net-next RFC v3] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-15 Thread Xie He
On Mon, Feb 15, 2021 at 1:25 AM Leon Romanovsky wrote: > > > + /* When transmitting data: > > + * first we'll remove a pseudo header of 1 byte, > > + * then the LAPB module will prepend an LAPB header of at most 3 > > bytes. > > + */ > > + dev->needed_headroom = 3 - 1; > >

[PATCH net-next RFC v3] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-14 Thread Xie He
l X.25 device to send (L3) packets and then use the actual HDLC device to queue LAPB (L2) frames. The outgoing (L2) LAPB queue will be controlled by the hardware driver's netif_stop_queue and netif_wake_queue calls, while outgoing (L3) packets will not be affected by these calls. Cc: Martin S

Re: [PATCH net-next RFC v2] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-14 Thread Xie He
On Sun, Feb 14, 2021 at 10:27 PM Martin Schiller wrote: > > At first glance, the patch looks quite reasonable. The only thing I > noticed right away is that you also included the changes of your patch > "Return meaningful error code in x25_open". Thanks! It was because this patch was sent before

[PATCH net-next RFC v2] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-10 Thread Xie He
l not be affected by these calls. Cc: Martin Schiller Signed-off-by: Xie He --- Change from RFC v1: Properly initialize state(hdlc)->x25_dev and state(hdlc)->x25_dev_lock. --- drivers/net/wan/hdlc_x25.c | 158 ++--- 1 file changed, 129 insertions(+), 29

[PATCH net-next RFC] net: hdlc_x25: Queue outgoing LAPB frames

2021-02-10 Thread Xie He
l not be affected by these calls. Cc: Martin Schiller Signed-off-by: Xie He --- drivers/net/wan/hdlc_x25.c | 156 ++--- 1 file changed, 127 insertions(+), 29 deletions(-) diff --git a/drivers/net/wan/hdlc_x25.c b/drivers/net/wan/hdlc_x25.c index bb164805804e.

Re: [PATCH net-next] net/packet: Improve the comment about LL header visibility criteria

2021-02-06 Thread Xie He
On Sat, Feb 6, 2021 at 7:43 AM Eyal Birger wrote: > > As such, may I suggest making this explicit by making > dev_hard_header() use dev_has_header()? That is a good idea. We may submit another patch for that.

[PATCH net-next] net/packet: Improve the comment about LL header visibility criteria

2021-02-05 Thread Xie He
>header_ops, it also checks for dev->header_ops->create. When transmitting an skb on a device, dev_hard_header can be called to generate an LL header. dev_hard_header will only generate a header if dev->header_ops->create is present. Signed-off-by: Xie He --- net/packet/af_p

[PATCH net] net: hdlc_x25: Return meaningful error code in x25_open

2021-02-02 Thread Xie He
("Linux-2.6.12-rc2") Cc: Martin Schiller Signed-off-by: Xie He --- drivers/net/wan/hdlc_x25.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/wan/hdlc_x25.c b/drivers/net/wan/hdlc_x25.c index bb164805804e..4aaa6388b9ee 100644 --- a/drivers/net/wan

Re: [PATCH net] net: lapb: Copy the skb before sending a packet

2021-02-01 Thread Xie He
On Mon, Feb 1, 2021 at 8:42 PM Jakub Kicinski wrote: > > On Mon, 1 Feb 2021 08:14:31 -0800 Xie He wrote: > > On Mon, Feb 1, 2021 at 6:10 AM Julian Wiedmann wrote: > > > This sounds a bit like you want skb_cow_head() ... ? > > > > Calling "skb_cow_head&qu

Re: [PATCH net] net: lapb: Copy the skb before sending a packet

2021-02-01 Thread Xie He
On Mon, Feb 1, 2021 at 6:10 AM Julian Wiedmann wrote: > > This sounds a bit like you want skb_cow_head() ... ? Calling "skb_cow_head" before we call "skb_clone" would indeed solve the problem of writes to our clones affecting clones in other parts of the system. But since we are still writing to

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

2021-02-01 Thread Xie He
On Mon, Feb 1, 2021 at 5:14 AM Martin Schiller wrote: > > But control frames are currently sent past the lapb write_queue. > So another queue would have to be created. > > And wouldn't it be better to have it in the hdlc_x25 driver, leaving > LAPB unaffected? Hmm.. Indeed. I agree. I also think

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

2021-02-01 Thread Xie He
On Mon, Feb 1, 2021 at 1:18 AM Martin Schiller wrote: > > I have thought about this issue again. > > I also have to say that I have never noticed any problems in this area > before. > > So again for (my) understanding: > When a hardware driver calls netif_stop_queue, the frames sent from > layer 3

Re: [PATCH net] net: lapb: Copy the skb before sending a packet

2021-02-01 Thread Xie He
On Mon, Feb 1, 2021 at 2:05 AM Martin Schiller wrote: > > What kind of packages do you mean are corrupted? > ETH_P_X25 or ETH_P_HDLC? I mean ETH_P_X25. I was using "lapbether.c" to test so there was no ETH_P_HDLC. > I have also sent a patch here in the past that addressed corrupted > ETH_P_X25 f

[PATCH net] net: lapb: Copy the skb before sending a packet

2021-01-31 Thread Xie He
r before handing over the skb to us, if we don't copy the skb before prepending the LAPB header, the first byte of the packets received on AF_PACKET sockets can be corrupted. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: Martin Schiller Signed-off-by: Xie He --- net/lapb/lapb_out.c |

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

2021-01-30 Thread Xie He
On Sat, Jan 30, 2021 at 11:16 AM Jakub Kicinski 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

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

2021-01-30 Thread Xie He
On Fri, Jan 29, 2021 at 5:36 PM Jakub Kicinski wrote: > > 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? Hmm... Than

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

2021-01-28 Thread Xie He
On Thu, Jan 28, 2021 at 11:47 AM Jakub Kicinski 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 netde

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

2021-01-27 Thread Xie He
On Wed, Jan 27, 2021 at 2:14 AM David Laight wrote: > > If I read this correctly it adds a (potentially big) queue between the > LAPB code that adds the sequence numbers to the frames and the hardware > that actually sends them. Yes. The actual number of outgoing LAPB frames being queued depends

[PATCH net] net: hdlc_x25: Use qdisc to queue outgoing LAPB frames

2021-01-27 Thread Xie He
es: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: Martin Schiller Cc: Krzysztof Halasa Signed-off-by: Xie He --- drivers/net/wan/hdlc_x25.c | 17 + 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/net/wan/hdlc_x25.c b/drivers/net/wan/hdlc_x25.c index bb1648

[PATCH net v6] net: lapb: Add locking to the lapb module

2021-01-26 Thread Xie He
3. Let lapb_unregister wait for other API functions and running timers to stop. 4. The lapb_device_event function calls lapb_disconnect_request. In order to avoid trying to hold the lock twice, add a new function named "__lapb_disconnect_request" which assumes the lock is held, and make it

Re: [PATCH net v5] net: lapb: Add locking to the lapb module

2021-01-24 Thread Xie He
On Sat, Jan 23, 2021 at 8:45 PM Jakub Kicinski wrote: > > > > @@ -178,11 +182,23 @@ int lapb_unregister(struct net_device *dev) > > > goto out; > > > lapb_put(lapb); > > > > > > + /* Wait for other refs to "lapb" to drop */ > > > + while (refcount_read(&lapb->refcnt) > 2) > > >

Re: [PATCH net v5] net: lapb: Add locking to the lapb module

2021-01-22 Thread Xie He
On Fri, Jan 22, 2021 at 1:07 AM Martin Schiller wrote: > > I don't have the opportunity to test this at the moment, but code looks > reasonable so far. Have you tested this at runtime? Thanks! Yes, I have tested this using hdlc_x25.c, lapbether.c and (the deleted) x25_asy.c drivers.

Re: [PATCH net v4] net: lapb: Add locking to the lapb module

2021-01-20 Thread Xie He
On Wed, Jan 20, 2021 at 12:42 PM Xie He wrote: > > With this patch, there is still a problem that lapb_unregister may run > concurrently with other LAPB API functions (such as > lapb_data_received). This other LAPB API function can get the > lapb->lock after lapb-&g

[PATCH net v5] net: lapb: Add locking to the lapb module

2021-01-20 Thread Xie He
3. Let lapb_unregister wait for other API functions and running timers to stop. 4. The lapb_device_event function calls lapb_disconnect_request. In order to avoid trying to hold the lock twice, add a new function named "__lapb_disconnect_request" which assumes the lock is held, and make it

Re: [PATCH net v4] net: lapb: Add locking to the lapb module

2021-01-20 Thread Xie He
With this patch, there is still a problem that lapb_unregister may run concurrently with other LAPB API functions (such as lapb_data_received). This other LAPB API function can get the lapb->lock after lapb->lock is released by lapb_unregister, and continue to do its work. This is not correct. We

Re: [PATCH net v4] net: lapb: Add locking to the lapb module

2021-01-20 Thread Xie He
On Wed, Jan 20, 2021 at 2:58 AM Martin Schiller wrote: > > Can you please add a Changelog. What was changed in v4? Sorry, I forgot this. Here is the change log: --- Changes from v3 to v4 --- Only lapb_unregister has been changed. v3 has a problem. When "del_timer_sync(&lapb->t1timer)" is calle

[PATCH net v4] net: lapb: Add locking to the lapb module

2021-01-20 Thread Xie He
2timer". 3. In lapb_unregister, add "del_timer_sync" calls to make sure all running timers have exited. 4. The lapb_device_event function calls lapb_disconnect_request. In order to avoid trying to hold the lock twice, add a new function named "__lapb_disconnect_request" which assumes t

Re: [PATCH net v2] net: lapb: Add locking to the lapb module

2021-01-19 Thread Xie He
On Tue, Jan 19, 2021 at 3:34 AM Martin Schiller wrote: > > > 4. In lapb_device_event, replace the "lapb_disconnect_request" call > > with > > the content of "lapb_disconnect_request", to avoid trying to hold the > > lock twice. When I do this, I removed "lapb_start_t1timer" because I > > don't thi

[PATCH net v3] net: lapb: Add locking to the lapb module

2021-01-19 Thread Xie He
named "__lapb_disconnect_request" which assumes the lock is held, and make it called by lapb_disconnect_request and lapb_device_event. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: Martin Schiller Signed-off-by: Xie He --- include/net/lapb.h| 2 ++ net/lapb/lapb_iface

[PATCH net v2] net: lapb: Add locking to the lapb module

2021-01-17 Thread Xie He
;, to avoid trying to hold the lock twice. When I do this, I removed "lapb_start_t1timer" because I don't think it's necessary to start the timer when "NETDEV_GOING_DOWN". Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: Martin Schiller Signed-off-by: Xie He --

[PATCH net] net: lapb: Add locking to the lapb module

2021-01-17 Thread Xie He
t", to avoid trying to hold the lock twice. When doing this, "lapb_start_t1timer" is removed because I don't think it's necessary to start the timer when "NETDEV_GOING_DOWN". Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: Martin Schiller Signed-off-by: Xie

Re: [PATCH] net: remove disc_data_lock in ppp line discipline

2020-12-31 Thread Xie He
> In tty layer, it use tty->ldisc_sem to proect tty_ldisc_ops. > So I think tty->ldisc_sem can also protect tty->disc_data; It might help by CC'ing TTY people, so that we could get this reviewed by people who are familiar with TTY code. Greg Kroah-Hartman (supporter:TTY LAYER) Jiri Slaby (suppo

[PATCH net] net: lapb: Decrease the refcount of "struct lapb_cb" in lapb_device_event

2020-12-31 Thread Xie He
es this problem. Fixes: a4989fa91110 ("net/lapb: support netdev events") Cc: Martin Schiller Signed-off-by: Xie He --- net/lapb/lapb_iface.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/lapb/lapb_iface.c b/net/lapb/lapb_iface.c index 213ea7abc9ab..40961889e9c0 100644 --- a/net/la

[PATCH net v2] net: hdlc_ppp: Fix issues when mod_timer is called while timer is running

2020-12-27 Thread Xie He
nning and it can just exit. If we let ppp_timer continue running, it may call add_timer. This causes kernel panic because add_timer can't be called with a timer pending. This patch fixes this problem. Fixes: e022c2f07ae5 ("WAN: new synchronous PPP implementation for generic HDLC."

[PATCH net] net: hdlc_ppp: Fix issues when mod_timer is called while timer is running

2020-12-27 Thread Xie He
nning and it can just exit. If we let ppp_timer continue running, it may call add_timer. This causes kernel panic because add_timer can't be called with a timer pending. This patch fixes this problem. Cc: Krzysztof Halasa Signed-off-by: Xie He --- drivers/net/wan/hdlc_ppp.c | 7 +++ 1

Re: [net,v2] net/packet: fix packet receive on L3 devices without visible hard header

2020-12-24 Thread Xie He
On Fri, Nov 20, 2020 at 10:28 PM Eyal Birger wrote: > > Fix by changing af_packet RX ll visibility criteria to include the > existence of a '.create()' header operation, which is used when creating > a device hard header - via dev_hard_header() - by upper layers, and does > not exist in these L3 d

Re: [PATCH AUTOSEL 5.4 075/130] net/lapb: fix t1 timer handling for LAPB_STATE_0

2020-12-24 Thread Xie He
On Wed, Dec 23, 2020 at 9:01 AM Xie He wrote: > > I don't think this patch is suitable for stable branches. This patch is > part of a patch series that changes the lapb module from "establishing the > L2 connection only when needed by L3", to "establishing t

Re: [PATCH AUTOSEL 5.4 075/130] net/lapb: fix t1 timer handling for LAPB_STATE_0

2020-12-23 Thread Xie He
> From: Martin Schiller > > [ Upstream commit 62480b992ba3fb1d7260b11293aed9d6557831c7 ] > > 1. DTE interface changes immediately to LAPB_STATE_1 and start sending >SABM(E). > > 2. DCE interface sends N2-times DM and changes to LAPB_STATE_1 >afterwards if there is no response in the meanti

Re: [PATCH net-next] net: x25: Remove unimplemented X.25-over-LLC code stubs

2020-12-10 Thread Xie He
On Thu, Dec 10, 2020 at 1:14 AM David Laight wrote: > > > To me, LLC1 and LLC2 are to Ethernet what UDP and TCP are to IP > > networks. I think we can use LLC1 and LLC2 wherever UDP and TCP can be > > used, as long as we are in the same LAN and are willing to use MAC > > addresses as the addresses

Re: [PATCH net-next] net: x25: Fix handling of Restart Request and Restart Confirmation

2020-12-10 Thread Xie He
On Wed, Dec 9, 2020 at 10:35 PM Martin Schiller wrote: > > Yes, that's also the reason why I already acked this patch. We can > solve this later a little bit cleaner if necessary. > > My patch that takes care of the orphaned packets in x25_receive_data() > has again a dependency on other patches,

Re: [PATCH net-next v7 4/5] net/x25: fix restart request/confirm handling

2020-12-10 Thread Xie He
On Wed, Dec 9, 2020 at 10:27 PM Martin Schiller wrote: > > > Hi Martin, > > > > When you submit future patch series, can you try ensuring the code to > > be in a completely working state after each patch in the series? This > > makes reviewing the patches easier. After the patches get applied, > >

Re: [PATCH net-next] net: x25: Remove unimplemented X.25-over-LLC code stubs

2020-12-09 Thread Xie He
On Wed, Dec 9, 2020 at 1:21 PM David Laight wrote: > > I always wondered about running Class 2 transport directly over LLC2 > (rather than Class 4 over LLC1). > But the only LLC2 user was netbios - and microsoft's LLC2 was broken. > Not to mention the window probing needed to handle systems that >

Re: [PATCH net-next v7 4/5] net/x25: fix restart request/confirm handling

2020-12-09 Thread Xie He
On Wed, Dec 9, 2020 at 1:47 AM Xie He wrote: > > On Wed, Dec 9, 2020 at 1:41 AM Martin Schiller wrote: > > > > Right. > > By the way: A "Restart Collision" is in practice a very common event to > > establish the Layer 3. > > Oh, I see. Thanks! H

Re: [PATCH net-next] net: x25: Fix handling of Restart Request and Restart Confirmation

2020-12-09 Thread Xie He
On Wed, Dec 9, 2020 at 2:31 AM Martin Schiller wrote: > > >> 1. When the x25 module gets loaded, layer 2 may already be running and > >> connected. In this case, although we are in X25_LINK_STATE_0, we still > >> need to handle the Restart Request received, rather than ignore it. > > > > Hmm... I'

Re: [PATCH net-next v7 4/5] net/x25: fix restart request/confirm handling

2020-12-09 Thread Xie He
On Wed, Dec 9, 2020 at 1:41 AM Martin Schiller wrote: > > Right. > By the way: A "Restart Collision" is in practice a very common event to > establish the Layer 3. Oh, I see. Thanks!

Re: [PATCH net-next v7 4/5] net/x25: fix restart request/confirm handling

2020-12-09 Thread Xie He
On Wed, Dec 9, 2020 at 1:01 AM Xie He wrote: > > On Wed, Nov 25, 2020 at 10:36 PM Martin Schiller wrote: > > > > switch (nb->state) { > > case X25_LINK_STATE_0: > > - nb->state = X25_LINK_STATE_2; > > -

Re: [PATCH net-next v7 4/5] net/x25: fix restart request/confirm handling

2020-12-09 Thread Xie He
On Wed, Nov 25, 2020 at 10:36 PM Martin Schiller wrote: > > We have to take the actual link state into account to handle > restart requests/confirms well. > > @@ -214,8 +241,6 @@ void x25_link_established(struct x25_neigh *nb) > { > switch (nb->state) { > case X25_LINK_STATE_0: >

[PATCH net-next] net: x25: Fix handling of Restart Request and Restart Confirmation

2020-12-09 Thread Xie He
Martin Schiller Signed-off-by: Xie He --- net/x25/x25_link.c | 25 + 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/net/x25/x25_link.c b/net/x25/x25_link.c index f92073f3cb11..57a81100c5da 100644 --- a/net/x25/x25_link.c +++ b/net/x25/x25_link.c @@ -58,11

[PATCH net-next] net: x25: Remove unimplemented X.25-over-LLC code stubs

2020-12-08 Thread Xie He
clear that this is not a ongoing plan anymore. Change words like "will" to "could", "would", etc. Cc: Martin Schiller Signed-off-by: Xie He --- Documentation/networking/x25.rst | 12 +--- net/x25/af_x25.c | 6 +- net/x25/x25_dev.c

[PATCH net-next v3] net: hdlc_x25: Remove unnecessary skb_reset_network_header calls

2020-12-08 Thread Xie He
, called by __netif_receive_skb, called by process_backlog). So we don't need to call skb_reset_network_header by ourselves. Cc: Martin Schiller Signed-off-by: Xie He --- drivers/net/wan/hdlc_x25.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/wan/hdlc_x25.c b/drivers/net/wan/hdlc_x25.c in

[PATCH net-next v2] net: hdlc_x25: Remove unnecessary skb_reset_network_header calls

2020-12-08 Thread Xie He
, called by __netif_receive_skb, called by process_backlog). So we don't need to call skb_reset_network_header by ourselves. Cc: Martin Schiller Signed-off-by: Xie He --- drivers/net/wan/hdlc_x25.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/wan/hdlc_x25.c b/drivers/net/wan/

[PATCH net-next] net: hdlc_x25: Remove unnecessary skb_reset_network_header calls

2020-12-08 Thread Xie He
, called by __netif_receive_skb, called by process_backlog). So we don't need to call skb_reset_network_header by ourselves. Cc: Martin Schiller Signed-off-by: Xie He --- drivers/net/wan/hdlc_x25.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/wan/hdlc_x25.c b/drivers/net/wan/

[PATCH net-next] net: lapbether: Consider it successful if (dis)connecting when already (dis)connected

2020-12-08 Thread Xie He
ned-off-by: Xie He --- drivers/net/wan/lapbether.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/drivers/net/wan/lapbether.c b/drivers/net/wan/lapbether.c index b6be2454b8bd..605fe555e157 100644 --- a/drivers/net/wan/lapbether.c +++ b/drivers/net/wan/lapbether.

Re: [PATCH net-next v7 2/5] net/lapb: support netdev events

2020-11-26 Thread Xie He
Acked-by: Xie He

  1   2   3   4   >