Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 4/4] asix: Add a new driver for the AX88172A
From: Christian Riesch @ 2012-07-11 15:10 UTC (permalink / raw)
  To: michael
  Cc: Ben Hutchings, netdev, Oliver Neukum, Eric Dumazet, Allan Chou,
	Mark Lord, Grant Grundler, Ming Lei
In-Reply-To: <CABkLObo5v00QKo-X7hEVbMcXA_QwKFA6HfL-Le5VvU2J5Cs2eg@mail.gmail.com>

Hi again,

On Wed, Jul 11, 2012 at 10:27 AM, Christian Riesch
<christian.riesch@omicron.at> wrote:
> Hi Ben and Michael,
>
> On Mon, Jul 9, 2012 at 12:30 PM, Christian Riesch
> <christian.riesch@omicron.at> wrote:
>> Hi Ben and Michael,
>>
>> On Sun, Jul 8, 2012 at 5:39 PM, Michael Riesch <michael@riesch.at> wrote:
>>> On Fri, 2012-07-06 at 18:37 +0100, Ben Hutchings wrote:
>>>> > +   priv->mdio->priv = (void *)dev;
>>>> > +   priv->mdio->read = &asix_mdio_bus_read;
>>>> > +   priv->mdio->write = &asix_mdio_bus_write;
>>>> > +   priv->mdio->name = "Asix MDIO Bus";
>>>> > +   snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "asix-%s",
>>>> > +            dev_name(dev->net->dev.parent));
>>>> [...]
>>>>
>>>> I think you need to ensure that the bus identifier is unique throughout
>>>> its lifetime, but net devices can be renamed and that could lead to a
>>>> collision.  Perhaps you could use the ifindex or the USB device path
>>>
>>> Ben,
>>>
>>> the dev_name function in the code above returns the sysfs filename of
>>> the USB device (e.g. 1-0:1.0).
>>>
>>>> (though that might be too long).
>>>
>>> This may be a problem. The bus identifier may be 17 characters long, so
>>> if we leave the endpoint/configuration part (:1.0) and the prefix away
>>> it should be fine in any "normal" system. However, on a system with a
>>> more-than-9-root-hubs 5-tier 127-devices-each USB infrastructure it
>>> results in collisions. So is this approach acceptable?
>>>
>>> Using the ifindex sounds good to me,
>>>
>>> snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "asix-%d",
>>>         dev->net->ifindex);
>>>
>>> works on any system with less than 10^12 network interfaces.
>>
>> Ok, I'll change that to use ifindex.
>
> No, I won't.
> At the time the mdio bus is registered, ifindex is not yet set, so the
> snprintf would always result in "asix-0".

What do you think about
snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "usb-%03d:%03d",
dev->udev->bus->busnum, dev->udev->devnum);
??

This would use the busnum/devnum identifier as reported by lsusb and
would be short enough for an mdio bus name.

Thanks, Christian

^ permalink raw reply

* Re: FW: [patch] net/mlx4_en: dereferencing freed memory
From: Hadar Hen Zion @ 2012-07-11 15:01 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: David S. Miller, amirv, Or Gerlitz, Alexander Guller, netdev,
	kernel-janitors
In-Reply-To: <EB097B4067A7C64DA511817018705AE595EDE0BE@MTLDAG02.mtl.com>

On 7/11/2012 11:32 AM, Amir Vadai wrote:
>
>
> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> Sent: Wednesday, July 11, 2012 9:34 AM
> To: Yevgeny Petrilin
> Cc: David S. Miller; Amir Vadai; Or Gerlitz; Alexander Guller; netdev@vger.kernel.org; kernel-janitors@vger.kernel.org
> Subject: [patch] net/mlx4_en: dereferencing freed memory
>
> We dereferenced "mclist" after the kfree().
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index 94375a8..4ce5ca8 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -503,9 +503,7 @@ static void mlx4_en_do_set_multicast(struct work_struct *work)
>   				/* remove from list */
>   				list_del(&mclist->list);
>   				kfree(mclist);
> -			}
> -
> -			if (mclist->action == MCLIST_ADD) {
> +			} else if (mclist->action == MCLIST_ADD) {
>   				/* attach the address */
>   				memcpy(&mc_list[10], mclist->addr, ETH_ALEN);
>   				/* needed for B0 steering support */
>

Hi Dan,

It's the same in here. This is indeed a bug, thanks for spotting this over,

Please add:
Acked-by: Hadar Hen Zion <hadarh@mellanox.co.il>

Hadar

^ permalink raw reply

* Re: [patch] net/mlx4: off by one in parse_trans_rule()
From: Hadar Hen Zion @ 2012-07-11 14:51 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Hadar Hen Zion, David S. Miller, Or Gerlitz, Eugenia Emantayev,
	Yevgeny Petrilin, netdev, kernel-janitors
In-Reply-To: <20120711063336.GC11812@elgon.mountain>

On 7/11/2012 9:33 AM, Dan Carpenter wrote:
> This should be ">=" here instead of ">".  MLX4_NET_TRANS_RULE_NUM is 6.
> We use "spec->id" as an array offset into the __rule_hw_sz[] and
> __sw_id_hw[] arrays which have 6 elements.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/mcg.c b/drivers/net/ethernet/mellanox/mlx4/mcg.c
> index bc62f53..5bac0df 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/mcg.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/mcg.c
> @@ -773,7 +773,7 @@ static int parse_trans_rule(struct mlx4_dev *dev, struct mlx4_spec_list *spec,
>   		[MLX4_NET_TRANS_RULE_ID_UDP] =
>   			sizeof(struct mlx4_net_trans_rule_hw_tcp_udp)
>   	};
> -	if (spec->id > MLX4_NET_TRANS_RULE_NUM) {
> +	if (spec->id >= MLX4_NET_TRANS_RULE_NUM) {
>   		mlx4_err(dev, "Invalid network rule id. id = %d\n", spec->id);
>   		return -EINVAL;
>   	}
>

Hi Dan,

This is indeed a bug, thanks for spotting this over,

Please add:
Acked-by: Hadar Hen Zion <hadarh@mellanox.co.il>

Hadar

^ permalink raw reply

* Re: [PATCH] tc_util: fix incorrect bare number process in get_rate.
From: Stephen Hemminger @ 2012-07-11 14:51 UTC (permalink / raw)
  To: Li Wei; +Cc: netdev
In-Reply-To: <4FFD2A42.9080507@cn.fujitsu.com>

On Wed, 11 Jul 2012 15:24:50 +0800
Li Wei <lw@cn.fujitsu.com> wrote:

> 
> As the comment and manpage indicated that the bare number means
> bytes per second, so the division is not needed.
> ---
>  tc/tc_util.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/tc/tc_util.c b/tc/tc_util.c
> index 926ed08..e8d89c1 100644
> --- a/tc/tc_util.c
> +++ b/tc/tc_util.c
> @@ -153,7 +153,7 @@ int get_rate(unsigned *rate, const char *str)
>  		return -1;
>  
>  	if (*p == '\0') {
> -		*rate = bps / 8.;	/* assume bytes/sec */
> +		*rate = bps;	/* assume bytes/sec */
>  		return 0;
>  	}
>  
Thanks for finding this. The documentation, code and comment do
all need to be the same!

But changing the code as you propose would break existing usage
by scripts. Instead, the man page and comment need to change
to match the reality of the existing application.

^ permalink raw reply

* pull-request: can-next 2012-07-11
From: Marc Kleine-Budde @ 2012-07-11 14:05 UTC (permalink / raw)
  To: David Miller; +Cc: Linux Netdev List, linux-can@vger.kernel.org

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

Hello David,

the fourth pull request for upcoming v3.6 net-next consist of a series
of can_gw netlink cleanups done by Thomas Graf.

regards, Marc

--

The following changes since commit 061a5c316b6526dbc729049a16243ec27937cc31:

  Merge branch 'davem-next.r8169' of git://violet.fr.zoreil.com/romieu/linux (2012-07-09 16:09:47 -0700)

are available in the git repository at:


  git://gitorious.org/linux-can/linux-can-next.git for-davem

for you to fetch changes up to 5d91efa8dd8ced8647798d067f2ac8125194be58:

  can: gw: Remove pointless casts (2012-07-10 22:36:17 +0200)

----------------------------------------------------------------
Thomas Graf (4):
      can: gw: Don't bump nlmsg_len manually
      can: gw: Use nla_policy to validate netlink attributes
      can: gw: Properly fill the netlink header when responding to RTM_GETROUTE
      can: gw: Remove pointless casts

 net/can/gw.c |   90 +++++++++++++++++++++-------------------------------------
 1 file changed, 33 insertions(+), 57 deletions(-)

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

^ permalink raw reply

* Re: UDP ordering when using multiple rx queue
From: Jean-Michel Hautbois @ 2012-07-11 13:41 UTC (permalink / raw)
  To: Merav Sicron; +Cc: netdev
In-Reply-To: <CAL8zT=jMPCv_Lc7gJYGHRhHZop9aWJfp2w1o_cD0Z8SRdtck7Q@mail.gmail.com>

2012/7/11 Jean-Michel Hautbois <jhautbois@gmail.com>:
> 2012/7/11 Merav Sicron <meravs@broadcom.com>:
>> On Wed, 2012-07-11 at 00:53 -0700, Jean-Michel Hautbois wrote:
>>
>>> Several tests lead to a simple conclusion : when the NIC has only one
>>> RX queue, everything is ok (like be2net for instance), but when it has
>>> more than one RX queue, then I can have "lost packets".
>>> This is the case for bnx2x or mlx4 for instance.
>> >From what you describe I assume that you use different source IP /
>> destination IP in each packet - is this something that you can control?
>> Because with the same IP addresses the traffic will be steered to the
>> same queue.
>
> OK, sorry for not having explained that : the packets are multicast
> with a port for each stream. Sending one stream multicast on a bnx2x
> based NIC can lead to several queues used (two, for what I can see)
> and then, to the problem reported.
>
>>> Here are my questions :
>>> - Is it possible to force a driver to use only one rx queue, even if
>>> it can use more without reloading the driver (and this is feasible
>>> only when a parameter exists for that !) ?
>> You can reduce the number of queues using "ethtool -L ethX combined 1".
>> Note however that it will cause automatic driver unload/load.
>
> OK, thanks for this tip :).
>
> JM

I confirm that using ethtool -L eth1 combined 1 solves my issue.
I can have 3Gbps per sec with 5 multicast on 5 ports without any
"packet loss" (again, for my application) and it uses one RX queue
only (of course :)).
One multicast (one port) but with the default combined=8 splits in two
rx queues...
Unicast traffic seems ok (I used netperf in order to check this assumption).

JM

^ permalink raw reply

* The Items are needed urgently and also in large quantity
From: Kapil Bhardwaj @ 2012-07-11  3:07 UTC (permalink / raw)
  To: netdev

Compliments of the day,

My company is interested in a different but a similar design of your product. please confirm to us if your company can make provision for the exact product as shown on the Chinese trading firm page below. You can view the sample pictures by loggin in to the link below with your correct business email to view pictures. Please note that this items are needed urgently and also in large quantity.

http://www.elcivismo.com.ar/img/viewtradeorder.htm

Pls send us quote after preview immediately.

Thanks & Regards,
Kapil Bhardwaj
(MANAGING PARTENER)
M/S Shree Ganesh Tradings
69-A, Vijay Block
Laxmi Nagar
Delhi-110092
Tel.: 0091 11 47603504
Fax.: 0091 11 47603501

^ permalink raw reply

* 3.5-rc6: kernel BUG at kernel/timer.c:711!
From: Dave Jones @ 2012-07-11 13:03 UTC (permalink / raw)
  To: netdev; +Cc: Linux Kernel

