Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH bpf] tools: bpf: fix NULL return handling in bpf__prepare_load
From: Arnaldo Carvalho de Melo @ 2018-05-15 14:11 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: YueHaibing, alexander.shishkin, mingo, peterz, netdev, namhyung
In-Reply-To: <605c7b96-83dc-9a9e-7037-32b2c2dfcb2e@iogearbox.net>

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,

- Arnaldo
 
> > diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> > index af7ad81..cee6587 100644
> > --- a/tools/perf/util/bpf-loader.c
> > +++ b/tools/perf/util/bpf-loader.c
> > @@ -66,7 +66,7 @@ bpf__prepare_load_buffer(void *obj_buf, size_t obj_buf_sz, const char *name)
> >  	}
> >  
> >  	obj = bpf_object__open_buffer(obj_buf, obj_buf_sz, name);
> > -	if (IS_ERR(obj)) {
> > +	if (IS_ERR_OR_NULL(obj)) {
> >  		pr_debug("bpf: failed to load buffer\n");
> >  		return ERR_PTR(-EINVAL);
> >  	}
> > @@ -102,14 +102,14 @@ struct bpf_object *bpf__prepare_load(const char *filename, bool source)
> >  			pr_debug("bpf: successfull builtin compilation\n");
> >  		obj = bpf_object__open_buffer(obj_buf, obj_buf_sz, filename);
> >  
> > -		if (!IS_ERR(obj) && llvm_param.dump_obj)
> > +		if (!IS_ERR_OR_NULL(obj) && llvm_param.dump_obj)
> >  			llvm__dump_obj(filename, obj_buf, obj_buf_sz);
> >  
> >  		free(obj_buf);
> >  	} else
> >  		obj = bpf_object__open(filename);
> >  
> > -	if (IS_ERR(obj)) {
> > +	if (IS_ERR_OR_NULL(obj)) {
> >  		pr_debug("bpf: failed to load %s\n", filename);
> >  		return obj;
> >  	}
> > 

^ permalink raw reply

* Re: [PATCH 34/40] atm: simplify procfs code
From: Christoph Hellwig @ 2018-05-15 14:12 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: <87r2mq2rll.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>

On Sat, May 05, 2018 at 07:51:18AM -0500, Eric W. Biederman wrote:
> Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org> writes:
> 
> > Use remove_proc_subtree to remove the whole subtree on cleanup, and
> > unwind the registration loop into individual calls.  Switch to use
> > proc_create_seq where applicable.
> 
> Can you please explain why you are removing the error handling when
> you are unwinding the registration loop?

Because there is no point in handling these errors.  The code work
perfectly fine without procfs, or without given proc files and the
removal works just fine if they don't exist either.  This is a very
common patter in various parts of the kernel already.

I'll document it better in the changelog.

^ permalink raw reply

* [PATCH net-next 1/1] tc-testing: fixed copy-pasting error in police tests
From: Roman Mashak @ 2018-05-15 14:14 UTC (permalink / raw)
  To: davem; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri, Roman Mashak

Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
 tools/testing/selftests/tc-testing/tc-tests/actions/police.json | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/police.json b/tools/testing/selftests/tc-testing/tc-tests/actions/police.json
index 38d85a1d7492..f03763d81617 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/police.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/police.json
@@ -401,11 +401,11 @@
         ],
         "cmdUnderTest": "$TC actions add action police rate 10mbit burst 10k index 4294967295",
         "expExitCode": "0",
-        "verifyCmd": "$TC actions get action mirred index 4294967295",
+        "verifyCmd": "$TC actions get action police index 4294967295",
         "matchPattern": "action order [0-9]*:  police 0xffffffff rate 10Mbit burst 10Kb mtu 2Kb",
         "matchCount": "1",
         "teardown": [
-            "$TC actions flush action mirred"
+            "$TC actions flush action police"
         ]
     },
     {
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH net-next 3/3] udp: only use paged allocation with scatter-gather
From: Willem de Bruijn @ 2018-05-15 14:14 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Network Development, David Miller, Willem de Bruijn
In-Reply-To: <7557fc96-eb5a-56cb-28b5-a49abe8dae7c@gmail.com>

On Mon, May 14, 2018 at 7:45 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
> On 05/14/2018 04:30 PM, Willem de Bruijn wrote:
>
>> I don't quite follow. The reported crash happens in the protocol layer,
>> because of this check. With pagedlen we have not allocated
>> sufficient space for the skb_put.
>>
>>                 if (!(rt->dst.dev->features&NETIF_F_SG)) {
>>                         unsigned int off;
>>
>>                         off = skb->len;
>>                         if (getfrag(from, skb_put(skb, copy),
>>                                         offset, copy, off, skb) < 0) {
>>                                 __skb_trim(skb, off);
>>                                 err = -EFAULT;
>>                                 goto error;
>>                         }
>>                 } else {
>>                         int i = skb_shinfo(skb)->nr_frags;
>>
>> Are you referring to a separate potential issue in the gso layer?
>> If a bonding device advertises SG, but a slave does not, then
>> skb_segment on the slave should build linear segs? I have not
>> tested that.
>
> Given that the device attribute could change under us, we need to not
> crash, even if initially we thought NETIF_F_SG was available.
>
> Unless you want to hold RTNL in UDP xmit :)
>
> Ideally, GSO should be always on, as we did for TCP.
>
> Otherwise, I can guarantee syzkaller will hit again.

Ah, right. Thanks, Eric!

I'll read that feature bit only once.

^ permalink raw reply

* 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


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