Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] net: ethoc: Make needlessly global struct ethtool_ops static
From: David Miller @ 2017-01-17 20:51 UTC (permalink / raw)
  To: tklauser; +Cc: netdev, f.fainelli, thierry.reding, colin.king, tremyfr
In-Reply-To: <20170117140108.1544-1-tklauser@distanz.ch>

From: Tobias Klauser <tklauser@distanz.ch>
Date: Tue, 17 Jan 2017 15:01:08 +0100

> Make the needlessly global struct ethtool_ops ethoc_ethtool_ops static
> to fix a sparse warning.
> 
> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next] tcp: accept RST for rcv_nxt - 1 after receiving a FIN
From: David Miller @ 2017-01-17 20:52 UTC (permalink / raw)
  To: jbaron; +Cc: netdev, edumazet
In-Reply-To: <1484678239-19199-1-git-send-email-jbaron@akamai.com>

From: Jason Baron <jbaron@akamai.com>
Date: Tue, 17 Jan 2017 13:37:19 -0500

> From: Jason Baron <jbaron@akamai.com>
> 
> Using a Mac OSX box as a client connecting to a Linux server, we have found
> that when certain applications (such as 'ab'), are abruptly terminated
> (via ^C), a FIN is sent followed by a RST packet on tcp connections. The
> FIN is accepted by the Linux stack but the RST is sent with the same
> sequence number as the FIN, and Linux responds with a challenge ACK per
> RFC 5961. The OSX client then sometimes (they are rate-limited) does not
> reply with any RST as would be expected on a closed socket.
> 
> This results in sockets accumulating on the Linux server left mostly in
> the CLOSE_WAIT state, although LAST_ACK and CLOSING are also possible.
> This sequence of events can tie up a lot of resources on the Linux server
> since there may be a lot of data in write buffers at the time of the RST.
> Accepting a RST equal to rcv_nxt - 1, after we have already successfully
> processed a FIN, has made a significant difference for us in practice, by
> freeing up unneeded resources in a more expedient fashion.
> 
> A packetdrill test demonstrating the behavior:
 ...
> Signed-off-by: Jason Baron <jbaron@akamai.com>

Applied, thanks Jason.

^ permalink raw reply

* Re: [PATCH net] lwtunnel: fix autoload of lwt modules
From: David Miller @ 2017-01-17 20:54 UTC (permalink / raw)
  To: dsa; +Cc: rshearma, netdev, roopa
In-Reply-To: <7655b85f-4c03-7f08-315c-b6a99a1631f0@cumulusnetworks.com>

From: David Ahern <dsa@cumulusnetworks.com>
Date: Tue, 17 Jan 2017 13:46:22 -0700

> In short seems like removing the dev + the current patch dropping
> the lock fixes the current deadlock problem and should be fine.

What about the state recorded by fib_get_nhs() and similar?  There is
a mapping from ifindex to ->nh_dev which would be invalidated if the
RTNL semaphore is dropped.

It won't get updated by device events, which is what normally happens,
because the fib_info is not in any of the fib_trie tables yet.

So I think you still have a huge problem without doing proper restarts.

^ permalink raw reply

* Re: fs, net: deadlock between bind/splice on af_unix
From: Cong Wang @ 2017-01-17 21:21 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Al Viro, linux-fsdevel@vger.kernel.org, LKML, David Miller,
	Rainer Weikusat, Hannes Frederic Sowa, netdev, Eric Dumazet,
	syzkaller
In-Reply-To: <CACT4Y+ad1s6HPdw-ocQ5YfFx7qCeWb3z1k7F9+9KsNmw6+Tszw@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1326 bytes --]

On Mon, Jan 16, 2017 at 1:32 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Fri, Dec 9, 2016 at 7:41 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>> On Thu, Dec 08, 2016 at 10:32:00PM -0800, Cong Wang wrote:
>>
>>> > Why do we do autobind there, anyway, and why is it conditional on
>>> > SOCK_PASSCRED?  Note that e.g. for SOCK_STREAM we can bloody well get
>>> > to sending stuff without autobind ever done - just use socketpair()
>>> > to create that sucker and we won't be going through the connect()
>>> > at all.
>>>
>>> In the case Dmitry reported, unix_dgram_sendmsg() calls unix_autobind(),
>>> not SOCK_STREAM.
>>
>> Yes, I've noticed.  What I'm asking is what in there needs autobind triggered
>> on sendmsg and why doesn't the same need affect the SOCK_STREAM case?
>>
>>> I guess some lock, perhaps the u->bindlock could be dropped before
>>> acquiring the next one (sb_writer), but I need to double check.
>>
>> Bad idea, IMO - do you *want* autobind being able to come through while
>> bind(2) is busy with mknod?
>
>
> Ping. This is still happening on HEAD.
>

Thanks for your reminder. Mind to give the attached patch (compile only)
a try? I take another approach to fix this deadlock, which moves the
unix_mknod() out of unix->bindlock. Not sure if there is any unexpected
impact with this way.

Thanks.

[-- Attachment #2: unix.diff --]
[-- Type: text/plain, Size: 1678 bytes --]

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 127656e..5d4b4d1 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -995,6 +995,7 @@ static int unix_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 	unsigned int hash;
 	struct unix_address *addr;
 	struct hlist_head *list;
+	struct path path;
 
 	err = -EINVAL;
 	if (sunaddr->sun_family != AF_UNIX)
@@ -1010,9 +1011,20 @@ static int unix_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 		goto out;
 	addr_len = err;
 
+	if (sun_path[0]) {
+		umode_t mode = S_IFSOCK |
+		       (SOCK_INODE(sock)->i_mode & ~current_umask());
+		err = unix_mknod(sun_path, mode, &path);
+		if (err) {
+			if (err == -EEXIST)
+				err = -EADDRINUSE;
+			goto out;
+		}
+	}
+
 	err = mutex_lock_interruptible(&u->bindlock);
 	if (err)
-		goto out;
+		goto out_put;
 
 	err = -EINVAL;
 	if (u->addr)
@@ -1029,16 +1041,6 @@ static int unix_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 	atomic_set(&addr->refcnt, 1);
 
 	if (sun_path[0]) {
-		struct path path;
-		umode_t mode = S_IFSOCK |
-		       (SOCK_INODE(sock)->i_mode & ~current_umask());
-		err = unix_mknod(sun_path, mode, &path);
-		if (err) {
-			if (err == -EEXIST)
-				err = -EADDRINUSE;
-			unix_release_addr(addr);
-			goto out_up;
-		}
 		addr->hash = UNIX_HASH_SIZE;
 		hash = d_backing_inode(path.dentry)->i_ino & (UNIX_HASH_SIZE - 1);
 		spin_lock(&unix_table_lock);
@@ -1065,6 +1067,9 @@ static int unix_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 	spin_unlock(&unix_table_lock);
 out_up:
 	mutex_unlock(&u->bindlock);
+out_put:
+	if (err)
+		path_put(&path);
 out:
 	return err;
 }

^ permalink raw reply related

* 52bd2d62ce6758d811edcbd2256eb9ea7f6a56cb fixing crashes? -> 4.4 stable?
From: Nikola Ciprich @ 2017-01-17 21:48 UTC (permalink / raw)
  To: netdev; +Cc: edumazet, nik

Dear netdev developers,

I'd like to ask for a consultation regarding 4.4 kernel crashes.
we're using intel X540-AT2 10g controllers (onboard ones, on supermicro
boards) and we've noticed, then when using openvswitch, system very quickly
crashes on 4.4.x kernels we're usign. 4.5 is fine though.

here's backtrace gathered from system pstore:

<1>[ 1084.114586] BUG: unable to handle kernel paging request at ffff8840c365b5c4
<1>[ 1084.114918] IP: [<ffffffff81589802>] __netdev_pick_tx+0x92/0x140
<4>[ 1084.115101] PGD 2018067 PUD 0
<4>[ 1084.115270] Oops: 0000 [#1] SMP
<4>[ 1084.115439] Modules linked in: bonding(E) openvswitch(E) nf_defrag_ipv6(E) nf_conntrack(E) crc32_pclmul(E) aesni_intel(E) lrw(E) gf128mul(E) glue_helper(E) ablk_helper(E) cryptd(E) kvm
_intel(E) kvm(E) irqbypass(E) coretemp(E) crct10dif_pclmul(E) intel_powerclamp(E) x86_pkg_temp_thermal(E) ses(E) enclosure(E) iTCO_wdt(E) iTCO_vendor_support(E) mxm_wmi(E) i2c_i801(E) lpc_ic
h(E) mei_me(E) mfd_core(E) i2c_core(E) sb_edac(E) sg(E) mei(E) pcspkr(E) edac_core(E) ipmi_devintf(E) ioatdma(E) shpchp(E) wmi(E) ipmi_si(E) ipmi_msghandler(E) 8250_fintek(E) acpi_power_mete
r(E) acpi_pad(E) nfsd(E) auth_rpcgss(E) nfs_acl(E) lockd(E) grace(E) sunrpc(E) ip_tables(E) ext4(E) jbd2(E) mbcache(E) raid1(E) sd_mod(E) ahci(E) libahci(E) bnx2x(E) libcrc32c(E) ixgbe(E) cr
c32c_intel(E) libata(E) mdio(E) ptp(E) dca(E) megaraid_sas(E) pps_core(E) dm_mirror(E) dm_region_hash(E) dm_log(E) dm_mod(E)
<4>[ 1084.117683] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G            E   4.4.33lb7.01 #1
<4>[ 1084.118012] Hardware name: Supermicro X10DRi/X10DRI-T, BIOS 2.1 09/13/2016
<4>[ 1084.118181] task: ffffffff819f14c0 ti: ffffffff819e0000 task.ti: ffffffff819e0000
<4>[ 1084.118501] RIP: 0010:[<ffffffff81589802>]  [<ffffffff81589802>] __netdev_pick_tx+0x92/0x140
<4>[ 1084.118828] RSP: 0018:ffff883f7f003638  EFLAGS: 00010a02
<4>[ 1084.118994] RAX: 00000000aef55a76 RBX: 0000000000000000 RCX: 000000009d6e7dcd
<4>[ 1084.119164] RDX: 00000000ba9f4f5f RSI: ffff883f63f14d00 RDI: ffff883f7f0035ec
<4>[ 1084.119333] RBP: ffff883f7f003668 R08: 0000000000000003 R09: 00000000c8cfdbe1
<4>[ 1084.119506] R10: ffff883f61206042 R11: ffff883f7f0035c0 R12: 00000000ffffffff
<4>[ 1084.119679] R13: ffff883f657b00c0 R14: ffff883f5d920000 R15: 00000000f0000012
<4>[ 1084.119850] FS:  0000000000000000(0000) GS:ffff883f7f000000(0000) knlGS:0000000000000000
<4>[ 1084.120171] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
<4>[ 1084.120338] CR2: ffff8840c365b5c4 CR3: 00000000019ea000 CR4: 00000000003406f0
<4>[ 1084.120509] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
<4>[ 1084.120678] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
<4>[ 1084.120847] Stack:
<4>[ 1084.121006]  ffff883f63f14d00 ffff883f63f14d00 000000000000000e 0000000000000000
<4>[ 1084.121339]  ffff883f5d920000 ffff883f60a7f840 ffff883f7f0036a0 ffffffffa00fbed4
<4>[ 1084.121672]  ffff883f603612ac ffff883f5d920000 ffff883f63f14d00 0000000000000000
<4>[ 1084.122006] Call Trace:
<4>[ 1084.122168]  <IRQ>
<4>[ 1084.122193]  [<ffffffffa00fbed4>] ixgbe_select_queue+0xc4/0x150 [ixgbe]
<4>[ 1084.122519]  [<ffffffff8159111e>] netdev_pick_tx+0x5e/0xf0
<4>[ 1084.122687]  [<ffffffff81591252>] __dev_queue_xmit+0xa2/0x560
<4>[ 1084.122856]  [<ffffffff81591720>] dev_queue_xmit+0x10/0x20
<4>[ 1084.123034]  [<ffffffffa05e93a2>] bond_dev_queue_xmit+0x32/0x80 [bonding]
<4>[ 1084.123207]  [<ffffffffa05eb0d6>] bond_start_xmit+0x1a6/0x3f0 [bonding]
<4>[ 1084.123382]  [<ffffffff8124faa5>] ? ep_poll_callback+0xb5/0x160
<4>[ 1084.123551]  [<ffffffff81590f08>] dev_hard_start_xmit+0x238/0x3f0
<4>[ 1084.123721]  [<ffffffff815908cf>] ? netif_skb_features+0xff/0x200
<4>[ 1084.123890]  [<ffffffff815915f2>] __dev_queue_xmit+0x442/0x560
<4>[ 1084.124059]  [<ffffffff81591720>] dev_queue_xmit+0x10/0x20
<4>[ 1084.124232]  [<ffffffffa04fe70a>] ovs_vport_send+0x4a/0xc0 [openvswitch]
<4>[ 1084.124404]  [<ffffffffa04f1263>] do_output.isra.30+0x43/0x160 [openvswitch]
<4>[ 1084.124575]  [<ffffffff81579c5e>] ? __skb_clone+0x2e/0x140
<4>[ 1084.124744]  [<ffffffffa04f25c4>] do_execute_actions+0x684/0x7e0 [openvswitch]
<4>[ 1084.125067]  [<ffffffffa04f2752>] ovs_execute_actions+0x32/0xd0 [openvswitch]
<4>[ 1084.125240]  [<ffffffffa04f5ed4>] ovs_dp_process_packet+0x84/0x110 [openvswitch]
<4>[ 1084.125565]  [<ffffffffa04fdfec>] ovs_vport_receive+0x6c/0xd0 [openvswitch]
<4>[ 1084.125740]  [<ffffffff810b1645>] ? check_preempt_curr+0x75/0x90
<4>[ 1084.125912]  [<ffffffff810b1679>] ? ttwu_do_wakeup+0x19/0xe0
<4>[ 1084.126081]  [<ffffffff810b195d>] ? ttwu_do_activate.constprop.95+0x5d/0x70
<4>[ 1084.126252]  [<ffffffff810b23c7>] ? try_to_wake_up+0x47/0x340
<4>[ 1084.126427]  [<ffffffff810b2772>] ? default_wake_function+0x12/0x20
<4>[ 1084.126600]  [<ffffffff810ca51b>] ? autoremove_wake_function+0x2b/0x40
<4>[ 1084.126773]  [<ffffffffa04ff127>] netdev_frame_hook+0xe7/0x150 [openvswitch]
<4>[ 1084.126945]  [<ffffffff8158e840>] __netif_receive_skb_core+0x1e0/0x9e0
<4>[ 1084.127115]  [<ffffffff8167d4e6>] ? ipv6_gro_receive+0x246/0x360
<4>[ 1084.127284]  [<ffffffff8158f058>] __netif_receive_skb+0x18/0x60
<4>[ 1084.127453]  [<ffffffff8158f0e0>] netif_receive_skb_internal+0x40/0xb0
<4>[ 1084.127623]  [<ffffffff8158fd23>] napi_gro_receive+0xc3/0x110
<4>[ 1084.127813]  [<ffffffffa01e41fc>] bnx2x_rx_int+0x101c/0x19d0 [bnx2x]
<4>[ 1084.127984]  [<ffffffff810c37e3>] ? load_balance+0x163/0x8d0
<4>[ 1084.128166]  [<ffffffffa01e6a64>] bnx2x_poll+0x284/0x340 [bnx2x]
<4>[ 1084.128334]  [<ffffffff8158f4eb>] net_rx_action+0x16b/0x370
<4>[ 1084.128503]  [<ffffffff8108c032>] __do_softirq+0xe2/0x2e0
<4>[ 1084.128671]  [<ffffffff8108c4d5>] irq_exit+0xf5/0x100
<4>[ 1084.128843]  [<ffffffff816a0b06>] do_IRQ+0x56/0xd0
<4>[ 1084.129010]  [<ffffffff8169eb47>] common_interrupt+0x87/0x87
<4>[ 1084.129176]  <EOI>
<4>[ 1084.129188]  [<ffffffff8153e168>] ? cpuidle_enter_state+0xd8/0x250
<4>[ 1084.129510]  [<ffffffff8153e144>] ? cpuidle_enter_state+0xb4/0x250
<4>[ 1084.129681]  [<ffffffff8153e317>] cpuidle_enter+0x17/0x20
<4>[ 1084.129849]  [<ffffffff810ca832>] call_cpuidle+0x32/0x60
<4>[ 1084.130016]  [<ffffffff8153e2f3>] ? cpuidle_select+0x13/0x20
<4>[ 1084.130184]  [<ffffffff810caaf9>] cpu_startup_entry+0x299/0x360
<4>[ 1084.130354]  [<ffffffff8169201c>] rest_init+0x7c/0x80
<4>[ 1084.130521]  [<ffffffff81b5716a>] start_kernel+0x4cf/0x4f0
<4>[ 1084.134763]  [<ffffffff81b56a86>] ? set_init_arg+0x55/0x55
<4>[ 1084.134931]  [<ffffffff81b56120>] ? early_idt_handler_array+0x120/0x120
<4>[ 1084.135101]  [<ffffffff81b565ee>] x86_64_start_reservations+0x2a/0x2c
<4>[ 1084.135269]  [<ffffffff81b5673c>] x86_64_start_kernel+0x14c/0x16f
<4>[ 1084.135437] Code: 8b 7d 00 41 83 ff 01 0f 84 8b 00 00 00 f6 86 91 00 00 00 30 0f 84 85 00 00 00 8b 96 a4 00 00 00 44 89 f8 48 0f af c2 48 c1 e8 20 <41> 0f b7 44 45 18 41 3b 86 cc 03 00
 00 0f 83 81 00 00 00 44 39
<1>[ 1084.136184] RIP  [<ffffffff81589802>] __netdev_pick_tx+0x92/0x140
<4>[ 1084.136357]  RSP <ffff883f7f003638>
<4>[ 1084.136518] CR2: ffff8840c365b5c4
<4>[ 1084.137174] ---[ end trace 17b59260de82e18d ]---
<0>[ 1084.212189] Kernel panic - not syncing: Fatal exception in interrupt
<0>[ 1084.212482] Kernel Offset: disabled


I've bisected this to following commit:

commit 52bd2d62ce6758d811edcbd2256eb9ea7f6a56cb
Author: Eric Dumazet <edumazet@google.com>
Date:   Wed Nov 18 06:30:50 2015 -0800

    net: better skb->sender_cpu and skb->napi_id cohabitation
    
    skb->sender_cpu and skb->napi_id share a common storage,
    and we had various bugs about this.
    
    We had to call skb_sender_cpu_clear() in some places to
    not leave a prior skb->napi_id and fool netdev_pick_tx()
    
    As suggested by Alexei, we could split the space so that
    these errors can not happen.
    
    0 value being reserved as the common (not initialized) value,
    let's reserve [1 .. NR_CPUS] range for valid sender_cpu,
    and [NR_CPUS+1 .. ~0U] for valid napi_id.
    
    This will allow proper busy polling support over tunnels.


I'm by no means kernel developer and it doesn't make any sense
to me why this patch should be fixing it, but it is.. I've confirmed
it multiple times, that 4.4.32 without the patch crashes within
minutes, with it applied (it applies cleanly), it's rock solid.

therefore I'd probably like to propose this patch to -stable,
but I'd like to hear you, -netdev people opinion, especially
Erics..

what do you think about it?

thanks a lot in advance for reply

BR

nik







-- 
-------------------------------------
Ing. Nikola CIPRICH
LinuxBox.cz, s.r.o.
28.rijna 168, 709 00 Ostrava

tel.:   +420 591 166 214
fax:    +420 596 621 273
mobil:  +420 777 093 799
www.linuxbox.cz

mobil servis: +420 737 238 656
email servis: servis@linuxbox.cz
-------------------------------------

^ permalink raw reply

* Getting a handle on all these new NIC features
From: Tom Herbert @ 2017-01-17 22:05 UTC (permalink / raw)
  To: Linux Kernel Network Developers

There was some discussion about the problems of dealing with the
explosion of NIC features in the mlx directory restructuring proposal,
but I think the is a deeper issue here that should be discussed.

It's hard not to notice that there has been quite a proliferation of
NIC features in several drivers. This trend had resulted in very
complex driver code that may or may not segment individual features.
One visible manifestation of this is number of ndo functions which is
somewhere around seventy-five now.

I suspect the vast majority of these advances NIC features (e.g.
bridging, UDP offloads, tc offload, etc.) are only relevant to some of
the people some of the time. The problem we have, in this case those
of us that are attempting to deploy and maintain NICs at scale, is
when we have to deal with the ramifications of these features being
intertwined with core driver functionality that is relevant to
everyone. This becomes very obvious when we need to backport drivers
from later versions of kernel.

I realize that backports of a driver is not a specific concern of the
Linux kernel, but nevertheless this is a real problem and a fact of
life for many users. Rebasing the full kernel is still a major effort
and it seems the best we could ever do is one rebase per year. In the
interim we need to occasionally backport drivers. Backporting drivers
is difficult precisely because of new features or API changes to
existing ones. These sort of changes tend to have a spiderweb of
dependencies in other parts of the stack so that the number of patches
we need to cherry-pick goes way beyond those that touch the driver we
are interested in.

Currently we (FB) need to backport two NIC drivers. I've already gave
details of backporting mlx5 on the thread to restructure the driver
directories. The other driver being backporting seems to suffer from
the same type of feature complexity.

In short, I would like to ask if driver maintainers to start to
modularize driver features. If something being added is obviously a
narrow feature that only a subset of users will need can we allow
config options to #ifdef those out somehow? Furthermore can the file
and directory structure of drivers reflect that; our lives would be
_so_ much simpler to maintain drivers in production if we have such
modularity and the ability to build drivers with the features of our
choosing.

Thanks,
Tom

^ permalink raw reply

* [net PATCH v5 1/6] virtio_net: use dev_kfree_skb for small buffer XDP receive
From: John Fastabend @ 2017-01-17 22:19 UTC (permalink / raw)
  To: jasowang, mst
  Cc: john.r.fastabend, netdev, john.fastabend, alexei.starovoitov,
	daniel
In-Reply-To: <20170117221443.20280.62546.stgit@john-Precision-Tower-5810>

In the small buffer case during driver unload we currently use
put_page instead of dev_kfree_skb. Resolve this by adding a check
for virtnet mode when checking XDP queue type. Also name the
function so that the code reads correctly to match the additional
check.

Fixes: bb91accf2733 ("virtio-net: XDP support for small buffers")
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
Acked-by: Jason Wang <jasowang@redhat.com>
---
 drivers/net/virtio_net.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 4a10500..d97bb71 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1890,8 +1890,12 @@ static void free_receive_page_frags(struct virtnet_info *vi)
 			put_page(vi->rq[i].alloc_frag.page);
 }
 
