Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] netfilter: nf_conntrack: fix RCU race in nf_conntrack_find_get
From: Eric Dumazet @ 2014-01-08 13:42 UTC (permalink / raw)
  To: Florian Westphal
  Cc: Andrey Vagin, netfilter-devel, netfilter, coreteam, netdev,
	linux-kernel, vvs, Pablo Neira Ayuso, Patrick McHardy,
	Jozsef Kadlecsik, David S. Miller, Cyrill Gorcunov
In-Reply-To: <20140107152520.GF9894@breakpoint.cc>

On Tue, 2014-01-07 at 16:25 +0100, Florian Westphal wrote:
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > > diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
> > > index 43549eb..7a34bb2 100644
> > > --- a/net/netfilter/nf_conntrack_core.c
> > > +++ b/net/netfilter/nf_conntrack_core.c
> > > @@ -387,8 +387,12 @@ begin:
> > >  			     !atomic_inc_not_zero(&ct->ct_general.use)))
> > >  			h = NULL;
> > >  		else {
> > > +			/* A conntrack can be recreated with the equal tuple,
> > > +			 * so we need to check that the conntrack is initialized
> > > +			 */
> > >  			if (unlikely(!nf_ct_tuple_equal(tuple, &h->tuple) ||
> > > -				     nf_ct_zone(ct) != zone)) {
> > > +				     nf_ct_zone(ct) != zone) ||
> > > +				     !nf_ct_is_confirmed(ct)) {
> > >  				nf_ct_put(ct);
> > >  				goto begin;
> > >  			}
> > 
> > I do not think this is the right way to fix this problem (if said
> > problem is confirmed)
> > 
> > Remember the rule about SLAB_DESTROY_BY_RCU :
> > 
> > When a struct is freed, then reused, its important to set the its refcnt
> > (from 0 to 1) only when the structure is fully ready for use.
> > 
> > If a lookup finds a structure which is not yet setup, the
> > atomic_inc_not_zero() will fail.
> 
> Indeed.  But, the structure itself might be ready (or rather,
> can be ready since the allocation side will set the refcount to one
> after doing the initial work, such as zapping old ->status flags and
> setting tuple information).
> 
> The problem is with nat extension area stored in the ct->ext area.
> This extension area is preallocated but the snat/dnat action
> information is only set up after the ct (or rather, the skb that grabbed
> a reference to the nf_conn entry) traverses nat pre/postrouting.
> 
> This will also set up a null-binding when no matching SNAT/DNAT/MASQERUADE
> rule existed.
> 
> The manipulations of the skb->nfct->ext nat area are performed without
> a lock.  Concurrent access is supposedly impossible as the conntrack
> should not (yet) be in the hash table.
> 
> The confirmed bit is set right before we insert the conntrack into
> the hash table (after we traversed rules, ct is ready to be
> 'published').
> 
> i.e. when the confirmed bit is NOT set we should not be 'seeing' the nf_conn
> struct when we perform the lookup, as it should still be sitting on the
> 'unconfirmed' list, being invisible to readers.
> 
> Does that explanation make sense to you?
> 
> Thanks for looking into this.

Still, this patch adds a loop. And maybe an infinite one if confirmed
bit is set from an context that was interrupted by this one.

If you need to test the confirmed bit, then you also need to test it
before taking the refcount.

^ permalink raw reply

* Re: [PATCH] netfilter: nf_conntrack: fix RCU race in nf_conntrack_find_get (v2)
From: Eric Dumazet @ 2014-01-08 13:47 UTC (permalink / raw)
  To: Andrey Vagin
  Cc: netfilter-devel, netfilter, coreteam, netdev, linux-kernel, vvs,
	Florian Westphal, Pablo Neira Ayuso, Patrick McHardy,
	Jozsef Kadlecsik, David S. Miller, Cyrill Gorcunov
In-Reply-To: <1389187051-7794-1-git-send-email-avagin@openvz.org>

On Wed, 2014-01-08 at 17:17 +0400, Andrey Vagin wrote:
> Lets look at destroy_conntrack:
> 
> hlist_nulls_del_rcu(&ct->tuplehash[IP_CT_DIR_ORIGINAL].hnnode);
> ...
> nf_conntrack_free(ct)
> 	kmem_cache_free(net->ct.nf_conntrack_cachep, ct);
> 
> net->ct.nf_conntrack_cachep is created with SLAB_DESTROY_BY_RCU.
> 
> The hash is protected by rcu, so readers look up conntracks without
> locks.
> A conntrack is removed from the hash, but in this moment a few readers
> still can use the conntrack. Then this conntrack is released and another
> thread creates conntrack with the same address and the equal tuple.
> After this a reader starts to validate the conntrack:
> * It's not dying, because a new conntrack was created
> * nf_ct_tuple_equal() returns true.
> 
> But this conntrack is not initialized yet, so it can not be used by two
> threads concurrently. In this case BUG_ON may be triggered from
> nf_nat_setup_info().
> 
> Florian Westphal suggested to check the confirm bit too. I think it's
> right.
> 
> task 1			task 2			task 3
> 			nf_conntrack_find_get
> 			 ____nf_conntrack_find
> destroy_conntrack
>  hlist_nulls_del_rcu
>  nf_conntrack_free
>  kmem_cache_free
> 						__nf_conntrack_alloc
> 						 kmem_cache_alloc
> 						 memset(&ct->tuplehash[IP_CT_DIR_MAX],
> 			 if (nf_ct_is_dying(ct))
> 			 if (!nf_ct_tuple_equal()
> 
> I'm not sure, that I have ever seen this race condition in a real life.
> Currently we are investigating a bug, which is reproduced on a few node.
> In our case one conntrack is initialized from a few tasks concurrently,
> we don't have any other explanation for this.
> 
> <2>[46267.083061] kernel BUG at net/ipv4/netfilter/nf_nat_core.c:322!
> ...
> <4>[46267.083951] RIP: 0010:[<ffffffffa01e00a4>]  [<ffffffffa01e00a4>] nf_nat_setup_info+0x564/0x590 [nf_nat]
> ...
> <4>[46267.085549] Call Trace:
> <4>[46267.085622]  [<ffffffffa023421b>] alloc_null_binding+0x5b/0xa0 [iptable_nat]
> <4>[46267.085697]  [<ffffffffa02342bc>] nf_nat_rule_find+0x5c/0x80 [iptable_nat]
> <4>[46267.085770]  [<ffffffffa0234521>] nf_nat_fn+0x111/0x260 [iptable_nat]
> <4>[46267.085843]  [<ffffffffa0234798>] nf_nat_out+0x48/0xd0 [iptable_nat]
> <4>[46267.085919]  [<ffffffff814841b9>] nf_iterate+0x69/0xb0
> <4>[46267.085991]  [<ffffffff81494e70>] ? ip_finish_output+0x0/0x2f0
> <4>[46267.086063]  [<ffffffff81484374>] nf_hook_slow+0x74/0x110
> <4>[46267.086133]  [<ffffffff81494e70>] ? ip_finish_output+0x0/0x2f0
> <4>[46267.086207]  [<ffffffff814b5890>] ? dst_output+0x0/0x20
> <4>[46267.086277]  [<ffffffff81495204>] ip_output+0xa4/0xc0
> <4>[46267.086346]  [<ffffffff814b65a4>] raw_sendmsg+0x8b4/0x910
> <4>[46267.086419]  [<ffffffff814c10fa>] inet_sendmsg+0x4a/0xb0
> <4>[46267.086491]  [<ffffffff814459aa>] ? sock_update_classid+0x3a/0x50
> <4>[46267.086562]  [<ffffffff81444d67>] sock_sendmsg+0x117/0x140
> <4>[46267.086638]  [<ffffffff8151997b>] ? _spin_unlock_bh+0x1b/0x20
> <4>[46267.086712]  [<ffffffff8109d370>] ? autoremove_wake_function+0x0/0x40
> <4>[46267.086785]  [<ffffffff81495e80>] ? do_ip_setsockopt+0x90/0xd80
> <4>[46267.086858]  [<ffffffff8100be0e>] ? call_function_interrupt+0xe/0x20
> <4>[46267.086936]  [<ffffffff8118cb10>] ? ub_slab_ptr+0x20/0x90
> <4>[46267.087006]  [<ffffffff8118cb10>] ? ub_slab_ptr+0x20/0x90
> <4>[46267.087081]  [<ffffffff8118f2e8>] ? kmem_cache_alloc+0xd8/0x1e0
> <4>[46267.087151]  [<ffffffff81445599>] sys_sendto+0x139/0x190
> <4>[46267.087229]  [<ffffffff81448c0d>] ? sock_setsockopt+0x16d/0x6f0
> <4>[46267.087303]  [<ffffffff810efa47>] ? audit_syscall_entry+0x1d7/0x200
> <4>[46267.087378]  [<ffffffff810ef795>] ? __audit_syscall_exit+0x265/0x290
> <4>[46267.087454]  [<ffffffff81474885>] ? compat_sys_setsockopt+0x75/0x210
> <4>[46267.087531]  [<ffffffff81474b5f>] compat_sys_socketcall+0x13f/0x210
> <4>[46267.087607]  [<ffffffff8104dea3>] ia32_sysret+0x0/0x5
> <4>[46267.087676] Code: 91 20 e2 01 75 29 48 89 de 4c 89 f7 e8 56 fa ff ff 85 c0 0f 84 68 fc ff ff 0f b6 4d c6 41 8b 45 00 e9 4d fb ff ff e8 7c 19 e9 e0 <0f> 0b eb fe f6 05 17 91 20 e2 80 74 ce 80 3d 5f 2e 00 00 00 74
> <1>[46267.088023] RIP  [<ffffffffa01e00a4>] nf_nat_setup_info+0x564/0x590
> 
> v2: move nf_ct_is_confirmed into the unlikely() annotation
> 
> Cc: Florian Westphal <fw@strlen.de>
> Cc: Pablo Neira Ayuso <pablo@netfilter.org>
> Cc: Patrick McHardy <kaber@trash.net>
> Cc: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Cyrill Gorcunov <gorcunov@openvz.org>
> Signed-off-by: Andrey Vagin <avagin@openvz.org>
> ---
>  net/netfilter/nf_conntrack_core.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
> index 43549eb..403f634 100644
> --- a/net/netfilter/nf_conntrack_core.c
> +++ b/net/netfilter/nf_conntrack_core.c
> @@ -387,8 +387,12 @@ begin:
>  			     !atomic_inc_not_zero(&ct->ct_general.use)))
>  			h = NULL;
>  		else {
> +			/* A conntrack can be recreated with the equal tuple,
> +			 * so we need to check that the conntrack is initialized
> +			 */
>  			if (unlikely(!nf_ct_tuple_equal(tuple, &h->tuple) ||
> -				     nf_ct_zone(ct) != zone)) {
> +				     nf_ct_zone(ct) != zone ||
> +				     !nf_ct_is_confirmed(ct))) {
>  				nf_ct_put(ct);
>  				goto begin;
>  			}


