Netdev List
 help / color / mirror / Atom feed
* [net-next 1/6] e1000e: locking bug introduced by commit 67fd4fcb
From: Jeff Kirsher @ 2011-10-14  6:21 UTC (permalink / raw)
  To: davem; +Cc: Bruce Allan, netdev, gospo, sassmann
In-Reply-To: <1318573288-18286-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Bruce Allan <bruce.w.allan@intel.com>

Commit 67fd4fcb (e1000e: convert to stats64) added the ability to update
statistics more accurately and on-demand through the net_device_ops
.ndo_get_stats64 hook, but introduced a locking bug on 82577/8/9 when
linked at half-duplex (seen on kernels with CONFIG_DEBUG_ATOMIC_SLEEP=y and
CONFIG_PROVE_LOCKING=y).  The commit introduced code paths that caused a
mutex to be locked in atomic contexts, e.g. an rcu_read_lock is held when
irqbalance reads the stats from /sys/class/net/ethX/statistics causing the
mutex to be locked to read the Phy half-duplex statistics registers.

The mutex was originally introduced to prevent concurrent accesses of
resources (the NVM and Phy) shared by the driver, firmware and hardware
a few years back when there was an issue with the NVM getting corrupted.
It was later split into two mutexes - one for the NVM and one for the Phy
when it was determined the NVM, unlike the Phy, should not be protected by
the software/firmware/hardware semaphore (arbitration of which is done in
part with the SWFLAG bit in the EXTCNF_CTRL register).  This latter
semaphore should be sufficient to prevent resource contention of the Phy in
the driver (i.e. the mutex for Phy accesses is not needed), but to be sure
the mutex is replaced with an atomic bit flag which will warn if any
contention is possible.

Also add additional debug output to help determine when the sw/fw/hw
semaphore is owned by the firmware or hardware.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Reported-by: Francois Romieu <romieu@fr.zoreil.com>
Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
---
 drivers/net/ethernet/intel/e1000e/e1000.h   |    1 +
 drivers/net/ethernet/intel/e1000e/ich8lan.c |   21 +++++++++++++--------
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h
index 7877b9c..9fe18d1 100644
--- a/drivers/net/ethernet/intel/e1000e/e1000.h
+++ b/drivers/net/ethernet/intel/e1000e/e1000.h
@@ -469,6 +469,7 @@ struct e1000_info {
 enum e1000_state_t {
 	__E1000_TESTING,
 	__E1000_RESETTING,
+	__E1000_ACCESS_SHARED_RESOURCE,
 	__E1000_DOWN
 };
 
diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index 4f70974..6a17c62 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -852,8 +852,6 @@ static void e1000_release_nvm_ich8lan(struct e1000_hw *hw)
 	mutex_unlock(&nvm_mutex);
 }
 
-static DEFINE_MUTEX(swflag_mutex);
-
 /**
  *  e1000_acquire_swflag_ich8lan - Acquire software control flag
  *  @hw: pointer to the HW structure
@@ -866,7 +864,12 @@ static s32 e1000_acquire_swflag_ich8lan(struct e1000_hw *hw)
 	u32 extcnf_ctrl, timeout = PHY_CFG_TIMEOUT;
 	s32 ret_val = 0;
 
-	mutex_lock(&swflag_mutex);
+	if (test_and_set_bit(__E1000_ACCESS_SHARED_RESOURCE,
+			     &hw->adapter->state)) {
+		WARN(1, "e1000e: %s: contention for Phy access\n",
+		     hw->adapter->netdev->name);
+		return -E1000_ERR_PHY;
+	}
 
 	while (timeout) {
 		extcnf_ctrl = er32(EXTCNF_CTRL);
@@ -878,7 +881,7 @@ static s32 e1000_acquire_swflag_ich8lan(struct e1000_hw *hw)
 	}
 
 	if (!timeout) {
-		e_dbg("SW/FW/HW has locked the resource for too long.\n");
+		e_dbg("SW has already locked the resource.\n");
 		ret_val = -E1000_ERR_CONFIG;
 		goto out;
 	}
@@ -898,7 +901,9 @@ static s32 e1000_acquire_swflag_ich8lan(struct e1000_hw *hw)
 	}
 
 	if (!timeout) {
-		e_dbg("Failed to acquire the semaphore.\n");
+		e_dbg("Failed to acquire the semaphore, FW or HW has it: "
+		      "FWSM=0x%8.8x EXTCNF_CTRL=0x%8.8x)\n",
+		      er32(FWSM), extcnf_ctrl);
 		extcnf_ctrl &= ~E1000_EXTCNF_CTRL_SWFLAG;
 		ew32(EXTCNF_CTRL, extcnf_ctrl);
 		ret_val = -E1000_ERR_CONFIG;
@@ -907,7 +912,7 @@ static s32 e1000_acquire_swflag_ich8lan(struct e1000_hw *hw)
 
 out:
 	if (ret_val)
-		mutex_unlock(&swflag_mutex);
+		clear_bit(__E1000_ACCESS_SHARED_RESOURCE, &hw->adapter->state);
 
 	return ret_val;
 }
@@ -932,7 +937,7 @@ static void e1000_release_swflag_ich8lan(struct e1000_hw *hw)
 		e_dbg("Semaphore unexpectedly released by sw/fw/hw\n");
 	}
 
-	mutex_unlock(&swflag_mutex);
+	clear_bit(__E1000_ACCESS_SHARED_RESOURCE, &hw->adapter->state);
 }
 
 /**
@@ -3139,7 +3144,7 @@ static s32 e1000_reset_hw_ich8lan(struct e1000_hw *hw)
 	msleep(20);
 
 	if (!ret_val)
-		mutex_unlock(&swflag_mutex);
+		clear_bit(__E1000_ACCESS_SHARED_RESOURCE, &hw->adapter->state);
 
 	if (ctrl & E1000_CTRL_PHY_RST) {
 		ret_val = hw->phy.ops.get_cfg_done(hw);
-- 
1.7.6.4

^ permalink raw reply related

* [net-next 2/6] if_link: Add additional parameter to IFLA_VF_INFO for spoof checking
From: Jeff Kirsher @ 2011-10-14  6:21 UTC (permalink / raw)
  To: davem; +Cc: Greg Rose, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1318573288-18286-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Greg Rose <gregory.v.rose@intel.com>

Add configuration setting for drivers to turn spoof checking on or off
for discrete VFs.

v2 - Fix indentation problem, wrap the ifla_vf_info structure in
     #ifdef __KERNEL__ to prevent user space from accessing and
     change function paramater for the spoof check setting netdev
     op from u8 to bool.
v3 - Preset spoof check setting to -1 so that user space tools such
     as ip can detect that the driver didn't report a spoofcheck
     setting.  Prevents incorrect display of spoof check settings
     for drivers that don't report it.

Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 include/linux/if_link.h   |   10 ++++++++++
 include/linux/netdevice.h |    3 +++
 net/core/rtnetlink.c      |   33 ++++++++++++++++++++++++++++++---
 3 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/include/linux/if_link.h b/include/linux/if_link.h
index 0ee969a..c52d4b5 100644
--- a/include/linux/if_link.h
+++ b/include/linux/if_link.h
@@ -279,6 +279,7 @@ enum {
 	IFLA_VF_MAC,		/* Hardware queue specific attributes */
 	IFLA_VF_VLAN,
 	IFLA_VF_TX_RATE,	/* TX Bandwidth Allocation */
