* Re: [net-next PATCH 0/8] Fix Tunnel features and enable GSO partial for Mellanox adapters
From: David Miller @ 2016-04-29 17:31 UTC (permalink / raw)
To: aduyck; +Cc: talal, netdev, galp, ogerlitz, eranbe
In-Reply-To: <20160425182442.11331.88349.stgit@ahduyck-xeon-server>
From: Alexander Duyck <aduyck@mirantis.com>
Date: Mon, 25 Apr 2016 11:30:54 -0700
> This patch series is meant to allow us to get the best performance possible
> for Mellanox ConnectX-3/4 adapters in terms of VXLAN tunnels.
I'm going to mark this as "deferred" in patchwork, so Alex why don't you just
respin these and repost next week when you get final feedback from the Mellanox
folks?
THanks.
^ permalink raw reply
* [PATCH] sctp: signal sk_data_ready earlier on data chunks reception
From: Marcelo Ricardo Leitner @ 2016-04-29 17:17 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Neil Horman, Vlad Yasevich, linux-sctp
Dave Miller pointed out that fb586f25300f ("sctp: delay calls to
sk_data_ready() as much as possible") may insert latency specially if
the receiving application is running on another CPU and that it would be
better if we signalled as early as possible.
This patch thus basically inverts the logic on fb586f25300f and signals
it as early as possible, similar to what we had before.
Fixes: fb586f25300f ("sctp: delay calls to sk_data_ready() as much as possible")
Reported-by: Dave Miller <davem@davemloft.net>
Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
include/net/sctp/structs.h | 2 +-
net/sctp/sm_sideeffect.c | 7 +++----
net/sctp/ulpqueue.c | 25 ++++++++++++++++---------
3 files changed, 20 insertions(+), 14 deletions(-)
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 558bae3cbe0d5107d52c8cb31b324cfd5479def0..16b013a6191cf1c416e4dd1aeb1707a8569ea49b 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -218,7 +218,7 @@ struct sctp_sock {
frag_interleave:1,
recvrcvinfo:1,
recvnxtinfo:1,
- pending_data_ready:1;
+ data_ready_signalled:1;
atomic_t pd_mode;
/* Receive to here while partial delivery is in effect. */
diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index e8f0112f9b28472c39c4c91dcb28576373c858e7..aa37122593684d8501fdca15983fbd8620fabe07 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -1741,10 +1741,9 @@ out:
} else if (local_cork)
error = sctp_outq_uncork(&asoc->outqueue, gfp);
- if (sp->pending_data_ready) {
- sk->sk_data_ready(sk);
- sp->pending_data_ready = 0;
- }
+ if (sp->data_ready_signalled)
+ sp->data_ready_signalled = 0;
+
return error;
nomem:
error = -ENOMEM;
diff --git a/net/sctp/ulpqueue.c b/net/sctp/ulpqueue.c
index ec12a8920e5fd7a0f26d19f1695bc2feeae41518..ec166d2bd2d95d9aa69369da2ead9437da4ce8ed 100644
--- a/net/sctp/ulpqueue.c
+++ b/net/sctp/ulpqueue.c
@@ -194,6 +194,7 @@ static int sctp_ulpq_clear_pd(struct sctp_ulpq *ulpq)
int sctp_ulpq_tail_event(struct sctp_ulpq *ulpq, struct sctp_ulpevent *event)
{
struct sock *sk = ulpq->asoc->base.sk;
+ struct sctp_sock *sp = sctp_sk(sk);
struct sk_buff_head *queue, *skb_list;
struct sk_buff *skb = sctp_event2skb(event);
int clear_pd = 0;
@@ -211,7 +212,7 @@ int sctp_ulpq_tail_event(struct sctp_ulpq *ulpq, struct sctp_ulpevent *event)
sk_incoming_cpu_update(sk);
}
/* Check if the user wishes to receive this event. */
- if (!sctp_ulpevent_is_enabled(event, &sctp_sk(sk)->subscribe))
+ if (!sctp_ulpevent_is_enabled(event, &sp->subscribe))
goto out_free;
/* If we are in partial delivery mode, post to the lobby until
@@ -219,7 +220,7 @@ int sctp_ulpq_tail_event(struct sctp_ulpq *ulpq, struct sctp_ulpevent *event)
* the association the cause of the partial delivery.
*/
- if (atomic_read(&sctp_sk(sk)->pd_mode) == 0) {
+ if (atomic_read(&sp->pd_mode) == 0) {
queue = &sk->sk_receive_queue;
} else {
if (ulpq->pd_mode) {
@@ -231,7 +232,7 @@ int sctp_ulpq_tail_event(struct sctp_ulpq *ulpq, struct sctp_ulpevent *event)
if ((event->msg_flags & MSG_NOTIFICATION) ||
(SCTP_DATA_NOT_FRAG ==
(event->msg_flags & SCTP_DATA_FRAG_MASK)))
- queue = &sctp_sk(sk)->pd_lobby;
+ queue = &sp->pd_lobby;
else {
clear_pd = event->msg_flags & MSG_EOR;
queue = &sk->sk_receive_queue;
@@ -242,10 +243,10 @@ int sctp_ulpq_tail_event(struct sctp_ulpq *ulpq, struct sctp_ulpevent *event)
* can queue this to the receive queue instead
* of the lobby.
*/
- if (sctp_sk(sk)->frag_interleave)
+ if (sp->frag_interleave)
queue = &sk->sk_receive_queue;
else
- queue = &sctp_sk(sk)->pd_lobby;
+ queue = &sp->pd_lobby;
}
}
@@ -264,8 +265,10 @@ int sctp_ulpq_tail_event(struct sctp_ulpq *ulpq, struct sctp_ulpevent *event)
if (clear_pd)
sctp_ulpq_clear_pd(ulpq);
- if (queue == &sk->sk_receive_queue)
- sctp_sk(sk)->pending_data_ready = 1;
+ if (queue == &sk->sk_receive_queue && !sp->data_ready_signalled) {
+ sp->data_ready_signalled = 1;
+ sk->sk_data_ready(sk);
+ }
return 1;
out_free:
@@ -1126,11 +1129,13 @@ void sctp_ulpq_abort_pd(struct sctp_ulpq *ulpq, gfp_t gfp)
{
struct sctp_ulpevent *ev = NULL;
struct sock *sk;
+ struct sctp_sock *sp;
if (!ulpq->pd_mode)
return;
sk = ulpq->asoc->base.sk;
+ sp = sctp_sk(sk);
if (sctp_ulpevent_type_enabled(SCTP_PARTIAL_DELIVERY_EVENT,
&sctp_sk(sk)->subscribe))
ev = sctp_ulpevent_make_pdapi(ulpq->asoc,
@@ -1140,6 +1145,8 @@ void sctp_ulpq_abort_pd(struct sctp_ulpq *ulpq, gfp_t gfp)
__skb_queue_tail(&sk->sk_receive_queue, sctp_event2skb(ev));
/* If there is data waiting, send it up the socket now. */
- if (sctp_ulpq_clear_pd(ulpq) || ev)
- sctp_sk(sk)->pending_data_ready = 1;
+ if ((sctp_ulpq_clear_pd(ulpq) || ev) && !sp->data_ready_signalled) {
+ sp->data_ready_signalled = 1;
+ sk->sk_data_ready(sk);
+ }
}
--
2.5.0
^ permalink raw reply related
* [PATCH 2/3] fs: poll/select/recvmmsg: use timespec64 for timeout events
From: Deepa Dinamani @ 2016-04-29 16:39 UTC (permalink / raw)
To: linux-kernel
Cc: arnd, y2038, linux-fsdevel, Alexander Viro, David S. Miller,
netdev
In-Reply-To: <1461947989-21926-1-git-send-email-deepa.kernel@gmail.com>
struct timespec is not y2038 safe.
Even though timespec might be sufficient to represent
timeouts, use struct timespec64 here as the plan is to
get rid of all timespec reference in the kernel.
The patch transitions the common functions:
poll_select_set_timeout() and select_estimate_accuracy()
to use timespec64. And, all the syscalls that use these
functions are transitioned in the same patch.
The restart block parameters for poll uses monotonic time.
Use timespec64 here as well to assign timeout value. This
parameter in the restart block need not change because
this only holds the monotonic timestamp at which timeout
should occur. And, unsigned long data type should be big
enough for this timestamp.
The system call interfaces will be handled in a separate
series.
Compat interfaces need not change as timespec64 is an
alias to struct timespec on a 64 bit system.
Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
---
fs/eventpoll.c | 12 +++++-----
fs/select.c | 67 +++++++++++++++++++++++++++++-----------------------
include/linux/poll.h | 11 +++++----
net/socket.c | 8 ++++---
4 files changed, 54 insertions(+), 44 deletions(-)
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 8a74a2a..10db912 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -1583,15 +1583,15 @@ static int ep_send_events(struct eventpoll *ep,
return ep_scan_ready_list(ep, ep_send_events_proc, &esed, 0, false);
}
-static inline struct timespec ep_set_mstimeout(long ms)
+static inline struct timespec64 ep_set_mstimeout(long ms)
{
- struct timespec now, ts = {
+ struct timespec64 now, ts = {
.tv_sec = ms / MSEC_PER_SEC,
.tv_nsec = NSEC_PER_MSEC * (ms % MSEC_PER_SEC),
};
- ktime_get_ts(&now);
- return timespec_add_safe(now, ts);
+ ktime_get_ts64(&now);
+ return timespec64_add_safe(now, ts);
}
/**
@@ -1621,11 +1621,11 @@ static int ep_poll(struct eventpoll *ep, struct epoll_event __user *events,
ktime_t expires, *to = NULL;
if (timeout > 0) {
- struct timespec end_time = ep_set_mstimeout(timeout);
+ struct timespec64 end_time = ep_set_mstimeout(timeout);
slack = select_estimate_accuracy(&end_time);
to = &expires;
- *to = timespec_to_ktime(end_time);
+ *to = timespec64_to_ktime(end_time);
} else if (timeout == 0) {
/*
* Avoid the unnecessary trip to the wait queue loop, if the
diff --git a/fs/select.c b/fs/select.c
index 8692939..8ed9da5 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -47,7 +47,7 @@
#define MAX_SLACK (100 * NSEC_PER_MSEC)
-static long __estimate_accuracy(struct timespec *tv)
+static long __estimate_accuracy(struct timespec64 *tv)
{
long slack;
int divfactor = 1000;
@@ -70,10 +70,10 @@ static long __estimate_accuracy(struct timespec *tv)
return slack;
}
-u64 select_estimate_accuracy(struct timespec *tv)
+u64 select_estimate_accuracy(struct timespec64 *tv)
{
u64 ret;
- struct timespec now;
+ struct timespec64 now;
/*
* Realtime tasks get a slack of 0 for obvious reasons.
@@ -82,8 +82,8 @@ u64 select_estimate_accuracy(struct timespec *tv)
if (rt_task(current))
return 0;
- ktime_get_ts(&now);
- now = timespec_sub(*tv, now);
+ ktime_get_ts64(&now);
+ now = timespec64_sub(*tv, now);
ret = __estimate_accuracy(&now);
if (ret < current->timer_slack_ns)
return current->timer_slack_ns;
@@ -260,7 +260,7 @@ EXPORT_SYMBOL(poll_schedule_timeout);
/**
* poll_select_set_timeout - helper function to setup the timeout value
- * @to: pointer to timespec variable for the final timeout
+ * @to: pointer to timespec64 variable for the final timeout
* @sec: seconds (from user space)
* @nsec: nanoseconds (from user space)
*
@@ -269,26 +269,28 @@ EXPORT_SYMBOL(poll_schedule_timeout);
*
* Returns -EINVAL if sec/nsec are not normalized. Otherwise 0.
*/
-int poll_select_set_timeout(struct timespec *to, long sec, long nsec)
+int poll_select_set_timeout(struct timespec64 *to, time64_t sec, long nsec)
{
- struct timespec ts = {.tv_sec = sec, .tv_nsec = nsec};
+ struct timespec64 ts = {.tv_sec = sec, .tv_nsec = nsec};
- if (!timespec_valid(&ts))
+ if (!timespec64_valid(&ts))
return -EINVAL;
/* Optimize for the zero timeout value here */
if (!sec && !nsec) {
to->tv_sec = to->tv_nsec = 0;
} else {
- ktime_get_ts(to);
- *to = timespec_add_safe(*to, ts);
+ ktime_get_ts64(to);
+ *to = timespec64_add_safe(*to, ts);
}
return 0;
}
-static int poll_select_copy_remaining(struct timespec *end_time, void __user *p,
+static int poll_select_copy_remaining(struct timespec64 *end_time,
+ void __user *p,
int timeval, int ret)
{
+ struct timespec64 rts64;
struct timespec rts;
struct timeval rtv;
@@ -302,16 +304,18 @@ static int poll_select_copy_remaining(struct timespec *end_time, void __user *p,
if (!end_time->tv_sec && !end_time->tv_nsec)
return ret;
- ktime_get_ts(&rts);
- rts = timespec_sub(*end_time, rts);
- if (rts.tv_sec < 0)
- rts.tv_sec = rts.tv_nsec = 0;
+ ktime_get_ts64(&rts64);
+ rts64 = timespec64_sub(*end_time, rts64);
+ if (rts64.tv_sec < 0)
+ rts64.tv_sec = rts64.tv_nsec = 0;
+
+ rts = timespec64_to_timespec(rts64);
if (timeval) {
if (sizeof(rtv) > sizeof(rtv.tv_sec) + sizeof(rtv.tv_usec))
memset(&rtv, 0, sizeof(rtv));
- rtv.tv_sec = rts.tv_sec;
- rtv.tv_usec = rts.tv_nsec / NSEC_PER_USEC;
+ rtv.tv_sec = rts64.tv_sec;
+ rtv.tv_usec = rts64.tv_nsec / NSEC_PER_USEC;
if (!copy_to_user(p, &rtv, sizeof(rtv)))
return ret;
@@ -396,7 +400,7 @@ static inline void wait_key_set(poll_table *wait, unsigned long in,
wait->_key |= POLLOUT_SET;
}
-int do_select(int n, fd_set_bits *fds, struct timespec *end_time)
+int do_select(int n, fd_set_bits *fds, struct timespec64 *end_time)
{
ktime_t expire, *to = NULL;
struct poll_wqueues table;
@@ -522,7 +526,7 @@ int do_select(int n, fd_set_bits *fds, struct timespec *end_time)
* pointer to the expiry value.
*/
if (end_time && !to) {
- expire = timespec_to_ktime(*end_time);
+ expire = timespec64_to_ktime(*end_time);
to = &expire;
}
@@ -545,7 +549,7 @@ int do_select(int n, fd_set_bits *fds, struct timespec *end_time)
* I'm trying ERESTARTNOHAND which restart only when you want to.
*/
int core_sys_select(int n, fd_set __user *inp, fd_set __user *outp,
- fd_set __user *exp, struct timespec *end_time)
+ fd_set __user *exp, struct timespec64 *end_time)
{
fd_set_bits fds;
void *bits;
@@ -622,7 +626,7 @@ out_nofds:
SYSCALL_DEFINE5(select, int, n, fd_set __user *, inp, fd_set __user *, outp,
fd_set __user *, exp, struct timeval __user *, tvp)
{
- struct timespec end_time, *to = NULL;
+ struct timespec64 end_time, *to = NULL;
struct timeval tv;
int ret;
@@ -648,15 +652,17 @@ static long do_pselect(int n, fd_set __user *inp, fd_set __user *outp,
const sigset_t __user *sigmask, size_t sigsetsize)
{
sigset_t ksigmask, sigsaved;
- struct timespec ts, end_time, *to = NULL;
+ struct timespec ts;
+ struct timespec64 ts64, end_time, *to = NULL;
int ret;
if (tsp) {
if (copy_from_user(&ts, tsp, sizeof(ts)))
return -EFAULT;
+ ts64 = timespec_to_timespec64(ts);
to = &end_time;
- if (poll_select_set_timeout(to, ts.tv_sec, ts.tv_nsec))
+ if (poll_select_set_timeout(to, ts64.tv_sec, ts64.tv_nsec))
return -EINVAL;
}
@@ -779,7 +785,7 @@ static inline unsigned int do_pollfd(struct pollfd *pollfd, poll_table *pwait,
}
static int do_poll(struct poll_list *list, struct poll_wqueues *wait,
- struct timespec *end_time)
+ struct timespec64 *end_time)
{
poll_table* pt = &wait->pt;
ktime_t expire, *to = NULL;
@@ -854,7 +860,7 @@ static int do_poll(struct poll_list *list, struct poll_wqueues *wait,
* pointer to the expiry value.
*/
if (end_time && !to) {
- expire = timespec_to_ktime(*end_time);
+ expire = timespec64_to_ktime(*end_time);
to = &expire;
}
@@ -868,7 +874,7 @@ static int do_poll(struct poll_list *list, struct poll_wqueues *wait,
sizeof(struct pollfd))
int do_sys_poll(struct pollfd __user *ufds, unsigned int nfds,
- struct timespec *end_time)
+ struct timespec64 *end_time)
{
struct poll_wqueues table;
int err = -EFAULT, fdcount, len, size;
@@ -936,7 +942,7 @@ static long do_restart_poll(struct restart_block *restart_block)
{
struct pollfd __user *ufds = restart_block->poll.ufds;
int nfds = restart_block->poll.nfds;
- struct timespec *to = NULL, end_time;
+ struct timespec64 *to = NULL, end_time;
int ret;
if (restart_block->poll.has_timeout) {
@@ -957,7 +963,7 @@ static long do_restart_poll(struct restart_block *restart_block)
SYSCALL_DEFINE3(poll, struct pollfd __user *, ufds, unsigned int, nfds,
int, timeout_msecs)
{
- struct timespec end_time, *to = NULL;
+ struct timespec64 end_time, *to = NULL;
int ret;
if (timeout_msecs >= 0) {
@@ -993,7 +999,8 @@ SYSCALL_DEFINE5(ppoll, struct pollfd __user *, ufds, unsigned int, nfds,
size_t, sigsetsize)
{
sigset_t ksigmask, sigsaved;
- struct timespec ts, end_time, *to = NULL;
+ struct timespec ts;
+ struct timespec64 end_time, *to = NULL;
int ret;
if (tsp) {
diff --git a/include/linux/poll.h b/include/linux/poll.h
index 9fb4f40..37b057b 100644
--- a/include/linux/poll.h
+++ b/include/linux/poll.h
@@ -96,7 +96,7 @@ extern void poll_initwait(struct poll_wqueues *pwq);
extern void poll_freewait(struct poll_wqueues *pwq);
extern int poll_schedule_timeout(struct poll_wqueues *pwq, int state,
ktime_t *expires, unsigned long slack);
-extern u64 select_estimate_accuracy(struct timespec *tv);
+extern u64 select_estimate_accuracy(struct timespec64 *tv);
static inline int poll_schedule(struct poll_wqueues *pwq, int state)
@@ -153,12 +153,13 @@ void zero_fd_set(unsigned long nr, unsigned long *fdset)
#define MAX_INT64_SECONDS (((s64)(~((u64)0)>>1)/HZ)-1)
-extern int do_select(int n, fd_set_bits *fds, struct timespec *end_time);
+extern int do_select(int n, fd_set_bits *fds, struct timespec64 *end_time);
extern int do_sys_poll(struct pollfd __user * ufds, unsigned int nfds,
- struct timespec *end_time);
+ struct timespec64 *end_time);
extern int core_sys_select(int n, fd_set __user *inp, fd_set __user *outp,
- fd_set __user *exp, struct timespec *end_time);
+ fd_set __user *exp, struct timespec64 *end_time);
-extern int poll_select_set_timeout(struct timespec *to, long sec, long nsec);
+extern int poll_select_set_timeout(struct timespec64 *to, time64_t sec,
+ long nsec);
#endif /* _LINUX_POLL_H */
diff --git a/net/socket.c b/net/socket.c
index 5dbb0bb..bdfe115 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2171,7 +2171,8 @@ int __sys_recvmmsg(int fd, struct mmsghdr __user *mmsg, unsigned int vlen,
struct mmsghdr __user *entry;
struct compat_mmsghdr __user *compat_entry;
struct msghdr msg_sys;
- struct timespec end_time;
+ struct timespec64 end_time;
+ struct timespec64 timeout64;
if (timeout &&
poll_select_set_timeout(&end_time, timeout->tv_sec,
@@ -2223,8 +2224,9 @@ int __sys_recvmmsg(int fd, struct mmsghdr __user *mmsg, unsigned int vlen,
flags |= MSG_DONTWAIT;
if (timeout) {
- ktime_get_ts(timeout);
- *timeout = timespec_sub(end_time, *timeout);
+ ktime_get_ts64(&timeout64);
+ *timeout = timespec64_to_timespec(
+ timespec64_sub(end_time, timeout64));
if (timeout->tv_sec < 0) {
timeout->tv_sec = timeout->tv_nsec = 0;
break;
--
1.9.1
^ permalink raw reply related
* [PATCH 0/3] Use timespec64 for select like timeouts
From: Deepa Dinamani @ 2016-04-29 16:39 UTC (permalink / raw)
To: linux-kernel
Cc: arnd, y2038, linux-fsdevel, John Stultz, Thomas Gleixner,
Alexander Viro, David S. Miller, netdev
The series is part of y2038 changes.
This changes a few syscalls that have common functions to use
struct timespec64 instead of struct timespec.
This does not include changes to system call uapi interfaces.
Those will be in a different series.
Thanks to Arnd Bergmann for comments on the patches.
Deepa Dinamani (3):
time: Add missing implementation for timespec64_add_safe()
fs: poll/select/recvmmsg: use timespec64 for timeout events
time: Remove timespec_add_safe()
fs/eventpoll.c | 12 ++++-----
fs/select.c | 67 ++++++++++++++++++++++++++++----------------------
include/linux/poll.h | 11 +++++----
include/linux/time64.h | 17 ++++++-------
kernel/time/time.c | 21 ++++++++++++++++
net/socket.c | 8 +++---
6 files changed, 82 insertions(+), 54 deletions(-)
--
1.9.1
Cc: John Stultz <john.stultz@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
^ permalink raw reply
* Re: [PATCH v3 0/2] sctp: delay calls to sk_data_ready() as much as possible
From: marcelo.leitner @ 2016-04-29 16:28 UTC (permalink / raw)
To: Neil Horman
Cc: David Miller, netdev, vyasevich, linux-sctp, David.Laight, jkbs
In-Reply-To: <20160429161031.GB31121@hmsreliant.think-freely.org>
On Fri, Apr 29, 2016 at 12:10:31PM -0400, Neil Horman wrote:
> On Fri, Apr 29, 2016 at 10:47:25AM -0300, marcelo.leitner@gmail.com wrote:
> > On Fri, Apr 29, 2016 at 09:36:37AM -0400, Neil Horman wrote:
> > > On Thu, Apr 28, 2016 at 05:46:59PM -0300, marcelo.leitner@gmail.com wrote:
> > > > On Thu, Apr 14, 2016 at 05:19:00PM -0300, marcelo.leitner@gmail.com wrote:
> > > > > On Thu, Apr 14, 2016 at 04:03:51PM -0400, Neil Horman wrote:
> > > > > > On Thu, Apr 14, 2016 at 02:59:16PM -0400, David Miller wrote:
> > > > > > > From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> > > > > > > Date: Thu, 14 Apr 2016 14:00:49 -0300
> > > > > > >
> > > > > > > > Em 14-04-2016 10:03, Neil Horman escreveu:
> > > > > > > >> On Wed, Apr 13, 2016 at 11:05:32PM -0400, David Miller wrote:
> > > > > > > >>> From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> > > > > > > >>> Date: Fri, 8 Apr 2016 16:41:26 -0300
> > > > > > > >>>
> > > > > > > >>>> 1st patch is a preparation for the 2nd. The idea is to not call
> > > > > > > >>>> ->sk_data_ready() for every data chunk processed while processing
> > > > > > > >>>> packets but only once before releasing the socket.
> > > > > > > >>>>
> > > > > > > >>>> v2: patchset re-checked, small changelog fixes
> > > > > > > >>>> v3: on patch 2, make use of local vars to make it more readable
> > > > > > > >>>
> > > > > > > >>> Applied to net-next, but isn't this reduced overhead coming at the
> > > > > > > >>> expense of latency? What if that lower latency is important to the
> > > > > > > >>> application and/or consumer?
> > > > > > > >> Thats a fair point, but I'd make the counter argument that, as it
> > > > > > > >> currently
> > > > > > > >> stands, any latency introduced (or removed), is an artifact of our
> > > > > > > >> implementation rather than a designed feature of it. That is to say,
> > > > > > > >> we make no
> > > > > > > >> guarantees at the application level regarding how long it takes to
> > > > > > > >> signal data
> > > > > > > >> readines from the time we get data off the wire, so I would rather see
> > > > > > > >> our
> > > > > > > >> throughput raised if we can, as thats been sctp's more pressing
> > > > > > > >> achilles heel.
> > > > > > > >>
> > > > > > > >>
> > > > > > > >> Thats not to say I'd like to enable lower latency, but I'd rather have
> > > > > > > >> this now,
> > > > > > > >> and start pondering how to design that in. Perhaps we can convert the
> > > > > > > >> pending
> > > > > > > >> flag to a counter to count the number of events we enqueue, and call
> > > > > > > >> sk_data_ready every time we reach a sysctl defined threshold.
> > > > > > > >
> > > > > > > > That and also that there is no chance of the application reading the
> > > > > > > > first chunks before all current ToDo's are performed by either the bh
> > > > > > > > or backlog handlers for that packet. Socket lock won't be cycled in
> > > > > > > > between chunks so the application is going to wait all the processing
> > > > > > > > one way or another.
> > > > > > >
> > > > > > > But it takes time to signal the wakeup to the remote cpu the process
> > > > > > > was running on, schedule out the current process on that cpu (if it
> > > > > > > has in fact lost it's timeslice), and then finally look at the socket
> > > > > > > queue.
> > > > > > >
> > > > > > > Of course this is all assuming the process was sleeping in the first
> > > > > > > place, either in recv or more likely poll.
> > > > > > >
> > > > > > > I really think signalling early helps performance.
> > > > > > >
> > > > > >
> > > > > > Early, yes, often, not so much :). Perhaps what would be adventageous would be
> > > > > > to signal at the start of a set of enqueues, rather than at the end. That would
> > > > > > be equivalent in terms of not signaling more than needed, but would eliminate
> > > > > > the signaling on every chunk. Perhaps what you could do Marcelo would be to
> > > > > > change the sense of the signal_ready flag to be a has_signaled flag. e.g. call
> > > > > > sk_data_ready in ulp_event_tail like we used to, but only if the has_signaled
> > > > > > flag isn't set, then set the flag, and clear it at the end of the command
> > > > > > interpreter.
> > > > > >
> > > > > > That would be a best of both worlds solution, as long as theres no chance of
> > > > > > race with user space reading from the socket before we were done enqueuing (i.e.
> > > > > > you have to guarantee that the socket lock stays held, which I think we do).
> > > > >
> > > > > That is my feeling too. Will work on it. Thanks :-)
> > > >
> > > > I did the change and tested it on real machines set all for performance.
> > > > I couldn't spot any difference between both implementations.
> > > >
> > > > Set RSS and queue irq affinity for a cpu and taskset netperf and another
> > > > app I wrote to run on another cpu. It hits socket backlog quite often
> > > > but still do direct processing every now and then.
> > > >
> > > > With current state, netperf, scenario above. Results of perf sched
> > > > record for the CPUs in use, reported by perf sched latency:
> > > >
> > > > Task | Runtime ms | Switches | Average delay ms |
> > > > Maximum delay ms | Maximum delay at |
> > > > netserver:3205 | 9999.490 ms | 10 | avg: 0.003 ms |
> > > > max: 0.004 ms | max at: 69087.753356 s
> > > >
> > > > another run
> > > > netserver:3483 | 9999.412 ms | 15 | avg: 0.003 ms |
> > > > max: 0.004 ms | max at: 69194.749814 s
> > > >
> > > > With the patch below, same test:
> > > > netserver:2643 | 10000.110 ms | 14 | avg: 0.003 ms |
> > > > max: 0.004 ms | max at: 172.006315 s
> > > >
> > > > another run:
> > > > netserver:2698 | 10000.049 ms | 15 | avg: 0.003 ms |
> > > > max: 0.004 ms | max at: 368.061672 s
> > > >
> > > > I'll be happy to do more tests if you have any suggestions on how/what
> > > > to test.
> > > >
> > > > ---8<---
> > > >
> > > I think this looks reasonable, but can you post it properly please, as a patch
> > > against the head of teh net-next tree, rather than a diff from your previous
> > > work (which wasn't comitted)
> >
> > The idea was to not officially post it yet, more just as a reference,
> > because I can't see any gains from it. I'm reluctant just due to that,
> > no strong opinion here on one way or another.
> >
> > If you think it's better anyway to signal it early, I'll properly repost
> > it.
> >
> Yeah, your results seem to me to indicate that for your test at least, signaling
> early vs. late doesn't make alot of difference, but Dave I think made a point in
> principle in that allowing processes to wake up when we start enqueuing can be
> better in some situations. So all other things being equal, I'd say go with the
> method that you have here.
Okay, I'll rebase the patch and post it properly. Thanks Neil!
Marcelo
^ permalink raw reply
* Re: [PATCH v3 0/2] sctp: delay calls to sk_data_ready() as much as possible
From: Neil Horman @ 2016-04-29 16:10 UTC (permalink / raw)
To: marcelo.leitner
Cc: David Miller, netdev, vyasevich, linux-sctp, David.Laight, jkbs
In-Reply-To: <20160429134725.GB5676@localhost.localdomain>
On Fri, Apr 29, 2016 at 10:47:25AM -0300, marcelo.leitner@gmail.com wrote:
> On Fri, Apr 29, 2016 at 09:36:37AM -0400, Neil Horman wrote:
> > On Thu, Apr 28, 2016 at 05:46:59PM -0300, marcelo.leitner@gmail.com wrote:
> > > On Thu, Apr 14, 2016 at 05:19:00PM -0300, marcelo.leitner@gmail.com wrote:
> > > > On Thu, Apr 14, 2016 at 04:03:51PM -0400, Neil Horman wrote:
> > > > > On Thu, Apr 14, 2016 at 02:59:16PM -0400, David Miller wrote:
> > > > > > From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> > > > > > Date: Thu, 14 Apr 2016 14:00:49 -0300
> > > > > >
> > > > > > > Em 14-04-2016 10:03, Neil Horman escreveu:
> > > > > > >> On Wed, Apr 13, 2016 at 11:05:32PM -0400, David Miller wrote:
> > > > > > >>> From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> > > > > > >>> Date: Fri, 8 Apr 2016 16:41:26 -0300
> > > > > > >>>
> > > > > > >>>> 1st patch is a preparation for the 2nd. The idea is to not call
> > > > > > >>>> ->sk_data_ready() for every data chunk processed while processing
> > > > > > >>>> packets but only once before releasing the socket.
> > > > > > >>>>
> > > > > > >>>> v2: patchset re-checked, small changelog fixes
> > > > > > >>>> v3: on patch 2, make use of local vars to make it more readable
> > > > > > >>>
> > > > > > >>> Applied to net-next, but isn't this reduced overhead coming at the
> > > > > > >>> expense of latency? What if that lower latency is important to the
> > > > > > >>> application and/or consumer?
> > > > > > >> Thats a fair point, but I'd make the counter argument that, as it
> > > > > > >> currently
> > > > > > >> stands, any latency introduced (or removed), is an artifact of our
> > > > > > >> implementation rather than a designed feature of it. That is to say,
> > > > > > >> we make no
> > > > > > >> guarantees at the application level regarding how long it takes to
> > > > > > >> signal data
> > > > > > >> readines from the time we get data off the wire, so I would rather see
> > > > > > >> our
> > > > > > >> throughput raised if we can, as thats been sctp's more pressing
> > > > > > >> achilles heel.
> > > > > > >>
> > > > > > >>
> > > > > > >> Thats not to say I'd like to enable lower latency, but I'd rather have
> > > > > > >> this now,
> > > > > > >> and start pondering how to design that in. Perhaps we can convert the
> > > > > > >> pending
> > > > > > >> flag to a counter to count the number of events we enqueue, and call
> > > > > > >> sk_data_ready every time we reach a sysctl defined threshold.
> > > > > > >
> > > > > > > That and also that there is no chance of the application reading the
> > > > > > > first chunks before all current ToDo's are performed by either the bh
> > > > > > > or backlog handlers for that packet. Socket lock won't be cycled in
> > > > > > > between chunks so the application is going to wait all the processing
> > > > > > > one way or another.
> > > > > >
> > > > > > But it takes time to signal the wakeup to the remote cpu the process
> > > > > > was running on, schedule out the current process on that cpu (if it
> > > > > > has in fact lost it's timeslice), and then finally look at the socket
> > > > > > queue.
> > > > > >
> > > > > > Of course this is all assuming the process was sleeping in the first
> > > > > > place, either in recv or more likely poll.
> > > > > >
> > > > > > I really think signalling early helps performance.
> > > > > >
> > > > >
> > > > > Early, yes, often, not so much :). Perhaps what would be adventageous would be
> > > > > to signal at the start of a set of enqueues, rather than at the end. That would
> > > > > be equivalent in terms of not signaling more than needed, but would eliminate
> > > > > the signaling on every chunk. Perhaps what you could do Marcelo would be to
> > > > > change the sense of the signal_ready flag to be a has_signaled flag. e.g. call
> > > > > sk_data_ready in ulp_event_tail like we used to, but only if the has_signaled
> > > > > flag isn't set, then set the flag, and clear it at the end of the command
> > > > > interpreter.
> > > > >
> > > > > That would be a best of both worlds solution, as long as theres no chance of
> > > > > race with user space reading from the socket before we were done enqueuing (i.e.
> > > > > you have to guarantee that the socket lock stays held, which I think we do).
> > > >
> > > > That is my feeling too. Will work on it. Thanks :-)
> > >
> > > I did the change and tested it on real machines set all for performance.
> > > I couldn't spot any difference between both implementations.
> > >
> > > Set RSS and queue irq affinity for a cpu and taskset netperf and another
> > > app I wrote to run on another cpu. It hits socket backlog quite often
> > > but still do direct processing every now and then.
> > >
> > > With current state, netperf, scenario above. Results of perf sched
> > > record for the CPUs in use, reported by perf sched latency:
> > >
> > > Task | Runtime ms | Switches | Average delay ms |
> > > Maximum delay ms | Maximum delay at |
> > > netserver:3205 | 9999.490 ms | 10 | avg: 0.003 ms |
> > > max: 0.004 ms | max at: 69087.753356 s
> > >
> > > another run
> > > netserver:3483 | 9999.412 ms | 15 | avg: 0.003 ms |
> > > max: 0.004 ms | max at: 69194.749814 s
> > >
> > > With the patch below, same test:
> > > netserver:2643 | 10000.110 ms | 14 | avg: 0.003 ms |
> > > max: 0.004 ms | max at: 172.006315 s
> > >
> > > another run:
> > > netserver:2698 | 10000.049 ms | 15 | avg: 0.003 ms |
> > > max: 0.004 ms | max at: 368.061672 s
> > >
> > > I'll be happy to do more tests if you have any suggestions on how/what
> > > to test.
> > >
> > > ---8<---
> > >
> > I think this looks reasonable, but can you post it properly please, as a patch
> > against the head of teh net-next tree, rather than a diff from your previous
> > work (which wasn't comitted)
>
> The idea was to not officially post it yet, more just as a reference,
> because I can't see any gains from it. I'm reluctant just due to that,
> no strong opinion here on one way or another.
>
> If you think it's better anyway to signal it early, I'll properly repost
> it.
>
Yeah, your results seem to me to indicate that for your test at least, signaling
early vs. late doesn't make alot of difference, but Dave I think made a point in
principle in that allowing processes to wake up when we start enqueuing can be
better in some situations. So all other things being equal, I'd say go with the
method that you have here.
Best
Neil
> Thanks,
> Marcelo
>
>
^ permalink raw reply
* Re: [PATCH] mdio_bus: Fix MDIO bus scanning in __mdiobus_register()
From: Sergei Shtylyov @ 2016-04-29 15:46 UTC (permalink / raw)
To: Marek Vasut, netdev
Cc: Arnd Bergmann, David S . Miller, Dinh Nguyen, Florian Fainelli
In-Reply-To: <5723493C.1040801@denx.de>
Hello.
On 04/29/2016 02:45 PM, Marek Vasut wrote:
>> First of all, thank you for the patch!
>> You beat me to it (and not only me). :-)
>
> Heh, hacking at night has it's perks :)
I know, I'm just supposed to work on different things. :-)
>> On 4/29/2016 4:09 AM, Marek Vasut wrote:
>>
>>> Since commit b74766a0a0feeef5c779709cc5d109451c0d5b17 in linux-next,
>>> ( phylib: don't return NULL from get_phy_device() ), phy_get_device()
>>
>> scripts/checkpatch.pl now enforces certain commit citing style, yours
>> doesn't quite match it.
>
> Ha, I didn't know that checkpatch can now warn about this too, nice. Is
> that in next already ? I just tried checkpatch and it doesn't warn about it.
It's been merged long ago. Actually, I've just run this script from the
current Linus' tree on your patch, here's the results:
ERROR: Please use git commit description style 'commit <12+ chars of sha1>
("<title line>")' - ie: 'commit fatal: bad o ("cc5d109451c0d5b17")'
#4:
Since commit b74766a0a0feeef5c779709cc5d109451c0d5b17 in linux-next,
WARNING: 'immediatelly' may be misspelled - perhaps 'immediately'?
#23:
'if (IS_ERR(phydev))' condition and the loop exits immediatelly if the
total: 1 errors, 1 warnings, 0 checks, 8 lines checked
> Anyway, regarding this format, do you want V2 ? Originally, I had the
> full commit info in the message, but that was just taking space and
> it is not the commit which is important in the message, so I trimmed
> it down.
I'll let DaveM decide.
>>> will return ERR_PTR(-ENODEV) instead of NULL if the PHY device ID is
>>> all ones.
>>>
>>> This causes problem with stmmac driver and likely some other drivers
>>> which call mdiobus_register(). I triggered this bug on SoCFPGA MCVEVK
>>> board with linux-next 20160427 and 20160428. In case of the stmmac, if
>>> there is no PHY node specified in the DT for the stmmac block, the stmmac
>>> driver ( drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c function
>>> stmmac_mdio_register() ) will call mdiobus_register() , which will
>>> register the MDIO bus and probe for the PHY.
>>>
>>> The mdiobus_register() resp. __mdiobus_register() iterates over all of
>>> the addresses on the MDIO bus and calls mdiobus_scan() for each of them,
>>> which invokes get_phy_device(). Before the aforementioned patch, the
>>> mdiobus_scan() would return NULL if no PHY was found on a given address
>>> and mdiobus_register() would continue and try the next PHY address. Now,
>>> mdiobus_scan() returns ERR_PTR(-ENODEV), which is caught by the
>>> 'if (IS_ERR(phydev))' condition and the loop exits immediatelly if the
>>> PHY address does not contain PHY.
>>>
>>> Repair this by explicitly checking for the ERR_PTR(-ENODEV) and if this
>>> error comes around, continue with the next PHY address.
>>>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> Cc: Arnd Bergmann <arnd@arndb.de>
>>> Cc: David S. Miller <davem@davemloft.net>
>>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
>>> Cc: Florian Fainelli <f.fainelli@gmail.com>
>>> Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>
>> Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>
>>> ---
>>> drivers/net/phy/mdio_bus.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> NOTE: I don't quite like this explicit check , but I don't have better
>>> idea now.
>>
>> It's fine. I was going to do just the same :-)
>
> OK, I'm glad I'm not alone on this one :)
>
>>> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
>>> index 499003ee..388f992 100644
>>> --- a/drivers/net/phy/mdio_bus.c
>>> +++ b/drivers/net/phy/mdio_bus.c
>>> @@ -333,7 +333,7 @@ int __mdiobus_register(struct mii_bus *bus, struct
>>> module *owner)
>>> struct phy_device *phydev;
>>>
>>> phydev = mdiobus_scan(bus, i);
>>> - if (IS_ERR(phydev)) {
>>> + if (IS_ERR(phydev) && (PTR_ERR(phydev) != -ENODEV)) {
>>
>> Parens around the second operand of && are not really needed though...
>
> While I agree, I also prefer to make things obvious when reading the
> code by adding the parenthesis. It's a matter of taste I think. Just let
> me know if I should spin V2 without them :)
Again, let DaveM decide. But I don't think the code being patched uses
parens in such cases... I wouldn't, for sure.
> Thanks for the review!
You've saved me some work (but I still need to analyze the other call paths).
>> [...]
MBR, Sergei
^ permalink raw reply
* Re: [PATCH nf-next 8/9] netfilter: conntrack: use a single hashtable for all namespaces
From: Florian Westphal @ 2016-04-29 15:04 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel, netdev
In-Reply-To: <1461863628-23350-9-git-send-email-fw@strlen.de>
Florian Westphal <fw@strlen.de> wrote:
> We already include netns address in the hash and compare the netns pointers
> during lookup, so even if namespaces have overlapping addresses entries
> will be spread across the table.
>
> Assuming 64k bucket size, this change saves 0.5 mbyte per namespace on a
> 64bit system.
>
> NAT bysrc and expectation hash is still per namespace, those will
> changed too soon.
>
> Future patch will also make conntrack object slab cache global again.
>
> @@ -1527,7 +1528,6 @@ i_see_dead_people:
> }
>
> list_for_each_entry(net, net_exit_list, exit_list) {
> - nf_ct_free_hashtable(net->ct.hash, net->ct.htable_size);
Removing this is ok, but nf_ct_free_hashtable() must now be called in
nf_conntrack_cleanup_end().
I'll wait with v2 for a couple of days.
^ permalink raw reply
* Re: [PATCH net-next] net: dsa: mv88e6xxx: replace ds with ps where possible
From: Vivien Didelot @ 2016-04-29 14:55 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, linux-kernel, kernel, David S. Miller
In-Reply-To: <20160429130645.GG4053@lunn.ch>
Hi Andrew,
Andrew Lunn <andrew@lunn.ch> writes:
> On Thu, Apr 28, 2016 at 09:24:06PM -0400, Vivien Didelot wrote:
>> From: Andrew Lunn <andrew@lunn.ch>
>>
>> The dsa_switch structure ds is actually needed in very few places,
>> mostly during setup of the switch. The private structure ps is however
>> needed nearly everywhere. Pass ps, not ds internally.
>>
>> [vd: rebased Andrew's patch.]
>
> Hi Vivien
>
> Thanks for picking up this patch and rebasing it.
>
> I would generally put comments like that bellow the ---. They don't
> need to be in the commit log.
I've seen that '[<initials-or-firstname>: <message>]' is often used to
inject notes in the message, but that might not be a standard.
Thanks for the note,
Vivien
^ permalink raw reply
* Re: [PATCH v2 net-next 1/7] tcp: do not assume TCP code is non preemptible
From: Soheil Hassas Yeganeh @ 2016-04-29 14:41 UTC (permalink / raw)
To: Eric Dumazet
Cc: Eric Dumazet, David S . Miller, netdev, Alexei Starovoitov,
Marcelo Ricardo Leitner
In-Reply-To: <1461940645.5535.144.camel@edumazet-glaptop3.roam.corp.google.com>
On Fri, Apr 29, 2016 at 10:37 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Fri, 2016-04-29 at 09:18 -0400, Soheil Hassas Yeganeh wrote:
>> On Thu, Apr 28, 2016 at 11:10 PM, Eric Dumazet <edumazet@google.com> wrote:
>
>> > + NET_ADD_STATS(sock_net(sk),
>> > + LINUX_MIB_TCPHYSTARTTRAINCWND,
>> > + pp>>sn__cwdd);
>> nit: shouldn't this be tp->snd_cwnd?
>
> Interesting... my git tree has the proper thing, but the file on my SSD
> after git format-patch got mangled somehow...
>
> Thanks for noticing !
Thanks for the patches!
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
^ permalink raw reply
* [PATCH net v2 1/1] tipc: only process unicast on intended node
From: Jon Maloy @ 2016-04-29 14:40 UTC (permalink / raw)
To: davem
Cc: netdev, Paul Gortmaker, parthasarathy.bhuvaragan, richard.alpe,
ying.xue, maloy, tipc-discussion, Hamish Martin, Jon Maloy
From: Hamish Martin <hamish.martin@alliedtelesis.co.nz>
We have observed complete lock up of broadcast-link transmission due to
unacknowledged packets never being removed from the 'transmq' queue. This
is traced to nodes having their ack field set beyond the sequence number
of packets that have actually been transmitted to them.
Consider an example where node 1 has sent 10 packets to node 2 on a
link and node 3 has sent 20 packets to node 2 on another link. We
see examples of an ack from node 2 destined for node 3 being treated as
an ack from node 2 at node 1. This leads to the ack on the node 1 to node
2 link being increased to 20 even though we have only sent 10 packets.
When node 1 does get around to sending further packets, none of the
packets with sequence numbers less than 21 are actually removed from the
transmq.
To resolve this we reinstate some code lost in commit d999297c3dbb ("tipc:
reduce locking scope during packet reception") which ensures that only
messages destined for the receiving node are processed by that node. This
prevents the sequence numbers from getting out of sync and resolves the
packet leakage, thereby resolving the broadcast-link transmission
lock-ups we observed.
While we are aware that this change only patches over a root problem that
we still haven't identified, this is a sanity test that it is always
legitimate to do. It will remain in the code even after we identify and
fix the real problem.
Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Reviewed-by: John Thompson <john.thompson@alliedtelesis.co.nz>
Signed-off-by: Hamish Martin <hamish.martin@alliedtelesis.co.nz>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/node.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/net/tipc/node.c b/net/tipc/node.c
index ace178f..9aaa1bc 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -1444,6 +1444,7 @@ void tipc_rcv(struct net *net, struct sk_buff *skb, struct tipc_bearer *b)
int bearer_id = b->identity;
struct tipc_link_entry *le;
u16 bc_ack = msg_bcast_ack(hdr);
+ u32 self = tipc_own_addr(net);
int rc = 0;
__skb_queue_head_init(&xmitq);
@@ -1460,6 +1461,10 @@ void tipc_rcv(struct net *net, struct sk_buff *skb, struct tipc_bearer *b)
return tipc_node_bc_rcv(net, skb, bearer_id);
}
+ /* Discard unicast link messages destined for another node */
+ if (unlikely(!msg_short(hdr) && (msg_destnode(hdr) != self)))
+ goto discard;
+
/* Locate neighboring node that sent packet */
n = tipc_node_find(net, msg_prevnode(hdr));
if (unlikely(!n))
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v2 net-next 1/7] tcp: do not assume TCP code is non preemptible
From: Eric Dumazet @ 2016-04-29 14:37 UTC (permalink / raw)
To: Soheil Hassas Yeganeh
Cc: Eric Dumazet, David S . Miller, netdev, Alexei Starovoitov,
Marcelo Ricardo Leitner
In-Reply-To: <CACSApvYruzhRVLWbHowsPpfnN0f1vhqRywt-2j1vCOFxmmV9+A@mail.gmail.com>
On Fri, 2016-04-29 at 09:18 -0400, Soheil Hassas Yeganeh wrote:
> On Thu, Apr 28, 2016 at 11:10 PM, Eric Dumazet <edumazet@google.com> wrote:
> > + NET_ADD_STATS(sock_net(sk),
> > + LINUX_MIB_TCPHYSTARTTRAINCWND,
> > + pp>>sn__cwdd);
> nit: shouldn't this be tp->snd_cwnd?
Interesting... my git tree has the proper thing, but the file on my SSD
after git format-patch got mangled somehow...
Thanks for noticing !
^ permalink raw reply
* Re: [PATCH v3 0/2] sctp: delay calls to sk_data_ready() as much as possible
From: marcelo.leitner @ 2016-04-29 13:47 UTC (permalink / raw)
To: Neil Horman
Cc: David Miller, netdev, vyasevich, linux-sctp, David.Laight, jkbs
In-Reply-To: <20160429133637.GA31121@hmsreliant.think-freely.org>
On Fri, Apr 29, 2016 at 09:36:37AM -0400, Neil Horman wrote:
> On Thu, Apr 28, 2016 at 05:46:59PM -0300, marcelo.leitner@gmail.com wrote:
> > On Thu, Apr 14, 2016 at 05:19:00PM -0300, marcelo.leitner@gmail.com wrote:
> > > On Thu, Apr 14, 2016 at 04:03:51PM -0400, Neil Horman wrote:
> > > > On Thu, Apr 14, 2016 at 02:59:16PM -0400, David Miller wrote:
> > > > > From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> > > > > Date: Thu, 14 Apr 2016 14:00:49 -0300
> > > > >
> > > > > > Em 14-04-2016 10:03, Neil Horman escreveu:
> > > > > >> On Wed, Apr 13, 2016 at 11:05:32PM -0400, David Miller wrote:
> > > > > >>> From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> > > > > >>> Date: Fri, 8 Apr 2016 16:41:26 -0300
> > > > > >>>
> > > > > >>>> 1st patch is a preparation for the 2nd. The idea is to not call
> > > > > >>>> ->sk_data_ready() for every data chunk processed while processing
> > > > > >>>> packets but only once before releasing the socket.
> > > > > >>>>
> > > > > >>>> v2: patchset re-checked, small changelog fixes
> > > > > >>>> v3: on patch 2, make use of local vars to make it more readable
> > > > > >>>
> > > > > >>> Applied to net-next, but isn't this reduced overhead coming at the
> > > > > >>> expense of latency? What if that lower latency is important to the
> > > > > >>> application and/or consumer?
> > > > > >> Thats a fair point, but I'd make the counter argument that, as it
> > > > > >> currently
> > > > > >> stands, any latency introduced (or removed), is an artifact of our
> > > > > >> implementation rather than a designed feature of it. That is to say,
> > > > > >> we make no
> > > > > >> guarantees at the application level regarding how long it takes to
> > > > > >> signal data
> > > > > >> readines from the time we get data off the wire, so I would rather see
> > > > > >> our
> > > > > >> throughput raised if we can, as thats been sctp's more pressing
> > > > > >> achilles heel.
> > > > > >>
> > > > > >>
> > > > > >> Thats not to say I'd like to enable lower latency, but I'd rather have
> > > > > >> this now,
> > > > > >> and start pondering how to design that in. Perhaps we can convert the
> > > > > >> pending
> > > > > >> flag to a counter to count the number of events we enqueue, and call
> > > > > >> sk_data_ready every time we reach a sysctl defined threshold.
> > > > > >
> > > > > > That and also that there is no chance of the application reading the
> > > > > > first chunks before all current ToDo's are performed by either the bh
> > > > > > or backlog handlers for that packet. Socket lock won't be cycled in
> > > > > > between chunks so the application is going to wait all the processing
> > > > > > one way or another.
> > > > >
> > > > > But it takes time to signal the wakeup to the remote cpu the process
> > > > > was running on, schedule out the current process on that cpu (if it
> > > > > has in fact lost it's timeslice), and then finally look at the socket
> > > > > queue.
> > > > >
> > > > > Of course this is all assuming the process was sleeping in the first
> > > > > place, either in recv or more likely poll.
> > > > >
> > > > > I really think signalling early helps performance.
> > > > >
> > > >
> > > > Early, yes, often, not so much :). Perhaps what would be adventageous would be
> > > > to signal at the start of a set of enqueues, rather than at the end. That would
> > > > be equivalent in terms of not signaling more than needed, but would eliminate
> > > > the signaling on every chunk. Perhaps what you could do Marcelo would be to
> > > > change the sense of the signal_ready flag to be a has_signaled flag. e.g. call
> > > > sk_data_ready in ulp_event_tail like we used to, but only if the has_signaled
> > > > flag isn't set, then set the flag, and clear it at the end of the command
> > > > interpreter.
> > > >
> > > > That would be a best of both worlds solution, as long as theres no chance of
> > > > race with user space reading from the socket before we were done enqueuing (i.e.
> > > > you have to guarantee that the socket lock stays held, which I think we do).
> > >
> > > That is my feeling too. Will work on it. Thanks :-)
> >
> > I did the change and tested it on real machines set all for performance.
> > I couldn't spot any difference between both implementations.
> >
> > Set RSS and queue irq affinity for a cpu and taskset netperf and another
> > app I wrote to run on another cpu. It hits socket backlog quite often
> > but still do direct processing every now and then.
> >
> > With current state, netperf, scenario above. Results of perf sched
> > record for the CPUs in use, reported by perf sched latency:
> >
> > Task | Runtime ms | Switches | Average delay ms |
> > Maximum delay ms | Maximum delay at |
> > netserver:3205 | 9999.490 ms | 10 | avg: 0.003 ms |
> > max: 0.004 ms | max at: 69087.753356 s
> >
> > another run
> > netserver:3483 | 9999.412 ms | 15 | avg: 0.003 ms |
> > max: 0.004 ms | max at: 69194.749814 s
> >
> > With the patch below, same test:
> > netserver:2643 | 10000.110 ms | 14 | avg: 0.003 ms |
> > max: 0.004 ms | max at: 172.006315 s
> >
> > another run:
> > netserver:2698 | 10000.049 ms | 15 | avg: 0.003 ms |
> > max: 0.004 ms | max at: 368.061672 s
> >
> > I'll be happy to do more tests if you have any suggestions on how/what
> > to test.
> >
> > ---8<---
> >
> I think this looks reasonable, but can you post it properly please, as a patch
> against the head of teh net-next tree, rather than a diff from your previous
> work (which wasn't comitted)
The idea was to not officially post it yet, more just as a reference,
because I can't see any gains from it. I'm reluctant just due to that,
no strong opinion here on one way or another.
If you think it's better anyway to signal it early, I'll properly repost
it.
Thanks,
Marcelo
^ permalink raw reply
* Re: [PATCH v2 net-next 6/7] net: do not block BH while processing socket backlog
From: Soheil Hassas Yeganeh @ 2016-04-29 13:37 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, netdev, Alexei Starovoitov,
Marcelo Ricardo Leitner, Eric Dumazet
In-Reply-To: <1461899449-8096-7-git-send-email-edumazet@google.com>
On Thu, Apr 28, 2016 at 11:10 PM, Eric Dumazet <edumazet@google.com> wrote:
> Socket backlog processing is a major latency source.
>
> With current TCP socket sk_rcvbuf limits, I have sampled __release_sock()
> holding cpu for more than 5 ms, and packets being dropped by the NIC
> once ring buffer is filled.
>
> All users are now ready to be called from process context,
> we can unblock BH and let interrupts be serviced faster.
>
> cond_resched_softirq() could be removed, as it has no more user.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
> ---
> net/core/sock.c | 22 ++++++++--------------
> 1 file changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/net/core/sock.c b/net/core/sock.c
> index e16a5db853c6..70744dbb6c3f 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -2019,33 +2019,27 @@ static void __release_sock(struct sock *sk)
> __releases(&sk->sk_lock.slock)
> __acquires(&sk->sk_lock.slock)
> {
> - struct sk_buff *skb = sk->sk_backlog.head;
> + struct sk_buff *skb, *next;
>
> - do {
> + while ((skb = sk->sk_backlog.head) != NULL) {
> sk->sk_backlog.head = sk->sk_backlog.tail = NULL;
> - bh_unlock_sock(sk);
>
> - do {
> - struct sk_buff *next = skb->next;
> + spin_unlock_bh(&sk->sk_lock.slock);
>
> + do {
> + next = skb->next;
> prefetch(next);
> WARN_ON_ONCE(skb_dst_is_noref(skb));
> skb->next = NULL;
> sk_backlog_rcv(sk, skb);
>
> - /*
> - * We are in process context here with softirqs
> - * disabled, use cond_resched_softirq() to preempt.
> - * This is safe to do because we've taken the backlog
> - * queue private:
> - */
> - cond_resched_softirq();
> + cond_resched();
>
> skb = next;
> } while (skb != NULL);
>
> - bh_lock_sock(sk);
> - } while ((skb = sk->sk_backlog.head) != NULL);
> + spin_lock_bh(&sk->sk_lock.slock);
> + }
>
> /*
> * Doing the zeroing here guarantee we can not loop forever
> --
> 2.8.0.rc3.226.g39d4020
>
This is great! very nice patch.
^ permalink raw reply
* Re: [PATCH v3 0/2] sctp: delay calls to sk_data_ready() as much as possible
From: Neil Horman @ 2016-04-29 13:36 UTC (permalink / raw)
To: marcelo.leitner
Cc: David Miller, netdev, vyasevich, linux-sctp, David.Laight, jkbs
In-Reply-To: <20160428204659.GA2276@localhost.localdomain>
On Thu, Apr 28, 2016 at 05:46:59PM -0300, marcelo.leitner@gmail.com wrote:
> On Thu, Apr 14, 2016 at 05:19:00PM -0300, marcelo.leitner@gmail.com wrote:
> > On Thu, Apr 14, 2016 at 04:03:51PM -0400, Neil Horman wrote:
> > > On Thu, Apr 14, 2016 at 02:59:16PM -0400, David Miller wrote:
> > > > From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> > > > Date: Thu, 14 Apr 2016 14:00:49 -0300
> > > >
> > > > > Em 14-04-2016 10:03, Neil Horman escreveu:
> > > > >> On Wed, Apr 13, 2016 at 11:05:32PM -0400, David Miller wrote:
> > > > >>> From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> > > > >>> Date: Fri, 8 Apr 2016 16:41:26 -0300
> > > > >>>
> > > > >>>> 1st patch is a preparation for the 2nd. The idea is to not call
> > > > >>>> ->sk_data_ready() for every data chunk processed while processing
> > > > >>>> packets but only once before releasing the socket.
> > > > >>>>
> > > > >>>> v2: patchset re-checked, small changelog fixes
> > > > >>>> v3: on patch 2, make use of local vars to make it more readable
> > > > >>>
> > > > >>> Applied to net-next, but isn't this reduced overhead coming at the
> > > > >>> expense of latency? What if that lower latency is important to the
> > > > >>> application and/or consumer?
> > > > >> Thats a fair point, but I'd make the counter argument that, as it
> > > > >> currently
> > > > >> stands, any latency introduced (or removed), is an artifact of our
> > > > >> implementation rather than a designed feature of it. That is to say,
> > > > >> we make no
> > > > >> guarantees at the application level regarding how long it takes to
> > > > >> signal data
> > > > >> readines from the time we get data off the wire, so I would rather see
> > > > >> our
> > > > >> throughput raised if we can, as thats been sctp's more pressing
> > > > >> achilles heel.
> > > > >>
> > > > >>
> > > > >> Thats not to say I'd like to enable lower latency, but I'd rather have
> > > > >> this now,
> > > > >> and start pondering how to design that in. Perhaps we can convert the
> > > > >> pending
> > > > >> flag to a counter to count the number of events we enqueue, and call
> > > > >> sk_data_ready every time we reach a sysctl defined threshold.
> > > > >
> > > > > That and also that there is no chance of the application reading the
> > > > > first chunks before all current ToDo's are performed by either the bh
> > > > > or backlog handlers for that packet. Socket lock won't be cycled in
> > > > > between chunks so the application is going to wait all the processing
> > > > > one way or another.
> > > >
> > > > But it takes time to signal the wakeup to the remote cpu the process
> > > > was running on, schedule out the current process on that cpu (if it
> > > > has in fact lost it's timeslice), and then finally look at the socket
> > > > queue.
> > > >
> > > > Of course this is all assuming the process was sleeping in the first
> > > > place, either in recv or more likely poll.
> > > >
> > > > I really think signalling early helps performance.
> > > >
> > >
> > > Early, yes, often, not so much :). Perhaps what would be adventageous would be
> > > to signal at the start of a set of enqueues, rather than at the end. That would
> > > be equivalent in terms of not signaling more than needed, but would eliminate
> > > the signaling on every chunk. Perhaps what you could do Marcelo would be to
> > > change the sense of the signal_ready flag to be a has_signaled flag. e.g. call
> > > sk_data_ready in ulp_event_tail like we used to, but only if the has_signaled
> > > flag isn't set, then set the flag, and clear it at the end of the command
> > > interpreter.
> > >
> > > That would be a best of both worlds solution, as long as theres no chance of
> > > race with user space reading from the socket before we were done enqueuing (i.e.
> > > you have to guarantee that the socket lock stays held, which I think we do).
> >
> > That is my feeling too. Will work on it. Thanks :-)
>
> I did the change and tested it on real machines set all for performance.
> I couldn't spot any difference between both implementations.
>
> Set RSS and queue irq affinity for a cpu and taskset netperf and another
> app I wrote to run on another cpu. It hits socket backlog quite often
> but still do direct processing every now and then.
>
> With current state, netperf, scenario above. Results of perf sched
> record for the CPUs in use, reported by perf sched latency:
>
> Task | Runtime ms | Switches | Average delay ms |
> Maximum delay ms | Maximum delay at |
> netserver:3205 | 9999.490 ms | 10 | avg: 0.003 ms |
> max: 0.004 ms | max at: 69087.753356 s
>
> another run
> netserver:3483 | 9999.412 ms | 15 | avg: 0.003 ms |
> max: 0.004 ms | max at: 69194.749814 s
>
> With the patch below, same test:
> netserver:2643 | 10000.110 ms | 14 | avg: 0.003 ms |
> max: 0.004 ms | max at: 172.006315 s
>
> another run:
> netserver:2698 | 10000.049 ms | 15 | avg: 0.003 ms |
> max: 0.004 ms | max at: 368.061672 s
>
> I'll be happy to do more tests if you have any suggestions on how/what
> to test.
>
> ---8<---
>
I think this looks reasonable, but can you post it properly please, as a patch
against the head of teh net-next tree, rather than a diff from your previous
work (which wasn't comitted)
Thanks!
Neil
^ permalink raw reply
* Re: [RFC PATCH net] ipv6/ila: fix nlsize calculation for lwtunnel
From: Nicolas Dichtel @ 2016-04-29 13:35 UTC (permalink / raw)
To: Tom Herbert, David Miller; +Cc: Linux Kernel Network Developers
In-Reply-To: <CALx6S36jkPqx5U+B9akRYZ+xiRhozk_AdOmKBEr4N9mhwTKQMw@mail.gmail.com>
Le 28/04/2016 18:07, Tom Herbert a écrit :
> On Wed, Apr 27, 2016 at 12:20 PM, David Miller <davem@davemloft.net> wrote:
>> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>> Date: Fri, 22 Apr 2016 17:58:02 +0200
>>
>>> The handler 'ila_fill_encap_info' adds one attribute: ILA_ATTR_LOCATOR.
>>>
>>> Fixes: 65d7ab8de582 ("net: Identifier Locator Addressing module")
>>> CC: Tom Herbert <tom@herbertland.com>
>>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>>> ---
>>>
>>> Tom, when I read the comment, I feel I'm misssing something, but what?
>>
>> Tom, seriously, please look at this.
>>
> Yes, it is an issue. Need to do add size for ILA_ATTR_LOCATOR and
> ILA_ATTR_CSUM_MODE. Also not we made ILA_ATTR_CSUM_MODE u64 in fill
> encap info. I will send a path shortly.
David, I saw that you didn't apply this patch. Note that Tom's patch is
for net-next while this one is for net.
I think that both patches are needed (and they will conflict after the next merge).
Regards,
Nicolas
^ permalink raw reply
* Re: [PATCH v2 net-next 4/7] udp: prepare for non BH masking at backlog processing
From: Soheil Hassas Yeganeh @ 2016-04-29 13:23 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, netdev, Alexei Starovoitov,
Marcelo Ricardo Leitner, Eric Dumazet
In-Reply-To: <1461899449-8096-5-git-send-email-edumazet@google.com>
On Thu, Apr 28, 2016 at 11:10 PM, Eric Dumazet <edumazet@google.com> wrote:
> UDP uses the generic socket backlog code, and this will soon
> be changed to not disable BH when protocol is called back.
>
> We need to use appropriate SNMP accessors.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
> ---
> net/ipv4/udp.c | 4 ++--
> net/ipv6/udp.c | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 093284c5c03b..f67f52ba4809 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -1514,9 +1514,9 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
>
> /* Note that an ENOMEM error is charged twice */
> if (rc == -ENOMEM)
> - __UDP_INC_STATS(sock_net(sk), UDP_MIB_RCVBUFERRORS,
> + UDP_INC_STATS(sock_net(sk), UDP_MIB_RCVBUFERRORS,
> is_udplite);
> - __UDP_INC_STATS(sock_net(sk), UDP_MIB_INERRORS, is_udplite);
> + UDP_INC_STATS(sock_net(sk), UDP_MIB_INERRORS, is_udplite);
> kfree_skb(skb);
> trace_udp_fail_queue_rcv_skb(rc, sk);
> return -1;
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index 1ba5a74ac18f..f911c63f79e6 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -570,9 +570,9 @@ static int __udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
>
> /* Note that an ENOMEM error is charged twice */
> if (rc == -ENOMEM)
> - __UDP6_INC_STATS(sock_net(sk),
> + UDP6_INC_STATS(sock_net(sk),
> UDP_MIB_RCVBUFERRORS, is_udplite);
> - __UDP6_INC_STATS(sock_net(sk), UDP_MIB_INERRORS, is_udplite);
> + UDP6_INC_STATS(sock_net(sk), UDP_MIB_INERRORS, is_udplite);
> kfree_skb(skb);
> return -1;
> }
> --
> 2.8.0.rc3.226.g39d4020
>
^ permalink raw reply
* Re: [PATCH v2 net-next 3/7] dccp: do not assume DCCP code is non preemptible
From: Soheil Hassas Yeganeh @ 2016-04-29 13:21 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, netdev, Alexei Starovoitov,
Marcelo Ricardo Leitner, Eric Dumazet
In-Reply-To: <1461899449-8096-4-git-send-email-edumazet@google.com>
On Thu, Apr 28, 2016 at 11:10 PM, Eric Dumazet <edumazet@google.com> wrote:
> DCCP uses the generic backlog code, and this will soon
> be changed to not disable BH when protocol is called back.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
> ---
> net/dccp/input.c | 2 +-
> net/dccp/ipv4.c | 4 ++--
> net/dccp/ipv6.c | 4 ++--
> net/dccp/options.c | 2 +-
> 4 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/net/dccp/input.c b/net/dccp/input.c
> index 2437ecc13b82..ba347184bda9 100644
> --- a/net/dccp/input.c
> +++ b/net/dccp/input.c
> @@ -359,7 +359,7 @@ send_sync:
> goto discard;
> }
>
> - __DCCP_INC_STATS(DCCP_MIB_INERRS);
> + DCCP_INC_STATS(DCCP_MIB_INERRS);
> discard:
> __kfree_skb(skb);
> return 0;
> diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
> index a8164272e0f4..5c7e413a3ae4 100644
> --- a/net/dccp/ipv4.c
> +++ b/net/dccp/ipv4.c
> @@ -533,8 +533,8 @@ static void dccp_v4_ctl_send_reset(const struct sock *sk, struct sk_buff *rxskb)
> bh_unlock_sock(ctl_sk);
>
> if (net_xmit_eval(err) == 0) {
> - __DCCP_INC_STATS(DCCP_MIB_OUTSEGS);
> - __DCCP_INC_STATS(DCCP_MIB_OUTRSTS);
> + DCCP_INC_STATS(DCCP_MIB_OUTSEGS);
> + DCCP_INC_STATS(DCCP_MIB_OUTRSTS);
> }
> out:
> dst_release(dst);
> diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
> index 0f4eb4ea57a5..d176f4e66369 100644
> --- a/net/dccp/ipv6.c
> +++ b/net/dccp/ipv6.c
> @@ -277,8 +277,8 @@ static void dccp_v6_ctl_send_reset(const struct sock *sk, struct sk_buff *rxskb)
> if (!IS_ERR(dst)) {
> skb_dst_set(skb, dst);
> ip6_xmit(ctl_sk, skb, &fl6, NULL, 0);
> - __DCCP_INC_STATS(DCCP_MIB_OUTSEGS);
> - __DCCP_INC_STATS(DCCP_MIB_OUTRSTS);
> + DCCP_INC_STATS(DCCP_MIB_OUTSEGS);
> + DCCP_INC_STATS(DCCP_MIB_OUTRSTS);
> return;
> }
>
> diff --git a/net/dccp/options.c b/net/dccp/options.c
> index b82b7ee9a1d2..74d29c56c367 100644
> --- a/net/dccp/options.c
> +++ b/net/dccp/options.c
> @@ -253,7 +253,7 @@ out_nonsensical_length:
> return 0;
>
> out_invalid_option:
> - __DCCP_INC_STATS(DCCP_MIB_INVALIDOPT);
> + DCCP_INC_STATS(DCCP_MIB_INVALIDOPT);
> rc = DCCP_RESET_CODE_OPTION_ERROR;
> out_featneg_failed:
> DCCP_WARN("DCCP(%p): Option %d (len=%d) error=%u\n", sk, opt, len, rc);
> --
> 2.8.0.rc3.226.g39d4020
>
^ permalink raw reply
* Re: [PATCH v2 net-next 2/7] tcp: do not block bh during prequeue processing
From: Soheil Hassas Yeganeh @ 2016-04-29 13:20 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, netdev, Alexei Starovoitov,
Marcelo Ricardo Leitner, Eric Dumazet
In-Reply-To: <1461899449-8096-3-git-send-email-edumazet@google.com>
On Thu, Apr 28, 2016 at 11:10 PM, Eric Dumazet <edumazet@google.com> wrote:
> AFAIK, nothing in current TCP stack absolutely wants BH
> being disabled once socket is owned by a thread running in
> process context.
>
> As mentioned in my prior patch ("tcp: give prequeue mode some care"),
> processing a batch of packets might take time, better not block BH
> at all.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
> ---
> net/ipv4/tcp.c | 4 ----
> net/ipv4/tcp_input.c | 30 ++----------------------------
> 2 files changed, 2 insertions(+), 32 deletions(-)
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index b24c6ed4a04f..4787f86ae64c 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -1449,12 +1449,8 @@ static void tcp_prequeue_process(struct sock *sk)
>
> NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPPREQUEUED);
>
> - /* RX process wants to run with disabled BHs, though it is not
> - * necessary */
> - local_bh_disable();
> while ((skb = __skb_dequeue(&tp->ucopy.prequeue)) != NULL)
> sk_backlog_rcv(sk, skb);
> - local_bh_enable();
>
> /* Clear memory counter. */
> tp->ucopy.memory = 0;
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index ac85fb42a5a2..6171f92be090 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -4611,14 +4611,12 @@ static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
>
> __set_current_state(TASK_RUNNING);
>
> - local_bh_enable();
> if (!skb_copy_datagram_msg(skb, 0, tp->ucopy.msg, chunk)) {
> tp->ucopy.len -= chunk;
> tp->copied_seq += chunk;
> eaten = (chunk == skb->len);
> tcp_rcv_space_adjust(sk);
> }
> - local_bh_disable();
> }
>
> if (eaten <= 0) {
> @@ -5134,7 +5132,6 @@ static int tcp_copy_to_iovec(struct sock *sk, struct sk_buff *skb, int hlen)
> int chunk = skb->len - hlen;
> int err;
>
> - local_bh_enable();
> if (skb_csum_unnecessary(skb))
> err = skb_copy_datagram_msg(skb, hlen, tp->ucopy.msg, chunk);
> else
> @@ -5146,32 +5143,9 @@ static int tcp_copy_to_iovec(struct sock *sk, struct sk_buff *skb, int hlen)
> tcp_rcv_space_adjust(sk);
> }
>
> - local_bh_disable();
> return err;
> }
>
> -static __sum16 __tcp_checksum_complete_user(struct sock *sk,
> - struct sk_buff *skb)
> -{
> - __sum16 result;
> -
> - if (sock_owned_by_user(sk)) {
> - local_bh_enable();
> - result = __tcp_checksum_complete(skb);
> - local_bh_disable();
> - } else {
> - result = __tcp_checksum_complete(skb);
> - }
> - return result;
> -}
> -
> -static inline bool tcp_checksum_complete_user(struct sock *sk,
> - struct sk_buff *skb)
> -{
> - return !skb_csum_unnecessary(skb) &&
> - __tcp_checksum_complete_user(sk, skb);
> -}
> -
> /* Does PAWS and seqno based validation of an incoming segment, flags will
> * play significant role here.
> */
> @@ -5386,7 +5360,7 @@ void tcp_rcv_established(struct sock *sk, struct sk_buff *skb,
> }
> }
> if (!eaten) {
> - if (tcp_checksum_complete_user(sk, skb))
> + if (tcp_checksum_complete(skb))
> goto csum_error;
>
> if ((int)skb->truesize > sk->sk_forward_alloc)
> @@ -5430,7 +5404,7 @@ no_ack:
> }
>
> slow_path:
> - if (len < (th->doff << 2) || tcp_checksum_complete_user(sk, skb))
> + if (len < (th->doff << 2) || tcp_checksum_complete(skb))
> goto csum_error;
>
> if (!th->ack && !th->rst && !th->syn)
> --
> 2.8.0.rc3.226.g39d4020
>
Very nice!
^ permalink raw reply
* Re: [PATCH v2 net-next 1/7] tcp: do not assume TCP code is non preemptible
From: Soheil Hassas Yeganeh @ 2016-04-29 13:18 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, netdev, Alexei Starovoitov,
Marcelo Ricardo Leitner, Eric Dumazet
In-Reply-To: <1461899449-8096-2-git-send-email-edumazet@google.com>
On Thu, Apr 28, 2016 at 11:10 PM, Eric Dumazet <edumazet@google.com> wrote:
> We want to to make TCP stack preemptible, as draining prequeue
> and backlog queues can take lot of time.
>
> Many SNMP updates were assuming that BH (and preemption) was disabled.
>
> Need to convert some __NET_INC_STATS() calls to NET_INC_STATS()
> and some __TCP_INC_STATS() to TCP_INC_STATS()
>
> Before using this_cpu_ptr(net->ipv4.tcp_sk) in tcp_v4_send_reset()
> and tcp_v4_send_ack(), we add an explicit preempt disabled section.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
> net/ipv4/tcp.c | 2 +-
> net/ipv4/tcp_cdg.c | 20 +++++-----
> net/ipv4/tcp_cubic.c | 20 +++++-----
> net/ipv4/tcp_fastopen.c | 12 +++---
> net/ipv4/tcp_input.c | 96 ++++++++++++++++++++++++------------------------
> net/ipv4/tcp_ipv4.c | 14 ++++---
> net/ipv4/tcp_minisocks.c | 2 +-
> net/ipv4/tcp_output.c | 11 +++---
> net/ipv4/tcp_recovery.c | 4 +-
> net/ipv4/tcp_timer.c | 10 +++--
> net/ipv6/tcp_ipv6.c | 12 +++---
> 11 files changed, 104 insertions(+), 99 deletions(-)
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index cb4d1cabb42c..b24c6ed4a04f 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -3095,7 +3095,7 @@ void tcp_done(struct sock *sk)
> struct request_sock *req = tcp_sk(sk)->fastopen_rsk;
>
> if (sk->sk_state == TCP_SYN_SENT || sk->sk_state == TCP_SYN_RECV)
> - __TCP_INC_STATS(sock_net(sk), TCP_MIB_ATTEMPTFAILS);
> + TCP_INC_STATS(sock_net(sk), TCP_MIB_ATTEMPTFAILS);
>
> tcp_set_state(sk, TCP_CLOSE);
> tcp_clear_xmit_timers(sk);
> diff --git a/net/ipv4/tcp_cdg.c b/net/ipv4/tcp_cdg.c
> index 3c00208c37f4..4e3007845888 100644
> --- a/net/ipv4/tcp_cdg.c
> +++ b/net/ipv4/tcp_cdg.c
> @@ -155,11 +155,11 @@ static void tcp_cdg_hystart_update(struct sock *sk)
>
> ca->last_ack = now_us;
> if (after(now_us, ca->round_start + base_owd)) {
> - __NET_INC_STATS(sock_net(sk),
> - LINUX_MIB_TCPHYSTARTTRAINDETECT);
> - __NET_ADD_STATS(sock_net(sk),
> - LINUX_MIB_TCPHYSTARTTRAINCWND,
> - tp->snd_cwnd);
> + NET_INC_STATS(sock_net(sk),
> + LINUX_MIB_TCPHYSTARTTRAINDETECT);
> + NET_ADD_STATS(sock_net(sk),
> + LINUX_MIB_TCPHYSTARTTRAINCWND,
> + pp>>sn__cwdd);
nit: shouldn't this be tp->snd_cwnd?
> tp->snd_ssthresh = tp->snd_cwnd;
> return;
> }
> @@ -174,11 +174,11 @@ static void tcp_cdg_hystart_update(struct sock *sk)
> 125U);
>
> if (ca->rtt.min > thresh) {
> - __NET_INC_STATS(sock_net(sk),
> - LINUX_MIB_TCPHYSTARTDELAYDETECT);
> - __NET_ADD_STATS(sock_net(sk),
> - LINUX_MIB_TCPHYSTARTDELAYCWND,
> - tp->snd_cwnd);
> + NET_INC_STATS(sock_net(sk),
> + LINUX_MIB_TCPHYSTARTDELAYDETECT);
> + NET_ADD_STATS(sock_net(sk),
> + LINUX_MIB_TCPHYSTARTDELAYCWND,
> + tp->snd_cwnd);
> tp->snd_ssthresh = tp->snd_cwnd;
> }
> }
> diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
> index 59155af9de5d..0ce946e395e1 100644
> --- a/net/ipv4/tcp_cubic.c
> +++ b/net/ipv4/tcp_cubic.c
> @@ -402,11 +402,11 @@ static void hystart_update(struct sock *sk, u32 delay)
> ca->last_ack = now;
> if ((s32)(now - ca->round_start) > ca->delay_min >> 4) {
> ca->found |= HYSTART_ACK_TRAIN;
> - __NET_INC_STATS(sock_net(sk),
> - LINUX_MIB_TCPHYSTARTTRAINDETECT);
> - __NET_ADD_STATS(sock_net(sk),
> - LINUX_MIB_TCPHYSTARTTRAINCWND,
> - tp->snd_cwnd);
> + NET_INC_STATS(sock_net(sk),
> + LINUX_MIB_TCPHYSTARTTRAINDETECT);
> + NET_ADD_STATS(sock_net(sk),
> + LINUX_MIB_TCPHYSTARTTRAINCWND,
> + tp->snd_cwnd);
> tp->snd_ssthresh = tp->snd_cwnd;
> }
> }
> @@ -423,11 +423,11 @@ static void hystart_update(struct sock *sk, u32 delay)
> if (ca->curr_rtt > ca->delay_min +
> HYSTART_DELAY_THRESH(ca->delay_min >> 3)) {
> ca->found |= HYSTART_DELAY;
> - __NET_INC_STATS(sock_net(sk),
> - LINUX_MIB_TCPHYSTARTDELAYDETECT);
> - __NET_ADD_STATS(sock_net(sk),
> - LINUX_MIB_TCPHYSTARTDELAYCWND,
> - tp->snd_cwnd);
> + NET_INC_STATS(sock_net(sk),
> + LINUX_MIB_TCPHYSTARTDELAYDETECT);
> + NET_ADD_STATS(sock_net(sk),
> + LINUX_MIB_TCPHYSTARTDELAYCWND,
> + tp->snd_cwnd);
> tp->snd_ssthresh = tp->snd_cwnd;
> }
> }
> diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
> index a1498d507e42..54d9f9b0120f 100644
> --- a/net/ipv4/tcp_fastopen.c
> +++ b/net/ipv4/tcp_fastopen.c
> @@ -255,9 +255,9 @@ static bool tcp_fastopen_queue_check(struct sock *sk)
> spin_lock(&fastopenq->lock);
> req1 = fastopenq->rskq_rst_head;
> if (!req1 || time_after(req1->rsk_timer.expires, jiffies)) {
> - spin_unlock(&fastopenq->lock);
> __NET_INC_STATS(sock_net(sk),
> LINUX_MIB_TCPFASTOPENLISTENOVERFLOW);
> + spin_unlock(&fastopenq->lock);
> return false;
> }
> fastopenq->rskq_rst_head = req1->dl_next;
> @@ -282,7 +282,7 @@ struct sock *tcp_try_fastopen(struct sock *sk, struct sk_buff *skb,
> struct sock *child;
>
> if (foc->len == 0) /* Client requests a cookie */
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPFASTOPENCOOKIEREQD);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPFASTOPENCOOKIEREQD);
>
> if (!((sysctl_tcp_fastopen & TFO_SERVER_ENABLE) &&
> (syn_data || foc->len >= 0) &&
> @@ -311,13 +311,13 @@ fastopen:
> child = tcp_fastopen_create_child(sk, skb, dst, req);
> if (child) {
> foc->len = -1;
> - __NET_INC_STATS(sock_net(sk),
> - LINUX_MIB_TCPFASTOPENPASSIVE);
> + NET_INC_STATS(sock_net(sk),
> + LINUX_MIB_TCPFASTOPENPASSIVE);
> return child;
> }
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPFASTOPENPASSIVEFAIL);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPFASTOPENPASSIVEFAIL);
> } else if (foc->len > 0) /* Client presents an invalid cookie */
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPFASTOPENPASSIVEFAIL);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPFASTOPENPASSIVEFAIL);
>
> valid_foc.exp = foc->exp;
> *foc = valid_foc;
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 1fb19c91e091..ac85fb42a5a2 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -869,7 +869,7 @@ static void tcp_update_reordering(struct sock *sk, const int metric,
> else
> mib_idx = LINUX_MIB_TCPSACKREORDER;
>
> - __NET_INC_STATS(sock_net(sk), mib_idx);
> + NET_INC_STATS(sock_net(sk), mib_idx);
> #if FASTRETRANS_DEBUG > 1
> pr_debug("Disorder%d %d %u f%u s%u rr%d\n",
> tp->rx_opt.sack_ok, inet_csk(sk)->icsk_ca_state,
> @@ -1062,7 +1062,7 @@ static bool tcp_check_dsack(struct sock *sk, const struct sk_buff *ack_skb,
> if (before(start_seq_0, TCP_SKB_CB(ack_skb)->ack_seq)) {
> dup_sack = true;
> tcp_dsack_seen(tp);
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPDSACKRECV);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPDSACKRECV);
> } else if (num_sacks > 1) {
> u32 end_seq_1 = get_unaligned_be32(&sp[1].end_seq);
> u32 start_seq_1 = get_unaligned_be32(&sp[1].start_seq);
> @@ -1071,7 +1071,7 @@ static bool tcp_check_dsack(struct sock *sk, const struct sk_buff *ack_skb,
> !before(start_seq_0, start_seq_1)) {
> dup_sack = true;
> tcp_dsack_seen(tp);
> - __NET_INC_STATS(sock_net(sk),
> + NET_INC_STATS(sock_net(sk),
> LINUX_MIB_TCPDSACKOFORECV);
> }
> }
> @@ -1289,7 +1289,7 @@ static bool tcp_shifted_skb(struct sock *sk, struct sk_buff *skb,
>
> if (skb->len > 0) {
> BUG_ON(!tcp_skb_pcount(skb));
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_SACKSHIFTED);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_SACKSHIFTED);
> return false;
> }
>
> @@ -1314,7 +1314,7 @@ static bool tcp_shifted_skb(struct sock *sk, struct sk_buff *skb,
> tcp_unlink_write_queue(skb, sk);
> sk_wmem_free_skb(sk, skb);
>
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_SACKMERGED);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_SACKMERGED);
>
> return true;
> }
> @@ -1473,7 +1473,7 @@ noop:
> return skb;
>
> fallback:
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_SACKSHIFTFALLBACK);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_SACKSHIFTFALLBACK);
> return NULL;
> }
>
> @@ -1661,7 +1661,7 @@ tcp_sacktag_write_queue(struct sock *sk, const struct sk_buff *ack_skb,
> mib_idx = LINUX_MIB_TCPSACKDISCARD;
> }
>
> - __NET_INC_STATS(sock_net(sk), mib_idx);
> + NET_INC_STATS(sock_net(sk), mib_idx);
> if (i == 0)
> first_sack_index = -1;
> continue;
> @@ -1913,7 +1913,7 @@ void tcp_enter_loss(struct sock *sk)
> skb = tcp_write_queue_head(sk);
> is_reneg = skb && (TCP_SKB_CB(skb)->sacked & TCPCB_SACKED_ACKED);
> if (is_reneg) {
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPSACKRENEGING);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPSACKRENEGING);
> tp->sacked_out = 0;
> tp->fackets_out = 0;
> }
> @@ -2399,7 +2399,7 @@ static bool tcp_try_undo_recovery(struct sock *sk)
> else
> mib_idx = LINUX_MIB_TCPFULLUNDO;
>
> - __NET_INC_STATS(sock_net(sk), mib_idx);
> + NET_INC_STATS(sock_net(sk), mib_idx);
> }
> if (tp->snd_una == tp->high_seq && tcp_is_reno(tp)) {
> /* Hold old state until something *above* high_seq
> @@ -2421,7 +2421,7 @@ static bool tcp_try_undo_dsack(struct sock *sk)
> if (tp->undo_marker && !tp->undo_retrans) {
> DBGUNDO(sk, "D-SACK");
> tcp_undo_cwnd_reduction(sk, false);
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPDSACKUNDO);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPDSACKUNDO);
> return true;
> }
> return false;
> @@ -2436,9 +2436,9 @@ static bool tcp_try_undo_loss(struct sock *sk, bool frto_undo)
> tcp_undo_cwnd_reduction(sk, true);
>
> DBGUNDO(sk, "partial loss");
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPLOSSUNDO);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPLOSSUNDO);
> if (frto_undo)
> - __NET_INC_STATS(sock_net(sk),
> + NET_INC_STATS(sock_net(sk),
> LINUX_MIB_TCPSPURIOUSRTOS);
> inet_csk(sk)->icsk_retransmits = 0;
> if (frto_undo || tcp_is_sack(tp))
> @@ -2563,7 +2563,7 @@ static void tcp_mtup_probe_failed(struct sock *sk)
>
> icsk->icsk_mtup.search_high = icsk->icsk_mtup.probe_size - 1;
> icsk->icsk_mtup.probe_size = 0;
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMTUPFAIL);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMTUPFAIL);
> }
>
> static void tcp_mtup_probe_success(struct sock *sk)
> @@ -2583,7 +2583,7 @@ static void tcp_mtup_probe_success(struct sock *sk)
> icsk->icsk_mtup.search_low = icsk->icsk_mtup.probe_size;
> icsk->icsk_mtup.probe_size = 0;
> tcp_sync_mss(sk, icsk->icsk_pmtu_cookie);
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMTUPSUCCESS);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMTUPSUCCESS);
> }
>
> /* Do a simple retransmit without using the backoff mechanisms in
> @@ -2647,7 +2647,7 @@ static void tcp_enter_recovery(struct sock *sk, bool ece_ack)
> else
> mib_idx = LINUX_MIB_TCPSACKRECOVERY;
>
> - __NET_INC_STATS(sock_net(sk), mib_idx);
> + NET_INC_STATS(sock_net(sk), mib_idx);
>
> tp->prior_ssthresh = 0;
> tcp_init_undo(tp);
> @@ -2740,7 +2740,7 @@ static bool tcp_try_undo_partial(struct sock *sk, const int acked)
>
> DBGUNDO(sk, "partial recovery");
> tcp_undo_cwnd_reduction(sk, true);
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPPARTIALUNDO);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPPARTIALUNDO);
> tcp_try_keep_open(sk);
> return true;
> }
> @@ -3434,7 +3434,7 @@ bool tcp_oow_rate_limited(struct net *net, const struct sk_buff *skb,
> s32 elapsed = (s32)(tcp_time_stamp - *last_oow_ack_time);
>
> if (0 <= elapsed && elapsed < sysctl_tcp_invalid_ratelimit) {
> - __NET_INC_STATS(net, mib_idx);
> + NET_INC_STATS(net, mib_idx);
> return true; /* rate-limited: don't send yet! */
> }
> }
> @@ -3467,7 +3467,7 @@ static void tcp_send_challenge_ack(struct sock *sk, const struct sk_buff *skb)
> challenge_count = 0;
> }
> if (++challenge_count <= sysctl_tcp_challenge_ack_limit) {
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPCHALLENGEACK);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPCHALLENGEACK);
> tcp_send_ack(sk);
> }
> }
> @@ -3516,7 +3516,7 @@ static void tcp_process_tlp_ack(struct sock *sk, u32 ack, int flag)
> tcp_set_ca_state(sk, TCP_CA_CWR);
> tcp_end_cwnd_reduction(sk);
> tcp_try_keep_open(sk);
> - __NET_INC_STATS(sock_net(sk),
> + NET_INC_STATS(sock_net(sk),
> LINUX_MIB_TCPLOSSPROBERECOVERY);
> } else if (!(flag & (FLAG_SND_UNA_ADVANCED |
> FLAG_NOT_DUP | FLAG_DATA_SACKED))) {
> @@ -3621,14 +3621,14 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
>
> tcp_in_ack_event(sk, CA_ACK_WIN_UPDATE);
>
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPHPACKS);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPHPACKS);
> } else {
> u32 ack_ev_flags = CA_ACK_SLOWPATH;
>
> if (ack_seq != TCP_SKB_CB(skb)->end_seq)
> flag |= FLAG_DATA;
> else
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPPUREACKS);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPPUREACKS);
>
> flag |= tcp_ack_update_window(sk, skb, ack, ack_seq);
>
> @@ -4131,7 +4131,7 @@ static void tcp_dsack_set(struct sock *sk, u32 seq, u32 end_seq)
> else
> mib_idx = LINUX_MIB_TCPDSACKOFOSENT;
>
> - __NET_INC_STATS(sock_net(sk), mib_idx);
> + NET_INC_STATS(sock_net(sk), mib_idx);
>
> tp->rx_opt.dsack = 1;
> tp->duplicate_sack[0].start_seq = seq;
> @@ -4155,7 +4155,7 @@ static void tcp_send_dupack(struct sock *sk, const struct sk_buff *skb)
>
> if (TCP_SKB_CB(skb)->end_seq != TCP_SKB_CB(skb)->seq &&
> before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt)) {
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_DELAYEDACKLOST);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_DELAYEDACKLOST);
> tcp_enter_quickack_mode(sk);
>
> if (tcp_is_sack(tp) && sysctl_tcp_dsack) {
> @@ -4305,7 +4305,7 @@ static bool tcp_try_coalesce(struct sock *sk,
>
> atomic_add(delta, &sk->sk_rmem_alloc);
> sk_mem_charge(sk, delta);
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPRCVCOALESCE);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPRCVCOALESCE);
> TCP_SKB_CB(to)->end_seq = TCP_SKB_CB(from)->end_seq;
> TCP_SKB_CB(to)->ack_seq = TCP_SKB_CB(from)->ack_seq;
> TCP_SKB_CB(to)->tcp_flags |= TCP_SKB_CB(from)->tcp_flags;
> @@ -4393,7 +4393,7 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
> tcp_ecn_check_ce(tp, skb);
>
> if (unlikely(tcp_try_rmem_schedule(sk, skb, skb->truesize))) {
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPOFODROP);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPOFODROP);
> tcp_drop(sk, skb);
> return;
> }
> @@ -4402,7 +4402,7 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
> tp->pred_flags = 0;
> inet_csk_schedule_ack(sk);
>
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPOFOQUEUE);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPOFOQUEUE);
> SOCK_DEBUG(sk, "out of order segment: rcv_next %X seq %X - %X\n",
> tp->rcv_nxt, TCP_SKB_CB(skb)->seq, TCP_SKB_CB(skb)->end_seq);
>
> @@ -4457,7 +4457,7 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
> if (skb1 && before(seq, TCP_SKB_CB(skb1)->end_seq)) {
> if (!after(end_seq, TCP_SKB_CB(skb1)->end_seq)) {
> /* All the bits are present. Drop. */
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPOFOMERGE);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPOFOMERGE);
> tcp_drop(sk, skb);
> skb = NULL;
> tcp_dsack_set(sk, seq, end_seq);
> @@ -4496,7 +4496,7 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
> __skb_unlink(skb1, &tp->out_of_order_queue);
> tcp_dsack_extend(sk, TCP_SKB_CB(skb1)->seq,
> TCP_SKB_CB(skb1)->end_seq);
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPOFOMERGE);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPOFOMERGE);
> tcp_drop(sk, skb1);
> }
>
> @@ -4661,7 +4661,7 @@ queue_and_out:
>
> if (!after(TCP_SKB_CB(skb)->end_seq, tp->rcv_nxt)) {
> /* A retransmit, 2nd most common case. Force an immediate ack. */
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_DELAYEDACKLOST);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_DELAYEDACKLOST);
> tcp_dsack_set(sk, TCP_SKB_CB(skb)->seq, TCP_SKB_CB(skb)->end_seq);
>
> out_of_window:
> @@ -4707,7 +4707,7 @@ static struct sk_buff *tcp_collapse_one(struct sock *sk, struct sk_buff *skb,
>
> __skb_unlink(skb, list);
> __kfree_skb(skb);
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPRCVCOLLAPSED);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPRCVCOLLAPSED);
>
> return next;
> }
> @@ -4866,7 +4866,7 @@ static bool tcp_prune_ofo_queue(struct sock *sk)
> bool res = false;
>
> if (!skb_queue_empty(&tp->out_of_order_queue)) {
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_OFOPRUNED);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_OFOPRUNED);
> __skb_queue_purge(&tp->out_of_order_queue);
>
> /* Reset SACK state. A conforming SACK implementation will
> @@ -4895,7 +4895,7 @@ static int tcp_prune_queue(struct sock *sk)
>
> SOCK_DEBUG(sk, "prune_queue: c=%x\n", tp->copied_seq);
>
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_PRUNECALLED);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_PRUNECALLED);
>
> if (atomic_read(&sk->sk_rmem_alloc) >= sk->sk_rcvbuf)
> tcp_clamp_window(sk);
> @@ -4925,7 +4925,7 @@ static int tcp_prune_queue(struct sock *sk)
> * drop receive data on the floor. It will get retransmitted
> * and hopefully then we'll have sufficient space.
> */
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_RCVPRUNED);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_RCVPRUNED);
>
> /* Massive buffer overcommit. */
> tp->pred_flags = 0;
> @@ -5184,7 +5184,7 @@ static bool tcp_validate_incoming(struct sock *sk, struct sk_buff *skb,
> if (tcp_fast_parse_options(skb, th, tp) && tp->rx_opt.saw_tstamp &&
> tcp_paws_discard(sk, skb)) {
> if (!th->rst) {
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_PAWSESTABREJECTED);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_PAWSESTABREJECTED);
> if (!tcp_oow_rate_limited(sock_net(sk), skb,
> LINUX_MIB_TCPACKSKIPPEDPAWS,
> &tp->last_oow_ack_time))
> @@ -5236,8 +5236,8 @@ static bool tcp_validate_incoming(struct sock *sk, struct sk_buff *skb,
> if (th->syn) {
> syn_challenge:
> if (syn_inerr)
> - __TCP_INC_STATS(sock_net(sk), TCP_MIB_INERRS);
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPSYNCHALLENGE);
> + TCP_INC_STATS(sock_net(sk), TCP_MIB_INERRS);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPSYNCHALLENGE);
> tcp_send_challenge_ack(sk, skb);
> goto discard;
> }
> @@ -5352,7 +5352,7 @@ void tcp_rcv_established(struct sock *sk, struct sk_buff *skb,
> tcp_data_snd_check(sk);
> return;
> } else { /* Header too small */
> - __TCP_INC_STATS(sock_net(sk), TCP_MIB_INERRS);
> + TCP_INC_STATS(sock_net(sk), TCP_MIB_INERRS);
> goto discard;
> }
> } else {
> @@ -5380,7 +5380,7 @@ void tcp_rcv_established(struct sock *sk, struct sk_buff *skb,
>
> __skb_pull(skb, tcp_header_len);
> tcp_rcv_nxt_update(tp, TCP_SKB_CB(skb)->end_seq);
> - __NET_INC_STATS(sock_net(sk),
> + NET_INC_STATS(sock_net(sk),
> LINUX_MIB_TCPHPHITSTOUSER);
> eaten = 1;
> }
> @@ -5403,7 +5403,7 @@ void tcp_rcv_established(struct sock *sk, struct sk_buff *skb,
>
> tcp_rcv_rtt_measure_ts(sk, skb);
>
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPHPHITS);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPHPHITS);
>
> /* Bulk data transfer: receiver */
> eaten = tcp_queue_rcv(sk, skb, tcp_header_len,
> @@ -5460,8 +5460,8 @@ step5:
> return;
>
> csum_error:
> - __TCP_INC_STATS(sock_net(sk), TCP_MIB_CSUMERRORS);
> - __TCP_INC_STATS(sock_net(sk), TCP_MIB_INERRS);
> + TCP_INC_STATS(sock_net(sk), TCP_MIB_CSUMERRORS);
> + TCP_INC_STATS(sock_net(sk), TCP_MIB_INERRS);
>
> discard:
> tcp_drop(sk, skb);
> @@ -5553,13 +5553,13 @@ static bool tcp_rcv_fastopen_synack(struct sock *sk, struct sk_buff *synack,
> break;
> }
> tcp_rearm_rto(sk);
> - __NET_INC_STATS(sock_net(sk),
> + NET_INC_STATS(sock_net(sk),
> LINUX_MIB_TCPFASTOPENACTIVEFAIL);
> return true;
> }
> tp->syn_data_acked = tp->syn_data;
> if (tp->syn_data_acked)
> - __NET_INC_STATS(sock_net(sk),
> + NET_INC_STATS(sock_net(sk),
> LINUX_MIB_TCPFASTOPENACTIVE);
>
> tcp_fastopen_add_skb(sk, synack);
> @@ -5595,7 +5595,7 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
> if (tp->rx_opt.saw_tstamp && tp->rx_opt.rcv_tsecr &&
> !between(tp->rx_opt.rcv_tsecr, tp->retrans_stamp,
> tcp_time_stamp)) {
> - __NET_INC_STATS(sock_net(sk),
> + NET_INC_STATS(sock_net(sk),
> LINUX_MIB_PAWSACTIVEREJECTED);
> goto reset_and_undo;
> }
> @@ -5965,7 +5965,7 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
> (TCP_SKB_CB(skb)->end_seq != TCP_SKB_CB(skb)->seq &&
> after(TCP_SKB_CB(skb)->end_seq - th->fin, tp->rcv_nxt))) {
> tcp_done(sk);
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPABORTONDATA);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPABORTONDATA);
> return 1;
> }
>
> @@ -6022,7 +6022,7 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
> if (sk->sk_shutdown & RCV_SHUTDOWN) {
> if (TCP_SKB_CB(skb)->end_seq != TCP_SKB_CB(skb)->seq &&
> after(TCP_SKB_CB(skb)->end_seq - th->fin, tp->rcv_nxt)) {
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPABORTONDATA);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPABORTONDATA);
> tcp_reset(sk);
> return 1;
> }
> @@ -6224,7 +6224,7 @@ int tcp_conn_request(struct request_sock_ops *rsk_ops,
> * timeout.
> */
> if (sk_acceptq_is_full(sk) && inet_csk_reqsk_queue_young(sk) > 1) {
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_LISTENOVERFLOWS);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_LISTENOVERFLOWS);
> goto drop;
> }
>
> @@ -6271,7 +6271,7 @@ int tcp_conn_request(struct request_sock_ops *rsk_ops,
> if (dst && strict &&
> !tcp_peer_is_proven(req, dst, true,
> tmp_opt.saw_tstamp)) {
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_PAWSPASSIVEREJECTED);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_PAWSPASSIVEREJECTED);
> goto drop_and_release;
> }
> }
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index 87b173b563b0..761bc492c5e3 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -692,6 +692,7 @@ static void tcp_v4_send_reset(const struct sock *sk, struct sk_buff *skb)
> offsetof(struct inet_timewait_sock, tw_bound_dev_if));
>
> arg.tos = ip_hdr(skb)->tos;
> + preempt_disable();
> ip_send_unicast_reply(*this_cpu_ptr(net->ipv4.tcp_sk),
> skb, &TCP_SKB_CB(skb)->header.h4.opt,
> ip_hdr(skb)->saddr, ip_hdr(skb)->daddr,
> @@ -699,6 +700,7 @@ static void tcp_v4_send_reset(const struct sock *sk, struct sk_buff *skb)
>
> __TCP_INC_STATS(net, TCP_MIB_OUTSEGS);
> __TCP_INC_STATS(net, TCP_MIB_OUTRSTS);
> + preempt_enable();
>
> #ifdef CONFIG_TCP_MD5SIG
> out:
> @@ -774,12 +776,14 @@ static void tcp_v4_send_ack(struct net *net,
> if (oif)
> arg.bound_dev_if = oif;
> arg.tos = tos;
> + preempt_disable();
> ip_send_unicast_reply(*this_cpu_ptr(net->ipv4.tcp_sk),
> skb, &TCP_SKB_CB(skb)->header.h4.opt,
> ip_hdr(skb)->saddr, ip_hdr(skb)->daddr,
> &arg, arg.iov[0].iov_len);
>
> __TCP_INC_STATS(net, TCP_MIB_OUTSEGS);
> + preempt_enable();
> }
>
> static void tcp_v4_timewait_ack(struct sock *sk, struct sk_buff *skb)
> @@ -1151,12 +1155,12 @@ static bool tcp_v4_inbound_md5_hash(const struct sock *sk,
> return false;
>
> if (hash_expected && !hash_location) {
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMD5NOTFOUND);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMD5NOTFOUND);
> return true;
> }
>
> if (!hash_expected && hash_location) {
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMD5UNEXPECTED);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMD5UNEXPECTED);
> return true;
> }
>
> @@ -1342,7 +1346,7 @@ struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
> return newsk;
>
> exit_overflow:
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_LISTENOVERFLOWS);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_LISTENOVERFLOWS);
> exit_nonewsk:
> dst_release(dst);
> exit:
> @@ -1432,8 +1436,8 @@ discard:
> return 0;
>
> csum_err:
> - __TCP_INC_STATS(sock_net(sk), TCP_MIB_CSUMERRORS);
> - __TCP_INC_STATS(sock_net(sk), TCP_MIB_INERRS);
> + TCP_INC_STATS(sock_net(sk), TCP_MIB_CSUMERRORS);
> + TCP_INC_STATS(sock_net(sk), TCP_MIB_INERRS);
> goto discard;
> }
> EXPORT_SYMBOL(tcp_v4_do_rcv);
> diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
> index ffbfecdae471..4b95ec4ed2c8 100644
> --- a/net/ipv4/tcp_minisocks.c
> +++ b/net/ipv4/tcp_minisocks.c
> @@ -337,7 +337,7 @@ void tcp_time_wait(struct sock *sk, int state, int timeo)
> * socket up. We've got bigger problems than
> * non-graceful socket closings.
> */
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPTIMEWAITOVERFLOW);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPTIMEWAITOVERFLOW);
> }
>
> tcp_update_metrics(sk);
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 1a487ff95d4c..25d527922b18 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -2221,14 +2221,13 @@ bool tcp_schedule_loss_probe(struct sock *sk)
> /* Thanks to skb fast clones, we can detect if a prior transmit of
> * a packet is still in a qdisc or driver queue.
> * In this case, there is very little point doing a retransmit !
> - * Note: This is called from BH context only.
> */
> static bool skb_still_in_host_queue(const struct sock *sk,
> const struct sk_buff *skb)
> {
> if (unlikely(skb_fclone_busy(sk, skb))) {
> - __NET_INC_STATS(sock_net(sk),
> - LINUX_MIB_TCPSPURIOUS_RTX_HOSTQUEUES);
> + NET_INC_STATS(sock_net(sk),
> + LINUX_MIB_TCPSPURIOUS_RTX_HOSTQUEUES);
> return true;
> }
> return false;
> @@ -2290,7 +2289,7 @@ void tcp_send_loss_probe(struct sock *sk)
> tp->tlp_high_seq = tp->snd_nxt;
>
> probe_sent:
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPLOSSPROBES);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPLOSSPROBES);
> /* Reset s.t. tcp_rearm_rto will restart timer from now */
> inet_csk(sk)->icsk_pending = 0;
> rearm_timer:
> @@ -2699,7 +2698,7 @@ int tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb, int segs)
> tp->retrans_stamp = tcp_skb_timestamp(skb);
>
> } else if (err != -EBUSY) {
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPRETRANSFAIL);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPRETRANSFAIL);
> }
>
> if (tp->undo_retrans < 0)
> @@ -2823,7 +2822,7 @@ begin_fwd:
> if (tcp_retransmit_skb(sk, skb, segs))
> return;
>
> - __NET_INC_STATS(sock_net(sk), mib_idx);
> + NET_INC_STATS(sock_net(sk), mib_idx);
>
> if (tcp_in_cwnd_reduction(sk))
> tp->prr_out += tcp_skb_pcount(skb);
> diff --git a/net/ipv4/tcp_recovery.c b/net/ipv4/tcp_recovery.c
> index e0d0afaf15be..e36df4fcfeba 100644
> --- a/net/ipv4/tcp_recovery.c
> +++ b/net/ipv4/tcp_recovery.c
> @@ -65,8 +65,8 @@ int tcp_rack_mark_lost(struct sock *sk)
> if (scb->sacked & TCPCB_SACKED_RETRANS) {
> scb->sacked &= ~TCPCB_SACKED_RETRANS;
> tp->retrans_out -= tcp_skb_pcount(skb);
> - __NET_INC_STATS(sock_net(sk),
> - LINUX_MIB_TCPLOSTRETRANSMIT);
> + NET_INC_STATS(sock_net(sk),
> + LINUX_MIB_TCPLOSTRETRANSMIT);
> }
> } else if (!(scb->sacked & TCPCB_RETRANS)) {
> /* Original data are sent sequentially so stop early
> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
> index 35f643d8ffbb..debdd8b33e69 100644
> --- a/net/ipv4/tcp_timer.c
> +++ b/net/ipv4/tcp_timer.c
> @@ -162,8 +162,8 @@ static int tcp_write_timeout(struct sock *sk)
> if (tp->syn_fastopen || tp->syn_data)
> tcp_fastopen_cache_set(sk, 0, NULL, true, 0);
> if (tp->syn_data && icsk->icsk_retransmits == 1)
> - __NET_INC_STATS(sock_net(sk),
> - LINUX_MIB_TCPFASTOPENACTIVEFAIL);
> + NET_INC_STATS(sock_net(sk),
> + LINUX_MIB_TCPFASTOPENACTIVEFAIL);
> }
> retry_until = icsk->icsk_syn_retries ? : net->ipv4.sysctl_tcp_syn_retries;
> syn_set = true;
> @@ -178,8 +178,8 @@ static int tcp_write_timeout(struct sock *sk)
> tp->bytes_acked <= tp->rx_opt.mss_clamp) {
> tcp_fastopen_cache_set(sk, 0, NULL, true, 0);
> if (icsk->icsk_retransmits == net->ipv4.sysctl_tcp_retries1)
> - __NET_INC_STATS(sock_net(sk),
> - LINUX_MIB_TCPFASTOPENACTIVEFAIL);
> + NET_INC_STATS(sock_net(sk),
> + LINUX_MIB_TCPFASTOPENACTIVEFAIL);
> }
> /* Black hole detection */
> tcp_mtu_probing(icsk, sk);
> @@ -209,6 +209,7 @@ static int tcp_write_timeout(struct sock *sk)
> return 0;
> }
>
> +/* Called with BH disabled */
> void tcp_delack_timer_handler(struct sock *sk)
> {
> struct tcp_sock *tp = tcp_sk(sk);
> @@ -493,6 +494,7 @@ out_reset_timer:
> out:;
> }
>
> +/* Called with BH disabled */
> void tcp_write_timer_handler(struct sock *sk)
> {
> struct inet_connection_sock *icsk = inet_csk(sk);
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 52914714b923..7bdc9c9c231b 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -649,12 +649,12 @@ static bool tcp_v6_inbound_md5_hash(const struct sock *sk,
> return false;
>
> if (hash_expected && !hash_location) {
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMD5NOTFOUND);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMD5NOTFOUND);
> return true;
> }
>
> if (!hash_expected && hash_location) {
> - __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMD5UNEXPECTED);
> + NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMD5UNEXPECTED);
> return true;
> }
>
> @@ -825,9 +825,9 @@ static void tcp_v6_send_response(const struct sock *sk, struct sk_buff *skb, u32
> if (!IS_ERR(dst)) {
> skb_dst_set(buff, dst);
> ip6_xmit(ctl_sk, buff, &fl6, NULL, tclass);
> - __TCP_INC_STATS(net, TCP_MIB_OUTSEGS);
> + TCP_INC_STATS(net, TCP_MIB_OUTSEGS);
> if (rst)
> - __TCP_INC_STATS(net, TCP_MIB_OUTRSTS);
> + TCP_INC_STATS(net, TCP_MIB_OUTRSTS);
> return;
> }
>
> @@ -1276,8 +1276,8 @@ discard:
> kfree_skb(skb);
> return 0;
> csum_err:
> - __TCP_INC_STATS(sock_net(sk), TCP_MIB_CSUMERRORS);
> - __TCP_INC_STATS(sock_net(sk), TCP_MIB_INERRS);
> + TCP_INC_STATS(sock_net(sk), TCP_MIB_CSUMERRORS);
> + TCP_INC_STATS(sock_net(sk), TCP_MIB_INERRS);
> goto discard;
>
>
> --
> 2.8.0.rc3.226.g39d4020
>
^ permalink raw reply
* Re: [PATCH v2 net-next 7/7] tcp: make tcp_sendmsg() aware of socket backlog
From: Soheil Hassas Yeganeh @ 2016-04-29 13:13 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, netdev, Alexei Starovoitov,
Marcelo Ricardo Leitner, Eric Dumazet
In-Reply-To: <1461899449-8096-8-git-send-email-edumazet@google.com>
On Thu, Apr 28, 2016 at 11:10 PM, Eric Dumazet <edumazet@google.com> wrote:
> Large sendmsg()/write() hold socket lock for the duration of the call,
> unless sk->sk_sndbuf limit is hit. This is bad because incoming packets
> are parked into socket backlog for a long time.
> Critical decisions like fast retransmit might be delayed.
> Receivers have to maintain a big out of order queue with additional cpu
> overhead, and also possible stalls in TX once windows are full.
>
> Bidirectional flows are particularly hurt since the backlog can become
> quite big if the copy from user space triggers IO (page faults)
>
> Some applications learnt to use sendmsg() (or sendmmsg()) with small
> chunks to avoid this issue.
>
> Kernel should know better, right ?
>
> Add a generic sk_flush_backlog() helper and use it right
> before a new skb is allocated. Typically we put 64KB of payload
> per skb (unless MSG_EOR is requested) and checking socket backlog
> every 64KB gives good results.
>
> As a matter of fact, tests with TSO/GSO disabled give very nice
> results, as we manage to keep a small write queue and smaller
> perceived rtt.
>
> Note that sk_flush_backlog() maintains socket ownership,
> so is not equivalent to a {release_sock(sk); lock_sock(sk);},
> to ensure implicit atomicity rules that sendmsg() was
> giving to (possibly buggy) applications.
>
> In this simple implementation, I chose to not call tcp_release_cb(),
> but we might consider this later.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Soheil Hassas Yeganeh <soheil@google.com>
> Cc: Alexei Starovoitov <ast@fb.com>
> Cc: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Acked-By: Soheil Hassas Yeganeh <soheil@google.com>
> ---
> include/net/sock.h | 11 +++++++++++
> net/core/sock.c | 7 +++++++
> net/ipv4/tcp.c | 8 ++++++--
> 3 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 3df778ccaa82..1dbb1f9f7c1b 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -926,6 +926,17 @@ void sk_stream_kill_queues(struct sock *sk);
> void sk_set_memalloc(struct sock *sk);
> void sk_clear_memalloc(struct sock *sk);
>
> +void __sk_flush_backlog(struct sock *sk);
> +
> +static inline bool sk_flush_backlog(struct sock *sk)
> +{
> + if (unlikely(READ_ONCE(sk->sk_backlog.tail))) {
> + __sk_flush_backlog(sk);
> + return true;
> + }
> + return false;
> +}
> +
> int sk_wait_data(struct sock *sk, long *timeo, const struct sk_buff *skb);
>
> struct request_sock_ops;
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 70744dbb6c3f..f615e9391170 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -2048,6 +2048,13 @@ static void __release_sock(struct sock *sk)
> sk->sk_backlog.len = 0;
> }
>
> +void __sk_flush_backlog(struct sock *sk)
> +{
> + spin_lock_bh(&sk->sk_lock.slock);
> + __release_sock(sk);
> + spin_unlock_bh(&sk->sk_lock.slock);
> +}
> +
> /**
> * sk_wait_data - wait for data to arrive at sk_receive_queue
> * @sk: sock to wait on
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 4787f86ae64c..b945c2b046c5 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -1136,11 +1136,12 @@ int tcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
> /* This should be in poll */
> sk_clear_bit(SOCKWQ_ASYNC_NOSPACE, sk);
>
> - mss_now = tcp_send_mss(sk, &size_goal, flags);
> -
> /* Ok commence sending. */
> copied = 0;
>
> +restart:
> + mss_now = tcp_send_mss(sk, &size_goal, flags);
> +
> err = -EPIPE;
> if (sk->sk_err || (sk->sk_shutdown & SEND_SHUTDOWN))
> goto out_err;
> @@ -1166,6 +1167,9 @@ new_segment:
> if (!sk_stream_memory_free(sk))
> goto wait_for_sndbuf;
>
> + if (sk_flush_backlog(sk))
> + goto restart;
> +
> skb = sk_stream_alloc_skb(sk,
> select_size(sk, sg),
> sk->sk_allocation,
> --
> 2.8.0.rc3.226.g39d4020
>
This is superb Eric! Thanks.
^ permalink raw reply
* Re: [PATCH net-next] net: dsa: mv88e6xxx: replace ds with ps where possible
From: Andrew Lunn @ 2016-04-29 13:06 UTC (permalink / raw)
To: Vivien Didelot; +Cc: netdev, linux-kernel, kernel, David S. Miller
In-Reply-To: <1461893046-28200-1-git-send-email-vivien.didelot@savoirfairelinux.com>
On Thu, Apr 28, 2016 at 09:24:06PM -0400, Vivien Didelot wrote:
> From: Andrew Lunn <andrew@lunn.ch>
>
> The dsa_switch structure ds is actually needed in very few places,
> mostly during setup of the switch. The private structure ps is however
> needed nearly everywhere. Pass ps, not ds internally.
>
> [vd: rebased Andrew's patch.]
Hi Vivien
Thanks for picking up this patch and rebasing it.
I would generally put comments like that bellow the ---. They don't
need to be in the commit log.
Andrew
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
> drivers/net/dsa/mv88e6123.c | 14 +-
> drivers/net/dsa/mv88e6131.c | 22 +-
> drivers/net/dsa/mv88e6171.c | 14 +-
> drivers/net/dsa/mv88e6352.c | 24 +-
> drivers/net/dsa/mv88e6xxx.c | 917 ++++++++++++++++++++++----------------------
> drivers/net/dsa/mv88e6xxx.h | 14 +-
> 6 files changed, 511 insertions(+), 494 deletions(-)
>
> diff --git a/drivers/net/dsa/mv88e6123.c b/drivers/net/dsa/mv88e6123.c
> index 534ebc8..5535a42 100644
> --- a/drivers/net/dsa/mv88e6123.c
> +++ b/drivers/net/dsa/mv88e6123.c
> @@ -50,6 +50,7 @@ static const char *mv88e6123_drv_probe(struct device *dsa_dev,
>
> static int mv88e6123_setup_global(struct dsa_switch *ds)
> {
> + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> u32 upstream_port = dsa_upstream_port(ds);
> int ret;
> u32 reg;
> @@ -62,7 +63,7 @@ static int mv88e6123_setup_global(struct dsa_switch *ds)
> * external PHYs to poll), don't discard packets with
> * excessive collisions, and mask all interrupt sources.
> */
> - ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL, 0x0000);
> + ret = mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_CONTROL, 0x0000);
> if (ret)
> return ret;
>
> @@ -73,26 +74,29 @@ static int mv88e6123_setup_global(struct dsa_switch *ds)
> reg = upstream_port << GLOBAL_MONITOR_CONTROL_INGRESS_SHIFT |
> upstream_port << GLOBAL_MONITOR_CONTROL_EGRESS_SHIFT |
> upstream_port << GLOBAL_MONITOR_CONTROL_ARP_SHIFT;
> - ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_MONITOR_CONTROL, reg);
> + ret = mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_MONITOR_CONTROL, reg);
> if (ret)
> return ret;
>
> /* Disable remote management for now, and set the switch's
> * DSA device number.
> */
> - return mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL_2,
> + return mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_CONTROL_2,
> ds->index & 0x1f);
> }
>
> static int mv88e6123_setup(struct dsa_switch *ds)
> {
> + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> int ret;
>
> - ret = mv88e6xxx_setup_common(ds);
> + ps->ds = ds;
> +
> + ret = mv88e6xxx_setup_common(ps);
> if (ret < 0)
> return ret;
>
> - ret = mv88e6xxx_switch_reset(ds, false);
> + ret = mv88e6xxx_switch_reset(ps, false);
> if (ret < 0)
> return ret;
>
> diff --git a/drivers/net/dsa/mv88e6131.c b/drivers/net/dsa/mv88e6131.c
> index c3eb9a8..357ab79 100644
> --- a/drivers/net/dsa/mv88e6131.c
> +++ b/drivers/net/dsa/mv88e6131.c
> @@ -56,6 +56,7 @@ static const char *mv88e6131_drv_probe(struct device *dsa_dev,
>
> static int mv88e6131_setup_global(struct dsa_switch *ds)
> {
> + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> u32 upstream_port = dsa_upstream_port(ds);
> int ret;
> u32 reg;
> @@ -69,14 +70,14 @@ static int mv88e6131_setup_global(struct dsa_switch *ds)
> * to arbitrate between packet queues, set the maximum frame
> * size to 1632, and mask all interrupt sources.
> */
> - ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL,
> + ret = mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_CONTROL,
> GLOBAL_CONTROL_PPU_ENABLE |
> GLOBAL_CONTROL_MAX_FRAME_1632);
> if (ret)
> return ret;
>
> /* Set the VLAN ethertype to 0x8100. */
> - ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CORE_TAG_TYPE, 0x8100);
> + ret = mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_CORE_TAG_TYPE, 0x8100);
> if (ret)
> return ret;
>
> @@ -87,7 +88,7 @@ static int mv88e6131_setup_global(struct dsa_switch *ds)
> reg = upstream_port << GLOBAL_MONITOR_CONTROL_INGRESS_SHIFT |
> upstream_port << GLOBAL_MONITOR_CONTROL_EGRESS_SHIFT |
> GLOBAL_MONITOR_CONTROL_ARP_DISABLED;
> - ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_MONITOR_CONTROL, reg);
> + ret = mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_MONITOR_CONTROL, reg);
> if (ret)
> return ret;
>
> @@ -96,11 +97,11 @@ static int mv88e6131_setup_global(struct dsa_switch *ds)
> * DSA device number.
> */
> if (ds->dst->pd->nr_chips > 1)
> - ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL_2,
> + ret = mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_CONTROL_2,
> GLOBAL_CONTROL_2_MULTIPLE_CASCADE |
> (ds->index & 0x1f));
> else
> - ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL_2,
> + ret = mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_CONTROL_2,
> GLOBAL_CONTROL_2_NO_CASCADE |
> (ds->index & 0x1f));
> if (ret)
> @@ -109,7 +110,7 @@ static int mv88e6131_setup_global(struct dsa_switch *ds)
> /* Force the priority of IGMP/MLD snoop frames and ARP frames
> * to the highest setting.
> */
> - return mv88e6xxx_reg_write(ds, REG_GLOBAL2, GLOBAL2_PRIO_OVERRIDE,
> + return mv88e6xxx_reg_write(ps, REG_GLOBAL2, GLOBAL2_PRIO_OVERRIDE,
> GLOBAL2_PRIO_OVERRIDE_FORCE_SNOOP |
> 7 << GLOBAL2_PRIO_OVERRIDE_SNOOP_SHIFT |
> GLOBAL2_PRIO_OVERRIDE_FORCE_ARP |
> @@ -118,15 +119,18 @@ static int mv88e6131_setup_global(struct dsa_switch *ds)
>
> static int mv88e6131_setup(struct dsa_switch *ds)
> {
> + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> int ret;
>
> - ret = mv88e6xxx_setup_common(ds);
> + ps->ds = ds;
> +
> + ret = mv88e6xxx_setup_common(ps);
> if (ret < 0)
> return ret;
>
> - mv88e6xxx_ppu_state_init(ds);
> + mv88e6xxx_ppu_state_init(ps);
>
> - ret = mv88e6xxx_switch_reset(ds, false);
> + ret = mv88e6xxx_switch_reset(ps, false);
> if (ret < 0)
> return ret;
>
> diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
> index 841ffe1..f75164d 100644
> --- a/drivers/net/dsa/mv88e6171.c
> +++ b/drivers/net/dsa/mv88e6171.c
> @@ -56,6 +56,7 @@ static const char *mv88e6171_drv_probe(struct device *dsa_dev,
>
> static int mv88e6171_setup_global(struct dsa_switch *ds)
> {
> + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> u32 upstream_port = dsa_upstream_port(ds);
> int ret;
> u32 reg;
> @@ -67,7 +68,7 @@ static int mv88e6171_setup_global(struct dsa_switch *ds)
> /* Discard packets with excessive collisions, mask all
> * interrupt sources, enable PPU.
> */
> - ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL,
> + ret = mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_CONTROL,
> GLOBAL_CONTROL_PPU_ENABLE |
> GLOBAL_CONTROL_DISCARD_EXCESS);
> if (ret)
> @@ -81,26 +82,29 @@ static int mv88e6171_setup_global(struct dsa_switch *ds)
> upstream_port << GLOBAL_MONITOR_CONTROL_EGRESS_SHIFT |
> upstream_port << GLOBAL_MONITOR_CONTROL_ARP_SHIFT |
> upstream_port << GLOBAL_MONITOR_CONTROL_MIRROR_SHIFT;
> - ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_MONITOR_CONTROL, reg);
> + ret = mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_MONITOR_CONTROL, reg);
> if (ret)
> return ret;
>
> /* Disable remote management for now, and set the switch's
> * DSA device number.
> */
> - return mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL_2,
> + return mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_CONTROL_2,
> ds->index & 0x1f);
> }
>
> static int mv88e6171_setup(struct dsa_switch *ds)
> {
> + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> int ret;
>
> - ret = mv88e6xxx_setup_common(ds);
> + ps->ds = ds;
> +
> + ret = mv88e6xxx_setup_common(ps);
> if (ret < 0)
> return ret;
>
> - ret = mv88e6xxx_switch_reset(ds, true);
> + ret = mv88e6xxx_switch_reset(ps, true);
> if (ret < 0)
> return ret;
>
> diff --git a/drivers/net/dsa/mv88e6352.c b/drivers/net/dsa/mv88e6352.c
> index 4afc24d..c622a1d 100644
> --- a/drivers/net/dsa/mv88e6352.c
> +++ b/drivers/net/dsa/mv88e6352.c
> @@ -73,6 +73,7 @@ static const char *mv88e6352_drv_probe(struct device *dsa_dev,
>
> static int mv88e6352_setup_global(struct dsa_switch *ds)
> {
> + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> u32 upstream_port = dsa_upstream_port(ds);
> int ret;
> u32 reg;
> @@ -84,7 +85,7 @@ static int mv88e6352_setup_global(struct dsa_switch *ds)
> /* Discard packets with excessive collisions,
> * mask all interrupt sources, enable PPU (bit 14, undocumented).
> */
> - ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL,
> + ret = mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_CONTROL,
> GLOBAL_CONTROL_PPU_ENABLE |
> GLOBAL_CONTROL_DISCARD_EXCESS);
> if (ret)
> @@ -97,14 +98,14 @@ static int mv88e6352_setup_global(struct dsa_switch *ds)
> reg = upstream_port << GLOBAL_MONITOR_CONTROL_INGRESS_SHIFT |
> upstream_port << GLOBAL_MONITOR_CONTROL_EGRESS_SHIFT |
> upstream_port << GLOBAL_MONITOR_CONTROL_ARP_SHIFT;
> - ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_MONITOR_CONTROL, reg);
> + ret = mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_MONITOR_CONTROL, reg);
> if (ret)
> return ret;
>
> /* Disable remote management for now, and set the switch's
> * DSA device number.
> */
> - return mv88e6xxx_reg_write(ds, REG_GLOBAL, 0x1c, ds->index & 0x1f);
> + return mv88e6xxx_reg_write(ps, REG_GLOBAL, 0x1c, ds->index & 0x1f);
> }
>
> static int mv88e6352_setup(struct dsa_switch *ds)
> @@ -112,13 +113,15 @@ static int mv88e6352_setup(struct dsa_switch *ds)
> struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> int ret;
>
> - ret = mv88e6xxx_setup_common(ds);
> + ps->ds = ds;
> +
> + ret = mv88e6xxx_setup_common(ps);
> if (ret < 0)
> return ret;
>
> mutex_init(&ps->eeprom_mutex);
>
> - ret = mv88e6xxx_switch_reset(ds, true);
> + ret = mv88e6xxx_switch_reset(ps, true);
> if (ret < 0)
> return ret;
>
> @@ -136,7 +139,7 @@ static int mv88e6352_read_eeprom_word(struct dsa_switch *ds, int addr)
>
> mutex_lock(&ps->eeprom_mutex);
>
> - ret = mv88e6xxx_reg_write(ds, REG_GLOBAL2, GLOBAL2_EEPROM_OP,
> + ret = mv88e6xxx_reg_write(ps, REG_GLOBAL2, GLOBAL2_EEPROM_OP,
> GLOBAL2_EEPROM_OP_READ |
> (addr & GLOBAL2_EEPROM_OP_ADDR_MASK));
> if (ret < 0)
> @@ -146,7 +149,7 @@ static int mv88e6352_read_eeprom_word(struct dsa_switch *ds, int addr)
> if (ret < 0)
> goto error;
>
> - ret = mv88e6xxx_reg_read(ds, REG_GLOBAL2, GLOBAL2_EEPROM_DATA);
> + ret = mv88e6xxx_reg_read(ps, REG_GLOBAL2, GLOBAL2_EEPROM_DATA);
> error:
> mutex_unlock(&ps->eeprom_mutex);
> return ret;
> @@ -217,9 +220,10 @@ static int mv88e6352_get_eeprom(struct dsa_switch *ds,
>
> static int mv88e6352_eeprom_is_readonly(struct dsa_switch *ds)
> {
> + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> int ret;
>
> - ret = mv88e6xxx_reg_read(ds, REG_GLOBAL2, GLOBAL2_EEPROM_OP);
> + ret = mv88e6xxx_reg_read(ps, REG_GLOBAL2, GLOBAL2_EEPROM_OP);
> if (ret < 0)
> return ret;
>
> @@ -237,11 +241,11 @@ static int mv88e6352_write_eeprom_word(struct dsa_switch *ds, int addr,
>
> mutex_lock(&ps->eeprom_mutex);
>
> - ret = mv88e6xxx_reg_write(ds, REG_GLOBAL2, GLOBAL2_EEPROM_DATA, data);
> + ret = mv88e6xxx_reg_write(ps, REG_GLOBAL2, GLOBAL2_EEPROM_DATA, data);
> if (ret < 0)
> goto error;
>
> - ret = mv88e6xxx_reg_write(ds, REG_GLOBAL2, GLOBAL2_EEPROM_OP,
> + ret = mv88e6xxx_reg_write(ps, REG_GLOBAL2, GLOBAL2_EEPROM_OP,
> GLOBAL2_EEPROM_OP_WRITE |
> (addr & GLOBAL2_EEPROM_OP_ADDR_MASK));
> if (ret < 0)
> diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
> index 028f92f..61150af 100644
> --- a/drivers/net/dsa/mv88e6xxx.c
> +++ b/drivers/net/dsa/mv88e6xxx.c
> @@ -25,12 +25,10 @@
> #include <net/switchdev.h>
> #include "mv88e6xxx.h"
>
> -static void assert_smi_lock(struct dsa_switch *ds)
> +static void assert_smi_lock(struct mv88e6xxx_priv_state *ps)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> -
> if (unlikely(!mutex_is_locked(&ps->smi_mutex))) {
> - dev_err(ds->master_dev, "SMI lock not held!\n");
> + dev_err(ps->dev, "SMI lock not held!\n");
> dump_stack();
> }
> }
> @@ -92,30 +90,29 @@ static int __mv88e6xxx_reg_read(struct mii_bus *bus, int sw_addr, int addr,
> return ret & 0xffff;
> }
>
> -static int _mv88e6xxx_reg_read(struct dsa_switch *ds, int addr, int reg)
> +static int _mv88e6xxx_reg_read(struct mv88e6xxx_priv_state *ps,
> + int addr, int reg)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> int ret;
>
> - assert_smi_lock(ds);
> + assert_smi_lock(ps);
>
> ret = __mv88e6xxx_reg_read(ps->bus, ps->sw_addr, addr, reg);
> if (ret < 0)
> return ret;
>
> - dev_dbg(ds->master_dev, "<- addr: 0x%.2x reg: 0x%.2x val: 0x%.4x\n",
> + dev_dbg(ps->dev, "<- addr: 0x%.2x reg: 0x%.2x val: 0x%.4x\n",
> addr, reg, ret);
>
> return ret;
> }
>
> -int mv88e6xxx_reg_read(struct dsa_switch *ds, int addr, int reg)
> +int mv88e6xxx_reg_read(struct mv88e6xxx_priv_state *ps, int addr, int reg)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> int ret;
>
> mutex_lock(&ps->smi_mutex);
> - ret = _mv88e6xxx_reg_read(ds, addr, reg);
> + ret = _mv88e6xxx_reg_read(ps, addr, reg);
> mutex_unlock(&ps->smi_mutex);
>
> return ret;
> @@ -153,26 +150,24 @@ static int __mv88e6xxx_reg_write(struct mii_bus *bus, int sw_addr, int addr,
> return 0;
> }
>
> -static int _mv88e6xxx_reg_write(struct dsa_switch *ds, int addr, int reg,
> - u16 val)
> +static int _mv88e6xxx_reg_write(struct mv88e6xxx_priv_state *ps, int addr,
> + int reg, u16 val)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> + assert_smi_lock(ps);
>
> - assert_smi_lock(ds);
> -
> - dev_dbg(ds->master_dev, "-> addr: 0x%.2x reg: 0x%.2x val: 0x%.4x\n",
> + dev_dbg(ps->dev, "-> addr: 0x%.2x reg: 0x%.2x val: 0x%.4x\n",
> addr, reg, val);
>
> return __mv88e6xxx_reg_write(ps->bus, ps->sw_addr, addr, reg, val);
> }
>
> -int mv88e6xxx_reg_write(struct dsa_switch *ds, int addr, int reg, u16 val)
> +int mv88e6xxx_reg_write(struct mv88e6xxx_priv_state *ps, int addr,
> + int reg, u16 val)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> int ret;
>
> mutex_lock(&ps->smi_mutex);
> - ret = _mv88e6xxx_reg_write(ds, addr, reg, val);
> + ret = _mv88e6xxx_reg_write(ps, addr, reg, val);
> mutex_unlock(&ps->smi_mutex);
>
> return ret;
> @@ -180,24 +175,26 @@ int mv88e6xxx_reg_write(struct dsa_switch *ds, int addr, int reg, u16 val)
>
> int mv88e6xxx_set_addr_direct(struct dsa_switch *ds, u8 *addr)
> {
> + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> int err;
>
> - err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_MAC_01,
> + err = mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_MAC_01,
> (addr[0] << 8) | addr[1]);
> if (err)
> return err;
>
> - err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_MAC_23,
> + err = mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_MAC_23,
> (addr[2] << 8) | addr[3]);
> if (err)
> return err;
>
> - return mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_MAC_45,
> + return mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_MAC_45,
> (addr[4] << 8) | addr[5]);
> }
>
> int mv88e6xxx_set_addr_indirect(struct dsa_switch *ds, u8 *addr)
> {
> + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> int ret;
> int i;
>
> @@ -205,7 +202,7 @@ int mv88e6xxx_set_addr_indirect(struct dsa_switch *ds, u8 *addr)
> int j;
>
> /* Write the MAC address byte. */
> - ret = mv88e6xxx_reg_write(ds, REG_GLOBAL2, GLOBAL2_SWITCH_MAC,
> + ret = mv88e6xxx_reg_write(ps, REG_GLOBAL2, GLOBAL2_SWITCH_MAC,
> GLOBAL2_SWITCH_MAC_BUSY |
> (i << 8) | addr[i]);
> if (ret)
> @@ -213,7 +210,7 @@ int mv88e6xxx_set_addr_indirect(struct dsa_switch *ds, u8 *addr)
>
> /* Wait for the write to complete. */
> for (j = 0; j < 16; j++) {
> - ret = mv88e6xxx_reg_read(ds, REG_GLOBAL2,
> + ret = mv88e6xxx_reg_read(ps, REG_GLOBAL2,
> GLOBAL2_SWITCH_MAC);
> if (ret < 0)
> return ret;
> @@ -228,39 +225,40 @@ int mv88e6xxx_set_addr_indirect(struct dsa_switch *ds, u8 *addr)
> return 0;
> }
>
> -static int _mv88e6xxx_phy_read(struct dsa_switch *ds, int addr, int regnum)
> +static int _mv88e6xxx_phy_read(struct mv88e6xxx_priv_state *ps, int addr,
> + int regnum)
> {
> if (addr >= 0)
> - return _mv88e6xxx_reg_read(ds, addr, regnum);
> + return _mv88e6xxx_reg_read(ps, addr, regnum);
> return 0xffff;
> }
>
> -static int _mv88e6xxx_phy_write(struct dsa_switch *ds, int addr, int regnum,
> - u16 val)
> +static int _mv88e6xxx_phy_write(struct mv88e6xxx_priv_state *ps, int addr,
> + int regnum, u16 val)
> {
> if (addr >= 0)
> - return _mv88e6xxx_reg_write(ds, addr, regnum, val);
> + return _mv88e6xxx_reg_write(ps, addr, regnum, val);
> return 0;
> }
>
> #ifdef CONFIG_NET_DSA_MV88E6XXX_NEED_PPU
> -static int mv88e6xxx_ppu_disable(struct dsa_switch *ds)
> +static int mv88e6xxx_ppu_disable(struct mv88e6xxx_priv_state *ps)
> {
> int ret;
> unsigned long timeout;
>
> - ret = mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_CONTROL);
> + ret = mv88e6xxx_reg_read(ps, REG_GLOBAL, GLOBAL_CONTROL);
> if (ret < 0)
> return ret;
>
> - ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL,
> + ret = mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_CONTROL,
> ret & ~GLOBAL_CONTROL_PPU_ENABLE);
> if (ret)
> return ret;
>
> timeout = jiffies + 1 * HZ;
> while (time_before(jiffies, timeout)) {
> - ret = mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_STATUS);
> + ret = mv88e6xxx_reg_read(ps, REG_GLOBAL, GLOBAL_STATUS);
> if (ret < 0)
> return ret;
>
> @@ -273,23 +271,23 @@ static int mv88e6xxx_ppu_disable(struct dsa_switch *ds)
> return -ETIMEDOUT;
> }
>
> -static int mv88e6xxx_ppu_enable(struct dsa_switch *ds)
> +static int mv88e6xxx_ppu_enable(struct mv88e6xxx_priv_state *ps)
> {
> int ret, err;
> unsigned long timeout;
>
> - ret = mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_CONTROL);
> + ret = mv88e6xxx_reg_read(ps, REG_GLOBAL, GLOBAL_CONTROL);
> if (ret < 0)
> return ret;
>
> - err = mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_CONTROL,
> + err = mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_CONTROL,
> ret | GLOBAL_CONTROL_PPU_ENABLE);
> if (err)
> return err;
>
> timeout = jiffies + 1 * HZ;
> while (time_before(jiffies, timeout)) {
> - ret = mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_STATUS);
> + ret = mv88e6xxx_reg_read(ps, REG_GLOBAL, GLOBAL_STATUS);
> if (ret < 0)
> return ret;
>
> @@ -308,9 +306,7 @@ static void mv88e6xxx_ppu_reenable_work(struct work_struct *ugly)
>
> ps = container_of(ugly, struct mv88e6xxx_priv_state, ppu_work);
> if (mutex_trylock(&ps->ppu_mutex)) {
> - struct dsa_switch *ds = ps->ds;
> -
> - if (mv88e6xxx_ppu_enable(ds) == 0)
> + if (mv88e6xxx_ppu_enable(ps) == 0)
> ps->ppu_disabled = 0;
> mutex_unlock(&ps->ppu_mutex);
> }
> @@ -323,9 +319,8 @@ static void mv88e6xxx_ppu_reenable_timer(unsigned long _ps)
> schedule_work(&ps->ppu_work);
> }
>
> -static int mv88e6xxx_ppu_access_get(struct dsa_switch *ds)
> +static int mv88e6xxx_ppu_access_get(struct mv88e6xxx_priv_state *ps)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> int ret;
>
> mutex_lock(&ps->ppu_mutex);
> @@ -336,7 +331,7 @@ static int mv88e6xxx_ppu_access_get(struct dsa_switch *ds)
> * it.
> */
> if (!ps->ppu_disabled) {
> - ret = mv88e6xxx_ppu_disable(ds);
> + ret = mv88e6xxx_ppu_disable(ps);
> if (ret < 0) {
> mutex_unlock(&ps->ppu_mutex);
> return ret;
> @@ -350,19 +345,15 @@ static int mv88e6xxx_ppu_access_get(struct dsa_switch *ds)
> return ret;
> }
>
> -static void mv88e6xxx_ppu_access_put(struct dsa_switch *ds)
> +static void mv88e6xxx_ppu_access_put(struct mv88e6xxx_priv_state *ps)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> -
> /* Schedule a timer to re-enable the PHY polling unit. */
> mod_timer(&ps->ppu_timer, jiffies + msecs_to_jiffies(10));
> mutex_unlock(&ps->ppu_mutex);
> }
>
> -void mv88e6xxx_ppu_state_init(struct dsa_switch *ds)
> +void mv88e6xxx_ppu_state_init(struct mv88e6xxx_priv_state *ps)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> -
> mutex_init(&ps->ppu_mutex);
> INIT_WORK(&ps->ppu_work, mv88e6xxx_ppu_reenable_work);
> init_timer(&ps->ppu_timer);
> @@ -372,12 +363,13 @@ void mv88e6xxx_ppu_state_init(struct dsa_switch *ds)
>
> int mv88e6xxx_phy_read_ppu(struct dsa_switch *ds, int addr, int regnum)
> {
> + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> int ret;
>
> - ret = mv88e6xxx_ppu_access_get(ds);
> + ret = mv88e6xxx_ppu_access_get(ps);
> if (ret >= 0) {
> - ret = mv88e6xxx_reg_read(ds, addr, regnum);
> - mv88e6xxx_ppu_access_put(ds);
> + ret = mv88e6xxx_reg_read(ps, addr, regnum);
> + mv88e6xxx_ppu_access_put(ps);
> }
>
> return ret;
> @@ -386,96 +378,79 @@ int mv88e6xxx_phy_read_ppu(struct dsa_switch *ds, int addr, int regnum)
> int mv88e6xxx_phy_write_ppu(struct dsa_switch *ds, int addr,
> int regnum, u16 val)
> {
> + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> int ret;
>
> - ret = mv88e6xxx_ppu_access_get(ds);
> + ret = mv88e6xxx_ppu_access_get(ps);
> if (ret >= 0) {
> - ret = mv88e6xxx_reg_write(ds, addr, regnum, val);
> - mv88e6xxx_ppu_access_put(ds);
> + ret = mv88e6xxx_reg_write(ps, addr, regnum, val);
> + mv88e6xxx_ppu_access_put(ps);
> }
>
> return ret;
> }
> #endif
>
> -static bool mv88e6xxx_6065_family(struct dsa_switch *ds)
> +static bool mv88e6xxx_6065_family(struct mv88e6xxx_priv_state *ps)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> -
> return ps->info->family == MV88E6XXX_FAMILY_6065;
> }
>
> -static bool mv88e6xxx_6095_family(struct dsa_switch *ds)
> +static bool mv88e6xxx_6095_family(struct mv88e6xxx_priv_state *ps)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> -
> return ps->info->family == MV88E6XXX_FAMILY_6095;
> }
>
> -static bool mv88e6xxx_6097_family(struct dsa_switch *ds)
> +static bool mv88e6xxx_6097_family(struct mv88e6xxx_priv_state *ps)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> -
> return ps->info->family == MV88E6XXX_FAMILY_6097;
> }
>
> -static bool mv88e6xxx_6165_family(struct dsa_switch *ds)
> +static bool mv88e6xxx_6165_family(struct mv88e6xxx_priv_state *ps)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> -
> return ps->info->family == MV88E6XXX_FAMILY_6165;
> }
>
> -static bool mv88e6xxx_6185_family(struct dsa_switch *ds)
> +static bool mv88e6xxx_6185_family(struct mv88e6xxx_priv_state *ps)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> -
> return ps->info->family == MV88E6XXX_FAMILY_6185;
> }
>
> -static bool mv88e6xxx_6320_family(struct dsa_switch *ds)
> +static bool mv88e6xxx_6320_family(struct mv88e6xxx_priv_state *ps)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> -
> return ps->info->family == MV88E6XXX_FAMILY_6320;
> }
>
> -static bool mv88e6xxx_6351_family(struct dsa_switch *ds)
> +static bool mv88e6xxx_6351_family(struct mv88e6xxx_priv_state *ps)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> -
> return ps->info->family == MV88E6XXX_FAMILY_6351;
> }
>
> -static bool mv88e6xxx_6352_family(struct dsa_switch *ds)
> +static bool mv88e6xxx_6352_family(struct mv88e6xxx_priv_state *ps)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> -
> return ps->info->family == MV88E6XXX_FAMILY_6352;
> }
>
> -static unsigned int mv88e6xxx_num_databases(struct dsa_switch *ds)
> +static unsigned int mv88e6xxx_num_databases(struct mv88e6xxx_priv_state *ps)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> -
> return ps->info->num_databases;
> }
>
> -static bool mv88e6xxx_has_fid_reg(struct dsa_switch *ds)
> +static bool mv88e6xxx_has_fid_reg(struct mv88e6xxx_priv_state *ps)
> {
> /* Does the device have dedicated FID registers for ATU and VTU ops? */
> - if (mv88e6xxx_6097_family(ds) || mv88e6xxx_6165_family(ds) ||
> - mv88e6xxx_6351_family(ds) || mv88e6xxx_6352_family(ds))
> + if (mv88e6xxx_6097_family(ps) || mv88e6xxx_6165_family(ps) ||
> + mv88e6xxx_6351_family(ps) || mv88e6xxx_6352_family(ps))
> return true;
>
> return false;
> }
>
> -static bool mv88e6xxx_has_stu(struct dsa_switch *ds)
> +static bool mv88e6xxx_has_stu(struct mv88e6xxx_priv_state *ps)
> {
> /* Does the device have STU and dedicated SID registers for VTU ops? */
> - if (mv88e6xxx_6097_family(ds) || mv88e6xxx_6165_family(ds) ||
> - mv88e6xxx_6351_family(ds) || mv88e6xxx_6352_family(ds))
> + if (mv88e6xxx_6097_family(ps) || mv88e6xxx_6165_family(ps) ||
> + mv88e6xxx_6351_family(ps) || mv88e6xxx_6352_family(ps))
> return true;
>
> return false;
> @@ -497,7 +472,7 @@ void mv88e6xxx_adjust_link(struct dsa_switch *ds, int port,
>
> mutex_lock(&ps->smi_mutex);
>
> - ret = _mv88e6xxx_reg_read(ds, REG_PORT(port), PORT_PCS_CTRL);
> + ret = _mv88e6xxx_reg_read(ps, REG_PORT(port), PORT_PCS_CTRL);
> if (ret < 0)
> goto out;
>
> @@ -511,7 +486,7 @@ void mv88e6xxx_adjust_link(struct dsa_switch *ds, int port,
> if (phydev->link)
> reg |= PORT_PCS_CTRL_LINK_UP;
>
> - if (mv88e6xxx_6065_family(ds) && phydev->speed > SPEED_100)
> + if (mv88e6xxx_6065_family(ps) && phydev->speed > SPEED_100)
> goto out;
>
> switch (phydev->speed) {
> @@ -533,7 +508,7 @@ void mv88e6xxx_adjust_link(struct dsa_switch *ds, int port,
> if (phydev->duplex == DUPLEX_FULL)
> reg |= PORT_PCS_CTRL_DUPLEX_FULL;
>
> - if ((mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds)) &&
> + if ((mv88e6xxx_6352_family(ps) || mv88e6xxx_6351_family(ps)) &&
> (port >= ps->info->num_ports - 2)) {
> if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
> reg |= PORT_PCS_CTRL_RGMII_DELAY_RXCLK;
> @@ -543,19 +518,19 @@ void mv88e6xxx_adjust_link(struct dsa_switch *ds, int port,
> reg |= (PORT_PCS_CTRL_RGMII_DELAY_RXCLK |
> PORT_PCS_CTRL_RGMII_DELAY_TXCLK);
> }
> - _mv88e6xxx_reg_write(ds, REG_PORT(port), PORT_PCS_CTRL, reg);
> + _mv88e6xxx_reg_write(ps, REG_PORT(port), PORT_PCS_CTRL, reg);
>
> out:
> mutex_unlock(&ps->smi_mutex);
> }
>
> -static int _mv88e6xxx_stats_wait(struct dsa_switch *ds)
> +static int _mv88e6xxx_stats_wait(struct mv88e6xxx_priv_state *ps)
> {
> int ret;
> int i;
>
> for (i = 0; i < 10; i++) {
> - ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_STATS_OP);
> + ret = _mv88e6xxx_reg_read(ps, REG_GLOBAL, GLOBAL_STATS_OP);
> if ((ret & GLOBAL_STATS_OP_BUSY) == 0)
> return 0;
> }
> @@ -563,52 +538,54 @@ static int _mv88e6xxx_stats_wait(struct dsa_switch *ds)
> return -ETIMEDOUT;
> }
>
> -static int _mv88e6xxx_stats_snapshot(struct dsa_switch *ds, int port)
> +static int _mv88e6xxx_stats_snapshot(struct mv88e6xxx_priv_state *ps,
> + int port)
> {
> int ret;
>
> - if (mv88e6xxx_6320_family(ds) || mv88e6xxx_6352_family(ds))
> + if (mv88e6xxx_6320_family(ps) || mv88e6xxx_6352_family(ps))
> port = (port + 1) << 5;
>
> /* Snapshot the hardware statistics counters for this port. */
> - ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_STATS_OP,
> + ret = _mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_STATS_OP,
> GLOBAL_STATS_OP_CAPTURE_PORT |
> GLOBAL_STATS_OP_HIST_RX_TX | port);
> if (ret < 0)
> return ret;
>
> /* Wait for the snapshotting to complete. */
> - ret = _mv88e6xxx_stats_wait(ds);
> + ret = _mv88e6xxx_stats_wait(ps);
> if (ret < 0)
> return ret;
>
> return 0;
> }
>
> -static void _mv88e6xxx_stats_read(struct dsa_switch *ds, int stat, u32 *val)
> +static void _mv88e6xxx_stats_read(struct mv88e6xxx_priv_state *ps,
> + int stat, u32 *val)
> {
> u32 _val;
> int ret;
>
> *val = 0;
>
> - ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_STATS_OP,
> + ret = _mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_STATS_OP,
> GLOBAL_STATS_OP_READ_CAPTURED |
> GLOBAL_STATS_OP_HIST_RX_TX | stat);
> if (ret < 0)
> return;
>
> - ret = _mv88e6xxx_stats_wait(ds);
> + ret = _mv88e6xxx_stats_wait(ps);
> if (ret < 0)
> return;
>
> - ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_STATS_COUNTER_32);
> + ret = _mv88e6xxx_reg_read(ps, REG_GLOBAL, GLOBAL_STATS_COUNTER_32);
> if (ret < 0)
> return;
>
> _val = ret << 16;
>
> - ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_STATS_COUNTER_01);
> + ret = _mv88e6xxx_reg_read(ps, REG_GLOBAL, GLOBAL_STATS_COUNTER_01);
> if (ret < 0)
> return;
>
> @@ -677,26 +654,26 @@ static struct mv88e6xxx_hw_stat mv88e6xxx_hw_stats[] = {
> { "out_management", 4, 0x1f | GLOBAL_STATS_OP_BANK_1, BANK1, },
> };
>
> -static bool mv88e6xxx_has_stat(struct dsa_switch *ds,
> +static bool mv88e6xxx_has_stat(struct mv88e6xxx_priv_state *ps,
> struct mv88e6xxx_hw_stat *stat)
> {
> switch (stat->type) {
> case BANK0:
> return true;
> case BANK1:
> - return mv88e6xxx_6320_family(ds);
> + return mv88e6xxx_6320_family(ps);
> case PORT:
> - return mv88e6xxx_6095_family(ds) ||
> - mv88e6xxx_6185_family(ds) ||
> - mv88e6xxx_6097_family(ds) ||
> - mv88e6xxx_6165_family(ds) ||
> - mv88e6xxx_6351_family(ds) ||
> - mv88e6xxx_6352_family(ds);
> + return mv88e6xxx_6095_family(ps) ||
> + mv88e6xxx_6185_family(ps) ||
> + mv88e6xxx_6097_family(ps) ||
> + mv88e6xxx_6165_family(ps) ||
> + mv88e6xxx_6351_family(ps) ||
> + mv88e6xxx_6352_family(ps);
> }
> return false;
> }
>
> -static uint64_t _mv88e6xxx_get_ethtool_stat(struct dsa_switch *ds,
> +static uint64_t _mv88e6xxx_get_ethtool_stat(struct mv88e6xxx_priv_state *ps,
> struct mv88e6xxx_hw_stat *s,
> int port)
> {
> @@ -707,13 +684,13 @@ static uint64_t _mv88e6xxx_get_ethtool_stat(struct dsa_switch *ds,
>
> switch (s->type) {
> case PORT:
> - ret = _mv88e6xxx_reg_read(ds, REG_PORT(port), s->reg);
> + ret = _mv88e6xxx_reg_read(ps, REG_PORT(port), s->reg);
> if (ret < 0)
> return UINT64_MAX;
>
> low = ret;
> if (s->sizeof_stat == 4) {
> - ret = _mv88e6xxx_reg_read(ds, REG_PORT(port),
> + ret = _mv88e6xxx_reg_read(ps, REG_PORT(port),
> s->reg + 1);
> if (ret < 0)
> return UINT64_MAX;
> @@ -722,9 +699,9 @@ static uint64_t _mv88e6xxx_get_ethtool_stat(struct dsa_switch *ds,
> break;
> case BANK0:
> case BANK1:
> - _mv88e6xxx_stats_read(ds, s->reg, &low);
> + _mv88e6xxx_stats_read(ps, s->reg, &low);
> if (s->sizeof_stat == 8)
> - _mv88e6xxx_stats_read(ds, s->reg + 1, &high);
> + _mv88e6xxx_stats_read(ps, s->reg + 1, &high);
> }
> value = (((u64)high) << 16) | low;
> return value;
> @@ -732,12 +709,13 @@ static uint64_t _mv88e6xxx_get_ethtool_stat(struct dsa_switch *ds,
>
> void mv88e6xxx_get_strings(struct dsa_switch *ds, int port, uint8_t *data)
> {
> + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> struct mv88e6xxx_hw_stat *stat;
> int i, j;
>
> for (i = 0, j = 0; i < ARRAY_SIZE(mv88e6xxx_hw_stats); i++) {
> stat = &mv88e6xxx_hw_stats[i];
> - if (mv88e6xxx_has_stat(ds, stat)) {
> + if (mv88e6xxx_has_stat(ps, stat)) {
> memcpy(data + j * ETH_GSTRING_LEN, stat->string,
> ETH_GSTRING_LEN);
> j++;
> @@ -747,12 +725,13 @@ void mv88e6xxx_get_strings(struct dsa_switch *ds, int port, uint8_t *data)
>
> int mv88e6xxx_get_sset_count(struct dsa_switch *ds)
> {
> + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> struct mv88e6xxx_hw_stat *stat;
> int i, j;
>
> for (i = 0, j = 0; i < ARRAY_SIZE(mv88e6xxx_hw_stats); i++) {
> stat = &mv88e6xxx_hw_stats[i];
> - if (mv88e6xxx_has_stat(ds, stat))
> + if (mv88e6xxx_has_stat(ps, stat))
> j++;
> }
> return j;
> @@ -769,15 +748,15 @@ mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds,
>
> mutex_lock(&ps->smi_mutex);
>
> - ret = _mv88e6xxx_stats_snapshot(ds, port);
> + ret = _mv88e6xxx_stats_snapshot(ps, port);
> if (ret < 0) {
> mutex_unlock(&ps->smi_mutex);
> return;
> }
> for (i = 0, j = 0; i < ARRAY_SIZE(mv88e6xxx_hw_stats); i++) {
> stat = &mv88e6xxx_hw_stats[i];
> - if (mv88e6xxx_has_stat(ds, stat)) {
> - data[j] = _mv88e6xxx_get_ethtool_stat(ds, stat, port);
> + if (mv88e6xxx_has_stat(ps, stat)) {
> + data[j] = _mv88e6xxx_get_ethtool_stat(ps, stat, port);
> j++;
> }
> }
> @@ -793,6 +772,7 @@ int mv88e6xxx_get_regs_len(struct dsa_switch *ds, int port)
> void mv88e6xxx_get_regs(struct dsa_switch *ds, int port,
> struct ethtool_regs *regs, void *_p)
> {
> + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> u16 *p = _p;
> int i;
>
> @@ -803,13 +783,13 @@ void mv88e6xxx_get_regs(struct dsa_switch *ds, int port,
> for (i = 0; i < 32; i++) {
> int ret;
>
> - ret = mv88e6xxx_reg_read(ds, REG_PORT(port), i);
> + ret = mv88e6xxx_reg_read(ps, REG_PORT(port), i);
> if (ret >= 0)
> p[i] = ret;
> }
> }
>
> -static int _mv88e6xxx_wait(struct dsa_switch *ds, int reg, int offset,
> +static int _mv88e6xxx_wait(struct mv88e6xxx_priv_state *ps, int reg, int offset,
> u16 mask)
> {
> unsigned long timeout = jiffies + HZ / 10;
> @@ -817,7 +797,7 @@ static int _mv88e6xxx_wait(struct dsa_switch *ds, int reg, int offset,
> while (time_before(jiffies, timeout)) {
> int ret;
>
> - ret = _mv88e6xxx_reg_read(ds, reg, offset);
> + ret = _mv88e6xxx_reg_read(ps, reg, offset);
> if (ret < 0)
> return ret;
> if (!(ret & mask))
> @@ -828,74 +808,80 @@ static int _mv88e6xxx_wait(struct dsa_switch *ds, int reg, int offset,
> return -ETIMEDOUT;
> }
>
> -static int mv88e6xxx_wait(struct dsa_switch *ds, int reg, int offset, u16 mask)
> +static int mv88e6xxx_wait(struct mv88e6xxx_priv_state *ps, int reg,
> + int offset, u16 mask)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> int ret;
>
> mutex_lock(&ps->smi_mutex);
> - ret = _mv88e6xxx_wait(ds, reg, offset, mask);
> + ret = _mv88e6xxx_wait(ps, reg, offset, mask);
> mutex_unlock(&ps->smi_mutex);
>
> return ret;
> }
>
> -static int _mv88e6xxx_phy_wait(struct dsa_switch *ds)
> +static int _mv88e6xxx_phy_wait(struct mv88e6xxx_priv_state *ps)
> {
> - return _mv88e6xxx_wait(ds, REG_GLOBAL2, GLOBAL2_SMI_OP,
> + return _mv88e6xxx_wait(ps, REG_GLOBAL2, GLOBAL2_SMI_OP,
> GLOBAL2_SMI_OP_BUSY);
> }
>
> int mv88e6xxx_eeprom_load_wait(struct dsa_switch *ds)
> {
> - return mv88e6xxx_wait(ds, REG_GLOBAL2, GLOBAL2_EEPROM_OP,
> + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> +
> + return mv88e6xxx_wait(ps, REG_GLOBAL2, GLOBAL2_EEPROM_OP,
> GLOBAL2_EEPROM_OP_LOAD);
> }
>
> int mv88e6xxx_eeprom_busy_wait(struct dsa_switch *ds)
> {
> - return mv88e6xxx_wait(ds, REG_GLOBAL2, GLOBAL2_EEPROM_OP,
> + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> +
> + return mv88e6xxx_wait(ps, REG_GLOBAL2, GLOBAL2_EEPROM_OP,
> GLOBAL2_EEPROM_OP_BUSY);
> }
>
> -static int _mv88e6xxx_atu_wait(struct dsa_switch *ds)
> +static int _mv88e6xxx_atu_wait(struct mv88e6xxx_priv_state *ps)
> {
> - return _mv88e6xxx_wait(ds, REG_GLOBAL, GLOBAL_ATU_OP,
> + return _mv88e6xxx_wait(ps, REG_GLOBAL, GLOBAL_ATU_OP,
> GLOBAL_ATU_OP_BUSY);
> }
>
> -static int _mv88e6xxx_phy_read_indirect(struct dsa_switch *ds, int addr,
> - int regnum)
> +static int _mv88e6xxx_phy_read_indirect(struct mv88e6xxx_priv_state *ps,
> + int addr, int regnum)
> {
> int ret;
>
> - ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL2, GLOBAL2_SMI_OP,
> + ret = _mv88e6xxx_reg_write(ps, REG_GLOBAL2, GLOBAL2_SMI_OP,
> GLOBAL2_SMI_OP_22_READ | (addr << 5) |
> regnum);
> if (ret < 0)
> return ret;
>
> - ret = _mv88e6xxx_phy_wait(ds);
> + ret = _mv88e6xxx_phy_wait(ps);
> if (ret < 0)
> return ret;
>
> - return _mv88e6xxx_reg_read(ds, REG_GLOBAL2, GLOBAL2_SMI_DATA);
> + ret = _mv88e6xxx_reg_read(ps, REG_GLOBAL2, GLOBAL2_SMI_DATA);
> +
> + return ret;
> }
>
> -static int _mv88e6xxx_phy_write_indirect(struct dsa_switch *ds, int addr,
> - int regnum, u16 val)
> +static int _mv88e6xxx_phy_write_indirect(struct mv88e6xxx_priv_state *ps,
> + int addr, int regnum, u16 val)
> {
> int ret;
>
> - ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL2, GLOBAL2_SMI_DATA, val);
> + ret = _mv88e6xxx_reg_write(ps, REG_GLOBAL2, GLOBAL2_SMI_DATA, val);
> if (ret < 0)
> return ret;
>
> - ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL2, GLOBAL2_SMI_OP,
> + ret = _mv88e6xxx_reg_write(ps, REG_GLOBAL2, GLOBAL2_SMI_OP,
> GLOBAL2_SMI_OP_22_WRITE | (addr << 5) |
> regnum);
>
> - return _mv88e6xxx_phy_wait(ds);
> + return _mv88e6xxx_phy_wait(ps);
> }
>
> int mv88e6xxx_get_eee(struct dsa_switch *ds, int port, struct ethtool_eee *e)
> @@ -905,14 +891,14 @@ int mv88e6xxx_get_eee(struct dsa_switch *ds, int port, struct ethtool_eee *e)
>
> mutex_lock(&ps->smi_mutex);
>
> - reg = _mv88e6xxx_phy_read_indirect(ds, port, 16);
> + reg = _mv88e6xxx_phy_read_indirect(ps, port, 16);
> if (reg < 0)
> goto out;
>
> e->eee_enabled = !!(reg & 0x0200);
> e->tx_lpi_enabled = !!(reg & 0x0100);
>
> - reg = _mv88e6xxx_reg_read(ds, REG_PORT(port), PORT_STATUS);
> + reg = _mv88e6xxx_reg_read(ps, REG_PORT(port), PORT_STATUS);
> if (reg < 0)
> goto out;
>
> @@ -933,7 +919,7 @@ int mv88e6xxx_set_eee(struct dsa_switch *ds, int port,
>
> mutex_lock(&ps->smi_mutex);
>
> - ret = _mv88e6xxx_phy_read_indirect(ds, port, 16);
> + ret = _mv88e6xxx_phy_read_indirect(ps, port, 16);
> if (ret < 0)
> goto out;
>
> @@ -943,28 +929,28 @@ int mv88e6xxx_set_eee(struct dsa_switch *ds, int port,
> if (e->tx_lpi_enabled)
> reg |= 0x0100;
>
> - ret = _mv88e6xxx_phy_write_indirect(ds, port, 16, reg);
> + ret = _mv88e6xxx_phy_write_indirect(ps, port, 16, reg);
> out:
> mutex_unlock(&ps->smi_mutex);
>
> return ret;
> }
>
> -static int _mv88e6xxx_atu_cmd(struct dsa_switch *ds, u16 fid, u16 cmd)
> +static int _mv88e6xxx_atu_cmd(struct mv88e6xxx_priv_state *ps, u16 fid, u16 cmd)
> {
> int ret;
>
> - if (mv88e6xxx_has_fid_reg(ds)) {
> - ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_ATU_FID, fid);
> + if (mv88e6xxx_has_fid_reg(ps)) {
> + ret = _mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_ATU_FID, fid);
> if (ret < 0)
> return ret;
> - } else if (mv88e6xxx_num_databases(ds) == 256) {
> + } else if (mv88e6xxx_num_databases(ps) == 256) {
> /* ATU DBNum[7:4] are located in ATU Control 15:12 */
> - ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_ATU_CONTROL);
> + ret = _mv88e6xxx_reg_read(ps, REG_GLOBAL, GLOBAL_ATU_CONTROL);
> if (ret < 0)
> return ret;
>
> - ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_ATU_CONTROL,
> + ret = _mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_ATU_CONTROL,
> (ret & 0xfff) |
> ((fid << 8) & 0xf000));
> if (ret < 0)
> @@ -974,14 +960,14 @@ static int _mv88e6xxx_atu_cmd(struct dsa_switch *ds, u16 fid, u16 cmd)
> cmd |= fid & 0xf;
> }
>
> - ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_ATU_OP, cmd);
> + ret = _mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_ATU_OP, cmd);
> if (ret < 0)
> return ret;
>
> - return _mv88e6xxx_atu_wait(ds);
> + return _mv88e6xxx_atu_wait(ps);
> }
>
> -static int _mv88e6xxx_atu_data_write(struct dsa_switch *ds,
> +static int _mv88e6xxx_atu_data_write(struct mv88e6xxx_priv_state *ps,
> struct mv88e6xxx_atu_entry *entry)
> {
> u16 data = entry->state & GLOBAL_ATU_DATA_STATE_MASK;
> @@ -1001,21 +987,21 @@ static int _mv88e6xxx_atu_data_write(struct dsa_switch *ds,
> data |= (entry->portv_trunkid << shift) & mask;
> }
>
> - return _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_ATU_DATA, data);
> + return _mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_ATU_DATA, data);
> }
>
> -static int _mv88e6xxx_atu_flush_move(struct dsa_switch *ds,
> +static int _mv88e6xxx_atu_flush_move(struct mv88e6xxx_priv_state *ps,
> struct mv88e6xxx_atu_entry *entry,
> bool static_too)
> {
> int op;
> int err;
>
> - err = _mv88e6xxx_atu_wait(ds);
> + err = _mv88e6xxx_atu_wait(ps);
> if (err)
> return err;
>
> - err = _mv88e6xxx_atu_data_write(ds, entry);
> + err = _mv88e6xxx_atu_data_write(ps, entry);
> if (err)
> return err;
>
> @@ -1027,21 +1013,22 @@ static int _mv88e6xxx_atu_flush_move(struct dsa_switch *ds,
> GLOBAL_ATU_OP_FLUSH_MOVE_NON_STATIC;
> }
>
> - return _mv88e6xxx_atu_cmd(ds, entry->fid, op);
> + return _mv88e6xxx_atu_cmd(ps, entry->fid, op);
> }
>
> -static int _mv88e6xxx_atu_flush(struct dsa_switch *ds, u16 fid, bool static_too)
> +static int _mv88e6xxx_atu_flush(struct mv88e6xxx_priv_state *ps,
> + u16 fid, bool static_too)
> {
> struct mv88e6xxx_atu_entry entry = {
> .fid = fid,
> .state = 0, /* EntryState bits must be 0 */
> };
>
> - return _mv88e6xxx_atu_flush_move(ds, &entry, static_too);
> + return _mv88e6xxx_atu_flush_move(ps, &entry, static_too);
> }
>
> -static int _mv88e6xxx_atu_move(struct dsa_switch *ds, u16 fid, int from_port,
> - int to_port, bool static_too)
> +static int _mv88e6xxx_atu_move(struct mv88e6xxx_priv_state *ps, u16 fid,
> + int from_port, int to_port, bool static_too)
> {
> struct mv88e6xxx_atu_entry entry = {
> .trunk = false,
> @@ -1055,14 +1042,14 @@ static int _mv88e6xxx_atu_move(struct dsa_switch *ds, u16 fid, int from_port,
> entry.portv_trunkid = (to_port & 0x0f) << 4;
> entry.portv_trunkid |= from_port & 0x0f;
>
> - return _mv88e6xxx_atu_flush_move(ds, &entry, static_too);
> + return _mv88e6xxx_atu_flush_move(ps, &entry, static_too);
> }
>
> -static int _mv88e6xxx_atu_remove(struct dsa_switch *ds, u16 fid, int port,
> - bool static_too)
> +static int _mv88e6xxx_atu_remove(struct mv88e6xxx_priv_state *ps, u16 fid,
> + int port, bool static_too)
> {
> /* Destination port 0xF means remove the entries */
> - return _mv88e6xxx_atu_move(ds, fid, port, 0x0f, static_too);
> + return _mv88e6xxx_atu_move(ps, fid, port, 0x0f, static_too);
> }
>
> static const char * const mv88e6xxx_port_state_names[] = {
> @@ -1072,12 +1059,14 @@ static const char * const mv88e6xxx_port_state_names[] = {
> [PORT_CONTROL_STATE_FORWARDING] = "Forwarding",
> };
>
> -static int _mv88e6xxx_port_state(struct dsa_switch *ds, int port, u8 state)
> +static int _mv88e6xxx_port_state(struct mv88e6xxx_priv_state *ps, int port,
> + u8 state)
> {
> + struct dsa_switch *ds = ps->ds;
> int reg, ret = 0;
> u8 oldstate;
>
> - reg = _mv88e6xxx_reg_read(ds, REG_PORT(port), PORT_CONTROL);
> + reg = _mv88e6xxx_reg_read(ps, REG_PORT(port), PORT_CONTROL);
> if (reg < 0)
> return reg;
>
> @@ -1092,13 +1081,13 @@ static int _mv88e6xxx_port_state(struct dsa_switch *ds, int port, u8 state)
> oldstate == PORT_CONTROL_STATE_FORWARDING)
> && (state == PORT_CONTROL_STATE_DISABLED ||
> state == PORT_CONTROL_STATE_BLOCKING)) {
> - ret = _mv88e6xxx_atu_remove(ds, 0, port, false);
> + ret = _mv88e6xxx_atu_remove(ps, 0, port, false);
> if (ret)
> return ret;
> }
>
> reg = (reg & ~PORT_CONTROL_STATE_MASK) | state;
> - ret = _mv88e6xxx_reg_write(ds, REG_PORT(port), PORT_CONTROL,
> + ret = _mv88e6xxx_reg_write(ps, REG_PORT(port), PORT_CONTROL,
> reg);
> if (ret)
> return ret;
> @@ -1111,11 +1100,12 @@ static int _mv88e6xxx_port_state(struct dsa_switch *ds, int port, u8 state)
> return ret;
> }
>
> -static int _mv88e6xxx_port_based_vlan_map(struct dsa_switch *ds, int port)
> +static int _mv88e6xxx_port_based_vlan_map(struct mv88e6xxx_priv_state *ps,
> + int port)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> struct net_device *bridge = ps->ports[port].bridge_dev;
> const u16 mask = (1 << ps->info->num_ports) - 1;
> + struct dsa_switch *ds = ps->ds;
> u16 output_ports = 0;
> int reg;
> int i;
> @@ -1138,14 +1128,14 @@ static int _mv88e6xxx_port_based_vlan_map(struct dsa_switch *ds, int port)
> /* prevent frames from going back out of the port they came in on */
> output_ports &= ~BIT(port);
>
> - reg = _mv88e6xxx_reg_read(ds, REG_PORT(port), PORT_BASE_VLAN);
> + reg = _mv88e6xxx_reg_read(ps, REG_PORT(port), PORT_BASE_VLAN);
> if (reg < 0)
> return reg;
>
> reg &= ~mask;
> reg |= output_ports & mask;
>
> - return _mv88e6xxx_reg_write(ds, REG_PORT(port), PORT_BASE_VLAN, reg);
> + return _mv88e6xxx_reg_write(ps, REG_PORT(port), PORT_BASE_VLAN, reg);
> }
>
> void mv88e6xxx_port_stp_state_set(struct dsa_switch *ds, int port, u8 state)
> @@ -1178,13 +1168,14 @@ void mv88e6xxx_port_stp_state_set(struct dsa_switch *ds, int port, u8 state)
> schedule_work(&ps->bridge_work);
> }
>
> -static int _mv88e6xxx_port_pvid(struct dsa_switch *ds, int port, u16 *new,
> - u16 *old)
> +static int _mv88e6xxx_port_pvid(struct mv88e6xxx_priv_state *ps, int port,
> + u16 *new, u16 *old)
> {
> + struct dsa_switch *ds = ps->ds;
> u16 pvid;
> int ret;
>
> - ret = _mv88e6xxx_reg_read(ds, REG_PORT(port), PORT_DEFAULT_VLAN);
> + ret = _mv88e6xxx_reg_read(ps, REG_PORT(port), PORT_DEFAULT_VLAN);
> if (ret < 0)
> return ret;
>
> @@ -1194,7 +1185,7 @@ static int _mv88e6xxx_port_pvid(struct dsa_switch *ds, int port, u16 *new,
> ret &= ~PORT_DEFAULT_VLAN_MASK;
> ret |= *new & PORT_DEFAULT_VLAN_MASK;
>
> - ret = _mv88e6xxx_reg_write(ds, REG_PORT(port),
> + ret = _mv88e6xxx_reg_write(ps, REG_PORT(port),
> PORT_DEFAULT_VLAN, ret);
> if (ret < 0)
> return ret;
> @@ -1209,55 +1200,56 @@ static int _mv88e6xxx_port_pvid(struct dsa_switch *ds, int port, u16 *new,
> return 0;
> }
>
> -static int _mv88e6xxx_port_pvid_get(struct dsa_switch *ds, int port, u16 *pvid)
> +static int _mv88e6xxx_port_pvid_get(struct mv88e6xxx_priv_state *ps,
> + int port, u16 *pvid)
> {
> - return _mv88e6xxx_port_pvid(ds, port, NULL, pvid);
> + return _mv88e6xxx_port_pvid(ps, port, NULL, pvid);
> }
>
> -static int _mv88e6xxx_port_pvid_set(struct dsa_switch *ds, int port, u16 pvid)
> +static int _mv88e6xxx_port_pvid_set(struct mv88e6xxx_priv_state *ps,
> + int port, u16 pvid)
> {
> - return _mv88e6xxx_port_pvid(ds, port, &pvid, NULL);
> + return _mv88e6xxx_port_pvid(ps, port, &pvid, NULL);
> }
>
> -static int _mv88e6xxx_vtu_wait(struct dsa_switch *ds)
> +static int _mv88e6xxx_vtu_wait(struct mv88e6xxx_priv_state *ps)
> {
> - return _mv88e6xxx_wait(ds, REG_GLOBAL, GLOBAL_VTU_OP,
> + return _mv88e6xxx_wait(ps, REG_GLOBAL, GLOBAL_VTU_OP,
> GLOBAL_VTU_OP_BUSY);
> }
>
> -static int _mv88e6xxx_vtu_cmd(struct dsa_switch *ds, u16 op)
> +static int _mv88e6xxx_vtu_cmd(struct mv88e6xxx_priv_state *ps, u16 op)
> {
> int ret;
>
> - ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_OP, op);
> + ret = _mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_VTU_OP, op);
> if (ret < 0)
> return ret;
>
> - return _mv88e6xxx_vtu_wait(ds);
> + return _mv88e6xxx_vtu_wait(ps);
> }
>
> -static int _mv88e6xxx_vtu_stu_flush(struct dsa_switch *ds)
> +static int _mv88e6xxx_vtu_stu_flush(struct mv88e6xxx_priv_state *ps)
> {
> int ret;
>
> - ret = _mv88e6xxx_vtu_wait(ds);
> + ret = _mv88e6xxx_vtu_wait(ps);
> if (ret < 0)
> return ret;
>
> - return _mv88e6xxx_vtu_cmd(ds, GLOBAL_VTU_OP_FLUSH_ALL);
> + return _mv88e6xxx_vtu_cmd(ps, GLOBAL_VTU_OP_FLUSH_ALL);
> }
>
> -static int _mv88e6xxx_vtu_stu_data_read(struct dsa_switch *ds,
> +static int _mv88e6xxx_vtu_stu_data_read(struct mv88e6xxx_priv_state *ps,
> struct mv88e6xxx_vtu_stu_entry *entry,
> unsigned int nibble_offset)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> u16 regs[3];
> int i;
> int ret;
>
> for (i = 0; i < 3; ++i) {
> - ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL,
> + ret = _mv88e6xxx_reg_read(ps, REG_GLOBAL,
> GLOBAL_VTU_DATA_0_3 + i);
> if (ret < 0)
> return ret;
> @@ -1275,11 +1267,10 @@ static int _mv88e6xxx_vtu_stu_data_read(struct dsa_switch *ds,
> return 0;
> }
>
> -static int _mv88e6xxx_vtu_stu_data_write(struct dsa_switch *ds,
> +static int _mv88e6xxx_vtu_stu_data_write(struct mv88e6xxx_priv_state *ps,
> struct mv88e6xxx_vtu_stu_entry *entry,
> unsigned int nibble_offset)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> u16 regs[3] = { 0 };
> int i;
> int ret;
> @@ -1292,7 +1283,7 @@ static int _mv88e6xxx_vtu_stu_data_write(struct dsa_switch *ds,
> }
>
> for (i = 0; i < 3; ++i) {
> - ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL,
> + ret = _mv88e6xxx_reg_write(ps, REG_GLOBAL,
> GLOBAL_VTU_DATA_0_3 + i, regs[i]);
> if (ret < 0)
> return ret;
> @@ -1301,27 +1292,27 @@ static int _mv88e6xxx_vtu_stu_data_write(struct dsa_switch *ds,
> return 0;
> }
>
> -static int _mv88e6xxx_vtu_vid_write(struct dsa_switch *ds, u16 vid)
> +static int _mv88e6xxx_vtu_vid_write(struct mv88e6xxx_priv_state *ps, u16 vid)
> {
> - return _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_VID,
> + return _mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_VTU_VID,
> vid & GLOBAL_VTU_VID_MASK);
> }
>
> -static int _mv88e6xxx_vtu_getnext(struct dsa_switch *ds,
> +static int _mv88e6xxx_vtu_getnext(struct mv88e6xxx_priv_state *ps,
> struct mv88e6xxx_vtu_stu_entry *entry)
> {
> struct mv88e6xxx_vtu_stu_entry next = { 0 };
> int ret;
>
> - ret = _mv88e6xxx_vtu_wait(ds);
> + ret = _mv88e6xxx_vtu_wait(ps);
> if (ret < 0)
> return ret;
>
> - ret = _mv88e6xxx_vtu_cmd(ds, GLOBAL_VTU_OP_VTU_GET_NEXT);
> + ret = _mv88e6xxx_vtu_cmd(ps, GLOBAL_VTU_OP_VTU_GET_NEXT);
> if (ret < 0)
> return ret;
>
> - ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_VTU_VID);
> + ret = _mv88e6xxx_reg_read(ps, REG_GLOBAL, GLOBAL_VTU_VID);
> if (ret < 0)
> return ret;
>
> @@ -1329,22 +1320,22 @@ static int _mv88e6xxx_vtu_getnext(struct dsa_switch *ds,
> next.valid = !!(ret & GLOBAL_VTU_VID_VALID);
>
> if (next.valid) {
> - ret = _mv88e6xxx_vtu_stu_data_read(ds, &next, 0);
> + ret = _mv88e6xxx_vtu_stu_data_read(ps, &next, 0);
> if (ret < 0)
> return ret;
>
> - if (mv88e6xxx_has_fid_reg(ds)) {
> - ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL,
> + if (mv88e6xxx_has_fid_reg(ps)) {
> + ret = _mv88e6xxx_reg_read(ps, REG_GLOBAL,
> GLOBAL_VTU_FID);
> if (ret < 0)
> return ret;
>
> next.fid = ret & GLOBAL_VTU_FID_MASK;
> - } else if (mv88e6xxx_num_databases(ds) == 256) {
> + } else if (mv88e6xxx_num_databases(ps) == 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(ds, REG_GLOBAL,
> + ret = _mv88e6xxx_reg_read(ps, REG_GLOBAL,
> GLOBAL_VTU_OP);
> if (ret < 0)
> return ret;
> @@ -1353,8 +1344,8 @@ static int _mv88e6xxx_vtu_getnext(struct dsa_switch *ds,
> next.fid |= ret & 0xf;
> }
>
> - if (mv88e6xxx_has_stu(ds)) {
> - ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL,
> + if (mv88e6xxx_has_stu(ps)) {
> + ret = _mv88e6xxx_reg_read(ps, REG_GLOBAL,
> GLOBAL_VTU_SID);
> if (ret < 0)
> return ret;
> @@ -1378,16 +1369,16 @@ int mv88e6xxx_port_vlan_dump(struct dsa_switch *ds, int port,
>
> mutex_lock(&ps->smi_mutex);
>
> - err = _mv88e6xxx_port_pvid_get(ds, port, &pvid);
> + err = _mv88e6xxx_port_pvid_get(ps, port, &pvid);
> if (err)
> goto unlock;
>
> - err = _mv88e6xxx_vtu_vid_write(ds, GLOBAL_VTU_VID_MASK);
> + err = _mv88e6xxx_vtu_vid_write(ps, GLOBAL_VTU_VID_MASK);
> if (err)
> goto unlock;
>
> do {
> - err = _mv88e6xxx_vtu_getnext(ds, &next);
> + err = _mv88e6xxx_vtu_getnext(ps, &next);
> if (err)
> break;
>
> @@ -1418,14 +1409,14 @@ unlock:
> return err;
> }
>
> -static int _mv88e6xxx_vtu_loadpurge(struct dsa_switch *ds,
> +static int _mv88e6xxx_vtu_loadpurge(struct mv88e6xxx_priv_state *ps,
> struct mv88e6xxx_vtu_stu_entry *entry)
> {
> u16 op = GLOBAL_VTU_OP_VTU_LOAD_PURGE;
> u16 reg = 0;
> int ret;
>
> - ret = _mv88e6xxx_vtu_wait(ds);
> + ret = _mv88e6xxx_vtu_wait(ps);
> if (ret < 0)
> return ret;
>
> @@ -1433,23 +1424,23 @@ static int _mv88e6xxx_vtu_loadpurge(struct dsa_switch *ds,
> goto loadpurge;
>
> /* Write port member tags */
> - ret = _mv88e6xxx_vtu_stu_data_write(ds, entry, 0);
> + ret = _mv88e6xxx_vtu_stu_data_write(ps, entry, 0);
> if (ret < 0)
> return ret;
>
> - if (mv88e6xxx_has_stu(ds)) {
> + if (mv88e6xxx_has_stu(ps)) {
> reg = entry->sid & GLOBAL_VTU_SID_MASK;
> - ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_SID, reg);
> + ret = _mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_VTU_SID, reg);
> if (ret < 0)
> return ret;
> }
>
> - if (mv88e6xxx_has_fid_reg(ds)) {
> + if (mv88e6xxx_has_fid_reg(ps)) {
> reg = entry->fid & GLOBAL_VTU_FID_MASK;
> - ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_FID, reg);
> + ret = _mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_VTU_FID, reg);
> if (ret < 0)
> return ret;
> - } else if (mv88e6xxx_num_databases(ds) == 256) {
> + } else if (mv88e6xxx_num_databases(ps) == 256) {
> /* VTU DBNum[7:4] are located in VTU Operation 11:8, and
> * VTU DBNum[3:0] are located in VTU Operation 3:0
> */
> @@ -1460,46 +1451,46 @@ static int _mv88e6xxx_vtu_loadpurge(struct dsa_switch *ds,
> reg = GLOBAL_VTU_VID_VALID;
> loadpurge:
> reg |= entry->vid & GLOBAL_VTU_VID_MASK;
> - ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_VID, reg);
> + ret = _mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_VTU_VID, reg);
> if (ret < 0)
> return ret;
>
> - return _mv88e6xxx_vtu_cmd(ds, op);
> + return _mv88e6xxx_vtu_cmd(ps, op);
> }
>
> -static int _mv88e6xxx_stu_getnext(struct dsa_switch *ds, u8 sid,
> +static int _mv88e6xxx_stu_getnext(struct mv88e6xxx_priv_state *ps, u8 sid,
> struct mv88e6xxx_vtu_stu_entry *entry)
> {
> struct mv88e6xxx_vtu_stu_entry next = { 0 };
> int ret;
>
> - ret = _mv88e6xxx_vtu_wait(ds);
> + ret = _mv88e6xxx_vtu_wait(ps);
> if (ret < 0)
> return ret;
>
> - ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_SID,
> + ret = _mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_VTU_SID,
> sid & GLOBAL_VTU_SID_MASK);
> if (ret < 0)
> return ret;
>
> - ret = _mv88e6xxx_vtu_cmd(ds, GLOBAL_VTU_OP_STU_GET_NEXT);
> + ret = _mv88e6xxx_vtu_cmd(ps, GLOBAL_VTU_OP_STU_GET_NEXT);
> if (ret < 0)
> return ret;
>
> - ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_VTU_SID);
> + ret = _mv88e6xxx_reg_read(ps, REG_GLOBAL, GLOBAL_VTU_SID);
> if (ret < 0)
> return ret;
>
> next.sid = ret & GLOBAL_VTU_SID_MASK;
>
> - ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_VTU_VID);
> + ret = _mv88e6xxx_reg_read(ps, REG_GLOBAL, GLOBAL_VTU_VID);
> if (ret < 0)
> return ret;
>
> next.valid = !!(ret & GLOBAL_VTU_VID_VALID);
>
> if (next.valid) {
> - ret = _mv88e6xxx_vtu_stu_data_read(ds, &next, 2);
> + ret = _mv88e6xxx_vtu_stu_data_read(ps, &next, 2);
> if (ret < 0)
> return ret;
> }
> @@ -1508,13 +1499,13 @@ static int _mv88e6xxx_stu_getnext(struct dsa_switch *ds, u8 sid,
> return 0;
> }
>
> -static int _mv88e6xxx_stu_loadpurge(struct dsa_switch *ds,
> +static int _mv88e6xxx_stu_loadpurge(struct mv88e6xxx_priv_state *ps,
> struct mv88e6xxx_vtu_stu_entry *entry)
> {
> u16 reg = 0;
> int ret;
>
> - ret = _mv88e6xxx_vtu_wait(ds);
> + ret = _mv88e6xxx_vtu_wait(ps);
> if (ret < 0)
> return ret;
>
> @@ -1522,40 +1513,41 @@ static int _mv88e6xxx_stu_loadpurge(struct dsa_switch *ds,
> goto loadpurge;
>
> /* Write port states */
> - ret = _mv88e6xxx_vtu_stu_data_write(ds, entry, 2);
> + ret = _mv88e6xxx_vtu_stu_data_write(ps, entry, 2);
> if (ret < 0)
> return ret;
>
> reg = GLOBAL_VTU_VID_VALID;
> loadpurge:
> - ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_VID, reg);
> + ret = _mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_VTU_VID, reg);
> if (ret < 0)
> return ret;
>
> reg = entry->sid & GLOBAL_VTU_SID_MASK;
> - ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_SID, reg);
> + ret = _mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_VTU_SID, reg);
> if (ret < 0)
> return ret;
>
> - return _mv88e6xxx_vtu_cmd(ds, GLOBAL_VTU_OP_STU_LOAD_PURGE);
> + return _mv88e6xxx_vtu_cmd(ps, GLOBAL_VTU_OP_STU_LOAD_PURGE);
> }
>
> -static int _mv88e6xxx_port_fid(struct dsa_switch *ds, int port, u16 *new,
> - u16 *old)
> +static int _mv88e6xxx_port_fid(struct mv88e6xxx_priv_state *ps, int port,
> + u16 *new, u16 *old)
> {
> + struct dsa_switch *ds = ps->ds;
> u16 upper_mask;
> u16 fid;
> int ret;
>
> - if (mv88e6xxx_num_databases(ds) == 4096)
> + if (mv88e6xxx_num_databases(ps) == 4096)
> upper_mask = 0xff;
> - else if (mv88e6xxx_num_databases(ds) == 256)
> + else if (mv88e6xxx_num_databases(ps) == 256)
> upper_mask = 0xf;
> else
> return -EOPNOTSUPP;
>
> /* Port's default FID bits 3:0 are located in reg 0x06, offset 12 */
> - ret = _mv88e6xxx_reg_read(ds, REG_PORT(port), PORT_BASE_VLAN);
> + ret = _mv88e6xxx_reg_read(ps, REG_PORT(port), PORT_BASE_VLAN);
> if (ret < 0)
> return ret;
>
> @@ -1565,14 +1557,14 @@ static int _mv88e6xxx_port_fid(struct dsa_switch *ds, int port, u16 *new,
> ret &= ~PORT_BASE_VLAN_FID_3_0_MASK;
> ret |= (*new << 12) & PORT_BASE_VLAN_FID_3_0_MASK;
>
> - ret = _mv88e6xxx_reg_write(ds, REG_PORT(port), PORT_BASE_VLAN,
> + ret = _mv88e6xxx_reg_write(ps, REG_PORT(port), PORT_BASE_VLAN,
> ret);
> if (ret < 0)
> return ret;
> }
>
> /* Port's default FID bits 11:4 are located in reg 0x05, offset 0 */
> - ret = _mv88e6xxx_reg_read(ds, REG_PORT(port), PORT_CONTROL_1);
> + ret = _mv88e6xxx_reg_read(ps, REG_PORT(port), PORT_CONTROL_1);
> if (ret < 0)
> return ret;
>
> @@ -1582,7 +1574,7 @@ static int _mv88e6xxx_port_fid(struct dsa_switch *ds, int port, u16 *new,
> ret &= ~upper_mask;
> ret |= (*new >> 4) & upper_mask;
>
> - ret = _mv88e6xxx_reg_write(ds, REG_PORT(port), PORT_CONTROL_1,
> + ret = _mv88e6xxx_reg_write(ps, REG_PORT(port), PORT_CONTROL_1,
> ret);
> if (ret < 0)
> return ret;
> @@ -1596,19 +1588,20 @@ static int _mv88e6xxx_port_fid(struct dsa_switch *ds, int port, u16 *new,
> return 0;
> }
>
> -static int _mv88e6xxx_port_fid_get(struct dsa_switch *ds, int port, u16 *fid)
> +static int _mv88e6xxx_port_fid_get(struct mv88e6xxx_priv_state *ps,
> + int port, u16 *fid)
> {
> - return _mv88e6xxx_port_fid(ds, port, NULL, fid);
> + return _mv88e6xxx_port_fid(ps, port, NULL, fid);
> }
>
> -static int _mv88e6xxx_port_fid_set(struct dsa_switch *ds, int port, u16 fid)
> +static int _mv88e6xxx_port_fid_set(struct mv88e6xxx_priv_state *ps,
> + int port, u16 fid)
> {
> - return _mv88e6xxx_port_fid(ds, port, &fid, NULL);
> + return _mv88e6xxx_port_fid(ps, port, &fid, NULL);
> }
>
> -static int _mv88e6xxx_fid_new(struct dsa_switch *ds, u16 *fid)
> +static int _mv88e6xxx_fid_new(struct mv88e6xxx_priv_state *ps, u16 *fid)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> DECLARE_BITMAP(fid_bitmap, MV88E6XXX_N_FID);
> struct mv88e6xxx_vtu_stu_entry vlan;
> int i, err;
> @@ -1617,7 +1610,7 @@ static int _mv88e6xxx_fid_new(struct dsa_switch *ds, u16 *fid)
>
> /* Set every FID bit used by the (un)bridged ports */
> for (i = 0; i < ps->info->num_ports; ++i) {
> - err = _mv88e6xxx_port_fid_get(ds, i, fid);
> + err = _mv88e6xxx_port_fid_get(ps, i, fid);
> if (err)
> return err;
>
> @@ -1625,12 +1618,12 @@ static int _mv88e6xxx_fid_new(struct dsa_switch *ds, u16 *fid)
> }
>
> /* Set every FID bit used by the VLAN entries */
> - err = _mv88e6xxx_vtu_vid_write(ds, GLOBAL_VTU_VID_MASK);
> + err = _mv88e6xxx_vtu_vid_write(ps, GLOBAL_VTU_VID_MASK);
> if (err)
> return err;
>
> do {
> - err = _mv88e6xxx_vtu_getnext(ds, &vlan);
> + err = _mv88e6xxx_vtu_getnext(ps, &vlan);
> if (err)
> return err;
>
> @@ -1644,24 +1637,24 @@ static int _mv88e6xxx_fid_new(struct dsa_switch *ds, u16 *fid)
> * databases are not needed. Return the next positive available.
> */
> *fid = find_next_zero_bit(fid_bitmap, MV88E6XXX_N_FID, 1);
> - if (unlikely(*fid >= mv88e6xxx_num_databases(ds)))
> + if (unlikely(*fid >= mv88e6xxx_num_databases(ps)))
> return -ENOSPC;
>
> /* Clear the database */
> - return _mv88e6xxx_atu_flush(ds, *fid, true);
> + return _mv88e6xxx_atu_flush(ps, *fid, true);
> }
>
> -static int _mv88e6xxx_vtu_new(struct dsa_switch *ds, u16 vid,
> +static int _mv88e6xxx_vtu_new(struct mv88e6xxx_priv_state *ps, u16 vid,
> struct mv88e6xxx_vtu_stu_entry *entry)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> + struct dsa_switch *ds = ps->ds;
> struct mv88e6xxx_vtu_stu_entry vlan = {
> .valid = true,
> .vid = vid,
> };
> int i, err;
>
> - err = _mv88e6xxx_fid_new(ds, &vlan.fid);
> + err = _mv88e6xxx_fid_new(ps, &vlan.fid);
> if (err)
> return err;
>
> @@ -1671,8 +1664,8 @@ static int _mv88e6xxx_vtu_new(struct dsa_switch *ds, u16 vid,
> ? GLOBAL_VTU_DATA_MEMBER_TAG_UNMODIFIED
> : GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER;
>
> - if (mv88e6xxx_6097_family(ds) || mv88e6xxx_6165_family(ds) ||
> - mv88e6xxx_6351_family(ds) || mv88e6xxx_6352_family(ds)) {
> + if (mv88e6xxx_6097_family(ps) || mv88e6xxx_6165_family(ps) ||
> + mv88e6xxx_6351_family(ps) || mv88e6xxx_6352_family(ps)) {
> struct mv88e6xxx_vtu_stu_entry vstp;
>
> /* Adding a VTU entry requires a valid STU entry. As VSTP is not
> @@ -1680,7 +1673,7 @@ static int _mv88e6xxx_vtu_new(struct dsa_switch *ds, u16 vid,
> * entries. Thus, validate the SID 0.
> */
> vlan.sid = 0;
> - err = _mv88e6xxx_stu_getnext(ds, GLOBAL_VTU_SID_MASK, &vstp);
> + err = _mv88e6xxx_stu_getnext(ps, GLOBAL_VTU_SID_MASK, &vstp);
> if (err)
> return err;
>
> @@ -1689,7 +1682,7 @@ static int _mv88e6xxx_vtu_new(struct dsa_switch *ds, u16 vid,
> vstp.valid = true;
> vstp.sid = vlan.sid;
>
> - err = _mv88e6xxx_stu_loadpurge(ds, &vstp);
> + err = _mv88e6xxx_stu_loadpurge(ps, &vstp);
> if (err)
> return err;
> }
> @@ -1699,7 +1692,7 @@ static int _mv88e6xxx_vtu_new(struct dsa_switch *ds, u16 vid,
> return 0;
> }
>
> -static int _mv88e6xxx_vtu_get(struct dsa_switch *ds, u16 vid,
> +static int _mv88e6xxx_vtu_get(struct mv88e6xxx_priv_state *ps, u16 vid,
> struct mv88e6xxx_vtu_stu_entry *entry, bool creat)
> {
> int err;
> @@ -1707,11 +1700,11 @@ static int _mv88e6xxx_vtu_get(struct dsa_switch *ds, u16 vid,
> if (!vid)
> return -EINVAL;
>
> - err = _mv88e6xxx_vtu_vid_write(ds, vid - 1);
> + err = _mv88e6xxx_vtu_vid_write(ps, vid - 1);
> if (err)
> return err;
>
> - err = _mv88e6xxx_vtu_getnext(ds, entry);
> + err = _mv88e6xxx_vtu_getnext(ps, entry);
> if (err)
> return err;
>
> @@ -1722,7 +1715,7 @@ static int _mv88e6xxx_vtu_get(struct dsa_switch *ds, u16 vid,
> * -EOPNOTSUPP to inform bridge about an eventual software VLAN.
> */
>
> - err = _mv88e6xxx_vtu_new(ds, vid, entry);
> + err = _mv88e6xxx_vtu_new(ps, vid, entry);
> }
>
> return err;
> @@ -1740,12 +1733,12 @@ static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
>
> mutex_lock(&ps->smi_mutex);
>
> - err = _mv88e6xxx_vtu_vid_write(ds, vid_begin - 1);
> + err = _mv88e6xxx_vtu_vid_write(ps, vid_begin - 1);
> if (err)
> goto unlock;
>
> do {
> - err = _mv88e6xxx_vtu_getnext(ds, &vlan);
> + err = _mv88e6xxx_vtu_getnext(ps, &vlan);
> if (err)
> goto unlock;
>
> @@ -1799,7 +1792,7 @@ int mv88e6xxx_port_vlan_filtering(struct dsa_switch *ds, int port,
>
> mutex_lock(&ps->smi_mutex);
>
> - ret = _mv88e6xxx_reg_read(ds, REG_PORT(port), PORT_CONTROL_2);
> + ret = _mv88e6xxx_reg_read(ps, REG_PORT(port), PORT_CONTROL_2);
> if (ret < 0)
> goto unlock;
>
> @@ -1809,7 +1802,7 @@ int mv88e6xxx_port_vlan_filtering(struct dsa_switch *ds, int port,
> ret &= ~PORT_CONTROL_2_8021Q_MASK;
> ret |= new & PORT_CONTROL_2_8021Q_MASK;
>
> - ret = _mv88e6xxx_reg_write(ds, REG_PORT(port), PORT_CONTROL_2,
> + ret = _mv88e6xxx_reg_write(ps, REG_PORT(port), PORT_CONTROL_2,
> ret);
> if (ret < 0)
> goto unlock;
> @@ -1846,13 +1839,13 @@ int mv88e6xxx_port_vlan_prepare(struct dsa_switch *ds, int port,
> return 0;
> }
>
> -static int _mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port, u16 vid,
> - bool untagged)
> +static int _mv88e6xxx_port_vlan_add(struct mv88e6xxx_priv_state *ps, int port,
> + u16 vid, bool untagged)
> {
> struct mv88e6xxx_vtu_stu_entry vlan;
> int err;
>
> - err = _mv88e6xxx_vtu_get(ds, vid, &vlan, true);
> + err = _mv88e6xxx_vtu_get(ps, vid, &vlan, true);
> if (err)
> return err;
>
> @@ -1860,7 +1853,7 @@ static int _mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port, u16 vid,
> GLOBAL_VTU_DATA_MEMBER_TAG_UNTAGGED :
> GLOBAL_VTU_DATA_MEMBER_TAG_TAGGED;
>
> - return _mv88e6xxx_vtu_loadpurge(ds, &vlan);
> + return _mv88e6xxx_vtu_loadpurge(ps, &vlan);
> }
>
> void mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port,
> @@ -1875,24 +1868,25 @@ void mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port,
> mutex_lock(&ps->smi_mutex);
>
> for (vid = vlan->vid_begin; vid <= vlan->vid_end; ++vid)
> - if (_mv88e6xxx_port_vlan_add(ds, port, vid, untagged))
> + if (_mv88e6xxx_port_vlan_add(ps, port, vid, untagged))
> netdev_err(ds->ports[port], "failed to add VLAN %d%c\n",
> vid, untagged ? 'u' : 't');
>
> - if (pvid && _mv88e6xxx_port_pvid_set(ds, port, vlan->vid_end))
> + if (pvid && _mv88e6xxx_port_pvid_set(ps, port, vlan->vid_end))
> netdev_err(ds->ports[port], "failed to set PVID %d\n",
> vlan->vid_end);
>
> mutex_unlock(&ps->smi_mutex);
> }
>
> -static int _mv88e6xxx_port_vlan_del(struct dsa_switch *ds, int port, u16 vid)
> +static int _mv88e6xxx_port_vlan_del(struct mv88e6xxx_priv_state *ps,
> + int port, u16 vid)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> + struct dsa_switch *ds = ps->ds;
> struct mv88e6xxx_vtu_stu_entry vlan;
> int i, err;
>
> - err = _mv88e6xxx_vtu_get(ds, vid, &vlan, false);
> + err = _mv88e6xxx_vtu_get(ps, vid, &vlan, false);
> if (err)
> return err;
>
> @@ -1914,11 +1908,11 @@ static int _mv88e6xxx_port_vlan_del(struct dsa_switch *ds, int port, u16 vid)
> }
> }
>
> - err = _mv88e6xxx_vtu_loadpurge(ds, &vlan);
> + err = _mv88e6xxx_vtu_loadpurge(ps, &vlan);
> if (err)
> return err;
>
> - return _mv88e6xxx_atu_remove(ds, vlan.fid, port, false);
> + return _mv88e6xxx_atu_remove(ps, vlan.fid, port, false);
> }
>
> int mv88e6xxx_port_vlan_del(struct dsa_switch *ds, int port,
> @@ -1930,17 +1924,17 @@ int mv88e6xxx_port_vlan_del(struct dsa_switch *ds, int port,
>
> mutex_lock(&ps->smi_mutex);
>
> - err = _mv88e6xxx_port_pvid_get(ds, port, &pvid);
> + err = _mv88e6xxx_port_pvid_get(ps, port, &pvid);
> if (err)
> goto unlock;
>
> for (vid = vlan->vid_begin; vid <= vlan->vid_end; ++vid) {
> - err = _mv88e6xxx_port_vlan_del(ds, port, vid);
> + err = _mv88e6xxx_port_vlan_del(ps, port, vid);
> if (err)
> goto unlock;
>
> if (vid == pvid) {
> - err = _mv88e6xxx_port_pvid_set(ds, port, 0);
> + err = _mv88e6xxx_port_pvid_set(ps, port, 0);
> if (err)
> goto unlock;
> }
> @@ -1952,14 +1946,14 @@ unlock:
> return err;
> }
>
> -static int _mv88e6xxx_atu_mac_write(struct dsa_switch *ds,
> +static int _mv88e6xxx_atu_mac_write(struct mv88e6xxx_priv_state *ps,
> const unsigned char *addr)
> {
> int i, ret;
>
> for (i = 0; i < 3; i++) {
> ret = _mv88e6xxx_reg_write(
> - ds, REG_GLOBAL, GLOBAL_ATU_MAC_01 + i,
> + ps, REG_GLOBAL, GLOBAL_ATU_MAC_01 + i,
> (addr[i * 2] << 8) | addr[i * 2 + 1]);
> if (ret < 0)
> return ret;
> @@ -1968,12 +1962,13 @@ static int _mv88e6xxx_atu_mac_write(struct dsa_switch *ds,
> return 0;
> }
>
> -static int _mv88e6xxx_atu_mac_read(struct dsa_switch *ds, unsigned char *addr)
> +static int _mv88e6xxx_atu_mac_read(struct mv88e6xxx_priv_state *ps,
> + unsigned char *addr)
> {
> int i, ret;
>
> for (i = 0; i < 3; i++) {
> - ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL,
> + ret = _mv88e6xxx_reg_read(ps, REG_GLOBAL,
> GLOBAL_ATU_MAC_01 + i);
> if (ret < 0)
> return ret;
> @@ -1984,27 +1979,27 @@ static int _mv88e6xxx_atu_mac_read(struct dsa_switch *ds, unsigned char *addr)
> return 0;
> }
>
> -static int _mv88e6xxx_atu_load(struct dsa_switch *ds,
> +static int _mv88e6xxx_atu_load(struct mv88e6xxx_priv_state *ps,
> struct mv88e6xxx_atu_entry *entry)
> {
> int ret;
>
> - ret = _mv88e6xxx_atu_wait(ds);
> + ret = _mv88e6xxx_atu_wait(ps);
> if (ret < 0)
> return ret;
>
> - ret = _mv88e6xxx_atu_mac_write(ds, entry->mac);
> + ret = _mv88e6xxx_atu_mac_write(ps, entry->mac);
> if (ret < 0)
> return ret;
>
> - ret = _mv88e6xxx_atu_data_write(ds, entry);
> + ret = _mv88e6xxx_atu_data_write(ps, entry);
> if (ret < 0)
> return ret;
>
> - return _mv88e6xxx_atu_cmd(ds, entry->fid, GLOBAL_ATU_OP_LOAD_DB);
> + return _mv88e6xxx_atu_cmd(ps, entry->fid, GLOBAL_ATU_OP_LOAD_DB);
> }
>
> -static int _mv88e6xxx_port_fdb_load(struct dsa_switch *ds, int port,
> +static int _mv88e6xxx_port_fdb_load(struct mv88e6xxx_priv_state *ps, int port,
> const unsigned char *addr, u16 vid,
> u8 state)
> {
> @@ -2014,9 +2009,9 @@ static int _mv88e6xxx_port_fdb_load(struct dsa_switch *ds, int port,
>
> /* Null VLAN ID corresponds to the port private database */
> if (vid == 0)
> - err = _mv88e6xxx_port_fid_get(ds, port, &vlan.fid);
> + err = _mv88e6xxx_port_fid_get(ps, port, &vlan.fid);
> else
> - err = _mv88e6xxx_vtu_get(ds, vid, &vlan, false);
> + err = _mv88e6xxx_vtu_get(ps, vid, &vlan, false);
> if (err)
> return err;
>
> @@ -2028,7 +2023,7 @@ static int _mv88e6xxx_port_fdb_load(struct dsa_switch *ds, int port,
> entry.portv_trunkid = BIT(port);
> }
>
> - return _mv88e6xxx_atu_load(ds, &entry);
> + return _mv88e6xxx_atu_load(ps, &entry);
> }
>
> int mv88e6xxx_port_fdb_prepare(struct dsa_switch *ds, int port,
> @@ -2051,7 +2046,7 @@ void mv88e6xxx_port_fdb_add(struct dsa_switch *ds, int port,
> struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
>
> mutex_lock(&ps->smi_mutex);
> - if (_mv88e6xxx_port_fdb_load(ds, port, fdb->addr, fdb->vid, state))
> + if (_mv88e6xxx_port_fdb_load(ps, port, fdb->addr, fdb->vid, state))
> netdev_err(ds->ports[port], "failed to load MAC address\n");
> mutex_unlock(&ps->smi_mutex);
> }
> @@ -2063,14 +2058,14 @@ int mv88e6xxx_port_fdb_del(struct dsa_switch *ds, int port,
> int ret;
>
> mutex_lock(&ps->smi_mutex);
> - ret = _mv88e6xxx_port_fdb_load(ds, port, fdb->addr, fdb->vid,
> + ret = _mv88e6xxx_port_fdb_load(ps, port, fdb->addr, fdb->vid,
> GLOBAL_ATU_DATA_STATE_UNUSED);
> mutex_unlock(&ps->smi_mutex);
>
> return ret;
> }
>
> -static int _mv88e6xxx_atu_getnext(struct dsa_switch *ds, u16 fid,
> +static int _mv88e6xxx_atu_getnext(struct mv88e6xxx_priv_state *ps, u16 fid,
> struct mv88e6xxx_atu_entry *entry)
> {
> struct mv88e6xxx_atu_entry next = { 0 };
> @@ -2078,19 +2073,19 @@ static int _mv88e6xxx_atu_getnext(struct dsa_switch *ds, u16 fid,
>
> next.fid = fid;
>
> - ret = _mv88e6xxx_atu_wait(ds);
> + ret = _mv88e6xxx_atu_wait(ps);
> if (ret < 0)
> return ret;
>
> - ret = _mv88e6xxx_atu_cmd(ds, fid, GLOBAL_ATU_OP_GET_NEXT_DB);
> + ret = _mv88e6xxx_atu_cmd(ps, fid, GLOBAL_ATU_OP_GET_NEXT_DB);
> if (ret < 0)
> return ret;
>
> - ret = _mv88e6xxx_atu_mac_read(ds, next.mac);
> + ret = _mv88e6xxx_atu_mac_read(ps, next.mac);
> if (ret < 0)
> return ret;
>
> - ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_ATU_DATA);
> + ret = _mv88e6xxx_reg_read(ps, REG_GLOBAL, GLOBAL_ATU_DATA);
> if (ret < 0)
> return ret;
>
> @@ -2115,8 +2110,8 @@ static int _mv88e6xxx_atu_getnext(struct dsa_switch *ds, u16 fid,
> return 0;
> }
>
> -static int _mv88e6xxx_port_fdb_dump_one(struct dsa_switch *ds, u16 fid, u16 vid,
> - int port,
> +static int _mv88e6xxx_port_fdb_dump_one(struct mv88e6xxx_priv_state *ps,
> + u16 fid, u16 vid, int port,
> struct switchdev_obj_port_fdb *fdb,
> int (*cb)(struct switchdev_obj *obj))
> {
> @@ -2125,12 +2120,12 @@ static int _mv88e6xxx_port_fdb_dump_one(struct dsa_switch *ds, u16 fid, u16 vid,
> };
> int err;
>
> - err = _mv88e6xxx_atu_mac_write(ds, addr.mac);
> + err = _mv88e6xxx_atu_mac_write(ps, addr.mac);
> if (err)
> return err;
>
> do {
> - err = _mv88e6xxx_atu_getnext(ds, fid, &addr);
> + err = _mv88e6xxx_atu_getnext(ps, fid, &addr);
> if (err)
> break;
>
> @@ -2170,28 +2165,28 @@ int mv88e6xxx_port_fdb_dump(struct dsa_switch *ds, int port,
> mutex_lock(&ps->smi_mutex);
>
> /* Dump port's default Filtering Information Database (VLAN ID 0) */
> - err = _mv88e6xxx_port_fid_get(ds, port, &fid);
> + err = _mv88e6xxx_port_fid_get(ps, port, &fid);
> if (err)
> goto unlock;
>
> - err = _mv88e6xxx_port_fdb_dump_one(ds, fid, 0, port, fdb, cb);
> + err = _mv88e6xxx_port_fdb_dump_one(ps, fid, 0, port, fdb, cb);
> if (err)
> goto unlock;
>
> /* Dump VLANs' Filtering Information Databases */
> - err = _mv88e6xxx_vtu_vid_write(ds, vlan.vid);
> + err = _mv88e6xxx_vtu_vid_write(ps, vlan.vid);
> if (err)
> goto unlock;
>
> do {
> - err = _mv88e6xxx_vtu_getnext(ds, &vlan);
> + err = _mv88e6xxx_vtu_getnext(ps, &vlan);
> if (err)
> break;
>
> if (!vlan.valid)
> break;
>
> - err = _mv88e6xxx_port_fdb_dump_one(ds, vlan.fid, vlan.vid, port,
> + err = _mv88e6xxx_port_fdb_dump_one(ps, vlan.fid, vlan.vid, port,
> fdb, cb);
> if (err)
> break;
> @@ -2216,7 +2211,7 @@ int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
>
> for (i = 0; i < ps->info->num_ports; ++i) {
> if (ps->ports[i].bridge_dev == bridge) {
> - err = _mv88e6xxx_port_based_vlan_map(ds, i);
> + err = _mv88e6xxx_port_based_vlan_map(ps, i);
> if (err)
> break;
> }
> @@ -2240,7 +2235,7 @@ void mv88e6xxx_port_bridge_leave(struct dsa_switch *ds, int port)
>
> for (i = 0; i < ps->info->num_ports; ++i)
> if (i == port || ps->ports[i].bridge_dev == bridge)
> - if (_mv88e6xxx_port_based_vlan_map(ds, i))
> + if (_mv88e6xxx_port_based_vlan_map(ps, i))
> netdev_warn(ds->ports[i], "failed to remap\n");
>
> mutex_unlock(&ps->smi_mutex);
> @@ -2259,57 +2254,58 @@ static void mv88e6xxx_bridge_work(struct work_struct *work)
>
> for (port = 0; port < ps->info->num_ports; ++port)
> if (test_and_clear_bit(port, ps->port_state_update_mask) &&
> - _mv88e6xxx_port_state(ds, port, ps->ports[port].state))
> - netdev_warn(ds->ports[port], "failed to update state to %s\n",
> + _mv88e6xxx_port_state(ps, port, ps->ports[port].state))
> + netdev_warn(ds->ports[port],
> + "failed to update state to %s\n",
> mv88e6xxx_port_state_names[ps->ports[port].state]);
>
> mutex_unlock(&ps->smi_mutex);
> }
>
> -static int _mv88e6xxx_phy_page_write(struct dsa_switch *ds, int port, int page,
> - int reg, int val)
> +static int _mv88e6xxx_phy_page_write(struct mv88e6xxx_priv_state *ps,
> + int port, int page, int reg, int val)
> {
> int ret;
>
> - ret = _mv88e6xxx_phy_write_indirect(ds, port, 0x16, page);
> + ret = _mv88e6xxx_phy_write_indirect(ps, port, 0x16, page);
> if (ret < 0)
> goto restore_page_0;
>
> - ret = _mv88e6xxx_phy_write_indirect(ds, port, reg, val);
> + ret = _mv88e6xxx_phy_write_indirect(ps, port, reg, val);
> restore_page_0:
> - _mv88e6xxx_phy_write_indirect(ds, port, 0x16, 0x0);
> + _mv88e6xxx_phy_write_indirect(ps, port, 0x16, 0x0);
>
> return ret;
> }
>
> -static int _mv88e6xxx_phy_page_read(struct dsa_switch *ds, int port, int page,
> - int reg)
> +static int _mv88e6xxx_phy_page_read(struct mv88e6xxx_priv_state *ps,
> + int port, int page, int reg)
> {
> int ret;
>
> - ret = _mv88e6xxx_phy_write_indirect(ds, port, 0x16, page);
> + ret = _mv88e6xxx_phy_write_indirect(ps, port, 0x16, page);
> if (ret < 0)
> goto restore_page_0;
>
> - ret = _mv88e6xxx_phy_read_indirect(ds, port, reg);
> + ret = _mv88e6xxx_phy_read_indirect(ps, port, reg);
> restore_page_0:
> - _mv88e6xxx_phy_write_indirect(ds, port, 0x16, 0x0);
> + _mv88e6xxx_phy_write_indirect(ps, port, 0x16, 0x0);
>
> return ret;
> }
>
> -static int mv88e6xxx_power_on_serdes(struct dsa_switch *ds)
> +static int mv88e6xxx_power_on_serdes(struct mv88e6xxx_priv_state *ps)
> {
> int ret;
>
> - ret = _mv88e6xxx_phy_page_read(ds, REG_FIBER_SERDES, PAGE_FIBER_SERDES,
> + ret = _mv88e6xxx_phy_page_read(ps, REG_FIBER_SERDES, PAGE_FIBER_SERDES,
> MII_BMCR);
> if (ret < 0)
> return ret;
>
> if (ret & BMCR_PDOWN) {
> ret &= ~BMCR_PDOWN;
> - ret = _mv88e6xxx_phy_page_write(ds, REG_FIBER_SERDES,
> + ret = _mv88e6xxx_phy_page_write(ps, REG_FIBER_SERDES,
> PAGE_FIBER_SERDES, MII_BMCR,
> ret);
> }
> @@ -2325,24 +2321,24 @@ static int mv88e6xxx_setup_port(struct dsa_switch *ds, int port)
>
> mutex_lock(&ps->smi_mutex);
>
> - if (mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds) ||
> - mv88e6xxx_6165_family(ds) || mv88e6xxx_6097_family(ds) ||
> - mv88e6xxx_6185_family(ds) || mv88e6xxx_6095_family(ds) ||
> - mv88e6xxx_6065_family(ds) || mv88e6xxx_6320_family(ds)) {
> + if (mv88e6xxx_6352_family(ps) || mv88e6xxx_6351_family(ps) ||
> + mv88e6xxx_6165_family(ps) || mv88e6xxx_6097_family(ps) ||
> + mv88e6xxx_6185_family(ps) || mv88e6xxx_6095_family(ps) ||
> + mv88e6xxx_6065_family(ps) || mv88e6xxx_6320_family(ps)) {
> /* MAC Forcing register: don't force link, speed,
> * duplex or flow control state to any particular
> * values on physical ports, but force the CPU port
> * and all DSA ports to their maximum bandwidth and
> * full duplex.
> */
> - reg = _mv88e6xxx_reg_read(ds, REG_PORT(port), PORT_PCS_CTRL);
> + reg = _mv88e6xxx_reg_read(ps, REG_PORT(port), PORT_PCS_CTRL);
> if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)) {
> reg &= ~PORT_PCS_CTRL_UNFORCED;
> reg |= PORT_PCS_CTRL_FORCE_LINK |
> PORT_PCS_CTRL_LINK_UP |
> PORT_PCS_CTRL_DUPLEX_FULL |
> PORT_PCS_CTRL_FORCE_DUPLEX;
> - if (mv88e6xxx_6065_family(ds))
> + if (mv88e6xxx_6065_family(ps))
> reg |= PORT_PCS_CTRL_100;
> else
> reg |= PORT_PCS_CTRL_1000;
> @@ -2350,7 +2346,7 @@ static int mv88e6xxx_setup_port(struct dsa_switch *ds, int port)
> reg |= PORT_PCS_CTRL_UNFORCED;
> }
>
> - ret = _mv88e6xxx_reg_write(ds, REG_PORT(port),
> + ret = _mv88e6xxx_reg_write(ps, REG_PORT(port),
> PORT_PCS_CTRL, reg);
> if (ret)
> goto abort;
> @@ -2371,19 +2367,19 @@ static int mv88e6xxx_setup_port(struct dsa_switch *ds, int port)
> * forwarding of unknown unicasts and multicasts.
> */
> reg = 0;
> - if (mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds) ||
> - mv88e6xxx_6165_family(ds) || mv88e6xxx_6097_family(ds) ||
> - mv88e6xxx_6095_family(ds) || mv88e6xxx_6065_family(ds) ||
> - mv88e6xxx_6185_family(ds) || mv88e6xxx_6320_family(ds))
> + if (mv88e6xxx_6352_family(ps) || mv88e6xxx_6351_family(ps) ||
> + mv88e6xxx_6165_family(ps) || mv88e6xxx_6097_family(ps) ||
> + mv88e6xxx_6095_family(ps) || mv88e6xxx_6065_family(ps) ||
> + mv88e6xxx_6185_family(ps) || mv88e6xxx_6320_family(ps))
> reg = PORT_CONTROL_IGMP_MLD_SNOOP |
> PORT_CONTROL_USE_TAG | PORT_CONTROL_USE_IP |
> PORT_CONTROL_STATE_FORWARDING;
> if (dsa_is_cpu_port(ds, port)) {
> - if (mv88e6xxx_6095_family(ds) || mv88e6xxx_6185_family(ds))
> + if (mv88e6xxx_6095_family(ps) || mv88e6xxx_6185_family(ps))
> reg |= PORT_CONTROL_DSA_TAG;
> - if (mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds) ||
> - mv88e6xxx_6165_family(ds) || mv88e6xxx_6097_family(ds) ||
> - mv88e6xxx_6320_family(ds)) {
> + if (mv88e6xxx_6352_family(ps) || mv88e6xxx_6351_family(ps) ||
> + mv88e6xxx_6165_family(ps) || mv88e6xxx_6097_family(ps) ||
> + mv88e6xxx_6320_family(ps)) {
> if (ds->dst->tag_protocol == DSA_TAG_PROTO_EDSA)
> reg |= PORT_CONTROL_FRAME_ETHER_TYPE_DSA;
> else
> @@ -2392,20 +2388,20 @@ static int mv88e6xxx_setup_port(struct dsa_switch *ds, int port)
> PORT_CONTROL_FORWARD_UNKNOWN_MC;
> }
>
> - if (mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds) ||
> - mv88e6xxx_6165_family(ds) || mv88e6xxx_6097_family(ds) ||
> - mv88e6xxx_6095_family(ds) || mv88e6xxx_6065_family(ds) ||
> - mv88e6xxx_6185_family(ds) || mv88e6xxx_6320_family(ds)) {
> + if (mv88e6xxx_6352_family(ps) || mv88e6xxx_6351_family(ps) ||
> + mv88e6xxx_6165_family(ps) || mv88e6xxx_6097_family(ps) ||
> + mv88e6xxx_6095_family(ps) || mv88e6xxx_6065_family(ps) ||
> + mv88e6xxx_6185_family(ps) || mv88e6xxx_6320_family(ps)) {
> if (ds->dst->tag_protocol == DSA_TAG_PROTO_EDSA)
> reg |= PORT_CONTROL_EGRESS_ADD_TAG;
> }
> }
> if (dsa_is_dsa_port(ds, port)) {
> - if (mv88e6xxx_6095_family(ds) || mv88e6xxx_6185_family(ds))
> + if (mv88e6xxx_6095_family(ps) || mv88e6xxx_6185_family(ps))
> reg |= PORT_CONTROL_DSA_TAG;
> - if (mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds) ||
> - mv88e6xxx_6165_family(ds) || mv88e6xxx_6097_family(ds) ||
> - mv88e6xxx_6320_family(ds)) {
> + if (mv88e6xxx_6352_family(ps) || mv88e6xxx_6351_family(ps) ||
> + mv88e6xxx_6165_family(ps) || mv88e6xxx_6097_family(ps) ||
> + mv88e6xxx_6320_family(ps)) {
> reg |= PORT_CONTROL_FRAME_MODE_DSA;
> }
>
> @@ -2414,7 +2410,7 @@ static int mv88e6xxx_setup_port(struct dsa_switch *ds, int port)
> PORT_CONTROL_FORWARD_UNKNOWN_MC;
> }
> if (reg) {
> - ret = _mv88e6xxx_reg_write(ds, REG_PORT(port),
> + ret = _mv88e6xxx_reg_write(ps, REG_PORT(port),
> PORT_CONTROL, reg);
> if (ret)
> goto abort;
> @@ -2423,15 +2419,15 @@ static int mv88e6xxx_setup_port(struct dsa_switch *ds, int port)
> /* If this port is connected to a SerDes, make sure the SerDes is not
> * powered down.
> */
> - if (mv88e6xxx_6352_family(ds)) {
> - ret = _mv88e6xxx_reg_read(ds, REG_PORT(port), PORT_STATUS);
> + if (mv88e6xxx_6352_family(ps)) {
> + ret = _mv88e6xxx_reg_read(ps, REG_PORT(port), PORT_STATUS);
> if (ret < 0)
> goto abort;
> ret &= PORT_STATUS_CMODE_MASK;
> if ((ret == PORT_STATUS_CMODE_100BASE_X) ||
> (ret == PORT_STATUS_CMODE_1000BASE_X) ||
> (ret == PORT_STATUS_CMODE_SGMII)) {
> - ret = mv88e6xxx_power_on_serdes(ds);
> + ret = mv88e6xxx_power_on_serdes(ps);
> if (ret < 0)
> goto abort;
> }
> @@ -2444,17 +2440,17 @@ static int mv88e6xxx_setup_port(struct dsa_switch *ds, int port)
> * copy of all transmitted/received frames on this port to the CPU.
> */
> reg = 0;
> - if (mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds) ||
> - mv88e6xxx_6165_family(ds) || mv88e6xxx_6097_family(ds) ||
> - mv88e6xxx_6095_family(ds) || mv88e6xxx_6320_family(ds) ||
> - mv88e6xxx_6185_family(ds))
> + if (mv88e6xxx_6352_family(ps) || mv88e6xxx_6351_family(ps) ||
> + mv88e6xxx_6165_family(ps) || mv88e6xxx_6097_family(ps) ||
> + mv88e6xxx_6095_family(ps) || mv88e6xxx_6320_family(ps) ||
> + mv88e6xxx_6185_family(ps))
> reg = PORT_CONTROL_2_MAP_DA;
>
> - if (mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds) ||
> - mv88e6xxx_6165_family(ds) || mv88e6xxx_6320_family(ds))
> + if (mv88e6xxx_6352_family(ps) || mv88e6xxx_6351_family(ps) ||
> + mv88e6xxx_6165_family(ps) || mv88e6xxx_6320_family(ps))
> reg |= PORT_CONTROL_2_JUMBO_10240;
>
> - if (mv88e6xxx_6095_family(ds) || mv88e6xxx_6185_family(ds)) {
> + if (mv88e6xxx_6095_family(ps) || mv88e6xxx_6185_family(ps)) {
> /* Set the upstream port this port should use */
> reg |= dsa_upstream_port(ds);
> /* enable forwarding of unknown multicast addresses to
> @@ -2467,7 +2463,7 @@ static int mv88e6xxx_setup_port(struct dsa_switch *ds, int port)
> reg |= PORT_CONTROL_2_8021Q_DISABLED;
>
> if (reg) {
> - ret = _mv88e6xxx_reg_write(ds, REG_PORT(port),
> + ret = _mv88e6xxx_reg_write(ps, REG_PORT(port),
> PORT_CONTROL_2, reg);
> if (ret)
> goto abort;
> @@ -2483,24 +2479,24 @@ static int mv88e6xxx_setup_port(struct dsa_switch *ds, int port)
> if (dsa_is_cpu_port(ds, port))
> reg = 0;
>
> - ret = _mv88e6xxx_reg_write(ds, REG_PORT(port), PORT_ASSOC_VECTOR, reg);
> + ret = _mv88e6xxx_reg_write(ps, REG_PORT(port), PORT_ASSOC_VECTOR, reg);
> if (ret)
> goto abort;
>
> /* Egress rate control 2: disable egress rate control. */
> - ret = _mv88e6xxx_reg_write(ds, REG_PORT(port), PORT_RATE_CONTROL_2,
> + ret = _mv88e6xxx_reg_write(ps, REG_PORT(port), PORT_RATE_CONTROL_2,
> 0x0000);
> if (ret)
> goto abort;
>
> - if (mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds) ||
> - mv88e6xxx_6165_family(ds) || mv88e6xxx_6097_family(ds) ||
> - mv88e6xxx_6320_family(ds)) {
> + if (mv88e6xxx_6352_family(ps) || mv88e6xxx_6351_family(ps) ||
> + mv88e6xxx_6165_family(ps) || mv88e6xxx_6097_family(ps) ||
> + mv88e6xxx_6320_family(ps)) {
> /* Do not limit the period of time that this port can
> * be paused for by the remote end or the period of
> * time that this port can pause the remote end.
> */
> - ret = _mv88e6xxx_reg_write(ds, REG_PORT(port),
> + ret = _mv88e6xxx_reg_write(ps, REG_PORT(port),
> PORT_PAUSE_CTRL, 0x0000);
> if (ret)
> goto abort;
> @@ -2509,12 +2505,12 @@ static int mv88e6xxx_setup_port(struct dsa_switch *ds, int port)
> * address database entries that this port is allowed
> * to use.
> */
> - ret = _mv88e6xxx_reg_write(ds, REG_PORT(port),
> + ret = _mv88e6xxx_reg_write(ps, REG_PORT(port),
> PORT_ATU_CONTROL, 0x0000);
> /* Priority Override: disable DA, SA and VTU priority
> * override.
> */
> - ret = _mv88e6xxx_reg_write(ds, REG_PORT(port),
> + ret = _mv88e6xxx_reg_write(ps, REG_PORT(port),
> PORT_PRI_OVERRIDE, 0x0000);
> if (ret)
> goto abort;
> @@ -2522,14 +2518,14 @@ static int mv88e6xxx_setup_port(struct dsa_switch *ds, int port)
> /* Port Ethertype: use the Ethertype DSA Ethertype
> * value.
> */
> - ret = _mv88e6xxx_reg_write(ds, REG_PORT(port),
> + ret = _mv88e6xxx_reg_write(ps, REG_PORT(port),
> PORT_ETH_TYPE, ETH_P_EDSA);
> if (ret)
> goto abort;
> /* Tag Remap: use an identity 802.1p prio -> switch
> * prio mapping.
> */
> - ret = _mv88e6xxx_reg_write(ds, REG_PORT(port),
> + ret = _mv88e6xxx_reg_write(ps, REG_PORT(port),
> PORT_TAG_REGMAP_0123, 0x3210);
> if (ret)
> goto abort;
> @@ -2537,18 +2533,18 @@ static int mv88e6xxx_setup_port(struct dsa_switch *ds, int port)
> /* Tag Remap 2: use an identity 802.1p prio -> switch
> * prio mapping.
> */
> - ret = _mv88e6xxx_reg_write(ds, REG_PORT(port),
> + ret = _mv88e6xxx_reg_write(ps, REG_PORT(port),
> PORT_TAG_REGMAP_4567, 0x7654);
> if (ret)
> goto abort;
> }
>
> - if (mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds) ||
> - mv88e6xxx_6165_family(ds) || mv88e6xxx_6097_family(ds) ||
> - mv88e6xxx_6185_family(ds) || mv88e6xxx_6095_family(ds) ||
> - mv88e6xxx_6320_family(ds)) {
> + if (mv88e6xxx_6352_family(ps) || mv88e6xxx_6351_family(ps) ||
> + mv88e6xxx_6165_family(ps) || mv88e6xxx_6097_family(ps) ||
> + mv88e6xxx_6185_family(ps) || mv88e6xxx_6095_family(ps) ||
> + mv88e6xxx_6320_family(ps)) {
> /* Rate Control: disable ingress rate limiting. */
> - ret = _mv88e6xxx_reg_write(ds, REG_PORT(port),
> + ret = _mv88e6xxx_reg_write(ps, REG_PORT(port),
> PORT_RATE_CONTROL, 0x0001);
> if (ret)
> goto abort;
> @@ -2557,7 +2553,7 @@ static int mv88e6xxx_setup_port(struct dsa_switch *ds, int port)
> /* Port Control 1: disable trunking, disable sending
> * learning messages to this port.
> */
> - ret = _mv88e6xxx_reg_write(ds, REG_PORT(port), PORT_CONTROL_1, 0x0000);
> + ret = _mv88e6xxx_reg_write(ps, REG_PORT(port), PORT_CONTROL_1, 0x0000);
> if (ret)
> goto abort;
>
> @@ -2565,18 +2561,18 @@ static int mv88e6xxx_setup_port(struct dsa_switch *ds, int port)
> * database, and allow bidirectional communication between the
> * CPU and DSA port(s), and the other ports.
> */
> - ret = _mv88e6xxx_port_fid_set(ds, port, 0);
> + ret = _mv88e6xxx_port_fid_set(ps, port, 0);
> if (ret)
> goto abort;
>
> - ret = _mv88e6xxx_port_based_vlan_map(ds, port);
> + ret = _mv88e6xxx_port_based_vlan_map(ps, port);
> if (ret)
> goto abort;
>
> /* Default VLAN ID and priority: don't set a default VLAN
> * ID, and set the default packet priority to zero.
> */
> - ret = _mv88e6xxx_reg_write(ds, REG_PORT(port), PORT_DEFAULT_VLAN,
> + ret = _mv88e6xxx_reg_write(ps, REG_PORT(port), PORT_DEFAULT_VLAN,
> 0x0000);
> abort:
> mutex_unlock(&ps->smi_mutex);
> @@ -2597,11 +2593,8 @@ int mv88e6xxx_setup_ports(struct dsa_switch *ds)
> return 0;
> }
>
> -int mv88e6xxx_setup_common(struct dsa_switch *ds)
> +int mv88e6xxx_setup_common(struct mv88e6xxx_priv_state *ps)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> -
> - ps->ds = ds;
> mutex_init(&ps->smi_mutex);
>
> INIT_WORK(&ps->bridge_work, mv88e6xxx_bridge_work);
> @@ -2620,46 +2613,46 @@ int mv88e6xxx_setup_global(struct dsa_switch *ds)
> * enable address learn messages to be sent to all message
> * ports.
> */
> - err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_ATU_CONTROL,
> + err = _mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_ATU_CONTROL,
> 0x0140 | GLOBAL_ATU_CONTROL_LEARN2ALL);
> if (err)
> goto unlock;
>
> /* Configure the IP ToS mapping registers. */
> - err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_0, 0x0000);
> + err = _mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_IP_PRI_0, 0x0000);
> if (err)
> goto unlock;
> - err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_1, 0x0000);
> + err = _mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_IP_PRI_1, 0x0000);
> if (err)
> goto unlock;
> - err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_2, 0x5555);
> + err = _mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_IP_PRI_2, 0x5555);
> if (err)
> goto unlock;
> - err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_3, 0x5555);
> + err = _mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_IP_PRI_3, 0x5555);
> if (err)
> goto unlock;
> - err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_4, 0xaaaa);
> + err = _mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_IP_PRI_4, 0xaaaa);
> if (err)
> goto unlock;
> - err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_5, 0xaaaa);
> + err = _mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_IP_PRI_5, 0xaaaa);
> if (err)
> goto unlock;
> - err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_6, 0xffff);
> + err = _mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_IP_PRI_6, 0xffff);
> if (err)
> goto unlock;
> - err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IP_PRI_7, 0xffff);
> + err = _mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_IP_PRI_7, 0xffff);
> if (err)
> goto unlock;
>
> /* Configure the IEEE 802.1p priority mapping register. */
> - err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_IEEE_PRI, 0xfa41);
> + err = _mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_IEEE_PRI, 0xfa41);
> if (err)
> goto unlock;
>
> /* Send all frames with destination addresses matching
> * 01:80:c2:00:00:0x to the CPU port.
> */
> - err = _mv88e6xxx_reg_write(ds, REG_GLOBAL2, GLOBAL2_MGMT_EN_0X, 0xffff);
> + err = _mv88e6xxx_reg_write(ps, REG_GLOBAL2, GLOBAL2_MGMT_EN_0X, 0xffff);
> if (err)
> goto unlock;
>
> @@ -2668,7 +2661,7 @@ int mv88e6xxx_setup_global(struct dsa_switch *ds)
> * highest, and send all special multicast frames to the CPU
> * port at the highest priority.
> */
> - err = _mv88e6xxx_reg_write(ds, REG_GLOBAL2, GLOBAL2_SWITCH_MGMT,
> + err = _mv88e6xxx_reg_write(ps, REG_GLOBAL2, GLOBAL2_SWITCH_MGMT,
> 0x7 | GLOBAL2_SWITCH_MGMT_RSVD2CPU | 0x70 |
> GLOBAL2_SWITCH_MGMT_FORCE_FLOW_CTRL_PRI);
> if (err)
> @@ -2683,7 +2676,7 @@ int mv88e6xxx_setup_global(struct dsa_switch *ds)
> nexthop = ds->pd->rtable[i] & 0x1f;
>
> err = _mv88e6xxx_reg_write(
> - ds, REG_GLOBAL2,
> + ps, REG_GLOBAL2,
> GLOBAL2_DEVICE_MAPPING,
> GLOBAL2_DEVICE_MAPPING_UPDATE |
> (i << GLOBAL2_DEVICE_MAPPING_TARGET_SHIFT) | nexthop);
> @@ -2693,7 +2686,7 @@ int mv88e6xxx_setup_global(struct dsa_switch *ds)
>
> /* Clear all trunk masks. */
> for (i = 0; i < 8; i++) {
> - err = _mv88e6xxx_reg_write(ds, REG_GLOBAL2, GLOBAL2_TRUNK_MASK,
> + err = _mv88e6xxx_reg_write(ps, REG_GLOBAL2, GLOBAL2_TRUNK_MASK,
> 0x8000 |
> (i << GLOBAL2_TRUNK_MASK_NUM_SHIFT) |
> ((1 << ps->info->num_ports) - 1));
> @@ -2704,7 +2697,7 @@ int mv88e6xxx_setup_global(struct dsa_switch *ds)
> /* Clear all trunk mappings. */
> for (i = 0; i < 16; i++) {
> err = _mv88e6xxx_reg_write(
> - ds, REG_GLOBAL2,
> + ps, REG_GLOBAL2,
> GLOBAL2_TRUNK_MAPPING,
> GLOBAL2_TRUNK_MAPPING_UPDATE |
> (i << GLOBAL2_TRUNK_MAPPING_ID_SHIFT));
> @@ -2712,13 +2705,13 @@ int mv88e6xxx_setup_global(struct dsa_switch *ds)
> goto unlock;
> }
>
> - if (mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds) ||
> - mv88e6xxx_6165_family(ds) || mv88e6xxx_6097_family(ds) ||
> - mv88e6xxx_6320_family(ds)) {
> + if (mv88e6xxx_6352_family(ps) || mv88e6xxx_6351_family(ps) ||
> + mv88e6xxx_6165_family(ps) || mv88e6xxx_6097_family(ps) ||
> + mv88e6xxx_6320_family(ps)) {
> /* Send all frames with destination addresses matching
> * 01:80:c2:00:00:2x to the CPU port.
> */
> - err = _mv88e6xxx_reg_write(ds, REG_GLOBAL2,
> + err = _mv88e6xxx_reg_write(ps, REG_GLOBAL2,
> GLOBAL2_MGMT_EN_2X, 0xffff);
> if (err)
> goto unlock;
> @@ -2726,14 +2719,14 @@ int mv88e6xxx_setup_global(struct dsa_switch *ds)
> /* Initialise cross-chip port VLAN table to reset
> * defaults.
> */
> - err = _mv88e6xxx_reg_write(ds, REG_GLOBAL2,
> + err = _mv88e6xxx_reg_write(ps, REG_GLOBAL2,
> GLOBAL2_PVT_ADDR, 0x9000);
> if (err)
> goto unlock;
>
> /* Clear the priority override table. */
> for (i = 0; i < 16; i++) {
> - err = _mv88e6xxx_reg_write(ds, REG_GLOBAL2,
> + err = _mv88e6xxx_reg_write(ps, REG_GLOBAL2,
> GLOBAL2_PRIO_OVERRIDE,
> 0x8000 | (i << 8));
> if (err)
> @@ -2741,16 +2734,16 @@ int mv88e6xxx_setup_global(struct dsa_switch *ds)
> }
> }
>
> - if (mv88e6xxx_6352_family(ds) || mv88e6xxx_6351_family(ds) ||
> - mv88e6xxx_6165_family(ds) || mv88e6xxx_6097_family(ds) ||
> - mv88e6xxx_6185_family(ds) || mv88e6xxx_6095_family(ds) ||
> - mv88e6xxx_6320_family(ds)) {
> + if (mv88e6xxx_6352_family(ps) || mv88e6xxx_6351_family(ps) ||
> + mv88e6xxx_6165_family(ps) || mv88e6xxx_6097_family(ps) ||
> + mv88e6xxx_6185_family(ps) || mv88e6xxx_6095_family(ps) ||
> + mv88e6xxx_6320_family(ps)) {
> /* Disable ingress rate limiting by resetting all
> * ingress rate limit registers to their initial
> * state.
> */
> for (i = 0; i < ps->info->num_ports; i++) {
> - err = _mv88e6xxx_reg_write(ds, REG_GLOBAL2,
> + err = _mv88e6xxx_reg_write(ps, REG_GLOBAL2,
> GLOBAL2_INGRESS_OP,
> 0x9000 | (i << 8));
> if (err)
> @@ -2759,34 +2752,33 @@ int mv88e6xxx_setup_global(struct dsa_switch *ds)
> }
>
> /* Clear the statistics counters for all ports */
> - err = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_STATS_OP,
> + err = _mv88e6xxx_reg_write(ps, REG_GLOBAL, GLOBAL_STATS_OP,
> GLOBAL_STATS_OP_FLUSH_ALL);
> if (err)
> goto unlock;
>
> /* Wait for the flush to complete. */
> - err = _mv88e6xxx_stats_wait(ds);
> + err = _mv88e6xxx_stats_wait(ps);
> if (err < 0)
> goto unlock;
>
> /* Clear all ATU entries */
> - err = _mv88e6xxx_atu_flush(ds, 0, true);
> + err = _mv88e6xxx_atu_flush(ps, 0, true);
> if (err < 0)
> goto unlock;
>
> /* Clear all the VTU and STU entries */
> - err = _mv88e6xxx_vtu_stu_flush(ds);
> + err = _mv88e6xxx_vtu_stu_flush(ps);
> unlock:
> mutex_unlock(&ps->smi_mutex);
>
> return err;
> }
>
> -int mv88e6xxx_switch_reset(struct dsa_switch *ds, bool ppu_active)
> +int mv88e6xxx_switch_reset(struct mv88e6xxx_priv_state *ps, bool ppu_active)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> u16 is_reset = (ppu_active ? 0x8800 : 0xc800);
> - struct gpio_desc *gpiod = ds->pd->reset;
> + struct gpio_desc *gpiod = ps->ds->pd->reset;
> unsigned long timeout;
> int ret;
> int i;
> @@ -2795,11 +2787,11 @@ int mv88e6xxx_switch_reset(struct dsa_switch *ds, bool ppu_active)
>
> /* Set all ports to the disabled state. */
> for (i = 0; i < ps->info->num_ports; i++) {
> - ret = _mv88e6xxx_reg_read(ds, REG_PORT(i), PORT_CONTROL);
> + ret = _mv88e6xxx_reg_read(ps, REG_PORT(i), PORT_CONTROL);
> if (ret < 0)
> goto unlock;
>
> - ret = _mv88e6xxx_reg_write(ds, REG_PORT(i), PORT_CONTROL,
> + ret = _mv88e6xxx_reg_write(ps, REG_PORT(i), PORT_CONTROL,
> ret & 0xfffc);
> if (ret)
> goto unlock;
> @@ -2821,16 +2813,16 @@ int mv88e6xxx_switch_reset(struct dsa_switch *ds, bool ppu_active)
> * through global registers 0x18 and 0x19.
> */
> if (ppu_active)
> - ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, 0x04, 0xc000);
> + ret = _mv88e6xxx_reg_write(ps, REG_GLOBAL, 0x04, 0xc000);
> else
> - ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, 0x04, 0xc400);
> + ret = _mv88e6xxx_reg_write(ps, REG_GLOBAL, 0x04, 0xc400);
> if (ret)
> goto unlock;
>
> /* Wait up to one second for reset to complete. */
> timeout = jiffies + 1 * HZ;
> while (time_before(jiffies, timeout)) {
> - ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, 0x00);
> + ret = _mv88e6xxx_reg_read(ps, REG_GLOBAL, 0x00);
> if (ret < 0)
> goto unlock;
>
> @@ -2854,7 +2846,7 @@ int mv88e6xxx_phy_page_read(struct dsa_switch *ds, int port, int page, int reg)
> int ret;
>
> mutex_lock(&ps->smi_mutex);
> - ret = _mv88e6xxx_phy_page_read(ds, port, page, reg);
> + ret = _mv88e6xxx_phy_page_read(ps, port, page, reg);
> mutex_unlock(&ps->smi_mutex);
>
> return ret;
> @@ -2867,16 +2859,15 @@ int mv88e6xxx_phy_page_write(struct dsa_switch *ds, int port, int page,
> int ret;
>
> mutex_lock(&ps->smi_mutex);
> - ret = _mv88e6xxx_phy_page_write(ds, port, page, reg, val);
> + ret = _mv88e6xxx_phy_page_write(ps, port, page, reg, val);
> mutex_unlock(&ps->smi_mutex);
>
> return ret;
> }
>
> -static int mv88e6xxx_port_to_phy_addr(struct dsa_switch *ds, int port)
> +static int mv88e6xxx_port_to_phy_addr(struct mv88e6xxx_priv_state *ps,
> + int port)
> {
> - struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> -
> if (port >= 0 && port < ps->info->num_ports)
> return port;
> return -EINVAL;
> @@ -2886,14 +2877,14 @@ int
> mv88e6xxx_phy_read(struct dsa_switch *ds, int port, int regnum)
> {
> struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> - int addr = mv88e6xxx_port_to_phy_addr(ds, port);
> + int addr = mv88e6xxx_port_to_phy_addr(ps, port);
> int ret;
>
> if (addr < 0)
> - return addr;
> + return 0xffff;
>
> mutex_lock(&ps->smi_mutex);
> - ret = _mv88e6xxx_phy_read(ds, addr, regnum);
> + ret = _mv88e6xxx_phy_read(ps, addr, regnum);
> mutex_unlock(&ps->smi_mutex);
> return ret;
> }
> @@ -2902,14 +2893,14 @@ int
> mv88e6xxx_phy_write(struct dsa_switch *ds, int port, int regnum, u16 val)
> {
> struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> - int addr = mv88e6xxx_port_to_phy_addr(ds, port);
> + int addr = mv88e6xxx_port_to_phy_addr(ps, port);
> int ret;
>
> if (addr < 0)
> - return addr;
> + return 0xffff;
>
> mutex_lock(&ps->smi_mutex);
> - ret = _mv88e6xxx_phy_write(ds, addr, regnum, val);
> + ret = _mv88e6xxx_phy_write(ps, addr, regnum, val);
> mutex_unlock(&ps->smi_mutex);
> return ret;
> }
> @@ -2918,14 +2909,14 @@ int
> mv88e6xxx_phy_read_indirect(struct dsa_switch *ds, int port, int regnum)
> {
> struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> - int addr = mv88e6xxx_port_to_phy_addr(ds, port);
> + int addr = mv88e6xxx_port_to_phy_addr(ps, port);
> int ret;
>
> if (addr < 0)
> - return addr;
> + return 0xffff;
>
> mutex_lock(&ps->smi_mutex);
> - ret = _mv88e6xxx_phy_read_indirect(ds, addr, regnum);
> + ret = _mv88e6xxx_phy_read_indirect(ps, addr, regnum);
> mutex_unlock(&ps->smi_mutex);
> return ret;
> }
> @@ -2935,14 +2926,14 @@ mv88e6xxx_phy_write_indirect(struct dsa_switch *ds, int port, int regnum,
> u16 val)
> {
> struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> - int addr = mv88e6xxx_port_to_phy_addr(ds, port);
> + int addr = mv88e6xxx_port_to_phy_addr(ps, port);
> int ret;
>
> if (addr < 0)
> return addr;
>
> mutex_lock(&ps->smi_mutex);
> - ret = _mv88e6xxx_phy_write_indirect(ds, addr, regnum, val);
> + ret = _mv88e6xxx_phy_write_indirect(ps, addr, regnum, val);
> mutex_unlock(&ps->smi_mutex);
> return ret;
> }
> @@ -2959,44 +2950,45 @@ static int mv88e61xx_get_temp(struct dsa_switch *ds, int *temp)
>
> mutex_lock(&ps->smi_mutex);
>
> - ret = _mv88e6xxx_phy_write(ds, 0x0, 0x16, 0x6);
> + ret = _mv88e6xxx_phy_write(ps, 0x0, 0x16, 0x6);
> if (ret < 0)
> goto error;
>
> /* Enable temperature sensor */
> - ret = _mv88e6xxx_phy_read(ds, 0x0, 0x1a);
> + ret = _mv88e6xxx_phy_read(ps, 0x0, 0x1a);
> if (ret < 0)
> goto error;
>
> - ret = _mv88e6xxx_phy_write(ds, 0x0, 0x1a, ret | (1 << 5));
> + ret = _mv88e6xxx_phy_write(ps, 0x0, 0x1a, ret | (1 << 5));
> if (ret < 0)
> goto error;
>
> /* Wait for temperature to stabilize */
> usleep_range(10000, 12000);
>
> - val = _mv88e6xxx_phy_read(ds, 0x0, 0x1a);
> + val = _mv88e6xxx_phy_read(ps, 0x0, 0x1a);
> if (val < 0) {
> ret = val;
> goto error;
> }
>
> /* Disable temperature sensor */
> - ret = _mv88e6xxx_phy_write(ds, 0x0, 0x1a, ret & ~(1 << 5));
> + ret = _mv88e6xxx_phy_write(ps, 0x0, 0x1a, ret & ~(1 << 5));
> if (ret < 0)
> goto error;
>
> *temp = ((val & 0x1f) - 5) * 5;
>
> error:
> - _mv88e6xxx_phy_write(ds, 0x0, 0x16, 0x0);
> + _mv88e6xxx_phy_write(ps, 0x0, 0x16, 0x0);
> mutex_unlock(&ps->smi_mutex);
> return ret;
> }
>
> static int mv88e63xx_get_temp(struct dsa_switch *ds, int *temp)
> {
> - int phy = mv88e6xxx_6320_family(ds) ? 3 : 0;
> + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> + int phy = mv88e6xxx_6320_family(ps) ? 3 : 0;
> int ret;
>
> *temp = 0;
> @@ -3012,7 +3004,9 @@ static int mv88e63xx_get_temp(struct dsa_switch *ds, int *temp)
>
> int mv88e6xxx_get_temp(struct dsa_switch *ds, int *temp)
> {
> - if (mv88e6xxx_6320_family(ds) || mv88e6xxx_6352_family(ds))
> + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> +
> + if (mv88e6xxx_6320_family(ps) || mv88e6xxx_6352_family(ps))
> return mv88e63xx_get_temp(ds, temp);
>
> return mv88e61xx_get_temp(ds, temp);
> @@ -3020,10 +3014,11 @@ int mv88e6xxx_get_temp(struct dsa_switch *ds, int *temp)
>
> int mv88e6xxx_get_temp_limit(struct dsa_switch *ds, int *temp)
> {
> - int phy = mv88e6xxx_6320_family(ds) ? 3 : 0;
> + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> + int phy = mv88e6xxx_6320_family(ps) ? 3 : 0;
> int ret;
>
> - if (!mv88e6xxx_6320_family(ds) && !mv88e6xxx_6352_family(ds))
> + if (!mv88e6xxx_6320_family(ps) && !mv88e6xxx_6352_family(ps))
> return -EOPNOTSUPP;
>
> *temp = 0;
> @@ -3039,10 +3034,11 @@ int mv88e6xxx_get_temp_limit(struct dsa_switch *ds, int *temp)
>
> int mv88e6xxx_set_temp_limit(struct dsa_switch *ds, int temp)
> {
> - int phy = mv88e6xxx_6320_family(ds) ? 3 : 0;
> + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> + int phy = mv88e6xxx_6320_family(ps) ? 3 : 0;
> int ret;
>
> - if (!mv88e6xxx_6320_family(ds) && !mv88e6xxx_6352_family(ds))
> + if (!mv88e6xxx_6320_family(ps) && !mv88e6xxx_6352_family(ps))
> return -EOPNOTSUPP;
>
> ret = mv88e6xxx_phy_page_read(ds, phy, 6, 26);
> @@ -3055,10 +3051,11 @@ int mv88e6xxx_set_temp_limit(struct dsa_switch *ds, int temp)
>
> int mv88e6xxx_get_temp_alarm(struct dsa_switch *ds, bool *alarm)
> {
> - int phy = mv88e6xxx_6320_family(ds) ? 3 : 0;
> + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> + int phy = mv88e6xxx_6320_family(ps) ? 3 : 0;
> int ret;
>
> - if (!mv88e6xxx_6320_family(ds) && !mv88e6xxx_6352_family(ds))
> + if (!mv88e6xxx_6320_family(ps) && !mv88e6xxx_6352_family(ps))
> return -EOPNOTSUPP;
>
> *alarm = false;
> diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
> index 0dbe2d1..4f455d2 100644
> --- a/drivers/net/dsa/mv88e6xxx.h
> +++ b/drivers/net/dsa/mv88e6xxx.h
> @@ -388,6 +388,9 @@ struct mv88e6xxx_priv_state {
> /* The dsa_switch this private structure is related to */
> struct dsa_switch *ds;
>
> + /* The device this structure is associated to */
> + struct device *dev;
> +
> /* When using multi-chip addressing, this mutex protects
> * access to the indirect access registers. (In single-chip
> * mode, this mutex is effectively useless.)
> @@ -446,17 +449,18 @@ struct mv88e6xxx_hw_stat {
> enum stat_type type;
> };
>
> -int mv88e6xxx_switch_reset(struct dsa_switch *ds, bool ppu_active);
> +int mv88e6xxx_switch_reset(struct mv88e6xxx_priv_state *ps, bool ppu_active);
> const char *mv88e6xxx_drv_probe(struct device *dsa_dev, struct device *host_dev,
> int sw_addr, void **priv,
> const struct mv88e6xxx_info *table,
> unsigned int num);
>
> int mv88e6xxx_setup_ports(struct dsa_switch *ds);
> -int mv88e6xxx_setup_common(struct dsa_switch *ds);
> +int mv88e6xxx_setup_common(struct mv88e6xxx_priv_state *ps);
> int mv88e6xxx_setup_global(struct dsa_switch *ds);
> -int mv88e6xxx_reg_read(struct dsa_switch *ds, int addr, int reg);
> -int mv88e6xxx_reg_write(struct dsa_switch *ds, int addr, int reg, u16 val);
> +int mv88e6xxx_reg_read(struct mv88e6xxx_priv_state *ps, int addr, int reg);
> +int mv88e6xxx_reg_write(struct mv88e6xxx_priv_state *ps, int addr,
> + int reg, u16 val);
> int mv88e6xxx_set_addr_direct(struct dsa_switch *ds, u8 *addr);
> int mv88e6xxx_set_addr_indirect(struct dsa_switch *ds, u8 *addr);
> int mv88e6xxx_phy_read(struct dsa_switch *ds, int port, int regnum);
> @@ -464,7 +468,7 @@ int mv88e6xxx_phy_write(struct dsa_switch *ds, int port, int regnum, u16 val);
> int mv88e6xxx_phy_read_indirect(struct dsa_switch *ds, int port, int regnum);
> int mv88e6xxx_phy_write_indirect(struct dsa_switch *ds, int port, int regnum,
> u16 val);
> -void mv88e6xxx_ppu_state_init(struct dsa_switch *ds);
> +void mv88e6xxx_ppu_state_init(struct mv88e6xxx_priv_state *ps);
> int mv88e6xxx_phy_read_ppu(struct dsa_switch *ds, int addr, int regnum);
> int mv88e6xxx_phy_write_ppu(struct dsa_switch *ds, int addr,
> int regnum, u16 val);
> --
> 2.8.0
>
^ permalink raw reply
* Re: [PATCH v2] net: macb: do not scan PHYs manually
From: Andrew Lunn @ 2016-04-29 12:56 UTC (permalink / raw)
To: Nicolas Ferre
Cc: Josh Cartwright, Nathan Sullivan, netdev, linux-kernel,
Florian Fainelli, Alexandre Belloni
In-Reply-To: <57235655.3030104@atmel.com>
> > +static int macb_mii_of_init(struct macb *bp, struct device_node *np)
> > +{
> > + struct device_node *mdio;
> > + int err, i;
> > +
> > + mdio = of_get_child_by_name(np, "mdio");
> > + if (mdio)
> > + return of_mdiobus_register(bp->mii_bus, mdio);
> > +
> > + dev_warn(&bp->pdev->dev,
> > + "using deprecated PHY probing mechanism. Please update device tree.");
>
> Do we need to warn here?
>
> Too bad I was not aware of that earlier, I even updated some of my DTs
> recently with only a phy node without the "mdio" one as parents :-\
It is messy. Unfortunately, there is no binding documentation (yet)
suggesting the right way to do this. And as a result, we have
drivers/device trees doing different things, leading to workarounds
like manually scanning the bus, not listing PHYs in the device tree
and so or falling back to the old methods, etc.
We need to document how we expect this to be done, and then add
warnings in various places to encourage developers to migrate their
device trees to what has been documented.
Andrew
^ permalink raw reply
* Re: [PATCH v2] net: macb: do not scan PHYs manually
From: Andrew Lunn @ 2016-04-29 12:49 UTC (permalink / raw)
To: Josh Cartwright
Cc: Nathan Sullivan, Nicolas Ferre, netdev, linux-kernel,
Florian Fainelli, Alexandre Belloni
In-Reply-To: <20160429122501.GD30217@jcartwri.amer.corp.natinst.com>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index eec3200..d843bc9 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -419,11 +419,62 @@ static int macb_mii_probe(struct net_device *dev)
> return 0;
> }
>
> +static int macb_mii_of_init(struct macb *bp, struct device_node *np)
> +{
> + struct device_node *mdio;
> + int err, i;
> +
> + mdio = of_get_child_by_name(np, "mdio");
> + if (mdio)
> + return of_mdiobus_register(bp->mii_bus, mdio);
We want to encourage driver writers to use an mdio subnode inside
there MAC node. So i wounder if this looking for the child and using
it should go into the core code?
Florian: What do you think?
> +
> + dev_warn(&bp->pdev->dev,
> + "using deprecated PHY probing mechanism. Please update device tree.");
> +
> + /* try dt phy registration */
> + err = of_mdiobus_register(bp->mii_bus, np);
> + if (err)
> + return err;
> +
> + /* fallback to standard phy registration if no phy were
> + * found during dt phy registration
> + */
> + if (!phy_find_first(bp->mii_bus)) {
I would also suggest putting a warning here, saying that PHYs should
be listed in the device tree.
> + for (i = 0; i < PHY_MAX_ADDR; i++) {
> + struct phy_device *phydev;
> +
> + phydev = mdiobus_scan(bp->mii_bus, i);
> + if (IS_ERR(phydev)) {
> + err = PTR_ERR(phydev);
FYI: There is a change making its way through which will mean
mdiobus_scan() will return -ENODEV where there is nothing on the bus
at that address, rather than the current NULL. You will need to adopt
this here.
Andrew
^ permalink raw reply
* Re: [PATCH v2] net: macb: do not scan PHYs manually
From: Nicolas Ferre @ 2016-04-29 12:40 UTC (permalink / raw)
To: Josh Cartwright, Andrew Lunn
Cc: Nathan Sullivan, netdev, linux-kernel, Florian Fainelli,
Alexandre Belloni
In-Reply-To: <20160429122501.GD30217@jcartwri.amer.corp.natinst.com>
Le 29/04/2016 14:25, Josh Cartwright a écrit :
> On Thu, Apr 28, 2016 at 07:34:59PM -0500, Josh Cartwright wrote:
>> On Thu, Apr 28, 2016 at 11:23:15PM +0200, Andrew Lunn wrote:
>>> On Thu, Apr 28, 2016 at 04:03:57PM -0500, Josh Cartwright wrote:
>>>> On Thu, Apr 28, 2016 at 08:59:32PM +0200, Andrew Lunn wrote:
>>>>> On Thu, Apr 28, 2016 at 01:55:27PM -0500, Nathan Sullivan wrote:
>>>>>> On Thu, Apr 28, 2016 at 08:43:03PM +0200, Andrew Lunn wrote:
>>>>>>>> I agree that is a valid fix for AT91, however it won't solve our problem, since
>>>>>>>> we have no children on the second ethernet MAC in our devices' device trees. I'm
>>>>>>>> starting to feel like our second MAC shouldn't even really register the MDIO bus
>>>>>>>> since it isn't being used - maybe adding a DT property to not have a bus is a
>>>>>>>> better option?
>>>>>>>
>>>>>>> status = "disabled"
>>>>>>>
>>>>>>> would be the unusual way.
>>>>>>>
>>>>>>> Andrew
>>>>>>
>>>>>> Oh, sorry, I meant we use both MACs on Zynq, however the PHYs are on the MDIO
>>>>>> bus of the first MAC. So, the second MAC is used for ethernet but not for MDIO,
>>>>>> and so it does not have any PHYs under its DT node. It would be nice if there
>>>>>> were a way to tell macb not to bother with MDIO for the second MAC, since that's
>>>>>> handled by the first MAC.
>>>>>
>>>>> Yes, exactly, add support for status = "disabled" in the mdio node.
>>>>
>>>> Unfortunately, the 'macb' doesn't have a "mdio node", or alternatively:
>>>> the node representing the mdio bus is the same node which represents the
>>>> macb instance itself. Setting 'status = "disabled"' on this node will
>>>> just prevent the probing of the macb instance.
>>>
>>> :-(
>>>
>>> It is very common to have an mdio node within the MAC node, for example imx6sx-sdb.dtsi
>>
>> Okay, I think that makes sense. I think, then, perhaps the solution to
>> our problem is to:
>>
>> 1. Modify the macb driver to support an 'mdio' node. (And adjust the
>> binding document accordingly). If the node is found, it's used for
>> of_mdiobus_register() w/o any of the manual scan madness.
>> 2. For backwards compatibility, in the case where an 'mdio' node does
>> not exist, leave the existing behavior the way it is now
>> (of_mdiobus_register() followed by manual scan) [perhaps warn of
>> deprecation as well?]
>> 3. Update binding docs to reflect the above.
>>
>> In this way, for our usecase, the 'status = "disabled"' in the newly
>> created 'mdio' node isn't necessary. It's sufficient for the node to
>> exist and be empty.
>
> Here's a (only build tested) attempt at implementing a part of this. I
> macb_mii_init() was getting complicated enough that I lifted out two
> helper functions for the dt/no-dt case. Sweeping the in-tree
> devicetrees to update them to place phys under an 'mdio' node is still
> to be done.
>
> Josh
>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index eec3200..d843bc9 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -419,11 +419,62 @@ static int macb_mii_probe(struct net_device *dev)
> return 0;
> }
>
> +static int macb_mii_of_init(struct macb *bp, struct device_node *np)
> +{
> + struct device_node *mdio;
> + int err, i;
> +
> + mdio = of_get_child_by_name(np, "mdio");
> + if (mdio)
> + return of_mdiobus_register(bp->mii_bus, mdio);
> +
> + dev_warn(&bp->pdev->dev,
> + "using deprecated PHY probing mechanism. Please update device tree.");
Do we need to warn here?
Too bad I was not aware of that earlier, I even updated some of my DTs
recently with only a phy node without the "mdio" one as parents :-\
> + /* try dt phy registration */
> + err = of_mdiobus_register(bp->mii_bus, np);
> + if (err)
> + return err;
> +
> + /* fallback to standard phy registration if no phy were
> + * found during dt phy registration
> + */
> + if (!phy_find_first(bp->mii_bus)) {
> + for (i = 0; i < PHY_MAX_ADDR; i++) {
> + struct phy_device *phydev;
> +
> + phydev = mdiobus_scan(bp->mii_bus, i);
> + if (IS_ERR(phydev)) {
> + err = PTR_ERR(phydev);
> + break;
> + }
> + }
> +
> + if (err)
> + goto err_out_unregister_bus;
> + }
> +
> + return err;
> +
> +err_out_unregister_bus:
> + mdiobus_unregister(bp->mii_bus);
> + return err;
> +}
> +
> +static int macb_mii_pdata_init(struct macb *bp,
> + struct macb_platform_data *pdata)
> +{
> + if (pdata)
> + bp->mii_bus->phy_mask = pdata->phy_mask;
> +
> + return mdiobus_register(bp->mii_bus);
> +}
> +
> static int macb_mii_init(struct macb *bp)
> {
> struct macb_platform_data *pdata;
> struct device_node *np;
> - int err = -ENXIO, i;
> + int err = -ENXIO;
>
> /* Enable management port */
> macb_writel(bp, NCR, MACB_BIT(MPE));
> @@ -446,33 +497,10 @@ static int macb_mii_init(struct macb *bp)
> dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
>
> np = bp->pdev->dev.of_node;
> - if (np) {
> - /* try dt phy registration */
> - err = of_mdiobus_register(bp->mii_bus, np);
> -
> - /* fallback to standard phy registration if no phy were
> - * found during dt phy registration
> - */
> - if (!err && !phy_find_first(bp->mii_bus)) {
> - for (i = 0; i < PHY_MAX_ADDR; i++) {
> - struct phy_device *phydev;
> -
> - phydev = mdiobus_scan(bp->mii_bus, i);
> - if (IS_ERR(phydev)) {
> - err = PTR_ERR(phydev);
> - break;
> - }
> - }
> -
> - if (err)
> - goto err_out_unregister_bus;
> - }
> - } else {
> - if (pdata)
> - bp->mii_bus->phy_mask = pdata->phy_mask;
> -
> - err = mdiobus_register(bp->mii_bus);
> - }
> + if (np)
> + err = macb_mii_of_init(bp, np);
> + else
> + err = macb_mii_pdata_init(bp, pdata);
>
> if (err)
> goto err_out_free_mdiobus;
I'm okay with this. Thanks for having taken the initiative to implement it.
Bye,
--
Nicolas Ferre
^ permalink raw reply
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