[dpdk-dev,25/38] net/dpaa: add support for MTU update

Message ID 1497591668-3320-26-git-send-email-shreyansh.jain@nxp.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

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

Commit Message

Shreyansh Jain June 16, 2017, 5:40 a.m. UTC
  Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 doc/guides/nics/features/dpaa.ini |  1 +
 drivers/net/dpaa/dpaa_ethdev.c    | 21 +++++++++++++++++++++
 2 files changed, 22 insertions(+)
  

Comments

Ferruh Yigit June 28, 2017, 3:45 p.m. UTC | #1
On 6/16/2017 6:40 AM, Shreyansh Jain wrote:
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>

<...>

>  static int
> +dpaa_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
> +{
> +	struct dpaa_if *dpaa_intf = dev->data->dev_private;
> +
> +	PMD_INIT_FUNC_TRACE();
> +
> +	if (mtu < ETHER_MIN_MTU)
> +		return -EINVAL;
> +
> +	fman_if_set_maxfrm(dpaa_intf->fif, mtu);
> +
> +	if (mtu > ETHER_MAX_LEN)
> +		return -1

Is it OK to have this check after fman_if_set_maxfrm() ?

> +	dev->data->dev_conf.rxmode.jumbo_frame = 0;
> +
> +	dev->data->dev_conf.rxmode.max_rx_pkt_len = mtu;

I think this only makes sense when jumbo_frame is 1, although not hurts
to set...

> +	return 0;
> +}
<...>
  
Shreyansh Jain June 29, 2017, 2:56 p.m. UTC | #2
On Wednesday 28 June 2017 09:15 PM, Ferruh Yigit wrote:
> On 6/16/2017 6:40 AM, Shreyansh Jain wrote:
>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> 
> <...>
> 
>>   static int
>> +dpaa_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
>> +{
>> +	struct dpaa_if *dpaa_intf = dev->data->dev_private;
>> +
>> +	PMD_INIT_FUNC_TRACE();
>> +
>> +	if (mtu < ETHER_MIN_MTU)
>> +		return -EINVAL;
>> +
>> +	fman_if_set_maxfrm(dpaa_intf->fif, mtu);
>> +
>> +	if (mtu > ETHER_MAX_LEN)
>> +		return -1
> 
> Is it OK to have this check after fman_if_set_maxfrm() ?

Indeed - bad code. I will fix this.

> 
>> +	dev->data->dev_conf.rxmode.jumbo_frame = 0;
>> +
>> +	dev->data->dev_conf.rxmode.max_rx_pkt_len = mtu;
> 
> I think this only makes sense when jumbo_frame is 1, although not hurts
> to set...

Yes, that is true. But, I though it would be better for debugging 
purpose. Does it hurt keeping it?

> 
>> +	return 0;
>> +}
> <...>
> 
>
  
Ferruh Yigit June 29, 2017, 3:43 p.m. UTC | #3
On 6/29/2017 3:56 PM, Shreyansh Jain wrote:
> On Wednesday 28 June 2017 09:15 PM, Ferruh Yigit wrote:
>> On 6/16/2017 6:40 AM, Shreyansh Jain wrote:
>>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>

<...>

>>> +	dev->data->dev_conf.rxmode.jumbo_frame = 0;
>>> +
>>> +	dev->data->dev_conf.rxmode.max_rx_pkt_len = mtu;
>>
>> I think this only makes sense when jumbo_frame is 1, although not hurts
>> to set...
> 
> Yes, that is true. But, I though it would be better for debugging 
> purpose. Does it hurt keeping it?

It is OK, specially since you are updating the code in jumbo frame patch
to use this variable..

> 
>>
>>> +	return 0;
>>> +}
>> <...>
>>
>>
>
  

Patch

diff --git a/doc/guides/nics/features/dpaa.ini b/doc/guides/nics/features/dpaa.ini
index 29ba47e..0b992fd 100644
--- a/doc/guides/nics/features/dpaa.ini
+++ b/doc/guides/nics/features/dpaa.ini
@@ -5,5 +5,6 @@ 
 ;
 [Features]
 Queue start/stop     = Y
+MTU update           = Y
 ARMv8                = Y
 Usage doc            = Y
diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
index 5a8d8af..6d33ff8 100644
--- a/drivers/net/dpaa/dpaa_ethdev.c
+++ b/drivers/net/dpaa/dpaa_ethdev.c
@@ -76,6 +76,26 @@ 
 static int is_global_init;
 
 static int
+dpaa_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
+{
+	struct dpaa_if *dpaa_intf = dev->data->dev_private;
+
+	PMD_INIT_FUNC_TRACE();
+
+	if (mtu < ETHER_MIN_MTU)
+		return -EINVAL;
+
+	fman_if_set_maxfrm(dpaa_intf->fif, mtu);
+
+	if (mtu > ETHER_MAX_LEN)
+		return -1
+	dev->data->dev_conf.rxmode.jumbo_frame = 0;
+
+	dev->data->dev_conf.rxmode.max_rx_pkt_len = mtu;
+	return 0;
+}
+
+static int
 dpaa_eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
 {
 	PMD_INIT_FUNC_TRACE();
@@ -198,6 +218,7 @@  static struct eth_dev_ops dpaa_devops = {
 	.tx_queue_setup		  = dpaa_eth_tx_queue_setup,
 	.rx_queue_release	  = dpaa_eth_rx_queue_release,
 	.tx_queue_release	  = dpaa_eth_tx_queue_release,
+	.mtu_set		  = dpaa_mtu_set,
 };
 
 /* Initialise an Rx FQ */