Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next] igb: Add support for byte queue limits.
From: David Miller @ 2011-12-21 19:42 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, jeffrey.t.kirsher, alexander.h.duyck
In-Reply-To: <1324480695.2301.4.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 21 Dec 2011 16:18:15 +0100

> This adds support for byte queue limits (BQL)
> 
> Since this driver collects bytes count in 'bytecount' field, use it also
> in igb_tx_map()
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> CC: Alexander Duyck <alexander.h.duyck@intel.com>

Intel folks, you got this?

^ permalink raw reply

* Re: [PATCH] rps: fix insufficient bounds checking in store_rps_dev_flow_table_cnt()
From: David Miller @ 2011-12-21 19:41 UTC (permalink / raw)
  To: eric.dumazet; +Cc: xi.wang, therbert, netdev
In-Reply-To: <1324495344.2621.5.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 21 Dec 2011 20:22:24 +0100

> Le mercredi 21 décembre 2011 à 13:50 -0500, Xi Wang a écrit :
>> @@ -665,7 +665,7 @@ static ssize_t store_rps_dev_flow_table_cnt(struct netdev_rx_queue *queue,
>>  	if (count) {
>>  		int i;
>>  
>> -		if (count > 1<<30) {
>> +		if (count > 1<<28) {
>>  			/* Enforce a limit to prevent overflow */
>>  			return -EINVAL;
>>  		}
> 
> 
> Really, you should remove this magic number and use instead
> 
> (INT_MAX - RPS_DEV_FLOW_TABLE_SIZE(0)) / sizeof(struct rps_dev_flow)
> 
> Or something like that, because next time we add a field in
> rps_dev_flow, test will be obsolete.

Agreed.

^ permalink raw reply

* Re: pull request: wireless 2011-12-21
From: David Miller @ 2011-12-21 19:38 UTC (permalink / raw)
  To: linville-2XuSBdqkA4R54TAoqtyWWQ
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20111221185914.GD11842-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>

From: "John W. Linville" <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
Date: Wed, 21 Dec 2011 13:59:14 -0500

> A few last(?) fixes intended for 3.2...
> 
> The biggest portion are from Gustavo:
> 
> "3 fixes for 3.2! There one revert that was causing connection issues,
> a RFCOMM fix to a potential kernel panic, and the last on L2CAP to
> a possible access to uninitialized data due to broken remote devices."
> 
> Also, an iwlwifi fix for a problem causing firmware asserts.
> 
> Please let me know if there are problems!

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

^ permalink raw reply

* Re: [PATCH 0/4] skb paged fragment destructors
From: David Miller @ 2011-12-21 19:28 UTC (permalink / raw)
  To: eric.dumazet; +Cc: Ian.Campbell, jesse.brandeburg, netdev
In-Reply-To: <1324476138.2728.65.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 21 Dec 2011 15:02:18 +0100

> No idea on this +2 point.

I think I know, and I believe I instructed Alexey Kuznetsov to do
this.

When sendfile() is performed, we might start the SKB with the last few
bytes of one page, and end the SKB with the first few bytes of another
page.

In order to fit a full 64K frame into an SKB in this situation we have
to accomodate this case.

^ permalink raw reply

* Re: [PATCH] rps: fix insufficient bounds checking in store_rps_dev_flow_table_cnt()
From: Eric Dumazet @ 2011-12-21 19:22 UTC (permalink / raw)
  To: Xi Wang; +Cc: Tom Herbert, David S. Miller, netdev
In-Reply-To: <1324493459-19764-1-git-send-email-xi.wang@gmail.com>

Le mercredi 21 décembre 2011 à 13:50 -0500, Xi Wang a écrit :
> Setting a large rps_flow_cnt like 1073741824 (1 << 30) on 32-bit
> platform will cause a kernel oops due to insufficient bounds checking.
> 
> 	if (count > 1<<30) {
> 		/* Enforce a limit to prevent overflow */
> 		return -EINVAL;
> 	}
> 	count = roundup_pow_of_two(count);
> 	table = vmalloc(RPS_DEV_FLOW_TABLE_SIZE(count));
> 
> Note that the macro RPS_DEV_FLOW_TABLE_SIZE(count) is defined as:
> 
> 	... + (count * sizeof(struct rps_dev_flow))
> 
> where sizeof(struct rps_dev_flow) is 8.  (1 << 30) * 8 will overflow
> 32 bits.  This patch changes the upper bound to (1 << 28).
> 
> Signed-off-by: Xi Wang <xi.wang@gmail.com>
> ---
>  net/core/net-sysfs.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
> index c71c434..f53a947 100644
> --- a/net/core/net-sysfs.c
> +++ b/net/core/net-sysfs.c
> @@ -665,7 +665,7 @@ static ssize_t store_rps_dev_flow_table_cnt(struct netdev_rx_queue *queue,
>  	if (count) {
>  		int i;
>  
> -		if (count > 1<<30) {
> +		if (count > 1<<28) {
>  			/* Enforce a limit to prevent overflow */
>  			return -EINVAL;
>  		}


Really, you should remove this magic number and use instead

(INT_MAX - RPS_DEV_FLOW_TABLE_SIZE(0)) / sizeof(struct rps_dev_flow)

Or something like that, because next time we add a field in
rps_dev_flow, test will be obsolete.

^ permalink raw reply

* Re: [PATCH V2] IPv6 : add multicast routing verify which net_device is lo
From: David Miller @ 2011-12-21 19:11 UTC (permalink / raw)
  To: wangxingtong; +Cc: gaofeng, netdev
In-Reply-To: <4EF1A061.5030005@cn.fujitsu.com>

From: Wang Xingtong <wangxingtong@cn.fujitsu.com>
Date: Wed, 21 Dec 2011 17:01:21 +0800

> OK, David, I reproduce this as following :
> 
> 1) ip -6 route show | grep ff00
>    unreachable ff00::/8 dev lo  metric 1024  error -101
>    ff00::/8 dev eth1  metric 1024
> 
> 2) ip -6 route del ff00::/8 dev eth1
>    ip -6 route del ff00::/8 dev lo
> 
> 3) ip -6 route add ff00::/8 dev lo
>    ip -6 route add ff00::/8 dev eth1

My answer is "Don't ever do that."  The kernel sets up the
loopback device with all the necessary parameters you need
including the address, network prefix, and (once we revert
the recent bogus ipv6 autoconf patch) the multicast prefix.

There is no reason to ever explicit set the things that the
kernel takes care of for you.

That was exactly my point, the piece of userspace that has
started doing this needs to be fixed and we need to revert
Li Wei's recent patch.

^ permalink raw reply

* pull request: wireless 2011-12-21
From: John W. Linville @ 2011-12-21 18:59 UTC (permalink / raw)
  To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

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

commit b4949b84567f3ae1227d076fc95bbd8efea06506

Dave,

A few last(?) fixes intended for 3.2...

The biggest portion are from Gustavo:

"3 fixes for 3.2! There one revert that was causing connection issues,
a RFCOMM fix to a potential kernel panic, and the last on L2CAP to
a possible access to uninitialized data due to broken remote devices."

Also, an iwlwifi fix for a problem causing firmware asserts.

Please let me know if there are problems!

John

---

The following changes since commit cd7816d14953c8af910af5bb92f488b0b277e29d:

  net: have ipconfig not wait if no dev is available (2011-12-20 14:09:15 -0500)

are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless.git for-davem

Emmanuel Grumbach (1):
      iwlwifi: update SCD BC table for all SCD queues

Gustavo F. Padovan (1):
      Revert "Bluetooth: Revert: Fix L2CAP connection establishment"

John W. Linville (2):
      Merge branch 'master' of git://git.kernel.org/.../padovan/bluetooth
      Merge branch 'master' of git://git.kernel.org/.../linville/wireless into for-davem

Mat Martineau (2):
      Bluetooth: Prevent uninitialized data access in L2CAP configuration
      Bluetooth: Clear RFCOMM session timer when disconnecting last channel

 drivers/net/wireless/iwlwifi/iwl-trans-pcie.c |    4 +---
 net/bluetooth/hci_conn.c                      |    2 +-
 net/bluetooth/l2cap_core.c                    |   12 +++++++++++-
 net/bluetooth/rfcomm/core.c                   |    1 +
 4 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c b/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
index ce91898..5f17ab8 100644
--- a/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
+++ b/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
@@ -1197,9 +1197,7 @@ static int iwl_trans_pcie_tx(struct iwl_trans *trans, struct sk_buff *skb,
 	iwl_print_hex_dump(trans, IWL_DL_TX, (u8 *)tx_cmd->hdr, hdr_len);
 
 	/* Set up entry for this TFD in Tx byte-count array */
-	if (is_agg)
-		iwl_trans_txq_update_byte_cnt_tbl(trans, txq,
-					       le16_to_cpu(tx_cmd->len));
+	iwl_trans_txq_update_byte_cnt_tbl(trans, txq, le16_to_cpu(tx_cmd->len));
 
 	dma_sync_single_for_device(bus(trans)->dev, txcmd_phys, firstlen,
 			DMA_BIDIRECTIONAL);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index e0af723..c1c597e 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -673,7 +673,7 @@ int hci_conn_security(struct hci_conn *conn, __u8 sec_level, __u8 auth_type)
 		goto encrypt;
 
 auth:
-	if (test_and_set_bit(HCI_CONN_ENCRYPT_PEND, &conn->pend))
+	if (test_bit(HCI_CONN_ENCRYPT_PEND, &conn->pend))
 		return 0;
 
 	if (!hci_conn_auth(conn, sec_level, auth_type))
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 5ea94a1..17b5b1c 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -2152,7 +2152,7 @@ static int l2cap_parse_conf_rsp(struct l2cap_chan *chan, void *rsp, int len, voi
 	void *ptr = req->data;
 	int type, olen;
 	unsigned long val;
-	struct l2cap_conf_rfc rfc;
+	struct l2cap_conf_rfc rfc = { .mode = L2CAP_MODE_BASIC };
 
 	BT_DBG("chan %p, rsp %p, len %d, req %p", chan, rsp, len, data);
 
@@ -2271,6 +2271,16 @@ static void l2cap_conf_rfc_get(struct l2cap_chan *chan, void *rsp, int len)
 		}
 	}
 
+	/* Use sane default values in case a misbehaving remote device
+	 * did not send an RFC option.
+	 */
+	rfc.mode = chan->mode;
+	rfc.retrans_timeout = cpu_to_le16(L2CAP_DEFAULT_RETRANS_TO);
+	rfc.monitor_timeout = cpu_to_le16(L2CAP_DEFAULT_MONITOR_TO);
+	rfc.max_pdu_size = cpu_to_le16(chan->imtu);
+
+	BT_ERR("Expected RFC option was not found, using defaults");
+
 done:
 	switch (rfc.mode) {
 	case L2CAP_MODE_ERTM:
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 4e32e18..2d28dfe 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -1146,6 +1146,7 @@ static int rfcomm_recv_ua(struct rfcomm_session *s, u8 dlci)
 			if (list_empty(&s->dlcs)) {
 				s->state = BT_DISCONN;
 				rfcomm_send_disc(s, 0);
+				rfcomm_session_clear_timer(s);
 			}
 
 			break;
-- 
John W. Linville		Someday the world will need a hero, and you
linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org			might be all we have.  Be ready.

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

^ permalink raw reply related

* [PATCH] rps: fix insufficient bounds checking in store_rps_dev_flow_table_cnt()
From: Xi Wang @ 2011-12-21 18:50 UTC (permalink / raw)
  To: Tom Herbert, David S. Miller; +Cc: netdev, Xi Wang

Setting a large rps_flow_cnt like 1073741824 (1 << 30) on 32-bit
platform will cause a kernel oops due to insufficient bounds checking.

	if (count > 1<<30) {
		/* Enforce a limit to prevent overflow */
		return -EINVAL;
	}
	count = roundup_pow_of_two(count);
	table = vmalloc(RPS_DEV_FLOW_TABLE_SIZE(count));

Note that the macro RPS_DEV_FLOW_TABLE_SIZE(count) is defined as:

	... + (count * sizeof(struct rps_dev_flow))

where sizeof(struct rps_dev_flow) is 8.  (1 << 30) * 8 will overflow
32 bits.  This patch changes the upper bound to (1 << 28).

Signed-off-by: Xi Wang <xi.wang@gmail.com>
---
 net/core/net-sysfs.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index c71c434..f53a947 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -665,7 +665,7 @@ static ssize_t store_rps_dev_flow_table_cnt(struct netdev_rx_queue *queue,
 	if (count) {
 		int i;
 
-		if (count > 1<<30) {
+		if (count > 1<<28) {
 			/* Enforce a limit to prevent overflow */
 			return -EINVAL;
 		}
-- 
1.7.5.4

^ permalink raw reply related

* Re: [PATCH net-next v2 2/4] can: cc770: add legacy ISA bus driver for the CC770 and AN82527
From: Wolfgang Zarre @ 2011-12-21 18:32 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: Oliver Hartkopp, linux-can-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	socketcan-users-0fE9KPoRgkgATYTw5x5z8w
In-Reply-To: <4EE5EBBF.6080007-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>

Hello Wolfgang,

> On 12/12/2011 12:18 PM, Wolfgang Zarre wrote:
>> Hello Wolfgang,
>>> Hi Wolfgang,
>>>
>>> On 12/11/2011 07:33 PM, Wolfgang Zarre wrote:
>>>> Hello Wolfgang,
>>>>> On 12/07/2011 02:42 PM, Wolfgang Grandegger wrote:
>>>>>> Hi Wolfgang,
>>>>>>
>>>>>> On 12/06/2011 10:08 PM, Wolfgang Zarre wrote:
>>> ...
>>>>>>> Let me know if You need more or some other tests.
>>>>>>
>>>>>> You could provoke some state changes or bus-off conditions to see
>>>>>> if the
>>>>>> berr-counter shows reasonable results. I'm currently consolidating and
>>>>>> unifying error state and bus-off handling. Would be nice if you
>>>>>> could do
>>>>>> some further tests when I have the patches ready...
>>>>>
>>>>> I just pushed the mentioned modifications to the "devel" branch of my
>>>>> "wg-linux-can-next" [1] repository. You can get it as shown below:
>>>>>
>>>>>      $ git clone --reference=<some-recent-net-next-tree>    \
>>>>>
>>>>> git://gitorious.org/~wgrandegger/linux-can/wg-linux-can-next.git
>>>>>      $ git checkout -b devel devel
>>>>>
>>>>> [1] https://gitorious.org/~wgrandegger/linux-can/wg-linux-can-next
>>>>>
>>>>> Wolfgang.
>>>>
>>>> OK, I was trying so far and You will find below the results.
>>>> Just FYI the states on the PLC side couldn't be verified because the
>>>> function
>>>> provided by the manufacturer is not working at all and CAN analyser
>>>> was not
>>>> available.
>>>>
>>>> We are running CANopen and therefore the PLC will send automatically a
>>>> heartbeat.
>>>>
>>>> I produced the bus-off state through a short circuit between L/H
>>>> which was
>>>> working as expected.
>>>>
>>>> A bit odd was that on the second try I had to reload the module
>>>> because a ip down/up was not enough.
>>>
>>> Oops, not good.
>>>
>>
>> But might be in connection with the strange behaviour of the PLC.
>
> It's a bug! netif_start_queue is missing at the end of the open
> function. Got lost some how. I have just updated (rebased!) my
> wg-linux-can-next repository.

Ok, I was checking out last week and since I'm running one test series
after the other.

There are several odd issues I could found and I'm trying to trace them
down beside some other work.

Even with an assumed correct configuration like I was using with the lincan
driver I'm loosing telegrams so around 1 till 2 in 500000 but might be a
different sample-point at the PLC which is opaque due the predefined setting.
For the next test I'll set the BTR's directly.
Further sometimes I can find one in dropped but mostly not.

But more odd is that after an undefined time the transmission gets
stuck followed by a buffer overrun but can receive.
No error messages nor changes in ip -d -s link show can0.

Additional it seems that neither the automatic restart nor
the manual one works.

ip link set can0 up type can restart gives me 'RTNETLINK answers: Invalid
argument' and ip link set can0 up type can bitrate 500000 restart a
RTNETLINK answers: Device or resource busy but nothing connected to can0.

So I have to perform per example  ip link set can0 down;ip link set can0 up
type can bitrate 500000 restart-ms 2000 sample-point 0.75
but this is emptying the buffer and these telegrams are lost then as well.

I was comparing with my lincan driver which was running so far ok also
to confirm a proper working PLC.

First I assumed that maybe the set_reset_mode procedure is responsible for
that misbehaviour because according to the cc770 manual we should wait for
a zero of bit 7 RstST of the CPU interface register but when the transmission
gets stuck there was no call for set_reset_mode.

Maybe it's ending up somehow recessive.

Anyway, I might compare the registers of both drivers just to figure out
what's going on but maybe You have an idea as well.

Problem is just it runs always quite some time until the issues happen
otherwise it would be more easy.



>
> Wolfgang.


Wolfgang

^ permalink raw reply

* Re: [PATCH linux-firmware] linux-firmware: bnx2: Update mips firmware to fix iSCSI problems
From: Ben Hutchings @ 2011-12-21 18:13 UTC (permalink / raw)
  To: Michael Chan
  Cc: 'Eric Dumazet', 'dwmw2@infradead.org',
	'netdev@vger.kernel.org'
In-Reply-To: <0E685F8B314BFB42AE2422915B7BDCC30752F5@IRVEXCHMB07.corp.ad.broadcom.com>

On Wed, Dec 21, 2011 at 04:49:10PM +0000, Michael Chan wrote:
> Eric Dumazet wrote:
> 
> > Le dimanche 18 décembre 2011 à 20:13 -0800, Michael Chan a écrit :
> > > New firmware fixes iSCSI problems with some LeftHand targets that
> > don't
> > > set TTT=0xffffffff for Data-In according to spec.  Firmware generates
> > > exception warnings for this condition and becomes very slow.  This is
> > > fixed by suppressing these warnings when using default error mask.
> > >
> > > Signed-off-by: Michael Chan <mchan@broadcom.com>
> > > ---
> > >  WHENCE                      |    2 ++
> > >  bnx2/bnx2-mips-06-6.2.3.fw  |  Bin 0 -> 92824 bytes
> > >  bnx2/bnx2-mips-09-6.2.1b.fw |  Bin 0 -> 103904 bytes
> > >  3 files changed, 2 insertions(+), 0 deletions(-)
> > >  create mode 100644 bnx2/bnx2-mips-06-6.2.3.fw
> > >  create mode 100644 bnx2/bnx2-mips-09-6.2.1b.fw
> > 
> > IMHO, this should be applied _before_ the drivers/net patch.
> > 
> 
> Ben,
> 
> If David Woodhouse is not available, can you help us apply this firmware
> patch?

Yes, but please don't expect instant response.

Ben.

-- 
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
                                                              - Albert Camus

^ permalink raw reply

* Re: BUG: unable to handle kernel NULL pointer dereference in ipv6_select_ident
From: Eric Dumazet @ 2011-12-21 18:00 UTC (permalink / raw)
  To: Chris Boot; +Cc: lkml, netdev
In-Reply-To: <1324488984.2301.45.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>

Le mercredi 21 décembre 2011 à 18:36 +0100, Eric Dumazet a écrit :

> Good point, thats a different problem then, since 3.1 is not supposed to
> have this bug.
> 
> It seems rt->rt6i_peer points to invalid memory in your crash.
> 
> (RBX=00000000000001f4)
> 
> 8b 83 a4 00 00 00       mov    0xa4(%rbx),%eax    p->refcnt
> 1f4+a4 -> CR2=0000000000000298
> 

It would help if you can confirm latest linux tree can reproduce the
bug.

Thanks !

^ permalink raw reply

* Re: [PATCH] drivers/iwlwifi: use dma_zalloc_coherent() for DMA allocation
From: Guy, Wey-Yi @ 2011-12-21 16:52 UTC (permalink / raw)
  To: Djalal Harouni
  Cc: Intel Linux Wireless, John W. Linville,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	Andrew Morton
In-Reply-To: <20111221002147.GA4996@dztty>

Hi Djalal,


On Tue, 2011-12-20 at 16:21 -0800, Djalal Harouni wrote:
> Replace dma_alloc_coherent()+memset() with the new dma_zalloc_coherent()
> 
> Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
> ---

iwl-trans-pcie.c is one of the source files in iwlwifi driver hold
dual-license (GPL + BSD), are you ok to release your code for the BSD
license and relinquish the copyright?

btw, we are in the process of rewrite and re-architect major portion of
iwlwifi driver, and we also plan to make most of the code dual-license
(GPL + BSD). 

Thanks you in advance

Wey

^ permalink raw reply

* Re: [PATCH] Xen: consolidate and simplify struct xenbus_driver instantiation
From: Konrad Rzeszutek Wilk @ 2011-12-21 17:39 UTC (permalink / raw)
  To: Jan Beulich, dmitry.torokhov, axboe, FlorianSchandinat,
	ian.campbell, davem, netdev
  Cc: Konrad Rzeszutek Wilk, Jeremy Fitzhardinge,
	xen-devel@lists.xensource.com, linux-kernel
In-Reply-To: <4EF210E7020000780006965E@nat28.tlf.novell.com>

On Wed, Dec 21, 2011 at 04:01:27PM +0000, Jan Beulich wrote:
> The 'name', 'owner', and 'mod_name' members are redundant with the
> identically named fields in the 'driver' sub-structure. Rather than
> switching each instance to specify these fields explicitly, introduce
> a macro to simplify this.
> 
> Eliminate further redundancy by allowing the drvname argument to
> DEFINE_XENBUS_DRIVER() to be blank (in which case the first entry from
> the ID table will be used for .driver.name).
> 
> Also eliminate the questionable xenbus_register_{back,front}end()
> wrappers - their sole remaining purpose was the checking of the
> 'owner' field, proper setting of which shouldn't be an issue anymore
> when the macro gets used.

Looks good, except:

a)  It also needs four ACKs from:

 1) block for block maintainer (Jens Axboe <axboe@kernel.dk> )
 2) kbdinput for input maintainer (Dmitry Torokhov <dmitry.torokhov@gmail.com)
 3) fbfront for the video maintainer (FlorianSchandinat@gmx.de)
 4) network for the network maintainer (Ian Campbell <ian.campbell@citrix.com>, netdev@vger.kernel.org ,"David S. Miller" <davem@davemloft.net>)

CC-ing all of them.

b) this is changing it from xen-pciback to pciback:

> --- 3.2-rc6/drivers/xen/xen-pciback/xenbus.c
> +++ 3.2-rc6-struct-xenbus_driver/drivers/xen/xen-pciback/xenbus.c
> @@ -707,19 +707,16 @@ static int xen_pcibk_xenbus_remove(struc
>  	return 0;
>  }
>  
> -static const struct xenbus_device_id xenpci_ids[] = {
> +static const struct xenbus_device_id xen_pcibk_ids[] = {
>  	{"pci"},
>  	{""},
>  };
>  
> -static struct xenbus_driver xenbus_xen_pcibk_driver = {
> -	.name			= DRV_NAME,
> -	.owner			= THIS_MODULE,
> -	.ids			= xenpci_ids,
> +static DEFINE_XENBUS_DRIVER(xen_pcibk, "pciback",
                                          ^^^^^^^^^
I think that should be "xen-pciback" or just DRV_NAME ?

>  	.probe			= xen_pcibk_xenbus_probe,
>  	.remove			= xen_pcibk_xenbus_remove,
>  	.otherend_changed	= xen_pcibk_frontend_changed,
> -};
> +);

Otherwise all the other changes look OK to me.

Here is the unchanged version of the patch for the other maintainers.

Xen: consolidate and simplify struct xenbus_driver instantiation

The 'name', 'owner', and 'mod_name' members are redundant with the
identically named fields in the 'driver' sub-structure. Rather than
switching each instance to specify these fields explicitly, introduce
a macro to simplify this.

Eliminate further redundancy by allowing the drvname argument to
DEFINE_XENBUS_DRIVER() to be blank (in which case the first entry from
the ID table will be used for .driver.name).

Also eliminate the questionable xenbus_register_{back,front}end()
wrappers - their sole remaining purpose was the checking of the
'owner' field, proper setting of which shouldn't be an issue anymore
when the macro gets used.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

---
 drivers/block/xen-blkback/xenbus.c         |    9 ++------
 drivers/block/xen-blkfront.c               |   11 +++-------
 drivers/input/misc/xen-kbdfront.c          |    7 +-----
 drivers/net/xen-netback/xenbus.c           |    9 ++------
 drivers/net/xen-netfront.c                 |    9 ++------
 drivers/pci/xen-pcifront.c                 |   11 +++-------
 drivers/video/xen-fbfront.c                |    9 ++------
 drivers/xen/xen-pciback/xenbus.c           |   13 ++++--------
 drivers/xen/xenbus/xenbus_probe.c          |    7 ------
 drivers/xen/xenbus/xenbus_probe.h          |    4 ---
 drivers/xen/xenbus/xenbus_probe_backend.c  |    8 ++-----
 drivers/xen/xenbus/xenbus_probe_frontend.c |    8 ++-----
 include/xen/xenbus.h                       |   31 ++++++++---------------------
 13 files changed, 44 insertions(+), 92 deletions(-)

--- 3.2-rc6/drivers/block/xen-blkback/xenbus.c
+++ 3.2-rc6-struct-xenbus_driver/drivers/block/xen-blkback/xenbus.c
@@ -787,17 +787,14 @@ static const struct xenbus_device_id xen
 };
 
 
-static struct xenbus_driver xen_blkbk = {
-	.name = "vbd",
-	.owner = THIS_MODULE,
-	.ids = xen_blkbk_ids,
+static DEFINE_XENBUS_DRIVER(xen_blkbk, ,
 	.probe = xen_blkbk_probe,
 	.remove = xen_blkbk_remove,
 	.otherend_changed = frontend_changed
-};
+);
 
 
 int xen_blkif_xenbus_init(void)
 {
-	return xenbus_register_backend(&xen_blkbk);
+	return xenbus_register_backend(&xen_blkbk_driver);
 }
--- 3.2-rc6/drivers/block/xen-blkfront.c
+++ 3.2-rc6-struct-xenbus_driver/drivers/block/xen-blkfront.c
@@ -1437,16 +1437,13 @@ static const struct xenbus_device_id blk
 	{ "" }
 };
 
