Netdev List
 help / color / mirror / Atom feed
* [PATCH 05/12] netfilter: conntrack: lower timeout to RETRANS seconds if window is 0
From: Pablo Neira Ayuso @ 2017-12-13 18:45 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20171213184520.8193-1-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

When zero window is announced we can get into a situation where
connection stays around forever:

1. One side announces zero window.
2. Other side closes.

In this case, no FIN is sent (stuck in send queue).

Unless other side opens the window up again conntrack
stays in ESTABLISHED state for a very long time.

Lets alleviate this by lowering the timeout to RETRANS (5 minutes),
the other end should be sending zero window probes to keep the
connection established as long as a socket still exists.

Cc: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Signed-off-by: Florian Westphal <fw@strlen.de>
Acked-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_conntrack_proto_tcp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
index b12fc07111d0..37ef35b861f2 100644
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -1039,6 +1039,9 @@ static int tcp_packet(struct nf_conn *ct,
 		 IP_CT_TCP_FLAG_DATA_UNACKNOWLEDGED &&
 		 timeouts[new_state] > timeouts[TCP_CONNTRACK_UNACK])
 		timeout = timeouts[TCP_CONNTRACK_UNACK];
+	else if (ct->proto.tcp.last_win == 0 &&
+		 timeouts[new_state] > timeouts[TCP_CONNTRACK_RETRANS])
+		timeout = timeouts[TCP_CONNTRACK_RETRANS];
 	else
 		timeout = timeouts[new_state];
 	spin_unlock_bh(&ct->lock);
-- 
2.11.0

^ permalink raw reply related

* [PATCH 06/12] netfilter: conntrack: clamp timeouts to INT_MAX
From: Pablo Neira Ayuso @ 2017-12-13 18:45 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20171213184520.8193-1-pablo@netfilter.org>

From: Jay Elliott <jelliott@arista.com>

When the conntracking code multiplies a timeout by HZ, it can overflow
from positive to negative; this causes it to instantly expire.  To
protect against this the multiplication is done in 64-bit so we can
prevent it from exceeding INT_MAX.