I am still not convinced of this being the right fix.


The key we test after taking the refcount should be the same key that we
test before taking the refcount, otherwise we might add a loop here.

Your patch did not change ____nf_conntrack_find(), so I find it
confusing.




^ permalink raw reply

* Re: [PATCH net-next v3 6/9] xen-netback: Handle guests with too many frags
From: Zoltan Kiss @ 2014-01-08 13:49 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
	jonathan.davies
In-Reply-To: <1389147141.26646.74.camel@edumazet-glaptop2.roam.corp.google.com>

On 08/01/14 02:12, Eric Dumazet wrote:
> On Wed, 2014-01-08 at 00:10 +0000, Zoltan Kiss wrote:
>
>>
>> +		if (skb_shinfo(skb)->frag_list) {
>> +			nskb = skb_shinfo(skb)->frag_list;
>> +			xenvif_fill_frags(vif, nskb, INVALID_PENDING_IDX);
>> +			skb->len += nskb->len;
>> +			skb->data_len += nskb->len;
>> +			skb->truesize += nskb->truesize;
>> +			skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
>> +			skb_shinfo(nskb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
>> +			vif->tx_zerocopy_sent += 2;
>> +			nskb = skb;
>> +
>> +			skb = skb_copy_expand(skb,
>> +					0,
>> +					0,
>> +					GFP_ATOMIC | __GFP_NOWARN);
>
> skb can be NULL here

Thanks, fixed that.

Zoli

^ permalink raw reply

* Re: [PATCH net-next v3 6/9] xen-netback: Handle guests with too many frags
From: Eric Dumazet @ 2014-01-08 13:54 UTC (permalink / raw)
  To: Zoltan Kiss
  Cc: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
	jonathan.davies
In-Reply-To: <52CD5785.4050402@citrix.com>

On Wed, 2014-01-08 at 13:49 +0000, Zoltan Kiss wrote:
> On 08/01/14 02:12, Eric Dumazet wrote:
> > On Wed, 2014-01-08 at 00:10 +0000, Zoltan Kiss wrote:
> >
> >>
> >> +		if (skb_shinfo(skb)->frag_list) {
> >> +			nskb = skb_shinfo(skb)->frag_list;
> >> +			xenvif_fill_frags(vif, nskb, INVALID_PENDING_IDX);
> >> +			skb->len += nskb->len;
> >> +			skb->data_len += nskb->len;
> >> +			skb->truesize += nskb->truesize;
> >> +			skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
> >> +			skb_shinfo(nskb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
> >> +			vif->tx_zerocopy_sent += 2;
> >> +			nskb = skb;
> >> +
> >> +			skb = skb_copy_expand(skb,
> >> +					0,
> >> +					0,
> >> +					GFP_ATOMIC | __GFP_NOWARN);
> >
> > skb can be NULL here
> 
> Thanks, fixed that.

BTW, I am not sure why you copy the skb.

Is it to get rid of frag_list, and why ?

^ permalink raw reply

* [PATCH net-next 0/3] make skb_checksum_setup generally available
From: Paul Durrant @ 2014-01-08 13:58 UTC (permalink / raw)
  To: netdev, xen-devel

Both xen-netfront and xen-netback need to be able to set up the partial
checksum offset of an skb and may also need to recalculate the pseudo-
header checksum in the process. This functionality is currently private
and duplicated between the two drivers.

Patch #1 of this series moves the implementation into the core network code
as there is nothing xen-specific about it and it is potentially useful to
any network driver.
Patch #2 removes the private implementation from netback.
Patch #3 removes the private implementation from netfront.

^ permalink raw reply

* [PATCH net-next 1/3] net: add skb_checksum_setup
From: Paul Durrant @ 2014-01-08 13:58 UTC (permalink / raw)
  To: netdev, xen-devel
  Cc: Paul Durrant, David Miller, Eric Dumazet, Veaceslav Falico,
	Alexander Duyck, Nicolas Dichtel
In-Reply-To: <1389189511-14568-1-git-send-email-paul.durrant@citrix.com>

This patch adds a function to set up the partial checksum offset for IP
packets (and optionally re-calculate the pseudo-header checksum) into the
core network code.
The implementation was previously private and duplicated between xen-netback
and xen-netfront, however it is not xen-specific and is potentially useful
to any network driver.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: David Miller <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Veaceslav Falico <vfalico@redhat.com>
Cc: Alexander Duyck <alexander.h.duyck@intel.com>
Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 include/linux/netdevice.h |    1 +
 net/core/dev.c            |  271 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 272 insertions(+)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index a2a70cc..15b1003 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2940,6 +2940,7 @@ void netdev_upper_dev_unlink(struct net_device *dev,
 void *netdev_lower_dev_get_private(struct net_device *dev,
 				   struct net_device *lower_dev);
 int skb_checksum_help(struct sk_buff *skb);
+int skb_checksum_setup(struct sk_buff *skb, bool recalculate);
 struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
 				  netdev_features_t features, bool tx_path);
 struct sk_buff *skb_mac_gso_segment(struct sk_buff *skb,
diff --git a/net/core/dev.c b/net/core/dev.c
index ce01847..cf9fc30 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -101,6 +101,7 @@
 #include <net/dst.h>
 #include <net/pkt_sched.h>
 #include <net/checksum.h>
+#include <net/ip6_checksum.h>
 #include <net/xfrm.h>
 #include <linux/highmem.h>
 #include <linux/init.h>
@@ -2281,6 +2282,276 @@ out:
 }
 EXPORT_SYMBOL(skb_checksum_help);
 
+static inline int skb_maybe_pull_tail(struct sk_buff *skb, unsigned int len,
+				      unsigned int max)
+{
+	if (skb_headlen(skb) >= len)
+		return 0;
+
+	/* If we need to pullup then pullup to the max, so we
+	 * won't need to do it again.
+	 */
+	if (max > skb->len)
+		max = skb->len;
+
+	if (__pskb_pull_tail(skb, max - skb_headlen(skb)) == NULL)
+		return -ENOMEM;
+
+	if (skb_headlen(skb) < len)
+		return -EPROTO;
+
+	return 0;
+}
+
+/* This value should be large enough to cover a tagged ethernet header plus
+ * maximally sized IP and TCP or UDP headers.
+ */
+#define MAX_IP_HDR_LEN 128
+
+static int skb_checksum_setup_ip(struct sk_buff *skb, bool recalculate)
+{
+	unsigned int off;
+	bool fragment;
+	int err;
+
+	fragment = false;
+
+	err = skb_maybe_pull_tail(skb,
+				  sizeof(struct iphdr),
+				  MAX_IP_HDR_LEN);
+	if (err < 0)
+		goto out;
+
+	if (ip_hdr(skb)->frag_off & htons(IP_OFFSET | IP_MF))
+		fragment = true;
+
+	off = ip_hdrlen(skb);
+
+	err = -EPROTO;
+
+	if (fragment)
+		goto out;
+
+	switch (ip_hdr(skb)->protocol) {
+	case IPPROTO_TCP:
+		err = skb_maybe_pull_tail(skb,
+					  off + sizeof(struct tcphdr),
+					  MAX_IP_HDR_LEN);
+		if (err < 0)
+			goto out;
+
+		if (!skb_partial_csum_set(skb, off,
+					  offsetof(struct tcphdr, check))) {
+			err = -EPROTO;
+			goto out;
+		}
+
+		if (recalculate)
+			tcp_hdr(skb)->check =
+				~csum_tcpudp_magic(ip_hdr(skb)->saddr,
+						   ip_hdr(skb)->daddr,
+						   skb->len - off,
+						   IPPROTO_TCP, 0);
+		break;
+	case IPPROTO_UDP:
+		err = skb_maybe_pull_tail(skb,
+					  off + sizeof(struct udphdr),
+					  MAX_IP_HDR_LEN);
+		if (err < 0)
+			goto out;
+
+		if (!skb_partial_csum_set(skb, off,
+					  offsetof(struct udphdr, check))) {
+			err = -EPROTO;
+			goto out;
+		}
+
+		if (recalculate)
+			udp_hdr(skb)->check =
+				~csum_tcpudp_magic(ip_hdr(skb)->saddr,
+						   ip_hdr(skb)->daddr,
+						   skb->len - off,
+						   IPPROTO_UDP, 0);
+		break;
+	default:
+		goto out;
+	}
+
+	err = 0;
+
+out:
+	return err;
+}
+
+/* This value should be large enough to cover a tagged ethernet header plus
+ * an IPv6 header, all options, and a maximal TCP or UDP header.
+ */
+#define MAX_IPV6_HDR_LEN 256
+
+#define OPT_HDR(type, skb, off) \
+	(type *)(skb_network_header(skb) + (off))
+
+static int skb_checksum_setup_ipv6(struct sk_buff *skb, bool recalculate)
+{
+	int err;
+	u8 nexthdr;
+	unsigned int off;
+	unsigned int len;
+	bool fragment;
+	bool done;
+
+	fragment = false;
+	done = false;
+
+	off = sizeof(struct ipv6hdr);
+
+	err = skb_maybe_pull_tail(skb, off, MAX_IPV6_HDR_LEN);
+	if (err < 0)
+		goto out;
+
+	nexthdr = ipv6_hdr(skb)->nexthdr;
+
+	len = sizeof(struct ipv6hdr) + ntohs(ipv6_hdr(skb)->payload_len);
+	while (off <= len && !done) {
+		switch (nexthdr) {
+		case IPPROTO_DSTOPTS:
+		case IPPROTO_HOPOPTS:
+		case IPPROTO_ROUTING: {
+			struct ipv6_opt_hdr *hp;
+
+			err = skb_maybe_pull_tail(skb,
+						  off +
+						  sizeof(struct ipv6_opt_hdr),
+						  MAX_IPV6_HDR_LEN);
+			if (err < 0)
+				goto out;
+
+			hp = OPT_HDR(struct ipv6_opt_hdr, skb, off);
+			nexthdr = hp->nexthdr;
+			off += ipv6_optlen(hp);
+			break;
+		}
+		case IPPROTO_AH: {
+			struct ip_auth_hdr *hp;
+
+			err = skb_maybe_pull_tail(skb,
+						  off +
+						  sizeof(struct ip_auth_hdr),
+						  MAX_IPV6_HDR_LEN);
+			if (err < 0)
+				goto out;
+
+			hp = OPT_HDR(struct ip_auth_hdr, skb, off);
+			nexthdr = hp->nexthdr;
+			off += ipv6_authlen(hp);
+			break;
+		}
+		case IPPROTO_FRAGMENT: {
+			struct frag_hdr *hp;
+
+			err = skb_maybe_pull_tail(skb,
+						  off +
+						  sizeof(struct frag_hdr),
+						  MAX_IPV6_HDR_LEN);
+			if (err < 0)
+				goto out;
+
+			hp = OPT_HDR(struct frag_hdr, skb, off);
+
+			if (hp->frag_off & htons(IP6_OFFSET | IP6_MF))
+				fragment = true;
+
+			nexthdr = hp->nexthdr;
+			off += sizeof(struct frag_hdr);
+			break;
+		}
+		default:
+			done = true;
+			break;
+		}
+	}
+
+	err = -EPROTO;
+
+	if (!done || fragment)
+		goto out;
+
+	switch (nexthdr) {
+	case IPPROTO_TCP:
+		err = skb_maybe_pull_tail(skb,
+					  off + sizeof(struct tcphdr),
+					  MAX_IPV6_HDR_LEN);
+		if (err < 0)
+			goto out;
+
+		if (!skb_partial_csum_set(skb, off,
+					  offsetof(struct tcphdr, check))) {
+			err = -EPROTO;
+			goto out;
+		}
+
+		if (recalculate)
+			tcp_hdr(skb)->check =
+				~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
+						 &ipv6_hdr(skb)->daddr,
+						 skb->len - off,
+						 IPPROTO_TCP, 0);
+		break;
+	case IPPROTO_UDP:
+		err = skb_maybe_pull_tail(skb,
+					  off + sizeof(struct udphdr),
+					  MAX_IPV6_HDR_LEN);
+		if (err < 0)
+			goto out;
+
+		if (!skb_partial_csum_set(skb, off,
+					  offsetof(struct udphdr, check))) {
+			err = -EPROTO;
+			goto out;
+		}
+
+		if (recalculate)
+			udp_hdr(skb)->check =
+				~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
+						 &ipv6_hdr(skb)->daddr,
+						 skb->len - off,
+						 IPPROTO_UDP, 0);
+		break;
+	default:
+		goto out;
+	}
+
+	err = 0;
+
+out:
+	return err;
+}
+
+/* Set up partial checksum offset and optionally recalculate pseudo header
+ * checksum.
+ */
+int skb_checksum_setup(struct sk_buff *skb, bool recalculate)
+{
+	int err;
+
+	switch (skb->protocol) {
+	case htons(ETH_P_IP):
+		err = skb_checksum_setup_ip(skb, recalculate);
+		break;
+
+	case htons(ETH_P_IPV6):
+		err = skb_checksum_setup_ipv6(skb, recalculate);
+		break;
+
+	default:
+		err = -EPROTO;
+		break;
+	}
+
+	return err;
+}
+EXPORT_SYMBOL(skb_checksum_setup);
+
 __be16 skb_network_protocol(struct sk_buff *skb)
 {
 	__be16 type = skb->protocol;
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH net-next 3/3] xen-netfront: use new skb_checksum_setup function
From: Paul Durrant @ 2014-01-08 13:58 UTC (permalink / raw)
  To: netdev, xen-devel
  Cc: Paul Durrant, Konrad Rzeszutek Wilk, Boris Ostrovsky,
	David Vrabel
In-Reply-To: <1389189511-14568-1-git-send-email-paul.durrant@citrix.com>

Use skb_checksum_setup to set up partial checksum offsets rather
then a private implementation.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>
---
 drivers/net/xen-netfront.c |   48 +++-----------------------------------------
 1 file changed, 3 insertions(+), 45 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index e59acb1..c41537b 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -859,9 +859,7 @@ static RING_IDX xennet_fill_frags(struct netfront_info *np,
 
 static int checksum_setup(struct net_device *dev, struct sk_buff *skb)
 {
-	struct iphdr *iph;
-	int err = -EPROTO;
-	int recalculate_partial_csum = 0;
+	bool recalculate_partial_csum = false;
 
 	/*
 	 * A GSO SKB must be CHECKSUM_PARTIAL. However some buggy
@@ -873,54 +871,14 @@ static int checksum_setup(struct net_device *dev, struct sk_buff *skb)
 		struct netfront_info *np = netdev_priv(dev);
 		np->rx_gso_checksum_fixup++;
 		skb->ip_summed = CHECKSUM_PARTIAL;
-		recalculate_partial_csum = 1;
+		recalculate_partial_csum = true;
 	}
 
 	/* A non-CHECKSUM_PARTIAL SKB does not require setup. */
 	if (skb->ip_summed != CHECKSUM_PARTIAL)
 		return 0;
 
-	if (skb->protocol != htons(ETH_P_IP))
-		goto out;
-
-	iph = (void *)skb->data;
-
-	switch (iph->protocol) {
-	case IPPROTO_TCP:
-		if (!skb_partial_csum_set(skb, 4 * iph->ihl,
-					  offsetof(struct tcphdr, check)))
-			goto out;
-
-		if (recalculate_partial_csum) {
-			struct tcphdr *tcph = tcp_hdr(skb);
-			tcph->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr,
-							 skb->len - iph->ihl*4,
-							 IPPROTO_TCP, 0);
-		}
-		break;
-	case IPPROTO_UDP:
-		if (!skb_partial_csum_set(skb, 4 * iph->ihl,
-					  offsetof(struct udphdr, check)))
-			goto out;
-
-		if (recalculate_partial_csum) {
-			struct udphdr *udph = udp_hdr(skb);
-			udph->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr,
-							 skb->len - iph->ihl*4,
-							 IPPROTO_UDP, 0);
-		}
-		break;
-	default:
-		if (net_ratelimit())
-			pr_err("Attempting to checksum a non-TCP/UDP packet, dropping a protocol %d packet\n",
-			       iph->protocol);
-		goto out;
-	}
-
-	err = 0;
-
-out:
-	return err;
+	return skb_checksum_setup(skb, recalculate_partial_csum);
 }
 
 static int handle_incoming_queue(struct net_device *dev,
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH net-next 2/3] xen-netback: use new skb_checksum_setup function
From: Paul Durrant @ 2014-01-08 13:58 UTC (permalink / raw)
  To: netdev, xen-devel; +Cc: Paul Durrant, Ian Campbell, Wei Liu
In-Reply-To: <1389189511-14568-1-git-send-email-paul.durrant@citrix.com>

Use skb_checksum_setup to set up partial checksum offsets rather
then a private implementation.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 drivers/net/xen-netback/netback.c |  260 +------------------------------------
 1 file changed, 3 insertions(+), 257 deletions(-)

diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 4f81ac0..2605405 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -39,7 +39,6 @@
 #include <linux/udp.h>
 
 #include <net/tcp.h>
-#include <net/ip6_checksum.h>
 
 #include <xen/xen.h>
 #include <xen/events.h>
@@ -1048,257 +1047,9 @@ static int xenvif_set_skb_gso(struct xenvif *vif,
 	return 0;
 }
 
-static inline int maybe_pull_tail(struct sk_buff *skb, unsigned int len,
-				  unsigned int max)
-{
-	if (skb_headlen(skb) >= len)
-		return 0;
-
-	/* If we need to pullup then pullup to the max, so we
-	 * won't need to do it again.
-	 */
-	if (max > skb->len)
-		max = skb->len;
-
-	if (__pskb_pull_tail(skb, max - skb_headlen(skb)) == NULL)
-		return -ENOMEM;
-
-	if (skb_headlen(skb) < len)
-		return -EPROTO;
-
-	return 0;
-}
-
-/* This value should be large enough to cover a tagged ethernet header plus
- * maximally sized IP and TCP or UDP headers.
- */
-#define MAX_IP_HDR_LEN 128
-
-static int checksum_setup_ip(struct xenvif *vif, struct sk_buff *skb,
-			     int recalculate_partial_csum)
-{
-	unsigned int off;
-	bool fragment;
-	int err;
-
-	fragment = false;
-
-	err = maybe_pull_tail(skb,
-			      sizeof(struct iphdr),
-			      MAX_IP_HDR_LEN);
-	if (err < 0)
-		goto out;
-
-	if (ip_hdr(skb)->frag_off & htons(IP_OFFSET | IP_MF))
-		fragment = true;
-
-	off = ip_hdrlen(skb);
-
-	err = -EPROTO;
-
-	if (fragment)
-		goto out;
-
-	switch (ip_hdr(skb)->protocol) {
-	case IPPROTO_TCP:
-		err = maybe_pull_tail(skb,
-				      off + sizeof(struct tcphdr),
-				      MAX_IP_HDR_LEN);
-		if (err < 0)
-			goto out;
-
-		if (!skb_partial_csum_set(skb, off,
-					  offsetof(struct tcphdr, check))) {
-			err = -EPROTO;
-			goto out;
-		}
-
-		if (recalculate_partial_csum)
-			tcp_hdr(skb)->check =
-				~csum_tcpudp_magic(ip_hdr(skb)->saddr,
-						   ip_hdr(skb)->daddr,
-						   skb->len - off,
-						   IPPROTO_TCP, 0);
-		break;
-	case IPPROTO_UDP:
-		err = maybe_pull_tail(skb,
-				      off + sizeof(struct udphdr),
-				      MAX_IP_HDR_LEN);
-		if (err < 0)
-			goto out;
-
-		if (!skb_partial_csum_set(skb, off,
-					  offsetof(struct udphdr, check))) {
-			err = -EPROTO;
-			goto out;
-		}
-
-		if (recalculate_partial_csum)
-			udp_hdr(skb)->check =
-				~csum_tcpudp_magic(ip_hdr(skb)->saddr,
-						   ip_hdr(skb)->daddr,
-						   skb->len - off,
-						   IPPROTO_UDP, 0);
-		break;
-	default:
-		goto out;
-	}
-
-	err = 0;
-
-out:
-	return err;
-}
-
-/* This value should be large enough to cover a tagged ethernet header plus
- * an IPv6 header, all options, and a maximal TCP or UDP header.
- */
-#define MAX_IPV6_HDR_LEN 256
-
-#define OPT_HDR(type, skb, off) \
-	(type *)(skb_network_header(skb) + (off))
-
-static int checksum_setup_ipv6(struct xenvif *vif, struct sk_buff *skb,
-			       int recalculate_partial_csum)
-{
-	int err;
-	u8 nexthdr;
-	unsigned int off;
-	unsigned int len;
-	bool fragment;
-	bool done;
-
-	fragment = false;
-	done = false;
-
-	off = sizeof(struct ipv6hdr);
-
-	err = maybe_pull_tail(skb, off, MAX_IPV6_HDR_LEN);
-	if (err < 0)
-		goto out;
-
-	nexthdr = ipv6_hdr(skb)->nexthdr;
-
-	len = sizeof(struct ipv6hdr) + ntohs(ipv6_hdr(skb)->payload_len);
-	while (off <= len && !done) {
-		switch (nexthdr) {
-		case IPPROTO_DSTOPTS:
-		case IPPROTO_HOPOPTS:
-		case IPPROTO_ROUTING: {
-			struct ipv6_opt_hdr *hp;
-
-			err = maybe_pull_tail(skb,
-					      off +
-					      sizeof(struct ipv6_opt_hdr),
-					      MAX_IPV6_HDR_LEN);
-			if (err < 0)
-				goto out;
-
-			hp = OPT_HDR(struct ipv6_opt_hdr, skb, off);
-			nexthdr = hp->nexthdr;
-			off += ipv6_optlen(hp);
-			break;
-		}
-		case IPPROTO_AH: {
-			struct ip_auth_hdr *hp;
-
-			err = maybe_pull_tail(skb,
-					      off +
-					      sizeof(struct ip_auth_hdr),
-					      MAX_IPV6_HDR_LEN);
-			if (err < 0)
-				goto out;
-
-			hp = OPT_HDR(struct ip_auth_hdr, skb, off);
-			nexthdr = hp->nexthdr;
-			off += ipv6_authlen(hp);
-			break;
-		}
-		case IPPROTO_FRAGMENT: {
-			struct frag_hdr *hp;
-
-			err = maybe_pull_tail(skb,
-					      off +
-					      sizeof(struct frag_hdr),
-					      MAX_IPV6_HDR_LEN);
-			if (err < 0)
-				goto out;
-
-			hp = OPT_HDR(struct frag_hdr, skb, off);
-
-			if (hp->frag_off & htons(IP6_OFFSET | IP6_MF))
-				fragment = true;
-
-			nexthdr = hp->nexthdr;
-			off += sizeof(struct frag_hdr);
-			break;
-		}
-		default:
-			done = true;
-			break;
-		}
-	}
-
-	err = -EPROTO;
-
-	if (!done || fragment)
-		goto out;
-
-	switch (nexthdr) {
-	case IPPROTO_TCP:
-		err = maybe_pull_tail(skb,
-				      off + sizeof(struct tcphdr),
-				      MAX_IPV6_HDR_LEN);
-		if (err < 0)
-			goto out;
-
-		if (!skb_partial_csum_set(skb, off,
-					  offsetof(struct tcphdr, check))) {
-			err = -EPROTO;
-			goto out;
-		}
-
-		if (recalculate_partial_csum)
-			tcp_hdr(skb)->check =
-				~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
-						 &ipv6_hdr(skb)->daddr,
-						 skb->len - off,
-						 IPPROTO_TCP, 0);
-		break;
-	case IPPROTO_UDP:
-		err = maybe_pull_tail(skb,
-				      off + sizeof(struct udphdr),
-				      MAX_IPV6_HDR_LEN);
-		if (err < 0)
-			goto out;
-
-		if (!skb_partial_csum_set(skb, off,
-					  offsetof(struct udphdr, check))) {
-			err = -EPROTO;
-			goto out;
-		}
-
-		if (recalculate_partial_csum)
-			udp_hdr(skb)->check =
-				~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
-						 &ipv6_hdr(skb)->daddr,
-						 skb->len - off,
-						 IPPROTO_UDP, 0);
-		break;
-	default:
-		goto out;
-	}
-
-	err = 0;
-
-out:
-	return err;
-}
-
 static int checksum_setup(struct xenvif *vif, struct sk_buff *skb)
 {
-	int err = -EPROTO;
-	int recalculate_partial_csum = 0;
+	bool recalculate_partial_csum = false;
 
 	/* A GSO SKB must be CHECKSUM_PARTIAL. However some buggy
 	 * peers can fail to set NETRXF_csum_blank when sending a GSO
@@ -1308,19 +1059,14 @@ static int checksum_setup(struct xenvif *vif, struct sk_buff *skb)
 	if (skb->ip_summed != CHECKSUM_PARTIAL && skb_is_gso(skb)) {
 		vif->rx_gso_checksum_fixup++;
 		skb->ip_summed = CHECKSUM_PARTIAL;
-		recalculate_partial_csum = 1;
+		recalculate_partial_csum = true;
 	}
 
 	/* A non-CHECKSUM_PARTIAL SKB does not require setup. */
 	if (skb->ip_summed != CHECKSUM_PARTIAL)
 		return 0;
 
-	if (skb->protocol == htons(ETH_P_IP))
-		err = checksum_setup_ip(vif, skb, recalculate_partial_csum);
-	else if (skb->protocol == htons(ETH_P_IPV6))
-		err = checksum_setup_ipv6(vif, skb, recalculate_partial_csum);
-
-	return err;
+	return skb_checksum_setup(skb, recalculate_partial_csum);
 }
 
 static bool tx_credit_exceeded(struct xenvif *vif, unsigned size)
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH -next] ip_tunnel: fix sparse non static symbol warning
From: Wei Yongjun @ 2014-01-08 13:59 UTC (permalink / raw)
  To: davem, kuznet, jmorris, yoshfuji, kaber, therbert; +Cc: yongjun_wei, netdev

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

