Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next 1/3] bpf: Implement map_delete_elem for BPF_MAP_TYPE_LPM_TRIE
From: Craig Gallek @ 2017-09-19 15:08 UTC (permalink / raw)
  To: Alexei Starovoitov; +Cc: Daniel Mack, Daniel Borkmann, David S . Miller, netdev
In-Reply-To: <3e537b56-d0f2-de9a-5bb1-f60fbfc11ca5@fb.com>

On Mon, Sep 18, 2017 at 6:53 PM, Alexei Starovoitov <ast@fb.com> wrote:
Thanks for the review!  Please correct me if I'm wrong...

> On 9/18/17 12:30 PM, Craig Gallek wrote:
>>
>> From: Craig Gallek <kraig@google.com>
>>
>> This is a simple non-recursive delete operation.  It prunes paths
>> of empty nodes in the tree, but it does not try to further compress
>> the tree as nodes are removed.
>>
>> Signed-off-by: Craig Gallek <kraig@google.com>
>> ---
>>  kernel/bpf/lpm_trie.c | 80
>> +++++++++++++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 77 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c
>> index 1b767844a76f..9d58a576b2ae 100644
>> --- a/kernel/bpf/lpm_trie.c
>> +++ b/kernel/bpf/lpm_trie.c
>> @@ -389,10 +389,84 @@ static int trie_update_elem(struct bpf_map *map,
>>         return ret;
>>  }
>>
>> -static int trie_delete_elem(struct bpf_map *map, void *key)
>> +/* Called from syscall or from eBPF program */
>> +static int trie_delete_elem(struct bpf_map *map, void *_key)
>>  {
>> -       /* TODO */
>> -       return -ENOSYS;
>> +       struct lpm_trie *trie = container_of(map, struct lpm_trie, map);
>> +       struct bpf_lpm_trie_key *key = _key;
>> +       struct lpm_trie_node __rcu **trim;
>> +       struct lpm_trie_node *node;
>> +       unsigned long irq_flags;
>> +       unsigned int next_bit;
>> +       size_t matchlen = 0;
>> +       int ret = 0;
>> +
>> +       if (key->prefixlen > trie->max_prefixlen)
>> +               return -EINVAL;
>> +
>> +       raw_spin_lock_irqsave(&trie->lock, irq_flags);
>> +
>> +       /* Walk the tree looking for an exact key/length match and keeping
>> +        * track of where we could begin trimming the tree.  The
>> trim-point
>> +        * is the sub-tree along the walk consisting of only single-child
>> +        * intermediate nodes and ending at a leaf node that we want to
>> +        * remove.
>> +        */
>> +       trim = &trie->root;
>> +       node = rcu_dereference_protected(
>> +               trie->root, lockdep_is_held(&trie->lock));
>> +       while (node) {
>> +               matchlen = longest_prefix_match(trie, node, key);
>> +
>> +               if (node->prefixlen != matchlen ||
>> +                   node->prefixlen == key->prefixlen)
>> +                       break;
>
>
> curious why there is no need to do
> 'node->prefixlen == trie->max_prefixlen' in the above
> like update/lookup do?
I don't believe the node->prefixlen == trie->max_prefixlen check in
trie_update_elem is necessary. In order to get to this third clause,
it implies that the first two clauses evaluated false.  Which happens
when we find an exact prefix match for the current node, but the
to-be-inserted key prefix is different.  If the node we are comparing
against had a prefix of max_prefixlen, it would not be possible to
have both a full prefix match but different prefix lengths.  This
assumes that there are no nodes in the tree with > max_prefixlen
prefixes, but that is handled earlier in the update function.

There's a similar (I believe) unnecessary max_prefixlen check in
trie_lookup_elem.  The function should behave the same way without
that check, but at least in this case it's used as an early-out and
saves a few lines of execution.

>
>> +
>> +               next_bit = extract_bit(key->data, node->prefixlen);
>> +               /* If we hit a node that has more than one child or is a
>> valid
>> +                * prefix itself, do not remove it. Reset the root of the
>> trim
>> +                * path to its descendant on our path.
>> +                */
>> +               if (!(node->flags & LPM_TREE_NODE_FLAG_IM) ||
>> +                   (node->child[0] && node->child[1]))
>> +                       trim = &node->child[next_bit];
>> +               node = rcu_dereference_protected(
>> +                       node->child[next_bit],
>> lockdep_is_held(&trie->lock));
>> +       }
>> +
>> +       if (!node || node->prefixlen != key->prefixlen ||
>> +           (node->flags & LPM_TREE_NODE_FLAG_IM)) {
>> +               ret = -ENOENT;
>> +               goto out;
>> +       }
>> +
>> +       trie->n_entries--;
>> +
>> +       /* If the node we are removing is not a leaf node, simply mark it
>> +        * as intermediate and we are done.
>> +        */
>> +       if (rcu_access_pointer(node->child[0]) ||
>> +           rcu_access_pointer(node->child[1])) {
>> +               node->flags |= LPM_TREE_NODE_FLAG_IM;
>> +               goto out;
>> +       }
>> +
>> +       /* trim should now point to the slot holding the start of a path
>> from
>> +        * zero or more intermediate nodes to our leaf node for deletion.
>> +        */
>> +       while ((node = rcu_dereference_protected(
>> +                       *trim, lockdep_is_held(&trie->lock)))) {
>> +               RCU_INIT_POINTER(*trim, NULL);
>> +               trim = rcu_access_pointer(node->child[0]) ?
>> +                       &node->child[0] :
>> +                       &node->child[1];
>> +               kfree_rcu(node, rcu);
>
>
> can it be that some of the nodes this loop walks have
> both child[0] and [1] ?
No, the loop above will push trim down the walk every time it
encounters a node with two children.  The only other trim assignment
is the initial trim = &trie->root.  But the only time we would skip
the assignment in the loop is if the node being removed is the root.
If the root had multiple children and is being removed, it would be
handled by the case that turns the node into an intermediate node
rather than walking the trim path freeing things.

^ permalink raw reply

* Re: [PATCH net-next 4/4] test_rhashtable: add test case for rhl_table interface
From: kbuild test robot @ 2017-09-19 14:59 UTC (permalink / raw)
  To: Florian Westphal; +Cc: kbuild-all, netdev, Florian Westphal
In-Reply-To: <20170918210711.10202-5-fw@strlen.de>

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

Hi Florian,

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Florian-Westphal/test_rhashtable-add-test-case-for-rhl-table/20170919-135550
config: x86_64-randconfig-a0-09192105 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   lib/test_rhashtable.c: In function 'test_rhltable':
>> lib/test_rhashtable.c:433: warning: the frame size of 2144 bytes is larger than 2048 bytes

vim +433 lib/test_rhashtable.c

   254	
   255	static int __init test_rhltable(unsigned int entries)
   256	{
   257		struct test_obj_rhl *rhl_test_objects;
   258		unsigned long *obj_in_table;
   259		struct rhltable rhlt;
   260		unsigned int i, j, k;
   261		int ret, err;
   262	
   263		if (entries == 0)
   264			entries = 1;
   265	
   266		rhl_test_objects = vzalloc(sizeof(*rhl_test_objects) * entries);
   267		if (!rhl_test_objects)
   268			return -ENOMEM;
   269	
   270		ret = -ENOMEM;
   271		obj_in_table = vzalloc(BITS_TO_LONGS(entries) * sizeof(unsigned long));
   272		if (!obj_in_table)
   273			goto out_free;
   274	
   275		/* nulls_base not supported in rhlist interface */
   276		test_rht_params.nulls_base = 0;
   277		err = rhltable_init(&rhlt, &test_rht_params);
   278		if (WARN_ON(err))
   279			goto out_free;
   280	
   281		k = prandom_u32();
   282		ret = 0;
   283		for (i = 0; i < entries; i++) {
   284			rhl_test_objects[i].value.id = k;
   285			err = rhltable_insert(&rhlt, &rhl_test_objects[i].list_node,
   286					      test_rht_params);
   287			if (WARN(err, "error %d on element %d\n", err, i))
   288				break;
   289			if (err == 0)
   290				set_bit(i, obj_in_table);
   291		}
   292	
   293		if (err)
   294			ret = err;
   295	
   296		pr_info("test %d add/delete pairs into rhlist\n", entries);
   297		for (i = 0; i < entries; i++) {
   298			struct rhlist_head *h, *pos;
   299			struct test_obj_rhl *obj;
   300			struct test_obj_val key = {
   301				.id = k,
   302			};
   303			bool found;
   304	
   305			rcu_read_lock();
   306			h = rhltable_lookup(&rhlt, &key, test_rht_params);
   307			if (WARN(!h, "key not found during iteration %d of %d", i, entries)) {
   308				rcu_read_unlock();
   309				break;
   310			}
   311	
   312			if (i) {
   313				j = i - 1;
   314				rhl_for_each_entry_rcu(obj, pos, h, list_node) {
   315					if (WARN(pos == &rhl_test_objects[j].list_node, "old element found, should be gone"))
   316						break;
   317				}
   318			}
   319	
   320			cond_resched_rcu();
   321	
   322			found = false;
   323	
   324			rhl_for_each_entry_rcu(obj, pos, h, list_node) {
   325				if (pos == &rhl_test_objects[i].list_node) {
   326					found = true;
   327					break;
   328				}
   329			}
   330	
   331			rcu_read_unlock();
   332	
   333			if (WARN(!found, "element %d not found", i))
   334				break;
   335	
   336			err = rhltable_remove(&rhlt, &rhl_test_objects[i].list_node, test_rht_params);
   337			WARN(err, "rhltable_remove: err %d for iteration %d\n", err, i);
   338			if (err == 0)
   339				clear_bit(i, obj_in_table);
   340		}
   341	
   342		if (ret == 0 && err)
   343			ret = err;
   344	
   345		for (i = 0; i < entries; i++) {
   346			WARN(test_bit(i, obj_in_table), "elem %d allegedly still present", i);
   347	
   348			err = rhltable_insert(&rhlt, &rhl_test_objects[i].list_node,
   349					      test_rht_params);
   350			if (WARN(err, "error %d on element %d\n", err, i))
   351				break;
   352			if (err == 0)
   353				set_bit(i, obj_in_table);
   354		}
   355	
   356		pr_info("test %d random rhlist add/delete operations\n", entries);
   357		for (j = 0; j < entries; j++) {
   358			u32 i = prandom_u32_max(entries);
   359			u32 prand = prandom_u32();
   360	
   361			cond_resched();
   362	
   363			if (prand == 0)
   364				prand = prandom_u32();
   365	
   366			if (prand & 1) {
   367				prand >>= 1;
   368				continue;
   369			}
   370	
   371			err = rhltable_remove(&rhlt, &rhl_test_objects[i].list_node, test_rht_params);
   372			if (test_bit(i, obj_in_table)) {
   373				clear_bit(i, obj_in_table);
   374				if (WARN(err, "cannot remove element at slot %d", i))
   375					continue;
   376			} else {
   377				if (WARN(err != -ENOENT, "removed non-existant element %d, error %d not %d",
   378				     i, err, -ENOENT))
   379					continue;
   380			}
   381	
   382			if (prand & 1) {
   383				prand >>= 1;
   384				continue;
   385			}
   386	
   387			err = rhltable_insert(&rhlt, &rhl_test_objects[i].list_node, test_rht_params);
   388			if (err == 0) {
   389				if (WARN(test_and_set_bit(i, obj_in_table), "succeeded to insert same object %d", i))
   390					continue;
   391			} else {
   392				if (WARN(!test_bit(i, obj_in_table), "failed to insert object %d", i))
   393					continue;
   394			}
   395	
   396			if (prand & 1) {
   397				prand >>= 1;
   398				continue;
   399			}
   400	
   401			i = prandom_u32_max(entries);
   402			if (test_bit(i, obj_in_table)) {
   403				err = rhltable_remove(&rhlt, &rhl_test_objects[i].list_node, test_rht_params);
   404				WARN(err, "cannot remove element at slot %d", i);
   405				if (err == 0)
   406					clear_bit(i, obj_in_table);
   407			} else {
   408				err = rhltable_insert(&rhlt, &rhl_test_objects[i].list_node, test_rht_params);
   409				WARN(err, "failed to insert object %d", i);
   410				if (err == 0)
   411					set_bit(i, obj_in_table);
   412			}
   413		}
   414	
   415		for (i = 0; i < entries; i++) {
   416			cond_resched();
   417			err = rhltable_remove(&rhlt, &rhl_test_objects[i].list_node, test_rht_params);
   418			if (test_bit(i, obj_in_table)) {
   419				if (WARN(err, "cannot remove element at slot %d", i))
   420					continue;
   421			} else {
   422				if (WARN(err != -ENOENT, "removed non-existant element, error %d not %d",
   423					 err, -ENOENT))
   424				continue;
   425			}
   426		}
   427	
   428		rhltable_destroy(&rhlt);
   429	out_free:
   430		vfree(rhl_test_objects);
   431		vfree(obj_in_table);
   432		return ret;
 > 433	}
   434	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25659 bytes --]

^ permalink raw reply

* Re: software interrupts close to 100 with 9000 tc filter entries
From: Marco Berizzi @ 2017-09-19 14:57 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1505829713.29839.57.camel@edumazet-glaptop3.roam.corp.google.com>

> Eric Dumazet wrote:
> 
> On Tue, 2017-09-19 at 15:28 +0200, Marco Berizzi wrote:
> 
> > Hi Folks,
> > 
> > I'm running linux 4.12.10 x86_64 on a Slackware 14.2 64bit
> > as a simple 4 NIC router. Network throughput processed by
> > this machine is less than 200Mbit/s
> > The cpu model is Intel(R) Xeon(R) CPU 5160 @ 3.00GHz with
> > 2GB ram.
> > 
> > I need to blacklist about 9000 single ip addresses.
> > This is the relevant script to blacklist these ip addresses:
> > 
> > tc qdisc add dev eth0 ingress
> > tc qdisc add dev eth1 ingress
> > 
> > while read -r line
> > do
> >  tc filter add dev eth0 parent ffff: protocol ip prio 50 u32 match ip src $line action drop
> >  tc filter add dev eth1 parent ffff: protocol ip prio 50 u32 match ip src $line action drop
> > done < blacklisted_ip_addresses
> > 
> > After loading these ip addresses, the si (software interrupts)
> > number shown by top is always close to 100
> > If I delete the ingress qdisc on both the device, the si
> > fall down to less than 5
> > 
> > Running the same script with 'only' 700 ip addresses is
> > flawless.
> > 
> > Kindly I would like to ask if am I doing anything in
> > a wrong way or if the hardware is too old for this kind
> > of setup.
> > 
> > I have selected the tc filter setup instead of netfilter
> > one, because I was reading this from iproute2/doc/actions:
> > 
> > A side effect is that we can now get stateless firewalling to work with tc..
> > Essentially this is now an alternative to iptables.
> > I wont go into details of my dislike for iptables at times, but.
> > scalability is one of the main issues; however, if you need stateful
> > classification - use netfilter (for now).
> > 
> > Any response are welcome
> > TIA
> 
> Processing a list of 700 rules per incoming packet is not wise.
> 
> Alternatives :
> 
> *   netfilter with IPSET : This probably can be done with one lookup in a
> table. Probably easiest way to setup.
> 
> *   BPF filter (XDP or TC )

Thanks Eric for the quick response.
For better performance (latency time and network throughput) which is the better
solution? netfilter with ipset or BPF?

^ permalink raw reply

* [RFC 1/1] net/smc: add SMC rendezvous protocol
From: Ursula Braun @ 2017-09-19 14:55 UTC (permalink / raw)
  To: netdev; +Cc: hwippel, raspl, davem, ubraun

The SMC protocol [1] uses a rendezvous protocol to negotiate SMC
capability between peers. The current Linux implementation does not use
this rendezvous protocol and, thus, is not compliant to RFC7609 and
incompatible with other SMC implementations like in zOS. This patch adds
support for the SMC rendezvous protocol.

Details:

The SMC rendezvous protocol relies on the use of a new TCP experimental
option. With this option, SMC capabilities are exchanged between the
peers during the TCP three way handshake.

The goal of this patch is to leave common TCP code unmodified. Thus,
it uses netfilter hooks to intercept TCP SYN and SYN/ACK packets. For
outgoing packets originating from SMC sockets, the experimental option
is added. For inbound packets destined for SMC sockets, the experimental
option is checked.

Another goal was to minimize the performance impact on non-SMC traffic
(when SMC is enabled). The netfilter hooks used for SMC client
connections are active only during TCP connection establishment.
The netfilter hooks used for SMC servers are active as long as there are
listening SMC sockets.

When the hooks are active, the following additional operations are
performed on incoming and outgoing packets:
  (1) call SMC netfilter hook (all IPv4 packets)
  (2) check if TCP SYN or SYN/ACK packet (all IPv4 packets)
  (3) check if packet goes to/comes from SMC socket (SYN & SYN/ACK
      packets only)
  (4) check/add SMC experimental option (SMC sockets' SYN & SYN/ACK
      packets only)

References:
  [1] SMC-R Informational RFC: http://www.rfc-editor.org/info/rfc7609

Signed-off-by: Hans Wippel <hwippel@linux.net.ibm.com>
Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
 net/smc/Makefile |   2 +-
 net/smc/af_smc.c |  66 ++++++-
 net/smc/smc.h    |  10 +-
 net/smc/smc_rv.c | 542 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 net/smc/smc_rv.h |  31 ++++
 5 files changed, 644 insertions(+), 7 deletions(-)
 create mode 100644 net/smc/smc_rv.c
 create mode 100644 net/smc/smc_rv.h

diff --git a/net/smc/Makefile b/net/smc/Makefile
index 188104654b54..2155a7eff41d 100644
--- a/net/smc/Makefile
+++ b/net/smc/Makefile
@@ -1,4 +1,4 @@
 obj-$(CONFIG_SMC)	+= smc.o
 obj-$(CONFIG_SMC_DIAG)	+= smc_diag.o
 smc-y := af_smc.o smc_pnet.o smc_ib.o smc_clc.o smc_core.o smc_wr.o smc_llc.o
-smc-y += smc_cdc.o smc_tx.o smc_rx.o smc_close.o
+smc-y += smc_cdc.o smc_tx.o smc_rx.o smc_close.o smc_rv.o
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 8c6d24b2995d..6c280bbcd2fe 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -34,6 +34,7 @@
 #include <net/smc.h>
 
 #include "smc.h"
+#include "smc_rv.h"
 #include "smc_clc.h"
 #include "smc_llc.h"
 #include "smc_cdc.h"
@@ -109,6 +110,7 @@ static int smc_release(struct socket *sock)
 {
 	struct sock *sk = sock->sk;
 	struct smc_sock *smc;
+	int old_state;
 	int rc = 0;
 
 	if (!sk)
@@ -123,6 +125,7 @@ static int smc_release(struct socket *sock)
 		lock_sock_nested(sk, SINGLE_DEPTH_NESTING);
 	else
 		lock_sock(sk);
+	old_state = sk->sk_state;
 
 	if (smc->use_fallback) {
 		sk->sk_state = SMC_CLOSED;
@@ -132,6 +135,10 @@ static int smc_release(struct socket *sock)
 		sock_set_flag(sk, SOCK_DEAD);
 		sk->sk_shutdown |= SHUTDOWN_MASK;
 	}
+	if (old_state == SMC_LISTEN) {
+		smc_rv_nf_unregister_hook(sock_net(sk), &smc_nfho_serv);
+		kfree(smc->listen_pends);
+	}
 	if (smc->clcsock) {
 		sock_release(smc->clcsock);
 		smc->clcsock = NULL;
@@ -178,6 +185,7 @@ static struct sock *smc_sock_alloc(struct net *net, struct socket *sock)
 	sk->sk_destruct = smc_destruct;
 	sk->sk_protocol = SMCPROTO_SMC;
 	smc = smc_sk(sk);
+	smc->use_fallback = true; /* default: not SMC-capable */
 	INIT_WORK(&smc->tcp_listen_work, smc_tcp_listen_work);
 	INIT_LIST_HEAD(&smc->accept_q);
 	spin_lock_init(&smc->accept_q_lock);
@@ -386,6 +394,10 @@ static int smc_connect_rdma(struct smc_sock *smc)
 	int rc = 0;
 	u8 ibport;
 
+	if (smc->use_fallback)
+		/* peer has not signalled SMC-capability */
+		goto out_connected;
+
 	/* IPSec connections opt out of SMC-R optimizations */
 	if (using_ipsec(smc)) {
 		reason_code = SMC_CLC_DECL_IPSEC;
@@ -496,7 +508,6 @@ static int smc_connect_rdma(struct smc_sock *smc)
 	smc_tx_init(smc);
 
 out_connected:
-	smc_copy_sock_settings_to_clc(smc);
 	if (smc->sk.sk_state == SMC_INIT)
 		smc->sk.sk_state = SMC_ACTIVE;
 
@@ -551,7 +562,11 @@ static int smc_connect(struct socket *sock, struct sockaddr *addr,
 	}
 
 	smc_copy_sock_settings_to_clc(smc);
+	smc_rv_nf_register_hook(sock_net(sk), &smc_nfho_clnt);
+
 	rc = kernel_connect(smc->clcsock, addr, alen, flags);
+	if (rc != -EINPROGRESS)
+		smc_rv_nf_unregister_hook(sock_net(sk), &smc_nfho_clnt);
 	if (rc)
 		goto out;
 
@@ -570,10 +585,12 @@ static int smc_connect(struct socket *sock, struct sockaddr *addr,
 
 static int smc_clcsock_accept(struct smc_sock *lsmc, struct smc_sock **new_smc)
 {
+	struct smc_listen_pending *pnd;
 	struct sock *sk = &lsmc->sk;
 	struct socket *new_clcsock;
 	struct sock *new_sk;
-	int rc;
+	unsigned long flags;
+	int i, rc;
 
 	release_sock(&lsmc->sk);
 	new_sk = smc_sock_alloc(sock_net(sk), NULL);
@@ -609,6 +626,25 @@ static int smc_clcsock_accept(struct smc_sock *lsmc, struct smc_sock **new_smc)
 	}
 
 	(*new_smc)->clcsock = new_clcsock;
+
+	/* enable SMC-capability if an SMC-capable connecting socket is
+	 * contained in listen_pends; invalidate this entry
+	 */
+	spin_lock_irqsave(&lsmc->listen_pends_lock, flags);
+	for (i = 0; i < 2 * lsmc->sk.sk_max_ack_backlog; i++) {
+		pnd = lsmc->listen_pends + i;
+		if (pnd->used &&
+		    pnd->addr == new_clcsock->sk->sk_daddr &&
+		    pnd->port == new_clcsock->sk->sk_dport &&
+		    jiffies_to_msecs(get_jiffies_64() - pnd->time) <=
+						SMC_LISTEN_PEND_VALID_TIME) {
+			(*new_smc)->use_fallback = false;
+			pnd->used = false;
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&lsmc->listen_pends_lock, flags);
+
 out:
 	return rc;
 }
@@ -755,6 +791,10 @@ static void smc_listen_work(struct work_struct *work)
 	u8 prefix_len;
 	u8 ibport;
 
+	if (new_smc->use_fallback)
+		/* peer has not signalled SMC-capability */
+		goto out_connected;
+
 	/* do inband token exchange -
 	 *wait for and receive SMC Proposal CLC message
 	 */
@@ -927,7 +967,6 @@ static void smc_tcp_listen_work(struct work_struct *work)
 			continue;
 
 		new_smc->listen_smc = lsmc;
-		new_smc->use_fallback = false; /* assume rdma capability first*/
 		sock_hold(&lsmc->sk); /* sock_put in smc_listen_work */
 		INIT_WORK(&new_smc->smc_listen_work, smc_listen_work);
 		smc_copy_sock_settings_to_smc(new_smc);
@@ -952,16 +991,32 @@ static int smc_listen(struct socket *sock, int backlog)
 	if ((sk->sk_state != SMC_INIT) && (sk->sk_state != SMC_LISTEN))
 		goto out;
 
+	rc = -ENOMEM;
+	/* Addresses and ports of incoming SYN packets with experimental option
+	 * SMC are saved, but TCP might decide to drop them. Thus more slots
+	 * than the backlog value are allocated for pending connecting sockets
+	 */
+	smc->listen_pends = kzalloc(
+			2 * backlog * sizeof(struct smc_listen_pending),
+			GFP_KERNEL);
+	if (!smc->listen_pends)
+		goto out;
+	spin_lock_init(&smc->listen_pends_lock);
+
 	rc = 0;
 	if (sk->sk_state == SMC_LISTEN) {
 		sk->sk_max_ack_backlog = backlog;
 		goto out;
 	}
+
+	smc->use_fallback = false; /* listen sockets are SMC-capable */
 	/* some socket options are handled in core, so we could not apply
 	 * them to the clc socket -- copy smc socket options to clc socket
 	 */
 	smc_copy_sock_settings_to_clc(smc);
 
+	smc_rv_nf_register_hook(sock_net(sk), &smc_nfho_serv);
+
 	rc = kernel_listen(smc->clcsock, backlog);
 	if (rc)
 		goto out;
@@ -1112,7 +1167,7 @@ static unsigned int smc_poll(struct file *file, struct socket *sock,
 	struct sock *sk = sock->sk;
 	unsigned int mask = 0;
 	struct smc_sock *smc;
-	int rc;
+	int rc = 0;
 
 	smc = smc_sk(sock->sk);
 	if ((sk->sk_state == SMC_INIT) || smc->use_fallback) {
@@ -1121,6 +1176,7 @@ static unsigned int smc_poll(struct file *file, struct socket *sock,
 		/* if non-blocking connect finished ... */
 		lock_sock(sk);
 		if ((sk->sk_state == SMC_INIT) && (mask & POLLOUT)) {
+			smc_rv_nf_unregister_hook(sock_net(sk), &smc_nfho_clnt);
 			sk->sk_err = smc->clcsock->sk->sk_err;
 			if (sk->sk_err) {
 				mask |= POLLERR;
@@ -1346,7 +1402,6 @@ static int smc_create(struct net *net, struct socket *sock, int protocol,
 
 	/* create internal TCP socket for CLC handshake and fallback */
 	smc = smc_sk(sk);
-	smc->use_fallback = false; /* assume rdma capability first */
 	rc = sock_create_kern(net, PF_INET, SOCK_STREAM,
 			      IPPROTO_TCP, &smc->clcsock);
 	if (rc)
@@ -1368,6 +1423,7 @@ static int __init smc_init(void)
 {
 	int rc;
 
+	smc_rv_init();
 	rc = smc_pnet_init();
 	if (rc)
 		return rc;
diff --git a/net/smc/smc.h b/net/smc/smc.h
index 6e44313e4467..48097ca7a7fe 100644
--- a/net/smc/smc.h
+++ b/net/smc/smc.h
@@ -167,6 +167,13 @@ struct smc_connection {
 	struct work_struct	close_work;	/* peer sent some closing */
 };
 
+struct smc_listen_pending {
+	u64		time;			/* time when entry was created*/
+	bool		used;			/* true if entry is in use */
+	__be32		addr;			/* address of a listen socket */
+	__be16		port;			/* port of a listen socket */
+};
+
 struct smc_sock {				/* smc sock container */
 	struct sock		sk;
 	struct socket		*clcsock;	/* internal tcp socket */
@@ -175,6 +182,8 @@ struct smc_sock {				/* smc sock container */
 	struct smc_sock		*listen_smc;	/* listen parent */
 	struct work_struct	tcp_listen_work;/* handle tcp socket accepts */
 	struct work_struct	smc_listen_work;/* prepare new accept socket */
+	struct smc_listen_pending *listen_pends;/* listen pending SYNs */
+	spinlock_t		listen_pends_lock; /* protects listen_pends */
 	struct list_head	accept_q;	/* sockets to be accepted */
 	spinlock_t		accept_q_lock;	/* protects accept_q */
 	struct delayed_work	sock_put_work;	/* final socket freeing */
@@ -271,5 +280,4 @@ int smc_conn_create(struct smc_sock *smc, __be32 peer_in_addr,
 		    struct smc_clc_msg_local *lcl, int srv_first_contact);
 struct sock *smc_accept_dequeue(struct sock *parent, struct socket *new_sock);
 void smc_close_non_accepted(struct sock *sk);
-
 #endif	/* __SMC_H */
diff --git a/net/smc/smc_rv.c b/net/smc/smc_rv.c
new file mode 100644
index 000000000000..ae51e4120d7e
--- /dev/null
+++ b/net/smc/smc_rv.c
@@ -0,0 +1,542 @@
+/*
+ *  Shared Memory Communications over RDMA (SMC-R) and RoCE
+ *
+ *  SMC Rendezvous to determine SMC-capability of the peer
+ *
+ *  Copyright IBM Corp. 2017
+ *
+ *  Author(s):  Hans Wippel <hwippel@linux.vnet.ibm.com>
+ *              Ursula Braun <ubraun@linux.vnet.ibm.com>
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/netdevice.h>
+#include <linux/netfilter.h>
+#include <linux/netfilter_ipv4.h>
+#include <linux/ip.h>
+#include <linux/tcp.h>
+#include <net/tcp.h>
+
+#include "smc.h"
+#include "smc_rv.h"
+
+#define TCPOLEN_SMC			8
+#define TCPOLEN_SMC_BASE		6
+#define TCPOLEN_SMC_ALIGNED		2
+
+static const char TCPOPT_SMC_MAGIC[4] = {'\xe2', '\xd4', '\xc3', '\xd9'};
+
+/* in TCP header, replace EOL option and remaining header bytes with NOPs */
+static bool smc_rv_replace_eol_option(struct sk_buff *skb)
+{
+	struct tcphdr *tcph = tcp_hdr(skb);
+	int opt_bytes = tcp_optlen(skb);
+	unsigned char *buf;
+	int i = 0;
+
+	buf = (unsigned char *)(tcph + 1);
+	/* Parse TCP options. Based on tcp_parse_options in tcp_input.c */
+	while (i < opt_bytes) {
+		switch (buf[i]) {
+		/* one byte options */
+		case TCPOPT_EOL:
+			/* replace remaining bytes with NOPs */
+			while (i < opt_bytes) {
+				buf[i] = TCPOPT_NOP;
+				i++;
+			}
+			return true;
+		case TCPOPT_NOP:
+			i++;
+			continue;
+		default:
+			/* multi-byte options */
+			if (buf[i + 1] < 2 || i + buf[i + 1] > opt_bytes)
+				return false; /* bad option */
+			i += buf[i + 1];
+			continue;
+		}
+	}
+	return true;
+}
+
+/* check if TCP header contains SMC option */
+static bool smc_rv_has_smc_option(struct sk_buff *skb)
+{
+	struct tcphdr *tcph = tcp_hdr(skb);
+	int opt_bytes = tcp_optlen(skb);
+	unsigned char *buf;
+	int i = 0;
+
+	buf = (unsigned char *)(tcph + 1);
+	/* Parse TCP options. Based on tcp_parse_options in tcp_input.c */
+	while (i < opt_bytes) {
+		switch (buf[i]) {
+		/* one byte options */
+		case TCPOPT_EOL:
+			return 0;
+		case TCPOPT_NOP:
+			i++;
+			continue;
+		default:
+			/* multi-byte options */
+			if (buf[i + 1] < 2)
+				return 0; /* bad option */
+			/* check for SMC rendezvous option */
+			if (buf[i] == TCPOPT_EXP &&
+			    buf[i + 1] == TCPOLEN_SMC_BASE &&
+			    (opt_bytes - i >= TCPOLEN_SMC_BASE) &&
+			    !memcmp(&buf[i + 2], TCPOPT_SMC_MAGIC,
+						sizeof(TCPOPT_SMC_MAGIC)))
+				return true;
+			i += buf[i + 1];
+			continue;
+		}
+	}
+
+	return false;
+}
+
+/* Add SMC option to TCP header. Note: This assumes that there are no data after
+ * the TCP header.
+ */
+static int smc_rv_add_smc_option(struct sk_buff *skb)
+{
+	unsigned char smc_opt[] = {TCPOPT_NOP, TCPOPT_NOP,
+				   TCPOPT_EXP, TCPOLEN_SMC_BASE,
+				   TCPOPT_SMC_MAGIC[0], TCPOPT_SMC_MAGIC[1],
+				   TCPOPT_SMC_MAGIC[2], TCPOPT_SMC_MAGIC[3]};
+	struct tcphdr *tcph = tcp_hdr(skb);
+	struct iphdr *iph = ip_hdr(skb);
+	int tcplen = 0;
+
+	if (skb_tailroom(skb) < TCPOLEN_SMC)
+		return -EFAULT;
+
+	if (((tcph->doff << 2) - sizeof(*tcph) + TCPOLEN_SMC) >
+							MAX_TCP_OPTION_SPACE)
+		return -EFAULT;
+
+	if (smc_rv_has_smc_option(skb))
+		return -EFAULT;
+
+	if (!smc_rv_replace_eol_option(skb))
+		return -EFAULT;
+
+	iph->tot_len = cpu_to_be16(be16_to_cpu(iph->tot_len) + TCPOLEN_SMC);
+	iph->check = 0;
+	iph->check = ip_fast_csum(iph, iph->ihl);
+	skb_put_data(skb, smc_opt, TCPOLEN_SMC);
+	tcph->doff += TCPOLEN_SMC_ALIGNED;
+	tcplen = (skb->len - ip_hdrlen(skb));
+	tcph->check = 0;
+	tcph->check = tcp_v4_check(tcplen, iph->saddr, iph->daddr,
+				   csum_partial(tcph, tcplen, 0));
+	skb->ip_summed = CHECKSUM_NONE;
+	return 0;
+}
+
+/* return an smc socket with certain source and destination */
+static struct smc_sock *smc_rv_lookup_connecting_smc(struct net *net,
+						     __be32 dest_addr,
+						     __be16 dest_port,
+						     __be32 source_addr,
+						     __be16 source_port)
+{
+	struct smc_sock *smc = NULL;
+	struct hlist_head *head;
+	struct socket *clcsock;
+	struct sock *sk;
+
+	read_lock(&smc_proto.h.smc_hash->lock);
+	head = &smc_proto.h.smc_hash->ht;
+
+	if (hlist_empty(head))
+		goto out;
+
+	sk_for_each(sk, head) {
+		if (!net_eq(sock_net(sk), net))
+			continue;
+		if (sk->sk_state != SMC_INIT)
+			continue;
+		clcsock = smc_sk(sk)->clcsock;
+		if (!clcsock)
+			continue;
+		if (source_port != htons(clcsock->sk->sk_num))
+			continue;
+		if (source_addr != clcsock->sk->sk_rcv_saddr)
+			continue;
+		if (dest_port != clcsock->sk->sk_dport)
+			continue;
+		if (dest_addr == clcsock->sk->sk_daddr) {
+			smc = smc_sk(sk);
+			break;
+		}
+	}
+
+out:
+	read_unlock(&smc_proto.h.smc_hash->lock);
+	return smc;
+}
+
+/* for netfilter smc_rv_hook_out_clnt (outgoing SYN):
+ * check if there exists a connecting smc socket with certain source and
+ * destination
+ */
+static bool smc_rv_exists_connecting_smc(struct net *net,
+					 __be32 dest_addr,
+					 __be16 dest_port,
+					 __be32 source_addr,
+					 __be16 source_port)
+{
+	return (smc_rv_lookup_connecting_smc(net, dest_addr, dest_port,
+					     source_addr, source_port) ?
+								true : false);
+}
+
+/* for netfilter smc_rv_hook_in_clnt (incoming SYN ACK):
+ * enable SMC-capability for the corresponding socket
+ */
+static void smc_rv_accepting_smc_peer(struct net *net,
+				      __be32 dest_addr,
+				      __be16 dest_port,
+				      __be32 source_addr,
+				      __be16 source_port)
+{
+	struct smc_sock *smc;
+
+	smc = smc_rv_lookup_connecting_smc(net, dest_addr, dest_port,
+					   source_addr, source_port);
+	if (smc)
+		/* connection is SMC-capable */
+		smc->use_fallback = false;
+}
+
+/* return an smc socket listening on a certain port */
+static struct smc_sock *smc_rv_lookup_listen_socket(struct net *net,
+						    __be32 listen_addr,
+						    __be16 listen_port)
+{
+	struct smc_sock *smc = NULL;
+	struct hlist_head *head;
+	struct socket *clcsock;
+	struct sock *sk;
+
+	read_lock(&smc_proto.h.smc_hash->lock);
+	head = &smc_proto.h.smc_hash->ht;
+
+	if (hlist_empty(head))
+		goto out;
+
+	sk_for_each(sk, head) {
+		if (!net_eq(sock_net(sk), net))
+			continue;
+		if (sk->sk_state != SMC_LISTEN)
+			continue;
+		clcsock = smc_sk(sk)->clcsock;
+		if (listen_port != htons(clcsock->sk->sk_num))
+			continue;
+		if (!listen_addr || !clcsock->sk->sk_rcv_saddr ||
+		    listen_addr == clcsock->sk->sk_rcv_saddr) {
+			smc = smc_sk(sk);
+			break;
+		}
+	}
+
+out:
+	read_unlock(&smc_proto.h.smc_hash->lock);
+	return smc;
+}
+
+/* for netfilter smc_rv_hook_in_serv (incoming SYN):
+ * save addr and port of connecting smc peer
+ */
+static void smc_rv_connecting_smc_peer(struct net *net,
+				       __be32 listen_addr,
+				       __be16 listen_port,
+				       __be32 peer_addr,
+				       __be16 peer_port)
+{
+	struct smc_listen_pending *pnd;
+	struct smc_sock *lsmc;
+	unsigned long flags;
+	int i;
+
+	lsmc = smc_rv_lookup_listen_socket(net, listen_addr, listen_port);
+	if (!lsmc)
+		return;
+
+	spin_lock_irqsave(&lsmc->listen_pends_lock, flags);
+	for (i = 0; i < 2 * lsmc->sk.sk_max_ack_backlog; i++) {
+		pnd = lsmc->listen_pends + i;
+		/* either use an unused entry or reuse an outdated entry */
+		if (!pnd->used ||
+		    jiffies_to_msecs(get_jiffies_64() - pnd->time) >
+						SMC_LISTEN_PEND_VALID_TIME) {
+			pnd->used = true;
+			pnd->addr = peer_addr;
+			pnd->port = peer_port;
+			pnd->time = get_jiffies_64();
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&lsmc->listen_pends_lock, flags);
+}
+
+/* for netfilter smc_rv_hook_out_serv (outgoing SYN/ACK):
+ * remove listen_pends entry of connecting smc peer in case of a problem
+ */
+static void smc_rv_remove_smc_peer(struct net *net,
+				   __be32 listen_addr,
+				   __be16 listen_port,
+				   __be32 peer_addr,
+				   __be16 peer_port)
+{
+	struct smc_listen_pending *pnd;
+	struct smc_sock *lsmc;
+	unsigned long flags;
+	int i;
+
+	lsmc = smc_rv_lookup_listen_socket(net, listen_addr, listen_port);
+	if (!lsmc)
+		return;
+
+	spin_lock_irqsave(&lsmc->listen_pends_lock, flags);
+	for (i = 0; i < 2 * lsmc->sk.sk_max_ack_backlog; i++) {
+		pnd = lsmc->listen_pends + i;
+		if (pnd->used &&
+		    pnd->addr == peer_addr &&
+		    pnd->port == peer_port &&
+		    jiffies_to_msecs(get_jiffies_64() - pnd->time) <=
+						SMC_LISTEN_PEND_VALID_TIME) {
+			pnd->used = false;
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&lsmc->listen_pends_lock, flags);
+}
+
+/* for netfilter smc_rv_hook_out_serv (outgoing SYN ACK):
+ * check if there has been a connecting smc peer
+ */
+static bool smc_rv_exists_connecting_smc_peer(struct net *net,
+					      __be32 listen_addr,
+					      __be16 listen_port,
+					      __be32 peer_addr,
+					      __be16 peer_port)
+{
+	struct smc_listen_pending *pnd;
+	struct smc_sock *lsmc;
+	unsigned long flags;
+	int i;
+
+	lsmc = smc_rv_lookup_listen_socket(net, listen_addr, listen_port);
+	if (!lsmc)
+		return false;
+
+	spin_lock_irqsave(&lsmc->listen_pends_lock, flags);
+	for (i = 0; i < 2 * lsmc->sk.sk_max_ack_backlog; i++) {
+		pnd = lsmc->listen_pends + i;
+		if (pnd->used &&
+		    pnd->addr == peer_addr &&
+		    pnd->port == peer_port &&
+		    jiffies_to_msecs(get_jiffies_64() - pnd->time) <=
+						SMC_LISTEN_PEND_VALID_TIME) {
+			spin_unlock_irqrestore(&lsmc->listen_pends_lock, flags);
+			return true;
+		}
+	}
+	spin_unlock_irqrestore(&lsmc->listen_pends_lock, flags);
+	return false;
+}
+
+/* Netfilter hooks */
+
+/* netfilter hook for incoming packets (client) */
+static unsigned int smc_rv_hook_in_clnt(void *priv, struct sk_buff *skb,
+					const struct nf_hook_state *state)
+{
+	struct tcphdr *tcph = tcp_hdr(skb);
+	struct iphdr *iph;
+
+	if (skb_headlen(skb) - sizeof(*iph) < sizeof(*tcph))
+		return NF_ACCEPT;
+
+	iph = ip_hdr(skb);
+	if (iph->protocol != IPPROTO_TCP)
+		return NF_ACCEPT;
+
+	/* Local SMC client, incoming SYN,ACK from server
+	 * check if there really is a local SMC client
+	 * and tell the client connection if the server is SMC capable
+	 */
+	if (tcph->syn == 1 && tcph->ack == 1) {
+		/* check for experimental option */
+		if (!smc_rv_has_smc_option(skb))
+			return NF_ACCEPT;
+		/* add info about server SMC capability */
+		smc_rv_accepting_smc_peer(state->net, iph->saddr, tcph->source,
+					  iph->daddr, tcph->dest);
+	}
+	return NF_ACCEPT;
+}
+
+/* netfilter hook for incoming packets (server) */
+static unsigned int smc_rv_hook_in_serv(void *priv, struct sk_buff *skb,
+					const struct nf_hook_state *state)
+{
+	struct tcphdr *tcph = tcp_hdr(skb);
+	struct iphdr *iph;
+
+	if (skb_headlen(skb) - sizeof(*iph) < sizeof(*tcph))
+		return NF_ACCEPT;
+
+	iph = ip_hdr(skb);
+	if (iph->protocol != IPPROTO_TCP)
+		return NF_ACCEPT;
+
+	/* Local SMC Server, incoming SYN request from client
+	 * check if there is a local SMC server
+	 * and tell the server if there is a new SMC capable client
+	 */
+	if (tcph->syn == 1 && tcph->ack == 0) {
+		/* check for experimental option */
+		if (!smc_rv_has_smc_option(skb))
+			return NF_ACCEPT;
+		/* add info about new client SMC capability */
+		smc_rv_connecting_smc_peer(state->net, iph->daddr, tcph->dest,
+					   iph->saddr, tcph->source);
+	}
+	return NF_ACCEPT;
+}
+
+/* netfilter hook for outgoing packets (client) */
+static unsigned int smc_rv_hook_out_clnt(void *priv, struct sk_buff *skb,
+					 const struct nf_hook_state *state)
+{
+	struct tcphdr *tcph = tcp_hdr(skb);
+	struct iphdr *iph;
+
+	if (skb_headlen(skb) - sizeof(*iph) < sizeof(*tcph))
+		return NF_ACCEPT;
+
+	iph = ip_hdr(skb);
+	if (iph->protocol != IPPROTO_TCP)
+		return NF_ACCEPT;
+
+	/* Local SMC client, outgoing SYN request to server
+	 * add TCP experimental option if there really is a local SMC client
+	 */
+	if (tcph->syn == 1 && tcph->ack == 0) {
+		/* check for local SMC client */
+		if (!smc_rv_exists_connecting_smc(state->net,
+						  iph->daddr, tcph->dest,
+						  iph->saddr, tcph->source))
+			return NF_ACCEPT;
+		/* add experimental option */
+		smc_rv_add_smc_option(skb);
+	}
+	return NF_ACCEPT;
+}
+
+/* netfilter hook for outgoing packets (server) */
+static unsigned int smc_rv_hook_out_serv(void *priv, struct sk_buff *skb,
+					 const struct nf_hook_state *state)
+{
+	struct tcphdr *tcph = tcp_hdr(skb);
+	struct iphdr *iph;
+
+	if (skb_headlen(skb) - sizeof(*iph) < sizeof(*tcph))
+		return NF_ACCEPT;
+
+	iph = ip_hdr(skb);
+	if (iph->protocol != IPPROTO_TCP)
+		return NF_ACCEPT;
+
+	/* Local SMC server, outgoing SYN,ACK to client
+	 * add TCP experimental option if there really is a local SMC server
+	 */
+	if (tcph->syn == 1 && tcph->ack == 1) {
+		/* check if client's SYN contained the experimental option */
+		if (!smc_rv_exists_connecting_smc_peer(state->net,
+						       iph->saddr, tcph->source,
+						       iph->daddr, tcph->dest))
+			return NF_ACCEPT;
+		/* add experimental option */
+		if (smc_rv_add_smc_option(skb) < 0)
+			smc_rv_remove_smc_peer(state->net,
+					       iph->saddr, tcph->source,
+					       iph->daddr, tcph->dest);
+	}
+	return NF_ACCEPT;
+}
+
+static struct nf_hook_ops smc_nfho_ops_clnt[] = {
+	{
+		.hook = smc_rv_hook_in_clnt,
+		.hooknum = NF_INET_PRE_ROUTING,
+		.pf = PF_INET,
+		.priority = NF_IP_PRI_FIRST,
+	},
+	{
+		.hook = smc_rv_hook_out_clnt,
+		.hooknum = NF_INET_POST_ROUTING,
+		.pf = PF_INET,
+		.priority = NF_IP_PRI_FIRST,
+	},
+};
+
+static struct nf_hook_ops smc_nfho_ops_serv[] = {
+	{
+		.hook = smc_rv_hook_in_serv,
+		.hooknum = NF_INET_PRE_ROUTING,
+		.pf = PF_INET,
+		.priority = NF_IP_PRI_FIRST,
+	},
+	{
+		.hook = smc_rv_hook_out_serv,
+		.hooknum = NF_INET_POST_ROUTING,
+		.pf = PF_INET,
+		.priority = NF_IP_PRI_FIRST,
+	},
+};
+
+struct smc_nf_hook smc_nfho_clnt = {
+	.refcount = 0,
+	.hook = &smc_nfho_ops_clnt[0],
+};
+
+struct smc_nf_hook smc_nfho_serv = {
+	.refcount = 0,
+	.hook = &smc_nfho_ops_serv[0],
+};
+
+int smc_rv_nf_register_hook(struct net *net, struct smc_nf_hook *nfho)
+{
+	int rc = 0;
+
+	mutex_lock(&nfho->nf_hook_mutex);
+	if (!(nfho->refcount++)) {
+		rc = nf_register_net_hooks(net, nfho->hook, 2);
+		if (rc)
+			nfho->refcount--;
+	}
+	mutex_unlock(&nfho->nf_hook_mutex);
+	return rc;
+}
+
+void smc_rv_nf_unregister_hook(struct net *net, struct smc_nf_hook *nfho)
+{
+	mutex_lock(&nfho->nf_hook_mutex);
+	if (!(--nfho->refcount))
+		nf_unregister_net_hooks(net, nfho->hook, 2);
+	mutex_unlock(&nfho->nf_hook_mutex);
+}
+
+void __init smc_rv_init(void)
+{
+	mutex_init(&smc_nfho_clnt.nf_hook_mutex);
+	mutex_init(&smc_nfho_serv.nf_hook_mutex);
+}
diff --git a/net/smc/smc_rv.h b/net/smc/smc_rv.h
new file mode 100644
index 000000000000..d0aa9bc98a46
--- /dev/null
+++ b/net/smc/smc_rv.h
@@ -0,0 +1,31 @@
+/*
+ * Shared Memory Communications over RDMA (SMC-R) and RoCE
+ *
+ *  Definitions for SMC Rendezvous - SMC capability checking
+ *
+ *  Copyright IBM Corp. 2017
+ *
+ *  Author(s):  Hans Wippel <hwippel@linux.vnet.ibm.com>
+ *              Ursula Braun <ubraun@linux.vnet.ibm.com>
+ */
+
+#ifndef _SMC_RV_H
+#define _SMC_RV_H
+
+#include <linux/netfilter.h>
+
+#define SMC_LISTEN_PEND_VALID_TIME	(600 * HZ)
+
+struct smc_nf_hook {
+	struct mutex		nf_hook_mutex;	/* serialize nf register ops */
+	int			refcount;
+	struct nf_hook_ops	*hook;
+};
+
+extern struct smc_nf_hook smc_nfho_clnt;
+extern struct smc_nf_hook smc_nfho_serv;
+
+int smc_rv_nf_register_hook(struct net *net, struct smc_nf_hook *nfho);
+void smc_rv_nf_unregister_hook(struct net *net, struct smc_nf_hook *nfho);
+void smc_rv_init(void) __init;
+#endif
-- 
2.13.5

^ permalink raw reply related

* Re: [PATCH net-next v2 12/12] net: dsa: bcm_sf2: Utilize b53_{enable,disable}_port
From: Vivien Didelot @ 2017-09-19 14:45 UTC (permalink / raw)
  To: Florian Fainelli, netdev; +Cc: andrew, davem, Florian Fainelli
In-Reply-To: <20170919021947.8971-13-f.fainelli@gmail.com>

Florian Fainelli <f.fainelli@gmail.com> writes:

> Export b53_{enable,disable}_port and use these two functions in
> bcm_sf2_port_setup and bcm_sf2_port_disable. The generic functions
> cannot be used without wrapping because we need to manage additional
> switch integration details (PHY, Broadcom tag etc.).
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

^ permalink raw reply

* Re: [RFC net-next v2] bridge lwtunnel, VPLS & NVGRE
From: Amine Kherbouche @ 2017-09-19 14:46 UTC (permalink / raw)
  To: David Lamparter, netdev, bridge; +Cc: roopa, stephen
In-Reply-To: <6a538120-1941-7e81-c942-e97adeff2e3d@6wind.com>

Hi David,

What's next ? do you plan to send a v3 or should I do it ?

On 09/11/2017 10:02 AM, Amine Kherbouche wrote:
> Hi David,
>
> Do you plan to send a v3?
>
> On 21/08/2017 18:15, David Lamparter wrote:
>> Hi all,
>>
>>
>> this is an update on the earlier "[RFC net-next] VPLS support".  Note
>> I've changed the subject lines on some of the patches to better reflect
>> what they really do (tbh the earlier subject lines were crap.)
>>
>> As previously, iproute2 / FRR patches are at:
>> - https://github.com/eqvinox/vpls-iproute2
>> - https://github.com/opensourcerouting/frr/commits/vpls
>> while this patchset is also available at:
>> - https://github.com/eqvinox/vpls-linux-kernel
>> (but please be aware that I'm amending and rebasing commits)

^ permalink raw reply

* Re: [PATCH net-next v2 11/12] net: dsa: bcm_sf2: Use SF2_NUM_EGRESS_QUEUES for CFP
From: Vivien Didelot @ 2017-09-19 14:42 UTC (permalink / raw)
  To: Florian Fainelli, netdev; +Cc: andrew, davem, Florian Fainelli
In-Reply-To: <20170919021947.8971-12-f.fainelli@gmail.com>

Florian Fainelli <f.fainelli@gmail.com> writes:

> The magic number 8 in 3 locations in bcm_sf2_cfp.c actually designates the
> number of switch port egress queues, so use that define instead of open-coding
> it.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

^ permalink raw reply

* Re: [PATCH net-next v2 10/12] net: dsa: b53: Export b53_imp_vlan_setup()
From: Vivien Didelot @ 2017-09-19 14:41 UTC (permalink / raw)
  To: Florian Fainelli, netdev; +Cc: andrew, davem, Florian Fainelli
In-Reply-To: <20170919021947.8971-11-f.fainelli@gmail.com>

Florian Fainelli <f.fainelli@gmail.com> writes:

> bcm_sf2 and b53 do exactly the same thing, so share that piece.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

^ permalink raw reply

* Re: [PATCH RFC V1 net-next 0/6] Time based packet transmission
From: Miroslav Lichvar @ 2017-09-19 14:43 UTC (permalink / raw)
  To: Richard Cochran
  Cc: netdev, Andre Guedes, Henrik Austad, linux-kernel,
	Jesus Sanchez-Palencia, intel-wired-lan, John Stultz,
	Thomas Gleixner, Anna-Maria Gleixner, David Miller
In-Reply-To: <cover.1505719061.git.rcochran@linutronix.de>

On Mon, Sep 18, 2017 at 09:41:15AM +0200, Richard Cochran wrote:
> This series is an early RFC that introduces a new socket option
> allowing time based transmission of packets.  This option will be
> useful in implementing various real time protocols over Ethernet,
> including but not limited to P802.1Qbv, which is currently finding
> its way into 802.1Q.

If I understand it correctly, this also allows us to make a PTP/NTP
"one-step" clock with HW that doesn't support it directly.

> * Open questions about SO_TXTIME semantics
> 
>   - What should the kernel do if the dialed Tx time is in the past?
>     Should the packet be sent ASAP, or should we throw an error?

Dropping the packet with an error would make more sense to me.

>   - What should the timescale be for the dialed Tx time?  Should the
>     kernel select UTC when using the SW Qdisc and the HW time
>     otherwise?  Or should the socket option include a clockid_t?

I think for applications that don't (want to) bind their socket to a
specific interface it would be useful if the cmsg specified clockid_t
or maybe if_index. If the packet would be sent using a different
PHC/interface, it should be dropped.

>   |         | plain preempt_rt |     so_txtime | txtime @ 250 us |
>   |---------+------------------+---------------+-----------------|
>   | min:    |    +1.940800e+04 | +4.720000e+02 |   +4.720000e+02 |
>   | max:    |    +7.556000e+04 | +5.680000e+02 |   +5.760000e+02 |
>   | pk-pk:  |    +5.615200e+04 | +9.600000e+01 |   +1.040000e+02 |
>   | mean:   |    +3.292776e+04 | +5.072274e+02 |   +5.073602e+02 |
>   | stddev: |    +6.514709e+03 | +1.310849e+01 |   +1.507144e+01 |
>   | count:  |           600000 |        600000 |         2400000 |
> 
>   Using so_txtime, the peak to peak jitter is about 100 nanoseconds,

Nice!

-- 
Miroslav Lichvar

^ permalink raw reply

* Re: [PATCH net-next v2 09/12] net: dsa: b53: Wire-up EEE
From: Vivien Didelot @ 2017-09-19 14:37 UTC (permalink / raw)
  To: Florian Fainelli, netdev; +Cc: andrew, davem, Florian Fainelli
In-Reply-To: <20170919021947.8971-10-f.fainelli@gmail.com>

Florian Fainelli <f.fainelli@gmail.com> writes:

> Add support for enabling and disabling EEE, as well as re-negotiating it in
> .adjust_link() and in .port_enable().
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

^ permalink raw reply

* Re: [PATCH net-next v2 08/12] net: dsa: b53: Move EEE functions to b53
From: Vivien Didelot @ 2017-09-19 14:34 UTC (permalink / raw)
  To: Florian Fainelli, netdev; +Cc: andrew, davem, Florian Fainelli
In-Reply-To: <20170919021947.8971-9-f.fainelli@gmail.com>

Florian Fainelli <f.fainelli@gmail.com> writes:

> Move the bcm_sf2 EEE-related functions to the b53 driver because this is shared
> code amongst Gigabit capable switch, only 5325 and 5365 are too old to support
> that.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

^ permalink raw reply

* RE: [PATCH net-next 05/12] net: dsa: b53: Use a macro to define I/O operations
From: Florian Fainelli @ 2017-09-19 14:29 UTC (permalink / raw)
  To: Vivien Didelot, David Laight, netdev@vger.kernel.org
  Cc: davem@davemloft.net, andrew@lunn.ch
In-Reply-To: <87tvzy93ew.fsf@weeman.i-did-not-set--mail-host-address--so-tickle-me>

On September 19, 2017 7:19:35 AM PDT, Vivien Didelot <vivien.didelot@savoirfairelinux.com> wrote:
>Hi David,
>
>David Laight <David.Laight@ACULAB.COM> writes:
>
>> From: Florian Fainelli
>>> Sent: 18 September 2017 22:41
>>> Instead of repeating the same pattern: acquire mutex, read/write,
>release
>>> mutex, define a macro: b53_build_op() which takes the type
>(read|write), I/O
>>> size, and value (scalar or pointer). This helps with fixing bugs
>that could
>>> exit (e.g: missing barrier, lock etc.).
>> ....
>>> +#define b53_build_op(type, op_size, val_type)	\
>>> +static inline int b53_##type##op_size(struct b53_device *dev, u8
>page,		\
>>> +				      u8 reg, val_type val)			\
>>> +{										\
>>> +	int ret;								\
>>> +										\
>>> +	mutex_lock(&dev->reg_mutex);						\
>>> +	ret = dev->ops->type##op_size(dev, page, reg, val);			\
>>> +	mutex_unlock(&dev->reg_mutex);						\
>>> +										\
>>> +	return ret;								\
>>>  }
>>
>> Why separate the 'type' and 'op_size' arguments since they
>> are always pasted together?
>
>For read/write48, the value type is u64.

The way I read David's comment is that instead of calling the macro with read, 48, just combine that in a single argument: read48. I don't have a preference about that and can respin eventually.

-- 
Florian

^ permalink raw reply

* RE: [PATCH net-next 05/12] net: dsa: b53: Use a macro to define I/O operations
From: Vivien Didelot @ 2017-09-19 14:19 UTC (permalink / raw)
  To: David Laight, 'Florian Fainelli', netdev@vger.kernel.org
  Cc: davem@davemloft.net, andrew@lunn.ch
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DD007A003@AcuExch.aculab.com>

Hi David,

David Laight <David.Laight@ACULAB.COM> writes:

> From: Florian Fainelli
>> Sent: 18 September 2017 22:41
>> Instead of repeating the same pattern: acquire mutex, read/write, release
>> mutex, define a macro: b53_build_op() which takes the type (read|write), I/O
>> size, and value (scalar or pointer). This helps with fixing bugs that could
>> exit (e.g: missing barrier, lock etc.).
> ....
>> +#define b53_build_op(type, op_size, val_type)	\
>> +static inline int b53_##type##op_size(struct b53_device *dev, u8 page,		\
>> +				      u8 reg, val_type val)			\
>> +{										\
>> +	int ret;								\
>> +										\
>> +	mutex_lock(&dev->reg_mutex);						\
>> +	ret = dev->ops->type##op_size(dev, page, reg, val);			\
>> +	mutex_unlock(&dev->reg_mutex);						\
>> +										\
>> +	return ret;								\
>>  }
>
> Why separate the 'type' and 'op_size' arguments since they
> are always pasted together?

For read/write48, the value type is u64.


Thanks,

        Vivien

^ permalink raw reply

* RE: [PATCH net 0/7] Bug fixes for the HNS3 Ethernet Driver for Hip08 SoC
From: Salil Mehta @ 2017-09-19 14:12 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org,
	Zhuangyuzeng (Yisen), lipeng (Y),
	mehta.salil.lnk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linuxarm
In-Reply-To: <20170919035900.GI5788-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>

Hi Leon,

> -----Original Message-----
> From: Leon Romanovsky [mailto:leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> Sent: Tuesday, September 19, 2017 4:59 AM
> To: Salil Mehta
> Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org; Zhuangyuzeng (Yisen); lipeng (Y);
> mehta.salil.lnk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-
> kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Linuxarm
> Subject: Re: [PATCH net 0/7] Bug fixes for the HNS3 Ethernet Driver for
> Hip08 SoC
> 
> On Tue, Sep 19, 2017 at 02:06:21AM +0100, Salil Mehta wrote:
> > This patch set presents some bug fixes for the HNS3 Ethernet driver,
> identified
> > during internal testing & stabilization efforts.
> >
> > This patch series is meant for Linux 4.14 kernel.
> >
> > Lipeng (6):
> >   net: hns3: get phy addr from NCL_config
> >   net: hns3: fix the command used to unmap ring from vector
> >   net: hns3: Fix ring and vector map command
> >   net: hns3: fix a bug of set mac address
> >   net: hns3: set default vlan id to PF
> >   net: hns3: Fixes the premature exit of loop when matching clients
> >
> > Salil Mehta (1):
> >   net: hns3: fixes the ether address copy with more appropriate API
> 
> 1. The fixes patches should have Fixes line and not all of them have
> (I didn't look all patches).
> 2. Please decide on one style: fixes vs. Fixes, fix vs. Fix in the
> titles
> 3. Subject should be descriptive and usable, I don't know if it applies
> to the "fix a bug of set mac address" patch.
Yes, missed these. Will fix them. Thanks!

Salil
> 
> Thanks
> 
> >
> >  drivers/net/ethernet/hisilicon/hns3/hnae3.c        | 43 +++++-------
> ----------
> >  .../net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h |  8 +++-
> >  .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c    | 20 ++++++++--
> >  .../net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c |  7 ++--
> >  4 files changed, 35 insertions(+), 43 deletions(-)
> >
> > --
> > 2.11.0
> >
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* RE: [PATCH net-next 5/5] tls: Add generic NIC offload infrastructure.
From: Boris Pismenny @ 2017-09-19 14:02 UTC (permalink / raw)
  To: Hannes Frederic Sowa, Ilya Lesokhin
  Cc: netdev@vger.kernel.org, davem@davemloft.net, davejwatson@fb.com,
	tom@herbertland.com, Aviad Yehezkel, Liran Liss
In-Reply-To: <87wp4vt4aa.fsf@stressinduktion.org>

Hello,

Hannes Frederic Sowa <hannes@stressinduktion.org> writes:
> Hello,
> 
> Ilya Lesokhin <ilyal@mellanox.com> writes:
> 
> > Hannes Frederic Sowa <hannes@stressinduktion.org> writes:
> >
> >> The user should be aware of that they can't migrate the socket to
> >> another interface if they got hw offloaded. This is not the case for
> software offload.
> >> Thus I think the user has to opt in and it shouldn't be a heuristic
> >> until we can switch back to sw offload path.
> >>
> >> Maybe change flowi_oif to sk_bound_dev_if and somwhow lock it against
> >> further changes if hw tls is in use?
> >>
> >
> > I'm not sure I follow.
> > We do set sk->sk_bound_dev_if to prevent further changes.
> >
> > Do you recommend we enable TLS offload only if SO_BINDTODEVICE
> > was previously used on that socket?
> > and prevent even users with CAP_NET_RAW from unbinding it?
> >
> > I would rather avoid requiring CAP_NET_RAW to use TLS offload.
> > But admittedly I'm not sure setting sk->sk_bound_dev_if without
> > CAP_NET_RAW like we do is legit either.
> >
> > Finally, the reason we made HW offload the default is that the user
> > can use sudo ethtool -K enp0s4 tls-hw-tx-offload off to opt out of HW
> > offload and we currently don't have anything equivalent for opting out of
> SW KTLS.
> 
> IMHO the decision if a TCP flow should be bounded to hw and thus never
> push traffic to another interface should a decision the administrator and the
> application should opt in. You might have your management application
> which is accessible over multiple interfaces and your production application
> which might want to use hw offloaded tls. Thus I don't think only a single
> ethtool knob will do it.

IMO the configuration knob should be at the kTLS level and not at the
HW vs. SW level. The management application shouldn't be using kTLS.
I'd like to view TLS offload similarly to LSO. The default is opt-in if
possible, and the Kernel decides that based on device capabilities.

> 
> I agree that SO_BINDTODEVICE is bad for this use case. First, the
> CAP_NET_RAW limitation seems annoying and we don't want to enforce TLS
> apps to have this capability. Second, the user space application doesn't care
> which interface it should talk to (maybe?) but leave the routing decision to
> the kernel and just opt in to TLS. SO_BINDTODEVICE doesn't allow this.
> 
> sk_bound_dev_if can be rebound later with CAP_NET_RAW privileges, will
> this be a problem?

Yes it is a problem and we have some ideas for a software fallback that should
catch this. 

Is the software fallback a prerequisite for kTLS offload in Kernel?

> 
> Have you thought how the user space will configure the various offloading
> features (sw, hw, none)? Will it in e.g. OpenSSL be part of the Cipher Spec or
> will there be new functions around SSL_CTX to do so?
> 
> Maybe an enhancement of the TLS_TX setsockopt with a boolean for hw
> offload is a solution?

Yes, we think that OpenSSL should first configure whether it complies with
kTLS support. Next, we thought of using an environment variable to control
kTLS globally in OpenSSL as follows:
1. only software kTLS
2. only hardware kTLS - no fallback to software.
3. Try to use hardware kTLS and if it isn't supported fallback to software kTLS.

The above is something we plan for the future, assuming that kTLS wouldn't fit for
all use-cases. What do you think?

If you'd like to have more fine-grained control of kTLS, e.g. per socket,
then the application would need to be modified to configure that,
which is something we try to avoid.

> 
> Another question:
> 
> How is the dependency management done between socket layer and driver
> layer? It seems a bit cyclic but judging from this code you don't hold
> references to the device (dev_hold) (which is good, you don't want to have
> users creating refs to devices). OTOH you somehow need to match sockets
> from the device layer up to the socket. Will those be reference counted or
> does that work without?

Not sure I follow your question.
We use the socket from the device layer through the SKB that carries it,
so I think it should work without.
We don't attempt to perform a socket lookup or anything of this sort.

^ permalink raw reply

* Re: software interrupts close to 100 with 9000 tc filter entries
From: Eric Dumazet @ 2017-09-19 14:01 UTC (permalink / raw)
  To: Marco Berizzi; +Cc: netdev
In-Reply-To: <1627070802.486441.1505827738125@mail.libero.it>

On Tue, 2017-09-19 at 15:28 +0200, Marco Berizzi wrote:
> Hi Folks,
> 
> I'm running linux 4.12.10 x86_64 on a Slackware 14.2 64bit
> as a simple 4 NIC router. Network throughput processed by
> this machine is less than 200Mbit/s
> The cpu model is Intel(R) Xeon(R) CPU 5160  @ 3.00GHz with
> 2GB ram.
> 
> I need to blacklist about 9000 single ip addresses.
> This is the relevant script to blacklist these ip addresses:
> 
> tc qdisc add dev eth0 ingress
> tc qdisc add dev eth1 ingress
> 
> while read -r line
> do
>     tc filter add dev eth0 parent ffff: protocol ip prio 50 u32 match ip src $line action drop
>     tc filter add dev eth1 parent ffff: protocol ip prio 50 u32 match ip src $line action drop
> done < blacklisted_ip_addresses
> 
> After loading these ip addresses, the si (software interrupts)
> number shown by top is always close to 100
> If I delete the ingress qdisc on both the device, the si
> fall down to less than 5
> 
> Running the same script with 'only' 700 ip addresses is
> flawless.
> 
> Kindly I would like to ask if am I doing anything in
> a wrong way or if the hardware is too old for this kind
> of setup.
> 
> I have selected the tc filter setup instead of netfilter
> one, because I was reading this from iproute2/doc/actions:
> 
> A side effect is that we can now get stateless firewalling to work with tc..
> Essentially this is now an alternative to iptables.
> I wont go into details of my dislike for iptables at times, but.
> scalability is one of the main issues; however, if you need stateful
> classification - use netfilter (for now).
> 
> Any response are welcome
> TIA


Processing a list of 700 rules per incoming packet is not wise.

Alternatives :

- netfilter with IPSET : This probably can be done with one lookup in a
table. Probably easiest way to setup.

- BPF filter  (XDP or TC )

^ permalink raw reply

* [PATCH 3/3][v2] selftests: silence test output by default
From: josef @ 2017-09-19 13:51 UTC (permalink / raw)
  To: shuah, davem, netdev, linux-kselftest; +Cc: Josef Bacik
In-Reply-To: <1505829088-1823-1-git-send-email-jbacik@fb.com>

From: Josef Bacik <jbacik@fb.com>

Some of the networking tests are very noisy and make it impossible to
see if we actually passed the tests as they run.  Default to suppressing
the output from any tests run in order to make it easier to track what
failed.

Signed-off-by: Josef Bacik <jbacik@fb.com>
---
v1->v2:
- dump output into /tmp/testname instead of /dev/null

 tools/testing/selftests/lib.mk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index 693616651da5..4665463779f5 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -24,7 +24,7 @@ define RUN_TESTS
 			echo "selftests: Warning: file $$BASENAME_TEST is not executable, correct this.";\
 			echo "not ok 1..$$test_num selftests: $$BASENAME_TEST [FAIL]"; \
 		else					\
-			cd `dirname $$TEST` > /dev/null; (./$$BASENAME_TEST && echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") || echo "not ok 1..$$test_num selftests:  $$BASENAME_TEST [FAIL]"; cd - > /dev/null;\
+			cd `dirname $$TEST` > /dev/null; (./$$BASENAME_TEST > /tmp/$$BASENAME_TEST 2>&1 && echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") || echo "not ok 1..$$test_num selftests:  $$BASENAME_TEST [FAIL]"; cd - > /dev/null;\
 		fi;					\
 	done;
 endef
@@ -55,7 +55,7 @@ endif
 define EMIT_TESTS
 	@for TEST in $(TEST_GEN_PROGS) $(TEST_PROGS); do \
 		BASENAME_TEST=`basename $$TEST`;	\
-		echo "(./$$BASENAME_TEST && echo \"selftests: $$BASENAME_TEST [PASS]\") || echo \"selftests: $$BASENAME_TEST [FAIL]\""; \
+		echo "(./$$BASENAME_TEST > /tmp/$$BASENAME_TEST 2>&1 && echo \"selftests: $$BASENAME_TEST [PASS]\") || echo \"selftests: $$BASENAME_TEST [FAIL]\""; \
 	done;
 endef
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH 2/3][v2] selftests: actually run the various net selftests
From: josef @ 2017-09-19 13:51 UTC (permalink / raw)
  To: shuah, davem, netdev, linux-kselftest; +Cc: Josef Bacik
In-Reply-To: <1505829088-1823-1-git-send-email-jbacik@fb.com>

From: Josef Bacik <jbacik@fb.com>

These self tests are just self contained binaries, they are not run by
any of the scripts in the directory.  This means they need to be marked
with TEST_GEN_PROGS to actually be run, not TEST_GEN_FILES.

Signed-off-by: Josef Bacik <jbacik@fb.com>
---
v1->v2:
- Moved msg_zerocopy to TEST_GEN_FILES since it's not runnable in it's current
  state

 tools/testing/selftests/net/Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 3df542c84610..d86bca991f45 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -5,9 +5,9 @@ CFLAGS += -I../../../../usr/include/
 
 TEST_PROGS := run_netsocktests run_afpackettests test_bpf.sh netdevice.sh rtnetlink.sh
 TEST_GEN_FILES =  socket
-TEST_GEN_FILES += psock_fanout psock_tpacket
-TEST_GEN_FILES += reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
-TEST_GEN_FILES += reuseport_dualstack msg_zerocopy reuseaddr_conflict
+TEST_GEN_FILES += psock_fanout psock_tpacket msg_zerocopy
+TEST_GEN_PROGS = reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
+TEST_GEN_PROGS += reuseport_dualstack reuseaddr_conflict
 
 include ../lib.mk
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH 1/3][v2] selftest: add a reuseaddr test
From: josef @ 2017-09-19 13:51 UTC (permalink / raw)
  To: shuah, davem, netdev, linux-kselftest; +Cc: Josef Bacik

From: Josef Bacik <jbacik@fb.com>

This is to test for a regression introduced by

b9470c27607b ("inet: kill smallest_size and smallest_port")

which introduced a problem with reuseaddr and bind conflicts.

Signed-off-by: Josef Bacik <jbacik@fb.com>
---
 tools/testing/selftests/net/.gitignore           |   1 +
 tools/testing/selftests/net/Makefile             |   2 +-
 tools/testing/selftests/net/reuseaddr_conflict.c | 114 +++++++++++++++++++++++
 3 files changed, 116 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/net/reuseaddr_conflict.c

diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore
index 9801253e4802..c612d6e38c62 100644
--- a/tools/testing/selftests/net/.gitignore
+++ b/tools/testing/selftests/net/.gitignore
@@ -6,3 +6,4 @@ reuseport_bpf
 reuseport_bpf_cpu
 reuseport_bpf_numa
 reuseport_dualstack
+reuseaddr_conflict
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index de1f5772b878..3df542c84610 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -7,7 +7,7 @@ TEST_PROGS := run_netsocktests run_afpackettests test_bpf.sh netdevice.sh rtnetl
 TEST_GEN_FILES =  socket
 TEST_GEN_FILES += psock_fanout psock_tpacket
 TEST_GEN_FILES += reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
-TEST_GEN_FILES += reuseport_dualstack msg_zerocopy
+TEST_GEN_FILES += reuseport_dualstack msg_zerocopy reuseaddr_conflict
 
 include ../lib.mk
 
diff --git a/tools/testing/selftests/net/reuseaddr_conflict.c b/tools/testing/selftests/net/reuseaddr_conflict.c
new file mode 100644
index 000000000000..7c5b12664b03
--- /dev/null
+++ b/tools/testing/selftests/net/reuseaddr_conflict.c
@@ -0,0 +1,114 @@
+/*
+ * Test for the regression introduced by
+ *
+ * b9470c27607b ("inet: kill smallest_size and smallest_port")
+ *
+ * If we open an ipv4 socket on a port with reuseaddr we shouldn't reset the tb
+ * when we open the ipv6 conterpart, which is what was happening previously.
+ */
+#include <errno.h>
+#include <error.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/socket.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#define PORT 9999
+
+int open_port(int ipv6, int any)
+{
+	int fd = -1;
+	int reuseaddr = 1;
+	int v6only = 1;
+	int addrlen;
+	int ret = -1;
+	struct sockaddr *addr;
+	int family = ipv6 ? AF_INET6 : AF_INET;
+
+	struct sockaddr_in6 addr6 = {
+		.sin6_family = AF_INET6,
+		.sin6_port = htons(PORT),
+		.sin6_addr = in6addr_any
+	};
+	struct sockaddr_in addr4 = {
+		.sin_family = AF_INET,
+		.sin_port = htons(PORT),
+		.sin_addr.s_addr = any ? htonl(INADDR_ANY) : inet_addr("127.0.0.1"),
+	};
+
+
+	if (ipv6) {
+		addr = (struct sockaddr*)&addr6;
+		addrlen = sizeof(addr6);
+	} else {
+		addr = (struct sockaddr*)&addr4;
+		addrlen = sizeof(addr4);
+	}
+
+	if ((fd = socket(family, SOCK_STREAM, IPPROTO_TCP)) < 0) {
+		perror("socket");
+		goto out;
+	}
+
+	if (ipv6 && setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, (void*)&v6only,
+			       sizeof(v6only)) < 0) {
+		perror("setsockopt IPV6_V6ONLY");
+		goto out;
+	}
+
+	if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &reuseaddr,
+		       sizeof(reuseaddr)) < 0) {
+		perror("setsockopt SO_REUSEADDR");
+		goto out;
+	}
+
+	if (bind(fd, addr, addrlen) < 0) {
+		perror("bind");
+		goto out;
+	}
+
+	if (any)
+		return fd;
+
+	if (listen(fd, 1) < 0) {
+		perror("listen");
+		goto out;
+	}
+	return fd;
+out:
+	close(fd);
+	return ret;
+}
+
+int main(void)
+{
+	int listenfd;
+	int fd1, fd2;
+
+	fprintf(stderr, "Opening 127.0.0.1:%d\n", PORT);
+	listenfd = open_port(0, 0);
+	if (listenfd < 0)
+		error(1, errno, "Couldn't open listen socket");
+	fprintf(stderr, "Opening INADDR_ANY:%d\n", PORT);
+	fd1 = open_port(0, 1);
+	if (fd1 >= 0)
+		error(1, 0, "Was allowed to create an ipv4 reuseport on a already bound non-reuseport socket");
+	fprintf(stderr, "Opening in6addr_any:%d\n", PORT);
+	fd1 = open_port(1, 1);
+	if (fd1 < 0)
+		error(1, errno, "Couldn't open ipv6 reuseport");
+	fprintf(stderr, "Opening INADDR_ANY:%d\n", PORT);
+	fd2 = open_port(0, 1);
+	if (fd2 >= 0)
+		error(1, 0, "Was allowed to create an ipv4 reuseport on a already bound non-reuseport socket");
+	close(fd1);
+	fprintf(stderr, "Opening INADDR_ANY:%d after closing ipv6 socket\n", PORT);
+	fd1 = open_port(0, 1);
+	if (fd1 >= 0)
+		error(1, 0, "Was allowed to create an ipv4 reuseport on an already bound non-reuseport socket with no ipv6");
+	fprintf(stderr, "Success");
+	return 0;
+}
-- 
2.7.4

^ permalink raw reply related

* Re: [regression v4.11] 617f01211baf ("8139too: use napi_complete_done()")
From: Eric Dumazet @ 2017-09-19 13:51 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Eric Dumazet, David S. Miller, netdev, LKML
In-Reply-To: <20170919124503.GO4914@intel.com>

On Tue, 2017-09-19 at 15:45 +0300, Ville Syrjälä wrote:
> On Mon, Sep 18, 2017 at 12:52:15PM -0700, Eric Dumazet wrote:
> > On Mon, Sep 18, 2017 at 12:46 PM, Ville Syrjälä
> > <ville.syrjala@linux.intel.com> wrote:
> > 
> > > And five months later I'm still waiting for this patch to land...
> > 
> > Sure, while I was on vacation, I received thousands of emails that I
> > simply have not read.
> 
> Ah that explains where my previous ping went. Pardon the snarky comment,
> and thanks for taking care of this.
> 

No problem, this was my fault not following this bug.

Thanks a lot !

^ permalink raw reply

* [net-next,RESEND] igb: add function to get maximum RSS queues
From: Zhang Shengju @ 2017-09-19 13:40 UTC (permalink / raw)
  To: jeffrey.t.kirsher, intel-wired-lan, netdev

This patch adds a new function igb_get_max_rss_queues() to get maximum
RSS queues, this will reduce duplicate code and facilitate future
maintenance.

Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
---
 drivers/net/ethernet/intel/igb/igb.h         |  1 +
 drivers/net/ethernet/intel/igb/igb_ethtool.c | 32 +---------------------------
 drivers/net/ethernet/intel/igb/igb_main.c    | 12 +++++++++--
 3 files changed, 12 insertions(+), 33 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 06ffb2b..ca94eff 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -684,6 +684,7 @@ void igb_ptp_rx_pktstamp(struct igb_q_vector *q_vector, void *va,
 int igb_ptp_set_ts_config(struct net_device *netdev, struct ifreq *ifr);
 int igb_ptp_get_ts_config(struct net_device *netdev, struct ifreq *ifr);
 void igb_set_flag_queue_pairs(struct igb_adapter *, const u32);
+unsigned int igb_get_max_rss_queues(struct igb_adapter *);
 #ifdef CONFIG_IGB_HWMON
 void igb_sysfs_exit(struct igb_adapter *adapter);
 int igb_sysfs_init(struct igb_adapter *adapter);
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index d06a8db..606e676 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -3338,37 +3338,7 @@ static int igb_set_rxfh(struct net_device *netdev, const u32 *indir,
 
 static unsigned int igb_max_channels(struct igb_adapter *adapter)
 {
-	struct e1000_hw *hw = &adapter->hw;
-	unsigned int max_combined = 0;
-
-	switch (hw->mac.type) {
-	case e1000_i211:
-		max_combined = IGB_MAX_RX_QUEUES_I211;
-		break;
-	case e1000_82575:
-	case e1000_i210:
-		max_combined = IGB_MAX_RX_QUEUES_82575;
-		break;
-	case e1000_i350:
-		if (!!adapter->vfs_allocated_count) {
-			max_combined = 1;
-			break;
-		}
-		/* fall through */
-	case e1000_82576:
-		if (!!adapter->vfs_allocated_count) {
-			max_combined = 2;
-			break;
-		}
-		/* fall through */
-	case e1000_82580:
-	case e1000_i354:
-	default:
-		max_combined = IGB_MAX_RX_QUEUES;
-		break;
-	}
-
-	return max_combined;
+	return igb_get_max_rss_queues(adapter);
 }
 
 static void igb_get_channels(struct net_device *netdev,
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index ec62410..e6cc6b5 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3026,10 +3026,10 @@ static void igb_probe_vfs(struct igb_adapter *adapter)
 #endif /* CONFIG_PCI_IOV */
 }
 
-static void igb_init_queue_configuration(struct igb_adapter *adapter)
+unsigned int igb_get_max_rss_queues(struct igb_adapter *adapter)
 {
 	struct e1000_hw *hw = &adapter->hw;
-	u32 max_rss_queues;
+	unsigned int max_rss_queues;
 
 	/* Determine the maximum number of RSS queues supported. */
 	switch (hw->mac.type) {
@@ -3060,6 +3060,14 @@ static void igb_init_queue_configuration(struct igb_adapter *adapter)
 		break;
 	}
 
+	return max_rss_queues;
+}
+
+static void igb_init_queue_configuration(struct igb_adapter *adapter)
+{
+	u32 max_rss_queues;
+
+	max_rss_queues = igb_get_max_rss_queues(adapter);
 	adapter->rss_queues = min_t(u32, max_rss_queues, num_online_cpus());
 
 	igb_set_flag_queue_pairs(adapter, max_rss_queues);
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH net v2] l2tp: fix race condition in l2tp_tunnel_delete
From: Sabrina Dubroca @ 2017-09-19 13:40 UTC (permalink / raw)
  To: netdev; +Cc: Sabrina Dubroca, Guillaume Nault, Xin Long, Tom Parkin

If we try to delete the same tunnel twice, the first delete operation
does a lookup (l2tp_tunnel_get), finds the tunnel, calls
l2tp_tunnel_delete, which queues it for deletion by
l2tp_tunnel_del_work.

The second delete operation also finds the tunnel and calls
l2tp_tunnel_delete. If the workqueue has already fired and started
running l2tp_tunnel_del_work, then l2tp_tunnel_delete will queue the
same tunnel a second time, and try to free the socket again.

Add a dead flag to prevent firing the workqueue twice. Then we can
remove the check of queue_work's result that was meant to prevent that
race but doesn't.

Also check the flag in the tunnel lookup functions, to avoid returning a
tunnel that is already scheduled for destruction.

Reproducer:

    ip l2tp add tunnel tunnel_id 3000 peer_tunnel_id 4000 local 192.168.0.2 remote 192.168.0.1 encap udp udp_sport 5000 udp_dport 6000
    ip l2tp add session name l2tp1 tunnel_id 3000 session_id 1000 peer_session_id 2000
    ip link set l2tp1 up
    ip l2tp del tunnel tunnel_id 3000
    ip l2tp del tunnel tunnel_id 3000

Fixes: f8ccac0e4493 ("l2tp: put tunnel socket release on a workqueue")
Reported-by: Jianlin Shi <jishi@redhat.com>
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
v2: as Tom Parkin explained, we can't remove the tunnel from the
    per-net list from netlink. v2 uses only a dead flag, and adds
    corresponding checks during lookups

 net/l2tp/l2tp_core.c | 18 +++++++++---------
 net/l2tp/l2tp_core.h |  5 ++++-
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index ee485df73ccd..3891f0260f2b 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -203,7 +203,8 @@ struct l2tp_tunnel *l2tp_tunnel_get(const struct net *net, u32 tunnel_id)
 
 	rcu_read_lock_bh();
 	list_for_each_entry_rcu(tunnel, &pn->l2tp_tunnel_list, list) {
-		if (tunnel->tunnel_id == tunnel_id) {
+		if (tunnel->tunnel_id == tunnel_id &&
+		    !test_bit(0, &tunnel->dead)) {
 			l2tp_tunnel_inc_refcount(tunnel);
 			rcu_read_unlock_bh();
 
@@ -390,7 +391,8 @@ struct l2tp_tunnel *l2tp_tunnel_find(const struct net *net, u32 tunnel_id)
 
 	rcu_read_lock_bh();
 	list_for_each_entry_rcu(tunnel, &pn->l2tp_tunnel_list, list) {
-		if (tunnel->tunnel_id == tunnel_id) {
+		if (tunnel->tunnel_id == tunnel_id &&
+		    !test_bit(0, &tunnel->dead)) {
 			rcu_read_unlock_bh();
 			return tunnel;
 		}
@@ -409,7 +411,7 @@ struct l2tp_tunnel *l2tp_tunnel_find_nth(const struct net *net, int nth)
 
 	rcu_read_lock_bh();
 	list_for_each_entry_rcu(tunnel, &pn->l2tp_tunnel_list, list) {
-		if (++count > nth) {
+		if (++count > nth && !test_bit(0, &tunnel->dead)) {
 			rcu_read_unlock_bh();
 			return tunnel;
 		}
@@ -1685,14 +1687,12 @@ EXPORT_SYMBOL_GPL(l2tp_tunnel_create);
 
 /* This function is used by the netlink TUNNEL_DELETE command.
  */
-int l2tp_tunnel_delete(struct l2tp_tunnel *tunnel)
+void l2tp_tunnel_delete(struct l2tp_tunnel *tunnel)
 {
-	l2tp_tunnel_inc_refcount(tunnel);
-	if (false == queue_work(l2tp_wq, &tunnel->del_work)) {
-		l2tp_tunnel_dec_refcount(tunnel);
-		return 1;
+	if (!test_and_set_bit(0, &tunnel->dead)) {
+		l2tp_tunnel_inc_refcount(tunnel);
+		queue_work(l2tp_wq, &tunnel->del_work);
 	}
-	return 0;
 }
 EXPORT_SYMBOL_GPL(l2tp_tunnel_delete);
 
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index a305e0c5925a..deda869504d0 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -160,6 +160,9 @@ struct l2tp_tunnel_cfg {
 
 struct l2tp_tunnel {
 	int			magic;		/* Should be L2TP_TUNNEL_MAGIC */
+
+	unsigned long		dead;
+
 	struct rcu_head rcu;
 	rwlock_t		hlist_lock;	/* protect session_hlist */
 	bool			acpt_newsess;	/* Indicates whether this
@@ -254,7 +257,7 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id,
 		       u32 peer_tunnel_id, struct l2tp_tunnel_cfg *cfg,
 		       struct l2tp_tunnel **tunnelp);
 void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel);
-int l2tp_tunnel_delete(struct l2tp_tunnel *tunnel);
+void l2tp_tunnel_delete(struct l2tp_tunnel *tunnel);
 struct l2tp_session *l2tp_session_create(int priv_size,
 					 struct l2tp_tunnel *tunnel,
 					 u32 session_id, u32 peer_session_id,
-- 
2.14.1

^ permalink raw reply related

* Re: [PATCH 2/3] selftests: actually run the various net selftests
From: Josef Bacik @ 2017-09-19 13:34 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: josef, Josef Bacik, David S. Miller, linux-kernel,
	linux-kselftest, netdev, Shuah Khan, Shuah Khan
In-Reply-To: <09f17145-5f0a-ec61-3dfb-69216eac7133@kernel.org>

On Mon, Sep 18, 2017 at 04:14:41PM -0600, Shuah Khan wrote:
> On 09/18/2017 11:32 AM, josef@toxicpanda.com wrote:
> > From: Josef Bacik <jbacik@fb.com>
> > 
> > These self tests are just self contained binaries, they are not run by
> > any of the scripts in the directory.  This means they need to be marked
> > with TEST_GEN_PROGS to actually be run, not TEST_GEN_FILES.
> > 
> > Signed-off-by: Josef Bacik <jbacik@fb.com>
> > ---
> >  tools/testing/selftests/net/Makefile | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
> > index 3df542c84610..45a4e77a47c4 100644
> > --- a/tools/testing/selftests/net/Makefile
> > +++ b/tools/testing/selftests/net/Makefile
> > @@ -6,8 +6,8 @@ CFLAGS += -I../../../../usr/include/
> >  TEST_PROGS := run_netsocktests run_afpackettests test_bpf.sh netdevice.sh rtnetlink.sh
> >  TEST_GEN_FILES =  socket
> >  TEST_GEN_FILES += psock_fanout psock_tpacket
> > -TEST_GEN_FILES += reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
> > -TEST_GEN_FILES += reuseport_dualstack msg_zerocopy reuseaddr_conflict
> > +TEST_GEN_PROGS += reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
> > +TEST_GEN_PROGS += reuseport_dualstack msg_zerocopy reuseaddr_conflict
> 
> Hmm. I see msg_zerocopy.sh for running msg_zerocopy. msg_zerocopy should
> still stay in TEST_GEN_FILES and msg_zerocopy.sh needs to be added to
> TEST_PROGS so it runs.
> 

Actually the shell script requires arguments, it doesn't just run the test.
I'll fix this to just omit the test for now as it's not setup to run properly.

Willem, could you follow up with a patch so that the zero copy test is run
properly the way you envision it running?  You need to make sure that

make -C tools/testing/selftests TARGETS=net run_tests

actually runs your zero copy test the way you expect it to, otherwise it's just
sitting there collecting dust.  Thanks,

Josef

^ permalink raw reply

* software interrupts close to 100 with 9000 tc filter entries
From: Marco Berizzi @ 2017-09-19 13:28 UTC (permalink / raw)
  To: netdev

Hi Folks,

I'm running linux 4.12.10 x86_64 on a Slackware 14.2 64bit
as a simple 4 NIC router. Network throughput processed by
this machine is less than 200Mbit/s
The cpu model is Intel(R) Xeon(R) CPU 5160  @ 3.00GHz with
2GB ram.

I need to blacklist about 9000 single ip addresses.
This is the relevant script to blacklist these ip addresses:

tc qdisc add dev eth0 ingress
tc qdisc add dev eth1 ingress

while read -r line
do
    tc filter add dev eth0 parent ffff: protocol ip prio 50 u32 match ip src $line action drop
    tc filter add dev eth1 parent ffff: protocol ip prio 50 u32 match ip src $line action drop
done < blacklisted_ip_addresses

After loading these ip addresses, the si (software interrupts)
number shown by top is always close to 100
If I delete the ingress qdisc on both the device, the si
fall down to less than 5

Running the same script with 'only' 700 ip addresses is
flawless.

Kindly I would like to ask if am I doing anything in
a wrong way or if the hardware is too old for this kind
of setup.

I have selected the tc filter setup instead of netfilter
one, because I was reading this from iproute2/doc/actions:

A side effect is that we can now get stateless firewalling to work with tc..
Essentially this is now an alternative to iptables.
I wont go into details of my dislike for iptables at times, but.
scalability is one of the main issues; however, if you need stateful
classification - use netfilter (for now).

Any response are welcome
TIA

Marco

^ permalink raw reply

* Re: [RFC net-next 0/5] TSN: Add qdisc-based config interfaces for traffic shapers
From: Henrik Austad @ 2017-09-19 13:14 UTC (permalink / raw)
  To: Richard Cochran
  Cc: Vinicius Costa Gomes, netdev, jhs, xiyou.wangcong, jiri,
	intel-wired-lan, andre.guedes, ivan.briano,
	jesus.sanchez-palencia, boon.leong.ong
In-Reply-To: <20170919052244.77umdxuze53t6j22@localhost>

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

Hi all,

On Tue, Sep 19, 2017 at 07:22:44AM +0200, Richard Cochran wrote:
> On Mon, Sep 18, 2017 at 04:06:28PM -0700, Vinicius Costa Gomes wrote:
> > That's the point, the application does not need to know that, and asking
> > that would be stupid.
> 
> On the contrary, this information is essential to the application.
> Probably you have never seen an actual Ethernet field bus in
> operation?  In any case, you are missing the point.
> 
> > (And that's another nice point of how 802.1Qbv works, applications do
> > not need to be changed to use it, and I think we should work to achieve
> > this on the Linux side)
> 
> Once you start to care about real time performance, then you need to
> consider the applications.  This is industrial control, not streaming
> your tunes from your ipod.

Do not underestimate the need for media over TSN. I fully see your point of 
real-time systems, but they are not the only valid use-cases for TSN.

> > That being said, that only works for kinds of traffic that maps well to
> > this configuration in advance model, which is the model that the IEEE
> > (see 802.1Qcc) and the AVNU Alliance[1] are pushing for.
> 
> Again, you are missing the point of what they aiming for.  I have
> looked at a number of production systems, and in each case the
> developers want total control over the transmission, in order to
> reduce latency to an absolute minimum.  Typically the data to be sent
> are available only microseconds before the transmission deadline.
> 
> Consider OpenAVB on github that people are already using.  Take a look
> at simple_talker.c and explain how "applications do not need to be
> changed to use it."

I do not think simple-talker was everintended to be how users of AVB should 
be implemented, but as a demonstration of what the protocol could do.

ALSA/V4L2 should supply some interface to this so that you can attach 
media-applications to it without the application itself having to be "TSN 
aware".

> > [1]
> > http://avnu.org/theory-of-operation-for-tsn-enabled-industrial-systems/
> 
> Did you even read this?
> 
>     [page 24]
> 
>     As described in section 2, some industrial control systems require
>     predictable, very low latency and cycle-to-cycle variation to meet
>     hard real-time application requirements. In these systems,
>     multiple distributed controllers commonly synchronize their
>     sensor/actuator operations with other controllers by scheduling
>     these operations in time, typically using a repeating control
>     cycle.
>     ...
>     The gate control mechanism is itself a time-aware PTP application
>     operating within a bridge or end station port.
> 
> It is an application, not a "god box."
>
> > In short, I see a per-packet transmission time and a per-queue schedule
> > as solutions to different problems.
> 
> Well, I can agree with that.  For some non real-time applications,
> bandwidth shaping is enough, and your Qdisc idea is sufficient.  For
> the really challenging TSN targets (industrial control, automotive),
> your idea of an opaque schedule file won't fly.

Would it make sense to adapt the proposed Qdisc here as well as the 
back-o-the-napkin idea in the other thread to to a per-socket queue for 
each priority and then sort those sockets based on SO_TXTIME?

TSN operates on a per-StreamID basis, and that should map fairly well to a 
per-socket approach I think (let us just assume that an application that 
sends TSN traffic will open up a separate socket for each stream.

This should allow a userspace application that is _very_ aware of its 
timing constraints to send frames exactly when it needs to as you have 
SO_TXTIME available. It would also let applications that basically want a 
fine-grained rate control (audio and video comes to mind) to use the same 
qdisc.

For those sockets that do not support SO_TXTIME, but still map to a 
priority handled by sch_cbs (or whatever it'll end up being called) you can 
set the transmit-time to be the time of the last skb in the queue + an 
delta which will give you the correct rate (TSN operates on observation 
intervals which you can specify via tc when you create the queues).

Then you can have, as you propose in your other series, a hrtimer that is 
being called when the next SO_TXTIME enters, grab a skb and move it to the 
hw-queue. This should also allow you to keep a sorted per-socket queue 
should an application send frames in the wrong order, without having to 
rearrange descriptors for the DMA machinery.

If this makes sense, I am more than happy to give it a stab and see how it 
goes.

-Henrik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply


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