Re: [PATCH for-next 2/2] IB/core: Support for CMA multicast join flags

[Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2253 bytes --]

On Mon, Jul 04, 2016 at 03:40:10PM +0300, Or Gerlitz wrote:
> On Mon, Jul 4, 2016 at 7:51 AM, Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
> > On Sun, Jul 03, 2016 at 04:46:23PM +0300, Or Gerlitz wrote:
> >> On Sun, Jul 3, 2016 at 3:47 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> >> > From: Alex Vesker <valex-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >> >
> >>
> >> >  drivers/infiniband/core/cma.c    | 98 +++++++++++++++++++++++++++++++++++++---
> >> >  drivers/infiniband/core/ucma.c   | 18 ++++++--
> >> >  include/rdma/ib_sa.h             |  5 ++
> >> >  include/rdma/rdma_cm.h           |  4 +-
> >> >  include/uapi/rdma/rdma_user_cm.h |  9 +++-
> >> >  5 files changed, 122 insertions(+), 12 deletions(-)
> >>
> >>
> >> For the ease/robustness of review for UAPI changes, we have a long
> >> time common practice
> >> to break things like this one to IB core kernel only patch, and one
> >> that deals the user-space
> 
> > You are right, this practice exists and we are following it as much as it makes sense.
> > This specific case doesn't need to be separated, because it introduces one logical
> > change and separate patches will be useless as standalone patches.
> 
> Leon,
> 
> The point is that you need to get people to be used to that practice,
> and it seems we're not doing that. Otherwise I wouldn't have to chat
> for 20m with 2-3 people that wonder why I made these comments. I think
> we should require it from the developers, period and not argue on
> that. B/c in bunch of other places, it is totally required, for
> example, here you could just carve this small piece to be part of your
> UAPI patch, what's wrong with that?
> 
> --- a/include/uapi/rdma/rdma_user_cm.h
> +++ b/include/uapi/rdma/rdma_user_cm.h
> @@ -244,12 +244,19 @@ struct rdma_ucm_join_ip_mcast {
>   __u32 id;
>  };
> 
>  struct rdma_ucm_join_mcast {
>   __u64 response; /* rdma_ucma_create_id_resp */
>   __u64 uid;
>   __u32 id;
>   __u16 addr_size;
> - __u16 reserved;
> + __u16 join_flags;
>   struct sockaddr_storage addr;
>  };

The main issue here that new code is using this struct and new field while the old
code uses reserved field.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]