* [PATCH net-next 1/2 v2] netns: restrict uevents
From: Christian Brauner @ 2018-04-24 20:43 UTC (permalink / raw)
To: ebiederm, davem, netdev, linux-kernel
Cc: avagin, ktkhai, serge, gregkh, Christian Brauner
In-Reply-To: <20180424204335.12904-1-christian.brauner@ubuntu.com>
commit 07e98962fa77 ("kobject: Send hotplug events in all network namespaces")
enabled sending hotplug events into all network namespaces back in 2010.
Over time the set of uevents that get sent into all network namespaces has
shrunk a little. We have now reached the point where hotplug events for all
devices that carry a namespace tag are filtered according to that
namespace. Specifically, they are filtered whenever the namespace tag of
the kobject does not match the namespace tag of the netlink socket. One
example are network devices. Uevents for network devices only show up in
the network namespaces these devices are moved to or created in.
However, any uevent for a kobject that does not have a namespace tag
associated with it will not be filtered and we will broadcast it into all
network namespaces. This behavior stopped making sense when user namespaces
were introduced.
This patch restricts uevents to the initial user namespace for a couple of
reasons that have been extensively discusses on the mailing list [1].
- Thundering herd:
Broadcasting uevents into all network namespaces introduces significant
overhead.
All processes that listen to uevents running in non-initial user
namespaces will end up responding to uevents that will be meaningless to
them. Mainly, because non-initial user namespaces cannot easily manage
devices unless they have a privileged host-process helping them out. This
means that there will be a thundering herd of activity when there
shouldn't be any.
- Uevents from non-root users are already filtered in userspace:
Uevents are filtered by userspace in a user namespace because the
received uid != 0. Instead the uid associated with the event will be
65534 == "nobody" because the global root uid is not mapped.
This means we can safely and without introducing regressions modify the
kernel to not send uevents into all network namespaces whose owning user
namespace is not the initial user namespace because we know that
userspace will ignore the message because of the uid anyway. I have
a) verified that is is true for every udev implementation out there b)
that this behavior has been present in all udev implementations from the
very beginning.
- Removing needless overhead/Increasing performance:
Currently, the uevent socket for each network namespace is added to the
global variable uevent_sock_list. The list itself needs to be protected
by a mutex. So everytime a uevent is generated the mutex is taken on the
list. The mutex is held *from the creation of the uevent (memory
allocation, string creation etc. until all uevent sockets have been
handled*. This is aggravated by the fact that for each uevent socket that
has listeners the mc_list must be walked as well which means we're
talking O(n^2) here. Given that a standard Linux workload usually has
quite a lot of network namespaces and - in the face of containers - a lot
of user namespaces this quickly becomes a performance problem (see
"Thundering herd" above). By just recording uevent sockets of network
namespaces that are owned by the initial user namespace we significantly
increase performance in this codepath.
- Injecting uevents:
There's a valid argument that containers might be interested in receiving
device events especially if they are delegated to them by a privileged
userspace process. One prime example are SR-IOV enabled devices that are
explicitly designed to be handed of to other users such as VMs or
containers.
This use-case can now be correctly handled since
commit 692ec06d7c92 ("netns: send uevent messages"). This commit
introduced the ability to send uevents from userspace. As such we can let
a sufficiently privileged (CAP_SYS_ADMIN in the owning user namespace of
the network namespace of the netlink socket) userspace process make a
decision what uevents should be sent. This removes the need to blindly
broadcast uevents into all user namespaces and provides a performant and
safe solution to this problem.
- Filtering logic:
This patch filters by *owning user namespace of the network namespace a
given task resides in* and not by user namespace of the task per se. This
means if the user namespace of a given task is unshared but the network
namespace is kept and is owned by the initial user namespace a listener
that is opening the uevent socket in that network namespace can still
listen to uevents.
[1]: https://lkml.org/lkml/2018/4/4/739
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
Changelog v1->v2:
* patch unchanged
Changelog v0->v1:
* patch unchanged
---
lib/kobject_uevent.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c
index 15ea216a67ce..f5f5038787ac 100644
--- a/lib/kobject_uevent.c
+++ b/lib/kobject_uevent.c
@@ -703,9 +703,13 @@ static int uevent_net_init(struct net *net)
net->uevent_sock = ue_sk;
- mutex_lock(&uevent_sock_mutex);
- list_add_tail(&ue_sk->list, &uevent_sock_list);
- mutex_unlock(&uevent_sock_mutex);
+ /* Restrict uevents to initial user namespace. */
+ if (sock_net(ue_sk->sk)->user_ns == &init_user_ns) {
+ mutex_lock(&uevent_sock_mutex);
+ list_add_tail(&ue_sk->list, &uevent_sock_list);
+ mutex_unlock(&uevent_sock_mutex);
+ }
+
return 0;
}
@@ -713,9 +717,11 @@ static void uevent_net_exit(struct net *net)
{
struct uevent_sock *ue_sk = net->uevent_sock;
- mutex_lock(&uevent_sock_mutex);
- list_del(&ue_sk->list);
- mutex_unlock(&uevent_sock_mutex);
+ if (sock_net(ue_sk->sk)->user_ns == &init_user_ns) {
+ mutex_lock(&uevent_sock_mutex);
+ list_del(&ue_sk->list);
+ mutex_unlock(&uevent_sock_mutex);
+ }
netlink_kernel_release(ue_sk->sk);
kfree(ue_sk);
--
2.17.0
^ permalink raw reply related
* [PATCH net-next 0/2 v2] netns: uevent performance tweaks
From: Christian Brauner @ 2018-04-24 20:43 UTC (permalink / raw)
To: ebiederm, davem, netdev, linux-kernel
Cc: avagin, ktkhai, serge, gregkh, Christian Brauner
Hey everyone,
This is v2 of "netns: uevent performance tweaks" which contains *no
functional changes* just a minor indendation fix as requested by David.
Like Eric requested, I did extensive testing that prove significant
performance improvements when using per-netns uevent sequence numbers
with decoupled locks. The results and test descriptions were added to
the commit message of
[PATCH 2/2 v1] netns: isolate seqnums to use per-netns locks.
This series deals with a bunch of performance improvements when sending
out uevents that have been extensively discussed here:
https://lkml.org/lkml/2018/4/10/592
- Only record uevent sockets from network namespaces owned by the
initial user namespace in the global uevent socket list.
Eric, this is the exact patch we agreed upon in
https://lkml.org/lkml/2018/4/10/592.
A very detailed rationale is present in the commit message for
[PATCH 1/2] netns: restrict uevents
- Decouple the locking for network namespaces in the global uevent
socket list from the locking for network namespaces not in the global
uevent socket list.
A very detailed rationale including performance test results is
present in the commit message for
[PATCH 2/2] netns: isolate seqnums to use per-netns locks
Thanks!
Christian
Christian Brauner (2):
netns: restrict uevents
netns: isolate seqnums to use per-netns locks
include/linux/kobject.h | 2 +
include/net/net_namespace.h | 3 +
kernel/ksysfs.c | 11 +++-
lib/kobject_uevent.c | 122 ++++++++++++++++++++++++++++--------
net/core/net_namespace.c | 14 +++++
5 files changed, 126 insertions(+), 26 deletions(-)
--
2.17.0
^ permalink raw reply
* [GIT] Networking
From: David Miller @ 2018-04-24 20:38 UTC (permalink / raw)
To: torvalds; +Cc: akpm, netdev, linux-kernel
1) Fix rtnl deadlock in ipvs, from Julian Anastasov.
2) s390 qeth fixes from Julian Wiedmann (control IO completion stalls, bad MAC
address update sequence, request side races on command IO timeouts).
3) Handle seq_file overflow properly in l2tp, from Guillaume Nault.
4) Fix VLAN priority mappings in cpsw driver, from Ivan Khoronzhuk.
5) Packet scheduler ife action fixes (malformed TLV lengths, etc.)
from Alexander Aring.
6) Fix out of bounds access in tcp md5 option parser, from Jann Horn.
7) Missing netlink attribute policies in rtm_ipv6_policy table, from
Eric Dumazet.
8) Missing socket address length checks in l2tp and pppoe connect,
from Guillaume Nault.
9) Fix netconsole over team and bonding, from Xin Long.
10) Fix race with AF_PACKET socket state bitfields, from Willem de
Bruijn.
Pulled, thanks a lot!
The following changes since commit 83beed7b2b26f232d782127792dd0cd4362fdc41:
Merge branch 'fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal (2018-04-20 10:56:32 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git
for you to fetch changes up to d19efb729f10339f91c35003d480dc718cae3b3c:
Merge branch '1GbE' of git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-queue (2018-04-24 16:17:59 -0400)
----------------------------------------------------------------
Ahmed Abdelsalam (1):
ipv6: sr: fix NULL pointer dereference in seg6_do_srh_encap()- v4 pkts
Alexander Aring (3):
net: sched: ife: signal not finding metaid
net: sched: ife: handle malformed tlv length
net: sched: ife: check on metadata length
Anders Roxell (1):
selftests: bpf: update .gitignore with missing generated files
Anirudh Venkataramanan (2):
ice: Fix initialization for num_nodes_added
ice: Fix incorrect comment for action type
Arnd Bergmann (1):
netfilter: fix CONFIG_NF_REJECT_IPV6=m link error
Ben Shelton (1):
ice: Do not check INTEVENT bit for OICR interrupts
Colin Ian King (1):
ixgbevf: ensure xdp_ring resources are free'd on error exit
Cong Wang (3):
netfilter: conntrack: silent a memory leak warning
llc: delete timers synchronously in llc_sk_free()
llc: fix NULL pointer deref for SOCK_ZAPPED
David S. Miller (6):
Merge branch 's390-qeth-fixes'
Merge branch 'net-sched-ife-malformed-ife-packet-fixes'
Merge git://git.kernel.org/.../bpf/bpf
Merge git://git.kernel.org/.../pablo/nf
Merge branch 'amd-xgbe-fixes'
Merge branch '1GbE' of git://git.kernel.org/.../jkirsher/net-queue
Doron Roberts-Kedes (1):
strparser: Do not call mod_delayed_work with a timeout of LONG_MAX
Edward Cree (1):
sfc: ARFS filter IDs
Eric Dumazet (1):
ipv6: add RTA_TABLE and RTA_PREFSRC to rtm_ipv6_policy
Florian Fainelli (1):
net: ethtool: Add missing kernel doc for FEC parameters
Florian Westphal (4):
netfilter: nf_conntrack_sip: allow duplicate SDP expectations
netfilter: ebtables: don't attempt to allocate 0-sized compat array
netfilter: nf_tables: can't fail after linking rule into active rule list
netfilter: nf_tables: free set name in error path
Guillaume Nault (3):
l2tp: fix {pppol2tp, l2tp_dfs}_seq_stop() in case of seq_file overflow
l2tp: check sockaddr length in pppol2tp_connect()
pppoe: check sockaddr length in pppoe_connect()
Ivan Khoronzhuk (1):
net: ethernet: ti: cpsw: fix tx vlan priority mapping
Jack Ma (1):
netfilter: xt_connmark: Add bit mapping for bit-shift operation.
Jann Horn (2):
bpf: sockmap remove dead check
tcp: don't read out-of-bounds opsize
Jingju Hou (1):
net: phy: marvell: clear wol event before setting it
Julian Anastasov (1):
ipvs: fix rtnl_lock lockups caused by start_sync_thread
Julian Wiedmann (6):
s390/qeth: fix error handling in adapter command callbacks
s390/qeth: avoid control IO completion stalls
s390/qeth: handle failure on workqueue creation
s390/qeth: fix MAC address update sequence
s390/qeth: fix request-side race during cmd IO timeout
s390/qeth: use Read device to query hypervisor for MAC
Md Fahad Iqbal Polash (1):
ice: Fix insufficient memory issue in ice_aq_manage_mac_read
Pablo Neira Ayuso (2):
netfilter: nf_tables: NAT chain and extensions require NF_TABLES
netfilter: xt_connmark: do not cast xt_connmark_tginfo1 to xt_connmark_tginfo2
Stephen Rothwell (1):
netfilter: conntrack: include kmemleak.h for kmemleak_not_leak()
Taehee Yoo (1):
netfilter: nf_tables: fix out-of-bounds in nft_chain_commit_update
Thomas Falcon (1):
ibmvnic: Clean actual number of RX or TX pools
Tom Lendacky (3):
amd-xgbe: Add pre/post auto-negotiation phy hooks
amd-xgbe: Improve KR auto-negotiation and training
amd-xgbe: Only use the SFP supported transceiver signals
Vinicius Costa Gomes (1):
igb: Fix the transmission mode of queue 0 for Qav mode
Willem de Bruijn (1):
packet: fix bitfield update race
Xin Long (2):
bonding: do not set slave_dev npinfo before slave_enable_netpoll in bond_enslave
team: fix netconsole setup over team
Yonghong Song (2):
bpf/tracing: fix a deadlock in perf_event_detach_bpf_prog
tools/bpf: fix test_sock and test_sock_addr.sh failure
drivers/net/bonding/bond_main.c | 3 +-
drivers/net/ethernet/amd/xgbe/xgbe-common.h | 8 +++++
drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c | 16 +++++++++
drivers/net/ethernet/amd/xgbe/xgbe-main.c | 1 +
drivers/net/ethernet/amd/xgbe/xgbe-mdio.c | 24 ++++++++++---
drivers/net/ethernet/amd/xgbe/xgbe-pci.c | 2 ++
drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c | 196 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------
drivers/net/ethernet/amd/xgbe/xgbe.h | 9 +++++
drivers/net/ethernet/ibm/ibmvnic.c | 4 +--
drivers/net/ethernet/intel/ice/ice_adminq_cmd.h | 2 +-
drivers/net/ethernet/intel/ice/ice_common.c | 22 +++++++++---
drivers/net/ethernet/intel/ice/ice_hw_autogen.h | 2 --
drivers/net/ethernet/intel/ice/ice_main.c | 4 ---
drivers/net/ethernet/intel/ice/ice_sched.c | 4 +--
drivers/net/ethernet/intel/igb/igb_main.c | 17 ++++++++-
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 2 +-
drivers/net/ethernet/sfc/ef10.c | 80 +++++++++++++++++++++++------------------
drivers/net/ethernet/sfc/efx.c | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
drivers/net/ethernet/sfc/efx.h | 21 +++++++++++
drivers/net/ethernet/sfc/farch.c | 41 +++++++++++++++++----
drivers/net/ethernet/sfc/net_driver.h | 36 +++++++++++++++++++
drivers/net/ethernet/sfc/rx.c | 62 +++++++++++++++++++++++++++++---
drivers/net/ethernet/ti/cpsw.c | 2 +-
drivers/net/phy/marvell.c | 9 +++++
drivers/net/ppp/pppoe.c | 4 +++
drivers/net/team/team.c | 19 ++++++----
drivers/s390/net/qeth_core.h | 2 --
drivers/s390/net/qeth_core_main.c | 158 ++++++++++++++++++++++++++++++++++++--------------------------------------------
drivers/s390/net/qeth_core_mpc.h | 12 +++++++
drivers/s390/net/qeth_l2_main.c | 59 ++++++++++++++++--------------
include/linux/bpf.h | 4 +--
include/linux/ethtool.h | 2 ++
include/net/ife.h | 3 +-
include/net/llc_conn.h | 1 +
kernel/bpf/core.c | 45 ++++++++++++++---------
kernel/bpf/sockmap.c | 3 --
kernel/trace/bpf_trace.c | 25 ++++++++++---
net/bridge/netfilter/ebtables.c | 11 +++---
net/ife/ife.c | 38 ++++++++++++++++++--
net/ipv4/tcp_input.c | 7 ++--
net/ipv6/netfilter/Kconfig | 55 ++++++++++++++--------------
net/ipv6/route.c | 2 ++
net/ipv6/seg6_iptunnel.c | 2 +-
net/l2tp/l2tp_debugfs.c | 5 ++-
net/l2tp/l2tp_ppp.c | 12 ++++++-
net/llc/af_llc.c | 21 ++++++-----
net/llc/llc_c_ac.c | 9 +----
net/llc/llc_conn.c | 22 +++++++++++-
net/netfilter/Kconfig | 1 +
net/netfilter/ipvs/ip_vs_ctl.c | 8 -----
net/netfilter/ipvs/ip_vs_sync.c | 155 ++++++++++++++++++++++++++++++++++++++++--------------------------------------
net/netfilter/nf_conntrack_expect.c | 5 ++-
net/netfilter/nf_conntrack_extend.c | 2 ++
net/netfilter/nf_conntrack_sip.c | 16 ++++++---
net/netfilter/nf_tables_api.c | 69 +++++++++++++++++++----------------
net/netfilter/xt_connmark.c | 49 ++++++++++++++-----------
net/packet/af_packet.c | 60 ++++++++++++++++++++++---------
net/packet/internal.h | 10 +++---
net/sched/act_ife.c | 9 +++--
net/strparser/strparser.c | 2 +-
tools/testing/selftests/bpf/.gitignore | 3 ++
tools/testing/selftests/bpf/test_sock.c | 1 +
tools/testing/selftests/bpf/test_sock_addr.c | 1 +
tools/testing/selftests/bpf/test_sock_addr.sh | 4 +--
64 files changed, 1163 insertions(+), 463 deletions(-)
^ permalink raw reply
* Re: [PATCH net-next] liquidio: Swap VF representor Tx and Rx statistics
From: David Miller @ 2018-04-24 20:21 UTC (permalink / raw)
To: felix.manlunas
Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla,
srinivasa.jampala
In-Reply-To: <20180424172327.GA7060@felix-thinkpad.cavium.com>
From: Felix Manlunas <felix.manlunas@cavium.com>
Date: Tue, 24 Apr 2018 10:23:27 -0700
> From: Srinivas Jampala <srinivasa.jampala@cavium.com>
>
> Swap VF representor tx and rx interface statistics since it is a
> virtual switchdev port and tx for VM should be rx for VF representor
> and vice-versa.
>
> Signed-off-by: Srinivas Jampala <srinivasa.jampala@cavium.com>
> Acked-by: Derek Chickles <derek.chickles@cavium.com>
> Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next] net/ipv6: fix LOCKDEP issue in rt6_remove_exception_rt()
From: David Miller @ 2018-04-24 20:19 UTC (permalink / raw)
To: edumazet; +Cc: netdev, eric.dumazet, dsahern
In-Reply-To: <20180424162249.41820-1-edumazet@google.com>
From: Eric Dumazet <edumazet@google.com>
Date: Tue, 24 Apr 2018 09:22:49 -0700
> rt6_remove_exception_rt() is called under rcu_read_lock() only.
>
> We lock rt6_exception_lock a bit later, so we do not hold
> rt6_exception_lock yet.
>
> Fixes: 8a14e46f1402 ("net/ipv6: Fix missing rcu dereferences on from")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: syzbot <syzkaller@googlegroups.com>
Applied, thanks Eric.
^ permalink raw reply
* Re: [net 0/6][pull request] Intel Wired LAN Driver Updates 2018-04-24
From: David Miller @ 2018-04-24 20:18 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, nhorman, sassmann, jogreene
In-Reply-To: <1524599024.23142.0.camel@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 24 Apr 2018 12:43:44 -0700
> On Tue, 2018-04-24 at 12:29 -0700, Jeff Kirsher wrote:
>> This series contains fixes to ixgbevf, igb and ice drivers.
>>
>> Colin Ian King fixes the return value on error for the new XDP
>> support
>> that went into ixgbevf for 4.16.
>
> Oops, I meant 4.17, not 4.16.
Pulled with this fixed, thanks Jeff.
^ permalink raw reply
* packetdrill 2.0 release
From: Neal Cardwell @ 2018-04-24 20:13 UTC (permalink / raw)
To: packetdrill, Netdev
Hi All,
We're happy to announce the 2.0 release of the Google version of the
packetdrill network testing tool.
The code may be found at the packetdrill-v2.0 tag in the Google packetdrill
github repo:
https://github.com/google/packetdrill
The commit is here:
https://github.com/google/packetdrill/commit/9a0ade62b7c8e3a19854b5855178dc3bb9d7f453
The 2.0 commit message, summarizing features and contributors, is included
below for a quick overview.
cheers,
neal
---
net-test: packetdrill: merge Google packetdrill changes through April 2018
This commit merges into Google's public packetdrill repository the
majority of the packetdrill tool changes made at Google in the period
2013-2018 (after the initial open source release of packetdrill).
Major features added in this commit include:
+ support for testing:
+ cmsg data
+ TCP send timestamping
+ TCP timestamping opt stats (TCP_NLA_BUSY and friends)
+ TCP zero-copy (e.g. see --send_omit_free)
+ TCP MD5 options
+ TCP urgent pointer field
+ experimental and RFC-compliant TCP Fast Open options
+ ICMP sockets
+ the IPv4 or IPv6 TOS field
+ IPv6 flow labels
+ in IPv6-only environments
+ wider system call support:
+ epoll system calls (epoll_create(), epoll_ctl(), epoll_wait())
+ pipe()
+ splice()
+ cap_set()
+ optional final clean-up commands for destructor-like tear-down
commands that are basically always executed at termination, whether
scripts fail or succeed
+ improved Python support:
+ exporting symbolic names for tcpi_state values
+ exporting recent additions to Linux struct tcp_info
+ exporting TCP_CC_INFO for Vegas, DCTCP, BBR
+ exporting SO_MEMINFO
+ the ability to test shared libraries that support the sockets API,
rather than just the kernel sockets API (see packetdrill.h)
+ preprocessor-style symbol definitions, e.g. -Dfoo=bar
+ support for random local IP addresses
Willem de Bruijn spearheaded this effort to upstream this batch of
changes, and put in a huge amount of work to make this happen. I would
like to thank him for all his work on this.
I would also like to thank the following Googlers for their
contributions over the years to the packetdrill code base, which are
reflected in this patch:
Wei Wang
Maciej Żenczykowski
Yuchung Cheng
Eric Dumazet
Soheil Hassas Yeganeh
Dimitris Michailidis
Willem de Bruijn
Yaogong Wang
Eric Salo
Chonggang Li
Priyaranjan Jha
Andreas Terzis
Xiao Jia
Mike Maloney
Barath Raghavan
Yousuk Seung
Nandita Dukkipati
Michael Davidson
Hsiao-keng Jerry Chu
Greg Thelen
Chema Gonzalez
Luigi Rizzo
Kevin Athey
Jeff Grafton
Francis Y. Yan
Fabien Duchene
Bill Sommerfeld
Anatol Pomazau
This commit has been verified to build cleanly with the default gcc
compiler on the following Linux distributions:
Debian 8
Debian 9
Red Hat Enterprise Linux 7.4
Ubuntu 14.04
Ubuntu 17.10
This commit has not been tested on or ported to any BSD variants, due
to lack of time among members of our team. We are happy to accept
patches to get it to compile/run on popular BSD variants.
^ permalink raw reply
* Re: Boot failures with net-next after rebase to v4.17.0-rc1
From: Linus Torvalds @ 2018-04-24 20:04 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev@vger.kernel.org, LKML, David Miller,
Toke Høiland-Jørgensen, Paul E. McKenney, David Ahern
In-Reply-To: <20180424215429.1de8b1b3@redhat.com>
On Tue, Apr 24, 2018 at 12:54 PM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> Hi all,
>
> I'm experiencing boot failures with net-next git-tree after it got
> rebased/merged with Linus'es tree at v4.17.0-rc1.
I suspect it's the global bit stuff that came in very late in the
merge window, and had been developed and tested for a while before,
but showed some problems under some configs.
The fix is currently in the x86/pti tree in -tip, see:
x86/pti: Fix boot problems from Global-bit setting
and I expect it will percolate upstream soon.
In the meantime, it would be good to verify that merging that x86/pti
branch fixes it for you?
There is another candidate for boot problems - do you happen to have
CONFIG_DEFERRED_STRUCT_PAGE_INIT enabled? That can under certain
circumstances get a percpu setup page fault because memory hadn't been
initialized sufficiently.
The fix there is to move the mm_init() call one step earlier in
init_main(): start_kernel() (to before trap_init()).
And if it's neither of the above, I think you'll need to help bisect it.
Linus
^ permalink raw reply
* Re: [PATCH] net/tls: remove redundant second null check on sgout
From: David Miller @ 2018-04-24 20:02 UTC (permalink / raw)
To: colin.king
Cc: ilyal, aviadye, davejwatson, netdev, kernel-janitors,
linux-kernel
In-Reply-To: <20180424123658.6541-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Tue, 24 Apr 2018 13:36:58 +0100
> From: Colin Ian King <colin.king@canonical.com>
>
> A duplicated null check on sgout is redundant as it is known to be
> already true because of the identical earlier check. Remove it.
> Detected by cppcheck:
>
> net/tls/tls_sw.c:696: (warning) Identical inner 'if' condition is always
> true.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied to net-next, thank you.
^ permalink raw reply
* Re: [PATCH] fsl/fman_port: remove redundant check on port->rev_info.major
From: David Miller @ 2018-04-24 20:01 UTC (permalink / raw)
To: colin.king; +Cc: madalin.bucur, netdev, kernel-janitors, linux-kernel
In-Reply-To: <20180424113945.16371-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Tue, 24 Apr 2018 12:39:45 +0100
> From: Colin Ian King <colin.king@canonical.com>
>
> The check port->rev_info.major >= 6 is being performed twice, thus
> the inner second check is always true and is redundant, hence it
> can be removed. Detected by cppcheck.
>
> drivers/net/ethernet/freescale/fman/fman_port.c:1394]: (warning)
> Identical inner 'if' condition is always true.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied to net-next, thank you.
^ permalink raw reply
* Re: [PATCH net-next v5 0/3] kernel: add support to collect hardware logs in crash recovery kernel
From: David Miller @ 2018-04-24 19:54 UTC (permalink / raw)
To: rahul.lakkireddy
Cc: netdev, kexec, linux-fsdevel, linux-kernel, viro, ebiederm,
stephen, akpm, torvalds, ganeshgr, nirranjan, indranil
In-Reply-To: <cover.1524329561.git.rahul.lakkireddy@chelsio.com>
From: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
Date: Sat, 21 Apr 2018 22:35:52 +0530
> Patch 1 adds API to vmcore module to allow drivers to register callback
> to collect the device specific hardware/firmware logs. The logs will
> be added to /proc/vmcore as elf notes.
>
> Patch 2 updates read and mmap logic to append device specific hardware/
> firmware logs as elf notes.
>
> Patch 3 shows a cxgb4 driver example using the API to collect
> hardware/firmware logs in crash recovery kernel, before hardware is
> initialized.
Are there any serious remaining objections to this series? I'm going to
integrate this into net-next soon if not.
Thank you.
^ permalink raw reply
* Boot failures with net-next after rebase to v4.17.0-rc1
From: Jesper Dangaard Brouer @ 2018-04-24 19:54 UTC (permalink / raw)
To: netdev@vger.kernel.org
Cc: brouer, LKML, David Miller, Toke Høiland-Jørgensen,
Paul E. McKenney, Linus Torvalds, David Ahern
Hi all,
I'm experiencing boot failures with net-next git-tree after it got
rebased/merged with Linus'es tree at v4.17.0-rc1.
The boot problem only occurs for certain kernel configs. I've bisected
the config problem down to enabling CONFIG_PREEMPT=y and resulting
dependencies in below diff.
Is this a know problem?
Have others experienced this too?
This happens for me on two different (x86_64) testlab machines...
I also tested on Linus'es tree at v4.17-rc2, and problem also exists
for me there.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
--- config21-steps-works 2018-04-24 21:33:42.353751894 +0200
+++ config20-steps-bad 2018-04-24 21:27:19.852654328 +0200
@@ -131,7 +131,7 @@
#
# RCU Subsystem
#
-CONFIG_TREE_RCU=y
+CONFIG_PREEMPT_RCU=y
# CONFIG_RCU_EXPERT is not set
CONFIG_SRCU=y
CONFIG_TREE_SRCU=y
@@ -421,11 +421,7 @@
CONFIG_BFQ_GROUP_IOSCHED=y
CONFIG_PREEMPT_NOTIFIERS=y
CONFIG_ASN1=y
-CONFIG_INLINE_SPIN_UNLOCK_IRQ=y
-CONFIG_INLINE_READ_UNLOCK=y
-CONFIG_INLINE_READ_UNLOCK_IRQ=y
-CONFIG_INLINE_WRITE_UNLOCK=y
-CONFIG_INLINE_WRITE_UNLOCK_IRQ=y
+CONFIG_UNINLINE_SPIN_UNLOCK=y
CONFIG_ARCH_SUPPORTS_ATOMIC_RMW=y
CONFIG_MUTEX_SPIN_ON_OWNER=y
CONFIG_RWSEM_SPIN_ON_OWNER=y
@@ -497,9 +493,10 @@
CONFIG_SCHED_SMT=y
CONFIG_SCHED_MC=y
CONFIG_SCHED_MC_PRIO=y
-CONFIG_PREEMPT_NONE=y
+# CONFIG_PREEMPT_NONE is not set
# CONFIG_PREEMPT_VOLUNTARY is not set
-# CONFIG_PREEMPT is not set
+CONFIG_PREEMPT=y
+CONFIG_PREEMPT_COUNT=y
CONFIG_X86_LOCAL_APIC=y
CONFIG_X86_IO_APIC=y
CONFIG_X86_REROUTE_FOR_BROKEN_BOOT_IRQS=y
@@ -3931,6 +3928,7 @@
# CONFIG_SCHEDSTATS is not set
# CONFIG_SCHED_STACK_END_CHECK is not set
# CONFIG_DEBUG_TIMEKEEPING is not set
+# CONFIG_DEBUG_PREEMPT is not set
#
# Lock Debugging (spinlocks, mutexes, etc...)
@@ -3996,6 +3994,7 @@
CONFIG_FUNCTION_GRAPH_TRACER=y
# CONFIG_PREEMPTIRQ_EVENTS is not set
# CONFIG_IRQSOFF_TRACER is not set
+# CONFIG_PREEMPT_TRACER is not set
# CONFIG_SCHED_TRACER is not set
CONFIG_HWLAT_TRACER=y
# CONFIG_FTRACE_SYSCALLS is not set
^ permalink raw reply
* Re: [net 0/6][pull request] Intel Wired LAN Driver Updates 2018-04-24
From: Jeff Kirsher @ 2018-04-24 19:43 UTC (permalink / raw)
To: davem; +Cc: netdev, nhorman, sassmann, jogreene
In-Reply-To: <20180424192911.22786-1-jeffrey.t.kirsher@intel.com>
[-- Attachment #1: Type: text/plain, Size: 2310 bytes --]
On Tue, 2018-04-24 at 12:29 -0700, Jeff Kirsher wrote:
> This series contains fixes to ixgbevf, igb and ice drivers.
>
> Colin Ian King fixes the return value on error for the new XDP
> support
> that went into ixgbevf for 4.16.
Oops, I meant 4.17, not 4.16.
>
> Vinicius provides a fix for queue 0 for igb, which was not receiving
> all
> the credits it needed when QAV mode was enabled.
>
> Anirudh provides several fixes for the new ice driver, starting with
> properly initializing num_nodes_added to zero. Fixed up a code
> comment
> to better reflect what is really going on in the code. Fixed how to
> detect if an OICR interrupt has occurred to a more reliable method.
>
> Md Fahad fixes the ice driver to allocate the right amount of memory
> when reading and storing the devices MAC addresses. The device can
> have
> up to 2 MAC addresses (LAN and WoL), while WoL is currently not
> supported, we need to ensure it can be properly handled when support
> is
> added.
>
> The following are changes since commit
> 9cf2f437ca5b39828984064fad213e68fc17ef11:
> team: fix netconsole setup over team
> and are available in the git repository at:
> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-queue
> 1GbE
>
> Anirudh Venkataramanan (2):
> ice: Fix initialization for num_nodes_added
> ice: Fix incorrect comment for action type
>
> Ben Shelton (1):
> ice: Do not check INTEVENT bit for OICR interrupts
>
> Colin Ian King (1):
> ixgbevf: ensure xdp_ring resources are free'd on error exit
>
> Md Fahad Iqbal Polash (1):
> ice: Fix insufficient memory issue in ice_aq_manage_mac_read
>
> Vinicius Costa Gomes (1):
> igb: Fix the transmission mode of queue 0 for Qav mode
>
> drivers/net/ethernet/intel/ice/ice_adminq_cmd.h | 2 +-
> drivers/net/ethernet/intel/ice/ice_common.c | 22
> +++++++++++++++++-----
> drivers/net/ethernet/intel/ice/ice_hw_autogen.h | 2 --
> drivers/net/ethernet/intel/ice/ice_main.c | 4 ----
> drivers/net/ethernet/intel/ice/ice_sched.c | 4 ++--
> drivers/net/ethernet/intel/igb/igb_main.c | 17
> ++++++++++++++++-
> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 2 +-
> 7 files changed, 37 insertions(+), 16 deletions(-)
>
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 2/2 v1] netns: isolate seqnums to use per-netns locks
From: David Miller @ 2018-04-24 19:39 UTC (permalink / raw)
To: christian.brauner
Cc: ebiederm, netdev, linux-kernel, avagin, ktkhai, serge, gregkh
In-Reply-To: <20180423102443.16627-3-christian.brauner@ubuntu.com>
From: Christian Brauner <christian.brauner@ubuntu.com>
Date: Mon, 23 Apr 2018 12:24:43 +0200
> + #ifdef CONFIG_NET
> + seqnum = get_ns_uevent_seqnum_by_vpid();
> + #else
> + seqnum = uevent_seqnum;
> + #endif
Please don't indend the code like this.
By indenting the CPP directives, which should be at column zero, the
actual code became double indented.
Thank you.
^ permalink raw reply
* [net 6/6] ice: Fix insufficient memory issue in ice_aq_manage_mac_read
From: Jeff Kirsher @ 2018-04-24 19:29 UTC (permalink / raw)
To: davem
Cc: Md Fahad Iqbal Polash, netdev, nhorman, sassmann, jogreene,
Anirudh Venkataramanan, Jeff Kirsher
In-Reply-To: <20180424192911.22786-1-jeffrey.t.kirsher@intel.com>
From: Md Fahad Iqbal Polash <md.fahad.iqbal.polash@intel.com>
For the MAC read operation, the device can return up to two (LAN and WoL)
MAC addresses. Without access to adequate memory, the device will return
an error. Fixed this by allocating the right amount of memory. Also, logic
to detect and copy the LAN MAC address into the port_info structure has
been added. Note that the WoL MAC address is ignored currently as the WoL
feature isn't supported yet.
Fixes: dc49c7723676 ("ice: Get MAC/PHY/link info and scheduler topology")
Signed-off-by: Md Fahad Iqbal Polash <md.fahad.iqbal.polash@intel.com>
Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
Tested-by: Tony Brelinski <tonyx.brelinski@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ice/ice_common.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index 21977ec984c4..71d032cc5fa7 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -78,6 +78,7 @@ ice_aq_manage_mac_read(struct ice_hw *hw, void *buf, u16 buf_size,
struct ice_aq_desc desc;
enum ice_status status;
u16 flags;
+ u8 i;
cmd = &desc.params.mac_read;
@@ -98,8 +99,16 @@ ice_aq_manage_mac_read(struct ice_hw *hw, void *buf, u16 buf_size,
return ICE_ERR_CFG;
}
- ether_addr_copy(hw->port_info->mac.lan_addr, resp->mac_addr);
- ether_addr_copy(hw->port_info->mac.perm_addr, resp->mac_addr);
+ /* A single port can report up to two (LAN and WoL) addresses */
+ for (i = 0; i < cmd->num_addr; i++)
+ if (resp[i].addr_type == ICE_AQC_MAN_MAC_ADDR_TYPE_LAN) {
+ ether_addr_copy(hw->port_info->mac.lan_addr,
+ resp[i].mac_addr);
+ ether_addr_copy(hw->port_info->mac.perm_addr,
+ resp[i].mac_addr);
+ break;
+ }
+
return 0;
}
@@ -464,9 +473,12 @@ enum ice_status ice_init_hw(struct ice_hw *hw)
if (status)
goto err_unroll_sched;
- /* Get port MAC information */
- mac_buf_len = sizeof(struct ice_aqc_manage_mac_read_resp);
- mac_buf = devm_kzalloc(ice_hw_to_dev(hw), mac_buf_len, GFP_KERNEL);
+ /* Get MAC information */
+ /* A single port can report up to two (LAN and WoL) addresses */
+ mac_buf = devm_kcalloc(ice_hw_to_dev(hw), 2,
+ sizeof(struct ice_aqc_manage_mac_read_resp),
+ GFP_KERNEL);
+ mac_buf_len = 2 * sizeof(struct ice_aqc_manage_mac_read_resp);
if (!mac_buf) {
status = ICE_ERR_NO_MEMORY;
--
2.14.3
^ permalink raw reply related
* [net 5/6] ice: Do not check INTEVENT bit for OICR interrupts
From: Jeff Kirsher @ 2018-04-24 19:29 UTC (permalink / raw)
To: davem
Cc: Ben Shelton, netdev, nhorman, sassmann, jogreene,
Anirudh Venkataramanan, Jeff Kirsher
In-Reply-To: <20180424192911.22786-1-jeffrey.t.kirsher@intel.com>
From: Ben Shelton <benjamin.h.shelton@intel.com>
According to the hardware spec, checking the INTEVENT bit isn't a
reliable way to detect if an OICR interrupt has occurred. This is
because this bit can be cleared by the hardware/firmware before the
interrupt service routine has run. So instead, just check for OICR
events every time.
Fixes: 940b61af02f4 ("ice: Initialize PF and setup miscellaneous interrupt")
Signed-off-by: Ben Shelton <benjamin.h.shelton@intel.com>
Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
Tested-by: Tony Brelinski <tonyx.brelinski@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ice/ice_hw_autogen.h | 2 --
drivers/net/ethernet/intel/ice/ice_main.c | 4 ----
2 files changed, 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_hw_autogen.h b/drivers/net/ethernet/intel/ice/ice_hw_autogen.h
index 1b9e2ef48a9d..499904874b3f 100644
--- a/drivers/net/ethernet/intel/ice/ice_hw_autogen.h
+++ b/drivers/net/ethernet/intel/ice/ice_hw_autogen.h
@@ -121,8 +121,6 @@
#define PFINT_FW_CTL_CAUSE_ENA_S 30
#define PFINT_FW_CTL_CAUSE_ENA_M BIT(PFINT_FW_CTL_CAUSE_ENA_S)
#define PFINT_OICR 0x0016CA00
-#define PFINT_OICR_INTEVENT_S 0
-#define PFINT_OICR_INTEVENT_M BIT(PFINT_OICR_INTEVENT_S)
#define PFINT_OICR_HLP_RDY_S 14
#define PFINT_OICR_HLP_RDY_M BIT(PFINT_OICR_HLP_RDY_S)
#define PFINT_OICR_CPM_RDY_S 15
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 210b7910f1cd..5299caf55a7f 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -1722,9 +1722,6 @@ static irqreturn_t ice_misc_intr(int __always_unused irq, void *data)
oicr = rd32(hw, PFINT_OICR);
ena_mask = rd32(hw, PFINT_OICR_ENA);
- if (!(oicr & PFINT_OICR_INTEVENT_M))
- goto ena_intr;
-
if (oicr & PFINT_OICR_GRST_M) {
u32 reset;
/* we have a reset warning */
@@ -1782,7 +1779,6 @@ static irqreturn_t ice_misc_intr(int __always_unused irq, void *data)
}
ret = IRQ_HANDLED;
-ena_intr:
/* re-enable interrupt causes that are not handled during this pass */
wr32(hw, PFINT_OICR_ENA, ena_mask);
if (!test_bit(__ICE_DOWN, pf->state)) {
--
2.14.3
^ permalink raw reply related
* [net 2/6] igb: Fix the transmission mode of queue 0 for Qav mode
From: Jeff Kirsher @ 2018-04-24 19:29 UTC (permalink / raw)
To: davem
Cc: Vinicius Costa Gomes, netdev, nhorman, sassmann, jogreene,
Jeff Kirsher
In-Reply-To: <20180424192911.22786-1-jeffrey.t.kirsher@intel.com>
From: Vinicius Costa Gomes <vinicius.gomes@intel.com>
When Qav mode is enabled, queue 0 should be kept on Stream Reservation
mode. From the i210 datasheet, section 8.12.19:
"Note: Queue0 QueueMode must be set to 1b when TransmitMode is set to
Qav." ("QueueMode 1b" represents the Stream Reservation mode)
The solution is to give queue 0 the all the credits it might need, so
it has priority over queue 1.
A situation where this can happen is when cbs is "installed" only on
queue 1, leaving queue 0 alone. For example:
$ tc qdisc replace dev enp2s0 handle 100: parent root mqprio num_tc 3 \
map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 queues 1@0 1@1 2@2 hw 0
$ tc qdisc replace dev enp2s0 parent 100:2 cbs locredit -1470 \
hicredit 30 sendslope -980000 idleslope 20000 offload 1
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/igb_main.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index c1c0bc30a16d..cce7ada89255 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -1700,7 +1700,22 @@ static void igb_configure_cbs(struct igb_adapter *adapter, int queue,
WARN_ON(hw->mac.type != e1000_i210);
WARN_ON(queue < 0 || queue > 1);
- if (enable) {
+ if (enable || queue == 0) {
+ /* i210 does not allow the queue 0 to be in the Strict
+ * Priority mode while the Qav mode is enabled, so,
+ * instead of disabling strict priority mode, we give
+ * queue 0 the maximum of credits possible.
+ *
+ * See section 8.12.19 of the i210 datasheet, "Note:
+ * Queue0 QueueMode must be set to 1b when
+ * TransmitMode is set to Qav."
+ */
+ if (queue == 0 && !enable) {
+ /* max "linkspeed" idleslope in kbps */
+ idleslope = 1000000;
+ hicredit = ETH_FRAME_LEN;
+ }
+
set_tx_desc_fetch_prio(hw, queue, TX_QUEUE_PRIO_HIGH);
set_queue_mode(hw, queue, QUEUE_MODE_STREAM_RESERVATION);
--
2.14.3
^ permalink raw reply related
* [net 0/6][pull request] Intel Wired LAN Driver Updates 2018-04-24
From: Jeff Kirsher @ 2018-04-24 19:29 UTC (permalink / raw)
To: davem; +Cc: Jeff Kirsher, netdev, nhorman, sassmann, jogreene
This series contains fixes to ixgbevf, igb and ice drivers.
Colin Ian King fixes the return value on error for the new XDP support
that went into ixgbevf for 4.16.
Vinicius provides a fix for queue 0 for igb, which was not receiving all
the credits it needed when QAV mode was enabled.
Anirudh provides several fixes for the new ice driver, starting with
properly initializing num_nodes_added to zero. Fixed up a code comment
to better reflect what is really going on in the code. Fixed how to
detect if an OICR interrupt has occurred to a more reliable method.
Md Fahad fixes the ice driver to allocate the right amount of memory
when reading and storing the devices MAC addresses. The device can have
up to 2 MAC addresses (LAN and WoL), while WoL is currently not
supported, we need to ensure it can be properly handled when support is
added.
The following are changes since commit 9cf2f437ca5b39828984064fad213e68fc17ef11:
team: fix netconsole setup over team
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-queue 1GbE
Anirudh Venkataramanan (2):
ice: Fix initialization for num_nodes_added
ice: Fix incorrect comment for action type
Ben Shelton (1):
ice: Do not check INTEVENT bit for OICR interrupts
Colin Ian King (1):
ixgbevf: ensure xdp_ring resources are free'd on error exit
Md Fahad Iqbal Polash (1):
ice: Fix insufficient memory issue in ice_aq_manage_mac_read
Vinicius Costa Gomes (1):
igb: Fix the transmission mode of queue 0 for Qav mode
drivers/net/ethernet/intel/ice/ice_adminq_cmd.h | 2 +-
drivers/net/ethernet/intel/ice/ice_common.c | 22 +++++++++++++++++-----
drivers/net/ethernet/intel/ice/ice_hw_autogen.h | 2 --
drivers/net/ethernet/intel/ice/ice_main.c | 4 ----
drivers/net/ethernet/intel/ice/ice_sched.c | 4 ++--
drivers/net/ethernet/intel/igb/igb_main.c | 17 ++++++++++++++++-
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 2 +-
7 files changed, 37 insertions(+), 16 deletions(-)
--
2.14.3
^ permalink raw reply
* [net 1/6] ixgbevf: ensure xdp_ring resources are free'd on error exit
From: Jeff Kirsher @ 2018-04-24 19:29 UTC (permalink / raw)
To: davem; +Cc: Colin Ian King, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20180424192911.22786-1-jeffrey.t.kirsher@intel.com>
From: Colin Ian King <colin.king@canonical.com>
The current error handling for failed resource setup for xdp_ring
data is a break out of the loop and returning 0 indicated everything
was OK, when in fact it is not. Fix this by exiting via the
error exit label err_setup_tx that will clean up the resources
correctly and return and error status.
Detected by CoverityScan, CID#1466879 ("Logically dead code")
Fixes: 21092e9ce8b1 ("ixgbevf: Add support for XDP_TX action")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 3d9033f26eff..e3d04f226d57 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -3420,7 +3420,7 @@ static int ixgbevf_setup_all_tx_resources(struct ixgbevf_adapter *adapter)
if (!err)
continue;
hw_dbg(&adapter->hw, "Allocation for XDP Queue %u failed\n", j);
- break;
+ goto err_setup_tx;
}
return 0;
--
2.14.3
^ permalink raw reply related
* [net 4/6] ice: Fix incorrect comment for action type
From: Jeff Kirsher @ 2018-04-24 19:29 UTC (permalink / raw)
To: davem
Cc: Anirudh Venkataramanan, netdev, nhorman, sassmann, jogreene,
Jeff Kirsher
In-Reply-To: <20180424192911.22786-1-jeffrey.t.kirsher@intel.com>
From: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
Action type 5 defines large action generic values. Fix comment to
reflect that better.
Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
Tested-by: Tony Brelinski <tonyx.brelinski@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ice/ice_adminq_cmd.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
index 5b13ca1bd85f..7dc5f045e969 100644
--- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
@@ -586,7 +586,7 @@ struct ice_sw_rule_lg_act {
#define ICE_LG_ACT_MIRROR_VSI_ID_S 3
#define ICE_LG_ACT_MIRROR_VSI_ID_M (0x3FF << ICE_LG_ACT_MIRROR_VSI_ID_S)
- /* Action type = 5 - Large Action */
+ /* Action type = 5 - Generic Value */
#define ICE_LG_ACT_GENERIC 0x5
#define ICE_LG_ACT_GENERIC_VALUE_S 3
#define ICE_LG_ACT_GENERIC_VALUE_M (0xFFFF << ICE_LG_ACT_GENERIC_VALUE_S)
--
2.14.3
^ permalink raw reply related
* [net 3/6] ice: Fix initialization for num_nodes_added
From: Jeff Kirsher @ 2018-04-24 19:29 UTC (permalink / raw)
To: davem
Cc: Anirudh Venkataramanan, netdev, nhorman, sassmann, jogreene,
Jeff Kirsher
In-Reply-To: <20180424192911.22786-1-jeffrey.t.kirsher@intel.com>
From: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
ice_sched_add_nodes_to_layer is used recursively, and so we start
with num_nodes_added being 0. This way, in case of an error or if
num_nodes is NULL, the function just returns 0 to indicate that no
nodes were added.
Fixes: 5513b920a4f7 ("ice: Update Tx scheduler tree for VSI multi-Tx queue support")
Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
Tested-by: Tony Brelinski <tonyx.brelinski@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ice/ice_sched.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_sched.c b/drivers/net/ethernet/intel/ice/ice_sched.c
index f16ff3e4a840..2e6c1d92cc88 100644
--- a/drivers/net/ethernet/intel/ice/ice_sched.c
+++ b/drivers/net/ethernet/intel/ice/ice_sched.c
@@ -751,14 +751,14 @@ ice_sched_add_nodes_to_layer(struct ice_port_info *pi,
u16 num_added = 0;
u32 temp;
+ *num_nodes_added = 0;
+
if (!num_nodes)
return status;
if (!parent || layer < hw->sw_entry_point_layer)
return ICE_ERR_PARAM;
- *num_nodes_added = 0;
-
/* max children per node per layer */
max_child_nodes =
le16_to_cpu(hw->layer_info[parent->tx_sched_layer].max_children);
--
2.14.3
^ permalink raw reply related
* Re: [PATCH net-next v2 0/2] openvswitch: Support conntrack zone limit
From: David Miller @ 2018-04-24 19:03 UTC (permalink / raw)
To: yihung.wei; +Cc: pshelar, netdev, fw
In-Reply-To: <CAG1aQhKtZ_4AYuKBTzEwG1YwUr9sFchcyh+eWXB_i64GSW_Z8A@mail.gmail.com>
From: Yi-Hung Wei <yihung.wei@gmail.com>
Date: Tue, 24 Apr 2018 11:21:33 -0700
> On Tue, Apr 24, 2018 at 10:42 AM, David Miller <davem@davemloft.net> wrote:
>> From: Pravin Shelar <pshelar@ovn.org>
>> Date: Mon, 23 Apr 2018 23:34:48 -0700
>>
>>> OK. Thanks for the info.
>>
>> So, ACK, Reviewed-by, etc.? :-)
>>
>
> Parvin provides feedback in a previous email. I will address them and
> send out v3.
Aha, I see, thanks for explaining.
^ permalink raw reply
* Re: [RFC PATCH ghak32 V2 01/13] audit: add container id
From: Paul Moore @ 2018-04-24 19:01 UTC (permalink / raw)
To: Richard Guy Briggs
Cc: cgroups, containers, linux-api, Linux-Audit Mailing List,
linux-fsdevel, LKML, netdev, ebiederm, luto, jlayton, carlos,
dhowells, viro, simo, Eric Paris, serge
In-Reply-To: <20180424020200.imonhbkwtb73luxl@madcap2.tricolour.ca>
On Mon, Apr 23, 2018 at 10:02 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2018-04-23 19:15, Paul Moore wrote:
>> On Sat, Apr 21, 2018 at 10:34 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
>> > On 2018-04-18 19:47, Paul Moore wrote:
>> >> On Fri, Mar 16, 2018 at 5:00 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
>> >> > Implement the proc fs write to set the audit container ID of a process,
>> >> > emitting an AUDIT_CONTAINER record to document the event.
>> >> >
>> >> > This is a write from the container orchestrator task to a proc entry of
>> >> > the form /proc/PID/containerid where PID is the process ID of the newly
>> >> > created task that is to become the first task in a container, or an
>> >> > additional task added to a container.
>> >> >
>> >> > The write expects up to a u64 value (unset: 18446744073709551615).
>> >> >
>> >> > This will produce a record such as this:
>> >> > type=CONTAINER msg=audit(1519903238.968:261): op=set pid=596 uid=0 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 auid=0 tty=pts0 ses=1 opid=596 old-contid=18446744073709551615 contid=123455 res=0
>> >> >
>> >> > The "op" field indicates an initial set. The "pid" to "ses" fields are
>> >> > the orchestrator while the "opid" field is the object's PID, the process
>> >> > being "contained". Old and new container ID values are given in the
>> >> > "contid" fields, while res indicates its success.
>> >> >
>> >> > It is not permitted to self-set, unset or re-set the container ID. A
>> >> > child inherits its parent's container ID, but then can be set only once
>> >> > after.
>> >> >
>> >> > See: https://github.com/linux-audit/audit-kernel/issues/32
>> >> >
>> >> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
>> >> > ---
>> >> > fs/proc/base.c | 37 ++++++++++++++++++++
>> >> > include/linux/audit.h | 16 +++++++++
>> >> > include/linux/init_task.h | 4 ++-
>> >> > include/linux/sched.h | 1 +
>> >> > include/uapi/linux/audit.h | 2 ++
>> >> > kernel/auditsc.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++
>> >> > 6 files changed, 143 insertions(+), 1 deletion(-)
...
>> >> > /* audit_rule_data supports filter rules with both integer and string
>> >> > * fields. It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and
>> >> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
>> >> > index 4e0a4ac..29c8482 100644
>> >> > --- a/kernel/auditsc.c
>> >> > +++ b/kernel/auditsc.c
>> >> > @@ -2073,6 +2073,90 @@ int audit_set_loginuid(kuid_t loginuid)
>> >> > return rc;
>> >> > }
>> >> >
>> >> > +static int audit_set_containerid_perm(struct task_struct *task, u64 containerid)
>> >> > +{
>> >> > + struct task_struct *parent;
>> >> > + u64 pcontainerid, ccontainerid;
>> >> > +
>> >> > + /* Don't allow to set our own containerid */
>> >> > + if (current == task)
>> >> > + return -EPERM;
>> >>
>> >> Why not? Is there some obvious security concern that I missing?
>> >
>> > We then lose the distinction in the AUDIT_CONTAINER record between the
>> > initiating PID and the target PID. This was outlined in the proposal.
>>
>> I just went back and reread the v3 proposal and I still don't see a
>> good explanation of this. Why is this bad? What's the security
>> concern?
>
> I don't remember, specifically. Maybe this has been addressed by the
> check for children/threads or identical parent container ID. So, I'm
> reluctantly willing to remove that check for now.
Okay. For the record, if someone can explain to me why this
restriction saves us from some terrible situation I'm all for leaving
it. I'm just opposed to restrictions without solid reasoning behind
them.
>> > Having said that, I'm still not sure we have protected sufficiently from
>> > a child turning around and setting it's parent's as yet unset or
>> > inherited audit container ID.
>>
>> Yes, I believe we only want to let a task set the audit container for
>> it's children (or itself/threads if we decide to allow that, see
>> above). There *has* to be a function to check to see if a task if a
>> child of a given task ... right? ... although this is likely to be a
>> pointer traversal and locking nightmare ... hmmm.
>
> Isn't that just (struct task_struct)parent == (struct
> task_struct)child->parent (or ->real_parent)?
>
> And now that I say that, it is covered by the following patch's child
> check, so as long as we keep that, we should be fine.
I was thinking of checking not just current's immediate children, but
any of it's descendants as I believe that is what we want to limit,
yes? I just worry that it isn't really practical to perform that
check.
>> >> I ask because I suppose it might be possible for some container
>> >> runtime to do a fork, setup some of the environment and them exec the
>> >> container (before you answer the obvious "namespaces!" please remember
>> >> we're not trying to define containers).
>> >
>> > I don't think namespaces have any bearing on this concern since none are
>> > required.
>> >
>> >> > + /* Don't allow the containerid to be unset */
>> >> > + if (!cid_valid(containerid))
>> >> > + return -EINVAL;
>> >> > + /* if we don't have caps, reject */
>> >> > + if (!capable(CAP_AUDIT_CONTROL))
>> >> > + return -EPERM;
>> >> > + /* if containerid is unset, allow */
>> >> > + if (!audit_containerid_set(task))
>> >> > + return 0;
>> >> > + /* it is already set, and not inherited from the parent, reject */
>> >> > + ccontainerid = audit_get_containerid(task);
>> >> > + rcu_read_lock();
>> >> > + parent = rcu_dereference(task->real_parent);
>> >> > + rcu_read_unlock();
>> >> > + task_lock(parent);
>> >> > + pcontainerid = audit_get_containerid(parent);
>> >> > + task_unlock(parent);
>> >> > + if (ccontainerid != pcontainerid)
>> >> > + return -EPERM;
>> >> > + return 0;
I'm looking at the parent checks again and I wonder if the logic above
is what we really want. Maybe it is, but I'm not sure.
Things I'm wondering about:
* "ccontainerid" and "containerid" are too close in name, I kept
confusing myself when looking at this code. Please change one. Bonus
points if it is shorter.
* What if the orchestrator wants to move the task to a new container?
Right now it looks like you can only do that once, then then the
task's audit container ID will no longer be the same as real_parent
... or does the orchestrator change that? *Can* the orchestrator
change real_parent (I suspect the answer is "no")?
* I think the key is the relationship between current and task, not
between task and task->real_parent. I believe what we really care
about is that task is a descendant of current. We might also want to
allow current to change the audit container ID if it holds
CAP_AUDIT_CONTROL, regardless of it's relationship with task.
>> >> > +static void audit_log_set_containerid(struct task_struct *task, u64 oldcontainerid,
>> >> > + u64 containerid, int rc)
>> >> > +{
>> >> > + struct audit_buffer *ab;
>> >> > + uid_t uid;
>> >> > + struct tty_struct *tty;
>> >> > +
>> >> > + if (!audit_enabled)
>> >> > + return;
>> >> > +
>> >> > + ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONTAINER);
>> >> > + if (!ab)
>> >> > + return;
>> >> > +
>> >> > + uid = from_kuid(&init_user_ns, task_uid(current));
>> >> > + tty = audit_get_tty(current);
>> >> > +
>> >> > + audit_log_format(ab, "op=set pid=%d uid=%u", task_tgid_nr(current), uid);
>> >> > + audit_log_task_context(ab);
>> >> > + audit_log_format(ab, " auid=%u tty=%s ses=%u opid=%d old-contid=%llu contid=%llu res=%d",
>> >> > + from_kuid(&init_user_ns, audit_get_loginuid(current)),
>> >> > + tty ? tty_name(tty) : "(none)", audit_get_sessionid(current),
>> >> > + task_tgid_nr(task), oldcontainerid, containerid, !rc);
>> >> > +
>> >> > + audit_put_tty(tty);
>> >> > + audit_log_end(ab);
>> >> > +}
>> >> > +
>> >> > +/**
>> >> > + * audit_set_containerid - set current task's audit_context containerid
>> >> > + * @containerid: containerid value
>> >> > + *
>> >> > + * Returns 0 on success, -EPERM on permission failure.
>> >> > + *
>> >> > + * Called (set) from fs/proc/base.c::proc_containerid_write().
>> >> > + */
>> >> > +int audit_set_containerid(struct task_struct *task, u64 containerid)
>> >> > +{
>> >> > + u64 oldcontainerid;
>> >> > + int rc;
>> >> > +
>> >> > + oldcontainerid = audit_get_containerid(task);
>> >> > +
>> >> > + rc = audit_set_containerid_perm(task, containerid);
>> >> > + if (!rc) {
>> >> > + task_lock(task);
>> >> > + task->containerid = containerid;
>> >> > + task_unlock(task);
>> >> > + }
>> >> > +
>> >> > + audit_log_set_containerid(task, oldcontainerid, containerid, rc);
>> >> > + return rc;
>> >>
>> >> Why are audit_set_containerid_perm() and audit_log_containerid()
>> >> separate functions?
>> >
>> > (I assume you mean audit_log_set_containerid()?)
>>
>> Yep. My fingers got tired typing in that function name and decided a
>> shortcut was necessary.
>>
>> > It seemed clearer that all the permission checking was in one function
>> > and its return code could be used to report the outcome when logging the
>> > (attempted) action. This is the same structure as audit_set_loginuid()
>> > and it made sense.
>>
>> When possible I really like it when the permission checks are in the
>> same function as the code which does the work; it's less likely to get
>> abused that way (you have to willfully bypass the access checks). The
>> exceptions might be if you wanted to reuse the access control code, or
>> insert a modular access mechanism (e.g. LSMs).
>
> I don't follow how it could be abused. The return code from the perm
> check gates setting the value and is used in the success field in the
> log.
If the permission checks are in the same function body as the code
which does the work you have to either split the function, or rewrite
it, if you want to bypass the permission checks. It may be more of a
style issue than an actual safety issue, but the comments about
single-use functions in the same scope is the tie breaker.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH bpf-next 08/15] bpf: introduce new bpf AF_XDP map type BPF_MAP_TYPE_XSKMAP
From: Björn Töpel @ 2018-04-24 18:58 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Karlsson, Magnus, Alexander Duyck, Alexander Duyck,
John Fastabend, Alexei Starovoitov, Jesper Dangaard Brouer,
Daniel Borkmann, Michael S. Tsirkin, Network Development,
Björn Töpel, michael.lundkvist, Brandeburg, Jesse,
Singhai, Anjali, Zhang, Qi Z
In-Reply-To: <CAF=yD-+W8=wDXc1=wHi8KF0whgyFLqvo=tROOq16XfA7MDkR+Q@mail.gmail.com>
2018-04-24 18:56 GMT+02:00 Willem de Bruijn <willemdebruijn.kernel@gmail.com>:
> On Mon, Apr 23, 2018 at 9:56 AM, Björn Töpel <bjorn.topel@gmail.com> wrote:
>> From: Björn Töpel <bjorn.topel@intel.com>
>>
>> The xskmap is yet another BPF map, very much inspired by
>> dev/cpu/sockmap, and is a holder of AF_XDP sockets. A user application
>> adds AF_XDP sockets into the map, and by using the bpf_redirect_map
>> helper, an XDP program can redirect XDP frames to an AF_XDP socket.
>>
>> Note that a socket that is bound to certain ifindex/queue index will
>> *only* accept XDP frames from that netdev/queue index. If an XDP
>> program tries to redirect from a netdev/queue index other than what
>> the socket is bound to, the frame will not be received on the socket.
>>
>> A socket can reside in multiple maps.
>>
>> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
>
>> +struct xsk_map_entry {
>> + struct xdp_sock *xs;
>> + struct rcu_head rcu;
>> +};
>
>> +struct xdp_sock *__xsk_map_lookup_elem(struct bpf_map *map, u32 key)
>> +{
>> + struct xsk_map *m = container_of(map, struct xsk_map, map);
>> + struct xsk_map_entry *entry;
>> +
>> + if (key >= map->max_entries)
>> + return NULL;
>> +
>> + entry = READ_ONCE(m->xsk_map[key]);
>> + return entry ? entry->xs : NULL;
>> +}
>
> This dynamically allocated structure adds an extra cacheline lookup. If
> xdp_sock gets an rcu_head, it can be linked into the map directly.
Nice one! I'll try this out!
^ permalink raw reply
* Re: [PATCH net-next 5/8] net: mscc: Add initial Ocelot switch support
From: Alexandre Belloni @ 2018-04-24 18:52 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, David S . Miller, Allan Nielsen,
razvan.stefanescu, po.liu, Thomas Petazzoni, netdev, devicetree,
linux-kernel, linux-mips
In-Reply-To: <20180330145008.GE28244@lunn.ch>
I realise now that I didn't reply to this comment:
On 30/03/2018 16:50:08+0200, Andrew Lunn wrote:
> > The fact is that ocelot doesn't have separate controls. The port is
> > either forwarding or not. If it is not forwarding, then there is nothing
> > to tell the HW to do.
>
> Think about the following sequence:
>
> ip link set lan0 up
>
> After this command, i expect to see packets on lan0 arrive at the
> host, tcpdump to work, etc. This probably means the port is in
> 'forwarding' mode, or for B53, STP is disabled.
>
On Ocelot, forwarding packets to the host (i.e. forwarding frames
received on the port to the cpu port) is separate from bridging ports
together. So after that command, the host can receive packets on lan0.
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
^ 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