Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 1/2] net: phy: marvell10g: Let genphy_c45_pma_read_abilities set Aneg bit
From: Maxime Chevallier @ 2019-02-25 16:14 UTC (permalink / raw)
  To: davem
  Cc: Maxime Chevallier, netdev, linux-kernel, Andrew Lunn,
	Florian Fainelli, Heiner Kallweit, Russell King, linux-arm-kernel,
	Antoine Tenart, thomas.petazzoni, gregory.clement, miquel.raynal
In-Reply-To: <20190225161407.19510-1-maxime.chevallier@bootlin.com>

The genphy_c45_pma_read_abilities helper now sets the Autoneg ability
in phydev->supported according to what the AN MMD reports.

We therefore don't need to manually do that in mv3310_get_features().

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Suggested-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/marvell10g.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index 580e91deadbc..ab71da55c1e0 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -268,16 +268,6 @@ static int mv3310_get_features(struct phy_device *phydev)
 {
 	int ret, val;
 
-	if (phydev->c45_ids.devices_in_package & MDIO_DEVS_AN) {
-		val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
-		if (val < 0)
-			return val;
-
-		if (val & MDIO_AN_STAT1_ABLE)
-			linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
-					 phydev->supported);
-	}
-
 	ret = genphy_c45_pma_read_abilities(phydev);
 	if (ret)
 		return ret;
-- 
2.19.2


^ permalink raw reply related

* [PATCH net-next 2/2] net: phy: marvell10g: Use the generic C45 helper to read the 2110 features
From: Maxime Chevallier @ 2019-02-25 16:14 UTC (permalink / raw)
  To: davem
  Cc: Maxime Chevallier, netdev, linux-kernel, Andrew Lunn,
	Florian Fainelli, Heiner Kallweit, Russell King, linux-arm-kernel,
	Antoine Tenart, thomas.petazzoni, gregory.clement, miquel.raynal
In-Reply-To: <20190225161407.19510-1-maxime.chevallier@bootlin.com>

