Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v4 1/3] make jump_labels wait while updates are in place
From: Jason Baron @ 2012-04-27 13:53 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Glauber Costa, cgroups, netdev, linux-kernel, Li Zefan, Tejun Heo,
	kamezawa.hiroyu, linux-mm, devel, Johannes Weiner, Michal Hocko,
	Ingo Molnar
In-Reply-To: <20120427004305.GC23877@home.goodmis.org>

On Thu, Apr 26, 2012 at 08:43:06PM -0400, Steven Rostedt wrote:
> On Thu, Apr 26, 2012 at 07:51:05PM -0300, Glauber Costa wrote:
> > In mem cgroup, we need to guarantee that two concurrent updates
> > of the jump_label interface wait for each other. IOW, we can't have
> > other updates returning while the first one is still patching the
> > kernel around, otherwise we'll race.
> 
> But it shouldn't. The code as is should prevent that.
> 
> > 
> > I believe this is something that can fit well in the static branch
> > API, without noticeable disadvantages:
> > 
> > * in the common case, it will be a quite simple lock/unlock operation
> > * Every context that calls static_branch_slow* already expects to be
> >   in sleeping context because it will mutex_lock the unlikely case.
> > * static_key_slow_inc is not expected to be called in any fast path,
> >   otherwise it would be expected to have quite a different name. Therefore
> >   the mutex + atomic combination instead of just an atomic should not kill
> >   us.
> > 
> > Signed-off-by: Glauber Costa <glommer@parallels.com>
> > CC: Tejun Heo <tj@kernel.org>
> > CC: Li Zefan <lizefan@huawei.com>
> > CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > CC: Johannes Weiner <hannes@cmpxchg.org>
> > CC: Michal Hocko <mhocko@suse.cz>
> > CC: Ingo Molnar <mingo@elte.hu>
> > CC: Jason Baron <jbaron@redhat.com>
> > ---
> >  kernel/jump_label.c |   21 +++++++++++----------
> >  1 files changed, 11 insertions(+), 10 deletions(-)
> > 
> > diff --git a/kernel/jump_label.c b/kernel/jump_label.c
> > index 4304919..5d09cb4 100644
> > --- a/kernel/jump_label.c
> > +++ b/kernel/jump_label.c
> > @@ -57,17 +57,16 @@ static void jump_label_update(struct static_key *key, int enable);
> >  
> >  void static_key_slow_inc(struct static_key *key)
> >  {
> > +	jump_label_lock();
> >  	if (atomic_inc_not_zero(&key->enabled))
> > -		return;
> 
> If key->enabled is not zero, there's nothing to be done. As the jump
> label has already been enabled. Note, the key->enabled doesn't get set
> until after the jump label is updated. Thus, if two tasks were to come
> in, they both would be locked on the jump_label_lock().
> 

Right, for x86 which uses stop_machine currently, we guarantee that all
cpus are going to see the updated code, before the inc of key->enabled.
However, other arches (sparc, mips, powerpc, for example), seem to be
using much lighter weight updates, which I hope are ok :)

Thanks,

-Jason

--
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 v4 1/3] make jump_labels wait while updates are in place
From: Steven Rostedt @ 2012-04-27 14:07 UTC (permalink / raw)
  To: Jason Baron
  Cc: Glauber Costa, cgroups, netdev, linux-kernel, Li Zefan, Tejun Heo,
	kamezawa.hiroyu, linux-mm, devel, Johannes Weiner, Michal Hocko,
	Ingo Molnar
In-Reply-To: <20120427135320.GA13762@redhat.com>

On Fri, 2012-04-27 at 09:53 -0400, Jason Baron wrote:

> Right, for x86 which uses stop_machine currently, we guarantee that all
> cpus are going to see the updated code, before the inc of key->enabled.
> However, other arches (sparc, mips, powerpc, for example), seem to be
> using much lighter weight updates, which I hope are ok :)

And x86 will soon be removing stop_machine() from its path too. But all
archs should perform some kind of memory sync after patching code. Thus
the update should be treated as if a memory barrier was added after it,
and before the inc.

-- Steve


--
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 1/4 (V3)] net: ethtool: add the EEE support
From: Giuseppe CAVALLARO @ 2012-04-27 14:11 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: netdev, rayagond, davem
In-Reply-To: <1335460660.2712.15.camel@bwh-desktop.uk.solarflarecom.com>

On 4/26/2012 7:17 PM, Ben Hutchings wrote:
> On Thu, 2012-04-26 at 09:48 +0200, Giuseppe CAVALLARO wrote:
>> Hello Ben
>>
>> On 4/19/2012 5:30 PM, Ben Hutchings wrote:
>> [snip]
>>>> I'm changing the code for getting/setting the EEE capability and trying
>>>> to follow your suggestions.
>>>>
>>>> The "get" will show the following things; this is a bit different of the
>>>> points "a" "b" and "c" we had discussed. Maybe, this could also be a
>>>> more complete (*) .
>>>> The ethtool (see output below as example) could report the phy
>>>> (supported/advertised/lp_advertised) and mac eee capabilities separately.
>>>
>>> Sounds reasonable.
>>>
>>>> The "set" will be useful for some eth devices (like the stmmac) that can
>>>> stop/enable internally the eee capability (at mac level).
>>>
>>> I don't know much about EEE, but shouldn't the driver take care of
>>> configuring the MAC for this whenever the PHY is set to advertise EEE
>>> capability?
>>
>> Yes indeed this can be done at driver level. So could I definitely
>> remove it from ethtool? What do you suggest?
>>
>> In case of the stmmac I could add a specific driver option via sys to
>> enable/disable the eee and set timer.
> 
> Generally, ethtool doesn't distinguish MAC and PHY settings because they
> have to be configured consistently for the device to do anything useful.
> If there is some good use for enabling EEE in the MAC and not the PHY,
> or vice versa, then this should be exposed in the ethtool interface.
> But if not then I don't believe it needs to be in either an ethtool or a
> driver-specific interface.

Thanks Ben for this clarification: in case of the stmmac the option is
useful to stop a timer to enter in lpi state for the tx.
So it's worth having that and from ethtool.

> 
>>>> [snip]
>>>>
>>>> 	Current message level: 0x0000003f (63)
>>>> 			       drv probe link timer ifdown ifup
>>>> 	Link detected: yes
>>>> 	Energy-Efficient Ethernet:           -------------------------
>>>> 		MAC supports: yes            |-> related to MAC side  |
>>>> 		phy supports modes: ...      |-> from MMD 3.20        |
>>>> 		phy advertising modes: ...   |-> from MMD 7.60        |
>>>> 		LP advertising modes: ...    |-> from MMD 7.61        |
>>>> 					     --------------------------
>>>>                                                     (*)
>>>> PS. The "..." above means that we can actually dump: 100BASE-TX EEE etc
>>>> for each advertising modes and also for phy support (reg 3.20).
>>>
>>> Yes, that's the sort of information I would expect to see (but try to be
>>> consistent with the wording used for AN).:
>>
>> e.g. SUPPORTED_100baseT_EEE ... ADVERTISED_<...>
> 
> I meant the wording used in the ethtool output: 'supported',
> 'advertised', 'link partner advertised' rather than 'phy supports',
> 'phy advertising', 'LP advertising'.

ok :-)

> 
>>> The EEE advertising mask presumably should be changeable similarly to
>>> the AN advertising mask ('ethtool -s <devname> eeeadv <mask>').  But I
>>> don't know how the two advertising masks interact.  Is one supposed to
>>> be a subset of the other?  Currently ethtool automatically changes the
>>> AN advertising mask in response to a speed/duplex change; should it also
>>> try to change the EEE advertising mask?
>>
>> I've just verified the IEEE (Table 45–150a—EEE advertisement register
>> (Register 7.60) bit definitions) and sorry for my delay in reply but I
>> was in trouble because looking at the registers for the phy (I am using)
>> the reg 7.60 was in RO and I couldn't understand how to set the mask.
>> I confirm that the Adv reg from the std is R/W and the mask as you
>> suggest could be set according to the speed.
>> The EEE should work on duplex mode only.
>>
>> I wonder so if if the final patch I should have no new option for the
>> ethtool command and EEE info are directly passed from the kernel like
>> speed and duplex when call get_settings.
> 
> Are you suggesting to define EEE mode flags in the existing supported,
> advertising and lp_advertising masks?

Yes but I was wrong, I can use the existing flags.

Regards
Peppe

> 
> Ben.
> 

^ permalink raw reply

* Re: [PATCH v4 1/3] make jump_labels wait while updates are in place
From: Glauber Costa @ 2012-04-27 14:59 UTC (permalink / raw)
  To: Jason Baron
  Cc: Steven Rostedt, cgroups, netdev, linux-kernel, Li Zefan,
	Tejun Heo, kamezawa.hiroyu, linux-mm, devel, Johannes Weiner,
	Michal Hocko, Ingo Molnar
In-Reply-To: <20120427135320.GA13762@redhat.com>