[34323.844970] ------------[ cut here ]------------
[34323.845555] kernel BUG at kernel/timer.c:711!
[34323.846110] invalid opcode: 0000 [#1] PREEMPT SMP 
[34323.846657] CPU 6 
[34323.846670] Modules linked in:
[34323.853968]  ebtables
[34323.866941]  xt_cpu
[34323.890355]  msdos
[34323.920927]  ppp_synctty
[34323.959787]  raid6_pq
[34323.964221]  raid10 shpchp fakephp aer_inject ptp pps_core target_core_file target_core_iblock target_core_pscsi tcm_loop target_core_mod vga16fb sysimgblt fb_sys_fops syscopyarea vgastate output platform_lcd lcd sysfillrect n_r3964 n_gsm nozomi jsm serio_raw altera_ps2 input_polldev sparse_keymap uinput fuse ipt_ULOG nfnetlink tun binfmt_misc sctp libcrc32c caif_socket caif phonet bluetooth rfkill can llc2 pppoe pppox ppp_generic slhc irda crc_ccitt rds af_key decnet rose x25 atm netrom appletalk ipx p8023 psnap p8022 llc ax25 ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 xt_state nf_conntrack ip6table_filter ip6_tables kvm_intel kvm crc32c_intel ghash_clmulni_intel microcode pcspkr usb_debug i2c_i801 e1000e nfsd nfs_acl auth_rpcgss lockd sunrpc i915 video i2c_algo_bit drm_kms_helper dr
 m i2c_core [last unloaded: scsi_wait_scan]
[34323.997915] 
[34323.999802] Pid: 15419, comm: trinity-child6 Not tainted 3.5.0-rc6+ #102
[34324.004383] RIP: 0010:[<ffffffff8107de30>]  [<ffffffff8107de30>] mod_timer+0x4a0/0x4c0
[34324.006791] RSP: 0018:ffff88004567be08  EFLAGS: 00010246
[34324.008623] RAX: 0000000000000000 RBX: ffff88010eb916b0 RCX: 000000000000000b
[34324.010443] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff88004567be10
[34324.012731] RBP: ffff88004567be48 R08: 0000000000000000 R09: 0000000000000000
[34324.014511] R10: 0000000000000001 R11: 0000000000000000 R12: 00000001003f0800
[34324.016552] R13: 00000001003f0b71 R14: ffff880123269600 R15: 00000000ffffff01
[34324.018297] FS:  00007fc0dc317740(0000) GS:ffff880148800000(0000) knlGS:0000000000000000
[34324.020111] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[34324.021834] CR2: 00007fc0da305000 CR3: 000000001bad8000 CR4: 00000000001407e0
[34324.023532] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[34324.025221] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[34324.026874] Process trinity-child6 (pid: 15419, threadinfo ffff88004567a000, task ffff88002ee58000)
[34324.028515] Stack:
[34324.030127]  ffffffff8154108d 0000000000000b04 ffff88004567be28 ffff88010eb91290
[34324.031914]  0000000000000009 0000000000000004 ffff880123269600 00000000ffffff01
[34324.034029]  ffff88004567be68 ffffffff81540d1c ffffffff8154518c ffff88010eb91290
[34324.036201] Call Trace:
[34324.037779]  [<ffffffff8154108d>] ? lock_sock_nested+0x8d/0xa0
[34324.039370]  [<ffffffff81540d1c>] sk_reset_timer+0x1c/0x30
[34324.040921]  [<ffffffff8154518c>] ? sock_setsockopt+0x8c/0x950
[34324.042454]  [<ffffffff8159d350>] inet_csk_reset_keepalive_timer+0x20/0x30
[34324.043950]  [<ffffffff815b491d>] tcp_set_keepalive+0x3d/0x50
[34324.045483]  [<ffffffff81545a13>] sock_setsockopt+0x913/0x950
[34324.046946]  [<ffffffff811d5b0a>] ? fget_light+0x3ca/0x500
[34324.048379]  [<ffffffff81696b19>] ? sysret_check+0x22/0x5d
[34324.050103]  [<ffffffff8153f536>] sys_setsockopt+0xc6/0xe0
[34324.051593]  [<ffffffff81696aed>] system_call_fastpath+0x1a/0x1f
[34324.052966] Code: 4d ff ff ff 48 c7 c7 60 10 c3 81 e8 7b 6e 05 00 85 c0 0f 85 b8 fd ff ff eb 93 48 8b 75 08 48 89 df e8 85 f0 ff ff e9 df fb ff ff <0f> 0b 4d 89 f5 e9 ad fc ff ff 66 0f 1f 44 00 00 e8 c6 38 60 00 
[34324.057257] RIP  [<ffffffff8107de30>] mod_timer+0x4a0/0x4c0
[34324.059070]  RSP <ffff88004567be08>
[34324.062014] ---[ end trace fedc8673629f8303 ]---

That's...

	BUG_ON(!timer->function);

^ permalink raw reply

* Re: [PATCH v3] net: cgroup: fix access the unallocated memory in netprio cgroup
From: Gao feng @ 2012-07-11 12:51 UTC (permalink / raw)
  To: Neil Horman
  Cc: eric.dumazet, linux-kernel, netdev, lizefan, tj, davem,
	Eric Dumazet
In-Reply-To: <20120711121102.GB26643@hmsreliant.think-freely.org>

于 2012年07月11日 20:11, Neil Horman 写道:
> On Wed, Jul 11, 2012 at 04:30:06PM +0800, Gao feng wrote:
>> there are some out of bound accesses in netprio cgroup.
>>
>> now before accessing the dev->priomap.priomap array,we only check
>> if the dev->priomap exist.and because we don't want to see
>> additional bound checkings in fast path, so we should make sure
>> that dev->priomap is null or array size of dev->priomap.priomap
>> is equal to max_prioidx + 1;
>>
>> and it's not needed to call extend_netdev_tabel in write_priomap,
>> we can only allocate the net device's priomap which we change through
>> net_prio.ifpriomap.
>>
>> this patch add a return value for update_netdev_tables & extend_netdev_table,
>> so when new_priomap is allocated failed,write_priomap will stop to access
>> the priomap,and return -ENOMEM back to the userspace to tell the user
>> what happend.
>>
>> Change From v2:
>> 1. protect extend_netdev_table by RTNL.
>> 2. when extend_netdev_table failed,call dev_put to reduce device's refcount.
>>
>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
>> Cc: Neil Horman <nhorman@tuxdriver.com>
>> Cc: Eric Dumazet <edumazet@google.com>
>> ---
>>  net/core/netprio_cgroup.c |   54 ++++++++++++++++++++++++++++++++------------
>>  1 files changed, 39 insertions(+), 15 deletions(-)
>>
> I still think the use of max_priomap in write_priomap is racy (please see my
> previous note).
> 
> Neil

Yes, you are right :(
we need a v4 patch.

Thanks!

^ permalink raw reply

* [PATCH net-next] r8169: Remove rtl_ocpdr_cond
From: Hayes Wang @ 2012-07-11 12:31 UTC (permalink / raw)
  To: romieu; +Cc: netdev, linux-kernel, Hayes Wang

No waiting is needed for mac_ocp_{write / read}. And the bit 31 of
OCPDR would not change, so rtl_udelay_loop_wait_high always return
false. That is, the r8168_mac_ocp_read always retuen ~0.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/ethernet/realtek/r8169.c |   12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index c29c5fb..1f27318 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -1043,13 +1043,6 @@ static void rtl_w1w0_phy_ocp(struct rtl8169_private *tp, int reg, int p, int m)
 	r8168_phy_ocp_write(tp, reg, (val | p) & ~m);
 }
 
-DECLARE_RTL_COND(rtl_ocpdr_cond)
-{
-	void __iomem *ioaddr = tp->mmio_addr;
-
-	return RTL_R32(OCPDR) & OCPAR_FLAG;
-}
-
 static void r8168_mac_ocp_write(struct rtl8169_private *tp, u32 reg, u32 data)
 {
 	void __iomem *ioaddr = tp->mmio_addr;
@@ -1058,8 +1051,6 @@ static void r8168_mac_ocp_write(struct rtl8169_private *tp, u32 reg, u32 data)
 		return;
 
 	RTL_W32(OCPDR, OCPAR_FLAG | (reg << 15) | data);
-
-	rtl_udelay_loop_wait_low(tp, &rtl_ocpdr_cond, 25, 10);
 }
 
 static u16 r8168_mac_ocp_read(struct rtl8169_private *tp, u32 reg)
@@ -1071,8 +1062,7 @@ static u16 r8168_mac_ocp_read(struct rtl8169_private *tp, u32 reg)
 
 	RTL_W32(OCPDR, reg << 15);
 
-	return rtl_udelay_loop_wait_high(tp, &rtl_ocpdr_cond, 25, 10) ?
-		RTL_R32(OCPDR) : ~0;
+	return RTL_R32(OCPDR);
 }
 
 #define OCP_STD_PHY_BASE	0xa400
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH v3] net: cgroup: fix access the unallocated memory in netprio cgroup
From: Neil Horman @ 2012-07-11 12:11 UTC (permalink / raw)
  To: Gao feng
  Cc: eric.dumazet, linux-kernel, netdev, lizefan, tj, davem,
	Eric Dumazet