-static bool is_xdp_queue(struct virtnet_info *vi, int q)
+static bool is_xdp_raw_buffer_queue(struct virtnet_info *vi, int q)
 {
+	/* For small receive mode always use kfree_skb variants */
+	if (!vi->mergeable_rx_bufs)
+		return false;
+
 	if (q < (vi->curr_queue_pairs - vi->xdp_queue_pairs))
 		return false;
 	else if (q < vi->curr_queue_pairs)
@@ -1908,7 +1912,7 @@ static void free_unused_bufs(struct virtnet_info *vi)
 	for (i = 0; i < vi->max_queue_pairs; i++) {
 		struct virtqueue *vq = vi->sq[i].vq;
 		while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
-			if (!is_xdp_queue(vi, i))
+			if (!is_xdp_raw_buffer_queue(vi, i))
 				dev_kfree_skb(buf);
 			else
 				put_page(virt_to_head_page(buf));

^ permalink raw reply related

* [net PATCH v5 2/6] virtio_net: wrap rtnl_lock in test for calling with lock already held
From: John Fastabend @ 2017-01-17 22:20 UTC (permalink / raw)
  To: jasowang, mst
  Cc: john.r.fastabend, netdev, john.fastabend, alexei.starovoitov,
	daniel
In-Reply-To: <20170117221443.20280.62546.stgit@john-Precision-Tower-5810>

For XDP use case and to allow ethtool reset tests it is useful to be
able to use reset paths from contexts where rtnl lock is already
held.

This requries updating virtnet_set_queues and free_receive_bufs the
two places where rtnl_lock is taken in virtio_net. To do this we
use the following pattern,

	_foo(...) { do stuff }
	foo(...) { rtnl_lock(); _foo(...); rtnl_unlock()};

this allows us to use freeze()/restore() flow from both contexts.

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---
 drivers/net/virtio_net.c |   31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index d97bb71..ba0efee 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1331,7 +1331,7 @@ static void virtnet_ack_link_announce(struct virtnet_info *vi)
 	rtnl_unlock();
 }
 
-static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
+static int _virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
 {
 	struct scatterlist sg;
 	struct net_device *dev = vi->dev;
@@ -1357,6 +1357,16 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
 	return 0;
 }
 
+static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
+{
+	int err;
+
+	rtnl_lock();
+	err = _virtnet_set_queues(vi, queue_pairs);
+	rtnl_unlock();
+	return err;
+}
+
 static int virtnet_close(struct net_device *dev)
 {
 	struct virtnet_info *vi = netdev_priv(dev);
@@ -1609,7 +1619,7 @@ static int virtnet_set_channels(struct net_device *dev,
 		return -EINVAL;
 
 	get_online_cpus();
-	err = virtnet_set_queues(vi, queue_pairs);
+	err = _virtnet_set_queues(vi, queue_pairs);
 	if (!err) {
 		netif_set_real_num_tx_queues(dev, queue_pairs);
 		netif_set_real_num_rx_queues(dev, queue_pairs);
@@ -1736,7 +1746,7 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog)
 		return -ENOMEM;
 	}
 
-	err = virtnet_set_queues(vi, curr_qp + xdp_qp);
+	err = _virtnet_set_queues(vi, curr_qp + xdp_qp);
 	if (err) {
 		dev_warn(&dev->dev, "XDP Device queue allocation failure.\n");
 		return err;
@@ -1745,7 +1755,7 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog)
 	if (prog) {
 		prog = bpf_prog_add(prog, vi->max_queue_pairs - 1);
 		if (IS_ERR(prog)) {
-			virtnet_set_queues(vi, curr_qp);
+			_virtnet_set_queues(vi, curr_qp);
 			return PTR_ERR(prog);
 		}
 	}
@@ -1864,12 +1874,11 @@ static void virtnet_free_queues(struct virtnet_info *vi)
 	kfree(vi->sq);
 }
 
-static void free_receive_bufs(struct virtnet_info *vi)
+static void _free_receive_bufs(struct virtnet_info *vi)
 {
 	struct bpf_prog *old_prog;
 	int i;
 
-	rtnl_lock();
 	for (i = 0; i < vi->max_queue_pairs; i++) {
 		while (vi->rq[i].pages)
 			__free_pages(get_a_page(&vi->rq[i], GFP_KERNEL), 0);
@@ -1879,6 +1888,12 @@ static void free_receive_bufs(struct virtnet_info *vi)
 		if (old_prog)
 			bpf_prog_put(old_prog);
 	}
+}
+
+static void free_receive_bufs(struct virtnet_info *vi)
+{
+	rtnl_lock();
+	_free_receive_bufs(vi);
 	rtnl_unlock();
 }
 
@@ -2317,9 +2332,7 @@ static int virtnet_probe(struct virtio_device *vdev)
 		goto free_unregister_netdev;
 	}
 
-	rtnl_lock();
 	virtnet_set_queues(vi, vi->curr_queue_pairs);
-	rtnl_unlock();
 
 	/* Assume link up if device can't report link status,
 	   otherwise get link status from config. */
@@ -2428,9 +2441,7 @@ static int virtnet_restore(struct virtio_device *vdev)
 
 	netif_device_attach(vi->dev);
 
-	rtnl_lock();
 	virtnet_set_queues(vi, vi->curr_queue_pairs);
-	rtnl_unlock();
 
 	err = virtnet_cpu_notif_add(vi);
 	if (err)

^ permalink raw reply related

* [net PATCH v5 3/6] virtio_net: factor out xdp handler for readability
From: John Fastabend @ 2017-01-17 22:21 UTC (permalink / raw)
  To: jasowang, mst
  Cc: john.r.fastabend, netdev, john.fastabend, alexei.starovoitov,
	daniel
In-Reply-To: <20170117221443.20280.62546.stgit@john-Precision-Tower-5810>

At this point the do_xdp_prog is mostly if/else branches handling
the different modes of virtio_net. So remove it and handle running
the program in the per mode handlers.

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---
 drivers/net/virtio_net.c |   75 +++++++++++++++++-----------------------------
 1 file changed, 27 insertions(+), 48 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index ba0efee..6de0cbe 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -388,49 +388,6 @@ static void virtnet_xdp_xmit(struct virtnet_info *vi,
 	virtqueue_kick(sq->vq);
 }
 
-static u32 do_xdp_prog(struct virtnet_info *vi,
-		       struct receive_queue *rq,
-		       struct bpf_prog *xdp_prog,
-		       void *data, int len)
-{
-	int hdr_padded_len;
-	struct xdp_buff xdp;
-	void *buf;
-	unsigned int qp;
-	u32 act;
-
-	if (vi->mergeable_rx_bufs) {
-		hdr_padded_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
-		xdp.data = data + hdr_padded_len;
-		xdp.data_end = xdp.data + (len - vi->hdr_len);
-		buf = data;
-	} else { /* small buffers */
-		struct sk_buff *skb = data;
-
-		xdp.data = skb->data;
-		xdp.data_end = xdp.data + len;
-		buf = skb->data;
-	}
-
-	act = bpf_prog_run_xdp(xdp_prog, &xdp);
-	switch (act) {
-	case XDP_PASS:
-		return XDP_PASS;
-	case XDP_TX:
-		qp = vi->curr_queue_pairs -
-			vi->xdp_queue_pairs +
-			smp_processor_id();
-		xdp.data = buf;
-		virtnet_xdp_xmit(vi, rq, &vi->sq[qp], &xdp, data);
-		return XDP_TX;
-	default:
-		bpf_warn_invalid_xdp_action(act);
-	case XDP_ABORTED:
-	case XDP_DROP:
-		return XDP_DROP;
-	}
-}
-
 static struct sk_buff *receive_small(struct net_device *dev,
 				     struct virtnet_info *vi,
 				     struct receive_queue *rq,
@@ -446,19 +403,30 @@ static struct sk_buff *receive_small(struct net_device *dev,
 	xdp_prog = rcu_dereference(rq->xdp_prog);
 	if (xdp_prog) {
 		struct virtio_net_hdr_mrg_rxbuf *hdr = buf;
+		struct xdp_buff xdp;
+		unsigned int qp;
 		u32 act;
 
 		if (unlikely(hdr->hdr.gso_type || hdr->hdr.flags))
 			goto err_xdp;
-		act = do_xdp_prog(vi, rq, xdp_prog, skb, len);
+
+		xdp.data = skb->data;
+		xdp.data_end = xdp.data + len;
+		act = bpf_prog_run_xdp(xdp_prog, &xdp);
 		switch (act) {
 		case XDP_PASS:
 			break;
 		case XDP_TX:
+			qp = vi->curr_queue_pairs -
+				vi->xdp_queue_pairs +
+				smp_processor_id();
+			virtnet_xdp_xmit(vi, rq, &vi->sq[qp], &xdp, skb);
 			rcu_read_unlock();
 			goto xdp_xmit;
-		case XDP_DROP:
 		default:
+			bpf_warn_invalid_xdp_action(act);
+		case XDP_ABORTED:
+		case XDP_DROP:
 			goto err_xdp;
 		}
 	}
@@ -576,6 +544,9 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
 	xdp_prog = rcu_dereference(rq->xdp_prog);
 	if (xdp_prog) {
 		struct page *xdp_page;
+		struct xdp_buff xdp;
+		unsigned int qp;
+		void *data;
 		u32 act;
 
 		/* This happens when rx buffer size is underestimated */
@@ -598,8 +569,10 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
 		if (unlikely(hdr->hdr.gso_type))
 			goto err_xdp;
 
-		act = do_xdp_prog(vi, rq, xdp_prog,
-				  page_address(xdp_page) + offset, len);
+		data = page_address(xdp_page) + offset;
+		xdp.data = data + vi->hdr_len;
+		xdp.data_end = xdp.data + (len - vi->hdr_len);
+		act = bpf_prog_run_xdp(xdp_prog, &xdp);
 		switch (act) {
 		case XDP_PASS:
 			/* We can only create skb based on xdp_page. */
@@ -613,13 +586,19 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
 			}
 			break;
 		case XDP_TX:
+			qp = vi->curr_queue_pairs -
+				vi->xdp_queue_pairs +
+				smp_processor_id();
+			virtnet_xdp_xmit(vi, rq, &vi->sq[qp], &xdp, data);
 			ewma_pkt_len_add(&rq->mrg_avg_pkt_len, len);
 			if (unlikely(xdp_page != page))
 				goto err_xdp;
 			rcu_read_unlock();
 			goto xdp_xmit;
-		case XDP_DROP:
 		default:
+			bpf_warn_invalid_xdp_action(act);
+		case XDP_ABORTED:
+		case XDP_DROP:
 			if (unlikely(xdp_page != page))
 				__free_pages(xdp_page, 0);
 			ewma_pkt_len_add(&rq->mrg_avg_pkt_len, len);

^ permalink raw reply related

* [net PATCH v5 0/6] virtio_net XDP fixes and adjust_header support
From: John Fastabend @ 2017-01-17 22:19 UTC (permalink / raw)
  To: jasowang, mst
  Cc: john.r.fastabend, netdev, john.fastabend, alexei.starovoitov,
	daniel

This has a fix to handle small buffer free logic correctly and then
also adds adjust head support.

I pushed adjust head at net (even though its rc3) to avoid having
to push another exception case into virtio_net to catch if the
program uses adjust_head and then block it. If there are any strong
objections to this we can push it at net-next and use a patch from
Jakub to add the exception handling but then user space has to deal
with it either via try/fail logic or via kernel version checks. Granted
we already have some cases that need to be configured to enable XDP
but I don't see any reason to have yet another one when we can fix it
now vs delaying a kernel version.


v2: fix spelling error, convert unsigned -> unsigned int
v3: v2 git crashed during send so retrying sorry for the noise
v4: changed layout of rtnl_lock fixes (Stephen)
    moved reset logic into virtio core with new patch (MST)
    fixed up linearize and some code cleanup (Jason)

    Otherwise did some generic code cleanup so might be a bit
    cleaner this time at least that is the hope.
v5: fixed rtnl_lock issue (DaveM)

    In order to fix rtnl_lock issue and also to address Jason's
    comment questioning the need for a generic virtio_device_reset
    routine I exported some virtio core routines and then wrote
    virtio_net reset routine. This is the cleanest solution I
    came up with today and I do not at this time have any need
    for a more generic reset. If folks don't like this I could
    revert back to v3 variant but Stephen pointed out that the
    pattern used there is also not ideal.

Thanks for the review.

---

John Fastabend (6):
      virtio_net: use dev_kfree_skb for small buffer XDP receive
      virtio_net: wrap rtnl_lock in test for calling with lock already held
      virtio_net: factor out xdp handler for readability
      virtio_net: remove duplicate queue pair binding in XDP
      virtio_net: refactor freeze/restore logic into virtnet reset logic
      virtio_net: XDP support for adjust_head


 drivers/net/virtio_net.c |  332 ++++++++++++++++++++++++++++++----------------
 drivers/virtio/virtio.c  |   42 +++---
 include/linux/virtio.h   |    4 +
 3 files changed, 247 insertions(+), 131 deletions(-)

^ permalink raw reply

* [net PATCH v5 6/6] virtio_net: XDP support for adjust_head
From: John Fastabend @ 2017-01-17 22:22 UTC (permalink / raw)
  To: jasowang, mst
  Cc: john.r.fastabend, netdev, john.fastabend, alexei.starovoitov,
	daniel
In-Reply-To: <20170117221443.20280.62546.stgit@john-Precision-Tower-5810>

Add support for XDP adjust head by allocating a 256B header region
that XDP programs can grow into. This is only enabled when a XDP
program is loaded.

In order to ensure that we do not have to unwind queue headroom push
queue setup below bpf_prog_add. It reads better to do a prog ref
unwind vs another queue setup call.

At the moment this code must do a full reset to ensure old buffers
without headroom on program add or with headroom on program removal
are not used incorrectly in the datapath. Ideally we would only
have to disable/enable the RX queues being updated but there is no
API to do this at the moment in virtio so use the big hammer. In
practice it is likely not that big of a problem as this will only
happen when XDP is enabled/disabled changing programs does not
require the reset. There is some risk that the driver may either
have an allocation failure or for some reason fail to correctly
negotiate with the underlying backend in this case the driver will
be left uninitialized. I have not seen this ever happen on my test
systems and for what its worth this same failure case can occur
from probe and other contexts in virtio framework.

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---
 drivers/net/virtio_net.c |  149 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 125 insertions(+), 24 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 62dbf4b..3b129b4 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -41,6 +41,9 @@
 #define GOOD_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
 #define GOOD_COPY_LEN	128
 
+/* Amount of XDP headroom to prepend to packets for use by xdp_adjust_head */
+#define VIRTIO_XDP_HEADROOM 256
+
 /* RX packet size EWMA. The average packet size is used to determine the packet
  * buffer size when refilling RX rings. As the entire RX ring may be refilled
  * at once, the weight is chosen so that the EWMA will be insensitive to short-
@@ -359,6 +362,7 @@ static void virtnet_xdp_xmit(struct virtnet_info *vi,
 	}
 
 	if (vi->mergeable_rx_bufs) {
+		xdp->data -= sizeof(struct virtio_net_hdr_mrg_rxbuf);
 		/* Zero header and leave csum up to XDP layers */
 		hdr = xdp->data;
 		memset(hdr, 0, vi->hdr_len);
@@ -375,7 +379,9 @@ static void virtnet_xdp_xmit(struct virtnet_info *vi,
 		num_sg = 2;
 		sg_init_table(sq->sg, 2);
 		sg_set_buf(sq->sg, hdr, vi->hdr_len);
-		skb_to_sgvec(skb, sq->sg + 1, 0, skb->len);
+		skb_to_sgvec(skb, sq->sg + 1,
+			     xdp->data - xdp->data_hard_start,
+			     xdp->data_end - xdp->data);
 	}
 	err = virtqueue_add_outbuf(sq->vq, sq->sg, num_sg,
 				   data, GFP_ATOMIC);
@@ -401,7 +407,6 @@ static struct sk_buff *receive_small(struct net_device *dev,
 	struct bpf_prog *xdp_prog;
 
 	len -= vi->hdr_len;
-	skb_trim(skb, len);
 
 	rcu_read_lock();
 	xdp_prog = rcu_dereference(rq->xdp_prog);
@@ -413,11 +418,15 @@ static struct sk_buff *receive_small(struct net_device *dev,
 		if (unlikely(hdr->hdr.gso_type || hdr->hdr.flags))
 			goto err_xdp;
 
-		xdp.data = skb->data;
+		xdp.data_hard_start = skb->data;
+		xdp.data = skb->data + VIRTIO_XDP_HEADROOM;
 		xdp.data_end = xdp.data + len;
 		act = bpf_prog_run_xdp(xdp_prog, &xdp);
 		switch (act) {
 		case XDP_PASS:
+			/* Recalculate length in case bpf program changed it */
+			__skb_pull(skb, xdp.data - xdp.data_hard_start);
+			len = xdp.data_end - xdp.data;
 			break;
 		case XDP_TX:
 			virtnet_xdp_xmit(vi, rq, &xdp, skb);
@@ -432,6 +441,7 @@ static struct sk_buff *receive_small(struct net_device *dev,
 	}
 	rcu_read_unlock();
 
+	skb_trim(skb, len);
 	return skb;
 
 err_xdp:
@@ -480,7 +490,7 @@ static struct page *xdp_linearize_page(struct receive_queue *rq,
 				       unsigned int *len)
 {
 	struct page *page = alloc_page(GFP_ATOMIC);
-	unsigned int page_off = 0;
+	unsigned int page_off = VIRTIO_XDP_HEADROOM;
 
 	if (!page)
 		return NULL;
@@ -516,7 +526,8 @@ static struct page *xdp_linearize_page(struct receive_queue *rq,
 		put_page(p);
 	}
 
-	*len = page_off;
+	/* Headroom does not contribute to packet length */
+	*len = page_off - VIRTIO_XDP_HEADROOM;
 	return page;
 err_buf:
 	__free_pages(page, 0);
@@ -555,7 +566,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
 						      page, offset, &len);
 			if (!xdp_page)
 				goto err_xdp;
-			offset = 0;
+			offset = VIRTIO_XDP_HEADROOM;
 		} else {
 			xdp_page = page;
 		}
@@ -568,18 +579,29 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
 		if (unlikely(hdr->hdr.gso_type))
 			goto err_xdp;
 
+		/* Allow consuming headroom but reserve enough space to push
+		 * the descriptor on if we get an XDP_TX return code.
+		 */
 		data = page_address(xdp_page) + offset;
+		xdp.data_hard_start = data - VIRTIO_XDP_HEADROOM + vi->hdr_len;
 		xdp.data = data + vi->hdr_len;
 		xdp.data_end = xdp.data + (len - vi->hdr_len);
 		act = bpf_prog_run_xdp(xdp_prog, &xdp);
 		switch (act) {
 		case XDP_PASS:
+			/* recalculate offset to account for any header
+			 * adjustments. Note other cases do not build an
+			 * skb and avoid using offset
+			 */
+			offset = xdp.data -
+					page_address(xdp_page) - vi->hdr_len;
+
 			/* We can only create skb based on xdp_page. */
 			if (unlikely(xdp_page != page)) {
 				rcu_read_unlock();
 				put_page(page);
 				head_skb = page_to_skb(vi, rq, xdp_page,
-						       0, len, PAGE_SIZE);
+						       offset, len, PAGE_SIZE);
 				ewma_pkt_len_add(&rq->mrg_avg_pkt_len, len);
 				return head_skb;
 			}
@@ -744,23 +766,30 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
 	dev_kfree_skb(skb);
 }
 
+static unsigned int virtnet_get_headroom(struct virtnet_info *vi)
+{
+	return vi->xdp_queue_pairs ? VIRTIO_XDP_HEADROOM : 0;
+}
+
 static int add_recvbuf_small(struct virtnet_info *vi, struct receive_queue *rq,
 			     gfp_t gfp)
 {
+	int headroom = GOOD_PACKET_LEN + virtnet_get_headroom(vi);
+	unsigned int xdp_headroom = virtnet_get_headroom(vi);
 	struct sk_buff *skb;
 	struct virtio_net_hdr_mrg_rxbuf *hdr;
 	int err;
 
-	skb = __netdev_alloc_skb_ip_align(vi->dev, GOOD_PACKET_LEN, gfp);
+	skb = __netdev_alloc_skb_ip_align(vi->dev, headroom, gfp);
 	if (unlikely(!skb))
 		return -ENOMEM;
 
-	skb_put(skb, GOOD_PACKET_LEN);
+	skb_put(skb, headroom);
 
 	hdr = skb_vnet_hdr(skb);
 	sg_init_table(rq->sg, 2);
 	sg_set_buf(rq->sg, hdr, vi->hdr_len);
-	skb_to_sgvec(skb, rq->sg + 1, 0, skb->len);
+	skb_to_sgvec(skb, rq->sg + 1, xdp_headroom, skb->len - xdp_headroom);
 
 	err = virtqueue_add_inbuf(rq->vq, rq->sg, 2, skb, gfp);
 	if (err < 0)
@@ -828,24 +857,27 @@ static unsigned int get_mergeable_buf_len(struct ewma_pkt_len *avg_pkt_len)
 	return ALIGN(len, MERGEABLE_BUFFER_ALIGN);
 }
 
-static int add_recvbuf_mergeable(struct receive_queue *rq, gfp_t gfp)
+static int add_recvbuf_mergeable(struct virtnet_info *vi,
+				 struct receive_queue *rq, gfp_t gfp)
 {
 	struct page_frag *alloc_frag = &rq->alloc_frag;
+	unsigned int headroom = virtnet_get_headroom(vi);
 	char *buf;
 	unsigned long ctx;
 	int err;
 	unsigned int len, hole;
 
 	len = get_mergeable_buf_len(&rq->mrg_avg_pkt_len);
-	if (unlikely(!skb_page_frag_refill(len, alloc_frag, gfp)))
+	if (unlikely(!skb_page_frag_refill(len + headroom, alloc_frag, gfp)))
 		return -ENOMEM;
 
 	buf = (char *)page_address(alloc_frag->page) + alloc_frag->offset;
+	buf += headroom; /* advance address leaving hole at front of pkt */
 	ctx = mergeable_buf_to_ctx(buf, len);
 	get_page(alloc_frag->page);
-	alloc_frag->offset += len;
+	alloc_frag->offset += len + headroom;
 	hole = alloc_frag->size - alloc_frag->offset;
-	if (hole < len) {
+	if (hole < len + headroom) {
 		/* To avoid internal fragmentation, if there is very likely not
 		 * enough space for another buffer, add the remaining space to
 		 * the current buffer. This extra space is not included in
@@ -879,7 +911,7 @@ static bool try_fill_recv(struct virtnet_info *vi, struct receive_queue *rq,
 	gfp |= __GFP_COLD;
 	do {
 		if (vi->mergeable_rx_bufs)
-			err = add_recvbuf_mergeable(rq, gfp);
+			err = add_recvbuf_mergeable(vi, rq, gfp);
 		else if (vi->big_packets)
 			err = add_recvbuf_big(vi, rq, gfp);
 		else
@@ -1702,6 +1734,7 @@ static void virtnet_freeze_down(struct virtio_device *vdev)
 }
 
 static int init_vqs(struct virtnet_info *vi);
+static void _remove_vq_common(struct virtnet_info *vi);
 
 static int virtnet_restore_up(struct virtio_device *vdev)
 {
@@ -1727,12 +1760,45 @@ static int virtnet_restore_up(struct virtio_device *vdev)
 	return err;
 }
 
+static int virtnet_reset(struct virtnet_info *vi)
+{
+	struct virtio_device *dev = vi->vdev;
+	int ret;
+
+	virtio_config_disable(dev);
+	dev->failed = dev->config->get_status(dev) & VIRTIO_CONFIG_S_FAILED;
+	virtnet_freeze_down(dev);
+	_remove_vq_common(vi);
+
+	dev->config->reset(dev);
+	virtio_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);
+	virtio_add_status(dev, VIRTIO_CONFIG_S_DRIVER);
+
+	ret = virtio_finalize_features(dev);
+	if (ret)
+		goto err;
+
+	ret = virtnet_restore_up(dev);
+	if (ret)
+		goto err;
+	ret = _virtnet_set_queues(vi, vi->curr_queue_pairs);
+	if (ret)
+		goto err;
+
+	virtio_add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
+	virtio_config_enable(dev);
+	return 0;
+err:
+	virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED);
+	return ret;
+}
+
 static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog)
 {
 	unsigned long int max_sz = PAGE_SIZE - sizeof(struct padded_vnet_hdr);
 	struct virtnet_info *vi = netdev_priv(dev);
 	struct bpf_prog *old_prog;
-	u16 xdp_qp = 0, curr_qp;
+	u16 oxdp_qp, xdp_qp = 0, curr_qp;
 	int i, err;
 
 	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO4) ||
@@ -1764,21 +1830,32 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog)
 		return -ENOMEM;
 	}
 
+	if (prog) {
+		prog = bpf_prog_add(prog, vi->max_queue_pairs - 1);
+		if (IS_ERR(prog))
+			return PTR_ERR(prog);
+	}
+
 	err = _virtnet_set_queues(vi, curr_qp + xdp_qp);
 	if (err) {
 		dev_warn(&dev->dev, "XDP Device queue allocation failure.\n");
-		return err;
+		goto virtio_queue_err;
 	}
 
-	if (prog) {
-		prog = bpf_prog_add(prog, vi->max_queue_pairs - 1);
-		if (IS_ERR(prog)) {
-			_virtnet_set_queues(vi, curr_qp);
-			return PTR_ERR(prog);
-		}
+	oxdp_qp = vi->xdp_queue_pairs;
+
+	/* Changing the headroom in buffers is a disruptive operation because
+	 * existing buffers must be flushed and reallocated. This will happen
+	 * when a xdp program is initially added or xdp is disabled by removing
+	 * the xdp program resulting in number of XDP queues changing.
+	 */
+	if (vi->xdp_queue_pairs != xdp_qp) {
+		vi->xdp_queue_pairs = xdp_qp;
+		err = virtnet_reset(vi);
+		if (err)
+			goto virtio_reset_err;
 	}
 
-	vi->xdp_queue_pairs = xdp_qp;
 	netif_set_real_num_rx_queues(dev, curr_qp + xdp_qp);
 
 	for (i = 0; i < vi->max_queue_pairs; i++) {
@@ -1789,6 +1866,21 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog)
 	}
 
 	return 0;
+
+virtio_reset_err:
+	/* On reset error do our best to unwind XDP changes inflight and return
+	 * error up to user space for resolution. The underlying reset hung on
+	 * us so not much we can do here.
+	 */
+	dev_warn(&dev->dev, "XDP reset failure and queues unstable\n");
+	vi->xdp_queue_pairs = oxdp_qp;
+virtio_queue_err:
+	/* On queue set error we can unwind bpf ref count and user space can
+	 * retry this is most likely an allocation failure.
+	 */
+	if (prog)
+		bpf_prog_sub(prog, vi->max_queue_pairs - 1);
+	return err;
 }
 
 static bool virtnet_xdp_query(struct net_device *dev)
@@ -2382,6 +2474,15 @@ static int virtnet_probe(struct virtio_device *vdev)
 	return err;
 }
 
+static void _remove_vq_common(struct virtnet_info *vi)
+{
+	vi->vdev->config->reset(vi->vdev);
+	free_unused_bufs(vi);
+	_free_receive_bufs(vi);
+	free_receive_page_frags(vi);
+	virtnet_del_vqs(vi);
+}
+
 static void remove_vq_common(struct virtnet_info *vi)
 {
 	vi->vdev->config->reset(vi->vdev);

^ permalink raw reply related

* [PATCH] Revert "net: qcom/emac: configure the external phy to allow pause frames"
From: Timur Tabi @ 2017-01-17 22:31 UTC (permalink / raw)
  To: David Miller, netdev, Alok Chauhan

This reverts commit 3e884493448131179a5b7cae1ddca1028ffaecc8.

With commit 529ed1275263 ("net: phy: phy drivers should not set
SUPPORTED_[Asym_]Pause"), phylib now handles automatically enabling
pause frame support in the PHY, and the MAC driver should follow suit.

Since the EMAC driver driver does this,  we no longer need to force
pause frames support.

Signed-off-by: Timur Tabi <timur@codeaurora.org>
---
 drivers/net/ethernet/qualcomm/emac/emac-mac.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/emac/emac-mac.c b/drivers/net/ethernet/qualcomm/emac/emac-mac.c
index 0b4deb3..384e1be 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-mac.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-mac.c
@@ -1004,12 +1004,6 @@ int emac_mac_up(struct emac_adapter *adpt)
 	writel((u32)~DIS_INT, adpt->base + EMAC_INT_STATUS);
 	writel(adpt->irq.mask, adpt->base + EMAC_INT_MASK);
 
-	/* Enable pause frames.  Without this feature, the EMAC has been shown
-	 * to receive (and drop) frames with FCS errors at gigabit connections.
-	 */
-	adpt->phydev->supported |= SUPPORTED_Pause | SUPPORTED_Asym_Pause;
-	adpt->phydev->advertising |= SUPPORTED_Pause | SUPPORTED_Asym_Pause;
-
 	adpt->phydev->irq = PHY_IGNORE_INTERRUPT;
 	phy_start(adpt->phydev);
 
-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply related

* [net PATCH v5 4/6] virtio_net: remove duplicate queue pair binding in XDP
From: John Fastabend @ 2017-01-17 22:21 UTC (permalink / raw)
  To: jasowang, mst
  Cc: john.r.fastabend, netdev, john.fastabend, alexei.starovoitov,
	daniel
In-Reply-To: <20170117221443.20280.62546.stgit@john-Precision-Tower-5810>

Factor out qp assignment.

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---
 drivers/net/virtio_net.c |   18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 6de0cbe..922ca66 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -332,15 +332,19 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
 
 static void virtnet_xdp_xmit(struct virtnet_info *vi,
 			     struct receive_queue *rq,
-			     struct send_queue *sq,
 			     struct xdp_buff *xdp,
 			     void *data)
 {
 	struct virtio_net_hdr_mrg_rxbuf *hdr;
 	unsigned int num_sg, len;
+	struct send_queue *sq;
+	unsigned int qp;
 	void *xdp_sent;
 	int err;
 
+	qp = vi->curr_queue_pairs - vi->xdp_queue_pairs + smp_processor_id();
+	sq = &vi->sq[qp];
+
 	/* Free up any pending old buffers before queueing new ones. */
 	while ((xdp_sent = virtqueue_get_buf(sq->vq, &len)) != NULL) {
 		if (vi->mergeable_rx_bufs) {
@@ -404,7 +408,6 @@ static struct sk_buff *receive_small(struct net_device *dev,
 	if (xdp_prog) {
 		struct virtio_net_hdr_mrg_rxbuf *hdr = buf;
 		struct xdp_buff xdp;
-		unsigned int qp;
 		u32 act;
 
 		if (unlikely(hdr->hdr.gso_type || hdr->hdr.flags))
@@ -417,10 +420,7 @@ static struct sk_buff *receive_small(struct net_device *dev,
 		case XDP_PASS:
 			break;
 		case XDP_TX:
-			qp = vi->curr_queue_pairs -
-				vi->xdp_queue_pairs +
-				smp_processor_id();
-			virtnet_xdp_xmit(vi, rq, &vi->sq[qp], &xdp, skb);
+			virtnet_xdp_xmit(vi, rq, &xdp, skb);
 			rcu_read_unlock();
 			goto xdp_xmit;
 		default:
@@ -545,7 +545,6 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
 	if (xdp_prog) {
 		struct page *xdp_page;
 		struct xdp_buff xdp;
-		unsigned int qp;
 		void *data;
 		u32 act;
 
@@ -586,10 +585,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
 			}
 			break;
 		case XDP_TX:
-			qp = vi->curr_queue_pairs -
-				vi->xdp_queue_pairs +
-				smp_processor_id();
-			virtnet_xdp_xmit(vi, rq, &vi->sq[qp], &xdp, data);
+			virtnet_xdp_xmit(vi, rq, &xdp, data);
 			ewma_pkt_len_add(&rq->mrg_avg_pkt_len, len);
 			if (unlikely(xdp_page != page))
 				goto err_xdp;

^ permalink raw reply related

* [net PATCH v5 5/6] virtio_net: refactor freeze/restore logic into virtnet reset logic
From: John Fastabend @ 2017-01-17 22:22 UTC (permalink / raw)
  To: jasowang, mst
  Cc: john.r.fastabend, netdev, john.fastabend, alexei.starovoitov,
	daniel
In-Reply-To: <20170117221443.20280.62546.stgit@john-Precision-Tower-5810>

For XDP we will need to reset the queues to allow for buffer headroom
to be configured. In order to do this we need to essentially run the
freeze()/restore() code path. Unfortunately the locking requirements
between the freeze/restore and reset paths are different however so
we can not simply reuse the code.

This patch refactors the code path and adds a reset helper routine.

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---
 drivers/net/virtio_net.c |   75 ++++++++++++++++++++++++++++------------------
 drivers/virtio/virtio.c  |   42 ++++++++++++++------------
 include/linux/virtio.h   |    4 ++
 3 files changed, 73 insertions(+), 48 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 922ca66..62dbf4b 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1684,6 +1684,49 @@ static void virtnet_init_settings(struct net_device *dev)
 	.set_settings = virtnet_set_settings,
 };
 
+static void virtnet_freeze_down(struct virtio_device *vdev)
+{
+	struct virtnet_info *vi = vdev->priv;
+	int i;
+
+	/* Make sure no work handler is accessing the device */
+	flush_work(&vi->config_work);
+
+	netif_device_detach(vi->dev);
+	cancel_delayed_work_sync(&vi->refill);
+
+	if (netif_running(vi->dev)) {
+		for (i = 0; i < vi->max_queue_pairs; i++)
+			napi_disable(&vi->rq[i].napi);
+	}
+}
+
+static int init_vqs(struct virtnet_info *vi);
+
+static int virtnet_restore_up(struct virtio_device *vdev)
+{
+	struct virtnet_info *vi = vdev->priv;
+	int err, i;
+
+	err = init_vqs(vi);
+	if (err)
+		return err;
+
+	virtio_device_ready(vdev);
+
+	if (netif_running(vi->dev)) {
+		for (i = 0; i < vi->curr_queue_pairs; i++)
+			if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
+				schedule_delayed_work(&vi->refill, 0);
+
+		for (i = 0; i < vi->max_queue_pairs; i++)
+			virtnet_napi_enable(&vi->rq[i]);
+	}
+
+	netif_device_attach(vi->dev);
+	return err;
+}
+
 static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog)
 {
 	unsigned long int max_sz = PAGE_SIZE - sizeof(struct padded_vnet_hdr);
@@ -2374,21 +2417,9 @@ static void virtnet_remove(struct virtio_device *vdev)
 static int virtnet_freeze(struct virtio_device *vdev)
 {
 	struct virtnet_info *vi = vdev->priv;
-	int i;
 
 	virtnet_cpu_notif_remove(vi);
-
-	/* Make sure no work handler is accessing the device */
-	flush_work(&vi->config_work);
-
-	netif_device_detach(vi->dev);
-	cancel_delayed_work_sync(&vi->refill);
-
-	if (netif_running(vi->dev)) {
-		for (i = 0; i < vi->max_queue_pairs; i++)
-			napi_disable(&vi->rq[i].napi);
-	}
-
+	virtnet_freeze_down(vdev);
 	remove_vq_common(vi);
 
 	return 0;
@@ -2397,25 +2428,11 @@ static int virtnet_freeze(struct virtio_device *vdev)
 static int virtnet_restore(struct virtio_device *vdev)
 {
 	struct virtnet_info *vi = vdev->priv;
-	int err, i;
+	int err;
 
-	err = init_vqs(vi);
+	err = virtnet_restore_up(vdev);
 	if (err)
 		return err;
-
-	virtio_device_ready(vdev);
-
-	if (netif_running(vi->dev)) {
-		for (i = 0; i < vi->curr_queue_pairs; i++)
-			if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
-				schedule_delayed_work(&vi->refill, 0);
-
-		for (i = 0; i < vi->max_queue_pairs; i++)
-			virtnet_napi_enable(&vi->rq[i]);
-	}
-
-	netif_device_attach(vi->dev);
-
 	virtnet_set_queues(vi, vi->curr_queue_pairs);
 
 	err = virtnet_cpu_notif_add(vi);
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 7062bb0..400d70b 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -100,11 +100,6 @@ static int virtio_uevent(struct device *_dv, struct kobj_uevent_env *env)
 			      dev->id.device, dev->id.vendor);
 }
 
-static void add_status(struct virtio_device *dev, unsigned status)
-{
-	dev->config->set_status(dev, dev->config->get_status(dev) | status);
-}
-
 void virtio_check_driver_offered_feature(const struct virtio_device *vdev,
 					 unsigned int fbit)
 {
@@ -145,14 +140,15 @@ void virtio_config_changed(struct virtio_device *dev)
 }
 EXPORT_SYMBOL_GPL(virtio_config_changed);
 
-static void virtio_config_disable(struct virtio_device *dev)
+void virtio_config_disable(struct virtio_device *dev)
 {
 	spin_lock_irq(&dev->config_lock);
 	dev->config_enabled = false;
 	spin_unlock_irq(&dev->config_lock);
 }
+EXPORT_SYMBOL_GPL(virtio_config_disable);
 
-static void virtio_config_enable(struct virtio_device *dev)
+void virtio_config_enable(struct virtio_device *dev)
 {
 	spin_lock_irq(&dev->config_lock);
 	dev->config_enabled = true;
@@ -161,8 +157,15 @@ static void virtio_config_enable(struct virtio_device *dev)
 	dev->config_change_pending = false;
 	spin_unlock_irq(&dev->config_lock);
 }
+EXPORT_SYMBOL_GPL(virtio_config_enable);
+
+void virtio_add_status(struct virtio_device *dev, unsigned int status)
+{
+	dev->config->set_status(dev, dev->config->get_status(dev) | status);
+}
+EXPORT_SYMBOL_GPL(virtio_add_status);
 
-static int virtio_finalize_features(struct virtio_device *dev)
+int virtio_finalize_features(struct virtio_device *dev)
 {
 	int ret = dev->config->finalize_features(dev);
 	unsigned status;
@@ -173,7 +176,7 @@ static int virtio_finalize_features(struct virtio_device *dev)
 	if (!virtio_has_feature(dev, VIRTIO_F_VERSION_1))
 		return 0;
 
-	add_status(dev, VIRTIO_CONFIG_S_FEATURES_OK);
+	virtio_add_status(dev, VIRTIO_CONFIG_S_FEATURES_OK);
 	status = dev->config->get_status(dev);
 	if (!(status & VIRTIO_CONFIG_S_FEATURES_OK)) {
 		dev_err(&dev->dev, "virtio: device refuses features: %x\n",
@@ -182,6 +185,7 @@ static int virtio_finalize_features(struct virtio_device *dev)
 	}
 	return 0;
 }
+EXPORT_SYMBOL_GPL(virtio_finalize_features);
 
 static int virtio_dev_probe(struct device *_d)
 {
@@ -193,7 +197,7 @@ static int virtio_dev_probe(struct device *_d)
 	u64 driver_features_legacy;
 
 	/* We have a driver! */
-	add_status(dev, VIRTIO_CONFIG_S_DRIVER);
+	virtio_add_status(dev, VIRTIO_CONFIG_S_DRIVER);
 
 	/* Figure out what features the device supports. */
 	device_features = dev->config->get_features(dev);
@@ -247,7 +251,7 @@ static int virtio_dev_probe(struct device *_d)
 
 	return 0;
 err:
-	add_status(dev, VIRTIO_CONFIG_S_FAILED);
+	virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED);
 	return err;
 
 }
@@ -265,7 +269,7 @@ static int virtio_dev_remove(struct device *_d)
 	WARN_ON_ONCE(dev->config->get_status(dev));
 
 	/* Acknowledge the device's existence again. */
-	add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);
+	virtio_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);
 	return 0;
 }
 
@@ -316,7 +320,7 @@ int register_virtio_device(struct virtio_device *dev)
 	dev->config->reset(dev);
 
 	/* Acknowledge that we've seen the device. */
-	add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);
+	virtio_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);
 
 	INIT_LIST_HEAD(&dev->vqs);
 
@@ -325,7 +329,7 @@ int register_virtio_device(struct virtio_device *dev)
 	err = device_register(&dev->dev);
 out:
 	if (err)
-		add_status(dev, VIRTIO_CONFIG_S_FAILED);
+		virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED);
 	return err;
 }
 EXPORT_SYMBOL_GPL(register_virtio_device);
@@ -365,18 +369,18 @@ int virtio_device_restore(struct virtio_device *dev)
 	dev->config->reset(dev);
 
 	/* Acknowledge that we've seen the device. */
-	add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);
+	virtio_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);
 
 	/* Maybe driver failed before freeze.
 	 * Restore the failed status, for debugging. */
 	if (dev->failed)
-		add_status(dev, VIRTIO_CONFIG_S_FAILED);
+		virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED);
 
 	if (!drv)
 		return 0;
 
 	/* We have a driver! */
-	add_status(dev, VIRTIO_CONFIG_S_DRIVER);
+	virtio_add_status(dev, VIRTIO_CONFIG_S_DRIVER);
 
 	ret = virtio_finalize_features(dev);
 	if (ret)
@@ -389,14 +393,14 @@ int virtio_device_restore(struct virtio_device *dev)
 	}
 
 	/* Finally, tell the device we're all set */
