Netdev List
 help / color / mirror / Atom feed
* [PATCH] carl9170: Convert byte_rev_table uses to bitrev8
From: Wang, Yalin @ 2014-11-14  5:16 UTC (permalink / raw)
  To: 'chunkeey@googlemail.com',
	'linville@tuxdriver.com',
	'linux-wireless@vger.kernel.org',
	'netdev@vger.kernel.org',
	'linux-kernel@vger.kernel.org', 'joe@perches.com'

Use the inline function instead of directly indexing the array.

This allows some architectures with hardware instructions for bit
reversals to eliminate the array.

Signed-off-by: Joe Perches <joe@perches.com>
Signed-off-by: Yalin Wang <yalin.wang@sonymobile.com>
---
 drivers/net/wireless/ath/carl9170/phy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/carl9170/phy.c b/drivers/net/wireless/ath/carl9170/phy.c
index b80b213..dca6df1 100644
--- a/drivers/net/wireless/ath/carl9170/phy.c
+++ b/drivers/net/wireless/ath/carl9170/phy.c
@@ -994,7 +994,7 @@ static int carl9170_init_rf_bank4_pwr(struct ar9170 *ar, bool band5ghz,
 			refsel0 = 0;
 			refsel1 = 1;
 		}
-		chansel = byte_rev_table[chansel];
+		chansel = bitrev8(chansel);
 	} else {
 		if (freq == 2484) {
 			chansel = 10 + (freq - 2274) / 5;
@@ -1002,7 +1002,7 @@ static int carl9170_init_rf_bank4_pwr(struct ar9170 *ar, bool band5ghz,
 		} else
 			chansel = 16 + (freq - 2272) / 5;
 		chansel *= 4;
-		chansel = byte_rev_table[chansel];
+		chansel = bitrev8(chansel);
 	}
 
 	d1 =	chansel;
-- 
2.1.1

^ permalink raw reply related

* RE: [PATCH net-next 2/2] r8152: adjust rtl_start_rx
From: Hayes Wang @ 2014-11-14  5:14 UTC (permalink / raw)
  To: David Miller
  Cc: netdev@vger.kernel.org, nic_swsd, linux-kernel@vger.kernel.org,
	linux-usb@vger.kernel.org
In-Reply-To: <20141113.162240.1823683928052355016.davem@davemloft.net>

David Miller [mailto:davem@davemloft.net] 
> Sent: Friday, November 14, 2014 5:23 AM
[...]
> What if even the first r8152_submit_rx() fails?  What ever will cause
> any of these retries to trigger at all?

According to the patch #1 "adjust r8152_submit_rx", the
r8152_submit_rx() would add the rx to the list and schedule
the tasklet, when the error occurs. Each time the tasklet is
called, the rx_bottom() would deal with all the rx in the
list. If the actual_length isn't vaild, the rx buffer would be
submitted directly. By this way, the retries would be done.
That is, the retries would be triggered when the tasklet
is called. Therefore, any tx, rx, and tasklet scheduling
would result in the retries.

> Second, why does your patch increment 'i' with 'i++;' in the error
> break path?  You should mark the first failed entry as unallocated
> with actual_length == 0 and place it on the rx_done queue.

Because the r8152_submit_rx() would add the failed rx to
the list, I only have to deal with the remaining ones. That
is why I increase the "i", otherwise the failed one would
be added twice.

I remember the usb_submit_urb() would set actual_length
to 0, so I skip the step. I would check it again.
 
Best Regards,
Hayes

^ permalink raw reply

* RE: [PATCHv2 net 4/4] qlcnic: Implement ndo_gso_check()
From: Shahed Shaikh @ 2014-11-14  5:08 UTC (permalink / raw)
  To: Joe Stringer, netdev
  Cc: sathya.perla@emulex.com, amirv@mellanox.com,
	Dept-GE Linux NIC Dev, Tom Herbert (Partner - google),
	gerlitz.or@gmail.com, alexander.duyck@gmail.com, linux-kernel
In-Reply-To: <1415925495-59312-5-git-send-email-joestringer@nicira.com>

> -----Original Message-----
> From: Joe Stringer [mailto:joestringer@nicira.com]
> Sent: Friday, November 14, 2014 6:08 AM
> To: netdev
> Cc: sathya.perla@emulex.com; Shahed Shaikh; amirv@mellanox.com; Dept-
> GE Linux NIC Dev; Tom Herbert (Partner - google); gerlitz.or@gmail.com;
> alexander.duyck@gmail.com; linux-kernel
> Subject: [PATCHv2 net 4/4] qlcnic: Implement ndo_gso_check()
> 
> Use vxlan_gso_check() to advertise offload support for this NIC.
> 
> Signed-off-by: Joe Stringer <joestringer@nicira.com>
> ---
> v2: Refactor out vxlan helper.
> ---
>  drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c |    6 ++++++
>  1 file changed, 6 insertions(+)

Acked-by: Shahed Shaikh <shahed.shaikh@qlogic.com>

Thanks Joe.

-Shahed
> 
> diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> index f5e29f7..a913b3a 100644
> --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> @@ -503,6 +503,11 @@ static void qlcnic_del_vxlan_port(struct net_device
> *netdev,
> 
>  	adapter->flags |= QLCNIC_DEL_VXLAN_PORT;  }
> +
> +static bool qlcnic_gso_check(struct sk_buff *skb, struct net_device
> +*dev) {
> +	return vxlan_gso_check(skb);
> +}
>  #endif
> 
>  static const struct net_device_ops qlcnic_netdev_ops = { @@ -526,6 +531,7
> @@ static const struct net_device_ops qlcnic_netdev_ops = {  #ifdef
> CONFIG_QLCNIC_VXLAN
>  	.ndo_add_vxlan_port	= qlcnic_add_vxlan_port,
>  	.ndo_del_vxlan_port	= qlcnic_del_vxlan_port,
> +	.ndo_gso_check		= qlcnic_gso_check,
>  #endif
>  #ifdef CONFIG_NET_POLL_CONTROLLER
>  	.ndo_poll_controller = qlcnic_poll_controller,
> --
> 1.7.10.4

^ permalink raw reply

* Re: net-next panic in ovs call to arch_fast_hash2 since e5a2c899
From: Jay Vosburgh @ 2014-11-14  5:04 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, discuss, pshelar, ogerlitz, Hannes Frederic Sowa
In-Reply-To: <20141113.214549.1520205472319716774.davem@davemloft.net>


	[ adding Hannes to Cc, which I should've done initially ]

David Miller <davem@davemloft.net> wrote:

>From: Jay Vosburgh <jay.vosburgh@canonical.com>
>Date: Thu, 13 Nov 2014 18:15:32 -0800
>
>> 	The "have feature" function, __intel_crc4_2_hash2, does not
>> clobber %r8, and so the call does not panic on a system with
>> X86_FEATURE_XMM4_2, although I'm not sure if that's a deliberate
>> compiler action or just happenstance because __intel_crc4_2_hash2 uses
>> fewer registers than __jhash2.
>
>Perhaps alternative calls can only be used with assembler routines
>that use specific calling conventions, and they therefore generally
>don't work with C functions?

	I don't know the answer to that, but a quick search suggests
that arch_fast_hash and arch_fast_hash2 (both added by commit e5a2c899)
may be the only cases of alternative calls that aren't supplying either
single instructions or assembly language functions.

	From looking at how the alternative calls are implemented (code
patching at boot or module load time from a table stored in a special
section of the object file), I'm skeptical that the compiler could know
what's the right thing to do.

	Hannes, can you shed any light on this?

	-J

---
	-Jay Vosburgh, jay.vosburgh@canonical.com

^ permalink raw reply

* Re: net-next panic in ovs call to arch_fast_hash2 since e5a2c899
From: David Miller @ 2014-11-14  2:45 UTC (permalink / raw)
  To: jay.vosburgh; +Cc: netdev, discuss, pshelar, ogerlitz
In-Reply-To: <12086.1415931332@famine>

From: Jay Vosburgh <jay.vosburgh@canonical.com>
Date: Thu, 13 Nov 2014 18:15:32 -0800

> 	The "have feature" function, __intel_crc4_2_hash2, does not
> clobber %r8, and so the call does not panic on a system with
> X86_FEATURE_XMM4_2, although I'm not sure if that's a deliberate
> compiler action or just happenstance because __intel_crc4_2_hash2 uses
> fewer registers than __jhash2.

Perhaps alternative calls can only be used with assembler routines
that use specific calling conventions, and they therefore generally
don't work with C functions?

^ permalink raw reply

* Re: [PATCH net-next] icmp: Remove some spurious dropped packet profile hits from the ICMP path
From: Eric Dumazet @ 2014-11-14  2:17 UTC (permalink / raw)
  To: Rick Jones; +Cc: netdev, davem
In-Reply-To: <20141113225457.A3E502900805@tardy>

On Thu, 2014-11-13 at 14:54 -0800, Rick Jones wrote:
> From: Rick Jones <rick.jones2@hp.com>
> 
> If icmp_rcv() has successfully processed the incoming ICMP datagram, we
> should use consume_skb() rather than kfree_skb() because a hit on the likes
> of perf -e skb:kfree_skb is not called-for.
> 
> Signed-off-by: Rick Jones <rick.jones2@hp.com>
> 
> ---
> 
> A test system hit with a flood ping hits on perf top -e ksb:kfre_skb before
> the change and none after for the normal/success path.  The IPv6 path would
> be somewhat more ugly.  For the time being, just deal with the overlap on
> ping_rcv() between the two to avoid a possible double free of an skb.
> 
> diff --git a/include/net/ping.h b/include/net/ping.h
> index 026479b..f074060 100644
> --- a/include/net/ping.h
> +++ b/include/net/ping.h
> @@ -82,7 +82,7 @@ int  ping_common_sendmsg(int family, struct msghdr *msg, size_t len,
>  int  ping_v6_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
>  		     size_t len);
>  int  ping_queue_rcv_skb(struct sock *sk, struct sk_buff *skb);
> -void ping_rcv(struct sk_buff *skb);
> +bool ping_rcv(struct sk_buff *skb);
>  
>  #ifdef CONFIG_PROC_FS
>  struct ping_seq_afinfo {
> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> index 36b7bfa..b9f3653 100644
> --- a/net/ipv4/icmp.c
> +++ b/net/ipv4/icmp.c
> @@ -190,7 +190,7 @@ EXPORT_SYMBOL(icmp_err_convert);
>   */
>  
>  struct icmp_control {
> -	void (*handler)(struct sk_buff *skb);
> +	bool (*handler)(struct sk_buff *skb);
>  	short   error;		/* This ICMP is classed as an error message */
>  };
>  
> @@ -746,7 +746,7 @@ static bool icmp_tag_validation(int proto)
>   *	ICMP_PARAMETERPROB.
>   */
>  
> -static void icmp_unreach(struct sk_buff *skb)
> +static bool icmp_unreach(struct sk_buff *skb)
>  {
>  	const struct iphdr *iph;
>  	struct icmphdr *icmph;
> @@ -839,10 +839,11 @@ static void icmp_unreach(struct sk_buff *skb)
>  	icmp_socket_deliver(skb, info);
>  
>  out:
> -	return;
> +	return true;
>  out_err:
>  	ICMP_INC_STATS_BH(net, ICMP_MIB_INERRORS);
> -	goto out;
> +	kfree_skb(skb);
> +	return false;
>  }
>  
> 
> @@ -850,17 +851,22 @@ out_err:
>   *	Handle ICMP_REDIRECT.
>   */
>  
> -static void icmp_redirect(struct sk_buff *skb)
> +static bool icmp_redirect(struct sk_buff *skb)
>  {
>  	if (skb->len < sizeof(struct iphdr)) {
>  		ICMP_INC_STATS_BH(dev_net(skb->dev), ICMP_MIB_INERRORS);
> -		return;
> +		kfree_skb(skb);
> +		return false;
>  	}
>  
> -	if (!pskb_may_pull(skb, sizeof(struct iphdr)))
> -		return;
> +	if (!pskb_may_pull(skb, sizeof(struct iphdr))) {
> +		/* there aught to be a stat */
> +		kfree_skb(skb);
> +		return false;
> +	}
>  
>  	icmp_socket_deliver(skb, icmp_hdr(skb)->un.gateway);
> +	return true;
>  }
>  
>  /*
> @@ -875,7 +881,7 @@ static void icmp_redirect(struct sk_buff *skb)
>   *	See also WRT handling of options once they are done and working.
>   */
>  
> -static void icmp_echo(struct sk_buff *skb)
> +static bool icmp_echo(struct sk_buff *skb)
>  {
>  	struct net *net;
>  
> @@ -891,6 +897,8 @@ static void icmp_echo(struct sk_buff *skb)
>  		icmp_param.head_len	   = sizeof(struct icmphdr);
>  		icmp_reply(&icmp_param, skb);
>  	}
> +	/* should there be an ICMP stat for ignored echos? */
> +	return true;
>  }
>  
>  /*
> @@ -900,7 +908,7 @@ static void icmp_echo(struct sk_buff *skb)
>   *		  MUST be accurate to a few minutes.
>   *		  MUST be updated at least at 15Hz.
>   */
> -static void icmp_timestamp(struct sk_buff *skb)
> +static bool icmp_timestamp(struct sk_buff *skb)
>  {
>  	struct timespec tv;
>  	struct icmp_bxm icmp_param;
> @@ -927,15 +935,18 @@ static void icmp_timestamp(struct sk_buff *skb)
>  	icmp_param.data_len	   = 0;
>  	icmp_param.head_len	   = sizeof(struct icmphdr) + 12;
>  	icmp_reply(&icmp_param, skb);
> -out:
> -	return;
> +	return true;
> +
>  out_err:
>  	ICMP_INC_STATS_BH(dev_net(skb_dst(skb)->dev), ICMP_MIB_INERRORS);
> -	goto out;
> +	kfree_skb(skb);
> +	return false;
>  }
>  
> -static void icmp_discard(struct sk_buff *skb)
> +static bool icmp_discard(struct sk_buff *skb)
>  {
> +	/* pretend it was a success */
> +	return true;
>  }
>  
>  /*
> @@ -946,6 +957,7 @@ int icmp_rcv(struct sk_buff *skb)
>  	struct icmphdr *icmph;
>  	struct rtable *rt = skb_rtable(skb);
>  	struct net *net = dev_net(rt->dst.dev);
> +	bool success;
>  
>  	if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb)) {
>  		struct sec_path *sp = skb_sec_path(skb);
> @@ -1012,7 +1024,12 @@ int icmp_rcv(struct sk_buff *skb)
>  		}
>  	}
>  
> -	icmp_pointers[icmph->type].handler(skb);
> +	success = icmp_pointers[icmph->type].handler(skb);
> +
> +	if (success) 
> +		consume_skb(skb);
> +
> +	return 0;


This looks quite complicated to me.

Why are you adding kfree_skb() everywhere instead of :

	bool to_consume = icmp_pointers[icmph->type].handler(skb);
	if (ro_consume)
		consume_skb(skb);
	else
		kfree_skb(skb);

>  
>  drop:
>  	kfree_skb(skb);

^ permalink raw reply

* net-next panic in ovs call to arch_fast_hash2 since e5a2c899
From: Jay Vosburgh @ 2014-11-14  2:15 UTC (permalink / raw)
  To: netdev; +Cc: discuss, Pravin Shelar, Or Gerlitz


	I'm having an issue with recent net-next, wherein a call is now
using alternative_call, and this is apparently being mis-compiled for
the "don't have feature" case.

	I'm using gcc (Ubuntu 4.8.2-19ubuntu1) 4.8.2 on an Ubuntu 14.04
system.

	The call is in net/openvswitch/flow_table.c:flow_hash(), which
as of commit

commit e5a2c899957659cd1a9f789bc462f9c0b35f5150
Author: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date:   Wed Nov 5 00:23:04 2014 +0100

    fast_hash: avoid indirect function calls

	uses arch_fast_hash2, which is an alternative_call function,
selecting between __jhash2 and __intel_crc4_2_hash based on the
X86_FEATURE_XMM4_2:

static inline u32 arch_fast_hash2(const u32 *data, u32 len, u32 seed)
{
        u32 hash;

        alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
#ifdef CONFIG_X86_64
                         "=a" (hash), "D" (data), "S" (len), "d" (seed));
#else
                         "=a" (hash), "a" (data), "d" (len), "c" (seed));
#endif
        return hash;
}

	This is panicing on a system without X86_FEATURE_XMM4_2.

	Reverting just the above commit does make the problem go away.

	It appears that the alternative_call itself is not calling
__jhash2 correctly:

0xffffffffa01a55dd <ovs_flow_tbl_insert+0xcd>:	sub    %ecx,%esi
0xffffffffa01a55df <ovs_flow_tbl_insert+0xcf>:	lea    0x38(%r8,%rax,1),%rdi
0xffffffffa01a55e4 <ovs_flow_tbl_insert+0xd4>:	sar    $0x2,%esi
0xffffffffa01a55e7 <ovs_flow_tbl_insert+0xd7>:	callq  0xffffffff813a75c0 <__jhash2>
0xffffffffa01a55ec <ovs_flow_tbl_insert+0xdc>:	mov    %eax,0x30(%r8)
0xffffffffa01a55f0 <ovs_flow_tbl_insert+0xe0>:	mov    (%rbx),%r13
0xffffffffa01a55f3 <ovs_flow_tbl_insert+0xe3>:	mov    %r8,%rsi
0xffffffffa01a55f6 <ovs_flow_tbl_insert+0xe6>:	mov    %r13,%rdi
0xffffffffa01a55f9 <ovs_flow_tbl_insert+0xe9>:	callq  0xffffffffa01a4ba0 <table_instance_insert>

	but __jhash2 clobbers %r8 (which is not saved), resulting in a
panic on the next instruction at ovs_flow_tbl_insert+0xdc:

[   17.762419] BUG: unable to handle kernel paging request at 00000000f6cc13e5
[   17.765456] IP: [<ffffffffa01a6bec>] ovs_flow_tbl_insert+0xdc/0x1f0 [openvswi
tch]
[   17.765456] PGD b18da067 PUD 0 
[   17.765456] Oops: 0002 [#1] SMP 
[   17.765456] Modules linked in: openvswitch libcrc32c i915 video drm_kms_helpe
r coretemp kvm_intel drm kvm gpio_ich ppdev parport_pc lpc_ich i2c_algo_bit lp s
erio_raw parport mac_hid hid_generic usbhid hid psmouse r8169 mii sky2
[   17.765456] CPU: 0 PID: 901 Comm: ovs-vswitchd Not tainted 3.18.0-rc2-nn-4d3c
9d37+ #19
[   17.765456] Hardware name: LENOVO 0829F3U/To be filled by O.E.M., BIOS 90KT15
AUS 07/21/2010
[   17.765456] task: ffff8800b07c9900 ti: ffff8800b1a04000 task.ti: ffff8800b1a0
4000
[   17.765456] RIP: 0010:[<ffffffffa01a6bec>]  [<ffffffffa01a6bec>] ovs_flow_tbl
_insert+0xdc/0x1f0 [openvswitch]
[   17.765456] RSP: 0018:ffff8800b1a07798  EFLAGS: 00010293
[   17.765456] RAX: 00000000e81d0094 RBX: ffff8800b27a0b20 RCX: 000000007aa02ddf
[   17.765456] RDX: 000000005e013969 RSI: 00000000290f109c RDI: ffff880138d501a4
[   17.765456] RBP: ffff8800b1a077e8 R08: 00000000f6cc13b5 R09: 00000000748df07f
[   17.765456] R10: ffffffffa01a6c96 R11: 0000000000000004 R12: ffff8800b27a0b28
[   17.765456] R13: ffff8800b1a07850 R14: ffff8800b27a0b28 R15: ffff8800a5a99c00
[   17.765456] FS:  00007fcd60b8d980(0000) GS:ffff88013fc00000(0000) knlGS:0000000000000000
[   17.765456] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   17.765456] CR2: 00000000f6cc13e5 CR3: 0000000031846000 CR4: 00000000000407f0
[   17.765456] Stack:
[   17.765456]  ffff880138d50000 ffff8800b1a07a70 ffff880138d50000 0000000000000000
[   17.765456]  ffff880138d501c0 ffff8800b1a07a70 ffff880138d50000 0000000000000000
[   17.765456]  0000000000000000 ffff8800b27a0b20 ffff8800b1a07a38 ffffffffa019e1fe
[   17.765456] Call Trace:
[   17.765456]  [<ffffffffa019e1fe>] ovs_flow_cmd_new+0x23e/0x3c0 [openvswitch]
[   17.765456]  [<ffffffff8165f3e5>] genl_family_rcv_msg+0x1a5/0x3c0

	The "have feature" function, __intel_crc4_2_hash2, does not
clobber %r8, and so the call does not panic on a system with
X86_FEATURE_XMM4_2, although I'm not sure if that's a deliberate
compiler action or just happenstance because __intel_crc4_2_hash2 uses
fewer registers than __jhash2.

	As I said above, reverting the commit in question does resolve
the problem, but it does appear that there is a problem in the compiler
or alternative_call system that is the real root cause.

	I've discussed this with Jesse Gross <jesse@nicira.com> and
Pravin Shelar <pshelar@nicira.com>, who don't see the problem, but I
suspect that's because they have newer cpus with X86_FEATURE_XMM4_2.
Jesse, Pravin, can you confirm whether or not your test systems have
this cpu feature (it's "sse4_2" in /proc/cpuinfo's flags)?

	-J

---
	-Jay Vosburgh, jay.vosburgh@canonical.com

^ permalink raw reply

* Re: [GIT PULL nf] Second Round of IPVS Fixes for v3.18
From: Simon Horman @ 2014-11-14  1:59 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
	Julian Anastasov
In-Reply-To: <20141113113818.GA7570@salvia>

On Thu, Nov 13, 2014 at 12:38:18PM +0100, Pablo Neira Ayuso wrote:
> On Wed, Nov 12, 2014 at 11:21:59AM +0900, Simon Horman wrote:
> > Hi Pablo,
> > 
> > please consider this fix for v3.18.
> > 
> > It fixes handling of skb->sk which may cause incorrect handling
> > of connections from a local process.
> > 
> > This problem was introduced in its current form by 8052ba292559f907e
> > ("ipvs: support ipv4 in ipv6 and ipv6 in ipv4 tunnel forwarding") in
> > v3.18-rc1.
> 
> Pulled, thanks Simon.
> 
> > I believe it also exists in a different form in older kernels.
> > No fix for that is available at this time.
> 
> AFAIK -stable also accepts backports if there's a clear relation
> between this original patch in mainstream and the backported version.

Thanks. I will see about making one.

^ permalink raw reply

* [PATCH v2 net-next 7/7] bpf: remove test map scaffolding and user proper types
From: Alexei Starovoitov @ 2014-11-14  1:36 UTC (permalink / raw)
  To: David S. Miller
  Cc: Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
	Hannes Frederic Sowa, Eric Dumazet, linux-api, netdev,
	linux-kernel
In-Reply-To: <1415929010-9361-1-git-send-email-ast@plumgrid.com>

proper types and function helpers are ready. Use them in verifier testsuite.
Remove temporary stubs

Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
 kernel/bpf/test_stub.c      |   56 +++++++------------------------------------
 samples/bpf/test_verifier.c |   14 +++++------
 2 files changed, 16 insertions(+), 54 deletions(-)

diff --git a/kernel/bpf/test_stub.c b/kernel/bpf/test_stub.c
index fcaddff4003e..0ceae1e6e8b5 100644
--- a/kernel/bpf/test_stub.c
+++ b/kernel/bpf/test_stub.c
@@ -18,26 +18,18 @@ struct bpf_context {
 	u64 arg2;
 };
 
-static u64 test_func(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
-{
-	return 0;
-}
-
-static struct bpf_func_proto test_funcs[] = {
-	[BPF_FUNC_unspec] = {
-		.func = test_func,
-		.gpl_only = true,
-		.ret_type = RET_PTR_TO_MAP_VALUE_OR_NULL,
-		.arg1_type = ARG_CONST_MAP_PTR,
-		.arg2_type = ARG_PTR_TO_MAP_KEY,
-	},
-};
-
 static const struct bpf_func_proto *test_func_proto(enum bpf_func_id func_id)
 {
-	if (func_id < 0 || func_id >= ARRAY_SIZE(test_funcs))
+	switch (func_id) {
+	case BPF_FUNC_map_lookup_elem:
+		return &bpf_map_lookup_elem_proto;
+	case BPF_FUNC_map_update_elem:
+		return &bpf_map_update_elem_proto;
+	case BPF_FUNC_map_delete_elem:
+		return &bpf_map_delete_elem_proto;
+	default:
 		return NULL;
-	return &test_funcs[func_id];
+	}
 }
 
 static const struct bpf_context_access {
@@ -78,38 +70,8 @@ static struct bpf_prog_type_list tl_prog = {
 	.type = BPF_PROG_TYPE_UNSPEC,
 };
 
-static struct bpf_map *test_map_alloc(union bpf_attr *attr)
-{
-	struct bpf_map *map;
-
-	map = kzalloc(sizeof(*map), GFP_USER);
-	if (!map)
-		return ERR_PTR(-ENOMEM);
-
-	map->key_size = attr->key_size;
-	map->value_size = attr->value_size;
-	map->max_entries = attr->max_entries;
-	return map;
-}
-
-static void test_map_free(struct bpf_map *map)
-{
-	kfree(map);
-}
-
-static struct bpf_map_ops test_map_ops = {
-	.map_alloc = test_map_alloc,
-	.map_free = test_map_free,
-};
-
-static struct bpf_map_type_list tl_map = {
-	.ops = &test_map_ops,
-	.type = BPF_MAP_TYPE_UNSPEC,
-};
-
 static int __init register_test_ops(void)
 {
-	bpf_register_map_type(&tl_map);
 	bpf_register_prog_type(&tl_prog);
 	return 0;
 }
diff --git a/samples/bpf/test_verifier.c b/samples/bpf/test_verifier.c
index 63402742345e..b96175e90363 100644
--- a/samples/bpf/test_verifier.c
+++ b/samples/bpf/test_verifier.c
@@ -261,7 +261,7 @@ static struct bpf_test tests[] = {
 			BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
 			BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
 			BPF_LD_MAP_FD(BPF_REG_1, 0),
-			BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_unspec),
+			BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
 			BPF_EXIT_INSN(),
 		},
 		.fixup = {2},
@@ -417,7 +417,7 @@ static struct bpf_test tests[] = {
 			BPF_ALU64_REG(BPF_MOV, BPF_REG_2, BPF_REG_10),
 			BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
 			BPF_LD_MAP_FD(BPF_REG_1, 0),
-			BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_unspec),
+			BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_delete_elem),
 			BPF_EXIT_INSN(),
 		},
 		.errstr = "fd 0 is not pointing to valid bpf_map",
@@ -430,7 +430,7 @@ static struct bpf_test tests[] = {
 			BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
 			BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
 			BPF_LD_MAP_FD(BPF_REG_1, 0),
-			BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_unspec),
+			BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
 			BPF_ST_MEM(BPF_DW, BPF_REG_0, 0, 0),
 			BPF_EXIT_INSN(),
 		},
@@ -445,7 +445,7 @@ static struct bpf_test tests[] = {
 			BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
 			BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
 			BPF_LD_MAP_FD(BPF_REG_1, 0),
-			BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_unspec),
+			BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
 			BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 1),
 			BPF_ST_MEM(BPF_DW, BPF_REG_0, 4, 0),
 			BPF_EXIT_INSN(),
@@ -461,7 +461,7 @@ static struct bpf_test tests[] = {
 			BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
 			BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
 			BPF_LD_MAP_FD(BPF_REG_1, 0),
-			BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_unspec),
+			BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
 			BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 2),
 			BPF_ST_MEM(BPF_DW, BPF_REG_0, 0, 0),
 			BPF_EXIT_INSN(),
@@ -548,7 +548,7 @@ static struct bpf_test tests[] = {
 			BPF_ST_MEM(BPF_DW, BPF_REG_2, -56, 0),
 			BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -56),
 			BPF_LD_MAP_FD(BPF_REG_1, 0),
-			BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_unspec),
+			BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_delete_elem),
 			BPF_EXIT_INSN(),
 		},
 		.fixup = {24},
@@ -659,7 +659,7 @@ static int create_map(void)
 	long long key, value = 0;
 	int map_fd;
 
-	map_fd = bpf_create_map(BPF_MAP_TYPE_UNSPEC, sizeof(key), sizeof(value), 1024);
+	map_fd = bpf_create_map(BPF_MAP_TYPE_HASH, sizeof(key), sizeof(value), 1024);
 	if (map_fd < 0) {
 		printf("failed to create map '%s'\n", strerror(errno));
 	}
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v2 net-next 6/7] bpf: allow eBPF programs to use maps
From: Alexei Starovoitov @ 2014-11-14  1:36 UTC (permalink / raw)
  To: David S. Miller
  Cc: Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
	Hannes Frederic Sowa, Eric Dumazet, linux-api, netdev,
	linux-kernel
In-Reply-To: <1415929010-9361-1-git-send-email-ast@plumgrid.com>

expose bpf_map_lookup_elem(), bpf_map_update_elem(), bpf_map_delete_elem()
map accessors to eBPF programs

Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
Note, these helpers are exposed as '.gpl_only = false', so non-GPL eBPF programs
can use them. That was requested by AndyL and DavidL before.

 include/linux/bpf.h      |    5 +++
 include/uapi/linux/bpf.h |    3 ++
 kernel/bpf/Makefile      |    2 +-
 kernel/bpf/helpers.c     |   89 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 98 insertions(+), 1 deletion(-)
 create mode 100644 kernel/bpf/helpers.c

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 51e9242e4803..75e94eaa228b 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -133,4 +133,9 @@ struct bpf_prog *bpf_prog_get(u32 ufd);
 /* verify correctness of eBPF program */
 int bpf_check(struct bpf_prog *fp, union bpf_attr *attr);
 
+/* verifier prototypes for helper functions called from eBPF programs */
+extern struct bpf_func_proto bpf_map_lookup_elem_proto;
+extern struct bpf_func_proto bpf_map_update_elem_proto;
+extern struct bpf_func_proto bpf_map_delete_elem_proto;
+
 #endif /* _LINUX_BPF_H */
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 0d662fe75df5..4a3d0f84f178 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -158,6 +158,9 @@ union bpf_attr {
  */
 enum bpf_func_id {
 	BPF_FUNC_unspec,
+	BPF_FUNC_map_lookup_elem, /* void *map_lookup_elem(&map, &key) */
+	BPF_FUNC_map_update_elem, /* int map_update_elem(&map, &key, &value, flags) */
+	BPF_FUNC_map_delete_elem, /* int map_delete_elem(&map, &key) */
 	__BPF_FUNC_MAX_ID,
 };
 
diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
index 72ec98ba2d42..a5ae60f0b0a2 100644
--- a/kernel/bpf/Makefile
+++ b/kernel/bpf/Makefile
@@ -1,5 +1,5 @@
 obj-y := core.o
-obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o hashtab.o arraymap.o
+obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o hashtab.o arraymap.o helpers.o
 ifdef CONFIG_TEST_BPF
 obj-$(CONFIG_BPF_SYSCALL) += test_stub.o
 endif
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
new file mode 100644
index 000000000000..9e3414d85459
--- /dev/null
+++ b/kernel/bpf/helpers.c
@@ -0,0 +1,89 @@
+/* Copyright (c) 2011-2014 PLUMgrid, http://plumgrid.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+#include <linux/bpf.h>
+#include <linux/rcupdate.h>
+
+/* If kernel subsystem is allowing eBPF programs to call this function,
+ * inside its own verifier_ops->get_func_proto() callback it should return
+ * bpf_map_lookup_elem_proto, so that verifier can properly check the arguments
+ *
+ * Different map implementations will rely on rcu in map methods
+ * lookup/update/delete, therefore eBPF programs must run under rcu lock
+ * if program is allowed to access maps, so check rcu_read_lock_held in
+ * all three functions.
+ */
+static u64 bpf_map_lookup_elem(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
+{
+	/* verifier checked that R1 contains a valid pointer to bpf_map
+	 * and R2 points to a program stack and map->key_size bytes were
+	 * initialized
+	 */
+	struct bpf_map *map = (struct bpf_map *) (unsigned long) r1;
+	void *key = (void *) (unsigned long) r2;
+	void *value;
+
+	WARN_ON_ONCE(!rcu_read_lock_held());
+
+	value = map->ops->map_lookup_elem(map, key);
+
+	/* lookup() returns either pointer to element value or NULL
+	 * which is the meaning of PTR_TO_MAP_VALUE_OR_NULL type
+	 */
+	return (unsigned long) value;
+}
+
+struct bpf_func_proto bpf_map_lookup_elem_proto = {
+	.func = bpf_map_lookup_elem,
+	.gpl_only = false,
+	.ret_type = RET_PTR_TO_MAP_VALUE_OR_NULL,
+	.arg1_type = ARG_CONST_MAP_PTR,
+	.arg2_type = ARG_PTR_TO_MAP_KEY,
+};
+
+static u64 bpf_map_update_elem(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
+{
+	struct bpf_map *map = (struct bpf_map *) (unsigned long) r1;
+	void *key = (void *) (unsigned long) r2;
+	void *value = (void *) (unsigned long) r3;
+
+	WARN_ON_ONCE(!rcu_read_lock_held());
+
+	return map->ops->map_update_elem(map, key, value, r4);
+}
+
+struct bpf_func_proto bpf_map_update_elem_proto = {
+	.func = bpf_map_update_elem,
+	.gpl_only = false,
+	.ret_type = RET_INTEGER,
+	.arg1_type = ARG_CONST_MAP_PTR,
+	.arg2_type = ARG_PTR_TO_MAP_KEY,
+	.arg3_type = ARG_PTR_TO_MAP_VALUE,
+	.arg4_type = ARG_ANYTHING,
+};
+
+static u64 bpf_map_delete_elem(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
+{
+	struct bpf_map *map = (struct bpf_map *) (unsigned long) r1;
+	void *key = (void *) (unsigned long) r2;
+
+	WARN_ON_ONCE(!rcu_read_lock_held());
+
+	return map->ops->map_delete_elem(map, key);
+}
+
+struct bpf_func_proto bpf_map_delete_elem_proto = {
+	.func = bpf_map_delete_elem,
+	.gpl_only = false,
+	.ret_type = RET_INTEGER,
+	.arg1_type = ARG_CONST_MAP_PTR,
+	.arg2_type = ARG_PTR_TO_MAP_KEY,
+};
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v2 net-next 5/7] bpf: add a testsuite for eBPF maps
From: Alexei Starovoitov @ 2014-11-14  1:36 UTC (permalink / raw)
  To: David S. Miller
  Cc: Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
	Hannes Frederic Sowa, Eric Dumazet, linux-api, netdev,
	linux-kernel
In-Reply-To: <1415929010-9361-1-git-send-email-ast@plumgrid.com>

. check error conditions and sanity of hash and array map APIs
. check large maps (that kernel gracefully switches to vmalloc from kmalloc)
. check multi-process parallel access and stress test

Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
Eventually it can be moved tools/testing/selftests/bpf/, but for now keep
it in samples/bpf/, since that's where all subsequent samples are coming to.

 samples/bpf/Makefile    |    3 +-
 samples/bpf/libbpf.c    |    3 +-
 samples/bpf/libbpf.h    |    2 +-
 samples/bpf/test_maps.c |  291 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 296 insertions(+), 3 deletions(-)
 create mode 100644 samples/bpf/test_maps.c

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 634391797856..0718d9ce4619 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -2,9 +2,10 @@
 obj- := dummy.o
 
 # List of programs to build
-hostprogs-y := test_verifier
+hostprogs-y := test_verifier test_maps
 
 test_verifier-objs := test_verifier.o libbpf.o
+test_maps-objs := test_maps.o libbpf.o
 
 # Tell kbuild to always build the programs
 always := $(hostprogs-y)
diff --git a/samples/bpf/libbpf.c b/samples/bpf/libbpf.c
index ff6504420738..17bb520eb57f 100644
--- a/samples/bpf/libbpf.c
+++ b/samples/bpf/libbpf.c
@@ -27,12 +27,13 @@ int bpf_create_map(enum bpf_map_type map_type, int key_size, int value_size,
 	return syscall(__NR_bpf, BPF_MAP_CREATE, &attr, sizeof(attr));
 }
 
-int bpf_update_elem(int fd, void *key, void *value)
+int bpf_update_elem(int fd, void *key, void *value, unsigned long long flags)
 {
 	union bpf_attr attr = {
 		.map_fd = fd,
 		.key = ptr_to_u64(key),
 		.value = ptr_to_u64(value),
+		.flags = flags,
 	};
 
 	return syscall(__NR_bpf, BPF_MAP_UPDATE_ELEM, &attr, sizeof(attr));
diff --git a/samples/bpf/libbpf.h b/samples/bpf/libbpf.h
index 8a31babeca5d..f8678e5f48bf 100644
--- a/samples/bpf/libbpf.h
+++ b/samples/bpf/libbpf.h
@@ -6,7 +6,7 @@ struct bpf_insn;
 
 int bpf_create_map(enum bpf_map_type map_type, int key_size, int value_size,
 		   int max_entries);
-int bpf_update_elem(int fd, void *key, void *value);
+int bpf_update_elem(int fd, void *key, void *value, unsigned long long flags);
 int bpf_lookup_elem(int fd, void *key, void *value);
 int bpf_delete_elem(int fd, void *key);
 int bpf_get_next_key(int fd, void *key, void *next_key);
diff --git a/samples/bpf/test_maps.c b/samples/bpf/test_maps.c
new file mode 100644
index 000000000000..e286b42307f3
--- /dev/null
+++ b/samples/bpf/test_maps.c
@@ -0,0 +1,291 @@
+/*
+ * Testsuite for eBPF maps
+ *
+ * Copyright (c) 2014 PLUMgrid, http://plumgrid.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ */
+#include <stdio.h>
+#include <unistd.h>
+#include <linux/bpf.h>
+#include <errno.h>
+#include <string.h>
+#include <assert.h>
+#include <sys/wait.h>
+#include <stdlib.h>
+#include "libbpf.h"
+
+/* sanity tests for map API */
+static void test_hashmap_sanity(int i, void *data)
+{
+	long long key, next_key, value;
+	int map_fd;
+
+	map_fd = bpf_create_map(BPF_MAP_TYPE_HASH, sizeof(key), sizeof(value), 2);
+	if (map_fd < 0) {
+		printf("failed to create hashmap '%s'\n", strerror(errno));
+		exit(1);
+	}
+
+	key = 1;
+	value = 1234;
+	/* insert key=1 element */
+	assert(bpf_update_elem(map_fd, &key, &value, BPF_ANY) == 0);
+
+	value = 0;
+	/* BPF_NOEXIST means: add new element if it doesn't exist */
+	assert(bpf_update_elem(map_fd, &key, &value, BPF_NOEXIST) == -1 &&
+	       /* key=1 already exists */
+	       errno == EEXIST);
+
+	assert(bpf_update_elem(map_fd, &key, &value, -1) == -1 && errno == EINVAL);
+
+	/* check that key=1 can be found */
+	assert(bpf_lookup_elem(map_fd, &key, &value) == 0 && value == 1234);
+
+	key = 2;
+	/* check that key=2 is not found */
+	assert(bpf_lookup_elem(map_fd, &key, &value) == -1 && errno == ENOENT);
+
+	/* BPF_EXIST means: update existing element */
+	assert(bpf_update_elem(map_fd, &key, &value, BPF_EXIST) == -1 &&
+	       /* key=2 is not there */
+	       errno == ENOENT);
+
+	/* insert key=2 element */
+	assert(bpf_update_elem(map_fd, &key, &value, BPF_NOEXIST) == 0);
+
+	/* key=1 and key=2 were inserted, check that key=0 cannot be inserted
+	 * due to max_entries limit
+	 */
+	key = 0;
+	assert(bpf_update_elem(map_fd, &key, &value, BPF_NOEXIST) == -1 &&
+	       errno == E2BIG);
+
+	/* check that key = 0 doesn't exist */
+	assert(bpf_delete_elem(map_fd, &key) == -1 && errno == ENOENT);
+
+	/* iterate over two elements */
+	assert(bpf_get_next_key(map_fd, &key, &next_key) == 0 &&
+	       next_key == 2);
+	assert(bpf_get_next_key(map_fd, &next_key, &next_key) == 0 &&
+	       next_key == 1);
+	assert(bpf_get_next_key(map_fd, &next_key, &next_key) == -1 &&
+	       errno == ENOENT);
+
+	/* delete both elements */
+	key = 1;
+	assert(bpf_delete_elem(map_fd, &key) == 0);
+	key = 2;
+	assert(bpf_delete_elem(map_fd, &key) == 0);
+	assert(bpf_delete_elem(map_fd, &key) == -1 && errno == ENOENT);
+
+	key = 0;
+	/* check that map is empty */
+	assert(bpf_get_next_key(map_fd, &key, &next_key) == -1 &&
+	       errno == ENOENT);
+	close(map_fd);
+}
+
+static void test_arraymap_sanity(int i, void *data)
+{
+	int key, next_key, map_fd;
+	long long value;
+
+	map_fd = bpf_create_map(BPF_MAP_TYPE_ARRAY, sizeof(key), sizeof(value), 2);
+	if (map_fd < 0) {
+		printf("failed to create arraymap '%s'\n", strerror(errno));
+		exit(1);
+	}
+
+	key = 1;
+	value = 1234;
+	/* insert key=1 element */
+	assert(bpf_update_elem(map_fd, &key, &value, BPF_ANY) == 0);
+
+	value = 0;
+	assert(bpf_update_elem(map_fd, &key, &value, BPF_NOEXIST) == -1 &&
+	       errno == EEXIST);
+
+	/* check that key=1 can be found */
+	assert(bpf_lookup_elem(map_fd, &key, &value) == 0 && value == 1234);
+
+	key = 0;
+	/* check that key=0 is also found and zero initialized */
+	assert(bpf_lookup_elem(map_fd, &key, &value) == 0 && value == 0);
+
+
+	/* key=0 and key=1 were inserted, check that key=2 cannot be inserted
+	 * due to max_entries limit
+	 */
+	key = 2;
+	assert(bpf_update_elem(map_fd, &key, &value, BPF_EXIST) == -1 &&
+	       errno == E2BIG);
+
+	/* check that key = 2 doesn't exist */
+	assert(bpf_lookup_elem(map_fd, &key, &value) == -1 && errno == ENOENT);
+
+	/* iterate over two elements */
+	assert(bpf_get_next_key(map_fd, &key, &next_key) == 0 &&
+	       next_key == 0);
+	assert(bpf_get_next_key(map_fd, &next_key, &next_key) == 0 &&
+	       next_key == 1);
+	assert(bpf_get_next_key(map_fd, &next_key, &next_key) == -1 &&
+	       errno == ENOENT);
+
+	/* delete shouldn't succeed */
+	key = 1;
+	assert(bpf_delete_elem(map_fd, &key) == -1 && errno == EINVAL);
+
+	close(map_fd);
+}
+
+#define MAP_SIZE (32 * 1024)
+static void test_map_large(void)
+{
+	struct bigkey {
+		int a;
+		char b[116];
+		long long c;
+	} key;
+	int map_fd, i, value;
+
+	/* allocate 4Mbyte of memory */
+	map_fd = bpf_create_map(BPF_MAP_TYPE_HASH, sizeof(key), sizeof(value),
+				MAP_SIZE);
+	if (map_fd < 0) {
+		printf("failed to create large map '%s'\n", strerror(errno));
+		exit(1);
+	}
+
+	for (i = 0; i < MAP_SIZE; i++) {
+		key = (struct bigkey) {.c = i};
+		value = i;
+		assert(bpf_update_elem(map_fd, &key, &value, BPF_NOEXIST) == 0);
+	}
+	key.c = -1;
+	assert(bpf_update_elem(map_fd, &key, &value, BPF_NOEXIST) == -1 &&
+	       errno == E2BIG);
+
+	/* iterate through all elements */
+	for (i = 0; i < MAP_SIZE; i++)
+		assert(bpf_get_next_key(map_fd, &key, &key) == 0);
+	assert(bpf_get_next_key(map_fd, &key, &key) == -1 && errno == ENOENT);
+
+	key.c = 0;
+	assert(bpf_lookup_elem(map_fd, &key, &value) == 0 && value == 0);
+	key.a = 1;
+	assert(bpf_lookup_elem(map_fd, &key, &value) == -1 && errno == ENOENT);
+
+	close(map_fd);
+}
+
+/* fork N children and wait for them to complete */
+static void run_parallel(int tasks, void (*fn)(int i, void *data), void *data)
+{
+	pid_t pid[tasks];
+	int i;
+
+	for (i = 0; i < tasks; i++) {
+		pid[i] = fork();
+		if (pid[i] == 0) {
+			fn(i, data);
+			exit(0);
+		} else if (pid[i] == -1) {
+			printf("couldn't spawn #%d process\n", i);
+			exit(1);
+		}
+	}
+	for (i = 0; i < tasks; i++) {
+		int status;
+
+		assert(waitpid(pid[i], &status, 0) == pid[i]);
+		assert(status == 0);
+	}
+}
+
+static void test_map_stress(void)
+{
+	run_parallel(100, test_hashmap_sanity, NULL);
+	run_parallel(100, test_arraymap_sanity, NULL);
+}
+
+#define TASKS 1024
+#define DO_UPDATE 1
+#define DO_DELETE 0
+static void do_work(int fn, void *data)
+{
+	int map_fd = ((int *)data)[0];
+	int do_update = ((int *)data)[1];
+	int i;
+	int key, value;
+
+	for (i = fn; i < MAP_SIZE; i += TASKS) {
+		key = value = i;
+		if (do_update)
+			assert(bpf_update_elem(map_fd, &key, &value, BPF_NOEXIST) == 0);
+		else
+			assert(bpf_delete_elem(map_fd, &key) == 0);
+	}
+}
+
+static void test_map_parallel(void)
+{
+	int i, map_fd, key = 0, value = 0;
+	int data[2];
+
+	map_fd = bpf_create_map(BPF_MAP_TYPE_HASH, sizeof(key), sizeof(value),
+				MAP_SIZE);
+	if (map_fd < 0) {
+		printf("failed to create map for parallel test '%s'\n",
+		       strerror(errno));
+		exit(1);
+	}
+
+	data[0] = map_fd;
+	data[1] = DO_UPDATE;
+	/* use the same map_fd in children to add elements to this map
+	 * child_0 adds key=0, key=1024, key=2048, ...
+	 * child_1 adds key=1, key=1025, key=2049, ...
+	 * child_1023 adds key=1023, ...
+	 */
+	run_parallel(TASKS, do_work, data);
+
+	/* check that key=0 is already there */
+	assert(bpf_update_elem(map_fd, &key, &value, BPF_NOEXIST) == -1 &&
+	       errno == EEXIST);
+
+	/* check that all elements were inserted */
+	key = -1;
+	for (i = 0; i < MAP_SIZE; i++)
+		assert(bpf_get_next_key(map_fd, &key, &key) == 0);
+	assert(bpf_get_next_key(map_fd, &key, &key) == -1 && errno == ENOENT);
+
+	/* another check for all elements */
+	for (i = 0; i < MAP_SIZE; i++) {
+		key = MAP_SIZE - i - 1;
+		assert(bpf_lookup_elem(map_fd, &key, &value) == 0 &&
+		       value == key);
+	}
+
+	/* now let's delete all elemenets in parallel */
+	data[1] = DO_DELETE;
+	run_parallel(TASKS, do_work, data);
+
+	/* nothing should be left */
+	key = -1;
+	assert(bpf_get_next_key(map_fd, &key, &key) == -1 && errno == ENOENT);
+}
+
+int main(void)
+{
+	test_hashmap_sanity(0, NULL);
+	test_arraymap_sanity(0, NULL);
+	test_map_large();
+	test_map_parallel();
+	test_map_stress();
+	printf("test_maps: OK\n");
+	return 0;
+}
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v2 net-next 4/7] bpf: fix BPF_MAP_LOOKUP_ELEM command return code
From: Alexei Starovoitov @ 2014-11-14  1:36 UTC (permalink / raw)
  To: David S. Miller
  Cc: Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
	Hannes Frederic Sowa, Eric Dumazet, linux-api, netdev,
	linux-kernel
In-Reply-To: <1415929010-9361-1-git-send-email-ast@plumgrid.com>

fix errno of BPF_MAP_LOOKUP_ELEM command as bpf manpage
described it in commit b4fc1a460f30("Merge branch 'bpf-next'"):
-----
BPF_MAP_LOOKUP_ELEM
    int bpf_lookup_elem(int fd, void *key, void *value)
    {
        union bpf_attr attr = {
            .map_fd = fd,
            .key = ptr_to_u64(key),
            .value = ptr_to_u64(value),
        };

        return bpf(BPF_MAP_LOOKUP_ELEM, &attr, sizeof(attr));
    }
    bpf() syscall looks up an element with given key in  a  map  fd.
    If  element  is found it returns zero and stores element's value
    into value.  If element is not found  it  returns  -1  and  sets
    errno to ENOENT.

and further down in manpage:

   ENOENT For BPF_MAP_LOOKUP_ELEM or BPF_MAP_DELETE_ELEM,  indicates  that
          element with given key was not found.
-----

In general all BPF commands return ENOENT when map element is not found
(including BPF_MAP_GET_NEXT_KEY and BPF_MAP_UPDATE_ELEM with
 flags == BPF_MAP_UPDATE_ONLY)

Subsequent patch adds a testsuite to check return values for all of
these combinations.

Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---

I don't think this patch is needed for 'net', since 'net' has syscall shell
only. Actual map types and their implementations are being introduced by
this set of patches.

 kernel/bpf/syscall.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index c0d03bf317a2..088ac0b1b106 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -169,7 +169,7 @@ static int map_lookup_elem(union bpf_attr *attr)
 	if (copy_from_user(key, ukey, map->key_size) != 0)
 		goto free_key;
 
-	err = -ESRCH;
+	err = -ENOENT;
 	rcu_read_lock();
 	value = map->ops->map_lookup_elem(map, key);
 	if (!value)
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v2 net-next 3/7] bpf: add array type of eBPF maps
From: Alexei Starovoitov @ 2014-11-14  1:36 UTC (permalink / raw)
  To: David S. Miller
  Cc: Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
	Hannes Frederic Sowa, Eric Dumazet, linux-api, netdev,
	linux-kernel
In-Reply-To: <1415929010-9361-1-git-send-email-ast@plumgrid.com>

add new map type BPF_MAP_TYPE_ARRAY and its implementation

- optimized for fastest possible lookup()
  . in the future verifier/JIT may recognize lookup() with constant key
    and optimize it into constant pointer. Can optimize non-constant
    key into direct pointer arithmetic as well, since pointers and
    value_size are constant for the life of the eBPF program.
    In other words array_map_lookup_elem() may be 'inlined' by verifier/JIT
    while preserving concurrent access to this map from user space

- two main use cases for array type:
  . 'global' eBPF variables: array of 1 element with key=0 and value is a
    collection of 'global' variables which programs can use to keep the state
    between events
  . aggregation of tracing events into fixed set of buckets

- all array elements pre-allocated and zero initialized at init time

- key as an index in array and can only be 4 byte

- map_delete_elem() returns EINVAL, since elements cannot be deleted

- map_update_elem() replaces elements in an non-atomic way
  (for atomic updates hashtable type should be used instead)

Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---

Note, from eBPF program and from user space, all map types are accessed
through the same API.

Example of using array type for 'global' variables from eBPF program:
struct globals {
    u64 lat_ave;
    u64 lat_sum;
    u64 missed;
    u64 max_lat;
    int num_samples;
};

struct bpf_map_def SEC("maps") global_map = {
    .type = BPF_MAP_TYPE_ARRAY,
    .key_size = sizeof(int),
    .value_size = sizeof(struct globals),
    .max_entries = 1,
};

int bpf_prog(struct bpf_context *ctx)
{
    ...
    int ind = 0;
    struct globals *g = bpf_map_lookup_elem(&global_map, &ind);
    if (!g)
            return 0;
    if (g->lat_ave == 0) {
            g->num_samples++;
            g->lat_sum += delta;
            if (g->num_samples >= 100) {
                    g->lat_ave = g->lat_sum / g->num_samples;
    ...

The future verifier/JIT optimization will replace bpf_map_lookup_elem()
call inside eBPF program with const pointer to element value of key=0,
so that eBPF program will have no penalty whatsoever to access such
'global' variables.
At the same time user space can access this 'globals' via common map API.

Full example of both kernel and user side follows in later patches.

The array map is like C array of structures. Nothing protects concurrent access.
It's used in the cases where accuracy is not needed or when there is no
concurrent access. To compute a histogram of events in tracing the array
of integers is used. Every integer is a counter. Program increments it
(may be without using xadd) and user space periodically reads it back.
map_update_elem() is called by userspace once to initialize it if zero-init
is not enough. Programs do lookup() and modify the values.
For array type update() method is used rarely, delete() is never used and
get_next() is needed for completeness to browse maps through common map API.

 include/uapi/linux/bpf.h |    1 +
 kernel/bpf/Makefile      |    2 +-
 kernel/bpf/arraymap.c    |  151 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 153 insertions(+), 1 deletion(-)
 create mode 100644 kernel/bpf/arraymap.c

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 03a01fd609aa..0d662fe75df5 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -112,6 +112,7 @@ enum bpf_cmd {
 enum bpf_map_type {
 	BPF_MAP_TYPE_UNSPEC,
 	BPF_MAP_TYPE_HASH,
+	BPF_MAP_TYPE_ARRAY,
 };
 
 enum bpf_prog_type {
diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
index 2c0ec7f9da78..72ec98ba2d42 100644
--- a/kernel/bpf/Makefile
+++ b/kernel/bpf/Makefile
@@ -1,5 +1,5 @@
 obj-y := core.o
-obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o hashtab.o
+obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o hashtab.o arraymap.o
 ifdef CONFIG_TEST_BPF
 obj-$(CONFIG_BPF_SYSCALL) += test_stub.o
 endif
diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
new file mode 100644
index 000000000000..f4f6965f86cb
--- /dev/null
+++ b/kernel/bpf/arraymap.c
@@ -0,0 +1,151 @@
+/* Copyright (c) 2011-2014 PLUMgrid, http://plumgrid.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+#include <linux/bpf.h>
+#include <linux/err.h>
+#include <linux/vmalloc.h>
+#include <linux/slab.h>
+#include <linux/mm.h>
+
+struct bpf_array {
+	struct bpf_map map;
+	u32 elem_size;
+	char value[0] __aligned(8);
+};
+
+/* Called from syscall */
+static struct bpf_map *array_map_alloc(union bpf_attr *attr)
+{
+	struct bpf_array *array;
+	u32 elem_size;
+
+	/* check sanity of attributes */
+	if (attr->max_entries == 0 || attr->key_size != 4 ||
+	    attr->value_size == 0)
+		return ERR_PTR(-EINVAL);
+
+	elem_size = round_up(attr->value_size, 8);
+
+	/* allocate all map elements and zero-initialize them */
+	array = kzalloc(sizeof(*array) + attr->max_entries * elem_size,
+			GFP_USER | __GFP_NOWARN);
+	if (!array) {
+		array = vzalloc(array->map.max_entries * array->elem_size);
+		if (!array)
+			return ERR_PTR(-ENOMEM);
+	}
+
+	/* copy mandatory map attributes */
+	array->map.key_size = attr->key_size;
+	array->map.value_size = attr->value_size;
+	array->map.max_entries = attr->max_entries;
+
+	array->elem_size = elem_size;
+
+	return &array->map;
+
+}
+
+/* Called from syscall or from eBPF program */
+static void *array_map_lookup_elem(struct bpf_map *map, void *key)
+{
+	struct bpf_array *array = container_of(map, struct bpf_array, map);
+	u32 index = *(u32 *)key;
+
+	if (index >= array->map.max_entries)
+		return NULL;
+
+	return array->value + array->elem_size * index;
+}
+
+/* Called from syscall */
+static int array_map_get_next_key(struct bpf_map *map, void *key, void *next_key)
+{
+	struct bpf_array *array = container_of(map, struct bpf_array, map);
+	u32 index = *(u32 *)key;
+	u32 *next = (u32 *)next_key;
+
+	if (index >= array->map.max_entries) {
+		*next = 0;
+		return 0;
+	}
+
+	if (index == array->map.max_entries - 1)
+		return -ENOENT;
+
+	*next = index + 1;
+	return 0;
+}
+
+/* Called from syscall or from eBPF program */
+static int array_map_update_elem(struct bpf_map *map, void *key, void *value,
+				 u64 map_flags)
+{
+	struct bpf_array *array = container_of(map, struct bpf_array, map);
+	u32 index = *(u32 *)key;
+
+	if (map_flags > BPF_EXIST)
+		/* unknown flags */
+		return -EINVAL;
+
+	if (index >= array->map.max_entries)
+		/* all elements were pre-allocated, cannot insert a new one */
+		return -E2BIG;
+	
+	if (map_flags == BPF_NOEXIST)
+		/* all elemenets already exist */
+		return -EEXIST;
+
+	memcpy(array->value + array->elem_size * index, value, array->elem_size);
+	return 0;
+}
+
+/* Called from syscall or from eBPF program */
+static int array_map_delete_elem(struct bpf_map *map, void *key)
+{
+	return -EINVAL;
+}
+
+/* Called when map->refcnt goes to zero, either from workqueue or from syscall */
+static void array_map_free(struct bpf_map *map)
+{
+	struct bpf_array *array = container_of(map, struct bpf_array, map);
+
+	/* at this point bpf_prog->aux->refcnt == 0 and this map->refcnt == 0,
+	 * so the programs (can be more than one that used this map) were
+	 * disconnected from events. Wait for outstanding programs to complete
+	 * and free the array
+	 */
+	synchronize_rcu();
+
+	kvfree(array);
+}
+
+static struct bpf_map_ops array_ops = {
+	.map_alloc = array_map_alloc,
+	.map_free = array_map_free,
+	.map_get_next_key = array_map_get_next_key,
+	.map_lookup_elem = array_map_lookup_elem,
+	.map_update_elem = array_map_update_elem,
+	.map_delete_elem = array_map_delete_elem,
+};
+
+static struct bpf_map_type_list tl = {
+	.ops = &array_ops,
+	.type = BPF_MAP_TYPE_ARRAY,
+};
+
+static int __init register_array_map(void)
+{
+	bpf_register_map_type(&tl);
+	return 0;
+}
+late_initcall(register_array_map);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v2 net-next 2/7] bpf: add hashtable type of eBPF maps
From: Alexei Starovoitov @ 2014-11-14  1:36 UTC (permalink / raw)
  To: David S. Miller
  Cc: Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
	Hannes Frederic Sowa, Eric Dumazet, linux-api, netdev,
	linux-kernel
In-Reply-To: <1415929010-9361-1-git-send-email-ast@plumgrid.com>

add new map type BPF_MAP_TYPE_HASH and its implementation

- maps are created/destroyed by userspace. Both userspace and eBPF programs
  can lookup/update/delete elements from the map

- eBPF programs can be called in_irq(), so use spin_lock_irqsave() mechanism
  for concurrent updates

- key/value are opaque range of bytes (aligned to 8 bytes)

- user space provides 3 configuration attributes via BPF syscall:
  key_size, value_size, max_entries

- map takes care of allocating/freeing key/value pairs

- map_update_elem() must fail to insert new element when max_entries
  limit is reached to make sure that eBPF programs cannot exhaust memory

- map_update_elem() replaces elements in an atomic way

- optimized for speed of lookup() which can be called multiple times from
  eBPF program which itself is triggered by high volume of events
  . in the future JIT compiler may recognize lookup() call and optimize it
    further, since key_size is constant for life of eBPF program

Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
 include/uapi/linux/bpf.h |    1 +
 kernel/bpf/Makefile      |    2 +-
 kernel/bpf/hashtab.c     |  362 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 364 insertions(+), 1 deletion(-)
 create mode 100644 kernel/bpf/hashtab.c

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 3e9e1b77f29d..03a01fd609aa 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -111,6 +111,7 @@ enum bpf_cmd {
 
 enum bpf_map_type {
 	BPF_MAP_TYPE_UNSPEC,
+	BPF_MAP_TYPE_HASH,
 };
 
 enum bpf_prog_type {
diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
index 0daf7f6ae7df..2c0ec7f9da78 100644
--- a/kernel/bpf/Makefile
+++ b/kernel/bpf/Makefile
@@ -1,5 +1,5 @@
 obj-y := core.o
-obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o
+obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o hashtab.o
 ifdef CONFIG_TEST_BPF
 obj-$(CONFIG_BPF_SYSCALL) += test_stub.o
 endif
diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
new file mode 100644
index 000000000000..d234a012f046
--- /dev/null
+++ b/kernel/bpf/hashtab.c
@@ -0,0 +1,362 @@
+/* Copyright (c) 2011-2014 PLUMgrid, http://plumgrid.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+#include <linux/bpf.h>
+#include <linux/jhash.h>
+#include <linux/filter.h>
+#include <linux/vmalloc.h>
+
+struct bpf_htab {
+	struct bpf_map map;
+	struct hlist_head *buckets;
+	spinlock_t lock;
+	u32 count;	/* number of elements in this hashtable */
+	u32 n_buckets;	/* number of hash buckets */
+	u32 elem_size;	/* size of each element in bytes */
+};
+
+/* each htab element is struct htab_elem + key + value */
+struct htab_elem {
+	struct hlist_node hash_node;
+	struct rcu_head rcu;
+	u32 hash;
+	char key[0] __aligned(8);
+};
+
+/* Called from syscall */
+static struct bpf_map *htab_map_alloc(union bpf_attr *attr)
+{
+	struct bpf_htab *htab;
+	int err, i;
+
+	htab = kzalloc(sizeof(*htab), GFP_USER);
+	if (!htab)
+		return ERR_PTR(-ENOMEM);
+
+	/* mandatory map attributes */
+	htab->map.key_size = attr->key_size;
+	htab->map.value_size = attr->value_size;
+	htab->map.max_entries = attr->max_entries;
+
+	/* check sanity of attributes.
+	 * value_size == 0 may be allowed in the future to use map as a set
+	 */
+	err = -EINVAL;
+	if (htab->map.max_entries == 0 || htab->map.key_size == 0 ||
+	    htab->map.value_size == 0)
+		goto free_htab;
+
+	/* hash table size must be power of 2 */
+	htab->n_buckets = roundup_pow_of_two(htab->map.max_entries);
+
+	err = -E2BIG;
+	if (htab->map.key_size > MAX_BPF_STACK)
+		/* eBPF programs initialize keys on stack, so they cannot be
+		 * larger than max stack size
+		 */
+		goto free_htab;
+
+	err = -ENOMEM;
+	htab->buckets = kmalloc_array(htab->n_buckets, sizeof(struct hlist_head),
+				      GFP_USER | __GFP_NOWARN);
+
+	if (!htab->buckets) {
+		htab->buckets = vmalloc(htab->n_buckets * sizeof(struct hlist_head));
+		if (!htab->buckets)
+			goto free_htab;
+	}
+
+	for (i = 0; i < htab->n_buckets; i++)
+		INIT_HLIST_HEAD(&htab->buckets[i]);
+
+	spin_lock_init(&htab->lock);
+	htab->count = 0;
+
+	htab->elem_size = sizeof(struct htab_elem) +
+			  round_up(htab->map.key_size, 8) +
+			  htab->map.value_size;
+	return &htab->map;
+
+free_htab:
+	kfree(htab);
+	return ERR_PTR(err);
+}
+
+static inline u32 htab_map_hash(const void *key, u32 key_len)
+{
+	return jhash(key, key_len, 0);
+}
+
+static inline struct hlist_head *select_bucket(struct bpf_htab *htab, u32 hash)
+{
+	return &htab->buckets[hash & (htab->n_buckets - 1)];
+}
+
+static struct htab_elem *lookup_elem_raw(struct hlist_head *head, u32 hash,
+					 void *key, u32 key_size)
+{
+	struct htab_elem *l;
+
+	hlist_for_each_entry_rcu(l, head, hash_node)
+		if (l->hash == hash && !memcmp(&l->key, key, key_size))
+			return l;
+
+	return NULL;
+}
+
+/* Called from syscall or from eBPF program */
+static void *htab_map_lookup_elem(struct bpf_map *map, void *key)
+{
+	struct bpf_htab *htab = container_of(map, struct bpf_htab, map);
+	struct hlist_head *head;
+	struct htab_elem *l;
+	u32 hash, key_size;
+
+	/* Must be called with rcu_read_lock. */
+	WARN_ON_ONCE(!rcu_read_lock_held());
+
+	key_size = map->key_size;
+
+	hash = htab_map_hash(key, key_size);
+
+	head = select_bucket(htab, hash);
+
+	l = lookup_elem_raw(head, hash, key, key_size);
+
+	if (l)
+		return l->key + round_up(map->key_size, 8);
+
+	return NULL;
+}
+
+/* Called from syscall */
+static int htab_map_get_next_key(struct bpf_map *map, void *key, void *next_key)
+{
+	struct bpf_htab *htab = container_of(map, struct bpf_htab, map);
+	struct hlist_head *head;
+	struct htab_elem *l, *next_l;
+	u32 hash, key_size;
+	int i;
+
+	WARN_ON_ONCE(!rcu_read_lock_held());
+
+	key_size = map->key_size;
+
+	hash = htab_map_hash(key, key_size);
+
+	head = select_bucket(htab, hash);
+
+	/* lookup the key */
+	l = lookup_elem_raw(head, hash, key, key_size);
+
+	if (!l) {
+		i = 0;
+		goto find_first_elem;
+	}
+
+	/* key was found, get next key in the same bucket */
+	next_l = hlist_entry_safe(rcu_dereference_raw(hlist_next_rcu(&l->hash_node)),
+				  struct htab_elem, hash_node);
+
+	if (next_l) {
+		/* if next elem in this hash list is non-zero, just return it */
+		memcpy(next_key, next_l->key, key_size);
+		return 0;
+	}
+
+	/* no more elements in this hash list, go to the next bucket */
+	i = hash & (htab->n_buckets - 1);
+	i++;
+
+find_first_elem:
+	/* iterate over buckets */
+	for (; i < htab->n_buckets; i++) {
+		head = select_bucket(htab, i);
+
+		/* pick first element in the bucket */
+		next_l = hlist_entry_safe(rcu_dereference_raw(hlist_first_rcu(head)),
+					  struct htab_elem, hash_node);
+		if (next_l) {
+			/* if it's not empty, just return it */
+			memcpy(next_key, next_l->key, key_size);
+			return 0;
+		}
+	}
+
+	/* itereated over all buckets and all elements */
+	return -ENOENT;
+}
+
+/* Called from syscall or from eBPF program */
+static int htab_map_update_elem(struct bpf_map *map, void *key, void *value,
+				u64 map_flags)
+{
+	struct bpf_htab *htab = container_of(map, struct bpf_htab, map);
+	struct htab_elem *l_new, *l_old;
+	struct hlist_head *head;
+	unsigned long flags;
+	u32 key_size;
+	int ret;
+
+	if (map_flags > BPF_EXIST)
+		/* unknown flags */
+		return -EINVAL;
+
+	WARN_ON_ONCE(!rcu_read_lock_held());
+
+	/* allocate new element outside of lock */
+	l_new = kmalloc(htab->elem_size, GFP_ATOMIC);
+	if (!l_new)
+		return -ENOMEM;
+
+	key_size = map->key_size;
+
+	memcpy(l_new->key, key, key_size);
+	memcpy(l_new->key + round_up(key_size, 8), value, map->value_size);
+
+	l_new->hash = htab_map_hash(l_new->key, key_size);
+
+	/* bpf_map_update_elem() can be called in_irq() */
+	spin_lock_irqsave(&htab->lock, flags);
+
+	head = select_bucket(htab, l_new->hash);
+
+	l_old = lookup_elem_raw(head, l_new->hash, key, key_size);
+
+	if (!l_old && unlikely(htab->count >= map->max_entries)) {
+		/* if elem with this 'key' doesn't exist and we've reached
+		 * max_entries limit, fail insertion of new elem
+		 */
+		ret = -E2BIG;
+		goto err;
+	}
+
+	if (l_old && map_flags == BPF_NOEXIST) {
+		/* elem already exists */
+		ret = -EEXIST;
+		goto err;
+	}
+
+	if (!l_old && map_flags == BPF_EXIST) {
+		/* elem doesn't exist, cannot update it */
+		ret = -ENOENT;
+		goto err;
+	}
+
+	/* add new element to the head of the list, so that concurrent
+	 * search will find it before old elem
+	 */
+	hlist_add_head_rcu(&l_new->hash_node, head);
+	if (l_old) {
+		hlist_del_rcu(&l_old->hash_node);
+		kfree_rcu(l_old, rcu);
+	} else {
+		htab->count++;
+	}
+	spin_unlock_irqrestore(&htab->lock, flags);
+
+	return 0;
+err:
+	spin_unlock_irqrestore(&htab->lock, flags);
+	kfree(l_new);
+	return ret;
+}
+
+/* Called from syscall or from eBPF program */
+static int htab_map_delete_elem(struct bpf_map *map, void *key)
+{
+	struct bpf_htab *htab = container_of(map, struct bpf_htab, map);
+	struct hlist_head *head;
+	struct htab_elem *l;
+	unsigned long flags;
+	u32 hash, key_size;
+	int ret = -ENOENT;
+
+	WARN_ON_ONCE(!rcu_read_lock_held());
+
+	key_size = map->key_size;
+
+	hash = htab_map_hash(key, key_size);
+
+	spin_lock_irqsave(&htab->lock, flags);
+
+	head = select_bucket(htab, hash);
+
+	l = lookup_elem_raw(head, hash, key, key_size);
+
+	if (l) {
+		hlist_del_rcu(&l->hash_node);
+		htab->count--;
+		kfree_rcu(l, rcu);
+		ret = 0;
+	}
+
+	spin_unlock_irqrestore(&htab->lock, flags);
+	return ret;
+}
+
+static void delete_all_elements(struct bpf_htab *htab)
+{
+	int i;
+
+	for (i = 0; i < htab->n_buckets; i++) {
+		struct hlist_head *head = select_bucket(htab, i);
+		struct hlist_node *n;
+		struct htab_elem *l;
+
+		hlist_for_each_entry_safe(l, n, head, hash_node) {
+			hlist_del_rcu(&l->hash_node);
+			htab->count--;
+			kfree(l);
+		}
+	}
+}
+
+/* Called when map->refcnt goes to zero, either from workqueue or from syscall */
+static void htab_map_free(struct bpf_map *map)
+{
+	struct bpf_htab *htab = container_of(map, struct bpf_htab, map);
+
+	/* at this point bpf_prog->aux->refcnt == 0 and this map->refcnt == 0,
+	 * so the programs (can be more than one that used this map) were
+	 * disconnected from events. Wait for outstanding critical sections in
+	 * these programs to complete
+	 */
+	synchronize_rcu();
+
+	/* some of kfree_rcu() callbacks for elements of this map may not have
+	 * executed. It's ok. Proceed to free residual elements and map itself
+	 */
+	delete_all_elements(htab);
+	kvfree(htab->buckets);
+	kfree(htab);
+}
+
+static struct bpf_map_ops htab_ops = {
+	.map_alloc = htab_map_alloc,
+	.map_free = htab_map_free,
+	.map_get_next_key = htab_map_get_next_key,
+	.map_lookup_elem = htab_map_lookup_elem,
+	.map_update_elem = htab_map_update_elem,
+	.map_delete_elem = htab_map_delete_elem,
+};
+
+static struct bpf_map_type_list tl = {
+	.ops = &htab_ops,
+	.type = BPF_MAP_TYPE_HASH,
+};
+
+static int __init register_htab_map(void)
+{
+	bpf_register_map_type(&tl);
+	return 0;
+}
+late_initcall(register_htab_map);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v2 net-next 1/7] bpf: add 'flags' attribute to BPF_MAP_UPDATE_ELEM command
From: Alexei Starovoitov @ 2014-11-14  1:36 UTC (permalink / raw)
  To: David S. Miller
  Cc: Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
	Hannes Frederic Sowa, Eric Dumazet, linux-api, netdev,
	linux-kernel
In-Reply-To: <1415929010-9361-1-git-send-email-ast@plumgrid.com>

the current meaning of BPF_MAP_UPDATE_ELEM syscall command is:
either update existing map element or create a new one.
Initially the plan was to add a new command to handle the case of
'create new element if it didn't exist', but 'flags' style looks
cleaner and overall diff is much smaller (more code reused), so add 'flags'
attribute to BPF_MAP_UPDATE_ELEM command with the following meaning:
 #define BPF_ANY	0 /* create new element or update existing */
 #define BPF_NOEXIST	1 /* create new element if it didn't exist */
 #define BPF_EXIST	2 /* update existing element */

bpf_update_elem(fd, key, value, BPF_NOEXIST) call can fail with EEXIST
if element already exists.

bpf_update_elem(fd, key, value, BPF_EXIST) can fail with ENOENT
if element doesn't exist.

Userspace will call it as:
int bpf_update_elem(int fd, void *key, void *value, __u64 flags)
{
    union bpf_attr attr = {
        .map_fd = fd,
        .key = ptr_to_u64(key),
        .value = ptr_to_u64(value),
        .flags = flags;
    };

    return bpf(BPF_MAP_UPDATE_ELEM, &attr, sizeof(attr));
}

First two bits of 'flags' are used to encode style of bpf_update_elem() command.
Bits 2-63 are reserved for future use.

Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---

patch 5 of this set includes tests of bpf_update_elem() with these flags

 include/linux/bpf.h      |    2 +-
 include/uapi/linux/bpf.h |    8 +++++++-
 kernel/bpf/syscall.c     |    4 ++--
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 3cf91754a957..51e9242e4803 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -22,7 +22,7 @@ struct bpf_map_ops {
 
 	/* funcs callable from userspace and from eBPF programs */
 	void *(*map_lookup_elem)(struct bpf_map *map, void *key);
-	int (*map_update_elem)(struct bpf_map *map, void *key, void *value);
+	int (*map_update_elem)(struct bpf_map *map, void *key, void *value, u64 flags);
 	int (*map_delete_elem)(struct bpf_map *map, void *key);
 };
 
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index d18316f9e9c4..3e9e1b77f29d 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -82,7 +82,7 @@ enum bpf_cmd {
 
 	/* create or update key/value pair in a given map
 	 * err = bpf(BPF_MAP_UPDATE_ELEM, union bpf_attr *attr, u32 size)
-	 * Using attr->map_fd, attr->key, attr->value
+	 * Using attr->map_fd, attr->key, attr->value, attr->flags
 	 * returns zero or negative error
 	 */
 	BPF_MAP_UPDATE_ELEM,
@@ -117,6 +117,11 @@ enum bpf_prog_type {
 	BPF_PROG_TYPE_UNSPEC,
 };
 
+/* flags for BPF_MAP_UPDATE_ELEM command */
+#define BPF_ANY		0 /* create new element or update existing */
+#define BPF_NOEXIST	1 /* create new element if it didn't exist */
+#define BPF_EXIST	2 /* update existing element */
+
 union bpf_attr {
 	struct { /* anonymous struct used by BPF_MAP_CREATE command */
 		__u32	map_type;	/* one of enum bpf_map_type */
@@ -132,6 +137,7 @@ union bpf_attr {
 			__aligned_u64 value;
 			__aligned_u64 next_key;
 		};
+		__u64		flags;
 	};
 
 	struct { /* anonymous struct used by BPF_PROG_LOAD command */
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index ba61c8c16032..c0d03bf317a2 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -190,7 +190,7 @@ err_put:
 	return err;
 }
 
-#define BPF_MAP_UPDATE_ELEM_LAST_FIELD value
+#define BPF_MAP_UPDATE_ELEM_LAST_FIELD flags
 
 static int map_update_elem(union bpf_attr *attr)
 {
@@ -231,7 +231,7 @@ static int map_update_elem(union bpf_attr *attr)
 	 * therefore all map accessors rely on this fact, so do the same here
 	 */
 	rcu_read_lock();
-	err = map->ops->map_update_elem(map, key, value);
+	err = map->ops->map_update_elem(map, key, value, attr->flags);
 	rcu_read_unlock();
 
 free_value:
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v2 net-next 0/7] implementation of eBPF maps
From: Alexei Starovoitov @ 2014-11-14  1:36 UTC (permalink / raw)
  To: David S. Miller
  Cc: Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
	Hannes Frederic Sowa, Eric Dumazet, linux-api, netdev,
	linux-kernel

Hi All,

v1->v2:
renamed flags for MAP_UPDATE_ELEM command to be more concise,
clarified commit logs and improved comments in patches 1,3,7
per discussions with Daniel
 
Old v1 cover:

this set of patches adds implementation of HASH and ARRAY types of eBPF maps
which were described in manpage in commit b4fc1a460f30("Merge branch 'bpf-next'")

The difference vs previous version of these patches from August:
- added 'flags' attribute to BPF_MAP_UPDATE_ELEM
- in HASH type implementation removed per-map kmem_cache.
  I was doing kmem_cache_create() for every map to enable selective slub
  debugging to check for overflows and leaks. Now it's not needed, so just
  use normal kmalloc() for map elements.
- added ARRAY type which was mentioned in manpage, but wasn't public yet
- added map testsuite and removed temporary bits from test_stubs

Note, eBPF programs cannot be attached to events yet.
It will come in the next set.

Alexei Starovoitov (7):
  bpf: add 'flags' attribute to BPF_MAP_UPDATE_ELEM command
  bpf: add hashtable type of eBPF maps
  bpf: add array type of eBPF maps
  bpf: fix BPF_MAP_LOOKUP_ELEM command return code
  bpf: add a testsuite for eBPF maps
  bpf: allow eBPF programs to use maps
  bpf: remove test map scaffolding and user proper types

 include/linux/bpf.h         |    7 +-
 include/uapi/linux/bpf.h    |   13 +-
 kernel/bpf/Makefile         |    2 +-
 kernel/bpf/arraymap.c       |  151 ++++++++++++++++++
 kernel/bpf/hashtab.c        |  362 +++++++++++++++++++++++++++++++++++++++++++
 kernel/bpf/helpers.c        |   89 +++++++++++
 kernel/bpf/syscall.c        |    6 +-
 kernel/bpf/test_stub.c      |   56 ++-----
 samples/bpf/Makefile        |    3 +-
 samples/bpf/libbpf.c        |    3 +-
 samples/bpf/libbpf.h        |    2 +-
 samples/bpf/test_maps.c     |  291 ++++++++++++++++++++++++++++++++++
 samples/bpf/test_verifier.c |   14 +-
 13 files changed, 936 insertions(+), 63 deletions(-)
 create mode 100644 kernel/bpf/arraymap.c
 create mode 100644 kernel/bpf/hashtab.c
 create mode 100644 kernel/bpf/helpers.c
 create mode 100644 samples/bpf/test_maps.c

-- 
1.7.9.5

^ permalink raw reply

* Re: [PATCH net-next] icmp: Remove some spurious dropped packet profile hits from the ICMP path
From: David Miller @ 2014-11-14  1:32 UTC (permalink / raw)
  To: raj; +Cc: netdev
In-Reply-To: <20141113225457.A3E502900805@tardy>

From: raj@tardy.usa.hp.com (Rick Jones)
Date: Thu, 13 Nov 2014 14:54:57 -0800 (PST)

> +	/* until the v6 path can be better sorted we may still need
> +	 * to kfree_sbk() here but want to avoid a double free from

Typo "kfree_skb()".

^ permalink raw reply

* Re: Resume S4 - r8169 BROKEN
From: poma @ 2014-11-14  0:59 UTC (permalink / raw)
  To: netdev
In-Reply-To: <54647EFA.5020707@gmail.com>

On 13.11.2014 10:50, poma wrote:
> 
> Resume S4 - r8169 RealTek RTL-8169 Gigabit Ethernet driver - BROKEN
> 
> Tested with:
>   kernel-3.18.0-0.rc4.git0.2.fc22
>     http://koji.fedoraproject.org/koji/buildinfo?buildID=592269
>       Nov 11 2014
>   &
>   kernel-3.18.0-rc2.git-9dfa9a2-net-next+
>     https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=9dfa9a2
>       2014-11-13
> 
> 
> # lspci -knn -s 03:00.0
> 03:00.0 Ethernet controller [0200]: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller [10ec:8168] (rev 02)
> 	Subsystem: Gigabyte Technology Co., Ltd Motherboard [1458:e000]
> 	Kernel driver in use: r8169
> 	Kernel modules: r8169
> 
> 
> # modprobe -v r8169
> insmod /lib/modules/3.18.0-rc2.git-9dfa9a2-net-next+/kernel/drivers/net/mii.ko 
> insmod /lib/modules/3.18.0-rc2.git-9dfa9a2-net-next+/kernel/drivers/net/ethernet/realtek/r8169.ko 
> 
> 
> # dmesg
> [  142.344223] r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
> [  142.344229] r8169 0000:03:00.0: can't disable ASPM; OS doesn't have ASPM control
> [  142.344456] r8169 0000:03:00.0: irq 26 for MSI/MSI-X
> [  142.344600] r8169 0000:03:00.0 eth1: RTL8168c/8111c at 0xffffc9000291a000, 00:12:34:56:78:30, XID 1c4000c0 IRQ 26
> [  142.344602] r8169 0000:03:00.0 eth1: jumbo features [frames: 6128 bytes, tx checksumming: ko]
> [  142.388933] r8169 0000:03:00.0 enp3s0: renamed from eth1
> [  142.417502] r8169 0000:03:00.0 enp3s0: link down
> [  142.417512] r8169 0000:03:00.0 enp3s0: link down
> [  144.029918] r8169 0000:03:00.0 enp3s0: link up
> 
> 
> # ifconfig enp3s0
> enp3s0: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>  mtu 1500
>         inet 192.168.2.2  netmask 255.255.255.0  broadcast 192.168.2.255
>         inet6 fe80::212:34ff:fe56:7830  prefixlen 64  scopeid 0x20<link>
>         ether 00:12:34:56:78:30  txqueuelen 1000  (Ethernet)
>         RX packets 642  bytes 382970 (373.9 KiB)
>         RX errors 0  dropped 0  overruns 0  frame 0
>         TX packets 709  bytes 72634 (70.9 KiB)
>         TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0
> 
> 
> # ping kernel.org -c10
> PING kernel.org (199.204.44.194) 56(84) bytes of data.
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=1 ttl=49 time=134 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=2 ttl=49 time=133 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=3 ttl=49 time=134 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=4 ttl=49 time=133 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=5 ttl=49 time=134 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=6 ttl=49 time=134 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=7 ttl=49 time=134 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=8 ttl=49 time=133 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=9 ttl=49 time=133 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=10 ttl=49 time=134 ms
> 
> --- kernel.org ping statistics ---
> 10 packets transmitted, 10 received, 0% packet loss, time 9011ms
> rtt min/avg/max/mdev = 133.008/134.163/134.987/0.683 ms
> 
> ~~~~~~~~~~~~~~~~~~~
> # systemctl suspend
>           & RESUME
>           ~~~~~~~~
> 
> # dmesg
> [  673.601499] PM: Syncing filesystems ... done.
> [  673.793930] PM: Preparing system for mem sleep
> [  673.961198] Freezing user space processes ... (elapsed 0.002 seconds) done.
> [  673.963420] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
> [  673.964664] PM: Entering mem sleep
> [  676.457146] PM: suspend of devices complete after 2492.103 msecs
> [  676.457826] PM: late suspend of devices complete after 0.673 msecs
> [  676.469303] PM: noirq suspend of devices complete after 11.469 msecs
> [  676.469889] ACPI: Preparing to enter system sleep state S3
> [  676.470890] PM: Saving platform NVS memory
> [  676.479540] ACPI: Low-level resume complete
> [  676.479613] PM: Restoring platform NVS memory
> [  676.525167] ACPI: Waking up from system sleep state S3
> [  676.590780] r8169 0000:03:00.0 enp3s0: link down
> [  678.060741] r8169 0000:03:00.0 enp3s0: link up
> [  679.604068] PM: resume of devices complete after 3066.534 msecs
> [  679.604657] PM: Finishing wakeup.
> [  679.604662] Restarting tasks ... done.
> 
> 
> # ifconfig enp3s0
> enp3s0: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>  mtu 1500
>         inet 192.168.2.2  netmask 255.255.255.0  broadcast 192.168.2.255
>         inet6 fe80::212:34ff:fe56:7830  prefixlen 64  scopeid 0x20<link>
>         ether 00:12:34:56:78:30  txqueuelen 1000  (Ethernet)
>         RX packets 782  bytes 464566 (453.6 KiB)
>         RX errors 0  dropped 0  overruns 0  frame 0
>         TX packets 860  bytes 86524 (84.4 KiB)
>         TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0
> 
> 
> # ping kernel.org -c10
> PING kernel.org (199.204.44.194) 56(84) bytes of data.
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=1 ttl=49 time=134 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=2 ttl=49 time=134 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=3 ttl=49 time=133 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=4 ttl=49 time=132 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=5 ttl=49 time=134 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=6 ttl=49 time=133 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=7 ttl=49 time=133 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=8 ttl=49 time=133 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=9 ttl=49 time=133 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=10 ttl=49 time=134 ms
> 
> --- kernel.org ping statistics ---
> 10 packets transmitted, 10 received, 0% packet loss, time 9011ms
> rtt min/avg/max/mdev = 132.798/133.770/134.919/0.682 ms
> 
> ~~~~~~~~~~~~~~~~~~~~~
> # systemctl hibernate
>           & THAW
>           ~~~~~~
> 
> # dmesg
> [ 1187.902839] PM: Hibernation mode set to 'platform'
> [ 1188.098296] PM: Syncing filesystems ... done.
> [ 1188.545125] Freezing user space processes ... (elapsed 0.002 seconds) done.
> [ 1188.548022] PM: Marking nosave pages: [mem 0x00000000-0x00000fff]
> [ 1188.548031] PM: Marking nosave pages: [mem 0x0009e000-0x000fffff]
> [ 1188.548042] PM: Marking nosave pages: [mem 0xcfef0000-0xffffffff]
> [ 1188.549729] PM: Basic memory bitmaps created
> [ 1188.549755] PM: Preallocating image memory... done (allocated 641929 pages)
> [ 1189.133132] PM: Allocated 2567716 kbytes in 0.58 seconds (4427.09 MB/s)
> [ 1189.133241] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
> [ 1191.867738] PM: freeze of devices complete after 2732.078 msecs
> [ 1191.868597] PM: late freeze of devices complete after 0.855 msecs
> [ 1191.869951] PM: noirq freeze of devices complete after 1.344 msecs
> [ 1191.871294] ACPI: Preparing to enter system sleep state S4
> [ 1191.871956] PM: Saving platform NVS memory
> [ 1191.879106] PM: Creating hibernation image:
> [ 1192.100544] PM: Need to copy 359761 pages
> [ 1192.100558] PM: Normal pages needed: 359761 + 1024, available pages: 688358
> [ 1191.880196] PM: Restoring platform NVS memory
> [ 1191.925748] ACPI: Waking up from system sleep state S4
> [ 1191.936325] PM: noirq restore of devices complete after 10.428 msecs
> [ 1191.936796] PM: early restore of devices complete after 0.423 msecs
> [ 1192.043225] r8169 0000:03:00.0 enp3s0: link down
> [ 1192.689507] PM: restore of devices complete after 699.641 msecs
> [ 1192.689835] PM: Image restored successfully.
> [ 1192.689856] PM: Basic memory bitmaps freed
> [ 1192.689860] Restarting tasks ... done.
> [ 1193.672620] r8169 0000:03:00.0 enp3s0: link up
> 
> 
> # ifconfig enp3s0
> enp3s0: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>  mtu 1500
>         inet 192.168.2.2  netmask 255.255.255.0  broadcast 192.168.2.255
>         inet6 fe80::212:34ff:fe56:7830  prefixlen 64  scopeid 0x20<link>
>         ether 00:12:34:56:78:30  txqueuelen 1000  (Ethernet)
>         RX packets 794  bytes 465496 (454.5 KiB)
>         RX errors 0  dropped 0  overruns 0  frame 0
>         TX packets 939  bytes 90700 (88.5 KiB)
>         TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0
> 
> 
> # ping kernel.org -c10
> ping: unknown host kernel.org
> 
> # ping 199.204.44.194 -c10
> PING 199.204.44.194 (199.204.44.194) 56(84) bytes of data.
> From 192.168.2.2 icmp_seq=1 Destination Host Unreachable
> From 192.168.2.2 icmp_seq=2 Destination Host Unreachable
> From 192.168.2.2 icmp_seq=3 Destination Host Unreachable
> From 192.168.2.2 icmp_seq=4 Destination Host Unreachable
> From 192.168.2.2 icmp_seq=5 Destination Host Unreachable
> From 192.168.2.2 icmp_seq=6 Destination Host Unreachable
> From 192.168.2.2 icmp_seq=7 Destination Host Unreachable
> From 192.168.2.2 icmp_seq=8 Destination Host Unreachable
> From 192.168.2.2 icmp_seq=9 Destination Host Unreachable
> From 192.168.2.2 icmp_seq=10 Destination Host Unreachable
> 
> --- 199.204.44.194 ping statistics ---
> 10 packets transmitted, 0 received, +10 errors, 100% packet loss, time 9001ms
> pipe 4
> 
> ~~~~~~~~~~~~~~~~~~~~    ~~~~~~~~~~~~~~~~~
> # modprobe -rv r8169 && modprobe -v r8169
> rmmod r8169
> rmmod mii
> insmod /lib/modules/3.18.0-rc2.git-9dfa9a2-net-next+/kernel/drivers/net/mii.ko 
> insmod /lib/modules/3.18.0-rc2.git-9dfa9a2-net-next+/kernel/drivers/net/ethernet/realtek/r8169.ko 
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> # dmesg
> [ 1566.163904] r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
> [ 1566.163932] r8169 0000:03:00.0: can't disable ASPM; OS doesn't have ASPM control
> [ 1566.164443] r8169 0000:03:00.0: irq 26 for MSI/MSI-X
> [ 1566.164997] r8169 0000:03:00.0 eth1: RTL8168c/8111c at 0xffffc900027d0000, 00:12:34:56:78:30, XID 1c4000c0 IRQ 26
> [ 1566.165007] r8169 0000:03:00.0 eth1: jumbo features [frames: 6128 bytes, tx checksumming: ko]
> [ 1566.383002] r8169 0000:03:00.0 enp3s0: renamed from eth1
> [ 1566.401190] r8169 0000:03:00.0 enp3s0: link down
> [ 1567.909282] r8169 0000:03:00.0 enp3s0: link up
> 
> 
> # ifconfig enp3s0
> enp3s0: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>  mtu 1500
>         inet 192.168.2.2  netmask 255.255.255.0  broadcast 192.168.2.255
>         inet6 fe80::212:34ff:fe56:7830  prefixlen 64  scopeid 0x20<link>
>         ether 00:12:34:56:78:30  txqueuelen 1000  (Ethernet)
>         RX packets 59  bytes 14200 (13.8 KiB)
>         RX errors 0  dropped 0  overruns 0  frame 0
>         TX packets 77  bytes 7441 (7.2 KiB)
>         TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0
> 
> 
> # ping kernel.org -c10
> PING kernel.org (199.204.44.194) 56(84) bytes of data.
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=1 ttl=49 time=133 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=2 ttl=49 time=134 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=3 ttl=49 time=134 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=4 ttl=49 time=133 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=5 ttl=49 time=133 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=6 ttl=49 time=132 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=7 ttl=49 time=134 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=8 ttl=49 time=134 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=9 ttl=49 time=133 ms
> 64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=10 ttl=49 time=133 ms
> 
> --- kernel.org ping statistics ---
> 10 packets transmitted, 10 received, 0% packet loss, time 9012ms
> rtt min/avg/max/mdev = 132.657/133.636/134.708/0.728 ms
> 

This is definitely r8169 - RealTek RTL-8169 Gigabit Ethernet driver defect.

I tested with a variety of kernels starting with 3.10.0-123.9.3.el7.x86_64 up to the latest 3.18 series.

For example, skge - SysKonnect Gigabit Ethernet driver and rt2800usb - Ralink RT2800 USB Wireless LAN driver do not suffer from this issue.

The problem arises when cloned-mac-address is used, and to actually recover connection after 'thaw' there is no need to reload module per se
i.e. 'modprobe -rv r8169 && modprobe -v r8169'
it is sufficient to re-apply cloned-mac-address
i.e. 'ifconfig enp3s0 hw ether 00:12:34:56:78:30'
or use some other equivalent method.
Nevertheless this is broken behavior.

If you have an idea how to solve this within module or if you have a patch, don't hesitate to ping.


poma

^ permalink raw reply

* Re: [PATCHv2 net 1/2] fm10k: Check tunnel header length in encap offload
From: Joe Stringer @ 2014-11-14  0:54 UTC (permalink / raw)
  To: Vick, Matthew
  Cc: netdev@vger.kernel.org, Kirsher, Jeffrey T, Linux NICS,
	therbert@google.com, gerlitz.or@gmail.com,
	alexander.duyck@gmail.com, linux-kernel@vger.kernel.org
In-Reply-To: <D08A8E37.5FC83%matthew.vick@intel.com>

On Thursday, November 13, 2014 16:41:03 Vick, Matthew wrote:
> On 11/13/14, 3:36 PM, "Joe Stringer" <joestringer@nicira.com> wrote:
> >fm10k supports up to 184 bytes of inner+outer headers. Add an initial
> >check to fail encap offload if these are too large.
> >
> >Signed-off-by: Joe Stringer <joestringer@nicira.com>
> >---
> >Matthew, I didn't see the equivalent patch on netdev so I went ahead and
> >created it. If I've missed this somewhere, then please disregard.
> >
> >v2: First post.
> 
> You didn't miss it Joe--it just hasn't made it up yet. :) It's currently
> in Jeff's tree for testing. You're on the CC for the patch, so you'll get
> a notification once it goes up. It's basically the same as what you have,
> except the #define I use is 184 and I use inner_tcp_hdrlen() to account
> for the inner TCP header length.
> 
> Since your second patch should apply cleanly on top of mine, what do you
> think about dropping the first patch in this series and Jeff can send our
> two patches up together once they've passed testing?

Ah, that sounds great.

Cheers,
Joe

^ permalink raw reply

* Re: [PATCH 1/3] sh_eth: Remove redundant alignment adjustment
From: Simon Horman @ 2014-11-14  0:43 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Yoshihiro Kaneko, netdev, David S. Miller, Magnus Damm, linux-sh
In-Reply-To: <546532A2.80509@cogentembedded.com>

On Fri, Nov 14, 2014 at 01:37:22AM +0300, Sergei Shtylyov wrote:
> On 11/13/2014 10:04 AM, Yoshihiro Kaneko wrote:
> 
> >From: Mitsuhiro Kimura <mitsuhiro.kimura.kc@renesas.com>
> 
> >PTR_ALIGN macro after skb_reserve is redundant, because skb_reserve
> >function adjusts the alignment of skb->data.
> 
>    OK, but where is the bug? There must be one if you base this patch on the
> 'net' tree...

I suppose this patch would be more appropriate for the net-next tree.

> >Signed-off-by: Mitsuhiro Kimura <mitsuhiro.kimura.kc@renesas.com>
> >Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> 
> WBR, Sergei
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply

* Re: [PATCHv2 net 1/2] fm10k: Check tunnel header length in encap offload
From: Vick, Matthew @ 2014-11-14  0:41 UTC (permalink / raw)
  To: Joe Stringer, netdev@vger.kernel.org
  Cc: Kirsher, Jeffrey T, Linux NICS, therbert@google.com,
	gerlitz.or@gmail.com, alexander.duyck@gmail.com,
	linux-kernel@vger.kernel.org
In-Reply-To: <1415921801-10452-1-git-send-email-joestringer@nicira.com>

On 11/13/14, 3:36 PM, "Joe Stringer" <joestringer@nicira.com> wrote:

>fm10k supports up to 184 bytes of inner+outer headers. Add an initial
>check to fail encap offload if these are too large.
>
>Signed-off-by: Joe Stringer <joestringer@nicira.com>
>---
>Matthew, I didn't see the equivalent patch on netdev so I went ahead and
>created it. If I've missed this somewhere, then please disregard.
>
>v2: First post.

You didn't miss it Joe--it just hasn't made it up yet. :) It's currently
in Jeff's tree for testing. You're on the CC for the patch, so you'll get
a notification once it goes up. It's basically the same as what you have,
except the #define I use is 184 and I use inner_tcp_hdrlen() to account
for the inner TCP header length.

Since your second patch should apply cleanly on top of mine, what do you
think about dropping the first patch in this series and Jeff can send our
two patches up together once they've passed testing?

Cheers,
Matthew

^ permalink raw reply

* [PATCHv2 net 2/4] be2net: Implement ndo_gso_check()
From: Joe Stringer @ 2014-11-14  0:38 UTC (permalink / raw)
  To: netdev
  Cc: sathya.perla, shahed.shaikh, amirv, Dept-GELinuxNICDev, therbert,
	gerlitz.or, alexander.duyck, linux-kernel
In-Reply-To: <1415925495-59312-1-git-send-email-joestringer@nicira.com>

Use vxlan_gso_check() to advertise offload support for this NIC.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
---
v2: Refactor out vxlan helper.
---
 drivers/net/ethernet/emulex/benet/be_main.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 9a18e79..3e8475c 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -4421,6 +4421,11 @@ static void be_del_vxlan_port(struct net_device *netdev, sa_family_t sa_family,
 		 "Disabled VxLAN offloads for UDP port %d\n",
 		 be16_to_cpu(port));
 }
+
+static bool be_gso_check(struct sk_buff *skb, struct net_device *dev)
+{
+	return vxlan_gso_check(skb);
+}
 #endif
 
 static const struct net_device_ops be_netdev_ops = {
@@ -4450,6 +4455,7 @@ static const struct net_device_ops be_netdev_ops = {
 #ifdef CONFIG_BE2NET_VXLAN
 	.ndo_add_vxlan_port	= be_add_vxlan_port,
 	.ndo_del_vxlan_port	= be_del_vxlan_port,
+	.ndo_gso_check		= be_gso_check,
 #endif
 };
 
-- 
1.7.10.4

^ permalink raw reply related

* [PATCHv2 net 4/4] qlcnic: Implement ndo_gso_check()
From: Joe Stringer @ 2014-11-14  0:38 UTC (permalink / raw)
  To: netdev
  Cc: sathya.perla, shahed.shaikh, amirv, Dept-GELinuxNICDev, therbert,
	gerlitz.or, alexander.duyck, linux-kernel
In-Reply-To: <1415925495-59312-1-git-send-email-joestringer@nicira.com>

Use vxlan_gso_check() to advertise offload support for this NIC.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
---
v2: Refactor out vxlan helper.
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
index f5e29f7..a913b3a 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
@@ -503,6 +503,11 @@ static void qlcnic_del_vxlan_port(struct net_device *netdev,
 
 	adapter->flags |= QLCNIC_DEL_VXLAN_PORT;
 }
+
+static bool qlcnic_gso_check(struct sk_buff *skb, struct net_device *dev)
+{
+	return vxlan_gso_check(skb);
+}
 #endif
 
 static const struct net_device_ops qlcnic_netdev_ops = {
@@ -526,6 +531,7 @@ static const struct net_device_ops qlcnic_netdev_ops = {
 #ifdef CONFIG_QLCNIC_VXLAN
 	.ndo_add_vxlan_port	= qlcnic_add_vxlan_port,
 	.ndo_del_vxlan_port	= qlcnic_del_vxlan_port,
+	.ndo_gso_check		= qlcnic_gso_check,
 #endif
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	.ndo_poll_controller = qlcnic_poll_controller,
-- 
1.7.10.4

^ permalink raw reply related

* [PATCHv2 net 3/4] net/mlx4_en: Implement ndo_gso_check()
From: Joe Stringer @ 2014-11-14  0:38 UTC (permalink / raw)
  To: netdev
  Cc: sathya.perla, shahed.shaikh, amirv, Dept-GELinuxNICDev, therbert,
	gerlitz.or, alexander.duyck, linux-kernel
In-Reply-To: <1415925495-59312-1-git-send-email-joestringer@nicira.com>

Use vxlan_gso_check() to advertise offload support for this NIC.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
---
v2: Refactor out vxlan helper.
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 02266e3..c5fcc56 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -2355,6 +2355,11 @@ static void mlx4_en_del_vxlan_port(struct  net_device *dev,
 
 	queue_work(priv->mdev->workqueue, &priv->vxlan_del_task);
 }
+
+static bool mlx4_en_gso_check(struct sk_buff *skb, struct net_device *dev)
+{
+	return vxlan_gso_check(skb);
+}
 #endif
 
 static const struct net_device_ops mlx4_netdev_ops = {
@@ -2386,6 +2391,7 @@ static const struct net_device_ops mlx4_netdev_ops = {
 #ifdef CONFIG_MLX4_EN_VXLAN
 	.ndo_add_vxlan_port	= mlx4_en_add_vxlan_port,
 	.ndo_del_vxlan_port	= mlx4_en_del_vxlan_port,
+	.ndo_gso_check		= mlx4_en_gso_check,
 #endif
 };
 
-- 
1.7.10.4

^ permalink raw reply related

* [PATCHv2 net 1/4] net: Add vxlan_gso_check() helper
From: Joe Stringer @ 2014-11-14  0:38 UTC (permalink / raw)
  To: netdev
  Cc: sathya.perla, shahed.shaikh, amirv, Dept-GELinuxNICDev, therbert,
	gerlitz.or, alexander.duyck, linux-kernel
In-Reply-To: <1415925495-59312-1-git-send-email-joestringer@nicira.com>

Most NICs that report NETIF_F_GSO_UDP_TUNNEL support VXLAN, and not
other UDP-based encapsulation protocols where the format and size of the
header differs. This patch implements a generic ndo_gso_check() for
VXLAN which will only advertise GSO support when the skb looks like it
contains VXLAN (or no UDP tunnelling at all).

Implementation shamelessly stolen from Tom Herbert:
http://thread.gmane.org/gmane.linux.network/332428/focus=333111

Signed-off-by: Joe Stringer <joestringer@nicira.com>
---
v2: Merge helpers for be2net, mlx4, qlcnic
    Use (sizeof(struct udphdr) + sizeof(struct vxlanhdr))
v1: Initial post
---
 drivers/net/vxlan.c |   13 +++++++++++++
 include/net/vxlan.h |    2 ++
 2 files changed, 15 insertions(+)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index fa9dc45..6b65863 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1571,6 +1571,19 @@ static bool route_shortcircuit(struct net_device *dev, struct sk_buff *skb)
 	return false;
 }
 
+bool vxlan_gso_check(struct sk_buff *skb)
+{
+	if ((skb_shinfo(skb)->gso_type & SKB_GSO_UDP_TUNNEL) &&
+	    (skb->inner_protocol_type != ENCAP_TYPE_ETHER ||
+	     skb->inner_protocol != htons(ETH_P_TEB) ||
+	     (skb_inner_mac_header(skb) - skb_transport_header(skb) !=
+	      sizeof(struct udphdr) + sizeof(struct vxlanhdr))))
+		return false;
+
+	return true;
+}
+EXPORT_SYMBOL_GPL(vxlan_gso_check);
+
 #if IS_ENABLED(CONFIG_IPV6)
 static int vxlan6_xmit_skb(struct vxlan_sock *vs,
 			   struct dst_entry *dst, struct sk_buff *skb,
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index d5f59f3..afadf8e 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -45,6 +45,8 @@ int vxlan_xmit_skb(struct vxlan_sock *vs,
 		   __be32 src, __be32 dst, __u8 tos, __u8 ttl, __be16 df,
 		   __be16 src_port, __be16 dst_port, __be32 vni, bool xnet);
 
+bool vxlan_gso_check(struct sk_buff *skb);
+
 /* IP header + UDP + VXLAN + Ethernet header */
 #define VXLAN_HEADROOM (20 + 8 + 8 + 14)
 /* IPv6 header + UDP + VXLAN + Ethernet header */
-- 
1.7.10.4

^ 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