-static struct xenbus_driver blkfront = {
-	.name = "vbd",
-	.owner = THIS_MODULE,
-	.ids = blkfront_ids,
+static DEFINE_XENBUS_DRIVER(blkfront, ,
 	.probe = blkfront_probe,
 	.remove = blkfront_remove,
 	.resume = blkfront_resume,
 	.otherend_changed = blkback_changed,
 	.is_ready = blkfront_is_ready,
-};
+);
 
 static int __init xlblk_init(void)
 {
@@ -1461,7 +1458,7 @@ static int __init xlblk_init(void)
 		return -ENODEV;
 	}
 
-	ret = xenbus_register_frontend(&blkfront);
+	ret = xenbus_register_frontend(&blkfront_driver);
 	if (ret) {
 		unregister_blkdev(XENVBD_MAJOR, DEV_NAME);
 		return ret;
@@ -1474,7 +1471,7 @@ module_init(xlblk_init);
 
 static void __exit xlblk_exit(void)
 {
-	return xenbus_unregister_driver(&blkfront);
+	return xenbus_unregister_driver(&blkfront_driver);
 }
 module_exit(xlblk_exit);
 
--- 3.2-rc6/drivers/input/misc/xen-kbdfront.c
+++ 3.2-rc6-struct-xenbus_driver/drivers/input/misc/xen-kbdfront.c
@@ -361,15 +361,12 @@ static const struct xenbus_device_id xen
 	{ "" }
 };
 