-	add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
+	virtio_add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
 
 	virtio_config_enable(dev);
 
 	return 0;
 
 err:
-	add_status(dev, VIRTIO_CONFIG_S_FAILED);
+	virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(virtio_device_restore);
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index d5eb547..04b0d3f 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -132,12 +132,16 @@ static inline struct virtio_device *dev_to_virtio(struct device *_dev)
 	return container_of(_dev, struct virtio_device, dev);
 }
 
+void virtio_add_status(struct virtio_device *dev, unsigned int status);
 int register_virtio_device(struct virtio_device *dev);
 void unregister_virtio_device(struct virtio_device *dev);
 
 void virtio_break_device(struct virtio_device *dev);
 
 void virtio_config_changed(struct virtio_device *dev);
+void virtio_config_disable(struct virtio_device *dev);
+void virtio_config_enable(struct virtio_device *dev);
+int virtio_finalize_features(struct virtio_device *dev);
 #ifdef CONFIG_PM_SLEEP
 int virtio_device_freeze(struct virtio_device *dev);
 int virtio_device_restore(struct virtio_device *dev);

^ permalink raw reply related

* [PATCH net v2] lwtunnel: fix autoload of lwt modules
From: David Ahern @ 2017-01-17 22:57 UTC (permalink / raw)
  To: netdev; +Cc: rshearma, David Ahern

