diff mbox

[dpdk-dev] net/sfc: add missing Rx descriptor status callback

Message ID 1527490247-23324-1-git-send-email-arybchenko@solarflare.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Andrew Rybchenko May 28, 2018, 6:50 a.m. UTC 
  Rx descriptor status callback was lost and request of the Rx
descriptor status crashes application if equal stride super-buffer
Rx mode is used.

Fixes: 390f9b8d82c9 ("net/sfc: support equal stride super-buffer Rx mode")

Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/sfc/sfc_ef10_essb_rx.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Andrew Rybchenko May 30, 2018, 6:44 a.m. UTC | #1 
Thomas, Ferruh,

I've found out that the following patch is deferred in patchwork:
https://dpdk.org/dev/patchwork/patch/40455/

It is critical under certain conditions since fix application crash.
The patch is net/sfc local and just few lines - absolutely harmless.
The only potential DPDK-wide effect is build, but the patch is really
trivial and it is hard to imagine how it could affect the build.
I'd like it to be included in 18.05, if possible.

I understand that 18.05 is really very-very late already and it
could be too late even for such patch. Just would like to
double-check it.

Thanks,
Andrew.

On 05/28/2018 09:50 AM, Andrew Rybchenko wrote:
> Rx descriptor status callback was lost and request of the Rx
> descriptor status crashes application if equal stride super-buffer
> Rx mode is used.
>
> Fixes: 390f9b8d82c9 ("net/sfc: support equal stride super-buffer Rx mode")
>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
>   drivers/net/sfc/sfc_ef10_essb_rx.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
>
> diff --git a/drivers/net/sfc/sfc_ef10_essb_rx.c b/drivers/net/sfc/sfc_ef10_essb_rx.c
> index 289b61e52..5f5af602c 100644
> --- a/drivers/net/sfc/sfc_ef10_essb_rx.c
> +++ b/drivers/net/sfc/sfc_ef10_essb_rx.c
> @@ -413,6 +413,14 @@ sfc_ef10_essb_rx_qdesc_npending(__rte_unused struct sfc_dp_rxq *dp_rxq)
>   	return -ENOTSUP;
>   }
>   
> +static sfc_dp_rx_qdesc_status_t sfc_ef10_essb_rx_qdesc_status;
> +static int
> +sfc_ef10_essb_rx_qdesc_status(__rte_unused struct sfc_dp_rxq *dp_rxq,
> +			      __rte_unused uint16_t offset)
> +{
> +	return -ENOTSUP;
> +}
> +
>   static sfc_dp_rx_get_dev_info_t sfc_ef10_essb_rx_get_dev_info;
>   static void
>   sfc_ef10_essb_rx_get_dev_info(struct rte_eth_dev_info *dev_info)
> @@ -687,5 +695,6 @@ struct sfc_dp_rx sfc_ef10_essb_rx = {
>   	.qpurge			= sfc_ef10_essb_rx_qpurge,
>   	.supported_ptypes_get	= sfc_ef10_supported_ptypes_get,
>   	.qdesc_npending		= sfc_ef10_essb_rx_qdesc_npending,
> +	.qdesc_status		= sfc_ef10_essb_rx_qdesc_status,
>   	.pkt_burst		= sfc_ef10_essb_recv_pkts,
>   };
Ferruh Yigit May 30, 2018, 9:42 a.m. UTC | #2 
On 5/30/2018 7:44 AM, Andrew Rybchenko wrote:
> Thomas, Ferruh,
> 
> I've found out that the following patch is deferred in patchwork:
> https://dpdk.org/dev/patchwork/patch/40455/
> 
> It is critical under certain conditions since fix application crash.
> The patch is net/sfc local and just few lines - absolutely harmless.
> The only potential DPDK-wide effect is build, but the patch is really
> trivial and it is hard to imagine how it could affect the build.
> I'd like it to be included in 18.05, if possible.
> 
> I understand that 18.05 is really very-very late already and it
> could be too late even for such patch. Just would like to
> double-check it.

Hi Andrew,

I did mark it as deferred and I overlooked that this fixes a crash.

Agree that scope is local and only effect should be build, I confirmed the build
but to be sure I suggest get the patch and trigger an Intel daily build, and
include the patch into release if only that build also returns success.

> 
> Thanks,
> Andrew.
> 
> On 05/28/2018 09:50 AM, Andrew Rybchenko wrote:
>> Rx descriptor status callback was lost and request of the Rx
>> descriptor status crashes application if equal stride super-buffer
>> Rx mode is used.
>>
>> Fixes: 390f9b8d82c9 ("net/sfc: support equal stride super-buffer Rx mode")
>>
>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> ---
>>  drivers/net/sfc/sfc_ef10_essb_rx.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/net/sfc/sfc_ef10_essb_rx.c b/drivers/net/sfc/sfc_ef10_essb_rx.c
>> index 289b61e52..5f5af602c 100644
>> --- a/drivers/net/sfc/sfc_ef10_essb_rx.c
>> +++ b/drivers/net/sfc/sfc_ef10_essb_rx.c
>> @@ -413,6 +413,14 @@ sfc_ef10_essb_rx_qdesc_npending(__rte_unused struct sfc_dp_rxq *dp_rxq)
>>  	return -ENOTSUP;
>>  }
>>  
>> +static sfc_dp_rx_qdesc_status_t sfc_ef10_essb_rx_qdesc_status;
>> +static int
>> +sfc_ef10_essb_rx_qdesc_status(__rte_unused struct sfc_dp_rxq *dp_rxq,
>> +			      __rte_unused uint16_t offset)
>> +{
>> +	return -ENOTSUP;
>> +}
>> +
>>  static sfc_dp_rx_get_dev_info_t sfc_ef10_essb_rx_get_dev_info;
>>  static void
>>  sfc_ef10_essb_rx_get_dev_info(struct rte_eth_dev_info *dev_info)
>> @@ -687,5 +695,6 @@ struct sfc_dp_rx sfc_ef10_essb_rx = {
>>  	.qpurge			= sfc_ef10_essb_rx_qpurge,
>>  	.supported_ptypes_get	= sfc_ef10_supported_ptypes_get,
>>  	.qdesc_npending		= sfc_ef10_essb_rx_qdesc_npending,
>> +	.qdesc_status		= sfc_ef10_essb_rx_qdesc_status,
>>  	.pkt_burst		= sfc_ef10_essb_recv_pkts,
>>  };
>
Thomas Monjalon May 30, 2018, 10:30 a.m. UTC | #3 
30/05/2018 11:42, Ferruh Yigit:
> On 5/30/2018 7:44 AM, Andrew Rybchenko wrote:
> > Thomas, Ferruh,
> > 
> > I've found out that the following patch is deferred in patchwork:
> > https://dpdk.org/dev/patchwork/patch/40455/
> > 
> > It is critical under certain conditions since fix application crash.
> > The patch is net/sfc local and just few lines - absolutely harmless.
> > The only potential DPDK-wide effect is build, but the patch is really
> > trivial and it is hard to imagine how it could affect the build.
> > I'd like it to be included in 18.05, if possible.
> > 
> > I understand that 18.05 is really very-very late already and it
> > could be too late even for such patch. Just would like to
> > double-check it.
> 
> Hi Andrew,
> 
> I did mark it as deferred and I overlooked that this fixes a crash.
> 
> Agree that scope is local and only effect should be build, I confirmed the build
> but to be sure I suggest get the patch and trigger an Intel daily build, and
> include the patch into release if only that build also returns success.
> 
> > 
> > Thanks,
> > Andrew.
> > 
> > On 05/28/2018 09:50 AM, Andrew Rybchenko wrote:
> >> Rx descriptor status callback was lost and request of the Rx
> >> descriptor status crashes application if equal stride super-buffer
> >> Rx mode is used.
> >>
> >> Fixes: 390f9b8d82c9 ("net/sfc: support equal stride super-buffer Rx mode")
> >>
> >> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>

Applied, thanks for the heads up.
diff mbox

Patch

diff --git a/drivers/net/sfc/sfc_ef10_essb_rx.c b/drivers/net/sfc/sfc_ef10_essb_rx.c
index 289b61e52..5f5af602c 100644
--- a/drivers/net/sfc/sfc_ef10_essb_rx.c
+++ b/drivers/net/sfc/sfc_ef10_essb_rx.c
@@ -413,6 +413,14 @@  sfc_ef10_essb_rx_qdesc_npending(__rte_unused struct sfc_dp_rxq *dp_rxq)
 	return -ENOTSUP;
 }
 
+static sfc_dp_rx_qdesc_status_t sfc_ef10_essb_rx_qdesc_status;
+static int
+sfc_ef10_essb_rx_qdesc_status(__rte_unused struct sfc_dp_rxq *dp_rxq,
+			      __rte_unused uint16_t offset)
+{
+	return -ENOTSUP;
+}
+
 static sfc_dp_rx_get_dev_info_t sfc_ef10_essb_rx_get_dev_info;
 static void
 sfc_ef10_essb_rx_get_dev_info(struct rte_eth_dev_info *dev_info)
@@ -687,5 +695,6 @@  struct sfc_dp_rx sfc_ef10_essb_rx = {
 	.qpurge			= sfc_ef10_essb_rx_qpurge,
 	.supported_ptypes_get	= sfc_ef10_supported_ptypes_get,
 	.qdesc_npending		= sfc_ef10_essb_rx_qdesc_npending,
+	.qdesc_status		= sfc_ef10_essb_rx_qdesc_status,
 	.pkt_burst		= sfc_ef10_essb_recv_pkts,
 };