* Re: [PATCH net-next 1/8] net/ncsi: Avoid unused-value build warning from ia64-linux-gcc
From: Gavin Shan @ 2016-09-29 11:40 UTC (permalink / raw)
To: David Miller; +Cc: gwshan, netdev, joel, yuvali, benh
In-Reply-To: <20160929.015404.76401694814645494.davem@davemloft.net>
On Thu, Sep 29, 2016 at 01:54:04AM -0400, David Miller wrote:
>From: Gavin Shan <gwshan@linux.vnet.ibm.com>
>Date: Thu, 29 Sep 2016 15:03:08 +1000
>
>> This replaces the atomic access to NCSI channel's state with READ_ONCE()
>> and WRITE_ONCE() to avoid the above build warning. We needn't hold the
>> channel's lock when updating its state as well. No logical changes
>> introduced.
>
>I don't understand this.
>
>If it's important to take the lock for the list add/del, then it must
>be important to make the state change appear atomic wrt. that lock as
>well.
>
>Can parallel threads of control enter these functions which change the
>state? If so, then you need to make the state changes under the lock.
>In fact, you probably have to make the state tests under the locks as
>well.
>
>If not, please explain what prevents it from happening.
>
Dave, thanks for your comments. I think it's occasionally working on
AST2400 and AST2500 platforms. It's reasonable to grab the lock before
fetching or updating the NCSI channel's state. Adding and removing the
channel from the list also need taking the lock as well. I will modify
the code accordingly in next revision.
AST2400/AST2500 has single CPU. The channel's state (and the linked
list) are changed in softirq context (packet Rx handler or timer),
meaning they are not accessed in parallel mode. However, NCSI stack
cannot make assumption to be run on single CPU platforms only. So
yes, we need the lock to protect them.
Thanks,
Gavin
^ permalink raw reply
* Request for backport to 4.4 stable
From: Jorgen S. Hansen @ 2016-09-29 12:02 UTC (permalink / raw)
To: netdev@vger.kernel.org
Hello,
It would be great if the following commit could be backported to 4.4 stable. The commit fixes a bug that was introduced in 4.3 and fixed in 4.6. The bug may cause wrongful termination of vSocket connections, if there is more than one active vSocket connection in a given VM. This affects VMware services running inside VMs:
commit 8ab18d71de8b07d2c4d6f984b718418c09ea45c5
VSOCK: Detach QP check should filter out non matching QPs.
The check in vmci_transport_peer_detach_cb should only allow a
detach when the qp handle of the transport matches the one in
the detach message.
Thanks,
Jørgen
^ permalink raw reply
* [PATCH] af_unix: fix garbage collect vs. MSG_PEEK
From: Miklos Szeredi @ 2016-09-29 12:09 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, hannes, kernel, davem
Gc assumes that in-flight sockets that don't have an external ref can't
gain one while unix_gc_lock is held. That is true because
unix_notinflight() will be called before detaching fds, which takes
unix_gc_lock.
Only MSG_PEEK was somehow overlooked. That one also clones the fds, also
keeping them in the skb. But through MSG_PEEK an external reference can
definitely be gained without ever touching unix_gc_lock.
This patch adds unix_gc_barrier() that waits for a garbage collect run to
finish (if there is one), before actually installing the peeked in-flight
files to file descriptors. This prevents problems from a pure in-flight
socket having its buffers modified while the garbage collect is taking
place.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Cc: <stable@vger.kernel.org>
---
include/net/af_unix.h | 1 +
net/unix/af_unix.c | 15 +++++++++++++--
net/unix/garbage.c | 5 +++++
3 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/include/net/af_unix.h b/include/net/af_unix.h
index fd60eccb59a6..f73ce09da2b4 100644
--- a/include/net/af_unix.h
+++ b/include/net/af_unix.h
@@ -10,6 +10,7 @@ void unix_inflight(struct user_struct *user, struct file *fp);
void unix_notinflight(struct user_struct *user, struct file *fp);
void unix_gc(void);
void wait_for_unix_gc(void);
+void unix_gc_barrier(void);
struct sock *unix_get_socket(struct file *filp);
struct sock *unix_peer_get(struct sock *);
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 8309687a56b0..cece344b41b8 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1550,6 +1550,17 @@ static int unix_attach_fds(struct scm_cookie *scm, struct sk_buff *skb)
return max_level;
}
+static void unix_peek_fds(struct scm_cookie *scm, struct sk_buff *skb)
+{
+ scm->fp = scm_fp_dup(UNIXCB(skb).fp);
+ /*
+ * During garbage collection it is assumed that in-flight sockets don't
+ * get a new external reference. So we need to wait until current run
+ * finishes.
+ */
+ unix_gc_barrier();
+}
+
static int unix_scm_to_skb(struct scm_cookie *scm, struct sk_buff *skb, bool send_fds)
{
int err = 0;
@@ -2182,7 +2193,7 @@ static int unix_dgram_recvmsg(struct socket *sock, struct msghdr *msg,
sk_peek_offset_fwd(sk, size);
if (UNIXCB(skb).fp)
- scm.fp = scm_fp_dup(UNIXCB(skb).fp);
+ unix_peek_fds(&scm, skb);
}
err = (flags & MSG_TRUNC) ? skb->len - skip : size;
@@ -2422,7 +2433,7 @@ unlock:
/* It is questionable, see note in unix_dgram_recvmsg.
*/
if (UNIXCB(skb).fp)
- scm.fp = scm_fp_dup(UNIXCB(skb).fp);
+ unix_peek_fds(&scm, skb);
sk_peek_offset_fwd(sk, chunk);
diff --git a/net/unix/garbage.c b/net/unix/garbage.c
index 6a0d48525fcf..d4fb6ff8024d 100644
--- a/net/unix/garbage.c
+++ b/net/unix/garbage.c
@@ -266,6 +266,11 @@ void wait_for_unix_gc(void)
wait_event(unix_gc_wait, gc_in_progress == false);
}
+void unix_gc_barrier(void)
+{
+ spin_unlock_wait(&unix_gc_lock);
+}
+
/* The external entry point: unix_gc() */
void unix_gc(void)
{
--
2.5.5
^ permalink raw reply related
* Re: [PATCH net v2] L2TP:Adjust intf MTU,factor underlay L3,overlay L2
From: Jiri Benc @ 2016-09-29 12:21 UTC (permalink / raw)
To: R. Parameswaran
Cc: David Miller, kleptog, jchapman, netdev, linux-kernel, nprachan,
rshearma, dfawcus, stephen, acme, lboccass
In-Reply-To: <alpine.DEB.2.11.1609281857050.17015@duvvury48.eng.vyatta.net>
On Wed, 28 Sep 2016 19:36:45 -0700 (PDT), R. Parameswaran wrote:
> I'd like to point out one difference with VXLAN - in VXLAN, the
> local physical interface is directly specified at the time of
> creation of the tunnel, and the data structure seems to have the ifindex
> of the local interface with which it is able to directly pull up the
> underlay interface device. Whereas in L2TP, we only have the IP
> address of the remote tunnel end-point and thus only the socket and the
> dst from which we need to derive this.
Strictly speaking, VXLAN *may* know the underlying interface. It can
also be set up with just local and remote IP address, or even worse, in
metadata mode where we don't know the address nor the interface until
we get a packet (and each packet may have those different).
MTU wise, those cases are not accommodated for in the kernel. The vxlan
interface gets MTU of 1500 and it's up to the administrator to set it
correctly.
Btw, PMTU events won't help with the metadata mode. And even in
"normal" mode, it's not clear what should be done - the tunnel
interface may be in a bridge, thus there may be other interfaces that
depend on the same MTU, up to inside VMs.
Jiri
^ permalink raw reply
* Re: [PATCH RFC iproute2] iplink: Support envhdrlen
From: Jiri Benc @ 2016-09-29 12:49 UTC (permalink / raw)
To: Toshiaki Makita
Cc: netdev, Patrick McHardy, Stephen Hemminger, Vlad Yasevich,
Jeff Kirsher
In-Reply-To: <1474966541-4420-5-git-send-email-makita.toshiaki@lab.ntt.co.jp>
On Tue, 27 Sep 2016 17:55:40 +0900, Toshiaki Makita wrote:
> This adds support for envhdrlen.
>
> Example:
> # ip link set eno1 envhdrlen 8
I don't see why this should be user visible, let alone requiring user
to set it. This should be transparent, kernel should compute the value
as needed based on the configuration and set it up. Requiring the
administrator to pick up a calculator and sum up all the vlan, mpls and
whatever header lengths is silly.
I realize that we currently have no easy way to do that. Especially
with lwtunnels and stuff line MPLS where we don't easily know the
number of tags. But every uAPI we introduce will have to be supported
forever and going a particular way just because it is easy to implement
is not sustainable.
At the very least, it should be configurable from the other direction.
I.e. telling which interfaces can be used by vlans or MPLS (if it
cannot be inferred automatically) and configuring maximum number of
tags on the given vlan/mpls/whatever interface/route/whatever.
Jiri
^ permalink raw reply
* Re: [PATCH v2 net-next 4/5] xps_flows: XPS for packets that don't have a socket
From: Tom Herbert @ 2016-09-29 12:53 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S. Miller, Linux Kernel Network Developers, Kernel Team,
Rick Jones, Alexander Duyck
In-Reply-To: <1475124851.28155.135.camel@edumazet-glaptop3.roam.corp.google.com>
On Thu, Sep 29, 2016 at 12:54 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2016-09-28 at 20:54 -0700, Tom Herbert wrote:
>> xps_flows maintains a per device flow table that is indexed by the
>> skbuff hash. The table is only consulted when there is no queue saved in
>> a transmit socket for an skbuff.
>>
>> Each entry in the flow table contains a queue index and a queue
>> pointer. The queue pointer is set when a queue is chosen using a
>> flow table entry. This pointer is set to the head pointer in the
>> transmit queue (which is maintained by BQL).
>>
>> The new function get_xps_flows_index that looks up flows in the
>> xps_flows table. The entry returned gives the last queue a matching flow
>> used. The returned queue is compared against the normal XPS queue. If
>> they are different, then we only switch if the tail pointer in the TX
>> queue has advanced past the pointer saved in the entry. In this
>> way OOO should be avoided when XPS wants to use a different queue.
>
> There is something I do not understand with this.
>
> If this OOO avoidance is tied to BQL, it means that packets sitting in a
> qdisc wont be part of the detection.
>
> So packets of flow X could possibly be queued on multiple qdiscs.
>
Hi Eric,
I'm not sure I understand your concern. If packets for flow X can be
queued on multiple qdiscs wouldn't that mean that flow will be subject
to ooo transmission regardless of this patch? In other words here
we're trying to keep packets for the flow in order as they are emitted
from the qdiscs which we assume maintains ordering of packets in a
flow.
Tom
>
>
^ permalink raw reply
* Re: [PATCH v2 net-next 4/5] xps_flows: XPS for packets that don't have a socket
From: Eric Dumazet @ 2016-09-29 13:18 UTC (permalink / raw)
To: Tom Herbert
Cc: David S. Miller, Linux Kernel Network Developers, Kernel Team,
Rick Jones, Alexander Duyck
In-Reply-To: <CALx6S36FsjSioksB+O7Lp7-7PcC8iMc9LWqWiFr8RzFJ=GkKGQ@mail.gmail.com>
On Thu, 2016-09-29 at 08:53 -0400, Tom Herbert wrote:
> On Thu, Sep 29, 2016 at 12:54 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > On Wed, 2016-09-28 at 20:54 -0700, Tom Herbert wrote:
> >> xps_flows maintains a per device flow table that is indexed by the
> >> skbuff hash. The table is only consulted when there is no queue saved in
> >> a transmit socket for an skbuff.
> >>
> >> Each entry in the flow table contains a queue index and a queue
> >> pointer. The queue pointer is set when a queue is chosen using a
> >> flow table entry. This pointer is set to the head pointer in the
> >> transmit queue (which is maintained by BQL).
> >>
> >> The new function get_xps_flows_index that looks up flows in the
> >> xps_flows table. The entry returned gives the last queue a matching flow
> >> used. The returned queue is compared against the normal XPS queue. If
> >> they are different, then we only switch if the tail pointer in the TX
> >> queue has advanced past the pointer saved in the entry. In this
> >> way OOO should be avoided when XPS wants to use a different queue.
> >
> > There is something I do not understand with this.
> >
> > If this OOO avoidance is tied to BQL, it means that packets sitting in a
> > qdisc wont be part of the detection.
> >
> > So packets of flow X could possibly be queued on multiple qdiscs.
> >
> Hi Eric,
>
> I'm not sure I understand your concern. If packets for flow X can be
> queued on multiple qdiscs wouldn't that mean that flow will be subject
> to ooo transmission regardless of this patch? In other words here
> we're trying to keep packets for the flow in order as they are emitted
> from the qdiscs which we assume maintains ordering of packets in a
> flow.
Well, then what this patch series is solving ?
You have a producer of packets running on 8 vcpus in a VM.
Packets are exiting the VM and need to be queued on a mq NIC in the
hypervisor.
Flow X can be scheduled on any of these 8 vcpus, so XPS is currently
selecting different TXQ.
Your patch aims to detect this and steer packets to one TXQ, but not
using a static hash() % num_real_queues (as RSS would have done on a NIC
at RX), but trying to please XPS (as : selecting a queue close to the
CPU producing the packet. VM with one vcpu, and cpu bounded, would
really be happy to not spread packets all over the queues)
Your mechanism relies on counters updated at the time packets are given
to the NIC, not at the time packets are given to the txq (and eventually
sit for a while in the qdisc right before BQL layer)
dev_queue_xmit() is not talking to the device directly...
All I am saying is that the producer/consumer counters you added are not
at the right place.
You tried hard to not change the drivers, by adding something to
existing BQL. But packets can sit in a qdisc for a while...
So you added 2 potential cache lines misses per outgoing packet, but
with no guarantee that OOO are really avoided.
^ permalink raw reply
* Re: [PATCH net-next v3 2/3] udp: implement memory accounting helpers
From: Eric Dumazet @ 2016-09-29 13:24 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, David S. Miller, James Morris,
Trond Myklebust, Alexander Duyck, Daniel Borkmann, Eric Dumazet,
Tom Herbert, Hannes Frederic Sowa, Edward Cree,
linux-nfs-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1475141514.4676.28.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Thu, 2016-09-29 at 11:31 +0200, Paolo Abeni wrote:
> On Wed, 2016-09-28 at 18:42 -0700, Eric Dumazet wrote:
> > On Wed, 2016-09-28 at 12:52 +0200, Paolo Abeni wrote:
> >
> > > +static void udp_rmem_release(struct sock *sk, int partial)
> > > +{
> > > + struct udp_sock *up = udp_sk(sk);
> > > + int fwd, amt;
> > > +
> > > + if (partial && !udp_under_memory_pressure(sk))
> > > + return;
> > > +
> > > + /* we can have concurrent release; if we catch any conflict
> > > + * we let only one of them do the work
> > > + */
> > > + if (atomic_dec_if_positive(&up->can_reclaim) < 0)
> > > + return;
> > > +
> > > + fwd = __udp_forward(up, atomic_read(&sk->sk_rmem_alloc));
> > > + if (fwd < SK_MEM_QUANTUM + partial) {
> > > + atomic_inc(&up->can_reclaim);
> > > + return;
> > > + }
> > > +
> > > + amt = (fwd - partial) & ~(SK_MEM_QUANTUM - 1);
> > > + atomic_sub(amt, &up->mem_allocated);
> > > + atomic_inc(&up->can_reclaim);
> > > +
> > > + __sk_mem_reduce_allocated(sk, amt >> SK_MEM_QUANTUM_SHIFT);
> > > + sk->sk_forward_alloc = fwd - amt;
> > > +}
> >
> >
> > This is racy... all these atomics make me nervous...
>
> Ah, perhaps I got it: if we have a concurrent memory scheduling, we
> could end up with a value of mem_allocated below the real need.
>
> That mismatch will not drift: at worst we can end up with mem_allocated
> being single SK_MEM_QUANTUM below what is strictly needed.
>
> A possible alternative could be:
>
> static void udp_rmem_release(struct sock *sk, int partial)
> {
> struct udp_sock *up = udp_sk(sk);
> int fwd, amt, alloc_old, alloc;
>
> if (partial && !udp_under_memory_pressure(sk))
> return;
>
> alloc = atomic_read(&up->mem_allocated);
> fwd = alloc - atomic_read(&sk->sk_rmem_alloc);
> if (fwd < SK_MEM_QUANTUM + partial)
> return;
>
> amt = (fwd - partial) & ~(SK_MEM_QUANTUM - 1);
> alloc_old = atomic_cmpxchg(&up->mem_allocated, alloc, alloc - amt);
> /* if a concurrent update is detected, just do nothing; if said update
> * is due to another memory release, that release take care of
> * reclaiming the memory for us, too.
> * Otherwise we will be able to release on later dequeue, since
> * we will eventually stop colliding with the writer when it will
> * consume all the fwd allocated memory
> */
> if (alloc_old != alloc)
> return;
>
> __sk_mem_reduce_allocated(sk, amt >> SK_MEM_QUANTUM_SHIFT);
> sk->sk_forward_alloc = fwd - amt;
Can still be done from multiple cpus.
Add some ndelay() or udelay() before to simulate fact that current cpu
could be interrupted by an NMI handler (perf for example)... or hard IRQ
handler...
Then make sure your tests involve 16 concurrent cpus dealing with one
udp socket...
> }
>
> which is even more lazy in reclaiming but should never underestimate the
> needed forward allocation, and under pressure should eventually free the
> needed memory.
If this code is rarely used, why don't you simply use a real spinlock,
so that we do not have to worry about all this ?
A spinlock acquisition/release is a _single_ locked operation.
Faster than the 3 atomic you got in last version.
spinlock code (ticket or MCS) avoids starvation.
Then, you can safely update multiple fields in the socket.
And you get nice lockdep support as a bonus.
cmpxchg() is fine when a single field need an exclusion. But there you
have multiple fields to update at once :
sk_memory_allocated_add() and sk_memory_allocated_sub() can work using
atomic_long_add_return() and atomic_long_sub() because their caller owns
the socket lock and can safely update sk->sk_forward_alloc without
additional locking, but UDP wont have this luxury after your patches.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [ANNOUNCE] Netdev 1.2 program announced
From: Hajime Tazaki @ 2016-09-29 13:35 UTC (permalink / raw)
To: netdev, netfilter-devel, linux-wireless, people, lwn
Hello,
The program of Linux netdev 1.2 is now online !
http://netdevconf.org/1.2/schedule.html
As we stated, all of registered people is now in the mailing
list, people at netdevconf.org. If you wish to communicate
with the attendees of netdev 1.2, feel free to exchange
messages.
-- Hajime
^ permalink raw reply
* Re: [PATCH net-next v3 2/3] udp: implement memory accounting helpers
From: Paolo Abeni @ 2016-09-29 14:01 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, David S. Miller, James Morris,
Trond Myklebust, Alexander Duyck, Daniel Borkmann, Eric Dumazet,
Tom Herbert, Hannes Frederic Sowa, Edward Cree,
linux-nfs-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1475155472.28155.164.camel-XN9IlZ5yJG9HTL0Zs8A6p+yfmBU6pStAUsxypvmhUTTZJqsBc5GL+g@public.gmane.org>
On Thu, 2016-09-29 at 06:24 -0700, Eric Dumazet wrote:
> On Thu, 2016-09-29 at 11:31 +0200, Paolo Abeni wrote:
> > On Wed, 2016-09-28 at 18:42 -0700, Eric Dumazet wrote:
> > > On Wed, 2016-09-28 at 12:52 +0200, Paolo Abeni wrote:
> > >
> > > > +static void udp_rmem_release(struct sock *sk, int partial)
> > > > +{
> > > > + struct udp_sock *up = udp_sk(sk);
> > > > + int fwd, amt;
> > > > +
> > > > + if (partial && !udp_under_memory_pressure(sk))
> > > > + return;
> > > > +
> > > > + /* we can have concurrent release; if we catch any conflict
> > > > + * we let only one of them do the work
> > > > + */
> > > > + if (atomic_dec_if_positive(&up->can_reclaim) < 0)
> > > > + return;
> > > > +
> > > > + fwd = __udp_forward(up, atomic_read(&sk->sk_rmem_alloc));
> > > > + if (fwd < SK_MEM_QUANTUM + partial) {
> > > > + atomic_inc(&up->can_reclaim);
> > > > + return;
> > > > + }
> > > > +
> > > > + amt = (fwd - partial) & ~(SK_MEM_QUANTUM - 1);
> > > > + atomic_sub(amt, &up->mem_allocated);
> > > > + atomic_inc(&up->can_reclaim);
> > > > +
> > > > + __sk_mem_reduce_allocated(sk, amt >> SK_MEM_QUANTUM_SHIFT);
> > > > + sk->sk_forward_alloc = fwd - amt;
> > > > +}
> > >
> > >
> > > This is racy... all these atomics make me nervous...
> >
> > Ah, perhaps I got it: if we have a concurrent memory scheduling, we
> > could end up with a value of mem_allocated below the real need.
> >
> > That mismatch will not drift: at worst we can end up with mem_allocated
> > being single SK_MEM_QUANTUM below what is strictly needed.
> >
> > A possible alternative could be:
> >
> > static void udp_rmem_release(struct sock *sk, int partial)
> > {
> > struct udp_sock *up = udp_sk(sk);
> > int fwd, amt, alloc_old, alloc;
> >
> > if (partial && !udp_under_memory_pressure(sk))
> > return;
> >
> > alloc = atomic_read(&up->mem_allocated);
> > fwd = alloc - atomic_read(&sk->sk_rmem_alloc);
> > if (fwd < SK_MEM_QUANTUM + partial)
> > return;
> >
> > amt = (fwd - partial) & ~(SK_MEM_QUANTUM - 1);
> > alloc_old = atomic_cmpxchg(&up->mem_allocated, alloc, alloc - amt);
> > /* if a concurrent update is detected, just do nothing; if said update
> > * is due to another memory release, that release take care of
> > * reclaiming the memory for us, too.
> > * Otherwise we will be able to release on later dequeue, since
> > * we will eventually stop colliding with the writer when it will
> > * consume all the fwd allocated memory
> > */
> > if (alloc_old != alloc)
> > return;
> >
> > __sk_mem_reduce_allocated(sk, amt >> SK_MEM_QUANTUM_SHIFT);
> > sk->sk_forward_alloc = fwd - amt;
>
> Can still be done from multiple cpus.
>
> Add some ndelay() or udelay() before to simulate fact that current cpu
> could be interrupted by an NMI handler (perf for example)... or hard IRQ
> handler...
>
> Then make sure your tests involve 16 concurrent cpus dealing with one
> udp socket...
Thank you again reviewing this.
I'm working to this sort of tests right now.
>
> > }
> >
> > which is even more lazy in reclaiming but should never underestimate the
> > needed forward allocation, and under pressure should eventually free the
> > needed memory.
>
>
> If this code is rarely used, why don't you simply use a real spinlock,
> so that we do not have to worry about all this ?
>
> A spinlock acquisition/release is a _single_ locked operation.
> Faster than the 3 atomic you got in last version.
> spinlock code (ticket or MCS) avoids starvation.
I'd like to avoid adding a lock, if possible, to avoid any possible
source of contention.
> Then, you can safely update multiple fields in the socket.
>
> And you get nice lockdep support as a bonus.
>
> cmpxchg() is fine when a single field need an exclusion. But there you
> have multiple fields to update at once :
>
> sk_memory_allocated_add() and sk_memory_allocated_sub() can work using
> atomic_long_add_return() and atomic_long_sub() because their caller owns
> the socket lock and can safely update sk->sk_forward_alloc without
> additional locking, but UDP wont have this luxury after your patches.
When we reach __sk_mem_reduce_allocated() we are sure we can free the
specified amount of memory, so we only need to ensure consistent
sk_prot->memory_allocated updates. The current atomic operation suffices
to this.
Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH net] net: pktgen: fix pkt_size
From: Paolo Abeni @ 2016-09-29 14:04 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Bogdan Hamciuc, Ben Greear
The commit 879c7220e828 ("net: pktgen: Observe needed_headroom
of the device") increased the 'pkt_overhead' field value by
LL_RESERVED_SPACE.
As a side effect the generated packet size, computed as:
/* Eth + IPh + UDPh + mpls */
datalen = pkt_dev->cur_pkt_size - 14 - 20 - 8 -
pkt_dev->pkt_overhead;
is decreased by the same value.
The above changed slightly the behavior of existing pktgen users,
and made the interface somewhat inconsistent.
Fix it by restoring the previous pkt_overhead value and using
LL_RESERVED_SPACE as extralen in skb allocation.
Also, change pktgen_alloc_skb() to only partially reserve
the headroom to allow the caller to prefetch from ll header
start.
Fixes: 879c7220e828 ("net: pktgen: Observe needed_headroom of the device")
Suggested-by: Ben Greear <greearb@candelatech.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/core/pktgen.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index bbd118b..5c9b397 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -2286,7 +2286,7 @@ out:
static inline void set_pkt_overhead(struct pktgen_dev *pkt_dev)
{
- pkt_dev->pkt_overhead = LL_RESERVED_SPACE(pkt_dev->odev);
+ pkt_dev->pkt_overhead = 0;
pkt_dev->pkt_overhead += pkt_dev->nr_labels*sizeof(u32);
pkt_dev->pkt_overhead += VLAN_TAG_SIZE(pkt_dev);
pkt_dev->pkt_overhead += SVLAN_TAG_SIZE(pkt_dev);
@@ -2777,13 +2777,13 @@ static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb,
}
static struct sk_buff *pktgen_alloc_skb(struct net_device *dev,
- struct pktgen_dev *pkt_dev,
- unsigned int extralen)
+ struct pktgen_dev *pkt_dev)
{
+ unsigned int extralen = LL_RESERVED_SPACE(dev);
struct sk_buff *skb = NULL;
- unsigned int size = pkt_dev->cur_pkt_size + 64 + extralen +
- pkt_dev->pkt_overhead;
+ unsigned int size;
+ size = pkt_dev->cur_pkt_size + 64 + extralen + pkt_dev->pkt_overhead;
if (pkt_dev->flags & F_NODE) {
int node = pkt_dev->node >= 0 ? pkt_dev->node : numa_node_id();
@@ -2796,8 +2796,9 @@ static struct sk_buff *pktgen_alloc_skb(struct net_device *dev,
skb = __netdev_alloc_skb(dev, size, GFP_NOWAIT);
}
+ /* the caller prefetch from skb->data and reserve for the mac hdr */
if (likely(skb))
- skb_reserve(skb, LL_RESERVED_SPACE(dev));
+ skb_reserve(skb, extralen - 16);
return skb;
}
@@ -2830,16 +2831,14 @@ static struct sk_buff *fill_packet_ipv4(struct net_device *odev,
mod_cur_headers(pkt_dev);
queue_map = pkt_dev->cur_queue_map;
- datalen = (odev->hard_header_len + 16) & ~0xf;
-
- skb = pktgen_alloc_skb(odev, pkt_dev, datalen);
+ skb = pktgen_alloc_skb(odev, pkt_dev);
if (!skb) {
sprintf(pkt_dev->result, "No memory");
return NULL;
}
prefetchw(skb->data);
- skb_reserve(skb, datalen);
+ skb_reserve(skb, 16);
/* Reserve for ethernet and IP header */
eth = (__u8 *) skb_push(skb, 14);
@@ -2959,7 +2958,7 @@ static struct sk_buff *fill_packet_ipv6(struct net_device *odev,
mod_cur_headers(pkt_dev);
queue_map = pkt_dev->cur_queue_map;
- skb = pktgen_alloc_skb(odev, pkt_dev, 16);
+ skb = pktgen_alloc_skb(odev, pkt_dev);
if (!skb) {
sprintf(pkt_dev->result, "No memory");
return NULL;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v2 net-next 4/5] xps_flows: XPS for packets that don't have a socket
From: Tom Herbert @ 2016-09-29 14:08 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S. Miller, Linux Kernel Network Developers, Kernel Team,
Rick Jones, Alexander Duyck
In-Reply-To: <1475155080.28155.159.camel@edumazet-glaptop3.roam.corp.google.com>
On Thu, Sep 29, 2016 at 9:18 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2016-09-29 at 08:53 -0400, Tom Herbert wrote:
>> On Thu, Sep 29, 2016 at 12:54 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> > On Wed, 2016-09-28 at 20:54 -0700, Tom Herbert wrote:
>> >> xps_flows maintains a per device flow table that is indexed by the
>> >> skbuff hash. The table is only consulted when there is no queue saved in
>> >> a transmit socket for an skbuff.
>> >>
>> >> Each entry in the flow table contains a queue index and a queue
>> >> pointer. The queue pointer is set when a queue is chosen using a
>> >> flow table entry. This pointer is set to the head pointer in the
>> >> transmit queue (which is maintained by BQL).
>> >>
>> >> The new function get_xps_flows_index that looks up flows in the
>> >> xps_flows table. The entry returned gives the last queue a matching flow
>> >> used. The returned queue is compared against the normal XPS queue. If
>> >> they are different, then we only switch if the tail pointer in the TX
>> >> queue has advanced past the pointer saved in the entry. In this
>> >> way OOO should be avoided when XPS wants to use a different queue.
>> >
>> > There is something I do not understand with this.
>> >
>> > If this OOO avoidance is tied to BQL, it means that packets sitting in a
>> > qdisc wont be part of the detection.
>> >
>> > So packets of flow X could possibly be queued on multiple qdiscs.
>> >
>> Hi Eric,
>>
>> I'm not sure I understand your concern. If packets for flow X can be
>> queued on multiple qdiscs wouldn't that mean that flow will be subject
>> to ooo transmission regardless of this patch? In other words here
>> we're trying to keep packets for the flow in order as they are emitted
>> from the qdiscs which we assume maintains ordering of packets in a
>> flow.
>
> Well, then what this patch series is solving ?
>
It addresses the issue that Rick Jones pointed out was happening with
XPS. When packets are sent for a flow that has no socket and XPS is
enabled then each packet uses the XPS queue based on the running CPU.
Since the thread sending on a flow can be rescheduled on different
CPUs this is creating ooo packets. In this case the ooo is being
caused by interaction with XPS.
> You have a producer of packets running on 8 vcpus in a VM.
>
> Packets are exiting the VM and need to be queued on a mq NIC in the
> hypervisor.
>
> Flow X can be scheduled on any of these 8 vcpus, so XPS is currently
> selecting different TXQ.
>
> Your patch aims to detect this and steer packets to one TXQ, but not
> using a static hash() % num_real_queues (as RSS would have done on a NIC
> at RX), but trying to please XPS (as : selecting a queue close to the
> CPU producing the packet. VM with one vcpu, and cpu bounded, would
> really be happy to not spread packets all over the queues)
>
You're not describing this patch, you're describing how XPS works in
the first place. This patch is done to make socketless packets work
well with XPS. If all you want is a static hash() % num_real_queues
then just disable XPS to get that.
> Your mechanism relies on counters updated at the time packets are given
> to the NIC, not at the time packets are given to the txq (and eventually
> sit for a while in the qdisc right before BQL layer)
>
> dev_queue_xmit() is not talking to the device directly...
>
>
> All I am saying is that the producer/consumer counters you added are not
> at the right place.
>
> You tried hard to not change the drivers, by adding something to
> existing BQL. But packets can sit in a qdisc for a while...
>
But again, this patch is to prevent ooo being caused by an interaction
with XPS. It does not address ooo being caused by qdiscs or VMs, but
then I don't see that as being the issue reported by Rick.
> So you added 2 potential cache lines misses per outgoing packet, but
> with no guarantee that OOO are really avoided.
>
>
>
^ permalink raw reply
* Re: [PATCH net-next v3 2/3] udp: implement memory accounting helpers
From: Eric Dumazet @ 2016-09-29 14:13 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, David S. Miller, James Morris,
Trond Myklebust, Alexander Duyck, Daniel Borkmann, Eric Dumazet,
Tom Herbert, Hannes Frederic Sowa, Edward Cree,
linux-nfs-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1475157674.4676.52.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Thu, 2016-09-29 at 16:01 +0200, Paolo Abeni wrote:
> When we reach __sk_mem_reduce_allocated() we are sure we can free the
> specified amount of memory, so we only need to ensure consistent
> sk_prot->memory_allocated updates. The current atomic operation suffices
> to this.
Then why are you updating sk->sk_forward_alloc using racy operations ?
If this is not needed or racy, do not do it.
So that we can remove this annoying thing that a dynamic checker could
very well detect.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net-next v3 2/3] udp: implement memory accounting helpers
From: Paolo Abeni @ 2016-09-29 14:34 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, David S. Miller, James Morris, Trond Myklebust,
Alexander Duyck, Daniel Borkmann, Eric Dumazet, Tom Herbert,
Hannes Frederic Sowa, Edward Cree, linux-nfs
In-Reply-To: <1475158408.28155.166.camel@edumazet-glaptop3.roam.corp.google.com>
On Thu, 2016-09-29 at 07:13 -0700, Eric Dumazet wrote:
> On Thu, 2016-09-29 at 16:01 +0200, Paolo Abeni wrote:
>
> > When we reach __sk_mem_reduce_allocated() we are sure we can free the
> > specified amount of memory, so we only need to ensure consistent
> > sk_prot->memory_allocated updates. The current atomic operation suffices
> > to this.
>
> Then why are you updating sk->sk_forward_alloc using racy operations ?
>
> If this is not needed or racy, do not do it.
Thank you for all the feedback.
The actual forward allocated memory value is:
atomic_read(&up->mem_allocated) - atomic_read(&sk->sk_rmem_alloc).
sk_forward_alloc is updated only to hint to the user space the forward
allocated memory value via the diag interface.
If such information is not needed we can drop the update, and
sk_forward_alloc will always be seen as 0 even when the socket has some
forward allocation.
Cheers,
Paolo
^ permalink raw reply
* Re: [PATCH net-next v3 2/3] udp: implement memory accounting helpers
From: Eric Dumazet @ 2016-09-29 14:49 UTC (permalink / raw)
To: Paolo Abeni
Cc: Eric Dumazet, netdev, David S. Miller, James Morris,
Trond Myklebust, Alexander Duyck, Daniel Borkmann, Tom Herbert,
Hannes Frederic Sowa, Edward Cree, linux-nfs
In-Reply-To: <1475159680.4676.65.camel@redhat.com>
On Thu, Sep 29, 2016 at 7:34 AM, Paolo Abeni <pabeni@redhat.com> wrote:
> On Thu, 2016-09-29 at 07:13 -0700, Eric Dumazet wrote:
>> On Thu, 2016-09-29 at 16:01 +0200, Paolo Abeni wrote:
>>
>> > When we reach __sk_mem_reduce_allocated() we are sure we can free the
>> > specified amount of memory, so we only need to ensure consistent
>> > sk_prot->memory_allocated updates. The current atomic operation suffices
>> > to this.
>>
>> Then why are you updating sk->sk_forward_alloc using racy operations ?
>>
>> If this is not needed or racy, do not do it.
>
> Thank you for all the feedback.
>
> The actual forward allocated memory value is:
>
> atomic_read(&up->mem_allocated) - atomic_read(&sk->sk_rmem_alloc).
>
> sk_forward_alloc is updated only to hint to the user space the forward
> allocated memory value via the diag interface.
>
> If such information is not needed we can drop the update, and
> sk_forward_alloc will always be seen as 0 even when the socket has some
> forward allocation.
The information is needed and we want an accurate one, really.
Think about debugging on a live server, some stuck or mad sockets ;)
Please consider adding a proper accessor, able to deal with the UDP
peculiarities.
int sk_forward_alloc_get(const struct sock *sk)
{
if (sk is not UDP)
return sk->sk_forward_alloc;
return the precise amount using the private fields that UDP maintains,
}
Then use this accessor in these places :
net/ipv4/af_inet.c:155: WARN_ON(sk->sk_forward_alloc);
net/core/sock_diag.c:66: mem[SK_MEMINFO_FWD_ALLOC] =
sk->sk_forward_alloc;
net/ipv4/inet_diag.c:191: .idiag_fmem =
sk->sk_forward_alloc,
net/sched/em_meta.c:462: dst->value = sk->sk_forward_alloc;
Thanks.
^ permalink raw reply
* Re: [PATCH v2 net-next 4/5] xps_flows: XPS for packets that don't have a socket
From: Eric Dumazet @ 2016-09-29 14:51 UTC (permalink / raw)
To: Tom Herbert
Cc: David S. Miller, Linux Kernel Network Developers, Kernel Team,
Rick Jones, Alexander Duyck
In-Reply-To: <CALx6S344d16SVhO+FhXpyL09k4vSuv45F-bff-nMeqS4H=p+tw@mail.gmail.com>
On Thu, 2016-09-29 at 10:08 -0400, Tom Herbert wrote:
> It addresses the issue that Rick Jones pointed out was happening with
> XPS. When packets are sent for a flow that has no socket and XPS is
> enabled then each packet uses the XPS queue based on the running CPU.
> Since the thread sending on a flow can be rescheduled on different
> CPUs this is creating ooo packets. In this case the ooo is being
> caused by interaction with XPS.
>
Nope, your patch does not address the problem properly.
I am not sure I want to spend more time explaining the issue.
Lets talk about this in Tokyo next week.
^ permalink raw reply
* Re: [PATCH net-next v3 2/3] udp: implement memory accounting helpers
From: Paolo Abeni @ 2016-09-29 14:59 UTC (permalink / raw)
To: Eric Dumazet
Cc: Eric Dumazet, netdev, David S. Miller, James Morris,
Trond Myklebust, Alexander Duyck, Daniel Borkmann, Tom Herbert,
Hannes Frederic Sowa, Edward Cree, linux-nfs
In-Reply-To: <CANn89iKc=GEesKcB3OG5_wFBnZWiCBLCDkz3k5FgJFVDw-rOyQ@mail.gmail.com>
On Thu, 2016-09-29 at 07:49 -0700, Eric Dumazet wrote:
> On Thu, Sep 29, 2016 at 7:34 AM, Paolo Abeni <pabeni@redhat.com> wrote:
> > On Thu, 2016-09-29 at 07:13 -0700, Eric Dumazet wrote:
> >> On Thu, 2016-09-29 at 16:01 +0200, Paolo Abeni wrote:
> >>
> >> > When we reach __sk_mem_reduce_allocated() we are sure we can free the
> >> > specified amount of memory, so we only need to ensure consistent
> >> > sk_prot->memory_allocated updates. The current atomic operation suffices
> >> > to this.
> >>
> >> Then why are you updating sk->sk_forward_alloc using racy operations ?
> >>
> >> If this is not needed or racy, do not do it.
> >
> > Thank you for all the feedback.
> >
> > The actual forward allocated memory value is:
> >
> > atomic_read(&up->mem_allocated) - atomic_read(&sk->sk_rmem_alloc).
> >
> > sk_forward_alloc is updated only to hint to the user space the forward
> > allocated memory value via the diag interface.
> >
> > If such information is not needed we can drop the update, and
> > sk_forward_alloc will always be seen as 0 even when the socket has some
> > forward allocation.
>
> The information is needed and we want an accurate one, really.
>
> Think about debugging on a live server, some stuck or mad sockets ;)
>
> Please consider adding a proper accessor, able to deal with the UDP
> peculiarities.
Nice suggestion, thanks! I'll try that in v4. Perhaps is worth adding a
struct proto helper for this ?
I'm sorry, but I'll not be in Tokyo, so I'll probably produce some
traffic on netdev in the meanwhile.
Cheers,
Paolo
^ permalink raw reply
* Re: [PATCH v2 net-next 4/5] xps_flows: XPS for packets that don't have a socket
From: Eric Dumazet @ 2016-09-29 15:15 UTC (permalink / raw)
To: Tom Herbert
Cc: David S. Miller, Linux Kernel Network Developers, Kernel Team,
Rick Jones, Alexander Duyck
In-Reply-To: <1475160695.28155.168.camel@edumazet-glaptop3.roam.corp.google.com>
On Thu, 2016-09-29 at 07:51 -0700, Eric Dumazet wrote:
> On Thu, 2016-09-29 at 10:08 -0400, Tom Herbert wrote:
>
> > It addresses the issue that Rick Jones pointed out was happening with
> > XPS. When packets are sent for a flow that has no socket and XPS is
> > enabled then each packet uses the XPS queue based on the running CPU.
> > Since the thread sending on a flow can be rescheduled on different
> > CPUs this is creating ooo packets. In this case the ooo is being
> > caused by interaction with XPS.
> >
>
> Nope, your patch does not address the problem properly.
>
> I am not sure I want to spend more time explaining the issue.
>
> Lets talk about this in Tokyo next week.
>
Just as a reminder, sorry to bother you, stating some obvious facts for
both of us. We have public exchanges, so we also need to re-explain how
things work.
Queue selection on xmit happens before we hit the qdisc and its delays.
So when you access txq->dql.num_completed_ops and
txq->dql.num_enqueue_ops you can observe values that do not change for a
while.
Say a thread runs on a VM, and sends 2 packets P1, P2 on the same flow
(skb_get_hash() returns the same value for these 2 packets)
P1 is sent on behalf of CPU 1, we pickup queue txq1, and queue the
packet on its qdisc . Transmit does not happen because of some
constraints like rate limiting or scheduling constraints.
P2 is sent on behalf of CPU 2, we pickup queue txq2, notice that prior
packet chose txq1. We check txq1->dql and decide it is fine to use txq2,
since the dql params of txq1 were not changed yet.
( txq->dql.num_completed_ops == ent.queue_ptr )
Note that in RFS case, we have the guarantee that we observe 'live
queues' since they are the per cpu backlog.
So input_queue_head_incr() and input_queue_tail_incr_save() are
correctly doing the OOO prevention, because a queued packet immediately
changes the state.
So really your patch works if you have no qdisc, or a non congested
qdisc. (Think if P1 is dropped by a full pfifo or pfifo_fast : We really
want to avoid steering P2, P3, ..., PN on this full pfifo while maybe
other txq are idle). Strange attractors are back (check commit
9b462d02d6dd6 )
You could avoid (ab)using BQL with a different method, grabbing
skb->destructor for the packets that are socketless
The hash table would simply track the sum of skb->truesize to allow flow
migration. This would be self contained and not intrusive.
^ permalink raw reply
* Re: [PATCH net v2] L2TP:Adjust intf MTU,factor underlay L3,overlay L2
From: James Chapman @ 2016-09-29 15:18 UTC (permalink / raw)
To: R. Parameswaran
Cc: kleptog, netdev, davem, linux-kernel, nprachan, rshearma, dfawcus,
stephen, acme, lboccass
In-Reply-To: <alpine.DEB.2.11.1609221341500.6975@duvvury30.eng.vyatta.net>
On 22/09/16 21:52, R. Parameswaran wrote:
> From ed585bdd6d3d2b3dec58d414f514cd764d89159d Mon Sep 17 00:00:00 2001
> From: "R. Parameswaran" <rparames@brocade.com>
> Date: Thu, 22 Sep 2016 13:19:25 -0700
> Subject: [PATCH] L2TP:Adjust intf MTU,factor underlay L3,overlay L2
>
> Take into account all of the tunnel encapsulation headers when setting
> up the MTU on the L2TP logical interface device. Otherwise, packets
> created by the applications on top of the L2TP layer are larger
> than they ought to be, relative to the underlay MTU, leading to
> needless fragmentation once the outer IP encap is added.
>
> Specifically, take into account the (outer, underlay) IP header
> imposed on the encapsulated L2TP packet, and the Layer 2 header
> imposed on the inner IP packet prior to L2TP encapsulation.
>
> Do not assume an Ethernet (non-jumbo) underlay. Use the PMTU mechanism
> and the dst entry in the L2TP tunnel socket to directly pull up
> the underlay MTU (as the baseline number on top of which the
> encapsulation headers are factored in). Fall back to Ethernet MTU
> if this fails.
>
> Signed-off-by: R. Parameswaran <rparames@brocade.com>
>
> Reviewed-by: "N. Prachanda" <nprachan@brocade.com>,
> Reviewed-by: "R. Shearman" <rshearma@brocade.com>,
> Reviewed-by: "D. Fawcus" <dfawcus@brocade.com>
> ---
> net/l2tp/l2tp_eth.c | 48 ++++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 44 insertions(+), 4 deletions(-)
>
> diff --git a/net/l2tp/l2tp_eth.c b/net/l2tp/l2tp_eth.c
> index 57fc5a4..dbcd6bd 100644
> --- a/net/l2tp/l2tp_eth.c
> +++ b/net/l2tp/l2tp_eth.c
> @@ -30,6 +30,9 @@
> #include <net/xfrm.h>
> #include <net/net_namespace.h>
> #include <net/netns/generic.h>
> +#include <linux/ip.h>
> +#include <linux/ipv6.h>
> +#include <linux/udp.h>
>
> #include "l2tp_core.h"
>
> @@ -206,6 +209,46 @@ static void l2tp_eth_show(struct seq_file *m, void *arg)
> }
> #endif
>
> +static void l2tp_eth_adjust_mtu(struct l2tp_tunnel *tunnel,
> + struct l2tp_session *session,
> + struct net_device *dev)
> +{
> + unsigned int overhead = 0;
> + struct dst_entry *dst;
> +
> + if (session->mtu != 0) {
> + dev->mtu = session->mtu;
> + dev->needed_headroom += session->hdr_len;
> + if (tunnel->encap == L2TP_ENCAPTYPE_UDP)
> + dev->needed_headroom += sizeof(struct udphdr);
> + return;
> + }
> + overhead = session->hdr_len;
> + /* Adjust MTU, factor overhead - underlay L3 hdr, overlay L2 hdr*/
> + if (tunnel->sock->sk_family == AF_INET)
> + overhead += (ETH_HLEN + sizeof(struct iphdr));
> + else if (tunnel->sock->sk_family == AF_INET6)
> + overhead += (ETH_HLEN + sizeof(struct ipv6hdr));
What about options in the IP header? If certain options are set on the
socket, the IP header may be larger.
> + /* Additionally, if the encap is UDP, account for UDP header size */
> + if (tunnel->encap == L2TP_ENCAPTYPE_UDP)
> + overhead += sizeof(struct udphdr);
> + /* If PMTU discovery was enabled, use discovered MTU on L2TP device */
> + dst = sk_dst_get(tunnel->sock);
> + if (dst) {
> + u32 pmtu = dst_mtu(dst);
> +
> + if (pmtu != 0)
> + dev->mtu = pmtu;
> + dst_release(dst);
> + }
> + /* else (no PMTUD) L2TP dev MTU defaulted to Ethernet MTU in caller */
> + session->mtu = dev->mtu - overhead;
> + dev->mtu = session->mtu;
> + dev->needed_headroom += session->hdr_len;
> + if (tunnel->encap == L2TP_ENCAPTYPE_UDP)
> + dev->needed_headroom += sizeof(struct udphdr);
> +}
> +
> static int l2tp_eth_create(struct net *net, u32 tunnel_id, u32 session_id, u32 peer_session_id, struct l2tp_session_cfg *cfg)
> {
> struct net_device *dev;
> @@ -255,11 +298,8 @@ static int l2tp_eth_create(struct net *net, u32 tunnel_id, u32 session_id, u32 p
> }
>
> dev_net_set(dev, net);
> - if (session->mtu == 0)
> - session->mtu = dev->mtu - session->hdr_len;
> - dev->mtu = session->mtu;
> - dev->needed_headroom += session->hdr_len;
>
> + l2tp_eth_adjust_mtu(tunnel, session, dev);
> priv = netdev_priv(dev);
> priv->dev = dev;
> priv->session = session;
^ permalink raw reply
* Re: [PATCH net v2] L2TP:Adjust intf MTU,factor underlay L3,overlay L2
From: James Chapman @ 2016-09-29 15:39 UTC (permalink / raw)
To: R. Parameswaran
Cc: David Miller, kleptog, netdev, linux-kernel, nprachan, rshearma,
dfawcus, stephen, acme, lboccass
In-Reply-To: <alpine.DEB.2.11.1609281857050.17015@duvvury48.eng.vyatta.net>
On 29/09/16 03:36, R. Parameswaran wrote:
> I agree that something like 2. below would be needed in the long run (it
> will need some effort and redesign -e.g. how do I lookup the parent tunnel
> from the socket when receiving a PMTU update, existing pointer chain runs
> from tunnel to socket).
>> 2) Add code to handle PMTU events that land on the UDP tunnel
>> socket.
Another function pointer could be added to struct udp_sock, similar to
encap_rcv, such that the pmtu event could be handled by the UDP encap
protocol implementation.
James
^ permalink raw reply
* Re: [PATCH v2 3/3] net: make net namespace sysctls belong to container's owner
From: Dmitry Torokhov @ 2016-09-29 15:46 UTC (permalink / raw)
To: David S. Miller; +Cc: Al Viro, lkml, netdev, Eric W. Biederman
In-Reply-To: <1470864962-25056-4-git-send-email-dmitry.torokhov@gmail.com>
Hi David,
On Wed, Aug 10, 2016 at 2:36 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> If net namespace is attached to a user namespace let's make container's
> root owner of sysctls affecting said network namespace instead of global
> root.
>
> This also allows us to clean up net_ctl_permissions() because we do not
> need to fudge permissions anymore for the container's owner since it now
> owns the objects in question.
>
> Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
I was looking at linux-next today, and I noticed that, when you merged
my patch, you basically reverted the following commit:
commit d6e0d306449bcb5fa3c80e7a3edf11d45abf9ae9
Author: Tyler Hicks <tyhicks@canonical.com>
Date: Thu Jun 2 23:43:22 2016 -0500
net: Use ns_capable_noaudit() when determining net sysctl permissions
The capability check should not be audited since it is only being used
to determine the inode permissions. A failed check does not indicate a
violation of security policy but, when an LSM is enabled, a denial audit
message was being generated.
The denial audit message caused confusion for some application authors
because root-running Go applications always triggered the denial. To
prevent this confusion, the capability check in net_ctl_permissions() is
switched to the noaudit variant.
BugLink: https://launchpad.net/bugs/1465724
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Serge E. Hallyn <serge.hallyn@ubuntu.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
Thanks!
> ---
> fs/proc/proc_sysctl.c | 5 +++++
> include/linux/sysctl.h | 4 ++++
> net/sysctl_net.c | 29 ++++++++++++++++++++---------
> 3 files changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
> index 5e57c3e..28f9085 100644
> --- a/fs/proc/proc_sysctl.c
> +++ b/fs/proc/proc_sysctl.c
> @@ -430,6 +430,7 @@ static int sysctl_perm(struct ctl_table_header *head, struct ctl_table *table, i
> static struct inode *proc_sys_make_inode(struct super_block *sb,
> struct ctl_table_header *head, struct ctl_table *table)
> {
> + struct ctl_table_root *root = head->root;
> struct inode *inode;
> struct proc_inode *ei;
>
> @@ -457,6 +458,10 @@ static struct inode *proc_sys_make_inode(struct super_block *sb,
> if (is_empty_dir(head))
> make_empty_dir_inode(inode);
> }
> +
> + if (root->set_ownership)
> + root->set_ownership(head, table, &inode->i_uid, &inode->i_gid);
> +
> out:
> return inode;
> }
> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> index fa7bc29..55bec2f 100644
> --- a/include/linux/sysctl.h
> +++ b/include/linux/sysctl.h
> @@ -25,6 +25,7 @@
> #include <linux/rcupdate.h>
> #include <linux/wait.h>
> #include <linux/rbtree.h>
> +#include <linux/uidgid.h>
> #include <uapi/linux/sysctl.h>
>
> /* For the /proc/sys support */
> @@ -156,6 +157,9 @@ struct ctl_table_root {
> struct ctl_table_set default_set;
> struct ctl_table_set *(*lookup)(struct ctl_table_root *root,
> struct nsproxy *namespaces);
> + void (*set_ownership)(struct ctl_table_header *head,
> + struct ctl_table *table,
> + kuid_t *uid, kgid_t *gid);
> int (*permissions)(struct ctl_table_header *head, struct ctl_table *table);
> };
>
> diff --git a/net/sysctl_net.c b/net/sysctl_net.c
> index ed98c1f..5bc1a3d 100644
> --- a/net/sysctl_net.c
> +++ b/net/sysctl_net.c
> @@ -42,26 +42,37 @@ static int net_ctl_permissions(struct ctl_table_header *head,
> struct ctl_table *table)
> {
> struct net *net = container_of(head->set, struct net, sysctls);
> - kuid_t root_uid = make_kuid(net->user_ns, 0);
> - kgid_t root_gid = make_kgid(net->user_ns, 0);
>
> /* Allow network administrator to have same access as root. */
> - if (ns_capable(net->user_ns, CAP_NET_ADMIN) ||
> - uid_eq(root_uid, current_euid())) {
> + if (ns_capable(net->user_ns, CAP_NET_ADMIN)) {
> int mode = (table->mode >> 6) & 7;
> return (mode << 6) | (mode << 3) | mode;
> }
> - /* Allow netns root group to have the same access as the root group */
> - if (in_egroup_p(root_gid)) {
> - int mode = (table->mode >> 3) & 7;
> - return (mode << 3) | mode;
> - }
> +
> return table->mode;
> }
>
> +static void net_ctl_set_ownership(struct ctl_table_header *head,
> + struct ctl_table *table,
> + kuid_t *uid, kgid_t *gid)
> +{
> + struct net *net = container_of(head->set, struct net, sysctls);
> + kuid_t ns_root_uid;
> + kgid_t ns_root_gid;
> +
> + ns_root_uid = make_kuid(net->user_ns, 0);
> + if (uid_valid(ns_root_uid))
> + *uid = ns_root_uid;
> +
> + ns_root_gid = make_kgid(net->user_ns, 0);
> + if (gid_valid(ns_root_gid))
> + *gid = ns_root_gid;
> +}
> +
> static struct ctl_table_root net_sysctl_root = {
> .lookup = net_ctl_header_lookup,
> .permissions = net_ctl_permissions,
> + .set_ownership = net_ctl_set_ownership,
> };
>
> static int __net_init sysctl_net_init(struct net *net)
> --
> 2.8.0.rc3.226.g39d4020
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH RFC net-next] bnx2x: avoid printing unnecessary messages during register dump
From: Guilherme G. Piccoli @ 2016-09-29 16:19 UTC (permalink / raw)
To: David Miller, ariel.elior, Yuval.Mintz; +Cc: netdev
In-Reply-To: <20160927.224349.354006077948089224.davem@davemloft.net>
On 09/27/2016 11:43 PM, David Miller wrote:
> From: "Guilherme G. Piccoli" <gpiccoli@linux.vnet.ibm.com>
> Date: Tue, 27 Sep 2016 15:33:54 -0300
>
>> The bnx2x driver prints multiple error messages during register dump,
>> with "ethtool -d" for example. The driver even warn that many messages
>> might be seen during the register dump, but they are harmless. A typical
>> kernel log after register dump looks like this:
>>
>> [9.375] bnx2x: [bnx2x_get_regs:987(net0)]Generating register dump. Might trigger harmless GRC timeouts
>> [9.439] bnx2x: [bnx2x_attn_int_deasserted3:4342(net0)]LATCHED attention 0x04000000 (masked)
>> [9.439] bnx2x: [bnx2x_attn_int_deasserted3:4346(net0)]GRC time-out 0x010580cd
>> [...]
>>
>> The notation [...] means that some messages were supressed - in our
>> tests we saw 78 more "LATCHED attention" and "GRC time-out" messages,
>> supressed here.
>>
>> This patch avoid these messages to be printed on register dump instead
>> of just warn they are harmless.
>>
>> Signed-off-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>
>
> Although "ethtool -d" is really a debugging facility, I still think that
> serious care should be placed into arranging what gets dumped in such
> a way that such access timeouts and errors are minimized.
>
David, thanks for your comment. I confess I didn't understand your
statement quite well. You say we shouldn't dump registers that will
cause timeouts, that's it?
If yes, I guess this is a valid point. We will however loose some debug
information (as you mentioned, 'ethtool -d' is a debug facility). Now,
since I'm no expert in QLogic adapter hw/fw, I want to ask Yuval/Ariel
why those timeouts are hit anyway. Are they completely harmless?
In my understanding/opinion, hiding the messages entirely (as this patch
does) OR avoid the timeouts by disabling some registers' dump are both
better alternatives than the current behavior of the driver.
Thanks,
Guilherme
^ permalink raw reply
* [PATCH net-next 00/10] net: dsa: mv88e6xxx: Global (1) cosmetics
From: Vivien Didelot @ 2016-09-29 16:21 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
The Global (1) internal SMI device of Marvell switches is a set of
registers providing support to different units for MAC addresses (ATU),
VLANs (VTU), PHY polling (PPU), etc.
Chips (like 88E6060) may use a different address for it, or have
subtleties in the units (e.g. different number of databases, changing
how registers must be accessed), making it hard to maintain properly.
This patchset is a first step to polish the Global (1) support, with no
functional changes though. Here's basically what it does:
- add helpers to access Global1 registers (same for Global2)
- remove a few family checks (VTU/STU FID registers)
- s/mv88e6xxx_vtu_stu_entry/mv88e6xxx_vtu_entry/
- add a per-chip mv88e6xxx_ops structure of function pointers:
struct mv88e6xxx_ops {
int (*get_eeprom)(struct mv88e6xxx_chip *chip,
struct ethtool_eeprom *eeprom, u8 *data);
int (*set_eeprom)(struct mv88e6xxx_chip *chip,
struct ethtool_eeprom *eeprom, u8 *data);
int (*set_switch_mac)(struct mv88e6xxx_chip *chip, u8 *addr);
int (*phy_read)(struct mv88e6xxx_chip *chip, int addr, int reg,
u16 *val);
int (*phy_write)(struct mv88e6xxx_chip *chip, int addr, int reg,
u16 val);
};
Future patchsets will add ATU/VTU ops, software reset, etc.
Vivien Didelot (10):
net: dsa: mv88e6xxx: add global1 helpers
net: dsa: mv88e6xxx: abstract REG_GLOBAL2
net: dsa: mv88e6xxx: add flags for FID registers
net: dsa: mv88e6xxx: expose mv88e6xxx_num_databases
net: dsa: mv88e6xxx: add mv88e6xxx_num_ports helper
net: dsa: mv88e6xxx: rename mv88e6xxx_vtu_stu_entry
net: dsa: mv88e6xxx: rename mv88e6xxx_ops
net: dsa: mv88e6xxx: add chip-wide ops
net: dsa: mv88e6xxx: add set_switch_mac to ops
net: dsa: mv88e6xxx: add eeprom ops
drivers/net/dsa/mv88e6xxx/Makefile | 1 +
drivers/net/dsa/mv88e6xxx/chip.c | 804 +++++++++++++++++++---------------
drivers/net/dsa/mv88e6xxx/global1.c | 34 ++
drivers/net/dsa/mv88e6xxx/global1.h | 23 +
drivers/net/dsa/mv88e6xxx/global2.c | 86 ++--
drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 102 +++--
6 files changed, 607 insertions(+), 443 deletions(-)
create mode 100644 drivers/net/dsa/mv88e6xxx/global1.c
create mode 100644 drivers/net/dsa/mv88e6xxx/global1.h
--
2.10.0
^ permalink raw reply
* [PATCH net-next 01/10] net: dsa: mv88e6xxx: add global1 helpers
From: Vivien Didelot @ 2016-09-29 16:21 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
In-Reply-To: <20160929162202.10685-1-vivien.didelot@savoirfairelinux.com>
The Global (1) internal SMI device is an extended set of registers
containing ATU, PPU, VTU, STU, etc.
It is present on every switches, usually at SMI address 0x1B. But old
models such as 88E6060 access it at address 0xF, thus using REG_GLOBAL
is erroneous.
Add a global1_addr info member used by mv88e6xxx_g1_{read,write} and
mv88e6xxx_g1_wait helpers in a new global1.c file.
This patch finally removes _mv88e6xxx_reg_{read,write}, in favor on the
appropriate helpers. No functional changes here.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
drivers/net/dsa/mv88e6xxx/Makefile | 1 +
drivers/net/dsa/mv88e6xxx/chip.c | 505 +++++++++++++++++-----------------
drivers/net/dsa/mv88e6xxx/global1.c | 34 +++
drivers/net/dsa/mv88e6xxx/global1.h | 23 ++
drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 2 +-
5 files changed, 306 insertions(+), 259 deletions(-)
create mode 100644 drivers/net/dsa/mv88e6xxx/global1.c
create mode 100644 drivers/net/dsa/mv88e6xxx/global1.h
diff --git a/drivers/net/dsa/mv88e6xxx/Makefile b/drivers/net/dsa/mv88e6xxx/Makefile
index 6971039..10ce820 100644
--- a/drivers/net/dsa/mv88e6xxx/Makefile
+++ b/drivers/net/dsa/mv88e6xxx/Makefile
@@ -1,3 +1,4 @@
obj-$(CONFIG_NET_DSA_MV88E6XXX) += mv88e6xxx.o
mv88e6xxx-objs := chip.o
+mv88e6xxx-objs += global1.o
mv88e6xxx-$(CONFIG_NET_DSA_MV88E6XXX_GLOBAL2) += global2.o
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index b2c25da..98dee2c 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -31,6 +31,7 @@
#include <net/switchdev.h>
#include "mv88e6xxx.h"
+#include "global1.h"
#include "global2.h"
static void assert_reg_lock(struct mv88e6xxx_chip *chip)
@@ -361,46 +362,27 @@ int mv88e6xxx_update(struct mv88e6xxx_chip *chip, int addr, int reg, u16 update)
return mv88e6xxx_write(chip, addr, reg, val);
}
-static int _mv88e6xxx_reg_read(struct mv88e6xxx_chip *chip, int addr, int reg)
+static int mv88e6xxx_ppu_disable(struct mv88e6xxx_chip *chip)
{
u16 val;
- int err;
+ int i, err;
- err = mv88e6xxx_read(chip, addr, reg, &val);
+ err = mv88e6xxx_g1_read(chip, GLOBAL_CONTROL, &val);
if (err)
return err;
- return val;
-}
-
-static int _mv88e6xxx_reg_write(struct mv88e6xxx_chip *chip, int addr,
- int reg, u16 val)
-{
- return mv88e6xxx_write(chip, addr, reg, val);
-}
-
-static int mv88e6xxx_ppu_disable(struct mv88e6xxx_chip *chip)
-{
- int ret;
- int i;
-
- ret = _mv88e6xxx_reg_read(chip, REG_GLOBAL, GLOBAL_CONTROL);
- if (ret < 0)
- return ret;
-
- ret = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_CONTROL,
- ret & ~GLOBAL_CONTROL_PPU_ENABLE);
- if (ret)
- return ret;
+ err = mv88e6xxx_g1_write(chip, GLOBAL_CONTROL,
+ val & ~GLOBAL_CONTROL_PPU_ENABLE);
+ if (err)
+ return err;
for (i = 0; i < 16; i++) {
- ret = _mv88e6xxx_reg_read(chip, REG_GLOBAL, GLOBAL_STATUS);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_g1_read(chip, GLOBAL_STATUS, &val);
+ if (err)
+ return err;
usleep_range(1000, 2000);
- if ((ret & GLOBAL_STATUS_PPU_MASK) !=
- GLOBAL_STATUS_PPU_POLLING)
+ if ((val & GLOBAL_STATUS_PPU_MASK) != GLOBAL_STATUS_PPU_POLLING)
return 0;
}
@@ -409,25 +391,25 @@ static int mv88e6xxx_ppu_disable(struct mv88e6xxx_chip *chip)
static int mv88e6xxx_ppu_enable(struct mv88e6xxx_chip *chip)
{
- int ret, err, i;
+ u16 val;
+ int i, err;
- ret = _mv88e6xxx_reg_read(chip, REG_GLOBAL, GLOBAL_CONTROL);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_g1_read(chip, GLOBAL_CONTROL, &val);
+ if (err)
+ return err;
- err = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_CONTROL,
- ret | GLOBAL_CONTROL_PPU_ENABLE);
+ err = mv88e6xxx_g1_write(chip, GLOBAL_CONTROL,
+ val | GLOBAL_CONTROL_PPU_ENABLE);
if (err)
return err;
for (i = 0; i < 16; i++) {
- ret = _mv88e6xxx_reg_read(chip, REG_GLOBAL, GLOBAL_STATUS);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_g1_read(chip, GLOBAL_STATUS, &val);
+ if (err)
+ return err;
usleep_range(1000, 2000);
- if ((ret & GLOBAL_STATUS_PPU_MASK) ==
- GLOBAL_STATUS_PPU_POLLING)
+ if ((val & GLOBAL_STATUS_PPU_MASK) == GLOBAL_STATUS_PPU_POLLING)
return 0;
}
@@ -663,12 +645,12 @@ out:
static int _mv88e6xxx_stats_wait(struct mv88e6xxx_chip *chip)
{
- int ret;
- int i;
+ u16 val;
+ int i, err;
for (i = 0; i < 10; i++) {
- ret = _mv88e6xxx_reg_read(chip, REG_GLOBAL, GLOBAL_STATS_OP);
- if ((ret & GLOBAL_STATS_OP_BUSY) == 0)
+ err = mv88e6xxx_g1_read(chip, GLOBAL_STATS_OP, &val);
+ if ((val & GLOBAL_STATS_OP_BUSY) == 0)
return 0;
}
@@ -677,55 +659,52 @@ static int _mv88e6xxx_stats_wait(struct mv88e6xxx_chip *chip)
static int _mv88e6xxx_stats_snapshot(struct mv88e6xxx_chip *chip, int port)
{
- int ret;
+ int err;
if (mv88e6xxx_6320_family(chip) || mv88e6xxx_6352_family(chip))
port = (port + 1) << 5;
/* Snapshot the hardware statistics counters for this port. */
- ret = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_STATS_OP,
- GLOBAL_STATS_OP_CAPTURE_PORT |
- GLOBAL_STATS_OP_HIST_RX_TX | port);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_g1_write(chip, GLOBAL_STATS_OP,
+ GLOBAL_STATS_OP_CAPTURE_PORT |
+ GLOBAL_STATS_OP_HIST_RX_TX | port);
+ if (err)
+ return err;
/* Wait for the snapshotting to complete. */
- ret = _mv88e6xxx_stats_wait(chip);
- if (ret < 0)
- return ret;
-
- return 0;
+ return _mv88e6xxx_stats_wait(chip);
}
static void _mv88e6xxx_stats_read(struct mv88e6xxx_chip *chip,
int stat, u32 *val)
{
- u32 _val;
- int ret;
+ u32 value;
+ u16 reg;
+ int err;
*val = 0;
- ret = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_STATS_OP,
- GLOBAL_STATS_OP_READ_CAPTURED |
- GLOBAL_STATS_OP_HIST_RX_TX | stat);
- if (ret < 0)
+ err = mv88e6xxx_g1_write(chip, GLOBAL_STATS_OP,
+ GLOBAL_STATS_OP_READ_CAPTURED |
+ GLOBAL_STATS_OP_HIST_RX_TX | stat);
+ if (err)
return;
- ret = _mv88e6xxx_stats_wait(chip);
- if (ret < 0)
+ err = _mv88e6xxx_stats_wait(chip);
+ if (err)
return;
- ret = _mv88e6xxx_reg_read(chip, REG_GLOBAL, GLOBAL_STATS_COUNTER_32);
- if (ret < 0)
+ err = mv88e6xxx_g1_read(chip, GLOBAL_STATS_COUNTER_32, ®);
+ if (err)
return;
- _val = ret << 16;
+ value = reg << 16;
- ret = _mv88e6xxx_reg_read(chip, REG_GLOBAL, GLOBAL_STATS_COUNTER_01);
- if (ret < 0)
+ err = mv88e6xxx_g1_read(chip, GLOBAL_STATS_COUNTER_01, ®);
+ if (err)
return;
- *val = _val | ret;
+ *val = value | reg;
}
static struct mv88e6xxx_hw_stat mv88e6xxx_hw_stats[] = {
@@ -932,8 +911,7 @@ static void mv88e6xxx_get_regs(struct dsa_switch *ds, int port,
static int _mv88e6xxx_atu_wait(struct mv88e6xxx_chip *chip)
{
- return mv88e6xxx_wait(chip, REG_GLOBAL, GLOBAL_ATU_OP,
- GLOBAL_ATU_OP_BUSY);
+ return mv88e6xxx_g1_wait(chip, GLOBAL_ATU_OP, GLOBAL_ATU_OP_BUSY);
}
static int mv88e6xxx_get_eee(struct dsa_switch *ds, int port,
@@ -997,32 +975,31 @@ out:
static int _mv88e6xxx_atu_cmd(struct mv88e6xxx_chip *chip, u16 fid, u16 cmd)
{
- int ret;
+ u16 val;
+ int err;
if (mv88e6xxx_has_fid_reg(chip)) {
- ret = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_ATU_FID,
- fid);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_g1_write(chip, GLOBAL_ATU_FID, fid);
+ if (err)
+ return err;
} else if (mv88e6xxx_num_databases(chip) == 256) {
/* ATU DBNum[7:4] are located in ATU Control 15:12 */
- ret = _mv88e6xxx_reg_read(chip, REG_GLOBAL, GLOBAL_ATU_CONTROL);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_g1_read(chip, GLOBAL_ATU_CONTROL, &val);
+ if (err)
+ return err;
- ret = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_ATU_CONTROL,
- (ret & 0xfff) |
- ((fid << 8) & 0xf000));
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_g1_write(chip, GLOBAL_ATU_CONTROL,
+ (val & 0xfff) | ((fid << 8) & 0xf000));
+ if (err)
+ return err;
/* ATU DBNum[3:0] are located in ATU Operation 3:0 */
cmd |= fid & 0xf;
}
- ret = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_ATU_OP, cmd);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_g1_write(chip, GLOBAL_ATU_OP, cmd);
+ if (err)
+ return err;
return _mv88e6xxx_atu_wait(chip);
}
@@ -1047,7 +1024,7 @@ static int _mv88e6xxx_atu_data_write(struct mv88e6xxx_chip *chip,
data |= (entry->portv_trunkid << shift) & mask;
}
- return _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_ATU_DATA, data);
+ return mv88e6xxx_g1_write(chip, GLOBAL_ATU_DATA, data);
}
static int _mv88e6xxx_atu_flush_move(struct mv88e6xxx_chip *chip,
@@ -1277,17 +1254,16 @@ static int _mv88e6xxx_port_pvid_set(struct mv88e6xxx_chip *chip,
static int _mv88e6xxx_vtu_wait(struct mv88e6xxx_chip *chip)
{
- return mv88e6xxx_wait(chip, REG_GLOBAL, GLOBAL_VTU_OP,
- GLOBAL_VTU_OP_BUSY);
+ return mv88e6xxx_g1_wait(chip, GLOBAL_VTU_OP, GLOBAL_VTU_OP_BUSY);
}
static int _mv88e6xxx_vtu_cmd(struct mv88e6xxx_chip *chip, u16 op)
{
- int ret;
+ int err;
- ret = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_VTU_OP, op);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_g1_write(chip, GLOBAL_VTU_OP, op);
+ if (err)
+ return err;
return _mv88e6xxx_vtu_wait(chip);
}
@@ -1308,16 +1284,14 @@ static int _mv88e6xxx_vtu_stu_data_read(struct mv88e6xxx_chip *chip,
unsigned int nibble_offset)
{
u16 regs[3];
- int i;
- int ret;
+ int i, err;
for (i = 0; i < 3; ++i) {
- ret = _mv88e6xxx_reg_read(chip, REG_GLOBAL,
- GLOBAL_VTU_DATA_0_3 + i);
- if (ret < 0)
- return ret;
+ u16 *reg = ®s[i];
- regs[i] = ret;
+ err = mv88e6xxx_g1_read(chip, GLOBAL_VTU_DATA_0_3 + i, reg);
+ if (err)
+ return err;
}
for (i = 0; i < chip->info->num_ports; ++i) {
@@ -1347,8 +1321,7 @@ static int _mv88e6xxx_vtu_stu_data_write(struct mv88e6xxx_chip *chip,
unsigned int nibble_offset)
{
u16 regs[3] = { 0 };
- int i;
- int ret;
+ int i, err;
for (i = 0; i < chip->info->num_ports; ++i) {
unsigned int shift = (i % 4) * 4 + nibble_offset;
@@ -1358,10 +1331,11 @@ static int _mv88e6xxx_vtu_stu_data_write(struct mv88e6xxx_chip *chip,
}
for (i = 0; i < 3; ++i) {
- ret = _mv88e6xxx_reg_write(chip, REG_GLOBAL,
- GLOBAL_VTU_DATA_0_3 + i, regs[i]);
- if (ret < 0)
- return ret;
+ u16 reg = regs[i];
+
+ err = mv88e6xxx_g1_write(chip, GLOBAL_VTU_DATA_0_3 + i, reg);
+ if (err)
+ return err;
}
return 0;
@@ -1381,63 +1355,61 @@ static int mv88e6xxx_stu_data_write(struct mv88e6xxx_chip *chip,
static int _mv88e6xxx_vtu_vid_write(struct mv88e6xxx_chip *chip, u16 vid)
{
- return _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_VTU_VID,
- vid & GLOBAL_VTU_VID_MASK);
+ return mv88e6xxx_g1_write(chip, GLOBAL_VTU_VID,
+ vid & GLOBAL_VTU_VID_MASK);
}
static int _mv88e6xxx_vtu_getnext(struct mv88e6xxx_chip *chip,
struct mv88e6xxx_vtu_stu_entry *entry)
{
struct mv88e6xxx_vtu_stu_entry next = { 0 };
- int ret;
+ u16 val;
+ int err;
- ret = _mv88e6xxx_vtu_wait(chip);
- if (ret < 0)
- return ret;
+ err = _mv88e6xxx_vtu_wait(chip);
+ if (err)
+ return err;
- ret = _mv88e6xxx_vtu_cmd(chip, GLOBAL_VTU_OP_VTU_GET_NEXT);
- if (ret < 0)
- return ret;
+ err = _mv88e6xxx_vtu_cmd(chip, GLOBAL_VTU_OP_VTU_GET_NEXT);
+ if (err)
+ return err;
- ret = _mv88e6xxx_reg_read(chip, REG_GLOBAL, GLOBAL_VTU_VID);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_g1_read(chip, GLOBAL_VTU_VID, &val);
+ if (err)
+ return err;
- next.vid = ret & GLOBAL_VTU_VID_MASK;
- next.valid = !!(ret & GLOBAL_VTU_VID_VALID);
+ next.vid = val & GLOBAL_VTU_VID_MASK;
+ next.valid = !!(val & GLOBAL_VTU_VID_VALID);
if (next.valid) {
- ret = mv88e6xxx_vtu_data_read(chip, &next);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_vtu_data_read(chip, &next);
+ if (err)
+ return err;
if (mv88e6xxx_has_fid_reg(chip)) {
- ret = _mv88e6xxx_reg_read(chip, REG_GLOBAL,
- GLOBAL_VTU_FID);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_g1_read(chip, GLOBAL_VTU_FID, &val);
+ if (err)
+ return err;
- next.fid = ret & GLOBAL_VTU_FID_MASK;
+ next.fid = val & GLOBAL_VTU_FID_MASK;
} else if (mv88e6xxx_num_databases(chip) == 256) {
/* VTU DBNum[7:4] are located in VTU Operation 11:8, and
* VTU DBNum[3:0] are located in VTU Operation 3:0
*/
- ret = _mv88e6xxx_reg_read(chip, REG_GLOBAL,
- GLOBAL_VTU_OP);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_g1_read(chip, GLOBAL_VTU_OP, &val);
+ if (err)
+ return err;
- next.fid = (ret & 0xf00) >> 4;
- next.fid |= ret & 0xf;
+ next.fid = (val & 0xf00) >> 4;
+ next.fid |= val & 0xf;
}
if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_STU)) {
- ret = _mv88e6xxx_reg_read(chip, REG_GLOBAL,
- GLOBAL_VTU_SID);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_g1_read(chip, GLOBAL_VTU_SID, &val);
+ if (err)
+ return err;
- next.sid = ret & GLOBAL_VTU_SID_MASK;
+ next.sid = val & GLOBAL_VTU_SID_MASK;
}
}
@@ -1505,34 +1477,32 @@ static int _mv88e6xxx_vtu_loadpurge(struct mv88e6xxx_chip *chip,
{
u16 op = GLOBAL_VTU_OP_VTU_LOAD_PURGE;
u16 reg = 0;
- int ret;
+ int err;
- ret = _mv88e6xxx_vtu_wait(chip);
- if (ret < 0)
- return ret;
+ err = _mv88e6xxx_vtu_wait(chip);
+ if (err)
+ return err;
if (!entry->valid)
goto loadpurge;
/* Write port member tags */
- ret = mv88e6xxx_vtu_data_write(chip, entry);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_vtu_data_write(chip, entry);
+ if (err)
+ return err;
if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_STU)) {
reg = entry->sid & GLOBAL_VTU_SID_MASK;
- ret = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_VTU_SID,
- reg);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_g1_write(chip, GLOBAL_VTU_SID, reg);
+ if (err)
+ return err;
}
if (mv88e6xxx_has_fid_reg(chip)) {
reg = entry->fid & GLOBAL_VTU_FID_MASK;
- ret = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_VTU_FID,
- reg);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_g1_write(chip, GLOBAL_VTU_FID, reg);
+ if (err)
+ return err;
} else if (mv88e6xxx_num_databases(chip) == 256) {
/* VTU DBNum[7:4] are located in VTU Operation 11:8, and
* VTU DBNum[3:0] are located in VTU Operation 3:0
@@ -1544,9 +1514,9 @@ static int _mv88e6xxx_vtu_loadpurge(struct mv88e6xxx_chip *chip,
reg = GLOBAL_VTU_VID_VALID;
loadpurge:
reg |= entry->vid & GLOBAL_VTU_VID_MASK;
- ret = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_VTU_VID, reg);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_g1_write(chip, GLOBAL_VTU_VID, reg);
+ if (err)
+ return err;
return _mv88e6xxx_vtu_cmd(chip, op);
}
@@ -1555,37 +1525,38 @@ static int _mv88e6xxx_stu_getnext(struct mv88e6xxx_chip *chip, u8 sid,
struct mv88e6xxx_vtu_stu_entry *entry)
{
struct mv88e6xxx_vtu_stu_entry next = { 0 };
- int ret;
+ u16 val;
+ int err;
- ret = _mv88e6xxx_vtu_wait(chip);
- if (ret < 0)
- return ret;
+ err = _mv88e6xxx_vtu_wait(chip);
+ if (err)
+ return err;
- ret = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_VTU_SID,
- sid & GLOBAL_VTU_SID_MASK);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_g1_write(chip, GLOBAL_VTU_SID,
+ sid & GLOBAL_VTU_SID_MASK);
+ if (err)
+ return err;
- ret = _mv88e6xxx_vtu_cmd(chip, GLOBAL_VTU_OP_STU_GET_NEXT);
- if (ret < 0)
- return ret;
+ err = _mv88e6xxx_vtu_cmd(chip, GLOBAL_VTU_OP_STU_GET_NEXT);
+ if (err)
+ return err;
- ret = _mv88e6xxx_reg_read(chip, REG_GLOBAL, GLOBAL_VTU_SID);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_g1_read(chip, GLOBAL_VTU_SID, &val);
+ if (err)
+ return err;
- next.sid = ret & GLOBAL_VTU_SID_MASK;
+ next.sid = val & GLOBAL_VTU_SID_MASK;
- ret = _mv88e6xxx_reg_read(chip, REG_GLOBAL, GLOBAL_VTU_VID);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_g1_read(chip, GLOBAL_VTU_VID, &val);
+ if (err)
+ return err;
- next.valid = !!(ret & GLOBAL_VTU_VID_VALID);
+ next.valid = !!(val & GLOBAL_VTU_VID_VALID);
if (next.valid) {
- ret = mv88e6xxx_stu_data_read(chip, &next);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_stu_data_read(chip, &next);
+ if (err)
+ return err;
}
*entry = next;
@@ -1596,30 +1567,30 @@ static int _mv88e6xxx_stu_loadpurge(struct mv88e6xxx_chip *chip,
struct mv88e6xxx_vtu_stu_entry *entry)
{
u16 reg = 0;
- int ret;
+ int err;
- ret = _mv88e6xxx_vtu_wait(chip);
- if (ret < 0)
- return ret;
+ err = _mv88e6xxx_vtu_wait(chip);
+ if (err)
+ return err;
if (!entry->valid)
goto loadpurge;
/* Write port states */
- ret = mv88e6xxx_stu_data_write(chip, entry);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_stu_data_write(chip, entry);
+ if (err)
+ return err;
reg = GLOBAL_VTU_VID_VALID;
loadpurge:
- ret = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_VTU_VID, reg);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_g1_write(chip, GLOBAL_VTU_VID, reg);
+ if (err)
+ return err;
reg = entry->sid & GLOBAL_VTU_SID_MASK;
- ret = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_VTU_SID, reg);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_g1_write(chip, GLOBAL_VTU_SID, reg);
+ if (err)
+ return err;
return _mv88e6xxx_vtu_cmd(chip, GLOBAL_VTU_OP_STU_LOAD_PURGE);
}
@@ -2057,14 +2028,13 @@ unlock:
static int _mv88e6xxx_atu_mac_write(struct mv88e6xxx_chip *chip,
const unsigned char *addr)
{
- int i, ret;
+ int i, err;
for (i = 0; i < 3; i++) {
- ret = _mv88e6xxx_reg_write(
- chip, REG_GLOBAL, GLOBAL_ATU_MAC_01 + i,
- (addr[i * 2] << 8) | addr[i * 2 + 1]);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_g1_write(chip, GLOBAL_ATU_MAC_01 + i,
+ (addr[i * 2] << 8) | addr[i * 2 + 1]);
+ if (err)
+ return err;
}
return 0;
@@ -2073,15 +2043,16 @@ static int _mv88e6xxx_atu_mac_write(struct mv88e6xxx_chip *chip,
static int _mv88e6xxx_atu_mac_read(struct mv88e6xxx_chip *chip,
unsigned char *addr)
{
- int i, ret;
+ u16 val;
+ int i, err;
for (i = 0; i < 3; i++) {
- ret = _mv88e6xxx_reg_read(chip, REG_GLOBAL,
- GLOBAL_ATU_MAC_01 + i);
- if (ret < 0)
- return ret;
- addr[i * 2] = ret >> 8;
- addr[i * 2 + 1] = ret & 0xff;
+ err = mv88e6xxx_g1_read(chip, GLOBAL_ATU_MAC_01 + i, &val);
+ if (err)
+ return err;
+
+ addr[i * 2] = val >> 8;
+ addr[i * 2 + 1] = val & 0xff;
}
return 0;
@@ -2217,31 +2188,32 @@ static int _mv88e6xxx_atu_getnext(struct mv88e6xxx_chip *chip, u16 fid,
struct mv88e6xxx_atu_entry *entry)
{
struct mv88e6xxx_atu_entry next = { 0 };
- int ret;
+ u16 val;
+ int err;
next.fid = fid;
- ret = _mv88e6xxx_atu_wait(chip);
- if (ret < 0)
- return ret;
+ err = _mv88e6xxx_atu_wait(chip);
+ if (err)
+ return err;
- ret = _mv88e6xxx_atu_cmd(chip, fid, GLOBAL_ATU_OP_GET_NEXT_DB);
- if (ret < 0)
- return ret;
+ err = _mv88e6xxx_atu_cmd(chip, fid, GLOBAL_ATU_OP_GET_NEXT_DB);
+ if (err)
+ return err;
- ret = _mv88e6xxx_atu_mac_read(chip, next.mac);
- if (ret < 0)
- return ret;
+ err = _mv88e6xxx_atu_mac_read(chip, next.mac);
+ if (err)
+ return err;
- ret = _mv88e6xxx_reg_read(chip, REG_GLOBAL, GLOBAL_ATU_DATA);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_g1_read(chip, GLOBAL_ATU_DATA, &val);
+ if (err)
+ return err;
- next.state = ret & GLOBAL_ATU_DATA_STATE_MASK;
+ next.state = val & GLOBAL_ATU_DATA_STATE_MASK;
if (next.state != GLOBAL_ATU_DATA_STATE_UNUSED) {
unsigned int mask, shift;
- if (ret & GLOBAL_ATU_DATA_TRUNK) {
+ if (val & GLOBAL_ATU_DATA_TRUNK) {
next.trunk = true;
mask = GLOBAL_ATU_DATA_TRUNK_ID_MASK;
shift = GLOBAL_ATU_DATA_TRUNK_ID_SHIFT;
@@ -2251,7 +2223,7 @@ static int _mv88e6xxx_atu_getnext(struct mv88e6xxx_chip *chip, u16 fid,
shift = GLOBAL_ATU_DATA_PORT_VECTOR_SHIFT;
}
- next.portv_trunkid = (ret & mask) >> shift;
+ next.portv_trunkid = (val & mask) >> shift;
}
*entry = next;
@@ -2422,8 +2394,8 @@ static int mv88e6xxx_switch_reset(struct mv88e6xxx_chip *chip)
u16 is_reset = (ppu_active ? 0x8800 : 0xc800);
struct gpio_desc *gpiod = chip->reset;
unsigned long timeout;
- int err, ret;
u16 reg;
+ int err;
int i;
/* Set all ports to the disabled state. */
@@ -2454,20 +2426,20 @@ static int mv88e6xxx_switch_reset(struct mv88e6xxx_chip *chip)
* through global registers 0x18 and 0x19.
*/
if (ppu_active)
- err = _mv88e6xxx_reg_write(chip, REG_GLOBAL, 0x04, 0xc000);
+ err = mv88e6xxx_g1_write(chip, 0x04, 0xc000);
else
- err = _mv88e6xxx_reg_write(chip, REG_GLOBAL, 0x04, 0xc400);
+ err = mv88e6xxx_g1_write(chip, 0x04, 0xc400);
if (err)
return err;
/* Wait up to one second for reset to complete. */
timeout = jiffies + 1 * HZ;
while (time_before(jiffies, timeout)) {
- ret = _mv88e6xxx_reg_read(chip, REG_GLOBAL, 0x00);
- if (ret < 0)
- return ret;
+ err = mv88e6xxx_g1_read(chip, 0x00, ®);
+ if (err)
+ return err;
- if ((ret & is_reset) == is_reset)
+ if ((reg & is_reset) == is_reset)
break;
usleep_range(1000, 2000);
}
@@ -2749,22 +2721,23 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
return mv88e6xxx_port_write(chip, port, PORT_DEFAULT_VLAN, 0x0000);
}
-static int mv88e6xxx_g1_set_switch_mac(struct mv88e6xxx_chip *chip, u8 *addr)
+int mv88e6xxx_g1_set_switch_mac(struct mv88e6xxx_chip *chip, u8 *addr)
{
int err;
- err = mv88e6xxx_write(chip, REG_GLOBAL, GLOBAL_MAC_01,
- (addr[0] << 8) | addr[1]);
+ err = mv88e6xxx_g1_write(chip, GLOBAL_MAC_01, (addr[0] << 8) | addr[1]);
if (err)
return err;
- err = mv88e6xxx_write(chip, REG_GLOBAL, GLOBAL_MAC_23,
- (addr[2] << 8) | addr[3]);
+ err = mv88e6xxx_g1_write(chip, GLOBAL_MAC_23, (addr[2] << 8) | addr[3]);
if (err)
return err;
- return mv88e6xxx_write(chip, REG_GLOBAL, GLOBAL_MAC_45,
- (addr[4] << 8) | addr[5]);
+ err = mv88e6xxx_g1_write(chip, GLOBAL_MAC_45, (addr[4] << 8) | addr[5]);
+ if (err)
+ return err;
+
+ return 0;
}
static int mv88e6xxx_g1_set_age_time(struct mv88e6xxx_chip *chip,
@@ -2783,7 +2756,7 @@ static int mv88e6xxx_g1_set_age_time(struct mv88e6xxx_chip *chip,
/* Round to nearest multiple of coeff */
age_time = (msecs + coeff / 2) / coeff;
- err = mv88e6xxx_read(chip, REG_GLOBAL, GLOBAL_ATU_CONTROL, &val);
+ err = mv88e6xxx_g1_read(chip, GLOBAL_ATU_CONTROL, &val);
if (err)
return err;
@@ -2791,7 +2764,7 @@ static int mv88e6xxx_g1_set_age_time(struct mv88e6xxx_chip *chip,
val &= ~0xff0;
val |= age_time << 4;
- return mv88e6xxx_write(chip, REG_GLOBAL, GLOBAL_ATU_CONTROL, val);
+ return mv88e6xxx_g1_write(chip, GLOBAL_ATU_CONTROL, val);
}
static int mv88e6xxx_set_ageing_time(struct dsa_switch *ds,
@@ -2822,7 +2795,7 @@ static int mv88e6xxx_g1_setup(struct mv88e6xxx_chip *chip)
mv88e6xxx_has(chip, MV88E6XXX_FLAG_PPU_ACTIVE))
reg |= GLOBAL_CONTROL_PPU_ENABLE;
- err = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_CONTROL, reg);
+ err = mv88e6xxx_g1_write(chip, GLOBAL_CONTROL, reg);
if (err)
return err;
@@ -2832,15 +2805,14 @@ static int mv88e6xxx_g1_setup(struct mv88e6xxx_chip *chip)
reg = upstream_port << GLOBAL_MONITOR_CONTROL_INGRESS_SHIFT |
upstream_port << GLOBAL_MONITOR_CONTROL_EGRESS_SHIFT |
upstream_port << GLOBAL_MONITOR_CONTROL_ARP_SHIFT;
- err = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_MONITOR_CONTROL,
- reg);
+ err = mv88e6xxx_g1_write(chip, GLOBAL_MONITOR_CONTROL, reg);
if (err)
return err;
/* Disable remote management, and set the switch's DSA device number. */
- err = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_CONTROL_2,
- GLOBAL_CONTROL_2_MULTIPLE_CASCADE |
- (ds->index & 0x1f));
+ err = mv88e6xxx_g1_write(chip, GLOBAL_CONTROL_2,
+ GLOBAL_CONTROL_2_MULTIPLE_CASCADE |
+ (ds->index & 0x1f));
if (err)
return err;
@@ -2853,8 +2825,8 @@ static int mv88e6xxx_g1_setup(struct mv88e6xxx_chip *chip)
* enable address learn messages to be sent to all message
* ports.
*/
- err = mv88e6xxx_write(chip, REG_GLOBAL, GLOBAL_ATU_CONTROL,
- GLOBAL_ATU_CONTROL_LEARN2ALL);
+ err = mv88e6xxx_g1_write(chip, GLOBAL_ATU_CONTROL,
+ GLOBAL_ATU_CONTROL_LEARN2ALL);
if (err)
return err;
@@ -2868,39 +2840,39 @@ static int mv88e6xxx_g1_setup(struct mv88e6xxx_chip *chip)
return err;
/* Configure the IP ToS mapping registers. */
- err = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_IP_PRI_0, 0x0000);
+ err = mv88e6xxx_g1_write(chip, GLOBAL_IP_PRI_0, 0x0000);
if (err)
return err;
- err = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_IP_PRI_1, 0x0000);
+ err = mv88e6xxx_g1_write(chip, GLOBAL_IP_PRI_1, 0x0000);
if (err)
return err;
- err = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_IP_PRI_2, 0x5555);
+ err = mv88e6xxx_g1_write(chip, GLOBAL_IP_PRI_2, 0x5555);
if (err)
return err;
- err = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_IP_PRI_3, 0x5555);
+ err = mv88e6xxx_g1_write(chip, GLOBAL_IP_PRI_3, 0x5555);
if (err)
return err;
- err = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_IP_PRI_4, 0xaaaa);
+ err = mv88e6xxx_g1_write(chip, GLOBAL_IP_PRI_4, 0xaaaa);
if (err)
return err;
- err = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_IP_PRI_5, 0xaaaa);
+ err = mv88e6xxx_g1_write(chip, GLOBAL_IP_PRI_5, 0xaaaa);
if (err)
return err;
- err = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_IP_PRI_6, 0xffff);
+ err = mv88e6xxx_g1_write(chip, GLOBAL_IP_PRI_6, 0xffff);
if (err)
return err;
- err = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_IP_PRI_7, 0xffff);
+ err = mv88e6xxx_g1_write(chip, GLOBAL_IP_PRI_7, 0xffff);
if (err)
return err;
/* Configure the IEEE 802.1p priority mapping register. */
- err = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_IEEE_PRI, 0xfa41);
+ err = mv88e6xxx_g1_write(chip, GLOBAL_IEEE_PRI, 0xfa41);
if (err)
return err;
/* Clear the statistics counters for all ports */
- err = _mv88e6xxx_reg_write(chip, REG_GLOBAL, GLOBAL_STATS_OP,
- GLOBAL_STATS_OP_FLUSH_ALL);
+ err = mv88e6xxx_g1_write(chip, GLOBAL_STATS_OP,
+ GLOBAL_STATS_OP_FLUSH_ALL);
if (err)
return err;
@@ -3265,6 +3237,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
.num_databases = 4096,
.num_ports = 10,
.port_base_addr = 0x10,
+ .global1_addr = 0x1b,
.age_time_coeff = 15000,
.flags = MV88E6XXX_FLAGS_FAMILY_6097,
},
@@ -3276,6 +3249,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
.num_databases = 256,
.num_ports = 11,
.port_base_addr = 0x10,
+ .global1_addr = 0x1b,
.age_time_coeff = 15000,
.flags = MV88E6XXX_FLAGS_FAMILY_6095,
},
@@ -3287,6 +3261,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
.num_databases = 4096,
.num_ports = 3,
.port_base_addr = 0x10,
+ .global1_addr = 0x1b,
.age_time_coeff = 15000,
.flags = MV88E6XXX_FLAGS_FAMILY_6165,
},
@@ -3298,6 +3273,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
.num_databases = 256,
.num_ports = 8,
.port_base_addr = 0x10,
+ .global1_addr = 0x1b,
.age_time_coeff = 15000,
.flags = MV88E6XXX_FLAGS_FAMILY_6185,
},
@@ -3309,6 +3285,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
.num_databases = 4096,
.num_ports = 6,
.port_base_addr = 0x10,
+ .global1_addr = 0x1b,
.age_time_coeff = 15000,
.flags = MV88E6XXX_FLAGS_FAMILY_6165,
},
@@ -3320,6 +3297,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
.num_databases = 4096,
.num_ports = 6,
.port_base_addr = 0x10,
+ .global1_addr = 0x1b,
.age_time_coeff = 15000,
.flags = MV88E6XXX_FLAGS_FAMILY_6165,
},
@@ -3331,6 +3309,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
.num_databases = 4096,
.num_ports = 7,
.port_base_addr = 0x10,
+ .global1_addr = 0x1b,
.age_time_coeff = 15000,
.flags = MV88E6XXX_FLAGS_FAMILY_6351,
},
@@ -3342,6 +3321,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
.num_databases = 4096,
.num_ports = 7,
.port_base_addr = 0x10,
+ .global1_addr = 0x1b,
.age_time_coeff = 15000,
.flags = MV88E6XXX_FLAGS_FAMILY_6352,
},
@@ -3353,6 +3333,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
.num_databases = 4096,
.num_ports = 7,
.port_base_addr = 0x10,
+ .global1_addr = 0x1b,
.age_time_coeff = 15000,
.flags = MV88E6XXX_FLAGS_FAMILY_6351,
},
@@ -3364,6 +3345,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
.num_databases = 4096,
.num_ports = 7,
.port_base_addr = 0x10,
+ .global1_addr = 0x1b,
.age_time_coeff = 15000,
.flags = MV88E6XXX_FLAGS_FAMILY_6352,
},
@@ -3375,6 +3357,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
.num_databases = 256,
.num_ports = 10,
.port_base_addr = 0x10,
+ .global1_addr = 0x1b,
.age_time_coeff = 15000,
.flags = MV88E6XXX_FLAGS_FAMILY_6185,
},
@@ -3386,6 +3369,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
.num_databases = 4096,
.num_ports = 7,
.port_base_addr = 0x10,
+ .global1_addr = 0x1b,
.age_time_coeff = 15000,
.flags = MV88E6XXX_FLAGS_FAMILY_6352,
},
@@ -3397,6 +3381,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
.num_databases = 4096,
.num_ports = 7,
.port_base_addr = 0x10,
+ .global1_addr = 0x1b,
.age_time_coeff = 15000,
.flags = MV88E6XXX_FLAGS_FAMILY_6320,
},
@@ -3408,6 +3393,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
.num_databases = 4096,
.num_ports = 7,
.port_base_addr = 0x10,
+ .global1_addr = 0x1b,
.age_time_coeff = 15000,
.flags = MV88E6XXX_FLAGS_FAMILY_6320,
},
@@ -3419,6 +3405,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
.num_databases = 4096,
.num_ports = 7,
.port_base_addr = 0x10,
+ .global1_addr = 0x1b,
.age_time_coeff = 15000,
.flags = MV88E6XXX_FLAGS_FAMILY_6351,
},
@@ -3430,6 +3417,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
.num_databases = 4096,
.num_ports = 7,
.port_base_addr = 0x10,
+ .global1_addr = 0x1b,
.age_time_coeff = 15000,
.flags = MV88E6XXX_FLAGS_FAMILY_6351,
},
@@ -3441,6 +3429,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
.num_databases = 4096,
.num_ports = 7,
.port_base_addr = 0x10,
+ .global1_addr = 0x1b,
.age_time_coeff = 15000,
.flags = MV88E6XXX_FLAGS_FAMILY_6352,
},
diff --git a/drivers/net/dsa/mv88e6xxx/global1.c b/drivers/net/dsa/mv88e6xxx/global1.c
new file mode 100644
index 0000000..d358720
--- /dev/null
+++ b/drivers/net/dsa/mv88e6xxx/global1.c
@@ -0,0 +1,34 @@
+/*
+ * Marvell 88E6xxx Switch Global (1) Registers support
+ *
+ * Copyright (c) 2008 Marvell Semiconductor
+ *
+ * Copyright (c) 2016 Vivien Didelot <vivien.didelot@savoirfairelinux.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include "mv88e6xxx.h"
+#include "global1.h"
+
+int mv88e6xxx_g1_read(struct mv88e6xxx_chip *chip, int reg, u16 *val)
+{
+ int addr = chip->info->global1_addr;
+
+ return mv88e6xxx_read(chip, addr, reg, val);
+}
+
+int mv88e6xxx_g1_write(struct mv88e6xxx_chip *chip, int reg, u16 val)
+{
+ int addr = chip->info->global1_addr;
+
+ return mv88e6xxx_write(chip, addr, reg, val);
+}
+
+int mv88e6xxx_g1_wait(struct mv88e6xxx_chip *chip, int reg, u16 mask)
+{
+ return mv88e6xxx_wait(chip, chip->info->global1_addr, reg, mask);
+}
diff --git a/drivers/net/dsa/mv88e6xxx/global1.h b/drivers/net/dsa/mv88e6xxx/global1.h
new file mode 100644
index 0000000..62291e6
--- /dev/null
+++ b/drivers/net/dsa/mv88e6xxx/global1.h
@@ -0,0 +1,23 @@
+/*
+ * Marvell 88E6xxx Switch Global (1) Registers support
+ *
+ * Copyright (c) 2008 Marvell Semiconductor
+ *
+ * Copyright (c) 2016 Vivien Didelot <vivien.didelot@savoirfairelinux.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef _MV88E6XXX_GLOBAL1_H
+#define _MV88E6XXX_GLOBAL1_H
+
+#include "mv88e6xxx.h"
+
+int mv88e6xxx_g1_read(struct mv88e6xxx_chip *chip, int reg, u16 *val);
+int mv88e6xxx_g1_write(struct mv88e6xxx_chip *chip, int reg, u16 val);
+int mv88e6xxx_g1_wait(struct mv88e6xxx_chip *chip, int reg, u16 mask);
+
+#endif /* _MV88E6XXX_GLOBAL1_H */
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index 8279883..bf78f6d 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -159,7 +159,6 @@
#define PORT_TAG_REGMAP_0123 0x18
#define PORT_TAG_REGMAP_4567 0x19
-#define REG_GLOBAL 0x1b
#define GLOBAL_STATUS 0x00
#define GLOBAL_STATUS_PPU_STATE BIT(15) /* 6351 and 6171 */
/* Two bits for 6165, 6185 etc */
@@ -613,6 +612,7 @@ struct mv88e6xxx_info {
unsigned int num_databases;
unsigned int num_ports;
unsigned int port_base_addr;
+ unsigned int global1_addr;
unsigned int age_time_coeff;
unsigned long long flags;
};
--
2.10.0
^ permalink raw reply related
* [PATCH net-next 03/10] net: dsa: mv88e6xxx: add flags for FID registers
From: Vivien Didelot @ 2016-09-29 16:21 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
In-Reply-To: <20160929162202.10685-1-vivien.didelot@savoirfairelinux.com>
Add flags to describe the presence of Global 1 ATU FID register (0x01)
and VTU FID register (0x02), instead of checking families.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
drivers/net/dsa/mv88e6xxx/chip.c | 16 +++-------------
drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 24 ++++++++++++++++++++----
2 files changed, 23 insertions(+), 17 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 98dee2c..b7eecc9 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -565,16 +565,6 @@ static unsigned int mv88e6xxx_num_databases(struct mv88e6xxx_chip *chip)
return chip->info->num_databases;
}
-static bool mv88e6xxx_has_fid_reg(struct mv88e6xxx_chip *chip)
-{
- /* Does the device have dedicated FID registers for ATU and VTU ops? */
- if (mv88e6xxx_6097_family(chip) || mv88e6xxx_6165_family(chip) ||
- mv88e6xxx_6351_family(chip) || mv88e6xxx_6352_family(chip))
- return true;
-
- return false;
-}
-
/* We expect the switch to perform auto negotiation if there is a real
* phy. However, in the case of a fixed link phy, we force the port
* settings from the fixed link settings.
@@ -978,7 +968,7 @@ static int _mv88e6xxx_atu_cmd(struct mv88e6xxx_chip *chip, u16 fid, u16 cmd)
u16 val;
int err;
- if (mv88e6xxx_has_fid_reg(chip)) {
+ if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_G1_ATU_FID)) {
err = mv88e6xxx_g1_write(chip, GLOBAL_ATU_FID, fid);
if (err)
return err;
@@ -1386,7 +1376,7 @@ static int _mv88e6xxx_vtu_getnext(struct mv88e6xxx_chip *chip,
if (err)
return err;
- if (mv88e6xxx_has_fid_reg(chip)) {
+ if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_G1_VTU_FID)) {
err = mv88e6xxx_g1_read(chip, GLOBAL_VTU_FID, &val);
if (err)
return err;
@@ -1498,7 +1488,7 @@ static int _mv88e6xxx_vtu_loadpurge(struct mv88e6xxx_chip *chip,
return err;
}
- if (mv88e6xxx_has_fid_reg(chip)) {
+ if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_G1_VTU_FID)) {
reg = entry->fid & GLOBAL_VTU_FID_MASK;
err = mv88e6xxx_g1_write(chip, GLOBAL_VTU_FID, reg);
if (err)
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index 2f10108..6c8584f 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -170,8 +170,8 @@
#define GLOBAL_MAC_01 0x01
#define GLOBAL_MAC_23 0x02
#define GLOBAL_MAC_45 0x03
-#define GLOBAL_ATU_FID 0x01 /* 6097 6165 6351 6352 */
-#define GLOBAL_VTU_FID 0x02 /* 6097 6165 6351 6352 */
+#define GLOBAL_ATU_FID 0x01
+#define GLOBAL_VTU_FID 0x02
#define GLOBAL_VTU_FID_MASK 0xfff
#define GLOBAL_VTU_SID 0x03 /* 6097 6165 6351 6352 */
#define GLOBAL_VTU_SID_MASK 0x3f
@@ -408,6 +408,11 @@ enum mv88e6xxx_cap {
*/
MV88E6XXX_CAP_SERDES,
+ /* Switch Global (1) Registers.
+ */
+ MV88E6XXX_CAP_G1_ATU_FID, /* (0x01) ATU FID Register */
+ MV88E6XXX_CAP_G1_VTU_FID, /* (0x02) VTU FID Register */
+
/* Switch Global 2 Registers.
* The device contains a second set of global 16-bit registers.
*/
@@ -460,6 +465,9 @@ enum mv88e6xxx_cap {
#define MV88E6XXX_FLAG_SERDES BIT_ULL(MV88E6XXX_CAP_SERDES)
+#define MV88E6XXX_FLAG_G1_ATU_FID BIT_ULL(MV88E6XXX_CAP_G1_ATU_FID)
+#define MV88E6XXX_FLAG_G1_VTU_FID BIT_ULL(MV88E6XXX_CAP_G1_VTU_FID)
+
#define MV88E6XXX_FLAG_GLOBAL2 BIT_ULL(MV88E6XXX_CAP_GLOBAL2)
#define MV88E6XXX_FLAG_G2_MGMT_EN_2X BIT_ULL(MV88E6XXX_CAP_G2_MGMT_EN_2X)
#define MV88E6XXX_FLAG_G2_MGMT_EN_0X BIT_ULL(MV88E6XXX_CAP_G2_MGMT_EN_0X)
@@ -519,7 +527,9 @@ enum mv88e6xxx_cap {
MV88E6XXX_FLAGS_MULTI_CHIP)
#define MV88E6XXX_FLAGS_FAMILY_6097 \
- (MV88E6XXX_FLAG_GLOBAL2 | \
+ (MV88E6XXX_FLAG_G1_ATU_FID | \
+ MV88E6XXX_FLAG_G1_VTU_FID | \
+ MV88E6XXX_FLAG_GLOBAL2 | \
MV88E6XXX_FLAG_G2_MGMT_EN_2X | \
MV88E6XXX_FLAG_G2_MGMT_EN_0X | \
MV88E6XXX_FLAG_G2_POT | \
@@ -531,7 +541,9 @@ enum mv88e6xxx_cap {
MV88E6XXX_FLAGS_PVT)
#define MV88E6XXX_FLAGS_FAMILY_6165 \
- (MV88E6XXX_FLAG_GLOBAL2 | \
+ (MV88E6XXX_FLAG_G1_ATU_FID | \
+ MV88E6XXX_FLAG_G1_VTU_FID | \
+ MV88E6XXX_FLAG_GLOBAL2 | \
MV88E6XXX_FLAG_G2_MGMT_EN_2X | \
MV88E6XXX_FLAG_G2_MGMT_EN_0X | \
MV88E6XXX_FLAG_G2_SWITCH_MAC | \
@@ -570,6 +582,8 @@ enum mv88e6xxx_cap {
#define MV88E6XXX_FLAGS_FAMILY_6351 \
(MV88E6XXX_FLAG_EDSA | \
+ MV88E6XXX_FLAG_G1_ATU_FID | \
+ MV88E6XXX_FLAG_G1_VTU_FID | \
MV88E6XXX_FLAG_GLOBAL2 | \
MV88E6XXX_FLAG_G2_MGMT_EN_2X | \
MV88E6XXX_FLAG_G2_MGMT_EN_0X | \
@@ -587,6 +601,8 @@ enum mv88e6xxx_cap {
#define MV88E6XXX_FLAGS_FAMILY_6352 \
(MV88E6XXX_FLAG_EDSA | \
MV88E6XXX_FLAG_EEE | \
+ MV88E6XXX_FLAG_G1_ATU_FID | \
+ MV88E6XXX_FLAG_G1_VTU_FID | \
MV88E6XXX_FLAG_GLOBAL2 | \
MV88E6XXX_FLAG_G2_MGMT_EN_2X | \
MV88E6XXX_FLAG_G2_MGMT_EN_0X | \
--
2.10.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox