* RE: [PATCH v2 0/3]
From: Rakesh Pillai @ 2020-08-01 5:10 UTC (permalink / raw)
To: 'Florian Fainelli', ath10k
Cc: linux-wireless, linux-kernel, kvalo, davem, kuba, netdev
In-Reply-To: <c6c5b3c5-f862-9cee-6863-24f666cc28f5@gmail.com>
> -----Original Message-----
> From: Florian Fainelli <f.fainelli@gmail.com>
> Sent: Saturday, August 1, 2020 12:17 AM
> To: Rakesh Pillai <pillair@codeaurora.org>; ath10k@lists.infradead.org
> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> kvalo@codeaurora.org; davem@davemloft.net; kuba@kernel.org;
> netdev@vger.kernel.org
> Subject: Re: [PATCH v2 0/3]
>
> On 7/31/20 11:27 AM, Rakesh Pillai wrote:
> > The history recording will be compiled only if
> > ATH10K_DEBUG is enabled, and also enabled via
> > the module parameter. Once the history recording
> > is enabled via module parameter, it can be enabled
> > or disabled runtime via debugfs.
>
> Why not use trace prints and retrieving them via the function tracer?
> This seems very ad-hoc.
Tracing needs to be enabled to capture the events.
But these events can be turned on in some kind of a debug build and capture the history to help us debug in case there is a crash.
It wont even allocate memory if not enabled via module parameter.
>
> >
> > ---
> > Changes from v1:
> > - Add module param and debugfs to enable/disable history recording.
> >
> > Rakesh Pillai (3):
> > ath10k: Add history for tracking certain events
> > ath10k: Add module param to enable history
> > ath10k: Add debugfs support to enable event history
> >
> > drivers/net/wireless/ath/ath10k/ce.c | 1 +
> > drivers/net/wireless/ath/ath10k/core.c | 3 +
> > drivers/net/wireless/ath/ath10k/core.h | 82 ++++++++++++
> > drivers/net/wireless/ath/ath10k/debug.c | 207
> ++++++++++++++++++++++++++++++
> > drivers/net/wireless/ath/ath10k/debug.h | 75 +++++++++++
> > drivers/net/wireless/ath/ath10k/snoc.c | 15 ++-
> > drivers/net/wireless/ath/ath10k/wmi-tlv.c | 1 +
> > drivers/net/wireless/ath/ath10k/wmi.c | 10 ++
> > 8 files changed, 393 insertions(+), 1 deletion(-)
> >
>
>
> --
> Florian
^ permalink raw reply
* RE: [PATCH v2 1/3] ath10k: Add history for tracking certain events
From: Rakesh Pillai @ 2020-08-01 5:13 UTC (permalink / raw)
To: 'Ben Greear', ath10k
Cc: linux-wireless, linux-kernel, kvalo, davem, kuba, netdev
In-Reply-To: <bedc5fe0-1904-d045-4a84-0869ee1b0b2e@candelatech.com>
> -----Original Message-----
> From: Ben Greear <greearb@candelatech.com>
> Sent: Saturday, August 1, 2020 12:08 AM
> To: Rakesh Pillai <pillair@codeaurora.org>; ath10k@lists.infradead.org
> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> kvalo@codeaurora.org; davem@davemloft.net; kuba@kernel.org;
> netdev@vger.kernel.org
> Subject: Re: [PATCH v2 1/3] ath10k: Add history for tracking certain events
>
> On 7/31/20 11:27 AM, Rakesh Pillai wrote:
> > Add history for tracking the below events
> > - register read
> > - register write
> > - IRQ trigger
> > - NAPI poll
> > - CE service
> > - WMI cmd
> > - WMI event
> > - WMI tx completion
> >
> > This will help in debugging any crash or any
> > improper behaviour.
> >
> > Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
> >
> > Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
> > ---
> > drivers/net/wireless/ath/ath10k/ce.c | 1 +
> > drivers/net/wireless/ath/ath10k/core.h | 74 +++++++++++++++++
> > drivers/net/wireless/ath/ath10k/debug.c | 133
> ++++++++++++++++++++++++++++++
> > drivers/net/wireless/ath/ath10k/debug.h | 74 +++++++++++++++++
> > drivers/net/wireless/ath/ath10k/snoc.c | 15 +++-
> > drivers/net/wireless/ath/ath10k/wmi-tlv.c | 1 +
> > drivers/net/wireless/ath/ath10k/wmi.c | 10 +++
> > 7 files changed, 307 insertions(+), 1 deletion(-)
> >
>
> > +void ath10k_record_wmi_event(struct ath10k *ar, enum
> ath10k_wmi_type type,
> > + u32 id, unsigned char *data)
> > +{
> > + struct ath10k_wmi_event_entry *entry;
> > + u32 idx;
> > +
> > + if (type == ATH10K_WMI_EVENT) {
> > + if (!ar->wmi_event_history.record)
> > + return;
>
> This check above is duplicated below, add it once at top of the method
> instead.
The same function is used to record WMI events and CMD, which are stored in different memory locations.
Hence the check " if (type == ATH10K_WMI_EVENT) {" is necessary.
>
> > +
> > + spin_lock_bh(&ar->wmi_event_history.hist_lock);
> > + idx = ath10k_core_get_next_idx(&ar-
> >reg_access_history.index,
> > + ar-
> >wmi_event_history.max_entries);
> > + spin_unlock_bh(&ar->wmi_event_history.hist_lock);
> > + entry = &ar->wmi_event_history.record[idx];
> > + } else {
> > + if (!ar->wmi_cmd_history.record)
> > + return;
> > +
> > + spin_lock_bh(&ar->wmi_cmd_history.hist_lock);
> > + idx = ath10k_core_get_next_idx(&ar-
> >reg_access_history.index,
> > + ar-
> >wmi_cmd_history.max_entries);
> > + spin_unlock_bh(&ar->wmi_cmd_history.hist_lock);
> > + entry = &ar->wmi_cmd_history.record[idx];
> > + }
> > +
> > + entry->timestamp = ath10k_core_get_timestamp();
> > + entry->cpu_id = smp_processor_id();
> > + entry->type = type;
> > + entry->id = id;
> > + memcpy(&entry->data, data + 4, ATH10K_WMI_DATA_LEN);
> > +}
> > +EXPORT_SYMBOL(ath10k_record_wmi_event);
>
> > @@ -1660,6 +1668,11 @@ static int ath10k_snoc_probe(struct
> platform_device *pdev)
> > ar->ce_priv = &ar_snoc->ce;
> > msa_size = drv_data->msa_size;
> >
> > + ath10k_core_reg_access_history_init(ar,
> ATH10K_REG_ACCESS_HISTORY_MAX);
> > + ath10k_core_wmi_event_history_init(ar,
> ATH10K_WMI_EVENT_HISTORY_MAX);
> > + ath10k_core_wmi_cmd_history_init(ar,
> ATH10K_WMI_CMD_HISTORY_MAX);
> > + ath10k_core_ce_event_history_init(ar,
> ATH10K_CE_EVENT_HISTORY_MAX);
>
> Maybe only enable this once user turns it on? It sucks up a bit of memory?
This memory will be allocated only if the history is enabled via module param, else the function just returns 0.
>
> > +
> > ath10k_snoc_quirks_init(ar);
> >
> > ret = ath10k_snoc_resource_init(ar);
> > diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> > index 932266d..9df5748 100644
> > --- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> > +++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> > @@ -627,6 +627,7 @@ static void ath10k_wmi_tlv_op_rx(struct ath10k *ar,
> struct sk_buff *skb)
> > if (skb_pull(skb, sizeof(struct wmi_cmd_hdr)) == NULL)
> > goto out;
> >
> > + ath10k_record_wmi_event(ar, ATH10K_WMI_EVENT, id, skb->data);
> > trace_ath10k_wmi_event(ar, id, skb->data, skb->len);
> >
> > consumed = ath10k_tm_event_wmi(ar, id, skb);
> > diff --git a/drivers/net/wireless/ath/ath10k/wmi.c
> b/drivers/net/wireless/ath/ath10k/wmi.c
> > index a81a1ab..8ebd05c 100644
> > --- a/drivers/net/wireless/ath/ath10k/wmi.c
> > +++ b/drivers/net/wireless/ath/ath10k/wmi.c
> > @@ -1802,6 +1802,15 @@ struct sk_buff *ath10k_wmi_alloc_skb(struct
> ath10k *ar, u32 len)
> >
> > static void ath10k_wmi_htc_tx_complete(struct ath10k *ar, struct sk_buff
> *skb)
> > {
> > + struct wmi_cmd_hdr *cmd_hdr;
> > + enum wmi_tlv_event_id id;
> > +
> > + cmd_hdr = (struct wmi_cmd_hdr *)skb->data;
> > + id = MS(__le32_to_cpu(cmd_hdr->cmd_id),
> WMI_CMD_HDR_CMD_ID);
> > +
> > + ath10k_record_wmi_event(ar, ATH10K_WMI_TX_COMPL, id,
> > + skb->data + sizeof(struct wmi_cmd_hdr));
> > +
> > dev_kfree_skb(skb);
> > }
>
> I think guard the above new code with if (unlikely(ar-
> >ce_event_history.record)) { ... }
>
> All in all, I think I'd want to compile this out (while leaving other debug
> compiled
> in) since it seems this stuff would be rarely used and it adds method calls to
> hot
> paths.
>
> That is a decision for Kalle though, so see what he says...
Sure let me add this check.
>
> Thanks,
> Ben
>
>
> --
> Ben Greear <greearb@candelatech.com>
> Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH v2 0/3]
From: Florian Fainelli @ 2020-08-01 5:24 UTC (permalink / raw)
To: Rakesh Pillai, ath10k
Cc: linux-wireless, linux-kernel, kvalo, davem, kuba, netdev
In-Reply-To: <000701d667c2$0782fe70$1688fb50$@codeaurora.org>
On 7/31/2020 10:10 PM, Rakesh Pillai wrote:
>
>
>> -----Original Message-----
>> From: Florian Fainelli <f.fainelli@gmail.com>
>> Sent: Saturday, August 1, 2020 12:17 AM
>> To: Rakesh Pillai <pillair@codeaurora.org>; ath10k@lists.infradead.org
>> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
>> kvalo@codeaurora.org; davem@davemloft.net; kuba@kernel.org;
>> netdev@vger.kernel.org
>> Subject: Re: [PATCH v2 0/3]
>>
>> On 7/31/20 11:27 AM, Rakesh Pillai wrote:
>>> The history recording will be compiled only if
>>> ATH10K_DEBUG is enabled, and also enabled via
>>> the module parameter. Once the history recording
>>> is enabled via module parameter, it can be enabled
>>> or disabled runtime via debugfs.
>>
>> Why not use trace prints and retrieving them via the function tracer?
>> This seems very ad-hoc.
>
> Tracing needs to be enabled to capture the events.
> But these events can be turned on in some kind of a debug build and capture the history to help us debug in case there is a crash.
> It wont even allocate memory if not enabled via module parameter.
I would suggest researching what other drivers do and also considering
the benefits, for someone doing system analysis of plugging into the
kernel's general tracing mechanism to have all information in the same
place and just do filtering on the record/report side.
--
Florian
^ permalink raw reply
* Re: [Linux-kernel-mentees] [PATCH net] rds: Prevent kernel-infoleak in rds_notify_queue_get()
From: Leon Romanovsky @ 2020-08-01 5:38 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jason Gunthorpe, Peilin Ye, Santosh Shilimkar, David S. Miller,
Jakub Kicinski, Dan Carpenter, Arnd Bergmann,
linux-kernel-mentees, netdev, linux-rdma, rds-devel, linux-kernel
In-Reply-To: <20200731171924.GA2014207@kroah.com>
On Fri, Jul 31, 2020 at 07:19:24PM +0200, Greg Kroah-Hartman wrote:
> On Fri, Jul 31, 2020 at 11:36:04AM -0300, Jason Gunthorpe wrote:
> > On Fri, Jul 31, 2020 at 04:21:48PM +0200, Greg Kroah-Hartman wrote:
> >
> > > > The spec was updated in C11 to require zero'ing padding when doing
> > > > partial initialization of aggregates (eg = {})
> > > >
> > > > """if it is an aggregate, every member is initialized (recursively)
> > > > according to these rules, and any padding is initialized to zero
> > > > bits;"""
> > >
> > > But then why does the compilers not do this?
> >
> > Do you have an example?
>
> At the moment, no, but we have had them in the past due to security
> issues we have had to fix for this.
Is it still relevant after bump of required GCC version to build kernel?
I afraid that without solid example such changes will start to be
treated with cargo cult.
Jason,
I'm using {} instead of {0} because of this GCC bug.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119
Thanks
^ permalink raw reply
* Re: [PATCH net v2] net/sched: act_ct: fix miss set mru for ovs after defrag in act_ct
From: Cong Wang @ 2020-08-01 6:06 UTC (permalink / raw)
To: wenxu; +Cc: Linux Kernel Network Developers
In-Reply-To: <1596163501-7113-1-git-send-email-wenxu@ucloud.cn>
On Thu, Jul 30, 2020 at 7:45 PM <wenxu@ucloud.cn> wrote:
>
> From: wenxu <wenxu@ucloud.cn>
>
> When openvswitch conntrack offload with act_ct action. Fragment packets
> defrag in the ingress tc act_ct action and miss the next chain. Then the
> packet pass to the openvswitch datapath without the mru. The over
> mtu packet will be dropped in output action in openvswitch for over mtu.
>
> "kernel: net2: dropped over-mtu packet: 1528 > 1500"
>
> This patch add mru in the tc_skb_ext for adefrag and miss next chain
> situation. And also add mru in the qdisc_skb_cb. The act_ct set the mru
> to the qdisc_skb_cb when the packet defrag. And When the chain miss,
> The mru is set to tc_skb_ext which can be got by ovs datapath.
>
> Fixes: b57dc7c13ea9 ("net/sched: Introduce action ct")
> Signed-off-by: wenxu <wenxu@ucloud.cn>
Reviewed-by: Cong Wang <xiyou.wangcong@gmail.com>
Thanks.
^ permalink raw reply
* [PATCH net] vxlan: fix memleak of fdb
From: Taehee Yoo @ 2020-08-01 7:07 UTC (permalink / raw)
To: davem, kuba, netdev; +Cc: roopa, ap420073
When vxlan interface is deleted, all fdbs are deleted by vxlan_flush().
vxlan_flush() flushes fdbs but it doesn't delete fdb, which contains
all-zeros-mac because it is deleted by vxlan_uninit().
But vxlan_uninit() deletes only the fdb, which contains both all-zeros-mac
and default vni.
So, the fdb, which contains both all-zeros-mac and non-default vni
will not be deleted.
Test commands:
ip link add vxlan0 type vxlan dstport 4789 external
ip link set vxlan0 up
bridge fdb add to 00:00:00:00:00:00 dst 172.0.0.1 dev vxlan0 via lo \
src_vni 10000 self permanent
ip link del vxlan0
kmemleak reports as follows:
unreferenced object 0xffff9486b25ced88 (size 96):
comm "bridge", pid 2151, jiffies 4294701712 (age 35506.901s)
hex dump (first 32 bytes):
02 00 00 00 ac 00 00 01 40 00 09 b1 86 94 ff ff ........@.......
46 02 00 00 00 00 00 00 a7 03 00 00 12 b5 6a 6b F.............jk
backtrace:
[<00000000c10cf651>] vxlan_fdb_append.part.51+0x3c/0xf0 [vxlan]
[<000000006b31a8d9>] vxlan_fdb_create+0x184/0x1a0 [vxlan]
[<0000000049399045>] vxlan_fdb_update+0x12f/0x220 [vxlan]
[<0000000090b1ef00>] vxlan_fdb_add+0x12a/0x1b0 [vxlan]
[<0000000056633c2c>] rtnl_fdb_add+0x187/0x270
[<00000000dd5dfb6b>] rtnetlink_rcv_msg+0x264/0x490
[<00000000fc44dd54>] netlink_rcv_skb+0x4a/0x110
[<00000000dff433e7>] netlink_unicast+0x18e/0x250
[<00000000b87fb421>] netlink_sendmsg+0x2e9/0x400
[<000000002ed55153>] ____sys_sendmsg+0x237/0x260
[<00000000faa51c66>] ___sys_sendmsg+0x88/0xd0
[<000000006c3982f1>] __sys_sendmsg+0x4e/0x80
[<00000000a8f875d2>] do_syscall_64+0x56/0xe0
[<000000003610eefa>] entry_SYSCALL_64_after_hwframe+0x44/0xa9
unreferenced object 0xffff9486b1c40080 (size 128):
comm "bridge", pid 2157, jiffies 4294701754 (age 35506.866s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 f8 dc 42 b2 86 94 ff ff ..........B.....
6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
backtrace:
[<00000000a2981b60>] vxlan_fdb_create+0x67/0x1a0 [vxlan]
[<0000000049399045>] vxlan_fdb_update+0x12f/0x220 [vxlan]
[<0000000090b1ef00>] vxlan_fdb_add+0x12a/0x1b0 [vxlan]
[<0000000056633c2c>] rtnl_fdb_add+0x187/0x270
[<00000000dd5dfb6b>] rtnetlink_rcv_msg+0x264/0x490
[<00000000fc44dd54>] netlink_rcv_skb+0x4a/0x110
[<00000000dff433e7>] netlink_unicast+0x18e/0x250
[<00000000b87fb421>] netlink_sendmsg+0x2e9/0x400
[<000000002ed55153>] ____sys_sendmsg+0x237/0x260
[<00000000faa51c66>] ___sys_sendmsg+0x88/0xd0
[<000000006c3982f1>] __sys_sendmsg+0x4e/0x80
[<00000000a8f875d2>] do_syscall_64+0x56/0xe0
[<000000003610eefa>] entry_SYSCALL_64_after_hwframe+0x44/0xa9
Fixes: 3ad7a4b141eb ("vxlan: support fdb and learning in COLLECT_METADATA mode")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
drivers/net/vxlan.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 5efe1e28f270..a7c3939264b0 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -3076,8 +3076,10 @@ static void vxlan_flush(struct vxlan_dev *vxlan, bool do_all)
if (!do_all && (f->state & (NUD_PERMANENT | NUD_NOARP)))
continue;
/* the all_zeros_mac entry is deleted at vxlan_uninit */
- if (!is_zero_ether_addr(f->eth_addr))
- vxlan_fdb_destroy(vxlan, f, true, true);
+ if (is_zero_ether_addr(f->eth_addr) &&
+ f->vni == vxlan->cfg.vni)
+ continue;
+ vxlan_fdb_destroy(vxlan, f, true, true);
}
spin_unlock_bh(&vxlan->hash_lock[h]);
}
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v6 bpf-next 0/6] bpf: tailcalls in BPF subprograms
From: Maciej Fijalkowski @ 2020-08-01 7:13 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: ast, bpf, netdev, bjorn.topel, magnus.karlsson
In-Reply-To: <fbe6e5ca-65ba-7698-3b8d-1214b5881e88@iogearbox.net>
On Sat, Aug 01, 2020 at 03:03:19AM +0200, Daniel Borkmann wrote:
> On 7/31/20 2:03 AM, Maciej Fijalkowski wrote:
> > v5->v6:
> > - propagate only those poke descriptors that individual subprogram is
> > actually using (Daniel)
> > - drop the cumbersome check if poke desc got filled in map_poke_run()
> > - move poke->ip renaming in bpf_jit_add_poke_descriptor() from patch 4
> > to patch 3 to provide bisectability (Daniel)
>
> I did a basic test with Cilium on K8s with this set, spawning a few Pods
> and checking connectivity & whether we're not crashing since it has bit more
> elaborate tail call use. So far so good. I was inclined to push the series
> out, but there is one more issue I noticed and didn't notice earlier when
> reviewing, and that is overall stack size:
>
> What happens when you create a single program that has nested BPF to BPF
> calls e.g. either up to the maximum nesting or one call that is using up
> the max stack size which is then doing another BPF to BPF call that contains
> the tail call. In the tail call map, you have the same program in there.
> This means we create a worst case stack from BPF size of max_stack_size *
> max_tail_call_size, that is, 512*32. So that adds 16k worst case. For x86
> we have a stack of arch/x86/include/asm/page_64_types.h:
>
> #define THREAD_SIZE_ORDER (2 + KASAN_STACK_ORDER)
> #define THREAD_SIZE (PAGE_SIZE << THREAD_SIZE_ORDER)
>
> So we end up with 16k in a typical case. And this will cause kernel stack
> overflow; I'm at least not seeing where we handle this situation in the
> set. Hm, need to think more, but maybe this needs tracking of max stack
> across tail calls to force an upper limit..
My knee jerk reaction would be to decrement the allowed max tail calls,
but not sure if it's an option and if it would help.
Otherwise I'm not sure how to pass around the stack size just like we're
doing it in this set with tail call counter that sits in %rax.
>
> Thanks,
> Daniel
^ permalink raw reply
* Re: [PATCH net-next] tcp: fix build fong CONFIG_MPTCP=n
From: Florian Westphal @ 2020-08-01 7:46 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S . Miller, netdev, Eric Dumazet, Florian Westphal
In-Reply-To: <20200801020929.3000802-1-edumazet@google.com>
Eric Dumazet <edumazet@google.com> wrote:
> Fixes these errors:
>
> net/ipv4/syncookies.c: In function 'tcp_get_cookie_sock':
> net/ipv4/syncookies.c:216:19: error: 'struct tcp_request_sock' has no
> member named 'drop_req'
> 216 | if (tcp_rsk(req)->drop_req) {
> | ^~
> net/ipv4/syncookies.c: In function 'cookie_tcp_reqsk_alloc':
> net/ipv4/syncookies.c:289:27: warning: unused variable 'treq'
> [-Wunused-variable]
> 289 | struct tcp_request_sock *treq;
> | ^~~~
Ugh, sorry about this.
> make[3]: *** [scripts/Makefile.build:280: net/ipv4/syncookies.o] Error 1
> make[3]: *** Waiting for unfinished jobs....
>
> Fixes: 9466a1ccebbe ("mptcp: enable JOIN requests even if cookies are in use")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Florian Westphal <fw@strlen.de>
> ---
> net/ipv4/syncookies.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
> index 11b20474be8310d7070750a1c7b4013f2fba2f55..f0794f0232bae749244fff35d8b96b1f561a5e87 100644
> --- a/net/ipv4/syncookies.c
> +++ b/net/ipv4/syncookies.c
> @@ -213,7 +213,7 @@ struct sock *tcp_get_cookie_sock(struct sock *sk, struct sk_buff *skb,
> tcp_sk(child)->tsoffset = tsoff;
> sock_rps_save_rxhash(child, skb);
>
> - if (tcp_rsk(req)->drop_req) {
> + if (rsk_drop_req(req)) {
This hunk breaks join self test for mptcp, but it should not. Looks like
cookie path missed a ->is_mptcp = 1 somewhere. Investigating.
^ permalink raw reply
* Re: [Linux-kernel-mentees] [PATCH net] rds: Prevent kernel-infoleak in rds_notify_queue_get()
From: Dan Carpenter @ 2020-08-01 8:00 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Greg Kroah-Hartman, Leon Romanovsky, Peilin Ye, Santosh Shilimkar,
David S. Miller, Jakub Kicinski, Arnd Bergmann,
linux-kernel-mentees, netdev, linux-rdma, rds-devel, linux-kernel
In-Reply-To: <20200731182712.GI24045@ziepe.ca>
On Fri, Jul 31, 2020 at 03:27:12PM -0300, Jason Gunthorpe wrote:
> On Fri, Jul 31, 2020 at 07:19:24PM +0200, Greg Kroah-Hartman wrote:
>
> > > I tried for a bit and didn't find a way to get even old gcc 4.4 to not
> > > initialize the holes.
> >
> > Odd, so it is just the "= {0};" that does not zero out the holes?
>
> Nope, it seems to work fine too. I tried a number of situations and I
> could not get the compiler to not zero holes, even back to gcc 4.4
>
> It is not just accidental either, take this:
>
> struct rds_rdma_notify {
> unsigned long user_token;
> unsigned char status;
> unsigned long user_token1 __attribute__((aligned(32)));
> } foo = {0};
>
> Which has quite a big hole, clang generates:
>
> movq $0, 56(%rdi)
> movq $0, 48(%rdi)
> movq $0, 40(%rdi)
> movq $0, 32(%rdi)
> movq $0, 24(%rdi)
> movq $0, 16(%rdi)
> movq $0, 8(%rdi)
> movq $0, (%rdi)
>
> Deliberate extra instructions to fill both holes. gcc 10 does the
> same, older gcc's do create a rep stosq over the whole thing.
>
> Some fiddling with godbolt shows quite a variety of output, but I
> didn't see anything that looks like a compiler not filling
> padding. Even godbolt's gcc 4.1 filled the padding, which is super old.
>
> In several cases it seems the aggregate initializer produced better
> code than memset, in other cases it didn't
>
> Without an actual example where this doesn't work right it is hard to
> say anything more..
Here is the example that set off the recent patches:
https://lkml.org/lkml/2020/7/27/199
Another example is commit 5ff223e86f5a ("net: Zeroing the structure
ethtool_wolinfo in ethtool_get_wol()"). I tested this one with GCC 7.4
at the time and it was a real life bug.
The rest of these patches were based on static analysis from Smatch.
They're all "theoretical" bugs based on the C standard but it's
impossible to know if and when they'll turn into real life bugs.
It's not a super long list of code that's affected because we've known
that the bug was possible for a few years. It was only last year when
I saw that it had become a real life bug.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH 1/2] net: phy: Add fwnode helper functions
From: kernel test robot @ 2020-08-01 8:10 UTC (permalink / raw)
To: Vikas Singh, andrew, f.fainelli, hkallweit1, linux, netdev
Cc: kbuild-all, calvin.johnson, kuldip.dwivedi, madalin.bucur,
vikas.singh, Vikas Singh
In-Reply-To: <1595938400-13279-2-git-send-email-vikas.singh@puresoftware.com>
[-- Attachment #1: Type: text/plain, Size: 2806 bytes --]
Hi Vikas,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.8-rc7 next-20200731]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Vikas-Singh/Add-fwnode-helper-functions-to-MDIO-bus-driver/20200728-201610
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 92ed301919932f777713b9172e525674157e983d
config: h8300-randconfig-s031-20200731 (attached as .config)
compiler: h8300-linux-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.2-115-g5fc204f2-dirty
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=h8300
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
h8300-linux-ld: drivers/net/phy/mdio_bus.o: in function `fwnode_phy_connect':
>> drivers/net/phy/mdio_bus.c:102: undefined reference to `phy_connect_direct'
vim +102 drivers/net/phy/mdio_bus.c
76
77 /**
78 * fwnode_phy_connect - Connect to the phy described in the device tree
79 * @dev: pointer to net_device claiming the phy
80 * @phy_fwnode: Pointer to fwnode for the PHY
81 * @hndlr: Link state callback for the network device
82 * @flags: flags to pass to the PHY
83 * @iface: PHY data interface type
84 *
85 * If successful, returns a pointer to the phy_device with the embedded
86 * struct device refcount incremented by one, or NULL on failure. The
87 * refcount must be dropped by calling phy_disconnect() or phy_detach().
88 */
89 struct phy_device *fwnode_phy_connect(
90 struct net_device *dev, struct fwnode_handle *phy_fwnode,
91 void (*hndlr)(struct net_device *), u32 flags, u32 iface)
92 {
93 struct phy_device *phy_dev;
94
95 phy_dev = fwnode_phy_find_device(phy_fwnode);
96 if (!phy_dev)
97 return NULL;
98
99 phy_dev->dev_flags = flags;
100
101 /* If in case we fail to attach to PHY,then phy_dev must be NULL */
> 102 if (phy_connect_direct(dev, phy_dev, hndlr, iface))
103 return NULL;
104
105 return phy_dev;
106 }
107 EXPORT_SYMBOL(fwnode_phy_connect);
108
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25209 bytes --]
^ permalink raw reply
* Re: [PATCH v4 net-next 04/10] qed: implement devlink info request
From: Igor Russkikh @ 2020-08-01 8:22 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, David S . Miller, Ariel Elior, Michal Kalderon,
Denis Bolotin, Jiri Pirko, Alexander Lobakin
In-Reply-To: <20200731130402.2288f44a@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
>> Thus, here we create and store a private copy of this structure
>> in qed_dev root object.
>
> Please include example output of devlink info on you device.
Hi Jakub, will do. Here is an example:
~$ sudo ~/iproute2/devlink/devlink dev info
pci/0000:01:00.0:
driver qed
serial_number REE1915E44552
versions:
running:
fw 8.42.2.0
stored:
fw.mgmt 8.52.10.0
pci/0000:01:00.1:
driver qed
serial_number REE1915E44552
versions:
running:
fw 8.42.2.0
stored:
fw.mgmt 8.52.10.0
>> + memcpy(buf, cdev->hwfns[0].hw_info.part_num,
> sizeof(cdev->hwfns[0].hw_info.part_num));
>> + buf[sizeof(cdev->hwfns[0].hw_info.part_num)] = 0;
>
> Part number != serial number. What's the thing you're reporting here
> actually identifying.
From user manual and configuration point of view thats a serial number.
Existing internal structures name that as part number, double checked
the documentation - in this hardware manual these two things are the same.
> DEVLINK_INFO_VERSION_GENERIC_FW_MGMT, buf);
>> + if (err)
>> + return err;
>
> Assuming MFW means management FW - this looks good.
>> + return devlink_info_version_running_put(req,
Right,
> DEVLINK_INFO_VERSION_GENERIC_FW, buf);
>
> But what's this one?
This one is a fast path firmware which is being loaded from driver
dynamically. I can put this explanation to the patch description.
Regards
Igor
^ permalink raw reply
* [PATCH] net: core: explicitly call linkwatch_fire_event to speed up the startup of network services
From: Wen Yang @ 2020-08-01 8:58 UTC (permalink / raw)
To: davem, Jakub Kicinski
Cc: Xunlei Pang, Caspar Zhang, Wen Yang, Andrew Lunn, Eric Dumazet,
Jiri Pirko, Leon Romanovsky, Julian Wiedmann, netdev,
linux-kernel
The linkwatch_event work queue runs up to one second later.
When the MicroVM starts, it takes 300+ms for the ethX flag
to change from '+UP +LOWER_UP' to '+RUNNING', as follows:
Jul 20 22:00:47.432552 systemd-networkd[210]: eth0: bringing link up
...
Jul 20 22:00:47.446108 systemd-networkd[210]: eth0: flags change: +UP +LOWER_UP
...
Jul 20 22:00:47.781463 systemd-networkd[210]: eth0: flags change: +RUNNING
Let's manually trigger it here to make the network service start faster.
After applying this patch, the time consumption of
systemd-networkd.service was reduced from 366ms to 50ms.
Signed-off-by: Wen Yang <wenyang@linux.alibaba.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jiri Pirko <jiri@mellanox.com>
Cc: Leon Romanovsky <leon@kernel.org>
Cc: Julian Wiedmann <jwi@linux.ibm.com>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
net/core/link_watch.c | 3 +++
net/core/rtnetlink.c | 1 +
2 files changed, 4 insertions(+)
diff --git a/net/core/link_watch.c b/net/core/link_watch.c
index 75431ca..6b9d44b 100644
--- a/net/core/link_watch.c
+++ b/net/core/link_watch.c
@@ -98,6 +98,9 @@ static bool linkwatch_urgent_event(struct net_device *dev)
if (netif_is_lag_port(dev) || netif_is_lag_master(dev))
return true;
+ if ((dev->flags & IFF_UP) && dev->operstate == IF_OPER_DOWN)
+ return true;
+
return netif_carrier_ok(dev) && qdisc_tx_changing(dev);
}
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 58c484a..fd0b3b6 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2604,6 +2604,7 @@ static int do_setlink(const struct sk_buff *skb,
extack);
if (err < 0)
goto errout;
+ linkwatch_fire_event(dev);
}
if (tb[IFLA_MASTER]) {
--
1.8.3.1
^ permalink raw reply related
* [PATCH] mac80211: use eth_zero_addr() to clear mac address
From: linmiaohe @ 2020-08-01 9:12 UTC (permalink / raw)
To: johannes, davem, kuba; +Cc: linux-wireless, netdev, linux-kernel, linmiaohe
From: Miaohe Lin <linmiaohe@huawei.com>
Use eth_zero_addr() to clear mac address instead of memset().
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
net/mac80211/trace.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h
index 1b4709694d2a..50ab5b9d8eab 100644
--- a/net/mac80211/trace.h
+++ b/net/mac80211/trace.h
@@ -22,7 +22,8 @@
#define LOCAL_PR_ARG __entry->wiphy_name
#define STA_ENTRY __array(char, sta_addr, ETH_ALEN)
-#define STA_ASSIGN (sta ? memcpy(__entry->sta_addr, sta->addr, ETH_ALEN) : memset(__entry->sta_addr, 0, ETH_ALEN))
+#define STA_ASSIGN (sta ? memcpy(__entry->sta_addr, sta->addr, ETH_ALEN) : \
+ eth_zero_addr(__entry->sta_addr))
#define STA_NAMED_ASSIGN(s) memcpy(__entry->sta_addr, (s)->addr, ETH_ALEN)
#define STA_PR_FMT " sta:%pM"
#define STA_PR_ARG __entry->sta_addr
--
2.19.1
^ permalink raw reply related
* [PATCH] net: qede: use eth_zero_addr() to clear mac address
From: linmiaohe @ 2020-08-01 9:13 UTC (permalink / raw)
To: aelior, GR-everest-linux-l2, davem, kuba; +Cc: netdev, linux-kernel, linmiaohe
From: Miaohe Lin <linmiaohe@huawei.com>
Use eth_zero_addr() to clear mac address instead of memset().
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
drivers/net/ethernet/qlogic/qede/qede_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
index 29e285430f99..e1617a67a714 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -2678,8 +2678,8 @@ static void qede_get_generic_tlv_data(void *dev, struct qed_generic_tlvs *data)
data->feat_flags |= QED_TLV_LSO;
ether_addr_copy(data->mac[0], edev->ndev->dev_addr);
- memset(data->mac[1], 0, ETH_ALEN);
- memset(data->mac[2], 0, ETH_ALEN);
+ eth_zero_addr(data->mac[1]);
+ eth_zero_addr(data->mac[2]);
/* Copy the first two UC macs */
netif_addr_lock_bh(edev->ndev);
i = 1;
--
2.19.1
^ permalink raw reply related
* [PATCH] net: qed: use eth_zero_addr() to clear mac address
From: linmiaohe @ 2020-08-01 9:14 UTC (permalink / raw)
To: aelior, GR-everest-linux-l2, davem, kuba; +Cc: netdev, linux-kernel, linmiaohe
From: Miaohe Lin <linmiaohe@huawei.com>
Use eth_zero_addr() to clear mac address instead of memset().
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
drivers/net/ethernet/qlogic/qed/qed_sriov.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_sriov.c b/drivers/net/ethernet/qlogic/qed/qed_sriov.c
index 20679fd4204b..5015890a2a59 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_sriov.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_sriov.c
@@ -5067,8 +5067,7 @@ static void qed_update_mac_for_vf_trust_change(struct qed_hwfn *hwfn, int vf_id)
for (i = 0; i < QED_ETH_VF_NUM_MAC_FILTERS; i++) {
if (ether_addr_equal(vf->shadow_config.macs[i],
vf_info->mac)) {
- memset(vf->shadow_config.macs[i], 0,
- ETH_ALEN);
+ eth_zero_addr(vf->shadow_config.macs[i]);
DP_VERBOSE(hwfn, QED_MSG_IOV,
"Shadow MAC %pM removed for VF 0x%02x, VF trust mode is ON\n",
vf_info->mac, vf_id);
@@ -5077,7 +5076,7 @@ static void qed_update_mac_for_vf_trust_change(struct qed_hwfn *hwfn, int vf_id)
}
ether_addr_copy(vf_info->mac, force_mac);
- memset(vf_info->forced_mac, 0, ETH_ALEN);
+ eth_zero_addr(vf_info->forced_mac);
vf->bulletin.p_virt->valid_bitmap &=
~BIT(MAC_ADDR_FORCED);
qed_schedule_iov(hwfn, QED_IOV_WQ_BULLETIN_UPDATE_FLAG);
@@ -5088,7 +5087,7 @@ static void qed_update_mac_for_vf_trust_change(struct qed_hwfn *hwfn, int vf_id)
if (!vf_info->is_trusted_configured) {
u8 empty_mac[ETH_ALEN];
- memset(empty_mac, 0, ETH_ALEN);
+ eth_zero_addr(empty_mac);
for (i = 0; i < QED_ETH_VF_NUM_MAC_FILTERS; i++) {
if (ether_addr_equal(vf->shadow_config.macs[i],
empty_mac)) {
--
2.19.1
^ permalink raw reply related
* [PATCH] nl80211: use eth_zero_addr() to clear mac address
From: linmiaohe @ 2020-08-01 9:15 UTC (permalink / raw)
To: johannes, davem, kuba; +Cc: linux-wireless, netdev, linux-kernel, linmiaohe
From: Miaohe Lin <linmiaohe@huawei.com>
Use eth_zero_addr() to clear mac address instead of memset().
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
net/wireless/nl80211.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 7fbca0854265..c88f9d09cb25 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -10394,8 +10394,7 @@ static int nl80211_connect(struct sk_buff *skb, struct genl_info *info)
memcpy(dev->ieee80211_ptr->disconnect_bssid,
connect.bssid, ETH_ALEN);
else
- memset(dev->ieee80211_ptr->disconnect_bssid,
- 0, ETH_ALEN);
+ eth_zero_addr(dev->ieee80211_ptr->disconnect_bssid);
}
wdev_unlock(dev->ieee80211_ptr);
--
2.19.1
^ permalink raw reply related
* [PATCH] net: Use __skb_pagelen() directly in skb_cow_data()
From: linmiaohe @ 2020-08-01 9:30 UTC (permalink / raw)
To: davem, kuba, fw, pshelar, martin.varghese, pabeni, edumazet,
dcaratti, steffen.klassert, shmulik, kyk.segfault
Cc: netdev, linux-kernel, linmiaohe
From: Miaohe Lin <linmiaohe@huawei.com>
In fact, skb_pagelen() - skb_headlen() is equal to __skb_pagelen(), use it
directly to avoid unnecessary skb_headlen() call.
Also fix the CHECK note of checkpatch.pl:
Comparison to NULL could be written "!__pskb_pull_tail"
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
net/core/skbuff.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index b8afefe6f6b6..3219c26ddfae 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -4413,7 +4413,7 @@ int skb_cow_data(struct sk_buff *skb, int tailbits, struct sk_buff **trailer)
* at the moment even if they are anonymous).
*/
if ((skb_cloned(skb) || skb_shinfo(skb)->nr_frags) &&
- __pskb_pull_tail(skb, skb_pagelen(skb)-skb_headlen(skb)) == NULL)
+ !__pskb_pull_tail(skb, __skb_pagelen(skb)))
return -ENOMEM;
/* Easy case. Most of packets will go this way. */
--
2.19.1
^ permalink raw reply related
* Re: [PATCH net-next] tcp: fix build fong CONFIG_MPTCP=n
From: Florian Westphal @ 2020-08-01 9:29 UTC (permalink / raw)
To: Florian Westphal; +Cc: Eric Dumazet, David S . Miller, netdev, Eric Dumazet
In-Reply-To: <20200801074645.GG5271@breakpoint.cc>
Florian Westphal <fw@strlen.de> wrote:
> Eric Dumazet <edumazet@google.com> wrote:
> > Fixes these errors:
> >
> > net/ipv4/syncookies.c: In function 'tcp_get_cookie_sock':
> > net/ipv4/syncookies.c:216:19: error: 'struct tcp_request_sock' has no
> > member named 'drop_req'
> > 216 | if (tcp_rsk(req)->drop_req) {
> > | ^~
> > net/ipv4/syncookies.c: In function 'cookie_tcp_reqsk_alloc':
> > net/ipv4/syncookies.c:289:27: warning: unused variable 'treq'
> > [-Wunused-variable]
> > 289 | struct tcp_request_sock *treq;
> > | ^~~~
>
> Ugh, sorry about this.
>
> > make[3]: *** [scripts/Makefile.build:280: net/ipv4/syncookies.o] Error 1
> > make[3]: *** Waiting for unfinished jobs....
> >
> > Fixes: 9466a1ccebbe ("mptcp: enable JOIN requests even if cookies are in use")
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Cc: Florian Westphal <fw@strlen.de>
> > ---
> > net/ipv4/syncookies.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
> > index 11b20474be8310d7070750a1c7b4013f2fba2f55..f0794f0232bae749244fff35d8b96b1f561a5e87 100644
> > --- a/net/ipv4/syncookies.c
> > +++ b/net/ipv4/syncookies.c
> > @@ -213,7 +213,7 @@ struct sock *tcp_get_cookie_sock(struct sock *sk, struct sk_buff *skb,
> > tcp_sk(child)->tsoffset = tsoff;
> > sock_rps_save_rxhash(child, skb);
> >
> > - if (tcp_rsk(req)->drop_req) {
> > + if (rsk_drop_req(req)) {
>
> This hunk breaks join self test for mptcp, but it should not. Looks like
> cookie path missed a ->is_mptcp = 1 somewhere. Investigating.
I take that back, the failure is spurious, also sometimes occurs with
no-cookie part of test and also happens with a checkout before the
syncookie patches got merged. I will take another look at this on
Monday. No need to hold up this fix, so
Acked-by: Florian Westphal <fw@strlen.de>
^ permalink raw reply
* [PATCH] net: Pass NULL to skb_network_protocol() when we don't care about vlan depth
From: linmiaohe @ 2020-08-01 9:36 UTC (permalink / raw)
To: davem, kuba, pshelar, martin.varghese, fw, pabeni, edumazet,
dcaratti, steffen.klassert, shmulik, kyk.segfault
Cc: netdev, linux-kernel, linmiaohe
From: Miaohe Lin <linmiaohe@huawei.com>
When we don't care about vlan depth, we could pass NULL instead of the
address of a unused local variable to skb_network_protocol() as a param.
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
net/core/skbuff.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 3219c26ddfae..8a0c39e4ab0a 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3758,7 +3758,6 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
int err = -ENOMEM;
int i = 0;
int pos;
- int dummy;
if (list_skb && !list_skb->head_frag && skb_headlen(list_skb) &&
(skb_shinfo(head_skb)->gso_type & SKB_GSO_DODGY)) {
@@ -3780,7 +3779,7 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
}
__skb_push(head_skb, doffset);
- proto = skb_network_protocol(head_skb, &dummy);
+ proto = skb_network_protocol(head_skb, NULL);
if (unlikely(!proto))
return ERR_PTR(-EINVAL);
--
2.19.1
^ permalink raw reply related
* Re: [PATCH] net: Phy: Add PHY lookup support on MDIO bus in case of ACPI probe
From: Russell King - ARM Linux admin @ 2020-08-01 9:38 UTC (permalink / raw)
To: Vikas Singh
Cc: Andrew Lunn, f.fainelli, hkallweit1, netdev, Calvin Johnson (OSS),
Kuldip Dwivedi, Madalin Bucur (OSS), Vikas Singh
In-Reply-To: <CADvVLtUAd0X7c39BX791CuyWBcmfBsp7Xvw9Jyp05w45agJY9g@mail.gmail.com>
On Sat, Aug 01, 2020 at 10:23:38AM +0530, Vikas Singh wrote:
> Hi Andrew,
>
> As i have already mentioned that this patch is based on
> https://www.spinics.net/lists/netdev/msg662173.html,
> <https://www.spinics.net/lists/netdev/msg662173.html>
>
> When MDIO bus gets registered itself along with devices on it , the
> function mdiobus_register() inside of_mdiobus_register(), brings
> up all the PHYs on the mdio bus and attach them to the bus with the help
> of_mdiobus_link_mdiodev() inside mdiobus_scan() .
> Additionally it has been discussed with the maintainers that the
> mdiobus_register() function should be capable of handling both ACPI & DTB
> stuff
> without any change to existing implementation.
> Now of_mdiobus_link_mdiodev() inside mdiobus_scan() see if the auto-probed
> phy has a corresponding child in the bus node, and set the "of_node"
> pointer in DT case.
> But lacks to set the "fwnode" pointer in ACPI case which is resulting in
> mdiobus_register() failure as an end result theoretically.
>
> Now this patch set (changes) will attempt to fill this gap and generalise
> the mdiobus_register() implementation for both ACPI & DT with no duplicacy
> or redundancy.
Please do not top-post.
What Andrew is asking is why _should_ a DT specific function (which
starts with of_, meaning "open firmware") have anything to do with
ACPI. The function you refer to (of_mdiobus_link_mdiodev()) is only
built when CONFIG_OF_MDIO is enabled, which is again, a DT specific
thing.
So, why should a DT specific function care about ACPI?
We're not asking about the fine details, we're asking about the high
level idea that DT functions should know about ACPI.
The assignment in of_mdiobus_link_mdiodev() to dev->fwnode is not
itself about ACPI, it's about enabling drivers that wish to access
DT properties through the fwnode property APIs can do so.
IMHO, the right way to go about this is to implement it as a non-DT
function. Given that it is a static function, Andrew may find it
acceptable if you also renamed of_mdiobus_link_mdiodev() as
mdiobus_link_mdiodev() and moved it out of the #ifdef.
+ bus->dev.fwnode = bus->parent->fwnode;
That should be done elsewhere, not here. of_mdiobus_register() already
ensures that this is appropriately set, and if it isn't, maybe there's
a bug elsewhere.
Lastly, note that you don't need two loops, one for ACPI and one for
DT (it's a shame there isn't a device_for_each_available_child_node()):
int addr;
if (dev->fwnode && !bus->dev.fwnode)
return;
device_for_each_child_node(&bus->dev, fwnode) {
if (!fwnode_device_is_available(fwnode))
continue;
if (is_of_node(fwnode))
addr = of_mdio_parse_addr(dev, to_of_node(fwnode));
else if (fwnode_property_read_u32(fwnode, "reg", &addr))
continue;
if (addr == mdiodev->addr) {
dev->of_node = to_of_node(fwnode);
dev->fwnode = fwnode;
return;
}
}
which, I think, will behave identically to the existing implementation
when called in a DT setup, but should also add what you want.
So, maybe with the above, moving it out from under the ifdef, and
renaming it _may_ be acceptable. This is just a suggestion.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* Re: [PATCH 2/2] net: phy: Associate device node with fixed PHY
From: Russell King - ARM Linux admin @ 2020-08-01 9:41 UTC (permalink / raw)
To: Vikas Singh
Cc: Andrew Lunn, f.fainelli, hkallweit1, netdev, Calvin Johnson (OSS),
Kuldip Dwivedi, Madalin Bucur (OSS), Vikas Singh
In-Reply-To: <CADvVLtUrZDGqwEPO_ApCWK1dELkUEjrH47s1CbYEYOH9XgZMRg@mail.gmail.com>
On Sat, Aug 01, 2020 at 09:52:52AM +0530, Vikas Singh wrote:
> Hi Andrew,
>
> Please refer to the "fman" node under
> linux/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts
> I have two 10G ethernet interfaces out of which one is of fixed-link.
Please do not top post.
How does XGMII (which is a 10G only interface) work at 1G speed? Is
what is in DT itself a hack because fixed-phy doesn't support 10G
modes?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* linux-next: Fixes tag needs some work in the bpf-next tree
From: Stephen Rothwell @ 2020-08-01 11:00 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov, Networking
Cc: Linux Next Mailing List, Linux Kernel Mailing List,
Jerry Crunchtime
[-- Attachment #1: Type: text/plain, Size: 409 bytes --]
Hi all,
In commit
1acf8f90ea7e ("libbpf: Fix register in PT_REGS MIPS macros")
Fixes tag
Fixes: c1932cd ("bpf: Add MIPS support to samples/bpf.")
has these problem(s):
- SHA1 should be at least 12 digits long
Can be fixed by setting core.abbrev to 12 (or more) or (for git v2.11
or later) just making sure it is not set (or set to "auto").
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v1] qed: Use %pM format specifier for MAC addresses
From: Andy Shevchenko @ 2020-08-01 12:21 UTC (permalink / raw)
To: Alexander Lobakin
Cc: Ariel Elior, GR-everest-linux-l2, netdev, David S. Miller,
Jakub Kicinski, linux-kernel
In-Reply-To: <SKiAD0R1iJX4FHbr-_GUICKdDvuTvqrJjcR2CQEpE_-GCYtJq-lLbDeec-WmOCZ6NIxW6rca1CRm-d1tSRUu2zFyAapHAjvmgvI5iN6Zvp8=@pm.me>
On Thu, Jul 30, 2020 at 04:26:17PM +0000, Alexander Lobakin wrote:
> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Date: Thu, 30 Jul 2020 18:59:20 +0300
>
> > Convert to %pM instead of using custom code.
> Thanks!
>
> Acked-by: Alexander Lobakin <alobakin@pm.me>
Thanks, but no-one sees this properly (seems broken message-id -> in-reply-to
chain). You have to fix your email.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [RFC PATCH 00/17] Drop uses of pci_read_config_*() return value
From: Saheed O. Bolarinwa @ 2020-08-01 11:24 UTC (permalink / raw)
To: helgaas, Kalle Valo, David S. Miller, Jakub Kicinski,
Wolfgang Grandegger, Marc Kleine-Budde, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Joerg Roedel
Cc: Saheed O. Bolarinwa, bjorn, skhan, linux-kernel-mentees,
linux-pci, linux-kernel, linux-wireless, netdev, linux-mtd, iommu,
linux-rdma, linux-ide, linux-i2c, linux-hwmon, dri-devel,
intel-gfx, linux-gpio, linux-fpga, linux-edac, dmaengine,
linux-crypto, linux-atm-general
The return value of pci_read_config_*() may not indicate a device error.
However, the value read by these functions is more likely to indicate
this kind of error. This presents two overlapping ways of reporting
errors and complicates error checking.
It is possible to move to one single way of checking for error if the
dependencies on the return value of these functions are removed, then it
can later be made to return void.
Remove all uses of the return value of pci_read_config_*().
Check the actual value read for ~0. In this case, ~0 is an invalid value
thus it indicates some kind of error.
In some cases it madkes sence to make the calling function return void
without causing any bug. Future callers can use the value obtained from
these functions for validation. This case pertain to cs5536_read() and
edac_pci_read_dword()
MERGE:
There is no dependency.
Merge individually
Saheed O. Bolarinwa (17):
ata: Drop uses of pci_read_config_*() return value
atm: Drop uses of pci_read_config_*() return value
bcma: Drop uses of pci_read_config_*() return value
hwrng: Drop uses of pci_read_config_*() return value
dmaengine: ioat: Drop uses of pci_read_config_*() return value
edac: Drop uses of pci_read_config_*() return value
fpga: altera-cvp: Drop uses of pci_read_config_*() return value
gpio: Drop uses of pci_read_config_*() return value
drm/i915/vga: Drop uses of pci_read_config_*() return value
hwmon: Drop uses of pci_read_config_*() return value
intel_th: pci: Drop uses of pci_read_config_*() return value
i2c: Drop uses of pci_read_config_*() return value
ide: Drop uses of pci_read_config_*() return value
IB: Drop uses of pci_read_config_*() return value
iommu/vt-d: Drop uses of pci_read_config_*() return value
mtd: Drop uses of pci_read_config_*() return value
net: Drop uses of pci_read_config_*() return value
drivers/ata/pata_cs5536.c | 6 +--
drivers/ata/pata_rz1000.c | 3 +-
drivers/atm/eni.c | 3 +-
drivers/atm/he.c | 12 +++--
drivers/atm/idt77252.c | 9 ++--
drivers/atm/iphase.c | 46 ++++++++++---------
drivers/atm/lanai.c | 4 +-
drivers/atm/nicstar.c | 3 +-
drivers/atm/zatm.c | 9 ++--
drivers/bcma/host_pci.c | 6 ++-
drivers/char/hw_random/amd-rng.c | 6 +--
drivers/dma/ioat/dma.c | 6 +--
drivers/edac/amd64_edac.c | 8 ++--
drivers/edac/amd8111_edac.c | 16 ++-----
drivers/edac/amd8131_edac.c | 6 +--
drivers/edac/i82443bxgx_edac.c | 3 +-
drivers/edac/sb_edac.c | 12 +++--
drivers/edac/skx_common.c | 18 +++++---
drivers/fpga/altera-cvp.c | 8 ++--
drivers/gpio/gpio-amd8111.c | 7 ++-
drivers/gpio/gpio-rdc321x.c | 21 +++++----
drivers/gpu/drm/i915/display/intel_vga.c | 3 +-
drivers/hwmon/i5k_amb.c | 12 +++--
drivers/hwmon/vt8231.c | 8 ++--
drivers/hwtracing/intel_th/pci.c | 12 ++---
drivers/i2c/busses/i2c-ali15x3.c | 6 ++-
drivers/i2c/busses/i2c-elektor.c | 3 +-
drivers/i2c/busses/i2c-nforce2.c | 4 +-
drivers/i2c/busses/i2c-sis5595.c | 17 ++++---
drivers/i2c/busses/i2c-sis630.c | 7 +--
drivers/i2c/busses/i2c-viapro.c | 11 +++--
drivers/ide/cs5536.c | 6 +--
drivers/ide/rz1000.c | 3 +-
drivers/ide/setup-pci.c | 26 +++++++----
drivers/infiniband/hw/hfi1/pcie.c | 38 +++++++--------
drivers/infiniband/hw/mthca/mthca_reset.c | 19 ++++----
drivers/iommu/intel/iommu.c | 6 ++-
drivers/mtd/maps/ichxrom.c | 4 +-
drivers/net/can/peak_canfd/peak_pciefd_main.c | 6 ++-
drivers/net/can/sja1000/peak_pci.c | 6 ++-
drivers/net/ethernet/agere/et131x.c | 11 +++--
.../ethernet/broadcom/bnx2x/bnx2x_ethtool.c | 5 +-
.../cavium/liquidio/cn23xx_pf_device.c | 4 +-
drivers/net/ethernet/marvell/sky2.c | 5 +-
drivers/net/ethernet/mellanox/mlx4/catas.c | 7 +--
drivers/net/ethernet/mellanox/mlx4/reset.c | 10 ++--
.../net/ethernet/myricom/myri10ge/myri10ge.c | 4 +-
drivers/net/wan/farsync.c | 5 +-
.../broadcom/brcm80211/brcmfmac/pcie.c | 4 +-
.../net/wireless/intel/iwlwifi/pcie/trans.c | 15 ++++--
50 files changed, 270 insertions(+), 209 deletions(-)
--
2.18.4
^ permalink raw reply
* [RFC PATCH 02/17] atm: Drop uses of pci_read_config_*() return value
From: Saheed O. Bolarinwa @ 2020-08-01 11:24 UTC (permalink / raw)
To: helgaas, Chas Williams
Cc: Saheed O. Bolarinwa, bjorn, skhan, linux-kernel-mentees,
linux-pci, linux-kernel, linux-atm-general, netdev
In-Reply-To: <20200801112446.149549-1-refactormyself@gmail.com>
The return value of pci_read_config_*() may not indicate a device error.
However, the value read by these functions is more likely to indicate
this kind of error. This presents two overlapping ways of reporting
errors and complicates error checking.
It is possible to move to one single way of checking for error if the
dependency on the return value of these functions is removed, then it
can later be made to return void.
Remove all uses of the return value of pci_read_config_*().
Check the actual value read for ~0. In this case, ~0 is an invalid
value thus it indicates some kind of error.
Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
Signed-off-by: Saheed O. Bolarinwa <refactormyself@gmail.com>
---
drivers/atm/eni.c | 3 ++-
drivers/atm/he.c | 12 +++++++----
drivers/atm/idt77252.c | 9 ++++++---
drivers/atm/iphase.c | 46 +++++++++++++++++++++++-------------------
drivers/atm/lanai.c | 4 ++--
drivers/atm/nicstar.c | 3 ++-
drivers/atm/zatm.c | 9 +++++----
7 files changed, 50 insertions(+), 36 deletions(-)
diff --git a/drivers/atm/eni.c b/drivers/atm/eni.c
index 17d47ad03ab7..5beed8a25fa2 100644
--- a/drivers/atm/eni.c
+++ b/drivers/atm/eni.c
@@ -1585,7 +1585,8 @@ static char * const media_name[] = {
} })
#define GET_SEPROM \
({ if (!error && !pci_error) { \
- pci_error = pci_read_config_byte(eni_dev->pci_dev,PCI_TONGA_CTRL,&tonga); \
+ pci_read_config_byte(eni_dev->pci_dev, PCI_TONGA_CTRL, &tonga); \
+ pci_error = (tonga == (u8)~0) ? -1 : 0; \
udelay(10); /* 10 usecs */ \
} })
diff --git a/drivers/atm/he.c b/drivers/atm/he.c
index 8af793f5e811..8727ae7746fb 100644
--- a/drivers/atm/he.c
+++ b/drivers/atm/he.c
@@ -995,7 +995,8 @@ static int he_start(struct atm_dev *dev)
*/
/* 4.3 pci bus controller-specific initialization */
- if (pci_read_config_dword(pci_dev, GEN_CNTL_0, &gen_cntl_0) != 0) {
+ pci_read_config_dword(pci_dev, GEN_CNTL_0, &gen_cntl_0);
+ if (gen_cntl_0 == (u32)~0) {
hprintk("can't read GEN_CNTL_0\n");
return -EINVAL;
}
@@ -1005,7 +1006,8 @@ static int he_start(struct atm_dev *dev)
return -EINVAL;
}
- if (pci_read_config_word(pci_dev, PCI_COMMAND, &command) != 0) {
+ pci_read_config_word(pci_dev, PCI_COMMAND, &command);
+ if (command == (u16)~0) {
hprintk("can't read PCI_COMMAND.\n");
return -EINVAL;
}
@@ -1016,7 +1018,8 @@ static int he_start(struct atm_dev *dev)
return -EINVAL;
}
- if (pci_read_config_byte(pci_dev, PCI_CACHE_LINE_SIZE, &cache_size)) {
+ pci_read_config_byte(pci_dev, PCI_CACHE_LINE_SIZE, &cache_size);
+ if (cache_size == (u8)~0) {
hprintk("can't read cache line size?\n");
return -EINVAL;
}
@@ -1027,7 +1030,8 @@ static int he_start(struct atm_dev *dev)
hprintk("can't set cache line size to %d\n", cache_size);
}
- if (pci_read_config_byte(pci_dev, PCI_LATENCY_TIMER, &timer)) {
+ pci_read_config_byte(pci_dev, PCI_LATENCY_TIMER, &timer);
+ if (timer == (u8)~0) {
hprintk("can't read latency timer?\n");
return -EINVAL;
}
diff --git a/drivers/atm/idt77252.c b/drivers/atm/idt77252.c
index df51680e8931..f4b0c2ecae62 100644
--- a/drivers/atm/idt77252.c
+++ b/drivers/atm/idt77252.c
@@ -3271,7 +3271,8 @@ static int init_card(struct atm_dev *dev)
/* Set PCI Retry-Timeout and TRDY timeout */
IPRINTK("%s: Checking PCI retries.\n", card->name);
- if (pci_read_config_byte(pcidev, 0x40, &pci_byte) != 0) {
+ pci_read_config_byte(pcidev, 0x40, &pci_byte);
+ if (pci_byte == (u_char)~0) {
printk("%s: can't read PCI retry timeout.\n", card->name);
deinit_card(card);
return -1;
@@ -3287,7 +3288,8 @@ static int init_card(struct atm_dev *dev)
}
}
IPRINTK("%s: Checking PCI TRDY.\n", card->name);
- if (pci_read_config_byte(pcidev, 0x41, &pci_byte) != 0) {
+ pci_read_config_byte(pcidev, 0x41, &pci_byte);
+ if (pci_byte == (u_char)~0) {
printk("%s: can't read PCI TRDY timeout.\n", card->name);
deinit_card(card);
return -1;
@@ -3535,7 +3537,8 @@ static int idt77252_preset(struct idt77252_dev *card)
XPRINTK("%s: Enable PCI master and memory access for SAR.\n",
card->name);
- if (pci_read_config_word(card->pcidev, PCI_COMMAND, &pci_command)) {
+ pci_read_config_word(card->pcidev, PCI_COMMAND, &pci_command);
+ if (pci_command == (u16)~0) {
printk("%s: can't read PCI_COMMAND.\n", card->name);
deinit_card(card);
return -1;
diff --git a/drivers/atm/iphase.c b/drivers/atm/iphase.c
index 8c7a996d1f16..d3f2fac3a7d1 100644
--- a/drivers/atm/iphase.c
+++ b/drivers/atm/iphase.c
@@ -2287,25 +2287,29 @@ static int get_esi(struct atm_dev *dev)
return 0;
}
-static int reset_sar(struct atm_dev *dev)
-{
- IADEV *iadev;
- int i, error = 1;
- unsigned int pci[64];
-
- iadev = INPH_IA_DEV(dev);
- for(i=0; i<64; i++)
- if ((error = pci_read_config_dword(iadev->pci,
- i*4, &pci[i])) != PCIBIOS_SUCCESSFUL)
- return error;
- writel(0, iadev->reg+IPHASE5575_EXT_RESET);
- for(i=0; i<64; i++)
- if ((error = pci_write_config_dword(iadev->pci,
- i*4, pci[i])) != PCIBIOS_SUCCESSFUL)
- return error;
- udelay(5);
- return 0;
-}
+static int reset_sar(struct atm_dev *dev)
+{
+ IADEV *iadev;
+ int i, error = 1;
+ unsigned int pci[64];
+
+ iadev = INPH_IA_DEV(dev);
+ for (i = 0; i < 64; i++) {
+ pci_read_config_dword(iadev->pci, i*4, &pci[i]);
+ if (pci[i] == (u32)~0)
+ return error;
+ }
+
+ writel(0, iadev->reg+IPHASE5575_EXT_RESET);
+ for (i = 0; i < 64; i++) {
+ error = pci_write_config_dword(iadev->pci, i*4, pci[i]);
+ if (error != PCIBIOS_SUCCESSFUL)
+ return error;
+ }
+
+ udelay(5);
+ return 0;
+}
static int ia_init(struct atm_dev *dev)
@@ -2328,8 +2332,8 @@ static int ia_init(struct atm_dev *dev)
real_base = pci_resource_start (iadev->pci, 0);
iadev->irq = iadev->pci->irq;
- error = pci_read_config_word(iadev->pci, PCI_COMMAND, &command);
- if (error) {
+ pci_read_config_word(iadev->pci, PCI_COMMAND, &command);
+ if (command == (u16)~0) {
printk(KERN_ERR DEV_LABEL "(itf %d): init error 0x%x\n",
dev->number,error);
return -EINVAL;
diff --git a/drivers/atm/lanai.c b/drivers/atm/lanai.c
index 645a6bc1df88..aafe1f934385 100644
--- a/drivers/atm/lanai.c
+++ b/drivers/atm/lanai.c
@@ -1097,8 +1097,8 @@ static void pcistatus_check(struct lanai_dev *lanai, int clearonly)
{
u16 s;
int result;
- result = pci_read_config_word(lanai->pci, PCI_STATUS, &s);
- if (result != PCIBIOS_SUCCESSFUL) {
+ pci_read_config_word(lanai->pci, PCI_STATUS, &s);
+ if (s == (u16)~0) {
printk(KERN_ERR DEV_LABEL "(itf %d): can't read PCI_STATUS: "
"%d\n", lanai->number, result);
return;
diff --git a/drivers/atm/nicstar.c b/drivers/atm/nicstar.c
index 7af74fb450a0..74f49f54e024 100644
--- a/drivers/atm/nicstar.c
+++ b/drivers/atm/nicstar.c
@@ -399,7 +399,8 @@ static int ns_init_card(int i, struct pci_dev *pcidev)
pci_set_master(pcidev);
- if (pci_read_config_byte(pcidev, PCI_LATENCY_TIMER, &pci_latency) != 0) {
+ pci_read_config_byte(pcidev, PCI_LATENCY_TIMER, &pci_latency);
+ if (pci_latency == (u8)~0) {
printk("nicstar%d: can't read PCI latency timer.\n", i);
error = 6;
ns_init_card_error(card, error);
diff --git a/drivers/atm/zatm.c b/drivers/atm/zatm.c
index 57f97b95a453..8106ee20a94c 100644
--- a/drivers/atm/zatm.c
+++ b/drivers/atm/zatm.c
@@ -1112,11 +1112,11 @@ static void eprom_set(struct zatm_dev *zatm_dev, unsigned long value,
static unsigned long eprom_get(struct zatm_dev *zatm_dev, unsigned short cmd)
{
unsigned int value;
- int error;
- if ((error = pci_read_config_dword(zatm_dev->pci_dev,cmd,&value)))
+ pci_read_config_dword(zatm_dev->pci_dev, cmd, &value);
+ if (value == (u64)~0)
printk(KERN_ERR DEV_LABEL ": PCI read failed (0x%02x)\n",
- error);
+ value);
return value;
}
@@ -1197,7 +1197,8 @@ static int zatm_init(struct atm_dev *dev)
pci_dev = zatm_dev->pci_dev;
zatm_dev->base = pci_resource_start(pci_dev, 0);
zatm_dev->irq = pci_dev->irq;
- if ((error = pci_read_config_word(pci_dev,PCI_COMMAND,&command))) {
+ pci_read_config_word(pci_dev, PCI_COMMAND, &command);
+ if (command == (u16)~0) {
printk(KERN_ERR DEV_LABEL "(itf %d): init error 0x%02x\n",
dev->number,error);
return -EINVAL;
--
2.18.4
^ 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