Re: [PATCH rdma-next 0/7] Enrich DEVX support

[Doug Ledford]

On Tue, 2018-12-04 at 12:15 -0700, Jason Gunthorpe wrote:
> On Tue, Dec 4, 2018 at 12:02 PM Doug Ledford  wrote:
> > On Mon, 2018-11-26 at 08:28 +0200, Leon Romanovsky wrote:
> > > From: Leon Romanovsky 
> > > 
> > > From Yishai,
> > > ---
> > > This series enriches DEVX support in few aspects: it enables 
> > > interoperability
> > > between DEVX and verbs and improves mechanism for controlling privileged 
> > > DEVX
> > > commands.
> > > 
> > > The first patch updates mlx5 ifc file.
> > > 
> > > Next 3 patches enable modifying and querying verbs objects via the DEVX
> > > interface.
> > > 
> > > To achieve that the core layer introduced the 'UVERBS_IDR_ANY_OBJECT' type
> > > to match any IDR object. Once it's used by some driver's method, the
> > > infrastructure skips checking for the IDR type and it becomes the driver
> > > handler responsibility.
> > > 
> > > The DEVX methods of modify and query were changed to get any object type 
> > > via
> > > the 'UVERBS_IDR_ANY_OBJECT' mechanism. The type checking is done per 
> > > object as
> > > part of the driver code.
> > > 
> > > The next 3 patches introduce more robust mechanism for controlling 
> > > privileged
> > > DEVX commands. The responsibility to block/allow per command was moved to 
> > > be
> > > done in the firmware based on the UID credentials that the driver reports 
> > > upon
> > > user context creation. This enables more granularity per command based on 
> > > the
> > > device security model and the user credentials.
> > > 
> > > In addition, by introducing a valid range for 'general commands' we 
> > > prevent the
> > > need to touch the driver's code any time that a new future command will be
> > > added.
> > > 
> > > The last patch fixes the XRC verbs flow once a DEVX context is used. This 
> > > is
> > > needed as XRCD is some shared kernel resource and as such a kernel UID 
> > > (=0)
> > > should be used in its related resources.
> > > 
> > > Thanks
> > > 
> > > Yishai Hadas (7):
> > >   net/mlx5: Update mlx5_ifc with DEVX UCTX capabilities bits
> > >   IB/core: Introduce UVERBS_IDR_ANY_OBJECT
> > >   IB/core: Enable getting an object type from a given uobject
> > >   IB/mlx5: Enable modify and query verbs objects via DEVX
> > >   IB/mlx5: Enforce DEVX privilege by firmware
> > >   IB/mlx5: Update the supported DEVX commands
> > >   IB/mlx5: Allow XRC usage via verbs in DEVX context
> > > 
> > >  drivers/infiniband/core/rdma_core.c   |  27 +++--
> > >  drivers/infiniband/core/rdma_core.h   |  21 ++--
> > >  drivers/infiniband/core/uverbs_uapi.c |  10 +-
> > >  drivers/infiniband/hw/mlx5/devx.c | 142 ++
> > >  drivers/infiniband/hw/mlx5/main.c |   4 +-
> > >  drivers/infiniband/hw/mlx5/mlx5_ib.h  |   6 +-
> > >  drivers/infiniband/hw/mlx5/qp.c   |  12 +--
> > >  drivers/infiniband/hw/mlx5/srq.c  |   2 +-
> > >  include/linux/mlx5/mlx5_ifc.h |  26 -
> > >  include/rdma/uverbs_ioctl.h   |   6 ++
> > >  include/rdma/uverbs_std_types.h   |  12 +++
> > >  11 files changed, 215 insertions(+), 53 deletions(-)
> > > 
> > > --
> > > 2.19.1
> > > 
> > 
> > Hi Leon,
> > 
> > I've put this into my wip/dl-for-next branch.  Because it pulled in the
> > mlx5-next tree (which was significant because the patch we wanted was at
> > the tip and a whole slew of other patches were before it),
> 
> Yes, this is how the shared branch works, DaveM merges it, we don't
> have to unless we have a dependency, so it can accumulate a bit.

I get how it works :-P.  I was just commenting that it had in fact built
up an accumulation.

> > the changeset
> > for this patch looks much bigger.  And it had the merge conflict you
> > mentioned.  You might want to double check my work before it gets moved
> > over to the official for-next branch.
> 
> It is a bit easier to follow the git log if merges are done with the
> --log option to summarize what was pulled in.

I'll modify my merge alias to add it.  As it stands, I'll repush this to
my wip branch with the --log option added.

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


signature.asc
Description: This is a digitally signed message part

Re: [PATCH rdma-next 0/7] Enrich DEVX support

[Doug Ledford]

On Mon, 2018-11-26 at 08:28 +0200, Leon Romanovsky wrote:
> From: Leon Romanovsky 
> 
> From Yishai,
> ---
> This series enriches DEVX support in few aspects: it enables interoperability
> between DEVX and verbs and improves mechanism for controlling privileged DEVX
> commands.
> 
> The first patch updates mlx5 ifc file.
> 
> Next 3 patches enable modifying and querying verbs objects via the DEVX
> interface.
> 
> To achieve that the core layer introduced the 'UVERBS_IDR_ANY_OBJECT' type
> to match any IDR object. Once it's used by some driver's method, the
> infrastructure skips checking for the IDR type and it becomes the driver
> handler responsibility.
> 
> The DEVX methods of modify and query were changed to get any object type via
> the 'UVERBS_IDR_ANY_OBJECT' mechanism. The type checking is done per object as
> part of the driver code.
> 
> The next 3 patches introduce more robust mechanism for controlling privileged
> DEVX commands. The responsibility to block/allow per command was moved to be
> done in the firmware based on the UID credentials that the driver reports upon
> user context creation. This enables more granularity per command based on the
> device security model and the user credentials.
> 
> In addition, by introducing a valid range for 'general commands' we prevent 
> the
> need to touch the driver's code any time that a new future command will be
> added.
> 
> The last patch fixes the XRC verbs flow once a DEVX context is used. This is
> needed as XRCD is some shared kernel resource and as such a kernel UID (=0)
> should be used in its related resources.
> 
> Thanks
> 
> Yishai Hadas (7):
>   net/mlx5: Update mlx5_ifc with DEVX UCTX capabilities bits
>   IB/core: Introduce UVERBS_IDR_ANY_OBJECT
>   IB/core: Enable getting an object type from a given uobject
>   IB/mlx5: Enable modify and query verbs objects via DEVX
>   IB/mlx5: Enforce DEVX privilege by firmware
>   IB/mlx5: Update the supported DEVX commands
>   IB/mlx5: Allow XRC usage via verbs in DEVX context
> 
>  drivers/infiniband/core/rdma_core.c   |  27 +++--
>  drivers/infiniband/core/rdma_core.h   |  21 ++--
>  drivers/infiniband/core/uverbs_uapi.c |  10 +-
>  drivers/infiniband/hw/mlx5/devx.c | 142 ++
>  drivers/infiniband/hw/mlx5/main.c |   4 +-
>  drivers/infiniband/hw/mlx5/mlx5_ib.h  |   6 +-
>  drivers/infiniband/hw/mlx5/qp.c   |  12 +--
>  drivers/infiniband/hw/mlx5/srq.c  |   2 +-
>  include/linux/mlx5/mlx5_ifc.h |  26 -
>  include/rdma/uverbs_ioctl.h   |   6 ++
>  include/rdma/uverbs_std_types.h   |  12 +++
>  11 files changed, 215 insertions(+), 53 deletions(-)
> 
> --
> 2.19.1
> 

Hi Leon,

I've put this into my wip/dl-for-next branch.  Because it pulled in the
mlx5-next tree (which was significant because the patch we wanted was at
the tip and a whole slew of other patches were before it), the changeset
for this patch looks much bigger.  And it had the merge conflict you
mentioned.  You might want to double check my work before it gets moved
over to the official for-next branch.

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


signature.asc
Description: This is a digitally signed message part

Re: [PATCH mlx5-next 1/7] net/mlx5: Update mlx5_ifc with DEVX UCTX capabilities bits

[Doug Ledford]

On Mon, 2018-11-26 at 08:28 +0200, Leon Romanovsky wrote:
> From: Yishai Hadas 
> 
> Expose device capabilities for DEVX user context, it includes which caps
> the device is supported and a matching bit to set as part of user
> context creation.
> 
> Signed-off-by: Yishai Hadas 
> Reviewed-by: Artemy Kovalyov 
> Signed-off-by: Leon Romanovsky 

This looks fine to me.  Is it in mlx5-next yet?

> ---
>  include/linux/mlx5/mlx5_ifc.h | 16 ++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h
> index 6f64e814cc10..ece1b606c909 100644
> --- a/include/linux/mlx5/mlx5_ifc.h
> +++ b/include/linux/mlx5/mlx5_ifc.h
> @@ -883,6 +883,10 @@ enum {
>   MLX5_CAP_UMR_FENCE_NONE = 0x2,
>  };
>  
> +enum {
> + MLX5_UCTX_CAP_RAW_TX = 1UL << 0,
> +};
> +
>  struct mlx5_ifc_cmd_hca_cap_bits {
>   u8 reserved_at_0[0x30];
>   u8 vhca_id[0x10];
> @@ -1193,7 +1197,13 @@ struct mlx5_ifc_cmd_hca_cap_bits {
>   u8 num_vhca_ports[0x8];
>   u8 reserved_at_618[0x6];
>   u8 sw_owner_id[0x1];
> - u8 reserved_at_61f[0x1e1];
> + u8 reserved_at_61f[0x1];
> +
> + u8 reserved_at_620[0x80];
> +
> + u8 uctx_cap[0x20];
> +
> + u8 reserved_at_6c0[0x140];
>  };
>  
>  enum mlx5_flow_destination_type {
> @@ -9276,7 +9286,9 @@ struct mlx5_ifc_umem_bits {
>  struct mlx5_ifc_uctx_bits {
>   u8 modify_field_select[0x40];
>  
> - u8 reserved_at_40[0x1c0];
> + u8 cap[0x20];
> +
> + u8 reserved_at_60[0x1a0];
>  };
>  
>  struct mlx5_ifc_create_umem_in_bits {

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


signature.asc
Description: This is a digitally signed message part

Re: [PATCH rdma] net/mlx5: Fix XRC SRQ umem valid bits

[Doug Ledford]

On Tue, 2018-11-06 at 22:02 +, Jason Gunthorpe wrote:
> On Tue, Nov 06, 2018 at 04:31:08PM -0500, Doug Ledford wrote:
> > On Wed, 2018-10-31 at 12:20 +0200, Leon Romanovsky wrote:
> > > From: Yishai Hadas 
> > > 
> > > Adapt XRC SRQ to the latest HW specification with fixed definition
> > > around umem valid bits. The previous definition relied on a bit which
> > > was taken for other purposes in legacy FW.
> > > 
> > > Fixes: bd37197554eb ("net/mlx5: Update mlx5_ifc with DEVX UID bits")
> > > Signed-off-by: Yishai Hadas 
> > > Reviewed-by: Artemy Kovalyov 
> > > Signed-off-by: Leon Romanovsky 
> > > Hi Doug, Jason
> > > 
> > > This commit fixes code sent in this merge window, so I'm not marking it
> > > with any rdma-rc/rdma-next. It will be better to be sent during this merge
> > > window if you have extra pull request to issue, or as a -rc material, if
> > > not.
> > > 
> > > BTW, we didn't combine reserved fields, because our convention is to 
> > > align such
> > > fields to 32 bits for better readability.
> > > 
> > > Thanks
> > 
> > This looks fine.  Let me know when it's in the mlx5-next tree to pull.
> 
> It needs to go to -rc... 
> 
> This needs a mlx5-rc branch for this I guess?

I don't think so.  As long as it's the first commit in mlx5-next, and
mlx5-next is 4.20-rc1 based, then pulling this commit into the -rc tree
will only pull the single commit.  Then when we pull into for-next for
the first time, we will get this in for-next too.  That seems best to
me.

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


signature.asc
Description: This is a digitally signed message part

Re: [PATCH rdma] net/mlx5: Fix XRC SRQ umem valid bits

[Doug Ledford]

On Wed, 2018-10-31 at 12:20 +0200, Leon Romanovsky wrote:
> From: Yishai Hadas 
> 
> Adapt XRC SRQ to the latest HW specification with fixed definition
> around umem valid bits. The previous definition relied on a bit which
> was taken for other purposes in legacy FW.
> 
> Fixes: bd37197554eb ("net/mlx5: Update mlx5_ifc with DEVX UID bits")
> Signed-off-by: Yishai Hadas 
> Reviewed-by: Artemy Kovalyov 
> Signed-off-by: Leon Romanovsky 
> ---
> Hi Doug, Jason
> 
> This commit fixes code sent in this merge window, so I'm not marking it
> with any rdma-rc/rdma-next. It will be better to be sent during this merge
> window if you have extra pull request to issue, or as a -rc material, if
> not.
> 
> BTW, we didn't combine reserved fields, because our convention is to align 
> such
> fields to 32 bits for better readability.
> 
> Thanks

This looks fine.  Let me know when it's in the mlx5-next tree to pull.

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


signature.asc
Description: This is a digitally signed message part

Re: [PATCH rdma-next] IB/mlx5: Add support for extended atomic operations

[Doug Ledford]

On Wed, 2018-10-10 at 09:25 +0300, Leon Romanovsky wrote:
> From: Yonatan Cohen 
> 
> Extended atomic operations cmp and fetch is a Mellanox
> feature extending the standard atomic operation to use, varied
> operand sizes, as apposed to normal atomic operation that use
> an 8 byte operand only.
> Extended atomics allows masking the results and arguments.
> 
> This patch configures QP to support extended atomic operation
> with the maximum size possible, as exposed by HCA capabilities.
> 
> Signed-off-by: Yonatan Cohen 
> Reviewed-by: Guy Levi 
> Signed-off-by: Leon Romanovsky 

Thanks, applied to for-next.

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


signature.asc
Description: This is a digitally signed message part

Re: [PATCH rdma-next v1 0/4] Scatter to CQE

[Doug Ledford]

On Tue, 2018-10-16 at 22:00 +0300, Leon Romanovsky wrote:
> On Tue, Oct 16, 2018 at 02:39:01PM -0400, Doug Ledford wrote:
> > On Tue, 2018-10-09 at 12:05 +0300, Leon Romanovsky wrote:
> > > From: Leon Romanovsky 
> > > 
> > > Changelog v0->v1:
> > >  * Changed patch #3 to use check_mask function from rdma-core instead 
> > > define.
> > > 
> > > --
> > > From Yonatan,
> > > 
> > > Scatter to CQE is a HW offload feature that saves PCI writes by
> > > scattering the payload to the CQE.
> > > 
> > > The feature depends on the CQE size and if the CQE size is 64B, it will
> > > work for payload smaller than 32. If the CQE size is 128B, it will work 
> > > for
> > > payload smaller than 64.
> > > 
> > > The feature works for responder and requestor:
> > > 1. For responder, if the payload is small as required above, the data
> > > will be part of the CQE, and thus we save another PCI transaction the 
> > > recv buffers.
> > > 2. For requestor, this can be used to get the RDMA_READ response and
> > > RDMA_ATOMIC response in the CQE. This feature is already supported in 
> > > upstream.
> > > 
> > > As part of this series, we are adding support for DC transport type and
> > > ability to enable the feature (force enable) in the requestor when SQ
> > > is not configured to signal all WRs.
> > > 
> > > Thanks
> > > 
> > > Yonatan Cohen (4):
> > >   net/mlx5: Expose DC scatter to CQE capability bit
> > >   IB/mlx5: Support scatter to CQE for DC transport type
> > >   IB/mlx5: Verify that driver supports user flags
> > >   IB/mlx5: Allow scatter to CQE without global signaled WRs
> > > 
> > >  drivers/infiniband/hw/mlx5/cq.c  |  2 +-
> > >  drivers/infiniband/hw/mlx5/mlx5_ib.h |  2 +-
> > >  drivers/infiniband/hw/mlx5/qp.c  | 93 
> > > 
> > >  include/linux/mlx5/mlx5_ifc.h|  3 +-
> > >  include/uapi/rdma/mlx5-abi.h |  1 +
> > >  5 files changed, 79 insertions(+), 22 deletions(-)
> > > 
> > > --
> > > 2.14.4
> > > 
> > 
> > 
> > Hi Leon,
> > 
> > This series looks fine.  Let me know when the net/mlx5 portion has been
> > committed.
> 
> Thanks Doug,
> I pushed first patch to mlx5-next
> 94a04d1d3d36 ("net/mlx5: Expose DC scatter to CQE capability bit")

Thanks Leon, mlx5-next merged in, then remainder of series applied to
for-next.

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


signature.asc
Description: This is a digitally signed message part

Re: [PATCH rdma-next v1 0/4] Scatter to CQE

[Doug Ledford]

On Tue, 2018-10-09 at 12:05 +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky 
> 
> Changelog v0->v1:
>  * Changed patch #3 to use check_mask function from rdma-core instead define.
> 
> --
> From Yonatan,
> 
> Scatter to CQE is a HW offload feature that saves PCI writes by
> scattering the payload to the CQE.
> 
> The feature depends on the CQE size and if the CQE size is 64B, it will
> work for payload smaller than 32. If the CQE size is 128B, it will work for
> payload smaller than 64.
> 
> The feature works for responder and requestor:
> 1. For responder, if the payload is small as required above, the data
> will be part of the CQE, and thus we save another PCI transaction the recv 
> buffers.
> 2. For requestor, this can be used to get the RDMA_READ response and
> RDMA_ATOMIC response in the CQE. This feature is already supported in 
> upstream.
> 
> As part of this series, we are adding support for DC transport type and
> ability to enable the feature (force enable) in the requestor when SQ
> is not configured to signal all WRs.
> 
> Thanks
> 
> Yonatan Cohen (4):
>   net/mlx5: Expose DC scatter to CQE capability bit
>   IB/mlx5: Support scatter to CQE for DC transport type
>   IB/mlx5: Verify that driver supports user flags
>   IB/mlx5: Allow scatter to CQE without global signaled WRs
> 
>  drivers/infiniband/hw/mlx5/cq.c  |  2 +-
>  drivers/infiniband/hw/mlx5/mlx5_ib.h |  2 +-
>  drivers/infiniband/hw/mlx5/qp.c  | 93 
> 
>  include/linux/mlx5/mlx5_ifc.h|  3 +-
>  include/uapi/rdma/mlx5-abi.h |  1 +
>  5 files changed, 79 insertions(+), 22 deletions(-)
> 
> --
> 2.14.4
> 


Hi Leon,

This series looks fine.  Let me know when the net/mlx5 portion has been
committed.

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


signature.asc
Description: This is a digitally signed message part

Re: [PATCH rdma-next 0/4] mlx5 vport loopback

[Doug Ledford]

On Mon, 2018-09-17 at 13:30 +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky 
> 
> Hi,
> 
> This is short series from Mark which extends handling of loopback
> traffic. Originally mlx5 IB dynamically enabled/disabled both unicast
> and multicast based on number of users. However RAW ethernet QPs need
> more granular access.
> 
> Thanks
> 
> Mark Bloch (4):
>   net/mlx5: Rename incorrect naming in IFC file
>   RDMA/mlx5: Refactor transport domain bookkeeping logic
>   RDMA/mlx5: Allow creating RAW ethernet QP with loopback support
>   RDMA/mlx5: Enable vport loopback when user context or QP mandate
> 
>  drivers/infiniband/hw/mlx5/main.c  | 61 ++
>  drivers/infiniband/hw/mlx5/mlx5_ib.h   | 16 +++-
>  drivers/infiniband/hw/mlx5/qp.c| 96 
> +-
>  .../net/ethernet/mellanox/mlx5/core/en_common.c|  2 +-
>  include/linux/mlx5/mlx5_ifc.h  |  4 +-
>  include/uapi/rdma/mlx5-abi.h   |  2 +
>  6 files changed, 138 insertions(+), 43 deletions(-)
> 
> --
> 2.14.4
> 

Thanks, first patch pulled from mlx5-next, remainder of series applied.

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


signature.asc
Description: This is a digitally signed message part

Re: [PATCH rdma-next 0/4] mlx5 vport loopback

[Doug Ledford]

On Sat, 2018-09-22 at 00:40 +0300, Leon Romanovsky wrote:
> On Fri, Sep 21, 2018 at 04:05:53PM -0400, Doug Ledford wrote:
> > On Fri, 2018-09-21 at 22:33 +0300, Leon Romanovsky wrote:
> > > Hope it makes it clear now.
> > 
> > Clear enough.  Between yours and Jason's explanation I think it's well
> > covered.
> > 
> > > Are you ok with me/Saeed taking first patch to our branch so you will be
> > > able to take the rest?
> > 
> > Yep.  Let me know a tag when it's ready to merge.
> 
> Done, it doesn't have tag.
> Commit d773ff41a7c ("net/mlx5: Rename incorrect naming in IFC file")


You confused me for a minute!  There is a copy-paste issue in the above.
The actual commit hash is 5d773ff41a7c.  Thanks, got it.

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


signature.asc
Description: This is a digitally signed message part

Re: [PATCH rdma-next 0/4] mlx5 vport loopback

[Doug Ledford]

On Fri, 2018-09-21 at 22:33 +0300, Leon Romanovsky wrote:
> Hope it makes it clear now.

Clear enough.  Between yours and Jason's explanation I think it's well
covered.

> Are you ok with me/Saeed taking first patch to our branch so you will be
> able to take the rest?

Yep.  Let me know a tag when it's ready to merge.

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


signature.asc
Description: This is a digitally signed message part

Re: [PATCH rdma-next 0/4] mlx5 vport loopback

[Doug Ledford]

On Mon, 2018-09-17 at 13:30 +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky 
> 
> Hi,
> 
> This is short series from Mark which extends handling of loopback
> traffic. Originally mlx5 IB dynamically enabled/disabled both unicast
> and multicast based on number of users. However RAW ethernet QPs need
> more granular access.
> 
> Thanks
> 
> Mark Bloch (4):
>   net/mlx5: Rename incorrect naming in IFC file
>   RDMA/mlx5: Refactor transport domain bookkeeping logic
>   RDMA/mlx5: Allow creating RAW ethernet QP with loopback support
>   RDMA/mlx5: Enable vport loopback when user context or QP mandate

I've reviewed this series and I'm OK with it, but the first patch is for
net/mlx5.  How are you expecting the series to be applied?  Are you
wanting me or Jason to take the entire series, or does the first patch
need to go through the mlx5 tree and get picked up by Dave and us, and
then we take the rest?  This is unclear to me...

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


signature.asc
Description: This is a digitally signed message part

Re: [PATCH v4 0/3] IB/ipoib: Use dev_port to disambiguate port numbers

[Doug Ledford]

On Thu, 2018-09-06 at 17:51 +0300, Arseny Maslennikov wrote:
> Pre-3.15 userspace had trouble distinguishing different ports
> of a NIC on a single PCI bus/device/function. To solve this,
> a sysfs field `dev_port' was introduced quite a while ago
> (commit v3.14-rc3-739-g3f85944fe207), and some relevant device
> drivers were fixed to use it, but not in case of IPoIB.
> 
> The convention for some reason never got documented in the kernel, but
> was immediately adopted by userspace (notably udev[1][2], biosdevname[3])
> 
> 1/3 documents the sysfs field — that's why I'm CC-ing netdev.
> 
> This series was tested on and applies to 4.19-rc2.
> 
> [1] https://lists.freedesktop.org/archives/systemd-devel/2014-June/020788.html
> [2] https://lists.freedesktop.org/archives/systemd-devel/2014-July/020804.html
> [3] 
> https://github.com/CloudAutomationNTools/biosdevname/blob/c795d51dd93a5309652f0d635f12a3ecfabfaa72/src/eths.c#L38
> 
> v1->v2: replace a line instead of inserting and then removing.
> v2->v3: restore both attributes, output a notice of deprecation to kmsg.
> v3->v4: style adjustments, join the deprecation notice to single line.
> 
> Arseny Maslennikov (3):
>   Documentation/ABI: document /sys/class/net/*/dev_port
>   IB/ipoib: Use dev_port to expose network interface port numbers
>   IB/ipoib: Log sysfs 'dev_id' accesses from userspace
> 
>  Documentation/ABI/testing/sysfs-class-net | 18 +
>  drivers/infiniband/ulp/ipoib/ipoib_main.c | 33 +++
>  2 files changed, 51 insertions(+)
> 

Series applied to for-next.  But I think we should watch feedback from
people, and if people think the notification about using the wrong
variable is too noisy, then we might want to revert it or modify it to
only print out once per specific executable instead of once per run of
each executable.

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


signature.asc
Description: This is a digitally signed message part

Re: [PATCH v4 3/3] IB/ipoib: Log sysfs 'dev_id' accesses from userspace

[Doug Ledford]

On Sun, 2018-09-09 at 23:55 +0300, Arseny Maslennikov wrote:
> On Sun, Sep 09, 2018 at 09:11:46PM +0300, Arseny Maslennikov wrote:
> > On Fri, Sep 07, 2018 at 09:43:59AM -0600, Jason Gunthorpe wrote:
> > > On Thu, Sep 06, 2018 at 05:51:12PM +0300, Arseny Maslennikov wrote:
> > > > Some tools may currently be using only the deprecated attribute;
> > > > let's print an elaborate and clear deprecation notice to kmsg.
> > > > 
> > > > To do that, we have to replace the whole sysfs file, since we inherit
> > > > the original one from netdev.
> > > > 
> > > > Signed-off-by: Arseny Maslennikov 
> > > >  drivers/infiniband/ulp/ipoib/ipoib_main.c | 31 +++
> > > >  1 file changed, 31 insertions(+)
> > > > 
> > > > diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c 
> > > > b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > > > index 30f840f874b3..74732726ec6f 100644
> > > > +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > > > @@ -2386,6 +2386,35 @@ int ipoib_add_pkey_attr(struct net_device *dev)
> > > > return device_create_file(>dev, _attr_pkey);
> > > >  }
> > > >  
> > > > +/*
> > > > + * We erroneously exposed the iface's port number in the dev_id
> > > > + * sysfs field long after dev_port was introduced for that purpose[1],
> > > > + * and we need to stop everyone from relying on that.
> > > > + * Let's overload the shower routine for the dev_id file here
> > > > + * to gently bring the issue up.
> > > > + *
> > > > + * [1] https://www.spinics.net/lists/netdev/msg272123.html
> > > > + */
> > > > +static ssize_t dev_id_show(struct device *dev,
> > > > +  struct device_attribute *attr, char *buf)
> > > > +{
> > > > +   struct net_device *ndev = to_net_dev(dev);
> > > > +
> > > > +   if (ndev->dev_id == ndev->dev_port)
> > > > +   netdev_info_once(ndev,
> > > > +   "\"%s\" wants to know my dev_id. Should it look 
> > > > at dev_port instead? See Documentation/ABI/testing/sysfs-class-net for 
> > > > more info.\n",
> > > > +   current->comm);
> > > > +
> > > > +   return sprintf(buf, "%#x\n", ndev->dev_id);
> > > > +}
> > > > +static DEVICE_ATTR_RO(dev_id);
> > > > +
> > > > +int ipoib_intercept_dev_id_attr(struct net_device *dev)
> > > > +{
> > > > +   device_remove_file(>dev, _attr_dev_id);
> > > > +   return device_create_file(>dev, _attr_dev_id);
> > > > +}
> > > 
> > > Isn't this racey with userspace? Ie what happens if udev is querying
> > > the dev_id right here?
> > 
> > udev in particular does not use dev_id at all since 2014, because "why
> > would we keep using dev_id if it is not the right thing to use?".
> > 
> > > 
> > > Do we know there is no userspace doing this?
> > > 
> > 
> > Not for sure.
> > 
> > If we move all the sysfs handling stuff we introduce in _add_port():
> >  - pkey
> >  - umcast
> >  - {create,delete}_child
> >  - connected/datagram mode
> > to _ndo_init(), which is called by register_netdev before it sends
> > the netlink message, would that suffice to eliminate the race?
> > (Sysfs files for {create,delete}_child go to _parent_init() then).
> > 
> 
> No, we can't, sorry for the noise. ndo_init() runs before the kobject
> becomes available.
> 
> Anyway, our sysfs attributes being racy is unrelated to the patch series
> subject, and I can't come up with any other ideas what to do with them
> that do not involve adjustments to register_netdev.

Agreed (that fixing the race issues is a different patch series).

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


signature.asc
Description: This is a digitally signed message part

Re: [PATCH v3 3/3] IB/ipoib: Log sysfs 'dev_id' accesses from userspace

[Doug Ledford]

On Fri, 2018-09-07 at 20:28 +0300, Leon Romanovsky wrote:
> On Fri, Sep 07, 2018 at 01:14:37PM -0400, Doug Ledford wrote:
> > On Thu, 2018-09-06 at 16:03 +0300, Leon Romanovsky wrote:
> > > On Thu, Sep 06, 2018 at 10:04:33AM +0300, Arseny Maslennikov wrote:
> > > > On Wed, Sep 05, 2018 at 04:50:35PM +0300, Leon Romanovsky wrote:
> > > > > On Mon, Sep 03, 2018 at 07:13:16PM +0300, Arseny Maslennikov wrote:
> > > > > > Signed-off-by: Arseny Maslennikov 
> > > > > > ---
> > > > > >  drivers/infiniband/ulp/ipoib/ipoib_main.c | 38 
> > > > > > +++
> > > > > >  1 file changed, 38 insertions(+)
> > > > > > 
> > > > > > diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c 
> > > > > > b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > > > > > index 30f840f874b3..7386e5bde3d3 100644
> > > > > > --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > > > > > +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > > > > > @@ -2386,6 +2386,42 @@ int ipoib_add_pkey_attr(struct net_device 
> > > > > > *dev)
> > > > > > return device_create_file(>dev, _attr_pkey);
> > > > > >  }
> > > > > > 
> > > > > > +/*
> > > > > > + * We erroneously exposed the iface's port number in the dev_id
> > > > > > + * sysfs field long after dev_port was introduced for that 
> > > > > > purpose[1],
> > > > > > + * and we need to stop everyone from relying on that.
> > > > > > + * Let's overload the shower routine for the dev_id file here
> > > > > > + * to gently bring the issue up.
> > > > > > + *
> > > > > > + * [1] https://www.spinics.net/lists/netdev/msg272123.html
> > > > > > + */
> > > > > > +static ssize_t dev_id_show(struct device *dev,
> > > > > > +  struct device_attribute *attr, char *buf)
> > > > > > +{
> > > > > > +   struct net_device *ndev = to_net_dev(dev);
> > > > > > +   ssize_t ret = -EINVAL;
> > > > > > +
> > > > > > +   if (ndev->dev_id == ndev->dev_port) {
> > > > > > +   netdev_info_once(ndev,
> > > > > > +   "\"%s\" wants to know my dev_id. "
> > > > > > +   "Should it look at dev_port instead?\n",
> > > > > > +   current->comm);
> > > > > > +   netdev_info_once(ndev,
> > > > > > +   "See Documentation/ABI/testing/sysfs-class-net 
> > > > > > for more info.\n");
> > > > > > +   }
> > > > > > +
> > > > > > +   ret = sprintf(buf, "%#x\n", ndev->dev_id);
> > > > > > +
> > > > > > +   return ret;
> > > > > > +}
> > > > > > +static DEVICE_ATTR_RO(dev_id);
> > > > > > +
> > > > > 
> > > > > I don't see this field among exposed by IPoIB, why should we expose 
> > > > > it now?
> > > > > 
> > > > 
> > > > To deviate from standard netdev behaviour, which only prints the
> > > > field out. Doug wanted this to also print a deprecation message, and
> > > > netdev (obviously) does not do that. See below.
> > > > 
> > > > > > +int ipoib_intercept_dev_id_attr(struct net_device *dev)
> > > > > > +{
> > > > > > +   device_remove_file(>dev, _attr_dev_id);
> > > > > > +   return device_create_file(>dev, _attr_dev_id);
> > > > > 
> > > > > Why isn't enough to rely on netdev code?
> > > > > 
> > > > 
> > > > Netdev code relies on macros around a *static* function 'netdev_show',
> > > > which is defined in net/core/net-sysfs.c; it is not listed in any header
> > > > files, and the macros aren't as well. This all leads me to believe it
> > > > was not really meant to be used from outside net/core/net-sysfs.
> > > > 
> > > > The only way we could use any netdev code here is to set up our own
> > > > handler (again), printk() a message, then call netdev_show — but we have
> > > > no access to it.
> > > > 
> > > > Of course, it also may be that I'm terribly missing a clue.
> > > 
> > > Thanks,
> > > 
> > > IMHO, the end result of adequate Doug's request is a little bit too much.
> > > I don't think that it justifies such remove->create construction.
> > > 
> > > Personal opinion.
> > 
> > I agree with you that the end result is kinda bulky, *but*, we will need
> > to know if there are things using the old dev_id before we can remove
> > it.  In particular, I'm concerned the IPoIB handling code of
> > NetworkManager uses it.  It's worth the cost I think.
> 
> I did my checks now and saw that ibdev2netdev uses the value provided
> from this dev_id, so every invocation of that script will generate the
> warning.
> 
> See this line in Parav's repo
> https://github.com/Mellanox/container_scripts/blob/master/ibdev2netdev#L112

I'm pretty sure libvirt + qemu will trigger it too.

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


signature.asc
Description: This is a digitally signed message part

Re: [PATCH v3 3/3] IB/ipoib: Log sysfs 'dev_id' accesses from userspace

[Doug Ledford]

On Thu, 2018-09-06 at 16:03 +0300, Leon Romanovsky wrote:
> On Thu, Sep 06, 2018 at 10:04:33AM +0300, Arseny Maslennikov wrote:
> > On Wed, Sep 05, 2018 at 04:50:35PM +0300, Leon Romanovsky wrote:
> > > On Mon, Sep 03, 2018 at 07:13:16PM +0300, Arseny Maslennikov wrote:
> > > > Signed-off-by: Arseny Maslennikov 
> > > > ---
> > > >  drivers/infiniband/ulp/ipoib/ipoib_main.c | 38 +++
> > > >  1 file changed, 38 insertions(+)
> > > > 
> > > > diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c 
> > > > b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > > > index 30f840f874b3..7386e5bde3d3 100644
> > > > --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > > > +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > > > @@ -2386,6 +2386,42 @@ int ipoib_add_pkey_attr(struct net_device *dev)
> > > > return device_create_file(>dev, _attr_pkey);
> > > >  }
> > > > 
> > > > +/*
> > > > + * We erroneously exposed the iface's port number in the dev_id
> > > > + * sysfs field long after dev_port was introduced for that purpose[1],
> > > > + * and we need to stop everyone from relying on that.
> > > > + * Let's overload the shower routine for the dev_id file here
> > > > + * to gently bring the issue up.
> > > > + *
> > > > + * [1] https://www.spinics.net/lists/netdev/msg272123.html
> > > > + */
> > > > +static ssize_t dev_id_show(struct device *dev,
> > > > +  struct device_attribute *attr, char *buf)
> > > > +{
> > > > +   struct net_device *ndev = to_net_dev(dev);
> > > > +   ssize_t ret = -EINVAL;
> > > > +
> > > > +   if (ndev->dev_id == ndev->dev_port) {
> > > > +   netdev_info_once(ndev,
> > > > +   "\"%s\" wants to know my dev_id. "
> > > > +   "Should it look at dev_port instead?\n",
> > > > +   current->comm);
> > > > +   netdev_info_once(ndev,
> > > > +   "See Documentation/ABI/testing/sysfs-class-net 
> > > > for more info.\n");
> > > > +   }
> > > > +
> > > > +   ret = sprintf(buf, "%#x\n", ndev->dev_id);
> > > > +
> > > > +   return ret;
> > > > +}
> > > > +static DEVICE_ATTR_RO(dev_id);
> > > > +
> > > 
> > > I don't see this field among exposed by IPoIB, why should we expose it 
> > > now?
> > > 
> > 
> > To deviate from standard netdev behaviour, which only prints the
> > field out. Doug wanted this to also print a deprecation message, and
> > netdev (obviously) does not do that. See below.
> > 
> > > > +int ipoib_intercept_dev_id_attr(struct net_device *dev)
> > > > +{
> > > > +   device_remove_file(>dev, _attr_dev_id);
> > > > +   return device_create_file(>dev, _attr_dev_id);
> > > 
> > > Why isn't enough to rely on netdev code?
> > > 
> > 
> > Netdev code relies on macros around a *static* function 'netdev_show',
> > which is defined in net/core/net-sysfs.c; it is not listed in any header
> > files, and the macros aren't as well. This all leads me to believe it
> > was not really meant to be used from outside net/core/net-sysfs.
> > 
> > The only way we could use any netdev code here is to set up our own
> > handler (again), printk() a message, then call netdev_show — but we have
> > no access to it.
> > 
> > Of course, it also may be that I'm terribly missing a clue.
> 
> Thanks,
> 
> IMHO, the end result of adequate Doug's request is a little bit too much.
> I don't think that it justifies such remove->create construction.
> 
> Personal opinion.

I agree with you that the end result is kinda bulky, *but*, we will need
to know if there are things using the old dev_id before we can remove
it.  In particular, I'm concerned the IPoIB handling code of
NetworkManager uses it.  It's worth the cost I think.

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


signature.asc
Description: This is a digitally signed message part

Re: [PATCH v4 3/3] IB/ipoib: Log sysfs 'dev_id' accesses from userspace

[Doug Ledford]

On Fri, 2018-09-07 at 09:43 -0600, Jason Gunthorpe wrote:
> On Thu, Sep 06, 2018 at 05:51:12PM +0300, Arseny Maslennikov wrote:
> > Some tools may currently be using only the deprecated attribute;
> > let's print an elaborate and clear deprecation notice to kmsg.
> > 
> > To do that, we have to replace the whole sysfs file, since we inherit
> > the original one from netdev.
> > 
> > Signed-off-by: Arseny Maslennikov 
> >  drivers/infiniband/ulp/ipoib/ipoib_main.c | 31 +++
> >  1 file changed, 31 insertions(+)
> > 
> > diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c 
> > b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > index 30f840f874b3..74732726ec6f 100644
> > +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > @@ -2386,6 +2386,35 @@ int ipoib_add_pkey_attr(struct net_device *dev)
> > return device_create_file(>dev, _attr_pkey);
> >  }
> >  
> > +/*
> > + * We erroneously exposed the iface's port number in the dev_id
> > + * sysfs field long after dev_port was introduced for that purpose[1],
> > + * and we need to stop everyone from relying on that.
> > + * Let's overload the shower routine for the dev_id file here
> > + * to gently bring the issue up.
> > + *
> > + * [1] https://www.spinics.net/lists/netdev/msg272123.html
> > + */
> > +static ssize_t dev_id_show(struct device *dev,
> > +  struct device_attribute *attr, char *buf)
> > +{
> > +   struct net_device *ndev = to_net_dev(dev);
> > +
> > +   if (ndev->dev_id == ndev->dev_port)
> > +   netdev_info_once(ndev,
> > +   "\"%s\" wants to know my dev_id. Should it look at 
> > dev_port instead? See Documentation/ABI/testing/sysfs-class-net for more 
> > info.\n",
> > +   current->comm);
> > +
> > +   return sprintf(buf, "%#x\n", ndev->dev_id);
> > +}
> > +static DEVICE_ATTR_RO(dev_id);
> > +
> > +int ipoib_intercept_dev_id_attr(struct net_device *dev)
> > +{
> > +   device_remove_file(>dev, _attr_dev_id);
> > +   return device_create_file(>dev, _attr_dev_id);
> > +}
> 
> Isn't this racey with userspace? Ie what happens if udev is querying
> the dev_id right here?
> 
> Do we know there is no userspace doing this?

I don't think that it can race (or reasonably can).  The intercept
function is done as part of ipoib_add_port() so the port itself isn't
live yet.  Things like udev shouldn't be scanning it until after we've
finished bringing it up and added it to the system, so any race here is
unimportant IMO.

> >  static struct net_device *ipoib_add_port(const char *format,
> >      struct ib_device *hca, u8 port)
> >  {
> > @@ -2427,6 +2456,8 @@ static struct net_device *ipoib_add_port(const char 
> > *format,
> >  */
> > ndev->priv_destructor = ipoib_intf_free;
> >  
> > +   if (ipoib_intercept_dev_id_attr(ndev))
> > +   goto sysfs_failed;
> 
> No device_remove_file needed?
> 
> Jason

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


signature.asc
Description: This is a digitally signed message part

Re: [PATCH v2 1/2] IB/ipoib: Use dev_port to expose network interface port numbers

[Doug Ledford]

On Fri, 2018-08-31 at 11:57 +0300, Arseny Maslennikov wrote:
> On Thu, Aug 30, 2018 at 04:17:58PM -0400, Doug Ledford wrote:
> > On Thu, 2018-08-30 at 21:22 +0300, Arseny Maslennikov wrote:
> > > Some InfiniBand network devices have multiple ports on the same PCI
> > > function. This initializes the `dev_port' sysfs field of those
> > > network interfaces with their port number.
> > > 
> > > Prior to this the kernel erroneously used the `dev_id' sysfs
> > > field of those network interfaces to convey the port number to userspace.
> > > 
> > > The use of `dev_id' was considered correct until Linux 3.15,
> > > when another field, `dev_port', was defined for this particular
> > > purpose and `dev_id' was reserved for distinguishing stacked ifaces
> > > (e.g: VLANs) with the same hardware address as their parent device.
> > > 
> > > Similar fixes to net/mlx4_en and many other drivers, which started
> > > exporting this information through `dev_id' before 3.15, were accepted
> > > into the kernel 4 years ago.
> > > See 76a066f2a2a0 (`net/mlx4_en: Expose port number through sysfs').
> > > 
> > > Signed-off-by: Arseny Maslennikov 
> > > ---
> > >  drivers/infiniband/ulp/ipoib/ipoib_main.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c 
> > > b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > > index e3d28f9ad9c0..ba16a63ee303 100644
> > > --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > > +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > > @@ -1880,7 +1880,7 @@ static int ipoib_parent_init(struct net_device 
> > > *ndev)
> > >  sizeof(union ib_gid));
> > >  
> > >   SET_NETDEV_DEV(priv->dev, priv->ca->dev.parent);
> > > - priv->dev->dev_id = priv->port - 1;
> > > + priv->dev->dev_port = priv->port - 1;
> > 
> > I don't know that we can't do this.  At least not yet.  Expose the new
> > item to make us compliant with the new docs, and deprecate the old sysfs
> > item, but we can't just yank the old item.  Existing tools/scripts might
> > (probably) rely on it (existing tools already special case IPoIB
> > interfaces and we'll need to make sure they don't special case this
> > element too).
> 
> I'm good with keeping both items for a (probably long) while to not break
> things. But how exactly should we notify users of the deprecation, so they
> don't special case this again? A comment in the code seems too little —
> everyone's obviously too busy to look there and stumble upon that.
> A distinct notice in the doc seems too much. I can't think of another place
> for the deprecation notice where people would take note of it, however.
> 
> Anyway: would it be OK to just restore both items and put a small note in
> dev_id's doc entry? If yes, I'll then send a v3.

A warn_on_once in the code so that when someone reads the dev_id entry,
we get a deprecation warning in the dmesg output at info level would be
my suggestion.  Have it output the command name as part of the warning
so we know what tools are using it.

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [PATCH v2 1/2] IB/ipoib: Use dev_port to expose network interface port numbers

[Doug Ledford]

On Thu, 2018-08-30 at 21:22 +0300, Arseny Maslennikov wrote:
> Some InfiniBand network devices have multiple ports on the same PCI
> function. This initializes the `dev_port' sysfs field of those
> network interfaces with their port number.
> 
> Prior to this the kernel erroneously used the `dev_id' sysfs
> field of those network interfaces to convey the port number to userspace.
> 
> The use of `dev_id' was considered correct until Linux 3.15,
> when another field, `dev_port', was defined for this particular
> purpose and `dev_id' was reserved for distinguishing stacked ifaces
> (e.g: VLANs) with the same hardware address as their parent device.
> 
> Similar fixes to net/mlx4_en and many other drivers, which started
> exporting this information through `dev_id' before 3.15, were accepted
> into the kernel 4 years ago.
> See 76a066f2a2a0 (`net/mlx4_en: Expose port number through sysfs').
> 
> Signed-off-by: Arseny Maslennikov 
> ---
>  drivers/infiniband/ulp/ipoib/ipoib_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c 
> b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> index e3d28f9ad9c0..ba16a63ee303 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> @@ -1880,7 +1880,7 @@ static int ipoib_parent_init(struct net_device *ndev)
>  sizeof(union ib_gid));
>  
>   SET_NETDEV_DEV(priv->dev, priv->ca->dev.parent);
> - priv->dev->dev_id = priv->port - 1;
> + priv->dev->dev_port = priv->port - 1;

I don't know that we can't do this.  At least not yet.  Expose the new
item to make us compliant with the new docs, and deprecate the old sysfs
item, but we can't just yank the old item.  Existing tools/scripts might
(probably) rely on it (existing tools already special case IPoIB
interfaces and we'll need to make sure they don't special case this
element too).

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [PATCH V2 mlx5-next 0/2] Mellanox, mlx5 new device events

[Doug Ledford]

On Fri, 2018-06-01 at 11:45 -0400, David Miller wrote:
> From: Doug Ledford 
> Date: Fri, 01 Jun 2018 11:08:24 -0400
> 
> > On Thu, 2018-05-31 at 15:36 -0400, David Miller wrote:
> >> From: Saeed Mahameed 
> >> Date: Wed, 30 May 2018 10:59:48 -0700
> >> 
> >> > The following series is for mlx5-next tree [1], it adds the support of 
> >> > two
> >> > new device events, from Ilan Tayari:
> >> > 
> >> > 1. High temperature warnings.
> >> > 2. FPGA QP error event.
> >> > 
> >> > In case of no objection this series will be applied to mlx5-next tree
> >> > and will be sent later as a pull request to both rdma and net trees.
> >> > 
> >> > [1] 
> >> > https://git.kernel.org/pub/scm/linux/kernel/git/mellanox/linux.git/log/?h=mlx5-next
> >> > 
> >> > v1->v2:
> >> >   - improve commit message of the FPGA QP error event patch.
> >> 
> >> Series applied, thanks.
> > 
> > Hi Dave,
> > 
> > Although in this case it doesn't really matter and we can work around
> > it, this was supposed to be a case of the new methodology that Saeed and
> > Jason had worked out with you.  Specifically, when Saeed says in the
> > cover letter:
> > 
> >> In case of no objection this series will be applied to mlx5-next tree>
> >> and will be sent later as a pull request to both rdma and net trees.
> > 
> > then it is intended for you to ack the original patch series, not apply
> > it, and when acks from both the net and rdma side have been received,
> > then we will get a pull request of just that series.
> 
> Sorry, I saw your ACK and misinterpreted the situation.
> 
> I'll be more careful next time.

Understandable, thanks ;-)

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [PATCH V2 mlx5-next 0/2] Mellanox, mlx5 new device events

[Doug Ledford]

On Fri, 2018-06-01 at 19:21 +0300, Leon Romanovsky wrote:
> On Fri, Jun 01, 2018 at 11:45:58AM -0400, David Miller wrote:
> > From: Doug Ledford 
> > Date: Fri, 01 Jun 2018 11:08:24 -0400
> > 
> > > On Thu, 2018-05-31 at 15:36 -0400, David Miller wrote:
> > > > From: Saeed Mahameed 
> > > > Date: Wed, 30 May 2018 10:59:48 -0700
> > > > 
> > > > > The following series is for mlx5-next tree [1], it adds the support 
> > > > > of two
> > > > > new device events, from Ilan Tayari:
> > > > > 
> > > > > 1. High temperature warnings.
> > > > > 2. FPGA QP error event.
> > > > > 
> > > > > In case of no objection this series will be applied to mlx5-next tree
> > > > > and will be sent later as a pull request to both rdma and net trees.
> > > > > 
> > > > > [1] 
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/mellanox/linux.git/log/?h=mlx5-next
> > > > > 
> > > > > v1->v2:
> > > > >   - improve commit message of the FPGA QP error event patch.
> > > > 
> > > > Series applied, thanks.
> > > 
> > > Hi Dave,
> > > 
> > > Although in this case it doesn't really matter and we can work around
> > > it, this was supposed to be a case of the new methodology that Saeed and
> > > Jason had worked out with you.  Specifically, when Saeed says in the
> > > cover letter:
> > > 
> > > > In case of no objection this series will be applied to mlx5-next tree>
> > > > and will be sent later as a pull request to both rdma and net trees.
> > > 
> > > then it is intended for you to ack the original patch series, not apply
> > > it, and when acks from both the net and rdma side have been received,
> > > then we will get a pull request of just that series.
> > 
> > Sorry, I saw your ACK and misinterpreted the situation.
> > 
> > I'll be more careful next time.
> 
> Doug, Dave
> 
> I would like to clarify this point, we intend to send pull request to only
> one maintainer, who actually needs the accepted patches.
> 
> Let's take the RDMA flow counters series as an example:
> https://www.spinics.net/lists/linux-rdma/msg65620.html
> 
> This series includes 12 patches for RDMA and 2 shared code
> (mlx5-next). Those two patches, the RDMA side will receive as part
> of specific pull request and netdev will get them if some netdev
> feature down the road will need mlx5-next branch.
> 
> Of course, it is harmless for both of you to pull, but it looks like
> extra work which is not needed for you.

Ok, thanks for the clarification Leon.


-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [PATCH V2 mlx5-next 0/2] Mellanox, mlx5 new device events

[Doug Ledford]

On Thu, 2018-05-31 at 15:36 -0400, David Miller wrote:
> From: Saeed Mahameed 
> Date: Wed, 30 May 2018 10:59:48 -0700
> 
> > The following series is for mlx5-next tree [1], it adds the support of two
> > new device events, from Ilan Tayari:
> > 
> > 1. High temperature warnings.
> > 2. FPGA QP error event.
> > 
> > In case of no objection this series will be applied to mlx5-next tree
> > and will be sent later as a pull request to both rdma and net trees.
> > 
> > [1] 
> > https://git.kernel.org/pub/scm/linux/kernel/git/mellanox/linux.git/log/?h=mlx5-next
> > 
> > v1->v2:
> >   - improve commit message of the FPGA QP error event patch.
> 
> Series applied, thanks.

Hi Dave,

Although in this case it doesn't really matter and we can work around
it, this was supposed to be a case of the new methodology that Saeed and
Jason had worked out with you.  Specifically, when Saeed says in the
cover letter:

> In case of no objection this series will be applied to mlx5-next tree>
> and will be sent later as a pull request to both rdma and net trees.

then it is intended for you to ack the original patch series, not apply
it, and when acks from both the net and rdma side have been received,
then we will get a pull request of just that series.

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [PATCH V2 mlx5-next 0/2] Mellanox, mlx5 new device events

[Doug Ledford]

On Wed, 2018-05-30 at 10:59 -0700, Saeed Mahameed wrote:
> Hi, 
> 
> The following series is for mlx5-next tree [1], it adds the support of two
> new device events, from Ilan Tayari:
> 
> 1. High temperature warnings.
> 2. FPGA QP error event.
> 
> In case of no objection this series will be applied to mlx5-next tree
> and will be sent later as a pull request to both rdma and net trees.
> 
> [1] 
> https://git.kernel.org/pub/scm/linux/kernel/git/mellanox/linux.git/log/?h=mlx5-next
> 
> v1->v2:
>   - improve commit message of the FPGA QP error event patch.
> 
> Thanks,
> Saeed.
> 
> Ilan Tayari (2):
>   net/mlx5: Add temperature warning event to log
>   net/mlx5: Add FPGA QP error event
> 
>  drivers/net/ethernet/mellanox/mlx5/core/eq.c | 28 +++-
>  include/linux/mlx5/device.h  |  8 ++
>  include/linux/mlx5/mlx5_ifc.h|  3 ++-
>  include/linux/mlx5/mlx5_ifc_fpga.h   | 16 +++
>  4 files changed, 53 insertions(+), 2 deletions(-)
> 

For the RDMA community, series ack:

Acked-by: Doug Ledford 

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [pull request][for-next 00/15] Mellanox, mlx5 core and netdev updates 2018-05-17

[Doug Ledford]

On Fri, 2018-05-18 at 20:33 +, Saeed Mahameed wrote:
> On Fri, 2018-05-18 at 13:03 -0400, David Miller wrote:
> > From: Saeed Mahameed <sae...@mellanox.com>
> > Date: Thu, 17 May 2018 18:22:43 -0700
> > 
> > > Below you can find two pull requests,
> > > 
> > > 1. mlx5 core updates to be shared for both netdev and RDMA,
> > > (patches 1..9)
> > >  which is based on the last mlx5-next pull request
> > >  
> > > The following changes since commit
> > > a8408f4e6db775e245f20edf12b13fd58cc03a1c:
> > > 
> > >   net/mlx5: fix spelling mistake: "modfiy" -> "modify" (2018-05-04
> > > 12:11:51 -0700)
> > > 
> > > are available in the Git repository at:
> > > 
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/mellanox/linux.git
> > > tags/mlx5-updates-2018-05-17
> > > 
> > > for you to fetch changes up to
> > > 10ff5359f883412728ba816046ee3a696625ca02:
> > > 
> > >   net/mlx5e: Explicitly set source e-switch in offloaded TC rules
> > > (2018-05-17 14:17:35 -0700)
> > > 
> > > 2. mlx5e netdev updates only for net-next branch (patches 10..15)
> > > based on net-next
> > > and the above pull request.
> > > 
> > > The following changes since commit
> > > 538e2de104cfb4ef1acb35af42427bff42adbe4d:
> > > 
> > >   Merge branch 'net-Allow-more-drivers-with-COMPILE_TEST' (2018-05-
> > > 17 17:11:07 -0400)
> > > 
> > > are available in the Git repository at:
> > > 
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux.git
> > > tags/mlx5e-updates-2018-05-17
> > > 
> > > for you to fetch changes up to
> > > a228060a7c9ab88597eeac131e4578595d5d46ae:
> > > 
> > >   net/mlx5e: Add HW vport counters to representor ethtool stats
> > > (2018-05-17 17:48:54 -0700)
> > > 
> > > Dave, for your convenience you can either pull 1. and then 2. or
> > > pull 2.
> > > directly.
> > 
> > Looks good.
> > 
> > I pulled 1 then I pulled 2.  That seemed to work well.  Particularly
> > it allowed me to capture the two different merge commit messages one
> > by one.
> > 
> > Is this basically how you want to handle things moving forward?
> > 
> 
> Thanks Dave !
> Basically yes, we want to avoid sending netdev related patches to rdma,
> and vice versa.
> Unlike the previous "shared code" procedure, this is a more natural way
> to do things, since the mlx5 core is a shared arena, we want to
> maintain it separate from netdev and rdma.
> 
> Before, Leon and I needed to sync before each release and create a
> "shared code" pull requests that includes everything (core/rdma/netdev)
> that was conflicting in the core arena.

I've pulled tag #1.  Thanks.

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [PATCH v2 iproute2-next 1/3] rdma: update rdma_netlink.h to get new driver attributes

[Doug Ledford]

On Tue, 2018-05-15 at 15:28 -0500, Steve Wise wrote:
> > -Original Message-
> > From: linux-rdma-ow...@vger.kernel.org  > ow...@vger.kernel.org> On Behalf Of Doug Ledford
> > Sent: Tuesday, May 15, 2018 2:53 PM
> > To: David Ahern <dsah...@gmail.com>; Steve Wise
> > <sw...@opengridcomputing.com>; l...@kernel.org
> > Cc: step...@networkplumber.org; netdev@vger.kernel.org; linux-
> > r...@vger.kernel.org
> > Subject: Re: [PATCH v2 iproute2-next 1/3] rdma: update rdma_netlink.h to
> > get new driver attributes
> > 
> > On Tue, 2018-05-15 at 13:37 -0600, David Ahern wrote:
> > > On 5/14/18 9:42 AM, Steve Wise wrote:
> > > > diff --git a/rdma/include/uapi/rdma/rdma_netlink.h
> > 
> > b/rdma/include/uapi/rdma/rdma_netlink.h
> > > > index 60416ed..40be0d8 100644
> > > > --- a/rdma/include/uapi/rdma/rdma_netlink.h
> > > > +++ b/rdma/include/uapi/rdma/rdma_netlink.h
> > > > 
> > > > @@ -387,6 +399,20 @@ enum rdma_nldev_attr {
> > > > RDMA_NLDEV_ATTR_RES_PD_ENTRY,   /* nested table */
> > > > RDMA_NLDEV_ATTR_RES_LOCAL_DMA_LKEY, /* u32 */
> > > > RDMA_NLDEV_ATTR_RES_UNSAFE_GLOBAL_RKEY, /* u32 */
> > > > +   /*
> > > > +* driver-specific attributes.
> > > > +*/
> > > > +   RDMA_NLDEV_ATTR_DRIVER, /* nested table */
> > > > +   RDMA_NLDEV_ATTR_DRIVER_ENTRY,   /* nested table */
> > > > +   RDMA_NLDEV_ATTR_DRIVER_STRING,  /* string */
> > > > +   /*
> > > > +* u8 values from enum rdma_nldev_print_type
> > > > +*/
> > > > +   RDMA_NLDEV_ATTR_DRIVER_PRINT_TYPE,  /* u8 */
> > > > +   RDMA_NLDEV_ATTR_DRIVER_S32, /* s32 */
> > > > +   RDMA_NLDEV_ATTR_DRIVER_U32, /* u32 */
> > > > +   RDMA_NLDEV_ATTR_DRIVER_S64, /* s64 */
> > > > +   RDMA_NLDEV_ATTR_DRIVER_U64, /* u64 */
> > > 
> > > and again here.
> > 
> > This chunk, however, is a problem.  We'll need to fix that in the kernel
> > and in this patch too.
> 
> I'll fix this series once I fix the kernel side.  Doug, should I send a patch 
> that basically moves the DRIVER attributes to the bottom?
> 
> Sorry about missing this!

I missed it too.  I knew about the first hunk, but thought this one was
at the end.  Anyway, I've already pushed the fix to my wip/dl-for-next
branch, so you can get it there now.

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [PATCH v2 iproute2-next 1/3] rdma: update rdma_netlink.h to get new driver attributes

[Doug Ledford]

On Tue, 2018-05-15 at 14:01 -0600, David Ahern wrote:
> On 5/15/18 2:00 PM, Doug Ledford wrote:
> > I just sent an incremental fix to the list under separate cover.  You
> > can squash that fix into Steve's patch and it should resolve the issue. 
> > Or Steve can respin the set.  Either way.
> 
> Once the patch has been committed to the upstream repo, spin a new set
> with the correct header update.

Well, I wrote it, so it's in my repo now ;-).  I just sent it to the
list because that's what you do when you are writing fixup patches even
if you've already committed them.

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [PATCH v2 iproute2-next 1/3] rdma: update rdma_netlink.h to get new driver attributes

[Doug Ledford]

On Tue, 2018-05-15 at 13:53 -0600, David Ahern wrote:
> sure for the above, but not the ones below.
> 
> Before this patch, this program:
> $ cat a.c
> #include 
> #include 
> 
> int main(void)
> {
> printf("RDMA_NLDEV_ATTR_NDEV_INDEX = %d\n", 
> RDMA_NLDEV_ATTR_NDEV_INDEX);
> 
> return 0;
> }
> 
> prints this:
> $ ./a.out
> RDMA_NLDEV_ATTR_NDEV_INDEX = 50
> 
> After this patch,
> $ ./a.out
> RDMA_NLDEV_ATTR_NDEV_INDEX = 58

I just sent an incremental fix to the list under separate cover.  You
can squash that fix into Steve's patch and it should resolve the issue. 
Or Steve can respin the set.  Either way.

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [PATCH v2 iproute2-next 1/3] rdma: update rdma_netlink.h to get new driver attributes

[Doug Ledford]

On Tue, 2018-05-15 at 13:37 -0600, David Ahern wrote:
> On 5/14/18 9:42 AM, Steve Wise wrote:
> > diff --git a/rdma/include/uapi/rdma/rdma_netlink.h 
> > b/rdma/include/uapi/rdma/rdma_netlink.h
> > index 60416ed..40be0d8 100644
> > --- a/rdma/include/uapi/rdma/rdma_netlink.h
> > +++ b/rdma/include/uapi/rdma/rdma_netlink.h
> > 
> > @@ -387,6 +399,20 @@ enum rdma_nldev_attr {
> > RDMA_NLDEV_ATTR_RES_PD_ENTRY,   /* nested table */
> > RDMA_NLDEV_ATTR_RES_LOCAL_DMA_LKEY, /* u32 */
> > RDMA_NLDEV_ATTR_RES_UNSAFE_GLOBAL_RKEY, /* u32 */
> > +   /*
> > +* driver-specific attributes.
> > +*/
> > +   RDMA_NLDEV_ATTR_DRIVER, /* nested table */
> > +   RDMA_NLDEV_ATTR_DRIVER_ENTRY,   /* nested table */
> > +   RDMA_NLDEV_ATTR_DRIVER_STRING,  /* string */
> > +   /*
> > +* u8 values from enum rdma_nldev_print_type
> > +*/
> > +   RDMA_NLDEV_ATTR_DRIVER_PRINT_TYPE,  /* u8 */
> > +   RDMA_NLDEV_ATTR_DRIVER_S32, /* s32 */
> > +   RDMA_NLDEV_ATTR_DRIVER_U32, /* u32 */
> > +   RDMA_NLDEV_ATTR_DRIVER_S64, /* s64 */
> > +   RDMA_NLDEV_ATTR_DRIVER_U64,         /* u64 */
> 
> and again here.

This chunk, however, is a problem.  We'll need to fix that in the kernel
and in this patch too.

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [PATCH v2 iproute2-next 1/3] rdma: update rdma_netlink.h to get new driver attributes

[Doug Ledford]

On Tue, 2018-05-15 at 13:37 -0600, David Ahern wrote:
> On 5/14/18 9:42 AM, Steve Wise wrote:
> > diff --git a/rdma/include/uapi/rdma/rdma_netlink.h 
> > b/rdma/include/uapi/rdma/rdma_netlink.h
> > index 60416ed..40be0d8 100644
> > --- a/rdma/include/uapi/rdma/rdma_netlink.h
> > +++ b/rdma/include/uapi/rdma/rdma_netlink.h
> > @@ -249,10 +249,22 @@ enum rdma_nldev_command {
> > RDMA_NLDEV_NUM_OPS
> >  };
> >  
> > +enum {
> > +   RDMA_NLDEV_ATTR_ENTRY_STRLEN = 16,
> > +};
> > +
> > +enum rdma_nldev_print_type {
> > +   RDMA_NLDEV_PRINT_TYPE_UNSPEC,
> > +   RDMA_NLDEV_PRINT_TYPE_HEX,
> > +};
> > +
> >  enum rdma_nldev_attr {
> > /* don't change the order or add anything between, this is ABI! */
> 
> I asked this before and did not get a response. As the comment above
> states with an emphasis (!) ...
> 
> > RDMA_NLDEV_ATTR_UNSPEC,
> >  
> > +   /* Pad attribute for 64b alignment */
> > +   RDMA_NLDEV_ATTR_PAD = RDMA_NLDEV_ATTR_UNSPEC,
> > +
> 
> ... are you really adding new attributes in the middle?

Not really.  The new item is being explicitly set to the same value as
the item above it.  It therefore becomes two entries with the same enum
value.  The rest of the enum is all unchanged.

> > /* Identifier for ib_device */
> > RDMA_NLDEV_ATTR_DEV_INDEX,  /* u32 */
> >  
> > @@ -387,6 +399,20 @@ enum rdma_nldev_attr {
> > RDMA_NLDEV_ATTR_RES_PD_ENTRY,   /* nested table */
> > RDMA_NLDEV_ATTR_RES_LOCAL_DMA_LKEY, /* u32 */
> > RDMA_NLDEV_ATTR_RES_UNSAFE_GLOBAL_RKEY, /* u32 */
> > +   /*
> > +* driver-specific attributes.
> > +*/
> > +   RDMA_NLDEV_ATTR_DRIVER, /* nested table */
> > +   RDMA_NLDEV_ATTR_DRIVER_ENTRY,   /* nested table */
> > +   RDMA_NLDEV_ATTR_DRIVER_STRING,  /* string */
> > +   /*
> > +* u8 values from enum rdma_nldev_print_type
> > +*/
> > +   RDMA_NLDEV_ATTR_DRIVER_PRINT_TYPE,  /* u8 */
> > +   RDMA_NLDEV_ATTR_DRIVER_S32, /* s32 */
> > +   RDMA_NLDEV_ATTR_DRIVER_U32, /* u32 */
> > +   RDMA_NLDEV_ATTR_DRIVER_S64, /* s64 */
> > +   RDMA_NLDEV_ATTR_DRIVER_U64, /* u64 */
> 
> and again here.
> 
> >  
> > /*
> >  * Provides logical name and index of netdevice which is
> > 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [PATCH v1 iproute2-next 2/3] rdma: print driver resource attributes

[Doug Ledford]

On Tue, 2018-05-15 at 19:59 +0300, Leon Romanovsky wrote:
> On Tue, May 15, 2018 at 12:35:34PM -0400, Doug Ledford wrote:
> > On Mon, 2018-05-14 at 09:51 -0500, Steve Wise wrote:
> > > 
> > > On 5/13/2018 8:24 AM, Leon Romanovsky wrote:
> > > > On Mon, May 07, 2018 at 08:53:16AM -0700, Steve Wise wrote:
> > > > > This enhancement allows printing rdma device-specific state, if 
> > > > > provided
> > > > > by the kernel.  This is done in a generic manner, so rdma tool doesn't
> > > > 
> > > > Double space between "." and "This".
> > > > 
> > > > > need to know about the details of every type of rdma device.
> > > > > 
> > > > > Driver attributes for a rdma resource are in the form of <key,
> > > > > [print_type], value> tuples, where the key is a string and the value 
> > > > > can
> > > > > be any supported driver attribute.  The print_type attribute, if 
> > > > > present,
> > > > 
> > > > ditto
> > > 
> > > I'll fix these.
> > 
> > Fix it if you want, but don't do it because Leon told you to.  A double
> > space after period is perfectly acceptable.
> 
> It is very controversial thing [1],
> 
> "Most style guides indicate that single sentence spacing is proper for
> final or published work today, and most publishers require manuscripts
> to be submitted as they will appear in publication—single
> sentence spaced."
> 
> [1] https://en.wikipedia.org/wiki/Sentence_spacing

Yes...and the justification is that proportional font systems resolve
the issue of delineating sentences without the need for a second space
(they actually don't because they are improperly implemented, but that's
another issue).  But we don't work on proportional font systems, we work
with git, on command lines, with fixed spacing fonts, all the things
that indicate double spaces are actually preferred even by those people
that are proponents of single spacing.  And on top of that, current
research suggests maybe the single spacers are wrong after all :-o

https://www.theatlantic.com/science/archive/2018/05/two-spaces-after-a-p
eriod/559304/

Anyway, my original point stands.  It's controversial, as you said, so
Steve should do what Steve feels is best and not worry about anyone
else' opinion on the matter.

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [PATCH v1 iproute2-next 2/3] rdma: print driver resource attributes

[Doug Ledford]

On Mon, 2018-05-14 at 09:51 -0500, Steve Wise wrote:
> 
> On 5/13/2018 8:24 AM, Leon Romanovsky wrote:
> > On Mon, May 07, 2018 at 08:53:16AM -0700, Steve Wise wrote:
> > > This enhancement allows printing rdma device-specific state, if provided
> > > by the kernel.  This is done in a generic manner, so rdma tool doesn't
> > 
> > Double space between "." and "This".
> > 
> > > need to know about the details of every type of rdma device.
> > > 
> > > Driver attributes for a rdma resource are in the form of <key,
> > > [print_type], value> tuples, where the key is a string and the value can
> > > be any supported driver attribute.  The print_type attribute, if present,
> > 
> > ditto
> 
> I'll fix these.

Fix it if you want, but don't do it because Leon told you to.  A double
space after period is perfectly acceptable.

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [pull request][for-next 0/6] Mellanox, mlx5 updates 2018-05-07

[Doug Ledford]

On Thu, 2018-05-10 at 08:18 -0400, David Miller wrote:
> From: Saeed Mahameed <sae...@mellanox.com>
> Date: Mon,  7 May 2018 16:52:58 -0700
> 
> > This pull request includes misc updates and cleanups for mlx5 core
> > driver for both net and rdma next branches, for more information please
> > see tag log below.
> > 
> > Please pull and let me know if there's any problem.
> 
> Looks good, pulled into net-next.
> 
> Thanks.

Thanks, pulled into rdma-next.

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [PATCH rdma-next] MAINTAINERS: Remove bouncing @mellanox.com addresses

[Doug Ledford]

On Thu, 2018-05-03 at 21:37 +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leo...@mellanox.com>
> 
> Delete non-existent @mellanox.com addresses from MAINTAINERS file.
> 
> Signed-off-by: Leon Romanovsky <leo...@mellanox.com>

Thanks, applied to for-rc.

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [PATCH rdma-next] MAINTAINERS: Remove bouncing @mellanox.com addresses

[Doug Ledford]

On 5/3/2018 5:11 PM, Or Gerlitz wrote:
> On Thu, May 3, 2018 at 9:37 PM, LR wrote:
> 
>>  MELLANOX MLX5 core VPI driver
>>  M: Saeed Mahameed 
>> -M: Matan Barak 
> 
> Goodbye Matan!
> 
> You were a long time developer, maintainer, hacker and a very deeply thinking,
> pleasant, nice and open person in our team, enjoy your new adventures and 
> thanks
> a lot for your long time contributions to the upstream kernel

Indeed, Matan was always a pleasure to work with.  Best of luck on
whatever you are doing next!




signature.asc
Description: OpenPGP digital signature

Re: [PATCH 1/1] IB/rxe: avoid double kfree_skb

[Doug Ledford]

On Wed, 2018-04-25 at 14:56 +0800, Yanjun Zhu wrote:
> Hi, all
> 
> rxe_send [rdma_rxe]
>  ip_local_out
>  __ip_local_out
>  ip_output
>  ip_finish_output
>  ip_finish_output2
>  dev_queue_xmit
>  __dev_queue_xmit
>  dev_hard_start_xmit
>  e1000_xmit_frame [e1000]
> 
> When skb is sent, it will pass the above functions. I checked all the 
> above functions. If error occurs in the above functions after 
> ip_local_out, kfree_skb will be called.
> So when ip_local_out returns an error, skb should be freed. It is not 
> necessary to call kfree_skb in soft roce module again.
> 
> If I am wrong, please correct me.

No one from netdev has spoken up, and I don't see where you are wrong,
so I've applied this to for-rc.  Thanks.

> Zhu Yanjun
> On 2018/4/24 16:34, Yanjun Zhu wrote:
> > Hi, all
> > 
> > rxe_send
> > ip_local_out
> > __ip_local_out
> > nf_hook_slow
> > 
> > In the above call process, nf_hook_slow drops and frees skb, then 
> > -EPERM is returned when iptables rules(iptables -I OUTPUT -p udp 
> > --dport 4791 -j DROP) is set.
> > 
> > If skb->users is not changed in softroce, kfree_skb should not be 
> > called in this module.
> > 
> > I will make further investigations about other error handler after 
> > ip_local_out.
> > If I am wrong, please correct me.
> > 
> > Any reply is appreciated.
> > 
> > Zhu Yanjun
> > On 2018/4/20 13:46, Yanjun Zhu wrote:
> > > 
> > > 
> > > On 2018/4/20 10:19, Doug Ledford wrote:
> > > > On Thu, 2018-04-19 at 10:01 -0400, Zhu Yanjun wrote:
> > > > > When skb is dropped by iptables rules, the skb is freed at the same 
> > > > > time
> > > > > -EPERM is returned. So in softroce, it is not necessary to free skb 
> > > > > again.
> > > > > Or else, crash will occur.
> > > > > 
> > > > > The steps to reproduce:
> > > > > 
> > > > >   server   client
> > > > >  --
> > > > >  |1.1.1.1|<rxe-channel--->|1.1.1.2|
> > > > >  --
> > > > > 
> > > > > On server: rping -s -a 1.1.1.1 -v -C 1 -S 512
> > > > > On client: rping -c -a 1.1.1.1 -v -C 1 -S 512
> > > > > 
> > > > > The kernel configs CONFIG_DEBUG_KMEMLEAK and
> > > > > CONFIG_DEBUG_OBJECTS are enabled on both server and client.
> > > > > 
> > > > > When rping runs, run the following command in server:
> > > > > 
> > > > > iptables -I OUTPUT -p udp  --dport 4791 -j DROP
> > > > > 
> > > > > Without this patch, crash will occur.
> > > > > 
> > > > > CC: Srinivas Eeda <srinivas.e...@oracle.com>
> > > > > CC: Junxiao Bi <junxiao...@oracle.com>
> > > > > Signed-off-by: Zhu Yanjun <yanjun@oracle.com>
> > > > > Reviewed-by: Yuval Shaia <yuval.sh...@oracle.com>
> > > > 
> > > > I have no reason to doubt your analysis, but if there are a bunch of
> > > > error paths for net_xmit and they all return with your skb still being
> > > > valid and holding a reference, and then one oddball that returns with
> > > > your skb already gone, that just sounds like a mistake waiting to 
> > > > happen
> > > > (not to mention a bajillion special cases sprinkled everywhere to deal
> > > > with this apparent inconsistency).
> > > > 
> > > > Can we get a netdev@ confirmation on this being the right solution?
> > > 
> > > Yes. I agree with you.
> > > After iptables rule "iptables -I OUTPUT -p udp  --dport 4791 -j 
> > > DROP", the skb is freed in this function
> > > 
> > > /* Returns 1 if okfn() needs to be executed by the caller,
> > >  * -EPERM for NF_DROP, 0 otherwise.  Caller must hold rcu_read_lock. */
> > > int nf_hook_slow(struct sk_buff *skb, struct nf_hook_state *state,
> > >  const struct nf_hook_entries *e, unsigned int s)
> > > {
> > > unsigned int verdict;
> > > int ret;
> > > 
> > > for (; s < e->num_hook_entri

Re: [PATCH for-rc] uapi: Fix SPDX tags for files referring to the 'OpenIB.org' license

[Doug Ledford]

On Sun, 2018-04-22 at 21:08 -0400, David Miller wrote:
> From: Jason Gunthorpe <j...@mellanox.com>
> Date: Fri, 20 Apr 2018 09:49:10 -0600
> 
> > Based on discussion with Kate Stewart this license is not a
> > BSD-2-Clause, but is now formally identified as Linux-OpenIB
> > by SPDX.
> > 
> > The key difference between the licenses is in the 'warranty'
> > paragraph.
> > 
> > if_infiniband.h refers to the 'OpenIB.org' license, but
> > does not include the text, instead it links to an obsolete
> > web site that contains a license that matches the BSD-2-Clause
> > SPX. There is no 'three clause' version of the OpenIB.org
> > license.
> > 
> > Signed-off-by: Jason Gunthorpe <j...@mellanox.com>
> 
> Acked-by: David S. Miller <da...@davemloft.net>
> 

Thanks, applied.

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [PATCH 1/1] IB/rxe: avoid double kfree_skb

[Doug Ledford]

On Thu, 2018-04-19 at 10:01 -0400, Zhu Yanjun wrote:
> When skb is dropped by iptables rules, the skb is freed at the same time
> -EPERM is returned. So in softroce, it is not necessary to free skb again.
> Or else, crash will occur.
> 
> The steps to reproduce:
> 
>  server   client
> --
> |1.1.1.1|<rxe-channel--->|1.1.1.2|
> --
> 
> On server: rping -s -a 1.1.1.1 -v -C 1 -S 512
> On client: rping -c -a 1.1.1.1 -v -C 1 -S 512
> 
> The kernel configs CONFIG_DEBUG_KMEMLEAK and
> CONFIG_DEBUG_OBJECTS are enabled on both server and client.
> 
> When rping runs, run the following command in server:
> 
> iptables -I OUTPUT -p udp  --dport 4791 -j DROP
> 
> Without this patch, crash will occur.
> 
> CC: Srinivas Eeda <srinivas.e...@oracle.com>
> CC: Junxiao Bi <junxiao...@oracle.com>
> Signed-off-by: Zhu Yanjun <yanjun@oracle.com>
> Reviewed-by: Yuval Shaia <yuval.sh...@oracle.com>

I have no reason to doubt your analysis, but if there are a bunch of
error paths for net_xmit and they all return with your skb still being
valid and holding a reference, and then one oddball that returns with
your skb already gone, that just sounds like a mistake waiting to happen
(not to mention a bajillion special cases sprinkled everywhere to deal
with this apparent inconsistency).

Can we get a netdev@ confirmation on this being the right solution?

> ---
>  drivers/infiniband/sw/rxe/rxe_net.c  | 3 +++
>  drivers/infiniband/sw/rxe/rxe_req.c  | 5 +++--
>  drivers/infiniband/sw/rxe/rxe_resp.c | 9 ++---
>  3 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c 
> b/drivers/infiniband/sw/rxe/rxe_net.c
> index 9da6e37..2094434 100644
> --- a/drivers/infiniband/sw/rxe/rxe_net.c
> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> @@ -511,6 +511,9 @@ int rxe_send(struct rxe_pkt_info *pkt, struct sk_buff 
> *skb)
>  
> if (unlikely(net_xmit_eval(err))) {
> pr_debug("error sending packet: %d\n", err);
> +   /* -EPERM means the skb is dropped and freed. */
> +   if (err == -EPERM)
> +   return -EPERM;
> return -EAGAIN;
> }
>  
> diff --git a/drivers/infiniband/sw/rxe/rxe_req.c 
> b/drivers/infiniband/sw/rxe/rxe_req.c
> index 7bdaf71..9d2efec 100644
> --- a/drivers/infiniband/sw/rxe/rxe_req.c
> +++ b/drivers/infiniband/sw/rxe/rxe_req.c
> @@ -727,8 +727,9 @@ int rxe_requester(void *arg)
>  
> rollback_state(wqe, qp, _wqe, rollback_psn);
>  
> -   if (ret == -EAGAIN) {
> -   kfree_skb(skb);
> +   if ((ret == -EAGAIN) || (ret == -EPERM)) {
> +   if (ret == -EAGAIN)
> +   kfree_skb(skb);
> rxe_run_task(>req.task, 1);
> goto exit;
> }
> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c 
> b/drivers/infiniband/sw/rxe/rxe_resp.c
> index a65c996..6bdf9b2 100644
> --- a/drivers/infiniband/sw/rxe/rxe_resp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
> @@ -742,7 +742,8 @@ static enum resp_states read_reply(struct rxe_qp *qp,
> err = rxe_xmit_packet(rxe, qp, _pkt, skb);
> if (err) {
> pr_err("Failed sending RDMA reply.\n");
> -   kfree_skb(skb);
> +   if (err != -EPERM)
> +   kfree_skb(skb);
> return RESPST_ERR_RNR;
> }
>  
> @@ -956,7 +957,8 @@ static int send_ack(struct rxe_qp *qp, struct 
> rxe_pkt_info *pkt,
> err = rxe_xmit_packet(rxe, qp, _pkt, skb);
> if (err) {
> pr_err_ratelimited("Failed sending ack\n");
> -   kfree_skb(skb);
> +   if (err != -EPERM)
> +   kfree_skb(skb);
> }
>  
>  err1:
> @@ -1141,7 +1143,8 @@ static enum resp_states duplicate_request(struct rxe_qp 
> *qp,
> if (rc) {
> pr_err("Failed resending result. This flow is 
> not handled - skb ignored\n");
>         rxe_drop_ref(qp);
> -   kfree_skb(skb_copy);
> +   if (rc != -EPERM)
> +   kfree_skb(skb_copy);
> rc = RESPST_CLEANUP;
> goto out;
> }
> -- 
> 2.7.4
> 
-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [PATCH v2 iproute2-next 0/6] cm_id, cq, mr, and pd resource tracking

[Doug Ledford]

On 3/16/2018 12:18 PM, David Ahern wrote:
> On 3/13/18 1:58 PM, Doug Ledford wrote:
>> On Tue, 2018-03-13 at 13:45 -0700, David Ahern wrote:
>>> On 3/13/18 1:32 AM, Leon Romanovsky wrote:
>>>> On Mon, Mar 12, 2018 at 10:53:03AM -0700, David Ahern wrote:
>>>>> On 3/12/18 8:16 AM, Steve Wise wrote:
>>>>>> Hey all,
>>>>>>
>>>>>> The kernel side of this series has been merged for rdma-next [1].  Let me
>>>>>> know if this iproute2 series can be merged, of if it needs more changes.
>>>>>>
>>>>>
>>>>> The problem is that iproute2 headers are synced to kernel headers from
>>>>> DaveM's tree (net-next mainly). I take it this series will not appear in
>>>>> Dave's tree until after a merge through Linus' tree. Correct?
>>>>
>>>> David,
>>>>
>>>> Technically, you are right, and we would like to ask you for an extra tweak
>>>> to the flow for the RDMAtool, because current scheme causes delays at least
>>>> cycle.
>>>>
>>>> Every RDMAtool's patchset which requires changes to headers is always
>>>> includes header patch, can you please accept those series and once you
>>>> are bringing new net-next headers from Linus, simply overwrite all our
>>>> headers?
>>>
>>> I did not follow the discussion back when this decision was made, so how
>>> did rdma tool end up in iproute2?
>>
>> It is modeled after the ip command, and for better or worse, the
>> iproute2 package has become the standard drop box for low level kernel
>> network configuring tools.  The RDMA subsystem may not be IP networking,
>> but it is still networking, so it seemed an appropriate fit.
> 
> why doesn't the rdma tree go through Dave then?
> 

Because it doesn't use the core network stack hardly at all.  It creates
netdevs when it needs to bridge the two stacks, but otherwise the RDMA
subsystem core is apart and unique from the network stack Dave manages.
When I said it was networking, I meant it literally.  The RDMA fabrics
are networks.  It wasn't meant to imply that they shared anything
substantial in common with the typical Ethernet/IP networking that is
the core of what Dave manages.



signature.asc
Description: OpenPGP digital signature

Re: linux-next: manual merge of the net-next tree with the rdma-fixes tree

[Doug Ledford]

On Fri, 2018-03-16 at 11:56 +1100, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the net-next tree got a conflict in:
> 
>   drivers/infiniband/hw/mlx5/main.c
> 
> between commit:
> 
>   42cea83f9524 ("IB/mlx5: Fix cleanup order on unload")
> 
> from the rdma-fixes tree and commit:
> 
>   b5ca15ad7e61 ("IB/mlx5: Add proper representors support")
> 
> from the net-next tree.

We are aware of the merge conflict.  This is a result of the fact that
code had been submitted to the for-next area (the representors support)
and after that an issue was found by the syzkaller bot that deserved rc
fix status and which conflicted.  The fixup you list below is
insufficient to fix the merge conflict.  The full fixup can be found in
the rdma tree from where I merged the for-rc branch into the for-next
branch and created a complete fixup of the merge conflict.  The problem
is that one patch change the device init stage flow, while the other
patch duplicates the normal device init stage flow to the representor
device stage flow.  To resolve the fix, you not only have to resolve the
contextual diffs, but you have to duplicate the changes to the normal
device stage flow into the representor device stage flow.  It is very
far from a trivial merge.  We were planning on talking to Dave about
this issue tomorrow, but you beat us to raising the issue ;-).

Here's the commit (from the rdma git repo) with the proper merge fix
(although it also has other minor merge stuff that needs to be ignored):

2d873449a202 (Merge branch 'k.o/wip/dl-for-rc' into k.o/wip/dl-for-next)

> I fixed it up (see below and the merge fix patch as well) and can
> carry the fix as necessary. This is now fixed as far as linux-next is
> concerned, but any non trivial conflicts should be mentioned to your
> upstream maintainer when your tree is submitted for merging.  You may
> also want to consider cooperating with the maintainer of the conflicting
> tree to minimise any particularly complex conflicts.
> 
> From: Stephen Rothwell <s...@canb.auug.org.au>
> Date: Fri, 16 Mar 2018 11:54:01 +1100
> Subject: [PATCH] IB/mlx5: merge fix for "Fix cleanup order on unload"
> 
> Signed-off-by: Stephen Rothwell <s...@canb.auug.org.au>
> ---
>  drivers/infiniband/hw/mlx5/ib_rep.c  | 6 +++---
>  drivers/infiniband/hw/mlx5/mlx5_ib.h | 3 +--
>  2 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/mlx5/ib_rep.c 
> b/drivers/infiniband/hw/mlx5/ib_rep.c
> index 61cc3d7db257..7fb997dadd80 100644
> --- a/drivers/infiniband/hw/mlx5/ib_rep.c
> +++ b/drivers/infiniband/hw/mlx5/ib_rep.c
> @@ -33,9 +33,9 @@ static const struct mlx5_ib_profile rep_profile = {
>   STAGE_CREATE(MLX5_IB_STAGE_IB_REG,
>mlx5_ib_stage_ib_reg_init,
>mlx5_ib_stage_ib_reg_cleanup),
> - STAGE_CREATE(MLX5_IB_STAGE_UMR_RESOURCES,
> -  mlx5_ib_stage_umr_res_init,
> -  mlx5_ib_stage_umr_res_cleanup),
> + STAGE_CREATE(MLX5_IB_STAGE_POST_IB_REG_UMR,
> +  mlx5_ib_stage_post_ib_reg_umr_init,
> +  NULL),
>   STAGE_CREATE(MLX5_IB_STAGE_CLASS_ATTR,
>mlx5_ib_stage_class_attr_init,
>NULL),
> diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h 
> b/drivers/infiniband/hw/mlx5/mlx5_ib.h
> index 7ec753ec7962..c45a7abdbe3e 100644
> --- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
> +++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
> @@ -1071,8 +1071,7 @@ int mlx5_ib_stage_bfrag_init(struct mlx5_ib_dev *dev);
>  void mlx5_ib_stage_bfrag_cleanup(struct mlx5_ib_dev *dev);
>  int mlx5_ib_stage_ib_reg_init(struct mlx5_ib_dev *dev);
>  void mlx5_ib_stage_ib_reg_cleanup(struct mlx5_ib_dev *dev);
> -int mlx5_ib_stage_umr_res_init(struct mlx5_ib_dev *dev);
> -void mlx5_ib_stage_umr_res_cleanup(struct mlx5_ib_dev *dev);
> +int mlx5_ib_stage_post_ib_reg_umr_init(struct mlx5_ib_dev *dev);
>  int mlx5_ib_stage_class_attr_init(struct mlx5_ib_dev *dev);
>  void __mlx5_ib_remove(struct mlx5_ib_dev *dev,
> const struct mlx5_ib_profile *profile,
> -- 
> 2.16.1
> 

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [PATCH v2 iproute2-next 0/6] cm_id, cq, mr, and pd resource tracking

[Doug Ledford]

On Tue, 2018-03-13 at 13:45 -0700, David Ahern wrote:
> On 3/13/18 1:32 AM, Leon Romanovsky wrote:
> > On Mon, Mar 12, 2018 at 10:53:03AM -0700, David Ahern wrote:
> > > On 3/12/18 8:16 AM, Steve Wise wrote:
> > > > Hey all,
> > > > 
> > > > The kernel side of this series has been merged for rdma-next [1].  Let 
> > > > me
> > > > know if this iproute2 series can be merged, of if it needs more changes.
> > > > 
> > > 
> > > The problem is that iproute2 headers are synced to kernel headers from
> > > DaveM's tree (net-next mainly). I take it this series will not appear in
> > > Dave's tree until after a merge through Linus' tree. Correct?
> > 
> > David,
> > 
> > Technically, you are right, and we would like to ask you for an extra tweak
> > to the flow for the RDMAtool, because current scheme causes delays at least
> > cycle.
> > 
> > Every RDMAtool's patchset which requires changes to headers is always
> > includes header patch, can you please accept those series and once you
> > are bringing new net-next headers from Linus, simply overwrite all our
> > headers?
> 
> I did not follow the discussion back when this decision was made, so how
> did rdma tool end up in iproute2?

It is modeled after the ip command, and for better or worse, the
iproute2 package has become the standard drop box for low level kernel
network configuring tools.  The RDMA subsystem may not be IP networking,
but it is still networking, so it seemed an appropriate fit.

>  I do not need the overhead of
> sometimes I sync the rdma header file and sometimes I don't.
> 
> One option that comes to mind is to move the rdma header file under the
> rdma directory. It breaks the uapi model, but it seems that iproute2 is
> just a delivery vehicle for this command.



-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [pull request][for-next 00/11] Mellanox, mlx5 IPSec updates 2018-02-28-2 (Part 2)

[Doug Ledford]

On Mon, 2018-03-12 at 21:43 -0400, Doug Ledford wrote:
> On Mon, 2018-03-12 at 21:43 +, Saeed Mahameed wrote:
> > On Thu, 2018-03-08 at 14:14 -0500, Doug Ledford wrote:
> > > On 3/8/2018 1:04 PM, David Miller wrote:
> > > > From: Saeed Mahameed <sae...@mellanox.com>
> > > > Date: Wed,  7 Mar 2018 17:26:03 -0800
> > > > 
> > > > > Hi Dave and Doug,
> > > > > 
> > > > > This series includes shared code updates (IPSec part2) for mlx5
> > > > > core 
> > > > > driver for both netdev and rdma subsystems.  This series should
> > > > > be pulled
> > > > > to both trees so we can continue netdev and rdma specific
> > > > > submissions
> > > > > separately.
> > > > 
> > > > Doug, please give this series a quick review.
> > > > 
> > > > Thank you.
> > > > 
> > > 
> > > I'm good with it.  Pull it as you see fit.
> > > 
> > 
> > Hi Doug, 
> > 
> > Just FYI, I see that Dave already pulled the series into net-next.
> > I think you can pull as well, so Leon will start the mlx5 RDMA
> > submissions as planned on top this last pull request.
> > 
> > thanks,
> > Saeed.
> 
> Ok, thanks Saeed.
> 

Pulled and a new wip/dl-for-next is available.

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [pull request][for-next 00/11] Mellanox, mlx5 IPSec updates 2018-02-28-2 (Part 2)

[Doug Ledford]

On Mon, 2018-03-12 at 21:43 +, Saeed Mahameed wrote:
> On Thu, 2018-03-08 at 14:14 -0500, Doug Ledford wrote:
> > On 3/8/2018 1:04 PM, David Miller wrote:
> > > From: Saeed Mahameed <sae...@mellanox.com>
> > > Date: Wed,  7 Mar 2018 17:26:03 -0800
> > > 
> > > > Hi Dave and Doug,
> > > > 
> > > > This series includes shared code updates (IPSec part2) for mlx5
> > > > core 
> > > > driver for both netdev and rdma subsystems.  This series should
> > > > be pulled
> > > > to both trees so we can continue netdev and rdma specific
> > > > submissions
> > > > separately.
> > > 
> > > Doug, please give this series a quick review.
> > > 
> > > Thank you.
> > > 
> > 
> > I'm good with it.  Pull it as you see fit.
> > 
> 
> Hi Doug, 
> 
> Just FYI, I see that Dave already pulled the series into net-next.
> I think you can pull as well, so Leon will start the mlx5 RDMA
> submissions as planned on top this last pull request.
> 
> thanks,
> Saeed.

Ok, thanks Saeed.

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [pull request][for-next 00/11] Mellanox, mlx5 IPSec updates 2018-02-28-2 (Part 2)

[Doug Ledford]

On 3/8/2018 2:38 PM, Randy Dunlap wrote:
> On 03/08/2018 11:14 AM, Doug Ledford wrote:
>> On 3/8/2018 1:04 PM, David Miller wrote:
>>> From: Saeed Mahameed <sae...@mellanox.com>
>>> Date: Wed,  7 Mar 2018 17:26:03 -0800
>>>
>>>> Hi Dave and Doug,
>>>>
>>>> This series includes shared code updates (IPSec part2) for mlx5 core 
>>>> driver for both netdev and rdma subsystems.  This series should be pulled
>>>> to both trees so we can continue netdev and rdma specific submissions
>>>> separately.
>>>
>>> Doug, please give this series a quick review.
>>>
>>> Thank you.
>>>
>>
>> I'm good with it.  Pull it as you see fit.
>>
> 
> I got this build error in today's linux-next (20180308):
> 
> ../drivers/net/ethernet/mellanox/mlx5/core/fs_core.c: In function 
> 'mlx5_init_fs':
> ../drivers/net/ethernet/mellanox/mlx5/core/fs_core.c:2652:2: error: implicit 
> declaration of function 'mlx5_accel_ipsec_device_caps' 
> [-Werror=implicit-function-declaration]
>   if (mlx5_accel_ipsec_device_caps(steering->dev) &
>   ^
> ../drivers/net/ethernet/mellanox/mlx5/core/fs_core.c:2653:6: error: 
> 'MLX5_ACCEL_IPSEC_DEVICE' undeclared (first use in this function)
>   MLX5_ACCEL_IPSEC_DEVICE) {
>   ^
> ../drivers/net/ethernet
> 
> 
> Is this perhaps fixed already?
> Do you need the randconfig file?
> 
> thanks,
> 

That's fixed by patch 1 of this series.



signature.asc
Description: OpenPGP digital signature

Re: [pull request][for-next 00/11] Mellanox, mlx5 IPSec updates 2018-02-28-2 (Part 2)

[Doug Ledford]

On 3/8/2018 1:04 PM, David Miller wrote:
> From: Saeed Mahameed 
> Date: Wed,  7 Mar 2018 17:26:03 -0800
> 
>> Hi Dave and Doug,
>>
>> This series includes shared code updates (IPSec part2) for mlx5 core 
>> driver for both netdev and rdma subsystems.  This series should be pulled
>> to both trees so we can continue netdev and rdma specific submissions
>> separately.
> 
> Doug, please give this series a quick review.
> 
> Thank you.
> 

I'm good with it.  Pull it as you see fit.



signature.asc
Description: OpenPGP digital signature

Re: [pull request][for-next V2 00/13] Mellanox, mlx5 IPSec updates 2018-02-28-1

[Doug Ledford]

On Wed, 2018-03-07 at 15:41 -0500, Doug Ledford wrote:
> On Wed, 2018-03-07 at 15:31 -0500, David Miller wrote:
> > From: Saeed Mahameed <sae...@mellanox.com>
> > Date: Tue,  6 Mar 2018 22:35:03 -0800
> > 
> > > This series includes shared code updates for mlx5 core driver for both
> > > netdev and rdma subsystems.  This series should be pulled to both
> > > trees so we can continue netdev and rdma specific submissions separately.
> > > 
> > > For more information please see tag log below.
> > > 
> > > The series doesn't cause any conflict with the latest mlx5 rc fixes.
> > > 
> > > v1->v2:
> > >   - Drop sparse fixes patch
> > >   - Updated commit message of "net/mlx5: Add has_tag to mlx5_flow_act"
> > >   - Add const to  static mlx5_flow_cmd structs where needed.
> > 
> > Pulled, thanks Saeed.
> 
> Thanks, pulled here as well.
> 

Just FYI,

My .config might have been in an unreasonable state (I had jumped from
for-next to for-rc, built a kernel which ran a make oldconfig, then
jumped back to for-next and tried to build with this series applied but
without making any changes to the .config file), but I got a build
error.  My .config had both innova and the new ipsec accelerator turned
off or something like that, and I got this error:

  CC [M]  drivers/net/ethernet/mellanox/mlx5/core/en_main.o
drivers/net/ethernet/mellanox/mlx5/core/fs_core.c: In function
‘mlx5_init_fs’:
drivers/net/ethernet/mellanox/mlx5/core/fs_core.c:2645:6: error:
implicit declaration of function ‘mlx5_accel_ipsec_device_caps’; did you
mean ‘mlx5_accel_ipsec_cleanup’? [-Werror=implicit-function-declaration]
  if (mlx5_accel_ipsec_device_caps(steering->dev) &
  ^~~~
  mlx5_accel_ipsec_cleanup
  CC [M]  drivers/net/ethernet/silan/sc92031.o
  CC [M]  drivers/w1/slaves/w1_smem.o
  CC [M]  drivers/net/ethernet/mellanox/mlx5/core/en_common.o
  CC [M]  drivers/net/ethernet/sfc/falcon/nic.o
drivers/net/ethernet/mellanox/mlx5/core/fs_core.c:2646:6: error:
‘MLX5_ACCEL_IPSEC_DEVICE’ undeclared (first use in this function); did
you mean ‘__MLX5_ACCEL_IPSEC_H__’?
  MLX5_ACCEL_IPSEC_DEVICE) {
  ^~~
  __MLX5_ACCEL_IPSEC_H__
drivers/net/ethernet/mellanox/mlx5/core/fs_core.c:2646:6: note: each
undeclared identifier is reported only once for each function it appears
in
  CC [M]  drivers/w1/w1.o

Running make config and enabling innova support and ipsec accelerator
support fixed it.

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [pull request][for-next V2 00/13] Mellanox, mlx5 IPSec updates 2018-02-28-1

[Doug Ledford]

On Wed, 2018-03-07 at 15:31 -0500, David Miller wrote:
> From: Saeed Mahameed <sae...@mellanox.com>
> Date: Tue,  6 Mar 2018 22:35:03 -0800
> 
> > This series includes shared code updates for mlx5 core driver for both
> > netdev and rdma subsystems.  This series should be pulled to both
> > trees so we can continue netdev and rdma specific submissions separately.
> > 
> > For more information please see tag log below.
> > 
> > The series doesn't cause any conflict with the latest mlx5 rc fixes.
> > 
> > v1->v2:
> >   - Drop sparse fixes patch
> >   - Updated commit message of "net/mlx5: Add has_tag to mlx5_flow_act"
> >   - Add const to  static mlx5_flow_cmd structs where needed.
> 
> Pulled, thanks Saeed.

Thanks, pulled here as well.

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [for-next V2 12/13] {net,IB}/mlx5: Add flow steering helpers

[Doug Ledford]

On Tue, 2018-03-06 at 22:35 -0800, Saeed Mahameed wrote:
> diff --git a/include/linux/mlx5/fs_helpers.h b/include/linux/mlx5/fs_helpers.h
> new file mode 100644
> index ..7b476bbae731
> --- /dev/null
> +++ b/include/linux/mlx5/fs_helpers.h
> @@ -0,0 +1,134 @@
> +/*
> + * Copyright (c) 2018, Mellanox Technologies. All rights reserved.
> + *
> + * This software is available to you under a choice of one of two
> + * licenses.  You may choose to be licensed under the terms of the GNU
> + * General Public License (GPL) Version 2, available from the file
> + * COPYING in the main directory of this source tree, or the
> + * OpenIB.org BSD license below:
> + *
> + * Redistribution and use in source and binary forms, with or
> + * without modification, are permitted provided that the following
> + * conditions are met:
> + *
> + *  - Redistributions of source code must retain the above
> + *copyright notice, this list of conditions and the following
> + *disclaimer.
> + *
> + *  - Redistributions in binary form must reproduce the above
> + *copyright notice, this list of conditions and the following
> + *disclaimer in the documentation and/or other materials
> + *provided with the distribution.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +

Nit: I would prefer a license identifier line followed by just the
copyright tag, not the full boiler plate.

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [pull request][for-next V2 00/13] Mellanox, mlx5 IPSec updates 2018-02-28-1

[Doug Ledford]

On Tue, 2018-03-06 at 22:35 -0800, Saeed Mahameed wrote:
> Hi Dave and Doug,
> 
> This series includes shared code updates for mlx5 core driver for both
> netdev and rdma subsystems.  This series should be pulled to both
> trees so we can continue netdev and rdma specific submissions separately.
> 
> For more information please see tag log below.
> 
> The series doesn't cause any conflict with the latest mlx5 rc fixes.
> 
> v1->v2:
>   - Drop sparse fixes patch
>   - Updated commit message of "net/mlx5: Add has_tag to mlx5_flow_act"
>   - Add const to  static mlx5_flow_cmd structs where needed.
> 
> Thanks,
> Saeed.

FWIW, the IB portions are fine with me (they're all pretty simple).

Acked-by: Doug Ledford <dledf...@redhat.com>

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [pull request][for-next 00/15] Mellanox, mlx5 RDMA E-Switch representors 2018-02-23

[Doug Ledford]

On Wed, 2018-02-28 at 09:55 -0500, David Miller wrote:
> From: Saeed Mahameed <sae...@mellanox.com>
> Date: Mon, 26 Feb 2018 12:36:02 -0800
> 
> > This series includes shared code updates for mlx5 core driver for both
> > netdev and rdma subsystems.  This series should be pulled to both
> > trees so we can continue netdev and rdma specific submissions separately.
> > 
> > For more information please see tag log below.
> > 
> > P.S. We expect one more shared code pull requests.
> > 
> > Since this series contains more work on RDMA than mlx5 eswitch, I would
> > like to get Doug's or Jason's approval before pulling it in.
> > 
> > The series doesn't cause any conflict with the latest mlx5 net fixes
> > series.
> 
> With Doug's ACK, I've pulled this into net-next.
> 
> Thanks.

Series pulled here as well, thanks.

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [pull request][for-next 00/15] Mellanox, mlx5 RDMA E-Switch representors 2018-02-23

[Doug Ledford]

On Mon, 2018-02-26 at 12:36 -0800, Saeed Mahameed wrote:
> Hi Doug and Dave,
> 
> This series includes shared code updates for mlx5 core driver for both
> netdev and rdma subsystems.  This series should be pulled to both
> trees so we can continue netdev and rdma specific submissions separately.
> 
> For more information please see tag log below.
> 
> P.S. We expect one more shared code pull requests.
> 
> Since this series contains more work on RDMA than mlx5 eswitch, I would
> like to get Doug's or Jason's approval before pulling it in.

This series looks OK to me, I'm good to pull it in.

> The series doesn't cause any conflict with the latest mlx5 net fixes
> series.
> 
> Thanks,
> Saeed.
> 
> ---
> 
> The following changes since commit 388ca8be00370db132464e27f745b8a0add19fcb:
> 
>   IB/mlx5: Implement fragmented completion queue (CQ) (2018-02-15 00:30:03 
> -0800)
> 
> are available in the Git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/mellanox/linux.git 
> tags/mlx5-updates-2018-02-23
> 
> for you to fetch changes up to ec9c2fb8ceb5b514c4820f732537cb2982de0620:
> 
>   IB/mlx5: Disable self loopback check when in switchdev mode (2018-02-23 
> 12:36:39 -0800)
> 
> 
> mlx5-update-2018-02-23 (IB representors)
> 
> From: Mark Bloch <ma...@mellanox.com>
> =
> Add IB representor when in switchdev mode
> 
> The following series adds support for an IB (RAW Ethernet only) device
> representor which is created when the user switches to switchdev mode.
> 
> Today when switching to switchdev mode the only representors which are
> created are net devices. Each netdev is a representor of a virtual
> function and any data sent via the representor is received on the virtual
> function, and any data sent via the virtual function is received by the
> representor.
> 
> For the mlx5 driver the main use of this functionality is to be able to
> use Open vSwitch on the hypervisor in order to manage/control traffic
> from/to the virtual functions. Open vSwitch can also work with  DPDK
> devices and not just net devices, this series exposes an IB device, which
> Mellanox PMD driver uses, which then can be used by Open vSwitch DPDK.
> 
> An IB device representor exposes only RAW Ethernet QP capabilities and
> the ability to create flow rules to direct traffic to its RX queues. The
> state of the IB device (ACTIVE/DOWN etc..) is based on the state of the
> corresponding net device representor. No other RDMA/RoCE functionality is
> currently supported and no GID table is exposed.
> =
> 
> 
> Mark Bloch (15):
>   net/mlx5: E-Switch, Add callback to get representor device
>   net/mlx5: E-Switch, Move representors definition to a global scope
>   net/mlx5: E-Switch, Increase number of FTEs in FDB in switchdev mode
>   net/mlx5: E-Switch, Optimize HW steering tables in switchdev mode
>   net/mlx5: E-Switch, Add definition of IB representor
>   IB/mlx5: Add basic regiser/unregister representors code
>   IB/mlx5: Allocate flow DB only on PF IB device
>   IB/mlx5: Add match on vport when in switchdev mode
>   IB/mlx5: Listen to netdev register/unresiter events in switchdev mode
>   IB/mlx5: When in switchdev mode, expose only raw packet capabilities
>   IB/mlx5: Don't expose MR cache in switchdev mode
>   IB/mlx5: E-Switch, Add rule to forward traffic to vport
>   IB/mlx5: Add proper representors support
>   net/mlx5: E-Switch, Reload IB interface when switching devlink modes
>   IB/mlx5: Disable self loopback check when in switchdev mode
> 
>  drivers/infiniband/hw/mlx5/Makefile|   1 +
>  drivers/infiniband/hw/mlx5/ib_rep.c| 189 +++
>  drivers/infiniband/hw/mlx5/ib_rep.h|  72 
>  drivers/infiniband/hw/mlx5/main.c  | 361 
> -
>  drivers/infiniband/hw/mlx5/mlx5_ib.h   |  32 +-
>  drivers/infiniband/hw/mlx5/mr.c|   5 +-
>  drivers/infiniband/hw/mlx5/qp.c|  24 ++
>  drivers/net/ethernet/mellanox/mlx5/core/dev.c  |   8 +
>  drivers/net/ethernet/mellanox/mlx5/core/en_rep.c   |  11 +
>  drivers/net/ethernet/mellanox/mlx5/core/eswitch.c  |  23 +-
>  drivers/net/ethernet/mellanox/mlx5/core/eswitch.h  |  42 +--
>  .../ethernet/mellanox/mlx5/core/eswitch_offloads.c | 104 --
>  .../net/ethernet/mellanox/mlx5/core/mlx5_core.h|   7 +-
>  include/linux/mlx5/driver.h|   6 +
>  include/linux/mlx5/eswitch.h   |  58 
>

Re: [pull request][for-next 0/7] Mellanox, mlx5 shared code updates 2018-02-21

[Doug Ledford]

On Thu, 2018-02-22 at 14:39 -0500, David Miller wrote:
> From: Saeed Mahameed <sae...@mellanox.com>
> Date: Wed, 21 Feb 2018 12:13:47 -0800
> 
> > This series includes shared code updates for mlx5 core driver for both
> > netdev and rdma subsystems.  This series should be pulled to both
> > trees so we can continue netdev and rdma specific submissions separately.
> > 
> > For more information please see tag log below.
> > 
> > P.S. We expect two more shared code pull requests.
> > 
> > The series doesn't cause any conflict with the latest mlx5 net fixes
> > series.
> > 
> > Please pull and let me know if there's any issue,
> 
> Looks good to me, pulled into net-next, thanks.

Thanks, pulled into rdma-next.

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [PATCH v3] RDS: IB: Fix null pointer issue

[Doug Ledford]

On Mon, 2018-02-05 at 10:27 +0800, Guanglei Li wrote:
> Scenario:
> 1. Port down and do fail over
> 2. Ap do rds_bind syscall
> 
> PID: 47039  TASK: 89887e2fe640  CPU: 47  COMMAND: "kworker/u:6"
>  #0 [898e35f159f0] machine_kexec at 8103abf9
>  #1 [898e35f15a60] crash_kexec at 810b96e3
>  #2 [898e35f15b30] oops_end at 8150f518
>  #3 [898e35f15b60] no_context at 8104854c
>  #4 [898e35f15ba0] __bad_area_nosemaphore at 81048675
>  #5 [898e35f15bf0] bad_area_nosemaphore at 810487d3
>  #6 [898e35f15c00] do_page_fault at 815120b8
>  #7 [898e35f15d10] page_fault at 8150ea95
> [exception RIP: unknown or invalid address]
> RIP:   RSP: 898e35f15dc8  RFLAGS: 00010282
> RAX: fffe  RBX: 889b77f6fc00  RCX:81c99d88
> RDX:   RSI: 896019ee08e8  RDI:889b77f6fc00
> RBP: 898e35f15df0   R8: 896019ee08c8  R9:
> R10: 0400  R11:   R12:896019ee08c0
> R13: 889b77f6fe68  R14: 81c99d80  R15: a022a1e0
> ORIG_RAX:   CS: 0010 SS: 0018
>  #8 [898e35f15dc8] cma_ndev_work_handler at a022a228 [rdma_cm]
>  #9 [898e35f15df8] process_one_work at 8108a7c6
>  #10 [898e35f15e58] worker_thread at 8108bda0
>  #11 [898e35f15ee8] kthread at 81090fe6
> 
> PID: 45659  TASK: 880d313d2500  CPU: 31  COMMAND: "oracle_45659_ap"
>  #0 [881024ccfc98] __schedule at 8150bac4
>  #1 [881024ccfd40] schedule at 8150c2cf
>  #2 [881024ccfd50] __mutex_lock_slowpath at 8150cee7
>  #3 [881024ccfdc0] mutex_lock at 8150cdeb
>  #4 [881024ccfde0] rdma_destroy_id at a022a027 [rdma_cm]
>  #5 [881024ccfe10] rds_ib_laddr_check at a0357857 [rds_rdma]
>  #6 [881024ccfe50] rds_trans_get_preferred at a0324c2a [rds]
>  #7 [881024ccfe80] rds_bind at a031d690 [rds]
>  #8 [881024ccfeb0] sys_bind at 8142a670
> 
> PID: 45659  PID: 47039
> rds_ib_laddr_check
>   /* create id_priv with a null event_handler */
>   rdma_create_id
>   rdma_bind_addr
> cma_acquire_dev
>   /* add id_priv to cma_dev->id_list */
>   cma_attach_to_dev
> cma_ndev_work_handler
>   /* event_hanlder is null */
>   id_priv->id.event_handler
> 
> Signed-off-by: Guanglei Li <guanglei...@oracle.com>
> Signed-off-by: Honglei Wang <honglei.w...@oracle.com>
> Reviewed-by: Junxiao Bi <junxiao...@oracle.com>
> Reviewed-by: Yanjun Zhu <yanjun@oracle.com>
> Reviewed-by: Leon Romanovsky <leo...@mellanox.com>
> Acked-by: Santosh Shilimkar <santosh.shilim...@oracle.com>

RDS patches normally go through Dave's tree, so for the RDMA aspects of
this patch:

Acked-by: Doug Ledford <dledf...@redhat.com>

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [pull request][for-next V3 00/11] Mellanox, mlx5 E-Switch updates 2017-12-19

[Doug Ledford]

On Fri, 2018-01-05 at 12:26 -0700, Jason Gunthorpe wrote:
> On Fri, Jan 05, 2018 at 02:07:34PM -0500, Doug Ledford wrote:
> 
> > > > V2->V3:
> > > >  - Fixed compilation warning, reported by Dave.
> > > > 
> > > > Please pull and let me know if there's any problem.
> > > 
> > > Looks good, pulled, thank you.
> > 
> > Hi Jason,
> > 
> > Unless I missed something, you haven't pulled this for for-next,
> > correct?
> 
> I was told there are no patches for RDMA that depend on this for this
> cycle, so RDMA does not need it.

Works For Me, thanks.

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [pull request][for-next V3 00/11] Mellanox, mlx5 E-Switch updates 2017-12-19

[Doug Ledford]

On Thu, 2017-12-28 at 19:46 -0500, David Miller wrote:
> From: Saeed Mahameed <sae...@mellanox.com>
> Date: Fri, 29 Dec 2017 01:23:03 +0200
> 
> > ==
> > This series includes updates for mlx5 E-Switch infrastructures,
> > to be merged into net-next and rdma-next trees.
> > 
> > Mark's patches provide E-Switch refactoring that generalize the mlx5
> > E-Switch vf representors interfaces and data structures. The serious is
> > mainly focused on moving ethernet (netdev) specific representors logic out
> > of E-Switch (eswitch.c) into mlx5e representor module (en_rep.c), which
> > provides better separation and allows future support for other types of vf
> > representors (e.g. RDMA).
> > 
> > Gal's patches at the end of this serious, provide a simple syntax fix and
> > two other patches that handles vport ingress/egress ACL steering name
> > spaces to be aligned with the Firmware/Hardware specs.
> > ===
> > 
> > V1->V2:
> >  - Addressed coding style comments in patches #1 and #7
> >  - The series is still based on rc4, as now I see net-next is also @rc4.
> > 
> > V2->V3:
> >  - Fixed compilation warning, reported by Dave.
> > 
> > Please pull and let me know if there's any problem.
> 
> Looks good, pulled, thank you.

Hi Jason,

Unless I missed something, you haven't pulled this for for-next,
correct?

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [pull request][for-next 00/12] Mellanox, mlx5 IPoIB Muli Pkey support 2017-10-11,Re: [pull request][for-next 00/12] Mellanox, mlx5 IPoIB Muli Pkey support 2017-10-11

[Doug Ledford]

On Mon, 2017-10-16 at 05:45 +0100, David Miller wrote:
> > Once I hear that Dave is OK with the net changes, I'm ready to pull
> > (if
> > I need to).
> 
> They look fine and I've pulled this into net-next.

Thanks, pulled here as well.

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH v2 12/15] RDMA/cma: make config_item_type const

[Doug Ledford]

On Mon, 2017-10-16 at 17:18 +0200, Bhumika Goyal wrote:
> Make these structures const as they are either passed to the
> functions
> having the argument as const or stored as a reference in the
> "ci_type"
> const field of a config_item structure.
> 
> Signed-off-by: Bhumika Goyal <bhumi...@gmail.com>

Acked-by: Doug Ledford <dledf...@redhat.com>

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [pull request][for-next 00/12] Mellanox, mlx5 IPoIB Muli Pkey support 2017-10-11

[Doug Ledford]

On 10/14/2017 2:48 PM, Saeed Mahameed wrote:
> Hi Dave and Doug,
> 
> This series includes updates for mlx5 IPoIB offloading driver from Alex
> and Feras to add the support for Muli Pkey in the mlx5i ipoib offloading 
> netdev,
> to be merged into net-next and rdma-next trees.

As far as the two IPoIB patches are concerned, they're fine.

> Doug, I am sorry I couldn't base this on rc2 since the series needs and 
> conflicts
> with a fix that was submitted to rc3, so to keep things simple I based it on 
> rc4,
> I hope this is ok with you..

No worries, it just means I have to submit it under another branch.  But
I'm already holding one patch series in a stand alone branch, so no big
deal.  And, actually, the IPoIB changes are so small they can simply go
through Dave's tree if you don't have any dependent code in the IPoIB
driver to submit after this but still in this devel cycle.

> Please pull and let me know if there's any problem.

Once I hear that Dave is OK with the net changes, I'm ready to pull (if
I need to).

-- 
Doug Ledford <dledf...@redhat.com>
GPG Key ID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] net/mlx4_core: Convert timers to use timer_setup()

[Doug Ledford]

On Wed, 2017-10-04 at 17:51 -0700, Kees Cook wrote:
> In preparation for unconditionally passing the struct timer_list
> pointer to
> all timer callbacks, switch to using the new timer_setup() and
> from_timer()
> to pass the timer pointer explicitly.
> 
> Cc: Tariq Toukan <tar...@mellanox.com>
> Cc: netdev@vger.kernel.org
> Cc: linux-r...@vger.kernel.org
> Cc: Thomas Gleixner <t...@linutronix.de>
> Signed-off-by: Kees Cook <keesc...@chromium.org>
> ---
> This requires commit 686fef928bba ("timer: Prepare to change timer
> callback argument type") in v4.14-rc3, but should be otherwise
> stand-alone.

Thanks, applied.

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [pull request][for-next 0/9] Mellanox, mlx5 updates 2017-10-06

[Doug Ledford]

On Fri, 2017-10-06 at 16:37 -0700, Saeed Mahameed wrote:
> The following changes since commit
> e19b205be43d11bff638cad4487008c48d21c103:
> 
>   Linux 4.14-rc2 (2017-09-24 16:38:56 -0700)

Thanks for keeping the base at rc2 like I requested.  Pulled.

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH v12 0/8] Replace PCI pool by DMA pool API

[Doug Ledford]

On Tue, 2017-08-22 at 13:46 +0200, Romain Perier wrote:
> The current PCI pool API are simple macro functions direct expanded
> to
> the appropriate dma pool functions. The prototypes are almost the
> same
> and semantically, they are very similar. I propose to use the DMA
> pool
> API directly and get rid of the old API.
> 
> This set of patches, replaces the old API by the dma pool API
> and remove the defines.
> 
> Changes in v12:
> - Rebased series onto next-20170822

Hi Romain,

I've applied the three that are in my purview to my for-next branch
(mthca, mlx4, and mlx5 patches).  Thanks.

-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH v10 00/15] Replace PCI pool by DMA pool API

[Doug Ledford]

On Thu, 2017-07-06 at 10:12 +0200, Romain Perier wrote:
> The current PCI pool API are simple macro functions direct expanded
> to
> the appropriate dma pool functions. The prototypes are almost the
> same
> and semantically, they are very similar. I propose to use the DMA
> pool
> API directly and get rid of the old API.
> 
> This set of patches, replaces the old API by the dma pool API
> and remove the defines.

Is someone planning on merging this series?  If not, I'll send through
the patches I've personally tested (3, 5, and 6).

> Changes in v10:
> - Rebased series onto next-20170706
> - I have fixed and improved patch "scsi: megaraid: Replace PCI pool
> old API"
> 
> Changes in v9:
> - Rebased series onto next-20170522
> - I have fixed and improved the patch for lpfc driver
> 
> Changes in v8:
> - Rebased series onto next-20170428
> 
> Changes in v7:
> - Rebased series onto next-20170416
> - Added Acked-by, Tested-by and Reviwed-by tags
> 
> Changes in v6:
> - Fixed an issue reported by kbuild test robot about changes in
> DAC960
> - Removed patches 15/19,16/19,17/19,18/19. They have been merged by
> Greg
> - Added Acked-by Tags
> 
> Changes in v5:
> - Re-worded the cover letter (remove sentence about checkpatch.pl)
> - Rebased series onto next-20170308
> - Fix typos in commit message
> - Added Acked-by Tags
> 
> Changes in v4:
> - Rebased series onto next-20170301
> - Removed patch 20/20: checks done by checkpath.pl, no longer
> required.
>   Thanks to Peter and Joe for their feedbacks.
> - Added Reviewed-by tags
> 
> Changes in v3:
> - Rebased series onto next-20170224
> - Fix checkpath.pl reports for patch 11/20 and patch 12/20
> - Remove prefix RFC
> Changes in v2:
> - Introduced patch 18/20
> - Fixed cosmetic changes: spaces before brace, live over 80
> characters
> - Removed some of the check for NULL pointers before calling
> dma_pool_destroy
> - Improved the regexp in checkpatch for pci_pool, thanks to Joe
> Perches
> - Added Tested-by and Acked-by tags
> 
> Romain Perier (15):
>   block: DAC960: Replace PCI pool old API
>   dmaengine: pch_dma: Replace PCI pool old API
>   IB/mthca: Replace PCI pool old API
>   net: e100: Replace PCI pool old API
>   mlx4: Replace PCI pool old API
>   mlx5: Replace PCI pool old API
>   wireless: ipw2200: Replace PCI pool old API
>   scsi: be2iscsi: Replace PCI pool old API
>   scsi: csiostor: Replace PCI pool old API
>   scsi: lpfc: Replace PCI pool old API
>   scsi: megaraid: Replace PCI pool old API
>   scsi: mpt3sas: Replace PCI pool old API
>   scsi: mvsas: Replace PCI pool old API
>   scsi: pmcraid: Replace PCI pool old API
>   PCI: Remove PCI pool macro functions
> 
>  drivers/block/DAC960.c| 38 ++-
>  drivers/block/DAC960.h|  4 +-
>  drivers/dma/pch_dma.c | 12 ++--
>  drivers/infiniband/hw/mthca/mthca_av.c| 10 +--
>  drivers/infiniband/hw/mthca/mthca_cmd.c   |  8 +--
>  drivers/infiniband/hw/mthca/mthca_dev.h   |  4 +-
>  drivers/net/ethernet/intel/e100.c | 12 ++--
>  drivers/net/ethernet/mellanox/mlx4/cmd.c  | 10 +--
>  drivers/net/ethernet/mellanox/mlx4/mlx4.h |  2 +-
>  drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 11 ++--
>  drivers/net/wireless/intel/ipw2x00/ipw2200.c  | 13 ++--
>  drivers/scsi/be2iscsi/be_iscsi.c  |  6 +-
>  drivers/scsi/be2iscsi/be_main.c   |  6 +-
>  drivers/scsi/be2iscsi/be_main.h   |  2 +-
>  drivers/scsi/csiostor/csio_hw.h   |  2 +-
>  drivers/scsi/csiostor/csio_init.c | 11 ++--
>  drivers/scsi/csiostor/csio_scsi.c |  6 +-
>  drivers/scsi/lpfc/lpfc.h  | 16 ++---
>  drivers/scsi/lpfc/lpfc_init.c | 16 ++---
>  drivers/scsi/lpfc/lpfc_mem.c  | 90 +--
> 
>  drivers/scsi/lpfc/lpfc_nvme.c |  6 +-
>  drivers/scsi/lpfc/lpfc_nvmet.c|  4 +-
>  drivers/scsi/lpfc/lpfc_scsi.c | 12 ++--
>  drivers/scsi/lpfc/lpfc_sli.c  |  6 +-
>  drivers/scsi/megaraid/megaraid_mbox.c | 30 -
>  drivers/scsi/megaraid/megaraid_mm.c   | 29 -
>  drivers/scsi/megaraid/megaraid_sas_base.c | 27 
>  drivers/scsi/megaraid/megaraid_sas_fusion.c   | 46 +++---
>  drivers/scsi/mpt3sas/mpt3sas_base.c   | 73 ++---
> -
>  drivers/scsi/mvsas/mv_init.c  |  6 +-
>  drivers/scsi/mvsas/mv_sas.c   |  6 +-
>  drivers/scsi/pmcraid.c| 10 +--
>  drivers/scsi/pmcraid.h|  2 +-
>  include/linux/mlx5/driver.h   |  2 +-
>  include/linux/pci.h   |  9 ---
>  35 files changed, 269 insertions(+), 278 deletions(-)
> 
-- 
Doug Ledford <dledf...@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [pull request][for-next 0/6] Mellanox mlx5 updates 2017-05-23

[Doug Ledford]

On Thu, 2017-05-25 at 12:02 -0400, David Miller wrote:
> From: Saeed Mahameed <sae...@mellanox.com>
> Date: Tue, 23 May 2017 14:43:58 +0300
> 
> > Hi Dave and Doug,
> > 
> > This series introduces some small updates and FPGA support to the
> mlx5
> > core/ethernet and IB drivers.
> > 
> > For more details please see below.
> > 
> > Please pull and let me know if there's any problem.
> 
> Ok, I've pulled this into net-next.
> 
> Doug let me know if there are any merge hassles we need to coordinate
> on.

Turns out that you had pulled this prior to your net-next tree making
it up to v4.12-rc3, while I had my -rc branch based on v4.12-rc3, so
when I pulled just up to your merge commit for this pull request, it
meant a later merge of my -rc branch would pull in a bunch of stuff
between here and -rc3.  So I ended up pulling your latest net-next as
of today as the easiest way to resolve that issue.  Now I can merge my
-rc branch in and it won't cause any extra noise in the merge.  It
seems to be a fairly regular pattern that I'm going to have to hold my
pull request to Linus until after your tree is pulled, so I might just
start planning on that from now on ;-)

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [for-next 4/6] net/mlx5: FPGA, Add basic support for Innova

[Doug Ledford]

On Wed, 2017-06-07 at 13:21 -0600, Jason Gunthorpe wrote:
> On Wed, Jun 07, 2017 at 10:13:43PM +0300, Saeed Mahameed wrote:
> > 
> > No !!
> > I am just showing you that the ib_core eventually will end up
> > calling
> > mlx5_core to create a QP.
> > so mlx5_core can create the QP it self since it is the one
> > eventually
> > creating QPs.
> > we just call mlx5_core_create_qp directly.
> 
> Which is building a RDMA ULP inside a driver without using the core
> code :(

Aren't the transmit/receive queues of the Ethernet netdevice on
mlx4/mlx5 hardware QPs too?  Those bypass the RDMA subsystem entirely.
 Just because something uses a QP on hardware that does *everything*
via QPs doesn't necessarily mean it must go through the RDMA subsystem.

Now, the fact that the content of the packets is basically a RoCE
packet does make things a bit fuzzier, but if their packets are
specially crafted RoCE packets that aren't really intended to be fully
RoCE spec compliant (maybe they don't support all the options as normal
RoCE QPs), then I can see hiding them from the larger RoCE portion of
the RDMA stack.

> > 
> > > 
> > > This keep getting more ugly :(
> > > 
> > > What about security? What if user space sends some raw packets to
> > > the
> > > FPGA - can it reprogram the ISPEC settings or worse?
> > > 
> > 
> > No such thing. This QP is only for internal driver/HW
> > communications,
> > as it is faster from the existing command interface.
> > it is not meant to be exposed for any raw user space usages at all,
> > without proper standard API adapter of course.
> 
> I'm not asking about the QP, I'm asking what happens after the NIC
> part. You use ROCE packets to control the FPGA. What prevents
> userspace from forcibly constructing roce packets and sending them to
> the FPGA. How does the FPGA know for certain the packet came from the
> kernel QP and not someplace else.

This is a valid concern.

> This is especially true for mlx nics as there are many raw packet
> bypass mechanisms available to userspace.

Right.  The question becomes: Does the firmware filter outgoing raw ETH
QPs such that a nefarious user could not send a crafted RoCE packet
that the bump on the wire would intercept and accept?

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH net-next 0/3] mlx4 drivers: version update

[Doug Ledford]

On Wed, 2017-06-07 at 15:33 -0400, David Miller wrote:
> From: Tariq Toukan <tar...@mellanox.com>
> Date: Wed,  7 Jun 2017 16:26:12 +0300
> 
> > This patchset contains version updates for the MLX4 drivers:
> > Core, EN, and IB.
> > 
> > Just like we've done in mlx5, we modify the outdated driver
> > version (reported in ethtool for example).
> > This better reflects the current driver state, and removes the
> > redundant date string.
> > We are not going to change this frequently or even use it.
> > 
> > I include the IB patch in this series as it has similar subject
> > and content.
> > It does not cause any kind of conflict with Doug's tree.
> > The rdma mailing list is CCed.
> > Please let me know if I need to submit this differently.
> 
> Ok, series applied.
> 
> Doug, let me know if you want to handle this differently or want
> me to change something.
> 
> Thanks.

You've got it, I'll skip it, it's all good.

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [for-next 4/6] net/mlx5: FPGA, Add basic support for Innova

[Doug Ledford]

On Fri, 2017-06-02 at 16:31 -0400, Jes Sorensen wrote:
> 
> > It is already modular like that. See CONFIG_MLX5_FPGA.
> 
> [jes@xpeas netdev.git]$ grep CONFIG_MLX5_FPGA 
> drivers/net/ethernet/mellanox/mlx5/core/*
> [jes@xpeas netdev.git]$
> 
> Which git tree am I supposed to look at?

net-next

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [pull request][for-next 0/6] Mellanox mlx5 updates 2017-05-23

[Doug Ledford]

On Thu, 2017-05-25 at 12:02 -0400, David Miller wrote:
> From: Saeed Mahameed <sae...@mellanox.com>
> Date: Tue, 23 May 2017 14:43:58 +0300
> 
> > Hi Dave and Doug,
> > 
> > This series introduces some small updates and FPGA support to the
> mlx5
> > core/ethernet and IB drivers.
> > 
> > For more details please see below.
> > 
> > Please pull and let me know if there's any problem.
> 
> Ok, I've pulled this into net-next.
> 
> Doug let me know if there are any merge hassles we need to coordinate
> on.
> 
> Thanks.

Will do.  But is the question of a possible revert settled?  Because
once I pull, a revert means I have to pull all the way up to the revert
as well...

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH] net/smc: mark as BROKEN due to remote memory exposure

[Doug Ledford]

On 5/17/2017 6:37 PM, Parav Pandit wrote:
> Hi Doug,
> 

>> It would have been better with AF_INET/AF_INET6 and an option to enable
>> SMC than AF_SMC.  The first implementation simply assumes AF_INET in
>> the presence of AF_SMC.  When IPv6 support is added, some sort of
>> guessing logic will have to be put in place to try and determine if an
>> AF_SMC address is actually AF_INET or AF_INET6 since we won't have a
>> guaranteed way of telling.  Apps can use struct sockaddr_storage as their
>> normal element to stick the address into, and could rely on the kernel to
>> interpret it properly based on the AF_INET/AF_INET6 differentiation, and
>> this breaks that.  The RFC gives *some* thought to adding IPv6 in the
>> future, but not a lot.  It may be that the answer is that in the future, IPv6
>> support is enabled by making the IPv6 API be
>> AF_INET6 + setsockopt(SMC) or the equivalent.  If that's the case, then I
>> would suggest making the later API specifically call out AF_INET +
>> setsockopt(SMC) be identical to AF_SMC.
>>
> 
> What are the shortcomings in my proposal [1] which I am reiterating below.
> Bart also suggested to define new stream protocol for SMC similar to SCTP.
> 
> (a) address family should be either AF_INET or AF_INET6
> (b) socket() API to introduce new protocol as PROTO_SMC for in socket() 
> system call.
> 
> With this there is no additional setsockop() needed.
> 
> With this - user space applications, do getaddrinfo() with hint as
> hints.ai_family  = AF_INET;
> hints.ai_socktype = SOCK_STREAM;
> 
> getaddrinfo() returns back both the protocols TCP and SMC.
> Famous database application such as Redis client iterates over all entries of 
> getaddrinfo() and establishes connection to servers.
> 
> There are few advantages of this interface.
> 1. No change in any makefile of applications needed - unless user wants to 
> specify explicitly that it wants to force SMC protocol.
> 2. No need to do LD_LIBRARY_PRELOAD, (which won't work anyway because bind() 
> connect() of SMC checks for AF_INET).
> 3. No major changes to glibc to process AF_SMC differently
> glibc references IPPROTO_TCP at 22 places. (compare to AF_INET at 140+ 
> places).
> A lot simpler test matrix for glibc for new protocol
> 5. No need to recompile applications, as long as getaddrinfo returns all 
> streaming protocols (TCP, SMC)
> 6. for applications to make use of setsockopt() it needs another knob and 
> hint from other places, which can be avoided because SMC TCP option 
> negotiates with remote end
> 
> And representing new protocol as new protocol for a given address family 
> appears correct, compare to setting socket options.
> 
> Tools like CRIU or similar tool might find a race conditions - when it 
> queries socket option, SMC was not set, but later on SMC was set, and does 
> incorrect handling.
> Setting socket() with SMC protocol makes it easier to understand in this area 
> as well.

I have no problem with the proposal in itself, but as IBM released this
publication and did their own implementation prior to submitting things
upstream, and as there might exist in the field implementations, it
depends on whether we wish to call those in the field implementations
experimental and break them as we go to a final implementation of
version 1, or if we consider version 1 baked.  I'm fine breaking it.
After all, that's what happened with XRC back in the day and Mellanox
learned a valuable lesson about upstream first.  I have no problem with
IBM learning that same lesson IMO.  So, I find your proposal, including
breaking the API of the version 1 implementation just taken into the
kernel before it has had time to fully sit and gel, acceptable.

But this is where we kind of need a judgment from on high, and why I
Cc:ed Linus on this thread.  Any input on this issue Linus?

> I have additional proposal for link groups, resource creation area. I will 
> take that up after this discussion.

Look forward to hearing it.

> [1] https://patchwork.kernel.org/patch/9719375/


-- 
Doug Ledford <dledf...@redhat.com>
GPG Key ID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] net/smc: mark as BROKEN due to remote memory exposure

[Doug Ledford]

On Tue, 2017-05-16 at 15:28 -0400, Doug Ledford wrote:
> I hadn't realized EXPERIMENTAL was gone.  Which is too bad, because
> that's entirely appropriate in this case, and would have had the
> desired side effect of keeping it out of any non-cutting edge distros
> and warning people of possible API changes.  With EXPERIMENTAL gone,
> the closest thing we have is drivers/staging, since that tends to
> imply
> some of the same consequences.  I know you think BROKEN is overly
> harsh, but I'm not sure we should just do nothing.  How about we take
> a
> few days to let some of the RDMA people closely review the 143 page
> (egads!) rfc (http://www.rfc-editor.org/info/rfc7609) to see if we
> think it can be fixed to use multiple link layers with the existing
> API
> in place or if it will require something other than AF_SMC.  If we
> need
> to break API, then I think we should either fix it ASAP and send that
> fix to the 4.11 stable series (which probably violates the normative
> stable patch size/scope) or if the fix will take longer than this
> kernel cycle, then move it to staging both here and in 4.11 stable,
> and
> fix it there and then move it back.  Something like that would
> prevent
> the kind of API flappage we ought not do

So, I've skimmed the entire RFC, focusing on things were I needed to.
 Here's my take on it:

It would have been better with AF_INET/AF_INET6 and an option to enable
SMC than AF_SMC.  The first implementation simply assumes AF_INET in
the presence of AF_SMC.  When IPv6 support is added, some sort of
guessing logic will have to be put in place to try and determine if an
AF_SMC address is actually AF_INET or AF_INET6 since we won't have a
guaranteed way of telling.  Apps can use struct sockaddr_storage as
their normal element to stick the address into, and could rely on the
kernel to interpret it properly based on the AF_INET/AF_INET6
differentiation, and this breaks that.  The RFC gives *some* thought to
adding IPv6 in the future, but not a lot.  It may be that the answer is
that in the future, IPv6 support is enabled by making the IPv6 API be
AF_INET6 + setsockopt(SMC) or the equivalent.  If that's the case, then
I would suggest making the later API specifically call out AF_INET +
setsockopt(SMC) be identical to AF_SMC.

The protocol included a version header in the negotation messages.
 This is good as it allows us to almost immediately start work on
version 2 that fixes the shortcomings of version 1 while maintaining
back compatibility.

After reading the RFC, I can see why they only support one link layer:
RoCE.  The issue here is that they punted on the hard issue of doing
any sort of work to determine if security restrictions on the TCP
connection should also be applied to the RDMA connection.  The RFC
basically says "the RoCE link needs to have the same physical
restrictions (vlan) as the TCP/IP link so that the security limitations
are the same" because they don't do anything to check them essentially.
 For v2 of the protocol, and for different link layer support, this is
no longer sufficient, so there will be significant work to determine
the security context of specific TCP connections and then make sure
that they meet the security context of the RDMA links allowed.

Additionally, the same is likely to be true in terms of SELinux
options.  The addition of the IB/OPA link layers will throw a bit of a
monkey wrench in things because the SELinux control over those links is
slightly different than a normal TCP/IP SELinux control.  For instance,
you might create rules about processes and ports to make sure that the
httpd daemon can only access specific ports on TCP/IP, but on IB/OPA
you would need to create process and P_Key rules as IB/OPA don't have
the same port level semantics, and it's the P_Key on communications
that is enforced network wide, including in the switches, so
controlling what P_Key a process can send/receive on is your best way
to limit what a process can do.  Translation from one to the other
might be rather difficult to do in any sort of automated fashion.

There might have to be some additional work to get this to properly
account items for the RDMA control group elements that were recently
taken into the kernel.

Finally, the RDMA subsystem is in the process of switching to
structured option processing similar to how netlink does it.  For
version 2 of this protocol, since it will be interacting with the RDMA
core in many ways, will be simpler if it switches the on-wire
negotiation packets to follow the same methods as that will allow reuse
of code that it will have to have for properly dealing with the RDMA
subsystem in the future.

So, I'm fine with it being left as is since you accepted the patch that
makes note of the memory registration insecurity in the Kconfig text.
 The fact that this is a versioned protocol means that we can fix the
things we see as bein

Re: [PATCH] net/smc: mark as BROKEN due to remote memory exposure

[Doug Ledford]

On Tue, 2017-05-16 at 14:52 -0400, David Miller wrote:
> From: Doug Ledford <dledf...@redhat.com>
> Date: Tue, 16 May 2017 14:03:22 -0400
> 
> > On Tue, 2017-05-16 at 13:36 -0400, David Miller wrote:
> >> From: Doug Ledford <dledf...@redhat.com>
> >> Date: Tue, 16 May 2017 13:20:44 -0400
> >> 
> >> > Anyway, we're just talking out what happened, when what we
> really
> >> need
> >> > to focus on is moving forward.  Again, your thoughts on marking
> SMC
> >> > EXPERIMENTAL until it's fixed up and unfreezing the API in case
> we
> >> need
> >> > to adjust it to work on different link layers?
> >> 
> >> Something like:
> >> 
> >> http://patchwork.ozlabs.org/patch/762803/
> >> 
> >> with the addition of the EXPERIMENTAL dependency?
> >> 
> >> Sure.
> > 
> > Perfect.  I assume you'll submit it since it's in your patchworks?
> 
> Ok I applied the patch referenced above, but we don't actually have
> an EXPERIMENTAL symbol.  The closest thing we have is BROKEN and
> even in this situation that's a bit harsh.

I hadn't realized EXPERIMENTAL was gone.  Which is too bad, because
that's entirely appropriate in this case, and would have had the
desired side effect of keeping it out of any non-cutting edge distros
and warning people of possible API changes.  With EXPERIMENTAL gone,
the closest thing we have is drivers/staging, since that tends to imply
some of the same consequences.  I know you think BROKEN is overly
harsh, but I'm not sure we should just do nothing.  How about we take a
few days to let some of the RDMA people closely review the 143 page
(egads!) rfc (http://www.rfc-editor.org/info/rfc7609) to see if we
think it can be fixed to use multiple link layers with the existing API
in place or if it will require something other than AF_SMC.  If we need
to break API, then I think we should either fix it ASAP and send that
fix to the 4.11 stable series (which probably violates the normative
stable patch size/scope) or if the fix will take longer than this
kernel cycle, then move it to staging both here and in 4.11 stable, and
fix it there and then move it back.  Something like that would prevent
the kind of API flappage we ought not do

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH] net/smc: mark as BROKEN due to remote memory exposure

[Doug Ledford]

On Tue, 2017-05-16 at 13:36 -0400, David Miller wrote:
> From: Doug Ledford <dledf...@redhat.com>
> Date: Tue, 16 May 2017 13:20:44 -0400
> 
> > Anyway, we're just talking out what happened, when what we really
> need
> > to focus on is moving forward.  Again, your thoughts on marking SMC
> > EXPERIMENTAL until it's fixed up and unfreezing the API in case we
> need
> > to adjust it to work on different link layers?
> 
> Something like:
> 
> http://patchwork.ozlabs.org/patch/762803/
> 
> with the addition of the EXPERIMENTAL dependency?
> 
> Sure.

Perfect.  I assume you'll submit it since it's in your patchworks?

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH] net/smc: mark as BROKEN due to remote memory exposure

[Doug Ledford]

On Tue, 2017-05-16 at 12:49 -0400, David Miller wrote:
> From: Doug Ledford <dledf...@redhat.com>
> Date: Tue, 16 May 2017 12:42:43 -0400
> 
> > On Tue, 2017-05-16 at 12:29 -0400, David Miller wrote:
> >> From: Doug Ledford <dledf...@redhat.com>
> >> Date: Tue, 16 May 2017 11:57:04 -0400
> >> 
> >> > Regardless though, I'm rather purturbed about this entire thing.
> >>  If
> >> > you are right that because this got into 4.11, it's now a done
> >> deal,
> >> > then the fact that this went through 4 review cycles on netdev@
> >> that,
> >> > as I understand it, spanned roughly one years time, and not one
> >> single
> >> > person bothered to note that this was as much an RDMA driver as
> >> > anything else, and not one person bothered to note that linux-
> rdma@ 
> >> was
> >> > not on the Cc: list, and not one person told the submitters that
> >> they
> >> > needed to include linux-rdma@ on the Cc: list of these
> submissions,
> >> and
> >> > you took it without any review comments from any RDMA people in
> the
> >> > course of a year, or an ack from me to show that the RDMA
> portion
> >> of
> >> > this had at least been given some sort of review, was a collosal
> >> fuckup
> >> > of cross tree maintainer cooperation.
> >> 
> >> We rely on people from various areas of expertiece to contribute
> to
> >> patch review on netdev and give appropriate feedback.
> >> 
> >> If you actually look through the history, I made many semantic
> >> reviews
> >> of the SMC changes, and kept pushing back.
> >> 
> >> And in fact I did this several times, making them go through
> several
> >> revisions, in the hopes that someone would review more of the meat
> >> and
> >> substance of the patch set.
> > 
> > If you want to walk to the mailbox, you walk to the mailbox, you
> don't
> > walk to the grocery store, to the gym, and never even go to the
> > mailbox.  Likewise, if you want review from RDMA experts, most
> (maybe
> > even all) don't subscribe to netdev@ because it's too high traffic,
> you
> > don't waste time on silly semantic pushbacks, you send a single
> email
> > that says "Please get review from linux-rdma@, thank you."  Don't
> beat
> > around the bush, be direct and get it over with.  That's exactly
> what I
> > do for all netdev@ related patches coming to linux-rdma@ without a
> > proper Cc: to netdev@.
> 
> Read my other email, it wasn't %100 clear to me that this was so
> strictly RDMA related.  And I kept pushing back with semantic changes
> in part because it wasn't clear.

See my other email.  I think in the fact that you are overloaded on
netdev@ you skimmed the cover letter, which is the one thing that would
have made things clear.

> So as far as I was concerned I was not necessarily going to the wrong
> store, in fact I wasn't sure what store to go to.
> 
> And none of the thousands of subscribers to netdev intuit'd this
> either.  Maybe there is a reason for that.

Yeah, it's an overloaded list would be my guess.  I imagine no one can
keep up with it fully in truth.  Maybe it's time to split some of it
out into sublists?  netdev-core/netdev-ethernet/netdev-packet?  I don't
know, but I know I can't keep up with it.  That you do as well as you
do is probably only because you know how to not spend too much time on
any one thing.  I'm sure you have people you know are submitting
important work that effects the overall net subsystem and you focus on
their stuff, but ancillary things probably only get half attention at
double speed in order to allow you to make it through the day.

> Furthermore, if netdev is too much traffic for one or two RDMA people
> to just casually subscribe to on the off chance that a situationm
> like
> this comes up, guess what it is like for me who has to read and
> review
> pretty much every single posting that is placed there?

I get it.  I don't have the time to both be responsible for linux-rdma
and follow netdev.  I can already tell you what would happen if I
tried.  Eventually netdev would get so far behind I would just mass
delete and start over.  So that doesn't solve the problem.

Anyway, we're just talking out what happened, when what we really need
to focus on is moving forward.  Again, your thoughts on marking SMC
EXPERIMENTAL until it's fixed up and unfreezing the API in case we need
to adjust it to work on different link layers?

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH] net/smc: mark as BROKEN due to remote memory exposure

[Doug Ledford]

On Tue, 2017-05-16 at 12:41 -0400, David Miller wrote:
> From: Doug Ledford <dledf...@redhat.com>
> Date: Tue, 16 May 2017 12:36:01 -0400
> 
> > On Tue, 2017-05-16 at 18:30 +0200, Christoph Hellwig wrote:
> >> On Tue, May 16, 2017 at 12:29:23PM -0400, David Miller wrote:
> >> > 
> >> > I can't push back on people with silly coding style and small
> >> > semantic
> >> > issues forever.  And I think I made a serious effort to keep the
> >> > patches getting posted over and over again to make sure they got
> >> > more
> >> > exposure.
> >> 
> >> You can tell them to go to linux-rdma.  I'm sending people to the
> >> right
> >> mailing list all the time.
> > 
> > Indeed.  Every single time a patch comes into linux-rdma that
> touches
> > things in net/ or include/net, unless it is exceedingly minor, I
> check
> > the To:/Cc: lines on the email and if netdev@ isn't included, or in
> the
> > case of complex/tricky items, you aren't directly Cc:ed, then I
> > specifically tell them to include netdev@ and/or you.  I've even
> had
> > things like a 12 patch series that buried three netdev@ appropriate
> > patches at different points in the series and told the submitter to
> > move all of the netdev@ related patches to the front and submit
> them to
> > netdev@ so they can be reviewed as a group before I would move on
> to
> > the others.  It's just what you do.  I've always considered that
> part
> > of my job.
> 
> To be quite honest it wasn't exceedingly clear, even to me, that this
> had such implications or was directly a RDMA thing.  From my
> perspective while reviewing I saw a patch series adding it's own
> protocol stack living inside of it's own directory under net/

OK.  Fair enough.  That implies to me that you probably dove right into
the patches and skimmed the cover letter (http://marc.info/?l=linux-s39
0=148397751211964=2), because when I read the cover letter, it
seems pretty clear to me that this is very much RDMA related.  In any
case, there are already two other code bases that live under net/ but
also involve RDMA heavily: nfs and rds.  This is a third now.  So, for
future reference, just like the RDMA related nfs/rds patches already
get Cc:ed to linux-rdma@, these probably should be too.

> And, if even one RDMA/infiniband person said to me "you really
> shouldn't apply this" then I would have dropped it on the spot.

I'm glad to hear that.  I wish we had managed to get together on this
sooner.

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH] net/smc: mark as BROKEN due to remote memory exposure

[Doug Ledford]

On Tue, 2017-05-16 at 12:29 -0400, David Miller wrote:
> From: Doug Ledford <dledf...@redhat.com>
> Date: Tue, 16 May 2017 11:57:04 -0400
> 
> > Regardless though, I'm rather purturbed about this entire thing.
>  If
> > you are right that because this got into 4.11, it's now a done
> deal,
> > then the fact that this went through 4 review cycles on netdev@
> that,
> > as I understand it, spanned roughly one years time, and not one
> single
> > person bothered to note that this was as much an RDMA driver as
> > anything else, and not one person bothered to note that linux-rdma@ 
> was
> > not on the Cc: list, and not one person told the submitters that
> they
> > needed to include linux-rdma@ on the Cc: list of these submissions,
> and
> > you took it without any review comments from any RDMA people in the
> > course of a year, or an ack from me to show that the RDMA portion
> of
> > this had at least been given some sort of review, was a collosal
> fuckup
> > of cross tree maintainer cooperation.
> 
> We rely on people from various areas of expertiece to contribute to
> patch review on netdev and give appropriate feedback.
> 
> If you actually look through the history, I made many semantic
> reviews
> of the SMC changes, and kept pushing back.
> 
> And in fact I did this several times, making them go through several
> revisions, in the hopes that someone would review more of the meat
> and
> substance of the patch set.

If you want to walk to the mailbox, you walk to the mailbox, you don't
walk to the grocery store, to the gym, and never even go to the
mailbox.  Likewise, if you want review from RDMA experts, most (maybe
even all) don't subscribe to netdev@ because it's too high traffic, you
don't waste time on silly semantic pushbacks, you send a single email
that says "Please get review from linux-rdma@, thank you."  Don't beat
around the bush, be direct and get it over with.  That's exactly what I
do for all netdev@ related patches coming to linux-rdma@ without a
proper Cc: to netdev@.

> Nobody do this for over a year.
> 
> I can't push back on people with silly coding style and small
> semantic
> issues forever.  And I think I made a serious effort to keep the
> patches getting posted over and over again to make sure they got more
> exposure.
> 
> I think it's unsettling that there are no RDMA experts, or at least
> people remotely knowledgable about this "networking" technology,
> subscribed to netdev and taking a cursory look at pactches that might
> be relevant and effect that technology either directly or indirectly.
> 
> So there is a lot of blame to go around.

Fine, allocate blame however, you like.  What I want to actually settle
is how we are going to move forward.  You didn't respond to that part
of my email.  Your thoughts?

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH] net/smc: mark as BROKEN due to remote memory exposure

[Doug Ledford]

On Tue, 2017-05-16 at 18:30 +0200, Christoph Hellwig wrote:
> On Tue, May 16, 2017 at 12:29:23PM -0400, David Miller wrote:
> > 
> > I can't push back on people with silly coding style and small
> > semantic
> > issues forever.  And I think I made a serious effort to keep the
> > patches getting posted over and over again to make sure they got
> > more
> > exposure.
> 
> You can tell them to go to linux-rdma.  I'm sending people to the
> right
> mailing list all the time.

Indeed.  Every single time a patch comes into linux-rdma that touches
things in net/ or include/net, unless it is exceedingly minor, I check
the To:/Cc: lines on the email and if netdev@ isn't included, or in the
case of complex/tricky items, you aren't directly Cc:ed, then I
specifically tell them to include netdev@ and/or you.  I've even had
things like a 12 patch series that buried three netdev@ appropriate
patches at different points in the series and told the submitter to
move all of the netdev@ related patches to the front and submit them to
netdev@ so they can be reviewed as a group before I would move on to
the others.  It's just what you do.  I've always considered that part
of my job.

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH] net/smc: mark as BROKEN due to remote memory exposure

[Doug Ledford]

Hi Linus,

I've added you to this thread.  A quick synopsis: Dave sent you the
net/smc driver for 4.11.  Even though it lives in net/smc, it is, for
the most part, a net<->rdma translator and so it is as much an RDMA
driver as anything else.  And upon review, the rdma community does not
believe either the spec/rfc or the driver are the right way to engineer
this particular technology, and the implementation leaves much to be
desired.

On Sun, 2017-05-14 at 20:44 -0400, David Miller wrote:
> From: Bart Van Assche <bart.vanass...@sandisk.com>
> Date: Sun, 14 May 2017 19:08:50 +
> 
> > What is your plan to avoid that applications start using and
> > depending on AF_SMC?
> 
> The API is out there already so we are out of luck, and neither
> you nor I nor anyone else can "stop" this from happening.

That's not true at all.  There's nothing that says we can't revert this
now before it goes any further.  It's only been in two kernels, I'm
positive it hasn't landed in any distros yet, and it can go back to
being something people can add on the side.  Futher, the "standard"
this is based on is not a real standard, it's just a publication and
has not been through a standard track.  I wouldn't consider this "out
there already" until there is a standard that has gone through the
standard track.

Regardless though, I'm rather purturbed about this entire thing.  If
you are right that because this got into 4.11, it's now a done deal,
then the fact that this went through 4 review cycles on netdev@ that,
as I understand it, spanned roughly one years time, and not one single
person bothered to note that this was as much an RDMA driver as
anything else, and not one person bothered to note that linux-rdma@ was
not on the Cc: list, and not one person told the submitters that they
needed to include linux-rdma@ on the Cc: list of these submissions, and
you took it without any review comments from any RDMA people in the
course of a year, or an ack from me to show that the RDMA portion of
this had at least been given some sort of review, was a collosal fuckup
of cross tree maintainer cooperation.

The SMC driver makes several mistakes that people tried to avoid with
previous RDMA standards, it only supports one out of the five possible
link layers (iWARP, IB, OPA, RoCEv1, RoCEv2), it uses a highly
discouraged and deprecated technique for memory registration that is
considered horribly insecure (handing the keys to the castle to anyone
who connects to the machine, aka, the entire memory space is registered
with one key and that key is given to remote connections, so they can
read any bit of kernel memory they want as opposed to whatever we tell
them to read), and the design as articuled in the published rfc seems
incomplete for dealing with any of the other link layers, indicating
that this should have probably stayed out until the rfc was discussed
and updated to address the shortcomings obviously present in the
current rfc.  With all of these issues outstanding against it, I hope
you can see why I think the way I do about you taking it without ever
consulting any of the RDMA community.

But that leaves us with the question of what to do moving forward.
 Probably the number one concern is that this protocol chose to create
a new AF as opposed to reusing the IPv4 and IPv6 address families and
adding an option similar to SCTP for enabling the new protocol on a
specific socket.  The concern is that we have means of addressing all
of the link layers the RDMA subsystem supports using IPv4 or IPv6 (sort
of...it's possible to have IB or OPA without IPoIB, which leaves them
without an IPv4 or IPv6 address, in which case the rdmacm can use
native GUIDs to resolve the other side, but that only works for verbs
connections, in the case of TCP connections, we always require IPoIB to
be present, and so IPv4 or IPv6 is always sufficient).  In the end,
switching this protocol to use AF_INET and AF_INET6 and a protocol
option to enable SMC may be what we need to do.  That, of course,
changes the user space API.  So, are we truly locked in at this point?
 I would suggest that, since this is only present in 4.11 and 4.12, and
I'm sure this has not landed in any distros as of yet (except maybe
something like Fedora rawhide), we can submit a patch to both the
current kernel and the 4.11 stable to set this code as
CONFIG_EXPERIMENTAL and mark the API as possibly going to undergo
change.  Then let the RDMA community work with IBM to get this properly
fixed so that this is a reasonable RDMA driver and not something the
community is ready to immediately trash, and only after we've got it
whipped into shape and the RDMA community is satisfied it is a
reasonable driver that can continue to work with future planned RDMA
subsystem updates and across various link layers, we remove the
EXPERIMENTAL marker and freeze the API for user space.

-- 
Doug Ledford <dledf...@redhat.com>

Re: [RFC iproute2 0/8] RDMA tool

[Doug Ledford]

On Thu, 2017-05-04 at 15:26 -0400, Dennis Dalessandro wrote:
> On 05/04/2017 02:45 PM, Leon Romanovsky wrote:
> > 
> > On Thu, May 04, 2017 at 06:30:27PM +, Bart Van Assche wrote:
> > > 
> > > On Thu, 2017-05-04 at 21:25 +0300, Leon Romanovsky wrote:
> > > > 
> > > > On Thu, May 04, 2017 at 06:10:54PM +, Bart Van Assche
> > > > wrote:
> > > > > 
> > > > > On Thu, 2017-05-04 at 21:02 +0300, Leon Romanovsky wrote:
> > > > > > 
> > > > > > Following our discussion both in mailing list [1] and at
> > > > > > the LPC 2016 [2],
> > > > > > we would like to propose this RDMA tool to be part of
> > > > > > iproute2 package
> > > > > > and finally improve this situation.
> > > > > 
> > > > > Hello Leon,
> > > > > 
> > > > > Although I really appreciate your work: can you clarify why
> > > > > you would like to
> > > > > add *RDMA* functionality to an *IP routing* tool? I haven't
> > > > > found any motivation
> > > > > for adding RDMA functionality to iproute2 in [1].
> > > > 
> > > > We are planning to reuse the same infrastructure provided by
> > > > iproute2,
> > > > like netlink parsing, access to distributions, same CLI and
> > > > same standards.
> > > > 
> > > > Right now, RDMA is already tightened to netdev: iWARP, RoCE,
> > > > IPoIB, HFI-VNIC.
> > > > Many drivers (mlx, qed, i40, cxgb) are sharing code between net
> > > > and
> > > > RDMA.
> > > > 
> > > > I do expect that iproute2 will be installed on every machine
> > > > with any
> > > > type of connection, including IB and OPA.
> > > > 
> > > > So I think that it is enough to be part of that suite and don't
> > > > invent
> > > > our own for one specific tool.
> > > 
> > > Hello Leon,
> > > 
> > > Sorry but to me that sounds like a weak argument for including
> > > RDMA functionality
> > > in iproute2. There is already a library for communication over
> > > netlink sockets,
> > > namely libnl. Is there functionality that is in iproute2 but not
> > > in libnl and
> > > that is needed for the new tool? If so, have you considered to
> > > create a new
> > > library for that functionality?
> > 
> > It is not hard to create new tool, the hardest part is to ensure
> > that it is
> > part of the distributions. Did you count how many months we are
> > trying to
> > add rdma-core to debian?
> 
> I do agree that it is a strange pairing and am not really a fan.
> However 
> at the end of the day it's just a name for a repo/package. If the 
> iproute folks are fine to include rdma in their repo/package, great
> we 
> can leverage their code for CLI and other common stuff.

If you look into the iproute2 package, it becomes clear that the name
iproute2 is historical and not really accurate any more.  It contains
things like the bridge control software, tc for controlling send
queues, and many things network related but not routing related.  The
rdma tool is a perfectly fine fit in the sense that it is an additional
network management tool IMO.

For reference, here's the list of stuff already in iproute on my Fedora
24 box:

/usr/sbin/arpd
/usr/sbin/bridge
/usr/sbin/cbq
/usr/sbin/ctstat
/usr/sbin/genl
/usr/sbin/ifcfg
/usr/sbin/ifstat
/usr/sbin/ip
/usr/sbin/lnstat
/usr/sbin/nstat
/usr/sbin/routef
/usr/sbin/routel
/usr/sbin/rtacct
/usr/sbin/rtmon
/usr/sbin/rtpr
/usr/sbin/rtstat
/usr/sbin/ss
/usr/sbin/tc
/usr/sbin/tipc

And in fact, if you check, tipc is almost similar to RDMA ;-)  So, I
suggest people not get hung up on the name iproute2, the fit is fine
when you look deeper into the nature of the package.

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: net/smc and the RDMA core

[Doug Ledford]

sn't really a reason not to support them.  The real issue is the much
more difficult issue of namespaces and SELinux, which is different
between TCP sockets and IB/OPA connections.  That would have been the
difficult part to get right, but as we haven't investigated it, we
really don't know if it would have been solvable in a reasonable fashion.

>> * Since functionality that is similar what the SMC driver provides already
>>   exists in user space (rsockets), why has this functionality been
>>   reimplemented as a kernel driver (SMC)?
>>
>> Bart.
>>
> 
> Regards, Ursula
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
Doug Ledford <dledf...@redhat.com>
GPG Key ID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 1/1] IB/mlx5: Add port_xmit_wait to counter registers read

[Doug Ledford]

On Mon, 2017-05-01 at 17:30 +0100, Tim Wright wrote:
> Add port_xmit_wait to the error counters read by mlx5_ib_process_mad
> to
> ensure sysfs port counter provides correct value for PortXmitWait.
> Otherwise the sysfs port_xmit_wait file always contains zero.
> 
> The previous MAD_IFC implementation populated this counter, but it
> was
> removed during the migration to PPCNT for error counters (32-bit
> only).
> 
> Signed-off-by: Tim Wright <t...@binbash.co.uk>

Thanks, applied.

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH] IB/IPoIB: Check the headroom size

[Doug Ledford]

On Wed, 2017-04-26 at 22:11 +0800, Honggang LI wrote:
> On Wed, Apr 26, 2017 at 09:50:38AM -0400, Doug Ledford wrote:
> > 
> > On Wed, 2017-04-26 at 09:48 -0400, Doug Ledford wrote:
> > > 
> > > On Wed, 2017-04-26 at 21:33 +0800, Honggang LI wrote:
> > > > 
> > > > 
> > > > Yes, it is during the process of removing the final slave. The
> > > > reproducer looks like this:
> > > > 
> > > > ping remote_ip_over_bonding_interface &
> > > > while 1; do
> > > > ifdown bond0
> > > > ifup   bond0
> > > > done
> > 
> > BTW, rerunning your test as:
> > 
> > ping remote_ip_over_bonding_interface &
> > while 1; do
> >     echo -n "Downing interface..."
> >     ifdown bond0
> 
> Confirmed panic in here.

OK, this leads me to suspect that the bonding driver is possibly
reconfiguring the skb setup to Ethernet mode before the last slave is
fully dropped from the bond, and so the slave is seeing a packet to its
hard header routine with the wrong headroom.  I think Paolo's patch is
probably the best way to go.  The reason I say that is because we can't
definitively say that this is the only pathway we will ever see that
causes us to get a packet with Ethernet headroom on our IPoIB
interface, and Paolo's patch gives us the possibility of recovering and
sending the packet out.  Even in this case, we are downing the
interface, but it isn't down entirely yet, so sending the packet out
isn't necessarily wrong.  So a method of fix that allows us to
recover/continue seems preferable to me.  Can you test Paolo's patch
and see if it resolves the problem?

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH] IB/IPoIB: Check the headroom size

[Doug Ledford]

On Wed, 2017-04-26 at 09:48 -0400, Doug Ledford wrote:
> On Wed, 2017-04-26 at 21:33 +0800, Honggang LI wrote:
> > 
> > Yes, it is during the process of removing the final slave. The
> > reproducer looks like this:
> > 
> > ping remote_ip_over_bonding_interface &
> > while 1; do
> > ifdown bond0
> > ifup   bond0
> > done

BTW, rerunning your test as:

ping remote_ip_over_bonding_interface &
while 1; do
    echo -n "Downing interface..."
    ifdown bond0
    echo "done."
    sleep 1
    echo -n "Bringing interface up..."
    ifup bond0
    echo "done."
done

will allow you to more precisely identify whether the failure is during
the down or the up of the interface.

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH] IB/IPoIB: Check the headroom size

[Doug Ledford]

On Wed, 2017-04-26 at 21:33 +0800, Honggang LI wrote:
> Yes, it is during the process of removing the final slave. The
> reproducer looks like this:
> 
> ping remote_ip_over_bonding_interface &
> while 1; do
>   ifdown bond0
>   ifup   bond0
> done

Honestly, I would suspect the problem here is not when removing the
busy interface, but when bringing the interface back up.  IIRC, the
bonding driver defaults to assuming it will be used on an Ethernet
interface.  Once you attach an IB slave, it reconfigures itself for
running over IB instead.  But once it's configured it should stay
configured until after the last IB slave is removed (and once that
slave it removed, the bond should no longer even possess the pointer to
the ipoib_hard_header routine, so we should never call it).

The process, in the bonding driver, for going down and coming back up
needs to look something like this:

ifdown bond0:
  stop all queues
  remove all slaves
  possibly reconfigure to default state which is Ethernet suitable

ifup bond0:
  add first slave
  configure for IB instead of Ethernet
  start queues
  add additional slaves

I'm wondering if, when we have a current backlog, we aren't
accidentally doing this instead:

ifup bond0:
  add first slave
  release backlog queue
  configure for IB instead of Ethernet
  add additional slaves

Or, it might even be more subtle, such as:

ifup bond0:
  add first slave
  configure for IB instead of Ethernet
  start queues
   -> however, there was a backlog item on the queue from prior to
      the first slave being added and the port configured for IB mode,
      so the backlog skb is still configured for the default Ethernet
      mode, hence the failure
  add additional slaves

Maybe the real issue is that inside the bonding driver, when we
reconfigure the queue type from IB to Ethernet or Ethernet to IB, we
need to force either a drop or a reconfiguration of any packets already
currently in our waiting backlog queue of skbs.  Paolo?

> > 
> > but I don't think it can be after the final slave has been fully
> > removed from the bond.  Paolo, what should the bond driver be doing
> > once the slaves are gone?  Wouldn't it just be dropping every skb
> > on
> > the floor without calling anyone's hard header routine?
> > 
> > > 
> > >  so it is better to immediately
> > > give up and return error.
> > > 
> > > > 
> > > > 
> > > > +   if (ret)
> > > > +   return ret;
> > > >  
> > > > header = (struct ipoib_header *) skb_push(skb, sizeof
> > > > *header);
> > > > ---
> > > > 
> > > > Paolo
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe
> > > > linux-
> > > > rdma" in
> > > > the body of a message to majord...@vger.kernel.org
> > > > More majordomo info at  http://vger.kernel.org/majordomo-info.h
> > > > tml
> > -- 
> > Doug Ledford <dledf...@redhat.com>
> >     GPG KeyID: B826A3330E572FDD
> >    
> > Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57
> > 2FDD
> > 
-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH] IB/IPoIB: Check the headroom size

[Doug Ledford]

On Wed, 2017-04-26 at 20:52 +0800, Honggang LI wrote:
> On Wed, Apr 26, 2017 at 09:46:34AM +0200, Paolo Abeni wrote:
> > 
> > On Wed, 2017-04-26 at 00:50 +0300, Or Gerlitz wrote:
> > > 
> > > so maybe @ least for the time being, we should be picking Hong's
> > > patch
> > > with proper change log and without the giant stack dump till we
> > > have
> > > something better. If you agree, can you do the re-write of the
> > > change
> > > log?
> > 
> > I think that Hong's patch is following the correct way to fix the
> > issue: ipoib_hard_header() can't assume that the skb headroom is at
> > least IPOIB_HARD_LEN bytes, as wrongly implied by commit
> > fc791b633515
> > (my fault, I'm sorry).
> > 
> > Perhaps we can make the code a little more robust with something
> > alongside the following (only compile tested):
> > ---
> > diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > index d1d3fb7..d53d2e1 100644
> > --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > @@ -1160,6 +1160,11 @@ static int ipoib_hard_header(struct sk_buff
> > *skb,
> >  const void *daddr, const void *saddr,
> > unsigned len)
> >  {
> > struct ipoib_header *header;
> > +   int ret;
> > +
> > +   ret = skb_cow_head(skb, IPOIB_HARD_LEN);
> 
> I don't think so. When this skb_under_panic arise, all slaves had
> been
> removed from a busy bonding interface,

I'm not sure this entirely makes sense.  If all slaves had been
removed, then we should never end up in ipoib_hard_header.  That should
only be called as long as there is at least one slave still attached to
the bond.  It might be during the process of removing the final slave,
but I don't think it can be after the final slave has been fully
removed from the bond.  Paolo, what should the bond driver be doing
once the slaves are gone?  Wouldn't it just be dropping every skb on
the floor without calling anyone's hard header routine?

>  so it is better to immediately
> give up and return error.
> 
> > 
> > +   if (ret)
> > +   return ret;
> >  
> > header = (struct ipoib_header *) skb_push(skb, sizeof
> > *header);
> > ---
> > 
> > Paolo
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-
> > rdma" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [RESEND PATCH 3/3] infiniband: hns: avoid gcc-7.0.1 warning for uninitialized data

[Doug Ledford]

On Fri, 2017-03-24 at 23:02 +0100, Arnd Bergmann wrote:
> hns_roce_v1_cq_set_ci() calls roce_set_bit() on an uninitialized
> field,
> which will then change only a few of its bits, causing a warning with
> the latest gcc:
> 
> infiniband/hw/hns/hns_roce_hw_v1.c: In function
> 'hns_roce_v1_cq_set_ci':
> infiniband/hw/hns/hns_roce_hw_v1.c:1854:23: error: 'doorbell[1]' is
> used uninitialized in this function [-Werror=uninitialized]
>   roce_set_bit(doorbell[1], ROCEE_DB_OTHERS_H_ROCEE_DB_OTH_HW_SYNS_S,
> 1);
> 
> The code is actually correct since we always set all bits of the
> port_vlan field, but gcc correctly points out that the first
> access does contain uninitialized data.
> 
> This initializes the field to zero first before setting the
> individual bits.
> 
> Fixes: 9a4435375cd1 ("IB/hns: Add driver files for hns RoCE driver")
> Signed-off-by: Arnd Bergmann <a...@arndb.de>

Thanks, applied.

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH v4] {net,IB}/{rxe,usnic}: Utilize generic mac to eui32 function

[Doug Ledford]

On Mon, 2017-04-24 at 18:46 +0300, Yuval Shaia wrote:
> On Tue, Mar 14, 2017 at 07:58:43PM +0200, Leon Romanovsky wrote:
> > 
> > On Tue, Mar 14, 2017 at 04:01:57PM +0200, Yuval Shaia wrote:
> > > 
> > > This logic seems to be duplicated in (at least) three separate
> > > files.
> > > Move it to one place so code can be re-use.
> > > 
> > > Signed-off-by: Yuval Shaia <yuval.sh...@oracle.com>
> > > ---
> > > v0 -> v1:
> > >   * Add missing #include
> > >   * Rename to genaddrconf_ifid_eui48
> > > v1 -> v2:
> > >   * Reset eui[0] to default if dev_id is used
> > > v2 -> v3:
> > >   * Add helper function to avoid re-setting eui[0] to default if
> > >     dev_id is used
> > > v3 -> v4:
> > >   * Remove RXE wrappers
> > >   * Remove addrconf_addr_eui48_xor and do the eui[0] ^= 2 in the
> > >     basic implementation
> > > ---
> > >  drivers/infiniband/hw/usnic/usnic_common_util.h | 11 +++---
> > >  drivers/infiniband/sw/rxe/rxe.c |  4 +++-
> > >  drivers/infiniband/sw/rxe/rxe_loc.h |  2 --
> > >  drivers/infiniband/sw/rxe/rxe_net.c | 28 -
> > > 
> > >  drivers/infiniband/sw/rxe/rxe_verbs.c   |  4 +++-
> > >  include/net/addrconf.h  | 22
> > > +++
> > >  6 files changed, 27 insertions(+), 44 deletions(-)
> > > 
> > 
> > Thanks, Yuval.
> > Reviewed-by: Leon Romanovsky <leo...@mellanox.com>
> 
> Hi Doug,
> If no more comments on this one can you consider taking it?

It's applied, thanks.

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH 5/7] IB/hfi1: use pcie_flr instead of duplicating it

[Doug Ledford]

On Mon, 2017-04-24 at 16:35 +0200, Christoph Hellwig wrote:
> On Mon, Apr 24, 2017 at 02:16:31PM +, Byczkowski, Jakub wrote:
> > 
> > Tested-by: Jakub Byczkowski <jakub.byczkow...@intel.com>
> 
> Are you (and Doug) ok with queueing this up in the PCI tree?

I'm fine with that.  Feel free to add my Acked-by to the hfi1 patch.

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH] IB/IPoIB: Check the headroom size

[Doug Ledford]

On Tue, 2017-04-25 at 17:39 +0300, Or Gerlitz wrote:
> On Tue, Apr 25, 2017 at 2:43 PM, Erez Shitrit <ere...@dev.mellanox.co
> .il> wrote:
> > 
> > On Tue, Apr 25, 2017 at 2:14 PM, Or Gerlitz <gerlitz...@gmail.com>
> > wrote:
> 
> > 
> > > 
> > > thanks for the info. Is this bug there since ipoib/bonding day
> > > one (and hence my bug...)
> > > or was indeed introduced later? if later, can you explain how
> > > fc791b633515 introduced that or you only know it by bisection?
> 
> > 
> > commit "fc791b633515" changes the size of the dev_hardlen to be 24
> > and
> > required 24 extra bytes in the skb, before it was only 4, if skb is
> > aligned to eth "mode" it already has 14 bytes for hard-header.
> > So only after that commit we have the issue.
> 
> If got you right, Paolo's commit introduced a regression, so we (I
> guess you and
> Paolo) need to either solve it or we (community) should consider a
> revert, please suggest.

It's a little more complex than that.  Paolo's commit *re-introduced* a
regression.  If you recall, long ago the IPoIB layer stuck the dgid
into the skb, then pulled it out later.  However, we didn't actually
declare things properly back then, but it worked anyway.  Then we had
the commit you authored to start using the skb->cb to store the dgid,
and our usage of hard header dropped to only 4 bytes.  Paolo's commit
went back to the old way of doing things, but also did the proper
accounting and setup to tell the netstack what we were doing (which the
initial implementation never did IIRC).  So, this issue should be
reproducible either after Paolo's commit or with any kernel prior to
your commit to use the skb->cb area to store the DGID, but it probably
requires the specific series of actions in this bug to trigger it.  A
normal, clean shutdown of the interface doesn't demonstrate the issue.

> The bug is now in stable and distro kernels, so please act.

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH net-next 00/16] Mellanox, mlx5 RDMA net device support

[Doug Ledford]

On Thu, 2017-04-20 at 21:49 +0300, Leon Romanovsky wrote:
> On Thu, Apr 20, 2017 at 02:41:17PM -0400, Doug Ledford wrote:
> > 
> > On Mon, 2017-04-17 at 11:08 -0400, David Miller wrote:
> > > 
> > > From: Saeed Mahameed <sae...@mellanox.com>
> > > Date: Thu, 13 Apr 2017 06:36:50 +0300
> > > 
> > > > 
> > > > This series provides the lower level mlx5 support of RDMA
> > > > netdevice
> > > > creation API [1] suggested and introduced by Intel's HFI OPA
> > > > VNIC
> > > > netdevice driver [2], to enable IPoIB mlx5 RDMA netdevice
> > > > creation.
> > > 
> > > Series applied to net-next.
> > 
> > I've pulled Dave's net-next into a branch in my tree for the IPoIB
> > patches to go on top of so that way it can hold the final touchup
> > patch
> > without any conflicts anywhere.
> 
> Thanks,
> 
> How and when do you want us to forward it?

The branch in question is already pushed for 0day and linux-next
testing, so you can send the fixup patch to linux-rdma based upon my
k.o/for-4.12-rdma-netdevice branch any time.

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH net-next 00/16] Mellanox, mlx5 RDMA net device support

[Doug Ledford]

On Mon, 2017-04-17 at 11:08 -0400, David Miller wrote:
> From: Saeed Mahameed <sae...@mellanox.com>
> Date: Thu, 13 Apr 2017 06:36:50 +0300
> 
> > This series provides the lower level mlx5 support of RDMA netdevice
> > creation API [1] suggested and introduced by Intel's HFI OPA VNIC
> > netdevice driver [2], to enable IPoIB mlx5 RDMA netdevice creation.
> 
> Series applied to net-next.

I've pulled Dave's net-next into a branch in my tree for the IPoIB
patches to go on top of so that way it can hold the final touchup patch
without any conflicts anywhere.

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH rdma-next v2 00/12] Omni-Path Virtual Network Interface Controller (VNIC)

[Doug Ledford]

On Wed, 2017-04-12 at 20:29 -0700, Vishwanathapura, Niranjana wrote:
> ChangeLog:
> =
> v1 => v2:
> a) Change error code for unsupported rdma netdev type
> b) Remove some debug messages
> 
> v1: posted @ https://www.spinics.net/lists/linux-rdma/msg48518.html
> 
> v0 => v1:
> a) changes as required by new kernel base (for-4.12)
> b) moved rdma netdev interface into a separate patch 
> c) Return specific error code if specified rdma netdev type is not
> supported
> d) Some minor fixes (no changes to overall design or interface)
> 
> v0: Initial post @ https://www.spinics.net/lists/linux-rdma/msg46604.
> html
> 
> Description:
> 
> Intel Omni-Path (OPA) Virtual Network Interface Controller (VNIC)
> feature
> supports Ethernet functionality over Omni-Path fabric by
> encapsulating
> the Ethernet packets between HFI nodes.

I've pulled this into a custom branch based on DaveM's for-next tree so
that I could put the IPoIB changes into the same branch and the final
touchup patch Leon talked about into that branch as well.  So, this is
now queued for-next (albeit not on the normal branch).

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH v6 05/15] mlx4: Replace PCI pool old API

[Doug Ledford]

On Mon, 2017-03-20 at 08:32 +0200, Leon Romanovsky wrote:
> On Sun, Mar 19, 2017 at 06:03:54PM +0100, Romain Perier wrote:
> > 
> > The PCI pool API is deprecated. This commit replaces the PCI pool
> > old
> > API by the appropriate function with the DMA pool API.
> > 
> > Signed-off-by: Romain Perier <romain.per...@collabora.com>
> > Acked-by: Peter Senna Tschudin <peter.se...@collabora.com>
> > Tested-by: Peter Senna Tschudin <peter.se...@collabora.com>
> > ---
> >  drivers/net/ethernet/mellanox/mlx4/cmd.c  | 10 +-
> >  drivers/net/ethernet/mellanox/mlx4/mlx4.h |  2 +-
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> > 
> 
> Thanks,
> Reviewed-by: Leon Romanovsky <leo...@mellanox.com>

Changes look fine to me, and it has been tested on this specific
hardware and confirmed RDMA communications still work.

Acked-by: Doug Ledford <dledf...@redhat.com>
Tested-by: Doug Ledford <dledf...@redhat.com>

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH v6 03/15] IB/mthca: Replace PCI pool old API

[Doug Ledford]

On Sun, 2017-03-19 at 18:03 +0100, Romain Perier wrote:
> The PCI pool API is deprecated. This commit replaces the PCI pool old
> API by the appropriate function with the DMA pool API.
> 
> Signed-off-by: Romain Perier <romain.per...@collabora.com>
> Acked-by: Peter Senna Tschudin <peter.se...@collabora.com>
> Tested-by: Peter Senna Tschudin <peter.se...@collabora.com>
> 

Changes look ok, and I've tested them here as well.

Acked-by: Doug Ledford <dledf...@redhat.com>
Tested-by: Doug Ledford <dledf...@redhat.com>

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH v6 06/15] mlx5: Replace PCI pool old API

[Doug Ledford]

On Mon, 2017-03-20 at 08:31 +0200, Leon Romanovsky wrote:
> On Sun, Mar 19, 2017 at 06:03:55PM +0100, Romain Perier wrote:
> > 
> > The PCI pool API is deprecated. This commit replaces the PCI pool
> > old
> > API by the appropriate function with the DMA pool API.
> > 
> > Signed-off-by: Romain Perier <romain.per...@collabora.com>
> > Reviewed-by: Peter Senna Tschudin <peter.se...@collabora.com>
> > ---
> >  drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 11 ++-
> >  include/linux/mlx5/driver.h   |  2 +-
> >  2 files changed, 7 insertions(+), 6 deletions(-)
> > 
> 
> Thanks,
> Acked-by: Leon Romanovsky <leo...@mellanox.com>

Changes look fine to me, and in addition I've compiled and tested them
on mlx5 hardware and verified that RDMA communications still work.

Acked-by: Doug Ledford <dledf...@redhat.com>
Tested-by: Doug Ledford <dledf...@redhat.com>

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH] IB/rxe: increment msn only when completing a request

[Doug Ledford]

On Fri, 2017-02-24 at 15:38 +0100, David Marchand wrote:
> According to C9-147, MSN should only be incremented when the last
> packet of
> a multi packet request has been received.
> 
> "Logically, the requester associates a sequential Send Sequence
> Number
> (SSN) with each WQE posted to the send queue. The SSN bears a one-
> to-one relationship to the MSN returned by the responder in each re-
> sponse packet. Therefore, when the requester receives a response, it
> in-
> terprets the MSN as representing the SSN of the most recent request
> completed by the responder to determine which send WQE(s) can be
> completed."
> 
> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> 
> Signed-off-by: David Marchand <david.march...@6wind.com>

Thanks, applied.

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH] uapi: fix rdma/mlx5-abi.h userspace compilation errors

[Doug Ledford]

On Fri, 2017-02-24 at 03:28 +0300, Dmitry V. Levin wrote:
> Consistently use types from linux/types.h to fix the following
> rdma/mlx5-abi.h userspace compilation errors:
> 
> /usr/include/rdma/mlx5-abi.h:69:25: error: 'u64' undeclared here (not
> in a function)
>   MLX5_LIB_CAP_4K_UAR = (u64)1 << 0,
> /usr/include/rdma/mlx5-abi.h:69:29: error: expected ',' or '}' before
> numeric constant
>   MLX5_LIB_CAP_4K_UAR = (u64)1 << 0,
> 
> Include  to fix the following rdma/mlx5-abi.h
> userspace compilation error:
> 
> /usr/include/rdma/mlx5-abi.h:286:12: error: 'ETH_ALEN' undeclared
> here (not in a function)
>   __u8 dmac[ETH_ALEN];
> 
> Signed-off-by: Dmitry V. Levin <l...@altlinux.org>

Thanks, applied.

> ---
>  include/uapi/rdma/mlx5-abi.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/rdma/mlx5-abi.h b/include/uapi/rdma/mlx5-
> abi.h
> index da7cd62..0b3d308 100644
> --- a/include/uapi/rdma/mlx5-abi.h
> +++ b/include/uapi/rdma/mlx5-abi.h
> @@ -34,6 +34,7 @@
>  #define MLX5_ABI_USER_H
>  
>  #include 
> +#include   /* For ETH_ALEN. */
>  
>  enum {
>   MLX5_QP_FLAG_SIGNATURE  = 1 << 0,
> @@ -66,7 +67,7 @@ struct mlx5_ib_alloc_ucontext_req {
>  };
>  
>  enum mlx5_lib_caps {
> - MLX5_LIB_CAP_4K_UAR = (u64)1 << 0,
> + MLX5_LIB_CAP_4K_UAR = (__u64)1 << 0,
>  };
>  
>  struct mlx5_ib_alloc_ucontext_req_v2 {
-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: linux-next: build failure after merge of the rdma tree

[Doug Ledford]

On 2/14/2017 7:30 PM, Stephen Rothwell wrote:
> Hi Doug,
> 
> After merging the rdma tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
> 
> drivers/infiniband/hw/bnxt_re/main.c: In function 'bnxt_re_net_ring_free':
> drivers/infiniband/hw/bnxt_re/main.c:231:18: error: 
> 'RING_ALLOC_REQ_RING_TYPE_CMPL' undeclared (first use in this function)
>   req.ring_type = RING_ALLOC_REQ_RING_TYPE_CMPL;
>   ^
> drivers/infiniband/hw/bnxt_re/main.c:231:18: note: each undeclared identifier 
> is reported only once for each function it appears in
> drivers/infiniband/hw/bnxt_re/main.c: In function 'bnxt_re_net_ring_alloc':
> drivers/infiniband/hw/bnxt_re/main.c:271:18: error: 
> 'RING_ALLOC_REQ_RING_TYPE_CMPL' undeclared (first use in this function)
>   req.ring_type = RING_ALLOC_REQ_RING_TYPE_CMPL;
>   ^
> 
> Caused by commit
> 
>   1ac5a4047975 ("RDMA/bnxt_re: Add bnxt_re RoCE driver")
> 
> interacting with commit
> 
>   bac9a7e0f5d6 ("bnxt_en: Update to firmware interface spec 1.7.0.")
> 
> from the net-next tree.
> 
> I added this merge fix patch:
> 
> From: Stephen Rothwell <s...@canb.auug.org.au>
> Date: Wed, 15 Feb 2017 11:23:25 +1100
> Subject: [PATCH] RDMA/bnxt_re: fix for "bnxt_en: Update to firmware interface 
> spec 1.7.0."
> 
> Signed-off-by: Stephen Rothwell <s...@canb.auug.org.au>

Thanks Stephen.  I had been forewarned about this by Selvin and I
instructed him to send me a fixup patch that would resolve the issue.  I
would apply it to my tree before merging with Linus.  He just hadn't
time to send it yet.  Your patch is sufficient as well, so I may just
pull it in.  Thanks again.

> ---
>  drivers/infiniband/hw/bnxt_re/main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/bnxt_re/main.c 
> b/drivers/infiniband/hw/bnxt_re/main.c
> index 6b9f1178050f..bd452a92b386 100644
> --- a/drivers/infiniband/hw/bnxt_re/main.c
> +++ b/drivers/infiniband/hw/bnxt_re/main.c
> @@ -228,7 +228,7 @@ static int bnxt_re_net_ring_free(struct bnxt_re_dev 
> *rdev, u16 fw_ring_id,
>   }
>  
>   bnxt_re_init_hwrm_hdr(rdev, (void *), HWRM_RING_FREE, -1, -1);
> - req.ring_type = RING_ALLOC_REQ_RING_TYPE_CMPL;
> + req.ring_type = RING_ALLOC_REQ_RING_TYPE_L2_CMPL;
>   req.ring_id = cpu_to_le16(fw_ring_id);
>   bnxt_re_fill_fw_msg(_msg, (void *), sizeof(req), (void *),
>   sizeof(resp), DFLT_HWRM_CMD_TIMEOUT);
> @@ -268,7 +268,7 @@ static int bnxt_re_net_ring_alloc(struct bnxt_re_dev 
> *rdev, dma_addr_t *dma_arr,
>   /* Association of ring index with doorbell index and MSIX number */
>   req.logical_id = cpu_to_le16(map_index);
>   req.length = cpu_to_le32(ring_mask + 1);
> - req.ring_type = RING_ALLOC_REQ_RING_TYPE_CMPL;
> + req.ring_type = RING_ALLOC_REQ_RING_TYPE_L2_CMPL;
>   req.int_mode = RING_ALLOC_REQ_INT_MODE_MSIX;
>   bnxt_re_fill_fw_msg(_msg, (void *), sizeof(req), (void *),
>   sizeof(resp), DFLT_HWRM_CMD_TIMEOUT);
> 


-- 
Doug Ledford <dledf...@redhat.com>
GPG Key ID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD



signature.asc
Description: OpenPGP digital signature

Re: [PATCH V5 for-next 00/21] Broadcom RoCE Driver (bnxt_re)

[Doug Ledford]

On Fri, 2017-02-10 at 03:19 -0800, Selvin Xavier wrote:
> This series introduces the RoCE driver for the Broadcom
> NetXtreme-E 10/25/40/50G RoCE HCAs.
> This driver is dependent on the bnxt_en NIC driver and is
> based on the for-4.11 branch in linux-rdma repository.
> bnxt_en changes required for this patch series are already
> available afore mentioned branch.
> 
> These changes are available for your reference in
> the bnxt_re_v5 branch of following repository.
> https://github.com/Broadcom/linux-rdma-nxt/
> 
> Doug,
> This patchset addresses review comments from you and Leon.
> This series also includes some changes required for the latest FW.
> Please consider applying this to linux-rdma tree.

Thanks, v5 applied.  I squashed it down to just two commits: 1 to add
the driver itself, and 1 that only adds the build makefiles.  I've
pushed it to github so far, I'll push it to k.o once I get 0day build
tests back on it.  Feel free to look it over on github if you want to
see how it came out after the squashing and such.

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


signature.asc
Description: This is a digitally signed message part

Re: [PATCH for bnxt_re V4 00/21] Broadcom RoCE Driver (bnxt_re)

[Doug Ledford]

On Wed, 2016-12-21 at 03:41 -0800, Selvin Xavier wrote:
> This series introduces the RoCE driver for the Broadcom
> NetXtreme-E 10/25/40/50G RoCE HCAs.
> This driver is dependent on the bnxt_en NIC driver and is
> based on the bnxt_re branch in linux-rdma repository.
> bnxt_en changes required for this patch series are already
> available afore mentioned branch.
> 
> These changes are available for your reference in
> the bnxt_re_v4 branch of following repository.
> https://github.com/Broadcom/linux-rdma-nxt/
> 
> Doug,
>  This patch set has the file name related changes suggested
>  by you. Please let us know your other feedbacks as and when
>  you are ready.
> 
> Thanks,
> Selvin Xavier

Hi Selvin,

I've reviewed v4.  Aside from the comments I made on patch 03, and some
head scratching I did on patch 16 that I think I got figured out (I was
concerned that budget might get modified by two places without adequate
protection, but I think I see now that that can't happen), and the
comments Leon made in his reviews, things look fairly reasonable.
 Please spin up v5 and post to the list.

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


signature.asc
Description: This is a digitally signed message part

Re: [PATCH for bnxt_re V4 03/21] RDMA/bnxt_re: register with the NIC driver

[Doug Ledford]

On Wed, 2016-12-21 at 03:41 -0800, Selvin Xavier wrote:
> +static void bnxt_re_dev_remove(struct bnxt_re_dev *rdev)
> +{
> +   int i = BNXT_RE_REF_WAIT_COUNT;
> +
> +   /* Wait for rdev refcount to come down */
> +   while ((atomic_read(>ref_count) > 1) && i--)
> +   msleep(100);
> +
> +   if (atomic_read(>ref_count) > 1)
> +   dev_err(rdev_to_dev(rdev),
> +   "Failed waiting for ref count to deplete %d",
> +   atomic_read(>ref_count));
> +
> +   atomic_set(>ref_count, 0);
> +   dev_put(rdev->netdev);
> +   rdev->netdev = NULL;
> +
> +   mutex_lock(_re_dev_lock);
> +   list_del_rcu(>list);
> +   mutex_unlock(_re_dev_lock);
> +
> +   synchronize_rcu();
> +   flush_workqueue(bnxt_re_wq);
> +
> +   ib_dealloc_device(>ibdev);
> +   /* rdev is gone */
> +}

This looks bad.  Either your ref counting is right, and your ref count
should go to 1, or you have an issue that won't be helped by forcibly
removing the device.  In its current form, this looks like an oopser
waiting to happen.

If you know your ref counting is right, then simply wait until it goes
to 1, don't have this bailout logic.  If you truly need to bailout for
some reason, then you need to not free things.  Better to shutdown then
leak and live than release and have a use after free data corrupter.


-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


signature.asc
Description: This is a digitally signed message part

Re: [PATCH for-next V2 00/11] Mellanox mlx5 core and ODP updates 2017-01-01

[Doug Ledford]

On Tue, 2017-01-24 at 21:58 +0200, Leon Romanovsky wrote:
> On Tue, Jan 24, 2017 at 02:39:59PM -0500, Doug Ledford wrote:
> > 
> > On Tue, 2017-01-03 at 09:23 +0200, Leon Romanovsky wrote:
> > > 
> > > On Tue, Jan 03, 2017 at 01:30:16AM +0200, Saeed Mahameed wrote:
> > > > 
> > > > 
> > > > On Mon, Jan 2, 2017 at 10:53 PM, David Miller <davem@davemloft.
> > > > net>
> > > > wrote:
> > > > > 
> > > > > 
> > > > > From: Saeed Mahameed <sae...@mellanox.com>
> > > > > Date: Mon,  2 Jan 2017 11:37:37 +0200
> > > > > 
> > > > > > 
> > > > > > 
> > > > > > The following eleven patches mainly come from Artemy
> > > > > > Kovalyov
> > > > > > who expanded mlx5 on-demand-paging (ODP) support. In
> > > > > > addition
> > > > > > there are three cleanup patches which don't change any
> > > > > > functionality,
> > > > > > but are needed to align codebase prior accepting other
> > > > > > patches.
> > > > > 
> > > > > Series applied to net-next, thanks.
> > > > 
> > > > Whoops,
> > > > 
> > > > This series was meant as a pull request, you can blame it on me
> > > > I
> > > > kinda messed up the V2 title.
> > > > Doug will have to pull same patches later, will this produce a
> > > > conflict on merge window ?
> > > 
> > > Yes, but it can be easily avoided.
> > > 
> > > Doug,
> > > 
> > > We have another pull request to send and we will base its code on
> > > the
> > > Dave's tree instead of Linus's rc tag. In such way, you will have
> > > the
> > > same commits as Dave and won't have merge failures.
> > > 
> > > Please don't apply manually this specific patchset.
> > > 
> > > Sorry for the inconvenience.
> > 
> > Hi Leon and Saeed,
> > 
> > As I understand it, Dave took this series, but I have not.  You
> > were
> > going to base another pull request on top of this so I could get
> > both.
> 
> We did it, ODP was our first pull request and 4K UAR was second one.
> http://marc.info/?l=linux-rdma=148398855416090=2
> Dave pulled this tag:
> https://git.kernel.org/cgit/linux/kernel/git/mellanox/linux.git/tag/?
> h=mlx5-4kuar-for-4.11
> 
> Since 4K UAR tag was based on ODP, by pulling mlx5-4kuar-for-4.11 tag
> you
> will get ODP too.

OK, I've verified that I have it all, thanks.

> > 
> >  However, I believe that was the attempt to reorg the driver pull
> > request, which David NAKed.  
> 
> It was our third pull request
> http://marc.info/?l=linux-rdma=148424983101796=2
> https://git.kernel.org/cgit/linux/kernel/git/mellanox/linux.git/tag/?
> h=mlx5-dir-layout-reorg
> 
> > 
> > That means I have not picked this up.  Are
> > all of the other for-next patches you posted against the RDMA list
> > going to go in cleanly without this series, or should I expect
> > conflicts between Dave's tree and my own?
> 
> All patches sent by me are based on mlx5-4kuar-for-4.11 tag which
> exists in Dave's tree and everything will apply cleanly on your
> k.o/for-4.11 branch and won't have merge conflicts between your tree
> and
> Dave's net-next tree.
> 
> For you convenience, my submission queue can be found at:
> https://git.kernel.org/cgit/linux/kernel/git/leon/linux-rdma.git/log/
> ?h=rdma-next
> 
> Thanks
> 
> > 
> > 
> > --
> > Doug Ledford <dledf...@redhat.com>
> >     GPG KeyID: B826A3330E572FDD
> >    
> > Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57
> > 2FDD
> 
> 
-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


signature.asc
Description: This is a digitally signed message part

Re: [PATCH for-next V2 00/11] Mellanox mlx5 core and ODP updates 2017-01-01

[Doug Ledford]

On Tue, 2017-01-03 at 09:23 +0200, Leon Romanovsky wrote:
> On Tue, Jan 03, 2017 at 01:30:16AM +0200, Saeed Mahameed wrote:
> > 
> > On Mon, Jan 2, 2017 at 10:53 PM, David Miller <da...@davemloft.net>
> > wrote:
> > > 
> > > From: Saeed Mahameed <sae...@mellanox.com>
> > > Date: Mon,  2 Jan 2017 11:37:37 +0200
> > > 
> > > > 
> > > > The following eleven patches mainly come from Artemy Kovalyov
> > > > who expanded mlx5 on-demand-paging (ODP) support. In addition
> > > > there are three cleanup patches which don't change any
> > > > functionality,
> > > > but are needed to align codebase prior accepting other patches.
> > > 
> > > Series applied to net-next, thanks.
> > 
> > Whoops,
> > 
> > This series was meant as a pull request, you can blame it on me I
> > kinda messed up the V2 title.
> > Doug will have to pull same patches later, will this produce a
> > conflict on merge window ?
> 
> Yes, but it can be easily avoided.
> 
> Doug,
> 
> We have another pull request to send and we will base its code on the
> Dave's tree instead of Linus's rc tag. In such way, you will have the
> same commits as Dave and won't have merge failures.
> 
> Please don't apply manually this specific patchset.
> 
> Sorry for the inconvenience.

Hi Leon and Saeed,

As I understand it, Dave took this series, but I have not.  You were
going to base another pull request on top of this so I could get both.
 However, I believe that was the attempt to reorg the driver pull
request, which David NAKed.  That means I have not picked this up.  Are
all of the other for-next patches you posted against the RDMA list
going to go in cleanly without this series, or should I expect
conflicts between Dave's tree and my own?

-- 
Doug Ledford <dledf...@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


signature.asc
Description: This is a digitally signed message part