Trying to add an mpls encap route when the MPLS modules are not loaded
hangs. For example:

    CONFIG_MPLS=y
    CONFIG_NET_MPLS_GSO=m
    CONFIG_MPLS_ROUTING=m
    CONFIG_MPLS_IPTUNNEL=m

    $ ip route add 10.10.10.10/32 encap mpls 100 via inet 10.100.1.2

The ip command hangs:
root       880   826  0 21:25 pts/0    00:00:00 ip route add 10.10.10.10/32 encap mpls 100 via inet 10.100.1.2

    $ cat /proc/880/stack
    [<ffffffff81065a9b>] call_usermodehelper_exec+0xd6/0x134
    [<ffffffff81065efc>] __request_module+0x27b/0x30a
    [<ffffffff814542f6>] lwtunnel_build_state+0xe4/0x178
    [<ffffffff814aa1e4>] fib_create_info+0x47f/0xdd4
    [<ffffffff814ae451>] fib_table_insert+0x90/0x41f
    [<ffffffff814a8010>] inet_rtm_newroute+0x4b/0x52
    ...

modprobe is trying to load rtnl-lwt-MPLS:

root       881     5  0 21:25 ?        00:00:00 /sbin/modprobe -q -- rtnl-lwt-MPLS

and it hangs after loading mpls_router:

    $ cat /proc/881/stack
    [<ffffffff81441537>] rtnl_lock+0x12/0x14
    [<ffffffff8142ca2a>] register_netdevice_notifier+0x16/0x179
    [<ffffffffa0033025>] mpls_init+0x25/0x1000 [mpls_router]
    [<ffffffff81000471>] do_one_initcall+0x8e/0x13f
    [<ffffffff81119961>] do_init_module+0x5a/0x1e5
    [<ffffffff810bd070>] load_module+0x13bd/0x17d6
    ...

The problem is that lwtunnel_build_state is called with rtnl lock
held preventing mpls_init from registering.

Given the potential references held by the time lwtunnel_build_state it
can not drop the rtnl lock to the load module. So, extract the module
loading code from lwtunnel_build_state into a new function to validate
the encap type. The new function is called while converting the user
request into a fib_config which is well before any table, device or
fib entries are examined.

Fixes: 745041e2aaf1 ("lwtunnel: autoload of lwt modules")
Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
v2
- extract the module load attempt into a separate function that is
  called early in the newroute code paths

 include/net/lwtunnel.h  | 11 +++++++++
 net/core/lwtunnel.c     | 62 ++++++++++++++++++++++++++++++++++++++++++++-----
 net/ipv4/fib_frontend.c |  8 +++++++
 net/ipv6/route.c        | 12 +++++++++-
 4 files changed, 86 insertions(+), 7 deletions(-)

diff --git a/include/net/lwtunnel.h b/include/net/lwtunnel.h
index d4c1c75b8862..0b585f1fd340 100644
--- a/include/net/lwtunnel.h
+++ b/include/net/lwtunnel.h
@@ -105,6 +105,8 @@ int lwtunnel_encap_add_ops(const struct lwtunnel_encap_ops *op,
 			   unsigned int num);
 int lwtunnel_encap_del_ops(const struct lwtunnel_encap_ops *op,
 			   unsigned int num);