In-Reply-To: <1341995406-12719-1-git-send-email-gaofeng@cn.fujitsu.com>

On Wed, Jul 11, 2012 at 04:30:06PM +0800, Gao feng wrote:
> there are some out of bound accesses in netprio cgroup.
> 
> now before accessing the dev->priomap.priomap array,we only check
> if the dev->priomap exist.and because we don't want to see
> additional bound checkings in fast path, so we should make sure
> that dev->priomap is null or array size of dev->priomap.priomap
> is equal to max_prioidx + 1;
> 
> and it's not needed to call extend_netdev_tabel in write_priomap,
> we can only allocate the net device's priomap which we change through
> net_prio.ifpriomap.
> 
> this patch add a return value for update_netdev_tables & extend_netdev_table,
> so when new_priomap is allocated failed,write_priomap will stop to access
> the priomap,and return -ENOMEM back to the userspace to tell the user
> what happend.
> 
> Change From v2:
> 1. protect extend_netdev_table by RTNL.
> 2. when extend_netdev_table failed,call dev_put to reduce device's refcount.
> 
> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> Cc: Neil Horman <nhorman@tuxdriver.com>
> Cc: Eric Dumazet <edumazet@google.com>
> ---
>  net/core/netprio_cgroup.c |   54 ++++++++++++++++++++++++++++++++------------
>  1 files changed, 39 insertions(+), 15 deletions(-)
> 
I still think the use of max_priomap in write_priomap is racy (please see my
previous note).

Neil

^ permalink raw reply

* [PATCH firmware] rtl_nic: update firmware for RTL8168G
From: Hayes Wang @ 2012-07-11 12:10 UTC (permalink / raw)
  To: dwmw2, ben; +Cc: romieu, netdev, linux-kernel, Hayes Wang
In-Reply-To: <1341878937.25597.309.camel@deadeye.wl.decadent.org.uk>

File: rtl_nic/rtl8168g-1.fw
Version: 0.0.2

Change the ocp_base of linux driver to OCP_STD_PHY_BASE after setting
firmware. The firmware would modify the ocp_base, and that results the
driver uses the wrong ocp_base to access standard phy after setting
firmware.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 WHENCE                |    2 +-
 rtl_nic/rtl8168g-1.fw |  Bin 4272 -> 4272 bytes
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/WHENCE b/WHENCE
index 1fb7951..f46c842 100644
--- a/WHENCE
+++ b/WHENCE
@@ -1817,7 +1817,7 @@ File: rtl_nic/rtl8106e-1.fw
 Version: 0.0.1
 
 File: rtl_nic/rtl8168g-1.fw
-Version: 0.0.1
+Version: 0.0.2
 
 Licence:
  * Copyright © 2011, Realtek Semiconductor Corporation
diff --git a/rtl_nic/rtl8168g-1.fw b/rtl_nic/rtl8168g-1.fw
index dace1ea98c66de08107ee0857d8a8680d57d7898..bc03a5d28fd07d99a6cc89aa115d8ef8942629a6 100644
GIT binary patch
delta 78
zcmdm>xIs~u0SJmpax4tZEYfui;|=r-^o$e?%=HZo^$m?cl0c9E#6m0#40#iERoN}A
WOw6oI3^qo33-CB_$u|JyzyJVS1PxdK

delta 78
zcmdm>xIs~u0SJmpax4tZEYfui;|=r-^b8dY%=C>c^$m?cl0c9E#DXjg3}F*>RoTs~
Qj7)%NW2CnL4@eRQ08i5lSO5S3

-- 
1.7.10.4

^ permalink raw reply related

* [RFC PATCH] tun: don't zeroize sock->file on detach
From: Stanislav Kinsbursky @ 2012-07-11 11:48 UTC (permalink / raw)
  To: davem; +Cc: netdev, ruanzhijie, linux-kernel

This is a fix for bug, introduced in 3.4 kernel by commit
1ab5ecb90cb6a3df1476e052f76a6e8f6511cb3d, which, among other things, replaced
simple sock_put() by sk_release_kernel(). Below is sequence, which leads to
oops for non-persistent devices:

tun_chr_close()
tun_detach()				<== tun->socket.file = NULL
tun_free_netdev()
sk_release_sock()
sock_release(sock->file == NULL)
iput(SOCK_INODE(sock))			<== dereference on NULL pointer

This patch just removes zeroing of socket's file from __tun_detach().
sock_release() will do this.

Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
---
 drivers/net/tun.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 987aeef..c1639f3 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -185,7 +185,6 @@ static void __tun_detach(struct tun_struct *tun)
 	netif_tx_lock_bh(tun->dev);
 	netif_carrier_off(tun->dev);
 	tun->tfile = NULL;
-	tun->socket.file = NULL;
 	netif_tx_unlock_bh(tun->dev);
 
 	/* Drop read queue */

^ permalink raw reply related

* Re: [PATCH net-next 3/9] IB/ipoib: Add support for acting as VIF
From: Or Gerlitz @ 2012-07-11 11:46 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, roland, netdev, ali, sean.hefty, erezsh
In-Reply-To: <1342002453.3265.7447.camel@edumazet-glaptop>

On 7/11/2012 1:27 PM, Eric Dumazet wrote:
> On Wed, 2012-07-11 at 03:06 -0700, David Miller wrote:
>> We have them on the RX flow too, they are called ingress qdiscs (as
>> opposed to egress qdiscs on the TX side).
>>
>> There is nothing mysterious about them, they've been there for more
>> than 15 years.
>
> Sample script [...]

Hi Eric, thanks for the crash / detailed answer... would you agree to 
what I wrote, so we can
use the last twenty bytes of skb->cb[] e.g in a similar manner to the 
way struct ipoib_cb works?

Or.

^ permalink raw reply

* Re: UDP ordering when using multiple rx queue
From: Jean-Michel Hautbois @ 2012-07-11 11:13 UTC (permalink / raw)
  To: Merav Sicron; +Cc: netdev
In-Reply-To: <1342004939.27284.28.camel@lb-tlvb-meravs.il.broadcom.com>

2012/7/11 Merav Sicron <meravs@broadcom.com>:
> On Wed, 2012-07-11 at 00:53 -0700, Jean-Michel Hautbois wrote:
>
>> Several tests lead to a simple conclusion : when the NIC has only one
>> RX queue, everything is ok (like be2net for instance), but when it has
>> more than one RX queue, then I can have "lost packets".
>> This is the case for bnx2x or mlx4 for instance.
> >From what you describe I assume that you use different source IP /
> destination IP in each packet - is this something that you can control?
> Because with the same IP addresses the traffic will be steered to the
> same queue.

OK, sorry for not having explained that : the packets are multicast
with a port for each stream. Sending one stream multicast on a bnx2x
based NIC can lead to several queues used (two, for what I can see)
and then, to the problem reported.

>> Here are my questions :
>> - Is it possible to force a driver to use only one rx queue, even if
>> it can use more without reloading the driver (and this is feasible
>> only when a parameter exists for that !) ?
> You can reduce the number of queues using "ethtool -L ethX combined 1".
> Note however that it will cause automatic driver unload/load.

OK, thanks for this tip :).

JM

^ permalink raw reply

* Re: UDP ordering when using multiple rx queue
From: Merav Sicron @ 2012-07-11 11:08 UTC (permalink / raw)
  To: Jean-Michel Hautbois; +Cc: netdev
In-Reply-To: <CAL8zT=g5nHd6FprhxFc21XTgfXeaokt4QN1fS4ekcNVkuvZE9g@mail.gmail.com>

On Wed, 2012-07-11 at 00:53 -0700, Jean-Michel Hautbois wrote:

> Several tests lead to a simple conclusion : when the NIC has only one
> RX queue, everything is ok (like be2net for instance), but when it has
> more than one RX queue, then I can have "lost packets".
> This is the case for bnx2x or mlx4 for instance.
>From what you describe I assume that you use different source IP /
destination IP in each packet - is this something that you can control?
Because with the same IP addresses the traffic will be steered to the
same queue. 

> Here are my questions :
> - Is it possible to force a driver to use only one rx queue, even if
> it can use more without reloading the driver (and this is feasible
> only when a parameter exists for that !) ?
You can reduce the number of queues using "ethtool -L ethX combined 1".
Note however that it will cause automatic driver unload/load.

Thanks,
Merav

^ permalink raw reply

* Re: [PATCH net-next 3/9] IB/ipoib: Add support for acting as VIF
From: Eric Dumazet @ 2012-07-11 10:27 UTC (permalink / raw)
  To: David Miller; +Cc: ogerlitz, roland, netdev, ali, sean.hefty, erezsh
In-Reply-To: <20120711.030639.1906839917411889365.davem@davemloft.net>

On Wed, 2012-07-11 at 03:06 -0700, David Miller wrote:
> From: Or Gerlitz <ogerlitz@mellanox.com>
> Date: Wed, 11 Jul 2012 12:59:41 +0300
> 
> > On 7/11/2012 11:19 AM, David Miller wrote:
> >> handle_ing() goes into the packet scheduler and qdisc layer, the qdisc
> >> layer uses skb->cb[] via struct qdisc_skb_cb
> > 
> > Oh, I knew that the qdisc layer touches skb->cb[] but thought it only
> > happens on the TX flow...
> 
> We have them on the RX flow too, they are called ingress qdiscs (as
> opposed to egress qdiscs on the TX side).
> 
> There is nothing mysterious about them, they've been there for more
> than 15 years.

Sample script 

ETH=eth0
IFB=ifb0
LOCALNETS="172.16.0.0/12 192.168.0.0/16 10.0.0.0/8"
RATE="rate 7Mbit bandwidth 7Mbit maxburst 80 minburst 40"
ALLOT="allot 8000" 

