| Message ID | 20250107024553.2926983-1-kalesh-anakkur.purayil@broadcom.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | RDMA/bnxt_re: Support for FW async event handling | expand |
On Tue, Jan 07, 2025 at 08:15:48AM +0530, Kalesh AP wrote: > This patch series adds support for FW async event handling > in the bnxt_re driver. > > V1->V2: > 1. Rebased on top of the latest "for-next" tree. > 2. Split Patch#1 into 2 - one for Ethernet driver changes and > another one for RDMA driver changes. > 3. Addressed Leon's comments on Patch#1 and Patch #3. > V1: https://lore.kernel.org/linux-rdma/1725363051-19268-1-git-send-email-selvin.xavier@broadcom.com/T/#t > > Patch #1: > 1. Removed BNXT_EN_FLAG_ULP_STOPPED state check from bnxt_ulp_async_events(). > The ulp_ops are protected by RCU. This means that during bnxt_unregister_dev(), > Ethernet driver set the ulp_ops pointer to NULL and do RCU sync before return > to the RDMA driver. > So ulp_ops and the pointers in ulp_ops are always valid or NULL when the > Ethernet driver references ulp_ops. ULP_STOPPED is a state and should be > unrelated to async events. It should not affect whether async events should > or should not be passed to the RDMA driver. > 2. Changed Author of Ethernet driver changes to Michael Chan. > 3. Removed unnecessary export of function bnxt_ulp_async_events. > > Patch #3: > 1. Removed unnecessary flush_workqueue() before destroy_workqueue() > 2. Removed unnecessary NULL assignment after free. > 3. Changed to use "ibdev_xxx" and reduce level of couple of logs to debug. > > Please review and apply. > > Regards, > Kalesh > > > Kalesh AP (3): > RDMA/bnxt_re: Add Async event handling support > RDMA/bnxt_re: Query firmware defaults of CC params during probe > RDMA/bnxt_re: Add support to handle DCB_CONFIG_CHANGE event > > Michael Chan (1): > bnxt_en: Add ULP call to notify async events > > drivers/infiniband/hw/bnxt_re/bnxt_re.h | 3 + > drivers/infiniband/hw/bnxt_re/main.c | 156 ++++++++++++++++++ > drivers/infiniband/hw/bnxt_re/qplib_fp.h | 1 + > drivers/infiniband/hw/bnxt_re/qplib_sp.c | 113 +++++++++++++ > drivers/infiniband/hw/bnxt_re/qplib_sp.h | 3 + > drivers/net/ethernet/broadcom/bnxt/bnxt.c | 1 + > drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 28 ++++ > drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h | 2 + > 8 files changed, 307 insertions(+) Applied with the following fix diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c index 1dc305689d7bb..54dee0f5dd3f5 100644 --- a/drivers/infiniband/hw/bnxt_re/main.c +++ b/drivers/infiniband/hw/bnxt_re/main.c @@ -1802,30 +1802,22 @@ static int bnxt_re_setup_qos(struct bnxt_re_dev *rdev) static void bnxt_re_net_unregister_async_event(struct bnxt_re_dev *rdev) { - int rc; - if (rdev->is_virtfn) return; memset(&rdev->event_bitmap, 0, sizeof(rdev->event_bitmap)); - rc = bnxt_register_async_events(rdev->en_dev, &rdev->event_bitmap, - ASYNC_EVENT_CMPL_EVENT_ID_DCB_CONFIG_CHANGE); - if (rc) - ibdev_err(&rdev->ibdev, "Failed to unregister async event"); + bnxt_register_async_events(rdev->en_dev, &rdev->event_bitmap, + ASYNC_EVENT_CMPL_EVENT_ID_DCB_CONFIG_CHANGE); } static void bnxt_re_net_register_async_event(struct bnxt_re_dev *rdev) { - int rc; - if (rdev->is_virtfn) return; rdev->event_bitmap |= (1 << ASYNC_EVENT_CMPL_EVENT_ID_DCB_CONFIG_CHANGE); - rc = bnxt_register_async_events(rdev->en_dev, &rdev->event_bitmap, - ASYNC_EVENT_CMPL_EVENT_ID_DCB_CONFIG_CHANGE); - if (rc) - ibdev_err(&rdev->ibdev, "Failed to unregister async event"); + bnxt_register_async_events(rdev->en_dev, &rdev->event_bitmap, + ASYNC_EVENT_CMPL_EVENT_ID_DCB_CONFIG_CHANGE); } static void bnxt_re_query_hwrm_intf_version(struct bnxt_re_dev *rdev) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c index 59c280634bc5f..3e17db0a453e0 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c @@ -373,9 +373,8 @@ void bnxt_ulp_async_events(struct bnxt *bp, struct hwrm_async_event_cmpl *cmpl) rcu_read_unlock(); } -int bnxt_register_async_events(struct bnxt_en_dev *edev, - unsigned long *events_bmap, - u16 max_id) +void bnxt_register_async_events(struct bnxt_en_dev *edev, + unsigned long *events_bmap, u16 max_id) { struct net_device *dev = edev->net; struct bnxt *bp = netdev_priv(dev); @@ -387,7 +386,6 @@ int bnxt_register_async_events(struct bnxt_en_dev *edev, smp_wmb(); ulp->max_async_event_id = max_id; bnxt_hwrm_func_drv_rgtr(bp, events_bmap, max_id + 1, true); - return 0; } EXPORT_SYMBOL(bnxt_register_async_events); diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h index a21294cf197b8..ee6a5b8562c3e 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h @@ -126,6 +126,6 @@ int bnxt_register_dev(struct bnxt_en_dev *edev, struct bnxt_ulp_ops *ulp_ops, void *handle); void bnxt_unregister_dev(struct bnxt_en_dev *edev); int bnxt_send_msg(struct bnxt_en_dev *edev, struct bnxt_fw_msg *fw_msg); -int bnxt_register_async_events(struct bnxt_en_dev *edev, - unsigned long *events_bmap, u16 max_id); +void bnxt_register_async_events(struct bnxt_en_dev *edev, + unsigned long *events_bmap, u16 max_id); #endif > > -- > 2.43.5 >
On Tue, 07 Jan 2025 08:15:48 +0530, Kalesh AP wrote: > This patch series adds support for FW async event handling > in the bnxt_re driver. > > V1->V2: > 1. Rebased on top of the latest "for-next" tree. > 2. Split Patch#1 into 2 - one for Ethernet driver changes and > another one for RDMA driver changes. > 3. Addressed Leon's comments on Patch#1 and Patch #3. > V1: https://lore.kernel.org/linux-rdma/1725363051-19268-1-git-send-email-selvin.xavier@broadcom.com/T/#t > > [...] Applied, thanks! [1/4] bnxt_en: Add ULP call to notify async events https://git.kernel.org/rdma/rdma/c/184fe6f2382bab [2/4] RDMA/bnxt_re: Add Async event handling support https://git.kernel.org/rdma/rdma/c/7fea327840683e [3/4] RDMA/bnxt_re: Query firmware defaults of CC params during probe https://git.kernel.org/rdma/rdma/c/c0ad30eddc2858 [4/4] RDMA/bnxt_re: Add support to handle DCB_CONFIG_CHANGE event https://git.kernel.org/rdma/rdma/c/51dc5312dcd929 Best regards,
On Tue, Jan 14, 2025 at 2:35 PM Leon Romanovsky <leon@kernel.org> wrote: > > On Tue, Jan 07, 2025 at 08:15:48AM +0530, Kalesh AP wrote: > > This patch series adds support for FW async event handling > > in the bnxt_re driver. > > > > V1->V2: > > 1. Rebased on top of the latest "for-next" tree. > > 2. Split Patch#1 into 2 - one for Ethernet driver changes and > > another one for RDMA driver changes. > > 3. Addressed Leon's comments on Patch#1 and Patch #3. > > V1: https://lore.kernel.org/linux-rdma/1725363051-19268-1-git-send-email-selvin.xavier@broadcom.com/T/#t > > > > Patch #1: > > 1. Removed BNXT_EN_FLAG_ULP_STOPPED state check from bnxt_ulp_async_events(). > > The ulp_ops are protected by RCU. This means that during bnxt_unregister_dev(), > > Ethernet driver set the ulp_ops pointer to NULL and do RCU sync before return > > to the RDMA driver. > > So ulp_ops and the pointers in ulp_ops are always valid or NULL when the > > Ethernet driver references ulp_ops. ULP_STOPPED is a state and should be > > unrelated to async events. It should not affect whether async events should > > or should not be passed to the RDMA driver. > > 2. Changed Author of Ethernet driver changes to Michael Chan. > > 3. Removed unnecessary export of function bnxt_ulp_async_events. > > > > Patch #3: > > 1. Removed unnecessary flush_workqueue() before destroy_workqueue() > > 2. Removed unnecessary NULL assignment after free. > > 3. Changed to use "ibdev_xxx" and reduce level of couple of logs to debug. > > > > Please review and apply. > > > > Regards, > > Kalesh > > > > > > Kalesh AP (3): > > RDMA/bnxt_re: Add Async event handling support > > RDMA/bnxt_re: Query firmware defaults of CC params during probe > > RDMA/bnxt_re: Add support to handle DCB_CONFIG_CHANGE event > > > > Michael Chan (1): > > bnxt_en: Add ULP call to notify async events > > > > drivers/infiniband/hw/bnxt_re/bnxt_re.h | 3 + > > drivers/infiniband/hw/bnxt_re/main.c | 156 ++++++++++++++++++ > > drivers/infiniband/hw/bnxt_re/qplib_fp.h | 1 + > > drivers/infiniband/hw/bnxt_re/qplib_sp.c | 113 +++++++++++++ > > drivers/infiniband/hw/bnxt_re/qplib_sp.h | 3 + > > drivers/net/ethernet/broadcom/bnxt/bnxt.c | 1 + > > drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 28 ++++ > > drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h | 2 + > > 8 files changed, 307 insertions(+) > > Applied with the following fix > > diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c > index 1dc305689d7bb..54dee0f5dd3f5 100644 > --- a/drivers/infiniband/hw/bnxt_re/main.c > +++ b/drivers/infiniband/hw/bnxt_re/main.c > @@ -1802,30 +1802,22 @@ static int bnxt_re_setup_qos(struct bnxt_re_dev *rdev) > > static void bnxt_re_net_unregister_async_event(struct bnxt_re_dev *rdev) > { > - int rc; > - > if (rdev->is_virtfn) > return; > > memset(&rdev->event_bitmap, 0, sizeof(rdev->event_bitmap)); > - rc = bnxt_register_async_events(rdev->en_dev, &rdev->event_bitmap, > - ASYNC_EVENT_CMPL_EVENT_ID_DCB_CONFIG_CHANGE); > - if (rc) > - ibdev_err(&rdev->ibdev, "Failed to unregister async event"); > + bnxt_register_async_events(rdev->en_dev, &rdev->event_bitmap, > + ASYNC_EVENT_CMPL_EVENT_ID_DCB_CONFIG_CHANGE); > } > > static void bnxt_re_net_register_async_event(struct bnxt_re_dev *rdev) > { > - int rc; > - > if (rdev->is_virtfn) > return; > > rdev->event_bitmap |= (1 << ASYNC_EVENT_CMPL_EVENT_ID_DCB_CONFIG_CHANGE); > - rc = bnxt_register_async_events(rdev->en_dev, &rdev->event_bitmap, > - ASYNC_EVENT_CMPL_EVENT_ID_DCB_CONFIG_CHANGE); > - if (rc) > - ibdev_err(&rdev->ibdev, "Failed to unregister async event"); > + bnxt_register_async_events(rdev->en_dev, &rdev->event_bitmap, > + ASYNC_EVENT_CMPL_EVENT_ID_DCB_CONFIG_CHANGE); > } > > static void bnxt_re_query_hwrm_intf_version(struct bnxt_re_dev *rdev) > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c > index 59c280634bc5f..3e17db0a453e0 100644 > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c > @@ -373,9 +373,8 @@ void bnxt_ulp_async_events(struct bnxt *bp, struct hwrm_async_event_cmpl *cmpl) > rcu_read_unlock(); > } > > -int bnxt_register_async_events(struct bnxt_en_dev *edev, > - unsigned long *events_bmap, > - u16 max_id) > +void bnxt_register_async_events(struct bnxt_en_dev *edev, > + unsigned long *events_bmap, u16 max_id) > { > struct net_device *dev = edev->net; > struct bnxt *bp = netdev_priv(dev); > @@ -387,7 +386,6 @@ int bnxt_register_async_events(struct bnxt_en_dev *edev, > smp_wmb(); > ulp->max_async_event_id = max_id; > bnxt_hwrm_func_drv_rgtr(bp, events_bmap, max_id + 1, true); > - return 0; > } > EXPORT_SYMBOL(bnxt_register_async_events); > > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h > index a21294cf197b8..ee6a5b8562c3e 100644 > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h > @@ -126,6 +126,6 @@ int bnxt_register_dev(struct bnxt_en_dev *edev, struct bnxt_ulp_ops *ulp_ops, > void *handle); > void bnxt_unregister_dev(struct bnxt_en_dev *edev); > int bnxt_send_msg(struct bnxt_en_dev *edev, struct bnxt_fw_msg *fw_msg); > -int bnxt_register_async_events(struct bnxt_en_dev *edev, > - unsigned long *events_bmap, u16 max_id); > +void bnxt_register_async_events(struct bnxt_en_dev *edev, > + unsigned long *events_bmap, u16 max_id); > #endif Makes sense and LGTM, thank you Leon! > > > > > -- > > 2.43.5 > > -- Regards, Kalesh AP