* RE: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information
From: Steve Wise @ 2018-03-27 15:45 UTC (permalink / raw)
To: 'Jason Gunthorpe', 'Leon Romanovsky'
Cc: 'David Ahern', stephen, netdev, linux-rdma
In-Reply-To: <20180327152356.GH12318@ziepe.ca>
>
> On Tue, Mar 27, 2018 at 06:15:44PM +0300, Leon Romanovsky wrote:
> > On Tue, Mar 27, 2018 at 08:44:55AM -0600, Jason Gunthorpe wrote:
> > > On Tue, Mar 27, 2018 at 06:21:41AM +0300, Leon Romanovsky wrote:
> > > > On Mon, Mar 26, 2018 at 04:30:33PM -0600, Jason Gunthorpe wrote:
> > > > > On Mon, Mar 26, 2018 at 04:34:44PM -0500, Steve Wise wrote:
> > > > > >
> > > > > > On 3/26/2018 4:15 PM, Jason Gunthorpe wrote:
> > > > > > > On Mon, Mar 26, 2018 at 09:30:41AM -0500, Steve Wise wrote:
> > > > > > >>
> > > > > > >> On 3/26/2018 9:17 AM, David Ahern wrote:
> > > > > > >>> On 2/27/18 9:07 AM, Steve Wise wrote:
> > > > > > >>>> diff --git a/rdma/rdma.h b/rdma/rdma.h
> > > > > > >>>> index 5809f70..e55205b 100644
> > > > > > >>>> +++ b/rdma/rdma.h
> > > > > > >>>> @@ -18,10 +18,12 @@
> > > > > > >>>> #include <libmnl/libmnl.h>
> > > > > > >>>> #include <rdma/rdma_netlink.h>
> > > > > > >>>> #include <time.h>
> > > > > > >>>> +#include <net/if_arp.h>
> > > > > > >>>>
> > > > > > >>>> #include "list.h"
> > > > > > >>>> #include "utils.h"
> > > > > > >>>> #include "json_writer.h"
> > > > > > >>>> +#include <rdma/rdma_cma.h>
> > > > > > >>>>
> > > > > > >>> did you forget to add rdma_cma.h? I don't see that file in my
> repo.
> > > > > > >> It is provided by the rdma-core package, upon which rdma tool
> now
> > > > > > >> depends for the rdma_port_space enum.
> > > > > > > It is a kernel bug that enum is not in an include/uapi/rdma
> header
> > > > > > >
> > > > > > > Fix it there and don't try to use rdma-core headers to get kernel
> ABI.
> > > > > > >
> > > > > > > Jason
> > > > > >
> > > > > > I wish you'd commented on this just a little sooner. I just resent
> v3
> > > > > > of this series... with rdma_cma.h included. :)
> > > > > >
> > > > > > How about the restrack/nldev code just translates the port space
> from
> > > > > > enum rdma_port_space to a new ABI enum, say
> nldev_rdma_port_space, that
> > > > > > i add to rdma_netlink.h? I'd hate to open the can of worms of
> trying to
> > > > > > split rdma_cma.h into uabi and no uabi headers. :(
> > > > >
> > > > > If port space is already part of the ABI there isn't much reason to
> > > > > translate it.
> > > > >
> > > > > You just need to pick the right header to put it in, since it is a verbs
> > > > > define it doesn't belong in the netlink header.
> > > >
> > > > I completely understand Steve's concerns.
> > > >
> > > > I tried to do such thing (expose kernel headers) in first incarnation of
> > > > rdmatool with attempt to clean IB/core as well to ensure that we
> won't expose
> > > > anything that is not implemented. It didn't go well.
> > >
> > > rdma-core is now using the kernel uapi/ headers natively, seems to be
> > > going OK. What problem did you face?
> >
> > I didn't agree to move to UAPI defines which are not implemented and
> not
> > used in the kernel, so I sent small number of patches similar to those [1,
> 2].
> >
> > Those patches were rejected.
> >
> > So please don't mix Steve's need to use 3 defines with very large and
> > painful task to expose proper UAPIs.
>
> Steve can just move the 3 defines he needs to the uapi, we are doing
> this incrementally..
>
> rdma_core does not define kernel ABI and it is totally wrong to use
> random constants from rdma_cma.h as kernel ABI.
>
Proposal:
Since the cm_id port space is part of the rdma_ucm_create_id struct in include/uapi/rdma/rdma_user_cm.h, I'll move the rdma_port_space enum there. And then my iproute2 series will have to add a copy of rdma_user_cm.h locally into rdma/include/uapi/rdma, right?
Will that work for everyone?
Steve.
^ permalink raw reply
* Re: [PATCH 1/6] rhashtable: improve documentation for rhashtable_walk_peek()
From: David Miller @ 2018-03-27 15:47 UTC (permalink / raw)
To: neilb; +Cc: tgraf, herbert, netdev, linux-kernel
In-Reply-To: <152210718418.11435.11573013181393548255.stgit@noble>
From: NeilBrown <neilb@suse.com>
Date: Tue, 27 Mar 2018 10:33:04 +1100
> The documentation for rhashtable_walk_peek() wrong. It claims to
> return the *next* entry, whereas it in fact returns the *previous*
> entry.
> However if no entries have yet been returned - or if the iterator
> was reset due to a resize event, then rhashtable_walk_peek()
> *does* return the next entry, but also advances the iterator.
>
> I suspect that this interface should be discarded and the one user
> should be changed to not require it. Possibly this patch should be
> seen as a first step in that conversation.
>
> This patch mostly corrects the documentation, but does make a
> small code change so that the documentation can be correct without
> listing too many special cases. I don't think the one user will
> be affected by the code change.
>
> Signed-off-by: NeilBrown <neilb@suse.com>
Please mention the "one user" explicitly in both locations where
you refer to it in this commit message.
Thank you.
^ permalink raw reply
* Re: [PATCH 3/6] rhashtable: reset intr when rhashtable_walk_start sees new table
From: Herbert Xu @ 2018-03-27 15:47 UTC (permalink / raw)
To: NeilBrown; +Cc: Thomas Graf, netdev, linux-kernel
In-Reply-To: <152210718427.11435.17689950338335773862.stgit@noble>
On Tue, Mar 27, 2018 at 10:33:04AM +1100, NeilBrown wrote:
> The documentation claims that when rhashtable_walk_start_check()
> detects a resize event, it will rewind back to the beginning
> of the table. This is not true. We need to set ->slot and
> ->skip to be zero for it to be true.
>
> Signed-off-by: NeilBrown <neilb@suse.com>
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [bpf-next PATCH] net: br_vlan build error
From: Nikolay Aleksandrov @ 2018-03-27 15:49 UTC (permalink / raw)
To: John Fastabend, stephen, 3chas3; +Cc: netdev, davem
In-Reply-To: <20180327153859.28865.74511.stgit@john-Precision-Tower-5810>
On 27/03/18 18:38, John Fastabend wrote:
> Fix build error in br_if.c
>
> net/bridge/br_if.c: In function ‘br_mtu’:
> net/bridge/br_if.c:458:8: error: ‘const struct net_bridge’ has no member named ‘vlan_enabled’
> if (br->vlan_enabled)
> ^
> net/bridge/br_if.c:462:1: warning: control reaches end of non-void function [-Wreturn-type]
> }
> ^
> Fixes: 419d14af9e07f ("bridge: Allow max MTU when multiple VLANs present")
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
> net/bridge/br_if.c | 27 +++++++++++++++++----------
> 1 file changed, 17 insertions(+), 10 deletions(-)
>
I'm not sure what the rules about merging are, but just in case I
already fixed this in net-next couple of days ago.
commit 82792a070b16
Author: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date: Fri Mar 23 18:27:06 2018 +0200
net: bridge: fix direct access to bridge vlan_enabled and use helper
https://patchwork.ozlabs.org/patch/890043/
Cheers,
Nik
^ permalink raw reply
* Re: [PATCH 4/6] rhashtable: allow a walk of the hash table without missing objects.
From: David Miller @ 2018-03-27 15:49 UTC (permalink / raw)
To: neilb; +Cc: tgraf, herbert, netdev, linux-kernel
In-Reply-To: <152210718430.11435.5761213978298714527.stgit@noble>
From: NeilBrown <neilb@suse.com>
Date: Tue, 27 Mar 2018 10:33:04 +1100
> In many cases where the walker needs to drop out of RCU protection,
> it will take a reference to the object and this can prevent it from
> being removed from the hash table. In those cases, the last-returned
> object can still be used as a cursor. rhashtable cannot detect
> these cases itself.
Merely having an elevated reference count does not explicitly prevent
the object from being removed from the hash table.
This invariant might hold for the particular user of the rhashtable
instance, but it is not always the case.
^ permalink raw reply
* Re: [PATCH 4/6] rhashtable: allow a walk of the hash table without missing objects.
From: Herbert Xu @ 2018-03-27 15:51 UTC (permalink / raw)
To: NeilBrown; +Cc: Thomas Graf, netdev, linux-kernel
In-Reply-To: <152210718430.11435.5761213978298714527.stgit@noble>
On Tue, Mar 27, 2018 at 10:33:04AM +1100, NeilBrown wrote:
>
> -int rhashtable_walk_start_check(struct rhashtable_iter *iter)
> +int rhashtable_walk_start_continue(struct rhashtable_iter *iter, struct rhash_head *obj)
> __acquires(RCU)
> {
> struct rhashtable *ht = iter->ht;
>
> rcu_read_lock();
>
> + if (!obj || iter->p != obj)
> + iter->p = NULL;
Why bother with this check at all? Couldn't we make it so that
if you call continue then you continue with the cursor otherwise
you set it to NULL as we currently do.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH 4/6] rhashtable: allow a walk of the hash table without missing objects.
From: Herbert Xu @ 2018-03-27 15:54 UTC (permalink / raw)
To: David Miller; +Cc: neilb, tgraf, netdev, linux-kernel
In-Reply-To: <20180327.114941.997071660018188736.davem@davemloft.net>
On Tue, Mar 27, 2018 at 11:49:41AM -0400, David Miller wrote:
>
> Merely having an elevated reference count does not explicitly prevent
> the object from being removed from the hash table.
>
> This invariant might hold for the particular user of the rhashtable
> instance, but it is not always the case.
I think having a new interface like this should work as if the
user knows that the element has not been removed from the hash
table then it should be safe to continue from it.
Of course people may misuse it but even using the current interface
correctly may result in lost objects due to concurrent removals.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v6 bpf-next 07/11] tracepoint: introduce kernel_tracepoint_find_by_name
From: Alexei Starovoitov @ 2018-03-27 15:53 UTC (permalink / raw)
To: Steven Rostedt, Mathieu Desnoyers
Cc: David S. Miller, Daniel Borkmann, Linus Torvalds, Peter Zijlstra,
netdev, kernel-team, linux-api
In-Reply-To: <20180327104242.3594bcac@gandalf.local.home>
On 3/27/18 7:42 AM, Steven Rostedt wrote:
> On Tue, 27 Mar 2018 10:18:24 -0400 (EDT)
> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
>
>> ----- On Mar 27, 2018, at 10:07 AM, rostedt rostedt@goodmis.org wrote:
>>
>>> On Mon, 26 Mar 2018 19:47:02 -0700
>>> Alexei Starovoitov <ast@fb.com> wrote:
>>>
>>>> From: Alexei Starovoitov <ast@kernel.org>
>>>>
>>>> introduce kernel_tracepoint_find_by_name() helper to let bpf core
>>>> find tracepoint by name and later attach bpf probe to a tracepoint
>>>>
>>>> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>>>
>>> Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
>>
>> Steven showed preference for tracepoint_kernel_find_by_name() at some
>> point (starting with a tracepoint_ prefix). I'm find with either of
>> the names.
>
> Yeah, I do prefer tracepoint_kernel_find_by_name() to stay consistent
> with the other tracepoint functions. But we have
> "for_each_kernel_tracepoint()" and not "for_each_tracepoint_kernel()",
> thus we need to pick being consistent with one or the other. One answer
> is to use tracpoint_kernel_find_by_name() and rename the for_each to
> for_each_tracpoint_kernel().
yep. that's exactly the reason I picked kernel_tracepoint_find_by_name()
to match for_each_kernel_tracepoint() naming.
I can certainly send a follow up patch to rename both to
*tracepoint_kernel* and then you can nack it because it breaks lttng :)
but let's do it in a separate thread.
Daniel,
do you mind adding { } as Steven requested while applying or
you want me to resubmit the whole thing?
Thanks!
^ permalink raw reply
* Re: [PATCH 5/6] rhashtable: support guaranteed successful insertion.
From: Herbert Xu @ 2018-03-27 15:56 UTC (permalink / raw)
To: NeilBrown; +Cc: Thomas Graf, netdev, linux-kernel
In-Reply-To: <152210718434.11435.6551477417902631683.stgit@noble>
On Tue, Mar 27, 2018 at 10:33:04AM +1100, NeilBrown wrote:
> The current rhashtable will fail an insertion if the hashtable
> it "too full", one of:
> - table already has 2^31 elements (-E2BIG)
> - a max_size was specified and table already has that
> many elements (rounded up to power of 2) (-E2BIG)
> - a single chain has more than 16 elements (-EBUSY)
> - table has more elements than the current table size,
> and allocating a new table fails (-ENOMEM)
> - a new page needed to be allocated for a nested table,
> and the memory allocation failed (-ENOMEM).
>
> A traditional hash table does not have a concept of "too full", and
> insertion only fails if the key already exists. Many users of hash
> tables have separate means of limiting the total number of entries,
> and are not susceptible to an attack which could cause unusually large
> hash chains. For those users, the need to check for errors when
> inserting objects to an rhashtable is an unnecessary burden and hence
> a potential source of bugs (as these failures are likely to be rare).
Did you actually encounter an insertion failure? The current code
should never fail an insertion until you actually run ouf memory.
That is unless you're using rhashtable when you should be using
rhlist instead.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [bpf-next PATCH] net: br_vlan build error
From: John Fastabend @ 2018-03-27 15:57 UTC (permalink / raw)
To: stephen, 3chas3; +Cc: netdev, davem
In-Reply-To: <20180327153859.28865.74511.stgit@john-Precision-Tower-5810>
On 03/27/2018 08:38 AM, John Fastabend wrote:
> Fix build error in br_if.c
>
> net/bridge/br_if.c: In function ‘br_mtu’:
> net/bridge/br_if.c:458:8: error: ‘const struct net_bridge’ has no member named ‘vlan_enabled’
> if (br->vlan_enabled)
> ^
> net/bridge/br_if.c:462:1: warning: control reaches end of non-void function [-Wreturn-type]
> }
> ^
> Fixes: 419d14af9e07f ("bridge: Allow max MTU when multiple VLANs present")
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
This is against net-next sorry for any confusion. Let me know
if you need me to respin.
@Chas, also I only build tested this to get my kernels building
again. You should probably verify I implemented what you expect
in the case without vlan filtering. It seems correct to me but
I'm not the expert here.
Thanks,
John
^ permalink raw reply
* Re: [Patch net] llc: properly handle dev_queue_xmit() return value
From: David Miller @ 2018-03-27 15:57 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev, noamr
In-Reply-To: <20180326220833.17351-1-xiyou.wangcong@gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Mon, 26 Mar 2018 15:08:33 -0700
> llc_conn_send_pdu() pushes the skb into write queue and
> calls llc_conn_send_pdus() to flush them out. However, the
> status of dev_queue_xmit() is not returned to caller,
> in this case, llc_conn_state_process().
>
> llc_conn_state_process() needs hold the skb no matter
> success or failure, because it still uses it after that,
> therefore we should hold skb before dev_queue_xmit() when
> that skb is the one being processed by llc_conn_state_process().
>
> For other callers, they can just pass NULL and ignore
> the return value as they are.
>
> Reported-by: Noam Rathaus <noamr@beyondsecurity.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Applied, thanks Cong.
^ permalink raw reply
* Re: [bpf-next PATCH] net: br_vlan build error
From: John Fastabend @ 2018-03-27 15:58 UTC (permalink / raw)
To: Nikolay Aleksandrov, stephen, 3chas3; +Cc: netdev, davem
In-Reply-To: <f49dbcf5-7ba2-9225-603c-ca2ad95de6bd@cumulusnetworks.com>
On 03/27/2018 08:49 AM, Nikolay Aleksandrov wrote:
> On 27/03/18 18:38, John Fastabend wrote:
>> Fix build error in br_if.c
>>
>> net/bridge/br_if.c: In function ‘br_mtu’:
>> net/bridge/br_if.c:458:8: error: ‘const struct net_bridge’ has no member named ‘vlan_enabled’
>> if (br->vlan_enabled)
>> ^
>> net/bridge/br_if.c:462:1: warning: control reaches end of non-void function [-Wreturn-type]
>> }
>> ^
>> Fixes: 419d14af9e07f ("bridge: Allow max MTU when multiple VLANs present")
>> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
>> ---
>> net/bridge/br_if.c | 27 +++++++++++++++++----------
>> 1 file changed, 17 insertions(+), 10 deletions(-)
>>
>
> I'm not sure what the rules about merging are, but just in case I
> already fixed this in net-next couple of days ago.
>
> commit 82792a070b16
> Author: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> Date: Fri Mar 23 18:27:06 2018 +0200
>
> net: bridge: fix direct access to bridge vlan_enabled and use helper
>
> https://patchwork.ozlabs.org/patch/890043/
>
>
> Cheers,
> Nik
>
Ah dang sorry about the noise then drop my patch.
^ permalink raw reply
* Re: [PATCH net-next v2 0/2] kernel: add support to collect hardware logs in crash recovery kernel
From: Eric W. Biederman @ 2018-03-27 15:59 UTC (permalink / raw)
To: Rahul Lakkireddy
Cc: netdev@vger.kernel.org, linux-fsdevel@vger.kernel.org,
kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
davem@davemloft.net, viro@zeniv.linux.org.uk,
stephen@networkplumber.org, akpm@linux-foundation.org,
torvalds@linux-foundation.org, Ganesh GR, Nirranjan Kirubaharan,
Indranil Choudhury
In-Reply-To: <20180327152715.GA18097@chelsio.com>
Rahul Lakkireddy <rahul.lakkireddy@chelsio.com> writes:
> On Tuesday, March 03/27/18, 2018 at 18:47:34 +0530, Eric W. Biederman wrote:
>> Rahul Lakkireddy <rahul.lakkireddy@chelsio.com> writes:
>>
>> > On Saturday, March 03/24/18, 2018 at 20:50:52 +0530, Eric W. Biederman wrote:
>> >>
>> >> Rahul Lakkireddy <rahul.lakkireddy@chelsio.com> writes:
>> >>
>> >> > On production servers running variety of workloads over time, kernel
>> >> > panic can happen sporadically after days or even months. It is
>> >> > important to collect as much debug logs as possible to root cause
>> >> > and fix the problem, that may not be easy to reproduce. Snapshot of
>> >> > underlying hardware/firmware state (like register dump, firmware
>> >> > logs, adapter memory, etc.), at the time of kernel panic will be very
>> >> > helpful while debugging the culprit device driver.
>> >> >
>> >> > This series of patches add new generic framework that enable device
>> >> > drivers to collect device specific snapshot of the hardware/firmware
>> >> > state of the underlying device in the crash recovery kernel. In crash
>> >> > recovery kernel, the collected logs are exposed via /sys/kernel/crashdd/
>> >> > directory, which is copied by user space scripts for post-analysis.
>> >> >
>> >> > A kernel module crashdd is newly added. In crash recovery kernel,
>> >> > crashdd exposes /sys/kernel/crashdd/ directory containing device
>> >> > specific hardware/firmware logs.
>> >>
>> >> Have you looked at instead of adding a sysfs file adding the dumps
>> >> as additional elf notes in /proc/vmcore?
>> >>
>> >
>> > I see the crash recovery kernel's memory is not present in any of the
>> > the PT_LOAD headers. So, makedumpfile is not collecting the dumps
>> > that are in crash recovery kernel's memory.
>> >
>> > Also, are you suggesting exporting the dumps themselves as PT_NOTE
>> > instead? I'll look into doing it this way.
>>
>> Yes. I was suggesting exporting the dumps themselves as PT_NOTE
>> in /proc/vmcore. I think that will allow makedumpfile to collect
>> your new information without modification.
>>
>
> If I export the dumps themselves as PT_NOTE in /proc/vmcore, can the
> crash tool work without modification; i.e can crash tool extract these
> notes?
I believe crash would need to be taught about these notes. This is
something new.
However "readelf -a random_elf_file" does display elf notes, and elf
notes in general are not hard to extract.
What I expect from an enconding in ELF core dump format is a way to
captuer the data, a way to encode the data, and a way to transport the
data to the people who care. Analysis tools are easy enough after the
fact.
Eric
^ permalink raw reply
* Re: [PATCH net 1/1] net/smc: use announced length in sock_recvmsg()
From: David Miller @ 2018-03-27 16:00 UTC (permalink / raw)
To: ubraun; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl
In-Reply-To: <20180327084350.92847-2-ubraun@linux.vnet.ibm.com>
From: Ursula Braun <ubraun@linux.vnet.ibm.com>
Date: Tue, 27 Mar 2018 10:43:50 +0200
> Not every CLC proposal message needs the maximum buffer length.
> Due to the MSG_WAITALL flag, it is important to use the peeked
> real length when receiving the message.
>
> Fixes: d63d271ce2b5ce ("smc: switch to sock_recvmsg()")
> Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH iproute2] Revert "iproute: "list/flush/save default" selected all of the routes"
From: Stephen Hemminger @ 2018-03-27 16:01 UTC (permalink / raw)
To: Alexander Zubkov; +Cc: Serhey Popovych, Luca Boccassi, netdev@vger.kernel.org
In-Reply-To: <2ee0179a-79c8-8504-afe1-6310ed4e858b@msu.ru>
On Wed, 14 Mar 2018 21:26:40 +0100
Alexander Zubkov <green@msu.ru> wrote:
> Hello,
>
> For example, it can be fixed in such way (patch is below):
> - split handling of default and all/any
> - set needed attributes in get_addr: PREFIXLEN_SPECIFIED flag for default
> - and AF_UNSPEC for all/any
> In this case "ip route show default" shows only default route and "ip
> route show all" shows all routes. And both also work when family (-4 or
> -6) is specified.
> Serhey, does it goes in line with what you wanted to achieve? Because I
> do not know - may be there are reasons why all/any should be provided
> with specific family. If you think this solution is suitable, I'll do
> some additional tests and package the patch in a proper way for this
> mailing list.
> And I'm unsure if check for AF_DECnet and AF_MPLS should be kept in both
> branches. May be someone have some additional thoughts on that?
>
I applied this to master.
We can work on the other cases after that.
^ permalink raw reply
* Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information
From: Leon Romanovsky @ 2018-03-27 16:01 UTC (permalink / raw)
To: Steve Wise
Cc: 'Jason Gunthorpe', 'David Ahern', stephen, netdev,
linux-rdma
In-Reply-To: <023301d3c5e2$a2a48c90$e7eda5b0$@opengridcomputing.com>
[-- Attachment #1: Type: text/plain, Size: 3987 bytes --]
On Tue, Mar 27, 2018 at 10:45:30AM -0500, Steve Wise wrote:
>
> >
> > On Tue, Mar 27, 2018 at 06:15:44PM +0300, Leon Romanovsky wrote:
> > > On Tue, Mar 27, 2018 at 08:44:55AM -0600, Jason Gunthorpe wrote:
> > > > On Tue, Mar 27, 2018 at 06:21:41AM +0300, Leon Romanovsky wrote:
> > > > > On Mon, Mar 26, 2018 at 04:30:33PM -0600, Jason Gunthorpe wrote:
> > > > > > On Mon, Mar 26, 2018 at 04:34:44PM -0500, Steve Wise wrote:
> > > > > > >
> > > > > > > On 3/26/2018 4:15 PM, Jason Gunthorpe wrote:
> > > > > > > > On Mon, Mar 26, 2018 at 09:30:41AM -0500, Steve Wise wrote:
> > > > > > > >>
> > > > > > > >> On 3/26/2018 9:17 AM, David Ahern wrote:
> > > > > > > >>> On 2/27/18 9:07 AM, Steve Wise wrote:
> > > > > > > >>>> diff --git a/rdma/rdma.h b/rdma/rdma.h
> > > > > > > >>>> index 5809f70..e55205b 100644
> > > > > > > >>>> +++ b/rdma/rdma.h
> > > > > > > >>>> @@ -18,10 +18,12 @@
> > > > > > > >>>> #include <libmnl/libmnl.h>
> > > > > > > >>>> #include <rdma/rdma_netlink.h>
> > > > > > > >>>> #include <time.h>
> > > > > > > >>>> +#include <net/if_arp.h>
> > > > > > > >>>>
> > > > > > > >>>> #include "list.h"
> > > > > > > >>>> #include "utils.h"
> > > > > > > >>>> #include "json_writer.h"
> > > > > > > >>>> +#include <rdma/rdma_cma.h>
> > > > > > > >>>>
> > > > > > > >>> did you forget to add rdma_cma.h? I don't see that file in my
> > repo.
> > > > > > > >> It is provided by the rdma-core package, upon which rdma tool
> > now
> > > > > > > >> depends for the rdma_port_space enum.
> > > > > > > > It is a kernel bug that enum is not in an include/uapi/rdma
> > header
> > > > > > > >
> > > > > > > > Fix it there and don't try to use rdma-core headers to get kernel
> > ABI.
> > > > > > > >
> > > > > > > > Jason
> > > > > > >
> > > > > > > I wish you'd commented on this just a little sooner. I just resent
> > v3
> > > > > > > of this series... with rdma_cma.h included. :)
> > > > > > >
> > > > > > > How about the restrack/nldev code just translates the port space
> > from
> > > > > > > enum rdma_port_space to a new ABI enum, say
> > nldev_rdma_port_space, that
> > > > > > > i add to rdma_netlink.h? I'd hate to open the can of worms of
> > trying to
> > > > > > > split rdma_cma.h into uabi and no uabi headers. :(
> > > > > >
> > > > > > If port space is already part of the ABI there isn't much reason to
> > > > > > translate it.
> > > > > >
> > > > > > You just need to pick the right header to put it in, since it is a verbs
> > > > > > define it doesn't belong in the netlink header.
> > > > >
> > > > > I completely understand Steve's concerns.
> > > > >
> > > > > I tried to do such thing (expose kernel headers) in first incarnation of
> > > > > rdmatool with attempt to clean IB/core as well to ensure that we
> > won't expose
> > > > > anything that is not implemented. It didn't go well.
> > > >
> > > > rdma-core is now using the kernel uapi/ headers natively, seems to be
> > > > going OK. What problem did you face?
> > >
> > > I didn't agree to move to UAPI defines which are not implemented and
> > not
> > > used in the kernel, so I sent small number of patches similar to those [1,
> > 2].
> > >
> > > Those patches were rejected.
> > >
> > > So please don't mix Steve's need to use 3 defines with very large and
> > > painful task to expose proper UAPIs.
> >
> > Steve can just move the 3 defines he needs to the uapi, we are doing
> > this incrementally..
> >
> > rdma_core does not define kernel ABI and it is totally wrong to use
> > random constants from rdma_cma.h as kernel ABI.
> >
>
> Proposal:
>
> Since the cm_id port space is part of the rdma_ucm_create_id struct in include/uapi/rdma/rdma_user_cm.h, I'll move the rdma_port_space enum there. And then my iproute2 series will have to add a copy of rdma_user_cm.h locally into rdma/include/uapi/rdma, right?
>
> Will that work for everyone?
You need to remove _PS from that structure and from the kernel with
justification that it is safe to do.
Thanks
>
> Steve.
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v3 iproute2 1/1] tc: fix conversion types when printing actions unsigned values
From: Stephen Hemminger @ 2018-03-27 16:02 UTC (permalink / raw)
To: Roman Mashak; +Cc: netdev, jhs, xiyou.wangcong, jiri
In-Reply-To: <1521493541-31340-1-git-send-email-mrv@mojatatu.com>
On Mon, 19 Mar 2018 17:05:41 -0400
Roman Mashak <mrv@mojatatu.com> wrote:
> diff --git a/tc/m_gact.c b/tc/m_gact.c
> index 16c4413f4217..52022415db48 100644
> --- a/tc/m_gact.c
> +++ b/tc/m_gact.c
> @@ -194,7 +194,7 @@ print_gact(struct action_util *au, FILE *f, struct rtattr *arg)
> print_string(PRINT_ANY, "random_type", "\n\t random type %s",
> prob_n2a(pp->ptype));
> print_action_control(f, " ", pp->paction, " ");
> - print_int(PRINT_ANY, "val", "val %d", pp->pval);
> + print_int(PRINT_ANY, "val", "val %u", pp->pval);
> close_json_object();
This needs to be print_uint in order to work correctly with json output.
Also, please consider doing json on all the match types in a later patch
for net-next.
^ permalink raw reply
* Re: [PATCH net 0/2] mlx4 misc fixes for 4.16
From: David Miller @ 2018-03-27 16:02 UTC (permalink / raw)
To: tariqt; +Cc: netdev, eranbe
In-Reply-To: <1522150879-16501-1-git-send-email-tariqt@mellanox.com>
From: Tariq Toukan <tariqt@mellanox.com>
Date: Tue, 27 Mar 2018 14:41:17 +0300
> This patchset contains misc bug fixes from the team
> to the mlx4 Core and Eth drivers.
>
> Patch 1 by Eran fixes a control mix of PFC and Global pauses, please queue it
> to -stable for >= v4.8.
> Patch 2 by Moshe fixes a resource leak in slave's delete flow, please queue it
> to -stable for >= v4.5.
>
> Series generated against net commit:
> 3c82b372a9f4 net: dsa: mt7530: fix module autoloading for OF platform drivers
Series applied and queued up for -stable, thank you.
^ permalink raw reply
* Re: [PATCH iproute2 1/1] tc: print index, refcnt & bindcnt for nat action
From: Stephen Hemminger @ 2018-03-27 16:05 UTC (permalink / raw)
To: Roman Mashak; +Cc: netdev, jhs, xiyou.wangcong, jiri
In-Reply-To: <1521570107-30024-1-git-send-email-mrv@mojatatu.com>
On Tue, 20 Mar 2018 14:21:47 -0400
Roman Mashak <mrv@mojatatu.com> wrote:
> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
> ---
> tc/m_nat.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/tc/m_nat.c b/tc/m_nat.c
> index 1e4ff51fe75a..f6e373957c1b 100644
> --- a/tc/m_nat.c
> +++ b/tc/m_nat.c
> @@ -169,6 +169,9 @@ print_nat(struct action_util *au, FILE * f, struct rtattr *arg)
> format_host_r(AF_INET, 4, &sel->new_addr, buf2, sizeof(buf2)));
> print_action_control(f, " ", sel->action, "");
>
> + fprintf(f, "\n\t index %u ref %d bind %d",
> + sel->index, sel->refcnt, sel->bindcnt);
> +
> if (show_stats) {
> if (tb[TCA_NAT_TM]) {
> struct tcf_t *tm = RTA_DATA(tb[TCA_NAT_TM]);
> @@ -177,6 +180,8 @@ print_nat(struct action_util *au, FILE * f, struct rtattr *arg)
> }
> }
>
> + fprintf(f, "\n");
> +
> return 0;
> }
Rather than printing newline all the time, you need to use _SL_ to keep the optional
oneline output format.
I.e
fprintf(f, "%s\t index %u ref %d bind %d",
_SL_, sel->index, sel->refcnt, sel->bindcnt);
^ permalink raw reply
* Re: [PATCH v6 bpf-next 07/11] tracepoint: introduce kernel_tracepoint_find_by_name
From: Mathieu Desnoyers @ 2018-03-27 16:09 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: rostedt, David S. Miller, Daniel Borkmann, Linus Torvalds,
Peter Zijlstra, netdev, kernel-team, linux-api
In-Reply-To: <8600dbae-389d-dd01-f003-44f1af7dcf0a@fb.com>
----- On Mar 27, 2018, at 11:53 AM, Alexei Starovoitov ast@fb.com wrote:
> On 3/27/18 7:42 AM, Steven Rostedt wrote:
>> On Tue, 27 Mar 2018 10:18:24 -0400 (EDT)
>> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
>>
>>> ----- On Mar 27, 2018, at 10:07 AM, rostedt rostedt@goodmis.org wrote:
>>>
>>>> On Mon, 26 Mar 2018 19:47:02 -0700
>>>> Alexei Starovoitov <ast@fb.com> wrote:
>>>>
>>>>> From: Alexei Starovoitov <ast@kernel.org>
>>>>>
>>>>> introduce kernel_tracepoint_find_by_name() helper to let bpf core
>>>>> find tracepoint by name and later attach bpf probe to a tracepoint
>>>>>
>>>>> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>>>>
>>>> Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
>>>
>>> Steven showed preference for tracepoint_kernel_find_by_name() at some
>>> point (starting with a tracepoint_ prefix). I'm find with either of
>>> the names.
>>
>> Yeah, I do prefer tracepoint_kernel_find_by_name() to stay consistent
>> with the other tracepoint functions. But we have
>> "for_each_kernel_tracepoint()" and not "for_each_tracepoint_kernel()",
>> thus we need to pick being consistent with one or the other. One answer
>> is to use tracpoint_kernel_find_by_name() and rename the for_each to
>> for_each_tracpoint_kernel().
>
> yep. that's exactly the reason I picked kernel_tracepoint_find_by_name()
> to match for_each_kernel_tracepoint() naming.
>
> I can certainly send a follow up patch to rename both to
> *tracepoint_kernel* and then you can nack it because it breaks lttng :)
If Steven prefers changing the name of for_each_kernel_tracepoint() to
for_each_tracepoint_kernel(), I'll adapt LTTng accordingly. I don't
mind either way, as long as the change is justified.
Thanks,
Mathieu
> but let's do it in a separate thread.
>
> Daniel,
> do you mind adding { } as Steven requested while applying or
> you want me to resubmit the whole thing?
>
> Thanks!
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* RE: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information
From: Steve Wise @ 2018-03-27 16:20 UTC (permalink / raw)
To: 'Leon Romanovsky'
Cc: 'Jason Gunthorpe', 'David Ahern', stephen, netdev,
linux-rdma
In-Reply-To: <20180327160150.GB1877@mtr-leonro.local>
> On Tue, Mar 27, 2018 at 10:45:30AM -0500, Steve Wise wrote:
> >
> > >
> > > On Tue, Mar 27, 2018 at 06:15:44PM +0300, Leon Romanovsky wrote:
> > > > On Tue, Mar 27, 2018 at 08:44:55AM -0600, Jason Gunthorpe wrote:
> > > > > On Tue, Mar 27, 2018 at 06:21:41AM +0300, Leon Romanovsky
> wrote:
> > > > > > On Mon, Mar 26, 2018 at 04:30:33PM -0600, Jason Gunthorpe
> wrote:
> > > > > > > On Mon, Mar 26, 2018 at 04:34:44PM -0500, Steve Wise wrote:
> > > > > > > >
> > > > > > > > On 3/26/2018 4:15 PM, Jason Gunthorpe wrote:
> > > > > > > > > On Mon, Mar 26, 2018 at 09:30:41AM -0500, Steve Wise
> wrote:
> > > > > > > > >>
> > > > > > > > >> On 3/26/2018 9:17 AM, David Ahern wrote:
> > > > > > > > >>> On 2/27/18 9:07 AM, Steve Wise wrote:
> > > > > > > > >>>> diff --git a/rdma/rdma.h b/rdma/rdma.h
> > > > > > > > >>>> index 5809f70..e55205b 100644
> > > > > > > > >>>> +++ b/rdma/rdma.h
> > > > > > > > >>>> @@ -18,10 +18,12 @@
> > > > > > > > >>>> #include <libmnl/libmnl.h>
> > > > > > > > >>>> #include <rdma/rdma_netlink.h>
> > > > > > > > >>>> #include <time.h>
> > > > > > > > >>>> +#include <net/if_arp.h>
> > > > > > > > >>>>
> > > > > > > > >>>> #include "list.h"
> > > > > > > > >>>> #include "utils.h"
> > > > > > > > >>>> #include "json_writer.h"
> > > > > > > > >>>> +#include <rdma/rdma_cma.h>
> > > > > > > > >>>>
> > > > > > > > >>> did you forget to add rdma_cma.h? I don't see that file
in
> my
> > > repo.
> > > > > > > > >> It is provided by the rdma-core package, upon which rdma
> tool
> > > now
> > > > > > > > >> depends for the rdma_port_space enum.
> > > > > > > > > It is a kernel bug that enum is not in an
include/uapi/rdma
> > > header
> > > > > > > > >
> > > > > > > > > Fix it there and don't try to use rdma-core headers to get
> kernel
> > > ABI.
> > > > > > > > >
> > > > > > > > > Jason
> > > > > > > >
> > > > > > > > I wish you'd commented on this just a little sooner. I just
> resent
> > > v3
> > > > > > > > of this series... with rdma_cma.h included. :)
> > > > > > > >
> > > > > > > > How about the restrack/nldev code just translates the port
> space
> > > from
> > > > > > > > enum rdma_port_space to a new ABI enum, say
> > > nldev_rdma_port_space, that
> > > > > > > > i add to rdma_netlink.h? I'd hate to open the can of worms
of
> > > trying to
> > > > > > > > split rdma_cma.h into uabi and no uabi headers. :(
> > > > > > >
> > > > > > > If port space is already part of the ABI there isn't much
reason to
> > > > > > > translate it.
> > > > > > >
> > > > > > > You just need to pick the right header to put it in, since it
is a
> verbs
> > > > > > > define it doesn't belong in the netlink header.
> > > > > >
> > > > > > I completely understand Steve's concerns.
> > > > > >
> > > > > > I tried to do such thing (expose kernel headers) in first
incarnation
> of
> > > > > > rdmatool with attempt to clean IB/core as well to ensure that we
> > > won't expose
> > > > > > anything that is not implemented. It didn't go well.
> > > > >
> > > > > rdma-core is now using the kernel uapi/ headers natively, seems to
> be
> > > > > going OK. What problem did you face?
> > > >
> > > > I didn't agree to move to UAPI defines which are not implemented and
> > > not
> > > > used in the kernel, so I sent small number of patches similar to
those
> [1,
> > > 2].
> > > >
> > > > Those patches were rejected.
> > > >
> > > > So please don't mix Steve's need to use 3 defines with very large
and
> > > > painful task to expose proper UAPIs.
> > >
> > > Steve can just move the 3 defines he needs to the uapi, we are doing
> > > this incrementally..
> > >
> > > rdma_core does not define kernel ABI and it is totally wrong to use
> > > random constants from rdma_cma.h as kernel ABI.
> > >
> >
> > Proposal:
> >
> > Since the cm_id port space is part of the rdma_ucm_create_id struct in
> include/uapi/rdma/rdma_user_cm.h, I'll move the rdma_port_space enum
> there. And then my iproute2 series will have to add a copy of
> rdma_user_cm.h locally into rdma/include/uapi/rdma, right?
> >
> > Will that work for everyone?
>
> You need to remove _PS from that structure and from the kernel with
> justification that it is safe to do.
>
> Thanks
I'm pretty sure port space is needed. That struct is used to create a user
mode cm_id...
^ permalink raw reply
* [RFC PATCH iproute2] Drop capabilities if not running ip exec vrf with libcap
From: Luca Boccassi @ 2018-03-27 16:24 UTC (permalink / raw)
To: netdev; +Cc: dsahern, luto, stephen, Luca Boccassi
ip vrf exec requires root or CAP_NET_ADMIN, CAP_SYS_ADMIN and
CAP_DAC_OVERRIDE. It is not possible to run unprivileged commands like
ping as non-root or non-cap-enabled due to this requirement.
To allow users and administrators to safely add the required
capabilities to the binary, drop all capabilities on start if not
invoked with "vrf exec".
Update the manpage with the requirements.
Signed-off-by: Luca Boccassi <bluca@debian.org>
---
I'd like to be able to run ip vrf exec as a normal user, does this approach
sound sensible? Any concerns? Are there any other alternatives?
It would be up to each user/admin/whatever to make sure the program is
built with libcap and to add the capabilities manually, so nothing will be
there by default.
configure | 17 +++++++++++++++++
ip/ip.c | 34 ++++++++++++++++++++++++++++++++++
man/man8/ip-vrf.8 | 8 ++++++++
3 files changed, 59 insertions(+)
diff --git a/configure b/configure
index f7c2d7a7..5ef5cd4c 100755
--- a/configure
+++ b/configure
@@ -336,6 +336,20 @@ EOF
rm -f $TMPDIR/strtest.c $TMPDIR/strtest
}
+check_cap()
+{
+ if ${PKG_CONFIG} libcap --exists
+ then
+ echo "HAVE_CAP:=y" >>$CONFIG
+ echo "yes"
+
+ echo 'CFLAGS += -DHAVE_LIBCAP' `${PKG_CONFIG} libcap --cflags` >>$CONFIG
+ echo 'LDLIBS +=' `${PKG_CONFIG} libcap --libs` >> $CONFIG
+ else
+ echo "no"
+ fi
+}
+
quiet_config()
{
cat <<EOF
@@ -410,6 +424,9 @@ check_berkeley_db
echo -n "need for strlcpy: "
check_strlcpy
+echo -n "libcap support: "
+check_cap
+
echo >> $CONFIG
echo "%.o: %.c" >> $CONFIG
echo ' $(QUIET_CC)$(CC) $(CFLAGS) $(EXTRA_CFLAGS) -c -o $@ $<' >> $CONFIG
diff --git a/ip/ip.c b/ip/ip.c
index e0cd96cb..49739571 100644
--- a/ip/ip.c
+++ b/ip/ip.c
@@ -17,6 +17,10 @@
#include <netinet/in.h>
#include <string.h>
#include <errno.h>
+#include <sys/types.h>
+#ifdef HAVE_LIBCAP
+#include <sys/capability.h>
+#endif
#include "SNAPSHOT.h"
#include "utils.h"
@@ -68,6 +72,24 @@ static int do_help(int argc, char **argv)
return 0;
}
+static void drop_cap(void)
+{
+#ifdef HAVE_LIBCAP
+ /* don't harmstring root/sudo */
+ if (getuid() != 0 && geteuid() != 0) {
+ cap_t capabilities;
+ capabilities = cap_get_proc();
+ if (!capabilities)
+ exit(EXIT_FAILURE);
+ if (cap_clear(capabilities) != 0)
+ exit(EXIT_FAILURE);
+ if (cap_set_proc(capabilities) != 0)
+ exit(EXIT_FAILURE);
+ cap_free(capabilities);
+ }
+#endif
+}
+
static const struct cmd {
const char *cmd;
int (*func)(int argc, char **argv);
@@ -174,6 +196,18 @@ int main(int argc, char **argv)
char *batch_file = NULL;
int color = 0;
+ /* to run vrf exec without root, capabilities might be set, drop them
+ * if not needed as the first thing.
+ * execv will drop them for the child command.
+ * vrf exec requires:
+ * - cap_dac_override to create the cgroup subdir in /sys
+ * - cap_sys_admin to load the BPF program
+ * - cap_net_admin to set the socket into the cgroup
+ */
+ if (argc < 3 || strcmp(argv[1], "vrf") != 0 ||
+ strcmp(argv[2], "exec") != 0)
+ drop_cap();
+
basename = strrchr(argv[0], '/');
if (basename == NULL)
basename = argv[0];
diff --git a/man/man8/ip-vrf.8 b/man/man8/ip-vrf.8
index 18789339..1a42cebe 100644
--- a/man/man8/ip-vrf.8
+++ b/man/man8/ip-vrf.8
@@ -63,6 +63,14 @@ a VRF other than the default VRF (main table). A command can be run against
the default VRF by passing the "default" as the VRF name. This is useful if
the current shell is associated with another VRF (e.g, Management VRF).
+This command requires the system to be booted with cgroup v2 (e.g. with systemd,
+add systemd.unified_cgroup_hierarchy=1 to the kernel command line).
+
+This command also requires to be ran as root or with the CAP_SYS_ADMIN,
+CAP_NET_ADMIN and CAP_DAC_OVERRIDE capabilities. If built with libcap and if
+capabilities are added to the ip binary program via setcap, the program will
+drop them as the first thing when invoked, unless the command is vrf exec.
+
.TP
.B ip vrf identify [PID] - Report VRF association for process
.sp
--
2.14.2
^ permalink raw reply related
* Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information
From: Leon Romanovsky @ 2018-03-27 16:30 UTC (permalink / raw)
To: Steve Wise
Cc: 'Jason Gunthorpe', 'David Ahern', stephen, netdev,
linux-rdma
In-Reply-To: <026701d3c5e7$838c65d0$8aa53170$@opengridcomputing.com>
[-- Attachment #1: Type: text/plain, Size: 4629 bytes --]
On Tue, Mar 27, 2018 at 11:20:25AM -0500, Steve Wise wrote:
> > On Tue, Mar 27, 2018 at 10:45:30AM -0500, Steve Wise wrote:
> > >
> > > >
> > > > On Tue, Mar 27, 2018 at 06:15:44PM +0300, Leon Romanovsky wrote:
> > > > > On Tue, Mar 27, 2018 at 08:44:55AM -0600, Jason Gunthorpe wrote:
> > > > > > On Tue, Mar 27, 2018 at 06:21:41AM +0300, Leon Romanovsky
> > wrote:
> > > > > > > On Mon, Mar 26, 2018 at 04:30:33PM -0600, Jason Gunthorpe
> > wrote:
> > > > > > > > On Mon, Mar 26, 2018 at 04:34:44PM -0500, Steve Wise wrote:
> > > > > > > > >
> > > > > > > > > On 3/26/2018 4:15 PM, Jason Gunthorpe wrote:
> > > > > > > > > > On Mon, Mar 26, 2018 at 09:30:41AM -0500, Steve Wise
> > wrote:
> > > > > > > > > >>
> > > > > > > > > >> On 3/26/2018 9:17 AM, David Ahern wrote:
> > > > > > > > > >>> On 2/27/18 9:07 AM, Steve Wise wrote:
> > > > > > > > > >>>> diff --git a/rdma/rdma.h b/rdma/rdma.h
> > > > > > > > > >>>> index 5809f70..e55205b 100644
> > > > > > > > > >>>> +++ b/rdma/rdma.h
> > > > > > > > > >>>> @@ -18,10 +18,12 @@
> > > > > > > > > >>>> #include <libmnl/libmnl.h>
> > > > > > > > > >>>> #include <rdma/rdma_netlink.h>
> > > > > > > > > >>>> #include <time.h>
> > > > > > > > > >>>> +#include <net/if_arp.h>
> > > > > > > > > >>>>
> > > > > > > > > >>>> #include "list.h"
> > > > > > > > > >>>> #include "utils.h"
> > > > > > > > > >>>> #include "json_writer.h"
> > > > > > > > > >>>> +#include <rdma/rdma_cma.h>
> > > > > > > > > >>>>
> > > > > > > > > >>> did you forget to add rdma_cma.h? I don't see that file
> in
> > my
> > > > repo.
> > > > > > > > > >> It is provided by the rdma-core package, upon which rdma
> > tool
> > > > now
> > > > > > > > > >> depends for the rdma_port_space enum.
> > > > > > > > > > It is a kernel bug that enum is not in an
> include/uapi/rdma
> > > > header
> > > > > > > > > >
> > > > > > > > > > Fix it there and don't try to use rdma-core headers to get
> > kernel
> > > > ABI.
> > > > > > > > > >
> > > > > > > > > > Jason
> > > > > > > > >
> > > > > > > > > I wish you'd commented on this just a little sooner. I just
> > resent
> > > > v3
> > > > > > > > > of this series... with rdma_cma.h included. :)
> > > > > > > > >
> > > > > > > > > How about the restrack/nldev code just translates the port
> > space
> > > > from
> > > > > > > > > enum rdma_port_space to a new ABI enum, say
> > > > nldev_rdma_port_space, that
> > > > > > > > > i add to rdma_netlink.h? I'd hate to open the can of worms
> of
> > > > trying to
> > > > > > > > > split rdma_cma.h into uabi and no uabi headers. :(
> > > > > > > >
> > > > > > > > If port space is already part of the ABI there isn't much
> reason to
> > > > > > > > translate it.
> > > > > > > >
> > > > > > > > You just need to pick the right header to put it in, since it
> is a
> > verbs
> > > > > > > > define it doesn't belong in the netlink header.
> > > > > > >
> > > > > > > I completely understand Steve's concerns.
> > > > > > >
> > > > > > > I tried to do such thing (expose kernel headers) in first
> incarnation
> > of
> > > > > > > rdmatool with attempt to clean IB/core as well to ensure that we
> > > > won't expose
> > > > > > > anything that is not implemented. It didn't go well.
> > > > > >
> > > > > > rdma-core is now using the kernel uapi/ headers natively, seems to
> > be
> > > > > > going OK. What problem did you face?
> > > > >
> > > > > I didn't agree to move to UAPI defines which are not implemented and
> > > > not
> > > > > used in the kernel, so I sent small number of patches similar to
> those
> > [1,
> > > > 2].
> > > > >
> > > > > Those patches were rejected.
> > > > >
> > > > > So please don't mix Steve's need to use 3 defines with very large
> and
> > > > > painful task to expose proper UAPIs.
> > > >
> > > > Steve can just move the 3 defines he needs to the uapi, we are doing
> > > > this incrementally..
> > > >
> > > > rdma_core does not define kernel ABI and it is totally wrong to use
> > > > random constants from rdma_cma.h as kernel ABI.
> > > >
> > >
> > > Proposal:
> > >
> > > Since the cm_id port space is part of the rdma_ucm_create_id struct in
> > include/uapi/rdma/rdma_user_cm.h, I'll move the rdma_port_space enum
> > there. And then my iproute2 series will have to add a copy of
> > rdma_user_cm.h locally into rdma/include/uapi/rdma, right?
> > >
> > > Will that work for everyone?
> >
> > You need to remove _PS from that structure and from the kernel with
> > justification that it is safe to do.
> >
> > Thanks
>
> I'm pretty sure port space is needed. That struct is used to create a user
> mode cm_id...
Sorry, it is RDMA_PS_SDP.
Thanks
>
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [net-next PATCH v2 00/10] Add support for netcp driver on K2G SoC
From: Murali Karicheri @ 2018-03-27 16:31 UTC (permalink / raw)
To: robh+dt, mark.rutland, ssantosh, malat, w-kwok2, devicetree,
linux-kernel, linux-arm-kernel, davem, netdev
K2G SoC is another variant of Keystone family of SoCs. This patch
series add support for NetCP driver on this SoC. The QMSS found on
K2G SoC is a cut down version of the QMSS found on other keystone
devices with less number of queues, internal link ram etc. The patch
series has 2 patch sets that goes into the drivers/soc and the
rest has to be applied to net sub system. Please review and merge
if this looks good.
Thanks
The boot logs on K2G ICE board (tftp boot over Ethernet)
is at https://pastebin.ubuntu.com/p/VQTv3c2XBS/
The boot logs on K2G GP board (tftp boot over Ethernet)
is at https://pastebin.ubuntu.com/p/6Vh55DW8vT/
This series applies to net-next master branch.
Change history:
v2 - Addressed following comments on initial version
- split patch 3/5 to multiple patches from Andrew Lunn
Murali Karicheri (10):
soc: ti: K2G: enhancement to support QMSS in NSS
soc: ti: K2G: provide APIs to support driver probe deferral
net: netcp: ethss: make call to gbe_sgmii_config() conditional
net: netcp: ethss: add support for handling sgmii link interface
net: netcp: ethss: use rgmii link status for 2u cpsw hardware
net: netcp: ethss: map vlan priorities to zero flow
net: netcp: ethss: re-use stats handling code for 2u hardware
net: netcp: ethss: use of_get_phy_mode() to support different RGMII
modes
Revert "net: netcp: remove dead code from the driver"
net: netcp: support probe deferral
.../bindings/soc/ti/keystone-navigator-qmss.txt | 7 ++
drivers/net/ethernet/ti/netcp.h | 2 +
drivers/net/ethernet/ti/netcp_core.c | 13 +++
drivers/net/ethernet/ti/netcp_ethss.c | 75 ++++++++++++++---
drivers/soc/ti/knav_dma.c | 8 ++
drivers/soc/ti/knav_qmss.h | 6 ++
drivers/soc/ti/knav_qmss_queue.c | 98 +++++++++++++++++-----
include/linux/soc/ti/knav_dma.h | 12 +++
include/linux/soc/ti/knav_qmss.h | 1 +
9 files changed, 189 insertions(+), 33 deletions(-)
--
1.9.1
^ permalink raw reply
* [net-next PATCH v2 01/10] soc: ti: K2G: enhancement to support QMSS in NSS
From: Murali Karicheri @ 2018-03-27 16:31 UTC (permalink / raw)
To: robh+dt, mark.rutland, ssantosh, malat, w-kwok2, devicetree,
linux-kernel, linux-arm-kernel, davem, netdev
In-Reply-To: <1522168309-12338-1-git-send-email-m-karicheri2@ti.com>
Navigator Subsystem (NSS) available on K2G SoC has a cut down
version of QMSS with less number of queues, internal linking ram
with lesser number of buffers etc. It doesn't have status and
explicit push register space as in QMSS available on other K2 SoCs.
So define reg indices specific to QMSS on K2G. This patch introduces
"keystone-navigator-qmss-l" compatibility to identify QMSS on
K2G NSS (QMSS Lite) and to customize the dts handling code. Per
Device manual, descriptors with index less than or equal to
regions0_size is in region 0 in the case of QMSS where as for
QMSS Lite, descriptors with index less than regions0_size is in
region 0. So update the size accordingly in the regions0_size bits
of the linking ram size 0 register.
Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
---
.../bindings/soc/ti/keystone-navigator-qmss.txt | 7 ++
drivers/soc/ti/knav_qmss.h | 6 ++
drivers/soc/ti/knav_qmss_queue.c | 90 ++++++++++++++++------
3 files changed, 81 insertions(+), 22 deletions(-)
diff --git a/Documentation/devicetree/bindings/soc/ti/keystone-navigator-qmss.txt b/Documentation/devicetree/bindings/soc/ti/keystone-navigator-qmss.txt
index 77cd42c..1b0878a 100644
--- a/Documentation/devicetree/bindings/soc/ti/keystone-navigator-qmss.txt
+++ b/Documentation/devicetree/bindings/soc/ti/keystone-navigator-qmss.txt
@@ -18,6 +18,7 @@ pool management.
Required properties:
- compatible : Must be "ti,keystone-navigator-qmss";
+ : Must be "ti,keystone-navigator-qmss-l" for NSS Lite
- clocks : phandle to the reference clock for this device.
- queue-range : <start number> total range of queue numbers for the device.
- linkram0 : <address size> for internal link ram, where size is the total
@@ -39,6 +40,12 @@ Required properties:
- Descriptor memory setup region.
- Queue Management/Queue Proxy region for queue Push.
- Queue Management/Queue Proxy region for queue Pop.
+
+For NSS lite, following QMSS reg indexes are used in that order
+ - Queue Peek region.
+ - Queue configuration region.
+ - Queue Management/Queue Proxy region for queue Push/Pop.
+
- queue-pools : child node classifying the queue ranges into pools.
Queue ranges are grouped into 3 type of pools:
- qpend : pool of qpend(interruptible) queues
diff --git a/drivers/soc/ti/knav_qmss.h b/drivers/soc/ti/knav_qmss.h
index 905b974..5fa1ce6 100644
--- a/drivers/soc/ti/knav_qmss.h
+++ b/drivers/soc/ti/knav_qmss.h
@@ -292,6 +292,11 @@ struct knav_queue {
struct list_head list;
};
+enum qmss_version {
+ QMSS,
+ QMSS_LITE,
+};
+
struct knav_device {
struct device *dev;
unsigned base_id;
@@ -305,6 +310,7 @@ struct knav_device {
struct list_head pools;
struct list_head pdsps;
struct list_head qmgrs;
+ enum qmss_version version;
};
struct knav_range_ops {
diff --git a/drivers/soc/ti/knav_qmss_queue.c b/drivers/soc/ti/knav_qmss_queue.c
index 77d6b5c..5b2c4e9 100644
--- a/drivers/soc/ti/knav_qmss_queue.c
+++ b/drivers/soc/ti/knav_qmss_queue.c
@@ -42,6 +42,15 @@
#define KNAV_QUEUE_PUSH_REG_INDEX 4
#define KNAV_QUEUE_POP_REG_INDEX 5
+/* Queue manager register indices in DTS for QMSS in NSS lite.
+ * There are no status and vbusm push registers on this version
+ * QMSS. Push registers are same as pop, So all indices above 1
+ * are to be re-defined
+ */
+#define KNAV_L_QUEUE_CONFIG_REG_INDEX 1
+#define KNAV_L_QUEUE_REGION_REG_INDEX 2
+#define KNAV_L_QUEUE_PUSH_REG_INDEX 3
+
/* PDSP register indices in DTS */
#define KNAV_QUEUE_PDSP_IRAM_REG_INDEX 0
#define KNAV_QUEUE_PDSP_REGS_REG_INDEX 1
@@ -1169,8 +1178,12 @@ static int knav_queue_setup_link_ram(struct knav_device *kdev)
dev_dbg(kdev->dev, "linkram0: dma:%pad, virt:%p, size:%x\n",
&block->dma, block->virt, block->size);
writel_relaxed((u32)block->dma, &qmgr->reg_config->link_ram_base0);
- writel_relaxed(block->size, &qmgr->reg_config->link_ram_size0);
-
+ if (kdev->version == QMSS_LITE)
+ writel_relaxed(block->size,
+ &qmgr->reg_config->link_ram_size0);
+ else
+ writel_relaxed(block->size - 1,
+ &qmgr->reg_config->link_ram_size0);
block++;
if (!block->size)
continue;
@@ -1387,42 +1400,64 @@ static int knav_queue_init_qmgrs(struct knav_device *kdev,
qmgr->reg_peek =
knav_queue_map_reg(kdev, child,
KNAV_QUEUE_PEEK_REG_INDEX);
- qmgr->reg_status =
- knav_queue_map_reg(kdev, child,
- KNAV_QUEUE_STATUS_REG_INDEX);
+
+ if (kdev->version == QMSS) {
+ qmgr->reg_status =
+ knav_queue_map_reg(kdev, child,
+ KNAV_QUEUE_STATUS_REG_INDEX);
+ }
+
qmgr->reg_config =
knav_queue_map_reg(kdev, child,
+ (kdev->version == QMSS_LITE) ?
+ KNAV_L_QUEUE_CONFIG_REG_INDEX :
KNAV_QUEUE_CONFIG_REG_INDEX);
qmgr->reg_region =
knav_queue_map_reg(kdev, child,
+ (kdev->version == QMSS_LITE) ?
+ KNAV_L_QUEUE_REGION_REG_INDEX :
KNAV_QUEUE_REGION_REG_INDEX);
+
qmgr->reg_push =
knav_queue_map_reg(kdev, child,
- KNAV_QUEUE_PUSH_REG_INDEX);
- qmgr->reg_pop =
- knav_queue_map_reg(kdev, child,
- KNAV_QUEUE_POP_REG_INDEX);
+ (kdev->version == QMSS_LITE) ?
+ KNAV_L_QUEUE_PUSH_REG_INDEX :
+ KNAV_QUEUE_PUSH_REG_INDEX);
+
+ if (kdev->version == QMSS) {
+ qmgr->reg_pop =
+ knav_queue_map_reg(kdev, child,
+ KNAV_QUEUE_POP_REG_INDEX);
+ }
- if (IS_ERR(qmgr->reg_peek) || IS_ERR(qmgr->reg_status) ||
+ if (IS_ERR(qmgr->reg_peek) ||
+ ((kdev->version == QMSS) &&
+ (IS_ERR(qmgr->reg_status) || IS_ERR(qmgr->reg_pop))) ||
IS_ERR(qmgr->reg_config) || IS_ERR(qmgr->reg_region) ||
- IS_ERR(qmgr->reg_push) || IS_ERR(qmgr->reg_pop)) {
+ IS_ERR(qmgr->reg_push)) {
dev_err(dev, "failed to map qmgr regs\n");
+ if (kdev->version == QMSS) {
+ if (!IS_ERR(qmgr->reg_status))
+ devm_iounmap(dev, qmgr->reg_status);
+ if (!IS_ERR(qmgr->reg_pop))
+ devm_iounmap(dev, qmgr->reg_pop);
+ }
if (!IS_ERR(qmgr->reg_peek))
devm_iounmap(dev, qmgr->reg_peek);
- if (!IS_ERR(qmgr->reg_status))
- devm_iounmap(dev, qmgr->reg_status);
if (!IS_ERR(qmgr->reg_config))
devm_iounmap(dev, qmgr->reg_config);
if (!IS_ERR(qmgr->reg_region))
devm_iounmap(dev, qmgr->reg_region);
if (!IS_ERR(qmgr->reg_push))
devm_iounmap(dev, qmgr->reg_push);
- if (!IS_ERR(qmgr->reg_pop))
- devm_iounmap(dev, qmgr->reg_pop);
devm_kfree(dev, qmgr);
continue;
}
+ /* Use same push register for pop as well */
+ if (kdev->version == QMSS_LITE)
+ qmgr->reg_pop = qmgr->reg_push;
+
list_add_tail(&qmgr->list, &kdev->qmgrs);
dev_info(dev, "added qmgr start queue %d, num of queues %d, reg_peek %p, reg_status %p, reg_config %p, reg_region %p, reg_push %p, reg_pop %p\n",
qmgr->start_queue, qmgr->num_queues,
@@ -1681,10 +1716,24 @@ static int knav_queue_init_queues(struct knav_device *kdev)
return 0;
}
+/* Match table for of_platform binding */
+static const struct of_device_id keystone_qmss_of_match[] = {
+ {
+ .compatible = "ti,keystone-navigator-qmss",
+ },
+ {
+ .compatible = "ti,keystone-navigator-qmss-l",
+ .data = (void *)QMSS_LITE,
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, keystone_qmss_of_match);
+
static int knav_queue_probe(struct platform_device *pdev)
{
struct device_node *node = pdev->dev.of_node;
struct device_node *qmgrs, *queue_pools, *regions, *pdsps;
+ const struct of_device_id *match;
struct device *dev = &pdev->dev;
u32 temp[2];
int ret;
@@ -1700,6 +1749,10 @@ static int knav_queue_probe(struct platform_device *pdev)
return -ENOMEM;
}
+ match = of_match_device(of_match_ptr(keystone_qmss_of_match), dev);
+ if (match && match->data)
+ kdev->version = QMSS_LITE;
+
platform_set_drvdata(pdev, kdev);
kdev->dev = dev;
INIT_LIST_HEAD(&kdev->queue_ranges);
@@ -1815,13 +1868,6 @@ static int knav_queue_remove(struct platform_device *pdev)
return 0;
}
-/* Match table for of_platform binding */
-static struct of_device_id keystone_qmss_of_match[] = {
- { .compatible = "ti,keystone-navigator-qmss", },
- {},
-};
-MODULE_DEVICE_TABLE(of, keystone_qmss_of_match);
-
static struct platform_driver keystone_qmss_driver = {
.probe = knav_queue_probe,
.remove = knav_queue_remove,
--
1.9.1
^ 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