+	IFLA_VF_SPOOFCHK,	/* Spoof Checking on/off switch */
 	__IFLA_VF_MAX,
 };
 
@@ -300,13 +301,22 @@ struct ifla_vf_tx_rate {
 	__u32 rate; /* Max TX bandwidth in Mbps, 0 disables throttling */
 };
 
+struct ifla_vf_spoofchk {
+	__u32 vf;
+	__u32 setting;
+};
+#ifdef __KERNEL__
+
+/* We don't want this structure exposed to user space */
 struct ifla_vf_info {
 	__u32 vf;
 	__u8 mac[32];
 	__u32 vlan;
 	__u32 qos;
 	__u32 tx_rate;
+	__u32 spoofchk;
 };
+#endif
 
 /* VF ports management section
  *
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 43b3298..0db1f5f 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -781,6 +781,7 @@ struct netdev_tc_txq {
  * int (*ndo_set_vf_mac)(struct net_device *dev, int vf, u8* mac);
  * int (*ndo_set_vf_vlan)(struct net_device *dev, int vf, u16 vlan, u8 qos);
  * int (*ndo_set_vf_tx_rate)(struct net_device *dev, int vf, int rate);
+ * int (*ndo_set_vf_spoofchk)(struct net_device *dev, int vf, bool setting);
  * int (*ndo_get_vf_config)(struct net_device *dev,
  *			    int vf, struct ifla_vf_info *ivf);
  * int (*ndo_set_vf_port)(struct net_device *dev, int vf,
@@ -900,6 +901,8 @@ struct net_device_ops {
 						   int queue, u16 vlan, u8 qos);
 	int			(*ndo_set_vf_tx_rate)(struct net_device *dev,
 						      int vf, int rate);
+	int			(*ndo_set_vf_spoofchk)(struct net_device *dev,
+						       int vf, bool setting);
 	int			(*ndo_get_vf_config)(struct net_device *dev,
 						     int vf,
 						     struct ifla_vf_info *ivf);
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 39f8dd6..9083e82 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -731,7 +731,8 @@ static inline int rtnl_vfinfo_size(const struct net_device *dev)
 		size += num_vfs *
 			(nla_total_size(sizeof(struct ifla_vf_mac)) +
 			 nla_total_size(sizeof(struct ifla_vf_vlan)) +
-			 nla_total_size(sizeof(struct ifla_vf_tx_rate)));
+			 nla_total_size(sizeof(struct ifla_vf_tx_rate)) +
+			 nla_total_size(sizeof(struct ifla_vf_spoofchk)));
 		return size;
 	} else
 		return 0;
@@ -954,13 +955,27 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 			struct ifla_vf_mac vf_mac;
 			struct ifla_vf_vlan vf_vlan;
 			struct ifla_vf_tx_rate vf_tx_rate;
+			struct ifla_vf_spoofchk vf_spoofchk;
+
+			/*
+			 * Not all SR-IOV capable drivers support the
+			 * spoofcheck query.  Preset to -1 so the user
+			 * space tool can detect that the driver didn't
+			 * report anything.
+			 */
+			ivi.spoofchk = -1;
 			if (dev->netdev_ops->ndo_get_vf_config(dev, i, &ivi))
 				break;
-			vf_mac.vf = vf_vlan.vf = vf_tx_rate.vf = ivi.vf;
+			vf_mac.vf =
+				vf_vlan.vf =
+				vf_tx_rate.vf =
+				vf_spoofchk.vf = ivi.vf;
+
 			memcpy(vf_mac.mac, ivi.mac, sizeof(ivi.mac));
 			vf_vlan.vlan = ivi.vlan;
 			vf_vlan.qos = ivi.qos;
 			vf_tx_rate.rate = ivi.tx_rate;
+			vf_spoofchk.setting = ivi.spoofchk;
 			vf = nla_nest_start(skb, IFLA_VF_INFO);
 			if (!vf) {
 				nla_nest_cancel(skb, vfinfo);
@@ -968,7 +983,10 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 			}
 			NLA_PUT(skb, IFLA_VF_MAC, sizeof(vf_mac), &vf_mac);
 			NLA_PUT(skb, IFLA_VF_VLAN, sizeof(vf_vlan), &vf_vlan);
-			NLA_PUT(skb, IFLA_VF_TX_RATE, sizeof(vf_tx_rate), &vf_tx_rate);
+			NLA_PUT(skb, IFLA_VF_TX_RATE, sizeof(vf_tx_rate),
+				&vf_tx_rate);
+			NLA_PUT(skb, IFLA_VF_SPOOFCHK, sizeof(vf_spoofchk),
+				&vf_spoofchk);
 			nla_nest_end(skb, vf);
 		}
 		nla_nest_end(skb, vfinfo);
@@ -1202,6 +1220,15 @@ static int do_setvfinfo(struct net_device *dev, struct nlattr *attr)
 							      ivt->rate);
 			break;
 		}
+		case IFLA_VF_SPOOFCHK: {
+			struct ifla_vf_spoofchk *ivs;
+			ivs = nla_data(vf);
+			err = -EOPNOTSUPP;
+			if (ops->ndo_set_vf_spoofchk)
+				err = ops->ndo_set_vf_spoofchk(dev, ivs->vf,
+							       ivs->setting);
+			break;
+		}
 		default:
 			err = -EINVAL;
 			break;
-- 
1.7.6.4

^ permalink raw reply related

* [net-next 0/6][pull request] Intel Wired LAN Driver Updates
From: Jeff Kirsher @ 2011-10-14  6:21 UTC (permalink / raw)
  To: davem; +Cc: Jeff Kirsher, netdev, gospo, sassmann

The following series contains updates to e1000e, if_link, ixgbe, igbvf
and igb.  This version of the series contains the following changes:

- e1000e not sure what happened in the pull on Tuesday which has this fix
  so re-posting this fix
- igb fix for timecompare_update and enable L4 timestamping
- igbvf final conversion to ndo_fix_features
- if_link/ixgbe add spoof checking feature

The following are changes since commit 7ae60b3f3b297b7f04025c93f1cb2275c3a1dfcd:
  sky2: fix skb truesize underestimation
and are available in the git repository at
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next.git

Bruce Allan (1):
  e1000e: locking bug introduced by commit 67fd4fcb

Greg Rose (2):
  if_link: Add additional parameter to IFLA_VF_INFO for spoof checking
  ixgbe: Add new netdev op to turn spoof checking on or off per VF

Jacob Keller (2):
  igb: enable l4 timestamping for v2 event packets
  igb: fix timecompare_upate race condition

Michał Mirosław (1):
  igbvf: convert to ndo_fix_features

 drivers/net/ethernet/intel/e1000e/e1000.h      |    1 +
 drivers/net/ethernet/intel/e1000e/ich8lan.c    |   21 +++++---
 drivers/net/ethernet/intel/igb/igb_main.c      |   11 ++++-
 drivers/net/ethernet/intel/igbvf/ethtool.c     |   57 ------------------------
 drivers/net/ethernet/intel/igbvf/netdev.c      |   25 ++++++++--
 drivers/net/ethernet/intel/ixgbe/ixgbe.h       |    3 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c  |   10 +++-
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c |   48 +++++++++++++++++---
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h |    1 +
 include/linux/if_link.h                        |   10 ++++
 include/linux/netdevice.h                      |    3 +
 net/core/rtnetlink.c                           |   33 ++++++++++++-
 12 files changed, 139 insertions(+), 84 deletions(-)

-- 
1.7.6.4

