Netdev List
 help / color / mirror / Atom feed
* [PATCH] amd-xgbe: Fix error path in xgbe_mod_init()
From: YueHaibing @ 2019-08-29  2:46 UTC (permalink / raw)
  To: thomas.lendacky, davem; +Cc: netdev, linux-kernel, YueHaibing

In xgbe_mod_init(), we should do cleanup if some error occurs

Reported-by: Hulk Robot <hulkci@huawei.com>
Fixes: efbaa828330a ("amd-xgbe: Add support to handle device renaming")
Fixes: 47f164deab22 ("amd-xgbe: Add PCI device support")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/ethernet/amd/xgbe/xgbe-main.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-main.c b/drivers/net/ethernet/amd/xgbe/xgbe-main.c
index b41f236..7ce9c69 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-main.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-main.c
@@ -469,13 +469,19 @@ static int __init xgbe_mod_init(void)
 
 	ret = xgbe_platform_init();
 	if (ret)
-		return ret;
+		goto err_platform_init;
 
 	ret = xgbe_pci_init();
 	if (ret)
-		return ret;
+		goto err_pci_init;
 
 	return 0;
+
+err_pci_init:
+	xgbe_platform_exit();
+err_platform_init:
+	unregister_netdevice_notifier(&xgbe_netdev_notifier);
+	return ret;
 }
 
 static void __exit xgbe_mod_exit(void)
-- 
1.8.3.1



^ permalink raw reply related

* Re: [PATCH v2] riscv: add support for SECCOMP and SECCOMP_FILTER
From: Paul Walmsley @ 2019-08-29  1:30 UTC (permalink / raw)
  To: Kees Cook, Tycho Andersen
  Cc: David Abdurachmanov, Palmer Dabbelt, Albert Ou, Oleg Nesterov,
	Andy Lutomirski, Will Drewry, Shuah Khan, Alexei Starovoitov,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
	David Abdurachmanov, Thomas Gleixner, Allison Randal,
	Alexios Zavras, Anup Patel, Vincent Chen, Alan Kao, linux-riscv,
	linux-kernel, linux-kselftest, netdev, bpf, me
In-Reply-To: <201908261043.08510F5E66@keescook>

Hi Kees,

On Mon, 26 Aug 2019, Kees Cook wrote:

> On Mon, Aug 26, 2019 at 09:39:50AM -0700, David Abdurachmanov wrote:
> > I don't have the a build with SECCOMP for the board right now, so it
> > will have to wait. I just finished a new kernel (almost rc6) for Fedora,
> 
> FWIW, I don't think this should block landing the code: all the tests
> fail without seccomp support. ;) So this patch is an improvement!

Am sympathetic to this -- we did it with the hugetlb patches for RISC-V -- 
but it would be good to understand a little bit more about why the test 
fails before we merge it.

Once we merge the patch, it will probably reduce the motivation for others 
to either understand and fix the underlying problem with the RISC-V code 
-- or, if it truly is a flaky test, to drop (or fix) the test in the 
seccomp_bpf kselftests.

Thanks for helping to take a closer look at this,

- Paul

^ permalink raw reply

* Re: [PATCH net-next 0/4] mlxsw: Various updates
From: David Miller @ 2019-08-29  1:24 UTC (permalink / raw)
  To: idosch; +Cc: netdev, jiri, mlxsw, idosch
In-Reply-To: <20190828155437.9852-1-idosch@idosch.org>

From: Ido Schimmel <idosch@idosch.org>
Date: Wed, 28 Aug 2019 18:54:33 +0300

> From: Ido Schimmel <idosch@mellanox.com>
> 
> Patch #1 from Amit removes 56G speed support. The reasons for this are
> detailed in the commit message.
> 
> Patch #2 from Shalom ensures that the hardware does not auto negotiate
> the number of used lanes. For example, if a four lane port supports 100G
> over both two and four lanes, it will not advertise the two lane link
> mode.
> 
> Patch #3 bumps the firmware version supported by the driver.
> 
> Patch #4 from Petr adds ethtool counters to help debug the internal PTP
> implementation in mlxsw. I copied Richard on this patch in case he has
> comments.

Series applied.

^ permalink raw reply

* RE: [PATCH net v4 0/2] r8152: fix side effect
From: Hayes Wang @ 2019-08-29  1:08 UTC (permalink / raw)
  To: David Miller
  Cc: netdev@vger.kernel.org, nic_swsd, linux-kernel@vger.kernel.org
In-Reply-To: <20190828.161735.1528060932193718727.davem@davemloft.net>

David Miller [mailto:davem@davemloft.net]
> Sent: Thursday, August 29, 2019 7:18 AM
[...]
> > v4:
> > Add Fixes tag for both patch #1 and #2.
> 
> I applied v3, sorry.
> 
> I think it is OK as I will backport things to v5.2 -stable anyways.

Thanks.

Best Regards,
Hayes



^ permalink raw reply

* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Andy Lutomirski @ 2019-08-29  0:58 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andy Lutomirski, Alexei Starovoitov, Kees Cook, LSM List,
	James Morris, Jann Horn, Peter Zijlstra, Masami Hiramatsu,
	Steven Rostedt, David S. Miller, Daniel Borkmann,
	Network Development, bpf, kernel-team, Linux API
In-Reply-To: <20190828233828.p7xddyw3fjzfinm6@ast-mbp.dhcp.thefacebook.com>