Contrary to the 3310, the 2110 PHY correctly reports it's 2.5G/5G
abilities. We can therefore use the genphy_c45_pma_read_abilities helper
to build the list of features.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/phy/marvell10g.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index ab71da55c1e0..79106e70010f 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -472,7 +472,7 @@ static struct phy_driver mv3310_drivers[] = {
 		.phy_id		= MARVELL_PHY_ID_88E2110,
 		.phy_id_mask	= MARVELL_PHY_ID_MASK,
 		.name		= "mv88x2110",
-		.features	= PHY_10GBIT_FEATURES,
+		.get_features	= genphy_c45_pma_read_abilities,
 		.probe		= mv3310_probe,
 		.soft_reset	= gen10g_no_soft_reset,
 		.config_init	= mv3310_config_init,
-- 
2.19.2


^ permalink raw reply related

* Re: [PATCH net-next 01/12] net: sched: flower: don't check for rtnl on head dereference
From: Vlad Buslov @ 2019-02-25 16:11 UTC (permalink / raw)
  To: Cong Wang
  Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
	David Miller
In-Reply-To: <CAM_iQpVHggO6vQDr6RaWdr-g1nZGScO_mkWNVqaDXQSVbdiOCA@mail.gmail.com>


On Fri 22 Feb 2019 at 19:32, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Thu, Feb 21, 2019 at 9:45 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>>
>>
>> On Wed 20 Feb 2019 at 22:33, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> > On Tue, Feb 19, 2019 at 1:46 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>> >>
>> >>
>> >> On Mon 18 Feb 2019 at 19:08, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> >> > On Wed, Feb 13, 2019 at 11:47 PM Vlad Buslov <vladbu@mellanox.com> wrote:
>> >> >>
>> >> >> Flower classifier only changes root pointer during init and destroy. Cls
>> >> >> API implements reference counting for tcf_proto, so there is no danger of
>> >> >> concurrent access to tp when it is being destroyed, even without protection
>> >> >> provided by rtnl lock.
>> >> >
>> >> > How about atomicity? Refcnt doesn't guarantee atomicity, how do
>> >> > you make sure two concurrent modifications are atomic?
>> >>
>> >> In order to guarantee atomicity I lock shared flower classifier data
>> >> structures with tp->lock in following patches.
>> >
>> > Sure, I meant the atomicity of the _whole_ change, as you know
>> > the TC filters are stored in hierarchical structures: a block, a chain,
>> > a tp struct, some type-specific data structure like a hash table.
>> >
>> > Locking tp only solves a partial of the atomicity here. Are you
>> > going to restart the whole change from top down to the bottom?
>>
>> You mean in cases where there is a conflict? Yes, processing EAGAIN in
>> tc_new_tfilter() involves releasing and re-obtaining all locks and
>> references.
>
> Sure, restart only happens when a conflict is detected, this is
> why I called it worst scenario.
>
>
>>
>> >
>> >
>> >>
>> >> >
>> >> >
>> >> >>
>> >> >> Implement new function fl_head_dereference() to dereference tp->root
>> >> >> without checking for rtnl lock. Use it in all flower function that obtain
>> >> >> head pointer instead of rtnl_dereference().
>> >> >>
>> >> >
>> >> > So what lock protects RCU writers after this patch?
>> >>
>> >> I explained it in comment for fl_head_dereference(), but should have
>> >> copied this information to changelog as well:
>> >> Flower classifier only changes root pointer during init and destroy.
>> >> Cls API implements reference counting for tcf_proto, so there is no
>> >> danger of concurrent access to tp when it is being destroyed, even
>> >> without protection provided by rtnl lock.
>> >
>> > So you are saying an RCU pointer is okay to deference without
>> > any lock eve without RCU read lock, right?
>> >
>> >
>> >>
>> >> In initial version of this change I used tp->lock to protect tp->root
>> >> access and verified it with lockdep, but during internal review Jiri
>> >> noted that this is not needed in current flower implementation.
>> >
>> > Let's see what you have on top of your own branch
>> > unlocked_flower_cong_1:
>> >
>> > 1458 static int fl_change(struct net *net, struct sk_buff *in_skb,
>> > 1459                      struct tcf_proto *tp, unsigned long base,
>> > 1460                      u32 handle, struct nlattr **tca,
>> > 1461                      void **arg, bool ovr, bool rtnl_held,
>> > 1462                      struct netlink_ext_ack *extack)
>> > 1463 {
>> > 1464         struct cls_fl_head *head = fl_head_dereference(tp);
>> >
>> > At the point of line 1464, there is no lock taken, tp->lock is taken
>> > after it, block->lock or chain lock is already unlocked before ->change().
>> >
>> > So, what protects this RCU structure? According to RCU, it must be
>> > either RCU read lock or some writer lock. I see none here. :(
>> >
>> > What am I missing?
>>
>> Initially I had flower implementation that protected tp->root access
>> with tp->lock, re-obtaining lock and head reference each time it was
>> used, sometimes multiple times per function (head reference is usually
>> obtained in beginning of every flower API function and used multiple
>> times afterwards). This complicated the code and reduced parallelism.
>> However, in case of flower classifier tp->root is never reassigned after
>> init, so essentially there is no "CU" part in this "RCU" pointer. This
>> realization allowed me to refactor unlocked flower code to look much
>> nicer and perform better. Am I missing something in flower classifier
>> implementation?
>
> So if it is no longer RCU any more, why do you still use
> rcu_dereference_protected()? That is, why not just deref it as a raw
> pointer?
>
> And, I don't think I can buy your argument here. The RCU infrastructure
> should not be changed even after your patches, the fast path is still
> protocted by RCU read lock, while the slow path now is protected by
> some smaller-scope locks. What makes cls_flower so unique that
> it doesn't even need RCU here? tp->root is not reassigned but it is still
> freed via RCU infra, that is in fl_destroy_sleepable().
>
> Thanks.

My cls API patch set introduced reference counting for tcf_proto
structure. With that change tp->ops->destroy() (which calls fl_destroy()
and fl_destroy_sleepable(), in case of flower classifier) is only called
after last reference to tp is released. All slow path users of tp->ops
must obtain reference to tp, so concurrent call to fl_destroy() is not
possible. Before this change tcf_proto structure didn't have reference
counting support and required users to obtain rtnl mutex before calling
its ops callbacks. This was verified in flower by using rtnl_dereference
to obtain tp->root.

^ permalink raw reply

* Re: [PATCH] net: netem: fix skb length BUG_ON in __skb_to_sgvec
From: Stephen Hemminger @ 2019-02-25 16:01 UTC (permalink / raw)
  To: Sheng Lan
  Cc: davem, netdev, netem, xuhanbing, zhengshaoyu, jiqin.ji,
	liuzhiqiang26
In-Reply-To: <c610177c-60d5-ef96-e6b7-23f3e49245a2@huawei.com>

On Mon, 25 Feb 2019 22:49:39 +0800
Sheng Lan <lansheng@huawei.com> wrote:

> From: Sheng Lan <lansheng@huawei.com>
> Subject: [PATCH] net: netem: fix skb length BUG_ON in __skb_to_sgvec
> 
> It can be reproduced by following steps:
> 1. virtio_net NIC is configured with gso/tso on
> 2. configure nginx as http server with an index file bigger than 1M bytes
> 3. use tc netem to produce duplicate packets and delay:
>    tc qdisc add dev eth0 root netem delay 100ms 10ms 30% duplicate 90%
> 4. continually curl the nginx http server to get index file on client
> 5. BUG_ON is seen quickly
> 
> [10258690.371129] kernel BUG at net/core/skbuff.c:4028!
> [10258690.371748] invalid opcode: 0000 [#1] SMP PTI
> [10258690.372094] CPU: 5 PID: 0 Comm: swapper/5 Tainted: G        W         5.0.0-rc6 #2
> [10258690.372094] RSP: 0018:ffffa05797b43da0 EFLAGS: 00010202
> [10258690.372094] RBP: 00000000000005ea R08: 0000000000000000 R09: 00000000000005ea
> [10258690.372094] R10: ffffa0579334d800 R11: 00000000000002c0 R12: 0000000000000002
> [10258690.372094] R13: 0000000000000000 R14: ffffa05793122900 R15: ffffa0578f7cb028
> [10258690.372094] FS:  0000000000000000(0000) GS:ffffa05797b40000(0000) knlGS:0000000000000000
> [10258690.372094] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [10258690.372094] CR2: 00007f1a6dc00868 CR3: 000000001000e000 CR4: 00000000000006e0
> [10258690.372094] Call Trace:
> [10258690.372094]  <IRQ>
> [10258690.372094]  skb_to_sgvec+0x11/0x40
> [10258690.372094]  start_xmit+0x38c/0x520 [virtio_net]
> [10258690.372094]  dev_hard_start_xmit+0x9b/0x200
> [10258690.372094]  sch_direct_xmit+0xff/0x260
> [10258690.372094]  __qdisc_run+0x15e/0x4e0
> [10258690.372094]  net_tx_action+0x137/0x210
> [10258690.372094]  __do_softirq+0xd6/0x2a9
> [10258690.372094]  irq_exit+0xde/0xf0
> [10258690.372094]  smp_apic_timer_interrupt+0x74/0x140
> [10258690.372094]  apic_timer_interrupt+0xf/0x20
> [10258690.372094]  </IRQ>
> 
> In __skb_to_sgvec, the skb->len is not equal to the sum of the skb's
> linear data size and nonlinear data size, thus BUG_ON triggered. The
> bad skb's nonlinear data size is less than skb->data_len, because the
> skb is cloned and a part of related cloned skb's nonlinear data is
> split off.
> 
> Duplicate packet is cloned by skb_clone in netem_enqueue and may be delayed
> some time in qdisc. Due to the delay time, the original skb will be pushed
> again later in __tcp_push_pending_frames when tcp receives new packets.
> In tcp_write_xmit, when the tcp_mss_split_point returns a smaller limit,
> the original skb will be fragmented and the skb's nonlinear data will be
> split off. The length of the skb cloned by netem will not be updated.
> When we use virtio_net NIC, the duplicated cloned skb will be filled into
> a scatter-gather list in __skb_to_sgvec and trigger the BUG_ON.
> 
> Here I replace the skb_clone with skb_copy in netem_enqueue to ensure
> the duplicated skb's nonlinear data is independent.
> 
> Signed-off-by: Sheng Lan <lansheng@huawei.com>
> Reported-by: Qin Ji <jiqin.ji@huawei.com>
> 
> Fixes: 0afb51e7 ("netem: reinsert for duplication")

This sounds like a bug in the other layers (either TCP or Virtio net)
not handling a cloned skb properly.



^ permalink raw reply

* [PATCH 2/2] bpf: initial implementation for BPF_PROG_TYPE_GTRACE
From: Kris Van Hees @ 2019-02-25 15:54 UTC (permalink / raw)
  To: netdev

Many tracing use-cases associate events with the task that triggers
them.  BPF programs triggered from kernel probes/events do not provide
information about the current task.  Some information can be obtained
with helper functions but it is limited and adding more helper functions
doesn't seem like a good idea in the long term.

This patch implements a new BPF program type (gtrace) to offer support
for generic tracing BPF programs.  It is meant to be used through the
mechanism for converting contexts when performing a tail call from one
program type to another.  Programs of type BPF_PROG_TYPE_GTRACE are not
triggered from anywhere explicitly.  Adding this higher level program
type is a better fit for programs that are not specific to one probe or
event type, but rather operate on a variety of them, as tracers often
do.

The context for BPF_PROG_TYPE_GTRACE programs is in this initial
version a combination of struct pt_regs content and select fields from
the struct task_struct.  As can be seen in the included sample code,
this makes it possible to no longer depend on helper functions to get
to task data.

Only one context will ever be in use at a time on a given CPU, because
the context is only initialised when the program is called through a
tail-call from a different (supported) program type.

Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
Reviewed-by: Nick Alcock <nick.alcock@oracle.com>
---
 include/linux/bpf_gtrace.h     |  27 +++++
 include/linux/bpf_types.h      |   3 +
 include/uapi/linux/bpf.h       |   1 +
 kernel/trace/Kconfig           |   7 ++
 kernel/trace/Makefile          |   1 +
 kernel/trace/gtrace.c          | 205 +++++++++++++++++++++++++++++++++
 samples/bpf/Makefile           |   3 +
 samples/bpf/bpf_load.c         |  10 +-
 samples/bpf/tracex5b_kern.c    |  71 ++++++++++++
 samples/bpf/tracex5b_user.c    |  52 +++++++++
 tools/include/uapi/linux/bpf.h |   1 +
 tools/lib/bpf/libbpf.c         |   1 +
 tools/lib/bpf/libbpf_probes.c  |   1 +
 13 files changed, 380 insertions(+), 3 deletions(-)
 create mode 100644 include/linux/bpf_gtrace.h
 create mode 100644 kernel/trace/gtrace.c
 create mode 100644 samples/bpf/tracex5b_kern.c
 create mode 100644 samples/bpf/tracex5b_user.c

diff --git a/include/linux/bpf_gtrace.h b/include/linux/bpf_gtrace.h
new file mode 100644
index 000000000000..10c71e27d714
--- /dev/null
+++ b/include/linux/bpf_gtrace.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/* Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ */
+#ifndef _UAPI__LINUX_BPF_GTRACE_H__
+#define _UAPI__LINUX_BPF_GTRACE_H__
+
+#include <asm/bpf_perf_event.h>
+
+struct bpf_gtrace_context {
+	/* CPU registers */
+	bpf_user_pt_regs_t regs;
+
+	/* Current task info */
+	u64 task;	/* current */
+	u64 state;	/* current->state */
+	u32 prio;	/* current->prio */
+	u32 cpu;	/* current->cpu or current->thread_info->cpu */
+	u32 tid;	/* current->pid */
+	u32 pid;	/* current->tgid */
+	u32 ppid;	/* current->real_parent->tgid */
+	u32 uid;	/* from_kuid(&init_user_ns, current_real_cred()->uid */
+	u32 gid;	/* from_kgid(&init_user_ns, current_real_cred()->gid */
+	u32 euid;	/* from_kuid(&init_user_ns, current_real_cred()->euid */
+	u32 egid;	/* from_kgid(&init_user_ns, current_real_cred()->egid */
+};
+
+#endif /* _UAPI__LINUX_BPF_GTRACE_H__ */
diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index 08bf2f1fe553..4a2961db6cdd 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -35,6 +35,9 @@ BPF_PROG_TYPE(BPF_PROG_TYPE_LIRC_MODE2, lirc_mode2)
 #ifdef CONFIG_INET
 BPF_PROG_TYPE(BPF_PROG_TYPE_SK_REUSEPORT, sk_reuseport)
 #endif
+#ifdef CONFIG_BPF_GTRACE
+BPF_PROG_TYPE(BPF_PROG_TYPE_GTRACE, gtrace)
+#endif
 
 BPF_MAP_TYPE(BPF_MAP_TYPE_ARRAY, array_map_ops)
 BPF_MAP_TYPE(BPF_MAP_TYPE_PERCPU_ARRAY, percpu_array_map_ops)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index bcdd2474eee7..6b463071b245 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -166,6 +166,7 @@ enum bpf_prog_type {
 	BPF_PROG_TYPE_LIRC_MODE2,
 	BPF_PROG_TYPE_SK_REUSEPORT,
 	BPF_PROG_TYPE_FLOW_DISSECTOR,
+	BPF_PROG_TYPE_GTRACE,
 };
 
 enum bpf_attach_type {
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index fa8b1fe824f3..8beed291ee68 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -792,6 +792,13 @@ config GCOV_PROFILE_FTRACE
 	  Note that on a kernel compiled with this config, ftrace will
 	  run significantly slower.
 
+config BPF_GTRACE
+	bool "Generic BPF Tracing Support"
+	depends on BPF_SYSCALL
+	default y
+	help
+	  Enable generic tracing BPF program support.
+
 endif # FTRACE
 
 endif # TRACING_SUPPORT
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index c2b2148bb1d2..4deeff2a8b70 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -82,6 +82,7 @@ endif
 obj-$(CONFIG_DYNAMIC_EVENTS) += trace_dynevent.o
 obj-$(CONFIG_PROBE_EVENTS) += trace_probe.o
 obj-$(CONFIG_UPROBE_EVENTS) += trace_uprobe.o
+obj-$(CONFIG_BPF_GTRACE) += gtrace.o
 
 obj-$(CONFIG_TRACEPOINT_BENCHMARK) += trace_benchmark.o
 
diff --git a/kernel/trace/gtrace.c b/kernel/trace/gtrace.c
new file mode 100644
index 000000000000..5ea2d4d7d0c4
--- /dev/null
+++ b/kernel/trace/gtrace.c
@@ -0,0 +1,205 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ */
+#include <linux/bpf.h>
+#include <linux/bpf_gtrace.h>
+#include <linux/filter.h>
+#include <linux/ptrace.h>
+#include <linux/sched.h>
+
+struct bpf_gtrace_ctx {
+	struct pt_regs *regs;
+	struct task_struct *task;
+};
+
+static const struct bpf_func_proto *
+gtrace_get_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
+{
+	switch (func_id) {
+	case BPF_FUNC_trace_printk:
+		return bpf_get_trace_printk_proto();
+	default:
+		return NULL;
+	}
+}
+
+static bool gtrace_is_valid_access(int off, int size, enum bpf_access_type type,
+				   const struct bpf_prog *prog,
+				   struct bpf_insn_access_aux *info)
+{
+	/* Ensure offset is within the context structure. */
+	if (off < 0 || off >= sizeof(struct bpf_gtrace_context))
+		return false;
+
+	/* Only READ access is allowed. */
+	if (type != BPF_READ)
+		return false;
+
+	/* Ensure offset is aligned (verifier guarantees size > 0). */
+	if (off % size != 0)
+		return false;
+
+	switch (off) {
+	case bpf_ctx_range(struct bpf_gtrace_context, task):
+	case bpf_ctx_range(struct bpf_gtrace_context, state):
+		bpf_ctx_record_field_size(info, sizeof(u64));
+		if (bpf_ctx_narrow_access_ok(off, size, sizeof(u64)))
+			return true;
+		break;
+	case bpf_ctx_range(struct bpf_gtrace_context, prio):
+	case bpf_ctx_range(struct bpf_gtrace_context, cpu):
+	case bpf_ctx_range(struct bpf_gtrace_context, tid):
+	case bpf_ctx_range(struct bpf_gtrace_context, pid):
+	case bpf_ctx_range(struct bpf_gtrace_context, ppid):
+	case bpf_ctx_range(struct bpf_gtrace_context, uid):
+	case bpf_ctx_range(struct bpf_gtrace_context, gid):
+	case bpf_ctx_range(struct bpf_gtrace_context, euid):
+	case bpf_ctx_range(struct bpf_gtrace_context, egid):
+		bpf_ctx_record_field_size(info, sizeof(u32));
+		if (bpf_ctx_narrow_access_ok(off, size, sizeof(u32)))
+			return true;
+		break;
+	default:
+		if (size == sizeof(unsigned long))
+			return true;
+	}
+
+	return false;
+}
+
+#define BPF_LDX_CTX_PTR(type, member, si) \
+	BPF_LDX_MEM(BPF_FIELD_SIZEOF(type, member), \
+		    (si)->dst_reg, (si)->src_reg, offsetof(type, member))
+#define BPF_LDX_LNK_PTR(type, member, si) \
+	BPF_LDX_MEM(BPF_FIELD_SIZEOF(type, member), \
+		    (si)->dst_reg, (si)->dst_reg, offsetof(type, member))
+#define BPF_LDX_CTX_FIELD(type, member, si, target_size) \
+	BPF_LDX_MEM(BPF_FIELD_SIZEOF(type, member), \
+		    (si)->dst_reg, (si)->src_reg, \
+		    ({ \
+			*(target_size) = FIELD_SIZEOF(type, member); \
+			offsetof(type, member); \
+		    }))
+#define BPF_LDX_LNK_FIELD(type, member, si, target_size) \
+	BPF_LDX_MEM(BPF_FIELD_SIZEOF(type, member), \
+		    (si)->dst_reg, (si)->dst_reg, \
+		    ({ \
+			*(target_size) = FIELD_SIZEOF(type, member); \
+			offsetof(type, member); \
+		    }))
+
+static u32 gtrace_convert_ctx_access(enum bpf_access_type type,
+				     const struct bpf_insn *si,
+				     struct bpf_insn *insn_buf,
+				     struct bpf_prog *prog, u32 *target_size)
+{
+	struct bpf_insn *insn = insn_buf;
+
+	switch (si->off) {
+	case offsetof(struct bpf_gtrace_context, task):
+		*insn++ = BPF_LDX_CTX_FIELD(struct bpf_gtrace_ctx, task, si,
+					    target_size);
+		break;
+	case offsetof(struct bpf_gtrace_context, state):
+		*insn++ = BPF_LDX_CTX_PTR(struct bpf_gtrace_ctx, task, si);
+		*insn++ = BPF_LDX_LNK_FIELD(struct task_struct, state, si,
+					    target_size);
+		break;
+	case offsetof(struct bpf_gtrace_context, prio):
+		*insn++ = BPF_LDX_CTX_PTR(struct bpf_gtrace_ctx, task, si);
+		*insn++ = BPF_LDX_LNK_FIELD(struct task_struct, prio, si,
+					    target_size);
+		break;
+	case offsetof(struct bpf_gtrace_context, cpu):
+		*insn++ = BPF_LDX_CTX_PTR(struct bpf_gtrace_ctx, task, si);
+#ifdef CONFIG_THREAD_INFO_IN_TASK
+		*insn++ = BPF_LDX_LNK_FIELD(struct task_struct, cpu, si,
+					    target_size);
+#else
+		*insn++ = BPF_LDX_LNK_PTR(struct task_struct, stack, si);
+		*insn++ = BPF_LDX_LNK_FIELD(struct thread_info, cpu, si,
+					    target_size);
+#endif
+		break;
+	case offsetof(struct bpf_gtrace_context, tid):
+		*insn++ = BPF_LDX_CTX_PTR(struct bpf_gtrace_ctx, task, si);
+		*insn++ = BPF_LDX_LNK_FIELD(struct task_struct, pid, si,
+					    target_size);
+		break;
+	case offsetof(struct bpf_gtrace_context, pid):
+		*insn++ = BPF_LDX_CTX_PTR(struct bpf_gtrace_ctx, task, si);
+		*insn++ = BPF_LDX_LNK_FIELD(struct task_struct, tgid, si,
+					    target_size);
+		break;
+	case offsetof(struct bpf_gtrace_context, ppid):
+		*insn++ = BPF_LDX_CTX_PTR(struct bpf_gtrace_ctx, task, si);
+		*insn++ = BPF_LDX_LNK_PTR(struct task_struct, real_parent, si);
+		*insn++ = BPF_LDX_LNK_FIELD(struct task_struct, tgid, si,
+					    target_size);
+		break;
+	case offsetof(struct bpf_gtrace_context, uid):
+		*insn++ = BPF_LDX_CTX_PTR(struct bpf_gtrace_ctx, task, si);
+		*insn++ = BPF_LDX_LNK_PTR(struct task_struct, cred, si);
+		*insn++ = BPF_LDX_LNK_FIELD(struct cred, uid, si, target_size);
+		break;
+	case offsetof(struct bpf_gtrace_context, gid):
+		*insn++ = BPF_LDX_CTX_PTR(struct bpf_gtrace_ctx, task, si);
+		*insn++ = BPF_LDX_LNK_PTR(struct task_struct, cred, si);
+		*insn++ = BPF_LDX_LNK_FIELD(struct cred, gid, si, target_size);
+		break;
+	case offsetof(struct bpf_gtrace_context, euid):
+		*insn++ = BPF_LDX_CTX_PTR(struct bpf_gtrace_ctx, task, si);
+		*insn++ = BPF_LDX_LNK_PTR(struct task_struct, cred, si);
+		*insn++ = BPF_LDX_LNK_FIELD(struct cred, euid, si, target_size);
+		break;
+	case offsetof(struct bpf_gtrace_context, egid):
+		*insn++ = BPF_LDX_CTX_PTR(struct bpf_gtrace_ctx, task, si);
+		*insn++ = BPF_LDX_LNK_PTR(struct task_struct, cred, si);
+		*insn++ = BPF_LDX_LNK_FIELD(struct cred, egid, si, target_size);
+		break;
+	default:
+		*insn++ = BPF_LDX_CTX_PTR(struct bpf_gtrace_ctx, regs, si);
+		*insn++ = BPF_LDX_MEM(BPF_SIZEOF(long), si->dst_reg, si->dst_reg,
+				      si->off);
+		break;
+	}
+
+	return insn - insn_buf;
+}
+
+const struct bpf_verifier_ops gtrace_verifier_ops = {
+	.get_func_proto		= gtrace_get_func_proto,
+	.is_valid_access	= gtrace_is_valid_access,
+	.convert_ctx_access	= gtrace_convert_ctx_access,
+};
+
+static bool gtrace_can_cast(enum bpf_prog_type stype, enum bpf_prog_type ttype)
+{
+	if (stype == BPF_PROG_TYPE_KPROBE)
+		return true;
+
+	return false;
+}
+
+DEFINE_PER_CPU(struct bpf_gtrace_ctx, gtrace_ctx);
+
+static void *gtrace_cast_context(enum bpf_prog_type stype,
+				 enum bpf_prog_type ttype, void *ctx)
+{
+	struct bpf_gtrace_ctx *gctx;
+
+	if (stype == BPF_PROG_TYPE_KPROBE) {
+		gctx = this_cpu_ptr(&gtrace_ctx);
+		gctx->regs = (struct pt_regs *)ctx;
+		gctx->task = current;
+
+		return gctx;
+	}
+
+	return NULL;
+}
+
+const struct bpf_prog_ops gtrace_prog_ops = {
+	.can_cast		= gtrace_can_cast,
+	.cast_context		= gtrace_cast_context,
+};
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index a0ef7eddd0b3..a5f8988acdb2 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -15,6 +15,7 @@ hostprogs-y += tracex2
 hostprogs-y += tracex3
 hostprogs-y += tracex4
 hostprogs-y += tracex5
+hostprogs-y += tracex5b
 hostprogs-y += tracex6
 hostprogs-y += tracex7
 hostprogs-y += test_probe_write_user
@@ -69,6 +70,7 @@ tracex2-objs := bpf_load.o tracex2_user.o
 tracex3-objs := bpf_load.o tracex3_user.o
 tracex4-objs := bpf_load.o tracex4_user.o
 tracex5-objs := bpf_load.o tracex5_user.o
+tracex5b-objs := bpf_load.o tracex5b_user.o
 tracex6-objs := bpf_load.o tracex6_user.o
 tracex7-objs := bpf_load.o tracex7_user.o
 load_sock_ops-objs := bpf_load.o load_sock_ops.o
@@ -120,6 +122,7 @@ always += tracex2_kern.o
 always += tracex3_kern.o
 always += tracex4_kern.o
 always += tracex5_kern.o
+always += tracex5b_kern.o
 always += tracex6_kern.o
 always += tracex7_kern.o
 always += sock_flags_kern.o
diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c
index eae7b635343d..847b46a64315 100644
--- a/samples/bpf/bpf_load.c
+++ b/samples/bpf/bpf_load.c
@@ -87,6 +87,7 @@ static int load_and_attach(const char *event, struct bpf_insn *prog, int size)
 	bool is_sockops = strncmp(event, "sockops", 7) == 0;
 	bool is_sk_skb = strncmp(event, "sk_skb", 6) == 0;
 	bool is_sk_msg = strncmp(event, "sk_msg", 6) == 0;
+	bool is_gtrace = strncmp(event, "gtrace", 6) == 0;
 	size_t insns_cnt = size / sizeof(struct bpf_insn);
 	enum bpf_prog_type prog_type;
 	char buf[256];
@@ -120,6 +121,8 @@ static int load_and_attach(const char *event, struct bpf_insn *prog, int size)
 		prog_type = BPF_PROG_TYPE_SK_SKB;
 	} else if (is_sk_msg) {
 		prog_type = BPF_PROG_TYPE_SK_MSG;
+	} else if (is_gtrace) {
+		prog_type = BPF_PROG_TYPE_GTRACE;
 	} else {
 		printf("Unknown event '%s'\n", event);
 		return -1;
@@ -140,8 +143,8 @@ static int load_and_attach(const char *event, struct bpf_insn *prog, int size)
 	if (is_xdp || is_perf_event || is_cgroup_skb || is_cgroup_sk)
 		return 0;
 
-	if (is_socket || is_sockops || is_sk_skb || is_sk_msg) {
-		if (is_socket)
+	if (is_socket || is_sockops || is_sk_skb || is_sk_msg || is_gtrace) {
+		if (is_socket || is_gtrace)
 			event += 6;
 		else
 			event += 7;
@@ -643,7 +646,8 @@ static int do_load_bpf_file(const char *path, fixup_map_cb fixup_map)
 		    memcmp(shname, "cgroup/", 7) == 0 ||
 		    memcmp(shname, "sockops", 7) == 0 ||
 		    memcmp(shname, "sk_skb", 6) == 0 ||
-		    memcmp(shname, "sk_msg", 6) == 0) {
+		    memcmp(shname, "sk_msg", 6) == 0 ||
+		    memcmp(shname, "gtrace", 6) == 0) {
 			ret = load_and_attach(shname, data->d_buf,
 					      data->d_size);
 			if (ret != 0)
diff --git a/samples/bpf/tracex5b_kern.c b/samples/bpf/tracex5b_kern.c
new file mode 100644
index 000000000000..3bcc4779df3f
--- /dev/null
+++ b/samples/bpf/tracex5b_kern.c
@@ -0,0 +1,71 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ */
+#include <linux/ptrace.h>
+#include <linux/version.h>
+#include <uapi/linux/bpf.h>
+#include <linux/bpf_gtrace.h>
+#include <uapi/linux/seccomp.h>
+#include <uapi/linux/unistd.h>
+#include "syscall_nrs.h"
+#include "bpf_helpers.h"
+
+#define PROG(F) SEC("gtrace/"__stringify(F)) int bpf_func_##F
+
+struct bpf_map_def SEC("maps") progs = {
+	.type = BPF_MAP_TYPE_PROG_ARRAY,
+	.key_size = sizeof(u32),
+	.value_size = sizeof(u32),
+#ifdef __mips__
+	.max_entries = 6000, /* MIPS n64 syscalls start at 5000 */
+#else
+	.max_entries = 1024,
+#endif
+};
+
+/* we jump here when syscall number == __NR_write */
+PROG(SYS__NR_write)(struct bpf_gtrace_context *ctx)
+{
+	char fmt1[] = "write for task %p (state %x, ppid %d)\n";
+	char fmt2[] = "write for task %p (tid %d, pid %d)\n";
+	char fmt3[] = "write for task %p (uid %d, gid %d)\n";
+	char fmt4[] = "write for task %p (euid %d, egid %d)\n";
+	bpf_trace_printk(fmt1, sizeof(fmt1), ctx->task, ctx->state, ctx->ppid);
+	bpf_trace_printk(fmt2, sizeof(fmt2), ctx->task, ctx->tid, ctx->pid);
+	bpf_trace_printk(fmt3, sizeof(fmt3), ctx->task, ctx->uid, ctx->gid);
+	bpf_trace_printk(fmt4, sizeof(fmt4), ctx->task, ctx->euid, ctx->egid);
+	return 0;
+}
+
+PROG(SYS__NR_read)(struct bpf_gtrace_context *ctx)
+{
+	char fmt[] = "read for task %p\n";
+	bpf_trace_printk(fmt, sizeof(fmt), ctx->task);
+	return 0;
+}
+
+PROG(SYS__NR_mmap)(struct bpf_gtrace_context *ctx)
+{
+	char fmt[] = "mmap for task %p\n";
+	bpf_trace_printk(fmt, sizeof(fmt), ctx->task);
+	return 0;
+}
+
+SEC("kprobe/__seccomp_filter")
+int bpf_prog1(struct pt_regs *ctx)
+{
+	int sc_nr = (int)PT_REGS_PARM1(ctx);
+
+	/* dispatch into next BPF program depending on syscall number */
+	bpf_tail_call(ctx, &progs, sc_nr);
+
+	/* fall through -> unknown syscall */
+	if (sc_nr >= __NR_getuid && sc_nr <= __NR_getsid) {
+		char fmt[] = "syscall=%d (one of get/set uid/pid/gid)\n";
+		bpf_trace_printk(fmt, sizeof(fmt), sc_nr);
+	}
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
+u32 _version SEC("version") = LINUX_VERSION_CODE;
diff --git a/samples/bpf/tracex5b_user.c b/samples/bpf/tracex5b_user.c
new file mode 100644
index 000000000000..f59d75e7805b
--- /dev/null
+++ b/samples/bpf/tracex5b_user.c
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ */
+#include <stdio.h>
+#include <linux/bpf.h>
+#include <unistd.h>
+#include <linux/filter.h>
+#include <linux/seccomp.h>
+#include <sys/prctl.h>
+#include <bpf/bpf.h>
+#include "bpf_load.h"
+#include <sys/resource.h>
+
+/* install fake seccomp program to enable seccomp code path inside the kernel,
+ * so that our kprobe attached to seccomp_phase1() can be triggered
+ */
+static void install_accept_all_seccomp(void)
+{
+	struct sock_filter filter[] = {
+		BPF_STMT(BPF_RET+BPF_K, SECCOMP_RET_ALLOW),
+	};
+	struct sock_fprog prog = {
+		.len = (unsigned short)(sizeof(filter)/sizeof(filter[0])),
+		.filter = filter,
+	};
+	if (prctl(PR_SET_SECCOMP, 2, &prog))
+		perror("prctl");
+}
+
+int main(int ac, char **argv)
+{
+	FILE *f;
+	char filename[256];
+	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
+
+	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
+	setrlimit(RLIMIT_MEMLOCK, &r);
+
+	if (load_bpf_file(filename)) {
+		printf("%s", bpf_log_buf);
+		return 1;
+	}
+
+	install_accept_all_seccomp();
+
+	f = popen("dd if=/dev/zero of=/dev/null count=5", "r");
+	(void) f;
+
+	read_trace_pipe();
+
+	return 0;
+}
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index bcdd2474eee7..6b463071b245 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -166,6 +166,7 @@ enum bpf_prog_type {
 	BPF_PROG_TYPE_LIRC_MODE2,
 	BPF_PROG_TYPE_SK_REUSEPORT,
 	BPF_PROG_TYPE_FLOW_DISSECTOR,
+	BPF_PROG_TYPE_GTRACE,
 };
 
 enum bpf_attach_type {
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index b38dcbe7460a..297a0a83086e 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1682,6 +1682,7 @@ static bool bpf_prog_type__needs_kver(enum bpf_prog_type type)
 	case BPF_PROG_TYPE_PERF_EVENT:
 		return false;
 	case BPF_PROG_TYPE_KPROBE:
+	case BPF_PROG_TYPE_GTRACE:
 	default:
 		return true;
 	}
diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
index 8c3a1c04dcb2..2b5bfcda7606 100644
--- a/tools/lib/bpf/libbpf_probes.c
+++ b/tools/lib/bpf/libbpf_probes.c
@@ -97,6 +97,7 @@ probe_load(enum bpf_prog_type prog_type, const struct bpf_insn *insns,
 	case BPF_PROG_TYPE_LIRC_MODE2:
 	case BPF_PROG_TYPE_SK_REUSEPORT:
 	case BPF_PROG_TYPE_FLOW_DISSECTOR:
+	case BPF_PROG_TYPE_GTRACE:
 	default:
 		break;
 	}
-- 
2.20.1


^ permalink raw reply related

* [PATCH 1/2] bpf: context casting for tail call
From: Kris Van Hees @ 2019-02-25 15:54 UTC (permalink / raw)
  To: netdev

Currently BPF programs are executed with a context that is provided by
code that initiates the execution.  Tracing tools that want to make use
of existing probes and events that allow BPF programs to be attached to
them are thus limited to the context information provided by the probe
or event source.  Often, more context is needed to allow tracing tools
the ablity to implement more complex constructs (e.g. more state-full
tracing).

This patch extends the tail-call mechanism to allow a BPF program of
one type to call a BPF program of another type.  E.g. a kprobe BPF
program (working with a struct pt_regs context) can call a BPF program
with a more extensive context.  The BPF program type is being extended
to provide can_cast() and cast_context() callback functions to handle
the context conversion.

The program array holding BPF programs that you can tail-call into
continues to require that all programs are of the same type.  But when
a compatibility check is made in a program that performs a tail-call,
the can_cast() function is called (if available) to allow the target
type to determine whether it can handle the conversion of a context
from the source type to the target type.  If can_cast() is not provided
by the program type, casting is denied.

During execution, the cast_context() function is called (if available)
to perform the conversion of the current context to the context that the
target type expects.  Since the program type of the executing BPF program
is not explicitly known during execution, the verifier inserts an
instruction right before the tail-call to assign the current BPF program
type to R4.

The interpreter calls cast_context() using the program type in R4 as
source program type, the program type associated with the program array
as target program type, and the context as provided in R1.

The bpf_prog_types array is now being exposed to the rest of the BPF
code (where before it was local to just the syscall handling) because
the can_cast and cat_context operations need to be accessible.

There is no noticeable effect on BPF program types that do not implement
this new feature.

A JIT implementation is not available yet in this first iteration.

Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
Reviewed-by: Nick Alcock <nick.alcock@oracle.com>
---
 include/linux/bpf.h   |  4 ++++
 kernel/bpf/core.c     | 27 ++++++++++++++++++++++++++-
 kernel/bpf/syscall.c  |  2 +-
 kernel/bpf/verifier.c | 16 ++++++++++++----
 4 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index de18227b3d95..117d2bae51b9 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -281,6 +281,9 @@ bpf_ctx_record_field_size(struct bpf_insn_access_aux *aux, u32 size)
 struct bpf_prog_ops {
 	int (*test_run)(struct bpf_prog *prog, const union bpf_attr *kattr,
 			union bpf_attr __user *uattr);
+	bool (*can_cast)(enum bpf_prog_type stype, enum bpf_prog_type ttype);
+	void *(*cast_context)(enum bpf_prog_type stype,
+			      enum bpf_prog_type ttype, void *ctx);
 };
 
 struct bpf_verifier_ops {
@@ -528,6 +531,7 @@ extern const struct file_operations bpf_prog_fops;
 #undef BPF_PROG_TYPE
 #undef BPF_MAP_TYPE
 
+extern const struct bpf_prog_ops * const bpf_prog_types[];
 extern const struct bpf_prog_ops bpf_offload_prog_ops;
 extern const struct bpf_verifier_ops tc_cls_act_analyzer_ops;
 extern const struct bpf_verifier_ops xdp_analyzer_ops;
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index ef88b167959d..1b7c718d4e9d 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1426,10 +1426,12 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn, u64 *stack)
 		CONT;
 
 	JMP_TAIL_CALL: {
+		void *ctx = (void *) (unsigned long) BPF_R1;
 		struct bpf_map *map = (struct bpf_map *) (unsigned long) BPF_R2;
 		struct bpf_array *array = container_of(map, struct bpf_array, map);
 		struct bpf_prog *prog;
 		u32 index = BPF_R3;
+		u32 type = BPF_R4;
 
 		if (unlikely(index >= array->map.max_entries))
 			goto out;
@@ -1441,6 +1443,14 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn, u64 *stack)
 		prog = READ_ONCE(array->ptrs[index]);
 		if (!prog)
 			goto out;
+		if (prog->aux->ops->cast_context) {
+			ctx = prog->aux->ops->cast_context(type, prog->type,
+							   ctx);
+			if (!ctx)
+				goto out;
+
+			BPF_R1 = (u64) ctx;
+		}
 
 		/* ARG1 at this point is guaranteed to point to CTX from
 		 * the verifier side due to the fact that the tail call is
@@ -1637,6 +1647,20 @@ bool bpf_prog_array_compatible(struct bpf_array *array,
 	       array->owner_jited == fp->jited;
 }
 
+bool bpf_prog_array_can_cast(struct bpf_array *array, const struct bpf_prog *fp)
+{
+	const struct bpf_prog_ops *ops;
+
+	if (array->owner_jited != fp->jited)
+		return false;
+
+	ops = bpf_prog_types[array->owner_prog_type];
+	if (ops->can_cast)
+		return ops->can_cast(fp->type, array->owner_prog_type);
+
+	return false;
+}
+
 static int bpf_check_tail_call(const struct bpf_prog *fp)
 {
 	struct bpf_prog_aux *aux = fp->aux;
@@ -1650,7 +1674,8 @@ static int bpf_check_tail_call(const struct bpf_prog *fp)
 			continue;
 
 		array = container_of(map, struct bpf_array, map);
-		if (!bpf_prog_array_compatible(array, fp))
+		if (!bpf_prog_array_compatible(array, fp) &&
+		    !bpf_prog_array_can_cast(array, fp))
 			return -EINVAL;
 	}
 
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index ec7c552af76b..d558d979100f 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1110,7 +1110,7 @@ static int map_lookup_and_delete_elem(union bpf_attr *attr)
 	return err;
 }
 
-static const struct bpf_prog_ops * const bpf_prog_types[] = {
+const struct bpf_prog_ops * const bpf_prog_types[] = {
 #define BPF_PROG_TYPE(_id, _name) \
 	[_id] = & _name ## _prog_ops,
 #define BPF_MAP_TYPE(_id, _ops)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 1b9496c41383..b49820e82cf8 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -7629,9 +7629,10 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
 			insn->imm = 0;
 			insn->code = BPF_JMP | BPF_TAIL_CALL;
 
+			cnt = 0;
 			aux = &env->insn_aux_data[i + delta];
 			if (!bpf_map_ptr_unpriv(aux))
-				continue;
+				goto privileged;
 
 			/* instead of changing every JIT dealing with tail_call
 			 * emit two extra insns:
@@ -7646,13 +7647,20 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
 
 			map_ptr = BPF_MAP_PTR(aux->map_state);
 			insn_buf[0] = BPF_JMP_IMM(BPF_JGE, BPF_REG_3,
-						  map_ptr->max_entries, 2);
+						  map_ptr->max_entries, 3);
 			insn_buf[1] = BPF_ALU32_IMM(BPF_AND, BPF_REG_3,
 						    container_of(map_ptr,
 								 struct bpf_array,
 								 map)->index_mask);
-			insn_buf[2] = *insn;
-			cnt = 3;
+			cnt = 2;
+
+privileged:
+			/* store the BPF program type of the currnet program in
+			 * R4 so it is known in case this tail call requires
+			 * casting the context to a different program type
+			 */
+			insn_buf[cnt++] = BPF_MOV64_IMM(BPF_REG_4, prog->type);
+			insn_buf[cnt++] = *insn;
 			new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt);
 			if (!new_prog)
 				return -ENOMEM;
-- 
2.20.1


^ permalink raw reply related

* [PATCH 0/2] bpf: context casting for tail call and gtrace prog type
From: Kris Van Hees @ 2019-02-25 15:54 UTC (permalink / raw)
  To: netdev

The patches in this set are part of an effort to provide support for
tracing tools beyond attaching programs to probes and events and working
with the context data they provide.  It is also aimed at avoiding adding
new helpers for every piece of task information that tracers may want to
include in trace data (as was discussed at the Linux Plumbers Conference
BPF mini-conference track last year).


One of the main characteristics of tracers is that a variety of
information can be collected at the time of a probe firing.  When using
BPF program to implement actions to be taken when a probe fires, The most
natural source of a large part of this information (task information,
probe data, tracer state) is the context that is associated with a BPF
program.  It is also possible to obtain (most of) this information by
means of full access helper calls like probe_read() but that isn't
something you want to make available to an unprivileged user.

So we have two areas where BPF programs can be very useful and powerful:

- BPF programs that are attached to a probe or event, operating on the
  context provided by the probe or event
- BPF programs that implement actions to be taken within the context of a
  tracing tool

The Linux kernel provides a wealth of probes and events to which we can
attach BPF programs.  These event sources do not have any knowledge of the
tracing tool that might be using them.  But being able to use them from any
tracing tool is definitely preferable over implementing your own probes and
events.  We definitely also do not want to 'teach' all the existing probes
and events about any possible BPF program type that would like to get called
from those probes and events.

So, to illustrate what we're trying to accomplish, consider a kprobe.  We
can attach a BPF program to it and it will be called with a 'struct pt_regs'
context.  From the side of our tracing tool, we also want information about
the task that triggered the kprobe to fire (beyond what is currently
available through helpers) and we want to be able to access that information
from a BPF program that implements what should happen when the probe fires
(e.g. recording the event and specific data that we are interested in).

The 2nd patch in this set implements a very basic generic tracer program
type BPF_PROG_TYPE_GTRACE) that provides the pt_regs data and select task
data in its context.  We cannot attach a program of this type to a kprobe
because that probe supports BPF_PROG_TYPE_KPROBE instead.

The 1st patch in this set implements a mechanism to solve this issue: it
allows a tail-call from one program type to another if the callee type
supports conversion of a caller context into a context for the callee.

So, in the sample, the BPF_PROG_TYPE_GTRACE provides can_cast() and
cast_context() functions that support converting a BPF_PROG_TYPE_KPROBE
context into a BPF_PROG_TYPE_GTRACE context.

The work flow a tracer can use is:

 1. The tracer creates a program array map, and inserts one or more programs
    of type BPF_PROG_TYPE_GTRACE.  These programs implement whatever actions
    are to be taken when a specific probe fires.  This step must be done first
    so that the program array is initialized with the correct program type.
    This type needs to be known so that when the calling program is verified,
    compatibility checking can be performed.
 2. The tracer loads a program of type BPF_PROG_TYPE_KPROBE and attaches it
    to the kprobe we're interested in.  This program contains a tail-call to
    a BPF_PROG_TYPE_GTRACE program in the program array.
 3. The kprobe fires and executes our program (of type BPF_PROG_TYPE_KPROBE).
   3.1 The program performs whatever operations that we need to have done
       at the level of the probe firing.
   3.2 The program performs a tail-call into a program from our program array.
     3.2.1 The execution of the tail-call instruction causes a call to be
           made to a cast_context() function provided by BPF_PROG_TYPE_GTRACE.
           This function creates a context structure, and populates it with
           task information and copies in the pt_regs data from the context
           that was passed to the BPF_PROG_TYPE_KPROBE program.
     3.2.2 The new context is assigned to R1 (replacing the original context),
           and execution is transferred to the called program.

The implementation is done in such way that existing tail-calls will work
without any change aside from the fact that the verifier is inserting an
instruction right before the tail-call.  That instruction simply loads the
BPF program type into R4.  This ensures that at the time of the tail-call,
the program type of the calling program can be passed to the cast_context()
function.  Knowledge about the program type of an executing program is not
available anywhere and we need to know what context we're trying to convert
from.  The function prototype for the (pseudo-)helper bpf_tail_call declares
only 3 arguments so existing code is not affected by this internal use of R4.

Obviously, if there is no conversion function or the conversion is not
supported, the tail-call will fail because that situation is effectively the
same as trying to call a program of an incompatible type.

The goal is to further extend the BPF_PROG_TYPE_GTRACE implementation to
support what tracers commonly need, and I am also looking at ways to further
extend this model to allow more tracer-specific features as well without the
need for adding a BPF program types for every tracer.

	Kris

^ permalink raw reply

* Re: [PATCH] tun: fix blocking read
From: Eric Dumazet @ 2019-02-25 15:53 UTC (permalink / raw)
  To: David Miller, mail; +Cc: netdev, linux-kernel
In-Reply-To: <20190224.221236.905722532971991928.davem@davemloft.net>



On 02/24/2019 10:12 PM, David Miller wrote:
> From: Timur Celik <mail@timurcelik.de>
> Date: Sat, 23 Feb 2019 12:53:13 +0100
> 
>> This patch moves setting of the current state into the loop. Otherwise
>> the task may end up in a busy wait loop if none of the break conditions
>> are met.
>>
>> Signed-off-by: Timur Celik <mail@timurcelik.de>
> 
> Applied and queued up for -stable, thanks.
> 

First part of the patch was matching the changelog, however
the second change was not really needed, or should have used 

__set_current_state(TASK_RUNNING);


^ permalink raw reply

* Re: [PATCH net-next] tcp: remove unused parameter of tcp_sacktag_bsearch()
From: Eric Dumazet @ 2019-02-25 15:47 UTC (permalink / raw)
  To: Taehee Yoo; +Cc: David Miller, netdev, Yuchung Cheng
In-Reply-To: <20190225094233.16231-1-ap420073@gmail.com>

On Mon, Feb 25, 2019 at 1:42 AM Taehee Yoo <ap420073@gmail.com> wrote:
>
> parameter state in the tcp_sacktag_bsearch() is not used.
> So, it can be removed.

Yes, this is a nice cleanup that should have followed commit
737ff314563ca27f044f9a3a041e9d42491ef7ce ("tcp: use sequence distance
to detect reordering")
since it removed fack_count

Signed-off-by: Eric Dumazet <edumazet@google.com>

Thanks.

^ permalink raw reply

* [PATCH net-next] net: sched: don't release block->lock when dumping chains
From: Vlad Buslov @ 2019-02-25 15:45 UTC (permalink / raw)
  To: netdev; +Cc: jhs, xiyou.wangcong, jiri, davem, Vlad Buslov

Function tc_dump_chain() obtains and releases block->lock on each iteration
of its inner loop that dumps all chains on block. Outputting chain template
info is fast operation so locking/unlocking mutex multiple times is an
overhead when lock is highly contested. Modify tc_dump_chain() to only
obtain block->lock once and dump all chains without releasing it.

Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Suggested-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/sched/cls_api.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 3543be31d400..5c6a9baa389f 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -2996,12 +2996,12 @@ static int tc_ctl_chain(struct sk_buff *skb, struct nlmsghdr *n,
 /* called with RTNL */
 static int tc_dump_chain(struct sk_buff *skb, struct netlink_callback *cb)
 {
-	struct tcf_chain *chain, *chain_prev;
 	struct net *net = sock_net(skb->sk);
 	struct nlattr *tca[TCA_MAX + 1];
 	struct Qdisc *q = NULL;
 	struct tcf_block *block;
 	struct tcmsg *tcm = nlmsg_data(cb->nlh);
+	struct tcf_chain *chain;
 	long index_start;
 	long index;
 	u32 parent;
@@ -3064,11 +3064,8 @@ static int tc_dump_chain(struct sk_buff *skb, struct netlink_callback *cb)
 	index_start = cb->args[0];
 	index = 0;
 
-	for (chain = __tcf_get_next_chain(block, NULL);
-	     chain;
-	     chain_prev = chain,
-		     chain = __tcf_get_next_chain(block, chain),
-		     tcf_chain_put(chain_prev)) {
+	mutex_lock(&block->lock);
+	list_for_each_entry(chain, &block->chain_list, list) {
 		if ((tca[TCA_CHAIN] &&
 		     nla_get_u32(tca[TCA_CHAIN]) != chain->index))
 			continue;
@@ -3076,17 +3073,18 @@ static int tc_dump_chain(struct sk_buff *skb, struct netlink_callback *cb)
 			index++;
 			continue;
 		}
+		if (tcf_chain_held_by_acts_only(chain))
+			continue;
 		err = tc_chain_fill_node(chain->tmplt_ops, chain->tmplt_priv,
 					 chain->index, net, skb, block,
 					 NETLINK_CB(cb->skb).portid,
 					 cb->nlh->nlmsg_seq, NLM_F_MULTI,
 					 RTM_NEWCHAIN);
-		if (err <= 0) {
-			tcf_chain_put(chain);
+		if (err <= 0)
 			break;
-		}
 		index++;
 	}
+	mutex_unlock(&block->lock);
 
 	if (tcm->tcm_ifindex == TCM_IFINDEX_MAGIC_BLOCK)
 		tcf_block_refcnt_put(block, true);
-- 
2.13.6


^ permalink raw reply related

* [PATCH ethtool V2] ethtool: Add support for 200Gbps (50Gbps per lane) link mode
From: Tariq Toukan @ 2019-02-25 15:44 UTC (permalink / raw)
  To: John W. Linville
  Cc: netdev, Eran Ben Elisha, alaa, Andrew Lunn, Michal Kubecek,
	Aya Levin, Tariq Toukan

From: Aya Levin <ayal@mellanox.com>

Introduce 50Gbps per lane link modes and 200Gbps speed, update print
functions and initialization functions accordingly.
In addition, update related man page accordingly.

Signed-off-by: Aya Levin <ayal@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
 ethtool-copy.h | 18 +++++++++++++++++-
 ethtool.8.in   | 15 +++++++++++++++
 ethtool.c      | 45 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 77 insertions(+), 1 deletion(-)

V2:
- Removed wrong and unneeded initialization of adv_bit.
- Removed an addition in ethtool-copy.h that does not exist in kernel.


diff --git a/ethtool-copy.h b/ethtool-copy.h
index 6bfbb85f9402..120407604232 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -1455,6 +1455,21 @@ enum ethtool_link_mode_bit_indices {
 	ETHTOOL_LINK_MODE_FEC_NONE_BIT	= 49,
 	ETHTOOL_LINK_MODE_FEC_RS_BIT	= 50,
 	ETHTOOL_LINK_MODE_FEC_BASER_BIT	= 51,
+	ETHTOOL_LINK_MODE_50000baseKR_Full_BIT		 = 52,
+	ETHTOOL_LINK_MODE_50000baseSR_Full_BIT		 = 53,
+	ETHTOOL_LINK_MODE_50000baseCR_Full_BIT		 = 54,
+	ETHTOOL_LINK_MODE_50000baseLR_ER_FR_Full_BIT	 = 55,
+	ETHTOOL_LINK_MODE_50000baseDR_Full_BIT		 = 56,
+	ETHTOOL_LINK_MODE_100000baseKR2_Full_BIT	 = 57,
+	ETHTOOL_LINK_MODE_100000baseSR2_Full_BIT	 = 58,
+	ETHTOOL_LINK_MODE_100000baseCR2_Full_BIT	 = 59,
+	ETHTOOL_LINK_MODE_100000baseLR2_ER2_FR2_Full_BIT = 60,
+	ETHTOOL_LINK_MODE_100000baseDR2_Full_BIT	 = 61,
+	ETHTOOL_LINK_MODE_200000baseKR4_Full_BIT	 = 62,
+	ETHTOOL_LINK_MODE_200000baseSR4_Full_BIT	 = 63,
+	ETHTOOL_LINK_MODE_200000baseLR4_ER4_FR4_Full_BIT = 64,
+	ETHTOOL_LINK_MODE_200000baseDR4_Full_BIT	 = 65,
+	ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT	 = 66,
 
 	/* Last allowed bit for __ETHTOOL_LINK_MODE_LEGACY_MASK is bit
 	 * 31. Please do NOT define any SUPPORTED_* or ADVERTISED_*
@@ -1463,7 +1478,7 @@ enum ethtool_link_mode_bit_indices {
 	 */
 
 	__ETHTOOL_LINK_MODE_LAST
-	  = ETHTOOL_LINK_MODE_FEC_BASER_BIT,
+	  = ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT,
 };
 
 #define __ETHTOOL_LINK_MODE_LEGACY_MASK(base_name)	\
@@ -1571,6 +1586,7 @@ enum ethtool_link_mode_bit_indices {
 #define SPEED_50000		50000
 #define SPEED_56000		56000
 #define SPEED_100000		100000
+#define SPEED_200000		200000
 
 #define SPEED_UNKNOWN		-1
 
diff --git a/ethtool.8.in b/ethtool.8.in
index 5a26cff5fb33..64ce0711ad5f 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -650,6 +650,11 @@ lB	l	lB.
 0x400000000	50000baseCR2 Full
 0x800000000	50000baseKR2 Full
 0x10000000000	50000baseSR2 Full
+0x10000000000000	50000baseKR Full
+0x20000000000000	50000baseSR Full
+0x40000000000000	50000baseCR Full
+0x80000000000000	50000baseLR_ER_FR Full
+0x100000000000000	50000baseDR Full
 0x8000000	56000baseKR4 Full
 0x10000000	56000baseCR4 Full
 0x20000000	56000baseSR4 Full
@@ -658,6 +663,16 @@ lB	l	lB.
 0x2000000000	100000baseSR4 Full
 0x4000000000	100000baseCR4 Full
 0x8000000000	100000baseLR4_ER4 Full
+0x200000000000000	100000baseKR2 Full
+0x400000000000000	100000baseSR2 Full
+0x800000000000000	100000baseCR2 Full
+0x1000000000000000	100000baseLR2_ER2_FR2 Full
+0x2000000000000000	100000baseDR2 Full
+0x4000000000000000	200000baseKR4 Full
+0x8000000000000000	200000baseSR4 Full
+0x10000000000000000	200000baseLR4_ER4_FR4 Full
+0x20000000000000000	200000baseDR4 Full
+0x40000000000000000	200000baseCR4 Full
 .TE
 .TP
 .BI phyad \ N
diff --git a/ethtool.c b/ethtool.c
index fb4c0886ca84..c36aafd5bb3a 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -530,6 +530,21 @@ static void init_global_link_mode_masks(void)
 		ETHTOOL_LINK_MODE_10000baseER_Full_BIT,
 		ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
 		ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
+		ETHTOOL_LINK_MODE_50000baseKR_Full_BIT,
+		ETHTOOL_LINK_MODE_50000baseSR_Full_BIT,
+		ETHTOOL_LINK_MODE_50000baseCR_Full_BIT,
+		ETHTOOL_LINK_MODE_50000baseLR_ER_FR_Full_BIT,
+		ETHTOOL_LINK_MODE_50000baseDR_Full_BIT,
+		ETHTOOL_LINK_MODE_100000baseKR2_Full_BIT,
+		ETHTOOL_LINK_MODE_100000baseSR2_Full_BIT,
+		ETHTOOL_LINK_MODE_100000baseCR2_Full_BIT,
+		ETHTOOL_LINK_MODE_100000baseLR2_ER2_FR2_Full_BIT,
+		ETHTOOL_LINK_MODE_100000baseDR2_Full_BIT,
+		ETHTOOL_LINK_MODE_200000baseKR4_Full_BIT,
+		ETHTOOL_LINK_MODE_200000baseSR4_Full_BIT,
+		ETHTOOL_LINK_MODE_200000baseLR4_ER4_FR4_Full_BIT,
+		ETHTOOL_LINK_MODE_200000baseDR4_Full_BIT,
+		ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT,
 	};
 	static const enum ethtool_link_mode_bit_indices
 		additional_advertised_flags_bits[] = {
@@ -689,6 +704,36 @@ static void dump_link_caps(const char *prefix, const char *an_prefix,
 		  "2500baseT/Full" },
 		{ 0, ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
 		  "5000baseT/Full" },
+		{ 0, ETHTOOL_LINK_MODE_50000baseKR_Full_BIT,
+		  "50000baseKR/Full" },
+		{ 0, ETHTOOL_LINK_MODE_50000baseSR_Full_BIT,
+		  "50000baseSR/Full" },
+		{ 0, ETHTOOL_LINK_MODE_50000baseCR_Full_BIT,
+		  "50000baseCR/Full" },
+		{ 0, ETHTOOL_LINK_MODE_50000baseLR_ER_FR_Full_BIT,
+		  "50000baseLR_ER_FR/Full" },
+		{ 0, ETHTOOL_LINK_MODE_50000baseDR_Full_BIT,
+		  "50000baseDR/Full" },
+		{ 0, ETHTOOL_LINK_MODE_100000baseKR2_Full_BIT,
+		  "100000baseKR2/Full" },
+		{ 0, ETHTOOL_LINK_MODE_100000baseSR2_Full_BIT,
+		  "100000baseSR2/Full" },
+		{ 0, ETHTOOL_LINK_MODE_100000baseCR2_Full_BIT,
+		  "100000baseCR2/Full" },
+		{ 0, ETHTOOL_LINK_MODE_100000baseLR2_ER2_FR2_Full_BIT,
+		  "100000baseLR2_ER2_FR2/Full" },
+		{ 0, ETHTOOL_LINK_MODE_100000baseDR2_Full_BIT,
+		  "100000baseDR2/Full" },
+		{ 0, ETHTOOL_LINK_MODE_200000baseKR4_Full_BIT,
+		  "200000baseKR4/Full" },
+		{ 0, ETHTOOL_LINK_MODE_200000baseSR4_Full_BIT,
+		  "200000baseSR4/Full" },
+		{ 0, ETHTOOL_LINK_MODE_200000baseLR4_ER4_FR4_Full_BIT,
+		  "200000baseLR4_ER4_FR4/Full" },
+		{ 0, ETHTOOL_LINK_MODE_200000baseDR4_Full_BIT,
+		  "200000baseDR4/Full" },
+		{ 0, ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT,
+		  "200000baseCR4/Full" },
 	};
 	int indent;
 	int did1, new_line_pend, i;
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCH bpf] bpf: properly check TCP_CONGESTION optlen
From: Eric Dumazet @ 2019-02-25 15:43 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Alexei Starovoitov, Eric Dumazet, Alexei Starovoitov, netdev,
	Martin KaFai Lau, Song Liu, Yonghong Song, bpf, Lawrence Brakmo
In-Reply-To: <739fbe32-d019-4d97-c902-2155a15c58b5@iogearbox.net>

On Mon, Feb 25, 2019 at 3:07 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 02/24/2019 12:11 AM, Alexei Starovoitov wrote:
> > On Sat, Feb 23, 2019 at 12:48:53PM -0800, Eric Dumazet wrote:
> >> On 02/23/2019 12:38 PM, Alexei Starovoitov wrote:
> >>> On Sat, Feb 23, 2019 at 11:07:09AM -0800, Eric Dumazet wrote:
> >>>> If caller of bpf_setsockopt() is silly passing a negative optlen
> >>>> bad things happen.
> >>>>
> >>>> Fixes: 91b5b21c7c16 ("bpf: Add support for changing congestion control")
> >>>> Signed-off-by: Eric Dumazet <edumazet@google.com>
> >>>> Cc: Lawrence Brakmo <brakmo@fb.com>
> >>>> ---
> >>>>  net/core/filter.c | 5 +++--
> >>>>  1 file changed, 3 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/net/core/filter.c b/net/core/filter.c
> >>>> index f7d0004fc16096eb42ece3a6acf645540ee2326b..6a5d89464168f2f35f43986c1dbc0446c9390a3c 100644
> >>>> --- a/net/core/filter.c
> >>>> +++ b/net/core/filter.c
> >>>> @@ -4194,8 +4194,9 @@ BPF_CALL_5(bpf_setsockopt, struct bpf_sock_ops_kern *, bpf_sock,
> >>>>                    char name[TCP_CA_NAME_MAX];
> >>>>                    bool reinit = bpf_sock->op > BPF_SOCK_OPS_NEEDS_ECN;
> >>>>
> >>>> -                  strncpy(name, optval, min_t(long, optlen,
> >>>> -                                              TCP_CA_NAME_MAX-1));
> >>>> +                  if (optlen < 0)
> >>>> +                          return -EINVAL;
> >>>> +                  strncpy(name, optval, min(optlen, TCP_CA_NAME_MAX - 1));
> >>>
> >>> Unnecessary.
> >>> The verifier guarantees that optlen > 0 because
> >>> static const struct bpf_func_proto bpf_setsockopt_proto = {
> >>>         .func           = bpf_setsockopt,
> >>> ...
> >>>         .arg5_type      = ARG_CONST_SIZE,
> >>> };
> >>
> >> Even on 32bit kernel ?
> >>
> >> The suspect thing to me is the min_t(long, ....)
> >>
> >> optlen is an integer, why do we need to promote to a long ?
> >
> > I think the code is actually fine as-is.
> > I bet it was copy pasted from do_tcp_setsockopt
> > where similar min_t(long) is used to match strncpy_from_user() declaration.
> > Here min_t(long) or min_t(int) or min() doesn't matter.
> > I would keep it as-is to avoid noisy patches.
>
> Max allowed input from verifier should be BPF_MAX_VAR_SIZ which is 1 << 29,
> but I totally agree that the bpf_setsockopt() and bpf_getsockopt() signature
> should change into u32 optlen as it's just confusing otherwise, same with the
> long in strncpy(). Agree with Alexei that this might have been a copy-paste
> kind of thing. Lawrence can probably clarify best?
>
> The same wrong assumption is in commit 1e215300f138 ("bpf: add TCP_SAVE_SYN/
> TCP_SAVED_SYN options for bpf_(set|get)sockopt") which tests for optlen <= 0.
> Neither can it be negative nor zero here due to ARG_CONST_SIZE. Given it's
> also confusing others, cleanup might still be worth considering. Maybe Lawrence
> can spin this into one of his next patches, if noone else gets to it first.

Yes, and considering the err_clear: error path does

memset(optval, 0, optlen);

This is even more confusing to think that optlen could be negative or
out of bound ;)

^ permalink raw reply

* [PATCH net-next] net: sched: set dedicated tcf_walker flag when tp is empty
From: Vlad Buslov @ 2019-02-25 15:38 UTC (permalink / raw)
  To: netdev; +Cc: jhs, xiyou.wangcong, jiri, davem, Vlad Buslov

Using tcf_walker->stop flag to determine when tcf_walker->fn() was called
at least once is unreliable. Some classifiers set 'stop' flag on error
before calling walker callback, other classifiers used to call it with NULL
filter pointer when empty. In order to prevent further regressions, extend
tcf_walker structure with dedicated 'nonempty' flag. Set this flag in
tcf_walker->fn() implementation that is used to check if classifier has
filters configured.

Fixes: 8b64678e0af8 ("net: sched: refactor tp insert/delete for concurrent execution")
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Suggested-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 include/net/pkt_cls.h |  1 +
 net/sched/cls_api.c   | 13 +++++++++----
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 232f801f2a21..422dd8800478 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -17,6 +17,7 @@ struct tcf_walker {
 	int	stop;
 	int	skip;
 	int	count;
+	bool	nonempty;
 	unsigned long cookie;
 	int	(*fn)(struct tcf_proto *, void *node, struct tcf_walker *);
 };
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index e2c888961379..3543be31d400 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -238,18 +238,23 @@ static void tcf_proto_put(struct tcf_proto *tp, bool rtnl_held,
 		tcf_proto_destroy(tp, rtnl_held, extack);
 }
 
-static int walker_noop(struct tcf_proto *tp, void *d, struct tcf_walker *arg)
+static int walker_check_empty(struct tcf_proto *tp, void *d,
+			      struct tcf_walker *arg)
 {
-	return -1;
+	if (tp) {
+		arg->nonempty = true;
+		return -1;
+	}
+	return 0;
 }
 
 static bool tcf_proto_is_empty(struct tcf_proto *tp, bool rtnl_held)
 {
-	struct tcf_walker walker = { .fn = walker_noop, };
+	struct tcf_walker walker = { .fn = walker_check_empty, };
 
 	if (tp->ops->walk) {
 		tp->ops->walk(tp, &walker, rtnl_held);
-		return !walker.stop;
+		return !walker.nonempty;
 	}
 	return true;
 }
-- 
2.13.6


^ permalink raw reply related

* Re: [PATCH net] net: sched: act_tunnel_key: fix NULL pointer dereference during init
From: Davide Caratti @ 2019-02-25 15:36 UTC (permalink / raw)
  To: Vlad Buslov, netdev; +Cc: jhs, xiyou.wangcong, jiri, davem, wenxu, roid
In-Reply-To: <20190225152827.8741-1-vladbu@mellanox.com>

On Mon, 2019-02-25 at 17:28 +0200, Vlad Buslov wrote:
> Metadata pointer is only initialized for action TCA_TUNNEL_KEY_ACT_SET, but
> it is unconditionally dereferenced in tunnel_key_init() error handler.
> Verify that metadata pointer is not NULL before dereferencing it in
> tunnel_key_init error handling code.
> 
> Fixes: ee28bb56ac5b ("net/sched: fix memory leak in act_tunnel_key_init()")
> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
> ---
>  net/sched/act_tunnel_key.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
> index 8b43fe0130f7..3f943de9a2c9 100644
> --- a/net/sched/act_tunnel_key.c
> +++ b/net/sched/act_tunnel_key.c
> @@ -377,7 +377,8 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
>  	return ret;
>  
>  release_tun_meta:
> -	dst_release(&metadata->dst);
> +	if (metadata)
> +		dst_release(&metadata->dst);
>  
>  err_out:
>  	if (exists)

Reviewed-by: Davide Caratti <dcaratti@redhat.com>


^ permalink raw reply

* [PATCH net-next v2] net: sched: act_tunnel_key: fix metadata handling
From: Vlad Buslov @ 2019-02-25 15:30 UTC (permalink / raw)
  To: netdev, dcaratti
  Cc: jhs, xiyou.wangcong, jiri, davem, wenxu, roid, Vlad Buslov
In-Reply-To: <4bde1d403d4ba9b51cf18bbaac1d46147011b959.camel@redhat.com>

Tunnel key action params->tcft_enc_metadata is only set when action is
TCA_TUNNEL_KEY_ACT_SET. However, metadata pointer is incorrectly
dereferenced during tunnel key init and release without verifying that
action is if correct type, which causes NULL pointer dereference. Metadata
tunnel dst_cache is also leaked on action overwrite.

Fix metadata handling:
- Verify that metadata pointer is not NULL before dereferencing it in
  tunnel_key_init error handling code.
- Move dst_cache destroy code into tunnel_key_release_params() function
  that is called in both action overwrite and release cases (fixes resource
  leak) and verifies that actions has correct type before dereferencing
  metadata pointer (fixes NULL pointer dereference).

Oops with KASAN enabled during tdc tests execution:

[  261.080482] ==================================================================
[  261.088049] BUG: KASAN: null-ptr-deref in dst_cache_destroy+0x21/0xa0
[  261.094613] Read of size 8 at addr 00000000000000b0 by task tc/2976
[  261.102524] CPU: 14 PID: 2976 Comm: tc Not tainted 5.0.0-rc7+ #157
[  261.108844] Hardware name: Supermicro SYS-2028TP-DECR/X10DRT-P, BIOS 2.0b 03/30/2017
[  261.116726] Call Trace:
[  261.119234]  dump_stack+0x9a/0xeb
[  261.122625]  ? dst_cache_destroy+0x21/0xa0
[  261.126818]  ? dst_cache_destroy+0x21/0xa0
[  261.131004]  kasan_report+0x176/0x192
[  261.134752]  ? idr_get_next+0xd0/0x120
[  261.138578]  ? dst_cache_destroy+0x21/0xa0
[  261.142768]  dst_cache_destroy+0x21/0xa0
[  261.146799]  tunnel_key_release+0x3a/0x50 [act_tunnel_key]
[  261.152392]  tcf_action_cleanup+0x2c/0xc0
[  261.156490]  tcf_generic_walker+0x4c2/0x5c0
[  261.160794]  ? tcf_action_dump_1+0x390/0x390
[  261.165163]  ? tunnel_key_walker+0x5/0x1a0 [act_tunnel_key]
[  261.170865]  ? tunnel_key_walker+0xe9/0x1a0 [act_tunnel_key]
[  261.176641]  tca_action_gd+0x600/0xa40
[  261.180482]  ? tca_get_fill.constprop.17+0x200/0x200
[  261.185548]  ? __lock_acquire+0x588/0x1d20
[  261.189741]  ? __lock_acquire+0x588/0x1d20
[  261.193922]  ? mark_held_locks+0x90/0x90
[  261.197944]  ? mark_held_locks+0x90/0x90
[  261.202018]  ? __nla_parse+0xfe/0x190
[  261.205774]  tc_ctl_action+0x218/0x230
[  261.209614]  ? tcf_action_add+0x230/0x230
[  261.213726]  rtnetlink_rcv_msg+0x3a5/0x600
[  261.217910]  ? lock_downgrade+0x2d0/0x2d0
[  261.222006]  ? validate_linkmsg+0x400/0x400
[  261.226278]  ? find_held_lock+0x6d/0xd0
[  261.230200]  ? match_held_lock+0x1b/0x210
[  261.234296]  ? validate_linkmsg+0x400/0x400
[  261.238567]  netlink_rcv_skb+0xc7/0x1f0
[  261.242489]  ? netlink_ack+0x470/0x470
[  261.246319]  ? netlink_deliver_tap+0x1f3/0x5a0
[  261.250874]  netlink_unicast+0x2ae/0x350
[  261.254884]  ? netlink_attachskb+0x340/0x340
[  261.261647]  ? _copy_from_iter_full+0xdd/0x380
[  261.268576]  ? __virt_addr_valid+0xb6/0xf0
[  261.275227]  ? __check_object_size+0x159/0x240
[  261.282184]  netlink_sendmsg+0x4d3/0x630
[  261.288572]  ? netlink_unicast+0x350/0x350
[  261.295132]  ? netlink_unicast+0x350/0x350
[  261.301608]  sock_sendmsg+0x6d/0x80
[  261.307467]  ___sys_sendmsg+0x48e/0x540
[  261.313633]  ? copy_msghdr_from_user+0x210/0x210
[  261.320545]  ? save_stack+0x89/0xb0
[  261.326289]  ? __lock_acquire+0x588/0x1d20
[  261.332605]  ? entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  261.340063]  ? mark_held_locks+0x90/0x90
[  261.346162]  ? do_filp_open+0x138/0x1d0
[  261.352108]  ? may_open_dev+0x50/0x50
[  261.357897]  ? match_held_lock+0x1b/0x210
[  261.364016]  ? __fget_light+0xa6/0xe0
[  261.369840]  ? __sys_sendmsg+0xd2/0x150
[  261.375814]  __sys_sendmsg+0xd2/0x150
[  261.381610]  ? __ia32_sys_shutdown+0x30/0x30
[  261.388026]  ? lock_downgrade+0x2d0/0x2d0
[  261.394182]  ? mark_held_locks+0x1c/0x90
[  261.400230]  ? do_syscall_64+0x1e/0x280
[  261.406172]  do_syscall_64+0x78/0x280
[  261.411932]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  261.419103] RIP: 0033:0x7f28e91a8b87
[  261.424791] Code: 64 89 02 48 c7 c0 ff ff ff ff eb b9 0f 1f 80 00 00 00 00 8b 05 6a 2b 2c 00 48 63 d2 48 63 ff 85 c0 75 18 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 59 f3 c3 0f 1f 80 00 00 00 00 53 48 89 f3 48
[  261.448226] RSP: 002b:00007ffdc5c4e2d8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
[  261.458183] RAX: ffffffffffffffda RBX: 000000005c73c202 RCX: 00007f28e91a8b87
[  261.467728] RDX: 0000000000000000 RSI: 00007ffdc5c4e340 RDI: 0000000000000003
[  261.477342] RBP: 0000000000000000 R08: 0000000000000001 R09: 000000000000000c
[  261.486970] R10: 000000000000000c R11: 0000000000000246 R12: 0000000000000001
[  261.496599] R13: 000000000067b4e0 R14: 00007ffdc5c5248c R15: 00007ffdc5c52480
[  261.506281] ==================================================================
[  261.516076] Disabling lock debugging due to kernel taint
[  261.523979] BUG: unable to handle kernel NULL pointer dereference at 00000000000000b0
[  261.534413] #PF error: [normal kernel read fault]
[  261.541730] PGD 8000000317400067 P4D 8000000317400067 PUD 316878067 PMD 0
[  261.551294] Oops: 0000 [#1] SMP KASAN PTI
[  261.557985] CPU: 14 PID: 2976 Comm: tc Tainted: G    B             5.0.0-rc7+ #157
[  261.568306] Hardware name: Supermicro SYS-2028TP-DECR/X10DRT-P, BIOS 2.0b 03/30/2017
[  261.578874] RIP: 0010:dst_cache_destroy+0x21/0xa0
[  261.586413] Code: f4 ff ff ff eb f6 0f 1f 00 0f 1f 44 00 00 41 56 41 55 49 c7 c6 60 fe 35 af 41 54 55 49 89 fc 53 bd ff ff ff ff e8 ef 98 73 ff <49> 83 3c 24 00 75 35 eb 6c 4c 63 ed e8 de 98 73 ff 4a 8d 3c ed 40
[  261.611247] RSP: 0018:ffff888316447160 EFLAGS: 00010282
[  261.619564] RAX: 0000000000000000 RBX: ffff88835b3e2f00 RCX: ffffffffad1c5071
[  261.629862] RDX: 0000000000000003 RSI: dffffc0000000000 RDI: 0000000000000297
[  261.640149] RBP: 00000000ffffffff R08: fffffbfff5dd4e89 R09: fffffbfff5dd4e89
[  261.650467] R10: 0000000000000001 R11: fffffbfff5dd4e88 R12: 00000000000000b0
[  261.660785] R13: ffff8883267a10c0 R14: ffffffffaf35fe60 R15: 0000000000000001
[  261.671110] FS:  00007f28ea3e6400(0000) GS:ffff888364200000(0000) knlGS:0000000000000000
[  261.682447] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  261.691491] CR2: 00000000000000b0 CR3: 00000003178ae004 CR4: 00000000001606e0
[  261.701283] Call Trace:
[  261.706374]  tunnel_key_release+0x3a/0x50 [act_tunnel_key]
[  261.714522]  tcf_action_cleanup+0x2c/0xc0
[  261.721208]  tcf_generic_walker+0x4c2/0x5c0
[  261.728074]  ? tcf_action_dump_1+0x390/0x390
[  261.734996]  ? tunnel_key_walker+0x5/0x1a0 [act_tunnel_key]
[  261.743247]  ? tunnel_key_walker+0xe9/0x1a0 [act_tunnel_key]
[  261.751557]  tca_action_gd+0x600/0xa40
[  261.757991]  ? tca_get_fill.constprop.17+0x200/0x200
[  261.765644]  ? __lock_acquire+0x588/0x1d20
[  261.772461]  ? __lock_acquire+0x588/0x1d20
[  261.779266]  ? mark_held_locks+0x90/0x90
[  261.785880]  ? mark_held_locks+0x90/0x90
[  261.792470]  ? __nla_parse+0xfe/0x190
[  261.798738]  tc_ctl_action+0x218/0x230
[  261.805145]  ? tcf_action_add+0x230/0x230
[  261.811760]  rtnetlink_rcv_msg+0x3a5/0x600
[  261.818564]  ? lock_downgrade+0x2d0/0x2d0
[  261.825433]  ? validate_linkmsg+0x400/0x400
[  261.832256]  ? find_held_lock+0x6d/0xd0
[  261.838624]  ? match_held_lock+0x1b/0x210
[  261.845142]  ? validate_linkmsg+0x400/0x400
[  261.851729]  netlink_rcv_skb+0xc7/0x1f0
[  261.857976]  ? netlink_ack+0x470/0x470
[  261.864132]  ? netlink_deliver_tap+0x1f3/0x5a0
[  261.870969]  netlink_unicast+0x2ae/0x350
[  261.877294]  ? netlink_attachskb+0x340/0x340
[  261.883962]  ? _copy_from_iter_full+0xdd/0x380
[  261.890750]  ? __virt_addr_valid+0xb6/0xf0
[  261.897188]  ? __check_object_size+0x159/0x240
[  261.903928]  netlink_sendmsg+0x4d3/0x630
[  261.910112]  ? netlink_unicast+0x350/0x350
[  261.916410]  ? netlink_unicast+0x350/0x350
[  261.922656]  sock_sendmsg+0x6d/0x80
[  261.928257]  ___sys_sendmsg+0x48e/0x540
[  261.934183]  ? copy_msghdr_from_user+0x210/0x210
[  261.940865]  ? save_stack+0x89/0xb0
[  261.946355]  ? __lock_acquire+0x588/0x1d20
[  261.952358]  ? entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  261.959468]  ? mark_held_locks+0x90/0x90
[  261.965248]  ? do_filp_open+0x138/0x1d0
[  261.970910]  ? may_open_dev+0x50/0x50
[  261.976386]  ? match_held_lock+0x1b/0x210
[  261.982210]  ? __fget_light+0xa6/0xe0
[  261.987648]  ? __sys_sendmsg+0xd2/0x150
[  261.993263]  __sys_sendmsg+0xd2/0x150
[  261.998613]  ? __ia32_sys_shutdown+0x30/0x30
[  262.004555]  ? lock_downgrade+0x2d0/0x2d0
[  262.010236]  ? mark_held_locks+0x1c/0x90
[  262.015758]  ? do_syscall_64+0x1e/0x280
[  262.021234]  do_syscall_64+0x78/0x280
[  262.026500]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  262.033207] RIP: 0033:0x7f28e91a8b87
[  262.038421] Code: 64 89 02 48 c7 c0 ff ff ff ff eb b9 0f 1f 80 00 00 00 00 8b 05 6a 2b 2c 00 48 63 d2 48 63 ff 85 c0 75 18 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 59 f3 c3 0f 1f 80 00 00 00 00 53 48 89 f3 48
[  262.060708] RSP: 002b:00007ffdc5c4e2d8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
[  262.070112] RAX: ffffffffffffffda RBX: 000000005c73c202 RCX: 00007f28e91a8b87
[  262.079087] RDX: 0000000000000000 RSI: 00007ffdc5c4e340 RDI: 0000000000000003
[  262.088122] RBP: 0000000000000000 R08: 0000000000000001 R09: 000000000000000c
[  262.097157] R10: 000000000000000c R11: 0000000000000246 R12: 0000000000000001
[  262.106207] R13: 000000000067b4e0 R14: 00007ffdc5c5248c R15: 00007ffdc5c52480
[  262.115271] Modules linked in: act_tunnel_key act_skbmod act_simple act_connmark nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 act_csum libcrc32c act_meta_skbtcindex act_meta_skbprio act_meta_mark act_ife ife act_police act_sample psample act_gact veth nfsv3 nfs_acl nfs lockd grace fscache bridge stp llc intel_rapl sb_edac mlx5_ib x86_pkg_temp_thermal sunrpc intel_powerclamp coretemp ib_uverbs kvm_intel ib_core kvm irqbypass mlx5_core crct10dif_pclmul crc32_pclmul crc32c_intel igb ghash_clmulni_intel intel_cstate mlxfw iTCO_wdt devlink intel_uncore iTCO_vendor_support ipmi_ssif ptp mei_me intel_rapl_perf ioatdma joydev pps_core ses mei i2c_i801 pcspkr enclosure lpc_ich dca wmi ipmi_si ipmi_devintf ipmi_msghandler acpi_pad acpi_power_meter pcc_cpufreq ast i2c_algo_bit drm_kms_helper ttm drm mpt3sas raid_class scsi_transport_sas
[  262.204393] CR2: 00000000000000b0
[  262.210390] ---[ end trace 2e41d786f2c7901a ]---
[  262.226790] RIP: 0010:dst_cache_destroy+0x21/0xa0
[  262.234083] Code: f4 ff ff ff eb f6 0f 1f 00 0f 1f 44 00 00 41 56 41 55 49 c7 c6 60 fe 35 af 41 54 55 49 89 fc 53 bd ff ff ff ff e8 ef 98 73 ff <49> 83 3c 24 00 75 35 eb 6c 4c 63 ed e8 de 98 73 ff 4a 8d 3c ed 40
[  262.258311] RSP: 0018:ffff888316447160 EFLAGS: 00010282
[  262.266304] RAX: 0000000000000000 RBX: ffff88835b3e2f00 RCX: ffffffffad1c5071
[  262.276251] RDX: 0000000000000003 RSI: dffffc0000000000 RDI: 0000000000000297
[  262.286208] RBP: 00000000ffffffff R08: fffffbfff5dd4e89 R09: fffffbfff5dd4e89
[  262.296183] R10: 0000000000000001 R11: fffffbfff5dd4e88 R12: 00000000000000b0
[  262.306157] R13: ffff8883267a10c0 R14: ffffffffaf35fe60 R15: 0000000000000001
[  262.316139] FS:  00007f28ea3e6400(0000) GS:ffff888364200000(0000) knlGS:0000000000000000
[  262.327146] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  262.335815] CR2: 00000000000000b0 CR3: 00000003178ae004 CR4: 00000000001606e0

Fixes: 41411e2fd6b8 ("net/sched: act_tunnel_key: Add dst_cache support")
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
Changes from V1 to V2:
- Extract metadata->dst error handler fix into standalone patch that targets
  net.

 net/sched/act_tunnel_key.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index 3404af72b4c1..fc2b884b170c 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -201,8 +201,14 @@ static void tunnel_key_release_params(struct tcf_tunnel_key_params *p)
 {
 	if (!p)
 		return;
-	if (p->tcft_action == TCA_TUNNEL_KEY_ACT_SET)
+	if (p->tcft_action == TCA_TUNNEL_KEY_ACT_SET) {
+#ifdef CONFIG_DST_CACHE
+		struct ip_tunnel_info *info = &p->tcft_enc_metadata->u.tun_info;
+
+		dst_cache_destroy(&info->dst_cache);
+#endif
 		dst_release(&p->tcft_enc_metadata->dst);
+	}
 	kfree_rcu(p, rcu);
 }
 
@@ -384,7 +390,8 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
 
 release_dst_cache:
 #ifdef CONFIG_DST_CACHE
-	dst_cache_destroy(&metadata->u.tun_info.dst_cache);
+	if (metadata)
+		dst_cache_destroy(&metadata->u.tun_info.dst_cache);
 #endif
 release_tun_meta:
 	dst_release(&metadata->dst);
@@ -401,15 +408,8 @@ static void tunnel_key_release(struct tc_action *a)
 {
 	struct tcf_tunnel_key *t = to_tunnel_key(a);
 	struct tcf_tunnel_key_params *params;
-#ifdef CONFIG_DST_CACHE
-	struct ip_tunnel_info *info;
-#endif
 
 	params = rcu_dereference_protected(t->params, 1);
-#ifdef CONFIG_DST_CACHE
-	info = &params->tcft_enc_metadata->u.tun_info;
-	dst_cache_destroy(&info->dst_cache);
-#endif
 	tunnel_key_release_params(params);
 }
 
-- 
2.13.6


^ permalink raw reply related

* Re: [stable 3.18 backport v2] netlink: Trim skb to alloc size to avoid MSG_TRUNC
From: Greg Kroah-Hartman @ 2019-02-25 15:30 UTC (permalink / raw)
  To: Mark Salyzyn
  Cc: linux-kernel, Arad, Ronen, David S . Miller, Dmitry Safonov,
	David Ahern, Kirill Tkhai, Andrei Vagin, Li RongQing, YU Bo,
	Denys Vlasenko, netdev, stable, Eric Dumazet, Alexander Potapenko
In-Reply-To: <20190222160330.34237-1-salyzyn@android.com>

On Fri, Feb 22, 2019 at 08:03:28AM -0800, Mark Salyzyn wrote:
> From: "Arad, Ronen" <ronen.arad@intel.com>
> 
> Direct this upstream db65a3aaf29ecce2e34271d52e8d2336b97bd9fe sha to
> stable 3.18.  This patch addresses a race condition where a call to
> 
>  nlk->max_recvmsg_len = max(nlk->max_recvmsg_len, len);
>  nlk->max_recvmsg_len = min_t(size_t, nlk->max_recvmsg_len,
> 
> one thread in-between another thread:
> 
>  skb = netlink_alloc_skb(sk,
> 
> and
> 
>  skb_reserve(skb, skb_tailroom(skb) -
>              nlk->max_recvmsg_len);
> 
> in netlink_dump.  The result can be a negative value and will cause
> a kernel panic ad BUG at net/core/skbuff.c because the negative value
> turns into an extremely large positive value.
> 
> Original commit:
> 
> netlink_dump() allocates skb based on the calculated min_dump_alloc or
> a per socket max_recvmsg_len.
> min_alloc_size is maximum space required for any single netdev
> attributes as calculated by rtnl_calcit().
> max_recvmsg_len tracks the user provided buffer to netlink_recvmsg.
> It is capped at 16KiB.
> The intention is to avoid small allocations and to minimize the number
> of calls required to obtain dump information for all net devices.
> 
> netlink_dump packs as many small messages as could fit within an skb
> that was sized for the largest single netdev information. The actual
> space available within an skb is larger than what is requested. It could
> be much larger and up to near 2x with align to next power of 2 approach.
> 
> Allowing netlink_dump to use all the space available within the
> allocated skb increases the buffer size a user has to provide to avoid
> truncaion (i.e. MSG_TRUNG flag set).
> 
> It was observed that with many VLANs configured on at least one netdev,
> a larger buffer of near 64KiB was necessary to avoid "Message truncated"
> error in "ip link" or "bridge [-c[ompressvlans]] vlan show" when
> min_alloc_size was only little over 32KiB.
> 
> This patch trims skb to allocated size in order to allow the user to
> avoid truncation with more reasonable buffer size.
> 
> Signed-off-by: Ronen Arad <ronen.arad@intel.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> 
> (cherry pick commit db65a3aaf29ecce2e34271d52e8d2336b97bd9fe)
> Signed-off-by: Mark Salyzyn <salyzyn@android.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Ronen Arad <ronen.arad@intel.com>
> Cc: "David S . Miller" <davem@davemloft.net>
> Cc: Dmitry Safonov <dima@arista.com>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
> Cc: Andrei Vagin <avagin@virtuozzo.com>
> Cc: Li RongQing <lirongqing@baidu.com>
> Cc: YU Bo <tsu.yubo@gmail.com>
> Cc: Denys Vlasenko <dvlasenk@redhat.com>
> Cc: netdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: stable@vger.kernel.org # 3.18
> ---
>  net/netlink/af_netlink.c | 34 ++++++++++++++++++++++------------
>  1 file changed, 22 insertions(+), 12 deletions(-)

Now queued up, thanks.

greg k-h

^ permalink raw reply

* Re: [PATCH ethtool] ethtool: Add support for 200Gbps (50Gbps per lane) link mode
From: Tariq Toukan @ 2019-02-25 15:30 UTC (permalink / raw)
  To: Andrew Lunn, Michal Kubecek
  Cc: Tariq Toukan, John W. Linville, netdev@vger.kernel.org,
	Eran Ben Elisha, Aya Levin
In-Reply-To: <20190224194015.GM26626@lunn.ch>



On 2/24/2019 9:40 PM, Andrew Lunn wrote:
>>> This is getting less friendly all the time, and it was never very
>>> friendly to start with. We have the strings which represent these link
>>> modes in the table used for dumping caps. How about allowing the user
>>> to list a comma separate list of modes.
>>>
>>> ethtool -s lan42 advertise 100000baseKR2/Full,100000baseSR2/Full,100000baseCR2/Full
>>
>> In my preliminary netlink patchset, I'm adding support for e.g.
>>
>>    ethtool -s eth0 advertise 100baseT/Half off 1000baseT/Full on
>>
>> I'm not sure what would be more useful, if switching individual modes or
>> listing the whole set. After all, we can also support both. But I fully
>> agree that the numeric bitmaps are already too inconvenient.
> 
> Hi Michal
> 
> So are you doing a read/modify/write? In that case, off/on makes
> sense. For a pure write, i don't see the need for off/on.
> 
> I've not had to use this much, so i don't know how it is typically
> used. When i have used it, it is because i've got an SFP which can do
> 1G and 2.5G, but the peer can only do 1G. I've needed to remove the
> 2.5G in order to get link. So in that case, read/modify/write with an
> off would make sense.
> 
>      Andrew
> 
>> And there is another problem:
>>
>> +		else if (speed_wanted == SPEED_20000 &&
>> +			 duplex_wanted == DUPLEX_FULL)
>> +			adv_bit = ETHTOOL_LINK_MODE_200000baseKR4_Full_BIT &
>> +				  ETHTOOL_LINK_MODE_200000baseSR4_Full_BIT &
>> +				  ETHTOOL_LINK_MODE_200000baseLR4_ER4_FR4_Full_BIT &
>> +				  ETHTOOL_LINK_MODE_200000baseDR4_Full_BIT &
>> +				  ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT;
>>
>> The test is for SPEED_20000 but then 200G modes are added.
> 
> Oh, yes. Easy to miss. Maybe we should consider adding aliases,
> #define SPEED_200G SPEED_200000, and

Totally agree. This will help.


> 
> #define ETHTOOL_LINK_MODE_200GbaseCR4_Full_BIT ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT

In case you do it please consider adding an underscore after the speed, 
so it becomes
ETHTOOL_LINK_MODE_200G_baseCR4_Full_BIT.
I think it's more convenient for the human eye.

^ permalink raw reply

* [PATCH net] net: sched: act_tunnel_key: fix NULL pointer dereference during init
From: Vlad Buslov @ 2019-02-25 15:28 UTC (permalink / raw)
  To: netdev, dcaratti
  Cc: jhs, xiyou.wangcong, jiri, davem, wenxu, roid, Vlad Buslov
In-Reply-To: <4bde1d403d4ba9b51cf18bbaac1d46147011b959.camel@redhat.com>

Metadata pointer is only initialized for action TCA_TUNNEL_KEY_ACT_SET, but
it is unconditionally dereferenced in tunnel_key_init() error handler.
Verify that metadata pointer is not NULL before dereferencing it in
tunnel_key_init error handling code.

Fixes: ee28bb56ac5b ("net/sched: fix memory leak in act_tunnel_key_init()")
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
 net/sched/act_tunnel_key.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index 8b43fe0130f7..3f943de9a2c9 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -377,7 +377,8 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
 	return ret;
 
 release_tun_meta:
-	dst_release(&metadata->dst);
+	if (metadata)
+		dst_release(&metadata->dst);
 
 err_out:
 	if (exists)
-- 
2.13.6


^ permalink raw reply related

* Re: [PATCH net] ipvs: get sctphdr by sctphoff in sctp_csum_check
From: Marcelo Ricardo Leitner @ 2019-02-25 15:28 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, netfilter-devel, Neil Horman, pablo
In-Reply-To: <cd1c829cfd9ee916e57c51f6879947f0f88c1eb1.1551094063.git.lucien.xin@gmail.com>

On Mon, Feb 25, 2019 at 07:27:43PM +0800, Xin Long wrote:
> sctp_csum_check() is called by sctp_s/dnat_handler() where it calls
> skb_make_writable() to ensure sctphdr to be linearized.
> 
> So there's no need to get sctphdr by calling skb_header_pointer()
> in sctp_csum_check().
> 
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

> ---
>  net/netfilter/ipvs/ip_vs_proto_sctp.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c b/net/netfilter/ipvs/ip_vs_proto_sctp.c
> index b0cd7d0..0ecf241 100644
> --- a/net/netfilter/ipvs/ip_vs_proto_sctp.c
> +++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c
> @@ -183,7 +183,7 @@ static int
>  sctp_csum_check(int af, struct sk_buff *skb, struct ip_vs_protocol *pp)
>  {
>  	unsigned int sctphoff;
> -	struct sctphdr *sh, _sctph;
> +	struct sctphdr *sh;
>  	__le32 cmp, val;
>  
>  #ifdef CONFIG_IP_VS_IPV6
> @@ -193,10 +193,7 @@ sctp_csum_check(int af, struct sk_buff *skb, struct ip_vs_protocol *pp)
>  #endif
>  		sctphoff = ip_hdrlen(skb);
>  
> -	sh = skb_header_pointer(skb, sctphoff, sizeof(_sctph), &_sctph);
> -	if (sh == NULL)
> -		return 0;
> -
> +	sh = (struct sctphdr *)(skb->data + sctphoff);
>  	cmp = sh->checksum;
>  	val = sctp_compute_cksum(skb, sctphoff);
>  
> -- 
> 2.1.0
> 

^ permalink raw reply

* Re: [PATCH net-next] net: sched: act_tunnel_key: fix metadata handling
From: Vlad Buslov @ 2019-02-25 15:12 UTC (permalink / raw)
  To: Davide Caratti
  Cc: netdev@vger.kernel.org, jhs@mojatatu.com,
	xiyou.wangcong@gmail.com, jiri@resnulli.us, davem@davemloft.net,
	wenxu@ucloud.cn, Roi Dayan
In-Reply-To: <4bde1d403d4ba9b51cf18bbaac1d46147011b959.camel@redhat.com>


On Mon 25 Feb 2019 at 14:04, Davide Caratti <dcaratti@redhat.com> wrote:
> On Mon, 2019-02-25 at 14:21 +0200, Vlad Buslov wrote:
>> Tunnel key action params->tcft_enc_metadata is only set when action is
>> TCA_TUNNEL_KEY_ACT_SET. However, metadata pointer is incorrectly
>> dereferenced during tunnel key init and release without verifying that
>> action is if correct type, which causes NULL pointer dereference. Metadata
>> tunnel dst_cache is also leaked on action overwrite.
>> 
>> Fix metadata handling:
>> - Verify that metadata pointer is not NULL before dereferencing it in
>>   tunnel_key_init error handling code.
>
> hello Vlad,
>
> thanks a lot for fixing this!
>
> <...>
>  
>> @@ -384,10 +390,12 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
>>  
>>  release_dst_cache:
>>  #ifdef CONFIG_DST_CACHE
>> -	dst_cache_destroy(&metadata->u.tun_info.dst_cache);
>> +	if (metadata)
>> +		dst_cache_destroy(&metadata->u.tun_info.dst_cache);
>>  #endif
>>  release_tun_meta:
>> -	dst_release(&metadata->dst);
>> +	if (metadata)
>> +		dst_release(&metadata->dst);
>
> on Linux 'net' tree we don't have commit 41411e2fd6b8 ("net/sched:
> act_tunnel_key: Add dst_cache support"), but still the above two lines can
> avoid a NULL dereference in tunnel_key_init() error path, in the following
> case:
>
> * create an action with tunnel "set", with success
> * replace the previous rule rule with tunnel "unset", and have a failure 
> here (e.g. allocation of 'params_new').
>
> At the cost of creating some conflicts during the merge, it would probably
> be safer to split this commit into two parts, one targeting 'net' and one
> targeting 'net-next', so that the first one can be proposed for stable
> backports (and also I can rebase/retest my 'goto chain' series on top of
> it :) )
>
> WDYT?

Makes sense. I'll send split patches.

^ permalink raw reply

* [PATCH net v3 1/2] selftests: pmtu: disable DAD in all namespaces
From: Paolo Abeni @ 2019-02-25 15:08 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller, David Ahern, Stefano Brivio, Sabrina Dubroca
In-Reply-To: <cover.1551106591.git.pabeni@redhat.com>

Otherwise, the configured IPv6 address could be still "tentative"
at test time, possibly causing tests failures.
We can also drop some sleep along the code and decrease the
timeout for most commands so that the test runtime decreases.

v1 -> v2:
 - fix comment (Stefano)

Fixes: d1f1b9cbf34c ("selftests: net: Introduce first PMTU test")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: David Ahern <dsahern@gmail.com>
---
 tools/testing/selftests/net/pmtu.sh | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
index e2c94e47707c..89aec2fdf4fa 100755
--- a/tools/testing/selftests/net/pmtu.sh
+++ b/tools/testing/selftests/net/pmtu.sh
@@ -263,8 +263,6 @@ setup_fou_or_gue() {
 
 	${ns_a} ip link set ${encap}_a up
 	${ns_b} ip link set ${encap}_b up
-
-	sleep 1
 }
 
 setup_fou44() {
@@ -302,6 +300,10 @@ setup_gue66() {
 setup_namespaces() {
 	for n in ${NS_A} ${NS_B} ${NS_R1} ${NS_R2}; do
 		ip netns add ${n} || return 1
+
+		# Disable DAD, so that we don't have to wait to use the
+		# configured IPv6 addresses
+		ip netns exec ${n} sysctl -q net/ipv6/conf/default/accept_dad=0
 	done
 }
 
@@ -337,8 +339,6 @@ setup_vti() {
 
 	${ns_a} ip link set vti${proto}_a up
 	${ns_b} ip link set vti${proto}_b up
-
-	sleep 1
 }
 
 setup_vti4() {
@@ -375,8 +375,6 @@ setup_vxlan_or_geneve() {
 
 	${ns_a} ip link set ${type}_a up
 	${ns_b} ip link set ${type}_b up
-
-	sleep 1
 }
 
 setup_geneve4() {
@@ -588,8 +586,8 @@ test_pmtu_ipvX() {
 	mtu "${ns_b}"  veth_B-R2 1500
 
 	# Create route exceptions
-	${ns_a} ${ping} -q -M want -i 0.1 -w 2 -s 1800 ${dst1} > /dev/null
-	${ns_a} ${ping} -q -M want -i 0.1 -w 2 -s 1800 ${dst2} > /dev/null
+	${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s 1800 ${dst1} > /dev/null
+	${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s 1800 ${dst2} > /dev/null
 
 	# Check that exceptions have been created with the correct PMTU
 	pmtu_1="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst1})"
@@ -621,7 +619,7 @@ test_pmtu_ipvX() {
 	# Decrease remote MTU on path via R2, get new exception
 	mtu "${ns_r2}" veth_R2-B 400
 	mtu "${ns_b}"  veth_B-R2 400
-	${ns_a} ${ping} -q -M want -i 0.1 -w 2 -s 1400 ${dst2} > /dev/null
+	${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s 1400 ${dst2} > /dev/null
 	pmtu_2="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst2})"
 	check_pmtu_value "lock 552" "${pmtu_2}" "exceeding MTU, with MTU < min_pmtu" || return 1
 
@@ -638,7 +636,7 @@ test_pmtu_ipvX() {
 	check_pmtu_value "1500" "${pmtu_2}" "increasing local MTU" || return 1
 
 	# Get new exception
-	${ns_a} ${ping} -q -M want -i 0.1 -w 2 -s 1400 ${dst2} > /dev/null
+	${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s 1400 ${dst2} > /dev/null
 	pmtu_2="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst2})"
 	check_pmtu_value "lock 552" "${pmtu_2}" "exceeding MTU, with MTU < min_pmtu" || return 1
 }
@@ -687,7 +685,7 @@ test_pmtu_ipvX_over_vxlanY_or_geneveY_exception() {
 
 	mtu "${ns_a}" ${type}_a $((${ll_mtu} + 1000))
 	mtu "${ns_b}" ${type}_b $((${ll_mtu} + 1000))
-	${ns_a} ${ping} -q -M want -i 0.1 -w 2 -s $((${ll_mtu} + 500)) ${dst} > /dev/null
+	${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s $((${ll_mtu} + 500)) ${dst} > /dev/null
 
 	# Check that exception was created
 	pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst})"
@@ -767,7 +765,7 @@ test_pmtu_ipvX_over_fouY_or_gueY() {
 
 	mtu "${ns_a}" ${encap}_a $((${ll_mtu} + 1000))
 	mtu "${ns_b}" ${encap}_b $((${ll_mtu} + 1000))
-	${ns_a} ${ping} -q -M want -i 0.1 -w 2 -s $((${ll_mtu} + 500)) ${dst} > /dev/null
+	${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s $((${ll_mtu} + 500)) ${dst} > /dev/null
 
 	# Check that exception was created
 	pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst})"
@@ -825,13 +823,13 @@ test_pmtu_vti4_exception() {
 
 	# Send DF packet without exceeding link layer MTU, check that no
 	# exception is created
-	${ns_a} ping -q -M want -i 0.1 -w 2 -s ${ping_payload} ${tunnel4_b_addr} > /dev/null
+	${ns_a} ping -q -M want -i 0.1 -w 1 -s ${ping_payload} ${tunnel4_b_addr} > /dev/null
 	pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${tunnel4_b_addr})"
 	check_pmtu_value "" "${pmtu}" "sending packet smaller than PMTU (IP payload length ${esp_payload_rfc4106})" || return 1
 
 	# Now exceed link layer MTU by one byte, check that exception is created
 	# with the right PMTU value
-	${ns_a} ping -q -M want -i 0.1 -w 2 -s $((ping_payload + 1)) ${tunnel4_b_addr} > /dev/null
+	${ns_a} ping -q -M want -i 0.1 -w 1 -s $((ping_payload + 1)) ${tunnel4_b_addr} > /dev/null
 	pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${tunnel4_b_addr})"
 	check_pmtu_value "${esp_payload_rfc4106}" "${pmtu}" "exceeding PMTU (IP payload length $((esp_payload_rfc4106 + 1)))"
 }
@@ -847,7 +845,7 @@ test_pmtu_vti6_exception() {
 	mtu "${ns_b}" veth_b 4000
 	mtu "${ns_a}" vti6_a 5000
 	mtu "${ns_b}" vti6_b 5000
-	${ns_a} ${ping6} -q -i 0.1 -w 2 -s 60000 ${tunnel6_b_addr} > /dev/null
+	${ns_a} ${ping6} -q -i 0.1 -w 1 -s 60000 ${tunnel6_b_addr} > /dev/null
 
 	# Check that exception was created
 	pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${tunnel6_b_addr})"
-- 
2.20.1


^ permalink raw reply related

* [PATCH net v3 2/2] selftests: pmtu: add explicit tests for PMTU exceptions cleanup
From: Paolo Abeni @ 2019-02-25 15:08 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller, David Ahern, Stefano Brivio, Sabrina Dubroca
In-Reply-To: <cover.1551106591.git.pabeni@redhat.com>

Add a couple of new tests, explicitly checking that the kernel
timely releases PMTU exceptions on related device removal.
This is mostly a regression test vs the issue fixed by
commit f5b51fe804ec ("ipv6: route: purge exception on removal")

Only 2 new test cases have been added, instead of extending all
the existing ones, because the reproducer requires executing
several commands and would slow down too much the tests otherwise.

v2 -> v3:
 - more cleanup, still from Stefano

v1 -> v2:
 - several script cleanups, as suggested by Stefano

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 tools/testing/selftests/net/pmtu.sh | 68 ++++++++++++++++++++++++++++-
 1 file changed, 67 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
index 89aec2fdf4fa..912b2dc50be3 100755
--- a/tools/testing/selftests/net/pmtu.sh
+++ b/tools/testing/selftests/net/pmtu.sh
@@ -103,6 +103,15 @@
 #	and check that configured MTU is used on link creation and changes, and
 #	that MTU is properly calculated instead when MTU is not configured from
 #	userspace
+#
+# - cleanup_ipv4_exception
+#	Similar to pmtu_ipv4_vxlan4_exception, but explicitly generate PMTU
+#	exceptions on multiple CPUs and check that the veth device tear-down
+# 	happens in a timely manner
+#
+# - cleanup_ipv6_exception
+#	Same as above, but use IPv6 transport from A to B
+
 
 # Kselftest framework requirement - SKIP code is 4.
 ksft_skip=4
@@ -135,7 +144,9 @@ tests="
 	pmtu_vti6_default_mtu		vti6: default MTU assignment
 	pmtu_vti4_link_add_mtu		vti4: MTU setting on link creation
 	pmtu_vti6_link_add_mtu		vti6: MTU setting on link creation
-	pmtu_vti6_link_change_mtu	vti6: MTU changes on link changes"
+	pmtu_vti6_link_change_mtu	vti6: MTU changes on link changes
+	cleanup_ipv4_exception		ipv4: cleanup of cached exceptions
+	cleanup_ipv6_exception		ipv6: cleanup of cached exceptions"
 
 NS_A="ns-$(mktemp -u XXXXXX)"
 NS_B="ns-$(mktemp -u XXXXXX)"
@@ -1006,6 +1017,61 @@ test_pmtu_vti6_link_change_mtu() {
 	return ${fail}
 }
 
+check_command() {
+	cmd=${1}
+
+	if ! which ${cmd} > /dev/null 2>&1; then
+		err "  missing required command: '${cmd}'"
+		return 1
+	fi
+	return 0
+}
+
+test_cleanup_vxlanX_exception() {
+	outer="${1}"
+	encap="vxlan"
+	ll_mtu=4000
+
+	check_command taskset || return 2
+	cpu_list=$(grep -m 2 processor /proc/cpuinfo | cut -d ' ' -f 2)
+
+	setup namespaces routing ${encap}${outer} || return 2
+	trace "${ns_a}" ${encap}_a   "${ns_b}"  ${encap}_b \
+	      "${ns_a}" veth_A-R1    "${ns_r1}" veth_R1-A \
+	      "${ns_b}" veth_B-R1    "${ns_r1}" veth_R1-B
+
+	# Create route exception by exceeding link layer MTU
+	mtu "${ns_a}"  veth_A-R1 $((${ll_mtu} + 1000))
+	mtu "${ns_r1}" veth_R1-A $((${ll_mtu} + 1000))
+	mtu "${ns_b}"  veth_B-R1 ${ll_mtu}
+	mtu "${ns_r1}" veth_R1-B ${ll_mtu}
+
+	mtu "${ns_a}" ${encap}_a $((${ll_mtu} + 1000))
+	mtu "${ns_b}" ${encap}_b $((${ll_mtu} + 1000))
+
+	# Fill exception cache for multiple CPUs (2)
+	# we can always use inner IPv4 for that
+	for cpu in ${cpu_list}; do
+		taskset --cpu-list ${cpu} ${ns_a} ping -q -M want -i 0.1 -w 1 -s $((${ll_mtu} + 500)) ${tunnel4_b_addr} > /dev/null
+	done
+
+	${ns_a} ip link del dev veth_A-R1 &
+	iplink_pid=$!
+	sleep 1
+	if [ "$(cat /proc/${iplink_pid}/cmdline 2>/dev/null | tr -d '\0')" = "iplinkdeldevveth_A-R1" ]; then
+		err "  can't delete veth device in a timely manner, PMTU dst likely leaked"
+		return 1
+	fi
+}
+
+test_cleanup_ipv6_exception() {
+	test_cleanup_vxlanX_exception 6
+}
+
+test_cleanup_ipv4_exception() {
+	test_cleanup_vxlanX_exception 4
+}
+
 usage() {
 	echo
 	echo "$0 [OPTIONS] [TEST]..."
-- 
2.20.1


^ permalink raw reply related

* [PATCH net v3 0/2] selftests: pmtu: fix and increase coverage
From: Paolo Abeni @ 2019-02-25 15:08 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller, David Ahern, Stefano Brivio, Sabrina Dubroca

This series includes a fixup for the pmtu.sh test script, related to IPv6
address management, and adds coverage for the recently reported and fixed
PMTU exception issue

v2 -> v3:
 - more cleanups

v1 -> v2:
 - several script cleanups

Paolo Abeni (2):
  selftests: pmtu: disable DAD in all namespaces
  selftests: pmtu: add explicit tests for PMTU exceptions cleanup

 tools/testing/selftests/net/pmtu.sh | 96 ++++++++++++++++++++++++-----
 1 file changed, 80 insertions(+), 16 deletions(-)

-- 
2.20.1


^ permalink raw reply

* [PATCH net-next] net: wan: z85230: replace dev_kfree_skb_irq by dev_consume_skb_irq for drop profiles
From: Yang Wei @ 2019-02-25 15:06 UTC (permalink / raw)
  To: netdev; +Cc: davem, yang.wei9, albin_yang

From: Yang Wei <yang.wei9@zte.com.cn>

dev_consume_skb_irq() should be called in z8530_tx_done() when skb
xmit done. It makes drop profiles(dropwatch, perf) more friendly.

Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
---
 drivers/net/wan/z85230.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wan/z85230.c b/drivers/net/wan/z85230.c
index deea41e..0e56ab0 100644
--- a/drivers/net/wan/z85230.c
+++ b/drivers/net/wan/z85230.c
@@ -1536,7 +1536,7 @@ static void z8530_tx_done(struct z8530_channel *c)
 	z8530_tx_begin(c);
 	c->netdevice->stats.tx_packets++;
 	c->netdevice->stats.tx_bytes += skb->len;
-	dev_kfree_skb_irq(skb);
+	dev_consume_skb_irq(skb);
 }
 
 /**
-- 
2.7.4



^ permalink raw reply related

* [PATCH net-next] net: wan: cosa: replace dev_kfree_skb_irq by dev_consume_skb_irq for drop profiles
From: Yang Wei @ 2019-02-25 15:05 UTC (permalink / raw)
  To: netdev; +Cc: kas, davem, yang.wei9, albin_yang

From: Yang Wei <yang.wei9@zte.com.cn>

dev_consume_skb_irq() should be called in cosa_net_tx_done() when skb
xmit done. It makes drop profiles(dropwatch, perf) more friendly.

Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
---
 drivers/net/wan/cosa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wan/cosa.c b/drivers/net/wan/cosa.c
index f6b000d..55f76e4 100644
--- a/drivers/net/wan/cosa.c
+++ b/drivers/net/wan/cosa.c
@@ -769,7 +769,7 @@ static int cosa_net_tx_done(struct channel_data *chan, int size)
 		chan->netdev->stats.tx_aborted_errors++;
 		return 1;
 	}
-	dev_kfree_skb_irq(chan->tx_skb);
+	dev_consume_skb_irq(chan->tx_skb);
 	chan->tx_skb = NULL;
 	chan->netdev->stats.tx_packets++;
 	chan->netdev->stats.tx_bytes += size;
-- 
2.7.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