^ permalink raw reply

* [PATCH] r8169: fix wrong eee setting for rlt8111evl
From: Hayes Wang @ 2011-10-14  6:14 UTC (permalink / raw)
  To: romieu; +Cc: netdev, linux-kernel, Hayes Wang

Correct the wrong parameter for setting EEE for RTL8111E-VL.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/r8169.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index c236670..27f7ebc 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -2859,7 +2859,7 @@ static void rtl8168e_2_hw_phy_config(struct rtl8169_private *tp)
 	rtl_writephy(tp, 0x1f, 0x0004);
 	rtl_writephy(tp, 0x1f, 0x0007);
 	rtl_writephy(tp, 0x1e, 0x0020);
-	rtl_w1w0_phy(tp, 0x06, 0x0000, 0x0100);
+	rtl_w1w0_phy(tp, 0x15, 0x0000, 0x0100);
 	rtl_writephy(tp, 0x1f, 0x0002);
 	rtl_writephy(tp, 0x1f, 0x0000);
 	rtl_writephy(tp, 0x0d, 0x0007);
-- 
1.7.6.2

^ permalink raw reply related

* Re: [PATCH 1/4] ipv4: Fix pmtu propagating
From: Steffen Klassert @ 2011-10-14  5:54 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20111013.135808.636341622629050648.davem@davemloft.net>

On Thu, Oct 13, 2011 at 01:58:08PM -0400, David Miller wrote:
> From: Steffen Klassert <steffen.klassert@secunet.com>
> Date: Thu, 13 Oct 2011 12:09:50 +0200
> 
> > At least it seems that raw_sendmsg() and ping_sendmsg() don't use
> > a cached route, they do the route lookup in any case. I don't see
> > where we check if we learned a new pmtu in this cases. 
> 
> A freshly looked up route should not have ->obsolete set.
> 
> That's why we don't do dst_check() in that part of the ip_output.c
> helper code you're modifying.
> 
> Please find out exactly why dst->obsolete is non-zero on a freshly
> looked up route.  It's unexpected.

Hm, on a slow path route lookup e.g. __mkroute_output() calls
rt_dst_alloc() which initializes dst->obsolete to -1. It seems
that ___dst_free() is the only function that ever changes the
initial obsolete value. After calling ___dst_free() dst->obsolete
is 2.

Btw. on a slow path route lookup, __mkroute_output() and friends
initialize the pmtu informations via rt_set_nexthop(). How do we
check if these informations are still valid if we get the route
via the routing hash cache? Do we need to check in this case?

The raw protocol uses ip4_datagram_connect() as it's connect function.
ip4_datagram_connect() uses sk_dst_set() to cache the dst_entry on
the socket, why we don't use this cached dst_entry on raw_sendmsg()
in the connected case?

^ permalink raw reply

* Re: [Bug] skb truesize does not update properly in many place
From: Eric Dumazet @ 2011-10-14  5:51 UTC (permalink / raw)
  To: roy.qing.li; +Cc: netdev
In-Reply-To: <1318571313.2533.77.camel@edumazet-laptop>

Le vendredi 14 octobre 2011 à 07:48 +0200, Eric Dumazet a écrit :

> I am now focusing in the TCP pruning effect, that we can see with WIFI
> drivers (and also with drivers using a full PAGE to store a 1500 byte
> tcp frame), when a single packet loss is happening on a session with
> large RTT.
> 

Here is what I am currently testing :

When one SKB has to be queued in out_of_order_queue,
possibly for a long time, try to reduce its truesize (by using
skb_copy_expand()) if skb->truesize is larger than
2*SKB_TRUESIZE(skb->len)

It seems to work very well, and should not happen in fast path.

Stay tuned

^ permalink raw reply

* Re: [Bug] skb truesize does not update properly in many place
From: Eric Dumazet @ 2011-10-14  5:48 UTC (permalink / raw)
  To: roy.qing.li; +Cc: netdev
In-Reply-To: <1318570381-4731-1-git-send-email-roy.qing.li@gmail.com>

Le vendredi 14 octobre 2011 à 13:33 +0800, roy.qing.li@gmail.com a
écrit :
> Hi Eric Dumazet:
> 
> I see you are correcting the wrong skb truesize, I found the
> same problem exists in many forms and many places, like:
> 
> After calling skb_fill_page_desc(), what should be updated 
> to truesize.
> 
> pskb_expand_head(), __pskb_pull_tail()... do not update the
> truesize.

All this is scheduled, but any help is appreciated.

I gave the general idea and patched some drivers, I hope other dev will
follow me in this work.

I am now focusing in the TCP pruning effect, that we can see with WIFI
drivers (and also with drivers using a full PAGE to store a 1500 byte
tcp frame), when a single packet loss is happening on a session with
large RTT.

All this truesize saga started because I was shocked by following
"netstat -s" extract on my laptop after few minutes of Internet stuff.

    848 packets collapsed in receive queue due to low socket buffer

^ permalink raw reply

* [Bug] skb truesize does not update properly in many place
From: roy.qing.li @ 2011-10-14  5:33 UTC (permalink / raw)
  To: eric.dumazet, netdev; +Cc: roy.qing.li

Hi Eric Dumazet:

I see you are correcting the wrong skb truesize, I found the
same problem exists in many forms and many places, like:

After calling skb_fill_page_desc(), what should be updated 
to truesize.

pskb_expand_head(), __pskb_pull_tail()... do not update the
truesize.

...

-RongQing.Li

^ permalink raw reply

* Re: [PATCH net-next] niu: fix skb truesize underestimation
From: David Miller @ 2011-10-14  4:34 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1318563231.2533.55.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 14 Oct 2011 05:33:51 +0200

> But then I see you also do in niu_rbr_add_page(), rigth after the
> alloc_page(), the thing I was thinking to add : (perform all needed
> get_page() in a single shot)
> 
> atomic_add(rp->rbr_blocks_per_page - 1,
> 	&compound_head(page)->_count);
> 
> So I am a bit lost here. Arent you doing too many page->_count
> increases ?

It would be pretty amazing for a leak of this magnitude to exist for
so long. :-)

A page can be split into multiple blocks, each block is some power
of two in size.

The chip splits up "blocks" into smaller (also power of two)
fragments, and these fragments are what we en-tail to the SKBs.

So at the top level we give the chip blocks.  We try to make this
equal to PAGE_SIZE.  But if PAGE_SIZE is really large we limit the
block size to 1 << 15.  Note that it is only when we enforce this
block size limit that the compount_page(page)->_count atomic increment
will occur.  As long as PAGE_SIZE <= 1 << 15, rbr_blocks_per_page
will be 1.

When the chip takes a block and starts using it, it decides which
fragment size to use for that block.  Once a fragment size has been
choosen for a block, it will not change.

The fragment sizes the chip can use is stored in rp->rbr_sizes[].  We
always configure the chip to use 256 byte and 1024 byte blocks, then
depending upon the MTU and the PAGE_SIZE we'll optionally enable other
sizes such as 2048, 4096, and 8192.

When we get an RX packet the descriptor tells us the DMA address
and the fragment size in use for the block that the memory at
DMA address belongs to.

So the two seperate page reference count grabs you see are handling
references for memory being chopped up at two different levels.

I can't see how we could optimize the intra-block refcounts any
further.  Part of the problem is that we don't know apriori what
fragment size the chip will use for a given block.

^ permalink raw reply

