diff mbox

[dpdk-dev] net/i40e/vf: reset scatter_rx flag when configuration complies

Message ID 1522354407-90598-1-git-send-email-eduser25@gmail.com (mailing list archive)
State Changes Requested, archived
Delegated to: Helin Zhang
Headers

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation fail Compilation issues

Commit Message

Eduard Serra March 29, 2018, 8:13 p.m. UTC 
  From: Eduard Serra <eduser25@gmail.com>

Scatter RX (scattered_rx) flag is currently not being resetted
when new configuration is suplied to reconfigure a port,
rendering the rx callback logic to always fall through the
scatter branch.

Signed-off-by: Eduard Serra <eduser25@gmail.com>
---
 drivers/net/i40e/i40e_ethdev_vf.c | 2 ++
 drivers/net/i40e/i40e_rxtx.c      | 2 ++
 2 files changed, 4 insertions(+)

Comments

Qi Zhang April 2, 2018, 12:14 p.m. UTC | #1 
Hi Serra:

> -----Original Message-----
> From: eduser25@gmail.com [mailto:eduser25@gmail.com]
> Sent: Friday, March 30, 2018 4:13 AM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Eduard Serra <eduser25@gmail.com>
> Subject: [PATCH] net/i40e/vf: reset scatter_rx flag when configuration
> complies
> 
> From: Eduard Serra <eduser25@gmail.com>
> 
> Scatter RX (scattered_rx) flag is currently not being resetted when new
> configuration is suplied to reconfigure a port, rendering the rx callback logic to

s/suplied/supplied

