Netdev List
 help / color / mirror / Atom feed
* Re: [RFC PATCH net-next] ppp: Allow ppp device connected to an l2tp session to change of namespace
From: James Chapman @ 2013-10-24 15:43 UTC (permalink / raw)
  To: François Cachereul; +Cc: Paul Mackerras, netdev, linux-ppp
In-Reply-To: <526923A7.8090108@alphalink.fr>

On 24/10/13 14:41, François Cachereul wrote:
> On 10/24/2013 12:55 PM, James Chapman wrote:
>> On 24/10/13 11:30, François Cachereul wrote:
>>> Remove NETIF_F_NETNS_LOCAL flag from ppp device in ppp_connect_channel()
>>> if the device is connected to a l2tp session socket.
>>> Restore the flag in ppp_disconnect_channel().
>>
>> What about pppd's network namespace? Also, L2TP's tunnel socket (UDP or
>> L2TP/IP) will be in a different namespace if the ppp interface is moved.
> 
> That's what I'm trying to achieve. I'm not using pppd and my problem is
> as follow:  I need to isolate ppp devices from each other, even when
> they are connected to sessions carried by the same L2TP tunnel.

I'm thinking about the implications of a skb in the net namespace of the
ppp interface passing through a tunnel socket which is in another
namespace. I think net namespaces are completely isolated.

To keep your ppp interfaces isolated from each other, have you
considered using netfilter to prevent data being passed between ppp
interfaces?

> Also, I
> need the authentication to be terminated to know the namespace in which
> the ppp will be moved. For that, the process runs in a namespace with
> its l2tp sockets (tunnel and session) in that same namespace and each
> ppp device is moved in a specific namespace after authentication.
>  
> Regards
> François
> 

-- 
James Chapman
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development

^ permalink raw reply

* Re: [PATCH net] net: sched: Don't free f before it is allocated in route4_change
From: Christoph Paasch @ 2013-10-24 15:41 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, David Miller, Jamal Hadi Salim, Jing Wang
In-Reply-To: <1382627157.7572.59.camel@edumazet-glaptop.roam.corp.google.com>

On 24/10/13 - 08:05:57, Eric Dumazet wrote:
> On Thu, 2013-10-24 at 16:59 +0200, Christoph Paasch wrote:
> > On 24/10/13 - 07:54:33, Eric Dumazet wrote:
> 
> > > I see no bug here, you missed the "goto reinsert;"
> > 
> > Ups - sorry...
> > 
> 
> Yeah, trying to find bugs in Alexey code is really tricky ;)

Yeah :)

Instead of the goto, a simple if {} else {} would have made it
more readable IMO.

^ permalink raw reply

* [net-next 11/11] ixgbevf: bump driver version
From: Jeff Kirsher @ 2013-10-24 15:27 UTC (permalink / raw)
  To: davem; +Cc: Don Skidmore, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1382628458-26601-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Don Skidmore <donald.c.skidmore@intel.com>

Bump patch to reflect what version of the out of tree driver it has
equivalent functionality with (2.11.3).

Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 8407fd2..87279c8a 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -58,7 +58,7 @@ const char ixgbevf_driver_name[] = "ixgbevf";
 static const char ixgbevf_driver_string[] =
 	"Intel(R) 10 Gigabit PCI Express Virtual Function Network Driver";
 
-#define DRV_VERSION "2.7.12-k"
+#define DRV_VERSION "2.11.3-k"
 const char ixgbevf_driver_version[] = DRV_VERSION;
 static char ixgbevf_copyright[] =
 	"Copyright (c) 2009 - 2012 Intel Corporation.";
-- 
1.8.3.1

^ permalink raw reply related

* [net-next 10/11] ixgbevf: implement ethtool get/set coalesce
From: Jeff Kirsher @ 2013-10-24 15:27 UTC (permalink / raw)
  To: davem; +Cc: Jacob Keller, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1382628458-26601-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jacob Keller <jacob.e.keller@intel.com>

This patch adds support for ethtool's get_coalesce and set_coalesce command for
the ixgbevf driver. This enables dynamically updating the minimum time between
interrupts.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbevf/ethtool.c      | 81 +++++++++++++++++++++++
 drivers/net/ethernet/intel/ixgbevf/ixgbevf.h      |  2 +
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |  2 +-
 3 files changed, 84 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ethtool.c b/drivers/net/ethernet/intel/ixgbevf/ethtool.c
index 84329b0..21adb1b 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ethtool.c
@@ -634,6 +634,85 @@ static int ixgbevf_nway_reset(struct net_device *netdev)
 	return 0;
 }
 