* [PATCH net-next] bnx2x: Disable LRO on FCoE or iSCSI boot device
From: Michael Chan @ 2011-10-14  3:38 UTC (permalink / raw)
  To: davem; +Cc: netdev, dmitry, eilong

From: Dmitry Kravkov <dmitry@broadcom.com>

For an FCoE or iSCSI boot device, the networking side must stay "up" all
the time.  Otherwise, the FCoE/iSCSI interface driven by bnx2i/bnx2fc
will be reset and we'll lose the root file system.

If LRO is enabled, scripts that enable IP forwarding or bridging will
disable LRO and cause the device to be reset.  Disabling LRO on these
boot devices will prevent the reset.

Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index 6486ab8..4960048 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -9794,6 +9794,7 @@ static int __devinit bnx2x_init_bp(struct bnx2x *bp)
 	int func;
 	int timer_interval;
 	int rc;
+	u32 cnic_boot_device;
 
 	mutex_init(&bp->port.phy_mutex);
 	mutex_init(&bp->fw_mb_mutex);
@@ -9840,8 +9841,11 @@ static int __devinit bnx2x_init_bp(struct bnx2x *bp)
 
 	bp->multi_mode = multi_mode;
 
+	cnic_boot_device =
+		!!SHMEM_RD(bp, func_mb[BP_FW_MB_IDX(bp)].iscsi_boot_signature);
+
 	/* Set TPA flags */
