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

["Min, JiaqiX" <jiaqix.min@intel.com>]

Hi, Xiaolong

> -----Original Message-----
> From: Ye, Xiaolong
> Sent: Tuesday, October 29, 2019 1:07 PM
> To: Min, JiaqiX <jiaqix.min@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] net/ice: fix setting max frame size
> 
> 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?
Agree, we should  set a default max frame size value to HW.

> 
> And since i40e doesn't update max frame size to HW in start ops, do you
> think it needs a fix?
It can't fix the issue to set max frame size to HW in start ops  in i40e, I don't know the cause.
It is ok to set max frame size to HW in link_update ops  in i40e, 
however when user opens port,  link_update ops will be called three times, 
so I don't think it makes sense to set max frame size to hw in link_update ops too.
Where to set the maximum frame size to HW in i40e is a problem.

> 
> 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.