Re: [dpdk-dev] [PATCH v2] net/ice: fix setting max frame size

[Ye Xiaolong <xiaolong.ye@intel.com>]

On 10/29, Min, JiaqiX wrote:
>Hi Xiaolong,
>
>> -----Original Message-----
>> From: Ye, Xiaolong
>> Sent: Monday, October 28, 2019 4:46 PM
>> To: Min, JiaqiX <jiaqix.min@intel.com>
>> Cc: dev@dpdk.org; stable@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH v2] net/ice: fix setting max frame size
>> 
>> On 10/28, Min JiaqiX wrote:
>> >Max frame size is not set to HW, so packets above the MTU do not get
>> >dropped by HW. The patch fixed the issue.
>> >
>> >Fixes: ae2bdd0219cb ("net/ice: support MTU setting")
>> 
>> We should set max frame size to HW in the beginning when the driver was
>> introduced, so the fix commit should be 50370662b727 ("net/ice: support
>> device and queue ops")
>Modify commit message in patch v3.
>> 
>> Fix this while merging.
>> 
>> >Cc: stable@dpdk.org
>> >
>> >Signed-off-by: Min JiaqiX <jiaqix.min@intel.com>
>> >---
>> >v2:
>> >* Changed commit message.
>> >---
>> > drivers/net/ice/ice_ethdev.c | 4 ++++
>> > 1 file changed, 4 insertions(+)
>> >
>> >diff --git a/drivers/net/ice/ice_ethdev.c
>> >b/drivers/net/ice/ice_ethdev.c index 022b58c01..403305cfb 100644
>> >--- a/drivers/net/ice/ice_ethdev.c
>> >+++ b/drivers/net/ice/ice_ethdev.c
>> >@@ -2506,6 +2506,10 @@ ice_dev_start(struct rte_eth_dev *dev)
>> >
>> > 	pf->adapter_stopped = false;
>> >
>> >+	/* Set the max frame size */
>> >+	ice_aq_set_mac_cfg(hw,
>> >+		pf->dev_data->dev_conf.rxmode.max_rx_pkt_len, NULL);
>> >+
>> 
>> Why set the max frame size to HW in start ops, not in init stage like in i40e?
>> and what if max_rx_pkt_len hasn't been set?
>> 
>max_rx_pkt_len need to be set to HW when set mtu ,  init stage won't be called when open port again.

Agree, I think it makes sense to set max frame size to hw in start ops, but what 
if user doesn't set mtu, then the max_rx_pkt_len would be 0, do we need a 
default max frame value in this case? 

And since i40e doesn't update max frame size to HW in start ops, do you think
it needs a fix?

Thanks,
Xiaolong


>
>Appendix:
>Steps for using Example/ethtool  to set mtu:
>EthApp>stop 0
>EthApp>mtu 0 200
>EthApp>open 0
>
>> 
>> Thanks,
>> Xiaolong
>> 
>> 
>> > 	return 0;
>> >
>> > 	/* stop the started queues if failed to start all queues */
>> >--
>> >2.17.1
>> >
>> 
>> Reviewed-by: Xiaolong Ye <xiaolong.ye@intel.com>
>> 
>> Applied to dpdk-next-net-intel. Thanks.