[PATCH net-next] net: hns3: Add support to change MTU in hardware & netdev

[PATCH net-next] net: hns3: Add support to change MTU in hardware & netdev

[Salil Mehta]

This patch adds the following support to the HNS3 driver:
1. Support to change the Maximum Transmission Unit of a
   netdevice and of a port in hardware .
2. Initializes the supported MTU range for the netdevice.

Signed-off-by: lipeng <lipeng321-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Salil Mehta <salil.mehta-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 .../net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c | 46 ++++++++++++++++++++++
 .../net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h |  1 +
 2 files changed, 47 insertions(+)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
index e731f87..8e3711e 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
@@ -1278,11 +1278,53 @@ static int hns3_ndo_set_vf_vlan(struct net_device *netdev, int vf, u16 vlan,
 	return ret;
 }
 
+static int hns3_nic_change_mtu(struct net_device *netdev, int new_mtu)
+{
+	struct hns3_nic_priv *priv = netdev_priv(netdev);
+	struct hnae3_handle *h = priv->ae_handle;
+	bool if_running = netif_running(netdev);
+	int ret;
+
+	/* no change in MTU */
+	if (new_mtu == netdev->mtu)
+		return 0;
+
+	if (!h->ae_algo->ops->set_mtu)
+		return -ENOTSUPP;
+
+	/* if this was called with netdev up then bring netdevice down */
+	if (if_running) {
+		(void)hns3_nic_net_stop(netdev);
+		msleep(100);
+	}
+
+	ret = h->ae_algo->ops->set_mtu(h, new_mtu);
+	if (ret) {
+		netdev_err(netdev, "failed to change MTU in hardware %d\n",
+			   ret);
+		return ret;
+	}
+
+	/* assign newly changed mtu to netdevice as well */
+	netdev->mtu = new_mtu;
+
+	/* if the netdev was running earlier, bring it up again */
+	if (if_running) {
+		if (hns3_nic_net_open(netdev)) {
+			netdev_err(netdev, "MTU, couldnt up netdev again\n");
+			ret = -EINVAL;
+		}
+	}
+
+	return ret;
+}
+
 static const struct net_device_ops hns3_nic_netdev_ops = {
 	.ndo_open		= hns3_nic_net_open,
 	.ndo_stop		= hns3_nic_net_stop,
 	.ndo_start_xmit		= hns3_nic_net_xmit,
 	.ndo_set_mac_address	= hns3_nic_net_set_mac_address,
+	.ndo_change_mtu		= hns3_nic_change_mtu,
 	.ndo_set_features	= hns3_nic_set_features,
 	.ndo_get_stats64	= hns3_nic_get_stats64,
 	.ndo_setup_tc		= hns3_nic_setup_tc,
@@ -2752,6 +2794,10 @@ static int hns3_client_init(struct hnae3_handle *handle)
 		goto out_reg_netdev_fail;
 	}
 
+	/* MTU range: 68 - 9706 */
+	netdev->min_mtu = ETH_MIN_MTU;
+	netdev->max_mtu = HNS3_MAX_MTU - (ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN);
+
 	return ret;
 
 out_reg_netdev_fail:
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h
index a6e8f15..7e87461 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h
@@ -76,6 +76,7 @@ enum hns3_nic_state {
 #define HNS3_RING_NAME_LEN			16
 #define HNS3_BUFFER_SIZE_2048			2048
 #define HNS3_RING_MAX_PENDING			32768
+#define HNS3_MAX_MTU				9728
 
 #define HNS3_BD_SIZE_512_TYPE			0
 #define HNS3_BD_SIZE_1024_TYPE			1
-- 
2.7.4


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next] net: hns3: Add support to change MTU in hardware & netdev

[Andrew Lunn]