> On Aug 28, 2019, at 4:38 PM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> 
>> On Tue, Aug 27, 2019 at 11:20:19PM -0700, Andy Lutomirski wrote:
>> On Tue, Aug 27, 2019 at 9:49 PM Alexei Starovoitov
>> <alexei.starovoitov@gmail.com> wrote:
>>> 
>>>> On Tue, Aug 27, 2019 at 07:00:40PM -0700, Andy Lutomirski wrote:
>>>> 
>>>> Let me put this a bit differently. Part of the point is that
>>>> CAP_TRACING should allow a user or program to trace without being able
>>>> to corrupt the system. CAP_BPF as you’ve proposed it *can* likely
>>>> crash the system.
>>> 
>>> Really? I'm still waiting for your example where bpf+kprobe crashes the system...
>>> 
>> 
>> That's not what I meant.  bpf+kprobe causing a crash is a bug.  I'm
>> referring to a totally different issue.  On my laptop:
>> 
>> $ sudo bpftool map
>> 48: hash  name foobar  flags 0x0
>>    key 8B  value 8B  max_entries 64  memlock 8192B
>> 181: lpm_trie  flags 0x1
>>    key 8B  value 8B  max_entries 1  memlock 4096B
>> 182: lpm_trie  flags 0x1
>>    key 20B  value 8B  max_entries 1  memlock 4096B
>> 183: lpm_trie  flags 0x1
>>    key 8B  value 8B  max_entries 1  memlock 4096B
>> 184: lpm_trie  flags 0x1
>>    key 20B  value 8B  max_entries 1  memlock 4096B
>> 185: lpm_trie  flags 0x1
>>    key 8B  value 8B  max_entries 1  memlock 4096B
>> 186: lpm_trie  flags 0x1
>>    key 20B  value 8B  max_entries 1  memlock 4096B
>> 187: lpm_trie  flags 0x1
>>    key 8B  value 8B  max_entries 1  memlock 4096B
>> 188: lpm_trie  flags 0x1
>>    key 20B  value 8B  max_entries 1  memlock 4096B
>> 
>> $ sudo bpftool map dump id 186
>> key:
>> 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
>> 00 00 00 00
>> value:
>> 02 00 00 00 00 00 00 00
>> Found 1 element
>> 
>> $ sudo bpftool map delete id 186 key hex 00 00 00 00 00 00 00 00 00 00
>> 00 00 00 00 00 00 00 00 00 00
>> [this worked]
>> 
>> I don't know what my laptop was doing with map id 186 in particular,
>> but, whatever it was, I definitely broke it.  If a BPF firewall is in
>> use on something important enough, this could easily remove
>> connectivity from part or all of the system.  Right now, this needs
>> CAP_SYS_ADMIN.  With your patch, CAP_BPF is sufficient to do this, but
>> you *also* need CAP_BPF to trace the system using BPF.  Tracing with
>> BPF is 'safe' in the absence of bugs.  Modifying other peoples' maps
>> is not.
> 
> That lpm_trie is likely systemd implementing IP sandboxing.
> Not sure whether it's white or black list.
> Deleting an IP address from that map will either allow or disallow
> network traffic.
> Out of band operation on bpf map broke some bpf program. Sure.
> But calling it 'breaking the system' is quite a stretch.
> Calling it 'crashing the system' is plain wrong.
> Yet you're generalizing this bpf map read/write as
> "CAP_BPF as you’ve proposed it *can* likely crash the system."
> This is what I have a problem with.

Well, after I sent that email, firewalld on my laptop exploded and the system eventually hung.  I call that broken, and I really made a minimal effort here to break things.

> 
> Anyway, changing gears...
> Yes. I did propose to make a task with CAP_BPF to be able to
> manipulate arbitrary maps in the system.
> You could have said that if CAP_BPF is given to 'bpftool'
> then any user will be able to mess with other maps because
> bpftool is likely chmod-ed 755.
> Absolutely correct!
> It's not a fault of the CAP_BPF scope.
> Just don't give that cap to bpftool or do different acl/chmod.

I see no reason that allowing a user to use most of bpftool’s functionality necessarily needs to allow that user to corrupt the system. It obviously will expand the attack surface available to that user, but that should be it.

I’m trying to convince you that bpf’s security model can be made better than what you’re proposing. I’m genuinely not trying to get in your way. I’m trying to help you improve bpf.

^ permalink raw reply

* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Andy Lutomirski @ 2019-08-29  0:53 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andy Lutomirski, Alexei Starovoitov, Kees Cook, LSM List,
	James Morris, Jann Horn, Peter Zijlstra, Masami Hiramatsu,
	Steven Rostedt, David S. Miller, Daniel Borkmann,
	Network Development, bpf, kernel-team, Linux API
In-Reply-To: <DA52992F-4862-4945-8482-FE619A04C753@amacapital.net>



> On Aug 28, 2019, at 5:45 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> 
> 
>>> On Aug 28, 2019, at 3:55 PM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>>> 
>>> On Tue, Aug 27, 2019 at 11:12:29PM -0700, Andy Lutomirski wrote:
>>>>> 
>>>>> 
>>>>> From the previous discussion, you want to make progress toward solving
>>>>> a lot of problems with CAP_BPF.  One of them was making BPF
>>>>> firewalling more generally useful. By making CAP_BPF grant the ability
>>>>> to read kernel memory, you will make administrators much more nervous
>>>>> to grant CAP_BPF.
>>>> 
>>>> Andy, were your email hacked?
>>>> I explained several times that in this proposal
>>>> CAP_BPF _and_ CAP_TRACING _both_ are necessary to read kernel memory.
>>>> CAP_BPF alone is _not enough_.
>>> 
>>> You have indeed said this many times.  You've stated it as a matter of
>>> fact as though it cannot possibly discussed.  I'm asking you to
>>> justify it.
>> 
>> That's not how I see it.
>> I kept stating that both CAP_BPF and CAP_TRACING are necessary to read
>> kernel memory whereas you kept distorting my statement by dropping second
>> part and then making claims that "CAP_BPF grant the ability to read
>> kernel memory, you will make administrators much more nervous".
> 
> Mea culpa. CAP_BPF does, however, appear to permit breaking kASLR due to unsafe pointer conversions, and it allows reading and writing everyone’s maps.  I stand by my overall point.
> 
>> 
>> Just s/CAP_BPF/CAP_BPF and CAP_TRACING/ in this above sentence.
>> See that meaning suddenly changes?
>> Now administrators would be worried about tasks that have both at once.
>> They also would be worried about tasks that have CAP_TRACING alone,
>> because that's what allows probe_kernel_read().
> 
> This is not all what I meant. Of course granting CAP_BPF+CAP_TRACING allows reading kernel memory. This is not at all a problem.  Here is a problem I see:
> 
> CAP_TRACING + CAP_BPF allows modification of other people’s maps and potentially other things that should not be implied by CAP_TRACING alone and that don’t need to be available to tracers. So CAP_TRACING, which is powerful but has somewhat limited scope, isn’t fully useful without CAP_BPF, and giving CAP_TRACING *and* CAP_BPF allows things that teachers shouldn’t be able to do. I think this would make the whole mechanism less useful to Android, for example.
> 
> (Also, I’m not sure quite what you mean by “CAP_TRACING ... allows probe_kernel_read()”. probe_kernel_read() is a kernel function that can’t be directly called by userspace. CAP_TRACING allows reading kernel memory in plenty of ways regardless.)
> 
>> 
>>> It seems like you are specifically trying to add a new switch to turn
>>> as much of BPF as possible on and off.  Why?
>> 
>> Didn't I explain it several times already with multiple examples
>> from systemd, daemons, bpftrace ?
>> 
>> Let's try again.
>> Take your laptop with linux distro.
>> You're the only user there. I'm assuming you're not sharing it with
>> partner and kids. This is my definition of 'single user system'.
>> You can sudo on it at any time, but obviously prefer to run as many
>> apps as possible without cap_sys_admin.
>> Now you found some awesome open source app on the web that monitors
>> the health of the kernel and will pop a nice message on a screen if
>> something is wrong. Currently this app needs root. You hesitate,
>> but the apps is so useful and it has strong upstream code review process
>> that you keep running it 24/7.
>> This is open source app. New versions come. You upgrade.
>> You have enough trust in that app that you keep running it as root.
>> But there is always a chance that new version doing accidentaly
>> something stupid as 'kill -9 -1'. It's an open source app at the end.
>> 
>> Now I come with this CAP* proposal to make this app safer.
>> I'm not making your system more secure and not making this app
>> more secure. I can only make your laptop safer for day to day work
>> by limiting the operations this app can do.
>> This particular app monitros the kernel via bpf and tracing.
>> Hence you can give it CAP_TRACING and CAP_BPF and drop the rest.
> 
> This won’t make me much more comfortable, since CAP_BPF lets it do an ever-growing set of nasty things. I’d much rather one or both of two things happen:
> 
> 1. Give it CAP_TRACING only. It can leak my data, but it’s rather hard for it to crash my laptop, lose data, or cause other shenanigans.
> 
> 2. Improve it a bit do all the privileged ops are wrapped by capset().
> 
> Does this make sense?  I’m a security person on occasion. I find vulnerabilities and exploit them deliberately and I break things by accident on a regular basis. In my considered opinion, CAP_TRACING alone, even extended to cover part of BPF as I’ve described, is decently safe. Getting root with just CAP_TRACING will be decently challenging, especially if I don’t get to read things like sshd’s memory, and improvements to mitigate even that could be added.  I am quite confident that attacks starting with CAP_TRACING will have clear audit signatures if auditing is on.  I am also confident that CAP_BPF *will* allow DoS and likely privilege escalation, and this will only get more likely as BPF gets more widely used. And, if BPF-based auditing ever becomes a thing, writing to the audit daemon’s maps will be a great way to cover one’s tracks.
> 
> 
>> I think they have no choice but to do kernel.unprivileged_bpf_disabled=1.
>> We, as a kernel community, are forcing the users into it.
>> Hence I really do not see a value in any proposal today that expands
>> unprivileged bpf usage.
> 
> I think you’re overemphasizing bpf’s role in the whole speculation mess. I realize that you’ve spent an insane amount of time on mitigations to stupid issues. I’ve spent a less insane amount of time on mitigating similar issues outside of bpf.  It’s a mess.  At the end of the day, the kernel does its best, and new bugs show up. New CPUs will be less buggy.

