* Re: [PATCHv2 net-next 2/2] selftests: add a selftest for directed broadcast forwarding
From: Xin Long @ 2018-07-03 11:36 UTC (permalink / raw)
To: David Ahern; +Cc: network dev, davem, Davide Caratti, Ido Schimmel
In-Reply-To: <fc462436-af77-2c95-ec79-6cc418ef126e@gmail.com>
On Mon, Jul 2, 2018 at 11:12 PM, David Ahern <dsahern@gmail.com> wrote:
> On 7/2/18 12:30 AM, Xin Long wrote:
>> +ping_ipv4()
>> +{
>> + sysctl_set net.ipv4.icmp_echo_ignore_broadcasts 0
>> + bc_forwarding_disable
>> + ping_test $h1 198.51.100.255
>> +
>> + iptables -A INPUT -i vrf-r1 -p icmp -j DROP
>> + bc_forwarding_restore
>> + bc_forwarding_enable
>> + ping_test $h1 198.51.100.255
>> +
>> + bc_forwarding_restore
>> + iptables -D INPUT -i vrf-r1 -p icmp -j DROP
>> + sysctl_restore net.ipv4.icmp_echo_ignore_broadcasts
>> +}
>
> Both tests fail for me:
> TEST: ping [FAIL]
> TEST: ping [FAIL]
I think 'ip vrf exec ...' is not working in your env, while
the testing is using "ip vrf exec vrf-h1 ping ..."
You can test it by:
# ip link add dev vrf-test type vrf table 1111
# ip vrf exec vrf-test ls
>
> Why the need for the iptables rule?
This iptables rule is to block the echo_request packet going to
route's local_in.
When bc_forwarding is NOT doing forwarding well but the packet
goes to the route's local_in, it will fail.
Without this rule, the 2nd ping will always succeed, we can't tell the
echo_reply is from route or h2.
Or you have a better way to test this?
>
> And, PAUSE_ON_FAIL is not working to take a look at why tests are
> failing. e.g.,
>
> PAUSE_ON_FAIL=yes ./router_broadcast.sh
>
> just continues on. Might be something with the infrastructure scripts.
Yes, in ./router_broadcast.sh, it loads lib.sh where it loads forwarding.config
where it has "PAUSE_ON_FAIL=no", which would override your
"PAUSE_ON_FAIL=yes".
^ permalink raw reply
* Re: Oops in sock_wfree
From: Christophe LEROY @ 2018-07-03 11:34 UTC (permalink / raw)
To: linuxppc-dev@lists.ozlabs.org, netdev, Eric Dumazet
In-Reply-To: <8929d3b5-5c07-b6a1-b3c8-8625ff1c79a8@c-s.fr>
Le 03/07/2018 à 10:51, Christophe LEROY a écrit :
> Hi,
>
> I was having strange unexpected memory corruption, therefore I activated
> DEBUG_PAGEALLOC and I now end up with the following Oops, which tends to
> make me think we have somewhere in the network code a use-after-free
> bug. I saw a few of such bugs have been fixed for IPv4 and IPv6. Maybe
> we have one remaining for Unix sockets ? How can I spot it off and fix it ?
>
> [ 39.645644] Unable to handle kernel paging request for data at
> address 0xc2235010
In fact, must be something else. This page has never been allocated.
In seems that skb->sk should be c6234fc0 and suddenly it has changed to
c2234fc0
How can I track that ?
Christophe
> [ 39.652860] Faulting instruction address: 0xc0334d5c
> [ 39.657783] Oops: Kernel access of bad area, sig: 11 [#1]
> [ 39.663085] BE PREEMPT DEBUG_PAGEALLOC CMPC885
> [ 39.667488] SAF3000 DIE NOTIFICATION
> [ 39.671050] CPU: 0 PID: 269 Comm: in:imuxsock Not tainted
> 4.14.52-00025-g5bada429cf #22
> [ 39.679633] task: c623e100 task.stack: c651e000
> [ 39.684106] NIP: c0334d5c LR: c043602c CTR: c0435fb8
> [ 39.689103] REGS: c651fc00 TRAP: 0300 Not tainted
> (4.14.52-00025-g5bada429cf)
> [ 39.697087] MSR: 00009032 <EE,ME,IR,DR,RI> CR: 28002822 XER: 20000000
> [ 39.703720] DAR: c2235010 DSISR: c0000000
> [ 39.703720] GPR00: c043602c c651fcb0 c623e100 c619eec0 c642c540
> 00000008 00000018 c651fd4c
> [ 39.703720] GPR08: c0435fb8 000002b0 c068d830 00000004 28004822
> 100d4208 00000000 77990848
> [ 39.703720] GPR16: 0ff58398 778eb4b0 1039f050 1039f0a8 1005ddbc
> 0ff5a7bc 00000000 00000000
> [ 39.703720] GPR24: 00000072 c5011650 c651feb8 00000072 c619eec0
> 00000040 c2234fc0 c619eec0
> [ 39.741401] NIP [c0334d5c] sock_wfree+0x18/0xa4
> [ 39.745843] LR [c043602c] unix_destruct_scm+0x74/0x88
> [ 39.750786] Call Trace:
> [ 39.753253] [c651fcb0] [c006348c] ns_to_timeval+0x4c/0x7c (unreliable)
> [ 39.759690] [c651fcc0] [c043602c] unix_destruct_scm+0x74/0x88
> [ 39.765385] [c651fcf0] [c033a10c] skb_release_head_state+0x8c/0x110
> [ 39.771571] [c651fd00] [c033a3c4] skb_release_all+0x18/0x50
> [ 39.777078] [c651fd10] [c033a7cc] consume_skb+0x38/0xec
> [ 39.782255] [c651fd20] [c0342d7c] skb_free_datagram+0x1c/0x68
> [ 39.787922] [c651fd30] [c0435c8c] unix_dgram_recvmsg+0x19c/0x4ac
> [ 39.793863] [c651fdb0] [c0331370] ___sys_recvmsg+0x98/0x138
> [ 39.799371] [c651feb0] [c0333280] __sys_recvmsg+0x40/0x84
> [ 39.804707] [c651ff10] [c0333680] SyS_socketcall+0xb8/0x1d4
> [ 39.810220] [c651ff40] [c000d1ac] ret_from_syscall+0x0/0x38
> [ 39.815673] Instruction dump:
> [ 39.818612] 41beffac 4bffff58 38800003 4bffffa0 38800001 4bffff98
> 7c0802a6 9421fff0
> [ 39.826267] bfc10008 90010014 83c30010 812300a8 <815e0050> 3bfe00e0
> 71480200 4082003c
> [ 39.834113] ---[ end trace 8affde0490d7e25e ]---
>
> Thanks
> Christophe
^ permalink raw reply
* [PATCH v2] can: m_can: Move accessing of message ram to after clocks are enabled
From: Faiz Abbas @ 2018-07-03 11:17 UTC (permalink / raw)
To: linux-kernel, netdev, linux-can, linux-omap; +Cc: wg, mkl, davem, faiz_abbas
MCAN message ram should only be accessed once clocks are enabled.
Therefore, move the call to parse/init the message ram to after
clocks are enabled.
Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
---
Changes in v2:
rebased to latest mainline
drivers/net/can/m_can/m_can.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index ac4c6dc2f8c8..04c48371ab2a 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1644,8 +1644,6 @@ static int m_can_plat_probe(struct platform_device *pdev)
priv->can.clock.freq = clk_get_rate(cclk);
priv->mram_base = mram_addr;
- m_can_of_parse_mram(priv, mram_config_vals);
-
platform_set_drvdata(pdev, dev);
SET_NETDEV_DEV(dev, &pdev->dev);
@@ -1668,6 +1666,8 @@ static int m_can_plat_probe(struct platform_device *pdev)
goto clk_disable;
}
+ m_can_of_parse_mram(priv, mram_config_vals);
+
devm_can_led_init(dev);
of_can_transceiver(dev);
@@ -1715,8 +1715,6 @@ static __maybe_unused int m_can_resume(struct device *dev)
pinctrl_pm_select_default_state(dev);
- m_can_init_ram(priv);
-
priv->can.state = CAN_STATE_ERROR_ACTIVE;
if (netif_running(ndev)) {
@@ -1726,6 +1724,7 @@ static __maybe_unused int m_can_resume(struct device *dev)
if (ret)
return ret;
+ m_can_init_ram(priv);
m_can_start(ndev);
netif_device_attach(ndev);
netif_start_queue(ndev);
--
2.17.0
^ permalink raw reply related
* [PATCH v2] can: m_can: Fix runtime resume call
From: Faiz Abbas @ 2018-07-03 11:11 UTC (permalink / raw)
To: linux-kernel, netdev, linux-can, linux-omap; +Cc: wg, mkl, davem, faiz_abbas
pm_runtime_get_sync() returns a 1 if the state of the device is already
'active'. This is not a failure case and should return a success.
Therefore fix error handling for pm_runtime_get_sync() call such that
it returns success when the value is 1.
Also cleanup the TODO for using runtime PM for sleep mode as that is
implemented.
Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
---
Changes in v2:
rebased to latest mainline
drivers/net/can/m_can/m_can.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index b397a33f3d32..ac4c6dc2f8c8 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -634,10 +634,12 @@ static int m_can_clk_start(struct m_can_priv *priv)
int err;
err = pm_runtime_get_sync(priv->device);
- if (err)
+ if (err < 0) {
pm_runtime_put_noidle(priv->device);
+ return err;
+ }
- return err;
+ return 0;
}
static void m_can_clk_stop(struct m_can_priv *priv)
@@ -1687,8 +1689,6 @@ static int m_can_plat_probe(struct platform_device *pdev)
return ret;
}
-/* TODO: runtime PM with power down or sleep mode */
-
static __maybe_unused int m_can_suspend(struct device *dev)
{
struct net_device *ndev = dev_get_drvdata(dev);
--
2.17.0
^ permalink raw reply related
* pull-request: wireless-drivers 2018-07-03
From: Kalle Valo @ 2018-07-03 10:54 UTC (permalink / raw)
To: David Miller; +Cc: linux-wireless, netdev, linux-kernel
Hi Dave,
this is the first pull request to net tree for 4.18, more info below and
please let me know if you have any problems.
Kalle
The following changes since commit ce397d215ccd07b8ae3f71db689aedb85d56ab40:
Linux 4.18-rc1 (2018-06-17 08:04:49 +0900)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git tags/wireless-drivers-for-davem-2018-07-03
for you to fetch changes up to 4fa9433f950a3df0c180be37f14efe033198aa24:
Merge ath-current from git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git (2018-06-29 13:29:17 +0300)
----------------------------------------------------------------
wireless-drivers fixes for 4.18
First set of fixes for 4.18 and for numerous drivers. Something to mention
about is the wcn36xx fix which makes it possible to compile with gcc older than
4.4 (though I'm not sure if we even support those anymore).
qtnfmac
* coverity fix for a new commit in v4.18-rc1
rtlwifi
* fix kernel oops during driver removal
* fix firmware image corruption for rtl8821ae
brcmfmac
* fix crash if there's no firmware image
mwifiex
* a revert and a better fix for a new commit v4.18-rc1
mt7601u
* fix a recent regression about unnecessary warning about avg_rssi
wcn36xx
* convert testmode.c to plain ASCII
ath10k
* fix a firmware crash during bandwidth change
----------------------------------------------------------------
Ganapathi Bhat (2):
Revert "mwifiex: handle race during mwifiex_usb_disconnect"
mwifiex: handle race during mwifiex_usb_disconnect
Geert Uytterhoeven (1):
wcn36xx: Remove Unicode Byte Order Mark from testcode
Gustavo A. R. Silva (1):
qtnfmac: fix NULL pointer dereference
Kalle Valo (1):
Merge ath-current from git://git.kernel.org/.../kvalo/ath.git
Michael Trimarchi (1):
brcmfmac: stop watchdog before detach and free everything
Ping-Ke Shih (2):
rtlwifi: Fix kernel Oops "Fw download fail!!"
rtlwifi: rtl8821ae: fix firmware is not ready to run
Ryan Hsu (1):
ath10k: update the phymode along with bandwidth change request
Stanislaw Gruszka (1):
mt7601u: remove warning when avg_rssi is zero
Xinming Hu (1):
MAINTAINERS: update Xinming's email address
MAINTAINERS | 2 +-
drivers/net/wireless/ath/ath10k/mac.c | 16 ++++++++++++++--
drivers/net/wireless/ath/ath10k/wmi.h | 1 +
drivers/net/wireless/ath/wcn36xx/testmode.c | 2 +-
drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 7 +++++++
drivers/net/wireless/marvell/mwifiex/usb.c | 7 ++-----
drivers/net/wireless/mediatek/mt7601u/phy.c | 6 ++++--
drivers/net/wireless/quantenna/qtnfmac/cfg80211.c | 3 +--
drivers/net/wireless/realtek/rtlwifi/base.c | 17 ++++++++++-------
drivers/net/wireless/realtek/rtlwifi/base.h | 2 +-
drivers/net/wireless/realtek/rtlwifi/core.c | 3 +--
drivers/net/wireless/realtek/rtlwifi/pci.c | 2 +-
drivers/net/wireless/realtek/rtlwifi/ps.c | 4 ++--
drivers/net/wireless/realtek/rtlwifi/usb.c | 2 +-
14 files changed, 47 insertions(+), 27 deletions(-)
^ permalink raw reply
* Re: [PATCHv2 net-next 0/5] sctp: fully support for dscp and flowlabel per transport
From: Neil Horman @ 2018-07-03 10:50 UTC (permalink / raw)
To: Xin Long
Cc: network dev, linux-sctp, Marcelo Ricardo Leitner, davem,
hideaki.yoshifuji
In-Reply-To: <cover.1530526661.git.lucien.xin@gmail.com>
On Mon, Jul 02, 2018 at 06:21:10PM +0800, Xin Long wrote:
> Now dscp and flowlabel are set from sock when sending the packets,
> but being multi-homing, sctp also supports for dscp and flowlabel
> per transport, which is described in section 8.1.12 in RFC6458.
>
> v1->v2:
> - define ip_queue_xmit as inline in net/ip.h, instead of exporting
> it in Patch 1/5 according to David's suggestion.
> - fix the param len check in sctp_s/getsockopt_peer_addr_params()
> in Patch 3/5 to guarantee that an old app built with old kernel
> headers could work on the newer kernel per Marcelo's point.
>
> Xin Long (5):
> ipv4: add __ip_queue_xmit() that supports tos param
> sctp: add support for dscp and flowlabel per transport
> sctp: add spp_ipv6_flowlabel and spp_dscp for sctp_paddrparams
> sctp: add support for setting flowlabel when adding a transport
> sctp: check for ipv6_pinfo legal sndflow with flowlabel in
> sctp_v6_get_dst
>
> include/linux/sctp.h | 7 ++
> include/net/ip.h | 9 ++-
> include/net/sctp/structs.h | 9 +++
> include/uapi/linux/sctp.h | 4 +
> net/ipv4/ip_output.c | 9 ++-
> net/sctp/associola.c | 15 ++++
> net/sctp/ipv6.c | 20 ++++-
> net/sctp/protocol.c | 16 +++-
> net/sctp/socket.c | 182 +++++++++++++++++++++++++++++++++++++++++++--
> 9 files changed, 254 insertions(+), 17 deletions(-)
>
> --
> 2.1.0
>
>
For the Series
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply
* Re: [PATCH] selftests: bpf: config: add config fragments
From: Anders Roxell @ 2018-07-03 10:51 UTC (permalink / raw)
To: William Tu
Cc: Daniel Borkmann, Alexei Starovoitov, Shuah Khan, Networking,
Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK
In-Reply-To: <CALDO+Sap3GTW3Ud4u-950ZicXpqfCh3kV5x8FVFpPmH+snOsGg@mail.gmail.com>
On Tue, 26 Jun 2018 at 23:27, William Tu <u9012063@gmail.com> wrote:
>
> >> >
> >> > --- ::11 ping statistics ---
> >> > 5 packets transmitted, 3 packets received, 40% packet loss
> >> > round-trip min/avg/max = 0.139/1.857/5.293 ms
> >> > + ip netns exec at_ns0 ping -c 3 -w 10 -q 10.1.1.200
> >> > PING 10.1.1.200 (10.1.1.200): 56 data bytes
> >> >
> >> > --- 10.1.1.200 ping statistics ---
> >> > 3 packets transmitted, 3 packets received, 0% packet loss
> >> > round-trip min/avg/max = 0.214/0.256/0.305 ms
> >> > + ping -c 3 -w 10 -q 10.1.1.100
> >> > PING 10.1.1.100 (10.1.1.100): 56 data bytes
> >> >
> >> > --- 10.1.1.100 ping statistics ---
> >> > 3 packets transmitted, 3 packets received, 0% packet loss
> >> > round-trip min/avg/max = 0.210/0.211/0.213 ms
> >> > + check_err 0
> >> > + '[' 0 -eq 0 ']'
> >> > + ret=0
>
> So looks like the ipv4 over ipv6 gre passes.
>
> >> > + ip netns exec at_ns0 ping6 -c 3 -w 10 -q fc80::200
> >> > PING fc80::200 (fc80::200): 56 data bytes
> >> >
> >> > --- fc80::200 ping statistics ---
> >> > 10 packets transmitted, 0 packets received, 100% packet loss
> `
> but the ipv6 over ipv6 gre fails.
> Do you have any firewall rules that block this traffic?
> or if possible, the packet might get dropped at function ip6gre_xmit_ipv6
> can you print the return value of this function?
>
>
> >> > + check_err 1
> >> > + '[' 0 -eq 0 ']'
> >> > + ret=1
> >> > + ip -s link show ip6gretap11
> >> > 19: ip6gretap11@NONE: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1434 qdisc
> >> > pfifo_fast state UNKNOWN mode DEFAULT group default qlen 1000
> >> > link/ether de:d2:0c:53:80:8c brd ff:ff:ff:ff:ff:ff
> >> > RX: bytes packets errors dropped overrun mcast
> >> > 2096 25 0 0 0 0
> >> > TX: bytes packets errors dropped carrier collsns
> >> > 5324 36 5 5 0 0
> >>
> >> So there are 5 errors at TX.
> >
> > and today when I tried it on next-20180620 I saw 8 errors at TX.
> >
> >> I couldn't reproduce in my local machine using 4.17-rc6.
> >> How do I checkin the "next-20180613" source code?
> >
> > You can find the source code here [1], and I would look in the latest tag that I
> > said that I was able to reproduce it on above.
> >
> > Cheers,
> > Anders
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/
>
> Hi Anders,
Hi William,
>
> I'm still not able to reproduce the issue on next-20180620.
I'm not able to reproduce it either, and I tried on todays tag as well
next-20180703.
The only test that is failing in bpf/test_tunnel.sh:
FAIL: xfrm tunnel
Cheers,
Anders
> Below is my test.
>
> Testing IP6GRETAP tunnel...
> PING ::11(::11) 56 data bytes
>
> --- ::11 ping statistics ---
> 5 packets transmitted, 3 received, 40% packet loss, time 4048ms
> rtt min/avg/max/mdev = 0.040/32.118/96.235/45.337 ms
> PING 10.1.1.200 (10.1.1.200) 56(84) bytes of data.
>
> --- 10.1.1.200 ping statistics ---
> 3 packets transmitted, 3 received, 0% packet loss, time 2026ms
> rtt min/avg/max/mdev = 0.074/0.099/0.117/0.018 ms
> PING 10.1.1.100 (10.1.1.100) 56(84) bytes of data.
>
> --- 10.1.1.100 ping statistics ---
> 3 packets transmitted, 3 received, 0% packet loss, time 2054ms
> rtt min/avg/max/mdev = 0.069/0.113/0.187/0.052 ms
> PING fc80::200(fc80::200) 56 data bytes
>
> --- fc80::200 ping statistics ---
> 3 packets transmitted, 3 received, 0% packet loss, time 2054ms
> rtt min/avg/max/mdev = 0.069/0.104/0.142/0.031 ms
> PASS: ip6gretap
> root@osboxes:~/linux-next/tools/testing/selftests/bpf#
> root@osboxes:~/linux-next/tools/testing/selftests/bpf# uname -a
> Linux osboxes 4.18.0-rc1-next-20180620 #1 SMP Tue Jun 26 12:26:00 PDT
> 2018 x86_64 x86_64 x86_64 GNU/Linux
^ permalink raw reply
* Re: [PATCH v4 net-next 7/9] net: ipv4: listified version of ip_rcv
From: Pablo Neira Ayuso @ 2018-07-03 10:50 UTC (permalink / raw)
To: Edward Cree; +Cc: davem, netdev
In-Reply-To: <68230d9e-e93c-ef91-4b54-e7aebedf1f84@solarflare.com>
On Mon, Jul 02, 2018 at 04:14:12PM +0100, Edward Cree wrote:
> Also involved adding a way to run a netfilter hook over a list of packets.
> Rather than attempting to make netfilter know about lists (which would be
> a major project in itself) we just let it call the regular okfn (in this
> case ip_rcv_finish()) for any packets it steals, and have it give us back
> a list of packets it's synchronously accepted (which normally NF_HOOK
> would automatically call okfn() on, but we want to be able to potentially
> pass the list to a listified version of okfn().)
> The netfilter hooks themselves are indirect calls that still happen per-
> packet (see nf_hook_entry_hookfn()), but again, changing that can be left
> for future work.
>
> There is potential for out-of-order receives if the netfilter hook ends up
> synchronously stealing packets, as they will be processed before any
> accepts earlier in the list. However, it was already possible for an
> asynchronous accept to cause out-of-order receives, so presumably this is
> considered OK.
I think we can simplify things if these chained packets don't follow
the standard forwarding path, this would require to revisit many
subsystems to handle these new chained packets - potentially a lot of
work and likely breaking many things - and I would expect we (and
other subsystems too) will not get very much benefits from these
chained packets.
In general I like this infrastructure, but I think we can get
something simpler if we combine it with the flowtable idea, so chained
packets follow the non-standard flowtable forwarding path as described
in [1].
We could generalize and place the flowtable code in the core if
needed, and make it not netfilter dependent if that's a problem.
Thanks.
[1] https://marc.info/?l=netfilter-devel&m=152898601419841&w=2
^ permalink raw reply
* Re: [PATCH v2 net-next 4/4] vhost_net: Avoid rx vring kicks during busyloop
From: Jason Wang @ 2018-07-03 9:05 UTC (permalink / raw)
To: Toshiaki Makita, David S. Miller, Michael S. Tsirkin
Cc: netdev, kvm, virtualization
In-Reply-To: <1530603094-2476-5-git-send-email-makita.toshiaki@lab.ntt.co.jp>
On 2018年07月03日 15:31, Toshiaki Makita wrote:
> We may run out of avail rx ring descriptor under heavy load but busypoll
> did not detect it so busypoll may have exited prematurely. Avoid this by
> checking rx ring full during busypoll.
Actually, we're checking whether it was empty in fact?
>
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> ---
> drivers/vhost/net.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 791bc8b..b224036 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -658,6 +658,7 @@ static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk,
> {
> struct vhost_net_virtqueue *rnvq = &net->vqs[VHOST_NET_VQ_RX];
> struct vhost_net_virtqueue *tnvq = &net->vqs[VHOST_NET_VQ_TX];
> + struct vhost_virtqueue *rvq = &rnvq->vq;
> struct vhost_virtqueue *tvq = &tnvq->vq;
> unsigned long uninitialized_var(endtime);
> int len = peek_head_len(rnvq, sk);
> @@ -677,7 +678,8 @@ static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk,
> *busyloop_intr = true;
> break;
> }
> - if (sk_has_rx_data(sk) ||
> + if ((sk_has_rx_data(sk) &&
> + !vhost_vq_avail_empty(&net->dev, rvq)) ||
> !vhost_vq_avail_empty(&net->dev, tvq))
> break;
> cpu_relax();
> @@ -827,7 +829,6 @@ static void handle_rx(struct vhost_net *net)
>
I thought below codes should belong to patch 3. Or I may miss something.
Thanks
> while ((sock_len = vhost_net_rx_peek_head_len(net, sock->sk,
> &busyloop_intr))) {
> - busyloop_intr = false;
> sock_len += sock_hlen;
> vhost_len = sock_len + vhost_hlen;
> headcount = get_rx_bufs(vq, vq->heads + nvq->done_idx,
> @@ -838,7 +839,9 @@ static void handle_rx(struct vhost_net *net)
> goto out;
> /* OK, now we need to know about added descriptors. */
> if (!headcount) {
> - if (unlikely(vhost_enable_notify(&net->dev, vq))) {
> + if (unlikely(busyloop_intr)) {
> + vhost_poll_queue(&vq->poll);
> + } else if (unlikely(vhost_enable_notify(&net->dev, vq))) {
> /* They have slipped one in as we were
> * doing that: check again. */
> vhost_disable_notify(&net->dev, vq);
> @@ -848,6 +851,7 @@ static void handle_rx(struct vhost_net *net)
> * they refilled. */
> goto out;
> }
> + busyloop_intr = false;
> if (nvq->rx_ring)
> msg.msg_control = vhost_net_buf_consume(&nvq->rxq);
> /* On overrun, truncate and discard */
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v2 net-next 2/4] vhost_net: Avoid tx vring kicks during busyloop
From: Jason Wang @ 2018-07-03 9:04 UTC (permalink / raw)
To: Toshiaki Makita, David S. Miller, Michael S. Tsirkin
Cc: netdev, kvm, virtualization
In-Reply-To: <1530603094-2476-3-git-send-email-makita.toshiaki@lab.ntt.co.jp>
On 2018年07月03日 15:31, Toshiaki Makita wrote:
> Under heavy load vhost busypoll may run without suppressing
> notification. For example tx zerocopy callback can push tx work while
> handle_tx() is running, then busyloop exits due to vhost_has_work()
> condition and enables notification but immediately reenters handle_tx()
> because the pushed work was tx. In this case handle_tx() tries to
> disable notification again, but when using event_idx it by design
> cannot. Then busyloop will run without suppressing notification.
> Another example is the case where handle_tx() tries to enable
> notification but avail idx is advanced so disables it again. This case
> also leads to the same situation with event_idx.
>
> The problem is that once we enter this situation busyloop does not work
> under heavy load for considerable amount of time, because notification
> is likely to happen during busyloop and handle_tx() immediately enables
> notification after notification happens. Specifically busyloop detects
> notification by vhost_has_work() and then handle_tx() calls
> vhost_enable_notify(). Because the detected work was the tx work, it
> enters handle_tx(), and enters busyloop without suppression again.
> This is likely to be repeated, so with event_idx we are almost not able
> to suppress notification in this case.
>
> To fix this, poll the work instead of enabling notification when
> busypoll is interrupted by something. IMHO vhost_has_work() is kind of
> interruption rather than a signal to completely cancel the busypoll, so
> let's run busypoll after the necessary work is done.
>
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> ---
> drivers/vhost/net.c | 35 ++++++++++++++++++++++-------------
> 1 file changed, 22 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 3939c50..811c0e5 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -396,13 +396,10 @@ static inline unsigned long busy_clock(void)
> return local_clock() >> 10;
> }
>
> -static bool vhost_can_busy_poll(struct vhost_dev *dev,
> - unsigned long endtime)
> +static bool vhost_can_busy_poll(unsigned long endtime)
> {
> - return likely(!need_resched()) &&
> - likely(!time_after(busy_clock(), endtime)) &&
> - likely(!signal_pending(current)) &&
> - !vhost_has_work(dev);
> + return likely(!need_resched() && !time_after(busy_clock(), endtime) &&
> + !signal_pending(current));
> }
>
> static void vhost_net_disable_vq(struct vhost_net *n,
> @@ -434,7 +431,8 @@ static int vhost_net_enable_vq(struct vhost_net *n,
> static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
> struct vhost_virtqueue *vq,
> struct iovec iov[], unsigned int iov_size,
> - unsigned int *out_num, unsigned int *in_num)
> + unsigned int *out_num, unsigned int *in_num,
> + bool *busyloop_intr)
> {
> unsigned long uninitialized_var(endtime);
> int r = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
> @@ -443,9 +441,15 @@ static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
> if (r == vq->num && vq->busyloop_timeout) {
> preempt_disable();
> endtime = busy_clock() + vq->busyloop_timeout;
> - while (vhost_can_busy_poll(vq->dev, endtime) &&
> - vhost_vq_avail_empty(vq->dev, vq))
> + while (vhost_can_busy_poll(endtime)) {
> + if (vhost_has_work(vq->dev)) {
> + *busyloop_intr = true;
> + break;
> + }
> + if (!vhost_vq_avail_empty(vq->dev, vq))
> + break;
> cpu_relax();
> + }
> preempt_enable();
> r = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
> out_num, in_num, NULL, NULL);
> @@ -501,20 +505,24 @@ static void handle_tx(struct vhost_net *net)
> zcopy = nvq->ubufs;
>
> for (;;) {
> + bool busyloop_intr;
> +
> /* Release DMAs done buffers first */
> if (zcopy)
> vhost_zerocopy_signal_used(net, vq);
>
> -
> + busyloop_intr = false;
> head = vhost_net_tx_get_vq_desc(net, vq, vq->iov,
> ARRAY_SIZE(vq->iov),
> - &out, &in);
> + &out, &in, &busyloop_intr);
> /* On error, stop handling until the next kick. */
> if (unlikely(head < 0))
> break;
> /* Nothing new? Wait for eventfd to tell us they refilled. */
> if (head == vq->num) {
> - if (unlikely(vhost_enable_notify(&net->dev, vq))) {
> + if (unlikely(busyloop_intr)) {
> + vhost_poll_queue(&vq->poll);
> + } else if (unlikely(vhost_enable_notify(&net->dev, vq))) {
> vhost_disable_notify(&net->dev, vq);
> continue;
> }
> @@ -663,7 +671,8 @@ static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk)
> preempt_disable();
> endtime = busy_clock() + tvq->busyloop_timeout;
>
> - while (vhost_can_busy_poll(&net->dev, endtime) &&
> + while (vhost_can_busy_poll(endtime) &&
> + !vhost_has_work(&net->dev) &&
> !sk_has_rx_data(sk) &&
> vhost_vq_avail_empty(&net->dev, tvq))
> cpu_relax();
Acked-by: Jason Wang <jasowang@redhat.com>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v2 net-next 1/4] vhost_net: Rename local variables in vhost_net_rx_peek_head_len
From: Jason Wang @ 2018-07-03 9:03 UTC (permalink / raw)
To: Toshiaki Makita, David S. Miller, Michael S. Tsirkin
Cc: netdev, kvm, virtualization, Tonghao Zhang
In-Reply-To: <1530603094-2476-2-git-send-email-makita.toshiaki@lab.ntt.co.jp>
On 2018年07月03日 15:31, Toshiaki Makita wrote:
> So we can easily see which variable is for which, tx or rx.
>
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> ---
> drivers/vhost/net.c | 34 +++++++++++++++++-----------------
> 1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 29756d8..3939c50 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -647,39 +647,39 @@ static void vhost_rx_signal_used(struct vhost_net_virtqueue *nvq)
>
> static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk)
> {
> - struct vhost_net_virtqueue *rvq = &net->vqs[VHOST_NET_VQ_RX];
> - struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_TX];
> - struct vhost_virtqueue *vq = &nvq->vq;
> + struct vhost_net_virtqueue *rnvq = &net->vqs[VHOST_NET_VQ_RX];
> + struct vhost_net_virtqueue *tnvq = &net->vqs[VHOST_NET_VQ_TX];
> + struct vhost_virtqueue *tvq = &tnvq->vq;
> unsigned long uninitialized_var(endtime);
> - int len = peek_head_len(rvq, sk);
> + int len = peek_head_len(rnvq, sk);
>
> - if (!len && vq->busyloop_timeout) {
> + if (!len && tvq->busyloop_timeout) {
> /* Flush batched heads first */
> - vhost_rx_signal_used(rvq);
> + vhost_rx_signal_used(rnvq);
> /* Both tx vq and rx socket were polled here */
> - mutex_lock_nested(&vq->mutex, 1);
> - vhost_disable_notify(&net->dev, vq);
> + mutex_lock_nested(&tvq->mutex, 1);
> + vhost_disable_notify(&net->dev, tvq);
>
> preempt_disable();
> - endtime = busy_clock() + vq->busyloop_timeout;
> + endtime = busy_clock() + tvq->busyloop_timeout;
>
> while (vhost_can_busy_poll(&net->dev, endtime) &&
> !sk_has_rx_data(sk) &&
> - vhost_vq_avail_empty(&net->dev, vq))
> + vhost_vq_avail_empty(&net->dev, tvq))
> cpu_relax();
>
> preempt_enable();
>
> - if (!vhost_vq_avail_empty(&net->dev, vq))
> - vhost_poll_queue(&vq->poll);
> - else if (unlikely(vhost_enable_notify(&net->dev, vq))) {
> - vhost_disable_notify(&net->dev, vq);
> - vhost_poll_queue(&vq->poll);
> + if (!vhost_vq_avail_empty(&net->dev, tvq)) {
> + vhost_poll_queue(&tvq->poll);
> + } else if (unlikely(vhost_enable_notify(&net->dev, tvq))) {
> + vhost_disable_notify(&net->dev, tvq);
> + vhost_poll_queue(&tvq->poll);
> }
>
> - mutex_unlock(&vq->mutex);
> + mutex_unlock(&tvq->mutex);
>
> - len = peek_head_len(rvq, sk);
> + len = peek_head_len(rnvq, sk);
> }
>
> return len;
Acked-by: Jason Wang <jasowang@redhat.com>
^ permalink raw reply
* Oops in sock_wfree
From: Christophe LEROY @ 2018-07-03 8:51 UTC (permalink / raw)
To: linuxppc-dev@lists.ozlabs.org, netdev, Eric Dumazet
Hi,
I was having strange unexpected memory corruption, therefore I activated
DEBUG_PAGEALLOC and I now end up with the following Oops, which tends to
make me think we have somewhere in the network code a use-after-free
bug. I saw a few of such bugs have been fixed for IPv4 and IPv6. Maybe
we have one remaining for Unix sockets ? How can I spot it off and fix it ?
[ 39.645644] Unable to handle kernel paging request for data at
address 0xc2235010
[ 39.652860] Faulting instruction address: 0xc0334d5c
[ 39.657783] Oops: Kernel access of bad area, sig: 11 [#1]
[ 39.663085] BE PREEMPT DEBUG_PAGEALLOC CMPC885
[ 39.667488] SAF3000 DIE NOTIFICATION
[ 39.671050] CPU: 0 PID: 269 Comm: in:imuxsock Not tainted
4.14.52-00025-g5bada429cf #22
[ 39.679633] task: c623e100 task.stack: c651e000
[ 39.684106] NIP: c0334d5c LR: c043602c CTR: c0435fb8
[ 39.689103] REGS: c651fc00 TRAP: 0300 Not tainted
(4.14.52-00025-g5bada429cf)
[ 39.697087] MSR: 00009032 <EE,ME,IR,DR,RI> CR: 28002822 XER: 20000000
[ 39.703720] DAR: c2235010 DSISR: c0000000
[ 39.703720] GPR00: c043602c c651fcb0 c623e100 c619eec0 c642c540
00000008 00000018 c651fd4c
[ 39.703720] GPR08: c0435fb8 000002b0 c068d830 00000004 28004822
100d4208 00000000 77990848
[ 39.703720] GPR16: 0ff58398 778eb4b0 1039f050 1039f0a8 1005ddbc
0ff5a7bc 00000000 00000000
[ 39.703720] GPR24: 00000072 c5011650 c651feb8 00000072 c619eec0
00000040 c2234fc0 c619eec0
[ 39.741401] NIP [c0334d5c] sock_wfree+0x18/0xa4
[ 39.745843] LR [c043602c] unix_destruct_scm+0x74/0x88
[ 39.750786] Call Trace:
[ 39.753253] [c651fcb0] [c006348c] ns_to_timeval+0x4c/0x7c (unreliable)
[ 39.759690] [c651fcc0] [c043602c] unix_destruct_scm+0x74/0x88
[ 39.765385] [c651fcf0] [c033a10c] skb_release_head_state+0x8c/0x110
[ 39.771571] [c651fd00] [c033a3c4] skb_release_all+0x18/0x50
[ 39.777078] [c651fd10] [c033a7cc] consume_skb+0x38/0xec
[ 39.782255] [c651fd20] [c0342d7c] skb_free_datagram+0x1c/0x68
[ 39.787922] [c651fd30] [c0435c8c] unix_dgram_recvmsg+0x19c/0x4ac
[ 39.793863] [c651fdb0] [c0331370] ___sys_recvmsg+0x98/0x138
[ 39.799371] [c651feb0] [c0333280] __sys_recvmsg+0x40/0x84
[ 39.804707] [c651ff10] [c0333680] SyS_socketcall+0xb8/0x1d4
[ 39.810220] [c651ff40] [c000d1ac] ret_from_syscall+0x0/0x38
[ 39.815673] Instruction dump:
[ 39.818612] 41beffac 4bffff58 38800003 4bffffa0 38800001 4bffff98
7c0802a6 9421fff0
[ 39.826267] bfc10008 90010014 83c30010 812300a8 <815e0050> 3bfe00e0
71480200 4082003c
[ 39.834113] ---[ end trace 8affde0490d7e25e ]---
Thanks
Christophe
^ permalink raw reply
* Re: [PATCH net] r8169: fix mac address change
From: Corinna Vinschen @ 2018-07-03 8:47 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: David Miller, netdev@vger.kernel.org
In-Reply-To: <1770bc23-75f1-d462-b8a3-61e81ade4c3c@gmail.com>
On Jul 2 22:49, Heiner Kallweit wrote:
> Network core refuses to change mac address because flag
> IFF_LIVE_ADDR_CHANGE isn't set. Set this missing flag.
>
> Fixes: 1f7aa2bc268e ("r8169: simplify rtl_set_mac_address")
> Reported-by: Corinna Vinschen <vinschen@redhat.com>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> drivers/net/ethernet/realtek/r8169.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> index f80ac894..a390db27 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -7607,6 +7607,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> NETIF_F_HW_VLAN_CTAG_RX;
> dev->vlan_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_TSO |
> NETIF_F_HIGHDMA;
> + dev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
>
> tp->cp_cmd |= RxChkSum | RxVlan;
>
> --
> 2.18.0
Tested-by: Corinna Vinschen <vinschen@redhat.com>
This patch allows to change the MAC any time, just as before
1f7aa2bc268e, and thus also fixes the problem reported in
https://www.spinics.net/lists/netdev/msg510926.html
Thanks,
Corinna
^ permalink raw reply
* [PATCHv2 net] sctp: fix the issue that pathmtu may be set lower than MINSEGMENT
From: Xin Long @ 2018-07-03 8:30 UTC (permalink / raw)
To: network dev, linux-sctp
Cc: davem, Marcelo Ricardo Leitner, Neil Horman, syzkaller
After commit b6c5734db070 ("sctp: fix the handling of ICMP Frag Needed
for too small MTUs"), sctp_transport_update_pmtu would refetch pathmtu
from the dst and set it to transport's pathmtu without any check.
The new pathmtu may be lower than MINSEGMENT if the dst is obsolete and
updated by .get_dst() in sctp_transport_update_pmtu. In this case, it
could have a smaller MTU as well, and thus we should validate it
against MINSEGMENT instead.
Syzbot reported a warning in sctp_mtu_payload caused by this.
This patch refetches the pathmtu by calling sctp_dst_mtu where it does
the check against MINSEGMENT.
v1->v2:
- refetch the pathmtu by calling sctp_dst_mtu instead as Marcelo's
suggestion.
Fixes: b6c5734db070 ("sctp: fix the handling of ICMP Frag Needed for too small MTUs")
Reported-by: syzbot+f0d9d7cba052f9344b03@syzkaller.appspotmail.com
Suggested-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/sctp/transport.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index 445b7ef..12cac85 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -282,7 +282,7 @@ bool sctp_transport_update_pmtu(struct sctp_transport *t, u32 pmtu)
if (dst) {
/* Re-fetch, as under layers may have a higher minimum size */
- pmtu = SCTP_TRUNC4(dst_mtu(dst));
+ pmtu = sctp_dst_mtu(dst);
change = t->pathmtu != pmtu;
}
t->pathmtu = pmtu;
--
2.1.0
^ permalink raw reply related
* Re: [PATCH v4 net-next 0/9] Handle multiple received packets at each stage
From: Paolo Abeni @ 2018-07-03 7:51 UTC (permalink / raw)
To: David Ahern, Edward Cree, davem; +Cc: netdev
In-Reply-To: <d7092205-ff36-e982-e8fb-1a056df8b0bc@gmail.com>
On Mon, 2018-07-02 at 09:40 -0600, David Ahern wrote:
> On 7/2/18 9:11 AM, Edward Cree wrote:
> > This patch series adds the capability for the network stack to receive a
> > list of packets and process them as a unit, rather than handling each
> > packet singly in sequence. This is done by factoring out the existing
> > datapath code at each layer and wrapping it in list handling code.
> >
>
> ...
>
> > drivers/net/ethernet/sfc/efx.c | 12 +++
> > drivers/net/ethernet/sfc/net_driver.h | 3 +
> > drivers/net/ethernet/sfc/rx.c | 7 +-
> > include/linux/list.h | 30 ++++++
> > include/linux/netdevice.h | 4 +
> > include/linux/netfilter.h | 22 +++++
> > include/net/ip.h | 2 +
> > include/trace/events/net.h | 7 ++
> > net/core/dev.c | 174 ++++++++++++++++++++++++++++++++--
> > net/ipv4/af_inet.c | 1 +
> > net/ipv4/ip_input.c | 114 ++++++++++++++++++++--
> > 11 files changed, 360 insertions(+), 16 deletions(-)
> >
>
> Nice work. Have you looked at IPv6 support yet?
I think this work opens opportunities for a lot of follow-ups, if there
is agreement on extending this approach to other areas. Onother item
I'd like to investigate is TC processing.
Cheers,
Paolo
^ permalink raw reply
* Re: [PATCH 1/5] m68k/io: Add missing ioremap define guards, fix typo
From: Geert Uytterhoeven @ 2018-07-03 7:41 UTC (permalink / raw)
To: Greg Ungerer
Cc: David S. Miller, Dmitry Torokhov, Helge Deller, linux-m68k,
netdev, linux-input, Linux Kernel Mailing List
In-Reply-To: <1349f685-e4a2-5e04-8227-dd55e11153f6@linux-m68k.org>
Hi Greg,
On Tue, Jul 3, 2018 at 4:13 AM Greg Ungerer <gerg@linux-m68k.org> wrote:
> On 02/07/18 23:35, Geert Uytterhoeven wrote:
> > - Add missing define guard for ioremap_wt(),
> > - Fix typo s/ioremap_fillcache/ioremap_fullcache/,
> > - Add define guard for iounmap() for consistency with other
> > architectures.
> >
> > Fixes: 9746882f547d2f00 ("m68k: group io mapping definitions and functions")
> > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>
> If I build for the m5475evb defconfig then I get warnings like this:
>
> CC init/main.o
> In file included from ./include/asm-generic/io.h:19:0,
> from ./arch/m68k/include/asm/io_no.h:147,
> from ./arch/m68k/include/asm/io.h:3,
> from ./include/linux/bio.h:28,
> from ./include/linux/blkdev.h:21,
> from init/main.c:80:
> ./include/asm-generic/iomap.h:79:0: warning: "ioremap_wt" redefined
> #define ioremap_wt ioremap_nocache
> ^
> In file included from ./arch/m68k/include/asm/io_no.h:145:0,
> from ./arch/m68k/include/asm/io.h:3,
> from ./include/linux/bio.h:28,
> from ./include/linux/blkdev.h:21,
> from init/main.c:80:
> ./arch/m68k/include/asm/kmap.h:37:0: note: this is the location of the previous definition
> #define ioremap_wt ioremap_wt
> ^
Thanks, I did some m68knommu compile tests, but didn't see this warning, which
happens for Coldfire+MMU only.
The issue is that there are two ways to declare an architecture has
ioremap_wt():
1. By defining ioremap_wt,
2. By defining ARCH_HAS_IOREMAP_WT.
1 is done in arch/m68k/include/asm/kmap.h.
2 is done in arch/m68k/include/asm/io_mm.h. Moving that to kmap.h fixes
the warning. Will send an update shortly.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: linux-next: build failure after merge of the net-next tree
From: Sabrina Dubroca @ 2018-07-03 7:34 UTC (permalink / raw)
To: Stephen Rothwell
Cc: David Miller, Networking, Linux-Next Mailing List,
Linux Kernel Mailing List
In-Reply-To: <20180703104712.30a36f1a@canb.auug.org.au>
2018-07-03, 10:47:12 +1000, Stephen Rothwell wrote:
> Hi all,
>
> After merging the net-next tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
>
[...]
> I took a guess and aplied the following merge fix patch:
>
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Tue, 3 Jul 2018 10:37:05 +1000
> Subject: [PATCH] net: update for conversion of GRO SKB handling
>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
Thanks Stephen, this looks correct.
--
Sabrina
^ permalink raw reply
* [PATCH v2 net-next 4/4] vhost_net: Avoid rx vring kicks during busyloop
From: Toshiaki Makita @ 2018-07-03 7:31 UTC (permalink / raw)
To: David S. Miller, Michael S. Tsirkin, Jason Wang
Cc: Toshiaki Makita, netdev, kvm, virtualization, Tonghao Zhang
In-Reply-To: <1530603094-2476-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
We may run out of avail rx ring descriptor under heavy load but busypoll
did not detect it so busypoll may have exited prematurely. Avoid this by
checking rx ring full during busypoll.
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
drivers/vhost/net.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 791bc8b..b224036 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -658,6 +658,7 @@ static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk,
{
struct vhost_net_virtqueue *rnvq = &net->vqs[VHOST_NET_VQ_RX];
struct vhost_net_virtqueue *tnvq = &net->vqs[VHOST_NET_VQ_TX];
+ struct vhost_virtqueue *rvq = &rnvq->vq;
struct vhost_virtqueue *tvq = &tnvq->vq;
unsigned long uninitialized_var(endtime);
int len = peek_head_len(rnvq, sk);
@@ -677,7 +678,8 @@ static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk,
*busyloop_intr = true;
break;
}
- if (sk_has_rx_data(sk) ||
+ if ((sk_has_rx_data(sk) &&
+ !vhost_vq_avail_empty(&net->dev, rvq)) ||
!vhost_vq_avail_empty(&net->dev, tvq))
break;
cpu_relax();
@@ -827,7 +829,6 @@ static void handle_rx(struct vhost_net *net)
while ((sock_len = vhost_net_rx_peek_head_len(net, sock->sk,
&busyloop_intr))) {
- busyloop_intr = false;
sock_len += sock_hlen;
vhost_len = sock_len + vhost_hlen;
headcount = get_rx_bufs(vq, vq->heads + nvq->done_idx,
@@ -838,7 +839,9 @@ static void handle_rx(struct vhost_net *net)
goto out;
/* OK, now we need to know about added descriptors. */
if (!headcount) {
- if (unlikely(vhost_enable_notify(&net->dev, vq))) {
+ if (unlikely(busyloop_intr)) {
+ vhost_poll_queue(&vq->poll);
+ } else if (unlikely(vhost_enable_notify(&net->dev, vq))) {
/* They have slipped one in as we were
* doing that: check again. */
vhost_disable_notify(&net->dev, vq);
@@ -848,6 +851,7 @@ static void handle_rx(struct vhost_net *net)
* they refilled. */
goto out;
}
+ busyloop_intr = false;
if (nvq->rx_ring)
msg.msg_control = vhost_net_buf_consume(&nvq->rxq);
/* On overrun, truncate and discard */
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 net-next 3/4] vhost_net: Avoid rx queue wake-ups during busypoll
From: Toshiaki Makita @ 2018-07-03 7:31 UTC (permalink / raw)
To: David S. Miller, Michael S. Tsirkin, Jason Wang
Cc: Toshiaki Makita, netdev, kvm, virtualization, Tonghao Zhang
In-Reply-To: <1530603094-2476-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
We may run handle_rx() while rx work is queued. For example a packet can
push the rx work during the window before handle_rx calls
vhost_net_disable_vq().
In that case busypoll immediately exits due to vhost_has_work()
condition and enables vq again. This can lead to another unnecessary rx
wake-ups, so poll rx work instead of enabling the vq.
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
drivers/vhost/net.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 811c0e5..791bc8b 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -653,7 +653,8 @@ static void vhost_rx_signal_used(struct vhost_net_virtqueue *nvq)
nvq->done_idx = 0;
}
-static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk)
+static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk,
+ bool *busyloop_intr)
{
struct vhost_net_virtqueue *rnvq = &net->vqs[VHOST_NET_VQ_RX];
struct vhost_net_virtqueue *tnvq = &net->vqs[VHOST_NET_VQ_TX];
@@ -671,11 +672,16 @@ static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk)
preempt_disable();
endtime = busy_clock() + tvq->busyloop_timeout;
- while (vhost_can_busy_poll(endtime) &&
- !vhost_has_work(&net->dev) &&
- !sk_has_rx_data(sk) &&
- vhost_vq_avail_empty(&net->dev, tvq))
+ while (vhost_can_busy_poll(endtime)) {
+ if (vhost_has_work(&net->dev)) {
+ *busyloop_intr = true;
+ break;
+ }
+ if (sk_has_rx_data(sk) ||
+ !vhost_vq_avail_empty(&net->dev, tvq))
+ break;
cpu_relax();
+ }
preempt_enable();
@@ -795,6 +801,7 @@ static void handle_rx(struct vhost_net *net)
s16 headcount;
size_t vhost_hlen, sock_hlen;
size_t vhost_len, sock_len;
+ bool busyloop_intr = false;
struct socket *sock;
struct iov_iter fixup;
__virtio16 num_buffers;
@@ -818,7 +825,9 @@ static void handle_rx(struct vhost_net *net)
vq->log : NULL;
mergeable = vhost_has_feature(vq, VIRTIO_NET_F_MRG_RXBUF);
- while ((sock_len = vhost_net_rx_peek_head_len(net, sock->sk))) {
+ while ((sock_len = vhost_net_rx_peek_head_len(net, sock->sk,
+ &busyloop_intr))) {
+ busyloop_intr = false;
sock_len += sock_hlen;
vhost_len = sock_len + vhost_hlen;
headcount = get_rx_bufs(vq, vq->heads + nvq->done_idx,
@@ -905,7 +914,10 @@ static void handle_rx(struct vhost_net *net)
goto out;
}
}
- vhost_net_enable_vq(net, vq);
+ if (unlikely(busyloop_intr))
+ vhost_poll_queue(&vq->poll);
+ else
+ vhost_net_enable_vq(net, vq);
out:
vhost_rx_signal_used(nvq);
mutex_unlock(&vq->mutex);
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 net-next 2/4] vhost_net: Avoid tx vring kicks during busyloop
From: Toshiaki Makita @ 2018-07-03 7:31 UTC (permalink / raw)
To: David S. Miller, Michael S. Tsirkin, Jason Wang
Cc: netdev, virtualization, kvm
In-Reply-To: <1530603094-2476-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
Under heavy load vhost busypoll may run without suppressing
notification. For example tx zerocopy callback can push tx work while
handle_tx() is running, then busyloop exits due to vhost_has_work()
condition and enables notification but immediately reenters handle_tx()
because the pushed work was tx. In this case handle_tx() tries to
disable notification again, but when using event_idx it by design
cannot. Then busyloop will run without suppressing notification.
Another example is the case where handle_tx() tries to enable
notification but avail idx is advanced so disables it again. This case
also leads to the same situation with event_idx.
The problem is that once we enter this situation busyloop does not work
under heavy load for considerable amount of time, because notification
is likely to happen during busyloop and handle_tx() immediately enables
notification after notification happens. Specifically busyloop detects
notification by vhost_has_work() and then handle_tx() calls
vhost_enable_notify(). Because the detected work was the tx work, it
enters handle_tx(), and enters busyloop without suppression again.
This is likely to be repeated, so with event_idx we are almost not able
to suppress notification in this case.
To fix this, poll the work instead of enabling notification when
busypoll is interrupted by something. IMHO vhost_has_work() is kind of
interruption rather than a signal to completely cancel the busypoll, so
let's run busypoll after the necessary work is done.
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
drivers/vhost/net.c | 35 ++++++++++++++++++++++-------------
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 3939c50..811c0e5 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -396,13 +396,10 @@ static inline unsigned long busy_clock(void)
return local_clock() >> 10;
}
-static bool vhost_can_busy_poll(struct vhost_dev *dev,
- unsigned long endtime)
+static bool vhost_can_busy_poll(unsigned long endtime)
{
- return likely(!need_resched()) &&
- likely(!time_after(busy_clock(), endtime)) &&
- likely(!signal_pending(current)) &&
- !vhost_has_work(dev);
+ return likely(!need_resched() && !time_after(busy_clock(), endtime) &&
+ !signal_pending(current));
}
static void vhost_net_disable_vq(struct vhost_net *n,
@@ -434,7 +431,8 @@ static int vhost_net_enable_vq(struct vhost_net *n,
static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
struct vhost_virtqueue *vq,
struct iovec iov[], unsigned int iov_size,
- unsigned int *out_num, unsigned int *in_num)
+ unsigned int *out_num, unsigned int *in_num,
+ bool *busyloop_intr)
{
unsigned long uninitialized_var(endtime);
int r = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
@@ -443,9 +441,15 @@ static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
if (r == vq->num && vq->busyloop_timeout) {
preempt_disable();
endtime = busy_clock() + vq->busyloop_timeout;
- while (vhost_can_busy_poll(vq->dev, endtime) &&
- vhost_vq_avail_empty(vq->dev, vq))
+ while (vhost_can_busy_poll(endtime)) {
+ if (vhost_has_work(vq->dev)) {
+ *busyloop_intr = true;
+ break;
+ }
+ if (!vhost_vq_avail_empty(vq->dev, vq))
+ break;
cpu_relax();
+ }
preempt_enable();
r = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
out_num, in_num, NULL, NULL);
@@ -501,20 +505,24 @@ static void handle_tx(struct vhost_net *net)
zcopy = nvq->ubufs;
for (;;) {
+ bool busyloop_intr;
+
/* Release DMAs done buffers first */
if (zcopy)
vhost_zerocopy_signal_used(net, vq);
-
+ busyloop_intr = false;
head = vhost_net_tx_get_vq_desc(net, vq, vq->iov,
ARRAY_SIZE(vq->iov),
- &out, &in);
+ &out, &in, &busyloop_intr);
/* On error, stop handling until the next kick. */
if (unlikely(head < 0))
break;
/* Nothing new? Wait for eventfd to tell us they refilled. */
if (head == vq->num) {
- if (unlikely(vhost_enable_notify(&net->dev, vq))) {
+ if (unlikely(busyloop_intr)) {
+ vhost_poll_queue(&vq->poll);
+ } else if (unlikely(vhost_enable_notify(&net->dev, vq))) {
vhost_disable_notify(&net->dev, vq);
continue;
}
@@ -663,7 +671,8 @@ static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk)
preempt_disable();
endtime = busy_clock() + tvq->busyloop_timeout;
- while (vhost_can_busy_poll(&net->dev, endtime) &&
+ while (vhost_can_busy_poll(endtime) &&
+ !vhost_has_work(&net->dev) &&
!sk_has_rx_data(sk) &&
vhost_vq_avail_empty(&net->dev, tvq))
cpu_relax();
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 net-next 1/4] vhost_net: Rename local variables in vhost_net_rx_peek_head_len
From: Toshiaki Makita @ 2018-07-03 7:31 UTC (permalink / raw)
To: David S. Miller, Michael S. Tsirkin, Jason Wang
Cc: netdev, virtualization, kvm
In-Reply-To: <1530603094-2476-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
So we can easily see which variable is for which, tx or rx.
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
drivers/vhost/net.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 29756d8..3939c50 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -647,39 +647,39 @@ static void vhost_rx_signal_used(struct vhost_net_virtqueue *nvq)
static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk)
{
- struct vhost_net_virtqueue *rvq = &net->vqs[VHOST_NET_VQ_RX];
- struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_TX];
- struct vhost_virtqueue *vq = &nvq->vq;
+ struct vhost_net_virtqueue *rnvq = &net->vqs[VHOST_NET_VQ_RX];
+ struct vhost_net_virtqueue *tnvq = &net->vqs[VHOST_NET_VQ_TX];
+ struct vhost_virtqueue *tvq = &tnvq->vq;
unsigned long uninitialized_var(endtime);
- int len = peek_head_len(rvq, sk);
+ int len = peek_head_len(rnvq, sk);
- if (!len && vq->busyloop_timeout) {
+ if (!len && tvq->busyloop_timeout) {
/* Flush batched heads first */
- vhost_rx_signal_used(rvq);
+ vhost_rx_signal_used(rnvq);
/* Both tx vq and rx socket were polled here */
- mutex_lock_nested(&vq->mutex, 1);
- vhost_disable_notify(&net->dev, vq);
+ mutex_lock_nested(&tvq->mutex, 1);
+ vhost_disable_notify(&net->dev, tvq);
preempt_disable();
- endtime = busy_clock() + vq->busyloop_timeout;
+ endtime = busy_clock() + tvq->busyloop_timeout;
while (vhost_can_busy_poll(&net->dev, endtime) &&
!sk_has_rx_data(sk) &&
- vhost_vq_avail_empty(&net->dev, vq))
+ vhost_vq_avail_empty(&net->dev, tvq))
cpu_relax();
preempt_enable();
- if (!vhost_vq_avail_empty(&net->dev, vq))
- vhost_poll_queue(&vq->poll);
- else if (unlikely(vhost_enable_notify(&net->dev, vq))) {
- vhost_disable_notify(&net->dev, vq);
- vhost_poll_queue(&vq->poll);
+ if (!vhost_vq_avail_empty(&net->dev, tvq)) {
+ vhost_poll_queue(&tvq->poll);
+ } else if (unlikely(vhost_enable_notify(&net->dev, tvq))) {
+ vhost_disable_notify(&net->dev, tvq);
+ vhost_poll_queue(&tvq->poll);
}
- mutex_unlock(&vq->mutex);
+ mutex_unlock(&tvq->mutex);
- len = peek_head_len(rvq, sk);
+ len = peek_head_len(rnvq, sk);
}
return len;
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 net-next 0/4] vhost_net: Avoid vq kicks during busyloop
From: Toshiaki Makita @ 2018-07-03 7:31 UTC (permalink / raw)
To: David S. Miller, Michael S. Tsirkin, Jason Wang
Cc: netdev, virtualization, kvm
Under heavy load vhost tx busypoll tend not to suppress vq kicks, which
causes poor guest tx performance. The detailed scenario is described in
commitlog of patch 2.
Rx seems not to have that serious problem, but for consistency I made a
similar change on rx to avoid rx wakeups (patch 3).
Additionary patch 4 is to avoid rx kicks under heavy load during
busypoll.
Tx performance is greatly improved by this change. I don't see notable
performance change on rx with this series though.
Performance numbers (tx):
- Bulk transfer from guest to external physical server.
[Guest]->vhost_net->tap--(XDP_REDIRECT)-->i40e --(wire)--> [Server]
- Set 10us busypoll.
- Guest disables checksum and TSO because of host XDP.
- Measured single flow Mbps by netperf, and kicks by perf kvm stat
(EPT_MISCONFIG event).
Before After
Mbps kicks/s Mbps kicks/s
UDP_STREAM 1472byte 247758 27
Send 3645.37 6958.10
Recv 3588.56 6958.10
1byte 9865 37
Send 4.34 5.43
Recv 4.17 5.26
TCP_STREAM 8801.03 45794 9592.77 2884
v2:
- Split patches into 3 parts (renaming variables, tx-kick fix, rx-wakeup
fix).
- Avoid rx-kicks too (patch 4).
- Don't memorize endtime as it is not needed for now.
Toshiaki Makita (4):
vhost_net: Rename local variables in vhost_net_rx_peek_head_len
vhost_net: Avoid tx vring kicks during busyloop
vhost_net: Avoid rx queue wake-ups during busypoll
vhost_net: Avoid rx vring kicks during busyloop
drivers/vhost/net.c | 95 +++++++++++++++++++++++++++++++++--------------------
1 file changed, 60 insertions(+), 35 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [net-next,v1] tcp: Improve setsockopt() TCP_USER_TIMEOUT accuracy
From: Jon Maxwell @ 2018-07-03 7:21 UTC (permalink / raw)
To: davem; +Cc: ncardwell, edumazet, kuznet, yoshfuji, netdev, linux-kernel,
jmaxwell
v1 contains the following suggestions by Neal Cardwell:
1) Fix up units mismatch regarding msec/jiffies.
2) Address possiblility of time_remaining being negative.
3) Add a helper routine to do the rto calculation.
Every time the TCP retransmission timer fires. It checks to see if there is a
timeout before scheduling the next retransmit timer. The retransmit interval
between each retransmission increases exponentially. The issue is that in order
for the timeout to occur the retransmit timer needs to fire again. If the user
timeout check happens after the 9th retransmit for example. It needs to wait for
the 10th retransmit timer to fire in order to evaluate whether a timeout has
occurred or not. If the interval is large enough then the timeout will be
inaccurate.
For example with a TCP_USER_TIMEOUT of 10 seconds without patch:
1st retransmit:
22:25:18.973488 IP host1.49310 > host2.search-agent: Flags [.]
Last retransmit:
22:25:26.205499 IP host1.49310 > host2.search-agent: Flags [.]
Timeout:
send: Connection timed out
Sun Jul 1 22:25:34 EDT 2018
We can see that last retransmit took ~7 seconds. Which pushed the total
timeout to ~15 seconds instead of the expected 10 seconds. This gets more
inaccurate the larger the TCP_USER_TIMEOUT value. As the interval increases.
Add tcp_clamp_rto_to_user_timeout() to determine if the user rto has expired.
Or whether the rto interval needs to be recalculated. Use the original interval
if user rto is not set.
Test results with the patch is the expected 10 second timeout:
1st retransmit:
01:37:59.022555 IP host1.49310 > host2.search-agent: Flags [.]
Last retransmit:
01:38:06.486558 IP host1.49310 > host2.search-agent: Flags [.]
Timeout:
send: Connection timed out
Mon Jul 2 01:38:09 EDT 2018
Signed-off-by: Jon Maxwell <jmaxwell37@gmail.com>
---
net/ipv4/tcp_timer.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 3b3611729928..82c2a3b3713c 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -22,6 +22,23 @@
#include <linux/gfp.h>
#include <net/tcp.h>
+static __u32 tcp_clamp_rto_to_user_timeout(struct sock *sk)
+{
+ struct inet_connection_sock *icsk = inet_csk(sk);
+ __u32 rto = icsk->icsk_rto;
+ __u32 elapsed, user_timeout;
+
+ if (!icsk->icsk_user_timeout)
+ return rto;
+ elapsed = tcp_time_stamp(tcp_sk(sk)) - tcp_sk(sk)->retrans_stamp;
+ user_timeout = jiffies_to_msecs(icsk->icsk_user_timeout);
+ if (elapsed >= user_timeout)
+ rto = 1; /* user timeout has passed; fire ASAP */
+ else
+ rto = min(rto, (__u32)msecs_to_jiffies(user_timeout - elapsed));
+ return rto;
+}
+
/**
* tcp_write_err() - close socket and save error info
* @sk: The socket the error has appeared on.
@@ -407,6 +424,7 @@ void tcp_retransmit_timer(struct sock *sk)
struct tcp_sock *tp = tcp_sk(sk);
struct net *net = sock_net(sk);
struct inet_connection_sock *icsk = inet_csk(sk);
+ __u32 rto;
if (tp->fastopen_rsk) {
WARN_ON_ONCE(sk->sk_state != TCP_SYN_RECV &&
@@ -535,7 +553,8 @@ void tcp_retransmit_timer(struct sock *sk)
/* Use normal (exponential) backoff */
icsk->icsk_rto = min(icsk->icsk_rto << 1, TCP_RTO_MAX);
}
- inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS, icsk->icsk_rto, TCP_RTO_MAX);
+ rto = tcp_clamp_rto_to_user_timeout(sk);
+ inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS, rto, TCP_RTO_MAX);
if (retransmits_timed_out(sk, net->ipv4.sysctl_tcp_retries1 + 1, 0))
__sk_dst_reset(sk);
--
2.13.6
^ permalink raw reply related
* Re: [PATCH][net-next] net: increase MAX_GRO_SKBS to 64
From: Li RongQing @ 2018-07-03 6:21 UTC (permalink / raw)
To: David Miller; +Cc: lirongqing, netdev, eric.dumazet
In-Reply-To: <20180702.204428.903903223700362229.davem@davemloft.net>
On 7/2/18, David Miller <davem@davemloft.net> wrote:
> From: Li RongQing <lirongqing@baidu.com>
> Date: Mon, 2 Jul 2018 19:41:43 +0800
>
>> After 07d78363dcffd [net: Convert NAPI gro list into a small hash table]
>> there is 8 hash buckets, which allow more flows to be held for merging.
>>
>> keep each as original list length, so increase MAX_GRO_SKBS to 64
>>
>> Signed-off-by: Li RongQing <lirongqing@baidu.com>
>
> I would like to hear some feedback from Eric, 64 might be too big.
>
How about the below change?
commit 6270b973a973b2944fedb4b5f9926ed3e379d0c2 (HEAD -> master)
Author: Li RongQing <lirongqing@baidu.com>
Date: Mon Jul 2 19:08:37 2018 +0800
net: limit each hash list length to MAX_GRO_SKBS
After 07d78363dcffd [net: Convert NAPI gro list into a small hash table]
there is 8 hash buckets, which allows more flows to be held for merging.
but MAX_GRO_SKBS, the total held skb for merging, is 8 skb still, limit
the hash table performance.
keep MAX_GRO_SKBS as 8 skb, but limit each hash list length to 8 skb, not
the total 8 skb
Signed-off-by: Li RongQing <lirongqing@baidu.com>
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 8bf8d6149f79..09d7764a8917 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -324,6 +324,7 @@ struct napi_struct {
#endif
struct net_device *dev;
struct list_head gro_hash[GRO_HASH_BUCKETS];
+ int list_len[GRO_HASH_BUCKETS];
struct sk_buff *skb;
struct hrtimer timer;
struct list_head dev_list;
diff --git a/net/core/dev.c b/net/core/dev.c
index 08d58e0debe5..3cf3c6676cb3 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -149,7 +149,6 @@
#include "net-sysfs.h"
-/* Instead of increasing this, you should create a hash table. */
#define MAX_GRO_SKBS 8
/* This should be increased if a protocol with a bigger head is added. */
@@ -4989,10 +4988,11 @@ static int napi_gro_complete(struct sk_buff *skb)
return netif_receive_skb_internal(skb);
}
-static void __napi_gro_flush_chain(struct napi_struct *napi, struct
list_head *head,
+static void __napi_gro_flush_chain(struct napi_struct *napi, int index,
bool flush_old)
{
struct sk_buff *skb, *p;
+ struct list_head *head = &napi->gro_hash[index];
list_for_each_entry_safe_reverse(skb, p, head, list) {
if (flush_old && NAPI_GRO_CB(skb)->age == jiffies)
@@ -5000,6 +5000,7 @@ static void __napi_gro_flush_chain(struct
napi_struct *napi, struct list_head *h
list_del_init(&skb->list);
napi_gro_complete(skb);
napi->gro_count--;
+ napi->list_len[index]--;
}
}
@@ -5011,11 +5012,8 @@ void napi_gro_flush(struct napi_struct *napi,
bool flush_old)
{
int i;
- for (i = 0; i < GRO_HASH_BUCKETS; i++) {
- struct list_head *head = &napi->gro_hash[i];
-
- __napi_gro_flush_chain(napi, head, flush_old);
- }
+ for (i = 0; i < GRO_HASH_BUCKETS; i++)
+ __napi_gro_flush_chain(napi, i, flush_old);
}
EXPORT_SYMBOL(napi_gro_flush);
@@ -5095,27 +5093,13 @@ static void gro_pull_from_frag0(struct sk_buff
*skb, int grow)
}
}
-static void gro_flush_oldest(struct napi_struct *napi)
+static void gro_flush_oldest(struct list_head *head)
{
struct sk_buff *oldest = NULL;
- unsigned long age = jiffies;
- int i;
- for (i = 0; i < GRO_HASH_BUCKETS; i++) {
- struct list_head *head = &napi->gro_hash[i];
- struct sk_buff *skb;
+ oldest = list_last_entry(head, struct sk_buff, list);
- if (list_empty(head))
- continue;
-
- skb = list_last_entry(head, struct sk_buff, list);
- if (!oldest || time_before(NAPI_GRO_CB(skb)->age, age)) {
- oldest = skb;
- age = NAPI_GRO_CB(skb)->age;
- }
- }
-
- /* We are called with napi->gro_count >= MAX_GRO_SKBS, so this is
+ /* We are called with head length >= MAX_GRO_SKBS, so this is
* impossible.
*/
if (WARN_ON_ONCE(!oldest))
@@ -5138,6 +5122,7 @@ static enum gro_result dev_gro_receive(struct
napi_struct *napi, struct sk_buff
enum gro_result ret;
int same_flow;
int grow;
+ u32 hash = skb_get_hash_raw(skb) & (GRO_HASH_BUCKETS - 1);
if (netif_elide_gro(skb->dev))
goto normal;
@@ -5196,6 +5181,7 @@ static enum gro_result dev_gro_receive(struct
napi_struct *napi, struct sk_buff
list_del_init(&pp->list);
napi_gro_complete(pp);
napi->gro_count--;
+ napi->list_len[hash]--;
}
if (same_flow)
@@ -5204,10 +5190,11 @@ static enum gro_result dev_gro_receive(struct
napi_struct *napi, struct sk_buff
if (NAPI_GRO_CB(skb)->flush)
goto normal;
- if (unlikely(napi->gro_count >= MAX_GRO_SKBS)) {
- gro_flush_oldest(napi);
+ if (unlikely(napi->list_len[hash] >= MAX_GRO_SKBS)) {
+ gro_flush_oldest(gro_head);
} else {
napi->gro_count++;
+ napi->list_len[hash]++;
}
NAPI_GRO_CB(skb)->count = 1;
NAPI_GRO_CB(skb)->age = jiffies;
@@ -5844,8 +5831,10 @@ void netif_napi_add(struct net_device *dev,
struct napi_struct *napi,
hrtimer_init(&napi->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_PINNED);
napi->timer.function = napi_watchdog;
napi->gro_count = 0;
- for (i = 0; i < GRO_HASH_BUCKETS; i++)
+ for (i = 0; i < GRO_HASH_BUCKETS; i++) {
INIT_LIST_HEAD(&napi->gro_hash[i]);
+ napi->list_len[i] = 0;
+ }
napi->skb = NULL;
napi->poll = poll;
if (weight > NAPI_POLL_WEIGHT)
@@ -5887,6 +5876,7 @@ static void flush_gro_hash(struct napi_struct *napi)
list_for_each_entry_safe(skb, n, &napi->gro_hash[i], list)
kfree_skb(skb);
+ napi->list_len[i] = 0;
}
}
-R
^ permalink raw reply related
* Re: [PATCH net-next v4 3/4] net: vhost: factor out busy polling logic to vhost_net_busy_poll()
From: Jason Wang @ 2018-07-03 5:55 UTC (permalink / raw)
To: Tonghao Zhang
Cc: mst, makita.toshiaki, virtualization,
Linux Kernel Network Developers, Tonghao Zhang
In-Reply-To: <CAMDZJNU6bvACS9=RS1MA8T1z8+1fVkGZw=Xx8DWDjwQYWy0k8g@mail.gmail.com>
On 2018年07月03日 13:48, Tonghao Zhang wrote:
> On Tue, Jul 3, 2018 at 10:12 AM Jason Wang <jasowang@redhat.com> wrote:
>>
>>
>> On 2018年07月02日 20:57, xiangxia.m.yue@gmail.com wrote:
>>> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>>>
>>> Factor out generic busy polling logic and will be
>>> used for in tx path in the next patch. And with the patch,
>>> qemu can set differently the busyloop_timeout for rx queue.
>>>
>>> Signed-off-by: Tonghao Zhang <zhangtonghao@didichuxing.com>
>>> ---
>>> drivers/vhost/net.c | 94 +++++++++++++++++++++++++++++++----------------------
>>> 1 file changed, 55 insertions(+), 39 deletions(-)
>>>
>>> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
>>> index 62bb8e8..2790959 100644
>>> --- a/drivers/vhost/net.c
>>> +++ b/drivers/vhost/net.c
>>> @@ -429,6 +429,52 @@ static int vhost_net_enable_vq(struct vhost_net *n,
>>> return vhost_poll_start(poll, sock->file);
>>> }
>>>
>>> +static int sk_has_rx_data(struct sock *sk)
>>> +{
>>> + struct socket *sock = sk->sk_socket;
>>> +
>>> + if (sock->ops->peek_len)
>>> + return sock->ops->peek_len(sock);
>>> +
>>> + return skb_queue_empty(&sk->sk_receive_queue);
>>> +}
>>> +
>>> +static void vhost_net_busy_poll(struct vhost_net *net,
>>> + struct vhost_virtqueue *rvq,
>>> + struct vhost_virtqueue *tvq,
>>> + bool rx)
>>> +{
>>> + unsigned long uninitialized_var(endtime);
>>> + unsigned long busyloop_timeout;
>>> + struct socket *sock;
>>> + struct vhost_virtqueue *vq = rx ? tvq : rvq;
>>> +
>>> + mutex_lock_nested(&vq->mutex, rx ? VHOST_NET_VQ_TX: VHOST_NET_VQ_RX);
>>> +
>>> + vhost_disable_notify(&net->dev, vq);
>>> + sock = rvq->private_data;
>>> + busyloop_timeout = rx ? rvq->busyloop_timeout : tvq->busyloop_timeout;
>>> +
>>> + preempt_disable();
>>> + endtime = busy_clock() + busyloop_timeout;
>>> + while (vhost_can_busy_poll(tvq->dev, endtime) &&
>>> + !(sock && sk_has_rx_data(sock->sk)) &&
>>> + vhost_vq_avail_empty(tvq->dev, tvq))
>>> + cpu_relax();
>>> + preempt_enable();
>>> +
>>> + if ((rx && !vhost_vq_avail_empty(&net->dev, vq)) ||
>>> + (!rx && (sock && sk_has_rx_data(sock->sk)))) {
>>> + vhost_poll_queue(&vq->poll);
>>> + } else if (unlikely(vhost_enable_notify(&net->dev, vq))) {
>> One last question, do we need this for rx? This check will be always
>> true under light or medium load.
> will remove the 'unlikely'
Not only the unlikely. We only want rx kick when packet is pending at
socket but we're out of available buffers. This is not the case here
(you have polled the vq above).
So we probably want to do this only when rx == true.
Thanks
>
>> Thanks
>>
>>> + vhost_disable_notify(&net->dev, vq);
>>> + vhost_poll_queue(&vq->poll);
>>> + }
>>> +
>>> + mutex_unlock(&vq->mutex);
>>> +}
>>> +
>>> +
>>> static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
>>> struct vhost_virtqueue *vq,
>>> struct iovec iov[], unsigned int iov_size,
>>> @@ -621,16 +667,6 @@ static int peek_head_len(struct vhost_net_virtqueue *rvq, struct sock *sk)
>>> return len;
>>> }
>>>
>>> -static int sk_has_rx_data(struct sock *sk)
>>> -{
>>> - struct socket *sock = sk->sk_socket;
>>> -
>>> - if (sock->ops->peek_len)
>>> - return sock->ops->peek_len(sock);
>>> -
>>> - return skb_queue_empty(&sk->sk_receive_queue);
>>> -}
>>> -
>>> static void vhost_rx_signal_used(struct vhost_net_virtqueue *nvq)
>>> {
>>> struct vhost_virtqueue *vq = &nvq->vq;
>>> @@ -645,39 +681,19 @@ static void vhost_rx_signal_used(struct vhost_net_virtqueue *nvq)
>>>
>>> static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk)
>>> {
>>> - struct vhost_net_virtqueue *rvq = &net->vqs[VHOST_NET_VQ_RX];
>>> - struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_TX];
>>> - struct vhost_virtqueue *vq = &nvq->vq;
>>> - unsigned long uninitialized_var(endtime);
>>> - int len = peek_head_len(rvq, sk);
>>> + struct vhost_net_virtqueue *rnvq = &net->vqs[VHOST_NET_VQ_RX];
>>> + struct vhost_net_virtqueue *tnvq = &net->vqs[VHOST_NET_VQ_TX];
>>>
>>> - if (!len && vq->busyloop_timeout) {
>>> - /* Flush batched heads first */
>>> - vhost_rx_signal_used(rvq);
>>> - /* Both tx vq and rx socket were polled here */
>>> - mutex_lock_nested(&vq->mutex, VHOST_NET_VQ_TX);
>>> - vhost_disable_notify(&net->dev, vq);
>>> + int len = peek_head_len(rnvq, sk);
>>>
>>> - preempt_disable();
>>> - endtime = busy_clock() + vq->busyloop_timeout;
>>> -
>>> - while (vhost_can_busy_poll(&net->dev, endtime) &&
>>> - !sk_has_rx_data(sk) &&
>>> - vhost_vq_avail_empty(&net->dev, vq))
>>> - cpu_relax();
>>> -
>>> - preempt_enable();
>>> -
>>> - if (!vhost_vq_avail_empty(&net->dev, vq))
>>> - vhost_poll_queue(&vq->poll);
>>> - else if (unlikely(vhost_enable_notify(&net->dev, vq))) {
>>> - vhost_disable_notify(&net->dev, vq);
>>> - vhost_poll_queue(&vq->poll);
>>> - }
>>> + if (!len && rnvq->vq.busyloop_timeout) {
>>> + /* Flush batched heads first */
>>> + vhost_rx_signal_used(rnvq);
>>>
>>> - mutex_unlock(&vq->mutex);
>>> + /* Both tx vq and rx socket were polled here */
>>> + vhost_net_busy_poll(net, &rnvq->vq, &tnvq->vq, true);
>>>
>>> - len = peek_head_len(rvq, sk);
>>> + len = peek_head_len(rnvq, sk);
>>> }
>>>
>>> return len;
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox