Linux RDMA and InfiniBand development
 help / color / mirror / Atom feed
* Re: [PATCH v15 00/17] arm64: untag user pointers passed to the kernel
From: Kees Cook @ 2019-06-02  5:06 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Mark Rutland, kvm, Szabolcs Nagy, Will Deacon, dri-devel,
	Linux Memory Management List, Khalid Aziz,
	open list:KERNEL SELFTEST FRAMEWORK, Vincenzo Frascino,
	Jacob Bramley, Leon Romanovsky, linux-rdma, amd-gfx,
	Dmitry Vyukov, Dave Martin, Evgenii Stepanov, linux-media,
	Kevin Brodsky, Ruben Ayrapetyan, Andrey Konovalov,
	Ramana Radhakrishnan, Alex Williamson, Yishai
In-Reply-To: <20190528170244.GF32006@arrakis.emea.arm.com>

On Tue, May 28, 2019 at 06:02:45PM +0100, Catalin Marinas wrote:
> On Thu, May 23, 2019 at 02:31:16PM -0700, Kees Cook wrote:
> > syzkaller already attempts to randomly inject non-canonical and
> > 0xFFFF....FFFF addresses for user pointers in syscalls in an effort to
> > find bugs like CVE-2017-5123 where waitid() via unchecked put_user() was
> > able to write directly to kernel memory[1].
> > 
> > It seems that using TBI by default and not allowing a switch back to
> > "normal" ABI without a reboot actually means that userspace cannot inject
> > kernel pointers into syscalls any more, since they'll get universally
> > stripped now. Is my understanding correct, here? i.e. exploiting
> > CVE-2017-5123 would be impossible under TBI?
> > 
> > If so, then I think we should commit to the TBI ABI and have a boot
> > flag to disable it, but NOT have a process flag, as that would allow
> > attackers to bypass the masking. The only flag should be "TBI or MTE".
> > 
> > If so, can I get top byte masking for other architectures too? Like,
> > just to strip high bits off userspace addresses? ;)
> 
> Just for fun, hack/attempt at your idea which should not interfere with
> TBI. Only briefly tested on arm64 (and the s390 __TYPE_IS_PTR macro is
> pretty weird ;)):

OMG, this is amazing and bonkers. I love it.

> --------------------------8<---------------------------------
> diff --git a/arch/s390/include/asm/compat.h b/arch/s390/include/asm/compat.h
> index 63b46e30b2c3..338455a74eff 100644
> --- a/arch/s390/include/asm/compat.h
> +++ b/arch/s390/include/asm/compat.h
> @@ -11,9 +11,6 @@
>  
>  #include <asm-generic/compat.h>
>  
> -#define __TYPE_IS_PTR(t) (!__builtin_types_compatible_p( \
> -				typeof(0?(__force t)0:0ULL), u64))
> -
>  #define __SC_DELOUSE(t,v) ({ \
>  	BUILD_BUG_ON(sizeof(t) > 4 && !__TYPE_IS_PTR(t)); \
>  	(__force t)(__TYPE_IS_PTR(t) ? ((v) & 0x7fffffff) : (v)); \
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index e2870fe1be5b..b1b9fe8502da 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -119,8 +119,15 @@ struct io_uring_params;
>  #define __TYPE_IS_L(t)	(__TYPE_AS(t, 0L))
>  #define __TYPE_IS_UL(t)	(__TYPE_AS(t, 0UL))
>  #define __TYPE_IS_LL(t) (__TYPE_AS(t, 0LL) || __TYPE_AS(t, 0ULL))
> +#define __TYPE_IS_PTR(t) (!__builtin_types_compatible_p(typeof(0 ? (__force t)0 : 0ULL), u64))
>  #define __SC_LONG(t, a) __typeof(__builtin_choose_expr(__TYPE_IS_LL(t), 0LL, 0L)) a
> +#ifdef CONFIG_64BIT
> +#define __SC_CAST(t, a)	(__TYPE_IS_PTR(t) \
> +				? (__force t) ((__u64)a & ~(1UL << 55)) \
> +				: (__force t) a)
> +#else
>  #define __SC_CAST(t, a)	(__force t) a
> +#endif
>  #define __SC_ARGS(t, a)	a
>  #define __SC_TEST(t, a) (void)BUILD_BUG_ON_ZERO(!__TYPE_IS_LL(t) && sizeof(t) > sizeof(long))

I'm laughing, I'm crying. Now I have to go look at the disassembly to
see how this actually looks. (I mean, it _does_ solve my specific case
of the waitid() flaw, but wouldn't help with pointers deeper in structs,
etc, though TBI does, I think still help with that. I have to catch back
up on the thread...) Anyway, if it's not too expensive it'd block
reachability for those kinds of flaws.

I wonder what my chances are of actually getting this landed? :)
(Though, I guess I need to find a "VA width" macro instead of a raw 55.)

Thanks for thinking of __SC_CAST() and __TYPE_IS_PTR() together. Really
made my day. :)

-- 
Kees Cook
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* [PATCH 1/1] net: rds: add per rds connection cache statistics
From: Zhu Yanjun @ 2019-06-01  7:54 UTC (permalink / raw)
  To: santosh.shilimkar, davem, netdev, linux-rdma, rds-devel

The variable cache_allocs is to indicate how many frags (KiB) are in one
rds connection frag cache.
The command "rds-info -Iv" will output the rds connection cache
statistics as below:
"
RDS IB Connections:
      LocalAddr RemoteAddr Tos SL  LocalDev            RemoteDev
      1.1.1.14 1.1.1.14   58 255  fe80::2:c903:a:7a31 fe80::2:c903:a:7a31
      send_wr=256, recv_wr=1024, send_sge=8, rdma_mr_max=4096,
      rdma_mr_size=257, cache_allocs=12
"
This means that there are about 12KiB frag in this rds connection frag
 cache.

Tested-by: RDS CI <rdsci_oslo@no.oracle.com>
Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
---
 include/uapi/linux/rds.h | 2 ++
 net/rds/ib.c             | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/include/uapi/linux/rds.h b/include/uapi/linux/rds.h
index 5d0f76c..fd6b5f6 100644
--- a/include/uapi/linux/rds.h
+++ b/include/uapi/linux/rds.h
@@ -250,6 +250,7 @@ struct rds_info_rdma_connection {
 	__u32		rdma_mr_max;
 	__u32		rdma_mr_size;
 	__u8		tos;
+	__u32		cache_allocs;
 };
 
 struct rds6_info_rdma_connection {
@@ -264,6 +265,7 @@ struct rds6_info_rdma_connection {
 	__u32		rdma_mr_max;
 	__u32		rdma_mr_size;
 	__u8		tos;
+	__u32		cache_allocs;
 };
 
 /* RDS message Receive Path Latency points */
diff --git a/net/rds/ib.c b/net/rds/ib.c
index 2da9b75..f9baf2d 100644
--- a/net/rds/ib.c
+++ b/net/rds/ib.c
@@ -318,6 +318,7 @@ static int rds_ib_conn_info_visitor(struct rds_connection *conn,
 		iinfo->max_recv_wr = ic->i_recv_ring.w_nr;
 		iinfo->max_send_sge = rds_ibdev->max_sge;
 		rds_ib_get_mr_info(rds_ibdev, iinfo);
+		iinfo->cache_allocs = atomic_read(&ic->i_cache_allocs);
 	}
 	return 1;
 }
@@ -351,6 +352,7 @@ static int rds6_ib_conn_info_visitor(struct rds_connection *conn,
 		iinfo6->max_recv_wr = ic->i_recv_ring.w_nr;
 		iinfo6->max_send_sge = rds_ibdev->max_sge;
 		rds6_ib_get_mr_info(rds_ibdev, iinfo6);
+		iinfo6->cache_allocs = atomic_read(&ic->i_cache_allocs);
 	}
 	return 1;
 }
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH iproute2-next v1 2/4] rdma: Add man pages for rdma system commands
From: David Ahern @ 2019-05-31 22:13 UTC (permalink / raw)
  To: Parav Pandit; +Cc: netdev, linux-rdma, stephen, leonro
In-Reply-To: <20190531031117.60984-3-parav@mellanox.com>

On 5/30/19 9:11 PM, Parav Pandit wrote:
> +
> +.SH SEE ALSO
> +.BR rdma (8),
> +.BR rdma-link (8),
> +.BR rdma-resource (8),
> +.BR network_namespaces(7),
> +.BR namespaces(7),

Added the missing space before (7) and applied all of them to iproute2-next.

^ permalink raw reply

* Re: [PATCH, RESEND] RDMA/srp: Accept again source addresses that do not have a port number
From: Bart Van Assche @ 2019-05-31 19:33 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Leon Romanovsky, Doug Ledford, linux-rdma, Laurence Oberman,
	stable
In-Reply-To: <20190530184437.GA29836@ziepe.ca>

On 5/30/19 11:44 AM, Jason Gunthorpe wrote:
> On Wed, May 29, 2019 at 09:38:31AM -0700, Bart Van Assche wrote:
>> The function srp_parse_in() is used both for parsing source address
>> specifications and for target address specifications. Target addresses
>> must have a port number. Having to specify a port number for source
>> addresses is inconvenient. Make sure that srp_parse_in() supports again
>> parsing addresses with no port number.
>>
>> Cc: Laurence Oberman <loberman@redhat.com>
>> Cc: <stable@vger.kernel.org>
>> Fixes: c62adb7def71 ("IB/srp: Fix IPv6 address parsing") # v4.17.
>> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
>> ---
>>   drivers/infiniband/ulp/srp/ib_srp.c | 21 +++++++++++++++------
>>   1 file changed, 15 insertions(+), 6 deletions(-)
> 
> Bart, do you want this applied now, or are we still waiting for
> Laurence?

Hi Jason,

Since Laurence has not replied to your e-mail please go ahead and apply 
this patch.

Thanks,

Bart.

^ permalink raw reply

* Re: [PATCH mlx5-next 0/6] Mellanox, mlx5-next minor updates 2019-05-29
From: Saeed Mahameed @ 2019-05-31 19:29 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org
In-Reply-To: <20190529224949.18194-1-saeedm@mellanox.com>

On Wed, 2019-05-29 at 22:50 +0000, Saeed Mahameed wrote:
> Hi All,
> 
> This series provides some low level updates for mlx5 driver needed
> for
> both rdma and netdev trees.
> 
> Eli adds termination flow steering table bits and hardware
> definitions.
> 
> Moshe introduces the core dump HW access registers definitions.
> 
> Parav refactors and cleans-up VF representors functions handlers.
> 
> Vu renames host_params bits to function_changed bits and add the
> support for eswitch functions change event in the eswitch general
> case.
> (for both legacy and switchdev modes).
> 
> In case of no objection this series will be applied to mlx5-next
> branch.
> 

Series applied to mlx5-next branch.

^ permalink raw reply

* Re: [PATCH v15 00/17] arm64: untag user pointers passed to the kernel
From: Catalin Marinas @ 2019-05-31 16:46 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Kees Cook, Evgenii Stepanov, Linux ARM,
	Linux Memory Management List, LKML, amd-gfx, dri-devel,
	linux-rdma, linux-media, kvm, open list:KERNEL SELFTEST FRAMEWORK,
	Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
	Greg Kroah-Hartman, Yishai Hadas, Felix Kuehling,
	Alexander Deucher
In-Reply-To: <CAAeHK+zRDD7ZPPUA9cpwHOdgTRrJLWAby8Wg9oPgmhqMpHwvFw@mail.gmail.com>

On Fri, May 31, 2019 at 06:24:06PM +0200, Andrey Konovalov wrote:
> On Fri, May 31, 2019 at 6:20 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > On Fri, May 31, 2019 at 04:29:10PM +0200, Andrey Konovalov wrote:
> > > On Thu, May 30, 2019 at 7:15 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > > On Tue, May 28, 2019 at 04:14:45PM +0200, Andrey Konovalov wrote:
> > > > > Thanks for a lot of valuable input! I've read through all the replies
> > > > > and got somewhat lost. What are the changes I need to do to this
> > > > > series?
> > > > >
> > > > > 1. Should I move untagging for memory syscalls back to the generic
> > > > > code so other arches would make use of it as well, or should I keep
> > > > > the arm64 specific memory syscalls wrappers and address the comments
> > > > > on that patch?
> > > >
> > > > Keep them generic again but make sure we get agreement with Khalid on
> > > > the actual ABI implications for sparc.
> > >
> > > OK, will do. I find it hard to understand what the ABI implications
> > > are. I'll post the next version without untagging in brk, mmap,
> > > munmap, mremap (for new_address), mmap_pgoff, remap_file_pages, shmat
> > > and shmdt.
> >
> > It's more about not relaxing the ABI to accept non-zero top-byte unless
> > we have a use-case for it. For mmap() etc., I don't think that's needed
> > but if you think otherwise, please raise it.
> >
> > > > > 2. Should I make untagging opt-in and controlled by a command line argument?
> > > >
> > > > Opt-in, yes, but per task rather than kernel command line option.
> > > > prctl() is a possibility of opting in.
> > >
> > > OK. Should I store a flag somewhere in task_struct? Should it be
> > > inheritable on clone?
> >
> > A TIF flag would do but I'd say leave it out for now (default opted in)
> > until we figure out the best way to do this (can be a patch on top of
> > this series).
> 
> You mean leave the whole opt-in/prctl part out? So the only change
> would be to move untagging for memory syscalls into generic code?

Yes (or just wait until next week to see if the discussion settles
down).

-- 
Catalin

^ permalink raw reply

* Re: [PATCH v15 00/17] arm64: untag user pointers passed to the kernel
From: Andrey Konovalov @ 2019-05-31 16:24 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Kees Cook, Evgenii Stepanov, Linux ARM,
	Linux Memory Management List, LKML, amd-gfx, dri-devel,
	linux-rdma, linux-media, kvm, open list:KERNEL SELFTEST FRAMEWORK,
	Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
	Greg Kroah-Hartman, Yishai Hadas, Felix Kuehling,
	Alexander Deucher
In-Reply-To: <20190531161954.GA3568@arrakis.emea.arm.com>

On Fri, May 31, 2019 at 6:20 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Fri, May 31, 2019 at 04:29:10PM +0200, Andrey Konovalov wrote:
> > On Thu, May 30, 2019 at 7:15 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > On Tue, May 28, 2019 at 04:14:45PM +0200, Andrey Konovalov wrote:
> > > > Thanks for a lot of valuable input! I've read through all the replies
> > > > and got somewhat lost. What are the changes I need to do to this
> > > > series?
> > > >
> > > > 1. Should I move untagging for memory syscalls back to the generic
> > > > code so other arches would make use of it as well, or should I keep
> > > > the arm64 specific memory syscalls wrappers and address the comments
> > > > on that patch?
> > >
> > > Keep them generic again but make sure we get agreement with Khalid on
> > > the actual ABI implications for sparc.
> >
> > OK, will do. I find it hard to understand what the ABI implications
> > are. I'll post the next version without untagging in brk, mmap,
> > munmap, mremap (for new_address), mmap_pgoff, remap_file_pages, shmat
> > and shmdt.
>
> It's more about not relaxing the ABI to accept non-zero top-byte unless
> we have a use-case for it. For mmap() etc., I don't think that's needed
> but if you think otherwise, please raise it.
>
> > > > 2. Should I make untagging opt-in and controlled by a command line argument?
> > >
> > > Opt-in, yes, but per task rather than kernel command line option.
> > > prctl() is a possibility of opting in.
> >
> > OK. Should I store a flag somewhere in task_struct? Should it be
> > inheritable on clone?
>
> A TIF flag would do but I'd say leave it out for now (default opted in)
> until we figure out the best way to do this (can be a patch on top of
> this series).

You mean leave the whole opt-in/prctl part out? So the only change
would be to move untagging for memory syscalls into generic code?

>
> Thanks.
>
> --
> Catalin

^ permalink raw reply

* Re: [PATCH v15 17/17] selftests, arm64: add a selftest for passing tagged pointers to kernel
From: Catalin Marinas @ 2019-05-31 16:22 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Vincenzo Frascino, Linux ARM, Linux Memory Management List, LKML,
	amd-gfx, dri-devel, linux-rdma, linux-media, kvm,
	open list:KERNEL SELFTEST FRAMEWORK, Will Deacon, Mark Rutland,
	Andrew Morton, Greg Kroah-Hartman, Kees Cook, Yishai Hadas,
	Felix Kuehling, Alexander Deucher,
	Christian Koenig <Christian.Koenig@
