net/iavf: fix core dump when the link is unstable

Message ID 20240806003527.2901320-1-kaiwenx.deng@intel.com (mailing list archive)
State Accepted
Delegated to: Bruce Richardson
Headers
Series net/iavf: fix core dump when the link is unstable |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-broadcom-Performance success Performance Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-marvell-Functional success Functional Testing PASS
ci/intel-Functional success Functional PASS
ci/github-robot: build success github build: passed
ci/iol-abi-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-sample-apps-testing success Testing PASS

Commit Message

Kaiwen Deng Aug. 6, 2024, 12:35 a.m. UTC
Physical link instability may cause a core dump.
Unstable physical links can result in a large number of link
change events.Link change events captured by vf before vf
resources are allocated will result in a core dump.

This commit will check if vf_res is invalid before calling it.

Fixes: 5e03e316c753 ("net/iavf: handle virtchnl event message without interrupt")
Cc: stable@dpdk.org

Signed-off-by: Kaiwen Deng <kaiwenx.deng@intel.com>
---
 drivers/net/iavf/iavf_vchnl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Bruce Richardson Aug. 22, 2024, 5:06 p.m. UTC | #1
On Tue, Aug 06, 2024 at 08:35:27AM +0800, Kaiwen Deng wrote:
> Physical link instability may cause a core dump.
> Unstable physical links can result in a large number of link
> change events.Link change events captured by vf before vf
> resources are allocated will result in a core dump.
> 
> This commit will check if vf_res is invalid before calling it.
> 
> Fixes: 5e03e316c753 ("net/iavf: handle virtchnl event message without interrupt")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Kaiwen Deng <kaiwenx.deng@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  drivers/net/iavf/iavf_vchnl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
> index 6d5969f084..b85debd40d 100644
> --- a/drivers/net/iavf/iavf_vchnl.c
> +++ b/drivers/net/iavf/iavf_vchnl.c
> @@ -255,7 +255,7 @@ iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len,
>  		case VIRTCHNL_EVENT_LINK_CHANGE:
>  			vf->link_up =
>  				vpe->event_data.link_event.link_status;
> -			if (vf->vf_res->vf_cap_flags &
> +			if (vf->vf_res != NULL && vf->vf_res->vf_cap_flags &
>  				VIRTCHNL_VF_CAP_ADV_LINK_SPEED) {

Since we are adjusting the conditional, we might as well fix the
indentation of it while we are at it. We can use up to 100 columns in DPDK
code, so there is no need to split the flags comparison across two lines.
Also the line continuation should not use a single tab indent - that makes
it look part of the body of the "if" statement, not part of the conditional
itself.
Will fix this on apply.

Thanks.
  
Bruce Richardson Aug. 22, 2024, 5:16 p.m. UTC | #2
On Thu, Aug 22, 2024 at 06:06:39PM +0100, Bruce Richardson wrote:
> On Tue, Aug 06, 2024 at 08:35:27AM +0800, Kaiwen Deng wrote:
> > Physical link instability may cause a core dump.
> > Unstable physical links can result in a large number of link
> > change events.Link change events captured by vf before vf
> > resources are allocated will result in a core dump.
> > 
> > This commit will check if vf_res is invalid before calling it.
> > 
> > Fixes: 5e03e316c753 ("net/iavf: handle virtchnl event message without interrupt")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Kaiwen Deng <kaiwenx.deng@intel.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> > ---
> >  drivers/net/iavf/iavf_vchnl.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
> > index 6d5969f084..b85debd40d 100644
> > --- a/drivers/net/iavf/iavf_vchnl.c
> > +++ b/drivers/net/iavf/iavf_vchnl.c
> > @@ -255,7 +255,7 @@ iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len,
> >  		case VIRTCHNL_EVENT_LINK_CHANGE:
> >  			vf->link_up =
> >  				vpe->event_data.link_event.link_status;
> > -			if (vf->vf_res->vf_cap_flags &
> > +			if (vf->vf_res != NULL && vf->vf_res->vf_cap_flags &
> >  				VIRTCHNL_VF_CAP_ADV_LINK_SPEED) {
> 
> Since we are adjusting the conditional, we might as well fix the
> indentation of it while we are at it. We can use up to 100 columns in DPDK
> code, so there is no need to split the flags comparison across two lines.
> Also the line continuation should not use a single tab indent - that makes
> it look part of the body of the "if" statement, not part of the conditional
> itself.
> Will fix this on apply.
> 
Patch applied to dpdk-next-net-intel

/Bruce
  

Patch

diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
index 6d5969f084..b85debd40d 100644
--- a/drivers/net/iavf/iavf_vchnl.c
+++ b/drivers/net/iavf/iavf_vchnl.c
@@ -255,7 +255,7 @@  iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len,
 		case VIRTCHNL_EVENT_LINK_CHANGE:
 			vf->link_up =
 				vpe->event_data.link_event.link_status;
-			if (vf->vf_res->vf_cap_flags &
+			if (vf->vf_res != NULL && vf->vf_res->vf_cap_flags &
 				VIRTCHNL_VF_CAP_ADV_LINK_SPEED) {
 				vf->link_speed =
 				    vpe->event_data.link_event_adv.link_speed;