+static int ixgbevf_get_coalesce(struct net_device *netdev,
+				struct ethtool_coalesce *ec)
+{
+	struct ixgbevf_adapter *adapter = netdev_priv(netdev);
+
+	/* only valid if in constant ITR mode */
+	if (adapter->rx_itr_setting <= 1)
+		ec->rx_coalesce_usecs = adapter->rx_itr_setting;
+	else
+		ec->rx_coalesce_usecs = adapter->rx_itr_setting >> 2;
+
+	/* if in mixed tx/rx queues per vector mode, report only rx settings */
+	if (adapter->q_vector[0]->tx.count && adapter->q_vector[0]->rx.count)
+		return 0;
+
+	/* only valid if in constant ITR mode */
+	if (adapter->tx_itr_setting <= 1)
+		ec->tx_coalesce_usecs = adapter->tx_itr_setting;
+	else
+		ec->tx_coalesce_usecs = adapter->tx_itr_setting >> 2;
+
+	return 0;
+}
+
+static int ixgbevf_set_coalesce(struct net_device *netdev,
+				struct ethtool_coalesce *ec)
+{
+	struct ixgbevf_adapter *adapter = netdev_priv(netdev);
+	struct ixgbevf_q_vector *q_vector;
+	int num_vectors, i;
+	u16 tx_itr_param, rx_itr_param;
+
+	/* don't accept tx specific changes if we've got mixed RxTx vectors */
+	if (adapter->q_vector[0]->tx.count && adapter->q_vector[0]->rx.count
+	    && ec->tx_coalesce_usecs)
+		return -EINVAL;
+
+
+	if ((ec->rx_coalesce_usecs > (IXGBE_MAX_EITR >> 2)) ||
+	    (ec->tx_coalesce_usecs > (IXGBE_MAX_EITR >> 2)))
+		return -EINVAL;
+
+	if (ec->rx_coalesce_usecs > 1)
+		adapter->rx_itr_setting = ec->rx_coalesce_usecs << 2;
+	else
+		adapter->rx_itr_setting = ec->rx_coalesce_usecs;
+
+	if (adapter->rx_itr_setting == 1)
+		rx_itr_param = IXGBE_20K_ITR;
+	else
+		rx_itr_param = adapter->rx_itr_setting;
+
+
+	if (ec->tx_coalesce_usecs > 1)
+		adapter->tx_itr_setting = ec->tx_coalesce_usecs << 2;
+	else
+		adapter->tx_itr_setting = ec->tx_coalesce_usecs;
+
+	if (adapter->tx_itr_setting == 1)
+		tx_itr_param = IXGBE_10K_ITR;
+	else
+		tx_itr_param = adapter->tx_itr_setting;
+
+	num_vectors = adapter->num_msix_vectors - NON_Q_VECTORS;
+
+	for (i = 0; i < num_vectors; i++) {
+		q_vector = adapter->q_vector[i];
+		if (q_vector->tx.count && !q_vector->rx.count)
+			/* tx only */
+			q_vector->itr = tx_itr_param;
+		else
+			/* rx only or mixed */
+			q_vector->itr = rx_itr_param;
+		ixgbevf_write_eitr(q_vector);
+	}
+
+	return 0;
+}
+
 static const struct ethtool_ops ixgbevf_ethtool_ops = {
 	.get_settings           = ixgbevf_get_settings,
 	.get_drvinfo            = ixgbevf_get_drvinfo,
@@ -649,6 +728,8 @@ static const struct ethtool_ops ixgbevf_ethtool_ops = {
 	.get_sset_count         = ixgbevf_get_sset_count,
 	.get_strings            = ixgbevf_get_strings,
 	.get_ethtool_stats      = ixgbevf_get_ethtool_stats,
+	.get_coalesce           = ixgbevf_get_coalesce,
+	.set_coalesce           = ixgbevf_set_coalesce,
 };
 
 void ixgbevf_set_ethtool_ops(struct net_device *netdev)
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
index 64a2b91..d7837dcc 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
@@ -293,6 +293,8 @@ void ixgbevf_free_tx_resources(struct ixgbevf_adapter *, struct ixgbevf_ring *);
 void ixgbevf_update_stats(struct ixgbevf_adapter *adapter);
 int ethtool_ioctl(struct ifreq *ifr);
 
+extern void ixgbevf_write_eitr(struct ixgbevf_q_vector *q_vector);
+
 void ixgbe_napi_add_all(struct ixgbevf_adapter *adapter);
 void ixgbe_napi_del_all(struct ixgbevf_adapter *adapter);
 
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index c0f9aad..8407fd2 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -580,7 +580,7 @@ static int ixgbevf_poll(struct napi_struct *napi, int budget)
  * ixgbevf_write_eitr - write VTEITR register in hardware specific way
  * @q_vector: structure containing interrupt and ring information
  */
-static void ixgbevf_write_eitr(struct ixgbevf_q_vector *q_vector)
+void ixgbevf_write_eitr(struct ixgbevf_q_vector *q_vector)
 {
 	struct ixgbevf_adapter *adapter = q_vector->adapter;
 	struct ixgbe_hw *hw = &adapter->hw;
-- 
1.8.3.1

^ permalink raw reply related

* [net-next 06/11] ixgbe: cleanup ixgbe_enumerate_functions
From: Jeff Kirsher @ 2013-10-24 15:27 UTC (permalink / raw)
  To: davem; +Cc: Jacob Keller, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1382628458-26601-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jacob Keller <jacob.e.keller@intel.com>

This function previously had the same check as used by the
ixgbe_pcie_from_parent. As the hardcode is due to the device having an internal
switch, this function should simply use the call from ixgbe_pcie_from_parent.
This reduces code complexity and makes it less likely a developer will forget
to update the list in the future.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 43b777a..6828d0e7 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7362,19 +7362,16 @@ static const struct net_device_ops ixgbe_netdev_ops = {
  **/
 static inline int ixgbe_enumerate_functions(struct ixgbe_adapter *adapter)
 {
-	struct ixgbe_hw *hw = &adapter->hw;
 	struct list_head *entry;
 	int physfns = 0;
 
-	/* Some cards can not use the generic count PCIe functions method, and
-	 * so must be hardcoded to the correct value.
+	/* Some cards can not use the generic count PCIe functions method,
+	 * because they are behind a parent switch, so we hardcode these with
+	 * the correct number of functions.
 	 */
-	switch (hw->device_id) {
-	case IXGBE_DEV_ID_82599_SFP_SF_QP:
-	case IXGBE_DEV_ID_82599_QSFP_SF_QP:
+	if (ixgbe_pcie_from_parent(&adapter->hw)) {
 		physfns = 4;
-		break;
-	default:
+	} else {
 		list_for_each(entry, &adapter->pdev->bus_list) {
 			struct pci_dev *pdev =
 				list_entry(entry, struct pci_dev, bus_list);
-- 
1.8.3.1

^ permalink raw reply related

* [net-next 08/11] ixgbe: fix rx-usecs range checks for BQL
From: Jeff Kirsher @ 2013-10-24 15:27 UTC (permalink / raw)
  To: davem; +Cc: Emil Tantilov, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1382628458-26601-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Emil Tantilov <emil.s.tantilov@intel.com>

This patch resolves an issue where the logic used to detect changes in rx-usecs
was incorrect and was masked by the call to ixgbe_update_rsc().

Setting rx-usecs between 0,2-9 and 1,10 and up requires a reset to allow
ixgbe_configure_tx_ring() to set the correct value for TXDCTL.WTHRESH in
order to avoid Tx hangs with BQL enabled.

Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index 90aac31..4e7c9b0 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -2257,13 +2257,13 @@ static int ixgbe_set_coalesce(struct net_device *netdev,
 
 #if IS_ENABLED(CONFIG_BQL)
 	/* detect ITR changes that require update of TXDCTL.WTHRESH */
-	if ((adapter->tx_itr_setting > 1) &&
+	if ((adapter->tx_itr_setting != 1) &&
 	    (adapter->tx_itr_setting < IXGBE_100K_ITR)) {
 		if ((tx_itr_prev == 1) ||
-		    (tx_itr_prev > IXGBE_100K_ITR))
+		    (tx_itr_prev >= IXGBE_100K_ITR))
 			need_reset = true;
 	} else {
-		if ((tx_itr_prev > 1) &&
+		if ((tx_itr_prev != 1) &&
 		    (tx_itr_prev < IXGBE_100K_ITR))
 			need_reset = true;
 	}
-- 
1.8.3.1

^ permalink raw reply related

* [net-next 07/11] ixgbe: use pcie_capability_read_word() to simplify code
From: Jeff Kirsher @ 2013-10-24 15:27 UTC (permalink / raw)
  To: davem; +Cc: Yijing Wang, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1382628458-26601-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Yijing Wang <wangyijing@huawei.com>

use pcie_capability_read_word() to simplify code.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 6828d0e7..ce3eb60 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -153,7 +153,6 @@ MODULE_VERSION(DRV_VERSION);
 static int ixgbe_read_pci_cfg_word_parent(struct ixgbe_adapter *adapter,
 					  u32 reg, u16 *value)
 {
-	int pos = 0;
 	struct pci_dev *parent_dev;
 	struct pci_bus *parent_bus;
 
@@ -165,11 +164,10 @@ static int ixgbe_read_pci_cfg_word_parent(struct ixgbe_adapter *adapter,
 	if (!parent_dev)
 		return -1;
 
-	pos = pci_find_capability(parent_dev, PCI_CAP_ID_EXP);
-	if (!pos)
+	if (!pci_is_pcie(parent_dev))
 		return -1;
 
-	pci_read_config_word(parent_dev, pos + reg, value);
+	pcie_capability_read_word(parent_dev, reg, value);
 	return 0;
 }
 
-- 
1.8.3.1

^ permalink raw reply related

* [net-next 09/11] ixgbevf: Adds function to set PSRTYPE register
From: Jeff Kirsher @ 2013-10-24 15:27 UTC (permalink / raw)
  To: davem; +Cc: Don Skidmore, netdev, gospo, sassmann, Alexander Duyck,
	Jeff Kirsher
In-Reply-To: <1382628458-26601-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Don Skidmore <donald.c.skidmore@intel.com>

This patch creates a new function to set PSRTYPE. This function helps lay
the ground work for eventual multi queue support.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index ce27d62..c0f9aad 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -1082,6 +1082,21 @@ static void ixgbevf_configure_srrctl(struct ixgbevf_adapter *adapter, int index)
 	IXGBE_WRITE_REG(hw, IXGBE_VFSRRCTL(index), srrctl);
 }
 
+static void ixgbevf_setup_psrtype(struct ixgbevf_adapter *adapter)
+{
+	struct ixgbe_hw *hw = &adapter->hw;
+
+	/* PSRTYPE must be initialized in 82599 */
+	u32 psrtype = IXGBE_PSRTYPE_TCPHDR | IXGBE_PSRTYPE_UDPHDR |
+		      IXGBE_PSRTYPE_IPV4HDR | IXGBE_PSRTYPE_IPV6HDR |
+		      IXGBE_PSRTYPE_L2HDR;
+
+	if (adapter->num_rx_queues > 1)
+		psrtype |= 1 << 29;
+
+	IXGBE_WRITE_REG(hw, IXGBE_VFPSRTYPE, psrtype);
+}
+
 static void ixgbevf_set_rx_buffer_len(struct ixgbevf_adapter *adapter)
 {
 	struct ixgbe_hw *hw = &adapter->hw;
@@ -1129,8 +1144,7 @@ static void ixgbevf_configure_rx(struct ixgbevf_adapter *adapter)
 	int i, j;
 	u32 rdlen;
 
-	/* PSRTYPE must be initialized in 82599 */
-	IXGBE_WRITE_REG(hw, IXGBE_VFPSRTYPE, 0);
+	ixgbevf_setup_psrtype(adapter);
 
 	/* set_rx_buffer_len must be called before ring initialization */
 	ixgbevf_set_rx_buffer_len(adapter);
-- 
1.8.3.1

^ permalink raw reply related

* [net-next 05/11] igb: fix driver reload with VF assigned to guest
From: Jeff Kirsher @ 2013-10-24 15:27 UTC (permalink / raw)
  To: davem; +Cc: Stefan Assmann, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1382628458-26601-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Stefan Assmann <sassmann@kpanic.de>

commit fa44f2f185f7f9da19d331929bb1b56c1ccd1d93 broke reloading of igb, when
VFs are assigned to a guest, in several ways.
1. on module load adapter->vf_data does not get properly allocated,
resulting in a null pointer exception when accessing adapter->vf_data in
igb_reset() on module reload.
 modprobe -r igb ; modprobe igb max_vfs=7
[  215.215837] igb 0000:01:00.1: removed PHC on eth1
[  216.932072] igb 0000:01:00.1: IOV Disabled
[  216.937038] igb 0000:01:00.0: removed PHC on eth0
[  217.127032] igb 0000:01:00.0: Cannot deallocate SR-IOV virtual functions while they are assigned - VFs will not be deallocated
[  217.146178] igb: Intel(R) Gigabit Ethernet Network Driver - version 5.0.5-k
[  217.154050] igb: Copyright (c) 2007-2013 Intel Corporation.
[  217.160688] igb 0000:01:00.0: Enabling SR-IOV VFs using the module parameter is deprecated - please use the pci sysfs interface.
[  217.173703] igb 0000:01:00.0: irq 103 for MSI/MSI-X
[  217.179227] igb 0000:01:00.0: irq 104 for MSI/MSI-X
[  217.184735] igb 0000:01:00.0: irq 105 for MSI/MSI-X
[  217.220082] BUG: unable to handle kernel NULL pointer dereference at 0000000000000048
[  217.228846] IP: [<ffffffffa007c5e5>] igb_reset+0xc5/0x4b0 [igb]
[  217.235472] PGD 3607ec067 PUD 36170b067 PMD 0
[  217.240461] Oops: 0002 [#1] SMP
[  217.244085] Modules linked in: igb(+) igbvf mptsas mptscsih mptbase scsi_transport_sas [last unloaded: igb]
[  217.255040] CPU: 4 PID: 4833 Comm: modprobe Not tainted 3.11.0+ #46
[...]
[  217.390007]  [<ffffffffa007fab2>] igb_probe+0x892/0xfd0 [igb]
[  217.396422]  [<ffffffff81470b3e>] local_pci_probe+0x1e/0x40
[  217.402641]  [<ffffffff81472029>] pci_device_probe+0xf9/0x110
[...]
2. A follow up issue, pci_enable_sriov() should only be called if no VFs were
still allocated on module unload. Otherwise pci_enable_sriov() gets called
multiple times in a row rendering the NIC unusable until reset.
3. simply calling igb_enable_sriov() in igb_probe_vfs() is not enough as the
interrupts need to be re-setup. Switching that to igb_pci_enable_sriov().

Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Tested-by: Sibai Li <Sibai.li@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 37 +++++++++++++------------------
 1 file changed, 16 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index a505d3b..ebe6370 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -182,6 +182,7 @@ static void igb_check_vf_rate_limit(struct igb_adapter *);
 
 #ifdef CONFIG_PCI_IOV
 static int igb_vf_configure(struct igb_adapter *adapter, int vf);
+static int igb_pci_enable_sriov(struct pci_dev *dev, int num_vfs);
 #endif
 
 #ifdef CONFIG_PM
@@ -2429,7 +2430,7 @@ err_dma:
 }
 
 #ifdef CONFIG_PCI_IOV
-static int  igb_disable_sriov(struct pci_dev *pdev)
+static int igb_disable_sriov(struct pci_dev *pdev)
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct igb_adapter *adapter = netdev_priv(netdev);
@@ -2470,27 +2471,19 @@ static int igb_enable_sriov(struct pci_dev *pdev, int num_vfs)
 	int err = 0;
 	int i;
 
-	if (!adapter->msix_entries) {
+	if (!adapter->msix_entries || num_vfs > 7) {
 		err = -EPERM;
 		goto out;
 	}
-
 	if (!num_vfs)
 		goto out;
-	else if (old_vfs && old_vfs == num_vfs)
-		goto out;
-	else if (old_vfs && old_vfs != num_vfs)
-		err = igb_disable_sriov(pdev);
-
-	if (err)
-		goto out;
 
-	if (num_vfs > 7) {
-		err = -EPERM;
-		goto out;
-	}
-
-	adapter->vfs_allocated_count = num_vfs;
+	if (old_vfs) {
+		dev_info(&pdev->dev, "%d pre-allocated VFs found - override max_vfs setting of %d\n",
+			 old_vfs, max_vfs);
+		adapter->vfs_allocated_count = old_vfs;
+	} else
+		adapter->vfs_allocated_count = num_vfs;
 
 	adapter->vf_data = kcalloc(adapter->vfs_allocated_count,
 				sizeof(struct vf_data_storage), GFP_KERNEL);
@@ -2504,10 +2497,12 @@ static int igb_enable_sriov(struct pci_dev *pdev, int num_vfs)
 		goto out;
 	}
 
-	err = pci_enable_sriov(pdev, adapter->vfs_allocated_count);
-	if (err)
-		goto err_out;
-
+	/* only call pci_enable_sriov() if no VFs are allocated already */
+	if (!old_vfs) {
+		err = pci_enable_sriov(pdev, adapter->vfs_allocated_count);
+		if (err)
+			goto err_out;
+	}
 	dev_info(&pdev->dev, "%d VFs allocated\n",
 		 adapter->vfs_allocated_count);
 	for (i = 0; i < adapter->vfs_allocated_count; i++)
@@ -2623,7 +2618,7 @@ static void igb_probe_vfs(struct igb_adapter *adapter)
 		return;
 
 	pci_sriov_set_totalvfs(pdev, 7);
-	igb_enable_sriov(pdev, max_vfs);
+	igb_pci_enable_sriov(pdev, max_vfs);
 
 #endif /* CONFIG_PCI_IOV */
 }
-- 
1.8.3.1

^ permalink raw reply related

* [net-next 04/11] igb: Fix master/slave mode for all m88 i354 PHY's
From: Jeff Kirsher @ 2013-10-24 15:27 UTC (permalink / raw)
  To: davem; +Cc: Carolyn Wyborny, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1382628458-26601-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Carolyn Wyborny <carolyn.wyborny@intel.com>

This patch calls code to set the master/slave mode for all m88 gen 2
PHY's. This patch also removes the call to this function for I210 devices
only from the function that is not called by I210 devices.

Signed-off-by: Carolyn Wyborny <carolyn.wyborny@intel.com>
Tested-by: Jeff Pieper  <jeffrey.e.pieper@gmail.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/e1000_phy.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/e1000_phy.c b/drivers/net/ethernet/intel/igb/e1000_phy.c
index e726675..c4c4fe3 100644
--- a/drivers/net/ethernet/intel/igb/e1000_phy.c
+++ b/drivers/net/ethernet/intel/igb/e1000_phy.c
@@ -708,11 +708,6 @@ s32 igb_copper_link_setup_m88(struct e1000_hw *hw)
 		hw_dbg("Error committing the PHY changes\n");
 		goto out;
 	}
-	if (phy->type == e1000_phy_i210) {
-		ret_val = igb_set_master_slave_mode(hw);
-		if (ret_val)
-			return ret_val;
-	}
 
 out:
 	return ret_val;
@@ -806,6 +801,9 @@ s32 igb_copper_link_setup_m88_gen2(struct e1000_hw *hw)
 		hw_dbg("Error committing the PHY changes\n");
 		return ret_val;
 	}
+	ret_val = igb_set_master_slave_mode(hw);
+	if (ret_val)
+		return ret_val;
 
 	return 0;
 }
-- 
1.8.3.1

^ permalink raw reply related

* [net-next 03/11] i40e: remove unused including <linux/version.h>
From: Jeff Kirsher @ 2013-10-24 15:27 UTC (permalink / raw)
  To: davem; +Cc: Wei Yongjun, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1382628458-26601-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

Remove including <linux/version.h> that don't need it.

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 49572dc..1ca9834 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -46,7 +46,6 @@
 #include <linux/sctp.h>
 #include <linux/pkt_sched.h>
 #include <linux/ipv6.h>
-#include <linux/version.h>
 #include <net/checksum.h>
 #include <net/ip6_checksum.h>
 #include <linux/ethtool.h>
-- 
1.8.3.1

^ permalink raw reply related

* [net-next 01/11] igbvf: integer wrapping bug setting the mtu
From: Jeff Kirsher @ 2013-10-24 15:27 UTC (permalink / raw)
  To: davem; +Cc: Dan Carpenter, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1382628458-26601-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Dan Carpenter <dan.carpenter@oracle.com>

If new_mtu is very large then "new_mtu + ETH_HLEN + ETH_FCS_LEN" can
wrap and the check on the next line can underflow. This is one of those
bugs which can be triggered by the user if you have namespaces
configured.

Also since this is something the user can trigger then we don't want to
have dev_err() message.

This is a static checker fix and I'm not sure what the impact is.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Tested-by: Sibai Li Sibai.li@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igbvf/netdev.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/igbvf/netdev.c b/drivers/net/ethernet/intel/igbvf/netdev.c
index 93eb7ee..f48ae71 100644
--- a/drivers/net/ethernet/intel/igbvf/netdev.c
+++ b/drivers/net/ethernet/intel/igbvf/netdev.c
@@ -2343,10 +2343,9 @@ static int igbvf_change_mtu(struct net_device *netdev, int new_mtu)
 	struct igbvf_adapter *adapter = netdev_priv(netdev);
 	int max_frame = new_mtu + ETH_HLEN + ETH_FCS_LEN;
 
-	if ((new_mtu < 68) || (max_frame > MAX_JUMBO_FRAME_SIZE)) {
-		dev_err(&adapter->pdev->dev, "Invalid MTU setting\n");
+	if (new_mtu < 68 || new_mtu > INT_MAX - ETH_HLEN - ETH_FCS_LEN ||
+	    max_frame > MAX_JUMBO_FRAME_SIZE)
 		return -EINVAL;
-	}
 
 #define MAX_STD_JUMBO_FRAME_SIZE 9234
 	if (max_frame > MAX_STD_JUMBO_FRAME_SIZE) {
-- 
1.8.3.1

^ permalink raw reply related

* [net-next 02/11] igbvf: add missing iounmap() on error in igbvf_probe()
From: Jeff Kirsher @ 2013-10-24 15:27 UTC (permalink / raw)
  To: davem; +Cc: Wei Yongjun, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1382628458-26601-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

Add the missing iounmap() before return from igbvf_probe()
in the error handling case.

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Tested-by: Sibai Li <Sibai.li@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igbvf/netdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igbvf/netdev.c b/drivers/net/ethernet/intel/igbvf/netdev.c
index f48ae71..9fadbb2 100644
--- a/drivers/net/ethernet/intel/igbvf/netdev.c
+++ b/drivers/net/ethernet/intel/igbvf/netdev.c
@@ -2698,7 +2698,7 @@ static int igbvf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (ei->get_variants) {
 		err = ei->get_variants(adapter);
 		if (err)
-			goto err_ioremap;
+			goto err_get_variants;
 	}
 
 	/* setup adapter struct */
@@ -2795,6 +2795,7 @@ err_hw_init:
 	kfree(adapter->rx_ring);
 err_sw_init:
 	igbvf_reset_interrupt_capability(adapter);
+err_get_variants:
 	iounmap(adapter->hw.hw_addr);
 err_ioremap:
 	free_netdev(netdev);
-- 
1.8.3.1

^ permalink raw reply related

* [net-next 00/11][pull request] Intel Wired LAN Driver Updates
From: Jeff Kirsher @ 2013-10-24 15:27 UTC (permalink / raw)
  To: davem; +Cc: Jeff Kirsher, netdev, gospo, sassmann

This series contains updates to igb, igbvf, i40e, ixgbe and ixgbevf.

Dan Carpenter provides a patch for igbvf to fix a bug found by a static
checker.  If the new MTU is very large, then "new_mtu + ETH_HLEN +
ETH_FCS_LEN" can wrap and the check on the next line can underflow.

Wei Yongjun provides 2 patches, the first against igbvf adds a missing
iounmap() before the return from igbvf_probe().  The second against
i40e, removes the include <linux/version.h> because it is not needed.

Carolyn provides a patch for igb to fix a call to set the master/slave
mode for all m88 generation 2 PHY's and removes the call for I210
devices which do not need it.

Stefan Assmann provides a patch for igb to fix an issue which was broke
by:
   commit fa44f2f185f7f9da19d331929bb1b56c1ccd1d93
   Author: Greg Rose <gregory.v.rose@intel.com>
   Date:   Thu Jan 17 01:03:06 2013 -0800
   igb: Enable SR-IOV configuration via PCI sysfs interface
which breaks the reloading of igb when VFs are assigned to a guest, in
several ways.

Jacob provides a patch for ixgbe and ixgbevf.  First, against ixgbe,
cleans up ixgbe_enumerate_functions to reduce code complexity.  The
second, against ixgbevf, adds support for ethtool's get_coalesce and
set_coalesce command for the ixgbevf driver.

Yijing Wang provides a patch for ixgbe to use pcie_capability_read_word()
to simplify the code.

Emil provides a ixgbe patch to fix an issue where the logic used to
detect changes in rx-usecs was incorrect and was masked by the call to
ixgbe_update_rsc().

Don provides 2 patches for ixgbevf.  First creates a new function to set
PSRTYPE.  The second bumps the ixgbevf driver version.

The following are changes since commit b45bd46decd947eaa3497699d450e0851d247534:
  Merge tag 'batman-adv-for-davem' of git://git.open-mesh.org/linux-merge
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master

Carolyn Wyborny (1):
  igb: Fix master/slave mode for all m88 i354 PHY's

Dan Carpenter (1):
  igbvf: integer wrapping bug setting the mtu

Don Skidmore (2):
  ixgbevf: Adds function to set PSRTYPE register
  ixgbevf: bump driver version

Emil Tantilov (1):
  ixgbe: fix rx-usecs range checks for BQL

Jacob Keller (2):
  ixgbe: cleanup ixgbe_enumerate_functions
  ixgbevf: implement ethtool get/set coalesce

Stefan Assmann (1):
  igb: fix driver reload with VF assigned to guest

Wei Yongjun (2):
  igbvf: add missing iounmap() on error in igbvf_probe()
  i40e: remove unused including <linux/version.h>

Yijing Wang (1):
  ixgbe: use pcie_capability_read_word() to simplify code

 drivers/net/ethernet/intel/i40e/i40e.h            |  1 -
 drivers/net/ethernet/intel/igb/e1000_phy.c        |  8 +--
 drivers/net/ethernet/intel/igb/igb_main.c         | 37 +++++------
 drivers/net/ethernet/intel/igbvf/netdev.c         |  8 +--
 drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c  |  6 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c     | 19 ++----
 drivers/net/ethernet/intel/ixgbevf/ethtool.c      | 81 +++++++++++++++++++++++
 drivers/net/ethernet/intel/ixgbevf/ixgbevf.h      |  2 +
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 22 ++++--
 9 files changed, 134 insertions(+), 50 deletions(-)

-- 
1.8.3.1

^ permalink raw reply

* RE: transmit lockup using smsc95xx ethernet on usb3
From: David Laight @ 2013-10-24 15:05 UTC (permalink / raw)
  To: Sarah Sharp; +Cc: netdev, linux-usb, Xenia Ragiadakou
In-Reply-To: <20131017000758.GL7082@xanatos>

> Have you tried the latest stable kernel or the latest -rc kernel?

I've built a kernel based on Linus's tree from last Friday, 3.12-rc6 ish.
Commented out the trace for short reads - happens all the time.

I've not seen an error on a Bo yet, the failure rate is depressingly low.
I have had an error that started with an interrupt completion (I think
from the root), that leads to a full unplug-replug sequence that
overfilled the kernel message buffer (rebuilt with a much bigger buffer).

I've just got a error -71 from a Bi. This generates:
(There are actually two separated by a Bo completion and setup.

[175908.563068] xhci_hcd 0000:00:14.0: Transfer error on endpoint
[175908.563070] xhci_hcd 0000:00:14.0: Cleaning up stalled endpoint ring
[175908.563072] xhci_hcd 0000:00:14.0: Finding segment containing stopped TRB.
[175908.563074] xhci_hcd 0000:00:14.0: Finding endpoint context
[175908.563076] xhci_hcd 0000:00:14.0: Finding segment containing last TRB in TD.
[175908.563078] xhci_hcd 0000:00:14.0: Cycle state = 0x1
[175908.563081] xhci_hcd 0000:00:14.0: New dequeue segment = ffff8800d6a817e0 (virtual)
[175908.563089] xhci_hcd 0000:00:14.0: New dequeue pointer = 0x2137a4b90 (DMA)
[175908.563096] xhci_hcd 0000:00:14.0: Queueing new dequeue state
[175908.563104] xhci_hcd 0000:00:14.0: Set TR Deq Ptr cmd, new deq seg = ffff8800d6a817e0 (0x2137a4800 dma), new deq ptr = ffff8802137a4b90 (0x2137a4b90 dma), new cycle = 1
[175908.563110] xhci_hcd 0000:00:14.0: // Ding dong!
[175908.563119] xhci_hcd 0000:00:14.0: Giveback URB ffff8802114fd9c0, len = 0, expected = 18944, status = -71
[175908.563122] xhci_hcd 0000:00:14.0: Ignoring reset ep completion code of 1
[175908.563125] xhci_hcd 0000:00:14.0: Successful Set TR Deq Ptr cmd, deq = @2137a4b91

What is interesting is that the usbmon trace then shows only 2 Bi URB being used
(rather than 4). After 125ms (assuming the usbmon timestamps are us)
another 2 URB are added.
Since the URB are usually recycled (or at least freed and immediately realloced
so getting the same address) I wonder if this is a memory leak?
In any case waiting that long before adding the URB back doesn't seem right.

I don't think I've seen an error that only affected the Bi side before, so
don't know whether the recover behaviour has changed.

FWIW we have identified something 'sub-optimal' with the pcb layout that might
be responsible for noise on the USB data/clock source. However the xhci driver
should recover from such errors.
If the hw guys fix the pcb I'm going to need to keep a failing system!

	David

^ permalink raw reply

* Re: [PATCH net] net: sched: Don't free f before it is allocated in route4_change
From: Eric Dumazet @ 2013-10-24 15:05 UTC (permalink / raw)
  To: Christoph Paasch; +Cc: netdev, David Miller, Jamal Hadi Salim, Jing Wang
In-Reply-To: <20131024145936.GB15936@cpaasch-mac>

On Thu, 2013-10-24 at 16:59 +0200, Christoph Paasch wrote:
> On 24/10/13 - 07:54:33, Eric Dumazet wrote:

> > I see no bug here, you missed the "goto reinsert;"
> 
> Ups - sorry...
> 

Yeah, trying to find bugs in Alexey code is really tricky ;)

^ permalink raw reply

* Re: [PATCH net] net: sched: Don't free f before it is allocated in route4_change
From: Christoph Paasch @ 2013-10-24 14:59 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, David Miller, Jamal Hadi Salim, Jing Wang
In-Reply-To: <1382626473.7572.58.camel@edumazet-glaptop.roam.corp.google.com>

On 24/10/13 - 07:54:33, Eric Dumazet wrote:
> On Thu, 2013-10-24 at 16:50 +0200, Christoph Paasch wrote:
> > f is set to *arg in route4_change at the beginning, which points to a
> > route4_filter in the hash-table (gotten through route4_get, called by
> > tc_ctl_filter). If the alloc of head fails, we should not goto errout,
> > because this will free f and thus freed memory will be referenced by
> > the hash-table.
> > Only later the pointer f will change to an allocated route4_filter.
> > 
> > This patch returns err if the allocation of head fails as f has not yet
> > been allocated inside route4_change.
> > 
> > Seems the code has been like this since Linus's original git-commit.
> > 
> > Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be>
> > ---
> >  net/sched/cls_route.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
> > index 37da567..f17c67f 100644
> > --- a/net/sched/cls_route.c
> > +++ b/net/sched/cls_route.c
> > @@ -470,7 +470,7 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
> >  	if (head == NULL) {
> >  		head = kzalloc(sizeof(struct route4_head), GFP_KERNEL);
> >  		if (head == NULL)
> > -			goto errout;
> > +			return err;
> >  
> >  		tcf_tree_lock(tp);
> >  		tp->root = head;
> 
> I see no bug here, you missed the "goto reinsert;"

Ups - sorry...

^ permalink raw reply

* Re: [PATCH net] net: sched: Don't free f before it is allocated in route4_change
From: Eric Dumazet @ 2013-10-24 14:54 UTC (permalink / raw)
  To: Christoph Paasch; +Cc: netdev, David Miller, Jamal Hadi Salim, Jing Wang
In-Reply-To: <1382626250-15676-1-git-send-email-christoph.paasch@uclouvain.be>

On Thu, 2013-10-24 at 16:50 +0200, Christoph Paasch wrote:
> f is set to *arg in route4_change at the beginning, which points to a
> route4_filter in the hash-table (gotten through route4_get, called by
> tc_ctl_filter). If the alloc of head fails, we should not goto errout,
> because this will free f and thus freed memory will be referenced by
> the hash-table.
> Only later the pointer f will change to an allocated route4_filter.
> 
> This patch returns err if the allocation of head fails as f has not yet
> been allocated inside route4_change.
> 
> Seems the code has been like this since Linus's original git-commit.
> 
> Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be>
> ---
>  net/sched/cls_route.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
> index 37da567..f17c67f 100644
> --- a/net/sched/cls_route.c
> +++ b/net/sched/cls_route.c
> @@ -470,7 +470,7 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
>  	if (head == NULL) {
>  		head = kzalloc(sizeof(struct route4_head), GFP_KERNEL);
>  		if (head == NULL)
> -			goto errout;
> +			return err;
>  
>  		tcf_tree_lock(tp);
>  		tp->root = head;

I see no bug here, you missed the "goto reinsert;"

Guys, if there are no bugs, could we calm down ?

Thanks !

^ permalink raw reply

* [PATCH net] net: sched: Don't free f before it is allocated in route4_change
From: Christoph Paasch @ 2013-10-24 14:50 UTC (permalink / raw)
  To: netdev; +Cc: Eric Dumazet, David Miller, Jamal Hadi Salim, Jing Wang

f is set to *arg in route4_change at the beginning, which points to a
route4_filter in the hash-table (gotten through route4_get, called by
tc_ctl_filter). If the alloc of head fails, we should not goto errout,
because this will free f and thus freed memory will be referenced by
the hash-table.
Only later the pointer f will change to an allocated route4_filter.

This patch returns err if the allocation of head fails as f has not yet
been allocated inside route4_change.

Seems the code has been like this since Linus's original git-commit.

Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be>
---
 net/sched/cls_route.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index 37da567..f17c67f 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -470,7 +470,7 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
 	if (head == NULL) {
 		head = kzalloc(sizeof(struct route4_head), GFP_KERNEL);
 		if (head == NULL)
-			goto errout;
+			return err;
 
 		tcf_tree_lock(tp);
 		tp->root = head;
-- 
1.8.3.2

^ permalink raw reply related

* Re: [PATCH NEXT 1/5] staging: r8188eu: Fix sparse warnings in ioctl_linux.c
From: Greg KH @ 2013-10-24 14:51 UTC (permalink / raw)
  To: Larry Finger; +Cc: Dan Carpenter, devel, netdev
In-Reply-To: <526932DD.2080602@lwfinger.net>

On Thu, Oct 24, 2013 at 09:46:53AM -0500, Larry Finger wrote:
> On 10/24/2013 05:10 AM, Dan Carpenter wrote:
> > I have looked at how this is called from ioctl_private_call() and it
> > seems like these are actual user pointers and the code is buggy.  The
> > patch silences the warnings instead of fixing the bugs.
> 
> Thanks for the review.
> 
> Greg - please drop this one and take only 2-5. Do you want a resubmission?

I can just drop it.  I'm way behind on patches due to the kernel summit
and LinuxCon, so it will be a week or so before I can catch up, sorry.

greg k-h

^ permalink raw reply

* Re: [PATCH NEXT 1/5] staging: r8188eu: Fix sparse warnings in ioctl_linux.c
From: Larry Finger @ 2013-10-24 14:46 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: devel, gregkh, netdev
In-Reply-To: <20131024101001.GD5871@mwanda>

On 10/24/2013 05:10 AM, Dan Carpenter wrote:
> I have looked at how this is called from ioctl_private_call() and it
> seems like these are actual user pointers and the code is buggy.  The
> patch silences the warnings instead of fixing the bugs.

Thanks for the review.

Greg - please drop this one and take only 2-5. Do you want a resubmission?

Larry

^ permalink raw reply

* Re: [PATCH 1/1] net:sched  fix a bug about memery leak
From: Christoph Paasch @ 2013-10-24 14:23 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Jing Wang, davem, jhs, netdev
In-Reply-To: <20131024140709.GA19470@cpaasch-mac>

On 24/10/13 - 16:07:09, Christoph Paasch wrote:
> On 24/10/13 - 02:33:38, Eric Dumazet wrote:
> > On Thu, 2013-10-24 at 17:12 +0800, Jing Wang wrote:
> > > From: Jing Wang <windsdaemon@gmail.com>
> > > 
> > > the code isn't properly release memory
> > > 
> > > Signed-off-by: Jing Wang <windsdaemon@gmail.com>
> > > ---
> > >  net/sched/cls_route.c |    9 ++++++---
> > >  1 files changed, 6 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
> > > index 37da567..118f8d5 100644
> > > --- a/net/sched/cls_route.c
> > > +++ b/net/sched/cls_route.c
> > > @@ -466,11 +466,11 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
> > >  		goto reinsert;
> > >  	}
> > >  
> > > -	err = -ENOBUFS;
> > > +	err = -ENOMEM;
> > >  	if (head == NULL) {
> > >  		head = kzalloc(sizeof(struct route4_head), GFP_KERNEL);
> > >  		if (head == NULL)
> > > -			goto errout;
> > > +			goto errhead;
> > >  
> > >  		tcf_tree_lock(tp);
> > >  		tp->root = head;
> > > @@ -479,7 +479,7 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
> > >  
> > >  	f = kzalloc(sizeof(struct route4_filter), GFP_KERNEL);
> > >  	if (f == NULL)
> > > -		goto errout;
> > > +		goto errflt;
> > >  
> > >  	err = route4_set_parms(net, tp, base, f, handle, head, tb,
> > >  		tca[TCA_RATE], 1);
> > > @@ -517,6 +517,9 @@ reinsert:
> > >  
> > >  errout:
> > >  	kfree(f);
> > > +errflt:
> > > +    kfree(head);
> > > +errhead:
> > >  	return err;
> > >  }
> > >  
> > 
> > I don't think this patch is needed or correct.
> > 
> > tp->root is the head, you cannot free it like that.
> > 
> > It will be freed properly in route4_destroy()
> > 
> > Please elaborate, thanks.
> 
> I think there is something else wrong in route4_change:
> 
> ----
> From 1409402bf964bef79667755a5d0d5e0c2bd663f3 Mon Sep 17 00:00:00 2001
> From: Christoph Paasch <christoph.paasch@uclouvain.be>
> Date: Thu, 24 Oct 2013 15:33:28 +0200
> Subject: [PATCH net] net: sched: Don't free f before it is allocated in
>  route4_change
> 
> f is set to *arg in route4_change at the beginning, which points to a                                                                                                                                                                                                          
> route4_filter in the hash-table (gotten through route4_get, called by
> tc_ctl_filter). If the alloc of head fails, we should not goto errout,
> because this will free f and thus freed memory will be referenced by
> the hash-table.
> Only later the pointer f will change to an allocated route4_filter.
> 
> This patch returns err if the allocation of head fails as f has not yet
> been allocated inside route4_change.
> 
> Seems the code has been like this since Linus's original git-commit.
> 
> Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be>
> ---
>  net/sched/cls_route.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
> index 37da567..f17c67f 100644
> --- a/net/sched/cls_route.c
> +++ b/net/sched/cls_route.c
> @@ -470,7 +470,7 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
>         if (head == NULL) {
>                 head = kzalloc(sizeof(struct route4_head), GFP_KERNEL);
>                 if (head == NULL)
> -                       goto errout;
> +                       return err;

Oh... copy-paste from vim into mutt actually replaced the tabs by spaces...

I will resend. Sorry.


Christoph

> 
>                 tcf_tree_lock(tp);
>                 tp->root = head;
> -- 
> 1.8.3.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC PATCH net-next] ppp: Allow ppp device connected to an l2tp session to change of namespace
From: Sergei Shtylyov @ 2013-10-24 14:23 UTC (permalink / raw)
  To: François Cachereul, Paul Mackerras, James Chapman; +Cc: netdev, linux-ppp
In-Reply-To: <5268F6CD.9070600@alphalink.fr>

Hello.

On 10/24/2013 02:30 PM, François Cachereul wrote:

> Remove NETIF_F_NETNS_LOCAL flag from ppp device in ppp_connect_channel()
> if the device is connected to a l2tp session socket.
> Restore the flag in ppp_disconnect_channel().

> Signed-off-by: François CACHEREUL <f.cachereul@alphalink.fr>
> ---
> I'm trying to get rid of this flag for ppp device connected to l2tp
> session, it's seem to be safe to do it for as l2tp_ppp module hasn't any
> reference to the ppp device except to the device name. We can probably
> do it for others modules but pppoe and pptp will require more work.

> I remove the flag for l2tp in ppp_generic.c because I couldn't find a
> place like a callback to do it in l2tp_ppp.c. The best will be to
> remove the flag for all ppp devices.

> François

>   drivers/net/ppp/ppp_generic.c |   11 +++++++++++
>   1 file changed, 11 insertions(+)

> diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
> index 72ff14b..7ccf2ae 100644
> --- a/drivers/net/ppp/ppp_generic.c
> +++ b/drivers/net/ppp/ppp_generic.c
[...]
> @@ -2883,6 +2886,13 @@ ppp_connect_channel(struct channel *pch, int unit)
>   	++ppp->n_channels;
>   	pch->ppp = ppp;
>   	atomic_inc(&ppp->file.refcnt);
> +
> +	/* allow ppp net device to be moved in another network namespace
> +	 * if it's connected to an l2tp session */

    Acording to Documentation/CodingStyle, the preferred comment style in the 
networking code is:

/* bla
  * bla
  */

WBR, Sergei


^ permalink raw reply

* Re: [PATCH 1/1] net:sched  fix a bug about memery leak
From: Sergei Shtylyov @ 2013-10-24 14:10 UTC (permalink / raw)
  To: Jing Wang, davem, jhs, netdev
In-Reply-To: <1382605964-2693-1-git-send-email-windsdaemon@gmail.com>

Hello.

On 10/24/2013 01:12 PM, Jing Wang wrote:

    s/mememry/memory/ in the subject.

> From: Jing Wang <windsdaemon@gmail.com>

> the code isn't properly release memory

    s/release/releasing/.

> Signed-off-by: Jing Wang <windsdaemon@gmail.com>

WBR, Sergei

^ permalink raw reply

* Re: [PATCH 1/1] net:sched  fix a bug about memery leak
From: Christoph Paasch @ 2013-10-24 14:07 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Jing Wang, davem, jhs, netdev
In-Reply-To: <1382607218.7572.45.camel@edumazet-glaptop.roam.corp.google.com>

On 24/10/13 - 02:33:38, Eric Dumazet wrote:
> On Thu, 2013-10-24 at 17:12 +0800, Jing Wang wrote:
> > From: Jing Wang <windsdaemon@gmail.com>
> > 
> > the code isn't properly release memory
> > 
> > Signed-off-by: Jing Wang <windsdaemon@gmail.com>
> > ---
> >  net/sched/cls_route.c |    9 ++++++---
> >  1 files changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
> > index 37da567..118f8d5 100644
> > --- a/net/sched/cls_route.c
> > +++ b/net/sched/cls_route.c
> > @@ -466,11 +466,11 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
> >  		goto reinsert;
> >  	}
> >  
> > -	err = -ENOBUFS;
> > +	err = -ENOMEM;
> >  	if (head == NULL) {
> >  		head = kzalloc(sizeof(struct route4_head), GFP_KERNEL);
> >  		if (head == NULL)
> > -			goto errout;
> > +			goto errhead;
> >  
> >  		tcf_tree_lock(tp);
> >  		tp->root = head;
> > @@ -479,7 +479,7 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
> >  
> >  	f = kzalloc(sizeof(struct route4_filter), GFP_KERNEL);
> >  	if (f == NULL)
> > -		goto errout;
> > +		goto errflt;
> >  
> >  	err = route4_set_parms(net, tp, base, f, handle, head, tb,
> >  		tca[TCA_RATE], 1);
> > @@ -517,6 +517,9 @@ reinsert:
> >  
> >  errout:
> >  	kfree(f);
> > +errflt:
> > +    kfree(head);
> > +errhead:
> >  	return err;
> >  }
> >  
> 
> I don't think this patch is needed or correct.
> 
> tp->root is the head, you cannot free it like that.
> 
> It will be freed properly in route4_destroy()
> 
> Please elaborate, thanks.

I think there is something else wrong in route4_change:

----
>From 1409402bf964bef79667755a5d0d5e0c2bd663f3 Mon Sep 17 00:00:00 2001
From: Christoph Paasch <christoph.paasch@uclouvain.be>
Date: Thu, 24 Oct 2013 15:33:28 +0200
Subject: [PATCH net] net: sched: Don't free f before it is allocated in
 route4_change

f is set to *arg in route4_change at the beginning, which points to a                                                                                                                                                                                                          
route4_filter in the hash-table (gotten through route4_get, called by
tc_ctl_filter). If the alloc of head fails, we should not goto errout,
because this will free f and thus freed memory will be referenced by
the hash-table.
Only later the pointer f will change to an allocated route4_filter.

This patch returns err if the allocation of head fails as f has not yet
been allocated inside route4_change.

Seems the code has been like this since Linus's original git-commit.

Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be>
---
 net/sched/cls_route.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index 37da567..f17c67f 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -470,7 +470,7 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
        if (head == NULL) {
                head = kzalloc(sizeof(struct route4_head), GFP_KERNEL);
                if (head == NULL)
-                       goto errout;
+                       return err;

                tcf_tree_lock(tp);
                tp->root = head;
-- 
1.8.3.2

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox