linux-kernel - RE: [PATCH net-next] r8152: divide the tx and rx bottom functions

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: <0835B3720019904CB8F7AA43166CEEB2F18D470D@RTITMBSVM03.realtek.com.tw>
Date:   Fri, 16 Aug 2019 08:10:28 +0000
From:   Hayes Wang <hayeswang@...ltek.com>
To:     Eric Dumazet <eric.dumazet@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:     nic_swsd <nic_swsd@...ltek.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH net-next] r8152: divide the tx and rx bottom functions

Eric Dumazet [mailto:eric.dumazet@...il.com]
> Sent: Friday, August 16, 2019 2:40 PM
[...]
> tasklet and NAPI are scheduled on the same core (the current
> cpu calling napi_schedule() or tasklet_schedule())
> 
> I would rather not add this dubious tasklet, and instead try to understand
> what is wrong in this driver ;)
> 
> The various napi_schedule() calls are suspect IMO.

The original method as following.

static int r8152_poll(struct napi_struct *napi, int budget)
{
	struct r8152 *tp = container_of(napi, struct r8152, napi);
	int work_done;

	work_done = rx_bottom(tp, budget); <-- RX
	bottom_half(tp); <-- Tx (tx_bottom)
	[...]

The rx_bottom and tx_bottom would only be called in r8152_poll.
That is, tx_bottom wouldn't be run unless rx_bottom is finished.
And, rx_bottom would be called if tx_bottom is running.

If the traffic is busy. rx_bottom or tx_bottom may take a lot
of time to deal with the packets. And the one would increase
the latency time for the other one.

Therefore, when I separate the tx_bottom and rx_bottom to
different tasklet and napi, the callback functions of tx and
rx may schedule the tasklet and napi to different cpu. Then,
the rx_bottom and tx_bottom may be run at the same time.

Take our arm platform for example. There are five cpus to
handle the interrupt of USB host controller. When the rx is
completed, cpu #1 may handle the interrupt and napi would
be scheduled. When the tx is finished, cpu #2 may handle
the interrupt and the tasklet is scheduled. Then, napi is
run on cpu #1, and tasklet is run on cpu #2.

> Also rtl8152_start_xmit() uses skb_queue_tail(&tp->tx_queue, skb);
> 
> But I see nothing really kicking the transmit if tx_free is empty ?

Tx callback function "write_bulk_callback" would deal with it.
The callback function would check if there are packets waiting
to be sent.

Best Regards,
Hayes