On 04/27/2012 10:53 AM, Jason Baron wrote:
> On Thu, Apr 26, 2012 at 08:43:06PM -0400, Steven Rostedt wrote:
>> On Thu, Apr 26, 2012 at 07:51:05PM -0300, Glauber Costa wrote:
>>> In mem cgroup, we need to guarantee that two concurrent updates
>>> of the jump_label interface wait for each other. IOW, we can't have
>>> other updates returning while the first one is still patching the
>>> kernel around, otherwise we'll race.
>>
>> But it shouldn't. The code as is should prevent that.
>>
>>>
>>> I believe this is something that can fit well in the static branch
>>> API, without noticeable disadvantages:
>>>
>>> * in the common case, it will be a quite simple lock/unlock operation
>>> * Every context that calls static_branch_slow* already expects to be
>>>    in sleeping context because it will mutex_lock the unlikely case.
>>> * static_key_slow_inc is not expected to be called in any fast path,
>>>    otherwise it would be expected to have quite a different name. Therefore
>>>    the mutex + atomic combination instead of just an atomic should not kill
>>>    us.
>>>
>>> Signed-off-by: Glauber Costa<glommer@parallels.com>
>>> CC: Tejun Heo<tj@kernel.org>
>>> CC: Li Zefan<lizefan@huawei.com>
>>> CC: Kamezawa Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com>
>>> CC: Johannes Weiner<hannes@cmpxchg.org>
>>> CC: Michal Hocko<mhocko@suse.cz>
>>> CC: Ingo Molnar<mingo@elte.hu>
>>> CC: Jason Baron<jbaron@redhat.com>
>>> ---
>>>   kernel/jump_label.c |   21 +++++++++++----------
>>>   1 files changed, 11 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/kernel/jump_label.c b/kernel/jump_label.c
>>> index 4304919..5d09cb4 100644
>>> --- a/kernel/jump_label.c
>>> +++ b/kernel/jump_label.c
>>> @@ -57,17 +57,16 @@ static void jump_label_update(struct static_key *key, int enable);
>>>
>>>   void static_key_slow_inc(struct static_key *key)
>>>   {
>>> +	jump_label_lock();
>>>   	if (atomic_inc_not_zero(&key->enabled))
>>> -		return;
>>
>> If key->enabled is not zero, there's nothing to be done. As the jump
>> label has already been enabled. Note, the key->enabled doesn't get set
>> until after the jump label is updated. Thus, if two tasks were to come
>> in, they both would be locked on the jump_label_lock().
>>
>

Okay, we seem to have been tricked by the usage of atomic while 
analyzing this. The fact that the atomic update happens after the code 
is patched seems enough to guarantee what we need, now that I read it 
again (and it seems so obvious =p )

--
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

* [PATCH] tcp: clean up use of jiffies in tcp_rcv_rtt_measure()
From: Neal Cardwell @ 2012-04-27 15:29 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, Eric Dumazet, Nandita Dukkipati, Yuchung Cheng,
	ilpo.jarvinen, Tom Herbert, Neal Cardwell

Clean up a reference to jiffies in tcp_rcv_rtt_measure() that should
instead reference tcp_time_stamp. Since the result of the subtraction
is passed into a function taking u32, this should not change any
behavior (and indeed the generated assembly does not change on
x86_64). However, it seems worth cleaning this up for consistency and
clarity (and perhaps to avoid bugs if this is copied and pasted
somewhere else).

Signed-off-by: Neal Cardwell <ncardwell@google.com>
---
 net/ipv4/tcp_input.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 3ff36406..2a702e3 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -495,7 +495,7 @@ static inline void tcp_rcv_rtt_measure(struct tcp_sock *tp)
 		goto new_measure;
 	if (before(tp->rcv_nxt, tp->rcv_rtt_est.seq))
 		return;
-	tcp_rcv_rtt_update(tp, jiffies - tp->rcv_rtt_est.time, 1);
+	tcp_rcv_rtt_update(tp, tcp_time_stamp - tp->rcv_rtt_est.time, 1);
 
 new_measure:
 	tp->rcv_rtt_est.seq = tp->rcv_nxt + tp->rcv_wnd;
-- 
1.7.7.3

^ permalink raw reply related

* Re: [PATCH] tcp: clean up use of jiffies in tcp_rcv_rtt_measure()
From: Eric Dumazet @ 2012-04-27 15:56 UTC (permalink / raw)
  To: Neal Cardwell
  Cc: David Miller, netdev, Eric Dumazet, Nandita Dukkipati,
	Yuchung Cheng, ilpo.jarvinen, Tom Herbert
In-Reply-To: <1335540577-32554-1-git-send-email-ncardwell@google.com>

On Fri, 2012-04-27 at 11:29 -0400, Neal Cardwell wrote:
> Clean up a reference to jiffies in tcp_rcv_rtt_measure() that should
> instead reference tcp_time_stamp. Since the result of the subtraction
> is passed into a function taking u32, this should not change any
> behavior (and indeed the generated assembly does not change on
> x86_64). However, it seems worth cleaning this up for consistency and
> clarity (and perhaps to avoid bugs if this is copied and pasted
> somewhere else).
> 
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
> ---

Acked-by: Eric Dumazet <edumazet@google.com>

^ permalink raw reply

* Re: [PATCH 2/4 net-next] tg3: provide frags as skb head
From: Ben Hutchings @ 2012-04-27 16:33 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, netdev, Ilpo Järvinen, Tom Herbert,
	Neal Cardwell, Matt Carlson, Jeff Kirsher, Michael Chan,
	Herbert Xu, Maciej Żenczykowski
In-Reply-To: <1335522889.2775.231.camel@edumazet-glaptop>

On Fri, 2012-04-27 at 12:34 +0200, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> This patch converts tg3 driver, one of our reference drivers, to use new
> build_skb() api in frag mode.
> 
> Instead of using kmalloc() to allocate the memory block that will be
> used by build_skb() as skb->head, we use a page fragment.
> 
> This is a followup of patch "net: allow skb->head to be a page fragment"
> 
> This allows GRO, TCP coalescing, and splice() to be more efficient.
> 
> Incidentally, this also removes SLUB slow path contention in kfree()
[...]
> @@ -5617,17 +5626,48 @@ static void tg3_tx(struct tg3_napi *tnapi)
>  	}
>  }
>  
> +static void *tg3_frag_alloc(struct tg3_rx_prodring_set *tpr)
> +{
> +	void *data;
> +
> +	if (tpr->rx_page_size < TG3_FRAGSIZE) {
> +		struct page *page = alloc_page(GFP_ATOMIC);
> +
> +		if (!page)
> +			return NULL;
> +		atomic_add((PAGE_SIZE / TG3_FRAGSIZE) - 1, &page->_count); 
> +		tpr->rx_page_addr = page_address(page);
[...]

page_address() may return NULL if CONFIG_HIGHMEM is enabled.  And that's
going to be a general problem with this scheme on 32-bit machines.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: [PATCH] tcp: clean up use of jiffies in tcp_rcv_rtt_measure()
From: David Miller @ 2012-04-27 16:35 UTC (permalink / raw)
  To: ncardwell; +Cc: netdev, edumazet, nanditad, ycheng, ilpo.jarvinen, therbert
In-Reply-To: <1335540577-32554-1-git-send-email-ncardwell@google.com>

From: Neal Cardwell <ncardwell@google.com>
Date: Fri, 27 Apr 2012 11:29:37 -0400

> Clean up a reference to jiffies in tcp_rcv_rtt_measure() that should
> instead reference tcp_time_stamp. Since the result of the subtraction
> is passed into a function taking u32, this should not change any
> behavior (and indeed the generated assembly does not change on
> x86_64). However, it seems worth cleaning this up for consistency and
> clarity (and perhaps to avoid bugs if this is copied and pasted
> somewhere else).
> 
> Signed-off-by: Neal Cardwell <ncardwell@google.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH 2/4 net-next] tg3: provide frags as skb head
From: David Miller @ 2012-04-27 16:38 UTC (permalink / raw)
  To: bhutchings
  Cc: eric.dumazet, netdev, ilpo.jarvinen, therbert, ncardwell,
	mcarlson, jeffrey.t.kirsher, mchan, herbert, maze
In-Reply-To: <1335544386.2618.50.camel@bwh-desktop.uk.solarflarecom.com>

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Fri, 27 Apr 2012 17:33:06 +0100

> page_address() may return NULL if CONFIG_HIGHMEM is enabled.  And that's
> going to be a general problem with this scheme on 32-bit machines.

It can only be NULL for highmem pages, which he is not going to
get from the kind of page allocation calls he is using.

^ permalink raw reply

* Re: [PATCH 2/4 net-next] tg3: provide frags as skb head
From: Eric Dumazet @ 2012-04-27 16:42 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: David Miller, netdev, Ilpo Järvinen, Tom Herbert,
	Neal Cardwell, Matt Carlson, Jeff Kirsher, Michael Chan,
	Herbert Xu, Maciej Żenczykowski
In-Reply-To: <1335544386.2618.50.camel@bwh-desktop.uk.solarflarecom.com>

On Fri, 2012-04-27 at 17:33 +0100, Ben Hutchings wrote:

> page_address() may return NULL if CONFIG_HIGHMEM is enabled.  And that's
> going to be a general problem with this scheme on 32-bit machines.

kmalloc(2048, GFP_ATOMIC) and alloc_page(GFP_ATOMIC) uses the same pool
of LOWMEM pages, so there is no difference here. page_address() is
guaranteed to work.

Thanks

^ permalink raw reply

* Re: [PATCH 2/4 net-next] tg3: provide frags as skb head
From: Ben Hutchings @ 2012-04-27 16:53 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, netdev, Ilpo Järvinen, Tom Herbert,
	Neal Cardwell, Matt Carlson, Jeff Kirsher, Michael Chan,
	Herbert Xu, Maciej Żenczykowski
In-Reply-To: <1335544964.2775.250.camel@edumazet-glaptop>

On Fri, 2012-04-27 at 18:42 +0200, Eric Dumazet wrote:
> On Fri, 2012-04-27 at 17:33 +0100, Ben Hutchings wrote:
> 
> > page_address() may return NULL if CONFIG_HIGHMEM is enabled.  And that's
> > going to be a general problem with this scheme on 32-bit machines.
> 
> kmalloc(2048, GFP_ATOMIC) and alloc_page(GFP_ATOMIC) uses the same pool
> of LOWMEM pages, so there is no difference here. page_address() is
> guaranteed to work.

Oh, of course, GFP_HIGH is not GFP_HIGHMEM...

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: [PATCH 2/2] ss: implement -M option to get all memory information
From: Stephen Hemminger @ 2012-04-27 17:21 UTC (permalink / raw)
  To: Shan Wei; +Cc: xemul, NetDev
In-Reply-To: <4F99FEA1.2000700@gmail.com>

On Fri, 27 Apr 2012 10:04:17 +0800
Shan Wei <shanwei88@gmail.com> wrote:

> Stephen Hemminger said, at 2012/4/26 1:41:
> 
> >>  "   -m, --memory        show socket memory usage\n"
> >> +"   -M, --skmem         show socket memory information.\n"
> >> +"                       this option which will replace -m, is valid since Linux 3.3.\n"
> > 
> > Please don't add reference to kernel version.
> > Why does this need to be a new option? Maybe just do the right thing
> > if present in the netlink response?
> 
> 
> Actually, I'm ok for your suggestion that using a existing option(-m) to
> export all memory information. But with a new option is better for us.

Why? if you are arguing to replace the option anyway.

> Maybe some people use latest iproute source with under kernel 3.3.0, some don't.
> -m option will take different user experience, specially for UDP socket.
> Currently -m option has no effect on UDP socket which i will submit another patch
> to export memory information for. new option, fresh experience.

Lots of options return more or different information based on kernel
version, probably the biggest example is how stats are processed.

> (Exporting out memory information will help us to analysis dropped packets 
> which recorded in RcvbufErrors and SndbufErrors.)
>  
> 
>  
> 

^ permalink raw reply

* [PATCH v2 1/4 net-next] net/ipv6/udp: UDP encapsulation: break backlog_rcv into __udpv6_queue_rcv_skb
From: Benjamin LaHaise @ 2012-04-27 18:23 UTC (permalink / raw)
  To: David S. Miller, James Chapman; +Cc: netdev

This is the first step in reworking the IPv6 UDP code to be structured more
like the IPv4 UDP code.  This patch creates __udpv6_queue_rcv_skb() with
the equivalent sematics to __udp_queue_rcv_skb(), and wires it up to the
backlog_rcv method.

Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
---
 net/ipv6/udp.c |   42 +++++++++++++++++++++++++++---------------
 1 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index d39bbc9..6c0367f 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -496,6 +496,28 @@ out:
 	sock_put(sk);
 }
 
