Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 1/2] net: dpaa2: move DPAA2 PTP driver out of staging/
From: Andrew Lunn @ 2018-09-27 13:25 UTC (permalink / raw)
  To: Yangbo Lu
  Cc: linux-kernel, devel, netdev, Richard Cochran, David S . Miller,
	Ioana Radulescu, Greg Kroah-Hartman
In-Reply-To: <20180927111228.46118-1-yangbo.lu@nxp.com>

On Thu, Sep 27, 2018 at 07:12:27PM +0800, Yangbo Lu wrote:
> This patch is to move DPAA2 PTP driver out of staging/
> since the dpaa2-eth had been moved out.
> 
> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> ---
>  drivers/net/ethernet/freescale/Kconfig             |    9 +--------
>  drivers/net/ethernet/freescale/dpaa2/Kconfig       |   15 +++++++++++++++
>  drivers/net/ethernet/freescale/dpaa2/Makefile      |    6 ++++--
>  .../ethernet/freescale/dpaa2}/dprtc-cmd.h          |    0
>  .../rtc => net/ethernet/freescale/dpaa2}/dprtc.c   |    0
>  .../rtc => net/ethernet/freescale/dpaa2}/dprtc.h   |    0
>  .../rtc => net/ethernet/freescale/dpaa2}/rtc.c     |    0
>  .../rtc => net/ethernet/freescale/dpaa2}/rtc.h     |    0
>  drivers/staging/fsl-dpaa2/Kconfig                  |    8 --------
>  drivers/staging/fsl-dpaa2/Makefile                 |    1 -
>  drivers/staging/fsl-dpaa2/rtc/Makefile             |    7 -------
>  11 files changed, 20 insertions(+), 26 deletions(-)
>  create mode 100644 drivers/net/ethernet/freescale/dpaa2/Kconfig
>  rename drivers/{staging/fsl-dpaa2/rtc => net/ethernet/freescale/dpaa2}/dprtc-cmd.h (100%)
>  rename drivers/{staging/fsl-dpaa2/rtc => net/ethernet/freescale/dpaa2}/dprtc.c (100%)
>  rename drivers/{staging/fsl-dpaa2/rtc => net/ethernet/freescale/dpaa2}/dprtc.h (100%)
>  rename drivers/{staging/fsl-dpaa2/rtc => net/ethernet/freescale/dpaa2}/rtc.c (100%)
>  rename drivers/{staging/fsl-dpaa2/rtc => net/ethernet/freescale/dpaa2}/rtc.h (100%)

Hi Yangbo

Calling a ptp driver rtc.[ch] seems rather odd. Could you fixup the
name, change it to ptp.[ch]. Also, some of the function names, and
structures, rtc_probe->ptp_probe, rtc_remove->ptp_remove,
rtc_match_id_table-> ptp_match_id_table, etc.

ptp_dpaa2_adjfreq() probably should return err, not 0.
ptp_dpaa2_gettime() again does not return the error.
If fact, it seems like all the main functions ignore errors.

kzalloc() could be changed to devm_kzalloc() to simplify the cleanup
Can ptp_dpaa2_caps be made const?
dpaa2_phc_index does not appear to be used.
dev_set_drvdata(dev, NULL); is not needed.
Can rtc_drv be made const?
Is rtc.h used by anything other than rtc.c? It seems like it can be removed.

It seems like there is a lot of code in dprtc.c which is unused. rtc.c
does nothing with interrupts for example. Do you plan to make use of
this extra code? Or can it be removed leaving just what is needed?

struct dprtc_cmd_get_irq - Putting pad at the beginning of a struct
seems very odd. And it is not the only example.

      Andrew

^ permalink raw reply

* Re: KASAN: use-after-free Read in tcf_block_find
From: Eric Dumazet @ 2018-09-27 13:24 UTC (permalink / raw)
  To: Dmitry Vyukov, Eric Dumazet
  Cc: Cong Wang, syzbot+37b8770e6d5a8220a039, David Miller,
	Jamal Hadi Salim, Jiri Pirko, LKML,
	Linux Kernel Network Developers, syzkaller-bugs
In-Reply-To: <CACT4Y+ZvoVsL_BjBWr7QPnXOPGXqkqKfo-NQcox7vaAFPWVUVg@mail.gmail.com>



On 09/27/2018 06:02 AM, Dmitry Vyukov wrote:

> I am not suggesting to commit this. This is just a hack for debugging.
> It in fact lead to some warnings, but still allowed me to reproduce
> the bug reliably.
> 

Had you got more meaningful stack traces ?

(Showing which context was actually doing the dst_release())

>>> +                               dst_destroy_rcu(&dst->rcu_head);
>>> +                       }
>>> +               }
>>>         }
>>>  }

Thanks.

^ permalink raw reply

* Re: KASAN: use-after-free Read in tcf_block_find
From: Dmitry Vyukov @ 2018-09-27 13:02 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Cong Wang, syzbot+37b8770e6d5a8220a039, David Miller,
	Jamal Hadi Salim, Jiri Pirko, LKML,
	Linux Kernel Network Developers, syzkaller-bugs
In-Reply-To: <de41f26b-efe6-9985-cf38-169c18791cd6@gmail.com>

On Thu, Sep 27, 2018 at 3:00 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
> On 09/27/2018 01:10 AM, Dmitry Vyukov wrote:
>
>>
>> Would a stack trace for call_rcu be helpful here? I have this idea for
>> a long time, but never get around to implementing it:
>> https://bugzilla.kernel.org/show_bug.cgi?id=198437
>>
>> Also FWIW I recently used the following hack for another net bug. It
>> made that other bug involving call_rcu way more likely to fire. Maybe
>> it will be helpful here too.
>>
>> diff --git a/net/core/dst.c b/net/core/dst.c
>> index 81ccf20e28265..591a8d0aca545 100644
>> --- a/net/core/dst.c
>> +++ b/net/core/dst.c
>> @@ -187,8 +187,16 @@ void dst_release(struct dst_entry *dst)
>>                 if (unlikely(newrefcnt < 0))
>>                         net_warn_ratelimited("%s: dst:%p refcnt:%d\n",
>>                                              __func__, dst, newrefcnt);
>> -               if (!newrefcnt)
>> -                       call_rcu(&dst->rcu_head, dst_destroy_rcu);
>> +               if (!newrefcnt) {
>> +                       if (lock_is_held(&rcu_bh_lock_map) ||
>> +                               lock_is_held(&rcu_lock_map) ||
>> +                               lock_is_held(&rcu_sched_lock_map)) {
>> +                               call_rcu(&dst->rcu_head, dst_destroy_rcu);
>> +                       } else {
>> +                               synchronize_rcu();
>
> dst_release() can be called in context we hold a spinlock, this would be bad to reschedule here.

I am not suggesting to commit this. This is just a hack for debugging.
It in fact lead to some warnings, but still allowed me to reproduce
the bug reliably.

>> +                               dst_destroy_rcu(&dst->rcu_head);
>> +                       }
>> +               }
>>         }
>>  }

^ permalink raw reply

* Re: KASAN: use-after-free Read in tcf_block_find
From: Eric Dumazet @ 2018-09-27 13:00 UTC (permalink / raw)
  To: Dmitry Vyukov, Cong Wang
  Cc: Eric Dumazet, syzbot+37b8770e6d5a8220a039, David Miller,
	Jamal Hadi Salim, Jiri Pirko, LKML,
	Linux Kernel Network Developers, syzkaller-bugs
In-Reply-To: <CACT4Y+YbQbZ7XTPK1HLvPiR9rmSm7Cz0H2LJAFfYfAta1Y3CiQ@mail.gmail.com>



On 09/27/2018 01:10 AM, Dmitry Vyukov wrote:

> 
> Would a stack trace for call_rcu be helpful here? I have this idea for
> a long time, but never get around to implementing it:
> https://bugzilla.kernel.org/show_bug.cgi?id=198437
> 
> Also FWIW I recently used the following hack for another net bug. It
> made that other bug involving call_rcu way more likely to fire. Maybe
> it will be helpful here too.
> 
> diff --git a/net/core/dst.c b/net/core/dst.c
> index 81ccf20e28265..591a8d0aca545 100644
> --- a/net/core/dst.c
> +++ b/net/core/dst.c
> @@ -187,8 +187,16 @@ void dst_release(struct dst_entry *dst)
>                 if (unlikely(newrefcnt < 0))
>                         net_warn_ratelimited("%s: dst:%p refcnt:%d\n",
>                                              __func__, dst, newrefcnt);
> -               if (!newrefcnt)
> -                       call_rcu(&dst->rcu_head, dst_destroy_rcu);
> +               if (!newrefcnt) {
> +                       if (lock_is_held(&rcu_bh_lock_map) ||
> +                               lock_is_held(&rcu_lock_map) ||
> +                               lock_is_held(&rcu_sched_lock_map)) {
> +                               call_rcu(&dst->rcu_head, dst_destroy_rcu);
> +                       } else {
> +                               synchronize_rcu();

dst_release() can be called in context we hold a spinlock, this would be bad to reschedule here.

> +                               dst_destroy_rcu(&dst->rcu_head);
> +                       }
> +               }
>         }
>  }
> 

^ permalink raw reply

* [PATCH net-next] qed: Remove set but not used variable 'p_archipelago'
From: YueHaibing @ 2018-09-27  6:45 UTC (permalink / raw)
  To: Ariel Elior, David S. Miller
  Cc: YueHaibing, everest-linux-l2, netdev, kernel-janitors

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/net/ethernet/qlogic/qed/qed_ooo.c: In function 'qed_ooo_delete_isles':
drivers/net/ethernet/qlogic/qed/qed_ooo.c:354:30: warning:
 variable 'p_archipelago' set but not used [-Wunused-but-set-variable]
 
drivers/net/ethernet/qlogic/qed/qed_ooo.c: In function 'qed_ooo_join_isles':
drivers/net/ethernet/qlogic/qed/qed_ooo.c:463:30: warning:
 variable 'p_archipelago' set but not used [-Wunused-but-set-variable]