On Fri, Aug 18, 2017 at 12:35:58PM +0100, Salil Mehta wrote:
> This patch adds the following support to the HNS3 driver:
> 1. Support to change the Maximum Transmission Unit of a
>    netdevice and of a port in hardware .
> 2. Initializes the supported MTU range for the netdevice.
> 
> Signed-off-by: lipeng <lipeng321@huawei.com>
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> ---
>  .../net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c | 46 ++++++++++++++++++++++
>  .../net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h |  1 +
>  2 files changed, 47 insertions(+)
> 
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
> index e731f87..8e3711e 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
> @@ -1278,11 +1278,53 @@ static int hns3_ndo_set_vf_vlan(struct net_device *netdev, int vf, u16 vlan,
>  	return ret;
>  }
>  
> +static int hns3_nic_change_mtu(struct net_device *netdev, int new_mtu)
> +{
> +	struct hns3_nic_priv *priv = netdev_priv(netdev);
> +	struct hnae3_handle *h = priv->ae_handle;
> +	bool if_running = netif_running(netdev);
> +	int ret;
> +
> +	/* no change in MTU */
> +	if (new_mtu == netdev->mtu)
> +		return 0;

Hi Salil

It is worth reading the core code:

http://elixir.free-electrons.com/linux/latest/source/net/core/dev.c#L6713

 +
> +	if (!h->ae_algo->ops->set_mtu)
> +		return -ENOTSUPP;
> +
> +	/* if this was called with netdev up then bring netdevice down */
> +	if (if_running) {
> +		(void)hns3_nic_net_stop(netdev);
> +		msleep(100);
> +	}
> +
> +	ret = h->ae_algo->ops->set_mtu(h, new_mtu);
> +	if (ret) {
> +		netdev_err(netdev, "failed to change MTU in hardware %d\n",
> +			   ret);
> +		return ret;
> +	}
> +
> +	/* assign newly changed mtu to netdevice as well */
> +	netdev->mtu = new_mtu;

http://elixir.free-electrons.com/linux/latest/source/net/core/dev.c#L6698

> +
> +	/* if the netdev was running earlier, bring it up again */
> +	if (if_running) {
> +		if (hns3_nic_net_open(netdev)) {
> +			netdev_err(netdev, "MTU, couldnt up netdev again\n");
> +			ret = -EINVAL;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
>  static const struct net_device_ops hns3_nic_netdev_ops = {
>  	.ndo_open		= hns3_nic_net_open,
>  	.ndo_stop		= hns3_nic_net_stop,
>  	.ndo_start_xmit		= hns3_nic_net_xmit,
>  	.ndo_set_mac_address	= hns3_nic_net_set_mac_address,
> +	.ndo_change_mtu		= hns3_nic_change_mtu,
>  	.ndo_set_features	= hns3_nic_set_features,
>  	.ndo_get_stats64	= hns3_nic_get_stats64,
>  	.ndo_setup_tc		= hns3_nic_setup_tc,
> @@ -2752,6 +2794,10 @@ static int hns3_client_init(struct hnae3_handle *handle)
>  		goto out_reg_netdev_fail;
>  	}
>  
> +	/* MTU range: 68 - 9706 */
> +	netdev->min_mtu = ETH_MIN_MTU;

http://elixir.free-electrons.com/linux/latest/source/net/ethernet/eth.c#L361

> +	netdev->max_mtu = HNS3_MAX_MTU - (ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN);
> +
>  	return ret;
>  
>  out_reg_netdev_fail:
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h
> index a6e8f15..7e87461 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h
> @@ -76,6 +76,7 @@ enum hns3_nic_state {
>  #define HNS3_RING_NAME_LEN			16
>  #define HNS3_BUFFER_SIZE_2048			2048
>  #define HNS3_RING_MAX_PENDING			32768
> +#define HNS3_MAX_MTU				9728

It seems odd that it does not already exists somewhere. The core does
not enforce the MTU.  You could be passed a frame which is bigger. So
you should check before trying to pass something to the hardware which
the hardware cannot handle.

    Andrew

RE: [PATCH net-next] net: hns3: Add support to change MTU in hardware & netdev

[Salil Mehta]

Hi Andrew

> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Friday, August 18, 2017 2:31 PM
> To: Salil Mehta
> Cc: davem@davemloft.net; Zhuangyuzeng (Yisen); lipeng (Y);
> dan.carpenter@oracle.com; mehta.salil.lnk@gmail.com;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> rdma@vger.kernel.org; Linuxarm
> Subject: Re: [PATCH net-next] net: hns3: Add support to change MTU in
> hardware & netdev
> 
> On Fri, Aug 18, 2017 at 12:35:58PM +0100, Salil Mehta wrote:
> > This patch adds the following support to the HNS3 driver:
> > 1. Support to change the Maximum Transmission Unit of a
> >    netdevice and of a port in hardware .
> > 2. Initializes the supported MTU range for the netdevice.
> >
> > Signed-off-by: lipeng <lipeng321@huawei.com>
> > Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> > ---
> >  .../net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c | 46
> ++++++++++++++++++++++
> >  .../net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h |  1 +
> >  2 files changed, 47 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
> b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
> > index e731f87..8e3711e 100644
> > --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
> > +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
> > @@ -1278,11 +1278,53 @@ static int hns3_ndo_set_vf_vlan(struct
> net_device *netdev, int vf, u16 vlan,
> >  	return ret;
> >  }
> >
> > +static int hns3_nic_change_mtu(struct net_device *netdev, int
> new_mtu)
> > +{
> > +	struct hns3_nic_priv *priv = netdev_priv(netdev);
> > +	struct hnae3_handle *h = priv->ae_handle;
> > +	bool if_running = netif_running(netdev);
> > +	int ret;
> > +
> > +	/* no change in MTU */
> > +	if (new_mtu == netdev->mtu)
> > +		return 0;
> 
> Hi Salil
> 
> It is worth reading the core code:
> 
> http://elixir.free-
> electrons.com/linux/latest/source/net/core/dev.c#L6713
Right. Looks like it is already being done since 4.10-rc1 because of
the patches Floated by Jarod Wilson <jarod@redhat.com>.
Link: https://lkml.org/lkml/2016/10/13/270

Will remove this duplicate check!

Thanks
Salil 
>  +
> > +	if (!h->ae_algo->ops->set_mtu)
> > +		return -ENOTSUPP;
> > +
> > +	/* if this was called with netdev up then bring netdevice down */
> > +	if (if_running) {
> > +		(void)hns3_nic_net_stop(netdev);
> > +		msleep(100);
> > +	}
> > +
> > +	ret = h->ae_algo->ops->set_mtu(h, new_mtu);
> > +	if (ret) {
> > +		netdev_err(netdev, "failed to change MTU in hardware %d\n",
> > +			   ret);
> > +		return ret;
> > +	}
> > +
> > +	/* assign newly changed mtu to netdevice as well */
> > +	netdev->mtu = new_mtu;
> 
> http://elixir.free-
> electrons.com/linux/latest/source/net/core/dev.c#L6698
Again, it is being done by core now since 4.10-rc1.
Link: https://lkml.org/lkml/2016/10/13/270

Will remove this duplicate code!

Thanks
Salil
> 
> > +
> > +	/* if the netdev was running earlier, bring it up again */
> > +	if (if_running) {
> > +		if (hns3_nic_net_open(netdev)) {
> > +			netdev_err(netdev, "MTU, couldnt up netdev again\n");
> > +			ret = -EINVAL;
> > +		}
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> >  static const struct net_device_ops hns3_nic_netdev_ops = {
> >  	.ndo_open		= hns3_nic_net_open,
> >  	.ndo_stop		= hns3_nic_net_stop,
> >  	.ndo_start_xmit		= hns3_nic_net_xmit,
> >  	.ndo_set_mac_address	= hns3_nic_net_set_mac_address,
> > +	.ndo_change_mtu		= hns3_nic_change_mtu,
> >  	.ndo_set_features	= hns3_nic_set_features,
> >  	.ndo_get_stats64	= hns3_nic_get_stats64,
> >  	.ndo_setup_tc		= hns3_nic_setup_tc,
> > @@ -2752,6 +2794,10 @@ static int hns3_client_init(struct
> hnae3_handle *handle)
> >  		goto out_reg_netdev_fail;
> >  	}
> >
> > +	/* MTU range: 68 - 9706 */
> > +	netdev->min_mtu = ETH_MIN_MTU;
> 
> http://elixir.free-
> electrons.com/linux/latest/source/net/ethernet/eth.c#L361
Supported range of Min and Max MTU should be at the discretion
of the driver. Therefore, initialization looks fine to me.

I could not clearly understand the problem being highlighted
over here. Could you further clarify?

Thanks
Salil
> 
> > +	netdev->max_mtu = HNS3_MAX_MTU - (ETH_HLEN + ETH_FCS_LEN +
> VLAN_HLEN);
> > +
> >  	return ret;
> >
> >  out_reg_netdev_fail:
> > diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h
> b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h
> > index a6e8f15..7e87461 100644
> > --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h
> > +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h
> > @@ -76,6 +76,7 @@ enum hns3_nic_state {
> >  #define HNS3_RING_NAME_LEN			16
> >  #define HNS3_BUFFER_SIZE_2048			2048
> >  #define HNS3_RING_MAX_PENDING			32768
> > +#define HNS3_MAX_MTU				9728
> 
> It seems odd that it does not already exists somewhere. The core does
> not enforce the MTU.  You could be passed a frame which is bigger. So
> you should check before trying to pass something to the hardware which
> the hardware cannot handle.
There is a check already in place for this as well since 4.10-rc1.
But perhaps this time there is no change required as it is being taken
care by the core. Thanks to sharp eyes of Jarod.
Link: https://lkml.org/lkml/2016/10/13/270

[...]
if (dev->max_mtu > 0 && new_mtu > dev->max_mtu) {
	net_err_ratelimited("%s: Invalid MTU %d requested, hw max %d\n",
			    dev->name, new_mtu, dev->max_mtu);
[...]

I think I missed this patch entirely so those rest above checks &
assignments were repeated. 

Thanks
Salil
> 
>     Andrew

Re: [PATCH net-next] net: hns3: Add support to change MTU in hardware & netdev

[Andrew Lunn]

> > > +	/* MTU range: 68 - 9706 */
> > > +	netdev->min_mtu = ETH_MIN_MTU;
> > 
> > http://elixir.free-
> > electrons.com/linux/latest/source/net/ethernet/eth.c#L361
> Supported range of Min and Max MTU should be at the discretion
> of the driver. Therefore, initialization looks fine to me.
> 
> I could not clearly understand the problem being highlighted
> over here. Could you further clarify?

This is already setting min_mtu to ETH_MIN_MTU. There is no need for
you to set it.

> > >  #define HNS3_RING_MAX_PENDING			32768
> > > +#define HNS3_MAX_MTU				9728
> > 
> > It seems odd that it does not already exists somewhere. The core does
> > not enforce the MTU.  You could be passed a frame which is bigger. So
> > you should check before trying to pass something to the hardware which
> > the hardware cannot handle.
> There is a check already in place for this as well since 4.10-rc1.

Yes, the core will check when changing the MTU.

But when passing frames to be transmitted, it does not check the frame
fits the MTU.  DSA actually makes use of this, when passing frames to
an Ethernet switch attached to the interface. We need to add an extra
header to the frame, which makes the frame bigger than the MTU. Most
Ethernet drivers are happy with this, they send the frame. Other
reject it, and we have had to make driver changes. And some just
explode :-(

If 9728 is a hard limit for your device, you should check when passed
a frame to make sure it is actually <= 9728 bytes in length.

     Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH net-next] net: hns3: Add support to change MTU in hardware & netdev

[Salil Mehta]

Hi Andrew

> -----Original Message-----
> From: Andrew Lunn [mailto:andrew-g2DYL2Zd6BY@public.gmane.org]
> Sent: Friday, August 18, 2017 4:04 PM
> To: Salil Mehta
> Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org; Zhuangyuzeng (Yisen); lipeng (Y);
> dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org; mehta.salil.lnk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org;
> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-
> rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Linuxarm
> Subject: Re: [PATCH net-next] net: hns3: Add support to change MTU in
> hardware & netdev
> 
> > > > +	/* MTU range: 68 - 9706 */
> > > > +	netdev->min_mtu = ETH_MIN_MTU;
> > >
> > > http://elixir.free-
> > > electrons.com/linux/latest/source/net/ethernet/eth.c#L361
> > Supported range of Min and Max MTU should be at the discretion
> > of the driver. Therefore, initialization looks fine to me.
> >
> > I could not clearly understand the problem being highlighted
> > over here. Could you further clarify?
> 
> This is already setting min_mtu to ETH_MIN_MTU. There is no need for
> you to set it.
I grep'ed entire code and could see min and max MTUs being set by the
Respective Ethernet driver code. I also verified by the original patch
floated by the Jarod where he did that Change across all the drivers
https://patchwork.kernel.org/patch/9387361/

for example,
file: drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 		netdev->priv_flags |= IFF_UNICAST_FLT;
 
+		/* MTU range: 81 - 9600 */
+		netdev->min_mtu = 81; 
+		netdev->max_mtu = MAX_MTU;

Many such changes are present in the above mentioned patch.
Hope I am not missing anything there?

Thanks
Salil
> 
> > > >  #define HNS3_RING_MAX_PENDING			32768
> > > > +#define HNS3_MAX_MTU				9728
> > >
> > > It seems odd that it does not already exists somewhere. The core
> does
> > > not enforce the MTU.  You could be passed a frame which is bigger.
> So
> > > you should check before trying to pass something to the hardware
> which
> > > the hardware cannot handle.
> > There is a check already in place for this as well since 4.10-rc1.
> 
> Yes, the core will check when changing the MTU.
> 
> But when passing frames to be transmitted, it does not check the frame
> fits the MTU.  DSA actually makes use of this, when passing frames to
> an Ethernet switch attached to the interface. We need to add an extra
> header to the frame, which makes the frame bigger than the MTU. Most
> Ethernet drivers are happy with this, they send the frame. Other
> reject it, and we have had to make driver changes. And some just
> explode :-(
I see. IMHO HNS3 is currently limited by maximum buffer per descriptor
which is 64k. I am sure such frames would get dropped in the hardware
itself and which I guess should be more preferable than dropping in
driver since it saves you some precious cpu cycles?

So I am not able to appreciate the presence of such a MTU check in
the live data-path. Maybe I am missing something here?

Thanks
Salil
> 
> If 9728 is a hard limit for your device, you should check when passed
> a frame to make sure it is actually <= 9728 bytes in length.
> 
>      Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next] net: hns3: Add support to change MTU in hardware & netdev

[Andrew Lunn]

> for example,
> file: drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  
>  		netdev->priv_flags |= IFF_UNICAST_FLT;
>  
> +		/* MTU range: 81 - 9600 */
> +		netdev->min_mtu = 81; 
> +		netdev->max_mtu = MAX_MTU;

In this cause, the driver is not using the default values. So it sets
them.

Anyway, try it. After your alloc_etherdev_mqs(), print the value of
min_mtu. It should already be set to MIN_ETH_MTU.

> I see. IMHO HNS3 is currently limited by maximum buffer per descriptor
> which is 64k. I am sure such frames would get dropped in the hardware
> itself and which I guess should be more preferable than dropping in
> driver since it saves you some precious cpu cycles?

If you hardware handles this, then you don't need to do anything.

   Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH net-next] net: hns3: Add support to change MTU in hardware & netdev

[Salil Mehta]

Hi Andrew

> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Friday, August 18, 2017 5:02 PM
> To: Salil Mehta
> Cc: davem@davemloft.net; Zhuangyuzeng (Yisen); lipeng (Y);
> mehta.salil.lnk@gmail.com; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-rdma@vger.kernel.org; Linuxarm
> Subject: Re: [PATCH net-next] net: hns3: Add support to change MTU in
> hardware & netdev
> 
> > for example,
> > file: drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> > static int init_one(struct pci_dev *pdev, const struct pci_device_id
> *ent)
> >
> >  		netdev->priv_flags |= IFF_UNICAST_FLT;
> >
> > +		/* MTU range: 81 - 9600 */
> > +		netdev->min_mtu = 81;
> > +		netdev->max_mtu = MAX_MTU;
> 
> In this cause, the driver is not using the default values. So it sets
> them.
> 
> Anyway, try it. After your alloc_etherdev_mqs(), print the value of
> min_mtu. It should already be set to MIN_ETH_MTU.
I understand your point. In this case, I would like to keep the
range being set by the driver just to be more explicit. 
So for now keep this initialization in the driver?

Thanks
Salil
> 
> > I see. IMHO HNS3 is currently limited by maximum buffer per
> descriptor
> > which is 64k. I am sure such frames would get dropped in the hardware
> > itself and which I guess should be more preferable than dropping in
> > driver since it saves you some precious cpu cycles?
> 
> If you hardware handles this, then you don't need to do anything.
Fine. Thanks!

Salil
> 
>    Andrew

RE: [PATCH net-next] net: hns3: Add support to change MTU in hardware & netdev

[Salil Mehta]

Hi Andrew,

> > -----Original Message-----
> > From: Andrew Lunn [mailto:andrew@lunn.ch]
> > Sent: Friday, August 18, 2017 5:02 PM
> > To: Salil Mehta
> > Cc: davem@davemloft.net; Zhuangyuzeng (Yisen); lipeng (Y);
> > mehta.salil.lnk@gmail.com; netdev@vger.kernel.org; linux-
> > kernel@vger.kernel.org; linux-rdma@vger.kernel.org; Linuxarm
> > Subject: Re: [PATCH net-next] net: hns3: Add support to change MTU in
> > hardware & netdev
> >
> > > for example,
> > > file: drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> > > static int init_one(struct pci_dev *pdev, const struct
> pci_device_id
> > *ent)
> > >
> > >  		netdev->priv_flags |= IFF_UNICAST_FLT;
> > >
> > > +		/* MTU range: 81 - 9600 */
> > > +		netdev->min_mtu = 81;
> > > +		netdev->max_mtu = MAX_MTU;
> >
> > In this cause, the driver is not using the default values. So it sets
> > them.
> >
> > Anyway, try it. After your alloc_etherdev_mqs(), print the value of
> > min_mtu. It should already be set to MIN_ETH_MTU.
> I understand your point. In this case, I would like to keep the
> range being set by the driver just to be more explicit.
> So for now keep this initialization in the driver?
Removed the min_mtu initialization from the driver code and added a
comment over it to be explicit. Please have a look at the V2 patch.

Thanks
Salil