+int lwtunnel_valid_encap_type(u16 encap_type);
+int lwtunnel_valid_encap_type_attr(struct nlattr *attr, int len);
 int lwtunnel_build_state(struct net_device *dev, u16 encap_type,
 			 struct nlattr *encap,
 			 unsigned int family, const void *cfg,
@@ -168,6 +170,15 @@ static inline int lwtunnel_encap_del_ops(const struct lwtunnel_encap_ops *op,
 	return -EOPNOTSUPP;
 }
 
+static inline int lwtunnel_valid_encap_type(u16 encap_type)
+{
+	return -EOPNOTSUPP;
+}
+static inline int lwtunnel_valid_encap_type_attr(struct nlattr *attr, int len)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline int lwtunnel_build_state(struct net_device *dev, u16 encap_type,
 				       struct nlattr *encap,
 				       unsigned int family, const void *cfg,
diff --git a/net/core/lwtunnel.c b/net/core/lwtunnel.c
index a5d4e866ce88..47b1dd65947b 100644
--- a/net/core/lwtunnel.c
+++ b/net/core/lwtunnel.c
@@ -26,6 +26,7 @@
 #include <net/lwtunnel.h>
 #include <net/rtnetlink.h>
 #include <net/ip6_fib.h>
+#include <net/nexthop.h>
 
 #ifdef CONFIG_MODULES
 
@@ -114,25 +115,74 @@ int lwtunnel_build_state(struct net_device *dev, u16 encap_type,
 	ret = -EOPNOTSUPP;
 	rcu_read_lock();
 	ops = rcu_dereference(lwtun_encaps[encap_type]);
+	if (likely(ops && ops->build_state))
+		ret = ops->build_state(dev, encap, family, cfg, lws);
+	rcu_read_unlock();
+
+	return ret;
+}
+EXPORT_SYMBOL(lwtunnel_build_state);
+
+int lwtunnel_valid_encap_type(u16 encap_type)
+{
+	const struct lwtunnel_encap_ops *ops;
+	int ret = -EINVAL;
+
+	if (encap_type == LWTUNNEL_ENCAP_NONE ||
+	    encap_type > LWTUNNEL_ENCAP_MAX)
+		return ret;
+
+	rcu_read_lock();
+	ops = rcu_dereference(lwtun_encaps[encap_type]);
+	rcu_read_unlock();
 #ifdef CONFIG_MODULES
 	if (!ops) {
 		const char *encap_type_str = lwtunnel_encap_str(encap_type);
 
 		if (encap_type_str) {
-			rcu_read_unlock();
+			__rtnl_unlock();
 			request_module("rtnl-lwt-%s", encap_type_str);
+			rtnl_lock();
+
 			rcu_read_lock();
 			ops = rcu_dereference(lwtun_encaps[encap_type]);
+			rcu_read_unlock();
 		}
 	}
 #endif
-	if (likely(ops && ops->build_state))
-		ret = ops->build_state(dev, encap, family, cfg, lws);
-	rcu_read_unlock();
+	return ops ? 0 : -EOPNOTSUPP;
+}
+EXPORT_SYMBOL(lwtunnel_valid_encap_type);
 
-	return ret;
+int lwtunnel_valid_encap_type_attr(struct nlattr *attr, int remaining)
+{
+	struct rtnexthop *rtnh = (struct rtnexthop *)attr;
+	struct nlattr *nla_entype;
+	struct nlattr *attrs;
+	struct nlattr *nla;
+	u16 encap_type;
+	int attrlen;
+
+	while (rtnh_ok(rtnh, remaining)) {
+		attrlen = rtnh_attrlen(rtnh);
+		if (attrlen > 0) {
+			attrs = rtnh_attrs(rtnh);
+			nla = nla_find(attrs, attrlen, RTA_ENCAP);
+			nla_entype = nla_find(attrs, attrlen, RTA_ENCAP_TYPE);
+
+			if (nla_entype) {
+				encap_type = nla_get_u16(nla_entype);
+
+				if (lwtunnel_valid_encap_type(encap_type) != 0)
+					return -EOPNOTSUPP;
+			}
+		}
+		rtnh = rtnh_next(rtnh, &remaining);
+	}
+
+	return 0;
 }
-EXPORT_SYMBOL(lwtunnel_build_state);
+EXPORT_SYMBOL(lwtunnel_valid_encap_type_attr);
 
 void lwtstate_free(struct lwtunnel_state *lws)
 {
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index eae0332b0e8c..7db2ad2e82d3 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -46,6 +46,7 @@
 #include <net/rtnetlink.h>
 #include <net/xfrm.h>
 #include <net/l3mdev.h>
+#include <net/lwtunnel.h>
 #include <trace/events/fib.h>
 
 #ifndef CONFIG_IP_MULTIPLE_TABLES
@@ -677,6 +678,10 @@ static int rtm_to_fib_config(struct net *net, struct sk_buff *skb,
 			cfg->fc_mx_len = nla_len(attr);
 			break;
 		case RTA_MULTIPATH:
+			err = lwtunnel_valid_encap_type_attr(nla_data(attr),
+							     nla_len(attr));
+			if (err < 0)
+				goto errout;
 			cfg->fc_mp = nla_data(attr);
 			cfg->fc_mp_len = nla_len(attr);
 			break;
@@ -691,6 +696,9 @@ static int rtm_to_fib_config(struct net *net, struct sk_buff *skb,
 			break;
 		case RTA_ENCAP_TYPE:
 			cfg->fc_encap_type = nla_get_u16(attr);
+			err = lwtunnel_valid_encap_type(cfg->fc_encap_type);
+			if (err < 0)
+				goto errout;
 			break;
 		}
 	}
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 4f6b067c8753..7ea85370c11c 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2896,6 +2896,11 @@ static int rtm_to_fib6_config(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (tb[RTA_MULTIPATH]) {
 		cfg->fc_mp = nla_data(tb[RTA_MULTIPATH]);
 		cfg->fc_mp_len = nla_len(tb[RTA_MULTIPATH]);
+
+		err = lwtunnel_valid_encap_type_attr(cfg->fc_mp,
+						     cfg->fc_mp_len);
+		if (err < 0)
+			goto errout;
 	}
 
 	if (tb[RTA_PREF]) {
@@ -2909,9 +2914,14 @@ static int rtm_to_fib6_config(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (tb[RTA_ENCAP])
 		cfg->fc_encap = tb[RTA_ENCAP];
 
-	if (tb[RTA_ENCAP_TYPE])
+	if (tb[RTA_ENCAP_TYPE]) {
 		cfg->fc_encap_type = nla_get_u16(tb[RTA_ENCAP_TYPE]);
 
+		err = lwtunnel_valid_encap_type(cfg->fc_encap_type);
+		if (err < 0)
+			goto errout;
+	}
+
 	if (tb[RTA_EXPIRES]) {
 		unsigned long timeout = addrconf_timeout_fixup(nla_get_u32(tb[RTA_EXPIRES]), HZ);
 
-- 
2.1.4

^ permalink raw reply related

* RE: [PATCHv1 7/7] IPVTAP: IP-VLAN based tap driver
From: Grandhi, Sainath @ 2017-01-17 23:06 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: netdev@vger.kernel.org, davem@davemloft.net, mahesh@bandewar.net,
	linux-kernel@vger.kernel.org
In-Reply-To: <1483744443.9712.45.camel@edumazet-glaptop3.roam.corp.google.com>



> -----Original Message-----
> From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
> Sent: Friday, January 06, 2017 3:14 PM
> To: Grandhi, Sainath <sainath.grandhi@intel.com>
> Cc: netdev@vger.kernel.org; davem@davemloft.net;
> mahesh@bandewar.net; linux-kernel@vger.kernel.org
> Subject: Re: [PATCHv1 7/7] IPVTAP: IP-VLAN based tap driver
> 
> On Fri, 2017-01-06 at 14:33 -0800, Sainath Grandhi wrote:
> > This patch adds a tap character device driver that is based on the
> > IP-VLAN network interface, called ipvtap. An ipvtap device can be
> > created in the same way as an ipvlan device, using 'type ipvtap', and
> > then accessed using the tap user space interface.
> >
> > Signed-off-by: Sainath Grandhi <sainath.grandhi@intel.com>
> > Tested-by: Sainath Grandhi <sainath.grandhi@intel.com>
> > ---
> 
> 
> > +module_exit(ipvtap_exit);
> > +MODULE_ALIAS_RTNL_LINK("ipvtap");
> > +MODULE_AUTHOR("Arnd Bergmann <arnd@arndb.de>");
> > +MODULE_LICENSE("GPL");
> 
> Who wrote this driver exactly ???
> 
> 
Sending out next version, modifying this.

^ permalink raw reply

* RE: [PATCHv1 5/7] TAP: Extending tap device create/destroy APIs
From: Grandhi, Sainath @ 2017-01-17 23:07 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: netdev@vger.kernel.org, davem@davemloft.net, mahesh@bandewar.net,
	linux-kernel@vger.kernel.org
In-Reply-To: <1483744559.9712.47.camel@edumazet-glaptop3.roam.corp.google.com>



> -----Original Message-----
> From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
> Sent: Friday, January 06, 2017 3:16 PM
> To: Grandhi, Sainath <sainath.grandhi@intel.com>
> Cc: netdev@vger.kernel.org; davem@davemloft.net;
> mahesh@bandewar.net; linux-kernel@vger.kernel.org
> Subject: Re: [PATCHv1 5/7] TAP: Extending tap device create/destroy APIs
> 
> On Fri, 2017-01-06 at 14:33 -0800, Sainath Grandhi wrote:
> 
> > +static int tap_list_add(dev_t major, const char *device_name) {
> > +	int err = 0;
> > +	struct major_info *tap_major;
> > +
> > +	tap_major = kzalloc(sizeof(*tap_major), GFP_ATOMIC);
> > +
> > +	tap_major->major = MAJOR(major);
> > +
> 
> 
> kzalloc() can perfectly return NULL.
> 
> You do not want to crash it that happens.
> 
Thanks for pointing out. Will send out next version that takes care of null pointer

^ permalink raw reply

* Re: [PATCH] net: ethernet: stmmac: add ARP management
From: Andy Shevchenko @ 2017-01-17 23:14 UTC (permalink / raw)
  To: Christophe Roullier
  Cc: Alexandre Torgue, Giuseppe Cavallaro, netdev,
	linux-kernel@vger.kernel.org
In-Reply-To: <1484672200-7755-1-git-send-email-christophe.roullier@st.com>

On Tue, Jan 17, 2017 at 6:56 PM, Christophe Roullier
<christophe.roullier@st.com> wrote:

> +static int dwmac4_arp_enable(struct mac_device_info *hw)
> +{
> +       void __iomem *ioaddr = hw->pcsr;

__iomem *config = hw->pcsr + GMAC_CONFIG;

> +       u32 value = readl(ioaddr + GMAC_CONFIG);
> +
> +       value |= GMAC_CONFIG_ARPEN;
> +
> +       writel(value, ioaddr + GMAC_CONFIG);

u32 value;

value = readl();
writel(value | ...);

?

> +
> +       value = readl(ioaddr + GMAC_CONFIG);
> +
> +       return !!(value & GMAC_CONFIG_ARPEN);
> +}

> +/* Set ARP Address */
> +static void dwmac4_set_arp_addr(void __iomem *ioaddr, bool set, u32 addr)
> +{

__iomem *arp_addr = ioaddr + GMAC_ARP_ADDR;

> +       u32 value;
> +
> +       value = readl(ioaddr + GMAC_ARP_ADDR);

Care to explain why do you need dummy readl() here?

> +
> +       if (set) {
> +               /* set arp address */
> +               value = addr;
> +       } else {
> +               /* unset arp address */
> +               value = 0;
> +       }

value = set ? addr : 0;


> +
> +       writel(value, ioaddr + GMAC_ARP_ADDR);
> +       value = readl(ioaddr + GMAC_ARP_ADDR);
> +}


> +       if ((priv->plat->arp_en) && (priv->dma_cap.arpoffsel)) {
> +               ret = priv->hw->mac->arp_en(priv->hw);
> +               if (!ret) {

Hmm... Most would expect

if (ret) {
 doing something
} else {
 doing something else
}

> +                       pr_warn(" ARP feature disabled\n");
> +               } else {

> +                       pr_info(" ARP feature enabled\n");

Wouldn't be too noisy?

pr_* -> dev_*

> +                       /* Copy MAC addr into MAC_ARP_ADDRESS register*/
> +                       priv->hw->dma->set_arp_addr(priv->ioaddr, 1,
> +                                                   priv->dev->dev_addr);
> +               }
> +       }

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH net] lwtunnel: fix autoload of lwt modules
From: David Ahern @ 2017-01-17 23:16 UTC (permalink / raw)
  To: David Miller; +Cc: rshearma, netdev, roopa
In-Reply-To: <20170117.155424.1815351091063210993.davem@davemloft.net>

On 1/17/17 1:54 PM, David Miller wrote:
> From: David Ahern <dsa@cumulusnetworks.com>
> Date: Tue, 17 Jan 2017 13:46:22 -0700
> 
>> In short seems like removing the dev + the current patch dropping
>> the lock fixes the current deadlock problem and should be fine.
> 
> What about the state recorded by fib_get_nhs() and similar?  There is
> a mapping from ifindex to ->nh_dev which would be invalidated if the
> RTNL semaphore is dropped.

As far as I can see through the call to build_state all device indices came from the user and have not been validated yet (once the dev arg to build_state is removed; sent that patch for net-next). The device index validation happens later in fib_create_info with the call to fib_check_nh (or dev_get_by_index for host scope).

I sent an alternative approach that pulls the module loading into a separate function that is called while creating the fib_config. Performance heavy for multipath but solves the autoload without delving into the restart problem.

^ permalink raw reply

* Re: 52bd2d62ce6758d811edcbd2256eb9ea7f6a56cb fixing crashes? -> 4.4 stable?
From: Greg @ 2017-01-17 23:09 UTC (permalink / raw)
  To: Nikola Ciprich, Pravin Shelar; +Cc: netdev, edumazet, nik
In-Reply-To: <20170117214832.GA17323@pcnci.linuxbox.cz>

On Tue, 2017-01-17 at 22:48 +0100, Nikola Ciprich wrote:
> Dear netdev developers,
> 
> I'd like to ask for a consultation regarding 4.4 kernel crashes.
> we're using intel X540-AT2 10g controllers (onboard ones, on supermicro
> boards) and we've noticed, then when using openvswitch, system very quickly
> crashes on 4.4.x kernels we're usign. 4.5 is fine though.
> 
> here's backtrace gathered from system pstore:

Adding the openvswitch maintainer, Pravin. Hopefully you'll get a
quicker response.

- Greg

> 
> <1>[ 1084.114586] BUG: unable to handle kernel paging request at ffff8840c365b5c4
> <1>[ 1084.114918] IP: [<ffffffff81589802>] __netdev_pick_tx+0x92/0x140
> <4>[ 1084.115101] PGD 2018067 PUD 0
> <4>[ 1084.115270] Oops: 0000 [#1] SMP
> <4>[ 1084.115439] Modules linked in: bonding(E) openvswitch(E) nf_defrag_ipv6(E) nf_conntrack(E) crc32_pclmul(E) aesni_intel(E) lrw(E) gf128mul(E) glue_helper(E) ablk_helper(E) cryptd(E) kvm
> _intel(E) kvm(E) irqbypass(E) coretemp(E) crct10dif_pclmul(E) intel_powerclamp(E) x86_pkg_temp_thermal(E) ses(E) enclosure(E) iTCO_wdt(E) iTCO_vendor_support(E) mxm_wmi(E) i2c_i801(E) lpc_ic
> h(E) mei_me(E) mfd_core(E) i2c_core(E) sb_edac(E) sg(E) mei(E) pcspkr(E) edac_core(E) ipmi_devintf(E) ioatdma(E) shpchp(E) wmi(E) ipmi_si(E) ipmi_msghandler(E) 8250_fintek(E) acpi_power_mete
> r(E) acpi_pad(E) nfsd(E) auth_rpcgss(E) nfs_acl(E) lockd(E) grace(E) sunrpc(E) ip_tables(E) ext4(E) jbd2(E) mbcache(E) raid1(E) sd_mod(E) ahci(E) libahci(E) bnx2x(E) libcrc32c(E) ixgbe(E) cr
> c32c_intel(E) libata(E) mdio(E) ptp(E) dca(E) megaraid_sas(E) pps_core(E) dm_mirror(E) dm_region_hash(E) dm_log(E) dm_mod(E)
> <4>[ 1084.117683] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G            E   4.4.33lb7.01 #1
> <4>[ 1084.118012] Hardware name: Supermicro X10DRi/X10DRI-T, BIOS 2.1 09/13/2016
> <4>[ 1084.118181] task: ffffffff819f14c0 ti: ffffffff819e0000 task.ti: ffffffff819e0000
> <4>[ 1084.118501] RIP: 0010:[<ffffffff81589802>]  [<ffffffff81589802>] __netdev_pick_tx+0x92/0x140
> <4>[ 1084.118828] RSP: 0018:ffff883f7f003638  EFLAGS: 00010a02
> <4>[ 1084.118994] RAX: 00000000aef55a76 RBX: 0000000000000000 RCX: 000000009d6e7dcd
> <4>[ 1084.119164] RDX: 00000000ba9f4f5f RSI: ffff883f63f14d00 RDI: ffff883f7f0035ec
> <4>[ 1084.119333] RBP: ffff883f7f003668 R08: 0000000000000003 R09: 00000000c8cfdbe1
> <4>[ 1084.119506] R10: ffff883f61206042 R11: ffff883f7f0035c0 R12: 00000000ffffffff
> <4>[ 1084.119679] R13: ffff883f657b00c0 R14: ffff883f5d920000 R15: 00000000f0000012
> <4>[ 1084.119850] FS:  0000000000000000(0000) GS:ffff883f7f000000(0000) knlGS:0000000000000000
> <4>[ 1084.120171] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> <4>[ 1084.120338] CR2: ffff8840c365b5c4 CR3: 00000000019ea000 CR4: 00000000003406f0
> <4>[ 1084.120509] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> <4>[ 1084.120678] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> <4>[ 1084.120847] Stack:
> <4>[ 1084.121006]  ffff883f63f14d00 ffff883f63f14d00 000000000000000e 0000000000000000
> <4>[ 1084.121339]  ffff883f5d920000 ffff883f60a7f840 ffff883f7f0036a0 ffffffffa00fbed4
> <4>[ 1084.121672]  ffff883f603612ac ffff883f5d920000 ffff883f63f14d00 0000000000000000
> <4>[ 1084.122006] Call Trace:
> <4>[ 1084.122168]  <IRQ>
> <4>[ 1084.122193]  [<ffffffffa00fbed4>] ixgbe_select_queue+0xc4/0x150 [ixgbe]
> <4>[ 1084.122519]  [<ffffffff8159111e>] netdev_pick_tx+0x5e/0xf0
> <4>[ 1084.122687]  [<ffffffff81591252>] __dev_queue_xmit+0xa2/0x560
> <4>[ 1084.122856]  [<ffffffff81591720>] dev_queue_xmit+0x10/0x20
> <4>[ 1084.123034]  [<ffffffffa05e93a2>] bond_dev_queue_xmit+0x32/0x80 [bonding]
> <4>[ 1084.123207]  [<ffffffffa05eb0d6>] bond_start_xmit+0x1a6/0x3f0 [bonding]
> <4>[ 1084.123382]  [<ffffffff8124faa5>] ? ep_poll_callback+0xb5/0x160
> <4>[ 1084.123551]  [<ffffffff81590f08>] dev_hard_start_xmit+0x238/0x3f0
> <4>[ 1084.123721]  [<ffffffff815908cf>] ? netif_skb_features+0xff/0x200
> <4>[ 1084.123890]  [<ffffffff815915f2>] __dev_queue_xmit+0x442/0x560
> <4>[ 1084.124059]  [<ffffffff81591720>] dev_queue_xmit+0x10/0x20
> <4>[ 1084.124232]  [<ffffffffa04fe70a>] ovs_vport_send+0x4a/0xc0 [openvswitch]
> <4>[ 1084.124404]  [<ffffffffa04f1263>] do_output.isra.30+0x43/0x160 [openvswitch]
> <4>[ 1084.124575]  [<ffffffff81579c5e>] ? __skb_clone+0x2e/0x140
> <4>[ 1084.124744]  [<ffffffffa04f25c4>] do_execute_actions+0x684/0x7e0 [openvswitch]
> <4>[ 1084.125067]  [<ffffffffa04f2752>] ovs_execute_actions+0x32/0xd0 [openvswitch]
> <4>[ 1084.125240]  [<ffffffffa04f5ed4>] ovs_dp_process_packet+0x84/0x110 [openvswitch]
> <4>[ 1084.125565]  [<ffffffffa04fdfec>] ovs_vport_receive+0x6c/0xd0 [openvswitch]
> <4>[ 1084.125740]  [<ffffffff810b1645>] ? check_preempt_curr+0x75/0x90
> <4>[ 1084.125912]  [<ffffffff810b1679>] ? ttwu_do_wakeup+0x19/0xe0
> <4>[ 1084.126081]  [<ffffffff810b195d>] ? ttwu_do_activate.constprop.95+0x5d/0x70
> <4>[ 1084.126252]  [<ffffffff810b23c7>] ? try_to_wake_up+0x47/0x340
> <4>[ 1084.126427]  [<ffffffff810b2772>] ? default_wake_function+0x12/0x20
> <4>[ 1084.126600]  [<ffffffff810ca51b>] ? autoremove_wake_function+0x2b/0x40
> <4>[ 1084.126773]  [<ffffffffa04ff127>] netdev_frame_hook+0xe7/0x150 [openvswitch]
> <4>[ 1084.126945]  [<ffffffff8158e840>] __netif_receive_skb_core+0x1e0/0x9e0
> <4>[ 1084.127115]  [<ffffffff8167d4e6>] ? ipv6_gro_receive+0x246/0x360
> <4>[ 1084.127284]  [<ffffffff8158f058>] __netif_receive_skb+0x18/0x60
> <4>[ 1084.127453]  [<ffffffff8158f0e0>] netif_receive_skb_internal+0x40/0xb0
> <4>[ 1084.127623]  [<ffffffff8158fd23>] napi_gro_receive+0xc3/0x110
> <4>[ 1084.127813]  [<ffffffffa01e41fc>] bnx2x_rx_int+0x101c/0x19d0 [bnx2x]
> <4>[ 1084.127984]  [<ffffffff810c37e3>] ? load_balance+0x163/0x8d0
> <4>[ 1084.128166]  [<ffffffffa01e6a64>] bnx2x_poll+0x284/0x340 [bnx2x]
> <4>[ 1084.128334]  [<ffffffff8158f4eb>] net_rx_action+0x16b/0x370
> <4>[ 1084.128503]  [<ffffffff8108c032>] __do_softirq+0xe2/0x2e0
> <4>[ 1084.128671]  [<ffffffff8108c4d5>] irq_exit+0xf5/0x100
> <4>[ 1084.128843]  [<ffffffff816a0b06>] do_IRQ+0x56/0xd0
> <4>[ 1084.129010]  [<ffffffff8169eb47>] common_interrupt+0x87/0x87
> <4>[ 1084.129176]  <EOI>
> <4>[ 1084.129188]  [<ffffffff8153e168>] ? cpuidle_enter_state+0xd8/0x250
> <4>[ 1084.129510]  [<ffffffff8153e144>] ? cpuidle_enter_state+0xb4/0x250
> <4>[ 1084.129681]  [<ffffffff8153e317>] cpuidle_enter+0x17/0x20
> <4>[ 1084.129849]  [<ffffffff810ca832>] call_cpuidle+0x32/0x60
> <4>[ 1084.130016]  [<ffffffff8153e2f3>] ? cpuidle_select+0x13/0x20
> <4>[ 1084.130184]  [<ffffffff810caaf9>] cpu_startup_entry+0x299/0x360
> <4>[ 1084.130354]  [<ffffffff8169201c>] rest_init+0x7c/0x80
> <4>[ 1084.130521]  [<ffffffff81b5716a>] start_kernel+0x4cf/0x4f0
> <4>[ 1084.134763]  [<ffffffff81b56a86>] ? set_init_arg+0x55/0x55
> <4>[ 1084.134931]  [<ffffffff81b56120>] ? early_idt_handler_array+0x120/0x120
> <4>[ 1084.135101]  [<ffffffff81b565ee>] x86_64_start_reservations+0x2a/0x2c
> <4>[ 1084.135269]  [<ffffffff81b5673c>] x86_64_start_kernel+0x14c/0x16f
> <4>[ 1084.135437] Code: 8b 7d 00 41 83 ff 01 0f 84 8b 00 00 00 f6 86 91 00 00 00 30 0f 84 85 00 00 00 8b 96 a4 00 00 00 44 89 f8 48 0f af c2 48 c1 e8 20 <41> 0f b7 44 45 18 41 3b 86 cc 03 00
>  00 0f 83 81 00 00 00 44 39
> <1>[ 1084.136184] RIP  [<ffffffff81589802>] __netdev_pick_tx+0x92/0x140
> <4>[ 1084.136357]  RSP <ffff883f7f003638>
> <4>[ 1084.136518] CR2: ffff8840c365b5c4
> <4>[ 1084.137174] ---[ end trace 17b59260de82e18d ]---
> <0>[ 1084.212189] Kernel panic - not syncing: Fatal exception in interrupt
> <0>[ 1084.212482] Kernel Offset: disabled
> 
> 
> I've bisected this to following commit:
> 
> commit 52bd2d62ce6758d811edcbd2256eb9ea7f6a56cb
> Author: Eric Dumazet <edumazet@google.com>
> Date:   Wed Nov 18 06:30:50 2015 -0800
> 
>     net: better skb->sender_cpu and skb->napi_id cohabitation
>     
>     skb->sender_cpu and skb->napi_id share a common storage,
>     and we had various bugs about this.
>     
>     We had to call skb_sender_cpu_clear() in some places to
>     not leave a prior skb->napi_id and fool netdev_pick_tx()
>     
>     As suggested by Alexei, we could split the space so that
>     these errors can not happen.
>     
>     0 value being reserved as the common (not initialized) value,
>     let's reserve [1 .. NR_CPUS] range for valid sender_cpu,
>     and [NR_CPUS+1 .. ~0U] for valid napi_id.
>     
>     This will allow proper busy polling support over tunnels.
> 
> 
> I'm by no means kernel developer and it doesn't make any sense
> to me why this patch should be fixing it, but it is.. I've confirmed
> it multiple times, that 4.4.32 without the patch crashes within
> minutes, with it applied (it applies cleanly), it's rock solid.
> 
> therefore I'd probably like to propose this patch to -stable,
> but I'd like to hear you, -netdev people opinion, especially
> Erics..
> 
> what do you think about it?
> 
> thanks a lot in advance for reply
> 
> BR
> 
> nik
> 
> 
> 
> 
> 
> 
> 

^ permalink raw reply

* [PATCH net-next v4 00/10] net: dsa: Support for pdata in dsa2
From: Florian Fainelli @ 2017-01-17 23:21 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, Jason Cooper, Andrew Lunn,
	Sebastian Hesselbarth, Gregory Clement, Russell King,
	Vivien Didelot, David S. Miller, Greg Kroah-Hartman,
	moderated list:ARM/Marvell Dove/MV78xx0/Orion SOC support,
	open list

Hi all,

This is not exactly new, and was sent before, although back then, I did not
have an user of the pre-declared MDIO board information, but now we do. Note
that I have additional changes queued up to have b53 register platform data for
MIPS bcm47xx and bcm63xx.

Yes I know that we should have the Orion platforms eventually be converted to
Device Tree, but until that happens, I don't want any remaining users of the
old "dsa" platform device (hence the previous DTS submissions for ARM/mvebu)
and, there will be platforms out there that most likely won't never see DT
coming their way (BCM47xx is almost 100% sure, BCM63xx maybe not in a distant
future).

We would probably want the whole series to be merged via David Miller's tree
to simplify things.

Greg, can you Ack/Nack patch 5 since it touched the core LDD?

Vivien, since some patches did change, I did not carry your Tested-by tag
to all patches.

Thanks!

Changes in v4:

- Changed device_find_class() to device_find_in_class_name()
- Added kerneldoc above device_find_in_class_name() to explain what it does
  and the calling convention regarding device reference counts
- Changed dev_to_net_device to device_to_net_device() added comments
  about what it does and the caller conventions regarding reference counts

Changes in v3:

- Tested EPROBE_DEFER from a mockup MDIO/DSA switch driver and everything
  is fine, once the driver finally probes we have access to platform data
  as expected

- added comment above dsa_port_is_valid() that port->name is mandatory
  for platform data cases

- added an extra check in dsa_parse_member() for a NULL pdata pointer

- fixed a bunch of checkpatch errors and warnings

Changes in v2:

- Rebased against latest net-next/master

- Moved dev_find_class() to device_find_class() into drivers/base/core.c

- Moved dev_to_net_device into net/core/dev.c

- Utilize dsa_chip_data directly instead of dsa_platform_data

- Augmented dsa_chip_data to be multi-CPU port ready

Changes from last submission (few months back):

- rebased against latest net-next

- do not introduce dsa2_platform_data which was overkill and was meant to
  allow us to do exaclty the same things with platform data and Device Tree
  we use the existing dsa_platform_data instead

- properly register MDIO devices when the MDIO bus is registered and associate
  platform_data with them

- add a change to the Orion platform code to demonstrate how this can be used

Thank you

Florian Fainelli (10):
  net: dsa: Pass device pointer to dsa_register_switch
  net: dsa: Make most functions take a dsa_port argument
  net: dsa: Suffix function manipulating device_node with _dn
  net: dsa: Move ports assignment closer to error checking
  drivers: base: Add device_find_in_class_name()
  net: dsa: Migrate to device_find_in_class_name()
  net: Relocate dev_to_net_device() into net/core/dev.c
  net: dsa: Add support for platform data
  net: phy: Allow pre-declaration of MDIO devices
  ARM: orion: Register DSA switch as a MDIO device

 arch/arm/mach-orion5x/common.c               |   2 +-
 arch/arm/mach-orion5x/common.h               |   4 +-
 arch/arm/mach-orion5x/rd88f5181l-fxo-setup.c |   7 +-
 arch/arm/mach-orion5x/rd88f5181l-ge-setup.c  |   7 +-
 arch/arm/mach-orion5x/rd88f6183ap-ge-setup.c |   7 +-
 arch/arm/mach-orion5x/wnr854t-setup.c        |   2 +-
 arch/arm/mach-orion5x/wrt350n-v2-setup.c     |   7 +-
 arch/arm/plat-orion/common.c                 |  25 +++-
 arch/arm/plat-orion/include/plat/common.h    |   4 +-
 drivers/base/core.c                          |  31 +++++
 drivers/net/dsa/b53/b53_common.c             |   2 +-
 drivers/net/dsa/mv88e6xxx/chip.c             |  11 +-
 drivers/net/dsa/qca8k.c                      |   2 +-
 drivers/net/phy/Makefile                     |   3 +-
 drivers/net/phy/mdio-boardinfo.c             |  86 +++++++++++++
 drivers/net/phy/mdio-boardinfo.h             |  19 +++
 drivers/net/phy/mdio_bus.c                   |   4 +
 drivers/net/phy/mdio_device.c                |  11 ++
 include/linux/device.h                       |   2 +
 include/linux/mdio.h                         |   3 +
 include/linux/mod_devicetable.h              |   1 +
 include/linux/netdevice.h                    |   2 +
 include/linux/phy.h                          |  19 +++
 include/net/dsa.h                            |   8 +-
 net/core/dev.c                               |  30 +++++
 net/dsa/dsa.c                                |  55 ++-------
 net/dsa/dsa2.c                               | 175 +++++++++++++++++++--------
 net/dsa/dsa_priv.h                           |   4 +-
 28 files changed, 391 insertions(+), 142 deletions(-)
 create mode 100644 drivers/net/phy/mdio-boardinfo.c
 create mode 100644 drivers/net/phy/mdio-boardinfo.h

-- 
2.9.3

^ permalink raw reply

* [PATCH net-next v4 01/10] net: dsa: Pass device pointer to dsa_register_switch
From: Florian Fainelli @ 2017-01-17 23:21 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, Jason Cooper, Andrew Lunn,
	Sebastian Hesselbarth, Gregory Clement, Russell King,
	Vivien Didelot, David S. Miller, Greg Kroah-Hartman,
	moderated list:ARM/Marvell Dove/MV78xx0/Orion SOC support,
	open list
In-Reply-To: <20170117232152.1661-1-f.fainelli@gmail.com>

In preparation for allowing dsa_register_switch() to be supplied with
device/platform data, pass down a struct device pointer instead of a
struct device_node.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c |  2 +-
 drivers/net/dsa/mv88e6xxx/chip.c | 11 ++++++-----
 drivers/net/dsa/qca8k.c          |  2 +-
 include/net/dsa.h                |  2 +-
 net/dsa/dsa2.c                   |  7 ++++---
 5 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 5102a3701a1a..7179eed9ee6d 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1882,7 +1882,7 @@ int b53_switch_register(struct b53_device *dev)
 
 	pr_info("found switch: %s, rev %i\n", dev->name, dev->core_rev);
 
-	return dsa_register_switch(dev->ds, dev->ds->dev->of_node);
+	return dsa_register_switch(dev->ds, dev->ds->dev);
 }
 EXPORT_SYMBOL(b53_switch_register);
 
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 987b2dbbd35a..3238a4752b98 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -4421,8 +4421,7 @@ static struct dsa_switch_driver mv88e6xxx_switch_drv = {
 	.ops			= &mv88e6xxx_switch_ops,
 };
 
-static int mv88e6xxx_register_switch(struct mv88e6xxx_chip *chip,
-				     struct device_node *np)
+static int mv88e6xxx_register_switch(struct mv88e6xxx_chip *chip)
 {
 	struct device *dev = chip->dev;
 	struct dsa_switch *ds;
@@ -4437,7 +4436,7 @@ static int mv88e6xxx_register_switch(struct mv88e6xxx_chip *chip,
 
 	dev_set_drvdata(dev, ds);
 
-	return dsa_register_switch(ds, np);
+	return dsa_register_switch(ds, dev);
 }
 
 static void mv88e6xxx_unregister_switch(struct mv88e6xxx_chip *chip)
@@ -4521,9 +4520,11 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 	if (err)
 		goto out_g2_irq;
 
-	err = mv88e6xxx_register_switch(chip, np);
-	if (err)
+	err = mv88e6xxx_register_switch(chip);
+	if (err) {
+		mv88e6xxx_mdio_unregister(chip);
 		goto out_mdio;
+	}
 
 	return 0;
 
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 54d270d59eb0..c084aa484d2b 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -964,7 +964,7 @@ qca8k_sw_probe(struct mdio_device *mdiodev)
 	mutex_init(&priv->reg_mutex);
 	dev_set_drvdata(&mdiodev->dev, priv);
 
-	return dsa_register_switch(priv->ds, priv->ds->dev->of_node);
+	return dsa_register_switch(priv->ds, &mdiodev->dev);
 }
 
 static void
diff --git a/include/net/dsa.h b/include/net/dsa.h
index b94d1f2ef912..16a502a6c26a 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -403,7 +403,7 @@ static inline bool dsa_uses_tagged_protocol(struct dsa_switch_tree *dst)
 }
 
 void dsa_unregister_switch(struct dsa_switch *ds);
-int dsa_register_switch(struct dsa_switch *ds, struct device_node *np);
+int dsa_register_switch(struct dsa_switch *ds, struct device *dev);
 #ifdef CONFIG_PM_SLEEP
 int dsa_switch_suspend(struct dsa_switch *ds);
 int dsa_switch_resume(struct dsa_switch *ds);
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 42a41d84053c..4170f7ea8e28 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -579,8 +579,9 @@ static struct device_node *dsa_get_ports(struct dsa_switch *ds,
 	return ports;
 }
 
-static int _dsa_register_switch(struct dsa_switch *ds, struct device_node *np)
+static int _dsa_register_switch(struct dsa_switch *ds, struct device *dev)
 {
+	struct device_node *np = dev->of_node;
 	struct device_node *ports = dsa_get_ports(ds, np);
 	struct dsa_switch_tree *dst;
 	u32 tree, index;
@@ -660,12 +661,12 @@ static int _dsa_register_switch(struct dsa_switch *ds, struct device_node *np)
 	return err;
 }
 
-int dsa_register_switch(struct dsa_switch *ds, struct device_node *np)
+int dsa_register_switch(struct dsa_switch *ds, struct device *dev)
 {
 	int err;
 
 	mutex_lock(&dsa2_mutex);
-	err = _dsa_register_switch(ds, np);
+	err = _dsa_register_switch(ds, dev);
 	mutex_unlock(&dsa2_mutex);
 
 	return err;
-- 
2.9.3

^ permalink raw reply related

* [PATCH net-next v4 02/10] net: dsa: Make most functions take a dsa_port argument
From: Florian Fainelli @ 2017-01-17 23:21 UTC (permalink / raw)
  To: netdev
  Cc: Andrew Lunn, Florian Fainelli, Jason Cooper, Vivien Didelot,
	Greg Kroah-Hartman, Russell King, open list, Gregory Clement,
	David S. Miller,
	moderated list:ARM/Marvell Dove/MV78xx0/Orion SOC support,
	Sebastian Hesselbarth
In-Reply-To: <20170117232152.1661-1-f.fainelli@gmail.com>

In preparation for allowing platform data, and therefore no valid
device_node pointer, make most DSA functions takes a pointer to a
dsa_port structure whenever possible. While at it, introduce a
dsa_port_is_valid() helper function which checks whether port->dn is
NULL or not at the moment.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/dsa/dsa.c      | 15 ++++++++------
 net/dsa/dsa2.c     | 61 +++++++++++++++++++++++++++++-------------------------
 net/dsa/dsa_priv.h |  4 ++--
 3 files changed, 44 insertions(+), 36 deletions(-)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index fd532487dfdf..2306d1b87c83 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -110,8 +110,9 @@ dsa_switch_probe(struct device *parent, struct device *host_dev, int sw_addr,
 
 /* basic switch operations **************************************************/
 int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev,
-		      struct device_node *port_dn, int port)
+		      struct dsa_port *dport, int port)
 {
+	struct device_node *port_dn = dport->dn;
 	struct phy_device *phydev;
 	int ret, mode;
 
@@ -141,15 +142,15 @@ int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev,
 
 static int dsa_cpu_dsa_setups(struct dsa_switch *ds, struct device *dev)
 {
-	struct device_node *port_dn;
+	struct dsa_port *dport;
 	int ret, port;
 
 	for (port = 0; port < DSA_MAX_PORTS; port++) {
 		if (!(dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)))
 			continue;
 
-		port_dn = ds->ports[port].dn;
-		ret = dsa_cpu_dsa_setup(ds, dev, port_dn, port);
+		dport = &ds->ports[port];
+		ret = dsa_cpu_dsa_setup(ds, dev, dport, port);
 		if (ret)
 			return ret;
 	}
@@ -366,8 +367,10 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
 	return ds;
 }
 
-void dsa_cpu_dsa_destroy(struct device_node *port_dn)
+void dsa_cpu_dsa_destroy(struct dsa_port *port)
 {
+	struct device_node *port_dn = port->dn;
+
 	if (of_phy_is_fixed_link(port_dn))
 		of_phy_deregister_fixed_link(port_dn);
 }
@@ -393,7 +396,7 @@ static void dsa_switch_destroy(struct dsa_switch *ds)
 	for (port = 0; port < DSA_MAX_PORTS; port++) {
 		if (!(dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)))
 			continue;
-		dsa_cpu_dsa_destroy(ds->ports[port].dn);
+		dsa_cpu_dsa_destroy(&ds->ports[port]);
 
 		/* Clearing a bit which is not set does no harm */
 		ds->cpu_port_mask |= ~(1 << port);
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 4170f7ea8e28..6e3675220fef 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -79,14 +79,19 @@ static void dsa_dst_del_ds(struct dsa_switch_tree *dst,
 	kref_put(&dst->refcount, dsa_free_dst);
 }
 
-static bool dsa_port_is_dsa(struct device_node *port)
+static bool dsa_port_is_valid(struct dsa_port *port)
 {
-	return !!of_parse_phandle(port, "link", 0);
+	return !!port->dn;
 }
 
-static bool dsa_port_is_cpu(struct device_node *port)
+static bool dsa_port_is_dsa(struct dsa_port *port)
 {
-	return !!of_parse_phandle(port, "ethernet", 0);
+	return !!of_parse_phandle(port->dn, "link", 0);
+}
+
+static bool dsa_port_is_cpu(struct dsa_port *port)
+{
+	return !!of_parse_phandle(port->dn, "ethernet", 0);
 }
 
 static bool dsa_ds_find_port(struct dsa_switch *ds,
@@ -120,7 +125,7 @@ static struct dsa_switch *dsa_dst_find_port(struct dsa_switch_tree *dst,
 
 static int dsa_port_complete(struct dsa_switch_tree *dst,
 			     struct dsa_switch *src_ds,
-			     struct device_node *port,
+			     struct dsa_port *port,
 			     u32 src_port)
 {
 	struct device_node *link;
@@ -128,7 +133,7 @@ static int dsa_port_complete(struct dsa_switch_tree *dst,
 	struct dsa_switch *dst_ds;
 
 	for (index = 0;; index++) {
-		link = of_parse_phandle(port, "link", index);
+		link = of_parse_phandle(port->dn, "link", index);
 		if (!link)
 			break;
 
@@ -151,13 +156,13 @@ static int dsa_port_complete(struct dsa_switch_tree *dst,
  */
 static int dsa_ds_complete(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 {
-	struct device_node *port;
+	struct dsa_port *port;
 	u32 index;
 	int err;
 
 	for (index = 0; index < DSA_MAX_PORTS; index++) {
-		port = ds->ports[index].dn;
-		if (!port)
+		port = &ds->ports[index];
+		if (!dsa_port_is_valid(port))
 			continue;
 
 		if (!dsa_port_is_dsa(port))
@@ -197,7 +202,7 @@ static int dsa_dst_complete(struct dsa_switch_tree *dst)
 	return 0;
 }
 
-static int dsa_dsa_port_apply(struct device_node *port, u32 index,
+static int dsa_dsa_port_apply(struct dsa_port *port, u32 index,
 			      struct dsa_switch *ds)
 {
 	int err;
@@ -212,13 +217,13 @@ static int dsa_dsa_port_apply(struct device_node *port, u32 index,
 	return 0;
 }
 
-static void dsa_dsa_port_unapply(struct device_node *port, u32 index,
+static void dsa_dsa_port_unapply(struct dsa_port *port, u32 index,
 				 struct dsa_switch *ds)
 {
 	dsa_cpu_dsa_destroy(port);
 }
 
-static int dsa_cpu_port_apply(struct device_node *port, u32 index,
+static int dsa_cpu_port_apply(struct dsa_port *port, u32 index,
 			      struct dsa_switch *ds)
 {
 	int err;
@@ -235,7 +240,7 @@ static int dsa_cpu_port_apply(struct device_node *port, u32 index,
 	return 0;
 }
 
-static void dsa_cpu_port_unapply(struct device_node *port, u32 index,
+static void dsa_cpu_port_unapply(struct dsa_port *port, u32 index,
 				 struct dsa_switch *ds)
 {
 	dsa_cpu_dsa_destroy(port);
@@ -243,13 +248,13 @@ static void dsa_cpu_port_unapply(struct device_node *port, u32 index,
 
 }
 
-static int dsa_user_port_apply(struct device_node *port, u32 index,
+static int dsa_user_port_apply(struct dsa_port *port, u32 index,
 			       struct dsa_switch *ds)
 {
 	const char *name;
 	int err;
 
-	name = of_get_property(port, "label", NULL);
+	name = of_get_property(port->dn, "label", NULL);
 	if (!name)
 		name = "eth%d";
 
@@ -263,7 +268,7 @@ static int dsa_user_port_apply(struct device_node *port, u32 index,
 	return 0;
 }
 
-static void dsa_user_port_unapply(struct device_node *port, u32 index,
+static void dsa_user_port_unapply(struct dsa_port *port, u32 index,
 				  struct dsa_switch *ds)
 {
 	if (ds->ports[index].netdev) {
@@ -275,7 +280,7 @@ static void dsa_user_port_unapply(struct device_node *port, u32 index,
 
 static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 {
-	struct device_node *port;
+	struct dsa_port *port;
 	u32 index;
 	int err;
 
@@ -309,8 +314,8 @@ static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 	}
 
 	for (index = 0; index < DSA_MAX_PORTS; index++) {
-		port = ds->ports[index].dn;
-		if (!port)
+		port = &ds->ports[index];
+		if (!dsa_port_is_valid(port))
 			continue;
 
 		if (dsa_port_is_dsa(port)) {
@@ -337,12 +342,12 @@ static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 
 static void dsa_ds_unapply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 {
-	struct device_node *port;
+	struct dsa_port *port;
 	u32 index;
 
 	for (index = 0; index < DSA_MAX_PORTS; index++) {
-		port = ds->ports[index].dn;
-		if (!port)
+		port = &ds->ports[index];
+		if (!dsa_port_is_valid(port))
 			continue;
 
 		if (dsa_port_is_dsa(port)) {
@@ -426,7 +431,7 @@ static void dsa_dst_unapply(struct dsa_switch_tree *dst)
 	dst->applied = false;
 }
 
-static int dsa_cpu_parse(struct device_node *port, u32 index,
+static int dsa_cpu_parse(struct dsa_port *port, u32 index,
 			 struct dsa_switch_tree *dst,
 			 struct dsa_switch *ds)
 {
@@ -434,7 +439,7 @@ static int dsa_cpu_parse(struct device_node *port, u32 index,
 	struct net_device *ethernet_dev;
 	struct device_node *ethernet;
 
-	ethernet = of_parse_phandle(port, "ethernet", 0);
+	ethernet = of_parse_phandle(port->dn, "ethernet", 0);
 	if (!ethernet)
 		return -EINVAL;
 
@@ -467,13 +472,13 @@ static int dsa_cpu_parse(struct device_node *port, u32 index,
 
 static int dsa_ds_parse(struct dsa_switch_tree *dst, struct dsa_switch *ds)
 {
-	struct device_node *port;
+	struct dsa_port *port;
 	u32 index;
 	int err;
 
 	for (index = 0; index < DSA_MAX_PORTS; index++) {
-		port = ds->ports[index].dn;
-		if (!port)
+		port = &ds->ports[index];
+		if (!dsa_port_is_valid(port))
 			continue;
 
 		if (dsa_port_is_cpu(port)) {
@@ -534,7 +539,7 @@ static int dsa_parse_ports_dn(struct device_node *ports, struct dsa_switch *ds)
 		 * to have access to a correct value, just like what
 		 * net/dsa/dsa.c::dsa_switch_setup_one does.
 		 */
-		if (!dsa_port_is_cpu(port))
+		if (!dsa_port_is_cpu(&ds->ports[reg]))
 			ds->enabled_port_mask |= 1 << reg;
 	}
 
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 7e3385ec73f4..a015ec97c289 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -50,8 +50,8 @@ struct dsa_slave_priv {
 
 /* dsa.c */
 int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev,
-		      struct device_node *port_dn, int port);
-void dsa_cpu_dsa_destroy(struct device_node *port_dn);
+		      struct dsa_port *dport, int port);
+void dsa_cpu_dsa_destroy(struct dsa_port *dport);
 const struct dsa_device_ops *dsa_resolve_tag_protocol(int tag_protocol);
 int dsa_cpu_port_ethtool_setup(struct dsa_switch *ds);
 void dsa_cpu_port_ethtool_restore(struct dsa_switch *ds);
-- 
2.9.3

^ permalink raw reply related

* [PATCH net-next v4 03/10] net: dsa: Suffix function manipulating device_node with _dn
From: Florian Fainelli @ 2017-01-17 23:21 UTC (permalink / raw)
  To: netdev
  Cc: Andrew Lunn, Florian Fainelli, Jason Cooper, Vivien Didelot,
	Greg Kroah-Hartman, Russell King, open list, Gregory Clement,
	David S. Miller,
	moderated list:ARM/Marvell Dove/MV78xx0/Orion SOC support,
	Sebastian Hesselbarth
In-Reply-To: <20170117232152.1661-1-f.fainelli@gmail.com>

Make it clear that these functions take a device_node structure pointer

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/dsa/dsa2.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 6e3675220fef..04ab62251fe3 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -94,8 +94,8 @@ static bool dsa_port_is_cpu(struct dsa_port *port)
 	return !!of_parse_phandle(port->dn, "ethernet", 0);
 }
 
-static bool dsa_ds_find_port(struct dsa_switch *ds,
-			     struct device_node *port)
+static bool dsa_ds_find_port_dn(struct dsa_switch *ds,
+				struct device_node *port)
 {
 	u32 index;
 
@@ -105,8 +105,8 @@ static bool dsa_ds_find_port(struct dsa_switch *ds,
 	return false;
 }
 
-static struct dsa_switch *dsa_dst_find_port(struct dsa_switch_tree *dst,
-					    struct device_node *port)
+static struct dsa_switch *dsa_dst_find_port_dn(struct dsa_switch_tree *dst,
+					       struct device_node *port)
 {
 	struct dsa_switch *ds;
 	u32 index;
@@ -116,7 +116,7 @@ static struct dsa_switch *dsa_dst_find_port(struct dsa_switch_tree *dst,
 		if (!ds)
 			continue;
 
-		if (dsa_ds_find_port(ds, port))
+		if (dsa_ds_find_port_dn(ds, port))
 			return ds;
 	}
 
@@ -137,7 +137,7 @@ static int dsa_port_complete(struct dsa_switch_tree *dst,
 		if (!link)
 			break;
 
-		dst_ds = dsa_dst_find_port(dst, link);
+		dst_ds = dsa_dst_find_port_dn(dst, link);
 		of_node_put(link);
 
 		if (!dst_ds)
@@ -546,7 +546,7 @@ static int dsa_parse_ports_dn(struct device_node *ports, struct dsa_switch *ds)
 	return 0;
 }
 
-static int dsa_parse_member(struct device_node *np, u32 *tree, u32 *index)
+static int dsa_parse_member_dn(struct device_node *np, u32 *tree, u32 *index)
 {
 	int err;
 
@@ -592,7 +592,7 @@ static int _dsa_register_switch(struct dsa_switch *ds, struct device *dev)
 	u32 tree, index;
 	int i, err;
 
-	err = dsa_parse_member(np, &tree, &index);
+	err = dsa_parse_member_dn(np, &tree, &index);
 	if (err)
 		return err;
 
-- 
2.9.3

^ permalink raw reply related

* [PATCH net-next v4 04/10] net: dsa: Move ports assignment closer to error checking
From: Florian Fainelli @ 2017-01-17 23:21 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, Jason Cooper, Andrew Lunn,
	Sebastian Hesselbarth, Gregory Clement, Russell King,
	Vivien Didelot, David S. Miller, Greg Kroah-Hartman,
	moderated list:ARM/Marvell Dove/MV78xx0/Orion SOC support,
	open list
In-Reply-To: <20170117232152.1661-1-f.fainelli@gmail.com>

Move the assignment of ports in _dsa_register_switch() closer to where
it is checked, no functional change. Re-order declarations to be
preserve the inverted christmas tree style.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/dsa/dsa2.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 04ab62251fe3..cd91070b5467 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -587,8 +587,8 @@ static struct device_node *dsa_get_ports(struct dsa_switch *ds,
 static int _dsa_register_switch(struct dsa_switch *ds, struct device *dev)
 {
 	struct device_node *np = dev->of_node;
-	struct device_node *ports = dsa_get_ports(ds, np);
 	struct dsa_switch_tree *dst;
+	struct device_node *ports;
 	u32 tree, index;
 	int i, err;
 
@@ -596,6 +596,7 @@ static int _dsa_register_switch(struct dsa_switch *ds, struct device *dev)
 	if (err)
 		return err;
 
+	ports = dsa_get_ports(ds, np);
 	if (IS_ERR(ports))
 		return PTR_ERR(ports);
 
-- 
2.9.3

^ permalink raw reply related


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