netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next] net: wangxun: Implement the ndo change mtu interface
@ 2023-02-16  8:44 Mengyuan Lou
  2023-02-16 13:11 ` Andrew Lunn
  2023-02-16 17:38 ` Alexander Lobakin
  0 siblings, 2 replies; 5+ messages in thread
From: Mengyuan Lou @ 2023-02-16  8:44 UTC (permalink / raw)
  To: netdev; +Cc: jiawenwu, Mengyuan Lou

Add ngbe and txgbe ndo_change_mtu support.

Signed-off-by: Mengyuan Lou <mengyuanlou@net-swift.com>
---
 drivers/net/ethernet/wangxun/libwx/wx_type.h  |  2 +
 drivers/net/ethernet/wangxun/ngbe/ngbe_main.c | 38 ++++++++++++++++++-
 drivers/net/ethernet/wangxun/ngbe/ngbe_type.h |  1 -
 .../net/ethernet/wangxun/txgbe/txgbe_main.c   | 38 ++++++++++++++++++-
 .../net/ethernet/wangxun/txgbe/txgbe_type.h   |  1 -
 5 files changed, 76 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/wangxun/libwx/wx_type.h b/drivers/net/ethernet/wangxun/libwx/wx_type.h
index 77d8d7f1707e..2b9efd13c500 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_type.h
+++ b/drivers/net/ethernet/wangxun/libwx/wx_type.h
@@ -300,6 +300,8 @@
 #define WX_MAX_RXD                   8192
 #define WX_MAX_TXD                   8192
 
+#define WX_MAX_JUMBO_FRAME_SIZE      9432 /* max payload 9414 */
+
 /* Supported Rx Buffer Sizes */
 #define WX_RXBUFFER_256      256    /* Used for skb receive header */
 #define WX_RXBUFFER_2K       2048
diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
index 5b564d348c09..78bfaff02aad 100644
--- a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
+++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
@@ -361,6 +361,15 @@ static void ngbe_up(struct wx *wx)
 	phy_start(wx->phydev);
 }
 
+static void ngbe_reinit_locked(struct wx *wx)
+{
+	/* prevent tx timeout */
+	netif_trans_update(wx->netdev);
+	ngbe_down(wx);
+	wx_configure(wx);
+	ngbe_up(wx);
+}
+
 /**
  * ngbe_open - Called when a network interface is made active
  * @netdev: network interface device structure
@@ -435,6 +444,32 @@ static int ngbe_close(struct net_device *netdev)
 	return 0;
 }
 
+/**
+ * ngbe_change_mtu - Change the Maximum Transfer Unit
+ * @netdev: network interface device structure
+ * @new_mtu: new value for maximum frame size
+ *
+ * Returns 0 on success, negative on failure
+ **/
+static int ngbe_change_mtu(struct net_device *netdev, int new_mtu)
+{
+	int max_frame = new_mtu + ETH_HLEN + ETH_FCS_LEN;
+	struct wx *wx = netdev_priv(netdev);
+
+	if (max_frame > WX_MAX_JUMBO_FRAME_SIZE)
+		return -EINVAL;
+
+	netdev_info(netdev, "Changing MTU from %d to %d.\n",
+		    netdev->mtu, new_mtu);
+
+	/* must set new MTU before calling down or up */
+	netdev->mtu = new_mtu;
+	if (netif_running(netdev))
+		ngbe_reinit_locked(wx);
+
+	return 0;
+}
+
 static void ngbe_dev_shutdown(struct pci_dev *pdev, bool *enable_wake)
 {
 	struct wx *wx = pci_get_drvdata(pdev);
@@ -470,6 +505,7 @@ static void ngbe_shutdown(struct pci_dev *pdev)
 static const struct net_device_ops ngbe_netdev_ops = {
 	.ndo_open               = ngbe_open,
 	.ndo_stop               = ngbe_close,
+	.ndo_change_mtu         = ngbe_change_mtu,
 	.ndo_start_xmit         = wx_xmit_frame,
 	.ndo_set_rx_mode        = wx_set_rx_mode,
 	.ndo_validate_addr      = eth_validate_addr,
@@ -562,7 +598,7 @@ static int ngbe_probe(struct pci_dev *pdev,
 	netdev->priv_flags |= IFF_SUPP_NOFCS;
 
 	netdev->min_mtu = ETH_MIN_MTU;
-	netdev->max_mtu = NGBE_MAX_JUMBO_FRAME_SIZE - (ETH_HLEN + ETH_FCS_LEN);
+	netdev->max_mtu = WX_MAX_JUMBO_FRAME_SIZE - (ETH_HLEN + ETH_FCS_LEN);
 
 	wx->bd_number = func_nums;
 	/* setup the private structure */
diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h b/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
index a2351349785e..373d5af628cd 100644
--- a/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
+++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
@@ -137,7 +137,6 @@ enum NGBE_MSCA_CMD_value {
 #define NGBE_RX_PB_SIZE				42
 #define NGBE_MC_TBL_SIZE			128
 #define NGBE_TDB_PB_SZ				(20 * 1024) /* 160KB Packet Buffer */
-#define NGBE_MAX_JUMBO_FRAME_SIZE		9432 /* max payload 9414 */
 
 /* TX/RX descriptor defines */
 #define NGBE_DEFAULT_TXD			512 /* default ring size */
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
index 6c0a98230557..0b09f982a2c8 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
@@ -454,6 +454,41 @@ static int txgbe_close(struct net_device *netdev)
 	return 0;
 }
 
+static void txgbe_reinit_locked(struct wx *wx)
+{
+	/* prevent tx timeout */
+	netif_trans_update(wx->netdev);
+	txgbe_down(wx);
+	wx_configure(wx);
+	txgbe_up_complete(wx);
+}
+
+/**
+ * txgbe_change_mtu - Change the Maximum Transfer Unit
+ * @netdev: network interface device structure
+ * @new_mtu: new value for maximum frame size
+ *
+ * Returns 0 on success, negative on failure
+ **/
+static int txgbe_change_mtu(struct net_device *netdev, int new_mtu)
+{
+	int max_frame = new_mtu + ETH_HLEN + ETH_FCS_LEN;
+	struct wx *wx = netdev_priv(netdev);
+
+	if (max_frame > WX_MAX_JUMBO_FRAME_SIZE)
+		return -EINVAL;
+
+	netdev_info(netdev, "Changing MTU from %d to %d.\n",
+		    netdev->mtu, new_mtu);
+
+	/* must set new MTU before calling down or up */
+	netdev->mtu = new_mtu;
+	if (netif_running(netdev))
+		txgbe_reinit_locked(wx);
+
+	return 0;
+}
+
 static void txgbe_dev_shutdown(struct pci_dev *pdev, bool *enable_wake)
 {
 	struct wx *wx = pci_get_drvdata(pdev);
@@ -487,6 +522,7 @@ static void txgbe_shutdown(struct pci_dev *pdev)
 static const struct net_device_ops txgbe_netdev_ops = {
 	.ndo_open               = txgbe_open,
 	.ndo_stop               = txgbe_close,
+	.ndo_change_mtu         = txgbe_change_mtu,
 	.ndo_start_xmit         = wx_xmit_frame,
 	.ndo_set_rx_mode        = wx_set_rx_mode,
 	.ndo_validate_addr      = eth_validate_addr,
@@ -605,7 +641,7 @@ static int txgbe_probe(struct pci_dev *pdev,
 	netdev->priv_flags |= IFF_SUPP_NOFCS;
 
 	netdev->min_mtu = ETH_MIN_MTU;
-	netdev->max_mtu = TXGBE_MAX_JUMBO_FRAME_SIZE - (ETH_HLEN + ETH_FCS_LEN);
+	netdev->max_mtu = WX_MAX_JUMBO_FRAME_SIZE - (ETH_HLEN + ETH_FCS_LEN);
 
 	/* make sure the EEPROM is good */
 	err = txgbe_validate_eeprom_checksum(wx, NULL);
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
index 563ea51deca6..63a1c733718d 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
@@ -79,7 +79,6 @@
 #define TXGBE_SP_MC_TBL_SIZE    128
 #define TXGBE_SP_RX_PB_SIZE     512
 #define TXGBE_SP_TDB_PB_SZ      (160 * 1024) /* 160KB Packet Buffer */
-#define TXGBE_MAX_JUMBO_FRAME_SIZE      9432 /* max payload 9414 */
 
 /* TX/RX descriptor defines */
 #define TXGBE_DEFAULT_TXD               512
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next] net: wangxun: Implement the ndo change mtu interface
  2023-02-16  8:44 [PATCH net-next] net: wangxun: Implement the ndo change mtu interface Mengyuan Lou
@ 2023-02-16 13:11 ` Andrew Lunn
  2023-02-16 17:38 ` Alexander Lobakin
  1 sibling, 0 replies; 5+ messages in thread
From: Andrew Lunn @ 2023-02-16 13:11 UTC (permalink / raw)
  To: Mengyuan Lou; +Cc: netdev, jiawenwu

> +static int ngbe_change_mtu(struct net_device *netdev, int new_mtu)
> +{
> +	int max_frame = new_mtu + ETH_HLEN + ETH_FCS_LEN;
> +	struct wx *wx = netdev_priv(netdev);
> +
> +	if (max_frame > WX_MAX_JUMBO_FRAME_SIZE)
> +		return -EINVAL;

You set netdev->max_mtu. So was it possible to actually make this
happen?

> +
> +	netdev_info(netdev, "Changing MTU from %d to %d.\n",
> +		    netdev->mtu, new_mtu);

netdev_dbg().

> +static void txgbe_reinit_locked(struct wx *wx)
> +{
> +	/* prevent tx timeout */
> +	netif_trans_update(wx->netdev);
> +	txgbe_down(wx);
> +	wx_configure(wx);
> +	txgbe_up_complete(wx);
> +}

None of these can fail? None allocate memory, etc?

> +static int txgbe_change_mtu(struct net_device *netdev, int new_mtu)
> +{
> +	int max_frame = new_mtu + ETH_HLEN + ETH_FCS_LEN;
> +	struct wx *wx = netdev_priv(netdev);
> +
> +	if (max_frame > WX_MAX_JUMBO_FRAME_SIZE)
> +		return -EINVAL;
> +
> +	netdev_info(netdev, "Changing MTU from %d to %d.\n",
> +		    netdev->mtu, new_mtu);

Same two comments as above.

     Andrew

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next] net: wangxun: Implement the ndo change mtu interface
  2023-02-16  8:44 [PATCH net-next] net: wangxun: Implement the ndo change mtu interface Mengyuan Lou
  2023-02-16 13:11 ` Andrew Lunn
@ 2023-02-16 17:38 ` Alexander Lobakin
  2023-02-20  7:49   ` mengyuanlou
  1 sibling, 1 reply; 5+ messages in thread
From: Alexander Lobakin @ 2023-02-16 17:38 UTC (permalink / raw)
  To: Mengyuan Lou; +Cc: netdev, jiawenwu, Andrew Lunn

From: Mengyuan Lou <mengyuanlou@net-swift.com>
Date: Thu, 16 Feb 2023 16:44:13 +0800

> Add ngbe and txgbe ndo_change_mtu support.
> 
> Signed-off-by: Mengyuan Lou <mengyuanlou@net-swift.com>
> ---
>  drivers/net/ethernet/wangxun/libwx/wx_type.h  |  2 +
>  drivers/net/ethernet/wangxun/ngbe/ngbe_main.c | 38 ++++++++++++++++++-
>  drivers/net/ethernet/wangxun/ngbe/ngbe_type.h |  1 -
>  .../net/ethernet/wangxun/txgbe/txgbe_main.c   | 38 ++++++++++++++++++-
>  .../net/ethernet/wangxun/txgbe/txgbe_type.h   |  1 -
>  5 files changed, 76 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_type.h b/drivers/net/ethernet/wangxun/libwx/wx_type.h
> index 77d8d7f1707e..2b9efd13c500 100644
> --- a/drivers/net/ethernet/wangxun/libwx/wx_type.h
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_type.h
> @@ -300,6 +300,8 @@
>  #define WX_MAX_RXD                   8192
>  #define WX_MAX_TXD                   8192
>  
> +#define WX_MAX_JUMBO_FRAME_SIZE      9432 /* max payload 9414 */

Please use tabs.

> +
>  /* Supported Rx Buffer Sizes */
>  #define WX_RXBUFFER_256      256    /* Used for skb receive header */
>  #define WX_RXBUFFER_2K       2048
> diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
> index 5b564d348c09..78bfaff02aad 100644
> --- a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
> +++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
> @@ -361,6 +361,15 @@ static void ngbe_up(struct wx *wx)
>  	phy_start(wx->phydev);
>  }
>  
> +static void ngbe_reinit_locked(struct wx *wx)
> +{
> +	/* prevent tx timeout */
> +	netif_trans_update(wx->netdev);

Why doing this? Your driver/device can reload for longer than 5 seconds
(default Tx timeout) or...?

> +	ngbe_down(wx);
> +	wx_configure(wx);
> +	ngbe_up(wx);
> +}
> +
>  /**
>   * ngbe_open - Called when a network interface is made active
>   * @netdev: network interface device structure
> @@ -435,6 +444,32 @@ static int ngbe_close(struct net_device *netdev)
>  	return 0;
>  }
>  
> +/**
> + * ngbe_change_mtu - Change the Maximum Transfer Unit
> + * @netdev: network interface device structure
> + * @new_mtu: new value for maximum frame size
> + *
> + * Returns 0 on success, negative on failure
> + **/
> +static int ngbe_change_mtu(struct net_device *netdev, int new_mtu)
> +{
> +	int max_frame = new_mtu + ETH_HLEN + ETH_FCS_LEN;

You must also account `2 * VLAN_HLEN`. The difference between MTU and
frame size is `ETH_HLEN + 2 * VLAN_HLEN + ETH_FCS_LEN`, i.e. 26 bytes.
...except for if your device doesn't handle VLANs, but I doubt so.

> +	struct wx *wx = netdev_priv(netdev);
> +
> +	if (max_frame > WX_MAX_JUMBO_FRAME_SIZE)
> +		return -EINVAL;

(Andrew already said that...)

> +
> +	netdev_info(netdev, "Changing MTU from %d to %d.\n",
> +		    netdev->mtu, new_mtu);

As Andrew already said, it's netdev_dbg() at most, but TBH I consider
this a development-time-only-debug-message that shouldn't go into the
release code.

> +
> +	/* must set new MTU before calling down or up */
> +	netdev->mtu = new_mtu;

If you look at the default implementation, you'll see that netdev->mtu
must now be accessed using READ_ONCE()/WRITE_ONCE(), so please change
accordingly. Otherwise there can be race around this field and you'll
get some unexpected results some day.

> +	if (netif_running(netdev))
> +		ngbe_reinit_locked(wx);
> +
> +	return 0;
> +}
> +
>  static void ngbe_dev_shutdown(struct pci_dev *pdev, bool *enable_wake)
>  {
>  	struct wx *wx = pci_get_drvdata(pdev);
> @@ -470,6 +505,7 @@ static void ngbe_shutdown(struct pci_dev *pdev)
>  static const struct net_device_ops ngbe_netdev_ops = {
>  	.ndo_open               = ngbe_open,
>  	.ndo_stop               = ngbe_close,
> +	.ndo_change_mtu         = ngbe_change_mtu,
>  	.ndo_start_xmit         = wx_xmit_frame,
>  	.ndo_set_rx_mode        = wx_set_rx_mode,
>  	.ndo_validate_addr      = eth_validate_addr,
> @@ -562,7 +598,7 @@ static int ngbe_probe(struct pci_dev *pdev,
>  	netdev->priv_flags |= IFF_SUPP_NOFCS;
>  
>  	netdev->min_mtu = ETH_MIN_MTU;
> -	netdev->max_mtu = NGBE_MAX_JUMBO_FRAME_SIZE - (ETH_HLEN + ETH_FCS_LEN);
> +	netdev->max_mtu = WX_MAX_JUMBO_FRAME_SIZE - (ETH_HLEN + ETH_FCS_LEN);

Same regarding frame size vs MTU.
Also, these braces are redundant.

>  
>  	wx->bd_number = func_nums;
>  	/* setup the private structure */
> diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h b/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
> index a2351349785e..373d5af628cd 100644
> --- a/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
> +++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
> @@ -137,7 +137,6 @@ enum NGBE_MSCA_CMD_value {
>  #define NGBE_RX_PB_SIZE				42
>  #define NGBE_MC_TBL_SIZE			128
>  #define NGBE_TDB_PB_SZ				(20 * 1024) /* 160KB Packet Buffer */
> -#define NGBE_MAX_JUMBO_FRAME_SIZE		9432 /* max payload 9414 */
>  
>  /* TX/RX descriptor defines */
>  #define NGBE_DEFAULT_TXD			512 /* default ring size */
> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> index 6c0a98230557..0b09f982a2c8 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
For the second driver, the questions are the same.

Thanks,
Olek

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next] net: wangxun: Implement the ndo change mtu interface
  2023-02-16 17:38 ` Alexander Lobakin
@ 2023-02-20  7:49   ` mengyuanlou
  2023-02-24 15:57     ` Alexander Lobakin
  0 siblings, 1 reply; 5+ messages in thread
From: mengyuanlou @ 2023-02-20  7:49 UTC (permalink / raw)
  To: Alexander Lobakin; +Cc: netdev, Jiawen Wu, Andrew Lunn



> 2023年2月17日 01:38,Alexander Lobakin <aleksander.lobakin@intel.com> 写道:
> 
> From: Mengyuan Lou <mengyuanlou@net-swift.com>
> Date: Thu, 16 Feb 2023 16:44:13 +0800
> 
>> Add ngbe and txgbe ndo_change_mtu support.
>> 
>> Signed-off-by: Mengyuan Lou <mengyuanlou@net-swift.com>
>> ---
>> drivers/net/ethernet/wangxun/libwx/wx_type.h  |  2 +
>> drivers/net/ethernet/wangxun/ngbe/ngbe_main.c | 38 ++++++++++++++++++-
>> drivers/net/ethernet/wangxun/ngbe/ngbe_type.h |  1 -
>> .../net/ethernet/wangxun/txgbe/txgbe_main.c   | 38 ++++++++++++++++++-
>> .../net/ethernet/wangxun/txgbe/txgbe_type.h   |  1 -
>> 5 files changed, 76 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_type.h b/drivers/net/ethernet/wangxun/libwx/wx_type.h
>> index 77d8d7f1707e..2b9efd13c500 100644
>> --- a/drivers/net/ethernet/wangxun/libwx/wx_type.h
>> +++ b/drivers/net/ethernet/wangxun/libwx/wx_type.h
>> @@ -300,6 +300,8 @@
>> #define WX_MAX_RXD                   8192
>> #define WX_MAX_TXD                   8192
>> 
>> +#define WX_MAX_JUMBO_FRAME_SIZE      9432 /* max payload 9414 */
> 
> Please use tabs.
> 
>> +
>> /* Supported Rx Buffer Sizes */
>> #define WX_RXBUFFER_256      256    /* Used for skb receive header */
>> #define WX_RXBUFFER_2K       2048
>> diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
>> index 5b564d348c09..78bfaff02aad 100644
>> --- a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
>> +++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
>> @@ -361,6 +361,15 @@ static void ngbe_up(struct wx *wx)
>> phy_start(wx->phydev);
>> }
>> 
>> +static void ngbe_reinit_locked(struct wx *wx)
>> +{
>> + /* prevent tx timeout */
>> + netif_trans_update(wx->netdev);
> 
> Why doing this? Your driver/device can reload for longer than 5 seconds
> (default Tx timeout) or...?
> 
>> + ngbe_down(wx);
>> + wx_configure(wx);
>> + ngbe_up(wx);
>> +}
>> +
>> /**
>>  * ngbe_open - Called when a network interface is made active
>>  * @netdev: network interface device structure
>> @@ -435,6 +444,32 @@ static int ngbe_close(struct net_device *netdev)
>> return 0;
>> }
>> 
>> +/**
>> + * ngbe_change_mtu - Change the Maximum Transfer Unit
>> + * @netdev: network interface device structure
>> + * @new_mtu: new value for maximum frame size
>> + *
>> + * Returns 0 on success, negative on failure
>> + **/
>> +static int ngbe_change_mtu(struct net_device *netdev, int new_mtu)
>> +{
>> + int max_frame = new_mtu + ETH_HLEN + ETH_FCS_LEN;
> 
> You must also account `2 * VLAN_HLEN`. The difference between MTU and
> frame size is `ETH_HLEN + 2 * VLAN_HLEN + ETH_FCS_LEN`, i.e. 26 bytes.
> ...except for if your device doesn't handle VLANs, but I doubt so.

The code to support vlan has not been added, so VLAN_HLEN is not considered for now.
> 
>> + struct wx *wx = netdev_priv(netdev);
>> +
>> + if (max_frame > WX_MAX_JUMBO_FRAME_SIZE)
>> + return -EINVAL;
> 
> (Andrew already said that...)
> 
>> +
>> + netdev_info(netdev, "Changing MTU from %d to %d.\n",
>> +     netdev->mtu, new_mtu);
> 
> As Andrew already said, it's netdev_dbg() at most, but TBH I consider
> this a development-time-only-debug-message that shouldn't go into the
> release code.
> 
>> +
>> + /* must set new MTU before calling down or up */
>> + netdev->mtu = new_mtu;
> 
> If you look at the default implementation, you'll see that netdev->mtu
> must now be accessed using READ_ONCE()/WRITE_ONCE(), so please change
> accordingly. Otherwise there can be race around this field and you'll
> get some unexpected results some day.
> 
>> + if (netif_running(netdev))
>> + ngbe_reinit_locked(wx);
>> +
>> + return 0;
>> +}
>> +
>> static void ngbe_dev_shutdown(struct pci_dev *pdev, bool *enable_wake)
>> {
>> struct wx *wx = pci_get_drvdata(pdev);
>> @@ -470,6 +505,7 @@ static void ngbe_shutdown(struct pci_dev *pdev)
>> static const struct net_device_ops ngbe_netdev_ops = {
>> .ndo_open               = ngbe_open,
>> .ndo_stop               = ngbe_close,
>> + .ndo_change_mtu         = ngbe_change_mtu,
>> .ndo_start_xmit         = wx_xmit_frame,
>> .ndo_set_rx_mode        = wx_set_rx_mode,
>> .ndo_validate_addr      = eth_validate_addr,
>> @@ -562,7 +598,7 @@ static int ngbe_probe(struct pci_dev *pdev,
>> netdev->priv_flags |= IFF_SUPP_NOFCS;
>> 
>> netdev->min_mtu = ETH_MIN_MTU;
>> - netdev->max_mtu = NGBE_MAX_JUMBO_FRAME_SIZE - (ETH_HLEN + ETH_FCS_LEN);
>> + netdev->max_mtu = WX_MAX_JUMBO_FRAME_SIZE - (ETH_HLEN + ETH_FCS_LEN);
> 
> Same regarding frame size vs MTU.
> Also, these braces are redundant.
> 
>> 
>> wx->bd_number = func_nums;
>> /* setup the private structure */
>> diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h b/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
>> index a2351349785e..373d5af628cd 100644
>> --- a/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
>> +++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
>> @@ -137,7 +137,6 @@ enum NGBE_MSCA_CMD_value {
>> #define NGBE_RX_PB_SIZE 42
>> #define NGBE_MC_TBL_SIZE 128
>> #define NGBE_TDB_PB_SZ (20 * 1024) /* 160KB Packet Buffer */
>> -#define NGBE_MAX_JUMBO_FRAME_SIZE 9432 /* max payload 9414 */
>> 
>> /* TX/RX descriptor defines */
>> #define NGBE_DEFAULT_TXD 512 /* default ring size */
>> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
>> index 6c0a98230557..0b09f982a2c8 100644
>> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
>> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> For the second driver, the questions are the same.
> 
> Thanks,
> Olek



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next] net: wangxun: Implement the ndo change mtu interface
  2023-02-20  7:49   ` mengyuanlou
@ 2023-02-24 15:57     ` Alexander Lobakin
  0 siblings, 0 replies; 5+ messages in thread
From: Alexander Lobakin @ 2023-02-24 15:57 UTC (permalink / raw)
  To: mengyuanlou@net-swift.com; +Cc: netdev, Jiawen Wu, Andrew Lunn

From: Mengyuan Lou <mengyuanlou@net-swift.com>
Date: Mon, 20 Feb 2023 15:49:35 +0800

> 
> 
>> 2023年2月17日 01:38,Alexander Lobakin <aleksander.lobakin@intel.com> 写道:
>>
>> From: Mengyuan Lou <mengyuanlou@net-swift.com>
>> Date: Thu, 16 Feb 2023 16:44:13 +0800

[...]

>>> +static int ngbe_change_mtu(struct net_device *netdev, int new_mtu)
>>> +{
>>> + int max_frame = new_mtu + ETH_HLEN + ETH_FCS_LEN;
>>
>> You must also account `2 * VLAN_HLEN`. The difference between MTU and
>> frame size is `ETH_HLEN + 2 * VLAN_HLEN + ETH_FCS_LEN`, i.e. 26 bytes.
>> ...except for if your device doesn't handle VLANs, but I doubt so.
> 
> The code to support vlan has not been added, so VLAN_HLEN is not considered for now.

What do you mean by "code to support vlan"? You want to say that without
some code, your NIC/driver doesn't receive VLAN-tagged packets at all or
what? VLANs can be handled purely by software if you don't configure
anything on a device. Unless you have some strange HW.

>>
>>> + struct wx *wx = netdev_priv(netdev);
>>> +
>>> + if (max_frame > WX_MAX_JUMBO_FRAME_SIZE)
>>> + return -EINVAL;
>>
>> (Andrew already said that...)
>>
>>> +
>>> + netdev_info(netdev, "Changing MTU from %d to %d.\n",
>>> +     netdev->mtu, new_mtu);
[...]

Thanks,
Olek

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-02-24 15:58 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-16  8:44 [PATCH net-next] net: wangxun: Implement the ndo change mtu interface Mengyuan Lou
2023-02-16 13:11 ` Andrew Lunn
2023-02-16 17:38 ` Alexander Lobakin
2023-02-20  7:49   ` mengyuanlou
2023-02-24 15:57     ` Alexander Lobakin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).