| Message ID | 20240207032910.3959426-1-huangjunxian6@hisilicon.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | RDMA/hns: Support configuring congestion control algorithm with QP granularity | expand |
On Wed, Feb 07, 2024 at 11:29:08AM +0800, Junxian Huang wrote: > Patch #1 reverts a previous bugfix that was intended to add restriction > to congestion control algorithm for UD but mistakenly introduced other > problem. First patch shouldn't be revert but a fix to "add a restriction that only DCQCN is supported for UD." and second patch should be a new feature. Thanks > > Patch #2 adds support for configuring congestion control algorithm with > QP granularity. The algorithm restriction for UD is added in this patch. > > Junxian Huang (1): > RDMA/hns: Support configuring congestion control algorithm with QP > granularity > > Luoyouming (1): > Revert "RDMA/hns: The UD mode can only be configured with DCQCN" > > drivers/infiniband/hw/hns/hns_roce_device.h | 26 +++++--- > drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 18 ++---- > drivers/infiniband/hw/hns/hns_roce_hw_v2.h | 3 +- > drivers/infiniband/hw/hns/hns_roce_main.c | 3 + > drivers/infiniband/hw/hns/hns_roce_qp.c | 71 +++++++++++++++++++++ > include/uapi/rdma/hns-abi.h | 17 +++++ > 6 files changed, 118 insertions(+), 20 deletions(-) > > -- > 2.30.0 >
On 2024/2/7 16:33, Leon Romanovsky wrote: > On Wed, Feb 07, 2024 at 11:29:08AM +0800, Junxian Huang wrote: >> Patch #1 reverts a previous bugfix that was intended to add restriction >> to congestion control algorithm for UD but mistakenly introduced other >> problem. > > First patch shouldn't be revert but a fix to "add a restriction that only DCQCN > is supported for UD." and second patch should be a new feature. > > Thanks > OK, but I have two questions here: 1. Of course we can not only revert but also completely fix the bug in patch #1. But since we are adding a new feature that can also fix this bug in patch #2, the fix in patch #1 will be immediately removed in patch #2. Is this acceptable? 2. Should I still put these two patches into one patchset in the next version, or seperate them into two individual patchset? Thanks, Junxian >> >> Patch #2 adds support for configuring congestion control algorithm with >> QP granularity. The algorithm restriction for UD is added in this patch. >> >> Junxian Huang (1): >> RDMA/hns: Support configuring congestion control algorithm with QP >> granularity >> >> Luoyouming (1): >> Revert "RDMA/hns: The UD mode can only be configured with DCQCN" >> >> drivers/infiniband/hw/hns/hns_roce_device.h | 26 +++++--- >> drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 18 ++---- >> drivers/infiniband/hw/hns/hns_roce_hw_v2.h | 3 +- >> drivers/infiniband/hw/hns/hns_roce_main.c | 3 + >> drivers/infiniband/hw/hns/hns_roce_qp.c | 71 +++++++++++++++++++++ >> include/uapi/rdma/hns-abi.h | 17 +++++ >> 6 files changed, 118 insertions(+), 20 deletions(-) >> >> -- >> 2.30.0 >> >
On Wed, Feb 07, 2024 at 04:59:56PM +0800, Junxian Huang wrote: > > > On 2024/2/7 16:33, Leon Romanovsky wrote: > > On Wed, Feb 07, 2024 at 11:29:08AM +0800, Junxian Huang wrote: > >> Patch #1 reverts a previous bugfix that was intended to add restriction > >> to congestion control algorithm for UD but mistakenly introduced other > >> problem. > > > > First patch shouldn't be revert but a fix to "add a restriction that only DCQCN > > is supported for UD." and second patch should be a new feature. > > > > Thanks > > > > OK, but I have two questions here: > > 1. Of course we can not only revert but also completely fix the bug in patch #1. > But since we are adding a new feature that can also fix this bug in patch #2, > the fix in patch #1 will be immediately removed in patch #2. Is this acceptable? I would expect that second patch will extend the first one and not remove. > > 2. Should I still put these two patches into one patchset in the next version, or > seperate them into two individual patchset? Better to separate, as we will take fix faster than new feature. Thanks > > Thanks, > Junxian > > >> > >> Patch #2 adds support for configuring congestion control algorithm with > >> QP granularity. The algorithm restriction for UD is added in this patch. > >> > >> Junxian Huang (1): > >> RDMA/hns: Support configuring congestion control algorithm with QP > >> granularity > >> > >> Luoyouming (1): > >> Revert "RDMA/hns: The UD mode can only be configured with DCQCN" > >> > >> drivers/infiniband/hw/hns/hns_roce_device.h | 26 +++++--- > >> drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 18 ++---- > >> drivers/infiniband/hw/hns/hns_roce_hw_v2.h | 3 +- > >> drivers/infiniband/hw/hns/hns_roce_main.c | 3 + > >> drivers/infiniband/hw/hns/hns_roce_qp.c | 71 +++++++++++++++++++++ > >> include/uapi/rdma/hns-abi.h | 17 +++++ > >> 6 files changed, 118 insertions(+), 20 deletions(-) > >> > >> -- > >> 2.30.0 > >> > >
On 2024/2/8 16:10, Leon Romanovsky wrote: > On Wed, Feb 07, 2024 at 04:59:56PM +0800, Junxian Huang wrote: >> >> >> On 2024/2/7 16:33, Leon Romanovsky wrote: >>> On Wed, Feb 07, 2024 at 11:29:08AM +0800, Junxian Huang wrote: >>>> Patch #1 reverts a previous bugfix that was intended to add restriction >>>> to congestion control algorithm for UD but mistakenly introduced other >>>> problem. >>> >>> First patch shouldn't be revert but a fix to "add a restriction that only DCQCN >>> is supported for UD." and second patch should be a new feature. >>> >>> Thanks >>> >> >> OK, but I have two questions here: >> >> 1. Of course we can not only revert but also completely fix the bug in patch #1. >> But since we are adding a new feature that can also fix this bug in patch #2, >> the fix in patch #1 will be immediately removed in patch #2. Is this acceptable? > > I would expect that second patch will extend the first one and not remove. > >> >> 2. Should I still put these two patches into one patchset in the next version, or >> seperate them into two individual patchset? > > Better to separate, as we will take fix faster than new feature. > > Thanks > Sorry for the late reply. Will separate and re-send the fix first. Please ignore the v2 of this series. Thanks, Junxian >> >> Thanks, >> Junxian >> >>>> >>>> Patch #2 adds support for configuring congestion control algorithm with >>>> QP granularity. The algorithm restriction for UD is added in this patch. >>>> >>>> Junxian Huang (1): >>>> RDMA/hns: Support configuring congestion control algorithm with QP >>>> granularity >>>> >>>> Luoyouming (1): >>>> Revert "RDMA/hns: The UD mode can only be configured with DCQCN" >>>> >>>> drivers/infiniband/hw/hns/hns_roce_device.h | 26 +++++--- >>>> drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 18 ++---- >>>> drivers/infiniband/hw/hns/hns_roce_hw_v2.h | 3 +- >>>> drivers/infiniband/hw/hns/hns_roce_main.c | 3 + >>>> drivers/infiniband/hw/hns/hns_roce_qp.c | 71 +++++++++++++++++++++ >>>> include/uapi/rdma/hns-abi.h | 17 +++++ >>>> 6 files changed, 118 insertions(+), 20 deletions(-) >>>> >>>> -- >>>> 2.30.0 >>>> >>>
Patch #1 reverts a previous bugfix that was intended to add restriction to congestion control algorithm for UD but mistakenly introduced other problem. Patch #2 adds support for configuring congestion control algorithm with QP granularity. The algorithm restriction for UD is added in this patch. Junxian Huang (1): RDMA/hns: Support configuring congestion control algorithm with QP granularity Luoyouming (1): Revert "RDMA/hns: The UD mode can only be configured with DCQCN" drivers/infiniband/hw/hns/hns_roce_device.h | 26 +++++--- drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 18 ++---- drivers/infiniband/hw/hns/hns_roce_hw_v2.h | 3 +- drivers/infiniband/hw/hns/hns_roce_main.c | 3 + drivers/infiniband/hw/hns/hns_roce_qp.c | 71 +++++++++++++++++++++ include/uapi/rdma/hns-abi.h | 17 +++++ 6 files changed, 118 insertions(+), 20 deletions(-) -- 2.30.0