Signed-off-by: Jay Elliott <jelliott@arista.com>
Acked-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_conntrack_netlink.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 59c08997bfdf..66d72a8fa87f 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -1566,9 +1566,11 @@ static int ctnetlink_change_helper(struct nf_conn *ct,
 static int ctnetlink_change_timeout(struct nf_conn *ct,
 				    const struct nlattr * const cda[])
 {
-	u_int32_t timeout = ntohl(nla_get_be32(cda[CTA_TIMEOUT]));
+	u64 timeout = (u64)ntohl(nla_get_be32(cda[CTA_TIMEOUT])) * HZ;
 
-	ct->timeout = nfct_time_stamp + timeout * HZ;
+	if (timeout > INT_MAX)
+		timeout = INT_MAX;
+	ct->timeout = nfct_time_stamp + (u32)timeout;
 
 	if (test_bit(IPS_DYING_BIT, &ct->status))
 		return -ETIME;
@@ -1768,6 +1770,7 @@ ctnetlink_create_conntrack(struct net *net,
 	int err = -EINVAL;
 	struct nf_conntrack_helper *helper;
 	struct nf_conn_tstamp *tstamp;
+	u64 timeout;
 
 	ct = nf_conntrack_alloc(net, zone, otuple, rtuple, GFP_ATOMIC);
 	if (IS_ERR(ct))
@@ -1776,7 +1779,10 @@ ctnetlink_create_conntrack(struct net *net,
 	if (!cda[CTA_TIMEOUT])
 		goto err1;
 
-	ct->timeout = nfct_time_stamp + ntohl(nla_get_be32(cda[CTA_TIMEOUT])) * HZ;
+	timeout = (u64)ntohl(nla_get_be32(cda[CTA_TIMEOUT])) * HZ;
+	if (timeout > INT_MAX)
+		timeout = INT_MAX;
+	ct->timeout = (u32)timeout + nfct_time_stamp;
 
 	rcu_read_lock();
  	if (cda[CTA_HELP]) {
-- 
2.11.0

^ permalink raw reply related

* [PATCH 10/12] netfilter: ipt_CLUSTERIP: fix clusterip_net_exit build regression
From: Pablo Neira Ayuso @ 2017-12-13 18:45 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20171213184520.8193-1-pablo@netfilter.org>

From: Arnd Bergmann <arnd@arndb.de>

The added check produces a build error when CONFIG_PROC_FS is
disabled:

net/ipv4/netfilter/ipt_CLUSTERIP.c: In function 'clusterip_net_exit':
net/ipv4/netfilter/ipt_CLUSTERIP.c:822:28: error: 'cn' undeclared (first use in this function)

This moves the variable declaration out of the #ifdef to make it
available to the WARN_ON_ONCE().

Fixes: 613d0776d3fe ("netfilter: exit_net cleanup check added")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Vasily Averin <vvs@virtuozzo.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/ipv4/netfilter/ipt_CLUSTERIP.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c b/net/ipv4/netfilter/ipt_CLUSTERIP.c
index e35b8d074f06..69060e3abe85 100644
--- a/net/ipv4/netfilter/ipt_CLUSTERIP.c
+++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c
@@ -813,8 +813,8 @@ static int clusterip_net_init(struct net *net)
 
 static void clusterip_net_exit(struct net *net)
 {
-#ifdef CONFIG_PROC_FS
 	struct clusterip_net *cn = net_generic(net, clusterip_net_id);
+#ifdef CONFIG_PROC_FS
 	proc_remove(cn->procdir);
 	cn->procdir = NULL;
 #endif
-- 
2.11.0

^ permalink raw reply related

* [PATCH 12/12] netfilter: ip6t_MASQUERADE: add dependency on conntrack module
From: Pablo Neira Ayuso @ 2017-12-13 18:45 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20171213184520.8193-1-pablo@netfilter.org>

From: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>

After commit 4d3a57f23dec ("netfilter: conntrack: do not enable connection
tracking unless needed") conntrack is disabled by default unless some
module explicitly declares dependency in particular network namespace.

Fixes: a357b3f80bc8 ("netfilter: nat: add dependencies on conntrack module")
Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/ipv6/netfilter/ip6t_MASQUERADE.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/netfilter/ip6t_MASQUERADE.c b/net/ipv6/netfilter/ip6t_MASQUERADE.c
index 2b1a15846f9a..92c0047e7e33 100644
--- a/net/ipv6/netfilter/ip6t_MASQUERADE.c
+++ b/net/ipv6/netfilter/ip6t_MASQUERADE.c
@@ -33,13 +33,19 @@ static int masquerade_tg6_checkentry(const struct xt_tgchk_param *par)
 
 	if (range->flags & NF_NAT_RANGE_MAP_IPS)
 		return -EINVAL;
-	return 0;
+	return nf_ct_netns_get(par->net, par->family);
+}
+
+static void masquerade_tg6_destroy(const struct xt_tgdtor_param *par)
+{
+	nf_ct_netns_put(par->net, par->family);
 }
 
 static struct xt_target masquerade_tg6_reg __read_mostly = {
 	.name		= "MASQUERADE",
 	.family		= NFPROTO_IPV6,
 	.checkentry	= masquerade_tg6_checkentry,
+	.destroy	= masquerade_tg6_destroy,
 	.target		= masquerade_tg6,
 	.targetsize	= sizeof(struct nf_nat_range),
 	.table		= "nat",
-- 
2.11.0

^ permalink raw reply related

* [PATCH 09/12] netfilter: xt_osf: Add missing permission checks
From: Pablo Neira Ayuso @ 2017-12-13 18:45 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20171213184520.8193-1-pablo@netfilter.org>

From: Kevin Cernekee <cernekee@chromium.org>

The capability check in nfnetlink_rcv() verifies that the caller
has CAP_NET_ADMIN in the namespace that "owns" the netlink socket.
However, xt_osf_fingers is shared by all net namespaces on the
system.  An unprivileged user can create user and net namespaces
in which he holds CAP_NET_ADMIN to bypass the netlink_net_capable()
check:

    vpnns -- nfnl_osf -f /tmp/pf.os

    vpnns -- nfnl_osf -f /tmp/pf.os -d

These non-root operations successfully modify the systemwide OS
fingerprint list.  Add new capable() checks so that they can't.

Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/xt_osf.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/netfilter/xt_osf.c b/net/netfilter/xt_osf.c
index 36e14b1f061d..a34f314a8c23 100644
--- a/net/netfilter/xt_osf.c
+++ b/net/netfilter/xt_osf.c
@@ -19,6 +19,7 @@
 #include <linux/module.h>
 #include <linux/kernel.h>
 
+#include <linux/capability.h>
 #include <linux/if.h>
 #include <linux/inetdevice.h>
 #include <linux/ip.h>
@@ -70,6 +71,9 @@ static int xt_osf_add_callback(struct net *net, struct sock *ctnl,
 	struct xt_osf_finger *kf = NULL, *sf;
 	int err = 0;
 
+	if (!capable(CAP_NET_ADMIN))
+		return -EPERM;
+
 	if (!osf_attrs[OSF_ATTR_FINGER])
 		return -EINVAL;
 
@@ -115,6 +119,9 @@ static int xt_osf_remove_callback(struct net *net, struct sock *ctnl,
 	struct xt_osf_finger *sf;
 	int err = -ENOENT;
 
+	if (!capable(CAP_NET_ADMIN))
+		return -EPERM;
+
 	if (!osf_attrs[OSF_ATTR_FINGER])
 		return -EINVAL;
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH 11/12] netfilter: exthdr: add missign attributes to policy
From: Pablo Neira Ayuso @ 2017-12-13 18:45 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20171213184520.8193-1-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

Add missing netlink attribute policy.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_exthdr.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/netfilter/nft_exthdr.c b/net/netfilter/nft_exthdr.c
index a0a93d987a3b..47ec1046ad11 100644
--- a/net/netfilter/nft_exthdr.c
+++ b/net/netfilter/nft_exthdr.c
@@ -214,6 +214,8 @@ static const struct nla_policy nft_exthdr_policy[NFTA_EXTHDR_MAX + 1] = {
 	[NFTA_EXTHDR_OFFSET]		= { .type = NLA_U32 },
 	[NFTA_EXTHDR_LEN]		= { .type = NLA_U32 },
 	[NFTA_EXTHDR_FLAGS]		= { .type = NLA_U32 },
+	[NFTA_EXTHDR_OP]		= { .type = NLA_U32 },
+	[NFTA_EXTHDR_SREG]		= { .type = NLA_U32 },
 };
 
 static int nft_exthdr_init(const struct nft_ctx *ctx,
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH] net/tls: Fix inverted error codes to avoid endless loop
From: David Miller @ 2017-12-13 18:48 UTC (permalink / raw)
  To: r.hering; +Cc: netdev
In-Reply-To: <OF6F0DE70F.1C59BE55-ONC12581F3.0066F1BE-C12581F3.0068093A@avm.de>

From: r.hering@avm.de
Date: Mon, 11 Dec 2017 19:56:21 +0100

> sendfile() calls can hang endless with using Kernel TLS if a socket error 
> occurs.
> Socket error codes must be inverted by Kernel TLS before returning because
> they are stored with positive sign. If returned non-inverted they are
> interpreted as number of bytes sent, causing endless looping of the
> splice mechanic behind sendfile().
> 
> Signed-off-by: Robert Hering <r.hering@avm.de>

Your patch is corrupted by your email client, it turned TAB characters
into spaces.

Please read Documentation/email-clients.txt to fix this.

Send a test patch to yourself, and make sure you can actually apply
the patch contained in that test email.

Do not repost the patch here until you can get such a test patch
to work properly.

Thank you.

^ permalink raw reply

* Re: [PATCH] net: igmp: Use correct source address on IGMPv3 reports
From: David Miller @ 2017-12-13 18:52 UTC (permalink / raw)
  To: cernekee; +Cc: kuznet, yoshfuji, netdev, andrew, linux-kernel
In-Reply-To: <20171211191345.104136-1-cernekee@chromium.org>

From: Kevin Cernekee <cernekee@chromium.org>
Date: Mon, 11 Dec 2017 11:13:45 -0800

> Closing a multicast socket after the final IPv4 address is deleted
> from an interface can generate a membership report that uses the
> source IP from a different interface.  The following test script, run
> from an isolated netns, reproduces the issue:
> 
>     #!/bin/bash
> 
>     ip link add dummy0 type dummy
>     ip link add dummy1 type dummy
>     ip link set dummy0 up
>     ip link set dummy1 up
>     ip addr add 10.1.1.1/24 dev dummy0
>     ip addr add 192.168.99.99/24 dev dummy1
> 
>     tcpdump -U -i dummy0 &
>     socat EXEC:"sleep 2" \
>         UDP4-DATAGRAM:239.101.1.68:8889,ip-add-membership=239.0.1.68:10.1.1.1 &
> 
>     sleep 1
>     ip addr del 10.1.1.1/24 dev dummy0
>     sleep 5
>     kill %tcpdump
> 
> RFC 3376 specifies that the report must be sent with a valid IP source
> address from the destination subnet, or from address 0.0.0.0.  Add an
> extra check to make sure this is the case.
> 
> Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
...
> +	return htonl(INADDR_ANY);
> +}
> +
>  static struct sk_buff *igmpv3_newpack(struct net_device *dev, unsigned int mtu)
>  {
>  	struct sk_buff *skb;
> @@ -368,7 +386,7 @@ static struct sk_buff *igmpv3_newpack(struct net_device *dev, unsigned int mtu)
>  	pip->frag_off = htons(IP_DF);
>  	pip->ttl      = 1;
>  	pip->daddr    = fl4.daddr;
> -	pip->saddr    = fl4.saddr;
> +	pip->saddr    = igmpv3_get_srcaddr(dev, &fl4);
>  	pip->protocol = IPPROTO_IGMP;
>  	pip->tot_len  = 0;	/* filled in later */
>  	ip_select_ident(net, skb, NULL);

Ok, and I checked that MC source address validation on the receiver
will pass because:

	if (ipv4_is_zeronet(saddr)) {
		if (!ipv4_is_local_multicast(daddr))
			return -EINVAL;

that check in ip_mc_validate_source() won't trigger.

^ permalink raw reply

* Re: net: phy: consolidate PHY reset in phy_init_hw()
From: Daniel Walker @ 2017-12-13 18:46 UTC (permalink / raw)
  To: Florian Fainelli, David S. Miller, netdev@vger.kernel.org


Hi,

https://sjc-apl-grt32.cisco.com:8081/gitweb?p=xe-linux/kernel.git;a=commit;h=87aa9f9c61ad56d505641681812e92ad976f8608

I've got a machine with an Marvell 88EE1112 , and it seems this patch 
causes a problem. When the reset happens under this code this phy always 
returns -ETIMEDOUT from inside phy_poll_reset() .. It would seem this 
phy doesn't fit whatever standard this code is expecting. I tried to 
increase the wait time, but it doesn't seems to change anything.

Any thoughts?

Daniel

^ permalink raw reply

* Re: [PATCH net,stable] net: qmi_wwan: add Sierra EM7565 1199:9091
From: David Miller @ 2017-12-13 18:57 UTC (permalink / raw)
  To: ssjoholm; +Cc: netdev, linux-usb, linux-kernel, bjorn, rspmn
In-Reply-To: <20171211205114.85097-1-ssjoholm@mac.com>

From: ssjoholm@mac.com
Date: Mon, 11 Dec 2017 21:51:14 +0100

> From: Sebastian Sjoholm <ssjoholm@mac.com>
> 
> Sierra Wireless EM7565 is an Qualcomm MDM9x50 based M.2 modem.
> The USB id is added to qmi_wwan.c to allow QMI communication 
> with the EM7565.
> 
> Signed-off-by: Sebastian Sjoholm <ssjoholm@mac.com>
> Acked-by: Bjørn Mork <bjorn@mork.no>

Applied and queued up for -stable.

^ permalink raw reply

* Re: [PATCH v9 0/5] Add the ability to do BPF directed error injection
From: Josef Bacik @ 2017-12-13 18:57 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: Josef Bacik, Masami Hiramatsu, rostedt, mingo, davem, netdev,
	linux-kernel, ast, kernel-team, daniel, linux-btrfs
In-Reply-To: <20171213180732.GE13436@magnolia>

On Wed, Dec 13, 2017 at 10:07:32AM -0800, Darrick J. Wong wrote:
> On Wed, Dec 13, 2017 at 01:03:57PM -0500, Josef Bacik wrote:
> > On Tue, Dec 12, 2017 at 03:11:50PM -0800, Darrick J. Wong wrote:
> > > On Mon, Dec 11, 2017 at 11:36:45AM -0500, Josef Bacik wrote:
> > > > This is the same as v8, just rebased onto the bpf tree.
> > > > 
> > > > v8->v9:
> > > > - rebased onto the bpf tree.
> > > > 
> > > > v7->v8:
> > > > - removed the _ASM_KPROBE_ERROR_INJECT since it was not needed.
> > > > 
> > > > v6->v7:
> > > > - moved the opt-in macro to bpf.h out of kprobes.h.
> > > > 
> > > > v5->v6:
> > > > - add BPF_ALLOW_ERROR_INJECTION() tagging for functions that will support this
> > > >   feature.  This way only functions that opt-in will be allowed to be
> > > >   overridden.
> > > > - added a btrfs patch to allow error injection for open_ctree() so that the bpf
> > > >   sample actually works.
> > > > 
> > > > v4->v5:
> > > > - disallow kprobe_override programs from being put in the prog map array so we
> > > >   don't tail call into something we didn't check.  This allows us to make the
> > > >   normal path still fast without a bunch of percpu operations.
> > > > 
> > > > v3->v4:
> > > > - fix a build error found by kbuild test bot (I didn't wait long enough
> > > >   apparently.)
> > > > - Added a warning message as per Daniels suggestion.
> > > > 
> > > > v2->v3:
> > > > - added a ->kprobe_override flag to bpf_prog.
> > > > - added some sanity checks to disallow attaching bpf progs that have
> > > >   ->kprobe_override set that aren't for ftrace kprobes.
> > > > - added the trace_kprobe_ftrace helper to check if the trace_event_call is a
> > > >   ftrace kprobe.
> > > > - renamed bpf_kprobe_state to bpf_kprobe_override, fixed it so we only read this
> > > >   value in the kprobe path, and thus only write to it if we're overriding or
> > > >   clearing the override.
> > > > 
> > > > v1->v2:
> > > > - moved things around to make sure that bpf_override_return could really only be
> > > >   used for an ftrace kprobe.
> > > > - killed the special return values from trace_call_bpf.
> > > > - renamed pc_modified to bpf_kprobe_state so bpf_override_return could tell if
> > > >   it was being called from an ftrace kprobe context.
> > > > - reworked the logic in kprobe_perf_func to take advantage of bpf_kprobe_state.
> > > > - updated the test as per Alexei's review.
> > > > 
> > > > - Original message -
> > > > 
> > > > A lot of our error paths are not well tested because we have no good way of
> > > > injecting errors generically.  Some subystems (block, memory) have ways to
> > > > inject errors, but they are random so it's hard to get reproduceable results.
> > > > 
> > > > With BPF we can add determinism to our error injection.  We can use kprobes and
> > > > other things to verify we are injecting errors at the exact case we are trying
> > > > to test.  This patch gives us the tool to actual do the error injection part.
> > > > It is very simple, we just set the return value of the pt_regs we're given to
> > > > whatever we provide, and then override the PC with a dummy function that simply
> > > > returns.
> > > 
> > > Heh, this looks cool.  I decided to try it to see what happens, and saw
> > > a bunch of dmesg pasted in below.  Is that supposed to happen?  Or am I
> > > the only fs developer still running with lockdep enabled? :)
> > > 
> > > It looks like bpf_override_return has some sort of side effect such that
> > > we get the splat, since commenting it out makes the symptom go away.
> > > 
> > > <shrug>
> > > 
> > > --D
> > > 
> > > [ 1847.769183] BTRFS error (device (null)): open_ctree failed
> > > [ 1847.770130] BUG: sleeping function called from invalid context at /storage/home/djwong/cdev/work/linux-xfs/kernel/locking/rwsem.c:69
> > > [ 1847.771976] in_atomic(): 1, irqs_disabled(): 0, pid: 1524, name: mount
> > > [ 1847.773016] 1 lock held by mount/1524:
> > > [ 1847.773530]  #0:  (&type->s_umount_key#34/1){+.+.}, at: [<00000000653a9bb4>] sget_userns+0x302/0x4f0
> > > [ 1847.774731] Preemption disabled at:
> > > [ 1847.774735] [<          (null)>]           (null)
> > > [ 1847.777009] CPU: 2 PID: 1524 Comm: mount Tainted: G        W        4.15.0-rc3-xfsx #3
> > > [ 1847.778800] Call Trace:
> > > [ 1847.779047]  dump_stack+0x7c/0xbe
> > > [ 1847.779361]  ___might_sleep+0x1f7/0x260
> > > [ 1847.779720]  down_write+0x29/0xb0
> > > [ 1847.780046]  unregister_shrinker+0x15/0x70
> > > [ 1847.780427]  deactivate_locked_super+0x2e/0x60
> > > [ 1847.780935]  btrfs_mount+0xbb6/0x1000 [btrfs]
> > > [ 1847.781353]  ? __lockdep_init_map+0x5c/0x1d0
> > > [ 1847.781750]  ? mount_fs+0xf/0x80
> > > [ 1847.782065]  ? alloc_vfsmnt+0x1a1/0x230
> > > [ 1847.782429]  mount_fs+0xf/0x80
> > > [ 1847.782733]  vfs_kern_mount+0x62/0x160
> > > [ 1847.783128]  btrfs_mount+0x3d3/0x1000 [btrfs]
> > > [ 1847.783493]  ? __lockdep_init_map+0x5c/0x1d0
> > > [ 1847.783849]  ? __lockdep_init_map+0x5c/0x1d0
> > > [ 1847.784207]  ? mount_fs+0xf/0x80
> > > [ 1847.784502]  mount_fs+0xf/0x80
> > > [ 1847.784835]  vfs_kern_mount+0x62/0x160
> > > [ 1847.785235]  do_mount+0x1b1/0xd50
> > > [ 1847.785594]  ? _copy_from_user+0x5b/0x90
> > > [ 1847.786028]  ? memdup_user+0x4b/0x70
> > > [ 1847.786501]  SyS_mount+0x85/0xd0
> > > [ 1847.786835]  entry_SYSCALL_64_fastpath+0x1f/0x96
> > > [ 1847.787311] RIP: 0033:0x7f6ebecc1b5a
> > > [ 1847.787691] RSP: 002b:00007ffc7bd1c958 EFLAGS: 00000202 ORIG_RAX: 00000000000000a5
> > > [ 1847.788383] RAX: ffffffffffffffda RBX: 00007f6ebefba63a RCX: 00007f6ebecc1b5a
> > > [ 1847.789106] RDX: 0000000000bfd010 RSI: 0000000000bfa230 RDI: 0000000000bfa210
> > > [ 1847.789807] RBP: 0000000000bfa0f0 R08: 0000000000000000 R09: 0000000000000014
> > > [ 1847.790511] R10: 00000000c0ed0000 R11: 0000000000000202 R12: 00007f6ebf1ca83c
> > > [ 1847.791211] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000001
> > > [ 1847.792029] BUG: scheduling while atomic: mount/1524/0x00000002
> > > [ 1847.792680] 1 lock held by mount/1524:
> > > [ 1847.793087]  #0:  (rcu_preempt_state.exp_mutex){+.+.}, at: [<00000000a6c536a9>] _synchronize_rcu_expedited+0x1ce/0x400
> > > [ 1847.794129] Modules linked in: xfs libcrc32c btrfs xor zstd_decompress zstd_compress xxhash lzo_compress lzo_decompress zlib_deflate raid6_pq dax_pmem device_dax nd_pmem sch_fq_codel af_packet [last unloaded: xfs]
> > > [ 1847.795949] Preemption disabled at:
> > > [ 1847.795951] [<          (null)>]           (null)
> > > [ 1847.796844] CPU: 2 PID: 1524 Comm: mount Tainted: G        W        4.15.0-rc3-xfsx #3
> > > [ 1847.797621] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.10.2-1ubuntu1djwong0 04/01/2014
> > > [ 1847.798510] Call Trace:
> > > [ 1847.798786]  dump_stack+0x7c/0xbe
> > > [ 1847.799134]  __schedule_bug+0x88/0xe0
> > > [ 1847.799517]  __schedule+0x78c/0xb20
> > > [ 1847.799890]  ? trace_hardirqs_on_caller+0x119/0x180
> > > [ 1847.800391]  schedule+0x40/0x90
> > > [ 1847.800729]  _synchronize_rcu_expedited+0x36b/0x400
> > > [ 1847.801218]  ? rcu_preempt_qs+0xa0/0xa0
> > > [ 1847.801616]  ? remove_wait_queue+0x60/0x60
> > > [ 1847.802040]  ? rcu_preempt_qs+0xa0/0xa0
> > > [ 1847.802433]  ? rcu_exp_wait_wake+0x630/0x630
> > > [ 1847.802872]  ? __lock_acquire+0xfb9/0x1120
> > > [ 1847.803302]  ? __lock_acquire+0x534/0x1120
> > > [ 1847.803725]  ? bdi_unregister+0x57/0x1a0
> > > [ 1847.804135]  bdi_unregister+0x5c/0x1a0
> > > [ 1847.804519]  bdi_put+0xcb/0xe0
> > > [ 1847.804746]  generic_shutdown_super+0xe2/0x110
> > > [ 1847.805066]  kill_anon_super+0xe/0x20
> > > [ 1847.805344]  btrfs_kill_super+0x12/0xa0 [btrfs]
> > > [ 1847.805664]  deactivate_locked_super+0x34/0x60
> > > [ 1847.806111]  btrfs_mount+0xbb6/0x1000 [btrfs]
> > > [ 1847.806476]  ? __lockdep_init_map+0x5c/0x1d0
> > > [ 1847.806824]  ? mount_fs+0xf/0x80
> > > [ 1847.807104]  ? alloc_vfsmnt+0x1a1/0x230
> > > [ 1847.807416]  mount_fs+0xf/0x80
> > > [ 1847.807712]  vfs_kern_mount+0x62/0x160
> > > [ 1847.808112]  btrfs_mount+0x3d3/0x1000 [btrfs]
> > > [ 1847.808565]  ? __lockdep_init_map+0x5c/0x1d0
> > > [ 1847.809005]  ? __lockdep_init_map+0x5c/0x1d0
> > > [ 1847.809425]  ? mount_fs+0xf/0x80
> > > [ 1847.809731]  mount_fs+0xf/0x80
> > > [ 1847.810070]  vfs_kern_mount+0x62/0x160
> > > [ 1847.810469]  do_mount+0x1b1/0xd50
> > > [ 1847.810821]  ? _copy_from_user+0x5b/0x90
> > > [ 1847.811237]  ? memdup_user+0x4b/0x70
> > > [ 1847.811622]  SyS_mount+0x85/0xd0
> > > [ 1847.811996]  entry_SYSCALL_64_fastpath+0x1f/0x96
> > > [ 1847.812465] RIP: 0033:0x7f6ebecc1b5a
> > > [ 1847.812840] RSP: 002b:00007ffc7bd1c958 EFLAGS: 00000202 ORIG_RAX: 00000000000000a5
> > > [ 1847.813615] RAX: ffffffffffffffda RBX: 00007f6ebefba63a RCX: 00007f6ebecc1b5a
> > > [ 1847.814302] RDX: 0000000000bfd010 RSI: 0000000000bfa230 RDI: 0000000000bfa210
> > > [ 1847.814770] RBP: 0000000000bfa0f0 R08: 0000000000000000 R09: 0000000000000014
> > > [ 1847.815246] R10: 00000000c0ed0000 R11: 0000000000000202 R12: 00007f6ebf1ca83c
> > > [ 1847.815720] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000001
> > i> 
> > 
> > Looks like this is new, Masami this is happening because of your change here
> > 
> > 5bb4fc2d8641 ("kprobes/x86: Disable preemption in ftrace-based jprobes")
> > 
> > which makes it not do the preempt_enable() if the handler returns 1.  Why is
> > that?  Should I be doing preempt_enable_no_resched() from the handler before
> > returning 1?  Or is this just an oversight on your part?  Thanks,
> 
> FWIW I shut up the preemption imbalance warnings with the attached
> coarse bandaid.  No idea if that's the correct fix...
> 
> --D
> 
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 5db8498..fd948e3 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -1215,8 +1215,10 @@ kprobe_perf_func(struct trace_kprobe *tk, struct pt_regs *regs)
>  		if (__this_cpu_read(bpf_kprobe_override)) {
>  			__this_cpu_write(bpf_kprobe_override, 0);
>  			reset_current_kprobe();
> +			preempt_enable();
>  			return 1;
>  		}
> +		preempt_enable();
>  		if (!ret)
>  			return 0;
>  	}

Yeah I'd like to avoid doing this and know why exactly we leave a unpaired
preempt_disable() in kprobe_ftrace_handler() so we don't do something like this
only to have the handler change again in the future and break us again.  Thanks,

Josef

^ permalink raw reply

* Re: [PATCH v3] net: ethernet: arc: fix error handling in emac_rockchip_probe
From: David Miller @ 2017-12-13 18:58 UTC (permalink / raw)
  To: branislav; +Cc: heiko, netdev, linux-arm-kernel, linux-rockchip, linux-kernel
In-Reply-To: <20171211231338.5207-1-branislav@radocaj.org>

From: Branislav Radocaj <branislav@radocaj.org>
Date: Tue, 12 Dec 2017 00:13:38 +0100

> If clk_set_rate() fails, we should disable clk before return.
> Found by Linux Driver Verification project (linuxtesting.org).
> 
> Changes since v2 [1]:
> * Merged with latest code changes
> 
> Changes since v1:
> Update made thanks to David's review, much appreciated David.
> * Improved inconsistent failure handling of clock rate setting
> * For completeness of usecase, added arc_emac_probe error handling
> 
> Signed-off-by: Branislav Radocaj <branislav@radocaj.org>

Applied.

^ permalink raw reply

* Re: [Patch net-next] net_sched: switch to exit_batch for action pernet ops
From: David Miller @ 2017-12-13 18:58 UTC (permalink / raw)
  To: xiyou.wangcong; +Cc: netdev, jhs, jiri
In-Reply-To: <20171211233503.28043-1-xiyou.wangcong@gmail.com>

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Mon, 11 Dec 2017 15:35:03 -0800

> Since we now hold RTNL lock in tc_action_net_exit(), it is good to
> batch them to speedup tc action dismantle.
> 
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Cc: Jiri Pirko <jiri@resnulli.us>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next] tcp: allow TLP in ECN CWR
From: David Miller @ 2017-12-13 18:59 UTC (permalink / raw)
  To: ycheng; +Cc: netdev, ncardwell, edumazet, nanditad, sibanez
In-Reply-To: <20171211234253.102924-1-ycheng@google.com>

From: Yuchung Cheng <ycheng@google.com>
Date: Mon, 11 Dec 2017 15:42:53 -0800

> From: Neal Cardwell <ncardwell@google.com>
> 
> This patch enables tail loss probe in cwnd reduction (CWR) state
> to detect potential losses. Prior to this patch, since the sender
> uses PRR to determine the cwnd in CWR state, the combination of
> CWR+PRR plus tcp_tso_should_defer() could cause unnecessary stalls
> upon losses: PRR makes cwnd so gentle that tcp_tso_should_defer()
> defers sending wait for more ACKs. The ACKs may not come due to
> packet losses.
> 
> Disallowing TLP when there is unused cwnd had the primary effect
> of disallowing TLP when there is TSO deferral, Nagle deferral,
> or we hit the rwin limit. Because basically every application
> write() or incoming ACK will cause us to run tcp_write_xmit()
> to see if we can send more, and then if we sent something we call
> tcp_schedule_loss_probe() to see if we should schedule a TLP. At
> that point, there are a few common reasons why some cwnd budget
> could still be unused:
> 
> (a) rwin limit
> (b) nagle check
> (c) TSO deferral
> (d) TSQ
> 
> For (d), after the next packet tx completion the TSQ mechanism
> will allow us to send more packets, so we don't really need a
> TLP (in practice it shouldn't matter whether we schedule one
> or not). But for (a), (b), (c) the sender won't send any more
> packets until it gets another ACK. But if the whole flight was
> lost, or all the ACKs were lost, then we won't get any more ACKs,
> and ideally we should schedule and send a TLP to get more feedback.
> In particular for a long time we have wanted some kind of timer for
> TSO deferral, and at least this would give us some kind of timer
> 
> Reported-by: Steve Ibanez <sibanez@stanford.edu>
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
> Signed-off-by: Yuchung Cheng <ycheng@google.com>
> Reviewed-by: Nandita Dukkipati <nanditad@google.com>
> Reviewed-by: Eric Dumazet <edumazet@google.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next v3 0/6] net: qualcomm: rmnet: Configuration options
From: David Miller @ 2017-12-13 19:01 UTC (permalink / raw)
  To: subashab; +Cc: netdev
In-Reply-To: <1513038615-16407-1-git-send-email-subashab@codeaurora.org>

From: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
Date: Mon, 11 Dec 2017 17:30:09 -0700

> This series adds support for configuring features on rmnet devices.
> The rmnet specific features to be configured here are aggregation and
> control commands.
> 
> Patch 1 is a cleanup of return codes in the transmit path.
> Patch 2 removes some redundant ingress and egress macros.
> Patch 3 restricts the creation of rmnet dev to one dev per mux id for a
> given real dev.
> Patch 4 adds ethernet data path support.
> Patches 5-6 add support for configuring features on new and existing
> rmnet devices.
> 
> v1->v2:
> The memory leak fixed as part of patch 1 is merged seperately as
> a896d94abd2c ("net: qualcomm: rmnet: Fix leak on transmit failure"). 
> Fix a use after free in patch 4 if a packet with headroom lesser than ethernet
> header length is received.
> 
> v2->v3: 
> Fix formatting problem in patch 5 in the return statement.

Series applied, thanks.

^ permalink raw reply

* Re: [PATCH] ipv6: ip6mr: Recalc UDP checksum before forwarding
From: Brendan McGrath @ 2017-12-13 19:02 UTC (permalink / raw)
  To: Eric Dumazet, David S . Miller, netdev, linux-kernel
In-Reply-To: <1513187564.25033.65.camel@gmail.com>

I should clarify that the packet being forwarded originated on the 
Virtual Interface (i.e. it wasn't received on it). When data is received 
on the Virtual Interface (i.e. sent by a virtual host) then ip_summed is 
CHECKSUM_PARTIAL and the checksum is calculated before transmission on 
the wire.

The scenario I was testing (which caused the csum error) was to:
a) send multicast data to virtual devices over the Virtual Interface; 
and then
b) add an MFC to forward traffic from the Virtual Interface to a 
Physical Interface (so one of the machines on the physical network could 
receive it).

The packet was accepted by the Virtual devices (even though tcpdump 
shows an invalid CRC) but rejected by the Physical devices. My 
assumption here was there is some kind of optimisation (for a 
virtualised Linux kernel on the same host) but I didn't find the code to 
verify that assumption.

My tests and findings were:
MR = Multicast Router
VM = Virtual Machine (connected to the MR via a virtual switch [virbr0])
PH = Physical Machine (connected via a physical switch [br0])

The asterisk indicates where packet originated.
The value of ip_summed is being checked by a netfilter hook registered 
to NF_INET_FORWARD.
The CRC value was checked by tcpdump on the receiving interface.
"Packet accepted" indicates that the application received the packet.

Scenario 1:
VM <------* MR --------> PH

VM CRC: Incorrect (packet accepted)
PH CRC: Incorrect (packet rejected)
ip_summed = 1 (CHECKSUM_UNNECESSARY)


Scenario 2:
VM *------> MR --------> PH

MR(br0) CRC: Incorrect (packet accepted)
PH CRC: Correct (packet accepted)
ip_summed = 3 (CHECKSUM_PARTIAL)


Scenario 3:
VM <------- MR *-------> PH

VM CRC: Correct (packet accepted)
PH CRC: Correct (packet accepted)
ip_summed = 1 (CHECKSUM_UNNECESSARY)


On 14/12/17 04:52, Eric Dumazet wrote:
> On Wed, 2017-12-13 at 22:20 +1100, Brendan McGrath wrote:
>> Currently, when forwarding from a Virtual Interface to a Physical
>> Interface, ip_summed is set to a value of CHECKSUM_UNNECESSARY and
>> the UDP checksum has not been calculated.
>>
> This seems a bug then ?
> CHECKSUM_UNNECESSARY means checksum has been validated.
> Not that we want it being computed later in the stack.
>
> If we force a checksum here, what guarantee do we have packet was not
> corrupted before we do this ?
>

^ permalink raw reply

* Re: [RFC][PATCH] new byteorder primitives - ..._{replace,get}_bits()
From: Jakub Kicinski @ 2017-12-13 19:04 UTC (permalink / raw)
  To: Al Viro; +Cc: Linus Torvalds, netdev, linux-kernel
In-Reply-To: <20171213142212.GD21978@ZenIV.linux.org.uk>

On Wed, 13 Dec 2017 14:22:12 +0000, Al Viro wrote:
> IMO it's not worth the trouble; let's go with the check inside of
> static inline and accept that on clang builds it'll do nothing.

Ack, thanks for investigating!

^ permalink raw reply

* Re: BUG REPORT: iproute2 seems to have bug with dsfield/tos in ip-rule and ip-route
From: Daniel Lakeland @ 2017-12-13 19:05 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20171213101259.65652da6@xeon-e3>

Following up my previous email with output from the machine:
Note that like some of those other people I was able to get ip rule to 
accept tos values with just low order bits set

Here is example of how ip rule accepts low order dsfield bits but not modern DSCP type bits, also including some version info

dlakelan@pingpong:~$ sudo ip rule add dsfield 0x0c table 100
dlakelan@pingpong:~$ ip rule show
0:	from all lookup local
32765:	from all tos 0x0c lookup 100
32766:	from all lookup main
32767:	from all lookup default


dlakelan@pingpong:~$ sudo ip rule add dsfield 0xc0 table 100
RTNETLINK answers: Invalid argument


dlakelan@pingpong:~$ cat /proc/version
Linux version 4.12.0-1-amd64 (debian-kernel@lists.debian.org) (gcc version 6.4.0 20170805 (Debian 6.4.0-3) ) #1 SMP Debian 4.12.6-1 (2017-08-12)


dlakelan@pingpong:~$ ip -V
ip utility, iproute2-ss160518

^ permalink raw reply

* Re: [PATCH net-next] net: llc: remove init_net check
From: Alexander Aring @ 2017-12-13 19:09 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, Jamal Hadi Salim, netdev
In-Reply-To: <1512072766.19682.27.camel@gmail.com>

Hi,

On Thu, Nov 30, 2017 at 3:12 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
...
>
>
> Well, we use different netns for isolation.
>
> You need more changes than simply removing this check, I guess.
>
> __llc_sap_find() would need a per netns list, or proper netns checks.
>

I looked deeper into this and try to move the list from global
resource to net struct and use per netns init... it's just a very big
task and I try to do my best there...
Also I figured out that the bridge code use a lot of global resources
which should also be per netns as well and I am worried that you can
actually move bridges to different namespaces. :-/

Just a status update - when I have time I try to do that... just need
time and it's not simple to do this change in llc.

- Alex

^ permalink raw reply

* Re: [PATCH 00/12] Netfilter fixes for net
From: David Miller @ 2017-12-13 19:13 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <20171213184520.8193-1-pablo@netfilter.org>

From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Wed, 13 Dec 2017 19:45:08 +0100

> The follow patchset contains Netfilter fixes for your net tree,
> they are:
 ...
> You can pull these changes from:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git

Pulled, but the attention to detail in these patches could be improved.

There were several obvious typos in the commit messages in this series.

^ permalink raw reply

* Re: [PATCH v1] Bluetooth: introduce DEFINE_SHOW_ATTRIBUTE() macro
From: Marcel Holtmann @ 2017-12-13 19:21 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Johan Hedberg, linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	David S. Miller, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171122211546.5682-1-andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Hi Andy,

> This macro deduplicates a lot of similar code across the hci_debugfs.c
> module. Targeting to be moved to seq_file.h eventually.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> ---
> net/bluetooth/hci_debugfs.c | 184 +++++---------------------------------------
> 1 file changed, 18 insertions(+), 166 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel

^ permalink raw reply

* Re: [PATCH net-next] tcp/dccp: avoid one atomic operation for timewait hashdance
From: David Miller @ 2017-12-13 19:33 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1513056312.25033.49.camel@gmail.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 11 Dec 2017 21:25:12 -0800

> From: Eric Dumazet <edumazet@google.com>
> 
> First, rename __inet_twsk_hashdance() to inet_twsk_hashdance()
> 
> Then, remove one inet_twsk_put() by setting tw_refcnt to 3 instead
> of 4, but adding a fat warning that we do not have the right to access
> tw anymore after inet_twsk_hashdance()
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied, thanks Eric.

^ permalink raw reply

* [PATCH iproute2 0/3] Improve tunnel local/remote endpoint params and gre link attribute handling
From: Serhey Popovych @ 2017-12-13 19:35 UTC (permalink / raw)
  To: netdev

In this series following problems addressed:

  1) Size of IFLA_GRE_LINK attribute is 32 bits long in , not 8 bit.

  2) Use get_addr() instead of get_prefix() to parse local/remote
     tunnel endpoints as IPADDR, not PREFIX as per ip-link(8).

  3) No need to check if local/remote endpoints are zero (e.g. INADDR_ANY):
     it is fully legal value, accepted by the kernel.

See individual patch description message for details.

Thanks,
Serhii

Serhey Popovych (3):
  ip/tunnel: Unify setup and accept zero address for local/remote
    endpoints
  ip/tunnel: Use get_addr() instead of get_prefix() for local/remote
    endpoints
  ip: gre: fix IFLA_GRE_LINK attribute sizing

 ip/ip6tunnel.c   |    8 ++------
 ip/iptunnel.c    |   10 ++--------
 ip/link_gre.c    |    8 +++-----
 ip/link_gre6.c   |    8 ++------
 ip/link_ip6tnl.c |   12 ++++--------
 ip/link_iptnl.c  |   10 ++--------
 ip/link_vti.c    |   14 ++------------
 ip/link_vti6.c   |   26 ++++++++------------------
 8 files changed, 25 insertions(+), 71 deletions(-)

-- 
1.7.10.4

^ permalink raw reply

* [PATCH iproute2 1/3] ip/tunnel: Unify setup and accept zero address for local/remote endpoints
From: Serhey Popovych @ 2017-12-13 19:36 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1513193762-1580-1-git-send-email-serhe.popovych@gmail.com>

It is fully legal to submit zero (INADDR_ANY/IN6ADDR_ANY_INIT)
value for local and/or remote endpoints for all tunnel drivers:
no need additionally check this in userspace.

Note that all tunnel specific code already can pass zero address
to the kernel.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/iptunnel.c   |   10 ++--------
 ip/link_gre.c   |    6 ++----
 ip/link_iptnl.c |   10 ++--------
 ip/link_vti.c   |   14 ++------------
 ip/link_vti6.c  |   26 ++++++++------------------
 5 files changed, 16 insertions(+), 50 deletions(-)

diff --git a/ip/iptunnel.c b/ip/iptunnel.c
index 208a1f0..ce610f8 100644
--- a/ip/iptunnel.c
+++ b/ip/iptunnel.c
@@ -127,16 +127,10 @@ static int parse_args(int argc, char **argv, int cmd, struct ip_tunnel_parm *p)
 			p->iph.frag_off = htons(IP_DF);
 		} else if (strcmp(*argv, "remote") == 0) {
 			NEXT_ARG();
-			if (strcmp(*argv, "any"))
-				p->iph.daddr = get_addr32(*argv);
-			else
-				p->iph.daddr = htonl(INADDR_ANY);
+			p->iph.daddr = get_addr32(*argv);
 		} else if (strcmp(*argv, "local") == 0) {
 			NEXT_ARG();
-			if (strcmp(*argv, "any"))
-				p->iph.saddr = get_addr32(*argv);
-			else
-				p->iph.saddr = htonl(INADDR_ANY);
+			p->iph.saddr = get_addr32(*argv);
 		} else if (strcmp(*argv, "dev") == 0) {
 			NEXT_ARG();
 			medium = *argv;
diff --git a/ip/link_gre.c b/ip/link_gre.c
index 43cb1af..6f82fb4 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -251,12 +251,10 @@ get_failed:
 			pmtudisc = 1;
 		} else if (!matches(*argv, "remote")) {
 			NEXT_ARG();
-			if (strcmp(*argv, "any"))
-				daddr = get_addr32(*argv);
+			daddr = get_addr32(*argv);
 		} else if (!matches(*argv, "local")) {
 			NEXT_ARG();
-			if (strcmp(*argv, "any"))
-				saddr = get_addr32(*argv);
+			saddr = get_addr32(*argv);
 		} else if (!matches(*argv, "dev")) {
 			NEXT_ARG();
 			link = if_nametoindex(*argv);
diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
index 4940b8b..521d6ba 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -195,16 +195,10 @@ get_failed:
 	while (argc > 0) {
 		if (strcmp(*argv, "remote") == 0) {
 			NEXT_ARG();
-			if (strcmp(*argv, "any"))
-				raddr = get_addr32(*argv);
-			else
-				raddr = 0;
+			raddr = get_addr32(*argv);
 		} else if (strcmp(*argv, "local") == 0) {
 			NEXT_ARG();
-			if (strcmp(*argv, "any"))
-				laddr = get_addr32(*argv);
-			else
-				laddr = 0;
+			laddr = get_addr32(*argv);
 		} else if (matches(*argv, "dev") == 0) {
 			NEXT_ARG();
 			link = if_nametoindex(*argv);
diff --git a/ip/link_vti.c b/ip/link_vti.c
index 07ac94e..05aefa3 100644
--- a/ip/link_vti.c
+++ b/ip/link_vti.c
@@ -167,20 +167,10 @@ get_failed:
 			okey = uval;
 		} else if (!matches(*argv, "remote")) {
 			NEXT_ARG();
-			if (!strcmp(*argv, "any")) {
-				fprintf(stderr, "invalid value for \"remote\": \"%s\"\n", *argv);
-				exit(-1);
-			} else {
-				daddr = get_addr32(*argv);
-			}
+			daddr = get_addr32(*argv);
 		} else if (!matches(*argv, "local")) {
 			NEXT_ARG();
-			if (!strcmp(*argv, "any")) {
-				fprintf(stderr, "invalid value for \"local\": \"%s\"\n", *argv);
-				exit(-1);
-			} else {
-				saddr = get_addr32(*argv);
-			}
+			saddr = get_addr32(*argv);
 		} else if (!matches(*argv, "dev")) {
 			NEXT_ARG();
 			link = if_nametoindex(*argv);
diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index 6d08bfe..f665520 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -161,27 +161,17 @@ get_failed:
 			}
 			okey = uval;
 		} else if (!matches(*argv, "remote")) {
-			NEXT_ARG();
-			if (!strcmp(*argv, "any")) {
-				fprintf(stderr, "invalid value for \"remote\": \"%s\"\n", *argv);
-				exit(-1);
-			} else {
-				inet_prefix addr;
+			inet_prefix addr;
 
-				get_prefix(&addr, *argv, AF_INET6);
-				memcpy(&daddr, addr.data, addr.bytelen);
-			}
-		} else if (!matches(*argv, "local")) {
 			NEXT_ARG();
-			if (!strcmp(*argv, "any")) {
-				fprintf(stderr, "invalid value for \"local\": \"%s\"\n", *argv);
-				exit(-1);
-			} else {
-				inet_prefix addr;
+			get_prefix(&addr, *argv, AF_INET6);
+			memcpy(&daddr, addr.data, addr.bytelen);
+		} else if (!matches(*argv, "local")) {
+			inet_prefix addr;
 
-				get_prefix(&addr, *argv, AF_INET6);
-				memcpy(&saddr, addr.data, addr.bytelen);
-			}
+			NEXT_ARG();
+			get_prefix(&addr, *argv, AF_INET6);
+			memcpy(&saddr, addr.data, addr.bytelen);
 		} else if (!matches(*argv, "dev")) {
 			NEXT_ARG();
 			link = if_nametoindex(*argv);
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH iproute2 2/3] ip/tunnel: Use get_addr() instead of get_prefix() for local/remote endpoints
From: Serhey Popovych @ 2017-12-13 19:36 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1513193762-1580-1-git-send-email-serhe.popovych@gmail.com>

Manual page ip-link(8) states that both local and remote accept
IPADDR not PREFIX. Use get_addr() instead of get_prefix() to
parse local/remote endpoint address correctly.

Force corresponding address family instead of using preferred_family
to catch weired cases as shown below.

Before this patch it is possible to create tunnel with commands:

  ip    li add dev ip6gre2 type ip6gre local fe80::1/64 remote fe80::2/64
  ip -4 li add dev ip6gre2 type ip6gre local 10.0.0.1/24 remote 10.0.0.2/24

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/ip6tunnel.c   |    8 ++------
 ip/link_gre6.c   |    8 ++------
 ip/link_ip6tnl.c |   12 ++++--------
 ip/link_vti6.c   |    8 ++++----
 4 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/ip/ip6tunnel.c b/ip/ip6tunnel.c
index 4563e1e..b8db49c 100644
--- a/ip/ip6tunnel.c
+++ b/ip/ip6tunnel.c
@@ -170,17 +170,13 @@ static int parse_args(int argc, char **argv, int cmd, struct ip6_tnl_parm2 *p)
 			inet_prefix raddr;
 
 			NEXT_ARG();
-			get_prefix(&raddr, *argv, preferred_family);
-			if (raddr.family == AF_UNSPEC)
-				invarg("\"remote\" address family is AF_UNSPEC", *argv);
+			get_addr(&raddr, *argv, AF_INET6);
 			memcpy(&p->raddr, &raddr.data, sizeof(p->raddr));
 		} else if (strcmp(*argv, "local") == 0) {
 			inet_prefix laddr;
 
 			NEXT_ARG();
-			get_prefix(&laddr, *argv, preferred_family);
-			if (laddr.family == AF_UNSPEC)
-				invarg("\"local\" address family is AF_UNSPEC", *argv);
+			get_addr(&laddr, *argv, AF_INET6);
 			memcpy(&p->laddr, &laddr.data, sizeof(p->laddr));
 		} else if (strcmp(*argv, "dev") == 0) {
 			NEXT_ARG();
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 0a82eae..c22fded 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -257,17 +257,13 @@ get_failed:
 			inet_prefix addr;
 
 			NEXT_ARG();
-			get_prefix(&addr, *argv, preferred_family);
-			if (addr.family == AF_UNSPEC)
-				invarg("\"remote\" address family is AF_UNSPEC", *argv);
+			get_addr(&addr, *argv, AF_INET6);
 			memcpy(&raddr, &addr.data, sizeof(raddr));
 		} else if (!matches(*argv, "local")) {
 			inet_prefix addr;
 
 			NEXT_ARG();
-			get_prefix(&addr, *argv, preferred_family);
-			if (addr.family == AF_UNSPEC)
-				invarg("\"local\" address family is AF_UNSPEC", *argv);
+			get_addr(&addr, *argv, AF_INET6);
 			memcpy(&laddr, &addr.data, sizeof(laddr));
 		} else if (!matches(*argv, "dev")) {
 			NEXT_ARG();
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 43287ab..4ab337f 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -184,18 +184,14 @@ get_failed:
 			inet_prefix addr;
 
 			NEXT_ARG();
-			get_prefix(&addr, *argv, preferred_family);
-			if (addr.family == AF_UNSPEC)
-				invarg("\"remote\" address family is AF_UNSPEC", *argv);
-			memcpy(&raddr, addr.data, addr.bytelen);
+			get_addr(&addr, *argv, AF_INET6);
+			memcpy(&raddr, addr.data, sizeof(raddr));
 		} else if (strcmp(*argv, "local") == 0) {
 			inet_prefix addr;
 
 			NEXT_ARG();
-			get_prefix(&addr, *argv, preferred_family);
-			if (addr.family == AF_UNSPEC)
-				invarg("\"local\" address family is AF_UNSPEC", *argv);
-			memcpy(&laddr, addr.data, addr.bytelen);
+			get_addr(&addr, *argv, AF_INET6);
+			memcpy(&laddr, addr.data, sizeof(laddr));
 		} else if (matches(*argv, "dev") == 0) {
 			NEXT_ARG();
 			link = if_nametoindex(*argv);
diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index f665520..84824a5 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -164,14 +164,14 @@ get_failed:
 			inet_prefix addr;
 
 			NEXT_ARG();
-			get_prefix(&addr, *argv, AF_INET6);
-			memcpy(&daddr, addr.data, addr.bytelen);
+			get_addr(&addr, *argv, AF_INET6);
+			memcpy(&daddr, addr.data, sizeof(daddr));
 		} else if (!matches(*argv, "local")) {
 			inet_prefix addr;
 
 			NEXT_ARG();
-			get_prefix(&addr, *argv, AF_INET6);
-			memcpy(&saddr, addr.data, addr.bytelen);
+			get_addr(&addr, *argv, AF_INET6);
+			memcpy(&saddr, addr.data, sizeof(saddr));
 		} else if (!matches(*argv, "dev")) {
 			NEXT_ARG();
 			link = if_nametoindex(*argv);
-- 
1.7.10.4

^ permalink raw reply related


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