* VLAN/bridge "compression" in wifi (was: Re: [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support)
From: Johannes Berg @ 2017-09-05 14:20 UTC (permalink / raw)
To: Sergey Matyukevich, linux-wireless-u79uwXL29TY76Z2rM5mHXA, netdev
Cc: Igor Mitsyanko, Avinash Patil
In-Reply-To: <1504619151.12380.16.camel-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
+netdev
On Tue, 2017-09-05 at 15:45 +0200, Johannes Berg wrote:
>
> In a way this feature seems mis-designed - you never have 802.1Q tags
> over the air, but you're inserting them on RX and stripping them on
> TX, probably in order to make bridging to ethernet easier and not
> have to have 802.1Q acceleration on the ethernet port, or - well - in
> order to have an ability to do this with an ethernet card that only
> has a single CPU port.
Ok this isn't really right either - it's only for saving the 802.1Q
acceleration on the Ethernet port, really - and saving the extra
bridges.
To clarify, I think what you - conceptually - want is the following
topology:
+--- eth0.1 --- br.1 --- wlan0.1
|
eth0 ---+--- eth0.2 --- br.2 --- wlan0.2
|
+--- eth0.3 --- br.3 --- wlan0.3
where eth0.N is just "ip link add link eth0 name eth0.N type vlan id N"
and br.N is obviously a bridge for each, and the wlan0.N are AP_VLAN
type interfaces that isolate the clients against each other as far as
wifi is concerned.
Is this correct? As far as I understand, that's the baseline topology
that you're trying to achieve, expressed in terms of Linux networking.
Now, you seem to want to compress this to
+--- wlan0.1
|
eth0 --- br ---+--- wlan0.2
|
+--- wlan0.3
and have the 802.1Q tag insertion/removal that's normally configured to
happen in eth0.N already be handled in wlan0.N.
Also correct?
We clearly don't have APIs for this, and I don't think it makes sense
in the Linux space - the bridge and wlan0.N suddenly have tagged
traffic rather than untagged, and the VLAN tagging is completely hidden
from the management view.
johannes
^ permalink raw reply
* Re: [RFC net-next] net: sch_clsact: add support for global per-netns classifier mode
From: Jiri Pirko @ 2017-09-05 14:23 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netdev, roopa, dsa, xiyou.wangcong, jhs
In-Reply-To: <20170905140750.GB7936@nanopsycho>
Tue, Sep 05, 2017 at 04:07:51PM CEST, jiri@resnulli.us wrote:
>Tue, Sep 05, 2017 at 02:48:21PM CEST, nikolay@cumulusnetworks.com wrote:
>>Hi all,
>>This RFC adds a new mode for clsact which designates a device's egress
>>classifier as global per netns. The packets that are not classified for
>>a particular device will be classified using the global classifier.
>>We have needed a global classifier for some time now for various
>>purposes and setting the single bridge or loopback/vrf device as the
>>global classifier device is acceptable for us. Doing it this way avoids
>>the act/cls device and queue dependencies.
>>
>>This is strictly an RFC patch just to show the intent, if we agree on
>>the details the proposed patch will have support for both ingress and
>>egress, and will be using a static key to avoid the fast path test when no
>>global classifier has been configured.
>>
>>Example (need a modified tc that adds TCA_OPTIONS when using q_clsact):
>>$ tc qdisc add dev lo clsact global
>>$ tc filter add dev lo egress protocol ip u32 match ip dst 4.3.2.1/32 action drop
>>
>>the last filter will be global for all devices that don't have a
>>specific egress_cl_list (i.e. have clsact configured).
>>
>>Any comments and thoughts would be greatly appreciated.
For the record, I think this "global" thing is a hack similar to
cls_u32 shared hashlists.
>
>Did you see my shared blocks work? I believe that it should resolve your
>usecase, in a generic way. You just have to bind the devices you need to
>the shared block. Please see the RFC:
>
>https://www.spinics.net/lists/netdev/msg444067.html
^ permalink raw reply
* Re: [oss-drivers] Re: [PATCH net] net: sched: don't use GFP_KERNEL under spin lock
From: Jakub Kicinski @ 2017-09-05 14:25 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, oss-drivers
In-Reply-To: <20170905125221.GA7936@nanopsycho>
On Tue, 5 Sep 2017 14:52:21 +0200, Jiri Pirko wrote:
> Tue, Sep 05, 2017 at 12:59:48PM CEST, kubakici@wp.pl wrote:
> >Jiri, FWIW I also just noticed these, but they seem related to the
> >earlier TC chain work?
>
> What do you do to trigger this?
Sorry, I wasted a bit of time there because kmemleak takes surprisingly
long to notice the memory leak... In the end this is sufficient:
modprobe nfp
ifconfig p4p1 10.8.1.1/24 up
tc qdisc add dev p4p1 handle ffff: ingress
tc filter add dev p4p1 parent ffff: protocol 802.1Q flower vlan_id 600 \
action mirred egress redirect dev p4p1
tc filter del dev p4p1 parent ffff:
rmmod nfp
^ permalink raw reply
* Re: [oss-drivers] Re: [PATCH net] net: sched: don't use GFP_KERNEL under spin lock
From: Jiri Pirko @ 2017-09-05 14:31 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, oss-drivers
In-Reply-To: <20170905152551.748e4af2@cakuba.netronome.com>
Tue, Sep 05, 2017 at 04:25:51PM CEST, kubakici@wp.pl wrote:
>On Tue, 5 Sep 2017 14:52:21 +0200, Jiri Pirko wrote:
>> Tue, Sep 05, 2017 at 12:59:48PM CEST, kubakici@wp.pl wrote:
>> >Jiri, FWIW I also just noticed these, but they seem related to the
>> >earlier TC chain work?
>>
>> What do you do to trigger this?
>
>Sorry, I wasted a bit of time there because kmemleak takes surprisingly
>long to notice the memory leak... In the end this is sufficient:
>
> modprobe nfp
> ifconfig p4p1 10.8.1.1/24 up
> tc qdisc add dev p4p1 handle ffff: ingress
> tc filter add dev p4p1 parent ffff: protocol 802.1Q flower vlan_id 600 \
> action mirred egress redirect dev p4p1
> tc filter del dev p4p1 parent ffff:
> rmmod nfp
Will try. Thanks!
^ permalink raw reply
* Re: [PATCH net-next] bpf: fix numa_node validation
From: Alexei Starovoitov @ 2017-09-05 14:43 UTC (permalink / raw)
To: Eric Dumazet, David Miller; +Cc: netdev, Martin KaFai Lau, Daniel Borkmann
In-Reply-To: <1504590062.15310.36.camel@edumazet-glaptop3.roam.corp.google.com>
On 9/4/17 10:41 PM, Eric Dumazet wrote:
> Call Trace:
> __do_kmalloc_node mm/slab.c:3688 [inline]
> __kmalloc_node+0x33/0x70 mm/slab.c:3696
> kmalloc_node include/linux/slab.h:535 [inline]
> alloc_htab_elem+0x2a8/0x480 kernel/bpf/hashtab.c:740
> htab_map_update_elem+0x740/0xb80 kernel/bpf/hashtab.c:820
> map_update_elem kernel/bpf/syscall.c:587 [inline]
> SYSC_bpf kernel/bpf/syscall.c:1468 [inline]
> SyS_bpf+0x20c5/0x4c40 kernel/bpf/syscall.c:1443
> entry_SYSCALL_64_fastpath+0x1f/0xbe
> RIP: 0033:0x440409
> RSP: 002b:00007ffd1f1792b8 EFLAGS: 00000246 ORIG_RAX: 0000000000000141
> RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 0000000000440409
> RDX: 0000000000000020 RSI: 0000000020006000 RDI: 0000000000000002
> RBP: 0000000000000086 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000401d70
> R13: 0000000000401e00 R14: 0000000000000000 R15: 0000000000000000
> Code: 83 c2 01 89 50 18 4c 03 70 08 e8 38 f4 ff ff 4d 85 f6 0f 85 3e ff ff ff 44 89 fe 4c 89 ef e8 94 fb ff ff 49 89 c6 e9 2b ff ff ff <0f> 0b 0f 0b 0f 0b 66 0f 1f 44 00 00 55 48 89 e5 41 57 41 56 41
> RIP: ____cache_alloc_node+0x1d4/0x1e0 mm/slab.c:3292 RSP: ffff8801c0c97638
> ---[ end trace d745f355da2e33ce ]---
> Kernel panic - not syncing: Fatal exception
>
> Fixes: 96eabe7a40aa ("bpf: Allow selecting numa node during map creation")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Good catch. Thanks!
Acked-by: Alexei Starovoitov <ast@kernel.org>
^ permalink raw reply
* Re: [RFC net-next] net: sch_clsact: add support for global per-netns classifier mode
From: Roopa Prabhu @ 2017-09-05 15:17 UTC (permalink / raw)
To: Jiri Pirko
Cc: Nikolay Aleksandrov, netdev@vger.kernel.org, David Ahern,
Cong Wang, Jamal Hadi Salim
In-Reply-To: <20170905140750.GB7936@nanopsycho>
On Tue, Sep 5, 2017 at 7:07 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> Tue, Sep 05, 2017 at 02:48:21PM CEST, nikolay@cumulusnetworks.com wrote:
>>Hi all,
>>This RFC adds a new mode for clsact which designates a device's egress
>>classifier as global per netns. The packets that are not classified for
>>a particular device will be classified using the global classifier.
>>We have needed a global classifier for some time now for various
>>purposes and setting the single bridge or loopback/vrf device as the
>>global classifier device is acceptable for us. Doing it this way avoids
>>the act/cls device and queue dependencies.
>>
>>This is strictly an RFC patch just to show the intent, if we agree on
>>the details the proposed patch will have support for both ingress and
>>egress, and will be using a static key to avoid the fast path test when no
>>global classifier has been configured.
>>
>>Example (need a modified tc that adds TCA_OPTIONS when using q_clsact):
>>$ tc qdisc add dev lo clsact global
>>$ tc filter add dev lo egress protocol ip u32 match ip dst 4.3.2.1/32 action drop
>>
>>the last filter will be global for all devices that don't have a
>>specific egress_cl_list (i.e. have clsact configured).
>>
>>Any comments and thoughts would be greatly appreciated.
>
> Did you see my shared blocks work? I believe that it should resolve your
> usecase, in a generic way. You just have to bind the devices you need to
> the shared block. Please see the RFC:
>
> https://www.spinics.net/lists/netdev/msg444067.html
Jiri, yes, we have seen this series. This still requires one to make
the association between dev and tc shared block..and
the rules are associated with every device. Your work will help the
case and is needed for tc in general and can co-exist.
It takes us closer but is still not a way to create global tc rules.
imagine thousands of netdevs. We would ideally like the show to also
display a single set of rules.
Given tc has a rich set of classifiers and actions (and very
extensible!), we are trying to see if those can
be easily applied globally than being tied to a device. Maybe there
are other better ways to achieve this...this thread
is to start that discussion. I think solving this once will help the
scale issue for your hardware offload case as well.
^ permalink raw reply
* [PATCH net v2] net: sched: don't use GFP_KERNEL under spin lock
From: Jakub Kicinski @ 2017-09-05 15:31 UTC (permalink / raw)
To: netdev, eric.dumazet
Cc: jiri, Chris Mi, xiyou.wangcong, jhs, oss-drivers, Jakub Kicinski
The new TC IDR code uses GFP_KERNEL under spin lock. Which leads
to:
[ 582.621091] BUG: sleeping function called from invalid context at ../mm/slab.h:416
[ 582.629721] in_atomic(): 1, irqs_disabled(): 0, pid: 3379, name: tc
[ 582.636939] 2 locks held by tc/3379:
[ 582.641049] #0: (rtnl_mutex){+.+.+.}, at: [<ffffffff910354ce>] rtnetlink_rcv_msg+0x92e/0x1400
[ 582.650958] #1: (&(&tn->idrinfo->lock)->rlock){+.-.+.}, at: [<ffffffff9110a5e0>] tcf_idr_create+0x2f0/0x8e0
[ 582.662217] Preemption disabled at:
[ 582.662222] [<ffffffff9110a5e0>] tcf_idr_create+0x2f0/0x8e0
[ 582.672592] CPU: 9 PID: 3379 Comm: tc Tainted: G W 4.13.0-rc7-debug-00648-g43503a79b9f0 #287
[ 582.683432] Hardware name: Dell Inc. PowerEdge R730/072T6D, BIOS 2.3.4 11/08/2016
[ 582.691937] Call Trace:
...
[ 582.742460] kmem_cache_alloc+0x286/0x540
[ 582.747055] radix_tree_node_alloc.constprop.6+0x4a/0x450
[ 582.753209] idr_get_free_cmn+0x627/0xf80
...
[ 582.815525] idr_alloc_cmn+0x1a8/0x270
...
[ 582.833804] tcf_idr_create+0x31b/0x8e0
...
Try to preallocate the memory with idr_prealloc(GFP_KERNEL)
(as suggested by Eric Dumazet), and change the allocation
flags under spin lock.
Fixes: 65a206c01e8e ("net/sched: Change act_api and act_xxx modules to use IDR")
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
net/sched/act_api.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 0eb545bcb247..a306974e2fb4 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -296,10 +296,12 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
spin_lock_init(&p->tcfa_lock);
/* user doesn't specify an index */
if (!index) {
+ idr_preload(GFP_KERNEL);
spin_lock_bh(&idrinfo->lock);
err = idr_alloc_ext(idr, NULL, &idr_index, 1, 0,
- GFP_KERNEL);
+ GFP_ATOMIC);
spin_unlock_bh(&idrinfo->lock);
+ idr_preload_end();
if (err) {
err3:
free_percpu(p->cpu_qstats);
@@ -307,10 +309,12 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
}
p->tcfa_index = idr_index;
} else {
+ idr_preload(GFP_KERNEL);
spin_lock_bh(&idrinfo->lock);
err = idr_alloc_ext(idr, NULL, NULL, index, index + 1,
- GFP_KERNEL);
+ GFP_ATOMIC);
spin_unlock_bh(&idrinfo->lock);
+ idr_preload_end();
if (err)
goto err3;
p->tcfa_index = index;
--
2.14.1
^ permalink raw reply related
* [PATCH net] rds: Fix non-atomic operation on shared flag variable
From: Håkon Bugge @ 2017-09-05 15:42 UTC (permalink / raw)
To: Santosh Shilimkar, David S . Miller
Cc: netdev, linux-rdma, rds-devel, linux-kernel, knut.omang,
wei.lin.guay
The bits in m_flags in struct rds_message are used for a plurality of
reasons, and from different contexts. To avoid any missing updates to
m_flags, use the atomic set_bit() instead of the non-atomic equivalent.
Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
Reviewed-by: Knut Omang <knut.omang@oracle.com>
Reviewed-by: Wei Lin Guay <wei.lin.guay@oracle.com>
---
net/rds/send.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/rds/send.c b/net/rds/send.c
index 41b9f0f..058a407 100644
--- a/net/rds/send.c
+++ b/net/rds/send.c
@@ -273,7 +273,7 @@ int rds_send_xmit(struct rds_conn_path *cp)
len = ntohl(rm->m_inc.i_hdr.h_len);
if (cp->cp_unacked_packets == 0 ||
cp->cp_unacked_bytes < len) {
- __set_bit(RDS_MSG_ACK_REQUIRED, &rm->m_flags);
+ set_bit(RDS_MSG_ACK_REQUIRED, &rm->m_flags);
cp->cp_unacked_packets =
rds_sysctl_max_unacked_packets;
@@ -829,7 +829,7 @@ static int rds_send_queue_rm(struct rds_sock *rs, struct rds_connection *conn,
* throughput hits a certain threshold.
*/
if (rs->rs_snd_bytes >= rds_sk_sndbuf(rs) / 2)
- __set_bit(RDS_MSG_ACK_REQUIRED, &rm->m_flags);
+ set_bit(RDS_MSG_ACK_REQUIRED, &rm->m_flags);
list_add_tail(&rm->m_sock_item, &rs->rs_send_queue);
set_bit(RDS_MSG_ON_SOCK, &rm->m_flags);
--
2.9.3
^ permalink raw reply related
* [PATCH 24/25 v2] net/cdc_ncm: Replace tasklet with softirq hrtimer
From: Sebastian Andrzej Siewior @ 2017-09-05 15:42 UTC (permalink / raw)
To: Bjørn Mork
Cc: Anna-Maria Gleixner, LKML, Peter Zijlstra, Ingo Molnar,
Christoph Hellwig, keescook, John Stultz, Thomas Gleixner,
Oliver Neukum, Greg Kroah-Hartman, linux-usb, netdev
In-Reply-To: <87mv6fzvpr.fsf@miraculix.mork.no>
From: Thomas Gleixner <tglx@linutronix.de>
The bh tasklet is used in invoke the hrtimer (cdc_ncm_tx_timer_cb) in
softirq context. This can be also achieved without the tasklet but with
CLOCK_MONOTONIC_SOFT as hrtimer base.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
Cc: Oliver Neukum <oliver@neukum.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Cc: netdev@vger.kernel.org
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
[bigeasy: using usb_get_intfdata() as suggested by Bjørn Mork]
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
On 2017-08-31 15:57:04 [+0200], Bjørn Mork wrote:
> I believe the struct usbnet pointer is redundant. We already have lots
> of pointers back and forth here. This should work, but is not tested:
>
> struct usbnet *dev = usb_get_intfdata(ctx->control):
I think so, too. Still untested as I don't have a working gadget around.
v1…v2: Updated as suggested by Bjørn and added Greg's Acked-by.
drivers/net/usb/cdc_ncm.c | 36 +++++++++++++++---------------------
include/linux/usb/cdc_ncm.h | 1 -
2 files changed, 15 insertions(+), 22 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 8f572b9f3625..42f7bd90e6a4 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -61,7 +61,6 @@ static bool prefer_mbim;
module_param(prefer_mbim, bool, S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(prefer_mbim, "Prefer MBIM setting on dual NCM/MBIM functions");
-static void cdc_ncm_txpath_bh(unsigned long param);
static void cdc_ncm_tx_timeout_start(struct cdc_ncm_ctx *ctx);
static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *hr_timer);
static struct usb_driver cdc_ncm_driver;
@@ -777,10 +776,8 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
if (!ctx)
return -ENOMEM;
- hrtimer_init(&ctx->tx_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+ hrtimer_init(&ctx->tx_timer, CLOCK_MONOTONIC_SOFT, HRTIMER_MODE_REL);
ctx->tx_timer.function = &cdc_ncm_tx_timer_cb;
- ctx->bh.data = (unsigned long)dev;
- ctx->bh.func = cdc_ncm_txpath_bh;
atomic_set(&ctx->stop, 0);
spin_lock_init(&ctx->mtx);
@@ -967,10 +964,7 @@ void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf)
atomic_set(&ctx->stop, 1);
- if (hrtimer_active(&ctx->tx_timer))
- hrtimer_cancel(&ctx->tx_timer);
-
- tasklet_kill(&ctx->bh);
+ hrtimer_cancel(&ctx->tx_timer);
/* handle devices with combined control and data interface */
if (ctx->control == ctx->data)
@@ -1348,20 +1342,9 @@ static void cdc_ncm_tx_timeout_start(struct cdc_ncm_ctx *ctx)
HRTIMER_MODE_REL);
}
-static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *timer)
+static void cdc_ncm_txpath_bh(struct cdc_ncm_ctx *ctx)
{
- struct cdc_ncm_ctx *ctx =
- container_of(timer, struct cdc_ncm_ctx, tx_timer);
-
- if (!atomic_read(&ctx->stop))
- tasklet_schedule(&ctx->bh);
- return HRTIMER_NORESTART;
-}
-
-static void cdc_ncm_txpath_bh(unsigned long param)
-{
- struct usbnet *dev = (struct usbnet *)param;
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ struct usbnet *dev = usb_get_intfdata(ctx->control);
spin_lock_bh(&ctx->mtx);
if (ctx->tx_timer_pending != 0) {
@@ -1379,6 +1362,17 @@ static void cdc_ncm_txpath_bh(unsigned long param)
}
}
+static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *timer)
+{
+ struct cdc_ncm_ctx *ctx =
+ container_of(timer, struct cdc_ncm_ctx, tx_timer);
+
+ if (!atomic_read(&ctx->stop))
+ cdc_ncm_txpath_bh(ctx);
+
+ return HRTIMER_NORESTART;
+}
+
struct sk_buff *
cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
{
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index 1a59699cf82a..62b506fddf8d 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -92,7 +92,6 @@
struct cdc_ncm_ctx {
struct usb_cdc_ncm_ntb_parameters ncm_parm;
struct hrtimer tx_timer;
- struct tasklet_struct bh;
const struct usb_cdc_ncm_desc *func_desc;
const struct usb_cdc_mbim_desc *mbim_desc;
--
2.14.1
^ permalink raw reply related
* Re: [pull request][net-next 0/3] Mellanox, mlx5 GRE tunnel offloads
From: Tom Herbert @ 2017-09-05 16:02 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Saeed Mahameed, Saeed Mahameed, David S. Miller,
Linux Netdev List
In-Reply-To: <87r2vls8hp.fsf@stressinduktion.org>
On Tue, Sep 5, 2017 at 4:14 AM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> Tom Herbert <tom@herbertland.com> writes:
>
>> There is absolutely no requirement in IP that packets are delivered in
>> order-- there never has been and there never will be! If the ULP, like
>> Ethernet encapsulation, requires in order deliver then it needs to
>> implement that itself like TCP, GRE, and other protocols ensure that
>> with sequence numbers and reassembly. All of these hoops we do make
>> sure that packets always follow the same path and are always in order
>> are done for benefit of middlebox devices like stateful firewalls that
>> have force us to adopt their concept of network architecture-- in the
>> long run this is self-defeating and kills our ability to innovate.
>>
>> I'm not saying that we shouldn't consider legacy devices, but we
>> should scrutinize new development or solutions that perpetuate
>> incorrect design or bad assumptions.
>
> So configure RSS per port and ensure no fragments are send to those
> ports. This is possible and rather easy to do. It solves the problem
> with legacy software and it spreads out packets for your applications.
>
> It is not perfect but it is working and solves both problems.
>
Hannes,
I don't see how that solves anything. The purpose of RSS is to
distribute the load of protocol packets across queues. This needs to
work for UDP applications. For instance, if I were building a QUIC
server I'd want the sort of flow distribution that a TCP server would
give. You can't do that by configuring a few ports in the device.
If I were to suggest any HW change it would be to not do DPI on
fragments (MF or offset is set). This ensures that all packets of the
fragment train get hashed to the same queue and is on fact what RPS
has been doing for years without any complaints.
But even before I'd make that recommendation, I'd really like
understand what the problem actually is. The only thing I can garner
from this discussion and the Intel patch is that when fragments are
received OOO that is perceived as a problem. But the by the protocol
specification clearly says this is not a problem. So the questions
are: who is negatively affected by this? Is this a problem because
some artificial test that checks for everything to be in order is now
failing? Is this affecting real users? Is this an issue in the stack
or really with some implementation outside of the stack? If it is an
implementation outside of the stack, then are we just bandaid'ing over
someone else's incorrect implementation by patching the kernel (like
would have be the case if we change the kernel to interoperate with
Facebook's switch that couldn't handle OOO in twstate).
Thanks,
Tom
> Bye,
> Hannes
^ permalink raw reply
* Re: [PATCH net-next] bpf: fix numa_node validation
From: David Miller @ 2017-09-05 16:10 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, kafai, daniel, ast
In-Reply-To: <1504590062.15310.36.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 04 Sep 2017 22:41:02 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> syzkaller reported crashes in bpf map creation or map update [1]
>
> Problem is that nr_node_ids is a signed integer,
> NUMA_NO_NODE is also an integer, so it is very tempting
> to declare numa_node as a signed integer.
>
> This means the typical test to validate a user provided value :
>
> if (numa_node != NUMA_NO_NODE &&
> (numa_node >= nr_node_ids ||
> !node_online(numa_node)))
>
> must be written :
>
> if (numa_node != NUMA_NO_NODE &&
> ((unsigned int)numa_node >= nr_node_ids ||
> !node_online(numa_node)))
>
>
> [1]
...
> Fixes: 96eabe7a40aa ("bpf: Allow selecting numa node during map creation")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] net/ncsi: fix ncsi_vlan_rx_{add,kill}_vid references
From: David Miller @ 2017-09-05 16:12 UTC (permalink / raw)
To: arnd; +Cc: gwshan, sam, joel, netdev, linux-kernel
In-Reply-To: <20170905080615.1131665-1-arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 5 Sep 2017 10:05:47 +0200
> We get a new link error in allmodconfig kernels after ftgmac100
> started using the ncsi helpers:
>
> ERROR: "ncsi_vlan_rx_kill_vid" [drivers/net/ethernet/faraday/ftgmac100.ko] undefined!
> ERROR: "ncsi_vlan_rx_add_vid" [drivers/net/ethernet/faraday/ftgmac100.ko] undefined!
>
> Related to that, we get another error when CONFIG_NET_NCSI is disabled:
>
> drivers/net/ethernet/faraday/ftgmac100.c:1626:25: error: 'ncsi_vlan_rx_add_vid' undeclared here (not in a function); did you mean 'ncsi_start_dev'?
> drivers/net/ethernet/faraday/ftgmac100.c:1627:26: error: 'ncsi_vlan_rx_kill_vid' undeclared here (not in a function); did you mean 'ncsi_vlan_rx_add_vid'?
>
> This fixes both problems at once, using a 'static inline' stub helper
> for the disabled case, and exporting the functions when they are present.
>
> Fixes: 51564585d8c6 ("ftgmac100: Support NCSI VLAN filtering when available")
> Fixes: 21acf63013ed ("net/ncsi: Configure VLAN tag filter")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied.
^ permalink raw reply
* Re: [PATCH] soc: ti/knav_dma: include dmaengine header
From: David Miller @ 2017-09-05 16:12 UTC (permalink / raw)
To: arnd; +Cc: arm, dave.jiang, netdev, m-karicheri2, linux-kernel
In-Reply-To: <20170905083146.1289546-1-arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 5 Sep 2017 10:31:35 +0200
> A header file cleanup apparently caused a build regression
> with one driver using the knav infrastructure:
>
> In file included from drivers/net/ethernet/ti/netcp_core.c:30:0:
> include/linux/soc/ti/knav_dma.h:129:30: error: field 'direction' has incomplete type
> enum dma_transfer_direction direction;
> ^~~~~~~~~
> drivers/net/ethernet/ti/netcp_core.c: In function 'netcp_txpipe_open':
> drivers/net/ethernet/ti/netcp_core.c:1349:21: error: 'DMA_MEM_TO_DEV' undeclared (first use in this function); did you mean 'DMA_MEMORY_MAP'?
> config.direction = DMA_MEM_TO_DEV;
> ^~~~~~~~~~~~~~
> DMA_MEMORY_MAP
> drivers/net/ethernet/ti/netcp_core.c:1349:21: note: each undeclared identifier is reported only once for each function it appears in
> drivers/net/ethernet/ti/netcp_core.c: In function 'netcp_setup_navigator_resources':
> drivers/net/ethernet/ti/netcp_core.c:1659:22: error: 'DMA_DEV_TO_MEM' undeclared (first use in this function); did you mean 'DMA_DESC_HOST'?
> config.direction = DMA_DEV_TO_MEM;
>
> As the header is no longer included implicitly through netdevice.h,
> we should include it in the header that references the enum.
>
> Fixes: 0dd5759dbb1c ("net: remove dmaengine.h inclusion from netdevice.h")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> If the cleanup patch hasn't been submitted for mainline yet, please
> add this fixup to the net-next tree, otherwise I'll merge it through
> arm-soc.
Applied to net-next.
^ permalink raw reply
* Re: [PATCH iproute2 master 0/2] Two minor BPF updates
From: Stephen Hemminger @ 2017-09-05 16:27 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: ast, netdev
In-Reply-To: <cover.1504570627.git.daniel@iogearbox.net>
On Tue, 5 Sep 2017 02:24:30 +0200
Daniel Borkmann <daniel@iogearbox.net> wrote:
> Two minor updates including a small cleanup for dumping
> the trace pipe and one for consolidating prog dumps for
> tc and xdp to use bpf_prog_info_by_fd() when possible.
>
> Thanks!
>
> Daniel Borkmann (2):
> bpf: minor cleanups for bpf_trace_pipe
> bpf: consolidate dumps to use bpf_dump_prog_info
>
> include/bpf_util.h | 2 +-
> ip/ipaddress.c | 6 ++++--
> ip/iplink_xdp.c | 19 +++++++++++++++----
> ip/xdp.h | 2 +-
> lib/bpf.c | 31 ++++++++++++++++++-------------
> tc/f_bpf.c | 8 ++++----
> tc/m_bpf.c | 8 ++++----
> 7 files changed, 47 insertions(+), 29 deletions(-)
>
Applied, thanks Daniel
^ permalink raw reply
* Re: [PATCH iproute2/master] tc actions: store and dump correct length of user cookies
From: Stephen Hemminger @ 2017-09-05 16:27 UTC (permalink / raw)
To: Simon Horman; +Cc: netdev, oss-drivers, Jamal Hadi Salim
In-Reply-To: <1504609584-20026-1-git-send-email-simon.horman@netronome.com>
On Tue, 5 Sep 2017 13:06:24 +0200
Simon Horman <simon.horman@netronome.com> wrote:
> Correct two errors which cancel each other out:
> * Do not send twice the length of the actual provided by the user to the kernel
> * Do not dump half the length of the cookie provided by the kernel
>
> As the cookie is now stored in the kernel at its correct length rather
> than double the that length cookies of up to the maximum size of 16 bytes
> may now be stored rather than a maximum of half that length.
>
> Output of dump is the same before and after this change,
> but the data stored in the kernel is now exactly the cookie
> rather than the cookie + as many trailing zeros.
>
> Before:
> # tc filter add dev eth0 protocol ip parent ffff: \
> flower ip_proto udp action drop \
> cookie 0123456789abcdef0123456789abcdef
> RTNETLINK answers: Invalid argument
>
> After:
> # tc filter add dev eth0 protocol ip parent ffff: \
> flower ip_proto udp action drop \
> cookie 0123456789abcdef0123456789abcdef
> # tc filter show dev eth0 ingress
> eth_type ipv4
> ip_proto udp
> not_in_hw
> action order 1: gact action drop
> random type none pass val 0
> index 1 ref 1 bind 1 installed 1 sec used 1 sec
> Action statistics:
> Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
> cookie len 16 0123456789abcdef0123456789abcdef
>
> Fixes: fd8b3d2c1b9b ("actions: Add support for user cookies")
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Signed-off-by: Simon Horman <simon.horman@netronome.com>
Applied thanks
^ permalink raw reply
* Re: [PATCH net] rds: Fix non-atomic operation on shared flag variable
From: Santosh Shilimkar @ 2017-09-05 16:33 UTC (permalink / raw)
To: Håkon Bugge
Cc: David S . Miller, netdev, linux-rdma, rds-devel, linux-kernel,
knut.omang, wei.lin.guay
In-Reply-To: <20170905154201.30198-1-Haakon.Bugge@oracle.com>
On 9/5/2017 8:42 AM, Håkon Bugge wrote:
> The bits in m_flags in struct rds_message are used for a plurality of
> reasons, and from different contexts. To avoid any missing updates to
> m_flags, use the atomic set_bit() instead of the non-atomic equivalent.
>
> Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
> Reviewed-by: Knut Omang <knut.omang@oracle.com>
> Reviewed-by: Wei Lin Guay <wei.lin.guay@oracle.com>
> ---
> net/rds/send.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/rds/send.c b/net/rds/send.c
> index 41b9f0f..058a407 100644
> --- a/net/rds/send.c
> +++ b/net/rds/send.c
> @@ -273,7 +273,7 @@ int rds_send_xmit(struct rds_conn_path *cp)
> len = ntohl(rm->m_inc.i_hdr.h_len);
> if (cp->cp_unacked_packets == 0 ||
> cp->cp_unacked_bytes < len) {
> - __set_bit(RDS_MSG_ACK_REQUIRED, &rm->m_flags);
> + set_bit(RDS_MSG_ACK_REQUIRED, &rm->m_flags);
>
> cp->cp_unacked_packets =
> rds_sysctl_max_unacked_packets;
> @@ -829,7 +829,7 @@ static int rds_send_queue_rm(struct rds_sock *rs, struct rds_connection *conn,
> * throughput hits a certain threshold.
> */
> if (rs->rs_snd_bytes >= rds_sk_sndbuf(rs) / 2)
> - __set_bit(RDS_MSG_ACK_REQUIRED, &rm->m_flags);
> + set_bit(RDS_MSG_ACK_REQUIRED, &rm->m_flags);
>
> list_add_tail(&rm->m_sock_item, &rs->rs_send_queue);
> set_bit(RDS_MSG_ON_SOCK, &rm->m_flags);
>
Indeed, these couple of instances remained for the m_flags.
Patch looks good. Thanks !!
Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>
^ permalink raw reply
* Re: [PATCH iproute2 master 2/2] bpf: consolidate dumps to use bpf_dump_prog_info
From: Stephen Hemminger @ 2017-09-05 16:35 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: ast, netdev
In-Reply-To: <1e96ef342a4f34ee8c8119ee8cc1ed7092ea5ced.1504570627.git.daniel@iogearbox.net>
On Tue, 5 Sep 2017 02:24:32 +0200
Daniel Borkmann <daniel@iogearbox.net> wrote:
> Consolidate dump of prog info to use bpf_dump_prog_info() when possible.
> Moving forward, we want to have a consistent output for BPF progs when
> being dumped. E.g. in cls/act case we used to dump tag as a separate
> netlink attribute before we had BPF_OBJ_GET_INFO_BY_FD bpf(2) command.
>
> Move dumping tag into bpf_dump_prog_info() as well, and only dump the
> netlink attribute for older kernels. Also, reuse bpf_dump_prog_info()
> for XDP case, so we can dump tag and whether program was jited, which
> we currently don't show.
>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
I applied this to master, and resolved conflicts with net-next.
But the dump with JSON of xdp is now incomplete.
^ permalink raw reply
* Re: [PATCH iproute2 master 2/2] bpf: consolidate dumps to use bpf_dump_prog_info
From: Daniel Borkmann @ 2017-09-05 16:37 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: ast, netdev
In-Reply-To: <20170905093501.2f1fbbfc@xeon-e3>
On 09/05/2017 06:35 PM, Stephen Hemminger wrote:
[...]
> I applied this to master, and resolved conflicts with net-next.
> But the dump with JSON of xdp is now incomplete.
Ok, I will check it out, and send a follow-up to make it
complete again.
^ permalink raw reply
* Re: 915f3e3f76 ("mac80211_hwsim: Replace bogus hrtimer clockid"): BUG: kernel reboot-without-warning in test stage
From: Sebastian Andrzej Siewior @ 2017-09-05 16:41 UTC (permalink / raw)
To: Thomas Gleixner
Cc: kernel test robot, LKP, linux-kernel, netdev, linux-wireless, wfg
In-Reply-To: <alpine.DEB.2.20.1709050910150.1900@nanos>
On 2017-09-05 09:12:40 [+0200], Thomas Gleixner wrote:
> Sorry, no. That bisect is completely bogus. The commit in question merily
> replaces the unsupported clockid with a valid one.
The bisect is correct. It just has problems to express itself properly. So
the table says:
| WARNING:at_kernel/time/hrtimer.c:#hrtimer_init | 7 | | | |
| BUG:kernel_reboot-without-warning_in_test_stage | 0 | 12 | 6 | 6 |
which means _before_ your commit it counted a warning in hrtimer_init()
(an unsupported clock id was used). With your commit, the warning was
gone and I *think* the userland then printed
"BUG:kernel_reboot-without-warning_in_test_stage" because it had no
warning.
It seems that the bot learned to live with that warning which was around
for more than three years. Now that you removed it, it seems to be a
mistake to do so because nobody complained about it so far.
> Thanks,
>
> tglx
Sebastian
^ permalink raw reply
* [ANNOUNCE] Iproute2 for Linux 4.13
From: Stephen Hemminger @ 2017-09-05 16:49 UTC (permalink / raw)
To: netdev, linux-kernel
Update to iproute2 utility to support new features in Linux 4.13.
This is a larger than usual release because of lots of updates for BPF
and the new RDMA utility. Lots of cleanups and Coverity reported
potential issues as well.
Source:
https://www.kernel.org/pub/linux/utils/net/iproute2/iproute2-4.13.0.tar.gz
Repository:
git://git.kernel.org/pub/scm/linux/kernel/git/shemminger/iproute2.git
Report problems (or enhancements) to the netdev@vger.kernel.org mailing list.
---
Alexander Alemayhu (1):
examples/bpf: update list of examples
Andreas Henriksson (1):
ss: fix help/man TCP-STATE description for listening
Arkadi Sharshevsky (1):
bridge: Distinguish between externally learned vs offloaded FDBs
Casey Callendrello (1):
netns: make /var/run/netns bind-mount recursive
Daniel Borkmann (8):
bpf: remove obsolete samples
bpf: support loading map in map from obj
bpf: dump id/jited info for cls/act programs
bpf: improve error reporting around tail calls
bpf: fix mnt path when from env
bpf: unbreak libelf linkage for bpf obj loader
bpf: minor cleanups for bpf_trace_pipe
bpf: consolidate dumps to use bpf_dump_prog_info
David Ahern (2):
lib: Dump ext-ack string by default
libnetlink: Fix extack attribute parsing
Girish Moodalbail (1):
geneve: support for modifying geneve device
Hangbin Liu (1):
utils: return default family when rtm_family is not RTNL_FAMILY_IPMR/IP6MR
Jakub Kicinski (3):
bpf: print xdp offloaded mode
bpf: add xdpdrv for requesting XDP driver mode
bpf: allow requesting XDP HW offload
Jiri Benc (2):
tc: m_tunnel_key: reformat the usage text
tc: m_tunnel_key: add csum/nocsum option
Jiri Pirko (7):
tc_filter: add support for chain index
tc: actions: add helpers to parse and print control actions
tc/actions: introduce support for goto chain action
tc: flower: add support for tcp flags
tc: gact: fix control action parsing
tc: add support for TRAP action
tc: don't print error message on miss when parsing action with default
Leon Romanovsky (8):
utils: Move BIT macro to common header
rdma: Add basic infrastructure for RDMA tool
rdma: Add dev object
rdma: Add link object
rdma: Add json and pretty outputs
rdma: Implement json output for dev object
rdma: Add json output to link object
rdma: Add initial manual for the tool
Martin KaFai Lau (1):
bpf: Add support for IFLA_XDP_PROG_ID
Matteo Croce (3):
tc: fix typo in manpage
netns: avoid directory traversal
netns: more input validation
Michal Kubecek (2):
iplink: check for message truncation in iplink_get()
iplink: double the buffer size also in iplink_get()
Or Gerlitz (1):
tc: flower: add support for matching on ip tos and ttl
Phil Sutter (58):
bpf: Make bytecode-file reading a little more robust
Really fix get_addr() and get_prefix() error messages
tc-simple: Fix documentation
examples: Some shell fixes to cbq.init
ifcfg: Quote left-hand side of [ ] expression
tipc/node: Fix socket fd check in cmd_node_get_addr()
iproute_lwtunnel: Argument to strerror must be positive
iproute_lwtunnel: csum_mode value checking was ineffective
ss: Don't leak fd in tcp_show_netlink_file()
tc/em_ipset: Don't leak sockfd on error path
ipvrf: Fix error path of vrf_switch()
ifstat: Fix memleak in error case
ifstat: Fix memleak in dump_kern_db() for json output
ss: Fix potential memleak in unix_stats_print()
tipc/bearer: Fix resource leak in error path
devlink: No need for this self-assignment
ipntable: No need to check and assign to parms_rta
iproute: Fix for missing 'Oifs:' display
lib/rt_names: Drop dead code in rtnl_rttable_n2a()
ss: Skip useless check in parse_hostcond()
ss: Drop useless assignment
tc/m_gact: Drop dead code
ipaddress: Avoid accessing uninitialized variable lcl
iplink_can: Prevent overstepping array bounds
ipmaddr: Avoid accessing uninitialized data
ss: Use C99 initializer in netlink_show_one()
netem/maketable: Check return value of fstat()
tc/q_multiq: Don't pass garbage in TCA_OPTIONS
iproute: Check mark value input
iplink_vrf: Complain if main table is not found
devlink: Check return code of strslashrsplit()
lib/bpf: Don't leak fp in bpf_find_mntpt()
ifstat, nstat: Check fdopen() return value
tc/q_netem: Don't dereference possibly NULL pointer
tc/tc_filter: Make sure filter name is not empty
tipc/bearer: Prevent NULL pointer dereference
ipntable: Avoid memory allocation for filter.name
lib/fs: Fix format string in find_fs_mount()
lib/inet_proto: Review inet_proto_{a2n,n2a}()
lnstat_util: Simplify alloc_and_open() a bit
tc/m_xt: Fix for potential string buffer overflows
lib/ll_map: Choose size of new cache items at run-time
ss: Make struct tcpstat fields 'timer' and 'timeout' unsigned
ss: Make sure scanned index value to unix_state_map is sane
netem/maketable: Check return value of fscanf()
lib/bpf: Check return value of write()
lib/fs: Fix and simplify make_path()
lib/libnetlink: Don't pass NULL parameter to memcpy()
ss: Fix for added diag support check
link_gre6: Fix for changing tclass/flowlabel
link_gre6: Print the tunnel's tclass setting
utils: Implement strlcpy() and strlcat()
Convert the obvious cases to strlcpy()
Convert harmful calls to strncpy() to strlcpy()
ipxfrm: Replace STRBUF_CAT macro with strlcat()
tc_util: No need to terminate an snprintf'ed buffer
lnstat_util: Make sure buffer is NUL-terminated
lib/bpf: Fix bytecode-file parsing
Philip Prindeville (1):
iproute2: add support for GRE ignore-df knob
Roopa Prabhu (2):
ip: extend route get to return matching fib route
iproute: extend route get for mpls routes
Simon Horman (1):
tc actions: store and dump correct length of user cookies
Stephen Hemminger (27):
update to current net-next headers
update headers to get changes for TCA_FLOWER
update headers to get IFLA_EVENT
updated headers from net-next
update headers from net-next (bpf and tc)
more bpf header updates
update headers to get TCA_TUNNEL_CSUM
update kernel headers from net-next
update headers to 4.13-rc1
remove duplicated #include's
Update headers from net-next
ip: change flag names to an array
update headers from 4.13-rc4
tc: fix m_simple usage
iproute: Add support for extended ack to rtnl_talk
ss: enclose IPv6 address in brackets
lib: fix extended ack with and without libmnl
lib: need to pass LIBMNL flag
tc, ip: more Makefile updates for LIBMNL
vti6: fix local/remote any addr handling
change how Config is used in Makefile's
vti: print keys in hex not dotted notation
rdma: fix duplicate initialization in port_names
libnetlink: drop unused parameter to rtnl_dump_done
bpf: drop unused parameter to bpf_report_map_in_map
tc: use named initializer for default mqprio options
v4.13.0
Vlad Yasevich (1):
ip: Add IFLA_EVENT output to ip monitor
Élie Bouttier (1):
ip route: replace exits with returns
^ permalink raw reply
* Re: [PATCH net v2] net: sched: don't use GFP_KERNEL under spin lock
From: Eric Dumazet @ 2017-09-05 17:17 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, jiri, Chris Mi, xiyou.wangcong, jhs, oss-drivers
In-Reply-To: <20170905153123.13209-1-jakub.kicinski@netronome.com>
On Tue, 2017-09-05 at 08:31 -0700, Jakub Kicinski wrote:
> The new TC IDR code uses GFP_KERNEL under spin lock. Which leads
> to:
>
> [ 582.621091] BUG: sleeping function called from invalid context at ../mm/slab.h:416
> [ 582.629721] in_atomic(): 1, irqs_disabled(): 0, pid: 3379, name: tc
> [ 582.636939] 2 locks held by tc/3379:
> [ 582.641049] #0: (rtnl_mutex){+.+.+.}, at: [<ffffffff910354ce>] rtnetlink_rcv_msg+0x92e/0x1400
> [ 582.650958] #1: (&(&tn->idrinfo->lock)->rlock){+.-.+.}, at: [<ffffffff9110a5e0>] tcf_idr_create+0x2f0/0x8e0
> [ 582.662217] Preemption disabled at:
> [ 582.662222] [<ffffffff9110a5e0>] tcf_idr_create+0x2f0/0x8e0
> [ 582.672592] CPU: 9 PID: 3379 Comm: tc Tainted: G W 4.13.0-rc7-debug-00648-g43503a79b9f0 #287
> [ 582.683432] Hardware name: Dell Inc. PowerEdge R730/072T6D, BIOS 2.3.4 11/08/2016
> [ 582.691937] Call Trace:
> ...
> [ 582.742460] kmem_cache_alloc+0x286/0x540
> [ 582.747055] radix_tree_node_alloc.constprop.6+0x4a/0x450
> [ 582.753209] idr_get_free_cmn+0x627/0xf80
> ...
> [ 582.815525] idr_alloc_cmn+0x1a8/0x270
> ...
> [ 582.833804] tcf_idr_create+0x31b/0x8e0
> ...
>
> Try to preallocate the memory with idr_prealloc(GFP_KERNEL)
> (as suggested by Eric Dumazet), and change the allocation
> flags under spin lock.
>
> Fixes: 65a206c01e8e ("net/sched: Change act_api and act_xxx modules to use IDR")
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Simon Horman <simon.horman@netronome.com>
> ---
Acked-by: Eric Dumazet <edumazet@google.com>
Not related to your patch, but it looks like idr_preload() has a bug
added in 4.11. I will send a fix to lkml.
^ permalink raw reply
* Re: [PATCH net-next 2/4] ip/options: explicitly provide net ns to __ip_options_echo()
From: Eric Dumazet @ 2017-09-05 17:18 UTC (permalink / raw)
To: Paolo Abeni, David S. Miller; +Cc: netdev, Eric Dumazet, Hannes Frederic Sowa
In-Reply-To: <31096a330a56cfd40eea136be505270184fbb5d4.1501775813.git.pabeni@redhat.com>
On Thu, 2017-08-03 at 18:07 +0200, Paolo Abeni wrote:
> __ip_options_echo() uses the current network namespace, and
> currently retrives it via skb->dst->dev.
>
> This commit adds an explicit 'net' argument to __ip_options_echo()
> and update all the call sites to provide it, usually via a simpler
> sock_net().
>
> After this change, __ip_options_echo() no more needs to access
> skb->dst and we can drop a couple of hack to preserve such
> info in the rx path.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
David, Paolo
This commit (91ed1e666a4ea2e260452a7d7d311ac5ae852cba "ip/options:
explicitly provide net ns to __ip_options_echo()")
needs to be backported to linux-4.13 stable version to avoid these kind
of crashes [1]
This is because of MSG_PEEK operation, hitting skb_consume_udp() while
skb is still in receive queue.
Next read() finding again the skb then can see a NULL skb->dst
Thanks !
[1]
general protection fault: 0000 [#1] SMP KASAN
Dumping ftrace buffer:
(ftrace buffer empty)
Modules linked in:
CPU: 0 PID: 3017 Comm: syzkaller446772 Not tainted 4.13.0+ #68
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
task: ffff8801cd0a4380 task.stack: ffff8801cc498000
RIP: 0010:__ip_options_echo+0xea8/0x1430 net/ipv4/ip_options.c:143
RSP: 0018:ffff8801cc49f628 EFLAGS: 00010246
RAX: dffffc0000000000 RBX: ffff8801cc49f928 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000000000004
RBP: ffff8801cc49f6b8 R08: ffff8801cc49f936 R09: ffffed0039893f28
R10: 0000000000000003 R11: ffffed0039893f27 R12: ffff8801cc49f918
R13: ffff8801ccbcf36c R14: 000000000000000f R15: 0000000000000018
FS: 0000000000979880(0000) GS:ffff8801db200000(0000)
knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000200c0ff0 CR3: 00000001cc4ed000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
ip_options_echo include/net/ip.h:574 [inline]
ip_cmsg_recv_retopts net/ipv4/ip_sockglue.c:91 [inline]
ip_cmsg_recv_offset+0xa17/0x1280 net/ipv4/ip_sockglue.c:207
udp_recvmsg+0xe0b/0x1260 net/ipv4/udp.c:1641
inet_recvmsg+0x14c/0x5f0 net/ipv4/af_inet.c:793
sock_recvmsg_nosec net/socket.c:792 [inline]
sock_recvmsg+0xc9/0x110 net/socket.c:799
SYSC_recvfrom+0x2dc/0x570 net/socket.c:1788
SyS_recvfrom+0x40/0x50 net/socket.c:1760
entry_SYSCALL_64_fastpath+0x1f/0xbe
RIP: 0033:0x444c89
RSP: 002b:00007ffd80c788e8 EFLAGS: 00000286 ORIG_RAX: 000000000000002d
RAX: ffffffffffffffda RBX: ffffffffffffffff RCX: 0000000000444c89
RDX: 0000000000000000 RSI: 0000000020bc0000 RDI: 0000000000000004
RBP: 0000000000000082 R08: 00000000200c0ff0 R09: 0000000000000010
R10: 0000000000000000 R11: 0000000000000286 R12: 0000000000402390
R13: 0000000000402420 R14: 0000000000000000 R15: 0000000000000000
Code: f6 c1 01 0f 85 a5 01 00 00 48 89 4d b8 e8 31 e9 6b fd 48 8b 4d b8
48 b8 00 00 00 00 00 fc ff df 48 83 e1 fe 48 89 ca 48 c1 ea 03 <80> 3c
02 00 0f 85 41 02 00 00 48 8b 09 48 b8 00 00 00 00 00 fc
RIP: __ip_options_echo+0xea8/0x1430 net/ipv4/ip_options.c:143 RSP:
ffff8801cc49f628
---[ end trace b30d95b284222843 ]---
Kernel panic - not syncing: Fatal exception
^ permalink raw reply
* [PATCH] radix-tree: must check __radix_tree_preload() return value
From: Eric Dumazet @ 2017-09-05 17:59 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Matthew Wilcox, Kirill A. Shutemov, Andrew Morton, netdev,
Linus Torvalds, linux-kernel
From: Eric Dumazet <edumazet@google.com>
__radix_tree_preload() only disables preemption if no error is returned.
So we really need to make sure callers always check the return value.
idr_preload() contract is to always disable preemption, so we need
to add a missing preempt_disable() if an error happened.
Similarly, ida_pre_get() only needs to call preempt_enable() in the
case no error happened.
Fixes: 0a835c4f090a ("Reimplement IDR and IDA using the radix tree")
Fixes: 7ad3d4d85c7a ("ida: Move ida_bitmap to a percpu variable")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: <stable@vger.kernel.org> [4.11+]
---
lib/radix-tree.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index 3527eb364964..fac702039304 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -463,7 +463,7 @@ radix_tree_node_free(struct radix_tree_node *node)
* To make use of this facility, the radix tree must be initialised without
* __GFP_DIRECT_RECLAIM being passed to INIT_RADIX_TREE().
*/
-static int __radix_tree_preload(gfp_t gfp_mask, unsigned nr)
+static __must_check int __radix_tree_preload(gfp_t gfp_mask, unsigned nr)
{
struct radix_tree_preload *rtp;
struct radix_tree_node *node;
@@ -2104,7 +2104,10 @@ EXPORT_SYMBOL(radix_tree_tagged);
*/
void idr_preload(gfp_t gfp_mask)
{
- __radix_tree_preload(gfp_mask, IDR_PRELOAD_SIZE);
+ int ret = __radix_tree_preload(gfp_mask, IDR_PRELOAD_SIZE);
+
+ if (ret)
+ preempt_disable();
}
EXPORT_SYMBOL(idr_preload);
@@ -2118,13 +2121,14 @@ EXPORT_SYMBOL(idr_preload);
*/
int ida_pre_get(struct ida *ida, gfp_t gfp)
{
- __radix_tree_preload(gfp, IDA_PRELOAD_SIZE);
+ int ret = __radix_tree_preload(gfp, IDA_PRELOAD_SIZE);
/*
* The IDA API has no preload_end() equivalent. Instead,
* ida_get_new() can return -EAGAIN, prompting the caller
* to return to the ida_pre_get() step.
*/
- preempt_enable();
+ if (!ret)
+ preempt_enable();
if (!this_cpu_read(ida_bitmap)) {
struct ida_bitmap *bitmap = kmalloc(sizeof(*bitmap), gfp);
^ permalink raw reply related
* Re: [PATCH] radix-tree: must check __radix_tree_preload() return value
From: Linus Torvalds @ 2017-09-05 18:07 UTC (permalink / raw)
To: Eric Dumazet
Cc: Matthew Wilcox, Matthew Wilcox, Kirill A. Shutemov, Andrew Morton,
netdev, linux-kernel
In-Reply-To: <1504634367.15310.59.camel@edumazet-glaptop3.roam.corp.google.com>
On Tue, Sep 5, 2017 at 10:59 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> @@ -2104,7 +2104,10 @@ EXPORT_SYMBOL(radix_tree_tagged);
> */
> void idr_preload(gfp_t gfp_mask)
> {
> - __radix_tree_preload(gfp_mask, IDR_PRELOAD_SIZE);
> + int ret = __radix_tree_preload(gfp_mask, IDR_PRELOAD_SIZE);
> +
> + if (ret)
> + preempt_disable();
> }
> EXPORT_SYMBOL(idr_preload);
Is there a reason for the "ret" variable that is entirely mis-named,
since it's never actually used as a return value?
(Sure. it's the return value of a function, but that is entirely
useless and pointless information, and adds no value. We should name
variables by the data they contain or how they are used, not by "it
was the return value of a function").
In other words, why isn't this just
if (__radix_tree_preload(..))
preempt_disable();
which is shorter and clearer and not confusing?
> @@ -2118,13 +2121,14 @@ EXPORT_SYMBOL(idr_preload);
> */
> int ida_pre_get(struct ida *ida, gfp_t gfp)
> {
> - __radix_tree_preload(gfp, IDA_PRELOAD_SIZE);
> + int ret = __radix_tree_preload(gfp, IDA_PRELOAD_SIZE);
> /*
> * The IDA API has no preload_end() equivalent. Instead,
> * ida_get_new() can return -EAGAIN, prompting the caller
> * to return to the ida_pre_get() step.
> */
> - preempt_enable();
> + if (!ret)
> + preempt_enable();
Same issue, but this time strengthened by an additional "why doesn't
this just use that idr_preload function then?" question..
Linus
^ permalink raw reply
* [PATCH 1/2] b43: fix unitialized reads of ret by initializing the array to zero
From: Colin King @ 2017-09-05 18:15 UTC (permalink / raw)
To: Kalle Valo, linux-wireless, b43-dev, netdev; +Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The u8 char array ret is not being initialized and elements outside
the range start to end contain just garbage values from the stack.
This results in a later scan of the array to read potentially
uninitialized values. Fix this by initializing the array to zero.
This seems to have been an issue since the very first commit.
Detected by CoverityScan CID#139652 ("Uninitialized scalar variable")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/wireless/broadcom/b43/phy_g.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/b43/phy_g.c b/drivers/net/wireless/broadcom/b43/phy_g.c
index 822dcaa8ace6..f59c02166462 100644
--- a/drivers/net/wireless/broadcom/b43/phy_g.c
+++ b/drivers/net/wireless/broadcom/b43/phy_g.c
@@ -2297,7 +2297,7 @@ static u8 b43_gphy_aci_detect(struct b43_wldev *dev, u8 channel)
static u8 b43_gphy_aci_scan(struct b43_wldev *dev)
{
struct b43_phy *phy = &dev->phy;
- u8 ret[13];
+ u8 ret[13] = { 0 };
unsigned int channel = phy->channel;
unsigned int i, j, start, end;
--
2.14.1
^ permalink raw reply related
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