-static struct xenbus_driver xenkbd_driver = {
-	.name = "vkbd",
-	.owner = THIS_MODULE,
-	.ids = xenkbd_ids,
+static DEFINE_XENBUS_DRIVER(xenkbd, ,
 	.probe = xenkbd_probe,
 	.remove = xenkbd_remove,
 	.resume = xenkbd_resume,
 	.otherend_changed = xenkbd_backend_changed,
-};
+);
 
 static int __init xenkbd_init(void)
 {
--- 3.2-rc6/drivers/net/xen-netback/xenbus.c
+++ 3.2-rc6-struct-xenbus_driver/drivers/net/xen-netback/xenbus.c
@@ -474,17 +474,14 @@ static const struct xenbus_device_id net
 };
 
 
-static struct xenbus_driver netback = {
-	.name = "vif",
-	.owner = THIS_MODULE,
-	.ids = netback_ids,
+static DEFINE_XENBUS_DRIVER(netback, ,
 	.probe = netback_probe,
 	.remove = netback_remove,
 	.uevent = netback_uevent,
 	.otherend_changed = frontend_changed,
-};
+);
 
 int xenvif_xenbus_init(void)
 {
-	return xenbus_register_backend(&netback);
+	return xenbus_register_backend(&netback_driver);
 }