modprobe ifb
ip link set dev $IFB up

tc qdisc add dev $ETH ingress 2>/dev/null

tc filter add dev $ETH parent ffff: \
   protocol ip u32 match u32 0 0 flowid 1:1 action mirred egress \
   redirect dev $IFB

tc qdisc del dev $IFB root


# Lets say our NIC is 100Mbit
tc qdisc add dev $IFB root handle 1: cbq avpkt 1000 \
    rate 100Mbit bandwidth 100Mbit

tc class add dev $IFB parent 1: classid 1:1 cbq allot 10000 \
	mpu 64 rate 100Mbit prio 1 \
	bandwidth 100Mbit maxburst 150 avpkt 1500 bounded

# Class for traffic coming from Internet : limited to X Mbits
tc class add dev $IFB parent 1:1 classid 1:11 \
	cbq $ALLOT mpu 64      \
	$RATE prio 2 \
	avpkt 1400 bounded

tc qdisc add dev $IFB parent 1:11 handle 11: fq_codel


# Traffic from machines in our LAN : no limit
for privnet in $LOCALNETS
do
	tc filter add dev $IFB parent 1: protocol ip prio 2 u32 \
		match ip src $privnet flowid 1:1
done

tc filter add dev $IFB parent 1: protocol ip prio 2 u32 \
	match ip protocol 0 0x00 flowid 1:11

^ permalink raw reply

* Re: [PATCH net-next 3/9] IB/ipoib: Add support for acting as VIF
From: David Miller @ 2012-07-11 10:06 UTC (permalink / raw)
  To: ogerlitz; +Cc: eric.dumazet, roland, netdev, ali, sean.hefty, erezsh
In-Reply-To: <4FFD4E8D.7040608@mellanox.com>

From: Or Gerlitz <ogerlitz@mellanox.com>
Date: Wed, 11 Jul 2012 12:59:41 +0300

> On 7/11/2012 11:19 AM, David Miller wrote:
>> handle_ing() goes into the packet scheduler and qdisc layer, the qdisc
>> layer uses skb->cb[] via struct qdisc_skb_cb
> 
> Oh, I knew that the qdisc layer touches skb->cb[] but thought it only
> happens on the TX flow...

We have them on the RX flow too, they are called ingress qdiscs (as
opposed to egress qdiscs on the TX side).

There is nothing mysterious about them, they've been there for more
than 15 years.

^ permalink raw reply

* Re: [PATCH net-next 3/9] IB/ipoib: Add support for acting as VIF
From: Or Gerlitz @ 2012-07-11  9:59 UTC (permalink / raw)
  To: David Miller, eric.dumazet; +Cc: roland, netdev, ali, sean.hefty, erezsh
In-Reply-To: <20120711.011912.2063611962030549397.davem@davemloft.net>

On 7/11/2012 11:19 AM, David Miller wrote:
> handle_ing() goes into the packet scheduler and qdisc layer, the qdisc 
> layer uses skb->cb[] via struct qdisc_skb_cb

Oh, I knew that the qdisc layer touches skb->cb[] but thought it only 
happens on the TX flow...

Here, the flow Eric pointed on relates to skb which was received by 
IPoIB and is soon going
to be consumed by the eIPoIB driver once the rx handler planted by it is 
executed on the skb.

So even in this "simple" flow we can't assume that qdisc will not 
interfere (why)?

if indeed this is case - I understand that we still can deploy the 
practice used by
ipoib_hard_header which uses struct ipoib_cb, or in other words we can 
use 20 out
of the 48 bytes of the cb, starting from offset of 28 bytes, correct?

Or.

^ permalink raw reply

* Re: [net-next:master 73/84] net/ipv4/tcp_metrics.c:252:3: error: implicit declaration of function 'inet6_twsk'
From: Fengguang Wu @ 2012-07-11  9:44 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, kernel-janitors
In-Reply-To: <20120711.023939.1269322960321058056.davem@davemloft.net>

On Wed, Jul 11, 2012 at 02:39:39AM -0700, David Miller wrote:
> From: wfg@linux.intel.com
> Date: Wed, 11 Jul 2012 17:12:33 +0800
> 
> > net/ipv4/tcp_metrics.c: In function '__tcp_get_metrics_tw':
> > net/ipv4/tcp_metrics.c:252:3: error: implicit declaration of function 'inet6_twsk' [-Werror=implicit-function-declaration]
> > net/ipv4/tcp_metrics.c:252:7: warning: assignment makes pointer from integer without a cast [enabled by default]
> > net/ipv4/tcp_metrics.c:253:41: error: dereferencing pointer to incomplete type
> 
> Fixed as follows:
> 
> ====================
> ipv6: Move ipv6 twsk accessors outside of CONFIG_IPV6 ifdefs.
> 
> Fixes build when ipv6 is disabled.
> 
> Reported-by: Fengguang Wu <wfg@linux.intel.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
>  include/linux/ipv6.h |   32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)

It worked, thanks!

Fengguang

^ permalink raw reply

* Re: [net-next:master 73/84] net/ipv4/tcp_metrics.c:252:3: error: implicit declaration of function 'inet6_twsk'
From: David Miller @ 2012-07-11  9:39 UTC (permalink / raw)
  To: wfg; +Cc: netdev, kernel-janitors
In-Reply-To: <20120711091233.GA24436@localhost>

From: wfg@linux.intel.com
Date: Wed, 11 Jul 2012 17:12:33 +0800

> net/ipv4/tcp_metrics.c: In function '__tcp_get_metrics_tw':
> net/ipv4/tcp_metrics.c:252:3: error: implicit declaration of function 'inet6_twsk' [-Werror=implicit-function-declaration]
> net/ipv4/tcp_metrics.c:252:7: warning: assignment makes pointer from integer without a cast [enabled by default]
> net/ipv4/tcp_metrics.c:253:41: error: dereferencing pointer to incomplete type

Fixed as follows:

====================
ipv6: Move ipv6 twsk accessors outside of CONFIG_IPV6 ifdefs.

Fixes build when ipv6 is disabled.

Reported-by: Fengguang Wu <wfg@linux.intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 include/linux/ipv6.h |   32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 8260ef7..bc6c8fd 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -410,6 +410,22 @@ struct tcp6_sock {
 
 extern int inet6_sk_rebuild_header(struct sock *sk);
 
+struct inet6_timewait_sock {
+	struct in6_addr tw_v6_daddr;
+	struct in6_addr	tw_v6_rcv_saddr;
+};
+
+struct tcp6_timewait_sock {
+	struct tcp_timewait_sock   tcp6tw_tcp;
+	struct inet6_timewait_sock tcp6tw_inet6;
+};
+
+static inline struct inet6_timewait_sock *inet6_twsk(const struct sock *sk)
+{
+	return (struct inet6_timewait_sock *)(((u8 *)sk) +
+					      inet_twsk(sk)->tw_ipv6_offset);
+}
+
 #if IS_ENABLED(CONFIG_IPV6)
 static inline struct ipv6_pinfo * inet6_sk(const struct sock *__sk)
 {
@@ -459,28 +475,12 @@ static inline void inet_sk_copy_descendant(struct sock *sk_to,
 #define __ipv6_only_sock(sk)	(inet6_sk(sk)->ipv6only)
 #define ipv6_only_sock(sk)	((sk)->sk_family == PF_INET6 && __ipv6_only_sock(sk))
 
-struct inet6_timewait_sock {
-	struct in6_addr tw_v6_daddr;
-	struct in6_addr	tw_v6_rcv_saddr;
-};
-
-struct tcp6_timewait_sock {
-	struct tcp_timewait_sock   tcp6tw_tcp;
-	struct inet6_timewait_sock tcp6tw_inet6;
-};
-
 static inline u16 inet6_tw_offset(const struct proto *prot)
 {
 	return prot->twsk_prot->twsk_obj_size -
 			sizeof(struct inet6_timewait_sock);
 }
 
-static inline struct inet6_timewait_sock *inet6_twsk(const struct sock *sk)
-{
-	return (struct inet6_timewait_sock *)(((u8 *)sk) +
-					      inet_twsk(sk)->tw_ipv6_offset);
-}
-
 static inline struct in6_addr *__inet6_rcv_saddr(const struct sock *sk)
 {
 	return likely(sk->sk_state != TCP_TIME_WAIT) ?
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH v2 4/5] cgroup: Make builtin vs. module build ifdefs consistent
From: Daniel Wagner @ 2012-07-11  9:35 UTC (permalink / raw)
  To: cgroups
  Cc: netdev, Daniel Wagner, Jamal Hadi Salim, Tejun Heo, Li Zefan,
	David S. Miller
In-Reply-To: <1341999341-1808-1-git-send-email-wagi@monom.org>

From: Daniel Wagner <daniel.wagner@bmw-carit.de>

The header file is using IS_MODULE and IS_BUILTIN macros. So let's
use them in the source file too.

Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Li Zefan <lizefan@huawei.com>
Cc: "David S. Miller" <davem@davemloft.net>
---
 net/sched/cls_cgroup.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
index 7743ea8..4bfb8f7 100644
--- a/net/sched/cls_cgroup.c
+++ b/net/sched/cls_cgroup.c
@@ -77,7 +77,7 @@ struct cgroup_subsys net_cls_subsys = {
 	.name		= "net_cls",
 	.create		= cgrp_create,
 	.destroy	= cgrp_destroy,
-#ifdef CONFIG_NET_CLS_CGROUP
+#if IS_BUILTIN(CONFIG_NET_CLS_CGROUP)
 	.subsys_id	= net_cls_subsys_id,
 #endif
 	.base_cftypes	= ss_files,
@@ -283,7 +283,7 @@ static int __init init_cgroup_cls(void)
 	if (ret)
 		goto out;
 
-#ifndef CONFIG_NET_CLS_CGROUP
+#if IS_MODULE(CONFIG_NET_CLS_CGROUP)
 	/* We can't use rcu_assign_pointer because this is an int. */
 	smp_wmb();
 	net_cls_subsys_id = net_cls_subsys.subsys_id;
@@ -301,7 +301,7 @@ static void __exit exit_cgroup_cls(void)
 {
 	unregister_tcf_proto_ops(&cls_cgroup_ops);
 
-#ifndef CONFIG_NET_CLS_CGROUP
+#if IS_MODULE(CONFIG_NET_CLS_CGROUP)
 	net_cls_subsys_id = -1;
 	synchronize_rcu();
 #endif
-- 
1.7.11.1.165.g299666c

^ permalink raw reply related

* [PATCH v2 3/5] cgroup: Do not add sock_update_netprioidx() when CONFIG_NETPRIO_CGROUP=0
From: Daniel Wagner @ 2012-07-11  9:35 UTC (permalink / raw)
  To: cgroups
  Cc: netdev, Daniel Wagner, Neil Horman, Glauber Costa,
	Kamezawa Hiroyuki, Tejun Heo, Li Zefan, Eric Dumazet,
	David S. Miller
In-Reply-To: <1341999341-1808-1-git-send-email-wagi@monom.org>

From: Daniel Wagner <daniel.wagner@bmw-carit.de>

With the configuration CONFIG_CGROUPS=y and CONFIG_NETPRIO_CGROUP=0
sock_update_netprioidx() was still added and introduced unnecessary load.

Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Cc: Neil Horman <nhorman@tuxdriver.com>
Cc: Glauber Costa <glommer@parallels.com>
Cc: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Li Zefan <lizefan@huawei.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: "David S. Miller" <davem@davemloft.net>
---
 include/net/netprio_cgroup.h | 31 ++++++++++++++-----------------
 net/core/sock.c              |  4 +++-
 2 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/include/net/netprio_cgroup.h b/include/net/netprio_cgroup.h
index d58fdec..d14dfbe 100644
--- a/include/net/netprio_cgroup.h
+++ b/include/net/netprio_cgroup.h
@@ -17,6 +17,7 @@
 #include <linux/hardirq.h>
 #include <linux/rcupdate.h>
 
+#if IS_ENABLED(CONFIG_NETPRIO_CGROUP)
 
 struct netprio_map {
 	struct rcu_head rcu;
@@ -24,19 +25,24 @@ struct netprio_map {
 	u32 priomap[];
 };
 
-#ifdef CONFIG_CGROUPS
-
 struct cgroup_netprio_state {
 	struct cgroup_subsys_state css;
 	u32 prioidx;
 };
 
-#ifndef CONFIG_NETPRIO_CGROUP
-extern int net_prio_subsys_id;
-#endif
-
 extern void sock_update_netprioidx(struct sock *sk);
 
+#else
+
+#define sock_update_netprioidx(sk)
+
+static inline u32 task_netprioidx(struct task_struct *p)
+{
+	return 0;
+}
+
+#endif
+
 #if IS_BUILTIN(CONFIG_NETPRIO_CGROUP)
 
 static inline u32 task_netprioidx(struct task_struct *p)
@@ -54,6 +60,8 @@ static inline u32 task_netprioidx(struct task_struct *p)
 
 #elif IS_MODULE(CONFIG_NETPRIO_CGROUP)
 
+extern int net_prio_subsys_id;
+
 static inline u32 task_netprioidx(struct task_struct *p)
 {
 	struct cgroup_netprio_state *state;
@@ -72,17 +80,6 @@ static inline u32 task_netprioidx(struct task_struct *p)
 	return idx;
 }
 
-#else
-
-static inline u32 task_netprioidx(struct task_struct *p)
-{
-	return 0;
-}
-
 #endif /* CONFIG_NETPRIO_CGROUP */
 
-#else
-#define sock_update_netprioidx(sk)
-#endif
-
 #endif  /* _NET_CLS_CGROUP_H */
diff --git a/net/core/sock.c b/net/core/sock.c
index 52c4a2a..94e0100 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -276,7 +276,7 @@ EXPORT_SYMBOL(sysctl_optmem_max);
 int net_cls_subsys_id = -1;
 EXPORT_SYMBOL_GPL(net_cls_subsys_id);
 #endif
-#if !defined(CONFIG_NETPRIO_CGROUP)
+#if IS_MODULE(CONFIG_NETPRIO_CGROUP)
 int net_prio_subsys_id = -1;
 EXPORT_SYMBOL_GPL(net_prio_subsys_id);
 #endif
@@ -1182,6 +1182,7 @@ void sock_update_classid(struct sock *sk)
 EXPORT_SYMBOL(sock_update_classid);
 #endif
 
+#if IS_ENABLED(CONFIG_NETPRIO_CGROUP)
 void sock_update_netprioidx(struct sock *sk)
 {
 	u32 prioidx;
@@ -1195,6 +1196,7 @@ void sock_update_netprioidx(struct sock *sk)
 }
 EXPORT_SYMBOL_GPL(sock_update_netprioidx);
 #endif
+#endif
 
 /**
  *	sk_alloc - All socket objects are allocated here
-- 
1.7.11.1.165.g299666c

^ permalink raw reply related

* [PATCH v2 1/5] cgroup: Only update sk_cgrp_prioidx on change
From: Daniel Wagner @ 2012-07-11  9:35 UTC (permalink / raw)
  To: cgroups
  Cc: netdev, Daniel Wagner, Kamezawa Hiroyuki, Glauber Costa,
	Tejun Heo, Li Zefan, Eric Dumazet, David S. Miller
In-Reply-To: <1341999341-1808-1-git-send-email-wagi@monom.org>

From: Daniel Wagner <daniel.wagner@bmw-carit.de>

Do not make the cache line dirty when nothing has changed.
sock_update_classid() does exactly the same thing.

Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
Cc: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Glauber Costa <glommer@parallels.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Li Zefan <lizefan@huawei.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: "David S. Miller" <davem@davemloft.net>
---
 net/core/sock.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/core/sock.c b/net/core/sock.c
index 929bdcc..bbab10d 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1182,10 +1182,14 @@ EXPORT_SYMBOL(sock_update_classid);
 
 void sock_update_netprioidx(struct sock *sk)
 {
+	u32 prioidx;
+
 	if (in_interrupt())
 		return;
 
-	sk->sk_cgrp_prioidx = task_netprioidx(current);
+	prioidx = task_netprioidx(current);
+	if (prioidx != sk->sk_cgrp_prioidx)
+		sk->sk_cgrp_prioidx = prioidx;
 }
 EXPORT_SYMBOL_GPL(sock_update_netprioidx);
 #endif
-- 
1.7.11.1.165.g299666c

^ permalink raw reply related

* [PATCH v2 0/5] cgroup cls & netprio 'cleanups'
From: Daniel Wagner @ 2012-07-11  9:35 UTC (permalink / raw)
  To: cgroups; +Cc: netdev, Daniel Wagner

From: Daniel Wagner <daniel.wagner@bmw-carit.de>

Hi,

While looking a bit at the cls and netprio controller I found a
few things which I think are 'not optimal'.

The first patch makes sock_update_netprioidx() a bit smatter, so that
only on changed value the sk->sk_cgroup_prioidx is written and not
all the time.

The next two patches are changing the built. In case someone 
builds a kernel with cgroup support but disabled cls or netprio
controller, there was still code added.

And the last two patches change the cls and netprio source files in
that way, that the use IS_BUILTIN and IS_MODULE as it used in the
header files.

The patches are against net-next.

cheers,
daniel

Daniel Wagner (5):
  cgroup: Only update sk_cgrp_prioidx on change
  cgroup: Do not add sock_update_classid() when CONFIG_NET_CLS_CGROUP=0
  cgroup: Do not add sock_update_netprioidx() when
    CONFIG_NETPRIO_CGROUP=0
  cgroup: Make builtin vs. module build ifdefs consistent
  cgroup: Make builtin vs. module build ifdefs consistent

 include/net/cls_cgroup.h     | 33 +++++++++++++++++++++++----------
 include/net/netprio_cgroup.h | 31 ++++++++++++++-----------------
 include/net/sock.h           |  8 --------
 net/core/netprio_cgroup.c    |  6 +++---
 net/core/sock.c              | 14 +++++++++++---
 net/sched/cls_cgroup.c       |  6 +++---
 6 files changed, 54 insertions(+), 44 deletions(-)

-- 
1.7.11.1.165.g299666c

^ 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