* [PATCH v3 3/7] net: Increase the size of skb_frag_t
From: Matthew Wilcox @ 2019-07-12 13:43 UTC (permalink / raw)
To: davem; +Cc: Matthew Wilcox (Oracle), hch, netdev
In-Reply-To: <20190712134345.19767-1-willy@infradead.org>
From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
To increase commonality between block and net, we are going to replace
the skb_frag_t with the bio_vec. This patch increases the size of
skb_frag_t on 32-bit machines from 8 bytes to 12 bytes. The size is
unchanged on 64-bit machines.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
include/linux/skbuff.h | 5 -----
1 file changed, 5 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index f9078e7edb53..7910935410e6 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -314,13 +314,8 @@ struct skb_frag_struct {
struct {
struct page *p;
} page;
-#if (BITS_PER_LONG > 32) || (PAGE_SIZE >= 65536)
__u32 page_offset;
__u32 size;
-#else
- __u16 page_offset;
- __u16 size;
-#endif
};
/**
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v4 bpf-next 0/4] selftests/bpf: fix compiling loop{1,2,3}.c on s390
From: Daniel Borkmann @ 2019-07-12 13:44 UTC (permalink / raw)
To: Ilya Leoshkevich, Stanislav Fomichev; +Cc: bpf, Networking, Y Song, davem, ast
In-Reply-To: <994CF53F-3E84-4CE8-92C5-B2983AD50EB8@linux.ibm.com>
On 07/12/2019 10:55 AM, Ilya Leoshkevich wrote:
>> Am 11.07.2019 um 22:35 schrieb Stanislav Fomichev <sdf@fomichev.me>:
>>
>> On 07/11, Ilya Leoshkevich wrote:
>>> Use PT_REGS_RC(ctx) instead of ctx->rax, which is not present on s390.
>>>
>>> This patch series consists of three preparatory commits, which make it
>>> possible to use PT_REGS_RC in BPF selftests, followed by the actual fix.
>>>
>> Still looks good to me, thanks!
>>
>> Reviewed-by: Stanislav Fomichev <sdf@google.com>
>>
>> Again, should probably go via bpf to fix the existing tests, not bpf-next
>> (but I see bpf tree is not synced with net tree yet).
>
> Sorry, I missed your comment the last time. You are right - that’s the
> reason I’ve been sending this to bpf-next so far — loop*.c don’t even
> exist in the bpf tree.
Applied to bpf tree (and also added Stanislav's Tested-by to the last
one), thanks!
^ permalink raw reply
* Re: [PATCH net-next 00/11] Add drop monitor for offloaded data paths
From: Ido Schimmel @ 2019-07-12 13:52 UTC (permalink / raw)
To: Neil Horman
Cc: David Miller, netdev, jiri, mlxsw, dsahern, roopa, nikolay, andy,
pablo, jakub.kicinski, pieter.jansenvanvuuren, andrew, f.fainelli,
vivien.didelot, idosch
In-Reply-To: <20190711235354.GA30396@hmswarspite.think-freely.org>
On Thu, Jul 11, 2019 at 07:53:54PM -0400, Neil Horman wrote:
> A few things here:
> IIRC we don't announce individual hardware drops, drivers record them in
> internal structures, and they are retrieved on demand via ethtool calls, so you
> will either need to include some polling (probably not a very performant idea),
> or some sort of flagging mechanism to indicate that on the next message sent to
> user space you should go retrieve hw stats from a given interface. I certainly
> wouldn't mind seeing this happen, but its more work than just adding a new
> netlink message.
Neil,
The idea of this series is to pass the dropped packets themselves to
user space along with metadata, such as the drop reason and the ingress
port. In the future more metadata could be added thanks to the
extensible nature of netlink.
In v1 these packets were notified to user space as devlink events
and my plan for v2 is to send them as drop_monitor events, given it's an
existing generic netlink channel used to monitor SW drops. This will
allow users to listen on one netlink channel to diagnose potential
problems in either SW or HW (and hopefully XDP in the future).
Please note that the packets I'm talking about are packets users
currently do not see. They are dropped - potentially silently - by the
underlying device, thereby making it hard to debug whatever issues you
might be experiencing in your network.
The control path that determines if these packets are even sent to the
CPU from the HW needs to remain in devlink for the reasons I outlined in
my previous reply. However, the monitoring of these drops will be over
drop_monitor. This is similar to what we are currently doing with
tc-sample, where the control path that triggers the sampling and
determines the sampling rate and truncation is done over rtnetlink (tc),
but the sampled packets are notified over the generic netlink psample
channel.
To make it more real, you can check the example of the dissected devlink
message that notifies the drop of a packet due to a multicast source
MAC: https://marc.info/?l=linux-netdev&m=156248736710238&w=2
I will obviously have to create another Wireshark dissector for
drop_monitor in order to get the same information.
> Thats an interesting idea, but dropwatch certainly isn't currently setup for
> that kind of messaging. It may be worth creating a v2 of the netlink protocol
> and really thinking out what you want to communicate.
I don't think we need a v2 of the netlink protocol. My current plan is
to extend the existing protocol with: New message type (e.g.,
NET_DM_CMD_HW_ALERT), new multicast group and a set of attributes to
encode the information that is currently encoded in the example message
I pasted above.
Thanks
^ permalink raw reply
* Re: [PATCH v1 1/6] rcu: Add support for consolidated-RCU reader checking
From: Joel Fernandes @ 2019-07-12 13:56 UTC (permalink / raw)
To: Oleg Nesterov
Cc: linux-kernel, Alexey Kuznetsov, Bjorn Helgaas, Borislav Petkov,
c0d1n61at3, David S. Miller, edumazet, Greg Kroah-Hartman,
Hideaki YOSHIFUJI, H. Peter Anvin, Ingo Molnar, Josh Triplett,
keescook, kernel-hardening, Lai Jiangshan, Len Brown, linux-acpi,
linux-pci, linux-pm, Mathieu Desnoyers, neilb, netdev,
Paul E. McKenney, Pavel Machek, peterz, Rafael J. Wysocki,
Rasmus Villemoes, rcu, Steven Rostedt, Tejun Heo, Thomas Gleixner,
will, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
In-Reply-To: <20190712121200.GC21989@redhat.com>
On Fri, Jul 12, 2019 at 02:12:00PM +0200, Oleg Nesterov wrote:
> On 07/11, Joel Fernandes (Google) wrote:
> >
> > +int rcu_read_lock_any_held(void)
>
> rcu_sync_is_idle() wants it. You have my ack in advance ;)
Cool, thanks ;)
- Joel
^ permalink raw reply
* [PATCH bpf] selftests/bpf: make directory prerequisites order-only
From: Ilya Leoshkevich @ 2019-07-12 13:56 UTC (permalink / raw)
To: bpf, netdev; +Cc: gor, heiko.carstens, Ilya Leoshkevich
When directories are used as prerequisites in Makefiles, they can cause
a lot of unnecessary rebuilds, because a directory is considered changed
whenever a file in this directory is added, removed or modified.
If the only thing a target is interested in is the existence of the
directory it depends on, which is the case for selftests/bpf, this
directory should be specified as an order-only prerequisite: it would
still be created in case it does not exist, but it would not trigger a
rebuild of a target in case it's considered changed.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/testing/selftests/bpf/Makefile | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 277d8605e340..0e003fb6641b 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -183,12 +183,12 @@ TEST_CUSTOM_PROGS += $(ALU32_BUILD_DIR)/test_progs_32
$(ALU32_BUILD_DIR):
mkdir -p $@
-$(ALU32_BUILD_DIR)/urandom_read: $(OUTPUT)/urandom_read
+$(ALU32_BUILD_DIR)/urandom_read: $(OUTPUT)/urandom_read | $(ALU32_BUILD_DIR)
cp $< $@
$(ALU32_BUILD_DIR)/test_progs_32: test_progs.c $(OUTPUT)/libbpf.a\
- $(ALU32_BUILD_DIR) \
- $(ALU32_BUILD_DIR)/urandom_read
+ $(ALU32_BUILD_DIR)/urandom_read \
+ | $(ALU32_BUILD_DIR)
$(CC) $(TEST_PROGS_CFLAGS) $(CFLAGS) \
-o $(ALU32_BUILD_DIR)/test_progs_32 \
test_progs.c test_stub.c trace_helpers.c prog_tests/*.c \
@@ -197,8 +197,8 @@ $(ALU32_BUILD_DIR)/test_progs_32: test_progs.c $(OUTPUT)/libbpf.a\
$(ALU32_BUILD_DIR)/test_progs_32: $(PROG_TESTS_H)
$(ALU32_BUILD_DIR)/test_progs_32: prog_tests/*.c
-$(ALU32_BUILD_DIR)/%.o: progs/%.c $(ALU32_BUILD_DIR) \
- $(ALU32_BUILD_DIR)/test_progs_32
+$(ALU32_BUILD_DIR)/%.o: progs/%.c $(ALU32_BUILD_DIR)/test_progs_32 \
+ | $(ALU32_BUILD_DIR)
($(CLANG) $(CLANG_FLAGS) -O2 -target bpf -emit-llvm -c $< -o - || \
echo "clang failed") | \
$(LLC) -march=bpf -mattr=+alu32 -mcpu=$(CPU) $(LLC_FLAGS) \
@@ -236,7 +236,7 @@ $(PROG_TESTS_DIR):
mkdir -p $@
PROG_TESTS_FILES := $(wildcard prog_tests/*.c)
-$(PROG_TESTS_H): $(PROG_TESTS_DIR) $(PROG_TESTS_FILES)
+$(PROG_TESTS_H): $(PROG_TESTS_FILES) | $(PROG_TESTS_DIR)
$(shell ( cd prog_tests/; \
echo '/* Generated header, do not edit */'; \
echo '#ifdef DECLARE'; \
@@ -257,7 +257,7 @@ MAP_TESTS_H := $(MAP_TESTS_DIR)/tests.h
test_maps.c: $(MAP_TESTS_H)
$(OUTPUT)/test_maps: CFLAGS += $(TEST_MAPS_CFLAGS)
MAP_TESTS_FILES := $(wildcard map_tests/*.c)
-$(MAP_TESTS_H): $(MAP_TESTS_DIR) $(MAP_TESTS_FILES)
+$(MAP_TESTS_H): $(MAP_TESTS_FILES) | $(MAP_TESTS_DIR)
$(shell ( cd map_tests/; \
echo '/* Generated header, do not edit */'; \
echo '#ifdef DECLARE'; \
@@ -279,7 +279,7 @@ $(VERIFIER_TESTS_DIR):
mkdir -p $@
VERIFIER_TEST_FILES := $(wildcard verifier/*.c)
-$(OUTPUT)/verifier/tests.h: $(VERIFIER_TESTS_DIR) $(VERIFIER_TEST_FILES)
+$(OUTPUT)/verifier/tests.h: $(VERIFIER_TEST_FILES) | $(VERIFIER_TESTS_DIR)
$(shell ( cd verifier/; \
echo '/* Generated header, do not edit */'; \
echo '#ifdef FILL_ARRAY'; \
--
2.21.0
^ permalink raw reply related
* Re: [PATCH bpf-next] selftests/bpf: remove logic duplication in test_verifier.c
From: Daniel Borkmann @ 2019-07-12 13:57 UTC (permalink / raw)
To: Krzesimir Nowak, Andrii Nakryiko
Cc: Andrii Nakryiko, Kernel Team, Alexei Starovoitov, bpf, Networking
In-Reply-To: <CAGGp+cHaV1EMXqeQvKN-p5gEZWcSgGfcbKimcS+C8u=dfeU=1Q@mail.gmail.com>
On 07/12/2019 09:53 AM, Krzesimir Nowak wrote:
> On Thu, Jul 11, 2019 at 4:43 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
>>
>> On Thu, Jul 11, 2019 at 5:13 AM Krzesimir Nowak <krzesimir@kinvolk.io> wrote:
>>>
>>> On Thu, Jul 11, 2019 at 3:08 AM Andrii Nakryiko <andriin@fb.com> wrote:
>>>>
>>>> test_verifier tests can specify single- and multi-runs tests. Internally
>>>> logic of handling them is duplicated. Get rid of it by making single run
>>>> retval specification to be a first retvals spec.
>>>>
>>>> Cc: Krzesimir Nowak <krzesimir@kinvolk.io>
>>>> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>>>
>>> Looks good, one nit below.
>>>
>>> Acked-by: Krzesimir Nowak <krzesimir@kinvolk.io>
>>>
>>>> ---
>>>> tools/testing/selftests/bpf/test_verifier.c | 37 ++++++++++-----------
>>>> 1 file changed, 18 insertions(+), 19 deletions(-)
>>>>
>>>> diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
>>>> index b0773291012a..120ecdf4a7db 100644
>>>> --- a/tools/testing/selftests/bpf/test_verifier.c
>>>> +++ b/tools/testing/selftests/bpf/test_verifier.c
>>>> @@ -86,7 +86,7 @@ struct bpf_test {
>>>> int fixup_sk_storage_map[MAX_FIXUPS];
>>>> const char *errstr;
>>>> const char *errstr_unpriv;
>>>> - uint32_t retval, retval_unpriv, insn_processed;
>>>> + uint32_t insn_processed;
>>>> int prog_len;
>>>> enum {
>>>> UNDEF,
>>>> @@ -95,16 +95,24 @@ struct bpf_test {
>>>> } result, result_unpriv;
>>>> enum bpf_prog_type prog_type;
>>>> uint8_t flags;
>>>> - __u8 data[TEST_DATA_LEN];
>>>> void (*fill_helper)(struct bpf_test *self);
>>>> uint8_t runs;
>>>> - struct {
>>>> - uint32_t retval, retval_unpriv;
>>>> - union {
>>>> - __u8 data[TEST_DATA_LEN];
>>>> - __u64 data64[TEST_DATA_LEN / 8];
>>>> + union {
>>>> + struct {
>>>
>>> Maybe consider moving the struct definition outside to further the
>>> removal of the duplication?
>>
>> Can't do that because then retval/retval_unpriv/data won't be
>> accessible as a normal field of struct bpf_test. It has to be in
>> anonymous structs/unions, unfortunately.
>>
>
> Ah, right.
>
> Meh.
>
> I tried something like this:
>
> #define BPF_DATA_STRUCT \
> struct { \
> uint32_t retval, retval_unpriv; \
> union { \
> __u8 data[TEST_DATA_LEN]; \
> __u64 data64[TEST_DATA_LEN / 8]; \
> }; \
> }
>
> and then:
>
> union {
> BPF_DATA_STRUCT;
> BPF_DATA_STRUCT retvals[MAX_TEST_RUNS];
> };
>
> And that seems to compile at least. But question is: is this
> acceptably ugly or unacceptably ugly? :)
Both a bit ugly, but I'd have a slight preference towards the above,
perhaps a bit more readable like:
#define bpf_testdata_struct_t \
struct { \
uint32_t retval, retval_unpriv; \
union { \
__u8 data[TEST_DATA_LEN]; \
__u64 data64[TEST_DATA_LEN / 8]; \
}; \
}
union {
bpf_testdata_struct_t;
bpf_testdata_struct_t retvals[MAX_TEST_RUNS];
};
Thanks,
Daniel
^ permalink raw reply
* [PATCH bpf] selftests/bpf: put test_stub.o into $(OUTPUT)
From: Ilya Leoshkevich @ 2019-07-12 13:59 UTC (permalink / raw)
To: bpf, netdev; +Cc: gor, heiko.carstens, Ilya Leoshkevich
Add a rule to put test_stub.o in $(OUTPUT) and change the references to
it accordingly. This prevents test_stub.o from being created in the
source directory.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/testing/selftests/bpf/Makefile | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 277d8605e340..66b6f7fb683c 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -83,13 +83,16 @@ all: $(TEST_CUSTOM_PROGS)
$(OUTPUT)/urandom_read: $(OUTPUT)/%: %.c
$(CC) -o $@ $< -Wl,--build-id
+$(OUTPUT)/test_stub.o: test_stub.c
+ $(CC) $(TEST_PROGS_CFLAGS) $(CFLAGS) -c -o $@ $<
+
$(OUTPUT)/test_maps: map_tests/*.c
BPFOBJ := $(OUTPUT)/libbpf.a
-$(TEST_GEN_PROGS): test_stub.o $(BPFOBJ)
+$(TEST_GEN_PROGS): $(OUTPUT)/test_stub.o $(BPFOBJ)
-$(TEST_GEN_PROGS_EXTENDED): test_stub.o $(OUTPUT)/libbpf.a
+$(TEST_GEN_PROGS_EXTENDED): $(OUTPUT)/test_stub.o $(OUTPUT)/libbpf.a
$(OUTPUT)/test_dev_cgroup: cgroup_helpers.c
$(OUTPUT)/test_skb_cgroup_id_user: cgroup_helpers.c
--
2.21.0
^ permalink raw reply related
* Re: [PATCH v3 0/3] kernel/notifier.c: avoid duplicate registration
From: gregkh @ 2019-07-12 14:07 UTC (permalink / raw)
To: Xiaoming Ni
Cc: Vasily Averin, adobriyan@gmail.com, akpm@linux-foundation.org,
anna.schumaker@netapp.com, arjan@linux.intel.com,
bfields@fieldses.org, chuck.lever@oracle.com, davem@davemloft.net,
jlayton@kernel.org, luto@kernel.org, mingo@kernel.org,
Nadia.Derbey@bull.net, paulmck@linux.vnet.ibm.com,
semen.protsenko@linaro.org, stable@kernel.org,
stern@rowland.harvard.edu, tglx@linutronix.de,
torvalds@linux-foundation.org, trond.myklebust@hammerspace.com,
viresh.kumar@linaro.org, Huangjianhui (Alex), Dailei,
linux-kernel@vger.kernel.org, linux-nfs@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <8ee6f763-ccce-ab58-3d96-21f5e1622916@huawei.com>
On Fri, Jul 12, 2019 at 09:11:57PM +0800, Xiaoming Ni wrote:
> On 2019/7/11 21:57, Vasily Averin wrote:
> > On 7/11/19 4:55 AM, Nixiaoming wrote:
> >> On Wed, July 10, 2019 1:49 PM Vasily Averin wrote:
> >>> On 7/10/19 6:09 AM, Xiaoming Ni wrote:
> >>>> Registering the same notifier to a hook repeatedly can cause the hook
> >>>> list to form a ring or lose other members of the list.
> >>>
> >>> I think is not enough to _prevent_ 2nd register attempt,
> >>> it's enough to detect just attempt and generate warning to mark host in bad state.
> >>>
> >>
> >> Duplicate registration is prevented in my patch, not just "mark host in bad state"
> >>
> >> Duplicate registration is checked and exited in notifier_chain_cond_register()
> >>
> >> Duplicate registration was checked in notifier_chain_register() but only
> >> the alarm was triggered without exiting. added by commit 831246570d34692e
> >> ("kernel/notifier.c: double register detection")
> >>
> >> My patch is like a combination of 831246570d34692e and notifier_chain_cond_register(),
> >> which triggers an alarm and exits when a duplicate registration is detected.
> >>
> >>> Unexpected 2nd register of the same hook most likely will lead to 2nd unregister,
> >>> and it can lead to host crash in any time:
> >>> you can unregister notifier on first attempt it can be too early, it can be still in use.
> >>> on the other hand you can never call 2nd unregister at all.
> >>
> >> Since the member was not added to the linked list at the time of the second registration,
> >> no linked list ring was formed.
> >> The member is released on the first unregistration and -ENOENT on the second unregistration.
> >> After patching, the fault has been alleviated
> >
> > You are wrong here.
> > 2nd notifier's registration is a pure bug, this should never happen.
> > If you know the way to reproduce this situation -- you need to fix it.
> >
> > 2nd registration can happen in 2 cases:
> > 1) missed rollback, when someone forget to call unregister after successfull registration,
> > and then tried to call register again. It can lead to crash for example when according module will be unloaded.
> > 2) some subsystem is registered twice, for example from different namespaces.
> > in this case unregister called during sybsystem cleanup in first namespace will incorrectly remove notifier used
> > in second namespace, it also can lead to unexpacted behaviour.
> >
> So in these two cases, is it more reasonable to trigger BUG() directly when checking for duplicate registration ?
> But why does current notifier_chain_register() just trigger WARN() without exiting ?
> notifier_chain_cond_register() direct exit without triggering WARN() ?
It should recover from this, if it can be detected. The main point is
that not all apis have to be this "robust" when used within the kernel
as we do allow for the callers to know what they are doing :)
If this does not cause any additional problems or slow downs, it's
probably fine to add.
thanks,
greg k-h
^ permalink raw reply
* Linux Plumbers BPF micro-conference CFP (reminder)
From: Daniel Borkmann @ 2019-07-12 14:26 UTC (permalink / raw)
To: bpf
Cc: netdev, linux-kernel, xdp-newbies, iovisor-dev, lpc-bpf,
alexei.starovoitov
In-Reply-To: <2e9f33c9-b772-396e-1e70-2e2d5027cac5@iogearbox.net>
This is a call for proposals for the BPF micro-conference at this
years' Linux Plumbers Conference (LPC) 2019 which will be held in
Lisbon, Portugal for September 9-11.
The goal of the BPF micro-conference is to bring BPF developers
together to discuss topics around Linux kernel work related to
the BPF core infrastructure as well as its many subsystems under
tracing, networking, security, and BPF user space tooling (LLVM,
libbpf, bpftool and many others).
The format of the micro-conference has a main focus on discussion,
therefore each accepted topic will provide a short 1-2 slide
introduction with subsequent discussion for the rest of the given
time slot.
The BPF micro-conference is a community-driven event and open to
all LPC attendees, there is no additional registration required.
Please submit your discussion proposals to the LPC BPF micro-conference
organizers at:
lpc-bpf@vger.kernel.org
Proposals must be submitted until August 2nd, and submitters will
be notified of acceptance at latest by August 9. (Please note that
proposals must not be sent as html mail as they are otherwise dropped
by vger.)
The format of the submission and many other details can be found at:
http://vger.kernel.org/lpc-bpf.html
Looking forward to seeing you all in Lisbon in September!
^ permalink raw reply
* Re: [PATCH v1 1/6] rcu: Add support for consolidated-RCU reader checking
From: Joel Fernandes @ 2019-07-12 14:49 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, Alexey Kuznetsov, Bjorn Helgaas, Borislav Petkov,
c0d1n61at3, David S. Miller, edumazet, Greg Kroah-Hartman,
Hideaki YOSHIFUJI, H. Peter Anvin, Ingo Molnar, Josh Triplett,
keescook, kernel-hardening, Lai Jiangshan, Len Brown, linux-acpi,
linux-pci, linux-pm, Mathieu Desnoyers, neilb, netdev, oleg,
Paul E. McKenney, Pavel Machek, Rafael J. Wysocki,
Rasmus Villemoes, rcu, Steven Rostedt, Tejun Heo, Thomas Gleixner,
will, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
In-Reply-To: <20190712110142.GS3402@hirez.programming.kicks-ass.net>
On Fri, Jul 12, 2019 at 01:01:42PM +0200, Peter Zijlstra wrote:
> On Thu, Jul 11, 2019 at 07:43:56PM -0400, Joel Fernandes (Google) wrote:
> > This patch adds support for checking RCU reader sections in list
> > traversal macros. Optionally, if the list macro is called under SRCU or
> > other lock/mutex protection, then appropriate lockdep expressions can be
> > passed to make the checks pass.
> >
> > Existing list_for_each_entry_rcu() invocations don't need to pass the
> > optional fourth argument (cond) unless they are under some non-RCU
> > protection and needs to make lockdep check pass.
> >
> > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > ---
> > include/linux/rculist.h | 29 ++++++++++++++++++++++++-----
> > include/linux/rcupdate.h | 7 +++++++
> > kernel/rcu/Kconfig.debug | 11 +++++++++++
> > kernel/rcu/update.c | 26 ++++++++++++++++++++++++++
> > 4 files changed, 68 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/linux/rculist.h b/include/linux/rculist.h
> > index e91ec9ddcd30..78c15ec6b2c9 100644
> > --- a/include/linux/rculist.h
> > +++ b/include/linux/rculist.h
> > @@ -40,6 +40,23 @@ static inline void INIT_LIST_HEAD_RCU(struct list_head *list)
> > */
> > #define list_next_rcu(list) (*((struct list_head __rcu **)(&(list)->next)))
> >
> > +/*
> > + * Check during list traversal that we are within an RCU reader
> > + */
> > +
> > +#define SIXTH_ARG(a1, a2, a3, a4, a5, a6, ...) a6
> > +#define COUNT_VARGS(...) SIXTH_ARG(dummy, ## __VA_ARGS__, 4, 3, 2, 1, 0)
>
> You don't seem to actually use it in this patch; also linux/kernel.h has
> COUNT_ARGS().
Yes, I replied after sending patches that I fixed this. I will remove them.
thanks,
- Joel
^ permalink raw reply
* Re: [PATCH v1 1/6] rcu: Add support for consolidated-RCU reader checking
From: Joel Fernandes @ 2019-07-12 15:10 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, Alexey Kuznetsov, Bjorn Helgaas, Borislav Petkov,
c0d1n61at3, David S. Miller, edumazet, Greg Kroah-Hartman,
Hideaki YOSHIFUJI, H. Peter Anvin, Ingo Molnar, Josh Triplett,
keescook, kernel-hardening, Lai Jiangshan, Len Brown, linux-acpi,
linux-pci, linux-pm, Mathieu Desnoyers, neilb, netdev, oleg,
Paul E. McKenney, Pavel Machek, Rafael J. Wysocki,
Rasmus Villemoes, rcu, Steven Rostedt, Tejun Heo, Thomas Gleixner,
will, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
In-Reply-To: <20190712111125.GT3402@hirez.programming.kicks-ass.net>
On Fri, Jul 12, 2019 at 01:11:25PM +0200, Peter Zijlstra wrote:
> On Thu, Jul 11, 2019 at 07:43:56PM -0400, Joel Fernandes (Google) wrote:
> > +int rcu_read_lock_any_held(void)
> > +{
> > + int lockdep_opinion = 0;
> > +
> > + if (!debug_lockdep_rcu_enabled())
> > + return 1;
> > + if (!rcu_is_watching())
> > + return 0;
> > + if (!rcu_lockdep_current_cpu_online())
> > + return 0;
> > +
> > + /* Preemptible RCU flavor */
> > + if (lock_is_held(&rcu_lock_map))
>
> you forgot debug_locks here.
Actually, it turns out debug_locks checking is not even needed. If
debug_locks == 0, then debug_lockdep_rcu_enabled() returns 0 and we would not
get to this point.
> > + return 1;
> > +
> > + /* BH flavor */
> > + if (in_softirq() || irqs_disabled())
>
> I'm not sure I'd put irqs_disabled() under BH, also this entire
> condition is superfluous, see below.
>
> > + return 1;
> > +
> > + /* Sched flavor */
> > + if (debug_locks)
> > + lockdep_opinion = lock_is_held(&rcu_sched_lock_map);
> > + return lockdep_opinion || !preemptible();
>
> that !preemptible() turns into:
>
> !(preempt_count()==0 && !irqs_disabled())
>
> which is:
>
> preempt_count() != 0 || irqs_disabled()
>
> and already includes irqs_disabled() and in_softirq().
>
> > +}
>
> So maybe something lke:
>
> if (debug_locks && (lock_is_held(&rcu_lock_map) ||
> lock_is_held(&rcu_sched_lock_map)))
> return true;
Agreed, I will do it this way (without the debug_locks) like:
---8<-----------------------
diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c
index ba861d1716d3..339aebc330db 100644
--- a/kernel/rcu/update.c
+++ b/kernel/rcu/update.c
@@ -296,27 +296,15 @@ EXPORT_SYMBOL_GPL(rcu_read_lock_bh_held);
int rcu_read_lock_any_held(void)
{
- int lockdep_opinion = 0;
-
if (!debug_lockdep_rcu_enabled())
return 1;
if (!rcu_is_watching())
return 0;
if (!rcu_lockdep_current_cpu_online())
return 0;
-
- /* Preemptible RCU flavor */
- if (lock_is_held(&rcu_lock_map))
- return 1;
-
- /* BH flavor */
- if (in_softirq() || irqs_disabled())
- return 1;
-
- /* Sched flavor */
- if (debug_locks)
- lockdep_opinion = lock_is_held(&rcu_sched_lock_map);
- return lockdep_opinion || !preemptible();
+ if (lock_is_held(&rcu_lock_map) || lock_is_held(&rcu_sched_lock_map))
+ return 1;
+ return !preemptible();
}
EXPORT_SYMBOL_GPL(rcu_read_lock_any_held);
^ permalink raw reply related
* Re: [PATCH 0/2] Fold checksum at the end of bpf_csum_diff and fix
From: Daniel Borkmann @ 2019-07-12 15:15 UTC (permalink / raw)
To: Andrii Nakryiko, Paolo Pisati
Cc: 20190710231439.GD32439, Alexei Starovoitov, Martin KaFai Lau,
Song Liu, Yonghong Song, David S . Miller, Shuah Khan,
Jakub Kicinski, Jiong Wang, Networking, bpf,
open list:KERNEL SELFTEST FRAMEWORK, open list
In-Reply-To: <CAEf4BzbGLmuZ48vFUCrDW6VC7_YrkW_0NpgpgXNQEzF_dEqgnA@mail.gmail.com>
On 07/12/2019 01:50 AM, Andrii Nakryiko wrote:
> On Thu, Jul 11, 2019 at 2:32 AM Paolo Pisati <p.pisati@gmail.com> wrote:
>> From: Paolo Pisati <paolo.pisati@canonical.com>
>>
>> After applying patch 0001, all checksum implementations i could test (x86-64, arm64 and
>> arm), now agree on the return value.
>>
>> Patch 0002 fix the expected return value for test #13: i did the calculation manually,
>> and it correspond.
>>
>> Unfortunately, after applying patch 0001, other test cases now fail in
>> test_verifier:
Thanks for catching, sigh. :/
>> $ sudo ./tools/testing/selftests/bpf/test_verifier
>> ...
>> #417/p helper access to variable memory: size = 0 allowed on NULL (ARG_PTR_TO_MEM_OR_NULL) FAIL retval 65535 != 0
>> #419/p helper access to variable memory: size = 0 allowed on != NULL stack pointer (ARG_PTR_TO_MEM_OR_NULL) FAIL retval 65535 != 0
>> #423/p helper access to variable memory: size possible = 0 allowed on != NULL packet pointer (ARG_PTR_TO_MEM_OR_NULL) FAIL retval 65535 != 0
>
> I'm not entirely sure this fix is correct, given these failures, to be honest.
>
> Let's wait for someone who understands intended semantics for
> bpf_csum_diff, before changing returned value so drastically.
>
> But in any case, fixes for these test failures should be in your patch
> series as well.
Your change would actually break applications. The bpf_csum_diff() helper is
heavily used with cascading so one result can be fed into another bpf_csum_diff()
call as seed. Quick test on x86-64:
static int __init foo(void)
{
u8 data[32 * sizeof(u32)];
u32 res1, res2, res3;
int i;
prandom_bytes(data, sizeof(data));
res1 = csum_fold(csum_partial(data, sizeof(data), 0));
for (i = sizeof(u32); i < sizeof(data); i += sizeof(u32)) {
res2 = csum_fold(csum_partial(data, i, 0));
res2 = csum_fold(csum_partial(data+i, sizeof(data)-i, res2));
res3 = csum_partial(data, i, 0);
res3 = csum_fold(csum_partial(data+i, sizeof(data)-i, res3));
printk("%8d: [%4x (reference), %4x (unfolded), %4x (folded)]\n", i, res1, res3, res2);
}
return -1;
}
Gives for all three:
[19113.233942] 4: [6b70 (reference), 6b70 (unfolded), 223d (folded)]
[19113.233943] 8: [6b70 (reference), 6b70 (unfolded), a812 (folded)]
[19113.233943] 12: [6b70 (reference), 6b70 (unfolded), 1c26 (folded)]
[19113.233944] 16: [6b70 (reference), 6b70 (unfolded), 4f76 (folded)]
[19113.233944] 20: [6b70 (reference), 6b70 (unfolded), 2801 (folded)]
[19113.233945] 24: [6b70 (reference), 6b70 (unfolded), b63 (folded)]
[19113.233945] 28: [6b70 (reference), 6b70 (unfolded), 2fe0 (folded)]
[19113.233946] 32: [6b70 (reference), 6b70 (unfolded), 18a2 (folded)]
[19113.233946] 36: [6b70 (reference), 6b70 (unfolded), 2597 (folded)]
[19113.233947] 40: [6b70 (reference), 6b70 (unfolded), 2f8e (folded)]
[19113.233947] 44: [6b70 (reference), 6b70 (unfolded), b8af (folded)]
[19113.233948] 48: [6b70 (reference), 6b70 (unfolded), fb8b (folded)]
[19113.233948] 52: [6b70 (reference), 6b70 (unfolded), e9c0 (folded)]
[19113.233949] 56: [6b70 (reference), 6b70 (unfolded), 6af1 (folded)]
[19113.233949] 60: [6b70 (reference), 6b70 (unfolded), d7f4 (folded)]
[19113.233949] 64: [6b70 (reference), 6b70 (unfolded), 8bc6 (folded)]
[19113.233950] 68: [6b70 (reference), 6b70 (unfolded), 8718 (folded)]
[19113.233950] 72: [6b70 (reference), 6b70 (unfolded), 27d8 (folded)]
[19113.233951] 76: [6b70 (reference), 6b70 (unfolded), a2db (folded)]
[19113.233952] 80: [6b70 (reference), 6b70 (unfolded), 3fd (folded)]
[19113.233952] 84: [6b70 (reference), 6b70 (unfolded), 4be5 (folded)]
[19113.233952] 88: [6b70 (reference), 6b70 (unfolded), 41ad (folded)]
[19113.233953] 92: [6b70 (reference), 6b70 (unfolded), ca9b (folded)]
[19113.233953] 96: [6b70 (reference), 6b70 (unfolded), f8ec (folded)]
[19113.233954] 100: [6b70 (reference), 6b70 (unfolded), 5451 (folded)]
[19113.233954] 104: [6b70 (reference), 6b70 (unfolded), 763 (folded)]
[19113.233955] 108: [6b70 (reference), 6b70 (unfolded), e37c (folded)]
[19113.233955] 112: [6b70 (reference), 6b70 (unfolded), 4ee6 (folded)]
[19113.233956] 116: [6b70 (reference), 6b70 (unfolded), 4f73 (folded)]
[19113.233956] 120: [6b70 (reference), 6b70 (unfolded), 1cfd (folded)]
[19113.233957] 124: [6b70 (reference), 6b70 (unfolded), 7d1a (folded)]
I'll take a look next week wrt fixing this uniformly for all archs.
Thanks,
Daniel
^ permalink raw reply
* Re: linux-next: build failure after merge of the net-next tree
From: Jason Gunthorpe @ 2019-07-12 15:21 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Bernard Metzler, Leon Romanovsky, Doug Ledford, David Miller,
Networking, Linux Next Mailing List, Linux Kernel Mailing List
In-Reply-To: <20190712114557.2b094876@canb.auug.org.au>
On Fri, Jul 12, 2019 at 11:45:57AM +1000, Stephen Rothwell wrote:
> Hi Jason,
>
> On Thu, 11 Jul 2019 14:33:07 +0000 Jason Gunthorpe <jgg@mellanox.com> wrote:
> >
> > I've added this patch to the rdma tree to fix the missing locking.
> >
> > The merge resolution will be simply swapping
> > for_ifa to in_dev_for_each_ifa_rtnl.
>
> OK, I added the below merge resolution patch to the merge of the rdma
> tree today (since Linus' has merged the net-next tree now).
Yes, this is what I'm planning on using
Thanks,
Jason
^ permalink raw reply
* Re: [PATCH net] net: fix use-after-free in __netif_receive_skb_core
From: Edward Cree @ 2019-07-12 15:29 UTC (permalink / raw)
To: Sabrina Dubroca; +Cc: netdev, Andreas Steinmetz
In-Reply-To: <20190710224724.GA28254@bistromath.localdomain>
On 10/07/2019 23:47, Sabrina Dubroca wrote:
> 2019-07-10, 16:07:43 +0100, Edward Cree wrote:
>> On 10/07/2019 14:52, Sabrina Dubroca wrote:
>>> -static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc,
>>> +static int __netif_receive_skb_core(struct sk_buff **pskb, bool pfmemalloc,
>>> struct packet_type **ppt_prev)
>>> {
>>> struct packet_type *ptype, *pt_prev;
>>> rx_handler_func_t *rx_handler;
>>> + struct sk_buff *skb = *pskb;
>> Would it not be simpler just to change all users of skb to *pskb?
>> Then you avoid having to keep doing "*pskb = skb;" whenever skb changes
>> (with concomitant risk of bugs if one gets missed).
> Yes, that would be less risky. I wrote a version of the patch that did
> exactly that, but found it really too ugly (both the patch and the
> resulting code).
If you've still got that version (or can dig it out of your reflog), can
you post it so we can see just how ugly it turns out?
> We have more than 50 occurences of skb, including
> things like:
>
> atomic_long_inc(&skb->dev->rx_dropped);
Ooh, yes, I can see why that ends up looking funny...
Here's a thought, how about switching round the return-vs-pass-by-pointer
and writing:
static struct sk_buff *__netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc,
struct packet_type **ppt_prev, int *ret)
?
(Although, you might want to rename 'ret' in that case.)
Does that make things any less ugly?
-Ed
^ permalink raw reply
* [PATCH] net: dsa: qca8k: replace legacy gpio include
From: Christian Lamparter @ 2019-07-12 15:33 UTC (permalink / raw)
To: netdev
Cc: David S . Miller, Florian Fainelli, Vivien Didelot, Andrew Lunn,
kbuild test robot
This patch replaces the legacy bulk gpio.h include
with the proper gpio/consumer.h variant. This was
caught by the kbuild test robot that was running
into an error because of this.
For more information why linux/gpio.h is bad can be found in:
commit 56a46b6144e7 ("gpio: Clarify that <linux/gpio.h> is legacy")
Reported-by: kbuild test robot <lkp@intel.com>
Link: https://www.spinics.net/lists/netdev/msg584447.html
Fixes: a653f2f538f9 ("net: dsa: qca8k: introduce reset via gpio feature")
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
---
drivers/net/dsa/qca8k.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 27709f866c23..232e8cc96f6d 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -14,7 +14,7 @@
#include <linux/of_platform.h>
#include <linux/if_bridge.h>
#include <linux/mdio.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
#include <linux/etherdevice.h>
#include "qca8k.h"
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v2 bpf-next 1/3] bpf: fix BTF verifier size resolution logic
From: Andrii Nakryiko @ 2019-07-12 15:36 UTC (permalink / raw)
To: Yonghong Song
Cc: Andrii Nakryiko, bpf@vger.kernel.org, netdev@vger.kernel.org,
Alexei Starovoitov, daniel@iogearbox.net, Kernel Team, Martin Lau
In-Reply-To: <ad29872e-a127-f21e-5581-03df5a388a55@fb.com>
On Thu, Jul 11, 2019 at 10:59 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 7/10/19 11:53 PM, Andrii Nakryiko wrote:
> > BTF verifier has a size resolution bug which in some circumstances leads to
> > invalid size resolution for, e.g., TYPEDEF modifier. This happens if we have
> > [1] PTR -> [2] TYPEDEF -> [3] ARRAY, in which case due to being in pointer
> > context ARRAY size won't be resolved (because for pointer it doesn't matter, so
> > it's a sink in pointer context), but it will be permanently remembered as zero
> > for TYPEDEF and TYPEDEF will be marked as RESOLVED. Eventually ARRAY size will
> > be resolved correctly, but TYPEDEF resolved_size won't be updated anymore.
> > This, subsequently, will lead to erroneous map creation failure, if that
> > TYPEDEF is specified as either key or value, as key_size/value_size won't
> > correspond to resolved size of TYPEDEF (kernel will believe it's zero).
> >
> > Note, that if BTF was ordered as [1] ARRAY <- [2] TYPEDEF <- [3] PTR, this
> > won't be a problem, as by the time we get to TYPEDEF, ARRAY's size is already
> > calculated and stored.
> >
> > This bug manifests itself in rejecting BTF-defined maps that use array
> > typedef as a value type:
> >
> > typedef int array_t[16];
> >
> > struct {
> > __uint(type, BPF_MAP_TYPE_ARRAY);
> > __type(value, array_t); /* i.e., array_t *value; */
> > } test_map SEC(".maps");
> >
> > The fix consists on not relying on modifier's resolved_size and instead using
> > modifier's resolved_id (type ID for "concrete" type to which modifier
> > eventually resolves) and doing size determination for that resolved type. This
> > allow to preserve existing "early DFS termination" logic for PTR or
> > STRUCT_OR_ARRAY contexts, but still do correct size determination for modifier
> > types.
> >
> > Fixes: eb3f595dab40 ("bpf: btf: Validate type reference")
> > Cc: Martin KaFai Lau <kafai@fb.com>
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> > kernel/bpf/btf.c | 14 ++++++++++----
> > 1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> > index cad09858a5f2..22fe8b155e51 100644
> > --- a/kernel/bpf/btf.c
> > +++ b/kernel/bpf/btf.c
> > @@ -1073,11 +1073,18 @@ const struct btf_type *btf_type_id_size(const struct btf *btf,
> > !btf_type_is_var(size_type)))
> > return NULL;
> >
> > - size = btf->resolved_sizes[size_type_id];
> > size_type_id = btf->resolved_ids[size_type_id];
> > size_type = btf_type_by_id(btf, size_type_id);
> > if (btf_type_nosize_or_null(size_type))
> > return NULL;
> > + else if (btf_type_has_size(size_type))
> > + size = size_type->size;
> > + else if (btf_type_is_array(size_type))
> > + size = btf->resolved_sizes[size_type_id];
> > + else if (btf_type_is_ptr(size_type))
> > + size = sizeof(void *);
> > + else
> > + return NULL;
>
> Looks good to me. Not sure whether we need to do any adjustment for
> var kind or not. Maybe we can do similar change in btf_var_resolve()
I don't think btf_var_resolve() needs any change. btf_var_resolve
can't be referenced by modifier, so it doesn't have any problem. It's
similar to array in that it's size will be determined correctly.
But I think btf_type_id_size() doesn't handle var case correctly, I'll do
+ else if (btf_type_is_array(size_type) ||
btf_type_is_var(size_type))
+ size = btf->resolved_sizes[size_type_id];
to fix that.
> to btf_modifier_resolve()? But I do not think it impacts correctness
> similar to btf_modifier_resolve() below as you changed
> btf_type_id_size() implementation in the above.
>
> > }
> >
> > *type_id = size_type_id;
> > @@ -1602,7 +1609,6 @@ static int btf_modifier_resolve(struct btf_verifier_env *env,
> > const struct btf_type *next_type;
> > u32 next_type_id = t->type;
> > struct btf *btf = env->btf;
> > - u32 next_type_size = 0;
> >
> > next_type = btf_type_by_id(btf, next_type_id);
> > if (!next_type || btf_type_is_resolve_source_only(next_type)) {
> > @@ -1620,7 +1626,7 @@ static int btf_modifier_resolve(struct btf_verifier_env *env,
> > * save us a few type-following when we use it later (e.g. in
> > * pretty print).
> > */
> > - if (!btf_type_id_size(btf, &next_type_id, &next_type_size)) {
> > + if (!btf_type_id_size(btf, &next_type_id, NULL)) {
> > if (env_type_is_resolved(env, next_type_id))
> > next_type = btf_type_id_resolve(btf, &next_type_id);
> >
> > @@ -1633,7 +1639,7 @@ static int btf_modifier_resolve(struct btf_verifier_env *env,
> > }
> > }
> >
> > - env_stack_pop_resolved(env, next_type_id, next_type_size);
> > + env_stack_pop_resolved(env, next_type_id, 0);
> >
> > return 0;
> > }
> >
^ permalink raw reply
* Re: [PATCH v2 bpf-next 0/3] fix BTF verification size resolution
From: Andrii Nakryiko @ 2019-07-12 15:42 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Yonghong Song, Andrii Nakryiko, bpf@vger.kernel.org,
netdev@vger.kernel.org, Alexei Starovoitov, Kernel Team
In-Reply-To: <bf74b176-9321-c175-359d-4c5cf58a72b4@iogearbox.net>
On Fri, Jul 12, 2019 at 5:59 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 07/12/2019 08:03 AM, Yonghong Song wrote:
> > On 7/10/19 11:53 PM, Andrii Nakryiko wrote:
> >> BTF size resolution logic isn't always resolving type size correctly, leading
> >> to erroneous map creation failures due to value size mismatch.
> >>
> >> This patch set:
> >> 1. fixes the issue (patch #1);
> >> 2. adds tests for trickier cases (patch #2);
> >> 3. and converts few test cases utilizing BTF-defined maps, that previously
> >> couldn't use typedef'ed arrays due to kernel bug (patch #3).
> >>
> >> Patch #1 can be applied against bpf tree, but selftest ones (#2 and #3) have
> >> to go against bpf-next for now.
> >
> > Why #2 and #3 have to go to bpf-next? bpf tree also accepts tests,
> > AFAIK. Maybe leave for Daniel and Alexei to decide in this particular case.
>
> Yes, corresponding test cases for fixes are also accepted for bpf tree, thanks.
Thanks for merging, Daniel! My thinking was that at the time I posted
patch set, BTF-defined map tests weren't yet merged into bpf, so I
assumed it has to go against bpf-next.
>
> >> Andrii Nakryiko (3):
> >> bpf: fix BTF verifier size resolution logic
> >> selftests/bpf: add trickier size resolution tests
> >> selftests/bpf: use typedef'ed arrays as map values
> >
> > Looks good to me. Except minor comments in patch 1/3, Ack the series.
> > Acked-by: Yonghong Song <yhs@fb.com>
> >
> >>
> >> kernel/bpf/btf.c | 14 ++-
> >> .../bpf/progs/test_get_stack_rawtp.c | 3 +-
> >> .../bpf/progs/test_stacktrace_build_id.c | 3 +-
> >> .../selftests/bpf/progs/test_stacktrace_map.c | 2 +-
> >> tools/testing/selftests/bpf/test_btf.c | 88 +++++++++++++++++++
> >> 5 files changed, 102 insertions(+), 8 deletions(-)
> >>
>
^ permalink raw reply
* Re: [PATCH bpf-next] net: Don't uninstall an XDP program when none is installed
From: Daniel Borkmann @ 2019-07-12 15:44 UTC (permalink / raw)
To: Maxim Mikityanskiy, Alexei Starovoitov, David S. Miller
Cc: Björn Töpel, bpf@vger.kernel.org,
netdev@vger.kernel.org, Saeed Mahameed, Jakub Kicinski,
Jesper Dangaard Brouer, John Fastabend, Martin KaFai Lau,
Song Liu, Yonghong Song
In-Reply-To: <3124b473-1322-e98e-d5ab-60e584e74200@mellanox.com>
On 07/10/2019 01:16 PM, Maxim Mikityanskiy wrote:
> On 2019-06-12 19:14, Maxim Mikityanskiy wrote:
>> dev_change_xdp_fd doesn't perform any checks in case it uninstalls an
>> XDP program. It means that the driver's ndo_bpf can be called with
>> XDP_SETUP_PROG asking to set it to NULL even if it's already NULL. This
>> case happens if the user runs `ip link set eth0 xdp off` when there is
>> no XDP program attached.
>>
>> The drivers typically perform some heavy operations on XDP_SETUP_PROG,
>> so they all have to handle this case internally to return early if it
>> happens. This patch puts this check into the kernel code, so that all
>> drivers will benefit from it.
>>
>> Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
>> ---
>> Björn, please take a look at this, Saeed told me you were doing
>> something related, but I couldn't find it. If this fix is already
>> covered by your work, please tell about that.
>>
>> net/core/dev.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 66f7508825bd..68b3e3320ceb 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -8089,6 +8089,9 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
>> bpf_prog_put(prog);
>> return -EINVAL;
>> }
>> + } else {
>> + if (!__dev_xdp_query(dev, bpf_op, query))
>> + return 0;
>> }
>>
>> err = dev_xdp_install(dev, bpf_op, extack, flags, prog);
>>
>
> Alexei, so what about this patch? It's marked as "Changed Requested" in
> patchwork, but Jakub's point looks resolved - I don't see any changes
> required from my side.
I believe part of Jakub's feedback was that if we make this generic that this
does not generally address the case where both prog pointers are equal (whether
NULL or non-NULL).
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH bpf-next] selftests/bpf: remove logic duplication in test_verifier.c
From: Andrii Nakryiko @ 2019-07-12 15:45 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Krzesimir Nowak, Andrii Nakryiko, Kernel Team, Alexei Starovoitov,
bpf, Networking
In-Reply-To: <d6ff6022-56f7-de63-d3e1-8949360296ca@iogearbox.net>
On Fri, Jul 12, 2019 at 6:57 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 07/12/2019 09:53 AM, Krzesimir Nowak wrote:
> > On Thu, Jul 11, 2019 at 4:43 PM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> >>
> >> On Thu, Jul 11, 2019 at 5:13 AM Krzesimir Nowak <krzesimir@kinvolk.io> wrote:
> >>>
> >>> On Thu, Jul 11, 2019 at 3:08 AM Andrii Nakryiko <andriin@fb.com> wrote:
> >>>>
> >>>> test_verifier tests can specify single- and multi-runs tests. Internally
> >>>> logic of handling them is duplicated. Get rid of it by making single run
> >>>> retval specification to be a first retvals spec.
> >>>>
> >>>> Cc: Krzesimir Nowak <krzesimir@kinvolk.io>
> >>>> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> >>>
> >>> Looks good, one nit below.
> >>>
> >>> Acked-by: Krzesimir Nowak <krzesimir@kinvolk.io>
> >>>
> >>>> ---
> >>>> tools/testing/selftests/bpf/test_verifier.c | 37 ++++++++++-----------
> >>>> 1 file changed, 18 insertions(+), 19 deletions(-)
> >>>>
> >>>> diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
> >>>> index b0773291012a..120ecdf4a7db 100644
> >>>> --- a/tools/testing/selftests/bpf/test_verifier.c
> >>>> +++ b/tools/testing/selftests/bpf/test_verifier.c
> >>>> @@ -86,7 +86,7 @@ struct bpf_test {
> >>>> int fixup_sk_storage_map[MAX_FIXUPS];
> >>>> const char *errstr;
> >>>> const char *errstr_unpriv;
> >>>> - uint32_t retval, retval_unpriv, insn_processed;
> >>>> + uint32_t insn_processed;
> >>>> int prog_len;
> >>>> enum {
> >>>> UNDEF,
> >>>> @@ -95,16 +95,24 @@ struct bpf_test {
> >>>> } result, result_unpriv;
> >>>> enum bpf_prog_type prog_type;
> >>>> uint8_t flags;
> >>>> - __u8 data[TEST_DATA_LEN];
> >>>> void (*fill_helper)(struct bpf_test *self);
> >>>> uint8_t runs;
> >>>> - struct {
> >>>> - uint32_t retval, retval_unpriv;
> >>>> - union {
> >>>> - __u8 data[TEST_DATA_LEN];
> >>>> - __u64 data64[TEST_DATA_LEN / 8];
> >>>> + union {
> >>>> + struct {
> >>>
> >>> Maybe consider moving the struct definition outside to further the
> >>> removal of the duplication?
> >>
> >> Can't do that because then retval/retval_unpriv/data won't be
> >> accessible as a normal field of struct bpf_test. It has to be in
> >> anonymous structs/unions, unfortunately.
> >>
> >
> > Ah, right.
> >
> > Meh.
> >
> > I tried something like this:
> >
> > #define BPF_DATA_STRUCT \
> > struct { \
> > uint32_t retval, retval_unpriv; \
> > union { \
> > __u8 data[TEST_DATA_LEN]; \
> > __u64 data64[TEST_DATA_LEN / 8]; \
> > }; \
> > }
> >
> > and then:
> >
> > union {
> > BPF_DATA_STRUCT;
> > BPF_DATA_STRUCT retvals[MAX_TEST_RUNS];
> > };
> >
> > And that seems to compile at least. But question is: is this
> > acceptably ugly or unacceptably ugly? :)
>
> Both a bit ugly, but I'd have a slight preference towards the above,
> perhaps a bit more readable like:
Heh, I had slight preference the other way :) I'll update diff with
macro, though.
>
> #define bpf_testdata_struct_t \
> struct { \
> uint32_t retval, retval_unpriv; \
> union { \
> __u8 data[TEST_DATA_LEN]; \
> __u64 data64[TEST_DATA_LEN / 8]; \
> }; \
> }
> union {
> bpf_testdata_struct_t;
> bpf_testdata_struct_t retvals[MAX_TEST_RUNS];
> };
>
> Thanks,
> Daniel
^ permalink raw reply
* Re: [PATCH v2 bpf-next 1/3] bpf: fix BTF verifier size resolution logic
From: Yonghong Song @ 2019-07-12 15:47 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Andrii Nakryiko, bpf@vger.kernel.org, netdev@vger.kernel.org,
Alexei Starovoitov, daniel@iogearbox.net, Kernel Team, Martin Lau
In-Reply-To: <CAEf4Bzb4vzwRVPegF51Kv6oqTXUAWqnhK-jAVs8SESyh74+XTA@mail.gmail.com>
On 7/12/19 8:36 AM, Andrii Nakryiko wrote:
> On Thu, Jul 11, 2019 at 10:59 PM Yonghong Song <yhs@fb.com> wrote:
>>
>>
>>
>> On 7/10/19 11:53 PM, Andrii Nakryiko wrote:
>>> BTF verifier has a size resolution bug which in some circumstances leads to
>>> invalid size resolution for, e.g., TYPEDEF modifier. This happens if we have
>>> [1] PTR -> [2] TYPEDEF -> [3] ARRAY, in which case due to being in pointer
>>> context ARRAY size won't be resolved (because for pointer it doesn't matter, so
>>> it's a sink in pointer context), but it will be permanently remembered as zero
>>> for TYPEDEF and TYPEDEF will be marked as RESOLVED. Eventually ARRAY size will
>>> be resolved correctly, but TYPEDEF resolved_size won't be updated anymore.
>>> This, subsequently, will lead to erroneous map creation failure, if that
>>> TYPEDEF is specified as either key or value, as key_size/value_size won't
>>> correspond to resolved size of TYPEDEF (kernel will believe it's zero).
>>>
>>> Note, that if BTF was ordered as [1] ARRAY <- [2] TYPEDEF <- [3] PTR, this
>>> won't be a problem, as by the time we get to TYPEDEF, ARRAY's size is already
>>> calculated and stored.
>>>
>>> This bug manifests itself in rejecting BTF-defined maps that use array
>>> typedef as a value type:
>>>
>>> typedef int array_t[16];
>>>
>>> struct {
>>> __uint(type, BPF_MAP_TYPE_ARRAY);
>>> __type(value, array_t); /* i.e., array_t *value; */
>>> } test_map SEC(".maps");
>>>
>>> The fix consists on not relying on modifier's resolved_size and instead using
>>> modifier's resolved_id (type ID for "concrete" type to which modifier
>>> eventually resolves) and doing size determination for that resolved type. This
>>> allow to preserve existing "early DFS termination" logic for PTR or
>>> STRUCT_OR_ARRAY contexts, but still do correct size determination for modifier
>>> types.
>>>
>>> Fixes: eb3f595dab40 ("bpf: btf: Validate type reference")
>>> Cc: Martin KaFai Lau <kafai@fb.com>
>>> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>>> ---
>>> kernel/bpf/btf.c | 14 ++++++++++----
>>> 1 file changed, 10 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
>>> index cad09858a5f2..22fe8b155e51 100644
>>> --- a/kernel/bpf/btf.c
>>> +++ b/kernel/bpf/btf.c
>>> @@ -1073,11 +1073,18 @@ const struct btf_type *btf_type_id_size(const struct btf *btf,
>>> !btf_type_is_var(size_type)))
>>> return NULL;
>>>
>>> - size = btf->resolved_sizes[size_type_id];
>>> size_type_id = btf->resolved_ids[size_type_id];
>>> size_type = btf_type_by_id(btf, size_type_id);
>>> if (btf_type_nosize_or_null(size_type))
>>> return NULL;
>>> + else if (btf_type_has_size(size_type))
>>> + size = size_type->size;
>>> + else if (btf_type_is_array(size_type))
>>> + size = btf->resolved_sizes[size_type_id];
>>> + else if (btf_type_is_ptr(size_type))
>>> + size = sizeof(void *);
>>> + else
>>> + return NULL;
>>
>> Looks good to me. Not sure whether we need to do any adjustment for
>> var kind or not. Maybe we can do similar change in btf_var_resolve()
>
> I don't think btf_var_resolve() needs any change. btf_var_resolve
> can't be referenced by modifier, so it doesn't have any problem. It's
> similar to array in that it's size will be determined correctly.
Correct. With your previous patch, the resolved_sizes[..] for var type
is not used, so that is why I suggest to just set it to 0.
>
> But I think btf_type_id_size() doesn't handle var case correctly, I'll do
>
> + else if (btf_type_is_array(size_type) ||
> btf_type_is_var(size_type))
> + size = btf->resolved_sizes[size_type_id];
This change should work too (to use btf->resolved_sizes[...]).
>
> to fix that.
>
>> to btf_modifier_resolve()? But I do not think it impacts correctness
>> similar to btf_modifier_resolve() below as you changed
>> btf_type_id_size() implementation in the above.
>>
>>> }
>>>
>>> *type_id = size_type_id;
>>> @@ -1602,7 +1609,6 @@ static int btf_modifier_resolve(struct btf_verifier_env *env,
>>> const struct btf_type *next_type;
>>> u32 next_type_id = t->type;
>>> struct btf *btf = env->btf;
>>> - u32 next_type_size = 0;
>>>
>>> next_type = btf_type_by_id(btf, next_type_id);
>>> if (!next_type || btf_type_is_resolve_source_only(next_type)) {
>>> @@ -1620,7 +1626,7 @@ static int btf_modifier_resolve(struct btf_verifier_env *env,
>>> * save us a few type-following when we use it later (e.g. in
>>> * pretty print).
>>> */
>>> - if (!btf_type_id_size(btf, &next_type_id, &next_type_size)) {
>>> + if (!btf_type_id_size(btf, &next_type_id, NULL)) {
>>> if (env_type_is_resolved(env, next_type_id))
>>> next_type = btf_type_id_resolve(btf, &next_type_id);
>>>
>>> @@ -1633,7 +1639,7 @@ static int btf_modifier_resolve(struct btf_verifier_env *env,
>>> }
>>> }
>>>
>>> - env_stack_pop_resolved(env, next_type_id, next_type_size);
>>> + env_stack_pop_resolved(env, next_type_id, 0);
>>>
>>> return 0;
>>> }
>>>
^ permalink raw reply
* Re: [PATCH v2 bpf-next 0/3] fix BTF verification size resolution
From: Daniel Borkmann @ 2019-07-12 15:51 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Yonghong Song, Andrii Nakryiko, bpf@vger.kernel.org,
netdev@vger.kernel.org, Alexei Starovoitov, Kernel Team
In-Reply-To: <CAEf4BzY-7MLt8hvBqMMdhpAq3ih_KFjgWitN3TSf74FypeAPRw@mail.gmail.com>
On 07/12/2019 05:42 PM, Andrii Nakryiko wrote:
> On Fri, Jul 12, 2019 at 5:59 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>> On 07/12/2019 08:03 AM, Yonghong Song wrote:
>>> On 7/10/19 11:53 PM, Andrii Nakryiko wrote:
>>>> BTF size resolution logic isn't always resolving type size correctly, leading
>>>> to erroneous map creation failures due to value size mismatch.
>>>>
>>>> This patch set:
>>>> 1. fixes the issue (patch #1);
>>>> 2. adds tests for trickier cases (patch #2);
>>>> 3. and converts few test cases utilizing BTF-defined maps, that previously
>>>> couldn't use typedef'ed arrays due to kernel bug (patch #3).
>>>>
>>>> Patch #1 can be applied against bpf tree, but selftest ones (#2 and #3) have
>>>> to go against bpf-next for now.
>>>
>>> Why #2 and #3 have to go to bpf-next? bpf tree also accepts tests,
>>> AFAIK. Maybe leave for Daniel and Alexei to decide in this particular case.
>>
>> Yes, corresponding test cases for fixes are also accepted for bpf tree, thanks.
>
> Thanks for merging, Daniel! My thinking was that at the time I posted
> patch set, BTF-defined map tests weren't yet merged into bpf, so I
> assumed it has to go against bpf-next.
Not yet merged given the minor change needed resulting from Yonghong's feedback.
Thanks,
Daniel
^ permalink raw reply
* Re: [RFC PATCH net-next 0/3] net: batched receive in GRO path
From: Edward Cree @ 2019-07-12 15:59 UTC (permalink / raw)
To: Eric Dumazet, Paolo Abeni, David Miller; +Cc: netdev
In-Reply-To: <38ff0ce0-7e26-1683-90f0-adc9c0ac9abe@gmail.com>
On 10/07/2019 18:39, Eric Dumazet wrote:
> Holding a small packet in the list up to the point we call busy_poll_stop()
> will basically make busypoll non working anymore.
>
> napi_complete_done() has special behavior when busy polling is active.
Yep, I get it now, sorry for being dumb :)
Essentially we're saying that things coalesced by GRO are 'bulk' traffic and
can wait around, but the rest is the stuff we're polling for for low latency.
I'm putting a gro_normal_list() call after the trace_napi_poll() in
napi_busy_loop() and testing that, let's see how it goes...
^ permalink raw reply
* Re: [PATCH v1 1/6] rcu: Add support for consolidated-RCU reader checking
From: Peter Zijlstra @ 2019-07-12 15:58 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, Alexey Kuznetsov, Bjorn Helgaas, Borislav Petkov,
c0d1n61at3, David S. Miller, edumazet, Greg Kroah-Hartman,
Hideaki YOSHIFUJI, H. Peter Anvin, Ingo Molnar, Josh Triplett,
keescook, kernel-hardening, Lai Jiangshan, Len Brown, linux-acpi,
linux-pci, linux-pm, Mathieu Desnoyers, neilb, netdev, oleg,
Paul E. McKenney, Pavel Machek, Rafael J. Wysocki,
Rasmus Villemoes, rcu, Steven Rostedt, Tejun Heo, Thomas Gleixner,
will, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
In-Reply-To: <20190712151051.GB235410@google.com>
On Fri, Jul 12, 2019 at 11:10:51AM -0400, Joel Fernandes wrote:
> Agreed, I will do it this way (without the debug_locks) like:
>
> ---8<-----------------------
>
> diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c
> index ba861d1716d3..339aebc330db 100644
> --- a/kernel/rcu/update.c
> +++ b/kernel/rcu/update.c
> @@ -296,27 +296,15 @@ EXPORT_SYMBOL_GPL(rcu_read_lock_bh_held);
>
> int rcu_read_lock_any_held(void)
> {
> if (!debug_lockdep_rcu_enabled())
> return 1;
> if (!rcu_is_watching())
> return 0;
> if (!rcu_lockdep_current_cpu_online())
> return 0;
> + if (lock_is_held(&rcu_lock_map) || lock_is_held(&rcu_sched_lock_map))
> + return 1;
> + return !preemptible();
> }
> EXPORT_SYMBOL_GPL(rcu_read_lock_any_held);
OK, that looks sane. Thanks!
^ permalink raw reply
* Re: [ovs-dev] [PATCH net-next] net: openvswitch: do not update max_headroom if new headroom is equal to old headroom
From: Gregory Rose @ 2019-07-12 16:18 UTC (permalink / raw)
To: Taehee Yoo, davem, pshelar, netdev, dev
In-Reply-To: <20190705160809.5202-1-ap420073@gmail.com>
On 7/5/2019 9:08 AM, Taehee Yoo wrote:
> When a vport is deleted, the maximum headroom size would be changed.
> If the vport which has the largest headroom is deleted,
> the new max_headroom would be set.
> But, if the new headroom size is equal to the old headroom size,
> updating routine is unnecessary.
>
> Signed-off-by: Taehee Yoo <ap420073@gmail.com>
> ---
> net/openvswitch/datapath.c | 39 +++++++++++++++++++++++++++-----------
> 1 file changed, 28 insertions(+), 11 deletions(-)
>
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index 33b388103741..892287d06c17 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -1958,10 +1958,9 @@ static struct vport *lookup_vport(struct net *net,
>
> }
>
> -/* Called with ovs_mutex */
> -static void update_headroom(struct datapath *dp)
> +static unsigned int ovs_get_max_headroom(struct datapath *dp)
> {
> - unsigned dev_headroom, max_headroom = 0;
> + unsigned int dev_headroom, max_headroom = 0;
> struct net_device *dev;
> struct vport *vport;
> int i;
> @@ -1975,10 +1974,19 @@ static void update_headroom(struct datapath *dp)
> }
> }
>
> - dp->max_headroom = max_headroom;
> + return max_headroom;
> +}
> +
> +/* Called with ovs_mutex */
> +static void ovs_update_headroom(struct datapath *dp, unsigned int new_headroom)
> +{
> + struct vport *vport;
> + int i;
> +
> + dp->max_headroom = new_headroom;
> for (i = 0; i < DP_VPORT_HASH_BUCKETS; i++)
> hlist_for_each_entry_rcu(vport, &dp->ports[i], dp_hash_node)
> - netdev_set_rx_headroom(vport->dev, max_headroom);
> + netdev_set_rx_headroom(vport->dev, new_headroom);
> }
>
> static int ovs_vport_cmd_new(struct sk_buff *skb, struct genl_info *info)
> @@ -1989,6 +1997,7 @@ static int ovs_vport_cmd_new(struct sk_buff *skb, struct genl_info *info)
> struct sk_buff *reply;
> struct vport *vport;
> struct datapath *dp;
> + unsigned int new_headroom;
> u32 port_no;
> int err;
>
> @@ -2050,8 +2059,10 @@ static int ovs_vport_cmd_new(struct sk_buff *skb, struct genl_info *info)
> info->snd_portid, info->snd_seq, 0,
> OVS_VPORT_CMD_NEW);
>
> - if (netdev_get_fwd_headroom(vport->dev) > dp->max_headroom)
> - update_headroom(dp);
> + new_headroom = netdev_get_fwd_headroom(vport->dev);
> +
> + if (new_headroom > dp->max_headroom)
> + ovs_update_headroom(dp, new_headroom);
> else
> netdev_set_rx_headroom(vport->dev, dp->max_headroom);
>
> @@ -2122,11 +2133,12 @@ static int ovs_vport_cmd_set(struct sk_buff *skb, struct genl_info *info)
>
> static int ovs_vport_cmd_del(struct sk_buff *skb, struct genl_info *info)
> {
> - bool must_update_headroom = false;
> + bool update_headroom = false;
> struct nlattr **a = info->attrs;
> struct sk_buff *reply;
> struct datapath *dp;
> struct vport *vport;
> + unsigned int new_headroom;
> int err;
>
> reply = ovs_vport_cmd_alloc_info();
> @@ -2152,12 +2164,17 @@ static int ovs_vport_cmd_del(struct sk_buff *skb, struct genl_info *info)
> /* the vport deletion may trigger dp headroom update */
> dp = vport->dp;
> if (netdev_get_fwd_headroom(vport->dev) == dp->max_headroom)
> - must_update_headroom = true;
> + update_headroom = true;
> +
> netdev_reset_rx_headroom(vport->dev);
> ovs_dp_detach_port(vport);
>
> - if (must_update_headroom)
> - update_headroom(dp);
> + if (update_headroom) {
> + new_headroom = ovs_get_max_headroom(dp);
> +
> + if (new_headroom < dp->max_headroom)
> + ovs_update_headroom(dp, new_headroom);
> + }
> ovs_unlock();
>
> ovs_notify(&dp_vport_genl_family, reply, info);
I did a compile test and ran the OVS kernel self-test suite. Looks OK
to me.
Tested-by: Greg Rose <gvrose8192@gmail.com>
Reviewed-by: Greg Rose <gvrose8192@gmail.com>
^ permalink raw reply
* Re: [PATCH] net: dsa: qca8k: replace legacy gpio include
From: Vivien Didelot @ 2019-07-12 16:42 UTC (permalink / raw)
To: Christian Lamparter
Cc: netdev, David S . Miller, Florian Fainelli, Andrew Lunn,
kbuild test robot
In-Reply-To: <20190712153336.5018-1-chunkeey@gmail.com>
On Fri, 12 Jul 2019 17:33:36 +0200, Christian Lamparter <chunkeey@gmail.com> wrote:
> This patch replaces the legacy bulk gpio.h include
> with the proper gpio/consumer.h variant. This was
> caught by the kbuild test robot that was running
> into an error because of this.
>
> For more information why linux/gpio.h is bad can be found in:
> commit 56a46b6144e7 ("gpio: Clarify that <linux/gpio.h> is legacy")
>
> Reported-by: kbuild test robot <lkp@intel.com>
> Link: https://www.spinics.net/lists/netdev/msg584447.html
> Fixes: a653f2f538f9 ("net: dsa: qca8k: introduce reset via gpio feature")
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
Reviewed-by: Vivien Didelot <vivien.didelot@gmail.com>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox