Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next 2/3] sfc: refactor debug-or-warnings printks
From: David Miller @ 2017-01-26 19:36 UTC (permalink / raw)
  To: ecree; +Cc: linux-net-drivers, netdev
In-Reply-To: <15f3ecb9-1785-8688-93b9-51d400ee12bc@solarflare.com>

From: Edward Cree <ecree@solarflare.com>
Date: Thu, 26 Jan 2017 17:53:48 +0000

> diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
> index 5927c20..c640955 100644
> --- a/drivers/net/ethernet/sfc/net_driver.h
> +++ b/drivers/net/ethernet/sfc/net_driver.h
> @@ -51,6 +51,15 @@
>  #define EFX_WARN_ON_PARANOID(x) do {} while (0)
>  #endif
>  
> +/* if @cond then downgrade to debug, else print at @level */
> +#define netif_cond_dbg(priv, type, netdev, cond, level, fmt, args...)     \
> +	do {                                                              \
> +		if (cond)                                                 \
> +			netif_dbg(priv, type, netdev, fmt, ##args);       \
> +		else                                                      \
> +			netif_ ## level(priv, type, netdev, fmt, ##args); \
> +	} while (0)
> +
>  /**************************************************************************
>   *
>   * Efx data structures
> 

Please do not define locally in a driver an interface that looks like a generic
one and might be useful to code outside of your driver.

Thanks.

^ permalink raw reply

* Re: [PATCHv2 perf/core 5/7] tools lib bpf: Add bpf_program__pin()
From: Joe Stringer @ 2017-01-26 19:43 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: ast, Wangnan (F), Daniel Borkmann, LKML, netdev
In-Reply-To: <20170126193240.GC17504@kernel.org>

On 26 January 2017 at 11:32, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> Em Wed, Jan 25, 2017 at 10:18:22AM +0800, Wangnan (F) escreveu:
>> On 2017/1/25 9:16, Joe Stringer wrote:
>> > On 24 January 2017 at 17:06, Wangnan (F) <wangnan0@huawei.com> wrote:
>> > > On 2017/1/25 9:04, Wangnan (F) wrote:
>> > > Is it possible to use directory tree instead?
>
>> > > %s/object/mapname
>> > > %s/object/prog/instance
>> > I don't think objects have names, so let's assume an object with two
>> > program instances named foo, and one map named bar.
>
>> > A call of bpf_object__pin(obj, "/sys/fs/bpf/myobj") would mount with
>> > the following files and directories:
>> > /sys/fs/bpf/myobj/foo/1
>> > /sys/fs/bpf/myobj/foo/2
>> > /sys/fs/bpf/myobj/bar
>
>> > Alternatively, if you want to control exactly where you want the
>> > progs/maps to be pinned, you can call eg
>> > bpf_program__pin_instance(prog, "/sys/fs/bpf/wherever", 0) and that
>> > instance will be mounted to /sys/fs/bpf/wherever, or alternatively
>> > bpf_program__pin(prog, "/sys/fs/bpf/foo"), and you will end up with
>> > /sys/fs/bpf/foo/{0,1}.
>
>> > This looks pretty reasonable to me.
>
>> It looks good to me.
>
> Ok, please continue from perf/core, Ingo merged the first patch of this
> patchset today,

Ok thanks, I'll continue from there.

^ permalink raw reply

* Re: [PATCH net-next 1/3] trace: add variant without spacing in trace_print_hex_seq
From: Arnaldo Carvalho de Melo @ 2017-01-26 19:53 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: davem, ast, netdev, linux-kernel, Steven Rostedt,
	Arnaldo Carvalho de Melo
In-Reply-To: <f860273ed9fd0c423f4d1ee2ab93565992d76ff4.1485306168.git.daniel@iogearbox.net>

Em Wed, Jan 25, 2017 at 02:28:16AM +0100, Daniel Borkmann escreveu:
> For upcoming tracepoint support for BPF, we want to dump the program's
> tag. Format should be similar to __print_hex(), but without spacing.
> Add a __print_hex_str() variant for exactly that purpose that reuses
> trace_print_hex_seq().

Steven should be back to his side of the wall soon, will wait for his
Ack, ok?

- Arnaldo
 
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  include/linux/trace_events.h | 3 ++-
>  include/trace/trace_events.h | 8 +++++++-
>  kernel/trace/trace_output.c  | 7 ++++---
>  3 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
> index be00761..cfa475a 100644
> --- a/include/linux/trace_events.h
> +++ b/include/linux/trace_events.h
> @@ -33,7 +33,8 @@ const char *trace_print_bitmask_seq(struct trace_seq *p, void *bitmask_ptr,
>  				    unsigned int bitmask_size);
>  
>  const char *trace_print_hex_seq(struct trace_seq *p,
> -				const unsigned char *buf, int len);
> +				const unsigned char *buf, int len,
> +				bool spacing);
>  
>  const char *trace_print_array_seq(struct trace_seq *p,
>  				   const void *buf, int count,
> diff --git a/include/trace/trace_events.h b/include/trace/trace_events.h
> index 467e12f..9f68462 100644
> --- a/include/trace/trace_events.h
> +++ b/include/trace/trace_events.h
> @@ -297,7 +297,12 @@
>  #endif
>  
>  #undef __print_hex
> -#define __print_hex(buf, buf_len) trace_print_hex_seq(p, buf, buf_len)
> +#define __print_hex(buf, buf_len)					\
> +	trace_print_hex_seq(p, buf, buf_len, true)
> +
> +#undef __print_hex_str
> +#define __print_hex_str(buf, buf_len)					\
> +	trace_print_hex_seq(p, buf, buf_len, false)
>  
>  #undef __print_array
>  #define __print_array(array, count, el_size)				\
> @@ -711,6 +716,7 @@
>  #undef __print_flags
>  #undef __print_symbolic
>  #undef __print_hex
> +#undef __print_hex_str
>  #undef __get_dynamic_array
>  #undef __get_dynamic_array_len
>  #undef __get_str
> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
> index 5d33a73..30a144b1 100644
> --- a/kernel/trace/trace_output.c
> +++ b/kernel/trace/trace_output.c
> @@ -163,14 +163,15 @@ enum print_line_t trace_print_printk_msg_only(struct trace_iterator *iter)
>  EXPORT_SYMBOL_GPL(trace_print_bitmask_seq);
>  
>  const char *
> -trace_print_hex_seq(struct trace_seq *p, const unsigned char *buf, int buf_len)
> +trace_print_hex_seq(struct trace_seq *p, const unsigned char *buf, int buf_len,
> +		    bool spacing)
>  {
>  	int i;
>  	const char *ret = trace_seq_buffer_ptr(p);
>  
>  	for (i = 0; i < buf_len; i++)
> -		trace_seq_printf(p, "%s%2.2x", i == 0 ? "" : " ", buf[i]);
> -
> +		trace_seq_printf(p, "%s%2.2x", !spacing || i == 0 ? "" : " ",
> +				 buf[i]);
>  	trace_seq_putc(p, 0);
>  
>  	return ret;
> -- 
> 1.9.3

^ permalink raw reply

* [PATCH net-next] liquidio: Avoid accessing skb after submitting to input queue
From: Felix Manlunas @ 2017-01-26 19:52 UTC (permalink / raw)
  To: davem; +Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla

From: Satanand Burla <satananda.burla@cavium.com>

Accessing skb after submitting to input queue can cause
access to stale pointers if the skb ends up being transmitted
and freed by that time.

Signed-off-by: Satanand Burla <satananda.burla@cavium.com>
Signed-off-by: Derek Chickles <derek.chickles@cavium.com>
Signed-off-by: Raghu Vatsavayi <raghu.vatsavayi@cavium.com>
Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
---
 drivers/net/ethernet/cavium/liquidio/lio_main.c    | 6 +++---
 drivers/net/ethernet/cavium/liquidio/lio_vf_main.c | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c
index 5ee3f00..9261ddc 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
@@ -3316,11 +3316,11 @@ static int liquidio_xmit(struct sk_buff *skb, struct net_device *netdev)
 
 	netif_trans_update(netdev);
 
-	if (skb_shinfo(skb)->gso_size)
-		stats->tx_done += skb_shinfo(skb)->gso_segs;
+	if (tx_info->s.gso_segs)
+		stats->tx_done += tx_info->s.gso_segs;
 	else
 		stats->tx_done++;
-	stats->tx_tot_bytes += skb->len;
+	stats->tx_tot_bytes += ndata.datasize;
 
 	return NETDEV_TX_OK;
 
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
index e96cf6c..a6587d7 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
@@ -2433,11 +2433,11 @@ static int liquidio_xmit(struct sk_buff *skb, struct net_device *netdev)
 
 	netif_trans_update(netdev);
 
-	if (skb_shinfo(skb)->gso_size)
-		stats->tx_done += skb_shinfo(skb)->gso_segs;
+	if (tx_info->s.gso_segs)
+		stats->tx_done += tx_info->s.gso_segs;
 	else
 		stats->tx_done++;
-	stats->tx_tot_bytes += skb->len;
+	stats->tx_tot_bytes += ndata.datasize;
 
 	return NETDEV_TX_OK;

^ permalink raw reply related

* Re: [PATCHv3 4/5] arm: mvebu: Add device tree for 98DX3236 SoCs
From: Chris Packham @ 2017-01-26 20:07 UTC (permalink / raw)
  To: Gregory CLEMENT
  Cc: Mark Rutland, Andrew Lunn, Jason Cooper,
	devicetree@vger.kernel.org, netdev@vger.kernel.org, Russell King,
	linux-kernel@vger.kernel.org, Rob Herring,
	linux-arm-kernel@lists.infradead.org, Sebastian Hesselbarth
In-Reply-To: <87inp1zwlg.fsf@free-electrons.com>

On 27/01/17 04:10, Gregory CLEMENT wrote:
> Hi Chris,
>
>  On ven., janv. 06 2017, Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:
>
>> The Marvell 98DX3236, 98DX3336, 98DX4521 and variants are switch ASICs
>> with integrated CPUs. They are similar to the Armada XP SoCs but have
>> different I/O interfaces.
>
> Before sending a new version I have a few remarks:
>
>

[snip]

I'll update the dtsi files to use the node labels and correct the 
commends as requested

>
> Why the following node is not part of the dtsi?
>
> Gregory
>
>> +			resume@20980 {
>> +				compatible = "marvell,98dx3336-resume-ctrl";
>> +				reg = <0x20980 0x10>;
>> +			};
>> +		};
>> +	};

The 98DX9236 has a single ARMv7 core. As such this resume control isn't 
present on it. The 98DX3336 and 98DX4521 have dual ARMv7 cores and this 
is used to boot the second core (SMP support is a little different 
compared to Armada-XP).

In other words {98DX3336, 98DX4521} = 98DX9236 + an additional core. At 
the switch packet processor level there are more differences but as far 
as the kernel is concerned the only real difference is the number of cores.

^ permalink raw reply

* Re: [PATCH net] bpf: expose netns inode to bpf programs
From: Alexei Starovoitov @ 2017-01-26 19:25 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Linus Torvalds, Eric W. Biederman, David S . Miller,
	Daniel Borkmann, David Ahern, Tejun Heo, Thomas Graf,
	Network Development
In-Reply-To: <CALCETrXgdY_Kt8wn4uiATUnNJ3YXttCUREgEeQReG7u29Lc44g@mail.gmail.com>

On 1/26/17 11:07 AM, Andy Lutomirski wrote:
> On Thu, Jan 26, 2017 at 10:32 AM, Alexei Starovoitov <ast@fb.com> wrote:
>> On 1/26/17 10:12 AM, Andy Lutomirski wrote:
>>>
>>> On Thu, Jan 26, 2017 at 9:46 AM, Alexei Starovoitov <ast@fb.com> wrote:
>>>>
>>>> On 1/26/17 8:37 AM, Andy Lutomirski wrote:
>>>>>>
>>>>>>
>>>>>> Think of bpf programs as safe kernel modules. They don't have
>>>>>> confined boundaries and program authors, if not careful, can shoot
>>>>>> themselves in the foot. We're not trying to prevent that because
>>>>>> it's impossible to check that the program is sane. Just like
>>>>>> it's impossible to check that kernel module is sane.
>>>>>> But in case of bpf we check that bpf program is _safe_ from the kernel
>>>>>> point of view. If it's doing some garbage, it's program's business.
>>>>>> Does it make more sense now?
>>>>>>
>>>>>
>>>>> With all due respect, I think this is not an acceptable way to think
>>>>> about BPF at all.  If you think of BPF this way, I think there needs
>>>>> to be a real discussion at KS or similar as to whether this is okay.
>>>>> The reason is simple: the kernel promises a stable ABI to userspace
>>>>> but not to kernel modules.  By thinking of BPF as more like a module,
>>>>> you're taking a big shortcut that will either result in ABI breakage
>>>>> down the road or in committing to a problematic stable ABI.
>>>>
>>>>
>>>>
>>>> you misunderstood the analogy.
>>>> bpf abi is certainly stable. that's why we were careful of not
>>>> exposing anything to it that is not already stable.
>>>>
>>>
>>> In that case I don't understand what you're trying to say.  Eric
>>> thinks your patch exposes a bad interface.  A bad interface for
>>> userspace is a very different thing from a bad interface available to
>>> kernel modules.  Are you saying that BPF is kernel-module-like in that
>>> the ABI exposed to BPF programs doesn't need to meet the same quality
>>> standards as userspace ABIs?
>>
>>
>> of course not.
>> ns.inum is already exposed to user space as a value.
>> This patch exposes it to bpf program in a convenient and stable way,
>
> Here's what I'm imaging Eric is thinking:
>
> ns.inum is currently exposed to userspace via procfs.  In principle,
> the value could be local to a namespace, though, which would enable
> CRIU to be able to preserve namespace inode numbers across a
> checkpoint+restore operation.  If this happened, the contained and
> restored procfs would see a different inode number than the outermost
> procfs.

sure. there are many different ways for the program to see inode
that either was already reused or disappeared.
What I'm saying that it is expected. We cannot prevent that from
bpf side. Just like ifindex value read by the program can be bogus
as in the example I just provided.

> If you start exposing the raw ns.inum field to BPF programs and those
> programs are not themselves scoped to a namespace, then this could
> create a problem for CRIU.

criu doesn't support ebpf because maps are not snapshot-able and
programs are detached from the control plane. I cannot see how one can
criu of xdp or cls program. The ssh connection to the box might die in
the middle while criu is messing with unknown. Hence the analogy to
the kernel modules. Imagine a set of mini-kernel modules and a set
of apps that depend on them. What kind of criu can we even talk about?

> But you told Eric that his nack doesn't matter, and maybe it would be
> nice to ask him to clarify instead.

Fair enough. Eric, thoughts?

^ permalink raw reply

* Re: [PATCH RFC net-next] packet: always ensure that we pass hard_header_len bytes in skb_headlen() to the driver
From: Willem de Bruijn @ 2017-01-26 20:21 UTC (permalink / raw)
  To: Sowmini Varadhan; +Cc: David Miller, Network Development
In-Reply-To: <1485274309-201670-1-git-send-email-sowmini.varadhan@oracle.com>

> If the application has provided fewer than hard_header_len bytes,
> dev_validate_header() will zero out the skb->data as needed. This is
> acceptable for SOCK_DGRAM/PF_PACKET sockets but in all other cases,

This was added not for datagram sockets, but to be able to bypass
validation. See the message in commit 2793a23aacbd ("net: validate
variable length ll header") and discussion leading up to that patch.

> the application must provide a full L2 header, and the PF_PACKET Tx
> paths must fail with an error when fewer than hard_header_len bytes
> are detected.

As David pointed out, this does not handle variable length headers
correctly. In link layers that support these, hard_header_len defines
the maximum header length. A hard failure on len < hard_header_len
would be incorrect.

The ->validate callback was added to allow specifying additional
constraints on a per protocol basis. This is where a min constraint
can be added, e.g., for ethernet.

> All invocations to dev_validate_header() already adjusts the
> skb's data, len, tail etc pointers based on hard_header_len before
> invoking dev_validate_header(), so additional skb pointers should
> not be needed after dev_validate_header().
>
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
> ---

> -       if (!dev_validate_header(dev, skb->data, len)) {
> +       newlen = dev_validate_header(dev, skb->data, len);
> +       /* As comments above this function indicate, a full L2 header
> +        * must be passed to this function, so if newlen > len, bail.
> +        */
> +       if (newlen < 0 || newlen > len) {

If callers only care whether the function returned failure or
increased len, which also indicates failure, it is cleaner to leave it
a boolean and fail in cases where len < the minimum for that link
layer type. No caller actually uses newlen.

> +               /* Caller has allocated for copylen in non-paged part of
> +                * skb so we should never find newlen > hdrlen
> +                */
> +               WARN_ON(newlen > hdrlen);

WARN_ON_ONCE is safer.

^ permalink raw reply

* Re: [PATCHv3 4/5] arm: mvebu: Add device tree for 98DX3236 SoCs
From: Chris Packham @ 2017-01-26 20:24 UTC (permalink / raw)
  To: Gregory CLEMENT
  Cc: linux-arm-kernel@lists.infradead.org, Rob Herring, Mark Rutland,
	Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Russell King,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org
In-Reply-To: <87inp1zwlg.fsf@free-electrons.com>

On 27/01/17 04:10, Gregory CLEMENT wrote:
>> +		internal-regs {

[snip]

>> +
>> +		dfx-registers {
> node label
>

[snip]

>> +		switch {
> node label
>

These are peers to the internal-regs, i.e. parts of the SoC with 
mappable windows in the address space. Do they really need a label? 
Their subnodes absolutely need (and have) labels.


^ permalink raw reply

* Re: [PATCH 0/6 v3] kvmalloc
From: Daniel Borkmann @ 2017-01-26 20:34 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Alexei Starovoitov, Andrew Morton, Vlastimil Babka, Mel Gorman,
	Johannes Weiner, linux-mm, LKML, netdev@vger.kernel.org,
	marcelo.leitner
In-Reply-To: <20170126134004.GM6590@dhcp22.suse.cz>

On 01/26/2017 02:40 PM, Michal Hocko wrote:
> On Thu 26-01-17 14:10:06, Daniel Borkmann wrote:
>> On 01/26/2017 12:58 PM, Michal Hocko wrote:
>>> On Thu 26-01-17 12:33:55, Daniel Borkmann wrote:
>>>> On 01/26/2017 11:08 AM, Michal Hocko wrote:
>>> [...]
>>>>> If you disagree I can drop the bpf part of course...
>>>>
>>>> If we could consolidate these spots with kvmalloc() eventually, I'm
>>>> all for it. But even if __GFP_NORETRY is not covered down to all
>>>> possible paths, it kind of does have an effect already of saying
>>>> 'don't try too hard', so would it be harmful to still keep that for
>>>> now? If it's not, I'd personally prefer to just leave it as is until
>>>> there's some form of support by kvmalloc() and friends.
>>>
>>> Well, you can use kvmalloc(size, GFP_KERNEL|__GFP_NORETRY). It is not
>>> disallowed. It is not _supported_ which means that if it doesn't work as
>>> you expect you are on your own. Which is actually the situation right
>>> now as well. But I still think that this is just not right thing to do.
>>> Even though it might happen to work in some cases it gives a false
>>> impression of a solution. So I would rather go with
>>
>> Hmm. 'On my own' means, we could potentially BUG somewhere down the
>> vmalloc implementation, etc, presumably? So it might in-fact be
>> harmful to pass that, right?
>
> No it would mean that it might eventually hit the behavior which you are
> trying to avoid - in other words it may invoke OOM killer even though
> __GFP_NORETRY means giving up before any system wide disruptive actions
> a re taken.

Ok, thanks for clarifying, more on that further below.

>>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>>> index 8697f43cf93c..a6dc4d596f14 100644
>>> --- a/kernel/bpf/syscall.c
>>> +++ b/kernel/bpf/syscall.c
>>> @@ -53,6 +53,11 @@ void bpf_register_map_type(struct bpf_map_type_list *tl)
>>>
>>>    void *bpf_map_area_alloc(size_t size)
>>>    {
>>> +	/*
>>> +	 * FIXME: we would really like to not trigger the OOM killer and rather
>>> +	 * fail instead. This is not supported right now. Please nag MM people
>>> +	 * if these OOM start bothering people.
>>> +	 */
>>
>> Ok, I know this is out of scope for this series, but since i) this
>> is _not_ the _only_ spot right now which has such a construct and ii)
>> I am already kind of nagging a bit ;), my question would be, what
>> would it take to start supporting it?
>
> propagate gfp mask all the way down from vmalloc to all places which
> might allocate down the path and especially page table allocation
> function are PITA because they are really deep. This is a lot of work...
>
> But realistically, how big is this problem really? Is it really worth
> it? You said this is an admin only interface and admin can kill the
> machine by OOM and other means already.
>
> Moreover and I should probably mention it explicitly, your d407bd25a204b
> reduced the likelyhood of oom for other reason. kmalloc used GPF_USER
> previously and with order > 0 && order <= PAGE_ALLOC_COSTLY_ORDER this
> could indeed hit the OOM e.g. due to memory fragmentation. It would be
> much harder to hit the OOM killer from vmalloc which doesn't issue
> higher order allocation requests. Or have you ever seen the OOM killer
> pointing to the vmalloc fallback path?

The case I was concerned about was from vmalloc() path, not kmalloc().
That was where the stack trace indicating OOM pointed to. As an example,
there could be really large allocation requests for maps where the map
has pre-allocated memory for its elements. Thus, if we get to the point
where we need to kill others due to shortage of mem for satisfying this,
I'd much much rather prefer to just not let vmalloc() work really hard
and fail early on instead. In my (crafted) test case, I was connected
via ssh and it each time reliably killed my connection, which is really
suboptimal.

F.e., I could also imagine a buggy or miscalculated map definition for
a prog that is provisioned to multiple places, which then accidentally
triggers this. Or if large on purpose, but we crossed the line, it
could be handled more gracefully, f.e. I could imagine an option to
falling back to a non-pre-allocated map flavor from the application
loading the program. Trade-off for sure, but still allowing it to
operate up to a certain extend. Granted, if vmalloc() succeeded without
trying hard and we then OOM elsewhere, too bad, but we don't have much
control over that one anyway, only about our own request. Reason I
asked above was whether having __GFP_NORETRY in would be fatal
somewhere down the path, but seems not as you say.

So to answer your second email with the bpf and netfilter hunks, why
not replacing them with kvmalloc() and __GFP_NORETRY flag and add that
big fat FIXME comment above there, saying explicitly that __GFP_NORETRY
is not harmful though has only /partial/ effect right now and that full
support needs to be implemented in future. That would still be better
that not having it, imo, and the FIXME would make expectations clear
to anyone reading that code.

Thanks,
Daniel

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH net-next 1/3] trace: add variant without spacing in trace_print_hex_seq
From: Daniel Borkmann @ 2017-01-26 20:40 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: davem, ast, netdev, linux-kernel, Steven Rostedt,
	Arnaldo Carvalho de Melo
In-Reply-To: <20170126195306.GD17504@kernel.org>

On 01/26/2017 08:53 PM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Jan 25, 2017 at 02:28:16AM +0100, Daniel Borkmann escreveu:
>> For upcoming tracepoint support for BPF, we want to dump the program's
>> tag. Format should be similar to __print_hex(), but without spacing.
>> Add a __print_hex_str() variant for exactly that purpose that reuses
>> trace_print_hex_seq().
>
> Steven should be back to his side of the wall soon, will wait for his
> Ack, ok?

Ok, seems this set got applied already to net-next in the meantime, so
if there are any objections on this, I will follow up with a patch of
course.

Thanks,
Daniel

> - Arnaldo

^ permalink raw reply

* Re: [PATCH net-next] liquidio: Avoid accessing skb after submitting to input queue
From: David Miller @ 2017-01-26 20:42 UTC (permalink / raw)
  To: felix.manlunas; +Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla
In-Reply-To: <20170126195235.GA3875@felix.cavium.com>

From: Felix Manlunas <felix.manlunas@cavium.com>
Date: Thu, 26 Jan 2017 11:52:35 -0800

> From: Satanand Burla <satananda.burla@cavium.com>
> 
> Accessing skb after submitting to input queue can cause
> access to stale pointers if the skb ends up being transmitted
> and freed by that time.
> 
> Signed-off-by: Satanand Burla <satananda.burla@cavium.com>
> Signed-off-by: Derek Chickles <derek.chickles@cavium.com>
> Signed-off-by: Raghu Vatsavayi <raghu.vatsavayi@cavium.com>
> Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next v2 0/4] net: dsa: Preparatory patches
From: David Miller @ 2017-01-26 20:44 UTC (permalink / raw)
  To: f.fainelli; +Cc: netdev, andrew, vivien.didelot
In-Reply-To: <20170126184554.1325-1-f.fainelli@gmail.com>

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Thu, 26 Jan 2017 10:45:50 -0800

> This patch series extracts the 4 patches of the larger: net: dsa: Support for
> pdata in dsa2 while we wait for feedback from Greg KH on the device references.
> 
> Changes in v2:
> 
> - rebased properly after the multi-MDIO bus support added to mv88e6xxx

Series applied, thanks Florian.

^ permalink raw reply

* RE: netvsc NAPI patch process
From: KY Srinivasan @ 2017-01-26 20:46 UTC (permalink / raw)
  To: Stephen Hemminger, davem@davemloft.net, Greg KH; +Cc: netdev@vger.kernel.org
In-Reply-To: <20170126100405.0eac2dea@xeon-e3>



> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Thursday, January 26, 2017 10:04 AM
> To: davem@davemloft.net; KY Srinivasan <kys@microsoft.com>; Greg KH
> <greg@kroah.com>
> Cc: netdev@vger.kernel.org
> Subject: netvsc NAPI patch process
> 
> I have a working set of patches to enable NAPI in the netvsc driver.
> The problem is that it requires a set of patches to vmbus layer as well.
> Since vmbus patches have been going through char-misc-next tree rather
> than net-next, it is difficult to stage these.

In the past, we have done this in two stages - get the supporting vmbus patches into
Greg's tree first and in the next merge cycle get the netvsc patches in. Why not continue to
do what we have done in the past to address cross-tree dependencies.

Regards,

K. Y 
> 
> How about if I send the vmbus patches through normal driver-devel
> upstream
> and during the 4.10 merge window send the last 3 patches for NAPI for linux-
> net
> tree to get into 4.10?

^ permalink raw reply

* Re: [PATCH v2 net] net: free ip_vs_dest structs when refcnt=0
From: Julian Anastasov @ 2017-01-26 20:49 UTC (permalink / raw)
  To: David Windsor
  Cc: netdev, kernel-hardening, netfilter-devel, lvs-devel, wensong,
	horms, pablo, keescook, elena.reshetova, ishkamiel
In-Reply-To: <1485228269-21758-1-git-send-email-dwindsor@gmail.com>


	Hello,

On Mon, 23 Jan 2017, David Windsor wrote:

> Currently, the ip_vs_dest cache frees ip_vs_dest objects when their
> reference count becomes < 0.  Aside from not being semantically sound,
> this is problematic for the new type refcount_t, which will be introduced
> shortly in a separate patch. refcount_t is the new kernel type for
> holding reference counts, and provides overflow protection and a
> constrained interface relative to atomic_t (the type currently being
> used for kernel reference counts).
> 
> Per Julian Anastasov: "The problem is that dest_trash currently holds
> deleted dests (unlinked from RCU lists) with refcnt=0."  Changing
> dest_trash to hold dest with refcnt=1 will allow us to free ip_vs_dest
> structs when their refcnt=0, in ip_vs_dest_put_and_free().
> 
> Signed-off-by: David Windsor <dwindsor@gmail.com>

	Thanks! I tested the first version and this one
just adds the needed changes in comments, so

Signed-off-by: Julian Anastasov <ja@ssi.bg>

	Simon and Pablo, this is more appropriate for
ipvs-next/nf-next. Please apply!

> ---
>  include/net/ip_vs.h            | 2 +-
>  net/netfilter/ipvs/ip_vs_ctl.c | 8 +++-----
>  2 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
> index cd6018a..a3e78ad 100644
> --- a/include/net/ip_vs.h
> +++ b/include/net/ip_vs.h
> @@ -1421,7 +1421,7 @@ static inline void ip_vs_dest_put(struct ip_vs_dest *dest)
>  
>  static inline void ip_vs_dest_put_and_free(struct ip_vs_dest *dest)
>  {
> -	if (atomic_dec_return(&dest->refcnt) < 0)
> +	if (atomic_dec_and_test(&dest->refcnt))
>  		kfree(dest);
>  }
>  
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index 55e0169..5fc4836 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -711,7 +711,6 @@ ip_vs_trash_get_dest(struct ip_vs_service *svc, int dest_af,
>  		      dest->vport == svc->port))) {
>  			/* HIT */
>  			list_del(&dest->t_list);
> -			ip_vs_dest_hold(dest);
>  			goto out;
>  		}
>  	}
> @@ -741,7 +740,7 @@ static void ip_vs_dest_free(struct ip_vs_dest *dest)
>   *  When the ip_vs_control_clearup is activated by ipvs module exit,
>   *  the service tables must have been flushed and all the connections
>   *  are expired, and the refcnt of each destination in the trash must
> - *  be 0, so we simply release them here.
> + *  be 1, so we simply release them here.
>   */
>  static void ip_vs_trash_cleanup(struct netns_ipvs *ipvs)
>  {
> @@ -1080,11 +1079,10 @@ static void __ip_vs_del_dest(struct netns_ipvs *ipvs, struct ip_vs_dest *dest,
>  	if (list_empty(&ipvs->dest_trash) && !cleanup)
>  		mod_timer(&ipvs->dest_trash_timer,
>  			  jiffies + (IP_VS_DEST_TRASH_PERIOD >> 1));
> -	/* dest lives in trash without reference */
> +	/* dest lives in trash with reference */
>  	list_add(&dest->t_list, &ipvs->dest_trash);
>  	dest->idle_start = 0;
>  	spin_unlock_bh(&ipvs->dest_trash_lock);
> -	ip_vs_dest_put(dest);
>  }
>  
>  
> @@ -1160,7 +1158,7 @@ static void ip_vs_dest_trash_expire(unsigned long data)
>  
>  	spin_lock(&ipvs->dest_trash_lock);
>  	list_for_each_entry_safe(dest, next, &ipvs->dest_trash, t_list) {
> -		if (atomic_read(&dest->refcnt) > 0)
> +		if (atomic_read(&dest->refcnt) > 1)
>  			continue;
>  		if (dest->idle_start) {
>  			if (time_before(now, dest->idle_start +
> -- 
> 2.7.4

Regards

^ permalink raw reply

* [PATCH] net: intel: e1000e: use new api ethtool_{get|set}_link_ksettings
From: Philippe Reynes @ 2017-01-26 21:19 UTC (permalink / raw)
  To: jeffrey.t.kirsher, davem
  Cc: intel-wired-lan, netdev, linux-kernel, Philippe Reynes

The ethtool api {get|set}_settings is deprecated.
We move this driver to new api {get|set}_link_ksettings.

As I don't have the hardware, I'd be very pleased if
someone may test this patch.

Signed-off-by: Philippe Reynes <tremyfr@gmail.com>
---
 drivers/net/ethernet/intel/e1000e/ethtool.c |   91 ++++++++++++++------------
 1 files changed, 49 insertions(+), 42 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c
index 7aff68a..3768a5c 100644
--- a/drivers/net/ethernet/intel/e1000e/ethtool.c
+++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
@@ -117,15 +117,15 @@ struct e1000_stats {
 
 #define E1000_TEST_LEN ARRAY_SIZE(e1000_gstrings_test)
 
-static int e1000_get_settings(struct net_device *netdev,
-			      struct ethtool_cmd *ecmd)
+static int e1000_get_link_ksettings(struct net_device *netdev,
+				    struct ethtool_link_ksettings *cmd)
 {
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	struct e1000_hw *hw = &adapter->hw;
-	u32 speed;
+	u32 speed, supported, advertising;
 
 	if (hw->phy.media_type == e1000_media_type_copper) {
-		ecmd->supported = (SUPPORTED_10baseT_Half |
+		supported = (SUPPORTED_10baseT_Half |
 				   SUPPORTED_10baseT_Full |
 				   SUPPORTED_100baseT_Half |
 				   SUPPORTED_100baseT_Full |
@@ -133,39 +133,36 @@ static int e1000_get_settings(struct net_device *netdev,
 				   SUPPORTED_Autoneg |
 				   SUPPORTED_TP);
 		if (hw->phy.type == e1000_phy_ife)
-			ecmd->supported &= ~SUPPORTED_1000baseT_Full;
-		ecmd->advertising = ADVERTISED_TP;
+			supported &= ~SUPPORTED_1000baseT_Full;
+		advertising = ADVERTISED_TP;
 
 		if (hw->mac.autoneg == 1) {
-			ecmd->advertising |= ADVERTISED_Autoneg;
+			advertising |= ADVERTISED_Autoneg;
 			/* the e1000 autoneg seems to match ethtool nicely */
-			ecmd->advertising |= hw->phy.autoneg_advertised;
+			advertising |= hw->phy.autoneg_advertised;
 		}
 
-		ecmd->port = PORT_TP;
-		ecmd->phy_address = hw->phy.addr;
-		ecmd->transceiver = XCVR_INTERNAL;
-
+		cmd->base.port = PORT_TP;
+		cmd->base.phy_address = hw->phy.addr;
 	} else {
-		ecmd->supported   = (SUPPORTED_1000baseT_Full |
+		supported   = (SUPPORTED_1000baseT_Full |
 				     SUPPORTED_FIBRE |
 				     SUPPORTED_Autoneg);
 
-		ecmd->advertising = (ADVERTISED_1000baseT_Full |
+		advertising = (ADVERTISED_1000baseT_Full |
 				     ADVERTISED_FIBRE |
 				     ADVERTISED_Autoneg);
 
-		ecmd->port = PORT_FIBRE;
-		ecmd->transceiver = XCVR_EXTERNAL;
+		cmd->base.port = PORT_FIBRE;
 	}
 
 	speed = SPEED_UNKNOWN;
-	ecmd->duplex = DUPLEX_UNKNOWN;
+	cmd->base.duplex = DUPLEX_UNKNOWN;
 
 	if (netif_running(netdev)) {
 		if (netif_carrier_ok(netdev)) {
 			speed = adapter->link_speed;
-			ecmd->duplex = adapter->link_duplex - 1;
+			cmd->base.duplex = adapter->link_duplex - 1;
 		}
 	} else if (!pm_runtime_suspended(netdev->dev.parent)) {
 		u32 status = er32(STATUS);
@@ -179,30 +176,36 @@ static int e1000_get_settings(struct net_device *netdev,
 				speed = SPEED_10;
 
 			if (status & E1000_STATUS_FD)
-				ecmd->duplex = DUPLEX_FULL;
+				cmd->base.duplex = DUPLEX_FULL;
 			else
-				ecmd->duplex = DUPLEX_HALF;
+				cmd->base.duplex = DUPLEX_HALF;
 		}
 	}
 
-	ethtool_cmd_speed_set(ecmd, speed);
-	ecmd->autoneg = ((hw->phy.media_type == e1000_media_type_fiber) ||
+	cmd->base.speed = speed;
+	cmd->base.autoneg = ((hw->phy.media_type == e1000_media_type_fiber) ||
 			 hw->mac.autoneg) ? AUTONEG_ENABLE : AUTONEG_DISABLE;
 
 	/* MDI-X => 2; MDI =>1; Invalid =>0 */
 	if ((hw->phy.media_type == e1000_media_type_copper) &&
 	    netif_carrier_ok(netdev))
-		ecmd->eth_tp_mdix = hw->phy.is_mdix ? ETH_TP_MDI_X : ETH_TP_MDI;
+		cmd->base.eth_tp_mdix = hw->phy.is_mdix ?
+			ETH_TP_MDI_X : ETH_TP_MDI;
 	else
-		ecmd->eth_tp_mdix = ETH_TP_MDI_INVALID;
+		cmd->base.eth_tp_mdix = ETH_TP_MDI_INVALID;
 
 	if (hw->phy.mdix == AUTO_ALL_MODES)
-		ecmd->eth_tp_mdix_ctrl = ETH_TP_MDI_AUTO;
+		cmd->base.eth_tp_mdix_ctrl = ETH_TP_MDI_AUTO;
 	else
-		ecmd->eth_tp_mdix_ctrl = hw->phy.mdix;
+		cmd->base.eth_tp_mdix_ctrl = hw->phy.mdix;
 
 	if (hw->phy.media_type != e1000_media_type_copper)
-		ecmd->eth_tp_mdix_ctrl = ETH_TP_MDI_INVALID;
+		cmd->base.eth_tp_mdix_ctrl = ETH_TP_MDI_INVALID;
+
+	ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.supported,
+						supported);
+	ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.advertising,
+						advertising);
 
 	return 0;
 }
@@ -262,12 +265,16 @@ static int e1000_set_spd_dplx(struct e1000_adapter *adapter, u32 spd, u8 dplx)
 	return -EINVAL;
 }
 
-static int e1000_set_settings(struct net_device *netdev,
-			      struct ethtool_cmd *ecmd)
+static int e1000_set_link_ksettings(struct net_device *netdev,
+				    const struct ethtool_link_ksettings *cmd)
 {
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	struct e1000_hw *hw = &adapter->hw;
 	int ret_val = 0;
+	u32 advertising;
+
+	ethtool_convert_link_mode_to_legacy_u32(&advertising,
+						cmd->link_modes.advertising);
 
 	pm_runtime_get_sync(netdev->dev.parent);
 
@@ -285,14 +292,14 @@ static int e1000_set_settings(struct net_device *netdev,
 	 * some hardware doesn't allow MDI setting when speed or
 	 * duplex is forced.
 	 */
-	if (ecmd->eth_tp_mdix_ctrl) {
+	if (cmd->base.eth_tp_mdix_ctrl) {
 		if (hw->phy.media_type != e1000_media_type_copper) {
 			ret_val = -EOPNOTSUPP;
 			goto out;
 		}
 
-		if ((ecmd->eth_tp_mdix_ctrl != ETH_TP_MDI_AUTO) &&
-		    (ecmd->autoneg != AUTONEG_ENABLE)) {
+		if ((cmd->base.eth_tp_mdix_ctrl != ETH_TP_MDI_AUTO) &&
+		    (cmd->base.autoneg != AUTONEG_ENABLE)) {
 			e_err("forcing MDI/MDI-X state is not supported when link speed and/or duplex are forced\n");
 			ret_val = -EINVAL;
 			goto out;
@@ -302,35 +309,35 @@ static int e1000_set_settings(struct net_device *netdev,
 	while (test_and_set_bit(__E1000_RESETTING, &adapter->state))
 		usleep_range(1000, 2000);
 
-	if (ecmd->autoneg == AUTONEG_ENABLE) {
+	if (cmd->base.autoneg == AUTONEG_ENABLE) {
 		hw->mac.autoneg = 1;
 		if (hw->phy.media_type == e1000_media_type_fiber)
 			hw->phy.autoneg_advertised = ADVERTISED_1000baseT_Full |
 			    ADVERTISED_FIBRE | ADVERTISED_Autoneg;
 		else
-			hw->phy.autoneg_advertised = ecmd->advertising |
+			hw->phy.autoneg_advertised = advertising |
 			    ADVERTISED_TP | ADVERTISED_Autoneg;
-		ecmd->advertising = hw->phy.autoneg_advertised;
+		advertising = hw->phy.autoneg_advertised;
 		if (adapter->fc_autoneg)
 			hw->fc.requested_mode = e1000_fc_default;
 	} else {
-		u32 speed = ethtool_cmd_speed(ecmd);
+		u32 speed = cmd->base.speed;
 		/* calling this overrides forced MDI setting */
-		if (e1000_set_spd_dplx(adapter, speed, ecmd->duplex)) {
+		if (e1000_set_spd_dplx(adapter, speed, cmd->base.duplex)) {
 			ret_val = -EINVAL;
 			goto out;
 		}
 	}
 
 	/* MDI-X => 2; MDI => 1; Auto => 3 */
-	if (ecmd->eth_tp_mdix_ctrl) {
+	if (cmd->base.eth_tp_mdix_ctrl) {
 		/* fix up the value for auto (3 => 0) as zero is mapped
 		 * internally to auto
 		 */
-		if (ecmd->eth_tp_mdix_ctrl == ETH_TP_MDI_AUTO)
+		if (cmd->base.eth_tp_mdix_ctrl == ETH_TP_MDI_AUTO)
 			hw->phy.mdix = AUTO_ALL_MODES;
 		else
-			hw->phy.mdix = ecmd->eth_tp_mdix_ctrl;
+			hw->phy.mdix = cmd->base.eth_tp_mdix_ctrl;
 	}
 
 	/* reset the link */
@@ -2313,8 +2320,6 @@ static int e1000e_get_ts_info(struct net_device *netdev,
 }
 
 static const struct ethtool_ops e1000_ethtool_ops = {
-	.get_settings		= e1000_get_settings,
-	.set_settings		= e1000_set_settings,
 	.get_drvinfo		= e1000_get_drvinfo,
 	.get_regs_len		= e1000_get_regs_len,
 	.get_regs		= e1000_get_regs,
@@ -2342,6 +2347,8 @@ static int e1000e_get_ts_info(struct net_device *netdev,
 	.get_ts_info		= e1000e_get_ts_info,
 	.get_eee		= e1000e_get_eee,
 	.set_eee		= e1000e_set_eee,
+	.get_link_ksettings	= e1000_get_link_ksettings,
+	.set_link_ksettings	= e1000_set_link_ksettings,
 };
 
 void e1000e_set_ethtool_ops(struct net_device *netdev)
-- 
1.7.4.4

^ permalink raw reply related

* [PATCHv3 perf/core 1/6] tools lib bpf: Add BPF program pinning APIs.
From: Joe Stringer @ 2017-01-26 21:19 UTC (permalink / raw)
  To: acme; +Cc: wangnan0, ast, daniel, linux-kernel, netdev
In-Reply-To: <20170126212001.14103-1-joe@ovn.org>

Add new APIs to pin a BPF program (or specific instances) to the filesystem.
The user can specify the path full path within a BPF filesystem to pin the
program.

bpf_program__pin_instance(prog, path, n) will pin the nth instance of
'prog' to the specified path.
bpf_program__pin(prog, path) will create the directory 'path' (if it
does not exist) and pin each instance within that directory. For
instance, path/0, path/1, path/2.

Signed-off-by: Joe Stringer <joe@ovn.org>
---
v3: Add per-instance pinning.
    Use path for bpf_program__pin() as directory.
v2: Don't automount BPF filesystem
    Split program, map, object pinning into separate APIs and separate
    patches.
---
 tools/lib/bpf/libbpf.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h |   3 ++
 2 files changed, 115 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index e6cd62b1264b..d1d7638b7c21 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4,6 +4,7 @@
  * Copyright (C) 2013-2015 Alexei Starovoitov <ast@kernel.org>
  * Copyright (C) 2015 Wang Nan <wangnan0@huawei.com>
  * Copyright (C) 2015 Huawei Inc.
+ * Copyright (C) 2017 Nicira, Inc.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -22,6 +23,7 @@
 #include <stdlib.h>
 #include <stdio.h>
 #include <stdarg.h>
+#include <libgen.h>
 #include <inttypes.h>
 #include <string.h>
 #include <unistd.h>
@@ -31,7 +33,10 @@
 #include <linux/err.h>
 #include <linux/kernel.h>
 #include <linux/bpf.h>
+#include <linux/magic.h>
 #include <linux/list.h>
+#include <linux/limits.h>
+#include <sys/vfs.h>
 #include <libelf.h>
 #include <gelf.h>
 
@@ -1237,6 +1242,113 @@ int bpf_object__load(struct bpf_object *obj)
 	return err;
 }
 
+static int check_path(const char *path)
+{
+	struct statfs st_fs;
+	char *dname, *dir;
+	int err = 0;
+
+	if (path == NULL)
+		return -EINVAL;
+
+	dname = strdup(path);
+	dir = dirname(dname);
+	if (statfs(dir, &st_fs)) {
+		pr_warning("failed to statfs %s: %s\n", dir, strerror(errno));
+		err = -errno;
+	}
+	free(dname);
+
+	if (!err && st_fs.f_type != BPF_FS_MAGIC) {
+		pr_warning("specified path %s is not on BPF FS\n", path);
+		err = -EINVAL;
+	}
+
+	return err;
+}
+
+int bpf_program__pin_instance(struct bpf_program *prog, const char *path,
+			      int instance)
+{
+	int err;
+
+	err = check_path(path);
+	if (err)
+		return err;
+
+	if (prog == NULL) {
+		pr_warning("invalid program pointer\n");
+		return -EINVAL;
+	}
+
+	if (instance < 0 || instance >= prog->instances.nr) {
+		pr_warning("invalid prog instance %d of prog %s (max %d)\n",
+			   instance, prog->section_name, prog->instances.nr);
+		return -EINVAL;
+	}
+
+	if (bpf_obj_pin(prog->instances.fds[instance], path)) {
+		pr_warning("failed to pin program: %s\n", strerror(errno));
+		return -errno;
+	}
+	pr_debug("pinned program '%s'\n", path);
+
+	return 0;
+}
+
+static int make_dir(const char *path)
+{
+	int err = 0;
+
+	if (mkdir(path, 0700) && errno != EEXIST)
+		err = -errno;
+
+	if (err)
+		pr_warning("failed to mkdir %s: %s\n", path, strerror(-err));
+	return err;
+}
+
+int bpf_program__pin(struct bpf_program *prog, const char *path)
+{
+	int i, err;
+
+	err = check_path(path);
+	if (err)
+		return err;
+
+	if (prog == NULL) {
+		pr_warning("invalid program pointer\n");
+		return -EINVAL;
+	}
+
+	if (prog->instances.nr <= 0) {
+		pr_warning("no instances of prog %s to pin\n",
+			   prog->section_name);
+		return -EINVAL;
+	}
+
+	err = make_dir(path);
+	if (err)
+		return err;
+
+	for (i = 0; i < prog->instances.nr; i++) {
+		char buf[PATH_MAX];
+		int len;
+
+		len = snprintf(buf, PATH_MAX, "%s/%d", path, i);
+		if (len < 0)
+			return -EINVAL;
+		else if (len > PATH_MAX)
+			return -ENAMETOOLONG;
+
+		err = bpf_program__pin_instance(prog, buf, i);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
 void bpf_object__close(struct bpf_object *obj)
 {
 	size_t i;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 4014d1ba5e3d..9f8aa63b95f4 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -106,6 +106,9 @@ void *bpf_program__priv(struct bpf_program *prog);
 const char *bpf_program__title(struct bpf_program *prog, bool needs_copy);
 
 int bpf_program__fd(struct bpf_program *prog);
+int bpf_program__pin_instance(struct bpf_program *prog, const char *path,
+			      int instance);
+int bpf_program__pin(struct bpf_program *prog, const char *path);
 
 struct bpf_insn;
 
-- 
2.11.0

^ permalink raw reply related

* [PATCHv3 perf/core 2/6] tools lib bpf: Add bpf_map__pin()
From: Joe Stringer @ 2017-01-26 21:19 UTC (permalink / raw)
  To: acme; +Cc: wangnan0, ast, daniel, linux-kernel, netdev
In-Reply-To: <20170126212001.14103-1-joe@ovn.org>

Add a new API to pin a BPF map to the filesystem. The user can
specify the path full path within a BPF filesystem to pin the map.

Signed-off-by: Joe Stringer <joe@ovn.org>
---
v3: No change.
v2: Don't automount BPF filesystem
    Split program, map, object pinning into separate APIs and separate
    patches.
---
 tools/lib/bpf/libbpf.c | 22 ++++++++++++++++++++++
 tools/lib/bpf/libbpf.h |  1 +
 2 files changed, 23 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index d1d7638b7c21..ce987c02363e 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1349,6 +1349,28 @@ int bpf_program__pin(struct bpf_program *prog, const char *path)
 	return 0;
 }
 
+int bpf_map__pin(struct bpf_map *map, const char *path)
+{
+	int err;
+
+	err = check_path(path);
+	if (err)
+		return err;
+
+	if (map == NULL) {
+		pr_warning("invalid map pointer\n");
+		return -EINVAL;
+	}
+
+	if (bpf_obj_pin(map->fd, path)) {
+		pr_warning("failed to pin map: %s\n", strerror(errno));
+		return -errno;
+	}
+
+	pr_debug("pinned map '%s'\n", path);
+	return 0;
+}
+
 void bpf_object__close(struct bpf_object *obj)
 {
 	size_t i;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 9f8aa63b95f4..2addf9d5b13c 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -236,6 +236,7 @@ typedef void (*bpf_map_clear_priv_t)(struct bpf_map *, void *);
 int bpf_map__set_priv(struct bpf_map *map, void *priv,
 		      bpf_map_clear_priv_t clear_priv);
 void *bpf_map__priv(struct bpf_map *map);
+int bpf_map__pin(struct bpf_map *map, const char *path);
 
 long libbpf_get_error(const void *ptr);
 
-- 
2.11.0

^ permalink raw reply related

* [PATCHv3 perf/core 4/6] tools perf util: Make rm_rf(path) argument const
From: Joe Stringer @ 2017-01-26 21:19 UTC (permalink / raw)
  To: acme; +Cc: wangnan0, ast, daniel, linux-kernel, netdev
In-Reply-To: <20170126212001.14103-1-joe@ovn.org>

rm_rf() doesn't modify its path argument, and a future caller will pass
a string constant into it to delete.

Signed-off-by: Joe Stringer <joe@ovn.org>
---
v3: Initial post.
---
 tools/perf/util/util.c | 2 +-
 tools/perf/util/util.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index bf29aed16bd6..d8b45cea54d0 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -85,7 +85,7 @@ int mkdir_p(char *path, mode_t mode)
 	return (stat(path, &st) && mkdir(path, mode)) ? -1 : 0;
 }
 
-int rm_rf(char *path)
+int rm_rf(const char *path)
 {
 	DIR *dir;
 	int ret = 0;
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 6e8be174ec0b..c74708da8571 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -209,7 +209,7 @@ static inline int sane_case(int x, int high)
 }
 
 int mkdir_p(char *path, mode_t mode);
-int rm_rf(char *path);
+int rm_rf(const char *path);
 struct strlist *lsdir(const char *name, bool (*filter)(const char *, struct dirent *));
 bool lsdir_no_dot_filter(const char *name, struct dirent *d);
 int copyfile(const char *from, const char *to);
-- 
2.11.0

^ permalink raw reply related

* [PATCHv3 perf/core 5/6] tools lib api fs: Add bpf_fs filesystem detector
From: Joe Stringer @ 2017-01-26 21:20 UTC (permalink / raw)
  To: acme; +Cc: wangnan0, ast, daniel, linux-kernel, netdev
In-Reply-To: <20170126212001.14103-1-joe@ovn.org>

Allow mounting of the BPF filesystem at /sys/fs/bpf.

Signed-off-by: Joe Stringer <joe@ovn.org>
---
v3: Initial post.
---
 tools/lib/api/fs/fs.c | 16 ++++++++++++++++
 tools/lib/api/fs/fs.h |  1 +
 2 files changed, 17 insertions(+)

diff --git a/tools/lib/api/fs/fs.c b/tools/lib/api/fs/fs.c
index f99f49e4a31e..4b6bfc43cccf 100644
--- a/tools/lib/api/fs/fs.c
+++ b/tools/lib/api/fs/fs.c
@@ -38,6 +38,10 @@
 #define HUGETLBFS_MAGIC        0x958458f6
 #endif
 
+#ifndef BPF_FS_MAGIC
+#define BPF_FS_MAGIC           0xcafe4a11
+#endif
+
 static const char * const sysfs__fs_known_mountpoints[] = {
 	"/sys",
 	0,
@@ -75,6 +79,11 @@ static const char * const hugetlbfs__known_mountpoints[] = {
 	0,
 };
 
+static const char * const bpf_fs__known_mountpoints[] = {
+	"/sys/fs/bpf",
+	0,
+};
+
 struct fs {
 	const char		*name;
 	const char * const	*mounts;
@@ -89,6 +98,7 @@ enum {
 	FS__DEBUGFS = 2,
 	FS__TRACEFS = 3,
 	FS__HUGETLBFS = 4,
+	FS__BPF_FS = 5,
 };
 
 #ifndef TRACEFS_MAGIC
@@ -121,6 +131,11 @@ static struct fs fs__entries[] = {
 		.mounts = hugetlbfs__known_mountpoints,
 		.magic	= HUGETLBFS_MAGIC,
 	},
+	[FS__BPF_FS] = {
+		.name	= "bpf",
+		.mounts = bpf_fs__known_mountpoints,
+		.magic	= BPF_FS_MAGIC,
+	},
 };
 
 static bool fs__read_mounts(struct fs *fs)
@@ -280,6 +295,7 @@ FS(procfs,  FS__PROCFS);
 FS(debugfs, FS__DEBUGFS);
 FS(tracefs, FS__TRACEFS);
 FS(hugetlbfs, FS__HUGETLBFS);
+FS(bpf_fs, FS__BPF_FS);
 
 int filename__read_int(const char *filename, int *value)
 {
diff --git a/tools/lib/api/fs/fs.h b/tools/lib/api/fs/fs.h
index a63269f5d20c..6b332dc74498 100644
--- a/tools/lib/api/fs/fs.h
+++ b/tools/lib/api/fs/fs.h
@@ -22,6 +22,7 @@ FS(procfs)
 FS(debugfs)
 FS(tracefs)
 FS(hugetlbfs)
+FS(bpf_fs)
 
 #undef FS
 
-- 
2.11.0

^ permalink raw reply related

* [PATCHv3 perf/core 6/6] perf test: Add libbpf pinning test
From: Joe Stringer @ 2017-01-26 21:20 UTC (permalink / raw)
  To: acme; +Cc: wangnan0, ast, daniel, linux-kernel, netdev
In-Reply-To: <20170126212001.14103-1-joe@ovn.org>

Add a test for the newly added BPF object pinning functionality.

For example:
  # tools/perf/perf test 37
    37: BPF filter                                 :
    37.1: Basic BPF filtering                      : Ok
    37.2: BPF pinning                              : Ok
    37.3: BPF prologue generation                  : Ok
    37.4: BPF relocation checker                   : Ok

  # tools/perf/perf test 37 -v 2>&1 | grep pinned
    libbpf: pinned map '/sys/fs/bpf/perf_test/flip_table'
    libbpf: pinned program '/sys/fs/bpf/perf_test/func=SyS_epoll_wait/0'

Signed-off-by: Joe Stringer <joe@ovn.org>
Cc: Wang Nan <wangnan0@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
---
v3: Initial post.
---
 tools/perf/tests/bpf.c | 42 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
index 92343f43e44a..1a04fe77487d 100644
--- a/tools/perf/tests/bpf.c
+++ b/tools/perf/tests/bpf.c
@@ -5,11 +5,13 @@
 #include <util/evlist.h>
 #include <linux/bpf.h>
 #include <linux/filter.h>
+#include <api/fs/fs.h>
 #include <bpf/bpf.h>
 #include "tests.h"
 #include "llvm.h"
 #include "debug.h"
 #define NR_ITERS       111
+#define PERF_TEST_BPF_PATH "/sys/fs/bpf/perf_test"
 
 #ifdef HAVE_LIBBPF_SUPPORT
 
@@ -54,6 +56,7 @@ static struct {
 	const char *msg_load_fail;
 	int (*target_func)(void);
 	int expect_result;
+	bool	pin;
 } bpf_testcase_table[] = {
 	{
 		LLVM_TESTCASE_BASE,
@@ -63,6 +66,17 @@ static struct {
 		"load bpf object failed",
 		&epoll_wait_loop,
 		(NR_ITERS + 1) / 2,
+		false,
+	},
+	{
+		LLVM_TESTCASE_BASE,
+		"BPF pinning",
+		"[bpf_pinning]",
+		"fix kbuild first",
+		"check your vmlinux setting?",
+		&epoll_wait_loop,
+		(NR_ITERS + 1) / 2,
+		true,
 	},
 #ifdef HAVE_BPF_PROLOGUE
 	{
@@ -73,6 +87,7 @@ static struct {
 		"check your vmlinux setting?",
 		&llseek_loop,
 		(NR_ITERS + 1) / 4,
+		false,
 	},
 #endif
 	{
@@ -83,6 +98,7 @@ static struct {
 		"libbpf error when dealing with relocation",
 		NULL,
 		0,
+		false,
 	},
 };
 
@@ -226,10 +242,34 @@ static int __test__bpf(int idx)
 		goto out;
 	}
 
-	if (obj)
+	if (obj) {
 		ret = do_test(obj,
 			      bpf_testcase_table[idx].target_func,
 			      bpf_testcase_table[idx].expect_result);
+		if (ret != TEST_OK)
+			goto out;
+		if (bpf_testcase_table[idx].pin) {
+			int err;
+
+			if (!bpf_fs__mount()) {
+				pr_debug("BPF filesystem not mounted\n");
+				ret = TEST_FAIL;
+				goto out;
+			}
+			err = mkdir(PERF_TEST_BPF_PATH, 0777);
+			if (err && errno != EEXIST) {
+				pr_debug("Failed to make perf_test dir: %s\n",
+					 strerror(errno));
+				ret = TEST_FAIL;
+				goto out;
+			}
+			if (bpf_object__pin(obj, PERF_TEST_BPF_PATH))
+				ret = TEST_FAIL;
+			if (rm_rf(PERF_TEST_BPF_PATH))
+				ret = TEST_FAIL;
+		}
+	}
+
 out:
 	bpf__clear();
 	return ret;
-- 
2.11.0

^ permalink raw reply related

* [PATCHv3 perf/core 0/6] Libbpf object pinning
From: Joe Stringer @ 2017-01-26 21:19 UTC (permalink / raw)
  To: acme; +Cc: wangnan0, ast, daniel, linux-kernel, netdev

This series adds pinning functionality for maps, programs, and objects.
Library users may call bpf_map__pin(map, path) or bpf_program__pin(prog, path)
to pin maps and programs separately, or use bpf_object__pin(obj, path) to
pin all maps and programs from the BPF object to the path. The map and program
variations require a path where it will be pinned in the filesystem,
and the object variation will create named directories for each program with
instances within, and mount the maps by name under the path.

For example, with the directory '/sys/fs/bpf/foo' and a BPF object which
contains two instances of a program named 'bar', and a map named 'baz':
/sys/fs/bpf/foo/bar/0
/sys/fs/bpf/foo/bar/1
/sys/fs/bpf/foo/baz

---
v3: Split out bpf_program__pin_instance().
    Change the paths from PATH/{maps,progs}/foo to the above.
    Drop the patches that were applied.
    Add a perf test to check that pinning works.
v2: Wang Nan provided improvements to patch 1.
    Dropped patch 2 from v1.
    Added acks for acked patches.
    Split the bpf_obj__pin() to also provide map / program pinning APIs.
    Allow users to provide full filesystem path (don't autodetect/mount BPFFS).
v1: Initial post.

Joe Stringer (6):
  tools lib bpf: Add BPF program pinning APIs.
  tools lib bpf: Add bpf_map__pin()
  tools lib bpf: Add bpf_object__pin()
  tools perf util: Make rm_rf(path) argument const
  tools lib api fs: Add bpf_fs filesystem detector
  perf test: Add libbpf pinning test

 tools/lib/api/fs/fs.c  |  16 +++++
 tools/lib/api/fs/fs.h  |   1 +
 tools/lib/bpf/libbpf.c | 188 +++++++++++++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h |   5 ++
 tools/perf/tests/bpf.c |  42 ++++++++++-
 tools/perf/util/util.c |   2 +-
 tools/perf/util/util.h |   2 +-
 7 files changed, 253 insertions(+), 3 deletions(-)

-- 
2.11.0

^ permalink raw reply

* [PATCHv3 perf/core 3/6] tools lib bpf: Add bpf_object__pin()
From: Joe Stringer @ 2017-01-26 21:19 UTC (permalink / raw)
  To: acme; +Cc: wangnan0, ast, daniel, linux-kernel, netdev
In-Reply-To: <20170126212001.14103-1-joe@ovn.org>

Add a new API to pin a BPF object to the filesystem. The user can
specify the path within a BPF filesystem to pin the object.
Programs will be pinned under a subdirectory named the same as the
program, with each instance appearing as a numbered file under that
directory, and maps will be pinned under the path using the name of
the map as the file basename.

For example, with the directory '/sys/fs/bpf/foo' and a BPF object which
contains two instances of a program named 'bar', and a map named 'baz':
/sys/fs/bpf/foo/bar/0
/sys/fs/bpf/foo/bar/1
/sys/fs/bpf/foo/baz

Signed-off-by: Joe Stringer <joe@ovn.org>
---
v3: Mount to PATH/MAPNAME, PATH/PROGNAME/N (N = instance number)
v2: Don't automount BPF filesystem
    Split program, map, object pinning into separate APIs and separate
    patches.
---
 tools/lib/bpf/libbpf.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h |  1 +
 2 files changed, 55 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index ce987c02363e..703cfa986b34 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -36,6 +36,7 @@
 #include <linux/magic.h>
 #include <linux/list.h>
 #include <linux/limits.h>
+#include <sys/stat.h>
 #include <sys/vfs.h>
 #include <libelf.h>
 #include <gelf.h>
@@ -1371,6 +1372,59 @@ int bpf_map__pin(struct bpf_map *map, const char *path)
 	return 0;
 }
 
+int bpf_object__pin(struct bpf_object *obj, const char *path)
+{
+	struct bpf_program *prog;
+	struct bpf_map *map;
+	int err;
+
+	if (!obj)
+		return -ENOENT;
+
+	if (!obj->loaded) {
+		pr_warning("object not yet loaded; load it first\n");
+		return -ENOENT;
+	}
+
+	err = make_dir(path);
+	if (err)
+		return err;
+
+	bpf_map__for_each(map, obj) {
+		char buf[PATH_MAX];
+		int len;
+
+		len = snprintf(buf, PATH_MAX, "%s/%s", path,
+			       bpf_map__name(map));
+		if (len < 0)
+			return -EINVAL;
+		else if (len > PATH_MAX)
+			return -ENAMETOOLONG;
+
+		err = bpf_map__pin(map, buf);
+		if (err)
+			return err;
+	}
+
+	bpf_object__for_each_program(prog, obj) {
+		char buf[PATH_MAX];
+		int len;
+
+		len = snprintf(buf, PATH_MAX, "%s/%s", path,
+			       prog->section_name);
+		if (len < 0)
+			return -EINVAL;
+		else if (len > PATH_MAX)
+			return -ENAMETOOLONG;
+
+		err = bpf_program__pin(prog, buf);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
 void bpf_object__close(struct bpf_object *obj)
 {
 	size_t i;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 2addf9d5b13c..b30394f9947a 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -65,6 +65,7 @@ struct bpf_object *bpf_object__open(const char *path);
 struct bpf_object *bpf_object__open_buffer(void *obj_buf,
 					   size_t obj_buf_sz,
 					   const char *name);
+int bpf_object__pin(struct bpf_object *object, const char *path);
 void bpf_object__close(struct bpf_object *object);
 
 /* Load/unload object into/from kernel */
-- 
2.11.0

^ permalink raw reply related

* Re: netvsc NAPI patch process
From: David Miller @ 2017-01-26 20:48 UTC (permalink / raw)
  To: kys; +Cc: stephen, greg, netdev
In-Reply-To: <DM5PR03MB249073BDE0362C6A7051AA8EA0770@DM5PR03MB2490.namprd03.prod.outlook.com>

From: KY Srinivasan <kys@microsoft.com>
Date: Thu, 26 Jan 2017 20:46:40 +0000

> In the past, we have done this in two stages - get the supporting
> vmbus patches into Greg's tree first and in the next merge cycle get
> the netvsc patches in. Why not continue to do what we have done in
> the past to address cross-tree dependencies.

It takes too damn long.

^ permalink raw reply

* Re: [PATCH RFC net-next] packet: always ensure that we pass hard_header_len bytes in skb_headlen() to the driver
From: Sowmini Varadhan @ 2017-01-26 21:37 UTC (permalink / raw)
  To: Willem de Bruijn; +Cc: David Miller, Network Development
In-Reply-To: <CAF=yD-KXqpODBVQ0vUt1a1p_mMXzzQp=5GU33f9GY9_cbPJxDw@mail.gmail.com>

On (01/26/17 15:21), Willem de Bruijn wrote:
> > If the application has provided fewer than hard_header_len bytes,
> > dev_validate_header() will zero out the skb->data as needed. This is
> > acceptable for SOCK_DGRAM/PF_PACKET sockets but in all other cases,
> 
> This was added not for datagram sockets, but to be able to bypass
> validation. See the message in commit 2793a23aacbd ("net: validate
> variable length ll header") and discussion leading up to that patch.

some context, I got inot this patch as a result of  the comments in
 https://www.mail-archive.com/netdev@vger.kernel.org/msg149031.html

> As David pointed out, this does not handle variable length headers
> correctly. In link layers that support these, hard_header_len defines
> the maximum header length. A hard failure on len < hard_header_len
> would be incorrect.

right, since DaveM's comments, I took a look at the drivers
that have a ->validate - afaict (from cscope) ax25 is the only 
in-kernel driver that actually passes a ->validate pointer.. 
I tried patching ax25 here:
  http://marc.info/?l=linux-hams&m=148537926422828&w=2
Still waiting to hear back from that list (which doesnt seem to have
much traffic so maybe I should time out on it). Does that
patch make better sense (I'll look up the comments leading up
to 2793a23aacbd later tonight)

> The ->validate callback was added to allow specifying additional
> constraints on a per protocol basis. This is where a min constraint
> can be added, e.g., for ethernet.
> 
> > -       if (!dev_validate_header(dev, skb->data, len)) {
> > +       newlen = dev_validate_header(dev, skb->data, len);
> > +       /* As comments above this function indicate, a full L2 header
> > +        * must be passed to this function, so if newlen > len, bail.
> > +        */
> > +       if (newlen < 0 || newlen > len) {
> 
> If callers only care whether the function returned failure or
> increased len, which also indicates failure, it is cleaner to leave it
> a boolean and fail in cases where len < the minimum for that link
> layer type. No caller actually uses newlen.
> 
> > +               /* Caller has allocated for copylen in non-paged part of
> > +                * skb so we should never find newlen > hdrlen
> > +                */
> > +               WARN_ON(newlen > hdrlen);
> 
> WARN_ON_ONCE is safer.

Ok that's easy enough to do.

^ permalink raw reply

* [PATCH net-next v2] net: ipv6: ignore null_entry on route dumps
From: David Ahern @ 2017-01-26 21:54 UTC (permalink / raw)
  To: netdev; +Cc: xiaolong.ye, David Ahern

lkp-robot reported a BUG:
[   10.151226] BUG: unable to handle kernel NULL pointer dereference at 00000198
[   10.152525] IP: rt6_fill_node+0x164/0x4b8
[   10.153307] *pdpt = 0000000012ee5001 *pde = 0000000000000000
[   10.153309]
[   10.154492] Oops: 0000 [#1]
[   10.154987] CPU: 0 PID: 909 Comm: netifd Not tainted 4.10.0-rc4-00722-g41e8c70ee162-dirty #10
[   10.156482] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.7.5-20140531_083030-gandalf 04/01/2014
[   10.158254] task: d0deb000 task.stack: d0e0c000
[   10.159059] EIP: rt6_fill_node+0x164/0x4b8
[   10.159780] EFLAGS: 00010296 CPU: 0
[   10.160404] EAX: 00000000 EBX: d10c2358 ECX: c1f7c6cc EDX: c1f6ff44
[   10.161469] ESI: 00000000 EDI: c2059900 EBP: d0e0dc4c ESP: d0e0dbe4
[   10.162534]  DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 0068
[   10.163482] CR0: 80050033 CR2: 00000198 CR3: 10d94660 CR4: 000006b0
[   10.164535] Call Trace:
[   10.164993]  ? paravirt_sched_clock+0x9/0xd
[   10.165727]  ? sched_clock+0x9/0xc
[   10.166329]  ? sched_clock_cpu+0x19/0xe9
[   10.166991]  ? lock_release+0x13e/0x36c
[   10.167652]  rt6_dump_route+0x4c/0x56
[   10.168276]  fib6_dump_node+0x1d/0x3d
[   10.168913]  fib6_walk_continue+0xab/0x167
[   10.169611]  fib6_walk+0x2a/0x40
[   10.170182]  inet6_dump_fib+0xfb/0x1e0
[   10.170855]  netlink_dump+0xcd/0x21f

This happens when the loopback device is set down and a ipv6 fib route
dump is requested.

ip6_null_entry is the root of all ipv6 fib tables making it integrated
into the table and hence passed to the ipv6 route dump code. The
null_entry route uses the loopback device for dst.dev but may not have
rt6i_idev set because of the order in which initializations are done --
ip6_route_net_init is run before addrconf_init has initialized the
loopback device. Fixing the initialization order is a much bigger problem
with no obvious solution thus far.

The BUG is triggered when the loopback is set down and the netif_running
check added by a1a22c1206 fails. The fill_node descends to checking
rt->rt6i_idev for ignore_routes_with_linkdown and since rt6i_idev is
NULL it faults.

The null_entry route should not be processed in a dump request. Catch
and ignore. This check is done in rt6_dump_route as it is the highest
place in the callchain with knowledge of both the route and the network
namespace.

Fixes: a1a22c1206("net: ipv6: Keep nexthop of multipath route on admin down")
Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
v2
- updated commit message; no code change

Dave: per last email you suggested putting this in fib6_dump_node, but
      fib6_dump_node does not have knowledge of the network namespace
      because of the way the fib_walker works. I could put
      struct rt6_rtnl_dump_arg *arg = (struct rt6_rtnl_dump_arg *) w->args;
      into fib6_dump_node to get the net pointer from it but did not
      want to duplicate the typecast.

 net/ipv6/route.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 4b1f0f98a0e9..47499ed429da 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -3320,6 +3320,10 @@ static int rt6_fill_node(struct net *net,
 int rt6_dump_route(struct rt6_info *rt, void *p_arg)
 {
 	struct rt6_rtnl_dump_arg *arg = (struct rt6_rtnl_dump_arg *) p_arg;
+	struct net *net = arg->net;
+
+	if (rt == net->ipv6.ip6_null_entry)
+		return 0;
 
 	if (nlmsg_len(arg->cb->nlh) >= sizeof(struct rtmsg)) {
 		struct rtmsg *rtm = nlmsg_data(arg->cb->nlh);
@@ -3332,7 +3336,7 @@ int rt6_dump_route(struct rt6_info *rt, void *p_arg)
 		}
 	}
 
-	return rt6_fill_node(arg->net,
+	return rt6_fill_node(net,
 		     arg->skb, rt, NULL, NULL, 0, RTM_NEWROUTE,
 		     NETLINK_CB(arg->cb->skb).portid, arg->cb->nlh->nlmsg_seq,
 		     NLM_F_MULTI);
-- 
2.1.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