> always fall through the scatter branch.
> 
> Signed-off-by: Eduard Serra <eduser25@gmail.com>
> ---
>  drivers/net/i40e/i40e_ethdev_vf.c | 2 ++
>  drivers/net/i40e/i40e_rxtx.c      | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> b/drivers/net/i40e/i40e_ethdev_vf.c
> index 750d849..a072154 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -1750,6 +1750,8 @@ i40evf_rxq_init(struct rte_eth_dev *dev, struct
> i40e_rx_queue *rxq)
>  	if (dev_data->dev_conf.rxmode.enable_scatter ||
>  	    (rxq->max_pkt_len + 2 * I40E_VLAN_TAG_SIZE) > buf_size) {
>  		dev_data->scattered_rx = 1;
> +	} else {
> +		dev_data->scattered_rx = 0;

Scattered_rx should not be reset if any queue already required
You should reset it before the queue loop 

>  	}
> 
>  	return 0;
> diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c index
> 1217e5a..8698747 100644
> --- a/drivers/net/i40e/i40e_rxtx.c
> +++ b/drivers/net/i40e/i40e_rxtx.c
> @@ -2561,6 +2561,8 @@ i40e_rx_queue_init(struct i40e_rx_queue *rxq)
>  	/* Check if scattered RX needs to be used. */
>  	if ((rxq->max_pkt_len + 2 * I40E_VLAN_TAG_SIZE) > buf_size) {
>  		dev_data->scattered_rx = 1;
> +	} else {
> +		dev_data->scattered_rx = 0;

Same as above.

Regards
Qi

>  	}
> 
>  	/* Init the RX tail regieter. */
> --
> 2.7.4
Zhang, Helin April 6, 2018, 8:46 a.m. UTC | #2 
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhang, Qi Z
> Sent: Monday, April 2, 2018 8:14 PM
> To: eduser25@gmail.com
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/i40e/vf: reset scatter_rx flag when
> configuration complies
> 
> Hi Serra:
> 
> > -----Original Message-----
> > From: eduser25@gmail.com [mailto:eduser25@gmail.com]
> > Sent: Friday, March 30, 2018 4:13 AM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>
> > Cc: dev@dpdk.org; Eduard Serra <eduser25@gmail.com>
> > Subject: [PATCH] net/i40e/vf: reset scatter_rx flag when configuration
> > complies
> >
> > From: Eduard Serra <eduser25@gmail.com>
> >
> > Scatter RX (scattered_rx) flag is currently not being resetted when
> > new configuration is suplied to reconfigure a port, rendering the rx
> > callback logic to
> 
> s/suplied/supplied
> 
> > always fall through the scatter branch.
> >
> > Signed-off-by: Eduard Serra <eduser25@gmail.com>
> > ---
> >  drivers/net/i40e/i40e_ethdev_vf.c | 2 ++
> >  drivers/net/i40e/i40e_rxtx.c      | 2 ++
> >  2 files changed, 4 insertions(+)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> > b/drivers/net/i40e/i40e_ethdev_vf.c
> > index 750d849..a072154 100644
> > --- a/drivers/net/i40e/i40e_ethdev_vf.c
> > +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> > @@ -1750,6 +1750,8 @@ i40evf_rxq_init(struct rte_eth_dev *dev, struct
> > i40e_rx_queue *rxq)
> >  	if (dev_data->dev_conf.rxmode.enable_scatter ||
> >  	    (rxq->max_pkt_len + 2 * I40E_VLAN_TAG_SIZE) > buf_size) {
> >  		dev_data->scattered_rx = 1;
> > +	} else {
> > +		dev_data->scattered_rx = 0;
> 
> Scattered_rx should not be reset if any queue already required You should
> reset it before the queue loop
Does it mean a NACK?

/Helin

> 
> >  	}
> >
> >  	return 0;
> > diff --git a/drivers/net/i40e/i40e_rxtx.c
> > b/drivers/net/i40e/i40e_rxtx.c index
> > 1217e5a..8698747 100644
> > --- a/drivers/net/i40e/i40e_rxtx.c
> > +++ b/drivers/net/i40e/i40e_rxtx.c
> > @@ -2561,6 +2561,8 @@ i40e_rx_queue_init(struct i40e_rx_queue *rxq)
> >  	/* Check if scattered RX needs to be used. */
> >  	if ((rxq->max_pkt_len + 2 * I40E_VLAN_TAG_SIZE) > buf_size) {
> >  		dev_data->scattered_rx = 1;
> > +	} else {
> > +		dev_data->scattered_rx = 0;
> 
> Same as above.
> 
> Regards
> Qi
> 
> >  	}
> >
> >  	/* Init the RX tail regieter. */
> > --
> > 2.7.4
Zhang, Helin April 6, 2018, 3:53 p.m. UTC | #3 
> -----Original Message-----
> From: Zhang, Helin
> Sent: Friday, April 6, 2018 4:46 PM
> To: Zhang, Qi Z; eduser25@gmail.com
> Cc: dev@dpdk.org
> Subject: RE: [PATCH] net/i40e/vf: reset scatter_rx flag when configuration
> complies
> 
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhang, Qi Z
> > Sent: Monday, April 2, 2018 8:14 PM
> > To: eduser25@gmail.com
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] net/i40e/vf: reset scatter_rx flag
> > when configuration complies
> >
> > Hi Serra:
> >
> > > -----Original Message-----
> > > From: eduser25@gmail.com [mailto:eduser25@gmail.com]
> > > Sent: Friday, March 30, 2018 4:13 AM
> > > To: Zhang, Qi Z <qi.z.zhang@intel.com>
> > > Cc: dev@dpdk.org; Eduard Serra <eduser25@gmail.com>
> > > Subject: [PATCH] net/i40e/vf: reset scatter_rx flag when
> > > configuration complies
> > >
> > > From: Eduard Serra <eduser25@gmail.com>
> > >
> > > Scatter RX (scattered_rx) flag is currently not being resetted when
> > > new configuration is suplied to reconfigure a port, rendering the rx
> > > callback logic to
> >
> > s/suplied/supplied
> >
> > > always fall through the scatter branch.
> > >
> > > Signed-off-by: Eduard Serra <eduser25@gmail.com>
> > > ---
> > >  drivers/net/i40e/i40e_ethdev_vf.c | 2 ++
> > >  drivers/net/i40e/i40e_rxtx.c      | 2 ++
> > >  2 files changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> > > b/drivers/net/i40e/i40e_ethdev_vf.c
> > > index 750d849..a072154 100644
> > > --- a/drivers/net/i40e/i40e_ethdev_vf.c
> > > +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> > > @@ -1750,6 +1750,8 @@ i40evf_rxq_init(struct rte_eth_dev *dev,
> > > struct i40e_rx_queue *rxq)
> > >  	if (dev_data->dev_conf.rxmode.enable_scatter ||
> > >  	    (rxq->max_pkt_len + 2 * I40E_VLAN_TAG_SIZE) > buf_size) {
> > >  		dev_data->scattered_rx = 1;
> > > +	} else {
> > > +		dev_data->scattered_rx = 0;
> >
> > Scattered_rx should not be reset if any queue already required You
> > should reset it before the queue loop
> Does it mean a NACK?
> 
> /Helin
Anyway, please fix the checkpatch warnings reported on patchwork. Thanks!

/Helin
> 
> >
> > >  	}
> > >
> > >  	return 0;
> > > diff --git a/drivers/net/i40e/i40e_rxtx.c
> > > b/drivers/net/i40e/i40e_rxtx.c index
> > > 1217e5a..8698747 100644
> > > --- a/drivers/net/i40e/i40e_rxtx.c
> > > +++ b/drivers/net/i40e/i40e_rxtx.c
> > > @@ -2561,6 +2561,8 @@ i40e_rx_queue_init(struct i40e_rx_queue *rxq)
> > >  	/* Check if scattered RX needs to be used. */
> > >  	if ((rxq->max_pkt_len + 2 * I40E_VLAN_TAG_SIZE) > buf_size) {
> > >  		dev_data->scattered_rx = 1;
> > > +	} else {
> > > +		dev_data->scattered_rx = 0;
> >
> > Same as above.
> >
> > Regards
> > Qi
> >
> > >  	}
> > >
> > >  	/* Init the RX tail regieter. */
> > > --
> > > 2.7.4
Qi Zhang April 6, 2018, 4 p.m. UTC | #4 
> -----Original Message-----
> From: Zhang, Helin
> Sent: Friday, April 6, 2018 11:54 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; eduser25@gmail.com
> Cc: dev@dpdk.org
> Subject: RE: [PATCH] net/i40e/vf: reset scatter_rx flag when configuration
> complies
> 
> 
> 
> > -----Original Message-----
> > From: Zhang, Helin
> > Sent: Friday, April 6, 2018 4:46 PM
> > To: Zhang, Qi Z; eduser25@gmail.com
> > Cc: dev@dpdk.org
> > Subject: RE: [PATCH] net/i40e/vf: reset scatter_rx flag when
> > configuration complies
> >
> >
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhang, Qi Z
> > > Sent: Monday, April 2, 2018 8:14 PM
> > > To: eduser25@gmail.com
> > > Cc: dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH] net/i40e/vf: reset scatter_rx flag
> > > when configuration complies
> > >
> > > Hi Serra:
> > >
> > > > -----Original Message-----
> > > > From: eduser25@gmail.com [mailto:eduser25@gmail.com]
> > > > Sent: Friday, March 30, 2018 4:13 AM
> > > > To: Zhang, Qi Z <qi.z.zhang@intel.com>
> > > > Cc: dev@dpdk.org; Eduard Serra <eduser25@gmail.com>
> > > > Subject: [PATCH] net/i40e/vf: reset scatter_rx flag when
> > > > configuration complies
> > > >
> > > > From: Eduard Serra <eduser25@gmail.com>
> > > >
> > > > Scatter RX (scattered_rx) flag is currently not being resetted
> > > > when new configuration is suplied to reconfigure a port, rendering
> > > > the rx callback logic to
> > >
> > > s/suplied/supplied
> > >
> > > > always fall through the scatter branch.
> > > >
> > > > Signed-off-by: Eduard Serra <eduser25@gmail.com>
> > > > ---
> > > >  drivers/net/i40e/i40e_ethdev_vf.c | 2 ++
> > > >  drivers/net/i40e/i40e_rxtx.c      | 2 ++
> > > >  2 files changed, 4 insertions(+)
> > > >
> > > > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> > > > b/drivers/net/i40e/i40e_ethdev_vf.c
> > > > index 750d849..a072154 100644
> > > > --- a/drivers/net/i40e/i40e_ethdev_vf.c
> > > > +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> > > > @@ -1750,6 +1750,8 @@ i40evf_rxq_init(struct rte_eth_dev *dev,
> > > > struct i40e_rx_queue *rxq)
> > > >  	if (dev_data->dev_conf.rxmode.enable_scatter ||
> > > >  	    (rxq->max_pkt_len + 2 * I40E_VLAN_TAG_SIZE) > buf_size) {
> > > >  		dev_data->scattered_rx = 1;
> > > > +	} else {
> > > > +		dev_data->scattered_rx = 0;
> > >
> > > Scattered_rx should not be reset if any queue already required You
> > > should reset it before the queue loop
> > Does it mean a NACK?

Yes the fix is not correct.

> >
> > /Helin
> Anyway, please fix the checkpatch warnings reported on patchwork. Thanks!
> 
> /Helin
> >
> > >
> > > >  	}
> > > >
> > > >  	return 0;
> > > > diff --git a/drivers/net/i40e/i40e_rxtx.c
> > > > b/drivers/net/i40e/i40e_rxtx.c index
> > > > 1217e5a..8698747 100644
> > > > --- a/drivers/net/i40e/i40e_rxtx.c
> > > > +++ b/drivers/net/i40e/i40e_rxtx.c
> > > > @@ -2561,6 +2561,8 @@ i40e_rx_queue_init(struct i40e_rx_queue *rxq)
> > > >  	/* Check if scattered RX needs to be used. */
> > > >  	if ((rxq->max_pkt_len + 2 * I40E_VLAN_TAG_SIZE) > buf_size) {
> > > >  		dev_data->scattered_rx = 1;
> > > > +	} else {
> > > > +		dev_data->scattered_rx = 0;
> > >
> > > Same as above.
> > >
> > > Regards
> > > Qi
> > >
> > > >  	}
> > > >
> > > >  	/* Init the RX tail regieter. */
> > > > --
> > > > 2.7.4
diff mbox

Patch

diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 750d849..a072154 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1750,6 +1750,8 @@  i40evf_rxq_init(struct rte_eth_dev *dev, struct i40e_rx_queue *rxq)
 	if (dev_data->dev_conf.rxmode.enable_scatter ||
 	    (rxq->max_pkt_len + 2 * I40E_VLAN_TAG_SIZE) > buf_size) {
 		dev_data->scattered_rx = 1;
+	} else {
+		dev_data->scattered_rx = 0;
 	}
 
 	return 0;
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 1217e5a..8698747 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -2561,6 +2561,8 @@  i40e_rx_queue_init(struct i40e_rx_queue *rxq)
 	/* Check if scattered RX needs to be used. */
 	if ((rxq->max_pkt_len + 2 * I40E_VLAN_TAG_SIZE) > buf_size) {
 		dev_data->scattered_rx = 1;
+	} else {
+		dev_data->scattered_rx = 0;
 	}
 
 	/* Init the RX tail regieter. */