Re: [PATCH v2 2/2] RDMA/core: Initialize port_num in qp_attr
From: Ismail, MustafaSent: Wednesday, July 19, 2017 5:38 PM > > > > Fixes: 5ecce4c9b17b("Check port number supplied by user verbs cmds") > > > > Cc: # v2.6.14+ > > > > Reviewed-by: Steve Wise > > > > Signed-off-by: Mustafa Ismail > > > > > Why is the second patch required if you only validate the port_num if the > > IB_QP_PORT mask is on? > > Given the first patch [PATCH v2 1/2] RDMA/uverbs: Fix the check for port > > number, this one seems redundant. > Strictly speaking it is not required, but we felt it safer to always return a > valid port number > as is done in the IB case. It's not always initialized in the IB case either. More than that if at this point you'll initialize it for ib as well you'll get a failure on ib_modify_qp_is_ok, since when transitioning to RTR / RTS providing IB_QP_PORT is not a valid option. We actually hit this issue when running rping over RoCE. (prior to your fix i mean ) I agree that in general there's no real harm, but it seems a bit out of context, and if we make the change common for ib/iwarp we'll have to modify ib_modify_qp_is_ok which is written close to the spec. thanks, Michal
Re: [PATCH v2 2/2] RDMA/core: Initialize port_num in qp_attr
From: Ismail, Mustafa Sent: Wednesday, July 19, 2017 5:38 PM > > > > Fixes: 5ecce4c9b17b("Check port number supplied by user verbs cmds") > > > > Cc: # v2.6.14+ > > > > Reviewed-by: Steve Wise > > > > Signed-off-by: Mustafa Ismail > > > > > Why is the second patch required if you only validate the port_num if the > > IB_QP_PORT mask is on? > > Given the first patch [PATCH v2 1/2] RDMA/uverbs: Fix the check for port > > number, this one seems redundant. > Strictly speaking it is not required, but we felt it safer to always return a > valid port number > as is done in the IB case. It's not always initialized in the IB case either. More than that if at this point you'll initialize it for ib as well you'll get a failure on ib_modify_qp_is_ok, since when transitioning to RTR / RTS providing IB_QP_PORT is not a valid option. We actually hit this issue when running rping over RoCE. (prior to your fix i mean ) I agree that in general there's no real harm, but it seems a bit out of context, and if we make the change common for ib/iwarp we'll have to modify ib_modify_qp_is_ok which is written close to the spec. thanks, Michal
RE: [PATCH v2 2/2] RDMA/core: Initialize port_num in qp_attr
> -Original Message- > From: Kalderon, Michal [mailto:michal.kalde...@cavium.com] > Sent: Wednesday, July 19, 2017 3:10 AM > To: Marciniszyn, Mike <mike.marcinis...@intel.com>; Ismail, Mustafa > <mustafa.ism...@intel.com>; linux-r...@vger.kernel.org; > dledf...@redhat.com > Cc: sw...@opengridcomputing.com; linux-kernel@vger.kernel.org; > sta...@vger.kernel.org; e1000-r...@lists.sourceforge.net; Saleem, Shiraz > <shiraz.sal...@intel.com>; Amrani, Ram <ram.amr...@cavium.com> > Subject: RE: [PATCH v2 2/2] RDMA/core: Initialize port_num in qp_attr > > > From: linux-rdma-ow...@vger.kernel.org [mailto:linux-rdma- > > ow...@vger.kernel.org] On Behalf Of Marciniszyn, Mike > > > Initialize the port_num for iWARP in rdma_init_qp_attr. > > > > > > Fixes: 5ecce4c9b17b("Check port number supplied by user verbs cmds") > > > Cc: <sta...@vger.kernel.org> # v2.6.14+ > > > Reviewed-by: Steve Wise <sw...@opengridcomputing.com> > > > Signed-off-by: Mustafa Ismail <mustafa.ism...@intel.com> > > > Why is the second patch required if you only validate the port_num if the > IB_QP_PORT mask is on? > Given the first patch [PATCH v2 1/2] RDMA/uverbs: Fix the check for port > number, this one seems redundant. Strictly speaking it is not required, but we felt it safer to always return a valid port number as is done in the IB case. Regards, Mustafa
RE: [PATCH v2 2/2] RDMA/core: Initialize port_num in qp_attr
> -Original Message- > From: Kalderon, Michal [mailto:michal.kalde...@cavium.com] > Sent: Wednesday, July 19, 2017 3:10 AM > To: Marciniszyn, Mike ; Ismail, Mustafa > ; linux-r...@vger.kernel.org; > dledf...@redhat.com > Cc: sw...@opengridcomputing.com; linux-kernel@vger.kernel.org; > sta...@vger.kernel.org; e1000-r...@lists.sourceforge.net; Saleem, Shiraz > ; Amrani, Ram > Subject: RE: [PATCH v2 2/2] RDMA/core: Initialize port_num in qp_attr > > > From: linux-rdma-ow...@vger.kernel.org [mailto:linux-rdma- > > ow...@vger.kernel.org] On Behalf Of Marciniszyn, Mike > > > Initialize the port_num for iWARP in rdma_init_qp_attr. > > > > > > Fixes: 5ecce4c9b17b("Check port number supplied by user verbs cmds") > > > Cc: # v2.6.14+ > > > Reviewed-by: Steve Wise > > > Signed-off-by: Mustafa Ismail > > > Why is the second patch required if you only validate the port_num if the > IB_QP_PORT mask is on? > Given the first patch [PATCH v2 1/2] RDMA/uverbs: Fix the check for port > number, this one seems redundant. Strictly speaking it is not required, but we felt it safer to always return a valid port number as is done in the IB case. Regards, Mustafa
RE: [PATCH v2 2/2] RDMA/core: Initialize port_num in qp_attr
> From: linux-rdma-ow...@vger.kernel.org [mailto:linux-rdma- > ow...@vger.kernel.org] On Behalf Of Marciniszyn, Mike > > Initialize the port_num for iWARP in rdma_init_qp_attr. > > > > Fixes: 5ecce4c9b17b("Check port number supplied by user verbs cmds") > > Cc:# v2.6.14+ > > Reviewed-by: Steve Wise > > Signed-off-by: Mustafa Ismail > Why is the second patch required if you only validate the port_num if the IB_QP_PORT mask is on? Given the first patch [PATCH v2 1/2] RDMA/uverbs: Fix the check for port number, this one seems redundant. Thanks, Michal
RE: [PATCH v2 2/2] RDMA/core: Initialize port_num in qp_attr
> From: linux-rdma-ow...@vger.kernel.org [mailto:linux-rdma- > ow...@vger.kernel.org] On Behalf Of Marciniszyn, Mike > > Initialize the port_num for iWARP in rdma_init_qp_attr. > > > > Fixes: 5ecce4c9b17b("Check port number supplied by user verbs cmds") > > Cc: # v2.6.14+ > > Reviewed-by: Steve Wise > > Signed-off-by: Mustafa Ismail > Why is the second patch required if you only validate the port_num if the IB_QP_PORT mask is on? Given the first patch [PATCH v2 1/2] RDMA/uverbs: Fix the check for port number, this one seems redundant. Thanks, Michal
RE: [PATCH v2 2/2] RDMA/core: Initialize port_num in qp_attr
> Subject: [PATCH v2 2/2] RDMA/core: Initialize port_num in qp_attr > > Initialize the port_num for iWARP in rdma_init_qp_attr. > > Fixes: 5ecce4c9b17b("Check port number supplied by user verbs cmds") > Cc:# v2.6.14+ > Reviewed-by: Steve Wise > Signed-off-by: Mustafa Ismail Tested-by: Mike Marciniszyn
RE: [PATCH v2 2/2] RDMA/core: Initialize port_num in qp_attr
> Subject: [PATCH v2 2/2] RDMA/core: Initialize port_num in qp_attr > > Initialize the port_num for iWARP in rdma_init_qp_attr. > > Fixes: 5ecce4c9b17b("Check port number supplied by user verbs cmds") > Cc: # v2.6.14+ > Reviewed-by: Steve Wise > Signed-off-by: Mustafa Ismail Tested-by: Mike Marciniszyn