+static int __udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
+{
+	int rc;
+
+	if (!ipv6_addr_any(&inet6_sk(sk)->daddr))
+		sock_rps_save_rxhash(sk, skb);
+
+	rc = sock_queue_rcv_skb(sk, skb);
+	if (rc < 0) {
+		int is_udplite = IS_UDPLITE(sk);
+
+		/* Note that an ENOMEM error is charged twice */
+		if (rc == -ENOMEM)
+			UDP6_INC_STATS_BH(sock_net(sk),
+					UDP_MIB_RCVBUFERRORS, is_udplite);
+		UDP6_INC_STATS_BH(sock_net(sk), UDP_MIB_INERRORS, is_udplite);
+		kfree_skb(skb);
+		return -1;
+	}
+	return 0;
+}
+
 static __inline__ void udpv6_err(struct sk_buff *skb,
 				 struct inet6_skb_parm *opt, u8 type,
 				 u8 code, int offset, __be32 info     )
@@ -503,15 +525,12 @@ static __inline__ void udpv6_err(struct sk_buff *skb,
 	__udp6_lib_err(skb, opt, type, code, offset, info, &udp_table);
 }
 
-int udpv6_queue_rcv_skb(struct sock * sk, struct sk_buff *skb)
+int udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
 {
 	struct udp_sock *up = udp_sk(sk);
 	int rc;
 	int is_udplite = IS_UDPLITE(sk);
 
-	if (!ipv6_addr_any(&inet6_sk(sk)->daddr))
-		sock_rps_save_rxhash(sk, skb);
-
 	if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
 		goto drop;
 
@@ -540,19 +559,12 @@ int udpv6_queue_rcv_skb(struct sock * sk, struct sk_buff *skb)
 	}
 
 	skb_dst_drop(skb);
-	rc = sock_queue_rcv_skb(sk, skb);
-	if (rc < 0) {
-		/* Note that an ENOMEM error is charged twice */
-		if (rc == -ENOMEM)
-			UDP6_INC_STATS_BH(sock_net(sk),
-					UDP_MIB_RCVBUFERRORS, is_udplite);
-		goto drop_no_sk_drops_inc;
-	}
 
-	return 0;
+	rc = __udpv6_queue_rcv_skb(sk, skb);
+
+	return rc;
 drop:
 	atomic_inc(&sk->sk_drops);
-drop_no_sk_drops_inc:
 	UDP6_INC_STATS_BH(sock_net(sk), UDP_MIB_INERRORS, is_udplite);
 	kfree_skb(skb);
 	return -1;
@@ -1471,7 +1483,7 @@ struct proto udpv6_prot = {
 	.getsockopt	   = udpv6_getsockopt,
 	.sendmsg	   = udpv6_sendmsg,
 	.recvmsg	   = udpv6_recvmsg,
-	.backlog_rcv	   = udpv6_queue_rcv_skb,
+	.backlog_rcv	   = __udpv6_queue_rcv_skb,
 	.hash		   = udp_lib_hash,
 	.unhash		   = udp_lib_unhash,
 	.rehash		   = udp_v6_rehash,
-- 
1.7.4.1


-- 
"Thought is the essence of where you are now."

^ permalink raw reply related

* [PATCH v2 2/4 net-next] net/ipv6/udp: UDP encapsulation: move socket locking into udpv6_queue_rcv_skb()
From: Benjamin LaHaise @ 2012-04-27 18:23 UTC (permalink / raw)
  To: David S. Miller, James Chapman; +Cc: netdev

In order to make sure that when the encap_rcv() hook is introduced it is
not called with the socket lock held, move socket locking from callers into
udpv6_queue_rcv_skb(), matching what happens in IPv4.

Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
---
 net/ipv6/udp.c |   97 +++++++++++++++++++++++++------------------------------
 1 files changed, 44 insertions(+), 53 deletions(-)

diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 6c0367f..bc533ea 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -558,14 +558,25 @@ int udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
 			goto drop;
 	}
 
+	if (sk_rcvqueues_full(sk, skb, sk->sk_rcvbuf))
+		goto drop;
+
 	skb_dst_drop(skb);
 
-	rc = __udpv6_queue_rcv_skb(sk, skb);
+	bh_lock_sock(sk);
+	rc = 0;
+	if (!sock_owned_by_user(sk))
+		rc = __udpv6_queue_rcv_skb(sk, skb);
+	else if (sk_add_backlog(sk, skb, sk->sk_rcvbuf)) {
+		bh_unlock_sock(sk);
+		goto drop;
+	}
+	bh_unlock_sock(sk);
 
 	return rc;
 drop:
-	atomic_inc(&sk->sk_drops);
 	UDP6_INC_STATS_BH(sock_net(sk), UDP_MIB_INERRORS, is_udplite);
+	atomic_inc(&sk->sk_drops);
 	kfree_skb(skb);
 	return -1;
 }
