Skip to contentSkip to conversation

e1000e:Fix incorrect assumption that the function e1000e_up always runs successfully in e1000_change_mtu

PATCH In review Nicholas Krause 2 participants 3 messages Oct 24, 2015

Subsystems (3)
Networking Network drivers Intel Ethernet
lore
Share & tools
lore

Nicholas Krause

author
#0 lore raw

PATCH 1/1 e1000e:Fix incorrect assumption that the function e1000e_up always runs successfully in e1000_change_mtu

This fixes the function e1000_change_mtu to properly check and run the error code by e1000e_up as this function can fail and the error code should be returned to the caller of the function e1000_change_mtu to signal a error has occurred when calling this particular function

Signed-off-by Nicholas Krause <[email protected]>
---
 drivers/net/ethernet/intel/e1000e/netdev.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/net/ethernet/intel/e1000e/netdev.c +5 -2
index faf4b3f..aebccb1 100644
@@ -5931,6 +5931,7 @@ static int e1000_change_mtu(struct net_device *netdev, int new_mtu)
 {
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	int max_frame = new_mtu + VLAN_ETH_HLEN + ETH_FCS_LEN;
+	int err = 0;
 
 	/* Jumbo frame support */
 	if ((max_frame > (VLAN_ETH_FRAME_LEN + ETH_FCS_LEN)) &&

@@ -5984,7 +5985,9 @@ static int e1000_change_mtu(struct net_device *netdev, int new_mtu)
 		adapter->rx_buffer_len = VLAN_ETH_FRAME_LEN + ETH_FCS_LEN;
 
 	if (netif_running(netdev))
-		e1000e_up(adapter);
+		err = e1000e_up(adapter);
+		if (err)
+			e_err("Failed to successfully bring up this adapter\n");
 	else
 		e1000e_reset(adapter);
 

@@ -5992,7 +5995,7 @@ static int e1000_change_mtu(struct net_device *netdev, int new_mtu)
 
 	clear_bit(__E1000_RESETTING, &adapter->state);
 
-	return 0;
+	return err;
 }
 
 static int e1000_mii_ioctl(struct net_device *netdev, struct ifreq *ifr,
Signature · 1 line
2.1.4
#1 lore raw
Quoted 4 lines · Nicholas Krause

This fixes the function e1000_change_mtu to properly check and run the error code by e1000e_up as this function can fail and the error code should be returned to the caller of the function e1000_change_mtu to signal a error has occurred when calling this particular function

The function e1000e_up always returns 0. It should probably be switched to a void instead of an int. You could probably go through and drop a bunch of dead code that is checking for the result of that function as well.

Quoted 37 lines
Signed-off-by Nicholas Krause <[email protected]>
---
  drivers/net/ethernet/intel/e1000e/netdev.c | 7 +++++--
  1 file changed, 5 insertions(+), 2 deletions(-)
drivers/net/ethernet/intel/e1000e/netdev.c +5 -2
index faf4b3f..aebccb1 100644
@@ -5931,6 +5931,7 @@ static int e1000_change_mtu(struct net_device *netdev, int new_mtu)
  {
  	struct e1000_adapter *adapter = netdev_priv(netdev);
  	int max_frame = new_mtu + VLAN_ETH_HLEN + ETH_FCS_LEN;
+	int err = 0;
 
  	/* Jumbo frame support */
  	if ((max_frame > (VLAN_ETH_FRAME_LEN + ETH_FCS_LEN)) &&

@@ -5984,7 +5985,9 @@ static int e1000_change_mtu(struct net_device *netdev, int new_mtu)
  		adapter->rx_buffer_len = VLAN_ETH_FRAME_LEN + ETH_FCS_LEN;
 
  	if (netif_running(netdev))
-		e1000e_up(adapter);
+		err = e1000e_up(adapter);
+		if (err)
+			e_err("Failed to successfully bring up this adapter\n");
  	else
  		e1000e_reset(adapter);
 

@@ -5992,7 +5995,7 @@ static int e1000_change_mtu(struct net_device *netdev, int new_mtu)
 
  	clear_bit(__E1000_RESETTING, &adapter->state);
 
-	return 0;
+	return err;
  }
 
  static int e1000_mii_ioctl(struct net_device *netdev, struct ifreq *ifr,
author
#2 lore raw

PATCH 1/1 e1000e:Fix incorrect assumption that the function e1000e_up always runs successfully in e1000_change_mtu

Quoted 8 lines · Alexander Duyck
Quoted 4 lines · Nicholas Krause

This fixes the function e1000_change_mtu to properly check and run the error code by e1000e_up as this function can fail and the error code should be returned to the caller of the function e1000_change_mtu to signal a error has occurred when calling this particular function

The function e1000e_up always returns 0. It should probably be switched to a void instead of an int. You could probably go through and drop a bunch of dead code that is checking for the result of that function as well.

There is none that I known of but if you want I will check again to make sure and make the function e1000_up_always void.
Nick

Quoted 32 lines
Quoted 31 lines
Signed-off-by Nicholas Krause <[email protected]>
---
  drivers/net/ethernet/intel/e1000e/netdev.c | 7 +++++--
  1 file changed, 5 insertions(+), 2 deletions(-)
drivers/net/ethernet/intel/e1000e/netdev.c +4 -1
index faf4b3f..aebccb1 100644
@@ -5931,6 +5931,7 @@ static int e1000_change_mtu(struct net_device *netdev, int new_mtu)
  {
      struct e1000_adapter *adapter = netdev_priv(netdev);
      int max_frame = new_mtu + VLAN_ETH_HLEN + ETH_FCS_LEN;
+    int err = 0;
        /* Jumbo frame support */
      if ((max_frame > (VLAN_ETH_FRAME_LEN + ETH_FCS_LEN)) &&

@@ -5984,7 +5985,9 @@ static int e1000_change_mtu(struct net_device *netdev, int new_mtu)
          adapter->rx_buffer_len = VLAN_ETH_FRAME_LEN + ETH_FCS_LEN;
        if (netif_running(netdev))
-        e1000e_up(adapter);
+        err = e1000e_up(adapter);
+        if (err)
+            e_err("Failed to successfully bring up this adapter\n");
      else
          e1000e_reset(adapter);
  @@ -5992,7 +5995,7 @@ static int e1000_change_mtu(struct net_device *netdev, int new_mtu)
        clear_bit(__E1000_RESETTING, &adapter->state);

- return 0;
+ return err;
}
static int e1000_mii_ioctl(struct net_device *netdev, struct ifreq *ifr,