Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] net/macb: add DT support
From: Nicolas Ferre @ 2011-12-05 11:51 UTC (permalink / raw)
  To: Jamie Iles
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20111205114838.GC21006@totoro>

On 12/05/2011 12:48 PM, Jamie Iles :
> On Fri, Dec 02, 2011 at 06:53:58PM +0100, Nicolas Ferre wrote:
>> On 12/02/2011 06:28 PM, Jamie Iles :
> [...]
>>> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
>>> index 103c6e6..89060e6 100644
>>> --- a/drivers/net/ethernet/cadence/macb.c
>>> +++ b/drivers/net/ethernet/cadence/macb.c
>>> @@ -1247,6 +1247,8 @@ static const struct of_device_id macb_dt_ids[] = {
>>>   	{ .compatible = "cdns,at32ap7000-macb" },
>>>   	{ .compatible = "cdns,at91sam9260-macb" },
>>>   	{ .compatible = "cdns,macb" },
>>> +	{ .compatible = "cdns,pc302-gem" },
>>> +	{ .compatible = "cdns,gem" },
>>>   	{ /* sentinel */ }
>>>   };
>>
>> BTW, I think we may also modify the MII/RMII selection code for
>> adding gigabit selection... but maybe you already have the patches?
>
> No, I don't have access to a version of the GEM with gigabit support.
> The platforms I was working on used it as a 10/100 controller, but had
> to use the GEM to get hardware timestamping support.

Ok, Thanks for your feedback. I send a v3 patch now.

Best regards,
-- 
Nicolas Ferre

^ permalink raw reply

* Re: [PATCH] net/macb: add DT support
From: Jamie Iles @ 2011-12-05 11:48 UTC (permalink / raw)
  To: Nicolas Ferre
  Cc: Jamie Iles, robherring2, devicetree-discuss, netdev, plagnioj,
	linux-arm-kernel, grant.likely, linux-kernel
In-Reply-To: <4ED910B6.7080000@atmel.com>

On Fri, Dec 02, 2011 at 06:53:58PM +0100, Nicolas Ferre wrote:
> On 12/02/2011 06:28 PM, Jamie Iles :
[...]
> >diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> >index 103c6e6..89060e6 100644
> >--- a/drivers/net/ethernet/cadence/macb.c
> >+++ b/drivers/net/ethernet/cadence/macb.c
> >@@ -1247,6 +1247,8 @@ static const struct of_device_id macb_dt_ids[] = {
> >  	{ .compatible = "cdns,at32ap7000-macb" },
> >  	{ .compatible = "cdns,at91sam9260-macb" },
> >  	{ .compatible = "cdns,macb" },
> >+	{ .compatible = "cdns,pc302-gem" },
> >+	{ .compatible = "cdns,gem" },
> >  	{ /* sentinel */ }
> >  };
> 
> BTW, I think we may also modify the MII/RMII selection code for
> adding gigabit selection... but maybe you already have the patches?

No, I don't have access to a version of the GEM with gigabit support.  
The platforms I was working on used it as a 10/100 controller, but had 
to use the GEM to get hardware timestamping support.

Jamie

^ permalink raw reply

* Re: [PATCH] sch_red: fix red_change
From: Ilpo Järvinen @ 2011-12-05 11:42 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Dave Taht, Stephen Hemminger, Thomas Graf, netdev, Jim Gettys
In-Reply-To: <1322776643.2750.45.camel@edumazet-laptop>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 4237 bytes --]

On Thu, 1 Dec 2011, Eric Dumazet wrote:
> Le jeudi 01 décembre 2011 à 22:35 +0100, Dave Taht a écrit :
> > On Thu, Dec 1, 2011 at 10:06 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > > Le mercredi 30 novembre 2011 à 14:36 -0800, Stephen Hemminger a écrit :
> > >
> > >> (Almost) nobody uses RED because they can't figure it out.
> > >> According to Wikipedia, VJ says that:
> > >>  "there are not one, but two bugs in classic RED."
> > 
> > Heh. "There were not two, but four bugs in Linux red".
> > 
> > Now reduced to 2. :)
> 
> This story about VJ and bugs in classic RED is urban legend if you ask
> me :)

I think at least one bug he claims to find in "the" manuscript is not a 
bug at all :-). Essentially he assumes that once instantaneous queue is 
zero, the load is zero, which is not true (e.g., during any typical slow 
start while the link is still not saturated).

> > RED is useful for high throughput routers, I doubt many linux machines
> > > act as such devices.
> > 
> > "High throughput" at the time red was designed was not much faster
> > than a T1 line.
> > 
> > RED appears to be used by default in both gargoyle's and openwrt's QoS systems,
> > underneath unholy combinations of HTB, HSFC, and SFQ
> > so it's more widely used than you might think. Not that works well.
> > 
> > RED doesn't work worth beans on variable bandwidth links (cable
> > modems/wireless).
> 
> Adaptative RED is the answer

I disagree. Slowly responding adaptation is not going to do much good if 
put together with variable something.

And besides, Adaptive RED is as hard if not even harder to configure than 
the standard RED. That is, it has one key parameter and absolutely no info 
is given how that should be configured?!? :-)

> > Once you are simulating a fixed rate link (e.g with HTB), then it sort of
> > kinda maybe can apply.
> > 
> > RED was also designed at a time when long distance traffic was fixed rate
> > and bidirectional, so the 'average packet' parameter made sense.
> > Modern day traffic is far more asymmetric.
> > 
> 
> The truth is : For RED be effective (with say 20 to 100 flows), you need
> a reasonable amount of packets in queue, and low wq (high burst value in
> linux), depending on the RTT. And on consumer links (ADSL, cable
> modem ...), RTT is quite big.

To be more exact, it's BDP (BW-delay product) which matters, not the RTT 
alone.

> RED performance is best when the average queue size is estimated over a
> small _multiple_ of round-trip times, not over a fraction of a single
> round-trip time.

I disagree. If there's any slow starting flow that alone can fill the 
bottleneck, anything significantly larger than RTT just harms. RED is 
just "too slow" if you follow the recommended parametrization..

In a core router you can probably get away with multiple RTTs, but near 
edge that is a grave mistake due to how slow-start behaves. With average 
based on many RTTs, RED still estimates that the link has low load while 
congestion has escalated to higher dimensions due to slow start. As a 
result, RED graciously falls back to tail-drop once the physical queue 
runs out and the flows respond allowing the load to decrease. However, 
finally RED reaches a state where it starts to "pro-actively" react to an 
"incipient congestion"?!? :-/ => Problem is made worse by those extra 
drops/marks happening too late.

...And the obvious looking solution by making physical buffer size 
larger brings in even worse problems. There's simply no other way around 
this than making wq larger instead of smaller in order to arrest the slow 
start in time. (We have a paper to appear about these in AINA 2012.)

I know this is pretty much against the mantra repeated about RED. And I'm 
not too surprised why so many have found out that RED does not help.

> In this respect, your RED setups are pathological (minimum burst value,
> meaning wq = 0.5 or so), so in a small fraction of RTT, avgqsz value is
> completely changed, so flows have no chance to be able to react
> smoothly.

Here I agree, 0.5 is probably too much though... only if BDP is very small 
this is useful but then RED is probably having other problems due to 
granularities affecting its measurement accuracy.


-- 
 i.

^ permalink raw reply

* Re: [PATCH] ARM: at91/net: add macb ethernet controller in 9g45 DT
From: Nicolas Ferre @ 2011-12-05 11:39 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20111203055659.GL18533-RQcB7r2h9QmfDR2tN2SG5Ni2O/JbrIOy@public.gmane.org>

On 12/03/2011 06:56 AM, Jean-Christophe PLAGNIOL-VILLARD :
> On 18:50 Fri 02 Dec     , Nicolas Ferre wrote:
>> Add the Cadence macb ethernet controller in at91sam9g45 .dtsi and
>> enable it in at91sam9m10g45ek board device tree file.
>>
>> Signed-off-by: Nicolas Ferre<nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
>> ---
>>   arch/arm/boot/dts/at91sam9g45.dtsi     |    7 +++++++
>>   arch/arm/boot/dts/at91sam9m10g45ek.dts |    6 ++++++
>>   2 files changed, 13 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/at91sam9g45.dtsi b/arch/arm/boot/dts/at91sam9g45.dtsi
>> index e89b1d7..67f94d3 100644
>> --- a/arch/arm/boot/dts/at91sam9g45.dtsi
>> +++ b/arch/arm/boot/dts/at91sam9g45.dtsi
>> @@ -101,6 +101,13 @@
>>   				atmel,use-dma-tx;
>>   				status = "disabled";
>>   			};
>> +
>> +			macb0: ethernet@fffbc000 {
>> +				compatible = "cdns,at32ap7000-macb", "cdns,macb";
>> +				reg =<0xfffbc000 0x100>;
>> +				interrupts =<25 4>;
> why?

It is the new AIC specification that was reworked in this patch:
https://lkml.org/lkml/2011/12/1/238

>> +				status = "disabled";
>> +			};
>>   		};
>>   	};
>>   };
>> diff --git a/arch/arm/boot/dts/at91sam9m10g45ek.dts b/arch/arm/boot/dts/at91sam9m10g45ek.dts
>> index 85b34f5..17377a2 100644
>> --- a/arch/arm/boot/dts/at91sam9m10g45ek.dts
>> +++ b/arch/arm/boot/dts/at91sam9m10g45ek.dts
>> @@ -30,6 +30,12 @@
>>   			usart1: serial@fff90000 {
>>   				status = "okay";
>>   			};
>> +
>> +			macb0: ethernet@fffbc000 {
>> +				local-mac-address = [3a 0e 03 04 05 06];
> please drop this is not supposed to be in the dts but updated for each board

Yes, sure: I kept in because it was allowing me to test. But for sure, 
this should go away...

Bye,
-- 
Nicolas Ferre

^ permalink raw reply

* Re: [PATCH v2] net/macb: add DT support
From: Nicolas Ferre @ 2011-12-05 11:36 UTC (permalink / raw)
  To: David Miller, jamie-wmLquQDDieKakBO8gow8eQ
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20111202.125832.1208514279272697863.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

On 12/02/2011 06:58 PM, David Miller :
> From: Nicolas Ferre<nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> Date: Fri,  2 Dec 2011 18:43:02 +0100
>
>> From: Jean-Christophe PLAGNIOL-VILLARD<plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
>>
>> Allow the device tree to provide the mac address and the phy mode.
>>
>> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD<plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
>> [nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org: change "compatible" node property, doc and DT hwaddr]
>> Signed-off-by: Nicolas Ferre<nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
>> Cc: Jamie Iles<jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org>
>> ---
>> v2: modify macb_get_hwaddr_dt() parameter
>
> You'll have to respin these two macb patches, as they don't apply properly
> to the net-next tree which is where they should be targetted.

David,

In fact, the patches are designed to be added on top of Jamie's rework 
of the macb driver that is now in Arnd's "arm-soc" git tree (for-next 
branch). You discussed with him here:

http://www.spinics.net/lists/arm-kernel/msg118717.html

Should I go this path also or wait for this material to be merged and 
update on net-next at this time (after 3.3 merge window...)?

Best regards,
-- 
Nicolas Ferre

^ permalink raw reply

* [PATCH net-next] tcp: remove TCP_OFF and TCP_PAGE macros
From: Eric Dumazet @ 2011-12-05 11:07 UTC (permalink / raw)
  To: Joe Perches; +Cc: David Miller, netdev
In-Reply-To: <1323019253.1785.7.camel@joe2Laptop>

Le dimanche 04 décembre 2011 à 09:20 -0800, Joe Perches a écrit :
> And maybe the TCP_OFF and TCP_PAGE macros should be removed.
> 

Agreed, thanks.

[PATCH net-next] tcp: remove TCP_OFF and TCP_PAGE macros

As mentioned by Joe Perches, TCP_OFF() and TCP_PAGE() macros are
useless.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/ipv4/tcp.c |   23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index a09fe25..43dfccc 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -888,9 +888,6 @@ int tcp_sendpage(struct sock *sk, struct page *page, int offset,
 }
 EXPORT_SYMBOL(tcp_sendpage);
 
-#define TCP_PAGE(sk)	(sk->sk_sndmsg_page)
-#define TCP_OFF(sk)	(sk->sk_sndmsg_off)
-
 static inline int select_size(const struct sock *sk, bool sg)
 {
 	const struct tcp_sock *tp = tcp_sk(sk);
@@ -1008,13 +1005,13 @@ new_segment:
 			} else {
 				int merge = 0;
 				int i = skb_shinfo(skb)->nr_frags;
-				struct page *page = TCP_PAGE(sk);
+				struct page *page = sk->sk_sndmsg_page;
 				int off;
 
 				if (page && page_count(page) == 1)
-					TCP_OFF(sk) = 0;
+					sk->sk_sndmsg_off = 0;
 
-				off = TCP_OFF(sk);
+				off = sk->sk_sndmsg_off;
 
 				if (skb_can_coalesce(skb, i, page, off) &&
 				    off != PAGE_SIZE) {
@@ -1031,7 +1028,7 @@ new_segment:
 				} else if (page) {
 					if (off == PAGE_SIZE) {
 						put_page(page);
-						TCP_PAGE(sk) = page = NULL;
+						sk->sk_sndmsg_page = page = NULL;
 						off = 0;
 					}
 				} else
@@ -1057,9 +1054,9 @@ new_segment:
 					/* If this page was new, give it to the
 					 * socket so it does not get leaked.
 					 */
-					if (!TCP_PAGE(sk)) {
-						TCP_PAGE(sk) = page;
-						TCP_OFF(sk) = 0;
+					if (!sk->sk_sndmsg_page) {
+						sk->sk_sndmsg_page = page;
+						sk->sk_sndmsg_off = 0;
 					}
 					goto do_error;
 				}
@@ -1069,15 +1066,15 @@ new_segment:
 					skb_frag_size_add(&skb_shinfo(skb)->frags[i - 1], copy);
 				} else {
 					skb_fill_page_desc(skb, i, page, off, copy);
-					if (TCP_PAGE(sk)) {
+					if (sk->sk_sndmsg_page) {
 						get_page(page);
 					} else if (off + copy < PAGE_SIZE) {
 						get_page(page);
-						TCP_PAGE(sk) = page;
+						sk->sk_sndmsg_page = page;
 					}
 				}
 
-				TCP_OFF(sk) = off + copy;
+				sk->sk_sndmsg_off = off + copy;
 			}
 
 			if (!copied)

^ permalink raw reply related

* Re: Time in Queue, bufferbloat, and... our accidentally interplanetary network
From: Eric Dumazet @ 2011-12-05 10:59 UTC (permalink / raw)
  To: Dave Taht; +Cc: bloat, bloat-devel, netdev, linux-wireless
In-Reply-To: <CAA93jw7kQYONQiAozatRmdRRJeK0GB9eNDWtRZ3AL+OVKn0OpA@mail.gmail.com>

Le lundi 05 décembre 2011 à 10:05 +0100, Dave Taht a écrit :

> And Eric dumazet also produced a preliminary patch a few weeks back
> that tied timestamping to before the head of a queue, but that tried to use a
> reserved field in the skb that appears from points A to Z is not guaranteed
> to be preserved.

It is guaranteed to be preserved, as it is part of skb->cb[], and
skb->cb[] content is private to each layer.

Here, Qdisc layer.

Adding a time limit is possible, all we need is a proper design and
implementation :)

Here is my suggestion :

Design a new tfifo/tred qdisc, with following properties :

Adaptative RED, (ECN enabled + head drop), but instead of using
bytes/packet qlen average, use time_in_queue average.

A single mandatory parameter to specify the tmin value
tmax default value would be tmin*3  (so that target avg is 2*tmin)
tlimit default value would be tmax*8

Adaptative RED dynamically adjusts maxp between 1% and 50%
[Using a timer, every 500ms, and AIMD]

tc qdisc add dev eth0 root tfifo tmin 1ms [tmax val] [tlimit val]


I volunteer to work on this new tfifo experimental qdisc :)

This can be used in replacement of pfifo/bfifo in a complex qdisc setup.
As it has RED included, it also can replace RED.

Please note that once skb leaves Qdisc and is delivered to device, any
time limit feature must be handled in the driver itself (if it can queue
packet for a long time period)

^ permalink raw reply

* Re: [net-next RFC PATCH 5/5] virtio-net: flow director support
From: Stefan Hajnoczi @ 2011-12-05 10:55 UTC (permalink / raw)
  To: Jason Wang
  Cc: krkumar2, kvm, mst, netdev, virtualization, levinsasha928,
	bhutchings
In-Reply-To: <20111205085925.6116.94352.stgit@dhcp-8-146.nay.redhat.com>

On Mon, Dec 5, 2011 at 8:59 AM, Jason Wang <jasowang@redhat.com> wrote:
> +static int virtnet_set_fd(struct net_device *dev, u32 pfn)
> +{
> +       struct virtnet_info *vi = netdev_priv(dev);
> +       struct virtio_device *vdev = vi->vdev;
> +
> +       if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_FD)) {
> +               vdev->config->set(vdev,
> +                                 offsetof(struct virtio_net_config_fd, addr),
> +                                 &pfn, sizeof(u32));

Please use the virtio model (i.e. virtqueues) instead of shared
memory.  Mapping a page breaks the virtio abstraction.

Stefan

^ permalink raw reply

* RE: WARNING: at mm/slub.c:3357, kernel BUG at mm/slub.c:3413
From: David Laight @ 2011-12-05 10:44 UTC (permalink / raw)
  To: Markus Trippelsdorf, Dave Airlie
  Cc: Jerome Glisse, Christoph Lameter, Alex, Shi, Eric Dumazet, netdev,
	linux-kernel, dri-devel, Pekka Enberg, linux-mm, Matt Mackall, tj,
	Alex Deucher
In-Reply-To: <20111203122900.GA1617@x4.trippels.de>

 
> > If I had to guess it looks like 0 is getting written back to some
> > random page by the GPU maybe, it could be that the GPU is in some
half
> > setup state at boot or on a reboot does it happen from a cold boot
or
> > just warm boot or kexec?
> 
> Only happened with kexec thus far. Cold boot seems to be fine.

Sounds like the GPU is writing to physical memory from the
old mappings.
This can happen to other devices if they aren't completely
disabled - which may not happen since the kexec case probably
avoids some of the hardware resets that occurr diring a normal
reboot.

I remember an ethernet chip writing into its rx ring/buffer
area following a reboot (and reinstall!) when connected
to a quiet lan.

	David


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

^ permalink raw reply

* Re: [net-next RFC PATCH 2/5] tuntap: simple flow director support
From: Stefan Hajnoczi @ 2011-12-05 10:38 UTC (permalink / raw)
  To: Jason Wang
  Cc: krkumar2, kvm, mst, netdev, virtualization, levinsasha928,
	bhutchings
In-Reply-To: <20111205085857.6116.99252.stgit@dhcp-8-146.nay.redhat.com>

On Mon, Dec 5, 2011 at 8:58 AM, Jason Wang <jasowang@redhat.com> wrote:
> This patch adds a simple flow director to tun/tap device. It is just a
> page that contains the hash to queue mapping which could be changed by
> user-space. The backend (tap/macvtap) would query this table to get
> the desired queue of a packets when it send packets to userspace.
>
> The page address were set through a new kind of ioctl - TUNSETFD and
> were pinned until device exit or another new page were specified.

Please use "flow" or "fdir" instead of "fd" in the ioctl and code.
"fd" reminds of file descriptor.  The ixgbe driver uses "fdir".

Stefan

^ permalink raw reply

* Re: [PATCH v7 00/10] Request for Inclusion: per-cgroup tcp memory pressure
From: Glauber Costa @ 2011-12-05 10:28 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: linux-kernel, paul, lizf, ebiederm, davem, gthelen, netdev,
	linux-mm, kirill, avagin, devel, eric.dumazet, cgroups
In-Reply-To: <20111205185108.099f393e.kamezawa.hiroyu@jp.fujitsu.com>

On 12/05/2011 07:51 AM, KAMEZAWA Hiroyuki wrote:
> On Mon, 5 Dec 2011 07:09:51 -0200
> Glauber Costa<glommer@parallels.com>  wrote:
>
>> On 12/05/2011 12:06 AM, KAMEZAWA Hiroyuki wrote:
>>> On Fri, 2 Dec 2011 16:04:08 -0200
>>> Glauber Costa<glommer@parallels.com>   wrote:
>>>
>>>> On 11/30/2011 12:11 AM, KAMEZAWA Hiroyuki wrote:
>>>>> On Tue, 29 Nov 2011 21:56:51 -0200
>>>>> Glauber Costa<glommer@parallels.com>    wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> This patchset implements per-cgroup tcp memory pressure controls. It did not change
>>>>>> significantly since last submission: rather, it just merges the comments Kame had.
>>>>>> Most of them are style-related and/or Documentation, but there are two real bugs he
>>>>>> managed to spot (thanks)
>>>>>>
>>>>>> Please let me know if there is anything else I should address.
>>>>>>
>>>>>
>>>>> After reading all codes again, I feel some strange. Could you clarify ?
>>>>>
>>>>> Here.
>>>>> ==
>>>>> +void sock_update_memcg(struct sock *sk)
>>>>> +{
>>>>> +	/* right now a socket spends its whole life in the same cgroup */
>>>>> +	if (sk->sk_cgrp) {
>>>>> +		WARN_ON(1);
>>>>> +		return;
>>>>> +	}
>>>>> +	if (static_branch(&memcg_socket_limit_enabled)) {
>>>>> +		struct mem_cgroup *memcg;
>>>>> +
>>>>> +		BUG_ON(!sk->sk_prot->proto_cgroup);
>>>>> +
>>>>> +		rcu_read_lock();
>>>>> +		memcg = mem_cgroup_from_task(current);
>>>>> +		if (!mem_cgroup_is_root(memcg))
>>>>> +			sk->sk_cgrp = sk->sk_prot->proto_cgroup(memcg);
>>>>> +		rcu_read_unlock();
>>>>> ==
>>>>>
>>>>> sk->sk_cgrp is set to a memcg without any reference count.
>>>>>
>>>>> Then, no check for preventing rmdir() and freeing memcgroup.
>>>>>
>>>>> Is there some css_get() or mem_cgroup_get() somewhere ?
>>>>>
>>>>
>>>> There were a css_get in the first version of this patchset. It was
>>>> removed, however, because it was deemed anti-intuitive to prevent rmdir,
>>>> since we can't know which sockets are blocking it, or do anything about
>>>> it. Or did I misunderstand something ?
>>>>
>>>
>>> Maybe I misuderstood. Thank you. Ok, there is no css_get/put and
>>> rmdir() is allowed. But, hmm....what's guarding threads from stale
>>> pointer access ?
>>>
>>> Does a memory cgroup which is pointed by sk->sk_cgrp always exist ?
>>>
>> If I am not mistaken, yes, it will. (Ok, right now it won't)
>>
>> Reason is a cgroup can't be removed if it is empty.
>> To make it empty, you need to move the tasks away.
>>
>> So the sockets will be moved away as well when you do it. So right now
>> they are not, so it would then probably be better to increase a
>> reference count with a comment saying that it is temporary.
>>
>
> I'm sorry if I misunderstand.
>
> At task exit, __fput() will be called against file descriptors, yes.
> __fput() calles f_op->release() =>  inet_release() =>  tcp_close().
>
> But TCP socket may be alive after task exit until it gets down to
> protocol close. For example, until the all message in send buffer
> is acked, socket and tcp connection will not be disappear.
>
> In short, socket's lifetime is different from it's task's.
> So, there may be sockets which are not belongs to any task.
>

Yeah, you're right. I guess this is one more reason for us to just keep 
the memcg reference around.

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

^ permalink raw reply

* Re: [PATCH] net, fixed phy: make BUSID configurable
From: Heiko Schocher @ 2011-12-05 10:21 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, vbordug, akpm, jeff, wd
In-Reply-To: <20111204.132321.9566486322084069.davem@davemloft.net>

Hello David,

David Miller wrote:
> From: Heiko Schocher <hs@denx.de>
> Date: Sun,  4 Dec 2011 10:37:24 +0100
> 
>> Signed-off-by: Heiko Schocher <hs@denx.de>
> 
> Give me a break.
> 
> You can't submit a change like this with essentially an
> empty commit message.

Uh, yes, sorry!

> Why in the world does this ever need to be configurable?
> Are there alternative (and much better) ways to deal with
> whatever problem this patch solves?
> 
> You've presented no information about why you need to make this
> change, and more importantly why no viable alternatives exist
> to solve your problem.

On a am1808 based board porting to current kernel, I have connected
a KSZ8864RMN switch and using the drivers/net/ethernet/ti/davinci_emac.c
driver. To the switch I have always 100/FULL connection and so
I decided to use the fixed PHY driver. But the busid 0 is used
from the drivers/net/ethernet/ti/davinci_emac.c driver, and the
fixed PHY driver is fix at busid 0 ...

Looking in the drivers/net/ethernet/ti/davinci_emac.c in emac_dev_open()
there is the possibility if no priv->phy_id is defined through
plattform code, to use a fixed 100/FULL speed and duplex settings ...
which would fix my needs ...

But code did not reach this, because first a:

        if (!priv->phy_id) {
                struct device *phy;

                phy = bus_find_device(&mdio_bus_type, NULL, NULL,
                                      match_first_device);
                if (phy)
                        priv->phy_id = dev_name(phy);
        }

is checked ... maybe it is better to fix this here by removing this
"if (!priv->phy_id) {" and do not use the fixed PHY driver?

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

^ permalink raw reply

* Re: [PATCH 2/2] unix: Add /proc/net/unix_peers file
From: Pavel Emelyanov @ 2011-12-05 10:25 UTC (permalink / raw)
  To: David Miller; +Cc: netdev@vger.kernel.org, eric.dumazet@gmail.com
In-Reply-To: <20111204.132510.1708510157080644595.davem@davemloft.net>

On 12/04/2011 10:25 PM, David Miller wrote:
> From: Pavel Emelyanov <xemul@parallels.com>
> Date: Thu, 01 Dec 2011 21:42:06 +0400
> 
>> Currently it's not possible to find out what processes are connected
>> to each other via a unix socket. In the proposed proc file a socket
>> inode number and its peer inode number are shown. With these two at
>> hands it's possible to determine the unix connections endpoints.
>>
>> Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
> 
> I'm basically against new networking procfs based information
> retrieval mechanisms.

So am I, just wanted to bring this topic up again.

> Please extend the netlink socket dumping so that it works with
> AF_UNIX sockets and subsequently added the necessary netlink
> attribute to provide the peer value.

OK, will do it this way. AFAIU you're talking about the NETLINK_INET_DIAG, right?

> I plan to stand pretty firm on this, so you may want to save your
> effort and use said effort to implement this properly.
> 
> Thanks.
> .
> 

Thanks,
Pavel

^ permalink raw reply

* Re: [Linux-decnet-user] Proposed removal of DECnet support (was:Re: [BUG] 3.2-rc2:BUG kmalloc-8: Redzone overwritten)
From: Philipp Schafft @ 2011-12-05 10:14 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Steven Whitehouse, mike.gair, Chrissie Caulfield,
	Christoph Lameter, David Miller, Eric Dumazet, Sasha Levin,
	Linux-DECnet user, linux-kernel, linux-mm, Matt Mackall, netdev,
	Pekka Enberg, RoarAudio
In-Reply-To: <1323048232.7454.161.camel@deadeye>

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

reflum,

On Mon, 2011-12-05 at 01:23 +0000, Ben Hutchings wrote:
> On Sun, 2011-12-04 at 20:50 +0100, Philipp Schafft wrote:
> > On Wed, 2011-11-30 at 14:52 +0000, Steven Whitehouse wrote:
> [...]
> > > It is good to know that people are still using the Linux DECnet code
> > > too. It has lived far beyond the time when I'd envisioned it still being
> > > useful :-)
> > 
> > There are still some people interested in it. Btw. on Debian popcon
> > counts 5356 users.
> 
> This is grossly misleading.  Here's the historical graph showing <100
> installations of libdnet until early 2011:
> http://qa.debian.org/popcon-graph.php?packages=libdnet

Maybe my statement was missleading. popcon shows 5356 installs. This
includes real users and non-real users. Both groups *may* be affected by
droping the kernel module (in diffrent ways).


> For some reason (a joke?) roaraudio has DECnet
> support and its packages depend on libdnet.

Maybe just because it is usefull for the RoarAudio project.

Anyway, don't take the number too important. It was just a minor note.

-- 
Philipp.
 (Rah of PH2)

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 482 bytes --]

^ permalink raw reply

* Re: [PATCH v7 00/10] Request for Inclusion: per-cgroup tcp memory pressure
From: KAMEZAWA Hiroyuki @ 2011-12-05  9:51 UTC (permalink / raw)
  To: Glauber Costa
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, paul-inf54ven1CmVyaH7bEyXVA,
	lizf-BthXqXjhjHXQFUHtdCDX3A, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, gthelen-hpIqsD4AKlfQT0dZR+AlfA,
	netdev-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	kirill-oKw7cIdHH8eLwutG50LtGA, avagin-bzQdu9zFT3WakBO8gow8eQ,
	devel-GEFAQzZX7r8dnm+yROfE0A, eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
	cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4EDC8A5F.8040402-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>

On Mon, 5 Dec 2011 07:09:51 -0200
Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> wrote:

> On 12/05/2011 12:06 AM, KAMEZAWA Hiroyuki wrote:
> > On Fri, 2 Dec 2011 16:04:08 -0200
> > Glauber Costa<glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>  wrote:
> >
> >> On 11/30/2011 12:11 AM, KAMEZAWA Hiroyuki wrote:
> >>> On Tue, 29 Nov 2011 21:56:51 -0200
> >>> Glauber Costa<glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>   wrote:
> >>>
> >>>> Hi,
> >>>>
> >>>> This patchset implements per-cgroup tcp memory pressure controls. It did not change
> >>>> significantly since last submission: rather, it just merges the comments Kame had.
> >>>> Most of them are style-related and/or Documentation, but there are two real bugs he
> >>>> managed to spot (thanks)
> >>>>
> >>>> Please let me know if there is anything else I should address.
> >>>>
> >>>
> >>> After reading all codes again, I feel some strange. Could you clarify ?
> >>>
> >>> Here.
> >>> ==
> >>> +void sock_update_memcg(struct sock *sk)
> >>> +{
> >>> +	/* right now a socket spends its whole life in the same cgroup */
> >>> +	if (sk->sk_cgrp) {
> >>> +		WARN_ON(1);
> >>> +		return;
> >>> +	}
> >>> +	if (static_branch(&memcg_socket_limit_enabled)) {
> >>> +		struct mem_cgroup *memcg;
> >>> +
> >>> +		BUG_ON(!sk->sk_prot->proto_cgroup);
> >>> +
> >>> +		rcu_read_lock();
> >>> +		memcg = mem_cgroup_from_task(current);
> >>> +		if (!mem_cgroup_is_root(memcg))
> >>> +			sk->sk_cgrp = sk->sk_prot->proto_cgroup(memcg);
> >>> +		rcu_read_unlock();
> >>> ==
> >>>
> >>> sk->sk_cgrp is set to a memcg without any reference count.
> >>>
> >>> Then, no check for preventing rmdir() and freeing memcgroup.
> >>>
> >>> Is there some css_get() or mem_cgroup_get() somewhere ?
> >>>
> >>
> >> There were a css_get in the first version of this patchset. It was
> >> removed, however, because it was deemed anti-intuitive to prevent rmdir,
> >> since we can't know which sockets are blocking it, or do anything about
> >> it. Or did I misunderstand something ?
> >>
> >
> > Maybe I misuderstood. Thank you. Ok, there is no css_get/put and
> > rmdir() is allowed. But, hmm....what's guarding threads from stale
> > pointer access ?
> >
> > Does a memory cgroup which is pointed by sk->sk_cgrp always exist ?
> >
> If I am not mistaken, yes, it will. (Ok, right now it won't)
> 
> Reason is a cgroup can't be removed if it is empty.
> To make it empty, you need to move the tasks away.
> 
> So the sockets will be moved away as well when you do it. So right now 
> they are not, so it would then probably be better to increase a 
> reference count with a comment saying that it is temporary.
> 

I'm sorry if I misunderstand.

At task exit, __fput() will be called against file descriptors, yes.
__fput() calles f_op->release() => inet_release() => tcp_close().

But TCP socket may be alive after task exit until it gets down to 
protocol close. For example, until the all message in send buffer
is acked, socket and tcp connection will not be disappear.

In short, socket's lifetime is different from it's task's. 
So, there may be sockets which are not belongs to any task.



Thanks,
-Kame













--
To unsubscribe from this list: send the line "unsubscribe cgroups" 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: [Bonding-devel] ethernet bonding + VLAN: additional VLAN tag in tcpdump
From: Thomas De Schampheleire @ 2011-12-05  9:50 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Nicolas de Pesloüan, bonding-devel, tcpdump-workers,
	Ronny Meeus, netdev@vger.kernel.org
In-Reply-To: <CAAXf6LXDYmjaQK6g0q6vZLeDCH1yb5MT03c21Ehqv6TgUB_R3w@mail.gmail.com>

Hi,

On Wed, Nov 30, 2011 at 10:06 AM, Thomas De Schampheleire
<patrickdepinguin@gmail.com> wrote:
> On Wed, Nov 30, 2011 at 8:52 AM, Jiri Pirko <jpirko@redhat.com> wrote:
>> Tue, Nov 29, 2011 at 09:35:00PM CET, nicolas.2p.debian@gmail.com wrote:
>>>Le 29/11/2011 14:38, Thomas De Schampheleire a écrit :
>>>>Hi,
>>>>
>>>>I'm seeing incorrect tcpdump output in the following scenario:
>>>>
>>>>* ethernet bonding enabled in the kernel, and a single network
>>>>interface (eth0) added as slave
>>>>* bonding mode was set to broadcast, but I don't think this matters
>>>>* VLAN added to the bond0 network interface
>>>>* ip address set on the vlan interface (bond0.1234)
>>>>* tcpdump capturing full packets (-xx or even -x) on the eth0 interface
>>>>
>>>>Then, when pinging from another machine to this ip address, the ping
>>>>reply packets shown by tcpdump incorrectly have a double VLAN tag.
>>>>However, what really appears on the wire is correct: a single VLAN
>>>>tag.
>>>
>>>Copied netdev, because bonding and vlan developers are there.
>>>
>>>Jiri, don't you think this might be related to the work you have done
>>>to make non-hw-accel rx path similar to hw-accel?
>>
>> I do not think so. The changes you are reffering to are unrelated to tx
>> path (where this issue has most probably roots in)
>>
>>>
>>>       Nicolas.
>>>
>>>>
>>>>Here is the output from tcpdump:
>>>># /tmp/tcpdump  -i eth0 -xx
>>
>> What hw is this?
>
> This is on a Freescale P4080 DPA mac (fsl,p4080-fman-1g-mac).
>
>>
>>>>tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
>>>>listening on eth0, link-type EN10MB (Ethernet), capture size 65535 bytes
>>>>01:04:04.607880 IP 192.168.1.2>  192.168.1.1: ICMP echo request, id 26933, seq 4
>>>>16, length 64
>>>>         0x0000:  0600 0000 0020 0600 0000 0020 8100 0ffe
>>>>         0x0010:  0800 4500 0054 0000 4000 4001 b755 c0a8
>>>>         0x0020:  0102 c0a8 0101 0800 98d7 6935 01a0 e528
>>>>         0x0030:  0f2a 0000 0000 0000 0000 0000 0000 0000
>>>>         0x0040:  0000 0000 0000 0000 0000 0000 0000 0000
>>>>         0x0050:  0000 0000 0000 0000 0000 0000 0000 0000
>>>>         0x0060:  0000 0000 0000
>>>>01:04:04.607889 IP 192.168.1.1>  192.168.1.2: ICMP echo reply, id 26933, seq 416
>>>>, length 64
>>>>         0x0000:  0600 0000 0020 0600 0000 0020 8100 0ffe
>>>>         0x0010:  8100 0ffe 0800 4500 0054 cc07 0000 4001<--------
>>>>extra VLAN header at 0x10
>>>>         0x0020:  2b4e c0a8 0101 c0a8 0102 0000 a0d7 6935
>>>>         0x0030:  01a0 e528 0f2a 0000 0000 0000 0000 0000
>>>>         0x0040:  0000 0000 0000 0000 0000 0000 0000 0000
>>>>         0x0050:  0000 0000 0000 0000 0000 0000 0000 0000
>>>>         0x0060:  0000 0000 0000 0000 0000
>>>>
>>>>
>>>>Initial debugging showed that the addition of the extra VLAN header
>>>>takes place in function pcap_read_linux_mmap() of libpcap, in the
>>>>following snippet:
>>>>
>>>>#ifdef HAVE_TPACKET2
>>>>                 if (handle->md.tp_version == TPACKET_V2&&  h.h2->tp_vlan_tci&&
>>>>                     tp_snaplen>= 2 * ETH_ALEN) {
>>>>                         struct vlan_tag *tag;
>>>>
>>>>                         bp -= VLAN_TAG_LEN;
>>>>                         memmove(bp, bp + VLAN_TAG_LEN, 2 * ETH_ALEN);
>>>>
>>>>                         tag = (struct vlan_tag *)(bp + 2 * ETH_ALEN);
>>>>                         tag->vlan_tpid = htons(ETH_P_8021Q);
>>>>                         tag->vlan_tci = htons(h.h2->tp_vlan_tci);
>>>>
>>>>                         pcaphdr.caplen += VLAN_TAG_LEN;
>>>>                         pcaphdr.len += VLAN_TAG_LEN;
>>>>                 }
>>>>#endif
>>
>> I haven't look into this code yet, but where's the code which does the
>> first header inclusion?
>
> I would assume this is done by the VLAN layer. This is a ping reply
> originating from the icmp code, passing down to the vlan layer, then
> to the ethernet bonding layer, and then to the hardware. But before
> this is passed to hardware, libpcap captures the packet.
>
> I haven't debugged that part, though, so I can't give you a direct
> pointer to the code that does it.
>
>>
>>
>>>>
>>>>Upon entry of this code, the packet in bp already contains a VLAN header.
>>>>
>>>>It's unclear to me where the problem lies exactly. I suspect it has
>>>>something to do with the ethernet bonding layer indicating it has
>>>>hardware vlan tagging support, while it does already fill in the vlan
>>>>header, and libpcap being confused by this.
>>>>
>>>>As mentioned previously, the packets on the wire are correct, and this
>>>>is purely a capturing problem.
>>>>
>

Does anyone have an idea on how this is supposed to work and why the
extra header gets inserted?

Thanks,
Thomas

^ permalink raw reply

* RE: [RFCv2 9/9] caif-xshm: Add CAIF driver for Shared memory for M7400
From: Sjur BRENDELAND @ 2011-12-05  9:43 UTC (permalink / raw)
  To: Paul Bolle
  Cc: linux-kernel@vger.kernel.org, Linus Walleij, sjurbren@gmail.com,
	netdev@vger.kernel.org
In-Reply-To: <1322826606.2313.14.camel@x61.thuisdomein>

Hi Paul,
 
> A few minor comments follow. They're basically identical to some of my
> comments on v1 of 8/9 ("xshm: Makefile and Kconfig for M7400 Shared
> Memory Drivers"). I guess I just didn't spot these the first time.

No worries, I should have fix this all over based on your previous comments anyway.


> > +config CAIF_XSHM
> > +	tristate "CAIF external memory protocol driver"
> > +	depends on XSHM && CAIF
> > +	default n
> > +	---help---
> > +	Say "yes" if you want to support CAIF over External Shared Memory
> (XSHM)
> 
> "Say Y"?
> 
> > +	IPC mechanism (e.g. over Chip to Chip).
> > +	This will normally be built-in, loadable module is used for
> testing.
> 
> Perhaps something like: "Only say M here if you want to test CAIF over
> XSHM and need to load and unload its module."?

So I will change drivers/net/caif/Kconfig like this:
-       Say "yes" if you want to support CAIF over External Shared Memory (XSHM)
-       IPC mechanism (e.g. over Chip to Chip).
-       This will normally be built-in, loadable module is used for testing.
+       Say "Y" if you want to support CAIF over External Shared Memory (XSHM)
+       IPC mechanism (e.g. over Chip to Chip). Only say M here if you want to
+       test CAIF over XSHM and need to load and unload its module.
        If unsure say N.

Thank you for reviewing this Paul.

Regards,
Sjur

^ permalink raw reply

* MESSAGE FROM FBI OFFICE, CAREFULLY READ THE ATTACHMENT AND COMPLY IMMEDIATELY
From: MUELLER @ 2011-12-05  9:24 UTC (permalink / raw)


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



[-- Attachment #2: Fund_Transfer_(1).doc --]
[-- Type: application/msword, Size: 33792 bytes --]

^ permalink raw reply

* Re: [PATCH v7 10/10] Disable task moving when using kernel memory accounting
From: Glauber Costa @ 2011-12-05  9:18 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: linux-kernel, paul, lizf, ebiederm, davem, gthelen, netdev,
	linux-mm, kirill, avagin, devel, eric.dumazet, cgroups
In-Reply-To: <20111205111835.b1432603.kamezawa.hiroyu@jp.fujitsu.com>

On 12/05/2011 12:18 AM, KAMEZAWA Hiroyuki wrote:
> On Fri, 2 Dec 2011 16:11:56 -0200
> Glauber Costa<glommer@parallels.com>  wrote:
>
>> On 11/30/2011 12:22 AM, KAMEZAWA Hiroyuki wrote:
>>> On Tue, 29 Nov 2011 21:57:01 -0200
>>> Glauber Costa<glommer@parallels.com>   wrote:
>>>
>>>> Since this code is still experimental, we are leaving the exact
>>>> details of how to move tasks between cgroups when kernel memory
>>>> accounting is used as future work.
>>>>
>>>> For now, we simply disallow movement if there are any pending
>>>> accounted memory.
>>>>
>>>> Signed-off-by: Glauber Costa<glommer@parallels.com>
>>>> CC: Hiroyouki Kamezawa<kamezawa.hiroyu@jp.fujitsu.com>
>>>> ---
>>>>    mm/memcontrol.c |   23 ++++++++++++++++++++++-
>>>>    1 files changed, 22 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>>>> index a31a278..dd9a6d9 100644
>>>> --- a/mm/memcontrol.c
>>>> +++ b/mm/memcontrol.c
>>>> @@ -5453,10 +5453,19 @@ static int mem_cgroup_can_attach(struct cgroup_subsys *ss,
>>>>    {
>>>>    	int ret = 0;
>>>>    	struct mem_cgroup *memcg = mem_cgroup_from_cont(cgroup);
>>>> +	struct mem_cgroup *from = mem_cgroup_from_task(p);
>>>> +
>>>> +#if defined(CONFIG_CGROUP_MEM_RES_CTLR_KMEM)&&   defined(CONFIG_INET)
>>>> +	if (from != memcg&&   !mem_cgroup_is_root(from)&&
>>>> +	    res_counter_read_u64(&from->tcp_mem.tcp_memory_allocated, RES_USAGE)) {
>>>> +		printk(KERN_WARNING "Can't move tasks between cgroups: "
>>>> +			"Kernel memory held.\n");
>>>> +		return 1;
>>>> +	}
>>>> +#endif
>>>
>>> I wonder....reading all codes again, this is incorrect check.
>>>
>>> Hm, let me cralify. IIUC, in old code, "prevent moving" is because you hold
>>> reference count of cgroup, which can cause trouble at rmdir() as leaking refcnt.
>> right.
>>
>>> BTW, because socket is a shared resource between cgroup, changes in mm->owner
>>> may cause task cgroup moving implicitly. So, if you allow leak of resource
>>> here, I guess... you can take mem_cgroup_get() refcnt which is memcg-local and
>>> allow rmdir(). Then, this limitation may disappear.
>>
>> Sorry, I didn't fully understand. Can you clarify further?
>> If the task is implicitly moved, it will end up calling can_attach as
>> well, right?
>>
> I'm sorry that my explanation is bad.
>
> You can take memory cgroup itself's reference count by mem_cgroup_put/get.
> By getting this, memory cgroup object will continue to exist even after
> its struct cgroup* is freed by rmdir().
>
> So, assume you do mem_cgroup_get()/put at socket attaching/detatching.
>
> 0) A task has a tcp socekts in memcg0.
>
> task(memcg0)
>   +- socket0 -->  memcg0,usage=4096
>
> 1) move this task to memcg1
>
> task(memcg1)
>   +- socket0 -->  memcg0,usage=4096
>
> 2) The task create a new socket.
>
> task(memcg1)
>   +- socekt0 -->  memcg0,usage=4096
>   +- socket1 -->  memcg1,usage=xxxx
>
> Here, the task will hold 4096bytes of usage in memcg0 implicitly.
>
> 3) an admin removes memcg0
> task(memcg1)
>   +- socket0 -->memcg0, usage=4096<-----(*)
>   +- socket1 -->memcg1, usage=xxxx
>
> (*) is invisible to users....but this will not be very big problem.
>
Hi Kame,

Thanks for the explanation.

Hummm, Do you think that by doing it, we get rid of the need of moving 
sockets to another memcg when the task is moved? So in my original 
patchset, if you recall, I wanted to keep a socket forever in the same 
cgroup. I didn't, because then rmdir would be blocked.

By using this memcg reference trick, both can be achieved. What do you 
think ?

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

^ permalink raw reply

* Re: [PATCH v7 00/10] Request for Inclusion: per-cgroup tcp memory pressure
From: Glauber Costa @ 2011-12-05  9:09 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: linux-kernel, paul, lizf, ebiederm, davem, gthelen, netdev,
	linux-mm, kirill, avagin, devel, eric.dumazet, cgroups
In-Reply-To: <20111205110619.ecc538a0.kamezawa.hiroyu@jp.fujitsu.com>

On 12/05/2011 12:06 AM, KAMEZAWA Hiroyuki wrote:
> On Fri, 2 Dec 2011 16:04:08 -0200
> Glauber Costa<glommer@parallels.com>  wrote:
>
>> On 11/30/2011 12:11 AM, KAMEZAWA Hiroyuki wrote:
>>> On Tue, 29 Nov 2011 21:56:51 -0200
>>> Glauber Costa<glommer@parallels.com>   wrote:
>>>
>>>> Hi,
>>>>
>>>> This patchset implements per-cgroup tcp memory pressure controls. It did not change
>>>> significantly since last submission: rather, it just merges the comments Kame had.
>>>> Most of them are style-related and/or Documentation, but there are two real bugs he
>>>> managed to spot (thanks)
>>>>
>>>> Please let me know if there is anything else I should address.
>>>>
>>>
>>> After reading all codes again, I feel some strange. Could you clarify ?
>>>
>>> Here.
>>> ==
>>> +void sock_update_memcg(struct sock *sk)
>>> +{
>>> +	/* right now a socket spends its whole life in the same cgroup */
>>> +	if (sk->sk_cgrp) {
>>> +		WARN_ON(1);
>>> +		return;
>>> +	}
>>> +	if (static_branch(&memcg_socket_limit_enabled)) {
>>> +		struct mem_cgroup *memcg;
>>> +
>>> +		BUG_ON(!sk->sk_prot->proto_cgroup);
>>> +
>>> +		rcu_read_lock();
>>> +		memcg = mem_cgroup_from_task(current);
>>> +		if (!mem_cgroup_is_root(memcg))
>>> +			sk->sk_cgrp = sk->sk_prot->proto_cgroup(memcg);
>>> +		rcu_read_unlock();
>>> ==
>>>
>>> sk->sk_cgrp is set to a memcg without any reference count.
>>>
>>> Then, no check for preventing rmdir() and freeing memcgroup.
>>>
>>> Is there some css_get() or mem_cgroup_get() somewhere ?
>>>
>>
>> There were a css_get in the first version of this patchset. It was
>> removed, however, because it was deemed anti-intuitive to prevent rmdir,
>> since we can't know which sockets are blocking it, or do anything about
>> it. Or did I misunderstand something ?
>>
>
> Maybe I misuderstood. Thank you. Ok, there is no css_get/put and
> rmdir() is allowed. But, hmm....what's guarding threads from stale
> pointer access ?
>
> Does a memory cgroup which is pointed by sk->sk_cgrp always exist ?
>
If I am not mistaken, yes, it will. (Ok, right now it won't)

Reason is a cgroup can't be removed if it is empty.
To make it empty, you need to move the tasks away.

So the sockets will be moved away as well when you do it. So right now 
they are not, so it would then probably be better to increase a 
reference count with a comment saying that it is temporary.

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

^ permalink raw reply

* Re: [PATCH v7 02/10] foundations of per-cgroup memory pressure controlling.
From: Glauber Costa @ 2011-12-05  9:06 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: linux-kernel, paul, lizf, ebiederm, davem, gthelen, netdev,
	linux-mm, kirill, avagin, devel, eric.dumazet, cgroups,
	Peter Zijlstra, Paul Turner, Johannes Weiner, Michal Hocko,
	vgoyal
In-Reply-To: <20111205105916.eeb55989.kamezawa.hiroyu@jp.fujitsu.com>

On 12/04/2011 11:59 PM, KAMEZAWA Hiroyuki wrote:
> On Fri, 2 Dec 2011 15:46:46 -0200
> Glauber Costa<glommer@parallels.com>  wrote:
>
>>
>>>>    static void proto_seq_printf(struct seq_file *seq, struct proto *proto)
>>>>    {
>>>> +	struct mem_cgroup *memcg = mem_cgroup_from_task(current);
>>>> +
>>>>    	seq_printf(seq, "%-9s %4u %6d  %6ld   %-3s %6u   %-3s  %-10s "
>>>>    			"%2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c %2c\n",
>>>>    		   proto->name,
>>>>    		   proto->obj_size,
>>>>    		   sock_prot_inuse_get(seq_file_net(seq), proto),
>>>> -		   proto->memory_allocated != NULL ? atomic_long_read(proto->memory_allocated) : -1L,
>>>> -		   proto->memory_pressure != NULL ? *proto->memory_pressure ? "yes" : "no" : "NI",
>>>> +		   sock_prot_memory_allocated(proto, memcg),
>>>> +		   sock_prot_memory_pressure(proto, memcg),
>>>
>>> I wonder I should say NO, here. (Networking guys are ok ??)
>>>
>>> IIUC, this means there is no way to see aggregated sockstat of all system.
>>> And the result depends on the cgroup which the caller is under control.
>>>
>>> I think you should show aggregated sockstat(global + per-memcg) here and
>>> show per-memcg ones via /cgroup interface or add private_sockstat to show
>>> per cgroup summary.
>>>
>>
>> Hi Kame,
>>
>> Yes, the statistics displayed depends on which cgroup you live.
>> Also, note that the parent cgroup here is always updated (even when
>> use_hierarchy is set to 0). So it is always possible to grab global
>> statistics, by being in the root cgroup.
>>
>> For the others, I believe it to be a question of naturalization. Any
>> tool that is fetching these values is likely interested in the amount of
>> resources available/used. When you are on a cgroup, the amount of
>> resources available/used changes, so that's what you should see.
>>
>> Also brings the point of resource isolation: if you shouldn't interfere
>> with other set of process' resources, there is no reason for you to see
>> them in the first place.
>>
>> So given all that, I believe that whenever we talk about resources in a
>> cgroup, we should talk about cgroup-local ones.
>
> But you changes /proc/ information without any arguments with other guys.
> If you go this way, you should move this patch as independent add-on patch
> and discuss what this should be. For example, /proc/meminfo doesn't reflect
> memcg's information (for now). And scheduler statiscits in /proc/stat doesn't
> reflect cgroup's information.

No, I do not.
I may not have discussed it with everybody, but I did send some mails 
about it a while ago:

https://lkml.org/lkml/2011/10/3/60 (I sent it to containers as well 
once, but I now realize it was during the time the ML was down).

At the time, *I* was probably the only one, arguing not to do it. I've 
changed my mind since then.

> So, please discuss the problem in open way. This issue is not only related to
> this patch but also to other cgroups. Sneaking this kind of _big_ change in
> a middle of complicated patch series isn't good.

Absolutely. I can even remove this entirely and queue it for a following 
patchset if you prefer.

> In short, could you divide this patch into a independent patch and discuss
> again ? If we agree the general diection should go this way, other guys will
> post patches for cpu, memory, blkio, etc.

Yes I can.

I am expanding the CC list here so other people that cares for other 
controllers can chime in. You are welcome to give your opinion as the 
memcg maintainer as well.

^ permalink raw reply

* Time in Queue, bufferbloat, and... our accidentally interplanetary network
From: Dave Taht @ 2011-12-05  9:05 UTC (permalink / raw)
  To: bloat, bloat-devel, netdev-u79uwXL29TY76Z2rM5mHXA, linux-wireless

I was tickled to see that expiring packets based on 'time in queue'
was already a
key design feature of the IPN,

http://www.science20.com/interwebometry/lessons_nasa_can_learn_internet-84861

and the idea was suggested also independently in saturday's CACM
article's comments...

http://queue.acm.org/detail.cfm?id=2071893


Making decisions based on time in queue (TiQ), rather than length of
queue, would
seem to be a win, especially for wireless, but also for  that does
'soft' traffic
shaping with HTB-like qdiscs and sub qdiscs. Anything that is not running
at GigE plus speeds would benefit.

... since basically what's been happening with bufferbloat is a too early
implementation of the IPN, with detours between here and the moon!

... and so far I haven't seen any major theoretical holes in with TiQ, except
for deciding as to how long is too long as to consider a packet as 'expired',
(which can be measured in ms or 10s of ms), and having reasonably
monotonic time.

I just wish I (or someone) could come up with a way to implement it in Linux
without multiple layer violations. skb->queuestamp has a nice ring to it, but
when I look at this portion of the stack I freely admit quivering in ignorance
and fear.

I did a bit of work on a set of timestamping fifos, but realized that
it was the
entire queue's duration from entrance to exit (through multiple scheduling
qdiscs) was what needed to be  measured, and the drop/no drop decision
needs to be made as late as possible - and preferably, the queue being
pulled from needs the next packet pulled forward so as to inform the
receiver that congestion is happening...

And Eric dumazet also produced a preliminary patch a few weeks back
that tied timestamping to before the head of a queue, but that tried to use a
reserved field in the skb that appears from points A to Z is not guaranteed
to be preserved.

Thoughts?

-- 
Dave Täht
SKYPE: davetaht
US Tel: 1-239-829-5608
FR Tel: 0638645374
http://www.bufferbloat.net
--
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

* [net-next RFC PATCH 5/5] virtio-net: flow director support
From: Jason Wang @ 2011-12-05  8:59 UTC (permalink / raw)
  To: krkumar2, kvm, mst, netdev, rusty, virtualization, levinsasha928,
	bhutchings
In-Reply-To: <20111205085603.6116.65101.stgit@dhcp-8-146.nay.redhat.com>

In order to let the packets of a flow to be passed to the desired
guest cpu, we can co-operate with devices through programming the flow
director which was just a hash to queue table.

This kinds of co-operation is done through the accelerate RFS support,
a device specific flow sterring method virtnet_fd() is used to modify
the flow director based on rfs mapping. The desired queue were
calculated through reverse mapping of the irq affinity table. In order
to parallelize the ingress path, irq affinity of rx queue were also
provides by the driver.

In addition to accelerate RFS, we can also use the guest scheduler to
balance the load of TX and reduce the lock contention on egress path,
so the processor_id() were used to tx queue selection.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/net/virtio_net.c   |  165 +++++++++++++++++++++++++++++++++++++++++++-
 include/linux/virtio_net.h |    6 ++
 2 files changed, 169 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 0d871f8..89bb5e7 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -26,6 +26,10 @@
 #include <linux/scatterlist.h>
 #include <linux/if_vlan.h>
 #include <linux/slab.h>
+#include <linux/highmem.h>
+#include <linux/cpu_rmap.h>
+#include <linux/interrupt.h>
+#include <linux/cpumask.h>
 
 static int napi_weight = 128;
 module_param(napi_weight, int, 0444);
@@ -40,6 +44,7 @@ module_param(gso, bool, 0444);
 
 #define VIRTNET_SEND_COMMAND_SG_MAX    2
 #define VIRTNET_DRIVER_VERSION "1.0.0"
+#define TAP_HASH_MASK 0xFF
 
 struct virtnet_send_stats {
 	struct u64_stats_sync syncp;
@@ -89,6 +94,9 @@ struct receive_queue {
 
 	/* Active rx statistics */
 	struct virtnet_recv_stats __percpu *stats;
+
+	/* FIXME: per vector instead of per queue ?? */
+	cpumask_var_t affinity_mask;
 };
 
 struct virtnet_info {
@@ -110,6 +118,11 @@ struct virtnet_info {
 
 	/* Host will pass rxhash to us. */
 	bool has_rxhash;
+
+	/* A page of flow director */
+	struct page *fd_page;
+
+	cpumask_var_t affinity_mask;
 };
 
 struct skb_vnet_hdr {
@@ -386,6 +399,7 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
 	if (vi->has_rxhash)
 		skb->rxhash = hdr->rhdr.rxhash;
 
+	skb_record_rx_queue(skb, rq->vq->queue_index / 2);
 	netif_receive_skb(skb);
 	return;
 
@@ -722,6 +736,19 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 	return NETDEV_TX_OK;
 }
 
+static int virtnet_set_fd(struct net_device *dev, u32 pfn)
+{
+	struct virtnet_info *vi = netdev_priv(dev);
+	struct virtio_device *vdev = vi->vdev;
+
+	if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_FD)) {
+		vdev->config->set(vdev,
+				  offsetof(struct virtio_net_config_fd, addr),
+				  &pfn, sizeof(u32));
+	}
+	return 0;
+}
+
 static int virtnet_set_mac_address(struct net_device *dev, void *p)
 {
 	struct virtnet_info *vi = netdev_priv(dev);
@@ -1017,6 +1044,39 @@ static int virtnet_change_mtu(struct net_device *dev, int new_mtu)
 	return 0;
 }
 
+#ifdef CONFIG_RFS_ACCEL
+
+int virtnet_fd(struct net_device *net_dev, const struct sk_buff *skb,
+	       u16 rxq_index, u32 flow_id)
+{
+	struct virtnet_info *vi = netdev_priv(net_dev);
+	u16 *table = NULL;
+
+	if (skb->protocol != htons(ETH_P_IP) || !skb->rxhash)
+		return -EPROTONOSUPPORT;
+
+	table = kmap_atomic(vi->fd_page);
+	table[skb->rxhash & TAP_HASH_MASK] = rxq_index;
+	kunmap_atomic(table);
+
+	return 0;
+}
+#endif
+
+static u16 virtnet_select_queue(struct net_device *dev, struct sk_buff *skb)
+{
+	int txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
+					       smp_processor_id();
+
+	/* As we make use of the accelerate rfs which let the scheduler to
+	 * balance the load, it make sense to choose the tx queue also based on
+	 * theprocessor id?
+	 */
+	while (unlikely(txq >= dev->real_num_tx_queues))
+		txq -= dev->real_num_tx_queues;
+	return txq;
+}
+
 static const struct net_device_ops virtnet_netdev = {
 	.ndo_open            = virtnet_open,
 	.ndo_stop   	     = virtnet_close,
@@ -1028,9 +1088,13 @@ static const struct net_device_ops virtnet_netdev = {
 	.ndo_get_stats64     = virtnet_stats,
 	.ndo_vlan_rx_add_vid = virtnet_vlan_rx_add_vid,
 	.ndo_vlan_rx_kill_vid = virtnet_vlan_rx_kill_vid,
+	.ndo_select_queue    = virtnet_select_queue,
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	.ndo_poll_controller = virtnet_netpoll,
 #endif
+#ifdef CONFIG_RFS_ACCEL
+	.ndo_rx_flow_steer   = virtnet_fd,
+#endif
 };
 
 static void virtnet_update_status(struct virtnet_info *vi)
@@ -1272,12 +1336,76 @@ static int virtnet_setup_vqs(struct virtnet_info *vi)
 	return ret;
 }
 
+static int virtnet_init_rx_cpu_rmap(struct virtnet_info *vi)
+{
+#ifdef CONFIG_RFS_ACCEL
+	struct virtio_device *vdev = vi->vdev;
+	int i, rc;
+
+	vi->dev->rx_cpu_rmap = alloc_irq_cpu_rmap(vi->num_queue_pairs);
+	if (!vi->dev->rx_cpu_rmap)
+		return -ENOMEM;
+	for (i = 0; i < vi->num_queue_pairs; i++) {
+		rc = irq_cpu_rmap_add(vi->dev->rx_cpu_rmap,
+				vdev->config->get_vq_irq(vdev, vi->rq[i]->vq));
+		if (rc) {
+			free_irq_cpu_rmap(vi->dev->rx_cpu_rmap);
+			vi->dev->rx_cpu_rmap = NULL;
+			return rc;
+		}
+	}
+#endif
+	return 0;
+}
+
+static int virtnet_init_rq_affinity(struct virtnet_info *vi)
+{
+	struct virtio_device *vdev = vi->vdev;
+	int i;
+
+	/* FIXME: TX/RX share a vector */
+	for (i = 0; i < vi->num_queue_pairs; i++) {
+		if (!alloc_cpumask_var(&vi->rq[i]->affinity_mask, GFP_KERNEL))
+			goto err_out;
+		cpumask_set_cpu(i, vi->rq[i]->affinity_mask);
+		irq_set_affinity_hint(vdev->config->get_vq_irq(vdev,
+							       vi->rq[i]->vq),
+				      vi->rq[i]->affinity_mask);
+	}
+
+	return 0;
+err_out:
+	while (i) {
+		i--;
+		irq_set_affinity_hint(vdev->config->get_vq_irq(vdev,
+							       vi->rq[i]->vq),
+				      NULL);
+		free_cpumask_var(vi->rq[i]->affinity_mask);
+	}
+	return -ENOMEM;
+}
+
+static void virtnet_free_rq_affinity(struct virtnet_info *vi)
+{
+	struct virtio_device *vdev = vi->vdev;
+	int i;
+
+	for (i = 0; i < vi->num_queue_pairs; i++) {
+		irq_set_affinity_hint(vdev->config->get_vq_irq(vdev,
+							       vi->rq[i]->vq),
+				      NULL);
+		free_cpumask_var(vi->rq[i]->affinity_mask);
+	}
+}
+
 static int virtnet_probe(struct virtio_device *vdev)
 {
 	int i, err;
 	struct net_device *dev;
 	struct virtnet_info *vi;
 	u16 num_queues, num_queue_pairs;
+	struct page *page = NULL;
+	u16 *table = NULL;
 
 	/* Find if host supports multiqueue virtio_net device */
 	err = virtio_config_val(vdev, VIRTIO_NET_F_MULTIQUEUE,
@@ -1298,7 +1426,7 @@ static int virtnet_probe(struct virtio_device *vdev)
 	/* Set up network device as normal. */
 	dev->priv_flags |= IFF_UNICAST_FLT;
 	dev->netdev_ops = &virtnet_netdev;
-	dev->features = NETIF_F_HIGHDMA;
+	dev->features = NETIF_F_HIGHDMA | NETIF_F_NTUPLE;
 
 	SET_ETHTOOL_OPS(dev, &virtnet_ethtool_ops);
 	SET_NETDEV_DEV(dev, &vdev->dev);
@@ -1342,6 +1470,7 @@ static int virtnet_probe(struct virtio_device *vdev)
 	vdev->priv = vi;
 	vi->num_queue_pairs = num_queue_pairs;
 
+
 	/* If we can receive ANY GSO packets, we must allocate large ones. */
 	if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO4) ||
 	    virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO6) ||
@@ -1382,6 +1511,31 @@ static int virtnet_probe(struct virtio_device *vdev)
 		}
 	}
 
+	/* Config flow director */
+	if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_FD)) {
+		page = alloc_page(GFP_KERNEL);
+		if (!page)
+			return -ENOMEM;
+		table = (u16 *)kmap_atomic(page);
+		for (i = 0; i < (PAGE_SIZE / 16); i++) {
+			/* invalid all entries */
+			table[i] = num_queue_pairs;
+		}
+
+		vi->fd_page = page;
+		kunmap_atomic(table);
+		virtnet_set_fd(dev, page_to_pfn(page));
+
+		err = virtnet_init_rx_cpu_rmap(vi);
+		if (err)
+			goto free_recv_bufs;
+
+		err = virtnet_init_rq_affinity(vi);
+		if (err)
+			goto free_recv_bufs;
+
+	}
+
 	/* Assume link up if device can't report link status,
 	   otherwise get link status from config. */
 	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_STATUS)) {
@@ -1437,6 +1591,13 @@ static void __devexit virtnet_remove(struct virtio_device *vdev)
 	/* Free memory for send and receive queues */
 	free_rq_sq(vi);
 
+	/* Free the page of flow director */
+	if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_FD)) {
+		if (vi->fd_page)
+			put_page(vi->fd_page);
+
+		virtnet_free_rq_affinity(vi);
+	}
 	free_netdev(vi->dev);
 }
 