Bah, accidentally hit send.

If the kernel’s mitigations aren’t good enough or you’re subject to direct user attack (e.g. via insufficient IBPB, SMT attack, etc) then you’re vulnerable. Otherwise you’re less vulnerable. BPF is by no means the whole story. Heck, the kernel *could*, at unfortunate performance cost, more aggressively flush around BPF and effectively treat it like user code.

So I think we should design bpf’s API’s security with the philosophy that speculation attacks are just one more type of bug, and we should make sure that real-world useful configurations don’t give BPF to tasks that don’t need it. My unpriv proposal tries to do this. This is *why* my proposal keeps test_run locked down and restricts running each program type to tasks that are explicitly granted the ability to attach it.

So let’s let CAP_TRACING use bpf. Speculation attacks are mostly irrelevant to tracers anyway, but all the rest of the stuff I’ve been talking is relevant.

^ permalink raw reply

* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Andy Lutomirski @ 2019-08-29  0:45 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andy Lutomirski, Alexei Starovoitov, Kees Cook, LSM List,
	James Morris, Jann Horn, Peter Zijlstra, Masami Hiramatsu,
	Steven Rostedt, David S. Miller, Daniel Borkmann,
	Network Development, bpf, kernel-team, Linux API
In-Reply-To: <20190828225512.q6qbvkdiqih2iewk@ast-mbp.dhcp.thefacebook.com>


> On Aug 28, 2019, at 3:55 PM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> 
>> On Tue, Aug 27, 2019 at 11:12:29PM -0700, Andy Lutomirski wrote:
>>>> 
>>>> 
>>>> From the previous discussion, you want to make progress toward solving
>>>> a lot of problems with CAP_BPF.  One of them was making BPF
>>>> firewalling more generally useful. By making CAP_BPF grant the ability
>>>> to read kernel memory, you will make administrators much more nervous
>>>> to grant CAP_BPF.
>>> 
>>> Andy, were your email hacked?
>>> I explained several times that in this proposal
>>> CAP_BPF _and_ CAP_TRACING _both_ are necessary to read kernel memory.
>>> CAP_BPF alone is _not enough_.
>> 
>> You have indeed said this many times.  You've stated it as a matter of
>> fact as though it cannot possibly discussed.  I'm asking you to
>> justify it.
> 
> That's not how I see it.
> I kept stating that both CAP_BPF and CAP_TRACING are necessary to read
> kernel memory whereas you kept distorting my statement by dropping second
> part and then making claims that "CAP_BPF grant the ability to read
> kernel memory, you will make administrators much more nervous".

Mea culpa. CAP_BPF does, however, appear to permit breaking kASLR due to unsafe pointer conversions, and it allows reading and writing everyone’s maps.  I stand by my overall point.

> 
> Just s/CAP_BPF/CAP_BPF and CAP_TRACING/ in this above sentence.
> See that meaning suddenly changes?
> Now administrators would be worried about tasks that have both at once.
> They also would be worried about tasks that have CAP_TRACING alone,
> because that's what allows probe_kernel_read().

This is not all what I meant. Of course granting CAP_BPF+CAP_TRACING allows reading kernel memory. This is not at all a problem.  Here is a problem I see:

CAP_TRACING + CAP_BPF allows modification of other people’s maps and potentially other things that should not be implied by CAP_TRACING alone and that don’t need to be available to tracers. So CAP_TRACING, which is powerful but has somewhat limited scope, isn’t fully useful without CAP_BPF, and giving CAP_TRACING *and* CAP_BPF allows things that teachers shouldn’t be able to do. I think this would make the whole mechanism less useful to Android, for example.

(Also, I’m not sure quite what you mean by “CAP_TRACING ... allows probe_kernel_read()”. probe_kernel_read() is a kernel function that can’t be directly called by userspace. CAP_TRACING allows reading kernel memory in plenty of ways regardless.)