@@ -614,37 +625,27 @@ static struct sock *udp_v6_mcast_next(struct net *net, struct sock *sk,
 static void flush_stack(struct sock **stack, unsigned int count,
 			struct sk_buff *skb, unsigned int final)
 {
-	unsigned int i;
+	struct sk_buff *skb1 = NULL;
 	struct sock *sk;
-	struct sk_buff *skb1;
+	unsigned int i;
 
 	for (i = 0; i < count; i++) {
-		skb1 = (i == final) ? skb : skb_clone(skb, GFP_ATOMIC);
-
 		sk = stack[i];
-		if (skb1) {
-			if (sk_rcvqueues_full(sk, skb1, sk->sk_rcvbuf)) {
-				kfree_skb(skb1);
-				goto drop;
-			}
-			bh_lock_sock(sk);
-			if (!sock_owned_by_user(sk))
-				udpv6_queue_rcv_skb(sk, skb1);
-			else if (sk_add_backlog(sk, skb1, sk->sk_rcvbuf)) {
-				kfree_skb(skb1);
-				bh_unlock_sock(sk);
-				goto drop;
-			}
-			bh_unlock_sock(sk);
-			continue;
+		if (likely(skb1 == NULL))
+			skb1 = (i == final) ? skb : skb_clone(skb, GFP_ATOMIC);
+		if (!skb1) {
+			atomic_inc(&sk->sk_drops);
+			UDP6_INC_STATS_BH(sock_net(sk), UDP_MIB_RCVBUFERRORS,
+					  IS_UDPLITE(sk));
+			UDP6_INC_STATS_BH(sock_net(sk), UDP_MIB_INERRORS,
+					  IS_UDPLITE(sk));
 		}
-drop:
-		atomic_inc(&sk->sk_drops);
-		UDP6_INC_STATS_BH(sock_net(sk),
-				UDP_MIB_RCVBUFERRORS, IS_UDPLITE(sk));
-		UDP6_INC_STATS_BH(sock_net(sk),
-				UDP_MIB_INERRORS, IS_UDPLITE(sk));
+
+		if (skb1 && udpv6_queue_rcv_skb(sk, skb1) <= 0)
+			skb1 = NULL;
 	}
+	if (unlikely(skb1))
+		kfree_skb(skb1);
 }
 /*
  * Note: called only from the BH handler context,
@@ -784,39 +785,29 @@ int __udp6_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
 	 * for sock caches... i'll skip this for now.
 	 */
 	sk = __udp6_lib_lookup_skb(skb, uh->source, uh->dest, udptable);
+	if (sk != NULL) {
+		int ret = udpv6_queue_rcv_skb(sk, skb);
+		sock_put(sk);
 
-	if (sk == NULL) {
-		if (!xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb))
-			goto discard;
-
-		if (udp_lib_checksum_complete(skb))
-			goto discard;
-		UDP6_INC_STATS_BH(net, UDP_MIB_NOPORTS,
-				proto == IPPROTO_UDPLITE);
-
-		icmpv6_send(skb, ICMPV6_DEST_UNREACH, ICMPV6_PORT_UNREACH, 0);
+		/* a return value > 0 means to resubmit the input, but
+		 * it wants the return to be -protocol, or 0
+		 */
+		if (ret > 0)
+			return -ret;
 
-		kfree_skb(skb);
 		return 0;
 	}
 
-	/* deliver */
-
-	if (sk_rcvqueues_full(sk, skb, sk->sk_rcvbuf)) {
-		sock_put(sk);
+	if (!xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb))
 		goto discard;
-	}
-	bh_lock_sock(sk);
-	if (!sock_owned_by_user(sk))
-		udpv6_queue_rcv_skb(sk, skb);
-	else if (sk_add_backlog(sk, skb, sk->sk_rcvbuf)) {
-		atomic_inc(&sk->sk_drops);
-		bh_unlock_sock(sk);
-		sock_put(sk);
+
+	if (udp_lib_checksum_complete(skb))
 		goto discard;
-	}
-	bh_unlock_sock(sk);
-	sock_put(sk);
+
+	UDP6_INC_STATS_BH(net, UDP_MIB_NOPORTS, proto == IPPROTO_UDPLITE);
+	icmpv6_send(skb, ICMPV6_DEST_UNREACH, ICMPV6_PORT_UNREACH, 0);
+
+	kfree_skb(skb);
 	return 0;
 
 short_packet:
-- 
1.7.4.1


-- 
"Thought is the essence of where you are now."

^ permalink raw reply related

* [PATCH v2 3/4 net-next] net/ipv6/udp: UDP encapsulation: introduce encap_rcv hook into IPv6
From: Benjamin LaHaise @ 2012-04-27 18:24 UTC (permalink / raw)
  To: David S. Miller, James Chapman; +Cc: netdev

Now that the sematics of udpv6_queue_rcv_skb() match IPv4's
udp_queue_rcv_skb(), introduce the UDP encap_rcv() hook for IPv6.

Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
---
 include/net/udp.h |    3 +++
 net/ipv6/udp.c    |   39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 0 deletions(-)

diff --git a/include/net/udp.h b/include/net/udp.h
index e66fc29..065f379 100644
--- a/include/net/udp.h
+++ b/include/net/udp.h
@@ -268,4 +268,7 @@ extern int udp4_ufo_send_check(struct sk_buff *skb);
 extern struct sk_buff *udp4_ufo_fragment(struct sk_buff *skb,
 	netdev_features_t features);
 extern void udp_encap_enable(void);
+#if IS_ENABLED(CONFIG_IPV6)
+extern void udpv6_encap_enable(void);
+#endif
 #endif	/* _UDP_H */
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index bc533ea..c1d91a7 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -525,6 +525,14 @@ static __inline__ void udpv6_err(struct sk_buff *skb,
 	__udp6_lib_err(skb, opt, type, code, offset, info, &udp_table);
 }
 
+static struct static_key udpv6_encap_needed __read_mostly;
+void udpv6_encap_enable(void)
+{
+	if (!static_key_enabled(&udpv6_encap_needed))
+		static_key_slow_inc(&udpv6_encap_needed);
+}
+EXPORT_SYMBOL(udpv6_encap_enable);
+
 int udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
 {
 	struct udp_sock *up = udp_sk(sk);
@@ -534,6 +542,37 @@ int udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
 	if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
 		goto drop;
 
+	if (static_key_false(&udpv6_encap_needed) && up->encap_type) {
+		int (*encap_rcv)(struct sock *sk, struct sk_buff *skb);
+
+		/*
+		 * This is an encapsulation socket so pass the skb to
+		 * the socket's udp_encap_rcv() hook. Otherwise, just
+		 * fall through and pass this up the UDP socket.
+		 * up->encap_rcv() returns the following value:
+		 * =0 if skb was successfully passed to the encap
+		 *    handler or was discarded by it.
+		 * >0 if skb should be passed on to UDP.
+		 * <0 if skb should be resubmitted as proto -N
+		 */
+
+		/* if we're overly short, let UDP handle it */
+		encap_rcv = ACCESS_ONCE(up->encap_rcv);
+		if (skb->len > sizeof(struct udphdr) && encap_rcv != NULL) {
+			int ret;
+
+			ret = encap_rcv(sk, skb);
+			if (ret <= 0) {
+				UDP_INC_STATS_BH(sock_net(sk),
+						 UDP_MIB_INDATAGRAMS,
+						 is_udplite);
+				return -ret;
+			}
+		}
+
+		/* FALLTHROUGH -- it's a UDP Packet */
+	}
+
 	/*
 	 * UDP-Lite specific tests, ignored on UDP sockets (see net/ipv4/udp.c).
 	 */
-- 
1.7.4.1


-- 
"Thought is the essence of where you are now."

^ permalink raw reply related

* [PATCH v2 4/4 net-next] net/l2tp: add support for L2TP over IPv6 UDP
From: Benjamin LaHaise @ 2012-04-27 18:24 UTC (permalink / raw)
  To: David S. Miller, James Chapman; +Cc: netdev

Now that encap_rcv() works on IPv6 UDP sockets, wire L2TP up to IPv6.
Support has been tested with and without hardware offloading.  This
version fixes the L2TP over localhost issue with incorrect checksums
being reported.

Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
---
 include/linux/if_pppol2tp.h |   28 +++++++++++++-
 include/linux/if_pppox.h    |   12 ++++++
 net/l2tp/l2tp_core.c        |   89 +++++++++++++++++++++++++++++++++++++------
 net/l2tp/l2tp_ppp.c         |   42 ++++++++++++++++++++-
 4 files changed, 157 insertions(+), 14 deletions(-)

diff --git a/include/linux/if_pppol2tp.h b/include/linux/if_pppol2tp.h
index 23cefa1..b477541 100644
--- a/include/linux/if_pppol2tp.h
+++ b/include/linux/if_pppol2tp.h
@@ -19,10 +19,11 @@
 
 #ifdef __KERNEL__
 #include <linux/in.h>
+#include <linux/in6.h>
 #endif
 
 /* Structure used to connect() the socket to a particular tunnel UDP
- * socket.
+ * socket over IPv4.
  */
 struct pppol2tp_addr {
 	__kernel_pid_t	pid;		/* pid that owns the fd.
@@ -35,6 +36,20 @@ struct pppol2tp_addr {
 	__u16 d_tunnel, d_session;	/* For sending outgoing packets */
 };
 
+/* Structure used to connect() the socket to a particular tunnel UDP
+ * socket over IPv6.
+ */
+struct pppol2tpin6_addr {
+	__kernel_pid_t	pid;		/* pid that owns the fd.
+					 * 0 => current */
+	int	fd;			/* FD of UDP socket to use */
+
+	__u16 s_tunnel, s_session;	/* For matching incoming packets */
+	__u16 d_tunnel, d_session;	/* For sending outgoing packets */
+
+	struct sockaddr_in6 addr;	/* IP address and port to send to */
+};
+
 /* The L2TPv3 protocol changes tunnel and session ids from 16 to 32
  * bits. So we need a different sockaddr structure.
  */
@@ -49,6 +64,17 @@ struct pppol2tpv3_addr {
 	__u32 d_tunnel, d_session;	/* For sending outgoing packets */
 };
 
+struct pppol2tpv3in6_addr {
+	__kernel_pid_t	pid;		/* pid that owns the fd.
+					 * 0 => current */
+	int	fd;			/* FD of UDP or IP socket to use */
+
+	__u32 s_tunnel, s_session;	/* For matching incoming packets */
+	__u32 d_tunnel, d_session;	/* For sending outgoing packets */
+
+	struct sockaddr_in6 addr;	/* IP address and port to send to */
+};
+
 /* Socket options:
  * DEBUG	- bitmask of debug message categories
  * SENDSEQ	- 0 => don't send packets with sequence numbers
diff --git a/include/linux/if_pppox.h b/include/linux/if_pppox.h
index b5f927f..6720d57 100644
--- a/include/linux/if_pppox.h
+++ b/include/linux/if_pppox.h
@@ -83,6 +83,12 @@ struct sockaddr_pppol2tp {
 	struct pppol2tp_addr pppol2tp;
 } __attribute__((packed));
 
+struct sockaddr_pppol2tpin6 {
+	__kernel_sa_family_t sa_family; /* address family, AF_PPPOX */
+	unsigned int    sa_protocol;    /* protocol identifier */
+	struct pppol2tpin6_addr pppol2tp;
+} __attribute__((packed));
+
 /* The L2TPv3 protocol changes tunnel and session ids from 16 to 32
  * bits. So we need a different sockaddr structure.
  */
@@ -92,6 +98,12 @@ struct sockaddr_pppol2tpv3 {
 	struct pppol2tpv3_addr pppol2tp;
 } __attribute__((packed));
 
+struct sockaddr_pppol2tpv3in6 {
+	__kernel_sa_family_t sa_family; /* address family, AF_PPPOX */
+	unsigned int    sa_protocol;    /* protocol identifier */
+	struct pppol2tpv3in6_addr pppol2tp;
+} __attribute__((packed));
+
 /*********************************************************************
  *
  * ioctl interface for defining forwarding of connections
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index f6732b6..e91d559 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -53,6 +53,9 @@
 #include <net/inet_common.h>
 #include <net/xfrm.h>
 #include <net/protocol.h>
+#include <net/inet6_connection_sock.h>
+#include <net/inet_ecn.h>
+#include <net/ip6_route.h>
 
 #include <asm/byteorder.h>
 #include <linux/atomic.h>
@@ -446,21 +449,43 @@ static inline int l2tp_verify_udp_checksum(struct sock *sk,
 {
 	struct udphdr *uh = udp_hdr(skb);
 	u16 ulen = ntohs(uh->len);
-	struct inet_sock *inet;
 	__wsum psum;
 
-	if (sk->sk_no_check || skb_csum_unnecessary(skb) || !uh->check)
-		return 0;
-
-	inet = inet_sk(sk);
-	psum = csum_tcpudp_nofold(inet->inet_saddr, inet->inet_daddr, ulen,
-				  IPPROTO_UDP, 0);
-
-	if ((skb->ip_summed == CHECKSUM_COMPLETE) &&
-	    !csum_fold(csum_add(psum, skb->csum)))
+	if (sk->sk_no_check || skb_csum_unnecessary(skb))
 		return 0;
 
-	skb->csum = psum;
+#if IS_ENABLED(CONFIG_IPV6)
+	if (sk->sk_family == PF_INET6) {
+		if (!uh->check) {
+			LIMIT_NETDEBUG(KERN_INFO "L2TP: IPv6: checksum is 0\n");
+			return 1;
+		}
+		if ((skb->ip_summed == CHECKSUM_COMPLETE) &&
+		    !csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
+				     &ipv6_hdr(skb)->daddr, ulen,
+				     IPPROTO_UDP, skb->csum)) {
+			skb->ip_summed = CHECKSUM_UNNECESSARY;
+			return 0;
+		}
+		skb->csum = ~csum_unfold(csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
+							 &ipv6_hdr(skb)->daddr,
+							 skb->len, IPPROTO_UDP,
+							 0));
+	} else
+#endif
+	{
+		struct inet_sock *inet;
+		if (!uh->check)
+			return 0;
+		inet = inet_sk(sk);
+		psum = csum_tcpudp_nofold(inet->inet_saddr, inet->inet_daddr,
+					  ulen, IPPROTO_UDP, 0);
+
+		if ((skb->ip_summed == CHECKSUM_COMPLETE) &&
+		    !csum_fold(csum_add(psum, skb->csum)))
+			return 0;
+		skb->csum = psum;
+	}
 
 	return __skb_checksum_complete(skb);
 }
@@ -988,7 +1013,12 @@ static int l2tp_xmit_core(struct l2tp_session *session, struct sk_buff *skb,
 
 	/* Queue the packet to IP for output */
 	skb->local_df = 1;
-	error = ip_queue_xmit(skb, fl);
+#if IS_ENABLED(CONFIG_IPV6)
+	if (skb->sk->sk_family == PF_INET6)
+		error = inet6_csk_xmit(skb, NULL);
+	else
+#endif
+		error = ip_queue_xmit(skb, fl);
 
 	/* Update stats */
 	if (error >= 0) {
@@ -1021,6 +1051,31 @@ static inline void l2tp_skb_set_owner_w(struct sk_buff *skb, struct sock *sk)
 	skb->destructor = l2tp_sock_wfree;
 }
 
+#if IS_ENABLED(CONFIG_IPV6)
+static void l2tp_xmit_ipv6_csum(struct sock *sk, struct sk_buff *skb,
+				int udp_len)
+{
+	struct ipv6_pinfo *np = inet6_sk(sk);
+	struct udphdr *uh = udp_hdr(skb);
+
+	if (!skb_dst(skb) || !skb_dst(skb)->dev ||
+	    !(skb_dst(skb)->dev->features & NETIF_F_IPV6_CSUM)) {
+		__wsum csum = skb_checksum(skb, 0, udp_len, 0);
+		skb->ip_summed = CHECKSUM_UNNECESSARY;
+		uh->check = csum_ipv6_magic(&np->saddr, &np->daddr, udp_len,
+					    IPPROTO_UDP, csum);
+		if (uh->check == 0)
+			uh->check = CSUM_MANGLED_0;
+	} else {
+		skb->ip_summed = CHECKSUM_PARTIAL;
+		skb->csum_start = skb_transport_header(skb) - skb->head;
+		skb->csum_offset = offsetof(struct udphdr, check);
+		uh->check = ~csum_ipv6_magic(&np->saddr, &np->daddr,
+					     udp_len, IPPROTO_UDP, 0);
+	}
+}
+#endif
+
 /* If caller requires the skb to have a ppp header, the header must be
  * inserted in the skb data before calling this function.
  */
@@ -1089,6 +1144,11 @@ int l2tp_xmit_skb(struct l2tp_session *session, struct sk_buff *skb, int hdr_len
 		uh->check = 0;
 
 		/* Calculate UDP checksum if configured to do so */
+#if IS_ENABLED(CONFIG_IPV6)
+		if (sk->sk_family == PF_INET6)
+			l2tp_xmit_ipv6_csum(sk, skb, udp_len);
+		else
+#endif
 		if (sk->sk_no_check == UDP_CSUM_NOXMIT)
 			skb->ip_summed = CHECKSUM_NONE;
 		else if ((skb_dst(skb) && skb_dst(skb)->dev) &&
@@ -1424,6 +1484,11 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32
 		/* Mark socket as an encapsulation socket. See net/ipv4/udp.c */
 		udp_sk(sk)->encap_type = UDP_ENCAP_L2TPINUDP;
 		udp_sk(sk)->encap_rcv = l2tp_udp_encap_recv;
+#if IS_ENABLED(CONFIG_IPV6)
+		if (sk->sk_family == PF_INET6)
+			udpv6_encap_enable();
+		else
+#endif
 		udp_encap_enable();
 	}
 
diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index 1addd9f..27b9dec 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -916,7 +916,7 @@ static int pppol2tp_getname(struct socket *sock, struct sockaddr *uaddr,
 	}
 
 	inet = inet_sk(tunnel->sock);
-	if (tunnel->version == 2) {
+	if ((tunnel->version == 2) && (tunnel->sock->sk_family == AF_INET)) {
 		struct sockaddr_pppol2tp sp;
 		len = sizeof(sp);
 		memset(&sp, 0, len);
@@ -932,6 +932,46 @@ static int pppol2tp_getname(struct socket *sock, struct sockaddr *uaddr,
 		sp.pppol2tp.addr.sin_port = inet->inet_dport;
 		sp.pppol2tp.addr.sin_addr.s_addr = inet->inet_daddr;
 		memcpy(uaddr, &sp, len);
+#if IS_ENABLED(CONFIG_IPV6)
+	} else if ((tunnel->version == 2) &&
+		   (tunnel->sock->sk_family == AF_INET6)) {
+		struct ipv6_pinfo *np = inet6_sk(tunnel->sock);
+		struct sockaddr_pppol2tpin6 sp;
+		len = sizeof(sp);
+		memset(&sp, 0, len);
+		sp.sa_family	= AF_PPPOX;
+		sp.sa_protocol	= PX_PROTO_OL2TP;
+		sp.pppol2tp.fd  = tunnel->fd;
+		sp.pppol2tp.pid = pls->owner;
+		sp.pppol2tp.s_tunnel = tunnel->tunnel_id;
+		sp.pppol2tp.d_tunnel = tunnel->peer_tunnel_id;
+		sp.pppol2tp.s_session = session->session_id;
+		sp.pppol2tp.d_session = session->peer_session_id;
+		sp.pppol2tp.addr.sin6_family = AF_INET6;
+		sp.pppol2tp.addr.sin6_port = inet->inet_dport;
+		memcpy(&sp.pppol2tp.addr.sin6_addr, &np->daddr,
+		       sizeof(np->daddr));
+		memcpy(uaddr, &sp, len);
+	} else if ((tunnel->version == 3) &&
+		   (tunnel->sock->sk_family == AF_INET6)) {
+		struct ipv6_pinfo *np = inet6_sk(tunnel->sock);
+		struct sockaddr_pppol2tpv3in6 sp;
+		len = sizeof(sp);
+		memset(&sp, 0, len);
+		sp.sa_family	= AF_PPPOX;
+		sp.sa_protocol	= PX_PROTO_OL2TP;
+		sp.pppol2tp.fd  = tunnel->fd;
+		sp.pppol2tp.pid = pls->owner;
+		sp.pppol2tp.s_tunnel = tunnel->tunnel_id;
+		sp.pppol2tp.d_tunnel = tunnel->peer_tunnel_id;
+		sp.pppol2tp.s_session = session->session_id;
+		sp.pppol2tp.d_session = session->peer_session_id;
+		sp.pppol2tp.addr.sin6_family = AF_INET6;
+		sp.pppol2tp.addr.sin6_port = inet->inet_dport;
+		memcpy(&sp.pppol2tp.addr.sin6_addr, &np->daddr,
+		       sizeof(np->daddr));
+		memcpy(uaddr, &sp, len);
+#endif
 	} else if (tunnel->version == 3) {
 		struct sockaddr_pppol2tpv3 sp;
 		len = sizeof(sp);
-- 
1.7.4.1


-- 
"Thought is the essence of where you are now."

^ permalink raw reply related

* [PATCH RFC 1/2] netfilter: conntrack: remove RCU usage in conntrack notifier
From: Benjamin Poirier @ 2012-04-27 18:28 UTC (permalink / raw)
  To: netdev
  Cc: Pablo Neira Ayuso, Patrick McHardy, David S. Miller,
	Andrew Morton, Eric Dumazet, Mike Frysinger, Arun Sharma,
	netfilter-devel, netfilter, coreteam, linux-kernel,
	Paul E. McKenney

I think that the rcu usage in this code is pointless. It should either be
removed or, if it was intended to protect against something, it ought to make
that clear.

1) The code does not make use of the deferred deletion/wait for completion rcu
api (ie. synchronize_rcu(), call_rcu()).
2) It does not benefit from the barriers implied by the rcu primitives used.
The code deals with callback pointers. There's no need to order writes to the
function code (!) before writes to the function pointers here.
---
 include/net/netfilter/nf_conntrack_ecache.h |   20 +++------
 include/net/netns/conntrack.h               |    4 +-
 net/netfilter/nf_conntrack_ecache.c         |   58 +++++++-------------------
 3 files changed, 25 insertions(+), 57 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_ecache.h b/include/net/netfilter/nf_conntrack_ecache.h
index a88fb69..ac5b8d2 100644
--- a/include/net/netfilter/nf_conntrack_ecache.h
+++ b/include/net/netfilter/nf_conntrack_ecache.h
@@ -99,14 +99,13 @@ nf_conntrack_eventmask_report(unsigned int eventmask,
 	struct nf_ct_event_notifier *notify;
 	struct nf_conntrack_ecache *e;
 
-	rcu_read_lock();
-	notify = rcu_dereference(net->ct.nf_conntrack_event_cb);
+	notify = net->ct.nf_conntrack_event_cb;
 	if (notify == NULL)
-		goto out_unlock;
+		return ret;
 
 	e = nf_ct_ecache_find(ct);
 	if (e == NULL)
-		goto out_unlock;
+		return ret;
 
 	if (nf_ct_is_confirmed(ct) && !nf_ct_is_dying(ct)) {
 		struct nf_ct_event item = {
@@ -118,7 +117,7 @@ nf_conntrack_eventmask_report(unsigned int eventmask,
 		unsigned long missed = e->pid ? 0 : e->missed;
 
 		if (!((eventmask | missed) & e->ctmask))
-			goto out_unlock;
+			return ret;
 
 		ret = notify->fcn(eventmask | missed, &item);
 		if (unlikely(ret < 0 || missed)) {
@@ -137,8 +136,6 @@ nf_conntrack_eventmask_report(unsigned int eventmask,
 			spin_unlock_bh(&ct->lock);
 		}
 	}
-out_unlock:
-	rcu_read_unlock();
 	return ret;
 }
 
@@ -178,14 +175,13 @@ nf_ct_expect_event_report(enum ip_conntrack_expect_events event,
 	struct nf_exp_event_notifier *notify;
 	struct nf_conntrack_ecache *e;
 
-	rcu_read_lock();
-	notify = rcu_dereference(net->ct.nf_expect_event_cb);
+	notify = net->ct.nf_expect_event_cb;
 	if (notify == NULL)
-		goto out_unlock;
+		return;
 
 	e = nf_ct_ecache_find(exp->master);
 	if (e == NULL)
-		goto out_unlock;
+		return;
 
 	if (e->expmask & (1 << event)) {
 		struct nf_exp_event item = {
@@ -195,8 +191,6 @@ nf_ct_expect_event_report(enum ip_conntrack_expect_events event,
 		};
 		notify->fcn(1 << event, &item);
 	}
-out_unlock:
-	rcu_read_unlock();
 }
 
 static inline void
diff --git a/include/net/netns/conntrack.h b/include/net/netns/conntrack.h
index 7a911ec..c96fd8c 100644
--- a/include/net/netns/conntrack.h
+++ b/include/net/netns/conntrack.h
@@ -18,8 +18,8 @@ struct netns_ct {
 	struct hlist_nulls_head	unconfirmed;
 	struct hlist_nulls_head	dying;
 	struct ip_conntrack_stat __percpu *stat;
-	struct nf_ct_event_notifier __rcu *nf_conntrack_event_cb;
-	struct nf_exp_event_notifier __rcu *nf_expect_event_cb;
+	struct nf_ct_event_notifier *nf_conntrack_event_cb;
+	struct nf_exp_event_notifier *nf_expect_event_cb;
 	int			sysctl_events;
 	unsigned int		sysctl_events_retry_timeout;
 	int			sysctl_acct;
diff --git a/net/netfilter/nf_conntrack_ecache.c b/net/netfilter/nf_conntrack_ecache.c
index b924f3a..0134009 100644
--- a/net/netfilter/nf_conntrack_ecache.c
+++ b/net/netfilter/nf_conntrack_ecache.c
@@ -38,19 +38,18 @@ void nf_ct_deliver_cached_events(struct nf_conn *ct)
 	struct nf_ct_event item;
 	int ret;
 
-	rcu_read_lock();
-	notify = rcu_dereference(net->ct.nf_conntrack_event_cb);
+	notify = net->ct.nf_conntrack_event_cb;
 	if (notify == NULL)
-		goto out_unlock;
+		return;
 
 	e = nf_ct_ecache_find(ct);
 	if (e == NULL)
-		goto out_unlock;
+		return;
 
 	events = xchg(&e->cache, 0);
 
 	if (!nf_ct_is_confirmed(ct) || nf_ct_is_dying(ct) || !events)
-		goto out_unlock;
+		return;
 
 	/* We make a copy of the missed event cache without taking
 	 * the lock, thus we may send missed events twice. However,
@@ -58,7 +57,7 @@ void nf_ct_deliver_cached_events(struct nf_conn *ct)
 	missed = e->missed;
 
 	if (!((events | missed) & e->ctmask))
-		goto out_unlock;
+		return;
 
 	item.ct = ct;
 	item.pid = 0;
@@ -67,7 +66,7 @@ void nf_ct_deliver_cached_events(struct nf_conn *ct)
 	ret = notify->fcn(events | missed, &item);
 
 	if (likely(ret >= 0 && !missed))
-		goto out_unlock;
+		return;
 
 	spin_lock_bh(&ct->lock);
 	if (ret < 0)
@@ -75,9 +74,6 @@ void nf_ct_deliver_cached_events(struct nf_conn *ct)
 	else
 		e->missed &= ~missed;
 	spin_unlock_bh(&ct->lock);
-
-out_unlock:
-	rcu_read_unlock();
 }
 EXPORT_SYMBOL_GPL(nf_ct_deliver_cached_events);
 
@@ -85,21 +81,14 @@ int nf_conntrack_register_notifier(struct net *net,
 				   struct nf_ct_event_notifier *new)
 {
 	int ret = 0;
-	struct nf_ct_event_notifier *notify;
 
 	mutex_lock(&nf_ct_ecache_mutex);
-	notify = rcu_dereference_protected(net->ct.nf_conntrack_event_cb,
-					   lockdep_is_held(&nf_ct_ecache_mutex));
-	if (notify != NULL) {
+	if (net->ct.nf_conntrack_event_cb != NULL)
 		ret = -EBUSY;
-		goto out_unlock;
-	}
-	rcu_assign_pointer(net->ct.nf_conntrack_event_cb, new);
+	else
+		net->ct.nf_conntrack_event_cb = new;
 	mutex_unlock(&nf_ct_ecache_mutex);
-	return ret;
 
-out_unlock:
-	mutex_unlock(&nf_ct_ecache_mutex);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(nf_conntrack_register_notifier);
@@ -107,13 +96,9 @@ EXPORT_SYMBOL_GPL(nf_conntrack_register_notifier);
 void nf_conntrack_unregister_notifier(struct net *net,
 				      struct nf_ct_event_notifier *new)
 {
-	struct nf_ct_event_notifier *notify;
-
 	mutex_lock(&nf_ct_ecache_mutex);
-	notify = rcu_dereference_protected(net->ct.nf_conntrack_event_cb,
-					   lockdep_is_held(&nf_ct_ecache_mutex));
-	BUG_ON(notify != new);
-	RCU_INIT_POINTER(net->ct.nf_conntrack_event_cb, NULL);
+	BUG_ON(net->ct.nf_conntrack_event_cb != new);
+	net->ct.nf_conntrack_event_cb = NULL;
 	mutex_unlock(&nf_ct_ecache_mutex);
 }
 EXPORT_SYMBOL_GPL(nf_conntrack_unregister_notifier);
@@ -122,21 +107,14 @@ int nf_ct_expect_register_notifier(struct net *net,
 				   struct nf_exp_event_notifier *new)
 {
 	int ret = 0;
-	struct nf_exp_event_notifier *notify;
 
 	mutex_lock(&nf_ct_ecache_mutex);
-	notify = rcu_dereference_protected(net->ct.nf_expect_event_cb,
-					   lockdep_is_held(&nf_ct_ecache_mutex));
-	if (notify != NULL) {
+	if (net->ct.nf_expect_event_cb != NULL)
 		ret = -EBUSY;
-		goto out_unlock;
-	}
-	rcu_assign_pointer(net->ct.nf_expect_event_cb, new);
+	else
+		net->ct.nf_expect_event_cb = new;
 	mutex_unlock(&nf_ct_ecache_mutex);
-	return ret;
 
-out_unlock:
-	mutex_unlock(&nf_ct_ecache_mutex);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(nf_ct_expect_register_notifier);
@@ -144,13 +122,9 @@ EXPORT_SYMBOL_GPL(nf_ct_expect_register_notifier);
 void nf_ct_expect_unregister_notifier(struct net *net,
 				      struct nf_exp_event_notifier *new)
 {
-	struct nf_exp_event_notifier *notify;
-
 	mutex_lock(&nf_ct_ecache_mutex);
-	notify = rcu_dereference_protected(net->ct.nf_expect_event_cb,
-					   lockdep_is_held(&nf_ct_ecache_mutex));
-	BUG_ON(notify != new);
-	RCU_INIT_POINTER(net->ct.nf_expect_event_cb, NULL);
+	BUG_ON(net->ct.nf_expect_event_cb != new);
+	net->ct.nf_expect_event_cb = NULL;
 	mutex_unlock(&nf_ct_ecache_mutex);
 }
 EXPORT_SYMBOL_GPL(nf_ct_expect_unregister_notifier);
-- 
1.7.7


^ permalink raw reply related

* [PATCH RFC 2/2] netfilter: conntrack: replace mutex with cmpxchg
From: Benjamin Poirier @ 2012-04-27 18:28 UTC (permalink / raw)
  To: netdev
  Cc: Pablo Neira Ayuso, Patrick McHardy, David S. Miller,
	Andrew Morton, Eric Dumazet, Mike Frysinger, Arun Sharma,
	netfilter-devel, netfilter, coreteam, linux-kernel,
	Paul E. McKenney
In-Reply-To: <1335551333-6103-1-git-send-email-bpoirier@suse.de>

This mutex protects a single pointer.
---
 net/netfilter/nf_conntrack_ecache.c |   38 +++++++++-------------------------
 1 files changed, 10 insertions(+), 28 deletions(-)

diff --git a/net/netfilter/nf_conntrack_ecache.c b/net/netfilter/nf_conntrack_ecache.c
index 0134009..603eb69 100644
--- a/net/netfilter/nf_conntrack_ecache.c
+++ b/net/netfilter/nf_conntrack_ecache.c
@@ -25,8 +25,6 @@
 #include <net/netfilter/nf_conntrack_core.h>
 #include <net/netfilter/nf_conntrack_extend.h>
 
-static DEFINE_MUTEX(nf_ct_ecache_mutex);
-
 /* deliver cached events and clear cache entry - must be called with locally
  * disabled softirqs */
 void nf_ct_deliver_cached_events(struct nf_conn *ct)
@@ -80,52 +78,36 @@ EXPORT_SYMBOL_GPL(nf_ct_deliver_cached_events);
 int nf_conntrack_register_notifier(struct net *net,
 				   struct nf_ct_event_notifier *new)
 {
-	int ret = 0;
-
-	mutex_lock(&nf_ct_ecache_mutex);
-	if (net->ct.nf_conntrack_event_cb != NULL)
-		ret = -EBUSY;
+	if (cmpxchg(&net->ct.nf_conntrack_event_cb, NULL, new) != NULL)
+		return -EBUSY;
 	else
-		net->ct.nf_conntrack_event_cb = new;
-	mutex_unlock(&nf_ct_ecache_mutex);
-
-	return ret;
+		return 0;
 }
 EXPORT_SYMBOL_GPL(nf_conntrack_register_notifier);
 
 void nf_conntrack_unregister_notifier(struct net *net,
 				      struct nf_ct_event_notifier *new)
 {
-	mutex_lock(&nf_ct_ecache_mutex);
-	BUG_ON(net->ct.nf_conntrack_event_cb != new);
-	net->ct.nf_conntrack_event_cb = NULL;
-	mutex_unlock(&nf_ct_ecache_mutex);
+	if (xchg(&net->ct.nf_conntrack_event_cb, NULL) != new)
+		BUG();
 }
 EXPORT_SYMBOL_GPL(nf_conntrack_unregister_notifier);
 
 int nf_ct_expect_register_notifier(struct net *net,
 				   struct nf_exp_event_notifier *new)
 {
-	int ret = 0;
-
-	mutex_lock(&nf_ct_ecache_mutex);
-	if (net->ct.nf_expect_event_cb != NULL)
-		ret = -EBUSY;
+	if (cmpxchg(&net->ct.nf_expect_event_cb, NULL, new) != NULL)
+		return -EBUSY;
 	else
-		net->ct.nf_expect_event_cb = new;
-	mutex_unlock(&nf_ct_ecache_mutex);
-
-	return ret;
+		return 0;
 }
 EXPORT_SYMBOL_GPL(nf_ct_expect_register_notifier);
 
 void nf_ct_expect_unregister_notifier(struct net *net,
 				      struct nf_exp_event_notifier *new)
 {
-	mutex_lock(&nf_ct_ecache_mutex);
-	BUG_ON(net->ct.nf_expect_event_cb != new);
-	net->ct.nf_expect_event_cb = NULL;
-	mutex_unlock(&nf_ct_ecache_mutex);
+	if (xchg(&net->ct.nf_expect_event_cb, NULL) != new)
+		BUG();
 }
 EXPORT_SYMBOL_GPL(nf_ct_expect_unregister_notifier);
 
-- 
1.7.7


^ permalink raw reply related

* Re: [PATCH] mwl8k: Add 0x2a02 PCI device-id (Marvell 88W8361)
From: Lennert Buytenhek @ 2012-04-27 18:58 UTC (permalink / raw)
  To: sedat.dilek
  Cc: John W. Linville, linux-wireless, netdev, linux-kernel, lautriv,
	Jim Cromie
In-Reply-To: <CA+icZUV76wSBj88EpzmWbO=u2dP23yi5P98nN2KZehBUni9mEg@mail.gmail.com>

On Fri, Apr 27, 2012 at 03:29:26PM +0200, Sedat Dilek wrote:

> >>> >> >> >> Are you planning to or even working on support (for) 8361 devices?
> >>> >> >> >
> >>> >> >> > I don't have any such plans, and I don't know of anyone who does.
> >>> >> >>
> >>> >> >> Does 8361 require firmware support?
> >>> >> >
> >>> >> > What do you mean by that?
> >>> >>
> >>> >> IIRC ath5k for example needs no external firmware file.
> >>> >
> >>> > The 8361 requires firmware to be loaded into it before it'll do
> >>> > anything useful.
> >>> >
> >>> >
> >>> >> >> Does a firmware file exist (name?)?
> >>> >> >
> >>> >> > There's firmware for the 8361 out there, however, that version of
> >>> >> > the firmware implements a firmware API that is different from the
> >>> >> > one that mwl8k currently implements.
> >>> >> >
> >>> >> > You could add 8361 support to mwl8k, but then you'd have to go over
> >>> >> > all the firmware command invocations in mwl8k and make sure that they
> >>> >> > will work on the 8361 firmware that you're trying to support as well.
> >>> >>
> >>> >> Without having a 8361 this will be even harder to walk through.
> >>> >>
> >>> >> Anyway, thanks for your detailed explanations.
> >>> >>
> >>> >> What's the alternative for such affected users?
> >>> >> Use ndis-wrapper?
> >>> >
> >>> > I'm not sure.  I've never tried to get a 8361 work under Linux.
> >>>
> >>> Just found on [1] this same wrong patch in [2] :-).
> >>> Dunno if [3] worked and from where they have stolen fw-files.
> >>>
> >>> - Sedat -
> >>>
> >>> [1] https://dev.openwrt.org/ticket/7209
> >>> [2] https://dev.openwrt.org/attachment/ticket/7209/mwl8k_fix_pci_id.patch
> >>> [3] https://dev.openwrt.org/attachment/ticket/7209/mwl8k_8361p.patch
> >>
> >> I doubt that [3] is really all that's needed to make it work.  But if
> >> there's someone for whom it works, I'd like them to run some tests on
> >> mwl8k + [3] on 88w8361p.
> >
> > Just asked lautriv to do so (he will do against linux-3.3.3), lets' see.
> > Also, I refreshed and adapted a bit the instructions (see attachment).
> >
> > - Sedat -
> 
> [ CC Jim Cromie ]
> 
> Now, with extracted firmware files renamed & copied to
> /lib/firmware/mwl8k/ and applied patch (see attachment) against
> Linux-3.4-rc4+ I am seeing with modinfo:
> 
> $ sudo modinfo mwl8k
> filename:
> /lib/modules/3.4.0-rc4-5-generic/kernel/drivers/net/wireless/mwl8k.ko
> license:        GPL
> author:         Lennert Buytenhek <buytenh@marvell.com>
> version:        0.13
> description:    Marvell TOPDOG(R) 802.11 Wireless Network Driver
> firmware:       mwl8k/fmimage_8366_ap-2.fw
> firmware:       mwl8k/fmimage_8366.fw
> firmware:       mwl8k/helper_8366.fw
> firmware:       mwl8k/fmimage_8687.fw
> firmware:       mwl8k/helper_8687.fw
> firmware:       mwl8k/fmimage_8363.fw
> firmware:       mwl8k/helper_8363.fw
> firmware:       mwl8k/fmimage_8361p.fw
> firmware:       mwl8k/helper_8361p.fw
> srcversion:     9E1479A05C8D67E6AE90746
> alias:          pci:v000011ABd00002A43sv*sd*bc*sc*i*
> alias:          pci:v000011ABd00002A40sv*sd*bc*sc*i*
> alias:          pci:v000011ABd00002A30sv*sd*bc*sc*i*
> alias:          pci:v000011ABd00002A2Bsv*sd*bc*sc*i*
> alias:          pci:v000011ABd00002A24sv*sd*bc*sc*i*
> alias:          pci:v000011ABd00002A0Csv*sd*bc*sc*i*
> alias:          pci:v000011ABd00002A0Asv*sd*bc*sc*i*
> alias:          pci:v000011ABd00002A02sv*sd*bc*sc*i* <--- 8361P:
> 0x2a02 PCI device-id
> depends:        mac80211,cfg80211
> vermagic:       3.4.0-rc4-5-generic SMP mod_unload modversions
> parm:           ap_mode_default:Set to 1 to make ap mode the default
> instead of sta mode (bool)
> 
> $ ls -l /lib/firmware/mwl8k/
> insgesamt 456
> -rw-r--r-- 1 root root  75848 Apr 27 13:49 fmimage_8361p.fw <---
> 8361P: Firmware image
> -rw-r--r-- 1 root root 101780 Mär 19 19:32 fmimage_8366_ap-1.fw
> -rw-r--r-- 1 root root 101976 Mär 19 19:32 fmimage_8366_ap-2.fw
> -rw-r--r-- 1 root root  96664 Mär 19 19:32 fmimage_8366.fw
> -rw-r--r-- 1 root root  73252 Feb 23 20:07 fmimage_8687.fw
> -rw-r--r-- 1 root root   2476 Apr 27 13:49 helper_8361p.fw <--- 8361P:
> Helper image
> -rw-r--r-- 1 root root   2476 Mär 19 19:32 helper_8366.fw
> -rw-r--r-- 1 root root   2476 Feb 23 20:07 helper_8687.fw
> 
> As said... ***compile-tested*** only here.

Which doesn't say much at all, but..


> I had a short query with lautriv on #linux-wireless this afternoon:
> * mwl8k kernel-module was autoloaded
> * wlan0 interface got fired up
> * ESSID was accepted
> * logs reported 802.11bgn support is active
> * (he tested on Linux-3.3.3)
> 
> With WPA/WPA2 lautriv had some problems as his installation was
> missing wpasupplicant.
> He setup a classic /etc/network/interfaces.
> After resetting his router mwl8k worked nicely.
> 
> Unfortunately, he was on the run and promised me to send logs and do
> more testing this evening.

..but this is pretty interesting.  I thought that the only available
8361p firmware used an incompatible firmware API, and I didn't know
of the existence of an apparently compatible firmware.


> So, Lennert if you want more testing - What? How? etc.

For one, the output of 'iw phy', please.

Also, does monitor mode work?  Do you get plausible channel/rxpower
values in tcpdump in monitor mode?

Are there any messages in the syslog about failing commands?


thanks,
Lennert

^ permalink raw reply

* Re: [PATCH RFC 1/2] netfilter: conntrack: remove RCU usage in conntrack notifier
From: Eric Dumazet @ 2012-04-27 18:58 UTC (permalink / raw)
  To: Benjamin Poirier
  Cc: netdev, Pablo Neira Ayuso, Patrick McHardy, David S. Miller,
	Andrew Morton, Mike Frysinger, Arun Sharma, netfilter-devel,
	netfilter, coreteam, linux-kernel, Paul E. McKenney
In-Reply-To: <1335551333-6103-1-git-send-email-bpoirier@suse.de>

On Fri, 2012-04-27 at 14:28 -0400, Benjamin Poirier wrote:
> I think that the rcu usage in this code is pointless. It should either be
> removed or, if it was intended to protect against something, it ought to make
> that clear.
> 
> 1) The code does not make use of the deferred deletion/wait for completion rcu
> api (ie. synchronize_rcu(), call_rcu()).
> 2) It does not benefit from the barriers implied by the rcu primitives used.
> The code deals with callback pointers. There's no need to order writes to the
> function code (!) before writes to the function pointers here.
> ---

At a first glance, this seems pretty wrong.

code can disappear under you, thats for sure.

CONFIG_NF_CT_NETLINK=m

hint : module unload contains an rcu barrier.




^ permalink raw reply

* [PATCH 0/4 net-next] another transmit timestamping driver set
From: Willem de Bruijn @ 2012-04-27 19:04 UTC (permalink / raw)
  To: davem, netdev, jeffrey.t.kirscher, eilong, aabdulla

This series adds skb_tx_timestamp calls into the ndo_start_xmit
implementations of a few more drivers. All calls are added before
increasing the txqueue write offset, to avoid the race conditions
with the completion handler experienced earlier.

All changes have been tested with the txtimestamp test at
http://kernel.googlecode.com/files/txtimestamp.c before and after
applying the change (to a slightly different kernel, though) to
verify that it actually adds the missing functionality and
introduces no obvious bugs.

Apologies if I missed any driver maintainers.

Signed-off-by Willem de Bruijn <willemb@google.com>

^ permalink raw reply

* [PATCH 1/4] e1000: add transmit timestamping support
From: Willem de Bruijn @ 2012-04-27 19:04 UTC (permalink / raw)
  To: davem, netdev, jeffrey.t.kirscher, eilong, aabdulla; +Cc: Willem de Bruijn
In-Reply-To: <1335553447-11964-1-git-send-email-willemb@google.com>

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 drivers/net/ethernet/intel/e1000/e1000_main.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index 3d712f2..40438ea 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -3261,6 +3261,8 @@ static netdev_tx_t e1000_xmit_frame(struct sk_buff *skb,
 	                     nr_frags, mss);
 
 	if (count) {
+		skb_tx_timestamp(skb);
+
 		e1000_tx_queue(adapter, tx_ring, tx_flags, count);
 		/* Make sure there is space in the ring for the next send. */
 		e1000_maybe_stop_tx(netdev, tx_ring, MAX_SKB_FRAGS + 2);
-- 
1.7.7.3

^ permalink raw reply related

* [PATCH 3/4] bnx2x: add transmit timestamping support
From: Willem de Bruijn @ 2012-04-27 19:04 UTC (permalink / raw)
  To: davem, netdev, jeffrey.t.kirscher, eilong, aabdulla; +Cc: Willem de Bruijn
In-Reply-To: <1335553447-11964-1-git-send-email-willemb@google.com>

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index 60d5b54..afa6cbb 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -2953,6 +2953,8 @@ netdev_tx_t bnx2x_start_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	netdev_tx_sent_queue(txq, skb->len);
 
+	skb_tx_timestamp(skb);
+
 	txdata->tx_pkt_prod++;
 	/*
 	 * Make sure that the BD data is updated before updating the producer
-- 
1.7.7.3

^ permalink raw reply related

* [PATCH 2/4] e1000e: add transmit timestamping support
From: Willem de Bruijn @ 2012-04-27 19:04 UTC (permalink / raw)
  To: davem, netdev, jeffrey.t.kirscher, eilong, aabdulla; +Cc: Willem de Bruijn
In-Reply-To: <1335553447-11964-1-git-send-email-willemb@google.com>

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 drivers/net/ethernet/intel/e1000e/netdev.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 851f793..c5d9ad7 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -5150,6 +5150,8 @@ static netdev_tx_t e1000_xmit_frame(struct sk_buff *skb,
 	/* if count is 0 then mapping error has occurred */
 	count = e1000_tx_map(tx_ring, skb, first, max_per_txd, nr_frags, mss);
 	if (count) {
+		skb_tx_timestamp(skb);
+
 		netdev_sent_queue(netdev, skb->len);
 		e1000_tx_queue(tx_ring, tx_flags, count);
 		/* Make sure there is space in the ring for the next send. */
-- 
1.7.7.3

^ permalink raw reply related

* [PATCH 4/4] forcedeth: add transmit timestamping support
From: Willem de Bruijn @ 2012-04-27 19:04 UTC (permalink / raw)
  To: davem, netdev, jeffrey.t.kirscher, eilong, aabdulla; +Cc: Willem de Bruijn
In-Reply-To: <1335553447-11964-1-git-send-email-willemb@google.com>

Insert an skb_tx_timestamp call in both ndo_start_xmit routines
Tested to work for the nv_start_xmit_optimized case

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 drivers/net/ethernet/nvidia/forcedeth.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index d93a088..928913c 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -2279,6 +2279,8 @@ static netdev_tx_t nv_start_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	netdev_sent_queue(np->dev, skb->len);
 
+	skb_tx_timestamp(skb);
+
 	np->put_tx.orig = put_tx;
 
 	spin_unlock_irqrestore(&np->lock, flags);
@@ -2426,6 +2428,8 @@ static netdev_tx_t nv_start_xmit_optimized(struct sk_buff *skb,
 
 	netdev_sent_queue(np->dev, skb->len);
 
+	skb_tx_timestamp(skb);
+
 	np->put_tx.ex = put_tx;
 
 	spin_unlock_irqrestore(&np->lock, flags);
-- 
1.7.7.3

^ 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