* Re: [PATCH net 1/2] net: sctp: spare unnecessary comparison in sctp_trans_elect_best
From: Neil Horman @ 2014-08-22 12:13 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev, linux-sctp
In-Reply-To: <1408705410-28558-2-git-send-email-dborkman@redhat.com>
On Fri, Aug 22, 2014 at 01:03:29PM +0200, Daniel Borkmann wrote:
> When both transports are the same, we don't have to go down that
> road only to realize that we will return the very same transport.
> We are guaranteed that curr is always non-NULL. Therefore, just
> short-circuit this special case.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> ---
> net/sctp/associola.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index aaafb32..104fae4 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -1245,7 +1245,7 @@ static struct sctp_transport *sctp_trans_elect_best(struct sctp_transport *curr,
> {
> u8 score_curr, score_best;
>
> - if (best == NULL)
> + if (best == NULL || curr == best)
> return curr;
>
> score_curr = sctp_trans_score(curr);
Acked-by: Neil Horman <nhorman@tuxdriver.com>
> --
> 1.7.11.7
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [RFC 2/4] tuntap: Publish tuntap maximum number of queues as module_param
From: Pankaj Gupta @ 2014-08-22 11:52 UTC (permalink / raw)
To: Jason Wang
Cc: Michael S. Tsirkin, Jiri Pirko, linux-kernel, netdev, davem,
dgibson, vfalico, edumazet, vyasevic, hkchu, wuzhy, xemul,
therbert, bhutchings, xii, stephen
In-Reply-To: <53F575F8.7000208@redhat.com>
>
> On 08/20/2014 07:17 PM, Michael S. Tsirkin wrote:
> > On Wed, Aug 20, 2014 at 12:58:17PM +0200, Jiri Pirko wrote:
> >> > Mon, Aug 18, 2014 at 03:37:18PM CEST, pagupta@redhat.com wrote:
> >>> > > This patch publishes maximum number of tun/tap queues allocated as a
> >>> > > read_only module parameter which a user space application like
> >>> > > libvirt
> >>> > > can make use of to limit maximum number of queues. Value of read_only
> >>> > > module parameter can be writable only at module load time. If no
> >>> > > value is set
> >>> > > at module load time a default value 256 is used which is equal to
> >>> > > maximum number
> >>> > > of vCPUS allowed by KVM.
> >>> > >
> >>> > > Administrator can specify maximum number of queues only at the driver
> >>> > > module load time.
> >>> > >
> >>> > >Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> >>> > >---
> >>> > > drivers/net/tun.c | 13 +++++++++++--
> >>> > > 1 files changed, 11 insertions(+), 2 deletions(-)
> >>> > >
> >>> > >diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> >>> > >index acaaf67..1f518e2 100644
> >>> > >--- a/drivers/net/tun.c
> >>> > >+++ b/drivers/net/tun.c
> >>> > >@@ -119,6 +119,9 @@ struct tap_filter {
> >>> > >
> >>> > > #define TUN_FLOW_EXPIRE (3 * HZ)
> >>> > >
> >>> > >+static int max_tap_queues = MAX_TAP_QUEUES;
> >>> > >+module_param(max_tap_queues, int, S_IRUGO);
> >> >
> >> > Please do not introduce new module paramaters. Please other ways to
> >> > interchange values with userspace.
> > I suggested this initially, but thinking more about it, I agree.
> >
> > It's a global limit (necessary to limit memory utilization by
> > userspace), but it should be possible to change it
> > after module load.
>
> How about pass this limit through ifr during TUNSETIFF, then
> alloc_netdev_mq() can use this limit.
Any other ideas/comments from the experts. Or shall I re-repost other patches
in the series except this patch until we agree on one.
>
^ permalink raw reply
* RE: [PATCH] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()
From: Sharma, Sanjeev @ 2014-08-22 11:40 UTC (permalink / raw)
To: Johannes Berg
Cc: dsd@gentoo.org, kune@deine-taler.de, linville@tuxdriver.com,
linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1408707084.4349.0.camel@jlt4.sipsolutions.net>
Yes, it compiling successfully.
-----Original Message-----
From: Johannes Berg [mailto:johannes@sipsolutions.net]
Sent: Friday, August 22, 2014 5:01 PM
To: Sharma, Sanjeev
Cc: dsd@gentoo.org; kune@deine-taler.de; linville@tuxdriver.com; linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()
On Tue, 2014-08-19 at 06:39 +0000, Sharma, Sanjeev wrote:
> Ping for review the patch.
Make sure it compiles ...
^ permalink raw reply
* Re: [PATCH] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()
From: Johannes Berg @ 2014-08-22 11:31 UTC (permalink / raw)
To: Sharma, Sanjeev
Cc: dsd@gentoo.org, kune@deine-taler.de, linville@tuxdriver.com,
linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <FDC088D3B5555644AE135ED28A7ABDE94DF2FDC2@EU-MBX-02.mgc.mentorg.com>
On Tue, 2014-08-19 at 06:39 +0000, Sharma, Sanjeev wrote:
> Ping for review the patch.
Make sure it compiles ...
^ permalink raw reply
* [PATCH net 2/2] net: sctp: fix suboptimal edge-case on non-active active/retrans path selection
From: Daniel Borkmann @ 2014-08-22 11:03 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-sctp
In-Reply-To: <1408705410-28558-1-git-send-email-dborkman@redhat.com>
In SCTP, selection of active (T.ACT) and retransmission (T.RET)
transports is being done whenever transport control operations
(UP, DOWN, PF, ...) are engaged through sctp_assoc_control_transport().
Commits 4c47af4d5eb2 ("net: sctp: rework multihoming retransmission
path selection to rfc4960") and a7288c4dd509 ("net: sctp: improve
sctp_select_active_and_retran_path selection") have both improved
it towards a more fine-grained and optimal path selection.
Currently, the selection algorithm for T.ACT and T.RET is as follows:
1) Elect the two most recently used ACTIVE transports T1, T2 for
T.ACT, T.RET, where T.ACT<-T1 and T1 is most recently used
2) In case primary path T.PRI not in {T1, T2} but ACTIVE, set
T.ACT<-T.PRI and T.RET<-T1
3) If only T1 is ACTIVE from the set, set T.ACT<-T1 and T.RET<-T1
4) If none is ACTIVE, set T.ACT<-best(T.PRI, T.RET, T3) where
T3 is the most recently used (if avail) in PF, set T.RET<-T.PRI
Prior to above commits, 4) was simply a camp on T.ACT<-T.PRI and
T.RET<-T.PRI, ignoring possible paths in PF. Camping on T.PRI is
still slightly suboptimal as it can lead to the following scenario:
Setup:
<A> <B>
T1: p1p1 (10.0.10.10) <==> .'`) <==> p1p1 (10.0.10.12) <= T.PRI
T2: p1p2 (10.0.10.20) <==> (_ . ) <==> p1p2 (10.0.10.22)
net.sctp.rto_min = 1000
net.sctp.path_max_retrans = 2
net.sctp.pf_retrans = 0
net.sctp.hb_interval = 1000
T.PRI is permanently down, T2 is put briefly into PF state (e.g. due to
link flapping). Here, the first time transmission is sent over PF path
T2 as it's the only non-INACTIVE path, but the retransmitted data-chunks
are sent over the INACTIVE path T1 (T.PRI), which is not good.
After the patch, it's choosing better transports in both cases by
modifying step 4):
4) If none is ACTIVE, set T.ACT_new<-best(T.ACT_old, T3) where T3 is
the most recently used (if avail) in PF, set T.RET<-T.ACT_new
This will still select a best possible path in PF if available (which
can also include T.PRI/T.RET), and set both T.ACT/T.RET to it.
In case sctp_assoc_control_transport() *just* put T.ACT_old into INACTIVE
as it transitioned from ACTIVE->PF->INACTIVE and stays in INACTIVE just
for a very short while before going back ACTIVE, it will guarantee that
this path will be reselected for T.ACT/T.RET since T3 (PF) is not
available.
Previously, this was not possible, as we would only select between T.PRI
and T.RET, and a possible T3 would be NULL due to the fact that we have
just transitioned T3 in sctp_assoc_control_transport() from PF->INACTIVE
and would select a suboptimal path when T.PRI/T.RET have worse properties.
In the case that T.ACT_old permanently went to INACTIVE during this
transition and there's no PF path available, plus T.PRI and T.RET are
INACTIVE as well, we would now camp on T.ACT_old, but if everything is
being INACTIVE there's really not much we can do except hoping for a
successful HB to bring one of the transports back up again and, thus
cause a new selection through sctp_assoc_control_transport().
Now both tests work fine:
Case 1:
1. T1 S(ACTIVE) T.ACT
T2 S(ACTIVE) T.RET
2. T1 S(ACTIVE) T.ACT, T.RET
T2 S(PF)
3. T1 S(ACTIVE) T.ACT, T.RET
T2 S(INACTIVE)
5. T1 S(PF) T.ACT, T.RET
T2 S(INACTIVE)
[ 5.1 T1 S(INACTIVE) T.ACT, T.RET
T2 S(INACTIVE) ]
6. T1 S(ACTIVE) T.ACT, T.RET
T2 S(INACTIVE)
7. T1 S(ACTIVE) T.ACT
T2 S(ACTIVE) T.RET
Case 2:
1. T1 S(ACTIVE) T.ACT
T2 S(ACTIVE) T.RET
2. T1 S(PF)
T2 S(ACTIVE) T.ACT, T.RET
3. T1 S(INACTIVE)
T2 S(ACTIVE) T.ACT, T.RET
5. T1 S(INACTIVE)
T2 S(PF) T.ACT, T.RET
[ 5.1 T1 S(INACTIVE)
T2 S(INACTIVE) T.ACT, T.RET ]
6. T1 S(INACTIVE)
T2 S(ACTIVE) T.ACT, T.RET
7. T1 S(ACTIVE) T.ACT
T2 S(ACTIVE) T.RET
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
net/sctp/associola.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 104fae4..a88b852 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -1356,14 +1356,11 @@ static void sctp_select_active_and_retran_path(struct sctp_association *asoc)
trans_sec = trans_pri;
/* If we failed to find a usable transport, just camp on the
- * primary or retran, even if they are inactive, if possible
- * pick a PF iff it's the better choice.
+ * active or pick a PF iff it's the better choice.
*/
if (trans_pri == NULL) {
- trans_pri = sctp_trans_elect_best(asoc->peer.primary_path,
- asoc->peer.retran_path);
- trans_pri = sctp_trans_elect_best(trans_pri, trans_pf);
- trans_sec = asoc->peer.primary_path;
+ trans_pri = sctp_trans_elect_best(asoc->peer.active_path, trans_pf);
+ trans_sec = trans_pri;
}
/* Set the active and retran transports. */
--
1.7.11.7
^ permalink raw reply related
* [PATCH net 0/2] SCTP fix
From: Daniel Borkmann @ 2014-08-22 11:03 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-sctp
Daniel Borkmann (2):
net: sctp: spare unnecessary comparison in sctp_trans_elect_best
net: sctp: fix suboptimal edge-case on non-active active/retrans path selection
net/sctp/associola.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
--
1.7.11.7
^ permalink raw reply
* [PATCH net 1/2] net: sctp: spare unnecessary comparison in sctp_trans_elect_best
From: Daniel Borkmann @ 2014-08-22 11:03 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-sctp
In-Reply-To: <1408705410-28558-1-git-send-email-dborkman@redhat.com>
When both transports are the same, we don't have to go down that
road only to realize that we will return the very same transport.
We are guaranteed that curr is always non-NULL. Therefore, just
short-circuit this special case.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
net/sctp/associola.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index aaafb32..104fae4 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -1245,7 +1245,7 @@ static struct sctp_transport *sctp_trans_elect_best(struct sctp_transport *curr,
{
u8 score_curr, score_best;
- if (best == NULL)
+ if (best == NULL || curr == best)
return curr;
score_curr = sctp_trans_score(curr);
--
1.7.11.7
^ permalink raw reply related
* [PATCH] netdev_adjacent_sysfs_*: fix cross-namespace symlinks
From: Alexander Y. Fomichev @ 2014-08-22 10:34 UTC (permalink / raw)
To: netdev; +Cc: Alexander Y. Fomichev, vfalico
In-Reply-To: <20140821152940.GT9476@redhat.com>
fix code manipulating symlinks of adjacent net devices.
- prevent creation of sysfs symlink to / from adjacent device on
another net_ns.
- Drop all existing symlinks from / to all adj_dev(s) on the current
namespace before switching net_ns and recreate them just after.
---
net/core/dev.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 61 insertions(+), 1 deletion(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index b65a505..683cedf 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4889,7 +4889,8 @@ static void __netdev_adjacent_dev_remove(struct net_device *dev,
if (adj->master)
sysfs_remove_link(&(dev->dev.kobj), "master");
- if (netdev_adjacent_is_neigh_list(dev, dev_list))
+ if (netdev_adjacent_is_neigh_list(dev, dev_list) &&
+ net_eq(dev_net(dev),dev_net(adj_dev)))
netdev_adjacent_sysfs_del(dev, adj_dev->name, dev_list);
list_del_rcu(&adj->list);
@@ -5159,11 +5160,65 @@ void netdev_upper_dev_unlink(struct net_device *dev,
}
EXPORT_SYMBOL(netdev_upper_dev_unlink);
+void netdev_adjacent_add_links(struct net_device *dev)
+{
+ struct netdev_adjacent *iter;
+
+ struct net *net = dev_net(dev);
+
+ list_for_each_entry(iter, &dev->adj_list.upper, list) {
+ if (!net_eq(net,dev_net(iter->dev)))
+ continue;
+ netdev_adjacent_sysfs_add(iter->dev, dev,
+ &iter->dev->adj_list.lower);
+ netdev_adjacent_sysfs_add(dev, iter->dev,
+ &dev->adj_list.upper);
+ }
+
+ list_for_each_entry(iter, &dev->adj_list.lower, list) {
+ if (!net_eq(net,dev_net(iter->dev)))
+ continue;
+ netdev_adjacent_sysfs_add(iter->dev, dev,
+ &iter->dev->adj_list.upper);
+ netdev_adjacent_sysfs_add(dev, iter->dev,
+ &dev->adj_list.lower);
+ }
+}
+
+void netdev_adjacent_del_links(struct net_device *dev)
+{
+ struct netdev_adjacent *iter;
+
+ struct net *net = dev_net(dev);
+
+ list_for_each_entry(iter, &dev->adj_list.upper, list) {
+ if (!net_eq(net,dev_net(iter->dev)))
+ continue;
+ netdev_adjacent_sysfs_del(iter->dev, dev->name,
+ &iter->dev->adj_list.lower);
+ netdev_adjacent_sysfs_del(dev, iter->dev->name,
+ &dev->adj_list.upper);
+ }
+
+ list_for_each_entry(iter, &dev->adj_list.lower, list) {
+ if (!net_eq(net,dev_net(iter->dev)))
+ continue;
+ netdev_adjacent_sysfs_del(iter->dev, dev->name,
+ &iter->dev->adj_list.upper);
+ netdev_adjacent_sysfs_del(dev, iter->dev->name,
+ &dev->adj_list.lower);
+ }
+}
+
void netdev_adjacent_rename_links(struct net_device *dev, char *oldname)
{
struct netdev_adjacent *iter;
+ struct net *net = dev_net(dev);
+
list_for_each_entry(iter, &dev->adj_list.upper, list) {
+ if (!net_eq(net,dev_net(iter->dev)))
+ continue;
netdev_adjacent_sysfs_del(iter->dev, oldname,
&iter->dev->adj_list.lower);
netdev_adjacent_sysfs_add(iter->dev, dev,
@@ -5171,6 +5226,8 @@ void netdev_adjacent_rename_links(struct net_device *dev, char *oldname)
}
list_for_each_entry(iter, &dev->adj_list.lower, list) {
+ if (!net_eq(net,dev_net(iter->dev)))
+ continue;
netdev_adjacent_sysfs_del(iter->dev, oldname,
&iter->dev->adj_list.upper);
netdev_adjacent_sysfs_add(iter->dev, dev,
@@ -6771,8 +6828,10 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
dev_uc_flush(dev);
dev_mc_flush(dev);
+
/* Send a netdev-removed uevent to the old namespace */
kobject_uevent(&dev->dev.kobj, KOBJ_REMOVE);
+ netdev_adjacent_del_links(dev);
/* Actually switch the network namespace */
dev_net_set(dev, net);
@@ -6787,6 +6846,7 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
/* Send a netdev-add uevent to the new namespace */
kobject_uevent(&dev->dev.kobj, KOBJ_ADD);
+ netdev_adjacent_add_links(dev);
/* Fixup kobjects */
err = device_rename(&dev->dev, dev->name);
--
2.0.4
^ permalink raw reply related
* Re: team: Bug when macvlans are defined on top of the slaves
From: Jiri Pirko @ 2014-08-22 10:13 UTC (permalink / raw)
To: Erik Hugne; +Cc: netdev, libteam
In-Reply-To: <20140820160804.GG32293@eerihug-hybrid.rnd.ki.sw.ericsson.se>
Wed, Aug 20, 2014 at 06:08:04PM CEST, erik.hugne@ericsson.com wrote:
>When macvlans are defined on top of the team port devices, the following oops
>happens immediately when teamd is started.
>
>
>[ 108.224148] team0: Mode changed to "activebackup"
>[ 108.230450] e1000: eth1 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: None
>[ 108.234107] 8021q: adding VLAN 0 to HW filter on device eth1
>[ 108.235593] BUG: unable to handle kernel NULL pointer dereference at 0000000000000018
>[ 108.236397] IP: [<ffffffff815ddb7e>] __mutex_lock_slowpath+0x4e/0x310
>[ 108.236397] PGD 3fb2e067 PUD 3fb23067 PMD 0
>[ 108.236397] Oops: 0002 [#1] SMP
>[ 108.236397] Modules linked in: team_mode_activebackup team macvlan
>[ 108.236397] CPU: 0 PID: 201 Comm: teamd Not tainted 3.16.0+ #237
>[ 108.236397] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
>[ 108.236397] task: ffff88003f958ed0 ti: ffff88003d15c000 task.ti: ffff88003d15c000
>[ 108.236397] RIP: 0010:[<ffffffff815ddb7e>] [<ffffffff815ddb7e>] __mutex_lock_slowpath+0x4e/0x310
>[ 108.236397] RSP: 0018:ffff88003d15f698 EFLAGS: 00010046
>[ 108.236397] RAX: 0000000000000100 RBX: 0000000000000010 RCX: 0000000000000001
>[ 108.236397] RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000000000010
>[ 108.236397] RBP: ffff88003d15f6e8 R08: ffff88003f850900 R09: ffff88003e400008
>[ 108.236397] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000010
>[ 108.236397] R13: ffff88003f958ed0 R14: 0000000000000018 R15: 0000000000000246
>[ 108.236397] FS: 00007ff10a811740(0000) GS:ffff88003ec00000(0000) knlGS:0000000000000000
>[ 108.236397] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
>[ 108.236397] CR2: 0000000000000018 CR3: 000000003fa96000 CR4: 00000000000006f0
>[ 108.236397] Stack:
>[ 108.236397] ffff88003d3d6000 0000000000000081 ffff88003d3d6000 0000000000000001
>[ 108.251189] ffff88003d15f710 0000000000000010 0000000000000010 0000000000000001
>[ 108.251189] ffff88003d15f7d0 0000000000000000 ffff88003d15f700 ffffffff815dde55
>[ 108.251189] Call Trace:
>[ 108.251189] [<ffffffff815dde55>] mutex_lock+0x15/0x25
>[ 108.251189] [<ffffffffa000a0df>] team_port_change_check+0x1f/0x60 [team]
>[ 108.251189] [<ffffffffa000afc8>] team_device_event+0xa8/0x150 [team]
>[ 108.251189] [<ffffffff810631ec>] notifier_call_chain+0x4c/0x70
>[ 108.251189] [<ffffffff81063301>] raw_notifier_call_chain+0x11/0x20
>[ 108.251189] [<ffffffff814b5330>] call_netdevice_notifiers_info+0x30/0x60
>[ 108.251189] [<ffffffff814bcdac>] dev_open+0x5c/0x70
>[ 108.251189] [<ffffffffa000a582>] team_add_slave+0x2b2/0x6c0 [team]
I see the problem. team_port_add->team_port_enter sets IFF_TEAM_PORT before
netdev_rx_handler_register is called and therefore team_device_event
thinks that the rx_handler_data pointer is port, but it is macvlan
instead. I'll fix this.
But the thing is, since both macvlan and team are rx_handler users, you
cannot have them both on a single device. If the oops would not appear,
netdev_rx_handler_register would fail anyway. You have to create macvlan on
top of team device instead.
>[ 108.251189] [<ffffffff81564390>] ? inet6_fill_ifla6_attrs+0x360/0x390
>[ 108.251189] [<ffffffff814cb73e>] do_setlink+0x9be/0xa20
>[ 108.251189] [<ffffffff814c99f9>] ? rtnl_fill_ifinfo+0x899/0xb40
>[ 108.251189] [<ffffffff814cbdcf>] rtnl_newlink+0x4ff/0x730
>[ 108.251189] [<ffffffff814cb9d8>] ? rtnl_newlink+0x108/0x730
>[ 108.251189] [<ffffffff814e5e51>] ? netlink_sendskb+0x11/0x40
>[ 108.251189] [<ffffffff814e6033>] ? netlink_unicast+0x1b3/0x250
>[ 108.251189] [<ffffffff814ca8e0>] ? rtnl_getlink+0x130/0x1d0
>[ 108.251189] [<ffffffff814caa70>] rtnetlink_rcv_msg+0x90/0x250
>[ 108.251189] [<ffffffff814ca9e0>] ? rtnetlink_rcv+0x30/0x30
>[ 108.251189] [<ffffffff814e6739>] netlink_rcv_skb+0xa9/0xc0
>[ 108.251189] [<ffffffff814ca9d3>] rtnetlink_rcv+0x23/0x30
>[ 108.251189] [<ffffffff814e5fd8>] netlink_unicast+0x158/0x250
>[ 108.251189] [<ffffffff8128912d>] ? memcpy_fromiovec+0x4d/0x90
>[ 108.251189] [<ffffffff814e6487>] netlink_sendmsg+0x317/0x410
>[ 108.251189] [<ffffffff814a0490>] sock_sendmsg+0xa0/0xc0
>[ 108.251189] [<ffffffff814a1d68>] ? move_addr_to_kernel+0x38/0x80
>[ 108.251189] [<ffffffff814a1d23>] ___sys_sendmsg+0x373/0x380
>[ 108.251189] [<ffffffff8103d034>] ? __do_page_fault+0x284/0x520
>[ 108.251189] [<ffffffff814a2bbd>] __sys_sendmsg+0x3d/0x80
>[ 108.251189] [<ffffffff814a2c0d>] SyS_sendmsg+0xd/0x20
>[ 108.251189] [<ffffffff815dfb52>] system_call_fastpath+0x16/0x1b
>[ 108.251189] Code: 35 20 0d 60 00 45 85 f6 75 13 65 8b 04 25 20 b8 00 00 a9 00 ff 1f 00 0f 85 83 02 00 00 9c 41 5f fa b8 00 01 00 00 4d 8d 74 24 08 <f0> 66 41 0f c1 44 24 08 0f b6 d4 38 c2 0f 85 43 02 00 00 44 8b
>[ 108.251189] RIP [<ffffffff815ddb7e>] __mutex_lock_slowpath+0x4e/0x310
>[ 108.251189] RSP <ffff88003d15f698>
>[ 108.251189] CR2: 0000000000000018
>[ 108.251189] ---[ end trace 97bf1f31f04db01b ]---
>
>
>Interface config:
>
>1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP mode DEFAULT group default qlen 1000
> link/ether 00:0f:ff:10:03:01 brd ff:ff:ff:ff:ff:ff
>3: eth1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
> link/ether 00:0f:ff:11:04:01 brd ff:ff:ff:ff:ff:ff
>4: eth2: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
> link/ether 00:0f:ff:11:05:01 brd ff:ff:ff:ff:ff:ff
>5: macvlan0@eth1: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state DOWN mode DEFAULT group default
> link/ether da:ad:a9:57:7a:82 brd ff:ff:ff:ff:ff:ff
>6: macvlan1@eth2: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state DOWN mode DEFAULT group default
> link/ether e6:4d:af:76:55:a2 brd ff:ff:ff:ff:ff:ff
>
>
>Teamd config:
>{
> "device": "team0",
> "runner": {"name": "activebackup"},
> "link_watch": {
> "name": "arp_ping",
> "interval": 100,
> "missed_max": 30,
> "source_host": "192.168.123.101",
> "target_host": "192.168.123.1"
> },
> "ports": {
> "eth1": {
> "prio": -10,
> "sticky": true
> },
> "eth2": {
> "prio": 100
> }
> }
>}
>
>teamd version: v1.12
>kernel version: recent net-next (33caee3)
^ permalink raw reply
* Re: [PATCH] vhost: Add polling mode
From: Zhang Haoyu @ 2014-08-22 10:01 UTC (permalink / raw)
To: Zhang Haoyu, Razya Ladelsky, Christian Borntraeger, mashirle,
Jason Wang, Michael S.Tsirkin
Cc: abel.gordon, Alex Glikson, Eran Raichstein, Joel Nider, kvm,
linux-kernel, mst, netdev, virtualization, Yossi Kuperman1
In-Reply-To: <OF9D20825B.8FFA2829-ONC2257D3B.004A777F-C2257D3B.004C47BC@il.ibm.com>
>>> >
>>> > Results:
>>> >
>>> > Netperf, 1 vm:
>>> > The polling patch improved throughput by ~33% (1516 MB/sec -> 2046 MB/sec).
>>> > Number of exits/sec decreased 6x.
>>> > The same improvement was shown when I tested with 3 vms running netperf
>>> > (4086 MB/sec -> 5545 MB/sec).
>>> >
>>> > filebench, 1 vm:
>>> > ops/sec improved by 13% with the polling patch. Number of exits
>>> was reduced by
>>> > 31%.
>>> > The same experiment with 3 vms running filebench showed similar numbers.
>>> >
>>> > Signed-off-by: Razya Ladelsky <razya@il.ibm.com>
>>>
>>> Gave it a quick try on s390/kvm. As expected it makes no difference
>>> for big streaming workload like iperf.
>>> uperf with a 1-1 round robin got indeed faster by about 30%.
>>> The high CPU consumption is something that bothers me though, as
>>> virtualized systems tend to be full.
>>>
>>>
>>
>>Thanks for confirming the results!
>>The best way to use this patch would be along with a shared vhost thread
>>for multiple
>>devices/vms, as described in:
>>http://domino.research.ibm.com/library/cyberdig.nsf/1e4115aea78b6e7c85256b360066f0d4/479e3578ed05bfac85257b4200427735!OpenDocument
>>This work assumes having a dedicated I/O core where the vhost thread
>>serves multiple vms, which
>>makes the high cpu utilization less of a concern.
>>
>Hi, Razya, Shirley
>I am going to test the combination of
>"several (depends on total number of cpu on host, e.g., total_number * 1/3) vhost threads server all VMs" and "vhost: add polling mode",
>now I get the patch "http://thread.gmane.org/gmane.comp.emulators.kvm.devel/88682/focus=88723" posted by Shirley,
>any update to this patch?
>
>And, I want to make a bit change on this patch, create total_cpu_number * 1/N(N={3,4}) vhost threads instead of per-cpu vhost thread to server all VMs,
Just like xen netback threads, whose number is equal to num_online_cpus on Dom0,
but for kvm host, I think per-cpu vhost thread is too many.
>any ideas?
>
>Thanks,
>Zhang Haoyu
>>
>>
>>> > +static int poll_start_rate = 0;
>>> > +module_param(poll_start_rate, int, S_IRUGO|S_IWUSR);
>>> > +MODULE_PARM_DESC(poll_start_rate, "Start continuous polling of
>>> virtqueue when rate of events is at least this number per jiffy. If
>>> 0, never start polling.");
>>> > +
>>> > +static int poll_stop_idle = 3*HZ; /* 3 seconds */
>>> > +module_param(poll_stop_idle, int, S_IRUGO|S_IWUSR);
>>> > +MODULE_PARM_DESC(poll_stop_idle, "Stop continuous polling of
>>> virtqueue after this many jiffies of no work.");
>>>
>>> This seems ridicoudly high. Even one jiffie is an eternity, so
>>> setting it to 1 as a default would reduce the CPU overhead for most cases.
>>> If we dont have a packet in one millisecond, we can surely go back
>>> to the kick approach, I think.
>>>
>>> Christian
>>>
>>
>>Good point, will reduce it and recheck.
>>Thank you,
>>Razya
^ permalink raw reply
* [PATCH] netdev_adjacent_sysfs_*: fix cross-namespace symlinks
From: Alexander Y. Fomichev @ 2014-08-22 8:53 UTC (permalink / raw)
To: netdev
In-Reply-To: <20140821152940.GT9476@redhat.com>
fix code manipulating symlinks of adjacent net devices.
- prevent creation of sysfs symlink to / from adjacent device on
another net_ns.
- Drop all existing symlinks from / to all adj_dev(s) on the current
namespace before switching net_ns and recreate them just after.
---
net/core/dev.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 61 insertions(+), 1 deletion(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index b65a505..683cedf 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4889,7 +4889,8 @@ static void __netdev_adjacent_dev_remove(struct net_device *dev,
if (adj->master)
sysfs_remove_link(&(dev->dev.kobj), "master");
- if (netdev_adjacent_is_neigh_list(dev, dev_list))
+ if (netdev_adjacent_is_neigh_list(dev, dev_list) &&
+ net_eq(dev_net(dev),dev_net(adj_dev)))
netdev_adjacent_sysfs_del(dev, adj_dev->name, dev_list);
list_del_rcu(&adj->list);
@@ -5159,11 +5160,65 @@ void netdev_upper_dev_unlink(struct net_device *dev,
}
EXPORT_SYMBOL(netdev_upper_dev_unlink);
+void netdev_adjacent_add_links(struct net_device *dev)
+{
+ struct netdev_adjacent *iter;
+
+ struct net *net = dev_net(dev);
+
+ list_for_each_entry(iter, &dev->adj_list.upper, list) {
+ if (!net_eq(net,dev_net(iter->dev)))
+ continue;
+ netdev_adjacent_sysfs_add(iter->dev, dev,
+ &iter->dev->adj_list.lower);
+ netdev_adjacent_sysfs_add(dev, iter->dev,
+ &dev->adj_list.upper);
+ }
+
+ list_for_each_entry(iter, &dev->adj_list.lower, list) {
+ if (!net_eq(net,dev_net(iter->dev)))
+ continue;
+ netdev_adjacent_sysfs_add(iter->dev, dev,
+ &iter->dev->adj_list.upper);
+ netdev_adjacent_sysfs_add(dev, iter->dev,
+ &dev->adj_list.lower);
+ }
+}
+
+void netdev_adjacent_del_links(struct net_device *dev)
+{
+ struct netdev_adjacent *iter;
+
+ struct net *net = dev_net(dev);
+
+ list_for_each_entry(iter, &dev->adj_list.upper, list) {
+ if (!net_eq(net,dev_net(iter->dev)))
+ continue;
+ netdev_adjacent_sysfs_del(iter->dev, dev->name,
+ &iter->dev->adj_list.lower);
+ netdev_adjacent_sysfs_del(dev, iter->dev->name,
+ &dev->adj_list.upper);
+ }
+
+ list_for_each_entry(iter, &dev->adj_list.lower, list) {
+ if (!net_eq(net,dev_net(iter->dev)))
+ continue;
+ netdev_adjacent_sysfs_del(iter->dev, dev->name,
+ &iter->dev->adj_list.upper);
+ netdev_adjacent_sysfs_del(dev, iter->dev->name,
+ &dev->adj_list.lower);
+ }
+}
+
void netdev_adjacent_rename_links(struct net_device *dev, char *oldname)
{
struct netdev_adjacent *iter;
+ struct net *net = dev_net(dev);
+
list_for_each_entry(iter, &dev->adj_list.upper, list) {
+ if (!net_eq(net,dev_net(iter->dev)))
+ continue;
netdev_adjacent_sysfs_del(iter->dev, oldname,
&iter->dev->adj_list.lower);
netdev_adjacent_sysfs_add(iter->dev, dev,
@@ -5171,6 +5226,8 @@ void netdev_adjacent_rename_links(struct net_device *dev, char *oldname)
}
list_for_each_entry(iter, &dev->adj_list.lower, list) {
+ if (!net_eq(net,dev_net(iter->dev)))
+ continue;
netdev_adjacent_sysfs_del(iter->dev, oldname,
&iter->dev->adj_list.upper);
netdev_adjacent_sysfs_add(iter->dev, dev,
@@ -6771,8 +6828,10 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
dev_uc_flush(dev);
dev_mc_flush(dev);
+
/* Send a netdev-removed uevent to the old namespace */
kobject_uevent(&dev->dev.kobj, KOBJ_REMOVE);
+ netdev_adjacent_del_links(dev);
/* Actually switch the network namespace */
dev_net_set(dev, net);
@@ -6787,6 +6846,7 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
/* Send a netdev-add uevent to the new namespace */
kobject_uevent(&dev->dev.kobj, KOBJ_ADD);
+ netdev_adjacent_add_links(dev);
/* Fixup kobjects */
err = device_rename(&dev->dev, dev->name);
--
2.0.4
^ permalink raw reply related
* Re: [PATCH] vhost: Add polling mode
From: Zhang Haoyu @ 2014-08-22 9:30 UTC (permalink / raw)
To: Razya Ladelsky, Christian Borntraeger, mashirle, Jason Wang,
Michael S.Tsirkin
Cc: abel.gordon, Alex Glikson, Eran Raichstein, Joel Nider, kvm,
linux-kernel, mst, netdev, virtualization, Yossi Kuperman1
In-Reply-To: <OF9D20825B.8FFA2829-ONC2257D3B.004A777F-C2257D3B.004C47BC@il.ibm.com>
>> >
>> > Results:
>> >
>> > Netperf, 1 vm:
>> > The polling patch improved throughput by ~33% (1516 MB/sec -> 2046 MB/sec).
>> > Number of exits/sec decreased 6x.
>> > The same improvement was shown when I tested with 3 vms running netperf
>> > (4086 MB/sec -> 5545 MB/sec).
>> >
>> > filebench, 1 vm:
>> > ops/sec improved by 13% with the polling patch. Number of exits
>> was reduced by
>> > 31%.
>> > The same experiment with 3 vms running filebench showed similar numbers.
>> >
>> > Signed-off-by: Razya Ladelsky <razya@il.ibm.com>
>>
>> Gave it a quick try on s390/kvm. As expected it makes no difference
>> for big streaming workload like iperf.
>> uperf with a 1-1 round robin got indeed faster by about 30%.
>> The high CPU consumption is something that bothers me though, as
>> virtualized systems tend to be full.
>>
>>
>
>Thanks for confirming the results!
>The best way to use this patch would be along with a shared vhost thread
>for multiple
>devices/vms, as described in:
>http://domino.research.ibm.com/library/cyberdig.nsf/1e4115aea78b6e7c85256b360066f0d4/479e3578ed05bfac85257b4200427735!OpenDocument
>This work assumes having a dedicated I/O core where the vhost thread
>serves multiple vms, which
>makes the high cpu utilization less of a concern.
>
Hi, Razya, Shirley
I am going to test the combination of
"several (depends on total number of cpu on host, e.g., total_number * 1/3) vhost threads server all VMs" and "vhost: add polling mode",
now I get the patch "http://thread.gmane.org/gmane.comp.emulators.kvm.devel/88682/focus=88723" posted by Shirley,
any update to this patch?
And, I want to make a bit change on this patch, create total_cpu_number * 1/N(N={3,4}) vhost threads instead of per-cpu vhost thread to server all VMs,
any ideas?
Thanks,
Zhang Haoyu
>
>
>> > +static int poll_start_rate = 0;
>> > +module_param(poll_start_rate, int, S_IRUGO|S_IWUSR);
>> > +MODULE_PARM_DESC(poll_start_rate, "Start continuous polling of
>> virtqueue when rate of events is at least this number per jiffy. If
>> 0, never start polling.");
>> > +
>> > +static int poll_stop_idle = 3*HZ; /* 3 seconds */
>> > +module_param(poll_stop_idle, int, S_IRUGO|S_IWUSR);
>> > +MODULE_PARM_DESC(poll_stop_idle, "Stop continuous polling of
>> virtqueue after this many jiffies of no work.");
>>
>> This seems ridicoudly high. Even one jiffie is an eternity, so
>> setting it to 1 as a default would reduce the CPU overhead for most cases.
>> If we dont have a packet in one millisecond, we can surely go back
>> to the kick approach, I think.
>>
>> Christian
>>
>
>Good point, will reduce it and recheck.
>Thank you,
>Razya
^ permalink raw reply
* Re: [PATCH net-next 2/2] net: exit busy loop when another process is runnable
From: Jason Wang @ 2014-08-22 9:08 UTC (permalink / raw)
To: Ingo Molnar, Mike Galbraith
Cc: davem, netdev, linux-kernel, mst, Peter Zijlstra, Ingo Molnar
In-Reply-To: <20140822073653.GA7372@gmail.com>
On 08/22/2014 03:36 PM, Ingo Molnar wrote:
>>> diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h
>>> index 1d67fb6..8a33fb2 100644
>>> --- a/include/net/busy_poll.h
>>> +++ b/include/net/busy_poll.h
>>> @@ -109,7 +109,8 @@ static inline bool sk_busy_loop(struct sock *sk, int nonblock)
>>> cpu_relax();
>>>
>>> } while (!nonblock && skb_queue_empty(&sk->sk_receive_queue) &&
>>> - !need_resched() && !busy_loop_timeout(end_time));
>>> + !need_resched() && !busy_loop_timeout(end_time) &&
>>> + nr_running_this_cpu() < 2);
> So it's generally a bad idea to couple to the scheduler through
> such a low level, implementation dependent value like
> 'nr_running', causing various problems:
>
> - It misses important work that might be pending on this CPU,
> like RCU callbacks.
>
> - It will also over-credit task contexts that might be
> runnable, but which are less important than the currently
> running one: such as a SCHED_IDLE task
>
> - It will also over-credit even regular SCHED_NORMAL tasks, if
> this current task is more important than them: say
> SCHED_FIFO. A SCHED_FIFO workload should run just as fast
> with SCHED_NORMAL tasks around, as a SCHED_NORMAL workload
> on an otherwise idle system.
I see.
> So what you want is a more sophisticated query to the
> scheduler, a sched_expected_runtime() method that returns the
> number of nsecs this task is expected to run in the future,
> which returns 0 if you will be scheduled away on the next
> schedule(), and returns infinity for a high prio SCHED_FIFO
> task, or if this SCHED_NORMAL task is on an otherwise idle CPU.
>
> It will return a regular time slice value in other cases, when
> there's some load on the CPU.
>
> The polling logic can then do its decision based on that time
> value.
But this is just for current process. We want to determine whether or
not it was worth to loop busily in current process by checking if
there's any another runnable processes or callbacks. And what we need
here is just a simple and lockless hint which can't be wrong but may be
inaccurate to exit the busy loop. The net code does not depends on this
hint to do scheduling or yielding.
How about just introducing a boolean helper like current_can_busy_loop()
and return true in one of the following conditions:
- Current task is SCHED_FIFO
- Current task is neither SCHED_FIFO nor SCHED_IDLE and no other
runnable processes or pending RCU callbacks in current cpu
And add warns to make sure it can only be called in process context.
Thanks
>
> All this can be done reasonably fast and lockless in most
> cases, so that it can be called from busy-polling code.
>
> An added advantage would be that this approach consolidates the
> somewhat random need_resched() checks into this method as well.
>
> In any case I don't agree with the nr_running_this_cpu()
> method.
>
> (Please Cc: me and lkml to future iterations of this patchset.)
>
> Thanks,
>
> Ingo
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [patch net-next RFC 07/12] dsa: implement ndo_swdev_get_id
From: David Laight @ 2014-08-22 9:05 UTC (permalink / raw)
To: 'Florian Fainelli', Jiri Pirko
Cc: Sergey Ryazanov, jasowang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
John Fastabend, Neil Jerram, Eric Dumazet, Andy Gospodarek, dev,
Felix Fietkau, ronye-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
Jeff Kirsher, ogerlitz, Ben Hutchings, Lennert Buytenhek,
Roopa Prabhu, Jamal Hadi Salim, Aviad Raveh, Nicolas Dichtel,
vyasevic, Neil Horman, netdev, Stephen Hemminger, dborkman,
"Eric W. Biederman" <ebiederm@
In-Reply-To: <CAGVrzcb=vkqPw2LUc4YO4Bs-eady2=1uN-jkG=kW2RnGx=24PQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
From: Florian Fainelli
> 2014-08-21 10:06 GMT-07:00 Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>:
> > Thu, Aug 21, 2014 at 06:56:13PM CEST, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
> >>2014-08-21 9:19 GMT-07:00 Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>:
> >>> Signed-off-by: Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>
> >>> ---
> >>> net/dsa/Kconfig | 2 +-
> >>> net/dsa/slave.c | 16 ++++++++++++++++
> >>> 2 files changed, 17 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
> >>> index f5eede1..66c445a 100644
> >>> --- a/net/dsa/Kconfig
> >>> +++ b/net/dsa/Kconfig
> >>> @@ -1,6 +1,6 @@
> >>> config HAVE_NET_DSA
> >>> def_bool y
> >>> - depends on NETDEVICES && !S390
> >>> + depends on NETDEVICES && NET_SWITCHDEV && !S390
> >>>
> >>> # Drivers must select NET_DSA and the appropriate tagging format
> >>>
> >>> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> >>> index 45a1e34..e069ba3 100644
> >>> --- a/net/dsa/slave.c
> >>> +++ b/net/dsa/slave.c
> >>> @@ -171,6 +171,19 @@ static int dsa_slave_ioctl(struct net_device *dev, struct ifreq *ifr, int
> cmd)
> >>> return -EOPNOTSUPP;
> >>> }
> >>>
> >>> +static int dsa_slave_swdev_get_id(struct net_device *dev,
> >>> + struct netdev_phys_item_id *psid)
> >>> +{
> >>> + struct dsa_slave_priv *p = netdev_priv(dev);
> >>> + struct dsa_switch *ds = p->parent;
> >>> + u64 tmp = (u64) ds;
> >>> +
> >>> + /* TODO: add more sophisticated id generation */
> >>> + memcpy(&psid->id, &tmp, sizeof(tmp));
> >>> + psid->id_len = sizeof(tmp);
> >>
> >>There is already an unique id generated, which is the index in the
> >>switch tree, and which is stored in struct dsa_switch, so this could
> >>probably be simplified to:
> >>
> >>psid->id = ds->index
> >
> > That index is 0..n if I understand that correctly. That is not enough.
> > The point is to have unique id for every chip in the system. If we would
> > have 0,1,2... the collision is very likely.
>
> Good point, so an unique index for DSA switches could look like the
> DSA platform device id plus the switch index in the tree..., but then
> we would need something like (pdev->id << N) | switch index, so that
> would not give a consistent naming scheme across different devices.
Do you also need to worry about the 'lifetime' of these ids?
In which case some of the high bits need to be used as 'generation number'.
David
> Maybe we are just better with using the Linux IDR API in include/linux/idr.h?
> --
> Florian
^ permalink raw reply
* RE: order:1 failure in ipoib_cm_alloc_rx_skb
From: David Laight @ 2014-08-22 9:00 UTC (permalink / raw)
To: 'Venkat Venkatsubra', netdev@vger.kernel.org
In-Reply-To: <b2996322-2711-466b-8c41-7f4c13c7c0dc@default>
From: Venkat Venkatsubra
> Hello All,
>
> Many of our customers have hit this order:1 failure in ipoib_cm_alloc_rx_skb causing softlockup:
> ....
> skb = dev_alloc_skb(IPOIB_CM_HEAD_SIZE + 12);
> ....
Notwithstanding any other problems, that allocation could fail even if the size were
small enough that the entire skb would fit in one page.
So the driver needs changing so that the allocation failure doesn't cause a deadlock.
OTOH if you can't allocate 8k of contiguous address space and memory the system
is unlikely to be working very well at all.
David
^ permalink raw reply
* Re: [PATCH net-next 2/2] net: exit busy loop when another process is runnable
From: Ingo Molnar @ 2014-08-22 7:42 UTC (permalink / raw)
To: Jason Wang
Cc: Mike Galbraith, davem, netdev, linux-kernel, mst, Peter Zijlstra,
Ingo Molnar
In-Reply-To: <53F6F14B.1030609@redhat.com>
* Jason Wang <jasowang@redhat.com> wrote:
> Polling could be done by either rx busy loop in process
> context or NAPI in softirq. [...]
Note that this shows another reason why it's a bad idea to
query nr_running directly: depending on the softirq processing
method, a softirq might run:
- directly in process context
- in an idle thread's context
- or in a ksoftirqd context.
'nr_running' will have different values in these cases, causing
assymetries in busy-poll handling!
Another class of assymetry is when there are other softirq bits
pending, beyond NET_RX (or NET_TX): a nr_running check misses
them.
The solution I outlined in the previous mail (using a
sched_expected_runtime() method) would be able to avoid most of
these artifacts.
Thanks,
Ingo
^ permalink raw reply
* Re: [PATCH net-next 2/2] net: exit busy loop when another process is runnable
From: Ingo Molnar @ 2014-08-22 7:36 UTC (permalink / raw)
To: Mike Galbraith, Jason Wang
Cc: Jason Wang, davem, netdev, linux-kernel, mst, Peter Zijlstra,
Ingo Molnar
In-Reply-To: <1408683665.5648.69.camel@marge.simpson.net>
> > diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h
> > index 1d67fb6..8a33fb2 100644
> > --- a/include/net/busy_poll.h
> > +++ b/include/net/busy_poll.h
> > @@ -109,7 +109,8 @@ static inline bool sk_busy_loop(struct sock *sk, int nonblock)
> > cpu_relax();
> >
> > } while (!nonblock && skb_queue_empty(&sk->sk_receive_queue) &&
> > - !need_resched() && !busy_loop_timeout(end_time));
> > + !need_resched() && !busy_loop_timeout(end_time) &&
> > + nr_running_this_cpu() < 2);
So it's generally a bad idea to couple to the scheduler through
such a low level, implementation dependent value like
'nr_running', causing various problems:
- It misses important work that might be pending on this CPU,
like RCU callbacks.
- It will also over-credit task contexts that might be
runnable, but which are less important than the currently
running one: such as a SCHED_IDLE task
- It will also over-credit even regular SCHED_NORMAL tasks, if
this current task is more important than them: say
SCHED_FIFO. A SCHED_FIFO workload should run just as fast
with SCHED_NORMAL tasks around, as a SCHED_NORMAL workload
on an otherwise idle system.
So what you want is a more sophisticated query to the
scheduler, a sched_expected_runtime() method that returns the
number of nsecs this task is expected to run in the future,
which returns 0 if you will be scheduled away on the next
schedule(), and returns infinity for a high prio SCHED_FIFO
task, or if this SCHED_NORMAL task is on an otherwise idle CPU.
It will return a regular time slice value in other cases, when
there's some load on the CPU.
The polling logic can then do its decision based on that time
value.
All this can be done reasonably fast and lockless in most
cases, so that it can be called from busy-polling code.
An added advantage would be that this approach consolidates the
somewhat random need_resched() checks into this method as well.
In any case I don't agree with the nr_running_this_cpu()
method.
(Please Cc: me and lkml to future iterations of this patchset.)
Thanks,
Ingo
^ permalink raw reply
* Re: [PATCH] net: ethernet: broadcom: bnx2x: Remove redundant #ifdef
From: Rasmus Villemoes @ 2014-08-22 7:33 UTC (permalink / raw)
To: David Miller; +Cc: ariel.elior, netdev, linux-kernel
In-Reply-To: <20140821.213750.2151406448495608410.davem@davemloft.net>
David Miller <davem@davemloft.net> writes:
> From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Date: Wed, 20 Aug 2014 15:14:49 +0200
>
>> Nothing defines _ASM_GENERIC_INT_L64_H, it is a weird way to check for
>> 64 bit longs, and u64 should be printed using %llx anyway.
>>
>> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>
> It's not correct and will warn on some platforms where "u64" is just
> a plain "unsigned long".
>
> Ie. all of those which use include/asm-generic/int-l64.h
Of which there are none; that file no longer exists (as I said, nothing
defines that macro). Please see 0c79a8e2 ("asm/types.h: Remove
include/asm-generic/int-l64.h") and 2a7930bd
("Documentation/printk-formats.txt: No casts needed for u64/s64").
Thanks,
Rasmus
^ permalink raw reply
* Re: [PATCH net-next 2/2] net: exit busy loop when another process is runnable
From: Jason Wang @ 2014-08-22 7:29 UTC (permalink / raw)
To: Mike Galbraith
Cc: davem, netdev, linux-kernel, mst, Peter Zijlstra, Ingo Molnar
In-Reply-To: <1408683665.5648.69.camel@marge.simpson.net>
On 08/22/2014 01:01 PM, Mike Galbraith wrote:
> On Thu, 2014-08-21 at 16:05 +0800, Jason Wang wrote:
>> > Rx busy loop does not scale well in the case when several parallel
>> > sessions is active. This is because we keep looping even if there's
>> > another process is runnable. For example, if that process is about to
>> > send packet, keep busy polling in current process will brings extra
>> > delay and damage the performance.
>> >
>> > This patch solves this issue by exiting the busy loop when there's
>> > another process is runnable in current cpu. Simple test that pin two
>> > netperf sessions in the same cpu in receiving side shows obvious
>> > improvement:
> That patch says to me it's a bad idea to spin when someone (anyone) else
> can get some work done on a CPU, which intuitively makes sense. But..
>
> (ponders net goop: with silly 1 byte ping-pong load, throughput is bound
> by fastpath latency, net plus sched plus fixable nohz and governor crud
> if not polling, so you can't get a lot of data moved byte at a time no
> matter how sexy the pipe whether polling or not due to bound. If OTOH
> net hardware is a blazing fast large bore packet cannon, net overhead
> per unit payload drops, sched+crud is a constant)
Polling could be done by either rx busy loop in process context or NAPI
in softirq. Rx busy loop may only spin and poll when no packet were
found in socket receive queue. It spins in the hope that at least one
packet will come (in this case the process will exit rx busy loop) in a
short while. In this way, it eliminates the overheads of NAPI, wakeup
and scheduling. This patch just make the busy polling less aggressive:
Since the process finds nothing to receive when still spinning in this
loop, there's no need to waste cpu cycles ( or even call cpu_relax()) if
there's another work could be done by current CPU.
For stream workload like you mentioned here, if the card was fast
enough, the socket receive queue was not easy to be drained. Rx busy
loop won't help or even won't be triggered in this case.
>
> Seems the only time it's a good idea to poll is if blasting big packets
> on sexy hardware, and if you're doing that, you want to poll regardless
> of whether somebody else is waiting, or?
NAPI will work instead of rx busy loop in this case. It will poll and
try to drain nic's rx ring in softirq regardless somebody else.
Btw, current rx busy loop does not perform well on stream workload since
it bypasses GRO to reduce latency. But this issue beyond the scope of
this patch.
^ permalink raw reply
* Re: [PATCH net-next 1/2] sched: introduce nr_running_this_cpu()
From: Jason Wang @ 2014-08-22 7:27 UTC (permalink / raw)
To: Ingo Molnar; +Cc: davem, netdev, linux-kernel, mst, Ingo Molnar, Peter Zijlstra
In-Reply-To: <20140821135203.GA29591@gmail.com>
On 08/21/2014 09:52 PM, Ingo Molnar wrote:
> * Jason Wang <jasowang@redhat.com> wrote:
>
>> This patch introduces a helper nr_running_this_cpu() to return the
>> number of runnable processes in current cpu.
>>
>> The first user will be net rx busy polling. It will use this to exit
>> the busy loop when it finds more than one processes is runnable in
>> current cpu. This can give us better performance of busy polling under
>> heavy load.
> s/one processes/one process
>
> More importantly, please Cc: scheduler maintainers and lkml to
> both patches, so that the whole intent of the change can be
> reviewed, in full context.
>
> Thanks,
>
> Ingo
Will do this.
Thanks
^ permalink raw reply
* Re: BUG: lockdep (inconsistent usage) in netlink
From: Lan Tianyu @ 2014-08-22 7:20 UTC (permalink / raw)
To: Benjamin Block, David S. Miller; +Cc: netdev, linux-kernel, linux-acpi
In-Reply-To: <53F65A02.2080503@mageta.org>
On 08/22/2014 04:43 AM, Benjamin Block wrote:
> On 08/21/2014 08:52 PM, Benjamin Block wrote:
>> Hello,
>>
>> while rebooting one of my dev-machines I stumbled over this
>> lockdep-mess-up:
>>
>>> =================================
>>> [ INFO: inconsistent lock state ]
>>> 3.17.0-rc1-00001-gb83ca8c #2 Tainted: G O
>>> ---------------------------------
>>> inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage.
>>> swapper/0/0 [HC1[1]:SC0[0]:HE0:SE1] takes:
>>> (&(&list->lock)->rlock#3){?.-...}, at: [<ffffffff819580db>] skb_queue_tail+0x2b/0x60
>>> {HARDIRQ-ON-W} state was registered at:
>>> [<ffffffff8111c9f7>] __lock_acquire+0x877/0x1c90
>>> [<ffffffff8111e45a>] lock_acquire+0xca/0x120
>>> [<ffffffff81afc744>] _raw_spin_lock_bh+0x44/0x80
>>> [<ffffffff819a8918>] netlink_poll+0xf8/0x1c0
>>> [<ffffffff8194e031>] sock_poll+0x161/0x190
>>> [<ffffffff81271ffb>] SyS_epoll_ctl+0x51b/0xd10
>>> [<ffffffff81afd452>] system_call_fastpath+0x16/0x1b
>>> irq event stamp: 1699744
>>> hardirqs last enabled at (1699741): [<ffffffff8189b1d4>] cpuidle_enter_state+0xc4/0x190
>>> hardirqs last disabled at (1699742): [<ffffffff81afdfaa>] common_interrupt+0x6a/0x6f
>>> softirqs last enabled at (1699744): [<ffffffff810d7fda>] _local_bh_enable+0x4a/0x50
>>> softirqs last disabled at (1699743): [<ffffffff810d88f0>] irq_enter+0x30/0x70
>>>
>>> other info that might help us debug this:
>>> Possible unsafe locking scenario:
>>>
>>> CPU0
>>> ----
>>> lock(&(&list->lock)->rlock#3);
>>> <Interrupt>
>>> lock(&(&list->lock)->rlock#3);
>>>
>>> *** DEADLOCK ***
>>>
>>> no locks held by swapper/0/0.
>>>
>>> stack backtrace:
>>> CPU: 0 PID: 0 Comm: swapper/0 Tainted: G O 3.17.0-rc1-00001-gb83ca8c #2
>>> Hardware name: ASUS All Series/Q87T, BIOS 0216 10/16/2013
>>> ffffffff8295a5b0 ffff8802158039a8 ffffffff81af20fa 0000000000000000
>>> ffffffff822164e0 ffff880215803a08 ffffffff81aee400 0000000000000000
>>> ffffffff00000000 ffff880200000001 ffffffff8105ac0f ffffffff82d2abe0
>>> Call Trace:
>>> <IRQ> [<ffffffff81af20fa>] dump_stack+0x4e/0x68
>>> [<ffffffff81aee400>] print_usage_bug+0x1ec/0x1fd
>>> [<ffffffff8105ac0f>] ? save_stack_trace+0x2f/0x50
>>> [<ffffffff8111b600>] ? print_irq_inversion_bug+0x200/0x200
>>> [<ffffffff8111c061>] mark_lock+0x191/0x2b0
>>> [<ffffffff8111c96a>] __lock_acquire+0x7ea/0x1c90
>>> [<ffffffff8111ca94>] ? __lock_acquire+0x914/0x1c90
>>> [<ffffffff8111b600>] ? print_irq_inversion_bug+0x200/0x200
>>> [<ffffffff8111ca94>] ? __lock_acquire+0x914/0x1c90
>>> [<ffffffff8111e45a>] lock_acquire+0xca/0x120
>>> [<ffffffff819580db>] ? skb_queue_tail+0x2b/0x60
>>> [<ffffffff81afc590>] _raw_spin_lock_irqsave+0x50/0x90
>>> [<ffffffff819580db>] ? skb_queue_tail+0x2b/0x60
>>> [<ffffffff819580db>] skb_queue_tail+0x2b/0x60
>>> [<ffffffff819a774f>] __netlink_sendskb+0x21f/0x250
>>> [<ffffffff819a7d63>] netlink_broadcast_filtered+0x273/0x3b0
>>> [<ffffffff819a7ebd>] netlink_broadcast+0x1d/0x20
>>> [<ffffffff8152fb8a>] ? nla_reserve+0x2a/0x40
>>> [<ffffffff81589728>] acpi_bus_generate_netlink_event+0x160/0x178
>>> [<ffffffff815a8db9>] acpi_button_notify+0xe1/0xec
>>> [<ffffffff81580648>] acpi_device_notify+0x19/0x1b
>>> [<ffffffff81580662>] acpi_device_notify_fixed+0x18/0x1c
>>> [<ffffffff8158f039>] acpi_ev_fixed_event_detect+0xe6/0x10d
>>> [<ffffffff8159157a>] acpi_ev_sci_xrupt_handler+0x19/0x3f
>>> [<ffffffff8157c1a9>] acpi_irq+0x16/0x31
>>> [<ffffffff81131e2a>] handle_irq_event_percpu+0x6a/0x1d0
>>> [<ffffffff81131fd8>] handle_irq_event+0x48/0x70
>>> [<ffffffff8113534f>] ? handle_fasteoi_irq+0x2f/0x160
>>> [<ffffffff811353e7>] handle_fasteoi_irq+0xc7/0x160
>>> [<ffffffff8104cd94>] handle_irq+0x134/0x150
>>> [<ffffffff810f4876>] ? atomic_notifier_call_chain+0x16/0x20
>>> [<ffffffff81054dec>] ? __exit_idle+0x2c/0x30
>>> [<ffffffff81affe7e>] do_IRQ+0x5e/0x100
>>> [<ffffffff81afdfaf>] common_interrupt+0x6f/0x6f
>>> <EOI> [<ffffffff8189b1df>] ? cpuidle_enter_state+0xcf/0x190
>>> [<ffffffff8189b1d4>] ? cpuidle_enter_state+0xc4/0x190
>>> [<ffffffff8189b387>] cpuidle_enter+0x17/0x20
>>> [<ffffffff81111ae1>] cpu_startup_entry+0x3a1/0x3c0
>>> [<ffffffff81ae92a4>] rest_init+0xc4/0xd0
>>> [<ffffffff81ae91e5>] ? rest_init+0x5/0xd0
>>> [<ffffffff825718a1>] ? ftrace_init+0xa8/0x13b
>>> [<ffffffff8255103a>] start_kernel+0x461/0x46e
>>> [<ffffffff82550939>] ? set_init_arg+0x57/0x57
>>> [<ffffffff825505af>] x86_64_start_reservations+0x2a/0x2c
>>> [<ffffffff825506ae>] x86_64_start_kernel+0xfd/0x101
>>
>> Sadly I couldn't reproduce it. This looks all to be very general
>> functions and my best guess is, netlink_poll() needs to be irq-save.
>> Thing is, the corresponding code is quite old and I can't really bisec
>> it, because the none-reproducibility.
>>
>
> Thinking more about it.. this seems to be unlikely. More like the
> acpi-irq chain should not do netlink-events still in irq-context - just
> guessing here, sry :).
Hi Benjamin:
Basically, I think ACPI fixed button device's notify callback should not run in
the interrupt context. This prevents calling function with mutex lock(E,G
evaluating ACPI method). I will write a patch to do that.
>
> I tracked around a little and came up with more recent commits in that
> call-chain:
>
> commit 0bf6368ee8f25826d0645c0f7a4f17c8845356a4
> - adds acpi_bus_generate_netlink_event to the chain
>
> Again, all other places around the chain seems quit old or unrelated.
>
>>
>> There is only the small ipv6-fib patch applied, I send in earlier today
>> (https://lkml.org/lkml/2014/8/21/506). This should have nothing to do
>> with this here.
>>
>
> - Benjamin
>
^ permalink raw reply
* Re: stmmac: Needs to check dma_map return values (oops when build with CONFIG_DMA_API_DEBUG)
From: Giuseppe CAVALLARO @ 2014-08-22 6:53 UTC (permalink / raw)
To: Denis Kirjanov; +Cc: Hans de Goede, netdev
In-Reply-To: <CAOJe8K2=AwaBxmRYgbEUyTsRqK8XNkDjnYQw_Nk=yBhZg=X4Ww@mail.gmail.com>
On 8/19/2014 10:56 PM, Denis Kirjanov wrote:
> On 8/18/14, Giuseppe CAVALLARO <peppe.cavallaro@st.com> wrote:
>> On 8/1/2014 12:11 PM, Denis Kirjanov wrote:
>>> On 8/1/14, Hans de Goede <hdegoede@redhat.com> wrote:
>>>> Hi,
>>>>
>>>> When booting a Fedora rawhide kernel, which is build with
>>>> CONFIG_DMA_API_DEBUG enabled, I get the following oops:
>>>>
>>>> [ 162.398116] ------------[ cut here ]------------
>>>> [ 162.402770] WARNING: CPU: 0 PID: 0 at lib/dma-debug.c:1140
>>>> check_unmap+0x6ec/0x824()
>>>> [ 162.410507] stmmaceth 1c50000.ethernet: DMA-API: device driver failed
>>>> to
>>>> check map error[device address=0x0000000068a1d002] [size=90 bytes]
>>>> [mapped
>>>> as single]
>>>> [ 162.424480] Modules linked in: ip6t_rpfilter ip6t_REJECT xt_conntrack
>>>> cfg80211 rfkill ebtable_nat ebtable_broute bridge stp llc ebtable_filter
>>>> ebtables ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6
>>>> ip6table_mangle ip6table_security ip6table_raw ip6table_filter
>>>> ip6_tables
>>>> iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat
>>>> nf_conntrack
>>>> iptable_mangle iptable_security iptable_raw stmmac ptp pps_core
>>>> i2c_mv64xxx
>>>> sun4i_ts ohci_platform sunxi_sid rtc_sunxi phy_sun4i_usb sunxi_wdt
>>>> leds_gpio
>>>> mmc_block sunxi_mmc mmc_core ehci_platform ahci_sunxi libahci_platform
>>>> [ 162.476435] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
>>>> 3.16.0-0.rc7.git1.1.fc22.armv7hl #1
>>>> [ 162.484622] [<c02188dc>] (unwind_backtrace) from [<c0212bb0>]
>>>> (show_stack+0x18/0x1c)
>>>> [ 162.492366] [<c0212bb0>] (show_stack) from [<c0938d20>]
>>>> (dump_stack+0x84/0xb0)
>>>> [ 162.499590] [<c0938d20>] (dump_stack) from [<c0252444>]
>>>> (warn_slowpath_common+0x70/0x94)
>>>> [ 162.507678] [<c0252444>] (warn_slowpath_common) from [<c025249c>]
>>>> (warn_slowpath_fmt+0x34/0x44)
>>>> [ 162.516374] [<c025249c>] (warn_slowpath_fmt) from [<c058a65c>]
>>>> (check_unmap+0x6ec/0x824)
>>>> [ 162.524463] [<c058a65c>] (check_unmap) from [<c058a7f8>]
>>>> (debug_dma_unmap_page+0x64/0x70)
>>>> [ 162.532667] [<c058a7f8>] (debug_dma_unmap_page) from [<bf08dde0>]
>>>> (stmmac_tx_clean+0x260/0x494 [stmmac])
>>>> [ 162.542242] [<bf08dde0>] (stmmac_tx_clean [stmmac]) from [<c025f988>]
>>>> (call_timer_fn+0xd0/0x310)
>>>> [ 162.551025] [<c025f988>] (call_timer_fn) from [<c0260578>]
>>>> (run_timer_softirq+0x268/0x360)
>>>> [ 162.559287] [<c0260578>] (run_timer_softirq) from [<c0258628>]
>>>> (__do_softirq+0x184/0x4d0)
>>>> [ 162.567461] [<c0258628>] (__do_softirq) from [<c0258c58>]
>>>> (irq_exit+0x8c/0xfc)
>>>> [ 162.574683] [<c0258c58>] (irq_exit) from [<c020f52c>]
>>>> (handle_IRQ+0x74/0x98)
>>>> [ 162.581730] [<c020f52c>] (handle_IRQ) from [<c0208920>]
>>>> (gic_handle_irq+0x4c/0x70)
>>>> [ 162.589299] [<c0208920>] (gic_handle_irq) from [<c0942b04>]
>>>> (__irq_svc+0x44/0x58)
>>>> [ 162.596774] Exception stack(0xc0d95f58 to 0xc0d95fa0)
>>>> [ 162.601823] 5f40:
>>>> 00000001 00000004
>>>> [ 162.609995] 5f60: 00000000 c0da2128 c0d94000 00000000 00000000
>>>> c0d915d0
>>>> 00000000 ffffffed
>>>> [ 162.618168] 5f80: c0d94000 c0d9de8c 00000000 c0d95fa0 c02a6d4c
>>>> c020f8bc
>>>> 200f0013 ffffffff
>>>> [ 162.626344] [<c0942b04>] (__irq_svc) from [<c020f8bc>]
>>>> (arch_cpu_idle+0x38/0x44)
>>>> [ 162.633742] [<c020f8bc>] (arch_cpu_idle) from [<c029e780>]
>>>> (cpu_startup_entry+0x21c/0x6ac)
>>>> [ 162.642008] [<c029e780>] (cpu_startup_entry) from [<c0cabb98>]
>>>> (start_kernel+0x374/0x3dc)
>>>> [ 162.650176] ---[ end trace 70ec89df0de626f3 ]---
>>>> [ 162.654787] Mapped at:
>>>> [ 162.657145] [<c07ed840>] dev_hard_start_xmit+0x350/0x8a0
>>>> [ 162.662550] [<c080fcf8>] sch_direct_xmit+0x74/0x264
>>>> [ 162.667526] [<c07ee1ec>] __dev_queue_xmit+0x45c/0x8e0
>>>> [ 162.672675] [<c08a8c84>] ip6_finish_output2+0x7cc/0xa80
>>>> [ 162.677999] [<c08d3a20>] mld_sendpack+0x3fc/0x808
>>>>
>>>> This is caused by various calls to dma_map_single not being error
>>>> checked.
>>>>
>>>> A call to dma_map_single should look something like this:
>>>>
>>>> dma_handle = dma_map_single(dev, addr, size, direction);
>>>> if (dma_mapping_error(dma_handle))
>>>> /* do error */
>>>>
>>>> I believe that the calls to skb_frag_dma_map should be error
>>>> checked similarly, but I'm not 100% sure on that.
>>> Yes. You should unwind your frags.
>>> It's been done already in many places.
>>>
>>>>
>>>> I looked into fixing this myself, but esp. for the jumbo frame paths
>>>> fixing this looks like it is non trivial.
>>
>> Hello Denis.
>>
>> I have a patch that must be ported on top of the net.dev kernel
>> and I will provide it asap.
>>
>> For reference this is the patch I will rework, give me you opinion too.
>>
>> http://git.stlinux.com/?p=stm/linux-stm.git;a=commit;h=755f606c7efc5d9c111fa2be00b6e9916e101920
>
> Reviewed-by: Denis Kirjanov <kda@linux-powerpc.org>
>
> Please note that you _do still ignore_ the dma_* return values
Ok Denis, I will take care of this too.
I will rework the patch and send it to this mailing list to be reviewed
again.
br
Peppe
>>
>> br
>> peppe
>>
>>>>
>>>> Regards,
>>>>
>>>> Hans
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>
>>>
>>
>>
>
>
^ permalink raw reply
* Re: [patch net-next 1/3] net: propagate sock pointer through netfilter hooks
From: Jiri Pirko @ 2014-08-22 5:56 UTC (permalink / raw)
To: David Miller
Cc: netdev, kuznet, jmorris, yoshfuji, stephen, cwang, pshelar,
nicolas.dichtel, therbert, dborkman, edumazet
In-Reply-To: <20140821.173339.1409243624518012670.davem@davemloft.net>
Fri, Aug 22, 2014 at 02:33:39AM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Fri, 15 Aug 2014 20:32:54 +0200
>
>> When output function (ip6_finish_output2 for example) needs to be called
>> with sock pointer, we need to push sock pointer through the netfilter
>> hooks. This patch does that.
>>
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>
>Ok, I'm going to admit that I am having second thoughts about this
>approach. This is a quite large set of churn to fix this bug.
>
>However, in the same breath, I can't come up with a simpler way to
>propagate this information without the really unacceptable overhead of
>adding another sk_buff member.
>
>And even if we found some simple way to deal with that sk_mc_loop()
>test, ipv6 has other demons in this area.
>
>For example, look at what ip6_fragment() does, it also assumes skb->sk
>is an inet6 socket.
>
> struct ipv6_pinfo *np = skb->sk ? inet6_sk(skb->sk) : NULL;
> ...
> if (np && np->frag_size < mtu) {
> if (np->frag_size)
> mtu = np->frag_size;
> }
>
>The rest of the skb->sk usage in these place is fine, as they are
>simply propagating socket ownership from one packet to another, rather
>than doing protocol specific things with them.
You are correct. Another code can benefit from proparated sk as well. I
found couple of other possible places on the way (which I unfortunatelly
did not noted :/).
^ permalink raw reply
* Re: [PATCH net-next 2/2] net: exit busy loop when another process is runnable
From: Mike Galbraith @ 2014-08-22 5:01 UTC (permalink / raw)
To: Jason Wang; +Cc: davem, netdev, linux-kernel, mst, Peter Zijlstra, Ingo Molnar
In-Reply-To: <1408608310-13579-2-git-send-email-jasowang@redhat.com>
On Thu, 2014-08-21 at 16:05 +0800, Jason Wang wrote:
> Rx busy loop does not scale well in the case when several parallel
> sessions is active. This is because we keep looping even if there's
> another process is runnable. For example, if that process is about to
> send packet, keep busy polling in current process will brings extra
> delay and damage the performance.
>
> This patch solves this issue by exiting the busy loop when there's
> another process is runnable in current cpu. Simple test that pin two
> netperf sessions in the same cpu in receiving side shows obvious
> improvement:
That patch says to me it's a bad idea to spin when someone (anyone) else
can get some work done on a CPU, which intuitively makes sense. But..
(ponders net goop: with silly 1 byte ping-pong load, throughput is bound
by fastpath latency, net plus sched plus fixable nohz and governor crud
if not polling, so you can't get a lot of data moved byte at a time no
matter how sexy the pipe whether polling or not due to bound. If OTOH
net hardware is a blazing fast large bore packet cannon, net overhead
per unit payload drops, sched+crud is a constant)
Seems the only time it's a good idea to poll is if blasting big packets
on sexy hardware, and if you're doing that, you want to poll regardless
of whether somebody else is waiting, or?
> Before:
> netperf -H 192.168.100.2 -T 0,0 -t TCP_RR -P 0 & \
> netperf -H 192.168.100.2 -T 1,0 -t TCP_RR -P 0
> 16384 87380 1 1 10.00 15513.74
> 16384 87380
> 16384 87380 1 1 10.00 15092.78
> 16384 87380
>
> After:
> netperf -H 192.168.100.2 -T 0,0 -t TCP_RR -P 0 & \
> netperf -H 192.168.100.2 -T 1,0 -t TCP_RR -P 0
> 16384 87380 1 1 10.00 23334.53
> 16384 87380
> 16384 87380 1 1 10.00 23327.58
> 16384 87380
>
> Benchmark was done through two 8 cores Xeon machine back to back connected
> with mlx4 through netperf TCP_RR test (busy_read were set to 50):
>
> sessions/bytes/before/after/+improvement%/busy_read=0/
> 1/1/30062.10/30034.72/+0%/20228.96/
> 16/1/214719.83/307669.01/+43%/268997.71/
> 32/1/231252.81/345845.16/+49%/336157.442/
> 64/512/212467.39/373464.93/+75%/397449.375/
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> include/net/busy_poll.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h
> index 1d67fb6..8a33fb2 100644
> --- a/include/net/busy_poll.h
> +++ b/include/net/busy_poll.h
> @@ -109,7 +109,8 @@ static inline bool sk_busy_loop(struct sock *sk, int nonblock)
> cpu_relax();
>
> } while (!nonblock && skb_queue_empty(&sk->sk_receive_queue) &&
> - !need_resched() && !busy_loop_timeout(end_time));
> + !need_resched() && !busy_loop_timeout(end_time) &&
> + nr_running_this_cpu() < 2);
>
> rc = !skb_queue_empty(&sk->sk_receive_queue);
> out:
^ permalink raw reply
* Re: [PATCH net] cxgb4: Free completed tx skbs promptly
From: David Miller @ 2014-08-22 4:55 UTC (permalink / raw)
To: hariprasad; +Cc: netdev, leedom, nirranjan, kumaras, anish
In-Reply-To: <1408620886-20065-1-git-send-email-hariprasad@chelsio.com>
From: Hariprasad Shenai <hariprasad@chelsio.com>
Date: Thu, 21 Aug 2014 17:04:46 +0530
> Description of problem:
> The NIC card is not reporting back to the driver the transmitted skbs,
> so they get stuck in the TX ring causing issues with reference
> counters in other kernel components.
>
> Developed a new Automatic Egress Queue Update firmware facility to slowly tick
> through Egress Queues and send back any outstanding CIDX Updates which are
> laying around.
>
> Based on original work by Casey Leedom <leedom@chelsio.com>
>
> Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
Applied, thank you.
^ 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