> 
>> It seems like you are specifically trying to add a new switch to turn
>> as much of BPF as possible on and off.  Why?
> 
> Didn't I explain it several times already with multiple examples
> from systemd, daemons, bpftrace ?
> 
> Let's try again.
> Take your laptop with linux distro.
> You're the only user there. I'm assuming you're not sharing it with
> partner and kids. This is my definition of 'single user system'.
> You can sudo on it at any time, but obviously prefer to run as many
> apps as possible without cap_sys_admin.
> Now you found some awesome open source app on the web that monitors
> the health of the kernel and will pop a nice message on a screen if
> something is wrong. Currently this app needs root. You hesitate,
> but the apps is so useful and it has strong upstream code review process
> that you keep running it 24/7.
> This is open source app. New versions come. You upgrade.
> You have enough trust in that app that you keep running it as root.
> But there is always a chance that new version doing accidentaly
> something stupid as 'kill -9 -1'. It's an open source app at the end.
> 
> Now I come with this CAP* proposal to make this app safer.
> I'm not making your system more secure and not making this app
> more secure. I can only make your laptop safer for day to day work
> by limiting the operations this app can do.
> This particular app monitros the kernel via bpf and tracing.
> Hence you can give it CAP_TRACING and CAP_BPF and drop the rest.

This won’t make me much more comfortable, since CAP_BPF lets it do an ever-growing set of nasty things. I’d much rather one or both of two things happen:

1. Give it CAP_TRACING only. It can leak my data, but it’s rather hard for it to crash my laptop, lose data, or cause other shenanigans.

2. Improve it a bit do all the privileged ops are wrapped by capset().

Does this make sense?  I’m a security person on occasion. I find vulnerabilities and exploit them deliberately and I break things by accident on a regular basis. In my considered opinion, CAP_TRACING alone, even extended to cover part of BPF as I’ve described, is decently safe. Getting root with just CAP_TRACING will be decently challenging, especially if I don’t get to read things like sshd’s memory, and improvements to mitigate even that could be added.  I am quite confident that attacks starting with CAP_TRACING will have clear audit signatures if auditing is on.  I am also confident that CAP_BPF *will* allow DoS and likely privilege escalation, and this will only get more likely as BPF gets more widely used. And, if BPF-based auditing ever becomes a thing, writing to the audit daemon’s maps will be a great way to cover one’s tracks.


> I think they have no choice but to do kernel.unprivileged_bpf_disabled=1.
> We, as a kernel community, are forcing the users into it.
> Hence I really do not see a value in any proposal today that expands
> unprivileged bpf usage.

I think you’re overemphasizing bpf’s role in the whole speculation mess. I realize that you’ve spent an insane amount of time on mitigations to stupid issues. I’ve spent a less insane amount of time on mitigating similar issues outside of bpf.  It’s a mess.  At the end of the day, the kernel does its best, and new bugs show up. New CPUs will be less buggy. 

^ permalink raw reply

* Re: [PATCH net-next v2 6/9] r8169: don't use bit LastFrag in tx descriptor after send
From: Jakub Kicinski @ 2019-08-29  0:29 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Realtek linux nic maintainers, David Miller,
	netdev@vger.kernel.org, Chun-Hao Lin
In-Reply-To: <5b4c94bf-4571-7b36-1d83-c169980a6867@gmail.com>

On Wed, 28 Aug 2019 22:27:30 +0200, Heiner Kallweit wrote:
> On RTL8125 this bit is always cleared after send. Therefore check for
> tx_skb->skb being set what is functionally equivalent.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/net/ethernet/realtek/r8169_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index 652bacf62..4489cd9f2 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -5713,7 +5713,7 @@ static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp,
>  
>  		rtl8169_unmap_tx_skb(tp_to_dev(tp), tx_skb,
>  				     tp->TxDescArray + entry);
> -		if (status & LastFrag) {
> +		if (tx_skb->skb) {
>  			pkts_compl++;
>  			bytes_compl += tx_skb->skb->len;
>  			napi_consume_skb(tx_skb->skb, budget);

Hmm.. the dma_rmb() looks a little sus. Honestly I'm unclear on what it
was doing in the first place. READ_ONCE() should've been sufficient..

And it's not obviously clear what does the smp_rmb() at the start of
the function pair with.

But I don't think you're making anything worse here :)

^ permalink raw reply

* Re: [pull request][net-next v2 0/8] Mellanox, mlx5 updates 2019-08-22
From: Jakub Kicinski @ 2019-08-29  0:19 UTC (permalink / raw)
  To: Saeed Mahameed; +Cc: David S. Miller, netdev@vger.kernel.org
In-Reply-To: <20190828185720.2300-1-saeedm@mellanox.com>

On Wed, 28 Aug 2019 18:57:39 +0000, Saeed Mahameed wrote:
> This series provides some misc updates to mlx5 driver.
> For more information please see tag log below.
> 
> Please pull and let me know if there is any problem.
> 
> Please note that the series starts with a merge of mlx5-next branch,
> to resolve and avoid dependency with rdma tree.
> 
> v2: 
>  - Change statistics counter name to dev_internal_queue_oob as
>    suggested by Jakub.
>  - Fixed an issue with IP-in-IP TSO patch, found by regression testing.


Thanks! LGTM now

^ permalink raw reply

* Re: [PATCH V3 net 1/2] openvswitch: Properly set L4 keys on "later" IP fragments
From: Gregory Rose @ 2019-08-28 23:47 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, pshelar, joe
In-Reply-To: <20190828.145409.412910250799244993.davem@davemloft.net>

On 8/28/2019 2:54 PM, David Miller wrote:
> From: Greg Rose <gvrose8192@gmail.com>
> Date: Tue, 27 Aug 2019 07:58:09 -0700
>
>> When IP fragments are reassembled before being sent to conntrack, the
>> key from the last fragment is used.  Unless there are reordering
>> issues, the last fragment received will not contain the L4 ports, so the
>> key for the reassembled datagram won't contain them.  This patch updates
>> the key once we have a reassembled datagram.
>>
>> The handle_fragments() function works on L3 headers so we pull the L3/L4
>> flow key update code from key_extract into a new function
>> 'key_extract_l3l4'.  Then we add a another new function
>> ovs_flow_key_update_l3l4() and export it so that it is accessible by
>> handle_fragments() for conntrack packet reassembly.
>>
>> Co-authored by: Justin Pettit <jpettit@ovn.org>
>> Signed-off-by: Greg Rose <gvrose8192@gmail.com>
> Applied with Co-authored-by fixed.
Thanks for fixing that up Dave.

- Greg

^ permalink raw reply

* auto-split of commit. Was: [PATCH bpf-next 04/10] tools/bpf: add libbpf_prog_type_(from|to)_str helpers
From: Alexei Starovoitov @ 2019-08-28 23:46 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Julia Kartseva, ast, Greg Kroah-Hartman, Thomas Gleixner, rdna,
	bpf, daniel, netdev, kernel-team
In-Reply-To: <20190828163422.3d167c4b@cakuba.netronome.com>

On Wed, Aug 28, 2019 at 04:34:22PM -0700, Jakub Kicinski wrote:
> 
> Greg, Thomas, libbpf is extracted from the kernel sources and
> maintained in a clone repo on GitHub for ease of packaging.
> 
> IIUC Alexei's concern is that since we are moving the commits from
> the kernel repo to the GitHub one we have to preserve the commits
> exactly as they are, otherwise SOB lines lose their power.
> 
> Can you provide some guidance on whether that's a valid concern, 
> or whether it's perfectly fine to apply a partial patch?

Right. That's exactly the concern.

Greg, Thomas,
could you please put your legal hat on and clarify the following.
Say some developer does a patch that modifies
include/uapi/linux/bpf.h
..some other kernel code...and
tools/include/uapi/linux/bpf.h

That tools/include/uapi/linux/bpf.h is used by perf and by libbpf.
We have automatic mirror of tools/libbpf into github/libbpf/
so that external projects and can do git submodule of it,
can build packages out of it, etc.

The question is whether it's ok to split tools/* part out of
original commit, keep Author and SOB, create new commit out of it,
and automatically push that auto-generated commit into github mirror.

So far we've requested all developers to split their patches manually.
So that tools/* update is an individual commit that mirror can
simply git cherry-pick.


^ permalink raw reply

* Re: [PATCH net-next 0/4] mlxsw: Various updates
From: Jakub Kicinski @ 2019-08-28 23:45 UTC (permalink / raw)
  To: Ido Schimmel; +Cc: netdev, davem, jiri, mlxsw, Ido Schimmel
In-Reply-To: <20190828155437.9852-1-idosch@idosch.org>

On Wed, 28 Aug 2019 18:54:33 +0300, Ido Schimmel wrote:
> From: Ido Schimmel <idosch@mellanox.com>
> 
> Patch #1 from Amit removes 56G speed support. The reasons for this are
> detailed in the commit message.
> 
> Patch #2 from Shalom ensures that the hardware does not auto negotiate
> the number of used lanes. For example, if a four lane port supports 100G
> over both two and four lanes, it will not advertise the two lane link
> mode.
> 
> Patch #3 bumps the firmware version supported by the driver.
> 
> Patch #4 from Petr adds ethtool counters to help debug the internal PTP
> implementation in mlxsw. I copied Richard on this patch in case he has
> comments.

LGTM

^ permalink raw reply

* Re: [PATCH v2] riscv: add support for SECCOMP and SECCOMP_FILTER
From: Kees Cook @ 2019-08-28 23:44 UTC (permalink / raw)
  To: David Abdurachmanov
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Oleg Nesterov,
	Andy Lutomirski, Will Drewry, Shuah Khan, Alexei Starovoitov,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
	David Abdurachmanov, Thomas Gleixner, Allison Randal,
	Alexios Zavras, Anup Patel, Vincent Chen, Alan Kao, linux-riscv,
	linux-kernel, linux-kselftest, netdev, bpf, me
In-Reply-To: <CAEn-LToB1atxDvehBanVaxg6sk8zDkMe_CbqeTVgKNzOvD9-Sw@mail.gmail.com>

On Wed, Aug 28, 2019 at 02:37:34PM -0700, David Abdurachmanov wrote:
>     --disk path=$PWD/disk \
>     --boot kernel=$PWD/${FIRMWARE} \

This is where I tripped over things. How do I specify the kernel to boot
from OUTSIDE the disk image?

-- 
Kees Cook

^ permalink raw reply

* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Alexei Starovoitov @ 2019-08-28 23:38 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Alexei Starovoitov, Kees Cook, LSM List, James Morris, Jann Horn,
	Peter Zijlstra, Masami Hiramatsu, Steven Rostedt, David S. Miller,
	Daniel Borkmann, Network Development, bpf, kernel-team, Linux API
In-Reply-To: <CALCETrX-bn2SpVzTkPz+A=z_oWDs7PNeouzK7wRWMzyaBd4+7g@mail.gmail.com>

On Tue, Aug 27, 2019 at 11:20:19PM -0700, Andy Lutomirski wrote:
> On Tue, Aug 27, 2019 at 9:49 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Tue, Aug 27, 2019 at 07:00:40PM -0700, Andy Lutomirski wrote:
> > >
> > > Let me put this a bit differently. Part of the point is that
> > > CAP_TRACING should allow a user or program to trace without being able
> > > to corrupt the system. CAP_BPF as you’ve proposed it *can* likely
> > > crash the system.
> >
> > Really? I'm still waiting for your example where bpf+kprobe crashes the system...
> >
> 
> That's not what I meant.  bpf+kprobe causing a crash is a bug.  I'm
> referring to a totally different issue.  On my laptop:
> 
> $ sudo bpftool map
> 48: hash  name foobar  flags 0x0
>     key 8B  value 8B  max_entries 64  memlock 8192B
> 181: lpm_trie  flags 0x1
>     key 8B  value 8B  max_entries 1  memlock 4096B
> 182: lpm_trie  flags 0x1
>     key 20B  value 8B  max_entries 1  memlock 4096B
> 183: lpm_trie  flags 0x1
>     key 8B  value 8B  max_entries 1  memlock 4096B
> 184: lpm_trie  flags 0x1
>     key 20B  value 8B  max_entries 1  memlock 4096B
> 185: lpm_trie  flags 0x1
>     key 8B  value 8B  max_entries 1  memlock 4096B
> 186: lpm_trie  flags 0x1
>     key 20B  value 8B  max_entries 1  memlock 4096B
> 187: lpm_trie  flags 0x1
>     key 8B  value 8B  max_entries 1  memlock 4096B
> 188: lpm_trie  flags 0x1
>     key 20B  value 8B  max_entries 1  memlock 4096B
> 
> $ sudo bpftool map dump id 186
> key:
> 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
> 00 00 00 00
> value:
> 02 00 00 00 00 00 00 00
> Found 1 element
> 
> $ sudo bpftool map delete id 186 key hex 00 00 00 00 00 00 00 00 00 00
> 00 00 00 00 00 00 00 00 00 00
> [this worked]
> 
> I don't know what my laptop was doing with map id 186 in particular,
> but, whatever it was, I definitely broke it.  If a BPF firewall is in
> use on something important enough, this could easily remove
> connectivity from part or all of the system.  Right now, this needs
> CAP_SYS_ADMIN.  With your patch, CAP_BPF is sufficient to do this, but
> you *also* need CAP_BPF to trace the system using BPF.  Tracing with
> BPF is 'safe' in the absence of bugs.  Modifying other peoples' maps
> is not.

That lpm_trie is likely systemd implementing IP sandboxing.
Not sure whether it's white or black list.
Deleting an IP address from that map will either allow or disallow
network traffic.
Out of band operation on bpf map broke some bpf program. Sure.
But calling it 'breaking the system' is quite a stretch.
Calling it 'crashing the system' is plain wrong.
Yet you're generalizing this bpf map read/write as
"CAP_BPF as you’ve proposed it *can* likely crash the system."
This is what I have a problem with.

Anyway, changing gears...
Yes. I did propose to make a task with CAP_BPF to be able to
manipulate arbitrary maps in the system.
You could have said that if CAP_BPF is given to 'bpftool'
then any user will be able to mess with other maps because
bpftool is likely chmod-ed 755.
Absolutely correct!
It's not a fault of the CAP_BPF scope.
Just don't give that cap to bpftool or do different acl/chmod.

> If the answer is the latter, then maybe it would make sense to try to
> implement some of the unprivileged bpf stuff and then to see whether
> CAP_BPF is still needed.

<broken_record_mode=on> Nack to extensions to unprivileged bpf.


^ permalink raw reply

* Re: [PATCH bpf-next 04/10] tools/bpf: add libbpf_prog_type_(from|to)_str helpers
From: Jakub Kicinski @ 2019-08-28 23:34 UTC (permalink / raw)
  To: Julia Kartseva, ast, Greg Kroah-Hartman, Thomas Gleixner
  Cc: rdna, bpf, daniel, netdev, kernel-team
In-Reply-To: <467620c966825173dbd65b37a3f9bd7dd4fb8184.1567024943.git.hex@fb.com>

On Wed, 28 Aug 2019 14:03:07 -0700, Julia Kartseva wrote:
> Standardize string representation of prog types by putting commonly used
> names to libbpf.
> The prog_type to string mapping is taken from bpftool:
> tools/bpf/bpftool/main.h
> 
> Signed-off-by: Julia Kartseva <hex@fb.com>

This "libbpf patches have to be completely separate" just went to
another level :/ Now we are splitting code moves into add and remove
parts which are 5 patches apart? How are we supposed to review this?


Greg, Thomas, libbpf is extracted from the kernel sources and
maintained in a clone repo on GitHub for ease of packaging.

IIUC Alexei's concern is that since we are moving the commits from
the kernel repo to the GitHub one we have to preserve the commits
exactly as they are, otherwise SOB lines lose their power.

Can you provide some guidance on whether that's a valid concern, 
or whether it's perfectly fine to apply a partial patch?

(HW vendors also back port tree-wide cleanups into their drivers,
 so if SOB lines are voided by git format-patch -- driver/path/
 that'd be quite an issue..)

> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 72e6e5eb397f..946a4d41f223 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -296,6 +296,35 @@ struct bpf_object {
>  };
>  #define obj_elf_valid(o)	((o)->efile.elf)
>  
> +static const char *const prog_type_strs[] = {
> +	[BPF_PROG_TYPE_UNSPEC] = "unspec",
> +	[BPF_PROG_TYPE_SOCKET_FILTER] = "socket_filter",
> +	[BPF_PROG_TYPE_KPROBE] = "kprobe",
> +	[BPF_PROG_TYPE_SCHED_CLS] = "sched_cls",
> +	[BPF_PROG_TYPE_SCHED_ACT] = "sched_act",
> +	[BPF_PROG_TYPE_TRACEPOINT] = "tracepoint",
> +	[BPF_PROG_TYPE_XDP] = "xdp",
> +	[BPF_PROG_TYPE_PERF_EVENT] = "perf_event",
> +	[BPF_PROG_TYPE_CGROUP_SKB] = "cgroup_skb",
> +	[BPF_PROG_TYPE_CGROUP_SOCK] = "cgroup_sock",
> +	[BPF_PROG_TYPE_LWT_IN] = "lwt_in",
> +	[BPF_PROG_TYPE_LWT_OUT] = "lwt_out",
> +	[BPF_PROG_TYPE_LWT_XMIT] = "lwt_xmit",
> +	[BPF_PROG_TYPE_SOCK_OPS] = "sock_ops",
> +	[BPF_PROG_TYPE_SK_SKB] = "sk_skb",
> +	[BPF_PROG_TYPE_CGROUP_DEVICE] = "cgroup_device",
> +	[BPF_PROG_TYPE_SK_MSG] = "sk_msg",
> +	[BPF_PROG_TYPE_RAW_TRACEPOINT] = "raw_tracepoint",
> +	[BPF_PROG_TYPE_CGROUP_SOCK_ADDR] = "cgroup_sock_addr",
> +	[BPF_PROG_TYPE_LWT_SEG6LOCAL] = "lwt_seg6local",
> +	[BPF_PROG_TYPE_LIRC_MODE2] = "lirc_mode2",
> +	[BPF_PROG_TYPE_SK_REUSEPORT] = "sk_reuseport",
> +	[BPF_PROG_TYPE_FLOW_DISSECTOR] = "flow_dissector",
> +	[BPF_PROG_TYPE_CGROUP_SYSCTL] = "cgroup_sysctl",
> +	[BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE] = "raw_tracepoint_writable",
> +	[BPF_PROG_TYPE_CGROUP_SOCKOPT] = "cgroup_sockopt",
> +};
> +
>  void bpf_program__unload(struct bpf_program *prog)
>  {
>  	int i;
> @@ -4632,6 +4661,28 @@ int libbpf_attach_type_by_name(const char *name,
>  	return -EINVAL;
>  }
>  
> +int libbpf_prog_type_from_str(const char *str, enum bpf_prog_type *type)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(prog_type_strs); i++)
> +		if (prog_type_strs[i] && strcmp(prog_type_strs[i], str) == 0) {
> +			*type = i;
> +			return 0;
> +		}
> +
> +	return -EINVAL;
> +}
> +
> +int libbpf_prog_type_to_str(enum bpf_prog_type type, const char **str)
> +{
> +	if (type < BPF_PROG_TYPE_UNSPEC || type >= ARRAY_SIZE(prog_type_strs))
> +		return -EINVAL;
> +
> +	*str = prog_type_strs[type];
> +	return 0;
> +}
> +
>  static int
>  bpf_program__identify_section(struct bpf_program *prog,
>  			      enum bpf_prog_type *prog_type,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index e8f70977d137..6846c488d8a2 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -122,12 +122,20 @@ LIBBPF_API int bpf_object__set_priv(struct bpf_object *obj, void *priv,
>  				    bpf_object_clear_priv_t clear_priv);
>  LIBBPF_API void *bpf_object__priv(const struct bpf_object *prog);
>  
> +/* Program and expected attach types by section name */
>  LIBBPF_API int
>  libbpf_prog_type_by_name(const char *name, enum bpf_prog_type *prog_type,
>  			 enum bpf_attach_type *expected_attach_type);
> +/* Attach type by section name */
>  LIBBPF_API int libbpf_attach_type_by_name(const char *name,
>  					  enum bpf_attach_type *attach_type);
>  
> +/* String representation of program type */
> +LIBBPF_API int libbpf_prog_type_from_str(const char *str,
> +					 enum bpf_prog_type *type);
> +LIBBPF_API int libbpf_prog_type_to_str(enum bpf_prog_type type,
> +				       const char **str);
> +
>  /* Accessors of bpf_program */
>  struct bpf_program;
>  LIBBPF_API struct bpf_program *bpf_program__next(struct bpf_program *prog,
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 664ce8e7a60e..2ea7c99f1579 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -188,4 +188,6 @@ LIBBPF_0.0.4 {
>  LIBBPF_0.0.5 {
>  	global:
>  		bpf_btf_get_next_id;
> +		libbpf_prog_type_from_str;
> +		libbpf_prog_type_to_str;
>  } LIBBPF_0.0.4;


^ permalink raw reply

* Re: [PATCH net-next 00/12] net: hns3: add some cleanups and optimizations
From: Jakub Kicinski @ 2019-08-28 23:19 UTC (permalink / raw)
  To: Huazhong Tan
  Cc: davem, netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
	Andrew Lunn
In-Reply-To: <1567002196-63242-1-git-send-email-tanhuazhong@huawei.com>

On Wed, 28 Aug 2019 22:23:04 +0800, Huazhong Tan wrote:
> This patch-set includes cleanups, optimizations and bugfix for
> the HNS3 ethernet controller driver.

The phy loopback (patch 10) could probably benefit from an expert look
but in general LGTM.

^ permalink raw reply

* Re: [PATCH net v4 0/2] r8152: fix side effect
From: David Miller @ 2019-08-28 23:17 UTC (permalink / raw)
  To: hayeswang; +Cc: netdev, nic_swsd, linux-kernel
In-Reply-To: <1394712342-15778-323-Taiwan-albertk@realtek.com>

From: Hayes Wang <hayeswang@realtek.com>
Date: Wed, 28 Aug 2019 20:56:11 +0800

> v4:
> Add Fixes tag for both patch #1 and #2.

I applied v3, sorry.

I think it is OK as I will backport things to v5.2 -stable anyways.

^ permalink raw reply

* Re: [PATCH net-next] net: phy: force phy suspend when calling phy_stop
From: David Miller @ 2019-08-28 23:17 UTC (permalink / raw)
  To: shenjian15
  Cc: andrew, f.fainelli, hkallweit1, sergei.shtylyov, netdev,
	forest.zhouchang, linuxarm
In-Reply-To: <1566956087-37096-1-git-send-email-shenjian15@huawei.com>

From: Jian Shen <shenjian15@huawei.com>
Date: Wed, 28 Aug 2019 09:34:47 +0800

> Some ethernet drivers may call phy_start() and phy_stop() from
> ndo_open() and ndo_close() respectively.
> 
> When network cable is unconnected, and operate like below:
> step 1: ifconfig ethX up -> ndo_open -> phy_start ->start
> autoneg, and phy is no link.
> step 2: ifconfig ethX down -> ndo_close -> phy_stop -> just stop
> phy state machine.
> 
> This patch forces phy suspend even phydev->link is off.
> 
> Signed-off-by: Jian Shen <shenjian15@huawei.com>

Applied.

^ permalink raw reply

* [PATCH][V2] arcnet: capmode: remove redundant assignment to pointer pkt
From: Colin King @ 2019-08-28 23:14 UTC (permalink / raw)
  To: Michael Grzeschik, David S . Miller, netdev; +Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

Pointer pkt is being initialized with a value that is never read
and pkt is being re-assigned a little later on. The assignment is
redundant and hence can be removed.

Addresses-Coverity: ("Ununsed value")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---

V2: fix typo in patch description, pkg -> pkt

---
 drivers/net/arcnet/capmode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/arcnet/capmode.c b/drivers/net/arcnet/capmode.c
index b780be6f41ff..c09b567845e1 100644
--- a/drivers/net/arcnet/capmode.c
+++ b/drivers/net/arcnet/capmode.c
@@ -44,7 +44,7 @@ static void rx(struct net_device *dev, int bufnum,
 {
 	struct arcnet_local *lp = netdev_priv(dev);
 	struct sk_buff *skb;
-	struct archdr *pkt = pkthdr;
+	struct archdr *pkt;
 	char *pktbuf, *pkthdrbuf;
 	int ofs;
 
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH] sky2: Disable MSI on yet another ASUS boards (P6Xxxx)
From: David Miller @ 2019-08-28 23:09 UTC (permalink / raw)
  To: tiwai; +Cc: mlindner, stephen, netdev, linux-kernel, swm
In-Reply-To: <20190828063119.22248-1-tiwai@suse.de>

From: Takashi Iwai <tiwai@suse.de>
Date: Wed, 28 Aug 2019 08:31:19 +0200

> A similar workaround for the suspend/resume problem is needed for yet
> another ASUS machines, P6X models.  Like the previous fix, the BIOS
> doesn't provide the standard DMI_SYS_* entry, so again DMI_BOARD_*
> entries are used instead.
> 
> Reported-and-tested-by: SteveM <swm@swm1.com>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>

Applied, but this is getting suspicious.

It looks like MSI generally is not restored properly on resume on these
boards, so maybe there simply needs to be a generic PCI quirk for that?

^ permalink raw reply

* Re: [PATCH net 0/2] nfp: flower: fix bugs in merge tunnel encap code
From: David Miller @ 2019-08-28 23:08 UTC (permalink / raw)
  To: jakub.kicinski; +Cc: netdev, oss-drivers
In-Reply-To: <20190828055630.17331-1-jakub.kicinski@netronome.com>

From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Tue, 27 Aug 2019 22:56:28 -0700

> John says:
> 
> There are few bugs in the merge encap code that have come to light with
> recent driver changes. Effectively, flow bind callbacks were being
> registered twice when using internal ports (new 'busy' code triggers
> this). There was also an issue with neighbour notifier messages being
> ignored for internal ports.

Series applied and queued up for v5.2 -stable, thanks.

^ permalink raw reply

* Re: [PATCH net-next 09/15] net: sgi: ioc3-eth: split ring cleaning/freeing and allocation
From: Jakub Kicinski @ 2019-08-28 23:04 UTC (permalink / raw)
  To: Thomas Bogendoerfer
  Cc: Ralf Baechle, Paul Burton, James Hogan, David S. Miller,
	linux-mips, linux-kernel, netdev
In-Reply-To: <20190828140315.17048-10-tbogendoerfer@suse.de>

On Wed, 28 Aug 2019 16:03:08 +0200, Thomas Bogendoerfer wrote:
> Do tx ring cleaning and freeing of rx buffers, when chip is shutdown and
> allocate buffers before bringing chip up.
> 
> Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>

Ah, so you are moving the alloc/free to open/close, that's good.

^ permalink raw reply

* Re: [PATCH net-next 06/15] net: sgi: ioc3-eth: get rid of ioc3_clean_rx_ring()
From: Jakub Kicinski @ 2019-08-28 23:02 UTC (permalink / raw)
  To: Thomas Bogendoerfer
  Cc: Ralf Baechle, Paul Burton, James Hogan, David S. Miller,
	linux-mips, linux-kernel, netdev
In-Reply-To: <20190828140315.17048-7-tbogendoerfer@suse.de>

On Wed, 28 Aug 2019 16:03:05 +0200, Thomas Bogendoerfer wrote:
> Clean rx ring is just called once after a new ring is allocated, which
> is per definition clean. So there is not need for this function.
> 
> Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
> ---
>  drivers/net/ethernet/sgi/ioc3-eth.c | 21 ---------------------
>  1 file changed, 21 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c
> index 6ca560d4ab79..39631e067b71 100644
> --- a/drivers/net/ethernet/sgi/ioc3-eth.c
> +++ b/drivers/net/ethernet/sgi/ioc3-eth.c
> @@ -761,26 +761,6 @@ static void ioc3_mii_start(struct ioc3_private *ip)
>  	add_timer(&ip->ioc3_timer);
>  }
>  
> -static inline void ioc3_clean_rx_ring(struct ioc3_private *ip)
> -{
> -	struct ioc3_erxbuf *rxb;
> -	struct sk_buff *skb;
> -	int i;
> -
> -	for (i = ip->rx_ci; i & 15; i++) {
> -		ip->rx_skbs[ip->rx_pi] = ip->rx_skbs[ip->rx_ci];
> -		ip->rxr[ip->rx_pi++] = ip->rxr[ip->rx_ci++];
> -	}
> -	ip->rx_pi &= RX_RING_MASK;
> -	ip->rx_ci &= RX_RING_MASK;
> -
> -	for (i = ip->rx_ci; i != ip->rx_pi; i = (i + 1) & RX_RING_MASK) {
> -		skb = ip->rx_skbs[i];
> -		rxb = (struct ioc3_erxbuf *)(skb->data - RX_OFFSET);
> -		rxb->w0 = 0;

There's gotta be some purpose to setting this w0 word to zero no?
ioc3_rx() uses that to see if the descriptor is done, and dutifully
clears it after..

> -	}
> -}
> -
>  static inline void ioc3_clean_tx_ring(struct ioc3_private *ip)
>  {
>  	struct sk_buff *skb;
> @@ -860,7 +840,6 @@ static void ioc3_init_rings(struct net_device *dev)
>  	ioc3_free_rings(ip);
>  	ioc3_alloc_rings(dev);
>  
> -	ioc3_clean_rx_ring(ip);
>  	ioc3_clean_tx_ring(ip);
>  
>  	/* Now the rx ring base, consume & produce registers.  */


^ permalink raw reply

* Re: [PATCH net v3 0/2] r8152: fix side effect
From: David Miller @ 2019-08-28 23:02 UTC (permalink / raw)
  To: hayeswang; +Cc: netdev, nic_swsd, linux-kernel
In-Reply-To: <1394712342-15778-320-Taiwan-albertk@realtek.com>

From: Hayes Wang <hayeswang@realtek.com>
Date: Wed, 28 Aug 2019 09:51:40 +0800

> v3:
> Update the commit message for patch #1.
> 
> v2:
> Replace patch #2 with "r8152: remove calling netif_napi_del".
> 
> v1:
> The commit 0ee1f4734967 ("r8152: napi hangup fix after disconnect")
> add a check to avoid using napi_disable after netif_napi_del. However,
> the commit ffa9fec30ca0 ("r8152: set RTL8152_UNPLUG only for real
> disconnection") let the check useless.
> 
> Therefore, I revert commit 0ee1f4734967 ("r8152: napi hangup fix
> after disconnect") first, and add another patch to fix it.

Series applied, thank you.

^ permalink raw reply

* Re: [PATCH net-next] net/ncsi: add response handlers for PLDM over NC-SI
From: David Miller @ 2019-08-28 23:00 UTC (permalink / raw)
  To: benwei; +Cc: sam, netdev, linux-kernel, openbmc
In-Reply-To: <CH2PR15MB3686302D8210855E5AB643B1A3A00@CH2PR15MB3686.namprd15.prod.outlook.com>

From: Ben Wei <benwei@fb.com>
Date: Tue, 27 Aug 2019 23:03:53 +0000

> This patch adds handlers for PLDM over NC-SI command response.
> 
> This enables NC-SI driver recognizes the packet type so the responses don't get dropped as unknown packet type.
> 
> PLDM over NC-SI are not handled in kernel driver for now, but can be passed back to user space via Netlink for further handling.
> 
> Signed-off-by: Ben Wei <benwei@fb.com>

I don't know why but patchwork puts part of your patch into the commit message, see:

https://patchwork.ozlabs.org/patch/1154104/

It's probably an encoding issue or similar.

> +static int ncsi_rsp_handler_pldm(struct ncsi_request *nr) {
> +	return 0;
> +}
> +
>  static int ncsi_rsp_handler_netlink(struct ncsi_request *nr)  {

I know other functions in this file do it, but please put the openning
curly braces of a function on a separate line.

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