* Re: [PATCH bpf] tools: bpf: fix NULL return handling in bpf__prepare_load
From: Daniel Borkmann @ 2018-05-15 14:20 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: YueHaibing, alexander.shishkin, mingo, peterz, netdev, namhyung
In-Reply-To: <20180515141143.GH2917@kernel.org>
On 05/15/2018 04:11 PM, Arnaldo Carvalho de Melo wrote:
> Em Sun, May 13, 2018 at 01:20:22AM +0200, Daniel Borkmann escreveu:
>> [ +Arnaldo ]
>>
>> On 05/11/2018 01:21 PM, YueHaibing wrote:
>>> bpf_object__open()/bpf_object__open_buffer can return error pointer or NULL,
>>> check the return values with IS_ERR_OR_NULL() in bpf__prepare_load and
>>> bpf__prepare_load_buffer
>>>
>>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
>>> ---
>>> tools/perf/util/bpf-loader.c | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> This should probably be routed via Arnaldo due to the fix in perf itself. If
>> there's no particular preference on which tree, we could potentially route it
>> as well via bpf with Acked-by from Arnaldo, but that is up to him. Arnaldo,
>> any preference?
>
> I'm preparing a pull req right now, and working a bit on perf's BPF
> support, so why not, I'll merge it, thanks,
Sounds good, thanks Arnaldo!
> - Arnaldo
^ permalink raw reply
* [PATCH net-next 0/2] sched: refactor NOLOCK qdiscs
From: Paolo Abeni @ 2018-05-15 14:24 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
John Fastabend, Michael S. Tsirkin
With the introduction of NOLOCK qdiscs, pfifo_fast performances in the
uncontended scenario degraded measurably, especially after the commit
eb82a9944792 ("net: sched, fix OOO packets with pfifo_fast").
This series restore the pfifo_fast performances in such scenario back the
previous level, mainly reducing the number of atomic operations required to
perform the qdisc_run() call. Even performances in the contended scenario
increase measurably.
Note: This series is on top of:
sched: manipulate __QDISC_STATE_RUNNING in qdisc_run_* helpers
Paolo Abeni (2):
sched: replace __QDISC_STATE_RUNNING bit with a spin lock
pfifo_fast: drop unneeded additional lock on dequeue
include/linux/skb_array.h | 5 +++++
include/net/sch_generic.h | 10 +++++-----
net/sched/sch_generic.c | 15 +++++++++++++--
3 files changed, 23 insertions(+), 7 deletions(-)
--
2.14.3
^ permalink raw reply
* [PATCH net-next 1/2] sched: replace __QDISC_STATE_RUNNING bit with a spin lock
From: Paolo Abeni @ 2018-05-15 14:24 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
John Fastabend, Michael S. Tsirkin
In-Reply-To: <cover.1526392746.git.pabeni@redhat.com>
So that we can use lockdep on it.
The newly introduced sequence lock has the same scope of busylock,
so it shares the same lockdep annotation, but it's only used for
NOLOCK qdiscs.
With this changeset we acquire such lock in the control path around
flushing operation (qdisc reset), to allow more NOLOCK qdisc perf
improvement in the next patch.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
include/net/sch_generic.h | 10 +++++-----
net/sched/sch_generic.c | 11 +++++++++++
2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 4d2b37226e75..98c10a28cd01 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -30,7 +30,6 @@ struct qdisc_rate_table {
enum qdisc_state_t {
__QDISC_STATE_SCHED,
__QDISC_STATE_DEACTIVATED,
- __QDISC_STATE_RUNNING,
};
struct qdisc_size_table {
@@ -102,6 +101,7 @@ struct Qdisc {
refcount_t refcnt;
spinlock_t busylock ____cacheline_aligned_in_smp;
+ spinlock_t seqlock;
};
static inline void qdisc_refcount_inc(struct Qdisc *qdisc)
@@ -111,17 +111,17 @@ static inline void qdisc_refcount_inc(struct Qdisc *qdisc)
refcount_inc(&qdisc->refcnt);
}
-static inline bool qdisc_is_running(const struct Qdisc *qdisc)
+static inline bool qdisc_is_running(struct Qdisc *qdisc)
{
if (qdisc->flags & TCQ_F_NOLOCK)
- return test_bit(__QDISC_STATE_RUNNING, &qdisc->state);
+ return spin_is_locked(&qdisc->seqlock);
return (raw_read_seqcount(&qdisc->running) & 1) ? true : false;
}
static inline bool qdisc_run_begin(struct Qdisc *qdisc)
{
if (qdisc->flags & TCQ_F_NOLOCK) {
- if (test_and_set_bit(__QDISC_STATE_RUNNING, &qdisc->state))
+ if (!spin_trylock(&qdisc->seqlock))
return false;
} else if (qdisc_is_running(qdisc)) {
return false;
@@ -138,7 +138,7 @@ static inline void qdisc_run_end(struct Qdisc *qdisc)
{
write_seqcount_end(&qdisc->running);
if (qdisc->flags & TCQ_F_NOLOCK)
- clear_bit(__QDISC_STATE_RUNNING, &qdisc->state);
+ spin_unlock(&qdisc->seqlock);
}
static inline bool qdisc_may_bulk(const struct Qdisc *qdisc)
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index ff3ce71aec93..a126f16bc30b 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -858,6 +858,11 @@ struct Qdisc *qdisc_alloc(struct netdev_queue *dev_queue,
lockdep_set_class(&sch->busylock,
dev->qdisc_tx_busylock ?: &qdisc_tx_busylock);
+ /* seqlock has the same scope of busylock, for NOLOCK qdisc */
+ spin_lock_init(&sch->seqlock);
+ lockdep_set_class(&sch->busylock,
+ dev->qdisc_tx_busylock ?: &qdisc_tx_busylock);
+
seqcount_init(&sch->running);
lockdep_set_class(&sch->running,
dev->qdisc_running_key ?: &qdisc_running_key);
@@ -1097,6 +1102,10 @@ static void dev_deactivate_queue(struct net_device *dev,
qdisc = rtnl_dereference(dev_queue->qdisc);
if (qdisc) {
+ bool nolock = qdisc->flags & TCQ_F_NOLOCK;
+
+ if (nolock)
+ spin_lock_bh(&qdisc->seqlock);
spin_lock_bh(qdisc_lock(qdisc));
if (!(qdisc->flags & TCQ_F_BUILTIN))
@@ -1106,6 +1115,8 @@ static void dev_deactivate_queue(struct net_device *dev,
qdisc_reset(qdisc);
spin_unlock_bh(qdisc_lock(qdisc));
+ if (nolock)
+ spin_unlock_bh(&qdisc->seqlock);
}
}
--
2.14.3
^ permalink raw reply related
* [PATCH net-next 2/2] pfifo_fast: drop unneeded additional lock on dequeue
From: Paolo Abeni @ 2018-05-15 14:24 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
John Fastabend, Michael S. Tsirkin
In-Reply-To: <cover.1526392746.git.pabeni@redhat.com>
After the previous patch, for NOLOCK qdiscs, q->seqlock is
always held when the dequeue() is invoked, we can drop
any additional locking to protect such operation.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
include/linux/skb_array.h | 5 +++++
net/sched/sch_generic.c | 4 ++--
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/include/linux/skb_array.h b/include/linux/skb_array.h
index a6b6e8bb3d7b..62d9b0a6329f 100644
--- a/include/linux/skb_array.h
+++ b/include/linux/skb_array.h
@@ -97,6 +97,11 @@ static inline bool skb_array_empty_any(struct skb_array *a)
return ptr_ring_empty_any(&a->ring);
}
+static inline struct sk_buff *__skb_array_consume(struct skb_array *a)
+{
+ return __ptr_ring_consume(&a->ring);
+}
+
static inline struct sk_buff *skb_array_consume(struct skb_array *a)
{
return ptr_ring_consume(&a->ring);
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index a126f16bc30b..760ab1b09f8b 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -656,7 +656,7 @@ static struct sk_buff *pfifo_fast_dequeue(struct Qdisc *qdisc)
if (__skb_array_empty(q))
continue;
- skb = skb_array_consume_bh(q);
+ skb = __skb_array_consume(q);
}
if (likely(skb)) {
qdisc_qstats_cpu_backlog_dec(qdisc, skb);
@@ -697,7 +697,7 @@ static void pfifo_fast_reset(struct Qdisc *qdisc)
if (!q->ring.queue)
continue;
- while ((skb = skb_array_consume_bh(q)) != NULL)
+ while ((skb = __skb_array_consume(q)) != NULL)
kfree_skb(skb);
}
--
2.14.3
^ permalink raw reply related
* Re: i.MX6S/DL and QCA8334 switch using DSA driver - CPU port not working
From: Michal Vokáč @ 2018-05-15 14:25 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, Vivien Didelot, Florian Fainelli
In-Reply-To: <20180510142904.GB7698@lunn.ch>
On 10.5.2018 16:29, Andrew Lunn wrote:
> I would probably add code to dump all the qca8k registers. Compare the
> values for your working setup and your non-working setup. Hopefully
> they are not too different, and you can quickly get to the bits which
> matter.
Perfect! Thanks to your suggestion I did that again and much more carefully.
After some tedious comparison I think I finally found the problem.
The RGMII works only if I write 0x7e to the PORT0_STATUS (0x7c) register
from setup. Then I found out that this setup is also described in
Qualcomm QCA8334 Q&A document. When I do that, everything work as expected.
Both PORT0 and PORT6 may be configured as xGMII, xMII and SerDes and their
functions may be exchanged. In all cases the port status register should be
set to 0x7X where X depends on the link speed setup.
Translated into register bits this means:
- clear BIT(12) - disable MAC flow control auto-negotiation (set by default)
- clear BIT(7) - disable MAC Tx flow control in half-duplex (set by default)
- set BIT(6) - use full-duplex
- set BIT(5,4) - enable Rx/Tx flow control
- set BIT(3) - enable Rx MAC - this one is tricky, the bit is described as
R/O in datasheet but it does not work when not set.
- set BIT(2) - enable Tx MAC
- set BIT(1,0) - set speed to 1000Mb
In general the fixed-link subnode is not handled in qca8k driver. That is why
the link speed and flow control is not set properly for the CPU port.
And obviously autonegotiation can not be used here.
I wonder whether there are some users of this driver and what may be their
setup that they are not affected by that?
I would like to have confirmed that I understand it correctly and that
the problem is really in the driver not handling fixed-link.
Then I can try to prepare a patch. It will be a small challenge for me
but I would like to do the work. I will look at the other drivers where
I think this is already implemented but any advice on how this should be
done will be appreciated.
Please confirm or correct my suggestions.
Thanks, Michal
^ permalink raw reply
* 'Adding' a writable proc file under /proc/net/afs/ [was [PATCH 3/3] afs: Implement namespacing]
From: David Howells @ 2018-05-15 14:28 UTC (permalink / raw)
To: Christoph Hellwig
Cc: dhowells, linux-afs, linux-kernel, netdev, linux-fsdevel
In-Reply-To: <20180515135333.GA31296@lst.de>
Christoph Hellwig <hch@lst.de> wrote:
> So the real question here is why afs needs a write callback as the first
> user of /proc/net ever in a day and time were we've deprecated adding
> new proc files. This is a discussion that should be had with linux-fsdevel
> and netdev in Cc.
/proc/fs/afs/cells and /proc/fs/afs/rootcell are not new proc files, nor is
the ability to configure the in-kernel afs filesystem by writing to them new.
The cells file has been there since 2002 and the rootcell file before the
start of the git history in 2005 - it's just that they've been in
/proc/fs/afs/ not /proc/net/afs/.
However, the afs procfiles need to be somewhere under /proc/net/ for the
network namespacing, so I moved the directory over and emplaced a symlink at
/proc/fs/afs.
David
^ permalink raw reply
* RE: [PATCH v1 iproute2-next 2/3] rdma: print driver resource attributes
From: Steve Wise @ 2018-05-15 14:31 UTC (permalink / raw)
To: 'Jason Gunthorpe'
Cc: 'Leon Romanovsky', dsahern, stephen, netdev, linux-rdma
In-Reply-To: <20180515135335.GA5615@ziepe.ca>
> From: Jason Gunthorpe <jgg@ziepe.ca>
> On Tue, May 15, 2018 at 08:18:51AM -0500, Steve Wise wrote:
> >
> > > > On Mon, May 14, 2018 at 05:04:26PM -0500, Steve Wise wrote:
> > > > >
> > > > >
> > > > > On 5/14/2018 3:41 PM, Jason Gunthorpe wrote:
> > > > > > On Mon, May 07, 2018 at 08:53:16AM -0700, Steve Wise wrote:
> > > > > >> This enhancement allows printing rdma device-specific state, if
> > > provided
> > > > > >> by the kernel. This is done in a generic manner, so rdma tool
> > doesn't
> > > > > >> need to know about the details of every type of rdma device.
> > > > > >>
> > > > > >> Driver attributes for a rdma resource are in the form of <key,
> > > > > >> [print_type], value> tuples, where the key is a string and the
> > value can
> > > > > >> be any supported driver attribute. The print_type attribute, if
> > present,
> > > > > >> provides a print format to use vs the standard print format for the
> > > type.
> > > > > >> For example, the default print type for a PROVIDER_S32 value is
> "%d
> > ",
> > > > > >> but "0x%x " if the print_type of PRINT_TYPE_HEX is included inthe
> > > tuple.
> > > > > >>
> > > > > >> Driver resources are only printed when the -dd flag is present.
> > > > > >> If -p is present, then the output is formatted to not exceed 80
> > > columns,
> > > > > >> otherwise it is printed as a single row to be grep/awk friendly.
> > > > > >>
> > > > > >> Example output:
> > > > > >>
> > > > > >> # rdma resource show qp lqpn 1028 -dd -p
> > > > > >> link cxgb4_0/- lqpn 1028 rqpn 0 type RC state RTS rq-psn 0 sq-psn 0
> > > > path-mig-state MIGRATED pid 0 comm [nvme_rdma]
> > > > > >> sqid 1028 flushed 0 memsize 123968 cidx 85 pidx 85 wq_pidx 106
> > > > flush_cidx 85 in_use 0
> > > > > >> size 386 flags 0x0 rqid 1029 memsize 16768 cidx 43 pidx 41
> > wq_pidx
> > > > 171 msn 44 rqt_hwaddr 0x2a8a5d00
> > > > > >> rqt_size 256 in_use 128 size 130 idx 43 wr_id
> > 0xffff881057c03408 idx
> > > > 40 wr_id 0xffff881057c033f0
> > > > > > Hey some of these look like kernel pointers.. That is a no-no.. What
> > > > > > is up there?
> > > > >
> > > > > Nothing is defined as a kernel pointer. But wr_id is often a pointer
> > to
> > > > > the kernel rdma application's context...
> > > > >
> > > > > > The wr_id often contains a pointer, right? So we cannot just pass it
> > > > > > to user space..
> > > > >
> > > > > Hmm. It is useful for debugging kernel rdma applications. Perhaps
> > > > > these attrs can be only be sent up by the kernel if the capabilities
> > > > > allow. But previous review comments of the kernel series, which is
> > now
> > > > > merged, forced me to remove passing the capabilities information to
> > the
> > > > > driver resource fill functions.
> > > > >
> > > > > So what's the right way to do this?
> > > >
> > > > The reviewer asked do not pass to drivers whole CAP_.. bits, because
> > > > they anyway don't need such granularity.
> > > >
> > >
> > > Ok thanks.
> >
> > How's this?
> >
> > diff --git a/include/rdma/restrack.h b/include/rdma/restrack.h
> > index 6379685..2cf9c5c 100644
> > +++ b/include/rdma/restrack.h
> > @@ -66,7 +66,8 @@ struct rdma_restrack_root {
> > * Allows rdma drivers to add their own restrack attributes.
> > */
> > int (*fill_res_entry)(struct sk_buff *msg,
> > - struct rdma_restrack_entry *entry);
> > + struct rdma_restrack_entry *entry,
> > + bool net_admin_capable);
> > };
>
> cap net admin is not high enough privledge to see unhashed kernel
> pointers. CAP_RAW_IO? Or follow what printk does?
>
Do you mean CAP_NET_RAW? Here's the comments for it:
/* Allow use of RAW sockets */
/* Allow use of PACKET sockets */
/* Allow binding to any address for transparent proxying (also via NET_ADMIN) */
Func restricted_pointer() from lib/vsprintf.c uses CAP_SYSLOG. The comment for CAP_SYSLOG:
/* Allow configuring the kernel's syslog (printk behaviour) */
Func kallsyms_show_value() also uses CAP_SYSLOG. And there is a non-exported global kptr_restrict that they use apparently to allow overriding all this for profiling.
Here is NET_ADMIN's comments:
/* Allow interface configuration */
/* Allow administration of IP firewall, masquerading and accounting */
/* Allow setting debug option on sockets */
/* Allow modification of routing tables */
/* Allow setting arbitrary process / process group ownership on
sockets */
/* Allow binding to any address for transparent proxying (also via NET_RAW) */
/* Allow setting TOS (type of service) */
/* Allow setting promiscuous mode */
/* Allow clearing driver statistics */
/* Allow multicasting */
/* Allow read/write of device-specific registers */
/* Allow activation of ATM control sockets */
^ permalink raw reply
* Re: [pull request][for-next 0/6] Mellanox, mlx5 updates 2018-05-07
From: Doug Ledford @ 2018-05-15 14:34 UTC (permalink / raw)
To: David Miller, saeedm; +Cc: netdev, linux-rdma, leonro, jgg
In-Reply-To: <20180510.081836.382051449994824032.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 621 bytes --]
On Thu, 2018-05-10 at 08:18 -0400, David Miller wrote:
> From: Saeed Mahameed <saeedm@mellanox.com>
> Date: Mon, 7 May 2018 16:52:58 -0700
>
> > This pull request includes misc updates and cleanups for mlx5 core
> > driver for both net and rdma next branches, for more information please
> > see tag log below.
> >
> > Please pull and let me know if there's any problem.
>
> Looks good, pulled into net-next.
>
> Thanks.
Thanks, pulled into rdma-next.
--
Doug Ledford <dledford@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 04/40] proc: introduce proc_create_seq{, _data}
From: Christoph Hellwig @ 2018-05-15 14:42 UTC (permalink / raw)
To: David Howells
Cc: linux-rtc-u79uwXL29TY76Z2rM5mHXA, Alessandro Zummo,
Alexandre Belloni, devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
linux-scsi-u79uwXL29TY76Z2rM5mHXA,
linux-acpi-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman, Jiri Slaby,
megaraidlinux.pdl-dY08KVG/lbpWk0Htik3J/w,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-afs-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-ide-u79uwXL29TY76Z2rM5mHXA,
netfilter-devel-u79uwXL29TY76Z2rM5mHXA, Alexander Viro,
netdev-u79uwXL29TY76Z2rM5mHXA, Andrew Morton,
linux-ext4-u79uwXL29TY76Z2rM5mHXA,
drbd-dev-cunTk1MwBs8qoQakbn7OcQ, Christoph Hellwig,
jfs-discussion-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Alexey Dobriyan
In-Reply-To: <26540.1525094365-S6HVgzuS8uM4Awkfq6JHfwNdhmdF6hFW@public.gmane.org>
On Mon, Apr 30, 2018 at 02:19:25PM +0100, David Howells wrote:
> Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org> wrote:
>
> > +
> > +struct proc_dir_entry *proc_create_seq_data(const char *name, umode_t mode,
> > + struct proc_dir_entry *parent, const struct seq_operations *ops,
> > + void *data)
> > +{
> > ...
> > +EXPORT_SYMBOL(proc_create_seq_data);
>
> Please add documentation comments to exported functions when you add them.
None of the base functions are document, and we really want people to not
use procfs for new code anyway. But if I get some consensus from the
maintainers and the list I can throw in another patch to document
all proc_create* variants.
^ permalink raw reply
* Re: [PATCH v1 iproute2-next 2/3] rdma: print driver resource attributes
From: Jason Gunthorpe @ 2018-05-15 14:44 UTC (permalink / raw)
To: Steve Wise
Cc: 'Leon Romanovsky', dsahern, stephen, netdev, linux-rdma
In-Reply-To: <012201d3ec59$69242fa0$3b6c8ee0$@opengridcomputing.com>
On Tue, May 15, 2018 at 09:31:27AM -0500, Steve Wise wrote:
> > cap net admin is not high enough privledge to see unhashed kernel
> > pointers. CAP_RAW_IO? Or follow what printk does?
> >
>
> Do you mean CAP_NET_RAW? Here's the comments for it:
Nope..
> Func restricted_pointer() from lib/vsprintf.c uses CAP_SYSLOG. The comment for CAP_SYSLOG:
Yikes, yes, that is probably the required logic here, including the
kptr_restrict = 0 thing
Jason
^ permalink raw reply
* Re: [PATCH 11/40] ipv6/flowlabel: simplify pid namespace lookup
From: Christoph Hellwig @ 2018-05-15 14:56 UTC (permalink / raw)
To: Eric W. Biederman
Cc: linux-rtc-u79uwXL29TY76Z2rM5mHXA, Alessandro Zummo,
Alexandre Belloni, devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
linux-afs-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-scsi-u79uwXL29TY76Z2rM5mHXA,
linux-ide-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
jfs-discussion-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
Christoph Hellwig, linux-acpi-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
netfilter-devel-u79uwXL29TY76Z2rM5mHXA, Alexander Viro,
Jiri Slaby, Andrew Morton, linux-ext4-u79uwXL29TY76Z2rM5mHXA,
Alexey Dobriyan, megaraidlinux.pdl-dY08KVG/lbpWk0Htik3J/w,
drbd-dev-cunTk1MwBs8qoQakbn7OcQ
In-Reply-To: <878t8y46sy.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
On Sat, May 05, 2018 at 07:37:33AM -0500, Eric W. Biederman wrote:
> Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org> writes:
>
> > The shole seq_file sequence already operates under a single RCU lock pair,
> > so move the pid namespace lookup into it, and stop grabbing a reference
> > and remove all kinds of boilerplate code.
>
> This is wrong.
>
> Move task_active_pid_ns(current) from open to seq_start actually means
> that the results if you pass this proc file between callers the results
> will change. So this breaks file descriptor passing.
>
> Open is a bad place to access current. In the middle of read/write is
> broken.
>
>
> In this particular instance looking up the pid namespace with
> task_active_pid_ns was a personal brain fart. What the code should be
> doing (with an appropriate helper) is:
>
> struct pid_namespace *pid_ns = inode->i_sb->s_fs_info;
>
> Because each mount of proc is bound to a pid namespace. Looking up the
> pid namespace from the super_block is a much better way to go.
What do you have in mind for the helper? For now I've thrown it in
opencoded into my working tree, but I'd be glad to add a helper.
struct pid_namespace *proc_pid_namespace(struct inode *inode)
{
// maybe warn on for s_magic not on procfs??
return inode->i_sb->s_fs_info;
}
?
^ permalink raw reply
* RE: [PATCH v1 iproute2-next 2/3] rdma: print driver resource attributes
From: Steve Wise @ 2018-05-15 15:02 UTC (permalink / raw)
To: 'Jason Gunthorpe'
Cc: 'Leon Romanovsky', dsahern, stephen, netdev, linux-rdma
In-Reply-To: <20180515144420.GB5615@ziepe.ca>
> On Tue, May 15, 2018 at 09:31:27AM -0500, Steve Wise wrote:
> > > cap net admin is not high enough privledge to see unhashed kernel
> > > pointers. CAP_RAW_IO? Or follow what printk does?
> > >
> >
> > Do you mean CAP_NET_RAW? Here's the comments for it:
>
> Nope..
>
> > Func restricted_pointer() from lib/vsprintf.c uses CAP_SYSLOG. The
> comment for CAP_SYSLOG:
>
> Yikes, yes, that is probably the required logic here, including the
> kptr_restrict = 0 thing
>
Let's defer the ktpr_restrict issue for now; I want to finish the initial
work this cycle, and adding that will likely take too much time. I'll use
CAP_SYSLOG and add a FIXME comment. Ok?
Steve.
^ permalink raw reply
* [PATCH net-next 00/10] net/smc: enhancements 2018/05/15
From: Ursula Braun @ 2018-05-15 15:04 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
From: Ursula Braun <ursula.braun@de.ibm.com>
Dave,
here are smc patches for net-next. The first one is a fix for net-next
commit 01d2f7e2cdd3 "net/smc: sockopts TCP_NODELAY and TCP_CORK".
Patch 7 improves Connection Layer Control error handling, patch 10
improves abnormal termination of link groups. The remaining patches
from Karsten improve Link Layer Control code.
Thanks, Ursula
Karsten Graul (9):
net/smc: register new rmbs with the peer
net/smc: remove unnecessary cast
net/smc: simplify test_link function usage
net/smc: move link llc initialization to llc layer
net/smc: use a workqueue to defer llc send
net/smc: handle all error codes from smc_conn_create()
net/smc: set link inactive before calling smc_lgr_free()
net/smc: drop messages when link state is inactive
net/smc: check for pending termination
Ursula Braun (1):
net/smc: no tx work trigger for fallback sockets
net/smc/af_smc.c | 27 +++++---
net/smc/smc_clc.c | 2 +-
net/smc/smc_core.c | 23 ++++---
net/smc/smc_core.h | 6 +-
net/smc/smc_llc.c | 196 +++++++++++++++++++++++++++++++++++++++++------------
net/smc/smc_llc.h | 7 +-
6 files changed, 196 insertions(+), 65 deletions(-)
--
2.16.3
^ permalink raw reply
* [PATCH net-next 01/10] net/smc: no tx work trigger for fallback sockets
From: Ursula Braun @ 2018-05-15 15:04 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20180515150503.47265-1-ubraun@linux.ibm.com>
If TCP_NODELAY is set or TCP_CORK is reset, setsockopt triggers the
tx worker. This does not make sense, if the SMC socket switched to
the TCP fallback when the connection is created. This patch adds
the additional check for the fallback case.
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
net/smc/af_smc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 17688a02035b..83403be46a4a 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -1353,14 +1353,14 @@ static int smc_setsockopt(struct socket *sock, int level, int optname,
break;
case TCP_NODELAY:
if (sk->sk_state != SMC_INIT && sk->sk_state != SMC_LISTEN) {
- if (val)
+ if (val && !smc->use_fallback)
mod_delayed_work(system_wq, &smc->conn.tx_work,
0);
}
break;
case TCP_CORK:
if (sk->sk_state != SMC_INIT && sk->sk_state != SMC_LISTEN) {
- if (!val)
+ if (!val && !smc->use_fallback)
mod_delayed_work(system_wq, &smc->conn.tx_work,
0);
}
--
2.16.3
^ permalink raw reply related
* [PATCH net-next 02/10] net/smc: register new rmbs with the peer
From: Ursula Braun @ 2018-05-15 15:04 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20180515150503.47265-1-ubraun@linux.ibm.com>
From: Karsten Graul <kgraul@linux.ibm.com>
Register new rmb buffers with the remote peer by exchanging a
confirm_rkey llc message.
Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
net/smc/af_smc.c | 20 ++++++++++++++------
net/smc/smc_core.c | 1 +
net/smc/smc_core.h | 2 ++
net/smc/smc_llc.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
net/smc/smc_llc.h | 2 ++
5 files changed, 64 insertions(+), 8 deletions(-)
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 83403be46a4a..397ba2182453 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -293,14 +293,22 @@ static void smc_copy_sock_settings_to_smc(struct smc_sock *smc)
smc_copy_sock_settings(&smc->sk, smc->clcsock->sk, SK_FLAGS_CLC_TO_SMC);
}
-/* register a new rmb */
-static int smc_reg_rmb(struct smc_link *link, struct smc_buf_desc *rmb_desc)
+/* register a new rmb, optionally send confirm_rkey msg to register with peer */
+static int smc_reg_rmb(struct smc_link *link, struct smc_buf_desc *rmb_desc,
+ bool conf_rkey)
{
/* register memory region for new rmb */
if (smc_wr_reg_send(link, rmb_desc->mr_rx[SMC_SINGLE_LINK])) {
rmb_desc->regerr = 1;
return -EFAULT;
}
+ if (!conf_rkey)
+ return 0;
+ /* exchange confirm_rkey msg with peer */
+ if (smc_llc_do_confirm_rkey(link, rmb_desc)) {
+ rmb_desc->regerr = 1;
+ return -EFAULT;
+ }
return 0;
}
@@ -334,7 +342,7 @@ static int smc_clnt_conf_first_link(struct smc_sock *smc)
smc_wr_remember_qp_attr(link);
- if (smc_reg_rmb(link, smc->conn.rmb_desc))
+ if (smc_reg_rmb(link, smc->conn.rmb_desc, false))
return SMC_CLC_DECL_INTERR;
/* send CONFIRM LINK response over RoCE fabric */
@@ -488,7 +496,7 @@ static int smc_connect_rdma(struct smc_sock *smc)
}
} else {
if (!smc->conn.rmb_desc->reused) {
- if (smc_reg_rmb(link, smc->conn.rmb_desc)) {
+ if (smc_reg_rmb(link, smc->conn.rmb_desc, true)) {
reason_code = SMC_CLC_DECL_INTERR;
goto decline_rdma_unlock;
}
@@ -729,7 +737,7 @@ static int smc_serv_conf_first_link(struct smc_sock *smc)
link = &lgr->lnk[SMC_SINGLE_LINK];
- if (smc_reg_rmb(link, smc->conn.rmb_desc))
+ if (smc_reg_rmb(link, smc->conn.rmb_desc, false))
return SMC_CLC_DECL_INTERR;
/* send CONFIRM LINK request to client over the RoCE fabric */
@@ -866,7 +874,7 @@ static void smc_listen_work(struct work_struct *work)
if (local_contact != SMC_FIRST_CONTACT) {
if (!new_smc->conn.rmb_desc->reused) {
- if (smc_reg_rmb(link, new_smc->conn.rmb_desc)) {
+ if (smc_reg_rmb(link, new_smc->conn.rmb_desc, true)) {
reason_code = SMC_CLC_DECL_INTERR;
goto decline_rdma_unlock;
}
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 9c74844e2008..29ae077b0ec9 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -210,6 +210,7 @@ static int smc_lgr_create(struct smc_sock *smc,
init_completion(&lnk->llc_confirm_resp);
init_completion(&lnk->llc_add);
init_completion(&lnk->llc_add_resp);
+ init_completion(&lnk->llc_confirm_rkey);
smc->conn.lgr = lgr;
rwlock_init(&lgr->conns_lock);
diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h
index fca8624e5e71..9398e235d74b 100644
--- a/net/smc/smc_core.h
+++ b/net/smc/smc_core.h
@@ -105,6 +105,8 @@ struct smc_link {
struct delayed_work llc_testlink_wrk; /* testlink worker */
struct completion llc_testlink_resp; /* wait for rx of testlink */
int llc_testlink_time; /* testlink interval */
+ struct completion llc_confirm_rkey; /* wait 4 rx of cnf rkey */
+ int llc_confirm_rkey_rc; /* rc from cnf rkey msg */
};
/* For now we just allow one parallel link per link group. The SMC protocol
diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
index 33b4d856f4c6..b118f80b9c41 100644
--- a/net/smc/smc_llc.c
+++ b/net/smc/smc_llc.c
@@ -214,6 +214,31 @@ int smc_llc_send_confirm_link(struct smc_link *link, u8 mac[],
return rc;
}
+/* send LLC confirm rkey request */
+static int smc_llc_send_confirm_rkey(struct smc_link *link,
+ struct smc_buf_desc *rmb_desc)
+{
+ struct smc_llc_msg_confirm_rkey *rkeyllc;
+ struct smc_wr_tx_pend_priv *pend;
+ struct smc_wr_buf *wr_buf;
+ int rc;
+
+ rc = smc_llc_add_pending_send(link, &wr_buf, &pend);
+ if (rc)
+ return rc;
+ rkeyllc = (struct smc_llc_msg_confirm_rkey *)wr_buf;
+ memset(rkeyllc, 0, sizeof(*rkeyllc));
+ rkeyllc->hd.common.type = SMC_LLC_CONFIRM_RKEY;
+ rkeyllc->hd.length = sizeof(struct smc_llc_msg_confirm_rkey);
+ rkeyllc->rtoken[0].rmb_key =
+ htonl(rmb_desc->mr_rx[SMC_SINGLE_LINK]->rkey);
+ rkeyllc->rtoken[0].rmb_vaddr = cpu_to_be64(
+ (u64)sg_dma_address(rmb_desc->sgt[SMC_SINGLE_LINK].sgl));
+ /* send llc message */
+ rc = smc_wr_tx_send(link, pend);
+ return rc;
+}
+
/* send ADD LINK request or response */
int smc_llc_send_add_link(struct smc_link *link, u8 mac[],
union ib_gid *gid,
@@ -413,7 +438,9 @@ static void smc_llc_rx_confirm_rkey(struct smc_link *link,
lgr = container_of(link, struct smc_link_group, lnk[SMC_SINGLE_LINK]);
if (llc->hd.flags & SMC_LLC_FLAG_RESP) {
- /* unused as long as we don't send this type of msg */
+ link->llc_confirm_rkey_rc = llc->hd.flags &
+ SMC_LLC_FLAG_RKEY_NEG;
+ complete(&link->llc_confirm_rkey);
} else {
rc = smc_rtoken_add(lgr,
llc->rtoken[0].rmb_vaddr,
@@ -503,7 +530,7 @@ static void smc_llc_rx_handler(struct ib_wc *wc, void *buf)
}
}
-/***************************** worker ****************************************/
+/***************************** worker, utils *********************************/
static void smc_llc_testlink_work(struct work_struct *work)
{
@@ -562,6 +589,22 @@ void smc_llc_link_flush(struct smc_link *link)
cancel_delayed_work_sync(&link->llc_testlink_wrk);
}
+/* register a new rtoken at the remote peer */
+int smc_llc_do_confirm_rkey(struct smc_link *link,
+ struct smc_buf_desc *rmb_desc)
+{
+ int rc;
+
+ reinit_completion(&link->llc_confirm_rkey);
+ smc_llc_send_confirm_rkey(link, rmb_desc);
+ /* receive CONFIRM RKEY response from server over RoCE fabric */
+ rc = wait_for_completion_interruptible_timeout(&link->llc_confirm_rkey,
+ SMC_LLC_WAIT_TIME);
+ if (rc <= 0 || link->llc_confirm_rkey_rc)
+ return -EFAULT;
+ return 0;
+}
+
/***************************** init, exit, misc ******************************/
static struct smc_wr_rx_handler smc_llc_rx_handlers[] = {
diff --git a/net/smc/smc_llc.h b/net/smc/smc_llc.h
index d6e42116485e..728823dd8ae5 100644
--- a/net/smc/smc_llc.h
+++ b/net/smc/smc_llc.h
@@ -47,6 +47,8 @@ int smc_llc_send_test_link(struct smc_link *lnk, u8 user_data[16],
void smc_llc_link_active(struct smc_link *link, int testlink_time);
void smc_llc_link_inactive(struct smc_link *link);
void smc_llc_link_flush(struct smc_link *link);
+int smc_llc_do_confirm_rkey(struct smc_link *link,
+ struct smc_buf_desc *rmb_desc);
int smc_llc_init(void) __init;
#endif /* SMC_LLC_H */
--
2.16.3
^ permalink raw reply related
* [PATCH net-next 06/10] net/smc: use a workqueue to defer llc send
From: Ursula Braun @ 2018-05-15 15:04 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20180515150503.47265-1-ubraun@linux.ibm.com>
From: Karsten Graul <kgraul@linux.ibm.com>
SMC handles deferred work in tasklets. As tasklets cannot sleep this
can result in rare EBUSY conditions, so defer this work in a work queue.
The high level api functions do not defer work because they can sleep
until the llc send is actually completed.
Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
net/smc/smc_core.c | 10 ++--
net/smc/smc_core.h | 1 +
net/smc/smc_llc.c | 132 ++++++++++++++++++++++++++++++++++++++---------------
net/smc/smc_llc.h | 4 +-
4 files changed, 104 insertions(+), 43 deletions(-)
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index ba7dae819ac8..4523bbad8a15 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -194,10 +194,12 @@ static int smc_lgr_create(struct smc_sock *smc,
smc_ib_setup_per_ibdev(smcibdev);
get_random_bytes(rndvec, sizeof(rndvec));
lnk->psn_initial = rndvec[0] + (rndvec[1] << 8) + (rndvec[2] << 16);
- smc_llc_link_init(lnk);
- rc = smc_wr_alloc_link_mem(lnk);
+ rc = smc_llc_link_init(lnk);
if (rc)
goto free_lgr;
+ rc = smc_wr_alloc_link_mem(lnk);
+ if (rc)
+ goto clear_llc_lnk;
rc = smc_ib_create_protection_domain(lnk);
if (rc)
goto free_link_mem;
@@ -221,6 +223,8 @@ static int smc_lgr_create(struct smc_sock *smc,
smc_ib_dealloc_protection_domain(lnk);
free_link_mem:
smc_wr_free_link_mem(lnk);
+clear_llc_lnk:
+ smc_llc_link_clear(lnk);
free_lgr:
kfree(lgr);
out:
@@ -266,6 +270,7 @@ void smc_conn_free(struct smc_connection *conn)
static void smc_link_clear(struct smc_link *lnk)
{
lnk->peer_qpn = 0;
+ smc_llc_link_clear(lnk);
smc_ib_modify_qp_reset(lnk);
smc_wr_free_link(lnk);
smc_ib_destroy_queue_pair(lnk);
@@ -323,7 +328,6 @@ static void smc_lgr_free_bufs(struct smc_link_group *lgr)
/* remove a link group */
void smc_lgr_free(struct smc_link_group *lgr)
{
- smc_llc_link_flush(&lgr->lnk[SMC_SINGLE_LINK]);
smc_lgr_free_bufs(lgr);
smc_link_clear(&lgr->lnk[SMC_SINGLE_LINK]);
kfree(lgr);
diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h
index 9398e235d74b..d1753850684b 100644
--- a/net/smc/smc_core.h
+++ b/net/smc/smc_core.h
@@ -96,6 +96,7 @@ struct smc_link {
u8 link_id; /* unique # within link group */
enum smc_link_state state; /* state of link */
+ struct workqueue_struct *llc_wq; /* single thread work queue */
struct completion llc_confirm; /* wait for rx of conf link */
struct completion llc_confirm_resp; /* wait 4 rx of cnf lnk rsp */
int llc_confirm_rc; /* rc from confirm link msg */
diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
index 5dbeced6cea5..5d7926cc472c 100644
--- a/net/smc/smc_llc.c
+++ b/net/smc/smc_llc.c
@@ -239,6 +239,25 @@ static int smc_llc_send_confirm_rkey(struct smc_link *link,
return rc;
}
+/* prepare an add link message */
+static void smc_llc_prep_add_link(struct smc_llc_msg_add_link *addllc,
+ struct smc_link *link, u8 mac[],
+ union ib_gid *gid,
+ enum smc_llc_reqresp reqresp)
+{
+ memset(addllc, 0, sizeof(*addllc));
+ addllc->hd.common.type = SMC_LLC_ADD_LINK;
+ addllc->hd.length = sizeof(struct smc_llc_msg_add_link);
+ if (reqresp == SMC_LLC_RESP) {
+ addllc->hd.flags |= SMC_LLC_FLAG_RESP;
+ /* always reject more links for now */
+ addllc->hd.flags |= SMC_LLC_FLAG_ADD_LNK_REJ;
+ addllc->hd.add_link_rej_rsn = SMC_LLC_REJ_RSN_NO_ALT_PATH;
+ }
+ memcpy(addllc->sender_mac, mac, ETH_ALEN);
+ memcpy(addllc->sender_gid, gid, SMC_GID_SIZE);
+}
+
/* send ADD LINK request or response */
int smc_llc_send_add_link(struct smc_link *link, u8 mac[],
union ib_gid *gid,
@@ -253,22 +272,28 @@ int smc_llc_send_add_link(struct smc_link *link, u8 mac[],
if (rc)
return rc;
addllc = (struct smc_llc_msg_add_link *)wr_buf;
- memset(addllc, 0, sizeof(*addllc));
- addllc->hd.common.type = SMC_LLC_ADD_LINK;
- addllc->hd.length = sizeof(struct smc_llc_msg_add_link);
- if (reqresp == SMC_LLC_RESP) {
- addllc->hd.flags |= SMC_LLC_FLAG_RESP;
- /* always reject more links for now */
- addllc->hd.flags |= SMC_LLC_FLAG_ADD_LNK_REJ;
- addllc->hd.add_link_rej_rsn = SMC_LLC_REJ_RSN_NO_ALT_PATH;
- }
- memcpy(addllc->sender_mac, mac, ETH_ALEN);
- memcpy(addllc->sender_gid, gid, SMC_GID_SIZE);
+ smc_llc_prep_add_link(addllc, link, mac, gid, reqresp);
/* send llc message */
rc = smc_wr_tx_send(link, pend);
return rc;
}
+/* prepare a delete link message */
+static void smc_llc_prep_delete_link(struct smc_llc_msg_del_link *delllc,
+ struct smc_link *link,
+ enum smc_llc_reqresp reqresp)
+{
+ memset(delllc, 0, sizeof(*delllc));
+ delllc->hd.common.type = SMC_LLC_DELETE_LINK;
+ delllc->hd.length = sizeof(struct smc_llc_msg_add_link);
+ if (reqresp == SMC_LLC_RESP)
+ delllc->hd.flags |= SMC_LLC_FLAG_RESP;
+ /* DEL_LINK_ALL because only 1 link supported */
+ delllc->hd.flags |= SMC_LLC_FLAG_DEL_LINK_ALL;
+ delllc->hd.flags |= SMC_LLC_FLAG_DEL_LINK_ORDERLY;
+ delllc->link_num = link->link_id;
+}
+
/* send DELETE LINK request or response */
int smc_llc_send_delete_link(struct smc_link *link,
enum smc_llc_reqresp reqresp)
@@ -282,15 +307,7 @@ int smc_llc_send_delete_link(struct smc_link *link,
if (rc)
return rc;
delllc = (struct smc_llc_msg_del_link *)wr_buf;
- memset(delllc, 0, sizeof(*delllc));
- delllc->hd.common.type = SMC_LLC_DELETE_LINK;
- delllc->hd.length = sizeof(struct smc_llc_msg_add_link);
- if (reqresp == SMC_LLC_RESP)
- delllc->hd.flags |= SMC_LLC_FLAG_RESP;
- /* DEL_LINK_ALL because only 1 link supported */
- delllc->hd.flags |= SMC_LLC_FLAG_DEL_LINK_ALL;
- delllc->hd.flags |= SMC_LLC_FLAG_DEL_LINK_ORDERLY;
- delllc->link_num = link->link_id;
+ smc_llc_prep_delete_link(delllc, link, reqresp);
/* send llc message */
rc = smc_wr_tx_send(link, pend);
return rc;
@@ -317,20 +334,46 @@ static int smc_llc_send_test_link(struct smc_link *link, u8 user_data[16])
return rc;
}
-/* send a prepared message */
-static int smc_llc_send_message(struct smc_link *link, void *llcbuf, int llclen)
+struct smc_llc_send_work {
+ struct work_struct work;
+ struct smc_link *link;
+ int llclen;
+ union smc_llc_msg llcbuf;
+};
+
+/* worker that sends a prepared message */
+static void smc_llc_send_message_work(struct work_struct *work)
{
+ struct smc_llc_send_work *llcwrk = container_of(work,
+ struct smc_llc_send_work, work);
struct smc_wr_tx_pend_priv *pend;
struct smc_wr_buf *wr_buf;
int rc;
- rc = smc_llc_add_pending_send(link, &wr_buf, &pend);
+ if (llcwrk->link->state == SMC_LNK_INACTIVE)
+ goto out;
+ rc = smc_llc_add_pending_send(llcwrk->link, &wr_buf, &pend);
if (rc)
- return rc;
- memcpy(wr_buf, llcbuf, llclen);
- /* send llc message */
- rc = smc_wr_tx_send(link, pend);
- return rc;
+ goto out;
+ memcpy(wr_buf, &llcwrk->llcbuf, llcwrk->llclen);
+ smc_wr_tx_send(llcwrk->link, pend);
+out:
+ kfree(llcwrk);
+}
+
+/* copy llcbuf and schedule an llc send on link */
+static int smc_llc_send_message(struct smc_link *link, void *llcbuf, int llclen)
+{
+ struct smc_llc_send_work *wrk = kmalloc(sizeof(*wrk), GFP_ATOMIC);
+
+ if (!wrk)
+ return -ENOMEM;
+ INIT_WORK(&wrk->work, smc_llc_send_message_work);
+ wrk->link = link;
+ wrk->llclen = llclen;
+ memcpy(&wrk->llcbuf, llcbuf, llclen);
+ queue_work(link->llc_wq, &wrk->work);
+ return 0;
}
/********************************* receive ***********************************/
@@ -381,17 +424,18 @@ static void smc_llc_rx_add_link(struct smc_link *link,
}
if (lgr->role == SMC_SERV) {
- smc_llc_send_add_link(link,
+ smc_llc_prep_add_link(llc, link,
link->smcibdev->mac[link->ibport - 1],
&link->smcibdev->gid[link->ibport - 1],
SMC_LLC_REQ);
} else {
- smc_llc_send_add_link(link,
+ smc_llc_prep_add_link(llc, link,
link->smcibdev->mac[link->ibport - 1],
&link->smcibdev->gid[link->ibport - 1],
SMC_LLC_RESP);
}
+ smc_llc_send_message(link, llc, sizeof(*llc));
}
}
@@ -407,9 +451,11 @@ static void smc_llc_rx_delete_link(struct smc_link *link,
} else {
if (lgr->role == SMC_SERV) {
smc_lgr_forget(lgr);
- smc_llc_send_delete_link(link, SMC_LLC_REQ);
+ smc_llc_prep_delete_link(llc, link, SMC_LLC_REQ);
+ smc_llc_send_message(link, llc, sizeof(*llc));
} else {
- smc_llc_send_delete_link(link, SMC_LLC_RESP);
+ smc_llc_prep_delete_link(llc, link, SMC_LLC_RESP);
+ smc_llc_send_message(link, llc, sizeof(*llc));
smc_lgr_terminate(lgr);
}
}
@@ -559,11 +605,19 @@ static void smc_llc_testlink_work(struct work_struct *work)
}
next_interval = link->llc_testlink_time;
out:
- schedule_delayed_work(&link->llc_testlink_wrk, next_interval);
+ queue_delayed_work(link->llc_wq, &link->llc_testlink_wrk,
+ next_interval);
}
-void smc_llc_link_init(struct smc_link *link)
+int smc_llc_link_init(struct smc_link *link)
{
+ struct smc_link_group *lgr = container_of(link, struct smc_link_group,
+ lnk[SMC_SINGLE_LINK]);
+ link->llc_wq = alloc_ordered_workqueue("llc_wq-%x:%x)", WQ_MEM_RECLAIM,
+ *((u32 *)lgr->id),
+ link->link_id);
+ if (!link->llc_wq)
+ return -ENOMEM;
init_completion(&link->llc_confirm);
init_completion(&link->llc_confirm_resp);
init_completion(&link->llc_add);
@@ -571,6 +625,7 @@ void smc_llc_link_init(struct smc_link *link)
init_completion(&link->llc_confirm_rkey);
init_completion(&link->llc_testlink_resp);
INIT_DELAYED_WORK(&link->llc_testlink_wrk, smc_llc_testlink_work);
+ return 0;
}
void smc_llc_link_active(struct smc_link *link, int testlink_time)
@@ -578,8 +633,8 @@ void smc_llc_link_active(struct smc_link *link, int testlink_time)
link->state = SMC_LNK_ACTIVE;
if (testlink_time) {
link->llc_testlink_time = testlink_time * HZ;
- schedule_delayed_work(&link->llc_testlink_wrk,
- link->llc_testlink_time);
+ queue_delayed_work(link->llc_wq, &link->llc_testlink_wrk,
+ link->llc_testlink_time);
}
}
@@ -591,9 +646,10 @@ void smc_llc_link_inactive(struct smc_link *link)
}
/* called in worker context */
-void smc_llc_link_flush(struct smc_link *link)
+void smc_llc_link_clear(struct smc_link *link)
{
- cancel_delayed_work_sync(&link->llc_testlink_wrk);
+ flush_workqueue(link->llc_wq);
+ destroy_workqueue(link->llc_wq);
}
/* register a new rtoken at the remote peer */
diff --git a/net/smc/smc_llc.h b/net/smc/smc_llc.h
index d7f52a60c8d6..65c8645e96a1 100644
--- a/net/smc/smc_llc.h
+++ b/net/smc/smc_llc.h
@@ -42,10 +42,10 @@ int smc_llc_send_add_link(struct smc_link *link, u8 mac[], union ib_gid *gid,
enum smc_llc_reqresp reqresp);
int smc_llc_send_delete_link(struct smc_link *link,
enum smc_llc_reqresp reqresp);
-void smc_llc_link_init(struct smc_link *link);
+int smc_llc_link_init(struct smc_link *link);
void smc_llc_link_active(struct smc_link *link, int testlink_time);
void smc_llc_link_inactive(struct smc_link *link);
-void smc_llc_link_flush(struct smc_link *link);
+void smc_llc_link_clear(struct smc_link *link);
int smc_llc_do_confirm_rkey(struct smc_link *link,
struct smc_buf_desc *rmb_desc);
int smc_llc_init(void) __init;
--
2.16.3
^ permalink raw reply related
* [PATCH net-next 05/10] net/smc: move link llc initialization to llc layer
From: Ursula Braun @ 2018-05-15 15:04 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20180515150503.47265-1-ubraun@linux.ibm.com>
From: Karsten Graul <kgraul@linux.ibm.com>
Move the llc layer specific initialization and cleanup out of smc_core.c
into smc_llc.c (smc_llc_link_init and smc_llc_link_clear). Move all
initialization of a link into the new init function.
Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
net/smc/smc_core.c | 6 +-----
net/smc/smc_llc.c | 11 ++++++++++-
net/smc/smc_llc.h | 1 +
3 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 29ae077b0ec9..ba7dae819ac8 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -194,6 +194,7 @@ static int smc_lgr_create(struct smc_sock *smc,
smc_ib_setup_per_ibdev(smcibdev);
get_random_bytes(rndvec, sizeof(rndvec));
lnk->psn_initial = rndvec[0] + (rndvec[1] << 8) + (rndvec[2] << 16);
+ smc_llc_link_init(lnk);
rc = smc_wr_alloc_link_mem(lnk);
if (rc)
goto free_lgr;
@@ -206,11 +207,6 @@ static int smc_lgr_create(struct smc_sock *smc,
rc = smc_wr_create_link(lnk);
if (rc)
goto destroy_qp;
- init_completion(&lnk->llc_confirm);
- init_completion(&lnk->llc_confirm_resp);
- init_completion(&lnk->llc_add);
- init_completion(&lnk->llc_add_resp);
- init_completion(&lnk->llc_confirm_rkey);
smc->conn.lgr = lgr;
rwlock_init(&lgr->conns_lock);
diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
index 229e967c7940..5dbeced6cea5 100644
--- a/net/smc/smc_llc.c
+++ b/net/smc/smc_llc.c
@@ -562,10 +562,19 @@ static void smc_llc_testlink_work(struct work_struct *work)
schedule_delayed_work(&link->llc_testlink_wrk, next_interval);
}
-void smc_llc_link_active(struct smc_link *link, int testlink_time)
+void smc_llc_link_init(struct smc_link *link)
{
+ init_completion(&link->llc_confirm);
+ init_completion(&link->llc_confirm_resp);
+ init_completion(&link->llc_add);
+ init_completion(&link->llc_add_resp);
+ init_completion(&link->llc_confirm_rkey);
init_completion(&link->llc_testlink_resp);
INIT_DELAYED_WORK(&link->llc_testlink_wrk, smc_llc_testlink_work);
+}
+
+void smc_llc_link_active(struct smc_link *link, int testlink_time)
+{
link->state = SMC_LNK_ACTIVE;
if (testlink_time) {
link->llc_testlink_time = testlink_time * HZ;
diff --git a/net/smc/smc_llc.h b/net/smc/smc_llc.h
index fb137f24aa6b..d7f52a60c8d6 100644
--- a/net/smc/smc_llc.h
+++ b/net/smc/smc_llc.h
@@ -42,6 +42,7 @@ int smc_llc_send_add_link(struct smc_link *link, u8 mac[], union ib_gid *gid,
enum smc_llc_reqresp reqresp);
int smc_llc_send_delete_link(struct smc_link *link,
enum smc_llc_reqresp reqresp);
+void smc_llc_link_init(struct smc_link *link);
void smc_llc_link_active(struct smc_link *link, int testlink_time);
void smc_llc_link_inactive(struct smc_link *link);
void smc_llc_link_flush(struct smc_link *link);
--
2.16.3
^ permalink raw reply related
* [PATCH net-next 08/10] net/smc: set link inactive before calling smc_lgr_free()
From: Ursula Braun @ 2018-05-15 15:05 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20180515150503.47265-1-ubraun@linux.ibm.com>
From: Karsten Graul <kgraul@linux.ibm.com>
Before smc_lgr_free() is called the link must be set inactive by calling
smc_llc_link_inactive().
Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
net/smc/af_smc.c | 1 +
net/smc/smc_core.c | 5 ++++-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index ecf9ba68501b..d15762b057c0 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -1644,6 +1644,7 @@ static void __exit smc_exit(void)
spin_unlock_bh(&smc_lgr_list.lock);
list_for_each_entry_safe(lgr, lg, &lgr_freeing_list, list) {
list_del_init(&lgr->list);
+ smc_llc_link_inactive(&lgr->lnk[SMC_SINGLE_LINK]);
cancel_delayed_work_sync(&lgr->free_work);
smc_lgr_free(lgr); /* free link group */
}
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 4523bbad8a15..a48ee00b65ff 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -148,8 +148,11 @@ static void smc_lgr_free_work(struct work_struct *work)
list_del_init(&lgr->list); /* remove from smc_lgr_list */
free:
spin_unlock_bh(&smc_lgr_list.lock);
- if (!delayed_work_pending(&lgr->free_work))
+ if (!delayed_work_pending(&lgr->free_work)) {
+ if (lgr->lnk[SMC_SINGLE_LINK].state != SMC_LNK_INACTIVE)
+ smc_llc_link_inactive(&lgr->lnk[SMC_SINGLE_LINK]);
smc_lgr_free(lgr);
+ }
}
/* create a new SMC link group */
--
2.16.3
^ permalink raw reply related
* [PATCH net-next 04/10] net/smc: simplify test_link function usage
From: Ursula Braun @ 2018-05-15 15:04 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20180515150503.47265-1-ubraun@linux.ibm.com>
From: Karsten Graul <kgraul@linux.ibm.com>
Make smc_llc_send_test_link() static and remove it from the header file.
And to send a test_link response set the response flag and send the
message back as-is, without using smc_llc_send_test_link(). And because
smc_llc_send_test_link() must no longer send responses, remove the
response flag handling from the function.
Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
net/smc/smc_llc.c | 12 +++++-------
net/smc/smc_llc.h | 2 --
2 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
index 29f59b47b559..229e967c7940 100644
--- a/net/smc/smc_llc.c
+++ b/net/smc/smc_llc.c
@@ -296,9 +296,8 @@ int smc_llc_send_delete_link(struct smc_link *link,
return rc;
}
-/* send LLC test link request or response */
-int smc_llc_send_test_link(struct smc_link *link, u8 user_data[16],
- enum smc_llc_reqresp reqresp)
+/* send LLC test link request */
+static int smc_llc_send_test_link(struct smc_link *link, u8 user_data[16])
{
struct smc_llc_msg_test_link *testllc;
struct smc_wr_tx_pend_priv *pend;
@@ -312,8 +311,6 @@ int smc_llc_send_test_link(struct smc_link *link, u8 user_data[16],
memset(testllc, 0, sizeof(*testllc));
testllc->hd.common.type = SMC_LLC_TEST_LINK;
testllc->hd.length = sizeof(struct smc_llc_msg_test_link);
- if (reqresp == SMC_LLC_RESP)
- testllc->hd.flags |= SMC_LLC_FLAG_RESP;
memcpy(testllc->user_data, user_data, sizeof(testllc->user_data));
/* send llc message */
rc = smc_wr_tx_send(link, pend);
@@ -425,7 +422,8 @@ static void smc_llc_rx_test_link(struct smc_link *link,
if (link->state == SMC_LNK_ACTIVE)
complete(&link->llc_testlink_resp);
} else {
- smc_llc_send_test_link(link, llc->user_data, SMC_LLC_RESP);
+ llc->hd.flags |= SMC_LLC_FLAG_RESP;
+ smc_llc_send_message(link, llc, sizeof(*llc));
}
}
@@ -551,7 +549,7 @@ static void smc_llc_testlink_work(struct work_struct *work)
goto out;
}
reinit_completion(&link->llc_testlink_resp);
- smc_llc_send_test_link(link, user_data, SMC_LLC_REQ);
+ smc_llc_send_test_link(link, user_data);
/* receive TEST LINK response over RoCE fabric */
rc = wait_for_completion_interruptible_timeout(&link->llc_testlink_resp,
SMC_LLC_WAIT_TIME);
diff --git a/net/smc/smc_llc.h b/net/smc/smc_llc.h
index 728823dd8ae5..fb137f24aa6b 100644
--- a/net/smc/smc_llc.h
+++ b/net/smc/smc_llc.h
@@ -42,8 +42,6 @@ int smc_llc_send_add_link(struct smc_link *link, u8 mac[], union ib_gid *gid,
enum smc_llc_reqresp reqresp);
int smc_llc_send_delete_link(struct smc_link *link,
enum smc_llc_reqresp reqresp);
-int smc_llc_send_test_link(struct smc_link *lnk, u8 user_data[16],
- enum smc_llc_reqresp reqresp);
void smc_llc_link_active(struct smc_link *link, int testlink_time);
void smc_llc_link_inactive(struct smc_link *link);
void smc_llc_link_flush(struct smc_link *link);
--
2.16.3
^ permalink raw reply related
* [PATCH net-next 09/10] net/smc: drop messages when link state is inactive
From: Ursula Braun @ 2018-05-15 15:05 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20180515150503.47265-1-ubraun@linux.ibm.com>
From: Karsten Graul <kgraul@linux.ibm.com>
Drop incoming messages when the link is flagged as inactive.
Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
net/smc/smc_llc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
index 5d7926cc472c..5800a6b43d83 100644
--- a/net/smc/smc_llc.c
+++ b/net/smc/smc_llc.c
@@ -548,6 +548,8 @@ static void smc_llc_rx_handler(struct ib_wc *wc, void *buf)
return; /* short message */
if (llc->raw.hdr.length != sizeof(*llc))
return; /* invalid message */
+ if (link->state == SMC_LNK_INACTIVE)
+ return; /* link not active, drop msg */
switch (llc->raw.hdr.common.type) {
case SMC_LLC_TEST_LINK:
--
2.16.3
^ permalink raw reply related
* [PATCH net-next 07/10] net/smc: handle all error codes from smc_conn_create()
From: Ursula Braun @ 2018-05-15 15:05 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20180515150503.47265-1-ubraun@linux.ibm.com>
From: Karsten Graul <kgraul@linux.ibm.com>
Always set a reason_code when smc_conn_create() returns an error code.
Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
net/smc/af_smc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 397ba2182453..ecf9ba68501b 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -463,6 +463,8 @@ static int smc_connect_rdma(struct smc_sock *smc)
reason_code = SMC_CLC_DECL_MEM;/* insufficient memory*/
else if (rc == -ENOLINK)
reason_code = SMC_CLC_DECL_SYNCERR; /* synchr. error */
+ else
+ reason_code = SMC_CLC_DECL_INTERR; /* other error */
goto decline_rdma_unlock;
}
link = &smc->conn.lgr->lnk[SMC_SINGLE_LINK];
--
2.16.3
^ permalink raw reply related
* [PATCH net-next 10/10] net/smc: check for pending termination
From: Ursula Braun @ 2018-05-15 15:05 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20180515150503.47265-1-ubraun@linux.ibm.com>
From: Karsten Graul <kgraul@linux.ibm.com>
Avoid to run the processing in smc_lgr_terminate() more than once,
remember when the link group termination is triggered.
Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
net/smc/smc_clc.c | 2 +-
net/smc/smc_core.c | 5 ++++-
net/smc/smc_core.h | 3 ++-
3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
index 3a988c22f627..236cb3f12c71 100644
--- a/net/smc/smc_clc.c
+++ b/net/smc/smc_clc.c
@@ -316,7 +316,7 @@ int smc_clc_wait_msg(struct smc_sock *smc, void *buf, int buflen,
if (clcm->type == SMC_CLC_DECLINE) {
reason_code = SMC_CLC_DECL_REPLY;
if (((struct smc_clc_msg_decline *)buf)->hdr.flag) {
- smc->conn.lgr->sync_err = true;
+ smc->conn.lgr->sync_err = 1;
smc_lgr_terminate(smc->conn.lgr);
}
}
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index a48ee00b65ff..08c05cd0bbae 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -172,7 +172,7 @@ static int smc_lgr_create(struct smc_sock *smc,
goto out;
}
lgr->role = smc->listen_smc ? SMC_SERV : SMC_CLNT;
- lgr->sync_err = false;
+ lgr->sync_err = 0;
memcpy(lgr->peer_systemid, peer_systemid, SMC_SYSTEMID_LEN);
lgr->vlan_id = vlan_id;
rwlock_init(&lgr->sndbufs_lock);
@@ -352,6 +352,9 @@ void smc_lgr_terminate(struct smc_link_group *lgr)
struct smc_sock *smc;
struct rb_node *node;
+ if (lgr->terminating)
+ return; /* lgr already terminating */
+ lgr->terminating = 1;
smc_lgr_forget(lgr);
smc_llc_link_inactive(&lgr->lnk[SMC_SINGLE_LINK]);
diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h
index d1753850684b..845dc073de13 100644
--- a/net/smc/smc_core.h
+++ b/net/smc/smc_core.h
@@ -166,7 +166,8 @@ struct smc_link_group {
u8 id[SMC_LGR_ID_SIZE]; /* unique lgr id */
struct delayed_work free_work; /* delayed freeing of an lgr */
- bool sync_err; /* lgr no longer fits to peer */
+ u8 sync_err : 1; /* lgr no longer fits to peer */
+ u8 terminating : 1;/* lgr is terminating */
};
/* Find the connection associated with the given alert token in the link group.
--
2.16.3
^ permalink raw reply related
* [PATCH net-next 03/10] net/smc: remove unnecessary cast
From: Ursula Braun @ 2018-05-15 15:04 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20180515150503.47265-1-ubraun@linux.ibm.com>
From: Karsten Graul <kgraul@linux.ibm.com>
Remove an unneeded (void *) cast from the calls to
smc_llc_send_message(). No functional changes.
Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
net/smc/smc_llc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
index b118f80b9c41..29f59b47b559 100644
--- a/net/smc/smc_llc.c
+++ b/net/smc/smc_llc.c
@@ -451,7 +451,7 @@ static void smc_llc_rx_confirm_rkey(struct smc_link *link,
llc->hd.flags |= SMC_LLC_FLAG_RESP;
if (rc < 0)
llc->hd.flags |= SMC_LLC_FLAG_RKEY_NEG;
- smc_llc_send_message(link, (void *)llc, sizeof(*llc));
+ smc_llc_send_message(link, llc, sizeof(*llc));
}
}
@@ -463,7 +463,7 @@ static void smc_llc_rx_confirm_rkey_cont(struct smc_link *link,
} else {
/* ignore rtokens for other links, we have only one link */
llc->hd.flags |= SMC_LLC_FLAG_RESP;
- smc_llc_send_message(link, (void *)llc, sizeof(*llc));
+ smc_llc_send_message(link, llc, sizeof(*llc));
}
}
@@ -491,7 +491,7 @@ static void smc_llc_rx_delete_rkey(struct smc_link *link,
}
llc->hd.flags |= SMC_LLC_FLAG_RESP;
- smc_llc_send_message(link, (void *)llc, sizeof(*llc));
+ smc_llc_send_message(link, llc, sizeof(*llc));
}
}
--
2.16.3
^ permalink raw reply related
* [PATCH net-next v11 0/7] sched: Add Common Applications Kept Enhanced (cake) qdisc
From: Toke Høiland-Jørgensen @ 2018-05-15 15:12 UTC (permalink / raw)
To: netdev; +Cc: cake
This patch series adds the CAKE qdisc, and has been split up to ease
review.
I have attempted to split out each configurable feature into its own patch.
The first commit adds the base shaper and packet scheduler, while
subsequent commits add the optional features. The full userspace API and
most data structures are included in this commit, but options not
understood in the base version will be ignored.
The result of applying the entire series is identical to the out of tree
version that have seen extensive testing in previous deployments, most
notably as an out of tree patch to OpenWrt. However, note that I have only
compile tested the individual patches; so the whole series should be
considered as a unit.
---
Changelog
v11:
- Fix overhead compensation calculation for GSO packets
- Change configured rate to be u64 (I ran out of bits before I ran out
of CPU when testing the effects of the above)
v10:
- Christmas tree gardening (fix variable declarations to be in reverse
line length order)
v9:
- Remove duplicated checks around kvfree() and just call it
unconditionally.
- Don't pass __GFP_NOWARN when allocating memory
- Move options in cake_dump() that are related to optional features to
later patches implementing the features.
- Support attaching filters to the qdisc and use the classification
result to select flow queue.
- Support overriding diffserv priority tin from skb->priority
v8:
- Remove inline keyword from function definitions
- Simplify ACK filter; remove the complex state handling to make the
logic easier to follow. This will potentially be a bit less efficient,
but I have not been able to measure a difference.
v7:
- Split up patch into a series to ease review.
- Constify the ACK filter.
v6:
- Fix 6in4 encapsulation checks in ACK filter code
- Checkpatch fixes
v5:
- Refactor ACK filter code and hopefully fix the safety issues
properly this time.
v4:
- Only split GSO packets if shaping at speeds <= 1Gbps
- Fix overhead calculation code to also work for GSO packets
- Don't re-implement kvzalloc()
- Remove local header include from out-of-tree build (fixes kbuild-bot
complaint).
- Several fixes to the ACK filter:
- Check pskb_may_pull() before deref of transport headers.
- Don't run ACK filter logic on split GSO packets
- Fix TCP sequence number compare to deal with wraparounds
v3:
- Use IS_REACHABLE() macro to fix compilation when sch_cake is
built-in and conntrack is a module.
- Switch the stats output to use nested netlink attributes instead
of a versioned struct.
- Remove GPL boilerplate.
- Fix array initialisation style.
v2:
- Fix kbuild test bot complaint
- Clean up the netlink ABI
- Fix checkpatch complaints
- A few tweaks to the behaviour of cake based on testing carried out
while writing the paper.
---
Toke Høiland-Jørgensen (7):
sched: Add Common Applications Kept Enhanced (cake) qdisc
sch_cake: Add ingress mode
sch_cake: Add optional ACK filter
sch_cake: Add NAT awareness to packet classifier
sch_cake: Add DiffServ handling
sch_cake: Add overhead compensation support to the rate shaper
sch_cake: Conditionally split GSO segments
include/uapi/linux/pkt_sched.h | 105 ++
net/sched/Kconfig | 11
net/sched/Makefile | 1
net/sched/sch_cake.c | 2692 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 2809 insertions(+)
create mode 100644 net/sched/sch_cake.c
^ permalink raw reply
* [PATCH net-next v11 3/7] sch_cake: Add optional ACK filter
From: Toke Høiland-Jørgensen @ 2018-05-15 15:12 UTC (permalink / raw)
To: netdev; +Cc: cake
In-Reply-To: <152639705919.2525.3718182212727523960.stgit@alrua-kau>
The ACK filter is an optional feature of CAKE which is designed to improve
performance on links with very asymmetrical rate limits. On such links
(which are unfortunately quite prevalent, especially for DSL and cable
subscribers), the downstream throughput can be limited by the number of
ACKs capable of being transmitted in the *upstream* direction.
Filtering ACKs can, in general, have adverse effects on TCP performance
because it interferes with ACK clocking (especially in slow start), and it
reduces the flow's resiliency to ACKs being dropped further along the path.
To alleviate these drawbacks, the ACK filter in CAKE tries its best to
always keep enough ACKs queued to ensure forward progress in the TCP flow
being filtered. It does this by only filtering redundant ACKs. In its
default 'conservative' mode, the filter will always keep at least two
redundant ACKs in the queue, while in 'aggressive' mode, it will filter
down to a single ACK.
The ACK filter works by inspecting the per-flow queue on every packet
enqueue. Starting at the head of the queue, the filter looks for another
eligible packet to drop (so the ACK being dropped is always closer to the
head of the queue than the packet being enqueued). An ACK is eligible only
if it ACKs *fewer* cumulative bytes than the new packet being enqueued.
This prevents duplicate ACKs from being filtered (unless there is also SACK
options present), to avoid interfering with retransmission logic. In
aggressive mode, an eligible packet is always dropped, while in
conservative mode, at least two ACKs are kept in the queue. Only pure ACKs
(with no data segments) are considered eligible for dropping, but when an
ACK with data segments is enqueued, this can cause another pure ACK to
become eligible for dropping.
The approach described above ensures that this ACK filter avoids most of
the drawbacks of a naive filtering mechanism that only keeps flow state but
does not inspect the queue. This is the rationale for including the ACK
filter in CAKE itself rather than as separate module (as the TC filter, for
instance).
Our performance evaluation has shown that on a 30/1 Mbps link with a
bidirectional traffic test (RRUL), turning on the ACK filter on the
upstream link improves downstream throughput by ~20% (both modes) and
upstream throughput by ~12% in conservative mode and ~40% in aggressive
mode, at the cost of ~5ms of inter-flow latency due to the increased
congestion.
In *really* pathological cases, the effect can be a lot more; for instance,
the ACK filter increases the achievable downstream throughput on a link
with 100 Kbps in the upstream direction by an order of magnitude (from ~2.5
Mbps to ~25 Mbps).
Finally, even though we consider the ACK filter to be safer than most, we
do not recommend turning it on everywhere: on more symmetrical link
bandwidths the effect is negligible at best.
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
net/sched/sch_cake.c | 260 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 258 insertions(+), 2 deletions(-)
diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index 8d0823d6d8dd..371c888cb982 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c
@@ -763,6 +763,239 @@ static void flow_queue_add(struct cake_flow *flow, struct sk_buff *skb)
skb->next = NULL;
}
+static struct iphdr *cake_get_iphdr(const struct sk_buff *skb,
+ struct ipv6hdr *buf)
+{
+ unsigned int offset = skb_network_offset(skb);
+ struct iphdr *iph;
+
+ iph = skb_header_pointer(skb, offset, sizeof(struct iphdr), buf);
+
+ if (!iph)
+ return NULL;
+
+ if (iph->version == 4 && iph->protocol == IPPROTO_IPV6)
+ return skb_header_pointer(skb, offset + iph->ihl * 4,
+ sizeof(struct ipv6hdr), buf);
+
+ else if (iph->version == 4)
+ return iph;
+
+ else if (iph->version == 6)
+ return skb_header_pointer(skb, offset, sizeof(struct ipv6hdr),
+ buf);
+
+ return NULL;
+}
+
+static struct tcphdr *cake_get_tcphdr(const struct sk_buff *skb,
+ void *buf, unsigned int bufsize)
+{
+ unsigned int offset = skb_network_offset(skb);
+ const struct ipv6hdr *ipv6h;
+ const struct tcphdr *tcph;
+ const struct iphdr *iph;
+ struct ipv6hdr _ipv6h;
+ struct tcphdr _tcph;
+
+ ipv6h = skb_header_pointer(skb, offset, sizeof(_ipv6h), &_ipv6h);
+
+ if (!ipv6h)
+ return NULL;
+
+ if (ipv6h->version == 4) {
+ iph = (struct iphdr *)ipv6h;
+ offset += iph->ihl * 4;
+
+ /* special-case 6in4 tunnelling, as that is a common way to get
+ * v6 connectivity in the home
+ */
+ if (iph->protocol == IPPROTO_IPV6) {
+ ipv6h = skb_header_pointer(skb, offset,
+ sizeof(_ipv6h), &_ipv6h);
+
+ if (!ipv6h || ipv6h->nexthdr != IPPROTO_TCP)
+ return NULL;
+
+ offset += sizeof(struct ipv6hdr);
+
+ } else if (iph->protocol != IPPROTO_TCP) {
+ return NULL;
+ }
+
+ } else if (ipv6h->version == 6) {
+ if (ipv6h->nexthdr != IPPROTO_TCP)
+ return NULL;
+
+ offset += sizeof(struct ipv6hdr);
+ } else {
+ return NULL;
+ }
+
+ tcph = skb_header_pointer(skb, offset, sizeof(_tcph), &_tcph);
+ if (!tcph)
+ return NULL;
+
+ return skb_header_pointer(skb, offset,
+ min(__tcp_hdrlen(tcph), bufsize), buf);
+}
+
+static bool cake_tcph_is_sack(const struct tcphdr *tcph)
+{
+ /* inspired by tcp_parse_options in tcp_input.c */
+ int length = __tcp_hdrlen(tcph) - sizeof(struct tcphdr);
+ const u8 *ptr = (const u8 *)(tcph + 1);
+
+ while (length > 0) {
+ int opcode = *ptr++;
+ int opsize;
+
+ if (opcode == TCPOPT_EOL)
+ break;
+ if (opcode == TCPOPT_NOP) {
+ length--;
+ continue;
+ }
+ opsize = *ptr++;
+ if (opsize < 2 || opsize > length)
+ break;
+ if (opcode == TCPOPT_SACK)
+ return true;
+ ptr += opsize - 2;
+ length -= opsize;
+ }
+
+ return false;
+}
+
+static struct sk_buff *cake_ack_filter(struct cake_sched_data *q,
+ struct cake_flow *flow)
+{
+ bool aggressive = q->ack_filter == CAKE_ACK_AGGRESSIVE;
+ struct sk_buff *elig_ack = NULL, *elig_ack_prev = NULL;
+ struct sk_buff *skb_check, *skb_prev = NULL;
+ const struct ipv6hdr *ipv6h, *ipv6h_check;
+ const struct tcphdr *tcph, *tcph_check;
+ const struct iphdr *iph, *iph_check;
+ struct ipv6hdr _iph, _iph_check;
+ const struct sk_buff *skb;
+ struct tcphdr _tcph_check;
+ int seglen, num_found = 0;
+ unsigned char _tcph[64]; /* need to hold maximum hdr size */
+
+ /* no other possible ACKs to filter */
+ if (flow->head == flow->tail)
+ return NULL;
+
+ skb = flow->tail;
+ tcph = cake_get_tcphdr(skb, _tcph, sizeof(_tcph));
+ iph = cake_get_iphdr(skb, &_iph);
+ if (!tcph)
+ return NULL;
+
+ /* the 'triggering' packet need only have the ACK flag set.
+ * also check that SYN is not set, as there won't be any previous ACKs.
+ */
+ if ((tcp_flag_word(tcph) &
+ (TCP_FLAG_ACK | TCP_FLAG_SYN)) != TCP_FLAG_ACK)
+ return NULL;
+
+ /* the 'triggering' ACK is at the tail of the queue, we have already
+ * returned if it is the only packet in the flow. loop through the rest
+ * of the queue looking for pure ACKs with the same 5-tuple as the
+ * triggering one.
+ */
+ for (skb_check = flow->head;
+ skb_check && skb_check != skb;
+ skb_prev = skb_check, skb_check = skb_check->next) {
+ iph_check = cake_get_iphdr(skb_check, &_iph_check);
+ tcph_check = cake_get_tcphdr(skb_check, &_tcph_check,
+ sizeof(_tcph_check));
+
+ /* only TCP packets with matching 5-tuple are eligible */
+ if (!tcph_check || iph->version != iph_check->version ||
+ tcph_check->source != tcph->source ||
+ tcph_check->dest != tcph->dest)
+ continue;
+
+ if (iph_check->version == 4) {
+ if (iph_check->saddr != iph->saddr ||
+ iph_check->daddr != iph->daddr)
+ continue;
+
+ seglen = ntohs(iph_check->tot_len) -
+ (4 * iph_check->ihl);
+ } else if (iph_check->version == 6) {
+ ipv6h = (struct ipv6hdr *)iph;
+ ipv6h_check = (struct ipv6hdr *)iph_check;
+
+ if (ipv6_addr_cmp(&ipv6h_check->saddr, &ipv6h->saddr) ||
+ ipv6_addr_cmp(&ipv6h_check->daddr, &ipv6h->daddr))
+ continue;
+
+ seglen = ntohs(ipv6h_check->payload_len);
+ } else {
+ WARN_ON(1); /* shouldn't happen */
+ continue;
+ }
+
+ /* stricter criteria apply to ACKs that we may filter
+ * 3 reserved flags must be unset to avoid future breakage
+ * ECE/CWR/NS can be safely ignored
+ * ACK must be set
+ * All other flags URG/PSH/RST/SYN/FIN must be unset
+ * 0x0FFF0000 = all TCP flags (confirm ACK=1, others zero)
+ * 0x01C00000 = NS/CWR/ECE (safe to ignore)
+ * 0x0E3F0000 = 0x0FFF0000 & ~0x01C00000
+ * must be 'pure' ACK, contain zero bytes of segment data
+ * options are ignored
+ */
+ if (((tcp_flag_word(tcph_check) &
+ cpu_to_be32(0x0E3F0000)) != TCP_FLAG_ACK) ||
+ ((seglen - __tcp_hdrlen(tcph_check)) != 0))
+ continue;
+
+ /* The triggering packet must ACK more data than the ACK under
+ * consideration, either because is has a strictly higher ACK
+ * sequence number or because it is a SACK
+ */
+ if ((ntohl(tcph_check->ack_seq) == ntohl(tcph->ack_seq) &&
+ !cake_tcph_is_sack(tcph)) ||
+ (int32_t)(ntohl(tcph_check->ack_seq) -
+ ntohl(tcph->ack_seq)) > 0)
+ continue;
+
+ /* At this point we have found an eligible pure ACK to drop; if
+ * we are in aggressive mode, we are done. Otherwise, keep
+ * searching unless this is the second eligible ACK we
+ * found.
+ *
+ * Since we want to drop ACK closest to the head of the queue,
+ * save the first eligible ACK we find, even if we need to loop
+ * again.
+ */
+ if (!elig_ack) {
+ elig_ack = skb_check;
+ elig_ack_prev = skb_prev;
+ }
+
+ if (num_found++ > 0 || aggressive)
+ goto found;
+ }
+
+ return NULL;
+
+found:
+ if (elig_ack_prev)
+ elig_ack_prev->next = elig_ack->next;
+ else
+ flow->head = elig_ack->next;
+
+ elig_ack->next = NULL;
+
+ return elig_ack;
+}
+
static cobalt_time_t cake_ewma(cobalt_time_t avg, cobalt_time_t sample,
u32 shift)
{
@@ -936,6 +1169,7 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch,
int len = qdisc_pkt_len(skb);
u64 now = cobalt_get_time();
int uninitialized_var(ret);
+ struct sk_buff *ack = NULL;
struct cake_tin_data *b;
struct cake_flow *flow;
u32 idx, tin;
@@ -979,8 +1213,24 @@ static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch,
cobalt_set_enqueue_time(skb, now);
flow_queue_add(flow, skb);
- sch->q.qlen++;
- q->buffer_used += skb->truesize;
+ if (q->ack_filter)
+ ack = cake_ack_filter(q, flow);
+
+ if (ack) {
+ b->ack_drops++;
+ sch->qstats.drops++;
+ b->bytes += qdisc_pkt_len(ack);
+ len -= qdisc_pkt_len(ack);
+ q->buffer_used += skb->truesize - ack->truesize;
+ if (q->rate_flags & CAKE_FLAG_INGRESS)
+ cake_advance_shaper(q, b, ack, now, true);
+
+ qdisc_tree_reduce_backlog(sch, 1, qdisc_pkt_len(ack));
+ consume_skb(ack);
+ } else {
+ sch->q.qlen++;
+ q->buffer_used += skb->truesize;
+ }
/* stats */
b->packets++;
@@ -1493,6 +1743,9 @@ static int cake_change(struct Qdisc *sch, struct nlattr *opt,
q->rate_flags &= ~CAKE_FLAG_INGRESS;
}
+ if (tb[TCA_CAKE_ACK_FILTER])
+ q->ack_filter = nla_get_u32(tb[TCA_CAKE_ACK_FILTER]);
+
if (tb[TCA_CAKE_MEMORY])
q->buffer_config_limit = nla_get_u32(tb[TCA_CAKE_MEMORY]);
@@ -1624,6 +1877,9 @@ static int cake_dump(struct Qdisc *sch, struct sk_buff *skb)
!!(q->rate_flags & CAKE_FLAG_INGRESS)))
goto nla_put_failure;
+ if (nla_put_u32(skb, TCA_CAKE_ACK_FILTER, q->ack_filter))
+ goto nla_put_failure;
+
return nla_nest_end(skb, opts);
nla_put_failure:
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox