Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] pktgen: Clone skb to avoid corruption of skbs in ndo_start_xmit methods
From: Neil Horman @ 2011-07-20  0:19 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Jiri Pirko, netdev, Alexey Dobriyan, David S. Miller
In-Reply-To: <1311108107.3113.22.camel@edumazet-laptop>

On Tue, Jul 19, 2011 at 10:41:47PM +0200, Eric Dumazet wrote:
> Le mardi 19 juillet 2011 à 22:29 +0200, Jiri Pirko a écrit :
> 
> > You are right, but it may not cause panic, right? In case this patch
> > would cause significant performance regression, how about to just forbid
> > pktgen to run on soft-netdevs ?
> > 
> 
> Please do
> 
You are correct Eric, this can cause a significant performance regression, but I
think that beats causing a panic or other unexpected behavior.  I read your
previous threads with others regarding fixing this with vlans, but I don't think
its fair to just say 'its fast, but it might cause oopses'. 

And its not sufficient to simply forbid soft drivers to make use of pktgen, its
not just a soft driver problem, its systemic.  Any driver which assumes that it
has exclusive access to an skb submitted for transmit is at risk from pktgen in
its current implementation.  That of course as a subset includes all the soft
drivers, but others are also suceptible.  As examples (some of which I noted in
the origional post) virtio_net uses the skb->cb to hold vnet header information
which will be corrupted on sucessive sends.  bnx2x linearizes skbs under certain
circumstances, which means pktgen, if it marshals a fragmented frame will not
send a fragmented frame after the first iteration.  The PPP and Slip drivers
skb_push the skb to prepend a header to the frame on send, meaning sucessive
uses, up until they get an skb_under_panic will get iteratively more malformed
frames on the wire as ppp headers get stacked on top of one another.  These are
ust a few of the examples I've found.

The long and the short of it in my mind, is that we have a fundamental
disconnect between driver asumptions and pktgen.  If its ok to submit shared
skbs to drivers, then we need to augment drivers that modify skbs on transmit to
clone the skb (likey not an efficient solution), or if its not ok to do so, we
need to change pktgen to not behave that way.

> Note : a sysadmin has other ways to make a machine panic or reboot or
> halt...
Yes, predictable ways, that the sysadmin can see coming based on what they're
doing (i.e. no one should be shocked if they dd /dev/random to /dev/kmem and get
a hang or panic, or if they issue a sysrq-c, etc).  This case is different.  A
sysadmin reasonably expects pktgen to send the frames they configure on the
interface they specify.  While its arguably reasonable to forsee that it may not
work with soft interfaces, pktgen just won't work with some hardware drivers (as
per the examples above).  And it won't always be an oops, it may be occasional
random behvaior in the output data, and its highly dependent not just on the use
of pktgen, but rather the specific command(s) issued.


I'm sensitive to the performance impact, but I would much rather see a lower
performing pktgen that doesn't randomly crash, and bring the performance back up
in a safe, reliable way.  To that end, I've been starting to think about
pre-allocating a ring buffer of skbs with a skb->users count biased up to
prevent driver freeing.  That way we could detect 'unused skb's' by a user count
that was at the bias level.  Thoughts?

Neil

> 
> 
> 
> 

^ permalink raw reply

* Re: [PATCH] pktgen: Clone skb to avoid corruption of skbs in ndo_start_xmit methods
From: Rick Jones @ 2011-07-20  0:25 UTC (permalink / raw)
  To: Neil Horman
  Cc: Eric Dumazet, Jiri Pirko, netdev, Alexey Dobriyan,
	David S. Miller
In-Reply-To: <20110720001904.GA1992@neilslaptop.think-freely.org>

How "everyday use" is pktgen considered?  As "everyday use" as 
netperf/iperf/whatnot? Is it actually considered something one would run 
on production rather than test systems?

rick jones

^ permalink raw reply

* Re: [PATCH] pktgen: Clone skb to avoid corruption of skbs in ndo_start_xmit methods
From: Eric Dumazet @ 2011-07-20  0:43 UTC (permalink / raw)
  To: Neil Horman; +Cc: Jiri Pirko, netdev, Alexey Dobriyan, David S. Miller
In-Reply-To: <20110720001904.GA1992@neilslaptop.think-freely.org>

Le mardi 19 juillet 2011 à 20:19 -0400, Neil Horman a écrit :
> > 
> You are correct Eric, this can cause a significant performance regression, but I
> think that beats causing a panic or other unexpected behavior.  I read your
> previous threads with others regarding fixing this with vlans, but I don't think
> its fair to just say 'its fast, but it might cause oopses'. 
> 
> And its not sufficient to simply forbid soft drivers to make use of pktgen, its
> not just a soft driver problem, its systemic.  Any driver which assumes that it
> has exclusive access to an skb submitted for transmit is at risk from pktgen in
> its current implementation.  That of course as a subset includes all the soft
> drivers, but others are also suceptible.  As examples (some of which I noted in
> the origional post) virtio_net uses the skb->cb to hold vnet header information
> which will be corrupted on sucessive sends.  bnx2x linearizes skbs under certain
> circumstances, which means pktgen, if it marshals a fragmented frame will not
> send a fragmented frame after the first iteration.  The PPP and Slip drivers
> skb_push the skb to prepend a header to the frame on send, meaning sucessive
> uses, up until they get an skb_under_panic will get iteratively more malformed
> frames on the wire as ppp headers get stacked on top of one another.  These are
> ust a few of the examples I've found.
> 
> The long and the short of it in my mind, is that we have a fundamental
> disconnect between driver asumptions and pktgen.  If its ok to submit shared
> skbs to drivers, then we need to augment drivers that modify skbs on transmit to
> clone the skb (likey not an efficient solution), or if its not ok to do so, we
> need to change pktgen to not behave that way.
> 

Its a known problem, please check mail archives. Nobody felt a fix was
needed.

> > Note : a sysadmin has other ways to make a machine panic or reboot or
> > halt...
> Yes, predictable ways, that the sysadmin can see coming based on what they're
> doing (i.e. no one should be shocked if they dd /dev/random to /dev/kmem and get
> a hang or panic, or if they issue a sysrq-c, etc).  This case is different.  A
> sysadmin reasonably expects pktgen to send the frames they configure on the
> interface they specify.  While its arguably reasonable to forsee that it may not
> work with soft interfaces, pktgen just won't work with some hardware drivers (as
> per the examples above).  And it won't always be an oops, it may be occasional
> random behvaior in the output data, and its highly dependent not just on the use
> of pktgen, but rather the specific command(s) issued.
> 
> 
> I'm sensitive to the performance impact, but I would much rather see a lower
> performing pktgen that doesn't randomly crash, and bring the performance back up
> in a safe, reliable way.  To that end, I've been starting to think about
> pre-allocating a ring buffer of skbs with a skb->users count biased up to
> prevent driver freeing.  That way we could detect 'unused skb's' by a user count
> that was at the bias level.  Thoughts?
> 

I dont know. I use pktgen maybe once per week and never got a single
crash like this. We probably are very few pktgen users in the world, and
we use it exactly to avoid calling skb_clone() or other expensive per
xmit setup.

Just remove pktgen from RedHat kernels, if you dont trust sysadmins.
# CONFIG_PKTGEN is not set

Alternatively, add a check to problematic drivers to _not_ mess skb if
skb_shared(skb) is true : eventually use skb_share_check()




^ permalink raw reply

* Re: [PATCH] pktgen: Clone skb to avoid corruption of skbs in ndo_start_xmit methods
From: Ben Greear @ 2011-07-20  0:52 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Neil Horman, Jiri Pirko, netdev, Alexey Dobriyan, David S. Miller
In-Reply-To: <1311122593.3113.46.camel@edumazet-laptop>

On 07/19/2011 05:43 PM, Eric Dumazet wrote:
> Le mardi 19 juillet 2011 à 20:19 -0400, Neil Horman a écrit :

>> I'm sensitive to the performance impact, but I would much rather see a lower
>> performing pktgen that doesn't randomly crash, and bring the performance back up
>> in a safe, reliable way.  To that end, I've been starting to think about
>> pre-allocating a ring buffer of skbs with a skb->users count biased up to
>> prevent driver freeing.  That way we could detect 'unused skb's' by a user count
>> that was at the bias level.  Thoughts?
>>
>
> I dont know. I use pktgen maybe once per week and never got a single
> crash like this. We probably are very few pktgen users in the world, and
> we use it exactly to avoid calling skb_clone() or other expensive per
> xmit setup.
>
> Just remove pktgen from RedHat kernels, if you dont trust sysadmins.
> # CONFIG_PKTGEN is not set
>
> Alternatively, add a check to problematic drivers to _not_ mess skb if
> skb_shared(skb) is true : eventually use skb_share_check()

When the features-flags work gets completed so that we can start adding
new flags, we could add a CANT_DO_MULTI_SKB flag to drivers with known
issues and then restrict pktgen config accordingly.

Upstream code already clears skb memory to avoid leaking kernel memory
contents, so if you take away multi-skb too, pktgen is going to suck
at what it is supposed to do:  run fast as possible.

If you want real fun, use pktgen on a wlan0 device...it will crash
regardless of whether you use multi-skb or not because of xmit-queue
number issues :P

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

^ permalink raw reply

* [PATCH] Constrain UFO fragment sizes to multiples of 8 bytes
From: Bill Sommerfeld @ 2011-07-20  1:22 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Tom Herbert, Maciej Żenczykowski, Bill Sommerfeld

Because the ip fragment offset field counts 8-byte chunks, ip
fragments other than the last must contain a multiple of 8 bytes of
payload.  ip_ufo_append_data wasn't respecting this constraint and,
depending on the MTU and ip option sizes, could create malformed
non-final fragments.

Google-Bug-Id: 5009328
Signed-off-by: Bill Sommerfeld <wsommerfeld@google.com>
---

Note to reviewers: The first two hunks simply rename the "mtu"
parameter to "maxfraglen"; the real change is in the third hunk.
maxfraglen (the size of the largest non-final fragment which fits
inside mtu) is already computed for the non-UFO fragmentation path.

We saw this problem in a 2.6.34-based kernel with a bond device that
had UFO enabled (as a result of changeset d9f5950f90292f7c);
1742f183fc218798 rewrote netdev_increment_features to no longer force
on UFO.  The MTU on this bond device was set to 1470; fragmented
datagrams were mangled, causing the receiver to get errors of the
form:

[506259.362640] UDP: short packet: From x.x.x.x:nnnn 1471/1469 to y.y.y.y:nnnn

 net/ipv4/ip_output.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index be27e60..ccaaa85 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -734,7 +734,7 @@ static inline int ip_ufo_append_data(struct sock *sk,
 			int getfrag(void *from, char *to, int offset, int len,
 			       int odd, struct sk_buff *skb),
 			void *from, int length, int hh_len, int fragheaderlen,
-			int transhdrlen, int mtu, unsigned int flags)
+			int transhdrlen, int maxfraglen, unsigned int flags)
 {
 	struct sk_buff *skb;
 	int err;
@@ -767,7 +767,7 @@ static inline int ip_ufo_append_data(struct sock *sk,
 		skb->csum = 0;
 
 		/* specify the length of each IP datagram fragment */
-		skb_shinfo(skb)->gso_size = mtu - fragheaderlen;
+		skb_shinfo(skb)->gso_size = maxfraglen - fragheaderlen;
 		skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
 		__skb_queue_tail(queue, skb);
 	}
@@ -831,7 +831,7 @@ static int __ip_append_data(struct sock *sk,
 	    (rt->dst.dev->features & NETIF_F_UFO) && !rt->dst.header_len) {
 		err = ip_ufo_append_data(sk, queue, getfrag, from, length,
 					 hh_len, fragheaderlen, transhdrlen,
-					 mtu, flags);
+					 maxfraglen, flags);
 		if (err)
 			goto error;
 		return 0;
-- 
1.7.3.1


^ permalink raw reply related

* Re: [PATCH] Constrain UFO fragment sizes to multiples of 8 bytes
From: Maciej Żenczykowski @ 2011-07-20  1:30 UTC (permalink / raw)
  To: Bill Sommerfeld; +Cc: David S. Miller, netdev, Tom Herbert
In-Reply-To: <1311124953-32159-1-git-send-email-wsommerfeld@google.com>

Ack.
This should probably make it into all manner of stable branches.

On Tue, Jul 19, 2011 at 6:22 PM, Bill Sommerfeld <wsommerfeld@google.com> wrote:
> Because the ip fragment offset field counts 8-byte chunks, ip
> fragments other than the last must contain a multiple of 8 bytes of
> payload.  ip_ufo_append_data wasn't respecting this constraint and,
> depending on the MTU and ip option sizes, could create malformed
> non-final fragments.
>
> Google-Bug-Id: 5009328
> Signed-off-by: Bill Sommerfeld <wsommerfeld@google.com>
> ---
>
> Note to reviewers: The first two hunks simply rename the "mtu"
> parameter to "maxfraglen"; the real change is in the third hunk.
> maxfraglen (the size of the largest non-final fragment which fits
> inside mtu) is already computed for the non-UFO fragmentation path.
>
> We saw this problem in a 2.6.34-based kernel with a bond device that
> had UFO enabled (as a result of changeset d9f5950f90292f7c);
> 1742f183fc218798 rewrote netdev_increment_features to no longer force
> on UFO.  The MTU on this bond device was set to 1470; fragmented
> datagrams were mangled, causing the receiver to get errors of the
> form:
>
> [506259.362640] UDP: short packet: From x.x.x.x:nnnn 1471/1469 to y.y.y.y:nnnn
>
>  net/ipv4/ip_output.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
> index be27e60..ccaaa85 100644
> --- a/net/ipv4/ip_output.c
> +++ b/net/ipv4/ip_output.c
> @@ -734,7 +734,7 @@ static inline int ip_ufo_append_data(struct sock *sk,
>                        int getfrag(void *from, char *to, int offset, int len,
>                               int odd, struct sk_buff *skb),
>                        void *from, int length, int hh_len, int fragheaderlen,
> -                       int transhdrlen, int mtu, unsigned int flags)
> +                       int transhdrlen, int maxfraglen, unsigned int flags)
>  {
>        struct sk_buff *skb;
>        int err;
> @@ -767,7 +767,7 @@ static inline int ip_ufo_append_data(struct sock *sk,
>                skb->csum = 0;
>
>                /* specify the length of each IP datagram fragment */
> -               skb_shinfo(skb)->gso_size = mtu - fragheaderlen;
> +               skb_shinfo(skb)->gso_size = maxfraglen - fragheaderlen;
>                skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
>                __skb_queue_tail(queue, skb);
>        }
> @@ -831,7 +831,7 @@ static int __ip_append_data(struct sock *sk,
>            (rt->dst.dev->features & NETIF_F_UFO) && !rt->dst.header_len) {
>                err = ip_ufo_append_data(sk, queue, getfrag, from, length,
>                                         hh_len, fragheaderlen, transhdrlen,
> -                                        mtu, flags);
> +                                        maxfraglen, flags);
>                if (err)
>                        goto error;
>                return 0;
> --
> 1.7.3.1
>
>



-- 
Maciej A. Żenczykowski
Kernel Networking Developer @ Google
1600 Amphitheatre Parkway, Mountain View, CA 94043
tel: +1 (650) 253-0062

^ permalink raw reply

* Re: [PATCH] pktgen: Clone skb to avoid corruption of skbs in ndo_start_xmit methods
From: Neil Horman @ 2011-07-20  1:59 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Jiri Pirko, netdev, Alexey Dobriyan, David S. Miller
In-Reply-To: <1311122593.3113.46.camel@edumazet-laptop>

On Wed, Jul 20, 2011 at 02:43:12AM +0200, Eric Dumazet wrote:
> Le mardi 19 juillet 2011 à 20:19 -0400, Neil Horman a écrit :
> > > 
> > You are correct Eric, this can cause a significant performance regression, but I
> > think that beats causing a panic or other unexpected behavior.  I read your
> > previous threads with others regarding fixing this with vlans, but I don't think
> > its fair to just say 'its fast, but it might cause oopses'. 
> > 
> > And its not sufficient to simply forbid soft drivers to make use of pktgen, its
> > not just a soft driver problem, its systemic.  Any driver which assumes that it
> > has exclusive access to an skb submitted for transmit is at risk from pktgen in
> > its current implementation.  That of course as a subset includes all the soft
> > drivers, but others are also suceptible.  As examples (some of which I noted in
> > the origional post) virtio_net uses the skb->cb to hold vnet header information
> > which will be corrupted on sucessive sends.  bnx2x linearizes skbs under certain
> > circumstances, which means pktgen, if it marshals a fragmented frame will not
> > send a fragmented frame after the first iteration.  The PPP and Slip drivers
> > skb_push the skb to prepend a header to the frame on send, meaning sucessive
> > uses, up until they get an skb_under_panic will get iteratively more malformed
> > frames on the wire as ppp headers get stacked on top of one another.  These are
> > ust a few of the examples I've found.
> > 
> > The long and the short of it in my mind, is that we have a fundamental
> > disconnect between driver asumptions and pktgen.  If its ok to submit shared
> > skbs to drivers, then we need to augment drivers that modify skbs on transmit to
> > clone the skb (likey not an efficient solution), or if its not ok to do so, we
> > need to change pktgen to not behave that way.
> > 
> 
> Its a known problem, please check mail archives. Nobody felt a fix was
> needed.
> 
As I said in my origional note, I said I read the archives, I didn't agree with
the conclusion that a fix was unnecessecary.  I'm sorry if you don't care for
dissenting opinions.

> > > Note : a sysadmin has other ways to make a machine panic or reboot or
> > > halt...
> > Yes, predictable ways, that the sysadmin can see coming based on what they're
> > doing (i.e. no one should be shocked if they dd /dev/random to /dev/kmem and get
> > a hang or panic, or if they issue a sysrq-c, etc).  This case is different.  A
> > sysadmin reasonably expects pktgen to send the frames they configure on the
> > interface they specify.  While its arguably reasonable to forsee that it may not
> > work with soft interfaces, pktgen just won't work with some hardware drivers (as
> > per the examples above).  And it won't always be an oops, it may be occasional
> > random behvaior in the output data, and its highly dependent not just on the use
> > of pktgen, but rather the specific command(s) issued.
> > 
> > 
> > I'm sensitive to the performance impact, but I would much rather see a lower
> > performing pktgen that doesn't randomly crash, and bring the performance back up
> > in a safe, reliable way.  To that end, I've been starting to think about
> > pre-allocating a ring buffer of skbs with a skb->users count biased up to
> > prevent driver freeing.  That way we could detect 'unused skb's' by a user count
> > that was at the bias level.  Thoughts?
> > 
> 
> I dont know. I use pktgen maybe once per week and never got a single
> crash like this. We probably are very few pktgen users in the world, and
> we use it exactly to avoid calling skb_clone() or other expensive per
> xmit setup.
Please re-read my origional post.  Lots of drivers work just fine, some don't.
Some just behave differently.  Its the random results thats broken and I feel
needs fixing.  I get that performance is an issue, and I'm open to other
solutions, I'm not open to just saying 'its fine, mostly'.

> 
> Just remove pktgen from RedHat kernels, if you dont trust sysadmins.
> # CONFIG_PKTGEN is not set
> 
You're twisting my words.  At what point in time did I say I
don't trust sysadmins?  I want to give them a tool that works reliably without
them having to comb through their nic drivers xmit patch to ensure that pktgen
works without crashing or causing other odd behavior.  I don't think thats too
much to ask.
 
> Alternatively, add a check to problematic drivers to _not_ mess skb if
> skb_shared(skb) is true : eventually use skb_share_check()
> 
The former isn't feasible, as many skb modifications are neccesitated by the
nature of the hardware.  The latter is possible, but far less scalable than just
modifying pktgen.

FWIW, I like Ben's solution, adding a flag to drivers noting that they can't
handle multi-skb.  Then we can dynamically enforce a clone in pktgen when needed
(or buffer up additional skbs)

^ permalink raw reply

* Re: [PATCH] pktgen: Clone skb to avoid corruption of skbs in ndo_start_xmit methods
From: Neil Horman @ 2011-07-20  2:07 UTC (permalink / raw)
  To: Ben Greear
  Cc: Eric Dumazet, Jiri Pirko, netdev, Alexey Dobriyan,
	David S. Miller
In-Reply-To: <4E2626E1.6030005@candelatech.com>

On Tue, Jul 19, 2011 at 05:52:49PM -0700, Ben Greear wrote:
> On 07/19/2011 05:43 PM, Eric Dumazet wrote:
> >Le mardi 19 juillet 2011 à 20:19 -0400, Neil Horman a écrit :
> 
> >>I'm sensitive to the performance impact, but I would much rather see a lower
> >>performing pktgen that doesn't randomly crash, and bring the performance back up
> >>in a safe, reliable way.  To that end, I've been starting to think about
> >>pre-allocating a ring buffer of skbs with a skb->users count biased up to
> >>prevent driver freeing.  That way we could detect 'unused skb's' by a user count
> >>that was at the bias level.  Thoughts?
> >>
> >
> >I dont know. I use pktgen maybe once per week and never got a single
> >crash like this. We probably are very few pktgen users in the world, and
> >we use it exactly to avoid calling skb_clone() or other expensive per
> >xmit setup.
> >
> >Just remove pktgen from RedHat kernels, if you dont trust sysadmins.
> ># CONFIG_PKTGEN is not set
> >
> >Alternatively, add a check to problematic drivers to _not_ mess skb if
> >skb_shared(skb) is true : eventually use skb_share_check()
> 
> When the features-flags work gets completed so that we can start adding
> new flags, we could add a CANT_DO_MULTI_SKB flag to drivers with known
> issues and then restrict pktgen config accordingly.
> 
I think this is a good idea.  It lets pktgen dynamically make the clone/share
decision dynamically and only impacts performance for those systems.

> Upstream code already clears skb memory to avoid leaking kernel memory
> contents, so if you take away multi-skb too, pktgen is going to suck
> at what it is supposed to do:  run fast as possible.
> 
I don't want to take away multi-skb, but I do want pktgen to work reliably.  I
think flagging drivers that need unshared skbs is the way to go.

> If you want real fun, use pktgen on a wlan0 device...it will crash
> regardless of whether you use multi-skb or not because of xmit-queue
> number issues :P
> 
I'll try that, thanks :)

Regards
Neil


^ permalink raw reply

* (unknown)
From: 168 @ 2011-07-20  2:16 UTC (permalink / raw)




^ permalink raw reply

* Re: [PATCH] pktgen: Clone skb to avoid corruption of skbs in ndo_start_xmit methods
From: Ben Greear @ 2011-07-20  4:19 UTC (permalink / raw)
  To: Neil Horman
  Cc: Eric Dumazet, Jiri Pirko, netdev, Alexey Dobriyan,
	David S. Miller
In-Reply-To: <20110720020737.GB2692@neilslaptop.think-freely.org>

On 07/19/2011 07:07 PM, Neil Horman wrote:
> On Tue, Jul 19, 2011 at 05:52:49PM -0700, Ben Greear wrote:
>> On 07/19/2011 05:43 PM, Eric Dumazet wrote:
>>> Le mardi 19 juillet 2011 à 20:19 -0400, Neil Horman a écrit :

>> When the features-flags work gets completed so that we can start adding
>> new flags, we could add a CANT_DO_MULTI_SKB flag to drivers with known
>> issues and then restrict pktgen config accordingly.
>>
> I think this is a good idea.  It lets pktgen dynamically make the clone/share
> decision dynamically and only impacts performance for those systems.
>
>> Upstream code already clears skb memory to avoid leaking kernel memory
>> contents, so if you take away multi-skb too, pktgen is going to suck
>> at what it is supposed to do:  run fast as possible.
>>
> I don't want to take away multi-skb, but I do want pktgen to work reliably.  I
> think flagging drivers that need unshared skbs is the way to go.

Lets all cheer on the Intel NIC driver guys and Mr. Miraslaw then!

>> If you want real fun, use pktgen on a wlan0 device...it will crash
>> regardless of whether you use multi-skb or not because of xmit-queue
>> number issues :P
>>
> I'll try that, thanks :)

Here's a probably-white-space damaged, and deemed-unfit-for-kernel patch
that at least works around the problem...my users don't really need pktgen
on wlans, so I didn't put any extra effort into it, but for someone
more motivated..this might be a good starting point :)

I guess it doesn't really crash, but if I recall correctly, it filled
up the queues in a bad way and effectively blocked all wifi traffic
until reboot, or maybe rmmod ath9k.


 From d57130f29843cab30196b11d4476231f245e3f92 Mon Sep 17 00:00:00 2001
From: Ben Greear <greearb@candelatech.com>
Date: Wed, 9 Feb 2011 16:58:42 -0800
Subject: [PATCH 31/38] mac80211: Set up tx-queue-mapping in subif_start_xmit.

Otherwise, ath9k gets confused about which queue to use
and spews a warning like this when driving traffic with
pktgen.

WARNING: at /home/greearb/git/linux.wireless-testing-ct/drivers/net/wireless/ath/ath9k/xmit.c:1748 ath_tx_start+0x4a2/0x662 [ath9k]()
Hardware name: To Be Filled By O.E.M.
Modules linked in: ath5k arc4 ath9k mac80211 ath9k_common ath9k_hw ath cfg80211 nfs lockd bluetooth cryptd aes_i586 aes_generic veth 8021q garp stp l]
Pid: 1729, comm: kpktgend_0 Tainted: G        W   2.6.38-rc4-wl+ #21
Call Trace:
  [<c043091b>] ? warn_slowpath_common+0x65/0x7a
  [<fabe784e>] ? ath_tx_start+0x4a2/0x662 [ath9k]
  [<c043093f>] ? warn_slowpath_null+0xf/0x13
  [<fabe784e>] ? ath_tx_start+0x4a2/0x662 [ath9k]
  [<fabe14d0>] ? ath9k_tx+0x14f/0x183 [ath9k]
  [<fab9026d>] ? __ieee80211_tx+0x10c/0x18c [mac80211]
  [<fab90397>] ? ieee80211_tx+0xaa/0x188 [mac80211]
  [<fab905f3>] ? ieee80211_xmit+0x17e/0x186 [mac80211]
  [<fab8ecc0>] ? ieee80211_skb_resize+0x8e/0xd2 [mac80211]
  [<fab9148b>] ? ieee80211_subif_start_xmit+0x643/0x65c [mac80211]
  [<c0440000>] ? rescuer_thread+0x25/0x1c8
  [<f92cd354>] ? pktgen_thread_worker+0x114c/0x1b44 [pktgen]
  [<fab90e48>] ? ieee80211_subif_start_xmit+0x0/0x65c [mac80211]
  [<c042d612>] ? default_wake_function+0xb/0xd
  [<c04254c7>] ? __wake_up_common+0x34/0x5c
  [<c0443a29>] ? autoremove_wake_function+0x0/0x2f
  [<f92cc208>] ? pktgen_thread_worker+0x0/0x1b44 [pktgen]
  [<c044371a>] ? kthread+0x62/0x67
  [<c04436b8>] ? kthread+0x0/0x67
  [<c04035f6>] ? kernel_thread_helper+0x6/0x10

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
:100644 100644 64e0f75... e8ff199... M	net/mac80211/tx.c
  net/mac80211/tx.c |    2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 64e0f75..e8ff199 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1971,6 +1971,8 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
  	} else
  		memcpy(skb_push(skb, hdrlen), &hdr, hdrlen);

+	skb_set_queue_mapping(skb, ieee80211_select_queue(sdata, skb));
+
  	nh_pos += hdrlen;
  	h_pos += hdrlen;

-- 
1.7.3.4


>
> Regards
> Neil


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

^ permalink raw reply related

* Re: [PATCH] pktgen: Clone skb to avoid corruption of skbs in ndo_start_xmit methods
From: Eric Dumazet @ 2011-07-20  4:24 UTC (permalink / raw)
  To: Neil Horman
  Cc: Ben Greear, Jiri Pirko, netdev, Alexey Dobriyan, David S. Miller
In-Reply-To: <20110720020737.GB2692@neilslaptop.think-freely.org>

Le mardi 19 juillet 2011 à 22:07 -0400, Neil Horman a écrit :
> > 
> I think this is a good idea.  It lets pktgen dynamically make the clone/share
> decision dynamically and only impacts performance for those systems.
> 

Just let pktgen refuse to use clone_skb command for these devices.

At that point, an userland application is going to be faster and more
flexible than pktgen.

So when I said "remove pktgen" from distro it was not a joke.
Maybe its time to admit pktgen has to be removed from kernel sources.

$ wc net/core/pktgen.c
 3788 10881 92771 net/core/pktgen.c

Hmmm, 3788 lines, patched 180 times for a thing that only sends UDP
frames...




^ permalink raw reply

* Re: [BUG] ipv6: all routes share same inetpeer
From: Eric Dumazet @ 2011-07-20  5:29 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20110719.115929.106510307852361614.davem@davemloft.net>

Le mardi 19 juillet 2011 à 11:59 -0700, David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Tue, 19 Jul 2011 20:57:50 +0200
> 
> > Le mardi 19 juillet 2011 à 20:20 +0200, Eric Dumazet a écrit :
> >> Le mardi 19 juillet 2011 à 10:37 -0700, David Miller a écrit :
> >> > From: Eric Dumazet <eric.dumazet@gmail.com>
> >> > Date: Tue, 19 Jul 2011 19:23:49 +0200
> >> > 
> >> > > Maybe you can find the bug before me ?
> >> > 
> >> > I think when we add the route we cow the metrics almost immediately.
> >> > The daddr is, unfortunately, fully prefixed at that point.
> >> 
> >> Yes, we shall provide a second ip6_rt_copy() argument, with the
> >> destination address.
> >> 
> > 
> > Hmm, or maybe just change the dst_copy_metrics(&rt->dst, &ort->dst);
> > call done from ip6_rt_copy(), to avoid doing the COW if not really
> > needed ?
> 
> This is ok if it handles the case where ort's metrics point to
> writable inetpeer memory.

OK but if ort's metrics are writeable we must perform the dst_copy_metrics()
and therefore fill rt6i_dst before ?

My first patch had an issue in rt6_alloc_cow(), line 710, where 
ipv6_addr_equal(&rt->rt6i_dst.addr, daddr) becomes always true.

I guess I can replace it by ipv6_addr_equal(&ort->rt6i_dst.addr, daddr)




^ permalink raw reply

* Re: [PATCH 1/3] stmmac: unify MAC and PHY configuration parameters
From: Giuseppe CAVALLARO @ 2011-07-20  5:33 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, stuart.menefy
In-Reply-To: <20110719.115738.131562530013095617.davem@davemloft.net>

Hello David,

On 7/19/2011 8:57 PM, David Miller wrote:
> From: Giuseppe CAVALLARO <peppe.cavallaro@st.com>
> Date: Tue, 19 Jul 2011 10:38:02 +0200
> 
>> Prior to this change, most PHY configuration parameters were passed
>> into the STMMAC device as a separate PHY device. As well as being
>> unusual, this made it difficult to make changes to the MAC/PHY
>> relationship.
>>
>> This patch moves all the PHY parameters into the MAC configuration
>> structure, mainly as a separate structure. This allows us to completly
>> ignore the MDIO bus attached to a stmmac if desired, and not create
>> the PHY bus. It also allows the stmmac driver to use a different PHY
>> from the one it is connected to, for example a fixed PHY or bit banging
>> PHY.
>>
>> Also derive the stmmac/PHY connection type (MII/RMII etc) from the
>> mode can be passed into <platf>_configure_ethernet.
>> STLinux kernel at git://git.stlinux.com/stm/linux-sh4-2.6.32.y.git
>> provides several examples how to use this new infrastructure (that
>> actually is easier to maintain and clearer).
>>
>> Signed-off-by: Stuart Menefy <stuart.menefy@st.com>
>> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> 
> I find these changes confusing, because I can't see where these
> platform data objects are created that end up being used by
> the stmmac driver.
> 
> I'm concerned about this because if you're changing these data
> structures, you'll need to update also the code that creates
> these platform data objects.

You are right and indeed I wanted to provide some other patches to show
how to use the new infrastructure instead of adding the stlinux git
where our platforms that already use that.

> Finally, this patch needs to update Documentation/networking/stmmac.txt

Agree and sorry to have forgotten that.
I'll update the driver's documentation trying to add a valid example how
to use the new structures (taking as example stm devel).
After that I'll send the patches again.

At any rate, as I also do via email, I'm happy to support all guys that
are starting to use in the driver on several architectures (arm, sh, ppc).

This patch is useful because tidy-up the code and make easier the MDIO
part (also in case we use fixed_link etc.).

Thanks for your feedback.

Regards
Peppe

> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


^ permalink raw reply

* Funds payment board
From: FINAL PAYMENT SETTLEMENT BOARD @ 2011-07-20  5:19 UTC (permalink / raw)



We wish to inform you now that the square peg is now in square hole and
can be accelerated so that your payment can be processed and will be
released to you as soon as you respond to this letter. Also note that from
the record in our file, YOUR APPROVED PAYMENT IS $850,000.00
(EIGHT HUNDRED AND FIFTY THOUSAND DOLLARS).I wrote to know if this is  
your valid email.

Please, let me know if this email is valid.

Email:debtsetlementboard@ozledim.net

Regards

Robin Steven






^ permalink raw reply

* [PATCH net-next-2.6] be2net: request native mode each time the card is reset
From: Sathya Perla @ 2011-07-20  5:52 UTC (permalink / raw)
  To: netdev

Currently be3-native mode is requested only in probe(). It must be requested, each time the card is reset either after an EEH error or after
sleep/hibernation.
Also, the be_cmd_check_native_mode() is better named be_cmd_req_native_mode()

Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/benet/be_cmds.c |    2 +-
 drivers/net/benet/be_cmds.h |    2 +-
 drivers/net/benet/be_main.c |    6 ++++--
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/benet/be_cmds.c b/drivers/net/benet/be_cmds.c
index f520a5c..054fa67 100644
--- a/drivers/net/benet/be_cmds.c
+++ b/drivers/net/benet/be_cmds.c
@@ -2390,7 +2390,7 @@ err:
 }
 
 /* Uses mbox */
-int be_cmd_check_native_mode(struct be_adapter *adapter)
+int be_cmd_req_native_mode(struct be_adapter *adapter)
 {
 	struct be_mcc_wrb *wrb;
 	struct be_cmd_req_set_func_cap *req;
diff --git a/drivers/net/benet/be_cmds.h b/drivers/net/benet/be_cmds.h
index 1151df6..8e4d488 100644
--- a/drivers/net/benet/be_cmds.h
+++ b/drivers/net/benet/be_cmds.h
@@ -1545,7 +1545,7 @@ extern int be_cmd_set_qos(struct be_adapter *adapter, u32 bps, u32 domain);
 extern void be_detect_dump_ue(struct be_adapter *adapter);
 extern int be_cmd_get_die_temperature(struct be_adapter *adapter);
 extern int be_cmd_get_cntl_attributes(struct be_adapter *adapter);
-extern int be_cmd_check_native_mode(struct be_adapter *adapter);
+extern int be_cmd_req_native_mode(struct be_adapter *adapter);
 extern int be_cmd_get_reg_len(struct be_adapter *adapter, u32 *log_size);
 extern void be_cmd_get_regs(struct be_adapter *adapter, u32 buf_len, void *buf);
 
diff --git a/drivers/net/benet/be_main.c b/drivers/net/benet/be_main.c
index ae2d262..c411bb1 100644
--- a/drivers/net/benet/be_main.c
+++ b/drivers/net/benet/be_main.c
@@ -2511,6 +2511,8 @@ static int be_setup(struct be_adapter *adapter)
 	int status;
 	u8 mac[ETH_ALEN];
 
+	be_cmd_req_native_mode(adapter);
+
 	cap_flags = en_flags = BE_IF_FLAGS_UNTAGGED |
 				BE_IF_FLAGS_BROADCAST |
 				BE_IF_FLAGS_MULTICAST;
@@ -2618,6 +2620,8 @@ static int be_clear(struct be_adapter *adapter)
 
 	be_cmd_if_destroy(adapter, adapter->if_handle,  0);
 
+	adapter->be3_native = 0;
+
 	/* tell fw we're done with firing cmds */
 	be_cmd_fw_clean(adapter);
 	return 0;
@@ -3215,8 +3219,6 @@ static int be_get_config(struct be_adapter *adapter)
 	if (status)
 		return status;
 
-	be_cmd_check_native_mode(adapter);
-
 	if ((num_vfs && adapter->sriov_enabled) ||
 		(adapter->function_mode & 0x400) ||
 		lancer_chip(adapter) || !be_physfn(adapter)) {
-- 
1.7.4


^ permalink raw reply related

* Re: [BUG] ipv6: all routes share same inetpeer
From: Eric Dumazet @ 2011-07-20  6:18 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <1311139774.3113.86.camel@edumazet-laptop>

Le mercredi 20 juillet 2011 à 07:29 +0200, Eric Dumazet a écrit :

> My first patch had an issue in rt6_alloc_cow(), line 710, where 
> ipv6_addr_equal(&rt->rt6i_dst.addr, daddr) becomes always true.
> 
> I guess I can replace it by ipv6_addr_equal(&ort->rt6i_dst.addr, daddr)
> 
> 

Here the combo patch I tested :

I also had to solve the icmp6_dst_alloc() problem
[it uses dst_metric_set(&rt->dst, RTAX_HOPLIMIT, 255);]

Note : this is based on net-2.6, but I really tested it on net-next-2.6
(with the frag ident patch applied too)

[PATCH] ipv6: unshare inetpeers

We currently cow metrics a bit too soon in IPv6 case : All routes are
tied to a single inetpeer entry.

Change ip6_rt_copy() to get destination address as second argument, so
that we fill rt6i_dst before the dst_copy_metrics() call.

icmp6_dst_alloc() must set rt6i_dst before calling dst_metric_set(), or
else the cow is done while rt6i_dst is still NULL.

If orig route points to readonly metrics, we can share the pointer
instead of performing the memory allocation and copy.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/ipv6/route.c |   33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 0ef1f08..5b5a32d 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -72,7 +72,8 @@
 #define RT6_TRACE(x...) do { ; } while (0)
 #endif
 
-static struct rt6_info * ip6_rt_copy(struct rt6_info *ort);
+static struct rt6_info *ip6_rt_copy(const struct rt6_info *ort,
+				    const struct in6_addr *dest);
 static struct dst_entry	*ip6_dst_check(struct dst_entry *dst, u32 cookie);
 static unsigned int	 ip6_default_advmss(const struct dst_entry *dst);
 static unsigned int	 ip6_default_mtu(const struct dst_entry *dst);
@@ -683,7 +684,8 @@ int ip6_ins_rt(struct rt6_info *rt)
 	return __ip6_ins_rt(rt, &info);
 }
 
-static struct rt6_info *rt6_alloc_cow(struct rt6_info *ort, const struct in6_addr *daddr,
+static struct rt6_info *rt6_alloc_cow(const struct rt6_info *ort,
+				      const struct in6_addr *daddr,
 				      const struct in6_addr *saddr)
 {
 	struct rt6_info *rt;
@@ -692,7 +694,7 @@ static struct rt6_info *rt6_alloc_cow(struct rt6_info *ort, const struct in6_add
 	 *	Clone the route.
 	 */
 
-	rt = ip6_rt_copy(ort);
+	rt = ip6_rt_copy(ort, daddr);
 
 	if (rt) {
 		struct neighbour *neigh;
@@ -700,12 +702,11 @@ static struct rt6_info *rt6_alloc_cow(struct rt6_info *ort, const struct in6_add
 
 		if (!(rt->rt6i_flags&RTF_GATEWAY)) {
 			if (rt->rt6i_dst.plen != 128 &&
-			    ipv6_addr_equal(&rt->rt6i_dst.addr, daddr))
+			    ipv6_addr_equal(&ort->rt6i_dst.addr, daddr))
 				rt->rt6i_flags |= RTF_ANYCAST;
 			ipv6_addr_copy(&rt->rt6i_gateway, daddr);
 		}
 
-		ipv6_addr_copy(&rt->rt6i_dst.addr, daddr);
 		rt->rt6i_dst.plen = 128;
 		rt->rt6i_flags |= RTF_CACHE;
 		rt->dst.flags |= DST_HOST;
@@ -752,11 +753,12 @@ static struct rt6_info *rt6_alloc_cow(struct rt6_info *ort, const struct in6_add
 	return rt;
 }
 
-static struct rt6_info *rt6_alloc_clone(struct rt6_info *ort, const struct in6_addr *daddr)
+static struct rt6_info *rt6_alloc_clone(struct rt6_info *ort,
+					const struct in6_addr *daddr)
 {
-	struct rt6_info *rt = ip6_rt_copy(ort);
+	struct rt6_info *rt = ip6_rt_copy(ort, daddr);
+
 	if (rt) {
-		ipv6_addr_copy(&rt->rt6i_dst.addr, daddr);
 		rt->rt6i_dst.plen = 128;
 		rt->rt6i_flags |= RTF_CACHE;
 		rt->dst.flags |= DST_HOST;
@@ -900,7 +902,10 @@ struct dst_entry *ip6_blackhole_route(struct net *net, struct dst_entry *dst_ori
 		new->input = dst_discard;
 		new->output = dst_discard;
 
-		dst_copy_metrics(new, &ort->dst);
+		if (dst_metrics_read_only(&ort->dst))
+			new->_metrics = ort->dst._metrics;
+		else
+			dst_copy_metrics(new, &ort->dst);
 		rt->rt6i_idev = ort->rt6i_idev;
 		if (rt->rt6i_idev)
 			in6_dev_hold(rt->rt6i_idev);
@@ -1060,6 +1065,7 @@ struct dst_entry *icmp6_dst_alloc(struct net_device *dev,
 	rt->rt6i_idev     = idev;
 	rt->rt6i_nexthop  = neigh;
 	atomic_set(&rt->dst.__refcnt, 1);
+	ipv6_addr_copy(&rt->rt6i_dst.addr, addr);
 	dst_metric_set(&rt->dst, RTAX_HOPLIMIT, 255);
 	rt->dst.output  = ip6_output;
 
@@ -1577,7 +1583,7 @@ void rt6_redirect(const struct in6_addr *dest, const struct in6_addr *src,
 	if (neigh == rt->dst.neighbour)
 		goto out;
 
-	nrt = ip6_rt_copy(rt);
+	nrt = ip6_rt_copy(rt, dest);
 	if (nrt == NULL)
 		goto out;
 
@@ -1585,7 +1591,6 @@ void rt6_redirect(const struct in6_addr *dest, const struct in6_addr *src,
 	if (on_link)
 		nrt->rt6i_flags &= ~RTF_GATEWAY;
 
-	ipv6_addr_copy(&nrt->rt6i_dst.addr, dest);
 	nrt->rt6i_dst.plen = 128;
 	nrt->dst.flags |= DST_HOST;
 
@@ -1723,7 +1728,8 @@ void rt6_pmtu_discovery(const struct in6_addr *daddr, const struct in6_addr *sad
  *	Misc support functions
  */
 
-static struct rt6_info * ip6_rt_copy(struct rt6_info *ort)
+static struct rt6_info *ip6_rt_copy(const struct rt6_info *ort,
+				    const struct in6_addr *dest)
 {
 	struct net *net = dev_net(ort->rt6i_dev);
 	struct rt6_info *rt = ip6_dst_alloc(&net->ipv6.ip6_dst_ops,
@@ -1733,6 +1739,8 @@ static struct rt6_info * ip6_rt_copy(struct rt6_info *ort)
 		rt->dst.input = ort->dst.input;
 		rt->dst.output = ort->dst.output;
 
+		ipv6_addr_copy(&rt->rt6i_dst.addr, dest);
+		rt->rt6i_dst.plen = ort->rt6i_dst.plen;
 		dst_copy_metrics(&rt->dst, &ort->dst);
 		rt->dst.error = ort->dst.error;
 		rt->rt6i_idev = ort->rt6i_idev;
@@ -1745,7 +1753,6 @@ static struct rt6_info * ip6_rt_copy(struct rt6_info *ort)
 		rt->rt6i_flags = ort->rt6i_flags & ~RTF_EXPIRES;
 		rt->rt6i_metric = 0;
 
-		memcpy(&rt->rt6i_dst, &ort->rt6i_dst, sizeof(struct rt6key));
 #ifdef CONFIG_IPV6_SUBTREES
 		memcpy(&rt->rt6i_src, &ort->rt6i_src, sizeof(struct rt6key));
 #endif
	


^ permalink raw reply related

* Re: [PATCH net-next-2.6] ipv6: make fragment identifications less predictable
From: Eric Dumazet @ 2011-07-20  6:50 UTC (permalink / raw)
  To: Matt Mackall; +Cc: Fernando Gont, David Miller, security, Eugene Teo, netdev
In-Reply-To: <1311109019.14555.11.camel@calx>

Le mardi 19 juillet 2011 à 15:56 -0500, Matt Mackall a écrit :
> On Tue, 2011-07-19 at 22:47 +0200, Eric Dumazet wrote:
> > IPv6 fragment identification generation is way beyond what we use for
> > IPv4 : It uses a single generator. Its not scalable and allows DOS
> > attacks.
> > 
> > Now inetpeer is IPv6 aware, we can use it to provide a more secure and
> > scalable frag ident generator (per destination, instead of system wide)
> 
> This code really needs to get moved out of random.c and into net/. Other
> than that, looks fine to me.

Sure. Can we agree to make this cleanup later ?

I added secure_ipv6_id() right after secure_ip_id() because it sounded
the right place ;)




^ permalink raw reply

* BlackBerry 2011 Email Promotion!
From: BlackBerry Online Promo @ 2011-07-20  4:27 UTC (permalink / raw)


Send Your Detail To Claim Your $350,000.00
Awarded To You By The BlackBerry
Online Promo in our British Mercedes Benz
Promo Online Promo Game.

Send Your:-
Name:__________
Address: _________
Tel: _______________

Thanks,
Coordinator,
The BlackBerry Promo Team,
Mrs. Eleanor Miller.




^ permalink raw reply

* Re: [PATCH] net: ibm_newemac: Don't start autonegotiation when disabled in BMCR (genmii)
From: Stefan Roese @ 2011-07-20  7:18 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, netdev
In-Reply-To: <1311078570.25044.421.camel@pasglop>

On Tuesday 19 July 2011 14:29:30 Benjamin Herrenschmidt wrote:
> > I feel that this BMCR_ANENABLE bit should be evaluated, but I have no
> > strong preference here. If you prefer that this should be handled via a
> > new dt property (phy-aneg = "disabled" ?), I can implement it this way.
> > Just let me know.
> 
> Don't we already have some bindings for PHY with a fixed setting ? I
> don't remember off hand, we need to dbl check.

The only related PHY property I found is "fixed-link" used in fs_enet-main.c. 
None in the emac driver. Here the description for "fixed-link":

Documentation/devicetree/bindings/net/fsl-tsec-phy.txt:

  - fixed-link : <a b c d e> where a is emulated phy id - choose any,
    but unique to the all specified fixed-links, b is duplex - 0 half,
    1 full, c is link speed - d#10/d#100/d#1000, d is pause - 0 no
    pause, 1 pause, e is asym_pause - 0 no asym_pause, 1 asym_pause.

But what I really want to achieve, is to skip auto-negotiation (use the 
strapped configuration). And not to define this fixed configuration (again) in 
the device-tree. So I would prefer something like phy-aneg = "disabled".

What do you think?

Thanks,
Stefan

^ permalink raw reply

* [patch] skbuff: use kfree_skb() instead of kfree()
From: Dan Carpenter @ 2011-07-20  7:23 UTC (permalink / raw)
  To: Shirley Ma
  Cc: David S. Miller, Eric Dumazet, Michał Mirosław,
	open list:NETWORKING [GENERAL], kernel-janitors

"n" was allocated with alloc_skb() so we should free it with
kfree_skb() instead of regular kfree().

Signed-off-by: Dan Carpenter <error27@gmail.com>

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index d220119..cc0c6f0 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -799,7 +799,7 @@ struct sk_buff *pskb_copy(struct sk_buff *skb, gfp_t gfp_mask)
 
 		if (skb_shinfo(skb)->tx_flags & SKBTX_DEV_ZEROCOPY) {
 			if (skb_copy_ubufs(skb, gfp_mask)) {
-				kfree(n);
+				kfree_skb(n);
 				goto out;
 			}
 			skb_shinfo(skb)->tx_flags &= ~SKBTX_DEV_ZEROCOPY;

^ permalink raw reply related

* Re: [patch] skbuff: use kfree_skb() instead of kfree()
From: Dan Carpenter @ 2011-07-20  7:25 UTC (permalink / raw)
  To: Shirley Ma
  Cc: David S. Miller, Eric Dumazet, Michał Mirosław,
	open list:NETWORKING [GENERAL], kernel-janitors
In-Reply-To: <20110720072343.GF6445@shale.localdomain>

Crap.  Sorry, I shouldn't have sent that.  We shouldn't return the
freed "n" here.  I'll send a v2 shortly.

regards,
dan carpenter


^ permalink raw reply

* Re: [patch] skbuff: use kfree_skb() instead of kfree()
From: Eric Dumazet @ 2011-07-20  7:42 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Shirley Ma, David S. Miller, Michał Mirosław,
	open list:NETWORKING [GENERAL], kernel-janitors
In-Reply-To: <20110720072549.GG6445@shale.localdomain>

Le mercredi 20 juillet 2011 à 10:25 +0300, Dan Carpenter a écrit :
> Crap.  Sorry, I shouldn't have sent that.  We shouldn't return the
> freed "n" here.  I'll send a v2 shortly.

Also, dont forget to say its a patch for net-next-2.6

The recommended way is to use 

[PATCH net-next-2.6] ...




^ permalink raw reply

* Re: [patch] skbuff: use kfree_skb() instead of kfree()
From: Dan Carpenter @ 2011-07-20  8:01 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Shirley Ma, David S. Miller, Michał Mirosław,
	open list:NETWORKING [GENERAL], kernel-janitors
In-Reply-To: <1311147723.3113.116.camel@edumazet-laptop>

On Wed, Jul 20, 2011 at 09:42:03AM +0200, Eric Dumazet wrote:
> Le mercredi 20 juillet 2011 à 10:25 +0300, Dan Carpenter a écrit :
> > Crap.  Sorry, I shouldn't have sent that.  We shouldn't return the
> > freed "n" here.  I'll send a v2 shortly.
> 
> Also, dont forget to say its a patch for net-next-2.6

If you're using linux-next, is there a way to tell which tree a
patch came from?  Obviously in this case it's core networking, but
in other cases how does that work?

regards,
dan carpenter


^ permalink raw reply

* Re: [patch] skbuff: use kfree_skb() instead of kfree()
From: Eric Dumazet @ 2011-07-20  8:21 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Shirley Ma, David S. Miller, Michał Mirosław,
	open list:NETWORKING [GENERAL], kernel-janitors
In-Reply-To: <20110720080109.GH6445@shale.localdomain>

Le mercredi 20 juillet 2011 à 11:01 +0300, Dan Carpenter a écrit :
> On Wed, Jul 20, 2011 at 09:42:03AM +0200, Eric Dumazet wrote:
> > Le mercredi 20 juillet 2011 à 10:25 +0300, Dan Carpenter a écrit :
> > > Crap.  Sorry, I shouldn't have sent that.  We shouldn't return the
> > > freed "n" here.  I'll send a v2 shortly.
> > 
> > Also, dont forget to say its a patch for net-next-2.6
> 
> If you're using linux-next, is there a way to tell which tree a
> patch came from?  Obviously in this case it's core networking, but
> in other cases how does that work?

In this particular case, David will know for sure since patch is very
recent, but I wanted to make a general advice.

Keep in mind David has to review dozens of patches _per_ day, so netdev
related patches need some extra cooperation from submitters to help the
maintainer.

This extra cooperation means to test the patch on either net-next-2.6 or
net-2.6 tree ;)




^ permalink raw reply

* Re: [PATCH net-next-2.6] ipv6: make fragment identifications less predictable
From: Eric Dumazet @ 2011-07-20  8:25 UTC (permalink / raw)
  To: Fernando Gont, David Miller; +Cc: security, Eugene Teo, netdev, Matt Mackall
In-Reply-To: <1311108423.3113.24.camel@edumazet-laptop>

Le mardi 19 juillet 2011 à 22:47 +0200, Eric Dumazet a écrit :
> IPv6 fragment identification generation is way beyond what we use for
> IPv4 : It uses a single generator. Its not scalable and allows DOS
> attacks.
> 
> Now inetpeer is IPv6 aware, we can use it to provide a more secure and
> scalable frag ident generator (per destination, instead of system wide)
> 
> This patch :
> 1) defines a new secure_ipv6_id() helper
> 2) extends inet_getid() to provide 32bit results
> 3) extends ipv6_select_ident() with a new dest parameter
> 
> Reported-by: Fernando Gont <fernando@gont.com.ar>
> CC: Matt Mackall <mpm@selenic.com>
> CC: Eugene Teo <eugeneteo@kernel.sg>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---


Please hold on, I'll make a different patch series to ease stable teams
job. It appears inetpeer & ipv6 are really not an option for old
kernels.

Common patch for all kernels :
1) Fix the problem without inetpeer help
--- 
Patches for next kernels
2) random split as suggested by Matt Mackal
3) Use inetpeer cache to scale identification generation



^ 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