Netdev List
 help / color / mirror / Atom feed
* [PATCH v2] ipv4: make DSCP values works with ip rules
From: Pavel Balaev @ 2018-11-20 13:29 UTC (permalink / raw)
  To: davem; +Cc: netdev

This patch adds ability to set DSCP values in ip rules.                                                                                          
Values presented in /etc/iproute3/rt_dsfield and now can be used in rules.

Example:
$ ip ru add from 10.88.0.2 tos AF23 lookup dscp.
Result:
---
32762:	from 10.88.0.2 tos AF43 lookup dscp
---

Patch was tested with such configuration:

+-----------+
| 10.88.0.2 | -> ping 172.16.0.1 -Q 0x58 ->
|   host0   |
+-----------+

   +-------------------------------------+
   | router with patched kernel          |
   |                                     |
-> | from 10.88.0.2 tos AF43 lookup dscp |->
   | table dscp:                         |
   |   172.16.0.0/24 via 10.200.0.2      |
   +-------------------------------------+

	 +------------------+
-> | eth0: 10.200.0.2 |
   | eth1: 172.16.0.1 |
   |      host1       |
   +------------------+

Signed-off-by: Pavel Balaev <mail@void.so>
---
 include/net/route.h     | 2 +-
 include/uapi/linux/ip.h | 2 ++
 net/ipv4/fib_rules.c    | 2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/net/route.h b/include/net/route.h
index bb53cdba..b984ecff 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -239,7 +239,7 @@ static inline void ip_rt_put(struct rtable *rt)
 	dst_release(&rt->dst);
 }
 
-#define IPTOS_RT_MASK	(IPTOS_TOS_MASK & ~3)
+#define IPTOS_RT_MASK	(IPTOS_DSCP_MASK & ~3)
 
 extern const __u8 ip_tos2prio[16];
 
diff --git a/include/uapi/linux/ip.h b/include/uapi/linux/ip.h
index e42d13b5..307ce2b1 100644
--- a/include/uapi/linux/ip.h
+++ b/include/uapi/linux/ip.h
@@ -38,6 +38,8 @@
 #define IPTOS_PREC_PRIORITY             0x20
 #define IPTOS_PREC_ROUTINE              0x00
 
+#define IPTOS_DSCP_MASK		(IPTOS_TOS_MASK | IPTOS_PREC_MASK)
+#define IPTOS_DSCP(tos)		((tos)&IPTOS_DSCP_MASK)
 
 /* IP options */
 #define IPOPT_COPY		0x80
diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c
index f8eb78d0..9ba91ef0 100644
--- a/net/ipv4/fib_rules.c
+++ b/net/ipv4/fib_rules.c
@@ -220,7 +220,7 @@ static int fib4_rule_configure(struct fib_rule *rule, struct sk_buff *skb,
 	int err = -EINVAL;
 	struct fib4_rule *rule4 = (struct fib4_rule *) rule;
 
-	if (frh->tos & ~IPTOS_TOS_MASK) {
+	if (frh->tos & ~IPTOS_DSCP_MASK) {
 		NL_SET_ERR_MSG(extack, "Invalid tos");
 		goto errout;
 	}
-- 
2.18.1

^ permalink raw reply related

* Re: [PATCH net-next] e100: Fix passing zero to 'PTR_ERR' warning in e100_load_ucode_wait
From: Jeff Kirsher @ 2018-11-21  0:03 UTC (permalink / raw)
  To: YueHaibing, davem; +Cc: linux-kernel, netdev, intel-wired-lan
In-Reply-To: <20181119124819.15860-1-yuehaibing@huawei.com>

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

On Mon, 2018-11-19 at 20:48 +0800, YueHaibing wrote:
> Fix a static code checker warning:
> drivers/net/ethernet/intel/e100.c:1349
>  e100_load_ucode_wait() warn: passing zero to 'PTR_ERR'
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
>  drivers/net/ethernet/intel/e100.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

This looks fine, I am just concerned of the regression testing needed on
hardware that is almost 20 years old now.  The availability of the hardware
for testing is also a concern.

> diff --git a/drivers/net/ethernet/intel/e100.c
> b/drivers/net/ethernet/intel/e100.c
> index 7c4b554..736115b 100644
> --- a/drivers/net/ethernet/intel/e100.c
> +++ b/drivers/net/ethernet/intel/e100.c
> @@ -1345,8 +1345,8 @@ static inline int e100_load_ucode_wait(struct nic
> *nic)
>  
>  	fw = e100_request_firmware(nic);
>  	/* If it's NULL, then no ucode is required */
> -	if (!fw || IS_ERR(fw))
> -		return PTR_ERR(fw);
> +	if (IS_ERR_OR_NULL(fw))
> +		return PTR_ERR_OR_ZERO(fw);
>  
>  	if ((err = e100_exec_cb(nic, (void *)fw, e100_setup_ucode)))
>  		netif_err(nic, probe, nic->netdev,


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [RFC PATCH] net: don't keep lonely packets forever in the gro hash
From: Eric Dumazet @ 2018-11-20 13:49 UTC (permalink / raw)
  To: Paolo Abeni, netdev; +Cc: David S. Miller, Willem de Bruijn, Eric Dumazet
In-Reply-To: <3c8b5aea0c812323d8e15b548789a1e240f499d7.1542709015.git.pabeni@redhat.com>



On 11/20/2018 02:17 AM, Paolo Abeni wrote:
> Eric noted that with UDP GRO and napi timeout, we could keep a single
> UDP packet inside the GRO hash forever, if the related NAPI instance
> calls napi_gro_complete() at an higher frequency than the napi timeout.
> Willem noted that even TCP packets could be trapped there, till the
> next retransmission.
> This patch tries to address the issue, flushing the oldest packets before
> scheduling the NAPI timeout. The rationale is that such a timeout should be
> well below a jiffy and we are not flushing packets eligible for sane GRO.
> 
> Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> Sending as RFC, as I fear I'm missing some relevant pieces.
> Also I'm unsure if this should considered a fixes for "udp: implement
> GRO for plain UDP sockets." or for "net: gro: add a per device gro flush timer"

You can add both, now worries.

Google DC TCP forces a PSH flag on all TSO packets, so for us the flush is done
because of the PSH flag, not upon a timer/jiffie.

Truth be told, relying on jiffies change is a bit fragile for HZ=100 or HZ=250 kernels.

See recent TCP commit that got rid of tcp_tso_should_defer() dependency on HZ/jiffies

https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/?id=a682850a114aef947da5d603f7fd2cfe7eabbd72


> ---
>  net/core/dev.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 5927f6a7c301..5cc4c4961869 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5975,11 +5975,14 @@ bool napi_complete_done(struct napi_struct *n, int work_done)
>  		if (work_done)
>  			timeout = n->dev->gro_flush_timeout;
>  
> +		/* When the NAPI instance uses a timeout, we still need to
> +		 * someout bound the time packets are keept in the GRO layer
> +		 * under heavy traffic
> +		 */
> +		napi_gro_flush(n, !!timeout);
>  		if (timeout)
>  			hrtimer_start(&n->timer, ns_to_ktime(timeout),
>  				      HRTIMER_MODE_REL_PINNED);
> -		else
> -			napi_gro_flush(n, false);
>  	}
>  	if (unlikely(!list_empty(&n->poll_list))) {
>  		/* If n->poll_list is not empty, we need to mask irqs */
> 

^ permalink raw reply

* [PATCH net] tcp: defer SACK compression after DupThresh
From: Eric Dumazet @ 2018-11-20 13:53 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Jean-Louis Dupond, Neal Cardwell, Yuchung Cheng,
	Eric Dumazet, Eric Dumazet

Jean-Louis reported a TCP regression and bisected to recent SACK
compression.

After a loss episode (receiver not able to keep up and dropping
packets because its backlog is full), linux TCP stack is sending
a single SACK (DUPACK).

Sender waits a full RTO timer before recovering losses.

While RFC 6675 says in section 5, "Algorithm Details",

   (2) If DupAcks < DupThresh but IsLost (HighACK + 1) returns true --
       indicating at least three segments have arrived above the current
       cumulative acknowledgment point, which is taken to indicate loss
       -- go to step (4).
...
   (4) Invoke fast retransmit and enter loss recovery as follows:

there are old TCP stacks not implementing this strategy, and
still counting the dupacks before starting fast retransmit.

While these stacks probably perform poorly when receivers implement
LRO/GRO, we should be a little more gentle to them.

This patch makes sure we do not enable SACK compression unless
3 dupacks have been sent since last rcv_nxt update.

Ideally we should even rearm the timer to send one or two
more DUPACK if no more packets are coming, but that will
be work aiming for linux-4.21.

Many thanks to Jean-Louis for bisecting the issue, providing
packet captures and testing this patch.

Fixes: 5d9f4262b7ea ("tcp: add SACK compression")
Reported-by: Jean-Louis Dupond <jean-louis@dupond.be>
Tested-by: Jean-Louis Dupond <jean-louis@dupond.be>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
---
 include/linux/tcp.h   |  1 +
 net/ipv4/tcp_input.c  | 14 ++++++++++++--
 net/ipv4/tcp_output.c |  6 +++---
 net/ipv4/tcp_timer.c  |  2 +-
 4 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 8ed77bb4ed8636e9294389a011529fd9a667dce4..a9b0280687d52797972506a8bac13ed0747e2182 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -196,6 +196,7 @@ struct tcp_sock {
 	u32	rcv_tstamp;	/* timestamp of last received ACK (for keepalives) */
 	u32	lsndtime;	/* timestamp of last sent data packet (for restart window) */
 	u32	last_oow_ack_time;  /* timestamp of last out-of-window ACK */
+	u32	compressed_ack_rcv_nxt;
 
 	u32	tsoffset;	/* timestamp offset */
 
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 2868ef28ce52179b3c5874e749b680ffbdc0521a..81e4264676b404fdfa23c2aeb9fe50459deee120 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4268,7 +4268,7 @@ static void tcp_sack_new_ofo_skb(struct sock *sk, u32 seq, u32 end_seq)
 	 * If the sack array is full, forget about the last one.
 	 */
 	if (this_sack >= TCP_NUM_SACKS) {
-		if (tp->compressed_ack)
+		if (tp->compressed_ack > TCP_FASTRETRANS_THRESH)
 			tcp_send_ack(sk);
 		this_sack--;
 		tp->rx_opt.num_sacks--;
@@ -5188,7 +5188,17 @@ static void __tcp_ack_snd_check(struct sock *sk, int ofo_possible)
 	if (!tcp_is_sack(tp) ||
 	    tp->compressed_ack >= sock_net(sk)->ipv4.sysctl_tcp_comp_sack_nr)
 		goto send_now;
-	tp->compressed_ack++;
+
+	if (tp->compressed_ack_rcv_nxt != tp->rcv_nxt) {
+		tp->compressed_ack_rcv_nxt = tp->rcv_nxt;
+		if (tp->compressed_ack > TCP_FASTRETRANS_THRESH)
+			NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPACKCOMPRESSED,
+				      tp->compressed_ack - TCP_FASTRETRANS_THRESH);
+		tp->compressed_ack = 0;
+	}
+
+	if (++tp->compressed_ack <= TCP_FASTRETRANS_THRESH)
+		goto send_now;
 
 	if (hrtimer_is_queued(&tp->compressed_ack_timer))
 		return;
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 9c34b97d365d719ff76250bc9fe7fa20495a3ed2..3f510cad0b3ec884aeb23f58aaa597ec98c82c88 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -180,10 +180,10 @@ static inline void tcp_event_ack_sent(struct sock *sk, unsigned int pkts,
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 
-	if (unlikely(tp->compressed_ack)) {
+	if (unlikely(tp->compressed_ack > TCP_FASTRETRANS_THRESH)) {
 		NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPACKCOMPRESSED,
-			      tp->compressed_ack);
-		tp->compressed_ack = 0;
+			      tp->compressed_ack - TCP_FASTRETRANS_THRESH);
+		tp->compressed_ack = TCP_FASTRETRANS_THRESH;
 		if (hrtimer_try_to_cancel(&tp->compressed_ack_timer) == 1)
 			__sock_put(sk);
 	}
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 676020663ce80a79341ad1a05352742cc8dd5850..5f8b6d3cd855dc639409e69d84ade5bb2be51626 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -740,7 +740,7 @@ static enum hrtimer_restart tcp_compressed_ack_kick(struct hrtimer *timer)
 
 	bh_lock_sock(sk);
 	if (!sock_owned_by_user(sk)) {
-		if (tp->compressed_ack)
+		if (tp->compressed_ack > TCP_FASTRETRANS_THRESH)
 			tcp_send_ack(sk);
 	} else {
 		if (!test_and_set_bit(TCP_DELACK_TIMER_DEFERRED,
-- 
2.19.1.1215.g8438c0b245-goog

^ permalink raw reply related

* Re: [PATCH net] sctp: hold transport before accessing its asoc in sctp_epaddr_lookup_transport
From: Neil Horman @ 2018-11-20 14:00 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, linux-sctp, davem, Marcelo Ricardo Leitner
In-Reply-To: <6d2d91c6173328d6e8868849686aae34e51c889f.1542712330.git.lucien.xin@gmail.com>

On Tue, Nov 20, 2018 at 07:12:10PM +0800, Xin Long wrote:
> Without holding transport to dereference its asoc, a use after
> free panic can be caused in sctp_epaddr_lookup_transport. Note
> that a sock lock can't protect these transports that belong to
> other socks.
> 
> A similar fix as Commit bab1be79a516 ("sctp: hold transport
> before accessing its asoc in sctp_transport_get_next") is
> needed to hold the transport before accessing its asoc in
> sctp_epaddr_lookup_transport.
> 
> Fixes: 7fda702f9315 ("sctp: use new rhlist interface on sctp transport rhashtable")
> Reported-by: syzbot+aad231d51b1923158444@syzkaller.appspotmail.com
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  net/sctp/input.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/net/sctp/input.c b/net/sctp/input.c
> index 69584e9..c2c0816 100644
> --- a/net/sctp/input.c
> +++ b/net/sctp/input.c
> @@ -972,9 +972,15 @@ struct sctp_transport *sctp_epaddr_lookup_transport(
>  	list = rhltable_lookup(&sctp_transport_hashtable, &arg,
>  			       sctp_hash_params);
>  
> -	rhl_for_each_entry_rcu(t, tmp, list, node)
> -		if (ep == t->asoc->ep)
> +	rhl_for_each_entry_rcu(t, tmp, list, node) {
> +		if (!sctp_transport_hold(t))
> +			continue;
> +		if (ep == t->asoc->ep) {
> +			sctp_transport_put(t);
>  			return t;
> +		}
> +		sctp_transport_put(t);
> +	}
>  
>  	return NULL;
>  }
> -- 
> 2.1.0
> 
> 
Same as the other patch, it seems like we shouldn't need to hold the transport
here as long as we either have a hold on the parent association already (which
we should), and or, properly quiesce the destruction of an assocation.

Neil

^ permalink raw reply

* Re: [PATCH v2] ipv4: make DSCP values works with ip rules
From: Sabrina Dubroca @ 2018-11-20 14:12 UTC (permalink / raw)
  To: Pavel Balaev; +Cc: davem, netdev
In-Reply-To: <20181120132927.GA4987@rnd.infotecs.ru>

Hi Pavel,

2018-11-20, 16:29:36 +0300, Pavel Balaev wrote:
> This patch adds ability to set DSCP values in ip rules.

You dropped the RFC reference that you had in v1.

> Values presented in /etc/iproute3/rt_dsfield and now can be used in rules.

iproute3?

> 
> Example:
> $ ip ru add from 10.88.0.2 tos AF23 lookup dscp.
> Result:
> ---

Be careful when you write commit messages. "git am" is going to cut
out everything that comes after the first ^---$ line it sees, so the
rest of those explanations, as well as your sign-off, will be dropped.

> 32762:	from 10.88.0.2 tos AF43 lookup dscp
> ---
> 
> Patch was tested with such configuration:
> 
> +-----------+
> | 10.88.0.2 | -> ping 172.16.0.1 -Q 0x58 ->
> |   host0   |
> +-----------+
> 
>    +-------------------------------------+
>    | router with patched kernel          |
>    |                                     |
> -> | from 10.88.0.2 tos AF43 lookup dscp |->
>    | table dscp:                         |
>    |   172.16.0.0/24 via 10.200.0.2      |
>    +-------------------------------------+
> 
> 	 +------------------+
> -> | eth0: 10.200.0.2 |
>    | eth1: 172.16.0.1 |
>    |      host1       |
>    +------------------+
> 
> Signed-off-by: Pavel Balaev <mail@void.so>

-- 
Sabrina

^ permalink raw reply

* Re: [PATCH] net: faraday: ftmac100: remove netif_running(netdev) check before disabling interrupts
From: Vincent Chen @ 2018-11-21  0:49 UTC (permalink / raw)
  To: David Miller
  Cc: gregkh@linuxfoundation.org, rdunlap@infradead.org,
	yuehaibing@huawei.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20181120.102843.2033247081781930217.davem@davemloft.net>

On Wed, Nov 21, 2018 at 02:28:43AM +0800, David Miller wrote:
> aFrom: Vincent Chen <vincentc@andestech.com>
> Date: Tue, 20 Nov 2018 13:37:46 +0800
> 
> > +	/* Disable interrupts for polling */
> > +	ftmac100_disable_all_int(priv);
> >  	if (likely(netif_running(netdev))) {
> > -		/* Disable interrupts for polling */
> > -		ftmac100_disable_all_int(priv);
> >  		napi_schedule(&priv->napi);
> >  	}
> 
> This is now a single-line basic block, so please remove the curly
> braces.
> 
> Thank you.

OK. I will remove it.
Thank you.

^ permalink raw reply

* [RFC v3 0/3] Add VRF support for VXLAN underlay
From: Alexis Bauvin @ 2018-11-20 14:23 UTC (permalink / raw)
  To: dsa, roopa; +Cc: netdev, abauvin, akherbouche

v2 -> v3:
- fix build when CONFIG_NET_IPV6 is off
- fix build "unused l3mdev_master_upper_ifindex_by_index" build error with some
  configs

v1 -> v2:
- move vxlan_get_l3mdev from vxlan driver to l3mdev driver as
  l3mdev_master_upper_ifindex_by_index
- vxlan: rename variables named l3mdev_ifindex to ifindex

v0 -> v1:
- fix typos

We are trying to isolate the VXLAN traffic from different VMs with VRF as shown
in the schemas below:

+-------------------------+   +----------------------------+
| +----------+            |   |     +------------+         |
| |          |            |   |     |            |         |
| | tap-red  |            |   |     |  tap-blue  |         |
| |          |            |   |     |            |         |
| +----+-----+            |   |     +-----+------+         |
|      |                  |   |           |                |
|      |                  |   |           |                |
| +----+---+              |   |      +----+----+           |
| |        |              |   |      |         |           |
| | br-red |              |   |      | br-blue |           |
| |        |              |   |      |         |           |
| +----+---+              |   |      +----+----+           |
|      |                  |   |           |                |
|      |                  |   |           |                |
|      |                  |   |           |                |
| +----+--------+         |   |     +--------------+       |
| |             |         |   |     |              |       |
| |  vxlan-red  |         |   |     |  vxlan-blue  |       |
| |             |         |   |     |              |       |
| +------+------+         |   |     +-------+------+       |
|        |                |   |             |              |
|        |           VRF  |   |             |          VRF |
|        |           red  |   |             |         blue |
+-------------------------+   +----------------------------+
         |                                  |
         |                                  |
 +---------------------------------------------------------+
 |       |                                  |              |
 |       |                                  |              |
 |       |         +--------------+         |              |
 |       |         |              |         |              |
 |       +---------+  eth0.2030   +---------+              |
 |                 |  10.0.0.1/24 |                        |
 |                 +-----+--------+                    VRF |
 |                       |                            green|
 +---------------------------------------------------------+
                         |
                         |
                    +----+---+
                    |        |
                    |  eth0  |
                    |        |
                    +--------+


iproute2 commands to reproduce the setup:

ip link add green type vrf table 1
ip link set green up
ip link add eth0.2030 link eth0 type vlan id 2030
ip link set eth0.2030 master green
ip addr add 10.0.0.1/24 dev eth0.2030
ip link set eth0.2030 up

ip link add blue type vrf table 2
ip link set blue up
ip link add br-blue type bridge
ip link set br-blue master blue
ip link set br-blue up
ip link add vxlan-blue type vxlan id 2 local 10.0.0.1 dev eth0.2030 \
 port 4789
ip link set vxlan-blue master br-blue
ip link set vxlan-blue up
ip link set tap-blue master br-blue
ip link set tap-blue up

ip link add red type vrf table 3
ip link set red up
ip link add br-red type bridge
ip link set br-red master red
ip link set br-red up
ip link add vxlan-red type vxlan id 3 local 10.0.0.1 dev eth0.2030 \
 port 4789
ip link set vxlan-red master br-red
ip link set vxlan-red up
ip link set tap-red master br-red
ip link set tap-red up

We faced some issue in the datapath, here are the details:

* Egress traffic:
The vxlan packets are sent directly to the default VRF because it's where the
socket is bound, therefore the traffic has a default route via eth0. the
workarount is to force this traffic to VRF green with ip rules.

* Ingress traffic:
When receiving the traffic on eth0.2030 the vxlan socket is unreachable from
VRF green. The workaround is to enable *udp_l3mdev_accept* sysctl, but
this breaks isolation between overlay and underlay: packets sent from
blue or red by e.g. a guest VM will be accepted by the socket, allowing
injection of VXLAN packets from the overlay.

This patch serie fixes the issues describe above by allowing VXLAN socket to be
bound to a specific VRF device therefore looking up in the correct table.

Alexis Bauvin (3):
  udp_tunnel: add config option to bind to a device
  vxlan: add support for underlay in non-default VRF
  vxlan: handle underlay VRF changes

 drivers/net/vxlan.c       | 126 +++++++++++++++++++++++++++++++++++---
 include/net/l3mdev.h      |  22 +++++++
 include/net/udp_tunnel.h  |   1 +
 net/ipv4/udp_tunnel.c     |  10 +++
 net/ipv6/ip6_udp_tunnel.c |   9 +++
 net/l3mdev/l3mdev.c       |  18 ++++++
 6 files changed, 178 insertions(+), 8 deletions(-)

-- 

^ permalink raw reply

* [RFC v3 1/3] udp_tunnel: add config option to bind to a device
From: Alexis Bauvin @ 2018-11-20 14:23 UTC (permalink / raw)
  To: dsa, roopa; +Cc: netdev, abauvin, akherbouche
In-Reply-To: <20181120142317.88277-1-abauvin@scaleway.com>

UDP tunnel sockets are always opened unbound to a specific device. This
patch allow the socket to be bound on a custom device, which
incidentally makes UDP tunnels VRF-aware if binding to an l3mdev.

Signed-off-by: Alexis Bauvin <abauvin@scaleway.com>
Reviewed-by: Amine Kherbouche <akherbouche@scaleway.com>
Tested-by: Amine Kherbouche <akherbouche@scaleway.com>
---
 include/net/udp_tunnel.h  |  1 +
 net/ipv4/udp_tunnel.c     | 10 ++++++++++
 net/ipv6/ip6_udp_tunnel.c |  9 +++++++++
 3 files changed, 20 insertions(+)

diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
index fe680ab6b15a..9f7970d010f9 100644
--- a/include/net/udp_tunnel.h
+++ b/include/net/udp_tunnel.h
@@ -30,6 +30,7 @@ struct udp_port_cfg {
 
 	__be16			local_udp_port;
 	__be16			peer_udp_port;
+	int			bind_ifindex;
 	unsigned int		use_udp_checksums:1,
 				use_udp6_tx_checksums:1,
 				use_udp6_rx_checksums:1,
diff --git a/net/ipv4/udp_tunnel.c b/net/ipv4/udp_tunnel.c
index 6539ff15e9a3..dc68e15a4f72 100644
--- a/net/ipv4/udp_tunnel.c
+++ b/net/ipv4/udp_tunnel.c
@@ -20,6 +20,16 @@ int udp_sock_create4(struct net *net, struct udp_port_cfg *cfg,
 	if (err < 0)
 		goto error;
 
+	if (cfg->bind_ifindex) {
+		struct net_device *dev;
+
+		dev = __dev_get_by_index(net, cfg->bind_ifindex);
+		err = kernel_setsockopt(sock, SOL_SOCKET, SO_BINDTODEVICE,
+					dev->name, strlen(dev->name) + 1);
+		if (err < 0)
+			goto error;
+	}
+
 	udp_addr.sin_family = AF_INET;
 	udp_addr.sin_addr = cfg->local_ip;
 	udp_addr.sin_port = cfg->local_udp_port;
diff --git a/net/ipv6/ip6_udp_tunnel.c b/net/ipv6/ip6_udp_tunnel.c
index b283f293ee4a..fc3811ef8787 100644
--- a/net/ipv6/ip6_udp_tunnel.c
+++ b/net/ipv6/ip6_udp_tunnel.c
@@ -31,6 +31,15 @@ int udp_sock_create6(struct net *net, struct udp_port_cfg *cfg,
 		if (err < 0)
 			goto error;
 	}
+	if (cfg->bind_ifindex) {
+		struct net_device *dev;
+
+		dev = __dev_get_by_index(net, cfg->bind_ifindex);
+		err = kernel_setsockopt(sock, SOL_SOCKET, SO_BINDTODEVICE,
+					dev->name, strlen(dev->name) + 1);
+		if (err < 0)
+			goto error;
+	}
 
 	udp6_addr.sin6_family = AF_INET6;
 	memcpy(&udp6_addr.sin6_addr, &cfg->local_ip6,
-- 

^ permalink raw reply related

* [RFC v3 2/3] vxlan: add support for underlay in non-default VRF
From: Alexis Bauvin @ 2018-11-20 14:23 UTC (permalink / raw)
  To: dsa, roopa; +Cc: netdev, abauvin, akherbouche
In-Reply-To: <20181120142317.88277-1-abauvin@scaleway.com>

Creating a VXLAN device with is underlay in the non-default VRF makes
egress route lookup fail or incorrect since it will resolve in the
default VRF, and ingress fail because the socket listens in the default
VRF.

This patch binds the underlying UDP tunnel socket to the l3mdev of the
lower device of the VXLAN device. This will listen in the proper VRF and
output traffic from said l3mdev, matching l3mdev routing rules and
looking up the correct routing table.

When the VXLAN device does not have a lower device, or the lower device
is in the default VRF, the socket will not be bound to any interface,
keeping the previous behaviour.

The underlay l3mdev is deduced from the VXLAN lower device
(IFLA_VXLAN_LINK).

The l3mdev_master_upper_ifindex_by_index function has been added to
l3mdev. Its goal is to fetch the effective l3mdev of an interface which
is not a direct slave of said l3mdev. It handles the following example,
properly resolving the l3mdev of eth0 to vrf-blue:

+----------+                         +---------+
|          |                         |         |
| vrf-blue |                         | vrf-red |
|          |                         |         |
+----+-----+                         +----+----+
     |                                    |
     |                                    |
+----+-----+                         +----+----+
|          |                         |         |
| br-blue  |                         | br-red  |
|          |                         |         |
+----+-----+                         +---+-+---+
     |                                   | |
     |                             +-----+ +-----+
     |                             |             |
+----+-----+                +------+----+   +----+----+
|          |  lower device  |           |   |         |
|   eth0   | <- - - - - - - | vxlan-red |   | tap-red | (... more taps)
|          |                |           |   |         |
+----------+                +-----------+   +---------+

Signed-off-by: Alexis Bauvin <abauvin@scaleway.com>
Reviewed-by: Amine Kherbouche <akherbouche@scaleway.com>
Tested-by: Amine Kherbouche <akherbouche@scaleway.com>
---
 drivers/net/vxlan.c  | 32 ++++++++++++++++++++++++--------
 include/net/l3mdev.h | 22 ++++++++++++++++++++++
 net/l3mdev/l3mdev.c  | 18 ++++++++++++++++++
 3 files changed, 64 insertions(+), 8 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 27bd586b94b0..a3de08122269 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -212,7 +212,7 @@ static inline struct vxlan_rdst *first_remote_rtnl(struct vxlan_fdb *fdb)
  * and enabled unshareable flags.
  */
 static struct vxlan_sock *vxlan_find_sock(struct net *net, sa_family_t family,
-					  __be16 port, u32 flags)
+					  __be16 port, u32 flags, int ifindex)
 {
 	struct vxlan_sock *vs;
 
@@ -221,7 +221,8 @@ static struct vxlan_sock *vxlan_find_sock(struct net *net, sa_family_t family,
 	hlist_for_each_entry_rcu(vs, vs_head(net, port), hlist) {
 		if (inet_sk(vs->sock->sk)->inet_sport == port &&
 		    vxlan_get_sk_family(vs) == family &&
-		    vs->flags == flags)
+		    vs->flags == flags &&
+		    vs->sock->sk->sk_bound_dev_if == ifindex)
 			return vs;
 	}
 	return NULL;
@@ -261,7 +262,7 @@ static struct vxlan_dev *vxlan_find_vni(struct net *net, int ifindex,
 {
 	struct vxlan_sock *vs;
 
-	vs = vxlan_find_sock(net, family, port, flags);
+	vs = vxlan_find_sock(net, family, port, flags, ifindex);
 	if (!vs)
 		return NULL;
 
@@ -2172,6 +2173,9 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 		struct rtable *rt;
 		__be16 df = 0;
 
+		if (!ifindex)
+			ifindex = sock4->sock->sk->sk_bound_dev_if;
+
 		rt = vxlan_get_route(vxlan, dev, sock4, skb, ifindex, tos,
 				     dst->sin.sin_addr.s_addr,
 				     &local_ip.sin.sin_addr.s_addr,
@@ -2210,6 +2214,9 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 	} else {
 		struct vxlan_sock *sock6 = rcu_dereference(vxlan->vn6_sock);
 
+		if (!ifindex)
+			ifindex = sock6->sock->sk->sk_bound_dev_if;
+
 		ndst = vxlan6_get_route(vxlan, dev, sock6, skb, ifindex, tos,
 					label, &dst->sin6.sin6_addr,
 					&local_ip.sin6.sin6_addr,
@@ -2813,7 +2820,7 @@ static const struct ethtool_ops vxlan_ethtool_ops = {
 };
 
 static struct socket *vxlan_create_sock(struct net *net, bool ipv6,
-					__be16 port, u32 flags)
+					__be16 port, u32 flags, int ifindex)
 {
 	struct socket *sock;
 	struct udp_port_cfg udp_conf;
@@ -2831,6 +2838,7 @@ static struct socket *vxlan_create_sock(struct net *net, bool ipv6,
 	}
 
 	udp_conf.local_udp_port = port;
+	udp_conf.bind_ifindex = ifindex;
 
 	/* Open UDP socket */
 	err = udp_sock_create(net, &udp_conf, &sock);
@@ -2842,7 +2850,8 @@ static struct socket *vxlan_create_sock(struct net *net, bool ipv6,
 
 /* Create new listen socket if needed */
 static struct vxlan_sock *vxlan_socket_create(struct net *net, bool ipv6,
-					      __be16 port, u32 flags)
+					      __be16 port, u32 flags,
+					      int ifindex)
 {
 	struct vxlan_net *vn = net_generic(net, vxlan_net_id);
 	struct vxlan_sock *vs;
@@ -2857,7 +2866,7 @@ static struct vxlan_sock *vxlan_socket_create(struct net *net, bool ipv6,
 	for (h = 0; h < VNI_HASH_SIZE; ++h)
 		INIT_HLIST_HEAD(&vs->vni_list[h]);
 
-	sock = vxlan_create_sock(net, ipv6, port, flags);
+	sock = vxlan_create_sock(net, ipv6, port, flags, ifindex);
 	if (IS_ERR(sock)) {
 		kfree(vs);
 		return ERR_CAST(sock);
@@ -2894,11 +2903,17 @@ static int __vxlan_sock_add(struct vxlan_dev *vxlan, bool ipv6)
 	struct vxlan_net *vn = net_generic(vxlan->net, vxlan_net_id);
 	struct vxlan_sock *vs = NULL;
 	struct vxlan_dev_node *node;
+	int l3mdev_index;
+
+	l3mdev_index =
+		l3mdev_master_upper_ifindex_by_index(vxlan->net,
+						     vxlan->cfg.remote_ifindex);
 
 	if (!vxlan->cfg.no_share) {
 		spin_lock(&vn->sock_lock);
 		vs = vxlan_find_sock(vxlan->net, ipv6 ? AF_INET6 : AF_INET,
-				     vxlan->cfg.dst_port, vxlan->cfg.flags);
+				     vxlan->cfg.dst_port, vxlan->cfg.flags,
+				     l3mdev_index);
 		if (vs && !refcount_inc_not_zero(&vs->refcnt)) {
 			spin_unlock(&vn->sock_lock);
 			return -EBUSY;
@@ -2907,7 +2922,8 @@ static int __vxlan_sock_add(struct vxlan_dev *vxlan, bool ipv6)
 	}
 	if (!vs)
 		vs = vxlan_socket_create(vxlan->net, ipv6,
-					 vxlan->cfg.dst_port, vxlan->cfg.flags);
+					 vxlan->cfg.dst_port, vxlan->cfg.flags,
+					 l3mdev_index);
 	if (IS_ERR(vs))
 		return PTR_ERR(vs);
 #if IS_ENABLED(CONFIG_IPV6)
diff --git a/include/net/l3mdev.h b/include/net/l3mdev.h
index 3832099289c5..78fa0ac4613c 100644
--- a/include/net/l3mdev.h
+++ b/include/net/l3mdev.h
@@ -101,6 +101,17 @@ struct net_device *l3mdev_master_dev_rcu(const struct net_device *_dev)
 	return master;
 }
 
+int l3mdev_master_upper_ifindex_by_index_rcu(struct net *net, int ifindex);
+static inline
+int l3mdev_master_upper_ifindex_by_index(struct net *net, int ifindex)
+{
+	rcu_read_lock();
+	ifindex = l3mdev_master_upper_ifindex_by_index_rcu(net, ifindex);
+	rcu_read_unlock();
+
+	return ifindex;
+}
+
 u32 l3mdev_fib_table_rcu(const struct net_device *dev);
 u32 l3mdev_fib_table_by_index(struct net *net, int ifindex);
 static inline u32 l3mdev_fib_table(const struct net_device *dev)
@@ -207,6 +218,17 @@ static inline int l3mdev_master_ifindex_by_index(struct net *net, int ifindex)
 	return 0;
 }
 
+static inline
+int l3mdev_master_upper_ifindex_by_index_rcu(struct net *net, int ifindex)
+{
+	return 0;
+}
+static inline
+int l3mdev_master_upper_ifindex_by_index(struct net *net, int ifindex)
+{
+	return 0;
+}
+
 static inline
 struct net_device *l3mdev_master_dev_rcu(const struct net_device *dev)
 {
diff --git a/net/l3mdev/l3mdev.c b/net/l3mdev/l3mdev.c
index 8da86ceca33d..309dee76724e 100644
--- a/net/l3mdev/l3mdev.c
+++ b/net/l3mdev/l3mdev.c
@@ -46,6 +46,24 @@ int l3mdev_master_ifindex_rcu(const struct net_device *dev)
 }
 EXPORT_SYMBOL_GPL(l3mdev_master_ifindex_rcu);
 
+/**
+ *	l3mdev_master_upper_ifindex_by_index - get index of upper l3 master
+ *					       device
+ *	@net: network namespace for device index lookup
+ *	@ifindex: targeted interface
+ */
+int l3mdev_master_upper_ifindex_by_index_rcu(struct net *net, int ifindex)
+{
+	struct net_device *dev;
+
+	dev = dev_get_by_index_rcu(net, ifindex);
+	while (dev && !netif_is_l3_master(dev))
+		dev = netdev_master_upper_dev_get(dev);
+
+	return dev ? dev->ifindex : 0;
+}
+EXPORT_SYMBOL_GPL(l3mdev_master_upper_ifindex_by_index_rcu);
+
 /**
  *	l3mdev_fib_table - get FIB table id associated with an L3
  *                             master interface
-- 

^ permalink raw reply related

* [RFC v3 3/3] vxlan: handle underlay VRF changes
From: Alexis Bauvin @ 2018-11-20 14:23 UTC (permalink / raw)
  To: dsa, roopa; +Cc: netdev, abauvin, akherbouche
In-Reply-To: <20181120142317.88277-1-abauvin@scaleway.com>

When underlay VRF changes, either because the lower device itself changed,
or its VRF changed, this patch releases the current socket of the VXLAN
device and recreates another one in the right VRF. This allows for
on-the-fly change of the underlay VRF of a VXLAN device.

Signed-off-by: Alexis Bauvin <abauvin@scaleway.com>
Reviewed-by: Amine Kherbouche <akherbouche@scaleway.com>
Tested-by: Amine Kherbouche <akherbouche@scaleway.com>
---
 drivers/net/vxlan.c | 94 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 94 insertions(+)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index a3de08122269..1e6ccad6df6a 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -208,6 +208,18 @@ static inline struct vxlan_rdst *first_remote_rtnl(struct vxlan_fdb *fdb)
 	return list_first_entry(&fdb->remotes, struct vxlan_rdst, list);
 }
 
+static int vxlan_is_in_l3mdev_chain(struct net_device *chain,
+				    struct net_device *dev)
+{
+	if (!chain)
+		return 0;
+
+	if (chain->ifindex == dev->ifindex)
+		return 1;
+	return vxlan_is_in_l3mdev_chain(netdev_master_upper_dev_get(chain),
+					dev);
+}
+
 /* Find VXLAN socket based on network namespace, address family and UDP port
  * and enabled unshareable flags.
  */
@@ -3720,6 +3732,33 @@ struct net_device *vxlan_dev_create(struct net *net, const char *name,
 }
 EXPORT_SYMBOL_GPL(vxlan_dev_create);
 
+static int vxlan_reopen(struct vxlan_net *vn, struct vxlan_dev *vxlan)
+{
+	int ret = 0;
+
+	if (vxlan_addr_multicast(&vxlan->default_dst.remote_ip) &&
+	    !vxlan_group_used(vn, vxlan))
+		ret = vxlan_igmp_leave(vxlan);
+	vxlan_sock_release(vxlan);
+
+	if (ret < 0)
+		return ret;
+
+	ret = vxlan_sock_add(vxlan);
+	if (ret < 0)
+		return ret;
+
+	if (vxlan_addr_multicast(&vxlan->default_dst.remote_ip)) {
+		ret = vxlan_igmp_join(vxlan);
+		if (ret == -EADDRINUSE)
+			ret = 0;
+		if (ret)
+			vxlan_sock_release(vxlan);
+	}
+
+	return ret;
+}
+
 static void vxlan_handle_lowerdev_unregister(struct vxlan_net *vn,
 					     struct net_device *dev)
 {
@@ -3742,6 +3781,55 @@ static void vxlan_handle_lowerdev_unregister(struct vxlan_net *vn,
 	unregister_netdevice_many(&list_kill);
 }
 
+static void vxlan_handle_change_upper(struct vxlan_net *vn,
+				      struct net_device *dev)
+{
+	struct vxlan_dev *vxlan, *next;
+
+	list_for_each_entry_safe(vxlan, next, &vn->vxlan_list, next) {
+		struct net_device *lower;
+		int err;
+
+		lower = __dev_get_by_index(vxlan->net,
+					   vxlan->cfg.remote_ifindex);
+		if (!vxlan_is_in_l3mdev_chain(lower, dev))
+			continue;
+
+		err = vxlan_reopen(vn, vxlan);
+		if (err < 0)
+			netdev_err(vxlan->dev, "Failed to reopen socket: %d\n",
+				   err);
+	}
+}
+
+static void vxlan_handle_change(struct vxlan_net *vn, struct net_device *dev)
+{
+	struct vxlan_dev *vxlan = netdev_priv(dev);
+	struct vxlan_sock *sock;
+	int l3mdev_index;
+
+#if IS_ENABLED(CONFIG_IPV6)
+	bool metadata = vxlan->cfg.flags & VXLAN_F_COLLECT_METADATA;
+	bool ipv6 = vxlan->cfg.flags & VXLAN_F_IPV6 || metadata;
+
+	sock = ipv6 ? rcu_dereference(vxlan->vn6_sock)
+		    : rcu_dereference(vxlan->vn4_sock);
+#else
+	sock = rcu_dereference(vxlan->vn4_sock);
+#endif
+
+	l3mdev_index =
+		l3mdev_master_upper_ifindex_by_index(vxlan->net,
+						     vxlan->cfg.remote_ifindex);
+	if (sock->sock->sk->sk_bound_dev_if != l3mdev_index) {
+		int ret = vxlan_reopen(vn, vxlan);
+
+		if (ret < 0)
+			netdev_err(vxlan->dev, "Failed to reopen socket: %d\n",
+				   ret);
+	}
+}
+
 static int vxlan_netdevice_event(struct notifier_block *unused,
 				 unsigned long event, void *ptr)
 {
@@ -3756,6 +3844,12 @@ static int vxlan_netdevice_event(struct notifier_block *unused,
 	} else if (event == NETDEV_UDP_TUNNEL_PUSH_INFO ||
 		   event == NETDEV_UDP_TUNNEL_DROP_INFO) {
 		vxlan_offload_rx_ports(dev, event == NETDEV_UDP_TUNNEL_PUSH_INFO);
+	} else if (event == NETDEV_CHANGEUPPER) {
+		vxlan_handle_change_upper(vn, dev);
+	} else if (event == NETDEV_CHANGE) {
+		if (dev->rtnl_link_ops &&
+		    !strcmp(dev->rtnl_link_ops->kind, vxlan_link_ops.kind))
+			vxlan_handle_change(vn, dev);
 	}
 
 	return NOTIFY_DONE;
-- 

^ permalink raw reply related

* Re: [PATCH v2] ipv4: make DSCP values works with ip rules
From: Pavel Balaev @ 2018-11-20 14:36 UTC (permalink / raw)
  To: Sabrina Dubroca; +Cc: netdev
In-Reply-To: <20181120141218.GA3474@bistromath.localdomain>

On Tue, Nov 20, 2018 at 03:12:18PM +0100, Sabrina Dubroca wrote:
> Hi Pavel,
> 
> 2018-11-20, 16:29:36 +0300, Pavel Balaev wrote:
> > This patch adds ability to set DSCP values in ip rules.
> 
> You dropped the RFC reference that you had in v1.
> 
RFC2474 and RFC2597 values.
> > Values presented in /etc/iproute3/rt_dsfield and now can be used in rules.
> 
> iproute3?
> 
That was a typo, iproute2.
> > 
> > Example:
> > $ ip ru add from 10.88.0.2 tos AF23 lookup dscp.
> > Result:
> > ---
> 
> Be careful when you write commit messages. "git am" is going to cut
> out everything that comes after the first ^---$ line it sees, so the
> rest of those explanations, as well as your sign-off, will be dropped.
> 
> > 32762:	from 10.88.0.2 tos AF43 lookup dscp
> > ---
> > 
> > Patch was tested with such configuration:
> > 
> > +-----------+
> > | 10.88.0.2 | -> ping 172.16.0.1 -Q 0x58 ->
> > |   host0   |
> > +-----------+
> > 
> >    +-------------------------------------+
> >    | router with patched kernel          |
> >    |                                     |
> > -> | from 10.88.0.2 tos AF43 lookup dscp |->
> >    | table dscp:                         |
> >    |   172.16.0.0/24 via 10.200.0.2      |
> >    +-------------------------------------+
> > 
> > 	 +------------------+
> > -> | eth0: 10.200.0.2 |
> >    | eth1: 172.16.0.1 |
> >    |      host1       |
> >    +------------------+
> > 
> > Signed-off-by: Pavel Balaev <mail@void.so>
> 
> -- 
> Sabrina
Ok. Got it. Tnx.

^ permalink raw reply

* Re: [RFC v3 0/3] Add VRF support for VXLAN underlay
From: David Ahern @ 2018-11-20 14:48 UTC (permalink / raw)
  To: Alexis Bauvin, roopa; +Cc: netdev, akherbouche
In-Reply-To: <20181120142317.88277-1-abauvin@scaleway.com>

On 11/20/18 7:23 AM, Alexis Bauvin wrote:
> We are trying to isolate the VXLAN traffic from different VMs with VRF as shown
> in the schemas below:
> 
> +-------------------------+   +----------------------------+
> | +----------+            |   |     +------------+         |
> | |          |            |   |     |            |         |
> | | tap-red  |            |   |     |  tap-blue  |         |
> | |          |            |   |     |            |         |
> | +----+-----+            |   |     +-----+------+         |
> |      |                  |   |           |                |
> |      |                  |   |           |                |
> | +----+---+              |   |      +----+----+           |
> | |        |              |   |      |         |           |
> | | br-red |              |   |      | br-blue |           |
> | |        |              |   |      |         |           |
> | +----+---+              |   |      +----+----+           |
> |      |                  |   |           |                |
> |      |                  |   |           |                |
> |      |                  |   |           |                |
> | +----+--------+         |   |     +--------------+       |
> | |             |         |   |     |              |       |
> | |  vxlan-red  |         |   |     |  vxlan-blue  |       |
> | |             |         |   |     |              |       |
> | +------+------+         |   |     +-------+------+       |
> |        |                |   |             |              |
> |        |           VRF  |   |             |          VRF |
> |        |           red  |   |             |         blue |
> +-------------------------+   +----------------------------+
>          |                                  |
>          |                                  |
>  +---------------------------------------------------------+
>  |       |                                  |              |
>  |       |                                  |              |
>  |       |         +--------------+         |              |
>  |       |         |              |         |              |
>  |       +---------+  eth0.2030   +---------+              |
>  |                 |  10.0.0.1/24 |                        |
>  |                 +-----+--------+                    VRF |
>  |                       |                            green|
>  +---------------------------------------------------------+
>                          |
>                          |
>                     +----+---+
>                     |        |
>                     |  eth0  |
>                     |        |
>                     +--------+
> 
> 
> iproute2 commands to reproduce the setup:

Thanks for putting the diagram and commands in the cover letter. Really
helps to understood (and test) what you are wanting to do.

^ permalink raw reply

* Re: [RFC v3 2/3] vxlan: add support for underlay in non-default VRF
From: David Ahern @ 2018-11-20 14:57 UTC (permalink / raw)
  To: Alexis Bauvin, roopa; +Cc: netdev, akherbouche
In-Reply-To: <20181120142317.88277-3-abauvin@scaleway.com>

On 11/20/18 7:23 AM, Alexis Bauvin wrote:
> Creating a VXLAN device with is underlay in the non-default VRF makes
> egress route lookup fail or incorrect since it will resolve in the
> default VRF, and ingress fail because the socket listens in the default
> VRF.
> 
> This patch binds the underlying UDP tunnel socket to the l3mdev of the
> lower device of the VXLAN device. This will listen in the proper VRF and
> output traffic from said l3mdev, matching l3mdev routing rules and
> looking up the correct routing table.
> 
> When the VXLAN device does not have a lower device, or the lower device
> is in the default VRF, the socket will not be bound to any interface,
> keeping the previous behaviour.
> 
> The underlay l3mdev is deduced from the VXLAN lower device
> (IFLA_VXLAN_LINK).
> 
> The l3mdev_master_upper_ifindex_by_index function has been added to
> l3mdev. Its goal is to fetch the effective l3mdev of an interface which
> is not a direct slave of said l3mdev. It handles the following example,
> properly resolving the l3mdev of eth0 to vrf-blue:
> 
> +----------+                         +---------+
> |          |                         |         |
> | vrf-blue |                         | vrf-red |
> |          |                         |         |
> +----+-----+                         +----+----+
>      |                                    |
>      |                                    |
> +----+-----+                         +----+----+
> |          |                         |         |
> | br-blue  |                         | br-red  |
> |          |                         |         |
> +----+-----+                         +---+-+---+
>      |                                   | |
>      |                             +-----+ +-----+
>      |                             |             |
> +----+-----+                +------+----+   +----+----+
> |          |  lower device  |           |   |         |
> |   eth0   | <- - - - - - - | vxlan-red |   | tap-red | (... more taps)
> |          |                |           |   |         |
> +----------+                +-----------+   +---------+

same here. Very helpful diagram.


> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 27bd586b94b0..a3de08122269 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c

The vxlan changes look ok to me. It would be good for someone who know
that code better than I do to review it.

Move the following l3mdev changes to a separate patch - introduce infra
changes separate from their use:

> diff --git a/include/net/l3mdev.h b/include/net/l3mdev.h
> index 3832099289c5..78fa0ac4613c 100644
> --- a/include/net/l3mdev.h
> +++ b/include/net/l3mdev.h
> @@ -101,6 +101,17 @@ struct net_device *l3mdev_master_dev_rcu(const struct net_device *_dev)
>  	return master;
>  }
>  
> +int l3mdev_master_upper_ifindex_by_index_rcu(struct net *net, int ifindex);
> +static inline
> +int l3mdev_master_upper_ifindex_by_index(struct net *net, int ifindex)
> +{
> +	rcu_read_lock();
> +	ifindex = l3mdev_master_upper_ifindex_by_index_rcu(net, ifindex);
> +	rcu_read_unlock();
> +
> +	return ifindex;
> +}
> +
>  u32 l3mdev_fib_table_rcu(const struct net_device *dev);
>  u32 l3mdev_fib_table_by_index(struct net *net, int ifindex);
>  static inline u32 l3mdev_fib_table(const struct net_device *dev)
> @@ -207,6 +218,17 @@ static inline int l3mdev_master_ifindex_by_index(struct net *net, int ifindex)
>  	return 0;
>  }
>  
> +static inline
> +int l3mdev_master_upper_ifindex_by_index_rcu(struct net *net, int ifindex)
> +{
> +	return 0;
> +}
> +static inline
> +int l3mdev_master_upper_ifindex_by_index(struct net *net, int ifindex)
> +{
> +	return 0;
> +}
> +
>  static inline
>  struct net_device *l3mdev_master_dev_rcu(const struct net_device *dev)
>  {
> diff --git a/net/l3mdev/l3mdev.c b/net/l3mdev/l3mdev.c
> index 8da86ceca33d..309dee76724e 100644
> --- a/net/l3mdev/l3mdev.c
> +++ b/net/l3mdev/l3mdev.c
> @@ -46,6 +46,24 @@ int l3mdev_master_ifindex_rcu(const struct net_device *dev)
>  }
>  EXPORT_SYMBOL_GPL(l3mdev_master_ifindex_rcu);
>  
> +/**
> + *	l3mdev_master_upper_ifindex_by_index - get index of upper l3 master
> + *					       device
> + *	@net: network namespace for device index lookup
> + *	@ifindex: targeted interface
> + */
> +int l3mdev_master_upper_ifindex_by_index_rcu(struct net *net, int ifindex)
> +{
> +	struct net_device *dev;
> +
> +	dev = dev_get_by_index_rcu(net, ifindex);
> +	while (dev && !netif_is_l3_master(dev))
> +		dev = netdev_master_upper_dev_get(dev);
> +
> +	return dev ? dev->ifindex : 0;
> +}
> +EXPORT_SYMBOL_GPL(l3mdev_master_upper_ifindex_by_index_rcu);
> +
>  /**
>   *	l3mdev_fib_table - get FIB table id associated with an L3
>   *                             master interface
> 

^ permalink raw reply

* Re: [RFC v3 3/3] vxlan: handle underlay VRF changes
From: David Ahern @ 2018-11-20 15:04 UTC (permalink / raw)
  To: Alexis Bauvin, roopa; +Cc: netdev, akherbouche
In-Reply-To: <20181120142317.88277-4-abauvin@scaleway.com>

On 11/20/18 7:23 AM, Alexis Bauvin wrote:
> When underlay VRF changes, either because the lower device itself changed,
> or its VRF changed, this patch releases the current socket of the VXLAN
> device and recreates another one in the right VRF. This allows for
> on-the-fly change of the underlay VRF of a VXLAN device.
> 
> Signed-off-by: Alexis Bauvin <abauvin@scaleway.com>
> Reviewed-by: Amine Kherbouche <akherbouche@scaleway.com>
> Tested-by: Amine Kherbouche <akherbouche@scaleway.com>
> ---
>  drivers/net/vxlan.c | 94 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 94 insertions(+)
> 
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index a3de08122269..1e6ccad6df6a 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -208,6 +208,18 @@ static inline struct vxlan_rdst *first_remote_rtnl(struct vxlan_fdb *fdb)
>  	return list_first_entry(&fdb->remotes, struct vxlan_rdst, list);
>  }
>  
> +static int vxlan_is_in_l3mdev_chain(struct net_device *chain,
> +				    struct net_device *dev)
> +{
> +	if (!chain)
> +		return 0;
> +
> +	if (chain->ifindex == dev->ifindex)
> +		return 1;
> +	return vxlan_is_in_l3mdev_chain(netdev_master_upper_dev_get(chain),
> +					dev);
> +}

This should return bool and true/false.

Also, why l3mdev in the name? None of the checks look at whether it is
an l3mdev master.

And again here, someone more familiar with the vxlan code should review it.

> +
>  /* Find VXLAN socket based on network namespace, address family and UDP port
>   * and enabled unshareable flags.
>   */
> @@ -3720,6 +3732,33 @@ struct net_device *vxlan_dev_create(struct net *net, const char *name,
>  }
>  EXPORT_SYMBOL_GPL(vxlan_dev_create);
>  
> +static int vxlan_reopen(struct vxlan_net *vn, struct vxlan_dev *vxlan)
> +{
> +	int ret = 0;
> +
> +	if (vxlan_addr_multicast(&vxlan->default_dst.remote_ip) &&
> +	    !vxlan_group_used(vn, vxlan))
> +		ret = vxlan_igmp_leave(vxlan);
> +	vxlan_sock_release(vxlan);
> +
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = vxlan_sock_add(vxlan);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (vxlan_addr_multicast(&vxlan->default_dst.remote_ip)) {
> +		ret = vxlan_igmp_join(vxlan);
> +		if (ret == -EADDRINUSE)
> +			ret = 0;
> +		if (ret)
> +			vxlan_sock_release(vxlan);
> +	}
> +
> +	return ret;
> +}
> +
>  static void vxlan_handle_lowerdev_unregister(struct vxlan_net *vn,
>  					     struct net_device *dev)
>  {
> @@ -3742,6 +3781,55 @@ static void vxlan_handle_lowerdev_unregister(struct vxlan_net *vn,
>  	unregister_netdevice_many(&list_kill);
>  }
>  
> +static void vxlan_handle_change_upper(struct vxlan_net *vn,
> +				      struct net_device *dev)
> +{
> +	struct vxlan_dev *vxlan, *next;
> +
> +	list_for_each_entry_safe(vxlan, next, &vn->vxlan_list, next) {
> +		struct net_device *lower;
> +		int err;
> +
> +		lower = __dev_get_by_index(vxlan->net,
> +					   vxlan->cfg.remote_ifindex);
> +		if (!vxlan_is_in_l3mdev_chain(lower, dev))
> +			continue;
> +
> +		err = vxlan_reopen(vn, vxlan);
> +		if (err < 0)
> +			netdev_err(vxlan->dev, "Failed to reopen socket: %d\n",
> +				   err);
> +	}
> +}
> +
> +static void vxlan_handle_change(struct vxlan_net *vn, struct net_device *dev)
> +{
> +	struct vxlan_dev *vxlan = netdev_priv(dev);
> +	struct vxlan_sock *sock;
> +	int l3mdev_index;
> +
> +#if IS_ENABLED(CONFIG_IPV6)
> +	bool metadata = vxlan->cfg.flags & VXLAN_F_COLLECT_METADATA;
> +	bool ipv6 = vxlan->cfg.flags & VXLAN_F_IPV6 || metadata;
> +
> +	sock = ipv6 ? rcu_dereference(vxlan->vn6_sock)
> +		    : rcu_dereference(vxlan->vn4_sock);
> +#else
> +	sock = rcu_dereference(vxlan->vn4_sock);
> +#endif
> +
> +	l3mdev_index =
> +		l3mdev_master_upper_ifindex_by_index(vxlan->net,
> +						     vxlan->cfg.remote_ifindex);
> +	if (sock->sock->sk->sk_bound_dev_if != l3mdev_index) {
> +		int ret = vxlan_reopen(vn, vxlan);
> +
> +		if (ret < 0)
> +			netdev_err(vxlan->dev, "Failed to reopen socket: %d\n",
> +				   ret);
> +	}
> +}
> +
>  static int vxlan_netdevice_event(struct notifier_block *unused,
>  				 unsigned long event, void *ptr)
>  {
> @@ -3756,6 +3844,12 @@ static int vxlan_netdevice_event(struct notifier_block *unused,
>  	} else if (event == NETDEV_UDP_TUNNEL_PUSH_INFO ||
>  		   event == NETDEV_UDP_TUNNEL_DROP_INFO) {
>  		vxlan_offload_rx_ports(dev, event == NETDEV_UDP_TUNNEL_PUSH_INFO);
> +	} else if (event == NETDEV_CHANGEUPPER) {
> +		vxlan_handle_change_upper(vn, dev);
> +	} else if (event == NETDEV_CHANGE) {
> +		if (dev->rtnl_link_ops &&
> +		    !strcmp(dev->rtnl_link_ops->kind, vxlan_link_ops.kind))
> +			vxlan_handle_change(vn, dev);
>  	}
>  
>  	return NOTIFY_DONE;
> 

^ permalink raw reply

* [PATCH v2] net: faraday: ftmac100: remove netif_running(netdev) check before disabling interrupts
From: Vincent Chen @ 2018-11-21  1:38 UTC (permalink / raw)
  To: davem, gregkh, rdunlap, yuehaibing; +Cc: vincentc, netdev, linux-kernel

In the original ftmac100_interrupt(), the interrupts are only disabled when
the condition "netif_running(netdev)" is true. However, this condition
causes kerenl hang in the following case. When the user requests to
disable the network device, kernel will clear the bit __LINK_STATE_START
from the dev->state and then call the driver's ndo_stop function. Network
device interrupts are not blocked during this process. If an interrupt
occurs between clearing __LINK_STATE_START and stopping network device,
kernel cannot disable the interrupts due to the condition
"netif_running(netdev)" in the ISR. Hence, kernel will hang due to the
continuous interruption of the network device.

In order to solve the above problem, the interrupts of the network device
should always be disabled in the ISR without being restricted by the
condition "netif_running(netdev)".

[V2]
Remove unnecessary curly braces.

Signed-off-by: Vincent Chen <vincentc@andestech.com>
---
 drivers/net/ethernet/faraday/ftmac100.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftmac100.c b/drivers/net/ethernet/faraday/ftmac100.c
index 21d7660..17a3625 100644
--- a/drivers/net/ethernet/faraday/ftmac100.c
+++ b/drivers/net/ethernet/faraday/ftmac100.c
@@ -875,11 +875,10 @@ static irqreturn_t ftmac100_interrupt(int irq, void *dev_id)
 	struct net_device *netdev = dev_id;
 	struct ftmac100 *priv = netdev_priv(netdev);
 
-	if (likely(netif_running(netdev))) {
-		/* Disable interrupts for polling */
-		ftmac100_disable_all_int(priv);
+	/* Disable interrupts for polling */
+	ftmac100_disable_all_int(priv);
+	if (likely(netif_running(netdev)))
 		napi_schedule(&priv->napi);
-	}
 
 	return IRQ_HANDLED;
 }
-- 
1.7.1

^ permalink raw reply related

* Fw: [Bug 201735] New: serial8250: too much work for irq4
From: Stephen Hemminger @ 2018-11-20 15:14 UTC (permalink / raw)
  To: netdev



Begin forwarded message:

Date: Tue, 20 Nov 2018 07:09:26 +0000
From: bugzilla-daemon@bugzilla.kernel.org
To: stephen@networkplumber.org
Subject: [Bug 201735] New: serial8250: too much work for irq4


https://bugzilla.kernel.org/show_bug.cgi?id=201735

            Bug ID: 201735
           Summary: serial8250: too much work for irq4
           Product: Networking
           Version: 2.5
    Kernel Version: 4.4.0-116-generic
          Hardware: All
                OS: Linux
              Tree: Mainline
            Status: NEW
          Severity: high
          Priority: P1
         Component: Other
          Assignee: stephen@networkplumber.org
          Reporter: rms200x@gmail.com
        Regression: No

Created attachment 279549
  --> https://bugzilla.kernel.org/attachment.cgi?id=279549&action=edit  
LSHW log

Virtual machines loosing network access randomly. What happens when issue is
present:

 - Network device on guest machine is lost. It's impossible to access machine.
 - Virsh console available for connection. PING from affected guest is
available but with massive packet loss (20% ~ 70%).
 - Networking restart from the guest does not resolve problem.
 - When virsh  editing and setting to: <cpu mode='host-passthrough'/> issue
seems to be resolved (after reboot).

Such issue observed on SUPERMICRO server. LSHW report attached.

-- 
You are receiving this mail because:
You are the assignee for the bug.

^ permalink raw reply

* Re: [RFC v3 2/3] vxlan: add support for underlay in non-default VRF
From: Roopa Prabhu @ 2018-11-20 15:25 UTC (permalink / raw)
  To: abauvin; +Cc: David Ahern, netdev, akherbouche
In-Reply-To: <20181120142317.88277-3-abauvin@scaleway.com>

On Tue, Nov 20, 2018 at 6:23 AM Alexis Bauvin <abauvin@scaleway.com> wrote:
>
> Creating a VXLAN device with is underlay in the non-default VRF makes
> egress route lookup fail or incorrect since it will resolve in the
> default VRF, and ingress fail because the socket listens in the default
> VRF.
>
> This patch binds the underlying UDP tunnel socket to the l3mdev of the
> lower device of the VXLAN device. This will listen in the proper VRF and
> output traffic from said l3mdev, matching l3mdev routing rules and
> looking up the correct routing table.
>
> When the VXLAN device does not have a lower device, or the lower device
> is in the default VRF, the socket will not be bound to any interface,
> keeping the previous behaviour.
>
> The underlay l3mdev is deduced from the VXLAN lower device
> (IFLA_VXLAN_LINK).
>
> The l3mdev_master_upper_ifindex_by_index function has been added to
> l3mdev. Its goal is to fetch the effective l3mdev of an interface which
> is not a direct slave of said l3mdev. It handles the following example,
> properly resolving the l3mdev of eth0 to vrf-blue:
>
> +----------+                         +---------+
> |          |                         |         |
> | vrf-blue |                         | vrf-red |
> |          |                         |         |
> +----+-----+                         +----+----+
>      |                                    |
>      |                                    |
> +----+-----+                         +----+----+
> |          |                         |         |
> | br-blue  |                         | br-red  |
> |          |                         |         |
> +----+-----+                         +---+-+---+
>      |                                   | |
>      |                             +-----+ +-----+
>      |                             |             |
> +----+-----+                +------+----+   +----+----+
> |          |  lower device  |           |   |         |
> |   eth0   | <- - - - - - - | vxlan-red |   | tap-red | (... more taps)
> |          |                |           |   |         |
> +----------+                +-----------+   +---------+
>
> Signed-off-by: Alexis Bauvin <abauvin@scaleway.com>
> Reviewed-by: Amine Kherbouche <akherbouche@scaleway.com>
> Tested-by: Amine Kherbouche <akherbouche@scaleway.com>
> ---
>  drivers/net/vxlan.c  | 32 ++++++++++++++++++++++++--------
>  include/net/l3mdev.h | 22 ++++++++++++++++++++++
>  net/l3mdev/l3mdev.c  | 18 ++++++++++++++++++
>  3 files changed, 64 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 27bd586b94b0..a3de08122269 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -212,7 +212,7 @@ static inline struct vxlan_rdst *first_remote_rtnl(struct vxlan_fdb *fdb)
>   * and enabled unshareable flags.
>   */
>  static struct vxlan_sock *vxlan_find_sock(struct net *net, sa_family_t family,
> -                                         __be16 port, u32 flags)
> +                                         __be16 port, u32 flags, int ifindex)
>  {
>         struct vxlan_sock *vs;
>
> @@ -221,7 +221,8 @@ static struct vxlan_sock *vxlan_find_sock(struct net *net, sa_family_t family,
>         hlist_for_each_entry_rcu(vs, vs_head(net, port), hlist) {
>                 if (inet_sk(vs->sock->sk)->inet_sport == port &&
>                     vxlan_get_sk_family(vs) == family &&
> -                   vs->flags == flags)
> +                   vs->flags == flags &&
> +                   vs->sock->sk->sk_bound_dev_if == ifindex)
>                         return vs;
>         }
>         return NULL;
> @@ -261,7 +262,7 @@ static struct vxlan_dev *vxlan_find_vni(struct net *net, int ifindex,
>  {
>         struct vxlan_sock *vs;
>
> -       vs = vxlan_find_sock(net, family, port, flags);
> +       vs = vxlan_find_sock(net, family, port, flags, ifindex);
>         if (!vs)
>                 return NULL;
>
> @@ -2172,6 +2173,9 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
>                 struct rtable *rt;
>                 __be16 df = 0;
>
> +               if (!ifindex)
> +                       ifindex = sock4->sock->sk->sk_bound_dev_if;
> +
>                 rt = vxlan_get_route(vxlan, dev, sock4, skb, ifindex, tos,
>                                      dst->sin.sin_addr.s_addr,
>                                      &local_ip.sin.sin_addr.s_addr,
> @@ -2210,6 +2214,9 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
>         } else {
>                 struct vxlan_sock *sock6 = rcu_dereference(vxlan->vn6_sock);
>
> +               if (!ifindex)
> +                       ifindex = sock6->sock->sk->sk_bound_dev_if;
> +
>                 ndst = vxlan6_get_route(vxlan, dev, sock6, skb, ifindex, tos,
>                                         label, &dst->sin6.sin6_addr,
>                                         &local_ip.sin6.sin6_addr,
> @@ -2813,7 +2820,7 @@ static const struct ethtool_ops vxlan_ethtool_ops = {
>  };
>
>  static struct socket *vxlan_create_sock(struct net *net, bool ipv6,
> -                                       __be16 port, u32 flags)
> +                                       __be16 port, u32 flags, int ifindex)
>  {
>         struct socket *sock;
>         struct udp_port_cfg udp_conf;
> @@ -2831,6 +2838,7 @@ static struct socket *vxlan_create_sock(struct net *net, bool ipv6,
>         }
>
>         udp_conf.local_udp_port = port;
> +       udp_conf.bind_ifindex = ifindex;
>
>         /* Open UDP socket */
>         err = udp_sock_create(net, &udp_conf, &sock);
> @@ -2842,7 +2850,8 @@ static struct socket *vxlan_create_sock(struct net *net, bool ipv6,
>
>  /* Create new listen socket if needed */
>  static struct vxlan_sock *vxlan_socket_create(struct net *net, bool ipv6,
> -                                             __be16 port, u32 flags)
> +                                             __be16 port, u32 flags,
> +                                             int ifindex)
>  {
>         struct vxlan_net *vn = net_generic(net, vxlan_net_id);
>         struct vxlan_sock *vs;
> @@ -2857,7 +2866,7 @@ static struct vxlan_sock *vxlan_socket_create(struct net *net, bool ipv6,
>         for (h = 0; h < VNI_HASH_SIZE; ++h)
>                 INIT_HLIST_HEAD(&vs->vni_list[h]);
>
> -       sock = vxlan_create_sock(net, ipv6, port, flags);
> +       sock = vxlan_create_sock(net, ipv6, port, flags, ifindex);
>         if (IS_ERR(sock)) {
>                 kfree(vs);
>                 return ERR_CAST(sock);
> @@ -2894,11 +2903,17 @@ static int __vxlan_sock_add(struct vxlan_dev *vxlan, bool ipv6)
>         struct vxlan_net *vn = net_generic(vxlan->net, vxlan_net_id);
>         struct vxlan_sock *vs = NULL;
>         struct vxlan_dev_node *node;
> +       int l3mdev_index;
> +
> +       l3mdev_index =
> +               l3mdev_master_upper_ifindex_by_index(vxlan->net,
> +                                                    vxlan->cfg.remote_ifindex);

vxlan->cfg.remote_ifindex is optional, so we can avoid trying to
derive the l3mdev_ifindex for cases where it is not present


>
>         if (!vxlan->cfg.no_share) {
>                 spin_lock(&vn->sock_lock);
>                 vs = vxlan_find_sock(vxlan->net, ipv6 ? AF_INET6 : AF_INET,
> -                                    vxlan->cfg.dst_port, vxlan->cfg.flags);
> +                                    vxlan->cfg.dst_port, vxlan->cfg.flags,
> +                                    l3mdev_index);
>                 if (vs && !refcount_inc_not_zero(&vs->refcnt)) {
>                         spin_unlock(&vn->sock_lock);
>                         return -EBUSY;
> @@ -2907,7 +2922,8 @@ static int __vxlan_sock_add(struct vxlan_dev *vxlan, bool ipv6)
>         }
>         if (!vs)
>                 vs = vxlan_socket_create(vxlan->net, ipv6,
> -                                        vxlan->cfg.dst_port, vxlan->cfg.flags);
> +                                        vxlan->cfg.dst_port, vxlan->cfg.flags,
> +                                        l3mdev_index);
>         if (IS_ERR(vs))
>                 return PTR_ERR(vs);
>  #if IS_ENABLED(CONFIG_IPV6)
> diff --git a/include/net/l3mdev.h b/include/net/l3mdev.h
> index 3832099289c5..78fa0ac4613c 100644
> --- a/include/net/l3mdev.h
> +++ b/include/net/l3mdev.h
> @@ -101,6 +101,17 @@ struct net_device *l3mdev_master_dev_rcu(const struct net_device *_dev)
>         return master;
>  }
>
> +int l3mdev_master_upper_ifindex_by_index_rcu(struct net *net, int ifindex);
> +static inline
> +int l3mdev_master_upper_ifindex_by_index(struct net *net, int ifindex)
> +{
> +       rcu_read_lock();
> +       ifindex = l3mdev_master_upper_ifindex_by_index_rcu(net, ifindex);
> +       rcu_read_unlock();
> +
> +       return ifindex;
> +}
> +
>  u32 l3mdev_fib_table_rcu(const struct net_device *dev);
>  u32 l3mdev_fib_table_by_index(struct net *net, int ifindex);
>  static inline u32 l3mdev_fib_table(const struct net_device *dev)
> @@ -207,6 +218,17 @@ static inline int l3mdev_master_ifindex_by_index(struct net *net, int ifindex)
>         return 0;
>  }
>
> +static inline
> +int l3mdev_master_upper_ifindex_by_index_rcu(struct net *net, int ifindex)
> +{
> +       return 0;
> +}
> +static inline
> +int l3mdev_master_upper_ifindex_by_index(struct net *net, int ifindex)
> +{
> +       return 0;
> +}
> +
>  static inline
>  struct net_device *l3mdev_master_dev_rcu(const struct net_device *dev)
>  {
> diff --git a/net/l3mdev/l3mdev.c b/net/l3mdev/l3mdev.c
> index 8da86ceca33d..309dee76724e 100644
> --- a/net/l3mdev/l3mdev.c
> +++ b/net/l3mdev/l3mdev.c
> @@ -46,6 +46,24 @@ int l3mdev_master_ifindex_rcu(const struct net_device *dev)
>  }
>  EXPORT_SYMBOL_GPL(l3mdev_master_ifindex_rcu);
>
> +/**
> + *     l3mdev_master_upper_ifindex_by_index - get index of upper l3 master
> + *                                            device
> + *     @net: network namespace for device index lookup
> + *     @ifindex: targeted interface
> + */
> +int l3mdev_master_upper_ifindex_by_index_rcu(struct net *net, int ifindex)
> +{
> +       struct net_device *dev;
> +
> +       dev = dev_get_by_index_rcu(net, ifindex);
> +       while (dev && !netif_is_l3_master(dev))
> +               dev = netdev_master_upper_dev_get(dev);
> +
> +       return dev ? dev->ifindex : 0;
> +}
> +EXPORT_SYMBOL_GPL(l3mdev_master_upper_ifindex_by_index_rcu);
> +
>  /**
>   *     l3mdev_fib_table - get FIB table id associated with an L3
>   *                             master interface
> --
>

^ permalink raw reply

* Re: [RFC v3 3/3] vxlan: handle underlay VRF changes
From: Roopa Prabhu @ 2018-11-20 15:35 UTC (permalink / raw)
  To: David Ahern; +Cc: abauvin, netdev, akherbouche
In-Reply-To: <e10919dc-8001-af17-f7c1-f9adc420c78a@cumulusnetworks.com>

On Tue, Nov 20, 2018 at 7:04 AM David Ahern <dsa@cumulusnetworks.com> wrote:
>
> On 11/20/18 7:23 AM, Alexis Bauvin wrote:
> > When underlay VRF changes, either because the lower device itself changed,
> > or its VRF changed, this patch releases the current socket of the VXLAN
> > device and recreates another one in the right VRF. This allows for
> > on-the-fly change of the underlay VRF of a VXLAN device.
> >
> > Signed-off-by: Alexis Bauvin <abauvin@scaleway.com>
> > Reviewed-by: Amine Kherbouche <akherbouche@scaleway.com>
> > Tested-by: Amine Kherbouche <akherbouche@scaleway.com>
> > ---
> >  drivers/net/vxlan.c | 94 +++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 94 insertions(+)
> >
> > diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> > index a3de08122269..1e6ccad6df6a 100644
> > --- a/drivers/net/vxlan.c
> > +++ b/drivers/net/vxlan.c
> > @@ -208,6 +208,18 @@ static inline struct vxlan_rdst *first_remote_rtnl(struct vxlan_fdb *fdb)
> >       return list_first_entry(&fdb->remotes, struct vxlan_rdst, list);
> >  }
> >
> > +static int vxlan_is_in_l3mdev_chain(struct net_device *chain,
> > +                                 struct net_device *dev)
> > +{
> > +     if (!chain)
> > +             return 0;
> > +
> > +     if (chain->ifindex == dev->ifindex)
> > +             return 1;
> > +     return vxlan_is_in_l3mdev_chain(netdev_master_upper_dev_get(chain),
> > +                                     dev);
> > +}
>
> This should return bool and true/false.
>
> Also, why l3mdev in the name? None of the checks look at whether it is
> an l3mdev master.
>
> And again here, someone more familiar with the vxlan code should review it.
>


I understand the need for patch 2. But I don't understand the need for
the complexity this patch introduces (especially implicit down and up
of the vxlan device).
Alexis, If your underlay routing changes,  you can down and up the
vxlan device from user-space correct ?. This should be true for any
tunnel device.

^ permalink raw reply

* Re: [RFC PATCH] net: don't keep lonely packets forever in the gro hash
From: Paolo Abeni @ 2018-11-20 15:42 UTC (permalink / raw)
  To: Eric Dumazet, netdev; +Cc: David S. Miller, Willem de Bruijn
In-Reply-To: <b403be03-6e7b-1dc5-c89d-47227b12bc6a@gmail.com>

Hi,

On Tue, 2018-11-20 at 05:49 -0800, Eric Dumazet wrote:
> 
> On 11/20/2018 02:17 AM, Paolo Abeni wrote:
> > Eric noted that with UDP GRO and napi timeout, we could keep a single
> > UDP packet inside the GRO hash forever, if the related NAPI instance
> > calls napi_gro_complete() at an higher frequency than the napi timeout.
> > Willem noted that even TCP packets could be trapped there, till the
> > next retransmission.
> > This patch tries to address the issue, flushing the oldest packets before
> > scheduling the NAPI timeout. The rationale is that such a timeout should be
> > well below a jiffy and we are not flushing packets eligible for sane GRO.
> > 
> > Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > ---
> > Sending as RFC, as I fear I'm missing some relevant pieces.
> > Also I'm unsure if this should considered a fixes for "udp: implement
> > GRO for plain UDP sockets." or for "net: gro: add a per device gro flush timer"

Thank you for your feedback!

> Truth be told, relying on jiffies change is a bit fragile for HZ=100 or HZ=250 kernels.

Yes, we have higher bound there.

> See recent TCP commit that got rid of tcp_tso_should_defer() dependency on HZ/jiffies
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/?id=a682850a114aef947da5d603f7fd2cfe7eabbd72

I'm unsure I follow correctly. Are you suggesting to use ns precision
for skb aging in GRO? If so, could that be a separate change? (looks
more invasive)

Thanks,

Paolo

^ permalink raw reply

* [PATCH v2 0/4] Fix unsafe BPF_PROG_TEST_RUN interface
From: Lorenz Bauer @ 2018-11-20 15:43 UTC (permalink / raw)
  To: ast, daniel; +Cc: netdev, linux-api, ys114321, Lorenz Bauer
In-Reply-To: <20181116125329.3974-1-lmb@cloudflare.com>

Right now, there is no safe way to use BPF_PROG_TEST_RUN with data_out.
This is because bpf_test_finish copies the output buffer to user space
without checking its size. This can lead to the kernel overwriting
data in user space after the buffer if xdp_adjust_head and friends are
in play.

Changes in v2:
* Make the syscall return ENOSPC if data_size_out is too small
* Make bpf_prog_test_run return EINVAL if size_out is missing
* Document the new behaviour of data_size_out

Lorenz Bauer (4):
  bpf: respect size hint to BPF_PROG_TEST_RUN if present
  tools: sync uapi/linux/bpf.h
  libbpf: require size hint in bpf_prog_test_run
  selftests: add a test for bpf_prog_test_run output size

 include/uapi/linux/bpf.h                 |  7 ++--
 net/bpf/test_run.c                       | 15 ++++++--
 tools/include/uapi/linux/bpf.h           |  7 ++--
 tools/lib/bpf/bpf.c                      |  7 +++-
 tools/testing/selftests/bpf/test_progs.c | 44 ++++++++++++++++++++++++
 5 files changed, 73 insertions(+), 7 deletions(-)

-- 
2.17.1

^ permalink raw reply

* [PATCH v2 1/4] bpf: respect size hint to BPF_PROG_TEST_RUN if present
From: Lorenz Bauer @ 2018-11-20 15:43 UTC (permalink / raw)
  To: ast, daniel; +Cc: netdev, linux-api, ys114321, Lorenz Bauer
In-Reply-To: <20181120154306.16657-1-lmb@cloudflare.com>

Use data_size_out as a size hint when copying test output to user space.
ENOSPC is returned if the output buffer is too small.
Callers which so far did not set data_size_out are not affected.

Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
 include/uapi/linux/bpf.h |  7 +++++--
 net/bpf/test_run.c       | 15 +++++++++++++--
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 05d95290b848..7f5a7d032cd1 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -356,8 +356,11 @@ union bpf_attr {
 	struct { /* anonymous struct used by BPF_PROG_TEST_RUN command */
 		__u32		prog_fd;
 		__u32		retval;
-		__u32		data_size_in;
-		__u32		data_size_out;
+		__u32		data_size_in;	/* input: len of data_in */
+		__u32		data_size_out;	/* input/output: len of data_out
+						 *   returns ENOSPC if data_out
+						 *   is too small.
+						 */
 		__aligned_u64	data_in;
 		__aligned_u64	data_out;
 		__u32		repeat;
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index c89c22c49015..7663e6a57280 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -74,8 +74,18 @@ static int bpf_test_finish(const union bpf_attr *kattr,
 {
 	void __user *data_out = u64_to_user_ptr(kattr->test.data_out);
 	int err = -EFAULT;
+	u32 copy_size = size;
 
-	if (data_out && copy_to_user(data_out, data, size))
+	/* Clamp copy if the user has provided a size hint, but copy the full
+	 * buffer if not to retain old behaviour.
+	 */
+	if (kattr->test.data_size_out &&
+	    copy_size > kattr->test.data_size_out) {
+		copy_size = kattr->test.data_size_out;
+		err = -ENOSPC;
+	}
+
+	if (data_out && copy_to_user(data_out, data, copy_size))
 		goto out;
 	if (copy_to_user(&uattr->test.data_size_out, &size, sizeof(size)))
 		goto out;
@@ -83,7 +93,8 @@ static int bpf_test_finish(const union bpf_attr *kattr,
 		goto out;
 	if (copy_to_user(&uattr->test.duration, &duration, sizeof(duration)))
 		goto out;
-	err = 0;
+	if (err != -ENOSPC)
+		err = 0;
 out:
 	return err;
 }
-- 
2.17.1

^ permalink raw reply related

* [PATCH v2 2/4] tools: sync uapi/linux/bpf.h
From: Lorenz Bauer @ 2018-11-20 15:43 UTC (permalink / raw)
  To: ast, daniel; +Cc: netdev, linux-api, ys114321, Lorenz Bauer
In-Reply-To: <20181120154306.16657-1-lmb@cloudflare.com>

Pull changes from "bpf: respect size hint to BPF_PROG_TEST_RUN if present".

Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
 tools/include/uapi/linux/bpf.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 05d95290b848..7f5a7d032cd1 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -356,8 +356,11 @@ union bpf_attr {
 	struct { /* anonymous struct used by BPF_PROG_TEST_RUN command */
 		__u32		prog_fd;
 		__u32		retval;
-		__u32		data_size_in;
-		__u32		data_size_out;
+		__u32		data_size_in;	/* input: len of data_in */
+		__u32		data_size_out;	/* input/output: len of data_out
+						 *   returns ENOSPC if data_out
+						 *   is too small.
+						 */
 		__aligned_u64	data_in;
 		__aligned_u64	data_out;
 		__u32		repeat;
-- 
2.17.1

^ permalink raw reply related

* [PATCH v2 3/4] libbpf: require size hint in bpf_prog_test_run
From: Lorenz Bauer @ 2018-11-20 15:43 UTC (permalink / raw)
  To: ast, daniel; +Cc: netdev, linux-api, ys114321, Lorenz Bauer
In-Reply-To: <20181120154306.16657-1-lmb@cloudflare.com>

Require size_out to be non-NULL if data_out is given. This prevents
accidental overwriting of process memory after the output buffer.

Adjust callers of bpf_prog_test_run to this behaviour.

Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
 tools/lib/bpf/bpf.c                      |  7 ++++++-
 tools/testing/selftests/bpf/test_progs.c | 10 ++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 961e1b9fc592..1a835ff27486 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -407,15 +407,20 @@ int bpf_prog_test_run(int prog_fd, int repeat, void *data, __u32 size,
 	union bpf_attr attr;
 	int ret;
 
+	if (data_out && !size_out)
+		return -EINVAL;
+
 	bzero(&attr, sizeof(attr));
 	attr.test.prog_fd = prog_fd;
 	attr.test.data_in = ptr_to_u64(data);
 	attr.test.data_out = ptr_to_u64(data_out);
 	attr.test.data_size_in = size;
+	if (data_out)
+		attr.test.data_size_out = *size_out;
 	attr.test.repeat = repeat;
 
 	ret = sys_bpf(BPF_PROG_TEST_RUN, &attr, sizeof(attr));
-	if (size_out)
+	if (data_out)
 		*size_out = attr.test.data_size_out;
 	if (retval)
 		*retval = attr.test.retval;
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index c1e688f61061..299938603cb6 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -150,6 +150,7 @@ static void test_xdp(void)
 	bpf_map_update_elem(map_fd, &key4, &value4, 0);
 	bpf_map_update_elem(map_fd, &key6, &value6, 0);
 
+	size = sizeof(buf);
 	err = bpf_prog_test_run(prog_fd, 1, &pkt_v4, sizeof(pkt_v4),
 				buf, &size, &retval, &duration);
 
@@ -158,6 +159,7 @@ static void test_xdp(void)
 	      "err %d errno %d retval %d size %d\n",
 	      err, errno, retval, size);
 
+	size = sizeof(buf);
 	err = bpf_prog_test_run(prog_fd, 1, &pkt_v6, sizeof(pkt_v6),
 				buf, &size, &retval, &duration);
 	CHECK(err || retval != XDP_TX || size != 114 ||
@@ -182,6 +184,7 @@ static void test_xdp_adjust_tail(void)
 		return;
 	}
 
+	size = sizeof(buf);
 	err = bpf_prog_test_run(prog_fd, 1, &pkt_v4, sizeof(pkt_v4),
 				buf, &size, &retval, &duration);
 
@@ -189,6 +192,7 @@ static void test_xdp_adjust_tail(void)
 	      "ipv4", "err %d errno %d retval %d size %d\n",
 	      err, errno, retval, size);
 
+	size = sizeof(buf);
 	err = bpf_prog_test_run(prog_fd, 1, &pkt_v6, sizeof(pkt_v6),
 				buf, &size, &retval, &duration);
 	CHECK(err || retval != XDP_TX || size != 54,
@@ -252,6 +256,7 @@ static void test_l4lb(const char *file)
 		goto out;
 	bpf_map_update_elem(map_fd, &real_num, &real_def, 0);
 
+	size = sizeof(buf);
 	err = bpf_prog_test_run(prog_fd, NUM_ITER, &pkt_v4, sizeof(pkt_v4),
 				buf, &size, &retval, &duration);
 	CHECK(err || retval != 7/*TC_ACT_REDIRECT*/ || size != 54 ||
@@ -259,6 +264,7 @@ static void test_l4lb(const char *file)
 	      "err %d errno %d retval %d size %d magic %x\n",
 	      err, errno, retval, size, *magic);
 
+	size = sizeof(buf);
 	err = bpf_prog_test_run(prog_fd, NUM_ITER, &pkt_v6, sizeof(pkt_v6),
 				buf, &size, &retval, &duration);
 	CHECK(err || retval != 7/*TC_ACT_REDIRECT*/ || size != 74 ||
@@ -341,6 +347,7 @@ static void test_xdp_noinline(void)
 		goto out;
 	bpf_map_update_elem(map_fd, &real_num, &real_def, 0);
 
+	size = sizeof(buf);
 	err = bpf_prog_test_run(prog_fd, NUM_ITER, &pkt_v4, sizeof(pkt_v4),
 				buf, &size, &retval, &duration);
 	CHECK(err || retval != 1 || size != 54 ||
@@ -348,6 +355,7 @@ static void test_xdp_noinline(void)
 	      "err %d errno %d retval %d size %d magic %x\n",
 	      err, errno, retval, size, *magic);
 
+	size = sizeof(buf);
 	err = bpf_prog_test_run(prog_fd, NUM_ITER, &pkt_v6, sizeof(pkt_v6),
 				buf, &size, &retval, &duration);
 	CHECK(err || retval != 1 || size != 74 ||
@@ -1795,6 +1803,7 @@ static void test_queue_stack_map(int type)
 			pkt_v4.iph.saddr = vals[MAP_SIZE - 1 - i] * 5;
 		}
 
+		size = sizeof(buf);
 		err = bpf_prog_test_run(prog_fd, 1, &pkt_v4, sizeof(pkt_v4),
 					buf, &size, &retval, &duration);
 		if (err || retval || size != sizeof(pkt_v4) ||
@@ -1808,6 +1817,7 @@ static void test_queue_stack_map(int type)
 	      err, errno, retval, size, iph->daddr);
 
 	/* Queue is empty, program should return TC_ACT_SHOT */
+	size = sizeof(buf);
 	err = bpf_prog_test_run(prog_fd, 1, &pkt_v4, sizeof(pkt_v4),
 				buf, &size, &retval, &duration);
 	CHECK(err || retval != 2 /* TC_ACT_SHOT */|| size != sizeof(pkt_v4),
-- 
2.17.1

^ permalink raw reply related

* [PATCH v2 4/4] selftests: add a test for bpf_prog_test_run output size
From: Lorenz Bauer @ 2018-11-20 15:43 UTC (permalink / raw)
  To: ast, daniel; +Cc: netdev, linux-api, ys114321, Lorenz Bauer
In-Reply-To: <20181120154306.16657-1-lmb@cloudflare.com>

Make sure that bpf_prog_test_run returns the correct length
in the size_out argument and that the kernel respects the
output size hint. Also check that errno indicates ENOSPC.

Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
 tools/testing/selftests/bpf/test_progs.c | 34 ++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 299938603cb6..92e48c2ba2c6 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -124,6 +124,39 @@ static void test_pkt_access(void)
 	bpf_object__close(obj);
 }
 
+static void test_output_size_hint(void)
+{
+	const char *file = "./test_pkt_access.o";
+	struct bpf_object *obj;
+	__u32 retval, size, duration;
+	int err, prog_fd;
+	char buf[10];
+
+	err = bpf_prog_load(file, BPF_PROG_TYPE_SCHED_CLS, &obj, &prog_fd);
+	if (CHECK(err, "load", "err %d errno %d\n", err, errno))
+		return;
+
+	memset(buf, 0, sizeof(buf));
+
+	size = 5;
+	err = bpf_prog_test_run(prog_fd, 1, &pkt_v4, sizeof(pkt_v4),
+				buf, &size, &retval, &duration);
+	CHECK(err != -1 || errno != ENOSPC || retval, "run",
+	      "err %d errno %d retval %d\n",
+	      err, errno, retval);
+
+	duration = 0;
+
+	CHECK(size != sizeof(pkt_v4), "out_size",
+	      "incorrect output size, want %lu have %u\n",
+	      sizeof(pkt_v4), size);
+
+	CHECK(buf[5] != 0, "overflow",
+	      "prog_test_run ignored size hint\n");
+
+	bpf_object__close(obj);
+}
+
 static void test_xdp(void)
 {
 	struct vip key4 = {.protocol = 6, .family = AF_INET};
@@ -1847,6 +1880,7 @@ int main(void)
 	jit_enabled = is_jit_enabled();
 
 	test_pkt_access();
+	test_output_size_hint();
 	test_xdp();
 	test_xdp_adjust_tail();
 	test_l4lb_all();
-- 
2.17.1

^ 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