@@ -1453,7 +1614,7 @@ static unsigned int features[] = {
 	VIRTIO_NET_F_GUEST_ECN, VIRTIO_NET_F_GUEST_UFO,
 	VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ,
 	VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, VIRTIO_NET_F_MULTIQUEUE,
-	VIRTIO_NET_F_GUEST_RXHASH,
+	VIRTIO_NET_F_GUEST_RXHASH, VIRTIO_NET_F_HOST_FD,
 };
 
 static struct virtio_driver virtio_net_driver = {
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 2291317..abcea52 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -51,6 +51,7 @@
 #define VIRTIO_NET_F_CTRL_RX_EXTRA 20	/* Extra RX mode control support */
 #define VIRTIO_NET_F_MULTIQUEUE	21	/* Device supports multiple TXQ/RXQ */
 #define VIRTIO_NET_F_GUEST_RXHASH 22    /* Guest can receive rxhash */
+#define VIRTIO_NET_F_HOST_FD    23      /* Host has a flow director */
 
 #define VIRTIO_NET_S_LINK_UP	1	/* Link is up */
 
@@ -63,6 +64,11 @@ struct virtio_net_config {
 	__u16 num_queues;
 } __attribute__((packed));
 
+struct virtio_net_config_fd {
+	struct virtio_net_config cfg;
+	u32 addr;
+} __packed;
+
 /* This is the first element of the scatter-gather list.  If you don't
  * specify GSO, CSUM or HASH features, you can simply ignore the header. */
 struct virtio_net_hdr {


^ permalink raw reply related

* [net-next RFC PATCH 4/5] virtio: introduce a method to get the irq of a specific virtqueue
From: Jason Wang @ 2011-12-05  8:59 UTC (permalink / raw)
  To: krkumar2, kvm, mst, netdev, rusty, virtualization, levinsasha928,
	bhutchings
In-Reply-To: <20111205085603.6116.65101.stgit@dhcp-8-146.nay.redhat.com>

Device specific irq configuration may be need in order to do some
optimization. So a new configuration is needed to get the irq of a
virtqueue.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/lguest/lguest_device.c |    8 ++++++++
 drivers/s390/kvm/kvm_virtio.c  |    6 ++++++
 drivers/virtio/virtio_mmio.c   |    8 ++++++++
 drivers/virtio/virtio_pci.c    |   12 ++++++++++++
 include/linux/virtio_config.h  |    4 ++++
 5 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c
index 595d731..6483bff 100644
--- a/drivers/lguest/lguest_device.c
+++ b/drivers/lguest/lguest_device.c
@@ -386,6 +386,13 @@ static const char *lg_bus_name(struct virtio_device *vdev)
 	return "";
 }
 
+static int lg_get_vq_irq(struct virtio_device *vdev, struct virtqueue *vq)
+{
+	struct lguest_vq_info *lvq = vq->priv;
+
+	return lvq->config.irq;
+}
+
 /* The ops structure which hooks everything together. */
 static struct virtio_config_ops lguest_config_ops = {
 	.get_features = lg_get_features,
@@ -398,6 +405,7 @@ static struct virtio_config_ops lguest_config_ops = {
 	.find_vqs = lg_find_vqs,
 	.del_vqs = lg_del_vqs,
 	.bus_name = lg_bus_name,
+	.get_vq_irq = lg_get_vq_irq,
 };
 
 /*
diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c
index 8af868b..a8d5ca1 100644
--- a/drivers/s390/kvm/kvm_virtio.c
+++ b/drivers/s390/kvm/kvm_virtio.c
@@ -268,6 +268,11 @@ static const char *kvm_bus_name(struct virtio_device *vdev)
 	return "";
 }
 
+static int kvm_get_vq_irq(struct virtio_device *vdev, struct virtqueue *vq)
+{
+	return 0x2603;
+}
+
 /*
  * The config ops structure as defined by virtio config
  */
@@ -282,6 +287,7 @@ static struct virtio_config_ops kvm_vq_configspace_ops = {
 	.find_vqs = kvm_find_vqs,
 	.del_vqs = kvm_del_vqs,
 	.bus_name = kvm_bus_name,
+	.get_vq_irq = kvm_get_vq_irq,
 };
 
 /*
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 2f57380..309d471 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -368,6 +368,13 @@ static const char *vm_bus_name(struct virtio_device *vdev)
 	return vm_dev->pdev->name;
 }
 
+static int vm_get_vq_irq(struct virtio_device *vdev, struct virtqueue *vq)
+{
+	struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
+
+	return platform_get_irq(vm_dev->pdev, 0);
+}
+
 static struct virtio_config_ops virtio_mmio_config_ops = {
 	.get		= vm_get,
 	.set		= vm_set,
@@ -379,6 +386,7 @@ static struct virtio_config_ops virtio_mmio_config_ops = {
 	.get_features	= vm_get_features,
 	.finalize_features = vm_finalize_features,
 	.bus_name	= vm_bus_name,
+	.get_vq_irq     = vm_get_vq_irq,
 };
 
 
diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index 229ea56..4f99164 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -583,6 +583,17 @@ static const char *vp_bus_name(struct virtio_device *vdev)
 	return pci_name(vp_dev->pci_dev);
 }
 
+static int vp_get_vq_irq(struct virtio_device *vdev, struct virtqueue *vq)
+{
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+	struct virtio_pci_vq_info *info = vq->priv;
+
+	if (vp_dev->intx_enabled)
+		return vp_dev->pci_dev->irq;
+	else
+		return vp_dev->msix_entries[info->msix_vector].vector;
+}
+
 static struct virtio_config_ops virtio_pci_config_ops = {
 	.get		= vp_get,
 	.set		= vp_set,
@@ -594,6 +605,7 @@ static struct virtio_config_ops virtio_pci_config_ops = {
 	.get_features	= vp_get_features,
 	.finalize_features = vp_finalize_features,
 	.bus_name	= vp_bus_name,
+	.get_vq_irq     = vp_get_vq_irq,
 };
 
 static void virtio_pci_release_dev(struct device *_d)
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 63f98d0..7b783a6 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -104,6 +104,9 @@
  *	vdev: the virtio_device
  *      This returns a pointer to the bus name a la pci_name from which
  *      the caller can then copy.
+ * @get_vq_irq: get the irq numer of the specific virt queue.
+ *      vdev: the virtio_device
+ *      vq: the virtqueue
  */
 typedef void vq_callback_t(struct virtqueue *);
 struct virtio_config_ops {
@@ -122,6 +125,7 @@ struct virtio_config_ops {
 	u32 (*get_features)(struct virtio_device *vdev);
 	void (*finalize_features)(struct virtio_device *vdev);
 	const char *(*bus_name)(struct virtio_device *vdev);
+	int (*get_vq_irq)(struct virtio_device *vdev, struct virtqueue *vq);
 };
 
 /* If driver didn't advertise the feature, it will never appear. */

^ permalink raw reply related

* [net-next RFC PATCH 3/5] macvtap: flow director support
From: Jason Wang @ 2011-12-05  8:59 UTC (permalink / raw)
  To: krkumar2, kvm, mst, netdev, rusty, virtualization, levinsasha928,
	bhutchings
In-Reply-To: <20111205085603.6116.65101.stgit@dhcp-8-146.nay.redhat.com>

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/net/macvlan.c      |    4 ++++
 drivers/net/macvtap.c      |   36 ++++++++++++++++++++++++++++++++++--
 include/linux/if_macvlan.h |    1 +
 3 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 7413497..b0cb7ce 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -706,6 +706,7 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
 	vlan->port     = port;
 	vlan->receive  = receive;
 	vlan->forward  = forward;
+	vlan->fd_page[0] = NULL;
 
 	vlan->mode     = MACVLAN_MODE_VEPA;
 	if (data && data[IFLA_MACVLAN_MODE])
@@ -749,6 +750,9 @@ void macvlan_dellink(struct net_device *dev, struct list_head *head)
 {
 	struct macvlan_dev *vlan = netdev_priv(dev);
 
+	if (vlan->fd_page[0])
+		put_page(vlan->fd_page[0]);
+
 	list_del(&vlan->list);
 	unregister_netdevice_queue(dev, head);
 }
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 504c745..a34eb84 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -14,6 +14,7 @@
 #include <linux/wait.h>
 #include <linux/cdev.h>
 #include <linux/fs.h>
+#include <linux/highmem.h>
 
 #include <net/net_namespace.h>
 #include <net/rtnetlink.h>
@@ -62,6 +63,8 @@ static DEFINE_IDR(minor_idr);
 static struct class *macvtap_class;
 static struct cdev macvtap_cdev;
 
+#define TAP_HASH_MASK 0xFF
+
 static const struct proto_ops macvtap_socket_ops;
 
 /*
@@ -189,6 +192,11 @@ static struct macvtap_queue *macvtap_get_queue(struct net_device *dev,
 	/* Check if we can use flow to select a queue */
 	rxq = skb_get_rxhash(skb);
 	if (rxq) {
+		if (vlan->fd_page[0]) {
+			u16 *table = kmap_atomic(vlan->fd_page[0]);
+			rxq = table[rxq & TAP_HASH_MASK];
+			kunmap_atomic(table);
+		}
 		tap = rcu_dereference(vlan->taps[rxq % numvtaps]);
 		if (tap)
 			goto out;
@@ -851,6 +859,7 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
 {
 	struct macvtap_queue *q = file->private_data;
 	struct macvlan_dev *vlan;
+	struct tun_fd tfd;
 	void __user *argp = (void __user *)arg;
 	struct ifreq __user *ifr = argp;
 	unsigned int __user *up = argp;
@@ -891,8 +900,8 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
 		return ret;
 
 	case TUNGETFEATURES:
-		if (put_user(IFF_TAP | IFF_NO_PI | IFF_VNET_HDR | IFF_RXHASH,
-			     up))
+		if (put_user(IFF_TAP | IFF_NO_PI | IFF_VNET_HDR | IFF_RXHASH |
+			     IFF_FD, up))
 			return -EFAULT;
 		return 0;
 
@@ -918,6 +927,29 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
 		q->vnet_hdr_sz = s;
 		return 0;
 
+	case TUNSETFD:
+		rcu_read_lock_bh();
+		vlan = rcu_dereference(q->vlan);
+		if (!vlan)
+			ret = -ENOLINK;
+		else {
+			if (copy_from_user(&tfd, argp, sizeof(tfd)))
+				ret = -EFAULT;
+			if (vlan->fd_page[0]) {
+				put_page(vlan->fd_page[0]);
+				vlan->fd_page[0] = NULL;
+			}
+
+			/* put_page() in macvlan_dellink() */
+			if (get_user_pages_fast(tfd.addr, 1, 0,
+						&vlan->fd_page[0]) != 1)
+				ret = -EFAULT;
+			else
+				ret = 0;
+		}
+		rcu_read_unlock_bh();
+		return ret;
+
 	case TUNSETOFFLOAD:
 		/* let the user check for future flags */
 		if (arg & ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
diff --git a/include/linux/if_macvlan.h b/include/linux/if_macvlan.h
index d103dca..69a87a1 100644
--- a/include/linux/if_macvlan.h
+++ b/include/linux/if_macvlan.h
@@ -65,6 +65,7 @@ struct macvlan_dev {
 	struct macvtap_queue	*taps[MAX_MACVTAP_QUEUES];
 	int			numvtaps;
 	int			minor;
+	struct page             *fd_page[1];
 };
 
 static inline void macvlan_count_rx(const struct macvlan_dev *vlan,

^ permalink raw reply related


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