Fixes the following sparse warning:

net/ipv4/ip_tunnel.c:116:18: warning:
 symbol 'tunnel_dst_check' was not declared. Should it be static?

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 net/ipv4/ip_tunnel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index 07a5ed3..d3929a6 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -113,7 +113,7 @@ static inline struct dst_entry *tunnel_dst_get(struct ip_tunnel *t)
 	return dst;
 }
 
-struct dst_entry *tunnel_dst_check(struct ip_tunnel *t, u32 cookie)
+static struct dst_entry *tunnel_dst_check(struct ip_tunnel *t, u32 cookie)
 {
 	struct dst_entry *dst = tunnel_dst_get(t);
 

^ permalink raw reply related

* Re: IPv6: Bug in net-next
From: Damien Wyart @ 2014-01-08 13:54 UTC (permalink / raw)
  To: François-Xavier Le Bail; +Cc: netdev, davem
In-Reply-To: <1389188139.44648.YahooMailBasic@web125506.mail.ne1.yahoo.com>

> > > When I execute this code and press Crtl-C, all the IPv6 Link-Layer
> > > addresses are deleted.

> It seems that the valid lifetime and preferred lft are set to 0 sec
> for autoconfigured LLA.

I am also seeing the problem with Linus' up-to-date tree (3.13-rc7+).

On eth0, LLA is not present after boot, as well as ::1 (Host scope) on
the loopback (which causes some daemons to not start properly as they
try to listen on ::1). Manually setting these adresses with ip addr
works, they do not disappear afterwards.

-- 
Damien

^ permalink raw reply

* Re: IPv6: Bug in net-next
From: Hannes Frederic Sowa @ 2014-01-08 14:04 UTC (permalink / raw)
  To: François-Xavier Le Bail; +Cc: netdev, davem
In-Reply-To: <1389188139.44648.YahooMailBasic@web125506.mail.ne1.yahoo.com>

On Wed, Jan 08, 2014 at 05:35:39AM -0800, François-Xavier Le Bail wrote:
> On Wed, 1/8/14, François-Xavier Le Bail <fx.lebail@yahoo.com> wrote:
> 
> > > I think there is a bug in actual net-next.
>   
> > > When I execute this code and press Crtl-C, all the IPv6
> > > Link-Layer addresses are deleted.
>   
> > > This happened between
> > > c1ddf295f5183a5189196a8035546842caa2055a and HEAD.
>   
> > > [...]
>  
> > Sorry, it is not linked to this code.
> > I'm looking for the reason ...
>  
> It seems that the valid lifetime and preferred lft are set to 0 sec for autoconfigured LLA.

Grrr, I guess we should switch in addrconf_add_linklocal to call ipv6_add_addr
with INFINITY_LIFE_TIME preferred and valid lft. IFA_PERMANENT may now expire,
too since commit fad8da3e0 ("ipv6 addrconf: fix preferred lifetime
state-changing behavior while valid_lft is infinity").

Greetings,

  Hannes

^ permalink raw reply

* Re: [PATCH] netfilter: nf_conntrack: fix RCU race in nf_conntrack_find_get
From: Florian Westphal @ 2014-01-08 14:04 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Florian Westphal, Andrey Vagin, netfilter-devel, netfilter,
	coreteam, netdev, linux-kernel, vvs, Pablo Neira Ayuso,
	Patrick McHardy, Jozsef Kadlecsik, David S. Miller,
	Cyrill Gorcunov
In-Reply-To: <1389188536.26646.84.camel@edumazet-glaptop2.roam.corp.google.com>

Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > This will also set up a null-binding when no matching SNAT/DNAT/MASQERUADE
> > rule existed.
> > 
> > The manipulations of the skb->nfct->ext nat area are performed without
> > a lock.  Concurrent access is supposedly impossible as the conntrack
> > should not (yet) be in the hash table.
> > 
> > The confirmed bit is set right before we insert the conntrack into
> > the hash table (after we traversed rules, ct is ready to be
> > 'published').
> > 
> > i.e. when the confirmed bit is NOT set we should not be 'seeing' the nf_conn
> > struct when we perform the lookup, as it should still be sitting on the
> > 'unconfirmed' list, being invisible to readers.
> > 
> > Does that explanation make sense to you?
> > 
> > Thanks for looking into this.
> 
> Still, this patch adds a loop. And maybe an infinite one if confirmed
> bit is set from an context that was interrupted by this one.

Hmm.  There should be at most one retry.

The confirmed bit should always be set here.
If it isn't then this conntrack shouldn't be in the hash table, i.e.
when we re-try we should find the same conntrack again with the bit set.

Asuming the other cpu git interrupted after setting confirmed
bit but before inserting it into the hash table, then our re-try
should not be able find a matching entry.

Maybe I am missing something, but I don't see how we could (upon
retry) find the very same entry again with the bit still not set.

> If you need to test the confirmed bit, then you also need to test it
> before taking the refcount.

I don't think that would make sense, because it should always be
set (inserting conntrack into hash table without confirmed set is
illegal, and it is never unset again).

[ when allocating a new conntrack, ct->status is zeroed, which also
  clears the flag.  This happens just before we set the new objects
  refcount to 1 ]

^ permalink raw reply

* Re: [PATCH net-next v3 1/9] xen-netback: Introduce TX grant map definitions
From: Zoltan Kiss @ 2014-01-08 14:08 UTC (permalink / raw)
  To: David Miller
  Cc: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
	jonathan.davies
In-Reply-To: <20140107.202951.729942261773265015.davem@davemloft.net>

On 08/01/14 01:29, David Miller wrote:
>> +static inline int tx_dealloc_work_todo(struct xenvif *vif)
>
> Make this return bool.
Done, also in the last patch.

>> +		wait_event_interruptible(vif->dealloc_wq,
>> +					tx_dealloc_work_todo(vif) ||
>> +					 kthread_should_stop());
>
> Inconsistent indentation.  You should make the arguments line up at
> exactly the first column after the openning parenthesis of the function
> call.
Done, thanks.

Zoli

^ permalink raw reply

* Re: [PATCH net-next v3 6/9] xen-netback: Handle guests with too many frags
From: Zoltan Kiss @ 2014-01-08 14:13 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
	jonathan.davies
In-Reply-To: <1389189272.26646.89.camel@edumazet-glaptop2.roam.corp.google.com>

On 08/01/14 13:54, Eric Dumazet wrote:
> On Wed, 2014-01-08 at 13:49 +0000, Zoltan Kiss wrote:
>> On 08/01/14 02:12, Eric Dumazet wrote:
>>> On Wed, 2014-01-08 at 00:10 +0000, Zoltan Kiss wrote:
>>>
>>>>
>>>> +		if (skb_shinfo(skb)->frag_list) {
>>>> +			nskb = skb_shinfo(skb)->frag_list;
>>>> +			xenvif_fill_frags(vif, nskb, INVALID_PENDING_IDX);
>>>> +			skb->len += nskb->len;
>>>> +			skb->data_len += nskb->len;
>>>> +			skb->truesize += nskb->truesize;
>>>> +			skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
>>>> +			skb_shinfo(nskb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
>>>> +			vif->tx_zerocopy_sent += 2;
>>>> +			nskb = skb;
>>>> +
>>>> +			skb = skb_copy_expand(skb,
>>>> +					0,
>>>> +					0,
>>>> +					GFP_ATOMIC | __GFP_NOWARN);
>>>
>>> skb can be NULL here
>>
>> Thanks, fixed that.
>
> BTW, I am not sure why you copy the skb.
>
> Is it to get rid of frag_list, and why ?

Yes, it is to get rid of the frag_list, just to be on the safe side. I'm 
not sure if it is normal to send a big skb with MAX_SKB_FRAGS frags plus 
an empty skb on the frag_list with one frag, so I just consolidate them 
here. This scenario shouldn't happen very often anyway, even guests 
which can send more than MAX_SKB_FRAGS slots tends to do it rarely.

Zoli

^ permalink raw reply

* Re: IPv6: Bug in net-next
From: Hannes Frederic Sowa @ 2014-01-08 14:13 UTC (permalink / raw)
  To: François-Xavier Le Bail, netdev, davem
In-Reply-To: <20140108140400.GH9007@order.stressinduktion.org>

On Wed, Jan 08, 2014 at 03:04:00PM +0100, Hannes Frederic Sowa wrote:
> On Wed, Jan 08, 2014 at 05:35:39AM -0800, François-Xavier Le Bail wrote:
> > On Wed, 1/8/14, François-Xavier Le Bail <fx.lebail@yahoo.com> wrote:
> > 
> > > > I think there is a bug in actual net-next.
> >   
> > > > When I execute this code and press Crtl-C, all the IPv6
> > > > Link-Layer addresses are deleted.
> >   
> > > > This happened between
> > > > c1ddf295f5183a5189196a8035546842caa2055a and HEAD.
> >   
> > > > [...]
> >  
> > > Sorry, it is not linked to this code.
> > > I'm looking for the reason ...
> >  
> > It seems that the valid lifetime and preferred lft are set to 0 sec for autoconfigured LLA.
> 
> Grrr, I guess we should switch in addrconf_add_linklocal to call ipv6_add_addr
> with INFINITY_LIFE_TIME preferred and valid lft. IFA_PERMANENT may now expire,
> too since commit fad8da3e0 ("ipv6 addrconf: fix preferred lifetime
> state-changing behavior while valid_lft is infinity").

I am testing this patch:

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 31f75ea..f7e86e3 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2528,7 +2528,8 @@ static void add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
 	struct inet6_ifaddr *ifp;
 
 	ifp = ipv6_add_addr(idev, addr, NULL, plen,
-			    scope, IFA_F_PERMANENT, 0, 0);
+			scope, IFA_F_PERMANENT,
+			INFINITY_LIFE_TIME, INFINITY_LIFE_TIME);
 	if (!IS_ERR(ifp)) {
 		spin_lock_bh(&ifp->lock);
 		ifp->flags &= ~IFA_F_TENTATIVE;
@@ -2656,7 +2657,8 @@ static void addrconf_add_linklocal(struct inet6_dev *idev, const struct in6_addr
 #endif
 
 
-	ifp = ipv6_add_addr(idev, addr, NULL, 64, IFA_LINK, addr_flags, 0, 0);
+	ifp = ipv6_add_addr(idev, addr, NULL, 64, IFA_LINK, addr_flags,
+			    INFINITY_LIFE_TIME, INFINITY_LIFE_TIME);
 	if (!IS_ERR(ifp)) {
 		addrconf_prefix_route(&ifp->addr, ifp->prefix_len, idev->dev, 0, 0);
 		addrconf_dad_start(ifp);

^ permalink raw reply related

* Re: [net-next 3/8] i40evf: core ethtool functionality
From: Ben Hutchings @ 2014-01-08 14:14 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: davem, Greg Rose, netdev, gospo, sassmann, Mitch Williams
In-Reply-To: <1388537594-24601-4-git-send-email-jeffrey.t.kirsher@intel.com>

On Tue, 2013-12-31 at 16:53 -0800, Jeff Kirsher wrote:
[...]
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c
[...]
> +#define I40EVF_QUEUE_STATS_LEN \
> +	(((struct i40evf_adapter *) \
> +		netdev_priv(netdev))->vsi_res->num_queue_pairs * 4)

netdev should be a macro parameter, not just assumed as a local
variable.

[...]
> +static int i40evf_get_settings(struct net_device *netdev,
> +			       struct ethtool_cmd *ecmd)
> +{
> +	/* In the future the VF will be able to query the PF for
> +	 * some information - for now use a dummy value
> +	 */
> +	ecmd->supported = SUPPORTED_10000baseT_Full;
> +	ecmd->autoneg = AUTONEG_DISABLE;
> +	ecmd->transceiver = XCVR_DUMMY1;
> +	ecmd->port = PORT_NONE;

This is not even consistent, as its claims that 1000BASE-T is supported
but speed/duplex are always set to 0.  Why not set supported = 0?

> +	return 0;
> +}
[...]
> +static int i40evf_get_sset_count(struct net_device *netdev, int sset)
> +{
> +	if (sset == ETH_SS_STATS)
> +		return I40EVF_STATS_LEN;
> +	else
> +		return -ENOTSUPP;

-EINVAL

> +}
[...]
> +static int i40evf_set_coalesce(struct net_device *netdev,
> +			     struct ethtool_coalesce *ec)
> +{
> +	struct i40evf_adapter *adapter = netdev_priv(netdev);
> +	struct i40e_hw *hw = &adapter->hw;
> +	struct i40e_vsi *vsi = &adapter->vsi;
> +	struct i40e_q_vector *q_vector;
> +	int i;
> +
> +	if (ec->tx_max_coalesced_frames || ec->rx_max_coalesced_frames)
> +		vsi->work_limit = ec->tx_max_coalesced_frames;

Why is the actual value of ec->rx_max_coalesced_frames ignored here?
Should this be min() or max() of the two fields?

> +	switch (ec->rx_coalesce_usecs) {
> +	case 0:
> +		vsi->rx_itr_setting = 0;
> +		break;
> +	case 1:
> +		vsi->rx_itr_setting = (I40E_ITR_DYNAMIC
> +				       | ITR_REG_TO_USEC(I40E_ITR_RX_DEF));
> +		break;

This looks a bit magic; why is 1 us interpreted as 'dynamic' (adaptive?)
when there is a separate flag for enabling adaptive moderation?

[...]
> +static struct ethtool_ops i40evf_ethtool_ops = {
[...]

Should be const.

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: [net-next 4/8] i40evf: virtual channel interface
From: Ben Hutchings @ 2014-01-08 14:16 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: davem, Greg Rose, netdev, gospo, sassmann, Mitch Williams
In-Reply-To: <1388537594-24601-5-git-send-email-jeffrey.t.kirsher@intel.com>

On Tue, 2013-12-31 at 16:53 -0800, Jeff Kirsher wrote:
[...]
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
[...]
> +void i40evf_virtchnl_completion(struct i40evf_adapter *adapter,
> +				enum i40e_virtchnl_ops v_opcode,
> +				i40e_status v_retval,
> +				u8 *msg, u16 msglen)
> +{
> +	struct net_device *netdev = adapter->netdev;
> +
> +	if (v_opcode == I40E_VIRTCHNL_OP_EVENT) {
> +		struct i40e_virtchnl_pf_event *vpe =
> +			(struct i40e_virtchnl_pf_event *)msg;
> +		switch (vpe->event) {
> +		case I40E_VIRTCHNL_EVENT_LINK_CHANGE:
> +			adapter->link_up =
> +				vpe->event_data.link_event.link_status;
> +			if (adapter->link_up && !netif_carrier_ok(netdev)) {
> +				dev_info(&adapter->pdev->dev, "NIC Link is Up\n");
> +				netif_carrier_on(netdev);
> +				netif_tx_wake_all_queues(netdev);
> +			} else if (!adapter->link_up) {
> +				dev_info(&adapter->pdev->dev, "NIC Link is Down\n");
> +				netif_carrier_off(netdev);
> +				netif_tx_stop_all_queues(netdev);
> +			}
[...]

The queue start/stop here should be redundant as linkwatch will do it
automatically.

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 -next] openvswitch: Use kmem_cache_free() instead of kfree()
From: Jesse Gross @ 2014-01-08 14:17 UTC (permalink / raw)
  To: Wei Yongjun
  Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org, netdev, David Miller,
	Wei Yongjun
In-Reply-To: <CAPgLHd_jaxf4jsvUzBhE_RFioHgZzvmWhA=d_J5phUOhBGy49g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Wed, Jan 8, 2014 at 5:13 AM, Wei Yongjun <weiyj.lk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Wei Yongjun <yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY@public.gmane.org>
>
> memory allocated by kmem_cache_alloc() should be freed using
> kmem_cache_free(), not kfree().
>
> Fixes: e298e5057006 ('openvswitch: Per cpu flow stats.')
> Signed-off-by: Wei Yongjun <yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY@public.gmane.org>

Good catch, thanks.

I'll let David pick this up directly since I just flushed my patch queue.

Acked-by: Jesse Gross <jesse-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>

^ permalink raw reply

* mlx4 w/ IOMMU broken, kernel 3.12.6
From: David Lamparter @ 2014-01-08 13:49 UTC (permalink / raw)
  To: Amir Vadai; +Cc: netdev

Hi,


mlx4 is currently broken when an IOMMU is enabled, the driver does not
seem to set up regions correctly (and also, the card seems to access things
before its driver is loaded):

(This is the in-kernel driver.  The separately distributed 1.5.10
Mellanox driver does not seem to build against kernel 3.12.6.)

[    1.897271] IOMMU 0 0xfbffc000: using Queued invalidation
[    1.897569] IOMMU: Setting RMRR:
[    1.897872] IOMMU: Setting identity map for device 0000:00:1d.0 [0x7dea2000 - 0x7deaefff]
[    1.898415] IOMMU: Setting identity map for device 0000:00:1a.0 [0x7dea2000 - 0x7deaefff]
[    1.898930] IOMMU: Prepare 0-16MiB unity mapping for LPC
[    1.899217] IOMMU: Setting identity map for device 0000:00:1f.0 [0x0 - 0xffffff]
[    1.899726] PCI-DMA: Intel(R) Virtualization Technology for Directed I/O
[    1.902086] dmar: DRHD: handling fault status reg 2
[    1.902362] dmar: DMAR:[DMA Read] Request device [06:00.0] fault addr 72469000 
DMAR:[fault reason 01] Present bit in root entry is clear
(...)
[    3.299985] mlx4_core: Mellanox ConnectX core driver v1.1 (Dec, 2011)
[    3.300247] mlx4_core: Initializing 0000:06:00.0
[    4.306091] dmar: DRHD: handling fault status reg 102
[    4.306359] dmar: DMAR:[DMA Read] Request device [06:00.0] fault addr 7236f000 
DMAR:[fault reason 06] PTE Read access is not set
[   14.309294] mlx4_core 0000:06:00.0: command 0x4 timed out (go bit not cleared)
[   14.309759] mlx4_core 0000:06:00.0: QUERY_FW command failed, aborting.
[   15.313896] mlx4_core: probe of 0000:06:00.0 failed with error -110

(PCI/IOMMU Host is an Intel Xeon E5-2630v2)

Since the driver hasn't been touched in net-next, I assume this issue
hasn't been fixed in the time since 3.12 was released;  my apologies if
this is incorrect.

Card information:
# lspci -vvnns 06:00.0
06:00.0 Ethernet controller [0200]: Mellanox Technologies MT27500 Family [ConnectX-3] [15b3:1003]
	Subsystem: Mellanox Technologies Device [15b3:0077]
	Control: I/O- Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort+ >SERR- <PERR- INTx-
	Interrupt: pin A routed to IRQ 42
	Region 0: Memory at fba00000 (64-bit, non-prefetchable) [size=1M]
	Region 2: Memory at 380fff000000 (64-bit, prefetchable) [size=8M]
	Expansion ROM at fb900000 [disabled] [size=1M]
	Capabilities: [40] Power Management version 3
		Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
		Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
	Capabilities: [48] Vital Product Data
pcilib: sysfs_read_vpd: read failed: Connection timed out
		Not readable
	Capabilities: [9c] MSI-X: Enable- Count=128 Masked-
		Vector table: BAR=0 offset=0007c000
		PBA: BAR=0 offset=0007d000
	Capabilities: [60] Express (v2) Endpoint, MSI 00
		DevCap:	MaxPayload 256 bytes, PhantFunc 0, Latency L0s <64ns, L1 unlimited
			ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset+
		DevCtl:	Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
			RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop- FLReset-
			MaxPayload 256 bytes, MaxReadReq 512 bytes
		DevSta:	CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr- TransPend-
		LnkCap:	Port #8, Speed 8GT/s, Width x8, ASPM L0s, Exit Latency L0s unlimited, L1 unlimited
			ClockPM- Surprise- LLActRep- BwNot-
		LnkCtl:	ASPM Disabled; RCB 64 bytes Disabled- CommClk+
			ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
		LnkSta:	Speed 8GT/s, Width x8, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
		DevCap2: Completion Timeout: Range ABCD, TimeoutDis+, LTR-, OBFF Not Supported
		DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR-, OBFF Disabled
		LnkCtl2: Target Link Speed: 8GT/s, EnterCompliance- SpeedDis-
			 Transmit Margin: Normal Operating Range, EnterModifiedCompliance- ComplianceSOS-
			 Compliance De-emphasis: -6dB
		LnkSta2: Current De-emphasis Level: -6dB, EqualizationComplete-, EqualizationPhase1-
			 EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest-
	Capabilities: [100 v1] Alternative Routing-ID Interpretation (ARI)
		ARICap:	MFVC- ACS-, Next Function: 0
		ARICtl:	MFVC- ACS-, Function Group: 0
	Capabilities: [148 v1] Device Serial Number 00-02-c9-03-00-ea-72-e0
	Capabilities: [154 v2] Advanced Error Reporting
		UESta:	DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
		UEMsk:	DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol-
		UESvrt:	DLP+ SDES- TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol-
		CESta:	RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+
		CEMsk:	RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+
		AERCap:	First Error Pointer: 00, GenCap- CGenEn- ChkCap- ChkEn-
	Capabilities: [18c v1] #19

Cheers,

David

^ permalink raw reply

* Re: single process receives own frames due to PACKET_MMAP
From: Norbert van Bolhuis @ 2014-01-08 14:18 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: Jesper Dangaard Brouer, netdev, David Miller, uaca
In-Reply-To: <52CC23F6.1070801@redhat.com>

On 01/07/14 16:57, Daniel Borkmann wrote:
> On 01/07/2014 04:46 PM, Norbert van Bolhuis wrote:
>> On 01/07/14 16:26, Daniel Borkmann wrote:
>>> On 01/07/2014 04:16 PM, Norbert van Bolhuis wrote:
>>>> On 01/07/14 15:09, Jesper Dangaard Brouer wrote:
>>>>> On Tue, 07 Jan 2014 14:16:03 +0100
>>>>> Norbert van Bolhuis<nvbolhuis@aimvalley.nl> wrote:
>>>>>> On 01/07/14 11:06, Jesper Dangaard Brouer wrote:
>>>>>>> On Tue, 07 Jan 2014 10:32:01 +0100
>>>>>>> Daniel Borkmann<dborkman@redhat.com> wrote:
>>>>>>>
>>>>>>>> On 01/06/2014 11:58 PM, Norbert van Bolhuis wrote:
>>>>>>>>>
>>>>> [...]
>>>>>>>>
>>>>>>>>> I'd say it makes no sense to make the same process receive its
>>>>>>>>> own transmitted frames on that same interface (unless its lo).
>>>>>>>
>>>>>>> Have you setup:
>>>>>>> ring->s_ll.sll_protocol = 0
>>>>>>>
>>>>>>> This is what I did in trafgen to avoid this problem.
>>>>>>>
>>>>>>> See line 55 in netsniff-ng/ring.c:
>>>>>>> https://github.com/borkmann/netsniff-ng/blob/c3602a995b21e8133c7f4fd1fb1e7e21b6a844f1/ring.c#L55
>>>>>>>
>>>>>>> Commit:
>>>>>>> https://github.com/borkmann/netsniff-ng/commit/c3602a995b21e8133c7f4fd1fb1e7e21b6a844f1
>>>>>>>
>>>>>>
>>>>>>
>>>>>> No I did not do that, I was checking my code against netsniff-ng-0.5.8-rc4.
>>>>>>
>>>>>> But I just tried it, I believe I do the same as netsniff-ng-0.5.8-rc5, but it doesn't
>>>>>> work for me. Maybe because I have an old FC14 system (kernel 2.6.35.14-106.fc14.x86_64).
>>>>>>
>>>>>> So I tried to see whether netsniff-ng-0.5.8-rc5/trafgen still makes the
>>>>>> kernel call packet_rcv() on my FC14 system. So I build and run it, but I'm not sure
>>>>>> how to (easily) check that.
>>>>>
>>>>> The easiest way is to:
>>>>> cat /proc/net/ptype
>>>>> And look if someone registered a proto handler/function: packet_rcv (or tpacket_rcv).
>>>>>
>>>>> The more exact method is, to run "perf record -a -g" and then look (at
>>>>> the result with "perf report") for a lock contention, and "expand" the
>>>>> spin_lock and see if packet_rcv() is calling this spin lock.
>>>>>
>>>>
>>>>
>>>> I checked the easy way.
>>>> Even on my old FC14 system the "protocol=0 patch" seems to make a difference
>>>> for trafgen.
>>>> Without the patch I see for each CPU in use by trafgen a "packet_rcv entry" in
>>>> /proc/net/ptype.
>>>> With the patch I see no additional "packet_rcv entry".
>>>
>>> Yes, that is expected behaviour. ;-) See more below.
>>>
>>>> It could be my Appl is wrong or maybe the "protocol=0 patch" does not help.
>>>> I think the latter, afterall my Appl has, unlike trafgen, another RX
>>>> (AF_PACKET) socket.
>>>>
>>>>
>>>>>
>>>>>> In anyway, Wireshark does capture the trafgen generated
>>>>>> frames, does that say anything ?
>>>>>
>>>>> Be careful not to start a wireshark/tcpdump, at the sametime, as this
>>>>> will slow you down.
>>>>>
>>>>>> In the future, I can at least use PACKET_QDISC_BYPASS as a "workaround".
>>>>>
>>>>> And in the future with PACKET_QDISC_BYPASS, your wireshark will not
>>>>> catch these packets, remember that.
>>>>>
>>>>
>>>>
>>>> Yes, this is why I would love to see the "protocol=0 patch" work for my Appl.
>>>>
>>>> So I will try my Appl with the latest net-next kernel to see if that makes
>>>> it work. Hopefully I can find some time in the next coming days, I will keep
>>>> you informed.
>>>
>>> As long as there's at least one single PF_PACKET receive socket open and you
>>> do not make use of PACKET_QDISC_BYPASS on your tx socket, then those packets go
>>> back the dev_queue_xmit_nit() path, even if your tx socket uses protocol=0.
>>>
>>> If you make use of PACKET_QDISC_BYPASS [1] for your particular tx socket, then
>>> packets generated by that socket will not hit the dev_queue_xmit_nit() path
>>> back to other possible rx listeners that are present on your system (w/ the
>>> side-effects for tx as described in [1]).
>>>
>>> [1] Documentation/networking/packet_mmap.txt +960
>>>
>>
>>
>> Ok, that's clear.
>>
>> But this means my PF_PACKET socket application performs worse because of
>> using PACKET_MMAP. I expected the opposit.
>>
>> Afterall my old PF_PACKET socket application (which does not use PACKET_MMAP)
>> uses only one PF_PACKET socket (for TX and RX). Because packets are never sent
>> back to the socket they originated from, my old PF_PACKET socket application
>> performs better.
>>
>> Is there a way to use one PF_PACKET socket for both TX and RX and use PACKET_MMAP ?
>
> Yep:
>
> http://thread.gmane.org/gmane.linux.network/269129/focus=269188
>
> Feel free to make a patch and add this to Documentation/networking/packet_mmap.txt
> I think could be useful for others as well.


Good, it all works fine now, though performance is still not as good as I'd hoped.

I will sent a doc patch soon.

thanks for all help!

---
Norbert.

^ permalink raw reply

* Re: [net-next 1/8] i40evf: main driver core
From: Ben Hutchings @ 2014-01-08 14:20 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: davem, Greg Rose, netdev, gospo, sassmann, Mitch Williams
In-Reply-To: <1388537594-24601-2-git-send-email-jeffrey.t.kirsher@intel.com>

On Tue, 2013-12-31 at 16:53 -0800, Jeff Kirsher wrote:
[...]
> +static int i40evf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> +{
> +	struct net_device *netdev;
> +	struct i40evf_adapter *adapter = NULL;
> +	struct i40e_hw *hw = NULL;
> +	int err, pci_using_dac;
> +
> +	err = pci_enable_device(pdev);
> +	if (err)
> +		return err;
> +
> +	if (!dma_set_mask(&pdev->dev, DMA_BIT_MASK(64))) {
> +		pci_using_dac = true;
> +		/* coherent mask for the same size will always succeed if
> +		 * dma_set_mask does
> +		 */
> +		dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64));
> +	} else if (!dma_set_mask(&pdev->dev, DMA_BIT_MASK(32))) {
> +		pci_using_dac = false;
> +		dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
> +	} else {
> +		dev_err(&pdev->dev, "%s: DMA configuration failed: %d\n",
> +			 __func__, err);
> +		err = -EIO;
> +		goto err_dma;
> +	}
[...]

Should use dma_set_mask_and_coherent().

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

* [PATCH net-next] packet: doc: how to PACKET_MMAP with one packet socket for rx and tx
From: Norbert van Bolhuis @ 2014-01-08 14:23 UTC (permalink / raw)
  To: netdev; +Cc: Daniel Borkmann


Describe how to use one AF_PACKET socket for rx and tx.

Cc: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: Norbert van Bolhuis <nvbolhuis@aimvalley.nl>
---
  Documentation/networking/packet_mmap.txt |   14 ++++++++++++++
  1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/Documentation/networking/packet_mmap.txt b/Documentation/networking/packet_mmap.txt
index 723bf3d..2503a5a 100644
--- a/Documentation/networking/packet_mmap.txt
+++ b/Documentation/networking/packet_mmap.txt
@@ -98,6 +98,11 @@ by the kernel.
  The destruction of the socket and all associated resources
  is done by a simple call to close(fd).

+As without PACKET_MMAP, it is possible to use one socket for
+capture and transmission. This can be done by mapping the
+allocated RX and TX buffer ring with a single mmap() call. See
+"Mapping and use of the circular buffer (ring)".
+
  Next I will describe PACKET_MMAP settings and its constraints,
  also the mapping of the circular buffer in the user process and
  the use of this buffer.
@@ -414,6 +419,15 @@ tp_block_size/tp_frame_size frames there will be a gap between
  the frames. This is because a frame cannot be spawn across two
  blocks.

+To use one socket for capture and transmission, the mapping of both the RX
+and TX buffer ring has to be done with one call to mmap:
+
+    rx_ring = mmap(0, size * 2, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
+    tx_ring = rx_ring + size;
+
+RX must be the first as the kernel maps the TX ring memory right after
+the RX one.
+
  At the beginning of each frame there is an status field (see
  struct tpacket_hdr). If this field is 0 means that the frame is ready
  to be used for the kernel, If not, there is a frame the user can read

^ permalink raw reply related

* Re: single process receives own frames due to PACKET_MMAP
From: Daniel Borkmann @ 2014-01-08 14:24 UTC (permalink / raw)
  To: Norbert van Bolhuis; +Cc: Jesper Dangaard Brouer, netdev, David Miller, uaca
In-Reply-To: <52CD5E37.5070104@aimvalley.nl>

On 01/08/2014 03:18 PM, Norbert van Bolhuis wrote:

>>> Is there a way to use one PF_PACKET socket for both TX and RX and use PACKET_MMAP ?
>>
>> Yep:
>>
>> http://thread.gmane.org/gmane.linux.network/269129/focus=269188
>>
>> Feel free to make a patch and add this to Documentation/networking/packet_mmap.txt
>> I think could be useful for others as well.
>
>
> Good, it all works fine now, though performance is still not as good as I'd hoped.

Does your use case allow for TPACKET_V3 or for using fanout ?

> I will sent a doc patch soon.

Sounds great, thanks!

> thanks for all help!

^ permalink raw reply

* Re: [PATCH net-next] packet: doc: how to PACKET_MMAP with one packet socket for rx and tx
From: Daniel Borkmann @ 2014-01-08 14:29 UTC (permalink / raw)
  To: Norbert van Bolhuis; +Cc: netdev
In-Reply-To: <52CD5F44.2090307@aimvalley.nl>

On 01/08/2014 03:23 PM, Norbert van Bolhuis wrote:
>
> Describe how to use one AF_PACKET socket for rx and tx.
>
> Cc: Daniel Borkmann <dborkman@redhat.com>
> Signed-off-by: Norbert van Bolhuis <nvbolhuis@aimvalley.nl>
> ---
>   Documentation/networking/packet_mmap.txt |   14 ++++++++++++++
>   1 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/networking/packet_mmap.txt b/Documentation/networking/packet_mmap.txt
> index 723bf3d..2503a5a 100644
> --- a/Documentation/networking/packet_mmap.txt
> +++ b/Documentation/networking/packet_mmap.txt
> @@ -98,6 +98,11 @@ by the kernel.
>   The destruction of the socket and all associated resources
>   is done by a simple call to close(fd).
>
> +As without PACKET_MMAP, it is possible to use one socket for
> +capture and transmission. This can be done by mapping the
> +allocated RX and TX buffer ring with a single mmap() call. See
> +"Mapping and use of the circular buffer (ring)".
> +
>   Next I will describe PACKET_MMAP settings and its constraints,
>   also the mapping of the circular buffer in the user process and
>   the use of this buffer.
> @@ -414,6 +419,15 @@ tp_block_size/tp_frame_size frames there will be a gap between
>   the frames. This is because a frame cannot be spawn across two
>   blocks.
>
> +To use one socket for capture and transmission, the mapping of both the RX
> +and TX buffer ring has to be done with one call to mmap:

As this caused some confusion in the past for some people, it would be
great to mention here in this example also:

        ...
        setsockopt(fd, SOL_PACKET, PACKET_RX_RING, &foo, sizeof(foo));
        setsockopt(fd, SOL_PACKET, PACKET_TX_RING, &bar, sizeof(bar));
        ...
> +    rx_ring = mmap(0, size * 2, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
> +    tx_ring = rx_ring + size;
> +
> +RX must be the first as the kernel maps the TX ring memory right after
> +the RX one.

Otherwise looks perfect!

>   At the beginning of each frame there is an status field (see
>   struct tpacket_hdr). If this field is 0 means that the frame is ready
>   to be used for the kernel, If not, there is a frame the user can read

^ permalink raw reply

* DSCP of ICMPv6 Echo Reply not copied from Echo Request
From: Simon Schneider @ 2014-01-08 14:34 UTC (permalink / raw)
  To: netdev


Hi,
with kernel version 2.6.33.32, I noticed for ping6 that the DSCP value of an Echo Request is not used for the corresponding Echo Reply.
 
With ICMPv4, this is the case and I think it makes sense.
 
This is not mandated by ICMP RFCs, but there are good reasons to use the same DSCP in both directions.
 
Is there a specific reason for this difference between v4 and v6?
 
Or was it even fixed in some newer kernel version?
 
best regards, Simon

^ 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