--- 3.2-rc6/drivers/net/xen-netfront.c
+++ 3.2-rc6-struct-xenbus_driver/drivers/net/xen-netfront.c
@@ -1910,7 +1910,7 @@ static void xennet_sysfs_delif(struct ne
 
 #endif /* CONFIG_SYSFS */
 
-static struct xenbus_device_id netfront_ids[] = {
+static const struct xenbus_device_id netfront_ids[] = {
 	{ "vif" },
 	{ "" }
 };
@@ -1937,15 +1937,12 @@ static int __devexit xennet_remove(struc
 	return 0;
 }
 
-static struct xenbus_driver netfront_driver = {
-	.name = "vif",
-	.owner = THIS_MODULE,
-	.ids = netfront_ids,
+static DEFINE_XENBUS_DRIVER(netfront, ,
 	.probe = netfront_probe,
 	.remove = __devexit_p(xennet_remove),
 	.resume = netfront_resume,
 	.otherend_changed = netback_changed,
-};
+);
 
 static int __init netif_init(void)
 {
--- 3.2-rc6/drivers/pci/xen-pcifront.c
+++ 3.2-rc6-struct-xenbus_driver/drivers/pci/xen-pcifront.c
@@ -1126,14 +1126,11 @@ static const struct xenbus_device_id xen
 	{""},
 };
 
-static struct xenbus_driver xenbus_pcifront_driver = {
-	.name			= "pcifront",
-	.owner			= THIS_MODULE,
-	.ids			= xenpci_ids,
+static DEFINE_XENBUS_DRIVER(xenpci, "pcifront",
 	.probe			= pcifront_xenbus_probe,
 	.remove			= pcifront_xenbus_remove,
 	.otherend_changed	= pcifront_backend_changed,
-};
+);
 
 static int __init pcifront_init(void)
 {
@@ -1142,12 +1139,12 @@ static int __init pcifront_init(void)
 
 	pci_frontend_registrar(1 /* enable */);
 
-	return xenbus_register_frontend(&xenbus_pcifront_driver);
+	return xenbus_register_frontend(&xenpci_driver);
 }
 
 static void __exit pcifront_cleanup(void)
 {
-	xenbus_unregister_driver(&xenbus_pcifront_driver);
+	xenbus_unregister_driver(&xenpci_driver);
 	pci_frontend_registrar(0 /* disable */);
 }
 module_init(pcifront_init);
--- 3.2-rc6/drivers/video/xen-fbfront.c
+++ 3.2-rc6-struct-xenbus_driver/drivers/video/xen-fbfront.c
@@ -671,20 +671,17 @@ InitWait:
 	}
 }
 
-static struct xenbus_device_id xenfb_ids[] = {
+static const struct xenbus_device_id xenfb_ids[] = {
 	{ "vfb" },
 	{ "" }
 };
 
-static struct xenbus_driver xenfb_driver = {
-	.name = "vfb",
-	.owner = THIS_MODULE,
-	.ids = xenfb_ids,
+static DEFINE_XENBUS_DRIVER(xenfb, ,
 	.probe = xenfb_probe,
 	.remove = xenfb_remove,
 	.resume = xenfb_resume,
 	.otherend_changed = xenfb_backend_changed,
-};
+);
 
 static int __init xenfb_init(void)
 {
--- 3.2-rc6/drivers/xen/xen-pciback/xenbus.c
+++ 3.2-rc6-struct-xenbus_driver/drivers/xen/xen-pciback/xenbus.c
@@ -707,19 +707,16 @@ static int xen_pcibk_xenbus_remove(struc
 	return 0;
 }
 
-static const struct xenbus_device_id xenpci_ids[] = {
+static const struct xenbus_device_id xen_pcibk_ids[] = {
 	{"pci"},
 	{""},
 };
 
-static struct xenbus_driver xenbus_xen_pcibk_driver = {
-	.name			= DRV_NAME,
-	.owner			= THIS_MODULE,
-	.ids			= xenpci_ids,
+static DEFINE_XENBUS_DRIVER(xen_pcibk, "pciback",
 	.probe			= xen_pcibk_xenbus_probe,
 	.remove			= xen_pcibk_xenbus_remove,
 	.otherend_changed	= xen_pcibk_frontend_changed,
-};
+);
 
 const struct xen_pcibk_backend *__read_mostly xen_pcibk_backend;
 
@@ -735,11 +732,11 @@ int __init xen_pcibk_xenbus_register(voi
 	if (passthrough)
 		xen_pcibk_backend = &xen_pcibk_passthrough_backend;
 	pr_info(DRV_NAME ": backend is %s\n", xen_pcibk_backend->name);
-	return xenbus_register_backend(&xenbus_xen_pcibk_driver);
+	return xenbus_register_backend(&xen_pcibk_driver);
 }
 
 void __exit xen_pcibk_xenbus_unregister(void)
 {
 	destroy_workqueue(xen_pcibk_wq);
-	xenbus_unregister_driver(&xenbus_xen_pcibk_driver);
+	xenbus_unregister_driver(&xen_pcibk_driver);
 }
--- 3.2-rc6/drivers/xen/xenbus/xenbus_probe.c
+++ 3.2-rc6-struct-xenbus_driver/drivers/xen/xenbus/xenbus_probe.c
@@ -291,14 +291,9 @@ void xenbus_dev_shutdown(struct device *
 EXPORT_SYMBOL_GPL(xenbus_dev_shutdown);
 
 int xenbus_register_driver_common(struct xenbus_driver *drv,
-				  struct xen_bus_type *bus,
-				  struct module *owner,
-				  const char *mod_name)
+				  struct xen_bus_type *bus)
 {
-	drv->driver.name = drv->name;
 	drv->driver.bus = &bus->bus;
-	drv->driver.owner = owner;
-	drv->driver.mod_name = mod_name;
 
 	return driver_register(&drv->driver);
 }
--- 3.2-rc6/drivers/xen/xenbus/xenbus_probe.h
+++ 3.2-rc6-struct-xenbus_driver/drivers/xen/xenbus/xenbus_probe.h
@@ -53,9 +53,7 @@ extern int xenbus_match(struct device *_
 extern int xenbus_dev_probe(struct device *_dev);
 extern int xenbus_dev_remove(struct device *_dev);
 extern int xenbus_register_driver_common(struct xenbus_driver *drv,
-					 struct xen_bus_type *bus,
-					 struct module *owner,
-					 const char *mod_name);
+					 struct xen_bus_type *bus);
 extern int xenbus_probe_node(struct xen_bus_type *bus,
 			     const char *type,
 			     const char *nodename);
--- 3.2-rc6/drivers/xen/xenbus/xenbus_probe_backend.c
+++ 3.2-rc6-struct-xenbus_driver/drivers/xen/xenbus/xenbus_probe_backend.c
@@ -232,15 +232,13 @@ int xenbus_dev_is_online(struct xenbus_d
 }
 EXPORT_SYMBOL_GPL(xenbus_dev_is_online);
 
-int __xenbus_register_backend(struct xenbus_driver *drv,
-			      struct module *owner, const char *mod_name)
+int xenbus_register_backend(struct xenbus_driver *drv)
 {
 	drv->read_otherend_details = read_frontend_details;
 
-	return xenbus_register_driver_common(drv, &xenbus_backend,
-					     owner, mod_name);
+	return xenbus_register_driver_common(drv, &xenbus_backend);
 }
-EXPORT_SYMBOL_GPL(__xenbus_register_backend);
+EXPORT_SYMBOL_GPL(xenbus_register_backend);
 
 static int backend_probe_and_watch(struct notifier_block *notifier,
 				   unsigned long event,
--- 3.2-rc6/drivers/xen/xenbus/xenbus_probe_frontend.c
+++ 3.2-rc6-struct-xenbus_driver/drivers/xen/xenbus/xenbus_probe_frontend.c
@@ -230,15 +230,13 @@ static void wait_for_devices(struct xenb
 			 print_device_status);
 }
 
-int __xenbus_register_frontend(struct xenbus_driver *drv,
-			       struct module *owner, const char *mod_name)
+int xenbus_register_frontend(struct xenbus_driver *drv)
 {
 	int ret;
 
 	drv->read_otherend_details = read_backend_details;
 
-	ret = xenbus_register_driver_common(drv, &xenbus_frontend,
-					    owner, mod_name);
+	ret = xenbus_register_driver_common(drv, &xenbus_frontend);
 	if (ret)
 		return ret;
 
@@ -247,7 +245,7 @@ int __xenbus_register_frontend(struct xe
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(__xenbus_register_frontend);
+EXPORT_SYMBOL_GPL(xenbus_register_frontend);
 
 static DECLARE_WAIT_QUEUE_HEAD(backend_state_wq);
 static int backend_state;
--- 3.2-rc6/include/xen/xenbus.h
+++ 3.2-rc6-struct-xenbus_driver/include/xen/xenbus.h
@@ -85,8 +85,6 @@ struct xenbus_device_id
 
 /* A xenbus driver. */
 struct xenbus_driver {
-	char *name;
-	struct module *owner;
 	const struct xenbus_device_id *ids;
 	int (*probe)(struct xenbus_device *dev,
 		     const struct xenbus_device_id *id);
@@ -101,31 +99,20 @@ struct xenbus_driver {
 	int (*is_ready)(struct xenbus_device *dev);
 };
 
-static inline struct xenbus_driver *to_xenbus_driver(struct device_driver *drv)
-{
-	return container_of(drv, struct xenbus_driver, driver);
+#define DEFINE_XENBUS_DRIVER(var, drvname, methods...)		\
+struct xenbus_driver var ## _driver = {				\
+	.driver.name = drvname + 0 ?: var ## _ids->devicetype,	\
+	.driver.owner = THIS_MODULE,				\
+	.ids = var ## _ids, ## methods				\
 }
 
-int __must_check __xenbus_register_frontend(struct xenbus_driver *drv,
-					    struct module *owner,
-					    const char *mod_name);
-
-static inline int __must_check
-xenbus_register_frontend(struct xenbus_driver *drv)
+static inline struct xenbus_driver *to_xenbus_driver(struct device_driver *drv)
 {
-	WARN_ON(drv->owner != THIS_MODULE);
-	return __xenbus_register_frontend(drv, THIS_MODULE, KBUILD_MODNAME);
+	return container_of(drv, struct xenbus_driver, driver);
 }
 
-int __must_check __xenbus_register_backend(struct xenbus_driver *drv,
-					   struct module *owner,
-					   const char *mod_name);
-static inline int __must_check
-xenbus_register_backend(struct xenbus_driver *drv)
-{
-	WARN_ON(drv->owner != THIS_MODULE);
-	return __xenbus_register_backend(drv, THIS_MODULE, KBUILD_MODNAME);
-}
+int __must_check xenbus_register_frontend(struct xenbus_driver *);
+int __must_check xenbus_register_backend(struct xenbus_driver *);
 
 void xenbus_unregister_driver(struct xenbus_driver *drv);
 

^ permalink raw reply

* Re: BUG: unable to handle kernel NULL pointer dereference in ipv6_select_ident
From: Eric Dumazet @ 2011-12-21 17:36 UTC (permalink / raw)
  To: Chris Boot; +Cc: lkml, netdev
In-Reply-To: <4EF2117F.6000803@bootc.net>

Le mercredi 21 décembre 2011 à 17:03 +0000, Chris Boot a écrit :
> On 21/12/2011 16:29, Eric Dumazet wrote:
> > Le mercredi 21 décembre 2011 à 15:52 +0000, Chris Boot a écrit :
> >> Hi folks,
> >>
> >> I'm working on getting a 2-node VM cluster up and running, with DRBD and
> >> Corosync/Pacemaker, running KVM VMs.
> >>
> >> I can trigger a kernel panic in either _host_ system when running an
> >> rsync on a _guest_ VM. The rsync is simply SSH over IPv6 from a remote
> >> mail store (containing maildirs) to a local filesystem. I'm basically
> >> working on migrating a physical IMAP server to one inside a VM.
> >>
> >> After a few seconds of fairly heavy IPv6 traffic, I get the panic below.
> >> You'll notice the panic refers to vhost_net, but I tried without that
> >> and the kernel panics at exactly the same call point.
> >>
> >> Panic:
> >>
> >> [snip]
> >>
> >> Any insight will be gratefully received.
> >>
> >> Thanks,
> >> Chris
> >>
> > Is it a debian kernel ?
> >
> > You need : https://lkml.org/lkml/2011/10/11/291
> 
> Eric,
> 
> Aha, that sounds like exactly the culprit, thanks. However I can't find 
> any reference to it in the 3.1 to 3.1.5 changelogs. Is it fixed in any 
> of those kernels or would I have to attempt to forward-port the fix myself?

Good point, thats a different problem then, since 3.1 is not supposed to
have this bug.

It seems rt->rt6i_peer points to invalid memory in your crash.

(RBX=00000000000001f4)

8b 83 a4 00 00 00       mov    0xa4(%rbx),%eax    p->refcnt
1f4+a4 -> CR2=0000000000000298

^ permalink raw reply

* Re: ICMP packets - ll_temac with Microblaze
From: Eric Dumazet @ 2011-12-21 17:11 UTC (permalink / raw)
  To: monstr; +Cc: David Miller, John Williams, netdev
In-Reply-To: <1324483144.2301.17.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>

Le mercredi 21 décembre 2011 à 16:59 +0100, Eric Dumazet a écrit :

> I wonder if you applied/tested my patch correctly, since it really
> should had help in your case  (allowing first received packet to be
> queued, even if very big)

Hmm, I missed another spot in sock_queue_rcv_skb().
(RAW sockets are not PACKET :) )

Here is the patch again, I tested it with 'busybox ping' and MTU=9000,
it solved the problem for me.

Thanks

[PATCH net-next] net: relax rcvbuf limits

skb->truesize might be big even for a small packet.

Its even bigger after commit 87fb4b7b533 (net: more accurate skb
truesize) and big MTU.

We should allow queueing at least one packet per receiver, even with a
low RCVBUF setting.

Reported-by: Michal Simek <monstr@monstr.eu>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/net/sock.h     |    4 +++-
 net/core/sock.c        |    6 +-----
 net/packet/af_packet.c |    6 ++----
 3 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index bf6b9fd..21bb3b5 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -662,12 +662,14 @@ static inline void __sk_add_backlog(struct sock *sk, struct sk_buff *skb)
 
 /*
  * Take into account size of receive queue and backlog queue
+ * Do not take into account this skb truesize,
+ * to allow even a single big packet to come.
  */
 static inline bool sk_rcvqueues_full(const struct sock *sk, const struct sk_buff *skb)
 {
 	unsigned int qsize = sk->sk_backlog.len + atomic_read(&sk->sk_rmem_alloc);
 
-	return qsize + skb->truesize > sk->sk_rcvbuf;
+	return qsize > sk->sk_rcvbuf;
 }
 
 /* The per-socket spinlock must be held here. */
diff --git a/net/core/sock.c b/net/core/sock.c
index a343286..347b6d9 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -339,11 +339,7 @@ int sock_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
 	unsigned long flags;
 	struct sk_buff_head *list = &sk->sk_receive_queue;
 
-	/* Cast sk->rcvbuf to unsigned... It's pointless, but reduces
-	   number of warnings when compiling with -W --ANK
-	 */
-	if (atomic_read(&sk->sk_rmem_alloc) + skb->truesize >=
-	    (unsigned)sk->sk_rcvbuf) {
+	if (atomic_read(&sk->sk_rmem_alloc) >= sk->sk_rcvbuf) {
 		atomic_inc(&sk->sk_drops);
 		trace_sock_rcvqueue_full(sk, skb);
 		return -ENOMEM;
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 0da505c..e56ca75 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1631,8 +1631,7 @@ static int packet_rcv(struct sk_buff *skb, struct net_device *dev,
 	if (snaplen > res)
 		snaplen = res;
 
-	if (atomic_read(&sk->sk_rmem_alloc) + skb->truesize >=
-	    (unsigned)sk->sk_rcvbuf)
+	if (atomic_read(&sk->sk_rmem_alloc) >= sk->sk_rcvbuf)
 		goto drop_n_acct;
 
 	if (skb_shared(skb)) {
@@ -1763,8 +1762,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
 	if (po->tp_version <= TPACKET_V2) {
 		if (macoff + snaplen > po->rx_ring.frame_size) {
 			if (po->copy_thresh &&
-				atomic_read(&sk->sk_rmem_alloc) + skb->truesize
-				< (unsigned)sk->sk_rcvbuf) {
+			    atomic_read(&sk->sk_rmem_alloc) < sk->sk_rcvbuf) {
 				if (skb_shared(skb)) {
 					copy_skb = skb_clone(skb, GFP_ATOMIC);
 				} else {

^ permalink raw reply related

* Re: BUG: unable to handle kernel NULL pointer dereference in ipv6_select_ident
From: Chris Boot @ 2011-12-21 17:03 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: lkml, netdev
In-Reply-To: <1324484956.2301.24.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>

On 21/12/2011 16:29, Eric Dumazet wrote:
> Le mercredi 21 décembre 2011 à 15:52 +0000, Chris Boot a écrit :
>> Hi folks,
>>
>> I'm working on getting a 2-node VM cluster up and running, with DRBD and
>> Corosync/Pacemaker, running KVM VMs.
>>
>> I can trigger a kernel panic in either _host_ system when running an
>> rsync on a _guest_ VM. The rsync is simply SSH over IPv6 from a remote
>> mail store (containing maildirs) to a local filesystem. I'm basically
>> working on migrating a physical IMAP server to one inside a VM.
>>
>> After a few seconds of fairly heavy IPv6 traffic, I get the panic below.
>> You'll notice the panic refers to vhost_net, but I tried without that
>> and the kernel panics at exactly the same call point.
>>
>> Panic:
>>
>> [snip]
>>
>> Any insight will be gratefully received.
>>
>> Thanks,
>> Chris
>>
> Is it a debian kernel ?
>
> You need : https://lkml.org/lkml/2011/10/11/291

Eric,

Aha, that sounds like exactly the culprit, thanks. However I can't find 
any reference to it in the 3.1 to 3.1.5 changelogs. Is it fixed in any 
of those kernels or would I have to attempt to forward-port the fix myself?

Cheers,
Chris

-- 
Chris Boot
bootc@bootc.net

^ permalink raw reply

* RE: ICMP packets - ll_temac with Microblaze
From: Eric Dumazet @ 2011-12-21 16:50 UTC (permalink / raw)
  To: David Laight; +Cc: Jun Zhao, monstr, David Miller, John Williams, netdev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6D8AF21@saturn3.aculab.com>

Le mercredi 21 décembre 2011 à 16:39 +0000, David Laight a écrit :

> Many, many moons ago I wrote an ethernet driver that received into
> an array of 128 (mostly) 512byte buffers. Full sized frames would have
> multiple rx ring entries, but could almost always be copied into a
> correctly
> sized buffer with a single aligned copy (cache-line aligned if useful).
> This was significantly faster than other schemes - especially
> on systems where the iommu needed setting to allow the ethernet
> hardware to access memory.
> 
> I don't know if the linux skb buffers would allow the ethernet
> driver to use (say) 1600 byte rx buffers, and link them together
> when a long frame arrives. Most ethernet HW I've seen will
> fragment long receives.
> 
> The painful hardware is that which enforces a 4n byte alignment
> on the rx buffer! - on systens that can't do misaligned accesses.
> 2 bytes of junk would be fine!

It all depends on hardware capabilities.

Old hardware required a single area per frame. So driver had to talk to
the hardware the same way.

We now have hardware able to split data into several frags to reduce the
overhead.

(Even using different pools to get a low number of frags, and good 
filling ratio)

Take a look at NIU for example.

^ permalink raw reply

* Re: [PATCH linux-firmware] linux-firmware: bnx2: Update mips firmware to fix iSCSI problems
From: Michael Chan @ 2011-12-21 16:49 UTC (permalink / raw)
  To: 'Eric Dumazet', 'ben@decadent.org.uk'
  Cc: 'dwmw2@infradead.org', 'netdev@vger.kernel.org'
In-Reply-To: <1324457726.2728.5.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>

Eric Dumazet wrote:

> Le dimanche 18 décembre 2011 à 20:13 -0800, Michael Chan a écrit :
> > New firmware fixes iSCSI problems with some LeftHand targets that
> don't
> > set TTT=0xffffffff for Data-In according to spec.  Firmware generates
> > exception warnings for this condition and becomes very slow.  This is
> > fixed by suppressing these warnings when using default error mask.
> >
> > Signed-off-by: Michael Chan <mchan@broadcom.com>
> > ---
> >  WHENCE                      |    2 ++
> >  bnx2/bnx2-mips-06-6.2.3.fw  |  Bin 0 -> 92824 bytes
> >  bnx2/bnx2-mips-09-6.2.1b.fw |  Bin 0 -> 103904 bytes
> >  3 files changed, 2 insertions(+), 0 deletions(-)
> >  create mode 100644 bnx2/bnx2-mips-06-6.2.3.fw
> >  create mode 100644 bnx2/bnx2-mips-09-6.2.1b.fw
> 
> IMHO, this should be applied _before_ the drivers/net patch.
> 

Ben,

If David Woodhouse is not available, can you help us apply this firmware
patch?

^ permalink raw reply

* Re: [PATCH 2/4] vfs: Export some file manipulation functions
From: Neil Horman @ 2011-12-21 16:42 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: netdev, Thomas Graf, David S. Miller, linux-fsdevel
In-Reply-To: <20111221153054.GA1489@infradead.org>

On Wed, Dec 21, 2011 at 10:30:54AM -0500, Christoph Hellwig wrote:
> On Wed, Dec 21, 2011 at 09:39:48AM -0500, Neil Horman wrote:
> > the networking cgroups can use the fd table of a task to make adjustments to the
> > sockets that they own, helping us set owners for various resources. Export
> > get_files_struct, put_files_struct and sock_from_file, so we can walk a tasks
> > fdarray easily.
> 
> No, no one has any business using these lowlevel routines form modules.
> 
> Please find a way to do this in core code, or find more highlevel
> primitives to export.
> 
> And not even Ccing linux-fsdevel on a change like this is one of the few
> things I'd consider extremely offensive.
> 
> 


Ok, after discussing with you and Al on irc, I'm going to rewrite this to not
use the file table.
Neil


^ permalink raw reply

* RE: ICMP packets - ll_temac with Microblaze
From: David Laight @ 2011-12-21 16:39 UTC (permalink / raw)
  To: Eric Dumazet, Jun Zhao; +Cc: monstr, David Miller, John Williams, netdev
In-Reply-To: <1324483543.2301.19.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>

 
> If not, a malicious attacker could send 1-byte frames and exhaust your
> kernel memory.

That used to happen in SVR4 - it was possible for single byte TCP (etc)
data to be queued at a STREAM head (which counted actual data bytes)
in a 2k message block.
Actually we also managed to use all kernel memory queueing zero sized
STREAMS messages.

> If you want to reduce it, you might use copybreak : Some drivers copy
> the data into a small skb instead of providing a jumbo frame to upper
> stack.

Many, many moons ago I wrote an ethernet driver that received into
an array of 128 (mostly) 512byte buffers. Full sized frames would have
multiple rx ring entries, but could almost always be copied into a
correctly
sized buffer with a single aligned copy (cache-line aligned if useful).
This was significantly faster than other schemes - especially
on systems where the iommu needed setting to allow the ethernet
hardware to access memory.

I don't know if the linux skb buffers would allow the ethernet
driver to use (say) 1600 byte rx buffers, and link them together
when a long frame arrives. Most ethernet HW I've seen will
fragment long receives.

The painful hardware is that which enforces a 4n byte alignment
on the rx buffer! - on systens that can't do misaligned accesses.
2 bytes of junk would be fine!

	David

^ permalink raw reply

* Re: BUG: unable to handle kernel NULL pointer dereference in ipv6_select_ident
From: Eric Dumazet @ 2011-12-21 16:29 UTC (permalink / raw)
  To: Chris Boot; +Cc: lkml, netdev
In-Reply-To: <4EF200BB.7000209@bootc.net>

Le mercredi 21 décembre 2011 à 15:52 +0000, Chris Boot a écrit :
> Hi folks,
> 
> I'm working on getting a 2-node VM cluster up and running, with DRBD and 
> Corosync/Pacemaker, running KVM VMs.
> 
> I can trigger a kernel panic in either _host_ system when running an 
> rsync on a _guest_ VM. The rsync is simply SSH over IPv6 from a remote 
> mail store (containing maildirs) to a local filesystem. I'm basically 
> working on migrating a physical IMAP server to one inside a VM.
> 
> After a few seconds of fairly heavy IPv6 traffic, I get the panic below. 
> You'll notice the panic refers to vhost_net, but I tried without that 
> and the kernel panics at exactly the same call point.
> 
> Panic:
> 
> [  461.232932] BUG: unable to handle kernel NULL pointer dereference at 
> 0000000000000298
> [  461.240790] IP: [<ffffffff812dde61>] ipv6_select_ident+0x31/0xa7
> [  461.246916] PGD 42d3e8067 PUD 41facf067 PMD 0
> [  461.251537] Oops: 0000 [#1] SMP
> [  461.254795] CPU 4
> [  461.256648] Modules linked in: sha1_generic hmac sha256_generic dlm 
> configfs ebtable_nat ebtables acpi_cpufreq mperf cpufreq_stats 
> cpufreq_conservative cpufreq_powersave cpufreq_userspace microcode 
> xt_NOTRACK ip_set_hash_net act_police cls_basic cls_flow cls_fw cls_u32 sch_
> tbf sch_prio sch_htb sch_hfsc sch_ingress sch_sfq xt_realm xt_connlimit 
> xt_addrtype ip_set_hash_ip iptable_raw xt_comment xt_recent ipt_ULOG 
> ipt_REJECT ipt_REDIRECT ip6_queue ipt_NETMAP ipt_MASQUERADE ipt_ECN 
> ipt_ecn nf_conntrack_proto_udplite ipt_CLUSTERIP ipt_ah xt_time xt_s
> et xt_TCPMSS ip_set xt_sctp xt_policy nf_nat_tftp nf_nat_snmp_basic 
> nf_conntrack_snmp nf_nat_sip nf_nat_pptp nf_nat_proto_gre nf_nat_irc 
> nf_nat_h323 nf_nat_ftp nf_nat_amanda ip6t_LOG ts_kmp ip6t_REJECT 
> nf_conntrack_amanda nf_conntrack_sane nf_conntrack_tftp nf_conntrack_sip nf
> _conntrack_proto_sctp nf_conntrack_pptp nf_conntrack_proto_gre 
> nf_conntrack_netlink nf_conntrack_netbios_ns nf_conntrack_broadcast 
> nf_conntrack_irc nf_conntrack_h323 nf_conntrack_ftp xt_TPROXY 
> nf_tproxy_core xt_tcpmss xt_pkttype xt_physdev xt_owner xt_NFQUEUE 
> xt_NFLOG nfnetlin
> k_log xt_multiport xt_mark xt_mac xt_limit xt_length xt_iprange 
> xt_helper xt_hashlimit xt_DSCP xt_dscp xt_dccp xt_connmark xt_CLASSIFY 
> xt_AUDIT ipt_LOG xt_tcpudp xt_state nf_conntrack_ipv6 nf_defrag_ipv6 
> iptable_nat nf_nat xt_conntrack nf_conntrack_ipv4 nf_defrag_ipv4 ip6table
> _raw nf_conntrack ip6table_mangle iptable_mangle nfnetlink 
> iptable_filter ip_tables ip6table_filter ip6_tables x_tables bridge stp 
> bonding w83627ehf hwmon_vid coretemp crc32c_intel aesni_intel cryptd 
> aes_x86_64 aes_generic ipmi_poweroff ipmi_devintf ipmi_si 
> ipmi_msghandler vho
> st_net macvtap macvlan tun drbd lru_cache cn loop kvm_intel kvm snd_pcm 
> snd_timer snd soundcore snd_page_alloc psmouse i2c_i801 processor 
> iTCO_wdt iTCO_vendor_support i2c_core evdev thermal_sys joydev serio_raw 
> pcspkr button ext4 mbcache jbd2 crc16 dm_mod raid1 md_mod sd_mod c
> rc_t10dif usb_storage uas usbhid hid ahci libahci libata igb ehci_hcd 
> scsi_mod usbcore e1000e dca [last unloaded: scsi_wait_scan]
> [  461.446246]
> [  461.447812] Pid: 5756, comm: vhost-5753 Not tainted 3.1.0-1-amd64 #1 
> Supermicro X9SCL/X9SCM/X9SCL/X9SCM
> [  461.457562] RIP: 0010:[<ffffffff812dde61>]  [<ffffffff812dde61>] 
> ipv6_select_ident+0x31/0xa7
> [  461.466271] RSP: 0018:ffff88043fd03758  EFLAGS: 00010202
> [  461.471708] RAX: ffff8803f7159038 RBX: 00000000000001f4 RCX: 
> 0000000000000000
> [  461.478979] RDX: 0000000000000016 RSI: ffff88042dadcf30 RDI: 
> ffff8803f715906e
> [  461.486207] RBP: ffff8803f715906e R08: 0ea8c9feff005256 R09: 
> 00000000000080fe
> [  461.493542] R10: 5256000000000000 R11: 80fe4e6fdcfeff00 R12: 
> 0000000040115ba9
> [  461.500851] R13: ffffffff8168ed11 R14: 0000000000000011 R15: 
> ffff8803f7159880
> [  461.508167] FS:  0000000000000000(0000) GS:ffff88043fd00000(0000) 
> knlGS:0000000000000000
> [  461.516444] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [  461.522317] CR2: 0000000000000298 CR3: 000000042d87e000 CR4: 
> 00000000000426e0
> [  461.529607] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 
> 0000000000000000
> [  461.536916] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 
> 0000000000000400
> [  461.544267] Process vhost-5753 (pid: 5756, threadinfo 
> ffff88042cb4a000, task ffff88042d13c970)
> [  461.553108] Stack:
> [  461.555202]  ffff8803f7159038 ffff88042be0e280 0000000000000028 
> ffffffff812efa7f
> [  461.562840]  0000000000000246 ffff8803f7159054 ffff88042be0e318 
> ffff88042be0e280
> [  461.570544]  ffffffffffffffa3 0000000040115ba9 ffffffff8168edc0 
> ffffffff812dbfe0
> [  461.578229] Call Trace:
> [  461.580742] <IRQ>
> [  461.582870]  [<ffffffff812efa7f>] ? udp6_ufo_fragment+0x124/0x1a2
> [  461.589054]  [<ffffffff812dbfe0>] ? ipv6_gso_segment+0xc0/0x155
> [  461.595140]  [<ffffffff812700c6>] ? skb_gso_segment+0x208/0x28b
> [  461.601198]  [<ffffffffa03f236b>] ? ipv6_confirm+0x146/0x15e 
> [nf_conntrack_ipv6]
> [  461.608786]  [<ffffffff81291c4d>] ? nf_iterate+0x41/0x77
> [  461.614227]  [<ffffffff81271d64>] ? dev_hard_start_xmit+0x357/0x543
> [  461.620659]  [<ffffffff81291cf6>] ? nf_hook_slow+0x73/0x111
> [  461.626440]  [<ffffffffa0379745>] ? br_parse_ip_options+0x19a/0x19a 
> [bridge]
> [  461.633581]  [<ffffffff812722ff>] ? dev_queue_xmit+0x3af/0x459
> [  461.639577]  [<ffffffffa03747d2>] ? br_dev_queue_push_xmit+0x72/0x76 
> [bridge]
> [  461.646887]  [<ffffffffa03791e3>] ? br_nf_post_routing+0x17d/0x18f 
> [bridge]
> [  461.653997]  [<ffffffff81291c4d>] ? nf_iterate+0x41/0x77
> [  461.659473]  [<ffffffffa0374760>] ? br_flood+0xfa/0xfa [bridge]
> [  461.665485]  [<ffffffff81291cf6>] ? nf_hook_slow+0x73/0x111
> [  461.671234]  [<ffffffffa0374760>] ? br_flood+0xfa/0xfa [bridge]
> [  461.677299]  [<ffffffffa0379215>] ? 
> nf_bridge_update_protocol+0x20/0x20 [bridge]
> [  461.684891]  [<ffffffffa03bb0e5>] ? nf_ct_zone+0xa/0x17 [nf_conntrack]
> [  461.691520]  [<ffffffffa0374760>] ? br_flood+0xfa/0xfa [bridge]
> [  461.697572]  [<ffffffffa0374812>] ? NF_HOOK.constprop.8+0x3c/0x56 
> [bridge]
> [  461.704616]  [<ffffffffa0379031>] ? 
> nf_bridge_push_encap_header+0x1c/0x26 [bridge]
> [  461.712329]  [<ffffffffa037929f>] ? br_nf_forward_finish+0x8a/0x95 
> [bridge]
> [  461.719490]  [<ffffffffa037900a>] ? 
> nf_bridge_pull_encap_header+0x1c/0x27 [bridge]
> [  461.727223]  [<ffffffffa0379974>] ? br_nf_forward_ip+0x1c0/0x1d4 [bridge]
> [  461.734292]  [<ffffffff81291c4d>] ? nf_iterate+0x41/0x77
> [  461.739758]  [<ffffffffa03748cc>] ? __br_deliver+0xa0/0xa0 [bridge]
> [  461.746203]  [<ffffffff81291cf6>] ? nf_hook_slow+0x73/0x111
> [  461.751950]  [<ffffffffa03748cc>] ? __br_deliver+0xa0/0xa0 [bridge]
> [  461.758378]  [<ffffffffa037533a>] ? NF_HOOK.constprop.4+0x56/0x56 
> [bridge]
> [  461.765454]  [<ffffffffa03748cc>] ? __br_deliver+0xa0/0xa0 [bridge]
> [  461.771881]  [<ffffffffa0374812>] ? NF_HOOK.constprop.8+0x3c/0x56 
> [bridge]
> [  461.778908]  [<ffffffffa03749a6>] ? br_forward+0x16/0x5a [bridge]
> [  461.785041]  [<ffffffffa03754db>] ? 
> br_handle_frame_finish+0x1a1/0x20f [bridge]
> [  461.792604]  [<ffffffffa0379333>] ? 
> br_nf_pre_routing_finish_ipv6+0x89/0x92 [bridge]
> [  461.800513]  [<ffffffffa0378e7b>] ? setup_pre_routing+0x38/0x5d [bridge]
> [  461.807440]  [<ffffffffa0379e65>] ? br_nf_pre_routing+0x3e8/0x3f5 
> [bridge]
> [  461.814463]  [<ffffffff81291c4d>] ? nf_iterate+0x41/0x77
> [  461.819908]  [<ffffffff8103f89d>] ? select_task_rq_fair+0x369/0x610
> [  461.826347]  [<ffffffffa037533a>] ? NF_HOOK.constprop.4+0x56/0x56 
> [bridge]
> [  461.833463]  [<ffffffffa037533a>] ? NF_HOOK.constprop.4+0x56/0x56 
> [bridge]
> [  461.840532]  [<ffffffff81291cf6>] ? nf_hook_slow+0x73/0x111
> [  461.846134]  [<ffffffffa037533a>] ? NF_HOOK.constprop.4+0x56/0x56 
> [bridge]
> [  461.853222]  [<ffffffff81036010>] ? test_tsk_need_resched+0xa/0x13
> [  461.859601]  [<ffffffffa037533a>] ? NF_HOOK.constprop.4+0x56/0x56 
> [bridge]
> [  461.866650]  [<ffffffffa0375320>] ? NF_HOOK.constprop.4+0x3c/0x56 
> [bridge]
> [  461.873686]  [<ffffffffa03756ed>] ? br_handle_frame+0x1a4/0x1bb [bridge]
> [  461.880569]  [<ffffffffa0375549>] ? 
> br_handle_frame_finish+0x20f/0x20f [bridge]
> [  461.888042]  [<ffffffff8126f132>] ? __netif_receive_skb+0x2d6/0x415
> [  461.894467]  [<ffffffff8126f2dd>] ? process_backlog+0x6c/0x123
> [  461.900414]  [<ffffffff81026b37>] ? native_apic_msr_write+0x2c/0x2f
> [  461.906790]  [<ffffffff81271034>] ? net_rx_action+0xa1/0x1af
> [  461.912625]  [<ffffffff81036010>] ? test_tsk_need_resched+0xa/0x13
> [  461.919010]  [<ffffffff8104ad04>] ? __do_softirq+0xb9/0x177
> [  461.924796]  [<ffffffff8133452c>] ? call_softirq+0x1c/0x30
> [  461.930474] <EOI>
> [  461.932684]  [<ffffffff8100f845>] ? do_softirq+0x3c/0x7b
> [  461.938106]  [<ffffffff81271324>] ? netif_rx_ni+0x1e/0x27
> [  461.943610]  [<ffffffffa0298721>] ? tun_get_user+0x39a/0x3c2 [tun]
> [  461.949923]  [<ffffffffa0298766>] ? tun_sendmsg+0x1d/0x1f [tun]
> [  461.955938]  [<ffffffffa02a6b50>] ? handle_tx+0x340/0x3de [vhost_net]
> [  461.962609]  [<ffffffffa02a46cb>] ? vhost_worker+0x10b/0x121 [vhost_net]
> [  461.969447]  [<ffffffffa02a45c0>] ? 
> vhost_attach_cgroups_work+0x1b/0x1b [vhost_net]
> [  461.977274]  [<ffffffff8105e5d1>] ? kthread+0x76/0x7e
> [  461.982440]  [<ffffffff81334434>] ? kernel_thread_helper+0x4/0x10
> [  461.988667]  [<ffffffff8105e55b>] ? kthread_worker_fn+0x139/0x139
> [  461.994940]  [<ffffffff81334430>] ? gs_change+0x13/0x13
> [  462.000283] Code: fd 53 48 89 f3 50 74 70 48 83 be 10 01 00 00 00 75 
> 0d be 01 00 00 00 48 89 df e8 be 9d 00 00 48 8b 9b 10 01 00 00 48 85 db 
> 74 4d <8b> 83 a4 00 00 00 85 c0 7f 21 80 3d 2e e5 51 00 01 74 18 be 68
> [  462.021671] RIP  [<ffffffff812dde61>] ipv6_select_ident+0x31/0xa7
> [  462.027977]  RSP <ffff88043fd03758>
> [  462.031554] CR2: 0000000000000298
> [  462.034892] ---[ end trace d158e8d429419372 ]---
> [  462.039878] Kernel panic - not syncing: Fatal exception in interrupt
> 
> Any insight will be gratefully received.
> 
> Thanks,
> Chris
> 

Is it a debian kernel ?

You need : https://lkml.org/lkml/2011/10/11/291

^ permalink raw reply

* Re: ICMP packets - ll_temac with Microblaze
From: Jun Zhao @ 2011-12-21 16:11 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: monstr, David Miller, John Williams, netdev
In-Reply-To: <1324483543.2301.19.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>

On Wed, 2011-12-21 at 17:05 +0100, Eric Dumazet wrote:
> Le jeudi 22 décembre 2011 à 00:01 +0800, Jun Zhao a écrit :
> 
> > Why receive buffer size MUST bigger than the jumbo frames size even if
> > the packet size is small?
> 
> Thats the way it is in linux.
> 
> We count the memory size used by the packet.
> 
> If not, a malicious attacker could send 1-byte frames and exhaust your
> kernel memory.
> 
> If you want to reduce it, you might use copybreak : Some drivers copy
> the data into a small skb instead of providing a jumbo frame to upper
> stack.
> 
> tg3 for example.
> 
> 
> 

Eric Dumazet:

I got it, thanks for you explanation. 

^ permalink raw reply

* Re: ICMP packets - ll_temac with Microblaze
From: Eric Dumazet @ 2011-12-21 16:05 UTC (permalink / raw)
  To: Jun Zhao; +Cc: monstr, David Miller, John Williams, netdev
In-Reply-To: <1324483302.6471.3.camel@barry.pixelworks.com>

Le jeudi 22 décembre 2011 à 00:01 +0800, Jun Zhao a écrit :

> Why receive buffer size MUST bigger than the jumbo frames size even if
> the packet size is small?

Thats the way it is in linux.

We count the memory size used by the packet.

If not, a malicious attacker could send 1-byte frames and exhaust your
kernel memory.

If you want to reduce it, you might use copybreak : Some drivers copy
the data into a small skb instead of providing a jumbo frame to upper
stack.

tg3 for example.

^ permalink raw reply

* Re: ICMP packets - ll_temac with Microblaze
From: Jun Zhao @ 2011-12-21 16:01 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: monstr, David Miller, John Williams, netdev
In-Reply-To: <1324482285.2301.9.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>

On Wed, 2011-12-21 at 16:44 +0100, Eric Dumazet wrote:
> Le mercredi 21 décembre 2011 à 16:30 +0100, Eric Dumazet a écrit :
> > Le mercredi 21 décembre 2011 à 15:24 +0100, Michal Simek a écrit :
> > > Eric Dumazet wrote:
> > > > Le mercredi 21 décembre 2011 à 14:28 +0100, Michal Simek a écrit :
> > > > 
> > > >> ok. Can you provide me any background why size should be setup by
> > > >> size = SKB_WITH_OVERHEAD(ksize(data));
> > > >> and not to use size which is passed to kmalloc in __alloc_skb.
> > > > 
> > > > Its all about memory accounting (based on skb->truesize)
> > > > 
> > > > Prior to the patch, we could fool memory accounting because skbs claimed
> > > > to use less memory than what they really used.
> > > > 
> > > > And crash machines eventually.
> > > > 
> > > > Now memory accouting is fixed, we probably need to change some points in
> > > > the kernel, where we previously accepted a small skb, but not a very
> > > > large one.
> > > > 
> > > > Since "ping" probably uses SOCK_RAW sockets, I'll try this one :
> > > > 
> > > > (We dont care of _this_ skb truesize, only on the count of previously
> > > > queued packets)
> > > > 
> > > > 
> > > > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> > > > index 0da505c..a809a48 100644
> > > > --- a/net/packet/af_packet.c
> > > > +++ b/net/packet/af_packet.c
> > > > @@ -1631,8 +1631,7 @@ static int packet_rcv(struct sk_buff *skb, struct net_device *dev,
> > > >  	if (snaplen > res)
> > > >  		snaplen = res;
> > > >  
> > > > -	if (atomic_read(&sk->sk_rmem_alloc) + skb->truesize >=
> > > > -	    (unsigned)sk->sk_rcvbuf)
> > > > +	if (atomic_read(&sk->sk_rmem_alloc) >= (unsigned)sk->sk_rcvbuf)
> > > >  		goto drop_n_acct;
> > > >  
> > > >  	if (skb_shared(skb)) {
> > > > @@ -1763,7 +1762,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
> > > >  	if (po->tp_version <= TPACKET_V2) {
> > > >  		if (macoff + snaplen > po->rx_ring.frame_size) {
> > > >  			if (po->copy_thresh &&
> > > > -				atomic_read(&sk->sk_rmem_alloc) + skb->truesize
> > > > +				atomic_read(&sk->sk_rmem_alloc)
> > > >  				< (unsigned)sk->sk_rcvbuf) {
> > > >  				if (skb_shared(skb)) {
> > > >  					copy_skb = skb_clone(skb, GFP_ATOMIC);
> > > > 
> > > > 
> > > > 
> > > > 
> > > 
> > > It doesn't work too.
> > > It is possible to see this behavior on qemu. What about if I prepare you package with cross toolchain, rootfs
> > > and you can add debug message where you want?
> > > 
> > > I have also tried ll_temac driver with ppc440 and behavior is the same.
> > > 
> > > Max FRAME_SIZE pass to netdev_alloc_skb_ip_align is 7966. For this value ping works.
> > > (For ll_temac driver it is #define XTE_JUMBO_MTU 7948 from ll_temac.h)
> > 
> > I did several tests with MTU 9000 on my machines and it works well.
> > 
> > It seems my pings (iputils-sss20071127 or iputils-sss20101006) uses a
> > big enough RCVBUF
> > 
> > setsockopt(3, SOL_SOCKET, SO_RCVBUF, [65536], 4) = 0
> > getsockopt(3, SOL_SOCKET, SO_RCVBUF, [131072], [4]) = 0
> > 
> > Could you check with "strace ping..." what is doing busybox ?
> 
> I found it : Its too small for jumbo frames.
> 
> setsockopt(3, SOL_SOCKET, SO_RCVBUF, [7280], 4) = 0
> 
> networking/ping.c
> 
> 
>         /* set recv buf (needed if we can get lots of responses: flood ping,
>          * broadcast ping etc) */
>         sockopt = (datalen * 2) + 7 * 1024; /* giving it a bit of extra room */
>         setsockopt(pingsock, SOL_SOCKET, SO_RCVBUF, &sockopt, sizeof(sockopt));
> 
> 
> 
> --

Why receive buffer size MUST bigger than the jumbo frames size even if
the packet size is small?

> 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: ICMP packets - ll_temac with Microblaze
From: Eric Dumazet @ 2011-12-21 15:59 UTC (permalink / raw)
  To: monstr; +Cc: David Miller, John Williams, netdev
In-Reply-To: <4EF1EC28.90008@monstr.eu>

Le mercredi 21 décembre 2011 à 15:24 +0100, Michal Simek a écrit :
> Eric Dumazet wrote:
> > Le mercredi 21 décembre 2011 à 14:28 +0100, Michal Simek a écrit :
> > 
> >> ok. Can you provide me any background why size should be setup by
> >> size = SKB_WITH_OVERHEAD(ksize(data));
> >> and not to use size which is passed to kmalloc in __alloc_skb.
> > 
> > Its all about memory accounting (based on skb->truesize)
> > 
> > Prior to the patch, we could fool memory accounting because skbs claimed
> > to use less memory than what they really used.
> > 
> > And crash machines eventually.
> > 
> > Now memory accouting is fixed, we probably need to change some points in
> > the kernel, where we previously accepted a small skb, but not a very
> > large one.
> > 
> > Since "ping" probably uses SOCK_RAW sockets, I'll try this one :
> > 
> > (We dont care of _this_ skb truesize, only on the count of previously
> > queued packets)
> > 
> > 
> > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> > index 0da505c..a809a48 100644
> > --- a/net/packet/af_packet.c
> > +++ b/net/packet/af_packet.c
> > @@ -1631,8 +1631,7 @@ static int packet_rcv(struct sk_buff *skb, struct net_device *dev,
> >  	if (snaplen > res)
> >  		snaplen = res;
> >  
> > -	if (atomic_read(&sk->sk_rmem_alloc) + skb->truesize >=
> > -	    (unsigned)sk->sk_rcvbuf)
> > +	if (atomic_read(&sk->sk_rmem_alloc) >= (unsigned)sk->sk_rcvbuf)
> >  		goto drop_n_acct;
> >  
> >  	if (skb_shared(skb)) {
> > @@ -1763,7 +1762,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
> >  	if (po->tp_version <= TPACKET_V2) {
> >  		if (macoff + snaplen > po->rx_ring.frame_size) {
> >  			if (po->copy_thresh &&
> > -				atomic_read(&sk->sk_rmem_alloc) + skb->truesize
> > +				atomic_read(&sk->sk_rmem_alloc)
> >  				< (unsigned)sk->sk_rcvbuf) {
> >  				if (skb_shared(skb)) {
> >  					copy_skb = skb_clone(skb, GFP_ATOMIC);
> > 
> > 
> > 
> > 
> 
> It doesn't work too.
> It is possible to see this behavior on qemu. What about if I prepare you package with cross toolchain, rootfs
> and you can add debug message where you want?
> 
> I have also tried ll_temac driver with ppc440 and behavior is the same.
> 
> Max FRAME_SIZE pass to netdev_alloc_skb_ip_align is 7966. For this value ping works.
> (For ll_temac driver it is #define XTE_JUMBO_MTU 7948 from ll_temac.h)
> 

I wonder if you applied/tested my patch correctly, since it really
should had help in your case  (allowing first received packet to be
queued, even if very big)

Given the way NIC drivers work (pre-allocating big buffers before
hardware fill frames in them), SO_RCVBUF limits are difficult to
respect.

We should allow at least one frame, even with a very small SO_RCVBUF
setting, or silently cap a minimum sane value.

^ permalink raw reply


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