-	if (disable_tpa) {
+	if (disable_tpa || cnic_boot_device) {
 		bp->flags &= ~TPA_ENABLE_FLAG;
 		bp->dev->features &= ~NETIF_F_LRO;
 	} else {
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next] tcp: use TCP_INIT_CWND in tcp_fixup_sndbuf()
From: Eric Dumazet @ 2011-10-14  4:24 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

Initial cwnd being 10 (TCP_INIT_CWND) instead of 3, change
tcp_fixup_sndbuf() to get more than 16384 bytes (sysctl_tcp_wmem[1]) in
initial sk_sndbuf

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
I believe a similar change in tcp_fixup_rcvbuf() is needed too.

 net/ipv4/tcp_input.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index c1653fe..1e848b2 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -267,11 +267,9 @@ static void tcp_fixup_sndbuf(struct sock *sk)
 {
 	int sndmem = SKB_TRUESIZE(tcp_sk(sk)->rx_opt.mss_clamp + MAX_TCP_HEADER);
 
-	if (sk->sk_sndbuf < 3 * sndmem) {
-		sk->sk_sndbuf = 3 * sndmem;
-		if (sk->sk_sndbuf > sysctl_tcp_wmem[2])
-			sk->sk_sndbuf = sysctl_tcp_wmem[2];
-	}
+	sndmem *= TCP_INIT_CWND;
+	if (sk->sk_sndbuf < sndmem)
+		sk->sk_sndbuf = min(sndmem, sysctl_tcp_wmem[2]);
 }
 
 /* 2. Tuning advertised window (window_clamp, rcv_ssthresh)

^ permalink raw reply related

* Re: [PATCH net-next] ftgmac100: fix skb truesize underestimation
From: Po-Yu Chuang @ 2011-10-14  3:50 UTC (permalink / raw)
  To: David Miller; +Cc: eric.dumazet, netdev, ratbert
In-Reply-To: <20111013.222550.2221506906969662746.davem@davemloft.net>

Dear Eric and David,

On Fri, Oct 14, 2011 at 10:25 AM, David Miller <davem@davemloft.net> wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Thu, 13 Oct 2011 23:30:52 +0200
>
>> ftgmac100 allocates a page per skb fragment. We must account
>> PAGE_SIZE increments on skb->truesize, not the actual frag length.
>>
>> If frame is under 64 bytes, page is freed, and truesize adjusted.
>>
>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>
> Applied.

Thank you.

best regards,
Po-Yu Chuang

^ permalink raw reply

* Re: [PATCH net-next] niu: fix skb truesize underestimation
From: Eric Dumazet @ 2011-10-14  3:33 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20111013.222659.12182837968152363.davem@davemloft.net>

Le jeudi 13 octobre 2011 à 22:26 -0400, David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Fri, 14 Oct 2011 00:39:27 +0200
> 
> > Add a 'truesize' argument to niu_rx_skb_append(), filled with rcr_size
> > by the caller to properly account frag sizes in skb->truesize
> > 
> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> > ---
> > Please David double check this one as I am not very familiar with NIU
> > code. Thanks !
> 
> It looks perfect!  And if it's not I'll soon find out :-)
> 

Thanks !

By the way, I noticed NIU uses a get_page() every time a chunk is
attached to a skb (only the last chunk of a page is given without the
get_page())

So I thought it might incur false sharing if a previous SKB using a
chunk from same page is processed by another CPU.

But then I see you also do in niu_rbr_add_page(), rigth after the
alloc_page(), the thing I was thinking to add : (perform all needed
get_page() in a single shot)

atomic_add(rp->rbr_blocks_per_page - 1,
	&compound_head(page)->_count);

So I am a bit lost here. Arent you doing too many page->_count
increases ?

Thanks !

^ permalink raw reply

* Re: [PATCH v7 0/8] Request for inclusion: tcp memory buffers
From: Valdis.Kletnieks @ 2011-10-14  2:55 UTC (permalink / raw)
  To: Glauber Costa
  Cc: David Miller, linux-kernel, akpm, lizf, kamezawa.hiroyu, ebiederm,
	paul, gthelen, netdev, linux-mm, kirill, avagin, devel
In-Reply-To: <4E9744A6.5010101@parallels.com>

[-- Attachment #1: Type: text/plain, Size: 921 bytes --]

On Fri, 14 Oct 2011 00:05:58 +0400, Glauber Costa said:
> On 10/14/2011 12:00 AM, David Miller wrote:

> > Make this evaluate into exactly the same exact code stream we have
> > now when the memory cgroup feature is not in use, which will be the
> > majority of users.
> 
> What exactly do you mean by "not in use" ? Not compiled in or not 
> actively being exercised ? If you mean the later, I appreciate tips on 
> how to achieve it.
> 
> Also, I kind of dispute the affirmation that !cgroup will encompass
> the majority of users, since cgroups is being enabled by default by
> most vendors. All systemd based systems use it extensively, for instance.

Yes, systemd requires a kernel that includes cgroups.  However, systemd does
*not* require the memory cgroup feature.  As a practical matter, if your patch
doesn't generate equivalent code for the "have cgroups, but no memory cgroup"
situation, it's a non-starter.

[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply

* Re: [PATCH net-next] ftmac100: fix skb truesize underestimation
From: David Miller @ 2011-10-14  2:28 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, ratbert
In-Reply-To: <1318540808.2533.27.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 13 Oct 2011 23:20:08 +0200

> ftmac100 allocates a page per skb fragment. We must account
> PAGE_SIZE increments on skb->truesize, not the actual frag length.
> 
> If frame is under 64 bytes, page is freed, so increase truesize only for
> bigger frames.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] niu: fix skb truesize underestimation
From: David Miller @ 2011-10-14  2:26 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1318545567.2533.46.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 14 Oct 2011 00:39:27 +0200

> Add a 'truesize' argument to niu_rx_skb_append(), filled with rcr_size
> by the caller to properly account frag sizes in skb->truesize
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> Please David double check this one as I am not very familiar with NIU
> code. Thanks !

It looks perfect!  And if it's not I'll soon find out :-)

Applied.

^ permalink raw reply

* Re: [PATCH net-next] vmxnet3: fix skb truesize underestimation
From: David Miller @ 2011-10-14  2:26 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, sbhatewara
In-Reply-To: <1318541897.2533.33.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 13 Oct 2011 23:38:17 +0200

> vmxnet3 allocates a page per skb fragment. We must account
> PAGE_SIZE increments on skb->truesize, not the actual frag length.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] ftgmac100: fix skb truesize underestimation
From: David Miller @ 2011-10-14  2:25 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, ratbert
In-Reply-To: <1318541452.2533.30.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 13 Oct 2011 23:30:52 +0200

> ftgmac100 allocates a page per skb fragment. We must account
> PAGE_SIZE increments on skb->truesize, not the actual frag length.
> 
> If frame is under 64 bytes, page is freed, and truesize adjusted.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH v7 0/8] Request for inclusion: tcp memory buffers
From: Andi Kleen @ 2011-10-14  2:12 UTC (permalink / raw)
  To: David Miller
  Cc: glommer, linux-kernel, akpm, lizf, kamezawa.hiroyu, ebiederm,
	paul, gthelen, netdev, linux-mm, kirill, avagin, devel
In-Reply-To: <20111013.160031.605700447623532119.davem@davemloft.net>

David Miller <davem@davemloft.net> writes:
>
> Make this evaluate into exactly the same exact code stream we have
> now when the memory cgroup feature is not in use, which will be the
> majority of users.

One possible way may be to guard it with static_branch() for 
no limit per cgroup set. That should be as near as practically
possible to the original code.

BTW the thing that usually worries me more is the cache line behaviour
when the feature is in use. In the past some of the namespace patches
have created some extremly hot global cache lines, that hurt on larger
systems (for example the Unix socket regression from uid namespaces that
is still not completly fixed). It would be good to double check that all
important state is distributed properly.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [net-next PATCH] net: allow vlan traffic to be received under bond
From: Jesse Gross @ 2011-10-14  0:22 UTC (permalink / raw)
  To: John Fastabend
  Cc: Hans Schillström, Jiri Pirko, Maxime Bizon,
	davem@davemloft.net, netdev@vger.kernel.org, fubar@us.ibm.com
In-Reply-To: <4E972301.4030004@intel.com>

2011/10/13 John Fastabend <john.r.fastabend@intel.com>:
> On 10/13/2011 8:59 AM, Hans Schillström wrote:
>> Thu, Oct 13, 2011 at 05:04:34PM CEST, mbizon@freebox.fr wrote:
>>>>
>>>> On Tue, 2011-10-11 at 00:37 +0200, Jiri Pirko wrote:
>>>>
>>>>> Hmm, I must look at this again tomorrow but I have strong feeling this
>>>>> will break some some scenario including vlan-bridge-macvlan.
>>>>
>>>> unless I'm mistaken, today's behaviour:
>>>>
>>>> # vconfig add eth0 100
>>>> # brctl addbr br0
>>>> # brctl addif br0 eth0
>>>>
>>>> => eth0.100 gets no more packets, br0.100 is to be used
>>>>
>>>> after the patch won't we get the opposite ?
>>>
>>> Looks like it. The question is what is the correct behaviour...
>>
>> I think this it become correct now, you should not destroy lover level if possible.
>> I.e. as John wrote "it's not an unexpected behaviour"
>>
>> Consider adding a bridge to a vlan like this
>>
>> vconfig add eth0 100
>> brctl addbr br1
>> brctl addif br1 eth0.100
>>
>> If you later add a bridge (or bond) should the previous added bridge still work ?
>> Yes I think so, for me it's the expected behaviour.
>>
>> brctl addbr br0
>> brctl addif br0 eth0
>>
>> Regards
>> Hans
>>
>>
>
> Sorry I'm not entirely sure I followed the above two posts. Note
> this patch restores behavior that has existed for most of the
> 2.6.x kernels. In the 3.x kernels VLAN100 is dropped in the
> schematic below (assuming eth0 is active), I think this is
> incorrect.

Actually, for most of 2.6.x the behavior was somewhat
non-deterministic since it depended on kernel version and the NIC.  As
a result, I think we can safely say that this wasn't a particularly
firm interface that we have to be wedded to.  Based on overwhelming
feedback, I think the interface in this patch is the preferred one and
what we should stabilize on.

^ permalink raw reply

* Re: [PATCH] net: ipv6: Allow netlink to set IPv6 address scope
From: Lorenzo Colitti @ 2011-10-13 23:55 UTC (permalink / raw)
  To: Brian Haley; +Cc: maze, yoshfuji, netdev
In-Reply-To: <4E931A46.1040905@hp.com>

On Mon, Oct 10, 2011 at 09:16, Brian Haley <brian.haley@hp.com> wrote:
> > net: ipv6: Allow netlink to set IPv6 address scope
> >
> > Currently, userspace cannot specify the scope of IPv6
> > addresses when creating or modifying them. Instead, the
> > scope is automatically determined from the address itself.
> > In IPv4, userspace can set whatever scope it likes.
> >
> > Allow userspace to specify the scope of IPv6 addresses in
> > a backwards-compatible way: if the scope passed in is zero,
> > use the old behaviour of automatically determining the
> > scope based on the address.
> >
> > Signed-off-by: Lorenzo Colitti <lorenzo@google.com>
>
> Hi Lorenzo,
>
> I remember someone proposing a similar patch before and it was not accepted, do you have a use case for doing this?  It just seems like it will cause problems.

Well, to begin with it's a question of feature parity. If we allow
users to set the scope of IPv4 addresses, then we should allow them to
set the scope of IPv6 addresses as well. Policy belongs in userspace,
not in the kernel.

One use case is as follows. I have a phone with an always-on IPv6
interface which is used for signaling, provisioning, SMS, etc. This
IPv6 address is a global unicast IPv6 address, but it belongs to a
closed carrier network, and cannot be used to communicate with the
IPv6 Internet in either direction.

The phone also supports wifi. When there is IPv6 on the wifi
interface, things get messy because the phone can decide to use the
carrier source address on the wifi interface. This will not work,
because the replies will be dropped in the carrier network.

You can stop this from happening in most cases by not putting a
default route on the walled garden interface, and only using host
routes as needed. Unfortunately, you can't stop it from happening in
at least the following two cases:

1. You receive an IPv6 RA on the wifi interface which provides a
default route and a 6to4 address, or a default route and no IPv6
address (for example, because the network uses DHCPv6 and not
RFC4862-style autoconf)

2. You receive an IPv6 RA on the wifi interface which provides a
default route and a "native" IPv6 address, but the phone attempts a
connection while the address is in the tentative state (i.e., the
phone is performing DAD on the address)

In both cases, ipv6_get_saddr_eval will return the carrier IPv6
address (because "avoid tentative and optimistic addresses" takes
precedence over "prefer outgoing interface" in RFC3484) and the kernel
will pick the default route on the wifi interface. The return packets
will get dropped and the connection will time out after several
minutes.
The only way I can think of to get this right is to set the carrier
IPv6 address to a scope less than global - which, in effect, it is,
because it can't reach the Internet.

> Also, there are other parts of the kernel (NFS, SCTP, IPv6 multicast) that are still calling ipv6_addr_scope() on a plain address - won't those be broken since they'll return the correct, RFC-implied scope?

Good point. I looked at these and don't think there is a serious problem though:

- SCTP doesn't look at the scope in IPv4 either, it just looks at the
address itself. So at worse this change will make IPv6 match IPv4.

- NFS only looks at the scope to check whether it's link-local, and if
so only declares an address to be unique if the scopes match. In this
case I think it's the right thing to do, because it's really the
network that decides whether an address can be duplicate on different
links, not the host.

- Unicast and multicast do it when dumping the addresses to userspace.
I need to fix these.

Does that make sense?

^ permalink raw reply

* [PATCH net-next] niu: fix skb truesize underestimation
From: Eric Dumazet @ 2011-10-13 22:39 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

Add a 'truesize' argument to niu_rx_skb_append(), filled with rcr_size
by the caller to properly account frag sizes in skb->truesize

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
Please David double check this one as I am not very familiar with NIU
code. Thanks !

 drivers/net/ethernet/sun/niu.c |   12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/sun/niu.c b/drivers/net/ethernet/sun/niu.c
index d133888..23740e8 100644
--- a/drivers/net/ethernet/sun/niu.c
+++ b/drivers/net/ethernet/sun/niu.c
@@ -3287,17 +3287,13 @@ static u16 tcam_get_valid_entry_cnt(struct niu *np)
 }
 
 static void niu_rx_skb_append(struct sk_buff *skb, struct page *page,
-			      u32 offset, u32 size)
+			      u32 offset, u32 size, u32 truesize)
 {
-	int i = skb_shinfo(skb)->nr_frags;
-
-	__skb_fill_page_desc(skb, i, page, offset, size);
+	skb_fill_page_desc(skb, skb_shinfo(skb)->nr_frags, page, offset, size);
 
 	skb->len += size;
 	skb->data_len += size;
-	skb->truesize += size;
-
-	skb_shinfo(skb)->nr_frags = i + 1;
+	skb->truesize += truesize;
 }
 
 static unsigned int niu_hash_rxaddr(struct rx_ring_info *rp, u64 a)
@@ -3480,7 +3476,7 @@ static int niu_process_rx_pkt(struct napi_struct *napi, struct niu *np,
 		} else if (!(val & RCR_ENTRY_MULTI))
 			append_size = len - skb->len;
 
-		niu_rx_skb_append(skb, page, off, append_size);
+		niu_rx_skb_append(skb, page, off, append_size, rcr_size);
 		if ((page->index + rp->rbr_block_size) - rcr_size == addr) {
 			*link = (struct page *) page->mapping;
 			np->ops->unmap_page(np->device, page->index,

^ permalink raw reply related

* [PATCH 8/8] caif-hsi: Added recovery check of CA wake status.
From: Sjur Brændeland @ 2011-10-13 21:29 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: dmitry.tarnyagin, daniel.martensson, Sjur Brændeland
In-Reply-To: <1318541369-8141-1-git-send-email-sjur.brandeland@stericsson.com>

From: Daniel Martensson <daniel.martensson@stericsson.com>

Added recovery check of CA wake status in case of wake up timeout.
Added check of CA wake status in case of wake down timeout.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 drivers/net/caif/caif_hsi.c |   42 ++++++++++++++++++++++++++++++++++++++++--
 include/net/caif/caif_hsi.h |    1 +
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/drivers/net/caif/caif_hsi.c b/drivers/net/caif/caif_hsi.c
index e9e7cbf..0733525 100644
--- a/drivers/net/caif/caif_hsi.c
+++ b/drivers/net/caif/caif_hsi.c
@@ -674,6 +674,7 @@ static void cfhsi_wake_up(struct work_struct *work)
 		/* It happenes when wakeup is requested by
 		 * both ends at the same time. */
 		clear_bit(CFHSI_WAKE_UP, &cfhsi->bits);
+		clear_bit(CFHSI_WAKE_UP_ACK, &cfhsi->bits);
 		return;
 	}
 
@@ -690,19 +691,47 @@ static void cfhsi_wake_up(struct work_struct *work)
 							&cfhsi->bits), ret);
 	if (unlikely(ret < 0)) {
 		/* Interrupted by signal. */
-		dev_info(&cfhsi->ndev->dev, "%s: Signalled: %ld.\n",
+		dev_err(&cfhsi->ndev->dev, "%s: Signalled: %ld.\n",
 			__func__, ret);
+
 		clear_bit(CFHSI_WAKE_UP, &cfhsi->bits);
 		cfhsi->dev->cfhsi_wake_down(cfhsi->dev);
 		return;
 	} else if (!ret) {
+		bool ca_wake = false;
+		size_t fifo_occupancy = 0;
+
 		/* Wakeup timeout */
 		dev_err(&cfhsi->ndev->dev, "%s: Timeout.\n",
 			__func__);
+
+		/* Check FIFO to check if modem has sent something. */
+		WARN_ON(cfhsi->dev->cfhsi_fifo_occupancy(cfhsi->dev,
+					&fifo_occupancy));
+
+		dev_err(&cfhsi->ndev->dev, "%s: Bytes in FIFO: %u.\n",
+				__func__, (unsigned) fifo_occupancy);
+
+		/* Check if we misssed the interrupt. */
+		WARN_ON(cfhsi->dev->cfhsi_get_peer_wake(cfhsi->dev,
+							&ca_wake));
+
+		if (ca_wake) {
+			dev_err(&cfhsi->ndev->dev, "%s: CA Wake missed !.\n",
+				__func__);
+
+			/* Clear the CFHSI_WAKE_UP_ACK bit to prevent race. */
+			clear_bit(CFHSI_WAKE_UP_ACK, &cfhsi->bits);
+
+			/* Continue execution. */
+			goto wake_ack;
+		}
+
 		clear_bit(CFHSI_WAKE_UP, &cfhsi->bits);
 		cfhsi->dev->cfhsi_wake_down(cfhsi->dev);
 		return;
 	}
+wake_ack:
 	dev_dbg(&cfhsi->ndev->dev, "%s: Woken.\n",
 		__func__);
 
@@ -779,12 +808,21 @@ static void cfhsi_wake_down(struct work_struct *work)
 							&cfhsi->bits), ret);
 	if (ret < 0) {
 		/* Interrupted by signal. */
-		dev_info(&cfhsi->ndev->dev, "%s: Signalled: %ld.\n",
+		dev_err(&cfhsi->ndev->dev, "%s: Signalled: %ld.\n",
 			__func__, ret);
 		return;
 	} else if (!ret) {
+		bool ca_wake = true;
+
 		/* Timeout */
 		dev_err(&cfhsi->ndev->dev, "%s: Timeout.\n", __func__);
+
+		/* Check if we misssed the interrupt. */
+		WARN_ON(cfhsi->dev->cfhsi_get_peer_wake(cfhsi->dev,
+							&ca_wake));
+		if (!ca_wake)
+			dev_err(&cfhsi->ndev->dev, "%s: CA Wake missed !.\n",
+				__func__);
 	}
 
 	/* Check FIFO occupancy. */
diff --git a/include/net/caif/caif_hsi.h b/include/net/caif/caif_hsi.h
index 3356769..8d55251 100644
--- a/include/net/caif/caif_hsi.h
+++ b/include/net/caif/caif_hsi.h
@@ -108,6 +108,7 @@ struct cfhsi_dev {
 	int (*cfhsi_rx) (u8 *ptr, int len, struct cfhsi_dev *dev);
 	int (*cfhsi_wake_up) (struct cfhsi_dev *dev);
 	int (*cfhsi_wake_down) (struct cfhsi_dev *dev);
+	int (*cfhsi_get_peer_wake) (struct cfhsi_dev *dev, bool *status);
 	int (*cfhsi_fifo_occupancy)(struct cfhsi_dev *dev, size_t *occupancy);
 	int (*cfhsi_rx_cancel)(struct cfhsi_dev *dev);
 	struct cfhsi_drv *drv;
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH 7/8] caif-hsi: Added sanity check for length of CAIF frames
From: Sjur Brændeland @ 2011-10-13 21:29 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: dmitry.tarnyagin, daniel.martensson, Sjur Brændeland
In-Reply-To: <1318541369-8141-1-git-send-email-sjur.brandeland@stericsson.com>

From: Daniel Martensson <daniel.martensson@stericsson.com>

Added sanity check for length of CAIF frames, and tear down of
CAIF link-layer device upon protocol error.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 drivers/net/caif/caif_hsi.c |   79 ++++++++++++++++++++++++++----------------
 include/net/caif/caif_hsi.h |    6 ++-
 2 files changed, 53 insertions(+), 32 deletions(-)

diff --git a/drivers/net/caif/caif_hsi.c b/drivers/net/caif/caif_hsi.c
index 1e1f0a3..e9e7cbf 100644
--- a/drivers/net/caif/caif_hsi.c
+++ b/drivers/net/caif/caif_hsi.c
@@ -18,6 +18,7 @@
 #include <linux/sched.h>
 #include <linux/if_arp.h>
 #include <linux/timer.h>
+#include <linux/rtnetlink.h>
 #include <net/caif/caif_layer.h>
 #include <net/caif/caif_hsi.h>
 
@@ -348,8 +349,7 @@ static void cfhsi_tx_done_cb(struct cfhsi_drv *drv)
 	cfhsi_tx_done(cfhsi);
 }
 
-static int cfhsi_rx_desc(struct cfhsi_desc *desc, struct cfhsi *cfhsi,
-				bool *dump)
+static int cfhsi_rx_desc(struct cfhsi_desc *desc, struct cfhsi *cfhsi)
 {
 	int xfer_sz = 0;
 	int nfrms = 0;
@@ -360,8 +360,7 @@ static int cfhsi_rx_desc(struct cfhsi_desc *desc, struct cfhsi *cfhsi,
 			(desc->offset > CFHSI_MAX_EMB_FRM_SZ)) {
 		dev_err(&cfhsi->ndev->dev, "%s: Invalid descriptor.\n",
 			__func__);
-		*dump = true;
-		return 0;
+		return -EPROTO;
 	}
 
 	/* Check for embedded CAIF frame. */
@@ -379,6 +378,12 @@ static int cfhsi_rx_desc(struct cfhsi_desc *desc, struct cfhsi *cfhsi,
 		len |= ((*(pfrm+1)) << 8) & 0xFF00;
 		len += 2;	/* Add FCS fields. */
 
+		/* Sanity check length of CAIF frame. */
+		if (unlikely(len > CFHSI_MAX_CAIF_FRAME_SZ)) {
+			dev_err(&cfhsi->ndev->dev, "%s: Invalid length.\n",
+				__func__);
+			return -EPROTO;
+		}
 
 		/* Allocate SKB (OK even in IRQ context). */
 		skb = alloc_skb(len + 1, GFP_ATOMIC);
@@ -423,18 +428,16 @@ static int cfhsi_rx_desc(struct cfhsi_desc *desc, struct cfhsi *cfhsi,
 	if (desc->header & CFHSI_PIGGY_DESC)
 		xfer_sz += CFHSI_DESC_SZ;
 
-	if (xfer_sz % 4) {
+	if ((xfer_sz % 4) || (xfer_sz > (CFHSI_BUF_SZ_RX - CFHSI_DESC_SZ))) {
 		dev_err(&cfhsi->ndev->dev,
 				"%s: Invalid payload len: %d, ignored.\n",
 			__func__, xfer_sz);
-		xfer_sz = 0;
-		*dump = true;
+		return -EPROTO;
 	}
 	return xfer_sz;
 }
 
-static int cfhsi_rx_pld(struct cfhsi_desc *desc, struct cfhsi *cfhsi,
-				bool *dump)
+static int cfhsi_rx_pld(struct cfhsi_desc *desc, struct cfhsi *cfhsi)
 {
 	int rx_sz = 0;
 	int nfrms = 0;
@@ -446,8 +449,7 @@ static int cfhsi_rx_pld(struct cfhsi_desc *desc, struct cfhsi *cfhsi,
 			(desc->offset > CFHSI_MAX_EMB_FRM_SZ))) {
 		dev_err(&cfhsi->ndev->dev, "%s: Invalid descriptor.\n",
 			__func__);
-		*dump = true;
-		return -EINVAL;
+		return -EPROTO;
 	}
 
 	/* Set frame pointer to start of payload. */
@@ -469,13 +471,6 @@ static int cfhsi_rx_pld(struct cfhsi_desc *desc, struct cfhsi *cfhsi,
 		u8 *pcffrm = NULL;
 		int len = 0;
 
-		if (WARN_ON(desc->cffrm_len[nfrms] > CFHSI_MAX_PAYLOAD_SZ)) {
-			dev_err(&cfhsi->ndev->dev, "%s: Invalid payload.\n",
-				__func__);
-			*dump = true;
-			return -EINVAL;
-		}
-
 		/* CAIF frame starts after head padding. */
 		pcffrm = pfrm + *pfrm + 1;
 
@@ -484,6 +479,13 @@ static int cfhsi_rx_pld(struct cfhsi_desc *desc, struct cfhsi *cfhsi,
 		len |= ((*(pcffrm + 1)) << 8) & 0xFF00;
 		len += 2;	/* Add FCS fields. */
 
+		/* Sanity check length of CAIF frames. */
+		if (unlikely(len > CFHSI_MAX_CAIF_FRAME_SZ)) {
+			dev_err(&cfhsi->ndev->dev, "%s: Invalid length.\n",
+				__func__);
+			return -EPROTO;
+		}
+
 		/* Allocate SKB (OK even in IRQ context). */
 		skb = alloc_skb(len + 1, GFP_ATOMIC);
 		if (!skb) {
@@ -528,7 +530,6 @@ static void cfhsi_rx_done(struct cfhsi *cfhsi)
 	int res;
 	int desc_pld_len = 0;
 	struct cfhsi_desc *desc = NULL;
-	bool dump = false;
 
 	desc = (struct cfhsi_desc *)cfhsi->rx_buf;
 
@@ -544,16 +545,20 @@ static void cfhsi_rx_done(struct cfhsi *cfhsi)
 	spin_unlock_bh(&cfhsi->lock);
 
 	if (cfhsi->rx_state.state == CFHSI_RX_STATE_DESC) {
-		desc_pld_len = cfhsi_rx_desc(desc, cfhsi, &dump);
+		desc_pld_len = cfhsi_rx_desc(desc, cfhsi);
 		if (desc_pld_len == -ENOMEM)
 			goto restart;
+		if (desc_pld_len == -EPROTO)
+			goto out_of_sync;
 	} else {
 		int pld_len;
 
 		if (!cfhsi->rx_state.piggy_desc) {
-			pld_len = cfhsi_rx_pld(desc, cfhsi, &dump);
+			pld_len = cfhsi_rx_pld(desc, cfhsi);
 			if (pld_len == -ENOMEM)
 				goto restart;
+			if (pld_len == -EPROTO)
+				goto out_of_sync;
 			cfhsi->rx_state.pld_len = pld_len;
 		} else {
 			pld_len = cfhsi->rx_state.pld_len;
@@ -567,7 +572,7 @@ static void cfhsi_rx_done(struct cfhsi *cfhsi)
 			cfhsi->rx_state.piggy_desc = true;
 
 			/* Extract piggy-backed descriptor. */
-			desc_pld_len = cfhsi_rx_desc(piggy_desc, cfhsi, &dump);
+			desc_pld_len = cfhsi_rx_desc(piggy_desc, cfhsi);
 			if (desc_pld_len == -ENOMEM)
 				goto restart;
 
@@ -577,15 +582,10 @@ static void cfhsi_rx_done(struct cfhsi *cfhsi)
 			 */
 			memcpy((u8 *)desc, (u8 *)piggy_desc,
 					CFHSI_DESC_SHORT_SZ);
-		}
-	}
 
-	if (unlikely(dump)) {
-		size_t rx_offset = cfhsi->rx_ptr - cfhsi->rx_buf;
-		dev_err(&cfhsi->ndev->dev, "%s: RX offset: %u.\n",
-			__func__, (unsigned) rx_offset);
-		print_hex_dump_bytes("--> ", DUMP_PREFIX_NONE,
-				cfhsi->rx_buf, cfhsi->rx_len + rx_offset);
+			if (desc_pld_len == -EPROTO)
+				goto out_of_sync;
+		}
 	}
 
 	memset(&cfhsi->rx_state, 0, sizeof(cfhsi->rx_state));
@@ -622,6 +622,13 @@ restart:
 		BUG();
 	}
 	mod_timer(&cfhsi->rx_slowpath_timer, jiffies + 1);
+	return;
+
+out_of_sync:
+	dev_err(&cfhsi->ndev->dev, "%s: Out of sync.\n", __func__);
+	print_hex_dump_bytes("--> ", DUMP_PREFIX_NONE,
+			cfhsi->rx_buf, CFHSI_DESC_SZ);
+	schedule_work(&cfhsi->out_of_sync_work);
 }
 
 static void cfhsi_rx_slowpath(unsigned long arg)
@@ -804,6 +811,17 @@ static void cfhsi_wake_down(struct work_struct *work)
 
 }
 
+static void cfhsi_out_of_sync(struct work_struct *work)
+{
+	struct cfhsi *cfhsi = NULL;
+
+	cfhsi = container_of(work, struct cfhsi, out_of_sync_work);
+
+	rtnl_lock();
+	dev_close(cfhsi->ndev);
+	rtnl_unlock();
+}
+
 static void cfhsi_wake_up_cb(struct cfhsi_drv *drv)
 {
 	struct cfhsi *cfhsi = NULL;
@@ -1023,6 +1041,7 @@ int cfhsi_probe(struct platform_device *pdev)
 	/* Initialize the work queues. */
 	INIT_WORK(&cfhsi->wake_up_work, cfhsi_wake_up);
 	INIT_WORK(&cfhsi->wake_down_work, cfhsi_wake_down);
+	INIT_WORK(&cfhsi->out_of_sync_work, cfhsi_out_of_sync);
 
 	/* Clear all bit fields. */
 	clear_bit(CFHSI_WAKE_UP_ACK, &cfhsi->bits);
diff --git a/include/net/caif/caif_hsi.h b/include/net/caif/caif_hsi.h
index 9b69d15..3356769 100644
--- a/include/net/caif/caif_hsi.h
+++ b/include/net/caif/caif_hsi.h
@@ -52,8 +52,9 @@ struct cfhsi_desc {
 /*
  * Maximum bytes transferred in one transfer.
  */
-/* TODO: 4096 is temporary... */
-#define CFHSI_MAX_PAYLOAD_SZ (CFHSI_MAX_PKTS * 4096)
+#define CFHSI_MAX_CAIF_FRAME_SZ 4096
+
+#define CFHSI_MAX_PAYLOAD_SZ (CFHSI_MAX_PKTS * CFHSI_MAX_CAIF_FRAME_SZ)
 
 /* Size of the complete HSI TX buffer. */
 #define CFHSI_BUF_SZ_TX (CFHSI_DESC_SZ + CFHSI_MAX_PAYLOAD_SZ)
@@ -143,6 +144,7 @@ struct cfhsi {
 	struct list_head list;
 	struct work_struct wake_up_work;
 	struct work_struct wake_down_work;
+	struct work_struct out_of_sync_work;
 	struct workqueue_struct *wq;
 	wait_queue_head_t wake_up_wait;
 	wait_queue_head_t wake_down_wait;
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH 5/8] caif-hsi: HSI-Platform device register and unregisters itself
From: Sjur Brændeland @ 2011-10-13 21:29 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: dmitry.tarnyagin, daniel.martensson, Sjur Brændeland
In-Reply-To: <1318541369-8141-1-git-send-email-sjur.brandeland@stericsson.com>

From: Daniel Martensson <daniel.martensson@stericsson.com>

Platform device is no longer removed from caif_hsi at shutdown.
The HSI-platform device must do it's own registration and unregistration.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 drivers/net/caif/caif_hsi.c |   16 ++++------------
 1 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/net/caif/caif_hsi.c b/drivers/net/caif/caif_hsi.c
index 478b025..f46ab4d 100644
--- a/drivers/net/caif/caif_hsi.c
+++ b/drivers/net/caif/caif_hsi.c
@@ -1083,7 +1083,7 @@ int cfhsi_probe(struct platform_device *pdev)
 	return res;
 }
 
-static void cfhsi_shutdown(struct cfhsi *cfhsi, bool remove_platform_dev)
+static void cfhsi_shutdown(struct cfhsi *cfhsi)
 {
 	u8 *tx_buf, *rx_buf;
 
@@ -1093,14 +1093,6 @@ static void cfhsi_shutdown(struct cfhsi *cfhsi, bool remove_platform_dev)
 	/* going to shutdown driver */
 	set_bit(CFHSI_SHUTDOWN, &cfhsi->bits);
 
-	if (remove_platform_dev) {
-		/* Flush workqueue */
-		flush_workqueue(cfhsi->wq);
-
-		/* Notify device. */
-		platform_device_unregister(cfhsi->pdev);
-	}
-
 	/* Flush workqueue */
 	flush_workqueue(cfhsi->wq);
 
@@ -1111,7 +1103,7 @@ static void cfhsi_shutdown(struct cfhsi *cfhsi, bool remove_platform_dev)
 	/* Cancel pending RX request (if any) */
 	cfhsi->dev->cfhsi_rx_cancel(cfhsi->dev);
 
-	/* Flush again and destroy workqueue */
+	/* Destroy workqueue */
 	destroy_workqueue(cfhsi->wq);
 
 	/* Store bufferes: will be freed later. */
@@ -1150,7 +1142,7 @@ int cfhsi_remove(struct platform_device *pdev)
 			spin_unlock(&cfhsi_list_lock);
 
 			/* Shutdown driver. */
-			cfhsi_shutdown(cfhsi, false);
+			cfhsi_shutdown(cfhsi);
 
 			return 0;
 		}
@@ -1183,7 +1175,7 @@ static void __exit cfhsi_exit_module(void)
 		spin_unlock(&cfhsi_list_lock);
 
 		/* Shutdown driver. */
-		cfhsi_shutdown(cfhsi, true);
+		cfhsi_shutdown(cfhsi);
 
 		spin_lock(&cfhsi_list_lock);
 	}
-- 
1.7.0.4

^ 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