In-Reply-To: <CAAeHK+wUerHQOV2PuaTwTxcCucZHZodLwg48228SB+ymxEqT2A@mail.gmail.com>

On Fri, May 31, 2019 at 04:21:48PM +0200, Andrey Konovalov wrote:
> On Wed, May 22, 2019 at 4:16 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > On Mon, May 06, 2019 at 06:31:03PM +0200, Andrey Konovalov wrote:
> > > This patch is a part of a series that extends arm64 kernel ABI to allow to
> > > pass tagged user pointers (with the top byte set to something else other
> > > than 0x00) as syscall arguments.
> > >
> > > This patch adds a simple test, that calls the uname syscall with a
> > > tagged user pointer as an argument. Without the kernel accepting tagged
> > > user pointers the test fails with EFAULT.
> >
> > That's probably sufficient for a simple example. Something we could add
> > to Documentation maybe is a small library that can be LD_PRELOAD'ed so
> > that you can run a lot more tests like LTP.
> 
> Should I add this into this series, or should this go into Vincenzo's patchset?

If you can tweak the selftest Makefile to build a library and force it
with LD_PRELOAD, you can keep it with this patch. It would be easier to
extend to other syscall tests, signal handling etc.

-- 
Catalin

^ permalink raw reply

* Re: [PATCH v15 00/17] arm64: untag user pointers passed to the kernel
From: Catalin Marinas @ 2019-05-31 16:19 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Mark Rutland, kvm, Szabolcs Nagy, Will Deacon, dri-devel,
	Linux Memory Management List, Khalid Aziz,
	open list:KERNEL SELFTEST FRAMEWORK, Vincenzo Frascino,
	Jacob Bramley, Leon Romanovsky, linux-rdma, amd-gfx,
	Dmitry Vyukov, Dave Martin, Evgenii Stepanov, linux-media,
	Kevin Brodsky, Kees Cook, Ruben Ayrapetyan, Ramana Radhakrishnan,
	Alex Williamson, Yishai Hadas <yis>
In-Reply-To: <CAAeHK+y34+SNz3Vf+_378bOxrPaj_3GaLCeC2Y2rHAczuaSz1A@mail.gmail.com>

On Fri, May 31, 2019 at 04:29:10PM +0200, Andrey Konovalov wrote:
> On Thu, May 30, 2019 at 7:15 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > On Tue, May 28, 2019 at 04:14:45PM +0200, Andrey Konovalov wrote:
> > > Thanks for a lot of valuable input! I've read through all the replies
> > > and got somewhat lost. What are the changes I need to do to this
> > > series?
> > >
> > > 1. Should I move untagging for memory syscalls back to the generic
> > > code so other arches would make use of it as well, or should I keep
> > > the arm64 specific memory syscalls wrappers and address the comments
> > > on that patch?
> >
> > Keep them generic again but make sure we get agreement with Khalid on
> > the actual ABI implications for sparc.
> 
> OK, will do. I find it hard to understand what the ABI implications
> are. I'll post the next version without untagging in brk, mmap,
> munmap, mremap (for new_address), mmap_pgoff, remap_file_pages, shmat
> and shmdt.

It's more about not relaxing the ABI to accept non-zero top-byte unless
we have a use-case for it. For mmap() etc., I don't think that's needed
but if you think otherwise, please raise it.

> > > 2. Should I make untagging opt-in and controlled by a command line argument?
> >
> > Opt-in, yes, but per task rather than kernel command line option.
> > prctl() is a possibility of opting in.
> 
> OK. Should I store a flag somewhere in task_struct? Should it be
> inheritable on clone?

A TIF flag would do but I'd say leave it out for now (default opted in)
until we figure out the best way to do this (can be a patch on top of
this series).

Thanks.

-- 
Catalin

^ permalink raw reply

* Re: [PATCH v15 00/17] arm64: untag user pointers passed to the kernel
From: Andrey Konovalov @ 2019-05-31 14:29 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Kees Cook, Evgenii Stepanov, Linux ARM,
	Linux Memory Management List, LKML, amd-gfx, dri-devel,
	linux-rdma, linux-media, kvm, open list:KERNEL SELFTEST FRAMEWORK,
	Vincenzo Frascino, Will Deacon, Mark Rutland, Andrew Morton,
	Greg Kroah-Hartman, Yishai Hadas, Felix Kuehling,
	Alexander Deucher
In-Reply-To: <20190530171540.GD35418@arrakis.emea.arm.com>

On Thu, May 30, 2019 at 7:15 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Tue, May 28, 2019 at 04:14:45PM +0200, Andrey Konovalov wrote:
> > Thanks for a lot of valuable input! I've read through all the replies
> > and got somewhat lost. What are the changes I need to do to this
> > series?
> >
> > 1. Should I move untagging for memory syscalls back to the generic
> > code so other arches would make use of it as well, or should I keep
> > the arm64 specific memory syscalls wrappers and address the comments
> > on that patch?
>
> Keep them generic again but make sure we get agreement with Khalid on
> the actual ABI implications for sparc.

OK, will do. I find it hard to understand what the ABI implications
are. I'll post the next version without untagging in brk, mmap,
munmap, mremap (for new_address), mmap_pgoff, remap_file_pages, shmat
and shmdt.

>
> > 2. Should I make untagging opt-in and controlled by a command line argument?
>
> Opt-in, yes, but per task rather than kernel command line option.
> prctl() is a possibility of opting in.

OK. Should I store a flag somewhere in task_struct? Should it be
inheritable on clone?

>
> > 3. Should I "add Documentation/core-api/user-addresses.rst to describe
> > proper care and handling of user space pointers with untagged_addr(),
> > with examples based on all the cases seen so far in this series"?
> > Which examples specifically should it cover?
>
> I think we can leave 3 for now as not too urgent. What I'd like is for
> Vincenzo's TBI user ABI document to go into a more common place since we
> can expand it to cover both sparc and arm64. We'd need an arm64-specific
> doc as well for things like prctl() and later MTE that sparc may support
> differently.

OK.

>
> --
> Catalin

^ permalink raw reply

* Re: [PATCH v15 17/17] selftests, arm64: add a selftest for passing tagged pointers to kernel
From: Andrey Konovalov @ 2019-05-31 14:21 UTC (permalink / raw)
  To: Catalin Marinas, Vincenzo Frascino
  Cc: Linux ARM, Linux Memory Management List, LKML, amd-gfx, dri-devel,
	linux-rdma, linux-media, kvm, open list:KERNEL SELFTEST FRAMEWORK,
	Will Deacon, Mark Rutland, Andrew Morton, Greg Kroah-Hartman,
	Kees Cook, Yishai Hadas, Felix Kuehling, Alexander Deucher,
	Christian Koenig, Mauro Carvalho Chehab
In-Reply-To: <20190522141612.GA28122@arrakis.emea.arm.com>

On Wed, May 22, 2019 at 4:16 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Mon, May 06, 2019 at 06:31:03PM +0200, Andrey Konovalov wrote:
> > This patch is a part of a series that extends arm64 kernel ABI to allow to
> > pass tagged user pointers (with the top byte set to something else other
> > than 0x00) as syscall arguments.
> >
> > This patch adds a simple test, that calls the uname syscall with a
> > tagged user pointer as an argument. Without the kernel accepting tagged
> > user pointers the test fails with EFAULT.
>
> That's probably sufficient for a simple example. Something we could add
> to Documentation maybe is a small library that can be LD_PRELOAD'ed so
> that you can run a lot more tests like LTP.

Should I add this into this series, or should this go into Vincenzo's patchset?

>
> We could add this to selftests but I think it's too glibc specific.
>
> --------------------8<------------------------------------
> #include <stdlib.h>
>
> #define TAG_SHIFT       (56)
> #define TAG_MASK        (0xffUL << TAG_SHIFT)
>
> void *__libc_malloc(size_t size);
> void __libc_free(void *ptr);
> void *__libc_realloc(void *ptr, size_t size);
> void *__libc_calloc(size_t nmemb, size_t size);
>
> static void *tag_ptr(void *ptr)
> {
>         unsigned long tag = rand() & 0xff;
>         if (!ptr)
>                 return ptr;
>         return (void *)((unsigned long)ptr | (tag << TAG_SHIFT));
> }
>
> static void *untag_ptr(void *ptr)
> {
>         return (void *)((unsigned long)ptr & ~TAG_MASK);
> }
>
> void *malloc(size_t size)
> {
>         return tag_ptr(__libc_malloc(size));
> }
>
> void free(void *ptr)
> {
>         __libc_free(untag_ptr(ptr));
> }
>
> void *realloc(void *ptr, size_t size)
> {
>         return tag_ptr(__libc_realloc(untag_ptr(ptr), size));
> }
>
> void *calloc(size_t nmemb, size_t size)
> {
>         return tag_ptr(__libc_calloc(nmemb, size));
> }

^ permalink raw reply

* [PATCH 2/2][next] RDMA/hns: fix inverted logic of readl read and shift
From: Colin King @ 2019-05-31  9:21 UTC (permalink / raw)
  To: Lijun Ou, Wei Hu, Doug Ledford, Jason Gunthorpe, linux-rdma
  Cc: kernel-janitors, linux-kernel
In-Reply-To: <20190531092101.28772-1-colin.king@canonical.com>

From: Colin Ian King <colin.king@canonical.com>

A previous change incorrectly changed the inverted logic and logically
negated the readl rather than the shifted readl result. Fix this by
adding in missing parentheses around the expression that needs to be
logically negated.

Addresses-Coverity: ("Logically dead code")
Fixes: 669cefb654cb ("RDMA/hns: Remove jiffies operation in disable interrupt context")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/infiniband/hw/hns/hns_roce_hem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hem.c b/drivers/infiniband/hw/hns/hns_roce_hem.c
index b3641aeff27a..a8e9329cbf4e 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hem.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hem.c
@@ -378,7 +378,7 @@ static int hns_roce_set_hem(struct hns_roce_dev *hr_dev,
 
 		end = HW_SYNC_TIMEOUT_MSECS;
 		while (end > 0) {
-			if (!readl(bt_cmd) >> BT_CMD_SYNC_SHIFT)
+			if (!(readl(bt_cmd) >> BT_CMD_SYNC_SHIFT))
 				break;
 
 			mdelay(HW_SYNC_SLEEP_TIME_INTERVAL);
-- 
2.20.1

^ permalink raw reply related

* [PATCH 1/2][next] RDMA/hns: fix comparison of unsigned long variable 'end' with less than zero
From: Colin King @ 2019-05-31  9:21 UTC (permalink / raw)
  To: Lijun Ou, Wei Hu, Doug Ledford, Jason Gunthorpe, linux-rdma
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

Currently the comparison of end with less than zero is always false
because end is an unsigned long.  Also, replace checks of end with
non-zero with end > 0 as it is possible that the #defined decrement
may be changed in the future causing end to step over zero and go
negative.

The initialization of end with 0 is also redundant as this value is
never read and is later set to HW_SYNC_TIMEOUT_MSECS, so fix this by
initializing it with this value to begin with.

Addresses-Coverity: ("Unsigned compared against 0")
Fixes: 669cefb654cb ("RDMA/hns: Remove jiffies operation in disable interrupt context")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/infiniband/hw/hns/hns_roce_hem.c   |  4 ++--
 drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 12 ++++++------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hem.c b/drivers/infiniband/hw/hns/hns_roce_hem.c
index 157c84a1f55f..b3641aeff27a 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hem.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hem.c
@@ -326,7 +326,7 @@ static int hns_roce_set_hem(struct hns_roce_dev *hr_dev,
 {
 	spinlock_t *lock = &hr_dev->bt_cmd_lock;
 	struct device *dev = hr_dev->dev;
-	unsigned long end = 0;
+	long end;
 	unsigned long flags;
 	struct hns_roce_hem_iter iter;
 	void __iomem *bt_cmd;
@@ -377,7 +377,7 @@ static int hns_roce_set_hem(struct hns_roce_dev *hr_dev,
 		bt_cmd = hr_dev->reg_base + ROCEE_BT_CMD_H_REG;
 
 		end = HW_SYNC_TIMEOUT_MSECS;
-		while (end) {
+		while (end > 0) {
 			if (!readl(bt_cmd) >> BT_CMD_SYNC_SHIFT)
 				break;
 
diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
index 1fc77b1f2c6d..e13fea71bcb8 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
@@ -966,7 +966,7 @@ static int hns_roce_v1_recreate_lp_qp(struct hns_roce_dev *hr_dev)
 	struct hns_roce_free_mr *free_mr;
 	struct hns_roce_v1_priv *priv;
 	struct completion comp;
-	unsigned long end = HNS_ROCE_V1_RECREATE_LP_QP_TIMEOUT_MSECS;
+	long end = HNS_ROCE_V1_RECREATE_LP_QP_TIMEOUT_MSECS;
 
 	priv = (struct hns_roce_v1_priv *)hr_dev->priv;
 	free_mr = &priv->free_mr;
@@ -986,7 +986,7 @@ static int hns_roce_v1_recreate_lp_qp(struct hns_roce_dev *hr_dev)
 
 	queue_work(free_mr->free_mr_wq, &(lp_qp_work->work));
 
-	while (end) {
+	while (end > 0) {
 		if (try_wait_for_completion(&comp))
 			return 0;
 		msleep(HNS_ROCE_V1_RECREATE_LP_QP_WAIT_VALUE);
@@ -1104,7 +1104,7 @@ static int hns_roce_v1_dereg_mr(struct hns_roce_dev *hr_dev,
 	struct hns_roce_free_mr *free_mr;
 	struct hns_roce_v1_priv *priv;
 	struct completion comp;
-	unsigned long end = HNS_ROCE_V1_FREE_MR_TIMEOUT_MSECS;
+	long end = HNS_ROCE_V1_FREE_MR_TIMEOUT_MSECS;
 	unsigned long start = jiffies;
 	int npages;
 	int ret = 0;
@@ -1134,7 +1134,7 @@ static int hns_roce_v1_dereg_mr(struct hns_roce_dev *hr_dev,
 
 	queue_work(free_mr->free_mr_wq, &(mr_work->work));
 
-	while (end) {
+	while (end > 0) {
 		if (try_wait_for_completion(&comp))
 			goto free_mr;
 		msleep(HNS_ROCE_V1_FREE_MR_WAIT_VALUE);
@@ -2425,7 +2425,8 @@ static int hns_roce_v1_clear_hem(struct hns_roce_dev *hr_dev,
 {
 	struct device *dev = &hr_dev->pdev->dev;
 	struct hns_roce_v1_priv *priv;
-	unsigned long end = 0, flags = 0;
+	unsigned long flags = 0;
+	long end = HW_SYNC_TIMEOUT_MSECS;
 	__le32 bt_cmd_val[2] = {0};
 	void __iomem *bt_cmd;
 	u64 bt_ba = 0;
@@ -2463,7 +2464,6 @@ static int hns_roce_v1_clear_hem(struct hns_roce_dev *hr_dev,
 
 	bt_cmd = hr_dev->reg_base + ROCEE_BT_CMD_H_REG;
 
-	end = HW_SYNC_TIMEOUT_MSECS;
 	while (1) {
 		if (readl(bt_cmd) >> BT_CMD_SYNC_SHIFT) {
 			if (end < 0) {
-- 
2.20.1

^ permalink raw reply related

* [PATCH iproute2-next v1 4/4] rdma: Add man page for rdma dev set netns command
From: Parav Pandit @ 2019-05-31  3:11 UTC (permalink / raw)
  To: dsahern; +Cc: netdev, linux-rdma, stephen, leonro, parav
In-Reply-To: <20190531031117.60984-1-parav@mellanox.com>

Add man page to describe additional set netns command
for rdma device.

Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
Signed-off-by: Parav Pandit <parav@mellanox.com>
---
 man/man8/rdma-dev.8 | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/man/man8/rdma-dev.8 b/man/man8/rdma-dev.8
index 069f4717..38e34b3b 100644
--- a/man/man8/rdma-dev.8
+++ b/man/man8/rdma-dev.8
@@ -28,13 +28,19 @@ rdma-dev \- RDMA device configuration
 .BR name
 .BR NEWNAME
 
+.ti -8
+.B rdma dev set
+.RI "[ " DEV " ]"
+.BR netns
+.BR NSNAME
+
 .ti -8
 .B rdma dev help
 
 .SH "DESCRIPTION"
-.SS rdma dev set - rename rdma device
+.SS rdma dev set - rename RDMA device or set network namespace
 
-.SS rdma dev show - display rdma device attributes
+.SS rdma dev show - display RDMA device attributes
 
 .PP
 .I "DEV"
@@ -58,11 +64,19 @@ rdma dev set mlx5_3 name rdma_0
 Renames the mlx5_3 device to rdma_0.
 .RE
 .PP
+rdma dev set mlx5_3 netns foo
+.RS 4
+Changes the network namespace of RDMA device to foo where foo is
+previously created using iproute2 ip command.
+.RE
+.PP
 
 .SH SEE ALSO
+.BR ip (8),
 .BR rdma (8),
 .BR rdma-link (8),
 .BR rdma-resource (8),
+.BR rdma-system (8),
 .br
 
 .SH AUTHOR
-- 
2.19.2

^ permalink raw reply related

* [PATCH iproute2-next v1 3/4] rdma: Add an option to set net namespace of rdma device
From: Parav Pandit @ 2019-05-31  3:11 UTC (permalink / raw)
  To: dsahern; +Cc: netdev, linux-rdma, stephen, leonro, parav
In-Reply-To: <20190531031117.60984-1-parav@mellanox.com>

Enrich rdmatool with an option to set network namespace of RDMA
device. After successful execution of it, rdma device will
be accessible only in assigned network namespace.

rdma tool command examples and output.

First set netns mode to exclusive.

$ rdma system set netns exclusive

Now create network namespace and assign RDMA device to this
network namespace.

$ ip netns add foo
$ rdma dev set mlx5_1 netns foo

Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
Signed-off-by: Parav Pandit <parav@mellanox.com>
---
 rdma/dev.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/rdma/dev.c b/rdma/dev.c
index 90483622..d28bf6b3 100644
--- a/rdma/dev.c
+++ b/rdma/dev.c
@@ -4,12 +4,14 @@
  * Authors:     Leon Romanovsky <leonro@mellanox.com>
  */
 
+#include <fcntl.h>
 #include "rdma.h"
 
 static int dev_help(struct rd *rd)
 {
 	pr_out("Usage: %s dev show [DEV]\n", rd->filename);
 	pr_out("       %s dev set [DEV] name DEVNAME\n", rd->filename);
+	pr_out("       %s dev set [DEV] netns NSNAME\n", rd->filename);
 	return 0;
 }
 
@@ -272,11 +274,46 @@ static int dev_set_name(struct rd *rd)
 	return rd_sendrecv_msg(rd, seq);
 }
 
+static int dev_set_netns(struct rd *rd)
+{
+	char *netns_path;
+	uint32_t seq;
+	int netns;
+	int ret;
+
+	if (rd_no_arg(rd)) {
+		pr_err("Please provide device name.\n");
+		return -EINVAL;
+	}
+
+	if (asprintf(&netns_path, "%s/%s", NETNS_RUN_DIR, rd_argv(rd)) < 0)
+		return -ENOMEM;
+
+	netns = open(netns_path, O_RDONLY | O_CLOEXEC);
+	if (netns < 0) {
+		fprintf(stderr, "Cannot open network namespace \"%s\": %s\n",
+			rd_argv(rd), strerror(errno));
+		ret = -EINVAL;
+		goto done;
+	}
+
+	rd_prepare_msg(rd, RDMA_NLDEV_CMD_SET,
+		       &seq, (NLM_F_REQUEST | NLM_F_ACK));
+	mnl_attr_put_u32(rd->nlh, RDMA_NLDEV_ATTR_DEV_INDEX, rd->dev_idx);
+	mnl_attr_put_u32(rd->nlh, RDMA_NLDEV_NET_NS_FD, netns);
+	ret = rd_sendrecv_msg(rd, seq);
+	close(netns);
+done:
+	free(netns_path);
+	return ret;
+}
+
 static int dev_one_set(struct rd *rd)
 {
 	const struct rd_cmd cmds[] = {
 		{ NULL,		dev_help},
 		{ "name",	dev_set_name},
+		{ "netns",	dev_set_netns},
 		{ 0 }
 	};
 
-- 
2.19.2

^ permalink raw reply related

* [PATCH iproute2-next v1 2/4] rdma: Add man pages for rdma system commands
From: Parav Pandit @ 2019-05-31  3:11 UTC (permalink / raw)
  To: dsahern; +Cc: netdev, linux-rdma, stephen, leonro, parav
In-Reply-To: <20190531031117.60984-1-parav@mellanox.com>

Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
Signed-off-by: Parav Pandit <parav@mellanox.com>
---
 man/man8/rdma-system.8 | 82 ++++++++++++++++++++++++++++++++++++++++++
 man/man8/rdma.8        |  7 +++-
 2 files changed, 88 insertions(+), 1 deletion(-)
 create mode 100644 man/man8/rdma-system.8

diff --git a/man/man8/rdma-system.8 b/man/man8/rdma-system.8
new file mode 100644
index 00000000..a6873b52
--- /dev/null
+++ b/man/man8/rdma-system.8
@@ -0,0 +1,82 @@
+.TH RDMA\-SYSTEM 8 "06 Jul 2017" "iproute2" "Linux"
+.SH NAME
+rdma-system \- RDMA subsystem configuration
+.SH SYNOPSIS
+.sp
+.ad l
+.in +8
+.ti -8
+.B rdma
+.RI "[ " OPTIONS " ]"
+.B sys
+.RI  " { " COMMAND " | "
+.BR help " }"
+.sp
+
+.ti -8
+.IR OPTIONS " := { "
+\fB\-V\fR[\fIersion\fR] |
+\fB\-d\fR[\fIetails\fR] }
+
+.ti -8
+.B rdma system show
+
+.ti -8
+.B rdma system set
+.BR netns
+.BR NEWMODE
+
+.ti -8
+.B rdma system help
+
+.SH "DESCRIPTION"
+.SS rdma system set - set RDMA subsystem network namespace mode
+
+.SS rdma system show - display RDMA subsystem network namespace mode
+
+.PP
+.I "NEWMODE"
+- specifies the RDMA subsystem mode. Either exclusive or shared.
+When user wants to assign dedicated RDMA device to a particular
+network namespace, exclusive mode should be set before creating
+any network namespace. If there are active network namespaces and if
+one or more RDMA devices exist, changing mode from shared to
+exclusive returns error code EBUSY.
+
+When RDMA subsystem is in shared mode, RDMA device is accessible in
+all network namespace. When RDMA device isolation among multiple
+network namespaces is not needed, shared mode can be used.
+
+It is preferred to not change the subsystem mode when there is active
+RDMA traffic running, even though it is supported.
+
+.SH "EXAMPLES"
+.PP
+rdma system show
+.RS 4
+Shows the state of RDMA subsystem network namespace mode on the system.
+.RE
+.PP
+rdma system set netns exclusive
+.RS 4
+Sets the RDMA subsystem in network namespace exclusive mode. In this mode RDMA devices
+are visible only in single network namespace.
+.RE
+.PP
+rdma system set netns shared
+.RS 4
+Sets the RDMA subsystem in network namespace shared mode. In this mode RDMA devices
+are shared among network namespaces.
+.RE
+.PP
+
+.SH SEE ALSO
+.BR rdma (8),
+.BR rdma-link (8),
+.BR rdma-resource (8),
+.BR network_namespaces(7),
+.BR namespaces(7),
+.br
+
+.SH AUTHOR
+Parav Pandit <parav@mellanox.com>
diff --git a/man/man8/rdma.8 b/man/man8/rdma.8
index b2b5aef8..3ae33987 100644
--- a/man/man8/rdma.8
+++ b/man/man8/rdma.8
@@ -19,7 +19,7 @@ rdma \- RDMA tool
 
 .ti -8
 .IR OBJECT " := { "
-.BR dev " | " link " }"
+.BR dev " | " link " | " system " }"
 .sp
 
 .ti -8
@@ -70,6 +70,10 @@ Generate JSON output.
 .B link
 - RDMA port related.
 
+.TP
+.B sys
+- RDMA subsystem related.
+
 .PP
 The names of all objects may be written in full or
 abbreviated form, for example
@@ -107,6 +111,7 @@ Exit status is 0 if command was successful or a positive integer upon failure.
 .BR rdma-dev (8),
 .BR rdma-link (8),
 .BR rdma-resource (8),
+.BR rdma-system (8),
 .br
 
 .SH REPORTING BUGS
-- 
2.19.2

^ permalink raw reply related

* [PATCH iproute2-next v1 1/4] rdma: Add an option to query,set net namespace sharing sys parameter
From: Parav Pandit @ 2019-05-31  3:11 UTC (permalink / raw)
  To: dsahern; +Cc: netdev, linux-rdma, stephen, leonro, parav
In-Reply-To: <20190531031117.60984-1-parav@mellanox.com>

Enrich rdmatool with an option to query rdma subsystem parameter
whether rdma devices are shared among multiple network namespaces
or exclusive to single network namespace.

rdma tool command examples and output.

$ rdma system show
netns shared

$ rdma system set netns exclusive

$ rdma system show
netns exclusive

Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
Signed-off-by: Parav Pandit <parav@mellanox.com>
---
 rdma/Makefile |   2 +-
 rdma/rdma.c   |   3 +-
 rdma/rdma.h   |   1 +
 rdma/sys.c    | 138 ++++++++++++++++++++++++++++++++++++++++++++++++++
 rdma/utils.c  |   1 +
 5 files changed, 143 insertions(+), 2 deletions(-)
 create mode 100644 rdma/sys.c

diff --git a/rdma/Makefile b/rdma/Makefile
index 6a424234..4847f27e 100644
--- a/rdma/Makefile
+++ b/rdma/Makefile
@@ -7,7 +7,7 @@ ifeq ($(HAVE_MNL),y)
 CFLAGS += -I./include/uapi/
 
 RDMA_OBJ = rdma.o utils.o dev.o link.o res.o res-pd.o res-mr.o res-cq.o \
-	   res-cmid.o res-qp.o
+	   res-cmid.o res-qp.o sys.o
 
 TARGETS += rdma
 endif
diff --git a/rdma/rdma.c b/rdma/rdma.c
index 676e03c2..e9f1b4bb 100644
--- a/rdma/rdma.c
+++ b/rdma/rdma.c
@@ -11,7 +11,7 @@ static void help(char *name)
 {
 	pr_out("Usage: %s [ OPTIONS ] OBJECT { COMMAND | help }\n"
 	       "       %s [ -f[orce] ] -b[atch] filename\n"
-	       "where  OBJECT := { dev | link | resource | help }\n"
+	       "where  OBJECT := { dev | link | resource | system | help }\n"
 	       "       OPTIONS := { -V[ersion] | -d[etails] | -j[son] | -p[retty]}\n", name, name);
 }
 
@@ -29,6 +29,7 @@ static int rd_cmd(struct rd *rd, int argc, char **argv)
 		{ "dev",	cmd_dev },
 		{ "link",	cmd_link },
 		{ "resource",	cmd_res },
+		{ "system",	cmd_sys },
 		{ 0 }
 	};
 
diff --git a/rdma/rdma.h b/rdma/rdma.h
index 9ed9e045..885a751e 100644
--- a/rdma/rdma.h
+++ b/rdma/rdma.h
@@ -93,6 +93,7 @@ char *rd_argv(struct rd *rd);
 int cmd_dev(struct rd *rd);
 int cmd_link(struct rd *rd);
 int cmd_res(struct rd *rd);
+int cmd_sys(struct rd *rd);
 int rd_exec_cmd(struct rd *rd, const struct rd_cmd *c, const char *str);
 int rd_exec_dev(struct rd *rd, int (*cb)(struct rd *rd));
 int rd_exec_require_dev(struct rd *rd, int (*cb)(struct rd *rd));
diff --git a/rdma/sys.c b/rdma/sys.c
new file mode 100644
index 00000000..cef39081
--- /dev/null
+++ b/rdma/sys.c
@@ -0,0 +1,138 @@
+// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
+/*
+ * sys.c	RDMA tool
+ */
+
+#include "rdma.h"
+
+static int sys_help(struct rd *rd)
+{
+	pr_out("Usage: %s system show [ netns ]\n", rd->filename);
+	pr_out("       %s system set netns { shared | exclusive }\n", rd->filename);
+	return 0;
+}
+
+static const char *netns_modes_str[] = {
+	"exclusive",
+	"shared",
+};
+
+static int sys_show_parse_cb(const struct nlmsghdr *nlh, void *data)
+{
+	struct nlattr *tb[RDMA_NLDEV_ATTR_MAX] = {};
+	struct rd *rd = data;
+
+	mnl_attr_parse(nlh, 0, rd_attr_cb, tb);
+
+	if (tb[RDMA_NLDEV_SYS_ATTR_NETNS_MODE]) {
+		const char *mode_str;
+		uint8_t netns_mode;
+
+		netns_mode =
+			mnl_attr_get_u8(tb[RDMA_NLDEV_SYS_ATTR_NETNS_MODE]);
+
+		if (netns_mode <= ARRAY_SIZE(netns_modes_str))
+			mode_str = netns_modes_str[netns_mode];
+		else
+			mode_str = "unknown";
+
+		if (rd->json_output)
+			jsonw_string_field(rd->jw, "netns", mode_str);
+		else
+			pr_out("netns %s\n", mode_str);
+	}
+	return MNL_CB_OK;
+}
+
+static int sys_show_no_args(struct rd *rd)
+{
+	uint32_t seq;
+	int ret;
+
+	rd_prepare_msg(rd, RDMA_NLDEV_CMD_SYS_GET,
+		       &seq, (NLM_F_REQUEST | NLM_F_ACK));
+	ret = rd_send_msg(rd);
+	if (ret)
+		return ret;
+
+	return rd_recv_msg(rd, sys_show_parse_cb, rd, seq);
+}
+
+static int sys_show(struct rd *rd)
+{
+	const struct rd_cmd cmds[] = {
+		{ NULL,		sys_show_no_args},
+		{ "netns",	sys_show_no_args},
+		{ 0 }
+	};
+
+	return rd_exec_cmd(rd, cmds, "parameter");
+}
+
+static int sys_set_netns_cmd(struct rd *rd, bool enable)
+{
+	uint32_t seq;
+
+	rd_prepare_msg(rd, RDMA_NLDEV_CMD_SYS_SET,
+		       &seq, (NLM_F_REQUEST | NLM_F_ACK));
+	mnl_attr_put_u8(rd->nlh, RDMA_NLDEV_SYS_ATTR_NETNS_MODE, enable);
+
+	return rd_sendrecv_msg(rd, seq);
+}
+
+static bool sys_valid_netns_cmd(const char *cmd)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(netns_modes_str); i++) {
+		if (!strcmp(cmd, netns_modes_str[i]))
+			return true;
+	}
+	return false;
+}
+
+static int sys_set_netns_args(struct rd *rd)
+{
+	bool cmd;
+
+	if (rd_no_arg(rd) || !sys_valid_netns_cmd(rd_argv(rd))) {
+		pr_err("valid options are: { shared | exclusive }\n");
+		return -EINVAL;
+	}
+
+	cmd = (strcmp(rd_argv(rd), "shared") == 0) ? true : false;
+
+	return sys_set_netns_cmd(rd, cmd);
+}
+
+static int sys_set_help(struct rd *rd)
+{
+	pr_out("Usage: %s system set [PARAM] value\n", rd->filename);
+	pr_out("            system set netns { shared | exclusive }\n");
+	return 0;
+}
+
+static int sys_set(struct rd *rd)
+{
+	const struct rd_cmd cmds[] = {
+		{ NULL,			sys_set_help },
+		{ "help",		sys_set_help },
+		{ "netns",		sys_set_netns_args},
+		{ 0 }
+	};
+
+	return rd_exec_cmd(rd, cmds, "parameter");
+}
+
+int cmd_sys(struct rd *rd)
+{
+	const struct rd_cmd cmds[] = {
+		{ NULL,		sys_show },
+		{ "show",	sys_show },
+		{ "set",	sys_set },
+		{ "help",	sys_help },
+		{ 0 }
+	};
+
+	return rd_exec_cmd(rd, cmds, "system command");
+}
diff --git a/rdma/utils.c b/rdma/utils.c
index 11ed8a73..558d1c29 100644
--- a/rdma/utils.c
+++ b/rdma/utils.c
@@ -435,6 +435,7 @@ static const enum mnl_attr_data_type nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
 	[RDMA_NLDEV_ATTR_DRIVER_U32] = MNL_TYPE_U32,
 	[RDMA_NLDEV_ATTR_DRIVER_S64] = MNL_TYPE_U64,
 	[RDMA_NLDEV_ATTR_DRIVER_U64] = MNL_TYPE_U64,
+	[RDMA_NLDEV_SYS_ATTR_NETNS_MODE] = MNL_TYPE_U8,
 };
 
 int rd_attr_check(const struct nlattr *attr, int *typep)
-- 
2.19.2

^ permalink raw reply related

* [PATCH iproute2-next v1 0/4] Enrich rdma tool for net namespace commands
From: Parav Pandit @ 2019-05-31  3:11 UTC (permalink / raw)
  To: dsahern; +Cc: netdev, linux-rdma, stephen, leonro, parav

RDMA subsystem can be running in either of the modes.
(a) Sharing RDMA devices among multiple net namespaces or
(b) Exclusive mode where RDMA device is bound to single net namespace 

This patch series adds
(1) query command to query rdma subsystem sharing mode
(2) set command to change rdma subsystem sharing mode
(3) assign rdma device to a net namespace

rdma tool examples:
(a) Query current rdma subsys net namespace sharing mode 
$ rdma sys show
netns shared

(b) Change rdma subsys mode to exclusive mode
$ rdma sys set netns exclusive

$ rdma sys show
netns exclusive

(c) Assign rdma device to a specific newly created net namespace
$ ip netns add foo
$ rdma dev set mlx5_1 netns foo 

---
Changelog:
---
v0->v1:
 - Addressed David Ahern's below comments
 - Updated license of rdma/sys.c to follow SPDX style
 - Avoid using ret variable in calling rd_recv_msg in sys_show_no_args()



Parav Pandit (4):
  rdma: Add an option to query,set net namespace sharing sys parameter
  rdma: Add man pages for rdma system commands
  rdma: Add an option to set net namespace of rdma device
  rdma: Add man page for rdma dev set netns command

 man/man8/rdma-dev.8    |  18 +++++-
 man/man8/rdma-system.8 |  82 ++++++++++++++++++++++++
 man/man8/rdma.8        |   7 ++-
 rdma/Makefile          |   2 +-
 rdma/dev.c             |  37 +++++++++++
 rdma/rdma.c            |   3 +-
 rdma/rdma.h            |   1 +
 rdma/sys.c             | 138 +++++++++++++++++++++++++++++++++++++++++
 rdma/utils.c           |   1 +
 9 files changed, 284 insertions(+), 5 deletions(-)
 create mode 100644 man/man8/rdma-system.8
 create mode 100644 rdma/sys.c

-- 
2.19.2

^ permalink raw reply

* RE: [PATCH iproute2-next 1/4] rdma: Add an option to query,set net namespace sharing sys parameter
From: Parav Pandit @ 2019-05-31  3:07 UTC (permalink / raw)
  To: David Ahern
  Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
	stephen@networkplumber.org, Leon Romanovsky
In-Reply-To: <014b3e56-9aa0-4b20-158c-d4907078d224@gmail.com>

Hi David,

> -----Original Message-----
> From: David Ahern <dsahern@gmail.com>
> Sent: Wednesday, May 29, 2019 9:43 PM
> To: Parav Pandit <parav@mellanox.com>
> Cc: netdev@vger.kernel.org; linux-rdma@vger.kernel.org;
> stephen@networkplumber.org; Leon Romanovsky <leonro@mellanox.com>
> Subject: Re: [PATCH iproute2-next 1/4] rdma: Add an option to query,set net
> namespace sharing sys parameter
> 
> On 5/21/19 8:22 AM, Parav Pandit wrote:
> > diff --git a/rdma/sys.c b/rdma/sys.c
> > new file mode 100644
> > index 00000000..78e5198f
> > --- /dev/null
> > +++ b/rdma/sys.c
> > @@ -0,0 +1,143 @@
> > +/*
> > + * sys.c	RDMA tool
> > + *
> > + *              This program is free software; you can redistribute it and/or
> > + *              modify it under the terms of the GNU General Public License
> > + *              as published by the Free Software Foundation; either version
> > + *              2 of the License, or (at your option) any later version.
> > + */
> 
> Please use the SPDX line like the other rdma files.
> 
Sure. Yes.

> > +
> > +#include "rdma.h"
> > +
> > +static int sys_help(struct rd *rd)
> > +{
> > +	pr_out("Usage: %s system show [ netns ]\n", rd->filename);
> > +	pr_out("       %s system set netns { shared | exclusive }\n", rd-
> >filename);
> > +	return 0;
> > +}
> > +
> > +static const char *netns_modes_str[] = {
> > +	"exclusive",
> > +	"shared",
> > +};
> > +
> > +static int sys_show_parse_cb(const struct nlmsghdr *nlh, void *data)
> > +{
> > +	struct nlattr *tb[RDMA_NLDEV_ATTR_MAX] = {};
> > +	struct rd *rd = data;
> > +
> > +	mnl_attr_parse(nlh, 0, rd_attr_cb, tb);
> > +
> > +	if (tb[RDMA_NLDEV_SYS_ATTR_NETNS_MODE]) {
> > +		const char *mode_str;
> > +		uint8_t netns_mode;
> > +
> > +		netns_mode =
> > +
> 	mnl_attr_get_u8(tb[RDMA_NLDEV_SYS_ATTR_NETNS_MODE]);
> > +
> > +		if (netns_mode <= ARRAY_SIZE(netns_modes_str))
> > +			mode_str = netns_modes_str[netns_mode];
> > +		else
> > +			mode_str = "unknown";
> > +
> > +		if (rd->json_output)
> > +			jsonw_string_field(rd->jw, "netns", mode_str);
> > +		else
> > +			pr_out("netns %s\n", mode_str);
> > +	}
> > +	return MNL_CB_OK;
> > +}
> > +
> > +static int sys_show_no_args(struct rd *rd) {
> > +	uint32_t seq;
> > +	int ret;
> > +
> > +	rd_prepare_msg(rd, RDMA_NLDEV_CMD_SYS_GET,
> > +		       &seq, (NLM_F_REQUEST | NLM_F_ACK));
> > +	ret = rd_send_msg(rd);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = rd_recv_msg(rd, sys_show_parse_cb, rd, seq);
> > +	return ret;
> 
> since you are fixing the header, why not just
> 	return rd_recv_msg(rd, sys_show_parse_cb, rd, seq);
> 
> like the other functions?

Yep. Changed.
Sending v1.

^ permalink raw reply

* Re: [PATCH, RESEND] RDMA/srp: Accept again source addresses that do not have a port number
From: Jason Gunthorpe @ 2019-05-30 18:44 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Leon Romanovsky, Doug Ledford, linux-rdma, Laurence Oberman,
	stable
In-Reply-To: <20190529163831.138926-1-bvanassche@acm.org>

On Wed, May 29, 2019 at 09:38:31AM -0700, Bart Van Assche wrote:
> The function srp_parse_in() is used both for parsing source address
> specifications and for target address specifications. Target addresses
> must have a port number. Having to specify a port number for source
> addresses is inconvenient. Make sure that srp_parse_in() supports again
> parsing addresses with no port number.
> 
> Cc: Laurence Oberman <loberman@redhat.com>
> Cc: <stable@vger.kernel.org>
> Fixes: c62adb7def71 ("IB/srp: Fix IPv6 address parsing") # v4.17.
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/infiniband/ulp/srp/ib_srp.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)

Bart, do you want this applied now, or are we still waiting for
Laurence?

Jason

^ permalink raw reply

* Re: [PATCH][next] IB/hfi1: Use struct_size() helper
From: Jason Gunthorpe @ 2019-05-30 18:43 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Mike Marciniszyn, Dennis Dalessandro, Doug Ledford, linux-rdma,
	linux-kernel
In-Reply-To: <20190529151528.GA24148@embeddedor>

On Wed, May 29, 2019 at 10:15:28AM -0500, Gustavo A. R. Silva wrote:
> Make use of the struct_size() helper instead of an open-coded version
> in order to avoid any potential type mistakes, in particular in the
> context in which this code is being used.
> 
> So, replace the following form:
> 
> sizeof(struct opa_port_status_rsp) + num_vls * sizeof(struct _vls_pctrs)
> 
> with:
> 
> struct_size(rsp, vls, num_vls)
> 
> and so on...
> 
> Also, notice that variable size is unnecessary, hence it is removed.
> 
> This code was detected with the help of Coccinelle.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> Reviewed-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
> ---
>  drivers/infiniband/hw/hfi1/mad.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)

Applied to for-next, thanks

Jason

^ permalink raw reply

* Re: [PATCH][next] IB/qib: Use struct_size() helper
From: Jason Gunthorpe @ 2019-05-30 18:43 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Dennis Dalessandro, Mike Marciniszyn, Doug Ledford, linux-rdma,
	linux-kernel
In-Reply-To: <20190529151326.GA24109@embeddedor>

On Wed, May 29, 2019 at 10:13:26AM -0500, Gustavo A. R. Silva wrote:
> Make use of the struct_size() helper instead of an open-coded version
> in order to avoid any potential type mistakes, in particular in the
> context in which this code is being used.
> 
> So, replace the following form:
> 
> sizeof(*pkt) + sizeof(pkt->addr[0])*n
> 
> with:
> 
> struct_size(pkt, addr, n)
> 
> Also, notice that variable size is unnecessary, hence it is removed.
> 
> This code was detected with the help of Coccinelle.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> Reviewed-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
> ---
>  drivers/infiniband/hw/qib/qib_user_sdma.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Applied to for-next, thanks

Jason

^ permalink raw reply

* Re: [PATCH][next] IB/rdmavt: Use struct_size() helper
From: Jason Gunthorpe @ 2019-05-30 18:43 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Dennis Dalessandro, Mike Marciniszyn, Doug Ledford, linux-rdma,
	linux-kernel
In-Reply-To: <20190529151248.GA24080@embeddedor>

On Wed, May 29, 2019 at 10:12:48AM -0500, Gustavo A. R. Silva wrote:
> Make use of the struct_size() helper instead of an open-coded version
> in order to avoid any potential type mistakes, in particular in the
> context in which this code is being used.
> 
> So, replace the following form:
> 
> sizeof(struct rvt_sge) * init_attr->cap.max_send_sge + sizeof(struct rvt_swqe)
> 
> with:
> 
> struct_size(swq, sg_list, init_attr->cap.max_send_sge)
> 
> and so on...
> 
> Also, notice that variable size is unnecessary, hence it is removed.
> 
> This code was detected with the help of Coccinelle.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> Reviewed-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
> ---
>  drivers/infiniband/sw/rdmavt/qp.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

Applied to for-next, thanks

Jason

^ permalink raw reply

* Re: [PATCH][next] IB/hfi1: Use struct_size() helper
From: Dennis Dalessandro @ 2019-05-30 18:30 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Mike Marciniszyn, Doug Ledford,
	Jason Gunthorpe
  Cc: linux-rdma, linux-kernel
In-Reply-To: <20190529151528.GA24148@embeddedor>

On 5/29/2019 11:15 AM, Gustavo A. R. Silva wrote:
> Make use of the struct_size() helper instead of an open-coded version
> in order to avoid any potential type mistakes, in particular in the
> context in which this code is being used.
> 
> So, replace the following form:
> 
> sizeof(struct opa_port_status_rsp) + num_vls * sizeof(struct _vls_pctrs)
> 
> with:
> 
> struct_size(rsp, vls, num_vls)
> 
> and so on...
> 
> Also, notice that variable size is unnecessary, hence it is removed.
> 
> This code was detected with the help of Coccinelle.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
>   drivers/infiniband/hw/hfi1/mad.c | 9 +++------
>   1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/hfi1/mad.c b/drivers/infiniband/hw/hfi1/mad.c
> index 4228393e6c4c..184dba3c2828 100644
> --- a/drivers/infiniband/hw/hfi1/mad.c
> +++ b/drivers/infiniband/hw/hfi1/mad.c
> @@ -2744,8 +2744,7 @@ static int pma_get_opa_portstatus(struct opa_pma_mad *pmp,
>   	u16 link_width;
>   	u16 link_speed;
>   
> -	response_data_size = sizeof(struct opa_port_status_rsp) +
> -				num_vls * sizeof(struct _vls_pctrs);
> +	response_data_size = struct_size(rsp, vls, num_vls);
>   	if (response_data_size > sizeof(pmp->data)) {
>   		pmp->mad_hdr.status |= OPA_PM_STATUS_REQUEST_TOO_LARGE;
>   		return reply((struct ib_mad_hdr *)pmp);
> @@ -3014,8 +3013,7 @@ static int pma_get_opa_datacounters(struct opa_pma_mad *pmp,
>   	}
>   
>   	/* Sanity check */
> -	response_data_size = sizeof(struct opa_port_data_counters_msg) +
> -				num_vls * sizeof(struct _vls_dctrs);
> +	response_data_size = struct_size(req, port[0].vls, num_vls);
>   
>   	if (response_data_size > sizeof(pmp->data)) {
>   		pmp->mad_hdr.status |= IB_SMP_INVALID_FIELD;
> @@ -3232,8 +3230,7 @@ static int pma_get_opa_porterrors(struct opa_pma_mad *pmp,
>   		return reply((struct ib_mad_hdr *)pmp);
>   	}
>   
> -	response_data_size = sizeof(struct opa_port_error_counters64_msg) +
> -				num_vls * sizeof(struct _vls_ectrs);
> +	response_data_size = struct_size(req, port[0].vls, num_vls);
>   
>   	if (response_data_size > sizeof(pmp->data)) {
>   		pmp->mad_hdr.status |= IB_SMP_INVALID_FIELD;
> 

Reviewed-by: Dennis Dalessandro <dennis.dalessandro@intel.com>

^ permalink raw reply

* Re: [PATCH][next] IB/qib: Use struct_size() helper
From: Dennis Dalessandro @ 2019-05-30 18:29 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Mike Marciniszyn, Doug Ledford,
	Jason Gunthorpe
  Cc: linux-rdma, linux-kernel
In-Reply-To: <20190529151326.GA24109@embeddedor>

On 5/29/2019 11:13 AM, Gustavo A. R. Silva wrote:
> Make use of the struct_size() helper instead of an open-coded version
> in order to avoid any potential type mistakes, in particular in the
> context in which this code is being used.
> 
> So, replace the following form:
> 
> sizeof(*pkt) + sizeof(pkt->addr[0])*n
> 
> with:
> 
> struct_size(pkt, addr, n)
> 
> Also, notice that variable size is unnecessary, hence it is removed.
> 
> This code was detected with the help of Coccinelle.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
>   drivers/infiniband/hw/qib/qib_user_sdma.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/qib/qib_user_sdma.c b/drivers/infiniband/hw/qib/qib_user_sdma.c
> index 0c204776263f..97649f64e09e 100644
> --- a/drivers/infiniband/hw/qib/qib_user_sdma.c
> +++ b/drivers/infiniband/hw/qib/qib_user_sdma.c
> @@ -904,10 +904,11 @@ static int qib_user_sdma_queue_pkts(const struct qib_devdata *dd,
>   		}
>   
>   		if (frag_size) {
> -			int pktsize, tidsmsize, n;
> +			int tidsmsize, n;
> +			size_t pktsize;
>   
>   			n = npages*((2*PAGE_SIZE/frag_size)+1);
> -			pktsize = sizeof(*pkt) + sizeof(pkt->addr[0])*n;
> +			pktsize = struct_size(pkt, addr, n);
>   
>   			/*
>   			 * Determine if this is tid-sdma or just sdma.
> 

Again, same minor objection but correct patch none the less.

Reviewed-by: Dennis Dalessandro <dennis.dalessandro@intel.com>

^ permalink raw reply


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