Since commit 1eec2437d14c ("qed: Make OOO archipelagos into an array"),
'p_archipelago' is no longer in use.

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/ethernet/qlogic/qed/qed_ooo.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_ooo.c b/drivers/net/ethernet/qlogic/qed/qed_ooo.c
index 63931df..ffac4ac 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_ooo.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_ooo.c
@@ -351,11 +351,9 @@ void qed_ooo_delete_isles(struct qed_hwfn *p_hwfn,
 			  struct qed_ooo_info *p_ooo_info,
 			  u32 cid, u8 drop_isle, u8 drop_size)
 {
-	struct qed_ooo_archipelago *p_archipelago = NULL;
 	struct qed_ooo_isle *p_isle = NULL;
 	u8 isle_idx;
 
-	p_archipelago = qed_ooo_seek_archipelago(p_hwfn, p_ooo_info, cid);
 	for (isle_idx = 0; isle_idx < drop_size; isle_idx++) {
 		p_isle = qed_ooo_seek_isle(p_hwfn, p_ooo_info, cid, drop_isle);
 		if (!p_isle) {
@@ -460,7 +458,6 @@ void qed_ooo_add_new_buffer(struct qed_hwfn *p_hwfn,
 void qed_ooo_join_isles(struct qed_hwfn *p_hwfn,
 			struct qed_ooo_info *p_ooo_info, u32 cid, u8 left_isle)
 {
-	struct qed_ooo_archipelago *p_archipelago = NULL;
 	struct qed_ooo_isle *p_right_isle = NULL;
 	struct qed_ooo_isle *p_left_isle = NULL;
 
@@ -473,7 +470,6 @@ void qed_ooo_join_isles(struct qed_hwfn *p_hwfn,
 		return;
 	}
 
-	p_archipelago = qed_ooo_seek_archipelago(p_hwfn, p_ooo_info, cid);
 	list_del(&p_right_isle->list_entry);
 	p_ooo_info->cur_isles_number--;
 	if (left_isle) {

^ permalink raw reply related

* [PATCH net V2] vhost-vsock: fix use after free
From: Jason Wang @ 2018-09-27 12:22 UTC (permalink / raw)
  To: stefanha, mst
  Cc: kvm, virtualization, netdev, linux-kernel, sergei.shtylyov,
	Jason Wang

The access of vsock is not protected by vhost_vsock_lock. This may
lead to use after free since vhost_vsock_dev_release() may free the
pointer at the same time.

Fix this by holding the lock during the access.

Reported-by: syzbot+e3e074963495f92a89ed@syzkaller.appspotmail.com
Fixes: 16320f363ae1 ("vhost-vsock: add pkt cancel capability")
Fixes: 433fc58e6bf2 ("VSOCK: Introduce vhost_vsock.ko")
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
- V2: fix typos
- The patch is needed for -stable.
---
 drivers/vhost/vsock.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 34bc3ab40c6d..7d0b292867fd 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -210,21 +210,27 @@ vhost_transport_send_pkt(struct virtio_vsock_pkt *pkt)
 	struct vhost_vsock *vsock;
 	int len = pkt->len;
 
+	spin_lock_bh(&vhost_vsock_lock);
+
 	/* Find the vhost_vsock according to guest context id  */
-	vsock = vhost_vsock_get(le64_to_cpu(pkt->hdr.dst_cid));
+	vsock = __vhost_vsock_get(le64_to_cpu(pkt->hdr.dst_cid));
 	if (!vsock) {
 		virtio_transport_free_pkt(pkt);
+		spin_unlock_bh(&vhost_vsock_lock);
 		return -ENODEV;
 	}
 
 	if (pkt->reply)
 		atomic_inc(&vsock->queued_replies);
 
-	spin_lock_bh(&vsock->send_pkt_list_lock);
+	spin_lock(&vsock->send_pkt_list_lock);
 	list_add_tail(&pkt->list, &vsock->send_pkt_list);
-	spin_unlock_bh(&vsock->send_pkt_list_lock);
+	spin_unlock(&vsock->send_pkt_list_lock);
 
 	vhost_work_queue(&vsock->dev, &vsock->send_pkt_work);
+
+	spin_unlock_bh(&vhost_vsock_lock);
+
 	return len;
 }
 
@@ -236,18 +242,22 @@ vhost_transport_cancel_pkt(struct vsock_sock *vsk)
 	int cnt = 0;
 	LIST_HEAD(freeme);
 
+	spin_lock_bh(&vhost_vsock_lock);
+
 	/* Find the vhost_vsock according to guest context id  */
-	vsock = vhost_vsock_get(vsk->remote_addr.svm_cid);
-	if (!vsock)
+	vsock = __vhost_vsock_get(vsk->remote_addr.svm_cid);
+	if (!vsock) {
+		spin_unlock_bh(&vhost_vsock_lock);
 		return -ENODEV;
+	}
 
-	spin_lock_bh(&vsock->send_pkt_list_lock);
+	spin_lock(&vsock->send_pkt_list_lock);
 	list_for_each_entry_safe(pkt, n, &vsock->send_pkt_list, list) {
 		if (pkt->vsk != vsk)
 			continue;
 		list_move(&pkt->list, &freeme);
 	}
-	spin_unlock_bh(&vsock->send_pkt_list_lock);
+	spin_unlock(&vsock->send_pkt_list_lock);
 
 	list_for_each_entry_safe(pkt, n, &freeme, list) {
 		if (pkt->reply)
@@ -265,6 +275,8 @@ vhost_transport_cancel_pkt(struct vsock_sock *vsk)
 			vhost_poll_queue(&tx_vq->poll);
 	}
 
+	spin_unlock_bh(&vhost_vsock_lock);
+
 	return 0;
 }
 
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH net] vhost-vsock: fix use after free
From: Jason Wang @ 2018-09-27 12:21 UTC (permalink / raw)
  To: Sergei Shtylyov, stefanha, mst; +Cc: kvm, virtualization, netdev, linux-kernel
In-Reply-To: <a413a8b4-302d-7fbc-092d-95e8aa421c4c@cogentembedded.com>



On 2018年09月27日 17:52, Sergei Shtylyov wrote:
> Hello!
>
> On 9/27/2018 11:43 AM, Jason Wang wrote:
>
>    Just a couple of typos...
>
>> The access of vsock is not protected by vhost_vsock_lock. This may
>> lead use after free since vhost_vsock_dev_release() may free the
>
>   Lead to use.
>
>> pointer at the same time.
>>
>> Fix this by holding the lock during the acess.
>
>    Access.
>
>> Reported-by: syzbot+e3e074963495f92a89ed@syzkaller.appspotmail.com
>> Fixes: 16320f363ae1 ("vhost-vsock: add pkt cancel capability")
>> Fixes: 433fc58e6bf2 ("VSOCK: Introduce vhost_vsock.ko")
>> Cc: Stefan Hajnoczi <stefanha@redhat.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
> [...]
>
> MBR, Sergei

Let me post V2.

Thanks

^ permalink raw reply

* (sin asunto)
From: Mrs. Mavis Wanczyk @ 2018-09-27  4:53 UTC (permalink / raw)




-- 
Good Day,

I, Mavis Wanczyk donates $ 5 Million Dollars to you, from part of my 
Powerball
Jackpot Lottery of $ 758 Million Dollars, respond with your details for 
claims.

I await your earliest response and God Bless you.

Good luck,
Mavis Wanczyk.

^ permalink raw reply

* [PATCH 1/1] Update maintainers for bnx2/bnx2x/qlge/qlcnic drivers.
From: Sudarsana Reddy Kalluru @ 2018-09-27  4:57 UTC (permalink / raw)
  To: davem; +Cc: netdev, Ameen.Rahman

Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
Signed-off-by: Ameen Rahman <Ameen.Rahman@cavium.com>
---
 MAINTAINERS | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 15565de..e941d1b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2956,7 +2956,6 @@ F:	include/linux/bcm963xx_tag.h
 
 BROADCOM BNX2 GIGABIT ETHERNET DRIVER
 M:	Rasesh Mody <rasesh.mody@cavium.com>
-M:	Harish Patil <harish.patil@cavium.com>
 M:	Dept-GELinuxNICDev@cavium.com
 L:	netdev@vger.kernel.org
 S:	Supported
@@ -2977,6 +2976,7 @@ F:	drivers/scsi/bnx2i/
 
 BROADCOM BNX2X 10 GIGABIT ETHERNET DRIVER
 M:	Ariel Elior <ariel.elior@cavium.com>
+M:	Sudarsana Kalluru <sudarsana.kalluru@cavium.com>
 M:	everest-linux-l2@cavium.com
 L:	netdev@vger.kernel.org
 S:	Supported
@@ -11982,7 +11982,7 @@ F:	Documentation/scsi/LICENSE.qla4xxx
 F:	drivers/scsi/qla4xxx/
 
 QLOGIC QLCNIC (1/10)Gb ETHERNET DRIVER
-M:	Harish Patil <harish.patil@cavium.com>
+M:	Shahed Shaikh <Shahed.Shaikh@cavium.com>
 M:	Manish Chopra <manish.chopra@cavium.com>
 M:	Dept-GELinuxNICDev@cavium.com
 L:	netdev@vger.kernel.org
@@ -11990,7 +11990,6 @@ S:	Supported
 F:	drivers/net/ethernet/qlogic/qlcnic/
 
 QLOGIC QLGE 10Gb ETHERNET DRIVER
-M:	Harish Patil <harish.patil@cavium.com>
 M:	Manish Chopra <manish.chopra@cavium.com>
 M:	Dept-GELinuxNICDev@cavium.com
 L:	netdev@vger.kernel.org
-- 
1.8.3.1

^ permalink raw reply related

* Re: KMSAN: uninit-value in memcmp (2)
From: Vladis Dronov @ 2018-09-27 11:17 UTC (permalink / raw)
  To: Dmitry Vyukov; +Cc: syzbot, syzkaller-bugs, LKML, Networking
In-Reply-To: <CACT4Y+a8_4=dmZavdmZjPOraqYAtjBB228TjurbwLKSgJ9+J7Q@mail.gmail.com>

Hello, Dmirty,

Thank you for the explanation of how syzkaller/syzbot works in this and
other emails. I understand that is it a complicated task to determine
and categorize bugs based on just crash dump and messages, and syzkaller
does a great job of doing so.

> Re __hw_addr_add_ex bug, as Alex noted the crash was detected _after_
> the fixing commit went in. So it's something new and different and
> can't be fixed by the older commit.

Indeed, you're right, there is another issue with tun/tap devices which
leads to this bug. I've posted a patch (https://lkml.org/lkml/2018/9/26/416)
to fix it.

I hope I did not do much damage, reporting previous fix as a fix for this bug,
as syzkaller will probably create another "KMSAN: uninit-value in <...>"
report.

Best regards,
Vladis Dronov | Red Hat, Inc. | Product Security Engineer

----- Original Message -----
> From: "Dmitry Vyukov" <dvyukov@google.com>
> To: "Alexander Potapenko" <glider@google.com>
> Cc: "Vladis Dronov" <vdronov@redhat.com>, "syzbot" <syzbot+d3402c47f680ff24b29c@syzkaller.appspotmail.com>,
> "syzkaller-bugs" <syzkaller-bugs@googlegroups.com>, "David Miller" <davem@davemloft.net>, "Eric Dumazet"
> <edumazet@google.com>, "LKML" <linux-kernel@vger.kernel.org>, "Networking" <netdev@vger.kernel.org>, "sunlianwen"
> <sunlw.fnst@cn.fujitsu.com>
> Sent: Monday, September 24, 2018 11:39:08 AM
> Subject: Re: KMSAN: uninit-value in memcmp (2)
> 
> On Mon, Sep 24, 2018 at 8:53 AM, Alexander Potapenko <glider@google.com>
> wrote:
> > On Mon, Sep 24, 2018 at 12:09 AM Vladis Dronov <vdronov@redhat.com> wrote:
> >>
> >> Hello, Dmirty,
> >>
> >> Thank you for the reply. Can we please, discuss this further?
> > Hi Vladis,
> >> > You can see on dashboard that the last crash
> >> > for the second version (2) happened just few days ago. So this is a
> >> > different bug.
> > FWIW I've just double-checked that the reproducer provided by
> > syzkaller in the original message still triggers the report from the
> > original message in the latest KMSAN tree (which already contains the
> > __hw_addr_add_ex() fix from April).
> >> Well... yes and no. When I was looking at this bug (bug?id=088efeac32fd) I
> >> was looking
> >> at the report at "2018/05/09 18:55"
> >> (https://syzkaller.appspot.com/text?tag=CrashReport&x=141b707b800000),
> >> since it was the only report with a reproducer. This was my error.
> >>
> >> The error and the call trace in this report are:
> >>
> >> >>>
> >> BUG: KMSAN: uninit-value in memcmp+0x119/0x180 lib/string.c:861
> >> CPU: 0 PID: 38 Comm: kworker/0:1 Not tainted 4.17.0-rc3+ #88
> >> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> >> Google 01/01/2011
> >> Workqueue: ipv6_addrconf addrconf_dad_work
> >> Call Trace:
> >>  __dump_stack lib/dump_stack.c:77 [inline]
> >>  dump_stack+0x185/0x1d0 lib/dump_stack.c:113
> >>  kmsan_report+0x142/0x240 mm/kmsan/kmsan.c:1067
> >>  __msan_warning_32+0x6c/0xb0 mm/kmsan/kmsan_instr.c:683
> >>  memcmp+0x119/0x180 lib/string.c:861
> >>  __hw_addr_add_ex net/core/dev_addr_lists.c:61 [inline]
> >>  __dev_mc_add+0x1fc/0x900 net/core/dev_addr_lists.c:670
> >>  dev_mc_add+0x6d/0x80 net/core/dev_addr_lists.c:687
> >>  igmp6_group_added+0x2db/0xa00 net/ipv6/mcast.c:662
> >>  ipv6_dev_mc_inc+0xe9e/0x1130 net/ipv6/mcast.c:914
> >>  addrconf_join_solict net/ipv6/addrconf.c:2103 [inline]
> >>  addrconf_dad_begin net/ipv6/addrconf.c:3853 [inline]
> >>  addrconf_dad_work+0x462/0x2a20 net/ipv6/addrconf.c:3979
> >>  process_one_work+0x12c6/0x1f60 kernel/workqueue.c:2145
> >>  worker_thread+0x113c/0x24f0 kernel/workqueue.c:2279
> >>  kthread+0x539/0x720 kernel/kthread.c:239
> >>  ret_from_fork+0x35/0x40 arch/x86/entry/entry_64.S:412
> >>
> >> Local variable description: ----buf@igmp6_group_added
> >> Variable was created at:
> >>  igmp6_group_added+0x4a/0xa00 net/ipv6/mcast.c:650
> >>  ipv6_dev_mc_inc+0xe9e/0x1130 net/ipv6/mcast.c:914
> >> <<<
> >>
> >> It is the same like in bug?id=3887c0d99aecb27d085180c5222d245d08a30806
> >> which, after some more test, made me believe these bugs are duplicate
> >> and are fixed by the same commit.
> >>
> >> But let's look at another report at "2018/09/12 21:00"
> >> (https://syzkaller.appspot.com/text?tag=CrashReport&x=14f99b71400000)
> >> at the bug (bug?id=088efeac32fd), the one you've mentioned as
> >> "the last crash for the second version (2) happened just few days ago".
> >>
> >> Its error and the call trace are completely different:
> >>
> >> >>>
> >> BUG: KMSAN: uninit-value in memcmp+0x11d/0x180 lib/string.c:863
> >> CPU: 0 PID: 6107 Comm: syz-executor4 Not tainted 4.19.0-rc3+ #45
> >> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> >> Google 01/01/2011
> >> Call Trace:
> >>  __dump_stack lib/dump_stack.c:77 [inline]
> >>  dump_stack+0x14b/0x190 lib/dump_stack.c:113
> >>  kmsan_report+0x183/0x2b0 mm/kmsan/kmsan.c:956
> >>  __msan_warning+0x70/0xc0 mm/kmsan/kmsan_instr.c:645
> >>  memcmp+0x11d/0x180 lib/string.c:863
> >>  dev_uc_add_excl+0x165/0x7b0 net/core/dev_addr_lists.c:464
> >>  ndo_dflt_fdb_add net/core/rtnetlink.c:3463 [inline]
> >>  rtnl_fdb_add+0x1081/0x1270 net/core/rtnetlink.c:3558
> >>  rtnetlink_rcv_msg+0xa0b/0x1530 net/core/rtnetlink.c:4715
> >>  netlink_rcv_skb+0x36e/0x5f0 net/netlink/af_netlink.c:2454
> >>  rtnetlink_rcv+0x50/0x60 net/core/rtnetlink.c:4733
> >>  netlink_unicast_kernel net/netlink/af_netlink.c:1317 [inline]
> >>  netlink_unicast+0x1638/0x1720 net/netlink/af_netlink.c:1343
> >>  netlink_sendmsg+0x1205/0x1290 net/netlink/af_netlink.c:1908
> >>  sock_sendmsg_nosec net/socket.c:621 [inline]
> >>  sock_sendmsg net/socket.c:631 [inline]
> >> ...
> >> Uninit was created at:
> >> ...
> >>  slab_post_alloc_hook mm/slab.h:446 [inline]
> >>  slab_alloc_node mm/slub.c:2718 [inline]
> >>  __kmalloc_node_track_caller+0x9e7/0x1160 mm/slub.c:4351
> >>  __kmalloc_reserve net/core/skbuff.c:138 [inline]
> >>  __alloc_skb+0x2f5/0x9e0 net/core/skbuff.c:206
> >>  alloc_skb include/linux/skbuff.h:996 [inline]
> >>  netlink_alloc_large_skb net/netlink/af_netlink.c:1189 [inline]
> >>  netlink_sendmsg+0xb49/0x1290 net/netlink/af_netlink.c:1883
> >>  sock_sendmsg_nosec net/socket.c:621 [inline]
> >>  sock_sendmsg net/socket.c:631 [inline]
> >>  ___sys_sendmsg+0xe70/0x1290 net/socket.c:2114
> >> <<<
> >>
> >> This is a different bug. How come these 2 different reports for 2
> >> different
> >> bugs have ended in the same syzkaller report (bug?id=088efeac32fd) ?
> >
> > I suspect this is because syzbot used the top stack frame as the
> > report signature.
> > There's a mechanism to ignore frames like memcmp() in the reports, not
> > sure why didn't it work in this case (maybe it just wasn't in place at
> > the time the report happened).
> >> One bug is fixed by the "net: fix uninit-value in __hw_addr_add_ex()"
> >> commit,
> >> the second one is not, but they are still in the same syzkaller report.
> >>
> >> This was the reason of my confusion. I'm not sure how to fix this. If it
> >> is possible,
> >> probably we need to cancel/revoke "#syz fix: net: fix uninit-value in
> >> __hw_addr_add_ex()"
> >> for this syzkaller report (bug?id=088efeac32fd). And then "split" it into
> >> 2 or
> >> more different reports, but I'm not sure if this is possible.
> >>
> >> Probably, syzkaller needs to look deeper into the KMSAN reports to
> >> differentiate
> >> KMSAN errors happening because of different reasons.
> >>
> >> > On Sun, Sep 23, 2018 at 6:02 PM, Vladis Dronov <vdronov@redhat.com>
> >> > wrote:
> >> > > #syz fix: net: fix uninit-value in __hw_addr_add_ex()
> >> >
> >> > Hi Vladis,
> >> >
> >> > This can be fixed with "net: fix uninit-value in __hw_addr_add_ex()".
> >> > That commit landed in April, syzbot waited till the commit reached all
> >> > tested trees, and then closed the bug.
> >> > But the similar bug continued to happen, so syzbot created second
> >> > version of this bug (2). You can see on dashboard that the last crash
> >> > for the second version (2) happened just few days ago. So this is a
> >> > different bug.
> 
> 
> Precisely discriminating bugs (root causes) bases on crash text is
> generally undecidable problem, even for humans. We even can have
> literally equal crash texts, which are still different bugs. And we
> can have significantly differently looking crash texts, which are
> actually caused by the same root cause. syzbot extracts some
> "identity" string for each crash and than uses that string to
> discriminate crashes and sort them into bins. This identity string is
> what you see in email subject and bug title on dashboard. This method
> can have both false positives and false negatives, but works
> reasonably well in most cases and looks like the best practical
> option.
> 
> For this exact instance (memcmp) we actually improved the analysis
> logic recently:
> https://github.com/google/syzkaller/commit/0e29942f77715486995d996f80f82742812d75a2#diff-abe1515f011fad2659ff218f9eea9ae1
> But this crash was analyzed and reported before the change. So if this
> crash happens again it should be reported as "in __hw_addr_add_ex"
> now.
> 
> Re __hw_addr_add_ex bug, as Alex noted the crash was detected _after_
> the fixing commit went in. So it's something new and different and
> can't be fixed by the older commit.
> 
> There are no general, single guideline as to what to do when several
> different bugs glued together into a single bug. Fixing at least one
> of them (any) in the context of the bug is good, fixing both is good
> too. When/if a bug is closed, new occurrences of similar crashes (the
> same identity string) will lead to creation of a new bug. So if we fix
> only one and close the bug, eventually the second one will lead to a
> new bug (won't be lost), now dedicated to this second crash.
> 
> Now syzbot thinks that this bug is fixed/closed:
> https://syzkaller.appspot.com/bug?extid=d3402c47f680ff24b29c
> There is specifically no "undo" functionality, because it's inherently
> racy with creation of a new version of this bugs by new crashes. So if
> of these crashes will happen again, syzbot will open new bugs (now
> with better discriminated titles). We can wait for that. Or we can
> submit new fixes without waiting for new syzbot bugs (adding
> Reported-by to new commits referencing this bug should not do any
> harm).
> 
> Hope this clarifies things a bit.
> 
> Thanks

^ permalink raw reply

* [PATCH 2/2] MAINTAINERS: update files maintained under DPAA2 PTP/ETHERNET
From: Yangbo Lu @ 2018-09-27 11:12 UTC (permalink / raw)
  To: linux-kernel, devel, netdev, Richard Cochran, David S . Miller,
	Ioana Radulescu, Greg Kroah-Hartman
  Cc: Yangbo Lu
In-Reply-To: <20180927111228.46118-1-yangbo.lu@nxp.com>

The files maintained under DPAA2 PTP/ETHERNET needs to
be updated since dpaa2 ptp driver had been moved into
drivers/net/ethernet/freescale/dpaa2/.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
 MAINTAINERS |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 15565de..ba6f441 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4530,7 +4530,11 @@ DPAA2 ETHERNET DRIVER
 M:	Ioana Radulescu <ruxandra.radulescu@nxp.com>
 L:	netdev@vger.kernel.org
 S:	Maintained
-F:	drivers/net/ethernet/freescale/dpaa2
+F:	drivers/net/ethernet/freescale/dpaa2/dpaa2-eth*
+F:	drivers/net/ethernet/freescale/dpaa2/dpni*
+F:	drivers/net/ethernet/freescale/dpaa2/dpkg.h
+F:	drivers/net/ethernet/freescale/dpaa2/Makefile
+F:	drivers/net/ethernet/freescale/dpaa2/Kconfig
 
 DPAA2 ETHERNET SWITCH DRIVER
 M:	Ioana Radulescu <ruxandra.radulescu@nxp.com>
@@ -4541,9 +4545,10 @@ F:	drivers/staging/fsl-dpaa2/ethsw
 
 DPAA2 PTP CLOCK DRIVER
 M:	Yangbo Lu <yangbo.lu@nxp.com>
-L:	linux-kernel@vger.kernel.org
+L:	netdev@vger.kernel.org
 S:	Maintained
-F:	drivers/staging/fsl-dpaa2/rtc
+F:	drivers/net/ethernet/freescale/dpaa2/rtc*
+F:	drivers/net/ethernet/freescale/dpaa2/dprtc*
 
 DPT_I2O SCSI RAID DRIVER
 M:	Adaptec OEM Raid Solutions <aacraid@microsemi.com>
-- 
1.7.1

^ permalink raw reply related

* Re: [PATCH v2 0/3] Add can support to RZ/G2M
From: Marc Kleine-Budde @ 2018-09-27 10:45 UTC (permalink / raw)
  To: Fabrizio Castro, Wolfgang Grandegger, Rob Herring, Mark Rutland
  Cc: David S. Miller, Sergei Shtylyov, Simon Horman, Magnus Damm,
	Chris Paterson, linux-can, netdev, devicetree, linux-renesas-soc,
	linux-kernel, Geert Uytterhoeven, Biju Das
In-Reply-To: <1536576195-11520-1-git-send-email-fabrizio.castro@bp.renesas.com>


[-- Attachment #1.1: Type: text/plain, Size: 532 bytes --]

On 09/10/2018 12:43 PM, Fabrizio Castro wrote:
> Dear All,
> 
> this series contains all that's necessary to add CAN support
> for RZ/G2M (a.k.a. r8a774a1).
> 
> v1-v2:
> * applied Geert's comments

Added 1/3 and 2/3 to linux-can.

Tnx.
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH] hv_netvsc: Make sure out channel is fully opened on send
From: Mohammed Gamal @ 2018-09-27 10:31 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Haiyang Zhang, Stephen Hemminger, netdev@vger.kernel.org,
	otubo@redhat.com, linux-kernel@vger.kernel.org,
	devel@linuxdriverproject.org, vkuznets
In-Reply-To: <20180927122355.470df119@shemminger-XPS-13-9360>

On Thu, 2018-09-27 at 12:23 +0200, Stephen Hemminger wrote:
> On Thu, 27 Sep 2018 10:57:05 +0200
> Mohammed Gamal <mgamal@redhat.com> wrote:
> 
> > On Wed, 2018-09-26 at 17:13 +0000, Haiyang Zhang wrote:
> > > > -----Original Message-----
> > > > From: Mohammed Gamal <mgamal@redhat.com>
> > > > Sent: Wednesday, September 26, 2018 12:34 PM
> > > > To: Stephen Hemminger <sthemmin@microsoft.com>; netdev@vger.ker
> > > > nel.
> > > > org
> > > > Cc: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> > > > <haiyangz@microsoft.com>; vkuznets <vkuznets@redhat.com>;
> > > > otubo@redhat.com; cavery <cavery@redhat.com>; linux-
> > > > kernel@vger.kernel.org; devel@linuxdriverproject.org; Mohammed
> > > > Gamal
> > > > <mgamal@redhat.com>
> > > > Subject: [PATCH] hv_netvsc: Make sure out channel is fully
> > > > opened
> > > > on send
> > > > 
> > > > Dring high network traffic changes to network interface
> > > > parameters
> > > > such as
> > > > number of channels or MTU can cause a kernel panic with a NULL
> > > > pointer
> > > > dereference. This is due to netvsc_device_remove() being called
> > > > and
> > > > deallocating the channel ring buffers, which can then be
> > > > accessed
> > > > by
> > > > netvsc_send_pkt() before they're allocated on calling
> > > > netvsc_device_add()
> > > > 
> > > > The patch fixes this problem by checking the channel state and
> > > > returning
> > > > ENODEV if not yet opened. We also move the call to
> > > > hv_ringbuf_avail_percent()
> > > > which may access the uninitialized ring buffer.
> > > > 
> > > > Signed-off-by: Mohammed Gamal <mgamal@redhat.com>
> > > > ---
> > > >  drivers/net/hyperv/netvsc.c | 7 ++++++-
> > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/net/hyperv/netvsc.c
> > > > b/drivers/net/hyperv/netvsc.c index
> > > > fe01e14..75f1b31 100644
> > > > --- a/drivers/net/hyperv/netvsc.c
> > > > +++ b/drivers/net/hyperv/netvsc.c
> > > > @@ -825,7 +825,12 @@ static inline int netvsc_send_pkt(
> > > >  	struct netdev_queue *txq = netdev_get_tx_queue(ndev,
> > > > packet->q_idx);
> > > >  	u64 req_id;
> > > >  	int ret;
> > > > -	u32 ring_avail =
> > > > hv_get_avail_to_write_percent(&out_channel-  
> > > > > outbound);  
> > > > 
> > > > +	u32 ring_avail;
> > > > +
> > > > +	if (out_channel->state != CHANNEL_OPENED_STATE)
> > > > +		return -ENODEV;
> > > > +
> > > > +	ring_avail =
> > > > hv_get_avail_to_write_percent(&out_channel-  
> > > > > outbound);  
> > > 
> > > When you reproducing the NULL ptr panic, does your kernel include
> > > the
> > > following patch?
> > > hv_netvsc: common detach logic
> > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.g
> > > it/c
> > > ommit/?id=7b2ee50c0cd513a176a26a71f2989facdd75bfea
> > >   
> > 
> > Yes it is included. And the commit did reduce the occurrence of
> > this
> > race condition, but it still nevertheless occurs albeit rarely.
> > 
> > > We call netif_tx_disable(ndev) and netif_device_detach(ndev)
> > > before
> > > doing the changes 
> > > on MTU or #channels. So there should be no call to start_xmit()
> > > when
> > > channel is not ready.
> > > 
> > > If you see the check for CHANNEL_OPENED_STATE is still necessary
> > > on
> > > upstream kernel (including 
> > > the patch " common detach logic "), we should debug further on
> > > the
> > > code and find out the 
> > > root cause.
> > > 
> > > Thanks,
> > > - Haiyang
> > >   
> > 
> > _______________________________________________
> > devel mailing list
> > devel@linuxdriverproject.org
> > http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-
> > devel
> 
> Is there some workload, that can be used to reproduce this?
> The stress test from Vitaly with changing parameters while running
> network traffic
> passes now.
> 
> Can you reproduce this with the upstream current kernel?
> 
> Adding the check in start xmit is still racy, and won't cure the
> problem.
> 
> Another solution would be to add a grace period in the netvsc detach
> logic.
> 

Steps to reproduce are listed here:
https://bugzilla.redhat.com/show_bug.cgi?id=1632653

We've also managed to reproduce the same issue upstream. It's more
likely to be reproduced on Windows 2012R2 than 2016.

Regards,
Mohammed

^ permalink raw reply

* Re: [PATCH] hv_netvsc: Make sure out channel is fully opened on send
From: Stephen Hemminger @ 2018-09-27 10:23 UTC (permalink / raw)
  To: Mohammed Gamal
  Cc: Haiyang Zhang, Stephen Hemminger, netdev@vger.kernel.org,
	otubo@redhat.com, linux-kernel@vger.kernel.org,
	devel@linuxdriverproject.org, vkuznets
In-Reply-To: <1538038625.19334.2.camel@redhat.com>

On Thu, 27 Sep 2018 10:57:05 +0200
Mohammed Gamal <mgamal@redhat.com> wrote:

> On Wed, 2018-09-26 at 17:13 +0000, Haiyang Zhang wrote:
> > > -----Original Message-----
> > > From: Mohammed Gamal <mgamal@redhat.com>
> > > Sent: Wednesday, September 26, 2018 12:34 PM
> > > To: Stephen Hemminger <sthemmin@microsoft.com>; netdev@vger.kernel.
> > > org
> > > Cc: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> > > <haiyangz@microsoft.com>; vkuznets <vkuznets@redhat.com>;
> > > otubo@redhat.com; cavery <cavery@redhat.com>; linux-
> > > kernel@vger.kernel.org; devel@linuxdriverproject.org; Mohammed
> > > Gamal
> > > <mgamal@redhat.com>
> > > Subject: [PATCH] hv_netvsc: Make sure out channel is fully opened
> > > on send
> > > 
> > > Dring high network traffic changes to network interface parameters
> > > such as
> > > number of channels or MTU can cause a kernel panic with a NULL
> > > pointer
> > > dereference. This is due to netvsc_device_remove() being called and
> > > deallocating the channel ring buffers, which can then be accessed
> > > by
> > > netvsc_send_pkt() before they're allocated on calling
> > > netvsc_device_add()
> > > 
> > > The patch fixes this problem by checking the channel state and
> > > returning
> > > ENODEV if not yet opened. We also move the call to
> > > hv_ringbuf_avail_percent()
> > > which may access the uninitialized ring buffer.
> > > 
> > > Signed-off-by: Mohammed Gamal <mgamal@redhat.com>
> > > ---
> > >  drivers/net/hyperv/netvsc.c | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/net/hyperv/netvsc.c
> > > b/drivers/net/hyperv/netvsc.c index
> > > fe01e14..75f1b31 100644
> > > --- a/drivers/net/hyperv/netvsc.c
> > > +++ b/drivers/net/hyperv/netvsc.c
> > > @@ -825,7 +825,12 @@ static inline int netvsc_send_pkt(
> > >  	struct netdev_queue *txq = netdev_get_tx_queue(ndev,
> > > packet->q_idx);
> > >  	u64 req_id;
> > >  	int ret;
> > > -	u32 ring_avail =
> > > hv_get_avail_to_write_percent(&out_channel-  
> > > > outbound);  
> > > 
> > > +	u32 ring_avail;
> > > +
> > > +	if (out_channel->state != CHANNEL_OPENED_STATE)
> > > +		return -ENODEV;
> > > +
> > > +	ring_avail = hv_get_avail_to_write_percent(&out_channel-  
> > > >outbound);  
> > 
> > When you reproducing the NULL ptr panic, does your kernel include the
> > following patch?
> > hv_netvsc: common detach logic
> > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/c
> > ommit/?id=7b2ee50c0cd513a176a26a71f2989facdd75bfea
> >   
> Yes it is included. And the commit did reduce the occurrence of this
> race condition, but it still nevertheless occurs albeit rarely.
> 
> > We call netif_tx_disable(ndev) and netif_device_detach(ndev) before
> > doing the changes 
> > on MTU or #channels. So there should be no call to start_xmit() when
> > channel is not ready.
> > 
> > If you see the check for CHANNEL_OPENED_STATE is still necessary on
> > upstream kernel (including 
> > the patch " common detach logic "), we should debug further on the
> > code and find out the 
> > root cause.
> > 
> > Thanks,
> > - Haiyang
> >   
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Is there some workload, that can be used to reproduce this?
The stress test from Vitaly with changing parameters while running network traffic
passes now.

Can you reproduce this with the upstream current kernel?

Adding the check in start xmit is still racy, and won't cure the problem.

Another solution would be to add a grace period in the netvsc detach logic.

^ permalink raw reply

* Re: [PATCH] net/ncsi: Add NCSI OEM command for FB Tiogapass
From: Samuel Mendoza-Jonas @ 2018-09-27  3:54 UTC (permalink / raw)
  To: Vijay Khemka, linux-aspeed @ lists . ozlabs . org,
	openbmc @ lists . ozlabs . org, Sai Dasari, Amithash Prasad,
	Justin.Lee1, netdev@vger.kernel.org
In-Reply-To: <81718e3fd6c883917aca540c032a7065fd00e79a.camel@mendozajonas.com>

On Thu, 2018-09-27 at 13:43 +1000, Samuel Mendoza-Jonas wrote:
> On Mon, 2018-09-24 at 17:08 -0700, Vijay Khemka wrote:
> > This patch adds OEM command to get mac address from NCSI device and and
> > configure the same to the network card.
> > 
> > ncsi_cmd_arg - Modified this structure to include bigger payload data.
> > ncsi_cmd_handler_oem: This function handles oem command request
> > ncsi_rsp_handler_oem: This function handles response for OEM command.
> > get_mac_address_oem_mlx: This function will send OEM command to get
> > mac address for Mellanox card
> > set_mac_affinity_mlx: This will send OEM command to set Mac affinity
> > for Mellanox card
> > 
> > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> 
> Hi Vijay,
> 
> Having had a chance to take a closer look, there is probably room for
> both this patchset and Justin's potential changes to coexist; while
> Justin's is a more general solution for sending arbitrary commands, the
> approach of this patch is also useful for handling commands we want
> included in the configure process (such as get-mac-address).
> 
> Some comments below:

Whoops, forgot to re-add netdev.

> 
> > ---
> >  net/ncsi/Kconfig       |  3 ++
> >  net/ncsi/internal.h    | 11 +++++--
> >  net/ncsi/ncsi-cmd.c    | 24 +++++++++++++--
> >  net/ncsi/ncsi-manage.c | 68 ++++++++++++++++++++++++++++++++++++++++++
> >  net/ncsi/ncsi-pkt.h    | 16 ++++++++++
> >  net/ncsi/ncsi-rsp.c    | 33 +++++++++++++++++++-
> >  6 files changed, 149 insertions(+), 6 deletions(-)
> > 
> > diff --git a/net/ncsi/Kconfig b/net/ncsi/Kconfig
> > index 08a8a6031fd7..b8bf89fea7c8 100644
> > --- a/net/ncsi/Kconfig
> > +++ b/net/ncsi/Kconfig
> > @@ -10,3 +10,6 @@ config NET_NCSI
> >  	  support. Enable this only if your system connects to a network
> >  	  device via NCSI and the ethernet driver you're using supports
> >  	  the protocol explicitly.
> > +config NCSI_OEM_CMD_GET_MAC
> > +	bool "Get NCSI OEM MAC Address"
> > +	depends on NET_NCSI
> 
> For the moment this isn't too bad but I wonder if in the future it would
> be more flexible to have something like NCSI_OEM_CMD_MELLANOX etc, so we
> could selectively enable a class of OEM commands based on vendor rather
> than per-command.
> 
> > diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
> > index 8055e3965cef..da17958e6a4b 100644
> > --- a/net/ncsi/internal.h
> > +++ b/net/ncsi/internal.h
> > @@ -68,6 +68,10 @@ enum {
> >  	NCSI_MODE_MAX
> >  };
> >  
> > +#define NCSI_OEM_MFR_MLX_ID             0x8119
> > +#define NCSI_OEM_MLX_CMD_GET_MAC        0x1b00
> > +#define NCSI_OEM_MLX_CMD_SET_AFFINITY   0x010700
> 
> I gather this is part of the OEM command but it would be good to describe
> what these bits mean. Is this command documented anywhere by Mellanox?
> 
> > +
> >  struct ncsi_channel_version {
> >  	u32 version;		/* Supported BCD encoded NCSI version */
> >  	u32 alpha2;		/* Supported BCD encoded NCSI version */
> > @@ -236,6 +240,7 @@ enum {
> >  	ncsi_dev_state_probe_dp,
> >  	ncsi_dev_state_config_sp	= 0x0301,
> >  	ncsi_dev_state_config_cis,
> > +	ncsi_dev_state_config_oem_gma,
> >  	ncsi_dev_state_config_clear_vids,
> >  	ncsi_dev_state_config_svf,
> >  	ncsi_dev_state_config_ev,
> > @@ -301,9 +306,9 @@ struct ncsi_cmd_arg {
> >  	unsigned short       payload;     /* Command packet payload length */
> >  	unsigned int         req_flags;   /* NCSI request properties       */
> >  	union {
> > -		unsigned char  bytes[16]; /* Command packet specific data  */
> > -		unsigned short words[8];
> > -		unsigned int   dwords[4];
> > +		unsigned char  bytes[64]; /* Command packet specific data  */
> > +		unsigned short words[32];
> > +		unsigned int   dwords[16];
> >  	};
> >  };
> >  
> > diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
> > index 7567ca63aae2..3205e22c1734 100644
> > --- a/net/ncsi/ncsi-cmd.c
> > +++ b/net/ncsi/ncsi-cmd.c
> > @@ -211,6 +211,25 @@ static int ncsi_cmd_handler_snfc(struct sk_buff *skb,
> >  	return 0;
> >  }
> >  
> > +static int ncsi_cmd_handler_oem(struct sk_buff *skb,
> > +				struct ncsi_cmd_arg *nca)
> > +{
> > +	struct ncsi_cmd_oem_pkt *cmd;
> > +	unsigned int len;
> > +
> > +	len = sizeof(struct ncsi_cmd_pkt_hdr) + 4;
> > +	if (nca->payload < 26)
> > +		len += 26;
> 
> This will have already happened in ncsi_alloc_command(), is this check
> needed?
> 
> > +	else
> > +		len += nca->payload;
> > +
> > +	cmd = skb_put_zero(skb, len);
> > +	memcpy(cmd->data, nca->bytes, nca->payload);
> > +	ncsi_cmd_build_header(&cmd->cmd.common, nca);
> > +
> > +	return 0;
> > +}
> > +
> >  static struct ncsi_cmd_handler {
> >  	unsigned char type;
> >  	int           payload;
> > @@ -244,7 +263,7 @@ static struct ncsi_cmd_handler {
> >  	{ NCSI_PKT_CMD_GNS,    0, ncsi_cmd_handler_default },
> >  	{ NCSI_PKT_CMD_GNPTS,  0, ncsi_cmd_handler_default },
> >  	{ NCSI_PKT_CMD_GPS,    0, ncsi_cmd_handler_default },
> > -	{ NCSI_PKT_CMD_OEM,    0, NULL                     },
> > +	{ NCSI_PKT_CMD_OEM,   -1, ncsi_cmd_handler_oem     },
> >  	{ NCSI_PKT_CMD_PLDM,   0, NULL                     },
> >  	{ NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default }
> >  };
> > @@ -317,7 +336,8 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca)
> >  	}
> >  
> >  	/* Get packet payload length and allocate the request */
> > -	nca->payload = nch->payload;
> > +	if (nch->payload >= 0)
> > +		nca->payload = nch->payload;
> 
> I think with this there is a chance of nca->payload being uninitialised
> and then used in ncsi_alloc_command(). Can you describe what the aim here
> is?
> 
> >  	nr = ncsi_alloc_command(nca);
> >  	if (!nr)
> >  		return -ENOMEM;
> > diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
> > index 091284760d21..3b2b86560cc8 100644
> > --- a/net/ncsi/ncsi-manage.c
> > +++ b/net/ncsi/ncsi-manage.c
> > @@ -635,6 +635,58 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
> >  	return 0;
> >  }
> >  
> > +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
> > +/* NCSI Facebook OEM APIs */
> 
> Are these Facebook OEM commands or Mellanox OEM commands?
> 
> > +static void get_mac_address_oem_mlx(struct ncsi_dev_priv *ndp)
> > +{
> > +	struct ncsi_cmd_arg nca;
> > +	int ret = 0;
> > +
> > +	memset(&nca, 0, sizeof(struct ncsi_cmd_arg));
> > +	nca.ndp = ndp;
> > +	nca.channel = ndp->active_channel->id;
> > +	nca.package = ndp->active_package->id;
> > +	nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN;
> > +	nca.type = NCSI_PKT_CMD_OEM;
> > +	nca.payload = 8;
> > +
> > +	nca.dwords[0] = ntohl(NCSI_OEM_MFR_MLX_ID);
> > +	nca.dwords[1] = ntohl(NCSI_OEM_MLX_CMD_GET_MAC);
> > +
> > +	ret = ncsi_xmit_cmd(&nca);
> > +	if (ret)
> > +		netdev_err(ndp->ndev.dev,
> > +			   "NCSI: Failed to transmit cmd 0x%x during probe\n",
> > +			   nca.type);
> > +}
> > +
> > +static void set_mac_affinity_mlx(struct ncsi_dev_priv *ndp)
> > +{
> > +	struct ncsi_cmd_arg nca;
> > +	int ret = 0;
> > +
> > +	memset(&nca, 0, sizeof(struct ncsi_cmd_arg));
> 
> Ah I see we initialise nca, and thus payload here - the path between
> these two points in the driver isn't super obvious (not this patch's
> fault :) ), it might be better to explicitly mention/handle this where we
> use payload later on.
> 
> > +	nca.ndp = ndp;
> > +	nca.channel = ndp->active_channel->id;
> > +	nca.package = ndp->active_package->id;
> 
> These should probably be set in ncsi_configure_channel (eg. see the CIS
> state before these are called).
> 
> > +	nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN;
> > +	nca.type = NCSI_PKT_CMD_OEM;
> > +	nca.payload = 60;
> > +
> > +	nca.dwords[0] = ntohl(NCSI_OEM_MFR_MLX_ID);
> > +	nca.dwords[1] = ntohl(NCSI_OEM_MLX_CMD_SET_AFFINITY);
> > +
> > +	memcpy(&(nca.bytes[8]), ndp->ndev.dev->dev_addr, ETH_ALEN);
> > +	nca.bytes[14] = 0x09;
> > +
> > +	ret = ncsi_xmit_cmd(&nca);
> > +	if (ret)
> > +		netdev_err(ndp->ndev.dev,
> > +			   "NCSI: Failed to transmit cmd 0x%x during probe\n",
> > +				   nca.type);
> > +}
> > +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
> > +
> >  static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> >  {
> >  	struct ncsi_dev *nd = &ndp->ndev;
> > @@ -685,6 +737,22 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> >  			goto error;
> >  		}
> >  
> > +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
> > +		/* Check Manufacture id if it is Mellanox then
> > +		 * get and set mac address. To Do: Add code for
> > +		 * other types of card if required
> > +		 */
> > +		if (nc->version.mf_id == NCSI_OEM_MFR_MLX_ID)
> > +			nd->state = ncsi_dev_state_config_oem_gma;
> > +		else
> > +			nd->state = ncsi_dev_state_config_clear_vids;
> > +		break;
> > +	case ncsi_dev_state_config_oem_gma:
> > +		ndp->pending_req_num = 2;
> > +		get_mac_address_oem_mlx(ndp);
> > +		msleep(500);
> 
> Is this msleep() required?
> 
> > +		set_mac_affinity_mlx(ndp);
> 
> Since this *sets* the MAC address, should we name the state and config
> option more accurately? How does this OEM command interact with the NCSI
> set-mac-address command?
> Does this command depend on the previous get_mac_address_oem_mlx()
> command succeeding?
> 
> > +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
> >  		nd->state = ncsi_dev_state_config_clear_vids;
> >  		break;
> >  	case ncsi_dev_state_config_clear_vids:
> > diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
> > index 91b4b66438df..0653a893eb12 100644
> > --- a/net/ncsi/ncsi-pkt.h
> > +++ b/net/ncsi/ncsi-pkt.h
> > @@ -151,6 +151,22 @@ struct ncsi_cmd_snfc_pkt {
> >  	unsigned char           pad[22];
> >  };
> >  
> > +/* Oem Request Command */
> 
> In general, s/Oem/OEM
> 
> > +struct ncsi_cmd_oem_pkt {
> > +	struct ncsi_cmd_pkt_hdr cmd;         /* Command header    */
> > +	unsigned char           data[64];    /* OEM Payload Data  */
> > +	__be32                  checksum;    /* Checksum          */
> > +};
> > +
> > +/* Oem Response Packet */
> > +struct ncsi_rsp_oem_pkt {
> > +	struct ncsi_rsp_pkt_hdr rsp;         /* Command header    */
> > +	__be32                  mfr_id;      /* Manufacture ID    */
> > +	__be32                  oem_cmd;     /* oem command       */
> > +	unsigned char           data[32];    /* Payload data      */
> > +	__be32                  checksum;    /* Checksum          */
> > +};
> > +
> >  /* Get Link Status */
> >  struct ncsi_rsp_gls_pkt {
> >  	struct ncsi_rsp_pkt_hdr rsp;        /* Response header   */
> > diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
> > index 930c1d3796f0..3b94c96b9c7f 100644
> > --- a/net/ncsi/ncsi-rsp.c
> > +++ b/net/ncsi/ncsi-rsp.c
> > @@ -596,6 +596,37 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr)
> >  	return 0;
> >  }
> >  
> > +static int ncsi_rsp_handler_oem(struct ncsi_request *nr)
> > +{
> > +	struct ncsi_rsp_oem_pkt *rsp;
> > +	struct ncsi_dev_priv *ndp = nr->ndp;
> > +	struct net_device *ndev = ndp->ndev.dev;
> > +	int ret = 0;
> > +	unsigned int oem_cmd, mfr_id;
> > +	const struct net_device_ops *ops = ndev->netdev_ops;
> > +	struct sockaddr saddr;
> > +
> > +	/* Get the response header */
> > +	rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
> > +
> > +	oem_cmd = ntohl(rsp->oem_cmd);
> > +	mfr_id = ntohl(rsp->mfr_id);
> > +
> > +	/* Check for Mellanox manufacturer id */
> > +	if (mfr_id != NCSI_OEM_MFR_MLX_ID)
> > +		return 0;
> > +
> > +	if (oem_cmd == NCSI_OEM_MLX_CMD_GET_MAC) {
> > +		saddr.sa_family = ndev->type;
> > +		ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
> > +		memcpy(saddr.sa_data, &(rsp->data[4]), ETH_ALEN);
> > +		ret = ops->ndo_set_mac_address(ndev, &saddr);
> > +		if (ret < 0)
> > +			netdev_warn(ndev, "NCSI: 'Writing mac address to device failed\n");
> > +	}
> > +	return ret;
> > +}
> > +
> >  static int ncsi_rsp_handler_gvi(struct ncsi_request *nr)
> >  {
> >  	struct ncsi_rsp_gvi_pkt *rsp;
> > @@ -932,7 +963,7 @@ static struct ncsi_rsp_handler {
> >  	{ NCSI_PKT_RSP_GNS,   172, ncsi_rsp_handler_gns     },
> >  	{ NCSI_PKT_RSP_GNPTS, 172, ncsi_rsp_handler_gnpts   },
> >  	{ NCSI_PKT_RSP_GPS,     8, ncsi_rsp_handler_gps     },
> > -	{ NCSI_PKT_RSP_OEM,     0, NULL                     },
> > +	{ NCSI_PKT_RSP_OEM,    -1, ncsi_rsp_handler_oem     },
> >  	{ NCSI_PKT_RSP_PLDM,    0, NULL                     },
> >  	{ NCSI_PKT_RSP_GPUUID, 20, ncsi_rsp_handler_gpuuid  }
> >  };

^ permalink raw reply

* Re: [PATCH net-next v2] wireless-drivers: rtnetlink wifi simulation device
From: Johannes Berg @ 2018-09-27 10:10 UTC (permalink / raw)
  To: Cody Schuffelen
  Cc: Kalle Valo, David S . Miller, linux-kernel, linux-wireless,
	netdev, kernel-team
In-Reply-To: <20180926194324.71290-1-schuffelen@google.com>

On Wed, 2018-09-26 at 12:43 -0700, Cody Schuffelen wrote:

> ip link set eth0 down
> ip link set eth0 name buried_eth0
> ip link set buried_eth0 up
> ip link add link buried_eth0 name wlan0 type virt_wifi
> 
> eth0 is renamed to buried_eth0 to avoid a network manager trying to
> manage it, 

I feel you should remove this "buried" thing at least from the plain
instructions - first of all, I'm not convinced it actually *works* in
general (network managers are able to find it anyway, right? perhaps
android's has some rules?), and it's not really necessary from a kernel
POV.

Perhaps add a note like "you may have to rename or otherwise hide the
eth0 from your connection manager" (also "NetworkManager" is a specific
software, so "network manager" is confusing - I actually first thought
you meant NM).

Anyway, just a thought.

> Thanks for the detailed review! I believe I've addressed all comments.

Thanks!

> The problem we've experienced with routing the hwsim virtual medium from the
> inside the VM to outside the VM is this required running hwsim on the host
> kernel as well. This is too intrusive for our use-case, whereas wifi that
> exists completely inside the VM kernel is much more palatable.

Fair enough. I still think it'd be worthwhile in the long run because
then you could start multiple APs to simulate roaming etc. without
having to implement all of this here.

IOW - I don't think we should extend this much. As is, I think it's
fine, but I wouldn't want to see extensions like "have two scan result",
"let userspace control the RSSI of the scan results to force 'roaming'"
etc.

> +static struct ieee80211_channel channel_2ghz = {
> +	.band = NL80211_BAND_2GHZ,
> +	.center_freq = 5500,
> +	.hw_value = 5500,

That doesn't seem right - a 2 GHz channel that's really 5.5 GHz?

> +	.max_power = 5500,

That seems more like a copy/paste bug rather than intentional?

> +static struct ieee80211_rate bitrates_2ghz[] = {
> +	{
> +		.bitrate = 10,
> +	}, {
> +		.bitrate = 20,
> +	}, {
> +		.bitrate = 55,
> +	}, {
> +		.bitrate = 60,
> +	}, {
> +		.bitrate = 110,
> +	}, {
> +		.bitrate = 120,
> +	}, {
> +		.bitrate = 240,
> +	},
> +};

That's ... strangely formatted? I guess we'd usually write

	{ .bitrate = 10 },
	{ .bitrate = 20 },
	...



> +static struct ieee80211_supported_band band_2ghz = {
> +	.channels = &channel_2ghz,
> +	.bitrates = bitrates_2ghz,
> +	.band = NL80211_BAND_2GHZ,
> +	.n_channels = 1,
> +	.n_bitrates = 7,

Please use ARRAY_SIZE()

> +	.ht_cap = {
> +		.ht_supported = true,
> +	},
> +	.vht_cap = {
> +		.vht_supported = true,
> +	},

That looks _really_ bare - you may want to copy something sane to here.

> +};
> +
> +static struct ieee80211_channel channel_5ghz = {

> +	.max_power = 5500,

same here - max_power 5500?

> +	.max_reg_power = 9999,
> +};
> +
> +static struct ieee80211_rate bitrates_5ghz[] = {
> +	{
> +		.bitrate = 60,
> +	}, {
> +		.bitrate = 120,
> +	}, {
> +		.bitrate = 240,
> +	},
> +};

Same comment here regarding formatting.

> +static struct ieee80211_supported_band band_5ghz = {
> +	.channels = &channel_5ghz,
> +	.bitrates = bitrates_5ghz,
> +	.band = NL80211_BAND_5GHZ,
> +	.n_channels = 1,
> +	.n_bitrates = 3,

and ARRAY_SIZE

> +	.ht_cap = {
> +		.ht_supported = true,
> +	},
> +	.vht_cap = {
> +		.vht_supported = true,
> +	},

and HT/VHT

> +/** Assigned at module init. Guaranteed locally-administered and unicast. */
> +static u8 fake_router_bssid[ETH_ALEN] = {};

Maybe make that __ro_after_init?

It doesn't matter that much, but is a good signal.

> +static void virt_wifi_scan_result(struct work_struct *work)
> +{
> +	char ssid[] = "__VirtWifi";
> +	struct cfg80211_bss *informed_bss;
> +	struct virt_wifi_priv *priv =
> +		container_of(work, struct virt_wifi_priv,
> +			     scan_result.work);
> +	struct wiphy *wiphy = priv_to_wiphy(priv);
> +	struct cfg80211_inform_bss mock_inform_bss = {
> +		.chan = &channel_5ghz,
> +		.scan_width = NL80211_BSS_CHAN_WIDTH_20,
> +		.signal = -60,
> +		.boottime_ns = ktime_get_boot_ns(),
> +	};
> +
> +	ssid[0] = WLAN_EID_SSID;
> +	/* size of the array minus null terminator, length byte, tag byte */
> +	ssid[1] = sizeof(ssid) - 3;

Seems like you could make that const?
	struct {
		u8 tag;
		u8 len;
		u8 ssid[8];
	} ssid = { .tag = 0, .len = 8, .ssid = "VirtWifi" };

or something like that?

Hmm, maybe that doesn't work.. whatever, not really important.

> +	informed_bss = cfg80211_inform_bss_data(wiphy, &mock_inform_bss,
> +						CFG80211_BSS_FTYPE_PRESP,
> +						fake_router_bssid,
> +						mock_inform_bss.boottime_ns,
> +						WLAN_CAPABILITY_ESS, 0, ssid,
> +						/* Truncate before the null. */
> +						sizeof(ssid) - 1, GFP_KERNEL);
> +	cfg80211_put_bss(wiphy, informed_bss);
> +
> +	informed_bss = cfg80211_inform_bss_data(wiphy, &mock_inform_bss,
> +						CFG80211_BSS_FTYPE_BEACON,
> +						fake_router_bssid,
> +						mock_inform_bss.boottime_ns,
> +						WLAN_CAPABILITY_ESS, 0, ssid,
> +						/* Truncate before the null. */
> +						sizeof(ssid) - 1, GFP_KERNEL);
> +	cfg80211_put_bss(wiphy, informed_bss);

Hmm, what's the point of doing it twice? You don't really need to
receive both PRESP/BEACON, just one is sufficient.

> +	schedule_delayed_work(&priv->scan_complete, HZ * 2);
> +}

I don't think you need to make that async again.

> +static int virt_wifi_connect(struct wiphy *wiphy, struct net_device *netdev,
> +			     struct cfg80211_connect_params *sme)
> +{
> +	struct virt_wifi_priv *priv = wiphy_priv(wiphy);
> +	bool could_schedule;
> +
> +	if (priv->being_deleted)
> +		return -EBUSY;
> +
> +	if (sme->bssid && !ether_addr_equal(sme->bssid, fake_router_bssid))
> +		return -EINVAL;

If you wanted to be more "real", you'd accept this and then check it in
the connect worker, rejecting it there if mismatched.

> +static int virt_wifi_get_station(struct wiphy *wiphy, struct net_device *dev,
> +				 const u8 *mac, struct station_info *sinfo)
> +{
> +	wiphy_debug(wiphy, "get_station\n");
> +	sinfo->filled = BIT(NL80211_STA_INFO_TX_PACKETS) |
> +		BIT(NL80211_STA_INFO_TX_FAILED) | BIT(NL80211_STA_INFO_SIGNAL) |
> +		BIT(NL80211_STA_INFO_TX_BITRATE);
> +	sinfo->tx_packets = 1;
> +	sinfo->tx_failed = 0;
> +	sinfo->signal = -60;
> +	sinfo->txrate = (struct rate_info) {
> +		.legacy = 10, /* units are 100kbit/s */
> +	};
> +	return 0;
> +}

You should check that mac is fake_router_bssid, and otherwise return not
found (-ENOENT), IIRC.

> +static netdev_tx_t virt_wifi_start_xmit(struct sk_buff *skb,
> +					struct net_device *dev)
> +{
> +	struct virt_wifi_netdev_priv *priv = netdev_priv(dev);
> +	struct virt_wifi_priv *w_priv = wiphy_priv(dev->ieee80211_ptr->wiphy);
> +
> +	if (!w_priv->is_connected)
> +		return NETDEV_TX_BUSY;

I think you should just drop the frame.

> +/* Called under rcu_read_lock() from netif_receive_skb */
> +static rx_handler_result_t virt_wifi_rx_handler(struct sk_buff **pskb)

FWIW, the stuff beyond this point, especially the netlink, I'm less
familiar with.

> +/** Called with rtnl lock held. */

/** is usually used only for kernel-doc

johannes

^ permalink raw reply

* Re: [PATCH net v2] bnxt_en: Fix TX timeout during netpoll.
From: Eric Dumazet @ 2018-09-27  3:50 UTC (permalink / raw)
  To: Song Liu; +Cc: David Miller, Michael Chan, netdev
In-Reply-To: <436AA90E-83A6-45A0-80B6-9ED843EE254E@fb.com>

On Wed, Sep 26, 2018 at 8:49 PM Song Liu <songliubraving@fb.com> wrote:
>

> We also need this patch from Eric:
>
> https://marc.info/?l=linux-netdev&m=153780304905946
>


I will submit this formally tomorrow, thanks.

^ permalink raw reply

* Re: [PATCH net v2] bnxt_en: Fix TX timeout during netpoll.
From: Song Liu @ 2018-09-27  3:49 UTC (permalink / raw)
  To: David Miller
  Cc: michael.chan@broadcom.com, edumazet@google.com,
	netdev@vger.kernel.org
In-Reply-To: <20180926.203304.405531282068457495.davem@davemloft.net>



> On Sep 26, 2018, at 8:33 PM, David Miller <davem@davemloft.net> wrote:
> 
> From: Michael Chan <michael.chan@broadcom.com>
> Date: Wed, 26 Sep 2018 00:41:04 -0400
> 
>> The current netpoll implementation in the bnxt_en driver has problems
>> that may miss TX completion events.  bnxt_poll_work() in effect is
>> only handling at most 1 TX packet before exiting.  In addition,
>> there may be in flight TX completions that ->poll() may miss even
>> after we fix bnxt_poll_work() to handle all visible TX completions.
>> netpoll may not call ->poll() again and HW may not generate IRQ
>> because the driver does not ARM the IRQ when the budget (0 for netpoll)
>> is reached.
>> 
>> We fix it by handling all TX completions and to always ARM the IRQ
>> when we exit ->poll() with 0 budget.
>> 
>> Also, the logic to ACK the completion ring in case it is almost filled
>> with TX completions need to be adjusted to take care of the 0 budget
>> case, as discussed with Eric Dumazet <edumazet@google.com>
>> 
>> Reported-by: Song Liu <songliubraving@fb.com>
>> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
> 
> Applied and queued up for -stable, thanks Michael.

Hi David,

We also need this patch from Eric:

https://marc.info/?l=linux-netdev&m=153780304905946

Thanks,
Song

^ permalink raw reply

* [PATCH net-next] net: dsa: b53: Fix build with B53_SRAB enabled and B53_SERDES=m
From: Arnd Bergmann @ 2018-09-27 10:02 UTC (permalink / raw)
  To: Florian Fainelli, Andrew Lunn, Vivien Didelot, David S. Miller
  Cc: Arnd Bergmann, netdev, linux-kernel

When B53_SERDES is a loadable module, a built-in srab driver still
cannot reach it, so the previous fix is incomplete:

b53_srab.c:(.text+0x3f4): undefined reference to `b53_serdes_init'
drivers/net/dsa/b53/b53_srab.o:(.rodata+0xe64): undefined reference to `b53_serdes_link_state'
drivers/net/dsa/b53/b53_srab.o:(.rodata+0xe74): undefined reference to `b53_serdes_link_set'
drivers/net/dsa/b53/b53_srab.o:(.rodata+0xe88): undefined reference to `b53_serdes_an_restart'
drivers/net/dsa/b53/b53_srab.o:(.rodata+0xea0): undefined reference to `b53_serdes_phylink_validate'
drivers/net/dsa/b53/b53_srab.o:(.rodata+0xea4): undefined reference to `b53_serdes_config'

Add a Kconfig dependency that forces srab to also be a module
in this case, but allow it to be built-in when serdes is
disabled or built-in.

Fixes: 7a8c7f5c30f9 ("net: dsa: b53: Fix build with B53_SRAB enabled and not B53_SERDES")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/dsa/b53/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/dsa/b53/Kconfig b/drivers/net/dsa/b53/Kconfig
index e83ebfafd881..d32469283f97 100644
--- a/drivers/net/dsa/b53/Kconfig
+++ b/drivers/net/dsa/b53/Kconfig
@@ -31,6 +31,7 @@ config B53_MMAP_DRIVER
 config B53_SRAB_DRIVER
 	tristate "B53 SRAB connected switch driver"
 	depends on B53 && HAS_IOMEM
+	depends on B53_SERDES || !B53_SERDES
 	default ARCH_BCM_IPROC
 	help
 	  Select to enable support for memory-mapped Switch Register Access
-- 
2.18.0

^ permalink raw reply related

* Re: [PATCH] net-tcp: /proc/sys/net/ipv4/tcp_probe_interval is a u32 not int
From: David Miller @ 2018-09-27  3:34 UTC (permalink / raw)
  To: zenczykowski; +Cc: maze, edumazet, netdev
In-Reply-To: <20180926045928.94911-1-zenczykowski@gmail.com>

From: "Maciej Żenczykowski" <zenczykowski@gmail.com>
Date: Tue, 25 Sep 2018 21:59:28 -0700

> From: Maciej Żenczykowski <maze@google.com>
> 
> (fix documentation and sysctl access to treat it as such)
> 
> Tested:
>   # zcat /proc/config.gz | egrep ^CONFIG_HZ
>   CONFIG_HZ_1000=y
>   CONFIG_HZ=1000
>   # echo $[(1<<32)/1000 + 1] | tee /proc/sys/net/ipv4/tcp_probe_interval
>   4294968
>   tee: /proc/sys/net/ipv4/tcp_probe_interval: Invalid argument
>   # echo $[(1<<32)/1000] | tee /proc/sys/net/ipv4/tcp_probe_interval
>   4294967
>   # echo 0 | tee /proc/sys/net/ipv4/tcp_probe_interval
>   # echo -1 | tee /proc/sys/net/ipv4/tcp_probe_interval
>   -1
>   tee: /proc/sys/net/ipv4/tcp_probe_interval: Invalid argument
> 
> Signed-off-by: Maciej Żenczykowski <maze@google.com>

Applied.

^ permalink raw reply

* Re: [PATCH net v2] bnxt_en: Fix TX timeout during netpoll.
From: David Miller @ 2018-09-27  3:33 UTC (permalink / raw)
  To: michael.chan; +Cc: songliubraving, edumazet, netdev
In-Reply-To: <1537936864-23472-1-git-send-email-michael.chan@broadcom.com>

From: Michael Chan <michael.chan@broadcom.com>
Date: Wed, 26 Sep 2018 00:41:04 -0400

> The current netpoll implementation in the bnxt_en driver has problems
> that may miss TX completion events.  bnxt_poll_work() in effect is
> only handling at most 1 TX packet before exiting.  In addition,
> there may be in flight TX completions that ->poll() may miss even
> after we fix bnxt_poll_work() to handle all visible TX completions.
> netpoll may not call ->poll() again and HW may not generate IRQ
> because the driver does not ARM the IRQ when the budget (0 for netpoll)
> is reached.
> 
> We fix it by handling all TX completions and to always ARM the IRQ
> when we exit ->poll() with 0 budget.
> 
> Also, the logic to ACK the completion ring in case it is almost filled
> with TX completions need to be adjusted to take care of the 0 budget
> case, as discussed with Eric Dumazet <edumazet@google.com>
> 
> Reported-by: Song Liu <songliubraving@fb.com>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>

Applied and queued up for -stable, thanks Michael.

^ permalink raw reply

* Re: [PATCH 2/2] net-ipv4: remove 2 always zero parameters from ipv4_redirect()
From: David Miller @ 2018-09-27  3:31 UTC (permalink / raw)
  To: zenczykowski; +Cc: maze, edumazet, netdev, dsahern
In-Reply-To: <20180926035627.221286-2-zenczykowski@gmail.com>

From: "Maciej Żenczykowski" <zenczykowski@gmail.com>
Date: Tue, 25 Sep 2018 20:56:27 -0700

> From: Maciej Żenczykowski <maze@google.com>
> 
> (the parameters in question are mark and flow_flags)
> 
> Reviewed-by: David Ahern <dsahern@gmail.com>
> Signed-off-by: Maciej Żenczykowski <maze@google.com>

Applied.

^ permalink raw reply

* Re: [PATCH 1/2] net-ipv4: remove 2 always zero parameters from ipv4_update_pmtu()
From: David Miller @ 2018-09-27  3:31 UTC (permalink / raw)
  To: zenczykowski; +Cc: maze, edumazet, netdev, dsahern
In-Reply-To: <20180926035627.221286-1-zenczykowski@gmail.com>

From: "Maciej Żenczykowski" <zenczykowski@gmail.com>
Date: Tue, 25 Sep 2018 20:56:26 -0700

> From: Maciej Żenczykowski <maze@google.com>
> 
> (the parameters in question are mark and flow_flags)
> 
> Reviewed-by: David Ahern <dsahern@gmail.com>
> Signed-off-by: Maciej Żenczykowski <maze@google.com>

Applied.

^ permalink raw reply

* Re: [PATCH net] vxlan: fill ttl inherit info
From: David Miller @ 2018-09-27  3:29 UTC (permalink / raw)
  To: liuhangbin; +Cc: netdev
In-Reply-To: <1537929342-10047-1-git-send-email-liuhangbin@gmail.com>

From: Hangbin Liu <liuhangbin@gmail.com>
Date: Wed, 26 Sep 2018 10:35:42 +0800

> When add vxlan ttl inherit support, I forgot to fill it when dump
> vlxan info. Fix it now.
> 
> Fixes: 72f6d71e491e6 ("vxlan: add ttl inherit support")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

Applied and queued up for -stable.

^ permalink raw reply

* Re: [REBASE PATCH net-next v9 0/4] net: vhost: improve performance when enable busyloop
From: David Miller @ 2018-09-27  3:26 UTC (permalink / raw)
  To: xiangxia.m.yue; +Cc: jasowang, mst, makita.toshiaki, netdev, virtualization
In-Reply-To: <1537879012-20859-1-git-send-email-xiangxia.m.yue@gmail.com>

From: xiangxia.m.yue@gmail.com
Date: Tue, 25 Sep 2018 05:36:48 -0700

> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> 
> This patches improve the guest receive performance.
> On the handle_tx side, we poll the sock receive queue
> at the same time. handle_rx do that in the same way.
> 
> For more performance report, see patch 4

Series applied, thank you.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox