* [PATCH] raw/ntb: add check for DB intr handler registering @ 2022-02-10 6:28 Junfeng Guo 2022-02-10 7:04 ` Wu, Jingjing 0 siblings, 1 reply; 8+ messages in thread From: Junfeng Guo @ 2022-02-10 6:28 UTC (permalink / raw) To: jingjing.wu; +Cc: dev, stable, junfeng.guo The callback registering of doorbell interrupt handler should be finished before enabling the interrupt event fd. Thus add the return value check for this callback registering. Fixes: 62012a76811e ("raw/ntb: add handshake process") Cc: stable@dpdk.org Signed-off-by: Junfeng Guo <junfeng.guo@intel.com> --- drivers/raw/ntb/ntb.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c index cc611dfbb9..0801e6d1ae 100644 --- a/drivers/raw/ntb/ntb.c +++ b/drivers/raw/ntb/ntb.c @@ -1403,8 +1403,12 @@ ntb_init_hw(struct rte_rawdev *dev, struct rte_pci_device *pci_dev) intr_handle = pci_dev->intr_handle; /* Register callback func to eal lib */ - rte_intr_callback_register(intr_handle, - ntb_dev_intr_handler, dev); + ret = rte_intr_callback_register(intr_handle, + ntb_dev_intr_handler, dev); + if (ret) { + NTB_LOG(ERR, "Unable to register doorbell intr handler."); + return ret; + } ret = rte_intr_efd_enable(intr_handle, hw->db_cnt); if (ret) -- 2.25.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH] raw/ntb: add check for DB intr handler registering 2022-02-10 6:28 [PATCH] raw/ntb: add check for DB intr handler registering Junfeng Guo @ 2022-02-10 7:04 ` Wu, Jingjing 2022-02-10 7:18 ` Guo, Junfeng 0 siblings, 1 reply; 8+ messages in thread From: Wu, Jingjing @ 2022-02-10 7:04 UTC (permalink / raw) To: Guo, Junfeng; +Cc: dev, stable > -----Original Message----- > From: Guo, Junfeng <junfeng.guo@intel.com> > Sent: Thursday, February 10, 2022 2:29 PM > To: Wu, Jingjing <jingjing.wu@intel.com> > Cc: dev@dpdk.org; stable@dpdk.org; Guo, Junfeng <junfeng.guo@intel.com> > Subject: [PATCH] raw/ntb: add check for DB intr handler registering > > The callback registering of doorbell interrupt handler should be > finished before enabling the interrupt event fd. Thus add the return > value check for this callback registering. > > Fixes: 62012a76811e ("raw/ntb: add handshake process") > Cc: stable@dpdk.org > > Signed-off-by: Junfeng Guo <junfeng.guo@intel.com> > --- > drivers/raw/ntb/ntb.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c > index cc611dfbb9..0801e6d1ae 100644 > --- a/drivers/raw/ntb/ntb.c > +++ b/drivers/raw/ntb/ntb.c > @@ -1403,8 +1403,12 @@ ntb_init_hw(struct rte_rawdev *dev, struct rte_pci_device > *pci_dev) > > intr_handle = pci_dev->intr_handle; > /* Register callback func to eal lib */ > - rte_intr_callback_register(intr_handle, > - ntb_dev_intr_handler, dev); > + ret = rte_intr_callback_register(intr_handle, > + ntb_dev_intr_handler, dev); > + if (ret) { > + NTB_LOG(ERR, "Unable to register doorbell intr handler."); > + return ret; > + } When will this register failure happen? Have you checked what is the root cause? > > ret = rte_intr_efd_enable(intr_handle, hw->db_cnt); > if (ret) Need roll back, such as rte_intr_callback_unregister is required when fail or driver remove? > -- > 2.25.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH] raw/ntb: add check for DB intr handler registering 2022-02-10 7:04 ` Wu, Jingjing @ 2022-02-10 7:18 ` Guo, Junfeng 2022-02-10 10:43 ` Guo, Junfeng 0 siblings, 1 reply; 8+ messages in thread From: Guo, Junfeng @ 2022-02-10 7:18 UTC (permalink / raw) To: Wu, Jingjing; +Cc: dev, stable > -----Original Message----- > From: Wu, Jingjing <jingjing.wu@intel.com> > Sent: Thursday, February 10, 2022 15:05 > To: Guo, Junfeng <junfeng.guo@intel.com> > Cc: dev@dpdk.org; stable@dpdk.org > Subject: RE: [PATCH] raw/ntb: add check for DB intr handler registering > > > > > -----Original Message----- > > From: Guo, Junfeng <junfeng.guo@intel.com> > > Sent: Thursday, February 10, 2022 2:29 PM > > To: Wu, Jingjing <jingjing.wu@intel.com> > > Cc: dev@dpdk.org; stable@dpdk.org; Guo, Junfeng > <junfeng.guo@intel.com> > > Subject: [PATCH] raw/ntb: add check for DB intr handler registering > > > > The callback registering of doorbell interrupt handler should be > > finished before enabling the interrupt event fd. Thus add the return > > value check for this callback registering. > > > > Fixes: 62012a76811e ("raw/ntb: add handshake process") > > Cc: stable@dpdk.org > > > > Signed-off-by: Junfeng Guo <junfeng.guo@intel.com> > > --- > > drivers/raw/ntb/ntb.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c > > index cc611dfbb9..0801e6d1ae 100644 > > --- a/drivers/raw/ntb/ntb.c > > +++ b/drivers/raw/ntb/ntb.c > > @@ -1403,8 +1403,12 @@ ntb_init_hw(struct rte_rawdev *dev, struct > rte_pci_device > > *pci_dev) > > > > intr_handle = pci_dev->intr_handle; > > /* Register callback func to eal lib */ > > - rte_intr_callback_register(intr_handle, > > - ntb_dev_intr_handler, dev); > > + ret = rte_intr_callback_register(intr_handle, > > + ntb_dev_intr_handler, dev); > > + if (ret) { > > + NTB_LOG(ERR, "Unable to register doorbell intr > handler."); > > + return ret; > > + } > When will this register failure happen? Have you checked what is the root > cause? When bind with vfio-pci, the DB interrupt callback function often cannot work as expected. I think this is due to that, the intr callback registering not finished before event fd enabled. Add the check of the return value here can make sure about correct processing sequence. > > > > > ret = rte_intr_efd_enable(intr_handle, hw->db_cnt); > > if (ret) > Need roll back, such as rte_intr_callback_unregister is required when fail > or driver remove? > > -- > > 2.25.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH] raw/ntb: add check for DB intr handler registering 2022-02-10 7:18 ` Guo, Junfeng @ 2022-02-10 10:43 ` Guo, Junfeng 2022-03-07 17:07 ` Thomas Monjalon 0 siblings, 1 reply; 8+ messages in thread From: Guo, Junfeng @ 2022-02-10 10:43 UTC (permalink / raw) To: Wu, Jingjing; +Cc: dev, stable > -----Original Message----- > From: Guo, Junfeng > Sent: Thursday, February 10, 2022 15:18 > To: Wu, Jingjing <jingjing.wu@intel.com> > Cc: dev@dpdk.org; stable@dpdk.org > Subject: RE: [PATCH] raw/ntb: add check for DB intr handler registering > > > > > -----Original Message----- > > From: Wu, Jingjing <jingjing.wu@intel.com> > > Sent: Thursday, February 10, 2022 15:05 > > To: Guo, Junfeng <junfeng.guo@intel.com> > > Cc: dev@dpdk.org; stable@dpdk.org > > Subject: RE: [PATCH] raw/ntb: add check for DB intr handler registering > > > > > > > > > -----Original Message----- > > > From: Guo, Junfeng <junfeng.guo@intel.com> > > > Sent: Thursday, February 10, 2022 2:29 PM > > > To: Wu, Jingjing <jingjing.wu@intel.com> > > > Cc: dev@dpdk.org; stable@dpdk.org; Guo, Junfeng > > <junfeng.guo@intel.com> > > > Subject: [PATCH] raw/ntb: add check for DB intr handler registering > > > > > > The callback registering of doorbell interrupt handler should be > > > finished before enabling the interrupt event fd. Thus add the return > > > value check for this callback registering. > > > > > > Fixes: 62012a76811e ("raw/ntb: add handshake process") > > > Cc: stable@dpdk.org > > > > > > Signed-off-by: Junfeng Guo <junfeng.guo@intel.com> > > > --- > > > drivers/raw/ntb/ntb.c | 8 ++++++-- > > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c > > > index cc611dfbb9..0801e6d1ae 100644 > > > --- a/drivers/raw/ntb/ntb.c > > > +++ b/drivers/raw/ntb/ntb.c > > > @@ -1403,8 +1403,12 @@ ntb_init_hw(struct rte_rawdev *dev, > struct > > rte_pci_device > > > *pci_dev) > > > > > > intr_handle = pci_dev->intr_handle; > > > /* Register callback func to eal lib */ > > > - rte_intr_callback_register(intr_handle, > > > - ntb_dev_intr_handler, dev); > > > + ret = rte_intr_callback_register(intr_handle, > > > + ntb_dev_intr_handler, dev); > > > + if (ret) { > > > + NTB_LOG(ERR, "Unable to register doorbell intr > > handler."); > > > + return ret; > > > + } > > When will this register failure happen? Have you checked what is the > root > > cause? > > When bind with vfio-pci, the DB interrupt callback function often cannot > work as expected. > I think this is due to that, the intr callback registering not finished before > event fd enabled. > Add the check of the return value here can make sure about correct > processing sequence. I think some compiler optimization would lead to this condition. So add return value check can prevent this (i.e., registering not finished). > > > > > > > > > ret = rte_intr_efd_enable(intr_handle, hw->db_cnt); > > > if (ret) > > Need roll back, such as rte_intr_callback_unregister is required when > fail > > or driver remove? > > > -- > > > 2.25.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] raw/ntb: add check for DB intr handler registering 2022-02-10 10:43 ` Guo, Junfeng @ 2022-03-07 17:07 ` Thomas Monjalon 2022-03-08 8:08 ` Guo, Junfeng 0 siblings, 1 reply; 8+ messages in thread From: Thomas Monjalon @ 2022-03-07 17:07 UTC (permalink / raw) To: Wu, Jingjing, Guo, Junfeng; +Cc: dev, stable 10/02/2022 11:43, Guo, Junfeng: > > > -----Original Message----- > > From: Guo, Junfeng > > Sent: Thursday, February 10, 2022 15:18 > > To: Wu, Jingjing <jingjing.wu@intel.com> > > Cc: dev@dpdk.org; stable@dpdk.org > > Subject: RE: [PATCH] raw/ntb: add check for DB intr handler registering > > > > > > > > > -----Original Message----- > > > From: Wu, Jingjing <jingjing.wu@intel.com> > > > Sent: Thursday, February 10, 2022 15:05 > > > To: Guo, Junfeng <junfeng.guo@intel.com> > > > Cc: dev@dpdk.org; stable@dpdk.org > > > Subject: RE: [PATCH] raw/ntb: add check for DB intr handler registering > > > > > > > > > > > > > -----Original Message----- > > > > From: Guo, Junfeng <junfeng.guo@intel.com> > > > > Sent: Thursday, February 10, 2022 2:29 PM > > > > To: Wu, Jingjing <jingjing.wu@intel.com> > > > > Cc: dev@dpdk.org; stable@dpdk.org; Guo, Junfeng > > > <junfeng.guo@intel.com> > > > > Subject: [PATCH] raw/ntb: add check for DB intr handler registering > > > > > > > > The callback registering of doorbell interrupt handler should be > > > > finished before enabling the interrupt event fd. Thus add the return > > > > value check for this callback registering. > > > > > > > > Fixes: 62012a76811e ("raw/ntb: add handshake process") > > > > Cc: stable@dpdk.org > > > > > > > > Signed-off-by: Junfeng Guo <junfeng.guo@intel.com> > > > > --- > > > > drivers/raw/ntb/ntb.c | 8 ++++++-- > > > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c > > > > index cc611dfbb9..0801e6d1ae 100644 > > > > --- a/drivers/raw/ntb/ntb.c > > > > +++ b/drivers/raw/ntb/ntb.c > > > > @@ -1403,8 +1403,12 @@ ntb_init_hw(struct rte_rawdev *dev, > > struct > > > rte_pci_device > > > > *pci_dev) > > > > > > > > intr_handle = pci_dev->intr_handle; > > > > /* Register callback func to eal lib */ > > > > - rte_intr_callback_register(intr_handle, > > > > - ntb_dev_intr_handler, dev); > > > > + ret = rte_intr_callback_register(intr_handle, > > > > + ntb_dev_intr_handler, dev); > > > > + if (ret) { > > > > + NTB_LOG(ERR, "Unable to register doorbell intr > > > handler."); > > > > + return ret; > > > > + } > > > When will this register failure happen? Have you checked what is the > > root > > > cause? > > > > When bind with vfio-pci, the DB interrupt callback function often cannot > > work as expected. > > I think this is due to that, the intr callback registering not finished before > > event fd enabled. > > Add the check of the return value here can make sure about correct > > processing sequence. > > I think some compiler optimization would lead to this condition. > So add return value check can prevent this (i.e., registering not finished). There is no formal ack. What is the status of this patch? ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH] raw/ntb: add check for DB intr handler registering 2022-03-07 17:07 ` Thomas Monjalon @ 2022-03-08 8:08 ` Guo, Junfeng 2022-03-08 8:09 ` Thomas Monjalon 0 siblings, 1 reply; 8+ messages in thread From: Guo, Junfeng @ 2022-03-08 8:08 UTC (permalink / raw) To: Thomas Monjalon, Wu, Jingjing; +Cc: dev, stable > -----Original Message----- > From: Thomas Monjalon <thomas@monjalon.net> > Sent: Tuesday, March 8, 2022 01:07 > To: Wu, Jingjing <jingjing.wu@intel.com>; Guo, Junfeng > <junfeng.guo@intel.com> > Cc: dev@dpdk.org; stable@dpdk.org > Subject: Re: [PATCH] raw/ntb: add check for DB intr handler registering > > 10/02/2022 11:43, Guo, Junfeng: > > > > > -----Original Message----- > > > From: Guo, Junfeng > > > Sent: Thursday, February 10, 2022 15:18 > > > To: Wu, Jingjing <jingjing.wu@intel.com> > > > Cc: dev@dpdk.org; stable@dpdk.org > > > Subject: RE: [PATCH] raw/ntb: add check for DB intr handler > registering > > > > > > > > > > > > > -----Original Message----- > > > > From: Wu, Jingjing <jingjing.wu@intel.com> > > > > Sent: Thursday, February 10, 2022 15:05 > > > > To: Guo, Junfeng <junfeng.guo@intel.com> > > > > Cc: dev@dpdk.org; stable@dpdk.org > > > > Subject: RE: [PATCH] raw/ntb: add check for DB intr handler > registering > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > From: Guo, Junfeng <junfeng.guo@intel.com> > > > > > Sent: Thursday, February 10, 2022 2:29 PM > > > > > To: Wu, Jingjing <jingjing.wu@intel.com> > > > > > Cc: dev@dpdk.org; stable@dpdk.org; Guo, Junfeng > > > > <junfeng.guo@intel.com> > > > > > Subject: [PATCH] raw/ntb: add check for DB intr handler > registering > > > > > > > > > > The callback registering of doorbell interrupt handler should be > > > > > finished before enabling the interrupt event fd. Thus add the return > > > > > value check for this callback registering. > > > > > > > > > > Fixes: 62012a76811e ("raw/ntb: add handshake process") > > > > > Cc: stable@dpdk.org > > > > > > > > > > Signed-off-by: Junfeng Guo <junfeng.guo@intel.com> > > > > > --- > > > > > drivers/raw/ntb/ntb.c | 8 ++++++-- > > > > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c > > > > > index cc611dfbb9..0801e6d1ae 100644 > > > > > --- a/drivers/raw/ntb/ntb.c > > > > > +++ b/drivers/raw/ntb/ntb.c > > > > > @@ -1403,8 +1403,12 @@ ntb_init_hw(struct rte_rawdev *dev, > > > struct > > > > rte_pci_device > > > > > *pci_dev) > > > > > > > > > > intr_handle = pci_dev->intr_handle; > > > > > /* Register callback func to eal lib */ > > > > > - rte_intr_callback_register(intr_handle, > > > > > - ntb_dev_intr_handler, dev); > > > > > + ret = rte_intr_callback_register(intr_handle, > > > > > + ntb_dev_intr_handler, > dev); > > > > > + if (ret) { > > > > > + NTB_LOG(ERR, "Unable to register doorbell intr > > > > handler."); > > > > > + return ret; > > > > > + } > > > > When will this register failure happen? Have you checked what is the > > > root > > > > cause? > > > > > > When bind with vfio-pci, the DB interrupt callback function often > cannot > > > work as expected. > > > I think this is due to that, the intr callback registering not finished > before > > > event fd enabled. > > > Add the check of the return value here can make sure about correct > > > processing sequence. > > > > I think some compiler optimization would lead to this condition. > > So add return value check can prevent this (i.e., registering not finished). > > There is no formal ack. What is the status of this patch? > Thanks for the reminding! Currently, we agreed that this fix is not for the root cause of no response of doorbell interrupt when building the connection between two NTB ports. So we decided to change the status to be superseded and do further investigation for NTB. Thanks for the reviewing! > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] raw/ntb: add check for DB intr handler registering 2022-03-08 8:08 ` Guo, Junfeng @ 2022-03-08 8:09 ` Thomas Monjalon 2022-03-08 8:11 ` Guo, Junfeng 0 siblings, 1 reply; 8+ messages in thread From: Thomas Monjalon @ 2022-03-08 8:09 UTC (permalink / raw) To: Wu, Jingjing, Guo, Junfeng; +Cc: dev, stable 08/03/2022 09:08, Guo, Junfeng: > From: Thomas Monjalon <thomas@monjalon.net> > > There is no formal ack. What is the status of this patch? > > > > Thanks for the reminding! > Currently, we agreed that this fix is not for the root cause of no response of doorbell > interrupt when building the connection between two NTB ports. So we decided to > change the status to be superseded and do further investigation for NTB. > Thanks for the reviewing! In this case, it should not be superseded but rejected. ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH] raw/ntb: add check for DB intr handler registering 2022-03-08 8:09 ` Thomas Monjalon @ 2022-03-08 8:11 ` Guo, Junfeng 0 siblings, 0 replies; 8+ messages in thread From: Guo, Junfeng @ 2022-03-08 8:11 UTC (permalink / raw) To: Thomas Monjalon, Wu, Jingjing; +Cc: dev, stable > -----Original Message----- > From: Thomas Monjalon <thomas@monjalon.net> > Sent: Tuesday, March 8, 2022 16:10 > To: Wu, Jingjing <jingjing.wu@intel.com>; Guo, Junfeng > <junfeng.guo@intel.com> > Cc: dev@dpdk.org; stable@dpdk.org > Subject: Re: [PATCH] raw/ntb: add check for DB intr handler registering > > 08/03/2022 09:08, Guo, Junfeng: > > From: Thomas Monjalon <thomas@monjalon.net> > > > There is no formal ack. What is the status of this patch? > > > > > > > Thanks for the reminding! > > Currently, we agreed that this fix is not for the root cause of no > response of doorbell > > interrupt when building the connection between two NTB ports. So we > decided to > > change the status to be superseded and do further investigation for NTB. > > Thanks for the reviewing! > > In this case, it should not be superseded but rejected. > Sure, thanks for your comment! > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-03-08 8:12 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-02-10 6:28 [PATCH] raw/ntb: add check for DB intr handler registering Junfeng Guo 2022-02-10 7:04 ` Wu, Jingjing 2022-02-10 7:18 ` Guo, Junfeng 2022-02-10 10:43 ` Guo, Junfeng 2022-03-07 17:07 ` Thomas Monjalon 2022-03-08 8:08 ` Guo, Junfeng 2022-03-08 8:09 ` Thomas Monjalon 2022-03-08 8:11 ` Guo, Junfeng
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).