* Re: [PATCH] slip: Check if rstate is initialized before uncompressing
From: David Miller @ 2018-04-09 15:04 UTC (permalink / raw)
To: tejaswit; +Cc: netdev
In-Reply-To: <20180409085345.GA5972@tejaswit-linux.qualcomm.com>
From: Tejaswi Tanikella <tejaswit@codeaurora.org>
Date: Mon, 9 Apr 2018 14:23:49 +0530
> @@ -673,6 +677,7 @@ struct slcompress *
> if (cs->cs_tcp.doff > 5)
> memcpy(cs->cs_tcpopt, icp + ihl*4 + sizeof(struct tcphdr), (cs->cs_tcp.doff - 5) * 4);
> cs->cs_hsize = ihl*2 + cs->cs_tcp.doff*2;
> + cs->initialized = 1;
> /* Put headers back on packet
...
> struct cstate {
> byte_t cs_this; /* connection id number (xmit) */
> + byte_t initialized; /* non-zero if initialized */
Please use 'bool' and true/false for 'initialized'.
^ permalink raw reply
* Re: [PATCH RESEND v2] vhost-net: set packet weight of tx polling to 2 * vq size
From: David Miller @ 2018-04-09 15:02 UTC (permalink / raw)
To: haibinzhang
Cc: mst, jasowang, kvm, virtualization, netdev, linux-kernel,
lidongchen, yunfangtai
In-Reply-To: <88D661ADF6AFBF42B2AB88D8E7682B0901FC4A32@EXMBX-SZMAIL011.tencent.com>
From: haibinzhang(张海斌) <haibinzhang@tencent.com>
Date: Mon, 9 Apr 2018 07:22:17 +0000
> handle_tx will delay rx for tens or even hundreds of milliseconds when tx busy
> polling udp packets with small length(e.g. 1byte udp payload), because setting
> VHOST_NET_WEIGHT takes into account only sent-bytes but no single packet length.
>
> Ping-Latencies shown below were tested between two Virtual Machines using
> netperf (UDP_STREAM, len=1), and then another machine pinged the client:
...
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Haibin Zhang <haibinzhang@tencent.com>
> Signed-off-by: Yunfang Tai <yunfangtai@tencent.com>
> Signed-off-by: Lidong Chen <lidongchen@tencent.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH v5] net: thunderx: rework mac addresses list to u64 array
From: David Miller @ 2018-04-09 15:00 UTC (permalink / raw)
To: Vadim.Lomovtsev
Cc: sgoutham, sunil.kovvuri, rric, linux-arm-kernel, netdev,
linux-kernel, dnelson, robin.murphy, hch, gustavo,
Vadim.Lomovtsev
In-Reply-To: <20180409132448.22278-1-Vadim.Lomovtsev@caviumnetworks.com>
From: Vadim Lomovtsev <Vadim.Lomovtsev@caviumnetworks.com>
Date: Mon, 9 Apr 2018 06:24:48 -0700
> From: Vadim Lomovtsev <Vadim.Lomovtsev@cavium.com>
>
> It is too expensive to pass u64 values via linked list, instead
> allocate array for them by overall number of mac addresses from netdev.
>
> This eventually removes multiple kmalloc() calls, aviod memory
> fragmentation and allow to put single null check on kmalloc
> return value in order to prevent a potential null pointer dereference.
>
> Addresses-Coverity-ID: 1467429 ("Dereference null return value")
> Fixes: 37c3347eb247 ("net: thunderx: add ndo_set_rx_mode callback implementation for VF")
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Vadim Lomovtsev <Vadim.Lomovtsev@cavium.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net] inetpeer: fix uninit-value in inet_getpeer
From: David Miller @ 2018-04-09 14:58 UTC (permalink / raw)
To: edumazet; +Cc: netdev, eric.dumazet
In-Reply-To: <20180409134327.22367-1-edumazet@google.com>
From: Eric Dumazet <edumazet@google.com>
Date: Mon, 9 Apr 2018 06:43:27 -0700
> syzbot/KMSAN reported that p->dtime was read while it was
> not yet initialized in :
>
> delta = (__u32)jiffies - p->dtime;
> if (delta < ttl || !refcount_dec_if_one(&p->refcnt))
> gc_stack[i] = NULL;
>
> This is a false positive, because the inetpeer wont be erased
> from rb-tree if the refcount_dec_if_one(&p->refcnt) does not
> succeed. And this wont happen before first inet_putpeer() call
> for this inetpeer has been done, and ->dtime field is written
> exactly before the refcount_dec_and_test(&p->refcnt).
>
> The KMSAN report was :
...
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: syzbot <syzkaller@googlegroups.com>
Applied, but it looks like we are just adding assignments simply
to placate these reports when the tools and facilities cannot
see through the logic properly.
^ permalink raw reply
* Re: [PATCH 2/2] alx: add disable_wol paramenter
From: David Miller @ 2018-04-09 14:50 UTC (permalink / raw)
To: andrew; +Cc: acelan.kao, jcliburn, chris.snook, rakesh, netdev, linux-kernel
In-Reply-To: <20180409123910.GB31060@lunn.ch>
From: Andrew Lunn <andrew@lunn.ch>
Date: Mon, 9 Apr 2018 14:39:10 +0200
> On Mon, Apr 09, 2018 at 07:35:14PM +0800, AceLan Kao wrote:
>> The WoL feature was reported broken and will lead to
>> the system resume immediately after suspending.
>> This symptom is not happening on every system, so adding
>> disable_wol option and disable WoL by default to prevent the issue from
>> happening again.
>
>> const char alx_drv_name[] = "alx";
>>
>> +/* disable WoL by default */
>> +bool disable_wol = 1;
>> +module_param(disable_wol, bool, 0);
>> +MODULE_PARM_DESC(disable_wol, "Disable Wake on Lan feature");
>> +
>
> Hi AceLan
>
> This seems like you are papering over the cracks. And module
> parameters are not liked.
>
> Please try to find the real problem.
Agreed.
^ permalink raw reply
* Re: [PATCH] net: decnet: Replace GFP_ATOMIC with GFP_KERNEL in dn_route_init
From: Jia-Ju Bai @ 2018-04-09 14:48 UTC (permalink / raw)
To: Eric Dumazet, davem, kafai, weiwan, dsa, johannes.berg, fw
Cc: linux-decnet-user, netdev, linux-kernel
In-Reply-To: <945ed4f2-e1e2-a038-2808-f18a79a35732@gmail.com>
On 2018/4/9 22:44, Eric Dumazet wrote:
>
> On 04/09/2018 07:10 AM, Jia-Ju Bai wrote:
>> dn_route_init() is never called in atomic context.
>>
>> The call chain ending up at dn_route_init() is:
>> [1] dn_route_init() <- decnet_init()
>> decnet_init() is only set as a parameter of module_init().
>>
>> Despite never getting called from atomic context,
>> dn_route_init() calls __get_free_pages() with GFP_ATOMIC,
>> which waits busily for allocation.
>> GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL,
>> to avoid busy waiting and improve the possibility of sucessful allocation.
>>
>> This is found by a static analysis tool named DCNS written by myself.
>> And I also manually check it.
>>
>> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
>> ---
>> net/decnet/dn_route.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
>> index 0bd3afd..59ed12a 100644
>> --- a/net/decnet/dn_route.c
>> +++ b/net/decnet/dn_route.c
>> @@ -1898,7 +1898,7 @@ void __init dn_route_init(void)
>> while(dn_rt_hash_mask & (dn_rt_hash_mask - 1))
>> dn_rt_hash_mask--;
>> dn_rt_hash_table = (struct dn_rt_hash_bucket *)
>> - __get_free_pages(GFP_ATOMIC, order);
>> + __get_free_pages(GFP_KERNEL, order);
>> } while (dn_rt_hash_table == NULL && --order > 0);
>>
>> if (!dn_rt_hash_table)
>>
> This might OOM under pressure.
Sorry, I do not understand this.
Could you please explain the reason?
> This would need __GFP_NOWARN | __GFP_NORETRY I guess, and would target net-next
>
Do you mean
__get_free_pages(__GFP_NOWARN | __GFP_NORETRY) or
__get_free_pages(__GFP_NOWARN | __GFP_NORETRY | GFP_KERNEL)?
Best wishes,
Jia-Ju Bai
^ permalink raw reply
* Re: [PATCH] net: dccp: Replace GFP_ATOMIC with GFP_KERNEL in dccp_init
From: Jia-Ju Bai @ 2018-04-09 14:46 UTC (permalink / raw)
To: Eric Dumazet, gerrit, davem; +Cc: dccp, netdev, linux-kernel
In-Reply-To: <8216dee8-5e52-421e-026a-4c75d654be54@gmail.com>
On 2018/4/9 22:42, Eric Dumazet wrote:
>
> On 04/09/2018 07:10 AM, Jia-Ju Bai wrote:
>> dccp_init() is never called in atomic context.
>> This function is only set as a parameter of module_init().
>>
>> Despite never getting called from atomic context,
>> dccp_init() calls __get_free_pages() with GFP_ATOMIC,
>> which waits busily for allocation.
> What do you mean by "waits busily" ?
>
> GFP_ATOMIC does not sleep, does not wait.
Sorry, I should modify it to "does not sleep".
Do you think it is okay?
Best wishes,
Jia-Ju Bai
^ permalink raw reply
* Re: [PATCH] net: decnet: Replace GFP_ATOMIC with GFP_KERNEL in dn_route_init
From: Eric Dumazet @ 2018-04-09 14:44 UTC (permalink / raw)
To: Jia-Ju Bai, davem, kafai, weiwan, dsa, johannes.berg, fw
Cc: linux-decnet-user, netdev, linux-kernel
In-Reply-To: <1523283035-25639-1-git-send-email-baijiaju1990@gmail.com>
On 04/09/2018 07:10 AM, Jia-Ju Bai wrote:
> dn_route_init() is never called in atomic context.
>
> The call chain ending up at dn_route_init() is:
> [1] dn_route_init() <- decnet_init()
> decnet_init() is only set as a parameter of module_init().
>
> Despite never getting called from atomic context,
> dn_route_init() calls __get_free_pages() with GFP_ATOMIC,
> which waits busily for allocation.
> GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL,
> to avoid busy waiting and improve the possibility of sucessful allocation.
>
> This is found by a static analysis tool named DCNS written by myself.
> And I also manually check it.
>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
> ---
> net/decnet/dn_route.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
> index 0bd3afd..59ed12a 100644
> --- a/net/decnet/dn_route.c
> +++ b/net/decnet/dn_route.c
> @@ -1898,7 +1898,7 @@ void __init dn_route_init(void)
> while(dn_rt_hash_mask & (dn_rt_hash_mask - 1))
> dn_rt_hash_mask--;
> dn_rt_hash_table = (struct dn_rt_hash_bucket *)
> - __get_free_pages(GFP_ATOMIC, order);
> + __get_free_pages(GFP_KERNEL, order);
> } while (dn_rt_hash_table == NULL && --order > 0);
>
> if (!dn_rt_hash_table)
>
This might OOM under pressure.
This would need __GFP_NOWARN | __GFP_NORETRY I guess, and would target net-next
^ permalink raw reply
* Re: WARNING in ip_rt_bug
From: David Miller @ 2018-04-09 14:43 UTC (permalink / raw)
To: dvyukov
Cc: syzbot+b09ac67a2af842b12eab, kuznet, linux-kernel, netdev,
syzkaller-bugs, yoshfuji, edumazet
In-Reply-To: <CACT4Y+YfM=SL3-cgeJ_7JCvAwX2KsyMWV=HHpGfLV=-RKo7TQQ@mail.gmail.com>
From: Dmitry Vyukov <dvyukov@google.com>
Date: Mon, 9 Apr 2018 08:06:20 +0200
> +Eric said that perhaps we just need to revert:
>
> commit c378a9c019cf5e017d1ed24954b54fae7bebd2bc
> Date: Sat May 21 07:16:42 2011 +0000
> ipv4: Give backtrace in ip_rt_bug().
And I replied to him that we shouldn't.
Reverting makes the backtrace, and all the useful debugging
information, go away. It won't fix the actual bug, which seems
to be that ICMP's route lookup tried to use an input route
for sending a packet.
^ permalink raw reply
* Re: [PATCH] netfilter: fix CONFIG_NF_REJECT_IPV6=m link error
From: Arnd Bergmann @ 2018-04-09 14:43 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Jozsef Kadlecsik, Florian Westphal, David S. Miller,
netfilter-devel, coreteam, Networking, Linux Kernel Mailing List
In-Reply-To: <20180409143741.dzooqrzcgi2bnvd5@salvia>
On Mon, Apr 9, 2018 at 4:37 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> Hi Arnd,
>
> On Mon, Apr 09, 2018 at 12:53:12PM +0200, Arnd Bergmann wrote:
>> We get a new link error with CONFIG_NFT_REJECT_INET=y and CONFIG_NF_REJECT_IPV6=m
>
> I think we can update NFT_REJECT_INET so it depends on NFT_REJECT_IPV4
> and NFT_REJECT_IPV6. This doesn't allow here CONFIG_NFT_REJECT_INET=y
> and CONFIG_NF_REJECT_IPV6=m.
>
> I mean, just like we do with NFT_FIB_INET.
That can only work if NFT_REJECT_INET can be made a 'tristate' symbol
again, so that code gets built as a loadable module if
CONFIG_NF_REJECT_IPV6=m.
> BTW, I think this problem has been is not related to the recent patch,
> but something older that kbuild robot has triggered more easily for
> some reason?
02c7b25e5f54 is the one that turned NF_TABLES_INET into a 'bool'
symbol. NFT_REJECT depends on NF_TABLES_INET, so it used to
restricted to a loadable module with IPV6=m, but can now be
built-in, which causes that link error.
Arnd
^ permalink raw reply
* Re: [PATCH] net: dccp: Replace GFP_ATOMIC with GFP_KERNEL in dccp_init
From: Eric Dumazet @ 2018-04-09 14:42 UTC (permalink / raw)
To: Jia-Ju Bai, gerrit, davem; +Cc: dccp, netdev, linux-kernel
In-Reply-To: <1523283004-25581-1-git-send-email-baijiaju1990@gmail.com>
On 04/09/2018 07:10 AM, Jia-Ju Bai wrote:
> dccp_init() is never called in atomic context.
> This function is only set as a parameter of module_init().
>
> Despite never getting called from atomic context,
> dccp_init() calls __get_free_pages() with GFP_ATOMIC,
> which waits busily for allocation.
What do you mean by "waits busily" ?
GFP_ATOMIC does not sleep, does not wait.
> GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL,
> to avoid busy waiting and improve the possibility of sucessful allocation.
>
> This is found by a static analysis tool named DCNS written by myself.
> And I also manually check it.
>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
> ---
^ permalink raw reply
* Re: [PATCH net] arp: fix arp_filter on l3slave devices
From: David Ahern @ 2018-04-09 14:41 UTC (permalink / raw)
To: Sasha Levin, Miguel Fadon Perlines, netdev@vger.kernel.org
Cc: stable@vger.kernel.org
In-Reply-To: <DM5PR2101MB1032EF9C8A7A3A1356D870CFFBBF0@DM5PR2101MB1032.namprd21.prod.outlook.com>
On 4/8/18 9:36 PM, Sasha Levin wrote:
> Hi,
>
> [This is an automated email]
>
> This commit has been processed by the -stable helper bot and determined
> to be a high probability candidate for -stable trees. (score: 33.5930)
>
> The bot has tested the following trees: v4.16, v4.15.15, v4.14.32, v4.9.92, v4.4.126.
>
> v4.16: Build OK!
> v4.15.15: Build OK!
> v4.14.32: Build OK!
> v4.9.92: Build OK!
> v4.4.126: Build OK!
All of those would be good for this patch. Thanks
^ permalink raw reply
* Re: [PATCH] netfilter: fix CONFIG_NF_REJECT_IPV6=m link error
From: Pablo Neira Ayuso @ 2018-04-09 14:37 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Jozsef Kadlecsik, Florian Westphal, David S. Miller,
netfilter-devel, coreteam, netdev, linux-kernel
In-Reply-To: <20180409105322.2247296-1-arnd@arndb.de>
[-- Attachment #1: Type: text/plain, Size: 549 bytes --]
Hi Arnd,
On Mon, Apr 09, 2018 at 12:53:12PM +0200, Arnd Bergmann wrote:
> We get a new link error with CONFIG_NFT_REJECT_INET=y and CONFIG_NF_REJECT_IPV6=m
I think we can update NFT_REJECT_INET so it depends on NFT_REJECT_IPV4
and NFT_REJECT_IPV6. This doesn't allow here CONFIG_NFT_REJECT_INET=y
and CONFIG_NF_REJECT_IPV6=m.
I mean, just like we do with NFT_FIB_INET.
BTW, I think this problem has been is not related to the recent patch,
but something older that kbuild robot has triggered more easily for
some reason?
Thanks for your patch!
[-- Attachment #2: x.patch --]
[-- Type: text/x-diff, Size: 358 bytes --]
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index d3220b43c832..b48c57bb9aaf 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -601,7 +601,8 @@ config NFT_REJECT
config NFT_REJECT_INET
depends on NF_TABLES_INET
- default NFT_REJECT
+ depends on NFT_REJECT_IPV4
+ depends on NFT_REJECT_IPV6
tristate
config NFT_COMPAT
^ permalink raw reply related
* Re: Enable and configure storm prevention in a network device
From: Murali Karicheri @ 2018-04-09 14:33 UTC (permalink / raw)
To: Florian Fainelli, David Miller; +Cc: netdev, andrew
In-Reply-To: <75eb56a4-ded2-5ed5-116c-776312f93cf3@gmail.com>
On 04/05/2018 06:35 PM, Florian Fainelli wrote:
> On 04/05/2018 01:20 PM, David Miller wrote:
>> From: Murali Karicheri <m-karicheri2@ti.com>
>> Date: Thu, 5 Apr 2018 16:14:49 -0400
>>
>>> Is there a standard way to implement and configure storm prevention
>>> in a Linux network device?
>>
>> What kind of "storm", an interrupt storm?
>>
>
> I would assume Murali is referring to L2 broadcast storms which is
> common in switches. There is not an API for that AFAICT and I am not
> sure what a proper API would look like.
>
Thanks Florian for adding more details. Yes, I am referring to
L2 broadcast or multicast storm. At this point I don't see a reason
why to exclude unicast storm as well if the hardware has the capability.
--
Murali Karicheri
Linux Kernel, Keystone
^ permalink raw reply
* Re: [PATCH net] net: dsa: mv88e6xxx: Fix receive time stamp race condition.
From: Richard Cochran @ 2018-04-09 14:19 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, Andrew Lunn, David Miller, Florian Fainelli,
Vivien Didelot
In-Reply-To: <20180409070314.12409-1-richardcochran@gmail.com>
On Mon, Apr 09, 2018 at 12:03:14AM -0700, Richard Cochran wrote:
> This patch fixes the race by moving the queue onto a list on the stack
> before reading out the latched time stamp value.
>
> Fixes: c6fe0ad2c3499 ("net: dsa: mv88e6xxx: add rx/tx timestamping support")
Dave, please hold off on this patch. I am seeing new problems in my
testing with this applied. I still need to get to the bottom of
this.
Thanks,
Richard
^ permalink raw reply
* [PATCH] net: tipc: Replace GFP_ATOMIC with GFP_KERNEL in tipc_mon_create
From: Jia-Ju Bai @ 2018-04-09 14:11 UTC (permalink / raw)
To: jon.maloy, ying.xue, davem
Cc: netdev, tipc-discussion, linux-kernel, Jia-Ju Bai
tipc_mon_create() is never called in atomic context.
The call chain ending up at dn_route_init() is:
[1] tipc_mon_create() <- tipc_enable_bearer() <- tipc_nl_bearer_enable()
tipc_nl_bearer_enable() calls rtnl_lock(), which indicates this function
is not called in atomic context.
Despite never getting called from atomic context,
tipc_mon_create() calls kzalloc() with GFP_ATOMIC,
which waits busily for allocation.
GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL,
to avoid busy waiting and improve the possibility of sucessful allocation.
This is found by a static analysis tool named DCNS written by myself.
And I also manually check it.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
net/tipc/monitor.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/tipc/monitor.c b/net/tipc/monitor.c
index 9e109bb..9714d80 100644
--- a/net/tipc/monitor.c
+++ b/net/tipc/monitor.c
@@ -604,9 +604,9 @@ int tipc_mon_create(struct net *net, int bearer_id)
if (tn->monitors[bearer_id])
return 0;
- mon = kzalloc(sizeof(*mon), GFP_ATOMIC);
- self = kzalloc(sizeof(*self), GFP_ATOMIC);
- dom = kzalloc(sizeof(*dom), GFP_ATOMIC);
+ mon = kzalloc(sizeof(*mon), GFP_KERNEL);
+ self = kzalloc(sizeof(*self), GFP_KERNEL);
+ dom = kzalloc(sizeof(*dom), GFP_KERNEL);
if (!mon || !self || !dom) {
kfree(mon);
kfree(self);
--
1.9.1
^ permalink raw reply related
* [PATCH] net: nsci: Replace GFP_ATOMIC with GFP_KERNEL in ncsi_register_dev
From: Jia-Ju Bai @ 2018-04-09 14:11 UTC (permalink / raw)
To: davem, arnd, sam; +Cc: netdev, linux-kernel, Jia-Ju Bai
ncsi_register_dev() is never called in atomic context.
This function is only called by ftgmac100_probe() in
drivers/net/ethernet/faraday/ftgmac100.c.
And ftgmac100_probe() is only set as ".probe" in "struct platform_driver".
Despite never getting called from atomic context,
ncsi_register_dev() calls kzalloc() with GFP_ATOMIC,
which waits busily for allocation.
GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL,
to avoid busy waiting and improve the possibility of sucessful allocation.
This is found by a static analysis tool named DCNS written by myself.
And I also manually check it.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
net/ncsi/ncsi-manage.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 3fd3c39..6b5b5a0 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -1508,7 +1508,7 @@ struct ncsi_dev *ncsi_register_dev(struct net_device *dev,
return nd;
/* Create NCSI device */
- ndp = kzalloc(sizeof(*ndp), GFP_ATOMIC);
+ ndp = kzalloc(sizeof(*ndp), GFP_KERNEL);
if (!ndp)
return NULL;
--
1.9.1
^ permalink raw reply related
* [PATCH] net: decnet: Replace GFP_ATOMIC with GFP_KERNEL in dn_route_init
From: Jia-Ju Bai @ 2018-04-09 14:10 UTC (permalink / raw)
To: davem, kafai, weiwan, dsa, johannes.berg, fw
Cc: linux-decnet-user, netdev, linux-kernel, Jia-Ju Bai
dn_route_init() is never called in atomic context.
The call chain ending up at dn_route_init() is:
[1] dn_route_init() <- decnet_init()
decnet_init() is only set as a parameter of module_init().
Despite never getting called from atomic context,
dn_route_init() calls __get_free_pages() with GFP_ATOMIC,
which waits busily for allocation.
GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL,
to avoid busy waiting and improve the possibility of sucessful allocation.
This is found by a static analysis tool named DCNS written by myself.
And I also manually check it.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
net/decnet/dn_route.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
index 0bd3afd..59ed12a 100644
--- a/net/decnet/dn_route.c
+++ b/net/decnet/dn_route.c
@@ -1898,7 +1898,7 @@ void __init dn_route_init(void)
while(dn_rt_hash_mask & (dn_rt_hash_mask - 1))
dn_rt_hash_mask--;
dn_rt_hash_table = (struct dn_rt_hash_bucket *)
- __get_free_pages(GFP_ATOMIC, order);
+ __get_free_pages(GFP_KERNEL, order);
} while (dn_rt_hash_table == NULL && --order > 0);
if (!dn_rt_hash_table)
--
1.9.1
^ permalink raw reply related
* [PATCH] net: dccp: Replace GFP_ATOMIC with GFP_KERNEL in dccp_init
From: Jia-Ju Bai @ 2018-04-09 14:10 UTC (permalink / raw)
To: gerrit, davem; +Cc: dccp, netdev, linux-kernel, Jia-Ju Bai
dccp_init() is never called in atomic context.
This function is only set as a parameter of module_init().
Despite never getting called from atomic context,
dccp_init() calls __get_free_pages() with GFP_ATOMIC,
which waits busily for allocation.
GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL,
to avoid busy waiting and improve the possibility of sucessful allocation.
This is found by a static analysis tool named DCNS written by myself.
And I also manually check it.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
net/dccp/proto.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/dccp/proto.c b/net/dccp/proto.c
index b68168f..f63ba93 100644
--- a/net/dccp/proto.c
+++ b/net/dccp/proto.c
@@ -1159,7 +1159,7 @@ static int __init dccp_init(void)
hash_size--;
dccp_hashinfo.ehash_mask = hash_size - 1;
dccp_hashinfo.ehash = (struct inet_ehash_bucket *)
- __get_free_pages(GFP_ATOMIC|__GFP_NOWARN, ehash_order);
+ __get_free_pages(GFP_KERNEL|__GFP_NOWARN, ehash_order);
} while (!dccp_hashinfo.ehash && --ehash_order > 0);
if (!dccp_hashinfo.ehash) {
@@ -1182,7 +1182,7 @@ static int __init dccp_init(void)
bhash_order > 0)
continue;
dccp_hashinfo.bhash = (struct inet_bind_hashbucket *)
- __get_free_pages(GFP_ATOMIC|__GFP_NOWARN, bhash_order);
+ __get_free_pages(GFP_KERNEL|__GFP_NOWARN, bhash_order);
} while (!dccp_hashinfo.bhash && --bhash_order >= 0);
if (!dccp_hashinfo.bhash) {
--
1.9.1
^ permalink raw reply related
* [PATCH net] inetpeer: fix uninit-value in inet_getpeer
From: Eric Dumazet @ 2018-04-09 13:43 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
syzbot/KMSAN reported that p->dtime was read while it was
not yet initialized in :
delta = (__u32)jiffies - p->dtime;
if (delta < ttl || !refcount_dec_if_one(&p->refcnt))
gc_stack[i] = NULL;
This is a false positive, because the inetpeer wont be erased
from rb-tree if the refcount_dec_if_one(&p->refcnt) does not
succeed. And this wont happen before first inet_putpeer() call
for this inetpeer has been done, and ->dtime field is written
exactly before the refcount_dec_and_test(&p->refcnt).
The KMSAN report was :
BUG: KMSAN: uninit-value in inet_peer_gc net/ipv4/inetpeer.c:163 [inline]
BUG: KMSAN: uninit-value in inet_getpeer+0x1567/0x1e70 net/ipv4/inetpeer.c:228
CPU: 0 PID: 9494 Comm: syz-executor5 Not tainted 4.16.0+ #82
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:17 [inline]
dump_stack+0x185/0x1d0 lib/dump_stack.c:53
kmsan_report+0x142/0x240 mm/kmsan/kmsan.c:1067
__msan_warning_32+0x6c/0xb0 mm/kmsan/kmsan_instr.c:676
inet_peer_gc net/ipv4/inetpeer.c:163 [inline]
inet_getpeer+0x1567/0x1e70 net/ipv4/inetpeer.c:228
inet_getpeer_v4 include/net/inetpeer.h:110 [inline]
icmpv4_xrlim_allow net/ipv4/icmp.c:330 [inline]
icmp_send+0x2b44/0x3050 net/ipv4/icmp.c:725
ip_options_compile+0x237c/0x29f0 net/ipv4/ip_options.c:472
ip_rcv_options net/ipv4/ip_input.c:284 [inline]
ip_rcv_finish+0xda8/0x16d0 net/ipv4/ip_input.c:365
NF_HOOK include/linux/netfilter.h:288 [inline]
ip_rcv+0x119d/0x16f0 net/ipv4/ip_input.c:493
__netif_receive_skb_core+0x47cf/0x4a80 net/core/dev.c:4562
__netif_receive_skb net/core/dev.c:4627 [inline]
netif_receive_skb_internal+0x49d/0x630 net/core/dev.c:4701
netif_receive_skb+0x230/0x240 net/core/dev.c:4725
tun_rx_batched drivers/net/tun.c:1555 [inline]
tun_get_user+0x6d88/0x7580 drivers/net/tun.c:1962
tun_chr_write_iter+0x1d4/0x330 drivers/net/tun.c:1990
do_iter_readv_writev+0x7bb/0x970 include/linux/fs.h:1776
do_iter_write+0x30d/0xd40 fs/read_write.c:932
vfs_writev fs/read_write.c:977 [inline]
do_writev+0x3c9/0x830 fs/read_write.c:1012
SYSC_writev+0x9b/0xb0 fs/read_write.c:1085
SyS_writev+0x56/0x80 fs/read_write.c:1082
do_syscall_64+0x309/0x430 arch/x86/entry/common.c:287
entry_SYSCALL_64_after_hwframe+0x3d/0xa2
RIP: 0033:0x455111
RSP: 002b:00007fae0365cba0 EFLAGS: 00000293 ORIG_RAX: 0000000000000014
RAX: ffffffffffffffda RBX: 000000000000002e RCX: 0000000000455111
RDX: 0000000000000001 RSI: 00007fae0365cbf0 RDI: 00000000000000fc
RBP: 0000000020000040 R08: 00000000000000fc R09: 0000000000000000
R10: 000000000000002e R11: 0000000000000293 R12: 00000000ffffffff
R13: 0000000000000658 R14: 00000000006fc8e0 R15: 0000000000000000
Uninit was created at:
kmsan_save_stack_with_flags mm/kmsan/kmsan.c:278 [inline]
kmsan_internal_poison_shadow+0xb8/0x1b0 mm/kmsan/kmsan.c:188
kmsan_kmalloc+0x94/0x100 mm/kmsan/kmsan.c:314
kmem_cache_alloc+0xaab/0xb90 mm/slub.c:2756
inet_getpeer+0xed8/0x1e70 net/ipv4/inetpeer.c:210
inet_getpeer_v4 include/net/inetpeer.h:110 [inline]
ip4_frag_init+0x4d1/0x740 net/ipv4/ip_fragment.c:153
inet_frag_alloc net/ipv4/inet_fragment.c:369 [inline]
inet_frag_create net/ipv4/inet_fragment.c:385 [inline]
inet_frag_find+0x7da/0x1610 net/ipv4/inet_fragment.c:418
ip_find net/ipv4/ip_fragment.c:275 [inline]
ip_defrag+0x448/0x67a0 net/ipv4/ip_fragment.c:676
ip_check_defrag+0x775/0xda0 net/ipv4/ip_fragment.c:724
packet_rcv_fanout+0x2a8/0x8d0 net/packet/af_packet.c:1447
deliver_skb net/core/dev.c:1897 [inline]
deliver_ptype_list_skb net/core/dev.c:1912 [inline]
__netif_receive_skb_core+0x314a/0x4a80 net/core/dev.c:4545
__netif_receive_skb net/core/dev.c:4627 [inline]
netif_receive_skb_internal+0x49d/0x630 net/core/dev.c:4701
netif_receive_skb+0x230/0x240 net/core/dev.c:4725
tun_rx_batched drivers/net/tun.c:1555 [inline]
tun_get_user+0x6d88/0x7580 drivers/net/tun.c:1962
tun_chr_write_iter+0x1d4/0x330 drivers/net/tun.c:1990
do_iter_readv_writev+0x7bb/0x970 include/linux/fs.h:1776
do_iter_write+0x30d/0xd40 fs/read_write.c:932
vfs_writev fs/read_write.c:977 [inline]
do_writev+0x3c9/0x830 fs/read_write.c:1012
SYSC_writev+0x9b/0xb0 fs/read_write.c:1085
SyS_writev+0x56/0x80 fs/read_write.c:1082
do_syscall_64+0x309/0x430 arch/x86/entry/common.c:287
entry_SYSCALL_64_after_hwframe+0x3d/0xa2
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
---
net/ipv4/inetpeer.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/ipv4/inetpeer.c b/net/ipv4/inetpeer.c
index 1f04bd91fc2e999ddb82f4be92d39d229166b691..d757b9642d0d1c418bffad5bcd50e8e7bf336c66 100644
--- a/net/ipv4/inetpeer.c
+++ b/net/ipv4/inetpeer.c
@@ -211,6 +211,7 @@ struct inet_peer *inet_getpeer(struct inet_peer_base *base,
p = kmem_cache_alloc(peer_cachep, GFP_ATOMIC);
if (p) {
p->daddr = *daddr;
+ p->dtime = (__u32)jiffies;
refcount_set(&p->refcnt, 2);
atomic_set(&p->rid, 0);
p->metrics[RTAX_LOCK-1] = INETPEER_METRICS_NEW;
--
2.17.0.484.g0c8726318c-goog
^ permalink raw reply related
* Re: [RFC bpf-next] bpf: document eBPF helpers and add a script to generate man page
From: Quentin Monnet @ 2018-04-09 13:33 UTC (permalink / raw)
To: Markus Heiser, Daniel Borkmann
Cc: Jonathan Corbet, ast, netdev, oss-drivers, Linux Doc Mailing List,
linux-man
In-Reply-To: <550CD0C6-10B5-4C8C-9C1E-70AA61ABDC34@darmarit.de>
2018-04-09 12:52 UTC+0200 ~ Markus Heiser <markus.heiser@darmarit.de>
>
>> Am 09.04.2018 um 12:08 schrieb Daniel Borkmann <daniel@iogearbox.net>:
> [...]
>
>>> May I completely misunderstood you, so correct my if I'am wrong:
>>>
>>> - ./scripts/bpf_helpers_doc.py : produces reST markup from C-comments
>>> - ./scripts/kerne-doc : produces reST markup from C-comments
>>>
>>> IMO: both are doing the same job, so why not using kernel-doc?
>>
>> They are not really doing the same job, in bpf_helpers_doc.py case you don't
>> want the whole header rendered, but just a fraction of it, that is, the
>> single big comment which describes all BPF helper functions that a BPF
>> program developer has available to use in user space - aka the entries in
>> the __BPF_FUNC_MAPPER() macro;
>
>
>> I also doubt the latter would actually qualify
>> in kdoc context as some sort of a function description.
>
> latter .. ah, OK .. thanks for clarifying.
>
> -- Markus --
As Daniel explained, kernel-doc does not apply in this case, we do not
have the full function prototype for eBPF helpers in the header file.
But to be honest, I didn't even realise kernel-doc was available and
could do something close to what I was looking for, so thanks for your
feedback! :)
Quentin
^ permalink raw reply
* v4.16 in_dev_finish_destroy() accessing freed idev->dev->pcpu_refcnt
From: Mark Rutland @ 2018-04-09 13:30 UTC (permalink / raw)
To: netdev, linux-kernel
Hi all,
As a heads-up, while fuzzing v4.16 on arm64, I'm hitting an intermittent
issue where in_dev_finish_destroy() calls dev_put() on idev->dev, where
idev->dev->pcpu_refcnt is NULL. Apparently idev->dev has already been
freed.
This results in a fault where we try to access the percpu offset of
NULL, such as in the example splat at the end of this mail.
So far I've had no luck in extracting a reliable reproducer, but I've
uploaded the relevant syzkaller logs (and reports) to my kernel.org
webspace [1].
Thanks,
Mark.
[1] https://www.kernel.org/pub/linux/kernel/people/mark/bugs/20180409-in_dev_finish_destroy-null-pcpu_refcnt/
Unable to handle kernel paging request at virtual address 60002be80000
Mem abort info:
ESR = 0x96000004
Exception class = DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
Data abort info:
ISV = 0, ISS = 0x00000004
CM = 0, WnR = 0
user pgtable: 4k pages, 48-bit VAs, pgdp = 00000000fb56b784
[000060002be80000] pgd=0000000000000000
Internal error: Oops: 96000004 [#1] PREEMPT SMP
Modules linked in:
CPU: 2 PID: 22 Comm: ksoftirqd/2 Not tainted 4.16.0-00006-g6eee13dfb529 #1
Hardware name: linux,dummy-virt (DT)
pstate: 80400005 (Nzcv daif +PAN -UAO)
pc : __percpu_add arch/arm64/include/asm/percpu.h:102 [inline]
pc : dev_put include/linux/netdevice.h:3395 [inline]
pc : in_dev_finish_destroy+0xcc/0x230 net/ipv4/devinet.c:231
lr : dev_put include/linux/netdevice.h:3395 [inline]
lr : in_dev_finish_destroy+0xa4/0x230 net/ipv4/devinet.c:231
sp : ffff800036317b70
x29: ffff800036317b70 x28: ffff80001f39d338
x27: ffff20000a917460 x26: ffff20000a917000
x25: 000000000000000a x24: ffff20000a919000
x23: ffff800036317c90 x22: ffff200009b84da8
x21: 1ffff00003e73a68 x20: ffff80006e7a9100
x19: ffff80001f39d180 x18: 0000000000000000
x17: 0000000000000000 x16: ffff2000082951f0
x15: 0000000000000001 x14: 00000000f2000000
x13: ffff20000a9fb560 x12: 0000000000000002
x11: 1ffff0000c147fbb x10: 0000000000000004
x9 : 0000000000000000 x8 : 1fffe4000178908c
x7 : 0000000000000000 x6 : 0000000000000000
x5 : 0000000000000000 x4 : 1fffe4000178908c
x3 : 1fffe4000178908c x2 : ffffffffffffffff
x1 : 000060002be80000 x0 : 000060002be80000
Process ksoftirqd/2 (pid: 22, stack limit = 0x00000000e96adbb2)
Call trace:
__percpu_add arch/arm64/include/asm/percpu.h:102 [inline]
dev_put include/linux/netdevice.h:3395 [inline]
in_dev_finish_destroy+0xcc/0x230 net/ipv4/devinet.c:231
in_dev_put include/linux/inetdevice.h:248 [inline]
in_dev_rcu_put+0x34/0x48 net/ipv4/devinet.c:287
__rcu_reclaim kernel/rcu/rcu.h:172 [inline]
rcu_do_batch kernel/rcu/tree.c:2674 [inline]
invoke_rcu_callbacks kernel/rcu/tree.c:2933 [inline]
__rcu_process_callbacks kernel/rcu/tree.c:2900 [inline]
rcu_process_callbacks+0x350/0x988 kernel/rcu/tree.c:2917
__do_softirq+0x318/0x734 kernel/softirq.c:285
run_ksoftirqd+0x70/0xa8 kernel/softirq.c:666
smpboot_thread_fn+0x544/0x9d0 kernel/smpboot.c:164
kthread+0x2f8/0x380 kernel/kthread.c:238
ret_from_fork+0x10/0x18 arch/arm64/kernel/entry.S:1158
Code: d538d081 f9425a80 92800002 8b010000 (885f7c04)
---[ end trace 577c2c0fbbf0d4d4 ]---
^ permalink raw reply
* Re: [RFC bpf-next] bpf: document eBPF helpers and add a script to generate man page
From: Quentin Monnet @ 2018-04-09 13:25 UTC (permalink / raw)
To: Daniel Borkmann, ast; +Cc: netdev, oss-drivers, linux-doc, linux-man
In-Reply-To: <05d2d03a-0b39-9d0c-9ba0-3461afc45fac@iogearbox.net>
2018-04-09 11:01 UTC+0200 ~ Daniel Borkmann <daniel@iogearbox.net>
> On 04/06/2018 01:11 PM, Quentin Monnet wrote:
>> eBPF helper functions can be called from within eBPF programs to perform
>> a variety of tasks that would be otherwise hard or impossible to do with
>> eBPF itself. There is a growing number of such helper functions in the
>> kernel, but documentation is scarce. The main user space header file
>> does contain a short commented description of most helpers, but it is
>> somewhat outdated and not complete. It is more a "cheat sheet" than a
>> real documentation accessible to new eBPF developers.
>>
>> This commit attempts to improve the situation by replacing the existing
>> overview for the helpers with a more developed description. Furthermore,
>> a Python script is added to generate a manual page for eBPF helpers. The
>> workflow is the following, and requires the rst2man utility:
>>
>> $ ./scripts/bpf_helpers_doc.py \
>> --filename include/uapi/linux/bpf.h > /tmp/bpf-helpers.rst
>> $ rst2man /tmp/bpf-helpers.rst > /tmp/bpf-helpers.7
>> $ man /tmp/bpf-helpers.7
>>
>> The objective is to keep all documentation related to the helpers in a
>> single place, and to be able to generate from here a manual page that
>> could be packaged in the man-pages repository and shipped with most
>> distributions [1].
>>
>> Additionally, parsing the prototypes of the helper functions could
>> hopefully be reused, with a different Printer object, to generate
>> header files needed in some eBPF-related projects.
>>
>> Regarding the description of each helper, it comprises several items:
>>
>> - The function prototype.
>> - A description of the function and of its arguments (except for a
>> couple of cases, when there are no arguments and the return value
>> makes the function usage really obvious).
>> - A description of return values (if not void).
>> - A listing of eBPF program types (if relevant, map types) compatible
>> with the helper.
>> - Information about the helper being restricted to GPL programs, or not.
>> - The kernel version in which the helper was introduced.
>> - The commit that introduced the helper (this is mostly to have it in
>> the source of the man page, as it can be used to track changes and
>> update the page).
>>
>> For several helpers, descriptions are inspired (at times, nearly copied)
>> from the commit logs introducing them in the kernel--Many thanks to
>> their respective authors! They were completed as much as possible, the
>> objective being to have something easily accessible even for people just
>> starting with eBPF. There is probably a bit more work to do in this
>> direction for some helpers.
>>
>> Some RST formatting is used in the descriptions (not in function
>> prototypes, to keep them readable, but the Python script provided in
>> order to generate the RST for the manual page does add formatting to
>> prototypes, to produce something pretty) to get "bold" and "italics" in
>> manual pages. Hopefully, the descriptions in bpf.h file remains
>> perfectly readable. Note that the few trailing white spaces are
>> intentional, removing them would break paragraphs for rst2man.
>>
>> The descriptions should ideally be updated each time someone adds a new
>> helper, or updates the behaviour (compatibility extended to new program
>> types, new socket option supported...) or the interface (new flags
>> available, ...) of existing ones.
>>
>> [1] I have not contacted people from the man-pages project prior to
>> sending this RFC, so I can offer no guaranty at this time that they
>> would accept to take the generated man page.
>>
>> Cc: linux-doc@vger.kernel.org
>> Cc: linux-man@vger.kernel.org
>> Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
>
> Great work, thanks a lot for doing this!
>
> [...]
>> + * int bpf_probe_read(void *dst, u32 size, const void *src)
>> + * Description
>> + * For tracing programs, safely attempt to read *size* bytes from
>> + * address *src* and store the data in *dst*.
>> + * Return
>> + * 0 on success, or a negative error in case of failure.
>> + * For
>> + * *Tracing programs*.
>> + * GPL only
>> + * Yes
>> + * Since
>> + * Linux 4.1
>> + *
>> + * .. commit 2541517c32be
>> *
>> * u64 bpf_ktime_get_ns(void)
>> - * Return: current ktime
>> - *
>> - * int bpf_trace_printk(const char *fmt, int fmt_size, ...)
>> - * Return: length of buffer written or negative error
>> + * Description
>> + * Return the time elapsed since system boot, in nanoseconds.
>> + * Return
>> + * Current *ktime*.
>> + * For
>> + * All program types, except
>> + * **BPF_PROG_TYPE_CGROUP_DEVICE**.
>
> I think we should probably always just list the actual program types instead,
> since when new types get added to the kernel not supporting bpf_ktime_get_ns()
> helper in this example, the above statement would be misleading (potentially
> more misleading than the other way around when it's not yet mentioned to be
> supported).
Agreed. I realise “All program types” is really awkward here.
>> + * GPL only
>> + * Yes
>> + * Since
>> + * Linux 4.1
>> + *
>> + * .. commit d9847d310ab4
>> + *
[...]
>> + * u32 bpf_get_smp_processor_id(void)
>> + * Return
>> + * The SMP (Symmetric multiprocessing) processor id.
>> + * For
>> + * *Networking programs*.
>
> Ditto plus above is actually not correct. See tracing_func_proto():
>
> [...]
> case BPF_FUNC_get_smp_processor_id:
> return &bpf_get_smp_processor_id_proto;
> [...]
Thanks for the catch! I will fix on my side, even if we drop the "For"
section for now.
>> + * GPL only
>> + * No
>> + * Since
>> + * Linux 4.1
>> + *
>> + * .. commit c04167ce2ca0
>> + *
>> + * int bpf_skb_store_bytes(struct sk_buff *skb, u32 offset, const void *from, u32 len, u64 flags)
>> + * Description
>> + * Store *len* bytes from address *from* into the packet
>> + * associated to *skb*, at *offset*. *flags* are a combination of
>> + * **BPF_F_RECOMPUTE_CSUM** (automatically recompute the
>> + * checksum for the packet after storing the bytes) and
>> + * **BPF_F_INVALIDATE_HASH** (set *skb*\ **->hash**, *skb*\
>> + * **->swhash** and *skb*\ **->l4hash** to 0).
>> + *
>> + * A call to this helper is susceptible to change data from the
>> + * packet. Therefore, at load time, all checks on pointers
>> + * previously done by the verifier are invalidated and must be
>> + * performed again.
>> + * Return
> [...]
>
> Agree with Alexei that 'Description' but also 'Return' is the most useful
> (the latter we can still improve a bit wrt errors). 'For' gets indeed quickly
> out of hand but I do think that for BPF program developers 'Since' has good
> value to quickly check when a helper could be available, but then again
> this is actually tied to an individual kconfig and/or program type, and could
> potentially require to add or (worst case) remove the info in a second commit
> e.g. after the merge window similar with the commit sha. Probably best indeed
> to stick to the signature, 'Description' and 'Return' initially.
I understand that the "For" section will be the hardest to maintain
up-to-date, however, I believe that--along with minimal kernel version
and GPL information--it would be useful for readers (Alexei, what is
your case against the "GPL only" section?). Daniel, your proposal for
the "Since" information in a second time sounds good! But I do not have
a solution right now for maintaining the "For" section on the long term.
Anyway, I am fine with keeping just signatures, descriptions and return
values for now. I will submit a new version with only those items.
> We should probably also have the bpf_helpers_doc.py workflow in a separate
> comment above this one such that developers don't have to look up the original
> commit message, but have the required steps to render the rst/man page available
> right where they need it.
Sure! I was considering adding some comments on top of the description,
but haven't done it for the RFC. I will do for the next version.
Thanks for the reviews!
Quentin
^ permalink raw reply
* [PATCH v5] net: thunderx: rework mac addresses list to u64 array
From: Vadim Lomovtsev @ 2018-04-09 13:24 UTC (permalink / raw)
To: sgoutham, sunil.kovvuri, rric, linux-arm-kernel, netdev,
linux-kernel, davem
Cc: dnelson, robin.murphy, hch, gustavo, Vadim Lomovtsev
In-Reply-To: <20180406195354.16037-1-Vadim.Lomovtsev@caviumnetworks.com>
From: Vadim Lomovtsev <Vadim.Lomovtsev@cavium.com>
It is too expensive to pass u64 values via linked list, instead
allocate array for them by overall number of mac addresses from netdev.
This eventually removes multiple kmalloc() calls, aviod memory
fragmentation and allow to put single null check on kmalloc
return value in order to prevent a potential null pointer dereference.
Addresses-Coverity-ID: 1467429 ("Dereference null return value")
Fixes: 37c3347eb247 ("net: thunderx: add ndo_set_rx_mode callback implementation for VF")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Vadim Lomovtsev <Vadim.Lomovtsev@cavium.com>
---
Changes from v1 to v2:
- C99 syntax: update xcast_addr_list struct field mc[0] -> mc[];
Changes from v2 to v3:
- update commit description with 'Reported-by: Dan Carpenter';
- update size calculations for mc list to offsetof() call
instead of explicit arithmetic;
Changes from v3 to v4:
- change loop control variable type from u8 to int, accordingly
to mc_count size;
Changes from v4 to v5:
- remove explicit initialization of 'idx' variable as per review comment;
---
drivers/net/ethernet/cavium/thunder/nic.h | 7 +-----
drivers/net/ethernet/cavium/thunder/nicvf_main.c | 28 +++++++++---------------
2 files changed, 11 insertions(+), 24 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/nic.h b/drivers/net/ethernet/cavium/thunder/nic.h
index 5fc46c5a4f36..448d1fafc827 100644
--- a/drivers/net/ethernet/cavium/thunder/nic.h
+++ b/drivers/net/ethernet/cavium/thunder/nic.h
@@ -265,14 +265,9 @@ struct nicvf_drv_stats {
struct cavium_ptp;
-struct xcast_addr {
- struct list_head list;
- u64 addr;
-};
-
struct xcast_addr_list {
- struct list_head list;
int count;
+ u64 mc[];
};
struct nicvf_work {
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index 1e9a31fef729..707db3304396 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -1929,7 +1929,7 @@ static void nicvf_set_rx_mode_task(struct work_struct *work_arg)
work.work);
struct nicvf *nic = container_of(vf_work, struct nicvf, rx_mode_work);
union nic_mbx mbx = {};
- struct xcast_addr *xaddr, *next;
+ int idx;
if (!vf_work)
return;
@@ -1956,16 +1956,10 @@ static void nicvf_set_rx_mode_task(struct work_struct *work_arg)
/* check if we have any specific MACs to be added to PF DMAC filter */
if (vf_work->mc) {
/* now go through kernel list of MACs and add them one by one */
- list_for_each_entry_safe(xaddr, next,
- &vf_work->mc->list, list) {
+ for (idx = 0; idx < vf_work->mc->count; idx++) {
mbx.xcast.msg = NIC_MBOX_MSG_ADD_MCAST;
- mbx.xcast.data.mac = xaddr->addr;
+ mbx.xcast.data.mac = vf_work->mc->mc[idx];
nicvf_send_msg_to_pf(nic, &mbx);
-
- /* after receiving ACK from PF release memory */
- list_del(&xaddr->list);
- kfree(xaddr);
- vf_work->mc->count--;
}
kfree(vf_work->mc);
}
@@ -1996,17 +1990,15 @@ static void nicvf_set_rx_mode(struct net_device *netdev)
mode |= BGX_XCAST_MCAST_FILTER;
/* here we need to copy mc addrs */
if (netdev_mc_count(netdev)) {
- struct xcast_addr *xaddr;
-
- mc_list = kmalloc(sizeof(*mc_list), GFP_ATOMIC);
- INIT_LIST_HEAD(&mc_list->list);
+ mc_list = kmalloc(offsetof(typeof(*mc_list),
+ mc[netdev_mc_count(netdev)]),
+ GFP_ATOMIC);
+ if (unlikely(!mc_list))
+ return;
+ mc_list->count = 0;
netdev_hw_addr_list_for_each(ha, &netdev->mc) {
- xaddr = kmalloc(sizeof(*xaddr),
- GFP_ATOMIC);
- xaddr->addr =
+ mc_list->mc[mc_list->count] =
ether_addr_to_u64(ha->addr);
- list_add_tail(&xaddr->list,
- &mc_list->list);
mc_list->count++;
}
}
--
2.14.3
^ permalink raw reply related
* Re: kernel BUG at drivers/vhost/vhost.c:LINE! (2)
From: Stefan Hajnoczi @ 2018-04-09 13:17 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: kvm, netdev, syzkaller-bugs, syzbot, linux-kernel,
Linux Virtualization
In-Reply-To: <20180409032835.GB1648@stefanha-x1.localdomain>
On Mon, Apr 9, 2018 at 11:28 AM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Mon, Apr 09, 2018 at 05:44:36AM +0300, Michael S. Tsirkin wrote:
>> On Mon, Apr 09, 2018 at 10:37:45AM +0800, Stefan Hajnoczi wrote:
>> > On Sat, Apr 7, 2018 at 3:02 AM, syzbot
>> > <syzbot+65a84dde0214b0387ccd@syzkaller.appspotmail.com> wrote:
>> > > syzbot hit the following crash on upstream commit
>> > > 38c23685b273cfb4ccf31a199feccce3bdcb5d83 (Fri Apr 6 04:29:35 2018 +0000)
>> > > Merge tag 'armsoc-drivers' of
>> > > git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc
>> > > syzbot dashboard link:
>> > > https://syzkaller.appspot.com/bug?extid=65a84dde0214b0387ccd
>> >
>> > To prevent duplicated work: I am working on this one.
>> >
>> > Stefan
>>
>> Do you want to try this patchset:
>> https://lkml.org/lkml/2018/4/5/665
>>
>> ?
>
> Thanks, I'll give it a shot.
>
> I also noticed a regression in commit
> d65026c6c62e7d9616c8ceb5a53b68bcdc050525 ("vhost: validate log when
> IOTLB is enabled") and am currently testing a fix.
I have sent a fix:
https://lkml.org/lkml/2018/4/9/390
Stefan
^ 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